Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 40 additions & 32 deletions docs/mt7612u.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
32 changes: 32 additions & 0 deletions src/mt7612u/Mt7612uRxCorr.h
Original file line number Diff line number Diff line change
@@ -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
13 changes: 9 additions & 4 deletions src/mt7612u/eeprom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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. */
Expand All @@ -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;
Expand Down
27 changes: 25 additions & 2 deletions src/mt7612u/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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. */
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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 */
Expand Down
16 changes: 12 additions & 4 deletions src/mt7612u/rx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion src/mt7612u/tools/bringup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading