Patch "usbnet: smsc95xx: Avoid link settings race on interrupt reception" has been added to the 5.18-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    usbnet: smsc95xx: Avoid link settings race on interrupt reception

to the 5.18-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     usbnet-smsc95xx-avoid-link-settings-race-on-interrup.patch
and it can be found in the queue-5.18 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit a7b73d5774f2b3d6656d48dbee4b2f995629a2c2
Author: Lukas Wunner <lukas@xxxxxxxxx>
Date:   Thu May 12 10:42:04 2022 +0200

    usbnet: smsc95xx: Avoid link settings race on interrupt reception
    
    [ Upstream commit 8960f878e39fadc03d74292a6731f1e914cf2019 ]
    
    When a PHY interrupt is signaled, the SMSC LAN95xx driver updates the
    MAC full duplex mode and PHY flow control registers based on cached data
    in struct phy_device:
    
      smsc95xx_status()                 # raises EVENT_LINK_RESET
        usbnet_deferred_kevent()
          smsc95xx_link_reset()         # uses cached data in phydev
    
    Simultaneously, phylib polls link status once per second and updates
    that cached data:
    
      phy_state_machine()
        phy_check_link_status()
          phy_read_status()
            lan87xx_read_status()
              genphy_read_status()      # updates cached data in phydev
    
    If smsc95xx_link_reset() wins the race against genphy_read_status(),
    the registers may be updated based on stale data.
    
    E.g. if the link was previously down, phydev->duplex is set to
    DUPLEX_UNKNOWN and that's what smsc95xx_link_reset() will use, even
    though genphy_read_status() may update it to DUPLEX_FULL afterwards.
    
    PHY interrupts are currently only enabled on suspend to trigger wakeup,
    so the impact of the race is limited, but we're about to enable them
    perpetually.
    
    Avoid the race by delaying execution of smsc95xx_link_reset() until
    phy_state_machine() has done its job and calls back via
    smsc95xx_handle_link_change().
    
    Signaling EVENT_LINK_RESET on wakeup is not necessary because phylib
    picks up link status changes through polling.  So drop the declaration
    of a ->link_reset() callback.
    
    Note that the semicolon on a line by itself added in smsc95xx_status()
    is a placeholder for a function call which will be added in a subsequent
    commit.  That function call will actually handle the INT_ENP_PHY_INT_
    interrupt.
    
    Tested-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> # LAN9514/9512/9500
    Tested-by: Ferry Toth <fntoth@xxxxxxxxx> # LAN9514
    Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
    Reviewed-by: Andrew Lunn <andrew@xxxxxxx>
    Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 2cb44d65bbc3..f5a208948d22 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -566,7 +566,7 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev)
 	return smsc95xx_write_reg(dev, AFC_CFG, afc_cfg);
 }
 
-static int smsc95xx_link_reset(struct usbnet *dev)
+static void smsc95xx_mac_update_fullduplex(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = dev->driver_priv;
 	unsigned long flags;
@@ -583,14 +583,16 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
 
 	ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		if (ret != -ENODEV)
+			netdev_warn(dev->net,
+				    "Error updating MAC full duplex mode\n");
+		return;
+	}
 
 	ret = smsc95xx_phy_update_flowcontrol(dev);
 	if (ret < 0)
 		netdev_warn(dev->net, "Error updating PHY flow control\n");
-
-	return ret;
 }
 
 static void smsc95xx_status(struct usbnet *dev, struct urb *urb)
@@ -607,7 +609,7 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb)
 	netif_dbg(dev, link, dev->net, "intdata: 0x%08X\n", intdata);
 
 	if (intdata & INT_ENP_PHY_INT_)
-		usbnet_defer_kevent(dev, EVENT_LINK_RESET);
+		;
 	else
 		netdev_warn(dev->net, "unexpected interrupt, intdata=0x%08X\n",
 			    intdata);
@@ -1088,6 +1090,7 @@ static void smsc95xx_handle_link_change(struct net_device *net)
 	struct usbnet *dev = netdev_priv(net);
 
 	phy_print_status(net->phydev);
+	smsc95xx_mac_update_fullduplex(dev);
 	usbnet_defer_kevent(dev, EVENT_LINK_CHANGE);
 }
 
@@ -1993,7 +1996,6 @@ static const struct driver_info smsc95xx_info = {
 	.description	= "smsc95xx USB 2.0 Ethernet",
 	.bind		= smsc95xx_bind,
 	.unbind		= smsc95xx_unbind,
-	.link_reset	= smsc95xx_link_reset,
 	.reset		= smsc95xx_reset,
 	.check_connect	= smsc95xx_start_phy,
 	.stop		= smsc95xx_stop,



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux