diff --git a/components/esp32/esp_timer.c b/components/esp32/esp_timer.c index 78f004acf..ef7fbe796 100644 --- a/components/esp32/esp_timer.c +++ b/components/esp32/esp_timer.c @@ -38,12 +38,17 @@ #endif #include "rom/queue.h" +#define EVENT_ID_DELETE_TIMER 0xF0DE1E1E + #define TIMER_EVENT_QUEUE_SIZE 16 struct esp_timer { uint64_t alarm; uint64_t period; - esp_timer_cb_t callback; + union { + esp_timer_cb_t callback; + uint32_t event_id; + }; void* arg; #if WITH_PROFILING const char* name; @@ -54,12 +59,12 @@ struct esp_timer { LIST_ENTRY(esp_timer) list_entry; }; -static bool is_initialized(); +static bool is_initialized(void); static esp_err_t timer_insert(esp_timer_handle_t timer); static esp_err_t timer_remove(esp_timer_handle_t timer); static bool timer_armed(esp_timer_handle_t timer); -static void timer_list_lock(); -static void timer_list_unlock(); +static void timer_list_lock(void); +static void timer_list_unlock(void); #if WITH_PROFILING static void timer_insert_inactive(esp_timer_handle_t timer); @@ -76,23 +81,18 @@ static LIST_HEAD(esp_timer_list, esp_timer) s_timers = // all the timers static LIST_HEAD(esp_inactive_timer_list, esp_timer) s_inactive_timers = LIST_HEAD_INITIALIZER(s_timers); -// used to keep track of the timer when executing the callback -static esp_timer_handle_t s_timer_in_callback; #endif // task used to dispatch timer callbacks static TaskHandle_t s_timer_task; // counting semaphore used to notify the timer task from ISR static SemaphoreHandle_t s_timer_semaphore; -// mutex which protects timers from deletion during callback execution -static SemaphoreHandle_t s_timer_delete_mutex; #if CONFIG_SPIRAM_USE_MALLOC -// memory for s_timer_semaphore and s_timer_delete_mutex +// memory for s_timer_semaphore static StaticQueue_t s_timer_semaphore_memory; -static StaticQueue_t s_timer_delete_mutex_memory; #endif -// lock protecting s_timers, s_inactive_timers, s_timer_in_callback +// lock protecting s_timers, s_inactive_timers static portMUX_TYPE s_timer_lock = portMUX_INITIALIZER_UNLOCKED; @@ -103,7 +103,7 @@ esp_err_t esp_timer_create(const esp_timer_create_args_t* args, if (!is_initialized()) { return ESP_ERR_INVALID_STATE; } - if (args->callback == NULL) { + if (args == NULL || args->callback == NULL || out_handle == NULL) { return ESP_ERR_INVALID_ARG; } esp_timer_handle_t result = (esp_timer_handle_t) calloc(1, sizeof(*result)); @@ -122,33 +122,48 @@ esp_err_t esp_timer_create(const esp_timer_create_args_t* args, esp_err_t IRAM_ATTR esp_timer_start_once(esp_timer_handle_t timer, uint64_t timeout_us) { + if (timer == NULL) { + return ESP_ERR_INVALID_ARG; + } if (!is_initialized() || timer_armed(timer)) { return ESP_ERR_INVALID_STATE; } + timer_list_lock(); timer->alarm = esp_timer_get_time() + timeout_us; timer->period = 0; #if WITH_PROFILING timer->times_armed++; #endif - return timer_insert(timer); + esp_err_t err = timer_insert(timer); + timer_list_unlock(); + return err; } esp_err_t IRAM_ATTR esp_timer_start_periodic(esp_timer_handle_t timer, uint64_t period_us) { + if (timer == NULL) { + return ESP_ERR_INVALID_ARG; + } if (!is_initialized() || timer_armed(timer)) { return ESP_ERR_INVALID_STATE; } + timer_list_lock(); period_us = MAX(period_us, esp_timer_impl_get_min_period_us()); timer->alarm = esp_timer_get_time() + period_us; timer->period = period_us; #if WITH_PROFILING timer->times_armed++; #endif - return timer_insert(timer); + esp_err_t err = timer_insert(timer); + timer_list_unlock(); + return err; } esp_err_t IRAM_ATTR esp_timer_stop(esp_timer_handle_t timer) { + if (timer == NULL) { + return ESP_ERR_INVALID_ARG; + } if (!is_initialized() || !timer_armed(timer)) { return ESP_ERR_INVALID_STATE; } @@ -163,21 +178,17 @@ esp_err_t esp_timer_delete(esp_timer_handle_t timer) if (timer_armed(timer)) { return ESP_ERR_INVALID_STATE; } - xSemaphoreTakeRecursive(s_timer_delete_mutex, portMAX_DELAY); -#if WITH_PROFILING - if (timer == s_timer_in_callback) { - s_timer_in_callback = NULL; - } - timer_remove_inactive(timer); -#endif - free(timer); - xSemaphoreGiveRecursive(s_timer_delete_mutex); + timer_list_lock(); + timer->event_id = EVENT_ID_DELETE_TIMER; + timer->alarm = esp_timer_get_time(); + timer->period = 0; + timer_insert(timer); + timer_list_unlock(); return ESP_OK; } static IRAM_ATTR esp_err_t timer_insert(esp_timer_handle_t timer) { - timer_list_lock(); #if WITH_PROFILING timer_remove_inactive(timer); #endif @@ -200,7 +211,6 @@ static IRAM_ATTR esp_err_t timer_insert(esp_timer_handle_t timer) if (timer == LIST_FIRST(&s_timers)) { esp_timer_impl_set_alarm(timer->alarm); } - timer_list_unlock(); return ESP_OK; } @@ -251,12 +261,12 @@ static IRAM_ATTR bool timer_armed(esp_timer_handle_t timer) return timer->alarm > 0; } -static IRAM_ATTR void timer_list_lock() +static IRAM_ATTR void timer_list_lock(void) { portENTER_CRITICAL(&s_timer_lock); } -static IRAM_ATTR void timer_list_unlock() +static IRAM_ATTR void timer_list_unlock(void) { portEXIT_CRITICAL(&s_timer_lock); } @@ -266,13 +276,17 @@ static void timer_process_alarm(esp_timer_dispatch_t dispatch_method) /* unused, provision to allow running callbacks from ISR */ (void) dispatch_method; - xSemaphoreTakeRecursive(s_timer_delete_mutex, portMAX_DELAY); timer_list_lock(); uint64_t now = esp_timer_impl_get_time(); esp_timer_handle_t it = LIST_FIRST(&s_timers); while (it != NULL && it->alarm < now) { LIST_REMOVE(it, list_entry); + if (it->event_id == EVENT_ID_DELETE_TIMER) { + free(it); + it = LIST_FIRST(&s_timers); + continue; + } if (it->period > 0) { it->alarm += it->period; timer_insert(it); @@ -284,21 +298,14 @@ static void timer_process_alarm(esp_timer_dispatch_t dispatch_method) } #if WITH_PROFILING uint64_t callback_start = now; - s_timer_in_callback = it; #endif timer_list_unlock(); (*it->callback)(it->arg); timer_list_lock(); now = esp_timer_impl_get_time(); #if WITH_PROFILING - /* The callback might have deleted the timer. - * If this happens, esp_timer_delete will set s_timer_in_callback - * to NULL. - */ - if (s_timer_in_callback) { - s_timer_in_callback->times_triggered++; - s_timer_in_callback->total_callback_run_time += now - callback_start; - } + it->times_triggered++; + it->total_callback_run_time += now - callback_start; #endif it = LIST_FIRST(&s_timers); } @@ -307,7 +314,6 @@ static void timer_process_alarm(esp_timer_dispatch_t dispatch_method) esp_timer_impl_set_alarm(first->alarm); } timer_list_unlock(); - xSemaphoreGiveRecursive(s_timer_delete_mutex); } static void timer_task(void* arg) @@ -331,7 +337,7 @@ static void IRAM_ATTR timer_alarm_handler(void* arg) } } -static IRAM_ATTR bool is_initialized() +static IRAM_ATTR bool is_initialized(void) { return s_timer_task != NULL; } @@ -355,18 +361,6 @@ esp_err_t esp_timer_init(void) goto out; } -#if CONFIG_SPIRAM_USE_MALLOC - memset(&s_timer_delete_mutex_memory, 0, sizeof(StaticQueue_t)); - s_timer_delete_mutex = xSemaphoreCreateRecursiveMutexStatic(&s_timer_delete_mutex_memory); -#else - s_timer_delete_mutex = xSemaphoreCreateRecursiveMutex(); -#endif - if (!s_timer_delete_mutex) { - err = ESP_ERR_NO_MEM; - goto out; - } - - int ret = xTaskCreatePinnedToCore(&timer_task, "esp_timer", ESP_TASK_TIMER_STACK, NULL, ESP_TASK_TIMER_PRIO, &s_timer_task, PRO_CPU_NUM); if (ret != pdPASS) { @@ -390,10 +384,6 @@ out: vSemaphoreDelete(s_timer_semaphore); s_timer_semaphore = NULL; } - if (s_timer_delete_mutex) { - vSemaphoreDelete(s_timer_delete_mutex); - s_timer_delete_mutex = NULL; - } return ESP_ERR_NO_MEM; } @@ -498,7 +488,7 @@ esp_err_t esp_timer_dump(FILE* stream) return ESP_OK; } -int64_t IRAM_ATTR esp_timer_get_next_alarm() +int64_t IRAM_ATTR esp_timer_get_next_alarm(void) { int64_t next_alarm = INT64_MAX; timer_list_lock(); @@ -510,7 +500,7 @@ int64_t IRAM_ATTR esp_timer_get_next_alarm() return next_alarm; } -int64_t IRAM_ATTR esp_timer_get_time() +int64_t IRAM_ATTR esp_timer_get_time(void) { return (int64_t) esp_timer_impl_get_time(); } diff --git a/components/esp32/esp_timer_esp32.c b/components/esp32/esp_timer_esp32.c index 75e3a58fa..7f26bb47e 100644 --- a/components/esp32/esp_timer_esp32.c +++ b/components/esp32/esp_timer_esp32.c @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include "sys/param.h" #include "esp_err.h" #include "esp_timer.h" #include "esp_system.h" @@ -127,13 +128,6 @@ static uint32_t s_timer_us_per_overflow; // will not increment s_time_base_us if this flag is set. static bool s_mask_overflow; -//The timer_overflow_happened read alarm register to tell if overflow happened. -//However, there is a monent that overflow happens, and before ISR function called -//alarm register is set to another value, then you call timer_overflow_happened, -//it will return false. -//So we store the overflow value when new alarm is to be set. -static bool s_overflow_happened; - #ifdef CONFIG_PM_DFS_USE_RTC_TIMER_REF // If DFS is enabled, upon the first frequency change this value is set to the // difference between esp_timer value and RTC timer value. On every subsequent @@ -152,10 +146,6 @@ portMUX_TYPE s_time_update_lock = portMUX_INITIALIZER_UNLOCKED; // Check if timer overflow has happened (but was not handled by ISR yet) static inline bool IRAM_ATTR timer_overflow_happened() { - if (s_overflow_happened) { - return true; - } - return ((REG_READ(FRC_TIMER_CTRL_REG(1)) & FRC_TIMER_INT_STATUS) != 0 && ((REG_READ(FRC_TIMER_ALARM_REG(1)) == ALARM_OVERFLOW_VAL && TIMER_IS_AFTER_OVERFLOW(REG_READ(FRC_TIMER_COUNT_REG(1))) && !s_mask_overflow) || (!TIMER_IS_AFTER_OVERFLOW(REG_READ(FRC_TIMER_ALARM_REG(1))) && TIMER_IS_AFTER_OVERFLOW(REG_READ(FRC_TIMER_COUNT_REG(1)))))); @@ -222,35 +212,31 @@ uint64_t IRAM_ATTR esp_timer_impl_get_time() void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) { portENTER_CRITICAL(&s_time_update_lock); - // Alarm time relative to the moment when counter was 0 - uint64_t time_after_timebase_us = timestamp - s_time_base_us; - // Adjust current time if overflow has happened - bool overflow = timer_overflow_happened(); - uint64_t cur_count = REG_READ(FRC_TIMER_COUNT_REG(1)); - - if (overflow) { - assert(time_after_timebase_us > s_timer_us_per_overflow); - time_after_timebase_us -= s_timer_us_per_overflow; - s_overflow_happened = true; - } - // Calculate desired timer compare value (may exceed 2^32-1) - uint64_t compare_val = time_after_timebase_us * s_timer_ticks_per_us; - uint32_t alarm_reg_val = ALARM_OVERFLOW_VAL; // Use calculated alarm value if it is less than ALARM_OVERFLOW_VAL. // Note that if by the time we update ALARM_REG, COUNT_REG value is higher, // interrupt will not happen for another ALARM_OVERFLOW_VAL timer ticks, // so need to check if alarm value is too close in the future (e.g. <2 us away). const uint32_t offset = s_timer_ticks_per_us * 2; - if (compare_val < ALARM_OVERFLOW_VAL) { - if (compare_val < cur_count + offset) { - compare_val = cur_count + offset; - if (compare_val > ALARM_OVERFLOW_VAL) { - compare_val = ALARM_OVERFLOW_VAL; - } + do { + // Adjust current time if overflow has happened + if (timer_overflow_happened()) { + timer_count_reload(); + s_time_base_us += s_timer_us_per_overflow; } - alarm_reg_val = (uint32_t) compare_val; - } - REG_WRITE(FRC_TIMER_ALARM_REG(1), alarm_reg_val); + s_mask_overflow = false; + uint64_t cur_count = REG_READ(FRC_TIMER_COUNT_REG(1)); + // Alarm time relative to the moment when counter was 0 + int64_t time_after_timebase_us = (int64_t)timestamp - s_time_base_us; + // Calculate desired timer compare value (may exceed 2^32-1) + int64_t compare_val = time_after_timebase_us * s_timer_ticks_per_us; + + compare_val = MAX(compare_val, cur_count + offset); + uint32_t alarm_reg_val = ALARM_OVERFLOW_VAL; + if (compare_val < ALARM_OVERFLOW_VAL) { + alarm_reg_val = (uint32_t) compare_val; + } + REG_WRITE(FRC_TIMER_ALARM_REG(1), alarm_reg_val); + } while (REG_READ(FRC_TIMER_ALARM_REG(1)) <= REG_READ(FRC_TIMER_COUNT_REG(1))); portEXIT_CRITICAL(&s_time_update_lock); } @@ -261,7 +247,6 @@ static void IRAM_ATTR timer_alarm_isr(void *arg) if (timer_overflow_happened()) { timer_count_reload(); s_time_base_us += s_timer_us_per_overflow; - s_overflow_happened = false; } s_mask_overflow = false; // Clear interrupt status @@ -336,7 +321,6 @@ void esp_timer_impl_advance(int64_t time_us) REG_WRITE(FRC_TIMER_ALARM_REG(1), 0); REG_WRITE(FRC_TIMER_LOAD_REG(1), 0); s_time_base_us += count / s_timer_ticks_per_us + time_us; - s_overflow_happened = false; portEXIT_CRITICAL(&s_time_update_lock); } diff --git a/components/esp32/test/test_esp_timer.c b/components/esp32/test/test_esp_timer.c index 510bed461..2001d7b5e 100644 --- a/components/esp32/test/test_esp_timer.c +++ b/components/esp32/test/test_esp_timer.c @@ -536,6 +536,61 @@ TEST_CASE("Can delete timer from callback", "[esp_timer]") vSemaphoreDelete(args.notify_from_timer_cb); } + +typedef struct { + SemaphoreHandle_t delete_start; + SemaphoreHandle_t delete_done; + SemaphoreHandle_t test_done; + esp_timer_handle_t timer; +} timer_delete_test_args_t; + +static void timer_delete_task(void* arg) +{ + timer_delete_test_args_t* args = (timer_delete_test_args_t*) arg; + xSemaphoreTake(args->delete_start, portMAX_DELAY); + printf("Deleting the timer\n"); + esp_timer_delete(args->timer); + printf("Timer deleted\n"); + xSemaphoreGive(args->delete_done); + vTaskDelete(NULL); +} + +static void timer_delete_test_callback(void* arg) +{ + timer_delete_test_args_t* args = (timer_delete_test_args_t*) arg; + printf("Timer callback called\n"); + xSemaphoreGive(args->delete_start); + xSemaphoreTake(args->delete_done, portMAX_DELAY); + printf("Callback complete\n"); + xSemaphoreGive(args->test_done); +} + +TEST_CASE("Can delete timer from a separate task, triggered from callback", "[esp_timer]") +{ + timer_delete_test_args_t args = { + .delete_start = xSemaphoreCreateBinary(), + .delete_done = xSemaphoreCreateBinary(), + .test_done = xSemaphoreCreateBinary(), + }; + + esp_timer_create_args_t timer_args = { + .callback = &timer_delete_test_callback, + .arg = &args + }; + esp_timer_handle_t timer; + TEST_ESP_OK(esp_timer_create(&timer_args, &timer)); + args.timer = timer; + + xTaskCreate(timer_delete_task, "deleter", 4096, &args, 5, NULL); + + esp_timer_start_once(timer, 100); + TEST_ASSERT(xSemaphoreTake(args.test_done, pdMS_TO_TICKS(1000))); + + vSemaphoreDelete(args.delete_done); + vSemaphoreDelete(args.delete_start); + vSemaphoreDelete(args.test_done); +} + TEST_CASE("esp_timer_impl_advance moves time base correctly", "[esp_timer]") { ref_clock_init(); @@ -594,3 +649,133 @@ TEST_CASE("after esp_timer_impl_advance, timers run when expected", "[esp_timer] ref_clock_deinit(); } + +#if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_ESP32_DPORT_WORKAROUND) + +#include "soc/dport_reg.h" +#include "soc/frc_timer_reg.h" +#include "esp_ipc.h" +static bool task_stop; +static bool time_jumped; + +static void task_check_time(void *p) +{ + int64_t t1 = 0, t2 = 0; + while (task_stop == false) { + + t1 = t2; + t2 = esp_timer_get_time(); + if (t1 > t2) { + int64_t shift_us = t2 - t1; + time_jumped = true; + printf("System clock jumps back: %lli us\n", shift_us); + } + + vTaskDelay(1); + } + vTaskDelete(NULL); +} + +static void timer_callback(void* arg) +{ + +} + +static void dport_task(void *pvParameters) +{ + while (task_stop == false) { + DPORT_STALL_OTHER_CPU_START(); + ets_delay_us(3); + DPORT_STALL_OTHER_CPU_END(); + } + vTaskDelete(NULL); +} + +TEST_CASE("esp_timer_impl_set_alarm does not set an alarm below the current time", "[esp_timer][timeout=62]") +{ + const int max_timers = 2; + time_jumped = false; + task_stop = false; + + xTaskCreatePinnedToCore(task_check_time, "task_check_time", 4096, NULL, 5, NULL, 0); + // dport_task is used here to interrupt the esp_timer_impl_set_alarm function. + // To interrupt it we can use an interrupt with 4 or 5 levels which will run on CPU0. + // Instead, an interrupt we use the dport workaround which has 4 interrupt level for stall CPU0. + xTaskCreatePinnedToCore(dport_task, "dport_task", 4096, NULL, 5, NULL, 1); + + const esp_timer_create_args_t periodic_timer_args = { + .callback = &timer_callback, + }; + + esp_timer_handle_t periodic_timer[max_timers]; + printf("timers created\n"); + + esp_timer_create(&periodic_timer_args, &periodic_timer[0]); + esp_timer_start_periodic(periodic_timer[0], 9000); + + esp_timer_create(&periodic_timer_args, &periodic_timer[1]); + esp_timer_start_periodic(periodic_timer[1], 9000); + + vTaskDelay(60 * 1000 / portTICK_PERIOD_MS); + task_stop = true; + + esp_timer_stop(periodic_timer[0]); + esp_timer_delete(periodic_timer[0]); + esp_timer_stop(periodic_timer[1]); + esp_timer_delete(periodic_timer[1]); + printf("timers deleted\n"); + + vTaskDelay(1000 / portTICK_PERIOD_MS); + + TEST_ASSERT(time_jumped == false); +} + + +static esp_timer_handle_t oneshot_timer; + +static void oneshot_timer_callback(void* arg) +{ + esp_timer_start_once(oneshot_timer, 5000); +} + +static const esp_timer_create_args_t oneshot_timer_args = { + .callback = &oneshot_timer_callback, +}; + +TEST_CASE("esp_timer_impl_set_alarm and using start_once do not lead that the System time jumps back", "[esp_timer][timeout=62]") +{ + time_jumped = false; + task_stop = false; + + xTaskCreatePinnedToCore(task_check_time, "task_check_time", 4096, NULL, 5, NULL, 0); + // dport_task is used here to interrupt the esp_timer_impl_set_alarm function. + // To interrupt it we can use an interrupt with 4 or 5 levels which will run on CPU0. + // Instead, an interrupt we use the dport workaround which has 4 interrupt level for stall CPU0. + xTaskCreatePinnedToCore(dport_task, "dport_task", 4096, NULL, 5, NULL, 1); + + const esp_timer_create_args_t periodic_timer_args = { + .callback = &timer_callback, + }; + + esp_timer_handle_t periodic_timer; + + esp_timer_create(&periodic_timer_args, &periodic_timer); + esp_timer_start_periodic(periodic_timer, 5000); + + esp_timer_create(&oneshot_timer_args, &oneshot_timer); + esp_timer_start_once(oneshot_timer, 9990); + printf("timers created\n"); + + vTaskDelay(60 * 1000 / portTICK_PERIOD_MS); + task_stop = true; + + esp_timer_stop(oneshot_timer); + esp_timer_delete(oneshot_timer); + printf("timers deleted\n"); + + vTaskDelay(1000 / portTICK_PERIOD_MS); + + TEST_ASSERT(time_jumped == false); +} + +#endif // !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_ESP32_DPORT_WORKAROUND)