Re: [PATCH 5/5] usb: xhci: Skip only one TD on Ring Underrun/Overrun

Mathias Nyman
2025-02-21T13:16:21Z

On 21.2.2025 3.17, Michał Pecio wrote:
> On Tue, 11 Feb 2025 17:41:39 +0200, Mathias Nyman wrote:
>> On 10.2.2025 9.42, Michal Pecio wrote:
>>> + if (ring_xrun_event) {
>>> + /*
>>> + * If we are here, we are on xHCI 1.0 host with no idea how
>>> + * many TDs were missed and where the xrun occurred. Don't
>>> + * skip more TDs, they may have been queued after the xrun.
>>> + */
>>> + xhci_dbg(xhci, "Skipped one TD for slot %u ep %u",
>>> + slot_id, ep_index);
>>> + break;
>>
>> This would be the same as return 0; right?
>>
>> Whole series looks good, I'll add it
>
> I hope you haven't sent it out yet because I found two minor issues.
>
>
> Firstly,
> [PATCH 3/5] usb: xhci: Fix isochronous Ring Underrun/Overrun event handling
>
> increases the number of xrun events that we handle but doesn't suppress
> the "Event TRB for slot %u ep %u with no TDs queued\n" warning, so the
> warning started to show up sometimes for no good reason. The fix is to
> add ring_xrun_event to the list of exception for this warning.
>
>
> Secondly,
> [PATCH 5/5] usb: xhci: Skip only one TD on Ring Underrun/Overrun
>
> can be improved to clear the skip flag if skipped TD was the only one.
> This eliminates any confusion and risk of skipping bugs in the future.
> The change is a matter of moving that code to a different branch.
>
> I also changed 'break' to 'return 0' because it gets hard to follow at
> this level of indentation.
>
>
> I'll send a v2 of those two patches. Sorry for any inconvenience.

Patches updated, they are now in my for-usb-next branch

Thanks
Mathias