Skip to content

usbtest: device-side peer for the Linux kernel usbtest battery + DCD fixes across 13 ports#3758

Open
hathach wants to merge 30 commits into
masterfrom
usbtest
Open

usbtest: device-side peer for the Linux kernel usbtest battery + DCD fixes across 13 ports#3758
hathach wants to merge 30 commits into
masterfrom
usbtest

Conversation

@hathach

@hathach hathach commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

Adds examples/device/usbtest — a device-side peer for the Linux kernel's usbtest.ko/testusb battery (gadget-zero source/sink protocol) — plus a host runner (test/hil/usbtest.py) and HIL integration. Driving the full 30-case battery (bulk / control / interrupt / isochronous, incl. unlink, halt and data-toggle cases) across the rig surfaced and fixed real bugs in 13 device ports, the vendor class and the usbd core. Every device-capable board on the HIL rig passes 30/30.

Device example + vendor class

  • examples/device/usbtest: vendor interface with bulk/int/iso source-sink pairs, iso in altsetting 1, tiered capability via bcdDevice/USBTEST_TIER, control 0x5b/0x5c write/read-back.
  • class/vendor: optional interrupt endpoints, isochronous + alternate-setting support, manual RX arming for halt recovery; de-selected iso endpoints are deactivated on SET_INTERFACE (stale transfers aborted).
  • usbd: harden endpoint/interface standard requests (GET_STATUS(interface), feature ACKs, GET_STATUS direction check); release endpoint claim on clear-stall.
  • Unique PID per example, enforced by a pre-commit hook (guarantees back-to-back re-enumeration on the rig).

DCD fixes (each from a failing battery case, root-caused on hardware)

Port Fix
dwc2 EP0 OUT dcache invalidate used the advanced buffer pointer under buffer DMA — corrupted a neighboring dirty line on every control-OUT data stage (found on ESP32-P4). EP0 chunk bookkeeping reworked per databook 7.1.83
ci_hs stale ACTIVE bit in the qhd overlay replayed a completed transfer on re-prime (lpc43s67, mimxrt1015)
rusb2 bounded the FIFO-ready spin that froze the stack when a double-buffered IN pipe filled; control-OUT DCP flow control; clear-halt re-arm via queued flag
musb flush TX FIFO on halt; don't load a disarmed endpoint in the IN ISR (device→host babble)
nrf5x Errata 199 workaround (USBD drops DMA tokens)
samd implement dcd_edpt_iso_alloc/activate (were false-returning stubs)
lpc_ip3511 clear Active before Stall so a queued endpoint actually halts (UM 41.8.5.2)
stm32_fsdev don't disarm an armed endpoint on clear-halt toggle reset; optional double-buffered iso
rp2040 re-issue the in-flight transfer on clear-halt toggle reset; single-buffered iso
ch32_usbfs accept iso on CH58x (≤64 B per datasheet); iso is DATA0-only in both directions; per-direction iso state; 1023 B EP3 iso kept on V20x/V30x
ch32_usbhs clear-stall re-queues an armed OUT receive instead of NAKing forever
ch32v20x/v30x BSP naked-asm USBD IRQ handlers (gcc interrupt attr corrupts nested QingKe HWSTK returns); real ESIG unique id

Host runner + HIL

  • test/hil/usbtest.py: tiered battery with per-case verdicts, dmesg capture, kernel-hang auto-recovery, --json; refuses known-incompatible host controllers (uPD720201 needs firmware ≥ 2.02.6.9).
  • test/hil/hil_test.py: per-host-controller flash/usbtest budgeting (8/4) with round-robin dispatch from a learned controller cache; failed runs emit an exact re-run spec (--accumulate -b … -bt …) consumed by CI retries; per-variant duration column in the report.
  • CI: esp-idf builds and espressif boards split into their own hil-build-esp/hil-tinyusb-esp jobs (selected by --flasher esptool, no board names in workflows) so the main HIL run no longer waits on the slowest toolchain.

Validation

  • 30/30 across all rig boards, incl. multi-cycle reflash reliability runs on newly-enabled ports; dwc2/ci_hs fixes cross-checked against databook/reference manuals and validated on hardware.
  • Multi-round adversarial code review (multi-agent + Copilot) with every finding verified on hardware or refuted with evidence; unit tests, codespell, formatting clean.

🤖 Generated with Claude Code

Copilot AI 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.

Pull request overview

This PR adds a new device-side “usbtest” example (to act as a peer for the Linux kernel usbtest/testusb battery), integrates a host-side runner into HIL, and includes a broad set of DCD/core fixes uncovered by running the battery across many device ports.

Changes:

  • Add examples/device/usbtest plus vendor-class extensions (altsettings + optional bulk/int/iso endpoints) to support the Linux usbtest source/sink protocol.
  • Add test/hil/usbtest.py runner and wire it into the HIL harness/reporting and rig configuration.
  • Fix multiple device-controller drivers and USBD core behaviors exposed by the usbtest battery (stall/clear-stall, toggle reset, iso support, FIFO readiness, etc.).

Reviewed changes

