Merge branch 'bugfix/forbid_uart_suspend_auto_lightsleep' into 'master'

uart: Fixed issue that TX be blocked by auto-lightsleep

Closes IDF-5518

See merge request espressif/esp-idf!21719

components/esp_driver_uart/src/uart.c

@@ -31,6 +31,7 @@
 #include "sdkconfig.h"
 #include "esp_rom_gpio.h"
 #include "clk_ctrl_os.h"
+#include "esp_pm.h"
 
 #ifdef CONFIG_UART_ISR_IN_IRAM
 #define UART_ISR_ATTR     IRAM_ATTR
@@ -40,6 +41,14 @@
 #define UART_MALLOC_CAPS  MALLOC_CAP_DEFAULT
 #endif
 
+// Whether to use the APB_MAX lock.
+// Requirement of each chip, to keep sending:
+// - ESP32, S2, C3, S3: Protect APB, which is the core clock and clock of UART FIFO
+// - ESP32-C2: Protect APB (UART FIFO clock), core clock (TODO: IDF-8348)
+// - ESP32-C6, H2 and later chips: Protect core clock. Run in light-sleep hasn't been developed yet (TODO: IDF-8349),
+//   also need to avoid auto light-sleep.
+#define PROTECT_APB     (CONFIG_PM_ENABLE)
+
 #define XOFF (0x13)
 #define XON (0x11)
 
@@ -140,6 +149,9 @@ typedef struct {
     SemaphoreHandle_t tx_fifo_sem;      /*!< UART TX FIFO semaphore*/
     SemaphoreHandle_t tx_done_sem;      /*!< UART TX done semaphore*/
     SemaphoreHandle_t tx_brk_sem;       /*!< UART TX send break done semaphore*/
+#if PROTECT_APB
+    esp_pm_lock_handle_t pm_lock;   ///< Power management lock
+#endif
 } uart_obj_t;
 
 typedef struct {
@@ -1271,15 +1283,19 @@ esp_err_t uart_wait_tx_done(uart_port_t uart_num, TickType_t ticks_to_wait)
     } else {
         ticks_to_wait = ticks_to_wait - (ticks_end - ticks_start);
     }
+
+#if PROTECT_APB
+    esp_pm_lock_acquire(p_uart_obj[uart_num]->pm_lock);
+#endif
     //take 2nd tx_done_sem, wait given from ISR
     res = xSemaphoreTake(p_uart_obj[uart_num]->tx_done_sem, (TickType_t)ticks_to_wait);
-    if (res == pdFALSE) {
-        // The TX_DONE interrupt will be disabled in ISR
-        xSemaphoreGive(p_uart_obj[uart_num]->tx_mux);
-        return ESP_ERR_TIMEOUT;
-    }
+#if PROTECT_APB
+    esp_pm_lock_release(p_uart_obj[uart_num]->pm_lock);
+#endif
+
+    // The TX_DONE interrupt will be disabled in ISR
     xSemaphoreGive(p_uart_obj[uart_num]->tx_mux);
-    return ESP_OK;
+    return (res == pdFALSE) ? ESP_ERR_TIMEOUT : ESP_OK;
 }
 
 int uart_tx_chars(uart_port_t uart_num, const char *buffer, uint32_t len)
@@ -1306,6 +1322,9 @@ static int uart_tx_all(uart_port_t uart_num, const char *src, size_t size, bool
 
     //lock for uart_tx
     xSemaphoreTake(p_uart_obj[uart_num]->tx_mux, (TickType_t)portMAX_DELAY);
+#if PROTECT_APB
+    esp_pm_lock_acquire(p_uart_obj[uart_num]->pm_lock);
+#endif
     p_uart_obj[uart_num]->coll_det_flg = false;
     if (p_uart_obj[uart_num]->tx_buf_size > 0) {
         size_t max_size = xRingbufferGetMaxItemSize(p_uart_obj[uart_num]->tx_ring_buf);
@@ -1349,6 +1368,9 @@ static int uart_tx_all(uart_port_t uart_num, const char *src, size_t size, bool
         }
         xSemaphoreGive(p_uart_obj[uart_num]->tx_fifo_sem);
     }
+#if PROTECT_APB
+    esp_pm_lock_release(p_uart_obj[uart_num]->pm_lock);
+#endif
     xSemaphoreGive(p_uart_obj[uart_num]->tx_mux);
     return original_size;
 }
