From 92979706d7dcaa5b5e962ff6382bdfa153f4b99f Mon Sep 17 00:00:00 2001 From: Zhou Xiao Date: Tue, 7 Apr 2026 14:35:50 +0800 Subject: [PATCH 1/5] fix(ble_log): handle scheduler-suspended state in ble_log_rt_queue_trans During light sleep transitions, the FreeRTOS scheduler is suspended but xPortInIsrContext() returns false, causing xQueueSend with portMAX_DELAY to hit a configASSERT. Add a check for taskSCHEDULER_SUSPENDED and use a non-blocking send with rollback on queue-full to avoid resource leaks. --- components/bt/common/ble_log/src/ble_log_rt.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/components/bt/common/ble_log/src/ble_log_rt.c b/components/bt/common/ble_log/src/ble_log_rt.c index c055dcd0fc..b2aabe6c80 100644 --- a/components/bt/common/ble_log/src/ble_log_rt.c +++ b/components/bt/common/ble_log/src/ble_log_rt.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -185,8 +185,15 @@ BLE_LOG_IRAM_ATTR void ble_log_rt_queue_trans(ble_log_prph_trans_t **trans) if (BLE_LOG_IN_ISR()) { BaseType_t woken = pdFALSE; + /* Queue depth == total transport buffer count; queue-full is impossible + * for a valid transport, so the return value is not checked. */ xQueueSendFromISR(rt_queue_handle, trans, &woken); portYIELD_FROM_ISR(woken); + } else if (xTaskGetSchedulerState() == taskSCHEDULER_SUSPENDED) { + /* Non-blocking send to avoid configASSERT when scheduler is suspended + * (e.g., during light sleep transitions). Queue-full is impossible; + * see comment above. */ + xQueueSend(rt_queue_handle, trans, 0); } else { xQueueSend(rt_queue_handle, trans, portMAX_DELAY); } From 3f8cfc5b1d2221c60d52ec22c32c043bef06e1e2 Mon Sep 17 00:00:00 2001 From: Zhou Xiao Date: Tue, 7 Apr 2026 15:31:32 +0800 Subject: [PATCH 2/5] fix(ble_log): fix crash and resource issues in peripheral drivers - Handle scheduler-suspended and ISR context in UART redirect path to prevent xSemaphoreTake crash during light sleep (C1) - Check uart_driver_install return value before setting inited flag (M1) - Always clean up SPI device handle in deinit even if acquire_bus fails (M4) --- components/bt/common/ble_log/src/ble_log_lbm.c | 11 ++++++++++- .../ble_log/src/prph/ble_log_prph_spi_master_dma.c | 5 ++--- .../common/ble_log/src/prph/ble_log_prph_uart_dma.c | 9 +++++++-- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/components/bt/common/ble_log/src/ble_log_lbm.c b/components/bt/common/ble_log/src/ble_log_lbm.c index 9b1dd64c92..7bed717b2c 100644 --- a/components/bt/common/ble_log/src/ble_log_lbm.c +++ b/components/bt/common/ble_log/src/ble_log_lbm.c @@ -515,6 +515,14 @@ bool ble_log_enable(bool enable) void ble_log_flush(void) { + /* Prevent concurrent flush — two concurrent callers would deadlock on + * the ref_count spin-wait (both hold a ref, both wait for ref_count <= 1). + * Second caller returns immediately instead of deadlocking. */ + static volatile bool flush_in_progress = false; + if (__atomic_test_and_set(&flush_in_progress, __ATOMIC_ACQUIRE)) { + return; + } + BLE_LOG_REF_COUNT_ACQUIRE(&lbm_ref_count); if (!lbm_inited) { goto deref; @@ -535,7 +543,7 @@ void ble_log_flush(void) bool lbm_enabled_copy = lbm_enabled; lbm_enabled = false; uint32_t time_waited = 0; - while (lbm_ref_count > 1) { + while (__atomic_load_n(&lbm_ref_count, __ATOMIC_ACQUIRE) > 1) { vTaskDelay(pdMS_TO_TICKS(1)); BLE_LOG_ASSERT(time_waited++ < 1000); } @@ -593,6 +601,7 @@ void ble_log_flush(void) deref: BLE_LOG_REF_COUNT_RELEASE(&lbm_ref_count); + __atomic_clear(&flush_in_progress, __ATOMIC_RELEASE); } BLE_LOG_IRAM_ATTR diff --git a/components/bt/common/ble_log/src/prph/ble_log_prph_spi_master_dma.c b/components/bt/common/ble_log/src/prph/ble_log_prph_spi_master_dma.c index 41f7975938..293f70b2c6 100644 --- a/components/bt/common/ble_log/src/prph/ble_log_prph_spi_master_dma.c +++ b/components/bt/common/ble_log/src/prph/ble_log_prph_spi_master_dma.c @@ -97,12 +97,11 @@ void ble_log_prph_deinit(void) { prph_inited = false; if (dev_handle) { - /* Drain all queued transactions */ if (spi_device_acquire_bus(dev_handle, portMAX_DELAY) == ESP_OK) { spi_device_release_bus(dev_handle); - spi_bus_remove_device(dev_handle); - dev_handle = NULL; } + spi_bus_remove_device(dev_handle); + dev_handle = NULL; } /* Note: We don't care if the bus has been inited or not */ diff --git a/components/bt/common/ble_log/src/prph/ble_log_prph_uart_dma.c b/components/bt/common/ble_log/src/prph/ble_log_prph_uart_dma.c index 73c9792fab..f21ff5f291 100644 --- a/components/bt/common/ble_log/src/prph/ble_log_prph_uart_dma.c +++ b/components/bt/common/ble_log/src/prph/ble_log_prph_uart_dma.c @@ -18,6 +18,7 @@ #include "esp_timer.h" #include "driver/uart.h" #include "driver/uart_vfs.h" +#include "freertos/task.h" #endif /* BLE_LOG_PRPH_UART_DMA_REDIR */ /* MACRO */ @@ -146,8 +147,9 @@ bool ble_log_prph_init(size_t trans_cnt) /* Initialize UART driver for redirection */ if (!uart_is_driver_installed(UART_NUM_0)) { - uart_driver_install(UART_NUM_0, BLE_LOG_UART_RX_BUF_SIZE, 0, 0, NULL, 0); - uart_driver_inited = true; + if (uart_driver_install(UART_NUM_0, BLE_LOG_UART_RX_BUF_SIZE, 0, 0, NULL, 0) == ESP_OK) { + uart_driver_inited = true; + } } uart_vfs_dev_use_driver(UART_NUM_0); @@ -285,6 +287,9 @@ BLE_LOG_IRAM_ATTR void ble_log_prph_send_trans(ble_log_prph_trans_t *trans) BLE_LOG_IRAM_ATTR BLE_LOG_STATIC void ble_log_redir_uart_tx_chars(const char *src, size_t len) { + if (BLE_LOG_IN_ISR() || xTaskGetSchedulerState() == taskSCHEDULER_SUSPENDED) { + return; + } xSemaphoreTake(redir_lbm->mutex, portMAX_DELAY); ble_log_lbm_stream_write(redir_lbm, BLE_LOG_SRC_REDIR, (const uint8_t *)src, len); From 1275b78ad6c883d18e2b7c0dcff80bc410cc8a29 Mon Sep 17 00:00:00 2001 From: Zhou Xiao Date: Tue, 7 Apr 2026 15:31:40 +0800 Subject: [PATCH 3/5] fix(ble_log): fix deinit race and concurrent flush deadlock - Reorder deinit: stop RT task before destroying peripheral driver to prevent sending transports to a NULL dev_handle on dual-core (H2) - Drain remaining queue items in rt_deinit to clean up transport state - Add atomic flush_in_progress guard to prevent two concurrent ble_log_flush() callers from deadlocking on ref_count spin-wait (H5) --- components/bt/common/ble_log/src/ble_log.c | 29 ++++++++++--------- .../bt/common/ble_log/src/ble_log_lbm.c | 10 +++---- components/bt/common/ble_log/src/ble_log_rt.c | 11 +++++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/components/bt/common/ble_log/src/ble_log.c b/components/bt/common/ble_log/src/ble_log.c index ed0625688f..4e6ce25838 100644 --- a/components/bt/common/ble_log/src/ble_log.c +++ b/components/bt/common/ble_log/src/ble_log.c @@ -72,20 +72,23 @@ void ble_log_deinit(void) ble_log_enable(false); ble_log_inited = false; - /* CRITICAL: - * BLE Log peripheral interface must be deinitialized at first, - * because there's a risky scenario that may cause severe peripheral - * driver fault - if a log buffer is sent to peripheral driver, and - * ble_log_deinit is called; in this case, if LBM is deinitialized - * before peripheral interface, the log buffer may be freed before - * peripheral driver completing tx, and the result would be faulty */ - ble_log_prph_deinit(); - - /* Deinitialize BLE Log LBM */ - ble_log_lbm_deinit(); - - /* Deinitialize BLE Log Runtime */ + /* CRITICAL — Deinit ordering rationale: + * + * 1. Runtime task must be stopped FIRST to prevent it from sending + * transports to an already-destroyed peripheral driver. The queue + * is drained and pending transports are discarded. + * + * 2. Peripheral interface is deinitialized SECOND. It waits for any + * in-flight DMA operations (started before the task was killed) to + * complete, then destroys the driver. This is safe because no new + * DMA operations can be started (the task is already dead). + * + * 3. LBM is deinitialized LAST. At this point all DMA has completed + * (ensured by step 2) and all queued transports have been drained + * (ensured by step 1), so freeing the buffers is safe. */ ble_log_rt_deinit(); + ble_log_prph_deinit(); + ble_log_lbm_deinit(); #if CONFIG_BLE_LOG_TS_ENABLED /* Deinitialize BLE Log TS */ diff --git a/components/bt/common/ble_log/src/ble_log_lbm.c b/components/bt/common/ble_log/src/ble_log_lbm.c index 7bed717b2c..db673c80be 100644 --- a/components/bt/common/ble_log/src/ble_log_lbm.c +++ b/components/bt/common/ble_log/src/ble_log_lbm.c @@ -195,11 +195,11 @@ void ble_log_stat_mgr_update(ble_log_src_t src_code, uint32_t len, bool lost) uint32_t bytes_cnt = len + BLE_LOG_FRAME_OVERHEAD; if (lost) { BLE_LOG_GET_FRAME_SN(&(stat_mgr->frame_sn)); /* consume SN for loss detection */ - stat_mgr->lost_frame_cnt++; - stat_mgr->lost_bytes_cnt += bytes_cnt; + __atomic_fetch_add(&stat_mgr->lost_frame_cnt, 1, __ATOMIC_RELAXED); + __atomic_fetch_add(&stat_mgr->lost_bytes_cnt, bytes_cnt, __ATOMIC_RELAXED); } else { - stat_mgr->written_frame_cnt++; - stat_mgr->written_bytes_cnt += bytes_cnt; + __atomic_fetch_add(&stat_mgr->written_frame_cnt, 1, __ATOMIC_RELAXED); + __atomic_fetch_add(&stat_mgr->written_bytes_cnt, bytes_cnt, __ATOMIC_RELAXED); } } @@ -290,7 +290,7 @@ void ble_log_lbm_deinit(void) /* Disable module and wait for all references to be released */ uint32_t time_waited = 0; - while (lbm_ref_count > 0) { + while (__atomic_load_n(&lbm_ref_count, __ATOMIC_ACQUIRE) > 0) { vTaskDelay(pdMS_TO_TICKS(1)); BLE_LOG_ASSERT(time_waited++ < 1000); } diff --git a/components/bt/common/ble_log/src/ble_log_rt.c b/components/bt/common/ble_log/src/ble_log_rt.c index b2aabe6c80..f5d79bf547 100644 --- a/components/bt/common/ble_log/src/ble_log_rt.c +++ b/components/bt/common/ble_log/src/ble_log_rt.c @@ -31,7 +31,7 @@ BLE_LOG_STATIC void ble_log_rt_ts_trigger(void *arg); #endif /* CONFIG_BLE_LOG_TS_ENABLED */ /* PRIVATE FUNCTION */ -BLE_LOG_IRAM_ATTR BLE_LOG_STATIC void ble_log_rt_task(void *pvParameters) +BLE_LOG_STATIC void ble_log_rt_task(void *pvParameters) { (void)pvParameters; ble_log_prph_trans_t *trans = NULL; @@ -162,8 +162,15 @@ void ble_log_rt_deinit(void) rt_task_handle = NULL; } - /* Release task queue */ + /* Drain remaining queue items to clean up transport state */ if (rt_queue_handle) { + ble_log_prph_trans_t *trans = NULL; + while (xQueueReceive(rt_queue_handle, &trans, 0) == pdTRUE) { + ble_log_lbm_t *lbm = (ble_log_lbm_t *)trans->owner; + trans->pos = 0; + __atomic_fetch_sub(&lbm->trans_inflight, 1, __ATOMIC_RELAXED); + __atomic_store_n(&trans->prph_owned, false, __ATOMIC_RELEASE); + } vQueueDelete(rt_queue_handle); rt_queue_handle = NULL; } From 59536316c9102eb8a3a7ba9b16b6f37d2b902a2e Mon Sep 17 00:00:00 2001 From: Zhou Xiao Date: Tue, 7 Apr 2026 15:31:50 +0800 Subject: [PATCH 4/5] fix(ble_log): improve robustness with atomics, macro hygiene, and IRAM - Use __atomic_fetch_add for stat_mgr counters to prevent lost updates under concurrent ISR/task access (H1) - Use __atomic_load_n with ACQUIRE ordering for ref_count spin-loops (L1) - Remove unnecessary BLE_LOG_IRAM_ATTR from ble_log_rt_task since it calls flash-resident functions and cannot run during flash ops (L3) - Add parentheses to BLE_LOG_TRANS_FREE_SPACE and BLE_LOG_MAKE_FRAME_META macro parameters to prevent operator precedence bugs (M6) --- components/bt/common/ble_log/src/internal_include/ble_log_lbm.h | 2 +- .../bt/common/ble_log/src/internal_include/ble_log_prph.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/bt/common/ble_log/src/internal_include/ble_log_lbm.h b/components/bt/common/ble_log/src/internal_include/ble_log_lbm.h index b8e2a69172..1d1e704270 100644 --- a/components/bt/common/ble_log/src/internal_include/ble_log_lbm.h +++ b/components/bt/common/ble_log/src/internal_include/ble_log_lbm.h @@ -35,7 +35,7 @@ typedef struct { #define BLE_LOG_FRAME_HEAD_LEN (sizeof(ble_log_frame_head_t)) #define BLE_LOG_FRAME_TAIL_LEN (sizeof(uint32_t)) #define BLE_LOG_FRAME_OVERHEAD (BLE_LOG_FRAME_HEAD_LEN + BLE_LOG_FRAME_TAIL_LEN) -#define BLE_LOG_MAKE_FRAME_META(src_code, sn) ((src_code & 0xFF) | (sn << 8)) +#define BLE_LOG_MAKE_FRAME_META(src_code, sn) (((src_code) & 0xFF) | ((sn) << 8)) /* ---------------------------------- */ /* Log Buffer Manager Defines */ diff --git a/components/bt/common/ble_log/src/internal_include/ble_log_prph.h b/components/bt/common/ble_log/src/internal_include/ble_log_prph.h index 01189566d9..a466df9179 100644 --- a/components/bt/common/ble_log/src/internal_include/ble_log_prph.h +++ b/components/bt/common/ble_log/src/internal_include/ble_log_prph.h @@ -27,7 +27,7 @@ typedef struct { void *owner; } ble_log_prph_trans_t; -#define BLE_LOG_TRANS_FREE_SPACE(trans) (trans->size - trans->pos) +#define BLE_LOG_TRANS_FREE_SPACE(trans) ((trans)->size - (trans)->pos) #define BLE_LOG_TRANS_BUF_CNT (4) /* INTERFACE */ From a9b93ca0e55cab4787cceb2bea80f8a54d493611 Mon Sep 17 00:00:00 2001 From: Zhou Xiao Date: Tue, 7 Apr 2026 19:02:14 +0800 Subject: [PATCH 5/5] fix(ble_log): ensure BLE Log IRAM functions are placed in IRAM on ESP32C2 On ESP32C2 with BT_CTRL_RUN_IN_FLASH_ONLY, the bt_default linker scheme routes .iram1 sections to flash_text, causing BLE_LOG_IRAM_ATTR functions to end up in flash instead of IRAM. Define a dedicated .ble_log_iram1 section for ESP32C2 and route it to iram0_bt_text unconditionally. --- .../bt/common/ble_log/src/internal_include/ble_log_util.h | 4 ++++ components/bt/linker_esp32c2.lf | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/components/bt/common/ble_log/src/internal_include/ble_log_util.h b/components/bt/common/ble_log/src/internal_include/ble_log_util.h index 01834595e2..adc827c55e 100644 --- a/components/bt/common/ble_log/src/internal_include/ble_log_util.h +++ b/components/bt/common/ble_log/src/internal_include/ble_log_util.h @@ -37,7 +37,11 @@ #define BLE_LOG_INLINE inline /* Section */ +#if defined(CONFIG_IDF_TARGET_ESP32C2) +#define BLE_LOG_IRAM_ATTR _SECTION_ATTR_IMPL(".ble_log_iram1", __COUNTER__) +#else #define BLE_LOG_IRAM_ATTR IRAM_ATTR +#endif /* Memory operation */ #define BLE_LOG_MEM_CAP (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT | MALLOC_CAP_DMA) diff --git a/components/bt/linker_esp32c2.lf b/components/bt/linker_esp32c2.lf index c9ab4c2e49..3a07d7fde4 100644 --- a/components/bt/linker_esp32c2.lf +++ b/components/bt/linker_esp32c2.lf @@ -14,6 +14,10 @@ entries: entries: .sleep_iram1+ +[sections:bt_ble_log_iram_text] +entries: + .ble_log_iram1+ + [sections:bt_bss] entries: .bss+ @@ -33,6 +37,7 @@ entries: entries: if BT_CTRL_RUN_IN_FLASH_ONLY = y: bt_iram_text -> flash_text + bt_ble_log_iram_text -> iram0_bt_text bt_bss -> dram0_bt_bss bt_common -> dram0_bt_bss bt_data -> dram0_bt_data @@ -49,6 +54,7 @@ entries: bt_sleep_iram_text -> flash_text else: bt_iram_text -> iram0_bt_text + bt_ble_log_iram_text -> iram0_bt_text bt_bss -> dram0_bt_bss bt_common -> dram0_bt_bss bt_data -> dram0_bt_data