From: Daniel Golle Some MDIO buses require programiming PHY polling registers depending on the PHY type. RealTek switch SoCs are the most prominent example of a DSA switch which doesn't allow to program MAC speed, duplex and flow-control settings without using PHY polling to do so [1]. Avoid a half-baked solution in the MDIO bus driver because - it must reinvent the bus scanning to determine the PHYs and - it must anticipate the right point in time (e.g. deferred PHYs). Hence there is a need to inform the MDIO bus driver that a PHY is being attached or detached. Provide two hooks in struct mii_bus - notify_phy_attach(): called in phy_attach_direct() right after PHY hardware has been initialized and before PHY is resumed. - notify_phy_detach(): called in phy_detach() right after PHY has been suspended. The implementation is symmetric and follows LIFO teardown. The detach notifier is only called when the attach notification succeeded. For this - Relocate code from phy_detach() into a new helper __phy_detach() - The helper takes an additional parameter notify_bus that decides if the bus notification should be sent or not. - Call the helper with notification from slimmed down version of phy_detach() and without notification from phy_attach_direct() error path. Remark! A slightly different version of this patch was part of a former series [2]. The discussion already showed that an initialization hook should be placed somewhere late during the whole setup. This commit implants it right after phy_init_hw() as suggested. On top of this it adds the detach hook. [1] https://github.com/openwrt/openwrt/pull/21515#discussion_r2714069716 [2] https://lore.kernel.org/netdev/cover.1769053496.git.daniel@makrotopia.org/ Signed-off-by: Daniel Golle Signed-off-by: Markus Stockhausen --- drivers/net/phy/phy_device.c | 179 +++++++++++++++++++---------------- include/linux/phy.h | 18 ++++ 2 files changed, 115 insertions(+), 82 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 94b2e85e00a3..38b31fc272d7 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1734,6 +1734,95 @@ static bool phy_drv_supports_irq(const struct phy_driver *phydrv) return phydrv->config_intr && phydrv->handle_interrupt; } +static void __phy_detach(struct phy_device *phydev, bool notify_bus) +{ + struct net_device *dev = phydev->attached_dev; + struct module *ndev_owner = NULL; + struct mii_bus *bus; + + if (phydev->devlink) { + device_link_del(phydev->devlink); + phydev->devlink = NULL; + } + + if (phydev->sysfs_links) { + if (dev) + sysfs_remove_link(&dev->dev.kobj, "phydev"); + sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev"); + } + + if (!phydev->attached_dev) + sysfs_remove_file(&phydev->mdio.dev.kobj, + &dev_attr_phy_standalone.attr); + + phy_suspend(phydev); + if (dev) { + struct hwtstamp_provider *hwprov; + + /* hwprov may technically be protected by ops lock but + * not for devices with a phydev, see phy_link_topo_add_phy() + */ + hwprov = rtnl_dereference(dev->hwprov); + /* Disable timestamp if it is the one selected */ + if (hwprov && hwprov->phydev == phydev) { + rcu_assign_pointer(dev->hwprov, NULL); + kfree_rcu(hwprov, rcu_head); + } + + phydev->attached_dev->phydev = NULL; + phydev->attached_dev = NULL; + phy_link_topo_del_phy(dev, phydev); + } + + phydev->phy_link_change = NULL; + phydev->phylink = NULL; + + if (notify_bus && phydev->mdio.bus->notify_phy_detach) + phydev->mdio.bus->notify_phy_detach(phydev); + + if (phydev->mdio.dev.driver) + module_put(phydev->mdio.dev.driver->owner); + + /* If the device had no specific driver before (i.e. - it + * was using the generic driver), we unbind the device + * from the generic driver so that there's a chance a + * real driver could be loaded + */ + if (phydev->is_genphy_driven) { + device_release_driver(&phydev->mdio.dev); + phydev->is_genphy_driven = 0; + } + + /* Assert the reset signal */ + phy_device_reset(phydev, 1); + + /* + * The phydev might go away on the put_device() below, so avoid + * a use-after-free bug by reading the underlying bus first. + */ + bus = phydev->mdio.bus; + + put_device(&phydev->mdio.dev); + if (dev) + ndev_owner = dev->dev.parent->driver->owner; + if (ndev_owner != bus->owner) + module_put(bus->owner); +} + +/** + * phy_detach - detach a PHY device from its network device + * @phydev: target phy_device struct + * + * This detaches the phy device from its network device and the phy + * driver, and drops the reference count taken in phy_attach_direct(). + */ +void phy_detach(struct phy_device *phydev) +{ + /* cleanup including bus notification */ + __phy_detach(phydev, true); +} +EXPORT_SYMBOL(phy_detach); + /** * phy_attach_direct - attach a network device to a given PHY device pointer * @dev: network device to attach @@ -1876,6 +1965,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); /** @@ -1890,8 +1985,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; error_module_put: @@ -1906,86 +2001,6 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, } EXPORT_SYMBOL(phy_attach_direct); -/** - * phy_detach - detach a PHY device from its network device - * @phydev: target phy_device struct - * - * This detaches the phy device from its network device and the phy - * driver, and drops the reference count taken in phy_attach_direct(). - */ -void phy_detach(struct phy_device *phydev) -{ - struct net_device *dev = phydev->attached_dev; - struct module *ndev_owner = NULL; - struct mii_bus *bus; - - if (phydev->devlink) { - device_link_del(phydev->devlink); - phydev->devlink = NULL; - } - - if (phydev->sysfs_links) { - if (dev) - sysfs_remove_link(&dev->dev.kobj, "phydev"); - sysfs_remove_link(&phydev->mdio.dev.kobj, "attached_dev"); - } - - if (!phydev->attached_dev) - sysfs_remove_file(&phydev->mdio.dev.kobj, - &dev_attr_phy_standalone.attr); - - phy_suspend(phydev); - if (dev) { - struct hwtstamp_provider *hwprov; - - /* hwprov may technically be protected by ops lock but - * not for devices with a phydev, see phy_link_topo_add_phy() - */ - hwprov = rtnl_dereference(dev->hwprov); - /* Disable timestamp if it is the one selected */ - if (hwprov && hwprov->phydev == phydev) { - rcu_assign_pointer(dev->hwprov, NULL); - kfree_rcu(hwprov, rcu_head); - } - - phydev->attached_dev->phydev = NULL; - phydev->attached_dev = NULL; - phy_link_topo_del_phy(dev, phydev); - } - - phydev->phy_link_change = NULL; - phydev->phylink = NULL; - - if (phydev->mdio.dev.driver) - module_put(phydev->mdio.dev.driver->owner); - - /* If the device had no specific driver before (i.e. - it - * was using the generic driver), we unbind the device - * from the generic driver so that there's a chance a - * real driver could be loaded - */ - if (phydev->is_genphy_driven) { - device_release_driver(&phydev->mdio.dev); - phydev->is_genphy_driven = 0; - } - - /* Assert the reset signal */ - phy_device_reset(phydev, 1); - - /* - * The phydev might go away on the put_device() below, so avoid - * a use-after-free bug by reading the underlying bus first. - */ - bus = phydev->mdio.bus; - - put_device(&phydev->mdio.dev); - if (dev) - ndev_owner = dev->dev.parent->driver->owner; - if (ndev_owner != bus->owner) - module_put(bus->owner); -} -EXPORT_SYMBOL(phy_detach); - int phy_suspend(struct phy_device *phydev) { struct net_device *netdev = phydev->attached_dev; diff --git a/include/linux/phy.h b/include/linux/phy.h index 11092c3175b3..849dd43a11b4 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. Called in phy_attach_direct() right after phy_init_hw() + * and before phy_resume(). At this point, the PHY is still + * suspended, and @phydev->attached_dev may be NULL (e.g. for + * standalone PHYs). Runs in process context and may sleep. Return + * 0 on success or a negative errno on failure (which aborts the + * attach sequence). + */ + int (*notify_phy_attach)(struct phy_device *phydev); + /** + * @notify_phy_detach: Perform pre-detach handling for MDIO bus + * drivers. Called in phy_detach() after phy_suspend() and link + * cleanup, but before driver unbind and device reference release. + * Runs in process context and may sleep. Only invoked if + * @notify_phy_attach was previously called and succeeded. + */ + void (*notify_phy_detach)(struct phy_device *phydev); /** @stats: Statistic counters per device on the bus */ struct mdio_bus_stats stats[PHY_MAX_ADDR]; -- 2.55.0