From dcbccb8b561146f516fe7e72e3b5e8c1cfae713a Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 22 May 2023 16:58:08 +0100 Subject: [PATCH] net: phy: avoid kernel warning dump when stopping an errored PHY When taking a network interface down (or removing a SFP module) after the PHY has encountered an error, phy_stop() complains incorrectly that it was called from HALTED state. The reason this is incorrect is that the network driver will have called phy_start() when the interface was brought up, and the fact that the PHY has a problem bears no relationship to the administrative state of the interface. Taking the interface administratively down (which calls phy_stop()) is always the right thing to do after a successful phy_start() call, whether or not the PHY has encountered an error. Signed-off-by: Russell King (Oracle) Acked-by: Florian Fainelli Signed-off-by: David S. Miller Signed-off-by: hongrongxuan Conflicts: drivers/net/phy/phy.c include/linux/phy.h --- drivers/net/phy/phy.c | 11 +++++++---- include/linux/phy.h | 9 ++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 7322471ea223..d4875c897fdb 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -44,6 +44,7 @@ static const char *phy_state_to_str(enum phy_state st) PHY_STATE_STR(RUNNING) PHY_STATE_STR(NOLINK) PHY_STATE_STR(HALTED) + PHY_STATE_STR(ERROR) } return NULL; @@ -734,7 +735,7 @@ void phy_stop_machine(struct phy_device *phydev) static void phy_process_error(struct phy_device *phydev) { mutex_lock(&phydev->lock); - phydev->state = PHY_HALTED; + phydev->state = PHY_ERROR; mutex_unlock(&phydev->lock); phy_trigger_machine(phydev); @@ -748,10 +749,10 @@ static void phy_error_precise(struct phy_device *phydev, } /** - * phy_error - enter HALTED state for this PHY device + * phy_error - enter ERROR state for this PHY device * @phydev: target phy_device struct * - * Moves the PHY to the HALTED state in response to a read + * Moves the PHY to the ERROR 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. @@ -904,7 +905,8 @@ EXPORT_SYMBOL(phy_free_interrupt); */ void phy_stop(struct phy_device *phydev) { - if (!phy_is_started(phydev) && phydev->state != PHY_DOWN) { + if (!phy_is_started(phydev) && phydev->state != PHY_DOWN && + phydev->state != PHY_ERROR) { WARN(1, "called from state %s\n", phy_state_to_str(phydev->state)); return; @@ -989,6 +991,7 @@ void phy_state_machine(struct work_struct *work) func = &phy_check_link_status; break; case PHY_HALTED: + case PHY_ERROR: if (phydev->link) { phydev->link = 0; phy_link_down(phydev, true); diff --git a/include/linux/phy.h b/include/linux/phy.h index 8c00bfec6665..10351f514ec4 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -302,14 +302,17 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr); * - irq or timer will set NOLINK if link goes down * - phy_stop moves to HALTED * - * HALTED: PHY is up, but no polling or interrupts are done. Or - * PHY is in an error state. - * - phy_start moves to UP + * @PHY_HALTED: PHY is up, but no polling or interrupts are done. + * - phy_start moves to @PHY_UP + * + * @PHY_ERROR: PHY is up, but is in an error state. + * - phy_stop moves to @PHY_HALTED */ enum phy_state { PHY_DOWN = 0, PHY_READY, PHY_HALTED, + PHY_ERROR, PHY_UP, PHY_RUNNING, PHY_NOLINK,