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

Message ID 0a5701fa-0c5b-2375-08d6-a65145f83d96@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series
  • [net] net: phy: fix too strict check in phy_start_aneg
Related show

Commit Message

Heiner Kallweit Jan. 9, 2019, 7:34 p.m.
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(-)

Comments

Chris Wilson Jan. 9, 2019, 9:30 p.m. | #1
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
Florian Fainelli Jan. 13, 2019, 5:13 a.m. | #2
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>
David Miller Jan. 15, 2019, 9:45 p.m. | #3
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.

Patch

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: