On Thu, 30 Jul 2009, Matthijs Kooijman wrote:
> Hi Alan,
>
> I've tried a clean kernel (without your patch), which shows the same problem.
> So it seems it's not caused by your patch. I also added some debug statements
> to see what is happening. It seems that the xacterrs value is never initalized
> for this particular qh, which voids my theory about rapidly decreasing
> xacterrs without any messages printed.
>
> I've made a log of just inserting the hub, using 2.6.31-rc4 with your patch
> and [1] applied. The resulting kernel output is at [2].
>
> I had a look at the kernel sources, but I really don't know enough about USB
> and the ehci driver to follow the code paths (or rather, know how they should
> be). So I hope you can make a decent guess from this log.
Good work; I figured it out. Although it's easy to forget, there are
_two_ routines which can add a QH to the schedule, and they both need
to initialize qh->xacterrs.
I decided to simplify the transaction error counting at the same time.
The resulting patch below should fix the problem. It is independent of
the other patch, although it touches some adjacent lines.
Alan Stern
Index: 2.6.31-rc4/drivers/usb/host/ehci-q.c
===================================================================
--- 2.6.31-rc4.orig/drivers/usb/host/ehci-q.c
+++ 2.6.31-rc4/drivers/usb/host/ehci-q.c
@@ -375,12 +375,11 @@ qh_completions (struct ehci_hcd *ehci, s
*/
if ((token & QTD_STS_XACT) &&
QTD_CERR(token) == 0 &&
- --qh->xacterrs > 0 &&
+ ++qh->xacterrs < QH_XACTERR_MAX &&
!urb->unlinked) {
ehci_dbg(ehci,
"detected XactErr len %zu/%zu retry %d\n",
- qtd->length - QTD_LENGTH(token), qtd->length,
- QH_XACTERR_MAX - qh->xacterrs);
+ qtd->length - QTD_LENGTH(token), qtd->length, qh->xacterrs);
/* reset the token in the qtd and the
* qh overlay (which still contains
@@ -494,7 +493,7 @@ halt:
last = qtd;
/* reinit the xacterr counter for the next qtd */
- qh->xacterrs = QH_XACTERR_MAX;
+ qh->xacterrs = 0;
}
/* last urb's completion might still need calling */
@@ -941,7 +940,7 @@ static void qh_link_async (struct ehci_h
head->hw_next = dma;
qh_get(qh);
- qh->xacterrs = QH_XACTERR_MAX;
+ qh->xacterrs = 0;
qh->qh_state = QH_STATE_LINKED;
/* qtd completions reported later by interrupt */
}
Index: 2.6.31-rc4/drivers/usb/host/ehci-sched.c
===================================================================
--- 2.6.31-rc4.orig/drivers/usb/host/ehci-sched.c
+++ 2.6.31-rc4/drivers/usb/host/ehci-sched.c
@@ -542,6 +542,7 @@ static int qh_link_periodic (struct ehci
}
}
qh->qh_state = QH_STATE_LINKED;
+ qh->xacterrs = 0;
qh_get (qh);
/* update per-qh bandwidth for usbfs */
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
