From 7fefa7aa8b9e0d2378738fed76711c978e97d75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= Date: Sun, 14 Nov 2021 23:18:45 +0100 Subject: [PATCH net] net: phylink: Force link down and retrigger resolve on interface change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On PHY state change the phylink_resolve() function can currently pass incorrect speed/duplex to MAC/PCS reconfiguration functions and dmesg output, because only PHY interface mode and pause are taken from current PHY state, while speed/duplex can be taken from the up state from before interface change. Example with a Marvell 88X3310 PHY connected to a SerDes port on Marvell 88E6393X switch: - PHY driver triggers state change due to PHY interface mode being changed from 10gbase-r to 2500base-x due to copper change in speed from 10Gbps to 2.5Gbps, but the PHY itself hasn't yet changed its interface to the host, so it is still at 10gbase-r - phylink_resolve() - phylink_mac_pcs_get_state() - this fills in speed=10g link=up because PHY hasn't changed SerDes yet - interface mode is updated to 2500base-x but speed is left at 10Gbps - phylink_major_config() - interface is changed to 2500base-x - phylink_link_up() - mv88e6xxx_mac_link_up() - .port_set_speed_duplex() - speed is set to 10Gbps - reports "Link is Up - 10Gbps/Full" to dmesg Afterwards even when the PHY finally changes the interface mode on its side to 2500base-x and we get the interrupt from mv88e6xxx, which forces another resolve in which we get the correct speed from phylink_mac_pcs_get_state(), since the interface is not being changed anymore, we don't call phylink_major_config() but only phylink_mac_config(), which does not set speed/duplex anymore. To fix this issue we need to force the link down and trigger another resolve on PHY interface change event. Fixes: 9525ae83959b ("phylink: add phylink infrastructure") Signed-off-by: Russell King (Oracle) Signed-off-by: Marek Behún --- drivers/net/phy/phylink.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 3ad7397b8119..708d2a984621 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -710,6 +710,7 @@ static void phylink_resolve(struct work_struct *w) struct phylink_link_state link_state; struct net_device *ndev = pl->netdev; bool mac_config = false; + bool retrigger = false; bool cur_link_state; mutex_lock(&pl->state_mutex); @@ -723,6 +724,7 @@ static void phylink_resolve(struct work_struct *w) link_state.link = false; } else if (pl->mac_link_dropped) { link_state.link = false; + retrigger = true; } else { switch (pl->cur_link_an_mode) { case MLO_AN_PHY: @@ -747,6 +749,15 @@ static void phylink_resolve(struct work_struct *w) /* Only update if the PHY link is up */ if (pl->phydev && pl->phy_state.link) { + /* If the interface has changed, force a + * link down event if the link isn't already + * down, and re-resolve. + */ + if (link_state.interface != + pl->phy_state.interface) { + retrigger = true; + link_state.link = false; + } link_state.interface = pl->phy_state.interface; /* If we have a PHY, we need to update with @@ -789,7 +800,7 @@ static void phylink_resolve(struct work_struct *w) else phylink_link_up(pl, link_state); } - if (!link_state.link && pl->mac_link_dropped) { + if (!link_state.link && retrigger) { pl->mac_link_dropped = false; queue_work(system_power_efficient_wq, &pl->resolve); } -- 2.32.0