[PATCH net-next v2 0/2] net: fix a stale phylink PHY pointer and a lost PHY interrupt

[PATCH net-next v2 0/2] net: fix a stale phylink PHY pointer and a lost PHY interrupt

[Aleksei Sviridkin]

Two independent fixes, both found while chasing a PHY whose driver is a
module on a rootfs that is not mounted yet when a DSA switch probes.
Neither one depends on that setup, and neither depends on the other.

Patch 1: phylink_bringup_phy() records the PHY in pl->phydev before its
last fallible step, so a failure there leaves a pointer to a PHY the
caller has already detached. A later phylink_disconnect_phy() detaches
it a second time and drops references the first detach already
released.

Patch 2: a PHY that goes through a generic-driver bind cycle comes out
of it in polling mode for good. The specific driver that binds
afterwards never sees the interrupt the firmware node declared.

Tested on an MT7981B board: an Airoha EN8811H on an MT7531 switch port,
its interrupt declared in the device tree, its driver a module on the
rootfs. lan4 attaches with irq=15 rather than irq=POLL, the line is
claimed as mt-eint 0 in /proc/interrupts, and its counter goes 1 -> 3
-> 5 across two forced aneg restarts, matching the link dropping and
coming back each time, and holding steady in between. wan, whose
internal PHY has no interrupt in the device tree, still attaches with
irq=POLL: that is the observation which says the bus table cannot hand
back an interrupt the device never had. Patch 2's other exit, the one
in phy_attach_direct(), needs a generic probe to fail and is
compile-tested only.

---
Changes in v2:
 - patch 1: the unwind now shares a helper with phylink_disconnect_phy()
   instead of repeating its inner block (Andrew Lunn), and the patch
   gained a Fixes: tag
 - patch 2: the interrupt comes back from the MDIO bus interrupt table,
   so the new phy_device member, its kernel-doc and the save site in
   phy_attach_direct() are gone (Andrew Lunn)
 - dropped "net: dsa: connect a late-arriving PHY at ifup": binding the
   MAC to the PHY from open is the wrong layer
 - v1: https://lore.kernel.org/netdev/20260822155259.87146-1-f@lex.la/

Aleksei Sviridkin (2):
  net: phylink: unwind the PHY binding when bringup fails late
  net: phy: restore the interrupt after a generic-driver bind cycle

 drivers/net/phy/phy_device.c | 14 ++++++++++++++
 drivers/net/phy/phylink.c    | 29 ++++++++++++++++++++---------
 2 files changed, 34 insertions(+), 9 deletions(-)

-- 
2.55.0

[PATCH net-next v2 1/2] net: phylink: unwind the PHY binding when bringup fails late

[Aleksei Sviridkin]

phylink_bringup_phy() records the PHY in pl->phydev before its last
fallible step: on a MAC whose phylink ops implement LPI,
phy_eee_rx_clock_stop() can fail with a real MDIO error. The callers
unwind with phy_detach(), which knows nothing about pl->phydev, so a
pointer to a PHY that is no longer attached outlives the failed
connect.

What that costs depends on how the caller got here.
phylink_connect_phy() and the SFP path go through
phylink_attach_phy(), which refuses to attach while pl->phydev is set
and turns a transient MDIO error into a permanent -EBUSY.
phylink_fwnode_phy_connect() has no such check, so a later connect
overwrites the stale pointer and hides the problem. A disconnect does
not: phylink_disconnect_phy() hands that pointer to phy_disconnect(),
and the second phy_detach() on the same PHY drops references the first
one already released.

Clear the binding on the failure path. This is the same operation
phylink_disconnect_phy() performs, so both now share a helper. The
PHY-side fields are left to phy_detach(), which every caller already
runs on this path.

Fixes: 03abf2a7c654 ("net: phylink: add EEE management")
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
 drivers/net/phy/phylink.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 5b8e956902fb..a55e4a64028f 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2083,6 +2083,18 @@ static int phylink_validate_phy(struct phylink *pl, struct phy_device *phy,
 	return phylink_validate(pl, supported, state);
 }
 
