usb: device_next: uac2: Support configurable polling rates #2910

vbrzeski · 2025-05-23T21:05:20Z

The UAC2 device class doesn't support configurable polling rates.

NordicBuilder · 2025-05-23T21:05:41Z

none

Note: This comment is automatically posted and updated by the Contribs GitHub Action.

tmon-nordic · 2025-05-26T06:20:21Z

dts/bindings/usb/uac2/zephyr,uac2-audio-streaming.yaml

+      Input or output type reports polling period in microseconds. For USB full
+      speed this could be clamped to 1ms or 255ms depending on the value.


Isochronous endpoint bInterval range 1 to 255 ms clamping is not valid.

For devices complying with USB 1.x specification, isochronous endpoint bInterval must be 1 (see Universal Serial Bus Specification Revision 1.1 Table 9-10. Standard Endpoint Descriptor bInterval).

For devices complying with USB 2 specification be it Full-Speed or High-Speed, isochronous endpoint bInterval range is from 1 to 16 where bInterval is used as exponent 2**(bInterval-1) (see Universal Serial Bus Specification Revision 2.0 Table 9-13. Standard Endpoint Descriptor bInterval). Therefore the largest interval in ms for Full-Speed isochronous endpoint is 2**(16-1)=32768 ms (and for High-Speed it is (2**(16-1))/8=4096 ms).

This same approach is taken in the HID driver, but will take another look :)

Interrupt endpoints are defined differently. See bInterval field description in Table 9-13. Standard Endpoint Descriptor in Universal Serial Bus Specification Revision 2.0.

Isochronous endpoints do have the power-of-two both at Full-Speed and High-Speed, while interrupt has 1 to 255 at Full-Speed and power-of-two at High-Speed. I believe this is inherited from USB 1.x due to backwards compatibility. In USB 1.x isochronous endpoints must have bInterval=1 while interrupt endpoints were using 1 to 255. Therefore without breaking backwards compatibility it was possible to add the power-of-two to isochronous endpoints (the only allowed value was 1 which has the same behavior in both USB 1.x and USB 2 Full-Speed operation), but not for interrupt endpoints.

tmon-nordic · 2025-05-26T06:25:26Z

subsys/usb/device_next/class/usbd_uac2_macros.h

@@ -707,13 +707,21 @@
 #define AS_BYTES_PER_SAMPLE(node)						\
 	DT_PROP(node, subslot_size)

+#define AS_FS_DATA_EP_BINTERVAL(node)						\
+	USB_FS_ISO_EP_INTERVAL(DT_PROP_OR(node, polling_period_us, 1000))


It looks like USB_FS_ISO_EP_INTERVAL() macro is not adhering to the spec. Can you fix it in this PR?

Don't commit this, this is a proof of concept for now.

vbrzeski · 2025-06-03T00:11:19Z

@tmon-nordic can you please take a look at the 2nd commit as without these changes, the bInterval change is moot:
b87293f

With this PR, we have acceptable performance for standard audio, though the changes in this commit are significant, and a little hacky. The most recent change is working well, though I had to deviate from the Linux gadget implementation given the "OUT Token on Disabled Endpoint" IRQ doesn't work for me. All the documentation for other MCU families with this USB Controller suggests this IRQ will only trigger for EP0.

Otherwise, the changes are a hacky grafting of this linux driver:
https://github.com/torvalds/linux/blob/master/drivers/usb/dwc2/gadget.c

sonarqubecloud · 2025-06-03T00:35:31Z

Quality Gate failed

Failed conditions
11.8% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

tmon-nordic · 2025-06-03T06:43:41Z

drivers/usb/udc/udc_dwc2.c

+
+	return (priv->enumspd == USB_DWC2_DSTS_ENUMSPD_HS3060) ?
+			USB_DWC2_DSTS_SOFFN_LIMIT :
+			USB_DWC2_DSTS_SOFFN_LIMIT >> 3;


There is no need, and in fact it is wrong, to divide SOFFN by 8 at Full-Speed. SOFFN contains microframe number when operating at High-Speed and Frame number when operating at Full-Speed.

tmon-nordic · 2025-06-03T06:45:04Z

drivers/usb/udc/udc_dwc2.c

+	return (priv->enumspd == USB_DWC2_DSTS_ENUMSPD_HS3060) ?
+			(1U << (cfg->interval - 1)) :
+			cfg->interval;


This is not standard compliant. It is valid for High-Speed, but not valid for Full-Speed. Using the same equation for High-Speed and Full-Speed will be standard compliant.

tmon-nordic · 2025-06-03T06:49:01Z

drivers/usb/udc/udc_dwc2.c

@@ -2196,6 +2268,10 @@ static int udc_dwc2_enable(const struct device *dev)

 	/* Disable soft disconnect */
 	sys_clear_bits((mem_addr_t)&base->dctl, USB_DWC2_DCTL_SFTDISCON);
+
+	// Disable frame number thing (out of curiousity)
+	// sys_clear_bits((mem_addr_t)&base->dctl, USB_DWC2_DCTL_IGNRFRMNUM);


IgnrFrmNum value after reset is 0, so clearing this bit has no effect. If you want to enable Periodic Transfer Interrupt then this bit should be set.

Note that this is only valid in the DMA modes (Buffer DMA or Descriptor DMA) and it is not valid in Completer mode. When IgnrFrmNum is set to 1, then IncompISOCIN interrupt should be masked (GINTMSK register should not have USB_DWC2_GINTSTS_INCOMPISOIN set).

