fix(rom): fixed systimer always using ROM implementation

components/esp_rom/CMakeLists.txt

@@ -122,6 +122,9 @@ if(BOOTLOADER_BUILD)
         if(CONFIG_HAL_WDT_USE_ROM_IMPL)
             rom_linker_script("wdt")
         endif()
+        if(CONFIG_HAL_SYSTIMER_USE_ROM_IMPL)
+            rom_linker_script("systimer")
+        endif()
         rom_linker_script("version")
 
     elseif(target STREQUAL "esp32h2")
@@ -129,12 +132,18 @@ if(BOOTLOADER_BUILD)
         if(CONFIG_HAL_WDT_USE_ROM_IMPL)
             rom_linker_script("wdt")
         endif()
+        if(CONFIG_HAL_SYSTIMER_USE_ROM_IMPL)
+            rom_linker_script("systimer")
+        endif()
 
     elseif(target STREQUAL "esp32p4")
         rom_linker_script("newlib")
         if(CONFIG_HAL_WDT_USE_ROM_IMPL)
             rom_linker_script("wdt")
         endif()
+        if(CONFIG_HAL_SYSTIMER_USE_ROM_IMPL)
+            rom_linker_script("systimer")
+        endif()
         rom_linker_script("version")
     endif()
 
@@ -257,6 +266,10 @@ else() # Regular app build
             rom_linker_script("wdt")
         endif()
 
+        if(CONFIG_HAL_SYSTIMER_USE_ROM_IMPL)
+            rom_linker_script("systimer")
+        endif()
+
         if(NOT CONFIG_NEWLIB_NANO_FORMAT)
             # Normal(Non-nano) formatting functions in ROM are also built for 64-bit time_t.
             rom_linker_script("newlib-normal")
@@ -274,6 +287,10 @@ else() # Regular app build
             rom_linker_script("wdt")
         endif()
 
+        if(CONFIG_HAL_SYSTIMER_USE_ROM_IMPL)
+            rom_linker_script("systimer")
+        endif()
+
         if(CONFIG_NEWLIB_NANO_FORMAT)
             # nano formatting functions in ROM are also built for 64-bit time_t.
             rom_linker_script("newlib-nano")
@@ -289,6 +306,10 @@ else() # Regular app build
             rom_linker_script("wdt")
         endif()
 
+        if(CONFIG_HAL_SYSTIMER_USE_ROM_IMPL)
+            rom_linker_script("systimer")
+        endif()
+
         if(CONFIG_NEWLIB_NANO_FORMAT)
             # nano formatting functions in ROM are also built for 64-bit time_t.
             rom_linker_script("newlib-nano")

components/esp_rom/esp32c6/ld/esp32c6.rom.ld

@@ -169,29 +169,6 @@ g_flash_guard_ops = 0x4087fff4;
 /* Note: esp_rom_spiflash_write_disable was moved from esp32c6.rom.spiflash.ld */
 esp_rom_spiflash_write_disable = 0x40000278;
 
