On 3.3.2025 12.34, Michał Pecio wrote:
Works here too, modulo the obvious build problem.
Thanks for testing on both Etron and Renesas hosts.
Etron with errors: [ 1064.865311] xhci_hcd 0000:06:00.0: Transfer error for slot 1 ep 2 on endpoint [ 1064.865322] xhci_hcd 0000:06:00.0: Successful completion on short TX for slot 1 ep 2 with last td comp code 4 [ 1064.865326] xhci_hcd 0000:06:00.0: Spurious event dma 0x00000000ffef88c0, comp_code 13 after 4 Renesas with short packets: [ 1365.299218] xhci_hcd 0000:08:00.0: Successful completion on short TX for slot 1 ep 2 with last td comp code 13 [ 1365.299223] xhci_hcd 0000:08:00.0: Spurious event dma 0x00000000ffbbf870, comp_code 13 after 13 BTW, it says "comp_code 13 after something" because of this crazy TRUST_TX_LENGTH hack, which changes trb_comp_code if it's success but the residual is nonzero. If I remove the hack, Etron: [ 2713.630443] xhci_hcd 0000:06:00.0: Spurious event dma 0x00000000ff3b6130, comp_code 1 after 4 Renesas: [ 4033.652300] xhci_hcd 0000:08:00.0: Spurious event dma 0x00000000ffcd1b80, comp_code 1 after 13 The hack could almost be removed now, but if there really are HCs which report Success on the first event, this won't work for them:
This looks better, and I agree that the hack/quirk is annoying, but in fear of regression I don't want to touch that in this patch yet.
+static bool xhci_spurious_success_tx_event(struct xhci_hcd *xhci, + struct xhci_ring *ring) +{ + switch (ring->old_trb_comp_code) { + case COMP_SHORT_PACKET: + return xhci->quirks & XHCI_SPURIOUS_SUCCESS;Could it work without relying on fictional COMP_SHORT_PACKET events?+ if (xhci_spurious_success_tx_event(xhci, ep_ring)) { + xhci_dbg(xhci, "Spurious event dma %pad, comp_code %u after %u\n", + &ep_trb_dma, trb_comp_code, ep_ring->old_trb_comp_code); + ep_ring->old_trb_comp_code = trb_comp_code;This part will (quite arbitrarily IMO) not execute if td_list is empty.
Yes, if td_list is empty we don't take this path, and we don't print any "ERROR Transfer event TRB DMA ptr not part of current TD..." message either, just like before.
I had this idea that "empty td_list" and "no matching TD on td_list" are practically identical cases, and their code could be merged.'
Possibly, but not in this patch. Thanks Mathias
