diff options
| author | HiFiPHile <[email protected]> | 2026-08-25 09:55:24 +0200 |
|---|---|---|
| committer | HiFiPHile <[email protected]> | 2026-09-02 10:50:02 +0200 |
| commit | a0d3de76869ce728c7c1d9713085bc970da39671 (patch) | |
| tree | bd24f3efb60daeda2e0aaf4df08da97504aafbf3 /docs/superpowers/specs | |
| parent | 278c0531a07e4fe8112ef91d0d2dbba8ef0a40b3 (diff) | |
| parent | 5c0e31cdabaf37f14e1f5e988a020abfc1000495 (diff) | |
Merge branch 'master' into agent/fix-dwc2-host-fifo-allocation
Diffstat (limited to 'docs/superpowers/specs')
4 files changed, 499 insertions, 11 deletions
diff --git a/docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md b/docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md index 8158758bc..898b3c8ab 100644 --- a/docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md +++ b/docs/superpowers/specs/2026-07-29-hil-pr-scoped-selection-design.md @@ -1,4 +1,4 @@ -# PR-scoped HIL selection: hil_select.py +# PR-scoped HIL selection: helper/hil_select.py **Date:** 2026-07-29 **Branch:** `claude/hil-select` (based on `claude/hil-pool-check`, which carries the @@ -26,17 +26,17 @@ confident; every uncertainty widens to the full matrix. - Scoping push/master/scheduled runs (always full). - Changing hil_test.py behavior (the selector only *composes* existing `-b`/`-bt` args). -## Component: `test/hil/hil_select.py` +## Component: `test/hil/helper/hil_select.py` Stdlib-only, importable and CLI. Lives beside the harness so `hil_ci.sh` copies are unaffected (it runs on the GitHub runner / dev PC, not on the rig). It must NOT import `hil_test.py` (which drags pyserial/pymtp onto the bare GitHub runner): the three test lists -(`device_tests`, `dual_tests`, `host_test`) move verbatim into a tiny stdlib-only -`test/hil/hil_examples.py` that both `hil_test.py` and `hil_select.py` import (behavior -preserving; `hil_ci.sh` scp list gains the new file). +(`device_tests`, `dual_tests`, `host_test`) move verbatim into the stdlib-only +`test/hil/helper/hil_util.py` that both `hil_test.py` and `hil_select.py` import (behavior +preserving; `hil_ci.sh` copies the whole `helper/` directory). ``` -python3 test/hil/hil_select.py --base <ref> [--diff-file <path>] CONFIG.json [CONFIG.json...] +python3 test/hil/helper/hil_select.py --base <ref> [--diff-file <path>] CONFIG.json [CONFIG.json...] ``` - `--base REF`: changed files = `git diff --name-only $(git merge-base HEAD REF)..HEAD` @@ -118,7 +118,7 @@ is skipped, not widened (running unrelated boards would test nothing relevant). ## CI wiring (`.github/workflows/build.yml`) - `set-matrix` (PR events only): after generating today's matrices, run - `hil_select.py --base origin/${{ github.base_ref }} test/hil/tinyusb.json test/hil/hfp.json` + `helper/hil_select.py --base origin/${{ github.base_ref }} test/hil/tinyusb.json test/hil/hfp.json` (checkout with enough history to reach the merge base: `fetch-depth: 0` on this one job, or an explicit `git fetch origin $BASE_REF`). New job outputs: `hil_select_full`, `hil_args_tinyusb`, `hil_args_hfp`, plus the selected-board list consumed by the matrix @@ -137,15 +137,15 @@ is skipped, not widened (running unrelated boards would test nothing relevant). ## Local use - pre-pr's "Map changes to boards" step delegates to - `python3 test/hil/hil_select.py --base $BASE test/hil/tinyusb.json` and derives its + `python3 test/hil/helper/hil_select.py --base $BASE test/hil/tinyusb.json` and derives its one-board-per-family sample from the selector's board set (its capping/sampling policy is unchanged — the selector provides the affected set, pre-pr samples it). -- Manual: `python3 test/hil/hil_test.py -B examples $(python3 test/hil/hil_select.py --base master test/hil/tinyusb.json | jq -r '.args["tinyusb.json"]') test/hil/tinyusb.json` +- Manual: `python3 test/hil/hil_test.py -B examples $(python3 test/hil/helper/hil_select.py --base master test/hil/tinyusb.json | jq -r '.args["tinyusb.json"]') test/hil/tinyusb.json` — documented in the hil skill. ## Testing -`test/hil/test_hil_select.py` — stdlib `unittest`, no hardware, injected diffs via +`test/hil/test/test_hil_select.py` — stdlib `unittest`, no hardware, injected diffs via `--diff-file`/API. Cases (the acceptance examples): 1. `src/portable/raspberrypi/rp2040/dcd_rp2040.c` → only rp2040-family roster boards, device tests only, host-only boards absent, `full` false. @@ -161,7 +161,7 @@ is skipped, not widened (running unrelated boards would test nothing relevant). 7. `hw/bsp/rp2040/family.cmake` → rp2040-family boards, all their tests. 8. Mixed device+host diff → no pruning (both roles present). The suite runs in `set-matrix` before the selector is used, and locally via -`python3 test/hil/test_hil_select.py`. +`python3 test/hil/test/test_hil_select.py`. ## Safety properties diff --git a/docs/superpowers/specs/2026-07-30-hil-usbtest-fleet-wedge-design.md b/docs/superpowers/specs/2026-07-30-hil-usbtest-fleet-wedge-design.md new file mode 100644 index 000000000..3ed0c1519 --- /dev/null +++ b/docs/superpowers/specs/2026-07-30-hil-usbtest-fleet-wedge-design.md @@ -0,0 +1,236 @@ +# HIL fleet-wedge containment + +Date: 2026-07-30 +Status: implemented, then superseded in part — addendum last checked 2026-08-12 +against the shipped code; where they disagree the CODE and the usb-kernel-recover +skill win, never this document. + +- **Pool guard.** A single constant, not the flat 4200s below and not a derivation: + `POOL_TIMEOUT = pos_int_env('HIL_POOL_TIMEOUT', 3600)`. A per-controller model briefly + lived here and was removed -- it under-modelled the flash phase and could INVERT + (adding a usbtest board lowered the guard, because the derived value fell below the + baseline it was meant to raise). The guard's only job is to stop a wedged pool short + of the job ceiling so the report still gets written; predicting a healthy run's + duration is a different problem. `pos_int_env` warns only on a non-integer or a value + <= 0: there is NO upper clamp and no warning above any threshold, so a pin larger than + a job ceiling silently restores the inversion this work removed. +- **Job ceilings.** 90/90/120 min (build.yml), not 60/60/90 and not the 85/115 below. + They must clear the 3600s guard plus the pre-pool checkout/artifact merge and the + post-guard sweep and report upload. No job pins `HIL_POOL_TIMEOUT`. +- **Battery budgets.** `USBTEST_BATTERY_BUDGET` 260s, `USBTEST_RECOVERY_BUDGET` 250s. + The 200s-with-a-197s-floor derivation recorded here was never shipped; the floor + assertion was removed with it. +- **HUNG recovery.** Reflash of the DUT through its roster flasher + (`usbtest.py --recover-board/--recover-fw`), not the root-cycle-first recovery in + section 1d — replaced after the 2026-08-11 ppps measurement (uhubctl never cuts + VBUS; root-cycle is probe-only). Since 2026-08-12 the reflash is SKIPPED + when `hil_flash.convoy_safe(board['flasher'])` is false (usbtest.py:675): the flasher + would enumerate by opening usbfs nodes, block on the same convoy, and become a second + stray rather than clear the first. A holder that owns the device lock inside a driver + ioctl is terminal either way -- a reflash only produces a disconnect, and + `usb_disconnect()` needs that same lock -- and that state needs a reboot. + +Step 0 done — the host was rebooted 2026-07-30 14:11 and the rig +came back clean. The device that triggered this incident was removed from the rig, so +only the containment work remains relevant. +Rig: `ci.lan` (Proxmox guest on `pve.lan`) + +## Problem + +On 2026-07-29/30 every board in the `ci.lan` usbtest fleet failed, `openocd` processes +landed in uninterruptible sleep, and no subsequent HIL run could start. Two GitHub +Actions runs were stranded: `30484641269` sat `in_progress` for over eight hours +(past GitHub's own 360-minute default), and `30485082274` sat `queued` behind it from +2026-07-29 19:35 UTC onward. Both report directories were written empty. + +A reboot of the `ci` guest at 10:48 did not clear the condition: the same kernel state +re-formed at 10:52:23. + +## Root cause + +Five layers, each independently observable. + +### 1. A permanently wedged hub worker holds a root-hub device lock + +A device that repeatedly re-asserts connect while failing to enumerate keeps +`hub_event()` busy, and `hub_event()` holds `usb_lock_device(hdev)` on its hub for its +whole run (hub.c:5896/5989). The `usb_hub_wq` worker sits in `hub_port_reset`, so that +hub's `device_lock` is effectively never released: + +``` +kworker/14:6+usb_hub_wq (state D, 400+ s) + msleep+0x2b + hub_port_reset+0x1a4 [usbcore] + hub_event+0x727 [usbcore] +``` + +`usb usbN-portM: Cannot enable. Maybe the USB cable is bad?` is logged every four seconds +for as long as it lasts. + +Verified against hub.c v6.12.96 rather than inferred: the kernel does **not** retry +without bound, and root and downstream ports are bounded identically — +`hub_port_reset()` tries `PORT_RESET_TRIES` then logs that message (hub.c:3149), +`hub_port_connect()` wraps it in `PORT_INIT_TRIES` = 4 and disables the port on give-up +(hub.c:5455/5619). A count in the thousands is therefore that many separate connect +events, not one runaway loop, and it indicts the device rather than the port. + +### 2. A parked board storms the second controller + +`ra6m5_ek` (`test/hil/tinyusb.json`, uid `8419032D32363657364EF4622D294B4E`, at +`13-3.3`) runs dfu firmware (`cafe:400b`) and re-enumerates every 1-2 seconds +continuously, wrapping the entire bus-13 devnum space (`...120 -> 127 -> 4 -> 6 -> 10`). +This is standing `hub_event` and Address-Device pressure on controller `03:00.0`, +concurrent with parallel usbtest batteries on the same silicon. + +The board is already listed in `boards-skip`, which is precisely why it storms: +`boards-skip` stops testing a board but never parks it, so it keeps running whatever +firmware it last received. Park-flash only runs as teardown of a board that actually +executed tests. + +### 3. The kernel `usbtest` control-queue case waits without a timeout + +`test_ctrl_queue` blocks on an untimed `wait_for_completion()` while `usbdev_ioctl` +holds the DUT's `device_lock`: + +``` +wait_for_completion+0x8a <- no _timeout variant +test_ctrl_queue+0x4ab [usbtest] +usbtest_do_ioctl+0x501 [usbtest] +usbdev_ioctl+0x6b8 [usbcore] +``` + +`--timeout 60` in `test/hil/usbtest.py` is a subprocess timeout only. `SIGKILL` is not +delivered to a task in uninterruptible sleep. `usbtest.py` already recognises this and +reports `HUNG`, then calls `usb_recover.sh root-cycle`. + +### 4. openocd inherits the convoy and the whole fleet dies + +Once a device lock is stuck, `port_event()` takes a child device's lock to warm-reset +it and blocks while still holding its hub's lock. Any later +`open("/dev/bus/usb/BBB/DDD")` against such a device blocks uninterruptibly: + +``` +usbdev_open+0xdc [usbcore] -> __mutex_lock +chrdev_open -> do_sys_openat2 -> __x64_sys_openat +``` + +That is the state of the three `openocd` processes at 04:16:51 (pids 207921, 207987, +208034) — the flasher, unkillable. Because one controller carries two buses, a single +convoy takes out every board on both, which is why the failure presents as the entire +fleet. + +The existing `HUNG` recovery cannot help here. A root-port VBUS cycle frees a +*device-lock* holder; it cannot free a lock held by a stuck *hub worker*, and on this +rig the cycle lands on the controller that is already wedged. + +### 5. Nothing bounds the damage, so one bad run becomes a CI outage + +- `hil-tinyusb` and `hil-tinyusb-esp` in `.github/workflows/build.yml` carry no + `timeout-minutes`. Only `hil-hfp-iar` does. +- `ci.lan` runs a single runner service, so there is one job slot. +- `test/hil/hil_test.py` bounds the pool with `POOL_TIMEOUT` (4200 s), and that guard + fires correctly — but the recovery path does not survive a D-state worker: + +```python +with Pool(processes=os.cpu_count() or 1, initializer=init_worker, initargs=initargs) as pool: + async_ret = pool.map_async(test_board, config_boards) + try: + mret = async_ret.get(timeout=POOL_TIMEOUT) + except MpTimeoutError: + pool.terminate() + pool.join() # blocks forever: a D-state worker never reaps + raise RuntimeError(f'HIL worker pool timed out after {POOL_TIMEOUT}s') +``` + +`multiprocessing` joins workers unbounded, so both `pool.terminate()` and +`pool.join()` hang, as does the `with Pool(...)` exit on the success path. Normal +`hil-tinyusb (tinyusb.json)` runs take 10-20 minutes; one recent run took 71.3 +minutes, which is the 70-minute guard firing and succeeding. The eight-hour run is the +pathological case. + +## Design + +### Step 0 — recovery (manual prerequisite) + +Power-cycle the PVE **host**, not the `ci` guest. A guest reboot is not sufficient; +hubs latch up across the PCIe reset, which the 10:48 reboot demonstrated. Nothing +below can be verified until the rig is clean. + +### Section 1 — CI containment + +**1a. Two layered timers.** An inner guard inside `hil_test.py` (`POOL_TIMEOUT`, 70 min) +that fails gracefully -- it writes a report naming the timeout and the dispatched boards, +shuts the pool down and exits -- and an outer `timeout-minutes` per rig job (85 for the +hil-tinyusb jobs; 115 for hil-hfp-iar, which also builds four boards with IAR in the same +job) as the backstop for when even exiting cannot free the runner. The ceiling must stay +ABOVE the inner guard, or GitHub kills the job before the report is written. + +> **Corrected after measurement.** An earlier revision cut the guard to 30 min on the +> reading that real runs take 9-17 min and everything longer was the old guard firing. +> That was wrong. `hil_lock.py` records 22.2/14.3/12.5/10.8 min at usbtest width 1/2/3/4, +> and raising the per-battery budget to 380s made hung boards cost more again. The 30 min +> guard then fired on 5 of the last 8 HIL job executions across both rigs, and because +> `map_async` is all-or-nothing each of those runs published a banner instead of any +> per-board result. Restored to 4200s, the value whose original rationale -- usbtest +> batteries are serialized fleet-wide, lengthening the tail -- was correct. + +**1b. Bound the pool shutdown.** Add a helper to `test/hil/hil_test.py`: + +```python +def _shutdown_pool(pool, grace=30): + """terminate() a Pool without ever blocking forever: multiprocessing joins its + workers unbounded, and a worker in uninterruptible sleep (wedged usbfs) never + reaps -- which would hold the runner's only job slot indefinitely.""" + t = threading.Thread(target=pool.terminate, daemon=True) + t.start() + t.join(grace) + return not t.is_alive() +``` + +On the `MpTimeoutError` path: write the report first, recording the boards that never +reported so the run stops producing an empty report directory; then `_shutdown_pool`; +then `os._exit(1)` if it did not return. The hard exit is the point — it is the only +way past a kernel-side unkillable child. Use the same helper for the `with Pool(...)` +exit path. + +**1c. Pre-flight rig health check.** `check_rig_health()` runs before the build and +**never aborts**. It probes `/proc` unprivileged (dmesg is restricted on the rig) for a +wedged `usb_hub_wq` worker, and reports a `/proc` too restricted to trust as its own +distinct cause rather than as a diagnosed fault. + +It is deliberately non-fatal: the rig is unattended and every remedy for a real wedge is +manual, so aborting would not fix anything -- it would discard the per-board results the +run can still collect and leave CI red until a human noticed. It emits a GitHub +`::error::` annotation and continues. The automatic containment is 1a and 1b, which bound +a stuck run and explain it without anyone touching the rig. + +**1d. Order the recovery correctly.** In `test/hil/usbtest.py`, attempt +`usb_recover.sh root-cycle` FIRST on a `HUNG` case, and only check for a wedged hub worker +*afterwards*. + +> **Corrected during implementation.** This section originally said to check for a wedged +> worker *before* the cycle and skip it on a hit. That is backwards. Our own stuck +> `testusb` holds the DUT's device lock, so any port event drives a hub worker into +> `usb_lock_device()` on it -- uninterruptible, so it reads `D` in ~100% of samples and the +> confirmation window makes the wrong verdict *more* confident, not less. Cutting VBUS is +> precisely what completes the in-flight URB, returns the ioctl and frees that worker, so +> gating on that signature would suppress the recovery in the exact ordering it exists for. +> A worker still wedged after the cycle is the genuinely unrecoverable case, and that is +> what the code now reports. + +## Verification + +- Unit-test `shutdown_pool` and the `hil_health` detectors against a synthetic `/proc`. + A real wedge cannot be manufactured on demand, so they are tested against fabricated + inputs rather than live hardware. +- Confirm the detectors flag a genuinely wedged rig, and return clean on a healthy one. +- One clean full-fleet `hil_test.py` run to prove `check_rig_health` does not + false-abort. + +## Out of scope + +- **`ra6m5_ek` park and its dfu reset loop.** Dropped by decision. Consequence: the + layer-2 devnum storm remains as standing pressure on controller `03:00.0`. Unplugging + the board or flashing `board_test` by hand resolves it without any code change. +- **An unattended PVE watchdog** that detects the wedge and power-cycles the host. + Declined: more moving parts, and it can cut a running CI job. 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. |