+/* Disassociate @phy from @pl. Caller must hold pl->phydev_mutex. */
+static void phylink_clear_phydev(struct phylink *pl, struct phy_device *phy)
+{
+	mutex_lock(&phy->lock);
+	mutex_lock(&pl->state_mutex);
+	pl->phydev = NULL;
+	pl->phy_enable_tx_lpi = false;
+	pl->mac_tx_clk_stop = false;
+	mutex_unlock(&pl->state_mutex);
+	mutex_unlock(&phy->lock);
+}
+
 static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
 			       phy_interface_t interface)
 {
@@ -2197,6 +2209,12 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
 	if (ret == 0 && phy_interrupt_is_valid(phy))
 		phy_request_interrupt(phy);
 
+	if (ret) {
+		mutex_lock(&pl->phydev_mutex);
+		phylink_clear_phydev(pl, phy);
+		mutex_unlock(&pl->phydev_mutex);
+	}
+
 	return ret;
 }
 
@@ -2347,15 +2365,8 @@ void phylink_disconnect_phy(struct phylink *pl)
 
 	mutex_lock(&pl->phydev_mutex);
 	phy = pl->phydev;
-	if (phy) {
-		mutex_lock(&phy->lock);
-		mutex_lock(&pl->state_mutex);
-		pl->phydev = NULL;
-		pl->phy_enable_tx_lpi = false;
-		pl->mac_tx_clk_stop = false;
-		mutex_unlock(&pl->state_mutex);
-		mutex_unlock(&phy->lock);
-	}
+	if (phy)
+		phylink_clear_phydev(pl, phy);
 	mutex_unlock(&pl->phydev_mutex);
 
 	if (phy) {
-- 
2.55.0

[PATCH net-next v2 2/2] net: phy: restore the interrupt after a generic-driver bind cycle

[Aleksei Sviridkin]

A PHY with no specific driver available at attach time gets the generic
one, and phy_probe() parks it in polling mode because that driver has
no interrupt callbacks. phy_detach() releases the generic driver so a
real one can bind later, but the interrupt is not given back, and a
generic probe that fails leaves the PHY the same way without reaching
phy_detach() at all. The specific driver then attaches with
irq == PHY_POLL and the PHY stays polled for the rest of the uptime,
with no warning on that path.

Take it back from the MDIO bus interrupt table on both exits from the
cycle: the cycle does not touch that table, so whatever the bus
recorded there still stands. A bus that never filled it has nothing to
give back and its PHY stays polled. Restore only where the cycle left
PHY_POLL, so that an interrupt mode installed on the attached PHY
afterwards is not reset.

Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
The cycle is easy to hit on a DSA switch that probes before the rootfs
is mounted: the switch connects its user ports during setup, the PHY
driver is still a module on that rootfs, so the generic driver binds
and is released again when the connect fails. The real driver binds at
ifup and gets irq == PHY_POLL.

Both exits from the cycle need the restore: phy_detach() for a generic
driver that bound and is being released, and phy_attach_direct()'s
error_module_put label for a generic probe that failed, which never
calls phy_detach() at all.

What the table covers and what it does not. Nothing writes
mii_bus->irq[] after the bus is registered except stmmac_mdio.c and
mlxbf_gige_main.c, and both assign the same value to phydev->irq in the
same breath, so a PHY whose interrupt came from firmware is fixed here.
A PHY handed its interrupt by its MAC driver is not: lan78xx and
smsc95xx write phydev->irq after registration and leave the table
alone, so they keep polling after a generic cycle exactly as they do
today, and the set of drivers that write only phydev->irq is larger
than those two. Which raises a question I would rather ask than settle
alone: if bus->irq[] is the per-address registry for a bus, should
those drivers be mirroring into it the way mlxbf_gige and stmmac
already do? If that is the intent I am happy to send it as a follow-up.

What the guard distinguishes and what it does not. It preserves an
interrupt mode installed on the attached PHY after connect, so
PHY_MAC_INTERRUPT from genet, tsnep or bcmasp survives the detach. It
cannot tell phy_probe()'s parking from the other ways phydev->irq
reaches PHY_POLL, so a PHY parked by PHY_F_NO_IRQ, by a failed
phy_request_interrupt(), or by a MAC taking the phy.rst advice to set
PHY_POLL, is restored here as well. That is harmless for the drivers
that do so today: phy_attach_direct() applies PHY_F_NO_IRQ again on the
next attach, a failed request is simply retried, and every MAC that
forces PHY_POLL does so in the same function that connects the PHY, so
a restored value is overwritten before anything can act on it.

The guard is not only tidiness. Without it a PHY that a MAC had put in
PHY_MAC_INTERRUPT mode would get a real interrupt number back at
detach, the next phy_connect_direct() would request it, and
phy_disconnect() would then skip phy_free_interrupt(), because the MAC
overwrites phydev->irq again right after connect and
phy_interrupt_is_valid() is false by the time the interrupt would be
freed. That asymmetry between phy_connect_direct() and
phy_disconnect() is not new, and it bites any such MAC whose PHY has an
interrupt to request; the guard keeps a generic-driver cycle from
walking a PHY into it.

The same interrupt is also lost on a plain sysfs unbind and rebind of a
PHY driver, and this patch does not cover that. A restore in
phy_remove() would cover both paths, but is_genphy_driven is what keeps
the intent narrow here, so I would rather not widen the fix on a guess.

No Fixes: tag on this one. The behaviour predates what I can bisect in
this tree; if someone can name the commit I will add it.
 drivers/net/phy/phy_device.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 94b2e85e00a3..8c93d35e1c94 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1734,6 +1734,18 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
 	return phydrv->config_intr && phydrv->handle_interrupt;
 }
 
