From d6a78e5d928decdfcd9884a7b94ae0429fe1cc9b Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:56:08 +0200 Subject: [PATCH] upstream sweep --- zephcore/ARCHITECTURE.md | 3 + zephcore/adapters/ble/ZephyrBLE.cpp | 61 +++++++ .../adapters/radio/lr11xx/lr11xx_hal_zephyr.c | 86 +++++++-- .../adapters/radio/lr20xx/lr20xx_hal_zephyr.c | 166 ++++++++++++------ 4 files changed, 248 insertions(+), 68 deletions(-) diff --git a/zephcore/ARCHITECTURE.md b/zephcore/ARCHITECTURE.md index e6a0591..935cc4b 100644 --- a/zephcore/ARCHITECTURE.md +++ b/zephcore/ARCHITECTURE.md @@ -479,6 +479,7 @@ The custom `lr11xx_lora.c` driver handles several LR1110 firmware bugs: - **Header error**: Can shift buffer pointer → standby before RX restart - **DIO1 stuck HIGH**: 5-cycle detection → full hardware reset + recovery - **BUSY-high wedge**: a command racing the autonomous `SetRxDutyCycle` sleep phase can leave the chip BUSY-high with DIO1 low. No IRQ ever fires, so the event-driven driver never re-arms and the node goes permanently deaf. A wedge-recovery watchdog on its own work queue (`lr11xx_wedge`, `K_PRIO_COOP(7)`) checks every 3 s: after 12 s of DIO1 silence it polls the BUSY GPIO continuously for 250 ms, and a dwell with no low edge means a genuine wedge → hardware reset + RX restart. False-positive free by construction — a healthy chip, continuous or duty-cycled, always drops BUSY low within one cycle. Reads the GPIO only (no SPI), so it cannot itself disturb the chip or race the autonomous DC state machine. +- **Stale SPI reply read as data**: a read is two NSS windows (command, then answer) with a BUSY wait between them. `wait_on_busy()` returns immediately on a BUSY that reads low and cannot distinguish "command finished" from "BUSY has not risen yet", so the answer window can clock out the chip's default status / IRQ stream instead of the payload — silently, since the caller parses IRQ bits as a plausible short integer. `lr11xx_hal_read()` therefore checks the stat1 command-status byte it used to discard and re-issues the command (3 attempts) unless it reports `CMD_DATA`. Same guard in the LR2021 HAL; upstream MeshCore hit this as [PR #3261](https://github.com/meshcore-dev/MeshCore/pull/3261). - **RX duty cycle**: wired via `SetRxDutyCycle` MODE_RX, sized by the shared adapter math (same as SX126x). The earlier "broken, 23-40% loss" verdict was a window-sizing bug (over-sleep + no header budget), not a chip defect — default-off, HW-verify before production use. ### 5.5 SX127x and LR2021 Paths @@ -490,6 +491,7 @@ The custom `lr11xx_lora.c` driver handles several LR1110 firmware bugs: - **Front-end calibration is a point calibration, not a band.** `CalibFE` takes up to three individual frequencies (4 MHz steps, bit 15 = LF/HF), unlike the SX126x/LR11xx `CalibrateImage` freq1/freq2 band with datasheet-prescribed edges. It is issued only at config, after a hardware reset, and on AGC reset — never on the Tx/Rx path (DS §6.4.2 keeps the values on chip across retention sleep). Both 4 MHz neighbours of the operating frequency are calibrated, nearest first, because the SDK rounds the argument up where the chip's own default truncates down. - **Side detectors** (multi-SF receive) are LR2021-only; see `lr20xx_configure_side_detectors()`. Mutually exclusive with CAD, whose SF ordering constraint is the inverse. - **Per-packet frequency error** is decoded and accumulated (`get freqerr`) — diagnostic only, nothing acts on it. + - **Reads are status-checked.** The two-window read (command, BUSY wait, answer) can clock its second window before BUSY rises, in which case the chip streams status / IRQ instead of the payload — `GetRxPacketLength` then returns `irq[31:16]`, exactly 4 with `RX_DONE` set, and a real frame is read out of the FIFO at the wrong length. `lr20xx_spi_read_frame()` accepts an answer only when the stat1 header reports `CMD_DATA`, re-issuing the command otherwise (3 attempts). Safe to retry because the Rx FIFO pop is not on this path (`lr20xx_hal_direct_read_fifo()`, single window, structurally immune). #### 5.5.1 LR2021 driver design notes @@ -722,6 +724,7 @@ Full command reference with constraints and remote-admin restrictions: `Repeater - `CONFIG_BT_PRIVACY` **disabled** on nRF52840 / MG24: identity address is advertised directly; both iOS and Android work without RPA. Android's Flutter BLE plugin fails `connectGatt()` to RPA-advertised devices from app context. - `CONFIG_BT_PRIVACY` **enabled** on ESP32-S3 (`boards/common/esp32_common.conf`): the Espressif controller's privacy-OFF Secure-Connections path produces a MIC failure against iOS (HCI disconnect `0x3d` at encryption start). Privacy ON keeps the controller on its working SC path. `USE_IDENTITY` advertising preserves Android compatibility. Do **not** remove `USE_IDENTITY` while ESP32 privacy is on. - Pairing triggered reactively: phone hits ATT error 0x05 on secured attribute → initiates SMP pairing (Apple Accessory Design Guidelines §55 compliant — no proactive Security Request) +- **Unpaired-connection timeout (15 s)**: a connection that never reaches security L2 is disconnected. With `CONFIG_BT_MAX_CONN=1` the stack stops advertising while the slot is held, and the companion advertising watchdog skips any state where a connection exists — so a client that connects and never pairs (a scanner app left open, iOS routinely) otherwise makes the node invisible until it is power cycled. Every characteristic on both services is `*_AUTHEN`, so an unsecured connection can do nothing and the drop costs a legitimate client nothing. Armed in `connected()`, cancelled by `security_changed()` at L2+ and by `disconnected()`; the expiry handler reaches the connection via `bt_conn_foreach()` rather than `current_conn`, which belongs to the Bluetooth callback thread. - TX congestion control: queue (12 frames) + overflow buffer + retry + timeout watchdog - Fast/slow advertising switching with post-disconnect flap prevention - DLE (Data Length Extension) to 251 bytes diff --git a/zephcore/adapters/ble/ZephyrBLE.cpp b/zephcore/adapters/ble/ZephyrBLE.cpp index 9485be0..72d3afd 100644 --- a/zephcore/adapters/ble/ZephyrBLE.cpp +++ b/zephcore/adapters/ble/ZephyrBLE.cpp @@ -603,6 +603,55 @@ K_WORK_DELAYABLE_DEFINE(tx_drain_work, tx_drain_work_fn); K_WORK_DELAYABLE_DEFINE(overflow_retry_work, overflow_retry_work_fn); K_WORK_DELAYABLE_DEFINE(adv_slow_work, adv_slow_work_fn); +/* ========== Unpaired-connection timeout ========== + * + * A connection that never reaches L2 holds the node's only peripheral slot. + * With CONFIG_BT_MAX_CONN=1 Zephyr stops advertising while that slot is taken, + * and the companion's advertising watchdog (main_companion.cpp) deliberately + * skips any state where a connection exists — so a client that connects and + * never pairs makes the node invisible to everyone else until it is power + * cycled. A BLE scanner app left connected does it by accident; iOS does it + * routinely. Nothing else times the connection out: pairing here is reactive + * by design (Apple §55 — we never send a Security Request, we wait for the + * phone to hit ATT insufficient-authentication and start pairing itself), so + * "connected but idle forever" is a state the node otherwise accepts happily. + * + * Dropping it costs a legitimate client nothing: every characteristic on both + * services is *_AUTHEN, so an unsecured connection cannot read, write or + * subscribe to anything. The window has to cover discovery plus the phone's + * own pairing dialog — 15 s matches upstream MeshCore PR #3263, which measured + * a real unpaired connection being dropped at ~13 s. */ +#define BLE_SECURITY_TIMEOUT_MS 15000 + +static void sec_timeout_conn_cb(struct bt_conn *conn, void *user_data) +{ + ARG_UNUSED(user_data); + + if (bt_conn_get_security(conn) >= BT_SECURITY_L2) { + return; + } + + char addr[BT_ADDR_LE_STR_LEN]; + bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr)); + LOG_WRN("security timeout: %s unpaired after %d ms, disconnecting", + addr, BLE_SECURITY_TIMEOUT_MS); + + /* recycled() restarts advertising once the stack releases the slot. */ + bt_conn_disconnect(conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN); +} + +/* Runs on the system work queue; current_conn belongs to the Bluetooth + * callback thread. The connection is therefore reached through + * bt_conn_foreach(), which hands the callback a reference held for its + * duration, rather than by dereferencing current_conn across threads. */ +static void sec_timeout_work_fn(struct k_work *work) +{ + ARG_UNUSED(work); + bt_conn_foreach(BT_CONN_TYPE_LE, sec_timeout_conn_cb, NULL); +} + +K_WORK_DELAYABLE_DEFINE(sec_timeout_work, sec_timeout_work_fn); + /* ========== TX completion callback ========== */ /* @@ -720,6 +769,10 @@ static void connected(struct bt_conn *conn, uint8_t err) /* Cancel fast→slow transition — already connected, no need to switch */ k_work_cancel_delayable(&adv_slow_work); + /* Arm the unpaired-connection timeout. Cancelled by security_changed() + * at L2+, and by disconnected() whichever way the connection ends. */ + k_work_reschedule(&sec_timeout_work, K_MSEC(BLE_SECURITY_TIMEOUT_MS)); + /* DLE is NOT requested here — the phone may start a PHY update LL * procedure immediately, and BLE allows only one at a time. * DLE is deferred to le_phy_updated() (after PHY negotiation completes) @@ -748,6 +801,8 @@ static void disconnected(struct bt_conn *conn, uint8_t reason) bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr)); LOG_INF("disconnected: %s reason 0x%02x", addr, reason); + k_work_cancel_delayable(&sec_timeout_work); + if (conn == current_conn) { bt_conn_unref(current_conn); current_conn = NULL; @@ -803,6 +858,12 @@ static void security_changed(struct bt_conn *conn, bt_security_t level, enum bt_ } LOG_INF("%s level %u", addr, level); + /* Paired (or bonded reconnect) — the connection has earned its slot, + * so stand the unpaired-connection timeout down. */ + if (level >= BT_SECURITY_L2) { + k_work_cancel_delayable(&sec_timeout_work); + } + /* Enable TX when we have sufficient security (level 2+ = encrypted). * This is the ONLY place that sets ble_tx_ready — security_changed is * the authority. CCC subscription (secure_nus_ccc_changed) only kicks diff --git a/zephcore/adapters/radio/lr11xx/lr11xx_hal_zephyr.c b/zephcore/adapters/radio/lr11xx/lr11xx_hal_zephyr.c index ea14afc..33c42ca 100644 --- a/zephcore/adapters/radio/lr11xx/lr11xx_hal_zephyr.c +++ b/zephcore/adapters/radio/lr11xx/lr11xx_hal_zephyr.c @@ -216,6 +216,15 @@ lr11xx_hal_status_t lr11xx_hal_write(const void *context, const uint8_t *command return wait_on_busy(ctx); } +/* Command status field of the stat1 byte (SWDR001, bits [2:1]). + * 0 FAIL, 1 PERR, 2 CMD_OK, 3 CMD_DATA. The vendor decoder in lr11xx_system.c + * shifts without masking; mask here, the field is 2 bits. */ +#define LR11XX_STAT1_CMD_STATUS(b) (((b) >> 1) & 0x03) +#define LR11XX_CMD_STATUS_DATA 0x03 + +/* Attempts at the command-then-response read before giving up. */ +#define LR11XX_READ_ATTEMPTS 3 + lr11xx_hal_status_t lr11xx_hal_read(const void *context, const uint8_t *command, const uint16_t command_length, uint8_t *data, const uint16_t data_length) @@ -256,26 +265,67 @@ lr11xx_hal_status_t lr11xx_hal_read(const void *context, const uint8_t *command, return wait_on_busy(ctx); } - /* Phase 2: wait BUSY, then read response — LR11XX prepends one dummy status byte */ - if (check_device_ready(ctx) != LR11XX_HAL_STATUS_OK) { - return LR11XX_HAL_STATUS_ERROR; + /* Phase 2: wait BUSY, then read response — LR11XX prepends one stat1 byte. + * + * That byte is checked, not discarded. wait_on_busy() returns immediately + * on a BUSY that reads low and cannot tell "the command finished" from + * "BUSY has not risen yet", so this window can clock out the chip's default + * status / IRQ stream instead of the payload — see the long note in + * adapters/radio/lr20xx/lr20xx_hal_zephyr.c and MeshCore PR #3261. On this + * family the miss lands in lr11xx_radio_get_rx_buffer_status(), where the + * payload length and buffer offset both come out of IRQ bits: a plausible + * short integer, no error reported, and the frame is read out of the buffer + * at the wrong length or the wrong offset. + * + * CMD_DATA ("processed read, data is being transmitted instead of IRQ + * status") is the only correct status for this window; a reply produced by + * the race reports CMD_OK. Re-issuing the command is safe for every caller + * that reaches here — all are getters or address-based register reads. */ + uint8_t stat1 = 0; + + for (int attempt = 0; attempt < LR11XX_READ_ATTEMPTS; attempt++) { + if (attempt > 0) { + /* Recovery means re-issuing the command: the chip only streams the + * answer in the window that follows it. */ + gpio_pin_set_dt(&ctx->nss, 1); + ret = spi_write(ctx->spi_dev, &ctx->spi_cfg, &tx); + gpio_pin_set_dt(&ctx->nss, 0); + + if (ret < 0) { + LOG_ERR("SPI write (cmd retry) failed: %d", ret); + return LR11XX_HAL_STATUS_ERROR; + } + } + + if (check_device_ready(ctx) != LR11XX_HAL_STATUS_OK) { + return LR11XX_HAL_STATUS_ERROR; + } + + const struct spi_buf rx_bufs[] = { + { .buf = &stat1, .len = 1 }, /* leading status byte */ + { .buf = data, .len = data_length }, + }; + const struct spi_buf_set rx = { .buffers = rx_bufs, .count = 2 }; + + gpio_pin_set_dt(&ctx->nss, 1); + ret = spi_read(ctx->spi_dev, &ctx->spi_cfg, &rx); + gpio_pin_set_dt(&ctx->nss, 0); + + if (ret < 0) { + LOG_ERR("SPI read failed: %d", ret); + return LR11XX_HAL_STATUS_ERROR; + } + + if (LR11XX_STAT1_CMD_STATUS(stat1) == LR11XX_CMD_STATUS_DATA) { + return LR11XX_HAL_STATUS_OK; + } } - uint8_t dummy; - const struct spi_buf rx_bufs[] = { - { .buf = &dummy, .len = 1 }, /* discard leading status byte */ - { .buf = data, .len = data_length }, - }; - const struct spi_buf_set rx = { .buffers = rx_bufs, .count = 2 }; - - gpio_pin_set_dt(&ctx->nss, 1); - ret = spi_read(ctx->spi_dev, &ctx->spi_cfg, &rx); - gpio_pin_set_dt(&ctx->nss, 0); - - if (ret < 0) { - LOG_ERR("SPI read failed: %d", ret); - return LR11XX_HAL_STATUS_ERROR; - } + /* Out of attempts. Hand back what came in rather than failing the call: + * that is exactly the previous behaviour, so no existing caller regresses on + * a read whose status is legitimately something else. */ + LOG_WRN("read op=0x%04x: stat1=0x%02x (cmd_status=%u, want CMD_DATA) after %d attempts", + last_opcode, stat1, LR11XX_STAT1_CMD_STATUS(stat1), LR11XX_READ_ATTEMPTS); return LR11XX_HAL_STATUS_OK; } diff --git a/zephcore/adapters/radio/lr20xx/lr20xx_hal_zephyr.c b/zephcore/adapters/radio/lr20xx/lr20xx_hal_zephyr.c index cf828b8..f45238f 100644 --- a/zephcore/adapters/radio/lr20xx/lr20xx_hal_zephyr.c +++ b/zephcore/adapters/radio/lr20xx/lr20xx_hal_zephyr.c @@ -252,75 +252,141 @@ lr20xx_hal_status_t lr20xx_hal_write(const void *context, const uint8_t *command #define LR20XX_STAT_LEN 2 /** - * @brief Clock one command-and-response frame inside a single NSS assertion. + * @brief Clock a command and read back its response, as two NSS windows. * - * The LR2021 streams the whole answer within the NSS window that carried the - * command: + * The command goes out in its own window. The chip raises BUSY while it + * prepares the answer, and a second window clocks that answer out: * - * MOSI [ command_length ][ LR20XX_STAT_LEN + data_length ] - * MISO [ undefined ][ stat header ][ payload ] + * window 1 MOSI [ command_length ] MISO [ undefined ] + * -- BUSY high while the chip prepares the answer -- + * window 2 MOSI [ LR20XX_STAT_LEN + data_length ] + * MISO [ stat header ][ payload ] * - * Releasing NSS between the command and the payload ends the frame, so the - * following read starts a fresh one and the chip answers it with the status / - * IRQ word rather than the payload the caller asked for. That misread is not - * obviously wrong at the call site — it is a plausible-looking short integer — - * so it surfaces as nonsense lengths and all-zero status reads rather than as - * an SPI error. Keep every read in one transaction. + * This mirrors Semtech's reference HAL (lr20xx_hal_read() in LoRa Basics + * Modem): command out, NSS released, wait on BUSY, then two dummy bytes and the + * payload in a fresh window. The Rx FIFO pop is the one read on this chip that + * does NOT work this way — single window, no stat header, no BUSY wait; see + * lr20xx_hal_direct_read_fifo(). + * + * The BUSY wait between the two windows is load-bearing, and the failure it + * guards is silent rather than loud: clock the second window too early and the + * chip answers with its default status / IRQ stream instead of the payload, + * which the caller then parses as a plausible-looking short integer rather than + * reporting an SPI error. The status check at the bottom of this function is + * the backstop for the residual case where BUSY has not yet risen when the wait + * samples it. * * NULL TX buffers clock the SPI controller's over-read character, NOT zero -- * Nordic defaults it to 0xff, which the LR2021 reads as a bogus opcode and * rejects. Boards using this driver must set overrun-character = <0x00> (the * LR2021 NOP) on the SPI node. */ +/* Command status field of the stat1 header (DS Table 6-38, bits [2:1]). + * 0 FAIL, 1 PERR, 2 CMD_OK, 3 CMD_DATA. The vendor decoder in + * lr20xx_system.c shifts without masking; mask here, the field is 2 bits. */ +#define LR20XX_STAT1_CMD_STATUS(b) (((b) >> 1) & 0x03) +#define LR20XX_CMD_STATUS_DATA 0x03 + +/* Attempts at the two-window read before giving up. Upstream MeshCore + * measured recovery on the first retry in every observed case (4/4 on LR11xx, + * 11/11 on LR2021, with the BUSY wait skipped on purpose). */ +#define LR20XX_READ_ATTEMPTS 3 + static int lr20xx_spi_read_frame(struct lr20xx_hal_context *ctx, const uint8_t *command, uint16_t command_length, uint8_t *data, uint16_t data_length) { - int ret; + int ret = 0; + uint8_t stat[LR20XX_STAT_LEN]; - /* Phase 1: the command, in its own NSS window. */ - { - const struct spi_buf tx_buf = { - .buf = (uint8_t *)command, - .len = command_length, - }; - const struct spi_buf_set tx = { .buffers = &tx_buf, .count = 1 }; + /* Retried as a whole: the chip only streams the answer in the window + * that follows its command, so recovering means re-issuing the command. + * Safe for every caller that lands here — lr20xx_hal_read() serves + * getters and address-based register reads, all idempotent. The one + * read that is NOT idempotent, the Rx FIFO pop, does not come through + * this path at all (lr20xx_hal_direct_read_fifo(), single NSS window, + * no stat header, no BUSY wait — structurally immune to the race + * below). */ + for (int attempt = 0; attempt < LR20XX_READ_ATTEMPTS; attempt++) { + /* Phase 1: the command, in its own NSS window. */ + { + const struct spi_buf tx_buf = { + .buf = (uint8_t *)command, + .len = command_length, + }; + const struct spi_buf_set tx = { .buffers = &tx_buf, .count = 1 }; - gpio_pin_set_dt(&ctx->nss, 1); - ret = spi_write(ctx->spi_dev, &ctx->spi_cfg, &tx); - gpio_pin_set_dt(&ctx->nss, 0); + gpio_pin_set_dt(&ctx->nss, 1); + ret = spi_write(ctx->spi_dev, &ctx->spi_cfg, &tx); + gpio_pin_set_dt(&ctx->nss, 0); - if (ret < 0) { + if (ret < 0) { + return ret; + } + } + + /* The answer is not ready until BUSY drops — without this the second + * window clocks out whatever the chip had, two bytes early. */ + if (wait_on_busy(ctx) != LR20XX_HAL_STATUS_OK) { + return -ETIMEDOUT; + } + + /* Phase 2: two dummy bytes absorb the stat header, then the payload. + * The stat header lands in a real buffer rather than being discarded + * with a NULL one: spi_nrfx_spim rejects a transfer whose first TX and + * RX buffers are both NULL (-EINVAL), and TX is legitimately NULL here. */ + { + const struct spi_buf tx_buf = { + .buf = NULL, + .len = LR20XX_STAT_LEN + data_length, + }; + const struct spi_buf rx_bufs[] = { + { .buf = stat, .len = LR20XX_STAT_LEN }, + { .buf = data, .len = data_length }, + }; + const struct spi_buf_set tx = { .buffers = &tx_buf, .count = 1 }; + const struct spi_buf_set rx = { .buffers = rx_bufs, .count = 2 }; + + gpio_pin_set_dt(&ctx->nss, 1); + ret = spi_transceive(ctx->spi_dev, &ctx->spi_cfg, &tx, &rx); + gpio_pin_set_dt(&ctx->nss, 0); + + if (ret < 0) { + return ret; + } + } + + /* Did the chip actually answer US? + * + * wait_on_busy() returns immediately on a BUSY that reads low, and + * cannot tell "the command finished" from "BUSY has not risen yet". + * The gap between releasing NSS above and sampling the pin is two + * GPIO calls, so losing that race is entirely possible; RadioLib + * loses it with a whole microsecond of margin (MeshCore PR #3261). + * When it is lost, phase 2 clocks out the chip's default + * [stat 2B][irq 4B] stream instead of the payload, stat[] absorbs + * the two status bytes, and the caller parses IRQ bits as its + * answer. Nothing reports an error: for GetRxPacketLength that is + * irq[31:16], which with RX_DONE (bit 18) set is exactly 4, so a + * real frame of any size is read out of the FIFO as a 4-byte one + * and silently dropped upstream as corrupt. + * + * The status separates the two cleanly. CMD_DATA means "a read was + * processed and data is being transmitted instead of IRQ status" — + * the only correct answer for this window. A reply produced by the + * race reports CMD_OK instead, i.e. the chip is streaming status, + * not data. The Rx FIFO is untouched at this point, so re-reading + * recovers the frame rather than losing it. */ + if (LR20XX_STAT1_CMD_STATUS(stat[0]) == LR20XX_CMD_STATUS_DATA) { return ret; } } - /* The answer is not ready until BUSY drops — without this the second - * window clocks out whatever the chip had, two bytes early. */ - if (wait_on_busy(ctx) != LR20XX_HAL_STATUS_OK) { - return -ETIMEDOUT; - } - - /* Phase 2: two dummy bytes absorb the stat header, then the payload. - * The stat header lands in a real buffer rather than being discarded - * with a NULL one: spi_nrfx_spim rejects a transfer whose first TX and - * RX buffers are both NULL (-EINVAL), and TX is legitimately NULL here. */ - { - uint8_t stat[LR20XX_STAT_LEN]; - const struct spi_buf tx_buf = { - .buf = NULL, - .len = LR20XX_STAT_LEN + data_length, - }; - const struct spi_buf rx_bufs[] = { - { .buf = stat, .len = LR20XX_STAT_LEN }, - { .buf = data, .len = data_length }, - }; - const struct spi_buf_set tx = { .buffers = &tx_buf, .count = 1 }; - const struct spi_buf_set rx = { .buffers = rx_bufs, .count = 2 }; - - gpio_pin_set_dt(&ctx->nss, 1); - ret = spi_transceive(ctx->spi_dev, &ctx->spi_cfg, &tx, &rx); - gpio_pin_set_dt(&ctx->nss, 0); - } + /* Out of attempts. Hand back what came in rather than failing the call: + * that is exactly today's behaviour, so no existing caller regresses on a + * read whose status is legitimately something else. Tighten to an error + * return only once hardware confirms every read reports CMD_DATA. */ + LOG_WRN("read op=0x%04x: stat1=0x%02x (cmd_status=%u, want CMD_DATA) after %d attempts", + last_opcode, stat[0], LR20XX_STAT1_CMD_STATUS(stat[0]), LR20XX_READ_ATTEMPTS); return ret; }