Skip to content

{arm,xtensa}/otg: unmask WKUP so USB resume reaches the class driver. - #19936

Merged
xiaoxiang781216 merged 2 commits into
apache:masterfrom
dakejahl:fix/otg-device-unmask-wkup
Aug 25, 2026
Merged

{arm,xtensa}/otg: unmask WKUP so USB resume reaches the class driver.#19936
xiaoxiang781216 merged 2 commits into
apache:masterfrom
dakejahl:fix/otg-device-unmask-wkup

Conversation

@dakejahl

@dakejahl dakejahl commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes PX4/PX4-Autopilot#28340

The Synopsys DWC2-derived USB device drivers enable USBSUSP in GINTMSK but not WKUP, and every one of them ANDs GINTSTS with GINTMSK at the top of its ISR before dispatching. The WKUP branch and the *_resumeinterrupt() it calls are therefore dead code: CLASS_SUSPEND is delivered, CLASS_RESUME never is.

For CDC/ACM the asymmetry is fatal rather than merely lossy. cdcacm_suspend() calls uart_connected(&priv->serdev, false), and from then on serial.c refuses every board-side open() and write() with -ENOTCONN. The cdcacm_resume() that would call uart_connected(true) never runs, so the port stays unusable for the rest of the boot even though the device remains enumerated and the host has long since resumed it.

Adding WKUP to the mask makes the existing handler reachable. The status bit is already acknowledged in the same read/ack cycle as the rest of the writable interrupts, so unmasking it cannot latch.

The host-side check used to characterise this is in review for nuttx-ntfc-testing: apache/nuttx-ntfc-testing#4

Impact

Affects any board whose USB device controller is one of these eight drivers and whose host suspends it. Linux hosts do that by default: with power/control=auto and the usual autosuspend_delay_ms=2000, closing the tty is enough. Software that opens, reads and closes the port — as opposed to holding it open — kills the link on the first close, which is why this reads as an intermittent wedge rather than a deterministic bug.

No configuration, API or wire-format change. One extra interrupt source per suspend/resume, handled by code already present.

Drivers touched:

arch/arm/src/at32/at32_otgfsdev.c
arch/arm/src/common/stm32/stm32_otgfsdev_m3m4_v1.c
arch/arm/src/common/stm32/stm32_otghsdev_m3m4_v1.c
arch/arm/src/efm32/efm32_usbdev.c
arch/arm/src/stm32f7/stm32_otgdev.c
arch/arm/src/stm32h7/stm32_otgdev.c
arch/arm/src/stm32l4/stm32l4_otgfsdev.c
arch/xtensa/src/esp32s3/esp32s3_otg_device.c

Testing

Host: Linux 7.0.0-28-generic x86_64, xhci_hcd, usbcore.autosuspend=2.
Board: ARK FMU v6X (STM32H743), CONFIG_STM32H7_OTGFS=y, CONFIG_CDCACM=y (which selects SERIAL_REMOVABLE), NuttX 11.0.0 under PX4. Console on a separate STLINK-V3 VCP so it survives the CDC port dying.

Only the STM32H7 path was exercised on hardware. The other seven drivers carry a line-for-line copy of the same GINTMSK initialisation and the same masked-off resume handler.

Each cycle below forces a verified runtime suspend, resumes the device by opening the port, reads for 2 s, then asks the board over its console whether the CDC port is writable again.

Before:

device      /dev/serial/by-id/usb-ARK_ARK_FMU_v6X.x_0-if00 -> /dev/ttyACM1
usb         1-5.4  3185:0039  ARK FMU v6X.x
power       control=on autosuspend_delay_ms=2000 (restored on exit)

[1] suspended=True   read=12179   tail=0       board-side open failed: -ENOTCONN
[2] suspended=True   read=0       tail=0       board-side open failed: -ENOTCONN
[3] suspended=True   read=0       tail=0       board-side open failed: -ENOTCONN
[4] suspended=True   read=0       tail=0       board-side open failed: -ENOTCONN
[5] suspended=True   read=0       tail=0       board-side open failed: -ENOTCONN

cycles with a verified suspend: 5/5
of those, still streaming after resume: 0/5

FAIL: the link died after suspend and did not come back.

The 12179 bytes on the first cycle are the stale CONFIG_CDCACM_TXBUFSIZE=12000 TX buffer flushing on resume, not a working link — hence tail=0, the bytes seen in the last second of the window.

After:

[1] suspended=True   read=40898   tail=23316   board-side open ok
[2] suspended=True   read=42441   tail=22020   board-side open ok
[3] suspended=True   read=36945   tail=20372   board-side open ok
[4] suspended=True   read=44866   tail=22897   board-side open ok
[5] suspended=True   read=37565   tail=20480   board-side open ok

