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

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

[Grygorii Strashko]

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

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

[Andrew Lunn]

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

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

[Grygorii Strashko]

On 12/21/2017 03:21 AM, Andrew Lunn wrote:
> 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.

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

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

[David Miller]

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.