Re: [PATCH] net: phy: fix autoneg invalid error state of GBSR register.
From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-09-01 12:47:31
Also in:
lkml
On Wed, Sep 01, 2021 at 06:56:08PM +0800, Qiang Ma wrote:
quoted hunk
When the PHY is not link and working in polling mode, PHY state machine will periodically read GBSR register. Normally, the GBSR register value is 0. But as the log show, in very small cases the value is 0x8640. The value 0x8640 means Master/Slave resolution failed. The PHY state machine will enter PHY_HALTED state and the PHY will never be able to link. [49176.903012] [debug]: lpagb:0x0 adv:0x300 [49177.927025] [debug]: lpagb:0x8640 adv:0x300 [49177.927034] Generic PHY stmmac-18:00: Master/Slave resolution failed [49177.927241] [debug]: lpagb:0x0 adv:0x300 According to the RTL8211E PHY chip datasheet, the contents of GBSR register are valid only when auto negotiation is completed(BMSR[5]: Auto- Negotiation Complete = 1). This patch adds the condition before reading GBSR register to fix this error state. Signed-off-by: Ming Wang <redacted> Signed-off-by: Qiang Ma <redacted> --- drivers/net/phy/phy_device.c | 10 ++++------ include/linux/phy.h | 1 + 2 files changed, 5 insertions(+), 6 deletions(-)diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index b884b681d5c5..b77ed5ec31c6 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c@@ -1532,10 +1532,8 @@ int genphy_update_link(struct phy_device *phydev) if (status < 0) return status; - if ((status & BMSR_LSTATUS) == 0) - phydev->link = 0; - else - phydev->link = 1; + phydev->link = status & BMSR_LSTATUS ? 1 : 0; + phydev->autoneg_complete = status & BMSR_ANEGCOMPLETE ? 1 : 0; return 0; }
What tree is this against? Both net-next/master and net/master have:
if (status < 0)
return status;
done:
phydev->link = status & BMSR_LSTATUS ? 1 : 0;
phydev->autoneg_complete = status & BMSR_ANEGCOMPLETE ? 1 : 0;
/* Consider the case that autoneg was started and "aneg complete"
* bit has been reset, but "link up" bit not yet.
*/
if (phydev->autoneg == AUTONEG_ENABLE && !phydev->autoneg_complete)
phydev->link = 0;
return 0;
}
It looks like you are using an old tree.
Andrew