Hi,
Kuangyi Chiang <ki.chiang65@xxxxxxxxx> 於 2025年3月1日 週六 上午10:05寫道：
>
> Hi,
>
> Thanks for the patch.
>
> Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> 於 2025年3月1日 週六 上午12:17寫道：
> >
> > Unplugging a USB3.0 webcam from Etron hosts while streaming results
> > in errors like this:
> >
> > [ 2.646387] xhci_hcd 0000:03:00.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 18 comp_code 13
> > [ 2.646446] xhci_hcd 0000:03:00.0: Looking for event-dma 000000002fdf8630 trb-start 000000002fdf8640 trb-end 000000002fdf8650
> > [ 2.646560] xhci_hcd 0000:03:00.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 18 comp_code 13
> > [ 2.646568] xhci_hcd 0000:03:00.0: Looking for event-dma 000000002fdf8660 trb-start 000000002fdf8670 trb-end 000000002fdf8670
> >
> > Etron xHC generates two transfer events for the TRB if an error is
> > detected while processing the last TRB of an isoc TD.
> >
> > The first event can be any sort of error (like USB Transaction or
> > Babble Detected, etc), and the final event is Success.
> >
> > The xHCI driver will handle the TD after the first event and remove it
> > from its internal list, and then print an "Transfer event TRB DMA ptr
> > not part of current TD" error message after the final event.
> >
> > Commit 5372c65e1311 ("xhci: process isoc TD properly when there was a
> > transaction error mid TD.") is designed to address isoc transaction
> > errors, but unfortunately it doesn't account for this scenario.
> >
> > This issue is similar to the XHCI_SPURIOUS_SUCCESS case where a success
> > event follows a 'short transfer' event, but the TD the event points to
> > is already given back.
> >
> > Expand the spurious success 'short transfer' event handling to cover
> > the spurious success after error on Etron hosts.
> >
> > Kuangyi Chiang reported this issue and submitted a different solution
> > based on using error_mid_td. This commit message is mostly taken
> > from that patch.
> >
> > Reported-by: Kuangyi Chiang <ki.chiang65@xxxxxxxxx>
> > Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
> > ---
> > drivers/usb/host/xhci-ring.c | 36 +++++++++++++++++++++++++-----------
> > drivers/usb/host/xhci.h | 2 +-
> > 2 files changed, 26 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index 965bffce301e..3d3e6cd69019 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -2644,6 +2644,22 @@ static int handle_transferless_tx_event(struct xhci_hcd *xhci, struct xhci_virt_
> > return 0;
> > }
> >
> > +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;
> > + case COMP_USB_TRANSACTION_ERROR:
> > + case COMP_BABBLE_DETECTED_ERROR:
> > + case COMP_ISOCH_BUFFER_OVERRUN:
> > + return xhci->quirks & XHCI_ETRON_HOST &&
> > + ring->type == TYPE_ISOC;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > /*
> > * If this function returns an error condition, it means it got a Transfer
> > * event with a corrupted Slot ID, Endpoint ID, or TRB DMA address.
> > @@ -2697,8 +2713,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> > case COMP_SUCCESS:
> > if (EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
> > trb_comp_code = COMP_SHORT_PACKET;
> > - xhci_dbg(xhci, "Successful completion on short TX for slot %u ep %u with last td short %d\n",
> > - slot_id, ep_index, ep_ring->last_td_was_short);
> > + xhci_dbg(xhci, "Successful completion on short TX for slot %u ep %u with last td comp code %d\n",
> > + slot_id, ep_index, ep_ring->old_trb_comp_code);
> > }
> > break;
> > case COMP_SHORT_PACKET:
> > @@ -2846,7 +2862,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> > */
> > if (trb_comp_code != COMP_STOPPED &&
> > trb_comp_code != COMP_STOPPED_LENGTH_INVALID &&
> > - !ep_ring->last_td_was_short) {
> > + !xhci_spurious_success_tx_event(xhci, ep_ring)) {
> > xhci_warn(xhci, "Event TRB for slot %u ep %u with no TDs queued\n",
> > slot_id, ep_index);
> > }
> > @@ -2890,11 +2906,12 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> >
> > /*
> > * Some hosts give a spurious success event after a short
> > - * transfer. Ignore it.
> > + * transfer or error on last TRB. Ignore it.
> > */
> > - if ((xhci->quirks & XHCI_SPURIOUS_SUCCESS) &&
> > - ep_ring->last_td_was_short) {
> > - ep_ring->last_td_was_short = false;
> > + 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;
> > return 0;
> > }
> >
> > @@ -2922,10 +2939,7 @@ static int handle_tx_event(struct xhci_hcd *xhci,
> > */
> > } while (ep->skip);
> >
> > - if (trb_comp_code == COMP_SHORT_PACKET)
> > - ep_ring->last_td_was_short = true;
> > - else
> > - ep_ring->last_td_was_short = false;
> > + ep_ring->old_trb_comp_code = trb_comp_code;
> >
> > ep_trb = &ep_seg->trbs[(ep_trb_dma - ep_seg->dma) / sizeof(*ep_trb)];
> > trace_xhci_handle_transfer(ep_ring, (struct xhci_generic_trb *) ep_trb, ep_trb_dma);
> > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > index 8c164340a2c3..c75c2c12ce53 100644
> > --- a/drivers/usb/host/xhci.h
> > +++ b/drivers/usb/host/xhci.h
> > @@ -1371,7 +1371,7 @@ struct xhci_ring {
> > unsigned int num_trbs_free; /* used only by xhci DbC */
> > unsigned int bounce_buf_len;
> > enum xhci_ring_type type;
> > - bool last_td_was_short;
> > + u32 last_td_comp_code;
>
> Should be changed to old_trb_comp_code.
>
> > struct radix_tree_root *trb_address_map;
> > };
> >
> > --
> > 2.43.0
> >
>
> I'll test this later.
It works. See the below dynamic debug messages:
[ 1138.281772] xhci_hcd 0000:04:00.0: Successful completion on short
TX for slot 1 ep 18 with last td comp code 13
[ 1138.281777] xhci_hcd 0000:04:00.0: Spurious event dma
0x000000010b5ba600, comp_code 13 after 13
[ 1138.282013] xhci_hcd 0000:04:00.0: Successful completion on short
TX for slot 1 ep 18 with last td comp code 13
[ 1138.282019] xhci_hcd 0000:04:00.0: Spurious event dma
0x000000010b5ba630, comp_code 13 after 13
[ 1138.282271] xhci_hcd 0000:04:00.0: Successful completion on short
TX for slot 1 ep 18 with last td comp code 13
[ 1138.282277] xhci_hcd 0000:04:00.0: Spurious event dma
0x000000010b5ba660, comp_code 13 after 13
[ 1138.282420] xhci_hcd 0000:04:00.0: Successful completion on short
TX for slot 1 ep 18 with last td comp code 13
[ 1138.282425] xhci_hcd 0000:04:00.0: Spurious event dma
0x000000010b5ba680, comp_code 13 after 13
[ 1138.282653] xhci_hcd 0000:04:00.0: Transfer error for slot 1 ep 18
on endpoint
[ 1138.282659] xhci_hcd 0000:04:00.0: Error mid isoc TD, wait for
final completion event
[ 1138.282779] xhci_hcd 0000:04:00.0: Successful completion on short
TX for slot 1 ep 18 with last td comp code 4
[ 1138.282785] xhci_hcd 0000:04:00.0: Transfer error for slot 1 ep 18
on endpoint
[ 1138.282911] xhci_hcd 0000:04:00.0: Successful completion on short
TX for slot 1 ep 18 with last td comp code 4
[ 1138.282916] xhci_hcd 0000:04:00.0: Spurious event dma
0x000000010b5ba6c0, comp_code 13 after 4
[ 1138.282920] xhci_hcd 0000:04:00.0: Transfer error for slot 1 ep 18
on endpoint
[ 1138.282923] xhci_hcd 0000:04:00.0: Error mid isoc TD, wait for
final completion event
[ 1138.283039] xhci_hcd 0000:04:00.0: Successful completion on short
TX for slot 1 ep 18 with last td comp code 4
[ 1138.283045] xhci_hcd 0000:04:00.0: Transfer error for slot 1 ep 18
on endpoint
[ 1138.283163] xhci_hcd 0000:04:00.0: Successful completion on short
TX for slot 1 ep 18 with last td comp code 4
[ 1138.283167] xhci_hcd 0000:04:00.0: Spurious event dma
0x000000010b5ba6f0, comp_code 13 after 4
>
> Thanks,
> Kuangyi Chiang
Thanks,
Kuangyi Chiang
