mt7612u: fix the receive collapse (1 Hz PHY tick) and recover the USB wedge

[snokvist]

Summary

Follow-up to #412 (merged as daabab7). Library-internal — no IRtlDevice
surface. Two faults found by exercising the subtree the way a ground station
would, both device-verified, plus a hardware review of the whole delta.

Public header (for the wrapper)

One addition, no changed signatures: int mt7612u_phy_tick(struct mt7612u_dev *dev) in include/mt7612u/mt7612u.h. It is required for
receive — a consumer that receives must call it about once a second or the
front end goes deaf against a strong transmitter (§1). The IRtlDevice wrapper
should call it on a 1 Hz timer once it drives RX; everything else in the header
is unchanged, so the rename and integration PRs that only consume this header
are unaffected apart from adding that one call. docs/mt7612u.md is updated in
this PR for the lock and the RX-undrained recovery story, as requested.

1. A receiver went deaf against a strong transmitter

Against a peer ~20 cm away airing 1400-byte HT-MCS7 at 3037 fps, arx took
3 frames in 10 s. Not the adapter (symmetric both directions; both hear
400–1000 fps of ambient on ch1/36/40/44/48), not the ring (its own counter
agreed at 3), not the air (an RTL8812AU witness counted 103805 frames from the
same transmitter). mt76 runs cal_work every second
(MT_CALIBRATE_INTERVAL == HZ, mt76x2/usb_phy.c:42); this port ran none of
it. Bisected with one verified peer per arm:

arm	frames / 10 s
nothing	3 (reproduced 8×)
MT_RX_STAT_* reads only	3
a periodic MCU calibration only	43902
the ported gain tracking only	4
the full tick	49002

New public mt7612u_phy_tick(), caller-driven (MCU commands share one 4-bit
sequence number and one response endpoint), called once a second from every
receiving path. Final reliability, both adapters as receiver, single verified
peer: eight consecutive runs at 5415–5470 fps. The MCU answers slowly under
RF load — nine calibration timeouts per bring-up on this bench — so
mcu_wait_resp() waits 10×300 ms and mt_mcu_send() drains any late reply off
EP 5 before the next command (without which the reply channel desynced and
cascaded mcu resp mismatch … (want 1)). The calibration takes regardless of
those timeouts; the channel is marked calibrated once, as mt76 does.

2. A run killed mid-transfer left the adapter unable to load firmware

Register reads and writes still round-trip, MAC and RF are fine, and the
kernel mt76x2u driver cannot bind it either — every bulk OUT NAKs.
libusb_reset_device (already on every open) does not reach it. Isolated one
step at a time against a freshly wedged adapter: clear_halt on all four
endpoints, MAC + USB DMA stop, WLAN_EN/WLAN_CLK_EN down, and the PBF block reset
each left it wedged; pulsing MT_USB_DMA_CFG_TX_CLR — a bit mt76 declares and
never writes — clears it alone. That plus an RX-pipe drain is mt_recover_usb(),
which runs after identify on both open paths — mt_open() and mt_adopt() —
so a libusb-owning consumer arriving through mt7612u_open_handle() self-heals
too. (Review fix: it lived in mt_open() alone, which was precisely the
consumer this subtree exists for getting none of it. bringup adopt now
exercises that path.)

A separately-observed transient: a post-kill firmware upload fails for 3–8 s
then succeeds by itself (12/12 kills, no repair applied) — a settling window,
not a wedge. mt_fw_init() absorbs it with one bounded retry after a 3 s
settle. On a healthy idle adapter the recovery is a no-op: 0/40 healing-open
failures.

Known issue, called out honestly: a harder tier — TX_BUSY stuck at
0x80c00020 — was seen once and is not cleared by any of the above; it
needed a physical replug, and it has not reproduced since (36+ kills).
docs/mt7612u-usb-wedge.md traces the vendor tree's separate UDMA and IFDMA/FCE
resets that none of the attempts reached, implemented as bringup swreset and
still awaiting a reproduction to test against. mt_open() warns clearly when it
hits this state rather than failing with an opaque timeout.

Verification

• Receiver: 8/8 runs 5415–5470 fps, both adapters.
• Wedge recovery: soft tier 3/3; healthy-adapter soak 0/40; short stress on the
  shipping binary 0/12 across the scenarios that actually wedge.
• Post-kill transient: 12/12 clear by themselves, absorbed by the loader retry.
• mt_mac_start() now refuses to enable the receiver with no ring draining
  (MT_RX_DRAIN_*; the public mt7612u_start() always derived this, three
  gates bypassed it).
• make check green; strict -Wall -Wextra -Wshadow -Wmissing-prototypes
  build clean.

Retracted in the course of this

Two claims made and then falsified: that the inverted ring/receiver order in
arx explained the 3 frames (it didn't), and that a synchronous MCU command
"hangs forever" beside the RX ring's event thread — it never reproduced in 8
stack-capture attempts, and libusb's own source shows that shape is supported.
The comments say so where they used to say otherwise.

Not in scope

The vendor-neutral rename, IRtlDevice wiring, and a proven recovery for the
stuck-TX_BUSY tier (needs a reproduction first).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj

---

Review round (head 4a6a14c)

Both blocking items and all eight inline points are addressed; see the review
reply for the point-by-point. Headlines: the recovery is shared by both open
paths, teardown clears ENABLE_RX before the EP 4 ring is cancelled, the EP 5
drain is armed rather than unconditional, the ambient-RSSI EMA is gone in favour
of mt76's monitor behaviour, and the library no longer reads
MT7612U_NO_AUTORECOVER (MT7612U_DEV stays deferred per #412).

Re-verified on this head — a different stimulus from §1's, so quoted
separately rather than folded into that figure. Peer: a second MT7612U flooding
ch149; DUT: MT7612U at 2-1.

check	result
RX rate	4902–4913 fps, 5 runs, rx_err/rx_invalid/rx_dropped all 0, peer verified airing before and after every run
adopt path	bringup adopt brings the device up; with MT7612U_NO_AUTORECOVER=1 the recovery announces itself on that path
MCU drain	9 calibration timeouts under RF load produced one drain of 9 replies, where the old code paid a 5 ms timeout before every command
retune	530 ms full / 50.6 ms fast
build	strict -Wall -Wextra -Wshadow -Wmissing-prototypes clean; make check green

Not reproduced this session: the soft USB wedge (5 kill cycles with TX+RX up,
U3DMA_CFG stayed healthy each time). So "the adopt path recovers a wedged
adapter" is a code-path fact — one shared function, two call sites — not a
measurement from this round.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

mt7612u: restore RX with PHY ticks and recover USB wedges

🐞 Bug fix ✨ Enhancement 📝 Documentation 🕐 40+ Minutes

AI Description

• Adds caller-driven 1 Hz PHY calibration and gain tracking to prevent RX collapse.
• Recovers soft USB DMA wedges and retries transient firmware upload failures.
• Enforces active RX draining and documents unresolved hard-wedge reset evidence.

Diagram

graph TD
  Consumer(["RX Consumer"]) -->|"1 Hz call"| Tick["PHY Tick"] --> MCU["MCU Calibration"] --> Device["MT7612U Device"]
  Parser["RX Parser"] -->|"RSSI sample"| Tick
  Tick --> Gain["Gain Tracking"] --> Device
  Open(["Device Open"]) --> Recovery["USB Recovery"] -->|"drain and retry"| Device

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Library-owned PHY timer

• ➕ Prevents receiving consumers from accidentally omitting the required tick.
• ➕ Provides a consistent calibration cadence independent of caller loops.
• ➖ Adds worker-thread lifecycle and shutdown complexity to the library.
• ➖ Requires stronger coordination with the shared MCU sequence and response endpoint.
• ➖ Introduces background behavior into an otherwise caller-driven API.

2. Full vendor reset during every open

• ➕ Could eventually cover both soft and hard DMA wedge states.
• ➕ Centralizes recovery into one comprehensive reset path.
• ➖ The UDMA and IFDMA/FCE sequence remains unverified on a hard-wedged device.
• ➖ Broad resets are more disruptive than the proven TX-clear recovery.
• ➖ Would increase open latency and risk corrupting otherwise healthy initialization.

Recommendation: Keep the caller-driven PHY tick and targeted open-time TX-clear recovery. They match the library's explicit lifecycle, serialize shared MCU traffic, and use device-verified recovery steps; retain the full vendor reset as an opt-in diagnostic until it is validated against a reproduced hard wedge.

Files changed (12) +776 / -44

Enhancement (3) +36 / -0

mt7612u.hExpose the required PHY maintenance tick +15/-0

Expose the required PHY maintenance tick

• Adds the public mt7612u_phy_tick() declaration and documents its cadence, receive requirement, MCU ownership constraints, statistics interaction, and error behavior.

src/mt7612u/include/mt7612u/mt7612u.h

internal.hAdd calibration, locking, and RX drain state +15/-0

Add calibration, locking, and RX drain state

• Extends calibration state with current AGC values, gain adjustment, gain class, and averaged RSSI. Adds the recursive I/O mutex and explicit none, ring, and synchronous RX drain modes.

src/mt7612u/internal.h

rx.cFeed averaged RSSI into PHY gain tracking +6/-0

Feed averaged RSSI into PHY gain tracking

• Maintains a lightweight moving average of primary-chain RSSI during frame parsing. The PHY tick uses this value to select and update receiver gain classes.

src/mt7612u/rx.c

Bug fix (6) +314 / -7

fw.cRetry firmware initialization after transient failures +18/-3

Retry firmware initialization after transient failures

• Retries ROM patch and firmware loading once after a three-second settling delay. This absorbs measured post-termination upload failures that resolve without additional recovery.

src/mt7612u/fw.c

init.cPrevent undrained receiver startup +13/-0

Prevent undrained receiver startup

• Rejects ring-based RX startup unless the asynchronous ring is active, preventing EP4 from filling without a consumer. Device close now destroys the recursive I/O mutex.

src/mt7612u/init.c

mcu.cSerialize MCU commands and recover delayed replies +42/-3

Serialize MCU commands and recover delayed replies

• Doubles the MCU response wait budget, serializes each command-response transaction, and drains bounded stale responses before sending. This prevents delayed replies from permanently desynchronizing sequence matching.

src/mt7612u/mcu.c

phy.cImplement periodic PHY calibration and gain tracking +164/-0

Implement periodic PHY calibration and gain tracking

• Ports mt76-style RSSI and false-CCA gain adjustment and adds mt7612u_phy_tick(). Each tick performs guarded channel calibration, an MCU temperature calibration, and channel-gain maintenance.

src/mt7612u/phy.c

regs.hDefine the USB DMA TX-clear control +1/-0

Define the USB DMA TX-clear control

• Adds the MT_USB_DMA_CFG_TX_CLR bit definition used by open-time soft-wedge recovery.

src/mt7612u/regs.h

usb.cRecover USB DMA state during device open +76/-1

Recover USB DMA state during device open

• Serializes control transfers with a recursively initialized I/O mutex. Device open now disables RX, drains pending input, pulses TX-clear to repair soft wedges, warns on persistent TX_BUSY, and supports disabling automatic recovery for experiments.

src/mt7612u/usb.c

Tests (1) +153 / -31

bringup.cExercise PHY ticks, drain modes, and software reset +153/-31

Exercise PHY ticks, drain modes, and software reset

• Updates receiving gates to tick once per second and start RX only after a valid drainer exists, while preserving a no-tick negative control. Adds vendor software-reset diagnostics, richer RX counters, and explicit drain modes across all MAC startup paths.

src/mt7612u/tools/bringup.c

Documentation (2) +273 / -6

mt7612u-usb-wedge.mdDocument hard USB wedge reset evidence +215/-0

Document hard USB wedge reset evidence

• Adds a source-backed investigation of the unrecovered TX_BUSY wedge, including vendor UDMA and IFDMA/FCE reset sequences, EP0 encodings, diagnostics, and experimental requirements. It clearly separates verified observations from untested recovery candidates.

docs/mt7612u-usb-wedge.md

mt7612u.mdDocument PHY tick and USB recovery requirements +58/-6

Document PHY tick and USB recovery requirements

• Explains the mandatory 1 Hz receive tick, measured RX behavior, drain-before-enable invariant, soft and hard wedge tiers, firmware retry, and I/O locking model.

docs/mt7612u.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Consumers can silently lose recovery ✓ Resolved 📘 Rule violation ≡ Correctness

Description

mt_open() reads MT7612U_NO_AUTORECOVER directly with getenv() and returns before the device
recovery sequence. Any host process that inherits or sets this variable bypasses both receive-pipe
cleanup and transmit-DMA repair for every library entry point, outside the caller's programmatic
configuration.

Code

src/mt7612u/usb.c[540]

+	if (getenv("MT7612U_NO_AUTORECOVER")) {

Evidence

Compliance rule 5 prohibits library implementation code from reading environment variables directly.
The added getenv() branch changes mt_open() behavior and skips recovery without receiving that
setting through a programmatic interface.

CLAUDE.md: Keep the Library Independent of Environment Variables: CLAUDE.md: Keep the Library Independent of Environment Variables: CLAUDE.md: Keep the Library Independent of Environment Variables: CLAUDE.md: Keep the Library Independent of Environment Variables
src/mt7612u/usb.c[538-544]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The library directly reads an environment variable to control automatic device recovery.
## Issue Context
Library behavior must be configured through the programmatic configuration boundary; environment parsing belongs in demo applications.
## Fix Focus Areas
- src/mt7612u/usb.c[538-544]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Maintainers can miss receive changes ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

docs/mt7612u.md restates the required one-second cadence, calibration behavior, shared MCU
endpoint rationale, and receive impact already documented on mt7612u_phy_tick() in the public
header. When the cadence or side effects change, maintainers must update both copies and consumers
can receive stale integration guidance from either source.

Code

docs/mt7612u.md[R167-170]

+no tick took **3 frames in 10 s**; calling `mt7612u_phy_tick()` once a second
+it takes **~4850/s** (nine runs). Not the adapter — symmetric in both
+directions, and both hear 400–1000 fps of ambient on ch1/36/40/44/48 — not
+the ring (its own counter agreed at 3), and not the air: an RTL8812AU witness

Evidence

Compliance rule 2 requires documentation to point to authoritative header comments rather than
restating their contracts. Both added regions describe the mandatory one-second call, the no-tick
and tick measurements, the calibration operation, and the single-user MCU response constraint.

CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md: CLAUDE.md: Do Not Duplicate Header Documentation in CLAUDE.md
docs/mt7612u.md[160-181]
src/mt7612u/include/mt7612u/mt7612u.h[289-302]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The receive-tick API contract is duplicated between the public header and repository documentation.
## Issue Context
Keep the public header authoritative, while retaining only unique experimental evidence or implementation background in the longer document.
## Fix Focus Areas
- docs/mt7612u.md[160-181]
- src/mt7612u/include/mt7612u/mt7612u.h[289-302]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Adopted devices use an invalid mutex ✓ Resolved 🐞 Bug ☼ Reliability

Description

mt7612u_open_handle() reaches mt_vendor_req() through mt_adopt() and mt_identify() before
the recursive io_lock is initialized, because mutex setup occurs only in mt_open(). Every
adopted handle therefore locks an uninitialized mutex during identification and later has it
unconditionally destroyed by mt7612u_close(), causing undefined pthread behavior that can
deadlock, crash, or fail unpredictably before device bring-up.

Code

src/mt7612u/usb.c[81]

+	pthread_mutex_lock(&d->io_lock);

Evidence

The public adopted-handle path calls mt_adopt(), which immediately invokes mt_identify() and
performs a register read through mt_vendor_req(); that request path now locks io_lock. The cited
code shows that recursive mutex initialization exists only in the separate mt_open() path, while
the public close path unconditionally destroys the mutex for both opened and adopted devices.

src/mt7612u/init.c[460-482]
src/mt7612u/usb.c[269-297]
src/mt7612u/usb.c[73-94]
src/mt7612u/usb.c[469-494]
src/mt7612u/init.c[460-483]
src/mt7612u/usb.c[269-298]
src/mt7612u/usb.c[75-94]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Initialize `io_lock` through a common device-initialization path that runs before any register I/O, including identification performed by `mt_adopt()` / `mt7612u_open_handle()`. Track successful initialization so every failure and close path destroys the mutex exactly once.
## Issue Context
`mt_adopt()` identifies the adapter immediately, and identification performs a register read through `mt_vendor_req()`, which now locks `io_lock`. Recursive mutex setup currently occurs only in `mt_open()`, leaving adopted handles to use and later destroy an uninitialized mutex; move device-local synchronization setup into a shared helper used by both open paths.
## Fix Focus Areas
- src/mt7612u/usb.c[81-93]
- src/mt7612u/usb.c[289-298]
- src/mt7612u/usb.c[469-494]
- src/mt7612u/init.c[448-483]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View action required (3)

4. Adopted receivers use invalid gain ✓ Resolved 🐞 Bug ≡ Correctness

Description

low_gain is initialized to -1 only in mt_open(), so devices created by mt7612u_open_handle()
retain zero and skip the first forced gain-class setup. On the first tick an ordinary fallback RSSI
produces no class change, while high false-CCA can call phy_set_gain_val() with the still-zero
agc_gain_cur values and wrap the programmed unsigned gains.

Code

src/mt7612u/usb.c[R491-493]

+		pthread_mutex_init(&d->io_lock, &ma);
+		pthread_mutexattr_destroy(&ma);
+		d->cal.low_gain = -1;

Evidence

The normal open path sets the sentinel, but the adopted-handle path calls mt_adopt() and
bring_up() directly. Gain tracking only initializes agc_gain_cur when it detects a gain-class
change, and its false-CCA branch can otherwise program those zero-initialized values.

src/mt7612u/init.c[448-473]
src/mt7612u/usb.c[482-494]
src/mt7612u/phy.c[512-525]
src/mt7612u/phy.c[561-563]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The adopted-handle lifecycle bypasses initialization required by the new periodic gain tracker.
## Issue Context
Initialize `low_gain` and any other new tracking sentinels in a common device-state initializer used by both public open paths before channel setup or PHY ticks.
## Fix Focus Areas
- src/mt7612u/usb.c[491-493]
- src/mt7612u/init.c[448-473]
- src/mt7612u/phy.c[512-525]
- src/mt7612u/phy.c[561-563]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Retunes reuse the previous gain state ✓ Resolved 🐞 Bug ≡ Correctness

Description

phy_update_channel_gain() caches low_gain, avg_rssi_all, agc_gain_adjust, and current gain
values, but mt_set_channel_ex() does not reset them when changing channel or band. The first ticks
after a retune classify the new channel using the old channel's RSSI and can retain its programmed
gains indefinitely when both channels fall into the same broad class; fast retunes additionally skip
refreshing agc_gain_init.

Code

src/mt7612u/phy.c[R517-520]

+	low_gain = (d->cal.avg_rssi_all > rssi_gain_thresh(d->bw)) +
+	           (d->cal.avg_rssi_all > low_rssi_gain_thresh(d->bw));
+
+	gain_change = d->cal.low_gain < 0 || ((d->cal.low_gain & 2) ^ (low_gain & 2));

Evidence

Channel setup changes chan, bw, and calibration inputs without resetting any newly added
tracking fields. The tracker only updates agc_gain_cur on a class transition, and fast mode
returns before taking the new channel's AGC snapshot.

src/mt7612u/phy.c[512-525]
src/mt7612u/phy.c[561-563]
src/mt7612u/phy.c[620-634]
src/mt7612u/phy.c[701-709]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Periodic gain tracking retains channel-specific RSSI and AGC state across channel changes.
## Issue Context
Reset the moving average, gain-class sentinel, fine adjustment, and current gain state when a tune starts. Ensure fast retunes also establish valid baseline AGC gains before the next tick can use them.
## Fix Focus Areas
- src/mt7612u/phy.c[512-525]
- src/mt7612u/phy.c[561-563]
- src/mt7612u/phy.c[625-634]
- src/mt7612u/phy.c[701-709]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Some receive tools still miss ticks 🐞 Bug ≡ Correctness

Description

The new wait_ticking() helper is not used by every RX-enabled tool: gate_duplex() omits ticks
throughout its transmit flood, gate_linkstat() keeps using wait_ms(), and gate_caps() performs
more than a second of active-RX work without a tick. Runs lasting beyond one second therefore enter
the exact no-calibration receive condition this PR documents as collapsing reception, making their
measurements and pass/fail results unreliable.

Code

src/mt7612u/tools/bringup.c[R61-63]

+/* wait_ms() plus mt7612u_phy_tick() once a second - what every receiving
+ * loop must do, see the public header.  Same return contract as wait_ms(). */
+static int wait_ticking(double ms)

Evidence

The public API states that every receiving consumer must tick about once per second. These three
gates enable RX but contain active intervals exceeding that cadence with no call to either
mt7612u_phy_tick() or wait_ticking().

src/mt7612u/include/mt7612u/mt7612u.h[289-302]
src/mt7612u/tools/bringup.c[971-1014]
src/mt7612u/tools/bringup.c[1230-1331]
src/mt7612u/tools/bringup.c[1581-1601]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Several RX-enabled bring-up commands still fail to invoke the required one-Hz PHY tick.
## Issue Context
Integrate monotonic tick scheduling into every interval where MAC RX remains enabled, including tight TX loops and statistics loops. Keep the tick cadence independent of individual transmission or sampling duration.
## Fix Focus Areas
- src/mt7612u/tools/bringup.c[61-74]
- src/mt7612u/tools/bringup.c[971-979]
- src/mt7612u/tools/bringup.c[1230-1331]
- src/mt7612u/tools/bringup.c[1585-1601]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Receive parsing races gain tracking ✓ Resolved 🐞 Bug ☼ Reliability

Description

mt_rx_parse() updates d->cal.avg_rssi_all on the libusb event thread while
phy_update_channel_gain() reads and can overwrite the same field on the caller-driven PHY-tick
thread, and the RX path never acquires the tick's io_lock. Any asynchronous receive concurrent
with the documented second-thread tick usage therefore races the active RX ring, allowing samples to
be lost and gain decisions to use undefined state rather than merely a one-second-stale RSSI value.

Code

src/mt7612u/rx.c[R110-112]

+	d->cal.avg_rssi_all = d->cal.avg_rssi_all
+	                    ? (int8_t)((d->cal.avg_rssi_all * 7 + info->rssi[0]) / 8)
+	                    : info->rssi[0];

Evidence

Completed asynchronous transfers invoke mt_rx_parse() through rx_done() on the libusb event
thread, where the calibration field is updated. The caller-driven tick accesses the same byte for
both reads and writes inside the gain tracker, but only the tick acquires io_lock; because the
parser does not use that lock, there is no common synchronization between the accesses.

src/mt7612u/async.c[36-57]
src/mt7612u/rx.c[107-112]
src/mt7612u/phy.c[512-521]
src/mt7612u/phy.c[583-594]
src/mt7612u/phy.c[504-525]
src/mt7612u/phy.c[583-595]
src/mt7612u/async.c[35-99]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Remove the unsynchronized cross-thread access to `avg_rssi_all` between the asynchronous RX callback and the caller-driven PHY tick. Use an atomic representation with an appropriate update strategy or a dedicated calibration lock, while preserving a consistent read-modify-write update of the moving average.
## Issue Context
The RX callback runs on the libusb event thread, while `mt7612u_phy_tick()` is intentionally caller-driven and documented as safe to invoke from another thread. Although the tick holds `io_lock`, the RX-side update does not acquire it; the chosen synchronization strategy should avoid unnecessarily holding the USB I/O mutex across RX parsing or blocking RX transfer resubmission during the multi-second MCU wait.
## Fix Focus Areas
- src/mt7612u/rx.c[107-112]
- src/mt7612u/async.c[35-99]
- src/mt7612u/phy.c[504-525]
- src/mt7612u/phy.c[583-595]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. A full reply queue stays desynced 🐞 Bug ☼ Reliability

Description

The stale-reply loop stops after 16 responses but sends the next command regardless of whether
another reply remains queued. When at least 17 late responses accumulate, mcu_wait_resp() consumes
the remaining stale reply, reports a sequence mismatch, and the response channel resumes the
cascading desynchronization this drain was added to prevent.

Code

src/mt7612u/mcu.c[R88-91]

+		while (n < 16 &&
+		       !mt_bulk(d, MT_EP_IN_CMD_RESP, stale, sizeof stale, &got, 5) &&
+		       got >= 4)
+			n++;

Evidence

The loop condition terminates at 16 without recording whether termination was caused by an empty
endpoint or the cap. Execution then unconditionally allocates a new sequence and sends, while the
response waiter treats queued replies with older sequences as mismatches.

src/mt7612u/mcu.c[77-101]
src/mt7612u/mcu.c[125-134]
src/mt7612u/mcu.c[26-56]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The bounded stale-response drain proceeds with a new command even when reaching its cap proves the endpoint may still contain old replies.
## Issue Context
After draining 16 valid responses, probe or fail the MCU transaction instead of sending into a possibly nonempty response queue. Emit a clear diagnostic and leave sequence state recoverable.
## Fix Focus Areas
- src/mt7612u/mcu.c[77-95]
- src/mt7612u/mcu.c[97-101]
- src/mt7612u/mcu.c[125-134]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

9. Failed tests leave USB transfers running ✓ Resolved 🐞 Bug ☼ Reliability

Description

The changed gates start the asynchronous RX ring and return directly when mt_mac_start() fails,
without calling mt7612u_rx_stop(). When WPDMA remains busy, main() then calls mt_close() with
submitted RX transfers still owned by libusb, so the tool closes its handle underneath the active
event thread instead of safely cancelling and reaping it.

Code

src/mt7612u/tools/bringup.c[R903-906]

if (mt7612u_rx_start(&dev, arx_cb, &ctx)) {
  printf("GATE arx: FAIL - rx_start failed\n"); return 1;
}
+	if (mt_mac_start(&dev, MT_RX_DRAIN_RING)) return 1;

Evidence

Each cited gate now starts RX before MAC start and directly returns if MAC start fails.
mt_mac_start() has a reachable WPDMA-busy failure, while the process-level cleanup calls only
mt_close(), which does not stop an active ring before closing the handle.

src/mt7612u/tools/bringup.c[900-937]
src/mt7612u/tools/bringup.c[960-973]
src/mt7612u/tools/bringup.c[1504-1516]
src/mt7612u/tools/bringup.c[1743-1753]
src/mt7612u/init.c[283-289]
src/mt7612u/tools/bringup.c[2416-2430]
src/mt7612u/usb.c[601-631]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Ensure every gate that starts an RX ring stops it before returning from a later `mt_mac_start()` failure. Consolidate setup/teardown through a cleanup label or equivalent so all partially initialized paths cancel and reap the ring before `main()` closes the device.
## Issue Context
The prior ordering enabled MAC RX before creating the ring, so this failure path did not own active transfers. The PR correctly reverses the setup order, but the newly reachable failure returns need matching cleanup.
## Fix Focus Areas
- src/mt7612u/tools/bringup.c[903-906]
- src/mt7612u/tools/bringup.c[971-973]
- src/mt7612u/tools/bringup.c[1508-1509]
- src/mt7612u/tools/bringup.c[1746-1747]
- src/mt7612u/init.c[283-289]
- src/mt7612u/usb.c[601-631]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[snokvist]

Thanks — several of these are real and are now fixed in two commits on the branch. Point by point:

#3 (adopted devices use an invalid mutex) — fixed, and it was the important one. mt_adopt() → mt_identify() → mt_vendor_req() locks io_lock, which was initialised only in mt_open(). A zeroed pthread_mutex_t is a valid non-recursive lock, so the first mt7612u_phy_tick() (which locks io_lock and nests mt_vendor_req/mt_mcu_send) would self-deadlock a device opened by a libusb-owning consumer — i.e. exactly the open_handle integration path. A shared mt_dev_state_init() now runs first on both open paths, and a guarded mt_dev_state_destroy() folded into mt_close() releases it exactly once. This also closed a leak the review didn't name: every mt_open() failure path and bring_up()'s fail path previously freed the device without destroying the mutex.

#4 (adopted receivers use invalid gain) — fixed, consolidated with #6. The gain sentinels (low_gain=-1, avg_rssi_all=0, agc_gain_adjust=0) are now reset per-tune in mt_set_channel_ex() instead of once in mt_open(). Since the tick refuses to run before a channel is set, and a channel is only ever set through that function, this covers the adopted handle and the retune case with one reset.

#6 (retunes reuse the previous gain state) — fixed by that same per-tune reset.

#9 (failed tests leave USB transfers running) — fixed. The reordered gates now call mt7612u_rx_stop() on the mt_mac_start() failure path (gate_arx/gate_duplex/gate_rxbytes/gate_linkrx/gate_linkstat), and gate_linkstat's mid-loop mt7612u_link_stats() failure return does too — thanks, that second return is the same bug and wasn't in the list.

#5 (RX parsing races gain tracking) — fixed as an atomic, not a lock. cal.avg_rssi_all is now _Atomic int8_t with relaxed load/store. Deliberately not taking io_lock on the RX path: that would block receive behind the tick's multi-second MCU calibration.

#7 (some receive tools miss ticks) — fixed where it was real. gate_duplex now ticks once a second inside its TX flood. gate_linkstat already issues an MCU calibration every second via mt7612u_link_stats() (that calibration is the part that keeps RX alive — the gain half doesn't), and gate_caps measures registers rather than RX rate and only needs the ring drained, so neither was changed.

#8 (full reply queue stays desynced) — hardened. The stale-reply drain cap is raised from 16 to 64 (observed depth is ~5) so a deeper transient drains fully, with a distinct warning if the cap is actually hit (which would mean a genuinely desynced channel).

#1 (env var) and #2 (doc duplication) — no change, and here's why. MT7612U_NO_AUTORECOVER is opt-in, read only in mt_open(), announced with a log line, and mirrors the already-shipping MT7612U_DEV env convention in the same file; the adopt path never reads it. Threading a config parameter through open() for a debug toggle seemed like more surface than the toggle warrants. And docs/mt7612u.md's tick section is the measurement/bisection narrative (which sub-step restores reception), not the API contract — the header stays authoritative and the doc keeps the empirical justification that doesn't belong in a header. Happy to revisit either if you'd prefer.

Verification: make check green, strict -Wall -Wextra -Wshadow -Wmissing-prototypes clean, and the receiver re-measured at 5422–5470 fps on both adapters after the changes (7 of 8 runs; the one miss is the first run's cold-start settling right after a driver unload, which recovers on the next run).

[josephnef]

Read every commit and the head diff against daabab7, walked the vendor gain code and the lock paths, checked the nine open Qodo threads against the head. The RX-collapse work is measured and the bisect table is convincing; the lock/adopt fixes from the Qodo round are in. Two things block for me, both because the PR's own contract ("every entry point self-heals", "the receiver must never come up with nothing draining EP 4") is not yet met by the code. The rest are inline.

Blocking

1. The wedge recovery is unreachable from mt7612u_open_handle() (inline at usb.c mt_adopt). The whole TX_CLR + MAC-off + RX-flush block sits in mt_open() only. The IRtlDevice wrapper — the consumer this subtree exists for — owns libusb and will come in through open_handle / mt_adopt, and gets none of it; after a killed run it pays both mt_fw_init attempts and fails with exactly the message this PR fixes. Hoist the recovery into a shared post-identify step both paths run (like mt_dev_state_init already is).

2. Teardown still runs the receiver undrained (inline at init.c mt7612u_close). mt_async_stop() cancels and reaps the EP 4 ring first; MAC RX stays enabled until mt_mac_stop() clears MT_MAC_SYS_CTRL, which happens after its own mt_rx_flush and the TX-idle wait. Every rx_stop(); mac_stop(); pair in bringup has the same shape. That is the condition the new mt_mac_start guard exists to prevent, on the path that runs at the end of every session. Pre-existing, but this PR is the one that names the invariant. Clearing ENABLE_RX before cancelling the ring closes it.

Non-blocking, worth doing in this PR

• Fast retune leaves agc_gain_init stale while forcing a reprogram (inline phy.c:717).
• avg_rssi_all is fed by every decoded frame, neighbours included; mt76 in monitor mode has no stations and sits at the -75 class forever (inline rx.c:114). Your own bisect says gain tracking is not what fixes RX, so this is fidelity-only code that is currently less faithful than doing nothing.
• The 5 ms stale-reply drain runs before every MCU command, including the 3-6 inside every tune (inline mcu.c:91).
• Two mt_rr() results are consumed without the ~0u sentinel check on the new paths (inline usb.c:585).
• mt_fw_init retry fires on any failure, so a missing blob now costs 3 s and blames a crashed run (inline fw.c:242).
• gate_rx ticks on the thread that is the only EP 4 drainer (inline bringup.c:508).

Process

• The qodo-gate check is red only because four threads are unresolved: #1 getenv (usb.c:556), #2 docs duplication, #6 retunes (fixed on head — resolve it), #8 drain cap (head warns distinctly and sends anyway — reply with that and resolve). On #1: this repo's rule is that the library reads no environment; MT7612U_DEV was already deferred to integration in #412, so this is the second one accumulating. A field on mt7612u_open()/a bringup-only flag would end the debt; if you defer again, say so in the thread and resolve it.
• The PR body says 8 runs at 5415–5470 fps, the header says ~4850 (nine runs), phy.c says 4362/s, docs/mt7612u.md says ~4850. Pick one figure and one home (the header) — Qodo #2 is really about this drift.
• docs/mt7612u-usb-wedge.md opens with a research date, a branch name and two commit hashes. Docs here are current-state; git is the changelog. Keep the register tables and the untested-status verdict, drop the provenance header.

No MT7612U on my bench, so nothing above is hardware-verified from this side; the blocking items are code-path facts, not measurements.

[josephnef]

Blocking. mt_adopt() returns straight from mt_identify(). The recovery block (MAC_SYS_CTRL=0, mt_rx_flush, the TX_CLR pulse loop, the TX_BUSY verdict) lives only in mt_open() below. mt7612u_open_handle() is the path a libusb-owning consumer — the IRtlDevice wrapper — uses, so after a killed run it sees U3DMA_CFG=0x00c00020, nothing pulses TX_CLR, and mt_fw_init times out twice (now +3 s) with the exact failure this PR fixes for mt7612u_open(). Factor the recovery into a helper both paths run after identify, the way mt_dev_state_init already is.

[josephnef]

Blocking. Ordering inverts the invariant mt_mac_start() now enforces: mt_async_stop() cancels and reaps the EP 4 ring here, and MAC RX stays enabled until mt_mac_stop() clears MT_MAC_SYS_CTRL — after its mt_rx_flush and the up-to-150 ms TX-idle wait (longer if a transfer is stuck and the ring strands). On a busy channel that window is the undrained-EP4 overflow you describe as stopping RX DMA for good, and it runs at the end of every session. Same shape in every bringup mt7612u_rx_stop(); mt_mac_stop(); pair. Clear MT_MAC_SYS_CTRL_ENABLE_RX (or run mt_mac_stop) before cancelling the ring.

[josephnef]

low_gain = -1 here forces the next tick to reprogram AGC 8/9 from agc_gain_init — but the fast path (if (fast) return 0; at ~line 717) returns before the agc_gain_init snapshot is re-read, while mt_mcu_init_gain (both paths) has just programmed AGC 8/9 for the new channel. So the first tick after a fast retune writes the previous full tune's gain base. mt76 re-reads agc_gain_init on every set_channel. Move the two mt_rr(AGC 8/9) reads above the fast return.

[josephnef]

This EMA takes rssi[0] of every frame the monitor receiver parses — third-party transmitters included (and CRC-error frames, if the filter admits them). mt76's mt76_get_min_avg_rssi averages associated stations only; in monitor mode there are none, so mt76 sits at the -75 fallback and never leaves the middle gain class. Here a -40 dBm neighbour AP drives low_gain=2, AGC 35/37=0x08080808 and gain_cur -= 10..14 against a wanted peer at -80 dBm. The bisect says gain tracking is not what restores RX, so this is fidelity-only code that is currently less faithful than a fixed -75. Either pin it to mt76's monitor behaviour, or feed it only from frames that pass the consumer's SA filter. (Minor: the int8 divide truncates toward zero, and an average that lands on exactly 0 reseeds to -75.)

[josephnef]

This 5 ms bulk-IN timeout is paid before every command, including the 3-6 inside mt_set_channel_ex (fast retune included) and no-wait commands that never produce a reply — a guaranteed LIBUSB_ERROR_TIMEOUT each, so every hop is +15-30 ms with nothing ever drained in the healthy case. Drain only when the previous mcu_wait_resp actually timed out (one d->mcu_stale_pending flag set on that path): zero cost on the hot path, same protection.

[josephnef]

mt_rr() returns ~0u on a failed control transfer, which has TX_BUSY set: one EP0 hiccup here sends a healthy adapter through TX_BULK_EN off, 20 TX_CLR pulses (400 ms) and a false "still busy" WARN when the re-read fails too. Same in phy_adjust_vga_gain: a failed MT_RX_STAT_1 read is 65535 false CCAs and a 2 dB gain step down. Use mt_rr_chk on these two.

[josephnef]

The retry keys on any non-zero return, so a missing or short blob (slurp fails, "rom patch too short") now also WARNs that a previous run died mid-transfer and sleeps 3 s before failing identically — sending the user to the wedge doc instead of to the missing file. Retry only on the upload-transfer failure (a distinct return from fw_send_data), which is the case you measured.

[josephnef]

gate_rx runs MT_RX_DRAIN_SYNC — this thread is the only EP 4 drainer — and the tick now blocks it for up to ~3.3 s when the MCU answers late (your own measurement under RF load). That is the undrained-receiver window on the sync gate. Bringup-only, but it is the negative control for the very fault, so either stop RX around the tick here or note that gate_rx is not a valid tick witness under load.

[josephnef]

regs.h has MT_MAC_STATUS_TX = BIT(1) and MT_MAC_STATUS_RX = BIT(0). This loop (and the "TX idle=%d" print two lines down) tests BIT(0), so swreset waits for RX idle and then pulses the UDMA TX reset with TX possibly still in flight — the precondition the vendor sequence states. A FAIL from this gate then cannot separate "sequence does not work" from "run too early". Use mt_poll(&dev, MT_MAC_STATUS, MT_MAC_STATUS_TX, 0, ...) as init.c does, or the MT_USB_DMA_CFG_TX_BUSY test mt_mac_stop uses.

[josephnef]

Second point on the retry: d->io_err is not cleared between attempts, and mt_init_hardware refuses to report success while it is non-zero (init.c ~417). An attempt 1 that also tripped any EP0 accessor (mt_single_wr FCE addr/len, the CPU_DESC_IDX read-modify-write, mt_poll) before the bulk chunk timed out poisons a clean attempt 2: firmware loads, open still fails with "failed register transfers ... refusing to report success". Add mt_io_clear(d) at the top of each attempt.

[josephnef]

Review of #414 (8 findings, ranked most severe first). Each was independently verified against the PR head; line-anchored comments below.

The four I would fix before merge: the wedge recovery is reachable only via mt7612u_open() and not mt7612u_open_handle(); the RSSI EMA that drives the gain class is fed by every frame on the channel rather than mt76's associated-station average; mt7612u_phy_tick() can block the caller thread ~3 s, which for a sync-drain consumer is the undrained-EP4 state the PR itself refuses at mt_mac_start; and teardown still disables RX only after the ring is cancelled, the mirror image of the new start-side invariant.

Duplicate of review 5144973019 (same findings, posted twice by tooling); its inline comments were removed. The earlier review stands.

[snokvist]

Both blocking items were right, and both were this PR's own contract not being met by the code. All eight inline points are fixed, plus the three process items. Head is 4a6a14c.

Blocking

1. Recovery unreachable from mt7612u_open_handle(). Factored exactly as you suggested: the whole block (MAC_SYS_CTRL=0, mt_rx_flush, the TX_CLR pulse loop, the TX_BUSY verdict) is now mt_recover_usb(), called after identify on both paths, the way mt_dev_state_init() already was.

Nothing in bringup had ever opened the second entry point, so I added a gate that does: bringup adopt opens libusb itself, claims interface 0, resets, and drives mt_adopt() + mt_init_hardware() — the two steps mt7612u_open_handle() is. Driving them directly rather than calling the public function is deliberate: that function allocates the device itself, so an observe-only wedge experiment could never be configured through it. That is also what makes the verification possible —

$ sudo MT7612U_NO_AUTORECOVER=1 MT7612U_DEV=2-1 bringup adopt
devourer [I] mt7612u: auto-recovery disabled: U3DMA_CFG=0x00c40020
ADOPT: PASS

— the "auto-recovery disabled" line is emitted from inside mt_recover_usb(), so its appearance on the adopt path is the code-path fact you asked for, observed rather than argued.

Honest limit: I could not reproduce the soft wedge this session (5 kill cycles with TX+RX up; U3DMA_CFG stayed 0x00c40020 each time). So "adopt recovers a wedged adapter" rests on it being one shared function with two call sites, not on a measurement. Same footing as the hard tier already has in the PR.

2. Teardown runs the receiver undrained. Fixed. New mt_mac_rx_disable() clears ENABLE_RX only, before the ring is cancelled — in mt7612u_close(), and at every ring-cancelling site in bringup through one rx_teardown() helper rather than the pair spelled out twenty times. That includes gate_caps, which cancels the same EP 4 ring under mt_async_stop. The three mt_async_stop sites in gate_soak/gate_ampdu are deliberately untouched — both run mt_mac_start(..., MT_RX_DRAIN_NONE) with mt_async_start(&dev, NULL, NULL), so ENABLE_RX is never set. gate_rx also got it: it has no ring, but it stops draining EP 4 on its own thread while mt_mac_stop() leaves RX on for up to 150 ms.

Inline

• agc_gain_init stale on a fast retune. The two AGC 8/9 reads moved above the fast return. One correction to the reasoning, not the fix: mt76 does not re-read on every set_channel — mt76x2/usb_phy.c:170-174 returns on scan before mt76x02_init_agc_gain(), as does pci_phy.c:233-237. What differs is that mt76x02_init_agc_gain() is the only place mt76 sets low_gain = -1, so its scan path leaves low_gain alone and gain_change comes out false. This port resets it unconditionally in mt_set_channel_ex(), which forces gain_change true after any tune — so here the stale base really does get written.
• avg_rssi_all fed by every frame. Removed, and you were right that it was less faithful than doing nothing. mt76_get_min_avg_rssi() (util.c:72) averages associated stations and returns 0 for a monitor consumer, so mt76x2/phy.c:283-285 substitutes -75; phy_update_channel_gain() now uses that directly. The RX hot path does no cross-thread write at all, and the _Atomic field and <stdatomic.h> are gone from internal.h. The low_gain == 2 arms are unreachable as a result — kept, because they are the faithful port and come back with a real per-peer RSSI source, with a comment saying so since the compiler cannot see it.
• The 5 ms drain before every command. Now armed by d->mcu_stale_pending. Measured under exactly the load that motivated it: nine calibration timeouts during channel setup produced one drain of nine replies.
• Unchecked mt_rr(). mt_rr_chk() on both — the U3DMA_CFG reads in mt_recover_usb() and MT_RX_STAT_1 in phy_adjust_vga_gain().
• mt_fw_init retry too broad. A distinct return now separates a bad blob (fails immediately, naming the file) from the wire upload and from the chip not starting what was uploaded — those last two both retry, since the mt_poll timeouts are the same settling window you measured and gate_swreset's whole verdict depends on it. Your second point is fixed too, though not quite as written: mt_io_clear() would have wiped the caller's whole bracket, because mt_init_hardware() opens it before mt_power_cycle() and there is only one accumulator — a flaky power-on plus a retried upload could then have reported success with the WLAN core partly powered. It samples and restores instead.
• gate_rx ticking its own only drainer. Documented rather than worked around, and the note is in the gate's own output, not just a comment. Gating RX around the tick would have changed what the gate measures — mt_mac_start() rewrites MT_RX_FILTR_CFG to the initvals, overwriting the monitor filter gate_rx sets deliberately — and a tool-side workaround would make the gate immune by construction to the hazard it should be able to witness. gate_arx remains the valid witness, with arx <ch> <secs> 1 its negative control.
• gate_swreset polling the wrong bit. Now mt_poll(&dev, MT_MAC_STATUS, MT_MAC_STATUS_TX, 0, …), plus a line saying so when TX never goes idle, so a FAIL is attributable.

Process

• getenv. MT7612U_NO_AUTORECOVER is a field now, so the debt stops accumulating. I left MT7612U_DEV deferred rather than converting it: there is no public way to pass a selector — mt7612u_open() allocates the device and the struct is opaque — so removing the read would leave a multi-adapter consumer unable to choose an adapter at all, and the wrapper does not need it (it selects the device before mt7612u_open_handle()). Ending it properly wants a small public setter or an open-with-config, which felt like the wrong thing to add to a bug-fix PR. Say the word and it is a short follow-up.
• Number drift. One figure, one home — the mt7612u_phy_tick() comment, which now says so. phy.c (three sites), bringup.c and docs/mt7612u.md point at it. 4362/s is deleted rather than re-homed: nothing in the tree says which build or window produced it, so relabelling it would have been a guess.
• docs/mt7612u-usb-wedge.md. Provenance header dropped; register tables and the untested-status verdict kept.
• Qodo Small mods to build on android #1, Log libusb error code in rtw_read #2, Remove color code in logger #6 and Fix 40 MHz bandwidth mode #8 answered and resolved.

Two more, found reviewing the merged result

Worth calling out because neither was visible in any single change:

• Hoisting the AGC reads above the fast return also moved them out of the only mt_io_errors() bracket guarding them. FIELD_GET(MT_BBP_AGC_GAIN, ~0u) is 0x7f, and low_gain has just been reset — so one EP0 hiccup during a fast retune would have programmed a gain base of 127 on the next tick and gone deaf until the next full tune, silently. Checked now, keeping the previous snapshot.
• The mt_io_clear() above.

Verification on this head

Peer is a second MT7612U flooding ch149 — a different stimulus from §1's, so quoted separately rather than folded into that figure:

check	result
RX	4902–4913 fps, 5 runs, rx_err/rx_invalid/rx_dropped all 0, peer verified airing before and after each run
adopt	bringup adopt PASS; recovery observed on that path
drain	9 timeouts → one drain of 9 replies
retune	530 ms full / 50.6 ms fast
build	strict -Wall -Wextra -Wshadow -Wmissing-prototypes clean, make check green

No MT7612U-side claim here is inferred: each row is from a run on 2-1 this round.