Copilot reviewed 35 out of 37 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/hil/usbtest.py New host-side runner for binding usbtest + executing/parsing the testusb battery.
test/hil/tinyusb.json Register CH32V307 variants and adjust rig board config/skip list.
test/hil/tinyusb-sudoer Add sudoers allowlist template for usbtest binding/recovery workflows.
test/hil/hil_test.py Integrate device/usbtest into HIL, add metric-aware failures and reporting tally fixes.
src/portable/wch/dcd_ch32_usbhs.c Fix clear-stall behavior to re-queue armed OUT transfers (toggle reset case).
src/portable/wch/dcd_ch32_usbfs.c Add/enable iso support details, per-direction iso state, EP3 sizing, and toggle handling fixes.
src/portable/wch/ch32_usbfs_reg.h Add EP3 1023B capability macro for relevant WCH MCUs.
src/portable/st/stm32_fsdev/dcd_stm32_fsdev.c Refine clear-stall so it doesn’t disarm VALID endpoints unintentionally.
src/portable/renesas/rusb2/dcd_rusb2.c Bound FIFO-ready spins, add ZLP deferral, improve control OUT flow control, and fix clear-stall re-arm logic.
src/portable/raspberrypi/rp2040/rp2040_usb.c Prevent iso endpoints from being double-buffered into neighboring DPRAM.
src/portable/raspberrypi/rp2040/dcd_rp2040.c Re-issue active transfers on clear-stall toggle reset so claims complete properly.
src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c Fix stall semantics and implement iso alloc/activate handling.
src/portable/nordic/nrf5x/dcd_nrf5x.c Add errata 199 workaround latch + implement iso alloc/activate path.
src/portable/microchip/samd/dcd_samd.c Implement iso alloc/activate to reserve bank sizes and enable ISO endpoints.
src/portable/mentor/musb/dcd_musb.c Prevent IN ISR from loading disarmed transfers; flush FIFO before stalling.
src/device/usbd.c Harden GET_STATUS handling and endpoint feature validation; adjust vendor control callback wiring; refine clear-stall claim release.
src/common/tusb_mcu.h Update endpoint-close vs iso-alloc capability documentation/deprecation notes.
src/class/vendor/vendor_device.h Add config gates and APIs for optional interrupt/iso endpoints + altsetting support.
src/class/vendor/vendor_device.c Implement vendor altsettings logic and add non-buffered helpers for bulk/int/iso endpoints.
src/class/audio/audio_device.c Remove prior “clear busy” workaround comment in favor of standardized endpoint state clearing.
lib/rt-thread/port/msc_device_port.c Bound INQUIRY string copies to avoid overflow.
hw/bsp/ch32v30x/family.c Implement board_get_unique_id() for CH32V30x using ESIG UID.
hw/bsp/ch32v20x/family.c Replace interrupt handlers with nested-safe naked ISRs for QingKe HWSTK scenario.
examples/device/video_capture/src/main.c Fix VIDEO_STACK_SIZE typo.
examples/device/video_capture_2ch/src/main.c Fix VIDEO_STACK_SIZE typo.
examples/device/usbtest/src/usb_descriptors.h New usbtest descriptor/tier constants and per-MCU endpoint sizing rules.
examples/device/usbtest/src/usb_descriptors.c New FS/HS descriptors (alt0 empty, alt1 endpoints) and string descriptors for usbtest.
examples/device/usbtest/src/tusb_config.h New example TinyUSB config enabling vendor bulk/int/iso + altsettings and manual RX arming.
examples/device/usbtest/src/main.c New device firmware implementing source/sink pumps + ctrl_out protocol.
examples/device/usbtest/src/CMakeLists.txt ESP-IDF component registration for usbtest example.
examples/device/usbtest/skip.txt Skip list for unsupported/unsuitable MCUs.
examples/device/usbtest/README.md Documentation for tiers, protocol, and running via test/hil/usbtest.py.
examples/device/usbtest/Makefile Make build integration for the new example.
examples/device/usbtest/CMakePresets.json Presets include for board presets (CMake).
examples/device/usbtest/CMakeLists.txt CMake build integration for the new example.
examples/device/CMakeLists.txt Add usbtest to the device example list.
.claude/skills/usb-recover/SKILL.md New rig USB recovery documentation.
.claude/skills/usb-recover/scripts/usb_recover.sh New restricted recovery helper script (sudoers-friendly).
.claude/skills/usb-debug/SKILL.md New rig USB dynamic-debug documentation.
.claude/skills/usb-debug/scripts/usb_dyndbg.sh New restricted dynamic-debug toggle helper script (sudoers-friendly).
.claude/skills/hil/SKILL.md Add guidance to stop/start the Actions runner on ci during manual hardware use.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread hw/bsp/ch32v30x/family.c
Comment thread test/hil/tinyusb-sudoer Outdated
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Size Difference Report

Because 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

file .text .rodata .data .bss size % diff
dcd_ch32_usbfs.c 1659 ➙ 1698 (+39) 0 0 2444 ➙ 1364 (-1080) 4103 ➙ 3062 (-1041) -25.4%
dcd_ch32_usbhs.c 1892 ➙ 1922 (+30) 0 0 481 2373 ➙ 2402 (+29) +1.2%
dcd_lpc_ip3511.c 1463 ➙ 1538 (+75) 0 0 264 1683 ➙ 1758 (+75) +4.5%
dcd_musb.c 2595 ➙ 2664 (+69) 0 0 179 2773 ➙ 2842 (+69) +2.5%
dcd_nrf5x.c 2939 ➙ 2974 (+35) 0 0 292 3231 ➙ 3266 (+35) +1.1%
dcd_rp2040.c 840 ➙ 1004 (+164) 0 764 653 2257 ➙ 2420 (+163) +7.2%
dcd_rusb2.c 2918 ➙ 3346 (+428) 0 0 156 3074 ➙ 3502 (+428) +13.9%
dcd_samd.c 1036 ➙ 1071 (+35) 0 0 266 1302 ➙ 1337 (+35) +2.7%
dcd_stm32_fsdev.c 2543 ➙ 2586 (+43) 0 0 291 2834 ➙ 2877 (+43) +1.5%
rp2040_usb.c 386 35 619 ➙ 632 (+13) 11 1051 ➙ 1065 (+14) +1.3%
tusb.c 457 ➙ 450 (-7) 0 389 3 459 ➙ 452 (-7) -1.5%
usbd.c 3536 ➙ 3618 (+82) 57 90 ➙ 91 (+1) 355 ➙ 354 (-1) 3955 ➙ 4035 (+80) +2.0%
vendor_device.c 639 ➙ 1119 (+480) 0 534 ➙ 538 (+4) 559 ➙ 1549 (+990) 1197 ➙ 2660 (+1463) +122.2%
TOTAL 22903 ➙ 24376 (+1473) 92 2396 ➙ 2414 (+18) 5954 ➙ 5863 (-91) 30292 ➙ 31678 (+1386) +4.6%
Changes <1% in size
file .text .rodata .data .bss size % diff
audio_device.c 2890 ➙ 2888 (-2) 0 1259 ➙ 1252 (-7) 1623 4508 ➙ 4506 (-2) -0.0%
dcd_ci_fs.c 1924 ➙ 1926 (+2) 0 0 1290 3214 ➙ 3216 (+2) +0.1%
dcd_ci_hs.c 1756 ➙ 1771 (+15) 0 0 1344 2534 ➙ 2549 (+15) +0.6%
dcd_dwc2.c 4223 ➙ 4229 (+6) 19 0 265 4506 ➙ 4513 (+7) +0.2%
dcd_khci.c 1952 ➙ 1954 (+2) 0 0 1290 3242 ➙ 3244 (+2) +0.1%
dcd_mm32f327x_otg.c 1477 ➙ 1478 (+1) 0 0 1290 2767 ➙ 2768 (+1) +0.0%
dcd_nuc121.c 1170 0 0 101 1270 ➙ 1271 (+1) +0.1%
hcd_rusb2.c 2923 ➙ 2951 (+28) 0 0 245 3168 ➙ 3196 (+28) +0.9%
tusb_fifo.c 855 ➙ 852 (-3) 0 486 0 850 ➙ 847 (-3) -0.4%
video_device.c 4433 ➙ 4442 (+9) 5 1235 480 4905 ➙ 4914 (+9) +0.2%
TOTAL 23603 ➙ 23661 (+58) 24 2980 ➙ 2973 (-7) 7928 30964 ➙ 31024 (+60) +0.2%
No changes
file .text .rodata .data .bss size % diff
cdc_device.c 1236 16 1092 728 1963 +0.0%
cdc_host.c 6399 487 15 971 7594 +0.0%
dcd_da146xx.c 3067 0 0 144 3211 +0.0%
dcd_eptri.c 2273 0 0 259 2532 +0.0%
dcd_ft9xx.c 3284 0 0 172 3456 +0.0%
dcd_lpc17_40.c 1481 0 0 648 1805 +0.0%
dcd_msp430x5xx.c 1801 0 0 176 1977 +0.0%
dcd_nuc120.c 1096 0 0 78 1174 +0.0%
dcd_nuc505.c 0 0 1533 157 1690 +0.0%
dcd_samg.c 1322 0 0 72 1394 +0.0%
dfu_device.c 776 28 712 136 912 +0.0%
dfu_rt_device.c 157 0 134 0 157 +0.0%
dwc2_common.c 603 22 0 0 615 +0.0%
ecm_rndis_device.c 1067 0 1 2759 3826 +0.0%
ehci.c 2763 0 0 6274 7783 +0.0%
fsdev_common.c 180 0 0 0 180 +0.0%
hcd_ch32_usbfs.c 2491 0 0 502 2993 +0.0%
hcd_ci_hs.c 181 0 0 0 181 +0.0%
hcd_dwc2.c 5071 25 1 545 5642 +0.0%
hcd_khci.c 2443 0 0 454 2897 +0.0%
hcd_musb.c 3071 0 0 157 3228 +0.0%
hcd_pio_usb.c 262 0 240 0 502 +0.0%
hcd_rp2040.c 1996 17 4 321 2338 +0.0%
hcd_samd.c 2220 0 0 324 2544 +0.0%
hcd_stm32_fsdev.c 3248 0 1 420 3670 +0.0%
hid_device.c 1124 44 997 119 1242 +0.0%
hid_host.c 1246 0 0 1270 2516 +0.0%
hub.c 1384 8 8 30 1419 +0.0%
midi2_device.c 3006 34 1342 561 3579 +0.0%
midi2_host.c 1802 0 0 5921 7723 +0.0%
midi_device.c 1149 0 1007 619 1765 +0.0%
midi_host.c 1339 7 7 3538 4880 +0.0%
msc_device.c 2527 108 2293 804 3331 +0.0%
msc_host.c 1633 0 0 395 2028 +0.0%
mtp_device.c 1713 22 743 589 2309 +0.0%
ncm_device.c 1766 28 821 4393 6172 +0.0%
ohci.c 1925 0 0 2503 4428 +0.0%
printer_device.c 828 0 706 560 1387 +0.0%
rusb2_common.c 160 0 16 0 176 +0.0%
typec_stm32.c 1230 8 2 19 1255 +0.0%
usbc.c 500 2 20 166 688 +0.0%
usbh.c 4979 57 82 1165 6250 +0.0%
usbtmc_device.c 2261 24 69 313 2606 +0.0%
TOTAL 79060 937 11846 38262 118018 +0.0%

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 099b6538dc

