On Thu, Mar 24, 2016 at 10:53:27AM +0300, Valentin Yakovenkov wrote:
> Sorry, I missed the branch.
>
> Here it is.
Thanks for the patch. Please resubmit it on a format that can be applied
(i.e. without quoted text, etc). Remember to include a patch
revision in the Subject when resubmitting patches (this one should be
[PATCH v3] or so). Putting a changelog below the cut-off line (---) is
also recommended.
> This patch adds DCD line support to CP210x USB serial driver.
>
> First it enables CP210x events embedding to incoming URB's by calling:
> cp210x_set_config_single(port, CP210X_EMBED_EVENTS, CP210X_ESCCHAR);
>
> Then it parses incoming URB's via custom routine:
> cp210x_process_read_urb(...)
> searches for event sequences and handles all of DCD changes calling
> usb_serial_handle_dcd_change(...)
>
> Signed-off-by: Valentin Yakovenkov <yakovenkov@xxxxxxxxx>
> ---
> drivers/usb/serial/cp210x.c | 118 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 117 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index c740592..ef0e8b1 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -47,6 +47,7 @@ static void cp210x_break_ctl(struct tty_struct *, int);
> static int cp210x_port_probe(struct usb_serial_port *);
> static int cp210x_port_remove(struct usb_serial_port *);
> static void cp210x_dtr_rts(struct usb_serial_port *p, int on);
> +static void cp210x_process_read_urb(struct urb *urb);
>
> static const struct usb_device_id id_table[] = {
> { USB_DEVICE(0x045B, 0x0053) }, /* Renesas RX610 RX-Stick */
> @@ -199,9 +200,21 @@ static const struct usb_device_id id_table[] = {
>
> MODULE_DEVICE_TABLE(usb, id_table);
>
> +#define CP210X_ESCCHAR 0x1e
Move this below the PURGE_ALL definition.
> +enum cp210x_rx_state {
> + CP210X_STATE_IDLE = 0,
> + CP210X_STATE_ESC,
> + CP210X_STATE_LS0,
> + CP210X_STATE_LS1,
> + CP210X_STATE_LS,
> + CP210X_STATE_MS
> +};
> +
> +
> struct cp210x_port_private {
> __u8 bInterfaceNumber;
> bool has_swapped_line_ctl;
> + enum cp210x_rx_state rx_state;
This shouldn't be needed (see below).
> };
>
> static struct usb_serial_driver cp210x_device = {
> @@ -222,7 +235,8 @@ static struct usb_serial_driver cp210x_device = {
> .tiocmset = cp210x_tiocmset,
> .port_probe = cp210x_port_probe,
> .port_remove = cp210x_port_remove,
> - .dtr_rts = cp210x_dtr_rts
> + .dtr_rts = cp210x_dtr_rts,
> + .process_read_urb = cp210x_process_read_urb
> };
>
> static struct usb_serial_driver * const serial_drivers[] = {
> @@ -588,6 +602,11 @@ static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *port)
> {
> int result;
>
> + struct usb_serial *serial = port->serial;
> + struct cp210x_port_private *spriv = usb_get_serial_data(serial);
> +
> + spriv->rx_state = CP210X_STATE_IDLE;
> +
> result = cp210x_write_u16_reg(port, CP210X_IFC_ENABLE, UART_ENABLE);
> if (result) {
> dev_err(&port->dev, "%s - Unable to enable UART\n", __func__);
> @@ -601,6 +620,15 @@ static int cp210x_open(struct tty_struct *tty, struct usb_serial_port *port)
> if (tty)
> cp210x_change_speed(tty, port, NULL);
>
> + /* Enable events embedding to data stream */
> + result = cp210x_write_u16_reg(port, CP210X_EMBED_EVENTS,
> + CP210X_ESCCHAR);
> + if (result) {
> + dev_err(&port->dev, "%s - Unable to enable event embedding on UART\n",
> + __func__);
> + return result;
> + }
> +
> return usb_serial_generic_open(tty, port);
> }
>
> @@ -659,6 +687,94 @@ static bool cp210x_tx_empty(struct usb_serial_port *port)
> return !count;
> }
>
> +static void cp210x_process_read_urb(struct urb *urb)
> +{
> + struct usb_serial_port *port = urb->context;
> + char *ch = (char *)urb->transfer_buffer;
> + char *tbuf = (char *)urb->transfer_buffer;
> + int i;
> + int tcnt = 0;
> + struct usb_serial *serial = port->serial;
> + struct cp210x_port_private *spriv = usb_get_serial_data(serial);
> +
> + if (!urb->actual_length)
> + return;
> +
> + /* Process escape chars */
> + for (i = 0; i < urb->actual_length; i++) {
> + char c = ch[i];
> +
> + switch (spriv->rx_state) {
> + case CP210X_STATE_IDLE:
> + if (c == CP210X_ESCCHAR)
> + spriv->rx_state = CP210X_STATE_ESC;
> + else
> + tbuf[tcnt++] = c;
> + break;
> +
> + case CP210X_STATE_ESC:
> + if (c == 0x01)
> + spriv->rx_state = CP210X_STATE_LS0;
> + else if (c == 0x02)
> + spriv->rx_state = CP210X_STATE_LS;
> + else if (c == 0x03)
> + spriv->rx_state = CP210X_STATE_MS;
> + else {
> + tbuf[tcnt++] = (c == 0x00) ? CP210X_ESCCHAR : c;
> + spriv->rx_state = CP210X_STATE_IDLE;
> + }
> + break;
> +
> + case CP210X_STATE_LS0:
> + spriv->rx_state = CP210X_STATE_LS1;
> + break;
> +
> + case CP210X_STATE_LS1:
> + tbuf[tcnt++] = c;
> + spriv->rx_state = CP210X_STATE_IDLE;
> + break;
> +
> + case CP210X_STATE_LS:
> + spriv->rx_state = CP210X_STATE_IDLE;
> + break;
> +
> + case CP210X_STATE_MS:
> + if (c & 0x08) {
> + /* DCD change event */
> + struct tty_struct *tty;
> +
> + port->icount.dcd++;
> + tty = tty_port_tty_get(&port->port);
> + if (tty)
> + usb_serial_handle_dcd_change(port, tty,
> + c & 0x80);
> + tty_kref_put(tty);
> +
> + }
> + wake_up_interruptible(&port->port.delta_msr_wait);
> + spriv->rx_state = CP210X_STATE_IDLE;
> + break;
> + }
> + }
Instead of maintaining this state machine (which should not need to be
global, or can the events really be split over multiple bulk
transfers?), just scan the buffer for the escape character and if found,
process the symbols one by one for that buffer. If line errors were
detected, this should also be reported to user space (e.g. using
tty_insert_flip_char and a non TTY_NORMAL flag), etc.
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
