Skip to content

Commit

Permalink
Merge branch 'feature/esp32c2_optimize_ble_init' into 'master'
Browse files Browse the repository at this point in the history
Fixed memory leak when RAM free size is insufficient or setting ext scan...

See merge request espressif/esp-idf!21261
  • Loading branch information
jack0c committed Nov 30, 2022
2 parents 5933355 + 81d6b8f commit 7869f4e
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 99 deletions.
42 changes: 32 additions & 10 deletions components/bt/controller/esp32c2/bt.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
#include "esp_private/periph_ctrl.h"
#include "esp_sleep.h"

#include "soc/syscon_reg.h"
#include "soc/dport_access.h"
/* Macro definition
************************************************************************
*/
Expand All @@ -60,7 +62,7 @@
#define OSI_COEX_VERSION 0x00010006
#define OSI_COEX_MAGIC_VALUE 0xFADEBEAD

#define EXT_FUNC_VERSION 0x20220125
#define EXT_FUNC_VERSION 0x20221122
#define EXT_FUNC_MAGIC_VALUE 0xA5A5A5A5

#define BT_ASSERT_PRINT ets_printf
Expand Down Expand Up @@ -102,6 +104,7 @@ struct ext_funcs_t {
int (* _ecc_gen_key_pair)(uint8_t *public, uint8_t *priv);
int (* _ecc_gen_dh_key)(const uint8_t *remote_pub_key_x, const uint8_t *remote_pub_key_y, const uint8_t *local_priv_key, uint8_t *dhkey);
void (* _esp_reset_rpa_moudle)(void);
void (* _esp_bt_track_pll_cap)(void);
uint32_t magic;
};

Expand Down Expand Up @@ -232,7 +235,8 @@ struct ext_funcs_t ext_funcs_ro = {

static void IRAM_ATTR esp_reset_rpa_moudle(void)
{
periph_module_reset(PERIPH_MODEM_RPA_MODULE);
DPORT_SET_PERI_REG_MASK(SYSTEM_CORE_RST_EN_REG, BLE_RPA_REST_BIT);
DPORT_CLEAR_PERI_REG_MASK(SYSTEM_CORE_RST_EN_REG, BLE_RPA_REST_BIT);
}

static void IRAM_ATTR osi_assert_wrapper(const uint32_t ln, const char *fn, uint32_t param1, uint32_t param2)
Expand Down Expand Up @@ -399,7 +403,7 @@ static int ble_hci_unregistered_hook(void*, void*)

static int esp_intr_alloc_wrapper(int source, int flags, intr_handler_t handler, void *arg, void **ret_handle_in)
{
int rc = esp_intr_alloc(source, flags, handler, arg, (intr_handle_t *)ret_handle_in);
int rc = esp_intr_alloc(source, flags | ESP_INTR_FLAG_IRAM, handler, arg, (intr_handle_t *)ret_handle_in);
return rc;
}

Expand Down Expand Up @@ -641,6 +645,8 @@ void ble_rtc_clk_init(void)

esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg)
{
esp_err_t ret = ESP_OK;

if (ble_controller_status != ESP_BT_CONTROLLER_STATUS_IDLE) {
ESP_LOGW(NIMBLE_PORT_LOG_TAG, "invalid controller state");
return ESP_FAIL;
Expand All @@ -659,7 +665,6 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg)

/* Initialize the function pointers for OS porting */
npl_freertos_funcs_init();

struct npl_funcs_t *p_npl_funcs = npl_freertos_funcs_get();
if (!p_npl_funcs) {
ESP_LOGW(NIMBLE_PORT_LOG_TAG, "npl functions get failed");
Expand All @@ -668,18 +673,21 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg)

if (esp_register_npl_funcs(p_npl_funcs) != 0) {
ESP_LOGW(NIMBLE_PORT_LOG_TAG, "npl functions register failed");
return ESP_ERR_INVALID_ARG;
ret = ESP_ERR_INVALID_ARG;
goto free_mem;
}

if (npl_freertos_mempool_init() != 0) {
ESP_LOGW(NIMBLE_PORT_LOG_TAG, "npl mempool init failed");
return ESP_ERR_INVALID_ARG;
ret = ESP_ERR_INVALID_ARG;
goto free_mem;
}

/* Initialize the global memory pool */
if (os_msys_buf_alloc() != 0) {
ESP_LOGW(NIMBLE_PORT_LOG_TAG, "os msys alloc failed");
return ESP_ERR_INVALID_ARG;
ret = ESP_ERR_INVALID_ARG;
goto free_mem;
}

os_msys_init();
Expand All @@ -699,7 +707,8 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg)

