Message ID | 20230125181602.861843-2-bjorn@mork.no (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | fixes for mtk_eth_soc | expand |
On Wed, Jan 25, 2023 at 07:16:00PM +0100, Bjørn Mork wrote: > From: Alexander Couzens <lynxis@fe80.eu> > > The code expect the PHY to be in power down which is only true after reset. > Allow changes of the SGMII parameters more than once. > > Only power down when reconfiguring to avoid bouncing the link when there's > no reason to - based on code from Russell King. > > There are cases when the SGMII_PHYA_PWD register contains 0x9 which > prevents SGMII from working. The SGMII still shows link but no traffic > can flow. Writing 0x0 to the PHYA_PWD register fix the issue. 0x0 was > taken from a good working state of the SGMII interface. > > Fixes: 42c03844e93d ("net-next: mediatek: add support for MediaTek MT7622 SoC") > Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk> > Signed-off-by: Alexander Couzens <lynxis@fe80.eu> > [ bmork: rebased and squashed into one patch ] > Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > Signed-off-by: Bjørn Mork <bjorn@mork.no> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.h | 2 ++ > drivers/net/ethernet/mediatek/mtk_sgmii.c | 39 +++++++++++++++------ > 2 files changed, 30 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h > index 18a50529ce7b..b299a7df3c30 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h > @@ -1037,10 +1037,12 @@ struct mtk_soc_data { > * SGMII modes > * @ana_rgc3: The offset refers to register ANA_RGC3 related to regmap > * @pcs: Phylink PCS structure > + * @interface: Currently configured interface mode nit: @interface should probably be above @pcs > */ > struct mtk_pcs { > struct regmap *regmap; > u32 ana_rgc3; > + phy_interface_t interface; > struct phylink_pcs pcs; > }; Thanks for tweaking the location of the interface field so there is no hole in mtk_pcs. Looks good (on x86_64).
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h index 18a50529ce7b..b299a7df3c30 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h @@ -1037,10 +1037,12 @@ struct mtk_soc_data { * SGMII modes * @ana_rgc3: The offset refers to register ANA_RGC3 related to regmap * @pcs: Phylink PCS structure + * @interface: Currently configured interface mode */ struct mtk_pcs { struct regmap *regmap; u32 ana_rgc3; + phy_interface_t interface; struct phylink_pcs pcs; }; diff --git a/drivers/net/ethernet/mediatek/mtk_sgmii.c b/drivers/net/ethernet/mediatek/mtk_sgmii.c index 5c286f2c9418..0a06995099cf 100644 --- a/drivers/net/ethernet/mediatek/mtk_sgmii.c +++ b/drivers/net/ethernet/mediatek/mtk_sgmii.c @@ -43,11 +43,6 @@ static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode, int advertise, link_timer; bool changed, use_an; - if (interface == PHY_INTERFACE_MODE_2500BASEX) - rgc3 = RG_PHY_SPEED_3_125G; - else - rgc3 = 0; - advertise = phylink_mii_c22_pcs_encode_advertisement(interface, advertising); if (advertise < 0) @@ -88,9 +83,22 @@ static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode, bmcr = 0; } - /* Configure the underlying interface speed */ - regmap_update_bits(mpcs->regmap, mpcs->ana_rgc3, - RG_PHY_SPEED_3_125G, rgc3); + if (mpcs->interface != interface) { + /* PHYA power down */ + regmap_update_bits(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, + SGMII_PHYA_PWD, SGMII_PHYA_PWD); + + if (interface == PHY_INTERFACE_MODE_2500BASEX) + rgc3 = RG_PHY_SPEED_3_125G; + else + rgc3 = 0; + + /* Configure the underlying interface speed */ + regmap_update_bits(mpcs->regmap, mpcs->ana_rgc3, + RG_PHY_SPEED_3_125G, rgc3); + + mpcs->interface = interface; + } /* Update the advertisement, noting whether it has changed */ regmap_update_bits_check(mpcs->regmap, SGMSYS_PCS_ADVERTISE, @@ -108,9 +116,17 @@ static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode, regmap_update_bits(mpcs->regmap, SGMSYS_PCS_CONTROL_1, SGMII_AN_RESTART | SGMII_AN_ENABLE, bmcr); - /* Release PHYA power down state */ - regmap_update_bits(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, - SGMII_PHYA_PWD, 0); + /* Release PHYA power down state + * Only removing bit SGMII_PHYA_PWD isn't enough. + * There are cases when the SGMII_PHYA_PWD register contains 0x9 which + * prevents SGMII from working. The SGMII still shows link but no traffic + * can flow. Writing 0x0 to the PHYA_PWD register fix the issue. 0x0 was + * taken from a good working state of the SGMII interface. + * Unknown how much the QPHY needs but it is racy without a sleep. + * Tested on mt7622 & mt7986. + */ + usleep_range(50, 100); + regmap_write(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, 0); return changed; } @@ -171,6 +187,7 @@ int mtk_sgmii_init(struct mtk_sgmii *ss, struct device_node *r, u32 ana_rgc3) return PTR_ERR(ss->pcs[i].regmap); ss->pcs[i].pcs.ops = &mtk_pcs_ops; + ss->pcs[i].interface = PHY_INTERFACE_MODE_NA; } return 0;