xhci_ring_endpoint_doorbell() needs a list of flags which prohibit running the endpoint. xhci_urb_dequeue() needs the same list, split in two parts, to know whether the endpoint is running and how to cancel TDs. Define the two partial lists in xhci.h and use them in both functions. Add a comment about the AMD Stop Endpoint bug, see commit 28a2369f7d72 ("usb: xhci: Issue stop EP command only when the EP state is running") Signed-off-by: Michal Pecio <michal.pecio@xxxxxxxxx> --- drivers/usb/host/xhci-ring.c | 10 ++-------- drivers/usb/host/xhci.c | 16 +++++++++++----- drivers/usb/host/xhci.h | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0f8acbb9cd21..8aab077d6183 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -555,14 +555,8 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, struct xhci_virt_ep *ep = &xhci->devs[slot_id]->eps[ep_index]; unsigned int ep_state = ep->ep_state; - /* Don't ring the doorbell for this endpoint if there are pending - * cancellations because we don't want to interrupt processing. - * We don't want to restart any stream rings if there's a set dequeue - * pointer command pending because the device can choose to start any - * stream once the endpoint is on the HW schedule. - */ - if (ep_state & (EP_STOP_CMD_PENDING | SET_DEQ_PENDING | EP_HALTED | - EP_CLEARING_TT | EP_STALLED)) + /* Don't start yet if certain endpoint operations are ongoing */ + if (ep_state & (EP_CANCEL_PENDING | EP_MISC_OPS_PENDING)) return; trace_xhci_ring_ep_doorbell(slot_id, DB_VALUE(ep_index, stream_id)); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 7492090fad5f..c33134a3003a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1762,16 +1762,22 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) } } - /* These completion handlers will sort out cancelled TDs for us */ - if (ep->ep_state & (EP_STOP_CMD_PENDING | EP_HALTED | SET_DEQ_PENDING)) { + /* + * We have a few strategies to give back the cancelled TDs. If the endpoint is running, + * no other choice - it must be stopped. But if it's not, we avoid queuing Stop Endpoint + * because this triggers a bug in "AMD SNPS 3.1 xHC" and because our completion handler + * is complex enough already without having to worry about such things. + */ + + /* If cancellation is already running, giveback of all cancelled TDs is guaranteed */ + if (ep->ep_state & EP_CANCEL_PENDING) { xhci_dbg(xhci, "Not queuing Stop Endpoint on slot %d ep %d in state 0x%x\n", urb->dev->slot_id, ep_index, ep->ep_state); goto done; } - /* In this case no commands are pending but the endpoint is stopped */ - if (ep->ep_state & (EP_CLEARING_TT | EP_STALLED)) { - /* and cancelled TDs can be given back right away */ + /* Cancel immediately if no commands are pending but the endpoint is held stopped */ + if (ep->ep_state & EP_MISC_OPS_PENDING) { xhci_dbg(xhci, "Invalidating TDs instantly on slot %d ep %d in state 0x%x\n", urb->dev->slot_id, ep_index, ep->ep_state); xhci_process_cancelled_tds(ep); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 46bbdc97cc4b..87d87ed08b8b 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -718,9 +718,23 @@ struct xhci_virt_ep { * xhci_ring_ep_doorbell() inspects the flags to decide if the endpoint can be restarted. Another * user is xhci_urb_dequeue(), which must not attempt to stop a Stopped endpoint, due to HW bugs. * An endpoint with pending URBs and no flags preventing restart must be Running for this to work. - * Call xhci_ring_doorbell_for_active_rings() or similar after clearing any such flag. + * Call xhci_ring_doorbell_for_active_rings() or similar after clearing flags on the lists below. */ +/* + * TD cancellation is in progress. New TDs can be marked as cancelled without further action and + * indeed no such action is possible until these commands complete. Their handlers must check for + * more cancelled TDs and continue until all are given back. The endpoint must not be restarted. + */ +#define EP_CANCEL_PENDING (SET_DEQ_PENDING | EP_HALTED | EP_STOP_CMD_PENDING) + +/* + * Some other operations are pending which preclude restarting the endpoint. If the endpoint isn't + * transitioning to the Stopped state, it has already reached this state and stays in it. + */ +#define EP_MISC_OPS_PENDING (EP_CLEARING_TT | EP_STALLED) + + enum xhci_overhead_type { LS_OVERHEAD_TYPE = 0, FS_OVERHEAD_TYPE, -- 2.48.1