if (ble_osi_coex_funcs_register((struct osi_coex_funcs_t *)&s_osi_coex_funcs_ro) != 0) {
ESP_LOGW(NIMBLE_PORT_LOG_TAG, "osi coex funcs reg failed");
return ESP_ERR_INVALID_ARG;
ret = ESP_ERR_INVALID_ARG;
goto free_phy;
}

#if CONFIG_SW_COEXIST_ENABLE
Expand All @@ -708,7 +717,8 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg)
int rc = ble_controller_init(cfg);
if (rc != 0) {
ESP_LOGW(NIMBLE_PORT_LOG_TAG, "ble_controller_init failed %d", rc);
return ESP_ERR_NO_MEM;
ret = ESP_ERR_NO_MEM;
goto free_phy;
}

controller_sleep_init();
Expand All @@ -724,8 +734,20 @@ esp_err_t esp_bt_controller_init(esp_bt_controller_config_t *cfg)

ble_hci_trans_cfg_hs((ble_hci_trans_rx_cmd_fn *)ble_hci_unregistered_hook,NULL,
(ble_hci_trans_rx_acl_fn *)ble_hci_unregistered_hook,NULL);

return ESP_OK;
free_phy:
esp_phy_disable();
esp_phy_pd_mem_deinit();
#if CONFIG_BT_NIMBLE_ENABLED
ble_npl_eventq_deinit(nimble_port_get_dflt_eventq());
#endif // CONFIG_BT_NIMBLE_ENABLED
free_mem:
os_msys_buf_free();
npl_freertos_mempool_deinit();
esp_unregister_npl_funcs();
npl_freertos_funcs_deinit();
esp_unregister_ext_funcs();
return ret;
}

esp_err_t esp_bt_controller_deinit(void)
Expand Down
8 changes: 6 additions & 2 deletions components/bt/controller/esp32h4/bt.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
#include "esp_private/periph_ctrl.h"
#include "esp_sleep.h"

#include "soc/syscon_reg.h"
#include "soc/dport_access.h"

