USB: serial: cp210x: fixes and CP2105/CP2108 fw version

PATCH In review Johan Hovold 2 participants 9 messages Jul 5, 2021 – Jul 30, 2021

Subsystems (3)

Stable branch USB USB serial

lore

Share & tools

lore

Johan Hovold

author maintainer

2021-07-05 08:20 UTC #0 lore raw

v2 cover USB: serial: cp210x: fixes and CP2105/CP2108 fw version

Here are couple of minor fixes and some cleanups related to the recent regression which broke RTS control on some CP2102N devices with buggy firmware.

In case we run into another one of these, let's log the firmware version also for CP2105 and CP2108 for which it can be retrieved.

Johan

Changes in v2
- keep the special-chars error message to make it more obvious that continuing on errors is intentional (1/6) (Greg)

Johan Hovold (6):
USB: serial: cp210x: fix control-characters error handling
USB: serial: cp210x: fix flow-control error handling
USB: serial: cp210x: clean up control-request timeout
USB: serial: cp210x: clean up set-chars request
USB: serial: cp210x: clean up type detection
USB: serial: cp210x: determine fw version for CP2105 and CP2108

 drivers/usb/serial/cp210x.c | 77 +++++++++++++++----------------------
 1 file changed, 32 insertions(+), 45 deletions(-)

Signature · 1 line

2.31.1

Johan Hovold

author maintainer

2021-07-05 08:20 UTC #1 lore raw

PATCH v2 1/6 USB: serial: cp210x: fix control-characters error handling

In the unlikely event that setting the software flow-control characters fails the other flow-control settings should still be updated (just like all other terminal settings).

Move out the error message printed by the set_chars() helper to make it more obvious that this is intentional.

Fixes 7748feffcd80 ("USB: serial: cp210x: add support for software flow control")

Cc [email protected] # 5.11

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/cp210x.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/usb/serial/cp210x.c +5 -5

index 09b845d0da41..fd198031de71 100644
@@ -1163,10 +1163,8 @@ static int cp210x_set_chars(struct usb_serial_port *port,
 
 	kfree(dmabuf);
 
-	if (result < 0) {
-		dev_err(&port->dev, "failed to set special chars: %d\n", result);
+	if (result < 0)
 		return result;
-	}
 
 	return 0;
 }

@@ -1218,8 +1216,10 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 		chars.bXoffChar = STOP_CHAR(tty);
 
 		ret = cp210x_set_chars(port, &chars);
-		if (ret)
-			return;
+		if (ret) {
+			dev_err(&port->dev, "failed to set special chars: %d\n",
+					ret);
+		}
 	}
 
 	mutex_lock(&port_priv->mutex);

Signature · 1 line

2.31.1

Johan Hovold

author maintainer

2021-07-05 08:20 UTC #2 lore raw

PATCH v2 2/6 USB: serial: cp210x: fix flow-control error handling

In reply to Johan Hovold

Make sure that the driver crtscts state is not updated in the unlikely event that the flow-control request fails. Not doing so could break RTS control.

Fixes 5951b8508855 ("USB: serial: cp210x: suppress modem-control errors")

Cc [email protected] # 5.11

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/cp210x.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

drivers/usb/serial/cp210x.c +8 -3

index fd198031de71..fd6bd574e2a5 100644
@@ -1189,6 +1189,7 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 	struct cp210x_flow_ctl flow_ctl;
 	u32 flow_repl;
 	u32 ctl_hs;
