Merge branch 'feature/add_uart_iram_safe_check' into 'master'

feat(uart): Add sdkconfig.ci.iram_safe test for UART driver See merge request espressif/esp-idf!25618
2024-10-05 20:47:46 -04:00 · 2023-08-31 12:57:46 +08:00 · 2023-08-31 12:57:46 +08:00 · 91aab6bda0
commit 91aab6bda0
parent eb8883cc20 767698b90d
20 changed files with 86 additions and 38 deletions
--- a/components/driver/CMakeLists.txt
+++ b/components/driver/CMakeLists.txt
@ -31,6 +31,10 @@ set(includes "include"
             "uart/include"
             "usb_serial_jtag/include")

+# Always included linker fragments
+set(ldfragments "linker.lf"
+                "gpio/linker.lf")
+
 # ADC related source files (dprecated)
 if(CONFIG_SOC_ADC_SUPPORTED)
    list(APPEND srcs "deprecated/adc_legacy.c")
@ -83,6 +87,8 @@ if(CONFIG_SOC_GPTIMER_SUPPORTED)
    list(APPEND srcs "gptimer/gptimer.c"
                     "gptimer/gptimer_priv.c"
                     "deprecated/timer_legacy.c")
+
+    list(APPEND ldfragments "gptimer/linker.lf")
 endif()

 if(CONFIG_SOC_TIMER_SUPPORT_ETM)
@ -95,6 +101,8 @@ if(CONFIG_SOC_I2C_SUPPORTED)
                     "i2c/i2c_master.c"
                     "i2c/i2c_common.c"
        )
+
+    list(APPEND ldfragments "i2c/linker.lf")
 endif()

 # I2S related source files
@ -190,11 +198,15 @@ endif()
 # TWAI related source files
 if(CONFIG_SOC_TWAI_SUPPORTED)
    list(APPEND srcs "twai/twai.c")
+
+    list(APPEND ldfragments "twai/linker.lf")
 endif()

 # UART related source files
 if(CONFIG_SOC_UART_SUPPORTED)
    list(APPEND srcs "uart/uart.c")
+
+    list(APPEND ldfragments "uart/linker.lf")
 endif()

 # USB Serial JTAG related source files
@ -216,10 +228,11 @@ else()
    # (Legacy drivers requires `esp_adc`, due to ADC HW resource mutex logics are there.
    # Can be removed together with legacy drivers)
    idf_component_register(SRCS "${srcs}"
-                        INCLUDE_DIRS ${includes}
-                        PRIV_REQUIRES efuse esp_timer
-                        REQUIRES esp_pm esp_ringbuf freertos soc hal esp_hw_support
-                        LDFRAGMENTS linker.lf gptimer/linker.lf gpio/linker.lf twai/linker.lf i2c/linker.lf)
+                           INCLUDE_DIRS ${includes}
+                           PRIV_REQUIRES efuse esp_timer
+                           REQUIRES esp_pm esp_ringbuf freertos soc hal esp_hw_support
+                           LDFRAGMENTS ${ldfragments}
+                          )
 endif()

 # If system needs to monitor USJ connection status, then usb_serial_jtag_connection_monitor object file has to be linked
--- a/components/driver/Kconfig
+++ b/components/driver/Kconfig
@ -224,17 +224,7 @@ menu "Driver Configurations"

    endmenu # TEMP_SENSOR Configuration

-    menu "UART Configuration"
-
-        config UART_ISR_IN_IRAM
-            bool "Place UART ISR function into IRAM"
-            depends on !RINGBUF_PLACE_ISR_FUNCTIONS_INTO_FLASH
-            default n
-            help
-                If this option is not selected, UART interrupt will be disabled for a long time and
-                may cause data lost when doing spi flash operation.
-
-    endmenu # UART Configuration
+    orsource "./uart/Kconfig.uart"

    menu "GPIO Configuration"
        config GPIO_ESP32_SUPPORT_SWITCH_SLP_PULL
--- a/components/driver/test_apps/uart/CMakeLists.txt
+++ b/components/driver/test_apps/uart/CMakeLists.txt
@ -8,3 +8,16 @@ set(EXTRA_COMPONENT_DIRS "$ENV{IDF_PATH}/tools/unit-test-app/components")

 include($ENV{IDF_PATH}/tools/cmake/project.cmake)
 project(uart_test)
