From cdd120fa7693c609f4c92d21760a7205571e3b5c Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:04:04 +0300 Subject: [PATCH 1/3] cca gates: make the Jaguar1 arm of the regcheck actually runnable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The harness shipped in #427 could not check Jaguar1 at all. Two independent reasons, both of which made the tracker cell report a clean-looking non-result rather than fail. The probe builds a default DeviceConfig, so tuning.phydm_watchdog is false. On Jaguar1 that watchdog IS the EDCCA tracker — HalModule constructs it only under that flag — so SetEdccaTrack was never reached and the cell reported "no EDCCA tracker running in the default arm" on a backend whose tracking path had simply never been instantiated. A --phydm-watchdog flag builds it; Jaguar3 ignores the field, its phydm runtime rides the RX/coex thread. Separately, peek32/poke32 addressed chipstate by --pid alone while the probe got --vid. Any adapter enumerating under a vendor VID rather than 0x0bda — TP-Link 0x2357, and most retail parts — failed every register cell, because the peek was looking for it under the default Realtek VID. Six failed cells on the first Jaguar1 run here, none of them about the gates. With both fixed, an RTL8821AU (Jaguar1, chip-id 0x05, 1T1R) reports 11 passed / 0 failed / 0 skipped, and the tracker cell gives a real verdict instead of a skip: note: 0x0120 track: tracker IS running at 0x8a4 in the default arm PASS: 0x0120 track: EDCCA tracking stops when EDCCA is the gate turned off note: 0x0120 cntdown: 0x524[11] constant at 1 — not an EDCCA gate here That last one is the documented known-limit path behaving correctly: Jaguar1 never writes 0x524, so the cell reports rather than fails. This also retires the "not hardware-validated" caveat on the tracker-ordering fix from #427. The race reproduces on this part once the window is widened and the disable lands while DIG is still walking IGI: pre-fix parked 0x8a4=0x7f7f, tracker still ON, window open PhydmWatchdog: EDCCA L2H/H2L re-tracked to 3/-4 (igi=0x1e) window closed, about to clear the tracker fixed 0 overwrites inside the same window, across three disable arms Worth recording that the tracker is write-on-change, so the race only fires while DIG is actively moving IGI — once it converges the pre-fix ordering looks correct. A first negative control missed the bug for exactly that reason. Verified: build clean, ctest 63/63, regcheck 11/0/0 on the RTL8821AU and no regression on an 8822E, 8822C, 8822BU or RTL8733BU. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0192ViRjaiFT9vTiiJpohZGT --- tests/cca_gates_probe.cpp | 21 ++++++++++++++++++--- tests/cca_gates_regcheck.sh | 23 ++++++++++++++++++----- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/tests/cca_gates_probe.cpp b/tests/cca_gates_probe.cpp index 495d021..52f9b44 100644 --- a/tests/cca_gates_probe.cpp +++ b/tests/cca_gates_probe.cpp @@ -11,6 +11,11 @@ * * sudo build/CcaGatesProbe --pid 0xc812 --channel 36 * sudo build/CcaGatesProbe --pid 0x8812 --channel 36 --hold 12 + * sudo build/CcaGatesProbe --pid 0x0120 --vid 0x2357 --phydm-watchdog + * + * --phydm-watchdog builds Jaguar1's optional phydm thread. Its EDCCA + * tracker is what SetCcaGates has to stop, and it does not exist without + * it, so the tracker cell needs this on Jaguar1 (Jaguar3 ignores it). * * --hold N keeps each state applied for N seconds so an external peek can * sample it. Exit 0 = every step behaved; 4 = not a Realtek radio; 5 = the @@ -56,7 +61,7 @@ void report(const char *tag, bool ret, bool primary, bool edcca) { int main(int argc, char **argv) { uint16_t vid = 0x0bda, pid = 0xc812; - int channel = 36, retune = 0, fast_retune = 0, hold = 0; + int channel = 36, retune = 0, fast_retune = 0, hold = 0, phydm_wd = 0; for (int i = 1; i < argc; i++) { if (!std::strcmp(argv[i], "--vid") && i + 1 < argc) vid = (uint16_t)std::strtoul(argv[++i], nullptr, 0); @@ -70,6 +75,8 @@ int main(int argc, char **argv) { fast_retune = std::atoi(argv[++i]); else if (!std::strcmp(argv[i], "--hold") && i + 1 < argc) hold = std::atoi(argv[++i]); + else if (!std::strcmp(argv[i], "--phydm-watchdog")) + phydm_wd = 1; } auto logger = std::make_shared(); @@ -95,6 +102,13 @@ int main(int argc, char **argv) { session.adopt_lock(lock); devourer::DeviceConfig cfg; + /* Jaguar1's EDCCA tracker only EXISTS when the phydm watchdog is built — + * HalModule constructs it solely under tuning.phydm_watchdog, which is off + * by default. Without this the tracker cell has nothing to catch tracking, + * and reports "no tracker running" on a backend whose tracking path is + * simply not instantiated. Jaguar3 ignores the field (its phydm runtime + * rides the RX/coex thread), so passing it there costs nothing. */ + cfg.tuning.phydm_watchdog = phydm_wd != 0; WiFiDriver driver(logger); std::unique_ptr owned = driver.CreateRadio(handle, ctx, lock, cfg); if (!owned) { @@ -114,8 +128,9 @@ int main(int argc, char **argv) { * pokes the wrong one reports "no tracker running" instead of failing — * a false negative on exactly the arm the split exists to serve. Caps are * resolved at construction, so this is readable before bring-up. */ - std::printf("GATES-GEN %s\n", - devourer::generation_name(dev->GetAdapterCaps().generation)); + std::printf("GATES-GEN %s phydm_watchdog=%d\n", + devourer::generation_name(dev->GetAdapterCaps().generation), + phydm_wd); std::fflush(stdout); /* Pre-bring-up: both calls must refuse, and the refusal must not write the diff --git a/tests/cca_gates_regcheck.sh b/tests/cca_gates_regcheck.sh index 4928355..16bc86a 100755 --- a/tests/cca_gates_regcheck.sh +++ b/tests/cca_gates_regcheck.sh @@ -41,7 +41,10 @@ # poking the other family's register reports "no tracker" and # passes a broken tracker silently. Skipped where no tracker runs # in the default arm, or where the generation has no known -# threshold register. +# threshold register. Runs the probe with --phydm-watchdog: +# Jaguar1's tracker IS the optional phydm thread, off by default, +# so without it this cell measures a backend whose tracking path +# was never built and calls that "no tracker". # retune the state survives SetMonitorChannel and FastRetune, within a # band and across a band change. Jaguar3 re-asserts by design; # Jaguar1 merely is not clobbered (see src/IRtlRadio.h) — so this @@ -51,6 +54,12 @@ # # Usage: sudo -v && tests/cca_gates_regcheck.sh # every plugged part # PIDS=0xc812 sudo -v && tests/cca_gates_regcheck.sh +# VID=0x2357 PIDS=0x0120 tests/cca_gates_regcheck.sh # non-Realtek VID +# +# VID applies to the register peeks as well as the probe, so an adapter that +# enumerates under a vendor's own VID (TP-Link 0x2357, and most retail parts) +# is checkable — addressing chipstate by PID alone silently looked for it +# under 0x0bda and failed every register cell. set -u ROOT="$(cd "$(dirname "$0")/.." && pwd)" OUT="${CCA_GATES_OUT:-/tmp/devourer-cca-gates}" @@ -100,7 +109,7 @@ done # dead probe, i.e. the default arm would pass on no evidence. peek32() { # $1=pid $2=addr local bytes n - bytes=$(sudo -n "$BUILD/chipstate" --pid "$1" --no-claim \ + bytes=$(sudo -n "$BUILD/chipstate" --vid "$VID" --pid "$1" --no-claim \ --peek "$(printf '0x%x-0x%x' "$2" $(( $2 + 3 )))" 2>&1 | sed -n 's/^0x[0-9a-fA-F]\{4\}://p' | tr -s ' ' '\n' | grep -E '^[0-9a-f]{2}$' | head -4) @@ -109,7 +118,7 @@ peek32() { # $1=pid $2=addr printf '%s\n' "$bytes" | awk '{b[NR]=strtonum("0x"$1)} END{printf "%u\n", b[1]+b[2]*256+b[3]*65536+b[4]*16777216}' } -poke32() { sudo -n "$BUILD/chipstate" --pid "$1" --no-claim \ +poke32() { sudo -n "$BUILD/chipstate" --vid "$VID" --pid "$1" --no-claim \ --poke "$(printf '0x%x=0x%x:4' "$2" "$3")" >/dev/null 2>&1; } bit() { echo $(( ( $1 >> $2 ) & 1 )); } @@ -185,7 +194,7 @@ for pid in $PIDS; do [ $rc -eq 0 ] && pass "$pid api: probe walk clean" \ || fail "$pid api: probe reported failures (see $log)" - gen=$(sed -n 's/^GATES-GEN //p' "$log" | head -1) + gen=$(sed -n 's/^GATES-GEN //p' "$log" | head -1 | cut -d' ' -f1) note "$pid generation: ${gen:-unknown}" # --- regs + cntdown --------------------------------------------------- @@ -282,7 +291,11 @@ for pid in $PIDS; do for arm in "0 0" "0 1"; do set -- $arm; want_p=$1; want_e=$2 tlog="$OUT/track-$pid-$want_p$want_e.log" - start_hold "$pid" "$tlog" + # --phydm-watchdog because Jaguar1's EDCCA tracker is only built when + # tuning.phydm_watchdog is set; without it this cell measures a + # backend whose tracking path was never instantiated and calls that + # "no tracker". Jaguar3 ignores the flag. + start_hold "$pid" "$tlog" --phydm-watchdog if ! wait_marker "$tlog" "^GATES set-primary$want_p-edcca$want_e "; then fail "$pid track: probe never reported primary=$want_p edcca=$want_e" stop_hold; continue From 3407a056241eba993a361acd0df79872b0ffdde5 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:14:52 +0300 Subject: [PATCH 2/3] cca gates: document what the tracker cell depends on, and what the flag is not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things a reader of this harness has to know and could not have learned from it. The tracker is write-on-change (l2h != _edcca_last_l2h), so the cell only sees it act while DIG is still walking IGI. Each arm restarts the probe, so DIG restarts with it and is in motion during the sample window — but if it has converged instead, the default arm reads as "no tracker" and the EDCCA-off arm skips. That degrades to no verdict rather than a false one, which is the right failure, but a SKIP there means the cell could not create the condition, not that the tracker behaved. This is the same property that made a first negative control miss the race entirely. And --phydm-watchdog is a test lever, not a hint at a better default. That thread is opt-in because its periodic BB traffic shares the libusb queue with the TX bulk path and costs throughput — 4500 to 1000 TX submits in 10s on an 8821 at ch100, per HalModule. This probe never transmits, so it pays none of it, which is why the cell can afford the flag and a normal session cannot. Comments only. Build clean, ctest 63/63, and an untouched end-to-end run of the committed script reports 13 passed / 0 failed / 0 skipped over an 8822C and an RTL8733BU. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0192ViRjaiFT9vTiiJpohZGT --- tests/cca_gates_probe.cpp | 4 ++++ tests/cca_gates_regcheck.sh | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/tests/cca_gates_probe.cpp b/tests/cca_gates_probe.cpp index 52f9b44..94167b3 100644 --- a/tests/cca_gates_probe.cpp +++ b/tests/cca_gates_probe.cpp @@ -16,6 +16,10 @@ * --phydm-watchdog builds Jaguar1's optional phydm thread. Its EDCCA * tracker is what SetCcaGates has to stop, and it does not exist without * it, so the tracker cell needs this on Jaguar1 (Jaguar3 ignores it). + * A test lever, not a hint at a better default: that thread is opt-in + * because its periodic BB traffic shares the libusb queue with the TX bulk + * path and costs throughput (see HalModule). This probe never transmits, + * so it pays none of that. * * --hold N keeps each state applied for N seconds so an external peek can * sample it. Exit 0 = every step behaved; 4 = not a Realtek radio; 5 = the diff --git a/tests/cca_gates_regcheck.sh b/tests/cca_gates_regcheck.sh index 16bc86a..61ccaeb 100755 --- a/tests/cca_gates_regcheck.sh +++ b/tests/cca_gates_regcheck.sh @@ -45,6 +45,14 @@ # Jaguar1's tracker IS the optional phydm thread, off by default, # so without it this cell measures a backend whose tracking path # was never built and calls that "no tracker". +# DEPENDS ON DIG BEING IN MOTION. The tracker is write-on-change +# (l2h != _edcca_last_l2h), so it only rewrites the marker while +# DIG is still walking IGI. Each arm restarts the probe, so DIG +# restarts with it and is walking during the sample window; if it +# has converged instead, the default arm reads as "no tracker" +# and the EDCCA-off arm SKIPs. That degrades to no verdict rather +# than a false one, but a SKIP here means the cell could not +# create the condition, not that the tracker behaved. # retune the state survives SetMonitorChannel and FastRetune, within a # band and across a band change. Jaguar3 re-asserts by design; # Jaguar1 merely is not clobbered (see src/IRtlRadio.h) — so this From a21615c0b0429a67e387044aceabe70a3c3d8742 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:17:31 +0300 Subject: [PATCH 3/3] jaguar1: stop the phydm watchdog before tearing the chip down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rtw_hal_deinit() clears REG_CR, zeroes REG_RCR and runs the card-disable power sequence with the phydm watchdog thread still ticking. That thread does periodic BB reads and writes — FA counters, DIG on 0xc50/0xe50, the EDCCA thresholds — so teardown races register access against power-off, which is the one ordering hazard this driver is careful about everywhere else. Stop() first. It compare-exchanges _running, so it is idempotent and the destructor's own Stop() after this is a no-op; the pointer is null whenever tuning.phydm_watchdog did not build one, which is the default. Pre-existing, but only reachable once something sets that config, and until now nothing in-tree did — the --phydm-watchdog probe flag in this branch is what makes it reachable, so it is fixed here rather than left for whoever next turns the watchdog on. Measured on an RTL8821AU, marking every tick and widening the de-init window to 6 s so the 2 s tick has to land in it: pre-fix clean de-init (stop TRX + card-disable) TICKMARK: watchdog touching the chip TICKMARK: watchdog touching the chip PowerOff: card-disable applied => 2 ticks inside the window fixed clean de-init (stop TRX + card-disable) PowerOff: card-disable applied => 0 ticks inside the window Found by Qodo on #429. Build clean, ctest 63/63. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0192ViRjaiFT9vTiiJpohZGT --- src/jaguar1/HalModule.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/jaguar1/HalModule.cpp b/src/jaguar1/HalModule.cpp index 8386dfc..f96d845 100644 --- a/src/jaguar1/HalModule.cpp +++ b/src/jaguar1/HalModule.cpp @@ -662,6 +662,18 @@ bool HalModule::rtl8812au_hal_init(uint8_t init_channel) { void HalModule::rtw_hal_deinit() { _logger->info("Jaguar1: clean de-init (stop TRX + card-disable)"); + /* Stop the phydm watchdog FIRST, before any of the writes below. Its + * thread does periodic BB reads/writes (FA counters, DIG on 0xc50/0xe50, + * the EDCCA thresholds), and everything after this point is tearing the + * chip down underneath it — MAC engines off, RX FIFO off, then the + * card-disable power sequence. Left running it races register access + * against power-off, which is the teardown-ordering hazard this driver + * takes seriously everywhere else. Stop() is idempotent (it + * compare-exchanges _running), so the destructor's own Stop() after this + * is a no-op, and a chip torn down twice is still safe. Null whenever + * tuning.phydm_watchdog did not build one, which is the default. */ + if (_phydmWatchdog) + _phydmWatchdog->Stop(); /* Halt the MAC engines before pulling power out from under them, so the * sequence isn't racing DMA that is still moving frames. Mirrors * HalJaguar3::rtw_hal_deinit. */