[PATCH net] net: phy: fix too strict check in phy_start_aneg

[PATCH net] net: phy: fix too strict check in phy_start_aneg

[Heiner Kallweit]

When adding checks to detect wrong usage of the phylib API we added
a check to phy_start_aneg() which is too strict. If the phylib
state machine is in state PHY_HALTED we should allow reconfiguring
and restarting aneg, and just don't touch the state.

Fixes: 2b3e88ea6528 ("net: phy: improve phy state checking")
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d33e7b3ca..189cd2048 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -543,13 +543,6 @@ int phy_start_aneg(struct phy_device *phydev)
 
 	mutex_lock(&phydev->lock);
 
-	if (!__phy_is_started(phydev)) {
-		WARN(1, "called from state %s\n",
-		     phy_state_to_str(phydev->state));
-		err = -EBUSY;
-		goto out_unlock;
-	}
-
 	if (AUTONEG_DISABLE == phydev->autoneg)
 		phy_sanitize_settings(phydev);
 
@@ -560,11 +553,13 @@ int phy_start_aneg(struct phy_device *phydev)
 	if (err < 0)
 		goto out_unlock;
 
-	if (phydev->autoneg == AUTONEG_ENABLE) {
-		err = phy_check_link_status(phydev);
-	} else {
-		phydev->state = PHY_FORCING;
-		phydev->link_timeout = PHY_FORCE_TIMEOUT;
+	if (__phy_is_started(phydev)) {
+		if (phydev->autoneg == AUTONEG_ENABLE) {
+			err = phy_check_link_status(phydev);
+		} else {
+			phydev->state = PHY_FORCING;
+			phydev->link_timeout = PHY_FORCE_TIMEOUT;
+		}
 	}
 
 out_unlock:
-- 
2.20.1

Re: [PATCH net] net: phy: fix too strict check in phy_start_aneg

[Chris Wilson]

Quoting Heiner Kallweit (2019-01-09 19:34:56)
> When adding checks to detect wrong usage of the phylib API we added
> a check to phy_start_aneg() which is too strict. If the phylib
> state machine is in state PHY_HALTED we should allow reconfiguring
> and restarting aneg, and just don't touch the state.
> 
> Fixes: 2b3e88ea6528 ("net: phy: improve phy state checking")
> Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Courtesy of intel-gfx's CI farm,
Tested-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks for the fix,
-Chris

Re: [PATCH net] net: phy: fix too strict check in phy_start_aneg

[Florian Fainelli]

On January 9, 2019 11:34:56 AM PST, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>When adding checks to detect wrong usage of the phylib API we added
>a check to phy_start_aneg() which is too strict. If the phylib
>state machine is in state PHY_HALTED we should allow reconfiguring
>and restarting aneg, and just don't touch the state.
>
>Fixes: 2b3e88ea6528 ("net: phy: improve phy state checking")
>Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
>Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

-- 
Florian

Re: [PATCH net] net: phy: fix too strict check in phy_start_aneg

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 9 Jan 2019 20:34:56 +0100

> When adding checks to detect wrong usage of the phylib API we added
> a check to phy_start_aneg() which is too strict. If the phylib
> state machine is in state PHY_HALTED we should allow reconfiguring
> and restarting aneg, and just don't touch the state.
> 
> Fixes: 2b3e88ea6528 ("net: phy: improve phy state checking")
> Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied.