Skip to contentSkip to conversation

USB: serial: cp210x: add software flow-control support

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: cp210x: add software flow-control support

This series, based on a patch by Wang Sheng Long, adds support for software flow control to the cp210x driver. Included are also some related cleanups.

Johan

Johan Hovold (5):
USB: serial: cp210x: set IXOFF thresholds
USB: serial: cp210x: update control-characters on every change
USB: serial: cp210x: drop short control-transfer checks
USB: serial: cp210x: drop unused includes
USB: serial: cp210x: add copyright notice

Wang Sheng Long (1):
USB: serial: cp210x: add support for software flow control

 drivers/usb/serial/cp210x.c | 100 +++++++++++++++++++++++++++++-------
 1 file changed, 81 insertions(+), 19 deletions(-)
Signature · 1 line
2.26.2
author maintainer
#1 lore raw

PATCH 1/6 USB: serial: cp210x: add support for software flow control

From: Wang Sheng Long <[email protected]>

When data is transmitted between two serial ports, the phenomenon of data loss often occurs. The two kinds of flow control commonly used in serial communication are hardware flow control and software flow control.

In serial communication, If you only use RX/TX/GND Pins, you can't do hardware flow. So we often used software flow control and prevent data loss. The user sets the software flow control through the application program, and the application program sets the software flow control mode for the serial port chip through the driver.

For the cp210 serial port chip, its driver lacks the software flow control setting code, so the user cannot set the software flow control function through the application program. This adds the missing software flow control.

Signed-off-by Wang Sheng Long <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
[ johan: rework properly on top of recent termios changes ]
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/cp210x.c | 67 +++++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 2 deletions(-)
drivers/usb/serial/cp210x.c +65 -2
index fbb10dfc56e3..5bd14770065b 100644
@@ -377,6 +377,16 @@ static struct usb_serial_driver * const serial_drivers[] = {
 #define CONTROL_WRITE_DTR	0x0100
 #define CONTROL_WRITE_RTS	0x0200
 
+/* CP210X_(GET|SET)_CHARS */
+struct cp210x_special_chars {
+	u8	bEofChar;
+	u8	bErrorChar;
+	u8	bBreakChar;
+	u8	bEventChar;
+	u8	bXonChar;
+	u8	bXoffChar;
+};
+
 /* CP210X_VENDOR_SPECIFIC values */
 #define CP210X_READ_2NCONFIG	0x000E
 #define CP210X_READ_LATCH	0x00C2

@@ -1074,11 +1084,38 @@ 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) {
+		dev_err(&port->dev, "failed to set special chars: %d\n", result);
+		return result;
+	}
+
+	return 0;
+}
+
 static bool cp210x_termios_change(const struct ktermios *a, const struct ktermios *b)
 {
 	bool iflag_change;
 
-	iflag_change = ((a->c_iflag ^ b->c_iflag) & INPCK);
+	iflag_change = ((a->c_iflag ^ b->c_iflag) & (INPCK | IXON | IXOFF));
 
 	return tty_termios_hw_change(a, b) || iflag_change;
 }

@@ -1086,13 +1123,29 @@ static bool cp210x_termios_change(const struct ktermios *a, const struct ktermio
 static void cp210x_set_flow_control(struct tty_struct *tty,
 		struct usb_serial_port *port, struct ktermios *old_termios)
 {
+	struct cp210x_special_chars chars;
 	struct cp210x_flow_ctl flow_ctl;
 	u32 flow_repl;
 	u32 ctl_hs;
 	int ret;
 
-	if (old_termios && C_CRTSCTS(tty) == (old_termios->c_cflag & CRTSCTS))
+	if (old_termios &&
+			C_CRTSCTS(tty) == (old_termios->c_cflag & CRTSCTS) &&
+			I_IXON(tty) == (old_termios->c_iflag & IXON) &&
+			I_IXOFF(tty) == (old_termios->c_iflag & IXOFF)) {
 		return;
+	}
+
+	if (I_IXON(tty) || I_IXOFF(tty)) {
+		memset(&chars, 0, sizeof(chars));
+
+		chars.bXonChar = START_CHAR(tty);
+		chars.bXoffChar = STOP_CHAR(tty);
+
+		ret = cp210x_set_chars(port, &chars);
+		if (ret)
+			return;
+	}
 
 	ret = cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl,
 			sizeof(flow_ctl));

@@ -1118,6 +1171,16 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 		flow_repl |= CP210X_SERIAL_RTS_SHIFT(CP210X_SERIAL_RTS_ACTIVE);
 	}
 
+	if (I_IXOFF(tty))
+		flow_repl |= CP210X_SERIAL_AUTO_RECEIVE;
+	else
+		flow_repl &= ~CP210X_SERIAL_AUTO_RECEIVE;
+
+	if (I_IXON(tty))
+		flow_repl |= CP210X_SERIAL_AUTO_TRANSMIT;
+	else
+		flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT;
+
 	dev_dbg(&port->dev, "%s - ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n",
 			__func__, ctl_hs, flow_repl);
