mt7612u: read the TSF coherently across a low-word wrap, and fail loudly

CMakeLists.txt

@@ -392,6 +392,7 @@ if(DEVOURER_MT7612U)
         src/mt7612u/Mt7612uRadio.cpp src/mt7612u/Mt7612uRadio.h
         src/mt7612u/Mt7612uMapping.h
         src/mt7612u/Mt7612uRxQueue.h
+        src/mt7612u/Mt7612uTsfRead.h
         src/mt7612u/internal.h
         src/mt7612u/regs.h
         src/mt7612u/initvals.h
@@ -927,6 +928,33 @@ target_link_libraries(Mt7612uRxQueueSelftest PRIVATE devourer Threads::Threads)
 
 add_test(NAME mt7612u_rx_queue COMMAND Mt7612uRxQueueSelftest)
 
+# Headless guard for the MT7612U coherent TSF read. The two register halves are
+# not latched, so the read order is the whole correctness argument, and on a
+# bench a wrong order only shows for a few hundred microseconds every 71.6 min.
+# The cell sweeps a scripted counter across the low-word wrap and requires the
+# pre-fix order to tear in the same sweep. Header-only; no libusb.
+add_executable(Mt7612uTsfReadSelftest
+    tests/mt7612u_tsf_read_selftest.cpp
+)
+target_link_libraries(Mt7612uTsfReadSelftest PRIVATE devourer)
+
+add_test(NAME mt7612u_tsf_read COMMAND Mt7612uTsfReadSelftest)
+
+# The C entry points' failure contract, which the cell above cannot reach: a
+# failed read must be distinguishable from a value, because 0xffffffff is a
+# legitimate register word on this part. Needs the subtree's symbols, so it is
+# gated on DEVOURER_MT7612U (CI's MediaTek jobs turn it on).
+if(DEVOURER_MT7612U)
+    add_executable(Mt7612uTsfApiSelftest
+        tests/mt7612u_tsf_api_selftest.cpp
+    )
+    target_link_libraries(Mt7612uTsfApiSelftest PRIVATE devourer)
+    target_include_directories(Mt7612uTsfApiSelftest PRIVATE
+        ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u/include)
+
+    add_test(NAME mt7612u_tsf_api COMMAND Mt7612uTsfApiSelftest)
+endif()
+
 # Headless guard for the TX quiesce seam (ITransport::quiesce_tx via
 # RtlAdapter): the explicit "stop TX and wait it out" call every device makes
 # before anything is released. UsbTransport's cancel/drain is validated on

docs/mt7612u.md

@@ -230,8 +230,9 @@ 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.
-- **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
+- **No tested sequence loads the TSF** (the contract is `mt7612u_caps::tsf_write`
+  and `IRadio::WriteTsf`): the DW0/DW1 registers hold the counter, they do not
+  load it. Every sequence
   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
@@ -248,10 +249,54 @@ saturation. A-MPDU, not USB parallelism, is what lifted 34 → 44.55 Mbit/s.
   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. 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.
+  every sequence is confirmed ignored. That result is what sets
+  `mt7612u_caps::tsf_write`, which `AdapterCaps::tsf_write_ok` is taken from.
+- **The two TSF halves are not latched.** A DW0 read does not freeze DW1, so a
+  read whose halves straddle a low-word wrap tears by 2^32 µs. Bring-up
+  restarts the counter near 0 (~0.35–0.40 s on both units), so the first wrap
+  is 71.6 min in. A plain DW0-then-DW1 read hits it about once per wrap when
+  polled back to back, and roughly (read time ÷ poll interval) per wrap
+  otherwise. The read that survives it, and what it costs, is the contract at
+  `mt7612u::tsf_read` (`Mt7612uTsfRead.h`) and the declarations it serves.
+
+  `bringup tsfwrap [gap]` (wrapped by `tests/mt7612u_tsf_wrap.sh`, which owns
+  the runtime and the re-run rule) holds that against the part, judging every read
+  against a host-clock model fitted over the preceding minute rather than
+  against the read under test. Around the wrap it forces the library's read
+  template to take the wrap in the chosen gap, interleaves a plain DW0,DW1 read
+  across the same wrap as a positive control, and checks every continuous
+  `mt7612u_read_tsf_chk` read near the wrap. Measured once per unit, one wrap
+  each:
+
+  | | `40:a5:ef:50:27:a1`, gap 1 | `40:a5:ef:5a:32:f8`, gap 2 |
+  |---|---|---|
+  | forced read | retried, −3 µs off the model | retried, −10 µs |
+  | DW0,DW1 control across the same wrap | +2^32 − 6 µs | +2^32 + 4955 µs |
+  | continuous reads checked near the wrap | 1.70 M of 31.7 M, worst 348 µs | 211 k of 3.9 M, worst 74 µs |
+  | failed reads, backwards steps, reads off the model | 0, 0, 0 | 0, 0, 0 |
+
+  The second unit sits on a USB 2.0 bus where a control transfer can take
+  ~10 ms, which is where its control's extra 4955 µs comes from — the read is
+  judged over the interval that bracketed its transfer, so that is slack in the
+  measurement, not in the counter.
+
+  **What that does not show.** The forced read occupies the wrap instant, so
+  the exported function never takes its own retry across a wrap in that run;
+  the retry path is the forced template read (the same code through another
+  reader) and the `mt7612u_tsf_read` cell. It is one wrap per unit, and the
+  gate's `wrap_bits` smoke mode cannot tear a read at all.
+
+  A failed transfer fails the read rather than joining a `0xffffffff` word
+  into a plausible clock; the return and throw semantics are the declarations'
+  (`mt7612u_read_tsf_chk` / `mt7612u_read_tsf` in `include/mt7612u/mt7612u.h`,
+  `IRadio::ReadTsf`). What is measured here is that the failure is reachable
+  and reaches the caller. Checked once by hand, not by a gate: under a
+  100 Hz `ReadTsf` poll, de-authorising the adapter's whole bus
+  (`usbN/authorized` — a logical disconnect, not a cable pull) gave 795
+  plausible reads and then 200 of 200 calls threw, with no value returned
+  after the disconnect and a clean teardown. De-authorising only the device
+  (its own `authorized`) is not a disconnect on this part: EP0 kept answering and
+  the TSF kept reading correctly for 40 s while the MCU bulk endpoint failed.
 - **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
@@ -330,15 +375,19 @@ and nothing was ever queued on the healthy path.
 
 ## Offline tests
 
-`ctest` runs five MediaTek cells. The first three are C++ and need neither
+`ctest` runs seven MediaTek cells. The first four are C++ and need neither
 hardware nor the `DEVOURER_MT7612U` option — the code they cover is
-header-only. The last two are Python and need the `reference/mt76` submodule:
+header-only. The fifth is C++ and needs the option, because it calls the
+subtree's own symbols. The last two are Python and need the `reference/mt76`
+submodule:
 
 | cell | what it holds |
 |---|---|
 | `mt7612u_usb_ids` | that the 16 MediaTek pairs never claim a device one of the 91 Realtek ids owns. The gate runs *before* the Realtek `SYS_CFG2` read, so an id we wrongly own is refused with no second chance |
 | `mt7612u_mapping` | the descriptor translations — RSSI bias, per-chain signal, rate codes, TID offset, widths. Every one of these has been wrong once |
 | `mt7612u_rx_queue` | the RX hand-off queue's two load-bearing properties: a full ring drops the **newest** frame and counts it, and a popped slot outlives the queue lock. Broken, the first reorders frames or wedges the part and the second is a use-after-free — and both look like a healthy link until a packet processor falls behind |
+| `mt7612u_tsf_read` | that the TSF read stays coherent when the low word wraps between any two of its register accesses: a scripted counter swept across the wrap one microsecond at a time, every failure position, and an all-ones low word as a value. The pre-fix DW0,DW1 order runs against the same sweep and must tear, or the cell cannot see the bug. Mutation-tested by hand three ways — no retry, a retry that keeps the first high word, a retry that skips the low-word re-read — each fails |
+| `mt7612u_tsf_api` | that a failed TSF read stays distinguishable from a value at the C entry points: `mt7612u_read_tsf_chk` refuses a NULL device or output with -1 and does not write through the pointer, and `mt7612u_read_tsf` answers 0. `0xffffffff` is a legitimate register word here, so only the return code can carry a failure. Not free: deleting the guard in `mt7612u_read_tsf_chk` segfaults the cell (hand-run). Reaches the NULL refusals only; a failed transfer on a live device, the `Mt7612uRadio::ReadTsf` throw and the `tsf_write` caps bit need the part (`bringup caps` prints it, and `Mt7612uRadio::GetAdapterCaps` takes `tsf_write_ok` from it rather than restating it) |
 | `mt7612u_usb_ids_vs_mt76` | that the id table above really is the complete `mt76x2u_device_table` from the pinned reference, byte-checked. An id we *forgot* is invisible to `mt7612u_usb_ids` — the adapter just falls through to the Realtek path — and the first draft of that header had 11 of the 16, taken from the host's kernel tree |
 | `mt7612u_initvals_generated` | that the checked-in `initvals.h` still matches what `tools/extract_mt7612u_tables.py` produces from the pinned reference |
 
@@ -347,6 +396,11 @@ The last two SKIP when `reference/mt76` is absent, which is the case in CI —
 are also registered only `if(Python3_Interpreter_FOUND)`, so on a host without
 Python they are *absent* rather than skipped, and `ctest` says nothing at all.
 
+`mt7612u_tsf_read` has the same limit: it holds the read discipline, not the
+wiring. Reverting `mt7612u_read_tsf_chk` to a plain two-register join, or
+`Mt7612uRadio::ReadTsf` to the non-throwing wrapper, passes it; those are held
+by the `bringup tsfwrap` gate recorded under *Other measured items*.
+
 What still has no cell is the *wiring*: `Mt7612uRadio`'s calls into the queue —
 the reset before the ring is armed, the commit after the processor returns —
 are covered by nothing headless. Deleting the `pop_commit()` compiles clean and
@@ -361,7 +415,7 @@ nothing. Each fails the cell, and each names the property it broke.
 
 | test | what it holds |
 |---|---|
-| `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++ |
+| `api_link` | takes the address of all 33 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 |
@@ -521,13 +575,15 @@ Stated because the numbers above are uniformly favourable.
   firmware-running bit.
 - **The 48 ms fast retune is our implementation, not the floor.** The floor is
   unmeasured.
-- **Three ctest cells run in CI, two more only on a bench, and the subtree's own
+- **Five ctest cells run in CI, two more only on a bench, and the subtree's own
   four still run nowhere automatically.** With `DEVOURER_MT7612U=ON` the whole
   platform matrix (gcc, clang, MSVC, mingw, macOS) builds the subtree and the
   sanitizer job links it, so a portability or lifetime regression is caught.
-  `mt7612u_usb_ids`, `mt7612u_mapping` and `mt7612u_rx_queue` run on every
-  configuration. That last one is the first cell to cover backend *behaviour*
-  rather than a lookup — but it covers the queue and nothing else. Bring-up,
+  `mt7612u_usb_ids`, `mt7612u_mapping`, `mt7612u_rx_queue` and
+  `mt7612u_tsf_read` run on every configuration, and `mt7612u_tsf_api`
+  wherever the option is on, which the MediaTek CI jobs set. The TSF pair and
+  the queue cover backend *behaviour* rather than a lookup, and nothing else
+  does. Bring-up,
   the teardown ordering, the 1 Hz tick and TX still have no automated coverage,
   and the two `reference/`-comparing cells SKIP in CI because the submodule is
   not fetched there. Every hardware claim in this document is hand-run except

examples/chanmig/main.cpp

@@ -28,6 +28,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <exception>
 #include <memory>
 #include <mutex>
 #include <string>
@@ -323,7 +324,20 @@ static Drone *g_drone = nullptr;
 
 static uint64_t read_tsf() {
   std::lock_guard<std::mutex> lk(g_dev_mu);
-  return g_dev ? g_dev->ReadTsf() : 0;
+  /* The stamp is informational; 0 already means "no TSF". A failed read
+   * throws (IRadio contract) and must not take the control plane down, but it
+   * is said once rather than swallowed. */
+  try {
+    return g_dev ? g_dev->ReadTsf() : 0;
+  } catch (const std::exception &e) {
+    static bool warned = false;
+    if (!warned) {
+      warned = true;
+      fprintf(stderr, "chanmig: TSF read failed (%s); stamping 0 from here on failure\n",
+              e.what());
+    }
+    return 0;
+  }
 }
 
 static void drone_do(const std::vector<cm::MigAction> &acts) {

examples/tdma/main.cpp

@@ -20,6 +20,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <exception>
 #include <memory>
 #include <string>
 #include <thread>
@@ -124,13 +125,31 @@ static void run_tx(IRadio* dev, const tdma::Config& c) {
     if (w != cur_w) { dev->FastSetBandwidth(w); cur_w = w; }
 
     if (a.phase == tdma::Phase::NB && a.burst != last_marker_burst) {
-      last_marker_burst = a.burst;
       // Stamp the marker with the TX's hardware TSF (works TX-side — no RX
       // flood starving the control read); the TSF-sync RX uses it for drift.
-      uint64_t tx_tsf = dev->ReadTsf();
-      auto f = tdma::build_frame(rt_marker, tdma::Class::Marker, seq[0]++,
-                                 (uint32_t)a.burst, tx_tsf);
-      dev->send_packet(f.data(), f.size());
+      // A failed read throws (IRadio contract): skip this burst's marker
+      // rather than hand the drift fit a wrong stamp, and leave the burst
+      // unmarked so the next pass through it tries again.
+      uint64_t tx_tsf = 0;
+      bool stamped = true;
+      try {
+        tx_tsf = dev->ReadTsf();
+      } catch (const std::exception &e) {
+        static bool warned = false;
+        stamped = false;
+        if (!warned) {
+          warned = true;
+          fprintf(stderr, "tdma: TSF read failed (%s), marker skipped "
+                          "(said once; markers keep being skipped while it fails)\n",
+                  e.what());
+        }
+      }
+      if (stamped) {
+        auto f = tdma::build_frame(rt_marker, tdma::Class::Marker, seq[0]++,
+                                   (uint32_t)a.burst, tx_tsf);
+        dev->send_packet(f.data(), f.size());
+        last_marker_burst = a.burst;
+      }
     }
     tdma::Class cls =
         a.phase == tdma::Phase::NB ? tdma::Class::Critical : tdma::Class::Bulk;

examples/timesync/main.cpp

@@ -38,6 +38,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <exception>
 #include <memory>
 #include <mutex>
 #include <thread>
@@ -157,9 +158,26 @@ static void run_master(IRadio* dev, const timesync::Config& c) {
   while (!g_devourer_should_stop) {
     // Stamp with the master's hardware TSF at send time. TX-side ReadTsf() is
     // reliable (no bulk-IN flood), unlike on a busy receiver.
-    uint64_t tsf = dev->ReadTsf();
-    auto f = tdma::build_frame(rt, tdma::Class::Marker, seq++, 0, tsf);
-    dev->send_packet(f.data(), f.size());
+    // A failed read throws (IRadio contract); skip this marker rather than
+    // stamp it with a time the slave would fit as real.
+    uint64_t tsf = 0;
+    bool stamped = true;
+    try {
+      tsf = dev->ReadTsf();
+    } catch (const std::exception &e) {
+      static bool warned = false;
+      stamped = false;
+      if (!warned) {
+        warned = true;
+        fprintf(stderr, "timesync master: TSF read failed (%s), marker skipped "
+                        "(said once; markers keep being skipped while it fails)\n",
+                e.what());
+      }
+    }
+    if (stamped) {
+      auto f = tdma::build_frame(rt, tdma::Class::Marker, seq++, 0, tsf);
+      dev->send_packet(f.data(), f.size());
+    }
 
     if (std::chrono::steady_clock::now() >= next_stat) {
       next_stat += std::chrono::seconds(1);

src/IRadio.h

@@ -354,8 +354,10 @@ class IRadio {
    * chip's crystal and is latched into every RX descriptor at receive
    * (rx_pkt_attrib::tsfl, the low 32 bits), so it is a precise, host-jitter-free
    * timing reference for multi-radio sync / TDOA / scheduled bursts. Returns 0
-   * where unsupported (default). NB: a register read is a control transfer —
-   * calling it concurrently with a heavy RX bulk-IN load can race (catch the
+   * where unsupported (default, and the RTL8733B). Over USB a failed read
+   * throws std::ios_base::failure on every backend that implements it; a PCIe
+   * register read cannot report failure at all (src/RtlTsf.h). NB: a register read is a control transfer — calling
+   * it concurrently with a heavy RX bulk-IN load can race (catch the
    * exception). */
   virtual uint64_t ReadTsf() { return 0; }
 

src/mt7612u/Mt7612uRadio.cpp

@@ -4,6 +4,7 @@
 #include <cstdlib>
 #include <exception>
 #include <filesystem>
+#include <ios>
 #include <span>
 #include <stdexcept>
 #include <string>
@@ -813,7 +814,16 @@ bool Mt7612uRadio::GetPermanentMacAddress(uint8_t out[6]) {
 
 uint64_t Mt7612uRadio::ReadTsf() {
   std::lock_guard<std::recursive_mutex> lock(_mu);
-  return _dev ? mt7612u_read_tsf(_dev) : 0;
+  if (!_dev)
+    return 0;
+  /* A failed read throws, as it does on every Realtek backend (a failed
+   * rtw_read throws std::ios_base::failure) and as IRadio::ReadTsf documents.
+   * Returning 0 would claim "unsupported"; returning the joined words would
+   * hand a timing consumer a plausible wrong clock. */
+  uint64_t tsf;
+  if (mt7612u_read_tsf_chk(_dev, &tsf))
+    throw std::ios_base::failure("mt7612u: TSF read failed");
+  return tsf;
 }
 
 /* Busy airtime from the MAC channel timers — the MediaTek half of the neutral
@@ -1102,8 +1112,11 @@ devourer::AdapterCaps Mt7612uRadio::GetAdapterCaps() {
    * 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;
+   * is ignored (docs/mt7612u.md), so WriteTsf stays on the IRadio default.
+   * Taken from the C caps rather than restated, so the two cannot drift; the
+   * C library owns the fact (mt7612u_get_caps), and `hw` is zeroed when there
+   * is no device, which is the same answer. */
+  c.tsf_write_ok = hw.tsf_write;
   /* 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