[alsa-devel] [PATCH 4/9 v2] ALSA: usb-audio: use usb_fill_int_urb()

[Takashi Iwai]

On Wed, 20 Jun 2018 15:52:49 +0200,
Sebastian Andrzej Siewior wrote:
> 
> On 2018-06-20 15:21:49 [+0200], Takashi Iwai wrote:
> > > usb_fill_int_urb() ensures that syncinterval is within the allowed range
> > > on HS/SS. The interval value seems to come from
> > > snd_usb_parse_datainterval() which is bInterval - 1 and only in the rage
> > > 1 … 4. So in order to keep the magic working I pass datainterval + 1.
> > 
> > This needs more explanation.  By this conversion, the interval
> > computation becomes really tricky.
> > 
> > There are two interval calculations.  The first one is
> > fp->datainterval and it's from snd_usb_parse_datainterval() as you
> > mentioned.  And a tricky part is that fp->datainterval is 0 on
> > FULL_SPEED.  Casually, fp->datainterval+1 will be translated to the
> > correct value since (0 + 1) == (1 << 0).
> > 
> > OTOH, for the sync EP, it's taken from ep->syncinterval, which is set
> > in snd_usb_add_endpoint().  Luckily, again, ep->syncinterval + 1 works
> > for FULL_SPEED as well, as (1 + 1) == (1 << 1).
> 
> Do you want me to add your additional explanation to the description?

Yes.  It's a very implicit assumption, and needs clarification.
In addition, the comment 1...4 is only for fp->datainterval, not about
ep->syncinterval.

Alternatively, give some comments in the places where putting
fp->datainterval and ep->syncinterval.


thanks,

Takashi