On Fri, Jan 22, 2010 at 02:21:12PM -0700, Pete Zaitcev wrote:
> I ran the following test:
>
> while true; do echo -n "01234567890abcdefghij" >/dev/ttyUSB0; done
>
> The data is sent over the serial line with random loss. It occurs because
> 1. when echo closes ttyUSB0, there's a chance that it cancels the write URB.
> 2. MCT U232 buffers the last character and acks the USB data before the
> serial bits are completely sent out.
>
> This was originally reported in Red Hat Linux bug #170490.
>
> The fix relies on modern facilities drain_delay (approximately the
> number of bytes outstanding, see case #2 above) and chars_in_buffer.
> The latter did not account for the last buffer that was removed
> from the kfifo but not completed yet. I considered it a bug and
> changed it.
>
> The test above works as expected with this patch applied.
>
> Signed-off-by: Pete Zaitcev <zaitcev@xxxxxxxxxx>
>
> ---
> drivers/usb/serial/generic.c | 7 +++++--
> drivers/usb/serial/mct_u232.c | 3 +++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> This is a resend due to misapplication. Should not fuzz this time!
>
> diff -urp -X dontdiff linux-2.6-gregkh/drivers/usb/serial/generic.c linux-2.6-tip/drivers/usb/serial/generic.c
> --- linux-2.6-gregkh/drivers/usb/serial/generic.c 2010-01-22 13:50:50.433875777 -0700
> +++ linux-2.6-tip/drivers/usb/serial/generic.c 2010-01-05 11:32:02.346260069 -0700
> @@ -387,10 +387,13 @@ int usb_serial_generic_chars_in_buffer(s
> dbg("%s - port %d", __func__, port->number);
>
> spin_lock_irqsave(&port->lock, flags);
> - if (serial->type->max_in_flight_urbs)
> + if (serial->type->max_in_flight_urbs) {
Superfluous {.
> chars = port->tx_bytes_flight;
> - else if (serial->num_bulk_out)
> + } else if (serial->num_bulk_out) {
> + /* This overcounts badly, but is good enough for drain wait. */
> chars = kfifo_len(&port->write_fifo);
> + chars += port->write_urb_busy * port->bulk_out_size;
> + }
> spin_unlock_irqrestore(&port->lock, flags);
>
> dbg("%s - returns %d", __func__, chars);
> diff -urp -X dontdiff linux-2.6-gregkh/drivers/usb/serial/mct_u232.c linux-2.6-tip/drivers/usb/serial/mct_u232.c
> --- linux-2.6-gregkh/drivers/usb/serial/mct_u232.c 2010-01-22 13:50:50.446875757 -0700
> +++ linux-2.6-tip/drivers/usb/serial/mct_u232.c 2009-12-07 20:18:13.649078959 -0700
> @@ -547,8 +540,11 @@ static void mct_u232_dtr_rts(struct usb_
>
> static void mct_u232_close(struct usb_serial_port *port)
> {
> +
White space error.
> dbg("%s port %d", __func__, port->number);
>
> + port->port.drain_delay = 2;
> +
You need to set drain_delay in open rather than close, as drain wait is
done before driver-specific close is called (see tty_port_close).
> if (port->serial->dev) {
> /* shutdown our urbs */
> usb_kill_urb(port->write_urb);
With those changes in place,
Acked-by: Johan Hovold <jhovold@xxxxxxxxx>
--
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