@@ -1533,6 +1555,11 @@ static void uart_free_driver_obj(uart_obj_t *uart_obj)
     }
 
     heap_caps_free(uart_obj->rx_data_buf);
+#if PROTECT_APB
+    if (uart_obj->pm_lock) {
+        esp_pm_lock_delete(uart_obj->pm_lock);
+    }
+#endif
     heap_caps_free(uart_obj);
 }
 
@@ -1568,6 +1595,12 @@ static uart_obj_t *uart_alloc_driver_obj(uart_port_t uart_num, int event_queue_s
             !uart_obj->tx_done_sem || !uart_obj->tx_fifo_sem) {
         goto err;
     }
+#if PROTECT_APB
+    if (esp_pm_lock_create(ESP_PM_APB_FREQ_MAX, 0, "uart_driver",
+                           &uart_obj->pm_lock) != ESP_OK) {
+        goto err;
+    }
+#endif
 
     return uart_obj;
 

components/esp_driver_uart/test_apps/uart/main/CMakeLists.txt

@@ -1,7 +1,8 @@
 # In order for the cases defined by `TEST_CASE` to be linked into the final elf,
 # the component can be registered as WHOLE_ARCHIVE
 idf_component_register(
-    SRCS "test_app_main.c" "test_uart.c"
-    REQUIRES esp_driver_uart unity esp_psram test_utils esp_driver_gpio
+    SRCS "test_app_main.c" "test_uart.c" "test_uart_auto_lightsleep.c"
+    REQUIRES esp_driver_uart unity esp_psram test_utils esp_driver_gpio esp_pm
+    PRIV_INCLUDE_DIRS .
     WHOLE_ARCHIVE
 )

components/esp_driver_uart/test_apps/uart/main/test_common.h

@@ -0,0 +1,16 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Unlicense OR CC0-1.0
+ */
+#include "driver/uart.h"
+
+typedef struct {
+    uart_port_t port_num;
+    soc_module_clk_t default_src_clk;
+    int tx_pin_num;
+    int rx_pin_num;
+    uint32_t rx_flow_ctrl_thresh;
+} uart_port_param_t;
+
+bool port_select(uart_port_param_t *port_param);

components/esp_driver_uart/test_apps/uart/main/test_uart.c

@@ -15,21 +15,14 @@
 #include "soc/uart_pins.h"
 #include "soc/soc_caps.h"
 #include "soc/clk_tree_defs.h"
+#include "test_common.h"
 
 #define BUF_SIZE         (100)
 #define UART_BAUD_11520  (11520)
 #define UART_BAUD_115200 (115200)
 #define TOLERANCE        (0.02)    //baud rate error tolerance 2%.
 