tmon-nordic · 2025-06-03T07:12:49Z

drivers/usb/udc/udc_dwc2.c

 	if (dwc2_in_buffer_dma_mode(dev)) {
 		doepmsk |= USB_DWC2_DOEPINT_STSPHSERCVD;
 	}

 	sys_write32(doepmsk, (mem_addr_t)&base->doepmsk);
-	sys_set_bits((mem_addr_t)&base->diepmsk, USB_DWC2_DIEPINT_XFERCOMPL);
+	sys_set_bits((mem_addr_t)&base->diepmsk, USB_DWC2_DIEPINT_XFERCOMPL | USB_DWC2_DIEPINT_NAKINTRPT);


This will be performance killer if e.g. CDC ACM is used. On Linux xHCI host, opening ttyACM instance for given CDC ACM results in 5 IN/NAK every microframe.

tmon-nordic · 2025-06-03T07:13:49Z

drivers/usb/udc/udc_dwc2.c

@@ -2417,6 +2493,7 @@ static void dwc2_on_bus_reset(const struct device *dev)
 	uint32_t doepmsk;

 	/* Set the NAK bit for all OUT endpoints */
+	sys_clear_bits((mem_addr_t)&base->diepmsk, USB_DWC2_DIEPINT_NAKINTRPT);


Why clear bit to later set it?

tmon-nordic · 2025-06-03T07:22:10Z

drivers/usb/udc/udc_dwc2.c

 	uint32_t bcnt;
 	struct net_buf *buf;
 	uint32_t doeptsiz;
 	const bool is_iso = dwc2_ep_is_iso(ep_cfg);

 	doeptsiz = sys_read32((mem_addr_t)&base->out_ep[ep_idx].doeptsiz);

+	// HACK: since it seems like OUTTKNEPDIS doesn't work for non-control endpoints, 


I have no idea how the isochronous endpoint synchronization mechanism using OUTTKNEPDIS works/ever worked in Linux. If anything, that would be abusing some undocumented behavior because DWC2 documentation is clear that this applies only to control OUT endpoints.

In my testing, it never seems to trigger. So I'm at a loss there. My next steps would be to trace ISO in the Linux kernel, but I'm losing faith in it as the Bespoke driver.

tmon-nordic · 2025-06-03T07:39:17Z

drivers/usb/udc/udc_dwc2.c

@@ -2734,6 +2919,11 @@ static inline void dwc2_handle_oepint(const struct device *dev)
 		if (status & USB_DWC2_DOEPINT_XFERCOMPL) {
 			dwc2_handle_out_xfercompl(dev, n);
 		}
+
+		// Why does this work for linux gadget when docs suggest its only for control EP


However weird it may sound, are you sure that this part of the Linux gadget driver really works as described in comments?

NordicBuilder added the external label May 23, 2025

vbrzeski force-pushed the uac2-binterval branch 2 times, most recently from 1bcbd0b to d66bfbe Compare May 24, 2025 04:43

tmon-nordic requested changes May 26, 2025

View reviewed changes

vbrzeski force-pushed the uac2-binterval branch 6 times, most recently from 9eeb868 to 616e9c5 Compare May 31, 2025 21:09

Victor Brzeski added 2 commits June 2, 2025 09:40

usb: device_next: uac2: Support configurable polling rates

d9cc76c

dwc2: support ISO transfer scheduling

b87293f

Don't commit this, this is a proof of concept for now.

vbrzeski force-pushed the uac2-binterval branch from 616e9c5 to b87293f Compare June 3, 2025 00:03

tmon-nordic reviewed Jun 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

usb: device_next: uac2: Support configurable polling rates #2910

usb: device_next: uac2: Support configurable polling rates #2910

vbrzeski commented May 23, 2025

Uh oh!

NordicBuilder commented May 23, 2025

Uh oh!

tmon-nordic May 26, 2025 •

edited

Loading

Uh oh!

vbrzeski May 27, 2025

Uh oh!

tmon-nordic Jun 3, 2025

Uh oh!

tmon-nordic May 26, 2025

Uh oh!

vbrzeski commented Jun 3, 2025

Uh oh!

sonarqubecloud bot commented Jun 3, 2025

Uh oh!

tmon-nordic Jun 3, 2025

Uh oh!

tmon-nordic Jun 3, 2025

Uh oh!

tmon-nordic Jun 3, 2025

Uh oh!

tmon-nordic Jun 3, 2025

Uh oh!

tmon-nordic Jun 3, 2025

Uh oh!

tmon-nordic Jun 3, 2025

Uh oh!

vbrzeski Jun 3, 2025

Uh oh!

tmon-nordic Jun 3, 2025

Uh oh!

Uh oh!

		Input or output type reports polling period in microseconds. For USB full
		speed this could be clamped to 1ms or 255ms depending on the value.

usb: device_next: uac2: Support configurable polling rates #2910

Are you sure you want to change the base?

usb: device_next: uac2: Support configurable polling rates #2910

Conversation

vbrzeski commented May 23, 2025

Uh oh!

NordicBuilder commented May 23, 2025

Uh oh!

tmon-nordic May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbrzeski commented Jun 3, 2025

Uh oh!

sonarqubecloud bot commented Jun 3, 2025

Quality Gate failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tmon-nordic May 26, 2025 •

edited

Loading