Re: [RFC PATCH] xhci: do not halt the secondary HCD

[Joel Stanley]

On Tue, Sep 20, 2016 at 5:56 PM, Mathias Nyman
<mathias.nyman@xxxxxxxxxxxxxxx> wrote:
> Quick Googling shows that that TI TUSB 73x0 USB3.0 xHCI host has an issue
> with halting.
>
> Errata says host needs 125us to 1ms between the last control transfer and
> clearing the run/stop bit. (halting the host)
>
> Suggested workaround is to wait at least 2ms before halting the host.
>
> See issue #10 in:
> http://www.ti.com/lit/er/sllz076/sllz076.pdf
>
> It might just be that the patch works because it forces halting the host to
> be done later (secondary hcd -> primary hcd),  giving it enough time after
> the last control transfer.

Well spotted.

I gave this a go, adding a quirk and performing a msleep:

+++ b/drivers/usb/host/xhci.c
@@ -109,6 +109,10 @@ int xhci_halt(struct xhci_hcd *xhci)
 {
        int ret;
        xhci_dbg_trace(xhci, trace_xhci_dbg_init, "// Halt the HC");
+
+       if (xhci->quirks & XHCI_HALT_DELAY_QUIRK)
+               msleep(2);
+
        xhci_quiesce(xhci);

However it didn't help.

Are we guaranteed that transfers are not in flight at that point?

>
>>> a first step.
>>>
>>> load primary
>>> load secondary  (starts the xhci controller
>>> ...
>>> unload secondary (halts the controller)
>>> unload primary   (free memory)
>
>
> Now thinking about it, it doesn't really make sense to halt the host
> controller hardware
> before removing the primary hcd. It will just cause devices under the
> primary (USB2) to
> be removed uncleanly.  So basically the idea of the workaround makes sense,
> it just needs
> to be cleaned up from a workaround to intended behavior.

Great. When you say clean up, do you just mean tidying the comments?

Cheers,

Joel


>
> We might also need an additional quirk for TI TUSB 73x0 that adds a msleep()
> before the
> xhci_halt, even if it's moved to the last hcd removed.
>
> -Mathias