-/***************************************
- Group hal_systimer
- ***************************************/
-
-/* Functions */
-/* The following ROM functions are commented out because they're patched in the esp_rom_systimer.c */
-/* systimer_hal_init = 0x400003c0; */
-/* systimer_hal_deinit = 0x400003c4; */
-
-systimer_hal_set_tick_rate_ops = 0x400003c8;
-systimer_hal_get_counter_value = 0x400003cc;
-systimer_hal_get_time = 0x400003d0;
-systimer_hal_set_alarm_target = 0x400003d4;
-systimer_hal_set_alarm_period = 0x400003d8;
-systimer_hal_get_alarm_value = 0x400003dc;
-systimer_hal_enable_alarm_int = 0x400003e0;
-systimer_hal_on_apb_freq_update = 0x400003e4;
-systimer_hal_counter_value_advance = 0x400003e8;
-systimer_hal_enable_counter = 0x400003ec;
-systimer_hal_select_alarm_mode = 0x400003f0;
-systimer_hal_connect_alarm_counter = 0x400003f4;
-systimer_hal_counter_can_stall_by_cpu = 0x400003f8;
-
 
 /***************************************
  Group cache

components/esp_rom/esp32c6/ld/esp32c6.rom.systimer.ld

@@ -0,0 +1,28 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/***************************************
+ Group hal_systimer
+ ***************************************/
+
+/* Functions */
+/* The following ROM functions are commented out because they're patched in the esp_rom_systimer.c */
+/* systimer_hal_init = 0x400003c0; */
+/* systimer_hal_deinit = 0x400003c4; */
+
+systimer_hal_set_tick_rate_ops = 0x400003c8;
+systimer_hal_get_counter_value = 0x400003cc;
+systimer_hal_get_time = 0x400003d0;
+systimer_hal_set_alarm_target = 0x400003d4;
+systimer_hal_set_alarm_period = 0x400003d8;
+systimer_hal_get_alarm_value = 0x400003dc;
+systimer_hal_enable_alarm_int = 0x400003e0;
+systimer_hal_on_apb_freq_update = 0x400003e4;
+systimer_hal_counter_value_advance = 0x400003e8;
+systimer_hal_enable_counter = 0x400003ec;
+systimer_hal_select_alarm_mode = 0x400003f0;
+systimer_hal_connect_alarm_counter = 0x400003f4;
+systimer_hal_counter_can_stall_by_cpu = 0x400003f8;

components/esp_rom/esp32h2/ld/esp32h2.rom.ld

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -160,30 +160,6 @@ g_flash_guard_ops = 0x4084fff4;
 /* Note: esp_rom_spiflash_write_disable was moved from esp32c6.rom.spiflash.ld */
 esp_rom_spiflash_write_disable = 0x40000270;
 
-/***************************************
- Group hal_systimer
- ***************************************/
-
-/* Functions */
-/* The following ROM functions are commented out because they're patched in the esp_rom_systimer.c */
-/* systimer_hal_init = 0x400003b8; */
-/* systimer_hal_deinit = 0x400003bc; */
-
-systimer_hal_set_tick_rate_ops = 0x400003c0;
-systimer_hal_get_counter_value = 0x400003c4;
-systimer_hal_get_time = 0x400003c8;
-systimer_hal_set_alarm_target = 0x400003cc;
-systimer_hal_set_alarm_period = 0x400003d0;
-systimer_hal_get_alarm_value = 0x400003d4;
-systimer_hal_enable_alarm_int = 0x400003d8;
-systimer_hal_on_apb_freq_update = 0x400003dc;
-systimer_hal_counter_value_advance = 0x400003e0;
-systimer_hal_enable_counter = 0x400003e4;
-systimer_hal_select_alarm_mode = 0x400003e8;
-systimer_hal_connect_alarm_counter = 0x400003ec;
-systimer_hal_counter_can_stall_by_cpu = 0x400003f0;
-
-
 /***************************************
  Group cache
  ***************************************/

components/esp_rom/esp32h2/ld/esp32h2.rom.systimer.ld

@@ -0,0 +1,28 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/***************************************
+ Group hal_systimer
+ ***************************************/
+
+/* Functions */
+/* The following ROM functions are commented out because they're patched in the esp_rom_systimer.c */
+/* systimer_hal_init = 0x400003b8; */
+/* systimer_hal_deinit = 0x400003bc; */
+
+systimer_hal_set_tick_rate_ops = 0x400003c0;
+systimer_hal_get_counter_value = 0x400003c4;
+systimer_hal_get_time = 0x400003c8;
+systimer_hal_set_alarm_target = 0x400003cc;
+systimer_hal_set_alarm_period = 0x400003d0;
+systimer_hal_get_alarm_value = 0x400003d4;
+systimer_hal_enable_alarm_int = 0x400003d8;
+systimer_hal_on_apb_freq_update = 0x400003dc;
+systimer_hal_counter_value_advance = 0x400003e0;
+systimer_hal_enable_counter = 0x400003e4;
+systimer_hal_select_alarm_mode = 0x400003e8;
+systimer_hal_connect_alarm_counter = 0x400003ec;
+systimer_hal_counter_can_stall_by_cpu = 0x400003f0;

