>From f1c1892e3ca21be9e241f6b8a3710a14f7ec304f Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 2 Apr 2025 16:20:33 +0300 Subject: [PATCH] net: phy: extend MII timestamper link state update to phylink Context: since 2017, struct phy_device has a "phy_link_change" hook, added by commit a81497bee70e ("net: phy: provide a hook for link up/link down events"), with two implementations in the kernel: - phylib's eponymous phy_link_change() - phylink's phylink_phy_change() Russell King points out here: https://lore.kernel.org/netdev/Z8Xvmqp2sukNPzvt@shell.armlinux.org.uk/ that commit 4715f65ffa05 ("net: Introduce a new MII time stamping interface.") from 2019 made the interesting design choice of placing the further phydev->mii_ts->link_state() hook in the phylib implementation, but not in the phylink one, due to an unknown reason. As such, converting MAC drivers from phylib to phylink poses a regression challenge if they use MII timestampers, because with phylink, these will no longer be notified of link state changes (which is something they may or may not care about). The only upstream user of mii_ts->link_state is ptp_ines.c. I also don't know in which systems it is integrated, and whether the attached MACs use phylib or phylink. In lack of link state updates coming from phylink, the ptp_ines.c driver retains the initial PORT_CONF setting, which assumes PHY_SPEED_1000 << PHY_SPEED_SHIFT. I'm unable to further assess the impact of this mismatch between the real MII speed and the initial assumption. Lacking a proper bug report, I am going to assume there is no breakage, but going forward, we should equally support phylib and phylink. That can be done by placing the mii_ts->link_state() hook at the phy_device->phy_link_state() call sites (i.e. phy_link_down() and phy_link_up()), rather than at the individual implementation sites. Signed-off-by: Vladimir Oltean --- drivers/net/phy/phy.c | 2 ++ drivers/net/phy/phy_device.c | 2 -- include/linux/phy.h | 10 ++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 13df28445f02..77b1d2d002ab 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -77,12 +77,14 @@ static void phy_link_up(struct phy_device *phydev) { phydev->phy_link_change(phydev, true); phy_led_trigger_change_speed(phydev); + phy_ts_link_change(phydev); } static void phy_link_down(struct phy_device *phydev) { phydev->phy_link_change(phydev, false); phy_led_trigger_change_speed(phydev); + phy_ts_link_change(phydev); WRITE_ONCE(phydev->link_down_events, phydev->link_down_events + 1); } diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 675fbd225378..f535a2862fc6 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1064,8 +1064,6 @@ static void phy_link_change(struct phy_device *phydev, bool up) else netif_carrier_off(netdev); phydev->adjust_link(netdev); - if (phydev->mii_ts && phydev->mii_ts->link_state) - phydev->mii_ts->link_state(phydev->mii_ts, phydev); } /** diff --git a/include/linux/phy.h b/include/linux/phy.h index a2bfae80c449..c6cc4403323c 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1609,6 +1609,16 @@ static inline bool phy_polling_mode(struct phy_device *phydev) return phydev->irq == PHY_POLL; } +/** + * phy_ts_link_change: Notify MII timestamper of changes to PHY link state + * @phydev: the phy_device struct + */ +static inline void phy_ts_link_change(struct phy_device *phydev) +{ + if (phydev->mii_ts && phydev->mii_ts->link_state) + phydev->mii_ts->link_state(phydev->mii_ts, phydev); +} + /** * phy_has_hwtstamp - Tests whether a PHY time stamp configuration. * @phydev: the phy_device struct -- 2.34.1