On Tue, Feb 24, 2015 at 06:16:18PM +0800, Peter Hung wrote: > The F81232 bulk-in is RX data + LSR channel, data format is > [LSR+Data][LSR+Data]..... , We had implemented in f81232_process_read_urb(). > > Signed-off-by: Peter Hung <hpeter+linux_kernel@xxxxxxxxx> > --- > drivers/usb/serial/f81232.c | 69 +++++++++++++++++++++++---------------------- > 1 file changed, 36 insertions(+), 33 deletions(-) > > diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c > index 1f29b95..419e2d6 100644 > --- a/drivers/usb/serial/f81232.c > +++ b/drivers/usb/serial/f81232.c > @@ -23,6 +23,7 @@ > #include <linux/uaccess.h> > #include <linux/usb.h> > #include <linux/usb/serial.h> > +#include <linux/serial_reg.h> > > static const struct usb_device_id id_table[] = { > { USB_DEVICE(0x1934, 0x0706) }, > @@ -183,44 +184,46 @@ exit: > static void f81232_process_read_urb(struct urb *urb) > { > struct usb_serial_port *port = urb->context; > - struct f81232_private *priv = usb_get_serial_port_data(port); > unsigned char *data = urb->transfer_buffer; > - char tty_flag = TTY_NORMAL; > - unsigned long flags; > - u8 line_status; > + char tty_flag; > int i; Use an unsigned type for i. > + u8 lsr; > > - /* update line status */ > - spin_lock_irqsave(&priv->lock, flags); > - line_status = priv->modem_status; > - priv->modem_status &= ~UART_STATE_TRANSIENT_MASK; > - spin_unlock_irqrestore(&priv->lock, flags); > - > - if (!urb->actual_length) > + if (urb->actual_length < 2) > return; > > - /* break takes precedence over parity, */ > - /* which takes precedence over framing errors */ > - if (line_status & UART_BREAK_ERROR) > - tty_flag = TTY_BREAK; > - else if (line_status & UART_PARITY_ERROR) > - tty_flag = TTY_PARITY; > - else if (line_status & UART_FRAME_ERROR) > - tty_flag = TTY_FRAME; > - dev_dbg(&port->dev, "%s - tty_flag = %d\n", __func__, tty_flag); > - > - /* overrun is special, not associated with a char */ > - if (line_status & UART_OVERRUN_ERROR) > - tty_insert_flip_char(&port->port, 0, TTY_OVERRUN); > - > - if (port->port.console && port->sysrq) { > - for (i = 0; i < urb->actual_length; ++i) > - if (!usb_serial_handle_sysrq_char(port, data[i])) > - tty_insert_flip_char(&port->port, data[i], > - tty_flag); > - } else { > - tty_insert_flip_string_fixed_flag(&port->port, data, tty_flag, > - urb->actual_length); > + /* bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]... */ > + > + for (i = 0; i < urb->actual_length; i += 2) { Again, what if the length is not a multiple of 2? > + tty_flag = TTY_NORMAL; > + lsr = data[i + 0]; Just use data[i] here. > + > + if (lsr & UART_LSR_BRK_ERROR_BITS) { > + if (lsr & UART_LSR_BI) { > + tty_flag = TTY_BREAK; > + port->icount.brk++; > + usb_serial_handle_break(port); > + } else if (lsr & UART_LSR_PE) { > + tty_flag = TTY_PARITY; > + port->icount.parity++; > + } else if (lsr & UART_LSR_FE) { > + tty_flag = TTY_FRAME; > + port->icount.frame++; > + } > + > + if (lsr & UART_LSR_OE) { > + port->icount.overrun++; > + tty_insert_flip_char(&port->port, 0, > + TTY_OVERRUN); Use at least two tabs indentation for continuation lines. > + } > + } > + > + if (port->port.console && port->sysrq) { > + if (usb_serial_handle_sysrq_char(port, data[i + 1])) > + continue; > + } > + > + tty_insert_flip_char(&port->port, data[i + 1], tty_flag); > } > > tty_flip_buffer_push(&port->port); Looks good now, otherwise. 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