feat(gptimer): make start and stop function idempotent

Closes https://github.com/espressif/esp-idf/issues/12325
Closes https://github.com/espressif/esp-idf/issues/13486
This commit is contained in:
morris 2025-02-14 16:36:49 +08:00
parent af429c89a2
commit 509b9d9a54
7 changed files with 110 additions and 83 deletions

View File

@ -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

View File

@ -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 <stdlib.h>
#include <sys/lock.h>
#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;

View File

@ -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 <sys/lock.h>
#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

View File

@ -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 <stdlib.h>
#include <sys/lock.h>
#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;
}

View File

@ -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 <stdint.h>
#include <stdatomic.h>
#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 {

View File

@ -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

View File

@ -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 = {