usb: pipelined register writes for the bring-up; Jaguar3 stage timing

[gilankpam]

Jaguar3 InitWrite on an RTL8812EU is ~14k synchronous EP0 round trips and
essentially nothing else — no waiting on hardware, just USB control-transfer
latency. This measures that, then removes most of it.

Measuring it

InitTimer already bracketed init stages with a duration. It now also reports
the number of USB vendor control transfers each stage spent
(ITransport::ctrl_xfers, a per-transport-instance counter kept by
UsbTransport and handed to InitTimer), which is the unit the bring-up is
actually paid in. init.timing gains an xfers field where a counter is
attached — the Jaguar3 stages; docs/logging.md carries the schema.

Every Jaguar3 rtw_hal_init / InitWrite stage is bracketed.

Removing it

A control transfer costs 76–80 µs synchronous on an embedded host (ssc338q)
and ~27 µs pipelined 8-deep. EP0 completes URBs in submission order, so a
queue of pending writes followed by a read behaves exactly like the
synchronous sequence — the host just doesn't sit through a round trip per
write.

ITransport (src/Transport.h) gains write_batch_begin / write_batch_end /
flush_writes. Inside a batch, UsbTransport submits register writes as
async URBs; a read (submitted behind the queue and waited on its own
completion), a bulk transfer, or an explicit flush is what waits. Jaguar3
InitWrite runs its whole bring-up in one RAII scope, ended before the coex
thread starts. Every settle delay flushes first, µs ones included, on both
dies (delay_us/delay_ms on Halrf8822c and Halrf8822e, the table
delay markers, the efuse power-cut): the drain is free on an empty queue
and bounded by its depth otherwise.

All three methods default to no-ops, so PcieTransport and every
generation other than Jaguar3 are untouched.

Contract

Batches are single-threaded: open one only while no other thread touches the
transport, and close it before any worker starts. This is enforced by
convention, not by an assert — worth a reviewer's attention.

Failure paths

A drain that times out cancels what is still submitted and keeps pumping for
the cancellations rather than declaring the queue empty. Zeroing the in-flight
count by hand would let a late callback drive it negative (silently disabling
every later drain), return a slot to the free list twice, and leave the
destructor calling libusb_free_transfer on a transfer libusb still owns.
Slots that genuinely cannot be reaped — dead event loop, yanked device — are
retired for the session (the batch closes, so the rest of the session runs
synchronously) and leaked at teardown instead, which is the lesser evil
against handing libusb a dangling transfer. The completion callback writes
only to an AsyncPool shared between the transport and every slot, so a
leaked slot whose callback fires later through a still-pumped libusb
context touches the pool it keeps alive, never a freed transport.

Separately: the RF radio-table load is write-only

Bits [31:20] of the direct window are not storage. tests/j3_rf_window_readback.sh
measures it two ways per die, both through chipstate --init: the
post-bring-up histogram (512/512 words read 0 on 8812CU and 8812EU), and
the leg that decides — 16 window words per die poked with the high 12 bits
set (low 20 bits unchanged, then restored) all read back 0. Bits that
were storage would read back set; these do not, on either die, regardless
of which bring-up ran first. So the vendor's MASK20BITS
read-modify-write preserved nothing while paying a synchronous read per
entry, about half the RF-table stage, and the plain write is bit-identical.

Results and limits

Measured on one drone-side 8812EU (ssc338q host): warm InitWrite
1.30 → 0.65 s, cold 2.04 → ~0.7 s. On the x86 bench, alternating
master-vs-branch floods (tests/j3_tx_flood_ab.sh, 3 reps per tree, time
from exec to first TX submit):

DUT	master	this branch
RTL8812CU (0bda:c812), ch36	1133 / 1305 / 1264 ms	860 / 822 / 880 ms
RTL8812EU (0bda:a81a), ch36	896 / 1066 / 866 ms	687 / 622 / 647 ms

(Final head, with the µs-settle flushes in. An earlier round without them
read CU 846–874 / EU 633–1081 ms on the branch — the extra drains cost
nothing measurable.)

One unit of each die, one host — the transfer-count reduction is
deterministic, the wall-clock figure is not replicated across parts.

Init (RX-only) opens no batch yet and is unmeasured on a ground-station
card, so the RX bring-up path is unchanged by this PR and gets none of
the speedup.

Validation

