Re: [PATCH v2] xhci: add quirk for host controllers that don't update endpoint DCS
From: Rik van Riel <riel@surriel.com>
Date: 2021-07-20 16:54:19
Also in:
stable
On Tue, 2021-07-20 at 17:09 +0200, Bjørn Mork wrote:
From: Jonathan Bell <redacted> Seen on a VLI VL805 PCIe to USB controller. For non-stream endpoints at least, if the xHC halts on a particular TRB due to an error then the DCS field in the Out Endpoint Context maintained by the hardware is not updated with the current cycle state.
I wonder if "some things getting out of sync" (probably not the same things) are the cause of the USB issues I see here with a noisy bus and the PCM2309B chip...
quoted hunk ↗ jump to hunk
@@ -598,7 +601,27 @@ static int xhci_move_dequeue_past_td(structxhci_hcd *xhci, hw_dequeue = xhci_get_hw_deq(xhci, dev, ep_index, stream_id); new_seg = ep_ring->deq_seg; new_deq = ep_ring->dequeue; - new_cycle = hw_dequeue & 0x1; + + /* + * Quirk: xHC write-back of the DCS field in the hardware dequeue + * pointer is wrong - use the cycle state of the TRB pointed to by + * the dequeue pointer. + */ + if (xhci->quirks & XHCI_EP_CTX_BROKEN_DCS && + !(ep->ep_state & EP_HAS_STREAMS)) + halted_seg = trb_in_td(xhci, td->start_seg, + td->first_trb, td->last_trb, + hw_dequeue & ~0xf, false); + if (halted_seg) { + index = ((dma_addr_t)(hw_dequeue & ~0xf) - halted_seg->dma) / + sizeof(*halted_trb); + halted_trb = &halted_seg->trbs[index]; + new_cycle = halted_trb->generic.field[3] & 0x1; + xhci_dbg(xhci, "Endpoint DCS = %d TRB index = %d cycle = %d\n", + (u8)(hw_dequeue & 0x1), index, new_cycle); + } else { + new_cycle = hw_dequeue & 0x1; + }
Is there ever a case where the cycle state is incorrect, and we should be using the DCS field, instead? I wonder if this is a quirk that should just be used everywhere, instead of only on a few systems where we know the hardware doesn't always behave right? Are there other places where the hardware is supposed to track the same information in multiple places, but might sometimes get them out of sync? If so, does the code have any detection of such issues? -- All Rights Reversed.
Attachments
- signature.asc [application/pgp-signature] 488 bytes