Skip to contentSkip to conversation

USB: serial: drop short control-transfer checks

PATCH In review Johan Hovold 2 participants 10 messages Jan 18, 2021

Subsystems (2)
USB USB serial
lore
Share & tools
lore

Johan Hovold

author maintainer
#0 lore raw

cover USB: serial: drop short control-transfer checks

There's no need to check for short control transfers when sending data so remove the redundant sanity checks from the drivers that had them.

Included is also a related fix of a copy-paste error in a debug message.

Johan

Johan Hovold (6):
USB: serial: mxuport: drop short control-transfer check
USB: serial: upd78f0730: drop short control-transfer check
USB: serial: io_ti: drop short control-transfer check
USB: serial: io_ti: fix a debug-message copy-paste error
USB: serial: f81232: drop short control-transfer checks
USB: serial: f81534: drop short control-transfer check

 drivers/usb/serial/f81232.c     | 12 ++----------
 drivers/usb/serial/f81534.c     |  4 +---
 drivers/usb/serial/io_ti.c      |  8 ++------
 drivers/usb/serial/mxuport.c    |  7 -------
 drivers/usb/serial/upd78f0730.c |  5 +----
 5 files changed, 6 insertions(+), 30 deletions(-)
Signature · 1 line
2.26.2
author maintainer
#1 lore raw

PATCH 1/6 USB: serial: mxuport: drop short control-transfer check

There's no need to check for short control transfers when sending data so remove the redundant sanity check.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/mxuport.c | 7 -------
 1 file changed, 7 deletions(-)
drivers/usb/serial/mxuport.c +0 -7
index 5d38c2a0f590..eb45a9b0005c 100644
@@ -261,13 +261,6 @@ static int mxuport_send_ctrl_data_urb(struct usb_serial *serial,
 		return status;
 	}
 
-	if (status != size) {
-		dev_err(&serial->interface->dev,
-			"%s - short write (%d / %zd)\n",
-			__func__, status, size);
-		return -EIO;
-	}
-
 	return 0;
 }
Signature · 1 line
2.26.2
author maintainer
#2 lore raw

PATCH 2/6 USB: serial: upd78f0730: drop short control-transfer check

In reply to Johan Hovold

