From 37d8abda0b001e6cb74806b3869245a460029be8 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 7 Apr 2022 15:31:49 +0800 Subject: [PATCH 1/6] gdma: add spin lock for gdma channel ... because we allow several control functions to be runable under ISR context --- components/driver/gdma.c | 18 ++++++++++++++---- components/driver/include/esp_private/gdma.h | 12 ++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/components/driver/gdma.c b/components/driver/gdma.c index 5250d49eac..c883318e2f 100644 --- a/components/driver/gdma.c +++ b/components/driver/gdma.c @@ -87,6 +87,7 @@ struct gdma_pair_t { struct gdma_channel_t { gdma_pair_t *pair; // which pair the channel belongs to intr_handle_t intr; // per-channel interrupt handle + portMUX_TYPE spinlock; // channel level spinlock gdma_channel_direction_t direction; // channel direction int periph_id; // Peripheral instance ID, indicates which peripheral is connected to this GDMA channel size_t sram_alignment; // alignment for memory in SRAM @@ -200,6 +201,7 @@ search_done: *ret_chan = &alloc_rx_channel->base; // return the installed channel } + (*ret_chan)->spinlock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED; ESP_LOGD(TAG, "new %s channel (%d,%d) at %p", (config->direction == GDMA_CHANNEL_DIRECTION_TX) ? "tx" : "rx", group->group_id, pair->pair_id, *ret_chan); return ESP_OK; @@ -451,10 +453,11 @@ esp_err_t gdma_start(gdma_channel_handle_t dma_chan, intptr_t desc_base_addr) esp_err_t ret = ESP_OK; gdma_pair_t *pair = NULL; gdma_group_t *group = NULL; - ESP_GOTO_ON_FALSE(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); + ESP_GOTO_ON_FALSE_ISR(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); pair = dma_chan->pair; group = pair->group; + portENTER_CRITICAL_SAFE(&dma_chan->spinlock); if (dma_chan->direction == GDMA_CHANNEL_DIRECTION_RX) { gdma_ll_rx_set_desc_addr(group->hal.dev, pair->pair_id, desc_base_addr); gdma_ll_rx_start(group->hal.dev, pair->pair_id); @@ -462,6 +465,7 @@ esp_err_t gdma_start(gdma_channel_handle_t dma_chan, intptr_t desc_base_addr) gdma_ll_tx_set_desc_addr(group->hal.dev, pair->pair_id, desc_base_addr); gdma_ll_tx_start(group->hal.dev, pair->pair_id); } + portEXIT_CRITICAL_SAFE(&dma_chan->spinlock); err: return ret; @@ -472,15 +476,17 @@ esp_err_t gdma_stop(gdma_channel_handle_t dma_chan) esp_err_t ret = ESP_OK; gdma_pair_t *pair = NULL; gdma_group_t *group = NULL; - ESP_GOTO_ON_FALSE(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); + ESP_GOTO_ON_FALSE_ISR(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); pair = dma_chan->pair; group = pair->group; + portENTER_CRITICAL_SAFE(&dma_chan->spinlock); if (dma_chan->direction == GDMA_CHANNEL_DIRECTION_RX) { gdma_ll_rx_stop(group->hal.dev, pair->pair_id); } else { gdma_ll_tx_stop(group->hal.dev, pair->pair_id); } + portEXIT_CRITICAL_SAFE(&dma_chan->spinlock); err: return ret; @@ -491,15 +497,17 @@ esp_err_t gdma_append(gdma_channel_handle_t dma_chan) esp_err_t ret = ESP_OK; gdma_pair_t *pair = NULL; gdma_group_t *group = NULL; - ESP_GOTO_ON_FALSE(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); + ESP_GOTO_ON_FALSE_ISR(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); pair = dma_chan->pair; group = pair->group; + portENTER_CRITICAL_SAFE(&dma_chan->spinlock); if (dma_chan->direction == GDMA_CHANNEL_DIRECTION_RX) { gdma_ll_rx_restart(group->hal.dev, pair->pair_id); } else { gdma_ll_tx_restart(group->hal.dev, pair->pair_id); } + portEXIT_CRITICAL_SAFE(&dma_chan->spinlock); err: return ret; @@ -510,15 +518,17 @@ esp_err_t gdma_reset(gdma_channel_handle_t dma_chan) esp_err_t ret = ESP_OK; gdma_pair_t *pair = NULL; gdma_group_t *group = NULL; - ESP_GOTO_ON_FALSE(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); + ESP_GOTO_ON_FALSE_ISR(dma_chan, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); pair = dma_chan->pair; group = pair->group; + portENTER_CRITICAL_SAFE(&dma_chan->spinlock); if (dma_chan->direction == GDMA_CHANNEL_DIRECTION_RX) { gdma_ll_rx_reset_channel(group->hal.dev, pair->pair_id); } else { gdma_ll_tx_reset_channel(group->hal.dev, pair->pair_id); } + portEXIT_CRITICAL_SAFE(&dma_chan->spinlock); err: return ret; diff --git a/components/driver/include/esp_private/gdma.h b/components/driver/include/esp_private/gdma.h index 88a45b56cf..f888cd5bc9 100644 --- a/components/driver/include/esp_private/gdma.h +++ b/components/driver/include/esp_private/gdma.h @@ -269,6 +269,9 @@ esp_err_t gdma_register_rx_event_callbacks(gdma_channel_handle_t dma_chan, gdma_ /** * @brief Set DMA descriptor address and start engine * + * @note This function is allowed to run within ISR context + * @note This function is also allowed to run when Cache is disabled, if `CONFIG_GDMA_CTRL_FUNC_IN_IRAM` is enabled + * * @param[in] dma_chan GDMA channel handle, allocated by `gdma_new_channel` * @param[in] desc_base_addr Base address of descriptors (usually the descriptors are chained into a link or ring) * @return @@ -281,6 +284,9 @@ esp_err_t gdma_start(gdma_channel_handle_t dma_chan, intptr_t desc_base_addr); /** * @brief Stop DMA engine * + * @note This function is allowed to run within ISR context + * @note This function is also allowed to run when Cache is disabled, if `CONFIG_GDMA_CTRL_FUNC_IN_IRAM` is enabled + * * @param[in] dma_chan GDMA channel handle, allocated by `gdma_new_channel` * @return * - ESP_OK: Stop DMA engine successfully @@ -291,6 +297,9 @@ esp_err_t gdma_stop(gdma_channel_handle_t dma_chan); /** * @brief Make the appended descriptors be aware to the DMA engine + * + * @note This function is allowed to run within ISR context + * @note This function is also allowed to run when Cache is disabled, if `CONFIG_GDMA_CTRL_FUNC_IN_IRAM` is enabled * @note This API could also resume a paused DMA engine, make sure new descriptors have been appended to the descriptor chain before calling it. * * @param[in] dma_chan GDMA channel handle, allocated by `gdma_new_channel` @@ -303,6 +312,9 @@ esp_err_t gdma_append(gdma_channel_handle_t dma_chan); /** * @brief Reset DMA channel FIFO and internal finite state machine + * + * @note This function is allowed to run within ISR context + * @note This function is also allowed to run when Cache is disabled, if `CONFIG_GDMA_CTRL_FUNC_IN_IRAM` is enabled * @note Resetting a DMA channel won't break the connection with the target peripheral * * @param[in] dma_chan GDMA channel handle, allocated by `gdma_new_channel` From bb2a57dc71856f41124403a448b1ee3a597db6e8 Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 8 Apr 2022 10:52:09 +0800 Subject: [PATCH 2/6] intr_alloc: fix mix boolean and bit operation --- components/esp_hw_support/intr_alloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/esp_hw_support/intr_alloc.c b/components/esp_hw_support/intr_alloc.c index 91a14acd2a..9c5d204e58 100644 --- a/components/esp_hw_support/intr_alloc.c +++ b/components/esp_hw_support/intr_alloc.c @@ -68,7 +68,7 @@ struct shared_vector_desc_t { //Pack using bitfields for better memory use struct vector_desc_t { - int flags: 16; //OR of VECDESC_FLAG_* defines + int flags: 16; //OR of VECDESC_FL_* defines unsigned int cpu: 1; unsigned int intno: 5; int source: 8; //Interrupt mux flags, used when not shared @@ -692,7 +692,7 @@ esp_err_t esp_intr_free(intr_handle_t handle) //Theoretically, we could free the vector_desc... not sure if that's worth the few bytes of memory //we save.(We can also not use the same exit path for empty shared ints anymore if we delete //the desc.) For now, just mark it as free. - handle->vector_desc->flags&=!(VECDESC_FL_NONSHARED|VECDESC_FL_RESERVED); + handle->vector_desc->flags&=~(VECDESC_FL_NONSHARED|VECDESC_FL_RESERVED|VECDESC_FL_SHARED); //Also kill non_iram mask bit. non_iram_int_mask[handle->vector_desc->cpu]&=~(1<<(handle->vector_desc->intno)); } From 483149e41b435ff3734c559222b6a644bbc1e2b1 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 7 Apr 2022 19:14:37 +0800 Subject: [PATCH 3/6] global: fix some potential out-of-bounds issue ...that found by Coverity Scan --- components/driver/rmt.c | 3 ++- components/esp_system/esp_ipc.c | 2 +- components/vfs/vfs.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/components/driver/rmt.c b/components/driver/rmt.c index 397845ac8f..c414786508 100644 --- a/components/driver/rmt.c +++ b/components/driver/rmt.c @@ -713,8 +713,9 @@ esp_err_t rmt_config(const rmt_config_t *rmt_param) static void IRAM_ATTR rmt_fill_memory(rmt_channel_t channel, const rmt_item32_t *item, uint16_t item_num, uint16_t mem_offset) { - volatile uint32_t *to = (volatile uint32_t *)&RMTMEM.chan[channel].data32[mem_offset].val; uint32_t *from = (uint32_t *)item; + volatile uint32_t *to = (volatile uint32_t *)&RMTMEM.chan[channel].data32[0].val; + to += mem_offset; while (item_num--) { *to++ = *from++; } diff --git a/components/esp_system/esp_ipc.c b/components/esp_system/esp_ipc.c index 2db3dbf921..d015cafb02 100644 --- a/components/esp_system/esp_ipc.c +++ b/components/esp_system/esp_ipc.c @@ -107,7 +107,7 @@ static void esp_ipc_init(void) __attribute__((constructor)); static void esp_ipc_init(void) { - char task_name[15]; + char task_name[configMAX_TASK_NAME_LEN]; for (int i = 0; i < portNUM_PROCESSORS; ++i) { snprintf(task_name, sizeof(task_name), "ipc%d", i); diff --git a/components/vfs/vfs.c b/components/vfs/vfs.c index a0e42cff78..d8c034bc0e 100644 --- a/components/vfs/vfs.c +++ b/components/vfs/vfs.c @@ -167,7 +167,7 @@ esp_err_t esp_vfs_register_with_id(const esp_vfs_t *vfs, void *ctx, esp_vfs_id_t esp_err_t esp_vfs_unregister_with_id(esp_vfs_id_t vfs_id) { - if (vfs_id < 0 || vfs_id >= MAX_FDS || s_vfs[vfs_id] == NULL) { + if (vfs_id < 0 || vfs_id >= VFS_MAX_COUNT || s_vfs[vfs_id] == NULL) { return ESP_ERR_INVALID_ARG; } vfs_entry_t* vfs = s_vfs[vfs_id]; From 5732e2a4be0f6c78c9c87471984d23554fa9298f Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 8 Apr 2022 13:31:29 +0800 Subject: [PATCH 4/6] driver: fix dead code in error handling path ... for gptimer and pulse_cnt driver, reported by Coverity Scan --- components/driver/deprecated/driver/pcnt.h | 1 + components/driver/gptimer.c | 116 ++++++++++-------- components/driver/include/driver/pulse_cnt.h | 4 + components/driver/pulse_cnt.c | 112 ++++++++++------- docs/en/api-reference/peripherals/gptimer.rst | 14 ++- docs/en/api-reference/peripherals/pcnt.rst | 13 +- 6 files changed, 164 insertions(+), 96 deletions(-) diff --git a/components/driver/deprecated/driver/pcnt.h b/components/driver/deprecated/driver/pcnt.h index 557d7f3348..94288601ca 100644 --- a/components/driver/deprecated/driver/pcnt.h +++ b/components/driver/deprecated/driver/pcnt.h @@ -6,6 +6,7 @@ #pragma once +#include #include "sdkconfig.h" #include "esp_err.h" #include "driver/pcnt_types_legacy.h" diff --git a/components/driver/gptimer.c b/components/driver/gptimer.c index 6044d8523a..362b05f4e5 100644 --- a/components/driver/gptimer.c +++ b/components/driver/gptimer.c @@ -100,27 +100,17 @@ static void gptimer_release_group_handle(gptimer_group_t *group); static esp_err_t gptimer_select_periph_clock(gptimer_t *timer, gptimer_clock_source_t src_clk, uint32_t resolution_hz); static void gptimer_default_isr(void *args); -esp_err_t gptimer_new_timer(const gptimer_config_t *config, gptimer_handle_t *ret_timer) +static esp_err_t gptimer_register_to_group(gptimer_t *timer) { - esp_err_t ret = ESP_OK; gptimer_group_t *group = NULL; - gptimer_t *timer = NULL; - int group_id = -1; int timer_id = -1; - ESP_GOTO_ON_FALSE(config && ret_timer, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); - ESP_GOTO_ON_FALSE(config->resolution_hz, ESP_ERR_INVALID_ARG, err, TAG, "invalid timer resolution:%d", config->resolution_hz); - - timer = heap_caps_calloc(1, sizeof(gptimer_t), GPTIMER_MEM_ALLOC_CAPS); - ESP_GOTO_ON_FALSE(timer, ESP_ERR_NO_MEM, err, TAG, "no mem for gptimer"); - - for (int i = 0; (i < SOC_TIMER_GROUPS) && (timer_id < 0); i++) { + for (int i = 0; i < SOC_TIMER_GROUPS; i++) { group = gptimer_acquire_group_handle(i); - ESP_GOTO_ON_FALSE(group, ESP_ERR_NO_MEM, err, TAG, "no mem for group (%d)", i); + ESP_RETURN_ON_FALSE(group, ESP_ERR_NO_MEM, TAG, "no mem for group (%d)", i); // loop to search free timer in the group portENTER_CRITICAL(&group->spinlock); for (int j = 0; j < SOC_TIMER_GROUP_TIMERS_PER_GROUP; j++) { if (!group->timers[j]) { - group_id = i; timer_id = j; group->timers[j] = timer; break; @@ -130,12 +120,60 @@ esp_err_t gptimer_new_timer(const gptimer_config_t *config, gptimer_handle_t *re if (timer_id < 0) { gptimer_release_group_handle(group); group = NULL; + } else { + timer->timer_id = timer_id; + timer->group = group; + break;; } } + ESP_RETURN_ON_FALSE(timer_id != -1, ESP_ERR_NOT_FOUND, TAG, "no free timer"); + return ESP_OK; +} + +static void gptimer_unregister_from_group(gptimer_t *timer) +{ + gptimer_group_t *group = timer->group; + int timer_id = timer->timer_id; + portENTER_CRITICAL(&group->spinlock); + group->timers[timer_id] = NULL; + portEXIT_CRITICAL(&group->spinlock); + // timer has a reference on group, release it now + gptimer_release_group_handle(group); +} + +static esp_err_t gptimer_destory(gptimer_t *timer) +{ + if (timer->pm_lock) { + ESP_RETURN_ON_ERROR(esp_pm_lock_delete(timer->pm_lock), TAG, "delete pm_lock failed"); + } + if (timer->intr) { + ESP_RETURN_ON_ERROR(esp_intr_free(timer->intr), TAG, "delete interrupt service failed"); + } + if (timer->group) { + gptimer_unregister_from_group(timer); + } + free(timer); + return ESP_OK; +} + +esp_err_t gptimer_new_timer(const gptimer_config_t *config, gptimer_handle_t *ret_timer) +{ +#if CONFIG_GPTIMER_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif + esp_err_t ret = ESP_OK; + gptimer_t *timer = NULL; + ESP_GOTO_ON_FALSE(config && ret_timer, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); + ESP_GOTO_ON_FALSE(config->resolution_hz, ESP_ERR_INVALID_ARG, err, TAG, "invalid timer resolution:%d", config->resolution_hz); + + timer = heap_caps_calloc(1, sizeof(gptimer_t), GPTIMER_MEM_ALLOC_CAPS); + ESP_GOTO_ON_FALSE(timer, ESP_ERR_NO_MEM, err, TAG, "no mem for gptimer"); + // register timer to the group (because one group can have several timers) + ESP_GOTO_ON_ERROR(gptimer_register_to_group(timer), err, TAG, "register timer failed"); + gptimer_group_t *group = timer->group; + int group_id = group->group_id; + int timer_id = timer->timer_id; - ESP_GOTO_ON_FALSE(timer_id != -1, ESP_ERR_NOT_FOUND, err, TAG, "no free timer"); - timer->timer_id = timer_id; - timer->group = group; // initialize HAL layer timer_hal_init(&timer->hal, group_id, timer_id); // stop counter, alarm, auto-reload @@ -165,49 +203,27 @@ esp_err_t gptimer_new_timer(const gptimer_config_t *config, gptimer_handle_t *re err: if (timer) { - if (timer->pm_lock) { - esp_pm_lock_delete(timer->pm_lock); - } - free(timer); - } - if (group) { - gptimer_release_group_handle(group); + gptimer_destory(timer); } return ret; } esp_err_t gptimer_del_timer(gptimer_handle_t timer) { - gptimer_group_t *group = NULL; - bool valid_state = true; ESP_RETURN_ON_FALSE(timer, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); - portENTER_CRITICAL(&timer->spinlock); - if (timer->fsm != GPTIMER_FSM_STOP) { - valid_state = false; - } - portEXIT_CRITICAL(&timer->spinlock); - ESP_RETURN_ON_FALSE(valid_state, ESP_ERR_INVALID_STATE, TAG, "can't delete timer as it's not in stop state"); - group = timer->group; + gptimer_group_t *group = timer->group; int group_id = group->group_id; int timer_id = timer->timer_id; - if (timer->intr) { - esp_intr_free(timer->intr); - ESP_LOGD(TAG, "uninstall interrupt service for timer (%d,%d)", group_id, timer_id); - } - if (timer->pm_lock) { - esp_pm_lock_delete(timer->pm_lock); - ESP_LOGD(TAG, "uninstall APB_FREQ_MAX lock for timer (%d,%d)", group_id, timer_id); - } - free(timer); + bool valid_state = true; + portENTER_CRITICAL(&timer->spinlock); + valid_state = timer->fsm == GPTIMER_FSM_STOP; + portEXIT_CRITICAL(&timer->spinlock); + ESP_RETURN_ON_FALSE(valid_state, ESP_ERR_INVALID_STATE, TAG, "can't delete timer as it's not stop yet"); + ESP_LOGD(TAG, "del timer (%d,%d)", group_id, timer_id); - - portENTER_CRITICAL(&group->spinlock); - group->timers[timer_id] = NULL; - portEXIT_CRITICAL(&group->spinlock); - // timer has a reference on group, release it now - gptimer_release_group_handle(group); - + // recycle memory resource + ESP_RETURN_ON_ERROR(gptimer_destory(timer), TAG, "destory gptimer failed"); return ESP_OK; } @@ -275,18 +291,20 @@ esp_err_t gptimer_set_alarm_action(gptimer_handle_t timer, const gptimer_alarm_c bool valid_auto_reload = !config->flags.auto_reload_on_alarm || config->alarm_count != config->reload_count; ESP_RETURN_ON_FALSE_ISR(valid_auto_reload, ESP_ERR_INVALID_ARG, TAG, "reload count can't equal to alarm count"); + portENTER_CRITICAL_SAFE(&timer->spinlock); timer->reload_count = config->reload_count; timer->alarm_count = config->alarm_count; timer->flags.auto_reload_on_alarm = config->flags.auto_reload_on_alarm; timer->flags.alarm_en = true; - portENTER_CRITICAL_SAFE(&timer->spinlock); timer_ll_set_reload_value(timer->hal.dev, timer->timer_id, config->reload_count); timer_ll_set_alarm_value(timer->hal.dev, timer->timer_id, config->alarm_count); portEXIT_CRITICAL_SAFE(&timer->spinlock); } else { + portENTER_CRITICAL_SAFE(&timer->spinlock); timer->flags.auto_reload_on_alarm = false; timer->flags.alarm_en = false; + portEXIT_CRITICAL_SAFE(&timer->spinlock); } portENTER_CRITICAL_SAFE(&timer->spinlock); diff --git a/components/driver/include/driver/pulse_cnt.h b/components/driver/include/driver/pulse_cnt.h index 17fa06bb9f..387be46efd 100644 --- a/components/driver/include/driver/pulse_cnt.h +++ b/components/driver/include/driver/pulse_cnt.h @@ -134,6 +134,7 @@ esp_err_t pcnt_unit_set_glitch_filter(pcnt_unit_handle_t unit, const pcnt_glitch * * @note This function will acquire the PM lock when power management is enabled. Also see `pcnt_unit_set_glitch_filter()` for the condition of PM lock installation. * @note The number of calls to this function should be equal to the number of calls to `pcnt_unit_stop()`. + * @note This function is allowed to run within ISR context * @note This function will be placed into IRAM if `CONFIG_PCNT_CTRL_FUNC_IN_IRAM`, so that it's allowed to be executed when Cache is disabled * * @param[in] unit PCNT unit handle created by `pcnt_new_unit()` @@ -151,6 +152,7 @@ esp_err_t pcnt_unit_start(pcnt_unit_handle_t unit); * Also see `pcnt_unit_set_glitch_filter()` for the condition of PM lock installation. * @note The stop operation won't clear the counter. Also see `pcnt_unit_clear_count()` for how to clear pulse count value. * @note The number of calls to this function should be equal to the number of calls to `pcnt_unit_start()`. + * @note This function is allowed to run within ISR context * @note This function will be placed into IRAM if `CONFIG_PCNT_CTRL_FUNC_IN_IRAM`, so that it is allowed to be executed when Cache is disabled * * @param[in] unit PCNT unit handle created by `pcnt_new_unit()` @@ -165,6 +167,7 @@ esp_err_t pcnt_unit_stop(pcnt_unit_handle_t unit); * @brief Clear PCNT pulse count value to zero * * @note It's recommended to call this function after adding a watch point by `pcnt_unit_add_watch_point()`, so that the newly added watch point is effective immediately. + * @note This function is allowed to run within ISR context * @note This function will be placed into IRAM if `CONFIG_PCNT_CTRL_FUNC_IN_IRAM`, so that it's allowed to be executed when Cache is disabled * * @param[in] unit PCNT unit handle created by `pcnt_new_unit()` @@ -178,6 +181,7 @@ esp_err_t pcnt_unit_clear_count(pcnt_unit_handle_t unit); /** * @brief Get PCNT count value * + * @note This function is allowed to run within ISR context * @note This function will be placed into IRAM if `CONFIG_PCNT_CTRL_FUNC_IN_IRAM`, so that it's allowed to be executed when Cache is disabled * * @param[in] unit PCNT unit handle created by `pcnt_new_unit()` diff --git a/components/driver/pulse_cnt.c b/components/driver/pulse_cnt.c index 15a8ebdb16..ada6015881 100644 --- a/components/driver/pulse_cnt.c +++ b/components/driver/pulse_cnt.c @@ -108,28 +108,17 @@ static pcnt_group_t *pcnt_acquire_group_handle(int group_id); static void pcnt_release_group_handle(pcnt_group_t *group); static void pcnt_default_isr(void *args); -esp_err_t pcnt_new_unit(const pcnt_unit_config_t *config, pcnt_unit_handle_t *ret_unit) +static esp_err_t pcnt_register_to_group(pcnt_unit_t *unit) { - esp_err_t ret = ESP_OK; pcnt_group_t *group = NULL; - pcnt_unit_t *unit = NULL; - int group_id = -1; int unit_id = -1; - ESP_GOTO_ON_FALSE(config && ret_unit, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); - ESP_GOTO_ON_FALSE(config->low_limit < 0 && config->high_limit > 0 && config->low_limit >= PCNT_LL_MIN_LIN && config->high_limit <= PCNT_LL_MAX_LIM, - ESP_ERR_INVALID_ARG, err, TAG, "invalid limit range:[%d,%d]", config->low_limit, config->high_limit); - - unit = heap_caps_calloc(1, sizeof(pcnt_unit_t), PCNT_MEM_ALLOC_CAPS); - ESP_GOTO_ON_FALSE(unit, ESP_ERR_NO_MEM, err, TAG, "no mem for unit"); - - for (int i = 0; (i < SOC_PCNT_GROUPS) && (unit_id < 0); i++) { + for (int i = 0; i < SOC_PCNT_GROUPS; i++) { group = pcnt_acquire_group_handle(i); - ESP_GOTO_ON_FALSE(group, ESP_ERR_NO_MEM, err, TAG, "no mem for group (%d)", i); + ESP_RETURN_ON_FALSE(group, ESP_ERR_NO_MEM, TAG, "no mem for group (%d)", i); // loop to search free unit in the group portENTER_CRITICAL(&group->spinlock); for (int j = 0; j < SOC_PCNT_UNITS_PER_GROUP; j++) { if (!group->units[j]) { - group_id = i; unit_id = j; group->units[j] = unit; break; @@ -139,12 +128,63 @@ esp_err_t pcnt_new_unit(const pcnt_unit_config_t *config, pcnt_unit_handle_t *re if (unit_id < 0) { pcnt_release_group_handle(group); group = NULL; + } else { + unit->group = group; + unit->unit_id = unit_id; + break; } } + ESP_RETURN_ON_FALSE(unit_id != -1, ESP_ERR_NOT_FOUND, TAG, "no free unit"); + return ESP_OK; +} - ESP_GOTO_ON_FALSE(unit_id != -1, ESP_ERR_NOT_FOUND, err, TAG, "no free unit"); - unit->group = group; - unit->unit_id = unit_id; +static void pcnt_unregister_from_group(pcnt_unit_t *unit) +{ + pcnt_group_t *group = unit->group; + int unit_id = unit->unit_id; + portENTER_CRITICAL(&group->spinlock); + group->units[unit_id] = NULL; + portEXIT_CRITICAL(&group->spinlock); + // unit has a reference on group, release it now + pcnt_release_group_handle(group); +} + +static esp_err_t pcnt_destory(pcnt_unit_t *unit) +{ + if (unit->pm_lock) { + // if pcnt_unit_start and pcnt_unit_stop are not called the same number of times, deleting pm_lock will return invalid state + // which in return also reflects an invalid state of PCNT driver + ESP_RETURN_ON_ERROR(esp_pm_lock_delete(unit->pm_lock), TAG, "delete pm lock failed"); + } + if (unit->intr) { + ESP_RETURN_ON_ERROR(esp_intr_free(unit->intr), TAG, "delete interrupt service failed"); + } + if (unit->group) { + pcnt_unregister_from_group(unit); + } + free(unit); + return ESP_OK; +} + +esp_err_t pcnt_new_unit(const pcnt_unit_config_t *config, pcnt_unit_handle_t *ret_unit) +{ +#if CONFIG_PCNT_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif + esp_err_t ret = ESP_OK; + pcnt_unit_t *unit = NULL; + ESP_GOTO_ON_FALSE(config && ret_unit, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); + ESP_GOTO_ON_FALSE(config->low_limit < 0 && config->high_limit > 0 && config->low_limit >= PCNT_LL_MIN_LIN && + config->high_limit <= PCNT_LL_MAX_LIM, ESP_ERR_INVALID_ARG, err, TAG, + "invalid limit range:[%d,%d]", config->low_limit, config->high_limit); + + unit = heap_caps_calloc(1, sizeof(pcnt_unit_t), PCNT_MEM_ALLOC_CAPS); + ESP_GOTO_ON_FALSE(unit, ESP_ERR_NO_MEM, err, TAG, "no mem for unit"); + // register unit to the group (because one group can have several units) + ESP_GOTO_ON_ERROR(pcnt_register_to_group(unit), err, TAG, "register unit failed"); + pcnt_group_t *group = unit->group; + int group_id = group->group_id; + int unit_id = unit->unit_id; // some events are enabled by default, disable them all pcnt_ll_disable_all_events(group->hal.dev, unit_id); @@ -176,19 +216,24 @@ esp_err_t pcnt_new_unit(const pcnt_unit_config_t *config, pcnt_unit_handle_t *re err: if (unit) { - free(unit); - } - if (group) { - pcnt_release_group_handle(group); + pcnt_destory(unit); } return ret; } esp_err_t pcnt_del_unit(pcnt_unit_handle_t unit) { - pcnt_group_t *group = NULL; ESP_RETURN_ON_FALSE(unit, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); - ESP_RETURN_ON_FALSE(unit->fsm == PCNT_FSM_STOP, ESP_ERR_INVALID_STATE, TAG, "can't delete unit as it's not in stop state"); + pcnt_group_t *group = unit->group; + int group_id = group->group_id; + int unit_id = unit->unit_id; + + bool valid_state = true; + portENTER_CRITICAL(&unit->spinlock); + valid_state = unit->fsm == PCNT_FSM_STOP; + portEXIT_CRITICAL(&unit->spinlock); + ESP_RETURN_ON_FALSE(valid_state, ESP_ERR_INVALID_STATE, TAG, "can't delete unit as it's not stop yet"); + for (int i = 0; i < SOC_PCNT_CHANNELS_PER_UNIT; i++) { ESP_RETURN_ON_FALSE(!unit->channels[i], ESP_ERR_INVALID_STATE, TAG, "channel %d still in working", i); } @@ -196,27 +241,10 @@ esp_err_t pcnt_del_unit(pcnt_unit_handle_t unit) ESP_RETURN_ON_FALSE(unit->watchers[i].event_id == PCNT_LL_WATCH_EVENT_INVALID, ESP_ERR_INVALID_STATE, TAG, "watch point %d still in working", unit->watchers[i].watch_point_value); } - group = unit->group; - int group_id = group->group_id; - int unit_id = unit->unit_id; - portENTER_CRITICAL(&group->spinlock); - group->units[unit_id] = NULL; - portEXIT_CRITICAL(&group->spinlock); - - if (unit->intr) { - esp_intr_free(unit->intr); - ESP_LOGD(TAG, "uninstall interrupt service for unit (%d,%d)", group_id, unit_id); - } - if (unit->pm_lock) { - esp_pm_lock_delete(unit->pm_lock); - ESP_LOGD(TAG, "uninstall APB_FREQ_MAX lock for unit (%d,%d)", group_id, unit_id); - } - free(unit); ESP_LOGD(TAG, "del unit (%d,%d)", group_id, unit_id); - // unit has a reference on group, release it now - pcnt_release_group_handle(group); - + // recycle memory resource + ESP_RETURN_ON_ERROR(pcnt_destory(unit), TAG, "destory pcnt unit failed"); return ESP_OK; } diff --git a/docs/en/api-reference/peripherals/gptimer.rst b/docs/en/api-reference/peripherals/gptimer.rst index ac5e6dcc0a..5b90a35c56 100644 --- a/docs/en/api-reference/peripherals/gptimer.rst +++ b/docs/en/api-reference/peripherals/gptimer.rst @@ -237,6 +237,8 @@ When power management is enabled (i.e. :ref:`CONFIG_PM_ENABLE` is on), the syste However, the driver can prevent the system from changing APB frequency by acquiring a power management lock of type :cpp:enumerator:`ESP_PM_APB_FREQ_MAX`. Whenever the driver creates a GPTimer instance that has selected :cpp:enumerator:`GPTIMER_CLK_SRC_APB` as its clock source, the driver will guarantee that the power management lock is acquired when the timer is started by :cpp:func:`gptimer_start`. Likewise, the driver releases the lock when :cpp:func:`gptimer_stop` is called for that timer. This requires that the :cpp:func:`gptimer_start` and :cpp:func:`gptimer_stop` should appear in pairs. +If the gptimer clock source is selected to others like :cpp:enumerator:`GPTIMER_CLK_SRC_XTAL`, then the driver won't install power management lock for it, which is more suitable for a low power application as long as the source clock can still provide sufficient resolution. + IRAM Safe ^^^^^^^^^ @@ -248,7 +250,7 @@ There's a Kconfig option :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` that will: 2. Place all functions that used by the ISR into IRAM [2]_ -3. Place driver object into DRAM (in case it's linked to PSRAM by accident) +3. Place driver object into DRAM (in case it's mapped to PSRAM by accident) This will allow the interrupt to run while the cache is disabled but will come at the cost of increased IRAM consumption. @@ -264,7 +266,15 @@ Thread Safety ^^^^^^^^^^^^^ The factory function :cpp:func:`gptimer_new_timer` is guaranteed to be thread safe by the driver, which means, user can call it from different RTOS tasks without protection by extra locks. -Other functions that take the :cpp:type:`gptimer_handle_t` as the first positional parameter, are not thread safe. The lifecycle of the gptimer handle is maintained by the user. So user should avoid calling them concurrently. If it has to, then one should introduce another mutex to prevent the gptimer handle being accessed concurrently. +The following functions are allowed to run under ISR context, the driver uses a critical section to prevent them being called concurrently in both task and ISR. + +- :cpp:func:`gptimer_start` +- :cpp:func:`gptimer_stop` +- :cpp:func:`gptimer_get_raw_count` +- :cpp:func:`gptimer_set_raw_count` +- :cpp:func:`gptimer_set_alarm_action` + +Other functions that take the :cpp:type:`gptimer_handle_t` as the first positional parameter, are not treated as thread safe. Which means the user should avoid calling them from multiple tasks. Kconfig Options ^^^^^^^^^^^^^^^ diff --git a/docs/en/api-reference/peripherals/pcnt.rst b/docs/en/api-reference/peripherals/pcnt.rst index 24c1fcee6a..f2435f5739 100644 --- a/docs/en/api-reference/peripherals/pcnt.rst +++ b/docs/en/api-reference/peripherals/pcnt.rst @@ -18,7 +18,7 @@ Typically, a PCNT module can be used in scenarios like: Functional Overview ------------------- -Description of the PCNT functionality is broken down into the following sections: +Description of the PCNT functionality is divided into into the following sections: - `Resource Allocation <#resource-allocation>`__ - covers how to allocate PCNT units and channels with properly set of configurations. It also covers how to recycle the resources when they finished working. @@ -215,7 +215,7 @@ There's a Kconfig option :ref:`CONFIG_PCNT_ISR_IRAM_SAFE` that will: 2. Place all functions that used by the ISR into IRAM [2]_ -3. Place driver object into DRAM (in case it's linked to PSRAM by accident) +3. Place driver object into DRAM (in case it's mapped to PSRAM by accident) This will allow the interrupt to run while the cache is disabled but will come at the cost of increased IRAM consumption. @@ -230,7 +230,14 @@ Thread Safety ^^^^^^^^^^^^^ The factory function :cpp:func:`pcnt_new_unit` and :cpp:func:`pcnt_new_channel` are guaranteed to be thread safe by the driver, which means, user can call them from different RTOS tasks without protection by extra locks. -Other functions that take the :cpp:type:`pcnt_unit_handle_t` and :cpp:type:`pcnt_channel_handle_t` as the first positional parameter, are not thread safe. The lifecycle of the PCNT unit and channel handle is maintained by the user. So user should avoid calling them concurrently. If it has to, another mutex should be added to prevent them being accessed concurrently. +The following functions are allowed to run under ISR context, the driver uses a critical section to prevent them being called concurrently in both task and ISR. + +- :cpp:func:`pcnt_unit_start` +- :cpp:func:`pcnt_unit_stop` +- :cpp:func:`pcnt_unit_clear_count` +- :cpp:func:`pcnt_unit_get_count` + +Other functions that take the :cpp:type:`pcnt_unit_handle_t` and :cpp:type:`pcnt_channel_handle_t` as the first positional parameter, are not treated as thread safe. Which means the user should avoid calling them from multiple tasks. Kconfig Options ^^^^^^^^^^^^^^^ From a7d380c80a2686426fe1d84a69cf5388a77846c9 Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 8 Apr 2022 15:11:59 +0800 Subject: [PATCH 5/6] driver: a better way to avoid new/old driver coexistence --- components/driver/CMakeLists.txt | 1 - components/driver/deprecated/pcnt_legacy.c | 13 ++++++----- .../deprecated/rtc_temperature_legacy.c | 13 ++++++----- components/driver/deprecated/timer_legacy.c | 11 +++++---- components/driver/gptimer.c | 17 -------------- components/driver/legacy_new_driver_coexist.c | 23 ------------------- components/driver/pulse_cnt.c | 17 -------------- components/driver/temperature_sensor.c | 16 +------------ 8 files changed, 21 insertions(+), 90 deletions(-) delete mode 100644 components/driver/legacy_new_driver_coexist.c diff --git a/components/driver/CMakeLists.txt b/components/driver/CMakeLists.txt index 89f347f637..d22d3f09af 100644 --- a/components/driver/CMakeLists.txt +++ b/components/driver/CMakeLists.txt @@ -5,7 +5,6 @@ set(srcs "gptimer.c" "i2c.c" "ledc.c" - "legacy_new_driver_coexist.c" "rtc_io.c" "rtc_module.c" "sdspi_crc.c" diff --git a/components/driver/deprecated/pcnt_legacy.c b/components/driver/deprecated/pcnt_legacy.c index 4b962aaee0..a71048f1b0 100644 --- a/components/driver/deprecated/pcnt_legacy.c +++ b/components/driver/deprecated/pcnt_legacy.c @@ -31,7 +31,7 @@ #define PCNT_ENTER_CRITICAL(mux) portENTER_CRITICAL(mux) #define PCNT_EXIT_CRITICAL(mux) portEXIT_CRITICAL(mux) -static const char *TAG = "pcnt"; +static const char *TAG = "pcnt(legacy)"; #define PCNT_CHECK(a, str, ret_val) ESP_RETURN_ON_FALSE(a, ret_val, TAG, "%s", str) @@ -552,11 +552,12 @@ void pcnt_isr_service_uninstall(void) __attribute__((constructor)) static void check_pcnt_driver_conflict(void) { - extern int pcnt_driver_init_count; - pcnt_driver_init_count++; - if (pcnt_driver_init_count > 1) { - ESP_EARLY_LOGE(TAG, "CONFLICT! The pulse_cnt driver can't work along with the legacy pcnt driver"); + // This function was declared as weak here. pulse_cnt driver has one implementation. + // So if pulse_cnt driver is not linked in, then `pcnt_new_unit` should be NULL at runtime. + extern __attribute__((weak)) esp_err_t pcnt_new_unit(const void *config, void **ret_unit); + if (pcnt_new_unit != NULL) { + ESP_EARLY_LOGE(TAG, "CONFLICT! driver_ng is not allowed to be used with the legacy driver"); abort(); } - ESP_EARLY_LOGW(TAG, "legacy pcnt driver is deprecated, please migrate to use driver/pulse_cnt.h"); + ESP_EARLY_LOGW(TAG, "legacy driver is deprecated, please migrate to `driver/pulse_cnt.h`"); } diff --git a/components/driver/deprecated/rtc_temperature_legacy.c b/components/driver/deprecated/rtc_temperature_legacy.c index 75078fd742..46cb5632b3 100644 --- a/components/driver/deprecated/rtc_temperature_legacy.c +++ b/components/driver/deprecated/rtc_temperature_legacy.c @@ -7,6 +7,7 @@ #include #include #include +#include "sdkconfig.h" #include "esp_types.h" #include "esp_log.h" #include "esp_check.h" @@ -18,7 +19,6 @@ #include "hal/temperature_sensor_ll.h" #include "driver/temp_sensor_types_legacy.h" #include "esp_private/periph_ctrl.h" -#include "sdkconfig.h" static const char *TAG = "tsens"; @@ -158,11 +158,12 @@ esp_err_t temp_sensor_read_celsius(float *celsius) __attribute__((constructor)) static void check_legacy_temp_sensor_driver_conflict(void) { - extern int temp_sensor_driver_init_count; - temp_sensor_driver_init_count++; - if (temp_sensor_driver_init_count > 1) { - ESP_EARLY_LOGE(TAG, "CONFLICT! The legacy temp sensor driver can't work along with the new temperature driver"); + // This function was declared as weak here. temperature_sensor driver has one implementation. + // So if temperature_sensor driver is not linked in, then `temperature_sensor_install()` should be NULL at runtime. + extern __attribute__((weak)) esp_err_t temperature_sensor_install(const void *tsens_config, void **ret_tsens); + if (temperature_sensor_install != NULL) { + ESP_EARLY_LOGE(TAG, "CONFLICT! driver_ng is not allowed to be used with the legacy driver"); abort(); } - ESP_EARLY_LOGW(TAG, "legacy temp sensor driver is deprecated, please migrate to use driver/temperature_sensor.h"); + ESP_EARLY_LOGW(TAG, "legacy driver is deprecated, please migrate to `driver/temperature_sensor.h`"); } diff --git a/components/driver/deprecated/timer_legacy.c b/components/driver/deprecated/timer_legacy.c index 1e8f3937cf..6713b69216 100644 --- a/components/driver/deprecated/timer_legacy.c +++ b/components/driver/deprecated/timer_legacy.c @@ -480,11 +480,12 @@ esp_err_t IRAM_ATTR timer_spinlock_give(timer_group_t group_num) __attribute__((constructor)) static void check_legacy_timer_driver_conflict(void) { - extern int timer_group_driver_init_count; - timer_group_driver_init_count++; - if (timer_group_driver_init_count > 1) { - ESP_EARLY_LOGE(TIMER_TAG, "CONFLICT! The legacy timer group driver can't work along with the gptimer driver"); + // This function was declared as weak here. gptimer driver has one implementation. + // So if gptimer driver is not linked in, then `gptimer_new_timer()` should be NULL at runtime. + extern __attribute__((weak)) esp_err_t gptimer_new_timer(const void *config, void **ret_timer); + if (gptimer_new_timer != NULL) { + ESP_EARLY_LOGE(TIMER_TAG, "CONFLICT! driver_ng is not allowed to be used with the legacy driver"); abort(); } - ESP_EARLY_LOGW(TIMER_TAG, "legacy timer group driver is deprecated, please migrate to use driver/gptimer.h"); + ESP_EARLY_LOGW(TIMER_TAG, "legacy driver is deprecated, please migrate to `driver/gptimer.h`"); } diff --git a/components/driver/gptimer.c b/components/driver/gptimer.c index 362b05f4e5..1b7b37d17a 100644 --- a/components/driver/gptimer.c +++ b/components/driver/gptimer.c @@ -504,20 +504,3 @@ esp_err_t gptimer_get_pm_lock(gptimer_handle_t timer, esp_pm_lock_handle_t *ret_ *ret_pm_lock = timer->pm_lock; return ESP_OK; } - -/** - * @brief This function will be called during start up, to check that gptimer driver is not running along with the legacy timer group driver - */ -__attribute__((constructor)) -static void check_gptimer_driver_conflict(void) -{ -#if CONFIG_GPTIMER_ENABLE_DEBUG_LOG - esp_log_level_set(TAG, ESP_LOG_DEBUG); -#endif - extern int timer_group_driver_init_count; - timer_group_driver_init_count++; - if (timer_group_driver_init_count > 1) { - ESP_EARLY_LOGE(TAG, "CONFLICT! The gptimer driver can't work along with the legacy timer group driver"); - abort(); - } -} diff --git a/components/driver/legacy_new_driver_coexist.c b/components/driver/legacy_new_driver_coexist.c deleted file mode 100644 index 427809d7e7..0000000000 --- a/components/driver/legacy_new_driver_coexist.c +++ /dev/null @@ -1,23 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD - * - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * @brief This count is used to prevent the coexistence of - * the legacy timer group driver (deprecated/driver/timer.h) and the new gptimer driver (driver/gptimer.h). - */ -int timer_group_driver_init_count = 0; - -/** - * @brief This count is used to prevent the coexistence of - * the legacy pcnt driver (deprecated/driver/pcnt.h) and the new pulse_cnt driver (driver/pulse_cnt.h). - */ -int pcnt_driver_init_count = 0; - -/** - * @brief This count is used to prevent the coexistence of - * the legacy temperature sensor driver (deprecated/driver/temp_sensor.h) and the new temperature sensor driver (driver/temperature_sensor.h). - */ -int temp_sensor_driver_init_count = 0; diff --git a/components/driver/pulse_cnt.c b/components/driver/pulse_cnt.c index ada6015881..7ee93309df 100644 --- a/components/driver/pulse_cnt.c +++ b/components/driver/pulse_cnt.c @@ -724,20 +724,3 @@ IRAM_ATTR static void pcnt_default_isr(void *args) portYIELD_FROM_ISR(); } } - -/** - * @brief This function will be called during start up, to check that pulse_cnt driver is not running along with the legacy pcnt driver - */ -__attribute__((constructor)) -static void check_pulse_cnt_driver_conflict(void) -{ -#if CONFIG_PCNT_ENABLE_DEBUG_LOG - esp_log_level_set(TAG, ESP_LOG_DEBUG); -#endif - extern int pcnt_driver_init_count; - pcnt_driver_init_count++; - if (pcnt_driver_init_count > 1) { - ESP_EARLY_LOGE(TAG, "CONFLICT! The pulse_cnt driver can't work along with the legacy pcnt driver"); - abort(); - } -} diff --git a/components/driver/temperature_sensor.c b/components/driver/temperature_sensor.c index a16f1c74f6..acec435cfc 100644 --- a/components/driver/temperature_sensor.c +++ b/components/driver/temperature_sensor.c @@ -83,10 +83,10 @@ static esp_err_t temperature_sensor_choose_best_range(temperature_sensor_handle_ esp_err_t temperature_sensor_install(const temperature_sensor_config_t *tsens_config, temperature_sensor_handle_t *ret_tsens) { - esp_err_t ret = ESP_OK; #if CONFIG_TEMP_SENSOR_ENABLE_DEBUG_LOG esp_log_level_set(TAG, ESP_LOG_DEBUG); #endif + esp_err_t ret = ESP_OK; ESP_RETURN_ON_FALSE((tsens_config && ret_tsens), ESP_ERR_INVALID_ARG, TAG, "Invalid argument"); ESP_RETURN_ON_FALSE((s_tsens_attribute_copy == NULL), ESP_ERR_INVALID_STATE, TAG, "Already installed"); temperature_sensor_handle_t tsens; @@ -198,17 +198,3 @@ esp_err_t temperature_sensor_get_celsius(temperature_sensor_handle_t tsens, floa } return ESP_OK; } - -/** - * @brief This function will be called during start up, to check the new temperature driver is not running along with the legacy temp sensor driver - */ -__attribute__((constructor)) -static void check_temperature_driver_conflict(void) -{ - extern int temp_sensor_driver_init_count; - temp_sensor_driver_init_count++; - if (temp_sensor_driver_init_count > 1) { - ESP_EARLY_LOGE(TAG, "CONFLICT! The temperature driver can't work along with the legacy temp sensor driver"); - abort(); - } -} From 0a36cad9e0174bf7e2b4d2316caecd3f0e5181ca Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 7 Apr 2022 13:08:47 +0800 Subject: [PATCH 6/6] driver: register test app component by WHOLE_ARCHIVE --- components/driver/test_apps/gpio/main/CMakeLists.txt | 11 ++++------- .../driver/test_apps/gpio/main/test_dedicated_gpio.c | 4 ---- components/driver/test_apps/gpio/main/test_gpio.c | 4 ---- .../driver/test_apps/gpio/main/test_sigmadelta.c | 4 ---- .../driver/test_apps/gptimer/main/CMakeLists.txt | 7 ++++--- .../driver/test_apps/gptimer/main/test_gptimer.c | 4 ---- .../driver/test_apps/gptimer/main/test_gptimer_iram.c | 4 ---- .../test_apps/legacy_pcnt_driver/main/CMakeLists.txt | 7 ++++--- .../legacy_pcnt_driver/main/test_legacy_pcnt.c | 4 ---- .../legacy_pcnt_driver/pytest_legacy_pcnt.py | 2 +- .../legacy_rtc_temp_driver/main/CMakeLists.txt | 6 +++--- .../main/test_rtc_temp_driver.c | 4 ---- .../test_apps/legacy_timer_driver/main/CMakeLists.txt | 7 ++++--- .../legacy_timer_driver/main/test_legacy_timer.c | 4 ---- .../driver/test_apps/pulse_cnt/main/CMakeLists.txt | 7 ++++--- .../driver/test_apps/pulse_cnt/main/test_pulse_cnt.c | 4 ---- .../driver/test_apps/pulse_cnt/pytest_pulse_cnt.py | 2 +- .../test_apps/temperature_sensor/CMakeLists.txt | 2 +- .../test_apps/temperature_sensor/main/CMakeLists.txt | 6 +++--- .../temperature_sensor/main/test_temperature_sensor.c | 4 ---- .../esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt | 7 ++++--- .../test_apps/i2c_lcd/main/test_i2c_lcd_panel.c | 4 ---- .../esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt | 7 ++++--- .../test_apps/i80_lcd/main/test_i80_lcd_panel.c | 4 ---- .../esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt | 7 ++++--- .../esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c | 4 ---- .../esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt | 7 ++++--- .../test_apps/spi_lcd/main/test_spi_lcd_panel.c | 4 ---- 28 files changed, 45 insertions(+), 96 deletions(-) diff --git a/components/driver/test_apps/gpio/main/CMakeLists.txt b/components/driver/test_apps/gpio/main/CMakeLists.txt index 532eca65f6..d60fc4d15e 100644 --- a/components/driver/test_apps/gpio/main/CMakeLists.txt +++ b/components/driver/test_apps/gpio/main/CMakeLists.txt @@ -1,18 +1,15 @@ set(srcs "test_app_main.c" "test_gpio.c") -set(include_tests "-u test_app_include_gpio") - if(CONFIG_SOC_DEDICATED_GPIO_SUPPORTED) list(APPEND srcs "test_dedicated_gpio.c") - list(APPEND include_tests "-u test_app_include_dedicated_gpio") endif() if(CONFIG_SOC_SIGMADELTA_SUPPORTED) list(APPEND srcs "test_sigmadelta.c") - list(APPEND include_tests "-u test_app_include_sigmadelta") endif() -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE ${include_tests}) +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/driver/test_apps/gpio/main/test_dedicated_gpio.c b/components/driver/test_apps/gpio/main/test_dedicated_gpio.c index 5f33da1923..6a81648912 100644 --- a/components/driver/test_apps/gpio/main/test_dedicated_gpio.c +++ b/components/driver/test_apps/gpio/main/test_dedicated_gpio.c @@ -15,10 +15,6 @@ #include "driver/gpio.h" #include "driver/dedic_gpio.h" -void test_app_include_dedicated_gpio(void) -{ -} - TEST_CASE("Dedicated_GPIO_bundle_install/uninstall", "[dedic_gpio]") { const int test_gpios[SOC_DEDIC_GPIO_OUT_CHANNELS_NUM / 2] = {0}; diff --git a/components/driver/test_apps/gpio/main/test_gpio.c b/components/driver/test_apps/gpio/main/test_gpio.c index 48e4a90403..d961bb930c 100644 --- a/components/driver/test_apps/gpio/main/test_gpio.c +++ b/components/driver/test_apps/gpio/main/test_gpio.c @@ -32,10 +32,6 @@ #include "esp_spi_flash.h" #include "esp_attr.h" -void test_app_include_gpio(void) -{ -} - // Enable internal routing for the output and input gpio pins #define TEST_GPIO_INTERNAL_ROUTING 1 diff --git a/components/driver/test_apps/gpio/main/test_sigmadelta.c b/components/driver/test_apps/gpio/main/test_sigmadelta.c index 6c25e57bf2..2cf3f472e2 100644 --- a/components/driver/test_apps/gpio/main/test_sigmadelta.c +++ b/components/driver/test_apps/gpio/main/test_sigmadelta.c @@ -10,10 +10,6 @@ #include "soc/soc_caps.h" #include "driver/sigmadelta.h" -void test_app_include_sigmadelta(void) -{ -} - TEST_CASE("SigmaDelta_config_test", "[sigma_delta]") { sigmadelta_config_t sigmadelta_cfg = { diff --git a/components/driver/test_apps/gptimer/main/CMakeLists.txt b/components/driver/test_apps/gptimer/main/CMakeLists.txt index 252a616260..4170f3fd44 100644 --- a/components/driver/test_apps/gptimer/main/CMakeLists.txt +++ b/components/driver/test_apps/gptimer/main/CMakeLists.txt @@ -2,6 +2,7 @@ set(srcs "test_app_main.c" "test_gptimer.c" "test_gptimer_iram.c") -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_gptimer" "-u test_app_include_gptimer_iram") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/driver/test_apps/gptimer/main/test_gptimer.c b/components/driver/test_apps/gptimer/main/test_gptimer.c index 58783b41d8..82b9fa0a37 100644 --- a/components/driver/test_apps/gptimer/main/test_gptimer.c +++ b/components/driver/test_apps/gptimer/main/test_gptimer.c @@ -19,10 +19,6 @@ #define TEST_ALARM_CALLBACK_ATTR #endif // CONFIG_GPTIMER_ISR_IRAM_SAFE -void test_app_include_gptimer(void) -{ -} - TEST_CASE("gptimer_set_get_raw_count", "[gptimer]") { gptimer_config_t config = { diff --git a/components/driver/test_apps/gptimer/main/test_gptimer_iram.c b/components/driver/test_apps/gptimer/main/test_gptimer_iram.c index cfbeb1bede..5b1e242e06 100644 --- a/components/driver/test_apps/gptimer/main/test_gptimer_iram.c +++ b/components/driver/test_apps/gptimer/main/test_gptimer_iram.c @@ -14,10 +14,6 @@ #include "esp_spi_flash.h" #include "soc/soc_caps.h" -void test_app_include_gptimer_iram(void) -{ -} - #if CONFIG_GPTIMER_ISR_IRAM_SAFE typedef struct { diff --git a/components/driver/test_apps/legacy_pcnt_driver/main/CMakeLists.txt b/components/driver/test_apps/legacy_pcnt_driver/main/CMakeLists.txt index cdcd148961..193f3ba02d 100644 --- a/components/driver/test_apps/legacy_pcnt_driver/main/CMakeLists.txt +++ b/components/driver/test_apps/legacy_pcnt_driver/main/CMakeLists.txt @@ -1,6 +1,7 @@ set(srcs "test_app_main.c" "test_legacy_pcnt.c") -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_legacy_pcnt") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/driver/test_apps/legacy_pcnt_driver/main/test_legacy_pcnt.c b/components/driver/test_apps/legacy_pcnt_driver/main/test_legacy_pcnt.c index 147cc1d354..af54d80722 100644 --- a/components/driver/test_apps/legacy_pcnt_driver/main/test_legacy_pcnt.c +++ b/components/driver/test_apps/legacy_pcnt_driver/main/test_legacy_pcnt.c @@ -666,7 +666,3 @@ TEST_CASE("PCNT_counting_mode_test", "[pcnt]") printf("PCNT mode test for negative count\n"); count_mode_test(PCNT_CTRL_GND_IO); } - -void test_app_include_legacy_pcnt(void) -{ -} diff --git a/components/driver/test_apps/legacy_pcnt_driver/pytest_legacy_pcnt.py b/components/driver/test_apps/legacy_pcnt_driver/pytest_legacy_pcnt.py index bb120bc193..8bcfdbd997 100644 --- a/components/driver/test_apps/legacy_pcnt_driver/pytest_legacy_pcnt.py +++ b/components/driver/test_apps/legacy_pcnt_driver/pytest_legacy_pcnt.py @@ -16,7 +16,7 @@ from pytest_embedded import Dut ], indirect=True, ) -def test_gptimer(dut: Dut) -> None: +def test_legacy_pcnt(dut: Dut) -> None: dut.expect_exact('Press ENTER to see the list of tests') dut.write('*') dut.expect_unity_test_output(timeout=240) diff --git a/components/driver/test_apps/legacy_rtc_temp_driver/main/CMakeLists.txt b/components/driver/test_apps/legacy_rtc_temp_driver/main/CMakeLists.txt index 483019a963..ab7674124a 100644 --- a/components/driver/test_apps/legacy_rtc_temp_driver/main/CMakeLists.txt +++ b/components/driver/test_apps/legacy_rtc_temp_driver/main/CMakeLists.txt @@ -1,7 +1,7 @@ set(srcs "test_app_main.c" "test_rtc_temp_driver.c") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE idf_component_register(SRCS ${srcs} - PRIV_REQUIRES driver unity) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_rtc_temp_driver") + WHOLE_ARCHIVE) diff --git a/components/driver/test_apps/legacy_rtc_temp_driver/main/test_rtc_temp_driver.c b/components/driver/test_apps/legacy_rtc_temp_driver/main/test_rtc_temp_driver.c index aa3071b852..7b3fc55a7a 100644 --- a/components/driver/test_apps/legacy_rtc_temp_driver/main/test_rtc_temp_driver.c +++ b/components/driver/test_apps/legacy_rtc_temp_driver/main/test_rtc_temp_driver.c @@ -9,10 +9,6 @@ #include "unity.h" #include "driver/temp_sensor.h" -void test_app_include_rtc_temp_driver(void) -{ -} - TEST_CASE("Temperature_legacy_workflow_test", "[hw_timer]") { printf("Initializing Temperature sensor\n"); diff --git a/components/driver/test_apps/legacy_timer_driver/main/CMakeLists.txt b/components/driver/test_apps/legacy_timer_driver/main/CMakeLists.txt index fc2def7714..fea9144c4d 100644 --- a/components/driver/test_apps/legacy_timer_driver/main/CMakeLists.txt +++ b/components/driver/test_apps/legacy_timer_driver/main/CMakeLists.txt @@ -1,6 +1,7 @@ set(srcs "test_app_main.c" "test_legacy_timer.c") -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_legacy_timer") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/driver/test_apps/legacy_timer_driver/main/test_legacy_timer.c b/components/driver/test_apps/legacy_timer_driver/main/test_legacy_timer.c index f2e5eabad3..604de5382e 100644 --- a/components/driver/test_apps/legacy_timer_driver/main/test_legacy_timer.c +++ b/components/driver/test_apps/legacy_timer_driver/main/test_legacy_timer.c @@ -1007,7 +1007,3 @@ TEST_CASE("Timer_check_reinitialization_sequence", "[hw_timer]") // The pending timer interrupt should not be triggered TEST_ASSERT_EQUAL(0, timer_group_get_intr_status_in_isr(TIMER_GROUP_0) & TIMER_INTR_T0); } - -void test_app_include_legacy_timer(void) -{ -} diff --git a/components/driver/test_apps/pulse_cnt/main/CMakeLists.txt b/components/driver/test_apps/pulse_cnt/main/CMakeLists.txt index 37a2f532da..c035b833ec 100644 --- a/components/driver/test_apps/pulse_cnt/main/CMakeLists.txt +++ b/components/driver/test_apps/pulse_cnt/main/CMakeLists.txt @@ -1,6 +1,7 @@ set(srcs "test_app_main.c" "test_pulse_cnt.c") -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_pulse_cnt") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/driver/test_apps/pulse_cnt/main/test_pulse_cnt.c b/components/driver/test_apps/pulse_cnt/main/test_pulse_cnt.c index e6dc900e04..8a8b3b6be9 100644 --- a/components/driver/test_apps/pulse_cnt/main/test_pulse_cnt.c +++ b/components/driver/test_apps/pulse_cnt/main/test_pulse_cnt.c @@ -402,7 +402,3 @@ TEST_CASE("pcnt_zero_cross_mode", "[pcnt]") TEST_ESP_OK(pcnt_del_channel(channelB)); TEST_ESP_OK(pcnt_del_unit(unit)); } - -void test_app_include_pulse_cnt(void) -{ -} diff --git a/components/driver/test_apps/pulse_cnt/pytest_pulse_cnt.py b/components/driver/test_apps/pulse_cnt/pytest_pulse_cnt.py index 8e1f892417..f127535505 100644 --- a/components/driver/test_apps/pulse_cnt/pytest_pulse_cnt.py +++ b/components/driver/test_apps/pulse_cnt/pytest_pulse_cnt.py @@ -17,7 +17,7 @@ from pytest_embedded import Dut ], indirect=True, ) -def test_gptimer(dut: Dut) -> None: +def test_pulse_cnt(dut: Dut) -> None: dut.expect_exact('Press ENTER to see the list of tests') dut.write('*') dut.expect_unity_test_output() diff --git a/components/driver/test_apps/temperature_sensor/CMakeLists.txt b/components/driver/test_apps/temperature_sensor/CMakeLists.txt index bc01ce7841..e8b2c77d1f 100644 --- a/components/driver/test_apps/temperature_sensor/CMakeLists.txt +++ b/components/driver/test_apps/temperature_sensor/CMakeLists.txt @@ -2,4 +2,4 @@ cmake_minimum_required(VERSION 3.5) include($ENV{IDF_PATH}/tools/cmake/project.cmake) -project(test_app_include_temperature_sensor) +project(test_temperature_sensor) diff --git a/components/driver/test_apps/temperature_sensor/main/CMakeLists.txt b/components/driver/test_apps/temperature_sensor/main/CMakeLists.txt index 2bffac2c61..56250ad55a 100644 --- a/components/driver/test_apps/temperature_sensor/main/CMakeLists.txt +++ b/components/driver/test_apps/temperature_sensor/main/CMakeLists.txt @@ -1,7 +1,7 @@ set(srcs "test_app_main.c" "test_temperature_sensor.c") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE idf_component_register(SRCS ${srcs} - PRIV_REQUIRES driver unity) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_temperature_sensor") + WHOLE_ARCHIVE) diff --git a/components/driver/test_apps/temperature_sensor/main/test_temperature_sensor.c b/components/driver/test_apps/temperature_sensor/main/test_temperature_sensor.c index f5dcbab265..0269e28fc9 100644 --- a/components/driver/test_apps/temperature_sensor/main/test_temperature_sensor.c +++ b/components/driver/test_apps/temperature_sensor/main/test_temperature_sensor.c @@ -9,10 +9,6 @@ #include "unity.h" #include "driver/temperature_sensor.h" -void test_app_include_temperature_sensor(void) -{ -} - TEST_CASE("Temperature_sensor_driver_workflow_test", "[temperature_sensor]") { printf("Initializing Temperature sensor\n"); diff --git a/components/esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt b/components/esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt index 1e685eb807..db4aa94946 100644 --- a/components/esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt +++ b/components/esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt @@ -1,6 +1,7 @@ set(srcs "test_app_main.c" "test_i2c_lcd_panel.c") -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_i2c_lcd") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/esp_lcd/test_apps/i2c_lcd/main/test_i2c_lcd_panel.c b/components/esp_lcd/test_apps/i2c_lcd/main/test_i2c_lcd_panel.c index 83ee5590d2..f71c1c4a3a 100644 --- a/components/esp_lcd/test_apps/i2c_lcd/main/test_i2c_lcd_panel.c +++ b/components/esp_lcd/test_apps/i2c_lcd/main/test_i2c_lcd_panel.c @@ -14,10 +14,6 @@ #include "esp_system.h" #include "test_i2c_board.h" -void test_app_include_i2c_lcd(void) -{ -} - TEST_CASE("lcd_panel_with_i2c_interface_(ssd1306)", "[lcd]") { const uint8_t pattern[][16] = {{ diff --git a/components/esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt b/components/esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt index eef50b7e7c..7e50a26ef7 100644 --- a/components/esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt +++ b/components/esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt @@ -1,6 +1,7 @@ set(srcs "test_app_main.c" "test_i80_lcd_panel.c") -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_i80_lcd") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/esp_lcd/test_apps/i80_lcd/main/test_i80_lcd_panel.c b/components/esp_lcd/test_apps/i80_lcd/main/test_i80_lcd_panel.c index 9c7ee6524c..c020a70c32 100644 --- a/components/esp_lcd/test_apps/i80_lcd/main/test_i80_lcd_panel.c +++ b/components/esp_lcd/test_apps/i80_lcd/main/test_i80_lcd_panel.c @@ -17,10 +17,6 @@ #include "driver/gpio.h" #include "test_i80_board.h" -void test_app_include_i80_lcd(void) -{ -} - #if SOC_I2S_LCD_I80_VARIANT #include "driver/i2s.h" diff --git a/components/esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt b/components/esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt index ecad51a17c..ade68f9236 100644 --- a/components/esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt +++ b/components/esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt @@ -1,6 +1,7 @@ set(srcs "test_app_main.c" "test_rgb_panel.c") -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_rgb_lcd") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c b/components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c index 0e35ad3cbd..60b2dd7716 100644 --- a/components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c +++ b/components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c @@ -23,10 +23,6 @@ #define TEST_IMG_SIZE (100 * 100 * sizeof(uint16_t)) -void test_app_include_rgb_lcd(void) -{ -} - static esp_lcd_panel_handle_t test_rgb_panel_initialization(bool stream_mode, esp_lcd_rgb_panel_frame_trans_done_cb_t cb, void *user_data) { esp_lcd_panel_handle_t panel_handle = NULL; diff --git a/components/esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt b/components/esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt index aacecc1821..2c2b5de923 100644 --- a/components/esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt +++ b/components/esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt @@ -1,6 +1,7 @@ set(srcs "test_app_main.c" "test_spi_lcd_panel.c") -idf_component_register(SRCS ${srcs}) - -target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_spi_lcd") +# In order for the cases defined by `TEST_CASE` to be linked into the final elf, +# the component can be registered as WHOLE_ARCHIVE +idf_component_register(SRCS ${srcs} + WHOLE_ARCHIVE) diff --git a/components/esp_lcd/test_apps/spi_lcd/main/test_spi_lcd_panel.c b/components/esp_lcd/test_apps/spi_lcd/main/test_spi_lcd_panel.c index ea9514fbbd..c02426160c 100644 --- a/components/esp_lcd/test_apps/spi_lcd/main/test_spi_lcd_panel.c +++ b/components/esp_lcd/test_apps/spi_lcd/main/test_spi_lcd_panel.c @@ -18,10 +18,6 @@ #define TEST_SPI_HOST_ID SPI2_HOST -void test_app_include_spi_lcd(void) -{ -} - void test_spi_lcd_common_initialize(esp_lcd_panel_io_handle_t *io_handle, esp_lcd_panel_io_color_trans_done_cb_t on_color_trans_done, void *user_data, int cmd_bits, int param_bits, bool oct_mode) {