Skip to content

Follow-ups left open by the WriteTsf / tsf_write_ok work (#430) #434

Description

@josephnef

Non-blocking items from the #430 review (merged as bb64508). Each was raised inline on the PR and resolved to merge; none changes what shipped. Measurements referenced are in the #430 review body.

1. Jaguar1 ReadTsf runs unlocked against the now-locked pair write

RtlJaguarDevice::WriteTsf writes REG_TSFTR low then high under _port0_mu; ReadTsf takes no lock. A read that lands between the two control transfers returns old_hi<<32 | new_lo. The hi/lo/hi retry in read_tsftr only fires when the high word changes between its own reads, so a high-word write that lands after the second read goes unnoticed. Jaguar2/3 hold _reg_mu in both. _port0_mu is recursive; taking it in ReadTsf is one line and matches the steer paths, which already read the pair under it.

While there: PinBeaconTbtt / AdjustBeaconTimingFine on all three Jaguar backends still hand-roll the same 0x0560/0x0564 pair (AdjustBeaconTimingFine without the wrap retry; every steer write discards rtw_write's bool). They already hold the lock RtlTsf.h prescribes, so read_tsftr / write_tsftr are drop-in (a no-readback variant if the bracket cannot afford the PCIe / near-wrap readback).

2. "Expect a Jaguar1 write to move an active beacon's TBTT" contradicts the 8814A note

RtlJaguarDevice.h (WriteTsf) and IRadio.h say the J1 TBTT is locked to the TSF grid "on all three dies". The same file's AdjustBeaconTimingFine note records the 8814A TBTT free-running across a TSF shift (a −5000 µs shift moved the phase by only the ~0.8 ms bracket off-time) until the DUAL_TSF_RST pulse, which the bare WriteTsf does not issue. Scope the expectation to the 8812A/8821A, or state the 8814A is expected not to follow. The beacon-armed WriteTsf case (cadence / seq / body timestamp across a ±5 s write) is unmeasured on every J1 die.

3. Measurement roster duplicated three times, and already stale

The per-die readback roster lives in AdapterCaps.h (the tsf_write_ok comment), docs/time-distribution.md (the table) and RtlJaguarDevice.cpp (WriteTsf comment). All three still say the J1 override "has not itself run on Jaguar1 hardware"; it has (review body: RTL8821AU, 10/10 forward and 10/10 backward WriteTsf calls, readback error minus host time −300…−15 µs). Collapse the two comments to the flag's meaning plus a pointer, and add the J1 row to the docs table.

4. IRadio::WriteTsf contract: false is wider than "transport failure"

write_tsftr returns false on PCIe (and on USB near a low-word wrap) whenever the readback lands ≥ 100 ms past the target, which the helper's own comment names as a host preemption or an ignored small backward write, not only a transport fault. A caller taking the IRadio.h sentence literally re-issues its stale precomputed target and steps the clock back by the preemption. Also, "a device that has gone reports false on every call" drops the hedge RtlTsf.h keeps: over vfio the readback is a raw volatile load on a BAR mapping that can fault rather than read all-ones.

5. MT7612U: C ABI has no tsf_write cap, and the library ReadTsf is unchecked

struct mt7612u_caps already carries negative bits (narrowband, fast_retune); with mt7612u_write_tsf deleted, a tsf_write : 1 bit would let the C ABI say what AdapterCaps::tsf_write_ok says instead of a link error being the only signal. Separately, Mt7612uRadio::ReadTsfmt7612u_read_tsf still reads DW0 then DW1 through unchecked mt_rr with no wrap retry; only the bringup gate got the checked tsf_read_chk reader.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions