yep, that's the major problem with removing interrupt queueing...but it
should happen at some point...
removing "automagic resubmit", not "removing interrupt queuing" ... ;)
If noone's looked yet, I can look through the current drivers for any that
use interrupt URBs and try to create a patch fixing them all. Of course
it also means that ohci and ehci need to remove their interrupt
resubmission too...
Here's a patch I sent by a while back, likely parts of it still apply.
But again, I'd really rather see that kind of patch be independent of
the rest of these UHCI-only changes.
- Dave
--- ./include/linux/usb.h-dist Tue Aug 27 16:09:36 2002
+++ ./include/linux/usb.h Tue Aug 27 16:27:40 2002
@@ -772,6 +772,5 @@
* request-specific driver context.
* @complete: Completion handler. This URB is passed as the parameter to the
- * completion function. Except for interrupt or isochronous transfers
- * that aren't being unlinked, the completion function may then do what
+ * completion function. The completion function may then do what
* it likes with the URB, including resubmitting or freeing it.
* @iso_frame_desc: Used to provide arrays of ISO transfer buffers and to
@@ -866,9 +865,4 @@
* actual_length field tells how many bytes were transferred.
*
- * For interrupt URBs, the URB provided to the callback
- * function is still "owned" by the USB core subsystem unless the status
- * indicates that the URB has been unlinked. Completion handlers should
- * not modify such URBs until they have been unlinked.
- *
* ISO transfer status is reported in the status and actual_length fields
* of the iso_frame_desc array, and the number of errors is reported in
--- ./drivers/usb-dist/core/urb.c Tue Aug 27 16:15:09 2002
+++ ./drivers/usb/core/urb.c Tue Aug 27 16:27:40 2002
@@ -109,44 +109,44 @@
* Successful submissions return 0; otherwise this routine returns a
* negative error number. If the submission is successful, the complete()
- * fuction of the urb will be called when the USB host driver is
- * finished with the urb (either a successful transmission, or some
- * error case.)
- *
- * Unreserved Bandwidth Transfers:
- *
- * Bulk or control requests complete only once. When the completion
+ * callback from the urb will be called exactly once, when the USB
+ * controller is finished with the urb. When the completion
* function is called, control of the URB is returned to the device
* driver which issued the request. The completion handler may then
* immediately free or reuse that URB.
*
- * Bulk URBs may be queued by submitting an URB to an endpoint before
- * previous ones complete. This can maximize bandwidth utilization by
- * letting the USB controller start work on the next URB without any
- * delay to report completion (scheduling and processing an interrupt)
- * and then submit that next request.
- *
* For control endpoints, the synchronous usb_control_msg() call is
* often used (in non-interrupt context) instead of this call.
+ * That is often used through convenience wrappers, for the requests
+ * that are standardized in the USB 2.0 specification. For bulk
+ * endpoints, a synchronous usb_bulk_msg() call is available.
+ *
+ * Request Queuing:
+ *
+ * URBs may be submitted to endpoints before previous ones complete, to
+ * minimize the impact of interrupt latencies and system overhead on data
+ * throughput. This is required for continuous isochronous data streams,
+ * and may also be required for some kinds of interrupt transfers. Such
+ * queueing also maximizes bandwidth utilization by letting USB controllers
+ * start work on later requests before driver software has finished the
+ * completion processing for earlier requests.
+ *
+ * Bulk and Isochronous URBs may always be queued. At this writing, there
+ * are some controller drivers which do not support queueing control or
+ * interrupt transfer requests.
*
* Reserved Bandwidth Transfers:
*
- * Periodic URBs (interrupt or isochronous) are performed repeatedly.
- *
- * For interrupt requests this is (currently) automagically done
- * until the original request is aborted. When the completion callback
- * indicates the URB has been unlinked (with a special status code),
- * control of that URB returns to the device driver. Otherwise, the
- * completion handler does not control the URB, and should not change
- * any of its fields.
- *
- * For isochronous requests, the completion handler is expected to
- * submit an urb, typically resubmitting its parameter, until drivers
- * stop wanting data transfers. (For example, audio playback might have
- * finished, or a webcam turned off.)
- *
+ * Periodic transfers (interrupt or isochronous) are performed repeatedly,
+ * using bandwidth that is reserved for that purpose.
* If the USB subsystem can't reserve sufficient bandwidth to perform
* the periodic request, and bandwidth reservation is being done for
* this controller, submitting such a periodic request will fail.
*
+ * Device drivers must explicitly request that repetition. It is normally
+ * done by having completion handlers update the URB returned to them, and
+ * then resubmitting it. The resubmission is stopped when device access
+ * is no longer needed, such as when a webcam view is stopped or when audio
+ * playback completes.
+ *
* Memory Flags:
*
@@ -245,7 +245,8 @@
}
- /* periodic transfers limit size per frame/uframe */
- switch (temp) {
- case PIPE_ISOCHRONOUS: {
+ /* isochronous transfers limit size per frame/uframe,
+ * and aren't automatically packetized by HCDs
+ */
+ if (temp == PIPE_ISOCHRONOUS) {
int n, len;
@@ -258,9 +259,4 @@
}
- }
- break;
- case PIPE_INTERRUPT:
- if (urb->transfer_buffer_length > max)
- return -EMSGSIZE;
}
--- ./drivers/usb-dist/core/hcd.c Tue Aug 27 16:15:08 2002
+++ ./drivers/usb/core/hcd.c Tue Aug 27 16:27:40 2002
@@ -496,23 +496,15 @@
urb->actual_length = length;
urb->status = 0;
+ urb->hcpriv = 0;
urb->complete (urb);
+ return;
}
- spin_lock_irqsave (&hcd_data_lock, flags);
- urb->status = -EINPROGRESS;
- if (HCD_IS_RUNNING (hcd->state)
- && rh_status_urb (hcd, urb) != 0) {
- /* another driver snuck in? */
- dbg ("%s, can't resubmit roothub status urb?",
- hcd->self.bus_name);
- spin_unlock_irqrestore (&hcd_data_lock, flags);
- BUG ();
- }
- spin_unlock_irqrestore (&hcd_data_lock, flags);
- } else {
+ } else
spin_unlock_irqrestore (&urb->lock, flags);
- spin_lock_irqsave (&hcd_data_lock, flags);
- rh_status_urb (hcd, urb);
- spin_unlock_irqrestore (&hcd_data_lock, flags);
- }
+
+ /* retrigger timer until completion: success or unlink */
+ spin_lock_irqsave (&hcd_data_lock, flags);
+ rh_status_urb (hcd, urb);
+ spin_unlock_irqrestore (&hcd_data_lock, flags);
} else {
/* this urb's been unlinked */
@@ -1155,5 +1147,8 @@
}
- /* maybe set up to block on completion notification */
+ /* maybe set up to block until the urb's completion fires. the
+ * lower level hcd code is always async, locking on urb->status
+ * updates; an intercepted completion unblocks us.
+ */
if ((urb->transfer_flags & USB_TIMEOUT_KILLED))
urb->status = -ETIMEDOUT;
@@ -1287,9 +1282,4 @@
* the device driver won't cause problems if it frees, modifies,
* or resubmits this URB.
- * Bandwidth and other resources will be deallocated.
- *
- * HCDs must not use this for periodic URBs that are still scheduled
- * and will be reissued. They should just call their completion handlers
- * until the urb is returned to the device driver by unlinking.
*/
void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb)
--- ./drivers/usb-dist/core/hub.c Tue Aug 27 16:15:08 2002
+++ ./drivers/usb/core/hub.c Tue Aug 27 16:27:40 2002
@@ -120,4 +120,5 @@
struct usb_hub *hub = (struct usb_hub *)urb->context;
unsigned long flags;
+ int status;
switch (urb->status) {
@@ -130,7 +131,7 @@
/* Cause a hub reset after 10 consecutive errors */
dbg("hub '%s' status %d for interrupt transfer",
- urb->dev->devpath, urb->status);
+ hub->dev->devpath, urb->status);
if ((++hub->nerrors < 10) || hub->error)
- return;
+ goto resubmit;
hub->error = urb->status;
/* FALL THROUGH */
@@ -150,4 +151,11 @@
}
spin_unlock_irqrestore(&hub_event_lock, flags);
+
+resubmit:
+ urb->dev = hub->dev;
+ if ((status = usb_submit_urb (hub->urb, GFP_ATOMIC)) != 0)
+ err ("hub '%s-%s' status %d for interrupt resubmit",
+ hub->dev->bus->bus_name, hub->dev->devpath,
+ status);
}
--- ./drivers/usb-dist/host/ehci-hcd.c Tue Aug 27 16:15:10 2002
+++ ./drivers/usb/host/ehci-hcd.c Tue Aug 27 16:27:40 2002
@@ -96,6 +96,4 @@
// #define have_split_iso
-#define INTR_AUTOMAGIC /* to be removed later in 2.5 */
-
/* magic numbers that can affect system performance */
#define EHCI_TUNE_CERR 3 /* 0-3 qtd retries; 0 == don't stop */
--- ./drivers/usb-dist/host/ehci-q.c Tue Aug 27 16:15:09 2002
+++ ./drivers/usb/host/ehci-q.c Tue Aug 27 16:27:40 2002
@@ -166,9 +166,4 @@
struct urb *urb
) {
-#ifdef INTR_AUTOMAGIC
- struct urb *resubmit = 0;
- struct usb_device *dev = 0;
-#endif
-
if (likely (urb->hcpriv != 0)) {
struct ehci_qh *qh = (struct ehci_qh *) urb->hcpriv;
@@ -179,12 +174,4 @@
/* ... update hc-wide periodic stats (for usbfs) */
ehci->hcd.self.bandwidth_int_reqs--;
-
-#ifdef INTR_AUTOMAGIC
- if (!((urb->status == -ENOENT)
- || (urb->status == -ECONNRESET))) {
- resubmit = usb_get_urb (urb);
- dev = urb->dev;
- }
-#endif
}
qh_put (ehci, qh);
@@ -201,23 +188,4 @@
usb_hcd_giveback_urb (&ehci->hcd, urb);
-
-#ifdef INTR_AUTOMAGIC
- if (resubmit && ((urb->status == -ENOENT)
- || (urb->status == -ECONNRESET))) {
- usb_put_urb (resubmit);
- resubmit = 0;
- }
- // device drivers will soon be doing something like this
- if (resubmit) {
- int status;
-
- resubmit->dev = dev;
- status = usb_submit_urb (resubmit, SLAB_KERNEL);
- if (status != 0)
- err ("can't resubmit interrupt urb %p: status %d",
- resubmit, status);
- usb_put_urb (resubmit);
- }
-#endif
}
--- ./drivers/usb-dist/host/ohci-q.c Tue Aug 27 16:15:10 2002
+++ ./drivers/usb/host/ohci-q.c Tue Aug 27 16:27:40 2002
@@ -6,5 +6,4 @@
*
* This file is licenced under the GPL.
- * $Id: ohci-q.c,v 1.8 2002/03/27 20:57:01 dbrownell Exp $
*/
@@ -59,45 +58,4 @@
}
-static void td_submit_urb (struct ohci_hcd *ohci, struct urb *urb);
-
-/* Report interrupt transfer completion, maybe reissue */
-static inline void intr_resub (struct ohci_hcd *hc, struct urb *urb)
-{
- struct urb_priv *urb_priv = urb->hcpriv;
- unsigned long flags;
-
-// FIXME going away along with the rest of interrrupt automagic...
-
- /* FIXME: MP race. If another CPU partially unlinks
- * this URB (urb->status was updated, hasn't yet told
- * us to dequeue) before we call complete() here, an
- * extra "unlinked" completion will be reported...
- */
-
- spin_lock_irqsave (&urb->lock, flags);
- if (likely (urb->status == -EINPROGRESS))
- urb->status = 0;
- spin_unlock_irqrestore (&urb->lock, flags);
-
-#ifdef OHCI_VERBOSE_DEBUG
- urb_print (urb, "INTR", usb_pipeout (urb->pipe));
-#endif
- urb->complete (urb);
-
- /* always requeued, but ED_SKIP if complete() unlinks.
- * EDs are removed from periodic table only at SOF intr.
- */
- urb->actual_length = 0;
- spin_lock_irqsave (&urb->lock, flags);
- if (urb_priv->state != URB_DEL)
- urb->status = -EINPROGRESS;
- spin_unlock (&urb->lock);
-
- spin_lock (&hc->lock);
- td_submit_urb (hc, urb);
- spin_unlock_irqrestore (&hc->lock, flags);
-}
-
-
/*-------------------------------------------------------------------------*
* ED handling functions
@@ -982,14 +940,6 @@
*/
if (urb_priv->td_cnt == urb_priv->length) {
- int resubmit;
-
- resubmit = usb_pipeint (urb->pipe)
- && (urb_priv->state != URB_DEL);
-
spin_unlock_irqrestore (&ohci->lock, flags);
- if (resubmit)
- intr_resub (ohci, urb);
- else
- finish_urb (ohci, urb);
+ finish_urb (ohci, urb);
spin_lock_irqsave (&ohci->lock, flags);
}
--- ./drivers/usb-dist/input/hid-core.c Tue Aug 27 16:15:02 2002
+++ ./drivers/usb/input/hid-core.c Tue Aug 27 16:27:40 2002
@@ -916,10 +916,24 @@
static void hid_irq_in(struct urb *urb)
{
- if (urb->status) {
- dbg("nonzero status in input irq %d", urb->status);
+ struct hid_device *hid = urb->context;
+ int status;
+
+ switch (urb->status) {
+ case 0: /* success */
+ hid_input_report(HID_INPUT_REPORT, urb);
+ break;
+ case -ECONNRESET: /* unlink completed */
+ case -ENOENT:
return;
+ default: /* error */
+ dbg("nonzero status in input irq %d", urb->status);
}
-
- hid_input_report(HID_INPUT_REPORT, urb);
+
+ urb->dev = hid->dev;
+ status = usb_submit_urb (urb, SLAB_ATOMIC);
+ if (status)
+ err ("can't resubmit intr, %s-%s/input%d, status %d",
+ hid->dev->bus->bus_name, hid->dev->devpath,
+ hid->ifnum, status);
}
--- ./drivers/usb-dist/input/usbkbd.c Tue Aug 27 16:15:02 2002
+++ ./drivers/usb/input/usbkbd.c Tue Aug 27 16:27:40 2002
@@ -83,5 +83,13 @@
int i;
- if (urb->status) return;
+ switch (urb->status) {
+ case 0: /* success */
+ break;
+ case -ENOENT: /* unlink */
+ case -ECONNRESET:
+ return;
+ default: /* error */
+ goto resubmit;
+ }
for (i = 0; i < 8; i++)
@@ -108,4 +116,11 @@
memcpy(kbd->old, kbd->new, 8);
+
+resubmit:
+ urb->dev = kbd->usbdev;
+ i = usb_submit_urb (urb, SLAB_ATOMIC);
+ if (i)
+ err ("can't resubmit intr, %s-%s/input0, status %d",
+ kbd->usbdev->bus->bus_name, kbd->usbdev->devpath, i);
}
--- ./drivers/usb-dist/net/pegasus.c Tue Aug 27 16:15:03 2002
+++ ./drivers/usb/net/pegasus.c Tue Aug 27 16:27:40 2002
@@ -671,4 +671,5 @@
struct net_device *net;
__u8 *d;
+ int status;
if (!pegasus)
@@ -701,4 +702,10 @@
}
}
+
+ urb->dev = pegasus->usb;
+ status = usb_submit_urb (urb, SLAB_ATOMIC);
+ if (status)
+ err ("%s: can't resubmit interrupt urb, %d",
+ net->name, status);
}
