* [PATCH net v11 0/4] net: phy: keep a PHY interrupt across a generic bind cycle
@ 2026-09-26 23:50 Aleksei Sviridkin
2026-09-26 23:50 ` [PATCH net v11 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus Aleksei Sviridkin
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Aleksei Sviridkin @ 2026-09-26 23:50 UTC (permalink / raw)
To: netdev
Cc: andrew, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel,
Aleksei Sviridkin
On a Keenetic KN-1012 (MT7981B, MT7531 switch), the Airoha EN8811H
behind lan4 has its PHY driver as a module on the root filesystem. The
switch brings its user ports up before that filesystem is mounted, so
the PHY attaches to the generic driver first. phy_probe() replaces
phydev->irq with PHY_POLL because genphy has no interrupt callbacks,
nothing puts the number back, and the PHY polls for the rest of the
uptime once its real driver takes over. The devicetree describes a
working interrupt for it and the number is never used again. On this
particular board the port does not come up at all while that happens,
because phylink rejects 2500base-x against the generic driver and DSA
drops the port; it is the PHY that carries the lost number across the
cycle.
Patch 3 takes the number back from mdiobus->irq[] at the end of the bind
cycle. Patch 4 covers the other end of the same cycle: a genphy bind
that fails inside phy_attach_direct() unwinds on a label that does not
reach phy_detach(); it puts back the value the field held when the
attach started. Patches 1 and 2 put two USB drivers' interrupt numbers
into the bus table, so that there is something to take back for them as
well.
What this does not cover: phy_probe() makes the substitution for any
driver without interrupt callbacks, not only for the hand-bound generic
one, and phy_remove() does not undo it. So a Generic PHY bound through
sysfs, or a real driver without interrupt support unbound through sysfs
or rmmod, still leaves PHY_POLL for the next driver. Undoing it in
phy_remove() is what v5 and v6 did - v5 from a saved field, which Andrew
asked to drop, v6 from the bus table behind a phy_link_change check that
races phy_attach_direct(), as nothing there holds a lock in common with
the driver core. Removing the substitution, below, closes those paths
too.
On the board above the number reaches that table through
fwnode_mdiobus_phy_device_register(), which writes phydev->irq and
mdiobus->irq[addr] together when the PHY node carries an interrupt; the
EN8811H hangs off the SoC MDIO bus, at the devicetree node
/soc/ethernet@15100000/mdio-bus/ethernet-phy@d. A driver that owns its
bus can fill the table itself instead, and several do - mt7530 writes
irq_create_mapping() results into ds->user_mii_bus->irq[] before
registering the bus, which is where the switch ports' own numbers in the
notes to patch 3 come from, and mlxbf_gige writes an ACPI GPIO interrupt
into its bus table too, though after registering it and after
phy_find_first(), the way stmmac does.
Andrew asked [2] for the full set of drivers that keep the interrupt
outside the bus, "so that the bus is the source of truth" [3]. Going
through that list against net/main, in his grouping:
- lan78xx and smsc95xx write a live interrupt into phydev->irq only.
Those are patches 1 and 2.
- ucc_geth never writes the field at all; its single use is a read in
ucc_geth_open() feeding device_set_wakeup_capable(), so nothing to
change, as he said.
- ixp4xx_eth, ax88796c and emac-mac force PHY_POLL into phydev->irq
around their connect, and each keeps doing it. ixp4xx and ax88796c
store it in probe just after the connect succeeds, and their only
detaches are their own probe error path and remove. emac-mac stores
it early in emac_mac_up(), on the line before the connect, and that
runs on every bringup. No attach in any of the three follows
a detach without the driver forcing PHY_POLL again, so a restore in
between cannot cost them anything.
- stmmac_mdio already writes mdiobus->irq[] next to phydev->irq, so
it is on the right side of this. The block is also unreachable in
tree: it is guarded by probed_phy_irq > 0 and nothing sets that
field, in stmmac or in sxgbe's copy of it.
- mlxbf_gige likewise writes the bus table, from ACPI.
- bcmasp_intf, bcmmii and tsnep set PHY_MAC_INTERRUPT, and genphy does
not touch it: phy_interrupt_is_valid() is false for both PHY_POLL
and PHY_MAC_INTERRUPT, and it guards the substitution, so the
generic driver never demotes a MAC-served interrupt. They also set
it after connect - tsnep unconditionally, bcmasp for its internal
PHY and genet for an internal PHY that is not on v5 - so what a
restore hands back is replaced again.
icplus sets it from ip175c_read_status() for its switch ports and is
safe for the same reason.
The v7 commit message also named sxgbe as losing the interrupt. That
was wrong - the same dead probed_phy_irq guard - and it is gone.
The review [4] asked how this sits with Documentation/networking/phy.rst
telling MAC drivers to set phydev->irq directly. That contract is
per-connect: set it "before you call phy_start". The restore happens in
phy_detach(), between connections, so the value a MAC installs after a
connect still stands when phy_start() runs.
Longer term the substitution itself is what wants removing. There are
two of them and they are identical - phy_probe() and phy_attach_direct()
both do "if the bound driver has no interrupt callbacks and the number
looks valid, replace it with PHY_POLL" - so phy_interrupt_is_valid()
could ask whether the bound driver can service the interrupt and neither
site would need to write anything. That reaches every caller of the
helper, so it belongs in net-next and not here.
v7 1/2 ("net: phylink: unwind the PHY binding when bringup fails late")
has nothing to do with the interrupt and is posted for net on its own
alongside this series, which is why the patch count changed.
Changes since v10 [9]:
- Patch 4 restores the value phydev->irq held on entry to
phy_attach_direct(), kept in a local, instead of reading
mdiobus->irq[] (addressed review). The label is also reached when a
second attach of an already attached PHY fails its bind, and there
the bus table would have overwritten the live number; nor does the
table hold a PHY_MAC_INTERRUPT that a MAC wrote into phydev->irq
alone.
- Patch 4's changelog no longer claims its store is ordered before
d->driver is cleared (addressed review). Two plain stores are not
ordered for another CPU, and the unwind runs without the device lock,
as the bind it undoes does.
- Patch 3's changelog said the generic driver stays bound until the
release. After a sysfs unbind of it under a consumer,
is_genphy_driven stays set with no driver bound, so the store can
meet the probe of a driver binding meanwhile. The changelog now says
so, and why the next attach still requests a number that matches the
driver bound then (addressed review). It also says that a
PHY_MAC_INTERRUPT is replaced under the generic driver, and why that
is harmless in tree.
- Patch 1's changelog said a devicetree PHY node "still" overrides the
table. Before this patch the driver's own write came last and won, so
it is a change, and the changelog now says so.
- The paths this series does not cover are named above.
- Patch 4 touches the error_module_put lines that patch 2 of the attach
guard series [10] also rewrites; whichever lands second needs a
trivial rebase.
Changes since v9 [8]:
- Patch 3 restores under is_genphy_driven rather than above the block.
Unconditionally it also wrote a field this series has no claim on: a
MAC installing PHY_MAC_INTERRUPT writes phydev->irq alone, and a
phy_request_interrupt() that failed left PHY_POLL there while the bus
table still held the number that could not be requested.
- The v9 cover named a behaviour change, a PHY_POLL fallback from a
failed phy_request_interrupt() no longer surviving a detach. The
scoping takes it back, and that paragraph with it:
phy_request_interrupt() runs behind phy_interrupt_is_valid(), which
is false for as long as the generic driver is bound, so the fallback
never meets the restore.
- The -EBUSY sentence in patches 3 and 4 claimed more than the code
gives. The prober's half holds - __driver_probe_device() returns
-EBUSY while dev->driver is set - but neither the hand-bind in
phy_attach_direct() nor the store in phy_detach() holds the device
lock that device_bind_driver() asks its callers for, so what is there
is statement ordering. Both bodies say that now. Taking the lock is a
separate series.
- Patches 1 and 2 keep their code. Both changelogs now give the real
reason for filling the whole bus table, which is that a loop is
smaller than a second branch on the chip id; phy_mask already pins the
address to one entry everywhere except lan78xx's 7801 and smsc95xx's
external PHY.
- Patch 2 also puts the declarations in smsc95xx_bind() back in longest
to shortest order: the i it adds had made the int line longer than
the char line above it.
- The question whether lan78xx's fill of the bus table overrides a
per-PHY interrupt from the devicetree was against the v8 shape, which
wrote the entry in lan78xx_phy_init() after the bus was registered.
Since v9 it is written in lan78xx_mdio_init() ahead of
of_mdiobus_register(), and
fwnode_mdiobus_phy_device_register() then overwrites it from the PHY
node.
Changes since v8 [5]:
- Patch 1 fills the lan78xx bus table before the bus is registered
rather than one entry after the scan, and drops the write to
phydev->irq that phylib now does from the table itself [6]. The
netdev_dbg() that printed the field goes with it -
phylink_bringup_phy() prints the same number, at info level, as this
driver connects.
- Patch 2 does the same for smsc95xx, and drops its phydev->irq write
[7].
Changes since v7 [1]:
- The restore moved above device_release_driver(). After that call the
mdio device is bindable and the device lock is dropped, so a
phy_probe() on another CPU writes the same field. Holding the lock
across both, as the review [4] asked, is not available:
device_release_driver_internal() takes it itself. Ordering the store
ahead of the release puts it where the generic driver is still bound
and a driver registering meanwhile is turned away with -EBUSY before
it reaches phy_probe().
- New patch 4 for the phy_attach_direct() failure path. The v7 commit
message claimed detach covered every substitution; it does not cover
that one.
- v7 2/2 carried no Fixes: tag at all. It does now, and patch 4 has
its own.
- New patches 1 and 2, for lan78xx and smsc95xx.
- The sxgbe claim dropped.
- The phylink patch split out, see above.
Patch 3 is measured on that board. The one condition arranged for the
run is that the PHY driver module loads after the root filesystem
instead of from the early boot list the distribution normally uses -
that early list is also why a shipped image does not trip over this.
The distribution's own late-PHY handling was also removed, that being the
one patch which could have changed the outcome; upstream has nothing like
it. The unwind block of the phylink patch posted alongside this series is
in the kernel too and is not reached on either path: the -EINVAL failure
returns from phylink_bringup_phy() at its validate call, before
pl->phydev is assigned, and the -EIO failure returns from
phy_attach_direct() before phylink_bringup_phy() runs. The kernel is
still a distribution one and its remaining patches to phylink and
phy_device do run on these paths; none of them writes phydev->irq. The
generic driver then binds at 1.87 s and the real one between 13.4 and
13.6 s depending on the boot, and phydev->irq afterwards reads -1 without
the patch and 15 with it, 15 being the irq the devicetree interrupt of
that PHY maps to.
Patch 4 needs a generic probe that fails, which the board does not
produce on its own, so it went through a debug-only module parameter
that fails it once for one address. The connect then ends in -EIO rather
than the -EINVAL of the validation path, and the unwind takes the label
that patch touches; the same reading is -1 with patch 3 alone and 15
with both. That was measured again for v11, whose patch 4 takes the
value from the local, on two images of a 6.18.52 distribution kernel
that differ only by patch 4.
Patches 1 and 2 are compile-tested only - I have no LAN78xx or LAN95xx
device, and a Tested-by from someone who has one would be welcome.
[1] https://lore.kernel.org/r/20260909204306.2374562-1-f@lex.la/
[2] https://lore.kernel.org/r/8f67d3ba-ce25-49bf-8378-c76d748879a9@lunn.ch/
[3] https://lore.kernel.org/r/a2a2a8fb-97c3-498f-9bf4-e0c44be2eff6@lunn.ch/
[4] https://lore.kernel.org/r/20260915005946.823736-1-kuba@kernel.org/
[5] https://lore.kernel.org/r/20260918015029.2518425-1-f@lex.la/
[6] https://lore.kernel.org/r/df5d4af1-a86a-4820-9aeb-b1449a60f37e@lunn.ch/
[7] https://lore.kernel.org/r/6e14d6b7-2a5a-40e2-920e-fc69a6e85173@lunn.ch/
[8] https://lore.kernel.org/r/20260919015326.499479-1-f@lex.la/
[9] https://lore.kernel.org/r/20260922131955.4175785-1-f@lex.la/
[10] https://lore.kernel.org/r/20260924215951.2127682-1-f@lex.la/
Aleksei Sviridkin (4):
net: usb: lan78xx: register the PHY interrupt with the MDIO bus
net: usb: smsc95xx: register the PHY interrupt with the MDIO bus
net: phy: take the interrupt back from the bus on detach
net: phy: restore the interrupt when the generic bind cycle fails
drivers/net/phy/phy_device.c | 4 ++++
drivers/net/usb/lan78xx.c | 12 +++++-------
drivers/net/usb/smsc95xx.c | 6 ++++--
3 files changed, 13 insertions(+), 9 deletions(-)
base-commit: 11536ee3d3e0b1bd35b6f3f8df55a6053eb0c71d
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v11 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus
2026-09-26 23:50 [PATCH net v11 0/4] net: phy: keep a PHY interrupt across a generic bind cycle Aleksei Sviridkin
@ 2026-09-26 23:50 ` Aleksei Sviridkin
2026-09-27 18:29 ` Andrew Lunn
2026-09-26 23:50 ` [PATCH net v11 2/4] net: usb: smsc95xx: " Aleksei Sviridkin
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Aleksei Sviridkin @ 2026-09-26 23:50 UTC (permalink / raw)
To: netdev
Cc: andrew, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel,
Aleksei Sviridkin
The interrupt this driver maps for its PHY is written only into
phydev->irq, while the bus table mdiobus->irq[] keeps reading PHY_POLL
for the same address. That table is where phylib records what the bus
described - phy_device_create() seeds phydev->irq from it - so the
number lives only as long as nothing else writes that one field.
Put it in the table before the bus is registered, so that the PHY the
scan creates is born with the number, and drop the write to phydev->irq
that phylib then makes by itself. Fill the whole table rather than one
entry: for 7801 the address is not known until the scan, and for the
other two phy_mask leaves only address 1 readable, so a loop costs less
than a second switch on the chip id. A devicetree PHY node that
describes an interrupt now overrides that, where the driver's number
used to win.
Found going through the drivers that keep a PHY interrupt outside the
bus table, so that the restore on detach later in this series has a
number to hand back here as well.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Notes:
Compile-tested only; I have no LAN78xx device.
No Fixes: tag on this one. On its own it fixes nothing - nothing reads the
bus table back until patch 3 - which is also why it sorts ahead of that
patch rather than after it.
lan78xx_setup_irq_domain() runs before lan78xx_mdio_init() in
lan78xx_bind(), so the number is already mapped where the table is filled.
The fill covers the whole table because only the 7801 case leaves the
address open until of_mdiobus_register() has scanned; 7800 and 7850 set
phy_mask a few lines above, so every entry but address 1 is unreachable
and writing them costs nothing.
The fill is a default rather than an override. For a PHY node that
describes an interrupt, fwnode_mdiobus_phy_device_register() writes the
devicetree number over the table entry, and into phydev->irq, once the
device exists. That inverts the old order, where the driver's own number
was written last and won. Neither in-tree lan78xx PHY node carries an
interrupts property, so nothing in tree changes, but a devicetree that
described one would now be believed.
Teardown order keeps the number live for as long as it is read:
lan78xx_disconnect() detaches the PHY through phylink_disconnect_phy(), and
lan78xx_unbind() calls lan78xx_remove_irq_domain() only afterwards.
drivers/net/usb/lan78xx.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 5655941f1478..522fb4daeb46 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2092,6 +2092,7 @@ static int lan78xx_mdio_init(struct lan78xx_net *dev)
{
struct device_node *node;
int ret;
+ int i;
dev->mdiobus = mdiobus_alloc();
if (!dev->mdiobus) {
@@ -2118,6 +2119,10 @@ static int lan78xx_mdio_init(struct lan78xx_net *dev)
break;
}
+ if (dev->domain_data.phyirq > 0)
+ for (i = 0; i < PHY_MAX_ADDR; i++)
+ dev->mdiobus->irq[i] = dev->domain_data.phyirq;
+
node = of_get_child_by_name(dev->udev->dev.of_node, "mdio");
ret = of_mdiobus_register(dev->mdiobus, node);
of_node_put(node);
@@ -2892,13 +2897,6 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
return 0;
}
- /* if phyirq is not set, use polling mode in phylib */
- if (dev->domain_data.phyirq > 0)
- phydev->irq = dev->domain_data.phyirq;
- else
- phydev->irq = PHY_POLL;
- netdev_dbg(dev->net, "phydev->irq = %d\n", phydev->irq);
-
ret = phylink_connect_phy(dev->phylink, phydev);
if (ret) {
netdev_err(dev->net, "can't attach PHY to %s, error %pe\n",
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v11 2/4] net: usb: smsc95xx: register the PHY interrupt with the MDIO bus
2026-09-26 23:50 [PATCH net v11 0/4] net: phy: keep a PHY interrupt across a generic bind cycle Aleksei Sviridkin
2026-09-26 23:50 ` [PATCH net v11 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus Aleksei Sviridkin
@ 2026-09-26 23:50 ` Aleksei Sviridkin
2026-09-27 18:29 ` Andrew Lunn
2026-09-26 23:50 ` [PATCH net v11 3/4] net: phy: take the interrupt back from the bus on detach Aleksei Sviridkin
2026-09-26 23:50 ` [PATCH net v11 4/4] net: phy: restore the interrupt when the generic bind cycle fails Aleksei Sviridkin
3 siblings, 1 reply; 9+ messages in thread
From: Aleksei Sviridkin @ 2026-09-26 23:50 UTC (permalink / raw)
To: netdev
Cc: andrew, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel,
Aleksei Sviridkin
The interrupt this driver maps for its PHY is written only into
phydev->irq, while the bus table mdiobus->irq[] keeps reading PHY_POLL
for the same address. That table is where phylib records what the bus
described - phy_device_create() seeds phydev->irq from it - so the
number lives only as long as nothing else writes that one field.
The bus is the one this function is about to register, so put the number
in its table first and let the scan seed the PHY from there. The whole
table gets it: with an external PHY the address is not known until the
scan, and with the internal one phy_mask has already left a single
reachable entry, so a loop costs less than a branch on which case this
is.
Found going through the drivers that keep a PHY interrupt outside the
bus table, so that the restore on detach later in this series has a
number to hand back here as well.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Notes:
Compile-tested only; I have no LAN95xx device.
No Fixes: tag, for the same reason as patch 1: the write has no reader until
patch 3 lands.
The mapping is created before mdiobus_alloc(), so the number is in hand
where the table is filled. mdiobus_alloc_size() fills it with PHY_POLL,
the loop here is the only other write before mdiobus_register(), and the
scan only reads it, in phy_device_create(), so the fill survives. The
fill covers the whole table because the external-PHY case leaves the
address to phy_find_first() afterwards; on the internal path phy_mask
has already reduced it to one entry.
Teardown order keeps the number live for as long as it is read:
smsc95xx_unbind() disconnects the PHY before it disposes the interrupt
mapping.
drivers/net/usb/smsc95xx.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 42e4048b574b..b629092b94c2 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1147,8 +1147,8 @@ static void smsc95xx_handle_link_change(struct net_device *net)
static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
{
struct smsc95xx_priv *pdata;
+ int ret, phy_irq, i;
char usb_path[64];
- int ret, phy_irq;
u32 val;
ret = usbnet_get_endpoints(dev, intf);
@@ -1239,6 +1239,9 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
snprintf(pdata->mdiobus->id, ARRAY_SIZE(pdata->mdiobus->id),
"usb-%03d:%03d", dev->udev->bus->busnum, dev->udev->devnum);
+ for (i = 0; i < PHY_MAX_ADDR; i++)
+ pdata->mdiobus->irq[i] = phy_irq;
+
ret = mdiobus_register(pdata->mdiobus);
if (ret) {
netdev_err(dev->net, "Could not register MDIO bus\n");
@@ -1252,7 +1255,6 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
goto unregister_mdio;
}
- pdata->phydev->irq = phy_irq;
pdata->phydev->is_internal = pdata->is_internal_phy;
/* detect device revision as different features may be available */
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v11 3/4] net: phy: take the interrupt back from the bus on detach
2026-09-26 23:50 [PATCH net v11 0/4] net: phy: keep a PHY interrupt across a generic bind cycle Aleksei Sviridkin
2026-09-26 23:50 ` [PATCH net v11 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus Aleksei Sviridkin
2026-09-26 23:50 ` [PATCH net v11 2/4] net: usb: smsc95xx: " Aleksei Sviridkin
@ 2026-09-26 23:50 ` Aleksei Sviridkin
2026-09-27 18:35 ` Andrew Lunn
2026-09-26 23:50 ` [PATCH net v11 4/4] net: phy: restore the interrupt when the generic bind cycle fails Aleksei Sviridkin
3 siblings, 1 reply; 9+ messages in thread
From: Aleksei Sviridkin @ 2026-09-26 23:50 UTC (permalink / raw)
To: netdev
Cc: andrew, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel,
Aleksei Sviridkin
A PHY whose own driver is a module on a filesystem that is not mounted
when the MAC probes gets the generic driver first. phy_probe() replaces
phydev->irq with PHY_POLL because that driver has no interrupt support,
nothing puts it back, and the PHY polls for the rest of the uptime once
its real driver takes over. That is where a Keenetic KN-1012 stands,
with an Airoha EN8811H behind an MT7531 port and its driver on the root
filesystem: the devicetree interrupt of the PHY maps to irq 15, and once
the real driver has taken over the field reads -1.
Take the number back in phy_detach(), from mdiobus->irq[], which is
where phy_device_create() seeded phydev->irq from and where the bus that
described the interrupt still holds it. Only under is_genphy_driven,
since that is the substitution being undone: elsewhere the field belongs
to whoever wrote it, a MAC installing PHY_MAC_INTERRUPT writes
phydev->irq alone, and a phy_request_interrupt() that failed leaves
PHY_POLL there while the bus table still holds the number that could not
be requested. Under the generic driver a PHY_MAC_INTERRUPT written into
phydev->irq alone is still replaced; bcmasp, genet and tsnep, the MACs
that write it that way, write it again after each connect.
Do it before device_release_driver() rather than after. That call
returns with the mdio device bindable and the device lock dropped, so
from then on a phy_probe() on another CPU is the other writer of this
field; before it, the generic driver is bound and the driver core turns
such a probe away with -EBUSY. That is ordering, not exclusion: nothing
on this side holds the device lock. Nor does the ordering hold after a
sysfs unbind of the generic driver under a consumer, which leaves
is_genphy_driven set with no driver bound, so the store can race or
follow the probe of a driver that binds meanwhile. The next
phy_attach_direct() applies the substitution again for whichever driver
it finds bound, so the number its callers request still matches that
driver.
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.")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Notes:
Found and measured on a Keenetic KN-1012 (MT7981B, MT7531 switch) with an
Airoha EN8811H behind lan4, whose interrupt the devicetree describes and
whose driver is a module.
The one condition arranged for the run is that the PHY driver module loads
after the root filesystem rather than from the early boot list this
distribution normally puts it in. The distribution's own late-PHY handling
was also removed, that being the one patch which could have changed the
outcome; upstream has nothing like it. The kernel is still a distribution
one and its remaining patches to phylink and phy_device do run on these
paths - none of them writes phydev->irq.
DSA then sets the port up at 1.87 s, the generic driver is bound by hand,
phy_probe() replaces the interrupt with PHY_POLL, and phylink rejects
2500base-x against it:
lan4 (uninitialized): validation of 2500base-x ... failed: -EINVAL
lan4 (uninitialized): failed to connect to PHY: -EINVAL
The real driver arrives between 13.4 and 13.6 s depending on the boot, and
binds. phydev->irq then reads -1 without this patch and 15 with it, 15
being what the devicetree gave that PHY. The three switch ports alongside
read 79, 80 and 81 in both runs, so the reading distinguishes rather than
printing one answer. The field has no sysfs attribute of its own, so it was
read with a debug-only module parameter that walks the MDIO bus and prints
it.
The reading predates the is_genphy_driven guard the store now sits
under, which should not be implied away. On the measured path the flag
is set: phylink_fwnode_phy_connect(), which DSA reaches through
phylink_of_phy_connect() for this port, calls phy_detach() from its own
failure check, and the three writes of that flag are all in
phy_device.c, none of them between the hand-bind and that call. So the
guard passes and the store is the one the reading came from. Its other
side, a detach with a real driver bound, was not measured.
drivers/net/phy/phy_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 94b2e85e00a3..a9c71a286118 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1965,6 +1965,8 @@ void phy_detach(struct phy_device *phydev)
* real driver could be loaded
*/
if (phydev->is_genphy_driven) {
+ /* The release below lets phy_probe() write this field. */
+ phydev->irq = phydev->mdio.bus->irq[phydev->mdio.addr];
device_release_driver(&phydev->mdio.dev);
phydev->is_genphy_driven = 0;
}
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v11 4/4] net: phy: restore the interrupt when the generic bind cycle fails
2026-09-26 23:50 [PATCH net v11 0/4] net: phy: keep a PHY interrupt across a generic bind cycle Aleksei Sviridkin
` (2 preceding siblings ...)
2026-09-26 23:50 ` [PATCH net v11 3/4] net: phy: take the interrupt back from the bus on detach Aleksei Sviridkin
@ 2026-09-26 23:50 ` Aleksei Sviridkin
2026-09-27 18:38 ` Andrew Lunn
3 siblings, 1 reply; 9+ messages in thread
From: Aleksei Sviridkin @ 2026-09-26 23:50 UTC (permalink / raw)
To: netdev
Cc: andrew, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel,
Aleksei Sviridkin
phy_attach_direct() binds the generic driver by hand, and the probe it
calls is phy_probe(), which replaces phydev->irq with PHY_POLL before
either point the hand-bind can fail at. That failure unwinds on a label
of its own, which does not go through phy_detach(), so the substitution
outlives a bind cycle that never completed and a later attach finds a
PHY that can only be polled. Found on a Keenetic KN-1012 while placing
the restore of the previous patch, as the other exit of the same bind
cycle.
Save phydev->irq on entry and put it back on that label. The unwind runs
inside the call that made the substitution, so the value from before it
is known exactly. The bus table the previous patch reads from would be
wrong here twice over: it does not hold a PHY_MAC_INTERRUPT that a MAC
wrote into phydev->irq alone, and the label is also reached when a
second attach of an attached PHY fails its bind, where the field is
live.
The store is not ordered against a concurrent bind: this unwind, like
the hand-bind it undoes, runs without the device lock that
device_bind_driver() asks its callers to hold.
Fixes: 6d9f66ac7fec ("net: phy: Fix PHY module checks and NULL deref in phy_attach_direct()")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Notes:
Both points the hand-bind can fail at are reachable. phy_probe() reaches
genphy_read_abilities() through genphy_driver's .get_features, and that
returns the error from phy_read(phydev, MII_BMSR); device_bind_driver()
returns whatever driver_sysfs_add() got, from either of its two
sysfs_create_link() calls or from the coredump attribute.
A failed genphy bind leaves the device with no driver bound at all, so the
next driver to arrive binds directly and never goes through phy_detach().
That is why patch 3 cannot cover this path, and why the Fixes: tag here is
6d9f66ac7fec rather than the one patch 3 carries. That commit did not
introduce the lost number - the substitution is far older - it created this
second exit from the bind cycle, splitting the failure off the label that
calls phy_detach(). Before it, patch 3 alone would have covered this, so
that is where the backport range for this one starts.
Exercised on the board described in patch 3, with a debug-only module
parameter that fails the hand-bound generic probe once for one MDIO
address. The connect then ends in -EIO rather than the -EINVAL of the
validation path, so the unwind takes the label this patch touches.
Measured again for this version, since the value now comes from the
local: two images of the distribution's 6.18.52 kernel differing only
by this patch, injected failure at 2.0 s, real driver bound at 6.4 s.
phydev->irq afterwards reads -1 with patch 3 alone and 15 with this
one; the three switch ports read 79, 80 and 81 in both.
One difference between the injector and a real failure, since it does not
affect what was measured but should not be implied away: a genuine error
inside phy_probe() leaves through its out: label, which re-asserts the PHY
reset before returning, while the injector returns earlier than that.
Neither path touches phydev->irq.
drivers/net/phy/phy_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a9c71a286118..8bfb154402ad 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1755,6 +1755,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
struct mii_bus *bus = phydev->mdio.bus;
struct device *d = &phydev->mdio.dev;
struct module *ndev_owner = NULL;
+ int irq = phydev->irq;
int err;
/* For Ethernet device drivers that register their own MDIO bus, we
@@ -1896,6 +1897,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
error_module_put:
module_put(d->driver->owner);
+ phydev->irq = irq;
phydev->is_genphy_driven = 0;
d->driver = NULL;
error_put_device:
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v11 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus
2026-09-26 23:50 ` [PATCH net v11 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus Aleksei Sviridkin
@ 2026-09-27 18:29 ` Andrew Lunn
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2026-09-27 18:29 UTC (permalink / raw)
To: Aleksei Sviridkin
Cc: netdev, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel
On Sun, Sep 27, 2026 at 02:50:21AM +0300, Aleksei Sviridkin wrote:
> The interrupt this driver maps for its PHY is written only into
> phydev->irq, while the bus table mdiobus->irq[] keeps reading PHY_POLL
> for the same address. That table is where phylib records what the bus
> described - phy_device_create() seeds phydev->irq from it - so the
> number lives only as long as nothing else writes that one field.
>
> Put it in the table before the bus is registered, so that the PHY the
> scan creates is born with the number, and drop the write to phydev->irq
> that phylib then makes by itself. Fill the whole table rather than one
> entry: for 7801 the address is not known until the scan, and for the
> other two phy_mask leaves only address 1 readable, so a loop costs less
> than a second switch on the chip id. A devicetree PHY node that
> describes an interrupt now overrides that, where the driver's number
> used to win.
>
> Found going through the drivers that keep a PHY interrupt outside the
> bus table, so that the restore on detach later in this series has a
> number to hand back here as well.
>
> Assisted-by: LLM
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v11 2/4] net: usb: smsc95xx: register the PHY interrupt with the MDIO bus
2026-09-26 23:50 ` [PATCH net v11 2/4] net: usb: smsc95xx: " Aleksei Sviridkin
@ 2026-09-27 18:29 ` Andrew Lunn
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2026-09-27 18:29 UTC (permalink / raw)
To: Aleksei Sviridkin
Cc: netdev, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel
On Sun, Sep 27, 2026 at 02:50:22AM +0300, Aleksei Sviridkin wrote:
> The interrupt this driver maps for its PHY is written only into
> phydev->irq, while the bus table mdiobus->irq[] keeps reading PHY_POLL
> for the same address. That table is where phylib records what the bus
> described - phy_device_create() seeds phydev->irq from it - so the
> number lives only as long as nothing else writes that one field.
>
> The bus is the one this function is about to register, so put the number
> in its table first and let the scan seed the PHY from there. The whole
> table gets it: with an external PHY the address is not known until the
> scan, and with the internal one phy_mask has already left a single
> reachable entry, so a loop costs less than a branch on which case this
> is.
>
> Found going through the drivers that keep a PHY interrupt outside the
> bus table, so that the restore on detach later in this series has a
> number to hand back here as well.
>
> Assisted-by: LLM
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v11 3/4] net: phy: take the interrupt back from the bus on detach
2026-09-26 23:50 ` [PATCH net v11 3/4] net: phy: take the interrupt back from the bus on detach Aleksei Sviridkin
@ 2026-09-27 18:35 ` Andrew Lunn
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2026-09-27 18:35 UTC (permalink / raw)
To: Aleksei Sviridkin
Cc: netdev, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel
On Sun, Sep 27, 2026 at 02:50:23AM +0300, Aleksei Sviridkin wrote:
> A PHY whose own driver is a module on a filesystem that is not mounted
> when the MAC probes gets the generic driver first. phy_probe() replaces
> phydev->irq with PHY_POLL because that driver has no interrupt support,
> nothing puts it back, and the PHY polls for the rest of the uptime once
> its real driver takes over. That is where a Keenetic KN-1012 stands,
> with an Airoha EN8811H behind an MT7531 port and its driver on the root
> filesystem: the devicetree interrupt of the PHY maps to irq 15, and once
> the real driver has taken over the field reads -1.
>
> Take the number back in phy_detach(), from mdiobus->irq[], which is
> where phy_device_create() seeded phydev->irq from and where the bus that
> described the interrupt still holds it. Only under is_genphy_driven,
> since that is the substitution being undone: elsewhere the field belongs
> to whoever wrote it, a MAC installing PHY_MAC_INTERRUPT writes
> phydev->irq alone, and a phy_request_interrupt() that failed leaves
> PHY_POLL there while the bus table still holds the number that could not
> be requested. Under the generic driver a PHY_MAC_INTERRUPT written into
> phydev->irq alone is still replaced; bcmasp, genet and tsnep, the MACs
> that write it that way, write it again after each connect.
>
> Do it before device_release_driver() rather than after. That call
> returns with the mdio device bindable and the device lock dropped, so
> from then on a phy_probe() on another CPU is the other writer of this
> field; before it, the generic driver is bound and the driver core turns
> such a probe away with -EBUSY. That is ordering, not exclusion: nothing
> on this side holds the device lock. Nor does the ordering hold after a
> sysfs unbind of the generic driver under a consumer, which leaves
> is_genphy_driven set with no driver bound, so the store can race or
> follow the probe of a driver that binds meanwhile. The next
> phy_attach_direct() applies the substitution again for whichever driver
> it finds bound, so the number its callers request still matches that
> driver.
>
> 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.")
> Assisted-by: LLM
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
> ---
>
> Notes:
> Found and measured on a Keenetic KN-1012 (MT7981B, MT7531 switch) with an
> Airoha EN8811H behind lan4, whose interrupt the devicetree describes and
> whose driver is a module.
>
> The one condition arranged for the run is that the PHY driver module loads
> after the root filesystem rather than from the early boot list this
> distribution normally puts it in. The distribution's own late-PHY handling
> was also removed, that being the one patch which could have changed the
> outcome; upstream has nothing like it. The kernel is still a distribution
> one and its remaining patches to phylink and phy_device do run on these
> paths - none of them writes phydev->irq.
>
> DSA then sets the port up at 1.87 s, the generic driver is bound by hand,
> phy_probe() replaces the interrupt with PHY_POLL, and phylink rejects
> 2500base-x against it:
>
> lan4 (uninitialized): validation of 2500base-x ... failed: -EINVAL
> lan4 (uninitialized): failed to connect to PHY: -EINVAL
>
> The real driver arrives between 13.4 and 13.6 s depending on the boot, and
> binds. phydev->irq then reads -1 without this patch and 15 with it, 15
> being what the devicetree gave that PHY. The three switch ports alongside
> read 79, 80 and 81 in both runs, so the reading distinguishes rather than
> printing one answer. The field has no sysfs attribute of its own, so it was
> read with a debug-only module parameter that walks the MDIO bus and prints
> it.
>
> The reading predates the is_genphy_driven guard the store now sits
> under, which should not be implied away. On the measured path the flag
> is set: phylink_fwnode_phy_connect(), which DSA reaches through
> phylink_of_phy_connect() for this port, calls phy_detach() from its own
> failure check, and the three writes of that flag are all in
> phy_device.c, none of them between the hand-bind and that call. So the
> guard passes and the store is the one the reading came from. Its other
> side, a detach with a real driver bound, was not measured.
This is still way too much text, which no human is going to read it.
I actually suggest you write this by hand. If you cannot do that, you
don't understand the code sufficiently to actually submit it with your
Signed-of-by.
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v11 4/4] net: phy: restore the interrupt when the generic bind cycle fails
2026-09-26 23:50 ` [PATCH net v11 4/4] net: phy: restore the interrupt when the generic bind cycle fails Aleksei Sviridkin
@ 2026-09-27 18:38 ` Andrew Lunn
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2026-09-27 18:38 UTC (permalink / raw)
To: Aleksei Sviridkin
Cc: netdev, andrew+netdev, hkallweit1, linux, davem, edumazet, kuba,
pabeni, horms, olteanv, Thangaraj.S, UNGLinuxDriver,
steve.glendinning, f.fainelli, linux-usb, linux-kernel
On Sun, Sep 27, 2026 at 02:50:24AM +0300, Aleksei Sviridkin wrote:
> phy_attach_direct() binds the generic driver by hand, and the probe it
> calls is phy_probe(), which replaces phydev->irq with PHY_POLL before
> either point the hand-bind can fail at. That failure unwinds on a label
> of its own, which does not go through phy_detach(), so the substitution
> outlives a bind cycle that never completed and a later attach finds a
> PHY that can only be polled. Found on a Keenetic KN-1012 while placing
> the restore of the previous patch, as the other exit of the same bind
> cycle.
>
> Save phydev->irq on entry and put it back on that label. The unwind runs
> inside the call that made the substitution, so the value from before it
> is known exactly. The bus table the previous patch reads from would be
> wrong here twice over: it does not hold a PHY_MAC_INTERRUPT that a MAC
> wrote into phydev->irq alone, and the label is also reached when a
> second attach of an attached PHY fails its bind, where the field is
> live.
>
> The store is not ordered against a concurrent bind: this unwind, like
> the hand-bind it undoes, runs without the device lock that
> device_bind_driver() asks its callers to hold.
>
> Fixes: 6d9f66ac7fec ("net: phy: Fix PHY module checks and NULL deref in phy_attach_direct()")
> Assisted-by: LLM
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
> ---
>
> Notes:
> Both points the hand-bind can fail at are reachable. phy_probe() reaches
> genphy_read_abilities() through genphy_driver's .get_features, and that
> returns the error from phy_read(phydev, MII_BMSR); device_bind_driver()
> returns whatever driver_sysfs_add() got, from either of its two
> sysfs_create_link() calls or from the coredump attribute.
>
> A failed genphy bind leaves the device with no driver bound at all, so the
> next driver to arrive binds directly and never goes through phy_detach().
> That is why patch 3 cannot cover this path, and why the Fixes: tag here is
> 6d9f66ac7fec rather than the one patch 3 carries. That commit did not
> introduce the lost number - the substitution is far older - it created this
> second exit from the bind cycle, splitting the failure off the label that
> calls phy_detach(). Before it, patch 3 alone would have covered this, so
> that is where the backport range for this one starts.
>
> Exercised on the board described in patch 3, with a debug-only module
> parameter that fails the hand-bound generic probe once for one MDIO
> address. The connect then ends in -EIO rather than the -EINVAL of the
> validation path, so the unwind takes the label this patch touches.
> Measured again for this version, since the value now comes from the
> local: two images of the distribution's 6.18.52 kernel differing only
> by this patch, injected failure at 2.0 s, real driver bound at 6.4 s.
> phydev->irq afterwards reads -1 with patch 3 alone and 15 with this
> one; the three switch ports read 79, 80 and 81 in both.
>
> One difference between the injector and a real failure, since it does not
> affect what was measured but should not be implied away: a genuine error
> inside phy_probe() leaves through its out: label, which re-asserts the PHY
> reset before returning, while the injector returns earlier than that.
> Neither path touches phydev->irq.
Again, way too much text.
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-09-27 18:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-26 23:50 [PATCH net v11 0/4] net: phy: keep a PHY interrupt across a generic bind cycle Aleksei Sviridkin
2026-09-26 23:50 ` [PATCH net v11 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus Aleksei Sviridkin
2026-09-27 18:29 ` Andrew Lunn
2026-09-26 23:50 ` [PATCH net v11 2/4] net: usb: smsc95xx: " Aleksei Sviridkin
2026-09-27 18:29 ` Andrew Lunn
2026-09-26 23:50 ` [PATCH net v11 3/4] net: phy: take the interrupt back from the bus on detach Aleksei Sviridkin
2026-09-27 18:35 ` Andrew Lunn
2026-09-26 23:50 ` [PATCH net v11 4/4] net: phy: restore the interrupt when the generic bind cycle fails Aleksei Sviridkin
2026-09-27 18:38 ` Andrew Lunn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®