• tests/regress.py 2x2, devourer→devourer cells (kernel cells read 0 on
  this rig: no vendor module is built for the receivers today):
  • 8812CU TX → CF-924AC (8822BU) RX: ch6 7400 hits / 8000 TX, ch36
    8100 / 8000.
  • 8812EU TX → T3U (8822BU) RX: ch36 8000 / 8000 (two runs; a first
    run read 2500/15 s and did not reproduce). ch6 reads 0 on this module
    with either tree — the documented 8822E 2.4 GHz TX kernel-parity cell
    (docs/8822e-quirks.md).
• The 8812EU logs ~10 bulk_send EP 5 FAIL rc=-7 and one failed coex
  BTC-window read per 15 s flood on 5 GHz. Same on master (12/12/16 vs
  6/8/13 bulk failures across the alternating reps of the final round;
  9/10/11 vs 10/18/12 in the earlier one), so it is the unit's known 5 GHz
  NAK behaviour, not this change.
• tests/tx_teardown_asan.sh (ASan build, max-duty + aggregation + gap-2000,
  3 reps each) on the 8812CU: 9/9 clean exits. 8812EU (ch36): 9/9 clean exits.
• cmake --build + ctest green (66/66).

Also folded in: tests/regress.py learns 0bda:b812 (the CF-924AC V2 the
bench notes recommend as ground station, which its DUT table did not list),
and chipstate --init followed by --peek/--poke runs the ops on the
configured chip after releasing the device object (its destructor joins the
coex thread and does not de-init the chip), with :4 for 32-bit-word reads
(the BB/RF windows answer 32-bit reads only). chipstate also discovers
0xb812 without --pid.

🤖 Generated with Claude Code

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

PR Summary by Qodo

Pipeline Jaguar3 USB bring-up writes and add transfer timing

✨ Enhancement 📝 Documentation 🕐 40+ Minutes

AI Description

• Pipeline Jaguar3 TX bring-up register writes over ordered asynchronous USB EP0 transfers.
• Add per-stage duration and control-transfer telemetry throughout Jaguar3 initialization.
• Remove redundant RF table reads and safely drain failed asynchronous transfers.

Diagram

sequenceDiagram
    participant Init as Jaguar3 Init
    participant Adapter as RtlAdapter
    participant USB as UsbTransport
    participant EP0 as USB EP0
    participant Count as Transfer Counter
    participant Timer as InitTimer
    participant Log as Event Log
    Init->>Adapter: Begin write batch
    Adapter->>USB: Enable pipeline
    loop Register writes
        Init->>Adapter: Write register
        Adapter->>USB: Queue write
        USB->>EP0: Submit async URB
        USB->>Count: Increment transfer
    end
    Init->>Adapter: Read or flush
    Adapter->>USB: Establish ordering barrier
    alt Read follows writes
        USB->>EP0: Submit ordered read
        EP0-->>USB: Complete queue and read
    else Explicit flush
        USB->>EP0: Wait for completions
        EP0-->>USB: Complete queued writes
    end
    USB-->>Init: Return result
    Init->>Adapter: End write batch
    Init->>Timer: Record stage
    Timer->>Count: Read transfer delta
    Timer->>Log: Emit ms and xfers

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Device-side multi-register command

• ➕ Could reduce both host wakeups and USB request count beyond an eight-deep pipeline.
• ➕ Would avoid managing multiple simultaneous libusb transfer objects.
• ➖ Requires firmware or vendor-protocol support that is not currently available.
• ➖ Creates substantially greater compatibility and hardware-validation risk.

2. Dedicated USB event thread

• ➕ Could continuously reap completions without initialization code pumping libusb events.
• ➕ Could support pipelining outside single-threaded bring-up scopes.
• ➖ Introduces synchronization with existing RX and TX event handling.
• ➖ Makes libusb teardown and device-removal ownership significantly harder.

3. Keep synchronous writes

• ➕ Preserves the simplest transfer lifetime and failure model.
• ➕ Requires no single-threaded batch contract.
• ➖ Retains roughly 14,000 serial EP0 round trips during Jaguar3 bring-up.
• ➖ Cannot deliver the measured initialization latency reduction.

Recommendation: The bounded, scoped libusb pipeline is the best available approach because it preserves register ordering without requiring firmware changes and leaves non-USB transports unchanged. Review should focus on slot ownership, cancellation callbacks, allocation failures, repeated batch termination, and enforcement of the single-threaded contract; hardware teardown testing remains important.

Files changed (13) +409 / -17

Enhancement (10) +384 / -15

InitTimer.hReport USB transfer deltas for initialization stages +22/-6

Report USB transfer deltas for initialization stages

• Snapshots the process-wide USB control-transfer counter and emits per-stage and total deltas alongside elapsed milliseconds.

src/InitTimer.h

RtlAdapter.hExpose register-write batching through the adapter +4/-0

Expose register-write batching through the adapter

• Adds adapter forwarding methods for beginning, ending, and explicitly flushing transport write batches.

src/RtlAdapter.h

RtlTransport.hDefine the transport write-batching contract +14/-0

Define the transport write-batching contract

• Adds default no-op batching and flushing hooks to 'IRtlTransport'. The documented contract preserves PCIe behavior while requiring exclusive, single-threaded transport access during a batch.

src/RtlTransport.h

UsbTransport.cppImplement bounded asynchronous USB register pipelines +193/-0

Implement bounded asynchronous USB register pipelines

• Adds an eight-slot libusb control-transfer pool with ordered reads, explicit drains, cancellation recovery, error accounting, and safe retirement of unreaped transfers. Bulk and byte transfers flush queued writes before proceeding.

src/UsbTransport.cpp

UsbTransport.hAdd USB pipeline state and count control transfers +63/-0

Add USB pipeline state and count control transfers

• Routes batched register reads and writes through asynchronous helpers while counting every vendor control transfer. Declares slot ownership, queue depth, completion state, and abandoned-transfer safeguards.

src/UsbTransport.h

UsbXferCount.hAdd a process-wide USB control-transfer counter +14/-0

Add a process-wide USB control-transfer counter

• Introduces a header-only relaxed atomic counter shared by 'UsbTransport' and 'InitTimer'.

src/UsbXferCount.h

HalJaguar3.cppInstrument Jaguar3 HAL stages and optimize RF table loading +40/-8

Instrument Jaguar3 HAL stages and optimize RF table loading

• Adds timing checkpoints around HAL initialization and table phases, flushing queued writes before millisecond settle delays. Replaces redundant masked RF-window read-modify-writes with direct 20-bit writes.

src/jaguar3/HalJaguar3.cpp

HalJaguar3.hAllow table loading to report timing checkpoints +1/-1

Allow table loading to report timing checkpoints

• Updates the table-loading helper to optionally receive an 'InitTimer' for detailed phase instrumentation.

src/jaguar3/HalJaguar3.h

Halrf8822e.cppDrain queued writes before calibration delays +1/-0

Drain queued writes before calibration delays

• Flushes pipelined register writes before millisecond RF calibration sleeps so settle periods begin after hardware receives the writes.

src/jaguar3/Halrf8822e.cpp

RtlJaguar3Device.cppBatch and time the Jaguar3 TX bring-up path +32/-0

Batch and time the Jaguar3 TX bring-up path

• Wraps 'InitWrite' in an RAII write batch, adds checkpoints across major stages, and closes the batch before starting the coexistence thread. The RX-only 'Init' path intentionally remains synchronous.

src/jaguar3/RtlJaguar3Device.cpp

Refactor (1) +1 / -1

Halrf8822e.hMake millisecond delays transport-aware +1/-1

Make millisecond delays transport-aware

• Converts 'delay_ms' from static to instance-bound so it can flush the device transport before sleeping.

src/jaguar3/Halrf8822e.h

Documentation (2) +24 / -1

logging.mdDocument transfer counts in initialization timing events +1/-1

Document transfer counts in initialization timing events

• Extends the documented 'init.timing' schema with the 'xfers' field and clarifies that PCIe reports zero.

docs/logging.md

CLAUDE.mdDocument Jaguar3 bring-up performance and batching constraints +23/-0

Document Jaguar3 bring-up performance and batching constraints

• Records measured initialization costs and speedups, the single-threaded pipeline contract, required delay flushes, RX-path limitation, and write-only RF table rationale.

src/jaguar3/CLAUDE.md

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

Code Review by Qodo

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

1. USB transfers outlive their context 📘 Rule violation ☼ Reliability

Description

flush_writes() abandons unreaped transfers while they remain submitted, and
UsbTransport::~UsbTransport() deliberately leaks their slots instead of establishing quiescence.
When cancellation cannot be reaped within the bounded deadline, DeviceSession::close()
subsequently releases the interface, closes the handle, and exits the context while libusb still
owns those transfers.

Code

src/UsbTransport.cpp[R646-647]

+       * submitted and off the free list; the destructor leaks them. The
+       * batch is closed here too: with those slots never returning, the

Evidence

Rule 10 requires every asynchronous transfer to be quiesced while the claimed handle and libusb
context still exist. The new abandonment path explicitly leaves transfers submitted, while the
session cleanup shown here closes the handle and exits that context immediately after transport
destruction.

CLAUDE.md: Preserve Correct libusb Ownership and Teardown Order: CLAUDE.md: Preserve Correct libusb Ownership and Teardown Order: CLAUDE.md: Preserve Correct libusb Ownership and Teardown Order: CLAUDE.md: Preserve Correct libusb Ownership and Teardown Order: CLAUDE.md: Preserve Correct libusb Ownership and Teardown Order: CLAUDE.md: Preserve Correct libusb Ownership and Teardown Order: CLAUDE.md: Preserve Correct libusb Ownership and Teardown Order: CLAUDE.md: Preserve Correct libusb Ownership and Teardown Order
src/UsbTransport.cpp[338-383]
src/UsbTransport.cpp[609-667]
examples/common/DeviceSession.h[72-85]

Agent prompt

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

## Issue description
Unreaped pipelined control transfers are deliberately left submitted, allowing them to survive release of their claimed handle and libusb context.
## Fix Focus Areas
- src/UsbTransport.cpp[338-383]
- src/UsbTransport.cpp[609-667]
- examples/common/DeviceSession.h[72-85]
## Recommended Fix
Refactor asynchronous-transfer cleanup so the transport destructor cannot return while libusb still owns a transfer. Keep the handle and context alive while cancellation completions are pumped, and release each transfer only after its callback has established that it is no longer active; do not use leaking submitted slots as the terminal teardown state.

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

---

2. Live receive races reinitialization ✓ Resolved 🐞 Bug ≡ Correctness

Description

InitWrite() opens WriteBatchScope after joining only the coexistence thread, without rejecting
or quiescing an active receive loop and its PHY-management worker. When a live device is
reinitialized while _rx_loop_active is true, that worker can issue register operations through the
same transport and join the global batch while bulk receive continues against partially reprogrammed
hardware.

Code

src/jaguar3/RtlJaguar3Device.cpp[R787-791]

+  InitTimer timer(_logger, "j3init",
+                  _device.is_usb() ? InitTimer::XferCounter{[this] { return _device.ctrl_xfers(); }}
+                                   : InitTimer::XferCounter{},
+                  [this] { _device.flush_writes(); });
+  WriteBatchScope batch(_device);

Evidence

The changed method explicitly supports a second bring-up and joins _coex_thread, but then opens
the transport-wide batch without checking _rx_loop_active. StartRxLoop() marks that state active
and runs bulk-IN processing, while its local PHY-management thread periodically takes _reg_mu and
performs register I/O; InitWrite() neither takes that mutex nor owns that thread, and the active
flag is currently cleared before the worker is joined. The transport contract requires no other
thread to touch the transport while a batch is open.

src/jaguar3/RtlJaguar3Device.cpp[763-791]
src/jaguar3/RtlJaguar3Device.cpp[180-188]
src/jaguar3/RtlJaguar3Device.cpp[256-280]
src/jaguar3/RtlJaguar3Device.cpp[421-429]
src/Transport.h[79-85]

Agent prompt

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

## Issue description
`InitWrite()` opens a transport-wide write batch after stopping only the coexistence thread. An active receive loop and its PHY-management worker can still use the transport, violating the batch's single-threaded contract and mixing their register operations into reinitialization.
## Fix Focus Areas
- src/jaguar3/RtlJaguar3Device.cpp[763-791]
- src/jaguar3/RtlJaguar3Device.cpp[180-188]
- src/jaguar3/RtlJaguar3Device.cpp[256-280]
- src/jaguar3/RtlJaguar3Device.cpp[421-429]
## Recommended Fix
Before constructing `WriteBatchScope`, reject reinitialization while `_rx_loop_active` is set or synchronously stop the receive loop and wait until both bulk-IN processing and its PHY-management worker have exited. Make `_rx_loop_active` become false only after the worker has joined, so observing false guarantees that no receive-owned transport access remains.

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

---

3. Repeated setup can hang indefinitely ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new re-initialization path writes _coex_stop while coex_runtime_loop() concurrently reads
it, but _coex_stop is a volatile bool rather than an atomic or mutex-protected value. A second
InitWrite() can therefore race while requesting the old worker to stop and block in join()
without a valid cross-thread stop signal.

Code

src/jaguar3/RtlJaguar3Device.cpp[R766-770]

+  if (_coex_thread.joinable()) {
+    _coex_stop = true;
+    _coex_thread.join();
+    _coex_stop = false;
+  }

Evidence

The PR adds a second-bring-up shutdown path that stores the stop flag before joining the existing
worker. The worker loop reads the same non-atomic field, and the field declaration supplies no
synchronization, making this newly exercised stop-and-join path undefined behavior.

src/jaguar3/RtlJaguar3Device.cpp[760-770]
src/jaguar3/RtlJaguar3Device.cpp[456-460]
src/jaguar3/RtlJaguar3Device.h[382-385]

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 new repeated-`InitWrite` path stops and joins an existing coex worker by concurrently writing a non-atomic `volatile bool` that the worker reads. This is a C++ data race and can prevent reliable worker shutdown.
## Fix Focus Areas
- src/jaguar3/RtlJaguar3Device.cpp[766-770]
- src/jaguar3/RtlJaguar3Device.cpp[460-460]
- src/jaguar3/RtlJaguar3Device.h[384-385]
## Recommended Fix
Replace `_coex_stop` with `std::atomic_bool` and use explicit acquire/release loads and stores at every worker-loop and shutdown/restart access. Alternatively, guard every access with the same mutex and condition mechanism; do not use `volatile` for inter-thread synchronization.

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

---

View action required (2)

4. Failed restarts still look ready ✓ Resolved 🐞 Bug ≡ Correctness

Description

BroughtUpGuard is constructed only after InitWrite has stopped the previous coex thread and
performed HAL initialization, channel setup, calibration, and initial power programming, so
exceptions in those operations leave the previous _brought_up value unchanged. On a second
bring-up, a failed USB read before the guard exists therefore leaves readiness-gated runtime APIs
enabled even though initialization failed and the prior worker is no longer running.

Code

src/jaguar3/RtlJaguar3Device.cpp[R862-863]

+  } brought_up_guard{_brought_up};
_brought_up = true;

Evidence

The new restart path joins the existing coex thread before entering several register-heavy
operations, while the rollback guard is not installed until much later. USB register reads can throw
on transfer failure, and multiple runtime APIs explicitly trust _brought_up to decide whether
hardware access is valid.

src/jaguar3/RtlJaguar3Device.cpp[763-775]
src/jaguar3/RtlJaguar3Device.cpp[850-863]
src/UsbTransport.h[289-297]
src/jaguar3/RtlJaguar3Device.cpp[1295-1319]

Agent prompt

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

## Issue description
A failed second `InitWrite` can retain `_brought_up == true` because the rollback guard is installed only after several throwing initialization stages and after the previous coex thread is joined.
## Fix Focus Areas
- src/jaguar3/RtlJaguar3Device.cpp[763-863]
## Recommended Fix
Move the readiness rollback guard to immediately after the previous coex thread is stopped, before any bring-up operation can throw. Keep readiness provisional during initialization and commit the guard only after the batch closes successfully, preserving the intended CW-tone behavior.

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

---

5. A shared USB context can corrupt memory ✓ Resolved 🐞 Bug ☼ Reliability

Description

async_write_cb clears both w->inflight and w->cb_busy before acquiring the pool mutex and
publishing w to AsyncPool::free, while UsbTransport::~UsbTransport frees any slot it observes
with those flags clear. When another adapter pumps the shared libusb context and runs that callback
as this transport is destroyed, destruction can free the slot in that gap and the callback
subsequently inserts a dangling pointer that async_take_slot reuses.

Code

src/UsbTransport.cpp[R487-490]

+  w->cb_busy = false;
+  {
+    std::lock_guard<std::mutex> lk(pool->mu);
+    pool->free.push_back(w); /* pointer value only — no access through it */

Evidence

The callback marks a slot non-inflight and non-busy before its mutex-protected free-list push. The
destructor uses exactly those two flags to decide that it can free the slot, and a later batch
obtains slots from that same free list; the header explicitly permits the callback to run on another
adapter's event-pump thread.

src/UsbTransport.cpp[458-491]
src/UsbTransport.cpp[338-377]
src/UsbTransport.cpp[494-525]
src/UsbTransport.h[134-141]

Agent prompt

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

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
The completion callback can expose a slot as safe for destruction before its final handoff to the shared free list is complete. A concurrent `UsbTransport` destructor can free that slot, after which the callback publishes a dangling pointer for a later batch to reuse.
Fix Focus Areas
- src/UsbTransport.cpp[458-491]
- src/UsbTransport.cpp[338-377]
Recommended Fix
Make the callback's final ownership handoff and the destructor's decision to free a completed slot mutually synchronized using the pool mutex (or an equivalent lifetime protocol). The destructor must not free a slot until it is impossible for an active callback to publish it, and it must remove or invalidate completed slots from the free list before deleting them.

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

---

6. Concurrent adapters can stall writes 🐞 Bug ➹ Performance

Description

async_write_cb() increments completed before publishing the completed slot to the protected free
list. When another adapter pumps the shared USB context, async_take_slot() can snapshot the
advanced counter while the list is still empty and then wait for its full two-second deadline even
though the callback subsequently returns the slot without another counter change.

Code

src/UsbTransport.cpp[R497-498]

+  pool->inflight--;
+  pool->completed++;

Evidence

The callback advances completed at src/UsbTransport.cpp[495-508] before acquiring the mutex and
pushing the slot onto free. The waiter at src/UsbTransport.cpp[525-538] snapshots that counter,
checks the list, and waits exclusively for another counter change, while
src/UsbTransport.cpp[578-591] gives that wait a two-second deadline.

src/UsbTransport.cpp[495-508]
src/UsbTransport.cpp[525-538]
src/UsbTransport.cpp[578-591]

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 completion counter is advanced before the completed transfer slot is returned to the free list. A waiter can therefore observe the new counter with no available slot and sleep until its two-second timeout because publishing the slot does not signal further progress.
## Fix Focus Areas
- src/UsbTransport.cpp[497-508]
- src/UsbTransport.cpp[512-538]
## Recommended Fix
Publish the slot to `AsyncPool::free` before advancing `completed`, using ordering that ensures a waiter observing the counter also observes the available slot. Preserve the callback's slot-lifetime synchronization with the pool mutex.

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

---

7. Recovered batches inherit old failures 🐞 Bug ≡ Correctness

Description

write_batch_begin() clears write_errors before advancing generation, while a late callback
compares its slot generation and can increment that same counter without synchronization with the
reset. After a drain retires an unreaped write, a callback running between those two operations
charges the prior cancellation to the next synchronous recovery batch, so write_batch_end()
rejects an otherwise successful retry.

Code

src/UsbTransport.cpp[R416-417]

+  _aw->write_errors = 0;
+  _aw->generation++;

Evidence

Abandonment pre-counts each unreaped write at src/UsbTransport.cpp[652-660], but the callback at
src/UsbTransport.cpp[491-494] can still count it whenever its generation matches. A subsequent begin
resets the shared error count and only then advances the generation at
src/UsbTransport.cpp[408-421], leaving a callback interleaving that writes the stale failure into
the new verdict consumed by src/UsbTransport.cpp[450-467].

src/UsbTransport.cpp[408-421]
src/UsbTransport.cpp[491-494]
src/UsbTransport.cpp[450-467]
src/UsbTransport.cpp[652-660]

Agent prompt

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

## Issue description
Callbacks from retired transfers can update `write_errors` while a later batch resets and advances the shared generation. This can make a successful recovery batch inherit a failure from an earlier abandoned batch.
## Fix Focus Areas
- src/UsbTransport.cpp[416-417]
- src/UsbTransport.cpp[491-494]
- src/UsbTransport.cpp[652-660]
## Recommended Fix
Make retiring a slot permanently mark its write failure as already accounted, and have the completion callback suppress further accounting for that slot. Ensure starting a new generation and resetting its verdict cannot race with callbacks that already selected the previous generation.

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

---

8. Teardown can pump a freed USB context 🐞 Bug ☼ Reliability

Description

UsbTransport::~UsbTransport() unconditionally calls flush_writes(), which reaches pump_once()
and passes _ctx to libusb_handle_events_timeout_completed(). When the last transport reference
outlives libusb_exit() while an async write remains in flight, destruction dereferences the
invalid context instead of safely retiring the transfer.

Code

src/UsbTransport.cpp[R339-352]

+  flush_writes();
+  /* Last chance while the handle and context are still valid: a queue that
+   * was retired earlier gets one more cancel + bounded reap here, in case
+   * the event loop has come back. What is still submitted after this is
+   * leaked deliberately — the alternative is freeing a transfer libusb
+   * owns, and the caller's libusb_exit will report it rather than crash. */
+  if (_aw->inflight > 0) {
+    for (auto *w : _aw_all)
+      if (w->inflight)
+        libusb_cancel_transfer(w->t);
+    const auto deadline =
+        std::chrono::steady_clock::now() + std::chrono::seconds(2);
+    while (_aw->inflight > 0 && std::chrono::steady_clock::now() < deadline)
+      if (!pump_once(100))

Evidence

The newly added destructor drain runs before its own later diagnostic acknowledges that _ctx may
already be freed. Draining waits through async_wait_progress(), whose event pump directly uses
_ctx, so an outstanding new pipelined transfer makes the invalid lifetime observable.

src/UsbTransport.cpp[339-352]
src/UsbTransport.cpp[394-403]
src/UsbTransport.cpp[578-606]
examples/common/DeviceSession.h[7-20]

Agent prompt

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

## Issue description
`UsbTransport` can outlive the libusb context, but its destructor now drains pipelined writes by pumping `_ctx` unconditionally. If libusb has already been exited, this invokes libusb on freed context state.
## Fix Focus Areas
- src/UsbTransport.cpp[339-352]
- src/UsbTransport.cpp[594-606]
## Recommended Fix
Make context shutdown an explicit transport lifecycle state that is set before `libusb_exit()`, and skip cancellation/event pumping when that state is set. Retire/leak still-submitted async-write slots in that case; retain the normal drain path only while the context is known live. Ensure all context-owning callers establish this ordering.

ⓘ 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 enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[josephnef]

The mechanism is sound and the reasoning behind it is the strongest part of the PR — never zeroing _aw_inflight by hand, retiring unreapable slots rather than handing libusb a dangling transfer, and WriteBatchScope's destructor closing the batch on a throw are all correct, and those comments earn their length. EP0 submission ordering does make the read-behind-writes trick safe, and the tx_async/tx_sync/write_bytes flushes cover the bulk paths.

I'd still hold it. Two of the four substantive issues are cross-variant validation gaps rather than logic errors, which on a bring-up-critical path is the expensive kind: every number in this PR is one 8812EU, and two of the changes ship to 8822C/8812CU as well.

1. The 8822C settle delays don't flush (inline on Halrf8822e.cpp). Halrf8822c::delay_ms is still static and untouched, while the batch scope covers dac_calibrate() and run_iqk() on both variants.

2. The write-only RF-table load is variant-agnostic (inline on HalJaguar3.cpp). The argument is airtight given readback 0 — an RMW that reads 0 writes exactly data, so the two are bit-identical — so the entire risk is whether 8822C's direct window behaves the same, and that's unmeasured.

3. _aw_abandoned is set but never acted on mid-batch (inline on UsbTransport.cpp).

4. libusb_alloc_transfer(0) isn't null-checked (inline).

The rebase is not mechanical. The PR is CONFLICTING. Post-#415 there is no IRtlTransport — RtlAdapter forwards to devourer::ITransport (src/Transport.h), so the three virtuals have to land there, and the IRtlTransport::write_batch_begin doc references in RtlAdapter.h, HalJaguar3.cpp and src/jaguar3/CLAUDE.md need retargeting.

Validation, by the repo's own bar. Neither the pipelining nor the RF-table rewrite has an on-air or regress.py run, and #417 changes how the RF radio tables are programmed on both dies. I'd want a 2x2 on an 8822EU and an 8822CU, plus tests/tx_teardown_asan.sh over the new drain/cancel paths, before this merges — the description already flags that those haven't been run, which is the right call, but it's also the reason to hold.

Everything else is inline and minor.

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit ecce836

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit 47e741c

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit cc74109

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit 796047b

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit bb8bbe3

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit 77b777c

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit 04c0cd2

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit 6275f80

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit 814cb6c

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit 2375a45

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit dd47acb

[josephnef]

/review

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

Code review by qodo was updated up to the latest commit 7ff9657

Superseded: every finding is addressed on the current head (rebased, fixed, validated on both Jaguar3 dies); merging under the skip-qodo-gate escape hatch by maintainer decision.