cycles with a verified suspend: 5/5
of those, still streaming after resume: 5/5

PASS: the link recovered from every suspend.

Board side on the unpatched build, after a single host suspend, the port is gone for good:

nsh> echo hi > /dev/ttyACM0
nsh: echo: open failed: Transport endpoint is not connected

Patched, the same command succeeds once the host resumes, and a 22 s suspend is followed by full recovery of the CDC stream (tx 21946 B/s, txerr 0.0 B/s, 217 kB read by the host over the following 10 s).

Build-tested beyond STM32H7: cubepilot_cubeyellow (stm32f7 OTGFS), airmind_mindpx-v2 (common/stm32 OTGFS), matek_gnss-m9n-f4 (common/stm32 OTGHS).

./tools/checkpatch.sh -c -u -m -g master..HEAD passes.

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected for:

Comment thread tools/usbdev_suspend_test.py Outdated
@dakejahl
dakejahl force-pushed the fix/otg-device-unmask-wkup branch from 7345f04 to 8096371 Compare August 23, 2026 18:02
@github-actions github-actions Bot added Size: S The size of the change in this PR is small and removed Area: Tooling Size: M The size of the change in this PR is medium labels Aug 23, 2026

@jerpelea jerpelea left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please write pr/commit title according to the contribution guidelines

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

@dakejahl please fix:

../nuttx/tools/checkpatch.sh -c -u -m -g  27e9ca7590ed0340000f46855b297f2b924281fb..HEAD
Used config files:
    1: .codespellrc
Error: /home/runner/work/nuttx/nuttx/nuttx/arch/arm/src/stm32h7/stm32_otgdev.c:3365:12: error: Bad right brace alignment
Some checks failed. For contributing guidelines, see:
  https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md
Error: Process completed with exit code 1.

@dakejahl
dakejahl force-pushed the fix/otg-device-unmask-wkup branch from 8096371 to 3e56a15 Compare August 24, 2026 17:10
Every DWC2-derived USB device driver enables USBSUSP in GINTMSK but not
WKUP, and every one of them ANDs GINTSTS with GINTMSK before dispatch.
The resume handler is therefore unreachable: CLASS_SUSPEND is delivered
on suspend, CLASS_RESUME never is.

For CDC/ACM that is fatal. cdcacm_suspend() calls uart_connected(false),
after which serial.c refuses every open() and write() with -ENOTCONN,
and the cdcacm_resume() that would clear it never runs. On a Linux host
with the default USB autosuspend (power/control=auto, 2000 ms) simply
closing the tty is enough to trip it, and the port stays dead for the
rest of the boot while the device remains enumerated.

Verified on STM32H7 (ARK FMU v6X): before, one host suspend leaves the
CDC/ACM port permanently -ENOTCONN; after, ten forced suspend/resume
cycles all recover with the MAVLink stream intact. The remaining
drivers carry a line-for-line copy of the same initialisation.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
@dakejahl dakejahl changed the title fix(arch/otg): unmask WKUP so USB resume reaches the class driver {arm,xtensa}/otg: unmask WKUP so USB resume reaches the class driver. Aug 24, 2026
@dakejahl
dakejahl force-pushed the fix/otg-device-unmask-wkup branch from 3e56a15 to 6dcc761 Compare August 24, 2026 17:23
@dakejahl
dakejahl requested a review from jerpelea August 24, 2026 17:28
nxstyle now checks case labels against their enclosing brace, and the
DWC2-derived OTG device drivers align every case label with the switch
brace itself, so any change that touches one of these files fails
checkpatch on hundreds of pre-existing lines.

Indent the switch bodies by two columns as the standard requires and
re-wrap the lines that this pushes past the width limit. Whitespace and
comment re-flow only; no code changes.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Jacob Dahl <dahl.jakejacob@gmail.com>
@dakejahl
dakejahl force-pushed the fix/otg-device-unmask-wkup branch from 0155b91 to 88e9ad1 Compare August 24, 2026 17:42
@github-actions github-actions Bot added Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. and removed Size: S The size of the change in this PR is small labels Aug 24, 2026
@dakejahl

Copy link
Copy Markdown
Contributor Author

Not really sure what's going on with the CI checks. Running locally is producing different results than the workflow. My Claude can't seem to figure it out which is surprising.

@xiaoxiang781216
xiaoxiang781216 merged commit b986fb8 into apache:master Aug 25, 2026
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: arm Issues related to ARM (32-bit) architecture Arch: xtensa Issues related to the Xtensa architecture Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] USB MAVLink dies permanently after the host closes the port (Linux USB autosuspend + CONFIG_SERIAL_REMOVABLE)

4 participants