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 9b1dd64c92..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); } @@ -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/ble_log_rt.c b/components/bt/common/ble_log/src/ble_log_rt.c index c055dcd0fc..f5d79bf547 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 */ @@ -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; } @@ -185,8 +192,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); } 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 */ 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/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); 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