Hi,
> > I also checked, how the read URB's are resubmitted, the logic in the
> > driver is correct. The upper layer (usb-serial.c) calls ftdi_close, only
> > when the port->open_count <= 0. The function ftdi_process_read(), that is
> > issued by the completion handler (ftdi_read_bulk_callback()), tries to
> > resubmit the URB iff the port hasn't been closed yet(see code snippet
> > below). I could think of, a race condition on an SMP system, where 1
> > thread has just finished the comparison (port->open_count > 0) and the
> > other thread would try killing the URB at the same time - last process
> > closed the device. Do we want the error be displayed??
>
> That race condition is exactly what I was talking about. Since it's bound
> to be pretty rare, there's probably no harm in displaying the error
> message. But it also wouldn't hurt to check for result != -EPERM. It's
> your choice.
>
> Alan Stern
I think, it's better to leave the error message in there as it is. The
attached patch covers the fixes in ftdi_sio.c and usb-serial.c.
Could anyone (Ian/Alan?) put the 'signed-off-by:' stamp on it after
looking at it, so that it can be applied by Greg?
Thanks in advance,
Jan
--- linux-2.6.9-rc1-mm1/drivers/usb/serial/ftdi_sio.c 2004-09-08 04:51:23.000000000
+0200
+++ linux-2.6.9-rc1-mm1-ccs/drivers/usb/serial/ftdi_sio.c 2004-09-08
14:00:53.000000000 +0200
@@ -1487,17 +1487,9 @@
}
} /* Note change no line if hupcl is off */
- /* shutdown our bulk read */
- if (port->read_urb) {
- if (usb_unlink_urb (port->read_urb) < 0) {
- /* Generally, this isn't an error. If the previous
- read bulk callback occurred (or is about to occur)
- while the port was being closed or was throtted
- (and is still throttled), the read urb will not
- have been submitted. */
- dbg("%s - failed to unlink read urb (generally not an error)",
__FUNCTION__);
- }
- }
+ /* shutdown our bulk read - synchronously */
+ usb_kill_urb(port->read_urb);
+
} /* ftdi_close */
--- linux-2.6.9-rc1-mm1/drivers/usb/serial/usb-serial.c 2004-09-08 04:51:23.000000000
+0200
+++ linux-2.6.9-rc1-mm1-ccs/drivers/usb/serial/usb-serial.c 2004-09-08
15:31:11.000000000 +0200
@@ -14,6 +14,14 @@
*
* See Documentation/usb/usb-serial.txt for more information on using this driver
*
+ * (09/08/2004) Jan Capek
+ * destroy_serial(), port_release() - removed test on URB's being NULL as
+ * these checks are always performed by underlying usb_kill_urb(),
+ * usb_unlink_urb, usb_free_urb() resp. Further, port->read_urb is now
+ * removed synchronously using usb_kill_urb() instead of usb_unlink_urb().
+ * This is to ensure consistency with the urb.c, so that usb_unlink_urb()
+ * is not used for synchronous unlinking anymore.
+ *
* (12/10/2002) gkh
* Split the ports off into their own struct device, and added a
* usb-serial bus driver.
@@ -454,18 +462,15 @@
port = serial->port[i];
if (!port)
continue;
- if (port->read_urb) {
- usb_unlink_urb(port->read_urb);
- usb_free_urb(port->read_urb);
- }
- if (port->write_urb) {
- usb_unlink_urb(port->write_urb);
- usb_free_urb(port->write_urb);
- }
- if (port->interrupt_in_urb) {
- usb_unlink_urb(port->interrupt_in_urb);
- usb_free_urb(port->interrupt_in_urb);
- }
+ usb_kill_urb(port->read_urb);
+ usb_free_urb(port->read_urb);
+
+ usb_unlink_urb(port->write_urb);
+ usb_free_urb(port->write_urb);
+
+ usb_unlink_urb(port->interrupt_in_urb);
+ usb_free_urb(port->interrupt_in_urb);
+
kfree(port->bulk_in_buffer);
kfree(port->bulk_out_buffer);
kfree(port->interrupt_in_buffer);
@@ -818,18 +823,16 @@
struct usb_serial_port *port = to_usb_serial_port(dev);
dbg ("%s - %s", __FUNCTION__, dev->bus_id);
- if (port->read_urb) {
- usb_unlink_urb(port->read_urb);
- usb_free_urb(port->read_urb);
- }
- if (port->write_urb) {
- usb_unlink_urb(port->write_urb);
- usb_free_urb(port->write_urb);
- }
- if (port->interrupt_in_urb) {
- usb_unlink_urb(port->interrupt_in_urb);
- usb_free_urb(port->interrupt_in_urb);
- }
+
+ usb_kill_urb(port->read_urb);
+ usb_free_urb(port->read_urb);
+
+ usb_unlink_urb(port->write_urb);
+ usb_free_urb(port->write_urb);
+
+ usb_unlink_urb(port->interrupt_in_urb);
+ usb_free_urb(port->interrupt_in_urb);
+
kfree(port->bulk_in_buffer);
kfree(port->bulk_out_buffer);
kfree(port->interrupt_in_buffer);