There's no need to check for short control transfers when sending data so remove the redundant sanity check.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/upd78f0730.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/usb/serial/upd78f0730.c +1 -4
index 0a2268c479af..1ca9c1881621 100644
@@ -145,14 +145,11 @@ static int upd78f0730_send_ctl(struct usb_serial_port *port,
 
 	kfree(buf);
 
-	if (res != size) {
+	if (res < 0) {
 		struct device *dev = &port->dev;
 
 		dev_err(dev, "failed to send control request %02x: %d\n",
 			*(u8 *)data, res);
-		/* The maximum expected length of a transfer is 6 bytes */
-		if (res >= 0)
-			res = -EIO;
 
 		return res;
 	}
Signature · 1 line
2.26.2
author maintainer
#3 lore raw

PATCH 3/6 USB: serial: io_ti: drop short control-transfer check

In reply to Johan Hovold

There's no need to check for short control transfers when sending data so remove the redundant sanity check.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/io_ti.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/usb/serial/io_ti.c +1 -5
index c327d4cf7928..0c4062698603 100644
@@ -283,11 +283,7 @@ static int ti_vsend_sync(struct usb_device *dev, u8 request, u16 value,
 			value, index, data, size, timeout);
 	if (status < 0)
 		return status;
-	if (status != size) {
-		dev_dbg(&dev->dev, "%s - wanted to write %d, but only wrote %d\n",
-			__func__, size, status);
-		return -ECOMM;
-	}
+
 	return 0;
 }
Signature · 1 line
2.26.2
author maintainer
#4 lore raw

PATCH 4/6 USB: serial: io_ti: fix a debug-message copy-paste error

In reply to Johan Hovold

Fix a copy-paste error in the ti_vread_sync() debug message.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/io_ti.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/serial/io_ti.c +1 -1
index 0c4062698603..0bbfa47e04b7 100644
@@ -266,7 +266,7 @@ static int ti_vread_sync(struct usb_device *dev, __u8 request,
 	if (status < 0)
 		return status;
 	if (status != size) {
-		dev_dbg(&dev->dev, "%s - wanted to write %d, but only wrote %d\n",
+		dev_dbg(&dev->dev, "%s - wanted to read %d, but only read %d\n",
 			__func__, size, status);
 		return -ECOMM;
 	}
Signature · 1 line
2.26.2
author maintainer
#5 lore raw

PATCH 5/6 USB: serial: f81232: drop short control-transfer checks

In reply to Johan Hovold

There's no need to check for short control transfers when sending data so remove the redundant sanity checks.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/f81232.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)
drivers/usb/serial/f81232.c +2 -10
index 0c7eacc630e0..6a8f39147d8e 100644
@@ -192,13 +192,9 @@ static int f81232_set_register(struct usb_serial_port *port, u16 reg, u8 val)
 				tmp,
 				sizeof(val),
 				USB_CTRL_SET_TIMEOUT);
-	if (status != sizeof(val)) {
+	if (status < 0) {
 		dev_err(&port->dev, "%s failed status: %d\n", __func__, status);
-
-		if (status < 0)
-			status = usb_translate_errors(status);
-		else
-			status = -EIO;
+		status = usb_translate_errors(status);
 	} else {
 		status = 0;
 	}

@@ -886,10 +882,6 @@ static int f81534a_ctrl_set_register(struct usb_interface *intf, u16 reg,
 			status = usb_translate_errors(status);
 			if (status == -EIO)
 				continue;
-		} else if (status != size) {
-			/* Retry on short transfers */
-			status = -EIO;
-			continue;
 		} else {
 			status = 0;
 		}
Signature · 1 line
2.26.2
author maintainer
#6 lore raw
In reply to Johan Hovold

Hi Himadri,

Quoted 16 lines · Johan Hovold

There's no need to check for short control transfers when sending data so remove the redundant sanity checks from the drivers that had them.

Included is also a related fix of a copy-paste error in a debug message.

Johan

Johan Hovold (6):
USB: serial: mxuport: drop short control-transfer check
USB: serial: upd78f0730: drop short control-transfer check
USB: serial: io_ti: drop short control-transfer check
USB: serial: io_ti: fix a debug-message copy-paste error
USB: serial: f81232: drop short control-transfer checks
USB: serial: f81534: drop short control-transfer check

Quoted 6 lines
 drivers/usb/serial/f81232.c     | 12 ++----------
 drivers/usb/serial/f81534.c     |  4 +---
 drivers/usb/serial/io_ti.c      |  8 ++------
 drivers/usb/serial/mxuport.c    |  7 -------
 drivers/usb/serial/upd78f0730.c |  5 +----
 5 files changed, 6 insertions(+), 30 deletions(-)

I intended to CC you on this series, but forgot to do so before hitting send.

The series can be found here:

	https://lore.kernel.org/r/[email protected]

I checked to make sure it doesn't interfere too much with your revised series.

Johan

author maintainer
#8 lore raw
Quoted 6 lines · Greg Kroah-Hartman
Quoted 2 lines · Johan Hovold

There's no need to check for short control transfers when sending data so remove the redundant sanity checks from the drivers that had them.

It seems impossible to do a "short write", so I wonder why we even check it...

Right, and this series removes precisely those redundant checks. Or what did you mean? :)

Quoted 3 lines

Anyway, nice cleanups:

Reviewed-by Greg Kroah-Hartman <[email protected]>

Thanks for reviewing, now applied.

Johan

author maintainer
#9 lore raw

PATCH 6/6 USB: serial: f81534: drop short control-transfer check

In reply to Johan Hovold

There's no need to check for short control transfers when sending data so remove the redundant sanity check.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/f81534.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/usb/serial/f81534.c +1 -3
index 5661fd03e545..dd7e55e822ef 100644
@@ -235,11 +235,9 @@ static int f81534_set_register(struct usb_serial *serial, u16 reg, u8 data)
 					 USB_TYPE_VENDOR | USB_DIR_OUT,
 					 reg, 0, tmp, sizeof(u8),
 					 F81534_USB_TIMEOUT);
-		if (status > 0) {
+		if (status == sizeof(u8)) {
 			status = 0;
 			break;
-		} else if (status == 0) {
-			status = -EIO;
 		}
 	}
Signature · 1 line
2.26.2