Re: [PATCH v2 1/1] usb: gadget: f_uac2: finalize wMaxPacketSize according to bandwidth

[Daniel Mack <daniel@zonque.org>]

On 07/23/2015 03:00 AM, Peter Chen wrote:
> Thanks, it is correct. But looking the code at afunc_set_alt:
> the method of calculating uac2->p_pktsize seems incorrect, it
> may need to change like below:

Ok, sorry. I just read the code again an figured you're right here, this
needs fixing. It doesn't really matter for the currently configured
values of bInterval though, as p_interval will always be 1000 for both
HS and FS.

> @@ -1176,15 +1188,16 @@ afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
>                         factor = 1000;
>                 } else {
>                         ep_desc = &hs_epin_desc;
> -                       factor = 125;
> +                       factor = 8000;
>                 }
>  
>                 /* pre-compute some values for iso_complete() */
>                 uac2->p_framesize = opts->p_ssize *
>                                     num_channels(opts->p_chmask);
>                 rate = opts->p_srate * uac2->p_framesize;
> -               uac2->p_interval = (1 << (ep_desc->bInterval - 1)) * factor;
> -               uac2->p_pktsize = min_t(unsigned int, rate / uac2->p_interval,
> +               uac2->p_interval =  factor / (1 << (ep_desc->bInterval - 1));

Your version is correct. b_interval needs to get larger when bInterval
decreases of course.

> +               uac2->p_pktsize = min_t(unsigned int, DIV_ROUND_UP(rate,
> +                                       uac2->p_interval),
>                                         prm->max_psize);

This change, however, is not needed. uac2->p_pktsize needs to be rounded
down, so an extra frame can be added when the residue accumulator
overflows. The reason is simply that we can only send packets that
contain full sample frames, so we have to evenly distribute those
left-over samples that accumulate in one go once we have enough to fill
a complete frame.

Could you put the above change in an extra patch, as it's not directly
related to your wMaxPacketSize change?


Thanks,
Daniel