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

[snokvist]

Standalone follow-up to #430, and #434 item 5 in full: the checked, wrap-safe
library ReadTsf, plus the tsf_write bit in the C caps. #434 items 1–4
(Jaguar1 ReadTsf lock, the 8814A TBTT note, the roster copies, the
WriteTsf false wording) are Realtek-side and are not touched here.

mt7612u_read_tsf read MT_TSF_TIMER_DW0 then DW1 with the unchecked mt_rr
and joined them. The two halves are not latched. A read whose halves straddle
the 2^32 µs low-word wrap therefore returns a value 2^32 µs (71.6 min) off. The
first wrap comes 71.6 min after bring-up, which restarts the counter. A failed
transfer joined 0xffffffff into a plausible wrong clock, and Mt7612uRadio::ReadTsf
passed both through as a TSF.

The read tears on hardware, and the fix does not

bringup tsfwrap [gap], new in this PR and reproducible from the tree, judges
every read against a least-squares host-clock model fitted over the preceding
minute, not against the read under test. The gate does three things:

1. Forced read. Around the predicted wrap it drives the library's read
   template through a reader that sleeps to a schedule, so the wrap lands in a
   chosen gap of the read.
2. Positive control. It interleaves a plain DW0,DW1 read across the same
   wrap.
3. Continuous check. It checks every continuous mt7612u_read_tsf_chk read
   near the wrap.

Each value is judged over the host interval bracketing its control transfer. One
transfer took ~10 ms on the slower unit's bus, which a single timestamp would
have misjudged by that much.

One run per unit, both at the real 32-bit wrap:

	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

Both runs PASS. The second unit is 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: a read
is judged over the interval that bracketed its transfer, so that is slack in the
measurement rather than in the counter.

The control tearing by 2^32 is also the latch answer: a DW0 read does not freeze
DW1.

What that does not show.

• The exported function's own retry never ran on hardware. The forced read
  occupies the wrap instant, so mt7612u_read_tsf_chk never takes its retry
  across the wrap in that run. The retry path is covered by the forced template
  read, which is the same code through another reader, and by the headless cell
  below.
• n is one wrap per unit.
• The smoke mode cannot tear a read. wrap_bits < 32 reports SMOKE,
  never PASS.

Failure path, checked once by hand, not a gate. Under a 100 Hz ReadTsf
poll I de-authorised the adapter's whole bus (usbN/authorized, a logical
disconnect, not a cable pull). That gave 795 plausible reads, then 200 of 200
calls threw, with no value returned after the disconnect and a clean teardown.
De-authorising only the device (8-1/authorized) is not a disconnect on this
part: EP0 kept answering and the TSF kept reading correctly for 40 s. The first
attempt would have "passed" vacuously on that.

Structure

• src/mt7612u/Mt7612uTsfRead.h is a pure template. It reads high, low,
  high; if the high word moved, it re-reads the low word and pairs it with the
  second high word.
  • Any failed access fails the read and leaves *out untouched.
  • 0xffffffff is a legitimate word, which is why a return code carries
    failure rather than a sentinel value.
  • The approach matches read_tsftr in src/RtlTsf.h, which re-reads both
    words instead.
• mt7612u_read_tsf_chk(dev, &out) is a new C entry point returning 0 or
  -1. mt7612u_read_tsf uses the same read and returns 0 on failure.
• Mt7612uRadio::ReadTsf throws std::ios_base::failure on a failed read,
  as the Realtek USB backends already do. The IRadio::ReadTsf note now says
  that for USB, keeps the PCIe hedge RtlTsf.h carries, and names the RTL8733B
  as returning 0.
• struct mt7612u_caps gains tsf_write : 1 (0 here), so the C ABI says
  what AdapterCaps::tsf_write_ok says.
• bringup:
  • The tsfwrite gate calls the library reader, so there is one copy of the
    read discipline instead of two.
  • The beacon gate fails on any failed TSF read. Before, a failed read cleared
    only the chain, so a failing transport could still report a live timer.
  • The caps gate prints no word-order verdict from failed raw reads.
• timesync, tdma, chanmig: the two timing demos skip that marker
  rather than stamp a wrong time; chanmig stamps its existing "no TSF" 0 and
  warns once.
• api_link gains mt7612u_read_tsf_chk plus the two public functions it
  was already missing, mt7612u_ch_time and mt7612u_phy_tick. All 33 now
  resolve. The two omissions predate this branch and would normally be their
  own change; they ride here because this PR rewrites the doc line that claims
  the cell covers all public entry points, and that claim was false. Say the
  word and I will split them out.
• tests/mt7612u_tsf_wrap.sh wraps the gate so the invocation, the ~72 min
  per wrap, one gap per run, one adapter being enough, and the re-run rule are
  not folk knowledge. SMOKE=1 checks the schedule, the model and the plumbing
  in ~2.5 min against a 16.7 s carry, and reports SMOKE, never PASS.

Caller-visible behaviour changes

• Mt7612uRadio::ReadTsf throws on a failed read, where it returned a
  garbage value. Any MT7612U consumer that does not catch exceptions will now
  exit on a transport failure instead of carrying a wrong clock. That matches
  what every Realtek USB backend already does.
• mt7612u_read_tsf returns 0 on failure instead of a value built from
  0xffffffff words.
• Each TSF read costs 3 control transfers (4 across a wrap) instead of 2.
  ReadTsf is caller-cadence, never on the send path.
• The timesync and tdma masters skip a marker when the TSF read fails,
  and each says so once rather than at marker rate.
• struct mt7612u_caps gains tsf_write. It lands in spare padding of an
  existing bitfield, so sizeof is unchanged (32 B on x86-64; no 32-bit
  toolchain here to check that layout), and mt7612u_get_caps zeroes the struct
  first, so a caller built against the new header and linked to an older library
  reads 0 — which is the right answer for this part.
• bringup tsfwrap exit codes: 0 PASS, 1 the part failed the measurement,
  2 bad invocation (as everywhere else in the tool), 3 no verdict — interrupted,
  or the wrap landed in the other gap, which is a re-run rather than a defect.

Out of scope, stated

