Re: [net-next,v15,05/13] net: phy: add (*notify_phy_attach/detach)() hooks to struct mii_bus

[netdev-bot+sashiko@kernel.org]

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

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

- [Low] New struct mii_bus callbacks (notify_phy_attach /
  notify_phy_detach) and their core call sites in phy_attach_direct() /…

--

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

This adds two optional callbacks to struct mii_bus so an MDIO bus driver
can be told when a PHY is attached or detached. notify_phy_attach() is
invoked in phy_attach_direct() after phy_init_hw() and before
phy_resume(), and notify_phy_detach() is invoked from phy_detach() right
after phy_suspend(). phy_detach_internal() gains a notify_bus argument so
the attach error path does not emit a detach notification.

References from the commit message:
[1] https://github.com/openwrt/openwrt/pull/21515#discussion_r2714069716
[2] https://lore.kernel.org/netdev/cover.1769053496.git.daniel@makrotopia.org/
[3] https://lore.kernel.org/netdev/9e40f50b-357a-4a93-9f59-94847850835d@lunn.ch/#t

> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 69b04a92b7766..41d1b20cde2c0 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c

[ ... ]

> @@ -1997,6 +2003,12 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
>  	if (err)
>  		goto error;
>  
> +	if (phydev->mdio.bus->notify_phy_attach) {
> +		err = phydev->mdio.bus->notify_phy_attach(phydev);
> +		if (err)
> +			goto error;
> +	}
> +
>  	phy_resume(phydev);

[ ... ]

> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 5f8d65868e0ff..3d8afe6b7f1c5 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -376,6 +376,24 @@ struct mii_bus {
>  			 int regnum, u16 val);
>  	/** @reset: Perform a reset of the bus */
>  	int (*reset)(struct mii_bus *bus);
> +	/**
> +	 * @notify_phy_attach: Perform post-attach handling for MDIO bus
> +	 * drivers. Optional and independent of @notify_phy_detach. Called
> +	 * in phy_attach_direct() right before phy_resume(). Runs in process
> +	 * context, may sleep and may be called with RTNL held. Must not
> +	 * acquire or rely on RTNL. Returns 0 on success or negative errno
> +	 * on failure. Must unwind its own state on error as attachment is
> +	 * aborted.
> +	 */
> +	int (*notify_phy_attach)(struct phy_device *phydev);
> +	/**
> +	 * @notify_phy_detach: Perform pre-detach handling for MDIO bus
> +	 * drivers. Optional and independent of @notify_phy_attach. Called
> +	 * in phy_detach() right after phy_suspend(). Runs in process context,
> +	 * may sleep and may be called with RTNL held. Must not acquire or
> +	 * rely on RTNL.
> +	 */
> +	void (*notify_phy_detach)(struct phy_device *phydev);

[Severity: Low]
This isn't a bug, but at this commit nothing in the tree assigns
mii_bus::notify_phy_attach or mii_bus::notify_phy_detach, so both new
members and the two call sites in phy_attach_direct() and
phy_detach_internal() are inert here. Both call sites are NULL-checked, so
there is no misbehaviour.

Checking the rest of the series, the first user does arrive later:
drivers/net/mdio/mdio-realtek-rtl9300.c adds
otto_emdio_notify_phy_attach()/otto_emdio_notify_phy_detach() and wires
them up in otto_emdio_probe_one():

	bus->notify_phy_attach = otto_emdio_notify_phy_attach;
	bus->notify_phy_detach = otto_emdio_notify_phy_detach;

Would it be worth mentioning in the changelog that the first consumer of
these hooks is added by the following patch in the same series ("net: mdio:
realtek-rtl9300: Configure hardware polling during probing"), so readers of
this commit alone know the API is not left without a user?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831143439.2404484-1-markus.stockhausen%40gmx.de