diff --git a/components/esp_driver_gptimer/Kconfig b/components/esp_driver_gptimer/Kconfig index cac9d11b17..2d2726eb32 100644 --- a/components/esp_driver_gptimer/Kconfig +++ b/components/esp_driver_gptimer/Kconfig @@ -1,31 +1,45 @@ menu "ESP-Driver:GPTimer Configurations" depends on SOC_GPTIMER_SUPPORTED + config GPTIMER_ISR_HANDLER_IN_IRAM - bool "Place GPTimer ISR handler into IRAM" + bool "Place GPTimer ISR handler in IRAM to reduce latency" default y + select GPTIMER_OBJ_CACHE_SAFE help - Place GPTimer ISR handler into IRAM for better performance and fewer cache misses. + Place GPTimer ISR handler in IRAM to reduce latency caused by cache miss. config GPTIMER_CTRL_FUNC_IN_IRAM - bool "Place GPTimer control functions into IRAM" + bool "Place GPTimer control functions in IRAM" default n + select GPTIMER_OBJ_CACHE_SAFE help - Place GPTimer control functions (like start/stop) into IRAM, - so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context. - Enabling this option can improve driver performance as well. + Place GPTimer control functions (like start/stop) in IRAM, to reduce latency caused by cache miss. + If enabled, these functions can also be called when cache is disabled. config GPTIMER_ISR_IRAM_SAFE - bool "GPTimer ISR IRAM-Safe" + bool "Allow GPTimer ISR to execute when cache is disabled" select GPTIMER_ISR_HANDLER_IN_IRAM default n help - Ensure the GPTimer interrupt is IRAM-Safe by allowing the interrupt handler to be - executable when the cache is disabled (e.g. SPI Flash write). + Enable this option to allow the GPTimer Interrupt Service Routine (ISR) + to execute even when the cache is disabled. This can be useful in scenarios where the cache + might be turned off, but the GPTimer functionality is still required to operate correctly. - config GPTIMER_ENABLE_DEBUG_LOG - bool "Enable debug log" + config GPTIMER_OBJ_CACHE_SAFE + bool default n help - whether to enable the debug log message for GPTimer driver. - Note that, this option only controls the GPTimer driver log, won't affect other drivers. + This will ensure the GPTimer object will not be allocated from a memory region + where its cache can be disabled. + + config GPTIMER_ENABLE_DEBUG_LOG + bool "Force enable debug log" + default n + help + If enabled, GPTimer component will: + 1. ignore the global logging settings + 2. compile all log messages into the binary + 3. set the runtime log level to VERBOSE + Please enable this option by caution, as it will increase the binary size. + endmenu diff --git a/components/esp_driver_gptimer/src/gptimer.c b/components/esp_driver_gptimer/src/gptimer.c index d02717dc38..6e01508665 100644 --- a/components/esp_driver_gptimer/src/gptimer.c +++ b/components/esp_driver_gptimer/src/gptimer.c @@ -1,27 +1,14 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ #include #include -#include "sdkconfig.h" -#if CONFIG_GPTIMER_ENABLE_DEBUG_LOG -// The local log level must be defined before including esp_log.h -// Set the maximum log level for this source file -#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG -#endif -#include "freertos/FreeRTOS.h" -#include "esp_attr.h" -#include "esp_err.h" -#include "esp_log.h" -#include "esp_check.h" #include "driver/gptimer.h" -#include "esp_memory_utils.h" #include "gptimer_priv.h" - -static const char *TAG = "gptimer"; +#include "esp_memory_utils.h" static void gptimer_default_isr(void *args); @@ -134,9 +121,6 @@ static esp_err_t gptimer_destroy(gptimer_t *timer) 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_RETURN_ON_FALSE(config && ret_timer, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); @@ -185,7 +169,7 @@ esp_err_t gptimer_new_timer(const gptimer_config_t *config, gptimer_handle_t *re timer->direction = config->direction; timer->intr_priority = config->intr_priority; timer->flags.intr_shared = config->flags.intr_shared; - ESP_LOGD(TAG, "new gptimer (%d,%d) at %p, resolution=%"PRIu32"Hz", group_id, timer_id, timer, timer->resolution_hz); + ESP_LOGD(TAG, "new gptimer (%d,%d) at %p, %zu bytes used", group_id, timer_id, timer, heap_caps_get_allocated_size(timer)); *ret_timer = timer; return ESP_OK; @@ -228,7 +212,9 @@ esp_err_t gptimer_del_timer(gptimer_handle_t timer) esp_err_t gptimer_set_raw_count(gptimer_handle_t timer, unsigned long long value) { - ESP_RETURN_ON_FALSE_ISR(timer, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + if (timer == NULL) { + return ESP_ERR_INVALID_ARG; + } portENTER_CRITICAL_SAFE(&timer->spinlock); timer_hal_set_counter_value(&timer->hal, value); @@ -238,7 +224,9 @@ esp_err_t gptimer_set_raw_count(gptimer_handle_t timer, unsigned long long value esp_err_t gptimer_get_raw_count(gptimer_handle_t timer, unsigned long long *value) { - ESP_RETURN_ON_FALSE_ISR(timer && value, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + if (timer == NULL || value == NULL) { + return ESP_ERR_INVALID_ARG; + } portENTER_CRITICAL_SAFE(&timer->spinlock); *value = timer_hal_capture_and_get_counter_value(&timer->hal); @@ -255,7 +243,9 @@ esp_err_t gptimer_get_resolution(gptimer_handle_t timer, uint32_t *out_resolutio esp_err_t gptimer_get_captured_count(gptimer_handle_t timer, uint64_t *value) { - ESP_RETURN_ON_FALSE_ISR(timer && value, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + if (timer == NULL || value == NULL) { + return ESP_ERR_INVALID_ARG; + } portENTER_CRITICAL_SAFE(&timer->spinlock); *value = timer_ll_get_counter_value(timer->hal.dev, timer->timer_id); @@ -305,14 +295,22 @@ esp_err_t gptimer_register_event_callbacks(gptimer_handle_t timer, const gptimer esp_err_t gptimer_set_alarm_action(gptimer_handle_t timer, const gptimer_alarm_config_t *config) { - ESP_RETURN_ON_FALSE_ISR(timer, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + if (timer == NULL) { + return ESP_ERR_INVALID_ARG; + } if (config) { #if CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM - ESP_RETURN_ON_FALSE_ISR(esp_ptr_internal(config), ESP_ERR_INVALID_ARG, TAG, "alarm config struct not in internal RAM"); + // when the function is placed in IRAM, we expect the config struct is also placed in internal RAM + // if the cache is disabled, the function can still access the config struct + if (esp_ptr_internal(config) == false) { + return ESP_ERR_INVALID_ARG; + } #endif // When auto_reload is enabled, alarm_count should not be equal to reload_count 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"); + if (valid_auto_reload == false) { + return ESP_ERR_INVALID_ARG; + } portENTER_CRITICAL_SAFE(&timer->spinlock); timer->reload_count = config->reload_count; @@ -340,6 +338,7 @@ esp_err_t gptimer_set_alarm_action(gptimer_handle_t timer, const gptimer_alarm_c esp_err_t gptimer_enable(gptimer_handle_t timer) { ESP_RETURN_ON_FALSE(timer, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + // the only acceptable FSM change: init->enable gptimer_fsm_t expected_fsm = GPTIMER_FSM_INIT; ESP_RETURN_ON_FALSE(atomic_compare_exchange_strong(&timer->fsm, &expected_fsm, GPTIMER_FSM_ENABLE), ESP_ERR_INVALID_STATE, TAG, "timer not in init state"); @@ -360,6 +359,7 @@ esp_err_t gptimer_enable(gptimer_handle_t timer) esp_err_t gptimer_disable(gptimer_handle_t timer) { ESP_RETURN_ON_FALSE(timer, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + // the only acceptable FSM change: enable->init gptimer_fsm_t expected_fsm = GPTIMER_FSM_ENABLE; ESP_RETURN_ON_FALSE(atomic_compare_exchange_strong(&timer->fsm, &expected_fsm, GPTIMER_FSM_INIT), ESP_ERR_INVALID_STATE, TAG, "timer not in enable state"); @@ -379,7 +379,14 @@ esp_err_t gptimer_disable(gptimer_handle_t timer) esp_err_t gptimer_start(gptimer_handle_t timer) { - ESP_RETURN_ON_FALSE_ISR(timer, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + if (timer == NULL) { + return ESP_ERR_INVALID_ARG; + } + + // if the timer is already started, do nothing + if (atomic_load(&timer->fsm) == GPTIMER_FSM_RUN) { + return ESP_OK; + } gptimer_fsm_t expected_fsm = GPTIMER_FSM_ENABLE; if (atomic_compare_exchange_strong(&timer->fsm, &expected_fsm, GPTIMER_FSM_RUN_WAIT)) { @@ -393,7 +400,8 @@ esp_err_t gptimer_start(gptimer_handle_t timer) atomic_store(&timer->fsm, GPTIMER_FSM_RUN); portEXIT_CRITICAL_SAFE(&timer->spinlock); } else { - ESP_RETURN_ON_FALSE_ISR(false, ESP_ERR_INVALID_STATE, TAG, "timer is not ready for a new start"); + // return error if the timer is not in the expected state + return ESP_ERR_INVALID_STATE; } return ESP_OK; @@ -401,7 +409,15 @@ esp_err_t gptimer_start(gptimer_handle_t timer) esp_err_t gptimer_stop(gptimer_handle_t timer) { - ESP_RETURN_ON_FALSE_ISR(timer, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + if (timer == NULL) { + // not printing error message here because the return value already indicates the error well + return ESP_ERR_INVALID_ARG; + } + + // if the timer is not started, do nothing + if (atomic_load(&timer->fsm) == GPTIMER_FSM_ENABLE) { + return ESP_OK; + } gptimer_fsm_t expected_fsm = GPTIMER_FSM_RUN; if (atomic_compare_exchange_strong(&timer->fsm, &expected_fsm, GPTIMER_FSM_ENABLE_WAIT)) { @@ -412,7 +428,8 @@ esp_err_t gptimer_stop(gptimer_handle_t timer) atomic_store(&timer->fsm, GPTIMER_FSM_ENABLE); portEXIT_CRITICAL_SAFE(&timer->spinlock); } else { - ESP_RETURN_ON_FALSE_ISR(false, ESP_ERR_INVALID_STATE, TAG, "timer is not running"); + // return error if the timer is not in the expected state + return ESP_ERR_INVALID_STATE; } return ESP_OK; diff --git a/components/esp_driver_gptimer/src/gptimer_common.c b/components/esp_driver_gptimer/src/gptimer_common.c index 6c1cd402bc..5435955ee0 100644 --- a/components/esp_driver_gptimer/src/gptimer_common.c +++ b/components/esp_driver_gptimer/src/gptimer_common.c @@ -1,15 +1,14 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -#include "esp_check.h" +#include #include "esp_clk_tree.h" #include "esp_private/gptimer.h" #include "gptimer_priv.h" - -static const char *TAG = "gptimer"; +#include "esp_private/esp_clk_tree_common.h" typedef struct gptimer_platform_t { _lock_t mutex; // platform level mutex lock @@ -171,3 +170,11 @@ 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; } + +#if CONFIG_GPTIMER_ENABLE_DEBUG_LOG +__attribute__((constructor)) +static void gptimer_override_default_log_level(void) +{ + esp_log_level_set(TAG, ESP_LOG_VERBOSE); +} +#endif diff --git a/components/esp_driver_gptimer/src/gptimer_etm.c b/components/esp_driver_gptimer/src/gptimer_etm.c index cada56500a..96b3115242 100644 --- a/components/esp_driver_gptimer/src/gptimer_etm.c +++ b/components/esp_driver_gptimer/src/gptimer_etm.c @@ -1,26 +1,17 @@ /* - * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ #include #include -#include "sdkconfig.h" -#include "freertos/FreeRTOS.h" -#include "esp_err.h" -#include "esp_log.h" -#include "esp_check.h" -#include "esp_heap_caps.h" #include "driver/gptimer.h" #include "gptimer_priv.h" -#include "hal/timer_ll.h" #include "esp_private/etm_interface.h" #define ETM_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT -static const char *TAG = "gptimer-etm"; - static esp_err_t gptimer_del_etm_event(esp_etm_event_t *event) { free(event); @@ -36,17 +27,16 @@ static esp_err_t gptimer_del_etm_task(esp_etm_task_t *task) esp_err_t gptimer_new_etm_event(gptimer_handle_t timer, const gptimer_etm_event_config_t *config, esp_etm_event_handle_t *out_event) { esp_etm_event_t *event = NULL; - esp_err_t ret = ESP_OK; - ESP_GOTO_ON_FALSE(timer && config && out_event, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); - ESP_GOTO_ON_FALSE(config->event_type < GPTIMER_ETM_EVENT_MAX, ESP_ERR_INVALID_ARG, err, TAG, "invalid event type"); + ESP_RETURN_ON_FALSE(timer && config && out_event, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + ESP_RETURN_ON_FALSE(config->event_type < GPTIMER_ETM_EVENT_MAX, ESP_ERR_INVALID_ARG, TAG, "invalid event type"); event = heap_caps_calloc(1, sizeof(esp_etm_event_t), ETM_MEM_ALLOC_CAPS); - ESP_GOTO_ON_FALSE(event, ESP_ERR_NO_MEM, err, TAG, "no memory for ETM event"); + ESP_RETURN_ON_FALSE(event, ESP_ERR_NO_MEM, TAG, "no memory for ETM event"); + // get the event ID that can be recognized by ETM hardware gptimer_group_t *group = timer->group; int group_id = group->group_id; int timer_id = timer->timer_id; uint32_t event_id = TIMER_LL_ETM_EVENT_TABLE(group_id, timer_id, config->event_type); - ESP_GOTO_ON_FALSE(event_id != 0, ESP_ERR_NOT_SUPPORTED, err, TAG, "not supported event type"); // fill the ETM event object event->event_id = event_id; @@ -54,28 +44,21 @@ esp_err_t gptimer_new_etm_event(gptimer_handle_t timer, const gptimer_etm_event_ event->del = gptimer_del_etm_event; *out_event = event; return ESP_OK; - -err: - if (event) { - gptimer_del_etm_event(event); - } - return ret; } esp_err_t gptimer_new_etm_task(gptimer_handle_t timer, const gptimer_etm_task_config_t *config, esp_etm_task_handle_t *out_task) { esp_etm_task_t *task = NULL; - esp_err_t ret = ESP_OK; - ESP_GOTO_ON_FALSE(timer && config && out_task, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); - ESP_GOTO_ON_FALSE(config->task_type < GPTIMER_ETM_TASK_MAX, ESP_ERR_INVALID_ARG, err, TAG, "invalid task type"); + ESP_RETURN_ON_FALSE(timer && config && out_task, ESP_ERR_INVALID_ARG, TAG, "invalid argument"); + ESP_RETURN_ON_FALSE(config->task_type < GPTIMER_ETM_TASK_MAX, ESP_ERR_INVALID_ARG, TAG, "invalid task type"); task = heap_caps_calloc(1, sizeof(esp_etm_task_t), ETM_MEM_ALLOC_CAPS); - ESP_GOTO_ON_FALSE(task, ESP_ERR_NO_MEM, err, TAG, "no memory for ETM task"); + ESP_RETURN_ON_FALSE(task, ESP_ERR_NO_MEM, TAG, "no memory for ETM task"); + // get the task ID that can be recognized by ETM hardware gptimer_group_t *group = timer->group; int group_id = group->group_id; int timer_id = timer->timer_id; uint32_t task_id = TIMER_LL_ETM_TASK_TABLE(group_id, timer_id, config->task_type); - ESP_GOTO_ON_FALSE(task_id != 0, ESP_ERR_NOT_SUPPORTED, err, TAG, "not supported task type"); // fill the ETM task object task->task_id = task_id; @@ -83,10 +66,4 @@ esp_err_t gptimer_new_etm_task(gptimer_handle_t timer, const gptimer_etm_task_co task->del = gptimer_del_etm_task; *out_task = task; return ESP_OK; - -err: - if (task) { - gptimer_del_etm_task(task); - } - return ret; } diff --git a/components/esp_driver_gptimer/src/gptimer_priv.h b/components/esp_driver_gptimer/src/gptimer_priv.h index fb64a88adf..27b1a6c288 100644 --- a/components/esp_driver_gptimer/src/gptimer_priv.h +++ b/components/esp_driver_gptimer/src/gptimer_priv.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -9,17 +9,25 @@ #include #include #include "sdkconfig.h" +#if CONFIG_GPTIMER_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for gptimer driver +#define LOG_LOCAL_LEVEL ESP_LOG_VERBOSE +#endif #include "soc/soc_caps.h" #include "freertos/FreeRTOS.h" #include "esp_err.h" +#include "esp_log.h" +#include "esp_check.h" +#include "esp_attr.h" #include "esp_intr_alloc.h" #include "esp_heap_caps.h" -#include "clk_ctrl_os.h" #include "esp_pm.h" #include "soc/timer_periph.h" #include "hal/timer_types.h" #include "hal/timer_hal.h" #include "hal/timer_ll.h" +#include "clk_ctrl_os.h" #include "esp_private/sleep_retention.h" #include "esp_private/periph_ctrl.h" @@ -29,7 +37,7 @@ extern "C" { // If ISR handler is allowed to run whilst cache is disabled, // Make sure all the code and related variables used by the handler are in the SRAM -#if CONFIG_GPTIMER_ISR_IRAM_SAFE || CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM +#if CONFIG_GPTIMER_OBJ_CACHE_SAFE #define GPTIMER_MEM_ALLOC_CAPS (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) #else #define GPTIMER_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT @@ -53,6 +61,10 @@ extern "C" { #define GPTIMER_CLOCK_SRC_ATOMIC() #endif +///!< Logging settings +#define TAG "gptimer" + +///!< Forward declaration typedef struct gptimer_t gptimer_t; typedef struct gptimer_group_t { diff --git a/components/esp_driver_gptimer/test_apps/gptimer/main/test_gptimer.c b/components/esp_driver_gptimer/test_apps/gptimer/main/test_gptimer.c index 757c2f715c..944d89e39f 100644 --- a/components/esp_driver_gptimer/test_apps/gptimer/main/test_gptimer.c +++ b/components/esp_driver_gptimer/test_apps/gptimer/main/test_gptimer.c @@ -299,7 +299,7 @@ TEST_ALARM_CALLBACK_ATTR static bool test_gptimer_alarm_normal_callback(gptimer_ * Also should account for the inaccuracy of the systick during DFS. */ #if CONFIG_PM_ENABLE -#define GPTIMER_ONE_SHOT_ALARM_COUNT_DELTA 15000 +#define GPTIMER_ONE_SHOT_ALARM_COUNT_DELTA 50000 #else #define GPTIMER_ONE_SHOT_ALARM_COUNT_DELTA 1000 #endif // CONFIG_PM_ENABLE diff --git a/components/esp_driver_gptimer/test_apps/gptimer/main/test_gptimer_iram.c b/components/esp_driver_gptimer/test_apps/gptimer/main/test_gptimer_iram.c index 2ff59fc5fa..3331171fc3 100644 --- a/components/esp_driver_gptimer/test_apps/gptimer/main/test_gptimer_iram.c +++ b/components/esp_driver_gptimer/test_apps/gptimer/main/test_gptimer_iram.c @@ -23,7 +23,7 @@ static void IRAM_ATTR test_delay_post_cache_disable(void *args) esp_rom_delay_us(1000); } -TEST_CASE("gptimer_interrupt_iram_safe", "[gptimer]") +TEST_CASE("gptimer works with cache disabled", "[gptimer]") { gptimer_handle_t gptimer = NULL; gptimer_config_t timer_config = {