Signature · 1 line
2.26.2
maintainer Reviewed-by on v1 1/6
#2 lore raw

Re: [PATCH 1/6] USB: serial: cp210x: add support for software flow control

Quoted 64 lines · Johan Hovold

From: Wang Sheng Long <[email protected]>

When data is transmitted between two serial ports, the phenomenon of data loss often occurs. The two kinds of flow control commonly used in serial communication are hardware flow control and software flow control.

In serial communication, If you only use RX/TX/GND Pins, you can't do hardware flow. So we often used software flow control and prevent data loss. The user sets the software flow control through the application program, and the application program sets the software flow control mode for the serial port chip through the driver.

For the cp210 serial port chip, its driver lacks the software flow control setting code, so the user cannot set the software flow control function through the application program. This adds the missing software flow control.

Signed-off-by Wang Sheng Long <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
[ johan: rework properly on top of recent termios changes ]
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/cp210x.c | 67 +++++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 2 deletions(-)
drivers/usb/serial/cp210x.c +23 -0
index fbb10dfc56e3..5bd14770065b 100644
@@ -377,6 +377,16 @@ static struct usb_serial_driver * const serial_drivers[] = {
 #define CONTROL_WRITE_DTR	0x0100
 #define CONTROL_WRITE_RTS	0x0200
 
+/* CP210X_(GET|SET)_CHARS */
+struct cp210x_special_chars {
+	u8	bEofChar;
+	u8	bErrorChar;
+	u8	bBreakChar;
+	u8	bEventChar;
+	u8	bXonChar;
+	u8	bXoffChar;
+};
+
 /* CP210X_VENDOR_SPECIFIC values */
 #define CP210X_READ_2NCONFIG	0x000E
 #define CP210X_READ_LATCH	0x00C2

@@ -1074,11 +1084,38 @@ 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),

drivers/usb/serial/cp210x.c static void cp210x_disable_event_mode(struct usb_serial_port *port) · line 1109

Quoted 10 lines
drivers/usb/serial/cp210x.c +10 -0
+				CP210X_SET_CHARS, REQTYPE_HOST_TO_INTERFACE, 0,
+				port_priv->bInterfaceNumber,
+				dmabuf, sizeof(*chars), USB_CTRL_SET_TIMEOUT);
+
+	kfree(dmabuf);
+
+	if (result < 0) {
+		dev_err(&port->dev, "failed to set special chars: %d\n", result);
+		return result;
+	}

This is an "open coded" usb_control_msg_send() call :)

Other than that minor thing:

Reviewed-by Greg Kroah-Hartman <[email protected]>
author maintainer
#3 lore raw

Re: [PATCH 1/6] USB: serial: cp210x: add support for software flow control

Quoted 20 lines · Greg Kroah-Hartman
Quoted 19 lines · Johan Hovold

From: Wang Sheng Long <[email protected]>

drivers/usb/serial/cp210x.c +17 -0
+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);
+
Quoted 8 lines

drivers/usb/serial/cp210x.c static void cp210x_disable_event_mode(struct usb_serial_port *port) · line 1109

Quoted 6 lines
drivers/usb/serial/cp210x.c +6 -0
+	kfree(dmabuf);
+
+	if (result < 0) {
+		dev_err(&port->dev, "failed to set special chars: %d\n", result);
+		return result;
+	}

This is an "open coded" usb_control_msg_send() call :)

Right, but there are a few more functions like this one in this driver so I kept it for consistency for now.

Quoted 3 lines

Other than that minor thing:

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

Thanks for reviewing all of these. Now applied.

Johan

author maintainer
#4 lore raw

PATCH 2/6 USB: serial: cp210x: set IXOFF thresholds

In reply to Johan Hovold

At least CP2102 requires the XON/XOFF limits to be initialised in order for software input flow control (IXOFF) to work. Specifically, XOFF is never sent if the XOFF limit is left at its default value of zero.

Set the limits so that input is throttled when the FIFO free level drops below 128 bytes and restarted when the FIFO fill level drops below 128 bytes.

Note that the threshold values have been chosen so that they can be used also with CP2105 which has the smallest FIFO of the currently supported device types (288 byte for the SCI port). If needed the limits can be made device specific later.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/cp210x.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/serial/cp210x.c +3 -0
index 5bd14770065b..ee0139eb6636 100644
@@ -1181,6 +1181,9 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 	else
 		flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT;
 
+	flow_ctl.ulXonLimit = cpu_to_le32(128);
+	flow_ctl.ulXoffLimit = cpu_to_le32(128);
+
 	dev_dbg(&port->dev, "%s - ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n",
 			__func__, ctl_hs, flow_repl);
