Hi again,
I've been doing lots of experimenting here. I eventually found that reverting
only the changes that led to version 1.48 of host/ohci-q.c does not make
2.6.4 work. In fact, the changes that led to both versions 1.48 and 1.49 need
to be reverted and then the timeout errors I have been experiencing seem to
be fixed in 2.6.4
Furthermore, I've manually reverted those changes from and built 2.6.10, and
that also now works. I've just spent most of the afternoon browsing websites
and downloading kernel tarballs through the Belkin wireless adapter, plugged
either directly into the usb port on my laptop or via the unpowered hub.
I have no idea whether I have left behind races or other potential
instabilities, so would be grateful for advice on any blunders I may have
unknowingly committed. Nor do I know whether all of the hunks of that patch
are actually needed to cure my timeouts, but there are too many combinations
even for someone with my patience. So again, any advive on hunks that can't
possibly be related to the timeouts and should be pout back into teh 2.6.10
code would also be appreciated. To facilitate this, I've made a patch of the
changes I made to back 1.48 and 1.49 out of ohci-q.c in 2.6.10.
Thanks for any further help.
Chris
On Tuesday 01 Feb 2005 23:39, Chris Clayton wrote:
> On Tue, 1 Feb 2005 14:38:43 -0800
>
> David Brownell <[EMAIL PROTECTED]> wrote:
> > What I think happened is that patch missed something. Specifically,
> > it didn't prevent the driver from setting the CLF or BLF bits while
> > the queue is empty because its only endpoint is being descheduled,
> > but a new transaction is submitted against it(*). Most hardware
> > evidently doesn't care; yours may. Certainly those bits were left
> > set, when they shouldn't have been.
> >
> > Please see if this patch makes a difference.
>
> Unfortunately, it didn't - I'm still getting timeouts with 2.6.10. BTW,
> this is 2.6.10 _without_ the msleep(10) in core/message.c::
> usb_internal_control_msg(), that we experimented with earlier.
> I think that's what you wanted.
>
> I've attached some dmesg and /sys/.../usb1/registers snapshots I took
> after the system had booted and whilst the hub was failing to attach. Hope
> these help
>
> > - Dave
--- linux-2.6.10/drivers/usb/host/ohci-q.c-2.6.10 2005-02-05 16:23:15.000000000 +0000
+++ linux-2.6.10/drivers/usb/host/ohci-q.c 2005-02-05 18:51:35.000000000 +0000
@@ -194,7 +194,6 @@
switch (ed->type) {
case PIPE_CONTROL:
if (ohci->ed_controltail == NULL) {
- WARN_ON (ohci->hc_control & OHCI_CTRL_CLE);
ohci_writel (ohci, ed->dma,
&ohci->regs->ed_controlhead);
} else {
@@ -215,7 +214,6 @@
case PIPE_BULK:
if (ohci->ed_bulktail == NULL) {
- WARN_ON (ohci->hc_control & OHCI_CTRL_BLE);
ohci_writel (ohci, ed->dma, &ohci->regs->ed_bulkhead);
} else {
ohci->ed_bulktail->ed_next = ed;
@@ -287,58 +285,29 @@
* just the link to the ed is unlinked.
* 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
- * (assuming it already started that, which needn't be true).
- *
- * ED_UNLINK is a transient state: the HC may still see this ED, but soon
- * it won't. ED_SKIP means the HC will finish its current transaction,
- * but won't start anything new. The TD queue may still grow; device
- * drivers don't know about this HCD-internal state.
- *
- * When the HC can't see the ED, something changes ED_UNLINK to one of:
- *
- * - ED_OPER: when there's any request queued, the ED gets rescheduled
- * immediately. HC should be working on them.
- *
- * - ED_IDLE: when there's no TD queue. there's no reason for the HC
- * to care about this ED; safe to disable the endpoint.
- *
- * When finish_unlinks() runs later, after SOF interrupt, it will often
- * complete one or more URB unlinks before making that state change.
*/
static void ed_deschedule (struct ohci_hcd *ohci, struct ed *ed)
{
ed->hwINFO |= cpu_to_hc32 (ohci, ED_SKIP);
- wmb ();
- ed->state = ED_UNLINK;
- /* To deschedule something from the control or bulk list, just
- * clear CLE/BLE and wait. There's no safe way to scrub out list
- * head/current registers until later, and "later" isn't very
- * tightly specified. Figure 6-5 and Section 6.4.2.2 show how
- * the HC is reading the ED queues (while we modify them).
- *
- * For now, ed_schedule() is "later". It might be good paranoia
- * to scrub those registers in finish_unlinks(), in case of bugs
- * that make the HC try to use them.
- */
switch (ed->type) {
case PIPE_CONTROL:
- /* remove ED from the HC's list: */
if (ed->ed_prev == NULL) {
if (!ed->hwNextED) {
ohci->hc_control &= ~OHCI_CTRL_CLE;
ohci_writel (ohci, ohci->hc_control,
&ohci->regs->control);
- // a ohci_readl() later syncs CLE with the HC
- } else
- ohci_writel (ohci,
- hc32_to_cpup (ohci, &ed->hwNextED),
+ ohci_writel (ohci, 0,
+ &ohci->regs->ed_controlcurrent);
+ // post those pci writes
+ (void) ohci_readl (ohci, &ohci->regs->control);
+ }
+ ohci_writel (ohci, hc32_to_cpup (ohci, &ed->hwNextED),
&ohci->regs->ed_controlhead);
} else {
ed->ed_prev->ed_next = ed->ed_next;
ed->ed_prev->hwNextED = ed->hwNextED;
}
- /* remove ED from the HCD's list: */
if (ohci->ed_controltail == ed) {
ohci->ed_controltail = ed->ed_prev;
if (ohci->ed_controltail)
@@ -349,22 +318,22 @@
break;
case PIPE_BULK:
- /* remove ED from the HC's list: */
if (ed->ed_prev == NULL) {
if (!ed->hwNextED) {
ohci->hc_control &= ~OHCI_CTRL_BLE;
ohci_writel (ohci, ohci->hc_control,
&ohci->regs->control);
- // a ohci_readl() later syncs BLE with the HC
- } else
- ohci_writel (ohci,
- hc32_to_cpup (ohci, &ed->hwNextED),
+ ohci_writel (ohci, 0,
+ &ohci->regs->ed_bulkcurrent);
+ // post those pci writes
+ (void) ohci_readl (ohci, &ohci->regs->control);
+ }
+ ohci_writel (ohci, hc32_to_cpup (ohci, &ed->hwNextED),
&ohci->regs->ed_bulkhead);
} else {
ed->ed_prev->ed_next = ed->ed_next;
ed->ed_prev->hwNextED = ed->hwNextED;
}
- /* remove ED from the HCD's list: */
if (ohci->ed_bulktail == ed) {
ohci->ed_bulktail = ed->ed_prev;
if (ohci->ed_bulktail)
@@ -380,6 +349,17 @@
periodic_unlink (ohci, ed);
break;
}
+ /* NOTE: Except for a couple of exceptionally clean unlink cases
+ * (like unlinking the only c/b ED, with no TDs) HCs may still be
+ * caching this operational ED (or its address). safe unlinking
+ * involves not marking it ED_IDLE till INTR_SF; we always do that
+ * if td_list isn't empty. Otherwise the race is small; but ...
+ */
+ if (ed->state == ED_OPER) {
+ ed->state = ED_IDLE;
+ ed->hwINFO &= ~(ED_SKIP | ED_DEQUEUE);
+ ed->hwHeadP &= ~ED_H;
+ }
}
@@ -474,25 +454,14 @@
/* request unlinking of an endpoint from an operational HC.
* put the ep on the rm_list
* real work is done at the next start frame (SF) hardware interrupt
- * caller guarantees HCD is running, so hardware access is safe,
- * and that ed->state is ED_OPER
+ * caller guarantees HCD is running, so hardware access is safe.
*/
static void start_ed_unlink (struct ohci_hcd *ohci, struct ed *ed)
{
ed->hwINFO |= cpu_to_hc32 (ohci, ED_DEQUEUE);
+ ed->state= ED_UNLINK;
ed_deschedule (ohci, ed);
- /* rm_list is just singly linked, for simplicity */
- ed->ed_next = ohci->ed_rm_list;
- ed->ed_prev = NULL;
- ohci->ed_rm_list = ed;
-
- /* enable SOF interrupt */
- ohci_writel (ohci, OHCI_INTR_SF, &ohci->regs->intrstatus);
- ohci_writel (ohci, OHCI_INTR_SF, &ohci->regs->intrenable);
- // flush those writes, and get latest HCCA contents
- (void) ohci_readl (ohci, &ohci->regs->control);
-
/* 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
@@ -500,6 +469,17 @@
*/
ed->tick = ohci_frame_no(ohci) + 1;
+ /* rm_list is just singly linked for simplicity */
+ ed->ed_next = ohci->ed_rm_list;
+ ed->ed_prev = 0;
+ ohci->ed_rm_list = ed;
+ /* enable SOF interrupt */
+ if (HCD_IS_RUNNING (ohci->hcd.state)) {
+ ohci_writel (ohci, OHCI_INTR_SF, &ohci->regs->intrstatus);
+ ohci_writel (ohci, OHCI_INTR_SF, &ohci->regs->intrenable);
+ // flush those pci writes
+ (void) ohci_readl (ohci, &ohci->regs->control);
+ }
}
/*-------------------------------------------------------------------------*
@@ -701,7 +681,6 @@
/* start periodic dma if needed */
if (periodic) {
- wmb ();
ohci->hc_control |= OHCI_CTRL_PLE|OHCI_CTRL_IE;
ohci_writel (ohci, ohci->hc_control, &ohci->regs->control);
}
@@ -837,6 +816,8 @@
next->next_dl_td = rev;
rev = next;
+ if (ed->hwTailP == cpu_to_hc32 (ohci, next->td_dma))
+ ed->hwTailP = next->hwNextTD;
ed->hwHeadP = next->hwNextTD | toggle;
}
@@ -959,10 +940,6 @@
/* unlink urbs as requested, but rescan the list after
* we call a completion since it might have unlinked
* another (earlier) urb
- *
- * When we get here, the HC doesn't see this ed. But it
- * must not be rescheduled until all completed URBs have
- * been given back to the driver.
*/
rescan_this:
completed = 0;
@@ -982,7 +959,12 @@
continue;
}
- /* patch pointer hc uses */
+ /* patch pointer hc uses ... tail, if we're removing
+ * an otherwise active td, and whatever td pointer
+ * points to thsi td
+ */
+ if (ed->hwTailP == cpu_to_hc32 (ohci, td->td_dma))
+ ed->hwTailP = td->hwNextTD;
savebits = *prev & ~cpu_to_hc32 (ohci, TD_MASK);
*prev = td->hwNextTD | savebits;
@@ -1001,10 +983,9 @@
/* ED's now officially unlinked, hc doesn't see */
ed->state = ED_IDLE;
+ ed->hwINFO &= ~(ED_SKIP | ED_DEQUEUE);
ed->hwHeadP &= ~cpu_to_hc32(ohci, ED_H);
ed->hwNextED = 0;
- wmb ();
- ed->hwINFO &= ~cpu_to_hc32 (ohci, ED_SKIP | ED_DEQUEUE);
/* but if there's work queued, reschedule */
if (!list_empty (&ed->td_list)) {
@@ -1083,7 +1064,7 @@
/* clean schedule: unlink EDs that are no longer busy */
if (list_empty (&ed->td_list)) {
if (ed->state == ED_OPER)
- start_ed_unlink (ohci, ed);
+ ed_deschedule (ohci, ed);
/* ... reenabling halted EDs only after fault cleanup */
} else if ((ed->hwINFO & cpu_to_hc32 (ohci, ED_SKIP | ED_DEQUEUE))
