David and Greg:
Stepping back and trying to look at the bigger picture led to the
following thoughts regarding the synchronous API.
First of all, what's the reason for including usb_control_msg() and
usb_bulk_msg() in the API at all? Clearly, it's to make things easier for
device drivers, in a number of ways:
(1). It only requires a single function call.
(2). There's no need to allocate/free memory or any other
resources; the API does it automatically.
(3). There's no need to write a completion routine.
(4). It automatically waits for the transfer to complete before
returning (i.e., it is synchronous).
(5). It provides timeouts, which are not so easy to program
using the lower-level API calls.
There is basically just one disadvantage: you can't abort or unlink the
transfer, either
(6). for some internal driver-related purpose, or
(7). because disconnect() was called and it needs to cancel all
the ongoing transfers.
A replacement API should keep (1) - (5) while providing a way to handle
(7) at least. (6) isn't so much of an issue -- obviously no driver is
trying to do it presently because it is currently impossible, so adding
(6) would only serve to assist future driver development.
Our recent efforts have been aimed at replacing the API with a pair of
function calls: one to submit and one to wait-for-completion/timeout.
This goes slightly against (1), but not enough to really matter. It
provides a way to handle (6) and (7) because the submit function can
return some sort of handle that can later be passed to a cancel routine
if needed.
David's last proposal was that this handle could simply be an urb (or more
properly, a pointer to an urb). If a struct completion member were added
into struct urb then all the required facilities would be present right
there, which would make things very simple.
Or would it? The urb in question has to be allocated and freed somehow,
and either the driver or the core has to take care of this. By (2), this
ought to be handled by the core. So the submit routine allocates the urb
and passes a pointer to it back to the caller, while the
wait-for-completion/timeout routine frees the urb. But this leads to an
unavoidable race should the driver try to unlink the urb: There's no way
to insure that the unlink call arrives before the urb has been freed.
Okay, so this means that the driver has to allocate and free the urb.
What used to be a simple function call has now turned into (without error
checking):
urb = usb_allocate_urb(...);
usb_submit_bulk_msg(urb, ...);
status = usb_wait_msg(urb, timeout, &bytes);
usb_free_urb(urb);
Now consider what happens when we add the following enhancements to the
low-level API...
David's io_wait_completion() is a good thing. It belongs in the kernel
proper, of course, but we are better off having it available in any form.
Adding a struct completion member to struct urb is also a powerful tool.
For instance, consider this advantage: There will no longer be any need
to have a completion routine for every urb. The core can simply check,
and if the pointer to the completion handler is NULL then the core can
just call complete_all() on the struct completion member.
What if there is a completion function -- who should call complete_all()
then? There are two concerns:
(a). The completion function will most likely need to examine
some of the fields in the urb. But the routines waiting on the struct
completion might want to alter those same fields. Hence complete_all()
can't be called _before_ the completion function.
(b). The completion function might reinitialize and resubmit the
urb. Once this is done it's too late to call complete_all() -- doing so
would awaken tasks that could be waiting for the newly-submitted urb
instead of the previously-finished one. Hence complete_all() can't be
called _after_ the completion function returns.
Putting (a) and (b) together, it's clear the complete_all() has to be
called from within the completion function. That makes sense; after all,
the driver best knows when the proper time is to awaken the waiting tasks.
Another advantage of doing this is that it entirely removes the need for a
synchronous form of usb_unlink_urb()! Provided the driver makes sure that
the completion function calls comlete_all() at the proper time (or if
there is no completion function), the effect of a synchronous unlink can
be achieved just be doing:
usb_unlink_urb(urb);
wait_for_completion(&urb->cmplt);
With just these two API additions, look at what usb_bulk_msg() turns into
as a series of low-level API calls (again without error checking):
urb = usb_allocate_urb(...);
usb_fill_bulk_urb(urb, ...); // No completion handler
usb_submit_urb(urb);
if (io_wait_completion(&urb->cmplt, timeout) == -ETIMEDOUT) {
usb_unlink_urb(urb);
io_wait_completion(&urb->cmplt, 0);
}
/* Save urb->status, urb->actual_length */
usb_free_urb(urb);
That's not too much worse than the high-level API. (Furthermore, the
fourth-through-seventh lines could be encapsulated in a simple function.)
Okay, the difference is more pronounced for control messages. But it
certainly reduces the impetus for using usb_bulk_msg().
Here is a radical new thought. Suppose we could gain advantage (7)
(although not (6)) without changing the synchronous API at all! Since (6)
isn't really necessary, this would be among the better, if not the best,
of all possible worlds.
Unfortunately, to do this requires a far-reaching change. It probably
should have been done earlier (for all I know, it was proposed but nobody
thought it was a good idea or wanted to do it). Here's the idea: Change
the
struct usb_device *dev;
member of struct urb into
struct usb_interface *intf;
This is only logical. Since drivers are bound to interfaces, not devices,
they should submit request blocks bound to their interface, not their
device. Of course the core can easily find the device given the interface
pointer.
One ramification would be the need to generate a pseudo-interface for each
device, so that the core's own device-configuration I/O can be carried out
without the need to refer to a real interface (which wouldn't be
accessible anyway if the device had not yet been configured). That
shouldn't present any problem.
The main point of the change is that drivers would not need to be
responsible for cancelling all the transfers when their disconnect()
routine is called -- the core would be able to do it for them. Since urbs
would now be marked as belonging to a particular interface, it would be
easy for the HC driver code to find and unlink all urbs attached to a
given interface.
The implementation would be simple. Before calling a driver's
disconnect() for whatever reason (physical disconnect, rmmod,
configuration change, altsetting change, etc.) -- and also following an
unsuccessful probe() -- first flag the struct interface not to allow any
new urb submissions. After calling disconnect(), tell the HC driver to
unlink all the urbs belonging to the interface in question. When the
entire procedure is finished, flag the interface to allow submissions
again (if appropriate).
I realize this would be a big change. But maybe it's worthwhile.
Alan Stern
-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger
for complex code. Debugging C/C++ programs can leave you feeling lost and
disoriented. TotalView can help you find your way. Available on major UNIX
and Linux platforms. Try it free. www.etnus.com
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
