[PATCH 1/2] xhci: fix reset for not halted endpoints

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



If a Reset Endpoint command is issued for an endpoint that is not in
the Halted state, xHC does nothing. Since the current implementation of
xhci_endpoint_reset() aborts any resets that address not halted endpoints,
the synchronization between the host-side and device-side toggle or sequence
number is lost.
This patch attempts to fix this issue by dropping and readding the endpoint
in order to enforce xHC to reset its internal toggle/sequence number.

Four new functions have been introduced to prevent xhci_endpoint_reset() from
becoming huge.

xhci_unlink_urbs(xhci, ring):
	unlinks the urbs from the endpoint's rings

xhci_stop_endpoint_for_config(xhci, udev, ep index):
	brings the endpoint to the Stopped state and calls xhci_unlink_urbs
	to unlink any queued urbs

xhci_reset_ep0(xhci, udev, ep):
	calls xhci_stop_endpoint_for_config to stop the endpoint and then
	reinitializes the ring and input context for the default control
	endpoint and issues an Evaluate Context command to reset xHC toggle

xhci_reset_not_halted_ep(xhci, udev, ep):
	calls xhci_stop_endpoint_for_config and if the endpoint to be reset is
	the endpoint 0 calls xhci_reset_ep0, otherwise reinitializes the
	endpoint rings and input context and then drops and readds the endpoint
	issueing a Configure Endpoint

Also, this patch introduces a new trace event for debugging output contexts
during the process of resetting the endpoint. At this early stage, it is
useful for debugging the xhci_reset_not_halted_ep() and it may be removed
later when further testing and revisioning show that the function works as
expected.

Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
---

This patch needs to be tested further because I did not find a satisfying
way to test it. I would appreciate if the device driver writers that had
problems with the xhci reset endpoint in the past could test it and let me
know if it fixes the problem or not.

 drivers/usb/host/xhci-ring.c  |  17 +++-
 drivers/usb/host/xhci-trace.h |   6 ++
 drivers/usb/host/xhci.c       | 210 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/xhci.h       |   3 +
 4 files changed, 233 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index faff6fc..f62131b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -433,6 +433,10 @@ static void ring_doorbell_for_active_rings(struct xhci_hcd *xhci,
 
 	ep = &xhci->devs[slot_id]->eps[ep_index];
 
+	/* If there is a pending configuration, don't ring the doorbell yet */
+	if (ep->ep_state & EP_CONFIG_PENDING)
+		return;
+
 	/* A ring has pending URBs if its TD list is not empty */
 	if (!(ep->ep_state & EP_HAS_STREAMS)) {
 		if (ep->ring && !(list_empty(&ep->ring->td_list)))
@@ -715,8 +719,8 @@ static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci,
 }
 
 /* Must be called with xhci->lock held in interrupt context */
-static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
-		struct xhci_td *cur_td, int status)
+void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, struct xhci_td *cur_td,
+		int status)
 {
 	struct usb_hcd *hcd;
 	struct urb	*urb;
@@ -788,6 +792,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id,
 		xhci_stop_watchdog_timer_in_irq(xhci, ep);
 		ep->stopped_td = NULL;
 		ep->stopped_trb = NULL;
+		if (ep->ep_state & EP_CONFIG_PENDING)
+			goto signal_completion;
 		ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
 		return;
 	}
@@ -881,6 +887,13 @@ remove_finished_td:
 			return;
 	} while (cur_td != last_unlinked_td);
 
+signal_completion:
+	/* signal the completion for the Stop Endpoint command */
+	if (ep->ep_state & EP_CONFIG_PENDING) {
+		virt_dev = xhci->devs[slot_id];
+		handle_cmd_in_cmd_wait_list(xhci, virt_dev, event);
+	}
+
 	/* Return to the event handler with xhci->lock re-acquired */
 }
 
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 132abda..1f8b3fb 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -113,6 +113,12 @@ DEFINE_EVENT(xhci_log_ctx, xhci_address_ctx,
 	TP_ARGS(xhci, ctx, ep_num)
 );
 
+DEFINE_EVENT(xhci_log_ctx, xhci_resetep_ctx,
+	TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
+		 unsigned int ep_num),
+	TP_ARGS(xhci, ctx, ep_num)
+);
+
 DECLARE_EVENT_CLASS(xhci_log_event,
 	TP_PROTO(void *trb_va, struct xhci_generic_trb *ev),
 	TP_ARGS(trb_va, ev),
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 9f22ddf..e6300b5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -2873,6 +2873,210 @@ void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci,
 	}
 }
 
