second audit sweep

.gitignore

@@ -159,3 +159,4 @@ HANDOVER_lr2021_audit.md
 # does not apply to paths already in the index (three of these went into
 # b839c2c for exactly that reason; untrack with `git rm -r --cached`).
 lr2021_bench/
+LR2021_MATH_AUDIT.md

zephcore/adapters/radio/LoRaRadioBase.cpp

@@ -65,7 +65,7 @@ LoRaRadioBase::LoRaRadioBase(const struct device *lora_dev, MainBoard &board,
 	  _override_sf(0), _override_cr(0),
 	  _rx_cb(nullptr), _rx_cb_user_data(nullptr),
 	  _tx_done_cb(nullptr), _tx_done_cb_user_data(nullptr),
-	  _tx_thread_running(false),
+	  _tx_thread_running(false), _tx_len(0),
 	  _packets_recv(0), _packets_sent(0), _packets_recv_errors(0)
 {
 	k_poll_signal_init(&_tx_signal);
@@ -76,6 +76,42 @@ LoRaRadioBase::LoRaRadioBase(const struct device *lora_dev, MainBoard &board,
 
 /* ── TX wait thread ──────────────────────────────────────────── */
 
+/* How long to wait for TX_DONE before declaring the transmit lost.
+ *
+ * TX_TIMEOUT_MS alone is a fixed 5 s, which is shorter than the airtime of a
+ * great many legal presets — a 255-byte packet is 28.6 s at SF12/BW62.5 and
+ * 10.2 s at SF9/BW31.25 — so the wait would expire mid-transmission and
+ * startReceive() would yank the radio out of TX, losing a packet that was
+ * transmitting perfectly well.  Scale from the driver's own airtime instead,
+ * with the same doubling the drivers' internal sync-send waits use.
+ *
+ * MAX(), never a bare replacement: on fast presets the scaled value is smaller
+ * than 5 s (SF7/BW62.5, 64 bytes: ~1.4 s), and shortening this deadline on the
+ * presets every radio in the fleet is running today would be a regression for
+ * no gain.  The floor keeps existing behaviour exactly; only slow presets move.
+ *
+ * lora_airtime() is a pure calculation on the cached modem config, so calling
+ * it from this thread costs no SPI and cannot race the radio. */
+uint32_t LoRaRadioBase::txWaitBudgetMs() const
+{
+	uint32_t air = lora_airtime(_dev, _tx_len ? _tx_len : 255);
+
+	/* All four drivers behind this class implement .airtime (native sx126x,
+	 * lr11xx, lr20xx, loramac-node sx127x), and lora_airtime() dereferences
+	 * the op without a NULL check, so there is no missing-op case to handle.
+	 * A zero can still come back from a degenerate modem config; fall back
+	 * to the flat budget rather than to no wait at all. */
+	if (air == 0) {
+		return TX_TIMEOUT_MS;
+	}
+	/* Cap the doubling before adding, so a pathological airtime cannot wrap
+	 * the 32-bit budget on its way into K_MSEC(). */
+	if (air > (UINT32_MAX - 1000U) / 2U) {
+		return UINT32_MAX - 1000U;
+	}
+	return MAX(TX_TIMEOUT_MS, 2U * air + 1000U);
+}
+
 void LoRaRadioBase::txWaitThreadFn(void *p1, void *p2, void *p3)
 {
 	LoRaRadioBase *self = static_cast<LoRaRadioBase *>(p1);
@@ -103,21 +139,43 @@ void LoRaRadioBase::txWaitThreadFn(void *p1, void *p2, void *p3)
 		int result;
 		k_poll_signal_check(&self->_tx_signal, &signaled, &result);
 		if (signaled) {
-			LOG_DBG("TX wait: signal already raised (result=%d)", result);
+			/* The result carries the driver's verdict, and it has to
+			 * be honoured: a driver that reports a failed transmit
+			 * by raising the signal with a negative result (the
+			 * SX126x does exactly this on a chip Tx timeout,
+			 * -ETIMEDOUT) was previously counted here as a
+			 * successful send.  Latent rather than live — that
+			 * SX126x path is unreachable while an RX callback is
+			 * registered, which it always is — but it is the reason
+			 * the LR11xx and LR20xx timeout handlers deliberately do
+			 * NOT raise the signal.  With the result honoured, a
+			 * driver reporting failure is now the correct thing to
+			 * do on all three. */
+			if (result < 0) {
+				LOG_ERR("TX wait: driver reported failure (%d) — packet lost",
+					result);
+			} else {
+				LOG_DBG("TX wait: signal already raised (result=%d)", result);
+			}
 			k_poll_signal_reset(&self->_tx_signal);
 			self->_board->onAfterTransmit();
 			self->startReceive();
 			atomic_set(&self->_tx_active, 0);
-			atomic_inc(&self->_packets_sent);
+			if (result >= 0) {
+				atomic_inc(&self->_packets_sent);
+			}
 			if (self->_tx_done_cb) {
 				self->_tx_done_cb(self->_tx_done_cb_user_data);
 			}
 			continue;
 		}
 
-		int ret = k_poll(events, 1, K_MSEC(TX_TIMEOUT_MS));
+		uint32_t budget_ms = self->txWaitBudgetMs();
+
+		int ret = k_poll(events, 1, K_MSEC(budget_ms));
 		if (ret == -EAGAIN) {
-			LOG_ERR("TX wait: TIMEOUT!");
+			LOG_ERR("TX wait: TIMEOUT after %u ms (len=%u) — packet lost",
+				budget_ms, (unsigned)self->_tx_len);
 			self->_board->onAfterTransmit();
 			self->startReceive();
 			atomic_set(&self->_tx_active, 0);
@@ -128,12 +186,25 @@ void LoRaRadioBase::txWaitThreadFn(void *p1, void *p2, void *p3)
 		}
 
 		if (ret == 0 && events[0].state == K_POLL_STATE_SIGNALED) {
+			/* Same rule as the already-raised path above: a negative
+			 * result is the driver reporting a lost transmit, not a
+			 * completed one. */
+			int sig_result = 0;
+			unsigned int sig_state = 0;
+
+			k_poll_signal_check(&self->_tx_signal, &sig_state,
+					    &sig_result);
 			k_poll_signal_reset(&self->_tx_signal);
 			self->_board->onAfterTransmit();
 			self->startReceive();
 			atomic_set(&self->_tx_active, 0);
-			atomic_inc(&self->_packets_sent);
-			LOG_INF("TX complete, RX restarted");
+			if (sig_result < 0) {
+				LOG_ERR("TX failed: driver reported %d — packet lost",
+					sig_result);
+			} else {
+				atomic_inc(&self->_packets_sent);
+				LOG_INF("TX complete, RX restarted");
+			}
 
 			if (self->_tx_done_cb) {
 				self->_tx_done_cb(self->_tx_done_cb_user_data);
@@ -693,6 +764,9 @@ bool LoRaRadioBase::startSendRaw(const uint8_t *bytes, int len)
 	configureTx();
 
 	memcpy(_tx_buf, bytes, len);
+	/* Published before the _tx_start_sem handoff below so txWaitBudgetMs()
+	 * sizes the wait for this packet, not the previous one. */
+	_tx_len = (uint16_t)len;
 	k_poll_signal_reset(&_tx_signal);
 
 	int ret = hwSendAsync(_tx_buf, (uint32_t)len, &_tx_signal);

zephcore/adapters/radio/LoRaRadioBase.h

@@ -329,9 +329,15 @@ private:
 
 	/* TX completion thread */
 	static void txWaitThreadFn(void *p1, void *p2, void *p3);
+	uint32_t txWaitBudgetMs() const;
 	struct k_thread _tx_wait_thread;
 	struct k_sem _tx_start_sem;
 	bool _tx_thread_running;
+	/* Length of the transmit in flight, for txWaitBudgetMs().  Written by
+	 * startSendRaw() before it releases _tx_start_sem, read by the wait
+	 * thread after it takes that sem — the sem is the handoff, so no
+	 * additional synchronisation is needed. */
+	uint16_t _tx_len;
 
 	/* Packet statistics */
 	atomic_t _packets_recv;

zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c

@@ -104,7 +104,14 @@ struct lr11xx_data {
 	void *cad_user_data;
 	struct k_sem cad_sem;
 	int cad_result;
-	bool cad_active;
+	/* No cad_active flag: there was one, written at four sites and read at
+	 * none.  The SX126x driver's copy IS read (sx126x_handle_irq_timeout()
+	 * returns early on it), so this one was inherited without the read that
+	 * gave it a purpose.  Not needed here: the TIMEOUT branch below is only
+	 * reachable during a CAD when the chip took the CAD->TX exit, and that
+	 * path sets tx_active first.  Exclusion between CADs comes from both
+	 * entry points (LBT from startSendRaw, probes from cadMaintenance)
+	 * running on the mesh loop thread. */
 	/* Adaptive-CAD: signed offset applied to the per-SF base detPeak on
 	 * every LBT CAD; cad_probe_peak overrides for one calibration probe. */
 	int8_t cad_peak_offset;
@@ -596,8 +603,6 @@ static void lr11xx_dio1_work_handler(struct k_work *work)
 	if (irq & LR11XX_SYSTEM_IRQ_CAD_DONE) {
 		bool detected = (irq & LR11XX_SYSTEM_IRQ_CAD_DETECTED) != 0;
 
-		data->cad_active = false;
-
 		if (data->cad_cb) {
 			lora_cad_cb cb = data->cad_cb;
 			void *ud = data->cad_user_data;
@@ -630,7 +635,23 @@ static void lr11xx_dio1_work_handler(struct k_work *work)
 
 	/* ── Timeout ── */
 	if (irq & LR11XX_SYSTEM_IRQ_TIMEOUT) {
-		if (!data->tx_active) {
+		if (data->tx_active) {
+			/* The chip's Tx timeout fired, so the transmission was
+			 * stopped and TX_DONE will never arrive.  This branch
+			 * used to do nothing at all here: no log, no recovery,
+			 * no signal — the radio simply sat in the post-TX state
+			 * until the host wait expired.  Mirror the TX_DONE path
+			 * so the receiver goes back on air, and say what
+			 * happened.  tx_signal is deliberately NOT raised: the
+			 * C++ wait thread treats a raised signal as a completed
+			 * send, so raising it here would book a packet that
+			 * never left.  Its own timeout owns the accounting. */
+			LOG_ERR("TX timeout — chip stopped the transmission, "
+				"packet lost");
+			data->tx_active = false;
+			lr11xx_start_rx(data, cfg);
+			rx_restarted = true;
+		} else {
 			lr11xx_restart_rx(data);
 			rx_restarted = true;
 		}
@@ -1114,10 +1135,10 @@ static uint32_t lr11xx_max_payload_ms(struct lr11xx_data *data)
 
 	/* Semtech payload-symbol count with PL=255, CRC on, explicit header,
 	 * CR = 4/8 (coded_bits = 8), DE = 1:
-	 *   n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - DE))) * 8
-	 * SF5/SF6 use SF-1 >= 4 so the divisor is always non-zero. */
+	 *   n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - 2*DE))) * 8
+	 * DE=1 so the divisor is 4*(SF-2); at SF5 that is 12, never zero. */
 	uint32_t numer = 8U * 255U + 28U + 16U;
-	uint32_t denom = 4U * (uint32_t)(sf - 1U);
+	uint32_t denom = 4U * (uint32_t)(sf - 2U);
 
 	if (numer > 4U * (uint32_t)sf) {
 		numer -= 4U * (uint32_t)sf;
@@ -1376,7 +1397,6 @@ static int lr11xx_do_cad(struct lr11xx_data *data)
 
 	lr11xx_system_clear_irq_status(ctx, LR11XX_SYSTEM_IRQ_ALL_MASK);
 	lr11xx_reset_rx_busy_signals(data);
-	data->cad_active = true;
 	lr11xx_radio_set_cad(ctx);
 
 	return 0;
@@ -1414,7 +1434,6 @@ static int lr11xx_lora_cad(const struct device *dev, k_timeout_t timeout)
 
 	ret = k_sem_take(&data->cad_sem, timeout);
 	if (ret == -EAGAIN) {
-		data->cad_active = false;
 		return -ETIMEDOUT;
 	}
 
@@ -1429,7 +1448,6 @@ static int lr11xx_lora_cad_async(const struct device *dev,
 	if (cb == NULL) {
 		data->cad_cb = NULL;
 		data->cad_user_data = NULL;
-		data->cad_active = false;
 		return 0;
 	}
 

zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c

@@ -107,7 +107,12 @@ struct lr20xx_data {
 	void *cad_user_data;
 	struct k_sem cad_sem;
 	int cad_result;	/* 0=free, 1=busy, <0=error */
-	bool cad_active;
+	/* No cad_active flag: there was one, written at five sites and read at
+	 * none.  It read like a concurrent-CAD guard and guarded nothing — the
+	 * real exclusion is that every CAD entry point (LBT from startSendRaw,
+	 * probes from cadMaintenance) runs on the mesh loop thread, so two CADs
+	 * cannot overlap by construction.  A flag nothing tests is worse than no
+	 * flag: it invites the next reader to assume the invariant is enforced. */
 	/* Adaptive-CAD: signed offset applied to the per-SF base detPeak on
 	 * every LBT CAD; cad_probe_peak overrides for one calibration probe. */
 	int8_t cad_peak_offset;
@@ -658,13 +663,40 @@ static lr20xx_status_t lr20xx_calibrate_front_end(void *ctx, uint32_t freq_hz)
 
 	for (int i = 0; i < LR20XX_MAX_CAL_ATTEMPTS; i++) {
 		lr20xx_system_errors_t errs = 0;
+		lr20xx_system_stat1_t s1 = { 0 };
+		bool rejected = false;
 		lr20xx_status_t rc;
 
 		lr20xx_system_clear_errors(ctx);
 
 		rc = lr20xx_radio_common_calibrate_front_end_helper(ctx, fe_cal, 3);
+
+		/* The SDK return code reflects the SPI write, not whether the chip
+		 * executed the command.  A CalibFE issued from Rx or Tx answers
+		 * CMD_FAIL (DS §6.4.2) while rc stays OK and no error bit is set,
+		 * so without this check the caller is told the front end was
+		 * calibrated when it was not — and the first symptom is degraded
+		 * RX with a RxFreqNoCalErr nobody connects to this call.  Must be
+		 * read BEFORE get_errors(): Stat reports the previous command.
+		 * DS Table 6-38: 0x2 CMD_OK, 0x3 CMD_DAT, anything else rejected. */
+		if (lr20xx_system_get_status(ctx, &s1, NULL, NULL) ==
+		    LR20XX_STATUS_OK) {
+			rejected = (s1.command_status != 2 &&
+				    s1.command_status != 3);
+		}
+
 		lr20xx_system_get_errors(ctx, &errs);
 
+		if (rejected) {
+			/* Not retried: the chip refused on mode grounds, and
+			 * repeating the command from the same mode cannot help. */
+			LOG_ERR("FE cal(%u Hz) REJECTED (cmd_status=%d) — CalibFE "
+				"needs STDBY_RC/XOSC/FS, not Rx or Tx (DS §6.4.2); "
+				"front end left uncalibrated",
+				freq_hz, s1.command_status);
+			return LR20XX_STATUS_ERROR;
+		}
+
 		if (rc == LR20XX_STATUS_OK &&
 		    !(errs & LR20XX_SYSTEM_ERRORS_SRC_SATURATION_CALIB_MASK)) {
 			/* Report the grid points actually programmed, not just
@@ -1390,7 +1422,6 @@ static void lr20xx_dio1_work_handler(struct k_work *work)
 		}
 
 		LOG_DBG("CAD done: %s", detected ? "activity" : "free");
-		data->cad_active = false;
 
 		if (data->cad_cb) {
 			lora_cad_cb cb = data->cad_cb;
@@ -1423,8 +1454,27 @@ static void lr20xx_dio1_work_handler(struct k_work *work)
 
 	/* ── Timeout ── */
 	if (irq & LR20XX_SYSTEM_IRQ_TIMEOUT) {
-		LOG_DBG("Timeout IRQ — restarting RX");
-		if (!data->tx_active) {
+		if (data->tx_active) {
+			/* The chip's Tx safeguard fired.  DS §6.3.6: "the
+			 * transmission is stopped prematurely" — the packet is
+			 * gone and TX_DONE will never arrive.  Leaving tx_active
+			 * set here (as this branch used to) made the whole event
+			 * invisible: no branch ran, nothing was logged above DBG,
+			 * and the radio sat in the post-TX state until the host's
+			 * own wait expired.  Say it and put the receiver back on
+			 * air, same as the TX_DONE path.
+			 *
+			 * tx_signal is deliberately NOT raised: the C++ wait
+			 * thread counts a raised signal as a completed send and
+			 * would increment packets_sent for a packet that never
+			 * left.  Its own timeout owns the accounting. */
+			LOG_ERR("TX timeout — chip stopped the transmission, "
+				"packet lost");
+			data->tx_active = false;
+			lr20xx_start_rx(data, cfg);
+			rx_restarted = true;
+		} else {
+			LOG_DBG("Timeout IRQ — restarting RX");
 			lr20xx_restart_rx(data);
 			rx_restarted = true;
 		}
@@ -1549,6 +1599,17 @@ static int lr20xx_lora_config(const struct device *dev,
 	 * ral_lr20xx_init() calibrates at init and never on the Tx/Rx path. */
 	k_mutex_lock(&data->spi_mutex, K_FOREVER);
 
+	/* Standby first — CalibFE "does not work if device is in Rx or Tx mode"
+	 * (DS §6.4.2).  On the reconfigure path the chip IS still in Rx: the C++
+	 * layer's hwCancelReceive() lowers lora_recv_async(NULL)'s bookkeeping
+	 * without commanding the chip, and continuous RX only ends when the host
+	 * changes the mode (DS §6.3.5).  Without this the calibration is refused
+	 * for every `set freq/sf/bw/cr`, tempradio set and tempradio clear, and
+	 * refused invisibly — see the note in lr20xx_calibrate_front_end().
+	 * Free at boot, where hw_init() has already left the chip in STDBY_RC. */
+	lr20xx_system_set_standby_mode(&data->hal_ctx,
+				       LR20XX_SYSTEM_STANDBY_MODE_RC);
+
 	lr20xx_calibrate_front_end(&data->hal_ctx, config->frequency);
 
 	DUMP_CHIP_STATE(data, "config-FEcal");
@@ -1631,6 +1692,16 @@ static uint32_t lr20xx_cad_timeout_ms(struct lr20xx_data *data)
 #define LR20XX_CAD_LBT_MAX_TX_TIMEOUT_STEPS 0x00FFFFFFU
 #define LR20XX_CAD_LBT_STEPS_PER_MS         32000U
 
+/* The OTHER time base on this chip.  SetTx/SetRx/SetRxDutyCycle timeouts are
+ * counted in 32.768 kHz RTC periods (DS §6.3.17: "expressed in periods of the
+ * 32.768kHz RTC"), not the 32 MHz periods cad_timeout uses above — the two sit
+ * three lines apart here precisely so nobody reaches for the wrong one.  Both
+ * fields are 24 bits, so the RTC one tops out at 16777215/32768 = 512 s. */
+#define LR20XX_RTC_FREQ_HZ                  32768U
+#define LR20XX_RTC_STEP_MAX                 0x00FFFFFFU
+/* Never shorten the Tx safeguard below what shipped before it was scaled. */
+#define LR20XX_TX_TIMEOUT_FLOOR_MS          5000U
+
 static int lr20xx_do_cad(struct lr20xx_data *data);
 
 static int lr20xx_lora_send_cad_lbt(const struct device *dev,
@@ -1705,7 +1776,6 @@ static int lr20xx_lora_send_cad_lbt(const struct device *dev,
 			 K_MSEC(lr20xx_cad_timeout_ms(data) + tx_timeout_ms));
 	if (ret == -EAGAIN) {
 		LOG_WRN("CAD_LBT: no CAD_DONE within budget — falling back");
-		data->cad_active = false;
 		ret = -EIO;
 		goto abort;
 	}
@@ -1785,8 +1855,19 @@ static int lr20xx_lora_send_async(const struct device *dev,
 	 * to re-arm. */
 	if (data->modem_cfg.cad.mode == LORA_CAD_MODE_LBT) {
 		bool was_in_rx = data->in_rx_mode;
-		int cad_ret = lr20xx_lora_cad(dev,
-					      K_MSEC(lr20xx_cad_timeout_ms(data)));
+		int cad_ret;
+
+		/* Discard any stamp left by a CAD that was not this transmit's.
+		 * The CAD_DONE handler stamps cad_done_cycles on every free CAD
+		 * — adaptive-CAD probes every ~15 s, and CAD_LBT, neither of
+		 * which consumes it — but the only reader is the latency log
+		 * below.  Without this the reported CAD->TX gap can be measured
+		 * from a probe that ran seconds earlier, which is worse than not
+		 * reporting it: it is a plausible number that is simply wrong. */
+		data->cad_done_cycles = 0U;
+
+		cad_ret = lr20xx_lora_cad(dev,
+					  K_MSEC(lr20xx_cad_timeout_ms(data)));
 		if (cad_ret > 0) {
 			LOG_DBG("LBT: channel busy");
 			/* Re-arm whenever there was anything to re-arm.  The
@@ -1829,7 +1910,6 @@ static int lr20xx_lora_send_async(const struct device *dev,
 		lr20xx_hardware_reset(data, cfg);
 	}
 
-
 	/* Clear errors before modem config */
 	lr20xx_system_clear_errors(ctx);
 	lr20xx_system_clear_irq_status(ctx, LR20XX_SYSTEM_IRQ_ALL_MASK);
@@ -1875,7 +1955,38 @@ static int lr20xx_lora_send_async(const struct device *dev,
 		data->cad_done_cycles = 0U;
 	}
 
-	lr20xx_radio_common_set_tx(ctx, 5000);
+	/* Chip-side Tx safeguard — DS §6.3.6: "Tx timeout can be used as a
+	 * safeguard in case transmission fails and TxDone interrupt never
+	 * occurs", and when it fires "the transmission is stopped prematurely".
+	 * So it has to exceed real airtime, and a fixed budget cannot: at
+	 * SF12/BW62.5 a 31-byte packet already needs more than 5 s, and at
+	 * SF12/BW31.25 so does a 6-byte one.  Scaled from the same airtime the
+	 * CAD_LBT path uses, floored at the previous 5000 ms so no preset that
+	 * works today gets a tighter deadline.
+	 *
+	 * Programmed in RTC steps rather than through the SDK's millisecond
+	 * wrapper: that wrapper computes `ms * 32768` in uint32 and overflows
+	 * above 131 071 ms, which is reachable here (SF12/BW7.81 at 255 bytes is
+	 * ~229 s of airtime).  Saturating at the 24-bit field maximum is 512 s,
+	 * comfortably past any airtime this chip can produce. */
+	{
+		uint32_t tx_air_ms = lr20xx_lora_airtime(dev, data_len);
+		uint32_t tx_tmo_ms = tx_air_ms + (tx_air_ms / 4U) + 500U;
+		uint64_t tx_tmo_steps;
+
+		if (tx_tmo_ms < LR20XX_TX_TIMEOUT_FLOOR_MS) {
+			tx_tmo_ms = LR20XX_TX_TIMEOUT_FLOOR_MS;
+		}
+		tx_tmo_steps = ((uint64_t)tx_tmo_ms * LR20XX_RTC_FREQ_HZ) / 1000U;
+		if (tx_tmo_steps > LR20XX_RTC_STEP_MAX) {
+			tx_tmo_steps = LR20XX_RTC_STEP_MAX;
+		}
+
+		LOG_DBG("SET_TX: airtime=%u ms, timeout=%u ms (%u steps)",
+			tx_air_ms, tx_tmo_ms, (uint32_t)tx_tmo_steps);
+		lr20xx_radio_common_set_tx_with_timeout_in_rtc_step(
+			ctx, (uint32_t)tx_tmo_steps);
+	}
 
 	/* Command status here is SET_TX's own (2=accepted, 1=rejected,
 	 * 0=not executed) and the mode should have left standby. */
@@ -2022,10 +2133,17 @@ static uint32_t lr20xx_max_payload_ms(struct lr20xx_data *data)
 
 	/* Semtech payload-symbol count with PL=255, CRC on, explicit header,
 	 * CR = 4/8 (coded_bits = 8), DE = 1:
-	 *   n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - DE))) * 8
-	 * SF5/SF6 use SF-1 >= 4 so the divisor is always non-zero. */
+	 *   n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - 2*DE))) * 8
+	 * The divisor is 4*(SF - 2*DE), not 4*(SF - DE) — same form
+	 * lr20xx_lora_airtime() uses.  With DE hard-coded to 1 that is 4*(SF-2),
+	 * which at SF5 is 12, so the divisor is always non-zero.
+	 *
+	 * DE = 1 unconditionally because this is a ceiling, and LDRO on yields
+	 * the larger symbol count: at SF12/BW62.5 it is 416 symbols against 384
+	 * with the wrong divisor, i.e. 27.3 s of real payload airtime that the
+	 * bound has to clear. */
 	uint32_t numer = 8U * 255U + 28U + 16U;
-	uint32_t denom = 4U * (uint32_t)(sf - 1U);
+	uint32_t denom = 4U * (uint32_t)(sf - 2U);
 
 	if (numer > 4U * (uint32_t)sf) {
 		numer -= 4U * (uint32_t)sf;
@@ -2468,7 +2586,6 @@ static int lr20xx_do_cad(struct lr20xx_data *data)
 	/* Clear any pending IRQ flags, then start CAD */
 	lr20xx_system_clear_irq_status(ctx, LR20XX_SYSTEM_IRQ_ALL_MASK);
 	lr20xx_reset_rx_busy_signals(data);
-	data->cad_active = true;
 	lr20xx_radio_lora_set_cad(ctx);
 	CHECK_CMD(ctx, "set_cad");
 
@@ -2481,6 +2598,14 @@ static int lr20xx_do_cad(struct lr20xx_data *data)
 	return 0;
 }
 
+/* Blocking CAD.  Leaves the chip in STANDBY on every exit — success, busy,
+ * timeout and error alike — and deliberately does NOT restore RX: the caller
+ * knows whether it is about to transmit (in which case re-entering RX would be
+ * undone immediately) or must go back on air.  Both current callers honour
+ * that: lr20xx_lora_send_async()'s LBT branch re-arms on the busy path, and
+ * LoRaRadioBase::cadMaintenance() calls startReceive() after every probe.  A
+ * third caller that forgets leaves the node deaf with nothing scheduled to
+ * recover it, so the contract is stated here rather than left to be inferred. */
 static int lr20xx_lora_cad(const struct device *dev, k_timeout_t timeout)
 {
 	struct lr20xx_data *data = dev->data;
@@ -2515,7 +2640,6 @@ static int lr20xx_lora_cad(const struct device *dev, k_timeout_t timeout)
 	/* Wait for DIO1 handler to signal CAD_DONE */
 	ret = k_sem_take(&data->cad_sem, timeout);
 	if (ret == -EAGAIN) {
-		data->cad_active = false;
 		return -ETIMEDOUT;
 	}
 
@@ -2528,10 +2652,16 @@ static int lr20xx_lora_cad_async(const struct device *dev,
 	struct lr20xx_data *data = dev->data;
 
 	if (cb == NULL) {
-		/* Cancel pending CAD */
+		/* Cancel pending CAD.  Under spi_mutex like every other write to
+		 * the CAD state: the DIO1 handler reads cad_cb to decide whether
+		 * to dispatch a callback, and this used to race it.  Dead today
+		 * (nothing in the tree calls lora_cad_async), which is exactly
+		 * why it is worth fixing now rather than when someone wires it
+		 * up and inherits an invisible race. */
+		k_mutex_lock(&data->spi_mutex, K_FOREVER);
 		data->cad_cb = NULL;
 		data->cad_user_data = NULL;
-		data->cad_active = false;
+		k_mutex_unlock(&data->spi_mutex);
 		return 0;
 	}
 

zephcore/patches/zephyr/0013-lora-sx126x-rx-latch-deadline.patch

@@ -83,10 +83,10 @@ index 822d0f5..00faca6 100644
 +
 +	/* Semtech payload-symbol count with PL=255, CRC on, explicit header,
 +	 * CR = 4/8 (coded_bits = 8), DE = 1:
-+	 *   n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - DE))) * 8
-+	 * SF5/SF6 use SF-1 >= 4 so the divisor is always non-zero. */
++	 *   n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - 2*DE))) * 8
++	 * DE=1 so the divisor is 4*(SF-2); at SF5 that is 12, never zero. */
 +	uint32_t numer = 8U * 255U + 28U + 16U;
-+	uint32_t denom = 4U * (uint32_t)(sf - 1U);
++	uint32_t denom = 4U * (uint32_t)(sf - 2U);
 +
 +	if (numer > 4U * (uint32_t)sf) {
 +		numer -= 4U * (uint32_t)sf;