ℹ️ 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".

Comment thread src/portable/raspberrypi/rp2040/dcd_rp2040.c
Comment thread test/hil/usbtest.py
Comment thread src/portable/nordic/nrf5x/dcd_nrf5x.c
Comment thread examples/device/usbtest/skip.txt
Comment thread src/class/vendor/vendor_device.c
@claude

claude Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

1 similar comment
@claude

claude Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

Top 10 targets by memory change (%) (out of 2467 targets) View Project Dashboard →

target .text .rodata .data .bss total % diff
raspberrypi_zero/webusb_serial 48,920 → 49,384 (+464) 1,087 → 4,719 (+3,632) 50,007 → 54,103 (+4,096) +8.2%
raspberrypi_zero/audio_4_channel_mic 57,216 → 57,592 (+376) 1,591 → 5,311 (+3,720) 58,807 → 62,903 (+4,096) +7.0%
ch32v103c_bluepill/dfu_runtime 8,556 → 8,668 (+112) 2,880 → 1,800 (-1,080) 13,900 → 12,932 (-968) -7.0%
raspberrypi_zero/dynamic_configuration 61,420 → 61,816 (+396) 939 → 4,639 (+3,700) 62,359 → 66,455 (+4,096) +6.6%
raspberrypi_cm4/cdc_uac2 61,248 → 61,568 (+320) 1,320 → 5,096 (+3,776) 62,568 → 66,664 (+4,096) +6.5%
ch32v103c_bluepill/hid_generic_inout 9,528 → 9,648 (+120) 3,084 → 2,004 (-1,080) 15,080 → 14,120 (-960) -6.4%
ch32v103c_bluepill/hid_multiple_interface 10,368 → 10,484 (+116) 2,960 → 1,880 (-1,080) 15,792 → 14,828 (-964) -6.1%
ch32v103c_bluepill/hid_boot_interface 10,388 → 10,504 (+116) 2,960 → 1,880 (-1,080) 15,812 → 14,848 (-964) -6.1%
ch32v103c_bluepill/hid_composite 10,612 → 10,732 (+120) 2,948 → 1,868 (-1,080) 16,028 → 15,068 (-960) -6.0%
portenta_c33/uac2_headset 15,420 → 16,340 (+920) 15,468 → 16,388 (+920) +5.9%

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Hardware-in-the-loop (HIL) Test Report

hfp-iar

✅ 42 passed · ❌ 14 failed · ⚪ 0 skipped · blank not run

Board usbtest cdc_msc_throughput audio_test_freertos cdc_dual_ports cdc_msc cdc_msc_freertos dfu dfu_runtime hid_boot_interface hid_generic_inout midi_test msc_dual_lun mtp printer_to_cdc duration
stm32l412nucleo ✅ 30/30 ✅ C 668/384k M 830/769k 79s
lpcxpresso43s67 29s
stm32f746disco ✅ 30/30 ✅ C 12.8/10.4M M 15.2/30.8M 63s
stm32f746disco-DMA ✅ 30/30 ✅ C 12.6/10.5M M 15.5/30.7M 66s

hfp.json

✅ 39 passed · ❌ 14 failed · ⚪ 0 skipped · blank not run

Board usbtest cdc_msc_throughput audio_test_freertos cdc_dual_ports cdc_msc cdc_msc_freertos dfu dfu_runtime hid_boot_interface hid_generic_inout midi_test msc_dual_lun mtp printer_to_cdc duration
stm32l412nucleo ✅ CDC 634k/387k MSC 844k/813k
stm32f746disco ✅ CDC 13.7M/10M MSC 15.7M/30.7M
stm32f746disco-DMA ✅ CDC 13.7M/9.2M MSC 13.7M/30.1M
lpcxpresso43s67

tinyusb-esp.json

✅ 21 passed · ❌ 3 failed · ⚪ 0 skipped · blank not run

Board usbtest msc_file_explorer_freertos audio_test_freertos cdc_msc_freertos device_info hid_composite_freertos duration
espressif_p4_function_ev ❌ 29/30 409 KB/s 121s
espressif_p4_function_ev-DMA ✅ 30/30 409 KB/s 115s
espressif_s3_devkitm ✅ 30/30 154s
espressif_s3_devkitm-DMA ✅ 30/30 115s

tinyusb.json

✅ 314 passed · ❌ 25 failed · ⚪ 11 skipped · blank not run

