1 From 7fefa7aa8b9e0d2378738fed76711c978e97d75d Mon Sep 17 00:00:00 2001
    2 From: =?UTF-8?q?Marek=20Beh=C3=BAn?= <kabel@kernel.org>
    3 Date: Sun, 14 Nov 2021 23:18:45 +0100
    4 Subject: [PATCH net] net: phylink: Force link down and retrigger resolve on
    5  interface change
    6 MIME-Version: 1.0
    7 Content-Type: text/plain; charset=UTF-8
    8 Content-Transfer-Encoding: 8bit
    9 
   10 On PHY state change the phylink_resolve() function can currently pass
   11 incorrect speed/duplex to MAC/PCS reconfiguration functions and dmesg
   12 output, because only PHY interface mode and pause are taken from current
   13 PHY state, while speed/duplex can be taken from the up state from before
   14 interface change.
   15 
   16 Example with a Marvell 88X3310 PHY connected to a SerDes port on Marvell
   17 88E6393X switch:
   18 - PHY driver triggers state change due to PHY interface mode being
   19   changed from 10gbase-r to 2500base-x due to copper change in speed
   20   from 10Gbps to 2.5Gbps, but the PHY itself hasn't yet changed its
   21   interface to the host, so it is still at 10gbase-r
   22 - phylink_resolve()
   23   - phylink_mac_pcs_get_state()
   24     - this fills in speed=10g link=up because PHY hasn't changed SerDes
   25       yet
   26   - interface mode is updated to 2500base-x but speed is left at 10Gbps
   27   - phylink_major_config()
   28     - interface is changed to 2500base-x
   29   - phylink_link_up()
   30     - mv88e6xxx_mac_link_up()
   31       - .port_set_speed_duplex()
   32         - speed is set to 10Gbps
   33     - reports "Link is Up - 10Gbps/Full" to dmesg
   34 
   35 Afterwards even when the PHY finally changes the interface mode on its
   36 side to 2500base-x and we get the interrupt from mv88e6xxx, which forces
   37 another resolve in which we get the correct speed from
   38 phylink_mac_pcs_get_state(), since the interface is not being changed
   39 anymore, we don't call phylink_major_config() but only
   40 phylink_mac_config(), which does not set speed/duplex anymore.
   41 
   42 To fix this issue we need to force the link down and trigger another
   43 resolve on PHY interface change event.
   44 
   45 Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
   46 Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
   47 Signed-off-by: Marek Behún <kabel@kernel.org>
   48 ---
   49  drivers/net/phy/phylink.c | 13 ++++++++++++-
   50  1 file changed, 12 insertions(+), 1 deletion(-)
   51 
   52 diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
   53 index 3ad7397b8119..708d2a984621 100644
   54 --- a/drivers/net/phy/phylink.c
   55 +++ b/drivers/net/phy/phylink.c
   56 @@ -710,6 +710,7 @@ static void phylink_resolve(struct work_struct *w)
   57         struct phylink_link_state link_state;
   58         struct net_device *ndev = pl->netdev;
   59         bool mac_config = false;
   60 +       bool retrigger = false;
   61         bool cur_link_state;
   62  
   63         mutex_lock(&pl->state_mutex);
   64 @@ -723,6 +724,7 @@ static void phylink_resolve(struct work_struct *w)
   65                 link_state.link = false;
   66         } else if (pl->mac_link_dropped) {
   67                 link_state.link = false;
   68 +               retrigger = true;
   69         } else {
   70                 switch (pl->cur_link_an_mode) {
   71                 case MLO_AN_PHY:
   72 @@ -747,6 +749,15 @@ static void phylink_resolve(struct work_struct *w)
   73  
   74                         /* Only update if the PHY link is up */
   75                         if (pl->phydev && pl->phy_state.link) {
   76 +                               /* If the interface has changed, force a
   77 +                                * link down event if the link isn't already
   78 +                                * down, and re-resolve.
   79 +                                */
   80 +                               if (link_state.interface !=
   81 +                                   pl->phy_state.interface) {
   82 +                                       retrigger = true;
   83 +                                       link_state.link = false;
   84 +                               }
   85                                 link_state.interface = pl->phy_state.interface;
   86  
   87                                 /* If we have a PHY, we need to update with
   88 @@ -789,7 +800,7 @@ static void phylink_resolve(struct work_struct *w)
   89                 else
   90                         phylink_link_up(pl, link_state);
   91         }
   92 -       if (!link_state.link && pl->mac_link_dropped) {
   93 +       if (!link_state.link && retrigger) {
   94                 pl->mac_link_dropped = false;
   95                 queue_work(system_power_efficient_wq, &pl->resolve);
   96         }
   97 -- 
   98 2.32.0
   99