From 6ca24f0d211c19da0f0b615d3489c15549e4c51a Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 29 Oct 2026 14:09:51 +0100 Subject: [PATCH 1/5] mt7612u: drain EP 4 when an MCU reply is late, so RXDCOC cannot stall bring-up On a channel with traffic, an MT7612U bring-up took ~27.5 s instead of ~0.87 s: from the RXDCOC calibration of the channel set on, every MCU command ran out its 3 s reply budget, and all the replies then appeared within ~7 ms of the RX ring's first EP 4 transfer. A register probe at the first timeout read MT_MAC_SYS_CTRL = 0 (MAC RX and TX off) with the USB RX bulk enable set, and the EP 4 read decoded there was a data frame from the peer's address. So received data reaches EP 4 during RXDCOC before anything reads it, and while it sits there the EP 5 replies are held back too. What in the calibration lets it through is not established. mcu_wait_resp() now drains EP 4 once per command, on its first 300 ms timeout, and only while nobody else reads EP 4. Ownership is a new mt7612u_dev::ep4_owner (NONE / RING / SYNC) rather than d->a->rx_active: it lives on the device, so the PHY tick can read it while StopRxLoop frees the ring (which it does without _mu). The ring claims it under io_lock before its first RX submit and releases it once no RX transfer is left in flight; an MT_RX_DRAIN_SYNC caller holds it until the receiver is switched off; every transition is a compare-exchange from the expected owner. A reply on time costs nothing; a late one costs one mt_rx_flush(), which now returns the bytes drained (partial reads included). The drain logs the command, length and first word. tests/mt7612u_busy_bringup.sh reproduces it with rxdemo + a txdemo peer (build with -DDEVOURER_MT7612U=ON; it runs under mt7612u_sta_lib.sh's root-run rules). One MT7612U, ch 100, an RTL8812EU peer 50 cm away at zero gap: master 09c2ff3 stalled 6 of 6 at 20 MHz and 6 of 6 at HT40 (27.5 s, 9 timeouts each); with this change 10 of 10 and 10 of 10 reached the RX ring in 1.18 s, 0.31 s over the quiet control. The odds rise with the peer's load (a 6 Mb/s peer at a 500 us gap: 1 of 6 on master). Not measured: a ring-less (TX-only) channel set under traffic, and whether the ten-read reply budget could go back to five. docs/mt7612u.md has the table and limits; src/mt7612u/CLAUDE.md replaces "bring this one up before the interferer" with the mechanism; the busy-window probe's notes that relied on that rule are updated (its loaded MediaTek arms stay skipped: not re-run with the sensor started second); tests/README.md lists the reproducer. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011dReg1L6xtBhf95NpAeuge --- docs/mt7612u.md | 35 ++++++ src/mt7612u/CLAUDE.md | 27 ++++- src/mt7612u/async.cpp | 15 ++- src/mt7612u/init.cpp | 33 ++++-- src/mt7612u/internal.h | 17 ++- src/mt7612u/mcu.cpp | 38 +++++-- tests/README.md | 12 +++ tests/busy_window_probe.cpp | 5 +- tests/busy_window_probe.sh | 6 +- tests/mt7612u_busy_bringup.sh | 194 ++++++++++++++++++++++++++++++++++ 10 files changed, 356 insertions(+), 26 deletions(-) create mode 100755 tests/mt7612u_busy_bringup.sh diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 0a2bde4a..c7d5b5ba 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -607,6 +607,41 @@ 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. +## Bring-up on a busy channel + +The mechanism and the fix are in `src/mt7612u/CLAUDE.md`, "MCU replies and +EP 4"; this section is the evidence. Reproducer: `tests/mt7612u_busy_bringup.sh` +(an RTL8812EU peer under `txdemo` loads the channel; the DUT is brought up +under it with `rxdemo`, each bring-up timed to its RX ring starting; the peer +was heard after every one). One MT7612U (`0e8d:7612`, USB 3), ch 100, peer +50 cm away at `DEVOURER_TX_PWR_OFFSET_QDB=-40` (−36 dBm at the DUT). + +| peer on the air during bring-up | master `09c2ff3` | with the drain | +|---|---|---| +| none (control) | 0.86–0.87 s | 0.86–0.87 s | +| 6 Mb/s legacy, 500 µs gap, 20 MHz | 1 of 6 slow (27.6 s) | 0 of 6 (0.86 s; no drain needed) | +| HT MCS3, 2 ms gap, 20 MHz | 4 of 6 slow (27.6 s) | not run | +| HT MCS3, zero gap, 20 MHz | **6 of 6 slow** (27.5 s, 9 timeouts) | **10 of 10 at 1.18 s**, 0 timeouts | +| HT MCS3 40 MHz, zero gap, DUT at HT40+ | **6 of 6 slow** (27.5 s, 9 timeouts) | **10 of 10 at 1.18 s**, 0 timeouts | + +Under full load every bring-up with the drain logged one `mcu wait: drained` +line at cmd 31 arg 3 (RXDCOC) — 8.6–10.1 kB at 20 MHz, 15.8–17.3 kB at 40 MHz — +and reached its ring 0.31 s after the control: the first 300 ms reply timeout. +An earlier run of the same arms on master, timed to the first delivered frame +instead, agreed (0 of 10, 1 of 4, 4 of 4, 6 of 6 slow). A register probe at +that first timeout read `MT_MAC_SYS_CTRL = 0` and `MT_USB_U3DMA_CFG = +0x00c40020`, and the one EP 4 read decoded was an RX descriptor followed by a +data frame from the peer's address. A second application on the same unit (an +MT7612U receiver restarted while an RTL8812EU airs ~1.7 k HT40 frames/s at it, +sessions pooled) showed 22 of 32 slow restarts before and 31 of 31 fast after, +across the drain's revisions. + +Limits: one MT7612U unit and one peer model, one channel, near field. The +stall's odds rise with the peer's load; frame rate, bytes/s and airtime all +moved together here, so which one matters is not separated. Not measured: a +ring-less (TX-only) channel set under traffic, which runs the same RXDCOC; and +whether mt76's five reply reads would now suffice in place of the ten kept. + ## Retune while the ring is up A channel change from the caller's thread while the libusb event thread is diff --git a/src/mt7612u/CLAUDE.md b/src/mt7612u/CLAUDE.md index f340e5fd..a12fda38 100644 --- a/src/mt7612u/CLAUDE.md +++ b/src/mt7612u/CLAUDE.md @@ -26,10 +26,8 @@ Two facts that measurement added. First, the reading **requires a running receiver**: with RX down the idle timer still advances, so busy+idle is non-zero and the ratio is a perfectly plausible 0% — a fabricated zero that a channel ranker reads as "emptiest". `GetChannelBusy` and `ArmChannelBusy` both -refuse unless `_rx_active`. Second, bring-up does not reliably complete while -the channel is already saturated (the arm then never becomes available, and the -MCU times out), so in a two-adapter test bring this one up BEFORE the -interferer. +refuse unless `_rx_active`. Second, the adapter can be brought up while an +interferer already runs (one peer measured): "MCU replies and EP 4" below. `ArmChannelBusy` here resets the timers and the interval mark through `mt7612u_ch_time_arm` — NOT `mt7612u_link_stats_start`, which would also clear @@ -56,6 +54,27 @@ submits 3500 fps host-side while an independent witness decodes none of it. `DEVOURER_MT7612U_PHY_TICK=0` is the control arm's switch; harness shape in the `mt7612u_phy_tick` doc comment. +## MCU replies and EP 4 + +Data parked in the RX bulk endpoint (EP 4) holds back the MCU's replies on +EP 5: with EP 4 undrained a command's reply does not arrive, and every reply of +the burst then appears within ~7 ms of the first EP 4 read. This is reachable +before any RX ring exists. During the RXDCOC calibration of a channel set +(`CMD_CALIBRATION_OP`, `MCU_CAL_RXDCOC`), received data reaches EP 4 while +`MT_MAC_SYS_CTRL` reads 0 (MAC RX and TX off) and the USB RX bulk enable +(`MT_USB_U3DMA_CFG` bit 22) is set; the one read decoded was a data frame from +the peer's address. What in the calibration lets it through is not +established. Undrained, RXDCOC and every command after it ran out the 3 s reply +budget, so a ~1 s bring-up took 27-34 s. + +`mcu_wait_resp()` therefore drains EP 4 once per command, on its first 300 ms +timeout, and only while `mt7612u_dev::ep4_owner` says nobody else reads it (the +field's comment says why it lives on the device, not the ring). A reply on time +costs nothing; a late one costs one `mt_rx_flush()`. The ten-read reply budget +above it predates this and is probably the same symptom; it was not re-measured. +Measurements and their limits: `docs/mt7612u.md`, "Bring-up on a busy channel"; +reproducer `tests/mt7612u_busy_bringup.sh`. + ## TX retries Where each piece lives: NOACK vs. ACK-requesting radiotap - diff --git a/src/mt7612u/async.cpp b/src/mt7612u/async.cpp index 37cd4394..0e0a9a64 100644 --- a/src/mt7612u/async.cpp +++ b/src/mt7612u/async.cpp @@ -216,6 +216,12 @@ int mt_async_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) a->evt_started = 1; if (cb) { + /* Under io_lock: an MCU wait holds it across its owner check and + * drain, so the claim cannot land in between. */ + d->io_lock.lock(); + int none = MT_EP4_NONE; + d->ep4_owner.compare_exchange_strong(none, MT_EP4_RING); + d->io_lock.unlock(); a->lock.lock(); a->rx_active = 1; a->lock.unlock(); @@ -313,6 +319,12 @@ void mt_async_stop(struct mt7612u_dev *d) a->evt.join(); d->a = NULL; + /* EP 4 is free again only when no RX transfer is left on it; a stranded + * one keeps the ring as owner for the rest of the device's life. */ + if (!stuck_rx) { + int ring = MT_EP4_RING; + d->ep4_owner.compare_exchange_strong(ring, MT_EP4_NONE); + } if (stuck_tx || stuck_rx) { /* Leaking the ring is the safe half. The other half is that libusb * still owns those transfers while the event thread has just been @@ -325,7 +337,8 @@ void mt_async_stop(struct mt7612u_dev *d) d->transfers_stranded = 1; ERR("async stop: %d TX and %d RX transfers still in flight after 2 s " "- leaking the ring, and the USB handle with it, rather than " - "freeing memory libusb still owns", stuck_tx, stuck_rx); + "freeing memory libusb still owns%s", stuck_tx, stuck_rx, + stuck_rx ? " (EP 4 stays owned: no MCU-wait drain on this device)" : ""); return; } diff --git a/src/mt7612u/init.cpp b/src/mt7612u/init.cpp index f1ea117b..383dce43 100644 --- a/src/mt7612u/init.cpp +++ b/src/mt7612u/init.cpp @@ -249,21 +249,30 @@ static void wcid_and_key_clear(struct mt7612u_dev *d) } /* - * Drain and discard whatever is sitting in the RX bulk endpoint. The MAC will - * happily fill its RX pool with ambient frames the moment RX is enabled, and - * nothing else in this HAL reads EP 4 unless the caller asked for RX - a full - * pool is the leading suspect for the chip wedging after repeated TX cycles - * (see BRINGUP-RESULTS.md). Cheap insurance either way. + * Drain and discard whatever is sitting in the RX bulk endpoint, and return + * how many bytes that was. The MAC will happily fill its RX pool with ambient + * frames the moment RX is enabled, and nothing reads EP 4 unless the caller + * asked for RX - a full pool is the leading suspect for the chip wedging after + * repeated TX cycles (see BRINGUP-RESULTS.md). Cheap insurance either way. + * mcu_wait_resp() also calls it, but only while d->ep4_owner says nobody else + * reads EP 4. */ -void mt_rx_flush(struct mt7612u_dev *d) +size_t mt_rx_flush(struct mt7612u_dev *d) { uint8_t buf[4096]; - int n; + size_t total = 0; for (int i = 0; i < 64; i++) { - if (mt_bulk(d, MT_EP_IN_PKT_RX, buf, sizeof buf, &n, 10) || n == 0) + int n = 0; + int rc = mt_bulk(d, MT_EP_IN_PKT_RX, buf, sizeof buf, &n, 10); + + /* A read that times out can still have moved bytes. */ + if (n > 0) + total += (size_t)n; + if (rc || n == 0) break; } + return total; } /* The value every station cell measured; the named bits must spell it. */ @@ -292,6 +301,10 @@ int mt_mac_start(struct mt7612u_dev *d, int enable_rx) return -1; } mt_wr(d, MT_RX_FILTR_CFG, MT_RX_FILTR_CFG_MANAGED); + if (enable_rx == MT_RX_DRAIN_SYNC) { + int none = MT_EP4_NONE; + d->ep4_owner.compare_exchange_strong(none, MT_EP4_SYNC); + } /* Only turn the receiver on when the caller will actually drain EP 4. * mt76's mac_start always sets both bits, but mt76 also keeps RX URBs * permanently queued; a TX-only injector that never reads has no such @@ -317,6 +330,8 @@ void mt_mac_rx_disable(struct mt7612u_dev *d) { if (!d || !d->h) return; mt_clear(d, MT_MAC_SYS_CTRL, MT_MAC_SYS_CTRL_ENABLE_RX); + int sync = MT_EP4_SYNC; /* a ring stays the owner until mt_async_stop() */ + d->ep4_owner.compare_exchange_strong(sync, MT_EP4_NONE); } int mt_mac_stop(struct mt7612u_dev *d) @@ -345,6 +360,8 @@ int mt_mac_stop(struct mt7612u_dev *d) } if (!mt_poll(d, MT_MAC_STATUS, MT_MAC_STATUS_RX, 0, 200000)) WARN("MAC RX failed to stop"); + int sync = MT_EP4_SYNC; /* a ring stays the owner until mt_async_stop() */ + d->ep4_owner.compare_exchange_strong(sync, MT_EP4_NONE); mt_wr(d, MT_TX_RTS_CFG, rts_cfg); return 0; diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 631f2507..9dfef427 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -170,6 +170,10 @@ struct mt_async { uint64_t rx_dropped; /* parser rejects: length, and invalid PHY too */ }; +#define MT_EP4_NONE 0 /* mt7612u_dev::ep4_owner values */ +#define MT_EP4_RING 1 +#define MT_EP4_SYNC 2 + struct mt7612u_dev { libusb_context *ctx; libusb_device_handle *h; @@ -190,6 +194,17 @@ struct mt7612u_dev { * then: an unconditional drain costs a guaranteed 5 ms bulk timeout on * every command, including the 3-6 inside each tune. */ uint8_t mcu_stale_pending; + /* Who reads EP 4: MT_EP4_NONE, _RING or _SYNC. On the device rather than + * the ring because it must outlive mt_async_stop()'s free of d->a: + * mcu_wait_resp() reads it from the PHY tick, which can overlap a ring + * teardown (Mt7612uRadio::StopRxLoop runs that without _mu). RING is + * claimed under io_lock before the first RX transfer is submitted, so it + * cannot land between that check and its drain, and released only once + * no RX transfer is left in flight; SYNC is held from + * mt_mac_start(MT_RX_DRAIN_SYNC) until the receiver is switched off. Every + * transition is a compare-exchange from the expected owner, so one owner + * never clears another's claim. */ + std::atomic ep4_owner{MT_EP4_NONE}; uint8_t chan; uint8_t bw; /* Recursive: the PHY tick holds this and then nests mt_vendor_req / @@ -387,7 +402,7 @@ int mt_init_hardware(struct mt7612u_dev *d, const char *fw_dir); #define MT_RX_DRAIN_RING 1 /* the async ring is already draining - asserted */ #define MT_RX_DRAIN_SYNC 2 /* caller drains with mt_rx_one() on this thread */ int mt_mac_start(struct mt7612u_dev *d, int enable_rx); -void mt_rx_flush(struct mt7612u_dev *d); +size_t mt_rx_flush(struct mt7612u_dev *d); int mt_mac_stop(struct mt7612u_dev *d); /* * Silence the receiver while leaving TX up. Must run BEFORE the EP 4 ring is diff --git a/src/mt7612u/mcu.cpp b/src/mt7612u/mcu.cpp index 9b76e34a..e9ab203c 100644 --- a/src/mt7612u/mcu.cpp +++ b/src/mt7612u/mcu.cpp @@ -23,25 +23,47 @@ static uint32_t get_le32(const uint8_t *p) ((uint32_t)p[2] << 16) | ((uint32_t)p[3] << 24); } -static int mcu_wait_resp(struct mt7612u_dev *d, uint8_t seq) +static int mcu_wait_resp(struct mt7612u_dev *d, uint8_t seq, int cmd, int len, + uint32_t arg0) { uint8_t buf[MCU_RESP_URB_SIZE]; - int len, rc; + int got, rc; + int ep4_drained = 0; /* Ten reads of 300 ms, not five. Measured on 2-1 with a peer 20 cm * away saturating the channel: the MCU answered every command, but * seconds late - seven calibrations in a row timed out at 1.5 s and * their nine replies then arrived together. mt76's own budget is the - * same five reads; it is not enough for this part under RF load. */ + * same five reads; it is not enough for this part under RF load. + * That signature matches the parked-EP-4 stall drained below, so it is + * probably the same mechanism; the budget is left at ten reads because + * five has not been re-measured with the drain in place. */ for (int i = 0; i < 10; i++) { - rc = mt_bulk(d, MT_EP_IN_CMD_RESP, buf, sizeof buf, &len, 300); - if (rc == LIBUSB_ERROR_TIMEOUT) + rc = mt_bulk(d, MT_EP_IN_CMD_RESP, buf, sizeof buf, &got, 300); + if (rc == LIBUSB_ERROR_TIMEOUT) { + /* Data parked in EP 4 holds back this EP 5 reply too + * (src/mt7612u/CLAUDE.md, "MCU replies and EP 4"). Drain it + * once per command, on the first timeout, and only while + * nobody else reads EP 4 - an RX ring or a synchronous + * drainer owns what arrives there. io_lock is held from + * this check through the drain, and a ring claims EP 4 + * under it, so none can start in between. */ + if (!ep4_drained && + d->ep4_owner.load(std::memory_order_acquire) == MT_EP4_NONE) { + size_t n = mt_rx_flush(d); + + ep4_drained = 1; + if (n) + LOG("mcu wait: drained %zu B from EP 4 (cmd %d len %d " + "arg0 0x%08x)", n, cmd, len, (unsigned)arg0); + } continue; + } if (rc) { ERR("mcu resp bulk: %s", libusb_error_name(rc)); goto unanswered; } - if (len < 4) + if (got < 4) continue; uint32_t rxfce = get_le32(buf); @@ -173,7 +195,9 @@ int mt_mcu_send(struct mt7612u_dev *d, int cmd, const void *data, int len, return -1; } - mcu_rc = wait_resp ? mcu_wait_resp(d, seq) : 0; + mcu_rc = wait_resp ? mcu_wait_resp(d, seq, cmd, len, + len >= 4 ? get_le32((const uint8_t *)data) : 0u) + : 0; d->io_lock.unlock(); return mcu_rc; } diff --git a/tests/README.md b/tests/README.md index a81eb207..005cc1ab 100644 --- a/tests/README.md +++ b/tests/README.md @@ -654,6 +654,18 @@ One adapter is enough to verify the claim; the second only buys another unit and the wall-clock. A run can end with no verdict (rc 3) when the wrap lands in the other gap — that is a re-run, which the script does once, not a defect. +### `mt7612u_busy_bringup.sh`: MT7612U bring-up on a channel already in use + +A Realtek peer under `txdemo` floods the channel at zero gap, then the MT7612U +is brought up `REPS` times under it with `rxdemo` and each bring-up is timed to +its RX ring starting; one bring-up before the peer starts is the control. It +guards the RXDCOC / EP 4 stall (`src/mt7612u/CLAUDE.md`, "MCU replies and +EP 4"): on master a loaded bring-up took ~27 s instead of ~1 s. Needs a build +with `-DDEVOURER_MT7612U=ON` and runs as root under `mt7612u_sta_lib.sh`'s rules +(private `OUT`, one run per `OUT`, recorded-PID kills, both adapters handed +back). A lighter peer makes a pass meaningless - the stall's odds fall with the +peer's load; numbers in `docs/mt7612u.md`, "Bring-up on a busy channel". + ## Supported DUTs Listed in `SUPPORTED_DUTS` at the top of `regress.py`. Extend the dict diff --git a/tests/busy_window_probe.cpp b/tests/busy_window_probe.cpp index d47b617a..30ef3188 100644 --- a/tests/busy_window_probe.cpp +++ b/tests/busy_window_probe.cpp @@ -262,9 +262,8 @@ int main(int argc, char **argv) { /* Bring-up finishes asynchronously, and the arm refuses until the * receiver is actually running, so wait for it rather than guessing. */ /* Bring-up finishes asynchronously and the arm refuses until the receiver - * is actually running, so wait for it rather than guessing. Note the - * MT7612U does not always get there while the channel is already - * saturated — bring it up before the interferer, not after. */ + * is actually running, so wait for it rather than guessing. (An MT7612U + * bring-up under a running interferer: tests/mt7612u_busy_bringup.sh.) */ for (int t = 0; t < 60; t++) { nap(250); if (dev->ArmChannelBusy(1000) != 0) { diff --git a/tests/busy_window_probe.sh b/tests/busy_window_probe.sh index 6e171d9c..83e4e92f 100755 --- a/tests/busy_window_probe.sh +++ b/tests/busy_window_probe.sh @@ -388,8 +388,10 @@ else fi if [ "$realtek_sensor" != "1" ]; then - echo "== SKIPPING the loaded arms: a MediaTek sensor must be brought up" - echo " BEFORE the interferer. Run those arms by hand:" + echo "== SKIPPING the loaded arms: they were written for a MediaTek sensor" + echo " brought up BEFORE the interferer and have not been re-run with it" + echo " started second (its bring-up stall is fixed: src/mt7612u/CLAUDE.md," + echo " \"MCU replies and EP 4\"). Run those arms by hand:" echo " build/BusyWindowProbe --vid $SENSOR_VID --pid $SENSOR_PID \\" echo " --channel $CHANNEL --mode window --reps 45 --rx &" echo " # then start the flooder once it prints its first sample" diff --git a/tests/mt7612u_busy_bringup.sh b/tests/mt7612u_busy_bringup.sh new file mode 100755 index 00000000..be192847 --- /dev/null +++ b/tests/mt7612u_busy_bringup.sh @@ -0,0 +1,194 @@ +#!/bin/sh +# mt7612u_busy_bringup.sh - does an MT7612U come up in about a second when the +# channel is already busy? +# +# The stall this guards against (src/mt7612u/CLAUDE.md, "MCU replies and +# EP 4"): frames on the channel reach EP 4 during the RXDCOC calibration of a +# channel set, before any RX ring reads EP 4, and while they sit there the +# MCU's EP 5 replies are held back too. Every command from RXDCOC on then ran +# out its 3 s reply budget, so a bring-up that takes ~1 s on a quiet channel +# took 27-34 s - on some bring-ups, not all, which is why one run proves little. +# +# How often depends on the peer's load (docs/mt7612u.md, "Bring-up on a busy +# channel"): a light peer stalls a bring-up rarely or never, a peer at zero gap +# stalled every one measured. The defaults are that shape; with a lighter peer +# a passing run means little. +# +# A Realtek adapter under devourer's txdemo floods the channel with the +# canonical test frame first. The DUT is then brought up REPS times under it +# with rxdemo, each timed from launch to its RX ring starting (the last step of +# bring-up); a canonical frame arriving afterwards (rx.txhit) shows the peer +# was audible. One more bring-up before the peer starts is the control. +# +# Needs a build with -DDEVOURER_MT7612U=ON (rxdemo must carry the backend). +# +# sudo tests/mt7612u_busy_bringup.sh +# sudo env DUT_SYSFS=8-1 PEER_SYSFS=1-1 PEER_PID=0xa81a CH=100 \ +# tests/mt7612u_busy_bringup.sh +# sudo env BW=40 PEER_RATE=MCS3/40/LDPC tests/mt7612u_busy_bringup.sh +# +# Exit status: 0 every loaded bring-up reached its RX ring within SLOW_MS; 1 +# at least one did not; 2 INCONCLUSIVE (the rig was refused, the control did +# not come up, a bring-up exited early or would not exit, or no bring-up heard +# the peer); 3 interrupted (INT/TERM). +# +# Env: DUT_SYSFS, PEER_VID, PEER_PID, PEER_SYSFS, CH, BW, PEER_RATE, REPS, PEER_GAP_US, +# PEER_PWR_QDB, SLOW_MS, FW_DIR, BUILD, OUT. + +set -u +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +BUILD="${BUILD:-$ROOT/build}" +cd "$ROOT" || exit 1 +DUT_SYSFS="${DUT_SYSFS:-7-1}" +PEER_VID="${PEER_VID:-0x0bda}" +PEER_PID="${PEER_PID:-0xc812}" +PEER_SYSFS="${PEER_SYSFS:-5-1}" +CH="${CH:-36}" +# Width of the DUT's tune and of the peer's frames; 40 = HT40+ on CH. +BW="${BW:-20}" +# Peer TX rate (txdemo DEVOURER_TX_RATE grammar); empty = txdemo's 6M default. +PEER_RATE="${PEER_RATE-MCS3/$BW/LDPC}" +REPS="${REPS:-10}" +# Zero gap: as many frames as the peer can air (see the load note above). +PEER_GAP_US="${PEER_GAP_US:-0}" +# Near-field rigs: keep the peer well below its calibrated power. +PEER_PWR_QDB="${PEER_PWR_QDB:--40}" +# Healthy bring-ups measured ~0.9-1.3 s, stalled ones 27-34 s. +SLOW_MS="${SLOW_MS:-10000}" +FW_DIR="${FW_DIR:-/lib/firmware/mediatek}" +OUT="${OUT:-}" + +[ "$(id -u)" = 0 ] || { echo "must run as root"; exit 2; } +for b in rxdemo txdemo; do + [ -x "$BUILD/$b" ] || { echo "missing $BUILD/$b - build first"; exit 2; } +done +# The option defaults OFF, and an rxdemo without the backend cannot open the +# DUT - which would read as "never came up", not as a missing build. +grep -q '^DEVOURER_MT7612U:BOOL=ON' "$BUILD/CMakeCache.txt" 2>/dev/null || { + echo "INCONCLUSIVE: $BUILD was not configured with -DDEVOURER_MT7612U=ON"; exit 2; } +# shellcheck source=tests/mt7612u_sta_lib.sh +. "$ROOT/tests/mt7612u_sta_lib.sh" +sta_out_prepare || exit 2 +sta_lock_take || exit 2 +sta_pid_init dut peer +sta_peer_record || { sta_lock_release; exit 2; } + +CLEANED=no +# shellcheck disable=SC2317,SC2329 # reached through the traps below +cleanup() { + trap '' INT TERM + [ "$CLEANED" = yes ] && return 0 + CLEANED=yes + sta_pid_kill_hard dut; dut_gone=$? + sta_pid_kill_hard peer INT; peer_gone=$? + if [ "$dut_gone" = 0 ]; then sta_dut_handback + else echo "DUT still running - not re-enumerating DUT_SYSFS=$DUT_SYSFS"; fi + if [ "$peer_gone" = 0 ]; then sta_peer_handback + else echo "peer still running - not re-enumerating PEER_SYSFS=$PEER_SYSFS"; fi + sta_lock_release +} +trap cleanup EXIT +trap 'cleanup; exit 3' INT TERM + +sta_dut_take || exit 2 + +# A sysfs path is -; the demos select by bus number + port path. +DUT_BUS="${DUT_SYSFS%%-*}"; DUT_PORT="${DUT_SYSFS#*-}" +PEER_BUS="${PEER_SYSFS%%-*}"; PEER_PORT="${PEER_SYSFS#*-}" + +# Milliseconds since boot from /proc/uptime (10 ms grain; no GNU date %N). +now_ms() { awk '{ printf "%d\n", $1 * 1000 }' /proc/uptime; } + +# One bring-up. $1 = tag. Timed from launch to the RX ring starting (the last +# step of bring-up; the same end point loaded or not), then up to 5 s more for +# a canonical frame to show the peer is audible. Prints three words: +# +# <1 if a canonical frame arrived, else 0> +# STUCK = the DUT would not die; the run must stop rather than start another +# on a held adapter. Everything else this function says goes to stderr, so it +# cannot land in the caller's $(...). +bringup() { + tag="$1" + : > "$OUT/dut_$tag.jsonl" + start=$(now_ms) + env DEVOURER_VID=0x0e8d DEVOURER_PID=0x7612 DEVOURER_USB_BUS="$DUT_BUS" \ + DEVOURER_USB_PORT="$DUT_PORT" DEVOURER_CHANNEL="$CH" DEVOURER_BW="$BW" \ + DEVOURER_MT7612U_FW_DIR="$FW_DIR" DEVOURER_LOG_LEVEL=info \ + "$BUILD/rxdemo" >"$OUT/dut_$tag.jsonl" 2>"$OUT/dut_$tag.log" & + sta_pid_record dut $! + ms=NONE; heard=0; t=0 + while [ "$t" -lt 600 ]; do + if grep -q 'RX transfers in flight' "$OUT/dut_$tag.log" 2>/dev/null; then + ms=$(( $(now_ms) - start )); break + fi + sta_pid_live dut || { ms=DIED; break; } + sleep 0.1; t=$((t + 1)) + done + if [ "$ms" != NONE ] && [ "$ms" != DIED ]; then + t=0 + while [ "$t" -lt 50 ]; do + grep -q '"ev":"rx.txhit"' "$OUT/dut_$tag.jsonl" 2>/dev/null && { heard=1; break; } + sleep 0.1; t=$((t + 1)) + done + fi + sta_pid_kill_hard dut >&2 + sta_pid_live dut && ms=STUCK + echo "$ms $heard $(grep -c 'mcu command timed out' "$OUT/dut_$tag.log")" +} + +echo "DUT MT7612U at $DUT_SYSFS; peer $PEER_VID:$PEER_PID at $PEER_SYSFS;" \ + "ch$CH ${BW} MHz, peer ${PEER_RATE:-6M} gap ${PEER_GAP_US} us;" \ + "$REPS loaded bring-ups; slow above $SLOW_MS ms; logs in $OUT" + +# Control: the same bring-up before the peer starts. +# shellcheck disable=SC2046 # three words by construction +set -- $(bringup control) +case "$1" in + STUCK) echo "INCONCLUSIVE: the control DUT would not exit"; exit 2 ;; + NONE|DIED) echo "INCONCLUSIVE: the DUT did not come up on a quiet channel ($1)"; exit 2 ;; +esac +echo " control (no peer yet): $1 ms to the RX ring, $3 MCU timeouts" + +# Recorded before the launch: an INT between the two must still hand it back. +sta_peer_opened +env DEVOURER_VID="$PEER_VID" DEVOURER_PID="$PEER_PID" DEVOURER_USB_BUS="$PEER_BUS" \ + DEVOURER_USB_PORT="$PEER_PORT" DEVOURER_CHANNEL="$CH" DEVOURER_HOP_BW="$BW" \ + ${PEER_RATE:+DEVOURER_TX_RATE="$PEER_RATE"} \ + DEVOURER_TX_GAP_US="$PEER_GAP_US" DEVOURER_TX_PAYLOAD_BYTES=1400 \ + DEVOURER_TX_PWR_OFFSET_QDB="$PEER_PWR_QDB" DEVOURER_LOG_LEVEL=warn \ + "$BUILD/txdemo" >/dev/null 2>"$OUT/peer.log" & +sta_pid_record peer $! +sleep 5 +sta_pid_live peer || { echo "INCONCLUSIVE: the peer exited (see $OUT/peer.log)"; exit 2; } + +slow=0; heard=0; died=0 +i=1 +while [ "$i" -le "$REPS" ]; do + # shellcheck disable=SC2046 # three words by construction + set -- $(bringup "load$i") + case "$1" in + STUCK) echo "INCONCLUSIVE: bring-up $i would not exit"; exit 2 ;; + DIED) died=$((died + 1)); r="exited before its RX ring" ;; + NONE) slow=$((slow + 1)); r="no RX ring within 60 s" ;; + *) [ "$1" -gt "$SLOW_MS" ] && slow=$((slow + 1)); r="$1 ms to the RX ring" ;; + esac + heard=$((heard + $2)) + printf ' bring-up %2d: %s, %s MCU timeouts, peer heard: %s\n' "$i" "$r" "$3" "$2" + sleep 3 + i=$((i + 1)) +done + +if [ "$died" -gt 0 ]; then + echo "INCONCLUSIVE: $died bring-up(s) exited early (see $OUT/dut_load*.log)" + exit 2 +fi +if [ "$heard" = 0 ]; then + echo "INCONCLUSIVE: no bring-up heard the peer - the channel was not loaded" + exit 2 +fi +if [ "$slow" -gt 0 ]; then + echo "FAIL: $slow of $REPS bring-ups under load took over $SLOW_MS ms" + exit 1 +fi +echo "PASS: all $REPS bring-ups under load reached the RX ring within $SLOW_MS ms" +exit 0 From 6c84a49b949ccaf6ae53cee324232ab2c2fb0382 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 29 Oct 2026 22:32:35 +0100 Subject: [PATCH 2/5] mt7612u: EP 4 drain review fixes - partial reads, one drain budget, refused claims From the review on #510: - mt_rx_flush() ended on a read that timed out after moving bytes, though more may be queued behind them; it now ends on an empty read, a hard error or its read budget. mcu_wait_resp() drained once per command; it drains again on a later timeout if the previous drain found data, all of a command's drains sharing one 64-read budget, so the per-command bound stays 3 s of reply reads + 0.64 s. - mt_async_start() submitted RX transfers, and mt_mac_start(MT_RX_DRAIN_SYNC) enabled RX, even when their ep4_owner claim failed. Both now refuse. The SYNC claim is taken under io_lock (the lock the drain's owner check holds) and before any register write, so a refusal leaves a running ring's receiver and filter untouched; a SYNC claimed by a call that then fails is released. - tests/mt7612u_busy_bringup.sh: the peer is checked before every loaded bring-up and every bring-up that comes up must hear it, else INCONCLUSIVE; a slow bring-up is FAIL ahead of that. - CLAUDE.md scoped: the MCU-wait drain never shares EP 4 with a ring or a synchronous drainer; the teardown/recovery flushes do not consult the owner. Hardware, same rig, master d0a9e10 as control: master 3/3 slow at each width (27.5 s); this 10/10 + 10/10 fast (1.18-1.20 s; one at 0.87 s needed no drain), and 20/20 with an RTL8812BU peer. docs/mt7612u.md has the table. ctest 86/86 with -DDEVOURER_MT7612U=ON. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011dReg1L6xtBhf95NpAeuge --- docs/mt7612u.md | 11 +++++++++++ src/mt7612u/CLAUDE.md | 20 +++++++++++++++---- src/mt7612u/async.cpp | 12 ++++++++++-- src/mt7612u/init.cpp | 37 +++++++++++++++++++++++++++-------- src/mt7612u/internal.h | 5 ++++- src/mt7612u/mcu.cpp | 23 +++++++++++++--------- tests/mt7612u_busy_bringup.sh | 21 +++++++++++--------- 7 files changed, 96 insertions(+), 33 deletions(-) diff --git a/docs/mt7612u.md b/docs/mt7612u.md index c7d5b5ba..8912e6ca 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -636,6 +636,17 @@ MT7612U receiver restarted while an RTL8812EU airs ~1.7 k HT40 frames/s at it, sessions pooled) showed 22 of 32 slow restarts before and 31 of 31 fast after, across the drain's revisions. +After review (the flush continues past a timed-out read that moved bytes; a +command's drains share one 64-read budget; both EP 4 readers claim it under +`io_lock` and a failed claim refuses to start), same rig, same day, master +`d0a9e10` as the control: master 3 of 3 slow at each width (27.5 s, 9 timeouts +each); the revised drain 10 of 10 at 1.18–1.20 s at 20 MHz (one drain each, +4.3–10.1 kB) and 10 of 10 at 40 MHz — nine at 1.18–1.19 s with one drain of +15.8–17.3 kB, one at 0.87 s whose reply came on time and needed none. With an +RTL8812BU as the peer instead (MCS3 zero gap, 20 and 40 MHz): 20 of 20 at +1.18–1.19 s, every one with one drain. No bring-up needed a second drain, so +that path is unexercised on hardware. + Limits: one MT7612U unit and one peer model, one channel, near field. The stall's odds rise with the peer's load; frame rate, bytes/s and airtime all moved together here, so which one matters is not separated. Not measured: a diff --git a/src/mt7612u/CLAUDE.md b/src/mt7612u/CLAUDE.md index a12fda38..6fe431ef 100644 --- a/src/mt7612u/CLAUDE.md +++ b/src/mt7612u/CLAUDE.md @@ -67,10 +67,22 @@ the peer's address. What in the calibration lets it through is not established. Undrained, RXDCOC and every command after it ran out the 3 s reply budget, so a ~1 s bring-up took 27-34 s. -`mcu_wait_resp()` therefore drains EP 4 once per command, on its first 300 ms -timeout, and only while `mt7612u_dev::ep4_owner` says nobody else reads it (the -field's comment says why it lives on the device, not the ring). A reply on time -costs nothing; a late one costs one `mt_rx_flush()`. The ten-read reply budget +`mcu_wait_resp()` therefore drains EP 4 on a 300 ms timeout, and only while +`mt7612u_dev::ep4_owner` says nobody else reads it (the field's comment says +why it lives on the device, not the ring). It drains again on a later timeout +only if the previous drain moved data; all of one command's drains share one +64-read budget (10 ms reads), so a command still costs at most its 3 s of +reply reads plus 0.64 s. A drain ends on an empty read, a hard error or that +budget — not on a timed-out read that moved bytes, since more may be queued +behind them. A reply on time costs nothing; the measured late ones cost one +drain each, during RXDCOC: 8.6–10.1 kB at 20 MHz, 15.8–17.3 kB at 40 MHz, +with the same ~0.3 s on the bring-up either way (`docs/mt7612u.md`). The RX +ring and the synchronous drain claim EP 4 under `io_lock`, the lock the +drain's owner check runs under, and a failed claim refuses to start either +one (before `mt_mac_start` writes a register), so the MCU-wait drain never +shares EP 4 with them. The teardown and recovery flushes (`mt_mac_stop`, +`mt_init_hardware`, `mt_recover_usb`) do not consult the owner; their +callers order them after the ring is reaped. The ten-read reply budget above it predates this and is probably the same symptom; it was not re-measured. Measurements and their limits: `docs/mt7612u.md`, "Bring-up on a busy channel"; reproducer `tests/mt7612u_busy_bringup.sh`. diff --git a/src/mt7612u/async.cpp b/src/mt7612u/async.cpp index 0e0a9a64..1567140c 100644 --- a/src/mt7612u/async.cpp +++ b/src/mt7612u/async.cpp @@ -219,9 +219,17 @@ int mt_async_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) /* Under io_lock: an MCU wait holds it across its owner check and * drain, so the claim cannot land in between. */ d->io_lock.lock(); - int none = MT_EP4_NONE; - d->ep4_owner.compare_exchange_strong(none, MT_EP4_RING); + int owner = MT_EP4_NONE; + const bool claimed = d->ep4_owner.compare_exchange_strong(owner, MT_EP4_RING); d->io_lock.unlock(); + /* A synchronous drainer would share EP 4 with the ring, and its + * release (SYNC -> NONE) would later let mcu_wait_resp() drain the + * ring's frames. (A stranded ring keeps RING, but that state is + * refused above, on transfers_stranded.) */ + if (!claimed) { + ERR("RX ring not started: EP 4 already has a reader (owner %d)", owner); + goto fail; + } a->lock.lock(); a->rx_active = 1; a->lock.unlock(); diff --git a/src/mt7612u/init.cpp b/src/mt7612u/init.cpp index 383dce43..9a9a1245 100644 --- a/src/mt7612u/init.cpp +++ b/src/mt7612u/init.cpp @@ -257,19 +257,24 @@ static void wcid_and_key_clear(struct mt7612u_dev *d) * mcu_wait_resp() also calls it, but only while d->ep4_owner says nobody else * reads EP 4. */ -size_t mt_rx_flush(struct mt7612u_dev *d) +size_t mt_rx_flush(struct mt7612u_dev *d, int *reads_left) { uint8_t buf[4096]; size_t total = 0; + int own = 64; + int *left = reads_left ? reads_left : &own; - for (int i = 0; i < 64; i++) { + while (*left > 0) { int n = 0; int rc = mt_bulk(d, MT_EP_IN_PKT_RX, buf, sizeof buf, &n, 10); - /* A read that times out can still have moved bytes. */ + (*left)--; + /* A read that times out can still have moved bytes, and then + * more may be queued behind them: an empty read, a hard error or + * the read budget ends the drain. */ if (n > 0) total += (size_t)n; - if (rc || n == 0) + if (n == 0 || (rc && rc != LIBUSB_ERROR_TIMEOUT)) break; } return total; @@ -293,18 +298,34 @@ int mt_mac_start(struct mt7612u_dev *d, int enable_rx) "mt7612u_rx_start() first, or pass MT_RX_DRAIN_SYNC"); return -1; } + /* SYNC claims EP 4 before any register is touched: a ring already + * reading it would share it with this thread's mt_rx_one(), and a + * refusal must leave that ring's receiver and filter as they are. SYNC + * is already ours on a repeated start. Under io_lock, as the ring's + * claim is: mcu_wait_resp() holds it from its owner check through its + * drain, so the claim cannot land in between. */ + bool claimed_now = false; + if (enable_rx == MT_RX_DRAIN_SYNC) { + std::lock_guard lk(d->io_lock); + int owner = MT_EP4_NONE; + claimed_now = d->ep4_owner.compare_exchange_strong(owner, MT_EP4_SYNC); + if (!claimed_now && owner != MT_EP4_SYNC) { + ERR("mac_start(MT_RX_DRAIN_SYNC) while an RX ring owns EP 4"); + return -1; + } + } mt_wr(d, MT_MAC_SYS_CTRL, MT_MAC_SYS_CTRL_ENABLE_TX); if (!mt_poll(d, MT_WPDMA_GLO_CFG, MT_WPDMA_GLO_CFG_TX_DMA_BUSY | MT_WPDMA_GLO_CFG_RX_DMA_BUSY, 0, 200000)) { ERR("mac_start: WPDMA stayed busy"); + if (claimed_now) { /* the receiver never came on: nothing to own */ + int sync = MT_EP4_SYNC; + d->ep4_owner.compare_exchange_strong(sync, MT_EP4_NONE); + } return -1; } mt_wr(d, MT_RX_FILTR_CFG, MT_RX_FILTR_CFG_MANAGED); - if (enable_rx == MT_RX_DRAIN_SYNC) { - int none = MT_EP4_NONE; - d->ep4_owner.compare_exchange_strong(none, MT_EP4_SYNC); - } /* Only turn the receiver on when the caller will actually drain EP 4. * mt76's mac_start always sets both bits, but mt76 also keeps RX URBs * permanently queued; a TX-only injector that never reads has no such diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 9dfef427..70eb0925 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -402,7 +402,10 @@ int mt_init_hardware(struct mt7612u_dev *d, const char *fw_dir); #define MT_RX_DRAIN_RING 1 /* the async ring is already draining - asserted */ #define MT_RX_DRAIN_SYNC 2 /* caller drains with mt_rx_one() on this thread */ int mt_mac_start(struct mt7612u_dev *d, int enable_rx); -size_t mt_rx_flush(struct mt7612u_dev *d); +/* Drain EP 4: reads until an empty read, a hard error, or *reads_left reads + * (64 when reads_left is null), decrementing *reads_left per read. Returns the + * bytes drained. */ +size_t mt_rx_flush(struct mt7612u_dev *d, int *reads_left = nullptr); int mt_mac_stop(struct mt7612u_dev *d); /* * Silence the receiver while leaving TX up. Must run BEFORE the EP 4 ring is diff --git a/src/mt7612u/mcu.cpp b/src/mt7612u/mcu.cpp index e9ab203c..6e6294d4 100644 --- a/src/mt7612u/mcu.cpp +++ b/src/mt7612u/mcu.cpp @@ -28,7 +28,8 @@ static int mcu_wait_resp(struct mt7612u_dev *d, uint8_t seq, int cmd, int len, { uint8_t buf[MCU_RESP_URB_SIZE]; int got, rc; - int ep4_drained = 0; + int ep4_reads = 64; /* one drain budget for the whole command */ + size_t ep4_last = 1; /* bytes the previous drain moved (1 = none yet) */ /* Ten reads of 300 ms, not five. Measured on 2-1 with a peer 20 cm * away saturating the channel: the MCU answered every command, but @@ -43,16 +44,20 @@ static int mcu_wait_resp(struct mt7612u_dev *d, uint8_t seq, int cmd, int len, if (rc == LIBUSB_ERROR_TIMEOUT) { /* Data parked in EP 4 holds back this EP 5 reply too * (src/mt7612u/CLAUDE.md, "MCU replies and EP 4"). Drain it - * once per command, on the first timeout, and only while - * nobody else reads EP 4 - an RX ring or a synchronous - * drainer owns what arrives there. io_lock is held from - * this check through the drain, and a ring claims EP 4 - * under it, so none can start in between. */ - if (!ep4_drained && + * on a timeout - again on a later one only if the previous + * drain found data (the channel can refill EP 4 while the + * MCU works) - all drains of one command sharing one 64-read + * budget, so a command costs at most the 3 s of reply reads + * plus 64 drain reads of 10 ms, as with a single drain. And + * only while nobody else reads EP 4: an RX ring or a + * synchronous drainer owns what arrives there. io_lock is + * held from this check through the drain, and both claim + * EP 4 under it, so neither can start in between. */ + if (ep4_reads > 0 && ep4_last && d->ep4_owner.load(std::memory_order_acquire) == MT_EP4_NONE) { - size_t n = mt_rx_flush(d); + size_t n = mt_rx_flush(d, &ep4_reads); - ep4_drained = 1; + ep4_last = n; if (n) LOG("mcu wait: drained %zu B from EP 4 (cmd %d len %d " "arg0 0x%08x)", n, cmd, len, (unsigned)arg0); diff --git a/tests/mt7612u_busy_bringup.sh b/tests/mt7612u_busy_bringup.sh index be192847..132c15c2 100755 --- a/tests/mt7612u_busy_bringup.sh +++ b/tests/mt7612u_busy_bringup.sh @@ -29,8 +29,9 @@ # # Exit status: 0 every loaded bring-up reached its RX ring within SLOW_MS; 1 # at least one did not; 2 INCONCLUSIVE (the rig was refused, the control did -# not come up, a bring-up exited early or would not exit, or no bring-up heard -# the peer); 3 interrupted (INT/TERM). +# not come up, a bring-up exited early or would not exit, the peer was not +# running before a bring-up, or a bring-up that came up did not hear the peer +# - each loaded run has to prove its own load); 3 interrupted (INT/TERM). # # Env: DUT_SYSFS, PEER_VID, PEER_PID, PEER_SYSFS, CH, BW, PEER_RATE, REPS, PEER_GAP_US, # PEER_PWR_QDB, SLOW_MS, FW_DIR, BUILD, OUT. @@ -161,18 +162,19 @@ sta_pid_record peer $! sleep 5 sta_pid_live peer || { echo "INCONCLUSIVE: the peer exited (see $OUT/peer.log)"; exit 2; } -slow=0; heard=0; died=0 +slow=0; deaf=0; died=0 i=1 while [ "$i" -le "$REPS" ]; do + sta_pid_live peer || { echo "INCONCLUSIVE: the peer exited before bring-up $i (see $OUT/peer.log)"; exit 2; } # shellcheck disable=SC2046 # three words by construction set -- $(bringup "load$i") case "$1" in STUCK) echo "INCONCLUSIVE: bring-up $i would not exit"; exit 2 ;; DIED) died=$((died + 1)); r="exited before its RX ring" ;; NONE) slow=$((slow + 1)); r="no RX ring within 60 s" ;; - *) [ "$1" -gt "$SLOW_MS" ] && slow=$((slow + 1)); r="$1 ms to the RX ring" ;; + *) [ "$1" -gt "$SLOW_MS" ] && slow=$((slow + 1)); r="$1 ms to the RX ring" + [ "$2" = 1 ] || deaf=$((deaf + 1)) ;; esac - heard=$((heard + $2)) printf ' bring-up %2d: %s, %s MCU timeouts, peer heard: %s\n' "$i" "$r" "$3" "$2" sleep 3 i=$((i + 1)) @@ -182,13 +184,14 @@ if [ "$died" -gt 0 ]; then echo "INCONCLUSIVE: $died bring-up(s) exited early (see $OUT/dut_load*.log)" exit 2 fi -if [ "$heard" = 0 ]; then - echo "INCONCLUSIVE: no bring-up heard the peer - the channel was not loaded" - exit 2 -fi +# A slow bring-up is the stall whether or not its own run heard the peer. if [ "$slow" -gt 0 ]; then echo "FAIL: $slow of $REPS bring-ups under load took over $SLOW_MS ms" exit 1 fi +if [ "$deaf" -gt 0 ]; then + echo "INCONCLUSIVE: $deaf bring-up(s) came up without hearing the peer - not proven loaded" + exit 2 +fi echo "PASS: all $REPS bring-ups under load reached the RX ring within $SLOW_MS ms" exit 0 From cee80f48e76e7761c525f553e6714abb4540f4ad Mon Sep 17 00:00:00 2001 From: snokvist Date: Fri, 30 Oct 2026 17:08:16 +0100 Subject: [PATCH 3/5] mt7612u: any error ends an EP 4 drain, and a full-buffer timeout is not data 6c84a49 kept draining past a timed-out read that reported bytes. On one host (libusb 1.0.30, kernel 6.18, xhci) a timed-out read of an idle EP 4 reports n == len with no data, so every flush ran its whole 64-read budget: ~2 s on every bring-up, quiet channel included (review 5708821119). Any error now ends the drain, a timeout included; a drain that ended on a timeout with real data is re-run by mcu_wait_resp() on its next reply timeout. A timed-out read reporting a full buffer is also not counted: a read that really fills the buffer completes, and counting the artefact would make every MCU-wait drain look like data, re-drain on each later reply timeout and log bytes that were never there. docs/mt7612u.md: one table against master d0a9e10, numbers from this code. Same rig (MT7612U 8-1, RTL8812EU peer at MCS3 zero gap, ch 100): master 3 of 3 slow at 20 and at 40 MHz (27.5-27.6 s, 9 timeouts each); this code 10 of 10 at each width, 1.18-1.20 s, one drain each (8.6-10.1 kB at 20 MHz, 15.8-17.3 kB at 40 MHz), control 0.86-0.87 s. Not run on the host that shows the artefact. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011dReg1L6xtBhf95NpAeuge --- docs/mt7612u.md | 56 ++++++++++++++++++++---------------------- src/mt7612u/CLAUDE.md | 11 ++++++--- src/mt7612u/init.cpp | 17 +++++++++---- src/mt7612u/internal.h | 7 +++--- 4 files changed, 50 insertions(+), 41 deletions(-) diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 8912e6ca..7bcd4b82 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -616,40 +616,36 @@ under it with `rxdemo`, each bring-up timed to its RX ring starting; the peer was heard after every one). One MT7612U (`0e8d:7612`, USB 3), ch 100, peer 50 cm away at `DEVOURER_TX_PWR_OFFSET_QDB=-40` (−36 dBm at the DUT). -| peer on the air during bring-up | master `09c2ff3` | with the drain | +| peer on the air during bring-up | master `d0a9e10` | with the drain | |---|---|---| | none (control) | 0.86–0.87 s | 0.86–0.87 s | -| 6 Mb/s legacy, 500 µs gap, 20 MHz | 1 of 6 slow (27.6 s) | 0 of 6 (0.86 s; no drain needed) | -| HT MCS3, 2 ms gap, 20 MHz | 4 of 6 slow (27.6 s) | not run | -| HT MCS3, zero gap, 20 MHz | **6 of 6 slow** (27.5 s, 9 timeouts) | **10 of 10 at 1.18 s**, 0 timeouts | -| HT MCS3 40 MHz, zero gap, DUT at HT40+ | **6 of 6 slow** (27.5 s, 9 timeouts) | **10 of 10 at 1.18 s**, 0 timeouts | - -Under full load every bring-up with the drain logged one `mcu wait: drained` -line at cmd 31 arg 3 (RXDCOC) — 8.6–10.1 kB at 20 MHz, 15.8–17.3 kB at 40 MHz — -and reached its ring 0.31 s after the control: the first 300 ms reply timeout. -An earlier run of the same arms on master, timed to the first delivered frame -instead, agreed (0 of 10, 1 of 4, 4 of 4, 6 of 6 slow). A register probe at -that first timeout read `MT_MAC_SYS_CTRL = 0` and `MT_USB_U3DMA_CFG = -0x00c40020`, and the one EP 4 read decoded was an RX descriptor followed by a -data frame from the peer's address. A second application on the same unit (an -MT7612U receiver restarted while an RTL8812EU airs ~1.7 k HT40 frames/s at it, -sessions pooled) showed 22 of 32 slow restarts before and 31 of 31 fast after, -across the drain's revisions. - -After review (the flush continues past a timed-out read that moved bytes; a -command's drains share one 64-read budget; both EP 4 readers claim it under -`io_lock` and a failed claim refuses to start), same rig, same day, master -`d0a9e10` as the control: master 3 of 3 slow at each width (27.5 s, 9 timeouts -each); the revised drain 10 of 10 at 1.18–1.20 s at 20 MHz (one drain each, -4.3–10.1 kB) and 10 of 10 at 40 MHz — nine at 1.18–1.19 s with one drain of -15.8–17.3 kB, one at 0.87 s whose reply came on time and needed none. With an -RTL8812BU as the peer instead (MCS3 zero gap, 20 and 40 MHz): 20 of 20 at -1.18–1.19 s, every one with one drain. No bring-up needed a second drain, so -that path is unexercised on hardware. +| HT MCS3, zero gap, 20 MHz | **3 of 3 slow** (27.6 s, 9 timeouts) | **10 of 10 at 1.18–1.20 s**, 0 timeouts | +| HT MCS3 40 MHz, zero gap, DUT at HT40+ | **3 of 3 slow** (27.5–27.6 s, 9 timeouts) | **10 of 10 at 1.18 s**, 0 timeouts | + +Under load every bring-up with the drain logged one `mcu wait: drained` line +at cmd 31 arg 3 (RXDCOC) — 8.6–10.1 kB at 20 MHz, 15.8–17.3 kB at 40 MHz — and +reached its ring 0.31–0.33 s after the control: the first 300 ms reply +timeout. No bring-up needed a second drain, so that path is unexercised on +hardware. The mechanism was read without the drain: a register probe at +that first timeout read `MT_MAC_SYS_CTRL = 0` and +`MT_USB_U3DMA_CFG = 0x00c40020`, and the one EP 4 read decoded was an RX +descriptor followed by a data frame from the peer's address. + +The drain stops on any error, a timeout included, and does not count a +timed-out read that reports a full buffer: on one host (libusb 1.0.30, kernel +6.18, xhci) a timed-out read of an idle EP 4 reports `n == len` with no data, +and a drain that read on past it spent its whole 64-read budget on every flush, +adding ~2 s to every bring-up, quiet channel included. On the host measured +above a timed-out read reports what it really moved. On that host the exit +rule measured 1.79 s quiet and 2.04–2.11 s loaded (four bring-ups) in a build +that still counted the full-buffer read; the code as it stands, which does +not, has not been run there. Limits: one MT7612U unit and one peer model, one channel, near field. The -stall's odds rise with the peer's load; frame rate, bytes/s and airtime all -moved together here, so which one matters is not separated. Not measured: a +stall's odds rise with the peer's load: without the drain, lighter peers (6 +Mb/s legacy at a 500 µs gap, MCS3 at a 2 ms gap) stalled some bring-ups, not +all; those arms have not been run with this code. Frame rate, bytes/s and airtime +all moved together, so which one matters is not separated. Not measured: a ring-less (TX-only) channel set under traffic, which runs the same RXDCOC; and whether mt76's five reply reads would now suffice in place of the ten kept. diff --git a/src/mt7612u/CLAUDE.md b/src/mt7612u/CLAUDE.md index 6fe431ef..d2d3db6a 100644 --- a/src/mt7612u/CLAUDE.md +++ b/src/mt7612u/CLAUDE.md @@ -72,9 +72,14 @@ budget, so a ~1 s bring-up took 27-34 s. why it lives on the device, not the ring). It drains again on a later timeout only if the previous drain moved data; all of one command's drains share one 64-read budget (10 ms reads), so a command still costs at most its 3 s of -reply reads plus 0.64 s. A drain ends on an empty read, a hard error or that -budget — not on a timed-out read that moved bytes, since more may be queued -behind them. A reply on time costs nothing; the measured late ones cost one +reply reads plus 0.64 s. A drain ends on any error (a timeout included), an +empty read or that budget. Bytes moved is not a reason to go on: on one host +(libusb 1.0.30, kernel 6.18, xhci) a timed-out read of an idle EP 4 reports a +full buffer with no data, and a drain that continued past it read its whole +budget on every flush, ~2 s per bring-up; such a full-buffer timeout is not +counted as data either. In `mcu_wait_resp()`, a drain that ended on a timeout +with data is re-run on the next reply timeout, which covers a read cut short. +A reply on time costs nothing; the measured late ones cost one drain each, during RXDCOC: 8.6–10.1 kB at 20 MHz, 15.8–17.3 kB at 40 MHz, with the same ~0.3 s on the bring-up either way (`docs/mt7612u.md`). The RX ring and the synchronous drain claim EP 4 under `io_lock`, the lock the diff --git a/src/mt7612u/init.cpp b/src/mt7612u/init.cpp index 9a9a1245..8ec6b5b0 100644 --- a/src/mt7612u/init.cpp +++ b/src/mt7612u/init.cpp @@ -269,12 +269,19 @@ size_t mt_rx_flush(struct mt7612u_dev *d, int *reads_left) int rc = mt_bulk(d, MT_EP_IN_PKT_RX, buf, sizeof buf, &n, 10); (*left)--; - /* A read that times out can still have moved bytes, and then - * more may be queued behind them: an empty read, a hard error or - * the read budget ends the drain. */ - if (n > 0) + /* Any error ends the drain, a timeout included: on one host a + * timed-out read of an idle EP 4 reports n == len with no data, + * so bytes moved is no reason to go on - continuing ran the whole + * 64-read budget, 0.64 s, on every flush. Nor is such a read + * counted: a read that really filled the buffer completes, so a + * timeout reporting a full one is that artefact, and counting it + * would make every mcu_wait_resp() drain look like data and + * re-drain on each later reply timeout. A drain that ended on a + * timeout with real data is re-run by mcu_wait_resp() on its next + * reply timeout (ep4_last > 0), which covers a partial read. */ + if (n > 0 && !(rc == LIBUSB_ERROR_TIMEOUT && n == (int)sizeof buf)) total += (size_t)n; - if (n == 0 || (rc && rc != LIBUSB_ERROR_TIMEOUT)) + if (rc || n == 0) break; } return total; diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 70eb0925..78d67092 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -402,9 +402,10 @@ int mt_init_hardware(struct mt7612u_dev *d, const char *fw_dir); #define MT_RX_DRAIN_RING 1 /* the async ring is already draining - asserted */ #define MT_RX_DRAIN_SYNC 2 /* caller drains with mt_rx_one() on this thread */ int mt_mac_start(struct mt7612u_dev *d, int enable_rx); -/* Drain EP 4: reads until an empty read, a hard error, or *reads_left reads - * (64 when reads_left is null), decrementing *reads_left per read. Returns the - * bytes drained. */ +/* Drain EP 4: reads until an empty read, any error (a timeout included), or + * *reads_left reads (64 when reads_left is null), decrementing *reads_left per + * read. Returns the bytes drained; a timed-out read reporting a full buffer is + * not counted (see the definition). */ size_t mt_rx_flush(struct mt7612u_dev *d, int *reads_left = nullptr); int mt_mac_stop(struct mt7612u_dev *d); /* From 2da8bf1bfb2d1a4a1a9081ad2fd4c37949018899 Mon Sep 17 00:00:00 2001 From: snokvist Date: Fri, 30 Oct 2026 17:08:16 +0100 Subject: [PATCH 4/5] tests: mt7612u_busy_bringup refuses a rig whose firmware is unreadable The script never checked the MT7612U blobs, so a host with compressed firmware only died in the control and printed a misleading INCONCLUSIVE ('did not come up on a quiet channel'). It now picks the directory the library will load from (FW_DIR, /lib/firmware/mediatek, ./firmware, as resolve_fw_dir and sta_client_onair.sh do) and refuses through sta_fw_readable, which names the compressed files. No ./firmware link is needed: rxdemo takes the directory from DEVOURER_MT7612U_FW_DIR. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011dReg1L6xtBhf95NpAeuge --- tests/mt7612u_busy_bringup.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/mt7612u_busy_bringup.sh b/tests/mt7612u_busy_bringup.sh index 132c15c2..9dc286b2 100755 --- a/tests/mt7612u_busy_bringup.sh +++ b/tests/mt7612u_busy_bringup.sh @@ -69,6 +69,18 @@ grep -q '^DEVOURER_MT7612U:BOOL=ON' "$BUILD/CMakeCache.txt" 2>/dev/null || { echo "INCONCLUSIVE: $BUILD was not configured with -DDEVOURER_MT7612U=ON"; exit 2; } # shellcheck source=tests/mt7612u_sta_lib.sh . "$ROOT/tests/mt7612u_sta_lib.sh" +# The firmware the library will load: the first of FW_DIR, +# /lib/firmware/mediatek and ./firmware that holds both blobs - the order and +# the test of resolve_fw_dir (src/mt7612u/Mt7612uRadio.cpp), as +# sta_client_onair.sh does. It must be readable: a host with compressed +# firmware only would otherwise read as a DUT that "did not come up". +fw_pick="" +for d in "$FW_DIR" /lib/firmware/mediatek firmware; do + if [ -e "$d/mt7662_rom_patch.bin" ] && [ -e "$d/mt7662.bin" ]; then + fw_pick="$d"; break + fi +done +sta_fw_readable "${fw_pick:-$FW_DIR}" || exit 2 sta_out_prepare || exit 2 sta_lock_take || exit 2 sta_pid_init dut peer From 447ec95ada564c89394b2a88f3277a3f5c4e2602 Mon Sep 17 00:00:00 2001 From: snokvist Date: Fri, 30 Oct 2026 17:08:16 +0100 Subject: [PATCH 5/5] tests: describe the MT7612U busy-channel bring-up as it is now busy_window_probe.sh said the stall 'is fixed'; it now says what is true: the MediaTek sensor can start second, and these arms have not been re-run that way. tests/README.md said 'on master', which stops being true when this merges: 'without the drain'. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011dReg1L6xtBhf95NpAeuge --- tests/README.md | 2 +- tests/busy_window_probe.sh | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/README.md b/tests/README.md index 005cc1ab..cdabbffd 100644 --- a/tests/README.md +++ b/tests/README.md @@ -660,7 +660,7 @@ A Realtek peer under `txdemo` floods the channel at zero gap, then the MT7612U is brought up `REPS` times under it with `rxdemo` and each bring-up is timed to its RX ring starting; one bring-up before the peer starts is the control. It guards the RXDCOC / EP 4 stall (`src/mt7612u/CLAUDE.md`, "MCU replies and -EP 4"): on master a loaded bring-up took ~27 s instead of ~1 s. Needs a build +EP 4"): without the drain a loaded bring-up took ~27 s instead of ~1 s. Needs a build with `-DDEVOURER_MT7612U=ON` and runs as root under `mt7612u_sta_lib.sh`'s rules (private `OUT`, one run per `OUT`, recorded-PID kills, both adapters handed back). A lighter peer makes a pass meaningless - the stall's odds fall with the diff --git a/tests/busy_window_probe.sh b/tests/busy_window_probe.sh index 83e4e92f..dfd8fc09 100755 --- a/tests/busy_window_probe.sh +++ b/tests/busy_window_probe.sh @@ -389,9 +389,10 @@ fi if [ "$realtek_sensor" != "1" ]; then echo "== SKIPPING the loaded arms: they were written for a MediaTek sensor" - echo " brought up BEFORE the interferer and have not been re-run with it" - echo " started second (its bring-up stall is fixed: src/mt7612u/CLAUDE.md," - echo " \"MCU replies and EP 4\"). Run those arms by hand:" + echo " brought up BEFORE the interferer. It can start second (a busy channel" + echo " no longer stalls its bring-up: src/mt7612u/CLAUDE.md, \"MCU replies" + echo " and EP 4\"), but these arms have not been re-run that way. Run them" + echo " by hand:" echo " build/BusyWindowProbe --vid $SENSOR_VID --pid $SENSOR_PID \\" echo " --channel $CHANNEL --mode window --reps 45 --rx &" echo " # then start the flooder once it prints its first sample"