> -----Original Message----- > From: Johan Hovold [mailto:jhovold@xxxxxxxxx] On Behalf Of Johan Hovold > Sent: Friday, April 29, 2016 05:09 > To: Valentin Yakovenkov > Cc: Konstantin Shkolnyy; linux-usb@xxxxxxxxxxxxxxx > Subject: [EXT] Re: [PATCH] Add DCD line support to CP210x driver > > 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, Do you know for sure that they can't be split over multiple transfers, in today's and future chips? > 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
