[PATCH v2 0/6] USB: serial: cp210x: fixes and CP2105/CP2108 fw version

[PATCH v2 0/6] USB: serial: cp210x: fixes and CP2105/CP2108 fw version

[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


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(-)

-- 
2.31.1

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

[Johan Hovold]

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: stable@vger.kernel.org	# 5.11
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 09b845d0da41..fd198031de71 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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);
-- 
2.31.1

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

[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: stable@vger.kernel.org	# 5.11
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index fd198031de71..fd6bd574e2a5 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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);
 }
-- 
2.31.1

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

[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 <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index fd6bd574e2a5..e560608afb1e 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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;
-- 
2.31.1

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

[Johan Hovold]

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

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index e560608afb1e..6e027e2a02ad 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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);
-- 
2.31.1

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

[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 <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 6e027e2a02ad..6aaf54f16f8c 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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);
-- 
2.31.1

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

[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 <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 6aaf54f16f8c..51670c80bf67 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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:
-- 
2.31.1

Re: [PATCH v2 0/6] USB: serial: cp210x: fixes and CP2105/CP2108 fw version

[Greg Kroah-Hartman]

On Mon, Jul 05, 2021 at 10:20:09AM +0200, Johan Hovold wrote:
> 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)

Thanks for the change, looks good to me!

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v2 0/6] USB: serial: cp210x: fixes and CP2105/CP2108 fw version

[Johan Hovold]

On Mon, Jul 05, 2021 at 10:32:14AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Jul 05, 2021 at 10:20:09AM +0200, Johan Hovold wrote:
> > 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)
> 
> Thanks for the change, looks good to me!
> 
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

Johan