esp_hw_support: Fix esp_light_sleep_start() deadlock

esp_light_sleep_start() will stall the other CPU via esp_ipc_isr_stall_other_cpu(). After stalling the other CPU,
will call esp_clk_... API which themselves take locks. If the other stalled CPU is holding those locks, this will
result in a deadlock.

This commit adds a workaround calling esp_clk_private_lock() to take the lock before stalling the other CPU.

components/esp_hw_support/esp_clk.c

@@ -136,3 +136,13 @@ uint64_t esp_clk_rtc_time(void)
     return 0;
 #endif
 }
+
+void esp_clk_private_lock(void)
+{
+    portENTER_CRITICAL(&s_esp_rtc_time_lock);
+}
+
+void esp_clk_private_unlock(void)
+{
+    portEXIT_CRITICAL(&s_esp_rtc_time_lock);
+}

components/esp_hw_support/include/esp_private/esp_clk.h

@@ -82,6 +82,19 @@ int esp_clk_xtal_freq(void);
  */
 uint64_t esp_clk_rtc_time(void);
 
+/**
+ * @brief obtain internal critical section used esp_clk implementation.
+ *
+ * This is used by the esp_light_sleep_start() to avoid deadlocking when it
+ * calls esp_clk related API after stalling the other CPU.
+ */
+void esp_clk_private_lock(void);
+
+/**
+ * @brief counterpart of esp_clk_private_lock
+ */
+void esp_clk_private_unlock(void);
+
 #ifdef __cplusplus
 }
 #endif

components/esp_hw_support/sleep_modes.c

@@ -655,12 +655,29 @@ esp_err_t esp_light_sleep_start(void)
     s_config.ccount_ticks_record = cpu_ll_get_cycle_count();
     static portMUX_TYPE light_sleep_lock = portMUX_INITIALIZER_UNLOCKED;
     portENTER_CRITICAL(&light_sleep_lock);
+    /*
+    Note: We are about to stall the other CPU via the esp_ipc_isr_stall_other_cpu(). However, there is a chance of
+    deadlock if after stalling the other CPU, we attempt to take spinlocks already held by the other CPU that is.
+
+    Thus any functions that we call after stalling the other CPU will need to have the locks taken first to avoid
+    deadlock.
+
+    Todo: IDF-5257
+    */
+
     /* We will be calling esp_timer_private_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_private_lock();
 
+    /* We will be calling esp_rtc_get_time_us() below. Make sure the code on the other CPU is not holding the
+     * esp_rtc_get_time_us() lock, otherwise there will be deadlock. esp_rtc_get_time_us() is called via:
+     *
+     * - esp_clk_slowclk_cal_set() -> esp_rtc_get_time_us()
+     */
+    esp_clk_private_lock();
+
     s_config.rtc_ticks_at_sleep_start = rtc_time_get();
     uint32_t ccount_at_sleep_start = cpu_ll_get_cycle_count();
     uint64_t frc_time_at_start = esp_timer_get_time();
@@ -796,6 +813,7 @@ esp_err_t esp_light_sleep_start(void)
     }
     esp_set_time_from_rtc();
 
+    esp_clk_private_unlock();
     esp_timer_private_unlock();
     esp_ipc_isr_release_other_cpu();
     if (!wdt_was_enabled) {

components/esp_ipc/include/esp_ipc_isr.h

@@ -62,6 +62,7 @@ void esp_ipc_isr_asm_call_blocking(esp_ipc_isr_func_t func, void* arg);
  * - If the stall feature is paused using esp_ipc_isr_stall_pause(), this function will have no effect
  *
  * @note This function is not available in single-core mode.
+ * @note It is the caller's responsibility to avoid deadlocking on spinlocks
  */
 void esp_ipc_isr_stall_other_cpu(void);
 

components/esp_timer/include/esp_private/esp_timer_private.h

@@ -52,11 +52,11 @@ void esp_timer_private_update_apb_freq(uint32_t apb_ticks_per_us);
 void esp_timer_private_advance(int64_t time_us);
 
 /**
- * @brief obtain internal critical section used esp_timer implementation
+ * @brief obtain internal critical section used in the 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_unlock to release the lock
+ * Call esp_timer_private_unlock to release the lock
  */
 void esp_timer_private_lock(void);