From 76983e48fc45bdd9d43c308ca229e077e30bfb99 Mon Sep 17 00:00:00 2001 From: Marius Vikhammer Date: Tue, 21 Apr 2026 12:43:49 +0800 Subject: [PATCH] fix(hal): fix LP WDT stage timeout and reset configuration for ESP32S31 --- .../esp32s31/include/hal/rwdt_ll.h | 25 +++++++------------ components/esp_hal_wdt/esp32s31/rom.wdt.ld | 6 ++--- components/esp_hal_wdt/rom_patch.c | 15 ++++------- .../main/test_rtc_wdt.c | 19 +++++++++++--- .../esp_rom/esp32p4/Kconfig.soc_caps.in | 4 +++ components/esp_rom/esp32p4/esp_rom_caps.h | 1 + .../esp_rom/esp32s31/Kconfig.soc_caps.in | 4 +++ components/esp_rom/esp32s31/esp_rom_caps.h | 1 + 8 files changed, 43 insertions(+), 32 deletions(-) diff --git a/components/esp_hal_wdt/esp32s31/include/hal/rwdt_ll.h b/components/esp_hal_wdt/esp32s31/include/hal/rwdt_ll.h index f4355fcbf08..fd64712c0aa 100644 --- a/components/esp_hal_wdt/esp32s31/include/hal/rwdt_ll.h +++ b/components/esp_hal_wdt/esp32s31/include/hal/rwdt_ll.h @@ -21,8 +21,6 @@ extern "C" { #include "esp32s31/rom/ets_sys.h" -// TODO: ["ESP32S31"] IDF-14636 - /* The value that needs to be written to RTC_WDT_WPROTECT_REG to write-enable the wdt registers */ #define RTC_WDT_WKEY_VALUE 0x50D83AA1 /* The value that needs to be written to RTC_WDT_SWD_WPROTECT_REG to write-enable the swd registers */ @@ -238,28 +236,23 @@ FORCE_INLINE_ATTR void rwdt_ll_set_pause_in_sleep_en(rwdt_dev_t *hw, bool enable } /** - * @brief Enable/Disable chip reset on RWDT timeout. - * - * A chip reset also resets the analog portion of the chip. It will appear as a - * POWERON reset rather than an RTC reset. - * - * @param hw Start address of the peripheral registers. - * @param enable True to enable, false to disable. + * @brief No register for setting chip_reset_en, we keep an empty function to + * provide the same HAL interface as other targets. */ FORCE_INLINE_ATTR void rwdt_ll_set_chip_reset_en(rwdt_dev_t *hw, bool enable) { - // hw->config0.wdt_chip_reset_en = (enable) ? 1 : 0; + (void)hw; + (void)enable; } /** - * @brief Set width of chip reset signal - * - * @param hw Start address of the peripheral registers. - * @param width Width of chip reset signal in terms of number of RTC_SLOW_CLK cycles + * @brief No register for setting reset width, we keep an empty function to + * provide the same HAL interface as other targets. */ FORCE_INLINE_ATTR void rwdt_ll_set_chip_reset_width(rwdt_dev_t *hw, uint32_t width) { - // HAL_FORCE_MODIFY_U32_REG_FIELD(hw->config0, wdt_chip_reset_width, width); + (void)hw; + (void)width; } /** @@ -271,7 +264,7 @@ FORCE_INLINE_ATTR void rwdt_ll_set_chip_reset_width(rwdt_dev_t *hw, uint32_t wid */ FORCE_INLINE_ATTR void rwdt_ll_feed(rwdt_dev_t *hw) { - // hw->feed.rtc_wdt_feed = 1; + hw->feed.feed = 1; } /** diff --git a/components/esp_hal_wdt/esp32s31/rom.wdt.ld b/components/esp_hal_wdt/esp32s31/rom.wdt.ld index 84e0754940d..baf7cbc43e1 100644 --- a/components/esp_hal_wdt/esp32s31/rom.wdt.ld +++ b/components/esp_hal_wdt/esp32s31/rom.wdt.ld @@ -18,9 +18,9 @@ ***************************************/ /* Functions */ -wdt_hal_init = 0x2f800368; -wdt_hal_deinit = 0x2f80036c; -wdt_hal_config_stage = 0x2f800370; +rom_wdt_hal_init = 0x2f800368; +rom_wdt_hal_deinit = 0x2f80036c; +rom_wdt_hal_config_stage = 0x2f800370; wdt_hal_write_protect_disable = 0x2f800374; wdt_hal_write_protect_enable = 0x2f800378; wdt_hal_enable = 0x2f80037c; diff --git a/components/esp_hal_wdt/rom_patch.c b/components/esp_hal_wdt/rom_patch.c index c883e1c81af..4bbe42b512d 100644 --- a/components/esp_hal_wdt/rom_patch.c +++ b/components/esp_hal_wdt/rom_patch.c @@ -5,11 +5,7 @@ */ #include -#include "hal/config.h" -#include "soc/soc_caps.h" -#include "soc/chip_revision.h" #include "esp_rom_caps.h" -#include "hal/efuse_hal.h" #include "hal/mwdt_periph.h" #include "hal/wdt_hal.h" @@ -41,16 +37,15 @@ void wdt_hal_deinit(wdt_hal_context_t *hal) rom_wdt_hal_deinit(hal); } -#if SOC_IS(ESP32P4) && (HAL_CONFIG(CHIP_SUPPORT_MIN_REV) <= 301) +#if ESP_ROM_WDT_CONFIG_STAGE_PATCH extern void rom_wdt_hal_config_stage(wdt_hal_context_t *hal, wdt_stage_t stage, uint32_t timeout, wdt_stage_action_t behavior); -/* rwdt_ll_config_stage is implemented erroneously in ESP32P4 rom code, TODO: PM-654*/ void wdt_hal_config_stage(wdt_hal_context_t *hal, wdt_stage_t stage, uint32_t timeout_ticks, wdt_stage_action_t behavior) { - if ((hal->inst == WDT_RWDT && stage == WDT_STAGE0) && !ESP_CHIP_REV_ABOVE(efuse_hal_chip_revision(), 302)) { - timeout_ticks = timeout_ticks >> (1 + REG_GET_FIELD(EFUSE_RD_REPEAT_DATA1_REG, EFUSE_WDT_DELAY_SEL)); - } rom_wdt_hal_config_stage(hal, stage, timeout_ticks, behavior); + if (hal->inst == WDT_RWDT) { + rwdt_ll_config_stage(hal->rwdt_dev, stage, timeout_ticks, behavior); + } } -#endif // SOC_IS(ESP32P4) +#endif // ESP_ROM_WDT_CONFIG_STAGE_PATCH #endif // ESP_ROM_WDT_INIT_PATCH diff --git a/components/esp_hw_support/test_apps/esp_hw_support_unity_tests/main/test_rtc_wdt.c b/components/esp_hw_support/test_apps/esp_hw_support_unity_tests/main/test_rtc_wdt.c index 6b05e3d70f8..28372a9e553 100644 --- a/components/esp_hw_support/test_apps/esp_hw_support_unity_tests/main/test_rtc_wdt.c +++ b/components/esp_hw_support/test_apps/esp_hw_support_unity_tests/main/test_rtc_wdt.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -126,14 +126,27 @@ TEST_CASE("RTC WDT triggers interrupt at expected time for all stages", "[rtc_wd esp_intr_free(ret_handle); #endif - /* Check that wdt interrupts for various stages occurred within a reasonable window (WDT_TIMEOUT_MARGIN_MS) */ + /* Check that wdt interrupts for various stages occurred within a reasonable window (WDT_TIMEOUT_MARGIN_MS) + * Do not abort on first failure: print all stage results and report aggregate failure at the end so later + * stage timings can be inspected even if earlier stages are out of tolerance. + */ int64_t elapsed_us[4], expected_us[4]; + int failures = 0; for (stage = WDT_STAGE0; stage <= WDT_STAGE3; stage++) { elapsed_us[stage] = wdt_stage_int_time[stage] - wdt_start_time[stage]; expected_us[stage] = wdt_stage_timeout_ms[stage] * 1000; printf("Stage-%d: interrupt time %lld us, start time %lld us\n", stage, wdt_stage_int_time[stage], wdt_start_time[stage]); printf("Stage-%d: expected %lld us, elapsed %lld us\n", stage, expected_us[stage], elapsed_us[stage]); - TEST_ASSERT_INT_WITHIN(WDT_TIMEOUT_MARGIN_MS(wdt_stage_timeout_ms[stage]) * 1000, expected_us[stage], elapsed_us[stage]); + int64_t margin_us = (int64_t)WDT_TIMEOUT_MARGIN_MS(wdt_stage_timeout_ms[stage]) * 1000; + if (llabs(expected_us[stage] - elapsed_us[stage]) > margin_us) { + failures++; + printf("Stage-%d: OUT OF TOLERANCE (expected %lld +/- %lld, got %lld)\n", stage, expected_us[stage], margin_us, elapsed_us[stage]); + } else { + printf("Stage-%d: within tolerance\n", stage); + } + } + if (failures) { + TEST_FAIL_MESSAGE("One or more WDT stages timed out outside allowed margin. See logs for details."); } } diff --git a/components/esp_rom/esp32p4/Kconfig.soc_caps.in b/components/esp_rom/esp32p4/Kconfig.soc_caps.in index c6c24fd4717..d3ebf6be187 100644 --- a/components/esp_rom/esp32p4/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32p4/Kconfig.soc_caps.in @@ -51,6 +51,10 @@ config ESP_ROM_WDT_INIT_PATCH bool default y +config ESP_ROM_WDT_CONFIG_STAGE_PATCH + bool + default y + config ESP_ROM_HAS_LP_ROM bool default y diff --git a/components/esp_rom/esp32p4/esp_rom_caps.h b/components/esp_rom/esp32p4/esp_rom_caps.h index d58a865a04a..cbe4b3f48a5 100644 --- a/components/esp_rom/esp32p4/esp_rom_caps.h +++ b/components/esp_rom/esp32p4/esp_rom_caps.h @@ -18,6 +18,7 @@ #define ESP_ROM_HAS_HAL_SYSTIMER (1) // ROM has the implementation of Systimer HAL driver #define ESP_ROM_HAS_LAYOUT_TABLE (1) // ROM has the layout table #define ESP_ROM_WDT_INIT_PATCH (1) // ROM version does not configure the clock +#define ESP_ROM_WDT_CONFIG_STAGE_PATCH (1) // ROM rwdt_ll_config_stage is implemented erroneously on old silicon #define ESP_ROM_HAS_LP_ROM (1) // ROM also has a LP ROM placed in LP memory #define ESP_ROM_HAS_NEWLIB (1) // ROM has newlib (at least parts of it) functions included #define ESP_ROM_HAS_NEWLIB_NANO_FORMAT (1) // ROM has the newlib nano version of formatting functions diff --git a/components/esp_rom/esp32s31/Kconfig.soc_caps.in b/components/esp_rom/esp32s31/Kconfig.soc_caps.in index 0db16c7ce01..b438c9f900d 100644 --- a/components/esp_rom/esp32s31/Kconfig.soc_caps.in +++ b/components/esp_rom/esp32s31/Kconfig.soc_caps.in @@ -67,6 +67,10 @@ config ESP_ROM_WDT_INIT_PATCH bool default y +config ESP_ROM_WDT_CONFIG_STAGE_PATCH + bool + default y + config ESP_ROM_HAS_NEWLIB bool default y diff --git a/components/esp_rom/esp32s31/esp_rom_caps.h b/components/esp_rom/esp32s31/esp_rom_caps.h index ec8b12429b5..9b2d20575ea 100644 --- a/components/esp_rom/esp32s31/esp_rom_caps.h +++ b/components/esp_rom/esp32s31/esp_rom_caps.h @@ -22,6 +22,7 @@ #define ESP_ROM_MULTI_HEAP_WALK_PATCH (1) // ROM does not contain the patch of multi_heap_walk() #define ESP_ROM_HAS_LAYOUT_TABLE (1) // ROM has the layout table #define ESP_ROM_WDT_INIT_PATCH (1) // ROM version does not configure the clock +#define ESP_ROM_WDT_CONFIG_STAGE_PATCH (1) // ROM rwdt_ll_config_stage is implemented erroneously #define ESP_ROM_HAS_HEAP_TLSF (1) // ROM has the implementation of the tlsf and multi-heap library #define ESP_ROM_HAS_NEWLIB (1) // ROM has newlib (at least parts of it) functions included #define ESP_ROM_HAS_NEWLIB_NANO_FORMAT (1) // ROM has the newlib nano version of formatting functions