Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation

[Michal Pecio <michal.pecio@gmail.com>]

On Sun, 30 Nov 2025 11:53:07 +0100, Takashi Iwai wrote:
> On Sat, 29 Nov 2025 20:56:39 +0100,
> Michal Pecio wrote:
> > 
> > On Mon, 24 Nov 2025 16:05:18 -0500, Dylan Robinson wrote:  
> > > The maxpacksize field in struct audioformat represents the maximum number
> > > of bytes per isochronous interval. The current implementation only
> > > special-cases high-speed endpoints and does not account for the different
> > > computations required for SuperSpeed, SuperSpeedPlus, or eUSB2. As a
> > > result, USB audio class devices operating at these speeds may fail to
> > > stream correctly. The issue was observed on a MOTU 16A (2025) interface,
> > > which requires more than 1024 bytes per interval at SuperSpeed.
> > > 
> > > This patch replaces the existing logic with a helper that computes the
> > > correct maximum bytes-per-interval for all USB speeds, borrowing the logic
> > > used in drivers/usb/core/urb.c.  
> > 
> > Hi,
> > 
> > Since v6.18 we have usb_endpoint_max_periodic_payload(), which looks
> > like the exact function you need. It is already used by uvcvideo and
> > xhci_hcd, the latter being particularly important because it ensures
> > that your endpoints will get the bandwidth allocation you expect.
> > 
> > The copy-pasta in urb.c should probably be cleaned up at this point,
> > but that would be a separate and unrelated patch, of course.  
> 
> Thanks for the information!  So we can clean up a lot with this new
> helper like below.

Yes, something like that.

Note that there is a small gotcha here: Dylan's patch and the original
code, as well as usb_submit_urb(), didn't take wBytesPerInterval into
account, while usb_endpoint_max_periodic_payload() and xhci_hcd do.

Odd SuperSpeed endpoints like those below will now be considered to
have 512B/1536B capacity, not 1KB/2KB. Whether any such UAC devices
exist (mine is UVC) I don't know. My only SuperSpeed UAC device uses
one packet per interval and wMaxPacketSize == wBytesPerInterval.

        wMaxPacketSize     0x0400  1x 1024 bytes
        bInterval               1
        bMaxBurst               0
        wBytesPerInterval     512

        wMaxPacketSize     0x0400  1x 1024 bytes
        bInterval               1
        bMaxBurst               1       /* two packets per interval */
        wBytesPerInterval    1536

I also don't know whether this affects UAC operation in any way, but
it's something to watch out for.

Ignoring wBytesPerInterval wasn't right either, because xhci_hcd would
still reserve wBytesPerInterval bandwidth (as per spec) and URBs which
exceed that could complete with an error.

If a device is found where wBytesPerInterval makes no sense and must be
ignored, it needs to be ignored consistently across the kernel.

> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: usb-audio: Simplify with usb_endpoint_max_periodic_payload()
> 
> Recently we received a new helper function,
> usb_endpoint_max_periodic_payload(), for calculating the max packet
> size for periodic transfer.
> 
> Simplify the former open code with the new helper function.
> 
> Fixes: a748e1dbb2df ("ALSA: usb-audio: Fix max bytes-per-interval calculation")
> Suggested-by: Michal Pecio <michal.pecio@gmail.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/usb/stream.c | 34 ++--------------------------------
>  1 file changed, 2 insertions(+), 32 deletions(-)
> 
> diff --git a/sound/usb/stream.c b/sound/usb/stream.c
> index 074a61215de6..ec7d756d78d1 100644
> --- a/sound/usb/stream.c
> +++ b/sound/usb/stream.c
> @@ -684,43 +684,13 @@ snd_usb_find_output_terminal_descriptor(struct usb_host_interface *ctrl_iface,
>  	return NULL;
>  }
>  
> -static unsigned int
> -snd_usb_max_bytes_per_interval(struct snd_usb_audio *chip,
> -					struct usb_host_interface *alts)
> -{
> -	struct usb_host_endpoint *ep = &alts->endpoint[0];
> -	unsigned int max_bytes = usb_endpoint_maxp(&ep->desc);
> -
> -	/* SuperSpeed isoc endpoints have up to 16 bursts of up to 3 packets each */
> -	if (snd_usb_get_speed(chip->dev) >= USB_SPEED_SUPER) {
> -		int burst = 1 + ep->ss_ep_comp.bMaxBurst;
> -		int mult = USB_SS_MULT(ep->ss_ep_comp.bmAttributes);
> -		max_bytes *= burst;
> -		max_bytes *= mult;
> -	}
> -
> -	if (snd_usb_get_speed(chip->dev) == USB_SPEED_SUPER_PLUS &&
> -		USB_SS_SSP_ISOC_COMP(ep->ss_ep_comp.bmAttributes)) {
> -		max_bytes = le32_to_cpu(ep->ssp_isoc_ep_comp.dwBytesPerInterval);
> -	}
> -
> -	/* High speed, 1-3 packets/uframe, max 6 for eUSB2 double bw */
> -	if (snd_usb_get_speed(chip->dev) == USB_SPEED_HIGH) {
> -		if (usb_endpoint_is_hs_isoc_double(chip->dev, ep))
> -			max_bytes = le32_to_cpu(ep->eusb2_isoc_ep_comp.dwBytesPerInterval);
> -		else
> -			max_bytes *= usb_endpoint_maxp_mult(&ep->desc);
> -	}
> -
> -	return max_bytes;
> -}
> -
>  static struct audioformat *
>  audio_format_alloc_init(struct snd_usb_audio *chip,
>  		       struct usb_host_interface *alts,
>  		       int protocol, int iface_no, int altset_idx,
>  		       int altno, int num_channels, int clock)
>  {
> +	struct usb_host_endpoint *ep = &alts->endpoint[0];
>  	struct audioformat *fp;
>  
>  	fp = kzalloc(sizeof(*fp), GFP_KERNEL);
> @@ -734,7 +704,7 @@ audio_format_alloc_init(struct snd_usb_audio *chip,
>  	fp->ep_attr = get_endpoint(alts, 0)->bmAttributes;
>  	fp->datainterval = snd_usb_parse_datainterval(chip, alts);
>  	fp->protocol = protocol;
> -	fp->maxpacksize = snd_usb_max_bytes_per_interval(chip, alts);
> +	fp->maxpacksize = usb_endpoint_max_periodic_payload(chip->dev, ep);
>  	fp->channels = num_channels;
>  	fp->clock = clock;
>  	INIT_LIST_HEAD(&fp->list);
> -- 
> 2.52.0
>