# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.454 -> 1.455
# drivers/usb/host/ohci-hcd.c 1.11 -> 1.12
# drivers/usb/host/ohci-q.c 1.7 -> 1.8
# drivers/usb/host/ohci.h 1.4 -> 1.5
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/06/05 [EMAIL PROTECTED] 1.455
# [PATCH] ohci-hcd, fix urb unlink races
#
# This patch:
#
# - Fixes a longstanding urb unlink race, by switching to a single queue
# for EDs being unlinked. The previous two-queue scheme was sensitive to
# IRQ latencies: one extra millisecond would make it use the wrong queue.
# This updated scheme should handle latencies of up to 32K microseconds
# (Cthulu forfend :) and slightly shrinks object code size.
#
# - Related (mostly) cleanup. Some functions and one ED field renamed, ED
# layout is a smidgeon more compact (even given more data), driver version
# string doesn't reflect CVS, debug message only comes out in verbose mode.
# --------------------------------------------
#
diff -Nru a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
--- a/drivers/usb/host/ohci-hcd.c Wed Jun 5 12:28:53 2002
+++ b/drivers/usb/host/ohci-hcd.c Wed Jun 5 12:28:53 2002
@@ -12,6 +12,9 @@
*
* History:
*
+ * 2002/06/01 remember frame when HC won't see EDs any more; use that info
+ * to fix urb unlink races caused by interrupt latency assumptions;
+ * minor ED field and function naming updates
* 2002/01/18 package as a patch for 2.5.3; this should match the
* 2.4.17 kernel modulo some bugs being fixed.
*
@@ -106,7 +109,7 @@
* - lots more testing!!
*/
-#define DRIVER_VERSION "$Revision: 1.9 $"
+#define DRIVER_VERSION "2002-Jun-01"
#define DRIVER_AUTHOR "Roman Weissgaerber <[EMAIL PROTECTED]>, David Brownell"
#define DRIVER_DESC "USB 1.1 'Open' Host Controller (OHCI) Driver"
@@ -287,7 +290,7 @@
}
urb_priv->state = URB_DEL;
- ed_unlink (urb->dev, urb_priv->ed);
+ start_urb_unlink (ohci, urb_priv->ed);
spin_unlock_irqrestore (&ohci->lock, flags);
} else {
/*
@@ -508,16 +511,15 @@
/* could track INTR_SO to reduce available PCI/... bandwidth */
- // FIXME: this assumes SOF (1/ms) interrupts don't get lost...
- if (ints & OHCI_INTR_SF) {
- unsigned int frame = le16_to_cpu (ohci->hcca->frame_no) & 1;
+ /* handle any pending URB/ED unlinks, leaving INTR_SF enabled
+ * when there's still unlinking to be done (next frame).
+ */
+ spin_lock (&ohci->lock);
+ if (ohci->ed_rm_list)
+ finish_unlinks (ohci, le16_to_cpu (ohci->hcca->frame_no));
+ if ((ints & OHCI_INTR_SF) != 0 && !ohci->ed_rm_list)
writel (OHCI_INTR_SF, ®s->intrdisable);
- if (ohci->ed_rm_list [!frame] != NULL) {
- dl_del_list (ohci, !frame);
- }
- if (ohci->ed_rm_list [frame] != NULL)
- writel (OHCI_INTR_SF, ®s->intrenable);
- }
+ spin_unlock (&ohci->lock);
writel (ints, ®s->intrstatus);
writel (OHCI_INTR_MIE, ®s->intrenable);
@@ -719,8 +721,7 @@
for (i = 0; i < NUM_INTS; i++) ohci->hcca->int_table [i] = 0;
/* no EDs to remove */
- ohci->ed_rm_list [0] = NULL;
- ohci->ed_rm_list [1] = NULL;
+ ohci->ed_rm_list = NULL;
/* empty control and bulk lists */
ohci->ed_isotail = NULL;
@@ -802,7 +803,7 @@
ohci->disabled = 0;
ohci->sleeping = 0;
ohci->hc_control = OHCI_CONTROL_INIT | OHCI_USB_OPER;
- if (!ohci->ed_rm_list [0] && !ohci->ed_rm_list [1]) {
+ if (!ohci->ed_rm_list) {
if (ohci->ed_controltail)
ohci->hc_control |= OHCI_CTRL_CLE;
if (ohci->ed_bulktail)
diff -Nru a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
--- a/drivers/usb/host/ohci-q.c Wed Jun 5 12:28:53 2002
+++ b/drivers/usb/host/ohci-q.c Wed Jun 5 12:28:53 2002
@@ -208,8 +208,7 @@
}
ed->ed_prev = ohci->ed_controltail;
if (!ohci->ed_controltail
- && !ohci->ed_rm_list [0]
- && !ohci->ed_rm_list [1]
+ && !ohci->ed_rm_list
&& !ohci->sleeping
) {
ohci->hc_control |= OHCI_CTRL_CLE;
@@ -227,8 +226,7 @@
}
ed->ed_prev = ohci->ed_bulktail;
if (!ohci->ed_bulktail
- && !ohci->ed_rm_list [0]
- && !ohci->ed_rm_list [1]
+ && !ohci->ed_rm_list
&& !ohci->sleeping
) {
ohci->hc_control |= OHCI_CTRL_BLE;
@@ -240,16 +238,16 @@
case PIPE_INTERRUPT:
load = ed->int_load;
interval = ep_2_n_interval (ed->int_period);
- ed->int_interval = interval;
+ ed->interval = interval;
int_branch = ep_int_balance (ohci, interval, load);
ed->int_branch = int_branch;
for (i = 0; i < ep_rev (6, interval); i += inter) {
inter = 1;
for (ed_p = & (ohci->hcca->int_table [ep_rev (5, i) +
int_branch]);
- (*ed_p != 0) && ((dma_to_ed (ohci, le32_to_cpup
(ed_p)))->int_interval >= interval);
+ (*ed_p != 0) && ((dma_to_ed (ohci, le32_to_cpup
+(ed_p)))->interval >= interval);
ed_p = & ((dma_to_ed (ohci, le32_to_cpup
(ed_p)))->hwNextED))
- inter = ep_rev (6, (dma_to_ed (ohci,
le32_to_cpup (ed_p)))->int_interval);
+ inter = ep_rev (6, (dma_to_ed (ohci,
+le32_to_cpup (ed_p)))->interval);
ed->hwNextED = *ed_p;
*ed_p = cpu_to_le32 (ed->dma);
}
@@ -260,7 +258,7 @@
case PIPE_ISOCHRONOUS:
ed->hwNextED = 0;
- ed->int_interval = 1;
+ ed->interval = 1;
if (ohci->ed_isotail != NULL) {
ohci->ed_isotail->hwNextED = cpu_to_le32 (ed->dma);
ed->ed_prev = ohci->ed_isotail;
@@ -270,7 +268,7 @@
for (ed_p = & (ohci->hcca->int_table [ep_rev (5, i)]);
*ed_p != 0;
ed_p = & ((dma_to_ed (ohci, le32_to_cpup
(ed_p)))->hwNextED))
- inter = ep_rev (6, (dma_to_ed (ohci,
le32_to_cpup (ed_p)))->int_interval);
+ inter = ep_rev (6, (dma_to_ed (ohci,
+le32_to_cpup (ed_p)))->interval);
*ed_p = cpu_to_le32 (ed->dma);
}
ed->ed_prev = NULL;
@@ -311,7 +309,7 @@
* the link from the ed still points to another operational ed or 0
* so the HC can eventually finish the processing of the unlinked ed
*/
-static int ep_unlink (struct ohci_hcd *ohci, struct ed *ed)
+static int start_ed_unlink (struct ohci_hcd *ohci, struct ed *ed)
{
int i;
@@ -357,8 +355,8 @@
break;
case PIPE_INTERRUPT:
- periodic_unlink (ohci, ed, ed->int_branch, ed->int_interval);
- for (i = ed->int_branch; i < NUM_INTS; i += ed->int_interval)
+ periodic_unlink (ohci, ed, ed->int_branch, ed->interval);
+ for (i = ed->int_branch; i < NUM_INTS; i += ed->interval)
ohci->ohci_int_load [i] -= ed->int_load;
#ifdef OHCI_VERBOSE_DEBUG
ohci_dump_periodic (ohci, "UNLINK_INT");
@@ -384,6 +382,10 @@
/* FIXME ED's "unlink" state is indeterminate;
* the HC might still be caching it (till SOF).
+ * - use ed_rm_list and finish_unlinks(), adding some state that
+ * prevents clobbering hw linkage before the appropriate SOF
+ * - a speedup: when only one urb is queued on the ed, save 1msec
+ * by making start_urb_unlink() use this routine to deschedule.
*/
ed->state = ED_UNLINK;
return 0;
@@ -478,11 +480,8 @@
* put the ep on the rm_list and stop the bulk or ctrl list
* real work is done at the next start frame (SF) hardware interrupt
*/
-static void ed_unlink (struct usb_device *usb_dev, struct ed *ed)
+static void start_urb_unlink (struct ohci_hcd *ohci, struct ed *ed)
{
- unsigned int frame;
- struct ohci_hcd *ohci = hcd_to_ohci (usb_dev->bus->hcpriv);
-
/* already pending? */
if (ed->state & ED_URB_DEL)
return;
@@ -503,9 +502,15 @@
break;
}
- frame = le16_to_cpu (ohci->hcca->frame_no) & 0x1;
- ed->ed_rm_list = ohci->ed_rm_list [frame];
- ohci->ed_rm_list [frame] = ed;
+ /* SF interrupt might get delayed; record the frame counter value that
+ * indicates when the HC isn't looking at it, so concurrent unlinks
+ * behave. frame_no wraps every 2^16 msec, and changes right before
+ * SF is triggered.
+ */
+ ed->tick = le16_to_cpu (ohci->hcca->frame_no) + 1;
+
+ ed->ed_rm_list = ohci->ed_rm_list;
+ ohci->ed_rm_list = ed;
/* enable SOF interrupt */
if (!ohci->sleeping) {
@@ -816,10 +821,12 @@
if (td_list->ed->hwHeadP & ED_H) {
if (urb_priv && ((td_list->index + 1)
< urb_priv->length)) {
+#ifdef OHCI_VERBOSE_DEBUG
dbg ("urb %p TD %d of %d, patch ED",
td_list->urb,
1 + td_list->index,
urb_priv->length);
+#endif
td_list->ed->hwHeadP =
(urb_priv->td [urb_priv->length - 1]->hwNextTD
& __constant_cpu_to_le32 (TD_MASK))
@@ -841,27 +848,37 @@
/*-------------------------------------------------------------------------*/
-/* there are some pending requests to unlink
- * - some URBs/TDs if urb_priv->state == URB_DEL
- */
-static void dl_del_list (struct ohci_hcd *ohci, unsigned int frame)
+/* wrap-aware logic stolen from <linux/jiffies.h> */
+#define tick_before(t1,t2) ((((s16)(t1))-((s16)(t2))) < 0)
+
+/* there are some urbs/eds to unlink; called in_irq(), with HCD locked */
+static void finish_unlinks (struct ohci_hcd *ohci, u16 tick)
{
- unsigned long flags;
- struct ed *ed;
- __u32 edINFO;
- __u32 tdINFO;
- struct td *td = NULL, *td_next = NULL,
- *tdHeadP = NULL, *tdTailP;
- __u32 *td_p;
+ struct ed *ed, **last;
int ctrl = 0, bulk = 0;
- spin_lock_irqsave (&ohci->lock, flags);
-
- for (ed = ohci->ed_rm_list [frame]; ed != NULL; ed = ed->ed_rm_list) {
+ for (last = &ohci->ed_rm_list, ed = *last; ed != NULL; ed = *last) {
+ struct td *td, *td_next, *tdHeadP, *tdTailP;
+ u32 *td_p;
+ int unlinked;
+
+ /* only take off EDs that the HC isn't using, accounting for
+ * frame counter wraps. completion callbacks might prepend
+ * EDs to the list, they'll be checked next irq.
+ */
+ if (tick_before (tick, ed->tick)) {
+ last = &ed->ed_rm_list;
+ continue;
+ }
+ *last = ed->ed_rm_list;
+ ed->ed_rm_list = 0;
+ unlinked = 0;
+ /* unlink urbs from first one requested to queue end;
+ * leave earlier urbs alone
+ */
tdTailP = dma_to_td (ohci, le32_to_cpup (&ed->hwTailP));
tdHeadP = dma_to_td (ohci, le32_to_cpup (&ed->hwHeadP));
- edINFO = le32_to_cpup (&ed->hwINFO);
td_p = &ed->hwHeadP;
for (td = tdHeadP; td != tdTailP; td = td_next) {
@@ -870,8 +887,11 @@
td_next = dma_to_td (ohci,
le32_to_cpup (&td->hwNextTD));
- if ((urb_priv->state == URB_DEL)) {
- tdINFO = le32_to_cpup (&td->hwINFO);
+ if (unlinked || (urb_priv->state == URB_DEL)) {
+ u32 tdINFO = le32_to_cpup (&td->hwINFO);
+
+ unlinked = 1;
+
/* HC may have partly processed this TD */
if (TD_CC_GET (tdINFO) < 0xE)
td_done (urb, td);
@@ -880,22 +900,32 @@
/* URB is done; clean up */
if (++ (urb_priv->td_cnt) == urb_priv->length) {
- spin_unlock_irqrestore (&ohci->lock,
- flags);
+ if (urb->status == -EINPROGRESS)
+ urb->status = -ECONNRESET;
+ spin_unlock (&ohci->lock);
finish_urb (ohci, urb);
- spin_lock_irqsave (&ohci->lock, flags);
+ spin_lock (&ohci->lock);
}
} else {
td_p = &td->hwNextTD;
}
}
+ /* FIXME actually want four cases here:
+ * (a) finishing URB unlink
+ * [a1] no URBs queued, so start ED unlink
+ * [a2] some (earlier) URBs still linked, re-enable
+ * (b) finishing ED unlink
+ * [b1] no URBs queued, ED is truly idle now
+ * [b2] URBs now queued, link ED back into schedule
+ * right now we only have (a)
+ */
ed->state &= ~ED_URB_DEL;
tdHeadP = dma_to_td (ohci, le32_to_cpup (&ed->hwHeadP));
if (tdHeadP == tdTailP) {
if (ed->state == ED_OPER)
- ep_unlink (ohci, ed);
+ start_ed_unlink (ohci, ed);
td_free (ohci, tdTailP);
ed->hwINFO = ED_SKIP;
ed->state = ED_NEW;
@@ -918,7 +948,7 @@
writel (0, &ohci->regs->ed_controlcurrent);
if (bulk) /* reset bulk list */
writel (0, &ohci->regs->ed_bulkcurrent);
- if (!ohci->ed_rm_list [!frame]) {
+ if (!ohci->ed_rm_list) {
if (ohci->ed_controltail)
ohci->hc_control |= OHCI_CTRL_CLE;
if (ohci->ed_bulktail)
@@ -926,9 +956,6 @@
writel (ohci->hc_control, &ohci->regs->control);
}
}
-
- ohci->ed_rm_list [frame] = NULL;
- spin_unlock_irqrestore (&ohci->lock, flags);
}
@@ -939,7 +966,7 @@
* Process normal completions (error or success) and clean the schedules.
*
* This is the main path for handing urbs back to drivers. The only other
- * path is dl_del_list(), which unlinks URBs by scanning EDs, instead of
+ * path is finish_unlinks(), which unlinks URBs using ed_rm_list, instead of
* scanning the (re-reversed) donelist as this does.
*/
static void dl_done_list (struct ohci_hcd *ohci, struct td *td)
@@ -960,7 +987,7 @@
/* If all this urb's TDs are done, call complete().
* Interrupt transfers are the only special case:
* they're reissued, until "deleted" by usb_unlink_urb
- * (real work done in a SOF intr, by dl_del_list).
+ * (real work done in a SOF intr, by finish_unlinks).
*/
if (urb_priv->td_cnt == urb_priv->length) {
int resubmit;
@@ -980,7 +1007,7 @@
if ((ed->hwHeadP & __constant_cpu_to_le32 (TD_MASK))
== ed->hwTailP
&& (ed->state == ED_OPER))
- ep_unlink (ohci, ed);
+ start_ed_unlink (ohci, ed);
td = td_next;
}
spin_unlock_irqrestore (&ohci->lock, flags);
diff -Nru a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
--- a/drivers/usb/host/ohci.h Wed Jun 5 12:28:53 2002
+++ b/drivers/usb/host/ohci.h Wed Jun 5 12:28:53 2002
@@ -27,22 +27,29 @@
__u32 hwNextED; /* next ED in list */
/* rest are purely for the driver's use */
- struct ed *ed_prev;
- __u8 int_period;
- __u8 int_branch;
- __u8 int_load;
- __u8 int_interval;
- __u8 state; // ED_{NEW,UNLINK,OPER}
+ dma_addr_t dma; /* addr of ED */
+ struct ed *ed_prev; /* for non-interrupt EDs */
+
+ u8 type; /* PIPE_{BULK,...} */
+ u8 interval; /* interrupt, isochronous */
+ union {
+ struct intr_info { /* interrupt */
+ u8 int_period;
+ u8 int_branch;
+ u8 int_load;
+ };
+ u16 last_iso; /* isochronous */
+ };
+
+ u8 state; /* ED_{NEW,UNLINK,OPER} */
#define ED_NEW 0x00 /* unused, no dummy td */
#define ED_UNLINK 0x01 /* dummy td, maybe linked to hc */
#define ED_OPER 0x02 /* dummy td, _is_ linked to hc */
#define ED_URB_DEL 0x08 /* for unlinking; masked in */
- __u8 type;
- __u16 last_iso;
+ /* HC may see EDs on rm_list until next frame (frame_no == tick) */
+ u16 tick;
struct ed *ed_rm_list;
-
- dma_addr_t dma; /* addr of ED */
} __attribute__ ((aligned(16)));
#define ED_MASK ((u32)~0x0f) /* strip hw status in low addr bits */
@@ -335,7 +342,7 @@
struct ohci_hcca *hcca;
dma_addr_t hcca_dma;
- struct ed *ed_rm_list [2]; /* to be removed */
+ struct ed *ed_rm_list; /* to be removed */
struct ed *ed_bulktail; /* last in bulk list */
struct ed *ed_controltail; /* last in ctrl list */
_______________________________________________________________
Don't miss the 2002 Sprint PCS Application Developer's Conference
August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
