[PATCH net v12 0/4] net: phy: keep a PHY interrupt across a generic bind cycle

[PATCH net v12 0/4] net: phy: keep a PHY interrupt across a generic bind cycle

[Aleksei Sviridkin]

On a Keenetic KN-1012 the EN8811H PHY behind lan4 has its driver as a
module on the root filesystem, so it binds to the generic driver first.
phy_probe() replaces its interrupt with PHY_POLL, and nothing puts it
back when the real driver takes over, so the PHY is polled for the rest
of the uptime.

Patches 1 and 2 put the lan78xx and smsc95xx PHY interrupts into the
MDIO bus table, so there is a number to restore for them too. Patch 3
restores the interrupt from that table in phy_detach(). Patch 4
restores it when the generic bind fails inside phy_attach_direct().

Not covered: a driver without interrupt support that is bound or
unbound through sysfs still leaves PHY_POLL behind. Removing the
substitution from phy_probe() would fix that too, and belongs in
net-next.

Patches 1 and 2 are compile-tested only; a Tested-by from someone with
a LAN78xx or LAN95xx device would help. Patch 4 touches lines that
patch 2 of the attach guard series [2] also changes, so whichever lands
second needs a trivial rebase.

Other drivers that write phydev->irq outside the bus table, checked
against net/main:
 - ixp4xx_eth, ax88796c and emac-mac force PHY_POLL themselves, and
   none of them attaches again after a detach without forcing it again.
 - stmmac and mlxbf_gige already write mdiobus->irq[] as well.
 - bcmasp and genet (for an internal PHY) and tsnep set
   PHY_MAC_INTERRUPT again after each connect, so a restore that
   overwrote it does not last. icplus sets it on every status read.
 - ucc_geth only reads the field.

The full changelog up to v11 is in the v11 cover [1].

Changes since v11 [1]:
 - Shorter commit messages for patches 3 and 4, and a shorter cover.
   No code change.
 - Reviewed-by from Andrew on patches 1 and 2.

[1] https://lore.kernel.org/r/20260926235024.705646-1-f@lex.la/
[2] 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: a7bfaba4823e3c165bb2004c74eff7c096672bc7
-- 
2.53.0

