yup,
you were right, the check for NULL is not necessary, usb_kill_urb,
handles everything for us. This yields even a lighter patch:
-------------------------------------------------------------------------
--- 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 */
---------------------------------------------------------------------------
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??
/* if the port is closed stop trying to read */
if (port->open_count > 0){
/* Continue trying to always read */
usb_fill_bulk_urb(port->read_urb, port->serial->dev,
usb_rcvbulkpipe(port->serial->dev,
port->bulk_in_endpointAddress),
port->read_urb->transfer_buffer,
port->read_urb->transfer_buffer_length,
ftdi_read_bulk_callback, port);
result = usb_submit_urb(port->read_urb, GFP_ATOMIC);
if (result)
err("%s - failed resubmitting read urb, error %d",
__FUNCTION__, result);
}
Jan
On Wed, 8 Sep 2004, Alan Stern wrote:
> On Wed, 8 Sep 2004, Jan Capek wrote:
>
> > Attached is a patch that replaces the use of usb_unlink_urb() of the read
> > URB when the device is being closed with usb_kill_urb(). The second call
> > is more appropriate and recommended by urb.c. In fact, the ftdi_close()
> > tries to shut down the URB's synchronously since the URB_ASYNC_UNLINK
> > flag is cleared. The current version with usb_unlink_urb(), causes the
> > kernel to generate a warning about this. Did I miss anything here, or is
> > it right to use the usb_kill_urb()?
>
> It is right. In fact, you can remove the test for whether port->read_urb
> is NULL; if you pass a NULL pointer to usb_kill_urb() it will simply
> return.
>
> The one thing to look out for is if the completion handler is running at
> about the same time and tries to resubmit the URB. When you call
> usb_kill_urb(), the resubmission will fail with -EPERM. The completion
> handler needs to be able to handle this properly.
>
> Alan Stern
>
>
>
>
-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