components/esp_rom/esp32p4/ld/esp32p4.rom.ld

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -170,28 +170,6 @@ rom_spiflash_legacy_data = 0x4ff3ffe8;
 g_flash_guard_ops = 0x4ff3fff0;
 
 
-/***************************************
- Group hal_systimer
- ***************************************/
-
-/* Functions */
-systimer_hal_init = 0x4fc00228;
-systimer_hal_deinit = 0x4fc0022c;
-systimer_hal_set_tick_rate_ops = 0x4fc00230;
-systimer_hal_get_counter_value = 0x4fc00234;
-systimer_hal_get_time = 0x4fc00238;
-systimer_hal_set_alarm_target = 0x4fc0023c;
-systimer_hal_set_alarm_period = 0x4fc00240;
-systimer_hal_get_alarm_value = 0x4fc00244;
-systimer_hal_enable_alarm_int = 0x4fc00248;
-systimer_hal_on_apb_freq_update = 0x4fc0024c;
-systimer_hal_counter_value_advance = 0x4fc00250;
-systimer_hal_enable_counter = 0x4fc00254;
-systimer_hal_select_alarm_mode = 0x4fc00258;
-systimer_hal_connect_alarm_counter = 0x4fc0025c;
-systimer_hal_counter_can_stall_by_cpu = 0x4fc00260;
-
-
 /***************************************
  Group cache
  ***************************************/

components/esp_rom/esp32p4/ld/esp32p4.rom.systimer.ld

@@ -0,0 +1,28 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+
+/***************************************
+ Group hal_systimer
+ ***************************************/
+
+/* Functions */
+/* The following ROM functions are commented out because they're patched in the esp_rom_systimer.c */
+/* systimer_hal_init = 0x4fc00228; */
+/* systimer_hal_deinit = 0x4fc0022c; */
+systimer_hal_set_tick_rate_ops = 0x4fc00230;
+systimer_hal_get_counter_value = 0x4fc00234;
+systimer_hal_get_time = 0x4fc00238;
+systimer_hal_set_alarm_target = 0x4fc0023c;
+systimer_hal_set_alarm_period = 0x4fc00240;
+systimer_hal_get_alarm_value = 0x4fc00244;
+systimer_hal_enable_alarm_int = 0x4fc00248;
+systimer_hal_on_apb_freq_update = 0x4fc0024c;
+systimer_hal_counter_value_advance = 0x4fc00250;
+systimer_hal_enable_counter = 0x4fc00254;
+systimer_hal_select_alarm_mode = 0x4fc00258;
+systimer_hal_connect_alarm_counter = 0x4fc0025c;
+systimer_hal_counter_can_stall_by_cpu = 0x4fc00260;

components/esp_rom/test_apps/.build-test-rules.yml

@@ -1,6 +1,12 @@
 # Documentation: .gitlab/ci/README.md#manifest-file-to-control-the-buildtest-apps
 
-components/esp_rom/test_apps:
+components/esp_rom/test_apps/rom_impl_components:
+  disable:
+    # For ROM impl build tests, disable them if none of the tested features are supported in the ROM
+    - if: CONFIG_NAME == "rom_impl_components" and ((ESP_ROM_HAS_HAL_WDT != 1  and ESP_ROM_HAS_HAL_SYSTIMER != 1) and (ESP_ROM_HAS_HEAP_TLSF != 1 and ESP_ROM_HAS_SPI_FLASH != 1))
+    - if: CONFIG_NAME == "no_rom_impl_components" and ((ESP_ROM_HAS_HAL_WDT != 1 and ESP_ROM_HAS_HAL_SYSTIMER != 1) and (ESP_ROM_HAS_HEAP_TLSF != 1 and ESP_ROM_HAS_SPI_FLASH != 1))
+
+components/esp_rom/test_apps/rom_tests:
   disable_test:
     - if: IDF_TARGET in ["esp32", "esp32c2"]
       temporary: false

components/esp_rom/test_apps/rom_impl_components/CMakeLists.txt

@@ -0,0 +1,9 @@
+# This is the project CMakeLists.txt file for the test subproject
+cmake_minimum_required(VERSION 3.16)
+
+include($ENV{IDF_PATH}/tools/cmake/project.cmake)
+
+#"Trim" the build. Include the minimal set of components, main, and anything it depends on.
+set(COMPONENTS main)
+
+project(esp_rom_impl_components)

components/esp_rom/test_apps/rom_impl_components/README.md

@@ -0,0 +1,2 @@
+| Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C6 | ESP32-H2 | ESP32-P4 | ESP32-S2 | ESP32-S3 |
+| ----------------- | ----- | -------- | -------- | -------- | -------- | -------- | -------- | -------- |

components/esp_rom/test_apps/rom_impl_components/main/CMakeLists.txt

@@ -0,0 +1,7 @@
+set(srcs "test_app_main.c")
+
+# 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 ${srcs}
+                       PRIV_REQUIRES unity
+                       WHOLE_ARCHIVE)

components/esp_rom/test_apps/rom_impl_components/main/test_app_main.c

@@ -0,0 +1,107 @@
+/*
+ * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Unlicense OR CC0-1.0
+ */
+
+#include "unity.h"
+#include "unity_test_runner.h"
+#include "esp_heap_caps.h"
+#include "soc/soc_caps.h"
+#include "hal/wdt_hal.h"
+#include "hal/systimer_hal.h"
+
+static bool fn_in_rom(void *fn)
+{
+    const int fnaddr = (int)fn;
+    return (fnaddr >= SOC_IROM_MASK_LOW && fnaddr < SOC_IROM_MASK_HIGH);
+}
+
+#if CONFIG_HAL_WDT_USE_ROM_IMPL
+TEST_CASE("Test that WDT implementation from ROM is used", "[rom-impl-components]")
+{
+    TEST_ASSERT_TRUE(fn_in_rom(wdt_hal_feed));
+}
+
+#else
+TEST_CASE("Test that WDT implementation from ROM is NOT used", "[rom-impl-components]")
+{
+    TEST_ASSERT_FALSE(fn_in_rom(wdt_hal_feed));
+}
+#endif // CONFIG_HAL_WDT_USE_ROM_IMPL
+
+
+#if CONFIG_HAL_SYSTIMER_USE_ROM_IMPL
+TEST_CASE("Test that systimer implementation from ROM is used", "[rom-impl-components]")
+{
+    TEST_ASSERT_TRUE(fn_in_rom(systimer_hal_get_counter_value));
+}
+
+#else
+TEST_CASE("Test that systimer implementation from ROM is NOT used", "[rom-impl-components]")
+{
+    TEST_ASSERT_FALSE(fn_in_rom(systimer_hal_get_counter_value));
+}
+#endif // CONFIG_HAL_SYSTIMER_USE_ROM_IMPL
+
+extern uint32_t tlsf_create;
+#if CONFIG_HEAP_TLSF_USE_ROM_IMPL
+TEST_CASE("Test that HEAP implementation from ROM is used", "[rom-impl-components]")
+{
+    TEST_ASSERT_TRUE(fn_in_rom((&tlsf_create)));
+}
+
+#else
+TEST_CASE("Test that HEAP implementation from ROM is NOT used", "[rom-impl-components]")
+{
+    TEST_ASSERT_FALSE(fn_in_rom(&tlsf_create));
+}
+#endif // CONFIG_HEAP_TLSF_USE_ROM_IMPL
+
+
+extern uint32_t spi_flash_chip_generic_probe;
+#if CONFIG_SPI_FLASH_ROM_IMPL
+TEST_CASE("Test that SPI flash implementation from ROM is used", "[rom-impl-components]")
+{
+    TEST_ASSERT_TRUE(fn_in_rom(&spi_flash_chip_generic_probe));
+}
+
+#else
+TEST_CASE("Test that SPI flash implementation from ROM is NOT used", "[rom-impl-components]")
+{
+    TEST_ASSERT_FALSE(fn_in_rom(&spi_flash_chip_generic_probe));
+}
+#endif // CONFIG_SPI_FLASH_ROM_IMPL
+
+
+#define TEST_MEMORY_LEAK_THRESHOLD (-100)
+
+static size_t before_free_8bit;
+static size_t before_free_32bit;
+
+static void check_leak(size_t before_free, size_t after_free, const char *type)
+{
+    ssize_t delta = after_free - before_free;
+    printf("MALLOC_CAP_%s: Before %u bytes free, After %u bytes free (delta %d)\n", type, before_free, after_free, delta);
+    TEST_ASSERT_MESSAGE(delta >= TEST_MEMORY_LEAK_THRESHOLD, "memory leak");
+}
+
+void setUp(void)
+{
+    before_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT);
+    before_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT);
+}
+
+void tearDown(void)
+{
+    size_t after_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT);
+    size_t after_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT);
+    check_leak(before_free_8bit, after_free_8bit, "8BIT");
+    check_leak(before_free_32bit, after_free_32bit, "32BIT");
+}
+
+
+void app_main(void)
+{
+    unity_run_menu();
+}

