Skip to content

Commit 6328bdc

Browse files
Michal Peciogregkh
authored andcommitted
usb: xhci: Don't trust the EP Context cycle bit when moving HW dequeue
VIA VL805 doesn't bother updating the EP Context cycle bit when the endpoint halts. This is seen by patching xhci_move_dequeue_past_td() to print the cycle bits of the EP Context and the TRB at hw_dequeue and then disconnecting a flash drive while reading it. Actual cycle state is random as expected, but the EP Context bit is always 1. This means that the cycle state produced by this function is wrong half the time, and then the endpoint stops working. Work around it by looking at the cycle bit of TD's end_trb instead of believing the Endpoint or Stream Context. Specifically: - rename cycle_found to hw_dequeue_found to avoid confusion - initialize new_cycle from td->end_trb instead of hw_dequeue - switch new_cycle toggling to happen after end_trb is found Now a workload which regularly stalls the device works normally for a few hours and clearly demonstrates the HW bug - the EP Context bit is not updated in a new cycle until Set TR Dequeue overwrites it: [ +0,000298] sd 10:0:0:0: [sdc] Attached SCSI disk [ +0,011758] cycle bits: TRB 1 EP Ctx 1 [ +5,947138] cycle bits: TRB 1 EP Ctx 1 [ +0,065731] cycle bits: TRB 0 EP Ctx 1 [ +0,064022] cycle bits: TRB 0 EP Ctx 0 [ +0,063297] cycle bits: TRB 0 EP Ctx 0 [ +0,069823] cycle bits: TRB 0 EP Ctx 0 [ +0,063390] cycle bits: TRB 1 EP Ctx 0 [ +0,063064] cycle bits: TRB 1 EP Ctx 1 [ +0,062293] cycle bits: TRB 1 EP Ctx 1 [ +0,066087] cycle bits: TRB 0 EP Ctx 1 [ +0,063636] cycle bits: TRB 0 EP Ctx 0 [ +0,066360] cycle bits: TRB 0 EP Ctx 0 Also tested on the buggy ASM1042 which moves EP Context dequeue to the next TRB after errors, one problem case addressed by the rework that implemented this loop. In this case hw_dequeue can be enqueue, so simply picking the cycle bit of TRB at hw_dequeue wouldn't work. Commit 5255660 ("xhci: add quirk for host controllers that don't update endpoint DCS") tried to solve the stale cycle problem, but it was more complex and got reverted due to a reported issue. Cc: Jonathan Bell <jonathan@raspberrypi.org> Cc: Oliver Neukum <oneukum@suse.com> Signed-off-by: Michal Pecio <michal.pecio@gmail.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Link: https://lore.kernel.org/r/20250505125630.561699-2-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 4e77d3e commit 6328bdc

File tree

1 file changed

+9
-10
lines changed

1 file changed

+9
-10
lines changed

drivers/usb/host/xhci-ring.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
699699
int new_cycle;
700700
dma_addr_t addr;
701701
u64 hw_dequeue;
702-
bool cycle_found = false;
702+
bool hw_dequeue_found = false;
703703
bool td_last_trb_found = false;
704704
u32 trb_sct = 0;
705705
int ret;
@@ -715,25 +715,24 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
715715
hw_dequeue = xhci_get_hw_deq(xhci, dev, ep_index, stream_id);
716716
new_seg = ep_ring->deq_seg;
717717
new_deq = ep_ring->dequeue;
718-
new_cycle = hw_dequeue & 0x1;
718+
new_cycle = le32_to_cpu(td->end_trb->generic.field[3]) & TRB_CYCLE;
719719

720720
/*
721-
* We want to find the pointer, segment and cycle state of the new trb
722-
* (the one after current TD's end_trb). We know the cycle state at
723-
* hw_dequeue, so walk the ring until both hw_dequeue and end_trb are
724-
* found.
721+
* Walk the ring until both the next TRB and hw_dequeue are found (don't
722+
* move hw_dequeue back if it went forward due to a HW bug). Cycle state
723+
* is loaded from a known good TRB, track later toggles to maintain it.
725724
*/
726725
do {
727-
if (!cycle_found && xhci_trb_virt_to_dma(new_seg, new_deq)
726+
if (!hw_dequeue_found && xhci_trb_virt_to_dma(new_seg, new_deq)
728727
== (dma_addr_t)(hw_dequeue & ~0xf)) {
729-
cycle_found = true;
728+
hw_dequeue_found = true;
730729
if (td_last_trb_found)
731730
break;
732731
}
733732
if (new_deq == td->end_trb)
734733
td_last_trb_found = true;
735734

736-
if (cycle_found && trb_is_link(new_deq) &&
735+
if (td_last_trb_found && trb_is_link(new_deq) &&
737736
link_trb_toggles_cycle(new_deq))
738737
new_cycle ^= 0x1;
739738

@@ -745,7 +744,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
745744
return -EINVAL;
746745
}
747746

748-
} while (!cycle_found || !td_last_trb_found);
747+
} while (!hw_dequeue_found || !td_last_trb_found);
749748

750749
/* Don't update the ring cycle state for the producer (us). */
751750
addr = xhci_trb_virt_to_dma(new_seg, new_deq);

0 commit comments

Comments
 (0)