Re: [PATCH net-next v8 3/9] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Calling phy_attach_direct() on an already-attached PHY destructively tears down the existing valid attachment and leaves the bus driver state dangling.
--

--- Patch [3]: [PATCH net-next v8 3/9] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus ---
commit f45546de5e393510abd217da3bdb7bad6d759d93
Author: Daniel Golle <daniel@makrotopia.org>

net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

This commit introduces notify_phy_attach and notify_phy_detach hooks to
the mii_bus structure. This allows MDIO bus drivers to handle PHY specific
configurations, such as polling registers, symmetrically during attach and
detach phases.

Link: https://github.com/openwrt/openwrt/pull/21515#discussion_r2714069716
Link: https://lore.kernel.org/netdev/cover.1769053496.git.daniel@makrotopia.org/

> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c

[ ... ]

> @@ -1890,8 +1986,8 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
>  	return err;
>  
>  error:
> -	/* phy_detach() does all of the cleanup below */
> -	phy_detach(phydev);
> +	/* cleanup without bus notification */
> +	__phy_detach(phydev, false);
>  	return err;

[Severity: High]
This is a pre-existing issue, but does this error path incorrectly tear down
a valid pre-existing attachment when phy_attach_direct() is called on an
already-attached PHY?

When phy_attach_direct() detects the PHY is already attached:

drivers/net/phy/phy_device.c:phy_attach_direct() {
        ...
        if (phydev->attached_dev) {
                dev_err(&dev->dev, "PHY already attached\n");
                err = -EBUSY;
                goto error;
        }
        ...
}

It jumps to the shared error path. Since the error path calls
__phy_detach(phydev, false) and phydev->attached_dev still points to the
old, valid network device, it destroys the existing attachment behind the
scenes.

Additionally, because the patch explicitly suppresses the bus detach
notification by passing false to __phy_detach(), doesn't this leave the
MDIO bus driver's state (like hardware polling configurations) permanently
out of sync?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729160227.155613-1-markus.stockhausen@gmx.de?part=3