-typedef struct {
-    uart_port_t port_num;
-    soc_module_clk_t default_src_clk;
-    int tx_pin_num;
-    int rx_pin_num;
-    uint32_t rx_flow_ctrl_thresh;
-} uart_port_param_t;
-
-static bool port_select(uart_port_param_t *port_param)
+bool port_select(uart_port_param_t *port_param)
 {
     char argv[10];
     unity_wait_for_signal_param("select to test 'uart' or 'lp_uart' port", argv, sizeof(argv));

components/esp_driver_uart/test_apps/uart/main/test_uart_auto_lightsleep.c

@@ -0,0 +1,91 @@
+/*
+ * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include "sdkconfig.h"
+#include "unity.h"
+#include "driver/uart.h"
+#include "esp_pm.h"
+#include "esp_log.h"
+#include "test_common.h"
+#include "esp_private/sleep_cpu.h" //for sleep_cpu_configure
+
+#define UART_TAG         "Uart"
+#define TEST_BUF_SIZE    256
+
+//This should be larger than FIFO_SIZE + 2 * TEST_DRIVER_BUF_SIZE, so that blocking will happen
+#define TEST_WRITE_SIZE 1024
+
+#define TEST_RTS   UART_PIN_NO_CHANGE
+#define TEST_CTS   UART_PIN_NO_CHANGE
+
+#define TEST_UART_BAUD_RATE     115200
+
+#define MAX_FREQ    (CONFIG_ESP_DEFAULT_CPU_FREQ_MHZ)
+
+#if CONFIG_XTAL_FREQ_40
+#define MIN_FREQ    10
+#elif CONFIG_XTAL_FREQ_32
+#define MIN_FREQ    8
+#elif CONFIG_XTAL_FREQ_26
+#define MIN_FREQ    13
+#endif
+
+#if !TEMPORARY_DISABLED_FOR_TARGETS(ESP32P4)
+#if CONFIG_PM_ENABLE
+
+TEST_CASE("uart tx won't be blocked by auto light sleep", "[uart]")
+{
+    uart_port_param_t port_param = {};
+    TEST_ASSERT(port_select(&port_param));
+
+    uart_port_t port_num = port_param.port_num;
+
+    // Configure dynamic frequency scaling:
+    // maximum and minimum frequencies are set in sdkconfig,
+    // automatic light sleep is enabled if tickless idle support is enabled.
+    esp_pm_config_t pm_config = {
+        .max_freq_mhz = MAX_FREQ,
+        .min_freq_mhz = MIN_FREQ,
+#if CONFIG_FREERTOS_USE_TICKLESS_IDLE
+        .light_sleep_enable = true
+#endif
+    };
+    TEST_ESP_OK(esp_pm_configure(&pm_config));
+
+    uart_config_t uart_config = {
+        .baud_rate = TEST_UART_BAUD_RATE,
+        .data_bits = UART_DATA_8_BITS,
+        .parity    = UART_PARITY_DISABLE,
+        .stop_bits = UART_STOP_BITS_1,
+        .flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
+        .source_clk = port_param.default_src_clk,
+    };
+    int intr_alloc_flags = 0;
+
+    TEST_ESP_OK(uart_driver_install(port_num, TEST_BUF_SIZE, 0, 0, NULL, intr_alloc_flags));
+    TEST_ESP_OK(uart_param_config(port_num, &uart_config));
+    TEST_ESP_OK(uart_set_pin(port_num, port_param.tx_pin_num, port_param.rx_pin_num, TEST_RTS, TEST_CTS));
+
+    // Configure a temporary buffer for the incoming data
+    const int len = TEST_WRITE_SIZE;
+    uint8_t *data = (uint8_t *) malloc(len);
+
+    //If auto lightsleep happen, there will be deadlock in either one of the two following functions
+    uart_write_bytes(port_num, (const char *) data, len);
+    uart_wait_tx_done(port_num, portMAX_DELAY);
+
+    ESP_LOGI(UART_TAG, "return from uart_write_bytes");
+
+    uart_driver_delete(port_num);
+    free(data);
+
+#if CONFIG_PM_POWER_DOWN_CPU_IN_LIGHT_SLEEP
+    //When PD_CPU enabled, retention may cause 14K memory leak. Workaround to release the memory
+    sleep_cpu_configure(false);
+#endif
+}
+#endif // CONFIG_PM_ENABLE
+#endif //!TEMPORARY_DISABLED_FOR_TARGETS(ESP32P4)

components/soc/include/soc/uart_periph.h

@@ -10,6 +10,8 @@
 #include "soc/gpio_sig_map.h"
 #include "soc/io_mux_reg.h"
 #include "soc/uart_pins.h"
+#include "soc/uart_struct.h"
+#include "soc/uart_reg.h"
 
 #ifdef __cplusplus
 extern "C" {