Re: [PATCH 1/2] usb: xhci: Fix the NEC stop bug workaround

Michał Pecio
2024-10-15T13:27:55Z

> Can we skip the new flag and just check for the correct flags here
> directly?
>
> if (ep->ep_state & (SET_DEQ_PENDING | EP_HALTED | EP_CLEARING_TT)
> break;

Unfortunately not, because those pending operations may (and usually
will) complete before our handler runs. They will not restart the EP
because we set EP_STOP_CMD_PENDING, but they will clear their flags.
So we know that Stop Endpoint is guaranteed to fail, but its handler
will not see those flags and will have no clue why it failed, hence
we store this one bit of knowledge specially for its use.

But you raise a valid point. If Stop EP fails on a Halted endpoint and
somebody else resets it before Stop EP handler runs, the handler will
see EP_HALTED, because Reset EP handler must run later if the commands
were queued and executed in this order.

So if Stop EP handler tests for EP_HALTED, nobody needs to worry about
updating EP_STOP_CMD_REDUNDANT for us. The helper function can go out,
the patch is shorter, and the solution more robust against any changes
to halt recovery code that anyone might do. All that "redundant" logic
becomes concentrated in queue/handle _stop_endpoint() functions.

I think I will do a v2.

By the way, is this list of conditions complete? There are other flags
like GETTING_STREAMS or CLEAR_TOGGLE, but I'm under impression that they
are valid only with no queued URBs, so nothing can be cancelled then.

Regards,
Michal