drivers: udc: disable SOF interrupt by default #88072

jfischer-no · 2025-04-02T20:51:23Z

Add UDC Kconfig option to enable SOF interrupts.
Disable SOF interrupt in drivers that are maintainable and do not obfuscate how interrupts are enabled. If the new Kconfig option is disabled, no SOF events are passed to the higher layer.

Resolves: #87732

I also wanted to address #74058 in the same PR, but that requires more upfront work.

henrikbrixandersen · 2025-04-03T06:21:13Z

Disable SOF interrupt in drivers that are maintainable and do not obfuscate how interrupts are enabled.

Could you elaborate on this?

MarkWangChinese · 2025-04-03T07:48:06Z

Hi @jfischer-no Could I add one commit in your pr to disable USB_DEVICE_CONFIG_SOF_NOTIFICATIONS based on CONFIG_UDC_ENABLE_SOF for NXP EHCI and IP3511 controller wrapper driver.

tmon-nordic · 2025-04-03T09:34:37Z

samples/subsys/usb/uac2_explicit_feedback/prj.conf

 CONFIG_USBD_AUDIO2_CLASS=y
+CONFIG_UDC_ENABLE_SOF=y


Should the class depend on CONFIG_UDC_ENABLE_SOF? UAC2 cannot work at all without SOF. Changing the default here just makes the actual samples work, but does not prevent in any way configuring an completely not working software.

I can add depends on UDC_ENABLE_SOF. I was not sure when I finished the commit. I am thinking about moving the SoF call to be in driver thread context, which could also be meta IRQ or have its own SoF meta IRQ thread, but that needs to be per usbd_context or have its own message queue and is IMO too much. Unfortunately, few drivers use the UDC work queue.

I don't quite think meta IRQ is the way to go. One very important feature of SOF is that if the SW is too late, then all the past work does not really matter (any missed isochronous data is lost forever). The SW has always to worry only what to do within current SOF and the fact how many was missed is not really important.

Using queue for deferring SOF handling allows the queue to become full. I don't think it is about ensuring how fast the queue is handled, but rather about how to avoid the now-obsolete (past deadline) work.

tmon-nordic · 2025-04-03T09:38:43Z

drivers/usb/udc/udc_dwc2.c

+		    IF_ENABLED(CONFIG_UDC_ENABLE_SOF, (USB_DWC2_GINTSTS_SOF |))
+		    USB_DWC2_GINTSTS_INCOMPISOOUT | USB_DWC2_GINTSTS_INCOMPISOIN,


Isochronous transfers won't work without SOF and therefore there is no point in having INCOMPISOOUT and INCOMPISOIN enabled without SOF.

I don't quite understand. Will these interrupts be triggered even if SOF is disabled and must not be enabled here?

INCOMPISOIN/INCOMPISOOUT will be triggered if SOF is disabled if an isochronous transfer was scheduled and host did not send the IN or OUT token within first 80% of the (micro-)frame.

Isochronous transfers won't really work on DWC2 without SOF interrupt due to how the scheduling works (even/odd bit) and that this manual reschedule is required from driver.

jfischer-no · 2025-04-03T14:16:29Z

Disable SOF interrupt in drivers that are maintainable and do not obfuscate how interrupts are enabled.

Could you elaborate on this?

I will try it very briefly. What started with shim drivers in Zephyr, bad idea, is now some kind of interface to the HAL driver. HAL drivers, code that vendors claim to reuse, but that is not true, because then it would finally be a native driver; use Zephyr primitives, Kconfig options, and enable/disable IRQ in their own or obfuscated way.

jfischer-no · 2025-04-03T14:20:17Z

Hi @jfischer-no Could I add one commit in your pr to disable USB_DEVICE_CONFIG_SOF_NOTIFICATIONS based on CONFIG_UDC_ENABLE_SOF for NXP EHCI and IP3511 controller wrapper driver.

@MarkWangChinese Yes. You can also wait for it to be merged and open a PR to disable interrupts. The driver will not send SoF events anyway, but there will be ISR noise.

tmon-nordic · 2025-04-08T07:42:03Z