+static void xhci_unlink_urbs(struct xhci_hcd *xhci, struct xhci_ring *ring)
+{
+	unsigned long flags;
+	struct xhci_td *cur_td;
+
+	spin_lock_irqsave(&xhci->lock, flags);
+	while (!list_empty(&ring->td_list)) {
+		cur_td = list_first_entry(&ring->td_list, struct xhci_td,
+						td_list);
+		cur_td->urb->transfer_buffer_length = 0;
+		xhci_giveback_urb_in_irq(xhci, cur_td, -EPROTO);
+		list_del_init(&cur_td->td_list);
+	}
+	spin_unlock_irqrestore(&xhci->lock, flags);
+}
+
+static int xhci_stop_endpoint_for_config(struct xhci_hcd *xhci,
+		struct usb_device *udev, unsigned int ep_index)
+{
+	struct xhci_virt_device *vdev;
+	struct xhci_virt_ep *virt_ep;
+	struct xhci_ep_ctx *ep_ctx_out;
+	unsigned int ep_state;
+	struct xhci_command *stop_ep_cmd;
+	unsigned long flags;
+	int ret, timeleft;
+
+	vdev = xhci->devs[udev->slot_id];
+	virt_ep = &vdev->eps[ep_index];
+
+	ep_ctx_out = xhci_get_ep_ctx(xhci, vdev->out_ctx, ep_index);
+	ep_state = le32_to_cpu(ep_ctx_out->ep_info) & EP_STATE_MASK;
+	if (ep_state == EP_STATE_STOPPED)
+		return 0;
+
+	/* set EP_CONFIG_PENDING flag to prevent xhci from ringing the
+	 * doorbell for this endpoint's rings, when it handles the Stop
+	 * Endpoint command. */
+	spin_lock_irqsave(&xhci->lock, flags);
+	virt_ep->ep_state |= EP_CONFIG_PENDING;
+	spin_unlock_irqrestore(&xhci->lock, flags);
+
+	stop_ep_cmd = xhci_alloc_command(xhci, false, true, GFP_NOIO);
+	if (!stop_ep_cmd) {
+		xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
+			"%s: Failed to allocate stop ep cmd", __func__);
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	spin_lock_irqsave(&xhci->lock, flags);
+	stop_ep_cmd->command_trb = xhci->cmd_ring->enqueue;
+	ret = xhci_queue_stop_endpoint(xhci, udev->slot_id, ep_index, 0);
+	if (ret) {
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
+			"%s: Failed to queue stop ep cmd", __func__);
+		goto cleanup_cmd;
+	}
+	list_add_tail(&stop_ep_cmd->cmd_list, &vdev->cmd_list);
+	xhci_ring_cmd_db(xhci);
+	spin_unlock_irqrestore(&xhci->lock, flags);
+
+	timeleft = wait_for_completion_interruptible_timeout(
+		stop_ep_cmd->completion, XHCI_CMD_DEFAULT_TIMEOUT);
+	if (timeleft <= 0) {
+		ep_state = le32_to_cpu(ep_ctx_out->ep_info) & EP_STATE_MASK;
+		if (ep_state != EP_STATE_STOPPED) {
+			spin_lock_irqsave(&xhci->lock, flags);
+			if (stop_ep_cmd->cmd_list.next != LIST_POISON1)
+				list_del(&stop_ep_cmd->cmd_list);
+			spin_unlock_irqrestore(&xhci->lock, flags);
+			ret = -ETIME;
+		}
+	}
+
+	/* unlink urbs queued to the endpoint */
+	if (!(virt_ep->ep_state & EP_HAS_STREAMS)) {
+		xhci_unlink_urbs(xhci, virt_ep->ring);
+	} else {
+		struct xhci_stream_info *stream_info = virt_ep->stream_info;
+		unsigned int stream;
+
+		for (stream = 1; stream < stream_info->num_streams; ++stream) {
+			xhci_unlink_urbs(xhci,
+				stream_info->stream_rings[stream]);
+		}
+	}
+
+cleanup_cmd:
+	xhci_free_command(xhci, stop_ep_cmd);
+
+out:
+	spin_lock_irqsave(&xhci->lock, flags);
+	virt_ep->ep_state &= ~EP_CONFIG_PENDING;
+	spin_unlock_irqrestore(&xhci->lock, flags);
+
+	return ret;
+}
+
+static int xhci_reset_ep0(struct xhci_hcd *xhci, struct usb_device *udev,
+		struct usb_host_endpoint *ep)
+{
+	struct xhci_virt_device *vdev;
+	struct xhci_input_control_ctx *ctrl_ctx;
+	struct xhci_slot_ctx *slot_ctx_out;
+	int ret;
+
+	vdev = xhci->devs[udev->slot_id];
+
+	/* reinitialize endpoint's ring and setup its input context based
+	 * on its descriptors */
+	if (xhci_endpoint_init(xhci, vdev, udev, ep, GFP_NOIO) < 0) {
+		xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
+				"%s: failed to initialize ep 0", __func__);
+		return -ENOMEM;
+	}
+
+	/* issue an evaluate context command */
+	ctrl_ctx = xhci_get_input_control_ctx(xhci, vdev->in_ctx);
+	if (!ctrl_ctx) {
+		xhci_warn(xhci, "%s: Could not get input context, bad type.\n",
+				__func__);
+
+		return -EINVAL;
+	}
+	ctrl_ctx->add_flags = cpu_to_le32(EP0_FLAG | SLOT_FLAG);
+	ctrl_ctx->drop_flags = 0;
+
+	ret = xhci_configure_endpoint(xhci, udev, NULL, true, false);
+
+	xhci_zero_in_ctx(xhci, vdev);
+
+	slot_ctx_out = xhci_get_slot_ctx(xhci, vdev->out_ctx);
+	trace_xhci_resetep_ctx(xhci, vdev->out_ctx,
+	       LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx_out->dev_info)) + 1);
+
+	return ret;
+}
+
+
+/* The Reset Endpoint command cannot be issued for an endpoint that is not in
+ * the Halted state. This function brings the endpoint in Stopped state,
+ * unlinks any urbs queued to this endpoint and issues a Configure Endpoint
+ * command to drop and re-add the endpoint.
+ * By dropping and readding the endpoint, xHC is forced to reset its USB2
+ * data toggle or USB3 sequence number and the synchronization between the
+ * host-side and the device-side toggle bit/sequence number is maintained.
+ * Endpoint 0 does not apply to the Configure Endpoint command and will be
+ * ignored by the xHC. Hence, for endpoint 0 we need to issue an Evaluate
+ * Context command instead.
+ */
+static int xhci_reset_not_halted_ep(struct xhci_hcd *xhci,
+		struct usb_device *udev, struct usb_host_endpoint *ep)
+{
+	struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+	struct xhci_virt_device *vdev;
+	struct xhci_slot_ctx *slot_ctx_out;
+	unsigned int ep_index;
+	int ret;
+
+	vdev = xhci->devs[udev->slot_id];
+	ep_index = xhci_get_endpoint_index(&ep->desc);
+
+	/* bring endpoint in Stopped state and unlink urbs from its rings */
+	xhci_stop_endpoint_for_config(xhci, udev, ep_index);
+
+	slot_ctx_out = xhci_get_slot_ctx(xhci, vdev->out_ctx);
+	trace_xhci_resetep_ctx(xhci, vdev->out_ctx,
+	       LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx_out->dev_info)) + 1);
+
+	/* for endpoint 0 we need to issue an Evaluate Context command */
+	if (ep_index == 0)
+		return xhci_reset_ep0(xhci, udev, ep);
+
+	/* setup input control flags to drop and re-add endpoint */
+	ret = xhci_drop_endpoint(hcd, udev, ep);
+	if (ret < 0) {
+		xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
+			       "%s: Failed to drop ep %d", __func__, ep_index);
+		return ret;
+	}
+	ret = xhci_add_endpoint(hcd, udev, ep);
+	if (ret < 0) {
+		xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
+			       "%s: Failed to add ep %d", __func__, ep_index);
+		goto reset_bw;
+	}
+
+	/* issue a Configure Endpoint command */
+	ret = xhci_check_bandwidth(hcd, udev);
+
+reset_bw:
+	if (ret < 0)
+		xhci_reset_bandwidth(hcd, udev);
+
+	slot_ctx_out = xhci_get_slot_ctx(xhci, vdev->out_ctx);
+	trace_xhci_resetep_ctx(xhci, vdev->out_ctx,
+	       LAST_CTX_TO_EP_NUM(le32_to_cpu(slot_ctx_out->dev_info)) + 1);
+
+	return ret;
+}
+
+
 /* Deal with stalled endpoints.  The core should have sent the control message
  * to clear the halt condition.  However, we need to make the xHCI hardware
  * reset its sequence number, since a device will expect a sequence number of
@@ -2900,8 +3104,12 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
 	virt_ep = &xhci->devs[udev->slot_id]->eps[ep_index];
 	if (!virt_ep->stopped_td) {
 		xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
-			"Endpoint 0x%x not halted, refusing to reset.",
+			"reset not halted endpoint 0x%x",
 			ep->desc.bEndpointAddress);
+		ret = xhci_reset_not_halted_ep(xhci, udev, ep);
+		if (ret < 0)
+			xhci_dbg_trace(xhci, trace_xhci_dbg_reset_ep,
+				"reset for not halted ep failed");
 		return;
 	}
 	if (usb_endpoint_xfer_control(&ep->desc)) {
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 9575088..eaaa0af 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -861,6 +861,7 @@ struct xhci_virt_ep {
 #define EP_HAS_STREAMS		(1 << 4)
 /* Transitioning the endpoint to not using streams, don't enqueue URBs */
 #define EP_GETTING_NO_STREAMS	(1 << 5)
+#define EP_CONFIG_PENDING	(1 << 6)
 	/* ----  Related to URB cancellation ---- */
 	struct list_head	cancelled_td_list;
 	/* The TRB that was last reported in a stopped endpoint ring */
@@ -1840,6 +1841,8 @@ int xhci_cancel_cmd(struct xhci_hcd *xhci, struct xhci_command *command,
 		union xhci_trb *cmd_trb);
 void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, unsigned int slot_id,
 		unsigned int ep_index, unsigned int stream_id);
+void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, struct xhci_td *cur_td,
+		int status);
 
 /* xHCI roothub code */
 void xhci_set_link_state(struct xhci_hcd *xhci, __le32 __iomem **port_array,
-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux