From fd48a60effac041fbb80238963058f3b53219879 Mon Sep 17 00:00:00 2001 From: snokvist Date: Sat, 12 Sep 2026 18:44:07 +0200 Subject: [PATCH 1/4] IRtlRadio: address the two carrier-sense gates separately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SetCcaMode is all-or-nothing, and on these families it is two gates doing different jobs: 0x520[14] primary CCA defers to a decodable preamble, 0x520[15] EDCCA defers to raw in-band energy. This adds SetCcaGates / GetCcaGates so a caller can address them one at a time, with not-ported defaults and SetCcaMode reduced to SetCcaGates(d, d). Why it is worth splitting: they do not behave the same way, and on Jaguar1 they behave OPPOSITELY to what tests/dis_cca_tx_onair.sh found. That test is Jaguar3 — it uses the 8812AU only as its flooder and has never run a Jaguar1 as the DUT. Running one (RTL8812AU injecting 300 broadcast frames at 6M on an otherwise idle ch6, fresh radio open per arm so nothing latches, two independent receivers, repeated): both gates on (the default) 0.0% 1.7% EDCCA off only 94.3% 94.7% primary CCA off only 13.7% 2.7% both off 94.3% 95.3% On Jaguar1 the energy bit is everything and the preamble bit is nearly null. Without the split there is no way to find that out, and no way to act on it except dis_cca, which turns off both. Acting on it matters, because leaving primary CCA on is strictly better than dis_cca. Same DUT against a saturating co-channel flooder: EDCCA off, primary CCA ON, no flooder 95.3% EDCCA off, primary CCA ON, flooding 78.0% both gates off, flooding 0.3% Carrier sense still defers, and both-off is worse for the injector's own delivery because it transmits into the flood and collides. Behaviour is unchanged for every existing caller. SetCcaMode's two states write exactly the bytes they wrote before: on Jaguar1 apply_cca(d, d) is the old body with the BB threshold half keyed on the EDCCA argument, and on Jaguar3 0x524[11] moves with the pair and only with the pair — what that bit does on its own is undocumented and unmeasured, so a mixed state leaves it at the enabled value rather than guessing. Jaguar3 tracks the two gates so SetMonitorChannel re-asserts what the caller asked for rather than only the all-or-nothing pair. Verified on hardware: RTL8812AU (Jaguar1) and RTL8822C (Jaguar3, an 8812CU). All four states read back from 0x520 on both, and the Jaguar3 state survives a retune. Defaults checked against a build without this change, alternating builds on the same bench: ambient RX rates, bridge drop counts and default-path TX delivery all inside the baseline-to-baseline spread, with the Jaguar3 injector at 100% on both. Not verified here: Jaguar2, Kestrel and RTL8733B have no hardware on this bench and are left on the not-ported default, as is MediaTek, which is not an IRtlRadio at all. --- src/IRtlRadio.h | 29 ++++++++++++++++++ src/jaguar1/RtlJaguarDevice.cpp | 35 +++++++++++++++++----- src/jaguar1/RtlJaguarDevice.h | 8 +++++ src/jaguar3/RtlJaguar3Device.cpp | 50 ++++++++++++++++++++++++++------ src/jaguar3/RtlJaguar3Device.h | 9 ++++++ 5 files changed, 114 insertions(+), 17 deletions(-) diff --git a/src/IRtlRadio.h b/src/IRtlRadio.h index a3f07943..11a9daca 100644 --- a/src/IRtlRadio.h +++ b/src/IRtlRadio.h @@ -85,6 +85,35 @@ class IRtlRadio : public IRadio { * tests/canary_diff.py. Reading a powered-down chip yields garbage or throws; * interpreting that is the caller's job. No-op where unsupported (default). */ virtual void DumpChipState() {} + + /* The MAC carrier-sense gate, one bit at a time. + * + * SetCcaMode is all-or-nothing, and on this family it is two gates: + * 0x520[14] primary CCA (defers to a decodable preamble) and 0x520[15] + * EDCCA (defers to raw in-band energy). They answer different questions + * and they do not behave the same way — tests/dis_cca_tx_onair.sh measured + * primary CCA costing a Jaguar3 injector 41-45% against a co-channel + * flooder while the energy bit alone was null, and on Jaguar1 the result + * inverts (see below). A caller that needs one of them should not have to + * turn off both, and a caller diagnosing a deferral needs to tell them + * apart. + * + * `true` means DISABLED, matching SetCcaMode's argument sense and the + * register's own polarity (bit set = gate off). SetCcaMode is exactly + * SetCcaGates(d, d) and writes the same bytes it always did. Returns false + * where the split is not ported; SetCcaMode remains the portable call. */ + virtual bool SetCcaGates(bool primary_disabled, bool edcca_disabled) { + (void)primary_disabled; + (void)edcca_disabled; + return false; + } + + /* Current gate state, read back from the hardware rather than remembered. */ + virtual bool GetCcaGates(bool &primary_disabled, bool &edcca_disabled) { + (void)primary_disabled; + (void)edcca_disabled; + return false; + } }; #endif /* IRTL_RADIO_H */ diff --git a/src/jaguar1/RtlJaguarDevice.cpp b/src/jaguar1/RtlJaguarDevice.cpp index 880f174c..a9e29a0b 100644 --- a/src/jaguar1/RtlJaguarDevice.cpp +++ b/src/jaguar1/RtlJaguarDevice.cpp @@ -980,16 +980,38 @@ void RtlJaguarDevice::ClearAckResponder() { (void)disarm_ack_responder(); } +bool RtlJaguarDevice::GetCcaGates(bool &primary_disabled, bool &edcca_disabled) { + const uint32_t v = _device.rtw_read(0x0520); + primary_disabled = (v & (1u << 14)) != 0; + edcca_disabled = (v & (1u << 15)) != 0; + return true; +} + +bool RtlJaguarDevice::SetCcaGates(bool primary_disabled, bool edcca_disabled) { + apply_cca(primary_disabled, edcca_disabled); + _logger->info("Jaguar1: CCA gates primary={} edcca={}", + primary_disabled ? "OFF" : "on", + edcca_disabled ? "OFF" : "on"); + return true; +} + void RtlJaguarDevice::SetCcaMode(bool disabled) { + apply_cca(disabled, disabled); + _logger->info("Jaguar1: MAC carrier-sense {}", + disabled ? "DISABLED (dis_cca: CCA+EDCCA)" + : "enabled (default)"); +} + +void RtlJaguarDevice::apply_cca(bool primary_disabled, bool edcca_disabled) { /* MAC carrier-sense gate: the same REG_TX_PTCL_CTRL bits as the HalMAC * generations — the vendor's phydm_mac_edcca_state drives 0x520[15] on - * this family too; [14] is the primary-CCA defer. */ + * this family too; [14] is the primary-CCA defer. A set bit disables. */ uint32_t v520 = _device.rtw_read(0x0520); - if (disabled) - v520 |= (1u << 15) | (1u << 14); - else - v520 &= ~((1u << 15) | (1u << 14)); + if (primary_disabled) v520 |= (1u << 14); else v520 &= ~(1u << 14); + if (edcca_disabled) v520 |= (1u << 15); else v520 &= ~(1u << 15); _device.rtw_write(0x0520, v520); + /* The BB threshold work below belongs to the EDCCA gate alone. */ + const bool disabled = edcca_disabled; /* BB EDCCA thresholds (rEDCCA_Jaguar 0x8a4: L2H byte0 / H2L byte1). The * BB init table parks them at 0x7f/0x7f = never-trigger — the vendor's @@ -1020,9 +1042,6 @@ void RtlJaguarDevice::SetCcaMode(bool disabled) { * threshold follows (vendor couples them per adaptivity cycle). */ if (auto *wd = _halModule.phydm_watchdog()) wd->SetEdccaTrack(!disabled); - _logger->info("Jaguar1: MAC carrier-sense {}", - disabled ? "DISABLED (dis_cca: CCA+EDCCA)" - : "enabled (default)"); } bool RtlJaguarDevice::SetAmpduMode(const devourer::AmpduMode &mode) { diff --git a/src/jaguar1/RtlJaguarDevice.h b/src/jaguar1/RtlJaguarDevice.h index bd1de233..6b451273 100644 --- a/src/jaguar1/RtlJaguarDevice.h +++ b/src/jaguar1/RtlJaguarDevice.h @@ -303,6 +303,9 @@ class RtlJaguarDevice : public IRtlRadio { * parked at never-trigger by the BB table, programmed to the vendor * operating point on enable (EDCCA only exists once they are set). */ void SetCcaMode(bool disabled) override; + /* The two gates independently — see IRtlRadio. */ + bool SetCcaGates(bool primary_disabled, bool edcca_disabled) override; + bool GetCcaGates(bool &primary_disabled, bool &edcca_disabled) override; /* A-MPDU TX mode (IRadio contract; src/AmpduMode.h). Programs the * Jaguar1 aggregate-fill timer (0x0456 — NOT the 0x0455 the HalMAC chips * use) + the 8814A burst-mode gate (0x04BC), and records the descriptor @@ -425,6 +428,11 @@ class RtlJaguarDevice : public IRtlRadio { bool la_capture_wedged() const { return _la && _la->is_wedged(); } private: + /* Programs 0x520[14]/[15] and, for the EDCCA gate only, the BB thresholds + * at 0x8a4. SetCcaMode is apply_cca(d, d) and writes exactly what it + * wrote before the split existed. */ + void apply_cca(bool primary_disabled, bool edcca_disabled); + void StartWithMonitorMode(SelectedChannel selectedChannel); bool NetDevOpen(SelectedChannel selectedChannel); diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 8cc33963..543a77a9 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -1182,22 +1182,54 @@ RxEnergy RtlJaguar3Device::GetRxEnergy(bool with_nhm) { * MEASURED: the full recipe dropped 8822EU delivery from ~6800 to ~10 frames. * These MAC 0x520/0x524 bits gate TX only and are safe on a live RX. */ void RtlJaguar3Device::apply_cca_mode_locked(bool disabled) { + apply_cca_gates_locked(disabled, disabled); +} + +void RtlJaguar3Device::apply_cca_gates_locked(bool primary_disabled, + bool edcca_disabled) { uint32_t v520 = _device.rtw_read(0x0520); uint32_t v524 = _device.rtw_read(0x0524); - if (disabled) { - v520 |= (1u << 15) | (1u << 14); /* DIS_EDCCA (energy) + DIS_CCA (carrier-sense) */ - v524 &= ~(1u << 11); - } else { - v520 &= ~((1u << 15) | (1u << 14)); - v524 |= (1u << 11); - } + /* DIS_EDCCA (energy) + DIS_CCA (carrier-sense); a set bit disables. */ + if (primary_disabled) v520 |= (1u << 14); else v520 &= ~(1u << 14); + if (edcca_disabled) v520 |= (1u << 15); else v520 &= ~(1u << 15); + /* 0x524[11] moves with the pair and only with the pair. Deliberate: the + * two pure states then write exactly the bytes the all-or-nothing path + * wrote before this split, so SetCcaMode is byte-identical. What this bit + * means on its own is not documented here and was not measured, so a + * mixed state leaves it at the enabled value rather than guessing. */ + if (primary_disabled && edcca_disabled) v524 &= ~(1u << 11); + else v524 |= (1u << 11); _device.rtw_write(0x0520, v520); _device.rtw_write(0x0524, v524); } +bool RtlJaguar3Device::GetCcaGates(bool &primary_disabled, bool &edcca_disabled) { + std::lock_guard lk(_reg_mu); + const uint32_t v = _device.rtw_read(0x0520); + primary_disabled = (v & (1u << 14)) != 0; + edcca_disabled = (v & (1u << 15)) != 0; + return true; +} + +bool RtlJaguar3Device::SetCcaGates(bool primary_disabled, bool edcca_disabled) { + std::lock_guard lk(_reg_mu); + /* Sticky the same way dis_cca is: a channel set rewrites the BB CCA + * registers and SetMonitorChannel re-asserts from these. */ + _cca_disabled = primary_disabled && edcca_disabled; + _cca_primary_disabled = primary_disabled; + _cca_edcca_disabled = edcca_disabled; + if (_brought_up) + apply_cca_gates_locked(primary_disabled, edcca_disabled); + _logger->info("Jaguar3: CCA gates primary={} edcca={}", + primary_disabled ? "OFF" : "on", edcca_disabled ? "OFF" : "on"); + return true; +} + void RtlJaguar3Device::SetCcaMode(bool disabled) { std::lock_guard lk(_reg_mu); _cca_disabled = disabled; + _cca_primary_disabled = disabled; + _cca_edcca_disabled = disabled; if (_brought_up) apply_cca_mode_locked(disabled); _logger->info("Jaguar3: MAC carrier-sense {}", @@ -1224,8 +1256,8 @@ void RtlJaguar3Device::SetMonitorChannel(SelectedChannel channel) { apply_tx_power_current(/*full=*/true); /* dis_cca is sticky — the channel set rewrote the BB CCA registers, so * re-assert the disable if it was armed. */ - if (_brought_up && _cca_disabled) - apply_cca_mode_locked(true); + if (_brought_up && (_cca_primary_disabled || _cca_edcca_disabled)) + apply_cca_gates_locked(_cca_primary_disabled, _cca_edcca_disabled); /* Per-packet power banks are sticky too (the lever contract): the channel * set doesn't touch 0x1e70[31:16] today, but a cheap RMW re-assert keeps * the contract robust against future channel-path changes. */ diff --git a/src/jaguar3/RtlJaguar3Device.h b/src/jaguar3/RtlJaguar3Device.h index ced9c7d8..a904e52b 100644 --- a/src/jaguar3/RtlJaguar3Device.h +++ b/src/jaguar3/RtlJaguar3Device.h @@ -200,6 +200,9 @@ class RtlJaguar3Device : public IRtlRadio { * downlink residual from ~472 µs to 0.39 µs on a crowded channel (the TBTT * beacon airs on schedule instead of after a CSMA backoff). */ void SetCcaMode(bool disabled) override; + /* The two gates independently — see IRtlRadio. */ + bool SetCcaGates(bool primary_disabled, bool edcca_disabled) override; + bool GetCcaGates(bool &primary_disabled, bool &edcca_disabled) override; /* Adapter-health probes (see src/AdapterHealth.h). EFUSE probe is 8822C * only — the 8822E's OTP is not reliably readable post-bring-up by design @@ -304,7 +307,13 @@ class RtlJaguar3Device : public IRtlRadio { /* dis_cca sticky state — re-applied after SetMonitorChannel (the channel set * rewrites the BB CCA registers). Caller holds _reg_mu. */ bool _cca_disabled = false; + /* The two gates, tracked separately so a channel set re-asserts exactly + * what the caller asked for. Both false is the default, which is what + * _cca_disabled == false always meant. */ + bool _cca_primary_disabled = false; + bool _cca_edcca_disabled = false; void apply_cca_mode_locked(bool disabled); + void apply_cca_gates_locked(bool primary_disabled, bool edcca_disabled); /* TX+RX intent (DEVOURER_TX_WITH_RX at InitWrite / an RX-side Init): * keeps the RX filters open across the TX bring-up. */ bool _rx_wanted = false; From b28158766b301ed3e68918bf37b7a159b78536f7 Mon Sep 17 00:00:00 2001 From: snokvist Date: Sat, 12 Sep 2026 18:51:53 +0200 Subject: [PATCH 2/4] jaguar1: refuse the gate ops before bring-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading 0x520 on a chip that was never brought up returns whatever the bus gives back, and reporting that as the gate state is a fabricated measurement; writing it pokes an uninitialised MAC. Jaguar3's SetCcaGates already guards on _brought_up — this is the Jaguar1 equivalent, on both the read and the write side. Found by review, then confirmed against an RTL8812AU: opened but not tuned, the op now refuses instead of answering. --- src/jaguar1/RtlJaguarDevice.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/jaguar1/RtlJaguarDevice.cpp b/src/jaguar1/RtlJaguarDevice.cpp index a9e29a0b..985d115e 100644 --- a/src/jaguar1/RtlJaguarDevice.cpp +++ b/src/jaguar1/RtlJaguarDevice.cpp @@ -981,6 +981,12 @@ void RtlJaguarDevice::ClearAckResponder() { } bool RtlJaguarDevice::GetCcaGates(bool &primary_disabled, bool &edcca_disabled) { + /* The MAC register is meaningless before bring-up, and handing back + * whatever the bus returns would be a fabricated gate state. Jaguar3's + * SetCcaGates already guards on _brought_up; this is the read side and + * the Jaguar1 equivalent. */ + if (!_brought_up) + return false; const uint32_t v = _device.rtw_read(0x0520); primary_disabled = (v & (1u << 14)) != 0; edcca_disabled = (v & (1u << 15)) != 0; @@ -988,6 +994,8 @@ bool RtlJaguarDevice::GetCcaGates(bool &primary_disabled, bool &edcca_disabled) } bool RtlJaguarDevice::SetCcaGates(bool primary_disabled, bool edcca_disabled) { + if (!_brought_up) + return false; apply_cca(primary_disabled, edcca_disabled); _logger->info("Jaguar1: CCA gates primary={} edcca={}", primary_disabled ? "OFF" : "on", From 68cb3f8bbdba6fa44180841d2a5f2345be4a6d0f Mon Sep 17 00:00:00 2001 From: snokvist Date: Sat, 12 Sep 2026 23:07:10 +0200 Subject: [PATCH 3/4] Scope 0x524[11] and phydm EDCCA tracking to the EDCCA gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Jaguar3 defects from review, both invisible on a Jaguar1 bench because every on-air number in the original description came from a Jaguar1 DUT. 0x524[11] is BIT_EDCCA_MSK_CNTDOWN_EN (REG_RD_CTRL), the same name and bit on 8822B, 8822C and 8822E in the vendor HALMAC headers, so the meaning is family-stable. Being EDCCA-scoped it follows edcca_disabled alone. Moving it with the pair — on the reasoning that an unmeasured bit should not be guessed at — was the wrong kind of caution, and it left the EDCCA-off arm this work recommends with EDCCA still masking the backoff countdown. Measured on an 8812CU, exactly one of the six states changes, and SetCcaMode's two states write what they always wrote. phydm's edcca_track was keyed on the all-or-nothing flag, so with EDCCA off and primary CCA on the ~2 s tick went on running PhydmRuntimeJaguar3::edcca() and rewriting the BB thresholds at 0x84c. It now follows the EDCCA gate, the way Jaguar1 already did via SetEdccaTrack at the end of apply_cca. Sampling 0x84c cannot detect this — the tracker recomputes the same th_l2h from a static IGI, so an active tracker writes identical bytes — so it is measured by poking the register with a value it would never choose and seeing whether it is restored. Jaguar3's GetCcaGates and SetCcaGates now refuse before bring-up, caching nothing, as Jaguar1 already did. Reading 0x520 on an unconfigured MAC returned a plausible-looking state that depended on whatever the previous session left behind, and the setter reported success for a write that nothing replayed. _cca_disabled is removed: once both readers moved to the per-gate flags it was write-only, and a write-only field invites a future reader to assume it means something. The contract moves to the declaration in IRtlRadio.h, since FastRetune and SetMonitorChannel are the normal operating pattern for this lever: both calls are post-bring-up only, `false` means either "not ported" or "not brought up", a refusal leaves GetCcaGates' out-parameters alone, and the state survives a retune on both families — but by different mechanisms, so the Jaguar1 case is documented as incidental rather than guaranteed. That last point corrects the review: measured on an 8812AU, the gates and the BB thresholds are intact across a same-band retune, a band change, and FastRetune. Jaguar1 readability: the re-aliased `disabled` local is gone and the four uses name the gate, and the braceless same-line if/else pairs are braced. tests/cca_gates_probe.cpp is the in-tree caller the split lacked, and tests/cca_gates_regcheck.sh the register-level check, in the shape of the txpwr regchecks: the four gate states, the pre-bring-up refusal, both legacy SetCcaMode states, the 0x524[11] scoping, EDCCA-tracking shutdown, and retune survival by both channel paths — cross-checking the API's readback against a raw chipstate peek so an API that lies about the silicon fails rather than passes. 23 cells across an RTL8812AU, an RTL8822C and an RTL8733BU; reverting either fix above makes the matching cell fail. CLAUDE.md no longer states the Jaguar3 result as general. Both families are given as disagreeing measurements of different things, the flooder arm is paired in so the Jaguar1 figure cannot be read as "disable everything", and SetCcaGates is named in Configuration. The three roundings of one run are replaced by one citation of it. Verified on RTL8812AU (Jaguar1), RTL8822C (Jaguar3) and RTL8733BU (an IRtlRadio that ports neither, confirming the not-ported defaults refuse on real silicon). The 8822EU/8812EU arms of the original tables are unchanged and not re-run. --- CLAUDE.md | 28 ++- CMakeLists.txt | 9 + src/IRtlRadio.h | 58 ++++-- src/jaguar1/RtlJaguarDevice.cpp | 22 ++- src/jaguar3/RtlJaguar3Device.cpp | 49 +++-- src/jaguar3/RtlJaguar3Device.h | 12 +- tests/cca_gates_probe.cpp | 204 +++++++++++++++++++++ tests/cca_gates_regcheck.sh | 304 +++++++++++++++++++++++++++++++ tests/radio_iface_selftest.cpp | 37 ++++ 9 files changed, 674 insertions(+), 49 deletions(-) create mode 100644 tests/cca_gates_probe.cpp create mode 100755 tests/cca_gates_regcheck.sh diff --git a/CLAUDE.md b/CLAUDE.md index ddc3ab25..5f749e3d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -323,11 +323,29 @@ Behavioural traps the per-field docs can't carry: Jaguar1/2/3; on Jaguar1 the enable is real work — its BB table parks the EDCCA thresholds (`0x8a4`) at never-trigger, so bring-up programs the vendor adaptivity operating point (IGI-coupled; the phydm watchdog - re-tracks it when running). The primary-CCA bit is the one that matters: - monitor injection is not CCA-free, it defers ~40–60% to a co-channel - 802.11 transmitter, and clearing `[14]` recovers ~1.5–2.2× (on-air - 8822EU/8812CU, `tests/dis_cca_tx_onair.sh`); the energy bit `[15]` alone - is null against a decodable preamble. **On by default on the streamtx FPV + re-tracks it when running). **Which bit matters is family-specific and the + two measured families disagree — do not generalise either result.** On + Jaguar3, monitor injection defers to a co-channel 802.11 transmitter and + clearing `[14]` recovers it while the energy bit `[15]` alone is null + against a decodable preamble (on-air 8822EU/8812CU, + `tests/dis_cca_tx_onair.sh`, measuring the DUT's host-side `submitted` + rate). On Jaguar1 it inverts: with an 8812AU injecting on an idle channel + and two independent witnesses decoding, clearing `[15]` alone recovers + ~95% while clearing `[14]` alone recovers little, because what stops this + family is the EDCCA its own bring-up turned on. The two are not in + conflict — they measure different things on different silicon — but + neither is the general answer. + + Turning both gates off is WORSE than turning off the one that matters: + with EDCCA off and primary CCA left on, the same Jaguar1 injector delivers + 95% on an idle channel and still 78% under a co-channel flooder; with both + gates off it collapses to 0.3%, because it stops waiting for a gap and + collides instead. `SetCcaGates` (`IRtlRadio`, Jaguar1 and Jaguar3) is the + one-bit-at-a-time form for exactly this; `SetCcaMode` remains the portable + all-or-nothing call and is `SetCcaGates(d, d)`. Both gate calls are + post-bring-up only and return false before it — see `src/IRtlRadio.h` for + the contract, and `tests/cca_gates_regcheck.sh` to reproduce the tables. + **On by default on the streamtx FPV downlink** (the link owns the channel — CSMA backoff only stutters it); `DEVOURER_DIS_CCA=0` forces standard carrier-sense back. On Kestrel the 8852C runs the same enabled default (measured: full-rate TX, 2.4x flood diff --git a/CMakeLists.txt b/CMakeLists.txt index 77b83465..2737915d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -829,6 +829,15 @@ add_executable(chipstate target_include_directories(chipstate PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/examples/common) target_link_libraries(chipstate PUBLIC devourer PRIVATE PkgConfig::libusb) +# The in-tree caller for the carrier-sense gate split. Needs hardware, so it is +# a tool rather than an add_test — tests/cca_gates_regcheck.sh drives it and +# cross-checks the registers with chipstate. +add_executable(CcaGatesProbe + tests/cca_gates_probe.cpp +) +target_include_directories(CcaGatesProbe PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/examples/common) +target_link_libraries(CcaGatesProbe PUBLIC devourer PRIVATE PkgConfig::libusb) + # Headless regression guard for the binary-stdin framing shared by the two # stream demos above (examples/common/stream_stdin.h). No libusb, no radio — just the # set_stdin_binary() + read_exact() path, so a text-mode regression (e.g. the diff --git a/src/IRtlRadio.h b/src/IRtlRadio.h index 11a9daca..600913af 100644 --- a/src/IRtlRadio.h +++ b/src/IRtlRadio.h @@ -85,30 +85,62 @@ class IRtlRadio : public IRadio { * tests/canary_diff.py. Reading a powered-down chip yields garbage or throws; * interpreting that is the caller's job. No-op where unsupported (default). */ virtual void DumpChipState() {} - /* The MAC carrier-sense gate, one bit at a time. * - * SetCcaMode is all-or-nothing, and on this family it is two gates: - * 0x520[14] primary CCA (defers to a decodable preamble) and 0x520[15] - * EDCCA (defers to raw in-band energy). They answer different questions - * and they do not behave the same way — tests/dis_cca_tx_onair.sh measured - * primary CCA costing a Jaguar3 injector 41-45% against a co-channel - * flooder while the energy bit alone was null, and on Jaguar1 the result - * inverts (see below). A caller that needs one of them should not have to - * turn off both, and a caller diagnosing a deferral needs to tell them - * apart. + * SetCcaMode is all-or-nothing, and on Jaguar1 and Jaguar3 it is two + * gates: 0x520[14] primary CCA (defers to a decodable preamble) and + * 0x520[15] EDCCA (defers to raw in-band energy). They answer different + * questions, and the two families measured so far DISAGREE about which one + * stops an injector — so a caller diagnosing a deferral has to tell them + * apart, and one that needs a single gate should not have to turn off + * both. CLAUDE.md summarises the on-air delivery figures and + * tests/dis_cca_tx_onair.sh is the harness behind them; + * tests/cca_gates_regcheck.sh is the register-level check that this + * contract holds, not a delivery measurement. * * `true` means DISABLED, matching SetCcaMode's argument sense and the * register's own polarity (bit set = gate off). SetCcaMode is exactly - * SetCcaGates(d, d) and writes the same bytes it always did. Returns false - * where the split is not ported; SetCcaMode remains the portable call. */ + * SetCcaGates(d, d) and writes the same bytes it always did; it stays the + * portable call, and is all a backend without the split offers. + * + * CONTRACT, because both halves of this have bitten: + * + * - POST-BRING-UP ONLY. Both calls return false before Init/InitWrite: + * 0x520 is meaningless until the MAC is configured, so reading it would + * be a fabricated gate state and writing it would poke an uninitialised + * MAC. `false` therefore means EITHER "not ported on this backend" OR + * "not brought up yet"; a caller probing capability at construction + * cannot tell those apart and must re-ask after bring-up. On a refusal + * GetCcaGates leaves its out-parameters untouched. + * + * SetCcaMode is NOT the same, and the difference is pre-existing rather + * than something the split introduced: it returns void, so a + * pre-bring-up call cannot report anything, and what it does with one + * is per-backend. The way to ask for a gate state from bring-up is the + * tuning.disable_cca config knob, which Init applies once the MAC is + * up. + * + * - STICKINESS SURVIVES A RETUNE ON BOTH, BUT ONLY ONE OF THEM MEANS IT. + * Measured on an 8812AU and an 8822C, the gate state is intact after + * SetMonitorChannel AND after FastRetune, both within a band and across + * a 5 GHz/2.4 GHz change, at 0x520, 0x524 and Jaguar1's BB thresholds. + * The mechanisms are not equivalent: Jaguar3 records the pair and + * re-asserts it in SetMonitorChannel (its FastRetune fallback does not, + * and does not need to), while Jaguar1 records nothing and survives + * only because its channel path happens not to rewrite those registers. + * Do not build on the Jaguar1 case — re-read with GetCcaGates rather + * than assume. Bring-up IS a reset on Jaguar1: Init/InitWrite + * unconditionally re-run SetCcaMode(_cfg.tuning.disable_cca), so a + * re-Init puts the gates back to the configured default. */ virtual bool SetCcaGates(bool primary_disabled, bool edcca_disabled) { (void)primary_disabled; (void)edcca_disabled; return false; } - /* Current gate state, read back from the hardware rather than remembered. */ + /* Current gate state, read back from the hardware rather than remembered. + * Same contract as SetCcaGates above: post-bring-up only, false where the + * split is unavailable, out-parameters untouched on a refusal. */ virtual bool GetCcaGates(bool &primary_disabled, bool &edcca_disabled) { (void)primary_disabled; (void)edcca_disabled; diff --git a/src/jaguar1/RtlJaguarDevice.cpp b/src/jaguar1/RtlJaguarDevice.cpp index 985d115e..419383a7 100644 --- a/src/jaguar1/RtlJaguarDevice.cpp +++ b/src/jaguar1/RtlJaguarDevice.cpp @@ -981,10 +981,8 @@ void RtlJaguarDevice::ClearAckResponder() { } bool RtlJaguarDevice::GetCcaGates(bool &primary_disabled, bool &edcca_disabled) { - /* The MAC register is meaningless before bring-up, and handing back - * whatever the bus returns would be a fabricated gate state. Jaguar3's - * SetCcaGates already guards on _brought_up; this is the read side and - * the Jaguar1 equivalent. */ + /* The MAC register is meaningless before bring-up, and reporting whatever + * the bus returns as the gate state would be a fabricated measurement. */ if (!_brought_up) return false; const uint32_t v = _device.rtw_read(0x0520); @@ -1015,11 +1013,15 @@ void RtlJaguarDevice::apply_cca(bool primary_disabled, bool edcca_disabled) { * generations — the vendor's phydm_mac_edcca_state drives 0x520[15] on * this family too; [14] is the primary-CCA defer. A set bit disables. */ uint32_t v520 = _device.rtw_read(0x0520); - if (primary_disabled) v520 |= (1u << 14); else v520 &= ~(1u << 14); - if (edcca_disabled) v520 |= (1u << 15); else v520 &= ~(1u << 15); + if (primary_disabled) + v520 |= (1u << 14); + else + v520 &= ~(1u << 14); + if (edcca_disabled) + v520 |= (1u << 15); + else + v520 &= ~(1u << 15); _device.rtw_write(0x0520, v520); - /* The BB threshold work below belongs to the EDCCA gate alone. */ - const bool disabled = edcca_disabled; /* BB EDCCA thresholds (rEDCCA_Jaguar 0x8a4: L2H byte0 / H2L byte1). The * BB init table parks them at 0x7f/0x7f = never-trigger — the vendor's @@ -1028,7 +1030,7 @@ void RtlJaguarDevice::apply_cca(bool primary_disabled, bool edcca_disabled) { * honour — enable must program the vendor operating point from the live * IGI for EDCCA to exist at all; disable re-parks. */ const auto ic = _eepromManager->version_id.ICType; - if (disabled) { + if (edcca_disabled) { _device.phy_set_bb_reg(0x8a4, 0xFFFF, 0x7f7f); } else { const int8_t th_ini = ic == CHIP_8814A ? -14 : -17; @@ -1049,7 +1051,7 @@ void RtlJaguarDevice::apply_cca(bool primary_disabled, bool edcca_disabled) { /* With the watchdog running, DIG walks IGI — hand it the re-track so the * threshold follows (vendor couples them per adaptivity cycle). */ if (auto *wd = _halModule.phydm_watchdog()) - wd->SetEdccaTrack(!disabled); + wd->SetEdccaTrack(!edcca_disabled); } bool RtlJaguarDevice::SetAmpduMode(const devourer::AmpduMode &mode) { diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 543a77a9..60334502 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -241,7 +241,9 @@ void RtlJaguar3Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { next += std::chrono::seconds(2); try { std::lock_guard lk(_reg_mu); - _phydm.tick(_channel.Channel, !_cca_disabled); + /* edcca_track follows the EDCCA gate alone — see the housekeeping + * tick below for why the all-or-nothing flag is the wrong input. */ + _phydm.tick(_channel.Channel, !_cca_edcca_disabled); } catch (...) { break; /* chip gone — the RX loop will wind down too */ } @@ -472,9 +474,13 @@ void RtlJaguar3Device::coex_runtime_loop() { _hal.coex_run_5g(); _hal.pwr_track(); /* thermal TX-power compensation (sustains upper 5 GHz) */ /* phydm dynamic mechanisms (vendor watchdog parity): FA/CCA window - * statistics -> DIG -> CCK-PD -> EDCCA. EDCCA tracking is owned by - * SetCcaMode when the EDCCA-disable knob is active. */ - _phydm.tick(_channel.Channel, !_cca_disabled); + * statistics -> DIG -> CCK-PD -> EDCCA. EDCCA tracking is owned by the + * EDCCA gate: keyed on the all-or-nothing flag instead, the watchdog + * would keep running PhydmRuntimeJaguar3::edcca() and rewriting the BB + * thresholds at 0x84c every ~2 s in the EDCCA-off/primary-on arm, + * undoing the disable the caller asked for. Jaguar1 does the same + * thing via SetEdccaTrack(!edcca_disabled) at the end of apply_cca. */ + _phydm.tick(_channel.Channel, !_cca_edcca_disabled); _hal.fw_update_wl_phy_info(); _hal.fw_set_pwr_mode_active(); _hal.fw_coex_query_bt_info(); @@ -1192,19 +1198,28 @@ void RtlJaguar3Device::apply_cca_gates_locked(bool primary_disabled, /* DIS_EDCCA (energy) + DIS_CCA (carrier-sense); a set bit disables. */ if (primary_disabled) v520 |= (1u << 14); else v520 &= ~(1u << 14); if (edcca_disabled) v520 |= (1u << 15); else v520 &= ~(1u << 15); - /* 0x524[11] moves with the pair and only with the pair. Deliberate: the - * two pure states then write exactly the bytes the all-or-nothing path - * wrote before this split, so SetCcaMode is byte-identical. What this bit - * means on its own is not documented here and was not measured, so a - * mixed state leaves it at the enabled value rather than guessing. */ - if (primary_disabled && edcca_disabled) v524 &= ~(1u << 11); - else v524 |= (1u << 11); + /* 0x524[11] is BIT_EDCCA_MSK_CNTDOWN_EN (REG_RD_CTRL) — EDCCA masking the + * backoff countdown. Same name and bit on 8822B/8822C/8822E, so the + * meaning is family-stable rather than an 8822C guess. Being EDCCA-scoped + * it follows edcca_disabled alone: leaving it set in the EDCCA-off arm + * would let EDCCA keep masking the countdown, i.e. only half-disable the + * gate the caller asked to turn off. SetCcaMode's two pure states are + * unaffected — both gates equal means this writes what it always did. */ + if (edcca_disabled) v524 &= ~(1u << 11); + else v524 |= (1u << 11); _device.rtw_write(0x0520, v520); _device.rtw_write(0x0524, v524); } bool RtlJaguar3Device::GetCcaGates(bool &primary_disabled, bool &edcca_disabled) { std::lock_guard lk(_reg_mu); + /* Before bring-up 0x520 holds whatever the chip's own boot left there (or + * whatever the bus returns on a powered-down part), and reporting that as + * the gate state would be a fabricated measurement. Same guard as Jaguar1, + * and it keeps the setter's refusal below honest: a caller that cannot set + * the gates yet cannot be handed a reading of them either. */ + if (!_brought_up) + return false; const uint32_t v = _device.rtw_read(0x0520); primary_disabled = (v & (1u << 14)) != 0; edcca_disabled = (v & (1u << 15)) != 0; @@ -1213,13 +1228,18 @@ bool RtlJaguar3Device::GetCcaGates(bool &primary_disabled, bool &edcca_disabled) bool RtlJaguar3Device::SetCcaGates(bool primary_disabled, bool edcca_disabled) { std::lock_guard lk(_reg_mu); + /* Post-bring-up only, and it says so rather than recording a request it + * will not carry out: neither Init nor InitWrite replays this state, so + * caching it here and returning true would report success for a write that + * never happens. The "configure it from bring-up" path is the existing + * tuning.disable_cca knob, which Init applies through SetCcaMode. */ + if (!_brought_up) + return false; /* Sticky the same way dis_cca is: a channel set rewrites the BB CCA * registers and SetMonitorChannel re-asserts from these. */ - _cca_disabled = primary_disabled && edcca_disabled; _cca_primary_disabled = primary_disabled; _cca_edcca_disabled = edcca_disabled; - if (_brought_up) - apply_cca_gates_locked(primary_disabled, edcca_disabled); + apply_cca_gates_locked(primary_disabled, edcca_disabled); _logger->info("Jaguar3: CCA gates primary={} edcca={}", primary_disabled ? "OFF" : "on", edcca_disabled ? "OFF" : "on"); return true; @@ -1227,7 +1247,6 @@ bool RtlJaguar3Device::SetCcaGates(bool primary_disabled, bool edcca_disabled) { void RtlJaguar3Device::SetCcaMode(bool disabled) { std::lock_guard lk(_reg_mu); - _cca_disabled = disabled; _cca_primary_disabled = disabled; _cca_edcca_disabled = disabled; if (_brought_up) diff --git a/src/jaguar3/RtlJaguar3Device.h b/src/jaguar3/RtlJaguar3Device.h index a904e52b..5b3e30cc 100644 --- a/src/jaguar3/RtlJaguar3Device.h +++ b/src/jaguar3/RtlJaguar3Device.h @@ -304,12 +304,12 @@ class RtlJaguar3Device : public IRtlRadio { std::atomic _bf_apply_on{false}; std::atomic _bf_cbr_count{0}; uint8_t _bf_peer[6] = {0}; - /* dis_cca sticky state — re-applied after SetMonitorChannel (the channel set - * rewrites the BB CCA registers). Caller holds _reg_mu. */ - bool _cca_disabled = false; - /* The two gates, tracked separately so a channel set re-asserts exactly - * what the caller asked for. Both false is the default, which is what - * _cca_disabled == false always meant. */ + /* dis_cca sticky state, one field per gate — re-applied after + * SetMonitorChannel (the channel set rewrites the BB CCA registers) and + * handed to phydm as edcca_track. Both false is the default. Caller holds + * _reg_mu. There is deliberately no combined flag: every consumer wants + * one specific gate, and the single all-or-nothing bool this replaced was + * how EDCCA tracking ended up keyed on the wrong one. */ bool _cca_primary_disabled = false; bool _cca_edcca_disabled = false; void apply_cca_mode_locked(bool disabled); diff --git a/tests/cca_gates_probe.cpp b/tests/cca_gates_probe.cpp new file mode 100644 index 00000000..53e44bd3 --- /dev/null +++ b/tests/cca_gates_probe.cpp @@ -0,0 +1,204 @@ +/* cca_gates_probe — drive IRtlRadio::SetCcaGates / GetCcaGates from the tree. + * + * The gate split had no in-tree caller, so nothing in the repo reproduced the + * tables that motivated it. This is that caller: it walks the four gate + * states, the two legacy SetCcaMode states, and the pre-bring-up refusal, and + * prints one machine-readable line per step for tests/cca_gates_regcheck.sh + * to assert against. Register-level confirmation is the regcheck's job (it + * peeks 0x520/0x524 with examples/chipstate --no-claim while this holds the + * interface); this binary reports what the API says, so a disagreement + * between the two is itself the finding. + * + * sudo build/CcaGatesProbe --pid 0xc812 --channel 36 + * sudo build/CcaGatesProbe --pid 0x8812 --channel 36 --hold 12 + * + * --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 + * backend does not implement the split (the not-ported default, not a + * failure). + */ +#include +#include +#include +#include +#include +#include + +#if __has_include() +#include +#else +#include +#endif + +#include "DeviceSession.h" +#include "IRtlRadio.h" +#include "WiFiDriver.h" +#include "logger.h" + +namespace { + +int fails = 0; + +void check(bool ok, const char *what) { + std::printf("%s %s\n", ok ? "PASS" : "FAIL", what); + if (!ok) + fails++; +} + +void report(const char *tag, bool ret, bool primary, bool edcca) { + std::printf("GATES %-26s ret=%d primary=%d edcca=%d\n", tag, ret ? 1 : 0, + primary ? 1 : 0, edcca ? 1 : 0); + std::fflush(stdout); +} + +} // namespace + +int main(int argc, char **argv) { + uint16_t vid = 0x0bda, pid = 0xc812; + int channel = 36, retune = 0, fast_retune = 0, hold = 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); + else if (!std::strcmp(argv[i], "--pid") && i + 1 < argc) + pid = (uint16_t)std::strtoul(argv[++i], nullptr, 0); + else if (!std::strcmp(argv[i], "--channel") && i + 1 < argc) + channel = std::atoi(argv[++i]); + else if (!std::strcmp(argv[i], "--retune") && i + 1 < argc) + retune = std::atoi(argv[++i]); + else if (!std::strcmp(argv[i], "--fast-retune") && i + 1 < argc) + fast_retune = std::atoi(argv[++i]); + else if (!std::strcmp(argv[i], "--hold") && i + 1 < argc) + hold = std::atoi(argv[++i]); + } + + auto logger = std::make_shared(); + libusb_context *ctx = nullptr; + if (libusb_init(&ctx) < 0) { + std::fprintf(stderr, "libusb_init failed\n"); + return 3; + } + devourer::DeviceSession session(logger); + libusb_device_handle *handle = libusb_open_device_with_vid_pid(ctx, vid, pid); + if (!handle) { + std::fprintf(stderr, "no adapter %04x:%04x\n", vid, pid); + return 3; + } + std::shared_ptr lock; + if (devourer::claim_interface_then_reset( + handle, devourer::find_wifi_interface(handle), logger, + /*do_reset=*/true, lock) != 0) { + session.adopt_handle(handle); + return 3; + } + session.adopt_handle(handle); + session.adopt_lock(lock); + + devourer::DeviceConfig cfg; + WiFiDriver driver(logger); + std::unique_ptr owned = driver.CreateRadio(handle, ctx, lock, cfg); + if (!owned) { + std::fprintf(stderr, "CreateRadio failed (chip support not built?)\n"); + return 3; + } + session.adopt_device(std::move(owned)); + IRadio *const dev = session.device(); + auto *const rtl = dynamic_cast(dev); + if (!rtl) { + std::printf("SKIP not a Realtek radio (IRtlRadio cast failed)\n"); + return 4; + } + + /* Pre-bring-up: both calls must refuse, and the refusal must not write the + * caller's variables. Poisoned true so an assignment is visible. */ + { + bool p = true, e = true; + const bool got = rtl->GetCcaGates(p, e); + report("pre-bringup-get", got, p, e); + check(!got, "GetCcaGates refuses before bring-up"); + check(p && e, "GetCcaGates leaves out-params alone when it refuses"); + const bool set = rtl->SetCcaGates(false, true); + report("pre-bringup-set", set, false, true); + check(!set, "SetCcaGates refuses before bring-up"); + } + + dev->InitWrite(SelectedChannel{.Channel = static_cast(channel), + .ChannelOffset = 0, + .ChannelWidth = CHANNEL_WIDTH_20}); + + { + bool p = true, e = true; + const bool got = rtl->GetCcaGates(p, e); + report("bringup-default", got, p, e); + if (!got) { + std::printf("SKIP backend does not implement the gate split " + "(not-ported default)\n"); + /* The not-ported exit still carries the pre-bring-up verdict: those + * checks ran above and a backend that failed them has not "cleanly + * refused", whatever it does about the split. */ + return fails ? 1 : 5; + } + check(!p && !e, "both gates are ENABLED at bring-up (the default)"); + } + + /* The four states, each read back from the hardware. */ + for (int i = 0; i < 4; i++) { + const bool want_p = (i & 2) != 0, want_e = (i & 1) != 0; + char tag[48]; + std::snprintf(tag, sizeof tag, "set-primary%d-edcca%d", want_p ? 1 : 0, + want_e ? 1 : 0); + const bool set = rtl->SetCcaGates(want_p, want_e); + bool p = false, e = false; + const bool got = rtl->GetCcaGates(p, e); + report(tag, set && got, p, e); + check(set && got && p == want_p && e == want_e, + "gate state reads back as written"); + /* Hold AFTER reporting, not before: the line above is what an external + * reader (tests/cca_gates_regcheck.sh peeking the registers with + * chipstate) waits for, so the state has to still be applied when it + * arrives. Holding first advertised each state one step too late. */ + if (hold) + std::this_thread::sleep_for(std::chrono::seconds(hold)); + } + + /* Retune survival. Measured on both families, the state is intact after a + * retune — within a band and across a band change — but by different + * mechanisms: Jaguar3 records the pair and re-asserts it in + * SetMonitorChannel, Jaguar1 records nothing and survives only because its + * channel path does not rewrite those registers. See src/IRtlRadio.h. */ + if (retune) { + rtl->SetCcaGates(true, false); + dev->SetMonitorChannel(SelectedChannel{.Channel = + static_cast(retune), + .ChannelOffset = 0, + .ChannelWidth = CHANNEL_WIDTH_20}); + bool p = false, e = false; + const bool got = rtl->GetCcaGates(p, e); + report("after-retune", got, p, e); + } + + /* FastRetune is the other channel path, and on Jaguar3 its fallback does + * not carry SetMonitorChannel's re-assert — so whether the state survives + * it is a separate question from --retune above, not the same one. */ + if (fast_retune) { + rtl->SetCcaGates(true, false); + dev->FastRetune(static_cast(fast_retune)); + bool p = false, e = false; + const bool got = rtl->GetCcaGates(p, e); + report("after-fast-retune", got, p, e); + } + + /* Legacy path: SetCcaMode must still be exactly SetCcaGates(d, d). */ + for (int i = 0; i < 2; i++) { + const bool d = i == 0; + dev->SetCcaMode(d); + bool p = false, e = false; + const bool got = rtl->GetCcaGates(p, e); + report(d ? "setccamode-true" : "setccamode-false", got, p, e); + check(got && p == d && e == d, "SetCcaMode moves both gates together"); + if (hold) + std::this_thread::sleep_for(std::chrono::seconds(hold)); + } + + std::printf("%s\n", fails ? "cca_gates_probe: FAIL" : "cca_gates_probe: PASS"); + return fails ? 1 : 0; +} diff --git a/tests/cca_gates_regcheck.sh b/tests/cca_gates_regcheck.sh new file mode 100755 index 00000000..0bf53279 --- /dev/null +++ b/tests/cca_gates_regcheck.sh @@ -0,0 +1,304 @@ +#!/usr/bin/env bash +# Register-level validation of the carrier-sense gate split +# (IRtlRadio::SetCcaGates / GetCcaGates), and the in-tree reproduction of the +# tables in the PR that added it. +# +# The API-level walk is build/CcaGatesProbe (tests/cca_gates_probe.cpp); this +# script cross-checks what the API says against what the chip holds, by +# peeking registers with examples/chipstate --no-claim while the probe owns +# the interface. The two disagreeing is itself the finding — an API reporting +# a gate state the silicon does not have is the failure mode the not-ported +# defaults exist to prevent. +# +# Cells, each of which must be able to FAIL: +# api the four gate states, the pre-bring-up refusal and the two legacy +# SetCcaMode states, as CcaGatesProbe reports them. +# regs 0x520[14] primary CCA and 0x520[15] EDCCA track the two arguments +# independently, read from the chip while the state is applied. +# cntdown 0x524[11] BIT_EDCCA_MSK_CNTDOWN_EN follows the EDCCA gate ALONE. +# Keyed on the pair instead, the EDCCA-off/primary-on arm leaves +# EDCCA masking the backoff countdown — the gate the caller asked +# to turn off is only half off. Whether a backend drives the bit +# at all is DISCOVERED rather than tabulated: SetCcaMode moves +# both gates, so a backend in which the bit has this role must +# move it between the two legacy states. The cell then fails if +# the two paths disagree in either direction. +# KNOWN LIMIT: a backend that stops writing the bit ENTIRELY is +# reported, not failed — with both paths silent there is nothing +# left to compare against, and deciding it "should" have moved +# would mean a per-chip expectation table, which this tree +# deliberately does not keep. The `track` cell is independent and +# does cover the functional half of the same arm. +# legacy SetCcaMode(d) writes what SetCcaGates(d, d) writes, so the split +# changed no default. The no-regression cell. +# track the phydm EDCCA tracker stops in an EDCCA-off arm. Poked rather +# than sampled: PhydmRuntimeJaguar3::edcca() recomputes the same +# th_l2h from a static IGI, so an active tracker rewrites the SAME +# bytes and is indistinguishable from an idle one by observation. +# Write a value it would never choose and see if it is restored. +# Skipped where no tracker is running in the default arm. +# retune the state survives SetMonitorChannel within a band and across a +# band change. Jaguar3 re-asserts by design; Jaguar1 merely is not +# clobbered (see src/IRtlRadio.h) — so this reports the mechanism +# and fails only on actual loss. +# +# Usage: sudo -v && tests/cca_gates_regcheck.sh # every plugged part +# PIDS=0xc812 sudo -v && tests/cca_gates_regcheck.sh +set -u +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +OUT="${CCA_GATES_OUT:-/tmp/devourer-cca-gates}" +# Honour a build directory elsewhere: this script is also run against a +# vendored copy of the tree whose build lives outside it. +BUILD="${BUILD:-$ROOT/build}" +VID=${VID:-0x0bda} +PIDS="${PIDS:-0x8812 0xc812 0xf72b}" # Jaguar1, Jaguar3, and an unported family +CH="${CH:-36}"; CH_SAME="${CH_SAME:-40}"; CH_BAND="${CH_BAND:-6}" +MARK_L2H=0x11 # nothing max(igi+8,48) can produce +mkdir -p "$OUT" + +PASS=0; FAIL=0; SKIP=0 +pass() { echo " PASS: $*"; PASS=$((PASS+1)); } +fail() { echo " FAIL: $*"; FAIL=$((FAIL+1)); } +skip() { echo " SKIP: $*"; SKIP=$((SKIP+1)); } +note() { echo " note: $*"; } +probe_pid="" +cleanup() { [ -n "$probe_pid" ] && kill "$probe_pid" 2>/dev/null; sudo -n pkill -x CcaGatesProbe 2>/dev/null; true; } +trap cleanup EXIT INT TERM + +echo "== building ==" +# A vendored copy's binaries may be built by an enclosing project, in which +# case $BUILD holds the outputs but no build system of its own. Try to build, +# then require the binaries either way — "cmake could not build here" is only +# a failure if the tools are actually missing. +cmake --build "$BUILD" -j --target CcaGatesProbe chipstate >/dev/null 2>&1 || true +for t in CcaGatesProbe chipstate; do + [ -x "$BUILD/$t" ] || { echo "missing $BUILD/$t — build it, or set BUILD="; exit 1; } +done + +# One little-endian dword, read out of band. Prints nothing and returns 1 when +# the read did not produce four bytes — a peek that silently returned 0 would +# make "both gates enabled" the reading for a failed sudo, a busy adapter or a +# 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 \ + --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) + n=$(printf '%s\n' "$bytes" | grep -c .) + [ "$n" -eq 4 ] || return 1 + 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 \ + --poke "$(printf '0x%x=0x%x:4' "$2" "$3")" >/dev/null 2>&1; } +bit() { echo $(( ( $1 >> $2 ) & 1 )); } + +# Start a probe that parks in each state for --hold, and wait until it has +# REPORTED the wanted arm (the probe holds after reporting, so the state is +# applied for the whole window the marker opens). +start_hold() { # $1=pid $2=log $3=extra args... + local pid="$1" log="$2"; shift 2 + sudo -n "$BUILD/CcaGatesProbe" --vid "$VID" --pid "$pid" \ + --channel "$CH" --hold 8 "$@" >"$log" 2>&1 & + probe_pid=$! +} +wait_marker() { # $1=log $2=marker + local i + for i in $(seq 1 90); do + grep -q "$2" "$1" && return 0 + kill -0 "$probe_pid" 2>/dev/null || return 1 + sleep 1 + done + return 1 +} +stop_hold() { [ -n "$probe_pid" ] && { kill "$probe_pid" 2>/dev/null; wait "$probe_pid" 2>/dev/null; }; probe_pid=""; } + +sudo -n true 2>/dev/null || { echo "needs a live sudo credential: sudo -v"; exit 2; } + +for pid in $PIDS; do + echo + echo "######## $pid ########" + lsusb -d "$(printf '%04x:%04x' "$VID" "$pid")" >/dev/null 2>&1 || { + skip "$pid not plugged"; continue; } + + log="$OUT/probe-$pid.log" + sudo -n "$BUILD/CcaGatesProbe" --vid "$VID" --pid "$pid" \ + --channel "$CH" --retune "$CH_SAME" >"$log" 2>&1 + rc=$? + if [ $rc -eq 3 ]; then + # Could not open the adapter at all: busy, absent, or CreateRadio + # refused. That is a bench fact, not a verdict on the gate split, and + # reporting it as eight failing cells is how a maintainer running this + # next to his own tools gets told his feature is broken. Quote what + # the probe said rather than inventing a diagnosis. + # Prefer the line that names the cause over the teardown noise that + # follows it — "libusb_release_interface rc=-5" is what happens after + # the open failed, not why. + why=$(grep -iE "is BUSY|already (in use|using)|no adapter|CreateRadio failed" "$log" | head -1) + [ -n "$why" ] || why=$(grep -iE "error|warn" "$log" | head -1) + skip "$pid could not be opened — ${why:-see $log}" + continue + fi + if [ $rc -eq 4 ]; then skip "$pid is not a Realtek radio"; continue; fi + if [ $rc -eq 5 ]; then + # The not-ported default is a PASS, not an absence of one: the point of + # the contract is that a backend without the split says so. Anchored on + # "^PASS " because the probe prints PASS and FAIL through the same + # formatter, so a substring match would accept the failure too. + if grep -q "^PASS GetCcaGates refuses before bring-up" "$log" && + grep -q "^PASS SetCcaGates refuses before bring-up" "$log" && + grep -q "^PASS GetCcaGates leaves out-params alone" "$log"; then + pass "$pid: gate split not ported, and both calls refuse cleanly" + else + fail "$pid: not-ported backend did not refuse cleanly (see $log)" + fi + continue + fi + [ $rc -eq 0 ] && pass "$pid api: probe walk clean" \ + || fail "$pid api: probe reported failures (see $log)" + + # --- regs + cntdown --------------------------------------------------- + cd_seen="" + for arm in "0 0" "0 1" "1 0" "1 1"; do + set -- $arm; want_p=$1; want_e=$2 + hold_log="$OUT/hold-$pid-$want_p$want_e.log" + start_hold "$pid" "$hold_log" + if ! wait_marker "$hold_log" "^GATES set-primary$want_p-edcca$want_e "; then + fail "$pid regs: probe never reported primary=$want_p edcca=$want_e" + stop_hold; continue + fi + v520=$(peek32 "$pid" $((0x520))) || { fail "$pid regs: 0x520 peek failed (primary=$want_p edcca=$want_e)"; stop_hold; continue; } + v524=$(peek32 "$pid" $((0x524))) || { fail "$pid regs: 0x524 peek failed (primary=$want_p edcca=$want_e)"; stop_hold; continue; } + stop_hold + + got_p=$(bit "$v520" 14); got_e=$(bit "$v520" 15) + if [ "$got_p" = "$want_p" ] && [ "$got_e" = "$want_e" ]; then + pass "$pid regs: primary=$want_p edcca=$want_e -> 0x520[14/15]=$got_p/$got_e" + else + fail "$pid regs: asked primary=$want_p edcca=$want_e, 0x520 says $got_p/$got_e" + fi + cd_seen="$cd_seen $want_p$want_e:$(bit "$v524" 11)" + done + + # The two legacy states, for the same register. SetCcaMode moves both + # gates, so these bracket what 0x524[11] does on this backend when the + # pair moves — the reference the split is judged against below. + cd_mode_on=""; cd_mode_off="" + for mode in true false; do + mlog="$OUT/mode-$pid-$mode.log" + start_hold "$pid" "$mlog" + if wait_marker "$mlog" "^GATES setccamode-$mode "; then + v=$(peek32 "$pid" $((0x524))) && { + if [ "$mode" = true ]; then cd_mode_on=$(bit "$v" 11) + else cd_mode_off=$(bit "$v" 11); fi + } + fi + stop_hold + done + + # cntdown: EDCCA-scoped means the bit is CLEAR exactly when EDCCA is + # disabled, whatever primary CCA is doing. A backend that never moves it + # does not use it in this role; one that moves it on primary CCA, or with + # the pair, fails here. + if [ "$(echo "$cd_seen" | tr ' ' '\n' | grep -c .)" -eq 4 ]; then + vals=$(echo "$cd_seen" | tr ' ' '\n' | grep . | cut -d: -f2 | sort -u | tr -d '\n') + # Whether this backend drives 0x524[11] is discovered, not tabulated: + # SetCcaMode moves both gates, so if the bit is in this role at all it + # must differ between the two legacy states. That turns "the bit never + # moved" from an untestable observation into a real expectation — a + # Jaguar3 that stopped writing it would otherwise pass silently. + uses_cd=0 + if [ "$cd_mode_on" != "$cd_mode_off" ]; then uses_cd=1; fi + if [ "$uses_cd" = 0 ]; then + if [ "$vals" = "01" ]; then + fail "$pid cntdown: 0x524[11] moves with the split but not with SetCcaMode ($cd_seen)" + else + note "$pid cntdown: 0x524[11] constant at $vals — not an EDCCA gate on this backend" + fi + elif [ "$vals" != "01" ]; then + fail "$pid cntdown: SetCcaMode moves 0x524[11] but the split leaves it at $vals ($cd_seen)" + elif true; then + ok=1 + for e in $cd_seen; do + want_e=${e%%:*}; want_e=${want_e#?}; got=${e##*:} + exp=$(( want_e == 1 ? 0 : 1 )) + [ "$got" = "$exp" ] || ok=0 + done + [ "$ok" = 1 ] \ + && pass "$pid cntdown: 0x524[11] follows the EDCCA gate alone ($cd_seen)" \ + || fail "$pid cntdown: 0x524[11] moves, but not with EDCCA ($cd_seen)" + else + note "$pid cntdown: 0x524[11] constant at $vals — not an EDCCA gate on this backend" + fi + fi + + grep -q "^PASS SetCcaMode moves both gates together" "$log" \ + && pass "$pid legacy: SetCcaMode(d) == SetCcaGates(d, d)" \ + || fail "$pid legacy: SetCcaMode no longer moves both gates" + + # --- track ------------------------------------------------------------ + # Does an EDCCA tracker overwrite the BB thresholds behind the caller? + tracker_in_default="" + 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" + 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 + fi + native=$(peek32 "$pid" $((0x84c))) || { fail "$pid track: 0x84c peek failed"; stop_hold; continue; } + poke32 "$pid" $((0x84c)) $(( (native & 0xff00ffff) | (MARK_L2H << 16) )) + sleep 5 + # Restore BEFORE any early exit: leaving the BB threshold at the + # marker would hand the next arm — and the next run — a chip in a + # state this script invented. + after=$(peek32 "$pid" $((0x84c))) + rc2=$? + poke32 "$pid" $((0x84c)) "$native" + [ $rc2 -eq 0 ] || { fail "$pid track: 0x84c re-read failed"; stop_hold; continue; } + stop_hold + if [ $(( (after >> 16) & 0xff )) -eq $(( MARK_L2H )) ]; then + restored=0; else restored=1; fi + if [ "$want_e" = 0 ]; then + tracker_in_default=$restored + [ "$restored" = 1 ] \ + && note "$pid track: tracker IS running in the default arm (as expected)" \ + || note "$pid track: no EDCCA tracker running in the default arm" + else + if [ -z "$tracker_in_default" ]; then + fail "$pid track: default arm never measured, so the EDCCA-off arm proves nothing" + elif [ "$tracker_in_default" = 0 ]; then + skip "$pid track: no tracker to stop in this configuration" + elif [ "$restored" = 0 ]; then + pass "$pid track: EDCCA tracking stops when EDCCA is the gate turned off" + else + fail "$pid track: tracker still rewriting 0x84c with EDCCA disabled" + fi + fi + done + + # --- retune ----------------------------------------------------------- + # Both channel paths, and both a same-band hop and a band change, because + # Jaguar3's FastRetune fallback does not carry SetMonitorChannel's + # re-assert and so is a separate question. + for target in "$CH_SAME" "$CH_BAND"; do + for path in retune fast-retune; do + rlog="$OUT/$path-$pid-$target.log" + sudo -n "$BUILD/CcaGatesProbe" --vid "$VID" --pid "$pid" \ + --channel "$CH" "--$path" "$target" >"$rlog" 2>&1 + marker=$([ "$path" = retune ] && echo after-retune || echo after-fast-retune) + line=$(grep "^GATES $marker" "$rlog" | head -1) + if echo "$line" | grep -q "ret=1 primary=1 edcca=0"; then + pass "$pid $path ch$CH->ch$target: gate state intact" + else + fail "$pid $path ch$CH->ch$target: expected primary=1 edcca=0, got '${line:-no line}'" + fi + done + done +done + +echo +echo "== $PASS passed, $FAIL failed, $SKIP skipped ==" +[ "$FAIL" -eq 0 ] diff --git a/tests/radio_iface_selftest.cpp b/tests/radio_iface_selftest.cpp index 99f48509..74dbc64f 100644 --- a/tests/radio_iface_selftest.cpp +++ b/tests/radio_iface_selftest.cpp @@ -23,6 +23,21 @@ struct NullRadio final : IRadio { void SetCcaMode(bool) override {} }; +/* The Realtek extension with nothing of its own implemented — the shape of a + * backend that derives from IRtlRadio but has not ported an optional member. + * RTL8733B is the live example: it overrides SetCcaMode and inherits the + * carrier-sense gate split. */ +struct NullRtlRadio final : IRtlRadio { + SelectedChannel ch_{}; + void Init(Action_ParsedRadioPacket, SelectedChannel c) override { ch_ = c; } + void InitWrite(SelectedChannel c) override { ch_ = c; } + void StartRxLoop(Action_ParsedRadioPacket) override {} + void SetMonitorChannel(SelectedChannel c) override { ch_ = c; } + bool send_packet(const uint8_t *, size_t) override { return false; } + SelectedChannel GetSelectedChannel() override { return ch_; } + void SetCcaMode(bool) override {} +}; + int fails = 0; void check(bool ok, const char *what) { if (!ok) { @@ -49,6 +64,28 @@ int main() { check(r->GetSelectedChannel().Channel == 6, "FastRetune default falls back to SetMonitorChannel"); + /* The Realtek-only members a backend may leave unported. The rule is that + * an unsupported optional member refuses instead of answering: a caller + * cannot tell a fabricated reading from a real one, so `false` is the only + * honest return. The gate split is the case with out-parameters, where + * refusing also means leaving the caller's variables alone. */ + std::unique_ptr rtl_owner = std::make_unique(); + auto *rtl = dynamic_cast(rtl_owner.get()); + check(rtl != nullptr, "a Realtek radio is reachable by dynamic_cast"); + + check(rtl->SetXtalCap(0) == -1, "SetXtalCap default refuses"); + check(rtl->GetXtalCap() == -1, "GetXtalCap default refuses"); + const RxEnergy energy = rtl->GetRxEnergy(false); + check(!energy.valid_fa && !energy.valid_igi && !energy.valid_nhm, + "GetRxEnergy default reports every field invalid"); + check(!rtl->ProbeEfuseStability().supported, + "ProbeEfuseStability default is unsupported"); + + check(!rtl->SetCcaGates(true, true), "SetCcaGates default refuses"); + bool primary = true, edcca = true; /* poison: a refusal must not write */ + check(!rtl->GetCcaGates(primary, edcca), "GetCcaGates default refuses"); + check(primary && edcca, "GetCcaGates leaves its out-params alone when it refuses"); + if (fails) return 1; std::puts("radio_iface: PASS"); return 0; From 2848c5a9290b7dca95c8420c75b52c93564081ab Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Sun, 13 Sep 2026 08:20:50 +0300 Subject: [PATCH 4/4] cca gates: close the Jaguar1 tracker race, and make the harness able to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the gate split. The first is a real defect of the same class the split already fixed once; the rest are the harness not being able to catch it. Jaguar1 apply_cca cleared the EDCCA tracker AFTER parking 0x8a4. The phydm watchdog owns that register while tracking and runs on its own thread from rtw_hal_init, so a tick landing between the park write and the flag store re-derived L2H from IGI and overwrote the park — live thresholds behind a disable the caller had asked for. Harmless while the gates could only be set at bring-up; SetCcaGates is what makes it a mid-session call. Reordering alone only narrows the window, so SetEdccaTrack is now synchronous: the tick's EDCCA block and the flag store share a mutex, and turning tracking off also drops the write-on-change cache, since the register no longer holds the value that cache names. apply_cca stops the tracker before it writes and hands the register back only after programming it. The harness could not have found that, because its track cell poked 0x84c on every family. That is Jaguar3's threshold register; Jaguar1's is 0x8a4. On a Jaguar1 DUT the cell poked something unrelated, saw no restore, and reported "no EDCCA tracker running" — a false negative on the family whose measurement motivated the split, and the one skipped cell in the original run. The probe now reports its generation from AdapterCaps and the cell picks the register from that, skipping a generation it has no address for rather than guessing. Also in the harness: - The retune cells only ever saw 0x520, because that is all GetCcaGates reads. They now peek 0x524[11] while the state is held. Negative control on an 8812CU with the countdown write deliberately broken: the API still reported ret=1 primary=1 edcca=0 while 0x524[11] read 0, so this is the half of the claim the API cannot speak for. - stop_hold could not stop anything: the probe is started through sudo, so a plain kill from an unprivileged shell got EPERM silently and wait then blocked for the probe's whole hold walk. Stops go through sudo now. - `elif true` made the cntdown cell's else unreachable, and the cell vanished with no verdict at all when fewer than four arms reported. Two scoping fixes. RtlJaguar2Device.cpp still asserted that primary CCA is the bit that stops an injector, on a family with no measurement of its own — the last unscoped copy of the claim this work scoped everywhere else. And in CLAUDE.md "on by default on the streamtx FPV downlink" had come to sit after the SetCcaGates sentence, where it read as describing the new API instead of DEVOURER_DIS_CCA. Verified: build clean, ctest 63/63, jaguar1-only config builds. tests/cca_gates_regcheck.sh 26 passed / 0 failed / 0 skipped over an 8822E (0xa81a), an 8822C (0xc812), an 8822BU and an RTL8733BU. No Jaguar1 on this bench, so that family's arm is still the one in the PR description. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0192ViRjaiFT9vTiiJpohZGT --- CLAUDE.md | 5 +- src/jaguar1/PhydmWatchdog.cpp | 38 +++++++--- src/jaguar1/PhydmWatchdog.h | 25 +++++-- src/jaguar1/RtlJaguarDevice.cpp | 21 +++++- src/jaguar2/RtlJaguar2Device.cpp | 10 ++- tests/cca_gates_probe.cpp | 16 +++++ tests/cca_gates_regcheck.sh | 115 +++++++++++++++++++++++-------- 7 files changed, 174 insertions(+), 56 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5f749e3d..b19cfba0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -345,8 +345,9 @@ Behavioural traps the per-field docs can't carry: all-or-nothing call and is `SetCcaGates(d, d)`. Both gate calls are post-bring-up only and return false before it — see `src/IRtlRadio.h` for the contract, and `tests/cca_gates_regcheck.sh` to reproduce the tables. - **On by default on the streamtx FPV - downlink** (the link owns the channel — CSMA backoff only stutters it); + + `DEVOURER_DIS_CCA` is **on by default on the streamtx FPV downlink** (the + link owns the channel — CSMA backoff only stutters it); `DEVOURER_DIS_CCA=0` forces standard carrier-sense back. On Kestrel the 8852C runs the same enabled default (measured: full-rate TX, 2.4x flood deferral); the 8852B TX bring-up still clears the gates and WARNS pending diff --git a/src/jaguar1/PhydmWatchdog.cpp b/src/jaguar1/PhydmWatchdog.cpp index b75c08f7..9cbead29 100644 --- a/src/jaguar1/PhydmWatchdog.cpp +++ b/src/jaguar1/PhydmWatchdog.cpp @@ -103,21 +103,37 @@ void PhydmWatchdog::TickOnce() { * vendor recomputes the 0x8a4 L2H/H2L from IGI every adaptivity cycle — * with DIG walking IGI above, a static threshold would drift off the * operating point. Write-on-change only. */ - if (_edcca_track.load(std::memory_order_relaxed)) { - const int8_t th_ini = - _eepromManager->version_id.ICType == CHIP_8814A ? -14 : -17; - const int8_t l2h = jaguar1_edcca_l2h(th_ini, _cur_ig_value); - if (static_cast(l2h) != _edcca_last_l2h) { - _device.phy_set_bb_reg(0x8a4, 0xFF, static_cast(l2h)); - _device.phy_set_bb_reg(0x8a4, 0xFF00, static_cast(l2h - 7)); - _edcca_last_l2h = static_cast(l2h); - _logger->info("PhydmWatchdog: EDCCA L2H/H2L re-tracked to {}/{} " - "(igi=0x{:02x})", - l2h, l2h - 7, _cur_ig_value); + { + /* Under _edcca_mu so a caller disabling the gate cannot have its park + * write raced by a tick that already passed the flag check. */ + std::lock_guard lk(_edcca_mu); + if (_edcca_track) { + const int8_t th_ini = + _eepromManager->version_id.ICType == CHIP_8814A ? -14 : -17; + const int8_t l2h = jaguar1_edcca_l2h(th_ini, _cur_ig_value); + if (static_cast(l2h) != _edcca_last_l2h) { + _device.phy_set_bb_reg(0x8a4, 0xFF, static_cast(l2h)); + _device.phy_set_bb_reg(0x8a4, 0xFF00, static_cast(l2h - 7)); + _edcca_last_l2h = static_cast(l2h); + _logger->info("PhydmWatchdog: EDCCA L2H/H2L re-tracked to {}/{} " + "(igi=0x{:02x})", + l2h, l2h - 7, _cur_ig_value); + } } } } +void PhydmWatchdog::SetEdccaTrack(bool on) { + std::lock_guard lk(_edcca_mu); + _edcca_track = on; + /* Turning tracking off hands 0x8a4 back to the caller, which parks it at + * the never-trigger value. Drop the write-on-change cache with it: the + * register no longer holds _edcca_last_l2h, so keeping it would let a + * later re-enable at the same IGI decide it had nothing to write. */ + if (!on) + _edcca_last_l2h = 0x7f; +} + void PhydmWatchdog::ReadFaCountersAc(FaCnt &out) { /* Port of `phydm_fa_cnt_statistics_ac` (phydm_dig.c:1421). * Reads OFDM/CCK FA + CCA + CRC32 counters from page-F BB diff --git a/src/jaguar1/PhydmWatchdog.h b/src/jaguar1/PhydmWatchdog.h index e0fb4b83..5d98e297 100644 --- a/src/jaguar1/PhydmWatchdog.h +++ b/src/jaguar1/PhydmWatchdog.h @@ -8,6 +8,7 @@ #include #include #include +#include #include class RadioManagementModule; @@ -53,12 +54,19 @@ class PhydmWatchdog { /* Run one watchdog cycle synchronously on the calling thread. */ void TickOnce(); - /* EDCCA threshold tracking (the SetCcaMode enable path): when on, each - * tick re-derives the BB 0x8a4 L2H/H2L from the IGI DIG just wrote — - * the vendor couples the EDCCA threshold to IGI per watchdog cycle - * (phydm_adaptivity). Off = leave 0x8a4 alone (SetCcaMode owns the - * parked/static value). */ - void SetEdccaTrack(bool on) { _edcca_track.store(on, std::memory_order_relaxed); } + /* EDCCA threshold tracking (the SetCcaMode / SetCcaGates enable path): + * when on, each tick re-derives the BB 0x8a4 L2H/H2L from the IGI DIG + * just wrote — the vendor couples the EDCCA threshold to IGI per watchdog + * cycle (phydm_adaptivity). Off = leave 0x8a4 alone (the caller owns the + * parked/static value). + * + * Synchronous by contract: this returns only once no tick is inside the + * EDCCA block and none can enter, so a caller turning tracking OFF may + * then write 0x8a4 knowing the watchdog will not overwrite it. Without + * that, a tick landing between the park write and the flag store leaves + * live thresholds behind a disable the caller already asked for — which + * only becomes reachable once the gates are settable mid-session. */ + void SetEdccaTrack(bool on); /* Most-recent FA counter snapshot — exposed for diagnostics / * future DIG integration. */ @@ -121,7 +129,10 @@ class PhydmWatchdog { * just walk based on FA count). */ bool _digInitialised = false; uint8_t _cur_ig_value = 0x20; - std::atomic _edcca_track{false}; + /* Serialises the tick's EDCCA block against SetEdccaTrack. Held only + * across that block, never across a whole tick. */ + std::mutex _edcca_mu; + bool _edcca_track = false; /* guarded by _edcca_mu */ uint8_t _edcca_last_l2h = 0x7f; /* parked sentinel — first tick writes */ uint8_t _dm_dig_max = 0x26; /* DIG_MAX_COVERAGR */ uint8_t _dm_dig_min = 0x1c; /* DIG_MIN_COVERAGE */ diff --git a/src/jaguar1/RtlJaguarDevice.cpp b/src/jaguar1/RtlJaguarDevice.cpp index 419383a7..933a78c5 100644 --- a/src/jaguar1/RtlJaguarDevice.cpp +++ b/src/jaguar1/RtlJaguarDevice.cpp @@ -1023,6 +1023,19 @@ void RtlJaguarDevice::apply_cca(bool primary_disabled, bool edcca_disabled) { v520 &= ~(1u << 15); _device.rtw_write(0x0520, v520); + /* Stop the EDCCA tracker BEFORE touching 0x8a4, not after. The phydm + * watchdog owns that register while tracking, and it runs on its own + * thread from rtw_hal_init — i.e. already before bring-up's SetCcaMode. + * Clearing the flag last left a window in which a tick could re-derive + * L2H from IGI and overwrite the park, leaving live thresholds behind a + * disable the caller had asked for. SetEdccaTrack is synchronous, so once + * it returns the writes below are ours. The enable direction hands the + * register over only after it is programmed, at the end of this function. + * Harmless when no watchdog was built (the default config). */ + if (edcca_disabled) + if (auto *wd = _halModule.phydm_watchdog()) + wd->SetEdccaTrack(false); + /* BB EDCCA thresholds (rEDCCA_Jaguar 0x8a4: L2H byte0 / H2L byte1). The * BB init table parks them at 0x7f/0x7f = never-trigger — the vendor's * adaptivity-off default (CONFIG_RTW_ADAPTIVITY_EN 0). Parked, the BB @@ -1049,9 +1062,11 @@ void RtlJaguarDevice::apply_cca(bool primary_disabled, bool edcca_disabled) { l2h, l2h - 7, igi); } /* With the watchdog running, DIG walks IGI — hand it the re-track so the - * threshold follows (vendor couples them per adaptivity cycle). */ - if (auto *wd = _halModule.phydm_watchdog()) - wd->SetEdccaTrack(!edcca_disabled); + * threshold follows (vendor couples them per adaptivity cycle). Only the + * enable direction is done here; the disable ran above, before the park. */ + if (!edcca_disabled) + if (auto *wd = _halModule.phydm_watchdog()) + wd->SetEdccaTrack(true); } bool RtlJaguarDevice::SetAmpduMode(const devourer::AmpduMode &mode) { diff --git a/src/jaguar2/RtlJaguar2Device.cpp b/src/jaguar2/RtlJaguar2Device.cpp index 60a120f5..7a1e9f7b 100644 --- a/src/jaguar2/RtlJaguar2Device.cpp +++ b/src/jaguar2/RtlJaguar2Device.cpp @@ -1929,9 +1929,13 @@ int32_t RtlJaguar2Device::PinBeaconTbtt(int32_t offset_us) { void RtlJaguar2Device::SetCcaMode(bool disabled) { std::lock_guard lk(_reg_mu); /* Both MAC carrier-sense bits in REG_TX_PTCL_CTRL: primary CCA 0x520[14] + - * EDCCA [15], plus EDCCA_MSK_COUNTDOWN 0x524[11]. The primary-CCA bit is the - * one that stops TX deferring to a co-channel transmitter; 0x520 - * is the same HalMAC layout as the on-air-validated Jaguar3. */ + * EDCCA [15], plus EDCCA_MSK_COUNTDOWN 0x524[11]. 0x520 is the same HalMAC + * layout as the on-air-validated Jaguar3, which is why the register writes + * are shared. Which of the two bits actually stops an injector is NOT: + * Jaguar3 and Jaguar1 measure opposite answers (see CLAUDE.md), and this + * family has no measurement of its own, so nothing here should be read as + * one. Jaguar2 has not ported the per-gate split — SetCcaGates is the + * not-ported default and this stays all-or-nothing. */ uint32_t v520 = _device.rtw_read(0x0520); uint32_t v524 = _device.rtw_read(0x0524); if (disabled) { v520 |= (1u << 15) | (1u << 14); v524 &= ~(1u << 11); } diff --git a/tests/cca_gates_probe.cpp b/tests/cca_gates_probe.cpp index 53e44bd3..495d021d 100644 --- a/tests/cca_gates_probe.cpp +++ b/tests/cca_gates_probe.cpp @@ -30,6 +30,7 @@ #include #endif +#include "AdapterCaps.h" #include "DeviceSession.h" #include "IRtlRadio.h" #include "WiFiDriver.h" @@ -108,6 +109,15 @@ int main(int argc, char **argv) { return 4; } + /* Name the family for the harness. The BB EDCCA threshold register is + * per-generation (Jaguar1 0x8a4, Jaguar3 0x84c) and a tracker cell that + * 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::fflush(stdout); + /* Pre-bring-up: both calls must refuse, and the refusal must not write the * caller's variables. Poisoned true so an assignment is visible. */ { @@ -174,6 +184,10 @@ int main(int argc, char **argv) { bool p = false, e = false; const bool got = rtl->GetCcaGates(p, e); report("after-retune", got, p, e); + /* GetCcaGates reads 0x520 only, so the API cannot speak for the rest of + * the gate state. Hold so the harness can peek 0x524[11] out of band. */ + if (hold) + std::this_thread::sleep_for(std::chrono::seconds(hold)); } /* FastRetune is the other channel path, and on Jaguar3 its fallback does @@ -185,6 +199,8 @@ int main(int argc, char **argv) { bool p = false, e = false; const bool got = rtl->GetCcaGates(p, e); report("after-fast-retune", got, p, e); + if (hold) + std::this_thread::sleep_for(std::chrono::seconds(hold)); } /* Legacy path: SetCcaMode must still be exactly SetCcaGates(d, d). */ diff --git a/tests/cca_gates_regcheck.sh b/tests/cca_gates_regcheck.sh index 0bf53279..49283554 100755 --- a/tests/cca_gates_regcheck.sh +++ b/tests/cca_gates_regcheck.sh @@ -32,15 +32,22 @@ # legacy SetCcaMode(d) writes what SetCcaGates(d, d) writes, so the split # changed no default. The no-regression cell. # track the phydm EDCCA tracker stops in an EDCCA-off arm. Poked rather -# than sampled: PhydmRuntimeJaguar3::edcca() recomputes the same -# th_l2h from a static IGI, so an active tracker rewrites the SAME -# bytes and is indistinguishable from an idle one by observation. -# Write a value it would never choose and see if it is restored. -# Skipped where no tracker is running in the default arm. -# retune the state survives SetMonitorChannel within a band and across a -# band change. Jaguar3 re-asserts by design; Jaguar1 merely is not -# clobbered (see src/IRtlRadio.h) — so this reports the mechanism -# and fails only on actual loss. +# than sampled: the tracker recomputes the same th_l2h from a +# static IGI, so an active tracker rewrites the SAME bytes and is +# indistinguishable from an idle one by observation. Write a value +# it would never choose and see if it is restored. The threshold +# register is PER-FAMILY (Jaguar1 0x8a4 bytes 0/1, Jaguar3 +# 0x84c[23:16]), taken from the generation the probe reports — +# 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. +# 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 +# reports the mechanism and fails only on actual loss. Checks +# 0x524[11] alongside the API readback, because GetCcaGates reads +# 0x520 alone and cannot see the countdown bit go missing. # # Usage: sudo -v && tests/cca_gates_regcheck.sh # every plugged part # PIDS=0xc812 sudo -v && tests/cca_gates_regcheck.sh @@ -54,6 +61,14 @@ VID=${VID:-0x0bda} PIDS="${PIDS:-0x8812 0xc812 0xf72b}" # Jaguar1, Jaguar3, and an unported family CH="${CH:-36}"; CH_SAME="${CH_SAME:-40}"; CH_BAND="${CH_BAND:-6}" MARK_L2H=0x11 # nothing max(igi+8,48) can produce +# The BB EDCCA threshold register is per-generation — Jaguar1 writes L2H/H2L +# as 0x8a4 bytes 0/1 (src/jaguar1/RtlJaguarDevice.cpp apply_cca and +# PhydmWatchdog::TickOnce), Jaguar3 writes th_l2h to 0x84c[23:16] +# (src/jaguar3/PhydmRuntimeJaguar3.cpp). Poking the other family's register +# reports "no tracker running" instead of failing, so the probe names its +# generation (GATES-GEN) and the track cell picks from here. +edcca_th_reg() { case "$1" in jaguar1) echo $((0x8a4));; jaguar3) echo $((0x84c));; *) echo "";; esac; } +edcca_th_shift() { case "$1" in jaguar1) echo 0;; jaguar3) echo 16;; *) echo "";; esac; } mkdir -p "$OUT" PASS=0; FAIL=0; SKIP=0 @@ -62,7 +77,11 @@ fail() { echo " FAIL: $*"; FAIL=$((FAIL+1)); } skip() { echo " SKIP: $*"; SKIP=$((SKIP+1)); } note() { echo " note: $*"; } probe_pid="" -cleanup() { [ -n "$probe_pid" ] && kill "$probe_pid" 2>/dev/null; sudo -n pkill -x CcaGatesProbe 2>/dev/null; true; } +# The probe is started through sudo, so its process is root-owned and a plain +# kill from this (unprivileged) shell gets EPERM — silently, leaving `wait` to +# block for the probe's whole hold walk. Every stop goes through sudo. +kill_probe() { [ -n "$probe_pid" ] && sudo -n kill "$probe_pid" 2>/dev/null; true; } +cleanup() { kill_probe; sudo -n pkill -x CcaGatesProbe 2>/dev/null; true; } trap cleanup EXIT INT TERM echo "== building ==" @@ -112,7 +131,15 @@ wait_marker() { # $1=log $2=marker done return 1 } -stop_hold() { [ -n "$probe_pid" ] && { kill "$probe_pid" 2>/dev/null; wait "$probe_pid" 2>/dev/null; }; probe_pid=""; } +stop_hold() { + [ -n "$probe_pid" ] || return 0 + kill_probe + # sudo forwards the signal to the child, but reaps it first; give the + # probe a moment to release the interface before the next claim. + wait "$probe_pid" 2>/dev/null + sudo -n pkill -x CcaGatesProbe 2>/dev/null + probe_pid="" +} sudo -n true 2>/dev/null || { echo "needs a live sudo credential: sudo -v"; exit 2; } @@ -158,8 +185,11 @@ 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) + note "$pid generation: ${gen:-unknown}" + # --- regs + cntdown --------------------------------------------------- - cd_seen="" + cd_seen=""; uses_cd=0 for arm in "0 0" "0 1" "1 0" "1 1"; do set -- $arm; want_p=$1; want_e=$2 hold_log="$OUT/hold-$pid-$want_p$want_e.log" @@ -201,7 +231,13 @@ for pid in $PIDS; do # disabled, whatever primary CCA is doing. A backend that never moves it # does not use it in this role; one that moves it on primary CCA, or with # the pair, fails here. - if [ "$(echo "$cd_seen" | tr ' ' '\n' | grep -c .)" -eq 4 ]; then + if [ "$(echo "$cd_seen" | tr ' ' '\n' | grep -c .)" -ne 4 ]; then + # Fewer than four arms reported, so there is nothing to compare. + # Say so: dropping the cell without a verdict moves no counter and + # reads, in the summary, exactly like a cell that was never meant + # to run here. + skip "$pid cntdown: only $(echo "$cd_seen" | tr ' ' '\n' | grep -c .)/4 arms reported (see $OUT)" + else vals=$(echo "$cd_seen" | tr ' ' '\n' | grep . | cut -d: -f2 | sort -u | tr -d '\n') # Whether this backend drives 0x524[11] is discovered, not tabulated: # SetCcaMode moves both gates, so if the bit is in this role at all it @@ -218,7 +254,7 @@ for pid in $PIDS; do fi elif [ "$vals" != "01" ]; then fail "$pid cntdown: SetCcaMode moves 0x524[11] but the split leaves it at $vals ($cd_seen)" - elif true; then + else ok=1 for e in $cd_seen; do want_e=${e%%:*}; want_e=${want_e#?}; got=${e##*:} @@ -228,8 +264,6 @@ for pid in $PIDS; do [ "$ok" = 1 ] \ && pass "$pid cntdown: 0x524[11] follows the EDCCA gate alone ($cd_seen)" \ || fail "$pid cntdown: 0x524[11] moves, but not with EDCCA ($cd_seen)" - else - note "$pid cntdown: 0x524[11] constant at $vals — not an EDCCA gate on this backend" fi fi @@ -239,7 +273,12 @@ for pid in $PIDS; do # --- track ------------------------------------------------------------ # Does an EDCCA tracker overwrite the BB thresholds behind the caller? + th_reg=$(edcca_th_reg "$gen"); th_shift=$(edcca_th_shift "$gen") tracker_in_default="" + if [ -z "$th_reg" ]; then + skip "$pid track: no EDCCA threshold register known for generation '${gen:-unknown}'" + else + th_name=$(printf '0x%x' "$th_reg") 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" @@ -248,24 +287,24 @@ for pid in $PIDS; do fail "$pid track: probe never reported primary=$want_p edcca=$want_e" stop_hold; continue fi - native=$(peek32 "$pid" $((0x84c))) || { fail "$pid track: 0x84c peek failed"; stop_hold; continue; } - poke32 "$pid" $((0x84c)) $(( (native & 0xff00ffff) | (MARK_L2H << 16) )) + native=$(peek32 "$pid" "$th_reg") || { fail "$pid track: $th_name peek failed"; stop_hold; continue; } + poke32 "$pid" "$th_reg" $(( (native & ~(0xff << th_shift)) | (MARK_L2H << th_shift) )) sleep 5 # Restore BEFORE any early exit: leaving the BB threshold at the # marker would hand the next arm — and the next run — a chip in a # state this script invented. - after=$(peek32 "$pid" $((0x84c))) + after=$(peek32 "$pid" "$th_reg") rc2=$? - poke32 "$pid" $((0x84c)) "$native" - [ $rc2 -eq 0 ] || { fail "$pid track: 0x84c re-read failed"; stop_hold; continue; } + poke32 "$pid" "$th_reg" "$native" + [ $rc2 -eq 0 ] || { fail "$pid track: $th_name re-read failed"; stop_hold; continue; } stop_hold - if [ $(( (after >> 16) & 0xff )) -eq $(( MARK_L2H )) ]; then + if [ $(( (after >> th_shift) & 0xff )) -eq $(( MARK_L2H )) ]; then restored=0; else restored=1; fi if [ "$want_e" = 0 ]; then tracker_in_default=$restored [ "$restored" = 1 ] \ - && note "$pid track: tracker IS running in the default arm (as expected)" \ - || note "$pid track: no EDCCA tracker running in the default arm" + && note "$pid track: tracker IS running at $th_name in the default arm (as expected)" \ + || note "$pid track: no EDCCA tracker running at $th_name in the default arm" else if [ -z "$tracker_in_default" ]; then fail "$pid track: default arm never measured, so the EDCCA-off arm proves nothing" @@ -274,10 +313,11 @@ for pid in $PIDS; do elif [ "$restored" = 0 ]; then pass "$pid track: EDCCA tracking stops when EDCCA is the gate turned off" else - fail "$pid track: tracker still rewriting 0x84c with EDCCA disabled" + fail "$pid track: tracker still rewriting $th_name with EDCCA disabled" fi fi done + fi # --- retune ----------------------------------------------------------- # Both channel paths, and both a same-band hop and a band change, because @@ -286,14 +326,29 @@ for pid in $PIDS; do for target in "$CH_SAME" "$CH_BAND"; do for path in retune fast-retune; do rlog="$OUT/$path-$pid-$target.log" - sudo -n "$BUILD/CcaGatesProbe" --vid "$VID" --pid "$pid" \ - --channel "$CH" "--$path" "$target" >"$rlog" 2>&1 marker=$([ "$path" = retune ] && echo after-retune || echo after-fast-retune) + # Held after the report so 0x524 can be peeked while the state is + # still applied: GetCcaGates reads 0x520 alone, so the API half of + # this cell cannot see the countdown bit being lost. + start_hold "$pid" "$rlog" "--$path" "$target" + if ! wait_marker "$rlog" "^GATES $marker "; then + fail "$pid $path ch$CH->ch$target: probe never reported $marker" + stop_hold; continue + fi + v524=$(peek32 "$pid" $((0x524))); rc3=$? + stop_hold line=$(grep "^GATES $marker" "$rlog" | head -1) - if echo "$line" | grep -q "ret=1 primary=1 edcca=0"; then - pass "$pid $path ch$CH->ch$target: gate state intact" - else + if ! echo "$line" | grep -q "ret=1 primary=1 edcca=0"; then fail "$pid $path ch$CH->ch$target: expected primary=1 edcca=0, got '${line:-no line}'" + continue + fi + # The arm is EDCCA ENABLED (edcca=0), so an EDCCA-scoped + # countdown bit must still be set. Only assert it where the + # cntdown cell established the backend drives the bit at all. + if [ "$uses_cd" = 1 ] && [ $rc3 -eq 0 ] && [ "$(bit "$v524" 11)" != 1 ]; then + fail "$pid $path ch$CH->ch$target: 0x520 survived but 0x524[11] was lost" + else + pass "$pid $path ch$CH->ch$target: gate state intact" fi done done