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

[Paolo Abeni <pabeni@redhat.com>]

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