From 2718fdbd9578b8cf5f4cc9960dd338d3327f1ca6 Mon Sep 17 00:00:00 2001 From: KonstantinKondrashov Date: Mon, 13 Jan 2020 14:36:38 +0800 Subject: [PATCH] esp_timer/esp32: Fix case when alarm_reg > counter_reg but FRC_TIMER_INT_STATUS is not set Closes: WIFI-1576 Closes: https://github.com/espressif/esp-idf/issues/2954 --- components/esp32/esp_timer_esp32.c | 33 +++++++++++++++++++++++--- components/esp32/test/test_esp_timer.c | 10 ++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/components/esp32/esp_timer_esp32.c b/components/esp32/esp_timer_esp32.c index 892588973..8e048f2d0 100644 --- a/components/esp32/esp_timer_esp32.c +++ b/components/esp32/esp_timer_esp32.c @@ -216,10 +216,14 @@ void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) // 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 int32_t offset = s_timer_ticks_per_us * 2; + int32_t offset = s_timer_ticks_per_us * 2; do { // Adjust current time if overflow has happened - if (timer_overflow_happened()) { + if (timer_overflow_happened() || + ((REG_READ(FRC_TIMER_COUNT_REG(1)) > ALARM_OVERFLOW_VAL) && + ((REG_READ(FRC_TIMER_CTRL_REG(1)) & FRC_TIMER_INT_STATUS) == 0))) { + // 1. timer_overflow_happened() checks overflow with the interrupt flag. + // 2. During several loops, the counter can be higher than the alarm and even step over ALARM_OVERFLOW_VAL boundary (the interrupt flag is not set). timer_count_reload(); s_time_base_us += s_timer_us_per_overflow; } @@ -236,7 +240,19 @@ void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) 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))); + int64_t delta = (int64_t)alarm_reg_val - (int64_t)REG_READ(FRC_TIMER_COUNT_REG(1)); + if (delta <= 0) { + /* + When the timestamp is a bit less than the current counter then the alarm = current_counter + offset. + But due to CPU_freq in some case can be equal APB_freq the offset time can not exceed the overhead + (the alarm will be less than the counter) and it leads to the infinity loop. + To exclude this behavior to the offset was added the delta to have the opportunity to go through it. + */ + offset += abs((int)delta) + s_timer_ticks_per_us * 2; + } else { + break; + } + } while (1); portEXIT_CRITICAL(&s_time_update_lock); } @@ -254,6 +270,17 @@ static void IRAM_ATTR timer_alarm_isr(void *arg) // Set alarm to the next overflow moment. Later, upper layer function may // call esp_timer_impl_set_alarm to change this to an earlier value. REG_WRITE(FRC_TIMER_ALARM_REG(1), ALARM_OVERFLOW_VAL); + if ((REG_READ(FRC_TIMER_COUNT_REG(1)) > ALARM_OVERFLOW_VAL) && + ((REG_READ(FRC_TIMER_CTRL_REG(1)) & FRC_TIMER_INT_STATUS) == 0)) { + /* + This check excludes the case when the alarm can be less than the counter. + Without this check, it is possible because DPORT uses 4-lvl, and users can use the 5 Hi-interrupt, + they can interrupt this function between FRC_TIMER_INT_CLR and setting the alarm = ALARM_OVERFLOW_VAL + that lead to the counter will go ahead leaving the alarm behind. + */ + timer_count_reload(); + s_time_base_us += s_timer_us_per_overflow; + } portEXIT_CRITICAL_ISR(&s_time_update_lock); // Call the upper layer handler (*s_alarm_handler)(arg); diff --git a/components/esp32/test/test_esp_timer.c b/components/esp32/test/test_esp_timer.c index ae9eb9025..6d386a442 100644 --- a/components/esp32/test/test_esp_timer.c +++ b/components/esp32/test/test_esp_timer.c @@ -798,3 +798,13 @@ TEST_CASE("Test case when esp_timer_impl_set_alarm needs set timer < now_time", printf("alarm_reg = 0x%x, count_reg 0x%x\n", alarm_reg, count_reg); TEST_ASSERT(alarm_reg <= (count_reg + offset)); } + +TEST_CASE("Test esp_timer_impl_set_alarm when the counter is near an overflow value", "[esp_timer]") +{ + for (int i = 0; i < 1024; ++i) { + uint32_t count_reg = 0xeffffe00 + i; + REG_WRITE(FRC_TIMER_LOAD_REG(1), count_reg); + printf("%d) count_reg = 0x%x\n", i, count_reg); + esp_timer_impl_set_alarm(1); // timestamp is expired + } +}