+	bool crtscts;
 	int ret;
 
 	/*

@@ -1248,14 +1249,14 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 			flow_repl |= CP210X_SERIAL_RTS_FLOW_CTL;
 		else
 			flow_repl |= CP210X_SERIAL_RTS_INACTIVE;
-		port_priv->crtscts = true;
+		crtscts = true;
 	} else {
 		ctl_hs &= ~CP210X_SERIAL_CTS_HANDSHAKE;
 		if (port_priv->rts)
 			flow_repl |= CP210X_SERIAL_RTS_ACTIVE;
 		else
 			flow_repl |= CP210X_SERIAL_RTS_INACTIVE;
-		port_priv->crtscts = false;
+		crtscts = false;
 	}
 
 	if (I_IXOFF(tty)) {

@@ -1278,8 +1279,12 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 	flow_ctl.ulControlHandshake = cpu_to_le32(ctl_hs);
 	flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl);
 
-	cp210x_write_reg_block(port, CP210X_SET_FLOW, &flow_ctl,
+	ret = cp210x_write_reg_block(port, CP210X_SET_FLOW, &flow_ctl,
 			sizeof(flow_ctl));
+	if (ret)
+		goto out_unlock;
+
+	port_priv->crtscts = crtscts;
 out_unlock:
 	mutex_unlock(&port_priv->mutex);
 }

Signature · 1 line

2.31.1

Johan Hovold

author maintainer

2021-07-05 08:20 UTC #3 lore raw

PATCH v2 3/6 USB: serial: cp210x: clean up control-request timeout

In reply to Johan Hovold

For consistency use the USB_CTRL_GET_TIMEOUT define for the read-register request timeout (same value as USB_CTRL_SET_TIMEOUT).

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/cp210x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/serial/cp210x.c +1 -1

index fd6bd574e2a5..e560608afb1e 100644
@@ -637,7 +637,7 @@ static int cp210x_read_reg_block(struct usb_serial_port *port, u8 req,
 	result = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
 			req, REQTYPE_INTERFACE_TO_HOST, 0,
 			port_priv->bInterfaceNumber, dmabuf, bufsize,
-			USB_CTRL_SET_TIMEOUT);
+			USB_CTRL_GET_TIMEOUT);
 	if (result == bufsize) {
 		memcpy(buf, dmabuf, bufsize);
 		result = 0;

Signature · 1 line

2.31.1

Johan Hovold

author maintainer

2021-07-05 08:20 UTC #4 lore raw

PATCH v2 4/6 USB: serial: cp210x: clean up set-chars request

In reply to Johan Hovold

Use the generic control request helper to implement the SET_CHARS request.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/cp210x.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

drivers/usb/serial/cp210x.c +2 ~~-26~~

index e560608afb1e..6e027e2a02ad 100644
@@ -1144,31 +1144,6 @@ static void cp210x_disable_event_mode(struct usb_serial_port *port)
 	port_priv->event_mode = false;
 }
 
-static int cp210x_set_chars(struct usb_serial_port *port,
-		struct cp210x_special_chars *chars)
-{
-	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
-	struct usb_serial *serial = port->serial;
-	void *dmabuf;
-	int result;
-
-	dmabuf = kmemdup(chars, sizeof(*chars), GFP_KERNEL);
-	if (!dmabuf)
-		return -ENOMEM;
-
-	result = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
-				CP210X_SET_CHARS, REQTYPE_HOST_TO_INTERFACE, 0,
-				port_priv->bInterfaceNumber,
-				dmabuf, sizeof(*chars), USB_CTRL_SET_TIMEOUT);
-
-	kfree(dmabuf);
-
-	if (result < 0)
-		return result;
-
-	return 0;
-}
-
 static bool cp210x_termios_change(const struct ktermios *a, const struct ktermios *b)
 {
 	bool iflag_change, cc_change;

@@ -1216,7 +1191,8 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 		chars.bXonChar = START_CHAR(tty);
 		chars.bXoffChar = STOP_CHAR(tty);
 
-		ret = cp210x_set_chars(port, &chars);
+		ret = cp210x_write_reg_block(port, CP210X_SET_CHARS, &chars,
+				sizeof(chars));
 		if (ret) {
 			dev_err(&port->dev, "failed to set special chars: %d\n",
 					ret);

Signature · 1 line

2.31.1

Johan Hovold

author maintainer

2021-07-05 08:20 UTC #5 lore raw

PATCH v2 5/6 USB: serial: cp210x: clean up type detection

In reply to Johan Hovold

Clean up attach somewhat by moving type detection into the quirk helper and giving it a more generic name.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/cp210x.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

drivers/usb/serial/cp210x.c +12 ~~-11~~

index 6e027e2a02ad..6aaf54f16f8c 100644
@@ -2091,11 +2091,21 @@ static int cp210x_get_fw_version(struct usb_serial *serial, u16 value)
 	return 0;
 }
 
-static void cp210x_determine_quirks(struct usb_serial *serial)
+static void cp210x_determine_type(struct usb_serial *serial)
 {
 	struct cp210x_serial_private *priv = usb_get_serial_data(serial);
 	int ret;
 
+	ret = cp210x_read_vendor_block(serial, REQTYPE_DEVICE_TO_HOST,
+			CP210X_GET_PARTNUM, &priv->partnum,
+			sizeof(priv->partnum));
+	if (ret < 0) {
+		dev_warn(&serial->interface->dev,
+				"querying part number failed\n");
+		priv->partnum = CP210X_PARTNUM_UNKNOWN;
+		return;
+	}
+
 	switch (priv->partnum) {
 	case CP210X_PARTNUM_CP2102N_QFN28:
 	case CP210X_PARTNUM_CP2102N_QFN24:

@@ -2120,18 +2130,9 @@ static int cp210x_attach(struct usb_serial *serial)
 	if (!priv)
 		return -ENOMEM;
 
-	result = cp210x_read_vendor_block(serial, REQTYPE_DEVICE_TO_HOST,
-					  CP210X_GET_PARTNUM, &priv->partnum,
-					  sizeof(priv->partnum));
-	if (result < 0) {
-		dev_warn(&serial->interface->dev,
-			 "querying part number failed\n");
-		priv->partnum = CP210X_PARTNUM_UNKNOWN;
-	}
-
 	usb_set_serial_data(serial, priv);
 
-	cp210x_determine_quirks(serial);
+	cp210x_determine_type(serial);
 	cp210x_init_max_speed(serial);
 
 	result = cp210x_gpio_init(serial);

Signature · 1 line

2.31.1

Johan Hovold

author maintainer

2021-07-05 08:20 UTC #6 lore raw

PATCH v2 6/6 USB: serial: cp210x: determine fw version for CP2105 and CP2108

In reply to Johan Hovold

CP2105, CP2108 and CP2102N have vendor requests that can be used to retrieve the firmware version. Having this information available is essential when trying to work around buggy firmware as a recent CP2102N regression showed.

Determine and log the firmware version also for CP2105 and CP2108 during type detection at probe.

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/usb/serial/cp210x.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/usb/serial/cp210x.c +5 -0

index 6aaf54f16f8c..51670c80bf67 100644
@@ -399,6 +399,7 @@ struct cp210x_special_chars {
 };
 
 /* CP210X_VENDOR_SPECIFIC values */
