On 30.6.2020 16.08, Rik van Riel wrote:
> I misread the code, it's not a bitfield, so state 1 means an endpoint marked 
> with running state. The next urb is never getting a response, though.
> 
> However, the xhci spec says an endpoint is halted upon a babble error.

I was looking at the same, so according to specs this state shouldn't be 
possible. 

> 
> The code right above the babble handling case adds halted into the endpoint 
> state itself. Does the code handling the babble error need to do something 
> similar to trigger cleanup elsewhere? 

It's a flag to prevent ringing the doorbell for a halted endpoint.
Anyway, reset endpoint is meant to recover an endpoint in a halted state.
Resetting non-halted endpoints will just lead to a context state error, and
besides, isoc endpoints shouldn't halt.

Anyways, I haven't got any better idea at the moment.
You can try and see what a forced reset does with:

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2c255d0620b0..d79aca0df6d4 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1900,8 +1900,7 @@ static int xhci_requires_manual_halt_cleanup(struct 
xhci_hcd *xhci,
                 * endpoint anyway.  Check if a babble halted the
                 * endpoint.
                 */
-               if (GET_EP_CTX_STATE(ep_ctx) == EP_STATE_HALTED)
-                       return 1;
+               return 1;
 
        return 0;
 }

Traces also showed thet endpoint doorbell was rang after th babble error, so
we know that didn't help restarting the endpoint.

-Mathias

Reply via email to