From ca51072461f785692c9a14b58e2c5a48f0a8e3d7 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 | 28 ++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/components/esp32/esp_timer_esp32.c b/components/esp32/esp_timer_esp32.c index 892588973a..8e048f2d0b 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 2001d7b5e2..df798e2b64 100644 --- a/components/esp32/test/test_esp_timer.c +++ b/components/esp32/test/test_esp_timer.c @@ -779,3 +779,31 @@ TEST_CASE("esp_timer_impl_set_alarm and using start_once do not lead that the Sy } #endif // !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_ESP32_DPORT_WORKAROUND) + +TEST_CASE("Test case when esp_timer_impl_set_alarm needs set timer < now_time", "[esp_timer]") +{ + REG_WRITE(FRC_TIMER_LOAD_REG(1), 0); + esp_timer_impl_advance(50331648); // 0xefffffff/80 = 50331647 + + ets_delay_us(2); + + portDISABLE_INTERRUPTS(); + esp_timer_impl_set_alarm(50331647); + uint32_t alarm_reg = REG_READ(FRC_TIMER_ALARM_REG(1)); + uint32_t count_reg = REG_READ(FRC_TIMER_COUNT_REG(1)); + portENABLE_INTERRUPTS(); + + const uint32_t offset = 80 * 2; // s_timer_ticks_per_us + 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 + } +}