[PATCH 0/6] USB: serial: cp210x: add software flow-control support

[PATCH 0/6] USB: serial: cp210x: add software flow-control support

[Johan Hovold]

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

-- 
2.26.2

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

[Johan Hovold]

From: Wang Sheng Long <shenglong.wang.ext@siemens.com>

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 <shenglong.wang.ext@siemens.com>
Link: https://lore.kernel.org/r/20210104094502.3942-1-china_shenglong@163.com
[ johan: rework properly on top of recent termios changes ]
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 67 +++++++++++++++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index fbb10dfc56e3..5bd14770065b 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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);
 
-- 
2.26.2

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

[Greg Kroah-Hartman]

On Mon, Jan 18, 2021 at 12:13:26PM +0100, Johan Hovold wrote:
> From: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> 
> 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 <shenglong.wang.ext@siemens.com>
> Link: https://lore.kernel.org/r/20210104094502.3942-1-china_shenglong@163.com
> [ johan: rework properly on top of recent termios changes ]
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/serial/cp210x.c | 67 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 65 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index fbb10dfc56e3..5bd14770065b 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -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;
> +	}

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

Other than that minor thing:

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

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

[Johan Hovold]

On Mon, Jan 18, 2021 at 01:42:59PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 18, 2021 at 12:13:26PM +0100, Johan Hovold wrote:
> > From: Wang Sheng Long <shenglong.wang.ext@siemens.com>

> > +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;
> > +	}
> 
> 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.

> Other than that minor thing:
> 
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks for reviewing all of these. Now applied.

Johan

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

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

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 5bd14770065b..ee0139eb6636 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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);
 
-- 
2.26.2

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

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

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index ee0139eb6636..4f90573c0d2b 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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;
 	}
 
-- 
2.26.2

[PATCH 4/6] USB: serial: cp210x: drop short control-transfer checks

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

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 4f90573c0d2b..360398665c17 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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
 
-- 
2.26.2

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

[Johan Hovold]

Drop include directives that are no longer used.

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

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 360398665c17..0d0fc1f9e99b 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -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"
 
-- 
2.26.2

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

[Johan Hovold]

Add a copyright notice for myself.

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

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index 0d0fc1f9e99b..d813a052738f 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -3,6 +3,7 @@
  * Silicon Laboratories CP210x USB to RS232 serial adaptor driver
  *
  * Copyright (C) 2005 Craig Shelley (craig@microtron.org.uk)
+ * Copyright (C) 2010-2021 Johan Hovold (johan@kernel.org)
  *
  * Support to set flow control line levels using TIOCMGET and TIOCMSET
  * thanks to Karl Hiramoto karl@hiramoto.org. RTSCTS hardware flow
-- 
2.26.2

Re: [PATCH 0/6] USB: serial: cp210x: add software flow-control support

[Greg Kroah-Hartman]

On Mon, Jan 18, 2021 at 12:13:25PM +0100, Johan Hovold wrote:
> 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.

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