| Message ID | 20220305184503.2954013-1-festevam@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | smsc95xx: Ignore -ENODEV errors when device is unplugged | expand |
On Sat, Mar 05, 2022 at 03:45:03PM -0300, Fabio Estevam wrote: > From: Fabio Estevam <festevam@denx.de> > > According to Documentation/driver-api/usb/URB.rst when a device > is unplugged usb_submit_urb() returns -ENODEV. > > This error code propagates all the way up to usbnet_read_cmd() and > usbnet_write_cmd() calls inside the smsc95xx.c driver during > Ethernet cable unplug, unbind or reboot. > > This causes the following errors to be shown on reboot, for example: > > ci_hdrc ci_hdrc.1: remove, state 1 > usb usb2: USB disconnect, device number 1 > usb 2-1: USB disconnect, device number 2 > usb 2-1.1: USB disconnect, device number 3 > smsc95xx 2-1.1:1.0 eth1: unregister 'smsc95xx' usb-ci_hdrc.1-1.1, smsc95xx USB 2.0 Ethernet > smsc95xx 2-1.1:1.0 eth1: Failed to read reg index 0x00000114: -19 > smsc95xx 2-1.1:1.0 eth1: Error reading MII_ACCESS > smsc95xx 2-1.1:1.0 eth1: __smsc95xx_mdio_read: MII is busy > smsc95xx 2-1.1:1.0 eth1: Failed to read reg index 0x00000114: -19 > smsc95xx 2-1.1:1.0 eth1: Error reading MII_ACCESS > smsc95xx 2-1.1:1.0 eth1: __smsc95xx_mdio_read: MII is busy > smsc95xx 2-1.1:1.0 eth1: hardware isn't capable of remote wakeup > usb 2-1.4: USB disconnect, device number 4 > ci_hdrc ci_hdrc.1: USB bus 2 deregistered > ci_hdrc ci_hdrc.0: remove, state 4 > usb usb1: USB disconnect, device number 1 > ci_hdrc ci_hdrc.0: USB bus 1 deregistered > imx2-wdt 30280000.watchdog: Device shutdown: Expect reboot! > reboot: Restarting system > > Ignore the -ENODEV errors inside __smsc95xx_mdio_read() and > __smsc95xx_phy_wait_not_busy() and do not print error messages > when -ENODEV is returned. > > Signed-off-by: Fabio Estevam <festevam@denx.de> > --- > Hi, > > Tested on 5.10.102 and 5.17-rc6. Please indicate what tree this patch is actually for. It should be against net, since you want it backporting. Please see the netdev FAQ. Please also include a Fixes: tag. > @@ -84,7 +84,7 @@ static int __must_check __smsc95xx_read_reg(struct usbnet *dev, u32 index, > ret = fn(dev, USB_VENDOR_REQUEST_READ_REGISTER, USB_DIR_IN > | USB_TYPE_VENDOR | USB_RECIP_DEVICE, > 0, index, &buf, 4); > - if (unlikely(ret < 0)) { > + if (ret < 0 && ret != -ENODEV) { > netdev_warn(dev->net, "Failed to read reg index 0x%08x: %d\n", > index, ret); > return ret; I suspect this will result in kasan warnings. The contents of buf is probably undefined because of the error, yet you continue to set *data to it. You probably need to explicitly handle the ENODEV case setting *data to some value. Andrew
Hi Andrew, On Sat, Mar 5, 2022 at 4:24 PM Andrew Lunn <andrew@lunn.ch> wrote: > Please indicate what tree this patch is actually for. It should be > against net, since you want it backporting. Please see the netdev FAQ. > Please also include a Fixes: tag. Thanks for your review. Fixed in v2. > I suspect this will result in kasan warnings. The contents of buf is > probably undefined because of the error, yet you continue to set *data > to it. You probably need to explicitly handle the ENODEV case setting > *data to some value. Good catch. Fixed in v2. Thanks
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index b17bff6a1015..61a5748da133 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -84,7 +84,7 @@ static int __must_check __smsc95xx_read_reg(struct usbnet *dev, u32 index, ret = fn(dev, USB_VENDOR_REQUEST_READ_REGISTER, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, 0, index, &buf, 4); - if (unlikely(ret < 0)) { + if (ret < 0 && ret != -ENODEV) { netdev_warn(dev->net, "Failed to read reg index 0x%08x: %d\n", index, ret); return ret; @@ -116,7 +116,7 @@ static int __must_check __smsc95xx_write_reg(struct usbnet *dev, u32 index, ret = fn(dev, USB_VENDOR_REQUEST_WRITE_REGISTER, USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, 0, index, &buf, 4); - if (unlikely(ret < 0)) + if (ret < 0 && ret != -ENODEV) netdev_warn(dev->net, "Failed to write reg index 0x%08x: %d\n", index, ret); @@ -159,6 +159,9 @@ static int __must_check __smsc95xx_phy_wait_not_busy(struct usbnet *dev, do { ret = __smsc95xx_read_reg(dev, MII_ADDR, &val, in_pm); if (ret < 0) { + /* Ignore -ENODEV error during disconnect() */ + if (ret == -ENODEV) + return 0; netdev_warn(dev->net, "Error reading MII_ACCESS\n"); return ret; } @@ -194,7 +197,8 @@ static int __smsc95xx_mdio_read(struct usbnet *dev, int phy_id, int idx, addr = mii_address_cmd(phy_id, idx, MII_READ_ | MII_BUSY_); ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm); if (ret < 0) { - netdev_warn(dev->net, "Error writing MII_ADDR\n"); + if (ret != -ENODEV) + netdev_warn(dev->net, "Error writing MII_ADDR\n"); goto done; } @@ -206,7 +210,8 @@ static int __smsc95xx_mdio_read(struct usbnet *dev, int phy_id, int idx, ret = __smsc95xx_read_reg(dev, MII_DATA, &val, in_pm); if (ret < 0) { - netdev_warn(dev->net, "Error reading MII_DATA\n"); + if (ret != -ENODEV) + netdev_warn(dev->net, "Error reading MII_DATA\n"); goto done; } @@ -214,6 +219,10 @@ static int __smsc95xx_mdio_read(struct usbnet *dev, int phy_id, int idx, done: mutex_unlock(&dev->phy_mutex); + + /* Ignore -ENODEV error during disconnect() */ + if (ret == -ENODEV) + return 0; return ret; } @@ -235,7 +244,8 @@ static void __smsc95xx_mdio_write(struct usbnet *dev, int phy_id, val = regval; ret = __smsc95xx_write_reg(dev, MII_DATA, val, in_pm); if (ret < 0) { - netdev_warn(dev->net, "Error writing MII_DATA\n"); + if (ret != -ENODEV) + netdev_warn(dev->net, "Error writing MII_DATA\n"); goto done; } @@ -243,7 +253,8 @@ static void __smsc95xx_mdio_write(struct usbnet *dev, int phy_id, addr = mii_address_cmd(phy_id, idx, MII_WRITE_ | MII_BUSY_); ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm); if (ret < 0) { - netdev_warn(dev->net, "Error writing MII_ADDR\n"); + if (ret != -ENODEV) + netdev_warn(dev->net, "Error writing MII_ADDR\n"); goto done; }