Merge branch 'bugfix/gptimer_hal_placement_wrong_condition_v5.1' into 'release/v5.1'

fix(gptimer): hal function placement under wrong condition (v5.1)

See merge request espressif/esp-idf!25280

components/driver/CMakeLists.txt

@@ -216,7 +216,7 @@ else()
                         INCLUDE_DIRS ${includes}
                         PRIV_REQUIRES efuse esp_timer
                         REQUIRES esp_pm esp_ringbuf freertos soc hal esp_hw_support
-                        LDFRAGMENTS linker.lf)
+                        LDFRAGMENTS linker.lf gptimer/linker.lf)
 endif()
 
 # If system needs to monitor USJ connection status, then usb_serial_jtag_connection_monitor object file has to be linked

components/driver/Kconfig

@@ -306,37 +306,7 @@ menu "Driver Configurations"
                 Note that, this option only controls the Analog Comparator driver log, won't affect other drivers.
     endmenu # Analog Comparator Configuration
 
-    menu "GPTimer Configuration"
+    orsource "./gptimer/Kconfig.gptimer"
-        config GPTIMER_CTRL_FUNC_IN_IRAM
-            bool "Place GPTimer control functions into IRAM"
-            default n
-            help
-                Place GPTimer control functions (like start/stop) into IRAM,
-                so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context.
-                Enabling this option can improve driver performance as well.
-
-        config GPTIMER_ISR_IRAM_SAFE
-            bool "GPTimer ISR IRAM-Safe"
-            default n
-            help
-                Ensure the GPTimer interrupt is IRAM-Safe by allowing the interrupt handler to be
-                executable when the cache is disabled (e.g. SPI Flash write).
-
-        config GPTIMER_SUPPRESS_DEPRECATE_WARN
-            bool "Suppress legacy driver deprecated warning"
-            default n
-            help
-                Wether to suppress the deprecation warnings when using legacy timer group driver (driver/timer.h).
-                If you want to continue using the legacy driver, and don't want to see related deprecation warnings,
-                you can enable this option.
-
-        config GPTIMER_ENABLE_DEBUG_LOG
-            bool "Enable debug log"
-            default n
-            help
-                Wether to enable the debug log message for GPTimer driver.
-                Note that, this option only controls the GPTimer driver log, won't affect other drivers.
-    endmenu # GPTimer Configuration
 
     menu "PCNT Configuration"
         depends on SOC_PCNT_SUPPORTED

components/driver/gptimer/Kconfig.gptimer

@@ -0,0 +1,38 @@
+menu "GPTimer Configuration"
+    config GPTIMER_ISR_HANDLER_IN_IRAM
+        bool "Place GPTimer ISR handler into IRAM"
+        default y
+        help
+            Place GPTimer ISR handler into IRAM for better performance and fewer cache misses.
+
+    config GPTIMER_CTRL_FUNC_IN_IRAM
+        bool "Place GPTimer control functions into IRAM"
+        default n
+        help
+            Place GPTimer control functions (like start/stop) into IRAM,
+            so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context.
+            Enabling this option can improve driver performance as well.
+
+    config GPTIMER_ISR_IRAM_SAFE
+        bool "GPTimer ISR IRAM-Safe"
+        select GPTIMER_ISR_HANDLER_IN_IRAM
+        default n
+        help
+            Ensure the GPTimer interrupt is IRAM-Safe by allowing the interrupt handler to be
+            executable when the cache is disabled (e.g. SPI Flash write).
+
+    config GPTIMER_SUPPRESS_DEPRECATE_WARN
+        bool "Suppress legacy driver deprecated warning"
+        default n
+        help
+            Wether to suppress the deprecation warnings when using legacy timer group driver (driver/timer.h).
+            If you want to continue using the legacy driver, and don't want to see related deprecation warnings,
+            you can enable this option.
+
+    config GPTIMER_ENABLE_DEBUG_LOG
+        bool "Enable debug log"
+        default n
+        help
+            Wether to enable the debug log message for GPTimer driver.
+            Note that, this option only controls the GPTimer driver log, won't affect other drivers.
+endmenu # GPTimer Configuration

components/driver/gptimer/gptimer.c

@@ -480,8 +480,7 @@ static esp_err_t gptimer_select_periph_clock(gptimer_t *timer, gptimer_clock_sou
     return ESP_OK;
 }
 
