The NEC uPD720200 has a bug which prevents reliably stopping
an endpoint shortly after it has been restarted. This usually
happens when a driver kills many URBs in quick succession and
it results in concurrent execution and cancellation of TDs.
This is handled by stopping the endpoint again if in doubt.
This "doubt" turns out to be a problem, because Stop Endpoint
may be queued when the EP is already Stopped (for Set TR Deq
execution, for example) or becomes Stopped concurrently (by
Reset Endpoint, for example). If the EP is truly Stopped, the
command fails and further retries just keep failing forever.
This is easily triggered by modifying uvcvideo to unlink its
isochronous URBs in 100us intervals instead of poisoning them.
Any driver that unlinks URBs asynchronously may trigger this,
and any URB unlink during ongoing halt recovery also can.
Fix the problem by flagging redundant Stop Endpoint commands
which are sure to fail, and by not retrying them. It's easy,
because xhci_urb_dequeue() is the only user ever queuing the
command with the default handler which can't guarantee that
the endpoint is Running and will not Halt before it Stops.
For this case, we assume that an endpoint with pending URBs
is always Running, unless certain operations are pending on
it which indicate known exceptions.
Also detect if a Halted EP was reset concurrently after our
command failed. As command handlers run in order, EP_HALTED
would still be set in this case, so simply test for it.
It's possible that other HCs have similar bugs (see also the
related "Running" case below), but the workaround is limited
to NEC chips, where it is known to give good results.
Fixes: fd9d55d190c0 ("xhci: retry Stop Endpoint on buggy NEC controllers")
CC: stable@xxxxxxxxxxxxxxx
Signed-off-by: Michal Pecio <michal.pecio@xxxxxxxxx>
---
drivers/usb/host/xhci-ring.c | 45 +++++++++++++++++++++++++++++++++---
drivers/usb/host/xhci.h | 2 ++
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b6eb928e260f..1441c196a5c0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1149,15 +1149,37 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
break;
ep->ep_state &= ~EP_STOP_CMD_PENDING;
return;
+
case EP_STATE_STOPPED:
/*
- * NEC uPD720200 sometimes sets this state and fails with
- * Context Error while continuing to process TRBs.
- * Be conservative and trust EP_CTX_STATE on other chips.
+ * Per xHCI 4.6.9, Stop Endpoint command on a Stopped
+ * EP is a Context State Error, and EP stays Stopped.
+ * This outcome is valid if the command was redundant.
+ */
+ if (ep->ep_state & EP_STOP_CMD_REDUNDANT)
+ break;
+ /*
+ * Or it really failed on Halted, but somebody ran Reset
+ * Endpoint later. EP state is now Stopped and EP_HALTED
+ * still set because Reset EP handler will run after us.
+ */
+ if (ep->ep_state & EP_HALTED)
+ break;
+ /*
+ * On some HCs EP state remains Stopped for some tens of
+ * us to a few ms or more after a doorbell ring, and any
+ * new Stop Endpoint fails without aborting the restart.
+ * This handler may run quickly enough to still see this
+ * Stopped state, but it will soon change to Running.
+ *
+ * Assume this bug on unexpected Stop Endpoint failures.
+ * Keep retrying until the EP starts and stops again, on
+ * chips known to have the bug and to react positively.
*/
if (!(xhci->quirks & XHCI_NEC_HOST))
break;
fallthrough;
+
case EP_STATE_RUNNING:
/* Race, HW handled stop ep cmd before ep was running */
xhci_dbg(xhci, "Stop ep completion ctx error, ep is running\n");
@@ -4375,11 +4397,28 @@ int xhci_queue_evaluate_context(struct xhci_hcd *xhci, struct xhci_command *cmd,
int xhci_queue_stop_endpoint(struct xhci_hcd *xhci, struct xhci_command *cmd,
int slot_id, unsigned int ep_index, int suspend)
{
+ struct xhci_virt_ep *ep;
u32 trb_slot_id = SLOT_ID_FOR_TRB(slot_id);
u32 trb_ep_index = EP_INDEX_FOR_TRB(ep_index);
u32 type = TRB_TYPE(TRB_STOP_RING);
u32 trb_suspend = SUSPEND_PORT_FOR_TRB(suspend);
+ /*
+ * Any of that means the EP is or will be Stopped by the time this
+ * command runs. Queue it anyway for its side effects like calling
+ * our default handler or complete(). But our handler must know if
+ * the command is redundant, so check it now. The handler can't do
+ * it later because those operations may finish before it runs.
+ */
+ ep = xhci_get_virt_ep(xhci, slot_id, ep_index);
+ if (ep) {
+ if (ep->ep_state & (SET_DEQ_PENDING | EP_HALTED | EP_CLEARING_TT))
+ ep->ep_state |= EP_STOP_CMD_REDUNDANT;
+ else
+ ep->ep_state &= ~EP_STOP_CMD_REDUNDANT;
+ }
+ /* else: don't care, the handler will do nothing anyway */
+
return queue_command(xhci, cmd, 0, 0, 0,
trb_slot_id | trb_ep_index | type | trb_suspend, false);
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index f0fb696d5619..4db2cb843a8b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -670,6 +670,8 @@ struct xhci_virt_ep {
#define EP_SOFT_CLEAR_TOGGLE (1 << 7)
/* usb_hub_clear_tt_buffer is in progress */
#define EP_CLEARING_TT (1 << 8)
+/* queued Stop Endpoint is expected to fail */
+#define EP_STOP_CMD_REDUNDANT (1 << 9)
/* ---- Related to URB cancellation ---- */
struct list_head cancelled_td_list;
struct xhci_hcd *xhci;
--
2.43.0
