On 7.3.2025 17.44, Michał Pecio wrote:
On Fri, 7 Mar 2025 16:23:17 +0200, Mathias Nyman wrote:
Any flag added to this list needs to be added to xhci_urb_dequeue()
too so it knowns that the endpoint is held in Stopped state and
URBs can be unlinked without trying to stop it again.
In this case it's intentional.
If we prevent xhci_urb_dequeue() from queuing a stop endpoint command
due to a flag, then we must make sure the cancelled URB is given back
in the same place we clear the flag, like we do in the command
completion handlers that clear EP_HALTED and SET_DEQ_PENDING.
I'm not sure why this would be, what's the problem with the approach
used for EP_CLEARING_TT currently? And if there is a problem, doesn't
EP_CLEARING_TT also have this problem?
In this case, xhci_urb_dequeue() simply takes xhci->lock and calls:
void xhci_process_cancelled_tds(struct xhci_virt_ep *ep)
{
xhci_invalidate_cancelled_tds(ep);
xhci_giveback_invalidated_tds(ep);
}
Unlinked URBs are either given back instantly, or Set TR Dequeue is
queued (and flagged on ep->ep_state) and the rest of the process goes
same way as usual when called from xhci_handle_cmd_stop_ep().
The EP will be restarted when the last flag is cleared, which may be
either SET_DEQ_PENDING or EP_CLEARING_TT/EP_STALLED.
It's practically an optimization which eliminates the dummy Stop EP
command from the process. I thought EP_STALLED could use it.
This should work, and avoid that unnecessary stop endpoint command.
Just need to make sure we check for EP_STALLED flag after the other
(EP_STOP_CMD_PENDING | EP_HALTED | SET_DEQ_PENDING) flags in
xhci_urb_dequeue(), just like EP_CLEARING_TT case.
Also need to protect clearing the EP_STALLED flag with the lock
I'll either send an update patch next week, or during rc cycle if
that's too late.
Thanks
Mathias
