mt7612u: make a retune race-free against the live RX ring (#444)

The two data races docs/mt7612u.md listed for a channel change while the RX
ring is up, reproduced under ThreadSanitizer on the first MT7612U in the lab
(Comfast CF-922AC) and fixed: the per-channel RSSI correction is one atomic
word (mt7612u_cal::rx_corr, Mt7612uRxCorr.h), and the sync helpers submit
through libusb's async API and wait on the library's own acquire/release
flag, with heap-owned transfer state (last party frees), transfers pooled
before the event thread exists, and a stalled transfer stranding the device
rather than being freed in flight.

Measured after: 0 library/libusb TSan reports over ~1000 cross-band retunes
and 7000+ frames; retune latency and witnessed delivery identical to the
sync-API build.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Author: josephnef

docs/mt7612u.md

@@ -598,46 +598,54 @@ Stated because the numbers above are uniformly favourable.
   metadata and every consumer that trims four bytes are in place - but the
   adapter still has to USE it: see Open.
 
+## Retune while the ring is up
+
+A channel change from the caller's thread while the libusb event thread is
+delivering frames is the one concurrent operation this backend has, and it is
+measured race-free: `rxdemo` under ThreadSanitizer (`DEVOURER_SANITIZE=thread`)
+with `DEVOURER_RX_SWEEP=1,6,11,36,40,44,48` at 150 ms dwells — a full,
+cross-band tune every dwell — ran 150 s on a Comfast CF-922AC (SuperSpeed,
+`40:a5:ef:5f:65:51`), about a thousand retunes and 7221 frames, with **zero
+reports inside the library or libusb**. The two reports that remain are in
+`rxdemo` itself (its frame counter and the demos' signal-handler stop flag)
+and are shared with every generation.
+
+Two facts about the design hold that result up:
+
+- **The per-channel RSSI correction is one word**, published once per tune
+  and loaded once per frame — the layout and why are on the declaration
+  (`mt7612u_cal::rx_corr`, `Mt7612uRxCorr.h`). What that bought: three
+  separate bytes gave a frame parsed mid-retune a wrong RSSI, which is the
+  class of bug this port cares most about.
+- **Synchronous transfers do not use libusb's synchronous API.** Every
+  register access and MCU exchange is an async submission waited on the
+  library's own acquire/release flag; the hazard in libusb's own sync layer
+  and the lifetime rules are documented once, at the helpers in `usb.cpp`. The
+  replacement is A/B-neutral where it could cost: retune latency on the
+  7-bin sweep above read p50 791 ms / mean 756 ms on both the sync-API build
+  and this one (65 dwells each), and a 2000-frame injection witnessed by an
+  RTL8822BU delivered the same count from both builds at two power back-offs
+  (1000 at −20 dB, 900 at −10 dB; the ground station sits 20 cm away in
+  saturation, so those are its ceiling, not the transmitter's).
+
+For scale: the same stress on the shipping RTL8812AU path produces **8** TSan
+reports, all in devourer's own Jaguar1 state (`RtlJaguarDevice.cpp`,
+`RtlAdapter.h`). Retune-during-RX is race-free here and not there.
+
 ## Open list
 
 Ordered, and honest about which are unknowns rather than typing:
 
-1. **Two data races the library has when a channel change runs while the RX
-   ring is up.** Found with ThreadSanitizer against real hardware, driving
-   `rxdemo` with `DEVOURER_RX_SWEEP` so the main thread retunes while the
-   libusb event thread delivers frames. Neither is in `Mt7612uRadio` — that
-   class's `_mu` discipline held, and TSan reported nothing inside it — and
-   neither is reachable from the bring-up harness, which sets the channel
-   before starting a ring and measured 0 warnings over 10667 frames.
-
-   - `mt_read_rx_gain()` (`eeprom.cpp`) rewrites the per-channel `lna_gain` and
-     `rssi_offset[]` from the EEPROM on every tune, while `mt_rx_parse()`
-     (`rx.cpp`) reads them on the event thread to correct each frame's RSSI. A
-     frame parsed mid-retune therefore gets a mixed correction, i.e. a wrong
-     RSSI for that frame. Wrong number, not a crash — which is the class this
-     port cares most about.
-   - A synchronous control transfer issued during the tune (`mt_rr_chk` ->
-     `libusb_control_transfer`) reaches `libusb_free_transfer()`, destroying a
-     transfer's mutex, while the async ring's event thread locks it. That is
-     inside libusb, and it is the documented hazard of mixing the synchronous
-     API with a dedicated event thread on one context.
-
-   For scale: the same stress on the shipping RTL8812AU path produces **8**
-   TSan reports, all in devourer's own Jaguar1 state (`RtlJaguarDevice.cpp`,
-   `RtlAdapter.h`). Retune-during-RX is not a race-free operation anywhere in
-   this project today, so this is a shared gap rather than a MediaTek
-   regression — but the two above are specific and fixable, and the second one
-   is a use-after-destroy rather than a torn read.
-2. `mt76x2_phy_tssi_compensate()` — periodic temperature correction. Without
+1. `mt76x2_phy_tssi_compensate()` — periodic temperature correction. Without
    it output power drifts with die temperature.
-3. Cold-boot verification on a host with switchable USB power.
-4. A witness on different silicon. The regression matrix added a second witness
+2. Cold-boot verification on a host with switchable USB power.
+3. A witness on different silicon. The regression matrix added a second witness
    *implementation* (`tcpdump` over `mt76x2u`), and a second *unit* of the part
    — but not a second board revision, and not a decoder outside the MediaTek
    family.
-5. 80 MHz; VHT and NSS=2 on air.
-6. Retune tuning — batch registers via `CMD_RANDOM_WRITE`, drop the inter-command
+4. 80 MHz; VHT and NSS=2 on air.
+5. Retune tuning — batch registers via `CMD_RANDOM_WRITE`, drop the inter-command
    sleep, skip the `RXDCOC` on a fast path. Worth doing only if 10–20 ms is
    useful to someone.
-7. Whether the single MCS6 frame in the rate-LUT control arm (1 of 84) is a
+6. Whether the single MCS6 frame in the rate-LUT control arm (1 of 84) is a
    witness decode artefact or a real fallback. Unexplained.

src/mt7612u/Mt7612uRxCorr.h

@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: BSD-3-Clause-Clear */
+/*
+ * The per-channel RSSI correction as one word.
+ *
+ * mt7612u_cal::rx_corr packs the two chain RSSI offsets and the LNA gain
+ * (mt76x02_mac_get_rssi's three inputs) into a single uint32_t so that a
+ * retune publishes all three at once and the RX parser reads them in one
+ * load. Pure integer packing, no device dependency - kept in its own header
+ * so the headless mapping selftest can pin the sign handling.
+ *
+ * Layout: byte 0 = chain-0 offset, byte 1 = chain-1 offset, byte 2 = LNA
+ * gain, each a signed dB; byte 3 unused.
+ */
+#ifndef MT7612U_RX_CORR_H
+#define MT7612U_RX_CORR_H
+
+#include <stdint.h>
+
+static inline uint32_t mt_rx_corr_pack(int8_t off0, int8_t off1, int8_t lna)
+{
+	return (uint32_t)(uint8_t)off0 | ((uint32_t)(uint8_t)off1 << 8) |
+	       ((uint32_t)(uint8_t)lna << 16);
+}
+
+static inline void mt_rx_corr_unpack(uint32_t v, int8_t off[2], int8_t *lna)
+{
+	off[0] = (int8_t)(v & 0xff);
+	off[1] = (int8_t)((v >> 8) & 0xff);
+	*lna   = (int8_t)((v >> 16) & 0xff);
+}
+
+#endif

src/mt7612u/eeprom.cpp

@@ -248,7 +248,7 @@ static uint8_t get_5g_rx_gain(struct mt7612u_dev *d, uint8_t chan)
 
 void mt_read_rx_gain(struct mt7612u_dev *d, uint8_t chan, int band)
 {
-	int8_t lna_2g, lna_5g[3];
+	int8_t lna_2g, lna_5g[3], rssi_offset[2], lna_gain;
 	uint16_t rssi_off, v;
 	uint8_t gain, lna = 0;
 
@@ -273,9 +273,9 @@ void mt_read_rx_gain(struct mt7612u_dev *d, uint8_t chan, int band)
 
 	rssi_off = (band == 0) ? mt_ee(d, MT_EE_RSSI_OFFSET_2G_0)
 	                       : mt_ee(d, MT_EE_RSSI_OFFSET_5G_0);
-	d->cal.rssi_offset[0] = field_valid(rssi_off & 0xff)
+	rssi_offset[0] = field_valid(rssi_off & 0xff)
 		? (int8_t)sign_extend_optional(rssi_off & 0xff, 7) : 0;
-	d->cal.rssi_offset[1] = field_valid(rssi_off >> 8)
+	rssi_offset[1] = field_valid(rssi_off >> 8)
 		? (int8_t)sign_extend_optional(rssi_off >> 8, 7) : 0;
 
 	/* mt76x02_get_lna_gain(): which LNA entry applies to this channel. */
@@ -290,8 +290,13 @@ void mt_read_rx_gain(struct mt7612u_dev *d, uint8_t chan, int band)
 		uint16_t c1 = mt_ee(d, MT_EE_NIC_CONF_1);
 		int ext = (band == 0) ? (c1 & MT_EE_NIC_CONF_1_LNA_EXT_2G)
 		                      : (c1 & MT_EE_NIC_CONF_1_LNA_EXT_5G);
-		d->cal.lna_gain = ext ? 0 : (int8_t)sign_extend(lna, 8);
+		lna_gain = ext ? 0 : (int8_t)sign_extend(lna, 8);
 	}
+	/* One store: the RX event thread reads this word per frame, and the
+	 * three values only make sense together (see mt7612u_cal::rx_corr). */
+	d->cal.rx_corr.store(mt_rx_corr_pack(rssi_offset[0], rssi_offset[1],
+	                                     lna_gain),
+	                     std::memory_order_relaxed);
 
 	d->cal.mcu_gain  = (uint32_t)(lna_2g & 0xff);
 	d->cal.mcu_gain |= (uint32_t)(lna_5g[0] & 0xff) << 8;

src/mt7612u/internal.h

@@ -20,6 +20,7 @@
  * because MSVC has no <pthread.h> and devourer builds Windows first-class.
  * Nothing outside this subtree includes this header; the public C ABI in
  * include/mt7612u/mt7612u.h is unaffected and stays C-includable. */
+#include <atomic>
 #include <chrono>
 #include <condition_variable>
 #include <mutex>
@@ -28,6 +29,7 @@
 #include <stddef.h>
 #include <stdio.h>
 #include "regs.h"
+#include "Mt7612uRxCorr.h"
 #include "include/mt7612u/mt7612u.h"
 
 /* Per-rate TX power, 0.5 dB units, exactly mt76x02_rate_power's layout. */
@@ -46,8 +48,18 @@ struct mt_tx_power_info {
 
 /* EEPROM-derived values the host computes with (firmware does the rest). */
 struct mt7612u_cal {
-	int8_t  rssi_offset[2];
-	int8_t  lna_gain;
+	/* The per-channel RSSI correction (mt76x02_mac_get_rssi's two chain
+	 * offsets and the LNA gain), packed into one word so that a retune
+	 * publishes all three at once. mt_read_rx_gain() rewrites them on every
+	 * tune from the caller's thread while mt_rx_parse() reads them on the
+	 * libusb event thread for every frame; as three separate bytes a frame
+	 * parsed mid-retune got the new offset with the old LNA gain - a wrong
+	 * RSSI, which ThreadSanitizer reported against real hardware. One
+	 * relaxed atomic word is the whole fix: the reader sees either the old
+	 * triple or the new, never a mix, and the hot path pays one load.
+	 * Encode/decode with mt_rx_corr_pack()/mt_rx_corr_unpack()
+	 * (Mt7612uRxCorr.h). */
+	std::atomic<uint32_t> rx_corr;
 	int8_t  high_gain[2];
 	uint8_t init_cal_done;
 	uint8_t channel_cal_done;
@@ -69,6 +81,7 @@ struct mt7612u_cal {
 	 * RX hot path does no cross-thread write at all. */
 };
 
+#define MT_SYNC_POOL 4   /* pooled transfers for the sync helpers */
 #define MT_RX_RING  16
 /* 16 slots, not 32: the slots now carry a full aggregate, so this is the
  * difference between 256 KB and 512 KB of ring. Depth is not what buys
@@ -208,6 +221,16 @@ struct mt7612u_dev {
 
 	unsigned io_err;          /* EP0 transfers that exhausted their retries */
 	int      transfers_stranded; /* libusb still owns a cancelled ring */
+	/* libusb_transfer objects for the synchronous helpers (usb.cpp), taken
+	 * from here rather than allocated per call. Allocated in
+	 * mt_dev_state_init(), i.e. before any event thread exists, so the
+	 * thread's first lock of a transfer's mutex is ordered after its
+	 * initialisation by thread creation - a per-call allocation is ordered
+	 * only through the kernel's URB handoff, which ThreadSanitizer cannot
+	 * see and reports. Empty pool = allocate fresh (correct, just noisier). */
+	std::mutex sync_pool_mu;
+	struct libusb_transfer *sync_pool[MT_SYNC_POOL];
+	int      sync_pool_n;
 	uint16_t max_mpdu_rx;     /* from MT_MAX_LEN_CFG at init, less the FCS */
 	uint64_t stats_last_us;   /* previous mt7612u_link_stats() mark */
 	int      ch_time_armed;   /* channel timers configured and zeroed */

src/mt7612u/rx.cpp

@@ -91,10 +91,18 @@ int mt_rx_parse(struct mt7612u_dev *d, uint8_t *buf, int n,
 	 * the EEPROM (mt76x02_mac_get_rssi); with them at zero these are the
 	 * raw chip values, which is still enough to compare two chains. */
 	info->n_chains = (uint8_t)((d->chainmask & 0xf) > 1 ? 2 : 1);
-	for (int c = 0; c < 4; c++)
-		info->rssi[c] = (int8_t)((int8_t)rxwi[12 + c] +
-		                         (c < 2 ? d->cal.rssi_offset[c] : 0) -
-		                         d->cal.lna_gain);
+	{
+		/* One load of the packed triple: a retune on another thread
+		 * republishes all three at once, so this frame is corrected
+		 * consistently with either the old channel or the new one. */
+		int8_t rssi_offset[2], lna_gain;
+		mt_rx_corr_unpack(d->cal.rx_corr.load(std::memory_order_relaxed),
+		                  rssi_offset, &lna_gain);
+		for (int c = 0; c < 4; c++)
+			info->rssi[c] = (int8_t)((int8_t)rxwi[12 + c] +
+			                         (c < 2 ? rssi_offset[c] : 0) -
+			                         lna_gain);
+	}
 
 	for (int i = 0; i < 4; i++)
 		info->bbp[i] = get_le32(rxwi + 16 + 4 * i);

src/mt7612u/tools/bringup.cpp

@@ -1812,9 +1812,12 @@ static int gate_caps(uint8_t chan)
 	       c.max_mpdu_tx, c.max_mpdu_rx,
 	       mt_rr(&dev, MT_MAX_LEN_CFG) & 0xfff);
 
+	int8_t rssi_offset[2], lna_gain;
+	mt_rx_corr_unpack(dev.cal.rx_corr.load(std::memory_order_relaxed),
+	                  rssi_offset, &lna_gain);
 	printf("\nRX gain from EEPROM: rssi_offset=[%d,%d] lna_gain=%d "
 	       "high_gain=[%d,%d] mcu_gain=0x%08x\n",
-	       dev.cal.rssi_offset[0], dev.cal.rssi_offset[1], dev.cal.lna_gain,
+	       rssi_offset[0], rssi_offset[1], lna_gain,
 	       dev.cal.high_gain[0], dev.cal.high_gain[1], dev.cal.mcu_gain);
 	printf("  raw EEPROM: LNA_GAIN=0x%04x RSSI_OFF_5G_0=0x%04x "
 	       "RSSI_OFF_5G_1=0x%04x GRP4_5_RX_HIGH_GAIN=0x%04x\n",