On Fri, Apr 29, 2016 at 04:37:49PM +0000, Konstantin Shkolnyy wrote: > > -----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: > > > +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? We don't need to care about future chips just yet, but you should know how the current firmware has been implemented, right? Does it really split escape chars over multiple transfers? 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
