netdev - [PATCH net-next 1/2] net: phy: don't stop state machine in case of MDIO error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6e57b405-e39e-91a1-c843-f461db9bc286@gmail.com>
Date:   Sat, 15 Dec 2018 17:18:33 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        David Miller <davem@...emloft.net>,
        Fugang Duan <fugang.duan@....com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: [PATCH net-next 1/2] net: phy: don't stop state machine in case of
 MDIO error

If we detect a MDIO error, it seems to be a little bit too aggressive
to stop the state machine and bring down the PHY completely.
E.g. when polling and we miss one update, then this has no relevant
impact. And in phy_stop_interrupts() actually interrupts should be
disabled already because phy_stop() is called before. The only caller
of phy_stop_interrupts() isn't interested in the return value,
so let's change return type to void.

Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
---
 drivers/net/phy/phy.c | 42 +++++++++---------------------------------
 include/linux/phy.h   |  2 +-
 2 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index e24708f1f..f926ec52a 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -714,24 +714,6 @@ void phy_stop_machine(struct phy_device *phydev)
 	mutex_unlock(&phydev->lock);
 }
 
-/**
- * phy_error - enter HALTED state for this PHY device
- * @phydev: target phy_device struct
- *
- * Moves the PHY to the HALTED state in response to a read
- * or write error, and tells the controller the link is down.
- * Must not be called from interrupt context, or while the
- * phydev->lock is held.
- */
-static void phy_error(struct phy_device *phydev)
-{
-	mutex_lock(&phydev->lock);
-	phydev->state = PHY_HALTED;
-	mutex_unlock(&phydev->lock);
-
-	phy_trigger_machine(phydev);
-}
-
 /**
  * phy_disable_interrupts - Disable the PHY interrupts from the PHY side
  * @phydev: target phy_device struct
@@ -769,13 +751,12 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
 	/* reschedule state queue work to run as soon as possible */
 	phy_trigger_machine(phydev);
 
-	if (phy_clear_interrupt(phydev))
-		goto phy_err;
-	return IRQ_HANDLED;
+	if (phy_clear_interrupt(phydev)) {
+		phydev_err(phydev, "failed to ack interrupt\n");
+		return IRQ_NONE;
+	}
 
-phy_err:
-	phy_error(phydev);
-	return IRQ_NONE;
+	return IRQ_HANDLED;
 }
 
 /**
@@ -821,16 +802,10 @@ EXPORT_SYMBOL(phy_start_interrupts);
  * phy_stop_interrupts - disable interrupts from a PHY device
  * @phydev: target phy_device struct
  */
-int phy_stop_interrupts(struct phy_device *phydev)
+void phy_stop_interrupts(struct phy_device *phydev)
 {
-	int err = phy_disable_interrupts(phydev);
-
-	if (err)
-		phy_error(phydev);
-
+	phy_disable_interrupts(phydev);
 	free_irq(phydev->irq, phydev);
-
-	return err;
 }
 EXPORT_SYMBOL(phy_stop_interrupts);
 
@@ -969,7 +944,8 @@ void phy_state_machine(struct work_struct *work)
 		phy_suspend(phydev);
 
 	if (err < 0)
-		phy_error(phydev);
+		phydev_err(phydev,
+			   "error %d executing PHY state machine\n", err);
 
 	if (old_state != phydev->state)
 		phydev_dbg(phydev, "PHY state change %s -> %s\n",
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 8f927246a..9b2235bd5 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -932,7 +932,7 @@ int phy_aneg_done(struct phy_device *phydev);
 int phy_speed_down(struct phy_device *phydev, bool sync);
 int phy_speed_up(struct phy_device *phydev);
 
-int phy_stop_interrupts(struct phy_device *phydev);
+void phy_stop_interrupts(struct phy_device *phydev);
 int phy_restart_aneg(struct phy_device *phydev);
 int phy_reset_after_clk_enable(struct phy_device *phydev);
 
-- 
2.20.0