[PATCH net-next v1 1/1] net: phy: clear EEE runtime state in PHY_HALTED/PHY_ERROR

[PATCH net-next v1 1/1] net: phy: clear EEE runtime state in PHY_HALTED/PHY_ERROR

[Oleksij Rempel]

Clear EEE runtime flags when the PHY transitions to HALTED or ERROR
and the state machine drops the link. This avoids stale EEE state being
reported via ethtool after the PHY is stopped or hits an error.

Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
Related discussion: https://lore.kernel.org/r/aKg7nf8YczCT6N0O@shell.armlinux.org.uk
---
 drivers/net/phy/phy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 13df28445f02..b8963d26af26 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1555,6 +1555,8 @@ static enum phy_state_work _phy_state_machine(struct phy_device *phydev)
 	case PHY_ERROR:
 		if (phydev->link) {
 			phydev->link = 0;
+			phydev->eee_active = false;
+			phydev->enable_tx_lpi = false;
 			phy_link_down(phydev);
 		}
 		state_work = PHY_STATE_WORK_SUSPEND;
--
2.47.3

Re: [PATCH net-next v1 1/1] net: phy: clear EEE runtime state in PHY_HALTED/PHY_ERROR

[Andrew Lunn]

On Tue, Sep 09, 2025 at 03:12:48PM +0200, Oleksij Rempel wrote:
> Clear EEE runtime flags when the PHY transitions to HALTED or ERROR
> and the state machine drops the link. This avoids stale EEE state being
> reported via ethtool after the PHY is stopped or hits an error.

One obvious question, why is EEE special? We have other state in
phydev which is not valid when the link is down. Are we setting speed
and duplex to UNKNOWN? lp_advertising, mdix, master_slave_state?

So while i agree it is nice not to show stale EEE state, maybe we
should not be showing any stale state and this patch needs extending?

	Andrew

Re: [PATCH net-next v1 1/1] net: phy: clear EEE runtime state in PHY_HALTED/PHY_ERROR

[Oleksij Rempel]

On Tue, Sep 09, 2025 at 03:58:32PM +0200, Andrew Lunn wrote:
> On Tue, Sep 09, 2025 at 03:12:48PM +0200, Oleksij Rempel wrote:
> > Clear EEE runtime flags when the PHY transitions to HALTED or ERROR
> > and the state machine drops the link. This avoids stale EEE state being
> > reported via ethtool after the PHY is stopped or hits an error.
> 
> One obvious question, why is EEE special? We have other state in
> phydev which is not valid when the link is down. Are we setting speed
> and duplex to UNKNOWN? lp_advertising, mdix, master_slave_state?
> 
> So while i agree it is nice not to show stale EEE state, maybe we
> should not be showing any stale state and this patch needs extending?

I decided to send the first step patch for the agreed subset (EEE
flags), so it can be merged faster.

As a follow-up I would propose a separate patch which clears additional
link-resolved state when the PHY enters HALTED, for example:

--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1552,6 +1552,16 @@ static enum phy_state_work _phy_state_machine(struct phy_device *phydev)
                }
                break;
        case PHY_HALTED:
+               if (phydev->link) {
+                       if (phydev->autoneg == AUTONEG_ENABLE) {
+                               phydev->speed = SPEED_UNKNOWN;
+                               phydev->duplex = DUPLEX_UNKNOWN;
+                       }
+                       if (phydev->master_slave_state != MASTER_SLAVE_STATE_UNSUPPORTED)
+                               phydev->master_slave_state = MASTER_SLAVE_STATE_UNKNOWN;
+                       phydev->mdix = ETH_TP_MDI_INVALID;
+                       linkmode_zero(phydev->lp_advertising);
+               }
        case PHY_ERROR:
                if (phydev->link) {
                        phydev->link = 0;

Would this approach be acceptable, or do you see hidden issues with clearing
these extra fields?

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 1/1] net: phy: clear EEE runtime state in PHY_HALTED/PHY_ERROR

[Andrew Lunn]

> As a follow-up I would propose a separate patch which clears additional
> link-resolved state when the PHY enters HALTED, for example:

This is good. Maybe mention in the commit message there will be a
follow up for other state variables.

Thanks
	Andrew