From dda2e71f2ee100461143a9c5e05769c8a5609bec Mon Sep 17 00:00:00 2001 From: snokvist Date: Mon, 14 Sep 2026 18:24:24 +0200 Subject: [PATCH 1/4] IRadio: WriteTsf reports success, and the MT7612U reports false IRadio::WriteTsf was a void method whose default was a silent no-op, so a caller could not tell a chip that moved its TSF from one that ignored the write. The MT7612U was exactly that case: we wrote MT_TSF_TIMER_DW0/DW1 and returned, while the registers hold a free-running counter that does not load. Swept on hardware (two MT7612U, ch6 and 5 GHz, firmware running, MT_BEACON_TIME_CFG TIMER_EN set): twelve writes across six sequences - both words in each order with the MAC running, each word alone, and the MAC- and timer-stopped variants - every one ignored, with the clock still advancing at wall rate and a positive control proving it was alive. No sequence loads the TSF on this part, so there is no sequence to fix. Make WriteTsf report whether the write is real. The default returns false (the RTL8733B and Kestrel have no TSF write either); the RTL8822C (jaguar3, readback-verified) and RTL8822B return true; the RTL8812AU moves its TSF only as part of the beacon-steer sequence, not a standalone write, so it returns false explicitly rather than by inheritance; and the MT7612U returns false without touching a register. A part whose registers exist but do not load and a part with no path at all both mean "no standalone write here", which is what false now says. `bringup tsfwrite [chan]` is the silicon-characterization cell: it PASSes only while every load sequence is confirmed ignored and the clock is confirmed alive, so a future firmware that enables loading trips it. docs/mt7612u.md and docs/time-distribution.md record the absence and the per-backend answers. API note for reviewers: this changes the public IRadio signature from void to bool and removes the standalone C symbol mt7612u_write_tsf (declaration, definition and its api_link presence check). There are no in-tree callers; out-of-tree callers must adapt, which is the intent - a silent no-op is worse than a compile error. --- docs/mt7612u.md | 11 ++ docs/time-distribution.md | 15 ++- src/IRadio.h | 15 ++- src/jaguar1/RtlJaguarDevice.h | 6 ++ src/jaguar2/RtlJaguar2Device.cpp | 9 +- src/jaguar2/RtlJaguar2Device.h | 2 +- src/jaguar3/RtlJaguar3Device.cpp | 7 +- src/jaguar3/RtlJaguar3Device.h | 2 +- src/mt7612u/Mt7612uRadio.cpp | 12 ++- src/mt7612u/Mt7612uRadio.h | 2 +- src/mt7612u/README.md | 1 + src/mt7612u/caps.cpp | 6 -- src/mt7612u/include/mt7612u/mt7612u.h | 4 +- src/mt7612u/tests/api_link.c | 1 - src/mt7612u/tools/bringup.cpp | 144 +++++++++++++++++++++++++- tests/radio_iface_selftest.cpp | 1 + 16 files changed, 212 insertions(+), 26 deletions(-) diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 3aaa933b..6618c474 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -230,6 +230,17 @@ saturation. A-MPDU, not USB parallelism, is what lifted 34 → 44.55 Mbit/s. `(dw0 << 32) | dw1`, which is backwards, but that value only feeds a `dev_dbg()` print upstream so the bug is never exercised. Ported faithfully it produced a clock advancing 8.6e14 "µs" per 200 ms. +- **There is no TSF write path.** `WriteTsf` is unsupported on this part: the + DW0/DW1 registers hold the counter, they do not load it. Every sequence the + gate tries was measured ignored on two units — `DW0` then `DW1` and the + reverse, each word alone (with a high word that actually differs), + `MT_BEACON_TIME_CFG_TIMER_EN` cleared and restored around the write, and a + write with the MAC stopped — after a positive control confirms the clock is + alive. In each case the clock kept free-running at wall rate and the value + never moved. The `bringup tsfwrite` gate runs that control and sweep and + PASSes only while the clock is alive and every sequence is confirmed ignored; + `Mt7612uRadio::WriteTsf` therefore returns false, and the register write is + not issued at all rather than pretending to land. - **Register-stream equivalence**: our EP0 write stream during bring-up was diffed against a `usbmon` capture of the kernel driver's own probe. 522 kernel writes vs 521 ours, 376 common addresses, one final-value mismatch diff --git a/docs/time-distribution.md b/docs/time-distribution.md index 43f3a6a6..9242615c 100644 --- a/docs/time-distribution.md +++ b/docs/time-distribution.md @@ -205,9 +205,18 @@ actuator, and the µs-class UE path is a PCIe UE. Harness: **Steering the TBTT.** The actuator is `AdjustBeaconTiming(microseconds)`, not a TSF write: on Jaguar2/3, `WriteTsf` moves the reported TSF (and the beacon-body timestamp) but NOT the TBTT air-time — a separate per-port timer drives the -beacon, so the TBTT is deaf to `REG_TSFTR`. (Jaguar1 is the opposite -architecture: its TBTT is hardware-locked to the TSF grid, so a TSF write moves -both — see the `PinBeaconTbtt` per-generation notes.) A one-shot +beacon, so the TBTT is deaf to `REG_TSFTR`. `WriteTsf` returns true on Jaguar3 +(measured on the bench: the reported TSF moves to the target plus the control +round trip) and on Jaguar2 (the same register pair, exercised by the fine-steer +path; a bare write there is inferred, not separately measured). It returns +false on the RTL8733B and Kestrel (no TSF write), on the MT7612U (its DW0/DW1 +registers do not load the counter — measured, `docs/mt7612u.md`), and explicitly +on Jaguar1, whose TSF moves only as part of the full beacon-steer sequence +rather than a standalone write: Jaguar1 is the opposite architecture, its TBTT +hardware-locked to the TSF grid, so the steer sequence moves both (see the +`PinBeaconTbtt` per-generation notes). `false` therefore means "no standalone +write here", and an adoption loop gets the failure instead of a silent no-op. +A one-shot beacon-interval tweak *does* steer the J2/J3 TBTT: running one interval at (nominal ± Δ) TU then restoring advances/retards the next TBTT — and the cadence thereafter — by Δ TU. Bench-proven to the microsecond on an diff --git a/src/IRadio.h b/src/IRadio.h index a6186a30..5a026d7d 100644 --- a/src/IRadio.h +++ b/src/IRadio.h @@ -367,8 +367,19 @@ class IRadio { * TSF (and the beacon-body timestamp) but NOT the beacon TBTT air-time — a * separate per-port timer drives the TBTT (bench-proven). To steer the * hardware-timed beacon (the uplink timing-advance actuator) use - * AdjustBeaconTiming. No-op where unsupported. */ - virtual void WriteTsf(uint64_t tsf) { (void)tsf; } + * AdjustBeaconTiming. + * + * Returns true when this backend drives a TSF write that the part's hardware + * accepts, false otherwise (the default). False means either that the part + * has no TSF load path (the MT7612U: DW0/DW1 writes were swept through every + * order, with TIMER_EN toggled and with the MAC stopped, and the clock ignored + * all of them — docs/mt7612u.md) or that no standalone write is implemented + * (Jaguar1 moves its TSF only as part of the full beacon-steer sequence; the + * RTL8733B and Kestrel have no TSF write). True does not by itself prove the value landed + * byte-for-byte: the counter keeps running, so a caller that needs certainty + * should still read back — a successful write reads as target + the control + * round trip. */ + virtual bool WriteTsf(uint64_t tsf) { (void)tsf; return false; } /* Load a beacon into the beacon reserved-page + enable the MAC beacon function, * so the chip AUTO-TRANSMITS it at each TBTT — hardware-timed and diff --git a/src/jaguar1/RtlJaguarDevice.h b/src/jaguar1/RtlJaguarDevice.h index 6b451273..60ee863d 100644 --- a/src/jaguar1/RtlJaguarDevice.h +++ b/src/jaguar1/RtlJaguarDevice.h @@ -320,6 +320,12 @@ class RtlJaguarDevice : public IRtlRadio { * so this is a lookup, not a chip access. */ bool GetPermanentMacAddress(uint8_t out[6]) override; uint64_t ReadTsf() override; + /* No standalone TSF write. On this generation the TSF is moved only by the + * complete beacon-steer sequence (see PinBeaconTbtt/AdjustBeaconTimingFine + * below), which shifts the reported TSF and the TBTT grid together; a bare + * REG_TSFTR write was never shown to take on its own. Reporting false keeps + * "unsupported" honest rather than inheriting a silent no-op. */ + bool WriteTsf(uint64_t tsf) override { (void)tsf; return false; } /* Hardware-timed beacon (IRadio contract): download the beacon MPDU to * the reserved page at the BCNQ boundary (the vendor rtl8812_download_rsvd_page diff --git a/src/jaguar2/RtlJaguar2Device.cpp b/src/jaguar2/RtlJaguar2Device.cpp index 7a1e9f7b..a7e10252 100644 --- a/src/jaguar2/RtlJaguar2Device.cpp +++ b/src/jaguar2/RtlJaguar2Device.cpp @@ -1960,12 +1960,17 @@ uint64_t RtlJaguar2Device::ReadTsf() { return (static_cast(hi) << 32) | lo; } -void RtlJaguar2Device::WriteTsf(uint64_t tsf) { +bool RtlJaguar2Device::WriteTsf(uint64_t tsf) { /* REG_TSFTR 0x0560 (low) / 0x0564 (high). Serialized on _reg_mu against the - * coex/thermal tick. The counter keeps running, so this sets it to ~tsf. */ + * coex/thermal tick. The counter keeps running, so this sets it to ~tsf. + * Same register pair as Jaguar3, and the fine-steer path that writes it is + * bench-proven to move the reported TSF there; a bare WriteTsf on this die + * has NOT been measured, so true means "the write was issued on a part with + * a load path", not "readback verified". */ std::lock_guard lk(_reg_mu); _device.rtw_write(0x0560, static_cast(tsf)); _device.rtw_write(0x0564, static_cast(tsf >> 32)); + return true; } void RtlJaguar2Device::Stop() { diff --git a/src/jaguar2/RtlJaguar2Device.h b/src/jaguar2/RtlJaguar2Device.h index 57bc3564..0e5735be 100644 --- a/src/jaguar2/RtlJaguar2Device.h +++ b/src/jaguar2/RtlJaguar2Device.h @@ -86,7 +86,7 @@ class RtlJaguar2Device : public IRtlRadio { /* EFUSE MAC at logical 0x107 (both dies — see HalJaguar2::perm_mac). */ bool GetPermanentMacAddress(uint8_t out[6]) override; uint64_t ReadTsf() override; - void WriteTsf(uint64_t tsf) override; + bool WriteTsf(uint64_t tsf) override; bool StartBeacon(const uint8_t *beacon, size_t len, int interval_tu) override; /* In-place beacon content swap (IRadio contract): retain the new MPDU + * ride the steer re-download; interval/TBTT/port identity untouched. */ diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 60334502..6446f6fc 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -2213,12 +2213,15 @@ uint64_t RtlJaguar3Device::ReadTsf() { return (static_cast(hi) << 32) | lo; } -void RtlJaguar3Device::WriteTsf(uint64_t tsf) { +bool RtlJaguar3Device::WriteTsf(uint64_t tsf) { /* REG_TSFTR 0x0560 (low) / 0x0564 (high). Serialized on _reg_mu against the - * coex tick. The counter keeps running, so this sets it to ~tsf. */ + * coex tick. The counter keeps running, so this sets it to ~tsf. Measured on + * the bench (RTL8822C): the reported TSF moves to the requested target plus + * the control round trip. */ std::lock_guard lk(_reg_mu); _device.rtw_write(0x0560, static_cast(tsf)); _device.rtw_write(0x0564, static_cast(tsf >> 32)); + return true; } bool RtlJaguar3Device::SetAckResponder(const devourer::MacAddr &mac) { diff --git a/src/jaguar3/RtlJaguar3Device.h b/src/jaguar3/RtlJaguar3Device.h index 5b3e30cc..b2573898 100644 --- a/src/jaguar3/RtlJaguar3Device.h +++ b/src/jaguar3/RtlJaguar3Device.h @@ -87,7 +87,7 @@ class RtlJaguar3Device : public IRtlRadio { * OTP is not reliably readable later), decoded on demand on 8822C. */ bool GetPermanentMacAddress(uint8_t out[6]) override; uint64_t ReadTsf() override; - void WriteTsf(uint64_t tsf) override; + bool WriteTsf(uint64_t tsf) override; bool StartBeacon(const uint8_t *beacon, size_t len, int interval_tu) override; /* In-place beacon content swap (IRadio contract): a fresh * download_beacon_page; interval/TBTT/port identity untouched. */ diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index a2acf9ab..4fd2413a 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -807,10 +807,14 @@ uint64_t Mt7612uRadio::ReadTsf() { return _dev ? mt7612u_read_tsf(_dev) : 0; } -void Mt7612uRadio::WriteTsf(uint64_t tsf) { - std::lock_guard lock(_mu); - if (_dev) - mt7612u_write_tsf(_dev, tsf); +bool Mt7612uRadio::WriteTsf(uint64_t tsf) { + /* This part has no TSF load path: the DW0/DW1 registers do not latch the + * counter. Every plausible sequence was measured ignored on two units — both + * word orders, TIMER_EN cleared and restored, and the write issued with the + * MAC stopped (the bringup `tsfwrite` gate; docs/mt7612u.md). Reporting true + * here would dress a silent no-op as success, so it reports false. */ + (void)tsf; + return false; } devourer::TxStats Mt7612uRadio::GetTxStats() { diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h index c0ede18b..bc54d3cf 100644 --- a/src/mt7612u/Mt7612uRadio.h +++ b/src/mt7612u/Mt7612uRadio.h @@ -97,7 +97,7 @@ class Mt7612uRadio : public IRadio { bool SetAmpduMode(const devourer::AmpduMode &mode) override; bool GetPermanentMacAddress(uint8_t out[6]) override; uint64_t ReadTsf() override; - void WriteTsf(uint64_t tsf) override; + bool WriteTsf(uint64_t tsf) override; devourer::TxStats GetTxStats() override; bool SetAckResponder(const devourer::MacAddr &mac) override; bool StartBeacon(const uint8_t *beacon, size_t len, int interval_tu) override; diff --git a/src/mt7612u/README.md b/src/mt7612u/README.md index fca6e073..201d0053 100644 --- a/src/mt7612u/README.md +++ b/src/mt7612u/README.md @@ -127,6 +127,7 @@ caps capabilities, TSF, 40 MHz soak sync vs async throughput pwr TX power vs the kernel's values ampdu aggregation A/B gateg per-frame rate control ack ACK responder (needs a stimulus) rtap send_packet / send_packets hop channel-switch cost +tsfwrite no TSF load path (positive control + every write sequence) ``` `make` here builds it as `./bringup`, which is what the hardware notes use. diff --git a/src/mt7612u/caps.cpp b/src/mt7612u/caps.cpp index fdba679a..65a006e0 100644 --- a/src/mt7612u/caps.cpp +++ b/src/mt7612u/caps.cpp @@ -29,12 +29,6 @@ uint64_t mt7612u_read_tsf(struct mt7612u_dev *d) return ((uint64_t)dw1 << 32) | dw0; } -void mt7612u_write_tsf(struct mt7612u_dev *d, uint64_t tsf) -{ - mt_wr(d, MT_TSF_TIMER_DW0, (uint32_t)tsf); - mt_wr(d, MT_TSF_TIMER_DW1, (uint32_t)(tsf >> 32)); -} - void mt7612u_get_caps(const struct mt7612u_dev *d, struct mt7612u_caps *c) { memset(c, 0, sizeof *c); diff --git a/src/mt7612u/include/mt7612u/mt7612u.h b/src/mt7612u/include/mt7612u/mt7612u.h index 4fdbf7c3..d0fb3c55 100644 --- a/src/mt7612u/include/mt7612u/mt7612u.h +++ b/src/mt7612u/include/mt7612u/mt7612u.h @@ -420,9 +420,9 @@ int mt7612u_link_stats(struct mt7612u_dev *dev, struct mt7612u_link_stats *out); */ int mt7612u_phy_tick(struct mt7612u_dev *dev); -/* TSF, the hardware microsecond clock. Two register reads. */ +/* TSF, the hardware microsecond clock. Two register reads. There is no writer: + * the DW0/DW1 registers do not load the counter (measured, see docs/mt7612u.md). */ uint64_t mt7612u_read_tsf(struct mt7612u_dev *dev); -void mt7612u_write_tsf(struct mt7612u_dev *dev, uint64_t tsf); /* What this adapter can do, so a caller need not assume. */ struct mt7612u_caps { diff --git a/src/mt7612u/tests/api_link.c b/src/mt7612u/tests/api_link.c index 42cf8572..d51029f1 100644 --- a/src/mt7612u/tests/api_link.c +++ b/src/mt7612u/tests/api_link.c @@ -41,7 +41,6 @@ static void *const api[] = { (void *)mt7612u_link_stats_start, (void *)mt7612u_link_stats, (void *)mt7612u_read_tsf, - (void *)mt7612u_write_tsf, (void *)mt7612u_get_caps, (void *)mt7612u_asic_version, (void *)mt7612u_mac_addr, diff --git a/src/mt7612u/tools/bringup.cpp b/src/mt7612u/tools/bringup.cpp index 44ad22a9..fcb87924 100644 --- a/src/mt7612u/tools/bringup.cpp +++ b/src/mt7612u/tools/bringup.cpp @@ -2863,6 +2863,145 @@ static int gate_rtap(uint8_t chan, int count) return 0; } +/* Gate TSF-WRITE: characterises whether this part has a TSF load path at all. + * The DW0/DW1 registers hold the free-running counter and do not load it: + * every sequence tried below was ignored on two units, with the clock first + * verified alive (a dead or wedged counter would also read as "no load path"). + * That measurement is why Mt7612uRadio::WriteTsf reports false. A future + * firmware that enables loading must fail this gate so the contract is + * revisited. Every arm builds its target from a fresh read so a stale write + * cannot look like a take. */ +static bool tsf_clock_alive(void) +{ + uint64_t t0 = mt7612u_read_tsf(&dev); + uint64_t t1; + int64_t d; + bool alive; + + mt_usleep(50000); + t1 = mt7612u_read_tsf(&dev); + d = (int64_t)(t1 - t0); + alive = d > 10000 && d < 200000; /* ~50 ms at the wall rate */ + printf(" %-34s t0=%llu t1=%llu delta=%+lld %s\n", + "clock control", (unsigned long long)t0, (unsigned long long)t1, + (long long)d, alive ? "ALIVE" : "DEAD"); + return alive; +} + +/* One load attempt. A take is the readback landing on the target; what the + * clock does afterwards is printed but does not decide, so a load that takes + * and then stalls is still a take. */ +static bool tsf_variant(const char *label, uint64_t target, bool stop_timer, + bool high_word_first, int which) +{ + uint32_t cfg = mt_rr(&dev, MT_BEACON_TIME_CFG); + uint64_t r1, r2; + int64_t err, rate; + bool took; + + if (stop_timer) + mt_wr(&dev, MT_BEACON_TIME_CFG, cfg & ~MT_BEACON_TIME_CFG_TIMER_EN); + + if (which == 0) { /* both words */ + if (high_word_first) { + mt_wr(&dev, MT_TSF_TIMER_DW1, (uint32_t)(target >> 32)); + mt_wr(&dev, MT_TSF_TIMER_DW0, (uint32_t)target); + } else { + mt_wr(&dev, MT_TSF_TIMER_DW0, (uint32_t)target); + mt_wr(&dev, MT_TSF_TIMER_DW1, (uint32_t)(target >> 32)); + } + } else if (which == 1) { /* low word only */ + mt_wr(&dev, MT_TSF_TIMER_DW0, (uint32_t)target); + } else { /* high word only */ + mt_wr(&dev, MT_TSF_TIMER_DW1, (uint32_t)(target >> 32)); + } + + if (stop_timer) + mt_wr(&dev, MT_BEACON_TIME_CFG, cfg); + + mt_usleep(20000); + r1 = mt7612u_read_tsf(&dev); + mt_usleep(50000); + r2 = mt7612u_read_tsf(&dev); + err = (int64_t)(r1 - target); + rate = (int64_t)(r2 - r1); + took = llabs(err) < 200000; + printf(" %-34s target=%llu read=%llu err=%+lld delta50ms=%+lld %s\n", + label, (unsigned long long)target, (unsigned long long)r1, + (long long)err, (long long)rate, took ? "TOOK" : "no-op"); + return took; +} + +static int gate_tsfwrite(uint8_t chan) +{ + uint64_t base; + uint32_t cfg; + bool any = false; + + if (mt_eeprom_init(&dev)) { + printf("GATE TSF-WRITE: FAIL - eeprom_init failed\n"); + return 1; + } + if (mt_init_hardware(&dev, NULL)) { + printf("GATE TSF-WRITE: FAIL - init_hardware failed\n"); + return 1; + } + if (mt_set_channel(&dev, chan, MT7612U_BW_20)) { + printf("GATE TSF-WRITE: FAIL - set_channel failed\n"); + return 1; + } + + cfg = mt_rr(&dev, MT_BEACON_TIME_CFG); + printf("MT_BEACON_TIME_CFG=0x%08x TIMER_EN=%u TBTT_EN=%u BEACON_TX=%u SYNC_MODE=%u\n", + cfg, !!(cfg & MT_BEACON_TIME_CFG_TIMER_EN), + !!(cfg & MT_BEACON_TIME_CFG_TBTT_EN), + !!(cfg & MT_BEACON_TIME_CFG_BEACON_TX), + (unsigned)FIELD_GET(MT_BEACON_TIME_CFG_SYNC_MODE, cfg)); + + /* A dead counter reads exactly like a counter that ignores loads. */ + if (!tsf_clock_alive()) { + printf("\nGATE TSF-WRITE: FAIL - the TSF clock is not running; no load conclusion\n"); + return 1; + } + + /* With the MAC running (the state a live link is in): both word orders, + * then each word alone. The high-word-only target sets a high word that + * differs from the live one, so the arm is not vacuous. */ + if (mt_mac_start(&dev, MT_RX_DRAIN_NONE)) { + printf("\nGATE TSF-WRITE: FAIL - mt_mac_start failed\n"); + return 1; + } + base = mt7612u_read_tsf(&dev); + any |= tsf_variant("MAC on, both words DW0,DW1", base + 5000000, false, false, 0); + base = mt7612u_read_tsf(&dev); + any |= tsf_variant("MAC on, both words DW1,DW0", base + 5000000, false, true, 0); + base = mt7612u_read_tsf(&dev); + any |= tsf_variant("MAC on, high word (DW1) only", base + (1ull << 32), false, false, 2); + base = mt7612u_read_tsf(&dev); + any |= tsf_variant("MAC on, low word (DW0) only", base + 5000000, false, false, 1); + + /* With the MAC stopped, and with the free-running timer disabled. */ + mt_mac_stop(&dev); + base = mt7612u_read_tsf(&dev); + any |= tsf_variant("MAC off, both words", base + 5000000, false, false, 0); + base = mt7612u_read_tsf(&dev); + any |= tsf_variant("MAC off, timer off, both words", base + 5000000, true, false, 0); + + /* A mid-run transport failure reads as all-ones, which every arm would + * otherwise report as "ignored" — the exact false conclusion this gate + * exists to prevent. */ + if (mt_io_errors(&dev) != 0) { + printf("\nGATE TSF-WRITE: FAIL - %u USB transfer(s) failed during the sweep; no load conclusion\n", + mt_io_errors(&dev)); + return 1; + } + + printf("\nGATE TSF-WRITE: %s\n", any + ? "FAIL - a write sequence takes; WriteTsf must report success" + : "PASS - confirmed: no sequence loads the TSF, so WriteTsf reports false"); + return any ? 1 : 0; +} + int main(int argc, char **argv) { const char *err = NULL, *cmd = argc > 1 ? argv[1] : "regs"; @@ -2922,6 +3061,8 @@ int main(int argc, char **argv) argc > 4 ? atoi(argv[4]) : 0); } else if (!strcmp(cmd, "caps")) { rc = gate_caps(argc > 2 ? (uint8_t)atoi(argv[2]) : 149); + } else if (!strcmp(cmd, "tsfwrite")) { + rc = gate_tsfwrite(argc > 2 ? (uint8_t)atoi(argv[2]) : 149); } else if (!strcmp(cmd, "rxbytes")) { rc = gate_rxbytes(argc > 2 ? (uint8_t)atoi(argv[2]) : 1, argc > 3 ? atoi(argv[3]) : 15); @@ -2996,8 +3137,9 @@ int main(int argc, char **argv) rc = gate_fw(argc > 2 ? argv[2] : NULL); } else { fprintf(stderr, "unknown subcommand '%s'\n", cmd); - fprintf(stderr, "usage: bringup [regs|fw|init|chan|tx|rx|hop|gateg] [chan] [count] [phy 0=CCK 1=OFDM 2=HT 4=VHT] [mcs]\n"); + fprintf(stderr, "usage: bringup [regs|fw|init|chan|tx|rx|hop|gateg|tsfwrite] [chan] [count] [phy 0=CCK 1=OFDM 2=HT 4=VHT] [mcs]\n"); fprintf(stderr, " bringup adopt (the mt_adopt path a libusb-owning consumer uses)\n"); + fprintf(stderr, " bringup tsfwrite [chan] (confirm this part has no TSF load path)\n"); fprintf(stderr, " bringup beacon [chan] [secs] (Stage A: static AP beacon on air)\n"); fprintf(stderr, " bringup ap [chan] [secs] (Stage B: beacon + RX, probe/auth/assoc)\n"); fprintf(stderr, " bringup [sweep|coding|vht] [chan] [count] [bw 0=20 1=40 2=80]\n"); diff --git a/tests/radio_iface_selftest.cpp b/tests/radio_iface_selftest.cpp index 74dbc64f..9c254bea 100644 --- a/tests/radio_iface_selftest.cpp +++ b/tests/radio_iface_selftest.cpp @@ -59,6 +59,7 @@ int main() { check(!r->GetTxPowerCaps().supported, "GetTxPowerCaps default is unsupported"); check(!r->SetAckResponder(devourer::MacAddr{}), "SetAckResponder default refuses"); check(r->ReadTsf() == 0, "ReadTsf default is 0"); + check(!r->WriteTsf(123456789ull), "WriteTsf default refuses and reports false"); r->FastRetune(6); check(r->GetSelectedChannel().Channel == 6, From e70a1e104b43d629eaf6976c1b28cde8b47e6a76 Mon Sep 17 00:00:00 2001 From: snokvist Date: Mon, 14 Sep 2026 19:08:01 +0200 Subject: [PATCH 2/4] jaguar2: record the measured TSF write, not an inference The WriteTsf comment and docs/time-distribution.md hedged Jaguar2's true as inferred from Jaguar3's register pair. It is now measured on the bench: RTL8822B, write of target 6230820 us read back 6284239 (the target plus the 110 us control round trip and the sleep between reads), took true. Say so; no behaviour change. --- docs/time-distribution.md | 18 ++++++++---------- src/jaguar2/RtlJaguar2Device.cpp | 6 ++---- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/docs/time-distribution.md b/docs/time-distribution.md index 9242615c..c037de33 100644 --- a/docs/time-distribution.md +++ b/docs/time-distribution.md @@ -205,16 +205,14 @@ actuator, and the µs-class UE path is a PCIe UE. Harness: **Steering the TBTT.** The actuator is `AdjustBeaconTiming(microseconds)`, not a TSF write: on Jaguar2/3, `WriteTsf` moves the reported TSF (and the beacon-body timestamp) but NOT the TBTT air-time — a separate per-port timer drives the -beacon, so the TBTT is deaf to `REG_TSFTR`. `WriteTsf` returns true on Jaguar3 -(measured on the bench: the reported TSF moves to the target plus the control -round trip) and on Jaguar2 (the same register pair, exercised by the fine-steer -path; a bare write there is inferred, not separately measured). It returns -false on the RTL8733B and Kestrel (no TSF write), on the MT7612U (its DW0/DW1 -registers do not load the counter — measured, `docs/mt7612u.md`), and explicitly -on Jaguar1, whose TSF moves only as part of the full beacon-steer sequence -rather than a standalone write: Jaguar1 is the opposite architecture, its TBTT -hardware-locked to the TSF grid, so the steer sequence moves both (see the -`PinBeaconTbtt` per-generation notes). `false` therefore means "no standalone +beacon, so the TBTT is deaf to `REG_TSFTR`. `WriteTsf` returns true on Jaguar3 and Jaguar2 (both readback-measured on the +bench: the reported TSF moves to the target plus the control round trip). It +returns false on the RTL8733B and Kestrel (no TSF write), on the MT7612U (its +DW0/DW1 registers do not load the counter — measured, `docs/mt7612u.md`), and +explicitly on Jaguar1, whose TSF moves only as part of the full beacon-steer +sequence rather than a standalone write: Jaguar1 is the opposite architecture, +its TBTT hardware-locked to the TSF grid, so the steer sequence moves both (see +the `PinBeaconTbtt` per-generation notes). `false` therefore means "no standalone write here", and an adoption loop gets the failure instead of a silent no-op. A one-shot beacon-interval tweak *does* steer the J2/J3 TBTT: running diff --git a/src/jaguar2/RtlJaguar2Device.cpp b/src/jaguar2/RtlJaguar2Device.cpp index a7e10252..10091fbe 100644 --- a/src/jaguar2/RtlJaguar2Device.cpp +++ b/src/jaguar2/RtlJaguar2Device.cpp @@ -1963,10 +1963,8 @@ uint64_t RtlJaguar2Device::ReadTsf() { bool RtlJaguar2Device::WriteTsf(uint64_t tsf) { /* REG_TSFTR 0x0560 (low) / 0x0564 (high). Serialized on _reg_mu against the * coex/thermal tick. The counter keeps running, so this sets it to ~tsf. - * Same register pair as Jaguar3, and the fine-steer path that writes it is - * bench-proven to move the reported TSF there; a bare WriteTsf on this die - * has NOT been measured, so true means "the write was issued on a part with - * a load path", not "readback verified". */ + * Same register pair as Jaguar3; both are bench-proven to move the reported + * TSF (RTL8822B readback: the target plus the control round trip). */ std::lock_guard lk(_reg_mu); _device.rtw_write(0x0560, static_cast(tsf)); _device.rtw_write(0x0564, static_cast(tsf >> 32)); From 9280d29f3050a7042cffd248e071a5d237e82e6d Mon Sep 17 00:00:00 2001 From: snokvist Date: Mon, 14 Sep 2026 19:24:50 +0200 Subject: [PATCH 3/4] IRadio: WriteTsf reports partial failures, and the gate proves the clock Review follow-ups on the WriteTsf contract change: - Jaguar2/Jaguar3 now return the AND of the two rtw_write results. Both words are still written even if the first transfer fails (the pair is the unit; a half-updated counter is not something to leave behind), but a failed transfer is no longer dressed as success. - The mt7612u tsfwrite gate reads MT_BEACON_TIME_CFG with mt_rr_chk and skips the timer-off arm when the read fails, instead of writing the 0xffffffff failure sentinel back as configuration; every arm now also requires the clock to still be advancing at the wall rate, so a counter that stalls during the sweep cannot be read as "ignored" and PASS. - mt_mac_stop runs before the mt_mac_start failure return, and the gate's own config read has a failure verdict. - Jaguar1's false override says why: the standalone method deliberately does not perform the beacon-steer sequence's EN_BCN_FUNCTION toggle and TBTT re-download, which move the beacon grid rather than set the clock. - docs: "no tested sequence loads the TSF" rather than an absolute, with the mainline mt76 absence of a mt76x02 .set_tsf beside it; the IRadio comment states the contract and points at time-distribution.md for the per-backend state instead of enumerating them; api_link's entry count corrected to 30. Re-verified on hardware both bands: ch6 and ch149 each pass with the clock control ALIVE and all six arms no-op; ctest 63/63; make -C src/mt7612u check passes. --- docs/mt7612u.md | 21 ++++--- docs/time-distribution.md | 9 +-- src/IRadio.h | 17 +++--- src/jaguar1/RtlJaguarDevice.h | 7 ++- src/jaguar2/RtlJaguar2Device.cpp | 9 ++- src/jaguar3/RtlJaguar3Device.cpp | 9 ++- src/mt7612u/Mt7612uRadio.cpp | 8 +-- src/mt7612u/include/mt7612u/mt7612u.h | 4 +- src/mt7612u/tools/bringup.cpp | 87 +++++++++++++++++++-------- 9 files changed, 109 insertions(+), 62 deletions(-) diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 6618c474..62f84919 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -230,17 +230,20 @@ saturation. A-MPDU, not USB parallelism, is what lifted 34 → 44.55 Mbit/s. `(dw0 << 32) | dw1`, which is backwards, but that value only feeds a `dev_dbg()` print upstream so the bug is never exercised. Ported faithfully it produced a clock advancing 8.6e14 "µs" per 200 ms. -- **There is no TSF write path.** `WriteTsf` is unsupported on this part: the - DW0/DW1 registers hold the counter, they do not load it. Every sequence the - gate tries was measured ignored on two units — `DW0` then `DW1` and the +- **No tested sequence loads the TSF.** `WriteTsf` is unsupported on this part: + the DW0/DW1 registers hold the counter, they do not load it. Every sequence + the gate tries was measured ignored on two units — `DW0` then `DW1` and the reverse, each word alone (with a high word that actually differs), `MT_BEACON_TIME_CFG_TIMER_EN` cleared and restored around the write, and a write with the MAC stopped — after a positive control confirms the clock is - alive. In each case the clock kept free-running at wall rate and the value - never moved. The `bringup tsfwrite` gate runs that control and sweep and - PASSes only while the clock is alive and every sequence is confirmed ignored; - `Mt7612uRadio::WriteTsf` therefore returns false, and the register write is - not issued at all rather than pretending to land. + alive, and with a per-arm check that the clock was still advancing. In each + case the clock kept free-running at wall rate and the value never moved. + Mainline mt76 registers no `.set_tsf` for the mt76x02 family, so the absence + is a property of the part rather than of this port. The `bringup tsfwrite` + gate runs that control and sweep and PASSes only while the clock is alive and + every sequence is confirmed ignored; `Mt7612uRadio::WriteTsf` therefore + returns false and issues no register write at all rather than pretending one + landed. - **Register-stream equivalence**: our EP0 write stream during bring-up was diffed against a `usbmon` capture of the kernel driver's own probe. 522 kernel writes vs 521 ours, 376 common addresses, one final-value mismatch @@ -350,7 +353,7 @@ nothing. Each fails the cell, and each names the property it broke. | test | what it holds | |---|---| -| `api_link` | takes the address of all 27 public entry points while including only the public header, so a declaration that loses its definition is a link error. Still compiled as C, which is what keeps the `extern "C"` guard honest now the library itself is C++ | +| `api_link` | takes the address of all 30 public entry points while including only the public header, so a declaration that loses its definition is a link error. Still compiled as C, which is what keeps the `extern "C"` guard honest now the library itself is C++ | | `frame_shape` | `mt_hdrlen_from_fc()` over management, all eight control subtypes and the five data shapes; the RX L2-pad fold on a synthetic QoS frame, with a negative control that redoes the old fixed-24 fold and asserts the QoS Control really is destroyed; the radiotap VHT bandwidth mapping over all eleven codes the part can express | | `field_macros` | `MT_CTZ` against `__builtin_ctz` over all 32 single-bit and all 528 contiguous masks, plus a `FIELD_PREP`/`FIELD_GET` round-trip, plus a static initialiser that fails to compile if the macro stops being constant-foldable | | `log_sink` | that `mt7612u_set_log_sink()` **diverts** rather than copies — stderr must stay silent while a sink is installed — that the sink gets the bare message with no prefix to double up, that every level letter arrives, and that NULL restores the built-in sink instead of silencing the library | diff --git a/docs/time-distribution.md b/docs/time-distribution.md index c037de33..ca79739a 100644 --- a/docs/time-distribution.md +++ b/docs/time-distribution.md @@ -209,10 +209,11 @@ beacon, so the TBTT is deaf to `REG_TSFTR`. `WriteTsf` returns true on Jaguar3 a bench: the reported TSF moves to the target plus the control round trip). It returns false on the RTL8733B and Kestrel (no TSF write), on the MT7612U (its DW0/DW1 registers do not load the counter — measured, `docs/mt7612u.md`), and -explicitly on Jaguar1, whose TSF moves only as part of the full beacon-steer -sequence rather than a standalone write: Jaguar1 is the opposite architecture, -its TBTT hardware-locked to the TSF grid, so the steer sequence moves both (see -the `PinBeaconTbtt` per-generation notes). `false` therefore means "no standalone +explicitly on Jaguar1: its TSF does move, but only as part of the full +beacon-steer sequence, whose `EN_BCN_FUNCTION` toggle and TBTT re-download the +standalone method deliberately does not perform (Jaguar1's TBTT is +hardware-locked to the TSF grid, so that sequence moves both; see the +`PinBeaconTbtt` per-generation notes). `false` therefore means "no standalone write here", and an adoption loop gets the failure instead of a silent no-op. A one-shot beacon-interval tweak *does* steer the J2/J3 TBTT: running diff --git a/src/IRadio.h b/src/IRadio.h index 5a026d7d..b40f3da8 100644 --- a/src/IRadio.h +++ b/src/IRadio.h @@ -369,16 +369,13 @@ class IRadio { * hardware-timed beacon (the uplink timing-advance actuator) use * AdjustBeaconTiming. * - * Returns true when this backend drives a TSF write that the part's hardware - * accepts, false otherwise (the default). False means either that the part - * has no TSF load path (the MT7612U: DW0/DW1 writes were swept through every - * order, with TIMER_EN toggled and with the MAC stopped, and the clock ignored - * all of them — docs/mt7612u.md) or that no standalone write is implemented - * (Jaguar1 moves its TSF only as part of the full beacon-steer sequence; the - * RTL8733B and Kestrel have no TSF write). True does not by itself prove the value landed - * byte-for-byte: the counter keeps running, so a caller that needs certainty - * should still read back — a successful write reads as target + the control - * round trip. */ + * Returns true when this backend drives a TSF write the part's hardware + * accepts, false otherwise (the default). False means "no standalone write + * here" — either the part has no load path or the write is not implemented. + * True does not by itself prove the value landed byte-for-byte: the counter + * keeps running, so a caller that needs certainty should still read back — a + * successful write reads as target + the control round trip. Per-backend + * state: docs/time-distribution.md. */ virtual bool WriteTsf(uint64_t tsf) { (void)tsf; return false; } /* Load a beacon into the beacon reserved-page + enable the MAC beacon function, diff --git a/src/jaguar1/RtlJaguarDevice.h b/src/jaguar1/RtlJaguarDevice.h index 60ee863d..dbe34f72 100644 --- a/src/jaguar1/RtlJaguarDevice.h +++ b/src/jaguar1/RtlJaguarDevice.h @@ -323,8 +323,11 @@ class RtlJaguarDevice : public IRtlRadio { /* No standalone TSF write. On this generation the TSF is moved only by the * complete beacon-steer sequence (see PinBeaconTbtt/AdjustBeaconTimingFine * below), which shifts the reported TSF and the TBTT grid together; a bare - * REG_TSFTR write was never shown to take on its own. Reporting false keeps - * "unsupported" honest rather than inheriting a silent no-op. */ + * REG_TSFTR write was never shown to take on its own. This method + * deliberately does not perform that sequence's EN_BCN_FUNCTION toggle and + * TBTT re-download: doing it here would move the beacon grid, which is a + * different operation than setting the counter. Reporting false keeps + * "unsupported here" honest rather than inheriting a silent no-op. */ bool WriteTsf(uint64_t tsf) override { (void)tsf; return false; } /* Hardware-timed beacon (IRadio contract): download the beacon MPDU to diff --git a/src/jaguar2/RtlJaguar2Device.cpp b/src/jaguar2/RtlJaguar2Device.cpp index 10091fbe..65368ceb 100644 --- a/src/jaguar2/RtlJaguar2Device.cpp +++ b/src/jaguar2/RtlJaguar2Device.cpp @@ -1966,9 +1966,12 @@ bool RtlJaguar2Device::WriteTsf(uint64_t tsf) { * Same register pair as Jaguar3; both are bench-proven to move the reported * TSF (RTL8822B readback: the target plus the control round trip). */ std::lock_guard lk(_reg_mu); - _device.rtw_write(0x0560, static_cast(tsf)); - _device.rtw_write(0x0564, static_cast(tsf >> 32)); - return true; + /* Both words are always attempted: true means both transfers landed; false + * means at least one did not, so the counter may be half-updated. The caller + * can read back, retry, or treat the write as failed. */ + const bool lo_ok = _device.rtw_write(0x0560, static_cast(tsf)); + const bool hi_ok = _device.rtw_write(0x0564, static_cast(tsf >> 32)); + return lo_ok && hi_ok; } void RtlJaguar2Device::Stop() { diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 6446f6fc..543fc62b 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -2219,9 +2219,12 @@ bool RtlJaguar3Device::WriteTsf(uint64_t tsf) { * the bench (RTL8822C): the reported TSF moves to the requested target plus * the control round trip. */ std::lock_guard lk(_reg_mu); - _device.rtw_write(0x0560, static_cast(tsf)); - _device.rtw_write(0x0564, static_cast(tsf >> 32)); - return true; + /* Both words are always attempted: true means both transfers landed; false + * means at least one did not, so the counter may be half-updated. The caller + * can read back, retry, or treat the write as failed. */ + const bool lo_ok = _device.rtw_write(0x0560, static_cast(tsf)); + const bool hi_ok = _device.rtw_write(0x0564, static_cast(tsf >> 32)); + return lo_ok && hi_ok; } bool RtlJaguar3Device::SetAckResponder(const devourer::MacAddr &mac) { diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 4fd2413a..e077a6f2 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -808,11 +808,9 @@ uint64_t Mt7612uRadio::ReadTsf() { } bool Mt7612uRadio::WriteTsf(uint64_t tsf) { - /* This part has no TSF load path: the DW0/DW1 registers do not latch the - * counter. Every plausible sequence was measured ignored on two units — both - * word orders, TIMER_EN cleared and restored, and the write issued with the - * MAC stopped (the bringup `tsfwrite` gate; docs/mt7612u.md). Reporting true - * here would dress a silent no-op as success, so it reports false. */ + /* No TSF load path on this part: every plausible sequence was measured + * ignored (the bringup `tsfwrite` gate; docs/mt7612u.md). Reporting true + * would dress a silent no-op as success, so it reports false. */ (void)tsf; return false; } diff --git a/src/mt7612u/include/mt7612u/mt7612u.h b/src/mt7612u/include/mt7612u/mt7612u.h index d0fb3c55..4df42c13 100644 --- a/src/mt7612u/include/mt7612u/mt7612u.h +++ b/src/mt7612u/include/mt7612u/mt7612u.h @@ -420,8 +420,8 @@ int mt7612u_link_stats(struct mt7612u_dev *dev, struct mt7612u_link_stats *out); */ int mt7612u_phy_tick(struct mt7612u_dev *dev); -/* TSF, the hardware microsecond clock. Two register reads. There is no writer: - * the DW0/DW1 registers do not load the counter (measured, see docs/mt7612u.md). */ +/* TSF, the hardware microsecond clock. Read only: there is no load path + * (measured, docs/mt7612u.md). */ uint64_t mt7612u_read_tsf(struct mt7612u_dev *dev); /* What this adapter can do, so a caller need not assume. */ diff --git a/src/mt7612u/tools/bringup.cpp b/src/mt7612u/tools/bringup.cpp index fcb87924..34e08fb6 100644 --- a/src/mt7612u/tools/bringup.cpp +++ b/src/mt7612u/tools/bringup.cpp @@ -2888,19 +2888,32 @@ static bool tsf_clock_alive(void) return alive; } -/* One load attempt. A take is the readback landing on the target; what the - * clock does afterwards is printed but does not decide, so a load that takes - * and then stalls is still a take. */ -static bool tsf_variant(const char *label, uint64_t target, bool stop_timer, - bool high_word_first, int which) +/* One load attempt. Returns 1 on a take, 0 on a no-op with the clock still + * running, and -1 when the arm produced no usable conclusion: a failed config + * read, or a counter that stopped during the arm. A stalled counter prints + * no-op and would otherwise let the gate PASS on a dead clock, so -1 is a + * failure of the run rather than a "the write was ignored" datum. A take is + * the readback landing on the target; what the clock does after a take does + * not decide, so a load that takes and then stalls is still a take. */ +static int tsf_variant(const char *label, uint64_t target, bool stop_timer, + bool high_word_first, int which) { - uint32_t cfg = mt_rr(&dev, MT_BEACON_TIME_CFG); + uint32_t cfg = 0; uint64_t r1, r2; int64_t err, rate; - bool took; - - if (stop_timer) + bool took, live; + const char *verdict; + + if (stop_timer) { + /* mt_rr returns 0xffffffff on a failed transfer, which must never be + * written back as configuration. If the read fails, skip the arm: the + * accumulated I/O error makes the gate FAIL at the sweep boundary. */ + if (mt_rr_chk(&dev, MT_BEACON_TIME_CFG, &cfg)) { + printf(" %-34s skipped: MT_BEACON_TIME_CFG read failed\n", label); + return -1; + } mt_wr(&dev, MT_BEACON_TIME_CFG, cfg & ~MT_BEACON_TIME_CFG_TIMER_EN); + } if (which == 0) { /* both words */ if (high_word_first) { @@ -2926,17 +2939,22 @@ static bool tsf_variant(const char *label, uint64_t target, bool stop_timer, err = (int64_t)(r1 - target); rate = (int64_t)(r2 - r1); took = llabs(err) < 200000; + live = rate > 10000 && rate < 200000; /* the control's wall-rate window */ + verdict = took ? "TOOK" : (live ? "no-op" : "no-op (clock stalled)"); printf(" %-34s target=%llu read=%llu err=%+lld delta50ms=%+lld %s\n", label, (unsigned long long)target, (unsigned long long)r1, - (long long)err, (long long)rate, took ? "TOOK" : "no-op"); - return took; + (long long)err, (long long)rate, verdict); + if (took) + return 1; + return live ? 0 : -1; } static int gate_tsfwrite(uint8_t chan) { uint64_t base; uint32_t cfg; - bool any = false; + bool any = false, invalid = false; + int r; if (mt_eeprom_init(&dev)) { printf("GATE TSF-WRITE: FAIL - eeprom_init failed\n"); @@ -2951,7 +2969,10 @@ static int gate_tsfwrite(uint8_t chan) return 1; } - cfg = mt_rr(&dev, MT_BEACON_TIME_CFG); + if (mt_rr_chk(&dev, MT_BEACON_TIME_CFG, &cfg)) { + printf("GATE TSF-WRITE: FAIL - MT_BEACON_TIME_CFG read failed\n"); + return 1; + } printf("MT_BEACON_TIME_CFG=0x%08x TIMER_EN=%u TBTT_EN=%u BEACON_TX=%u SYNC_MODE=%u\n", cfg, !!(cfg & MT_BEACON_TIME_CFG_TIMER_EN), !!(cfg & MT_BEACON_TIME_CFG_TBTT_EN), @@ -2968,31 +2989,49 @@ static int gate_tsfwrite(uint8_t chan) * then each word alone. The high-word-only target sets a high word that * differs from the live one, so the arm is not vacuous. */ if (mt_mac_start(&dev, MT_RX_DRAIN_NONE)) { + /* The start enables TX control before its DMA-idle poll, so stop + * before bailing out rather than leaving the MAC half-started. */ + mt_mac_stop(&dev); printf("\nGATE TSF-WRITE: FAIL - mt_mac_start failed\n"); return 1; } base = mt7612u_read_tsf(&dev); - any |= tsf_variant("MAC on, both words DW0,DW1", base + 5000000, false, false, 0); + r = tsf_variant("MAC on, both words DW0,DW1", base + 5000000, false, false, 0); + if (r > 0) any = true; + if (r < 0) invalid = true; base = mt7612u_read_tsf(&dev); - any |= tsf_variant("MAC on, both words DW1,DW0", base + 5000000, false, true, 0); + r = tsf_variant("MAC on, both words DW1,DW0", base + 5000000, false, true, 0); + if (r > 0) any = true; + if (r < 0) invalid = true; base = mt7612u_read_tsf(&dev); - any |= tsf_variant("MAC on, high word (DW1) only", base + (1ull << 32), false, false, 2); + r = tsf_variant("MAC on, high word (DW1) only", base + (1ull << 32), false, false, 2); + if (r > 0) any = true; + if (r < 0) invalid = true; base = mt7612u_read_tsf(&dev); - any |= tsf_variant("MAC on, low word (DW0) only", base + 5000000, false, false, 1); + r = tsf_variant("MAC on, low word (DW0) only", base + 5000000, false, false, 1); + if (r > 0) any = true; + if (r < 0) invalid = true; /* With the MAC stopped, and with the free-running timer disabled. */ mt_mac_stop(&dev); base = mt7612u_read_tsf(&dev); - any |= tsf_variant("MAC off, both words", base + 5000000, false, false, 0); + r = tsf_variant("MAC off, both words", base + 5000000, false, false, 0); + if (r > 0) any = true; + if (r < 0) invalid = true; base = mt7612u_read_tsf(&dev); - any |= tsf_variant("MAC off, timer off, both words", base + 5000000, true, false, 0); + r = tsf_variant("MAC off, timer off, both words", base + 5000000, true, false, 0); + if (r > 0) any = true; + if (r < 0) invalid = true; /* A mid-run transport failure reads as all-ones, which every arm would - * otherwise report as "ignored" — the exact false conclusion this gate - * exists to prevent. */ - if (mt_io_errors(&dev) != 0) { - printf("\nGATE TSF-WRITE: FAIL - %u USB transfer(s) failed during the sweep; no load conclusion\n", - mt_io_errors(&dev)); + * otherwise report as "ignored" - the exact false conclusion this gate + * exists to prevent. A stalled clock is the same class of false + * conclusion: no arm can be read as "the write was ignored" if the + * counter was not advancing while the arm ran. */ + if (invalid || mt_io_errors(&dev) != 0) { + printf("\nGATE TSF-WRITE: FAIL - %u USB transfer(s) failed%s; no load conclusion\n", + mt_io_errors(&dev), + invalid ? " and/or the clock stalled during an arm" : " during the sweep"); return 1; } From 45cf642abd764f3cf1b19cf99837addaa636e801 Mon Sep 17 00:00:00 2001 From: snokvist Date: Wed, 16 Sep 2026 17:14:31 +0200 Subject: [PATCH 4/4] IRadio: WriteTsf capability is static, Jaguar1 writes, PCIe reads back Review round from the maintainer: - Jaguar1 implements WriteTsf. The bare REG_TSFTR pair was measured to load on an RTL8821AU (raw pair, both word orders, no beacon armed), so the explicit false was a fake in the other direction. Under _port0_mu so it cannot interleave with a beacon-steer sequence. - The REG_TSFTR read/write moves into src/RtlTsf.h, shared by J1/J2/J3. A PCIe register write is a posted MMIO store that always reports success, so write_tsftr reads the counter back there; on USB it also reads back when the target's low word is close enough to wrap that the pair could tear between the two transfers. A throwing readback is false. - AdapterCaps::tsf_write_ok is the static half of the contract (emitted as adapter.caps tsf_write); the WriteTsf bool is only the per-call transport result. - The MT7612U drops its WriteTsf override (identical to the default) and records the measurement on tsf_write_ok = false instead. - The readback claim becomes a table of every recorded cell, with the sleep term, the source of each row, and the unmeasured dies named. The TBTT notes are scoped to J2/J3, with the J1 caveat. - bringup tsfwrite: every TSF read is checked (a failed read is a transport fault, not a dead clock); each arm waits out a low-word wrap before taking its base; an arm slower than its host-time bounds is inconclusive rather than "ignored"; the timer-off arm reads the counter while TIMER_EN is still clear; the arms are a table over a write-order enum sharing one named liveness window. PASS on two units, both bands. Co-Authored-By: Claude Opus 5 (1M context) --- docs/logging.md | 2 +- docs/mt7612u.md | 26 ++- docs/time-distribution.md | 56 +++++- examples/common/caps_event.h | 1 + src/AdapterCaps.h | 14 +- src/IRadio.h | 35 ++-- src/RtlTsf.h | 78 ++++++++ src/jaguar1/RtlJaguarDevice.cpp | 24 ++- src/jaguar1/RtlJaguarDevice.h | 15 +- src/jaguar2/RtlJaguar2Device.cpp | 26 +-- src/jaguar3/RtlJaguar3Device.cpp | 25 +-- src/mt7612u/Mt7612uRadio.cpp | 11 +- src/mt7612u/Mt7612uRadio.h | 1 - src/mt7612u/tools/bringup.cpp | 309 ++++++++++++++++++++++--------- tests/beacon_interval_shift.sh | 4 +- tests/radio_iface_selftest.cpp | 1 + 16 files changed, 448 insertions(+), 180 deletions(-) create mode 100644 src/RtlTsf.h diff --git a/docs/logging.md b/docs/logging.md index 50f671fc..0e35a8af 100644 --- a/docs/logging.md +++ b/docs/logging.md @@ -77,7 +77,7 @@ Emitters: L = library, RX/TX/... = demo. Optional fields in [brackets]; | ev | emitter | fields | |---|---|---| | `init.timing` | L (`src/InitTimer.h`) + demos | stage ("scope.stage", e.g. "demo.first_rx_frame", "txdemo.first_tx_submit"), ms | -| `adapter.caps` | RX, TX, doctor, txpower (`examples/common/caps_event.h`) | supported, chip, names, chip_id "0x..", gen, variant, transport, tx_chains, rx_chains, n_ss, stbc, ldpc, sgi, bw_max, bw[] (MHz), txpwr_max, txpwr_step_qdb, txpwr_step_measured, txpwr_min_qdb, txpwr_max_qdb, txpwr_rate_diffs, txpwr_rate_diffs_hw, txpwr_rate_diffs_measured, tune_2g4[]\|null, tune_5g[]\|null, char_2g4[]\|null, char_5g[]\|null, ldpc_rx_ht, ldpc_rx_vht, ldpc_rx_flag, per_pkt_txpwr, narrowband, fastretune, ack_responder, tx_retry_limit, he_er_su, per_chain_rssi | +| `adapter.caps` | RX, TX, doctor, txpower (`examples/common/caps_event.h`) | supported, chip, names, chip_id "0x..", gen, variant, transport, tx_chains, rx_chains, n_ss, stbc, ldpc, sgi, bw_max, bw[] (MHz), txpwr_max, txpwr_step_qdb, txpwr_step_measured, txpwr_min_qdb, txpwr_max_qdb, txpwr_rate_diffs, txpwr_rate_diffs_hw, txpwr_rate_diffs_measured, tune_2g4[]\|null, tune_5g[]\|null, char_2g4[]\|null, char_5g[]\|null, ldpc_rx_ht, ldpc_rx_vht, ldpc_rx_flag, vht_2g4, per_pkt_txpwr, per_pkt_txpwr_steps, per_pkt_txpwr_step_qdb, per_pkt_txpwr_min_qdb, per_pkt_txpwr_max_qdb, per_pkt_txpwr_measured, narrowband, fastretune, ack_responder, tx_retry_limit, he_er_su, per_chain_rssi, hw_rx_tsf, hw_beacon_txtsf, tsf_write, xtal_cap_max, xtal_cap_default | | `debug.wreg` | L (`DEVOURER_LOG_WRITES`) | addr "0x0nnn", width, val "0x…" | | `hop.prof` | L (`DEVOURER_HOP_PROF`) | gen, ch, `_us`…, total_us | | `tx.fail` | L (send failure; regress.py keys on it) | {status, actual_len, timeout} or {rc, timeout} | diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 62f84919..d9dcea1d 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -232,18 +232,26 @@ saturation. A-MPDU, not USB parallelism, is what lifted 34 → 44.55 Mbit/s. it produced a clock advancing 8.6e14 "µs" per 200 ms. - **No tested sequence loads the TSF.** `WriteTsf` is unsupported on this part: the DW0/DW1 registers hold the counter, they do not load it. Every sequence - the gate tries was measured ignored on two units — `DW0` then `DW1` and the - reverse, each word alone (with a high word that actually differs), - `MT_BEACON_TIME_CFG_TIMER_EN` cleared and restored around the write, and a - write with the MAC stopped — after a positive control confirms the clock is - alive, and with a per-arm check that the clock was still advancing. In each - case the clock kept free-running at wall rate and the value never moved. + the gate tries was measured ignored on two units (`40:a5:ef:50:27:a1` and + `40:a5:ef:5a:32:f8`, channels 6 and 149 each) — `DW0` then `DW1` and the + reverse, each word alone (with a high word that actually differs), a write + with the MAC stopped, and `MT_BEACON_TIME_CFG_TIMER_EN` cleared around the + write — after a positive control confirms the clock is alive, and with a + per-arm check that the clock was still advancing. In the running arms the + clock kept free-running at wall rate and never jumped to the target. The + timer-off arm needs its own read: clearing `TIMER_EN` zeroes the counter and + restoring it restarts the count from ~0, which would wipe a load before the + normal readback, so the gate also reads the counter while the timer is still + off — it reads 0, not the target. Each arm is also bounded in host time, so + a silently retried control transfer cannot turn a late readback of a real + load into a "no-op". Mainline mt76 registers no `.set_tsf` for the mt76x02 family, so the absence is a property of the part rather than of this port. The `bringup tsfwrite` gate runs that control and sweep and PASSes only while the clock is alive and - every sequence is confirmed ignored; `Mt7612uRadio::WriteTsf` therefore - returns false and issues no register write at all rather than pretending one - landed. + every sequence is confirmed ignored. The backend therefore reports + `AdapterCaps::tsf_write_ok = false` and does not override `WriteTsf`: the + `IRadio` default returns false and issues no register write at all, rather + than pretending one landed. - **Register-stream equivalence**: our EP0 write stream during bring-up was diffed against a `usbmon` capture of the kernel driver's own probe. 522 kernel writes vs 521 ours, 376 common addresses, one final-value mismatch diff --git a/docs/time-distribution.md b/docs/time-distribution.md index ca79739a..b9519167 100644 --- a/docs/time-distribution.md +++ b/docs/time-distribution.md @@ -205,16 +205,52 @@ actuator, and the µs-class UE path is a PCIe UE. Harness: **Steering the TBTT.** The actuator is `AdjustBeaconTiming(microseconds)`, not a TSF write: on Jaguar2/3, `WriteTsf` moves the reported TSF (and the beacon-body timestamp) but NOT the TBTT air-time — a separate per-port timer drives the -beacon, so the TBTT is deaf to `REG_TSFTR`. `WriteTsf` returns true on Jaguar3 and Jaguar2 (both readback-measured on the -bench: the reported TSF moves to the target plus the control round trip). It -returns false on the RTL8733B and Kestrel (no TSF write), on the MT7612U (its -DW0/DW1 registers do not load the counter — measured, `docs/mt7612u.md`), and -explicitly on Jaguar1: its TSF does move, but only as part of the full -beacon-steer sequence, whose `EN_BCN_FUNCTION` toggle and TBTT re-download the -standalone method deliberately does not perform (Jaguar1's TBTT is -hardware-locked to the TSF grid, so that sequence moves both; see the -`PinBeaconTbtt` per-generation notes). `false` therefore means "no standalone -write here", and an adoption loop gets the failure instead of a silent no-op. +beacon, so the TBTT is deaf to `REG_TSFTR`. (Jaguar1 is recorded the other +way — its TBTT is hardware-locked to the TSF grid, bench on all three dies, see +the `PinBeaconTbtt` per-generation notes below — so expect a J1 write to move an +active beacon's TBTT with it. Neither that nor whether the beacon keeps airing +without the steer's re-download has been measured through `WriteTsf`.) + +Whether a part has a standalone TSF write is `AdapterCaps::tsf_write_ok`; +`WriteTsf`'s bool is only the per-call transport result (on PCIe, a readback, +because an MMIO store has no completion — `src/RtlTsf.h`). `tsf_write_ok` is +true on Jaguar1, Jaguar2 and Jaguar3, which all load the bare `REG_TSFTR` pair. +Every recorded readback, each a ±5 s write read back after a host sleep (the +"bridge" rows are the devourer-mcp `radio_tsf` tool, whose readback follows the +write immediately; the "probe" rows are a scratch tool writing the raw pair +through a second `RtlAdapter`; the "timed" rows call `WriteTsf` itself with the +host clock bracketing each call): + +| die | path | sleep | readback error − sleep | +|---|---|---|---| +| RTL8821AU (J1), no beacon armed | probe, raw pair DW0→DW1 / DW1→DW0 | 20 ms | +1000 / +625 µs | +| RTL8822B (J2) | bridge, `WriteTsf` | 0 / 50 ms | +110 / **+3419** µs | +| RTL8822B (J2) | independent build, `WriteTsf` | 50 ms | +443 µs | +| RTL8822B (J2) | probe, raw pair DW0→DW1 / DW1→DW0 | 20 ms | +196 / +173 µs | +| RTL8812BU (J2, 8822B die), ch36 | timed, `WriteTsf` +5 s, n = 10 each | 0 / 20 / 50 ms | +98…+149 / +280…+881 / +186…+421 µs | +| RTL8812BU (J2, 8822B die), ch36 | timed, `WriteTsf` −5 s, n = 10 | 20 ms | +192…+676 µs | +| RTL8822C (J3) | bridge, `WriteTsf` | 0 ms | +1013 µs | +| RTL8822C (J3) | independent build, `WriteTsf` | 50 ms | +274 µs | + +So a write reads back as target + sleep + 0.1–3.4 ms. The timed rows say what +that residual is: on the 8812BU all 40 writes (forward and backward) returned +true and landed, the clock stayed at wall rate after each (50 ms re-read: ++50174…+51053 µs), and readback error minus the host time from `WriteTsf` +returning to `ReadTsf` returning was −485…+38 µs (medians −62…+20 µs). The +counter loads the target as the write completes and runs at wall rate from +there; the residual is host time — sleep overshoot plus the read's control +transfers — not a load offset. The 8822B-family rows total n = 45; the other +dies are n ≤ 2, a range rather than a characterised distribution, and the ++3419 µs bridge cell is the one outlier none of the timed rows reproduce. The Jaguar1 row is the raw register pair, which is exactly what +the J1 override writes, but the override itself has not run on J1 hardware. The +8812AU/8814AU, the 8821C (USB and the 8821CE) and the 8822E share the pair and +code path and are not separately measured. +`tsf_write_ok` is false on the MT7612U — its DW0/DW1 registers hold the +counter and do not load it (`bringup tsfwrite`, `docs/mt7612u.md`) — and on +Kestrel and the RTL8733B, where the source has no TSF write at all (a source +fact, not a bench measurement). There `WriteTsf` returns false without touching +a register, so an adoption loop gets a refusal instead of a silent no-op. + A one-shot beacon-interval tweak *does* steer the J2/J3 TBTT: running one interval at (nominal ± Δ) TU then restoring advances/retards the next TBTT — diff --git a/examples/common/caps_event.h b/examples/common/caps_event.h index 97583ce1..d8d5f370 100644 --- a/examples/common/caps_event.h +++ b/examples/common/caps_event.h @@ -87,6 +87,7 @@ inline void emit_adapter_caps(EventSink &sink, IRadio *dev) { .f("per_chain_rssi", c.per_chain_rssi ? 1 : 0) .f("hw_rx_tsf", c.hw_rx_timestamp ? 1 : 0) .f("hw_beacon_txtsf", c.hw_beacon_txtsf ? 1 : 0) + .f("tsf_write", c.tsf_write_ok ? 1 : 0) .f("xtal_cap_max", c.xtal_cap_max) .f("xtal_cap_default", c.xtal_cap_default); } diff --git a/src/AdapterCaps.h b/src/AdapterCaps.h index 4de388b2..80ffab0e 100644 --- a/src/AdapterCaps.h +++ b/src/AdapterCaps.h @@ -276,9 +276,21 @@ struct AdapterCaps { * (a genuine sub-µs TX-egress timestamp a receiver reads via * Packet::TxEgressTsf) — rides the hardware beacon function (StartBeacon); * true on all generations. Together they are the primitives for one-way - * hardware time distribution (see TsfSync). */ + * hardware time distribution (see TsfSync). tsf_write_ok: IRadio::WriteTsf + * drives a standalone write the part's counter loads (the static half of the + * WriteTsf contract; its bool return is the per-call transport result). + * Readback-measured through WriteTsf: 8822B (Jaguar2; incl. an RTL8812BU, + * 40/40 forward and backward writes landed), 8822C (Jaguar3). The + * 8821AU (Jaguar1) is measured on the raw REG_TSFTR pair with a scratch probe + * (both word orders, no beacon armed), which is exactly what the Jaguar1 + * WriteTsf writes; the override itself has not run on Jaguar1 hardware. The + * 8812A/8814A, 8821C (USB and PCIe) and 8822E ride the same pair and code + * path and are not separately measured. FALSE on the MT7612U (measured: its DW0/DW1 registers + * do not load the counter, docs/mt7612u.md), and on Kestrel and the RTL8733B + * (no TSF write in the source — not a bench fact). */ bool hw_rx_timestamp = false; bool hw_beacon_txtsf = false; + bool tsf_write_ok = false; /* 802.11ax scheduled UL (Kestrel/RTL8852 only). trigger_ul_ok: the adapter * can air an HE Trigger frame (UL-OFDMA grant) and program the fw UL-OFDMA * scheduler (SendTrigger / ConfigureUlOfdma). twt_ok: the fw exposes the TWT diff --git a/src/IRadio.h b/src/IRadio.h index b40f3da8..aa117ea3 100644 --- a/src/IRadio.h +++ b/src/IRadio.h @@ -363,19 +363,28 @@ class IRadio { * — the primitive for TSF *adoption* (a slave slewing its clock onto the * master's, so its per-frame `tsfl` reads in the master's timebase). The * counter keeps running, so a read-add-write shifts by an approximate delta (a - * control loop absorbs the read→write latency). NOTE: this moves the reported - * TSF (and the beacon-body timestamp) but NOT the beacon TBTT air-time — a - * separate per-port timer drives the TBTT (bench-proven). To steer the - * hardware-timed beacon (the uplink timing-advance actuator) use + * control loop absorbs the read→write latency). NOTE: on Jaguar2/3 this moves + * the reported TSF (and the beacon-body timestamp) but NOT the beacon TBTT + * air-time — a separate per-port timer drives the TBTT (bench-proven). On + * Jaguar1 the TBTT is recorded as hardware-locked to the TSF grid + * (PinBeaconTbtt, bench on all three dies), so expect a write there to move + * an active beacon's TBTT with it; neither that nor whether the beacon keeps + * airing without the steer's re-download was measured through this call. To + * steer the hardware-timed beacon (the uplink timing-advance actuator) use * AdjustBeaconTiming. * - * Returns true when this backend drives a TSF write the part's hardware - * accepts, false otherwise (the default). False means "no standalone write - * here" — either the part has no load path or the write is not implemented. - * True does not by itself prove the value landed byte-for-byte: the counter - * keeps running, so a caller that needs certainty should still read back — a - * successful write reads as target + the control round trip. Per-backend - * state: docs/time-distribution.md. */ + * Two questions, two answers. Whether this part HAS a standalone TSF write is + * static: AdapterCaps::tsf_write_ok, resolved at construction — check it once + * rather than inferring it from a return value. The return value is per call: + * true when the transport accepted the write (backend-specific; the Realtek + * rule, including the PCIe readback, is devourer::write_tsftr in + * src/RtlTsf.h), false when it did not OR when tsf_write_ok is false (the + * default here). So on a tsf_write_ok part, false is a transport failure and + * the counter may be half-updated: read back, and retry only while the device + * is still present - a device that has gone reports false on every call. True + * is not a byte-for-byte proof either: the counter keeps running, so a caller + * that needs certainty reads back. Per-backend state and the measured readbacks: + * docs/time-distribution.md. */ virtual bool WriteTsf(uint64_t tsf) { (void)tsf; return false; } /* Load a beacon into the beacon reserved-page + enable the MAC beacon function, @@ -452,8 +461,8 @@ class IRadio { * REG_BCN_INTERVAL tweak: runs one beacon interval at (nominal + round(µs/1024)) * TU then restores nominal, so the next TBTT — and the cadence thereafter — * shifts by that many TU. This is the beacon-timing / uplink timing-advance - * actuator: WriteTsf moves the reported TSF but NOT the TBTT air-time (a - * separate per-port timer drives it), whereas the interval tweak steers it + * actuator: on Jaguar2/3 WriteTsf moves the reported TSF but NOT the TBTT + * air-time (a separate per-port timer drives it), whereas the interval tweak steers it * deterministically (the 802.11 IBSS/TSF-merge mechanism; bench-proven to the * microsecond). Requires an active StartBeacon. BLOCKS the caller ~one beacon * interval (the tweaked interval must latch and fire once before restore). diff --git a/src/RtlTsf.h b/src/RtlTsf.h new file mode 100644 index 00000000..f5eab0d0 --- /dev/null +++ b/src/RtlTsf.h @@ -0,0 +1,78 @@ +#ifndef DEVOURER_RTL_TSF_H +#define DEVOURER_RTL_TSF_H + +#include +#include + +#include "RtlAdapter.h" + +/* REG_TSFTR — the port-0 MAC TSF on the Realtek generations that expose it + * as a plain register pair: 0x0560 (low 32) / 0x0564 (high 32). Shared by the + * Jaguar1/2/3 ReadTsf/WriteTsf overrides so the read discipline and the + * write's success rule live in one place. These helpers take no lock; each + * backend serializes as it needs (Jaguar2/3 hold _reg_mu against their coex + * tick, Jaguar1 holds _port0_mu for the write so it cannot interleave with a + * beacon steer). A lock keeps the two words atomic against other register + * users, not a caller's read-compute-write: a write that waits behind a long + * sequence lands its precomputed value that much late. */ + +namespace devourer { + +/* hi, lo, hi again, and retry the pair once if the low word wrapped between + * the reads. */ +inline uint64_t read_tsftr(RtlAdapter &a) { + uint32_t hi = a.rtw_read(0x0564); + uint32_t lo = a.rtw_read(0x0560); + if (a.rtw_read(0x0564) != hi) { + hi = a.rtw_read(0x0564); + lo = a.rtw_read(0x0560); + } + return (static_cast(hi) << 32) | lo; +} + +/* How far past the target a PCIe readback may land and still count as the + * write having taken. The MMIO store-then-read round trip is µs-scale; the + * slack only absorbs a host preemption between the two. */ +inline constexpr uint64_t kTsfWriteReadbackWindowUs = 100000; + +/* Load the counter. Both words are always attempted, low then high: the pair + * is the unit, and a half-written TSF is worse than a reported failure. + * + * True means the transport accepted both writes, and on USB that alone is two + * completed control transfers - with one exception. The counter runs between + * the two transfers, so a target whose low word is within the window of + * wrapping can carry into the high word before the high write lands, leaving + * target - 2^32 behind two successful transfers. That case is read back. + * + * A PCIe register write is a posted MMIO store with no completion + * (PcieTransport::guarded_write reports true for any register below the USB + * page, whether or not the device is still there), so on PCIe every write is + * read back: true only when the counter reads within kTsfWriteReadbackWindowUs + * after the target. A device that has left the bus typically reads all-ones + * and fails that (platform behaviour, not measured here). A readback cannot + * tell "ignored" from "took" when the target sits less than the window behind + * the live clock (an ignored small backward nudge still reads as target + a + * little), so it catches a dead transport or a torn pair, not an ignored + * nudge. A USB register read throws on a failed transfer where a write + * returns false; the readback folds that into false so the bool stays the + * whole answer. */ +inline bool write_tsftr(RtlAdapter &a, uint64_t tsf) { + const bool lo_ok = a.rtw_write(0x0560, static_cast(tsf)); + const bool hi_ok = + a.rtw_write(0x0564, static_cast(tsf >> 32)); + if (!(lo_ok && hi_ok)) + return false; + const bool near_wrap = static_cast(tsf) > + UINT32_MAX - kTsfWriteReadbackWindowUs; + if (a.is_usb() && !near_wrap) + return true; + try { + return read_tsftr(a) - tsf < kTsfWriteReadbackWindowUs; + } catch (const std::exception &) { + return false; + } +} + +} // namespace devourer + +#endif /* DEVOURER_RTL_TSF_H */ diff --git a/src/jaguar1/RtlJaguarDevice.cpp b/src/jaguar1/RtlJaguarDevice.cpp index 933a78c5..10af4d9c 100644 --- a/src/jaguar1/RtlJaguarDevice.cpp +++ b/src/jaguar1/RtlJaguarDevice.cpp @@ -6,6 +6,7 @@ #include "Hal8812PhyReg.h" #include "NhmReader.h" #include "NoiseFloorMath.h" /* active idle-noise-floor sign/pwdb helpers */ +#include "RtlTsf.h" /* REG_TSFTR read/write shared with Jaguar2/3 */ #include "RadioManagementModule.h" #include "AckResponder.h" /* hardware ACK responder recipe */ #include "RadiotapPeek.h" /* send_packets batch pre-parse */ @@ -441,15 +442,19 @@ bool RtlJaguarDevice::GetPermanentMacAddress(uint8_t out[6]) { } uint64_t RtlJaguarDevice::ReadTsf() { - /* REG_TSFTR (0x0560) = TSF low 32, 0x0564 = TSF high 32. Read hi, lo, hi - * again and retry the pair once if the low word wrapped between the reads. */ - uint32_t hi = _device.rtw_read(0x0564); - uint32_t lo = _device.rtw_read(0x0560); - if (_device.rtw_read(0x0564) != hi) { - hi = _device.rtw_read(0x0564); - lo = _device.rtw_read(0x0560); - } - return (static_cast(hi) << 32) | lo; + return devourer::read_tsftr(_device); +} + +bool RtlJaguarDevice::WriteTsf(uint64_t tsf) { + /* The bare REG_TSFTR pair, without the beacon-steer bracket. Under _port0_mu + * so it cannot interleave with a PinBeaconTbtt/AdjustBeaconTimingFine + * sequence, which writes the same pair (PinBeaconTbtt's restore step is + * this same bare write). The raw pair is readback-measured on an RTL8821AU + * (scratch probe, no beacon armed, both word orders); this override has not + * itself run on Jaguar1 hardware, and the 8812AU/8814AU share the register + * block without a separate measurement. Success rule: devourer::write_tsftr. */ + std::lock_guard lock(_port0_mu); + return devourer::write_tsftr(_device, tsf); } bool RtlJaguarDevice::download_rsvd_beacon(const uint8_t *mpdu, @@ -2158,6 +2163,7 @@ devourer::AdapterCaps RtlJaguarDevice::GetAdapterCaps() { c.hw_beacon_txtsf = true; /* StartBeacon: MAC inserts the egress TSF into * beacons (bench: 8821AU + 8814AU body-TS steps * live at the beacon interval) */ + c.tsf_write_ok = true; /* WriteTsf: bare REG_TSFTR (8821AU readback) */ c.xtal_cap_max = 0x3f; /* 6-bit AFE crystal-cap trim (0x2C) */ c.xtal_cap_default = _eepromManager->crystal_cap & 0x3f; devourer::set_standard_freq_ranges(c); diff --git a/src/jaguar1/RtlJaguarDevice.h b/src/jaguar1/RtlJaguarDevice.h index dbe34f72..27892597 100644 --- a/src/jaguar1/RtlJaguarDevice.h +++ b/src/jaguar1/RtlJaguarDevice.h @@ -320,15 +320,12 @@ class RtlJaguarDevice : public IRtlRadio { * so this is a lookup, not a chip access. */ bool GetPermanentMacAddress(uint8_t out[6]) override; uint64_t ReadTsf() override; - /* No standalone TSF write. On this generation the TSF is moved only by the - * complete beacon-steer sequence (see PinBeaconTbtt/AdjustBeaconTimingFine - * below), which shifts the reported TSF and the TBTT grid together; a bare - * REG_TSFTR write was never shown to take on its own. This method - * deliberately does not perform that sequence's EN_BCN_FUNCTION toggle and - * TBTT re-download: doing it here would move the beacon grid, which is a - * different operation than setting the counter. Reporting false keeps - * "unsupported here" honest rather than inheriting a silent no-op. */ - bool WriteTsf(uint64_t tsf) override { (void)tsf; return false; } + /* The bare REG_TSFTR write, without the beacon-steer bracket. The TBTT here + * is recorded as locked to the TSF grid (PinBeaconTbtt below), so expect an + * active beacon's TBTT to move with the write; that, and whether the beacon + * keeps airing without the steer's re-download, is unmeasured through this + * call. */ + bool WriteTsf(uint64_t tsf) override; /* Hardware-timed beacon (IRadio contract): download the beacon MPDU to * the reserved page at the BCNQ boundary (the vendor rtl8812_download_rsvd_page diff --git a/src/jaguar2/RtlJaguar2Device.cpp b/src/jaguar2/RtlJaguar2Device.cpp index 65368ceb..57c52cc2 100644 --- a/src/jaguar2/RtlJaguar2Device.cpp +++ b/src/jaguar2/RtlJaguar2Device.cpp @@ -27,6 +27,7 @@ #include "NhmReader.h" #include "ToneMask.h" #include "RateDefinitions.h" +#include "RtlTsf.h" /* REG_TSFTR read/write shared with Jaguar1/3 */ #include "RxPacket.h" #include "SignalStop.h" /* g_devourer_should_stop */ extern "C" { @@ -1187,6 +1188,7 @@ devourer::AdapterCaps RtlJaguar2Device::GetAdapterCaps() { c.tx_retry_limit_ok = _variant == jaguar2::ChipVariant::C8822B; c.hw_rx_timestamp = true; /* FrameParserJaguar2 fills RxAtrib.tsfl */ c.hw_beacon_txtsf = true; /* StartBeacon: MAC inserts the egress TSF into beacons */ + c.tsf_write_ok = true; /* WriteTsf: REG_TSFTR (8822B readback) */ c.xtal_cap_max = 0x3f; /* 6-bit AFE crystal-cap trim (0x24/0x28) */ c.xtal_cap_default = _hal.efuse_logical_byte(0xB9) == 0xFF ? 0x20 @@ -1951,27 +1953,17 @@ uint64_t RtlJaguar2Device::ReadTsf() { * _reg_mu (shared with the coex/thermal tick). NB starved to 0 under a heavy * RX bulk-IN flood — reliable from a quiet TX. */ std::lock_guard lk(_reg_mu); - uint32_t hi = _device.rtw_read(0x0564); - uint32_t lo = _device.rtw_read(0x0560); - if (_device.rtw_read(0x0564) != hi) { - hi = _device.rtw_read(0x0564); - lo = _device.rtw_read(0x0560); - } - return (static_cast(hi) << 32) | lo; + return devourer::read_tsftr(_device); } bool RtlJaguar2Device::WriteTsf(uint64_t tsf) { - /* REG_TSFTR 0x0560 (low) / 0x0564 (high). Serialized on _reg_mu against the - * coex/thermal tick. The counter keeps running, so this sets it to ~tsf. - * Same register pair as Jaguar3; both are bench-proven to move the reported - * TSF (RTL8822B readback: the target plus the control round trip). */ + /* REG_TSFTR, serialized on _reg_mu against the coex/thermal tick. The + * counter keeps running, so this sets it to ~tsf. Readback-measured on the + * RTL8822B over USB; the 8821C (USB and the 8821CE's PCIe) rides the same + * pair and is not separately measured. Success rule, including the PCIe + * readback: devourer::write_tsftr. */ std::lock_guard lk(_reg_mu); - /* Both words are always attempted: true means both transfers landed; false - * means at least one did not, so the counter may be half-updated. The caller - * can read back, retry, or treat the write as failed. */ - const bool lo_ok = _device.rtw_write(0x0560, static_cast(tsf)); - const bool hi_ok = _device.rtw_write(0x0564, static_cast(tsf >> 32)); - return lo_ok && hi_ok; + return devourer::write_tsftr(_device, tsf); } void RtlJaguar2Device::Stop() { diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 543fc62b..98c07167 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -21,6 +21,7 @@ #include "FrameParserJaguar3.h" #include "NhmReader.h" /* frame-free NHM power histogram (shared) */ #include "RateDefinitions.h" /* MGN_* rate enum (shared across the family) */ +#include "RtlTsf.h" /* REG_TSFTR read/write shared with Jaguar1/2 */ #include "SignalStop.h" /* g_devourer_should_stop — set by demo signal handlers */ #include "ToneMask.h" /* DEVOURER_RX_CSI_MASK / DEVOURER_RX_NBI knobs */ @@ -1645,6 +1646,7 @@ devourer::AdapterCaps RtlJaguar3Device::GetAdapterCaps() { c.narrowband_ok = true; /* 5/10 MHz baseband re-clock — Jaguar3 only */ c.hw_rx_timestamp = true; /* FrameParserJaguar3 fills RxAtrib.tsfl */ c.hw_beacon_txtsf = true; /* StartBeacon: MAC inserts the egress TSF into beacons */ + c.tsf_write_ok = true; /* WriteTsf: REG_TSFTR (8822C readback) */ c.xtal_cap_max = 0x7f; /* 7-bit AFE crystal-cap trim (0x1040) */ c.xtal_cap_default = 0x20; /* LDPC RX: both variants decode HT+VHT LDPC (bench: encoding-matrix @@ -2204,27 +2206,16 @@ uint64_t RtlJaguar3Device::ReadTsf() { * _reg_mu (shared with the coex runtime thread). Starved to 0 under a heavy * RX bulk-IN flood — reliable from a quiet TX. */ std::lock_guard lk(_reg_mu); - uint32_t hi = _device.rtw_read(0x0564); - uint32_t lo = _device.rtw_read(0x0560); - if (_device.rtw_read(0x0564) != hi) { - hi = _device.rtw_read(0x0564); - lo = _device.rtw_read(0x0560); - } - return (static_cast(hi) << 32) | lo; + return devourer::read_tsftr(_device); } bool RtlJaguar3Device::WriteTsf(uint64_t tsf) { - /* REG_TSFTR 0x0560 (low) / 0x0564 (high). Serialized on _reg_mu against the - * coex tick. The counter keeps running, so this sets it to ~tsf. Measured on - * the bench (RTL8822C): the reported TSF moves to the requested target plus - * the control round trip. */ + /* REG_TSFTR, serialized on _reg_mu against the coex tick. The counter keeps + * running, so this sets it to ~tsf. Readback-measured on the RTL8822C; the + * 8822E rides the same pair and is not separately measured. Success rule: + * devourer::write_tsftr. */ std::lock_guard lk(_reg_mu); - /* Both words are always attempted: true means both transfers landed; false - * means at least one did not, so the counter may be half-updated. The caller - * can read back, retry, or treat the write as failed. */ - const bool lo_ok = _device.rtw_write(0x0560, static_cast(tsf)); - const bool hi_ok = _device.rtw_write(0x0564, static_cast(tsf >> 32)); - return lo_ok && hi_ok; + return devourer::write_tsftr(_device, tsf); } bool RtlJaguar3Device::SetAckResponder(const devourer::MacAddr &mac) { diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index e077a6f2..5a6e358b 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -807,14 +807,6 @@ uint64_t Mt7612uRadio::ReadTsf() { return _dev ? mt7612u_read_tsf(_dev) : 0; } -bool Mt7612uRadio::WriteTsf(uint64_t tsf) { - /* No TSF load path on this part: every plausible sequence was measured - * ignored (the bringup `tsfwrite` gate; docs/mt7612u.md). Reporting true - * would dress a silent no-op as success, so it reports false. */ - (void)tsf; - return false; -} - devourer::TxStats Mt7612uRadio::GetTxStats() { devourer::TxStats out{}; @@ -1068,6 +1060,9 @@ devourer::AdapterCaps Mt7612uRadio::GetAdapterCaps() { * True since the beacon plane landed - it read false while the function it * describes sat three hundred lines above. */ c.hw_beacon_txtsf = true; + /* No TSF load path: every write sequence the bringup `tsfwrite` gate tries + * is ignored (docs/mt7612u.md), so WriteTsf stays on the IRadio default. */ + c.tsf_write_ok = false; /* Measured on air: 0 frames at the stimulus radio unarmed, 3500+ armed. */ c.ack_responder_ok = true; /* Unmeasured, so false rather than optimistic - nothing here drives the diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h index bc54d3cf..7c6fc3e4 100644 --- a/src/mt7612u/Mt7612uRadio.h +++ b/src/mt7612u/Mt7612uRadio.h @@ -97,7 +97,6 @@ class Mt7612uRadio : public IRadio { bool SetAmpduMode(const devourer::AmpduMode &mode) override; bool GetPermanentMacAddress(uint8_t out[6]) override; uint64_t ReadTsf() override; - bool WriteTsf(uint64_t tsf) override; devourer::TxStats GetTxStats() override; bool SetAckResponder(const devourer::MacAddr &mac) override; bool StartBeacon(const uint8_t *beacon, size_t len, int interval_tu) override; diff --git a/src/mt7612u/tools/bringup.cpp b/src/mt7612u/tools/bringup.cpp index 34e08fb6..065afa0d 100644 --- a/src/mt7612u/tools/bringup.cpp +++ b/src/mt7612u/tools/bringup.cpp @@ -2865,96 +2865,258 @@ static int gate_rtap(uint8_t chan, int count) /* Gate TSF-WRITE: characterises whether this part has a TSF load path at all. * The DW0/DW1 registers hold the free-running counter and do not load it: - * every sequence tried below was ignored on two units, with the clock first - * verified alive (a dead or wedged counter would also read as "no load path"). - * That measurement is why Mt7612uRadio::WriteTsf reports false. A future - * firmware that enables loading must fail this gate so the contract is - * revisited. Every arm builds its target from a fresh read so a stale write - * cannot look like a take. */ -static bool tsf_clock_alive(void) + * every sequence tried below was ignored on two units (docs/mt7612u.md), with + * the clock first verified alive (a dead or wedged counter would also read as + * "no load path"). + * That measurement is why this backend reports AdapterCaps::tsf_write_ok false + * and leaves WriteTsf on the refusing IRadio default. A future firmware that + * enables loading must fail this gate so the contract is revisited. Every arm + * builds its target from a fresh read so a stale write cannot look like a + * take. */ + +/* The wall-rate window the clock control and every arm's liveness check + * share: a kTsfLiveSleepUs sleep must advance the counter by more than + * kTsfLiveMinUs and less than kTsfLiveMaxUs. */ +static const unsigned kTsfLiveSleepUs = 50000; +static const int64_t kTsfLiveMinUs = 10000; +static const int64_t kTsfLiveMaxUs = 200000; + +/* How far from its target a readback may land and still count as a take: the + * 20 ms settle plus control round trips, with margin. */ +static const int64_t kTsfTakeWindowUs = 200000; + +/* How close to a low-word wrap an arm may start: the 5 s target plus the arm's + * own time, which kTsfArmMaxMs bounds, with margin. */ +static const uint64_t kTsfWrapGuardUs = 6000000; + +/* Host-time bounds that keep an arm's verdict meaningful. mt_vendor_req + * retries a timed-out EP0 transfer silently (no io_err on eventual success), + * so a stall between a real load and the readback could push the error past + * kTsfTakeWindowUs and print no-op on a part that loads - and a slow enough arm + * could outlast the wrap guard. An arm slower than either bound is + * inconclusive, never "ignored". */ +static const double kTsfWriteToReadbackMaxMs = 150.0; /* < kTsfTakeWindowUs */ +static const double kTsfArmMaxMs = 1000.0; /* << kTsfWrapGuardUs */ + +static bool tsf_live(int64_t delta) { - uint64_t t0 = mt7612u_read_tsf(&dev); - uint64_t t1; + return delta > kTsfLiveMinUs && delta < kTsfLiveMaxUs; +} + +/* A checked TSF read: DW1, DW0, DW1 again with one retry if the low word + * wrapped in between, every word through mt_rr_chk. mt7612u_read_tsf() goes + * through mt_rr, which reports a failed transfer as all-ones - two of those + * read as a stopped clock, and a flaky cable would be reported as a dead + * timer. False means a transfer failed and *out is not a TSF. */ +static bool tsf_read_chk(uint64_t *out) +{ + uint32_t hi, lo, hi2; + + if (mt_rr_chk(&dev, MT_TSF_TIMER_DW1, &hi) || + mt_rr_chk(&dev, MT_TSF_TIMER_DW0, &lo) || + mt_rr_chk(&dev, MT_TSF_TIMER_DW1, &hi2)) + return false; + if (hi2 != hi) { + hi = hi2; + if (mt_rr_chk(&dev, MT_TSF_TIMER_DW0, &lo)) + return false; + } + *out = ((uint64_t)hi << 32) | lo; + return true; +} + +/* A fresh base for one arm, clear of a low-word wrap. Each arm judges a take + * with a 64-bit compare against base + offset. Near a wrap that compare lies + * in both directions: a DW0-only load whose 5 s target carries into DW1 reads + * back ~2^32 short and prints no-op (a false PASS), and a natural carry + * between the base read and the readback makes the DW1-only arm read as a + * take. Waiting out the last kTsfWrapGuardUs before a wrap removes both. */ +static bool tsf_fresh_base(uint64_t *base) +{ + uint32_t lo; + + if (!tsf_read_chk(base)) + return false; + lo = (uint32_t)*base; + if (lo > 0xffffffffull - kTsfWrapGuardUs) { + mt_usleep((unsigned)(0x100000000ull - lo) + 100000); + if (!tsf_read_chk(base)) + return false; + } + return true; +} + +/* 1 = alive, 0 = the counter is not advancing, -1 = a read failed (the + * transport, not the clock). */ +static int tsf_clock_control(void) +{ + uint64_t t0, t1; int64_t d; - bool alive; - mt_usleep(50000); - t1 = mt7612u_read_tsf(&dev); + if (!tsf_read_chk(&t0)) + return -1; + mt_usleep(kTsfLiveSleepUs); + if (!tsf_read_chk(&t1)) + return -1; d = (int64_t)(t1 - t0); - alive = d > 10000 && d < 200000; /* ~50 ms at the wall rate */ printf(" %-34s t0=%llu t1=%llu delta=%+lld %s\n", "clock control", (unsigned long long)t0, (unsigned long long)t1, - (long long)d, alive ? "ALIVE" : "DEAD"); - return alive; + (long long)d, tsf_live(d) ? "ALIVE" : "DEAD"); + return tsf_live(d) ? 1 : 0; } +enum tsf_write_order { + TSF_DW0_THEN_DW1, + TSF_DW1_THEN_DW0, + TSF_DW0_ONLY, + TSF_DW1_ONLY, +}; + +struct tsf_arm { + const char *label; + enum tsf_write_order order; + uint64_t offset; /* target = fresh base + offset */ + /* Clear MT_BEACON_TIME_CFG_TIMER_EN around the write. Restoring TIMER_EN + * restarts the counter from ~0, which would wipe a load before the normal + * readback, so this arm also reads the counter while the timer is still + * off and judges the take on that read too. */ + bool stop_timer; +}; + /* One load attempt. Returns 1 on a take, 0 on a no-op with the clock still - * running, and -1 when the arm produced no usable conclusion: a failed config - * read, or a counter that stopped during the arm. A stalled counter prints - * no-op and would otherwise let the gate PASS on a dead clock, so -1 is a - * failure of the run rather than a "the write was ignored" datum. A take is - * the readback landing on the target; what the clock does after a take does - * not decide, so a load that takes and then stalls is still a take. */ -static int tsf_variant(const char *label, uint64_t target, bool stop_timer, - bool high_word_first, int which) + * running, and -1 when the arm produced no usable conclusion: a failed read, + * or a counter that stopped during the arm. A stalled counter prints no-op and + * would otherwise let the gate PASS on a dead clock, so -1 is a failure of the + * run rather than a "the write was ignored" datum. A take is the readback + * landing on the target; what the clock does after a take does not decide, so + * a load that takes and then stalls is still a take. */ +static int tsf_arm_run(const struct tsf_arm *a) { uint32_t cfg = 0; - uint64_t r1, r2; + uint64_t base, target, r1, r2, held = 0; int64_t err, rate; - bool took, live; - const char *verdict; + double t_base, t_write, t_r1, t_end; + bool took, live, slow; - if (stop_timer) { + if (!tsf_fresh_base(&base)) { + printf(" %-34s skipped: TSF read failed\n", a->label); + return -1; + } + t_base = now_ms(); + target = base + a->offset; + + if (a->stop_timer) { /* mt_rr returns 0xffffffff on a failed transfer, which must never be * written back as configuration. If the read fails, skip the arm: the * accumulated I/O error makes the gate FAIL at the sweep boundary. */ if (mt_rr_chk(&dev, MT_BEACON_TIME_CFG, &cfg)) { - printf(" %-34s skipped: MT_BEACON_TIME_CFG read failed\n", label); + printf(" %-34s skipped: MT_BEACON_TIME_CFG read failed\n", a->label); return -1; } mt_wr(&dev, MT_BEACON_TIME_CFG, cfg & ~MT_BEACON_TIME_CFG_TIMER_EN); } - if (which == 0) { /* both words */ - if (high_word_first) { - mt_wr(&dev, MT_TSF_TIMER_DW1, (uint32_t)(target >> 32)); - mt_wr(&dev, MT_TSF_TIMER_DW0, (uint32_t)target); - } else { - mt_wr(&dev, MT_TSF_TIMER_DW0, (uint32_t)target); - mt_wr(&dev, MT_TSF_TIMER_DW1, (uint32_t)(target >> 32)); - } - } else if (which == 1) { /* low word only */ + t_write = now_ms(); + switch (a->order) { + case TSF_DW0_THEN_DW1: + mt_wr(&dev, MT_TSF_TIMER_DW0, (uint32_t)target); + mt_wr(&dev, MT_TSF_TIMER_DW1, (uint32_t)(target >> 32)); + break; + case TSF_DW1_THEN_DW0: + mt_wr(&dev, MT_TSF_TIMER_DW1, (uint32_t)(target >> 32)); mt_wr(&dev, MT_TSF_TIMER_DW0, (uint32_t)target); - } else { /* high word only */ + break; + case TSF_DW0_ONLY: + mt_wr(&dev, MT_TSF_TIMER_DW0, (uint32_t)target); + break; + case TSF_DW1_ONLY: mt_wr(&dev, MT_TSF_TIMER_DW1, (uint32_t)(target >> 32)); + break; } - if (stop_timer) + if (a->stop_timer) { + bool held_ok = tsf_read_chk(&held); + + /* Restore before judging, so a failed read cannot leave the + * timer off. */ mt_wr(&dev, MT_BEACON_TIME_CFG, cfg); + if (!held_ok) { + printf(" %-34s TSF read with the timer off failed\n", a->label); + return -1; + } + } mt_usleep(20000); - r1 = mt7612u_read_tsf(&dev); - mt_usleep(50000); - r2 = mt7612u_read_tsf(&dev); + if (!tsf_read_chk(&r1)) { + printf(" %-34s TSF readback failed\n", a->label); + return -1; + } + t_r1 = now_ms(); + mt_usleep(kTsfLiveSleepUs); + if (!tsf_read_chk(&r2)) { + printf(" %-34s TSF liveness read failed\n", a->label); + return -1; + } + t_end = now_ms(); err = (int64_t)(r1 - target); rate = (int64_t)(r2 - r1); - took = llabs(err) < 200000; - live = rate > 10000 && rate < 200000; /* the control's wall-rate window */ - verdict = took ? "TOOK" : (live ? "no-op" : "no-op (clock stalled)"); + took = llabs(err) < kTsfTakeWindowUs || + (a->stop_timer && llabs((int64_t)(held - target)) < kTsfTakeWindowUs); + live = tsf_live(rate); + slow = t_r1 - t_write > kTsfWriteToReadbackMaxMs || + t_end - t_base > kTsfArmMaxMs; + if (a->stop_timer) + printf(" %-34s held=%llu (read with TIMER_EN clear)\n", "", + (unsigned long long)held); printf(" %-34s target=%llu read=%llu err=%+lld delta50ms=%+lld %s\n", - label, (unsigned long long)target, (unsigned long long)r1, - (long long)err, (long long)rate, verdict); + a->label, (unsigned long long)target, (unsigned long long)r1, + (long long)err, (long long)rate, + took ? "TOOK" : slow ? "inconclusive (arm too slow)" + : live ? "no-op" : "no-op (clock stalled)"); if (took) return 1; + if (slow) { + printf(" %-34s write->readback %.1f ms (max %.0f), arm %.1f ms (max %.0f)\n", + "", t_r1 - t_write, kTsfWriteToReadbackMaxMs, t_end - t_base, + kTsfArmMaxMs); + return -1; + } return live ? 0 : -1; } +/* Runs every arm, even after a take, so the printout is the whole picture. */ +static void tsf_arms_run(const struct tsf_arm *arms, size_t n, bool *any, + bool *invalid) +{ + for (size_t i = 0; i < n; i++) { + int r = tsf_arm_run(&arms[i]); + + if (r > 0) + *any = true; + if (r < 0) + *invalid = true; + } +} + static int gate_tsfwrite(uint8_t chan) { - uint64_t base; + /* With the MAC running (the state a live link is in): both word orders, + * then each word alone. The high-word-only target sets a high word that + * differs from the live one, so the arm is not vacuous. */ + static const struct tsf_arm mac_on[] = { + { "MAC on, both words DW0,DW1", TSF_DW0_THEN_DW1, 5000000, false }, + { "MAC on, both words DW1,DW0", TSF_DW1_THEN_DW0, 5000000, false }, + { "MAC on, high word (DW1) only", TSF_DW1_ONLY, 1ull << 32, false }, + { "MAC on, low word (DW0) only", TSF_DW0_ONLY, 5000000, false }, + }; + /* With the MAC stopped, and with the free-running timer disabled. */ + static const struct tsf_arm mac_off[] = { + { "MAC off, both words", TSF_DW0_THEN_DW1, 5000000, false }, + { "MAC off, timer off, both words", TSF_DW0_THEN_DW1, 5000000, true }, + }; uint32_t cfg; bool any = false, invalid = false; - int r; + int alive; if (mt_eeprom_init(&dev)) { printf("GATE TSF-WRITE: FAIL - eeprom_init failed\n"); @@ -2979,15 +3141,19 @@ static int gate_tsfwrite(uint8_t chan) !!(cfg & MT_BEACON_TIME_CFG_BEACON_TX), (unsigned)FIELD_GET(MT_BEACON_TIME_CFG_SYNC_MODE, cfg)); - /* A dead counter reads exactly like a counter that ignores loads. */ - if (!tsf_clock_alive()) { + /* A dead counter reads exactly like a counter that ignores loads, and a + * failed read reads like a dead counter - keep the three apart. */ + alive = tsf_clock_control(); + if (alive < 0) { + printf("\nGATE TSF-WRITE: FAIL - TSF read failed (%u USB transfer error(s)); " + "a transport fault, not a clock verdict\n", mt_io_errors(&dev)); + return 1; + } + if (alive == 0) { printf("\nGATE TSF-WRITE: FAIL - the TSF clock is not running; no load conclusion\n"); return 1; } - /* With the MAC running (the state a live link is in): both word orders, - * then each word alone. The high-word-only target sets a high word that - * differs from the live one, so the arm is not vacuous. */ if (mt_mac_start(&dev, MT_RX_DRAIN_NONE)) { /* The start enables TX control before its DMA-idle poll, so stop * before bailing out rather than leaving the MAC half-started. */ @@ -2995,33 +3161,9 @@ static int gate_tsfwrite(uint8_t chan) printf("\nGATE TSF-WRITE: FAIL - mt_mac_start failed\n"); return 1; } - base = mt7612u_read_tsf(&dev); - r = tsf_variant("MAC on, both words DW0,DW1", base + 5000000, false, false, 0); - if (r > 0) any = true; - if (r < 0) invalid = true; - base = mt7612u_read_tsf(&dev); - r = tsf_variant("MAC on, both words DW1,DW0", base + 5000000, false, true, 0); - if (r > 0) any = true; - if (r < 0) invalid = true; - base = mt7612u_read_tsf(&dev); - r = tsf_variant("MAC on, high word (DW1) only", base + (1ull << 32), false, false, 2); - if (r > 0) any = true; - if (r < 0) invalid = true; - base = mt7612u_read_tsf(&dev); - r = tsf_variant("MAC on, low word (DW0) only", base + 5000000, false, false, 1); - if (r > 0) any = true; - if (r < 0) invalid = true; - - /* With the MAC stopped, and with the free-running timer disabled. */ + tsf_arms_run(mac_on, sizeof mac_on / sizeof mac_on[0], &any, &invalid); mt_mac_stop(&dev); - base = mt7612u_read_tsf(&dev); - r = tsf_variant("MAC off, both words", base + 5000000, false, false, 0); - if (r > 0) any = true; - if (r < 0) invalid = true; - base = mt7612u_read_tsf(&dev); - r = tsf_variant("MAC off, timer off, both words", base + 5000000, true, false, 0); - if (r > 0) any = true; - if (r < 0) invalid = true; + tsf_arms_run(mac_off, sizeof mac_off / sizeof mac_off[0], &any, &invalid); /* A mid-run transport failure reads as all-ones, which every arm would * otherwise report as "ignored" - the exact false conclusion this gate @@ -3031,13 +3173,14 @@ static int gate_tsfwrite(uint8_t chan) if (invalid || mt_io_errors(&dev) != 0) { printf("\nGATE TSF-WRITE: FAIL - %u USB transfer(s) failed%s; no load conclusion\n", mt_io_errors(&dev), - invalid ? " and/or the clock stalled during an arm" : " during the sweep"); + invalid ? " and/or an arm was inconclusive (failed read, stalled clock, or too slow)" + : " during the sweep"); return 1; } printf("\nGATE TSF-WRITE: %s\n", any - ? "FAIL - a write sequence takes; WriteTsf must report success" - : "PASS - confirmed: no sequence loads the TSF, so WriteTsf reports false"); + ? "FAIL - a write sequence takes; tsf_write_ok and WriteTsf must be revisited" + : "PASS - confirmed: no sequence loads the TSF, so tsf_write_ok is false"); return any ? 1 : 0; } diff --git a/tests/beacon_interval_shift.sh b/tests/beacon_interval_shift.sh index 683f9836..b4d0e415 100755 --- a/tests/beacon_interval_shift.sh +++ b/tests/beacon_interval_shift.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash # beacon_interval_shift.sh — de-risk the uplink timing-advance actuator. # -# WriteTsf (REG_TSFTR 0x0560) was shown NOT to move the beacon TBTT air-time -# (the beacon engine runs off a separate/per-port timer). This exercises + +# On Jaguar2/3, WriteTsf (REG_TSFTR 0x0560) was shown NOT to move the beacon +# TBTT air-time (the beacon engine runs off a separate/per-port timer). This exercises + # validates the productized actuator IRadio::AdjustBeaconTiming(us): a # ONE-SHOT beacon-interval tweak (REG_BCN_INTERVAL 0x0554) — run one interval at # (nominal +/- delta) TU then restore, and a clean interval-phased engine diff --git a/tests/radio_iface_selftest.cpp b/tests/radio_iface_selftest.cpp index 9c254bea..18a56fdd 100644 --- a/tests/radio_iface_selftest.cpp +++ b/tests/radio_iface_selftest.cpp @@ -60,6 +60,7 @@ int main() { check(!r->SetAckResponder(devourer::MacAddr{}), "SetAckResponder default refuses"); check(r->ReadTsf() == 0, "ReadTsf default is 0"); check(!r->WriteTsf(123456789ull), "WriteTsf default refuses and reports false"); + check(!r->GetAdapterCaps().tsf_write_ok, "tsf_write_ok default is false"); r->FastRetune(6); check(r->GetSelectedChannel().Channel == 6,