[PATCH net 0/4] net: phy: restore driver state after userspace MII resets

[PATCH net 0/4] net: phy: restore driver state after userspace MII resets

[James Hilliard]

An MII ioctl reset needs to restore the PHY driver's configuration and
software state before the PHY worker can access the device again.

This was observed while testing AC200 receive statistics: a userspace
BMCR reset cleared the hardware counters, and the next update interpreted
the decrease as counter wraparound.

First remove b44's spinlock around the sleeping phylib ioctl. Make phylib
propagate failed writes, wait for reset completion and keep the PHY worker
disabled until driver reinitialization finishes. Then use this path for
phylink BMCR reset requests targeting the attached Clause 22 PHY.

Assisted-by: Codex:gpt-6-astra
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
James Hilliard (4):
      net: b44: keep phylib ioctls outside the spinlock
      net: phy: propagate MII register write errors
      net: phy: quiesce the state machine during MII resets
      net: phylink: reinitialize the PHY after an MII reset

 drivers/net/ethernet/broadcom/b44.c |  4 ++--
 drivers/net/phy/phy.c               | 42 +++++++++++++++++++++++++++++++------
 drivers/net/phy/phy_device.c        |  2 +-
 drivers/net/phy/phylib-internal.h   |  1 +
 drivers/net/phy/phylink.c           | 14 +++++++++----
 5 files changed, 50 insertions(+), 13 deletions(-)
---
base-commit: ead6fa9a0ab194f02a2f36d634b193e9c5f9a1c6
change-id: 20261113-submit-phy-mii-reset-v1-1b9ec0e2be11

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

[PATCH net 1/4] net: b44: keep phylib ioctls outside the spinlock

[James Hilliard]

b44_ioctl() holds bp->lock with interrupts disabled while calling
phy_mii_ioctl() for an external PHY. That helper takes sleeping MDIO
mutexes and can wait for a PHY reset to complete.

Keep the spinlock around the legacy internal-PHY ioctl only. External
PHY register accesses are serialized by the MDIO bus mutex, as they
already are when called from the PHY state machine.

Fixes: 86f4ea63e696 ("b44: add phylib support")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/net/ethernet/broadcom/b44.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index f994636fbd5f..e9f116dfaad9 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2126,14 +2126,14 @@ static int b44_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	if (!netif_running(dev))
 		goto out;
 
-	spin_lock_irq(&bp->lock);
 	if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
 		BUG_ON(!dev->phydev);
 		err = phy_mii_ioctl(dev->phydev, ifr, cmd);
 	} else {
+		spin_lock_irq(&bp->lock);
 		err = generic_mii_ioctl(&bp->mii_if, if_mii(ifr), cmd, NULL);
+		spin_unlock_irq(&bp->lock);
 	}
-	spin_unlock_irq(&bp->lock);
 out:
 	return err;
 }

-- 
2.53.0

[PATCH net 2/4] net: phy: propagate MII register write errors

[James Hilliard]

phy_mii_ioctl() ignores MDIO write failures and can report success after a
failed write. It also updates the cached link settings and may restart
auto-negotiation or reinitialize the PHY even though the requested write
failed.

Check the write result before updating the cached settings or invoking
those follow-up operations, and return the MDIO error to the caller.

Fixes: 00db8189d984 ("This patch adds a PHY Abstraction Layer to the Linux Kernel, enabling ethernet drivers to remain as ignorant as is reasonable of the connected PHY's design and operation details.")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/net/phy/phy.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index fce9bc7be330..4ac72a1b4f42 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -350,6 +350,15 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
 			prtad = mii_data->phy_id;
 			devad = mii_data->reg_num;
 		}
+
+		if (mdio_phy_id_is_c45(mii_data->phy_id))
+			ret = mdiobus_c45_write(phydev->mdio.bus, prtad, devad,
+						mii_data->reg_num, val);
+		else
+			ret = mdiobus_write(phydev->mdio.bus, prtad, devad, val);
+		if (ret < 0)
+			return ret;
+
 		if (prtad == phydev->mdio.addr) {
 			switch (devad) {
 			case MII_BMCR:
@@ -388,12 +397,6 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
 			}
 		}
 
-		if (mdio_phy_id_is_c45(mii_data->phy_id))
-			mdiobus_c45_write(phydev->mdio.bus, prtad, devad,
-					  mii_data->reg_num, val);
-		else
-			mdiobus_write(phydev->mdio.bus, prtad, devad, val);
-
 		if (prtad == phydev->mdio.addr &&
 		    devad == MII_BMCR &&
 		    val & BMCR_RESET)

-- 
2.53.0

[PATCH net 3/4] net: phy: quiesce the state machine during MII resets

[James Hilliard]

phy_mii_ioctl() performs a BMCR reset write and calls phy_init_hw() while
the PHY state machine remains active. The worker can access the PHY after
reset but before the driver has restored its configuration and software
state. A statistics update in this window can interpret cleared hardware
counters as a wraparound.

Disable and drain the state-machine work before resetting the attached
Clause 22 PHY. Reenable it and trigger a new check afterward, including on
failure. Disabling the work also prevents interrupt notifications from
queuing it during reinitialization.

Wait for BMCR_RESET to clear before calling phy_init_hw(). Drivers without
a soft_reset callback otherwise begin initialization while the requested
reset may still be in progress. Share phy_poll_reset() internally for this
wait and propagate its read errors or timeout before initializing the PHY.

Do not hold the PHY mutex across phy_init_hw(): some driver initialization
callbacks acquire that mutex themselves.

Fixes: 00db8189d984 ("This patch adds a PHY Abstraction Layer to the Linux Kernel, enabling ethernet drivers to remain as ignorant as is reasonable of the connected PHY's design and operation details.")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/net/phy/phy.c             | 27 +++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c      |  2 +-
 drivers/net/phy/phylib-internal.h |  1 +
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 4ac72a1b4f42..5da8a699446c 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -297,6 +297,30 @@ void phy_ethtool_ksettings_get(struct phy_device *phydev,
 }
 EXPORT_SYMBOL(phy_ethtool_ksettings_get);
 
+static int phy_mii_ioctl_reset(struct phy_device *phydev, u16 val)
+{
+	int ret;
+
+	/* Keep the worker out until the driver has restored its state. */
+	disable_delayed_work_sync(&phydev->state_queue);
+
+	ret = phy_write(phydev, MII_BMCR, val);
+	if (ret < 0)
+		goto out;
+
+	ret = phy_poll_reset(phydev);
+	if (ret)
+		goto out;
+
+	phydev->autoneg = AUTONEG_ENABLE;
+	ret = phy_init_hw(phydev);
+out:
+	if (enable_delayed_work(&phydev->state_queue))
+		phy_trigger_machine(phydev);
+
+	return ret;
+}
+
 /**
  * phy_mii_ioctl - generic PHY MII ioctl interface
  * @phydev: the phy_device struct
@@ -349,6 +373,9 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
 		} else {
 			prtad = mii_data->phy_id;
 			devad = mii_data->reg_num;
+			if (prtad == phydev->mdio.addr &&
+			    devad == MII_BMCR && val & BMCR_RESET)
+				return phy_mii_ioctl_reset(phydev, val);
 		}
 
 		if (mdio_phy_id_is_c45(mii_data->phy_id))
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b39e945cebd7..8b0c1f28963b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1232,7 +1232,7 @@ EXPORT_SYMBOL(phy_disconnect);
  *   standard phy_init_hw() which will zero all the other bits in the BMCR
  *   and reapply all driver-specific and board-specific fixups.
  */
-static int phy_poll_reset(struct phy_device *phydev)
+int phy_poll_reset(struct phy_device *phydev)
 {
 	/* Poll until the reset bit clears (50ms per retry == 0.6 sec) */
 	int ret, val;
diff --git a/drivers/net/phy/phylib-internal.h b/drivers/net/phy/phylib-internal.h
index 38ee294f3c96..680823633d98 100644
--- a/drivers/net/phy/phylib-internal.h
+++ b/drivers/net/phy/phylib-internal.h
@@ -24,6 +24,7 @@ void of_set_phy_timing_role(struct phy_device *phydev);
 int phy_speed_down_core(struct phy_device *phydev);
 void phy_check_downshift(struct phy_device *phydev);
 int phy_scan_fixups(struct phy_device *phydev);
+int phy_poll_reset(struct phy_device *phydev);
 
 int mdiobus_register_device(struct mdio_device *mdiodev);
 int mdiobus_unregister_device(struct mdio_device *mdiodev);

-- 
2.53.0

[PATCH net 4/4] net: phylink: reinitialize the PHY after an MII reset

[James Hilliard]

phylink handles SIOCSMIIREG with a raw MDIO write, bypassing the reset
handling in phy_mii_ioctl(). A BMCR reset of the attached Clause 22 PHY
therefore clears hardware state without running the driver initialization
callbacks. The PHY worker continues using the old software state.

Delegate BMCR reset writes for the attached Clause 22 PHY to
phy_mii_ioctl(), which quiesces the PHY worker across reset completion and
driver initialization. Select this path in the register-write helper after
the Clause 45 cases, retaining raw access for the other register writes.

Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/net/phy/phylink.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index e885da1c2831..2f0e3a30615c 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -3570,10 +3570,13 @@ static int phylink_phy_read(struct phylink *pl, unsigned int phy_id,
 	return mdiobus_read(pl->phydev->mdio.bus, phy_id, reg);
 }
 
-static int phylink_phy_write(struct phylink *pl, unsigned int phy_id,
-			     unsigned int reg, unsigned int val)
+static int phylink_phy_write(struct phylink *pl, struct ifreq *ifr)
 {
+	struct mii_ioctl_data *mii = if_mii(ifr);
 	struct phy_device *phydev = pl->phydev;
+	u16 phy_id = mii->phy_id;
+	u16 reg = mii->reg_num;
+	u16 val = mii->val_in;
 	int prtad, devad;
 
 	if (mdio_phy_id_is_c45(phy_id)) {
@@ -3608,6 +3611,10 @@ static int phylink_phy_write(struct phylink *pl, unsigned int phy_id,
 					 reg, val);
 	}
 
+	if (phy_id == phydev->mdio.addr && reg == MII_BMCR &&
+	    (val & BMCR_RESET))
+		return phy_mii_ioctl(phydev, ifr, SIOCSMIIREG);
+
 	return mdiobus_write(phydev->mdio.bus, phy_id, reg, val);
 }
 
@@ -3697,8 +3704,7 @@ int phylink_mii_ioctl(struct phylink *pl, struct ifreq *ifr, int cmd)
 			break;
 
 		case SIOCSMIIREG:
-			ret = phylink_phy_write(pl, mii->phy_id, mii->reg_num,
-						mii->val_in);
+			ret = phylink_phy_write(pl, ifr);
 			break;
 
 		default:

-- 
2.53.0

Re: [PATCH net 0/4] net: phy: restore driver state after userspace MII resets

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.