On Tue, 2021-07-20 at 17:09 +0200, Bjørn Mork wrote:
> From: Jonathan Bell <jonathan@xxxxxxxxxxxxxxx>
>
> 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...
> @@ -598,7 +601,27 @@ static int xhci_move_dequeue_past_td(struct
> xhci_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.
Attachment:
signature.asc
Description: This is a digitally signed message part
