summaryrefslogtreecommitdiff
path: root/docs/superpowers/specs
diff options
context:
space:
mode:
authorhathach <[email protected]>2026-08-17 01:02:54 +0700
committerhathach <[email protected]>2026-08-18 22:07:49 +0700
commit19ff2ed615e4a97984aab5551ac8835ead53b9e7 (patch)
tree1035bcaa6242d5bcbe5fd81827ecded0dafdcb6d /docs/superpowers/specs
parentb925231216eabf277938607ba50f1f4b78c0ce7d (diff)
examples: document and work around the i.MX RT and LPC55 USB errata
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.
Diffstat (limited to 'docs/superpowers/specs')
-rw-r--r--docs/superpowers/specs/2026-08-15-ci-hs-reset-edges-design.md162
-rw-r--r--docs/superpowers/specs/2026-08-16-drop-ep0-prime-verify-design.md90
2 files changed, 252 insertions, 0 deletions
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.