From 67131b7d78b82dbda27416b07955dcf16bea5894 Mon Sep 17 00:00:00 2001 From: morris Date: Fri, 10 Apr 2020 16:23:19 +0800 Subject: [PATCH] timer_group: fix intr_enable timer group interrupt enable is controled by level_int_ena instead of int_ena Closes https://github.com/espressif/esp-idf/issues/5103 --- components/driver/timer.c | 26 ++++++++++++++++--- .../freertos/test/test_suspend_scheduler.c | 1 + .../main/timer_group_example_main.c | 26 +++++++++---------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/components/driver/timer.c b/components/driver/timer.c index bca443faa6..2020278294 100644 --- a/components/driver/timer.c +++ b/components/driver/timer.c @@ -259,7 +259,12 @@ esp_err_t timer_group_intr_enable(timer_group_t group_num, uint32_t en_mask) { TIMER_CHECK(group_num < TIMER_GROUP_MAX, TIMER_GROUP_NUM_ERROR, ESP_ERR_INVALID_ARG); portENTER_CRITICAL(&timer_spinlock[group_num]); - TG[group_num]->int_ena.val |= en_mask; + for (int i = 0; i < 2; i++) { + if (en_mask & (1 << i)) { + TG[group_num]->hw_timer[i].config.level_int_en = 1; + TG[group_num]->int_ena.val |= (1 << i); + } + } portEXIT_CRITICAL(&timer_spinlock[group_num]); return ESP_OK; } @@ -268,7 +273,12 @@ esp_err_t timer_group_intr_disable(timer_group_t group_num, uint32_t disable_mas { TIMER_CHECK(group_num < TIMER_GROUP_MAX, TIMER_GROUP_NUM_ERROR, ESP_ERR_INVALID_ARG); portENTER_CRITICAL(&timer_spinlock[group_num]); - TG[group_num]->int_ena.val &= (~disable_mask); + for (int i = 0; i < 2; i++) { + if (disable_mask & (1 << i)) { + TG[group_num]->hw_timer[i].config.level_int_en = 0; + TG[group_num]->int_ena.val &= ~(1 << i); + } + } portEXIT_CRITICAL(&timer_spinlock[group_num]); return ESP_OK; } @@ -277,14 +287,22 @@ esp_err_t timer_enable_intr(timer_group_t group_num, timer_idx_t timer_num) { TIMER_CHECK(group_num < TIMER_GROUP_MAX, TIMER_GROUP_NUM_ERROR, ESP_ERR_INVALID_ARG); TIMER_CHECK(timer_num < TIMER_MAX, TIMER_NUM_ERROR, ESP_ERR_INVALID_ARG); - return timer_group_intr_enable(group_num, BIT(timer_num)); + portENTER_CRITICAL(&timer_spinlock[group_num]); + TG[group_num]->hw_timer[timer_num].config.level_int_en = 1; + TG[group_num]->int_ena.val |= (1 << timer_num); + portEXIT_CRITICAL(&timer_spinlock[group_num]); + return ESP_OK; } esp_err_t timer_disable_intr(timer_group_t group_num, timer_idx_t timer_num) { TIMER_CHECK(group_num < TIMER_GROUP_MAX, TIMER_GROUP_NUM_ERROR, ESP_ERR_INVALID_ARG); TIMER_CHECK(timer_num < TIMER_MAX, TIMER_NUM_ERROR, ESP_ERR_INVALID_ARG); - return timer_group_intr_disable(group_num, BIT(timer_num)); + portENTER_CRITICAL(&timer_spinlock[group_num]); + TG[group_num]->hw_timer[timer_num].config.level_int_en = 0; + TG[group_num]->int_ena.val &= ~(1 << timer_num); + portEXIT_CRITICAL(&timer_spinlock[group_num]); + return ESP_OK; } diff --git a/components/freertos/test/test_suspend_scheduler.c b/components/freertos/test/test_suspend_scheduler.c index 9aef2ce7ec..37cbb2eeb4 100644 --- a/components/freertos/test/test_suspend_scheduler.c +++ b/components/freertos/test/test_suspend_scheduler.c @@ -108,6 +108,7 @@ TEST_CASE("Scheduler disabled can handle a pending context switch on resume", "[ // When we resume scheduler, we expect the counter task // will preempt and count at least one more item esp_intr_noniram_enable(); + timer_enable_intr(TIMER_GROUP_0, TIMER_0); xTaskResumeAll(); TEST_ASSERT_NOT_EQUAL(count_config.counter, no_sched_task); diff --git a/examples/peripherals/timer_group/main/timer_group_example_main.c b/examples/peripherals/timer_group/main/timer_group_example_main.c index 0e1ee36fc6..d4ad14f91d 100644 --- a/examples/peripherals/timer_group/main/timer_group_example_main.c +++ b/examples/peripherals/timer_group/main/timer_group_example_main.c @@ -43,7 +43,7 @@ xQueueHandle timer_queue; static void inline print_timer_counter(uint64_t counter_value) { printf("Counter: 0x%08x%08x\n", (uint32_t) (counter_value >> 32), - (uint32_t) (counter_value)); + (uint32_t) (counter_value)); printf("Time : %.8f s\n", (double) counter_value / TIMER_SCALE); } @@ -63,7 +63,7 @@ void IRAM_ATTR timer_group0_isr(void *para) from the timer that reported the interrupt */ uint32_t intr_status = TIMERG0.int_st_timers.val; TIMERG0.hw_timer[timer_idx].update = 1; - uint64_t timer_counter_value = + uint64_t timer_counter_value = ((uint64_t) TIMERG0.hw_timer[timer_idx].cnt_high) << 32 | TIMERG0.hw_timer[timer_idx].cnt_low; @@ -104,17 +104,17 @@ void IRAM_ATTR timer_group0_isr(void *para) * auto_reload - should the timer auto reload on alarm? * timer_interval_sec - the interval of alarm to set */ -static void example_tg0_timer_init(int timer_idx, - bool auto_reload, double timer_interval_sec) +static void example_tg0_timer_init(int timer_idx, + bool auto_reload, double timer_interval_sec) { /* Select and initialize basic parameters of the timer */ - timer_config_t config; - config.divider = TIMER_DIVIDER; - config.counter_dir = TIMER_COUNT_UP; - config.counter_en = TIMER_PAUSE; - config.alarm_en = TIMER_ALARM_EN; - config.intr_type = TIMER_INTR_LEVEL; - config.auto_reload = auto_reload; + timer_config_t config = { + .divider = TIMER_DIVIDER, + .counter_dir = TIMER_COUNT_UP, + .counter_en = TIMER_PAUSE, + .alarm_en = TIMER_ALARM_EN, + .auto_reload = auto_reload, + }; // default clock source is APB timer_init(TIMER_GROUP_0, timer_idx, &config); /* Timer's counter will initially start from value below. @@ -124,8 +124,8 @@ static void example_tg0_timer_init(int timer_idx, /* Configure the alarm value and the interrupt on alarm. */ timer_set_alarm_value(TIMER_GROUP_0, timer_idx, timer_interval_sec * TIMER_SCALE); timer_enable_intr(TIMER_GROUP_0, timer_idx); - timer_isr_register(TIMER_GROUP_0, timer_idx, timer_group0_isr, - (void *) timer_idx, ESP_INTR_FLAG_IRAM, NULL); + timer_isr_register(TIMER_GROUP_0, timer_idx, timer_group0_isr, + (void *) timer_idx, ESP_INTR_FLAG_IRAM, NULL); timer_start(TIMER_GROUP_0, timer_idx); }