Board cdc_msc_throughput msc_file_explorer msc_file_explorer_freertos audio_test_freertos cdc_dual_ports cdc_msc cdc_msc_freertos cdc_msc_hid device_info dfu dfu_runtime hid_boot_interface hid_composite_freertos hid_generic_inout host_info_to_device_cdc midi_test msc_dual_lun mtp printer_to_cdc duration
ek_tm4c123gxl ✅ CDC 507k/511k MSC 513k/511k
espressif_p4_function_ev rd 409KB/s
espressif_p4_function_ev-DMA rd 409KB/s
espressif_s3_devkitm rd 409KB/s
espressif_s3_devkitm-DMA rd 409KB/s
feather_nrf52840_express ✅ CDC 507k/510k MSC 511k/510k
max32666fthr ✅ CDC 7M/14.3M MSC 14.5M/21.4M
metro_m4_express ✅ CDC 508k/512k MSC 515k/511k
mimxrt1015_evk
mimxrt1064_evk ✅ CDC 24.4M/18M MSC 37.6M/35.2M rd 1368KB/s rd 1365KB/s
lpcxpresso11u37 ✅ CDC 451k/325k MSC 659k/612k
ra4m1_ek ✅ CDC 663k/412k MSC 681k/618k
raspberry_pi_pico ✅ CDC 509k/512k MSC 556k/527k rd 62KB/s
raspberry_pi_pico_w rd 1106KB/s rd 1022KB/s
raspberry_pi_pico2 rd 1108KB/s rd 1022KB/s
adafruit_fruit_jam ✅ CDC 508k/512k MSC 562k/541k rd 62KB/s rd 62KB/s
stm32f072disco ✅ CDC 507k/256k MSC 510k/485k
stm32f407disco ✅ CDC 625k/513k MSC 535k/511k
stm32f723disco ✅ CDC 507k/545k MSC 517k/511k rd 15420KB/s rd 4032KB/s
stm32f723disco-DMA ✅ CDC 507k/511k MSC 511k/511k rd 16912KB/s rd 4002KB/s
stm32h743nucleo ✅ CDC 526k/515k MSC 612k/545k
stm32h743nucleo-DMA ✅ CDC 643k/643k MSC 518k/523k
stm32g0b1nucleo ✅ CDC 553k/485k MSC 567k/621k
stm32l476disco ✅ CDC 566k/553k MSC 512k/586k
stm32u083nucleo ✅ CDC 518k/503k MSC 541k/600k
nanoch32v203-fsdev ✅ CDC 508k/514k MSC 511k/511k
nanoch32v203-usbfs
ch32v103r_r1_1v0 ✅ CDC 499k/397k MSC 707k/542k
ch582m_evt ✅ CDC 230k/207k MSC 465k/437k

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

#if CFG_TUD_VENDOR_TXRX_BUFFERED
return (p_itf->rx_stream.ep_addr != 0) || (p_itf->tx_stream.ep_addr != 0);
#else
return (p_itf->ep_out != 0) || (p_itf->ep_in != 0);

P2 Badge Include new vendor endpoints in mounted checks

With the new interrupt/iso endpoint support, a valid vendor interface no longer necessarily has bulk endpoints. For an interrupt-only descriptor, or an altsetting that exposes only interrupt/iso endpoints, vendord_open() records the new endpoint fields but tud_vendor_n_mounted() still checks only the bulk streams/endpoints, so typical application code guarded by tud_vendor_mounted() will never arm or write those endpoints even though the interface is configured.

ℹ️ 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".

Comment thread src/class/vendor/vendor_device.c

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

return (p_itf->ep_out != 0) || (p_itf->ep_in != 0);

P2 Badge Include optional vendor endpoints in mounted checks

When CFG_TUD_VENDOR_TXRX_BUFFERED=0 and a descriptor uses the new interrupt-only vendor support (the open path explicitly allows bulk to be absent), vendord_open() records ep_int_in/out but leaves ep_in/out at zero, so tud_vendor_mounted() reports false even though the interface is configured and its interrupt endpoints are open. Applications that gate tud_vendor_int_read_xfer() or tud_vendor_int_write() on the mounted predicate will never arm/service these endpoints; include the optional interrupt/iso endpoint fields in this predicate.

ℹ️ 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".

Comment thread test/hil/usbtest.py Outdated
Comment thread src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c Outdated
Comment thread src/common/tusb_mcu.h
Comment thread test/hil/usbtest.py

Copilot AI 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.

Pull request overview

Copilot reviewed 37 out of 39 changed files in this pull request and generated 3 comments.

Comment thread hw/bsp/ch32v30x/family.c
Comment thread examples/device/usbtest/src/usb_descriptors.h
Comment thread src/portable/renesas/rusb2/dcd_rusb2.c
@hathach
hathach requested a review from Copilot July 6, 2026 17:35
@hathach

hathach commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Copilot AI 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.

Pull request overview

Copilot reviewed 38 out of 40 changed files in this pull request and generated 1 comment.

Comment thread test/hil/hil_test.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 24a92e4831

ℹ️ 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".

Comment thread src/portable/nordic/nrf5x/dcd_nrf5x.c
Comment thread src/class/vendor/vendor_device.c
Comment thread examples/device/usbtest/src/usb_descriptors.h
Comment thread src/class/vendor/vendor_device.c Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