Signature · 1 line
2.26.2
author maintainer
#5 lore raw

PATCH 3/6 USB: serial: cp210x: update control-characters on every change

In reply to Johan Hovold

Update the XON/XOFF control characters also when no other flow-control flag has changed and software flow control is enabled.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/cp210x.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/usb/serial/cp210x.c +7 -3
index ee0139eb6636..4f90573c0d2b 100644
@@ -1113,11 +1113,13 @@ static int cp210x_set_chars(struct usb_serial_port *port,
 
 static bool cp210x_termios_change(const struct ktermios *a, const struct ktermios *b)
 {
-	bool iflag_change;
+	bool iflag_change, cc_change;
 
 	iflag_change = ((a->c_iflag ^ b->c_iflag) & (INPCK | IXON | IXOFF));
+	cc_change = a->c_cc[VSTART] != b->c_cc[VSTART] ||
+			a->c_cc[VSTOP] != b->c_cc[VSTOP];
 
-	return tty_termios_hw_change(a, b) || iflag_change;
+	return tty_termios_hw_change(a, b) || iflag_change || cc_change;
 }
 
 static void cp210x_set_flow_control(struct tty_struct *tty,

@@ -1132,7 +1134,9 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 	if (old_termios &&
 			C_CRTSCTS(tty) == (old_termios->c_cflag & CRTSCTS) &&
 			I_IXON(tty) == (old_termios->c_iflag & IXON) &&
-			I_IXOFF(tty) == (old_termios->c_iflag & IXOFF)) {
+			I_IXOFF(tty) == (old_termios->c_iflag & IXOFF) &&
+			START_CHAR(tty) == old_termios->c_cc[VSTART] &&
+			STOP_CHAR(tty) == old_termios->c_cc[VSTOP]) {
 		return;
 	}
Signature · 1 line
2.26.2
author maintainer
#6 lore raw

PATCH 4/6 USB: serial: cp210x: 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/cp210x.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)
drivers/usb/serial/cp210x.c +6 -12
index 4f90573c0d2b..360398665c17 100644
@@ -674,16 +674,13 @@ static int cp210x_write_reg_block(struct usb_serial_port *port, u8 req,
 
 	kfree(dmabuf);
 
-	if (result == bufsize) {
-		result = 0;
-	} else {
+	if (result < 0) {
 		dev_err(&port->dev, "failed set req 0x%x size %d status: %d\n",
 				req, bufsize, result);
-		if (result >= 0)
-			result = -EIO;
+		return result;
 	}
 
-	return result;
+	return 0;
 }
 
 /*

@@ -720,17 +717,14 @@ static int cp210x_write_vendor_block(struct usb_serial *serial, u8 type,
 
 	kfree(dmabuf);
 
-	if (result == bufsize) {
-		result = 0;
-	} else {
+	if (result < 0) {
 		dev_err(&serial->interface->dev,
 			"failed to set vendor val 0x%04x size %d: %d\n", val,
 			bufsize, result);
-		if (result >= 0)
-			result = -EIO;
+		return result;
 	}
 
-	return result;
+	return 0;
 }
 #endif
Signature · 1 line
2.26.2
author maintainer
#8 lore raw

PATCH 5/6 USB: serial: cp210x: drop unused includes

In reply to Johan Hovold

Drop include directives that are no longer used.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/cp210x.c | 3 ---
 1 file changed, 3 deletions(-)
drivers/usb/serial/cp210x.c +0 -3
index 360398665c17..0d0fc1f9e99b 100644
@@ -16,13 +16,10 @@
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
 #include <linux/module.h>
-#include <linux/moduleparam.h>
 #include <linux/usb.h>
-#include <linux/uaccess.h>
 #include <linux/usb/serial.h>
 #include <linux/gpio/driver.h>
 #include <linux/bitops.h>
-#include <linux/mutex.h>
 
 #define DRIVER_DESC "Silicon Labs CP210x RS232 serial adaptor driver"
Signature · 1 line
2.26.2
author maintainer
#9 lore raw

PATCH 6/6 USB: serial: cp210x: add copyright notice

In reply to Johan Hovold

Add a copyright notice for myself.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/usb/serial/cp210x.c | 1 +
 1 file changed, 1 insertion(+)
drivers/usb/serial/cp210x.c +1 -0
index 0d0fc1f9e99b..d813a052738f 100644
@@ -3,6 +3,7 @@
  * Silicon Laboratories CP210x USB to RS232 serial adaptor driver
  *
  * Copyright (C) 2005 Craig Shelley ([email protected])
+ * Copyright (C) 2010-2021 Johan Hovold ([email protected])
  *
  * Support to set flow control line levels using TIOCMGET and TIOCMSET
  * thanks to Karl Hiramoto [email protected]. RTSCTS hardware flow
Signature · 1 line
2.26.2