From ee84cbdf4e97b5f0271611c637d3c67dec395b63 Mon Sep 17 00:00:00 2001 From: KonstantinKondrashov Date: Fri, 6 Sep 2019 20:17:25 +0800 Subject: [PATCH] esp_timer: Fix System time jumps back ~54secs Closes: https://github.com/espressif/esp-idf/issues/2513 --- components/esp32/esp_timer.c | 43 +++++++++++++++-------- components/esp32/esp_timer_esp32.c | 56 +++++++++++------------------- 2 files changed, 49 insertions(+), 50 deletions(-) diff --git a/components/esp32/esp_timer.c b/components/esp32/esp_timer.c index 86ab9f03b..ef7fbe796 100644 --- a/components/esp32/esp_timer.c +++ b/components/esp32/esp_timer.c @@ -59,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); @@ -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,16 +178,17 @@ esp_err_t esp_timer_delete(esp_timer_handle_t timer) if (timer_armed(timer)) { return ESP_ERR_INVALID_STATE; } + timer_list_lock(); timer->event_id = EVENT_ID_DELETE_TIMER; - timer->alarm = esp_timer_get_time() + 50; + 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 @@ -195,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; } @@ -246,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); } @@ -322,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; } @@ -473,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(); @@ -485,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); }