[PATCH] net: phy: micrel: ksz9031: reconfigure autoneg after phy autoneg workaround

Under some circumstances driver will perform PHY reset in
ksz9031_read_status() to fix autoneg failure case (idle error count =
0xFF). When this happens ksz9031 will not detect link status change any
more when connecting to Netgear 1G switch (link can be recovered sometimes by
restarting netdevice "ifconfig down up"). Reproduced with TI am572x board
equipped with ksz9031 PHY while connecting to Netgear 1G switch.

Fix the issue by reconfiguring autonegotiation after PHY reset in
ksz9031_read_status().

Fixes: d2fd719bcb0e ("net/phy: micrel: Add workaround for bad autoneg")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/phy/micrel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index ab46141..422ff63 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c

@@ -624,6 +624,7 @@ static int ksz9031_read_status(struct phy_device *phydev)
 		phydev->link = 0;
 		if (phydev->drv->config_intr && phy_interrupt_is_valid(phydev))
 			phydev->drv->config_intr(phydev);
+		return genphy_config_aneg(phydev);
 	}
 
 	return 0;

-- 
2.10.5

On Wed, Dec 20, 2017 at 06:45:10PM -0600, Grygorii Strashko wrote:

Under some circumstances driver will perform PHY reset in
ksz9031_read_status() to fix autoneg failure case (idle error count =
0xFF). When this happens ksz9031 will not detect link status change any
more when connecting to Netgear 1G switch (link can be recovered sometimes by
restarting netdevice "ifconfig down up"). Reproduced with TI am572x board
equipped with ksz9031 PHY while connecting to Netgear 1G switch.

Fix the issue by reconfiguring autonegotiation after PHY reset in
ksz9031_read_status().

Hi Grygorii

I can understand the fix.

But i'm wondering if there is a better way to do this. Can you call
phy_stop() and phy_start(). You then get the core phy code doing the
same initialisation as what happened the first time. However, i know
this is not easy. _read_status() is being called from the middle of
the state machine, and trying to change the state of the state machine
at this point is problematic.

   Andrew

On 12/21/2017 03:21 AM, Andrew Lunn wrote:

On Wed, Dec 20, 2017 at 06:45:10PM -0600, Grygorii Strashko wrote:

quoted

Under some circumstances driver will perform PHY reset in
ksz9031_read_status() to fix autoneg failure case (idle error count =
0xFF). When this happens ksz9031 will not detect link status change any
more when connecting to Netgear 1G switch (link can be recovered sometimes by
restarting netdevice "ifconfig down up"). Reproduced with TI am572x board
equipped with ksz9031 PHY while connecting to Netgear 1G switch.

Fix the issue by reconfiguring autonegotiation after PHY reset in
ksz9031_read_status().

Hi Grygorii

I can understand the fix.

But i'm wondering if there is a better way to do this. Can you call
phy_stop() and phy_start(). You then get the core phy code doing the
same initialisation as what happened the first time. However, i know
this is not easy. _read_status() is being called from the middle of
the state machine, and trying to change the state of the state machine
at this point is problematic.

It will not work. 
phy_state_machine()->mutex_lock(&phydev->lock);->phy_read_status()->phy_stop()->mutex_lock(&phydev->lock);
at least as is.

This is error recovery, which is, per my understanding, signalizing about hw or
configuration issue. What i'm thinking about is - may be it'll be reasonable
to add err message here:
         if ((regval & 0xFF) == 0xFF) {
+               dev_err_once(&phydev->mdio.dev,
+                            "PHY reset due idle error count maxed out");


-- 
regards,
-grygorii

From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Wed, 20 Dec 2017 18:45:10 -0600

Under some circumstances driver will perform PHY reset in
ksz9031_read_status() to fix autoneg failure case (idle error count =
0xFF). When this happens ksz9031 will not detect link status change any
more when connecting to Netgear 1G switch (link can be recovered sometimes by
restarting netdevice "ifconfig down up"). Reproduced with TI am572x board
equipped with ksz9031 PHY while connecting to Netgear 1G switch.

Fix the issue by reconfiguring autonegotiation after PHY reset in
ksz9031_read_status().

Fixes: d2fd719bcb0e ("net/phy: micrel: Add workaround for bad autoneg")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Applied and queued up for -stable.