From 4abe47437f6f11af43a8c3b571b715e667779da5 Mon Sep 17 00:00:00 2001 From: Hrishikesh Dhayagude Date: Tue, 28 Aug 2018 13:31:07 +0530 Subject: [PATCH] components/bt: Fix broken API esp_bt_mem_release() for parameter ESP_BT_MODE_BTDM Problem: The new API esp_bt_mem_release() that was added freed BTDM data to heap from esp_bt_controller_mem_release(). Now with the BT memory optimization commit ee787085f937a582c3b70692f0d19b58cdb2de7d, the BTDM data is optimized and reduced to only 32 bytes which is not sufficient amount to be added to heap. So, using the API leads to assert saying that the region is too small. Solution: Modify heap_caps_add_region_with_caps to return ESP_ERR_INVALID_SIZE in case the range is too small to create a new heap. Do not assert if return value is ESP_ERR_INVALID_SIZE This also fixes using API esp_bt_controller_mem_release() with ESP_BT_MODE_BTDM Signed-off-by: Hrishikesh Dhayagude --- components/bt/bt.c | 43 +++++++++++++------- components/heap/heap_caps_init.c | 2 +- components/heap/include/esp_heap_caps_init.h | 1 + 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/components/bt/bt.c b/components/bt/bt.c index a5d5e1424..b23750662 100644 --- a/components/bt/bt.c +++ b/components/bt/bt.c @@ -820,17 +820,30 @@ static void btdm_controller_mem_init(void) { /* initialise .data section */ memcpy(&_data_start_btdm, (void *)_data_start_btdm_rom, &_data_end_btdm - &_data_start_btdm); - ESP_LOGD(BTDM_LOG_TAG, ".data initialise [0x%08x] <== [0x%08x]\n", (uint32_t)&_data_start_btdm, _data_start_btdm_rom); + ESP_LOGD(BTDM_LOG_TAG, ".data initialise [0x%08x] <== [0x%08x]", (uint32_t)&_data_start_btdm, _data_start_btdm_rom); //initial em, .bss section for (int i = 1; i < sizeof(btdm_dram_available_region)/sizeof(btdm_dram_available_region_t); i++) { if (btdm_dram_available_region[i].mode != ESP_BT_MODE_IDLE) { memset((void *)btdm_dram_available_region[i].start, 0x0, btdm_dram_available_region[i].end - btdm_dram_available_region[i].start); - ESP_LOGD(BTDM_LOG_TAG, ".bss initialise [0x%08x] - [0x%08x]\n", btdm_dram_available_region[i].start, btdm_dram_available_region[i].end); + ESP_LOGD(BTDM_LOG_TAG, ".bss initialise [0x%08x] - [0x%08x]", btdm_dram_available_region[i].start, btdm_dram_available_region[i].end); } } } +static esp_err_t try_heap_caps_add_region(intptr_t start, intptr_t end) +{ + int ret = heap_caps_add_region(start, end); + /* heap_caps_add_region() returns ESP_ERR_INVALID_SIZE if the memory region is + * is too small to fit a heap. This cannot be termed as a fatal error and hence + * we replace it by ESP_OK + */ + if (ret == ESP_ERR_INVALID_SIZE) { + return ESP_OK; + } + return ret; +} + esp_err_t esp_bt_controller_mem_release(esp_bt_mode_t mode) { bool update = true; @@ -870,14 +883,14 @@ esp_err_t esp_bt_controller_mem_release(esp_bt_mode_t mode) && mem_end == btdm_dram_available_region[i+1].start) { continue; } else { - ESP_LOGD(BTDM_LOG_TAG, "Release DRAM [0x%08x] - [0x%08x]\n", mem_start, mem_end); - ESP_ERROR_CHECK(heap_caps_add_region(mem_start, mem_end)); + ESP_LOGD(BTDM_LOG_TAG, "Release DRAM [0x%08x] - [0x%08x]", mem_start, mem_end); + ESP_ERROR_CHECK(try_heap_caps_add_region(mem_start, mem_end)); update = true; } } else { mem_end = btdm_dram_available_region[i].end; - ESP_LOGD(BTDM_LOG_TAG, "Release DRAM [0x%08x] - [0x%08x]\n", mem_start, mem_end); - ESP_ERROR_CHECK(heap_caps_add_region(mem_start, mem_end)); + ESP_LOGD(BTDM_LOG_TAG, "Release DRAM [0x%08x] - [0x%08x]", mem_start, mem_end); + ESP_ERROR_CHECK(try_heap_caps_add_region(mem_start, mem_end)); update = true; } } @@ -886,14 +899,14 @@ esp_err_t esp_bt_controller_mem_release(esp_bt_mode_t mode) mem_start = (intptr_t)&_btdm_bss_start; mem_end = (intptr_t)&_btdm_bss_end; if (mem_start != mem_end) { - ESP_LOGD(BTDM_LOG_TAG, "Release BTDM BSS [0x%08x] - [0x%08x]\n", mem_start, mem_end); - ESP_ERROR_CHECK(heap_caps_add_region(mem_start, mem_end)); + ESP_LOGD(BTDM_LOG_TAG, "Release BTDM BSS [0x%08x] - [0x%08x]", mem_start, mem_end); + ESP_ERROR_CHECK(try_heap_caps_add_region(mem_start, mem_end)); } mem_start = (intptr_t)&_btdm_data_start; mem_end = (intptr_t)&_btdm_data_end; if (mem_start != mem_end) { - ESP_LOGD(BTDM_LOG_TAG, "Release BTDM Data [0x%08x] - [0x%08x]\n", mem_start, mem_end); - ESP_ERROR_CHECK(heap_caps_add_region(mem_start, mem_end)); + ESP_LOGD(BTDM_LOG_TAG, "Release BTDM Data [0x%08x] - [0x%08x]", mem_start, mem_end); + ESP_ERROR_CHECK(try_heap_caps_add_region(mem_start, mem_end)); } } return ESP_OK; @@ -913,14 +926,14 @@ esp_err_t esp_bt_mem_release(esp_bt_mode_t mode) mem_start = (intptr_t)&_bt_bss_start; mem_end = (intptr_t)&_bt_bss_end; if (mem_start != mem_end) { - ESP_LOGD(BTDM_LOG_TAG, "Release BT BSS [0x%08x] - [0x%08x]\n", mem_start, mem_end); - ESP_ERROR_CHECK(heap_caps_add_region(mem_start, mem_end)); + ESP_LOGD(BTDM_LOG_TAG, "Release BT BSS [0x%08x] - [0x%08x]", mem_start, mem_end); + ESP_ERROR_CHECK(try_heap_caps_add_region(mem_start, mem_end)); } mem_start = (intptr_t)&_bt_data_start; mem_end = (intptr_t)&_bt_data_end; if (mem_start != mem_end) { - ESP_LOGD(BTDM_LOG_TAG, "Release BT Data [0x%08x] - [0x%08x]\n", mem_start, mem_end); - ESP_ERROR_CHECK(heap_caps_add_region(mem_start, mem_end)); + ESP_LOGD(BTDM_LOG_TAG, "Release BT Data [0x%08x] - [0x%08x]", mem_start, mem_end); + ESP_ERROR_CHECK(try_heap_caps_add_region(mem_start, mem_end)); } } return ESP_OK; @@ -976,7 +989,7 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg) } #endif - ESP_LOGI(BTDM_LOG_TAG, "BT controller compile version [%s]\n", btdm_controller_get_compile_version()); + ESP_LOGI(BTDM_LOG_TAG, "BT controller compile version [%s]", btdm_controller_get_compile_version()); #if CONFIG_SPIRAM_USE_MALLOC btdm_queue_table_mux = xSemaphoreCreateMutex(); diff --git a/components/heap/heap_caps_init.c b/components/heap/heap_caps_init.c index af1e27df4..3aa64e61b 100644 --- a/components/heap/heap_caps_init.c +++ b/components/heap/heap_caps_init.c @@ -218,7 +218,7 @@ esp_err_t heap_caps_add_region_with_caps(const uint32_t caps[], intptr_t start, p_new->heap = multi_heap_register((void *)start, end - start); SLIST_NEXT(p_new, next) = NULL; if (p_new->heap == NULL) { - err = ESP_FAIL; + err = ESP_ERR_INVALID_SIZE; goto done; } multi_heap_set_lock(p_new->heap, &p_new->heap_mux); diff --git a/components/heap/include/esp_heap_caps_init.h b/components/heap/include/esp_heap_caps_init.h index 3cf23ff7f..3ae6b8e4f 100644 --- a/components/heap/include/esp_heap_caps_init.h +++ b/components/heap/include/esp_heap_caps_init.h @@ -81,6 +81,7 @@ esp_err_t heap_caps_add_region(intptr_t start, intptr_t end); * - ESP_OK on success * - ESP_ERR_INVALID_ARG if a parameter is invalid * - ESP_ERR_NO_MEM if no memory to register new heap. + * - ESP_ERR_INVALID_SIZE if the memory region is too small to fit a heap * - ESP_FAIL if region overlaps the start and/or end of an existing region */ esp_err_t heap_caps_add_region_with_caps(const uint32_t caps[], intptr_t start, intptr_t end);