Re: USB scanner stops working with xhci_hcd URB transfer length is wrong, xHC issue? req. len = 0, act. len = 4294967288

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

 



On 07.11.2015 05:50, Orion Poplawski wrote:
On 11/06/2015 10:34 AM, Felipe Balbi wrote:

Hi,

Orion Poplawski <orion@xxxxxxxxxxxxx> writes:
See https://bugzilla.kernel.org/show_bug.cgi?id=107331

Trying to use my scanner.  Worked for a while, but now access fails.  Get this
in log:

okay, first things first. Which kernel version are you using ? Care to
capture a usbmon trace ? (see Documentation/usb/usbmon.txt for instructions)

kernel is 4.2.5-201.fc22.x86_64

Output from:

cat /sys/kernel/debug/usb/usbmon/1u > usb-scanner.log

is attached.  Thanks.


The following part from the logs says that a "Clear Endpoint Halt" request was issued
after the first returned EPROTO URB status (-71)

fff8801932f0cc0 1015040427 S Bo:1:005:3 -115 5 = 00590001 66
ffff8801932f0cc0 1015040515 C Bo:1:005:3 -71 0
ffff8801932f0cc0 1015040598 S Co:1:005:0 s 02 01 0000 0003 0000 0
ffff8801932f0cc0 1015040690 C Co:1:005:0 -71 0

after this we get only EPROTO returns.

Which brings back some earlier scanner issues we had about clearing halts for scanners
when endpoint was not really halted.

The toggles get out of sync as the device side of the endpoint gets its toggle reset
during clear endpoint halt, but host side never resets its toggle as endpoint is not
really halted.

Hmm.. turns out a proper fix was never upsrteamed. The initial fix  was inclomplete and
reverted as it had some issues.

Could you test the attached patch and see if it helps?

-Mathias


>From cb733f9daa70c8a135717d4faf48bdc0c139e654 Mon Sep 17 00:00:00 2001
From: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
Date: Tue, 24 Feb 2015 18:27:02 +0200
Subject: [PATCH] xhci: Clear the host side toggle manually when endpoint is
 'soft reset'

Main benefit of this is to get xhci connected USB scanners to work.

Some devices use a clear endpoint halt request as a 'soft reset' even if
the endpoint is not halted. This will clear the toggle and sequence on the
device side. xHCI however refuses to reset a non-halted endpoint, so instead
we need to issue a configure endpoint command on xHCI to clear its host side
toggle and sequence, and get it in sync with the device side.

Make sure the endpoint dequeue pointer is the latest one so that the ring start
from the correct position. The one in the endpoint output context is stale.

Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
---
 drivers/usb/host/xhci-ring.c |   2 +-
 drivers/usb/host/xhci.c      | 106 +++++++++++++++++++++++++++++++++++++++----
 drivers/usb/host/xhci.h      |   2 +
 3 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index fa83625..8dbd6a0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1741,7 +1741,7 @@ static void xhci_cleanup_halted_endpoint(struct xhci_hcd *xhci,
 	if (!command)
 		return;
 
-	ep->ep_state |= EP_HALTED;
+	ep->ep_state |= EP_HALTED | EP_RECENTLY_HALTED;
 	ep->stopped_stream = stream_id;
 
 	xhci_queue_reset_ep(xhci, command, slot_id, ep_index);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6e7dc6f..eb1d9f3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1333,6 +1333,12 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
 		goto exit;
 	}
 
+	/* Reject urb if endpoint is in soft reset, queue must stay empty */
+	if (xhci->devs[slot_id]->eps[ep_index].ep_state & EP_CONFIG_PENDING) {
+		xhci_warn(xhci, "Can't enqueue URB while ep is in soft reset\n");
+		ret = -EINVAL;
+	}
+
 	if (usb_endpoint_xfer_isoc(&urb->ep->desc))
 		size = urb->number_of_packets;
 	else if (usb_endpoint_is_bulk_out(&urb->ep->desc) &&
@@ -2948,23 +2954,38 @@ void xhci_cleanup_stalled_ring(struct xhci_hcd *xhci,
 	}
 }
 