+/* Give back the interrupt phy_probe() parked when a driver with no interrupt
+ * callbacks bound. The bind cycle does not touch the bus interrupt table, so
+ * whatever the bus recorded there still stands; a bus that never filled it has
+ * nothing to give back. Only the parking is undone: any other value the PHY
+ * carries was put there by someone else.
+ */
+static void phy_restore_genphy_irq(struct phy_device *phydev)
+{
+	if (phydev->irq == PHY_POLL)
+		phydev->irq = phydev->mdio.bus->irq[phydev->mdio.addr];
+}
+
 /**
  * phy_attach_direct - attach a network device to a given PHY device pointer
  * @dev: network device to attach
@@ -1896,6 +1908,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 
 error_module_put:
 	module_put(d->driver->owner);
+	phy_restore_genphy_irq(phydev);
 	phydev->is_genphy_driven = 0;
 	d->driver = NULL;
 error_put_device:
@@ -1965,6 +1978,7 @@ void phy_detach(struct phy_device *phydev)
 	 * real driver could be loaded
 	 */
 	if (phydev->is_genphy_driven) {
+		phy_restore_genphy_irq(phydev);
 		device_release_driver(&phydev->mdio.dev);
 		phydev->is_genphy_driven = 0;
 	}
-- 
2.55.0

Re: [PATCH net-next v2 2/2] net: phy: restore the interrupt after a generic-driver bind cycle

[Paolo Abeni]

On 8/24/26 4:40 AM, Aleksei Sviridkin wrote:
> A PHY with no specific driver available at attach time gets the generic
> one, and phy_probe() parks it in polling mode because that driver has
> no interrupt callbacks. phy_detach() releases the generic driver so a
> real one can bind later, but the interrupt is not given back, and a
> generic probe that fails leaves the PHY the same way without reaching
> phy_detach() at all. The specific driver then attaches with
> irq == PHY_POLL and the PHY stays polled for the rest of the uptime,
> with no warning on that path.
> 
> Take it back from the MDIO bus interrupt table on both exits from the
> cycle: the cycle does not touch that table, so whatever the bus
> recorded there still stands. A bus that never filled it has nothing to
> give back and its PHY stays polled. Restore only where the cycle left
> PHY_POLL, so that an interrupt mode installed on the attached PHY
> afterwards is not reset.
> 
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
> ---
> The cycle is easy to hit on a DSA switch that probes before the rootfs
> is mounted: the switch connects its user ports during setup, the PHY
> driver is still a module on that rootfs, so the generic driver binds
> and is released again when the connect fails. The real driver binds at
> ifup and gets irq == PHY_POLL.
> 
> Both exits from the cycle need the restore: phy_detach() for a generic
> driver that bound and is being released, and phy_attach_direct()'s
> error_module_put label for a generic probe that failed, which never
> calls phy_detach() at all.
> 
> What the table covers and what it does not. Nothing writes
> mii_bus->irq[] after the bus is registered except stmmac_mdio.c and
> mlxbf_gige_main.c, and both assign the same value to phydev->irq in the
> same breath, so a PHY whose interrupt came from firmware is fixed here.
> A PHY handed its interrupt by its MAC driver is not: lan78xx and
> smsc95xx write phydev->irq after registration and leave the table
> alone, so they keep polling after a generic cycle exactly as they do
> today, and the set of drivers that write only phydev->irq is larger
> than those two. Which raises a question I would rather ask than settle
> alone: if bus->irq[] is the per-address registry for a bus, should
> those drivers be mirroring into it the way mlxbf_gige and stmmac
> already do? If that is the intent I am happy to send it as a follow-up.
> 
> What the guard distinguishes and what it does not. It preserves an
> interrupt mode installed on the attached PHY after connect, so
> PHY_MAC_INTERRUPT from genet, tsnep or bcmasp survives the detach. It
> cannot tell phy_probe()'s parking from the other ways phydev->irq
> reaches PHY_POLL, so a PHY parked by PHY_F_NO_IRQ, by a failed
> phy_request_interrupt(), or by a MAC taking the phy.rst advice to set
> PHY_POLL, is restored here as well. That is harmless for the drivers
> that do so today: phy_attach_direct() applies PHY_F_NO_IRQ again on the
> next attach, a failed request is simply retried, and every MAC that
> forces PHY_POLL does so in the same function that connects the PHY, so
> a restored value is overwritten before anything can act on it.
> 
> The guard is not only tidiness. Without it a PHY that a MAC had put in
> PHY_MAC_INTERRUPT mode would get a real interrupt number back at
> detach, the next phy_connect_direct() would request it, and
> phy_disconnect() would then skip phy_free_interrupt(), because the MAC
> overwrites phydev->irq again right after connect and
> phy_interrupt_is_valid() is false by the time the interrupt would be
> freed. That asymmetry between phy_connect_direct() and
> phy_disconnect() is not new, and it bites any such MAC whose PHY has an
> interrupt to request; the guard keeps a generic-driver cycle from
> walking a PHY into it.
> 
> The same interrupt is also lost on a plain sysfs unbind and rebind of a
> PHY driver, and this patch does not cover that. A restore in
> phy_remove() would cover both paths, but is_genphy_driven is what keeps
> the intent narrow here, so I would rather not widen the fix on a guess.
> 
> No Fixes: tag on this one. The behaviour predates what I can bisect in
> this tree; if someone can name the commit I will add it.

If the root cause predates git history, the fixes tag should be:

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

Otherwise the oldest commit that still reproduces the issue would do.

It looks like out sashiko instance did not pick this series, and this is
net material. Please re-submit with the correct target tree and fixes tag.

Please also try to condense the above writing in a much shorter text, or
point to prior discussion.

/P

Re: [PATCH net-next v2 1/2] net: phylink: unwind the PHY binding when bringup fails late

[Andrew Lunn]

On Mon, Aug 24, 2026 at 05:40:27AM +0300, Aleksei Sviridkin wrote:
> phylink_bringup_phy() records the PHY in pl->phydev before its last
> fallible step: on a MAC whose phylink ops implement LPI,
> phy_eee_rx_clock_stop() can fail with a real MDIO error. The callers
> unwind with phy_detach(), which knows nothing about pl->phydev, so a
> pointer to a PHY that is no longer attached outlives the failed
> connect.
> 
> What that costs depends on how the caller got here.
> phylink_connect_phy() and the SFP path go through
> phylink_attach_phy(), which refuses to attach while pl->phydev is set
> and turns a transient MDIO error into a permanent -EBUSY.
> phylink_fwnode_phy_connect() has no such check, so a later connect
> overwrites the stale pointer and hides the problem. A disconnect does
> not: phylink_disconnect_phy() hands that pointer to phy_disconnect(),
> and the second phy_detach() on the same PHY drops references the first
> one already released.
> 
> Clear the binding on the failure path. This is the same operation
> phylink_disconnect_phy() performs, so both now share a helper. The
> PHY-side fields are left to phy_detach(), which every caller already
> runs on this path.
> 
> Fixes: 03abf2a7c654 ("net: phylink: add EEE management")
> Signed-off-by: Aleksei Sviridkin <f@lex.la>

This should be for net, not net-next.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 2/2] net: phy: restore the interrupt after a generic-driver bind cycle

[Andrew Lunn]

> Take it back from the MDIO bus interrupt table on both exits from the
> cycle: the cycle does not touch that table, so whatever the bus
> recorded there still stands. A bus that never filled it has nothing to
> give back and its PHY stays polled.

Bit of an odd phrase.

It might be better to explain that mdiobus_alloc() defaults to filling
bus->irq[] for polling.

> +static void phy_restore_genphy_irq(struct phy_device *phydev)
> +{
> +	if (phydev->irq == PHY_POLL)
> +		phydev->irq = phydev->mdio.bus->irq[phydev->mdio.addr];

Why conditional? Maybe it is buried deep in that wall of text?

    Andrew