/* Macro definition
************************************************************************
*/
Expand Down Expand Up @@ -216,7 +219,8 @@ struct ext_funcs_t ext_funcs_ro = {

static int IRAM_ATTR esp_reset_rpa_moudle(void)
{
periph_module_reset(PERIPH_MODEM_RPA_MODULE);
DPORT_SET_PERI_REG_MASK(SYSTEM_MODEM_RST_EN_REG, SYSTEM_BLE_SEC_AAR_RST);
DPORT_CLEAR_PERI_REG_MASK(SYSTEM_MODEM_RST_EN_REG, SYSTEM_BLE_SEC_AAR_RST);
return 0;
}

Expand Down Expand Up @@ -385,7 +389,7 @@ static int ble_hci_unregistered_hook(void*, void*)

static int esp_intr_alloc_wrapper(int source, int flags, intr_handler_t handler, void *arg, void **ret_handle_in)
{
int rc = esp_intr_alloc(source, flags, handler, arg, (intr_handle_t *)ret_handle_in);
int rc = esp_intr_alloc(source, flags | ESP_INTR_FLAG_IRAM, handler, arg, (intr_handle_t *)ret_handle_in);
return rc;
}

Expand Down
2 changes: 1 addition & 1 deletion components/bt/controller/lib_esp32c2/esp32c2-bt-lib
Submodule esp32c2-bt-lib updated 2 files
+1 −1 NOTICE
+ libble_app.a
4 changes: 2 additions & 2 deletions components/bt/porting/mem/bt_osi_mem.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ IRAM_ATTR void *bt_osi_mem_calloc(size_t n, size_t size)

IRAM_ATTR void *bt_osi_mem_malloc_internal(size_t size)
{
return heap_caps_malloc(size, MALLOC_CAP_INTERNAL|MALLOC_CAP_8BIT);
return heap_caps_malloc(size, MALLOC_CAP_INTERNAL|MALLOC_CAP_8BIT|MALLOC_CAP_DMA);
}

IRAM_ATTR void *bt_osi_mem_calloc_internal(size_t n, size_t size)
{
return heap_caps_calloc(n, size, MALLOC_CAP_INTERNAL|MALLOC_CAP_8BIT);
return heap_caps_calloc(n, size, MALLOC_CAP_INTERNAL|MALLOC_CAP_8BIT|MALLOC_CAP_DMA);
}

IRAM_ATTR void bt_osi_mem_free(void *ptr)
Expand Down
2 changes: 1 addition & 1 deletion components/bt/porting/nimble/include/nimble/nimble_npl.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ uint16_t ble_npl_sem_get_count(struct ble_npl_sem *sem);
* Callouts
*/

void ble_npl_callout_init(struct ble_npl_callout *co, struct ble_npl_eventq *evq,
int ble_npl_callout_init(struct ble_npl_callout *co, struct ble_npl_eventq *evq,
ble_npl_event_fn *ev_cb, void *ev_arg);

ble_npl_error_t ble_npl_callout_reset(struct ble_npl_callout *co,
Expand Down
4 changes: 4 additions & 0 deletions components/bt/porting/nimble/src/os_msys_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ os_msys_buf_alloc(void)
#if OS_MSYS_2_BLOCK_COUNT > 0
os_msys_init_2_data = (os_membuf_t *)bt_osi_mem_calloc(1, (sizeof(os_membuf_t) * SYSINIT_MSYS_2_MEMPOOL_SIZE));
if (!os_msys_init_2_data) {
#if OS_MSYS_1_BLOCK_COUNT > 0
bt_osi_mem_free(os_msys_init_1_data);
os_msys_init_1_data = NULL;
#endif
return ESP_FAIL;
}
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ struct npl_funcs_t {
ble_npl_error_t (*p_ble_npl_sem_pend)(struct ble_npl_sem *, ble_npl_time_t);
ble_npl_error_t (*p_ble_npl_sem_release)(struct ble_npl_sem *);
uint16_t (*p_ble_npl_sem_get_count)(struct ble_npl_sem *);
void (*p_ble_npl_callout_init)(struct ble_npl_callout *, struct ble_npl_eventq *, ble_npl_event_fn *, void *);
int (*p_ble_npl_callout_init)(struct ble_npl_callout *, struct ble_npl_eventq *, ble_npl_event_fn *, void *);
ble_npl_error_t (*p_ble_npl_callout_reset)(struct ble_npl_callout *, ble_npl_time_t);
void (*p_ble_npl_callout_stop)(struct ble_npl_callout *);
void (*p_ble_npl_callout_deinit)(struct ble_npl_callout *);
Expand Down Expand Up @@ -259,12 +259,13 @@ IRAM_ATTR ble_npl_sem_get_count(struct ble_npl_sem *sem)
return npl_funcs->p_ble_npl_sem_get_count(sem);
}

static inline void
static inline int
IRAM_ATTR ble_npl_callout_init(struct ble_npl_callout *co, struct ble_npl_eventq *evq,
ble_npl_event_fn *ev_cb, void *ev_arg)
{
return npl_funcs->p_ble_npl_callout_init(co, evq, ev_cb, ev_arg);
}

static inline void
IRAM_ATTR ble_npl_callout_deinit(struct ble_npl_callout *co)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ ble_npl_error_t npl_freertos_sem_pend(struct ble_npl_sem *sem,

ble_npl_error_t npl_freertos_sem_release(struct ble_npl_sem *sem);

void npl_freertos_callout_init(struct ble_npl_callout *co,
int npl_freertos_callout_init(struct ble_npl_callout *co,
struct ble_npl_eventq *evq,
ble_npl_event_fn *ev_cb, void *ev_arg);

Expand Down
108 changes: 64 additions & 44 deletions components/bt/porting/npl/freertos/src/npl_os_freertos.c
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ IRAM_ATTR os_callout_timer_cb(TimerHandle_t timer)
}
#endif

void
int
npl_freertos_callout_init(struct ble_npl_callout *co, struct ble_npl_eventq *evq,
ble_npl_event_fn *ev_cb, void *ev_arg)
{
Expand All @@ -692,61 +692,82 @@ npl_freertos_callout_init(struct ble_npl_callout *co, struct ble_npl_eventq *evq
ble_npl_event_init(&callout->ev, ev_cb, ev_arg);

#if CONFIG_BT_NIMBLE_USE_ESP_TIMER
callout->evq = evq;

esp_timer_create_args_t create_args = {
.callback = ble_npl_event_fn_wrapper,
.arg = callout,
.name = "nimble_timer"
};

ESP_ERROR_CHECK(esp_timer_create(&create_args, &callout->handle));

callout->evq = evq;

esp_timer_create_args_t create_args = {
.callback = ble_npl_event_fn_wrapper,
.arg = callout,
.name = "nimble_timer"
};

if (esp_timer_create(&create_args, &callout->handle) != ESP_OK) {
ble_npl_event_deinit(&callout->ev);
os_memblock_put(&ble_freertos_co_pool,callout);
co->co = NULL;
return -1;
}
#else
callout->handle = xTimerCreate("co", 1, pdFALSE, callout, os_callout_timer_cb);
#endif
callout->handle = xTimerCreate("co", 1, pdFALSE, callout, os_callout_timer_cb);

BLE_LL_ASSERT(callout->handle);
if (!callout->handle) {
ble_npl_event_deinit(&callout->ev);
os_memblock_put(&ble_freertos_co_pool,callout);
co->co = NULL;
return -1;
}
#endif // CONFIG_BT_NIMBLE_USE_ESP_TIMER
} else {
callout = (struct ble_npl_callout_freertos *)co->co;
BLE_LL_ASSERT(callout);
callout->evq = evq;
ble_npl_event_init(&callout->ev, ev_cb, ev_arg);
callout = (struct ble_npl_callout_freertos *)co->co;
BLE_LL_ASSERT(callout);
callout->evq = evq;
ble_npl_event_init(&callout->ev, ev_cb, ev_arg);
}
#else

if(!co->co) {
co->co = malloc(sizeof(struct ble_npl_callout_freertos));
callout = (struct ble_npl_callout_freertos *)co->co;
BLE_LL_ASSERT(callout);
if (!callout) {
return -1;
}

memset(callout, 0, sizeof(*callout));
memset(callout, 0, sizeof(*callout));
ble_npl_event_init(&callout->ev, ev_cb, ev_arg);

#if CONFIG_BT_NIMBLE_USE_ESP_TIMER
callout->evq = evq;

esp_timer_create_args_t create_args = {
.callback = ble_npl_event_fn_wrapper,
.arg = callout,
.name = "nimble_timer"
};

ESP_ERROR_CHECK(esp_timer_create(&create_args, &callout->handle));
callout->evq = evq;

esp_timer_create_args_t create_args = {
.callback = ble_npl_event_fn_wrapper,
.arg = callout,
.name = "nimble_timer"
};

if (esp_timer_create(&create_args, &callout->handle) != ESP_OK) {
ble_npl_event_deinit(&callout->ev);
free((void *)callout);
co->co = NULL;
return -1;
}
#else
callout->handle = xTimerCreate("co", 1, pdFALSE, callout, os_callout_timer_cb);
#endif
callout->handle = xTimerCreate("co", 1, pdFALSE, callout, os_callout_timer_cb);

BLE_LL_ASSERT(callout->handle);
if (!callout->handle) {
ble_npl_event_deinit(&callout->ev);
free((void *)callout);
co->co = NULL;
return -1;
}
#endif // CONFIG_BT_NIMBLE_USE_ESP_TIMER
}
else {
callout = (struct ble_npl_callout_freertos *)co->co;
BLE_LL_ASSERT(callout);
callout->evq = evq;
ble_npl_event_init(&callout->ev, ev_cb, ev_arg);
callout->evq = evq;
ble_npl_event_init(&callout->ev, ev_cb, ev_arg);
}
#endif

#endif // OS_MEM_ALLOC
return 0;
}

void
Expand All @@ -756,11 +777,14 @@ npl_freertos_callout_deinit(struct ble_npl_callout *co)

/* Since we dynamically deinit timers, function can be called for NULL timers. Return for such scenarios */
if (!callout) {
return;
return;
}

BLE_LL_ASSERT(callout->handle);
if (!callout->handle) {
return;
}

ble_npl_event_deinit(&callout->ev);
#if CONFIG_BT_NIMBLE_USE_ESP_TIMER
esp_err_t err = esp_timer_stop(callout->handle);
if(err != ESP_OK) {
Expand All @@ -773,17 +797,13 @@ npl_freertos_callout_deinit(struct ble_npl_callout *co)
ESP_LOGW(TAG, "Timer not deleted");
}
#else

xTimerDelete(callout->handle, portMAX_DELAY);
ble_npl_event_deinit(&callout->ev);

#if OS_MEM_ALLOC
os_memblock_put(&ble_freertos_co_pool,callout);
#else
free((void *)callout);
#endif

#endif
#endif // OS_MEM_ALLOC
#endif // CONFIG_BT_NIMBLE_USE_ESP_TIMER
co->co = NULL;
memset(co, 0, sizeof(struct ble_npl_callout));
}
Expand Down
Loading

0 comments on commit 7869f4e

Please sign in to comment.