-// Put the default ISR handler in the IRAM for better performance
+static void gptimer_default_isr(void *args)
-IRAM_ATTR static void gptimer_default_isr(void *args)
 {
     bool need_yield = false;
     gptimer_t *timer = (gptimer_t *)args;

components/driver/gptimer/linker.lf

@@ -0,0 +1,21 @@
+[mapping:gptimer_driver]
+archive: libdriver.a
+entries:
+    if GPTIMER_ISR_HANDLER_IN_IRAM = y:
+        gptimer: gptimer_default_isr (noflash)
+    if GPTIMER_CTRL_FUNC_IN_IRAM = y:
+        gptimer: gptimer_set_raw_count (noflash)
+        gptimer: gptimer_get_raw_count (noflash)
+        gptimer: gptimer_get_captured_count (noflash)
+        gptimer: gptimer_set_alarm_action (noflash)
+        gptimer: gptimer_start (noflash)
+        gptimer: gptimer_stop (noflash)
+
+[mapping:gptimer_hal]
+archive: libhal.a
+entries:
+    if GPTIMER_ISR_HANDLER_IN_IRAM = y:
+        timer_hal: timer_hal_capture_and_get_counter_value (noflash)
+    if GPTIMER_CTRL_FUNC_IN_IRAM = y:
+        timer_hal: timer_hal_set_counter_value (noflash)
+        timer_hal: timer_hal_capture_and_get_counter_value (noflash)

components/driver/linker.lf

@@ -1,13 +1,6 @@
 [mapping:driver]
 archive: libdriver.a
 entries:
-    if GPTIMER_CTRL_FUNC_IN_IRAM = y:
-        gptimer: gptimer_set_raw_count (noflash)
-        gptimer: gptimer_get_raw_count (noflash)
-        gptimer: gptimer_get_captured_count (noflash)
-        gptimer: gptimer_set_alarm_action (noflash)
-        gptimer: gptimer_start (noflash)
-        gptimer: gptimer_stop (noflash)
     if PCNT_CTRL_FUNC_IN_IRAM = y:
         pulse_cnt: pcnt_unit_start (noflash)
         pulse_cnt: pcnt_unit_stop (noflash)

components/hal/CMakeLists.txt

@@ -58,7 +58,7 @@ if(NOT BOOTLOADER_BUILD)
     endif()
 
     if(CONFIG_SOC_GPTIMER_SUPPORTED)
-        list(APPEND srcs "timer_hal.c" "timer_hal_iram.c")
+        list(APPEND srcs "timer_hal.c")
     endif()
 
     if(CONFIG_SOC_LEDC_SUPPORTED)

components/hal/esp32/include/hal/timer_ll.h

@@ -193,6 +193,7 @@ static inline void timer_ll_set_reload_value(timg_dev_t *hw, uint32_t timer_num,
  * @param timer_num Timer number in the group
  * @return reload count value
  */
+__attribute__((always_inline))
 static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_num)
 {
     return ((uint64_t)hw->hw_timer[timer_num].loadhi.tx_load_hi << 32) | (hw->hw_timer[timer_num].loadlo.tx_load_lo);
@@ -204,6 +205,7 @@ static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_
  * @param hw Timer Group register base address
  * @param timer_num Timer number in the group
  */
+__attribute__((always_inline))
 static inline void timer_ll_trigger_soft_reload(timg_dev_t *hw, uint32_t timer_num)
 {
     hw->hw_timer[timer_num].load.tx_load = 1;

components/hal/esp32c2/include/hal/timer_ll.h

@@ -193,6 +193,7 @@ static inline void timer_ll_set_reload_value(timg_dev_t *hw, uint32_t timer_num,
  * @param timer_num Timer number in the group
  * @return reload count value
  */
+__attribute__((always_inline))
 static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_num)
 {
     return ((uint64_t)hw->hw_timer[timer_num].loadhi.tx_load_hi << 32) | (hw->hw_timer[timer_num].loadlo.tx_load_lo);
@@ -204,6 +205,7 @@ static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_
  * @param hw Timer Group register base address
  * @param timer_num Timer number in the group
  */
+__attribute__((always_inline))
 static inline void timer_ll_trigger_soft_reload(timg_dev_t *hw, uint32_t timer_num)
 {
     hw->hw_timer[timer_num].load.tx_load = 1;

components/hal/esp32c3/include/hal/timer_ll.h

@@ -193,6 +193,7 @@ static inline void timer_ll_set_reload_value(timg_dev_t *hw, uint32_t timer_num,
  * @param timer_num Timer number in the group
  * @return reload count value
  */
+__attribute__((always_inline))
 static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_num)
 {
     return ((uint64_t)hw->hw_timer[timer_num].loadhi.tx_load_hi << 32) | (hw->hw_timer[timer_num].loadlo.tx_load_lo);
@@ -204,6 +205,7 @@ static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_
  * @param hw Timer Group register base address
  * @param timer_num Timer number in the group
  */
+__attribute__((always_inline))
 static inline void timer_ll_trigger_soft_reload(timg_dev_t *hw, uint32_t timer_num)
 {
     hw->hw_timer[timer_num].load.tx_load = 1;

components/hal/esp32c6/include/hal/timer_ll.h

@@ -235,6 +235,7 @@ static inline void timer_ll_set_reload_value(timg_dev_t *hw, uint32_t timer_num,
  * @param timer_num Timer number in the group
  * @return reload count value
  */
+__attribute__((always_inline))
 static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_num)
 {
     return ((uint64_t)hw->hw_timer[timer_num].loadhi.tx_load_hi << 32) | (hw->hw_timer[timer_num].loadlo.tx_load_lo);
@@ -246,6 +247,7 @@ static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_
  * @param hw Timer Group register base address
  * @param timer_num Timer number in the group
  */
+__attribute__((always_inline))
 static inline void timer_ll_trigger_soft_reload(timg_dev_t *hw, uint32_t timer_num)
 {
     hw->hw_timer[timer_num].load.tx_load = 1;

components/hal/esp32h2/include/hal/timer_ll.h

@@ -235,6 +235,7 @@ static inline void timer_ll_set_reload_value(timg_dev_t *hw, uint32_t timer_num,
  * @param timer_num Timer number in the group
  * @return reload count value
  */
+__attribute__((always_inline))
 static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_num)
 {
     return ((uint64_t)hw->hw_timer[timer_num].loadhi.tx_load_hi << 32) | (hw->hw_timer[timer_num].loadlo.tx_load_lo);
@@ -246,6 +247,7 @@ static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_
  * @param hw Timer Group register base address
  * @param timer_num Timer number in the group
  */
+__attribute__((always_inline))
 static inline void timer_ll_trigger_soft_reload(timg_dev_t *hw, uint32_t timer_num)
 {
     hw->hw_timer[timer_num].load.tx_load = 1;

components/hal/esp32s2/include/hal/timer_ll.h

@@ -197,6 +197,7 @@ static inline void timer_ll_set_reload_value(timg_dev_t *hw, uint32_t timer_num,
  * @param timer_num Timer number in the group
  * @return reload count value
  */
+__attribute__((always_inline))
 static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_num)
 {
     return ((uint64_t)hw->hw_timer[timer_num].loadhi.tx_load_hi << 32) | (hw->hw_timer[timer_num].loadlo.tx_load_lo);
@@ -208,6 +209,7 @@ static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_
  * @param hw Timer Group register base address
  * @param timer_num Timer number in the group
  */
+__attribute__((always_inline))
 static inline void timer_ll_trigger_soft_reload(timg_dev_t *hw, uint32_t timer_num)
 {
     hw->hw_timer[timer_num].load.tx_load = 1;

components/hal/esp32s3/include/hal/timer_ll.h

@@ -196,6 +196,7 @@ static inline void timer_ll_set_reload_value(timg_dev_t *hw, uint32_t timer_num,
  * @param timer_num Timer number in the group
  * @return reload count value
  */
+__attribute__((always_inline))
 static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_num)
 {
     return ((uint64_t)hw->hw_timer[timer_num].loadhi.tn_load_hi << 32) | (hw->hw_timer[timer_num].loadlo.tn_load_lo);
@@ -207,6 +208,7 @@ static inline uint64_t timer_ll_get_reload_value(timg_dev_t *hw, uint32_t timer_
  * @param hw Timer Group register base address
  * @param timer_num Timer number in the group
  */
+__attribute__((always_inline))
 static inline void timer_ll_trigger_soft_reload(timg_dev_t *hw, uint32_t timer_num)
 {
     hw->hw_timer[timer_num].load.tn_load = 1;

components/hal/linker.lf

@@ -30,8 +30,6 @@ entries:
         twai_hal_iram (noflash)
     if IDF_TARGET_ESP32 = n:
         spi_flash_hal_gpspi (noflash)
-    if GPTIMER_CTRL_FUNC_IN_IRAM = y:
-        timer_hal_iram (noflash)
     if GPIO_CTRL_FUNC_IN_IRAM = y:
         gpio_hal: gpio_hal_intr_disable (noflash)
     if LCD_RGB_ISR_IRAM_SAFE = y:

components/hal/timer_hal.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2021-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -38,3 +38,25 @@ void timer_hal_deinit(timer_hal_context_t *hal)
 #endif
     hal->dev = NULL;
 }
+
+void timer_hal_set_counter_value(timer_hal_context_t *hal, uint64_t load_val)
+{
+    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+    // - `timer_ll_set_reload_value()` will only indicate the `reload_value`
+    // - `timer_ll_set_reload_value()` + ``timer_ll_trigger_soft_reload()` can update the HW counter value by software
+    // Therefore, after updating the HW counter value, we need to restore the previous `reload_value`.
+    // Attention: The following process should be protected by a lock in the driver layer.
+    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+    // save current reload value
+    uint64_t old_reload = timer_ll_get_reload_value(hal->dev, hal->timer_id);
+    timer_ll_set_reload_value(hal->dev, hal->timer_id, load_val);
+    timer_ll_trigger_soft_reload(hal->dev, hal->timer_id);
+    // restore the previous reload value
+    timer_ll_set_reload_value(hal->dev, hal->timer_id, old_reload);
+}
+
+uint64_t timer_hal_capture_and_get_counter_value(timer_hal_context_t *hal)
+{
+    timer_ll_trigger_soft_capture(hal->dev, hal->timer_id);
+    return timer_ll_get_counter_value(hal->dev, hal->timer_id);
+}

components/hal/timer_hal_iram.c

@@ -1,30 +0,0 @@
-/*
- * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD
- *
- * SPDX-License-Identifier: Apache-2.0
- */
-
-#include "hal/timer_hal.h"
-#include "hal/timer_ll.h"
-
-void timer_hal_set_counter_value(timer_hal_context_t *hal, uint64_t load_val)
-{
-    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-    // - `timer_ll_set_reload_value()` will only indicate the `reload_value`
-    // - `timer_ll_set_reload_value()` + ``timer_ll_trigger_soft_reload()` can update the HW counter value by software
-    // Therefore, after updating the HW counter value, we need to restore the previous `reload_value`.
-    // Attention: The following process should be protected by a lock in the driver layer.
-    //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-    // save current reload value
-    uint64_t old_reload = timer_ll_get_reload_value(hal->dev, hal->timer_id);
-    timer_ll_set_reload_value(hal->dev, hal->timer_id, load_val);
-    timer_ll_trigger_soft_reload(hal->dev, hal->timer_id);
-    // restore the previous reload value
-    timer_ll_set_reload_value(hal->dev, hal->timer_id, old_reload);
-}
-
-uint64_t timer_hal_capture_and_get_counter_value(timer_hal_context_t *hal)
-{
-    timer_ll_trigger_soft_capture(hal->dev, hal->timer_id);
-    return timer_ll_get_counter_value(hal->dev, hal->timer_id);
-}

docs/en/api-reference/peripherals/gptimer.rst

@@ -327,8 +327,9 @@ All the APIs provided by the driver are guaranteed to be thread safe, which mean
 Kconfig Options
 ^^^^^^^^^^^^^^^
 
-- :ref:`CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM` controls where to place the GPTimer control functions (IRAM or flash), see Section :ref:`gptimer-iram-safe` for more information.
+- :ref:`CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM` controls where to place the GPTimer control functions (IRAM or flash).
-- :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` controls whether the default ISR handler can work when the cache is disabled, see Section :ref:`gptimer-iram-safe` for more information.
+- :ref:`CONFIG_GPTIMER_ISR_HANDLER_IN_IRAM` controls where to place the GPTimer ISR handler (IRAM or flash).
+- :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` controls whether the default ISR handler should be masked when the cache is disabled, see Section :ref:`gptimer-iram-safe` for more information.
 - :ref:`CONFIG_GPTIMER_ENABLE_DEBUG_LOG` is used to enabled the debug log output. Enable this option will increase the firmware binary size.
 
 Application Examples

docs/zh_CN/api-reference/peripherals/gptimer.rst

@@ -327,8 +327,9 @@ IRAM 安全
 Kconfig 选项
 ^^^^^^^^^^^^^^^^^^^^^^
 
-- :ref:`CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM` 控制放置通用定时器控制函数（IRAM 或 flash）的位置。了解更多信息，请参考章节 :ref:`gptimer-iram-safe`。
+- :ref:`CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM` 控制着定时器控制函数的存放位置（IRAM 或 flash）。
-- :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` 控制默认 ISR 程序在 cache 禁用时是否可以运行。了解更多信息，请参考章节 :ref:`gptimer-iram-safe`。
+- :ref:`CONFIG_GPTIMER_ISR_HANDLER_IN_IRAM` 控制着定时器中断处理函数的存放位置（IRAM 或 flash）。
+- :ref:`CONFIG_GPTIMER_ISR_IRAM_SAFE` 控制着中断处理函数是否需要在 cache 关闭的时候被屏蔽掉。更多信息，请参阅 :ref:`gptimer-iram-safe`。
 - :ref:`CONFIG_GPTIMER_ENABLE_DEBUG_LOG` 用于启用调试日志输出。启用这一选项将增加固件二进制文件大小。
 
 应用示例