Re: XHCI vs PCM2903B/PCM2904 part 2

[Mathias Nyman <mathias.nyman@linux.intel.com>]

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