On Fri, 25 Feb 2011, Xu, Andiry wrote:
> Hi Alan & David,
>
> Currently we're debugging an issue reported by our OEM with an USB WWLAN device. When this device is plugged in to EHCI controller on our platform, the host controller is hung. From the traces we found that the qh prepared by the driver has one qTD that has active work load followed by an Idle qTD and then subsequently followed by a qTD that has both the Active bit and Halt bit set.
What do you mean by an "Idle" qTD?
As far as I know, ehci-hcd never sets both the Active and Halt bits in
a qTD. Where do you see this happening?
> The last qTD is confusing the hardware: the host controller does not know how to handle a qTD with both ACTIVE and HALT bit set and it hangs.
>
> The patch below works for us:
>
> diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
> index fe99895..8612ff0 100644
> --- a/drivers/usb/host/ehci-q.c
> +++ b/drivers/usb/host/ehci-q.c
> @@ -464,6 +464,7 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
> if ((halt & hw->hw_token) == 0) {
> halt:
> hw->hw_token |= halt;
> + hw->hw_token &= ~ACTIVE_BIT(ehci);
> wmb ();
> }
> }
I don't see why this should make any difference. Here hw doesn't point
to a qTD; it points to the overlay region inside a QH.
Furthermore, these statements get executed in only three ways:
The QH is unlinked. In this case the hardware should never
see the QH so it shouldn't be confused by it.
The QH is halted because an earlier qTD terminated with an
error. In this case the Active bit should already be turned
off, so turning on the Halt bit shouldn't hurt.
A short read occurred. Is this the case that's bothering you?
Now, I have to admit that I don't remember why this code sets the Halt
bit. Maybe it doesn't have to be done at all; I'm not sure.
> So if we want to resolve the issue, is the patch acceptable, do you have any suggestions? There is no way the hardware can workaround the issue and we must avoid a qTD with both ACTIVE and HALT bit set in driver. It does cause confusion, even from the software view.
>
> And I found that EHCI driver set the HALT bit of qTD in several places. However, EHCI spec describes HALT bit like this:
>
> Bit Status Field Description
> 6 Halted. Set to a 1 by the Host Controller during status updates to indicate that a serious error has occurred at the device/endpoint addressed by this qTD.
>
> Although it does not say that software cannot set the bit, the description implies it as a status report bit and only the host controller will set it. What's your opinion?
The driver should set the Halt bit only in situations where the QH or
qTD is not being accessed by the hardware.
Alan Stern
--
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