+
+if(CONFIG_COMPILER_DUMP_RTL_FILES)
+    add_custom_target(check_test_app_sections ALL
+                      COMMAND ${PYTHON} $ENV{IDF_PATH}/tools/ci/check_callgraph.py
+                      --rtl-dirs ${CMAKE_BINARY_DIR}/esp-idf/driver/,${CMAKE_BINARY_DIR}/esp-idf/hal/
+                      --elf-file ${CMAKE_BINARY_DIR}/uart_test.elf
+                      find-refs
+                      --from-sections=.iram0.text
+                      --to-sections=.flash.text,.flash.rodata
+                      --exit-code
+                      DEPENDS ${elf}
+                      )
+endif()
--- a/components/driver/test_apps/uart/pytest_uart.py
+++ b/components/driver/test_apps/uart/pytest_uart.py
@ -1,4 +1,4 @@
-# SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
+# SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
 # SPDX-License-Identifier: CC0-1.0

 import pytest
@ -6,6 +6,13 @@ import pytest

@pytest.mark.supported_targets
@pytest.mark.generic
-@pytest.mark.parametrize('config', ['release',], indirect=True,)
+@pytest.mark.parametrize(
+    'config',
+    [
+        'iram_safe',
+        'release',
+    ],
+    indirect=True,
+)
 def test_uart_single_dev(case_tester) -> None:                # type: ignore
    case_tester.run_all_normal_cases(reset=True)
--- a/components/driver/test_apps/uart/sdkconfig.ci.iram_safe
+++ b/components/driver/test_apps/uart/sdkconfig.ci.iram_safe
@ -0,0 +1,8 @@
+CONFIG_COMPILER_DUMP_RTL_FILES=y
+CONFIG_UART_ISR_IN_IRAM=y
+CONFIG_COMPILER_OPTIMIZATION_NONE=y
+# place non-ISR FreeRTOS functions in Flash
+CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y
+# silent the error check, as the error string are stored in rodata, causing RTL check failure
+CONFIG_COMPILER_OPTIMIZATION_CHECKS_SILENT=y
+CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y
--- a/components/driver/test_apps/uart/sdkconfig.defaults
+++ b/components/driver/test_apps/uart/sdkconfig.defaults
@ -1,2 +1,2 @@
 CONFIG_FREERTOS_HZ=1000
-CONFIG_ESP_TASK_WDT=n
+CONFIG_ESP_TASK_WDT_INIT=n
--- a/components/driver/uart/Kconfig.uart
+++ b/components/driver/uart/Kconfig.uart
@ -0,0 +1,11 @@
+menu "UART Configuration"
+
+    config UART_ISR_IN_IRAM
+        bool "Place UART ISR function into IRAM"
+        depends on !RINGBUF_PLACE_ISR_FUNCTIONS_INTO_FLASH
+        default n
+        help
+            If this option is not selected, UART interrupt will be disabled for a long time and
+            may cause data lost when doing spi flash operation.
+
+endmenu # UART Configuration
--- a/components/driver/uart/linker.lf
+++ b/components/driver/uart/linker.lf
@ -0,0 +1,5 @@
+[mapping:uart_hal]
+archive: libhal.a
+entries:
+    if UART_ISR_IN_IRAM = y:
+        uart_hal_iram (noflash)
--- a/components/driver/uart/uart.c
+++ b/components/driver/uart/uart.c
@ -355,7 +355,7 @@ esp_err_t uart_get_hw_flow_ctrl(uart_port_t uart_num, uart_hw_flowcontrol_t *flo

 esp_err_t UART_ISR_ATTR uart_clear_intr_status(uart_port_t uart_num, uint32_t clr_mask)
 {
-    ESP_RETURN_ON_FALSE((uart_num < UART_NUM_MAX), ESP_FAIL, UART_TAG, "uart_num error");
+    ESP_RETURN_ON_FALSE_ISR((uart_num < UART_NUM_MAX), ESP_FAIL, UART_TAG, "uart_num error");
    uart_hal_clr_intsts_mask(&(uart_context[uart_num].hal), clr_mask);
    return ESP_OK;
 }
--- a/components/esp_hw_support/sleep_modes.c
+++ b/components/esp_hw_support/sleep_modes.c
@ -450,10 +450,10 @@ FORCE_INLINE_ATTR void suspend_uarts(void)
 #if SOC_UART_SUPPORT_FSM_TX_WAIT_SEND
        uint32_t uart_fsm = 0;
        do {
-            uart_fsm = uart_ll_get_fsm_status(i);
+            uart_fsm = uart_ll_get_tx_fsm_status(i);
        } while (!(uart_fsm == UART_LL_FSM_IDLE || uart_fsm == UART_LL_FSM_TX_WAIT_SEND));
 #else
-        while (uart_ll_get_fsm_status(i) != 0) {}
+        while (uart_ll_get_tx_fsm_status(i) != 0) {}
 #endif
    }
 }
