Re: [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs

On Fri, Aug 07, 2026 at 02:10:35PM +0300, Mathias Nyman wrote:
> On 8/7/26 11:26, Michal Pecio wrote:
> > This was discussed all the way back in Bugzilla - usb_submit_urb()
> > kerneldoc spells out what we want and Alan was clear that it reflects
> > what HCDs have been doing for 20 years:
> > 
> 
> I'm doing exactly what kerneldoc and comments in bugzilla state.
> If queue empties out we schedule the next TD based on URB_ISO_ASAP flag.
> 
> The logic is from Alan's bugzilla comment. I'll quote the beginning here:
> 
> "When the queue underruns is exactly when URB_ISO_ASAP is supposed to matter.
>  If the flag is set in the new URB then that URB should be scheduled for the
>  first slot in the future, leaving a logical gap in the queue.
>  If the flag is clear then the new URB is supposed to be scheduled for the slot
>  that follows the preceding URB, which means that some of its packets will
>  never be sent because their slots have already expired.
>  This will still leave a physical gap in the queue, of course -- no way to
>  avoid that -- but it will maintain the logical alignment of URBs and frames.
> 
>  Thus by submitting all URBs with URB_ISO_ASAP clear, drivers can help ensure
>  that the queue remains synchronized to within the limits imposed by the host
>  controller driver.
>  If xhci-hcd doesn't behave this way then it should be changed."

This quote needs some context; it applies when the new URB is submitted 
while the queue is still active, for example, by a completion handler.

> So I changed it to behave this way.
> 
> >   * If the driver is unable to keep up and the queue empties out, the
> >   * behavior for new submissions is governed by the URB_ISO_ASAP flag.
> >   * If the flag is set, or if the queue is idle, then the URB is always
> >   * assigned to the first available (and not yet expired) slot in the
> >   * endpoint's schedule.
> 
> > 
> > "If the queue is idle" means all completions have already returned.
> 
> That's an odd interpretation of "queue is idle"

Michal is correct; that's what it's supposed to mean.

> This would mean that an URB queued after "queue runs out" (underrun)
> should always be treated as USB_ISO_ASAP case, making the flag useless.

... useless in that situation.  That's right.  The flag is used only 
while the queue is still active.

> Isn't it more likely that 'queue is idle' means 'not yet started' than
> 'class driver intentionally left it to dry out and underrun in order
> to later restart it, assuming it will restart ASAP on next URB enqueue
> without setting URB_ISO_ASAP flag?'

I don't know about likelihood, but the fact is that "queue is idle" does 
mean all completion handlers have returned.  Of course, this includes 
the "not yet started" case, but it includes other cases as well.

Do any existing class drivers actually rely on documented behavior of 
isochronous queues?  I have no idea.  In fact, I would expect that if a 
class driver wanted to stop a queue and restart it later, it would 
switch to altsetting 0 in the meantime, which would eliminate all 
ambiguities.

The point under discussion is really about what happens when an underrun 
occurs by accident -- for example, if the system is temporarily 
overloaded.  In this situation there are two logical choices of action 
if the class driver wants to keep the queue running: Keep the alignment 
of iso_packet_descriptors and (micro)frames as it was, which means 
assigning some new packets to times in the past, or forget about the old 
alignment and start fresh, which means assigning the next packet to the 
next available future time slot.  The URB_ISO_ASAP flag is how the 
driver communicates its choice to the HCD.

On the other hand, if the class driver doesn't respond right away and 
lets the queue dry out totally, the first choice becomes invalid.  After 
all, if the driver cared about keeping the slots aligned, it wouldn't 
have waited so long that the queue became idle.  And besides, there's 
no time limit on how a queue can remain idle before the driver decides 
to start using it again; we can't expect an HCD to keep track of the 
queue's state indefinitely while it's not being used.

For these reasons the URB_ISO_ASAP flag is ignored if the queue is idle 
-- meaning that all completion handlers have returned (because 
submission of new URBs in an iso stream typically occurs within a 
completion handler).

Alan Stern