-/* Called when clearing halted device. The core should have sent the control
+/* Called after clearing a halted device. USB core should have sent the control
  * message to clear the device halt condition. The host side of the halt should
- * already be cleared with a reset endpoint command issued when the STALL tx
- * event was received.
- *
- * Context: in_interrupt
+ * already be cleared with a reset endpoint command issued immediately when the
+ * STALL tx event was received.
  */
 
 void xhci_endpoint_reset(struct usb_hcd *hcd,
 		struct usb_host_endpoint *ep)
 {
 	struct xhci_hcd *xhci;
+	struct usb_device *udev;
+	struct xhci_virt_device *virt_dev;
+	struct xhci_virt_ep *virt_ep;
+	struct xhci_input_control_ctx *ctrl_ctx;
+	struct xhci_command *command;
+	unsigned int ep_index, ep_state;
+	unsigned long flags;
+	u32 ep_flag;
+	struct xhci_ep_ctx *ep_ctx;
+	dma_addr_t addr;
 
 	xhci = hcd_to_xhci(hcd);
+	udev = (struct usb_device *) ep->hcpriv;
+	if (!ep->hcpriv)
+		return;
+	virt_dev = xhci->devs[udev->slot_id];
+	ep_index = xhci_get_endpoint_index(&ep->desc);
+	virt_ep = &virt_dev->eps[ep_index];
+	ep_state = virt_ep->ep_state;
 
 	/*
-	 * We might need to implement the config ep cmd in xhci 4.8.1 note:
+	 * Implement the config ep command in xhci 4.6.8 additional note:
 	 * The Reset Endpoint Command may only be issued to endpoints in the
 	 * Halted state. If software wishes reset the Data Toggle or Sequence
 	 * Number of an endpoint that isn't in the Halted state, then software
@@ -2972,9 +2993,76 @@ void xhci_endpoint_reset(struct usb_hcd *hcd,
 	 * for the target endpoint. that is in the Stopped state.
 	 */
 
-	/* For now just print debug to follow the situation */
-	xhci_dbg(xhci, "Endpoint 0x%x ep reset callback called\n",
-		 ep->desc.bEndpointAddress);
+	if (ep_state & SET_DEQ_PENDING || ep_state & EP_RECENTLY_HALTED) {
+		virt_ep->ep_state &= ~EP_RECENTLY_HALTED;
+		xhci_dbg(xhci, "ep recently halted, no toggle reset needed\n");
+		return;
+	}
+
+	/* Only interrupt and bulk ep's use Data toggle, USB2 spec 5.5.4-> */
+	if (usb_endpoint_xfer_control(&ep->desc) ||
+	    usb_endpoint_xfer_isoc(&ep->desc))
+		return;
+
+	ep_flag = xhci_get_endpoint_flag(&ep->desc);
+
+	if (ep_flag == SLOT_FLAG || ep_flag == EP0_FLAG)
+		return;
+
+	command = xhci_alloc_command(xhci, true, true, GFP_NOWAIT);
+	if (!command) {
+		xhci_err(xhci, "Could not allocate xHCI command structure.\n");
+		return;
+	}
+
+	spin_lock_irqsave(&xhci->lock, flags);
+
+	/* block ringing ep doorbell */
+	virt_ep->ep_state |= EP_CONFIG_PENDING;
+
+	/*
+	 * Make sure endpoint ring is empty before resetting the toggle/seq.
+	 * Driver is required to synchronously cancel all transfer request.
+	 *
+	 * xhci 4.6.6 says we can issue a configure endpoint command on a
+	 * running endpoint ring as long as it's idle (queue empty)
+	 */
+
+	if (!list_empty(&virt_ep->ring->td_list)) {
+		dev_err(&udev->dev, "EP not empty, refuse reset\n");
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		goto cleanup;
+	}
+
+	xhci_dbg(xhci, "Reset toggle/seq for slot %d, ep_index: %d\n",
+		 udev->slot_id, ep_index);
+
+	ctrl_ctx = xhci_get_input_control_ctx(command->in_ctx);
+	if (!ctrl_ctx) {
+		xhci_err(xhci, "Could not get input context, bad type. virt_dev: %p, in_ctx %p\n",
+			 virt_dev, virt_dev->in_ctx);
+		spin_unlock_irqrestore(&xhci->lock, flags);
+		goto cleanup;
+	}
+	xhci_setup_input_ctx_for_config_ep(xhci, command->in_ctx,
+					   virt_dev->out_ctx, ctrl_ctx,
+					   ep_flag, ep_flag);
+	xhci_endpoint_copy(xhci, command->in_ctx, virt_dev->out_ctx, ep_index);
+
+	ep_ctx = xhci_get_ep_ctx(xhci, command->in_ctx, ep_index);
+	addr = xhci_trb_virt_to_dma(virt_ep->ring->deq_seg, virt_ep->ring->dequeue);
+	ep_ctx->deq = cpu_to_le64(addr | virt_ep->ring->cycle_state);
+
+	xhci_queue_configure_endpoint(xhci, command, command->in_ctx->dma,
+				     udev->slot_id, false);
+	xhci_ring_cmd_db(xhci);
+	spin_unlock_irqrestore(&xhci->lock, flags);
+
+	wait_for_completion(command->completion);
+
+cleanup:
+	virt_ep->ep_state &= ~EP_CONFIG_PENDING;
+	xhci_free_command(xhci, command);
 }
 
 static int xhci_check_streams_endpoint(struct xhci_hcd *xhci,
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index be9048e..05bc89d 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -915,6 +915,8 @@ 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_RECENTLY_HALTED	(1 << 6)
+#define EP_CONFIG_PENDING	(1 << 7)
 	/* ----  Related to URB cancellation ---- */
 	struct list_head	cancelled_td_list;
 	struct xhci_td		*stopped_td;
-- 
1.9.1


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

  Powered by Linux