The ReadTsf callers that still do not catch (kestrelprobe,
beacon_steer_check, dl_departure_tx, pcie_ptp_beacon,
pcie_txegress_tx, tsf_probe's first loop) are Kestrel-, Realtek- or
PCIe-specific bench tools. Their behaviour is unchanged by this PR: those
backends already throw.

Headless coverage

mt7612u_tsf_api (ctest, needs DEVOURER_MT7612U, which CI's MediaTek jobs
set) pins the C entry points' failure contract: _chk refuses a NULL device or
output with -1 and does not write through the pointer, mt7612u_read_tsf
answers 0, and struct mt7612u_caps carries tsf_write. 0xffffffff is a
legitimate register word here, so only the return code can carry a failure. It
reaches the NULL refusals only — a failed transfer on a live device, and the
Mt7612uRadio::ReadTsf throw, need the part.

mt7612u_tsf_read (ctest, runs with DEVOURER_MT7612U=OFF too) sweeps a
scripted counter across the wrap one microsecond at a time, so the wrap falls in
every gap of the read. It also covers 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 fails. I also hand-mutated the template three ways (no
retry; a retry that keeps the first high word; a retry that skips the low-word
re-read), and each fails the cell. That mutation run is not in the tree. The
cell covers the read discipline, not the wiring (docs/mt7612u.md says so); the
wiring is what the gate covers.

Test plan

cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DDEVOURER_MT7612U=ON
cmake --build build -j
ctest --test-dir build                        # 68/68, mt7612u_tsf_read + mt7612u_tsf_api
make -C src/mt7612u check                     # api_link: 33 resolved
# hardware (firmware from linux-firmware), ~72 min per run
SMOKE=1 MT7612U_DEV=<bus-port> tests/mt7612u_tsf_wrap.sh    # ~2.5 min, SMOKE
MT7612U_DEV=<bus-port> tests/mt7612u_tsf_wrap.sh            # both gaps, ~2.4 h
DEVS="<port-a> <port-b>" tests/mt7612u_tsf_wrap.sh          # one gap each, ~72 min
MT7612U_DEV=<bus-port> src/mt7612u/bringup tsfwrite 6       # still PASS through the library reader
MT7612U_DEV=<bus-port> src/mt7612u/bringup caps 149         # tsf_write=0, checked word-order read

🤖 Generated with Claude Code

https://claude.ai/code/session_01JP51Yp3WSbfiMDuDHJByHW

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Make MT7612U TSF reads wrap-safe and failure-aware

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Reads MT7612U TSF coherently across low-word wraps and propagates transport failures.
• Makes timing consumers skip invalid timestamps instead of emitting plausible corrupted values.
• Adds automated and hardware validation while exposing TSF-write capability through the C API.

Diagram

graph TD
E["Timing Examples"] --> R["Radio API"] --> C["Checked C API"] --> T["Coherent Reader"] --> H["TSF Registers"]
B["Bringup Gates"] --> C
S["Headless Tests"] --> T
S --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Re-read both register halves after a detected wrap

• ➕ Matches the existing Realtek TSF read discipline more literally.
• ➕ May simplify reasoning by treating the retry as a complete fresh sample.
• ➖ Requires an unnecessary additional USB control transfer during the already rare retry.
• ➖ Provides no additional coherence benefit within the documented wrap timing assumptions.

Recommendation: Keep the proposed high-low-high read with a low-word retry. It provides the required coherence with three normal control transfers and four across a wrap, preserves legitimate all-ones values through an explicit status channel, and aligns C++ failure behavior with existing USB backends. Re-reading both halves is valid but less efficient without improving the stated guarantee.

Files changed (15) +913 / -79

Enhancement (1) +18 / -2

mt7612u.hExpose checked TSF reads and TSF-write capability publicly +18/-2

Expose checked TSF reads and TSF-write capability publicly

• Declares the checked TSF entry point and documents its output-preservation and failure contracts. Extends mt7612u_caps with the measured tsf_write capability bit.

src/mt7612u/include/mt7612u/mt7612u.h

Bug fix (6) +153 / -15

main.cppFall back to zero when migration TSF reads fail +15/-1

Fall back to zero when migration TSF reads fail

• Catches ReadTsf exceptions, emits a one-time warning, and preserves the existing zero-valued “no TSF” stamp instead of terminating the control plane.

examples/chanmig/main.cpp

main.cppSkip TDMA markers after failed TSF reads +24/-5

Skip TDMA markers after failed TSF reads

• Catches TSF read failures and omits the affected marker rather than publishing an invalid timestamp. A burst remains eligible for a later retry, while warnings are limited to one.

examples/tdma/main.cpp

main.cppSkip time-sync markers with unavailable TSF timestamps +21/-3

Skip time-sync markers with unavailable TSF timestamps

• Handles ReadTsf exceptions in the master loop and suppresses markers that cannot receive a valid hardware timestamp. Reports the failure once without stopping subsequent operation.

examples/timesync/main.cpp

Mt7612uRadio.cppPropagate checked TSF failures through the C++ radio API +16/-3

Propagate checked TSF failures through the C++ radio API

• Routes ReadTsf through the checked C entry point and throws std::ios_base::failure when the transport read fails. Sources tsf_write_ok from the C capability structure to prevent duplicated capability declarations.

src/mt7612u/Mt7612uRadio.cpp

Mt7612uTsfRead.hAdd reusable coherent MT7612U TSF reader +61/-0

Add reusable coherent MT7612U TSF reader

• Introduces a pure high-low-high register-read template that re-reads the low word when the high word changes. Any failed access returns -1 without modifying the output, and tests may observe whether the retry ran.

src/mt7612u/Mt7612uTsfRead.h

caps.cppImplement checked coherent TSF reads and TSF-write capability +16/-3

Implement checked coherent TSF reads and TSF-write capability

• Adds mt7612u_read_tsf_chk using checked register accesses and the reusable wrap-safe algorithm. Makes the legacy value-only wrapper return zero on failure and reports that TSF writes are unsupported.

src/mt7612u/caps.cpp

Tests (4) +645 / -53

api_link.cExpand public API link coverage +3/-0

Expand public API link coverage

• Adds the checked TSF reader plus the previously omitted channel-time and PHY-tick functions to the public-symbol link test, covering all 33 entry points.

src/mt7612u/tests/api_link.c

bringup.cppAdd hardware TSF wrap validation and checked diagnostics +439/-53

Add hardware TSF wrap validation and checked diagnostics

• Introduces the tsfwrap gate, which combines continuous reads, a least-squares host-clock model, a scheduled wrap-straddling read, and a tearing positive control. Existing beacon, capability, and TSF-write gates now use checked library reads and refuse verdicts based on failed transfers.

src/mt7612u/tools/bringup.cpp

mt7612u_tsf_api_selftest.cppTest the exported TSF failure contract +57/-0

Test the exported TSF failure contract

• Verifies null-argument rejection, untouched output on failure, and zero from the value-only wrapper. The test explicitly distinguishes failed accesses from legitimate all-ones register values.

tests/mt7612u_tsf_api_selftest.cpp

mt7612u_tsf_read_selftest.cppTest TSF coherence across every wrap-access gap +146/-0

Test TSF coherence across every wrap-access gap

• Sweeps a scripted counter across the low-word wrap, verifies retry behavior and every failure position, and accepts all-ones low words as data. A pre-fix low-then-high reader serves as a required tearing negative control.

tests/mt7612u_tsf_read_selftest.cpp

Documentation (3) +69 / -9

mt7612u.mdDocument TSF wrap behavior, failure semantics, and validation +63/-7

Document TSF wrap behavior, failure semantics, and validation

• Records the unlatched-register behavior, coherent read algorithm, hardware measurements, and disconnect results. Expands the offline-test inventory and clearly states the coverage limitations of headless and hardware checks.

docs/mt7612u.md

IRadio.hClarify ReadTsf failure behavior across transports +5/-2

Clarify ReadTsf failure behavior across transports

• Documents that implemented USB backends throw std::ios_base::failure on read failure, while unsupported devices return zero and PCIe reads cannot report transport errors.

src/IRadio.h

README.mdList the TSF wrap hardware gate +1/-0

List the TSF wrap hardware gate

• Adds the approximately 72-minute tsfwrap validation command to the bring-up gate roster.

src/mt7612u/README.md

Other (1) +28 / -0

CMakeLists.txtRegister coherent TSF sources and self-tests +28/-0

Register coherent TSF sources and self-tests

• Adds the header-only TSF reader to the MT7612U source set. Registers wrap-coherence and C API contract tests, with the latter gated on the MT7612U build option.

CMakeLists.txt

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. TSF failure contract has two copies ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new docs/mt7612u.md paragraph restates the return and exception behavior already defined by
the declarations in mt7612u.h and IRadio.h. A later contract change must now update both the
authoritative headers and this narrative or callers may receive conflicting guidance.

Code

docs/mt7612u.md[R291-293]

+  A failed transfer fails the read: `_chk` returns -1, `mt7612u_read_tsf`
+  returns 0, and `Mt7612uRadio::ReadTsf` throws `std::ios_base::failure`, as
+  the Realtek USB backends do. Checked once by hand, not by a gate: under a

Evidence

Compliance rule 7 requires API and capability contracts to remain authoritative in their declaration
headers rather than being restated in repository guidance. The added documentation independently
repeats that _chk returns -1, the unchecked wrapper returns 0, and the C++ method throws.

CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation: CLAUDE.md: Do Not Duplicate Header Documentation
docs/mt7612u.md[291-299]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The MT7612U documentation duplicates the TSF read failure contract maintained by the public C and C++ headers, creating an independently maintained copy that can become stale.
## Fix Focus Areas
- docs/mt7612u.md[291-299]
## Recommended Fix
Replace the repeated return and exception contract with references to the authoritative declarations in `src/mt7612u/include/mt7612u/mt7612u.h` and `src/IRadio.h`, while retaining the hardware-disconnect measurement and its limitations.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Invalid wrap commands run unpredictably ✓ Resolved 🐞 Bug ☼ Reliability

Description

gate_tsfwrap evaluates 1ull << wrap_bits and converts max_min * 60e6 to a signed deadline
before validating either value. Negative or out-of-range shift counts and NaN, infinite, or
oversized durations passed from the bringup tsfwrap command-line parsers therefore reach undefined
arithmetic before the gate can return its documented failure verdict.

Code

src/mt7612u/tools/bringup.cpp[R3340-3343]

+	const uint64_t period = 1ull << wrap_bits, mask = period - 1;
+	const int64_t t_start = mono_us();
+	int64_t deadline = t_start + (int64_t)(max_min * 60e6);
+	int64_t last_pt = 0, last_status = 0, wrap_host = 0;

Evidence

The command dispatch forwards raw atoi and atof results directly into gate_tsfwrap, where
1ull << wrap_bits and the signed deadline are computed before the existing range check.
Consequently, inputs such as a negative shift count, 64, nan, inf, or an oversized duration
can invoke undefined behavior before validation occurs.

src/mt7612u/tools/bringup.cpp[3337-3359]
src/mt7612u/tools/bringup.cpp[3626-3632]
src/mt7612u/tools/bringup.cpp[3340-3358]
src/mt7612u/tools/bringup.cpp[3629-3632]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`gate_tsfwrap` performs a bit shift and floating-point-to-integer deadline conversion before validating command-line values, allowing malformed options to trigger undefined behavior.
## Fix Focus Areas
- src/mt7612u/tools/bringup.cpp[3340-3359]
- src/mt7612u/tools/bringup.cpp[3629-3632]
## Recommended Fix
Validate `wrap_bits` before computing `period` or `mask`, and require `max_min` to be finite, positive, and small enough that its conversion to microseconds and addition to `mono_us()` both fit in `int64_t`. Prefer checked numeric parsing in the command dispatch, such as `strtol` and `strtod` with end-pointer and range checks, pass only validated values to `gate_tsfwrap`, and compute `period`, `mask`, and `deadline` only after all checks succeed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[josephnef]

Reviewed at 9fd5faa (11 commits, up to date with master). Approving.

Checked on the branch here (headless; no MT7612U on this bench or in the inventory, so the tsfwrap gate itself was not re-run — the two-unit evidence is the author's run, which the gate makes reproducible):

• cmake -DDEVOURER_MT7612U=ON builds warning-free for the new code; ctest 68/68 (the two reference/mt76 cells skip, as documented). mt7612u_tsf_read: PASS (450 retries, old order tore 300 times), mt7612u_tsf_api: PASS.
• make -C src/mt7612u check: api_link: 33 public entry points resolved (master resolves 30).
• Mutation: disabling the hi2 != hi retry in Mt7612uTsfRead.h fails the cell on the coherence sweep, the access-3 failure positions and the "reaches the retry" pin. The negative control is real.
• Read discipline: hi, lo, hi, retry lo paired with the second hi — coherent for a wrap in either gap, and the retry can only tear on a second wrap 71.6 min later. Same shape as read_tsftr (src/RtlTsf.h) and RtlKestrelDevice::ReadTsf; the IRadio.h note ("throws over USB on every backend that implements it, PCIe cannot report failure, RTL8733B returns 0") matches what those backends actually do.
• gate_tsfwrap: schedule/gap arithmetic, the control's sign check (+2^32 only), the "which gap did the wrap really land in" credit from the first low word, and the rc 0/1/2/3 split all read correctly. The argument checks now precede the shift and the int64 conversion (qodo's thread).
• Mt7612uRadio::GetAdapterCaps takes tsf_write_ok from the zeroed-then-filled C caps, so the bit has one owner.

Nits, none blocking:

1. src/IRadio.h ~L359: the new sentence makes one ~120-col line inside an otherwise wrapped comment. Wrap it.
2. examples/tdma/main.cpp: leaving last_marker_burst unset on a failed read means the NB phase retries the 3-transfer read on every loop pass until it succeeds, not once per burst as before. That is the right call for the transient RX-flood race the IRadio note describes, but on a dead transport it is a tight loop of failing control transfers. One sentence in the comment saying that is deliberate would do; a per-burst backoff is the alternative.
3. tests/mt7612u_tsf_wrap.sh parallel mode: no trap on INT/TERM to kill ${pids[@]}. A tty Ctrl-C reaches the children via the process group and bringup handles it, but a dropped SSH session or a kill of the wrapper leaves two 72-min bringup runs holding the adapters. Cheap to add.
4. The mt7612u_read_tsf "0 on failure" contract collides with IRadio's "0 = unsupported" meaning if a C consumer ever forwards it; the header already says to use _chk wherever a failure has to be told apart, so no change — noting it for the record.

Splitting mt7612u_ch_time / mt7612u_phy_tick out of api_link is not needed; they belong with the "covers all entry points" claim this PR rewrites.