+#define CP210X_GET_FW_VER	0x000E
 #define CP210X_READ_2NCONFIG	0x000E
 #define CP210X_GET_FW_VER_2N	0x0010
 #define CP210X_READ_LATCH	0x00C2

@@ -2107,6 +2108,10 @@ static void cp210x_determine_type(struct usb_serial *serial)
 	}
 
 	switch (priv->partnum) {
+	case CP210X_PARTNUM_CP2105:
+	case CP210X_PARTNUM_CP2108:
+		cp210x_get_fw_version(serial, CP210X_GET_FW_VER);
+		break;
 	case CP210X_PARTNUM_CP2102N_QFN28:
 	case CP210X_PARTNUM_CP2102N_QFN24:
 	case CP210X_PARTNUM_CP2102N_QFN20:

Signature · 1 line

2.31.1

Greg Kroah-Hartman

maintainer Reviewed-by on the v2 series

2021-07-05 08:32 UTC #7 lore raw

In reply to Johan Hovold

Quoted 7 lines · Johan Hovold

Here are couple of minor fixes and some cleanups related to the recent regression which broke RTS control on some CP2102N devices with buggy firmware.
In case we run into another one of these, let's log the firmware version also for CP2105 and CP2108 for which it can be retrieved.

Quoted 6 lines

Johan
Changes in v2
- keep the special-chars error message to make it more obvious that continuing on errors is intentional (1/6) (Greg)

Thanks for the change, looks good to me!

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

Johan Hovold

author maintainer

2021-07-30 15:22 UTC #8 lore raw

Quoted 11 lines · Greg Kroah-Hartman

Quoted 10 lines · Johan Hovold
Here are couple of minor fixes and some cleanups related to the recent regression which broke RTS control on some CP2102N devices with buggy firmware.
In case we run into another one of these, let's log the firmware version also for CP2105 and CP2108 for which it can be retrieved.
Johan

Quoted 7 lines

Quoted 3 lines
Changes in v2
- keep the special-chars error message to make it more obvious that continuing on errors is intentional (1/6) (Greg)
Thanks for the change, looks good to me!
Reviewed-by Greg Kroah-Hartman <[email protected]>

Thanks also for reviewing these. Now applied for -next.

Johan