sleep: fix deadlock in esp_timer_impl_advance after light sleep

When light sleep is started, the other CPU gets halted using DPORT
stall mechanism. This can happen while it is inside an esp_timer
critical section, which may lead to a deadlock. This change adds
functions to take and release esp_timer lock before entering
DPORT critical section, preventing the deadlock.

diff --git a/components/esp32/esp_timer_esp32.c b/components/esp32/esp_timer_esp32.c
index 4f1200515acec036422fbcdef1996167d5dd3207..b2c29721b2876e55b2c178dee18b426cb303c49d 100644 (file)
--- a/components/esp32/esp_timer_esp32.c
+++ b/components/esp32/esp_timer_esp32.c
@@ -165,6 +165,16 @@ static inline void IRAM_ATTR timer_count_reload(void)
     REG_WRITE(FRC_TIMER_LOAD_REG(1), REG_READ(FRC_TIMER_COUNT_REG(1)) - ALARM_OVERFLOW_VAL);
 }
 
+void esp_timer_impl_lock()
+{
+    portENTER_CRITICAL(&s_time_update_lock);
+}
+
+void esp_timer_impl_unlock()
+{
+    portEXIT_CRITICAL(&s_time_update_lock);
+}
+
 uint64_t IRAM_ATTR esp_timer_impl_get_time()
 {
     uint32_t timer_val;

diff --git a/components/esp32/esp_timer_impl.h b/components/esp32/esp_timer_impl.h
index 9c4642c47445667e0ecb026941489402ee434bdd..5871428a5f898f1dac98a8758ebb37173d544c18 100644 (file)
--- a/components/esp32/esp_timer_impl.h
+++ b/components/esp32/esp_timer_impl.h
@@ -84,3 +84,18 @@ uint64_t esp_timer_impl_get_time();
  * @return minimal period of periodic timer, in microseconds
  */
 uint64_t esp_timer_impl_get_min_period_us();
+
+/**
+ * @brief obtain internal critical section used esp_timer implementation
+ * This can be used when a sequence of calls to esp_timer has to be made,
+ * and it is necessary that the state of the timer is consistent between
+ * the calls. Should be treated in the same way as a spinlock.
+ * Call esp_timer_impl_unlock to release the lock
+ */
+void esp_timer_impl_lock();
+
+
+/**
+ * @brief counterpart of esp_timer_impl_lock
+ */
+void esp_timer_impl_unlock();

diff --git a/components/esp32/sleep_modes.c b/components/esp32/sleep_modes.c
index a5f14d7f2bd5b102f3d9c5454d9e894dcf8ff27c..65d0f056833511ad69ae2243a6df5c80e9c6d492 100644 (file)
--- a/components/esp32/sleep_modes.c
+++ b/components/esp32/sleep_modes.c
@@ -279,6 +279,11 @@ esp_err_t esp_light_sleep_start()
 {
     static portMUX_TYPE light_sleep_lock = portMUX_INITIALIZER_UNLOCKED;
     portENTER_CRITICAL(&light_sleep_lock);
+    /* We will be calling esp_timer_impl_advance inside DPORT access critical
+     * section. Make sure the code on the other CPU is not holding esp_timer
+     * lock, otherwise there will be deadlock.
+     */
+    esp_timer_impl_lock();
     s_config.rtc_ticks_at_sleep_start = rtc_time_get();
     uint64_t frc_time_at_start = esp_timer_get_time();
     DPORT_STALL_OTHER_CPU_START();
@@ -332,6 +337,7 @@ esp_err_t esp_light_sleep_start()
     }
     esp_set_time_from_rtc();
 
+    esp_timer_impl_unlock();
     DPORT_STALL_OTHER_CPU_END();
     rtc_wdt_disable();
     portEXIT_CRITICAL(&light_sleep_lock);

diff --git a/components/esp32/test/test_sleep.c b/components/esp32/test/test_sleep.c
index ac090fff9821f6c0815a1c3a3252c88792ed4cb0..c298d3220b8fb1dc46bfa25e2e32554a8ca68c8b 100644 (file)
--- a/components/esp32/test/test_sleep.c
+++ b/components/esp32/test/test_sleep.c
@@ -91,6 +91,35 @@ TEST_CASE("light sleep stress test", "[deepsleep]")
     vSemaphoreDelete(done);
 }
 
+TEST_CASE("light sleep stress test with periodic esp_timer", "[deepsleep]")
+{
+    void timer_func(void* arg)
+    {
+        ets_delay_us(50);
+    }
+
+    SemaphoreHandle_t done = xSemaphoreCreateCounting(2, 0);
+    esp_sleep_enable_timer_wakeup(1000);
+    esp_timer_handle_t timer;
+    esp_timer_create_args_t config = {
+            .callback = &timer_func,
+    };
+    TEST_ESP_OK(esp_timer_create(&config, &timer));
+    esp_timer_start_periodic(timer, 500);
+    xTaskCreatePinnedToCore(&test_light_sleep, "ls1", 4096, done, UNITY_FREERTOS_PRIORITY + 1, NULL, 0);
+#if portNUM_PROCESSORS == 2
+    xTaskCreatePinnedToCore(&test_light_sleep, "ls1", 4096, done, UNITY_FREERTOS_PRIORITY + 1, NULL, 1);
+#endif
+    xSemaphoreTake(done, portMAX_DELAY);
+#if portNUM_PROCESSORS == 2
+    xSemaphoreTake(done, portMAX_DELAY);
+#endif
+    vSemaphoreDelete(done);
+    esp_timer_stop(timer);
+    esp_timer_delete(timer);
+}
+
+
 #ifdef CONFIG_ESP32_RTC_CLOCK_SOURCE_EXTERNAL_CRYSTAL
 #define MAX_SLEEP_TIME_ERROR_US 200
 #else