Hi,
Thank you for the review.
Michał Pecio <michal.pecio@xxxxxxxxx> 於 2025年2月6日 週四 上午5:45寫道：
>
> > case COMP_STOPPED:
> > + /* Think of it as the final event if TD had an error */
> > + if (td->error_mid_td)
> > + td->error_mid_td = false;
> > sum_trbs_for_length = true;
> > break;
>
> What was the reason for this part?
To prevent the driver from printing the following debug message twice:
"Error mid isoc TD, wait for final completion event"
This can happen if the driver queues a Stop Endpoint command after
mid isoc TD error occurred, see my debug messages below:
[ 110.514149] xhci_hcd 0000:01:00.0: xhci_queue_isoc_tx
[ 110.514164] xhci_hcd 0000:01:00.0: @000000002d119510 59600000
00000000 00009000 800b1725
[ 110.514169] xhci_hcd 0000:01:00.0: @000000002d119520 59609000
00000000 00107000 800b1515
[ 110.514173] xhci_hcd 0000:01:00.0: @000000002d119530 59610000
00000000 00002000 00000625
...
[ 110.530263] xhci_hcd 0000:01:00.0: xhci_handle_event_trb
[ 110.530266] xhci_hcd 0000:01:00.0: @000000010afe6350 2d119510
00000000 04009000 01138001
[ 110.530271] xhci_hcd 0000:01:00.0: Error mid isoc TD, wait for
final completion event
[ 110.530373] xhci_hcd 0000:01:00.0: xhci_handle_event_trb
[ 110.530378] xhci_hcd 0000:01:00.0: @000000010afe6360 2d119510
00000000 01009000 01138001
[ 110.530387] xhci_hcd 0000:01:00.0: xhci_handle_event_trb
[ 110.530391] xhci_hcd 0000:01:00.0: @000000010afe6370 2d119520
00000000 04007000 01138001
[ 110.530395] xhci_hcd 0000:01:00.0: Error mid isoc TD, wait for
final completion event
[ 110.530430] xhci_hcd 0000:01:00.0: queue_command
[ 110.530434] xhci_hcd 0000:01:00.0: @000000010afe5230 00000000
00000000 00000000 01133c01
[ 110.530470] xhci_hcd 0000:01:00.0: xhci_handle_event_trb
[ 110.530473] xhci_hcd 0000:01:00.0: @000000010afe6380 2d119520
00000000 1a000000 01138001
[ 110.530478] xhci_hcd 0000:01:00.0: Error mid isoc TD, wait for
final completion event
[ 110.530481] xhci_hcd 0000:01:00.0: xhci_handle_event_trb
[ 110.530484] xhci_hcd 0000:01:00.0: @000000010afe6390 0afe5230
00000001 01000000 01008401
...
This may become confusing.
>
> As written it is going to cause problems, the driver will forget about
> earlier errors if the endpoint is stopped and resumed on the same TD.
Yes, this can happen, I didn't account for this scenario.
>
>
> I think that the whole patch could be much simpler, like:
>
> case X_ERROR:
> frame->status = X;
> td->error_mid_td = true;
> break;
> case Y_ERROR:
> frame->status = Y;
> td->error_mid_td = true;
> break;
>
> and then
>
> if (error_mid_td && (ep_trb != td->end_trb || ETRON && SUPERSPEED)) {
> // error mid TD, wait for final event
> }
>
> finish_td()
Yes, this is much simpler than my patch, but we still need to fix
the issue with debug message being printed twice.
Maybe silence the debug message like this:
if (error_mid_td && (ep_trb != td->end_trb || ETRON && SUPERSPEED)) {
if (trb_comp_code != COMP_STOPPED)
xhci_dbg(xhci, "Error mid isoc TD, wait for final
completion event\n");
...
}
, or do nothing. Could you help with some suggestions?
>
>
> Regards,
> Michal
Thanks,
Kuangyi Chiang
