diff mbox series

[v3] usbserial: cp210x - icount support for parity error checking

Message ID 7bdff86f-0988-2afc-e1a6-35df2931fd5b@jrr.cz (mailing list archive)
State Superseded
Headers show
Series [v3] usbserial: cp210x - icount support for parity error checking | expand

Commit Message

Jaromir Skorpil June 21, 2020, 8:21 p.m. UTC
The current version of cp210x driver doesn't provide any way to detect
a parity error in received data from userspace. Some serial protocols like
STM32 bootloader protect data only by parity so application needs to
know whether parity error happened to repeat peripheral data reading.

Added support for icount (ioctl TIOCGICOUNT) which sends GET_COMM_STATUS
command to CP210X and according received flags increments fields for
parity error, frame error, break and overrun. An application can detect
an error condition after reading data from ttyUSB and reacts adequately.
There is no impact for applications which don't call ioctl TIOCGICOUNT.

The flag "hardware overrun" is not examined because CP2102 sets this bit
for the first received byte after openning of port which was previously
closed with some unreaded data in buffer. This is confusing and checking
"queue overrun" flag seems be enough.

Signed-off-by: Jaromír Škorpil <Jerry@jrr.cz>
---
v2: Simplified counting - only queue overrun checked
v3: Changed description + UTF-8 name  ;-)

  cp210x.c |   42 +++++++++++++++++++++++++++++++++++++-----
  1 file changed, 37 insertions(+), 5 deletions(-)

Comments

Greg KH June 22, 2020, 5:31 a.m. UTC | #1
On Sun, Jun 21, 2020 at 10:21:11PM +0200, Jaromír Škorpil wrote:
> The current version of cp210x driver doesn't provide any way to detect
> a parity error in received data from userspace. Some serial protocols like
> STM32 bootloader protect data only by parity so application needs to
> know whether parity error happened to repeat peripheral data reading.
> 
> Added support for icount (ioctl TIOCGICOUNT) which sends GET_COMM_STATUS
> command to CP210X and according received flags increments fields for
> parity error, frame error, break and overrun. An application can detect
> an error condition after reading data from ttyUSB and reacts adequately.
> There is no impact for applications which don't call ioctl TIOCGICOUNT.
> 
> The flag "hardware overrun" is not examined because CP2102 sets this bit
> for the first received byte after openning of port which was previously
> closed with some unreaded data in buffer. This is confusing and checking
> "queue overrun" flag seems be enough.
> 
> Signed-off-by: Jaromír Škorpil <Jerry@jrr.cz>
> ---
> v2: Simplified counting - only queue overrun checked
> v3: Changed description + UTF-8 name  ;-)

Much nicer, thanks for the changes!

I'll let Johan comment on the actual patch itself, as he's the
maintainer of this driver/subsystems.

thanks,

greg k-h
diff mbox series

Patch

diff -up linux-5.8-rc1/drivers/usb/serial/cp210x.c j/drivers/usb/serial/cp210x.c
--- linux-5.8-rc1/drivers/usb/serial/cp210x.c
+++ j/drivers/usb/serial/cp210x.c
@@ -43,6 +43,8 @@  static int cp210x_tiocmget(struct tty_st
  static int cp210x_tiocmset(struct tty_struct *, unsigned int, unsigned int);
  static int cp210x_tiocmset_port(struct usb_serial_port *port,
  		unsigned int, unsigned int);
+static int cp210x_get_icount(struct tty_struct *tty,
+		struct serial_icounter_struct *icount);
  static void cp210x_break_ctl(struct tty_struct *, int);
  static int cp210x_attach(struct usb_serial *);
  static void cp210x_disconnect(struct usb_serial *);
@@ -274,6 +276,7 @@  static struct usb_serial_driver cp210x_d
  	.tx_empty		= cp210x_tx_empty,
  	.tiocmget		= cp210x_tiocmget,
  	.tiocmset		= cp210x_tiocmset,
+	.get_icount		= cp210x_get_icount,
  	.attach			= cp210x_attach,
  	.disconnect		= cp210x_disconnect,
  	.release		= cp210x_release,
@@ -393,6 +396,13 @@  struct cp210x_comm_status {
  	u8       bReserved;
  } __packed;
  
+/* cp210x_comm_status::ulErrors */
+#define CP210X_SERIAL_ERR_BREAK	BIT(0)
+#define CP210X_SERIAL_ERR_FRAME	BIT(1)
+#define CP210X_SERIAL_ERR_HW_OVERRUN	BIT(2)
+#define CP210X_SERIAL_ERR_QUEUE_OVERRUN	BIT(3)
+#define CP210X_SERIAL_ERR_PARITY	BIT(4)
+
  /*
   * CP210X_PURGE - 16 bits passed in wValue of USB request.
   * SiLabs app note AN571 gives a strange description of the 4 bits:
@@ -836,10 +846,10 @@  static void cp210x_close(struct usb_seri
  }
  
  /*
- * Read how many bytes are waiting in the TX queue.
+ * Read how many bytes are waiting in the TX queue and update error counters.
   */
-static int cp210x_get_tx_queue_byte_count(struct usb_serial_port *port,
-		u32 *count)
+static int cp210x_get_comm_status(struct usb_serial_port *port,
+		u32 *tx_count)
  {
  	struct usb_serial *serial = port->serial;
  	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
@@ -855,7 +865,16 @@  static int cp210x_get_tx_queue_byte_coun
  			0, port_priv->bInterfaceNumber, sts, sizeof(*sts),
  			USB_CTRL_GET_TIMEOUT);
  	if (result == sizeof(*sts)) {
-		*count = le32_to_cpu(sts->ulAmountInOutQueue);
+		if (tx_count)
+			*tx_count = le32_to_cpu(sts->ulAmountInOutQueue);
+		if (sts->ulErrors & CP210X_SERIAL_ERR_BREAK)
+			port->icount.brk++;
+		if (sts->ulErrors & CP210X_SERIAL_ERR_FRAME)
+			port->icount.frame++;
+		if (sts->ulErrors & CP210X_SERIAL_ERR_QUEUE_OVERRUN)
+			port->icount.overrun++;
+		if (sts->ulErrors & CP210X_SERIAL_ERR_PARITY)
+			port->icount.parity++;
  		result = 0;
  	} else {
  		dev_err(&port->dev, "failed to get comm status: %d\n", result);
@@ -873,13 +892,26 @@  static bool cp210x_tx_empty(struct usb_s
  	int err;
  	u32 count;
  
-	err = cp210x_get_tx_queue_byte_count(port, &count);
+	err = cp210x_get_comm_status(port, &count);
  	if (err)
  		return true;
  
  	return !count;
  }
  
+static int cp210x_get_icount(struct tty_struct *tty,
+		struct serial_icounter_struct *icount)
+{
+	struct usb_serial_port *port = tty->driver_data;
+	int result;
+
+	result = cp210x_get_comm_status(port, NULL);
+	if (result)
+		return result;
+
+	return usb_serial_generic_get_icount(tty, icount);
+}
+
  /*
   * cp210x_get_termios
   * Reads the baud rate, data bits, parity, stop bits and flow control mode