| Message ID | 20230119171248.3882021-2-bjorn@mork.no (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | fixes for mtk_eth_soc | expand |
On Thu, Jan 19, 2023 at 06:12:46PM +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. > > 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. > > Signed-off-by: Alexander Couzens <lynxis@fe80.eu> > [ bmork: rebased and squashed into one patch ] > Signed-off-by: Bjørn Mork <bjorn@mork.no> > --- > drivers/net/ethernet/mediatek/mtk_sgmii.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/mediatek/mtk_sgmii.c b/drivers/net/ethernet/mediatek/mtk_sgmii.c > index 5c286f2c9418..481f2f1e39f5 100644 > --- a/drivers/net/ethernet/mediatek/mtk_sgmii.c > +++ b/drivers/net/ethernet/mediatek/mtk_sgmii.c > @@ -88,6 +88,10 @@ static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode, > bmcr = 0; > } > > + /* PHYA power down */ > + regmap_update_bits(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, > + SGMII_PHYA_PWD, SGMII_PHYA_PWD); > + Doing this unconditionally means that the link will drop - even when we aren't doing any reconfiguration (except changing the advertisement). That's why I made it conditional in the version of the patch I sent (which failed due to the unknown bits 3 and 0.) We should always avoid bouncing the link when there's no reason to.
"Russell King (Oracle)" <linux@armlinux.org.uk> writes: > Doing this unconditionally means that the link will drop - even when > we aren't doing any reconfiguration (except changing the advertisement). > That's why I made it conditional in the version of the patch I sent > (which failed due to the unknown bits 3 and 0.) Right. Sorry for missing that crucial point. Will fix. Bjørn
diff --git a/drivers/net/ethernet/mediatek/mtk_sgmii.c b/drivers/net/ethernet/mediatek/mtk_sgmii.c index 5c286f2c9418..481f2f1e39f5 100644 --- a/drivers/net/ethernet/mediatek/mtk_sgmii.c +++ b/drivers/net/ethernet/mediatek/mtk_sgmii.c @@ -88,6 +88,10 @@ static int mtk_pcs_config(struct phylink_pcs *pcs, unsigned int mode, bmcr = 0; } + /* PHYA power down */ + regmap_update_bits(mpcs->regmap, SGMSYS_QPHY_PWR_STATE_CTRL, + SGMII_PHYA_PWD, SGMII_PHYA_PWD); + /* Configure the underlying interface speed */ regmap_update_bits(mpcs->regmap, mpcs->ana_rgc3, RG_PHY_SPEED_3_125G, rgc3); @@ -108,9 +112,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; }