mt7612u: drain EP 4 when an MCU reply is late, so RXDCOC cannot stall bring-up

docs/mt7612u.md

@@ -607,6 +607,48 @@ 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 `d0a9e10` | with the drain |
+|---|---|---|
+| none (control) | 0.86–0.87 s | 0.86–0.87 s |
+| 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: 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.
+
 ## Retune while the ring is up
 
 A channel change from the caller's thread while the libusb event thread is

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,44 @@ 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 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 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
+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`.
+
 ## TX retries
 
 Where each piece lives: NOACK vs. ACK-requesting radiotap -

src/mt7612u/async.cpp

@@ -216,6 +216,20 @@ 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 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();
@@ -313,6 +327,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 +345,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;
 	}
 

src/mt7612u/init.cpp

@@ -249,21 +249,42 @@ 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, int *reads_left)
 {
 	uint8_t buf[4096];
-	int n;
-
-	for (int i = 0; i < 64; i++) {
-		if (mt_bulk(d, MT_EP_IN_PKT_RX, buf, sizeof buf, &n, 10) || n == 0)
+	size_t total = 0;
+	int own = 64;
+	int *left = reads_left ? reads_left : &own;
+
+	while (*left > 0) {
+		int n = 0;
+		int rc = mt_bulk(d, MT_EP_IN_PKT_RX, buf, sizeof buf, &n, 10);
+
+		(*left)--;
+		/* 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 (rc || n == 0)
 			break;
 	}
+	return total;
 }
 
 /* The value every station cell measured; the named bits must spell it. */
@@ -284,11 +305,31 @@ 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<std::recursive_mutex> 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);
@@ -317,6 +358,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 +388,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;

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<int> 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,11 @@ 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);
+/* 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);
 /*
  * Silence the receiver while leaving TX up. Must run BEFORE the EP 4 ring is

src/mt7612u/mcu.cpp

@@ -23,25 +23,52 @@ 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_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
 	 * 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
+			 * 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, &ep4_reads);
+
+				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);
+			}
 			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 +200,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;
 }

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"): 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
+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

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) {