From 04b216fdd826ae881403ad3407e1ab6b0cbd14ec Mon Sep 17 00:00:00 2001 From: Gilang Date: Wed, 9 Sep 2026 00:38:17 +0700 Subject: [PATCH 01/29] usb: pipelined register writes for the bring-up; Jaguar3 stage timing InitWrite on the RTL8812EU is ~14k synchronous EP0 round trips and nothing else. InitTimer now brackets every Jaguar3 rtw_hal_init / InitWrite stage and reports the control-transfer count per stage (UsbXferCount.h), which is the unit the bring-up is actually paid in. init.timing gains an `xfers` field; docs/logging.md carries the schema. A transfer costs 76-80 us synchronous on an embedded host (ssc338q) and ~27 us pipelined 8-deep. EP0 completes URBs in submission order, so IRtlTransport::write_batch_begin/end lets UsbTransport queue writes as async URBs and wait only on reads (submitted behind the queue), bulk transfers and flush_writes. Jaguar3 InitWrite runs its whole bring-up in one batch (RAII scope, closed before the coex thread starts); the ms-scale settle delays flush first. The three methods default to no-ops, so PCIe is unaffected. Batches are single-threaded by contract: open one only while no other thread touches the transport, and close it before any worker starts. A drain that times out cancels what is still submitted and keeps pumping for the cancellations rather than declaring the queue empty: an in-flight count zeroed by hand goes negative on the late callback, which silently disables every later drain, returns a slot to the free list twice, and leaves the destructor freeing a transfer libusb still owns. Slots that genuinely cannot be reaped (dead event loop, yanked device) are retired for the session and leaked at teardown instead. Separately, the RF radio-table load is write-only: bits [31:20] of the direct window read back 0 for all 1540 entries, cold and warm, so the vendor's MASK20BITS read-modify-write preserved nothing while paying a synchronous read per entry -- about half the RF-table stage. Measured on one drone-side unit: warm InitWrite 1.30 -> 0.65 s, cold 2.04 -> ~0.7 s. 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 commit. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JiAS32956Z3cmTbnmcXYkT --- docs/logging.md | 2 +- src/InitTimer.h | 28 ++++- src/RtlAdapter.h | 4 + src/Transport.h | 14 +++ src/UsbTransport.cpp | 193 +++++++++++++++++++++++++++++++ src/UsbTransport.h | 63 ++++++++++ src/UsbXferCount.h | 14 +++ src/jaguar3/CLAUDE.md | 23 ++++ src/jaguar3/HalJaguar3.cpp | 48 ++++++-- src/jaguar3/HalJaguar3.h | 2 +- src/jaguar3/Halrf8822e.cpp | 1 + src/jaguar3/Halrf8822e.h | 2 +- src/jaguar3/RtlJaguar3Device.cpp | 32 +++++ 13 files changed, 409 insertions(+), 17 deletions(-) create mode 100644 src/UsbXferCount.h diff --git a/docs/logging.md b/docs/logging.md index 327d135f..3e8e591b 100644 --- a/docs/logging.md +++ b/docs/logging.md @@ -76,7 +76,7 @@ Emitters: L = library, RX/TX/... = demo. Optional fields in [brackets]; ### Init / infrastructure | ev | emitter | fields | |---|---|---| -| `init.timing` | L (`src/InitTimer.h`) + demos | stage ("scope.stage", e.g. "demo.first_rx_frame", "txdemo.first_tx_submit"), ms | +| `init.timing` | L (`src/InitTimer.h`) + demos | stage ("scope.stage", e.g. "demo.first_rx_frame", "txdemo.first_tx_submit"), ms, xfers (USB vendor control transfers the stage spent; 0 on PCIe) | | `adapter.caps` | RX, TX, doctor, txpower (`examples/common/caps_event.h`) | supported, chip, names, chip_id "0x..", gen, variant, transport, tx_chains, rx_chains, n_ss, stbc, ldpc, sgi, bw_max, bw[] (MHz), txpwr_max, txpwr_step_qdb, txpwr_step_measured, txpwr_min_qdb, txpwr_max_qdb, txpwr_rate_diffs, txpwr_rate_diffs_hw, txpwr_rate_diffs_measured, tune_2g4[]\|null, tune_5g[]\|null, char_2g4[]\|null, char_5g[]\|null, ldpc_rx_ht, ldpc_rx_vht, ldpc_rx_flag, vht_2g4, per_pkt_txpwr, per_pkt_txpwr_steps, per_pkt_txpwr_step_qdb, per_pkt_txpwr_min_qdb, per_pkt_txpwr_max_qdb, per_pkt_txpwr_measured, narrowband, fastretune, ack_responder, tx_retry_limit, he_er_su, per_chain_rssi, hw_rx_tsf, hw_beacon_txtsf, tsf_write, xtal_cap_max, xtal_cap_default | | `debug.wreg` | L (`DEVOURER_LOG_WRITES`) | addr "0x0nnn", width, val "0x…" | | `hop.prof` | L (`DEVOURER_HOP_PROF`) | gen, ch, `_us`…, total_us | diff --git a/src/InitTimer.h b/src/InitTimer.h index 133c36b8..504448ff 100644 --- a/src/InitTimer.h +++ b/src/InitTimer.h @@ -6,10 +6,14 @@ #include #include "logger.h" +#include "UsbXferCount.h" /* Stage timer for init-path profiling. Emits one event per checkpoint: * - * {"ev":"init.timing","stage":".","ms":N} + * {"ev":"init.timing","stage":".","ms":N,"xfers":K} + * + * `xfers` is the number of USB control transfers (register reads/writes) + * the stage spent — see UsbXferCount.h; 0 on the PCIe transport. * * `stage()` reports time since the previous checkpoint (or construction); * `total()` reports time since construction. Always-on: a handful of events @@ -21,23 +25,33 @@ class InitTimer { public: InitTimer(Logger_t logger, const char *scope) : _logger{std::move(logger)}, _scope{scope}, _start{clock::now()}, - _last{_start} {} + _last{_start}, _x_start{xfers()}, _x_last{_x_start} {} void stage(const char *name) { const auto now = clock::now(); - emit(name, ms(_last, now)); + const auto x = xfers(); + emit(name, ms(_last, now), static_cast(x - _x_last)); _last = now; + _x_last = x; } - void total() { emit("total", ms(_start, clock::now())); } + void total() { + emit("total", ms(_start, clock::now()), + static_cast(xfers() - _x_start)); + } private: - void emit(const char *name, long long millis) { + static uint64_t xfers() { + return devourer::usb_ctrl_xfers().load(std::memory_order_relaxed); + } + + void emit(const char *name, long long millis, long long nx) { char stage[96]; std::snprintf(stage, sizeof(stage), "%s.%s", _scope, name); devourer::Ev(_logger->events(), "init.timing") .f("stage", stage) - .f("ms", millis); + .f("ms", millis) + .f("xfers", nx); } static long long ms(clock::time_point from, clock::time_point to) { @@ -49,6 +63,8 @@ class InitTimer { const char *_scope; clock::time_point _start; clock::time_point _last; + uint64_t _x_start; + uint64_t _x_last; }; #endif /* INIT_TIMER_H */ diff --git a/src/RtlAdapter.h b/src/RtlAdapter.h index 74efa7f3..a5a8a31f 100644 --- a/src/RtlAdapter.h +++ b/src/RtlAdapter.h @@ -88,6 +88,10 @@ class RtlAdapter { /* Pre-power-on HCI programming (rtw88 rtw_hci_setup slot): PCIe TRX ring * registers; no-op on USB. Call per bring-up attempt, before power-on. */ void hci_setup() { _transport->hci_setup(); } + /* Pipelined register writes — see IRtlTransport::write_batch_begin. */ + void write_batch_begin() { _transport->write_batch_begin(); } + void write_batch_end() { _transport->write_batch_end(); } + void flush_writes() { _transport->flush_writes(); } /* Kernel-style async RX: keep n_urbs concurrent bulk-IN transfers in flight * (USB) or reap the RX buffer-descriptor ring (PCIe), invoking diff --git a/src/Transport.h b/src/Transport.h index 82c63154..707efc3f 100644 --- a/src/Transport.h +++ b/src/Transport.h @@ -68,6 +68,20 @@ class ITransport { return read32(static_cast(addr)); } + /* ---- pipelined register writes ---- + * Inside a write batch, register writes are submitted as asynchronous + * in-order transfers and only a read (or a bulk transfer, or flush_writes) + * waits for them. Bring-up is ~14k synchronous EP0 round trips at ~80 us + * each on an embedded host; pipelined, a write costs ~27 us (measured, + * ssc338q + RTL8812EU, depth >= 8). Correctness rests on EP0 completing + * URBs in submission order, so a read that follows a write still sees it. + * Single-threaded by contract: open a batch only while no other thread + * touches the transport (the Jaguar3 InitWrite/Init bring-up), and close + * it before any worker thread starts. Defaults are no-ops (PCIe). */ + virtual void write_batch_begin() {} + virtual void write_batch_end() {} + virtual void flush_writes() {} + /* ---- frame plane ---- */ /* Fire-and-forget data TX (the send_packet hot path). `ep` is the USB * bulk-OUT endpoint choice; the PCIe transport ignores it (the ring is diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 4cf2aece..42865e44 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -336,6 +336,23 @@ UsbTransport::UsbTransport(libusb_device_handle *dev_handle, Logger_t logger, } UsbTransport::~UsbTransport() { + flush_writes(); + int leaked = 0; + for (auto *w : _aw_all) { + /* libusb forbids freeing an active transfer, and its callback still + * writes through `w`. If the drain above could not reap it (dead event + * loop / yanked device), leaking the slot is the lesser evil: a callback + * that somehow fires later touches leaked memory, whereas freeing here + * hands libusb a dangling transfer it is still holding. */ + if (w->inflight) { + ++leaked; + continue; + } + libusb_free_transfer(w->t); + delete w; + } + if (leaked) + _logger->error("USB: leaked {} unreaped pipelined transfer slot(s)", leaked); /* Backstop only. The device's Stop()/destructor quiesces TX while every * owner is alive, which is the path that makes teardown safe; reaching the * transport destructor with transfers still in flight means the caller tore @@ -352,7 +369,181 @@ UsbTransport::~UsbTransport() { quiesce_tx(); } +/* ---- pipelined register writes ------------------------------------------ + * Submission order == completion order on EP0, so a pending queue of writes + * followed by a read behaves exactly like the synchronous sequence; the win + * is that the host does not sit through a full URB round trip per write. */ +void UsbTransport::write_batch_begin() { + if (_batch) + return; + /* A session that already failed to reap its transfers has a short pool and + * a suspect event loop; stay synchronous rather than pipeline into it. */ + if (_aw_abandoned) + return; + if (_aw_all.empty()) { + for (int i = 0; i < kAsyncWriteDepth; ++i) { + auto *w = new AsyncWrite{}; + w->t = libusb_alloc_transfer(0); + w->self = this; + _aw_all.push_back(w); + _aw_free.push_back(w); + } + } + _aw_errors = 0; + _batch = true; +} + +void UsbTransport::write_batch_end() { + flush_writes(); + if (_batch && _aw_errors) + _logger->error("USB: {} pipelined register write(s) failed in this batch", + _aw_errors); + _batch = false; +} + +void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { + auto *w = static_cast(t->user_data); + UsbTransport *self = w->self; + self->_aw_inflight--; + self->_aw_completed++; + w->inflight = false; + w->done = true; + w->status = t->status; + w->actual = t->actual_length; + if (t->status != LIBUSB_TRANSFER_COMPLETED || + t->actual_length != t->length - LIBUSB_CONTROL_SETUP_SIZE) + self->_aw_errors++; + /* The slot goes back on the free list here; a reader that is waiting on + * this very slot copies its data out before it submits anything else + * (single-threaded by contract), so the buffer is still intact. */ + self->_aw_free.push_back(w); +} + +UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { + while (_aw_free.empty()) { + if (!async_wait_progress()) { + flush_writes(); /* recovers the pool on a stuck queue */ + if (_aw_free.empty()) + return nullptr; + } + } + AsyncWrite *w = _aw_free.back(); + _aw_free.pop_back(); + w->done = false; + w->inflight = false; + w->status = -1; + w->actual = 0; + return w; +} + +bool UsbTransport::async_read(uint16_t wvalue, uint16_t windex, void *data, + size_t n) { + if (n > kAsyncMaxPayload) + return false; + AsyncWrite *w = async_take_slot(); + if (!w) + return false; + libusb_fill_control_setup(w->buf, REALTEK_USB_VENQT_READ, 5, wvalue, windex, + static_cast(n)); + libusb_fill_control_transfer(w->t, _dev_handle, w->buf, async_write_cb, w, + USB_TIMEOUT); + const int rc = libusb_submit_transfer(w->t); + if (rc != 0) { + _aw_free.push_back(w); + _aw_errors++; + _logger->error("USB: pipelined read submit failed ({})", rc); + return false; + } + w->inflight = true; + _aw_inflight++; + while (!w->done) { + if (!async_wait_progress()) { + flush_writes(); /* cancels + recovers; w->done is set by the cancel */ + break; + } + } + if (!w->done || w->status != LIBUSB_TRANSFER_COMPLETED || + w->actual != static_cast(n)) + return false; + std::memcpy(data, w->buf + LIBUSB_CONTROL_SETUP_SIZE, n); + return true; +} + +bool UsbTransport::async_wait_progress() { + const uint64_t before = _aw_completed; + for (int turns = 0; turns < 8 && _aw_completed == before; ++turns) { + struct timeval tv {0, 250 * 1000}; + const int rc = libusb_handle_events_timeout_completed(_ctx, &tv, nullptr); + if (rc < 0) { + _logger->error("USB: event loop error {} while draining pipelined writes", + rc); + return false; + } + } + return _aw_completed != before; +} + +void UsbTransport::flush_writes() { + while (_aw_inflight > 0) { + if (async_wait_progress()) + continue; + /* Stuck queue (~2 s without a completion): cancel what is still submitted + * so the caller's next synchronous transfer is not queued behind it. */ + _logger->error("USB: pipelined write drain timed out ({} in flight)", + _aw_inflight); + for (auto *w : _aw_all) + if (w->inflight) + libusb_cancel_transfer(w->t); + /* A cancellation still completes through the callback, which is what + * clears `inflight` and returns the slot. Keep pumping for it: the count + * must never be zeroed by hand, or a late callback decrements it below + * zero (silently disabling every later drain), pushes its slot onto the + * free list a second time, and races the destructor's free. */ + for (int i = 0; i < kFlushCancelTurns && _aw_inflight > 0; ++i) + async_wait_progress(); + if (_aw_inflight > 0) { + /* The event loop itself is gone (a yanked device reports the error + * immediately, so the turns above cost nothing). Leave the slots + * submitted and off the free list; the destructor leaks them. */ + _aw_errors += _aw_inflight; + _aw_abandoned = true; + _logger->error("USB: {} pipelined transfer(s) could not be reaped; " + "their slots are retired for this session", + _aw_inflight); + } + return; + } +} + +bool UsbTransport::async_write(uint16_t wvalue, uint16_t windex, + const void *data, size_t n) { + if (n > kAsyncMaxPayload) + return false; + AsyncWrite *w = async_take_slot(); + if (!w) + return false; + libusb_fill_control_setup(w->buf, REALTEK_USB_VENQT_WRITE, 5, wvalue, windex, + static_cast(n)); + std::memcpy(w->buf + LIBUSB_CONTROL_SETUP_SIZE, data, n); + libusb_fill_control_transfer(w->t, _dev_handle, w->buf, async_write_cb, w, + USB_TIMEOUT); + const int rc = libusb_submit_transfer(w->t); + if (rc != 0) { + _aw_free.push_back(w); + _aw_errors++; + _logger->error("USB: pipelined write submit failed ({})", rc); + return false; + } + w->inflight = true; + _aw_inflight++; + return true; +} + bool UsbTransport::write_bytes(uint16_t reg_num, const uint8_t *ptr, size_t n) { + /* A vendor control transfer like any other -- counted so an InitTimer stage + * that downloads firmware this way reports what it actually spent. */ + usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + flush_writes(); return libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_WRITE, 5, reg_num, 0, const_cast(ptr), n, USB_TIMEOUT) == static_cast(n); @@ -854,6 +1045,7 @@ void UsbTransport::quiesce_tx() { bool UsbTransport::tx_async(uint8_t tx_ep, uint8_t *packet, size_t length, unsigned timeout_ms) { + flush_writes(); /* Reap completed async-TX transfers before submitting the next one — in the * caller's own thread, so there is no background pump to race libusb * teardown. A non-blocking handle_events (timeout 0) processes every ready @@ -1001,6 +1193,7 @@ bool UsbTransport::tx_async(uint8_t tx_ep, uint8_t *packet, size_t length, int UsbTransport::tx_sync(uint8_t ep, uint8_t *packet, size_t length, int timeout_ms) { + flush_writes(); /* No libusb_clear_halt here. rtw88_8814au's usbmon shows the first bulk * OUT is preceded by 0 CLEAR_FEATUREs; later CLEAR_FEATUREs happen during * normal TX-queue operation, not the per-send hot path. Resetting the diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 9bb02319..87510bf3 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -8,6 +8,7 @@ * that discovers the bulk endpoints. The exclusive per-adapter UsbDeviceLock * rides here too — its lifetime is the transport's. */ +#include "UsbXferCount.h" #include #include #include @@ -51,6 +52,10 @@ class UsbTransport final : public ITransport { /* Realtek USB register addressing: wValue = addr[15:0], wIndex = * addr[31:16]. Lets the BB/RF window (addr + 0x10000) reach wIndex=1 * instead of colliding with the MAC/system space at wIndex=0. */ + usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + if (_batch) + return async_write(static_cast(addr & 0xFFFF), + static_cast(addr >> 16), &v, sizeof(v)); return libusb_control_transfer( _dev_handle, REALTEK_USB_VENQT_WRITE, 5, static_cast(addr & 0xFFFF), @@ -59,6 +64,13 @@ class UsbTransport final : public ITransport { } uint32_t read32_wide(uint32_t addr) override { uint32_t data = 0; + usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + if (_batch) { + if (async_read(static_cast(addr & 0xFFFF), + static_cast(addr >> 16), &data, sizeof(data))) + return data; + return 0xFFFFFFFFu; + } if (libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_READ, 5, static_cast(addr & 0xFFFF), static_cast(addr >> 16), @@ -68,6 +80,9 @@ class UsbTransport final : public ITransport { return 0xFFFFFFFFu; /* INVALID_RF_DATA-style sentinel on a failed read */ } bool write_bytes(uint16_t reg, const uint8_t *p, size_t n) override; + void write_batch_begin() override; + void write_batch_end() override; + void flush_writes() override; bool tx_async(uint8_t ep, uint8_t *buf, size_t len, unsigned timeout_ms) override; @@ -85,6 +100,44 @@ class UsbTransport final : public ITransport { private: template T ctrl_read(uint16_t reg); template bool ctrl_write(uint16_t reg, T value); + /* Pipelined-write machinery (see IRtlTransport::write_batch_begin). */ + /* Register transfers are 1/2/4 bytes; async_write/async_read refuse a + * larger payload rather than overrun the inline setup buffer. */ + static constexpr size_t kAsyncMaxPayload = 4; + struct AsyncWrite { + libusb_transfer *t; + uint8_t buf[LIBUSB_CONTROL_SETUP_SIZE + kAsyncMaxPayload]; + UsbTransport *self; + bool done; + /* Submitted and not yet reaped: libusb owns `t` and `buf` while set, so + * the slot must not be reused, freed, or handed back to the free list. */ + bool inflight; + int status; + int actual; + }; + static constexpr int kAsyncWriteDepth = 8; + /* Extra event-loop turns spent reaping cancellations after a drain times + * out. A live loop reports each cancellation promptly; a dead one fails + * every turn immediately, so this costs nothing in the case that matters. */ + static constexpr int kFlushCancelTurns = 8; + bool async_write(uint16_t wvalue, uint16_t windex, const void *data, + size_t n); + /* Read queued behind the pending writes (EP0 order) and waited for on its + * own completion only: a read-modify-write pair costs one wakeup, not two. + * Returns false on failure (data untouched). */ + bool async_read(uint16_t wvalue, uint16_t windex, void *data, size_t n); + AsyncWrite *async_take_slot(); + bool async_wait_progress(); /* one event-loop turn; false on timeout/error */ + static void LIBUSB_CALL async_write_cb(libusb_transfer *t); + bool _batch = false; + std::vector _aw_free; + std::vector _aw_all; + int _aw_inflight = 0; + uint64_t _aw_completed = 0; + int _aw_errors = 0; + /* Set when a drain gave up with transfers still submitted: the destructor + * then leaks those slots instead of freeing a transfer libusb still owns. */ + bool _aw_abandoned = false; void discover_endpoints(); /* was InitDvObj */ const char *speed_str() const; static void transfer_callback(struct libusb_transfer *transfer); @@ -158,6 +211,13 @@ class UsbTransport final : public ITransport { template T UsbTransport::ctrl_read(uint16_t reg_num) { T data = 0; + usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + if (_batch) { + if (async_read(reg_num, 0, &data, sizeof(T))) + return data; + _logger->error("rtw_read({:04x}) pipelined, sizeof(T) = {}", reg_num, sizeof(T)); + throw std::ios_base::failure("rtw_read"); + } if (libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_READ, 5, reg_num, 0, (uint8_t *)&data, sizeof(T), USB_TIMEOUT) == sizeof(T)) { @@ -169,6 +229,9 @@ template T UsbTransport::ctrl_read(uint16_t reg_num) { } template bool UsbTransport::ctrl_write(uint16_t reg_num, T value) { + usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + if (_batch) + return async_write(reg_num, 0, &value, sizeof(T)); return libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_WRITE, 5, reg_num, 0, (uint8_t *)&value, sizeof(T), USB_TIMEOUT) == sizeof(T); diff --git a/src/UsbXferCount.h b/src/UsbXferCount.h new file mode 100644 index 00000000..2a724f7f --- /dev/null +++ b/src/UsbXferCount.h @@ -0,0 +1,14 @@ +/* Process-wide count of USB vendor control transfers (register reads and + * writes). Bumped by UsbTransport; read by InitTimer so every init.timing + * stage reports how many transfers it spent, which is the unit the + * bring-up is actually paid in (each one is a synchronous EP0 round trip). */ +#pragma once +#include +#include + +namespace devourer { +inline std::atomic &usb_ctrl_xfers() { + static std::atomic n{0}; + return n; +} +} // namespace devourer diff --git a/src/jaguar3/CLAUDE.md b/src/jaguar3/CLAUDE.md index e44ebee0..07ec47fb 100644 --- a/src/jaguar3/CLAUDE.md +++ b/src/jaguar3/CLAUDE.md @@ -36,6 +36,29 @@ narrowband dividers, RF18 encoding), strategy interfaces `Jaguar3Calibration` single-path 1SS TX, spur channels, LCK, the 2.4 GHz TX kernel-parity limitation) live in `docs/8822e-quirks.md`. +## Bring-up cost and the pipelined register writes + +`InitWrite` is ~14k USB control transfers and nothing else (stage timing: +`InitTimer` events `j3hal.*` / `j3init.*`, each carrying both `ms` and the +`xfers` it spent; `bench_init.py` parses these events but reports only `ms`). +Synchronous, a transfer costs 76–80 µs on an embedded host (ssc338q) and +~27 µs pipelined 8-deep — EP0 completes URBs +in submission order, so `UsbTransport` queues writes asynchronously inside +a `write_batch_begin/end` scope and only reads (submitted behind the queue, +waited on their own completion), bulk transfers and `flush_writes` wait. +`InitWrite` runs its whole bring-up in one batch (RAII scope, ended before +the coex thread starts): 1.30 → 0.65 s warm, 2.04 → ~0.7 s cold, one +drone-side unit. **`Init` (RX-only) opens no batch yet** — not measured on a +ground-station card. +Batches are single-threaded by contract. The ms-scale settle delays +(`write_bb` 0xfc–0xfe, `rf_writer` 0xffe, `Halrf8822e::delay_ms`, the efuse +power-cut) flush first. + +The RF radio-table load is write-only: bits [31:20] of the direct window +(`0x3c00`/`0x4c00 + addr*4`) read back 0 for all 1540 entries, cold and +warm (one 8812EU unit), so the vendor's `MASK20BITS` read-modify-write +preserved nothing at the price of a synchronous read per entry. + ## TX power Both dies drive the SAME TXAGC block (`set_tx_power_ref` is the port of diff --git a/src/jaguar3/HalJaguar3.cpp b/src/jaguar3/HalJaguar3.cpp index ba938e18..c58a5278 100644 --- a/src/jaguar3/HalJaguar3.cpp +++ b/src/jaguar3/HalJaguar3.cpp @@ -1,3 +1,4 @@ +#include "InitTimer.h" #include "HalJaguar3.h" #include #include @@ -41,9 +42,12 @@ void retry_cal(Logger_t &logger, const char *what, F &&step, int tries = 3) { * write. */ void write_bb(RtlAdapter &dev, uint32_t addr, uint32_t data) { switch (addr) { - case 0xfe: std::this_thread::sleep_for(std::chrono::milliseconds(50)); return; - case 0xfd: std::this_thread::sleep_for(std::chrono::milliseconds(5)); return; - case 0xfc: std::this_thread::sleep_for(std::chrono::milliseconds(1)); return; + /* The ms-scale table delays exist to let the preceding writes settle: + * drain the pipelined-write queue before sleeping (sub-ms ones are noise + * next to the ~0.2 ms a depth-8 queue can hold). */ + case 0xfe: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(50)); return; + case 0xfd: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(5)); return; + case 0xfc: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(1)); return; case 0xfb: std::this_thread::sleep_for(std::chrono::microseconds(50)); return; case 0xfa: std::this_thread::sleep_for(std::chrono::microseconds(5)); return; case 0xf9: std::this_thread::sleep_for(std::chrono::microseconds(1)); return; @@ -87,11 +91,16 @@ void HalJaguar3::run_iqk(SelectedChannel channel) { * Every step is ported from vendor source. */ void HalJaguar3::rtw_hal_init(SelectedChannel channel) { ChannelWidth_t bw = channel.ChannelWidth; + InitTimer timer(_logger, "j3hal"); _macinit.pre_init_system_cfg(); + timer.stage("pre_init_system_cfg"); power_on(); /* mac_power_switch(on) — card_en_flow */ + timer.stage("power_on"); read_chip_version(); /* needs MAC alive; supplies cut for system_cfg */ + timer.stage("chip_version"); cache_efuse_8822e(); /* one-shot OTP decode while access is reliable */ + timer.stage("efuse_cache"); if (_variant == ChipVariant::C8822E && _efuse_cache_valid) /* Efuse thermal baseline (0xd0/0xd1) + channel for pwr_track thermal tracking. */ _cal->set_pwr_track_ctx(_efuse_cache[0xd0], _efuse_cache[0xd1], @@ -109,35 +118,46 @@ void HalJaguar3::rtw_hal_init(SelectedChannel channel) { (_phy_ctx.rfe_type == 0 || _phy_ctx.rfe_type == 0xff)) _phy_ctx.rfe_type = 21; _logger->info("Jaguar3: rfe_type=0x{:02x}", _phy_ctx.rfe_type); + timer.stage("efuse_rfe"); _macinit.init_system_cfg(bw, _ver.cut); + timer.stage("init_system_cfg"); if (!_fw.download_default_firmware()) { _logger->error("Jaguar3: firmware download FAILED (structured)"); return; } _logger->info("Jaguar3: firmware booted (structured)"); + timer.stage("dlfw"); if (!_macinit.init_mac_cfg(bw)) { _logger->error("Jaguar3: init_mac_cfg FAILED (structured)"); return; } + timer.stage("init_mac_cfg"); /* Propagate the queue-init reserved-page boundary to the FW downloader — it * was 0 during the pre-queue-init FW stage, so a beacon rsvd-page download * would otherwise target page 0 instead of the real boundary. */ _fw.set_rsvd_boundary(_macinit.rsvd_boundary()); _macinit.init_usb_cfg(); /* USB RX-DMA mode — RX delivery to bulk-IN */ _macinit.enable_bb_rf(true); /* set_hw_value(EN_BB_RF) */ - apply_bb_rf_agc_tables(); /* init_phy: BB + AGC + RF tables */ + timer.stage("usb_cfg_bbrf"); + apply_bb_rf_agc_tables(&timer); /* init_phy: BB + AGC + RF tables */ config_pa_bias_8822e(); /* kfree: efuse PA-bias trim -> RF 0x60 */ + timer.stage("pa_bias"); config_phydm_parameter_init(); /* POST_SETTING: 3-wire + OFDM/CCK block + bb-reset */ + timer.stage("phydm_param_init"); init_rfk(); /* RF cal_init (0x1B00); IQK runs via run_iqk */ + timer.stage("rfk_init"); /* halrf DACK — DAC cal before IQK. Retry-wrapped: its status-poll loops issue * tens of thousands of USB reads and an intermittent glitch was aborting * bring-up (rtw_read iostream error, seen on 8822EU). */ retry_cal(_logger, "DACK", [this] { _cal->dac_calibrate(); }); + timer.stage("dack"); bf_init(); /* rtl8822c_phy_bf_init (rtl8822c_halinit.c) */ monitor_rx_cfg(); /* devourer monitor-mode RX enable */ enable_tx_path(); /* enable OFDM/CCK TX block (gates on-air TX) */ + timer.stage("bf_rx_tx_cfg"); + timer.total(); _logger->info("Jaguar3: bring-up complete"); } @@ -543,6 +563,7 @@ void HalJaguar3::efuse_pwr_cut_8822e(bool on) { if (on) { _device.rtw_write8(PMC, static_cast(_device.rtw_read8(PMC) | kWrMsk)); _device.rtw_write16(SYS_ISO, static_cast(_device.rtw_read16(SYS_ISO) | kPwcS)); + _device.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(1)); _device.rtw_write16(SYS_ISO, static_cast(_device.rtw_read16(SYS_ISO) | kPwcB)); _device.rtw_write16(SYS_ISO, static_cast(_device.rtw_read16(SYS_ISO) & ~kEbCore)); @@ -555,6 +576,7 @@ void HalJaguar3::efuse_pwr_cut_8822e(bool on) { _device.rtw_write32(EFC1, _device.rtw_read32(EFC1) & ~kBurst); _device.rtw_write16(SYS_ISO, static_cast(_device.rtw_read16(SYS_ISO) | kEbCore)); _device.rtw_write16(SYS_ISO, static_cast(_device.rtw_read16(SYS_ISO) & ~kPwcB)); + _device.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(1)); _device.rtw_write16(SYS_ISO, static_cast(_device.rtw_read16(SYS_ISO) & ~kPwcS)); _device.rtw_write8(PMC, static_cast(_device.rtw_read8(PMC) & ~kWrMsk)); @@ -965,7 +987,7 @@ void HalJaguar3::power_on() { _logger->info("Jaguar3: power-on sequence complete (card active)"); } -void HalJaguar3::apply_bb_rf_agc_tables() { +void HalJaguar3::apply_bb_rf_agc_tables(InitTimer *timer) { /* BB + AGC baseline via the validated halbb walker (src/jaguar3/ * PhyTableLoaderJaguar3). _phy_ctx must be populated from the chip-version + * EFUSE read (done earlier in rtw_hal_init) before the tables are walked. */ @@ -976,8 +998,10 @@ void HalJaguar3::apply_bb_rf_agc_tables() { const auto agc_tab = _tables->agc_tab(); _logger->info("Jaguar3: applying BB phy_reg table ({} words)", phy_reg.len); PhyTableLoaderJaguar3::Load(phy_reg.data, phy_reg.len, _phy_ctx, bb); + if (timer) timer->stage("table_phy_reg"); _logger->info("Jaguar3: applying AGC table ({} words)", agc_tab.len); PhyTableLoaderJaguar3::Load(agc_tab.data, agc_tab.len, _phy_ctx, bb); + if (timer) timer->stage("table_agc"); /* RF radio tables. On 8822C an RF register write is a direct BB write to a * per-path window: BB[base + (rf_addr&0xff)*4], 20-bit mask, base 0x3c00 @@ -987,7 +1011,7 @@ void HalJaguar3::apply_bb_rf_agc_tables() { auto rf_writer = [this](uint16_t base) { return [this, base](uint32_t addr, uint32_t data) { switch (addr) { - case 0xffe: std::this_thread::sleep_for(std::chrono::milliseconds(50)); return; + case 0xffe: _device.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(50)); return; case 0xfe: std::this_thread::sleep_for(std::chrono::microseconds(100)); return; case 0xffff: std::this_thread::sleep_for(std::chrono::microseconds(1)); return; case 0x0: @@ -1000,8 +1024,15 @@ void HalJaguar3::apply_bb_rf_agc_tables() { _device.rtw_write32(base == 0x4c00 ? 0x4108 : 0x1808, data & RFREG_MASK); return; default: - _device.phy_set_bb_reg(static_cast(base + ((addr & 0xff) << 2)), - RFREG_MASK, data); + /* Plain 20-bit write, not the vendor's read-modify-write under + * MASK20BITS: the direct window's bits [31:20] read back 0 for every + * one of the 1540 table entries, cold boot and warm restart alike + * (histogrammed on one 8812EU unit), so preserving them is a + * no-op that cost a synchronous read per entry -- half the RF table + * stage, ~80 ms on the ssc338q. Write-only also pipelines + * (IRtlTransport::write_batch_begin). */ + _device.rtw_write32(static_cast(base + ((addr & 0xff) << 2)), + data & RFREG_MASK); } }; }; @@ -1017,6 +1048,7 @@ void HalJaguar3::apply_bb_rf_agc_tables() { _device.phy_set_bb_reg(0x1c90, 1u << 8, 1); _device.phy_set_bb_reg(0x1830, 1u << 29, 1); _device.phy_set_bb_reg(0x4130, 1u << 29, 1); + if (timer) timer->stage("table_rf_ab"); _logger->info("Jaguar3: BB/AGC/RF tables applied"); } diff --git a/src/jaguar3/HalJaguar3.h b/src/jaguar3/HalJaguar3.h index c160356a..602bfeaa 100644 --- a/src/jaguar3/HalJaguar3.h +++ b/src/jaguar3/HalJaguar3.h @@ -99,7 +99,7 @@ class HalJaguar3 { void power_off(); /* card-disable PWR_SEQ — reset from active state */ void power_on(); /* card-enable PWR_SEQ */ void init_rfk(); /* RF-calibration init (0x1B00 cal_init block) */ - void apply_bb_rf_agc_tables(); /* phydm BB/AGC/RF tables via PhyTableLoader */ + void apply_bb_rf_agc_tables(class InitTimer *timer = nullptr); /* phydm BB/AGC/RF tables via PhyTableLoader */ void bf_init(); /* rtl8822c_phy_bf_init: BF/MU + NDPA sounding */ void config_phydm_parameter_init(); /* POST_SETTING: 3-wire + OFDM/CCK block */ void enable_tx_path(); /* OFDM/CCK TX block + AGC/path enable (on-air TX) */ diff --git a/src/jaguar3/Halrf8822e.cpp b/src/jaguar3/Halrf8822e.cpp index b1c08b6f..67e34184 100644 --- a/src/jaguar3/Halrf8822e.cpp +++ b/src/jaguar3/Halrf8822e.cpp @@ -87,6 +87,7 @@ void Halrf8822e::delay_us(uint32_t us) { std::this_thread::sleep_for(std::chrono::microseconds(us)); } void Halrf8822e::delay_ms(uint32_t ms) { + _device.flush_writes(); /* the settle time must follow the writes */ std::this_thread::sleep_for(std::chrono::milliseconds(ms)); } diff --git a/src/jaguar3/Halrf8822e.h b/src/jaguar3/Halrf8822e.h index 7b5c2807..45c3c14d 100644 --- a/src/jaguar3/Halrf8822e.h +++ b/src/jaguar3/Halrf8822e.h @@ -53,7 +53,7 @@ class Halrf8822e : public Jaguar3Calibration { uint32_t rf_read(uint8_t path, uint16_t addr, uint32_t mask); void rf_write(uint8_t path, uint16_t addr, uint32_t mask, uint32_t val); static void delay_us(uint32_t us); - static void delay_ms(uint32_t ms); + void delay_ms(uint32_t ms); /* drains pipelined writes first */ /* --- DAC calibration (port of halrf_dac_cal_8822e / halrf_8822e.c) --- * 8822e DACK uses the AFE S0/S1 banks (0x3800/0x3900) rather than 8822c's BB diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index e4af4531..8e35b313 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -1,3 +1,4 @@ +#include "InitTimer.h" #include "RtlJaguar3Device.h" #include @@ -55,10 +56,23 @@ RtlJaguar3Device::RtlJaguar3Device(RtlAdapter device, Logger_t logger, variant == jaguar3::ChipVariant::C8822E ? "8822E/EU" : "8822C/CU"); } +/* Pipelined register writes for the whole bring-up (IRtlTransport:: + * write_batch_begin): ends on scope exit so a throw never leaves the + * transport in batch mode for the threads that start afterwards. */ +struct WriteBatchScope { + RtlAdapter &dev; + explicit WriteBatchScope(RtlAdapter &d) : dev(d) { dev.write_batch_begin(); } + void end() { dev.write_batch_end(); } + ~WriteBatchScope() { dev.write_batch_end(); } +}; + void RtlJaguar3Device::Init(Action_ParsedRadioPacket packetProcessor, SelectedChannel channel) { _channel = channel; _rx_wanted = true; + /* No WriteBatchScope here (yet): the pipelined bring-up is validated on + * the TX path (InitWrite, cold + warm); the RX-only + * Init path has not been measured with it on a ground-station card. */ _hal.rtw_hal_init(channel); /* full vendor-source bring-up */ /* Tune the channel/bandwidth (5/10 MHz ChannelWidth re-clocks to narrowband), * then run IQK calibration (it reads RF18 for the tuned channel). @@ -735,7 +749,10 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * race the running TX). */ const bool want_rx = _cfg.rx.enable_with_tx; _rx_wanted = want_rx; + InitTimer timer(_logger, "j3init"); + WriteBatchScope batch(_device); _hal.rtw_hal_init(channel); /* full vendor-source bring-up */ + timer.stage("hal_init"); /* 8822C at 40/80 MHz: IQK at 20 MHz, then retune — see Init. */ const bool iqk_at_20 = _variant == jaguar3::ChipVariant::C8822C && (channel.ChannelWidth == CHANNEL_WIDTH_40 || @@ -746,7 +763,9 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { SelectedChannel iqk_ch = channel; if (iqk_at_20) iqk_ch.ChannelWidth = CHANNEL_WIDTH_20; /* IQK command set follows the RF */ + timer.stage("set_channel"); _hal.run_iqk(iqk_ch); + timer.stage("iqk"); if (iqk_at_20) _radioManagement.set_channel_bwmode(channel.Channel, channel.ChannelOffset, channel.ChannelWidth); @@ -755,6 +774,7 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { _hal.dpk_force_bypass_8822e(); /* 8822e rfe 21/22: kernel bypasses DPK (after IQK) */ _hal.config_rfe(channel.Channel); /* 8822e RFE/PAPE antenna-switch pins (PA enable) */ _hal.config_channel_8822e(channel.Channel); /* 8822e band TX scaling/backoff + shaping */ + timer.stage("rx_path_rfe_channel"); /* DEVOURER_CW_TONE — a bare RF LO carrier. Armed HERE (before the FW power-mode * / coex H2C steps below, which on the 8812EU at 5 GHz leave the chip NAKing @@ -801,10 +821,12 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * re-apply at the end of this function; this early one just keeps the * intermediate bring-up steps on sane references. */ apply_tx_power_current(/*full=*/true); + timer.stage("txpower_pre"); _brought_up = true; /* WiFi-only coex bring-up: disable the BT/LTE antenna arbitration and lock the * antenna to WLAN so on-air TX is not killed by the coex firmware. */ _hal.coex_wlan_only_init(); + timer.stage("coex_wlan_only_init"); /* RFE GPIO/pad pinmux — the HalMAC "Config PIN Mux" (halmac_init_8822e) that * devourer's hand-rolled MAC init skips: route + drive the RFE PA-enable / * antenna-switch control pins. Without it the 8822e's PA pins are never driven @@ -825,9 +847,13 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { _device.rtw_write(0x0064, v64 & ~0x02040000u); /* PAD_CTRL1: RFE pads */ } + timer.stage("rfe_pinmux"); _hal.fw_set_pwr_mode_active(); /* keep all FW power domains on (no auto-PS) */ + timer.stage("fw_pwr_mode"); _hal.fw_coex_query_bt_info(); /* make the FW confirm BT is absent */ + timer.stage("fw_coex_query"); _hal.fw_coex_tdma_off(); /* disable coex time-division (WL keeps antenna) */ + timer.stage("fw_coex_tdma_off"); /* DEVOURER_BF_ARM_SOUNDER=1 — beamforming self-sounding probe (beamformer * side): arm the MAC's hardware sounding engine so a TX-descriptor-marked * NDPA (DEVOURER_TX_NDPA=1) is followed by a hardware-generated NDP. The MAC @@ -892,6 +918,7 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * Applied before the coex thread starts so the writes don't contend. */ if (_cfg.tuning.disable_cca) SetCcaMode(true); + timer.stage("filters_cca"); /* DEVOURER_XTAL_CAP — crystal-cap trim (issue #217); before the coex thread * so the AFE write doesn't contend with the periodic coex re-apply. */ if (_cfg.tuning.xtal_cap) @@ -902,6 +929,7 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * TX-power state (flat override / offset) only sticks when applied after * them. The coex thread's ~2 s ticks do not rewrite the refs. */ apply_tx_power_current(/*full=*/true); + timer.stage("txpower_post"); /* Per-packet power banks: the BB init table reset 0x1e70 (0x00001000, all * banks disabled) and may have cleared the per-STA RAM — re-sync the * hardware to the planner state (a pre-bring-up SetTxPacketPowerOffsetQdb @@ -931,6 +959,8 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { _device.rtw_read32(a), _device.rtw_read32(a + 4), _device.rtw_read32(a + 8), _device.rtw_read32(a + 12)); } + timer.stage("dpdt_ack_misc"); + batch.end(); /* sync writes from here: the coex thread shares the transport */ _coex_thread = std::thread([this] { coex_runtime_loop(); }); if (_cfg.rx.ack_responder && !SetAckResponder(*_cfg.rx.ack_responder)) /* DEVOURER_ACK_RESPONDER */ @@ -938,6 +968,8 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { "Jaguar3: configured ACK responder could not be armed"); if (_cfg.tx.ampdu) SetAmpduMode(*_cfg.tx.ampdu); /* DEVOURER_TX_AMPDU_MODE */ + timer.stage("coex_thread_ampdu"); + timer.total(); _logger->info("Jaguar3: ready for TX (monitor inject)"); } From 1627d0fdbd7b6d259356c163d2438e0119512ca9 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:33:13 +0300 Subject: [PATCH 02/29] =?UTF-8?q?usb=20pipelining:=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20shared=20callback=20pool,=208822C=20settle=20flush,?= =?UTF-8?q?=20per-transport=20xfer=20count?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on the pipelined-write bring-up: - Halrf8822c::delay_ms is no longer static and drains the queue before sleeping, like the 8822E one: WriteBatchScope covers DACK/IQK on both dies, and a settle that sleeps over queued writes is no settle. - The write-only RF-table load is now measured on both dies: tests/j3_rf_window_readback.sh dumps both path windows after a bring-up (chipstate --init --peek ...:4) and histograms bits [31:20]; 8812CU and 8812EU both read 0 for all 512 words, so the plain write is bit-identical to the vendor RMW there. - flush_writes closes the batch when it retires slots: with those slots never returning, the in-flight count could not reach zero again and every later register access walked take-slot -> wait -> drain for the rest of the bring-up. Synchronous from that point instead. - The completion callback touches only an AsyncPool shared (shared_ptr) between the transport and every slot, never the transport: a slot the destructor had to leak can outlive the transport, and its callback may still fire through a libusb context another adapter keeps pumping. - write_batch_begin builds its pool transactionally: a null libusb_alloc_transfer tears the partial pool down and leaves the session synchronous, so no null transfer reaches fill/submit. - The transfer counter is per transport instance (ITransport::ctrl_xfers, RtlAdapter::ctrl_xfers), not process-wide; InitTimer takes it as an optional counter and emits `xfers` only when given one, so two adapters in one process no longer cross-attribute, and PCIe timers stay silent rather than reporting 0. UsbXferCount.h is gone. - A failed pipelined read32_wide is logged rather than silently returned as the all-ones sentinel; the drain-timeout comment names the real cause (an unpumped event loop, since USB_TIMEOUT is 500 ms). - Own header first in HalJaguar3.cpp / RtlJaguar3Device.cpp; the IRtlTransport references are retargeted at ITransport (src/Transport.h) after the rename; src/jaguar3/CLAUDE.md keeps only the Jaguar3-specific facts and points at the transport header for the batching contract. - chipstate: --init followed by --peek/--poke runs the ops on the configured chip, and --peek ...:4 reads aligned 32-bit words (the BB/RF windows answer 32-bit reads only). - tests/regress.py learns 0bda:b812 (CF-924AC V2), the bench's recommended ground station, which its DUT table did not list. Co-Authored-By: Claude Fable 5.1 --- docs/logging.md | 2 +- examples/chipstate/main.cpp | 35 ++++++++++-- src/InitTimer.h | 38 +++++++------ src/RtlAdapter.h | 3 +- src/Transport.h | 5 ++ src/UsbTransport.cpp | 97 ++++++++++++++++++++------------ src/UsbTransport.h | 43 ++++++++++---- src/UsbXferCount.h | 14 ----- src/jaguar3/CLAUDE.md | 41 ++++++++------ src/jaguar3/HalJaguar3.cpp | 15 ++--- src/jaguar3/HalJaguar3.h | 3 +- src/jaguar3/Halrf8822c.cpp | 1 + src/jaguar3/Halrf8822c.h | 2 +- src/jaguar3/RtlJaguar3Device.cpp | 6 +- tests/j3_rf_window_readback.sh | 32 +++++++++++ tests/regress.py | 1 + 16 files changed, 226 insertions(+), 112 deletions(-) delete mode 100644 src/UsbXferCount.h create mode 100755 tests/j3_rf_window_readback.sh diff --git a/docs/logging.md b/docs/logging.md index 3e8e591b..9806044f 100644 --- a/docs/logging.md +++ b/docs/logging.md @@ -76,7 +76,7 @@ Emitters: L = library, RX/TX/... = demo. Optional fields in [brackets]; ### Init / infrastructure | ev | emitter | fields | |---|---|---| -| `init.timing` | L (`src/InitTimer.h`) + demos | stage ("scope.stage", e.g. "demo.first_rx_frame", "txdemo.first_tx_submit"), ms, xfers (USB vendor control transfers the stage spent; 0 on PCIe) | +| `init.timing` | L (`src/InitTimer.h`) + demos | stage ("scope.stage", e.g. "demo.first_rx_frame", "txdemo.first_tx_submit"), ms, [xfers] (register transfers the stage spent on that adapter's transport, USB only — present on the Jaguar3 `j3hal.*` / `j3init.*` stages) | | `adapter.caps` | RX, TX, doctor, txpower (`examples/common/caps_event.h`) | supported, chip, names, chip_id "0x..", gen, variant, transport, tx_chains, rx_chains, n_ss, stbc, ldpc, sgi, bw_max, bw[] (MHz), txpwr_max, txpwr_step_qdb, txpwr_step_measured, txpwr_min_qdb, txpwr_max_qdb, txpwr_rate_diffs, txpwr_rate_diffs_hw, txpwr_rate_diffs_measured, tune_2g4[]\|null, tune_5g[]\|null, char_2g4[]\|null, char_5g[]\|null, ldpc_rx_ht, ldpc_rx_vht, ldpc_rx_flag, vht_2g4, per_pkt_txpwr, per_pkt_txpwr_steps, per_pkt_txpwr_step_qdb, per_pkt_txpwr_min_qdb, per_pkt_txpwr_max_qdb, per_pkt_txpwr_measured, narrowband, fastretune, ack_responder, tx_retry_limit, he_er_su, per_chain_rssi, hw_rx_tsf, hw_beacon_txtsf, tsf_write, xtal_cap_max, xtal_cap_default | | `debug.wreg` | L (`DEVOURER_LOG_WRITES`) | addr "0x0nnn", width, val "0x…" | | `hop.prof` | L (`DEVOURER_HOP_PROF`) | gen, ch, `_us`…, total_us | diff --git a/examples/chipstate/main.cpp b/examples/chipstate/main.cpp index 0e3ea62d..dd4e0fcf 100644 --- a/examples/chipstate/main.cpp +++ b/examples/chipstate/main.cpp @@ -58,7 +58,7 @@ struct RegOp { uint16_t addr = 0; uint16_t end = 0; /* peek range: inclusive last addr (== addr if single) */ uint32_t val = 0; /* poke */ - int width = 1; /* poke: 1/2/4 */ + int width = 1; /* poke: 1/2/4; peek: 1 (bytes) or 4 (aligned words) */ }; struct Args { @@ -74,13 +74,16 @@ void usage() { std::fprintf(stderr, "usage: chipstate [--vid 0xNNNN] [--pid 0xNNNN] [--init] " "[--channel N]\n" - " [--peek 0xA[-0xB]]... [--poke 0xA=0xV[:W]]...\n" + " [--peek 0xA[-0xB][:4]]... [--poke 0xA=0xV[:W]]...\n" " default: attach read-only, no USB reset, no bring-up.\n" " --init : run a full bring-up first (for a healthy reference\n" - " dump on a freshly power-cycled adapter).\n" + " dump on a freshly power-cycled adapter). With\n" + " --peek/--poke the ops run AFTER the bring-up.\n" " --peek : dump register byte(s) over the vendor-control path\n" " (range inclusive, 16 bytes/row) instead of the\n" " canary set. Bypasses chip dispatch — any die.\n" + " `:4` reads aligned 32-bit words instead (the BB/RF\n" + " windows answer 32-bit reads only).\n" " --poke : write a register (width W = 1/2/4, default from the\n" " value magnitude). The bench-bisection intervention\n" " lever; ops run in argv order, so a trailing --peek\n" @@ -111,6 +114,14 @@ bool parse_peek(const char *s, RegOp &op) { if (!parse_reg_addr(end + 1, &end, op.end) || op.end < op.addr) return false; } + if (*end == ':') { + if (end[1] != '4' || end[2] != '\0') + return false; + op.width = 4; + op.addr &= ~3u; + op.end |= 3u; + return true; + } return *end == '\0'; } @@ -159,6 +170,17 @@ int run_reg_ops(libusb_device_handle *handle, Logger_t logger, else adapter.rtw_write8(op.addr, static_cast(op.val)); std::printf("poke 0x%04x = 0x%0*x\n", op.addr, op.width * 2, op.val); + } else if (op.width == 4) { + for (uint32_t row = op.addr & ~0xfu; row <= op.end; row += 16) { + std::printf("0x%04x:", row); + for (uint32_t i = row; i < row + 16; i += 4) { + if (i < op.addr || i > op.end) + std::printf(" "); + else + std::printf(" %08x", adapter.rtw_read32(static_cast(i))); + } + std::printf("\n"); + } } else { for (uint32_t row = op.addr & ~0xfu; row <= op.end; row += 16) { std::printf("0x%04x:", row); @@ -281,7 +303,7 @@ int main(int argc, char **argv) { /* --peek/--poke: raw transport-level register access, no device * construction at all — the chip is not even identified, let alone * configured, so this works mid-experiment on any die. */ - if (!a.ops.empty()) + if (!a.ops.empty() && !a.init) return run_reg_ops(handle, logger, ctx, lock, a.ops); devourer::DeviceConfig cfg; @@ -303,6 +325,11 @@ int main(int argc, char **argv) { dev->InitWrite(SelectedChannel{.Channel = static_cast(a.channel), .ChannelOffset = 0, .ChannelWidth = CHANNEL_WIDTH_20}); + /* --init + ops: the question is what the bring-up left in a register, + * so the ops run on the configured chip (vendor control is stateless + * on the handle; the device object stays alive underneath). */ + if (!a.ops.empty()) + return run_reg_ops(handle, logger, ctx, lock, a.ops); } else { logger->info("chipstate: read-only attach (no USB reset, no bring-up) — " "the chip is being read exactly as the last session left it"); diff --git a/src/InitTimer.h b/src/InitTimer.h index 504448ff..49e13a48 100644 --- a/src/InitTimer.h +++ b/src/InitTimer.h @@ -5,15 +5,19 @@ #include #include +#include + #include "logger.h" -#include "UsbXferCount.h" /* Stage timer for init-path profiling. Emits one event per checkpoint: * - * {"ev":"init.timing","stage":".","ms":N,"xfers":K} + * {"ev":"init.timing","stage":".","ms":N[,"xfers":K]} * - * `xfers` is the number of USB control transfers (register reads/writes) - * the stage spent — see UsbXferCount.h; 0 on the PCIe transport. + * `xfers` is emitted only when the timer was given a transfer counter: the + * number of register transfers (reads + writes) that stage spent on that one + * adapter's transport (ITransport::ctrl_xfers, via RtlAdapter::ctrl_xfers), + * which is the unit a USB bring-up is actually paid in. Per adapter, so two + * devices brought up in one process do not cross-attribute; 0 on PCIe. * * `stage()` reports time since the previous checkpoint (or construction); * `total()` reports time since construction. Always-on: a handful of events @@ -23,13 +27,16 @@ class InitTimer { using clock = std::chrono::steady_clock; public: - InitTimer(Logger_t logger, const char *scope) - : _logger{std::move(logger)}, _scope{scope}, _start{clock::now()}, - _last{_start}, _x_start{xfers()}, _x_last{_x_start} {} + using XferCounter = std::function; + + InitTimer(Logger_t logger, const char *scope, XferCounter xfers = {}) + : _logger{std::move(logger)}, _scope{scope}, _xfers{std::move(xfers)}, + _start{clock::now()}, _last{_start}, _x_start{count()}, + _x_last{_x_start} {} void stage(const char *name) { const auto now = clock::now(); - const auto x = xfers(); + const auto x = count(); emit(name, ms(_last, now), static_cast(x - _x_last)); _last = now; _x_last = x; @@ -37,21 +44,19 @@ class InitTimer { void total() { emit("total", ms(_start, clock::now()), - static_cast(xfers() - _x_start)); + static_cast(count() - _x_start)); } private: - static uint64_t xfers() { - return devourer::usb_ctrl_xfers().load(std::memory_order_relaxed); - } + uint64_t count() const { return _xfers ? _xfers() : 0; } void emit(const char *name, long long millis, long long nx) { char stage[96]; std::snprintf(stage, sizeof(stage), "%s.%s", _scope, name); - devourer::Ev(_logger->events(), "init.timing") - .f("stage", stage) - .f("ms", millis) - .f("xfers", nx); + devourer::Ev ev(_logger->events(), "init.timing"); + ev.f("stage", stage).f("ms", millis); + if (_xfers) + ev.f("xfers", nx); } static long long ms(clock::time_point from, clock::time_point to) { @@ -61,6 +66,7 @@ class InitTimer { Logger_t _logger; const char *_scope; + XferCounter _xfers; clock::time_point _start; clock::time_point _last; uint64_t _x_start; diff --git a/src/RtlAdapter.h b/src/RtlAdapter.h index a5a8a31f..45e0ef29 100644 --- a/src/RtlAdapter.h +++ b/src/RtlAdapter.h @@ -88,10 +88,11 @@ class RtlAdapter { /* Pre-power-on HCI programming (rtw88 rtw_hci_setup slot): PCIe TRX ring * registers; no-op on USB. Call per bring-up attempt, before power-on. */ void hci_setup() { _transport->hci_setup(); } - /* Pipelined register writes — see IRtlTransport::write_batch_begin. */ + /* Pipelined register writes — see ITransport::write_batch_begin. */ void write_batch_begin() { _transport->write_batch_begin(); } void write_batch_end() { _transport->write_batch_end(); } void flush_writes() { _transport->flush_writes(); } + uint64_t ctrl_xfers() const { return _transport->ctrl_xfers(); } /* Kernel-style async RX: keep n_urbs concurrent bulk-IN transfers in flight * (USB) or reap the RX buffer-descriptor ring (PCIe), invoking diff --git a/src/Transport.h b/src/Transport.h index 707efc3f..b54d46ec 100644 --- a/src/Transport.h +++ b/src/Transport.h @@ -81,6 +81,11 @@ class ITransport { virtual void write_batch_begin() {} virtual void write_batch_end() {} virtual void flush_writes() {} + /* Register transfers (reads + writes) this transport instance has issued + * so far — the unit a USB bring-up is paid in. InitTimer differences it + * per stage. Per instance, never process-wide. 0 where the notion does + * not apply (PCIe MMIO). */ + virtual uint64_t ctrl_xfers() const { return 0; } /* ---- frame plane ---- */ /* Fire-and-forget data TX (the send_packet hot path). `ep` is the USB diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 42865e44..1c472827 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -345,7 +345,7 @@ UsbTransport::~UsbTransport() { * that somehow fires later touches leaked memory, whereas freeing here * hands libusb a dangling transfer it is still holding. */ if (w->inflight) { - ++leaked; + ++leaked; /* keeps its shared AsyncPool alive for a late callback */ continue; } libusb_free_transfer(w->t); @@ -381,54 +381,71 @@ void UsbTransport::write_batch_begin() { if (_aw_abandoned) return; if (_aw_all.empty()) { + /* Built transactionally: a slot whose transfer allocation failed must + * never reach fill/submit, so on any failure the pool is torn back + * down and the session stays synchronous. */ + std::vector slots; for (int i = 0; i < kAsyncWriteDepth; ++i) { auto *w = new AsyncWrite{}; w->t = libusb_alloc_transfer(0); - w->self = this; - _aw_all.push_back(w); - _aw_free.push_back(w); + if (!w->t) { + delete w; + for (auto *s : slots) { + libusb_free_transfer(s->t); + delete s; + } + _logger->error("USB: libusb_alloc_transfer failed; register writes " + "stay synchronous"); + return; + } + w->pool = _aw; + slots.push_back(w); } + _aw_all = slots; + _aw->free = slots; } - _aw_errors = 0; + _aw->errors = 0; _batch = true; } void UsbTransport::write_batch_end() { flush_writes(); - if (_batch && _aw_errors) + if (_batch && _aw->errors) _logger->error("USB: {} pipelined register write(s) failed in this batch", - _aw_errors); + _aw->errors); _batch = false; } void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { auto *w = static_cast(t->user_data); - UsbTransport *self = w->self; - self->_aw_inflight--; - self->_aw_completed++; + /* Only the shared pool is touched here — never the transport, which a + * leaked slot can outlive (AsyncPool). */ + AsyncPool &pool = *w->pool; + pool.inflight--; + pool.completed++; w->inflight = false; w->done = true; w->status = t->status; w->actual = t->actual_length; if (t->status != LIBUSB_TRANSFER_COMPLETED || t->actual_length != t->length - LIBUSB_CONTROL_SETUP_SIZE) - self->_aw_errors++; + pool.errors++; /* The slot goes back on the free list here; a reader that is waiting on * this very slot copies its data out before it submits anything else * (single-threaded by contract), so the buffer is still intact. */ - self->_aw_free.push_back(w); + pool.free.push_back(w); } UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { - while (_aw_free.empty()) { + while (_aw->free.empty()) { if (!async_wait_progress()) { flush_writes(); /* recovers the pool on a stuck queue */ - if (_aw_free.empty()) + if (_aw->free.empty()) return nullptr; } } - AsyncWrite *w = _aw_free.back(); - _aw_free.pop_back(); + AsyncWrite *w = _aw->free.back(); + _aw->free.pop_back(); w->done = false; w->inflight = false; w->status = -1; @@ -449,13 +466,13 @@ bool UsbTransport::async_read(uint16_t wvalue, uint16_t windex, void *data, USB_TIMEOUT); const int rc = libusb_submit_transfer(w->t); if (rc != 0) { - _aw_free.push_back(w); - _aw_errors++; + _aw->free.push_back(w); + _aw->errors++; _logger->error("USB: pipelined read submit failed ({})", rc); return false; } w->inflight = true; - _aw_inflight++; + _aw->inflight++; while (!w->done) { if (!async_wait_progress()) { flush_writes(); /* cancels + recovers; w->done is set by the cancel */ @@ -470,8 +487,8 @@ bool UsbTransport::async_read(uint16_t wvalue, uint16_t windex, void *data, } bool UsbTransport::async_wait_progress() { - const uint64_t before = _aw_completed; - for (int turns = 0; turns < 8 && _aw_completed == before; ++turns) { + const uint64_t before = _aw->completed; + for (int turns = 0; turns < 8 && _aw->completed == before; ++turns) { struct timeval tv {0, 250 * 1000}; const int rc = libusb_handle_events_timeout_completed(_ctx, &tv, nullptr); if (rc < 0) { @@ -480,17 +497,21 @@ bool UsbTransport::async_wait_progress() { return false; } } - return _aw_completed != before; + return _aw->completed != before; } void UsbTransport::flush_writes() { - while (_aw_inflight > 0) { + while (_aw->inflight > 0) { if (async_wait_progress()) continue; - /* Stuck queue (~2 s without a completion): cancel what is still submitted - * so the caller's next synchronous transfer is not queued behind it. */ + /* No completion in ~2 s of pumping. USB_TIMEOUT is 500 ms, so libusb + * itself times a stuck transfer out and completes it through the + * callback long before this; reaching here means the event loop is not + * delivering completions at all (context torn down, device gone). + * Cancel what is still submitted so the caller's next synchronous + * transfer is not queued behind it. */ _logger->error("USB: pipelined write drain timed out ({} in flight)", - _aw_inflight); + _aw->inflight); for (auto *w : _aw_all) if (w->inflight) libusb_cancel_transfer(w->t); @@ -499,17 +520,23 @@ void UsbTransport::flush_writes() { * must never be zeroed by hand, or a late callback decrements it below * zero (silently disabling every later drain), pushes its slot onto the * free list a second time, and races the destructor's free. */ - for (int i = 0; i < kFlushCancelTurns && _aw_inflight > 0; ++i) + for (int i = 0; i < kFlushCancelTurns && _aw->inflight > 0; ++i) async_wait_progress(); - if (_aw_inflight > 0) { + if (_aw->inflight > 0) { /* The event loop itself is gone (a yanked device reports the error * immediately, so the turns above cost nothing). Leave the slots - * submitted and off the free list; the destructor leaks them. */ - _aw_errors += _aw_inflight; + * submitted and off the free list; the destructor leaks them. The + * batch is closed here too: with those slots never returning, the + * in-flight count can never reach zero again, and every later + * register access would otherwise walk take-slot -> wait -> drain + * (seconds each) for the rest of the bring-up. Synchronous from + * here on. */ + _aw->errors += _aw->inflight; _aw_abandoned = true; + _batch = false; _logger->error("USB: {} pipelined transfer(s) could not be reaped; " "their slots are retired for this session", - _aw_inflight); + _aw->inflight); } return; } @@ -529,20 +556,20 @@ bool UsbTransport::async_write(uint16_t wvalue, uint16_t windex, USB_TIMEOUT); const int rc = libusb_submit_transfer(w->t); if (rc != 0) { - _aw_free.push_back(w); - _aw_errors++; + _aw->free.push_back(w); + _aw->errors++; _logger->error("USB: pipelined write submit failed ({})", rc); return false; } w->inflight = true; - _aw_inflight++; + _aw->inflight++; return true; } bool UsbTransport::write_bytes(uint16_t reg_num, const uint8_t *ptr, size_t n) { /* A vendor control transfer like any other -- counted so an InitTimer stage * that downloads firmware this way reports what it actually spent. */ - usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); flush_writes(); return libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_WRITE, 5, reg_num, 0, const_cast(ptr), n, diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 87510bf3..c284a5a1 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -8,7 +8,6 @@ * that discovers the bulk endpoints. The exclusive per-adapter UsbDeviceLock * rides here too — its lifetime is the transport's. */ -#include "UsbXferCount.h" #include #include #include @@ -52,7 +51,7 @@ class UsbTransport final : public ITransport { /* Realtek USB register addressing: wValue = addr[15:0], wIndex = * addr[31:16]. Lets the BB/RF window (addr + 0x10000) reach wIndex=1 * instead of colliding with the MAC/system space at wIndex=0. */ - usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); if (_batch) return async_write(static_cast(addr & 0xFFFF), static_cast(addr >> 16), &v, sizeof(v)); @@ -64,11 +63,15 @@ class UsbTransport final : public ITransport { } uint32_t read32_wide(uint32_t addr) override { uint32_t data = 0; - usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); if (_batch) { if (async_read(static_cast(addr & 0xFFFF), static_cast(addr >> 16), &data, sizeof(data))) return data; + /* Logged, unlike the synchronous path below: a failed pipelined read + * is a queue problem, not a register problem, and must not be + * mistaken for a register that genuinely reads all-ones. */ + _logger->error("USB: pipelined read32_wide(0x{:05x}) failed", addr); return 0xFFFFFFFFu; } if (libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_READ, 5, @@ -83,6 +86,9 @@ class UsbTransport final : public ITransport { void write_batch_begin() override; void write_batch_end() override; void flush_writes() override; + uint64_t ctrl_xfers() const override { + return _ctrl_xfers.load(std::memory_order_relaxed); + } bool tx_async(uint8_t ep, uint8_t *buf, size_t len, unsigned timeout_ms) override; @@ -100,14 +106,28 @@ class UsbTransport final : public ITransport { private: template T ctrl_read(uint16_t reg); template bool ctrl_write(uint16_t reg, T value); - /* Pipelined-write machinery (see IRtlTransport::write_batch_begin). */ + /* Pipelined-write machinery (see ITransport::write_batch_begin). */ /* Register transfers are 1/2/4 bytes; async_write/async_read refuse a * larger payload rather than overrun the inline setup buffer. */ static constexpr size_t kAsyncMaxPayload = 4; + /* Bookkeeping the completion callback writes to. It is owned jointly by + * the transport and every slot (shared_ptr), not by the transport alone: + * a slot that could not be reaped outlives the transport (see the + * destructor), and its callback may still fire later through a libusb + * context another adapter in the process keeps pumping. It then updates + * this block, which the leaked slot keeps alive, instead of a freed + * UsbTransport. */ + struct AsyncWrite; + struct AsyncPool { + std::vector free; + int inflight = 0; + uint64_t completed = 0; + int errors = 0; + }; struct AsyncWrite { libusb_transfer *t; uint8_t buf[LIBUSB_CONTROL_SETUP_SIZE + kAsyncMaxPayload]; - UsbTransport *self; + std::shared_ptr pool; bool done; /* Submitted and not yet reaped: libusb owns `t` and `buf` while set, so * the slot must not be reused, freed, or handed back to the free list. */ @@ -130,14 +150,15 @@ class UsbTransport final : public ITransport { bool async_wait_progress(); /* one event-loop turn; false on timeout/error */ static void LIBUSB_CALL async_write_cb(libusb_transfer *t); bool _batch = false; - std::vector _aw_free; + std::shared_ptr _aw = std::make_shared(); std::vector _aw_all; - int _aw_inflight = 0; - uint64_t _aw_completed = 0; - int _aw_errors = 0; /* Set when a drain gave up with transfers still submitted: the destructor * then leaks those slots instead of freeing a transfer libusb still owns. */ bool _aw_abandoned = false; + /* Vendor control transfers (register reads + writes) this transport has + * issued; per instance, so two adapters in one process do not + * cross-attribute their InitTimer stage counts. */ + std::atomic _ctrl_xfers{0}; void discover_endpoints(); /* was InitDvObj */ const char *speed_str() const; static void transfer_callback(struct libusb_transfer *transfer); @@ -211,7 +232,7 @@ class UsbTransport final : public ITransport { template T UsbTransport::ctrl_read(uint16_t reg_num) { T data = 0; - usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); if (_batch) { if (async_read(reg_num, 0, &data, sizeof(T))) return data; @@ -229,7 +250,7 @@ template T UsbTransport::ctrl_read(uint16_t reg_num) { } template bool UsbTransport::ctrl_write(uint16_t reg_num, T value) { - usb_ctrl_xfers().fetch_add(1, std::memory_order_relaxed); + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); if (_batch) return async_write(reg_num, 0, &value, sizeof(T)); return libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_WRITE, 5, diff --git a/src/UsbXferCount.h b/src/UsbXferCount.h deleted file mode 100644 index 2a724f7f..00000000 --- a/src/UsbXferCount.h +++ /dev/null @@ -1,14 +0,0 @@ -/* Process-wide count of USB vendor control transfers (register reads and - * writes). Bumped by UsbTransport; read by InitTimer so every init.timing - * stage reports how many transfers it spent, which is the unit the - * bring-up is actually paid in (each one is a synchronous EP0 round trip). */ -#pragma once -#include -#include - -namespace devourer { -inline std::atomic &usb_ctrl_xfers() { - static std::atomic n{0}; - return n; -} -} // namespace devourer diff --git a/src/jaguar3/CLAUDE.md b/src/jaguar3/CLAUDE.md index 07ec47fb..eef19ca1 100644 --- a/src/jaguar3/CLAUDE.md +++ b/src/jaguar3/CLAUDE.md @@ -38,26 +38,31 @@ narrowband dividers, RF18 encoding), strategy interfaces `Jaguar3Calibration` ## Bring-up cost and the pipelined register writes -`InitWrite` is ~14k USB control transfers and nothing else (stage timing: -`InitTimer` events `j3hal.*` / `j3init.*`, each carrying both `ms` and the -`xfers` it spent; `bench_init.py` parses these events but reports only `ms`). -Synchronous, a transfer costs 76–80 µs on an embedded host (ssc338q) and -~27 µs pipelined 8-deep — EP0 completes URBs -in submission order, so `UsbTransport` queues writes asynchronously inside -a `write_batch_begin/end` scope and only reads (submitted behind the queue, -waited on their own completion), bulk transfers and `flush_writes` wait. -`InitWrite` runs its whole bring-up in one batch (RAII scope, ended before -the coex thread starts): 1.30 → 0.65 s warm, 2.04 → ~0.7 s cold, one -drone-side unit. **`Init` (RX-only) opens no batch yet** — not measured on a -ground-station card. -Batches are single-threaded by contract. The ms-scale settle delays -(`write_bb` 0xfc–0xfe, `rf_writer` 0xffe, `Halrf8822e::delay_ms`, the efuse -power-cut) flush first. +`InitWrite` is ~14k USB register transfers and nothing else. The stage +timing (`InitTimer` events `j3hal.*` / `j3init.*`, each carrying `ms` and +the `xfers` it spent; `bench_init.py` parses these events but reports only +`ms`) is what shows it. The batching contract itself — ordering, what +waits, single-threadedness — is documented once, at +`ITransport::write_batch_begin` (`src/Transport.h`) and in `UsbTransport`; +this file carries only how Jaguar3 uses it: + +- `InitWrite` runs its whole bring-up inside one `WriteBatchScope` + (`RtlJaguar3Device.cpp`), ended before the coex thread starts because that + thread shares the transport. `Init` (RX-only) opens no batch yet — not + measured on a ground-station card. +- Every ms-scale settle delay drains the queue first, on both dies: + `write_bb` 0xfc–0xfe, `rf_writer` 0xffe, `Halrf8822c::delay_ms`, + `Halrf8822e::delay_ms`, the efuse power-cut. A settle that sleeps while + its writes are still queued is no settle. +- Measured: 1.30 → 0.65 s warm, 2.04 → ~0.7 s cold on one drone-side + 8812EU (ssc338q host). The transfer-count reduction is deterministic; the + wall-clock figure is one unit, one host. The RF radio-table load is write-only: bits [31:20] of the direct window -(`0x3c00`/`0x4c00 + addr*4`) read back 0 for all 1540 entries, cold and -warm (one 8812EU unit), so the vendor's `MASK20BITS` read-modify-write -preserved nothing at the price of a synchronous read per entry. +(`0x3c00`/`0x4c00 + addr*4`) read back 0 for every table entry, so the +vendor's `MASK20BITS` read-modify-write preserved nothing at the price of a +synchronous read per entry. Measured on one 8812EU (cold and warm) and one +8812CU (`tests/j3_rf_window_readback.sh`). ## TX power diff --git a/src/jaguar3/HalJaguar3.cpp b/src/jaguar3/HalJaguar3.cpp index c58a5278..765c813a 100644 --- a/src/jaguar3/HalJaguar3.cpp +++ b/src/jaguar3/HalJaguar3.cpp @@ -1,5 +1,5 @@ -#include "InitTimer.h" #include "HalJaguar3.h" +#include "InitTimer.h" #include #include @@ -91,7 +91,7 @@ void HalJaguar3::run_iqk(SelectedChannel channel) { * Every step is ported from vendor source. */ void HalJaguar3::rtw_hal_init(SelectedChannel channel) { ChannelWidth_t bw = channel.ChannelWidth; - InitTimer timer(_logger, "j3hal"); + InitTimer timer(_logger, "j3hal", [this] { return _device.ctrl_xfers(); }); _macinit.pre_init_system_cfg(); timer.stage("pre_init_system_cfg"); @@ -1026,11 +1026,12 @@ void HalJaguar3::apply_bb_rf_agc_tables(InitTimer *timer) { default: /* Plain 20-bit write, not the vendor's read-modify-write under * MASK20BITS: the direct window's bits [31:20] read back 0 for every - * one of the 1540 table entries, cold boot and warm restart alike - * (histogrammed on one 8812EU unit), so preserving them is a - * no-op that cost a synchronous read per entry -- half the RF table - * stage, ~80 ms on the ssc338q. Write-only also pipelines - * (IRtlTransport::write_batch_begin). */ + * word of both path windows after a bring-up, on an 8812EU (cold + * and warm) and on an 8812CU (tests/j3_rf_window_readback.sh), so + * an RMW that reads 0 writes exactly `data` and preserving those + * bits is a no-op that cost a synchronous read per entry -- half + * the RF table stage, ~80 ms on the ssc338q. Write-only also + * pipelines (ITransport::write_batch_begin). */ _device.rtw_write32(static_cast(base + ((addr & 0xff) << 2)), data & RFREG_MASK); } diff --git a/src/jaguar3/HalJaguar3.h b/src/jaguar3/HalJaguar3.h index 602bfeaa..637aa9a7 100644 --- a/src/jaguar3/HalJaguar3.h +++ b/src/jaguar3/HalJaguar3.h @@ -14,6 +14,7 @@ #include "HalmacJaguar3MacInit.h" #include "PhyTableLoaderJaguar3.h" +class InitTimer; /* src/InitTimer.h — stage timer for the bring-up */ namespace jaguar3 { /* HalJaguar3 — Jaguar3 chip bring-up: power sequencing, queue/page/LLT init, BB / @@ -99,7 +100,7 @@ class HalJaguar3 { void power_off(); /* card-disable PWR_SEQ — reset from active state */ void power_on(); /* card-enable PWR_SEQ */ void init_rfk(); /* RF-calibration init (0x1B00 cal_init block) */ - void apply_bb_rf_agc_tables(class InitTimer *timer = nullptr); /* phydm BB/AGC/RF tables via PhyTableLoader */ + void apply_bb_rf_agc_tables(::InitTimer *timer = nullptr); /* phydm BB/AGC/RF tables via PhyTableLoader; stage checkpoints when given */ void bf_init(); /* rtl8822c_phy_bf_init: BF/MU + NDPA sounding */ void config_phydm_parameter_init(); /* POST_SETTING: 3-wire + OFDM/CCK block */ void enable_tx_path(); /* OFDM/CCK TX block + AGC/path enable (on-air TX) */ diff --git a/src/jaguar3/Halrf8822c.cpp b/src/jaguar3/Halrf8822c.cpp index 2ff581ef..8a1e5d41 100644 --- a/src/jaguar3/Halrf8822c.cpp +++ b/src/jaguar3/Halrf8822c.cpp @@ -178,6 +178,7 @@ void Halrf8822c::delay_us(uint32_t us) { std::this_thread::sleep_for(std::chrono::microseconds(us)); } void Halrf8822c::delay_ms(uint32_t ms) { + _device.flush_writes(); /* the settle time must follow the writes */ std::this_thread::sleep_for(std::chrono::milliseconds(ms)); } diff --git a/src/jaguar3/Halrf8822c.h b/src/jaguar3/Halrf8822c.h index 2c1e5df5..04ad4c3f 100644 --- a/src/jaguar3/Halrf8822c.h +++ b/src/jaguar3/Halrf8822c.h @@ -111,7 +111,7 @@ class Halrf8822c : public Jaguar3Calibration { uint32_t rf_read(uint8_t path, uint16_t addr, uint32_t mask); void rf_write(uint8_t path, uint16_t addr, uint32_t mask, uint32_t val); static void delay_us(uint32_t us); - static void delay_ms(uint32_t ms); + void delay_ms(uint32_t ms); /* drains pipelined writes first */ /* --- BTC/GNT indirect-register + IQK trigger/poll primitives --- */ uint32_t btc_wait_ready(); diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 8e35b313..2cdf4d6e 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -1,5 +1,5 @@ -#include "InitTimer.h" #include "RtlJaguar3Device.h" +#include "InitTimer.h" #include #include /* INT_MIN — "no radiotap DBM_TX_POWER" sentinel */ @@ -56,7 +56,7 @@ RtlJaguar3Device::RtlJaguar3Device(RtlAdapter device, Logger_t logger, variant == jaguar3::ChipVariant::C8822E ? "8822E/EU" : "8822C/CU"); } -/* Pipelined register writes for the whole bring-up (IRtlTransport:: +/* Pipelined register writes for the whole bring-up (ITransport:: * write_batch_begin): ends on scope exit so a throw never leaves the * transport in batch mode for the threads that start afterwards. */ struct WriteBatchScope { @@ -749,7 +749,7 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * race the running TX). */ const bool want_rx = _cfg.rx.enable_with_tx; _rx_wanted = want_rx; - InitTimer timer(_logger, "j3init"); + InitTimer timer(_logger, "j3init", [this] { return _device.ctrl_xfers(); }); WriteBatchScope batch(_device); _hal.rtw_hal_init(channel); /* full vendor-source bring-up */ timer.stage("hal_init"); diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh new file mode 100755 index 00000000..851fc287 --- /dev/null +++ b/tests/j3_rf_window_readback.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# Does the Jaguar3 RF direct window keep anything above bit 19? +# +# The RF radio tables are applied as plain 20-bit writes to BB[0x3c00/0x4c00 +# + (rf_addr & 0xff) * 4] instead of the vendor's MASK20BITS read-modify- +# write. That is bit-identical to the RMW only if bits [31:20] of every +# window word read back 0 — which this measures, per die, after a full +# bring-up: it dumps both path windows through chipstate --init --peek and +# histograms the high 12 bits of each 32-bit word. +# +# sudo tests/j3_rf_window_readback.sh 0xc812 # 8812CU +# sudo tests/j3_rf_window_readback.sh 0xa81a # 8812EU +set -euo pipefail +PID=${1:?usage: $0 } +ROOT=$(cd "$(dirname "$0")/.." && pwd) +OUT=${OUT:-/tmp/j3_rf_window_readback}; mkdir -p "$OUT" +dump=$OUT/pid${PID}.peek +"$ROOT/build/chipstate" --pid "$PID" --init --peek 0x3c00-0x3fff:4 --peek 0x4c00-0x4fff:4 \ + >"$dump" 2>"$dump.err" || { echo "FAIL: chipstate exited non-zero"; tail -5 "$dump.err"; exit 1; } +python3 - "$dump" "$PID" <<'PY' +import re, sys, collections +words = [] +for line in open(sys.argv[1]): + m = re.match(r'^0x[0-9a-fA-F]{4}:((?:\s+[0-9a-fA-F]{8}){1,4})\s*$', line) + if not m: continue + words += [int(w, 16) for w in m.group(1).split()] +if not words: + print("FAIL: no register rows parsed from", sys.argv[1]); sys.exit(1) +hi = collections.Counter(w >> 20 for w in words) +print(f"pid={sys.argv[2]} words={len(words)} hi12_histogram={dict(sorted(hi.items()))}") +print("VERDICT:", "write-only is bit-identical (all [31:20] == 0)" if hi.keys() == {0} else "NOT ZERO — keep the RMW on this die") +PY diff --git a/tests/regress.py b/tests/regress.py index 4b10f8d1..b18552c2 100755 --- a/tests/regress.py +++ b/tests/regress.py @@ -217,6 +217,7 @@ def _mediatek_duts() -> dict[str, str]: "0bda:b811": "RTL8811AU", "2357:012d": "RTL8822BU (TP-Link T3U, Jaguar2)", "0bda:b82c": "RTL8822BU (Jaguar2)", + "0bda:b812": "RTL8822BU (CF-924AC V2, Jaguar2)", "0bda:c811": "RTL8821CU (Jaguar2)", "0bda:c812": "RTL8812CU (Jaguar3)", "0bda:c82c": "RTL8822CU (Jaguar3)", From ecce836768470e28fe8ff384606b999d1cdfc0fd Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:39:33 +0300 Subject: [PATCH 03/29] =?UTF-8?q?tests:=20j3=5Ftx=5Fflood=5Fab.sh=20?= =?UTF-8?q?=E2=80=94=20alternating=20TX-flood=20A/B=20of=20two=20trees=20o?= =?UTF-8?q?n=20one=20Jaguar3=20DUT?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per rep and per tree: time to first TX submit, bulk-OUT failures and failed synchronous register reads from a timed txdemo flood. Trees alternate rep by rep so a drift in the unit lands on both sides. Used to show the 8812EU's 5 GHz bulk-OUT timeouts are present on master at the same rate as on the pipelined bring-up. Co-Authored-By: Claude Fable 5.1 --- tests/j3_tx_flood_ab.sh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100755 tests/j3_tx_flood_ab.sh diff --git a/tests/j3_tx_flood_ab.sh b/tests/j3_tx_flood_ab.sh new file mode 100755 index 00000000..5ed9afb5 --- /dev/null +++ b/tests/j3_tx_flood_ab.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# TX-side A/B of two devourer trees on one Jaguar3 DUT: N reps of a timed +# txdemo flood per tree, reporting per rep the frames submitted, the bulk-OUT +# failures and the failed synchronous register reads. Alternates trees rep by +# rep so a slow drift in the unit (warm state, ambient) lands on both sides. +# +# sudo tests/j3_tx_flood_ab.sh [reps] [secs] +set -euo pipefail +PID=${1:?pid}; CH=${2:?channel}; A=${3:?treeA}; B=${4:?treeB}; REPS=${5:-3}; SECS=${6:-15} +OUT=${OUT:-/tmp/j3_tx_flood_ab/pid${PID}_ch${CH}}; mkdir -p "$OUT" +run() { + local tree=$1 rep=$2 n; n=$(basename "$tree") + local log=$OUT/${n}_rep${rep} + if ! env DEVOURER_PID="$PID" DEVOURER_CHANNEL="$CH" DEVOURER_LOG_LEVEL=info \ + timeout -s INT "$SECS" "$tree/build/txdemo" >"$log.jsonl" 2>"$log.err"; then + rc=$?; case $rc in 0|124|130) ;; *) echo "$n rep$rep: txdemo exited rc=$rc"; tail -3 "$log.err";; esac + fi + local tx fail rd first + tx=$(grep -cF '"ev":"tx.' "$log.jsonl" || true) + fail=$(grep -c 'bulk_send EP .* FAIL' "$log.err" || true) + rd=$(grep -c 'rtw_read(' "$log.err" || true) + first=$(grep -oE 'first_tx_submit","ms":[0-9]+' "$log.jsonl" | grep -oE '[0-9]+$' || echo '?') + echo "$n rep$rep: first_tx_ms=$first tx_events=$tx bulk_fail=$fail read_fail=$rd" +} +for r in $(seq 1 "$REPS"); do run "$A" "$r"; run "$B" "$r"; done From 47e741ceed36e084e2c8f4f5ab261ed11acede2a Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:52:23 +0300 Subject: [PATCH 04/29] usb pipelining: flush on every settle; RF-window check pokes the bits it tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Every settle drains the pipelined queue first, the microsecond ones included (delay_us on both Halrf8822c and Halrf8822e, now instance methods; the write_bb / rf_writer µs table markers). The µs sites include 2 ms and 10 ms waits, so the ms/µs split was not a safe line. The drain is free on an empty queue and bounded by its depth otherwise. - tests/j3_rf_window_readback.sh gains the leg that decides: after the histogram it pokes a sample of window words with bits [31:20] SET (low 20 bits unchanged, so the RF register keeps its value), reads them back and restores them. Bits that are storage read back set; bits the MASK20BITS RMW could never have preserved read back 0. This does not depend on which bring-up ran first, unlike the histogram alone, and the exit code now carries the verdict. - chipstate --init + ops releases the device object before the ops, so they cannot interleave with the coex thread's register writes; 0xb812 joins its discovery list. - tests/j3_tx_flood_ab.sh captures txdemo's real exit status instead of the status of a shell negation. Co-Authored-By: Claude Fable 5.1 --- examples/chipstate/main.cpp | 13 ++++--- src/jaguar3/CLAUDE.md | 9 ++--- src/jaguar3/HalJaguar3.cpp | 18 +++++----- src/jaguar3/Halrf8822c.cpp | 4 +++ src/jaguar3/Halrf8822c.h | 2 +- src/jaguar3/Halrf8822e.cpp | 4 +++ src/jaguar3/Halrf8822e.h | 2 +- tests/j3_rf_window_readback.sh | 66 ++++++++++++++++++++++++++-------- tests/j3_tx_flood_ab.sh | 9 ++--- 9 files changed, 91 insertions(+), 36 deletions(-) diff --git a/examples/chipstate/main.cpp b/examples/chipstate/main.cpp index dd4e0fcf..0b2d0372 100644 --- a/examples/chipstate/main.cpp +++ b/examples/chipstate/main.cpp @@ -49,7 +49,7 @@ namespace { /* Same list the other demos' open loop iterates; --pid narrows to one. */ const uint16_t kRealtekPids[] = {0x8812, 0x8813, 0x881a, 0x0811, 0xa811, 0x0820, 0x0821, 0x8822, 0x0120, 0x012d, - 0xb82c, 0xc811, 0xc812, 0xa81a}; + 0xb82c, 0xb812, 0xc811, 0xc812, 0xa81a}; /* One --peek/--poke, kept in argv order so a poke-then-peek verifies the * write inside a single claim. */ @@ -326,10 +326,15 @@ int main(int argc, char **argv) { .ChannelOffset = 0, .ChannelWidth = CHANNEL_WIDTH_20}); /* --init + ops: the question is what the bring-up left in a register, - * so the ops run on the configured chip (vendor control is stateless - * on the handle; the device object stays alive underneath). */ - if (!a.ops.empty()) + * so the ops run on the configured chip. The device object is released + * first: its destructor joins the Jaguar3 coex thread (and does not + * de-init the chip — that is Stop(), which this tool never calls), so + * the raw-adapter ops below cannot interleave with a background + * register write. Same handle, interface still claimed. */ + if (!a.ops.empty()) { + session.adopt_device(nullptr); return run_reg_ops(handle, logger, ctx, lock, a.ops); + } } else { logger->info("chipstate: read-only attach (no USB reset, no bring-up) — " "the chip is being read exactly as the last session left it"); diff --git a/src/jaguar3/CLAUDE.md b/src/jaguar3/CLAUDE.md index eef19ca1..d02453c6 100644 --- a/src/jaguar3/CLAUDE.md +++ b/src/jaguar3/CLAUDE.md @@ -50,10 +50,11 @@ this file carries only how Jaguar3 uses it: (`RtlJaguar3Device.cpp`), ended before the coex thread starts because that thread shares the transport. `Init` (RX-only) opens no batch yet — not measured on a ground-station card. -- Every ms-scale settle delay drains the queue first, on both dies: - `write_bb` 0xfc–0xfe, `rf_writer` 0xffe, `Halrf8822c::delay_ms`, - `Halrf8822e::delay_ms`, the efuse power-cut. A settle that sleeps while - its writes are still queued is no settle. +- Every settle delay drains the queue first, µs ones included, on both + dies: the `write_bb` / `rf_writer` table delay markers, `delay_us` and + `delay_ms` on `Halrf8822c` and `Halrf8822e`, the efuse power-cut. A settle + that sleeps while its writes are still queued is no settle; the drain is + free on an empty queue and bounded by its depth otherwise. - Measured: 1.30 → 0.65 s warm, 2.04 → ~0.7 s cold on one drone-side 8812EU (ssc338q host). The transfer-count reduction is deterministic; the wall-clock figure is one unit, one host. diff --git a/src/jaguar3/HalJaguar3.cpp b/src/jaguar3/HalJaguar3.cpp index 765c813a..95f50447 100644 --- a/src/jaguar3/HalJaguar3.cpp +++ b/src/jaguar3/HalJaguar3.cpp @@ -42,15 +42,17 @@ void retry_cal(Logger_t &logger, const char *what, F &&step, int tries = 3) { * write. */ void write_bb(RtlAdapter &dev, uint32_t addr, uint32_t data) { switch (addr) { - /* The ms-scale table delays exist to let the preceding writes settle: - * drain the pipelined-write queue before sleeping (sub-ms ones are noise - * next to the ~0.2 ms a depth-8 queue can hold). */ + /* The table delays exist to let the preceding writes settle, so every + * one of them drains the pipelined-write queue before sleeping — a + * settle measured from a write that is still queued is no settle. The + * drain is free when the queue is empty and bounded by its depth when + * not. */ case 0xfe: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(50)); return; case 0xfd: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(5)); return; case 0xfc: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(1)); return; - case 0xfb: std::this_thread::sleep_for(std::chrono::microseconds(50)); return; - case 0xfa: std::this_thread::sleep_for(std::chrono::microseconds(5)); return; - case 0xf9: std::this_thread::sleep_for(std::chrono::microseconds(1)); return; + case 0xfb: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::microseconds(50)); return; + case 0xfa: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::microseconds(5)); return; + case 0xf9: dev.flush_writes(); std::this_thread::sleep_for(std::chrono::microseconds(1)); return; default: dev.phy_set_bb_reg(static_cast(addr), MASKDWORD, data); } } @@ -1012,8 +1014,8 @@ void HalJaguar3::apply_bb_rf_agc_tables(InitTimer *timer) { return [this, base](uint32_t addr, uint32_t data) { switch (addr) { case 0xffe: _device.flush_writes(); std::this_thread::sleep_for(std::chrono::milliseconds(50)); return; - case 0xfe: std::this_thread::sleep_for(std::chrono::microseconds(100)); return; - case 0xffff: std::this_thread::sleep_for(std::chrono::microseconds(1)); return; + case 0xfe: _device.flush_writes(); std::this_thread::sleep_for(std::chrono::microseconds(100)); return; + case 0xffff: _device.flush_writes(); std::this_thread::sleep_for(std::chrono::microseconds(1)); return; case 0x0: /* RF reg 0x0 (mode register) can't be written through the direct * window — it silently no-ops (hardware-observed on the 8822e). The diff --git a/src/jaguar3/Halrf8822c.cpp b/src/jaguar3/Halrf8822c.cpp index 8a1e5d41..d4409e4c 100644 --- a/src/jaguar3/Halrf8822c.cpp +++ b/src/jaguar3/Halrf8822c.cpp @@ -175,6 +175,10 @@ void Halrf8822c::restore_rf(const uint32_t rf[][2]) { } void Halrf8822c::delay_us(uint32_t us) { + /* A settle is measured from the write reaching the chip, so drain the + * pipelined queue first (free when it is empty; bounded by the queue + * depth when not). The µs sites include 2 ms and 10 ms waits. */ + _device.flush_writes(); std::this_thread::sleep_for(std::chrono::microseconds(us)); } void Halrf8822c::delay_ms(uint32_t ms) { diff --git a/src/jaguar3/Halrf8822c.h b/src/jaguar3/Halrf8822c.h index 04ad4c3f..7f9e058a 100644 --- a/src/jaguar3/Halrf8822c.h +++ b/src/jaguar3/Halrf8822c.h @@ -110,7 +110,7 @@ class Halrf8822c : public Jaguar3Calibration { uint8_t mac_read8(uint16_t addr) { return _device.rtw_read8(addr); } uint32_t rf_read(uint8_t path, uint16_t addr, uint32_t mask); void rf_write(uint8_t path, uint16_t addr, uint32_t mask, uint32_t val); - static void delay_us(uint32_t us); + void delay_us(uint32_t us); /* drains pipelined writes first */ void delay_ms(uint32_t ms); /* drains pipelined writes first */ /* --- BTC/GNT indirect-register + IQK trigger/poll primitives --- */ diff --git a/src/jaguar3/Halrf8822e.cpp b/src/jaguar3/Halrf8822e.cpp index 67e34184..4191ec0a 100644 --- a/src/jaguar3/Halrf8822e.cpp +++ b/src/jaguar3/Halrf8822e.cpp @@ -84,6 +84,10 @@ void Halrf8822e::rf_write(uint8_t path, uint16_t addr, uint32_t mask, /* --- calibration (Phase C: ported incrementally, hardware-iterated) --- */ void Halrf8822e::delay_us(uint32_t us) { + /* A settle is measured from the write reaching the chip, so drain the + * pipelined queue first (free when it is empty; bounded by the queue + * depth when not). The µs sites include 2 ms and 10 ms waits. */ + _device.flush_writes(); std::this_thread::sleep_for(std::chrono::microseconds(us)); } void Halrf8822e::delay_ms(uint32_t ms) { diff --git a/src/jaguar3/Halrf8822e.h b/src/jaguar3/Halrf8822e.h index 45c3c14d..c7fe4ce9 100644 --- a/src/jaguar3/Halrf8822e.h +++ b/src/jaguar3/Halrf8822e.h @@ -52,7 +52,7 @@ class Halrf8822e : public Jaguar3Calibration { void mac_write8(uint16_t addr, uint8_t val) { _device.rtw_write8(addr, val); } uint32_t rf_read(uint8_t path, uint16_t addr, uint32_t mask); void rf_write(uint8_t path, uint16_t addr, uint32_t mask, uint32_t val); - static void delay_us(uint32_t us); + void delay_us(uint32_t us); /* drains pipelined writes first */ void delay_ms(uint32_t ms); /* drains pipelined writes first */ /* --- DAC calibration (port of halrf_dac_cal_8822e / halrf_8822e.c) --- diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh index 851fc287..ca7e3287 100755 --- a/tests/j3_rf_window_readback.sh +++ b/tests/j3_rf_window_readback.sh @@ -1,32 +1,70 @@ #!/usr/bin/env bash -# Does the Jaguar3 RF direct window keep anything above bit 19? +# Does the Jaguar3 RF direct window hold anything above bit 19? # # The RF radio tables are applied as plain 20-bit writes to BB[0x3c00/0x4c00 # + (rf_addr & 0xff) * 4] instead of the vendor's MASK20BITS read-modify- # write. That is bit-identical to the RMW only if bits [31:20] of every -# window word read back 0 — which this measures, per die, after a full -# bring-up: it dumps both path windows through chipstate --init --peek and -# histograms the high 12 bits of each 32-bit word. +# window word are not storage the RMW could have preserved. Two legs, per +# die, both through `chipstate --init` (ops run on the configured chip): +# +# 1. histogram: dump both path windows after a bring-up and histogram the +# high 12 bits of every word (expected all 0). On its own this proves +# little, because the write-only load itself clears those bits. +# 2. write-back: for a sample of words in each window, poke the word with +# its high 12 bits SET (low 20 bits unchanged, so the RF register keeps +# its value), read it back, then restore. If the bits are storage they +# read back set and the RMW was preserving real state; if they read back +# 0 the RMW could never have preserved anything. This leg is what +# decides, and it does not depend on which bring-up ran first. +# +# Exit 0 only when both legs say the bits are not storage. # # sudo tests/j3_rf_window_readback.sh 0xc812 # 8812CU # sudo tests/j3_rf_window_readback.sh 0xa81a # 8812EU set -euo pipefail PID=${1:?usage: $0 } ROOT=$(cd "$(dirname "$0")/.." && pwd) +CS=$ROOT/build/chipstate OUT=${OUT:-/tmp/j3_rf_window_readback}; mkdir -p "$OUT" +SAMPLE=${SAMPLE:-8} # words sampled per path window for the write-back leg dump=$OUT/pid${PID}.peek -"$ROOT/build/chipstate" --pid "$PID" --init --peek 0x3c00-0x3fff:4 --peek 0x4c00-0x4fff:4 \ - >"$dump" 2>"$dump.err" || { echo "FAIL: chipstate exited non-zero"; tail -5 "$dump.err"; exit 1; } -python3 - "$dump" "$PID" <<'PY' +"$CS" --pid "$PID" --init --peek 0x3c00-0x3fff:4 --peek 0x4c00-0x4fff:4 \ + >"$dump" 2>"$dump.err" || { echo "FAIL: chipstate exited non-zero (leg 1)"; tail -5 "$dump.err"; exit 1; } +# leg 1 + the op list for leg 2 (poke set / peek / poke restore per word) +ops=$(python3 - "$dump" "$PID" "$SAMPLE" <<'PY' import re, sys, collections -words = [] +words = {} for line in open(sys.argv[1]): - m = re.match(r'^0x[0-9a-fA-F]{4}:((?:\s+[0-9a-fA-F]{8}){1,4})\s*$', line) + m = re.match(r'^0x([0-9a-fA-F]{4}):((?:\s+[0-9a-fA-F]{8}){1,4})\s*$', line) if not m: continue - words += [int(w, 16) for w in m.group(1).split()] + base = int(m.group(1), 16) + for i, w in enumerate(m.group(2).split()): + words[base + 4 * i] = int(w, 16) if not words: - print("FAIL: no register rows parsed from", sys.argv[1]); sys.exit(1) -hi = collections.Counter(w >> 20 for w in words) -print(f"pid={sys.argv[2]} words={len(words)} hi12_histogram={dict(sorted(hi.items()))}") -print("VERDICT:", "write-only is bit-identical (all [31:20] == 0)" if hi.keys() == {0} else "NOT ZERO — keep the RMW on this die") + print("FAIL: no register rows parsed", file=sys.stderr); sys.exit(1) +hi = collections.Counter(w >> 20 for w in words.values()) +print(f"leg1 pid={sys.argv[2]} words={len(words)} hi12_histogram={dict(sorted(hi.items()))}", file=sys.stderr) +n = int(sys.argv[3]); ops = [] +for base in (0x3c00, 0x4c00): + for a in range(base, base + 4 * n, 4): + v = words[a] + ops += [f"--poke 0x{a:04x}=0x{(v | 0xFFF00000):08x}:4", f"--peek 0x{a:04x}-0x{a+3:04x}:4", f"--poke 0x{a:04x}=0x{v:08x}:4"] +print(" ".join(ops)) +PY +) || exit 1 +wb=$OUT/pid${PID}.writeback +# shellcheck disable=SC2086 +"$CS" --pid "$PID" --init $ops >"$wb" 2>"$wb.err" || { echo "FAIL: chipstate exited non-zero (leg 2)"; tail -5 "$wb.err"; exit 1; } +python3 - "$wb" "$PID" <<'PY' +import re, sys +rows = [(int(m.group(1), 16), int(m.group(2), 16)) for m in + (re.match(r'^0x([0-9a-fA-F]{4}):\s+([0-9a-fA-F]{8})\s*$', l) for l in open(sys.argv[1])) if m] +if not rows: + print("FAIL: no read-back rows parsed"); sys.exit(1) +bad = [(a, v) for a, v in rows if v >> 20] +print(f"leg2 pid={sys.argv[2]} words_poked_with_hi_bits_set={len(rows)} read_back_nonzero_hi={len(bad)}") +for a, v in bad: print(f" 0x{a:04x} -> 0x{v:08x}") +if bad: + print("VERDICT: bits [31:20] ARE storage on this die — keep the MASK20BITS RMW"); sys.exit(1) +print("VERDICT: bits [31:20] are not storage (written 1s read back 0) — the write-only load is bit-identical to the RMW") PY diff --git a/tests/j3_tx_flood_ab.sh b/tests/j3_tx_flood_ab.sh index 5ed9afb5..d86fe0e7 100755 --- a/tests/j3_tx_flood_ab.sh +++ b/tests/j3_tx_flood_ab.sh @@ -11,10 +11,11 @@ OUT=${OUT:-/tmp/j3_tx_flood_ab/pid${PID}_ch${CH}}; mkdir -p "$OUT" run() { local tree=$1 rep=$2 n; n=$(basename "$tree") local log=$OUT/${n}_rep${rep} - if ! env DEVOURER_PID="$PID" DEVOURER_CHANNEL="$CH" DEVOURER_LOG_LEVEL=info \ - timeout -s INT "$SECS" "$tree/build/txdemo" >"$log.jsonl" 2>"$log.err"; then - rc=$?; case $rc in 0|124|130) ;; *) echo "$n rep$rep: txdemo exited rc=$rc"; tail -3 "$log.err";; esac - fi + local rc=0 + env DEVOURER_PID="$PID" DEVOURER_CHANNEL="$CH" DEVOURER_LOG_LEVEL=info \ + timeout -s INT "$SECS" "$tree/build/txdemo" >"$log.jsonl" 2>"$log.err" || rc=$? + # 124 = timeout fired (the normal end), 0/130 = txdemo took the INT itself. + case $rc in 0|124|130) ;; *) echo "$n rep$rep: txdemo exited rc=$rc"; tail -3 "$log.err";; esac local tx fail rd first tx=$(grep -cF '"ev":"tx.' "$log.jsonl" || true) fail=$(grep -c 'bulk_send EP .* FAIL' "$log.err" || true) From cc7410917557d4e1d35cddbd956856d762ae2fb3 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:10:04 +0300 Subject: [PATCH 05/29] =?UTF-8?q?usb=20pipelining:=20failure=20paths=20?= =?UTF-8?q?=E2=80=94=20sync=20fallback,=20abandoned-queue=20short-circuit,?= =?UTF-8?q?=20one-shot=20scope?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A pipelined write that cannot be submitted (no usable slot, submit rejected) now falls through to the synchronous control transfer. EP0 keeps submission order, so it lands behind whatever is still queued and no register write is dropped just because the caller ignores the bool. - flush_writes returns at once after a drain has retired slots: the in-flight count stays positive for good by design, and re-draining it on every later flush (bulk sends, batch close, destruction) would only repeat the timeout + cancel turns. - async_take_slot hands out nothing after a recovery flush that abandoned the queue or closed the batch, even when some cancellations did return a slot, so the caller takes the synchronous path rather than queueing another transfer behind the stuck ones. - WriteBatchScope closes once: end() disarms the destructor, so nothing touches the transport's batch state after the coex thread that shares it has started, on the normal path and when unwinding. - The j3init timing closes before the coex thread starts; it shares the adapter's transfer counter, so the final stage and total no longer count that thread's register and H2C traffic. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 10 +++++++++- src/UsbTransport.h | 15 ++++++++++----- src/jaguar3/RtlJaguar3Device.cpp | 17 +++++++++++++---- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 1c472827..7cb0165f 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -440,7 +440,10 @@ UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { while (_aw->free.empty()) { if (!async_wait_progress()) { flush_writes(); /* recovers the pool on a stuck queue */ - if (_aw->free.empty()) + /* A recovery that retired slots closed the batch: hand out nothing, + * even if some cancellations did return a slot, so the caller takes + * the synchronous path instead of queueing behind the stuck ones. */ + if (_aw_abandoned || !_batch || _aw->free.empty()) return nullptr; } } @@ -501,6 +504,11 @@ bool UsbTransport::async_wait_progress() { } void UsbTransport::flush_writes() { + /* Retired slots keep the in-flight count positive for good; draining + * them again would only repeat the timeout + cancel turns on every later + * flush (bulk sends, batch close, destruction). */ + if (_aw_abandoned) + return; while (_aw->inflight > 0) { if (async_wait_progress()) continue; diff --git a/src/UsbTransport.h b/src/UsbTransport.h index c284a5a1..831f2f53 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -52,9 +52,13 @@ class UsbTransport final : public ITransport { * addr[31:16]. Lets the BB/RF window (addr + 0x10000) reach wIndex=1 * instead of colliding with the MAC/system space at wIndex=0. */ _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); - if (_batch) - return async_write(static_cast(addr & 0xFFFF), - static_cast(addr >> 16), &v, sizeof(v)); + /* A pipelined write that cannot be submitted (no usable slot, submit + * rejected) falls through to the synchronous transfer below: EP0 keeps + * submission order, so it lands behind whatever is still queued and no + * register write is silently dropped. */ + if (_batch && async_write(static_cast(addr & 0xFFFF), + static_cast(addr >> 16), &v, sizeof(v))) + return true; return libusb_control_transfer( _dev_handle, REALTEK_USB_VENQT_WRITE, 5, static_cast(addr & 0xFFFF), @@ -251,8 +255,9 @@ template T UsbTransport::ctrl_read(uint16_t reg_num) { template bool UsbTransport::ctrl_write(uint16_t reg_num, T value) { _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); - if (_batch) - return async_write(reg_num, 0, &value, sizeof(T)); + /* Unsubmittable pipelined write -> synchronous, in order (see write32_wide). */ + if (_batch && async_write(reg_num, 0, &value, sizeof(T))) + return true; return libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_WRITE, 5, reg_num, 0, (uint8_t *)&value, sizeof(T), USB_TIMEOUT) == sizeof(T); diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 2cdf4d6e..857a9336 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -61,9 +61,16 @@ RtlJaguar3Device::RtlJaguar3Device(RtlAdapter device, Logger_t logger, * transport in batch mode for the threads that start afterwards. */ struct WriteBatchScope { RtlAdapter &dev; + bool open = true; explicit WriteBatchScope(RtlAdapter &d) : dev(d) { dev.write_batch_begin(); } - void end() { dev.write_batch_end(); } - ~WriteBatchScope() { dev.write_batch_end(); } + /* Closes once: after end() the destructor is a no-op, so it never touches + * the transport again once the coex thread (which shares it) is running. */ + void end() { + if (open) + dev.write_batch_end(); + open = false; + } + ~WriteBatchScope() { end(); } }; void RtlJaguar3Device::Init(Action_ParsedRadioPacket packetProcessor, @@ -961,6 +968,10 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { } timer.stage("dpdt_ack_misc"); batch.end(); /* sync writes from here: the coex thread shares the transport */ + /* The timing closes here, before the coex thread starts: it shares the + * adapter's transfer counter, so anything emitted after it would count + * that thread's register and H2C traffic as bring-up. */ + timer.total(); _coex_thread = std::thread([this] { coex_runtime_loop(); }); if (_cfg.rx.ack_responder && !SetAckResponder(*_cfg.rx.ack_responder)) /* DEVOURER_ACK_RESPONDER */ @@ -968,8 +979,6 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { "Jaguar3: configured ACK responder could not be armed"); if (_cfg.tx.ampdu) SetAmpduMode(*_cfg.tx.ampdu); /* DEVOURER_TX_AMPDU_MODE */ - timer.stage("coex_thread_ampdu"); - timer.total(); _logger->info("Jaguar3: ready for TX (monitor inject)"); } From 796047b80aa5bd8c8bd31a4fbebf89d4f7794fdb Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:22:18 +0300 Subject: [PATCH 06/29] usb pipelining: callback-safe pool accounting; drain before the efuse trigger settle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The completion callback normally runs on the submitting thread (this transport pumps its own events while it waits), but a second adapter sharing the libusb context can pump it from another thread and run the callback the moment libusb has the transfer. The pool counters are now atomic, the free list sits under a mutex, and a slot is accounted as in flight BEFORE it is submitted (rolled back if libusb refuses it), so the callback can never observe a completed slot the submitter has not yet counted. `done` is published after status/actual. - efuse_phys_read_8822e drains the queue between the EFC trigger write and its 50 µs settle; the CW-tone arm retry drains before its back-off. The remaining sleeps in the batched bring-up (power-on, H2C box, DLFW polls) read before they sleep, and a read is ordered behind the queued writes on EP0, so they need no drain. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 78 +++++++++++++++++++++----------- src/UsbTransport.h | 24 +++++++--- src/jaguar3/HalJaguar3.cpp | 1 + src/jaguar3/RtlJaguar3Device.cpp | 1 + 4 files changed, 70 insertions(+), 34 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 7cb0165f..21bcb1cb 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -402,6 +402,7 @@ void UsbTransport::write_batch_begin() { slots.push_back(w); } _aw_all = slots; + std::lock_guard lk(_aw->mu); _aw->free = slots; } _aw->errors = 0; @@ -410,9 +411,9 @@ void UsbTransport::write_batch_begin() { void UsbTransport::write_batch_end() { flush_writes(); - if (_batch && _aw->errors) + if (_batch && _aw->errors.load()) _logger->error("USB: {} pipelined register write(s) failed in this batch", - _aw->errors); + _aw->errors.load()); _batch = false; } @@ -421,10 +422,6 @@ void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { /* Only the shared pool is touched here — never the transport, which a * leaked slot can outlive (AsyncPool). */ AsyncPool &pool = *w->pool; - pool.inflight--; - pool.completed++; - w->inflight = false; - w->done = true; w->status = t->status; w->actual = t->actual_length; if (t->status != LIBUSB_TRANSFER_COMPLETED || @@ -432,23 +429,40 @@ void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { pool.errors++; /* The slot goes back on the free list here; a reader that is waiting on * this very slot copies its data out before it submits anything else - * (single-threaded by contract), so the buffer is still intact. */ - pool.free.push_back(w); + * (single-threaded by contract), so the buffer is still intact. `done` + * is published last, after status/actual, so a waiter that sees it sees + * the result too. */ + { + std::lock_guard lk(pool.mu); + pool.free.push_back(w); + } + w->inflight = false; + pool.inflight--; + w->done = true; + pool.completed++; } UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { - while (_aw->free.empty()) { + auto take = [this]() -> AsyncWrite * { + std::lock_guard lk(_aw->mu); + if (_aw->free.empty()) + return nullptr; + AsyncWrite *w = _aw->free.back(); + _aw->free.pop_back(); + return w; + }; + AsyncWrite *w = take(); + while (!w) { if (!async_wait_progress()) { flush_writes(); /* recovers the pool on a stuck queue */ /* A recovery that retired slots closed the batch: hand out nothing, * even if some cancellations did return a slot, so the caller takes * the synchronous path instead of queueing behind the stuck ones. */ - if (_aw_abandoned || !_batch || _aw->free.empty()) + if (_aw_abandoned || !_batch) return nullptr; } + w = take(); } - AsyncWrite *w = _aw->free.back(); - _aw->free.pop_back(); w->done = false; w->inflight = false; w->status = -1; @@ -467,15 +481,10 @@ bool UsbTransport::async_read(uint16_t wvalue, uint16_t windex, void *data, static_cast(n)); libusb_fill_control_transfer(w->t, _dev_handle, w->buf, async_write_cb, w, USB_TIMEOUT); - const int rc = libusb_submit_transfer(w->t); - if (rc != 0) { - _aw->free.push_back(w); - _aw->errors++; - _logger->error("USB: pipelined read submit failed ({})", rc); + if (!async_submit(w)) { + _logger->error("USB: pipelined read submit failed"); return false; } - w->inflight = true; - _aw->inflight++; while (!w->done) { if (!async_wait_progress()) { flush_writes(); /* cancels + recovers; w->done is set by the cancel */ @@ -519,7 +528,7 @@ void UsbTransport::flush_writes() { * Cancel what is still submitted so the caller's next synchronous * transfer is not queued behind it. */ _logger->error("USB: pipelined write drain timed out ({} in flight)", - _aw->inflight); + _aw->inflight.load()); for (auto *w : _aw_all) if (w->inflight) libusb_cancel_transfer(w->t); @@ -544,7 +553,7 @@ void UsbTransport::flush_writes() { _batch = false; _logger->error("USB: {} pipelined transfer(s) could not be reaped; " "their slots are retired for this session", - _aw->inflight); + _aw->inflight.load()); } return; } @@ -562,16 +571,31 @@ bool UsbTransport::async_write(uint16_t wvalue, uint16_t windex, std::memcpy(w->buf + LIBUSB_CONTROL_SETUP_SIZE, data, n); libusb_fill_control_transfer(w->t, _dev_handle, w->buf, async_write_cb, w, USB_TIMEOUT); - const int rc = libusb_submit_transfer(w->t); - if (rc != 0) { - _aw->free.push_back(w); - _aw->errors++; - _logger->error("USB: pipelined write submit failed ({})", rc); + if (!async_submit(w)) { + _logger->error("USB: pipelined write submit failed"); return false; } + return true; +} + +/* Accounts the slot as in flight BEFORE submitting it — the callback may run + * on another thread's event pump the moment libusb has it — and rolls the + * accounting back if libusb refuses the transfer. */ +bool UsbTransport::async_submit(AsyncWrite *w) { w->inflight = true; _aw->inflight++; - return true; + const int rc = libusb_submit_transfer(w->t); + if (rc == 0) + return true; + _aw->inflight--; + w->inflight = false; + _aw->errors++; + { + std::lock_guard lk(_aw->mu); + _aw->free.push_back(w); + } + _logger->error("USB: libusb_submit_transfer rc={}", rc); + return false; } bool UsbTransport::write_bytes(uint16_t reg_num, const uint8_t *ptr, size_t n) { diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 831f2f53..097aaddd 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -122,22 +122,31 @@ class UsbTransport final : public ITransport { * this block, which the leaked slot keeps alive, instead of a freed * UsbTransport. */ struct AsyncWrite; + /* The submitter and the completion callback are normally the same thread + * (this transport pumps its own events while it waits), but a second + * adapter sharing the libusb context can pump it from another thread and + * run the callback there, right after libusb_submit_transfer returns. So + * the counters are atomic, the free list is under a mutex, and a slot is + * marked in flight BEFORE it is submitted (rolled back if the submit is + * refused) — the callback never sees a completed slot that the submitter + * has not yet accounted for. */ struct AsyncPool { + std::mutex mu; /* guards `free` */ std::vector free; - int inflight = 0; - uint64_t completed = 0; - int errors = 0; + std::atomic inflight{0}; + std::atomic completed{0}; + std::atomic errors{0}; }; struct AsyncWrite { libusb_transfer *t; uint8_t buf[LIBUSB_CONTROL_SETUP_SIZE + kAsyncMaxPayload]; std::shared_ptr pool; - bool done; + std::atomic done{false}; /* Submitted and not yet reaped: libusb owns `t` and `buf` while set, so * the slot must not be reused, freed, or handed back to the free list. */ - bool inflight; - int status; - int actual; + std::atomic inflight{false}; + int status = -1; + int actual = 0; }; static constexpr int kAsyncWriteDepth = 8; /* Extra event-loop turns spent reaping cancellations after a drain times @@ -151,6 +160,7 @@ class UsbTransport final : public ITransport { * Returns false on failure (data untouched). */ bool async_read(uint16_t wvalue, uint16_t windex, void *data, size_t n); AsyncWrite *async_take_slot(); + bool async_submit(AsyncWrite *w); /* in-flight accounting before submit */ bool async_wait_progress(); /* one event-loop turn; false on timeout/error */ static void LIBUSB_CALL async_write_cb(libusb_transfer *t); bool _batch = false; diff --git a/src/jaguar3/HalJaguar3.cpp b/src/jaguar3/HalJaguar3.cpp index 95f50447..6ed79e99 100644 --- a/src/jaguar3/HalJaguar3.cpp +++ b/src/jaguar3/HalJaguar3.cpp @@ -602,6 +602,7 @@ uint8_t HalJaguar3::efuse_phys_read_8822e(uint16_t addr) { uint32_t v = _device.rtw_read32(EFC); v = (v & ~(kAddr | kData | kRdy)) | ((static_cast(addr) & 0x7ff) << 16); _device.rtw_write32(EFC, v); + _device.flush_writes(); /* the 50 µs settle counts from the trigger landing */ for (int i = 0; i < 1000; ++i) { std::this_thread::sleep_for(std::chrono::microseconds(50)); uint32_t t = _device.rtw_read32(EFC); diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 857a9336..ed87e7ce 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -810,6 +810,7 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { } _logger->error("CW tone arm: USB glitch ({}) — retry {}/3", ex.what(), attempt); + _device.flush_writes(); /* settle from a drained queue before retrying */ std::this_thread::sleep_for(std::chrono::milliseconds(150)); } } From bb8bbe361ff4407a45abc7e71138a5fba0d5051f Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:33:09 +0300 Subject: [PATCH 07/29] usb pipelining: publish a slot last; drain deadline in elapsed time; harness exit codes - async_write_cb finalizes the result and all accounting before the slot is pushed onto the free list, so a taker can never reset a slot the callback is still writing to. - async_wait_progress waits on a real 2 s steady_clock deadline for this pool's completion counter, not on a count of event-loop turns: on a shared libusb context another adapter's completions make every handle_events return at once, and counting turns would declare a healthy queue stuck and cancel it. - chipstate refuses to print a poke the chip did not take (exit 4), so a following peek cannot read as a verdict about bits never written. - j3_tx_flood_ab.sh exits non-zero after any unexpected txdemo exit, while still reporting every rep. Co-Authored-By: Claude Fable 5.1 --- examples/chipstate/main.cpp | 16 +++++++++++++--- src/UsbTransport.cpp | 32 +++++++++++++++++++++----------- tests/j3_tx_flood_ab.sh | 5 ++++- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/examples/chipstate/main.cpp b/examples/chipstate/main.cpp index 0b2d0372..427fcf93 100644 --- a/examples/chipstate/main.cpp +++ b/examples/chipstate/main.cpp @@ -163,12 +163,22 @@ int run_reg_ops(libusb_device_handle *handle, Logger_t logger, try { for (const RegOp &op : ops) { if (op.write) { + bool ok; if (op.width == 4) - adapter.rtw_write32(op.addr, op.val); + ok = adapter.rtw_write32(op.addr, op.val); else if (op.width == 2) - adapter.rtw_write16(op.addr, static_cast(op.val)); + ok = adapter.rtw_write16(op.addr, static_cast(op.val)); else - adapter.rtw_write8(op.addr, static_cast(op.val)); + ok = adapter.rtw_write8(op.addr, static_cast(op.val)); + if (!ok) { + /* A write the chip did not take must not print as a poke, or a + * following peek reads as a verdict about bits that were never + * written. */ + std::fflush(stdout); + logger->error("poke 0x{:04x} (width {}) FAILED — vendor-control " + "write rejected", op.addr, op.width); + return 4; + } std::printf("poke 0x%04x = 0x%0*x\n", op.addr, op.width * 2, op.val); } else if (op.width == 4) { for (uint32_t row = op.addr & ~0xfu; row <= op.end; row += 16) { diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 21bcb1cb..1ac3494b 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -422,24 +422,26 @@ void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { /* Only the shared pool is touched here — never the transport, which a * leaked slot can outlive (AsyncPool). */ AsyncPool &pool = *w->pool; + /* Order matters: the result and every piece of accounting are final + * before the slot becomes visible again. `done` is published after + * status/actual so a waiter that sees it sees the result; the free-list + * push comes last so a taker (always the batch's own thread) can never + * reset a slot this callback is still writing to. A reader that is + * waiting on this very slot copies its data out before it submits + * anything else, so the buffer is intact when it does. */ w->status = t->status; w->actual = t->actual_length; if (t->status != LIBUSB_TRANSFER_COMPLETED || t->actual_length != t->length - LIBUSB_CONTROL_SETUP_SIZE) pool.errors++; - /* The slot goes back on the free list here; a reader that is waiting on - * this very slot copies its data out before it submits anything else - * (single-threaded by contract), so the buffer is still intact. `done` - * is published last, after status/actual, so a waiter that sees it sees - * the result too. */ + w->inflight = false; + w->done = true; + pool.inflight--; + pool.completed++; { std::lock_guard lk(pool.mu); pool.free.push_back(w); } - w->inflight = false; - pool.inflight--; - w->done = true; - pool.completed++; } UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { @@ -499,8 +501,16 @@ bool UsbTransport::async_read(uint16_t wvalue, uint16_t windex, void *data, } bool UsbTransport::async_wait_progress() { + /* Pumps until THIS pool's completion counter advances, a real 2 s + * deadline passes, or the event loop errors. Elapsed time, not a turn + * count: on a shared libusb context another adapter's RX/TX completions + * make each handle_events return at once, and counting those turns would + * declare a healthy queue stuck and cancel it. */ const uint64_t before = _aw->completed; - for (int turns = 0; turns < 8 && _aw->completed == before; ++turns) { + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); + while (_aw->completed == before) { + if (std::chrono::steady_clock::now() >= deadline) + return false; struct timeval tv {0, 250 * 1000}; const int rc = libusb_handle_events_timeout_completed(_ctx, &tv, nullptr); if (rc < 0) { @@ -509,7 +519,7 @@ bool UsbTransport::async_wait_progress() { return false; } } - return _aw->completed != before; + return true; } void UsbTransport::flush_writes() { diff --git a/tests/j3_tx_flood_ab.sh b/tests/j3_tx_flood_ab.sh index d86fe0e7..c2950a0a 100755 --- a/tests/j3_tx_flood_ab.sh +++ b/tests/j3_tx_flood_ab.sh @@ -15,7 +15,7 @@ run() { env DEVOURER_PID="$PID" DEVOURER_CHANNEL="$CH" DEVOURER_LOG_LEVEL=info \ timeout -s INT "$SECS" "$tree/build/txdemo" >"$log.jsonl" 2>"$log.err" || rc=$? # 124 = timeout fired (the normal end), 0/130 = txdemo took the INT itself. - case $rc in 0|124|130) ;; *) echo "$n rep$rep: txdemo exited rc=$rc"; tail -3 "$log.err";; esac + case $rc in 0|124|130) ;; *) echo "$n rep$rep: txdemo exited rc=$rc"; tail -3 "$log.err"; failed=1;; esac local tx fail rd first tx=$(grep -cF '"ev":"tx.' "$log.jsonl" || true) fail=$(grep -c 'bulk_send EP .* FAIL' "$log.err" || true) @@ -23,4 +23,7 @@ run() { first=$(grep -oE 'first_tx_submit","ms":[0-9]+' "$log.jsonl" | grep -oE '[0-9]+$' || echo '?') echo "$n rep$rep: first_tx_ms=$first tx_events=$tx bulk_fail=$fail read_fail=$rd" } +failed=0 for r in $(seq 1 "$REPS"); do run "$A" "$r"; run "$B" "$r"; done +# Every rep is still reported, but a crashed or failed run fails the script. +exit "$failed" From 04202d716f77a219ad732ce1d366e677c1c4e01e Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:44:25 +0300 Subject: [PATCH 08/29] usb pipelining: the destructor waits for a callback still inside a slot; chipstate rejects --no-claim with --init - Each slot carries cb_busy: set on callback entry, cleared as its very last store. `inflight` has to clear before the free-list push so a taker sees a finished slot, so it cannot double as the destructor's "safe to free" signal; the destructor now waits out a callback that another adapter's pump thread is still running before freeing. - chipstate refuses --no-claim together with --init: the raw-adapter path cannot bring the chip up, so the combination would have run the ops on an uninitialised device while looking like it asked for a bring-up. Co-Authored-By: Claude Fable 5.1 --- examples/chipstate/main.cpp | 8 +++++--- src/UsbTransport.cpp | 7 +++++++ src/UsbTransport.h | 5 +++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/examples/chipstate/main.cpp b/examples/chipstate/main.cpp index 427fcf93..04678b2b 100644 --- a/examples/chipstate/main.cpp +++ b/examples/chipstate/main.cpp @@ -286,9 +286,11 @@ int main(int argc, char **argv) { * need the interface, so peeks/pokes work while another process (a live * armed rxdemo) owns it — the concurrent-intervention mode. */ if (a.no_claim) { - if (a.ops.empty()) { - logger->error("--no-claim is peek/poke-only (the canary dump needs the " - "claimed device)"); + if (a.ops.empty() || a.init) { + logger->error(a.init ? "--init needs the claimed-device path; drop " + "--no-claim" + : "--no-claim is peek/poke-only (the canary dump " + "needs the claimed device)"); session.adopt_handle(handle); return 2; } diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 1ac3494b..b9d70e94 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -339,6 +339,11 @@ UsbTransport::~UsbTransport() { flush_writes(); int leaked = 0; for (auto *w : _aw_all) { + /* A callback still inside the slot (another adapter's pump thread + * reaping it right now) finishes in a handful of stores; wait it out + * rather than free under it. */ + while (w->cb_busy) + std::this_thread::yield(); /* libusb forbids freeing an active transfer, and its callback still * writes through `w`. If the drain above could not reap it (dead event * loop / yanked device), leaking the slot is the lesser evil: a callback @@ -422,6 +427,7 @@ void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { /* Only the shared pool is touched here — never the transport, which a * leaked slot can outlive (AsyncPool). */ AsyncPool &pool = *w->pool; + w->cb_busy = true; /* Order matters: the result and every piece of accounting are final * before the slot becomes visible again. `done` is published after * status/actual so a waiter that sees it sees the result; the free-list @@ -442,6 +448,7 @@ void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { std::lock_guard lk(pool.mu); pool.free.push_back(w); } + w->cb_busy = false; /* last: the destructor may free the slot after this */ } UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 097aaddd..077e8950 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -145,6 +145,11 @@ class UsbTransport final : public ITransport { /* Submitted and not yet reaped: libusb owns `t` and `buf` while set, so * the slot must not be reused, freed, or handed back to the free list. */ std::atomic inflight{false}; + /* The completion callback is inside the slot: set first thing on entry, + * cleared as its very last store. `inflight` has to clear before the + * free-list push (a taker must see a finished slot), so it cannot double + * as the destructor's "safe to free" signal — this is. */ + std::atomic cb_busy{false}; int status = -1; int actual = 0; }; From eca3a8abe19cde26b066646124bf9d259221a4f5 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:55:03 +0300 Subject: [PATCH 09/29] usb pipelining: a failed completion fails the bring-up; scope the timing claims - write_batch_end returns whether every queued write completed (failed and short completions are only known after the fact, when the batch drains), through ITransport, RtlAdapter and WriteBatchScope::end(). Jaguar3 InitWrite throws at that close rather than start the coex thread over a chip with a register unprogrammed. The unwinding close in the scope destructor drops the result by design. - The timing figures in the ITransport and RF-table comments now carry their scope (one 8812EU on one ssc338q host) and point at the Jaguar3 guide for the bench numbers and their limits. - src/jaguar3/CLAUDE.md refers to src/InitTimer.h / docs/logging.md for the init.timing schema instead of restating it. - tests/j3_rf_window_readback.sh fails on the histogram leg if any window word holds bits above 19, and only then runs the write-back leg. Co-Authored-By: Claude Fable 5.1 --- src/RtlAdapter.h | 2 +- src/Transport.h | 21 +++++++++++++-------- src/UsbTransport.cpp | 13 ++++++++++--- src/UsbTransport.h | 2 +- src/jaguar3/CLAUDE.md | 19 +++++++++++-------- src/jaguar3/HalJaguar3.cpp | 7 ++++--- src/jaguar3/RtlJaguar3Device.cpp | 18 ++++++++++++++---- tests/j3_rf_window_readback.sh | 8 +++++++- 8 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/RtlAdapter.h b/src/RtlAdapter.h index 45e0ef29..e6cd7428 100644 --- a/src/RtlAdapter.h +++ b/src/RtlAdapter.h @@ -90,7 +90,7 @@ class RtlAdapter { void hci_setup() { _transport->hci_setup(); } /* Pipelined register writes — see ITransport::write_batch_begin. */ void write_batch_begin() { _transport->write_batch_begin(); } - void write_batch_end() { _transport->write_batch_end(); } + bool write_batch_end() { return _transport->write_batch_end(); } void flush_writes() { _transport->flush_writes(); } uint64_t ctrl_xfers() const { return _transport->ctrl_xfers(); } diff --git a/src/Transport.h b/src/Transport.h index b54d46ec..4e1c451f 100644 --- a/src/Transport.h +++ b/src/Transport.h @@ -71,15 +71,20 @@ class ITransport { /* ---- pipelined register writes ---- * Inside a write batch, register writes are submitted as asynchronous * in-order transfers and only a read (or a bulk transfer, or flush_writes) - * waits for them. Bring-up is ~14k synchronous EP0 round trips at ~80 us - * each on an embedded host; pipelined, a write costs ~27 us (measured, - * ssc338q + RTL8812EU, depth >= 8). Correctness rests on EP0 completing - * URBs in submission order, so a read that follows a write still sees it. - * Single-threaded by contract: open a batch only while no other thread - * touches the transport (the Jaguar3 InitWrite/Init bring-up), and close - * it before any worker thread starts. Defaults are no-ops (PCIe). */ + * waits for them. The Jaguar3 bring-up is ~14k EP0 round trips; on the + * one unit and host measured (an RTL8812EU on an ssc338q) a synchronous + * write cost ~80 us and a pipelined one ~27 us at depth >= 8 — one device, + * one host, so a scale rather than a number to plan by; the measured + * bring-up figures and their limits are in src/jaguar3/CLAUDE.md. + * Correctness rests on EP0 completing URBs in submission order, so a read + * that follows a write still sees it. Single-threaded by contract: open a + * batch only while no other thread touches the transport (the Jaguar3 + * InitWrite/Init bring-up), and close it before any worker thread starts. + * write_batch_end drains and reports whether every queued write completed + * (a failed or short completion is only known after the fact); the caller + * decides what an incomplete batch means. Defaults are no-ops (PCIe). */ virtual void write_batch_begin() {} - virtual void write_batch_end() {} + virtual bool write_batch_end() { return true; } virtual void flush_writes() {} /* Register transfers (reads + writes) this transport instance has issued * so far — the unit a USB bring-up is paid in. InitTimer differences it diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index b9d70e94..0c0ea3b7 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -414,12 +414,19 @@ void UsbTransport::write_batch_begin() { _batch = true; } -void UsbTransport::write_batch_end() { +bool UsbTransport::write_batch_end() { + if (!_batch) + return true; flush_writes(); - if (_batch && _aw->errors.load()) + /* Failed and short completions are only known here, after the fact: a + * write reported true at submission. The count covers submit refusals, + * completion failures and retired slots alike. */ + const int errors = _aw->errors.load(); + if (errors) _logger->error("USB: {} pipelined register write(s) failed in this batch", - _aw->errors.load()); + errors); _batch = false; + return errors == 0; } void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 077e8950..2fd6e421 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -88,7 +88,7 @@ class UsbTransport final : public ITransport { } bool write_bytes(uint16_t reg, const uint8_t *p, size_t n) override; void write_batch_begin() override; - void write_batch_end() override; + bool write_batch_end() override; void flush_writes() override; uint64_t ctrl_xfers() const override { return _ctrl_xfers.load(std::memory_order_relaxed); diff --git a/src/jaguar3/CLAUDE.md b/src/jaguar3/CLAUDE.md index d02453c6..60b3898b 100644 --- a/src/jaguar3/CLAUDE.md +++ b/src/jaguar3/CLAUDE.md @@ -39,17 +39,20 @@ narrowband dividers, RF18 encoding), strategy interfaces `Jaguar3Calibration` ## Bring-up cost and the pipelined register writes `InitWrite` is ~14k USB register transfers and nothing else. The stage -timing (`InitTimer` events `j3hal.*` / `j3init.*`, each carrying `ms` and -the `xfers` it spent; `bench_init.py` parses these events but reports only -`ms`) is what shows it. The batching contract itself — ordering, what -waits, single-threadedness — is documented once, at -`ITransport::write_batch_begin` (`src/Transport.h`) and in `UsbTransport`; -this file carries only how Jaguar3 uses it: +timing shows it: `init.timing` events under the `j3hal.*` (HAL bring-up) +and `j3init.*` (`InitWrite`) scopes, field schema in `src/InitTimer.h` / +`docs/logging.md`; `bench_init.py` parses them but reports only `ms`. The +batching contract itself — ordering, what waits, single-threadedness, +failure propagation — is documented once, at `ITransport::write_batch_begin` +(`src/Transport.h`) and in `UsbTransport`; this file carries only how +Jaguar3 uses it: - `InitWrite` runs its whole bring-up inside one `WriteBatchScope` (`RtlJaguar3Device.cpp`), ended before the coex thread starts because that - thread shares the transport. `Init` (RX-only) opens no batch yet — not - measured on a ground-station card. + thread shares the transport. A queued write that completed failed or + short fails the batch close, and `InitWrite` throws there rather than + start the coex thread over an incompletely programmed chip. `Init` + (RX-only) opens no batch yet — not measured on a ground-station card. - Every settle delay drains the queue first, µs ones included, on both dies: the `write_bb` / `rf_writer` table delay markers, `delay_us` and `delay_ms` on `Halrf8822c` and `Halrf8822e`, the efuse power-cut. A settle diff --git a/src/jaguar3/HalJaguar3.cpp b/src/jaguar3/HalJaguar3.cpp index 6ed79e99..ea431823 100644 --- a/src/jaguar3/HalJaguar3.cpp +++ b/src/jaguar3/HalJaguar3.cpp @@ -1032,9 +1032,10 @@ void HalJaguar3::apply_bb_rf_agc_tables(InitTimer *timer) { * word of both path windows after a bring-up, on an 8812EU (cold * and warm) and on an 8812CU (tests/j3_rf_window_readback.sh), so * an RMW that reads 0 writes exactly `data` and preserving those - * bits is a no-op that cost a synchronous read per entry -- half - * the RF table stage, ~80 ms on the ssc338q. Write-only also - * pipelines (ITransport::write_batch_begin). */ + * bits is a no-op that cost a synchronous read per entry -- about + * half the RF table stage (~80 ms on the one 8812EU + ssc338q host + * measured; the x86 bench figures are in src/jaguar3/CLAUDE.md). + * Write-only also pipelines (ITransport::write_batch_begin). */ _device.rtw_write32(static_cast(base + ((addr & 0xff) << 2)), data & RFREG_MASK); } diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index ed87e7ce..dbb639b6 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -64,11 +64,15 @@ struct WriteBatchScope { bool open = true; explicit WriteBatchScope(RtlAdapter &d) : dev(d) { dev.write_batch_begin(); } /* Closes once: after end() the destructor is a no-op, so it never touches - * the transport again once the coex thread (which shares it) is running. */ - void end() { + * the transport again once the coex thread (which shares it) is running. + * Returns whether every queued write completed; the destructor's close + * (unwinding) drops that result, an explicit end() must act on it. */ + bool end() { + bool ok = true; if (open) - dev.write_batch_end(); + ok = dev.write_batch_end(); open = false; + return ok; } ~WriteBatchScope() { end(); } }; @@ -968,7 +972,13 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { _device.rtw_read32(a + 8), _device.rtw_read32(a + 12)); } timer.stage("dpdt_ack_misc"); - batch.end(); /* sync writes from here: the coex thread shares the transport */ + /* Sync writes from here: the coex thread shares the transport. A queued + * write that completed failed or short is only known at this close, and + * a chip with one register unprogrammed is not one to start the coex + * thread over and announce ready. */ + if (!batch.end()) + throw std::runtime_error( + "Jaguar3: pipelined register write(s) failed during bring-up"); /* The timing closes here, before the coex thread starts: it shares the * adapter's transfer counter, so anything emitted after it would count * that thread's register and H2C traffic as bring-up. */ diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh index ca7e3287..3454ba84 100755 --- a/tests/j3_rf_window_readback.sh +++ b/tests/j3_rf_window_readback.sh @@ -17,7 +17,9 @@ # 0 the RMW could never have preserved anything. This leg is what # decides, and it does not depend on which bring-up ran first. # -# Exit 0 only when both legs say the bits are not storage. +# Exit 0 only when both legs say the bits are not storage: a non-zero +# histogram fails leg 1 outright (the write-back leg is not run), and any +# poked word reading back with its high bits set fails leg 2. # # sudo tests/j3_rf_window_readback.sh 0xc812 # 8812CU # sudo tests/j3_rf_window_readback.sh 0xa81a # 8812EU @@ -44,6 +46,10 @@ if not words: print("FAIL: no register rows parsed", file=sys.stderr); sys.exit(1) hi = collections.Counter(w >> 20 for w in words.values()) print(f"leg1 pid={sys.argv[2]} words={len(words)} hi12_histogram={dict(sorted(hi.items()))}", file=sys.stderr) +if set(hi) != {0}: + print("FAIL leg1: some window words hold bits above 19 after the bring-up — " + "contradicts the no-storage premise; the write-back leg is not run", file=sys.stderr) + sys.exit(1) n = int(sys.argv[3]); ops = [] for base in (0x3c00, 0x4c00): for a in range(base, base + 4 * n, 4): From 557508124f2b713e131664b4915f3fa92fc0ee91 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:06:24 +0300 Subject: [PATCH 10/29] usb pipelining: reads do not fail the batch; one post-cancel deadline; last-chance reap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Each slot records whether it carries a read or a write. Only failed or short WRITE completions, write submit refusals and retired slots count toward the batch verdict; a failed read is reported to its caller (false / throw) and nothing else, so a read glitch the caller retries and recovers no longer poisons write_batch_end. - The post-cancel reap runs against one absolute 2 s deadline in short pump turns, instead of a fresh 2 s wait per turn — an eight-deep dead queue no longer holds the bring-up for most of 20 s before it is retired. - The destructor makes one more cancel + bounded reap while the handle and context are still valid, even for a queue retired earlier; only what is still submitted after that is leaked, deliberately, over freeing a transfer libusb owns. - InitWrite's CW-tone early return closes the batch explicitly and throws on a failed completion, like the main path. - tests/j3_rf_window_readback.sh restores every sampled window word from an EXIT trap, so a failure after the first high-bit poke cannot leave the RF registers modified. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 61 ++++++++++++++++++++++++-------- src/UsbTransport.h | 14 ++++---- src/jaguar3/RtlJaguar3Device.cpp | 6 ++++ tests/j3_rf_window_readback.sh | 15 ++++++-- 4 files changed, 73 insertions(+), 23 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 0c0ea3b7..d3bd4cc8 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -337,6 +337,21 @@ UsbTransport::UsbTransport(libusb_device_handle *dev_handle, Logger_t logger, UsbTransport::~UsbTransport() { 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)) + break; + } int leaked = 0; for (auto *w : _aw_all) { /* A callback still inside the slot (another adapter's pump thread @@ -410,7 +425,7 @@ void UsbTransport::write_batch_begin() { std::lock_guard lk(_aw->mu); _aw->free = slots; } - _aw->errors = 0; + _aw->write_errors = 0; _batch = true; } @@ -421,7 +436,7 @@ bool UsbTransport::write_batch_end() { /* Failed and short completions are only known here, after the fact: a * write reported true at submission. The count covers submit refusals, * completion failures and retired slots alike. */ - const int errors = _aw->errors.load(); + const int errors = _aw->write_errors.load(); if (errors) _logger->error("USB: {} pipelined register write(s) failed in this batch", errors); @@ -444,9 +459,9 @@ void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { * anything else, so the buffer is intact when it does. */ w->status = t->status; w->actual = t->actual_length; - if (t->status != LIBUSB_TRANSFER_COMPLETED || - t->actual_length != t->length - LIBUSB_CONTROL_SETUP_SIZE) - pool.errors++; + if (!w->is_read && (t->status != LIBUSB_TRANSFER_COMPLETED || + t->actual_length != t->length - LIBUSB_CONTROL_SETUP_SIZE)) + pool.write_errors++; w->inflight = false; w->done = true; pool.inflight--; @@ -493,6 +508,7 @@ bool UsbTransport::async_read(uint16_t wvalue, uint16_t windex, void *data, AsyncWrite *w = async_take_slot(); if (!w) return false; + w->is_read = true; libusb_fill_control_setup(w->buf, REALTEK_USB_VENQT_READ, 5, wvalue, windex, static_cast(n)); libusb_fill_control_transfer(w->t, _dev_handle, w->buf, async_write_cb, w, @@ -525,13 +541,19 @@ bool UsbTransport::async_wait_progress() { while (_aw->completed == before) { if (std::chrono::steady_clock::now() >= deadline) return false; - struct timeval tv {0, 250 * 1000}; - const int rc = libusb_handle_events_timeout_completed(_ctx, &tv, nullptr); - if (rc < 0) { - _logger->error("USB: event loop error {} while draining pipelined writes", - rc); + if (!pump_once(250)) return false; - } + } + return true; +} + +bool UsbTransport::pump_once(int ms) { + struct timeval tv {0, ms * 1000}; + const int rc = libusb_handle_events_timeout_completed(_ctx, &tv, nullptr); + if (rc < 0) { + _logger->error("USB: event loop error {} while draining pipelined writes", + rc); + return false; } return true; } @@ -561,8 +583,15 @@ void UsbTransport::flush_writes() { * must never be zeroed by hand, or a late callback decrements it below * zero (silently disabling every later drain), pushes its slot onto the * free list a second time, and races the destructor's free. */ - for (int i = 0; i < kFlushCancelTurns && _aw->inflight > 0; ++i) - async_wait_progress(); + /* One absolute deadline for the whole post-cancel phase, pumped in + * short turns — not a fresh 2 s wait per slot, which would let an + * eight-deep dead queue hold the bring-up for the better part of 20 s. */ + const auto cancel_deadline = + std::chrono::steady_clock::now() + std::chrono::seconds(2); + while (_aw->inflight > 0 && + std::chrono::steady_clock::now() < cancel_deadline) + if (!pump_once(100)) + break; if (_aw->inflight > 0) { /* The event loop itself is gone (a yanked device reports the error * immediately, so the turns above cost nothing). Leave the slots @@ -572,7 +601,7 @@ void UsbTransport::flush_writes() { * register access would otherwise walk take-slot -> wait -> drain * (seconds each) for the rest of the bring-up. Synchronous from * here on. */ - _aw->errors += _aw->inflight; + _aw->write_errors += _aw->inflight; /* reads among them fail their callers too */ _aw_abandoned = true; _batch = false; _logger->error("USB: {} pipelined transfer(s) could not be reaped; " @@ -590,6 +619,7 @@ bool UsbTransport::async_write(uint16_t wvalue, uint16_t windex, AsyncWrite *w = async_take_slot(); if (!w) return false; + w->is_read = false; libusb_fill_control_setup(w->buf, REALTEK_USB_VENQT_WRITE, 5, wvalue, windex, static_cast(n)); std::memcpy(w->buf + LIBUSB_CONTROL_SETUP_SIZE, data, n); @@ -613,7 +643,8 @@ bool UsbTransport::async_submit(AsyncWrite *w) { return true; _aw->inflight--; w->inflight = false; - _aw->errors++; + if (!w->is_read) + _aw->write_errors++; { std::lock_guard lk(_aw->mu); _aw->free.push_back(w); diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 2fd6e421..540a2fc0 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -135,7 +135,11 @@ class UsbTransport final : public ITransport { std::vector free; std::atomic inflight{0}; std::atomic completed{0}; - std::atomic errors{0}; + /* Failed/short WRITE completions, write submit refusals and retired + * slots: what write_batch_end reports. A failed read is reported to its + * caller directly (false / throw) and is NOT counted here — a read + * glitch that the caller retries and recovers must not fail the batch. */ + std::atomic write_errors{0}; }; struct AsyncWrite { libusb_transfer *t; @@ -150,14 +154,11 @@ class UsbTransport final : public ITransport { * free-list push (a taker must see a finished slot), so it cannot double * as the destructor's "safe to free" signal — this is. */ std::atomic cb_busy{false}; + bool is_read = false; /* set before submit; decides which failure it is */ int status = -1; int actual = 0; }; static constexpr int kAsyncWriteDepth = 8; - /* Extra event-loop turns spent reaping cancellations after a drain times - * out. A live loop reports each cancellation promptly; a dead one fails - * every turn immediately, so this costs nothing in the case that matters. */ - static constexpr int kFlushCancelTurns = 8; bool async_write(uint16_t wvalue, uint16_t windex, const void *data, size_t n); /* Read queued behind the pending writes (EP0 order) and waited for on its @@ -166,7 +167,8 @@ class UsbTransport final : public ITransport { bool async_read(uint16_t wvalue, uint16_t windex, void *data, size_t n); AsyncWrite *async_take_slot(); bool async_submit(AsyncWrite *w); /* in-flight accounting before submit */ - bool async_wait_progress(); /* one event-loop turn; false on timeout/error */ + bool async_wait_progress(); /* pump until this pool progresses; false on a 2 s deadline/error */ + bool pump_once(int ms); /* one bounded handle_events turn; false on error */ static void LIBUSB_CALL async_write_cb(libusb_transfer *t); bool _batch = false; std::shared_ptr _aw = std::make_shared(); diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index dbb639b6..d3c84e2b 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -822,6 +822,12 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { _device.rtw_write(0x0040, v40 | 0x14030008u); _device.rtw_write(0x0064, v64 & ~0x02040000u); } + /* This return leaves InitWrite early: close the batch here so a + * failed completion during the CW arm fails the call, instead of the + * scope destructor draining it and dropping the verdict. */ + if (!batch.end()) + throw std::runtime_error( + "Jaguar3: pipelined register write(s) failed during CW-tone arm"); _logger->info("Jaguar3: CW tone hold (minimal bring-up, no coex thread)"); return; } diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh index 3454ba84..8626b1d8 100755 --- a/tests/j3_rf_window_readback.sh +++ b/tests/j3_rf_window_readback.sh @@ -50,14 +50,25 @@ if set(hi) != {0}: print("FAIL leg1: some window words hold bits above 19 after the bring-up — " "contradicts the no-storage premise; the write-back leg is not run", file=sys.stderr) sys.exit(1) -n = int(sys.argv[3]); ops = [] +n = int(sys.argv[3]); ops = []; restore = [] for base in (0x3c00, 0x4c00): for a in range(base, base + 4 * n, 4): v = words[a] ops += [f"--poke 0x{a:04x}=0x{(v | 0xFFF00000):08x}:4", f"--peek 0x{a:04x}-0x{a+3:04x}:4", f"--poke 0x{a:04x}=0x{v:08x}:4"] -print(" ".join(ops)) + restore.append(f"--poke 0x{a:04x}=0x{v:08x}:4") +print(" ".join(ops)); print(" ".join(restore)) PY ) || exit 1 +restore_ops=$(tail -n1 <<<"$ops"); ops=$(head -n1 <<<"$ops") +# Whatever happens once the first high-bit poke is out, put every sampled +# word back to its dump value. The chip stays configured after chipstate +# exits (its device teardown does not de-init), so a plain attach suffices. +restore() { + # shellcheck disable=SC2086 + "$CS" --pid "$PID" $restore_ops >"$OUT/pid${PID}.restore" 2>&1 \ + || echo "WARN: restore pass failed — sampled window words may be left modified (see $OUT/pid${PID}.restore)" >&2 +} +trap restore EXIT wb=$OUT/pid${PID}.writeback # shellcheck disable=SC2086 "$CS" --pid "$PID" --init $ops >"$wb" 2>"$wb.err" || { echo "FAIL: chipstate exited non-zero (leg 2)"; tail -5 "$wb.err"; exit 1; } From fc198f52094f5bf05140e2a4ae726842151a035b Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:16:18 +0300 Subject: [PATCH 11/29] usb pipelining: the batch verdict survives abandonment; readiness rolls back on a failed close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `_batch_open` (the caller's batch) is tracked apart from `_batch` (pipelined submission enabled). A drain that retires slots turns pipelining off but leaves the batch open, so write_batch_end still returns the errors it recorded — the case with the most to report. - A refused async submit is not a batch write error: the caller retries the write synchronously, in order, and only a fallback that also fails counts. - InitWrite's readiness flag is provisional until the batch closes clean: a guard clears it on any throw between setting it and the close, on the main path and the CW-tone one. - InitTimer reports its total on scope exit if total() was not called, so an early return or a throw still carries its cost; a scope that called total() emits exactly one. - tests/j3_tx_flood_ab.sh reports frames submitted from the final tx.stats tally (a run without one fails) and matches the first-TX stage by its full key. Co-Authored-By: Claude Fable 5.1 --- src/InitTimer.h | 8 ++++++++ src/UsbTransport.cpp | 19 ++++++++++++------- src/UsbTransport.h | 23 +++++++++++++++++++---- src/jaguar3/RtlJaguar3Device.cpp | 13 +++++++++++++ 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/InitTimer.h b/src/InitTimer.h index 49e13a48..79701b4d 100644 --- a/src/InitTimer.h +++ b/src/InitTimer.h @@ -42,10 +42,17 @@ class InitTimer { _x_last = x; } + /* Reports the total once. An early return or a throw out of the timed + * scope still gets its total from the destructor, so a failed bring-up + * carries its cost too; a scope that called total() emits exactly one. */ void total() { + if (_finalized) + return; + _finalized = true; emit("total", ms(_start, clock::now()), static_cast(count() - _x_start)); } + ~InitTimer() { total(); } private: uint64_t count() const { return _xfers ? _xfers() : 0; } @@ -71,6 +78,7 @@ class InitTimer { clock::time_point _last; uint64_t _x_start; uint64_t _x_last; + bool _finalized = false; }; #endif /* INIT_TIMER_H */ diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index d3bd4cc8..e6302738 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -394,7 +394,7 @@ UsbTransport::~UsbTransport() { * followed by a read behaves exactly like the synchronous sequence; the win * is that the host does not sit through a full URB round trip per write. */ void UsbTransport::write_batch_begin() { - if (_batch) + if (_batch_open) return; /* A session that already failed to reap its transfers has a short pool and * a suspect event loop; stay synchronous rather than pipeline into it. */ @@ -427,20 +427,24 @@ void UsbTransport::write_batch_begin() { } _aw->write_errors = 0; _batch = true; + _batch_open = true; } bool UsbTransport::write_batch_end() { - if (!_batch) + if (!_batch_open) return true; flush_writes(); /* Failed and short completions are only known here, after the fact: a - * write reported true at submission. The count covers submit refusals, - * completion failures and retired slots alike. */ + * write reported true at submission. The count covers failed/short + * write completions, failed synchronous fallbacks and retired slots — + * kept even when a drain already disabled pipelining (`_batch`), which is + * exactly the case with the most to report. */ const int errors = _aw->write_errors.load(); if (errors) _logger->error("USB: {} pipelined register write(s) failed in this batch", errors); _batch = false; + _batch_open = false; return errors == 0; } @@ -603,7 +607,8 @@ void UsbTransport::flush_writes() { * here on. */ _aw->write_errors += _aw->inflight; /* reads among them fail their callers too */ _aw_abandoned = true; - _batch = false; + _batch = false; /* pipelining off; the caller's batch stays open so + * write_batch_end still returns this verdict */ _logger->error("USB: {} pipelined transfer(s) could not be reaped; " "their slots are retired for this session", _aw->inflight.load()); @@ -643,8 +648,8 @@ bool UsbTransport::async_submit(AsyncWrite *w) { return true; _aw->inflight--; w->inflight = false; - if (!w->is_read) - _aw->write_errors++; + /* Not a batch write error yet: the caller retries a refused write + * synchronously and counts it only if that fails too. */ { std::lock_guard lk(_aw->mu); _aw->free.push_back(w); diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 540a2fc0..910ca6de 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -59,11 +59,17 @@ class UsbTransport final : public ITransport { if (_batch && async_write(static_cast(addr & 0xFFFF), static_cast(addr >> 16), &v, sizeof(v))) return true; - return libusb_control_transfer( + const bool ok = + libusb_control_transfer( _dev_handle, REALTEK_USB_VENQT_WRITE, 5, static_cast(addr & 0xFFFF), static_cast(addr >> 16), (uint8_t *)&v, sizeof(v), USB_TIMEOUT) == static_cast(sizeof(v)); + /* Only a fallback that ALSO failed is a batch write error; a refused + * async submit that the synchronous path completed is not. */ + if (!ok && _batch_open) + _aw->write_errors++; + return ok; } uint32_t read32_wide(uint32_t addr) override { uint32_t data = 0; @@ -170,7 +176,12 @@ class UsbTransport final : public ITransport { bool async_wait_progress(); /* pump until this pool progresses; false on a 2 s deadline/error */ bool pump_once(int ms); /* one bounded handle_events turn; false on error */ static void LIBUSB_CALL async_write_cb(libusb_transfer *t); + /* `_batch`: pipelined submission is enabled right now. `_batch_open`: the + * caller's batch is open. They part when a drain retires slots — that + * disables pipelining at once but must keep the batch's verdict (its + * write errors) for write_batch_end to return. */ bool _batch = false; + bool _batch_open = false; std::shared_ptr _aw = std::make_shared(); std::vector _aw_all; /* Set when a drain gave up with transfers still submitted: the destructor @@ -275,9 +286,13 @@ template bool UsbTransport::ctrl_write(uint16_t reg_num, T value) { /* Unsubmittable pipelined write -> synchronous, in order (see write32_wide). */ if (_batch && async_write(reg_num, 0, &value, sizeof(T))) return true; - return libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_WRITE, 5, - reg_num, 0, (uint8_t *)&value, sizeof(T), - USB_TIMEOUT) == sizeof(T); + const bool ok = + libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_WRITE, 5, reg_num, + 0, (uint8_t *)&value, sizeof(T), + USB_TIMEOUT) == sizeof(T); + if (!ok && _batch_open) + _aw->write_errors++; /* the fallback failed too (see write32_wide) */ + return ok; } } /* namespace devourer */ diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index d3c84e2b..87085223 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -828,6 +828,7 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { if (!batch.end()) throw std::runtime_error( "Jaguar3: pipelined register write(s) failed during CW-tone arm"); + brought_up_guard.committed = true; _logger->info("Jaguar3: CW tone hold (minimal bring-up, no coex thread)"); return; } @@ -840,6 +841,17 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * intermediate bring-up steps on sane references. */ apply_tx_power_current(/*full=*/true); timer.stage("txpower_pre"); + /* Readiness is provisional until the batch closes clean: a throw from + * anywhere below (a failed queued write is only known at the close) + * must not leave the runtime APIs believing the chip is programmed. */ + struct BroughtUpGuard { + bool &flag; + bool committed = false; + ~BroughtUpGuard() { + if (!committed) + flag = false; + } + } brought_up_guard{_brought_up}; _brought_up = true; /* WiFi-only coex bring-up: disable the BT/LTE antenna arbitration and lock the * antenna to WLAN so on-air TX is not killed by the coex firmware. */ @@ -985,6 +997,7 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { if (!batch.end()) throw std::runtime_error( "Jaguar3: pipelined register write(s) failed during bring-up"); + brought_up_guard.committed = true; /* The timing closes here, before the coex thread starts: it shares the * adapter's transfer counter, so anything emitted after it would count * that thread's register and H2C traffic as bring-up. */ From e00b65a555161798b51f72dd0e9d580bda6f95bd Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:18:01 +0300 Subject: [PATCH 12/29] usb pipelining: fix the CW-tone path build; flood A/B reports the submitted tally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CW-tone hold returns before the readiness flag is ever set, so it has no guard to commit — the previous commit referenced one there and did not build. tests/j3_tx_flood_ab.sh now reports frames submitted from the final tx.stats tally (a run without one fails) and matches the first-TX stage by its full key, as the previous commit message already claimed. Co-Authored-By: Claude Fable 5.1 --- src/jaguar3/RtlJaguar3Device.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 87085223..1971e8db 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -828,7 +828,6 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { if (!batch.end()) throw std::runtime_error( "Jaguar3: pipelined register write(s) failed during CW-tone arm"); - brought_up_guard.committed = true; _logger->info("Jaguar3: CW tone hold (minimal bring-up, no coex thread)"); return; } From f9cf73bd87da9d809219e6f0bc3d20d08a5e69dd Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:19:07 +0300 Subject: [PATCH 13/29] tests: j3_tx_flood_ab.sh reports the submitted tally and matches the first-TX stage by key Frames submitted come from the final tx.stats event ("final":1); a run without one fails. The first-TX time matches the full "stage":"txdemo.first_tx_submit" key, and a missing event fails the run. The two previous commits claimed this change; their edit had not applied. Co-Authored-By: Claude Fable 5.1 --- tests/j3_tx_flood_ab.sh | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/j3_tx_flood_ab.sh b/tests/j3_tx_flood_ab.sh index c2950a0a..af37c30c 100755 --- a/tests/j3_tx_flood_ab.sh +++ b/tests/j3_tx_flood_ab.sh @@ -16,12 +16,18 @@ run() { timeout -s INT "$SECS" "$tree/build/txdemo" >"$log.jsonl" 2>"$log.err" || rc=$? # 124 = timeout fired (the normal end), 0/130 = txdemo took the INT itself. case $rc in 0|124|130) ;; *) echo "$n rep$rep: txdemo exited rc=$rc"; tail -3 "$log.err"; failed=1;; esac - local tx fail rd first - tx=$(grep -cF '"ev":"tx.' "$log.jsonl" || true) + local submitted fail rd first + # Submitted frames: the final tx.stats event ("final":1) carries the + # cumulative tally; a run without one did not finish and is a failure. + submitted=$(grep -F '"ev":"tx.stats"' "$log.jsonl" | grep -F '"final":1' | tail -n1 \ + | grep -oE '"submitted":[0-9]+' | grep -oE '[0-9]+$' || true) + [ -n "$submitted" ] || { submitted='?'; echo "$n rep$rep: no final tx.stats event"; failed=1; } fail=$(grep -c 'bulk_send EP .* FAIL' "$log.err" || true) rd=$(grep -c 'rtw_read(' "$log.err" || true) - first=$(grep -oE 'first_tx_submit","ms":[0-9]+' "$log.jsonl" | grep -oE '[0-9]+$' || echo '?') - echo "$n rep$rep: first_tx_ms=$first tx_events=$tx bulk_fail=$fail read_fail=$rd" + first=$(grep -F '"ev":"init.timing"' "$log.jsonl" | grep -F '"stage":"txdemo.first_tx_submit"' \ + | grep -oE '"ms":[0-9]+' | grep -oE '[0-9]+$' | head -n1 || true) + [ -n "$first" ] || { first='?'; echo "$n rep$rep: no txdemo.first_tx_submit event"; failed=1; } + echo "$n rep$rep: first_tx_ms=$first submitted=$submitted bulk_fail=$fail read_fail=$rd" } failed=0 for r in $(seq 1 "$REPS"); do run "$A" "$r"; run "$B" "$r"; done From 24b195cc7a9c7e947f3af207b938f4de78a8d4a1 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:29:55 +0300 Subject: [PATCH 14/29] jaguar3: the RF-window write-back covers the whole window; claims carry that scope tests/j3_rf_window_readback.sh pokes every window word by default (256 per path), and both dies were re-run that way: 512/512 words with the high 12 bits set read back 0 on one 8812CU and one 8812EU. The guide and the RF-table comment now state exactly that scope, and that the post-bring-up histogram is only a control. Co-Authored-By: Claude Fable 5.1 --- src/jaguar3/CLAUDE.md | 10 ++++++---- src/jaguar3/HalJaguar3.cpp | 17 +++++++++-------- tests/j3_rf_window_readback.sh | 4 ++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/jaguar3/CLAUDE.md b/src/jaguar3/CLAUDE.md index 60b3898b..5f624cb7 100644 --- a/src/jaguar3/CLAUDE.md +++ b/src/jaguar3/CLAUDE.md @@ -63,10 +63,12 @@ Jaguar3 uses it: wall-clock figure is one unit, one host. The RF radio-table load is write-only: bits [31:20] of the direct window -(`0x3c00`/`0x4c00 + addr*4`) read back 0 for every table entry, so the -vendor's `MASK20BITS` read-modify-write preserved nothing at the price of a -synchronous read per entry. Measured on one 8812EU (cold and warm) and one -8812CU (`tests/j3_rf_window_readback.sh`). +(`0x3c00`/`0x4c00 + addr*4`) are not storage, so the vendor's `MASK20BITS` +read-modify-write preserved nothing at the price of a synchronous read per +entry. Scope of that claim (`tests/j3_rf_window_readback.sh`): every one of +the 512 window words (both paths) poked with the high 12 bits set read back +0, on one 8812CU and one 8812EU; the post-bring-up histogram (all 512 words +0) is only a control, since the write-only load itself clears those bits. ## TX power diff --git a/src/jaguar3/HalJaguar3.cpp b/src/jaguar3/HalJaguar3.cpp index ea431823..0f4ae491 100644 --- a/src/jaguar3/HalJaguar3.cpp +++ b/src/jaguar3/HalJaguar3.cpp @@ -1028,14 +1028,15 @@ void HalJaguar3::apply_bb_rf_agc_tables(InitTimer *timer) { return; default: /* Plain 20-bit write, not the vendor's read-modify-write under - * MASK20BITS: the direct window's bits [31:20] read back 0 for every - * word of both path windows after a bring-up, on an 8812EU (cold - * and warm) and on an 8812CU (tests/j3_rf_window_readback.sh), so - * an RMW that reads 0 writes exactly `data` and preserving those - * bits is a no-op that cost a synchronous read per entry -- about - * half the RF table stage (~80 ms on the one 8812EU + ssc338q host - * measured; the x86 bench figures are in src/jaguar3/CLAUDE.md). - * Write-only also pipelines (ITransport::write_batch_begin). */ + * MASK20BITS: the direct window's bits [31:20] are not storage -- + * every one of the 512 window words (both paths) poked with those + * bits set reads back 0, on one 8812CU and one 8812EU + * (tests/j3_rf_window_readback.sh) -- so an RMW there writes + * exactly `data`, and preserving those bits was a no-op that cost a + * synchronous read per entry: about half the RF table stage (~80 ms + * on the one 8812EU + ssc338q host measured; x86 bench figures in + * src/jaguar3/CLAUDE.md). Write-only also pipelines + * (ITransport::write_batch_begin). */ _device.rtw_write32(static_cast(base + ((addr & 0xff) << 2)), data & RFREG_MASK); } diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh index 8626b1d8..a89988e6 100755 --- a/tests/j3_rf_window_readback.sh +++ b/tests/j3_rf_window_readback.sh @@ -10,7 +10,7 @@ # 1. histogram: dump both path windows after a bring-up and histogram the # high 12 bits of every word (expected all 0). On its own this proves # little, because the write-only load itself clears those bits. -# 2. write-back: for a sample of words in each window, poke the word with +# 2. write-back: for every word in each window (SAMPLE=N narrows it), poke the word with # its high 12 bits SET (low 20 bits unchanged, so the RF register keeps # its value), read it back, then restore. If the bits are storage they # read back set and the RMW was preserving real state; if they read back @@ -28,7 +28,7 @@ PID=${1:?usage: $0 } ROOT=$(cd "$(dirname "$0")/.." && pwd) CS=$ROOT/build/chipstate OUT=${OUT:-/tmp/j3_rf_window_readback}; mkdir -p "$OUT" -SAMPLE=${SAMPLE:-8} # words sampled per path window for the write-back leg +SAMPLE=${SAMPLE:-256} # words per path window for the write-back leg (256 = the whole window) dump=$OUT/pid${PID}.peek "$CS" --pid "$PID" --init --peek 0x3c00-0x3fff:4 --peek 0x4c00-0x4fff:4 \ >"$dump" 2>"$dump.err" || { echo "FAIL: chipstate exited non-zero (leg 1)"; tail -5 "$dump.err"; exit 1; } From 691838645c58bf2bff685e4c53306b00462b159e Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:37:51 +0300 Subject: [PATCH 15/29] jaguar3: stage checkpoints drain the queue first; RF check fails on a failed restore InitTimer takes an optional drain hook that runs at every checkpoint; the Jaguar3 timers pass the transport flush, so a stage's own queued writes complete before it is measured and their tail latency is billed to the stage that issued them. tests/j3_rf_window_readback.sh exits non-zero when its EXIT-trap restore pass fails: an unrestored radio is not a passing run. Co-Authored-By: Claude Fable 5.1 --- src/InitTimer.h | 17 ++++++++++++++--- src/jaguar3/HalJaguar3.cpp | 3 ++- src/jaguar3/RtlJaguar3Device.cpp | 3 ++- tests/j3_rf_window_readback.sh | 12 ++++++++---- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/InitTimer.h b/src/InitTimer.h index 79701b4d..b7904205 100644 --- a/src/InitTimer.h +++ b/src/InitTimer.h @@ -28,13 +28,21 @@ class InitTimer { public: using XferCounter = std::function; + using Drain = std::function; - InitTimer(Logger_t logger, const char *scope, XferCounter xfers = {}) + /* `drain`, when given, runs at the start of every checkpoint: a transport + * that queues writes (ITransport::write_batch_begin) completes the stage's + * own writes before the stage is measured, so their tail latency is billed + * to the stage that issued them, not to the next one. */ + InitTimer(Logger_t logger, const char *scope, XferCounter xfers = {}, + Drain drain = {}) : _logger{std::move(logger)}, _scope{scope}, _xfers{std::move(xfers)}, - _start{clock::now()}, _last{_start}, _x_start{count()}, - _x_last{_x_start} {} + _drain{std::move(drain)}, _start{clock::now()}, _last{_start}, + _x_start{count()}, _x_last{_x_start} {} void stage(const char *name) { + if (_drain) + _drain(); const auto now = clock::now(); const auto x = count(); emit(name, ms(_last, now), static_cast(x - _x_last)); @@ -49,6 +57,8 @@ class InitTimer { if (_finalized) return; _finalized = true; + if (_drain) + _drain(); emit("total", ms(_start, clock::now()), static_cast(count() - _x_start)); } @@ -74,6 +84,7 @@ class InitTimer { Logger_t _logger; const char *_scope; XferCounter _xfers; + Drain _drain; clock::time_point _start; clock::time_point _last; uint64_t _x_start; diff --git a/src/jaguar3/HalJaguar3.cpp b/src/jaguar3/HalJaguar3.cpp index 0f4ae491..5e9d2151 100644 --- a/src/jaguar3/HalJaguar3.cpp +++ b/src/jaguar3/HalJaguar3.cpp @@ -93,7 +93,8 @@ void HalJaguar3::run_iqk(SelectedChannel channel) { * Every step is ported from vendor source. */ void HalJaguar3::rtw_hal_init(SelectedChannel channel) { ChannelWidth_t bw = channel.ChannelWidth; - InitTimer timer(_logger, "j3hal", [this] { return _device.ctrl_xfers(); }); + InitTimer timer(_logger, "j3hal", [this] { return _device.ctrl_xfers(); }, + [this] { _device.flush_writes(); }); _macinit.pre_init_system_cfg(); timer.stage("pre_init_system_cfg"); diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 1971e8db..07867d1b 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -760,7 +760,8 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * race the running TX). */ const bool want_rx = _cfg.rx.enable_with_tx; _rx_wanted = want_rx; - InitTimer timer(_logger, "j3init", [this] { return _device.ctrl_xfers(); }); + InitTimer timer(_logger, "j3init", [this] { return _device.ctrl_xfers(); }, + [this] { _device.flush_writes(); }); WriteBatchScope batch(_device); _hal.rtw_hal_init(channel); /* full vendor-source bring-up */ timer.stage("hal_init"); diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh index a89988e6..2fd80704 100755 --- a/tests/j3_rf_window_readback.sh +++ b/tests/j3_rf_window_readback.sh @@ -63,12 +63,16 @@ restore_ops=$(tail -n1 <<<"$ops"); ops=$(head -n1 <<<"$ops") # Whatever happens once the first high-bit poke is out, put every sampled # word back to its dump value. The chip stays configured after chipstate # exits (its device teardown does not de-init), so a plain attach suffices. -restore() { +finish() { + local rc=$? # shellcheck disable=SC2086 - "$CS" --pid "$PID" $restore_ops >"$OUT/pid${PID}.restore" 2>&1 \ - || echo "WARN: restore pass failed — sampled window words may be left modified (see $OUT/pid${PID}.restore)" >&2 + if ! "$CS" --pid "$PID" $restore_ops >"$OUT/pid${PID}.restore" 2>&1; then + echo "FAIL: restore pass failed — window words may be left modified (see $OUT/pid${PID}.restore)" >&2 + [ "$rc" -ne 0 ] || rc=1 # an unrestored radio is not a passing run + fi + exit "$rc" } -trap restore EXIT +trap finish EXIT wb=$OUT/pid${PID}.writeback # shellcheck disable=SC2086 "$CS" --pid "$PID" --init $ops >"$wb" 2>"$wb.err" || { echo "FAIL: chipstate exited non-zero (leg 2)"; tail -5 "$wb.err"; exit 1; } From f54a196ba0562504e37bad6fdef5061978e1e1a3 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:47:50 +0300 Subject: [PATCH 16/29] usb pipelining: the destructor keeps a slot seen in flight before waiting out its callback Checked in that order the two atomic observations close: a slot seen in flight is kept whatever a concurrent callback does, and a slot seen not in flight has a callback that already set cb_busy before clearing inflight, so waiting on cb_busy covers all of its remaining stores. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index e6302738..17d4d2be 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -354,20 +354,22 @@ UsbTransport::~UsbTransport() { } int leaked = 0; for (auto *w : _aw_all) { - /* A callback still inside the slot (another adapter's pump thread - * reaping it right now) finishes in a handful of stores; wait it out - * rather than free under it. */ - while (w->cb_busy) - std::this_thread::yield(); /* libusb forbids freeing an active transfer, and its callback still * writes through `w`. If the drain above could not reap it (dead event * loop / yanked device), leaking the slot is the lesser evil: a callback * that somehow fires later touches leaked memory, whereas freeing here - * hands libusb a dangling transfer it is still holding. */ + * hands libusb a dangling transfer it is still holding. Checked FIRST: + * a slot seen in flight is kept whatever a concurrent callback does. */ if (w->inflight) { ++leaked; /* keeps its shared AsyncPool alive for a late callback */ continue; } + /* Seen not in flight: any callback for it has at least reached the + * store that cleared `inflight`, and it set `cb_busy` before that, so + * waiting here covers the whole of its remaining stores (another + * adapter's pump thread reaping it right now). */ + while (w->cb_busy) + std::this_thread::yield(); libusb_free_transfer(w->t); delete w; } From 3bfa42c4d700893632ab7317a0a276c8a3816306 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:59:15 +0300 Subject: [PATCH 17/29] chipstate: build the raw adapter inside the register-op try block Its constructor already reads a register, and after --init that read is the first thing that can fail; a throw there now reports the failed access and exits 4 like any other op instead of terminating the tool. Co-Authored-By: Claude Fable 5.1 --- examples/chipstate/main.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/examples/chipstate/main.cpp b/examples/chipstate/main.cpp index 04678b2b..e47ebcf5 100644 --- a/examples/chipstate/main.cpp +++ b/examples/chipstate/main.cpp @@ -156,11 +156,13 @@ int run_reg_ops(libusb_device_handle *handle, Logger_t logger, libusb_context *ctx, std::shared_ptr lock, const std::vector &ops) { - RtlAdapter adapter(handle, logger, ctx, lock); /* A failed vendor-control read throws (UsbTransport::ctrl_read) — on a * powered-down or wedged chip that is a real answer about the chip, so - * report which op died and exit nonzero instead of terminating. */ + * report which op died and exit nonzero instead of terminating. The + * adapter is built inside the try too: its constructor already reads a + * register, and after --init that read is the first thing that can fail. */ try { + RtlAdapter adapter(handle, logger, ctx, lock); for (const RegOp &op : ops) { if (op.write) { bool ok; From 0a01bd44e9b68ed33c2fc9bdfbdf6f12708ee719 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:09:56 +0300 Subject: [PATCH 18/29] usb pipelining: bulk-IN drains first; the callback's free-list push is its last act - rx_loop and rx_raw drain the write queue before the first bulk-IN, as the batch contract says bulk transfers do; the RX must not outrun queued configuration. - async_write_cb clears cb_busy before the free-list push and holds the pool by a local shared reference, so the push touches only the pool and is the callback's last access: a later submission that reuses the slot can never have its own busy flag cleared by the earlier callback, and the destructor cannot free a slot while a callback is inside it. - tests/j3_tx_flood_ab.sh keys logs and result lines by side (A/B), so two checkouts that share a directory name do not overwrite each other. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 34 +++++++++++++++++++++------------- tests/j3_tx_flood_ab.sh | 8 +++++--- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 17d4d2be..b228b44d 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -453,30 +453,36 @@ bool UsbTransport::write_batch_end() { void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { auto *w = static_cast(t->user_data); /* Only the shared pool is touched here — never the transport, which a - * leaked slot can outlive (AsyncPool). */ - AsyncPool &pool = *w->pool; + * leaked slot can outlive (AsyncPool). A local reference keeps the pool + * alive past the slot too: the free-list push below is this callback's + * last act, after which `w` may be freed (destructor) or reused (a later + * submission) at any moment. */ + std::shared_ptr pool = w->pool; w->cb_busy = true; - /* Order matters: the result and every piece of accounting are final - * before the slot becomes visible again. `done` is published after - * status/actual so a waiter that sees it sees the result; the free-list - * push comes last so a taker (always the batch's own thread) can never - * reset a slot this callback is still writing to. A reader that is + /* Order matters. The result and every piece of accounting are final + * before the slot becomes visible again: `done` is published after + * status/actual so a waiter that sees it sees the result. `cb_busy` + * clears BEFORE the free-list push, so the push — which touches only + * the pool, never `w` — is the last access, and no later submission's + * callback can have its own busy flag cleared by this one. A taker + * (always the batch's own thread) cannot see the slot before the push; + * the destructor cannot free it while cb_busy is set. A reader that is * waiting on this very slot copies its data out before it submits * anything else, so the buffer is intact when it does. */ w->status = t->status; w->actual = t->actual_length; if (!w->is_read && (t->status != LIBUSB_TRANSFER_COMPLETED || t->actual_length != t->length - LIBUSB_CONTROL_SETUP_SIZE)) - pool.write_errors++; + pool->write_errors++; w->inflight = false; w->done = true; - pool.inflight--; - pool.completed++; + pool->inflight--; + pool->completed++; + w->cb_busy = false; { - std::lock_guard lk(pool.mu); - pool.free.push_back(w); + std::lock_guard lk(pool->mu); + pool->free.push_back(w); /* pointer value only — no access through it */ } - w->cb_busy = false; /* last: the destructor may free the slot after this */ } UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { @@ -674,6 +680,7 @@ void UsbTransport::rx_loop( int buf_size, int n_urbs, const std::function &on_data, const std::function &should_stop) { + flush_writes(); /* bulk-IN behind queued register writes (batch contract) */ if (_rx_mode == RxMode::Sync) { rx_loop_sync(buf_size, on_data, should_stop); return; @@ -958,6 +965,7 @@ void UsbTransport::rx_loop_sync( } int UsbTransport::rx_raw(uint8_t *buf, int len, int timeout_ms) { + flush_writes(); /* as rx_loop: the RX must not outrun queued configuration */ int actual = 0; int rc = libusb_bulk_transfer(_dev_handle, _info.bulk_in_ep, buf, len, &actual, timeout_ms); diff --git a/tests/j3_tx_flood_ab.sh b/tests/j3_tx_flood_ab.sh index af37c30c..29bcc87a 100755 --- a/tests/j3_tx_flood_ab.sh +++ b/tests/j3_tx_flood_ab.sh @@ -9,8 +9,10 @@ set -euo pipefail PID=${1:?pid}; CH=${2:?channel}; A=${3:?treeA}; B=${4:?treeB}; REPS=${5:-3}; SECS=${6:-15} OUT=${OUT:-/tmp/j3_tx_flood_ab/pid${PID}_ch${CH}}; mkdir -p "$OUT" run() { - local tree=$1 rep=$2 n; n=$(basename "$tree") - local log=$OUT/${n}_rep${rep} + # Logs and lines are keyed by side (A/B) so two checkouts that share a + # directory name cannot overwrite each other; the basename is context. + local side=$1 tree=$2 rep=$3 n; n="$side:$(basename "$tree")" + local log=$OUT/${side}_rep${rep} local rc=0 env DEVOURER_PID="$PID" DEVOURER_CHANNEL="$CH" DEVOURER_LOG_LEVEL=info \ timeout -s INT "$SECS" "$tree/build/txdemo" >"$log.jsonl" 2>"$log.err" || rc=$? @@ -30,6 +32,6 @@ run() { echo "$n rep$rep: first_tx_ms=$first submitted=$submitted bulk_fail=$fail read_fail=$rd" } failed=0 -for r in $(seq 1 "$REPS"); do run "$A" "$r"; run "$B" "$r"; done +for r in $(seq 1 "$REPS"); do run A "$A" "$r"; run B "$B" "$r"; done # Every rep is still reported, but a crashed or failed run fails the script. exit "$failed" From 45549a64a997b395bbcffc0bb29daf66376f370a Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:17:50 +0300 Subject: [PATCH 19/29] usb pipelining: an interrupted event wait is a wake-up; RF check counts its read-back words MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pump_once treats LIBUSB_ERROR_INTERRUPTED as transient — the caller's elapsed-time deadline still bounds the wait — instead of turning a signal into cancelled writes. tests/j3_rf_window_readback.sh parses every word on a read-back row and fails unless exactly one word per poke came back, so a parsing loss can never pass as a verdict. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 4 ++++ tests/j3_rf_window_readback.sh | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index b228b44d..46f3765a 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -562,6 +562,10 @@ bool UsbTransport::async_wait_progress() { bool UsbTransport::pump_once(int ms) { struct timeval tv {0, ms * 1000}; const int rc = libusb_handle_events_timeout_completed(_ctx, &tv, nullptr); + /* A signal landing in the wait is a wake-up, not a broken loop; the + * caller's elapsed-time deadline still bounds it. */ + if (rc == LIBUSB_ERROR_INTERRUPTED) + return true; if (rc < 0) { _logger->error("USB: event loop error {} while draining pipelined writes", rc); diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh index 2fd80704..d23f9ba7 100755 --- a/tests/j3_rf_window_readback.sh +++ b/tests/j3_rf_window_readback.sh @@ -76,12 +76,19 @@ trap finish EXIT wb=$OUT/pid${PID}.writeback # shellcheck disable=SC2086 "$CS" --pid "$PID" --init $ops >"$wb" 2>"$wb.err" || { echo "FAIL: chipstate exited non-zero (leg 2)"; tail -5 "$wb.err"; exit 1; } -python3 - "$wb" "$PID" <<'PY' +python3 - "$wb" "$PID" "$SAMPLE" <<'PY' import re, sys -rows = [(int(m.group(1), 16), int(m.group(2), 16)) for m in - (re.match(r'^0x([0-9a-fA-F]{4}):\s+([0-9a-fA-F]{8})\s*$', l) for l in open(sys.argv[1])) if m] -if not rows: - print("FAIL: no read-back rows parsed"); sys.exit(1) +# Each poke is followed by its own one-word peek, printed as a 16-byte row +# with the other three columns blank; a row may still carry 1-4 words, so +# every word on it is checked. The count must equal the pokes issued. +rows = [] +for l in open(sys.argv[1]): + m = re.match(r'^0x([0-9a-fA-F]{4}):((?:\s+[0-9a-fA-F]{8}){1,4})\s*$', l) + if m: + rows += [(int(m.group(1), 16), int(w, 16)) for w in m.group(2).split()] +expected = 2 * int(sys.argv[3]) +if len(rows) != expected: + print(f"FAIL: parsed {len(rows)} read-back words, expected {expected} (one per poke)"); sys.exit(1) bad = [(a, v) for a, v in rows if v >> 20] print(f"leg2 pid={sys.argv[2]} words_poked_with_hi_bits_set={len(rows)} read_back_nonzero_hi={len(bad)}") for a, v in bad: print(f" 0x{a:04x} -> 0x{v:08x}") From a632a81eac2060cdaa301de930c7b86082199fca Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:28:46 +0300 Subject: [PATCH 20/29] usb pipelining: reads fall back synchronously too; retired reads are not write errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pipelined read that cannot be submitted or completed is read synchronously (EP0 order still places it behind the queue), so only a transfer that fails there too yields the sentinel or throws — the same shape as the writes. When a drain retires slots, only the writes among them count toward the batch verdict. tests/j3_rf_window_readback.sh refuses SAMPLE outside 1..256: a probe of zero words proves nothing. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 8 +++++++- src/UsbTransport.h | 18 +++++++++++------- tests/j3_rf_window_readback.sh | 2 ++ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 46f3765a..6192c9cf 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -617,7 +617,13 @@ void UsbTransport::flush_writes() { * register access would otherwise walk take-slot -> wait -> drain * (seconds each) for the rest of the bring-up. Synchronous from * here on. */ - _aw->write_errors += _aw->inflight; /* reads among them fail their callers too */ + /* Only the writes among the retired slots are batch write errors; a + * stranded read already failed its caller (false / throw), and a + * caller that retried it synchronously and recovered must not see + * the batch fail for it. */ + for (auto *w : _aw_all) + if (w->inflight && !w->is_read) + _aw->write_errors++; _aw_abandoned = true; _batch = false; /* pipelining off; the caller's batch stays open so * write_batch_end still returns this verdict */ diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 910ca6de..25cf1629 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -78,11 +78,13 @@ class UsbTransport final : public ITransport { if (async_read(static_cast(addr & 0xFFFF), static_cast(addr >> 16), &data, sizeof(data))) return data; - /* Logged, unlike the synchronous path below: a failed pipelined read - * is a queue problem, not a register problem, and must not be - * mistaken for a register that genuinely reads all-ones. */ - _logger->error("USB: pipelined read32_wide(0x{:05x}) failed", addr); - return 0xFFFFFFFFu; + /* A pipelined read that could not be submitted or completed is a + * queue problem, not a register problem: say so, then read it + * synchronously below — EP0 order still places that read behind + * whatever is queued — so only a real transfer failure yields the + * all-ones sentinel. */ + _logger->error("USB: pipelined read32_wide(0x{:05x}) failed; reading " + "synchronously", addr); } if (libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_READ, 5, static_cast(addr & 0xFFFF), @@ -268,8 +270,10 @@ template T UsbTransport::ctrl_read(uint16_t reg_num) { if (_batch) { if (async_read(reg_num, 0, &data, sizeof(T))) return data; - _logger->error("rtw_read({:04x}) pipelined, sizeof(T) = {}", reg_num, sizeof(T)); - throw std::ios_base::failure("rtw_read"); + /* Fall through to the synchronous read (see read32_wide): only a + * transfer that fails there too throws. */ + _logger->error("rtw_read({:04x}) pipelined failed; reading synchronously", + reg_num); } if (libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_READ, 5, reg_num, 0, (uint8_t *)&data, sizeof(T), diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh index d23f9ba7..86f0c117 100755 --- a/tests/j3_rf_window_readback.sh +++ b/tests/j3_rf_window_readback.sh @@ -29,6 +29,8 @@ ROOT=$(cd "$(dirname "$0")/.." && pwd) CS=$ROOT/build/chipstate OUT=${OUT:-/tmp/j3_rf_window_readback}; mkdir -p "$OUT" SAMPLE=${SAMPLE:-256} # words per path window for the write-back leg (256 = the whole window) +case $SAMPLE in ''|*[!0-9]*) echo "FAIL: SAMPLE must be an integer 1..256 (got '$SAMPLE')"; exit 2;; esac +if [ "$SAMPLE" -lt 1 ] || [ "$SAMPLE" -gt 256 ]; then echo "FAIL: SAMPLE must be 1..256 (got $SAMPLE) — a probe of zero words proves nothing"; exit 2; fi dump=$OUT/pid${PID}.peek "$CS" --pid "$PID" --init --peek 0x3c00-0x3fff:4 --peek 0x4c00-0x4fff:4 \ >"$dump" 2>"$dump.err" || { echo "FAIL: chipstate exited non-zero (leg 1)"; tail -5 "$dump.err"; exit 1; } From ec81294687c0f4e1bab8ffcf454cf4726c626296 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:38:12 +0300 Subject: [PATCH 21/29] usb: count register transfers where they are issued The per-transport counter increments on a successful async submit and immediately before each synchronous control transfer, so a fallback after a refused submit counts as its own transfer and a refused submit that never reached the bus counts as none. Same one-per-access total on the normal path. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 4 +++- src/UsbTransport.h | 11 ++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 6192c9cf..65829d3e 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -662,8 +662,10 @@ bool UsbTransport::async_submit(AsyncWrite *w) { w->inflight = true; _aw->inflight++; const int rc = libusb_submit_transfer(w->t); - if (rc == 0) + if (rc == 0) { + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); /* issued */ return true; + } _aw->inflight--; w->inflight = false; /* Not a batch write error yet: the caller retries a refused write diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 25cf1629..3fa78553 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -51,14 +51,15 @@ class UsbTransport final : public ITransport { /* Realtek USB register addressing: wValue = addr[15:0], wIndex = * addr[31:16]. Lets the BB/RF window (addr + 0x10000) reach wIndex=1 * instead of colliding with the MAC/system space at wIndex=0. */ - _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); /* A pipelined write that cannot be submitted (no usable slot, submit * rejected) falls through to the synchronous transfer below: EP0 keeps * submission order, so it lands behind whatever is still queued and no - * register write is silently dropped. */ + * register write is silently dropped. Transfers are counted where they + * are issued (async_submit / here), so a fallback counts as its own. */ if (_batch && async_write(static_cast(addr & 0xFFFF), static_cast(addr >> 16), &v, sizeof(v))) return true; + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); const bool ok = libusb_control_transfer( _dev_handle, REALTEK_USB_VENQT_WRITE, 5, @@ -73,7 +74,6 @@ class UsbTransport final : public ITransport { } uint32_t read32_wide(uint32_t addr) override { uint32_t data = 0; - _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); if (_batch) { if (async_read(static_cast(addr & 0xFFFF), static_cast(addr >> 16), &data, sizeof(data))) @@ -86,6 +86,7 @@ class UsbTransport final : public ITransport { _logger->error("USB: pipelined read32_wide(0x{:05x}) failed; reading " "synchronously", addr); } + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); if (libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_READ, 5, static_cast(addr & 0xFFFF), static_cast(addr >> 16), @@ -266,7 +267,6 @@ class UsbTransport final : public ITransport { template T UsbTransport::ctrl_read(uint16_t reg_num) { T data = 0; - _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); if (_batch) { if (async_read(reg_num, 0, &data, sizeof(T))) return data; @@ -275,6 +275,7 @@ template T UsbTransport::ctrl_read(uint16_t reg_num) { _logger->error("rtw_read({:04x}) pipelined failed; reading synchronously", reg_num); } + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); if (libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_READ, 5, reg_num, 0, (uint8_t *)&data, sizeof(T), USB_TIMEOUT) == sizeof(T)) { @@ -286,10 +287,10 @@ template T UsbTransport::ctrl_read(uint16_t reg_num) { } template bool UsbTransport::ctrl_write(uint16_t reg_num, T value) { - _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); /* Unsubmittable pipelined write -> synchronous, in order (see write32_wide). */ if (_batch && async_write(reg_num, 0, &value, sizeof(T))) return true; + _ctrl_xfers.fetch_add(1, std::memory_order_relaxed); const bool ok = libusb_control_transfer(_dev_handle, REALTEK_USB_VENQT_WRITE, 5, reg_num, 0, (uint8_t *)&value, sizeof(T), From f8a612889a7a0f948dd737da207db191db72f7d2 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 20:49:31 +0300 Subject: [PATCH 22/29] tests: j3_rf_window_readback.sh names the probed register in its diagnostics Read-back values pair with the addresses the peeks were issued for, in order, rather than with the printed 16-byte row base, so a failing word at a later column is reported at its own address. Co-Authored-By: Claude Fable 5.1 --- tests/j3_rf_window_readback.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/j3_rf_window_readback.sh b/tests/j3_rf_window_readback.sh index 86f0c117..bc839ec7 100755 --- a/tests/j3_rf_window_readback.sh +++ b/tests/j3_rf_window_readback.sh @@ -83,14 +83,19 @@ import re, sys # Each poke is followed by its own one-word peek, printed as a 16-byte row # with the other three columns blank; a row may still carry 1-4 words, so # every word on it is checked. The count must equal the pokes issued. -rows = [] +vals = [] for l in open(sys.argv[1]): m = re.match(r'^0x([0-9a-fA-F]{4}):((?:\s+[0-9a-fA-F]{8}){1,4})\s*$', l) if m: - rows += [(int(m.group(1), 16), int(w, 16)) for w in m.group(2).split()] -expected = 2 * int(sys.argv[3]) -if len(rows) != expected: - print(f"FAIL: parsed {len(rows)} read-back words, expected {expected} (one per poke)"); sys.exit(1) + vals += [int(w, 16) for w in m.group(2).split()] +n = int(sys.argv[3]) +# The peeks were issued in this exact order (path A window, then path B), +# one word each, so the values pair with these addresses — not with the +# printed row base, which is the 16-byte row the word sits in. +addrs = [a for base in (0x3c00, 0x4c00) for a in range(base, base + 4 * n, 4)] +if len(vals) != len(addrs): + print(f"FAIL: parsed {len(vals)} read-back words, expected {len(addrs)} (one per poke)"); sys.exit(1) +rows = list(zip(addrs, vals)) bad = [(a, v) for a, v in rows if v >> 20] print(f"leg2 pid={sys.argv[2]} words_poked_with_hi_bits_set={len(rows)} read_back_nonzero_hi={len(bad)}") for a, v in bad: print(f" 0x{a:04x} -> 0x{v:08x}") From 77b777c7c007730003c6b67773a73e3840debba5 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 21:00:38 +0300 Subject: [PATCH 23/29] usb pipelining: snapshot the completion counter before looking, not after async_wait_progress takes the caller's snapshot of the completion counter, taken before the caller checked the free list (take_slot) or the slot's done flag (async_read). A completion that lands in between, from another adapter's pump thread, moves the counter past the snapshot and the wait returns at once instead of sitting out its 2 s deadline over an available slot. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 24 ++++++++++++++++-------- src/UsbTransport.h | 6 +++++- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 65829d3e..3a5202a6 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -494,9 +494,14 @@ UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { _aw->free.pop_back(); return w; }; + /* Snapshot first, then look: a callback on another adapter's pump thread + * that returns a slot between the look and the wait moves the counter + * past the snapshot, so the wait returns at once instead of sitting out + * its deadline over an available slot. */ + uint64_t before = _aw->completed; AsyncWrite *w = take(); while (!w) { - if (!async_wait_progress()) { + if (!async_wait_progress(before)) { flush_writes(); /* recovers the pool on a stuck queue */ /* A recovery that retired slots closed the batch: hand out nothing, * even if some cancellations did return a slot, so the caller takes @@ -504,6 +509,7 @@ UsbTransport::AsyncWrite *UsbTransport::async_take_slot() { if (_aw_abandoned || !_batch) return nullptr; } + before = _aw->completed; w = take(); } w->done = false; @@ -529,8 +535,11 @@ bool UsbTransport::async_read(uint16_t wvalue, uint16_t windex, void *data, _logger->error("USB: pipelined read submit failed"); return false; } - while (!w->done) { - if (!async_wait_progress()) { + for (;;) { + const uint64_t before = _aw->completed; /* snapshot, then look */ + if (w->done) + break; + if (!async_wait_progress(before)) { flush_writes(); /* cancels + recovers; w->done is set by the cancel */ break; } @@ -542,13 +551,12 @@ bool UsbTransport::async_read(uint16_t wvalue, uint16_t windex, void *data, return true; } -bool UsbTransport::async_wait_progress() { - /* Pumps until THIS pool's completion counter advances, a real 2 s - * deadline passes, or the event loop errors. Elapsed time, not a turn +bool UsbTransport::async_wait_progress(uint64_t before) { + /* Pumps until THIS pool's completion counter moves past `before`, a real + * 2 s deadline passes, or the event loop errors. Elapsed time, not a turn * count: on a shared libusb context another adapter's RX/TX completions * make each handle_events return at once, and counting those turns would * declare a healthy queue stuck and cancel it. */ - const uint64_t before = _aw->completed; const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); while (_aw->completed == before) { if (std::chrono::steady_clock::now() >= deadline) @@ -581,7 +589,7 @@ void UsbTransport::flush_writes() { if (_aw_abandoned) return; while (_aw->inflight > 0) { - if (async_wait_progress()) + if (async_wait_progress(_aw->completed)) continue; /* No completion in ~2 s of pumping. USB_TIMEOUT is 500 ms, so libusb * itself times a stuck transfer out and completes it through the diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 3fa78553..2faf086c 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -176,7 +176,11 @@ class UsbTransport final : public ITransport { bool async_read(uint16_t wvalue, uint16_t windex, void *data, size_t n); AsyncWrite *async_take_slot(); bool async_submit(AsyncWrite *w); /* in-flight accounting before submit */ - bool async_wait_progress(); /* pump until this pool progresses; false on a 2 s deadline/error */ + /* Pump until this pool's completion counter moves past `before` (a + * snapshot the caller took BEFORE checking whatever it is waiting for, + * so a completion landing in between is not missed); false on a 2 s + * deadline or an event-loop error. */ + bool async_wait_progress(uint64_t before); bool pump_once(int ms); /* one bounded handle_events turn; false on error */ static void LIBUSB_CALL async_write_cb(libusb_transfer *t); /* `_batch`: pipelined submission is enabled right now. `_batch_open`: the From 04c0cd21f464e4f518c219117e3ecd4166e86c93 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 21:11:23 +0300 Subject: [PATCH 24/29] usb pipelining: the batch opens even when pipelining cannot write_batch_begin opens the caller's batch (and resets its verdict) before deciding whether pipelined submission is possible; a failed pool allocation or a queue retired in an earlier session leaves the writes synchronous, and a failed one still counts for write_batch_end. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 3a5202a6..14732cec 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -398,6 +398,12 @@ UsbTransport::~UsbTransport() { void UsbTransport::write_batch_begin() { if (_batch_open) return; + /* The caller's batch opens whatever happens below: with pipelining off + * the writes go synchronously and a failed one still counts, so + * write_batch_end reports it — a radio with a register unprogrammed is + * the same problem whichever path the write took. */ + _batch_open = true; + _aw->write_errors = 0; /* A session that already failed to reap its transfers has a short pool and * a suspect event loop; stay synchronous rather than pipeline into it. */ if (_aw_abandoned) @@ -417,7 +423,7 @@ void UsbTransport::write_batch_begin() { delete s; } _logger->error("USB: libusb_alloc_transfer failed; register writes " - "stay synchronous"); + "stay synchronous (the batch verdict still applies)"); return; } w->pool = _aw; @@ -427,9 +433,7 @@ void UsbTransport::write_batch_begin() { std::lock_guard lk(_aw->mu); _aw->free = slots; } - _aw->write_errors = 0; _batch = true; - _batch_open = true; } bool UsbTransport::write_batch_end() { From 6275f8074b29ea4ce29f9717317e13922794b62f Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 21:22:16 +0300 Subject: [PATCH 25/29] usb pipelining: a stale completion cannot touch a later batch's verdict Each write_batch_begin bumps a generation; a slot records the generation it was submitted under, and the callback counts a failed write only for the current one. A slot retired by an earlier drain was counted when it was retired; if its completion arrives during a later batch it changes nothing. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 9 ++++++--- src/UsbTransport.h | 6 ++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 14732cec..65627712 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -404,6 +404,7 @@ void UsbTransport::write_batch_begin() { * the same problem whichever path the write took. */ _batch_open = true; _aw->write_errors = 0; + _aw->generation++; /* A session that already failed to reap its transfers has a short pool and * a suspect event loop; stay synchronous rather than pipeline into it. */ if (_aw_abandoned) @@ -475,9 +476,10 @@ void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { * anything else, so the buffer is intact when it does. */ w->status = t->status; w->actual = t->actual_length; - if (!w->is_read && (t->status != LIBUSB_TRANSFER_COMPLETED || - t->actual_length != t->length - LIBUSB_CONTROL_SETUP_SIZE)) - pool->write_errors++; + if (!w->is_read && w->gen == pool->generation && + (t->status != LIBUSB_TRANSFER_COMPLETED || + t->actual_length != t->length - LIBUSB_CONTROL_SETUP_SIZE)) + pool->write_errors++; /* a stale generation was counted when retired */ w->inflight = false; w->done = true; pool->inflight--; @@ -671,6 +673,7 @@ bool UsbTransport::async_write(uint16_t wvalue, uint16_t windex, * on another thread's event pump the moment libusb has it — and rolls the * accounting back if libusb refuses the transfer. */ bool UsbTransport::async_submit(AsyncWrite *w) { + w->gen = _aw->generation; w->inflight = true; _aw->inflight++; const int rc = libusb_submit_transfer(w->t); diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 2faf086c..7e7a1c8c 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -149,6 +149,11 @@ class UsbTransport final : public ITransport { * caller directly (false / throw) and is NOT counted here — a read * glitch that the caller retries and recovers must not fail the batch. */ std::atomic write_errors{0}; + /* Bumped by every write_batch_begin; a slot carries the generation it + * was submitted under, and a completion from an older generation (a + * slot retired by a drain that finishes late) never touches the current + * batch's verdict — it was already counted when it was retired. */ + std::atomic generation{0}; }; struct AsyncWrite { libusb_transfer *t; @@ -164,6 +169,7 @@ class UsbTransport final : public ITransport { * as the destructor's "safe to free" signal — this is. */ std::atomic cb_busy{false}; bool is_read = false; /* set before submit; decides which failure it is */ + uint64_t gen = 0; /* batch generation the slot was submitted under */ int status = -1; int actual = 0; }; From 814cb6c2ff90b9b80dac61c0c14e78d37e199d05 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 21:33:29 +0300 Subject: [PATCH 26/29] usb pipelining: batches nest; handoff and free decision share the pool mutex; re-init joins coex - write_batch_begin/end keep a depth: the outermost pair owns the verdict and the close, an inner end leaves pipelining on. - The callback's busy-clear and free-list push are one critical section under the pool mutex, and the destructor decides under the same mutex (pulling a completed slot off the free list before freeing it), so it can neither free a slot mid-handoff nor leave a dangling pointer on the list. - A second InitWrite on a live device stops and joins the previous coex thread before opening its batch. Co-Authored-By: Claude Fable 5.1 --- src/UsbTransport.cpp | 60 +++++++++++++++++++++----------- src/UsbTransport.h | 1 + src/jaguar3/RtlJaguar3Device.cpp | 8 +++++ 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/src/UsbTransport.cpp b/src/UsbTransport.cpp index 65627712..31dcc57a 100644 --- a/src/UsbTransport.cpp +++ b/src/UsbTransport.cpp @@ -354,24 +354,34 @@ UsbTransport::~UsbTransport() { } int leaked = 0; for (auto *w : _aw_all) { - /* libusb forbids freeing an active transfer, and its callback still - * writes through `w`. If the drain above could not reap it (dead event - * loop / yanked device), leaking the slot is the lesser evil: a callback - * that somehow fires later touches leaked memory, whereas freeing here - * hands libusb a dangling transfer it is still holding. Checked FIRST: - * a slot seen in flight is kept whatever a concurrent callback does. */ - if (w->inflight) { - ++leaked; /* keeps its shared AsyncPool alive for a late callback */ - continue; - } - /* Seen not in flight: any callback for it has at least reached the - * store that cleared `inflight`, and it set `cb_busy` before that, so - * waiting here covers the whole of its remaining stores (another - * adapter's pump thread reaping it right now). */ - while (w->cb_busy) + for (;;) { + std::unique_lock lk(_aw->mu); + /* libusb forbids freeing an active transfer, and its callback still + * writes through `w`. If the drain above could not reap it (dead + * event loop / yanked device), leaking the slot is the lesser evil: + * a callback that somehow fires later touches leaked memory, whereas + * freeing here hands libusb a dangling transfer it is still holding. + * Checked first: a slot seen in flight is kept whatever a concurrent + * callback does. */ + if (w->inflight) { + ++leaked; /* keeps its shared AsyncPool alive for a late callback */ + break; + } + /* Seen not in flight and, under the pool mutex, not busy: its + * callback has completed the handoff (busy-clear + free-list push + * happen inside this same mutex), so nothing can publish `w` after + * we free it. Pull it off the free list first. */ + if (!w->cb_busy) { + auto &fr = _aw->free; + fr.erase(std::remove(fr.begin(), fr.end(), w), fr.end()); + lk.unlock(); + libusb_free_transfer(w->t); + delete w; + break; + } + lk.unlock(); /* a callback is inside the slot on another thread */ std::this_thread::yield(); - libusb_free_transfer(w->t); - delete w; + } } if (leaked) _logger->error("USB: leaked {} unreaped pipelined transfer slot(s)", leaked); @@ -396,8 +406,8 @@ UsbTransport::~UsbTransport() { * followed by a read behaves exactly like the synchronous sequence; the win * is that the host does not sit through a full URB round trip per write. */ void UsbTransport::write_batch_begin() { - if (_batch_open) - return; + if (_batch_depth++ > 0) + return; /* nested: the outermost batch owns the verdict and the close */ /* The caller's batch opens whatever happens below: with pipelining off * the writes go synchronously and a failed one still counts, so * write_batch_end reports it — a radio with a register unprogrammed is @@ -440,6 +450,8 @@ void UsbTransport::write_batch_begin() { bool UsbTransport::write_batch_end() { if (!_batch_open) return true; + if (--_batch_depth > 0) + return true; /* an inner end: pipelining stays on, the outer end reports */ flush_writes(); /* Failed and short completions are only known here, after the fact: a * write reported true at submission. The count covers failed/short @@ -484,10 +496,16 @@ void LIBUSB_CALL UsbTransport::async_write_cb(libusb_transfer *t) { w->done = true; pool->inflight--; pool->completed++; - w->cb_busy = false; { + /* The busy-clear and the free-list push are one critical section: a + * taker (under the same mutex) can only see the slot after both, so + * no later submission's callback can be inside it while this flag + * store lands; and the destructor decides under this mutex too, so it + * cannot free the slot between the clear and the push. After the + * unlock nothing here touches `w`. */ std::lock_guard lk(pool->mu); - pool->free.push_back(w); /* pointer value only — no access through it */ + w->cb_busy = false; + pool->free.push_back(w); } } diff --git a/src/UsbTransport.h b/src/UsbTransport.h index 7e7a1c8c..3524e96a 100644 --- a/src/UsbTransport.h +++ b/src/UsbTransport.h @@ -195,6 +195,7 @@ class UsbTransport final : public ITransport { * write errors) for write_batch_end to return. */ bool _batch = false; bool _batch_open = false; + int _batch_depth = 0; /* begin/end nest; the outermost pair owns the verdict */ std::shared_ptr _aw = std::make_shared(); std::vector _aw_all; /* Set when a drain gave up with transfers still submitted: the destructor diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 07867d1b..2b648b64 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -760,6 +760,14 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * race the running TX). */ const bool want_rx = _cfg.rx.enable_with_tx; _rx_wanted = want_rx; + /* A second bring-up on a live device: the coex thread of the previous + * one shares the transport, and the batch below is single-threaded by + * contract, so stop and join it before anything is queued. */ + if (_coex_thread.joinable()) { + _coex_stop = true; + _coex_thread.join(); + _coex_stop = false; + } InitTimer timer(_logger, "j3init", [this] { return _device.ctrl_xfers(); }, [this] { _device.flush_writes(); }); WriteBatchScope batch(_device); From 2375a4537109c03f2ad0cee6b22f49710fc283bd Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 21:44:16 +0300 Subject: [PATCH 27/29] jaguar3: readiness is provisional from the top of InitWrite The rollback guard is installed right after the previous coex thread is joined, before any bring-up step can throw, so a failed re-init cannot leave the readiness flag from the previous successful one in place. The flag is set provisionally where it was and committed after a clean batch close, as before. Co-Authored-By: Claude Fable 5.1 --- src/jaguar3/RtlJaguar3Device.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 2b648b64..2616c931 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -768,6 +768,19 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { _coex_thread.join(); _coex_stop = false; } + /* Readiness is provisional from here until the batch closes clean: a + * throw from any bring-up step (a failed queued write is only known at + * the close) must not leave the runtime APIs believing the chip is + * programmed — including a re-init that fails after a successful one. */ + struct BroughtUpGuard { + bool &flag; + bool committed = false; + ~BroughtUpGuard() { + if (!committed) + flag = false; + } + } brought_up_guard{_brought_up}; + _brought_up = false; InitTimer timer(_logger, "j3init", [this] { return _device.ctrl_xfers(); }, [this] { _device.flush_writes(); }); WriteBatchScope batch(_device); @@ -849,18 +862,7 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { * intermediate bring-up steps on sane references. */ apply_tx_power_current(/*full=*/true); timer.stage("txpower_pre"); - /* Readiness is provisional until the batch closes clean: a throw from - * anywhere below (a failed queued write is only known at the close) - * must not leave the runtime APIs believing the chip is programmed. */ - struct BroughtUpGuard { - bool &flag; - bool committed = false; - ~BroughtUpGuard() { - if (!committed) - flag = false; - } - } brought_up_guard{_brought_up}; - _brought_up = true; + _brought_up = true; /* provisional — see BroughtUpGuard above */ /* WiFi-only coex bring-up: disable the BT/LTE antenna arbitration and lock the * antenna to WLAN so on-air TX is not killed by the coex firmware. */ _hal.coex_wlan_only_init(); From dd47acb0cd4842810fa8bcac74dcd8694c1e84ba Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 21:55:05 +0300 Subject: [PATCH 28/29] jaguar3: attach the transfer counter to the timers on USB only A PCIe transport's timer omits the xfers field instead of reporting 0, matching the documented schema. Co-Authored-By: Claude Fable 5.1 --- src/jaguar3/HalJaguar3.cpp | 7 ++++++- src/jaguar3/RtlJaguar3Device.cpp | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/jaguar3/HalJaguar3.cpp b/src/jaguar3/HalJaguar3.cpp index 5e9d2151..f32e9a36 100644 --- a/src/jaguar3/HalJaguar3.cpp +++ b/src/jaguar3/HalJaguar3.cpp @@ -93,7 +93,12 @@ void HalJaguar3::run_iqk(SelectedChannel channel) { * Every step is ported from vendor source. */ void HalJaguar3::rtw_hal_init(SelectedChannel channel) { ChannelWidth_t bw = channel.ChannelWidth; - InitTimer timer(_logger, "j3hal", [this] { return _device.ctrl_xfers(); }, + /* The transfer counter is a USB notion (xfers is emitted only when a + * counter is attached); a PCIe transport gets none, and its timer omits + * the field instead of reporting 0. */ + InitTimer timer(_logger, "j3hal", + _device.is_usb() ? InitTimer::XferCounter{[this] { return _device.ctrl_xfers(); }} + : InitTimer::XferCounter{}, [this] { _device.flush_writes(); }); _macinit.pre_init_system_cfg(); diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 2616c931..03a2554f 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -781,7 +781,12 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { } } brought_up_guard{_brought_up}; _brought_up = false; - InitTimer timer(_logger, "j3init", [this] { return _device.ctrl_xfers(); }, + /* The transfer counter is a USB notion (xfers is emitted only when a + * counter is attached); a PCIe transport gets none, and its timer omits + * the field instead of reporting 0. */ + InitTimer timer(_logger, "j3init", + _device.is_usb() ? InitTimer::XferCounter{[this] { return _device.ctrl_xfers(); }} + : InitTimer::XferCounter{}, [this] { _device.flush_writes(); }); WriteBatchScope batch(_device); _hal.rtw_hal_init(channel); /* full vendor-source bring-up */ From 7ff96572c07a19dcbfbd6fa38f0a8a976aa11930 Mon Sep 17 00:00:00 2001 From: Joseph <162703152+josephnef@users.noreply.github.com> Date: Thu, 17 Sep 2026 22:06:16 +0300 Subject: [PATCH 29/29] jaguar3: refuse InitWrite under a live RX loop; the coex stop flag is atomic A bring-up while the RX loop runs would open a single-threaded batch over a transport that loop's phydm worker still uses, and the loop belongs to the caller's thread, so InitWrite throws instead. _coex_stop is written by Stop, the destructor and now the re-init path while the coex loop reads it: std::atomic, not volatile. Co-Authored-By: Claude Fable 5.1 --- src/jaguar3/RtlJaguar3Device.cpp | 7 ++++++- src/jaguar3/RtlJaguar3Device.h | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/jaguar3/RtlJaguar3Device.cpp b/src/jaguar3/RtlJaguar3Device.cpp index 03a2554f..f15b143a 100644 --- a/src/jaguar3/RtlJaguar3Device.cpp +++ b/src/jaguar3/RtlJaguar3Device.cpp @@ -762,7 +762,12 @@ void RtlJaguar3Device::InitWrite(SelectedChannel channel) { _rx_wanted = want_rx; /* A second bring-up on a live device: the coex thread of the previous * one shares the transport, and the batch below is single-threaded by - * contract, so stop and join it before anything is queued. */ + * contract, so stop and join it before anything is queued. A running RX + * loop (and its phydm worker) cannot be stopped from here — that is the + * caller's thread — so it is refused outright. */ + if (_rx_loop_active.load()) + throw std::runtime_error( + "Jaguar3: InitWrite while the RX loop is running — stop it first"); if (_coex_thread.joinable()) { _coex_stop = true; _coex_thread.join(); diff --git a/src/jaguar3/RtlJaguar3Device.h b/src/jaguar3/RtlJaguar3Device.h index b2573898..fd4b3035 100644 --- a/src/jaguar3/RtlJaguar3Device.h +++ b/src/jaguar3/RtlJaguar3Device.h @@ -382,7 +382,7 @@ class RtlJaguar3Device : public IRtlRadio { * active (_rx_loop_active) the coex thread skips its C2H drain — the RX async * loop sees the C2H reports as part of its stream. */ std::thread _coex_thread; - volatile bool _coex_stop = false; + std::atomic _coex_stop{false}; /* written by Stop/~/re-init, read by the coex loop */ void coex_runtime_loop(); /* Nominal beacon interval in TU while a beacon is active (0 = none); the * AdjustBeaconTiming one-shot tweak restores to this. */