* [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation @ 2025-11-24 21:05 Dylan Robinson 2025-11-25 6:59 ` Takashi Iwai 2025-11-29 19:56 ` Michal Pecio 0 siblings, 2 replies; 9+ messages in thread From: Dylan Robinson @ 2025-11-24 21:05 UTC (permalink / raw) To: linux-sound; +Cc: tiwai, perex, dylan_robinson 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. Signed-off-by: Dylan Robinson <dylan_robinson@motu.com> --- sound/usb/stream.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/sound/usb/stream.c b/sound/usb/stream.c index 5c235a5ba7e1..074a61215de6 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -684,6 +684,37 @@ 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, @@ -703,11 +734,8 @@ 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 = le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize); + fp->maxpacksize = snd_usb_max_bytes_per_interval(chip, alts); fp->channels = num_channels; - if (snd_usb_get_speed(chip->dev) == USB_SPEED_HIGH) - fp->maxpacksize = (((fp->maxpacksize >> 11) & 3) + 1) - * (fp->maxpacksize & 0x7ff); fp->clock = clock; INIT_LIST_HEAD(&fp->list); -- 2.51.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation 2025-11-24 21:05 [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation Dylan Robinson @ 2025-11-25 6:59 ` Takashi Iwai 2025-11-29 19:56 ` Michal Pecio 1 sibling, 0 replies; 9+ messages in thread From: Takashi Iwai @ 2025-11-25 6:59 UTC (permalink / raw) To: Dylan Robinson; +Cc: linux-sound, tiwai, perex On Mon, 24 Nov 2025 22:05:18 +0100, 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. > > Signed-off-by: Dylan Robinson <dylan_robinson@motu.com> Applied to for-next branch now. Thanks! Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation 2025-11-24 21:05 [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation Dylan Robinson 2025-11-25 6:59 ` Takashi Iwai @ 2025-11-29 19:56 ` Michal Pecio 2025-11-30 10:53 ` Takashi Iwai 1 sibling, 1 reply; 9+ messages in thread From: Michal Pecio @ 2025-11-29 19:56 UTC (permalink / raw) To: Dylan Robinson; +Cc: linux-sound, tiwai, perex, linux-usb 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. Regards, Michal > Signed-off-by: Dylan Robinson <dylan_robinson@motu.com> > --- > sound/usb/stream.c | 36 ++++++++++++++++++++++++++++++++---- > 1 file changed, 32 insertions(+), 4 deletions(-) > > diff --git a/sound/usb/stream.c b/sound/usb/stream.c > index 5c235a5ba7e1..074a61215de6 100644 > --- a/sound/usb/stream.c > +++ b/sound/usb/stream.c > @@ -684,6 +684,37 @@ 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, > @@ -703,11 +734,8 @@ 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 = le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize); > + fp->maxpacksize = snd_usb_max_bytes_per_interval(chip, alts); > fp->channels = num_channels; > - if (snd_usb_get_speed(chip->dev) == USB_SPEED_HIGH) > - fp->maxpacksize = (((fp->maxpacksize >> 11) & 3) + 1) > - * (fp->maxpacksize & 0x7ff); > fp->clock = clock; > INIT_LIST_HEAD(&fp->list); > > -- > 2.51.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation 2025-11-29 19:56 ` Michal Pecio @ 2025-11-30 10:53 ` Takashi Iwai 2025-11-30 12:00 ` Michal Pecio 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2025-11-30 10:53 UTC (permalink / raw) To: Michal Pecio; +Cc: Dylan Robinson, linux-sound, tiwai, perex, linux-usb 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. 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation 2025-11-30 10:53 ` Takashi Iwai @ 2025-11-30 12:00 ` Michal Pecio 2025-11-30 12:08 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Michal Pecio @ 2025-11-30 12:00 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dylan Robinson, linux-sound, tiwai, perex, linux-usb 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation 2025-11-30 12:00 ` Michal Pecio @ 2025-11-30 12:08 ` Takashi Iwai 2025-12-01 10:47 ` Michal Pecio 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2025-11-30 12:08 UTC (permalink / raw) To: Michal Pecio Cc: Takashi Iwai, Dylan Robinson, linux-sound, tiwai, perex, linux-usb On Sun, 30 Nov 2025 13:00:35 +0100, Michal Pecio wrote: > > 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. Agreed, it makes more sense to have the common criteria applied to all places. Dylan, could you check whether the cleanup is OK? I'll apply it once after the confirmation. thanks, Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation 2025-11-30 12:08 ` Takashi Iwai @ 2025-12-01 10:47 ` Michal Pecio 2025-12-01 23:10 ` Dylan Robinson 0 siblings, 1 reply; 9+ messages in thread From: Michal Pecio @ 2025-12-01 10:47 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dylan Robinson, linux-sound, tiwai, perex, linux-usb On Sun, 30 Nov 2025 13:08:56 +0100, Takashi Iwai wrote: > On Sun, 30 Nov 2025 13:00:35 +0100, > Michal Pecio wrote: > > > > 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. > > Agreed, it makes more sense to have the common criteria applied to all > places. FWIW, I cherry-picked these two patches on v6.18 and confirmed that my SuperSpeed HDMI capture adapter still works. But that was not surprising, its audio endpoint is completely boring and equivalent code existed in uvcvideo and xhci_hcd since forever. Endpoint Descriptor: bLength 9 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 13 Transfer Type Isochronous Synch Type Synchronous Usage Type Data wMaxPacketSize 0x00c0 1x 192 bytes bInterval 4 bRefresh 0 bSynchAddress 0 bMaxBurst 0 wBytesPerInterval 192 Michal ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation 2025-12-01 10:47 ` Michal Pecio @ 2025-12-01 23:10 ` Dylan Robinson 2025-12-02 7:08 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Dylan Robinson @ 2025-12-01 23:10 UTC (permalink / raw) To: Michal Pecio; +Cc: Takashi Iwai, linux-sound, tiwai, perex, linux-usb > Dylan, could you check whether the cleanup is OK? > I'll apply it once after the confirmation. Tested and seems to be working. The cleanup looks good to me. This is what our endpoint looks like: Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 37 Transfer Type Isochronous Synch Type Asynchronous Usage Type Implicit feedback Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 1 bMaxBurst 3 wBytesPerInterval 3205 -Dylan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation 2025-12-01 23:10 ` Dylan Robinson @ 2025-12-02 7:08 ` Takashi Iwai 0 siblings, 0 replies; 9+ messages in thread From: Takashi Iwai @ 2025-12-02 7:08 UTC (permalink / raw) To: Dylan Robinson Cc: Michal Pecio, Takashi Iwai, linux-sound, tiwai, perex, linux-usb On Tue, 02 Dec 2025 00:10:21 +0100, Dylan Robinson wrote: > > > Dylan, could you check whether the cleanup is OK? > > I'll apply it once after the confirmation. > > Tested and seems to be working. The cleanup looks good to me. > > This is what our endpoint looks like: > > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x82 EP 2 IN > bmAttributes 37 > Transfer Type Isochronous > Synch Type Asynchronous > Usage Type Implicit feedback Data > wMaxPacketSize 0x0400 1x 1024 bytes > bInterval 1 > bMaxBurst 3 > wBytesPerInterval 3205 Thanks for confirmation. Then let's take this. Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-12-02 7:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-24 21:05 [PATCH] ALSA: usb-audio: Fix max bytes-per-interval calculation Dylan Robinson 2025-11-25 6:59 ` Takashi Iwai 2025-11-29 19:56 ` Michal Pecio 2025-11-30 10:53 ` Takashi Iwai 2025-11-30 12:00 ` Michal Pecio 2025-11-30 12:08 ` Takashi Iwai 2025-12-01 10:47 ` Michal Pecio 2025-12-01 23:10 ` Dylan Robinson 2025-12-02 7:08 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox