On Sun, 24 Jan 2010 18:03:48 +0100
Johan Hovold <jhovold@xxxxxxxxx> wrote:
> > - 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;
> > + }
I resent this. The unbalanced braces in branches of an if() statement
are ugly like all get out. If I add them in one branch, I'm going to
add them in the other.
> > static void mct_u232_close(struct usb_serial_port *port)
> > {
> > +
>
> White space error.
>
> > dbg("%s port %d", __func__, port->number);
Um, why is that again? But whatever, I'll drop it.
> > + 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).
OK, I'll review this. My actual tests worked correctly with the
above code (in release method) and failed without. And the bug is
easily reproducible. But perhaps something changed in the last second
before I resent the patch.
-- Pete
--
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
