On 10/15/2016 08:28 PM, Timur Tabi wrote: > Andrew Lunn wrote: >> 1) Take the SerDes power down out of the suspend code for the at803x. >> >> 2) Assume MII_PHYID1/2 registers are not guaranteed to be available >> when the PHY is powered down. So get_phy_id should first read >> MII_BMCR. If it gets 0xffff, assume there is no PHY there. If the >> PDOWN bit is set, power up the PHY. Then reading the ID registers. > > Before we take approach #1, I'd like to hear from the developer of that patch, > Zefir. According to him, that patch is necessary to fix a bug. I don't know > if > that bug exists only on his system, though. >
Hi, the problem we observed was that in SGMII mode after a suspend/resume cycle the PHY link states get out of sync (e.g. gianfar reports link up, at803x down) and power cycling the SerDes was a reliable way to get them re-synchronized again. This obviously worked for some time after March 2016 and must have stopped only recently (maybe additional PHY suspends were added). Anyway, since the SGMII reset is required, instead of reverting the patch in full I suggest to move the SGMII power down from at803x_suspend() and do a SerDes power cycle in at803x_resume(). Could you please test if the patch below fixes the problem? Thanks, Zefir --- Subject: [PATCH] at803x: don't power down SerDes on suspend Powering down SerDes renders registers inaccessible and with that re-initializing a suspended PHY fails. Instead of powering down SerDes at suspend(), leave it as is and power-cycle it on resume(). Signed-off-by: Zefir Kurtisi <zefir.kurt...@neratec.com> --- drivers/net/phy/at803x.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c index f279a89..282ffbb 100644 --- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -225,16 +225,6 @@ static int at803x_suspend(struct phy_device *phydev) phy_write(phydev, MII_BMCR, value); - if (phydev->interface != PHY_INTERFACE_MODE_SGMII) - goto done; - - /* also power-down SGMII interface */ - ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG); - phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL); - phy_write(phydev, MII_BMCR, phy_read(phydev, MII_BMCR) | BMCR_PDOWN); - phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL); - -done: mutex_unlock(&phydev->lock); return 0; @@ -254,9 +244,11 @@ static int at803x_resume(struct phy_device *phydev) if (phydev->interface != PHY_INTERFACE_MODE_SGMII) goto done; - /* also power-up SGMII interface */ + /* reset SGMII interface for re-synchronization */ ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG); phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL); + phy_write(phydev, MII_BMCR, phy_read(phydev, MII_BMCR) | BMCR_PDOWN); + value = phy_read(phydev, MII_BMCR) & ~(BMCR_PDOWN | BMCR_ISOLATE); phy_write(phydev, MII_BMCR, value); phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL); -- 2.7.4