On Mon, Jun 22, 2020 at 05:13:41PM +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@xxxxxx>
> ---
> v2: Simplified counting - only queue overrun checked
> v3: Changed description + UTF8 name
> v4: Corrected endian
So let's go with this and then I can add support for in-band reporting
on top.
I was gonna apply it and the missing locking, but noticed that the patch
is white-space damaged. At least some leading tabs have been converted.
Try sending the patch yourself (e.g. using git-send-email) and make sure
you can apply it using git-am.
> cp210x.c | 43 ++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 38 insertions(+), 5 deletions(-)
>
> 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)
Can you drop the SERIAL_ infix here?
> +
> /*
> * 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,17 @@ 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);
> + u32 flags = le32_to_cpu(sts->ulErrors);
Here should be a newline.
> + if (flags & CP210X_SERIAL_ERR_BREAK)
> + port->icount.brk++;
> + if (flags & CP210X_SERIAL_ERR_FRAME)
> + port->icount.frame++;
> + if (flags & CP210X_SERIAL_ERR_QUEUE_OVERRUN)
> + port->icount.overrun++;
> + if (flags & CP210X_SERIAL_ERR_PARITY)
> + port->icount.parity++;
And these icount increments should be done under the port->lock as
TIOCGICOUNT can be called in parallel.
> + if (tx_count)
> + *tx_count = le32_to_cpu(sts->ulAmountInOutQueue);
> result = 0;
> } else {
> dev_err(&port->dev, "failed to get comm status: %d\n", result);
> @@ -873,13 +893,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
>
>
>
Johan
