diff mbox series

[v2,net] smsc95xx: Ignore -ENODEV errors when device is unplugged

Message ID 20220305204720.2978554-1-festevam@gmail.com
State New
Headers show
Series [v2,net] smsc95xx: Ignore -ENODEV errors when device is unplugged | expand

Commit Message

Fabio Estevam March 5, 2022, 8:47 p.m. UTC 
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.

Fixes: a049a30fc27c ("net: usb: Correct PHY handling of smsc95xx")
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v1:
- Added 'net' annotation - Andrew
- Added Fixes tag - Andrew
- Avoided undefined 'buf' behaviour in __smsc95xx_read_reg() - Andrew

 drivers/net/usb/smsc95xx.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

Comments

Alexander Stein March 9, 2022, 2:02 p.m. UTC | #1 
Am Samstag, 5. März 2022, 21:47:20 CET schrieb Fabio Estevam:
> 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.
> 
> Fixes: a049a30fc27c ("net: usb: Correct PHY handling of smsc95xx")
> Signed-off-by: Fabio Estevam <festevam@denx.de>

Oh BTW, is this queued for stable? Which versions? If 'Fixes: a049a30fc27c 
("net: usb: Correct PHY handling of smsc95xx")' is the indicator, it's not 
enough. This errors also shows up on v5.15.27 and is fixed with this patch.

Regards,
Alexander

> ---
> Changes since v1:
> - Added 'net' annotation - Andrew
> - Added Fixes tag - Andrew
> - Avoided undefined 'buf' behaviour in __smsc95xx_read_reg() - Andrew
> 
>  drivers/net/usb/smsc95xx.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index b17bff6a1015..e5b744851146 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -84,9 +84,10 @@ 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)) {
> -		netdev_warn(dev->net, "Failed to read reg index 0x%08x: 
%d\n",
> -			    index, ret);
> +	if (ret < 0) {
> +		if (ret != -ENODEV)
> +			netdev_warn(dev->net, "Failed to read reg 
index 0x%08x: %d\n",
> +				    index, ret);
>  		return ret;
>  	}
> 
> @@ -116,7 +117,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 +160,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 +198,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 +211,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 +220,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 +245,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 +254,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;
>  	}
Fabio Estevam March 9, 2022, 4:50 p.m. UTC | #2 
Hi Alexander,

On Wed, Mar 9, 2022 at 11:02 AM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:

> Oh BTW, is this queued for stable? Which versions? If 'Fixes: a049a30fc27c
> ("net: usb: Correct PHY handling of smsc95xx")' is the indicator, it's not
> enough. This errors also shows up on v5.15.27 and is fixed with this patch.

("net: usb: Correct PHY handling of smsc95xx")' is also present in 5.15:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/log/drivers/net/usb/smsc95xx.c?h=v5.15.27

so the fix should land in 5.15 too.

For 5.10, when the fix reaches Linus' tree, then I will ping
Dave/Jakub/Greg to apply the 3 latest smsc95xx commits
to 5.10 stable tree:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/log/drivers/net/usb/smsc95xx.c?id=c70c453abcbf

I have been running 5.10 + the 3 commits above and no more smsc95xx
errors are seen.
diff mbox series

Patch

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index b17bff6a1015..e5b744851146 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -84,9 +84,10 @@  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)) {
-		netdev_warn(dev->net, "Failed to read reg index 0x%08x: %d\n",
-			    index, ret);
+	if (ret < 0) {
+		if (ret != -ENODEV)
+			netdev_warn(dev->net, "Failed to read reg index 0x%08x: %d\n",
+				    index, ret);
 		return ret;
 	}
 
@@ -116,7 +117,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 +160,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 +198,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 +211,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 +220,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 +245,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 +254,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;
 	}