shared/tinyusb: Fix build errors with CDC support disabled. #17708

agatti · 2025-07-18T14:44:24Z

Summary

This PR makes it possible to build selected MicroPython ports that would have USB support enabled but CDC support disabled as well (whilst keeping MSC support for example).

The original code does support such a configuration but missed a few spots where build errors would arise. These changes fix the remaining issues, fixing also warnings caused by the changes needed to make the build succeed (like unused variables and so on).

Testing

This was checked by building the ESP32-S3 firmware with MICROPY_HW_USB_CDC set to 0, and then making sure the build process would succeed with that flag set back to 1. I also have another (unreleased) port where this behaviour was originally discovered and then fixed.

Trade-offs and Alternatives

There shouldn't be any impact, as these changes only affect a specific configuration whilst leaving the rest of the codebase as it is otherwise.

github-actions · 2025-07-18T14:53:31Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

codecov · 2025-07-18T14:53:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (ebc9525) to head (b7443bf).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #17708   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22239    22257   +18     
=======================================
+ Hits        21880    21898   +18     
  Misses        359      359

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dpgeorge · 2025-07-25T01:21:02Z

shared/tinyusb/mp_usbd_descriptor.c

@@ -95,10 +97,12 @@ const uint16_t *tud_descriptor_string_cb(uint8_t index, uint16_t langid) {
    if (desc_str == NULL) {
        // Fall back to the "static" string
        switch (index) {
+            #if CFG_TUD_CDC
            case USBD_STR_SERIAL:


USB devices should have a serial string, regardless of what descriptors are defined.

So I think this should stay unconditionally in the code.

Probably the fix is then to pull the definition of mp_usbd_port_get_serial_number() in the esp32 port outside it's #if conditional.

Indeed. Haven't looked at USB protocol details in a decade, I assumed that USBD_STR_SERIAL was related to CDC support somehow. Thanks for pointing it out, it should be fixed now.

agatti · 2025-07-25T10:04:14Z

I found build issues also on mimxrt, rp2 and samd, need to look into all the USB-enabled ports' behaviour without CDC so I can send a single PR fixing this for good.

There's a chance I may not be able to finish this in time for 1.26.0 - I suggest postponing this to 1.26.1 or 1.27.0 to be on the safe side. If I happen to finish it in time, then feel free to merge it in 1.26.0, I won't complain :)

This commit makes possible building MicroPython with USB CDC support disabled. The original code does support such a configuration but missed a few spots where build errors would arise. These changes fix the remaining issues, fixing also warnings caused by the changes needed to make the build succeed. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>

agatti · 2025-07-25T14:11:40Z

I believe this should be it for now - I expected it to take longer but:

alif, nrf, and renesas-ra do the right thing when it comes to CDC
stm32 looks like it needs at least one CDC port in order to remap that to MSC if needed? I'm not sure if making it compile with MICROPY_HW_USB_CDC_NUM set to 0 is the right thing to do; there's no other obvious way to disable CDC on that port (at least for my NUCLEO_F767ZI, maybe I've missed something). Edit: stm32/usb: Add support for using TinyUSB stack. #15592 already takes care of that, so it should be good to go w.r.t. building with CDC disabled :)

This will also fix cases where CDC is disabled but 1200BPS_TOUCH is not - given that they are handled separately in the integration code I assume they can be enabled independently of each other. If it's not the case, then it's a matter of removing "class/cdc/cdc_device.h" from shared/tinyusb/mp_usbd.h's includes.

There are also other minor issues I've found, like samd not building if USB is disabled altogether and so on, but that's something I can send a PR for at another time (or even better, somebody else doing it :)). To keep the PR scope limited, I've only looked at making ports build using their default configuration but with USB CDC disabled.

dpgeorge · 2025-07-29T04:39:17Z

ports/esp32/usb.c


 #include "shared/tinyusb/mp_usbd.h"

+#if MICROPY_HW_USB_CDC


If MICROPY_HW_ENABLE_USBDEV is enabled but MICROPY_HW_USB_CDC disabled, then usb_init() below is never compiled in. So that gives a link error if you disable CDC (and also USB_SERIAL_JTAG) on, eg, ESP32S3.

I just built this using ESP-IDF 5.4 with this PR applied:

[/micropython/ports/esp32] $ git diff . diff --git a/ports/esp32/mpconfigport.h b/ports/esp32/mpconfigport.h index 721f22de1..89d08eae4 100644 --- a/ports/esp32/mpconfigport.h +++ b/ports/esp32/mpconfigport.h @@ -258,7 +258,7 @@ // Enable stdio over native USB peripheral CDC via TinyUSB #ifndef MICROPY_HW_USB_CDC -#define MICROPY_HW_USB_CDC (MICROPY_HW_ENABLE_USBDEV) +#define MICROPY_HW_USB_CDC (0) #endif // Enable stdio over USB Serial/JTAG peripheral [/micropython/ports/esp32] $ make BOARD=ESP32_GENERIC_S3 clean Use make V=1 or set BUILD_VERBOSE in your environment to increase build verbosity. Executing action: fullclean Executing action: remove_managed_components Done [/micropython/ports/esp32] $ make BOARD=ESP32_GENERIC_S3 -j6 Use make V=1 or set BUILD_VERBOSE in your environment to increase build verbosity. Executing action: all (aliases: build) Running cmake in directory /micropython/ports/esp32/build-ESP32_GENERIC_S3 ... [1468/1469] Generating binary image from built executable esptool.py v4.9.0 Creating esp32s3 image... Merged 3 ELF sections Successfully created esp32s3 image. Generated /micropython/ports/esp32/build-ESP32_GENERIC_S3/micropython.bin [1469/1469] cd /micropython/ports/esp32/build-ESP32_GENERIC_S3/esp-idf/esptool_py &&.../partition-table.bin /micropython/ports/esp32/build-ESP32_GENERIC_S3/micropython.bin micropython.bin binary size 0x18bd90 bytes. Smallest app partition is 0x1f0000 bytes. 0x64270 bytes (20%) free. Project build complete. To flash, run: idf.py flash or idf.py -p PORT flash or python -m esptool --chip esp32s3 -b 460800 --before default_reset --after hard_reset write_flash --flash_mode dio --flash_size 4MB --flash_freq 80m 0x0 build-ESP32_GENERIC_S3/bootloader/bootloader.bin 0x8000 build-ESP32_GENERIC_S3/partition_table/partition-table.bin 0x10000 build-ESP32_GENERIC_S3/micropython.bin or from the "/micropython/ports/esp32/build-ESP32_GENERIC_S3" directory python -m esptool --chip esp32s3 -b 460800 --before default_reset --after hard_reset write_flash "@flash_args" bootloader @0x000000 19152 ( 13616 remaining) partitions @0x008000 3072 ( 1024 remaining) application @0x010000 1621392 ( 410224 remaining) total 1686928

I'll try with USB_SERIAL_JTAG disabled right now.

You were right - it did fail to build with JTAG Serial disabled. This has been taken care of, thank you!

dpgeorge · 2025-07-29T04:41:31Z

ports/rp2/mphalport.c

@@ -83,11 +83,12 @@ int mp_hal_stdin_rx_chr(void) {
        #if MICROPY_HW_USB_CDC
        mp_usbd_cdc_poll_interfaces(0);
        #endif
-
+        #if MICROPY_HW_ENABLE_UART_REPL || MICROPY_PY_OS_DUPTERM_NOTIFY


I'm not sure why this guard is added, because shared/tinyusb/mp_usbd_cdc.c pushes to stdin_ringbuf.

But so do uart_irq and mp_os_dupterm_notify. This guard covers those usages (not to mention it replicates the one used when defining stdin_ringbuf once the CDC part is taken care of).

But if CDC is enabled, and UART_REPL and DUPTERM_NOTIFY disabled, won't that mean input from CDC will no longer work?

Right! However without that guard it won't compile if CDC and UART and dupterm are disabled (often having to disable UART/debugging support of any kind is part of the security checklist of a product).

I'll update things to make it work in that case as well.

I've updated the guard so it should compile with all possible valid combinations of cdc, uart, and dupterm. Peeking from the ringbuffer is now performed if one or more of those three flags are set, matching the ringbuffer's backing buffer definition.

But this whole section of code is guarded with #if MICROPY_HW_ENABLE_UART_REPL || MICROPY_HW_USB_CDC || MICROPY_PY_OS_DUPTERM_NOTIFY so it's redundant to have that same guard here. What does it gain?

dpgeorge · 2025-07-29T04:42:22Z

shared/tinyusb/mp_usbd.h

@@ -38,13 +38,16 @@
 #ifndef NO_QSTR
 #include "tusb.h"
 #include "device/dcd.h"
+#include "class/cdc/cdc_device.h"


Is this extra include really needed?

This fixes samd (and mimxrt if memory serves) if CDC is disabled but MICROPY_HW_USB_CDC_1200BPS_TOUCH is left enabled. With MICROPY_HW_USB_CDC set to 0 and this include commented out this error occurs on the ADAFRUIT_ITSYBITSY_M4_EXPRESS board (the default):

../../shared/tinyusb/mp_usbd_cdc.c: In function 'tud_cdc_line_state_cb': ../../shared/tinyusb/mp_usbd_cdc.c:184:9: error: unknown type name 'cdc_line_coding_t' 184 | cdc_line_coding_t line_coding; | ^~~~~~~~~~~~~~~~~ ../../shared/tinyusb/mp_usbd_cdc.c:185:9: error: implicit declaration of function 'tud_cdc_n_get_line_coding' [-Werror=implicit-function-declaration] 185 | tud_cdc_n_get_line_coding(itf, &line_coding); | ^~~~~~~~~~~~~~~~~~~~~~~~~ ../../shared/tinyusb/mp_usbd_cdc.c:186:24: error: request for member 'bit_rate' in something not a structure or union 186 | if (line_coding.bit_rate == 1200) { | ^ cc1: all warnings being treated as errors

This commit fixes the linking issues in the RP2 port that arise when explicitly disabling USB CDC support. The console input ringbuffer needs to be made available if dupterm support is enabled, and the console character input function would always attempt to read from the input ringbuffer even if CDC support is disabled and the console isn't bound to any UART. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>

This commit fixes the build issues in the MIMXRT port that arise when explicitly disabling USB CDC support. The console code assumed CDC support is always enabled even if it was disabled in mpconfigport.h. These changes make accessing CDC conditional to that support configuration being enabled. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>

This commit fixes the build issues in the SAMD port that arise when explicitly disabling USB CDC support. The console code assumed CDC support is always enabled even if it was disabled in mpconfigport.h. These changes make accessing CDC conditional to that support configuration being enabled. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>

This commit fixes the build issues in the ESP32 port that arise when explicitly disabling USB CDC support. The USB support code gated the USB serial number retrieval behind the USB CDC support definition, even though all USB devices must be able to be queried for their serial number. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>

agatti · 2025-07-30T17:25:29Z

The nrf compile error doesn't seem to be related to this PR. (Although I was worried I messed something up in the tinyusb integration...)

dpgeorge added the shared Relates to shared/ directory in source label Jul 25, 2025

dpgeorge reviewed Jul 25, 2025

View reviewed changes

dpgeorge added this to the release-1.26.0 milestone Jul 25, 2025

agatti force-pushed the usb-fix-no-cdc-build branch 2 times, most recently from a0f92cd to a84adf2 Compare July 25, 2025 12:34

agatti force-pushed the usb-fix-no-cdc-build branch from a84adf2 to 4030666 Compare July 25, 2025 20:48

dpgeorge reviewed Jul 29, 2025

View reviewed changes

agatti force-pushed the usb-fix-no-cdc-build branch from 4030666 to ba076cb Compare July 29, 2025 10:34

agatti added 4 commits July 30, 2025 19:18

agatti force-pushed the usb-fix-no-cdc-build branch from ba076cb to b7443bf Compare July 30, 2025 17:18

dpgeorge modified the milestones: release-1.26.0, release-1.27.0 Aug 1, 2025

Uh oh!

shared/tinyusb: Fix build errors with CDC support disabled. #17708

Are you sure you want to change the base?

shared/tinyusb: Fix build errors with CDC support disabled. #17708

Conversation

agatti commented Jul 18, 2025

Summary

Testing

Trade-offs and Alternatives

Uh oh!

github-actions bot commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agatti commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agatti commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agatti Jul 29, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agatti commented Jul 30, 2025

Uh oh!

Uh oh!

github-actions bot commented Jul 18, 2025 •

edited

Loading

codecov bot commented Jul 18, 2025 •

edited

Loading

agatti commented Jul 25, 2025 •

edited

Loading

agatti commented Jul 25, 2025 •

edited

Loading

agatti Jul 29, 2025 •

edited

Loading