--- a/components/esp_system/linker.lf
+++ b/components/esp_system/linker.lf
@ -36,3 +36,9 @@ entries:
    if ESP_CONSOLE_USB_CDC_SUPPORT_ETS_PRINTF:
        vfs_cdcacm:cdcacm_tx_cb (noflash)
        vfs_cdcacm:cdcacm_rx_cb (noflash)
+
+[mapping:esp_system_hal]
+archive: libhal.a
+entries:
+    if ESP_PANIC_HANDLER_IRAM = y:
+        uart_hal_iram (noflash)
--- a/components/hal/esp32/include/hal/uart_ll.h
+++ b/components/hal/esp32/include/hal/uart_ll.h
@ -1008,7 +1008,7 @@ FORCE_INLINE_ATTR void uart_ll_force_xon(uart_port_t uart_num)
 *
 * @return UART module FSM status.
 */
-FORCE_INLINE_ATTR uint32_t uart_ll_get_fsm_status(uart_port_t uart_num)
+FORCE_INLINE_ATTR uint32_t uart_ll_get_tx_fsm_status(uart_port_t uart_num)
 {
    return REG_GET_FIELD(UART_STATUS_REG(uart_num), UART_ST_UTX_OUT);
 }
--- a/components/hal/esp32c2/include/hal/uart_ll.h
+++ b/components/hal/esp32c2/include/hal/uart_ll.h
@ -943,7 +943,7 @@ FORCE_INLINE_ATTR void uart_ll_force_xon(uart_port_t uart_num)
 *
 * @return UART module FSM status.
 */
-FORCE_INLINE_ATTR uint32_t uart_ll_get_fsm_status(uart_port_t uart_num)
+FORCE_INLINE_ATTR uint32_t uart_ll_get_tx_fsm_status(uart_port_t uart_num)
 {
    return REG_GET_FIELD(UART_FSM_STATUS_REG(uart_num), UART_ST_UTX_OUT);
 }
--- a/components/hal/esp32c3/include/hal/uart_ll.h
+++ b/components/hal/esp32c3/include/hal/uart_ll.h
@ -1006,7 +1006,7 @@ FORCE_INLINE_ATTR void uart_ll_force_xon(uart_port_t uart_num)
 *
 * @return UART module FSM status.
 */
-FORCE_INLINE_ATTR uint32_t uart_ll_get_fsm_status(uart_port_t uart_num)
+FORCE_INLINE_ATTR uint32_t uart_ll_get_tx_fsm_status(uart_port_t uart_num)
 {
    return REG_GET_FIELD(UART_FSM_STATUS_REG(uart_num), UART_ST_UTX_OUT);
 }
--- a/components/hal/esp32c6/include/hal/uart_ll.h
+++ b/components/hal/esp32c6/include/hal/uart_ll.h
@ -1241,7 +1241,7 @@ FORCE_INLINE_ATTR void uart_ll_force_xon(uart_port_t uart_num)
 *
 * @return UART module FSM status.
 */
