From d7cc4cbe1da4cdd78d7890ae7b33e94cdf5b5d8c Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Tue, 22 Sep 2026 22:03:47 +0200 Subject: [PATCH] Return DIO1 to the digital GPIO block after every light sleep --- zephcore/adapters/radio/LoRaRadioBase.cpp | 20 +++ .../zephyr/0003-lora-sx126x-native.patch | 150 +++++++++++++++++- zephcore/src/Dispatcher.cpp | 20 ++- 3 files changed, 185 insertions(+), 5 deletions(-) diff --git a/zephcore/adapters/radio/LoRaRadioBase.cpp b/zephcore/adapters/radio/LoRaRadioBase.cpp index bb1f226..0b64db5 100644 --- a/zephcore/adapters/radio/LoRaRadioBase.cpp +++ b/zephcore/adapters/radio/LoRaRadioBase.cpp @@ -5,6 +5,7 @@ #include "LoRaRadioBase.h" #include "radio_common.h" +#include "pm_sleep_guard.h" #include #include /* MAX_TRANS_UNIT */ #include @@ -139,6 +140,13 @@ void LoRaRadioBase::txWaitThreadFn(void *p1, void *p2, void *p3) for (;;) { k_sem_take(&self->_tx_start_sem, K_FOREVER); + /* startSendRaw() took one sleep lock for the transmit it just + * handed over; it is released when this iteration ends, on + * whichever of the paths below it takes. */ + struct TxSleepRelease { + ~TxSleepRelease() { zc_pm_unblock_sleep(); } + } tx_sleep_release; + if (!atomic_get(&self->_tx_active)) { continue; } @@ -806,6 +814,17 @@ bool LoRaRadioBase::startSendRaw(const uint8_t *bytes, int len) * out, never to this one -- upstream's STATE_IDLE reset in the same * place. */ atomic_set(&_tx_complete, 0); + + /* No SoC light sleep from here until the wait thread has concluded this + * transmit (it releases the lock), or until the failure path below. + * Covers the blocking LBT CAD inside hwSendAsync() as well as the + * airtime. With nothing else runnable during a transmit, an ESP32 + * repeater otherwise sleeps through it and meets TX_DONE as a wake + * event rather than an interrupt, and the completion lands late or not + * at all (see the EXT1 note in the sx126x HAL). Costs nothing worth + * counting: the PA dominates the current draw while keyed. A no-op + * without CONFIG_PM. */ + zc_pm_block_sleep(); _board->onBeforeTransmit(); _last_tx_start_ms = k_uptime_get_32(); @@ -848,6 +867,7 @@ bool LoRaRadioBase::startSendRaw(const uint8_t *bytes, int len) } _board->onAfterTransmit(); atomic_set(&_tx_active, 0); + zc_pm_unblock_sleep(); /* startReceive() is safe to call here regardless of failure * cause: on SX126x, recv_async early-returns if the driver * already restored RX on CAD-busy (Phase 2 idempotent fast diff --git a/zephcore/patches/zephyr/0003-lora-sx126x-native.patch b/zephcore/patches/zephyr/0003-lora-sx126x-native.patch index a2278dd..744d3c4 100644 --- a/zephcore/patches/zephyr/0003-lora-sx126x-native.patch +++ b/zephcore/patches/zephyr/0003-lora-sx126x-native.patch @@ -124,6 +124,36 @@ above that (xiao_esp32s3 GPIO39, station_g2 GPIO48, thinknode_m9 GPIO42) log "Pin N is not wakeup capable" and cannot use light sleep with LoRa RX at all. Heltec V3/V4/V43 and the Wireless Trackers put DIO1 on GPIO14 and are fine. +== Return DIO1 to the digital GPIO block after every light sleep == + +The wake above is only half of what EXT1 needs. Before each light sleep, +ESP-IDF's ext1_wakeup_prepare() (esp_hw_support/sleep_modes.c) routes the +wake pad to the RTC IO mux (RTCIO_LL_FUNC_RTC) and, with RTC_PERIPH powered +down, engages its hold. Nothing on the way out undoes either: +esp_light_sleep_start() leaves both, and Zephyr's esp32_sleep_gpio_restore() +releases only pins flagged ESP32_GPIO_SLEEP_HOLD_EN. Espressif puts the +obligation on the caller -- sleep_modes.rst, EXT1 warnings: rtc_gpio_deinit() +before using the pad as a digital GPIO again, rtc_gpio_hold_dis() after a +light-sleep wake. Nothing called either, so from the first sleep on DIO1 sat +on the RTC mux, held. + +sx126x_hal.c now registers a pm_notifier (CONFIG_PM on +SOC_FAMILY_ESPRESSIF_ESP32 only) whose state_exit does both, and then checks +the DIO1 level. The edge that woke the SoC happened while the pad belonged to +the RTC domain, so the digital edge detector never saw it; SX126x IRQs are +sticky, so DIO1 still being high means an IRQ is pending and the handler is run +by hand. Gated on the level, not unconditional: the handler reads IRQ status +over SPI, and an NSS edge in the duty-cycle sleep phase disturbs the chip's own +RX cycle. A duplicate kick is harmless (k_work_submit on a queued item is a +no-op; a handler that finds the status clear does nothing). + +Symptom it addresses: on the allowlisted Heltec repeaters a TX_DONE that lands +while the SoC sleeps -- the common case, nothing else runs during airtime -- is +lost; the adapter's wait thread times out and the packet, which did go out, +is counted nowhere (reported on a Heltec V4.3 as "sent never increments", +1.17.4/1.17.5c). The C++ adapter additionally holds a PM lock across each +transmit. Not yet verified on hardware. + == Bound the lifetime of the RX-busy latch (payload-phase deadline) == sx126x_is_receiving() reports "busy" for the whole payload phase from the @@ -3611,10 +3641,108 @@ index 0122958c3..9d39100a7 100644 #endif /* ZEPHYR_DRIVERS_LORA_SX126X_SX126X_INTERNAL_H_ */ diff --git a/drivers/lora/native/sx126x/sx126x_hal.c b/drivers/lora/native/sx126x/sx126x_hal.c -index 34f708958..394578956 100644 +index 34f708958..c21085e37 100644 --- a/drivers/lora/native/sx126x/sx126x_hal.c +++ b/drivers/lora/native/sx126x/sx126x_hal.c -@@ -150,7 +150,13 @@ int sx126x_hal_init(const struct device *dev) +@@ -10,6 +10,12 @@ + + #include "sx126x.h" + ++#if defined(CONFIG_PM) && defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32) ++#include ++#include ++#define SX126X_HAL_ESP32_EXT1_WAKE 1 ++#endif ++ + #include + LOG_MODULE_REGISTER(sx126x_hal, CONFIG_LORA_LOG_LEVEL); + +@@ -107,6 +113,84 @@ void sx126x_hal_dio1_irq_enable(const struct device *dev) + ARG_UNUSED(dev); + } + ++#ifdef SX126X_HAL_ESP32_EXT1_WAKE ++/* Hand DIO1 back to the digital GPIO block after every light sleep. ++ * ++ * GPIO_INT_WAKEUP (sx126x_hal_init) makes the Zephyr ESP32 GPIO driver arm ++ * DIO1 as an EXT1 wake source. Before each light sleep, ESP-IDF's ++ * ext1_wakeup_prepare() then routes the pad to the RTC IO mux ++ * (RTCIO_LL_FUNC_RTC) and, because RTC_PERIPH is powered down, puts it in ++ * hold. Nothing on the way out undoes either: esp_light_sleep_start() leaves ++ * both in place, and Zephyr's esp32_sleep_gpio_restore() only releases pins ++ * carrying ESP32_GPIO_SLEEP_HOLD_EN. Espressif documents the obligation on ++ * the caller (sleep_modes.rst, EXT1 warnings): call rtc_gpio_deinit() before ++ * using the pad as a digital GPIO again, and rtc_gpio_hold_dis() after a ++ * light-sleep wake. Nobody did, so from the first sleep on DIO1 sat on the ++ * RTC mux with its hold engaged. ++ * ++ * Restoring the pad is not enough on its own. The DIO1 edge that woke us ++ * happened while the pad belonged to the RTC domain, so the digital edge ++ * detector never saw it and the interrupt is gone. An SX126x IRQ is sticky ++ * -- DIO1 stays asserted until ClearIrqStatus -- so the level is the ++ * authoritative record: if DIO1 is high after the restore, an IRQ is pending ++ * and the handler has to be run by hand. Without it a TX_DONE that lands ++ * while the SoC sleeps (the common case: nothing else runs during airtime) ++ * is lost -- the C++ adapter's wait thread times out and the dispatcher never ++ * counts a packet that did go out on air -- and an RX_DONE waits until ++ * something else clears the chip's IRQ status. ++ * ++ * Deliberately gated on the level rather than kicking the handler on every ++ * wake: the handler reads IRQ status over SPI, and an NSS edge during the ++ * duty-cycle sleep phase disturbs the chip's own RX cycle. A duplicate kick ++ * when the digital side did catch the edge is harmless -- k_work_submit on a ++ * queued item is a no-op, and a handler that finds the status already ++ * cleared does nothing. ++ * ++ * Runs from pm_state_notify(false), which may be in the waking ISR; every ++ * call here is ISR-safe. One radio per board, so a single static device. */ ++static const struct device *sx126x_pm_wake_dev; ++ ++static int sx126x_hal_dio1_gpio_num(const struct sx126x_hal_config *config) ++{ ++#if DT_NODE_EXISTS(DT_NODELABEL(gpio1)) ++ if (config->dio1.port == DEVICE_DT_GET(DT_NODELABEL(gpio1))) { ++ return 32 + config->dio1.pin; ++ } ++#endif ++ return config->dio1.pin; ++} ++ ++static void sx126x_hal_pm_state_exit(enum pm_state state) ++{ ++ const struct device *dev = sx126x_pm_wake_dev; ++ const struct sx126x_hal_config *config; ++ struct sx126x_hal_data *data; ++ int num; ++ ++ if (state != PM_STATE_STANDBY || dev == NULL) { ++ return; ++ } ++ ++ config = dev->config; ++ data = get_hal_data(dev); ++ num = sx126x_hal_dio1_gpio_num(config); ++ if (!rtc_gpio_is_valid_gpio(num)) { ++ return; ++ } ++ ++ (void)rtc_gpio_hold_dis(num); ++ (void)rtc_gpio_deinit(num); ++ ++ if (data->dio1_callback != NULL && gpio_pin_get_dt(&config->dio1) > 0) { ++ data->dio1_callback(dev); ++ } ++} ++ ++static struct pm_notifier sx126x_pm_notifier = { ++ .state_exit = sx126x_hal_pm_state_exit, ++}; ++#endif /* SX126X_HAL_ESP32_EXT1_WAKE */ ++ + int sx126x_hal_init(const struct device *dev) + { + const struct sx126x_hal_config *config = dev->config; +@@ -150,7 +234,13 @@ int sx126x_hal_init(const struct device *dev) LOG_ERR("DIO1 GPIO not ready"); return -ENODEV; } @@ -3629,7 +3757,21 @@ index 34f708958..394578956 100644 if (ret < 0) { LOG_ERR("Failed to configure DIO1 GPIO: %d", ret); return ret; -@@ -183,6 +189,14 @@ int sx126x_hal_init(const struct device *dev) +@@ -164,6 +254,13 @@ int sx126x_hal_init(const struct device *dev) + return ret; + } + ++#ifdef SX126X_HAL_ESP32_EXT1_WAKE ++ if (sx126x_pm_wake_dev == NULL) { ++ sx126x_pm_wake_dev = dev; ++ pm_notifier_register(&sx126x_pm_notifier); ++ } ++#endif ++ + /* Configure optional GPIOs */ + ret = sx126x_hal_configure_gpio(&config->antenna_enable, GPIO_OUTPUT_INACTIVE, + "antenna enable"); +@@ -183,6 +280,14 @@ int sx126x_hal_init(const struct device *dev) return ret; } @@ -3644,7 +3786,7 @@ index 34f708958..394578956 100644 LOG_DBG("HAL initialized"); return 0; } -@@ -220,15 +234,54 @@ int sx126x_hal_configure_tx_params(const struct device *dev, int8_t power, +@@ -220,15 +325,54 @@ int sx126x_hal_configure_tx_params(const struct device *dev, int8_t power, } tx_power = CLAMP(power, SX1261_MIN_POWER, SX1261_MAX_POWER_TX_PARAM); } else { diff --git a/zephcore/src/Dispatcher.cpp b/zephcore/src/Dispatcher.cpp index c97adbb..c44c79b 100644 --- a/zephcore/src/Dispatcher.cpp +++ b/zephcore/src/Dispatcher.cpp @@ -130,6 +130,13 @@ uint32_t Dispatcher::getTxStarvationDuration() const void Dispatcher::loop() { if (outbound) { + /* Read BEFORE isSendComplete(): the radio publishes its + * completion before it drops isTxActive(), so a "not active" + * seen here guarantees a successful transmit is already latched + * and collected by the call below. The other order can see the + * latch still empty, then the flag already clear, and abandon a + * transmit that finished in between. */ + bool tx_active = _radio->isTxActive(); if (_radio->isSendComplete()) { /* Airtime is the modulation time of the packet that just * went out, not the wall-clock width of the send. @@ -170,7 +177,18 @@ void Dispatcher::loop() } releasePacket(outbound); outbound = nullptr; - } else if (millisHasNowPassed(outbound_expiry)) { + } else if (!tx_active && millisHasNowPassed(outbound_expiry)) { + /* Give up only once the radio itself has concluded. Upstream + * abandons at outbound_expiry alone because RadioLib has no + * watchdog of its own; ours does — the TX wait thread always + * ends with isTxActive() false, on a timeout of + * max(TX_TIMEOUT_MS, 2 x airtime + 1 s). Abandoning while it + * still runs raced that thread: a completion published after + * the expiry was then wiped by the next startSendRaw(), so a + * packet that went out on air was counted nowhere — neither + * sent_flood/direct nor the radio's total. Radios that do not + * implement isTxActive() report false and keep the upstream + * behaviour exactly. */ _radio->onSendFinished(); logTxFail(outbound, 2 + outbound->getPathByteLen() + outbound->payload_len); releasePacket(outbound);