[PATCH] usb: xhci: Fix Set TR Dequeue Pointer cycle state for quirky xHCs

[Julius Werner]

Commit 1f81b6d22 "usb: xhci: Prefer endpoint context dequeue pointer
over stopped_trb" changed the logic in xhci_find_new_dequeue_state() to
only use the Endpoint Context's TR Dequeue Pointer instead of the last
Event TRB's TRB Pointer as a basis from which to infer the host
controller state. This has uncovered a bug with certain Asmedia xHCs
which seem to advance their TR Dequeue Pointer one TRB behind the one
that caused a Stall Error.

This confuses the cycle state calculation since cur_td->last_trb is now
behind hw_dequeue (which the algorithm interprets as a single huge TD
that wraps around the whole transfer ring). This patch counteracts that
by explicitly looking for last_trb when searching for hw_dequeue from
the old software dequeue pointer. If it is found, we toggle the cycle
state once more to balance out the incorrect toggle that will happen
later.

In order to make this work for both single and multi segment rings, this
patch also expands the detection of wrap-around TDs to work on the
latter ones (which cannot normally happen because the kernel prevents
that case to allow for ring expansion, but it's how things appear to be
in the quirky case and allowing the code to handle it makes it easier to
generalize this fix across all kernels). The code will now toggle the
cycle state whenever last_trb is before hw_dequeue on the same segment,
regardless of how many segments there are in the ring.

This patch should be backported to kernels as old as 2.6.31 that contain
the commit ae636747146ea97efa18e04576acd3416e2514f5 "USB: xhci: URB
cancellation support."

Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Julius Werner <jwerner@xxxxxxxxxxxx>
---
 drivers/usb/host/xhci-ring.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 749fc68..50abc68 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -489,8 +489,17 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
 	/* Find virtual address and segment of hardware dequeue pointer */
 	state->new_deq_seg = ep_ring->deq_seg;
 	state->new_deq_ptr = ep_ring->dequeue;
+	state->new_cycle_state = hw_dequeue & 0x1;
 	while (xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr)
 			!= (dma_addr_t)(hw_dequeue & ~0xf)) {
+		/*
+		 * Quirky HCs can advance hw_dequeue behind cur_td->last_trb.
+		 * That violates the assumptions of the TD wrap around check
+		 * below, so toggle the cycle state once more to cancel it out.
+		 */
+		if (state->new_deq_ptr == cur_td->last_trb)
+			state->new_cycle_state ^= 1;
+
 		next_trb(xhci, ep_ring, &state->new_deq_seg,
 					&state->new_deq_ptr);
 		if (state->new_deq_ptr == ep_ring->dequeue) {
@@ -500,12 +509,11 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
 	}
 	/*
 	 * Find cycle state for last_trb, starting at old cycle state of
-	 * hw_dequeue. If there is only one segment ring, find_trb_seg() will
-	 * return immediately and cannot toggle the cycle state if this search
-	 * wraps around, so add one more toggle manually in that case.
+	 * hw_dequeue. If last_trb is on the current segment before hw_dequeue,
+	 * that means we wrap around the whole ring, but find_trb_seq() will
+	 * return immediately. Toggle the cycle state manually in that case.
 	 */
-	state->new_cycle_state = hw_dequeue & 0x1;
-	if (ep_ring->first_seg == ep_ring->first_seg->next &&
+	if (state->new_deq_seg->trbs < cur_td->last_trb &&
 			cur_td->last_trb < state->new_deq_ptr)
 		state->new_cycle_state ^= 0x1;
 
-- 
1.8.3.2

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