subsys/usb/device_next/class/Kconfig.uac2


 config USBD_AUDIO2_CLASS
 	bool "USB Audio 2 class support [EXPERIMENTAL]"
+	depends on UDC_ENABLE_SOF


Actually select UDC_ENABLE_SOF is better than depends on UDC_ENABLE_SOF because:

USB Audio 2 class support is then always visible in "New USB device stack" menu

The requirement is reflected by showing -*- SOF interrupt processing (can only be disabled if UAC2 is disabled)

With depends on UDC_ENABLE_SOF, disabling SOF interrupt processing disables USB Audio 2 class without any notice to the user.

This needs to be addressed.

I changed it to 'select' for now, but using 'depends on' is just fine and follows https://docs.zephyrproject.org/latest/build/kconfig/tips.html. We've already experienced a few terrible 'select -> depends on' changes in the past.

tmon-nordic · 2025-04-23T07:04:30Z

drivers/usb/udc/udc_common.h

+ * @brief Helper function to store UDC SOF stamp.
+ *
+ * @param[in] dev     Pointer to device struct of the driver instance
+ * @param[in] fnumber Frame number


While for Full-Speed this is enough, the description is insufficient regarding to what value is expected at High-Speed.

Universal Serial Bus Specification Revision 2.0 10.2.3 Frame and Microframe Generation

Host controllers operating with full-speed devices maintain a current frame number (at least 11 bits) that
increments at a 1 ms period. The host transmits the lower 11 bits of the current frame number in each SOF
token transmission.
Host controllers operating with high-speed devices maintain a current microframe number (at least 14 bits)
that increments at a 125 µs period. The host transmits bits 3 through 13 of the current microframe number
in each SOF token transmission. This results in the same SOF packet value being transmitted for eight
consecutive microframes before the SOF packet value increments.

Is the fnumber parameter supposed to be the actual microframe number (i.e. contain the bits 0 through 2, derived by device controller) when operating at High-Speed?

tmon-nordic · 2025-04-23T07:05:58Z

include/zephyr/drivers/usb/udc.h

 	struct net_buf *setup;
 	/** Driver private data */
 	void *priv;
+#if CONFIG_UDC_ENABLE_SOF


Can this new functionality be split away from this PR?

tmon-nordic · 2025-04-23T07:12:40Z

subsys/usb/device_next/usbd_core.c

+	/*
+	 * Queue the SOF events only if they are from the ISR context.
+	 * Primarily to avoid message queue flooding from high speed
+	 * controllers.
+	 */
+	if (event->type == UDC_EVT_SOF && !k_is_in_isr()) {
+		usbd_class_bcast_event(uds_ctx, event);
+		return 0;
+	}


What's the real point here? Isn't this just guarding against broken drivers? Shouldn't this be an assert?

tmon-nordic · 2025-04-23T07:12:48Z

subsys/usb/device_next/class/Kconfig.uac2


 config USBD_AUDIO2_CLASS
 	bool "USB Audio 2 class support [EXPERIMENTAL]"
+	depends on UDC_ENABLE_SOF


This needs to be addressed.

tmon-nordic · 2025-04-23T07:13:50Z

drivers/usb/udc/udc_dwc2.c

 	}

