Messages in this thread | | | Date | Tue, 18 Oct 2022 17:39:01 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH] net: mtk_sgmii: implement mtk_pcs_ops |
| |
Hi,
A couple of points:
On Tue, Oct 18, 2022 at 05:35:06PM +0200, Frank Wunderlich wrote: > diff --git a/drivers/net/ethernet/mediatek/mtk_sgmii.c b/drivers/net/ethernet/mediatek/mtk_sgmii.c > index 736839c84130..9614973fd9c4 100644 > --- a/drivers/net/ethernet/mediatek/mtk_sgmii.c > +++ b/drivers/net/ethernet/mediatek/mtk_sgmii.c > @@ -122,10 +122,25 @@ static void mtk_pcs_link_up(struct phylink_pcs *pcs, unsigned int mode, > regmap_write(mpcs->regmap, SGMSYS_SGMII_MODE, val); > } > > +static void mtk_pcs_get_state(struct phylink_pcs *pcs, struct phylink_link_state *state) > +{ > + struct mtk_pcs *mpcs = pcs_to_mtk_pcs(pcs); > + unsigned int val; > + > + regmap_read(mpcs->regmap, mpcs->ana_rgc3, &val); > + state->speed = val & RG_PHY_SPEED_3_125G ? SPEED_2500 : SPEED_1000; > + > + regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &val); > + state->an_complete = !!(val & SGMII_AN_COMPLETE); > + state->link = !!(val & SGMII_LINK_STATYS); > + state->pause = 0;
Finally, something approaching a reasonable implementation for this! Two points however: 1) There's no need to set state->pause if there is no way to get that state. 2) There should also be a setting for state->pause.
> +} > + > static const struct phylink_pcs_ops mtk_pcs_ops = { > .pcs_config = mtk_pcs_config, > .pcs_an_restart = mtk_pcs_restart_an, > .pcs_link_up = mtk_pcs_link_up, > + .pcs_get_state = mtk_pcs_get_state,
Please keep this in order - pcs_get_state should be just before pcs_config.
Thanks.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
| |