Hi Greg,
Here's an EHCI update, I'll send separate patches to sync 2.4 with
this version. Changes in this version include:
- An earlier locking update would give trouble on SPARC, where
irqsave "flags" aren't flags. This resolves that issue by
adding a module parameter to limit work done with irqs off.
(Some net drivers do the same thing.)
- Optionally (now #ifdef DEBUG) collects some statistics on IRQs
and URBs. There are more IAA interrupts than I want to see,
during extended usb-storage loading.
- Adds a commented-out workaround for a problem I've seen on one
VT8235. Seems likely an issue with this specific motherboard;
another tester hasn't reported such issues.
- Includes the jiffies time_after() patch from Tim Schmielau.
- Minor tweaks to the hcd portability (get rid of another #if).
- Minor doc/diagnostic/... updates
Please merge to Linus' latest.
- Dave
--- ./drivers/usb-dist/host/ehci.h Sun Sep 15 19:57:44 2002
+++ ./drivers/usb/host/ehci.h Mon Sep 23 10:22:33 2002
@@ -22,4 +22,21 @@
/* definitions used for the EHCI driver */
+/* statistics can be kept for for tuning/monitoring */
+struct ehci_stats {
+ /* irq usage */
+ unsigned long normal;
+ unsigned long error;
+ unsigned long reclaim;
+
+ /* termination of urbs from core */
+ unsigned long complete;
+ unsigned long unlink;
+
+ /* qhs patched to recover from td queueing race
+ * (can avoid by using 'dummy td', allowing fewer irqs)
+ */
+ unsigned long qpatch;
+};
+
/* ehci_hcd->lock guards shared data against other CPUs:
* ehci_hcd: async, reclaim, periodic (and shadow), ...
@@ -73,4 +90,11 @@
struct timer_list watchdog;
+
+#ifdef EHCI_STATS
+ struct ehci_stats stats;
+# define COUNT(x) do { (x)++; } while (0)
+#else
+# define COUNT(x) do {} while (0)
+#endif
};
@@ -401,8 +425,20 @@
#define STUB_DEBUG_FILES
+static inline int hcd_register_root (struct usb_hcd *hcd)
+{
+ return usb_new_device (hcd_to_bus (hcd)->root_hub);
+}
+
#else /* LINUX_VERSION_CODE */
+// hcd_to_bus() eventually moves to hcd.h on 2.5 too
static inline struct usb_bus *hcd_to_bus (struct usb_hcd *hcd)
{ return &hcd->self; }
+// ... as does hcd_register_root()
+static inline int hcd_register_root (struct usb_hcd *hcd)
+{
+ return usb_register_root_hub (
+ hcd_to_bus (hcd)->root_hub, &hcd->pdev->dev);
+}
#define SUBMIT_URB(urb,mem_flags) usb_submit_urb(urb,mem_flags)
--- ./drivers/usb-dist/host/ehci-hcd.c Sun Sep 15 19:57:44 2002
+++ ./drivers/usb/host/ehci-hcd.c Mon Sep 23 10:22:33 2002
@@ -64,6 +64,5 @@
* Next comes "CardBay", using USB 2.0 signals.
*
- * Contains additional contributions by: Brad Hards, Rory Bolt, ...
- *
+ * Contains additional contributions by Brad Hards, Rory Bolt, and others.
* Special thanks to Intel and VIA for providing host controllers to
* test this driver on, and Cypress (including In-System Design) for
@@ -94,12 +93,18 @@
*/
-#define DRIVER_VERSION "2002-Aug-28"
+#define DRIVER_VERSION "2002-Sep-23"
#define DRIVER_AUTHOR "David Brownell"
#define DRIVER_DESC "USB 2.0 'Enhanced' Host Controller (EHCI) Driver"
+static const char hcd_name [] = "ehci-hcd";
+
// #define EHCI_VERBOSE_DEBUG
// #define have_split_iso
+#ifdef DEBUG
+#define EHCI_STATS
+#endif
+
#define INTR_AUTOMAGIC /* to be removed later in 2.5 */
@@ -119,4 +124,10 @@
MODULE_PARM_DESC (log2_irq_thresh, "log2 IRQ latency, 1-64 microframes");
+/* allow irqs at least every N URB completions */
+static int max_completions = 16;
+MODULE_PARM (max_completions, "i");
+MODULE_PARM_DESC (max_completions,
+ "limit for urb completions called with irqs disenabled");
+
#define INTR_MASK (STS_IAA | STS_FATAL | STS_ERR | STS_INT)
@@ -427,9 +438,8 @@
pci_read_config_byte (hcd->pdev, 0x60, &tempbyte);
temp = readw (&ehci->caps->hci_version);
- info ("USB %x.%x support enabled, EHCI rev %x.%02x",
- ((tempbyte & 0xf0)>>4),
- (tempbyte & 0x0f),
- temp >> 8,
- temp & 0xff);
+ info ("USB %x.%x support enabled, EHCI rev %x.%02x, %s %s",
+ ((tempbyte & 0xf0)>>4), (tempbyte & 0x0f),
+ temp >> 8, temp & 0xff,
+ hcd_name, DRIVER_VERSION);
/*
@@ -442,9 +452,5 @@
usb_connect (udev);
udev->speed = USB_SPEED_HIGH;
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,32)
- if (usb_new_device (udev) != 0) {
-#else
- if (usb_register_root_hub (udev, &ehci->hcd.pdev->dev) != 0) {
-#endif
+ if (hcd_register_root (hcd) != 0) {
if (hcd->state == USB_STATE_RUNNING)
ehci_ready (ehci);
@@ -488,4 +494,11 @@
ehci_mem_cleanup (ehci);
+#ifdef EHCI_STATS
+ dbg ("irq normal %ld err %ld reclaim %ld",
+ ehci->stats.normal, ehci->stats.error, ehci->stats.reclaim);
+ dbg ("complete %ld unlink %ld qpatch %ld",
+ ehci->stats.complete, ehci->stats.unlink, ehci->stats.qpatch);
+#endif
+
dbg_status (ehci, "ehci_stop completed", readl (&ehci->regs->status));
}
@@ -592,19 +605,14 @@
{
struct ehci_hcd *ehci = (struct ehci_hcd *) param;
- unsigned long flags;
- // FIXME don't pass flags; on sparc they aren't really flags.
- // qh_completions can just leave irqs blocked,
- // then have scan_async() allow IRQs if it's very busy
-
- spin_lock_irqsave (&ehci->lock, flags);
+ spin_lock_irq (&ehci->lock);
if (ehci->reclaim_ready)
- flags = end_unlink_async (ehci, flags);
- flags = scan_async (ehci, flags);
+ end_unlink_async (ehci);
+ scan_async (ehci);
if (ehci->next_uframe != -1)
- flags = scan_periodic (ehci, flags);
+ scan_periodic (ehci);
- spin_unlock_irqrestore (&ehci->lock, flags);
+ spin_unlock_irq (&ehci->lock);
}
@@ -640,9 +648,15 @@
/* normal [4.15.1.2] or error [4.15.1.1] completion */
- if (likely ((status & (STS_INT|STS_ERR)) != 0))
+ if (likely ((status & (STS_INT|STS_ERR)) != 0)) {
+ if (likely ((status & STS_ERR) == 0))
+ COUNT (ehci->stats.normal);
+ else
+ COUNT (ehci->stats.error);
bh = 1;
+ }
/* complete the unlinking of some qh [4.15.2.3] */
if (status & STS_IAA) {
+ COUNT (ehci->stats.reclaim);
ehci->reclaim_ready = 1;
bh = 1;
@@ -766,8 +780,8 @@
if (qh->qh_state == QH_STATE_LINKED) {
/* messy, can spin or block a microframe ... */
- flags = intr_deschedule (ehci, qh, 1, flags);
+ intr_deschedule (ehci, qh, 1);
/* qh_state == IDLE */
}
- flags = qh_completions (ehci, qh, flags);
+ qh_completions (ehci, qh);
/* reschedule QH iff another request is queued */
@@ -882,6 +896,4 @@
/*-------------------------------------------------------------------------*/
-static const char hcd_name [] = "ehci-hcd";
-
static const struct hc_driver ehci_driver = {
.description = hcd_name,
--- ./drivers/usb-dist/host/ehci-q.c Sun Sep 15 19:57:44 2002
+++ ./drivers/usb/host/ehci-q.c Mon Sep 23 10:23:26 2002
@@ -159,14 +159,11 @@
}
-/* urb->lock ignored from here on (hcd is done with urb) */
-
-static unsigned long ehci_urb_done (
- struct ehci_hcd *ehci,
- struct urb *urb,
- unsigned long flags
-) {
+static void ehci_urb_done (struct ehci_hcd *ehci, struct urb *urb)
+{
#ifdef INTR_AUTOMAGIC
struct urb *resubmit = 0;
struct usb_device *dev = 0;
+
+ static int ehci_urb_enqueue (struct usb_hcd *, struct urb *, int);
#endif
@@ -200,6 +197,13 @@
}
+ if (likely (urb->status == 0))
+ COUNT (ehci->stats.complete);
+ else if (urb->status == -ECONNRESET || urb->status == -ENOENT)
+ COUNT (ehci->stats.unlink);
+ else
+ COUNT (ehci->stats.error);
+
/* complete() can reenter this HCD */
- spin_unlock_irqrestore (&ehci->lock, flags);
+ spin_unlock (&ehci->lock);
usb_hcd_giveback_urb (&ehci->hcd, urb);
@@ -223,6 +227,5 @@
#endif
- spin_lock_irqsave (&ehci->lock, flags);
- return flags;
+ spin_lock (&ehci->lock);
}
@@ -230,15 +233,17 @@
/*
* Process and free completed qtds for a qh, returning URBs to drivers.
- * Chases up to qh->hw_current, returns irqsave flags (maybe modified).
+ * Chases up to qh->hw_current. Returns number of completions called,
+ * indicating how much "real" work we did.
*/
-static unsigned long
-qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh, unsigned long flags)
+static unsigned
+qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
{
struct ehci_qtd *qtd, *last;
struct list_head *next, *qtd_list = &qh->qtd_list;
int unlink = 0, halted = 0;
+ unsigned count = 0;
if (unlikely (list_empty (qtd_list)))
- return flags;
+ return count;
/* scan QTDs till end of list, or we reach an active one */
@@ -253,6 +258,8 @@
/* clean up any state from previous QTD ...*/
if (last) {
- if (likely (last->urb != urb))
- flags = ehci_urb_done (ehci, last->urb, flags);
+ if (likely (last->urb != urb)) {
+ ehci_urb_done (ehci, last->urb);
+ count++;
+ }
/* qh overlays can have HC's old cached copies of
@@ -263,4 +270,5 @@
qh->hw_alt_next = last->hw_alt_next;
qh->hw_qtd_next = last->hw_next;
+ COUNT (ehci->stats.qpatch);
}
@@ -348,5 +356,6 @@
/* last urb's completion might still need calling */
if (likely (last != 0)) {
- flags = ehci_urb_done (ehci, last->urb, flags);
+ ehci_urb_done (ehci, last->urb);
+ count++;
ehci_qtd_free (ehci, last);
}
@@ -358,5 +367,5 @@
}
- return flags;
+ return count;
}
@@ -891,6 +900,5 @@
/* caller must not own ehci->lock */
-static unsigned long
-end_unlink_async (struct ehci_hcd *ehci, unsigned long flags)
+static void end_unlink_async (struct ehci_hcd *ehci)
{
struct ehci_qh *qh = ehci->reclaim;
@@ -904,5 +912,5 @@
ehci->reclaim_ready = 0;
- flags = qh_completions (ehci, qh, flags);
+ qh_completions (ehci, qh);
if (!list_empty (&qh->qtd_list)
@@ -911,9 +919,6 @@
else
qh_put (ehci, qh); // refcount from async list
-
- return flags;
}
-
/* makes sure the async qh will become idle */
/* caller must own ehci->lock */
@@ -945,10 +950,12 @@
/* can't get here without STS_ASS set */
if (ehci->hcd.state != USB_STATE_HALT) {
- if (cmd & CMD_PSE) {
- writel (cmd & ~CMD_ASE, &ehci->regs->command);
- (void) handshake (&ehci->regs->status,
- STS_ASS, 0, 150);
- } else
- ehci_ready (ehci);
+ writel (cmd & ~CMD_ASE, &ehci->regs->command);
+ (void) handshake (&ehci->regs->status, STS_ASS, 0, 150);
+#if 0
+ // one VT8235 system wants to die with STS_FATAL
+ // unless this qh is leaked here. others seem ok...
+ qh = qh_get (qh);
+ dbg_qh ("async/off", ehci, qh);
+#endif
}
qh->qh_next.qh = ehci->async = 0;
@@ -987,11 +994,13 @@
/*-------------------------------------------------------------------------*/
-static unsigned long
-scan_async (struct ehci_hcd *ehci, unsigned long flags)
+static void
+scan_async (struct ehci_hcd *ehci)
{
struct ehci_qh *qh;
+ unsigned count;
rescan:
qh = ehci->async;
+ count = 0;
if (likely (qh != 0)) {
do {
@@ -1002,5 +1011,5 @@
/* concurrent unlink could happen here */
- flags = qh_completions (ehci, qh, flags);
+ count += qh_completions (ehci, qh);
qh_put (ehci, qh);
}
@@ -1009,4 +1018,7 @@
* well as HCD schedule-scanning costs. removing
* the last qh is deferred, since it's costly.
+ *
+ * FIXME don't unlink idle entries so quickly; it
+ * can penalize (common) half duplex protocols.
*/
if (list_empty (&qh->qtd_list) && !ehci->reclaim) {
@@ -1021,4 +1033,13 @@
}
}
+
+ /* keep latencies down: let any irqs in */
+ if (count > max_completions) {
+ spin_unlock_irq (&ehci->lock);
+ cpu_relax ();
+ spin_lock_irq (&ehci->lock);
+ goto rescan;
+ }
+
qh = qh->qh_next.qh;
if (!qh) /* unlinked? */
@@ -1026,4 +1047,3 @@
} while (qh != ehci->async);
}
- return flags;
}
--- ./drivers/usb-dist/host/ehci-sched.c Sun Sep 15 19:57:44 2002
+++ ./drivers/usb/host/ehci-sched.c Mon Sep 23 10:22:33 2002
@@ -223,9 +223,8 @@
// FIXME microframe periods not yet handled
-static unsigned long intr_deschedule (
+static void intr_deschedule (
struct ehci_hcd *ehci,
struct ehci_qh *qh,
- int wait,
- unsigned long flags
+ int wait
) {
int status;
@@ -257,8 +256,6 @@
if (((ehci_get_frame (&ehci->hcd) - frame) % qh->period) == 0) {
if (wait) {
- spin_unlock_irqrestore (&ehci->lock, flags);
udelay (125);
qh->hw_next = EHCI_LIST_END;
- spin_lock_irqsave (&ehci->lock, flags);
} else {
/* we may not be IDLE yet, but if the qh is empty
@@ -277,8 +274,7 @@
(qh->usecs + qh->c_usecs) / qh->period;
- vdbg ("descheduled qh %p, per = %d frame = %d count = %d, urbs = %d",
+ dbg ("descheduled qh %p, period = %d frame = %d count = %d, urbs = %d",
qh, qh->period, frame,
atomic_read (&qh->refcount), ehci->periodic_sched);
- return flags;
}
@@ -415,5 +411,5 @@
/* stuff into the periodic schedule */
qh->qh_state = QH_STATE_LINKED;
- dbg ("qh %p usecs %d/%d period %d.0 starting %d.%d (gap %d)",
+ dbg ("scheduled qh %p usecs %d/%d period %d.0 starting %d.%d (gap %d)",
qh, qh->usecs, qh->c_usecs,
qh->period, frame, uframe, qh->gap_uf);
@@ -496,27 +492,28 @@
}
-static unsigned long
+static unsigned
intr_complete (
struct ehci_hcd *ehci,
unsigned frame,
- struct ehci_qh *qh,
- unsigned long flags /* caller owns ehci->lock ... */
+ struct ehci_qh *qh
) {
+ unsigned count;
+
/* nothing to report? */
if (likely ((qh->hw_token & __constant_cpu_to_le32 (QTD_STS_ACTIVE))
!= 0))
- return flags;
+ return 0;
if (unlikely (list_empty (&qh->qtd_list))) {
dbg ("intr qh %p no TDs?", qh);
- return flags;
+ return 0;
}
/* handle any completions */
- flags = qh_completions (ehci, qh, flags);
+ count = qh_completions (ehci, qh);
if (unlikely (list_empty (&qh->qtd_list)))
- flags = intr_deschedule (ehci, qh, 0, flags);
+ intr_deschedule (ehci, qh, 0);
- return flags;
+ return count;
}
@@ -867,10 +864,9 @@
#define ISO_ERRS (EHCI_ISOC_BUF_ERR | EHCI_ISOC_BABBLE | EHCI_ISOC_XACTERR)
-static unsigned long
+static unsigned
itd_complete (
struct ehci_hcd *ehci,
struct ehci_itd *itd,
- unsigned uframe,
- unsigned long flags
+ unsigned uframe
) {
struct urb *urb = itd->urb;
@@ -910,5 +906,5 @@
/* handle completion now? */
if ((itd->index + 1) != urb->number_of_packets)
- return flags;
+ return 0;
/*
@@ -925,7 +921,8 @@
urb->status = 0;
- spin_unlock_irqrestore (&ehci->lock, flags);
+ /* complete() can reenter this HCD */
+ spin_unlock (&ehci->lock);
usb_hcd_giveback_urb (&ehci->hcd, urb);
- spin_lock_irqsave (&ehci->lock, flags);
+ spin_lock (&ehci->lock);
/* defer stopping schedule; completion can submit */
@@ -934,5 +931,5 @@
(void) disable_periodic (ehci);
- return flags;
+ return 1;
}
@@ -946,8 +943,4 @@
dbg ("itd_submit urb %p", urb);
- /* NOTE DMA mapping assumes this ... */
- if (urb->iso_frame_desc [0].offset != 0)
- return -EINVAL;
-
/* allocate ITDs w/o locking anything */
status = itd_urb_transaction (ehci, urb, mem_flags);
@@ -980,8 +973,9 @@
/*-------------------------------------------------------------------------*/
-static unsigned long
-scan_periodic (struct ehci_hcd *ehci, unsigned long flags)
+static void
+scan_periodic (struct ehci_hcd *ehci)
{
unsigned frame, clock, now_uframe, mod;
+ unsigned count = 0;
mod = ehci->periodic_size << 3;
@@ -1006,4 +1000,12 @@
unsigned uframes;
+ /* keep latencies down: let any irqs in */
+ if (count > max_completions) {
+ spin_unlock_irq (&ehci->lock);
+ cpu_relax ();
+ count = 0;
+ spin_lock_irq (&ehci->lock);
+ }
+
restart:
/* scan schedule to _before_ current frame index */
@@ -1029,6 +1031,6 @@
temp = q.qh->qh_next;
type = Q_NEXT_TYPE (q.qh->hw_next);
- flags = intr_complete (ehci, frame,
- qh_get (q.qh), flags);
+ count += intr_complete (ehci, frame,
+ qh_get (q.qh));
qh_put (ehci, q.qh);
q = temp;
@@ -1062,6 +1064,6 @@
/* might free q.itd ... */
- flags = itd_complete (ehci,
- temp.itd, uf, flags);
+ count += itd_complete (ehci,
+ temp.itd, uf);
break;
}
@@ -1079,5 +1081,5 @@
case Q_TYPE_SITD:
last = (q.sitd->hw_next == EHCI_LIST_END);
- flags = sitd_complete (ehci, q.sitd, flags);
+ sitd_complete (ehci, q.sitd);
type = Q_NEXT_TYPE (q.sitd->hw_next);
@@ -1125,4 +1127,3 @@
frame = (frame + 1) % ehci->periodic_size;
}
- return flags;
}
--- ./drivers/usb-dist/host/ehci-dbg.c Sun Sep 15 19:57:44 2002
+++ ./drivers/usb/host/ehci-dbg.c Mon Sep 23 10:22:33 2002
@@ -80,5 +80,5 @@
if (HCC_EXT_CAPS (params)) {
// EHCI 0.96 ... could interpret these (legacy?)
- dbg ("%s extended capabilities at pci %d",
+ dbg ("%s extended capabilities at pci %2x",
label, HCC_EXT_CAPS (params));
}
@@ -547,4 +547,16 @@
}
+#ifdef EHCI_STATS
+ temp = snprintf (next, size, "irq normal %ld err %ld reclaim %ld\n",
+ ehci->stats.normal, ehci->stats.error, ehci->stats.reclaim);
+ size -= temp;
+ next += temp;
+
+ temp = snprintf (next, size, "complete %ld unlink %ld qpatch %ld\n",
+ ehci->stats.complete, ehci->stats.unlink, ehci->stats.qpatch);
+ size -= temp;
+ next += temp;
+#endif
+
spin_unlock_irqrestore (&ehci->lock, flags);
--- ./drivers/usb-dist/host/ehci-hub.c Sun Sep 15 19:57:44 2002
+++ ./drivers/usb/host/ehci-hub.c Mon Sep 23 10:22:33 2002
@@ -240,5 +240,6 @@
/* whoever resets must GetPortStatus to complete it!! */
if ((temp & PORT_RESET)
- && jiffies > ehci->reset_done [wIndex]) {
+ && time_after (jiffies,
+ ehci->reset_done [wIndex])) {
status |= 1 << USB_PORT_FEAT_C_RESET;