+	if (evt & BIT(DWC2_DRV_EVT_SOF)) {
+		udc_update_sof_stamp(dev, priv->sof_num);


This doesn't make sense. The timestamp will have a way too big jitter for any practical use.

Add helper to handle SOF interrupts/events and new Kconfig option to disable SOF interrupt. Signed-off-by: Johann Fischer <[email protected]>

If the new Kconfig option is disabled, no SOF events are passed to the higher layer. Signed-off-by: Johann Fischer <[email protected]>

sonarqubecloud · 2025-06-20T09:22:46Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
3.3% Duplication on New Code

See analysis details on SonarQube Cloud

jfischer-no added area: Drivers area: USB Universal Serial Bus Experimental Experimental features not enabled by default labels Apr 2, 2025

jfischer-no requested review from carlescufi, henrikbrixandersen and tmon-nordic April 2, 2025 20:51

github-actions bot added platform: Ambiq Ambiq platform: NXP Drivers NXP Semiconductors, drivers area: Samples Samples platform: STM32 ST Micro STM32 labels Apr 2, 2025

github-actions bot requested review from FRASTM, GeorgeCGV, djiatsaf-st, erwango, kartben, marwaiehm-st, mathieuchopstm, mmahadevan108 and nashif April 2, 2025 20:52

github-actions bot assigned mmahadevan108 and MarkWangChinese Apr 2, 2025

jfischer-no force-pushed the pr-resolve-87732 branch from 7550c10 to 9e31ee0 Compare April 2, 2025 20:58

tmon-nordic reviewed Apr 3, 2025

View reviewed changes

jfischer-no force-pushed the pr-resolve-87732 branch from 9e31ee0 to 4cc21ef Compare April 7, 2025 09:24

github-actions bot requested review from AlessandroLuo and MarkWangChinese April 7, 2025 09:24

AlessandroLuo previously approved these changes Apr 7, 2025

View reviewed changes

jfischer-no dismissed AlessandroLuo’s stale review via f2702ad April 7, 2025 15:31

jfischer-no force-pushed the pr-resolve-87732 branch from 4cc21ef to f2702ad Compare April 7, 2025 15:31

tmon-nordic reviewed Apr 8, 2025

View reviewed changes

jfischer-no force-pushed the pr-resolve-87732 branch 2 times, most recently from 173e047 to 0e18cab Compare April 22, 2025 11:28

tmon-nordic requested changes Apr 23, 2025

View reviewed changes

jfischer-no force-pushed the pr-resolve-87732 branch from 0e18cab to ff9c253 Compare June 20, 2025 08:41

github-actions bot added the platform: ITE ITE label Jun 20, 2025

jfischer-no added 2 commits June 20, 2025 11:17

drivers: udc: add SOF Kconfig option and SOF event helper

e65ca98

Add helper to handle SOF interrupts/events and new Kconfig option to disable SOF interrupt. Signed-off-by: Johann Fischer <[email protected]>

drivers: udc: disable SOF interrupt by default

b3d6234

If the new Kconfig option is disabled, no SOF events are passed to the higher layer. Signed-off-by: Johann Fischer <[email protected]>

jfischer-no force-pushed the pr-resolve-87732 branch from ff9c253 to b3d6234 Compare June 20, 2025 09:18

jfischer-no assigned jfischer-no and unassigned mmahadevan108 and MarkWangChinese Jun 20, 2025

jfischer-no added this to the v4.2.0 milestone Jun 20, 2025

tmon-nordic approved these changes Jun 23, 2025

View reviewed changes

carlescufi approved these changes Jun 23, 2025

View reviewed changes

henrikbrixandersen approved these changes Jun 23, 2025

View reviewed changes

fabiobaltieri merged commit 7b287ec into zephyrproject-rtos:main Jun 23, 2025
26 checks passed

jfischer-no deleted the pr-resolve-87732 branch June 23, 2025 14:51

		IF_ENABLED(CONFIG_UDC_ENABLE_SOF, (USB_DWC2_GINTSTS_SOF \|))
		USB_DWC2_GINTSTS_INCOMPISOOUT \| USB_DWC2_GINTSTS_INCOMPISOIN,

		CONFIG_USBD_AUDIO2_CLASS=y
		CONFIG_UDC_ENABLE_SOF=y

Uh oh!

drivers: udc: disable SOF interrupt by default #88072

drivers: udc: disable SOF interrupt by default #88072

Uh oh!

Conversation

jfischer-no commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henrikbrixandersen commented Apr 3, 2025

Uh oh!

MarkWangChinese commented Apr 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmon-nordic Apr 7, 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!

tmon-nordic Apr 7, 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!

jfischer-no commented Apr 3, 2025

Uh oh!

jfischer-no commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmon-nordic Apr 8, 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!

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!

sonarqubecloud bot commented Jun 20, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jfischer-no commented Apr 2, 2025 •

edited

Loading

tmon-nordic Apr 7, 2025 •

edited

Loading

tmon-nordic Apr 7, 2025 •

edited

Loading

jfischer-no commented Apr 3, 2025 •

edited

Loading

tmon-nordic Apr 8, 2025 •

edited

Loading