[PATCH net v12 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus

[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.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---

Notes:
    v12: no change.
    
    Compile-tested only; I have no LAN78xx device. No Fixes: tag, since
    nothing reads the bus table back until patch 3.

 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

[PATCH net v12 2/4] net: usb: smsc95xx: register the PHY interrupt with the MDIO bus

[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.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---

Notes:
    v12: no change.
    
    Compile-tested only; I have no LAN95xx device. No Fixes: tag, for the
    same reason as patch 1.

 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

[PATCH net v12 3/4] net: phy: take the interrupt back from the bus on detach

[Aleksei Sviridkin]

When a PHY's driver is a module that is not loaded yet when the MAC
connects, the PHY gets the generic driver first. phy_probe() then
replaces phydev->irq with PHY_POLL, because genphy has no interrupt
support. Nothing puts the number back, so after the real driver binds
the PHY is polled for the rest of the uptime.

Seen on a Keenetic KN-1012: the Airoha EN8811H behind an MT7531 port
has its driver on the root filesystem. Its devicetree interrupt maps to
irq 15, and after the real driver binds phydev->irq reads -1.

Restore the number when the PHY detaches. It comes from the bus table,
mdiobus->irq[], where the PHY got it at creation. Do it only when
phy_attach_direct() bound the generic driver, since that is the
substitution being undone; otherwise the field belongs to whoever wrote
it. Do it before device_release_driver(), because after the release a
probing driver can write the same field.

The store is ordered before the release rather than locked against it:
device_release_driver() takes the device lock itself. A MAC that sets
phydev->irq before phy_start(), as phy.rst describes, is not affected,
since the restore runs on detach, between connections.

Tested on the KN-1012 with a 6.18 distribution kernel: phydev->irq
reads 15 after the real driver binds, and -1 without this patch.

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:
    v12: shorter commit message, no code change.
    
    The PHY module was made to load after the root filesystem, and the
    distribution's own late-PHY patch was removed. The value was read with a
    debug-only module parameter; the three switch ports read 79, 80 and 81
    in both runs.

 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

[PATCH net v12 4/4] net: phy: restore the interrupt when the generic bind cycle fails

[Aleksei Sviridkin]

When the generic driver is bound by hand and the bind fails, the PHY is
left with polling in place of its interrupt. phy_probe() has already
replaced phydev->irq with PHY_POLL by then. The unwind does not go
through phy_detach(), so the restore there does not run, and a later
attach finds a PHY that can only be polled.

Found on a Keenetic KN-1012 while adding the restore in phy_detach(),
as the other way out of the same bind cycle.

Save the interrupt number on entry and put it back on the error path.
The bus table is not the right source here. The same label is reached
when a second attach of an already attached PHY fails, and there the
field is live. The table also misses a PHY_MAC_INTERRUPT that a MAC
wrote into phydev->irq.

Tested on the KN-1012 with a 6.18 distribution kernel and an injected
failure of the generic probe: phydev->irq reads 15 afterwards, and -1
with only the restore in phy_detach().

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:
    v12: shorter commit message, no code change.
    
    The Fixes: tag differs from patch 3 because 6d9f66ac7fec split this
    failure off the label that calls phy_detach(). The failure was injected
    with a debug-only module parameter, once for one MDIO address.

 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

Re: [PATCH net v12 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In lan78xx_mdio_init() (drivers/net/usb/lan78xx.c:2122-2124),
  the one mapped PHY interrupt (domain_data.phyirq, backed only by…

--

net: usb: lan78xx: register the PHY interrupt with the MDIO bus

This patch moves the lan78xx PHY interrupt out of lan78xx_phy_init(),
where it was written directly to phydev->irq. It now goes into the
mii_bus irq[] table before of_mdiobus_register() is called. Every entry
in the table is set to domain_data.phyirq, so the PHY created by the
scan picks it up through phy_device_create().

> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 5655941f14783..522fb4daeb468 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c

[ ... ]

> @@ -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;
> +

[Severity: Medium]
On LAN7801, could this mark PHYs that are not wired to the LAN7801 PHY_INT
input as interrupt driven?

The 7801 case in the switch above leaves phy_mask at 0:

	case ID_REV_CHIP_ID_7801_:
		break;

So every PHY the scan finds takes its irq from this table in
phy_device_create():

	dev->irq = bus->irq[addr];

A child of the "mdio" DT node that has no "interrupts" property gets the
same value from fwnode_mdiobus_phy_device_register():

	} else {
		phy->irq = mdio->irq[addr];
	}

But only one interrupt is mapped, in lan78xx_setup_irq_domain():

	irqmap = irq_create_mapping(irqdomain, INT_EP_PHY);

lan78xx_status() also raises it only for the single INT_ENP_PHY_INT
status bit.

Before this patch, only the PHY that phy_find_first() returned in
lan78xx_get_phy() had phydev->irq set to phyirq. Every other PHY on the
bus kept PHY_POLL.

Suppose another consumer attaches a second PHY on a 7801 MDIO bus, for
example a DSA port or another MAC that uses phy-handle. If that PHY's
driver supports interrupts, phylink_bringup_phy() or phy_connect() would
call phy_request_interrupt() on phyirq, and phylib would stop polling
that PHY.

Would link changes on that PHY then go unreported, since its INT pin is
not connected to the LAN7801?

A devicetree author also seems to lose the ability to ask for polling on
such a PHY, because leaving out "interrupts" now gives it phyirq.

The commit message says:

  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.

Does this reasoning cover the PHYs on the bus other than the one lan78xx
attaches itself? The later patches in the series don't seem to touch
lan78xx.c, so this behaviour would still be there once the whole series
is applied.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927235733.2619081-1-f%40lex.la

Re: [PATCH net v12 3/4] net: phy: take the interrupt back from the bus on detach

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In this patch alone ("net: phy: take the interrupt back from the
  bus on detach"), phydev->irq is restored only in phy_detach().

--

net: phy: take the interrupt back from the bus on detach

When a PHY is first bound to the generic driver, phy_probe() replaces
phydev->irq with PHY_POLL, and the value is never put back. After that,
the PHY stays in polling mode even once the real driver binds. This patch
restores phydev->irq from mdiobus->irq[] in phy_detach() before the
genphy driver is released.

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 94b2e85e00a37..a9c71a2861186 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];

[Severity: Low]
phy_attach_direct() has a second way to undo the genphy bind, and that
path doesn't restore phydev->irq.

This patch only adds the restore to phy_detach(). phy_attach_direct() can
also back out of the genphy bind:

drivers/net/phy/phy_device.c:phy_attach_direct() {
    ...
	if (phydev->is_genphy_driven) {
		err = d->driver->probe(d);
		if (err >= 0)
			err = device_bind_driver(d);

		if (err)
			goto error_module_put;
	}
    ...
error_module_put:
	module_put(d->driver->owner);
	phydev->is_genphy_driven = 0;
	d->driver = NULL;
    ...
}

phy_probe() sets phydev->irq = PHY_POLL before steps that can still fail.
Two examples are an MDIO read error in genphy_read_abilities() and
-ENOMEM in phy_setup_ports(). device_bind_driver() can also fail.

In those cases the error_module_put path clears is_genphy_driven and
d->driver, but phydev->irq keeps the PHY_POLL value. If the PHY device
survives and a real driver binds later, the PHY would still be polled.

The next patch in the series, "net: phy: restore the interrupt when the
generic bind cycle fails", closes this gap. It saves phydev->irq in a local
at the start of phy_attach_direct() and writes it back under
error_module_put. So the gap only exists between these two commits.

Would it be worth folding that change into this patch, so both genphy
unbind paths restore phydev->irq in the same commit?

>  		device_release_driver(&phydev->mdio.dev);
>  		phydev->is_genphy_driven = 0;
>  	}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927235733.2619081-1-f%40lex.la

Re: [PATCH net v12 4/4] net: phy: restore the interrupt when the generic bind cycle fails

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] When a genphy-driven PHY is attached a second time, the
  error_module_put unwind clears the state of the live first attachment.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the
  error_module_put unwind in phy_attach_direct() does not undo a…

--

net: phy: restore the interrupt when the generic bind cycle fails

This commit saves phydev->irq on entry to phy_attach_direct() and writes it
back on the error_module_put path. The goal is that a failed genphy
probe/bind no longer leaves the PHY in PHY_POLL.

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index a9c71a2861186..8bfb154402adc 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;

[Severity: High]
The commit message says irq is saved because "the same label is reached
when a second attach of an already attached PHY fails, and there the field
is live". Writing irq back on that path does no harm. What about the other
two writes under this label?

Take a second phy_attach_direct() on a genphy-driven PHY that is still
attached. d->driver is non-NULL, but is_genphy_driven is still 1 from the
first attach. So this block runs again, before the attached_dev check:

    if (phydev->is_genphy_driven) {
        err = d->driver->probe(d);
        if (err >= 0)
            err = device_bind_driver(d);

        if (err)
            goto error_module_put;
    }

phy_probe() runs again on the live PHY. device_bind_driver() then fails
with -EEXIST in driver_sysfs_add(), because the sysfs links from the first
bind are still there.

That sends control to error_module_put. It clears is_genphy_driven and
d->driver on a PHY that is still bound in the driver core and still
attached to the first net_device.

Later the first owner calls phy_detach(), and both of these branches are
skipped:

    if (phydev->mdio.dev.driver)
        module_put(phydev->mdio.dev.driver->owner);
    ...
    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;
    }

As a result:

  - The first attach's driver module reference leaks.
  - device_release_driver() and phy_remove() never run.
  - irq is never restored from the bus table.

Would every later attach then fail with -EEXIST, in device_bind_driver()
for genphy or in really_probe() for a real PHY driver? phydev->irq would
stay at PHY_POLL for good, and the phy_device would leak through its klist
reference.

That is the end state this patch tries to prevent, reached through the
path the commit message names.

The clearing itself is older than this patch. It comes from 6d9f66ac7fec
and from the persistent is_genphy_driven flag added in 2796ff1e3dcae7.
Since this patch depends on that path, could genphy be probed, bound and
unwound only when this call assigned d->driver (for example with a local
flag)? Another option would be to check attached_dev before the genphy
probe/bind.

[Severity: Medium]
This is a pre-existing issue, not one this patch introduces. When
phy_probe() succeeds and device_bind_driver() then fails, does
error_module_put leak what phy_probe() set up?

A successful phy_probe() leaves behind:

  - phydev->drv set
  - the reset line released by phy_device_reset(phydev, 0)
  - ports allocated by phy_setup_ports()
  - LED triggers allocated and registered by phy_led_triggers_register()

device_bind_driver() does not call the driver's remove callback when it
fails:

    ret = driver_sysfs_add(dev);
    if (!ret) {
        device_links_force_bind(dev);
        driver_bound(dev);
    }
    else
        bus_notify(dev, BUS_NOTIFY_DRIVER_NOT_BOUND);

error_module_put only restores irq, is_genphy_driven and d->driver. The
teardown that phy_remove() would do never happens:

  - phy_led_triggers_unregister() is not called
  - phy_cleanup_ports() is not called
  - phy_device_reset(phydev, 1) is not called
  - phydev->drv is not cleared

On a retried attach, phy_led_triggers_register() overwrites
phydev->led_link_trigger and phydev->phy_led_triggers. The duplicate
registrations fail with -EEXIST, so the original triggers stay registered
and leak. The PHY is also left out of reset with no driver bound.

This path is reached when driver_sysfs_add() fails on a first attach. It
is also reached on every attach after the second-attach case above.

>  error_put_device:

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927235733.2619081-1-f%40lex.la

Re: [PATCH net v12 1/4] net: usb: lan78xx: register the PHY interrupt with the MDIO bus

[Andrew Lunn]

On Wed, Sep 30, 2026 at 12:58:20AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] In lan78xx_mdio_init() (drivers/net/usb/lan78xx.c:2122-2124),
>   the one mapped PHY interrupt (domain_data.phyirq, backed only by…
> 
> --
> 
> net: usb: lan78xx: register the PHY interrupt with the MDIO bus
> 
> This patch moves the lan78xx PHY interrupt out of lan78xx_phy_init(),
> where it was written directly to phydev->irq. It now goes into the
> mii_bus irq[] table before of_mdiobus_register() is called. Every entry
> in the table is set to domain_data.phyirq, so the PHY created by the
> scan picks it up through phy_device_create().
> 
> > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> > index 5655941f14783..522fb4daeb468 100644
> > --- a/drivers/net/usb/lan78xx.c
> > +++ b/drivers/net/usb/lan78xx.c
> 
> [ ... ]
> 
> > @@ -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;
> > +
> 
> [Severity: Medium]
> On LAN7801, could this mark PHYs that are not wired to the LAN7801 PHY_INT
> input as interrupt driven?

Since this is a USB dongle, it is unlikely. However, i agree with the
general principal. The code should only change the one entry the PHY
is actually using. Please look at all the patches in this series.

   Andrew