On Wed, 12 Oct 2005, Franck wrote:
> > That's because the link belongs to usbcore, not to the HCD. Yes, you
> > could move the code that links the URB to just before calling urb_enqueue.
> > But then (a) you would have to handle the case of root-hub URBs
> > separately, and (b) you would also have to move the code that handles the
> > hcd_data_lock spinlock.
> >
>
> Does a patch that do this have any chance to be accepted ?
I think we should decide on the _correct_ solution first. Then acceptance
will be obvious.
> > The problem is that HCDs need to protect their links by locks, and usbcore
> > isn't aware of the HCDs' internal locks. That makes it hard to share the
> > list.
> >
>
> but HCD is aware of the lock that protects the ep's list.
No it isn't. That lock is declared static in core/hcd.c. It's called
hcd_data_lock.
> > Maybe a better idea would be for usbcore not to use the list at all, and
> > rely on the HCD to manage it. However that list is currently the only way
> > usbcore has of checking whether or not an URB actually is active. Maybe
> > instead it could simply look to see whether urb->hcpriv is non-NULL.
> >
>
> What do you mean by active exactly ? Does it mean linked ? or does it
> mean that the HCD _is_ transfering the URB ?
I mean linked. usbcore has no way to tell what an HCD is doing; as far
as it is concerned an URB is active from the time the urb_enqueue method
is called until giveback_urb is called.
Another part of the problem is that the endpoint's list is used by usbcore
in hcd_endpoint_disable. usbcore has to know what URBs are linked for an
endpoint so that they can all be unlinked when the endpoint is disabled.
One possible answer to your problem is the patch below. It's not a
complete answer because the DMA mapping isn't set up. On the whole, it
would be best for the HCD not to use the same list as usbcore. This could
be done by allocating private data, or by adding a new list field to
struct urb.
Alan Stern
Index: usb-2.6/drivers/usb/core/hcd.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/hcd.c
+++ usb-2.6/drivers/usb/core/hcd.c
@@ -1139,8 +1139,15 @@ static int hcd_submit_urb (struct urb *u
else switch (hcd->state) {
case HC_STATE_RUNNING:
case HC_STATE_RESUMING:
+
+ /* Increment urb's reference count in preparation for
+ * giving it to the HCD. The HCD will either return
+ * an error or call giveback(), but not both.
+ */
doit:
usb_get_dev (urb->dev);
+ urb = usb_get_urb (urb);
+ atomic_inc (&urb->use_count);
list_add_tail (&urb->urb_list, &ep->urb_list);
status = 0;
break;
@@ -1164,13 +1171,6 @@ doit:
return status;
}
- /* increment urb's reference count as part of giving it to the HCD
- * (which now controls it). HCD guarantees that it either returns
- * an error or calls giveback(), but not both.
- */
- urb = usb_get_urb (urb);
- atomic_inc (&urb->use_count);
-
if (urb->dev == hcd->self.root_hub) {
/* NOTE: requirement on hub callers (usbfs and the hub
* driver, for now) that URBs' urb->transfer_buffer be
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