-FORCE_INLINE_ATTR uint32_t uart_ll_get_fsm_status(uart_port_t uart_num)
+FORCE_INLINE_ATTR uint32_t uart_ll_get_tx_fsm_status(uart_port_t uart_num)
 {
    uart_dev_t *hw = UART_LL_GET_HW(uart_num);
    return hw->fsm_status.st_utx_out;
--- a/components/hal/esp32h2/include/hal/uart_ll.h
+++ b/components/hal/esp32h2/include/hal/uart_ll.h
@ -1069,7 +1069,7 @@ FORCE_INLINE_ATTR void uart_ll_force_xon(uart_port_t uart_num)
 *
 * @return UART module FSM status.
 */
-FORCE_INLINE_ATTR uint32_t uart_ll_get_fsm_status(uart_port_t uart_num)
+FORCE_INLINE_ATTR uint32_t uart_ll_get_tx_fsm_status(uart_port_t uart_num)
 {
    return REG_GET_FIELD(UART_FSM_STATUS_REG(uart_num), UART_ST_UTX_OUT);
 }
--- a/components/hal/esp32p4/include/hal/uart_ll.h
+++ b/components/hal/esp32p4/include/hal/uart_ll.h
@ -186,6 +186,7 @@ static inline uint32_t uart_ll_get_baudrate(uart_dev_t *hw, uint32_t sclk_freq)
    // typeof(hw->clkdiv_sync) div_reg = hw->clkdiv_sync;
    // return ((sclk_freq << 4)) / (((div_reg.clkdiv << 4) | div_reg.clkdiv_frag) * (HAL_FORCE_READ_U32_REG_FIELD(hw->clk_conf, sclk_div_num) + 1));
    HAL_ASSERT(false);
+    return 115200; // TODO: IDF-5338
 }

 /**
@ -937,8 +938,7 @@ static inline uint16_t uart_ll_get_rx_tout_thr(uart_dev_t *hw)
 */
 static inline uint16_t uart_ll_max_tout_thrd(uart_dev_t *hw)
 {
-    // return UART_RX_TOUT_THRHD_V;
-    HAL_ASSERT(false);
+    return UART_RX_TOUT_THRHD_V;
 }

 /**
@ -1035,16 +1035,15 @@ static inline void uart_ll_force_xon(uart_port_t uart_num)
 }

 /**
- * @brief  Get UART finite-state machine status.
+ * @brief  Get UART transmitter finite-state machine status.
 *
 * @param  uart_num UART port number, the max port number is (UART_NUM_MAX -1).
 *
 * @return UART module FSM status.
 */
-static inline uint32_t uart_ll_get_fsm_status(uart_port_t uart_num)
+static inline uint32_t uart_ll_get_tx_fsm_status(uart_port_t uart_num)
 {
-    // return REG_GET_FIELD(UART_FSM_STATUS_REG(uart_num), UART_ST_UTX_OUT);
-    HAL_ASSERT(false);
+    return REG_GET_FIELD(UART_FSM_STATUS_REG(uart_num), UART_ST_UTX_OUT);
 }

 #ifdef __cplusplus
--- a/components/hal/esp32s2/include/hal/uart_ll.h
+++ b/components/hal/esp32s2/include/hal/uart_ll.h
@ -939,7 +939,7 @@ FORCE_INLINE_ATTR void uart_ll_force_xon(uart_port_t uart_num)
 *
 * @return UART module FSM status.
 */
-FORCE_INLINE_ATTR uint32_t uart_ll_get_fsm_status(uart_port_t uart_num)
+FORCE_INLINE_ATTR uint32_t uart_ll_get_tx_fsm_status(uart_port_t uart_num)
 {
    return REG_GET_FIELD(UART_FSM_STATUS_REG(uart_num), UART_ST_UTX_OUT);
 }
--- a/components/hal/esp32s3/include/hal/uart_ll.h
+++ b/components/hal/esp32s3/include/hal/uart_ll.h
@ -961,7 +961,7 @@ FORCE_INLINE_ATTR void uart_ll_force_xon(uart_port_t uart_num)
 *
 * @return UART module FSM status.
 */
-FORCE_INLINE_ATTR uint32_t uart_ll_get_fsm_status(uart_port_t uart_num)
+FORCE_INLINE_ATTR uint32_t uart_ll_get_tx_fsm_status(uart_port_t uart_num)
 {
    return REG_GET_FIELD(UART_FSM_STATUS_REG(uart_num), UART_ST_UTX_OUT);
 }
--- a/components/hal/linker.lf
+++ b/components/hal/linker.lf
@ -14,10 +14,6 @@ entries:
            spi_hal_iram (noflash)
        if HAL_SPI_SLAVE_FUNC_IN_IRAM = y:
            spi_slave_hal_iram (noflash)
-    if UART_ISR_IN_IRAM = y || ESP_PANIC_HANDLER_IRAM = y:
-        uart_hal_iram (noflash)
-    else:
-        uart_hal_iram (default)
    if SOC_LEDC_SUPPORTED = y:
        ledc_hal_iram (noflash)
    if SOC_I2C_SUPPORTED = y: