ChangeSet 1.1504.2.8, 2003/12/05 11:14:07-08:00, [EMAIL PROTECTED]
[PATCH] USB: ohci, fix iso "bad entry" bug + misc
A while back there were some reports of ohci reporting a "bad entry"
diagnostic, mostly with ISO transfers, which were mysterious until
I recently found an easy way to reproduce it.
This patch:
- Fixes at least one cause of that "bad entry" diagnostic by
waiting for INTR_WDH before completing ED unlink processing.
(Else URB unlinking could free TDs on the donelist, so the
WDH processing would see those entries as "bad".)
- Merges the patch from Darwin Rambo <[EMAIL PROTECTED]>,
coping with CPUs that can't do 16 bit accesses (MIPS).
- Renames a function as start_ed_unlink(), matching its role.
- Fixes minor debug output issues, including a FIXME to tell
more info about TDs on the periodic schedule. And adding
some missing newlines (makes this patch seem big).
Nobody's complained much about that "bad entry" issue lately, but
if necessary that part would be particularly easy to split out.
Please merge to the next kernel that gets USB patches.
drivers/usb/host/ohci-dbg.c | 43 +++++++++++++++++++++++++------------------
drivers/usb/host/ohci-hcd.c | 8 ++++----
drivers/usb/host/ohci-q.c | 32 ++++++++++++++++++++++++--------
drivers/usb/host/ohci.h | 10 ++++++++--
4 files changed, 61 insertions(+), 32 deletions(-)
diff -Nru a/drivers/usb/host/ohci-dbg.c b/drivers/usb/host/ohci-dbg.c
--- a/drivers/usb/host/ohci-dbg.c Mon Dec 29 14:26:11 2003
+++ b/drivers/usb/host/ohci-dbg.c Mon Dec 29 14:26:11 2003
@@ -269,18 +269,19 @@
ohci_dump_status (controller, NULL, 0);
if (controller->hcca)
ohci_dbg (controller,
- "hcca frame #%04x\n", controller->hcca->frame_no);
+ "hcca frame #%04x\n", OHCI_FRAME_NO(controller->hcca));
ohci_dump_roothub (controller, 1, NULL, 0);
}
static const char data0 [] = "DATA0";
static const char data1 [] = "DATA1";
-static void ohci_dump_td (struct ohci_hcd *ohci, char *label, struct td *td)
+static void ohci_dump_td (const struct ohci_hcd *ohci, const char *label,
+ const struct td *td)
{
u32 tmp = le32_to_cpup (&td->hwINFO);
- ohci_dbg (ohci, "%s td %p%s; urb %p index %d; hw next td %08x",
+ ohci_dbg (ohci, "%s td %p%s; urb %p index %d; hw next td %08x\n",
label, td,
(tmp & TD_DONE) ? " (DONE)" : "",
td->urb, td->index,
@@ -301,28 +302,28 @@
case TD_DP_OUT: pid = "OUT"; break;
default: pid = "(bad pid)"; break;
}
- ohci_dbg (ohci, " info %08x CC=%x %s DI=%d %s %s", tmp,
+ ohci_dbg (ohci, " info %08x CC=%x %s DI=%d %s %s\n", tmp,
TD_CC_GET(tmp), /* EC, */ toggle,
(tmp & TD_DI) >> 21, pid,
(tmp & TD_R) ? "R" : "");
cbp = le32_to_cpup (&td->hwCBP);
be = le32_to_cpup (&td->hwBE);
- ohci_dbg (ohci, " cbp %08x be %08x (len %d)", cbp, be,
+ ohci_dbg (ohci, " cbp %08x be %08x (len %d)\n", cbp, be,
cbp ? (be + 1 - cbp) : 0);
} else {
unsigned i;
- ohci_dbg (ohci, " info %08x CC=%x FC=%d DI=%d SF=%04x", tmp,
+ ohci_dbg (ohci, " info %08x CC=%x FC=%d DI=%d SF=%04x\n", tmp,
TD_CC_GET(tmp),
(tmp >> 24) & 0x07,
(tmp & TD_DI) >> 21,
tmp & 0x0000ffff);
- ohci_dbg (ohci, " bp0 %08x be %08x",
+ ohci_dbg (ohci, " bp0 %08x be %08x\n",
le32_to_cpup (&td->hwCBP) & ~0x0fff,
le32_to_cpup (&td->hwBE));
for (i = 0; i < MAXPSW; i++) {
u16 psw = le16_to_cpup (&td->hwPSW [i]);
int cc = (psw >> 12) & 0x0f;
- ohci_dbg (ohci, " psw [%d] = %2x, CC=%x %s=%d", i,
+ ohci_dbg (ohci, " psw [%d] = %2x, CC=%x %s=%d\n", i,
psw, cc,
(cc >= 0x0e) ? "OFFSET" : "SIZE",
psw & 0x0fff);
@@ -332,12 +333,13 @@
/* caller MUST own hcd spinlock if verbose is set! */
static void __attribute__((unused))
-ohci_dump_ed (struct ohci_hcd *ohci, char *label, struct ed *ed, int verbose)
+ohci_dump_ed (const struct ohci_hcd *ohci, const char *label,
+ const struct ed *ed, int verbose)
{
u32 tmp = ed->hwINFO;
char *type = "";
- ohci_dbg (ohci, "%s, ed %p state 0x%x type %s; next ed %08x",
+ ohci_dbg (ohci, "%s, ed %p state 0x%x type %s; next ed %08x\n",
label,
ed, ed->state, edstring (ed->type),
le32_to_cpup (&ed->hwNextED));
@@ -347,7 +349,7 @@
/* else from TDs ... control */
}
ohci_dbg (ohci,
- " info %08x MAX=%d%s%s%s%s EP=%d%s DEV=%d", le32_to_cpu (tmp),
+ " info %08x MAX=%d%s%s%s%s EP=%d%s DEV=%d\n", le32_to_cpu (tmp),
0x03ff & (le32_to_cpu (tmp) >> 16),
(tmp & ED_DEQUEUE) ? " DQ" : "",
(tmp & ED_ISO) ? " ISO" : "",
@@ -356,7 +358,7 @@
0x000f & (le32_to_cpu (tmp) >> 7),
type,
0x007f & le32_to_cpu (tmp));
- ohci_dbg (ohci, " tds: head %08x %s%s tail %08x%s",
+ ohci_dbg (ohci, " tds: head %08x %s%s tail %08x%s\n",
tmp = le32_to_cpup (&ed->hwHeadP),
(ed->hwHeadP & ED_C) ? data1 : data0,
(ed->hwHeadP & ED_H) ? " HALT" : "",
@@ -541,24 +543,29 @@
if (temp == seen_count) {
u32 info = ed->hwINFO;
u32 scratch = cpu_to_le32p (&ed->hwINFO);
+ struct list_head *entry;
+ unsigned qlen = 0;
+
+ /* qlen measured here in TDs, not urbs */
+ list_for_each (entry, &ed->td_list)
+ qlen++;
temp = snprintf (next, size,
- " (%cs dev%d%s ep%d%s"
+ " (%cs dev%d ep%d%s-%s qlen %u"
" max %d %08x%s%s)",
(info & ED_LOWSPEED) ? 'l' : 'f',
scratch & 0x7f,
- (info & ED_ISO) ? " iso" : "",
(scratch >> 7) & 0xf,
(info & ED_IN) ? "in" : "out",
+ (info & ED_ISO) ? "iso" : "int",
+ qlen,
0x03ff & (scratch >> 16),
scratch,
- (info & ED_SKIP) ? " s" : "",
+ (info & ED_SKIP) ? " K" : "",
(ed->hwHeadP & ED_H) ? " H" : "");
size -= temp;
next += temp;
- // FIXME some TD info too
-
if (seen_count < DBG_SCHED_LIMIT)
seen [seen_count++] = ed;
@@ -617,7 +624,7 @@
/* hcca */
if (ohci->hcca)
ohci_dbg_sw (ohci, &next, &size,
- "hcca frame 0x%04x\n", ohci->hcca->frame_no);
+ "hcca frame 0x%04x\n", OHCI_FRAME_NO(ohci->hcca));
/* other registers mostly affect frame timings */
rdata = readl (®s->fminterval);
diff -Nru a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
--- a/drivers/usb/host/ohci-hcd.c Mon Dec 29 14:26:11 2003
+++ b/drivers/usb/host/ohci-hcd.c Mon Dec 29 14:26:11 2003
@@ -226,7 +226,7 @@
if (retval < 0)
goto fail;
if (ed->type == PIPE_ISOCHRONOUS) {
- u16 frame = le16_to_cpu (ohci->hcca->frame_no);
+ u16 frame = OHCI_FRAME_NO(ohci->hcca);
/* delay a few frames before the first TD */
frame += max_t (u16, 8, ed->interval);
@@ -281,7 +281,7 @@
urb_priv = urb->hcpriv;
if (urb_priv) {
if (urb_priv->ed->state == ED_OPER)
- start_urb_unlink (ohci, urb_priv->ed);
+ start_ed_unlink (ohci, urb_priv->ed);
}
} else {
/*
@@ -363,7 +363,7 @@
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
- return le16_to_cpu (ohci->hcca->frame_no);
+ return OHCI_FRAME_NO(ohci->hcca);
}
/*-------------------------------------------------------------------------*
@@ -591,7 +591,7 @@
*/
spin_lock (&ohci->lock);
if (ohci->ed_rm_list)
- finish_unlinks (ohci, le16_to_cpu (ohci->hcca->frame_no),
+ finish_unlinks (ohci, OHCI_FRAME_NO(ohci->hcca),
ptregs);
if ((ints & OHCI_INTR_SF) != 0 && !ohci->ed_rm_list
&& HCD_IS_RUNNING(ohci->hcd.state))
diff -Nru a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
--- a/drivers/usb/host/ohci-q.c Mon Dec 29 14:26:11 2003
+++ b/drivers/usb/host/ohci-q.c Mon Dec 29 14:26:11 2003
@@ -430,7 +430,7 @@
* put the ep on the rm_list
* real work is done at the next start frame (SF) hardware interrupt
*/
-static void start_urb_unlink (struct ohci_hcd *ohci, struct ed *ed)
+static void start_ed_unlink (struct ohci_hcd *ohci, struct ed *ed)
{
ed->hwINFO |= ED_DEQUEUE;
ed->state = ED_UNLINK;
@@ -441,7 +441,7 @@
* 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->tick = OHCI_FRAME_NO(ohci->hcca) + 1;
/* rm_list is just singly linked, for simplicity */
ed->ed_next = ohci->ed_rm_list;
@@ -479,7 +479,8 @@
* and iso; other urbs rarely need more than one TD per urb.
* this way, only final tds (or ones with an error) cause IRQs.
* at least immediately; use DI=6 in case any control request is
- * tempted to die part way through.
+ * tempted to die part way through. (and to force the hc to flush
+ * its donelist soonish, even on unlink paths.)
*
* NOTE: could delay interrupts even for the last TD, and get fewer
* interrupts ... increasing per-urb latency by sharing interrupts.
@@ -879,12 +880,27 @@
u32 *prev;
/* only take off EDs that the HC isn't using, accounting for
- * frame counter wraps.
+ * frame counter wraps and EDs with partially retired TDs
*/
- if (tick_before (tick, ed->tick)
- && HCD_IS_RUNNING(ohci->hcd.state)) {
- last = &ed->ed_next;
- continue;
+ if (likely (HCD_IS_RUNNING(ohci->hcd.state))) {
+ if (tick_before (tick, ed->tick)) {
+skip_ed:
+ last = &ed->ed_next;
+ continue;
+ }
+
+ if (!list_empty (&ed->td_list)) {
+ struct td *td;
+ u32 head;
+
+ td = list_entry (ed->td_list.next, struct td,
+ td_list);
+ head = cpu_to_le32 (ed->hwHeadP) & TD_MASK;
+
+ /* INTR_WDH may need to clean up first */
+ if (td->td_dma != head)
+ goto skip_ed;
+ }
}
/* reentrancy: if we drop the schedule lock, someone might
diff -Nru a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
--- a/drivers/usb/host/ohci.h Mon Dec 29 14:26:11 2003
+++ b/drivers/usb/host/ohci.h Mon Dec 29 14:26:11 2003
@@ -172,8 +172,14 @@
struct ohci_hcca {
#define NUM_INTS 32
__u32 int_table [NUM_INTS]; /* periodic schedule */
- __u16 frame_no; /* current frame number */
- __u16 pad1; /* set to 0 on each frame_no change */
+
+ /*
+ * OHCI defines u16 frame_no, followed by u16 zero pad.
+ * Since some processors can't do 16 bit bus accesses,
+ * portable access must be a 32 bit byteswapped access.
+ */
+ u32 frame_no; /* current frame number */
+#define OHCI_FRAME_NO(hccap) ((u16)le32_to_cpup(&(hccap)->frame_no))
__u32 done_head; /* info returned for an interrupt */
u8 reserved_for_hc [116];
u8 what [4]; /* spec only identifies 252 bytes :) */
-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills. Sign up for IBM's
Free Linux Tutorials. Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id78&alloc_id371&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