components/esp_rom/test_apps/rom_impl_components/pytest_esp_rom_impl_components.py

@@ -0,0 +1,18 @@
+# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+# SPDX-License-Identifier: CC0-1.0
+import pytest
+from pytest_embedded import Dut
+
+
+@pytest.mark.supported_targets
+@pytest.mark.generic
+@pytest.mark.parametrize(
+    'config',
+    [
+        'no_rom_impl_components',
+        'rom_impl_components',
+    ],
+    indirect=True,
+)
+def test_esp_rom_impl_components(dut: Dut) -> None:
+    dut.run_all_single_board_cases()

components/esp_rom/test_apps/rom_impl_components/sdkconfig.defaults

@@ -0,0 +1 @@
+CONFIG_ESP_TASK_WDT_EN=n

tools/test_apps/.build-test-rules.yml

@@ -95,9 +95,6 @@ tools/test_apps/system/bootloader_sections:
 
 tools/test_apps/system/build_test:
   disable:
-    # For ROM impl build tests, disable them if none of the tested features are supported in the ROM
-    - if: CONFIG_NAME == "rom_impl_components" and ((ESP_ROM_HAS_HAL_WDT != 1  and ESP_ROM_HAS_HAL_SYSTIMER != 1) and (ESP_ROM_HAS_HEAP_TLSF != 1 and ESP_ROM_HAS_SPI_FLASH != 1))
-    - if: CONFIG_NAME == "no_rom_impl_components" and ((ESP_ROM_HAS_HAL_WDT != 1 and ESP_ROM_HAS_HAL_SYSTIMER != 1) and (ESP_ROM_HAS_HEAP_TLSF != 1 and ESP_ROM_HAS_SPI_FLASH != 1))
     - if: CONFIG_NAME == "no_rvfplib" and ESP_ROM_HAS_RVFPLIB != 1
     - if: CONFIG_NAME == "usb_serial_jtag" AND SOC_USB_SERIAL_JTAG_SUPPORTED != 1
     - if: CONFIG_NAME == "usb_console_ets_printf" AND SOC_USB_OTG_SUPPORTED != 1