Only TDs with status TD_CLEARING_CACHE will be given back after cache is cleared with a set TR deq command. xhci_invalidate_cached_td() failed to set the TD_CLEARING_CACHE status for some cancelled TDs as it assumed an endpoint only needs to clear the TD it stopped on from cache. There are some cases this isn't true. For example with streams as an endpoint may have several stream rings, each stopping on different TDs. * FIXME *, explain that streams case isn't fully solved by this, but it's x100 better than before as we give back the URBs and thread won't hang. Some of the streams TRB cache still isn't cleared, and may point to data in URBs we alrady gave back. xHC controller may touch this. Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> --- drivers/usb/host/xhci-ring.c | 40 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index d0faa67a689d..9017986241f5 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -942,17 +942,21 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) td->urb->stream_id); hw_deq &= ~0xf; - if (td->cancel_status == TD_HALTED) { - cached_td = td; - } else if (trb_in_td(xhci, td->start_seg, td->first_trb, - td->last_trb, hw_deq, false)) { + if (td->cancel_status == TD_HALTED || + trb_in_td(xhci, td->start_seg, td->first_trb, td->last_trb, hw_deq, false)) { switch (td->cancel_status) { case TD_CLEARED: /* TD is already no-op */ case TD_CLEARING_CACHE: /* set TR deq command already queued */ break; case TD_DIRTY: /* TD is cached, clear it */ case TD_HALTED: - /* FIXME stream case, several stopped rings */ + td->cancel_status = TD_CLEARING_CACHE; + if (cached_td) + /* FIXME stream case, several stopped rings */ + xhci_dbg(xhci, + "Move dq past stream %u URB %p instead of stream %u URB %p\n", + td->urb->stream_id, td->urb, + cached_td->urb->stream_id, cached_td->urb); cached_td = td; break; } @@ -961,18 +965,24 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) td->cancel_status = TD_CLEARED; } } - if (cached_td) { - cached_td->cancel_status = TD_CLEARING_CACHE; - err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index, - cached_td->urb->stream_id, - cached_td); - /* Failed to move past cached td, try just setting it noop */ - if (err) { - td_to_noop(xhci, ring, cached_td, false); - cached_td->cancel_status = TD_CLEARED; + /* If there's no need to move the dequeue pointer then we're done */ + if (!cached_td) + return 0; + + err = xhci_move_dequeue_past_td(xhci, slot_id, ep->ep_index, + cached_td->urb->stream_id, + cached_td); + if (err) { + /* Failed to move past cached td, just set cached TDs to no-op */ + list_for_each_entry_safe(td, tmp_td, &ep->cancelled_td_list, cancelled_td_list) { + if (td->cancel_status != TD_CLEARING_CACHE) + continue; + xhci_dbg(xhci, "Failed to clear cancelled cached URB %p, mark clear anyway\n", + td->urb); + td_to_noop(xhci, ring, td, false); + td->cancel_status = TD_CLEARED; } - cached_td = NULL; } return 0; } -- 2.25.1