diff options
| author | hathach <[email protected]> | 2026-08-17 01:02:54 +0700 |
|---|---|---|
| committer | hathach <[email protected]> | 2026-08-18 20:51:21 +0700 |
| commit | 895f77f5534230f0264de881554f89cf6f66a506 (patch) | |
| tree | 88e6cf50b08d703d399672764c7acba3c9c6e553 | |
| parent | 2eaafa9d90c3fe1942e2633998767a4c87d2e6de (diff) | |
examples: document and work around the i.MX RT and LPC55 USB erratafix-ci-hs
ERR050101: while an isochronous IN endpoint is active, an IN token addressed to
that same endpoint number on ANOTHER device sharing the host can silently
unprime one of this device's OUT endpoints - control, bulk, interrupt or
isochronous alike. NXP states it cannot be detected by software and raises no
interrupt, so the endpoint simply stops answering and the transfer never
completes.
The workaround is a uniqueness requirement rather than a particular number: the
isochronous IN endpoint must not share its number with any IN endpoint in use on
the bus. One family-wide constant therefore defeats it, since two affected
boards on the same hub then pick the same number and each becomes the other's
aggressor. CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 is set only for the parts whose
errata list it - RT1015, RT1020, RT1024 and RT1050, where it is marked no fix
scheduled, plus RT1060 and RT1064 rev A - so RT1010 and the RT11xx family keep
the ordinary number and cannot collide with an affected board beside them.
Several affected boards on one hub can still be given distinct numbers with
-DEPNUM_ISO_IN.
The guard covers every example that has an isochronous IN endpoint: audio_test,
audio_4_channel_mic, uac2_headset, cdc_uac2, usbtest, video_capture and
video_capture_2ch. The video examples move the endpoint only when streaming
isochronously, since the bulk configuration is unaffected, and video_capture_2ch
takes two numbers because it has two streams.
The macro name follows CFG_TUSB_RP2_ERRATA_E2/E4/E15 already in tree, and its
is fixed, and which cannot be told apart at compile time - a way to define it to
0.
device_issues.rst records ERR050101 against every affected part with a link to
each errata sheet, and adds the LPC55S2x USB.3 speed-detection and USB.5
isochronous IN entries, neither of which TinyUSB works around. The branch's
design notes are included under docs/superpowers.
Verified: 340 wedge-free runs on mimxrt1064_evk, which previously wedged within
hours, and the macro resolving to endpoint 0x87 on mimxrt1064_evk against 0x83
on mimxrt1010_evk and stm32f407disco.
| -rw-r--r-- | docs/reference/device_issues.rst | 45 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-15-ci-hs-reset-edges.md | 782 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-16-drop-ep0-prime-verify.md | 314 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-15-ci-hs-reset-edges-design.md | 162 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-16-drop-ep0-prime-verify-design.md | 90 | ||||
| -rw-r--r-- | examples/device/audio_4_channel_mic/src/usb_descriptors.c | 4 | ||||
| -rw-r--r-- | examples/device/audio_test/src/usb_descriptors.c | 4 | ||||
| -rw-r--r-- | examples/device/cdc_uac2/src/usb_descriptors.c | 10 | ||||
| -rw-r--r-- | examples/device/uac2_headset/src/usb_descriptors.c | 7 | ||||
| -rw-r--r-- | examples/device/usbtest/src/usb_descriptors.c | 16 | ||||
| -rw-r--r-- | examples/device/video_capture/src/usb_descriptors.c | 4 | ||||
| -rw-r--r-- | examples/device/video_capture_2ch/src/usb_descriptors.c | 11 | ||||
| -rw-r--r-- | src/common/tusb_mcu.h | 19 |
13 files changed, 1466 insertions, 2 deletions
diff --git a/docs/reference/device_issues.rst b/docs/reference/device_issues.rst index 0850409cb..b95a3fc1e 100644 --- a/docs/reference/device_issues.rst +++ b/docs/reference/device_issues.rst @@ -20,6 +20,51 @@ Most severe issues are: - USB.5: In USB full-speed host mode, linked list on done queue is broken. - USB.15: USB high-speed device in endpoint TX data corruption +NXP i.MX RT1015/RT1020/RT1024/RT1050/RT1060/RT1064 +----------------------------------------------------- +**Severity: High** when an isochronous IN endpoint is used behind a hub + +Reference: ERR050101 "USB: Endpoint conflict issue in device mode", listed in the errata sheet of +every part above - `IMXRT1015CE`_, `IMXRT1020CE`_, `IMXRT1024CE`_, `IMXRT1050CE`_, `IMXRT1060CE`_ +and `IMXRT1064CE`_. On RT1060 and RT1064 it applies to rev A silicon only and is fixed in rev B; on +RT1015, RT1020, RT1024 and RT1050 it is marked *no fix scheduled*, so all silicon is affected. +RT1010, RT116x, RT117x and RT118x do not list it. + +.. _IMXRT1015CE: https://www.nxp.com/docs/en/errata/IMXRT1015CE.pdf +.. _IMXRT1020CE: https://www.nxp.com/docs/en/errata/IMXRT1020CE.pdf +.. _IMXRT1024CE: https://www.nxp.com/docs/en/errata/IMXRT1024CE.pdf +.. _IMXRT1050CE: https://www.nxp.com/docs/en/errata/IMXRT1050CE.pdf +.. _IMXRT1060CE: https://www.nxp.com/docs/en/errata/IMXRT1060CE.pdf +.. _IMXRT1064CE: https://www.nxp.com/docs/en/errata/IMXRT1064CE.pdf + +While an isochronous IN endpoint is active, an IN token addressed to *that same endpoint number on +another device sharing the host* can silently unprime one of this device's OUT endpoints - control, +bulk, interrupt or isochronous alike. NXP states the unpriming cannot be detected by software and +raises no interrupt, so the endpoint simply stops answering OUT tokens and the transfer never +completes. Typically seen when the device is behind a hub with other devices attached. + +Workaround: give isochronous IN endpoints a number that no other device on the same host uses for +any IN endpoint - endpoints 1-3 are used by nearly every composite device, so choose a high number +(``examples/device/usbtest`` uses endpoint 7 on this family for that reason). Devices without an +isochronous IN endpoint are unaffected. + +NXP LPC55S2x/LPC552x +--------------------------------- +**Severity: Low** (both need specific conditions) + +Reference: `LPC55S2x Errata Sheet`_ USB.3, USB.5 + +.. _LPC55S2x Errata Sheet: https://www.nxp.com/docs/en/errata/ES_LPC55S2x.pdf + +USB.3: As a high-speed device behind certain full-speed hubs, the device does not correctly detect +the host's KJ chirp sequence and can behave erratically due to wrong speed detection. The documented +workaround is to set the FORCE_FS bit in DEVCMDSTAT on bus reset when the reported link speed is +full speed. TinyUSB does not implement this workaround. + +USB.5: An isochronous IN endpoint sending a 1024-byte maximum-packet-size packet raises no endpoint +interrupt and its command/status entry is not updated. Workaround: cap the isochronous IN maximum +packet size at 1023 bytes in the descriptor. + WCH CH32F20x/CH32V20x/CH32V30x --------------------------------- **Severity: Medium** diff --git a/docs/superpowers/plans/2026-08-15-ci-hs-reset-edges.md b/docs/superpowers/plans/2026-08-15-ci-hs-reset-edges.md new file mode 100644 index 000000000..ec0834e55 --- /dev/null +++ b/docs/superpowers/plans/2026-08-15-ci-hs-reset-edges.md @@ -0,0 +1,782 @@ +# Bus-Reset Edge Events + Review Fix Wave Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give the device stack a "bus reset started" event so ci_hs can tell usbd to stand down at the URI interrupt instead of up to 50 ms later, and clear the ten findings agreed from the max review. + +**Architecture:** `DCD_EVENT_BUS_RESET` splits into `DCD_EVENT_BUS_RESET_START` / `_END` with a compatibility alias, so every other port stays byte-identical. `dcd_ci_hs.c`'s `bus_reset()` splits along the register/software line — registers at URI (`_START`), software structures at the port-change ending the reset (`_END`) — which eliminates the window where usbd believes it is configured over zeroed queue heads. A single bounded-flush helper absorbs the five flush sites. Seven mechanical fixes follow. + +**Tech Stack:** C99, TinyUSB device stack (`src/device/`), ChipIdea HS DCD (`src/portable/chipidea/ci_hs/`), NXP IP3511 DCD (`src/portable/nxp/lpc_ip3511/`), CMake+Ninja and Make builds, J-Link flashing, `test/hil/` HIL harness. + +## Global Constraints + +- Branch `fix-ci-hs` in worktree `/home/hathach/.herdr/worktrees/tinyusb/fix-ci-hs`. Do NOT push; the user pushes. +- C99, 2-space indent, no tabs. Match each file's surrounding style (`dcd_lpc_ip3511.c` mixes styles — follow the immediate neighbourhood). +- Commit messages: imperative mood, no `Co-Authored-By:` or `Claude-Session:` trailers (repo rule: hathach is sole author). +- The repo pre-commit hook (trailing-whitespace, end-of-file-fixer, codespell, unique-PIDs, ceedling unit tests) must pass. If it rewrites a file, re-stage and retry the commit once. +- Comments: short, only the non-obvious "why". Cite manuals as `UM10503 25.10.3` / `Errata LPC546xx USB.13` style — never `ES_` prefixes. +- Never edit anything under `hw/mcu/` or `lib/` (vendor code). +- Build commands used throughout (each ~30-60 s): + `cmake --build examples/cmake-build-<board>` for `mimxrt1064_evk`, `lpcxpresso18s37`, `lpcxpresso11u37`, `lpcxpresso55s28`. +- Design source of truth: `docs/superpowers/specs/2026-08-15-ci-hs-reset-edges-design.md`. + +## File Structure + +| File | Responsibility in this plan | +|---|---| +| `src/device/dcd.h` | Event enum + compatibility alias + contract comment | +| `src/device/usbd.c` | Handle both reset edges; log strings; stop breakpointing on DCD refusal | +| `src/portable/chipidea/ci_hs/dcd_ci_hs.c` | Flush helper; `bus_reset()` split; setup-flush wait; `dcd_set_address`; RESUME guard | +| `src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c` | Torn-setup delivery; USB.13 TODO token | +| `hw/bsp/lpc55/boards/lpcxpresso55s28/board.cmake` | Delete dead RHPORT block | +| `hw/bsp/lpc11/boards/lpcxpresso11u37/lpc11u37.ld` | Correct stale comment; relabel ASSERT | + +Tasks 1-3 are ordered (each builds on the previous); Tasks 4-6 are independent of each other. + +--- + +### Task 1: Split the bus-reset event into START/END edges + +**Files:** +- Modify: `src/device/dcd.h` (enum at lines 23-34; contract comment above it) +- Modify: `src/device/usbd.c` (`_usbd_event_str[]` at line 457; the `DCD_EVENT_BUS_RESET` case at line 700) + +**Interfaces:** +- Produces: `DCD_EVENT_BUS_RESET_START` and `DCD_EVENT_BUS_RESET_END` enum members; `#define DCD_EVENT_BUS_RESET DCD_EVENT_BUS_RESET_END`. Task 2 emits `_START` via the existing `dcd_event_bus_signal(uint8_t rhport, dcd_eventid_t eid, bool in_isr)` and `_END` via the existing `dcd_event_bus_reset(uint8_t rhport, tusb_speed_t speed, bool in_isr)`. + +- [ ] **Step 1: Replace the enum member in `src/device/dcd.h`** + +Replace: + +```c +typedef enum { + DCD_EVENT_INVALID = 0, // 0 + DCD_EVENT_BUS_RESET, // 1 + DCD_EVENT_UNPLUGGED, // 2 + DCD_EVENT_SOF, // 3 + DCD_EVENT_SUSPEND, // 4 TODO LPM Sleep L1 support + DCD_EVENT_RESUME, // 5 + DCD_EVENT_SETUP_RECEIVED, // 6 + DCD_EVENT_XFER_COMPLETE, // 7 + USBD_EVENT_FUNC_CALL, // 8 Not an DCD event, just a convenient way to defer ISR function + DCD_EVENT_COUNT +} dcd_eventid_t; +``` + +with: + +```c +// Bus reset is reported as two edges. BUS_RESET_START is optional: a controller that +// cannot tell the edges apart emits only BUS_RESET_END, which stays self-sufficient (it +// performs the full teardown with or without a preceding START). Emit START when reset +// signaling is detected - the link is unusable and the speed is not negotiated yet - so +// the stack stops using endpoints immediately instead of at the end of the reset. +typedef enum { + DCD_EVENT_INVALID = 0, // 0 + DCD_EVENT_BUS_RESET_START, // 1 + DCD_EVENT_BUS_RESET_END, // 2 with negotiated speed + DCD_EVENT_UNPLUGGED, // 3 + DCD_EVENT_SOF, // 4 + DCD_EVENT_SUSPEND, // 5 TODO LPM Sleep L1 support + DCD_EVENT_RESUME, // 6 + DCD_EVENT_SETUP_RECEIVED, // 7 + DCD_EVENT_XFER_COMPLETE, // 8 + USBD_EVENT_FUNC_CALL, // 9 Not an DCD event, just a convenient way to defer ISR function + DCD_EVENT_COUNT +} dcd_eventid_t; + +#define DCD_EVENT_BUS_RESET DCD_EVENT_BUS_RESET_END // backward compatibility +``` + +- [ ] **Step 2: Update the log-string table in `src/device/usbd.c`** + +At line 457 the table is indexed by event id and MUST stay in enum order. Replace the +`"Bus Reset",` entry (line 459) with two entries: + +```c + "Bus Reset Start", + "Bus Reset End", +``` + +- [ ] **Step 3: Handle both edges in the usbd task loop** + +Replace the case at `src/device/usbd.c:700`: + +```c + case DCD_EVENT_BUS_RESET: + TU_LOG_USBD(": %s Speed\r\n", tu_str_speed[event.bus_reset.speed]); + usbd_reset(event.rhport); + _usbd_dev.speed = event.bus_reset.speed; + break; +``` + +with: + +```c + case DCD_EVENT_BUS_RESET_START: + TU_LOG_USBD("\r\n"); + usbd_reset(event.rhport); + break; + + case DCD_EVENT_BUS_RESET_END: + TU_LOG_USBD(": %s Speed\r\n", tu_str_speed[event.bus_reset.speed]); + // TODO a DCD that reports both edges pays for two teardowns: track a per-rhport + // "start seen" flag and skip this reset, keeping it for the single-event DCDs. + usbd_reset(event.rhport); + _usbd_dev.speed = event.bus_reset.speed; + break; +``` + +- [ ] **Step 4: Verify legacy ports still build (the alias must carry them)** + +Run: + +```bash +cd examples && cmake -B cmake-build-stm32f407disco -DBOARD=stm32f407disco -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel . && cmake --build cmake-build-stm32f407disco +``` + +Expected: builds clean. This board's DCD (dwc2) still calls `dcd_event_bus_reset()`, which +now resolves to `_END` through the unchanged helper — proving the alias works. + +- [ ] **Step 5: Verify the unit tests still build and pass** + +Run: `cd test/unit-test && ceedling test:all` +Expected: all tests pass (they reference `DCD_EVENT_BUS_RESET` via the alias). + +- [ ] **Step 6: Commit** + +```bash +git add src/device/dcd.h src/device/usbd.c +git commit -m "usbd: split bus reset into start/end edge events + +A DCD that can see reset signaling begin has no way to say so: the only +event carries the negotiated speed, which does not exist until the reset +ends. On ChipIdea that leaves the stack believing it is configured for the +whole reset window (3 ms minimum, tens of ms in practice) while the +controller has already torn its endpoints down. + +Add DCD_EVENT_BUS_RESET_START for the leading edge and rename the existing +event to DCD_EVENT_BUS_RESET_END, keeping DCD_EVENT_BUS_RESET as an alias +so every other port and the unit tests are untouched. START is optional and +END stays self-sufficient, so single-event drivers keep working unchanged." +``` + +--- + +### Task 2: Split ci_hs `bus_reset()` across the two edges, behind one flush helper + +**Files:** +- Modify: `src/portable/chipidea/ci_hs/dcd_ci_hs.c` (`bus_reset()`; `dcd_deinit()`; `dcd_edpt_iso_activate()`; the `INTR_RESET` and `INTR_PORT_CHANGE` branches of `dcd_int_handler()`) + +**Interfaces:** +- Consumes: `DCD_EVENT_BUS_RESET_START` (Task 1), `dcd_event_bus_signal()`, `dcd_event_bus_reset()`. +- Produces: `static bool flush_endpoints(ci_hs_regs_t *dcd_reg, uint32_t mask)` — writes `ENDPTFLUSH = mask`, spins bounded by `CI_HS_BUSY_SPIN`, returns `true` if the bits cleared. Used by Task 3. + +- [ ] **Step 1: Add the flush helper next to `bus_reset()`** + +Insert above `bus_reset()`: + +```c +// Flush endpoint buffers and wait for the controller to acknowledge. Callers proceed +// regardless of the result; the bound only prevents an ISR-context hang on dead hardware. +static bool flush_endpoints(ci_hs_regs_t *dcd_reg, uint32_t mask) { + dcd_reg->ENDPTFLUSH = mask; + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTFLUSH & mask) { + if (!guard--) { + return false; + } + } + return true; +} +``` + +- [ ] **Step 2: Split `bus_reset()` into begin/complete** + +Replace the whole `bus_reset()` function with these two. `bus_reset_begin()` keeps only +register work; `bus_reset_complete()` owns everything that touches `_dcd_data`: + +```c +/// Register-side reset handling, must run inside the reset window (UM10503 25.10.3) +static void bus_reset_begin(uint8_t rhport) { + ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); + + // The reset value for all endpoint types is the control endpoint. If one endpoint + // direction is enabled and the paired endpoint of opposite direction is disabled, then the + // endpoint type of the unused direction must be changed from the control type to any other + // type (e.g. bulk). Leaving an un-configured endpoint control will cause undefined behavior + // for the data PID tracking on the active endpoint. + const uint8_t ep_count = ci_ep_count(dcd_reg); + for (uint8_t i = 1; i < ep_count; i++) { + dcd_reg->ENDPTCTRL[i] = ENDPTCTRL_RESET_MASK; + } + + //------------- Clear All Registers -------------// + dcd_reg->ENDPTNAK = dcd_reg->ENDPTNAK; + dcd_reg->ENDPTNAKEN = 0; + dcd_reg->ENDPTSETUPSTAT = dcd_reg->ENDPTSETUPSTAT; + dcd_reg->ENDPTCOMPLETE = dcd_reg->ENDPTCOMPLETE; + + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTPRIME && guard--) {} + flush_endpoints(dcd_reg, 0xFFFFFFFF); +} + +/// Software-side reset handling, deferred to the port change ending the reset so the queue +/// heads stay coherent until the stack is told - and so a prime issued by a task that had +/// not yet seen BUS_RESET_START is flushed here rather than surviving re-enumeration. +static void bus_reset_complete(uint8_t rhport) { + ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); + flush_endpoints(dcd_reg, 0xFFFFFFFF); + + //------------- Queue Head & Queue TD -------------// + tu_memclr(&_dcd_data, sizeof(dcd_data_t)); + + //------------- Set up Control Endpoints (0 OUT, 1 IN) -------------// + _dcd_data.qhd[0][0].zero_length_termination = _dcd_data.qhd[0][1].zero_length_termination = 1; + _dcd_data.qhd[0][0].max_packet_size = _dcd_data.qhd[0][1].max_packet_size = CFG_TUD_ENDPOINT0_SIZE; + _dcd_data.qhd[0][0].qtd_overlay.next = _dcd_data.qhd[0][1].qtd_overlay.next = QTD_NEXT_INVALID; + + _dcd_data.qhd[0][0].int_on_setup = 1; // OUT only + + dcd_dcache_clean_invalidate(&_dcd_data, sizeof(dcd_data_t)); +} +``` + +- [ ] **Step 3: Route the two ISR branches to the new functions** + +In `dcd_int_handler()`, the `INTR_RESET` branch becomes: + +```c + if (int_status & INTR_RESET) { + bus_reset_begin(rhport); + _port_change_reason[rhport] = PORT_CHANGE_REASON_RESET; + dcd_event_bus_signal(rhport, DCD_EVENT_BUS_RESET_START, true); + } +``` + +and inside the `INTR_PORT_CHANGE` branch, the reset arm (the `else` of the resume test) +becomes: + +```c + } else { + bus_reset_complete(rhport); + // PSPD: 0 full, 1 low, 2 high, 3 undefined (treated as full) + const uint32_t pspd = (dcd_reg->PORTSC1 & PORTSC1_PORT_SPEED) >> PORTSC1_PORT_SPEED_POS; + const tusb_speed_t speed = (pspd == 1) ? TUSB_SPEED_LOW : (pspd == 2) ? TUSB_SPEED_HIGH : TUSB_SPEED_FULL; + dcd_event_bus_reset(rhport, speed, true); + } +``` + +Delete the now-unused EP0 `ENDPTFLUSH` line that previously sat at the top of that arm — +`bus_reset_complete()` flushes all endpoints. + +- [ ] **Step 4: Route the remaining flush sites through the helper** + +In `dcd_deinit()`, replace the flush block with: + +```c + // flush all endpoints + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTPRIME && guard--) {} + flush_endpoints(dcd_reg, 0xFFFFFFFF); +``` + +In `dcd_edpt_iso_activate()`, replace the flush + spin with: + +```c + // Flush EP + flush_endpoints(dcd_reg, TU_BIT(epnum + (dir ? 16 : 0))); +``` + +- [ ] **Step 5: Build both ci_hs board families** + +Run: + +```bash +cmake --build examples/cmake-build-mimxrt1064_evk && cmake --build examples/cmake-build-lpcxpresso18s37 +``` + +Expected: both succeed with no new warnings. + +- [ ] **Step 6: Commit** + +```bash +git add src/portable/chipidea/ci_hs/dcd_ci_hs.c +git commit -m "dcd(ci_hs): report bus reset start at URI, finish at port change + +The RM wants the reset cleanup inside the reset window, but the negotiated +speed only exists once the port reaches its operational state, so the stack +was told nothing for the whole window - it kept believing it was configured +while the queue heads had been zeroed under it, and a transfer a class +driver started in that gap stayed primed across re-enumeration. + +Split the work along the register/software line: bus_reset_begin() does the +register cleanup at URI and signals BUS_RESET_START, bus_reset_complete() +re-flushes, resets the queue heads and reports BUS_RESET_END with the final +speed at the port change. Zeroing the queue heads now happens in the same +breath as telling the stack, and the second flush retires anything primed +in between. + +Fold the five hand-rolled endpoint flushes into one bounded helper while +the reset path is open." +``` + +--- + +### Task 3: Make the setup-time EP0 flush wait, and stop dropping the SET_ADDRESS status prime + +**Files:** +- Modify: `src/portable/chipidea/ci_hs/dcd_ci_hs.c` (`dcd_set_address()`; the `ENDPTSETUPSTAT` branch inside `dcd_int_handler()`) + +**Interfaces:** +- Consumes: `flush_endpoints()` (Task 2); `qhd_start_xfer()` returning `bool`, already propagated by `dcd_edpt_xfer()`. + +- [ ] **Step 1: Wait for the setup-time flush to complete** + +In the ISR's setup branch, replace the fire-and-forget flush line + +```c + dcd_reg->ENDPTFLUSH = TU_BIT(0) | TU_BIT(16); +``` + +with + +```c + // Wait it out: the flush retires a status/handshake phase left primed by the previous + // control sequence (UM10503 25.10.8.1.1), and an unfinished flush would otherwise + // still be asserted when the task primes the response to this setup and would retire + // that instead. A flush waits for any packet already in progress - microseconds at + // high speed - and the guard caps wedged hardware. + flush_endpoints(dcd_reg, TU_BIT(0) | TU_BIT(16)); +``` + +- [ ] **Step 2: Honour the status-prime result in `dcd_set_address`** + +Replace the body of `dcd_set_address()`: + +```c +void dcd_set_address(uint8_t rhport, uint8_t dev_addr) { + // Response with status first before changing device address. A refused prime means a new + // setup superseded this transfer; staging an address whose ACK will never arrive would + // leave the device answering on it, so only arm the address when the status went out. + if (dcd_edpt_xfer(rhport, tu_edpt_addr(0, TUSB_DIR_IN), NULL, 0, false)) { + ci_hs_regs_t *dcd_reg = CI_HS_REG(rhport); + dcd_reg->DEVICEADDR = (dev_addr << 25) | TU_BIT(24); + } +} +``` + +- [ ] **Step 3: Build and commit** + +Run: `cmake --build examples/cmake-build-mimxrt1064_evk && cmake --build examples/cmake-build-lpcxpresso18s37` +Expected: both succeed. + +```bash +git add src/portable/chipidea/ci_hs/dcd_ci_hs.c +git commit -m "dcd(ci_hs): wait out the setup flush, honour the set-address prime + +The flush issued on every new setup was fire-and-forget. A flush waits for +a packet already in progress, so it could still be asserted when the task +primed the response to that setup and retire the fresh prime instead - +leaving EP0 silent until the host gave up. + +dcd_set_address() also armed DEVICEADDR unconditionally, but the status +prime can now be refused when a newer setup supersedes the transfer; the +address was then staged behind an ACK that never came and the device sat at +address 0. Only arm it when the status transfer actually started." +``` + +--- + +### Task 4: Emit RESUME only when the port really left suspend + +**Files:** +- Modify: `src/portable/chipidea/ci_hs/dcd_ci_hs.c` (the resume arm of the `INTR_PORT_CHANGE` branch in `dcd_int_handler()`) + +**Interfaces:** none consumed or produced. + +- [ ] **Step 1: Restore the hardware guard** + +In the `INTR_PORT_CHANGE` branch, the resume arm currently reads: + +```c + if (pci_reason == PORT_CHANGE_REASON_SUSPEND) { + dcd_event_bus_signal(rhport, DCD_EVENT_RESUME, true); + } else { +``` + +Replace that condition with one that also consults live hardware: + +```c + if (pci_reason == PORT_CHANGE_REASON_SUSPEND) { + // Only when the port actually left suspend: a starved snapshot can hold the resume's + // port change together with a second suspend, and reporting a resume there would + // leave the stack awake on a sleeping bus with no further event to correct it. + if (!(dcd_reg->PORTSC1 & PORTSC1_SUSPEND)) { + dcd_event_bus_signal(rhport, DCD_EVENT_RESUME, true); + } + } else { +``` + +- [ ] **Step 2: Build and commit** + +Run: `cmake --build examples/cmake-build-mimxrt1064_evk && cmake --build examples/cmake-build-lpcxpresso18s37` +Expected: both succeed. + +```bash +git add src/portable/chipidea/ci_hs/dcd_ci_hs.c +git commit -m "dcd(ci_hs): only report resume when the port left suspend + +A suspend, resume and second suspend collapsed into one interrupt pass +queued suspend then resume from the recorded cause alone, so the stack +ended up awake while the bus was still suspended and nothing arrived to +correct it. Consult PORTSC1 before reporting the resume." +``` + +--- + +### Task 5: ip3511 — never deliver a knowingly-torn setup packet + +**Files:** +- Modify: `src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c` (setup branch of `dcd_int_handler()`; the `dcd_edpt_clear_stall()` comment) + +**Interfaces:** none consumed or produced. + +- [ ] **Step 1: Deliver only when the copy is known good** + +Replace: + +```c + // a SETUP that raced in after the acks (its bit0 consumed above, this copy possibly torn): + // its latch is visible again - re-raise the endpoint interrupt so the next pass redelivers + // the newer payload + if (dcd_reg->DEVCMDSTAT & DEVCMDSTAT_SETUP_RECEIVED_MASK) { + dcd_reg->INTSETSTAT = TU_BIT(0); + } + + dcd_event_setup_received(rhport, setup_copy, true); +``` + +with: + +```c + // a SETUP that raced in after the acks (its bit0 consumed above) makes this copy suspect: + // its latch is visible again, so re-raise the endpoint interrupt and let the next pass + // deliver the newer payload rather than passing up bytes that may be torn between the two + if (dcd_reg->DEVCMDSTAT & DEVCMDSTAT_SETUP_RECEIVED_MASK) { + dcd_reg->INTSETSTAT = TU_BIT(0); + } else { + dcd_event_setup_received(rhport, setup_copy, true); + } +``` + +- [ ] **Step 2: Add the TODO token to the USB.13 deferral** + +In `dcd_edpt_clear_stall()`, change the caveat's opening line from + +```c + // Known caveat (Errata LPC546xx USB.13, same semantics in UM11126): with RF/TV preserved at 1, TR +``` + +to + +```c + // TODO implement the Errata LPC546xx USB.13 work-around (same semantics in UM11126): with RF/TV preserved at 1, TR +``` + +- [ ] **Step 3: Build and commit** + +Run: `cmake --build examples/cmake-build-lpcxpresso11u37 && cmake --build examples/cmake-build-lpcxpresso55s28` +Expected: both succeed. + +```bash +git add src/portable/nxp/lpc_ip3511/dcd_lpc_ip3511.c +git commit -m "dcd(ip3511): drop a setup packet the hardware may have overwritten + +The handler already notices when a new setup landed while it was copying +the previous one, and re-raises the endpoint interrupt so the newer payload +is delivered next pass - but it then passed the suspect copy up anyway. +Usually harmless, since the redelivery supersedes it, but if that second +event cannot be queued the torn bytes are processed as a real request. +Deliver the copy only when no newer setup is pending." +``` + +--- + +### Task 6: BSP cleanups — dead RHPORT block and the stale linker comment + +**Files:** +- Modify: `hw/bsp/lpc55/boards/lpcxpresso55s28/board.cmake` +- Modify: `hw/bsp/lpc11/boards/lpcxpresso11u37/lpc11u37.ld` + +**Interfaces:** none consumed or produced. + +- [ ] **Step 1: Delete the redundant RHPORT block** + +`hw/bsp/lpc55/family.cmake` already applies the identical guarded defaults (`RHPORT_DEVICE 1`, +`RHPORT_HOST 0`) after including the board file, so remove these lines from +`board.cmake` entirely: + +```cmake +# device highspeed, host fullspeed; guarded so a -D override on the cmake command line wins +if (NOT DEFINED RHPORT_DEVICE) + set(RHPORT_DEVICE 1) +endif () +if (NOT DEFINED RHPORT_HOST) + set(RHPORT_HOST 0) +endif () +``` + +Leave `board.mk`'s `RHPORT_DEVICE ?= 1` / `RHPORT_HOST ?= 0` alone — `?=` is the idiomatic +Make form and matches sibling boards. + +- [ ] **Step 2: Prove the defaults and the override still work** + +Run: + +```bash +cd examples && rm -rf /tmp/rh-default /tmp/rh-override +cmake -B /tmp/rh-default -DBOARD=lpcxpresso55s28 -G Ninja . > /tmp/rh-default.log 2>&1 +grep -m1 "RHPORT_DEVICE" /tmp/rh-default.log || cmake -B /tmp/rh-default -DBOARD=lpcxpresso55s28 -G Ninja -LA . | grep -E "^RHPORT_(DEVICE|HOST)" +cmake -B /tmp/rh-override -DBOARD=lpcxpresso55s28 -DRHPORT_DEVICE=0 -DRHPORT_HOST=1 -G Ninja -LA . | grep -E "^RHPORT_(DEVICE|HOST)" +``` + +Expected: the default configure yields device 1 / host 0; the override configure yields +device 0 / host 1. Then rebuild the real tree: `cmake --build cmake-build-lpcxpresso55s28`. + +- [ ] **Step 3: Correct the linker-script comment and relabel the ASSERT** + +In `lpc11u37.ld`, replace the comment block above `__user_stack_top` and the ASSERT with: + +```text + /* Main (MSP/ISR) stack lives at the top of the USB SRAM bank: the 8K main bank is packed so + tight that only ~280 B remained above .bss, and ISR frames overflowed into the topmost task + stack (cdc_msc_freertos hard fault). Nothing else is placed in this bank in either build + system, so the stack owns all 2 KB; the ASSERT is future-proofing in case USB buffers are + ever mapped here again. */ + __user_stack_top = ORIGIN(RamUsb2) + LENGTH(RamUsb2); + ASSERT(__user_stack_top - (ADDR(.noinit_RAM2) + SIZEOF(.noinit_RAM2)) >= 0x200, + "main stack headroom in RamUsb2 below 512 bytes") +``` + +- [ ] **Step 4: Build both build systems for lpc11u37** + +Run: + +```bash +cmake --build examples/cmake-build-lpcxpresso11u37 +cd examples/device/cdc_msc_freertos && make -j8 BOARD=lpcxpresso11u37 all && cd ../../.. +``` + +Expected: both succeed. + +- [ ] **Step 5: Commit** + +```bash +git add hw/bsp/lpc55/boards/lpcxpresso55s28/board.cmake hw/bsp/lpc11/boards/lpcxpresso11u37/lpc11u37.ld +git commit -m "bsp: drop duplicated lpc55s28 rhport defaults, fix lpc11u37 comment + +hw/bsp/lpc55/family.cmake already applies the same guarded rhport defaults +after including the board file, so the board-level copy only added a second +place to keep in sync. + +The lpc11u37 linker comment still described USB buffers living in RamUsb2, +a placement the same branch removed; nothing lands there now, so say so and +label the headroom assert as future-proofing." +``` + +--- + +### Task 7: Stop halting the target when a DCD legitimately refuses a transfer + +**Files:** +- Modify: `src/device/usbd.c` (`usbd_edpt_xfer()` failure arm) + +**Interfaces:** none consumed or produced. + +- [ ] **Step 1: Remove the breakpoint from the DCD-refusal path** + +Replace the failure arm of `usbd_edpt_xfer()`: + +```c + } else { + // DCD error, mark endpoint as ready to allow next transfer + _usbd_dev.ep_status[epnum][dir] &= (uint8_t) ~(TU_EDPT_STATE_BUSY | TU_EDPT_STATE_CLAIMED); + TU_LOG_USBD("FAILED\r\n"); + TU_BREAKPOINT(); + return false; + } +``` + +with: + +```c + } else { + // Driver refused the transfer, mark endpoint as ready to allow next transfer. This is a + // recoverable condition (e.g. a new setup superseding a control response), not a bug, so + // do not break into the debugger - TU_BREAKPOINT() halts the CPU whenever a probe is + // attached, which on a test rig is always. + _usbd_dev.ep_status[epnum][dir] &= (uint8_t) ~(TU_EDPT_STATE_BUSY | TU_EDPT_STATE_CLAIMED); + TU_LOG_USBD("FAILED\r\n"); + return false; + } +``` + +- [ ] **Step 2: Confirm no other stack path relies on that breakpoint** + +Run: `grep -n "TU_BREAKPOINT" src/device/*.c src/device/*.h` +Expected: no remaining hits inside `usbd_edpt_xfer`; other occurrences (if any) are in +unrelated assert macros and stay as they are. + +- [ ] **Step 3: Build and run unit tests** + +Run: + +```bash +cmake --build examples/cmake-build-mimxrt1064_evk +cd test/unit-test && ceedling test:all && cd ../.. +``` + +Expected: build succeeds, all unit tests pass. + +- [ ] **Step 4: Commit** + +```bash +git add src/device/usbd.c +git commit -m "usbd: do not breakpoint when a driver refuses a transfer + +TU_BREAKPOINT() is not gated on CFG_TUSB_DEBUG - it halts the CPU whenever +a debugger is attached, which on a test rig is always. A driver declining a +transfer is recoverable (a new setup superseding a control response, for +one) and the endpoint is already released for the retry, so a halted target +turns a self-healing case into a dead board." +``` + +--- + +### Task 8: Full validation on hardware + +**Files:** none modified — this task produces the evidence for the PR description. + +**Interfaces:** consumes the firmware built by Tasks 1-7. + +- [ ] **Step 1: Software gate** + +Run: + +```bash +pre-commit run --all-files +cd examples +for b in mimxrt1064_evk lpcxpresso18s37 lpcxpresso11u37 lpcxpresso55s28; do + rm -rf cmake-build-$b && cmake -B cmake-build-$b -DBOARD=$b -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel . && cmake --build cmake-build-$b || echo "FAILED $b" +done +cd .. +``` + +Expected: pre-commit all green; all four boards build every example. + +- [ ] **Step 2: Make-build regression checks** + +Run: + +```bash +cd examples/host/cdc_msc_hid && make -j8 BOARD=lpcxpresso55s28 all && cd ../../.. +cd examples/device/cdc_msc_throughput && make -j8 BOARD=lpcxpresso11u37 all && cd ../../.. +``` + +Expected: both link (these two were broken earlier in the branch and are the regression +canaries for the BSP changes). + +- [ ] **Step 3: Flash with verification (mandatory)** + +The mimxrt1064_evk has twice accepted a flash that silently did not take, so every load in +this task uses `verifyfile`. For each board, write a J-Link script of this shape and run it: + +``` +r +h +loadfile examples/cmake-build-<board>/device/usbtest/usbtest.elf +verifyfile examples/cmake-build-<board>/device/usbtest/usbtest.elf +r +g +qc +``` + +Probes and devices: `mimxrt1064_evk` = `-USB 000725299165 -device MIMXRT1064xxx6A`, +`lpcxpresso55s28` = `-USB 000727031389 -device LPC55S28`, +`lpcxpresso11u37` = `-USB 000724441579 -device LPC11U37/401`. +Invoke as `JLinkExe <probe/device args> -if swd -speed 4000 -autoconnect 1 -NoGui 1 -CommandFile <script>`. +Expected: `Verify` reports O.K. and the board re-enumerates as `cafe:4010` with its own +serial before any test runs. + +- [ ] **Step 4: HIL batteries and stress** + +Hold each board's lock for its own leg (`python3 test/hil/hil_lock.py hold <board> --reason "reset-edge validation"`, +release after), never run two batteries at once, and abort if CI is active +(`pgrep -f "hil_test.py [-]-retry"`). + +```bash +# per board: full battery +timeout 700 python3 test/hil/usbtest.py --serial <serial> --json --keep-binding --timeout 60 + +# mimxrt1064_evk only: queued-control stress and the unlink storm +for i in $(seq 1 50); do timeout 200 python3 test/hil/usbtest.py --serial BAE96FB95AFA6DBB8F00005002001200 --tests 9,10 --json --keep-binding --timeout 60 > /dev/null || break; done +for i in $(seq 1 10); do timeout 300 python3 test/hil/usbtest.py --serial BAE96FB95AFA6DBB8F00005002001200 --tests 11,12,24 --json --keep-binding --timeout 60 > /dev/null || break; done +``` + +Serials: 1064 `BAE96FB95AFA6DBB8F00005002001200`, 55s28 `2BF1839A7D51F553A15AB03FD08F70AB`, +11u37 `17121919`. +Expected: 30/30 on all three boards, 50/50 and 10/10 loops, and +`ps -eo stat,comm | awk '$1 ~ /^D/'` empty after each leg. + +- [ ] **Step 5: Reset-path evidence with logging** + +Build and flash `device/cdc_msc` for `mimxrt1064_evk` with `-DLOG=2 -DLOGGER=rtt`, capture +RTT during one unplug/replug cycle (`timeout 20s JLinkRTTClient > /tmp/reset.log`), then: + +```bash +grep -cE "Bus Reset Start" /tmp/reset.log +grep -cE "Bus Reset End" /tmp/reset.log +grep -c "Resume" /tmp/reset.log +``` + +Expected: equal non-zero counts for start and end (one pair per enumeration) and no +`Resume` lines during a plain plug-in. + +- [ ] **Step 6: Suspend/resume pairing** + +With the same RTT build attached, suspend the port from the host and resume it: + +```bash +# find the 1064's busport, then: +echo auto | sudo tee /sys/bus/usb/devices/<busport>/power/control +sleep 5 +echo on | sudo tee /sys/bus/usb/devices/<busport>/power/control +``` + +Expected in the log: one `Suspend` followed by one `Resume`, and no `Bus Reset` of either +edge from the suspend cycle alone. + +- [ ] **Step 7: Record the evidence** + +Append the numbers from Steps 1-6 to the PR description draft. No commit. + +## Self-Review + +**Spec coverage:** §1 event split → Task 1. §2 ci_hs bus_reset split → Task 2. §3 flush +helper → Task 2 (Steps 1, 4). §4 mechanical: setup-flush wait and `dcd_set_address` → Task 3; +RESUME guard → Task 4; ip3511 torn setup and USB.13 TODO → Task 5; usbd breakpoint → Task 7; +BSP pair → Task 6. Verification matrix → Task 8 (legacy-DCD build guard is Task 1 Step 4). +Deferred items are deliberately absent from every task. No gaps. + +**Placeholder scan:** no TBD/TODO-as-placeholder; the two literal `TODO` strings are +deliverable code comments (Task 1 Step 3, Task 5 Step 2). Every code step carries the exact +text to write; every run step carries the command and expected result. + +**Type consistency:** `flush_endpoints(ci_hs_regs_t *dcd_reg, uint32_t mask) -> bool` is +defined in Task 2 Step 1 and used with that exact signature in Task 2 Steps 2/4 and Task 3 +Step 1. `DCD_EVENT_BUS_RESET_START` / `_END` are defined in Task 1 and used in Task 2 Step 3 +via `dcd_event_bus_signal()` / `dcd_event_bus_reset()`, whose signatures are quoted in Task 1's +Interfaces block. `bus_reset_begin()` / `bus_reset_complete()` are defined and called with +matching names in Task 2. diff --git a/docs/superpowers/plans/2026-08-16-drop-ep0-prime-verify.md b/docs/superpowers/plans/2026-08-16-drop-ep0-prime-verify.md new file mode 100644 index 000000000..aa999c9e3 --- /dev/null +++ b/docs/superpowers/plans/2026-08-16-drop-ep0-prime-verify.md @@ -0,0 +1,314 @@ +# Drop the EP0 Post-Prime Verify Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Remove the EP0 post-prime verification that was built on a theory the RT106x endpoint-conflict errata has superseded, and prove on hardware that nothing depended on it. + +**Architecture:** One deletion in `qhd_start_xfer()`, then a rebase onto current master, then an A/B validation whose "with it" arm is already banked (10x 30/30 batteries plus 40 targeted loops on 2026-08-16). No interfaces change: the pre-prime setup-lockout guard keeps `qhd_start_xfer()` returning `bool`, so `dcd_set_address()`'s gating and usbd's failure path stay exactly as they are. + +**Tech Stack:** C99, TinyUSB ChipIdea HS DCD (`src/portable/chipidea/ci_hs/`), CMake+Ninja and Make builds, J-Link (JLinkExe V9.66), `test/hil/usbtest.py` driving the Linux testusb battery. + +## Global Constraints + +- Branch `fix-ci-hs` in worktree `/home/hathach/.herdr/worktrees/tinyusb/fix-ci-hs`. Do NOT push; the user pushes. +- C99, 2-space indent. Commit messages imperative, no `Co-Authored-By:` or `Claude-Session:` trailers (repo rule: hathach is sole author). +- Pre-commit hook (trailing-whitespace, end-of-file-fixer, codespell, unique-PIDs, ceedling) must pass; if it rewrites a file, re-stage and retry the commit once. +- Never edit anything under `hw/mcu/` or `lib/` (vendor code). +- Rig etiquette: hold the board lock for hardware work (`python3 test/hil/hil_lock.py hold <board> --reason "..."`, release after); abort if CI is active (`pgrep -f "hil_test.py [-]-retry"`); NEVER use `uhubctl`, `pci-reset` or `pci-rebind`; never touch the actions-runner. +- JLinkExe on this rig is **V9.66 and has no `verifyfile` command** — use `loadfile` (built-in Program & Verify) plus a mandatory enumeration check. +- Board facts: `mimxrt1064_evk`, serial `BAE96FB95AFA6DBB8F00005002001200`, J-Link probe `000725299165`, device `MIMXRT1064xxx6A`, expected `cafe:4010`. +- Design source of truth: `docs/superpowers/specs/2026-08-16-drop-ep0-prime-verify-design.md`. + +## File Structure + +| File | Responsibility in this plan | +|---|---| +| `src/portable/chipidea/ci_hs/dcd_ci_hs.c` | The only code change: delete the post-prime block in `qhd_start_xfer()` | + +Tasks 2 and 3 change no files; they rebase and validate. + +--- + +### Task 1: Delete the EP0 post-prime verify + +**Files:** +- Modify: `src/portable/chipidea/ci_hs/dcd_ci_hs.c` (the tail of `qhd_start_xfer()`) + +**Interfaces:** +- Produces: `qhd_start_xfer()` keeps its existing signature `static bool qhd_start_xfer(uint8_t rhport, uint8_t epnum, uint8_t dir)` and still returns `false` from the pre-prime setup-lockout guard. No caller changes. + +- [ ] **Step 1: Apply the deletion** + +In `qhd_start_xfer()`, replace this (everything from the prime write to the closing `return true;`): + +```c + // start transfer + const uint32_t prime_bit = TU_BIT(epnum + (dir ? 16 : 0)); + dcd_reg->ENDPTPRIME = prime_bit; + + if (epnum == 0) { + // RM (RT1050 RM Executing a Transfer / UM10503 25.10.8): after priming EP0 the DCD must + // verify the prime completed - ENDPTPRIME bit clear AND the buffer reported ready in + // ENDPTSTAT - because the controller silently cancels an EP0 prime when a SETUP arrives + // during the prime operation. An undetected drop NAK-parks the endpoint forever: usbd never + // re-primes a busy endpoint. A very fast transfer may already have completed and retired the + // ENDPTSTAT bit, so ENDPTCOMPLETE also counts as the prime having taken. + uint32_t guard = CI_HS_BUSY_SPIN; + while (dcd_reg->ENDPTPRIME & prime_bit) { + if (!guard--) { + dcd_reg->ENDPTFLUSH = prime_bit; // never leave a wedged prime armed over a freed buffer + return false; + } + } + // Fail only when the cancel-cause is visibly pending: a completed transfer can have both + // status bits already retired by the ISR, and a cancel whose SETUP the ISR consumed is + // re-driven by that queued SETUP event anyway. + if (!((dcd_reg->ENDPTSTAT | dcd_reg->ENDPTCOMPLETE) & prime_bit) && + (dcd_reg->ENDPTSETUPSTAT & TU_BIT(0))) { + return false; // prime cancelled (setup mid-prime): the pending SETUP re-drives EP0 + } + } + return true; +``` + +with: + +```c + // start transfer + dcd_reg->ENDPTPRIME = TU_BIT(epnum + (dir ? 16 : 0)); + return true; +``` + +Leave the `if (epnum == 0)` setup-lockout block ABOVE the prime write completely untouched — +that one spins on `ENDPTSETUPSTAT` before priming and is required by UM10503 25.10.8.1.1 +step 4. + +- [ ] **Step 2: Confirm nothing else referenced the removed code** + +Run: + +```bash +grep -n "ENDPTSTAT\|ENDPTCOMPLETE\|prime_bit" src/portable/chipidea/ci_hs/dcd_ci_hs.c +``` + +Expected: no `prime_bit` hits at all; `ENDPTCOMPLETE` hits only in `bus_reset_begin()` and the +`INTR_USB` branch of `dcd_int_handler()`; `ENDPTSTAT` hits only in `ci_hs_type.h`-style register +declarations if any appear — none inside `qhd_start_xfer()`. + +- [ ] **Step 3: Build both ci_hs board families** + +Run: + +```bash +cmake --build examples/cmake-build-mimxrt1064_evk && cmake --build examples/cmake-build-lpcxpresso18s37 +``` + +Expected: both succeed, no new warnings (in particular no "unused variable" for anything the +deletion orphaned). + +- [ ] **Step 4: Commit** + +```bash +git add src/portable/chipidea/ci_hs/dcd_ci_hs.c +git commit -m "dcd(ci_hs): drop the EP0 post-prime verify + +The verify came from a theory that a setup arriving mid-prime silently +cancels an EP0 prime, which was how the recurring wedge on the test rig +looked at the time. The wedge turned out to be Errata i.MX RT1064_A +ERR050101: with an isochronous IN endpoint active, an IN token to that +endpoint number on another device sharing the host unprimes one of our OUT +endpoints, undetectably and with no interrupt. Moving the usbtest iso IN +endpoint clear of the conflict fixed it - 340 runs where the board used to +wedge within hours. + +The capture that motivated the verify (EP0 status stage armed but unprimed, +device a control transfer ahead of the host) is explained by that errata +just as well, because it covers control OUT endpoints and a control status +stage is one. So the verify has no independent evidence behind it, while it +does cost two register spins on every EP0 transfer and can misread a +transfer the interrupt handler already completed as a cancelled prime. + +The setup-lockout check before priming stays - that one is in the manual." +``` + +--- + +### Task 2: Rebase onto current master and re-run the software gates + +**Files:** none modified by hand. + +**Interfaces:** none. + +- [ ] **Step 1: Rebase** + +Master has advanced (midi2/usbtmc/video changes) since this branch last rebased. Validating a +tree that is not the one being merged would be a false pass. + +```bash +git fetch origin master +git rebase origin/master +``` + +Expected: clean rebase. If a conflict appears in `src/portable/chipidea/ci_hs/dcd_ci_hs.c` or +`src/device/usbd.c`, resolve it hunk-by-hunk keeping BOTH sides' intent (never `git checkout +--theirs/--ours` on a whole file), then `git rebase --continue`. + +- [ ] **Step 2: Rebuild everything from scratch** + +```bash +cd examples +for b in mimxrt1064_evk lpcxpresso18s37 lpcxpresso11u37 lpcxpresso55s28; do + rm -rf cmake-build-$b + cmake -B cmake-build-$b -DBOARD=$b -G Ninja -DCMAKE_BUILD_TYPE=MinSizeRel . && cmake --build cmake-build-$b || echo "FAILED $b" +done +cd .. +``` + +Expected: all four boards build every example, no "FAILED" line. + +- [ ] **Step 3: Make link canaries** + +```bash +cd examples/host/cdc_msc_hid && make -j8 BOARD=lpcxpresso55s28 all && cd ../../.. +cd examples/device/cdc_msc_throughput && make -j8 BOARD=lpcxpresso11u37 all && cd ../../.. +``` + +Expected: both link. These two were broken earlier in the branch's life and are the regression +canaries for the BSP changes. + +- [ ] **Step 4: Unit tests and pre-commit** + +```bash +cd test/unit-test && ceedling test:all && cd ../.. +pre-commit run --all-files +``` + +Expected: all unit tests pass; every pre-commit hook passes. + +- [ ] **Step 5: No commit** + +This task produces no commit of its own — the rebase rewrites existing commits and the builds +are throwaway. Record the resulting HEAD hash in the report for Task 3 to reference. + +--- + +### Task 3: Hardware A/B on mimxrt1064_evk + +**Files:** none modified — this task produces the evidence. + +**Interfaces:** consumes the firmware built in Task 2 at +`examples/cmake-build-mimxrt1064_evk/device/usbtest/usbtest.elf`. + +Only this board is tested: it is the sole ci_hs board on the rig. The lpcxpresso55s28 and +lpcxpresso11u37 run the ip3511 driver, which this change does not touch. + +- [ ] **Step 1: Preconditions** + +```bash +pgrep -f "hil_test.py [-]-retry" && echo "CI ACTIVE - wait" || echo "CI idle" +ps -eo stat,pid,etimes,comm | awk '$1 ~ /^D/' +python3 test/hil/hil_lock.py hold mimxrt1064_evk --reason "prime-verify removal A/B" +``` + +Expected: CI idle, no pre-existing D-state processes, lock acquired. If CI is active, wait for +it to drain rather than running concurrently. + +- [ ] **Step 2: Flash with verification** + +```bash +cat > /tmp/pv.jlink <<'EOF' +r +h +loadfile examples/cmake-build-mimxrt1064_evk/device/usbtest/usbtest.elf +r +g +qc +EOF +JLinkExe -device MIMXRT1064xxx6A -if SWD -speed 4000 -SelectEmuBySN 000725299165 \ + -autoconnect 1 -nogui 1 -CommandFile /tmp/pv.jlink +``` + +Expected: `Program & Verify` reports O.K. + +- [ ] **Step 3: Confirm the right image is actually running** + +```bash +sleep 5 +grep -l BAE96FB95AFA6DBB8F00005002001200 /sys/bus/usb/devices/*/serial +sudo lsusb -v -d cafe:4010 2>/dev/null | grep -A3 "Isochronous" | grep bEndpointAddress +``` + +Expected: the board is present, and the iso IN endpoint reads **0x87**. If it reads 0x83 the +flash did not take (this board has silently no-op'd a flash twice) — reflash and re-check +before running anything. + +- [ ] **Step 4: 5x full battery** + +```bash +for i in $(seq 1 5); do + timeout 700 python3 test/hil/usbtest.py --serial BAE96FB95AFA6DBB8F00005002001200 \ + --json --keep-binding --timeout 60 2>/dev/null | python3 -c " +import json,sys +d=json.load(sys.stdin) +bad=[str(c['num']) for c in d['cases'] if c['status']!='PASS'] +print(f\"run: {d['passed']}/30 speed={d['speed']}\" + (' FAILED:'+','.join(bad) if bad else '')) +" + ps -eo stat,pid,etimes,comm | awk '$1 ~ /^D/ && $4=="testusb"' +done +``` + +Expected: five lines each reading `30/30 speed=480`, and no testusb D-state line between runs. + +- [ ] **Step 5: 15x control-focused loop** + +These are the paths the removed verify actually protected — queued control, the ch9 subset, and +both ctrl_out cases. A full battery samples each only once per run. + +```bash +PASS=0 +for i in $(seq 1 15); do + timeout 300 python3 test/hil/usbtest.py --serial BAE96FB95AFA6DBB8F00005002001200 \ + --tests 9,10,14,21 --json --keep-binding --timeout 60 >/dev/null 2>&1 && PASS=$((PASS+1)) || { echo "FAILED at iteration $i"; break; } + D=$(ps -eo stat,comm | awk '$1 ~ /^D/ && $2=="testusb"' | wc -l) + [ "$D" != "0" ] && { echo "D-STATE at iteration $i"; break; } +done +echo "control loops: $PASS/15" +``` + +Expected: `control loops: 15/15`, no FAILED or D-STATE line. + +- [ ] **Step 6: Release the lock and record** + +```bash +python3 test/hil/hil_lock.py release mimxrt1064_evk +ps -eo stat,pid,etimes,comm | awk '$1 ~ /^D/' +``` + +Expected: lock released, no leftover D-state. + +**Acceptance:** 5/5 batteries at 30/30, 15/15 control loops, no `testusb` D-state outliving its +case runtime. + +**Rollback trigger:** any control-case failure (errno 110 or 71 on cases 9, 10, 14, 21) or a +lingering D-state means the verify was load-bearing after all. In that case: `git revert` the +Task 1 commit, re-run Steps 4-5 to confirm the failure disappears, and record the result — that +is a finding worth keeping, not a setback to hide. + +--- + +## Self-Review + +**Spec coverage:** the spec's change section → Task 1; "rebase first, then rebuild" → Task 2 +Steps 1-2; software gates → Task 2 Steps 3-4; hardware preconditions, verified flash and the +0x87 descriptor check → Task 3 Steps 1-3; 5x battery and 15x control loop → Task 3 Steps 4-5; +acceptance and rollback trigger → Task 3's closing block. The spec's "deliberately kept" list is +enforced negatively by Task 1 Step 1's instruction to leave the setup-lockout block untouched +and by Task 1 Step 2's grep. No gaps. + +**Placeholder scan:** no TBD/TODO/"handle edge cases"; every step carries its exact command or +code and its expected result. + +**Type consistency:** `qhd_start_xfer(uint8_t rhport, uint8_t epnum, uint8_t dir) -> bool` is +unchanged by this plan and no caller is touched, so there are no cross-task signatures to +reconcile. The only removed identifier, `prime_bit`, is local to the deleted block and Task 1 +Step 2 greps to confirm it has no remaining references. diff --git a/docs/superpowers/specs/2026-08-15-ci-hs-reset-edges-design.md b/docs/superpowers/specs/2026-08-15-ci-hs-reset-edges-design.md new file mode 100644 index 000000000..e01831d34 --- /dev/null +++ b/docs/superpowers/specs/2026-08-15-ci-hs-reset-edges-design.md @@ -0,0 +1,162 @@ +# Bus-reset edge events + review fix wave — design + +Date: 2026-08-15 +Branch: `fix-ci-hs` (unpushed, 6 commits over master `53fef2833`) + +## Problem + +A max-effort review of the branch produced 15 findings. Four are regressions the branch +itself introduced; the rest are pre-existing or cross-cutting. The load-bearing one: + +`dcd_ci_hs.c` now runs the RM-prescribed reset cleanup at the URI (reset-start) interrupt +but does not tell usbd until the Port Change Detect that ends the reset. For the whole +reset window — a minimum of 3 ms, typically 10–50 ms — usbd still believes the device is +configured while the DCD's queue heads have been zeroed. A class driver writing in that +window (`tud_hid_n_report()`, `tud_cdc_write_flush()`) primes a disabled endpoint over a +zeroed dQH, *after* the cleanup's flush, so the stale prime survives re-enumeration over a +buffer usbd has already released. On a 600 MHz M7 that window is enormous. Master had no +gap: cleanup and event were adjacent statements. + +The stack has no way to express "reset started" — `DCD_EVENT_BUS_RESET` carries the +negotiated speed, which does not exist until the reset ends. That missing vocabulary is +the actual defect; the driver-level workarounds considered (deferring the memclr, guarding +primes with a private flag) only shrink the window. + +## Design + +### 1. Stack: split the bus-reset event into two edges + +`src/device/dcd.h`: + +```c +DCD_EVENT_BUS_RESET_START, // reset signaling detected; bus unusable, speed unknown +DCD_EVENT_BUS_RESET_END, // reset complete; .bus_reset.speed is final +... +#define DCD_EVENT_BUS_RESET DCD_EVENT_BUS_RESET_END // backward compatibility +``` + +No new helper: `dcd_event_bus_reset(rhport, speed, in_isr)` keeps its name and emits +`_END`, so every other port is bit-identical to today; `_START` uses the existing +payload-free `dcd_event_bus_signal()`. The alias keeps unit-test/fuzz references +compiling. + +**Contract (documented in `dcd.h`):** `_START` is optional. A DCD that cannot distinguish +the two edges emits only `_END`, which stays self-sufficient — it performs the full +teardown with or without a preceding `_START`. + +`src/device/usbd.c`: +- `case DCD_EVENT_BUS_RESET_START:` → `usbd_reset(rhport)` only; speed untouched. +- `case DCD_EVENT_BUS_RESET_END:` → unchanged (`usbd_reset()` + latch speed). +- `_usbd_event_str[]` gains both names. +- `TODO:` note that a DCD signalling both edges should not pay for two teardowns — track + a per-rhport "start seen" flag and skip the redundant `usbd_reset()` in `_END`, keeping + the unconditional teardown for the legacy single-event path. + +Cost, accepted deliberately: one extra queued event and one extra `usbd_reset()` per +enumeration on ci_hs only, bounded at one per reset against a default +`CFG_TUD_TASK_QUEUE_SZ` of 16 (queue pressure is the failure PR #3817 fixed, hence the +explicit note). + +### 2. ci_hs: split `bus_reset()` along the register/software line + +- **`bus_reset_begin()` — at URI, inside the reset window (UM10503 25.10.3):** ENDPTCTRL + type-reset loop, `ENDPTNAK`/`ENDPTNAKEN`, `ENDPTSETUPSTAT` and `ENDPTCOMPLETE` + write-back clears, bounded `ENDPTPRIME` drain, `ENDPTFLUSH` all. Emit `_START`. + Registers only — nothing in `_dcd_data` is touched, so no software structure is pulled + out from under a task mid-`dcd_edpt_xfer`. +- **`bus_reset_complete()` — at the PCI ending the reset:** re-flush, `tu_memclr(&_dcd_data)`, + EP0 queue-head re-init, dcache clean. Emit `_END` with the final PSPD speed. + +Two properties fall out: the re-flush kills any prime armed during the window without a +new state flag, and the memclr now happens at the same instant usbd is told, so the +"configured over zeroed queue heads" mismatch is eliminated rather than shrunk. Residual +exposure (a task priming exactly as the ISR memclrs) equals master's. + +The reason-dispatch (`pci_reason`, suspend/URI ordering) is unchanged; only the reset +case's body moves. + +### 3. ci_hs: one bounded-flush helper + +Extract `flush_endpoints(dcd_reg, mask)` — writes `ENDPTFLUSH = mask`, spins bounded by +`CI_HS_BUSY_SPIN` until those bits clear, returns `true` if they cleared — and route all +five flush sites through it (`bus_reset_begin`, `bus_reset_complete`, `dcd_deinit`, +`dcd_edpt_iso_activate`, the setup-time EP0 flush). The unified part is the mechanism +(one bound, one spin idiom, one return convention); callers keep their existing reactions, +all of which currently proceed regardless, and that stays true here — no caller gains new +error handling in this wave. Without this, §2 adds a fifth site to a file that already +carried four hand-rolled variants. + +### 4. Mechanical fixes + +`dcd_ci_hs.c` +- Setup-time EP0 flush waits for completion (via §3's helper) before the SETUP event is + queued, so the flush can no longer still be asserted when the task primes the response — + which also dissolves its interaction with the post-prime verify. This adds a bounded + spin in ISR context; the RM notes a flush waits out any packet already in progress, so + the wait is one packet time (microseconds at HS) and the existing `CI_HS_BUSY_SPIN` + bound caps the pathological case, consistent with the file's other flush sites. +- `dcd_set_address()` writes `DEVICEADDR` only if the status-ZLP prime took. A refused + prime means a newer SETUP superseded the transfer; staging an address whose ACK will + never arrive is wrong. +- Emit `DCD_EVENT_RESUME` only when `!(PORTSC1 & PORTSC1_SUSPEND)` (restores master's + hardware guard, lost in the rework). + +`dcd_lpc_ip3511.c` +- Deliver the setup copy only when known-good: + `if (latch still set) { INTSETSTAT = TU_BIT(0); } else { dcd_event_setup_received(...); }`. +- `TODO:` token on the USB.13 deferral so backlog sweeps surface it. + +`usbd.c` +- The DCD-refusal path in `usbd_edpt_xfer` stops routing through the breakpoint-carrying + assert: a DCD declining a prime is documented and self-healing, not a programming error, + and `TU_BREAKPOINT()` is not gated on `CFG_TUSB_DEBUG` — with a probe attached (always, + on the rig) it halts the target. Log and return false instead. + +BSP +- Delete the seven-line RHPORT block in `lpcxpresso55s28/board.cmake` (byte-identical to + `family.cmake`'s own guards; `board.mk`'s `?=` stays as the idiomatic Make form). +- `lpc11u37.ld`: correct the stale comment (nothing lands in RamUsb2 in either build + system now — the stack owns the whole bank) and keep the ASSERT, re-labelled as + future-proofing. + +## Findings improved for free (documented, no code) + +A reset that starts and never completes — cable pulled mid-reset — now delivers `_START` +and tears usbd down, where before usbd stayed configured on a dead bus. This softens both +the adjudicated UNPLUGGED-removal finding and the deferred aborted-reset item: a stray +later PCI delivering `_END` becomes harmless (usbd already torn down, just latches a +speed) instead of deconfiguring a live device. True detach detection still requires OTGSC +B-session-valid VBUS sensing — board-dependent, still a follow-up. + +## Explicitly deferred + +- Prime verification generalized to all endpoints and all causes (RM 25.10.8.2); the + EP0/SETUP-gated form stays, its flush interaction fixed by §4. +- usbd discards `usbd_control_xfer_cb`/`tud_control_xfer` returns — cross-DCD behavior + change needing its own regression pass, despite `usbd.c` being open here. +- Timed-out flush still proceeds to the memclr (now confined to one helper). +- LPC55S2x USB.3 FORCE_FS workaround; iso-IN 1023 enforcement; 8-byte OUT-spill + enforcement; USB.13 INTONNAK workaround. +- Gating `TU_BREAKPOINT()` on `CFG_TUSB_DEBUG` stack-wide. +- Unguarded `set()` RHPORT knobs in ~14 sibling `board.cmake` files. + +## Verification + +1. `pre-commit run --all-files`; builds for mimxrt1064_evk, lpcxpresso18s37, + lpcxpresso11u37, lpcxpresso55s28, plus Make link checks for the two previously-broken + targets (`host/cdc_msc_hid` on 55s28, `device/cdc_msc_throughput` on 11u37). +2. Cross-DCD build guard: one non-ci_hs, non-ip3511 board (e.g. `stm32f407disco`) to prove + the `DCD_EVENT_BUS_RESET` alias keeps legacy ports compiling untouched. +3. HIL on byte-verified flash (`verifyfile` on every J-Link load — the 1064's silent + flash no-op has struck twice): usbtest 30/30 on mimxrt1064_evk, lpcxpresso55s28, + lpcxpresso11u37; 50× case-9/10 loops on the 1064; 10× case-11/12/24 unlink loops. +4. Reset-path specific: confirm HS enumeration (480) and, with `LOG=2`, that a single + enumeration shows exactly one `_START`/`_END` pair and no spurious RESUME. +5. Suspend/resume exercise on the 1064 (host-side autosuspend on the port) confirming + `SUSPEND`/`RESUME` pairing and no reset misclassification. + +## Success criteria + +All four regressions closed, no new findings in a scoped re-review of the wave diff, every +listed HIL result green on verified flash, and legacy DCDs provably untouched (alias build +check + unchanged `_END` semantics). diff --git a/docs/superpowers/specs/2026-08-16-drop-ep0-prime-verify-design.md b/docs/superpowers/specs/2026-08-16-drop-ep0-prime-verify-design.md new file mode 100644 index 000000000..cc1840972 --- /dev/null +++ b/docs/superpowers/specs/2026-08-16-drop-ep0-prime-verify-design.md @@ -0,0 +1,90 @@ +# Drop the EP0 post-prime verify — design + +Date: 2026-08-16 +Branch: `fix-ci-hs` (unpushed, 19 commits over merge-base `53fef2833`) + +## Context + +The branch grew while chasing a wedge on `mimxrt1064_evk`: the board would stop answering a +host transfer, the URB would never complete, `testusb` would block uninterruptibly and the +whole rig would follow it down. Eight occurrences over four days, across the Linux usbtest +battery's queued control and bulk tests. + +The cause turned out to be silicon: **Errata i.MX RT1064_A / RT1060_A ERR050101**. While an +isochronous IN endpoint is active, an IN token addressed to that same endpoint number on +another device sharing the host silently unprimes one of this device's OUT endpoints — +control, bulk, interrupt or isochronous. NXP states it cannot be detected by software and +raises no interrupt. Moving the usbtest example's iso IN endpoint from 3 to 7 (commit +`42870b15b`) cleared it: 340 consecutive wedge-free runs, where the board previously +re-wedged within hours. + +Before that was known, an earlier theory — a SETUP arriving mid-prime silently cancelling an +EP0 prime — produced a post-prime verification block in `qhd_start_xfer()`. That theory's +supporting capture (EP0's status ZLP armed but unprimed, the device a control transfer ahead +of the host) is explained by ERR050101 just as well, because the errata explicitly covers +*control* OUT endpoints and a control status stage **is** an OUT endpoint. The generalized +version of that verify was already reverted (`565bb0d99`) as both regression-prone and aimed +at a failure the vendor documents as undetectable in software. This spec removes what +remains of it. + +## Change + +Delete the post-prime block in `qhd_start_xfer()` (`src/portable/chipidea/ci_hs/dcd_ci_hs.c`): +the bounded `ENDPTPRIME` drain, the `ENDPTFLUSH`-on-timeout, and the +`ENDPTSTAT | ENDPTCOMPLETE` / `ENDPTSETUPSTAT` verdict. The tail becomes: + +```c + // start transfer + dcd_reg->ENDPTPRIME = TU_BIT(epnum + (dir ? 16 : 0)); + return true; +``` + +This removes two register spins and four volatile reads from every EP0 transfer, and with +them the false-fail path a reviewer flagged: a transfer the interrupt handler has already +completed reads identically to a cancelled prime. + +## Deliberately kept + +- **The pre-prime setup-lockout guard** directly above it — UM10503 25.10.8.1.1 step 4 + verbatim ("Before priming for status/handshake phases ensure that ENDPTSETUPSTAT is '0'"), + and older than the wedge theory. It also keeps `qhd_start_xfer()` returning `bool`, so + `dcd_set_address()`'s gating and the usbd breakpoint removal stay meaningful — no cascade. +- **The setup-time EP0 flush and its completion wait** — the flush is the 25.10.8.1.1 step-3 + remark; the wait exists because an unfinished flush can retire a freshly primed response, + an interaction independent of the verify. +- **The `BUS_RESET_START`/`END` split** and the rest of the review-driven hardening. +- Everything hardware-proven: the rf_tv fix, the lpc11u37 stack move, the lpc55s28 + onboarding, the lpc55 Make OHCI link, and the ERR050101 endpoint move itself. + +The commit message records the corrected attribution of the handoff capture, so the next +reader does not re-derive the superseded theory from the same evidence. + +## Validation + +The "with it" arm is already banked from 2026-08-16: 10x 30/30 batteries plus 15x TEST 27, +15x tests 9/10 and 10x tests 11/12/24, all clean. This is the second half of an A/B. + +1. **Rebase onto current master first** (master has moved: midi2/usbtmc/video), then rebuild — + otherwise the validated tree is not the tree that merges. +2. **Software gates:** `pre-commit run --all-files`; full example builds for + mimxrt1064_evk, lpcxpresso18s37, lpcxpresso11u37, lpcxpresso55s28; the two Make link + canaries (`host/cdc_msc_hid` on lpcxpresso55s28, `device/cdc_msc_throughput` on + lpcxpresso11u37); `ceedling test:all`. +3. **Hardware — mimxrt1064_evk only.** It is the only ci_hs board on the rig; the other two + run ip3511, which this change does not touch. Preconditions: CI idle + (`pgrep -f "hil_test.py [-]-retry"`), board lock held for the whole run. Flash with + `loadfile` (its built-in Program & Verify — JLinkExe V9.66 has no `verifyfile`), then + confirm re-enumeration as `cafe:4010` with serial `BAE96FB95AFA6DBB8F00005002001200`, and + confirm `lsusb -v` still reports the iso IN endpoint as **0x87** so a stale image cannot + masquerade as a pass. +4. **Runs:** 5x the full 30-case battery, then 15x `--tests 9,10,14,21` (queued control, ch9 + subset, both ctrl_out cases) — the control paths the verify actually protected, which a + plain battery samples only once per run. Print a `testusb` D-state scan after every + iteration. + +**Acceptance:** 5/5 batteries at 30/30, 15/15 loops, and no `testusb` D-state outliving its +case runtime. + +**Rollback trigger:** any control-case failure (errno 110 or 71 on cases 9, 10, 14, 21) or a +lingering D-state means the verify was load-bearing after all — restore it and record that +result in the commit message. A negative result is a finding, not a setback. diff --git a/examples/device/audio_4_channel_mic/src/usb_descriptors.c b/examples/device/audio_4_channel_mic/src/usb_descriptors.c index 42da9442c..020bc934e 100644 --- a/examples/device/audio_4_channel_mic/src/usb_descriptors.c +++ b/examples/device/audio_4_channel_mic/src/usb_descriptors.c @@ -92,6 +92,10 @@ enum // Only EP3 is available for ISO #define EPNUM_AUDIO 0x03 +#elif CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 + // ERR050101 (see CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101): an isochronous IN endpoint needs a number + // no other device on the bus uses + #define EPNUM_AUDIO 0x07 #else #define EPNUM_AUDIO 0x01 #endif diff --git a/examples/device/audio_test/src/usb_descriptors.c b/examples/device/audio_test/src/usb_descriptors.c index 8c25fc290..d16526116 100644 --- a/examples/device/audio_test/src/usb_descriptors.c +++ b/examples/device/audio_test/src/usb_descriptors.c @@ -92,6 +92,10 @@ enum // Only EP3 is available for ISO #define EPNUM_AUDIO 0x03 +#elif CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 + // ERR050101 (see CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101): an isochronous IN endpoint needs a number + // no other device on the bus uses + #define EPNUM_AUDIO 0x07 #else #define EPNUM_AUDIO 0x01 #endif diff --git a/examples/device/cdc_uac2/src/usb_descriptors.c b/examples/device/cdc_uac2/src/usb_descriptors.c index 9c1bbee47..648c56e71 100644 --- a/examples/device/cdc_uac2/src/usb_descriptors.c +++ b/examples/device/cdc_uac2/src/usb_descriptors.c @@ -110,6 +110,16 @@ uint8_t const * tud_descriptor_device_cb(void) #define EPNUM_CDC_IN 0x85 #endif +#elif CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 + // ERR050101 (see CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101): an isochronous IN endpoint needs a number + // no other device on the bus uses + #define EPNUM_AUDIO_IN 0x07 + #define EPNUM_AUDIO_OUT 0x01 + + #define EPNUM_CDC_NOTIF 0x83 + #define EPNUM_CDC_OUT 0x04 + #define EPNUM_CDC_IN 0x84 + #else #define EPNUM_AUDIO_IN 0x01 #define EPNUM_AUDIO_OUT 0x01 diff --git a/examples/device/uac2_headset/src/usb_descriptors.c b/examples/device/uac2_headset/src/usb_descriptors.c index 27b6c930c..d8cdd768e 100644 --- a/examples/device/uac2_headset/src/usb_descriptors.c +++ b/examples/device/uac2_headset/src/usb_descriptors.c @@ -109,6 +109,13 @@ uint8_t const * tud_descriptor_device_cb(void) #define EPNUM_AUDIO_INT 0x03 #endif +#elif CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 + // ERR050101 (see CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101): an isochronous IN endpoint needs a number + // no other device on the bus uses + #define EPNUM_AUDIO_IN 0x07 + #define EPNUM_AUDIO_OUT 0x01 + #define EPNUM_AUDIO_INT 0x02 + #else #define EPNUM_AUDIO_IN 0x01 #define EPNUM_AUDIO_OUT 0x01 diff --git a/examples/device/usbtest/src/usb_descriptors.c b/examples/device/usbtest/src/usb_descriptors.c index b4f46adb8..8453885c4 100644 --- a/examples/device/usbtest/src/usb_descriptors.c +++ b/examples/device/usbtest/src/usb_descriptors.c @@ -132,6 +132,22 @@ enum { #define EPNUM_ISO_OUT 0x08 #define EPNUM_ISO_IN 0x88 +#elif CFG_TUSB_MCU == OPT_MCU_MIMXRT1XXX + #define EPNUM_BULK_OUT 0x01 + #define EPNUM_BULK_IN 0x81 + #define EPNUM_INT_OUT 0x02 + #define EPNUM_INT_IN 0x82 + #define EPNUM_ISO_OUT 0x03 + // ERR050101 (see CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101): park the iso IN endpoint clear of the numbers + // other devices use. Override per board when several affected boards share a hub. + #ifndef EPNUM_ISO_IN + #if CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 + #define EPNUM_ISO_IN 0x87 + #else + #define EPNUM_ISO_IN 0x83 + #endif + #endif + #else #define EPNUM_BULK_OUT 0x01 #define EPNUM_BULK_IN 0x81 diff --git a/examples/device/video_capture/src/usb_descriptors.c b/examples/device/video_capture/src/usb_descriptors.c index d5d805f0b..11321a305 100644 --- a/examples/device/video_capture/src/usb_descriptors.c +++ b/examples/device/video_capture/src/usb_descriptors.c @@ -113,6 +113,10 @@ enum { #define EPNUM_VIDEO_IN (CFG_TUD_VIDEO_STREAMING_BULK ? 0x81 : 0x88) #elif TU_CHECK_MCU(OPT_MCU_MAX32650, OPT_MCU_MAX32666, OPT_MCU_MAX32690, OPT_MCU_MAX78002) #define EPNUM_VIDEO_IN 0x81 +#elif CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 + // ERR050101 (see CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101): an isochronous IN endpoint needs a number + // no other device on the bus uses + #define EPNUM_VIDEO_IN (CFG_TUD_VIDEO_STREAMING_BULK ? 0x81 : 0x87) #else #define EPNUM_VIDEO_IN 0x81 #endif diff --git a/examples/device/video_capture_2ch/src/usb_descriptors.c b/examples/device/video_capture_2ch/src/usb_descriptors.c index ad65cc019..2630be84b 100644 --- a/examples/device/video_capture_2ch/src/usb_descriptors.c +++ b/examples/device/video_capture_2ch/src/usb_descriptors.c @@ -110,8 +110,15 @@ enum { ITF_NUM_TOTAL }; -#define EPNUM_VIDEO_IN_1 0x81 -#define EPNUM_VIDEO_IN_2 0x82 +#if CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 && !CFG_TUD_VIDEO_STREAMING_BULK + // ERR050101 (see CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101): an isochronous IN endpoint needs a number + // no other device on the bus uses + #define EPNUM_VIDEO_IN_1 0x86 + #define EPNUM_VIDEO_IN_2 0x87 +#else + #define EPNUM_VIDEO_IN_1 0x81 + #define EPNUM_VIDEO_IN_2 0x82 +#endif #if defined(CFG_EXAMPLE_VIDEO_READONLY) && !defined(CFG_EXAMPLE_VIDEO_DISABLE_MJPEG) #define USE_MJPEG 1 diff --git a/src/common/tusb_mcu.h b/src/common/tusb_mcu.h index 93b4a2ee9..af43dfb12 100644 --- a/src/common/tusb_mcu.h +++ b/src/common/tusb_mcu.h @@ -126,6 +126,14 @@ #define CFG_TUSB_MEM_DCACHE_LINE_SIZE_DEFAULT 32 #endif + // Errata ERR050101, listed for RT1015/RT1020/RT1024/RT1050 (no fix scheduled) and for + // RT1060/RT1064 rev A (fixed in rev B); not listed for RT1010 or the RT11xx family. + #if defined(MIMXRT1015_SERIES) || defined(MIMXRT1021_SERIES) || defined(MIMXRT1024_SERIES) || \ + defined(MIMXRT1051_SERIES) || defined(MIMXRT1052_SERIES) || defined(MIMXRT1061_SERIES) || \ + defined(MIMXRT1062_SERIES) || defined(MIMXRT1064_SERIES) + #define CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 1 + #endif + #elif TU_CHECK_MCU(OPT_MCU_KINETIS_KL, OPT_MCU_KINETIS_K32L, OPT_MCU_KINETIS_K) #define TUP_USBIP_CHIPIDEA_FS #define TUP_USBIP_CHIPIDEA_FS_KINETIS @@ -768,6 +776,17 @@ #define TUP_DCD_EDPT_ISO_ALLOC #endif +// Set by silicon whose isochronous IN endpoint can be unprimed by an IN token sent to that same +// endpoint number on ANOTHER device sharing the host, taking one of this device's OUT endpoints +// down with it - undetectable in software. Descriptors must then give an isochronous IN endpoint +// a number no other device on the bus uses; a number is only safe while it stays unique, so two +// affected boards on one hub must not pick the same one. Default 0 (no such conflict). Set it to +// 0 by hand on RT1060/RT1064 rev B, which carry the fix - the revision cannot be told apart at +// compile time, so the affected parts are assumed to be rev A. +#ifndef CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 + #define CFG_TUSB_MIMXRT1XXX_ERRATA_ERR050101 0 +#endif + // Some USBIPs (SAMG, SAMX7X, PIC32, MAX3266x/MAX78002) cannot assign the same endpoint // number to both IN and OUT. Default to 0 (same endpoint number may be used for IN and OUT). #ifndef CFG_TUD_ENDPOINT_ONE_DIRECTION_ONLY |