# Conflicts:
#	.claude/skills/hil/SKILL.md
#	test/hil/hil_test.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a686c70631

ℹ️ 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".

static inline void vendord_abort_ep(uint8_t rhport, uint8_t ep_addr) {
if (ep_addr) {
usbd_edpt_stall(rhport, ep_addr);
usbd_edpt_clear_stall(rhport, ep_addr);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use a real abort for vendor alt switches

Fresh evidence beyond the earlier altsetting-abort thread is that several updated DCD clear-stall paths now intentionally preserve an armed OUT transfer for toggle-reset recovery (for example dcd_rusb2.c:989 checks _dcd.pipe[num].queued, and WCH checks xfer->valid). This helper's stall+clear sequence therefore is not an abort on those ports: switching a vendor interface from an endpoint altsetting to alt 0 while a read is armed can leave the hardware endpoint ACKing after the class zeros ep_out, so a zero-bandwidth altsetting can still consume packets and any late completion no longer matches the selected endpoints. Use a real DCD abort/close path or clear the DCD transfer state before the clear-stall step.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Acknowledged -- you're right that vendord_abort_ep's stall+clear-stall sequence isn't a true abort on ports that intentionally keep an armed OUT transfer queued through clear-stall for toggle-reset recovery (e.g. dcd_rusb2.c:989's _dcd.pipe[num].queued check, WCH's xfer->valid). This is a known limitation and is being deferred to a separate follow-up PR (a real DCD-level abort/close path) rather than fixed here.

- dcd_edpt_stall flushes the primed buffer (ENDPTFLUSH), but the aborted
  transfer's dQH overlay can be left ACTIVE with mid-transfer state; the
  next prime after clear-halt then resumes the stale overlay instead of
  loading the fresh qtd, so post-halt IN reads return mid-buffer data
  (usbtest case 13 'buf[32] = 56 (not 0)', with case 18 failing
  downstream of the same corruption in the full battery). qhd_start_xfer
  now clears overlay.active alongside overlay.halted before linking the
  new qtd.
- test/hil(hfp): drop lpcxpresso43s67's device/usbtest skip - the
  historical first-case wedge no longer reproduces on this branch, and
  with the overlay fix the board runs 30/30 on its Fresco xHCI host
  (previously 28/30 with deterministic case 13/18 failures).
  mimxrt1064_evk (imxrt dcache path) 30/30 regression-clean.
- docs(hil skill): document the external hifiphile rig - pool
  test/hil/hfp.json, SSH-reachable from htpc/ci with no outbound SSH,
  exercised by the CI hil-tinyusb (hfp.json) job; never run HIL against
  it during development unless the user explicitly asks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017TQZrFfU3K4Y198aLsUpBC
…3/p4 and mimxrt1015

edpt_schedule_packets() advanced xfer->buffer past each armed EP0
chunk, so the OUT-complete handler invalidated the cache at the
ADVANCED pointer: one line past the received data. The CPU then read
stale cached bytes instead of the DMA'd packet, and the misplaced
invalidate discarded a dirty line of whatever variable follows the
buffer - random neighbor corruption on every control-OUT data stage.
Found by usbtest ctrl_out (cases 14/21) on espressif_p4_function_ev
with DMA enabled, the first DWC2 target combining buffer DMA with a
data cache: usbd control state wedged after the first control write
(every later request stalled), and one build layout panicked in the
usbd memcpy with a wild pointer.

Rework the EP0 chunk bookkeeping so xfer->buffer always points at the
un-consumed position: the arm no longer advances it; instead the EP0
re-arm paths advance past each completed (full) chunk, invalidating it
first on the OUT side. The final OUT completion invalidates exactly
the received bytes of its last chunk, taken from DOEPDMA ("incremented
on every AHB transaction", databook 7.1.83 - the same semantics the
SETUP path relies on) before dma_setup_prepare() re-targets it. EP0
chunking state (ep0_pending) is now also dropped on bus reset and on
a new SETUP, so a stale latched completion can no longer re-arm EP0
DMA from dead state. No behavior change for targets without dcache.

While root-causing, the FIFO layout was cross-checked against the
DWC2 databook/programming guide v4.20a: the existing GDFIFOCFG
programming (EPInfoBaseAddr = otg_dfifo_depth - 2*ep_count, one SPRAM
word per endpoint direction for buffer DMA) is conformant and needs
no change; the P4 HS instance's reset GDFIFOCFG (0x03800400) merely
reflects a scatter/gather-sized EP_LOC_CNT of 128 that buffer DMA
does not need.

With the fix in place, enable the usbtest battery on the espressif
fleet: tools/build.py allowlists device/usbtest (a plain IDF component
like board_test/video_capture) and both espressif boards' only-lists
gain device/usbtest. Also re-enable device/usbtest on mimxrt1015_evk:
its skip predated the dcd_ci_hs stale-ACTIVE-overlay fix (already on
this branch), which cured the battery that previously killed the
uPD720201 host controller twice (2026-07-11 ROM fw, 2026-07-13 case 27
on fw 2.0.2.6); rig-validated 30/30 three consecutive runs.

Validated on rig (all 30/30): espressif_p4_function_ev(-DMA) (was
22/30 under DMA), espressif_s3_devkitm(-DMA), stm32f723disco(-DMA),
mimxrt1015_evk; p4/s3 slave-mode unaffected (DMA-only code path);
compile-checked stm32h743nucleo +TUD DMA, stm32f407disco,
stm32l476disco (device ports currently on the dead hub).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017TQZrFfU3K4Y198aLsUpBC
@hathach hathach changed the title usbtest: device-side peer for the Linux kernel usbtest battery + DCD fixes across 11 ports usbtest: device-side peer for the Linux kernel usbtest battery + DCD fixes across 13 ports Jul 15, 2026
hathach and others added 2 commits July 15, 2026 18:20
… incompatible

The MosChip MCS9990 card is physically removed from the rig: delete its
cases-11/25 SKIP workaround (and the now-orphaned SKIP accounting) from
usbtest.py and refuse to run outright if a DUT ever sits behind one again.

usb_recover.sh gains `hub-cycle <busport>`: uhubctl VBUS cycle of the port
feeding the device, walking upstream (parent hub -> root port) until it
re-enumerates. Verified on the rig: leaf-level recovery (13-4.4 usbtest
device) and full walk to the root port on a dead branch. SKILL.md updated
for the action and the two-Renesas topology (root-port ppps is real; leaf
1a40:0201 hubs fake their "ganged" switching).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WxUeX4Yn26KibfjvDg2pN9
stm32h533nucleo could never link with IAR: family.cmake points LD_FILE_IAR
at linker/stm32h533xx_flash.icf, which did not exist (every sibling H5
variant has one). Surfaced by CircleCI's one-random job picking
stm32h533nucleo+IAR (Fatal error[Lc002]). H533 and H523 have identical
memory maps (512K flash / 272K RAM; their GCC .ld files differ only in a
comment), so the icf is a copy of the H523 one.
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@hathach
hathach force-pushed the usbtest branch 4 times, most recently from 8c19913 to 1a89552 Compare July 16, 2026 17:33
@hathach
hathach requested a review from Copilot July 17, 2026 01:03
@hathach

hathach commented Jul 17, 2026

Copy link
Copy Markdown
Owner Author

@codex review

Copilot AI 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.

Pull request overview

Copilot reviewed 82 out of 84 changed files in this pull request and generated 2 comments.

Comment thread test/hil/usbtest.py
Comment on lines +162 to +165
root = Path(f"/sys/bus/usb/devices/usb{int(dev['node'].split('/')[-2])}")
drv = (root / '../driver').resolve().name
pci = (root / '..').resolve()
vid_did = ((pci / 'vendor').read_text().strip(), (pci / 'device').read_text().strip())

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Verified on the rig: Path.resolve() has realpath semantics — the usbN symlink is dereferenced before .. is applied, so (root / '..').resolve() lands on the actual parent controller, not back in /sys/bus/usb/devices (that would only happen with lexical normalization, e.g. os.path.normpath). Live check: (Path('/sys/bus/usb/devices/usb11') / '..').resolve()/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0 with vendor/device 0x1912/0x0014 (the Renesas uPD720201) and driver xhci-pci-renesas. The compatibility gate has been correctly identifying controllers and firmware on the HIL rig throughout.

Comment on lines +12 to +14
import re
import sys
from pathlib import Path

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

pids: dict[int, list[str]] = {} is a function-local annotated assignment inside main(), not module-level — per PEP 526, local-variable annotations are never evaluated at runtime (CPython ignores the expression entirely), so there is no import-time or call-time subscript of dict and no crash on Python 3.8. Only module/class-level annotations are evaluated; none of those here use PEP-585 generics.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1a89552548

ℹ️ 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".

Comment on lines +137 to +138
if (!no_dma && nrf52_errata_199()) {
NRF_USBD_ERRATA_199_REG = 0x00000082UL;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear the Errata 199 latch on reset paths

On nRF52840 revisions where nrf52_errata_199() is true, a USB reset or disconnect can arrive while an EasyDMA transfer is active; bus_reset() later clears _dcd without calling edpt_dma_end(), so if no END event accompanies the reset this new NRF_USBD_ERRATA_199_REG = 0x00000082UL latch remains set indefinitely. Since the workaround's purpose is to hold tokens while DMA is pending, leaving it asserted after the transfer was aborted can leave EP0 unable to accept the next SETUP after reset until a DMA completion that may never occur; clear the latch in the reset path as well.

Useful? React with 👍 / 👎.

Comment thread test/hil/tinyusb.json
Comment on lines +508 to +510
{
"name": "mimxrt1064_evk",
"uid": "BAE96FB95AFA6DBB8F00005002001200",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep mimxrt1015 in the HIL matrix

In this HIL config update, mimxrt1064_evk is moved to boards-skip, but the removed mimxrt1015_evk entry is not re-added anywhere (rg mimxrt1015 test/hil finds no remaining entry). That means hil_ci_set_matrix.py test/hil/tinyusb.json no longer builds or runs this ci_hs board at all, so regressions on the port that this change is meant to cover can land silently; please either restore it under boards or move it to boards-skip with an explicit reason.

Useful? React with 👍 / 👎.

hathach added 2 commits July 17, 2026 11:54
Full-fleet profiling (HIL_PROFILE=1 instrumentation, included) showed each
uPD720201 controller's serialized usbtest battery chain dominates wall time,
and a board whose marginal device port bounces during concurrent batteries
can wedge or kill the controller ("xHCI host not responding to stop endpoint
command"). Every such death traced to mimxrt1015's port (its old "kills the
uPD720201" reputation) - it is removed from the config until recabled;
mimxrt1064's enum-retry stalls were a loose device cable (re-seated).
nrf54lm20dk moves to boards-skip until its failing J-Link probe is replugged.
With the hardware fixed both cards run width-4 batteries plus full flash
churn clean, so scheduling stays simple: two symmetric knobs, flashes and
batteries budgeted per controller.

- schedule_boards(): dispatch boards round-robin across host controllers from
  a persisted hint cache (~/.cache/tinyusb-hil/ctrl_cache.json), learned and
  merge-on-write refreshed each run (concurrent HIL jobs keep each other's
  entries). Only the cached PCI address is consumed - dispatch order and
  first-flash budgeting, never battery serialization (batteries resolve live
  or fail closed to an all-slot permit).
- HIL_FLASH_PARALLEL (8) and HIL_USBTEST_PARALLEL (4) are budgeted per
  controller via lock slots assigned on first sight.
- re-runs: a failed run writes <report dir>/<config>.failed with the exact
  re-run spec (--accumulate -b <failed board> -bt <board>:<its failed
  tests>) instead of the inverted --skip-board list of everything that
  passed; --skip-board is gone, --flasher/--exclude-flasher scope a config
  across CI jobs by flasher type (no board names hardcoded in workflows),
  and -a/--accumulate merges a re-run into the existing report. The spec is
  stamped with GITHUB_RUN_ID and cleared on fresh runs, so a retry can never
  consume a spec left behind by a different run's dead or skipped attempt.
- CI: esp-idf firmware builds move out of hil-build into hil-build-esp, and
  the esptool-flashed boards run in their own hil-tinyusb-esp job, so the
  main hil-tinyusb run starts as soon as the fast toolchains finish instead
  of waiting on the slow esp-idf build (an esp toolchain flake previously
  skipped the whole rig run). Artifacts are namespaced per toolchain so the
  esp job downloads only esp-idf binaries.
- HIL_PROFILE=1: timestamped log lines, per-flash durations, permit-wait
  logging, uid->controller map dump for analysis.
- hil_report: per-variant test duration as a dedicated trailing column,
  recorded only by full runs.

Validated on the ci rig (fixed seeds 20260716/777, full fleet at 8/4):
738s/780s walls with only known-flake failures and no controller deaths,
vs 1134-1211s serialized-battery baseline.
FS-forced builds on the HS module are not a supported configuration, so
the 32-bit access path does not need to exist in full-speed builds:
CFG_TUSB_FIFO_HWFIFO_DATA_STRIDE returns to (2 | (TUD_OPT_HIGH_SPEED ? 4 : 0)).
Saves 28 bytes of text on FS-only parts (measured on ra4m1_ek cdc_msc);
high-speed builds keep both widths for the dual-module (FS+HS ports) case.

Build-verified: ra4m1_ek and ra6m5_ek cdc_msc.
vendord_set_alt() aborted bulk/interrupt endpoints of the outgoing
altsetting (stall/clear-stall) but only dropped the iso endpoints'
tracking: an armed iso transfer stayed live in the dcd with its usbd
claim held and no tracked handle to stop it, and its completion fired
into an endpoint the class no longer recognizes. Reachable through the
usbtest example's alt0 (bulk) <-> alt1 (iso) SET_INTERFACE switching.

Track each selected iso endpoint's descriptor (points into the app's
static descriptor set) and deactivate on de-selection: with the
iso-alloc API re-activation is the abort/scrub primitive (resets
ep_status, aborts the stale transfer); without it usbd_edpt_close does,
and the next selection re-opens. Iso cannot be stalled like
bulk/interrupt, hence the separate path.

Build-verified: usbtest for stm32f072disco, ra4m1_ek, raspberry_pi_pico
(iso-alloc) and ch32v307v_r1_1v0 (close API).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2f0c3c1ba

ℹ️ 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".

Comment thread test/hil/hil_test.py
'device/printer_to_cdc',
'device/midi_test',
'device/mtp',
'device/usbtest', # cafe:4010, unique PID; runs the Linux testusb tier-4 battery via usbtest.py

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Stage usbtest.py for remote HIL runs

When HIL is run through test/hil/hil_ci.sh, the remote setup step copies only hil_test.py, pymtp.py, and the JSON config (hil_ci.sh:52-55), not the new test/hil/usbtest.py helper or its recovery helper. Since this line adds device/usbtest to every device board's default run list, remote HIL invocations will flash the firmware and then fail with python3 .../usbtest.py: No such file instead of running the battery; copy the helper(s) in hil_ci.sh before enabling it by default.

Useful? React with 👍 / 👎.

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

ceedling 1.1.0 (released 2026-07-17) fails this project's mock
preprocessing ('Failed to read _build/test/preprocess/.../raw/*.h for
comment stripping'); reproduced locally with an isolated 1.1.0 install
while 1.0.1 passes all 61 tests on the same tree. Unpin once fixed
upstream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants