Re: [PATCH net v13 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 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 this commit only, the interrupt restore lives only in
  phy_detach() (drivers/net/phy/phy_device.c:1969).

--

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

This commit makes phy_detach() restore phydev->irq from the bus table
(mdio.bus->irq[]) when the generic driver was bound by
phy_attach_direct(). It undoes the PHY_POLL substitution done by
phy_probe() for genphy, so that a real driver bound later can use the
interrupt.

> 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: Medium]
This patch only restores the irq in phy_detach(). Is there a second way
out of the genphy bind cycle that still leaves phydev->irq at PHY_POLL?

In phy_attach_direct(), the generic probe and bind failures go straight
to error_module_put. That path never reaches the error label that calls
phy_detach():

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() overwrites the irq before any of its steps that can fail:

drivers/net/phy/phy_device.c:phy_probe() {
    ...
	if (!phy_drv_supports_irq(phydrv) && phy_interrupt_is_valid(phydev))
		phydev->irq = PHY_POLL;
    ...
}

The later steps include get_features, genphy_read_abilities() and
genphy_c45_pma_read_abilities() (all MDIO reads), phy_setup_ports() and
genphy_c45_read_eee_adv(). If one of them fails, or device_bind_driver()
fails, error_module_put clears is_genphy_driven and d->driver but does
not touch phydev->irq. A real driver that binds later would then still
poll, which is the symptom the commit message describes.

The last patch of this series, "net: phy: restore the interrupt when the
generic bind cycle fails", appears to close this gap. It saves
phydev->irq at the start of phy_attach_direct() and restores it under
error_module_put. At this commit on its own, though, the restore covers
only the phy_detach() path.

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

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