Add USB SuperSpeed (USB 3.2) device support with WCH CH569 and CH32H417 ports - #3779
Add USB SuperSpeed (USB 3.2) device support with WCH CH569 and CH32H417 ports#3779hathach wants to merge 72 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // EP0 transaction size: a SuperSpeed-capable build (EP0 = 512) can operate on a USB2 fallback | ||
| // link where EP0 max packet size is 64 | ||
| TU_ATTR_ALWAYS_INLINE static inline uint16_t ep0_xact_limit(void) { |
| switch (p_request->wValue) { //-V2520 | ||
| case TUSB_REQ_FEATURE_REMOTE_WAKEUP: | ||
| TU_LOG_USBD(" Disable Remote Wakeup\r\n"); | ||
| // Host may disable remote wake up after resuming | ||
| _usbd_dev.remote_wakeup_en = 0; | ||
| tud_control_status(rhport, p_request); | ||
| return true; | ||
|
|
||
| // Host may disable remote wake up after resuming | ||
| _usbd_dev.remote_wakeup_en = 0; | ||
| tud_control_status(rhport, p_request); | ||
| return true; | ||
| #if TUD_OPT_SUPER_SPEED | ||
| case TUSB_REQ_FEATURE_U1_ENABLE: | ||
| case TUSB_REQ_FEATURE_U2_ENABLE: | ||
| tud_control_status(rhport, p_request); | ||
| return true; | ||
| #endif | ||
|
|
||
| // Stall unsupported feature selector | ||
| default: return false; | ||
| } |
There was a problem hiding this comment.
Pull request overview
This PR introduces first-class USB SuperSpeed (USB 3.2 Gen1) device support to TinyUSB, adding core stack capability for SuperSpeed descriptors/control requests and integrating two new WCH SuperSpeed-capable MCU ports (CH569 and CH32H417) plus related examples and HIL automation updates.
Changes:
- Add SuperSpeed capability flags/options and core stack handling (EP0 sizing/chunking, SS requests, BOS + endpoint companion descriptor handling).
- Add WCH CH569 + CH32H417 MCU/board integrations (deps, BSP scaffolding, CI matrix/presets, example SS descriptors/configs).
- Extend unit tests + HIL tooling (usbtest quirk skips, WCH UART flash helper, updated flashing flows).
Reviewed changes
Copilot reviewed 125 out of 125 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/get_deps.py | Adds WCH CH32H417 + CH569 SDK dependencies. |
| test/unit-test/test/support/tusb_config.h | Enables SS code paths in unit-test config. |
| test/unit-test/test/device/usbd/test_usbd.c | Adds SS-focused USBD unit tests (validate, companion skipping, SET_SEL/SET_ISOCH_DELAY). |
| test/hil/wch_uart_flash.py | New stdlib-only UART flasher helper for CH32H417 rig use. |
| test/hil/usbtest.py | Adds bcdDevice quirk-driven per-case SKIP logic + reporting. |
| test/hil/tinyusb-sudoer | Adds documented sudoers allowlist template for HIL rigs. |
| test/hil/hil_test.py | Updates WCH flashing flows, adds UART-loader flashing path, adjusts usbtest timeouts. |
| test/hil/hfp.json | Skips usbtest for a wedging board with an explanatory comment. |
| src/tusb.c | Adds SuperSpeed selection + SS-aware endpoint validation + “Super” debug string. |
| src/tusb_option.h | Adds OPT_MODE_SUPER_SPEED + TUD_OPT_SUPER_SPEED + SS EP0 size validation. |
| src/portable/wch/dcd_ch56x.h | Adds CH56x shared USB2/USB3 DCD interface + DMA constraints helper. |
| src/portable/wch/dcd_ch32h417.h | Adds CH32H417 shared USB2/USB3 DCD interface. |
| src/portable/wch/ch32h417_usbhs_reg.h | Adds CH32H417 USBHS register access helpers + static layout checks. |
| src/portable/wch/ch32h417_usb30_reg.h | Adds CH32H417 USBSS register helpers + PHY backdoor defs + layout checks. |
| src/device/usbd.c | Adds SS control handling (SET_SEL/SET_ISOCH_DELAY, U1/U2 feature selectors), EP0 chunking for HS fallback, companion skipping in open pair. |
| src/device/usbd_pvt.h | Adds helper to skip SS endpoint companion descriptors. |
| src/common/tusb_types.h | Extends core types for SuperSpeed (speed enum, SS MPS constants, SS requests/features, SS descriptors, BOS caps). |
| src/common/tusb_mcu.h | Adds CH569/CH32H417 MCU config blocks and superspeed capability defaults. |
| src/class/usbtmc/usbtmc_device.c | Makes USBTMC buffers SS-sized and adjusts SS companion accounting. |
| src/class/printer/printer_device.c | Skips SS endpoint companion descriptors during open parsing. |
| src/class/net/ncm_device.c | Skips SS companion descriptors + accounts SS companion sizing. |
| src/class/net/ecm_rndis_device.c | Skips SS companions + accounts SS companion sizing. |
| src/class/mtp/mtp_device.c | Accounts SS companions in interface sizing + uses SS companion skip helper. |
| src/class/msc/msc_device.c | Accounts SS companions in driver length. |
| src/class/midi/midi2_device.c | Allows SS companion descriptors in descriptor iteration. |
| src/class/midi/midi_device.c | Uses SS companion skip helper after CS endpoint descriptor. |
| src/class/hid/hid_device.c | Accounts SS companions in driver length. |
| src/class/cdc/cdc_device.c | Uses SS companion skip helper during CDC endpoint parsing. |
| hw/bsp/ch569/wch-riscv.cfg | Adds OpenOCD wlinke SDI config for CH569. |
| hw/bsp/ch569/family.mk | Adds CH569 make-family build (USBSS/USBHS selection, fallback, DMA section/alignment). |
| hw/bsp/ch569/family.cmake | Adds CH569 CMake family support. |
| hw/bsp/ch569/debug_uart.h | Adds CH569 debug UART API header. |
| hw/bsp/ch569/debug_uart.c | Adds CH569 non-blocking UART TX implementation. |
| hw/bsp/ch569/CH56xSFR.H | Case-sensitive FS include shim for vendor EVT header naming. |
| hw/bsp/ch569/boards/hydrausb3_v1/board.mk | HydraUSB3 v1 make-board RAM/flash sizing flags. |
| hw/bsp/ch569/boards/hydrausb3_v1/board.h | HydraUSB3 v1 board pin/metadata definitions. |
| hw/bsp/ch569/boards/hydrausb3_v1/board.cmake | HydraUSB3 v1 CMake-board settings. |
| hw/bsp/ch32h417/wch-riscv.cfg | Adds OpenOCD wlinke SDI config for CH32H417. |
| hw/bsp/ch32h417/system_ch32h417.h | Adds BSP-owned system header expected by SDK. |
| hw/bsp/ch32h417/family.mk | Adds CH32H417 make-family build (USBSS/USBHS selection, fallback notes). |
| hw/bsp/ch32h417/family.cmake | Adds CH32H417 CMake family support. |
| hw/bsp/ch32h417/ch32h417_conf.h | Adds BSP-owned SDK config header with only required peripherals. |
| hw/bsp/ch32h417/boards/nanoch32h417/board.mk | nanoCH32H417 make-board stub. |
| hw/bsp/ch32h417/boards/nanoch32h417/board.h | nanoCH32H417 board pin/metadata definitions. |
| hw/bsp/ch32h417/boards/nanoch32h417/board.cmake | nanoCH32H417 CMake-board stub. |
| hw/bsp/BoardPresets.json | Adds build presets for hydrausb3_v1 and nanoch32h417. |
| examples/device/webusb_serial/src/tusb_config.h | SS-capable EP0 + SS-sized CDC/Vendor buffers/epsizes. |
| examples/device/video_capture/skip.txt | Skips CH569/CH32H417 for video example. |
| examples/device/video_capture_2ch/skip.txt | Skips CH569/CH32H417 for video 2ch example. |
| examples/device/usbtmc/src/tusb_config.h | Makes EP0 SS-aware. |
| examples/device/usbtest/src/usb_descriptors.h | Adds bcdDevice-advertised SS quirk bits for host script. |
| examples/device/usbtest/src/tusb_config.h | Makes EP0 SS-aware + sizes vendor RX ep for SS. |
| examples/device/usbtest/src/main.c | Treats SS like HS for interrupt/iso length selection. |
| examples/device/uac2_speaker_fb/skip.txt | Skips CH569/CH32H417 for UAC2 speaker example. |
| examples/device/uac2_headset/skip.txt | Skips CH569/CH32H417 for UAC2 headset example. |
| examples/device/printer_to_cdc/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/printer_to_cdc/src/tusb_config.h | SS-aware EP0 + SS-sized CDC/printer buffers/epsizes. |
| examples/device/net_lwip_webserver/src/tusb_config.h | SS-aware EP0 + WCH RAMX pool/EP max tuning for fit. |
| examples/device/net_lwip_webserver/src/lwipopts.h | WCH-specific lwIP tuning for SS/HS memory + checksum verify skip. |
| examples/device/net_lwip_webserver/src/arch/cc.h | Places lwIP heap/pools in WCH .dmadata section for RAM fit. |
| examples/device/mtp/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/mtp/src/tusb_config.h | SS-aware EP0 + SS-sized MTP bulk buffer. |
| examples/device/msc_dual_lun/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/msc_dual_lun/src/tusb_config.h | SS-aware EP0 + SS-sized MSC buffer. |
| examples/device/midi2_device/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + SS MIDI descriptors. |
| examples/device/midi2_device/src/tusb_config.h | SS-aware EP0. |
| examples/device/midi_test/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/midi_test/src/tusb_config.h | SS-aware EP0 + SS-sized MIDI FIFOs. |
| examples/device/midi_test_freertos/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/midi_test_freertos/src/tusb_config.h | SS-aware EP0 + SS-sized MIDI FIFOs. |
| examples/device/midi_test_freertos/skip.txt | Skips CH569/CH32H417 for this FreeRTOS example. |
| examples/device/hid_multiple_interface/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + SS HID configs. |
| examples/device/hid_multiple_interface/src/tusb_config.h | SS-aware EP0. |
| examples/device/hid_generic_inout/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + SS HID INOUT config. |
| examples/device/hid_generic_inout/src/tusb_config.h | SS-aware EP0. |
| examples/device/hid_composite/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/hid_composite/src/tusb_config.h | SS-aware EP0. |
| examples/device/hid_composite_freertos/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/hid_composite_freertos/src/tusb_config.h | SS-aware EP0. |
| examples/device/hid_composite_freertos/skip.txt | Skips CH569/CH32H417 for this FreeRTOS example. |
| examples/device/hid_boot_interface/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + SS HID boot config. |
| examples/device/hid_boot_interface/src/tusb_config.h | SS-aware EP0. |
| examples/device/dynamic_configuration/src/tusb_config.h | SS-aware EP0 + SS-sized class buffers (CDC/MIDI/MSC). |
| examples/device/dfu/src/usb_descriptors.c | Adds SS device/config support + SS BOS capability count. |
| examples/device/dfu/src/tusb_config.h | SS-aware EP0 + SS-sized DFU transfer buffer. |
| examples/device/dfu_runtime/src/usb_descriptors.c | Adds SS device/config support + SS BOS capability count. |
| examples/device/dfu_runtime/src/tusb_config.h | SS-aware EP0. |
| examples/device/cdc_uac2/skip.txt | Skips CH569/CH32H417 for CDC+UAC2 example. |
| examples/device/cdc_msc/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/cdc_msc/src/tusb_config.h | SS-aware EP0 + SS-sized CDC/MSC buffers. |
| examples/device/cdc_msc_throughput/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/cdc_msc_throughput/src/tusb_config.h | SS-aware EP0 + SS-sized throughput buffers. |
| examples/device/cdc_msc_throughput/src/main.c | Adds LED heartbeat task (noted as load-bearing on CH32H417 USBSS). |
| examples/device/cdc_msc_freertos/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/cdc_msc_freertos/src/tusb_config.h | SS-aware EP0 + SS-sized CDC/MSC buffers. |
| examples/device/cdc_msc_freertos/skip.txt | Skips CH569/CH32H417 for this FreeRTOS example. |
| examples/device/cdc_dual_ports/src/usb_descriptors.c | Adds SS device/config/BOS descriptors + EP0 FS/HS sizing fix. |
| examples/device/cdc_dual_ports/src/tusb_config.h | SS-aware EP0 + SS-sized CDC buffers. |
| examples/device/audio_test/skip.txt | Skips CH569/CH32H417 for audio test. |
| examples/device/audio_test_multi_rate/skip.txt | Skips CH569/CH32H417 for multi-rate audio test. |
| examples/device/audio_test_freertos/skip.txt | Skips CH569/CH32H417 for FreeRTOS audio test. |
| examples/device/audio_4_channel_mic/skip.txt | Skips CH569/CH32H417 for 4ch mic example. |
| examples/device/audio_4_channel_mic_freertos/skip.txt | Skips CH569/CH32H417 for FreeRTOS 4ch mic example. |
| docs/reference/dependencies.rst | Documents new WCH SDK dependencies and revisions. |
| docs/reference/boards.rst | Adds nanoch32h417 and hydrausb3_v1 to board reference table. |
| .github/workflows/ci_set_matrix.py | Adds ch32h417 and ch569 families to CI toolchain matrix. |
| .claude/skills/usbtest/SKILL.md | Documents SS quirk-skip reporting + dynamic-id safety details. |
| .claude/skills/usb-debug/SKILL.md | Updates usb-debug deployment guidance for rig environments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // SuperSpeed: advance past an endpoint companion descriptor if present | ||
| TU_ATTR_ALWAYS_INLINE static inline | ||
| const uint8_t* usbd_skip_ss_ep_companion(const uint8_t* p_desc, const uint8_t* desc_end) { | ||
| if (TUD_OPT_SUPER_SPEED && tu_desc_in_bounds(p_desc, desc_end) && | ||
| TUSB_DESC_SUPERSPEED_ENDPOINT_COMPANION == tu_desc_type(p_desc)) { | ||
| p_desc = tu_desc_next(p_desc); | ||
| } | ||
| return p_desc; | ||
| } |
| # Quirk flags advertised in bcdDevice bits 4-7 (tier is bits 0-3), see usb_descriptors.h. | ||
| # Each flag maps to the cases the silicon cannot pass at SuperSpeed (5 Gbps). | ||
| QUIRK_SKIPS = { | ||
| 0x10: ((14, 21), 'EP0 OUT data lost at 5 Gbps (silicon erratum)'), | ||
| 0x20: ((13,), 'halted EP answers a single STALL at 5 Gbps (silicon limitation)'), |
| if b in (bytes([c]) for c in want): | ||
| return True | ||
| if b not in (bytes([c]) for c in ok_noise): | ||
| # unexpected byte (e.g. 'E'/'X'): fail fast | ||
| return False |
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in size
Changes <1% in size
No changes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0deec1784
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { // TEMP: poll link status ~6 s post-attach (MIS_ST / INT_FG / SPEED_TYPE) | ||
| for (int t = 0; t < 6; t++) { | ||
| for (volatile uint32_t i = 0; i < 25000000u; i++) {} |
There was a problem hiding this comment.
When the CH32H417 USB2 path is used directly (SPEED=high) or via USB3 fallback, this temporary poll runs after USBHSD->CONTROL enables the device but before dcd_int_enable() can enable USBHS interrupts. That leaves the device attached for roughly six seconds without servicing reset/setup traffic, so hosts can time out or abandon enumeration before the driver starts handling the bus; this looks like leftover diagnostics and should be removed or moved behind an explicit debug build guard before enabling the controller.
Useful? React with 👍 / 👎.
| uint32_t status = USBSSD->USB_STATUS; | ||
|
|
||
| if ((status & USBSS_UDIF_SETUP) && !(status & USBSS_UDIF_STATUS)) { | ||
| USBSSD->USB_STATUS = USBSS_UDIF_SETUP; |
There was a problem hiding this comment.
For CH32H417 SuperSpeed enumeration, this clears USBSS_UDIF_SETUP before handle_setup() defers the response to the TinyUSB task, but the setup_flag_held mechanism above says the flag must stay asserted until the first EP0 arm/stall so the controller keeps NRDYing the host. With the flag released here, a host that asks for the data/status stage before task context arms EP0 can hit an unarmed control endpoint and time out; set the hold/mask path instead of clearing the flag immediately.
Useful? React with 👍 / 👎.
| NVIC_DisableIRQ(USBSS_IRQn); | ||
| NVIC_DisableIRQ(USBSS_LINK_IRQn); |
There was a problem hiding this comment.
When the CH32H417 SuperSpeed DCD is initialized with USB2 fallback, dcd_init() starts TIM12 via fallback_timer_start(true), and the retrain backoff can also arm it; however this disable path only masks the USBSS vectors. After tud_deinit()/tusb_deinit(), the shared ISR no longer dispatches to dcd_int_handler() once the role is invalid, so a still-enabled TIM12 update flag can interrupt continuously without being cleared; stop/disable TIM12 here as well.
Useful? React with 👍 / 👎.
|
|
||
| .bMaxPacketSize0 = 9, | ||
| .idVendor = 0xCafe, | ||
| .idProduct = USB_PID + 11, // should be different PID than desc0 |
There was a problem hiding this comment.
When the dynamic-configuration example is in mode 1 at SuperSpeed, this advertises 0x400d + 11 == 0x4018, which collides with net_lwip_webserver and differs from the FS/HS mode-1 descriptor that deliberately uses USB_PID + 0x0100 to stay outside the per-example PID range. That breaks the fresh host driver/configuration match the examples rely on when reflashing or switching modes; use the same out-of-band PID for the SuperSpeed mode-1 descriptor.
Useful? React with 👍 / 👎.
| if (tud_speed_get() == TUSB_SPEED_SUPER) { | ||
| return desc_ss_configuration; |
There was a problem hiding this comment.
When a SuperSpeed build runs on CH569's default USB2 fallback or another USB2-only connection, tud_speed_get() is TUSB_SPEED_HIGH, so this SuperSpeed-only branch is skipped and the callback falls through to desc_fs_configuration; that descriptor's MIDI bulk endpoints are 64 bytes, and usbd_edpt_open() rejects high-speed bulk endpoints unless MPS is 512. The example will fail to configure after fallback; add a HS configuration before returning the FS descriptor.
Useful? React with 👍 / 👎.
| TU_VERIFY(6u == p_request->wLength); | ||
| return tud_control_xfer(rhport, p_request, _ctrl_epbuf.buf, 6); |
There was a problem hiding this comment.
On SuperSpeed, SET_SEL is an OUT device request with a 6-byte data stage; if a host sends the same bRequest as an IN request with wLength == 6, this accepts it and tud_control_xfer() transmits six bytes from _ctrl_epbuf instead of stalling. That can leak stale control-buffer contents and leaves the control transfer out of spec; validate the request direction and zero fields before queuing the transfer.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| { // TEMP: poll link status ~6 s post-attach (MIS_ST / INT_FG / SPEED_TYPE) | ||
| for (int t = 0; t < 6; t++) { | ||
| for (volatile uint32_t i = 0; i < 25000000u; i++) {} | ||
| char s[12]; | ||
| const uint8_t ms = USBHSD->MIS_ST, fg = USBHSD->INT_FG, sp = (uint8_t)USBHSD->BUS; | ||
| const char hx[] = "0123456789abcdef"; | ||
| s[0]='s'; s[1]=hx[ms>>4]; s[2]=hx[ms&15]; s[3]=' '; | ||
| s[4]=hx[fg>>4]; s[5]=hx[fg&15]; s[6]=' '; | ||
| s[7]=hx[sp>>4]; s[8]=hx[sp&15]; s[9]='\r'; s[10]='\n'; | ||
| board_uart_write(s, 11); | ||
| } | ||
| } |
There was a problem hiding this comment.
Leftover // TEMP bring-up instrumentation — fails to build and stalls enumeration. This block reads as debug code left in from hardware bring-up, and has two concrete problems:
-
Build failure.
board_uart_write()(line 242) is declared only inhw/bsp/board_api.h, which this DCD translation unit does not include — it pulls in onlytusb_option.h,device/dcd.h,dcd_ch32h417.h,ch32h417_usbhs_reg.h. The call is therefore an implicit function declaration, which trips-Werror-implicit-function-declaration(set inhw/bsp/family_support.cmake) for exactly the configs that compile this file: the standalone USBHS build (SPEED=high) and the SuperSpeed→USB2 fallback build (CFG_TUD_WCH_USB30_FALLBACK=1). (The default SS-only preset compiles this file to nothing via the top-of-file#if, which is why the default CI build doesn't surface it.) It also reaches from the device stack into the BSP, crossing the layering boundary. -
~6 s busy-wait on the init path. Even setting the build aside, the loop spins ~6×25M
volatileiterations synchronously insidech32h417_usb2_init()— reached from bothdcd_init()and the USB3→USB2fallback_to_usb2()path — which stalls the usbd task through the enumeration window.
Recommend deleting this block (lines 233-244) before merge.
| #define usbss_status_ep_num(st) (((st) >> 8) & 7u) | ||
| #define usbss_status_ep_is_in(st) (((st) >> 12) & 1u) |
There was a problem hiding this comment.
Minor / CLAUDE.md consistency: these two function-like macros are lowercase, but the repo's CLAUDE.md requires UPPER_CASE macros. Every other macro in this file follows that — including the argument-taking ones a few lines below (USBSS_EP_R_EN(ep), USBSS_EP_T_EN(ep), …) — and the sibling ch32h417_usb30_reg.h:54-55 implements the same accessors as USBSS_STATUS_EP_NUM / USBSS_STATUS_EP_IN. Suggest renaming to match; the two call sites in dcd_ch56x_usb30.c:598-599 would need updating too.
Hardware-in-the-loop (HIL) Test Reporthfp-iar✅ 55 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run
hfp.json✅ 55 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run
tinyusb-esp.json✅ 22 passed · ❌ 2 failed · ⚪ 0 skipped · blank not run
tinyusb.json✅ 353 passed · ❌ 28 failed · ⚪ 17 skipped · blank not run
|
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| ch582m_evt/dfu_runtime | 6,564 → 6,820 (+256) | — | — | — | 17,444 → 14,836 (-2,608) | -15.0% |
| ch582m_evt/hid_generic_inout | 7,140 → 7,420 (+280) | — | — | — | 18,240 → 15,656 (-2,584) | -14.2% |
| ch582m_evt/hid_multiple_interface | 7,644 → 7,916 (+272) | — | — | — | 18,604 → 16,012 (-2,592) | -13.9% |
| ch582m_evt/hid_boot_interface | 7,812 → 8,084 (+272) | — | — | — | 18,804 → 16,212 (-2,592) | -13.8% |
| ch582m_evt/hid_composite | 7,948 → 8,228 (+280) | — | — | — | 18,928 → 16,344 (-2,584) | -13.7% |
| ch582m_evt/midi_test | 8,304 → 8,560 (+256) | — | — | — | 19,528 → 16,920 (-2,608) | -13.4% |
| ch582m_evt/cdc_dual_ports | 8,896 → 9,148 (+252) | — | — | — | 20,448 → 17,836 (-2,612) | -12.8% |
| ch582m_evt/printer_to_cdc | 9,212 → 9,468 (+256) | — | — | — | 20,696 → 18,088 (-2,608) | -12.6% |
| ch582m_evt/webusb_serial | 9,672 → 9,928 (+256) | — | — | — | 21,260 → 18,652 (-2,608) | -12.3% |
| ch582m_evt/usbtmc | 10,404 → 10,664 (+260) | — | — | — | 21,920 → 19,316 (-2,604) | -11.9% |
thank you, I am still clearing some other works, and still setting the hil rig up. Once I get both ch56x and h417 reliably in the hil pool, we can merge both SS PRs to get first ever SS support to tinyusb. |
a0deec1 to
023d235
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 023d235032
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| enum { | ||
| TUSB_EPSIZE_BULK_FS = 64, | ||
| TUSB_EPSIZE_BULK_HS = 512, | ||
| TUSB_EPSIZE_BULK_SS = 1024, |
There was a problem hiding this comment.
With SuperSpeed enabled, this adds a 1024-byte bulk MPS but leaves TUD_EPSIZE_BULK_MAX at the old HS/FS value, so any SuperSpeed class using the default CFG_TUD_*_EPSIZE/FIFO sizes allocates 512 bytes (or 64 when CFG_TUD_MAX_SPEED is explicitly OPT_MODE_SUPER_SPEED) while the new SS descriptor macros advertise 1024-byte bulk endpoints. For example, CDC/vendor RX paths queue tu_edpt_packet_size(desc_ep) bytes into endpoint buffers sized from those defaults, so a single max-size OUT packet can overrun the buffer; include TUD_OPT_SUPER_SPEED in the max-size default to preserve TinyUSB's memory-safe invariant.
AGENTS.md reference: AGENTS.md:L3-L3
Useful? React with 👍 / 👎.
| // WCH CH56x: lwIP memory lives in the 32 KB RAMX (see arch/cc.h). The SuperSpeed build | ||
| // reclaims the USB2-fallback bounce RAM (CFG_TUD_WCH_USBHS_EP_MAX) for a larger receive | ||
| // window - iperf is TCP-window-bound; the plain high-speed build keeps the smaller fit | ||
| #if defined(CFG_TUD_WCH_USBIP_USB30) |
There was a problem hiding this comment.
In SPEED=high WCH builds, tusb_mcu.h still defines CFG_TUD_WCH_USBIP_USB30 to 0 after this file includes tusb_option.h, so defined(CFG_TUD_WCH_USBIP_USB30) is true and the SuperSpeed branch is selected instead of the USBHS branch. That gives the plain high-speed build the 9-pbuf TCP window and disables inbound checksum verification despite the comment saying USBHS keeps the smaller fit; use value checks such as #if CFG_TUD_WCH_USBIP_USB30 / #elif CFG_TUD_WCH_USBIP_USBHS.
Useful? React with 👍 / 👎.
| /* WCH CH56x: only 16 KB general SRAM (RAMS); place the lwIP heap and memory pools in the | ||
| 32 KB RAMX region (the .dmadata section of the ch56x linker script) to make room. The | ||
| defines below come from the ch56x board build. */ | ||
| #if defined(CFG_TUD_WCH_USBIP_USB30) || defined(CFG_TUD_WCH_USBIP_USBHS) |
There was a problem hiding this comment.
This condition also matches other WCH targets once tusb_option.h has supplied default USBIP macros (for example CH32V307/CH32H417 USBHS/USB30 builds), but .dmadata is only defined and explicitly cleared in the CH569 linker/startup code. Building this example for those WCH boards will place lwIP heap/pool objects into an unintended orphan section instead of normal RAM; restrict the override to the CH56x target and use macro value checks rather than defined(...).
Useful? React with 👍 / 👎.
| // dcd API | ||
| //--------------------------------------------------------------------+ | ||
|
|
||
| bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { |
There was a problem hiding this comment.
These new WCH DCDs expose dcd_init() but none of the four new WCH backends (dcd_ch32h417_usb30.c, dcd_ch32h417_usbhs.c, dcd_ch56x_usb30.c, dcd_ch56x_usbhs.c) implement dcd_deinit(), so tud_deinit() falls through to the weak dcd_deinit() in usbd.c which returns false and trips TU_ASSERT(dcd_deinit(rhport)) after interrupts are disabled. Any application that deinitializes/reinitializes the stack on these boards will fail before class/queue cleanup; add a real deinit that shuts down the selected controller state and returns true.
Useful? React with 👍 / 👎.
| case TUSB_REQ_FEATURE_U1_ENABLE: | ||
| case TUSB_REQ_FEATURE_U2_ENABLE: | ||
| // Accept and ignore: U1/U2 entry is managed by the link layer/dcd | ||
| tud_control_status(rhport, p_request); |
There was a problem hiding this comment.
When a SuperSpeed host sends SET_FEATURE(U1_ENABLE) or SET_FEATURE(U2_ENABLE, this ACKs the request but leaves _usbd_dev.dev_state_bm with only the self-powered/remote-wakeup bits, and the matching CLEAR_FEATURE cases below also do nothing. A subsequent standard GET_STATUS(Device) therefore reports U1/U2 disabled even though the request succeeded, which breaks the control-state contract hosts and USB compliance tests observe; track bits 2/3 through SET/CLEAR or stall these requests if the stack is not going to maintain that state.
Useful? React with 👍 / 👎.
| void dcd_sof_enable(uint8_t rhport, bool en) { | ||
| (void)rhport; | ||
| (void)en; |
There was a problem hiding this comment.
When a CH32H417 SuperSpeed build runs with CFG_TUD_WCH_USB30_FALLBACK=1 and the link has switched to the USB2 controller, tud_sof_cb_enable() still reaches this no-op instead of ch32h417_usb2_sof_enable(). Any class or application that relies on SOF callbacks after fallback will never enable the USBHS SOF interrupt even though the USB2 backend implements it; add the same fallback dispatch used by the endpoint and status APIs.
Useful? React with 👍 / 👎.
| { // TEMP: poll link status ~6 s post-attach (MIS_ST / INT_FG / SPEED_TYPE) | ||
| for (int t = 0; t < 6; t++) { | ||
| for (volatile uint32_t i = 0; i < 25000000u; i++) {} | ||
| char s[12]; | ||
| const uint8_t ms = USBHSD->MIS_ST, fg = USBHSD->INT_FG, sp = (uint8_t)USBHSD->BUS; | ||
| const char hx[] = "0123456789abcdef"; | ||
| s[0]='s'; s[1]=hx[ms>>4]; s[2]=hx[ms&15]; s[3]=' '; | ||
| s[4]=hx[fg>>4]; s[5]=hx[fg&15]; s[6]=' '; | ||
| s[7]=hx[sp>>4]; s[8]=hx[sp&15]; s[9]='\r'; s[10]='\n'; | ||
| board_uart_write(s, 11); | ||
| } | ||
| } |
There was a problem hiding this comment.
Leftover // TEMP: debug scaffolding in ch32h417_usb2_init() — should be removed before merge. It has two concrete defects:
1. Compile error under -Werror. board_uart_write is declared in hw/bsp/board_api.h, but that header is not in this translation unit's include chain (only tusb_option.h, device/dcd.h, dcd_ch32h417.h, ch32h417_usbhs_reg.h). The call at L242 is therefore an implicit function declaration, which fails the build (-Werror + -Werror-implicit-function-declaration, set in hw/bsp/family_support.cmake and examples/build_system/make/toolchain/gcc_common.mk). The default nanoch32h417 build (SPEED=super, FALLBACK=0) doesn't compile this function body — so default CI stays green — but the SPEED=high build and the SPEED=super + CFG_TUD_WCH_USB30_FALLBACK=1 build (this PR's headline HS-fallback feature) both compile it and won't build.
2. Multi-second blocking busy-wait. 6 × 25,000,000 volatile iterations is several seconds of blocking inside ch32h417_usb2_init(), which runs from dcd_init() and from the USB3→USB2 fallback path — long enough to blow the host's USB2 enumeration timing.
Suggest deleting the entire block (L233–L244).
| // read-modify-write) so RB_UEP_R_SETUP_IS is cleared - otherwise the next OUT would be | ||
| // re-detected as a SETUP in the ISR. Applies to both the zero-length status and a | ||
| // (single-packet, EP0-sized) control-write data stage. | ||
| EP_RX_CTRL(0) = USBHS_UEP_R_TOG_DATA1 | USBHS_UEP_R_RES_ACK; |
There was a problem hiding this comment.
EP0 OUT data toggle is hard-wired to DATA1 on every transfer — multi-packet control-write data stages get a toggle mismatch. This DCD manages the data toggle in software, but nothing alternates the EP0 OUT toggle across successive data-stage packets: update_out() skips the flip for EP0 (its ep_num != 0 guard), so each dcd_edpt_xfer(0x00, …) re-arms DATA1 here.
On the USB2/fallback link, usbd chunks EP0 data stages to ep0_xact_limit() = 64 bytes, issuing one dcd_edpt_xfer(0x00, …, 64) per packet. For a control-write with wLength > 64 (DFU_DNLOAD, a large HID SET_REPORT, a WebUSB/vendor control-OUT): packet 1 arms DATA1 (host sends DATA1 → OK), then packet 2 arms DATA1 again while the host sends DATA0 → data-toggle mismatch → the SIE drops/NAKs it → the transfer hangs (or data is corrupted from byte 64 on). Enumeration itself uses only ZLP/IN control transfers, so the device enumerates fine and this bites later.
The sibling CH569 driver does this correctly — it XORs the toggle for data-stage packets and hard-wires DATA1 only for the zero-length status stage: dcd_ch56x_usbhs.c L402–L408. Suggest applying the same pattern here.
023d235 to
6429b8b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6429b8b1eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return False | ||
|
|
||
| for off in range(0, len(padded), CHUNK): | ||
| os.write(fd, b"C" + struct.pack("<H", CHUNK) + padded[off:off + CHUNK]) |
There was a problem hiding this comment.
When a signal or TTY-driver condition causes os.write() to accept only part of this 1027-byte frame, the script ignores the returned byte count and waits for an acknowledgement while the device is still waiting for the remainder. Because the flash was already erased, this can leave an unbootable partial image and abort the recovery path; send each header and chunk with a write-all loop that verifies every byte was accepted.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| case TUSB_REQ_SET_ISOCH_DELAY: | ||
| // wValue = isochronous delay in ns, no data stage: just ACK | ||
| tud_control_status(rhport, p_request); |
There was a problem hiding this comment.
When a host sends this request with the IN direction or a nonzero wLength, this path ACKs it and immediately queues what it treats as the status stage instead of stalling. The host is expecting a data transaction in the opposite direction, so EP0 becomes desynchronized and the transfer times out; validate the required host-to-device direction, zero wIndex, and zero wLength before acknowledging.
Useful? React with 👍 / 👎.
| } else { | ||
| EP_TX_CTRL(ep_num) = UEP_T_RES_NAK | RB_UEP_T_TOG_0; | ||
| ep_data_tog[ep_num][TUSB_DIR_IN] = false; |
There was a problem hiding this comment.
When CLEAR_FEATURE(ENDPOINT_HALT) is sent to a non-stalled IN endpoint with a transfer already armed, usbd_edpt_clear_stall() deliberately retains the endpoint's CLAIMED state, but this branch changes the hardware response to NAK without re-arming the still-valid DCD transfer. No replacement transfer can be claimed, so the endpoint remains NAKing indefinitely; mirror the OUT path by re-queuing the active IN transfer after resetting DATA0. The same omission exists in ch32h417_usb2_edpt_clear_stall().
Useful? React with 👍 / 👎.
| bool dcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { | ||
| (void)rhport; | ||
| (void)rh_init; |
There was a problem hiding this comment.
When an application calls tud_disconnect() to force a host re-enumeration and later calls tud_connect(), both APIs report success but the bus remains attached: a repository-wide search of the four new WCH DCD files finds no strong dcd_disconnect() or dcd_connect(), so the weak no-op implementations in usbd.c are used. Implement these operations for the selected USBSS/USBHS controller, including the active fallback controller.
Useful? React with 👍 / 👎.
| { // TEMP: poll link status ~6 s post-attach (MIS_ST / INT_FG / SPEED_TYPE) | ||
| for (int t = 0; t < 6; t++) { | ||
| for (volatile uint32_t i = 0; i < 25000000u; i++) {} | ||
| char s[12]; | ||
| const uint8_t ms = USBHSD->MIS_ST, fg = USBHSD->INT_FG, sp = (uint8_t)USBHSD->BUS; | ||
| const char hx[] = "0123456789abcdef"; | ||
| s[0]='s'; s[1]=hx[ms>>4]; s[2]=hx[ms&15]; s[3]=' '; | ||
| s[4]=hx[fg>>4]; s[5]=hx[fg&15]; s[6]=' '; | ||
| s[7]=hx[sp>>4]; s[8]=hx[sp&15]; s[9]='\r'; s[10]='\n'; | ||
| board_uart_write(s, 11); | ||
| } | ||
| } |
There was a problem hiding this comment.
Leftover TEMP debug scaffolding shipped in the driver — compile break + multi-second blocking stall
This TEMP-labeled block busy-waits ~6 s (6 × a 25M-iteration volatile loop) and then calls board_uart_write():
tinyusb/src/portable/wch/dcd_ch32h417_usbhs.c
Lines 232 to 245 in 6429b8b
Two independent problems:
-
Compile error.
board_uart_write()is declared only inhw/bsp/board_api.h, which this translation unit does not include (its includes aretusb_option.h,device/dcd.h,dcd_ch32h417.h,ch32h417_usbhs_reg.h). The call is therefore an implicit function declaration → a hard error under the tree's-Werror=implicit-function-declaration. (A portable DCD calling a BSP function is also a layering violation — it's the only such call insrc/portable/wch/.) -
Blocks enumeration / runs in ISR context. The spin sits right after the controller is enabled (
USBHSD->CONTROL = USBHS_UD_DEV_EN | …, L231) and beforedcd_init()returns, so on the standaloneSPEED=highpath the host sees the attach and then gets no response through reset/first SETUP for ~6 s (enumeration timeout). In theCFG_TUD_WCH_USB30_FALLBACK=1config,ch32h417_usb2_init()is reached fromfallback_to_usb2()inside the USB ISR (dcd_int_handler → handle_timer_irq/handle_link_irqindcd_ch32h417_usb30.c), so the entire blocking loop + blocking UART writes execute in the interrupt handler — contrary to CLAUDE.md's "defer ISR work to task context" (CLAUDE.md L20). The same file'shandle_link_irqeven notes "a blocking UART print here breaks SuperSpeed training."
ch32h417_usb2_init() compiles in the documented SPEED=high (CFG_TUD_WCH_USBIP_USBHS=1) and USB2-fallback builds — just not the default SuperSpeed-only build, which is likely why default CI didn't surface it.
Suggested fix: delete the entire TEMP block (lines 233–244).
6429b8b to
c6764ba
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6764bab02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (_fb_state == FB_USB2_ACTIVE) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
When the default CH569 SuperSpeed build has fallen back to USB2 and the host has enabled remote wakeup, tud_remote_wakeup() reports success but this branch returns without driving resume signaling, so a suspended host never wakes. Route this operation to the active USBHS controller; the CH32H417 fallback path similarly writes the disabled USBSS block instead of handling USB2.
Useful? React with 👍 / 👎.
| IWDG_SetPrescaler(IWDG_Prescaler_256); | ||
| IWDG_SetReload(1000); // ~8 s at the ~32 kHz LSI | ||
| IWDG_ReloadCounter(); | ||
| IWDG_Enable(); |
There was a problem hiding this comment.
On nanoCH32H417 this unconditionally starts an approximately eight-second watchdog whose only reload outside initialization is board_led_write(). Applications are not required to blink the board LED, and the newly supported printer_to_cdc example's main loop never calls it, so that example—and any similar user application—reboots continuously even while tud_task() is healthy; make this recovery watchdog opt-in or reload it from an actual liveness path.
Useful? React with 👍 / 👎.
| const uint8_t* p_bulk = usbd_skip_ss_ep_companion(tu_desc_next(ep_desc_int), (const uint8_t*)itf_desc + max_len); | ||
| const tusb_desc_endpoint_t* ep_desc_bulk = (const tusb_desc_endpoint_t*) p_bulk; | ||
| TU_ASSERT(usbd_open_edpt_pair(rhport, (const uint8_t*)ep_desc_bulk, 2, TUSB_XFER_BULK, &p_mtp->ep_out, &p_mtp->ep_in), 0); |
There was a problem hiding this comment.
When MTP opens this new SuperSpeed bulk endpoint, ep_sz_fs remains zero and the IN completion path therefore keeps using its hard-coded 512-byte threshold instead of the endpoint's 1024-byte MPS. If an object's final queued chunk is exactly 512 bytes, it is already a short SuperSpeed packet, but the driver treats it as full-sized and queues an extra ZLP; that ZLP can terminate the host's next bulk read prematurely. Store the negotiated bulk MPS and use it for short-packet and ZLP decisions at every speed.
Useful? React with 👍 / 👎.
| // superspeed configuration: bulk endpoints are 1024 with companion descriptors | ||
| static uint8_t const ncm_ss_configuration[] = { | ||
| // Config number (index+1), interface count, string index, total length, attribute, power in mA | ||
| TUD_CONFIG_SS_DESCRIPTOR(CONFIG_ID_NCM + 1, ITF_NUM_TOTAL, 0, NCM_SS_CONFIG_TOTAL_LEN, 0, 96), | ||
|
|
||
| // Interface number, description string index, MAC address string index, EP notification address and size, EP data address (out, in), max segment size, EP notification bInterval, NCM capabilities, bulk max burst. | ||
| TUD_CDC_NCM_SS_DESCRIPTOR(ITF_NUM_CDC, STRID_INTERFACE, STRID_MAC, EPNUM_NET_NOTIF, 64, EPNUM_NET_OUT, EPNUM_NET_IN, |
There was a problem hiding this comment.
When this new SuperSpeed NCM configuration is selected, notification_xmit() in src/class/net/ncm_device.c recognizes only TUSB_SPEED_HIGH and sends 12,000,000 bit/s for every other speed. Consequently a 5-Gbit/s link is announced to the host as full-speed, producing incorrect link metrics and potentially affecting host network policy; add an explicit SuperSpeed rate, capped to the notification's 32-bit fields.
Useful? React with 👍 / 👎.
| QUIRK_SKIPS = { | ||
| 0x20: ((13,), 'halted EP answers a single STALL at 5 Gbps (silicon limitation)'), | ||
| } |
There was a problem hiding this comment.
At SuperSpeed the CH569 DCD documents that EP0 OUT lengths congruent to 1 modulo 4 are intermittently dropped, and usbtest cases 14 and 21 exercise those lengths, but the new skip table contains only the halt quirk. The firmware likewise advertises only 0x20, despite the accompanying usbtest skill defining this control erratum as 0x10, so the supposedly quirk-aware HIL battery still runs the silicon-impossible cases and fails intermittently; advertise 0x10 for CH569 and map it here to cases 14 and 21.
Useful? React with 👍 / 👎.
| { // TEMP: poll link status ~6 s post-attach (MIS_ST / INT_FG / SPEED_TYPE) | ||
| for (int t = 0; t < 6; t++) { | ||
| for (volatile uint32_t i = 0; i < 25000000u; i++) {} | ||
| char s[12]; | ||
| const uint8_t ms = USBHSD->MIS_ST, fg = USBHSD->INT_FG, sp = (uint8_t)USBHSD->BUS; | ||
| const char hx[] = "0123456789abcdef"; | ||
| s[0]='s'; s[1]=hx[ms>>4]; s[2]=hx[ms&15]; s[3]=' '; | ||
| s[4]=hx[fg>>4]; s[5]=hx[fg&15]; s[6]=' '; | ||
| s[7]=hx[sp>>4]; s[8]=hx[sp&15]; s[9]='\r'; s[10]='\n'; | ||
| board_uart_write(s, 11); | ||
| } | ||
| } |
There was a problem hiding this comment.
Leftover // TEMP debug scaffolding blocks USB2 init — and breaks the Make build
This self-labeled // TEMP block busy-waits ~6 s (for (volatile uint32_t i = 0; i < 25000000u; i++) {} repeated 6×) printing raw register dumps, immediately after the device pull-up is enabled in ch32h417_usb2_init(). It runs unconditionally in two shipping, supported configurations:
- Standalone high-speed (
SPEED=high→CFG_TUD_WCH_USBIP_USBHS=1):dcd_init()→ch32h417_usb2_init()spins ~6 s before interrupts are enabled, so the host (which begins enumeration within ~100 ms of attach) times out / reset-loops. - USB3→USB2 runtime fallback (
CFG_TUD_WCH_USB30_FALLBACK=1):fallback_to_usb2()invokes this from the TIM12/LINK ISR, turning it into a ~6 s interrupt handler that wedges the system.
It also fails to compile under the Make toolchain: board_uart_write() (line 242) is not declared in this translation unit — none of the includes (tusb_option.h, device/dcd.h, dcd_ch32h417.h, ch32h417_usbhs_reg.h) reach hw/bsp/board_api.h, and examples/build_system/make/toolchain/gcc_common.mk sets -Werror-implicit-function-declaration.
This is bring-up scaffolding — please remove the entire block (lines 233-244). It's also a CLAUDE.md "Surgical changes" violation ("Remove only orphans your changes created").
tinyusb/src/portable/wch/dcd_ch32h417_usbhs.c
Lines 232 to 245 in c6764ba
| #define usbss_status_ep_num(st) (((st) >> 8) & 7u) | ||
| #define usbss_status_ep_is_in(st) (((st) >> 12) & 1u) |
There was a problem hiding this comment.
Macros should be UPPER_CASE
These two function-like macros are lowercase, whereas the other macros in this same file (e.g. USBSS_EP_R_EN(ep) on the lines just below) and the equivalent accessors in the sibling port header — USBSS_STATUS_EP_NUM / USBSS_STATUS_EP_IN in ch32h417_usb30_reg.h — are all UPPER_CASE.
Per CLAUDE.md: "... snake_case helpers, UPPER_CASE macros."
Note that renaming (e.g. to USBSS_STATUS_EP_NUM / USBSS_STATUS_EP_IS_IN) also requires updating the two call sites in dcd_ch56x_usb30.c (lines 598-599).
tinyusb/src/portable/wch/ch56x_usb30_reg.h
Lines 96 to 99 in c6764ba
c6764ba to
c2d984c
Compare
The re-arm fixed F1 (FB_USB3_UP is terminal, so a replug into a USB2-only host stayed dead) but introduced a worse regression, because the ladder advances on a timer regardless of whether anything is attached. Unplugged, nothing sets _fb_saw_terms, so the budget is 4 ticks of 0.55 s: at ~2.2 s training is declared exhausted and USB3 is torn down, and the 5th tick at ~2.75 s runs ch56x_usb2_init() and latches FB_USB2_ACTIVE, which is itself terminal. Any unplug longer than about three seconds therefore demoted the board to 480 Mbps permanently - including the ordinary case of replugging into the same SuperSpeed host, and this is the shipping default for hydrausb3_v1. Re-arming safely requires gating the ladder on an attached partner rather than on a timer, so F1 stays open. The rest of the state-owner work is unaffected: fallback_enter() remains the only writer of _fb_state, fallback_timer_start() still re-enables TMR0_IRQn (F10), and the deferred re-init still re-checks the state after its settle (F7).
…e real request Two defects in the reset added earlier in this branch. It was gated only on the compile-time TUD_OPT_SUPER_SPEED, while every other USB3-only handler in the same function also tests link_is_superspeed(). remote_wakeup_en is the SHARED USB 2.0 bit that SET_FEATURE(DEVICE_REMOTE_WAKEUP) sets, and USB 2.0 9.4.5 clears it only on a device reset - so on a runtime USB2 fallback link (CFG_TUD_WCH_USB30_FALLBACK, the CH569 default) a same-value SET_CONFIGURATION, which Linux's usb_reset_configuration() sends, silently revoked a wakeup authorization the host never revoked. The gate now lives in func_wakeup_clear_all() itself, covering all three call sites. The SET_INTERFACE reset also sat in the branch that runs only when the class driver does NOT handle the request. Every class implementing alternate settings - audio, video, ncm, ecm_rndis, vendor, bth, midi2, dfu - handles SET_INTERFACE itself and returns true, so the reset never fired for precisely the devices hosts send SET_INTERFACE to. It now runs before the class dispatch.
Both fixes shipped in the first remediation pass and both were wrong. F1's re-arm demotes the board permanently on any unplug because the ladder advances on a timer with nothing attached; F4's reordering performs the very write-0 on RB_UEP_R_DONE that set_rx_res() forces a 1 to avoid. Each entry records what a correct fix would need.
The sudoers grants and the wildcarded tee removal are out of this PR: the rig account is expected to have passwordless sudo, so nothing in that file gates anything there. Records what would need doing if the template is ever pointed at a host without blanket sudo.
ch32h417_usb2_deinit() reset the SIE but left INT_EN and INT_FG alone, and the USB3 dcd_init() path calls it without first disabling USBHS_IRQn. That vector aliases to the USB3 handler, which takes the USB3 branch and never writes INT_FG, so a flag latched by a prior fallback keeps the line asserted and the ISR re-enters forever - the failure mode this driver already documents elsewhere. ch56x_usb2_deinit() has always cleared INT_EN first.
fb_state selects which controller every dcd_* entry point delegates to. It is written from the LINK/TIM12 ISR and read from task context by dcd_edpt_xfer, dcd_edpt_open, dcd_edpt_stall, dcd_edpt_clear_stall, dcd_edpt_close_all, dcd_set_address and dcd_connect - and both WCH families build with -flto, so the compiler may inline those bodies into tud_task()'s loop and hoist the load, leaving the task path writing clock-gated USBSS registers after the ISR handed the port to USB2. Same exposure for fb_fail_count, pending_addr/valid (written in task context, read by the ISR) and tim12_clocked. The CH569 twin already marks its equivalents volatile.
usb30_hw_deinit()'s own GO_DISABLED write latches LINK_IF_DISABLE, and usb30_hw_init() re-enables that bit in LINK_INT_CTRL. usb30_hw_reinit_task() clears the flag register in between; usb30_bus_reset() and the TMR0 re-detect retry did not, so the next LINK interrupt could enter handle_link_irq() with a stale DISABLE and hand the port to USB2 - which would make the 4/8-tick retry budget unreachable against a host that is merely slow to train.
…region The section is NOLOAD and startup_CH56x.S zeroes it, so an object placed there WITH an initialiser silently loses it: the bytes never reach the .bin and the zero-fill runs over them, with no diagnostic from the compiler, linker or objcopy. That is reachable by ordinary code because CFG_TUSB_MEM_SECTION is defined to .dmadata for this family. Nothing currently relies on an initialiser there, so this documents the contract rather than changing behaviour, and drops the AT>FLASH load region, which is meaningless for a section that is never loaded (the ELF now reports LMA == VMA instead of a flash address).
Two halves of the same computation disagreed once TUSB_SPEED_SUPER existed. frame_div and k already resolve SUPER to the microframe form by testing "== TUSB_SPEED_FULL", but the fifo_count rate constants and the feedback interval tested "== TUSB_SPEED_HIGH", so at SuperSpeed the constants stayed 8x too large and tud_audio_feedback_interval_isr() fired 8x too rarely. Latent today - no DCD serves SuperSpeed isochronous yet - but it is shared class code.
The CH32H417 twin's terminal FB_USB3_UP and the CH569's unconditional demotion on the first LINK_IF_DISABLE are both real, but the obvious fix for each is the one that already regressed as F1, so they wait on a partner-aware ladder. Also prunes the runners-up list of the items now fixed.
usb30_bus_reset_from_isr() deinits the controller and then latches _hw_reinit_deferred only if usbd_defer_func() actually queued the work. When the usbd event queue is full the deferral is dropped - and nothing else can ever run it, because usb30_hw_deinit() has already zeroed LINK_INT_CTRL, so the dispatch test in dcd_int_handler is permanently false and no LINK interrupt can arrive to retry. Without the fallback ladder (the default) there is no timer either, and with it FB_USB3_UP has already stopped TMR0. The port stayed off the bus until the next dcd_init. The partner-gone caller queues DCD_EVENT_UNPLUGGED one line earlier, so a single free slot is enough to lose the re-init. Fall back to settling inline when the deferral fails: ~30 ms in the ISR only on a full queue, versus losing the device.
…AKEUP at SS func_wakeup_clear_all() on SET_INTERFACE wiped every function's bit plus the device-level aggregate, so on a composite SuperSpeed device a routine SET_INTERFACE(audio, alt=1) revoked the wake authorization the host had armed on an unrelated HID function: tud_remote_wakeup() silently no-ops while GET_STATUS(that interface) still reports D1=1. USB 3.2 Table 9-10 scopes the reset to the function being reconfigured, so it now clears only the addressed interface and re-derives the aggregate. SET/CLEAR_FEATURE(DEVICE_REMOTE_WAKEUP) were also the only device selectors left ungated by link_is_superspeed(). USB 3.2 9.2.5.4 does not use that selector on Enhanced SuperSpeed devices - Linux picks the recipient by speed for exactly this reason - and honouring it desynchronised remote_wakeup_en from the func_wakeup_bm it is now derived from, in both directions. They are refused on a SuperSpeed link, guarded so non-SuperSpeed builds still compile. The FUNCTION_SUSPEND handler's open-coded aggregate loop becomes func_wakeup_recompute(), the single definition of that rule.
test_usbd_set_sel, set_isoch_delay and their malformed variants never called switch_to_superspeed() and passed only because an unrelated preceding test left _usbd_dev.speed at SUPER - setUp() re-inits only when the stack is uninitialised and tearDown() is empty. Run alone they failed, and worse, the two malformed variants asserted exactly the double EP0 stall that a NON-SuperSpeed link produces, so they reported PASS without ever exercising the wIndex/wLength/ direction validation they exist for. Verified: each of the four now passes in isolation via --test-case.
hs_drop_rate.py ran its lsusb hammer with both streams discarded and no return code, so a refused or missing sudo made every iteration a no-op while main() still printed a rate - an enumeration-only figure presented as a hammered one. It now fails loudly with sudo's own message. flash_rom_read_words() spun on the SPI-ROM status bit in five unbounded loops, reached from GET_DESCRIPTOR(STRING) on every serial-string request rather than once at boot, on a BSP that arms no watchdog: a stuck bit hung the core mid-control-transfer until a power cycle. Every wait is now bounded (including the first command byte, which was previously unguarded) and board_get_unique_id() reports no serial rather than hanging.
Seven validated findings that are not patched here, each with why: the unit-test suite's SuperSpeed default (a project.yml restructure), the CH56x EP0 RX_CTRL race (wants the H417's software-toggle shape and hardware sign-off), the BTH iso walk, the lwIP checksum gate (a maintainer judgement call), the usbd_open_edpt_pair signature break (wants a changelog line), the lpcxpresso43s67 usbtest skip (wants an external baseline), and the triplicated USBHS transfer engine.
Stalling it was a regression, caught on hardware. USB 3.2 9.2.5.4 says the DEVICE_REMOTE_WAKEUP selector is "ignored and not used by Enhanced SuperSpeed devices" - ignored, not rejected - and I read that as license to STALL. On a CH32H417 at 5 Gbps a STALL there made usbtest ctrl_out (14, 21) and iso read (16, 23) fail; bisected to this change and confirmed by reverting only these two guards, with func_wakeup_clear_itf left in place and innocent. Now genuinely ignored: the status stage is acknowledged and only the state change is skipped, which still keeps remote_wakeup_en from being set while func_wakeup_bm is empty - the desync the gate existed to prevent.
Reverts 5322732 and its follow-up d33e6da. Hardware refutes them. On the CH32H417 at 5 Gbps the full usbtest battery drops from 29/30 to 25/30, failing ctrl_out (14, 21) and iso read (16, 23), and msc_dual_lun stops presenting its storage node. Bisected with full batteries: 775849bd9, f9a6e1f and this tip minus both commits all pass 29/30; the tip with them fails 25/30. Note the failures appear only in the FULL battery - all four cases pass when run with --tests in isolation on every build, including the failing one - so an isolated bisect proves nothing here and an earlier one of mine wrongly cleared the change. The review finding behind it is real: func_wakeup_clear_all() on SET_INTERFACE wipes every function's FUNCTION REMOTE WAKEUP bit, not just the addressed one. But clearing per-interface before the class dispatch regresses this device, and with the mechanism not understood the honest position is to keep the working behaviour and record the defect rather than ship a fix that costs four cases. The earlier link_is_superspeed() gate on func_wakeup_clear_all (3507142) stays: it was in f9a6e1f, which passes.
…KEUP gate" This restores 5322732 and d33e6da. My bisect was invalid and the revert was not justified. The error: I compared usbtest run through hil_test on this tip against usbtest run DIRECTLY on the baseline, and read the difference as a regression. Running the current harness against the pre-remediation baseline firmware reproduces the same 25/30 and the same four cases (14, 21, 16, 23), so the failure predates every change in this branch and is a property of how hil_test invokes the battery, not of the firmware. Direct invocation gives 29/30 on both firmwares - baseline and tip, with and without these commits - which is why the isolated --tests runs looked clean. The DEVICE_REMOTE_WAKEUP handling stays as the corrected "ignore" rather than the original stall: USB 3.2 9.2.5.4 says the selector is ignored by Enhanced SuperSpeed devices, and acknowledging without changing state is what that means.
25/30 through hil_test, 29/30 running usbtest.py directly on the same binary - and identical on the pre-remediation baseline, so it predates this branch. Records what has been ruled out, the one dmesg clue, and the method note that cost two bad bisects: never compare a harness run against a direct one.
find_firmware searches build roots in a fixed order, so a stale tree in the first root silently wins over a fresh build in a later one and the whole run reports results for firmware nobody meant to test. That is expensive to spot: the symptom is failing test cases rather than an error, and it survives reflashes and reboots. It cost a long debugging session here. hil_test defaults --build-dir to cmake-build (the tools/build.py layout), and a month-old cmake-build/ tree shadowed every run; its pre-SuperSpeed-iso-size CH32H417 binary failed usbtest cases 14, 21, 16 and 23 plus msc_dual_lun. The hil skill already documents passing -B examples for the CLAUDE.md build layout - the flag was simply omitted - so this adds no documentation, only a check that makes the mistake audible instead of silent. With -B examples both WCH boards are clean: nano 29/30 (1 skipped), hydra 27/30 (3 skipped), 0 failed. The check fires once per example, naming both paths and the age gap.
Replaces the open entry with the resolution: hil_test defaults --build-dir to cmake-build, the runs omitted -B examples, and a 17 July tree shadowed every lookup. The CH32H417 binary in it predates the SuperSpeed iso max-packet fix, which is the whole 512-errors-of-1024 signature. Both boards are clean with the flag, and it records that earlier HIL results in this session came from that stale tree.
_wch_uart_park() calls glob.glob() to find the WCH-LinkE VCP, but glob was never imported, so reset_wch_uart_loader() raised NameError instead of parking the board. It went unnoticed because call_flasher() catches Exception and turns it into a plain "reset failed", so the board simply never got parked and the run carried on. Reproduced before the fix (NameError: name 'glob' is not defined) and after it (_wch_uart_park returns True against the real probe).
dcd_init() cleared a hand-picked subset - xfer_status, _pool_brk, _pending_addr, _ep0_seq, _lmp_pending - and left _ep0_rx_seq, _ep0_status_pending, _ep0_status_dir, _ep0_early_len, _ep0_data_in and _link_suspended holding the previous session's values. After a dcd_deinit()/dcd_init() cycle the first control transfer of the new session could therefore complete a phantom status stage or replay stale early-OUT data. It now calls ep_state_reset(), which already exists for exactly this, plus the two fields outside it.
edpt_stall()/edpt_clear_stall() lacked the ep_num < EP_MAX guard that edpt_open()/edpt_close() carry on the CH32H417 (and that neither carried on the CH56x), so the two entry points reached by a host CLEAR/SET_FEATURE(HALT) were the unguarded ones. Hardening rather than a fix: usbd asserts ep_num < CFG_TUD_ENDPPOINT_MAX, which is 8 for both parts and equals EP_MAX, so an out-of-range id cannot reach the driver today - the comment says so rather than implying a live hole. hs_drop_rate.py's teardown could raise TimeoutExpired out of sniffer.wait() inside the finally, leaving the capture running and still writing - the exact leak that block exists to prevent. It now escalates to kill().
The header still said 'compile-verified; full hardware bring-up (usbtest at 5 Gbps) is pending' while the same file carried ten hardware-derived findings, and the board now runs 12/12 HIL examples and usbtest 29/30 at 5 Gbps. A reader deciding whether to trust this driver was being told the opposite of the truth.
The LINK_IF_DISABLE branch handed the port to the USB2 controller unconditionally, with no _fb_state or tick-budget test, and FB_USB2_ACTIVE has no exit short of dcd_init. So unplugging a board that had already trained SuperSpeed demoted it permanently: replugged into the SAME SuperSpeed port it came back at 480 Mbps. Reproduced on the hydra with the board powered from its WCH-Link so the cable move could not reset it - 5-3 at 480 after the replug, and 6-3 at 5000 again only after a reflash, which is dcd_init doing the recovery. With this change the same move returns 5000 directly. Reaching Disabled from FB_USB3_UP is a cable being pulled, not a partner refusing SuperSpeed, so it now reports the disconnect and rebuilds the link through the deferred path the TERM_PRESENT-lost branch already uses. The demotion stays for FB_USB3_TRAINING, where it belongs; the CH32H417 twin has always gated it that way (fb_state == FB_USB3_TRAINING && ++fb_fail_count >= FB_FAIL_LIMIT), so this also closes a disagreement between the two families. Does NOT fix F1: a replug into a genuinely USB2-only host still leaves the board dead, because the ladder cannot distinguish that from an empty port without a VBUS signal. Deliberately does not re-arm training on disconnect either - that was tried and reverted, since the ladder then advances on its timer while unplugged and lands in FB_USB2_ACTIVE within ~2.75 s, demoting the board on any unplug. HIL after the change: hydra 12/12 usbtest 27/30 (3 skipped), nano 12/12 usbtest 29/30 (1 skipped), 0 failed.
Splits the old 'two findings left alone' entry: the unconditional demotion on LINK_IF_DISABLE is fixed and hardware-verified, while F1 (replug into a USB2-only host) and the CH32H417 twin stay open on the same blocker - the ladder needs an attach signal rather than a timer.
8078ca4 to
1d15cc9
Compare
Hydra powered from the WCH-Link, so the cable move cannot reset it: SuperSpeed -> USB2-only port enumerates nothing at all (0 enumerations on any 480 root hub over ~30 s), and moving back to SuperSpeed returns 5000 with CDC and MSC bound. Confirms FB_USB3_UP is terminal for the USB2-only case while showing a failed excursion leaves the state recoverable.
First SuperSpeed device support in TinyUSB. The stack work is layered so a new SuperSpeed MCU only needs one DCD file + a BSP — to
usbd, SuperSpeed is just another speed.Stack
TUD_OPT_SUPER_SPEED/OPT_MODE_SUPER_SPEEDcapability, EP0 fixed at 512,ep0_xact_limit()chunkingSET_SEL/SET_ISOCH_DELAY/ U1-U2TUD_*_SS_*descriptor templates; class drivers skip and account for companions viausbd_skip_ss_ep_companion()tud_speed_get()— 0-byte code delta on non-SuperSpeed buildsPorts
hydrausb3_v1) and CH32H417 (nanoch32h417), each with a USB3 DCD, a USB2 high-speed DCD, and runtime HS fallback on a shared root porttest/hilflashing support (OpenOCD/wlink + a UART firmware loader) and rig configTesting
usbtestbatterytest_usbd.cSuperSpeed cases (endpoint validate, companion skipping,SET_SEL/SET_ISOCH_DELAY)adding-superspeed-portskill documenting the port contract + hardware-validated gotchasNotes