Commit e9df41c5 authored by Alan Stern's avatar Alan Stern Committed by Greg Kroah-Hartman
Browse files

USB: make HCDs responsible for managing endpoint queues



This patch (as954) implements a suggestion of David Brownell's.  Now
the host controller drivers are responsible for linking and unlinking
URBs to/from their endpoint queues.  This eliminates the possiblity of
strange situations where usbcore thinks an URB is linked but the HCD
thinks it isn't.  It also means HCDs no longer have to check for URBs
being dequeued before they were fully enqueued.

In addition to the core changes, this requires changing every host
controller driver and the root-hub URB handler.  For the most part the
required changes are fairly small; drivers have to call
usb_hcd_link_urb_to_ep() in their urb_enqueue method,
usb_hcd_check_unlink_urb() in their urb_dequeue method, and
usb_hcd_unlink_urb_from_ep() before giving URBs back.  A few HCDs make
matters more complicated by the way they split up the flow of control.

In addition some method interfaces get changed.  The endpoint argument
for urb_enqueue is now redundant so it is removed.  The unlink status
is required by usb_hcd_check_unlink_urb(), so it has been added to
urb_dequeue.
Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
CC: David Brownell <david-b@pacbell.net>
CC: Olav Kongas <ok@artecdesign.ee>
CC: Tony Olech <tony.olech@elandigitalsystems.com>
CC: Yoshihiro Shimoda <shimoda.yoshihiro@renesas.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent b0e396e3
......@@ -356,11 +356,17 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
const u8 *bufp = tbuf;
int len = 0;
int patch_wakeup = 0;
int status = 0;
int status;
int n;
might_sleep();
spin_lock_irq(&hcd_root_hub_lock);
status = usb_hcd_link_urb_to_ep(hcd, urb);
spin_unlock_irq(&hcd_root_hub_lock);
if (status)
return status;
cmd = (struct usb_ctrlrequest *) urb->setup_packet;
typeReq = (cmd->bRequestType << 8) | cmd->bRequest;
wValue = le16_to_cpu (cmd->wValue);
......@@ -525,10 +531,9 @@ error:
/* any errors get returned through the urb completion */
spin_lock_irq(&hcd_root_hub_lock);
spin_lock(&urb->lock);
if (urb->status == -EINPROGRESS)
urb->status = status;
spin_unlock(&urb->lock);
usb_hcd_unlink_urb_from_ep(hcd, urb);
/* This peculiar use of spinlocks echoes what real HC drivers do.
* Avoiding calls to local_irq_disable/enable makes the code
......@@ -571,26 +576,21 @@ void usb_hcd_poll_rh_status(struct usb_hcd *hcd)
spin_lock_irqsave(&hcd_root_hub_lock, flags);
urb = hcd->status_urb;
if (urb) {
spin_lock(&urb->lock);
if (urb->status == -EINPROGRESS) {
hcd->poll_pending = 0;
hcd->status_urb = NULL;
urb->status = 0;
urb->hcpriv = NULL;
urb->actual_length = length;
memcpy(urb->transfer_buffer, buffer, length);
} else /* urb has been unlinked */
length = 0;
spin_unlock(&urb->lock);
hcd->poll_pending = 0;
hcd->status_urb = NULL;
urb->status = 0;
urb->hcpriv = NULL;
urb->actual_length = length;
memcpy(urb->transfer_buffer, buffer, length);
usb_hcd_unlink_urb_from_ep(hcd, urb);
spin_unlock(&hcd_root_hub_lock);
usb_hcd_giveback_urb(hcd, urb);
spin_lock(&hcd_root_hub_lock);
} else
} else {
length = 0;
if (length <= 0)
hcd->poll_pending = 1;
}
spin_unlock_irqrestore(&hcd_root_hub_lock, flags);
}
......@@ -619,24 +619,26 @@ static int rh_queue_status (struct usb_hcd *hcd, struct urb *urb)
int len = 1 + (urb->dev->maxchild / 8);
spin_lock_irqsave (&hcd_root_hub_lock, flags);
if (urb->status != -EINPROGRESS) /* already unlinked */
retval = urb->status;
else if (hcd->status_urb || urb->transfer_buffer_length < len) {
if (hcd->status_urb || urb->transfer_buffer_length < len) {
dev_dbg (hcd->self.controller, "not queuing rh status urb\n");
retval = -EINVAL;
} else {
hcd->status_urb = urb;
urb->hcpriv = hcd; /* indicate it's queued */
goto done;
}
if (!hcd->uses_new_polling)
mod_timer (&hcd->rh_timer,
(jiffies/(HZ/4) + 1) * (HZ/4));
retval = usb_hcd_link_urb_to_ep(hcd, urb);
if (retval)
goto done;
/* If a status change has already occurred, report it ASAP */
else if (hcd->poll_pending)
mod_timer (&hcd->rh_timer, jiffies);
retval = 0;
}
hcd->status_urb = urb;
urb->hcpriv = hcd; /* indicate it's queued */
if (!hcd->uses_new_polling)
mod_timer(&hcd->rh_timer, (jiffies/(HZ/4) + 1) * (HZ/4));
/* If a status change has already occurred, report it ASAP */
else if (hcd->poll_pending)
mod_timer(&hcd->rh_timer, jiffies);
retval = 0;
done:
spin_unlock_irqrestore (&hcd_root_hub_lock, flags);
return retval;
}
......@@ -655,11 +657,16 @@ static int rh_urb_enqueue (struct usb_hcd *hcd, struct urb *urb)
/* Unlinks of root-hub control URBs are legal, but they don't do anything
* since these URBs always execute synchronously.
*/
static int usb_rh_urb_dequeue (struct usb_hcd *hcd, struct urb *urb)
static int usb_rh_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
{
unsigned long flags;
int rc;
spin_lock_irqsave(&hcd_root_hub_lock, flags);
rc = usb_hcd_check_unlink_urb(hcd, urb, status);
if (rc)
goto done;
if (usb_endpoint_num(&urb->ep->desc) == 0) { /* Control URB */
; /* Do nothing */
......@@ -669,14 +676,16 @@ static int usb_rh_urb_dequeue (struct usb_hcd *hcd, struct urb *urb)
if (urb == hcd->status_urb) {
hcd->status_urb = NULL;
urb->hcpriv = NULL;
usb_hcd_unlink_urb_from_ep(hcd, urb);
spin_unlock(&hcd_root_hub_lock);
usb_hcd_giveback_urb(hcd, urb);
spin_lock(&hcd_root_hub_lock);
}
}
done:
spin_unlock_irqrestore(&hcd_root_hub_lock, flags);
return 0;
return rc;
}
......@@ -977,12 +986,26 @@ EXPORT_SYMBOL (usb_calc_bus_time);
/*-------------------------------------------------------------------------*/
static int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb)
/**
* usb_hcd_link_urb_to_ep - add an URB to its endpoint queue
* @hcd: host controller to which @urb was submitted
* @urb: URB being submitted
*
* Host controller drivers should call this routine in their enqueue()
* method. The HCD's private spinlock must be held and interrupts must
* be disabled. The actions carried out here are required for URB
* submission, as well as for endpoint shutdown and for usb_kill_urb.
*
* Returns 0 for no error, otherwise a negative error code (in which case
* the enqueue() method must fail). If no error occurs but enqueue() fails
* anyway, it must call usb_hcd_unlink_urb_from_ep() before releasing
* the private spinlock and returning.
*/
int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb)
{
unsigned long flags;
int rc = 0;
spin_lock_irqsave(&hcd_urb_list_lock, flags);
spin_lock(&hcd_urb_list_lock);
/* Check that the URB isn't being killed */
if (unlikely(urb->reject)) {
......@@ -1009,48 +1032,48 @@ static int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb)
goto done;
}
done:
spin_unlock_irqrestore(&hcd_urb_list_lock, flags);
spin_unlock(&hcd_urb_list_lock);
return rc;
}
EXPORT_SYMBOL_GPL(usb_hcd_link_urb_to_ep);
static int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
/**
* usb_hcd_check_unlink_urb - check whether an URB may be unlinked
* @hcd: host controller to which @urb was submitted
* @urb: URB being checked for unlinkability
* @status: error code to store in @urb if the unlink succeeds
*
* Host controller drivers should call this routine in their dequeue()
* method. The HCD's private spinlock must be held and interrupts must
* be disabled. The actions carried out here are required for making
* sure than an unlink is valid.
*
* Returns 0 for no error, otherwise a negative error code (in which case
* the dequeue() method must fail). The possible error codes are:
*
* -EIDRM: @urb was not submitted or has already completed.
* The completion function may not have been called yet.
*
* -EBUSY: @urb has already been unlinked.
*/
int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
int status)
{
unsigned long flags;
struct list_head *tmp;
int rc = 0;
/*
* we contend for urb->status with the hcd core,
* which changes it while returning the urb.
*
* Caller guaranteed that the urb pointer hasn't been freed, and
* that it was submitted. But as a rule it can't know whether or
* not it's already been unlinked ... so we respect the reversed
* lock sequence needed for the usb_hcd_giveback_urb() code paths
* (urb lock, then hcd_urb_list_lock) in case some other CPU is now
* unlinking it.
*/
spin_lock_irqsave(&urb->lock, flags);
spin_lock(&hcd_urb_list_lock);
/* insist the urb is still queued */
list_for_each(tmp, &urb->ep->urb_list) {
if (tmp == &urb->urb_list)
break;
}
if (tmp != &urb->urb_list) {
rc = -EIDRM;
goto done;
}
if (tmp != &urb->urb_list)
return -EIDRM;
/* Any status except -EINPROGRESS means something already started to
* unlink this URB from the hardware. So there's no more work to do.
*/
if (urb->status != -EINPROGRESS) {
rc = -EBUSY;
goto done;
}
if (urb->status != -EINPROGRESS)
return -EBUSY;
urb->status = status;
/* IRQ setup can easily be broken so that USB controllers
......@@ -1065,21 +1088,28 @@ static int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
}
done:
spin_unlock(&hcd_urb_list_lock);
spin_unlock_irqrestore (&urb->lock, flags);
return rc;
return 0;
}
EXPORT_SYMBOL_GPL(usb_hcd_check_unlink_urb);
static void usb_hcd_unlink_urb_from_ep(struct usb_hcd *hcd, struct urb *urb)
/**
* usb_hcd_unlink_urb_from_ep - remove an URB from its endpoint queue
* @hcd: host controller to which @urb was submitted
* @urb: URB being unlinked
*
* Host controller drivers should call this routine before calling
* usb_hcd_giveback_urb(). The HCD's private spinlock must be held and
* interrupts must be disabled. The actions carried out here are required
* for URB completion.
*/
void usb_hcd_unlink_urb_from_ep(struct usb_hcd *hcd, struct urb *urb)
{
unsigned long flags;
/* clear all state linking urb to this dev (and hcd) */
spin_lock_irqsave(&hcd_urb_list_lock, flags);
spin_lock(&hcd_urb_list_lock);
list_del_init(&urb->urb_list);
spin_unlock_irqrestore(&hcd_urb_list_lock, flags);
spin_unlock(&hcd_urb_list_lock);
}
EXPORT_SYMBOL_GPL(usb_hcd_unlink_urb_from_ep);
static void map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
{
......@@ -1153,20 +1183,15 @@ int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags)
* URBs must be submitted in process context with interrupts
* enabled.
*/
status = usb_hcd_link_urb_to_ep(hcd, urb);
if (!status) {
map_urb_for_dma(hcd, urb);
if (is_root_hub(urb->dev))
status = rh_urb_enqueue(hcd, urb);
else
status = hcd->driver->urb_enqueue(hcd, urb->ep, urb,
mem_flags);
}
map_urb_for_dma(hcd, urb);
if (is_root_hub(urb->dev))
status = rh_urb_enqueue(hcd, urb);
else
status = hcd->driver->urb_enqueue(hcd, urb, mem_flags);
if (unlikely(status)) {
usbmon_urb_submit_error(&hcd->self, urb, status);
unmap_urb_for_dma(hcd, urb);
usb_hcd_unlink_urb_from_ep(hcd, urb);
INIT_LIST_HEAD(&urb->urb_list);
atomic_dec(&urb->use_count);
if (urb->reject)
......@@ -1183,24 +1208,19 @@ int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags)
* soon as practical. we've already set up the urb's return status,
* but we can't know if the callback completed already.
*/
static int
unlink1 (struct usb_hcd *hcd, struct urb *urb)
static int unlink1(struct usb_hcd *hcd, struct urb *urb, int status)
{
int value;
if (is_root_hub(urb->dev))
value = usb_rh_urb_dequeue (hcd, urb);
value = usb_rh_urb_dequeue(hcd, urb, status);
else {
/* The only reason an HCD might fail this call is if
* it has not yet fully queued the urb to begin with.
* Such failures should be harmless. */
value = hcd->driver->urb_dequeue (hcd, urb);
value = hcd->driver->urb_dequeue(hcd, urb, status);
}
if (value != 0)
dev_dbg (hcd->self.controller, "dequeue %p --> %d\n",
urb, value);
return value;
}
......@@ -1216,14 +1236,11 @@ int usb_hcd_unlink_urb (struct urb *urb, int status)
int retval;
hcd = bus_to_hcd(urb->dev->bus);
retval = usb_hcd_check_unlink_urb(hcd, urb, status);
if (!retval)
retval = unlink1(hcd, urb);
retval = unlink1(hcd, urb, status);
if (retval == 0)
retval = -EINPROGRESS;
else if (retval != -EIDRM)
else if (retval != -EIDRM && retval != -EBUSY)
dev_dbg(&urb->dev->dev, "hcd_unlink_urb %p fail %d\n",
urb, retval);
return retval;
......@@ -1245,7 +1262,6 @@ int usb_hcd_unlink_urb (struct urb *urb, int status)
*/
void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb)
{
usb_hcd_unlink_urb_from_ep(hcd, urb);
unmap_urb_for_dma(hcd, urb);
usbmon_urb_complete (&hcd->self, urb);
usb_unanchor_urb(urb);
......@@ -1282,7 +1298,6 @@ void usb_hcd_endpoint_disable (struct usb_device *udev,
rescan:
spin_lock_irq(&hcd_urb_list_lock);
list_for_each_entry (urb, &ep->urb_list, urb_list) {
int tmp;
int is_in;
/* the urb may already have been unlinked */
......@@ -1292,34 +1307,26 @@ rescan:
is_in = usb_urb_dir_in(urb);
spin_unlock(&hcd_urb_list_lock);
spin_lock (&urb->lock);
tmp = urb->status;
if (tmp == -EINPROGRESS)
urb->status = -ESHUTDOWN;
spin_unlock (&urb->lock);
/* kick hcd unless it's already returning this */
if (tmp == -EINPROGRESS) {
unlink1 (hcd, urb);
dev_dbg (hcd->self.controller,
"shutdown urb %p ep%d%s%s\n",
urb, usb_endpoint_num(&ep->desc),
is_in ? "in" : "out",
({ char *s;
switch (usb_endpoint_type(&ep->desc)) {
case USB_ENDPOINT_XFER_CONTROL:
s = ""; break;
case USB_ENDPOINT_XFER_BULK:
s = "-bulk"; break;
case USB_ENDPOINT_XFER_INT:
s = "-intr"; break;
default:
s = "-iso"; break;
};
s;
}));
}
/* kick hcd */
unlink1(hcd, urb, -ESHUTDOWN);
dev_dbg (hcd->self.controller,
"shutdown urb %p ep%d%s%s\n",
urb, usb_endpoint_num(&ep->desc),
is_in ? "in" : "out",
({ char *s;
switch (usb_endpoint_type(&ep->desc)) {
case USB_ENDPOINT_XFER_CONTROL:
s = ""; break;
case USB_ENDPOINT_XFER_BULK:
s = "-bulk"; break;
case USB_ENDPOINT_XFER_INT:
s = "-intr"; break;
default:
s = "-iso"; break;
};
s;
}));
usb_put_urb (urb);
/* list contents may have changed */
......
......@@ -189,11 +189,10 @@ struct hc_driver {
int (*get_frame_number) (struct usb_hcd *hcd);
/* manage i/o requests, device state */
int (*urb_enqueue) (struct usb_hcd *hcd,
struct usb_host_endpoint *ep,
struct urb *urb,
gfp_t mem_flags);
int (*urb_dequeue) (struct usb_hcd *hcd, struct urb *urb);
int (*urb_enqueue)(struct usb_hcd *hcd,
struct urb *urb, gfp_t mem_flags);
int (*urb_dequeue)(struct usb_hcd *hcd,
struct urb *urb, int status);
/* hw synch, freeing endpoint resources that urb_dequeue can't */
void (*endpoint_disable)(struct usb_hcd *hcd,
......@@ -211,6 +210,11 @@ struct hc_driver {
/* Needed only if port-change IRQs are level-triggered */
};
extern int usb_hcd_link_urb_to_ep(struct usb_hcd *hcd, struct urb *urb);
extern int usb_hcd_check_unlink_urb(struct usb_hcd *hcd, struct urb *urb,
int status);
extern void usb_hcd_unlink_urb_from_ep(struct usb_hcd *hcd, struct urb *urb);
extern int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags);
extern int usb_hcd_unlink_urb (struct urb *urb, int status);
extern void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb);
......
......@@ -962,13 +962,13 @@ static struct platform_driver dummy_udc_driver = {
static int dummy_urb_enqueue (
struct usb_hcd *hcd,
struct usb_host_endpoint *ep,
struct urb *urb,
gfp_t mem_flags
) {
struct dummy *dum;
struct urbp *urbp;
unsigned long flags;
int rc;
if (!urb->transfer_buffer && urb->transfer_buffer_length)
return -EINVAL;
......@@ -980,6 +980,11 @@ static int dummy_urb_enqueue (
dum = hcd_to_dummy (hcd);
spin_lock_irqsave (&dum->lock, flags);
rc = usb_hcd_link_urb_to_ep(hcd, urb);
if (rc) {
kfree(urbp);
goto done;
}
if (!dum->udev) {
dum->udev = urb->dev;
......@@ -997,22 +1002,28 @@ static int dummy_urb_enqueue (
mod_timer (&dum->timer, jiffies + 1);
spin_unlock_irqrestore (&dum->lock, flags);
return 0;
done:
return rc;
}
static int dummy_urb_dequeue (struct usb_hcd *hcd, struct urb *urb)
static int dummy_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
{
struct dummy *dum;
unsigned long flags;
int rc;
/* giveback happens automatically in timer callback,
* so make sure the callback happens */
dum = hcd_to_dummy (hcd);
spin_lock_irqsave (&dum->lock, flags);
if (dum->rh_state != DUMMY_RH_RUNNING && !list_empty(&dum->urbp_list))
rc = usb_hcd_check_unlink_urb(hcd, urb, status);
if (!rc && dum->rh_state != DUMMY_RH_RUNNING &&
!list_empty(&dum->urbp_list))
mod_timer (&dum->timer, jiffies);
spin_unlock_irqrestore (&dum->lock, flags);
return 0;
return rc;
}
static void maybe_set_status (struct urb *urb, int status)
......@@ -1511,6 +1522,7 @@ return_urb:
if (ep)
ep->already_seen = ep->setup_stage = 0;
usb_hcd_unlink_urb_from_ep(dummy_to_hcd(dum), urb);
spin_unlock (&dum->lock);
usb_hcd_giveback_urb (dummy_to_hcd(dum), urb);
spin_lock (&dum->lock);
......
......@@ -719,7 +719,6 @@ dead:
*/
static int ehci_urb_enqueue (
struct usb_hcd *hcd,
struct usb_host_endpoint *ep,
struct urb *urb,
gfp_t mem_flags
) {
......@@ -734,12 +733,12 @@ static int ehci_urb_enqueue (
default:
if (!qh_urb_transaction (ehci, urb, &qtd_list, mem_flags))
return -ENOMEM;
return submit_async (ehci, ep, urb, &qtd_list, mem_flags);
return submit_async(ehci, urb, &qtd_list, mem_flags);
case PIPE_INTERRUPT:
if (!qh_urb_transaction (ehci, urb, &qtd_list, mem_flags))
return -ENOMEM;
return intr_submit (ehci, ep, urb, &qtd_list, mem_flags);
return intr_submit(ehci, urb, &qtd_list, mem_flags);
case PIPE_ISOCHRONOUS:
if (urb->dev->speed == USB_SPEED_HIGH)
......@@ -777,13 +776,18 @@ static void unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
* completions normally happen asynchronously
*/
static int ehci_urb_dequeue (struct usb_hcd *hcd, struct urb *urb)
static int ehci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
{
struct ehci_hcd *ehci = hcd_to_ehci (hcd);
struct ehci_qh *qh;
unsigned long flags;
int rc;
spin_lock_irqsave (&ehci->lock, flags);
rc = usb_hcd_check_unlink_urb(hcd, urb, status);
if (rc)
goto done;
switch (usb_pipetype (urb->pipe)) {
// case PIPE_CONTROL:
// case PIPE_BULK:
......@@ -838,7 +842,7 @@ static int ehci_urb_dequeue (struct usb_hcd *hcd, struct urb *urb)
}
done:
spin_unlock_irqrestore (&ehci->lock, flags);
return 0;
return rc;
}
/*-------------------------------------------------------------------------*/
......
......@@ -262,6 +262,7 @@ __acquires(ehci->lock)
#endif
/* complete() can reenter this HCD */
usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
spin_unlock (&ehci->lock);
usb_hcd_giveback_urb (ehci_to_hcd(ehci), urb);
spin_lock (&ehci->lock);
......@@ -913,7 +914,6 @@ static struct ehci_qh *qh_append_tds (
static int
submit_async (
struct ehci_hcd *ehci,
struct usb_host_endpoint *ep,
struct urb *urb,
struct list_head *qtd_list,
gfp_t mem_flags
......@@ -922,10 +922,10 @@ submit_async (
int epnum;
unsigned long flags;
struct ehci_qh *qh = NULL;
int rc = 0;
int rc;
qtd = list_entry (qtd_list->next, struct ehci_qtd, qtd_list);
epnum = ep->desc.bEndpointAddress;
epnum = urb->ep->desc.bEndpointAddress;
#ifdef EHCI_URB_TRACE
ehci_dbg (ehci,
......@@ -933,7 +933,7 @@ submit_async (
__FUNCTION__, urb->dev->devpath, urb,
epnum & 0x0f, (epnum & USB_DIR_IN) ? "in" : "out",
urb->transfer_buffer_length,
qtd, ep->hcpriv);
qtd, urb->ep->hcpriv);
#endif
spin_lock_irqsave (&ehci->lock, flags);
......@@ -942,9 +942,13 @@ submit_async (
rc = -ESHUTDOWN;
goto done;
}
rc = usb_hcd_link_urb_to_ep(ehci_to_hcd(ehci), urb);
if (unlikely(rc))
goto done;
qh = qh_append_tds (ehci, urb, qtd_list, epnum, &ep->hcpriv);
qh = qh_append_tds(ehci, urb, qtd_list, epnum, &urb->ep->hcpriv);