From d0a37704a3b70eadcbf332b28452ade91bbe4845 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Thu, 14 Nov 2019 11:39:11 +0530 Subject: [PATCH 1/3] esp_timer: use freertos critical section compliant APIs Some modules use esp_timer from interrupt context and hence with vanilla FreeRTOS it should use correct critical section API --- components/esp32/esp_timer_esp32.c | 4 +- components/esp32/test/test_esp_timer.c | 38 +++++++++++++++++++ .../esp32s2beta/esp_timer_esp32s2beta.c | 4 +- components/esp_common/src/esp_timer.c | 4 +- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/components/esp32/esp_timer_esp32.c b/components/esp32/esp_timer_esp32.c index 37981a2e4..68b84f561 100644 --- a/components/esp32/esp_timer_esp32.c +++ b/components/esp32/esp_timer_esp32.c @@ -211,7 +211,7 @@ uint64_t IRAM_ATTR esp_timer_impl_get_time(void) void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) { - portENTER_CRITICAL(&s_time_update_lock); + portENTER_CRITICAL_SAFE(&s_time_update_lock); // 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, @@ -237,7 +237,7 @@ void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) } 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); + portEXIT_CRITICAL_SAFE(&s_time_update_lock); } static void IRAM_ATTR timer_alarm_isr(void *arg) diff --git a/components/esp32/test/test_esp_timer.c b/components/esp32/test/test_esp_timer.c index 4512f42b8..e50a2f177 100644 --- a/components/esp32/test/test_esp_timer.c +++ b/components/esp32/test/test_esp_timer.c @@ -11,6 +11,7 @@ #include "freertos/semphr.h" #include "test_utils.h" #include "esp_private/esp_timer_impl.h" +#include "esp_freertos_hooks.h" #ifdef CONFIG_ESP_TIMER_PROFILING #define WITH_PROFILING 1 @@ -650,6 +651,43 @@ TEST_CASE("after esp_timer_impl_advance, timers run when expected", "[esp_timer] ref_clock_deinit(); } +static esp_timer_handle_t timer1; +static SemaphoreHandle_t sem; +static void IRAM_ATTR test_tick_hook(void) +{ + static int i; + const int iterations = 16; + + if (++i <= iterations) { + if (i & 0x1) { + TEST_ESP_OK(esp_timer_start_once(timer1, 5000)); + } else { + TEST_ESP_OK(esp_timer_stop(timer1)); + } + } else { + xSemaphoreGiveFromISR(sem, 0); + } +} + +TEST_CASE("Can start/stop timer from ISR context", "[esp_timer]") +{ + void timer_func(void* arg) + { + printf("timer cb\n"); + } + + esp_timer_create_args_t create_args = { + .callback = &timer_func, + }; + TEST_ESP_OK(esp_timer_create(&create_args, &timer1)); + sem = xSemaphoreCreateBinary(); + esp_register_freertos_tick_hook(test_tick_hook); + TEST_ASSERT(xSemaphoreTake(sem, portMAX_DELAY)); + esp_deregister_freertos_tick_hook(test_tick_hook); + TEST_ESP_OK( esp_timer_delete(timer1) ); + vSemaphoreDelete(sem); +} + #if !defined(CONFIG_FREERTOS_UNICORE) && defined(CONFIG_ESP32_DPORT_WORKAROUND) #include "soc/dport_reg.h" diff --git a/components/esp32s2beta/esp_timer_esp32s2beta.c b/components/esp32s2beta/esp_timer_esp32s2beta.c index ff01cfe70..82be232e7 100644 --- a/components/esp32s2beta/esp_timer_esp32s2beta.c +++ b/components/esp32s2beta/esp_timer_esp32s2beta.c @@ -222,7 +222,7 @@ uint64_t IRAM_ATTR esp_timer_impl_get_time(void) void IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) { - portENTER_CRITICAL(&s_time_update_lock); + portENTER_CRITICAL_SAFE(&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 @@ -252,7 +252,7 @@ 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); - portEXIT_CRITICAL(&s_time_update_lock); + portEXIT_CRITICAL_SAFE(&s_time_update_lock); } static void IRAM_ATTR timer_alarm_isr(void *arg) diff --git a/components/esp_common/src/esp_timer.c b/components/esp_common/src/esp_timer.c index 8a343e34d..b24976b9c 100644 --- a/components/esp_common/src/esp_timer.c +++ b/components/esp_common/src/esp_timer.c @@ -264,12 +264,12 @@ static IRAM_ATTR bool timer_armed(esp_timer_handle_t timer) static IRAM_ATTR void timer_list_lock(void) { - portENTER_CRITICAL(&s_timer_lock); + portENTER_CRITICAL_SAFE(&s_timer_lock); } static IRAM_ATTR void timer_list_unlock(void) { - portEXIT_CRITICAL(&s_timer_lock); + portEXIT_CRITICAL_SAFE(&s_timer_lock); } static void timer_process_alarm(esp_timer_dispatch_t dispatch_method) From ecf09382dae98a61f0de15f9ea44602005ee50ac Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Thu, 14 Nov 2019 11:44:21 +0530 Subject: [PATCH 2/3] uart: critical section compliant API in ISR context --- components/driver/uart.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/driver/uart.c b/components/driver/uart.c index ebb6d0f1b..4fc5d00e0 100644 --- a/components/driver/uart.c +++ b/components/driver/uart.c @@ -413,7 +413,7 @@ static esp_err_t UART_ISR_ATTR uart_pattern_enqueue(uart_port_t uart_num, int po { UART_CHECK((p_uart_obj[uart_num]), "uart driver error", ESP_FAIL); esp_err_t ret = ESP_OK; - UART_ENTER_CRITICAL(&uart_spinlock[uart_num]); + UART_ENTER_CRITICAL_ISR(&uart_spinlock[uart_num]); uart_pat_rb_t *p_pos = &p_uart_obj[uart_num]->rx_pattern_pos; int next = p_pos->wr + 1; if (next >= p_pos->len) { @@ -427,7 +427,7 @@ static esp_err_t UART_ISR_ATTR uart_pattern_enqueue(uart_port_t uart_num, int po p_pos->wr = next; ret = ESP_OK; } - UART_EXIT_CRITICAL(&uart_spinlock[uart_num]); + UART_EXIT_CRITICAL_ISR(&uart_spinlock[uart_num]); return ret; } From f53f45038017e16a82b0f062c8d83843ee1bffe7 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Fri, 15 Nov 2019 14:23:00 +0530 Subject: [PATCH 3/3] nvs_flash: build nvs_encr.cpp only if relevant config option is enabled --- components/nvs_flash/CMakeLists.txt | 20 ++++++++++++-------- components/nvs_flash/component.mk | 4 ++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/components/nvs_flash/CMakeLists.txt b/components/nvs_flash/CMakeLists.txt index 1d7e5862d..f305dbec3 100644 --- a/components/nvs_flash/CMakeLists.txt +++ b/components/nvs_flash/CMakeLists.txt @@ -1,10 +1,14 @@ -idf_component_register(SRCS "src/nvs_api.cpp" - "src/nvs_encr.cpp" - "src/nvs_item_hash_list.cpp" - "src/nvs_ops.cpp" - "src/nvs_page.cpp" - "src/nvs_pagemanager.cpp" - "src/nvs_storage.cpp" - "src/nvs_types.cpp" +set(srcs "src/nvs_api.cpp" + "src/nvs_item_hash_list.cpp" + "src/nvs_ops.cpp" + "src/nvs_page.cpp" + "src/nvs_pagemanager.cpp" + "src/nvs_storage.cpp" + "src/nvs_types.cpp") +if(CONFIG_NVS_ENCRYPTION) + list(APPEND srcs "src/nvs_encr.cpp") +endif() + +idf_component_register(SRCS "${srcs}" REQUIRES spi_flash mbedtls INCLUDE_DIRS include) diff --git a/components/nvs_flash/component.mk b/components/nvs_flash/component.mk index 2341ae7b4..8b2fda4f5 100644 --- a/components/nvs_flash/component.mk +++ b/components/nvs_flash/component.mk @@ -5,3 +5,7 @@ COMPONENT_ADD_INCLUDEDIRS := include COMPONENT_SRCDIRS := src + +ifndef CONFIG_NVS_ENCRYPTION +COMPONENT_OBJEXCLUDE := src/nvs_encr.o +endif