From b62679a1755558b6f86bdbafc1437f187249cc9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 26 Aug 2025 16:47:57 +0200 Subject: [PATCH] WDT cleanup and fix tests (#3992) * Configure WDTs while disabled, clean up * Wait for less time, disable WDT after test --- esp-hal/src/lib.rs | 78 +++++++---------- esp-hal/src/rtc_cntl/mod.rs | 124 ++++++++++++++++------------ esp-hal/src/rtc_cntl/rtc/esp32c6.rs | 55 +----------- esp-hal/src/rtc_cntl/rtc/esp32h2.rs | 29 +------ hil-test/tests/init.rs | 39 ++++++--- 5 files changed, 131 insertions(+), 194 deletions(-) diff --git a/esp-hal/src/lib.rs b/esp-hal/src/lib.rs index 31bd03177..fed80b49c 100644 --- a/esp-hal/src/lib.rs +++ b/esp-hal/src/lib.rs @@ -643,62 +643,42 @@ pub fn init(config: Config) -> Peripherals { let mut rtc = crate::rtc_cntl::Rtc::new(peripherals.LPWR.reborrow()); // Handle watchdog configuration with defaults - cfg_if::cfg_if! { - if #[cfg(feature = "unstable")] - { - #[cfg(not(any(esp32, esp32s2)))] - if config.watchdog.swd() { - rtc.swd.enable(); - } else { - rtc.swd.disable(); - } + #[cfg(not(any(esp32, esp32s2)))] + rtc.swd.disable(); - match config.watchdog.rwdt() { - WatchdogStatus::Enabled(duration) => { - rtc.rwdt.set_timeout(crate::rtc_cntl::RwdtStage::Stage0, duration); - rtc.rwdt.enable(); - } - WatchdogStatus::Disabled => { - rtc.rwdt.disable(); - } - } + rtc.rwdt.disable(); - #[cfg(timergroup_timg0)] - match config.watchdog.timg0() { - WatchdogStatus::Enabled(duration) => { - let mut timg0_wd = crate::timer::timg::Wdt::>::new(); - timg0_wd.set_timeout(crate::timer::timg::MwdtStage::Stage0, duration); - timg0_wd.enable(); - } - WatchdogStatus::Disabled => { - crate::timer::timg::Wdt::>::new().disable(); - } - } + #[cfg(timergroup_timg0)] + crate::timer::timg::Wdt::>::new().disable(); - #[cfg(timergroup_timg1)] - match config.watchdog.timg1() { - WatchdogStatus::Enabled(duration) => { - let mut timg1_wd = crate::timer::timg::Wdt::>::new(); - timg1_wd.set_timeout(crate::timer::timg::MwdtStage::Stage0, duration); - timg1_wd.enable(); - } - WatchdogStatus::Disabled => { - crate::timer::timg::Wdt::>::new().disable(); - } - } + #[cfg(timergroup_timg1)] + crate::timer::timg::Wdt::>::new().disable(); + + #[cfg(feature = "unstable")] + { + #[cfg(not(any(esp32, esp32s2)))] + if config.watchdog.swd() { + rtc.swd.enable(); } - else - { - #[cfg(not(any(esp32, esp32s2)))] - rtc.swd.disable(); - rtc.rwdt.disable(); + if let WatchdogStatus::Enabled(duration) = config.watchdog.rwdt() { + rtc.rwdt + .set_timeout(crate::rtc_cntl::RwdtStage::Stage0, duration); + rtc.rwdt.enable(); + } - #[cfg(timergroup_timg0)] - crate::timer::timg::Wdt::>::new().disable(); + #[cfg(timergroup_timg0)] + if let WatchdogStatus::Enabled(duration) = config.watchdog.timg0() { + let mut timg0_wd = crate::timer::timg::Wdt::>::new(); + timg0_wd.set_timeout(crate::timer::timg::MwdtStage::Stage0, duration); + timg0_wd.enable(); + } - #[cfg(timergroup_timg1)] - crate::timer::timg::Wdt::>::new().disable(); + #[cfg(timergroup_timg1)] + if let WatchdogStatus::Enabled(duration) = config.watchdog.timg1() { + let mut timg1_wd = crate::timer::timg::Wdt::>::new(); + timg1_wd.set_timeout(crate::timer::timg::MwdtStage::Stage0, duration); + timg1_wd.enable(); } } diff --git a/esp-hal/src/rtc_cntl/mod.rs b/esp-hal/src/rtc_cntl/mod.rs index 78707a010..aebfca1b1 100644 --- a/esp-hal/src/rtc_cntl/mod.rs +++ b/esp-hal/src/rtc_cntl/mod.rs @@ -144,6 +144,8 @@ cfg_if::cfg_if! { use crate::peripherals::LP_WDT; use crate::peripherals::LP_TIMER; use crate::peripherals::LP_AON; + + use rtc::{RtcSlowClock, RtcCalSel}; // TODO: bring the types into this module } else { use crate::peripherals::LPWR as LP_WDT; use crate::peripherals::LPWR as LP_TIMER; @@ -585,9 +587,7 @@ impl RtcClock { /// Get the RTC_SLOW_CLK source. #[cfg(not(any(esp32c6, esp32h2)))] pub fn slow_freq() -> RtcSlowClock { - let rtc_cntl = LPWR::regs(); - let slow_freq = rtc_cntl.clk_conf().read().ana_clk_rtc_sel().bits(); - match slow_freq { + match LPWR::regs().clk_conf().read().ana_clk_rtc_sel().bits() { 0 => RtcSlowClock::RtcSlowClockRtc, 1 => RtcSlowClock::RtcSlowClock32kXtal, 2 => RtcSlowClock::RtcSlowClock8mD256, @@ -598,22 +598,22 @@ impl RtcClock { /// Select source for RTC_SLOW_CLK. #[cfg(not(any(esp32c6, esp32h2)))] fn set_slow_freq(slow_freq: RtcSlowClock) { - unsafe { - let rtc_cntl = LPWR::regs(); - rtc_cntl.clk_conf().modify(|_, w| { - w.ana_clk_rtc_sel() - .bits(slow_freq as u8) - // Why we need to connect this clock to digital? - // Or maybe this clock should be connected to digital when - // XTAL 32k clock is enabled instead? - .dig_xtal32k_en() - .bit(matches!(slow_freq, RtcSlowClock::RtcSlowClock32kXtal)) - // The clk_8m_d256 will be closed when rtc_state in SLEEP, - // so if the slow_clk is 8md256, clk_8m must be force power on - .ck8m_force_pu() - .bit(matches!(slow_freq, RtcSlowClock::RtcSlowClock8mD256)) - }); - }; + LPWR::regs().clk_conf().modify(|_, w| { + unsafe { + w.ana_clk_rtc_sel().bits(slow_freq as u8); + } + + // Why we need to connect this clock to digital? + // Or maybe this clock should be connected to digital when + // XTAL 32k clock is enabled instead? + w.dig_xtal32k_en() + .bit(matches!(slow_freq, RtcSlowClock::RtcSlowClock32kXtal)); + + // The clk_8m_d256 will be closed when rtc_state in SLEEP, + // so if the slow_clk is 8md256, clk_8m must be force power on + w.ck8m_force_pu() + .bit(matches!(slow_freq, RtcSlowClock::RtcSlowClock8mD256)) + }); crate::rom::ets_delay_us(300u32); } @@ -621,8 +621,7 @@ impl RtcClock { /// Select source for RTC_FAST_CLK. #[cfg(not(any(esp32c6, esp32h2)))] fn set_fast_freq(fast_freq: RtcFastClock) { - let rtc_cntl = LPWR::regs(); - rtc_cntl.clk_conf().modify(|_, w| { + LPWR::regs().clk_conf().modify(|_, w| { w.fast_clk_rtc_sel().bit(match fast_freq { RtcFastClock::RtcFastClock8m => true, RtcFastClock::RtcFastClockXtalD4 => false, @@ -635,7 +634,7 @@ impl RtcClock { /// Calibration of RTC_SLOW_CLK is performed using a special feature of /// TIMG0. This feature counts the number of XTAL clock cycles within a /// given number of RTC_SLOW_CLK cycles. - #[cfg(not(any(esp32c6, esp32h2)))] + #[cfg(not(any(esp32c6, esp32h2)))] // TODO: merge with C6/H2 impl fn calibrate_internal(cal_clk: RtcCalSel, slowclk_cycles: u32) -> u32 { // Except for ESP32, choosing RTC_CAL_RTC_MUX results in calibration of // the 150k RTC clock (90k on ESP32-S2) regardless of the currently selected @@ -656,18 +655,19 @@ impl RtcClock { // Enable requested clock (150k clock is always on) let dig_32k_xtal_enabled = rtc_cntl.clk_conf().read().dig_xtal32k_en().bit_is_set(); + let dig_clk8m_d256_enabled = rtc_cntl.clk_conf().read().dig_clk8m_d256_en().bit_is_set(); - if matches!(cal_clk, RtcCalSel::RtcCal32kXtal) && !dig_32k_xtal_enabled { - rtc_cntl - .clk_conf() - .modify(|_, w| w.dig_xtal32k_en().set_bit()); - } + rtc_cntl.clk_conf().modify(|_, w| { + if matches!(cal_clk, RtcCalSel::RtcCal32kXtal) { + w.dig_xtal32k_en().set_bit(); + } - if matches!(cal_clk, RtcCalSel::RtcCal8mD256) { - rtc_cntl - .clk_conf() - .modify(|_, w| w.dig_clk8m_d256_en().set_bit()); - } + if matches!(cal_clk, RtcCalSel::RtcCal8mD256) { + w.dig_clk8m_d256_en().set_bit(); + } + + w + }); // There may be another calibration process already running during we // call this function, so we should wait the last process is done. @@ -760,29 +760,34 @@ impl RtcClock { timg0 .rtccalicfg() .modify(|_, w| w.rtc_cali_start().clear_bit()); - rtc_cntl - .clk_conf() - .modify(|_, w| w.dig_xtal32k_en().bit(dig_32k_xtal_enabled)); - - if matches!(cal_clk, RtcCalSel::RtcCal8mD256) { - rtc_cntl - .clk_conf() - .modify(|_, w| w.dig_clk8m_d256_en().clear_bit()); - } + rtc_cntl.clk_conf().modify(|_, w| { + w.dig_xtal32k_en().bit(dig_32k_xtal_enabled); + w.dig_clk8m_d256_en().bit(dig_clk8m_d256_enabled) + }); cal_val } - /// Measure RTC slow clock's period, based on main XTAL frequency. + /// Measure RTC slow clock's period, based on main XTAL frequency /// /// This function will time out and return 0 if the time for the given /// number of cycles to be counted exceeds the expected time twice. This /// may happen if 32k XTAL is being calibrated, but the oscillator has /// not started up (due to incorrect loading capacitance, board design /// issue, or lack of 32 XTAL on board). - #[cfg(not(any(esp32c6, esp32h2)))] fn calibrate(cal_clk: RtcCalSel, slowclk_cycles: u32) -> u32 { let xtal_freq = RtcClock::xtal_freq(); + + #[cfg(esp32c6)] + let slowclk_cycles = if Efuse::chip_revision() > 0 && cal_clk == RtcCalSel::RtcCalRcFast { + // The Fosc CLK of calibration circuit is divided by 32 for ECO1. + // So we need to divide the calibrate cycles of the FOSC for ECO1 and above + // chips by 32 to avoid excessive calibration time. + slowclk_cycles >> 5 + } else { + slowclk_cycles + }; + let xtal_cycles = RtcClock::calibrate_internal(cal_clk, slowclk_cycles) as u64; let divider = xtal_freq.mhz() as u64 * slowclk_cycles as u64; let period_64 = ((xtal_cycles << RtcClock::CAL_FRACT) + divider / 2u64 - 1u64) / divider; @@ -791,17 +796,27 @@ impl RtcClock { } /// Calculate the necessary RTC_SLOW_CLK cycles to complete 1 millisecond. - #[cfg(not(any(esp32c6, esp32h2)))] - fn cycles_to_1ms() -> u16 { - let period_13q19 = RtcClock::calibrate( - match RtcClock::slow_freq() { - RtcSlowClock::RtcSlowClockRtc => RtcCalSel::RtcCalRtcMux, - RtcSlowClock::RtcSlowClock32kXtal => RtcCalSel::RtcCal32kXtal, - #[cfg(not(any(esp32c6, esp32h2)))] - RtcSlowClock::RtcSlowClock8mD256 => RtcCalSel::RtcCal8mD256, - }, - 1024, - ); + pub(crate) fn cycles_to_1ms() -> u16 { + cfg_if::cfg_if! { + if #[cfg(any(esp32c6, esp32h2))] { + let calibration_clock = match RtcClock::slow_freq() { + RtcSlowClock::RtcSlowClockRcSlow => RtcCalSel::RtcCalRtcMux, + RtcSlowClock::RtcSlowClock32kXtal => RtcCalSel::RtcCal32kXtal, + RtcSlowClock::RtcSlowClock32kRc => RtcCalSel::RtcCal32kRc, + RtcSlowClock::RtcSlowOscSlow => RtcCalSel::RtcCal32kOscSlow, + // RtcSlowClock::RtcCalRcFast => RtcCalSel::RtcCalRcFast, + }; + } else { + let calibration_clock = match RtcClock::slow_freq() { + RtcSlowClock::RtcSlowClockRtc => RtcCalSel::RtcCalRtcMux, + RtcSlowClock::RtcSlowClock32kXtal => RtcCalSel::RtcCal32kXtal, + RtcSlowClock::RtcSlowClock8mD256 => RtcCalSel::RtcCal8mD256, + }; + } + } + + // TODO: store the result somewhere + let period_13q19 = RtcClock::calibrate(calibration_clock, 1024); // 100_000_000 is used to get rid of `float` calculations let period = (100_000_000 * period_13q19 as u64) / (1 << RtcClock::CAL_FRACT); @@ -811,6 +826,7 @@ impl RtcClock { /// Return estimated XTAL frequency in MHz. pub(crate) fn estimate_xtal_frequency() -> u32 { + // TODO: this could reuse Self::calibrate_internal const SLOW_CLOCK_CYCLES: u32 = 100; cfg_if::cfg_if! { diff --git a/esp-hal/src/rtc_cntl/rtc/esp32c6.rs b/esp-hal/src/rtc_cntl/rtc/esp32c6.rs index 4220dc7da..521bb7375 100644 --- a/esp-hal/src/rtc_cntl/rtc/esp32c6.rs +++ b/esp-hal/src/rtc_cntl/rtc/esp32c6.rs @@ -1522,14 +1522,10 @@ impl RtcClock { // 32 times. And ensure that this modification will not affect // ECO0. // https://github.com/espressif/esp-idf/commit/e3148369f32fdc6de62c35a67f7adb6f4faef4e3 - if Efuse::chip_revision() > 0 { - if cal_clk == RtcCalSel::RtcCalRcFast { - break timg0.rtccalicfg1().read().rtc_cali_value().bits() >> 5; - } - break timg0.rtccalicfg1().read().rtc_cali_value().bits(); - } else { - break timg0.rtccalicfg1().read().rtc_cali_value().bits(); + if Efuse::chip_revision() > 0 && cal_clk == RtcCalSel::RtcCalRcFast { + break timg0.rtccalicfg1().read().rtc_cali_value().bits() >> 5; } + break timg0.rtccalicfg1().read().rtc_cali_value().bits(); } if timg0.rtccalicfg2().read().rtc_cali_timeout().bit_is_set() { @@ -1580,51 +1576,6 @@ impl RtcClock { cal_val } - - /// Measure RTC slow clock's period, based on main XTAL frequency - /// - /// This function will time out and return 0 if the time for the given - /// number of cycles to be counted exceeds the expected time twice. This - /// may happen if 32k XTAL is being calibrated, but the oscillator has - /// not started up (due to incorrect loading capacitance, board design - /// issue, or lack of 32 XTAL on board). - fn calibrate(cal_clk: RtcCalSel, slowclk_cycles: u32) -> u32 { - let xtal_freq = RtcClock::xtal_freq(); - - let mut slowclk_cycles = slowclk_cycles; - - // The Fosc CLK of calibration circuit is divided by 32 for ECO1. - // So we need to divide the calibrate cycles of the FOSC for ECO1 and above - // chips by 32 to avoid excessive calibration time. - if Efuse::chip_revision() > 0 && cal_clk == RtcCalSel::RtcCalRcFast { - slowclk_cycles >>= 5; - } - - let xtal_cycles = RtcClock::calibrate_internal(cal_clk, slowclk_cycles) as u64; - let divider = xtal_freq.mhz() as u64 * slowclk_cycles as u64; - let period_64 = ((xtal_cycles << RtcClock::CAL_FRACT) + divider / 2u64 - 1u64) / divider; - - (period_64 & u32::MAX as u64) as u32 - } - - /// Calculate the necessary RTC_SLOW_CLK cycles to complete 1 millisecond. - pub(crate) fn cycles_to_1ms() -> u16 { - let period_13q19 = RtcClock::calibrate( - match RtcClock::slow_freq() { - RtcSlowClock::RtcSlowClockRcSlow => RtcCalSel::RtcCalRtcMux, - RtcSlowClock::RtcSlowClock32kXtal => RtcCalSel::RtcCal32kXtal, - RtcSlowClock::RtcSlowClock32kRc => RtcCalSel::RtcCal32kRc, - RtcSlowClock::RtcSlowOscSlow => RtcCalSel::RtcCal32kOscSlow, - // RtcSlowClock::RtcCalRcFast => RtcCalSel::RtcCalRcFast, - }, - 1024, - ); - - // 100_000_000 is used to get rid of `float` calculations - let period = (100_000_000 * period_13q19 as u64) / (1 << RtcClock::CAL_FRACT); - - (100_000_000 * 1000 / period) as u16 - } } pub(crate) fn rtc_clk_cpu_freq_set_xtal() { diff --git a/esp-hal/src/rtc_cntl/rtc/esp32h2.rs b/esp-hal/src/rtc_cntl/rtc/esp32h2.rs index f9b17d345..2268fbbd7 100644 --- a/esp-hal/src/rtc_cntl/rtc/esp32h2.rs +++ b/esp-hal/src/rtc_cntl/rtc/esp32h2.rs @@ -297,19 +297,10 @@ impl RtcClock { } } - fn calibrate(cal_clk: RtcCalSel, slowclk_cycles: u32) -> u32 { - let xtal_freq = RtcClock::xtal_freq(); - let xtal_cycles = RtcClock::calibrate_internal(cal_clk, slowclk_cycles) as u64; - let divider = xtal_freq.mhz() as u64 * slowclk_cycles as u64; - let period_64 = ((xtal_cycles << RtcClock::CAL_FRACT) + divider / 2u64 - 1u64) / divider; - - (period_64 & u32::MAX as u64) as u32 - } - /// Calibration of RTC_SLOW_CLK is performed using a special feature of /// TIMG0. This feature counts the number of XTAL clock cycles within a /// given number of RTC_SLOW_CLK cycles. - fn calibrate_internal(mut cal_clk: RtcCalSel, slowclk_cycles: u32) -> u32 { + pub(crate) fn calibrate_internal(mut cal_clk: RtcCalSel, slowclk_cycles: u32) -> u32 { const SOC_CLK_RC_FAST_FREQ_APPROX: u32 = 17_500_000; const SOC_CLK_RC_SLOW_FREQ_APPROX: u32 = 136_000; const SOC_CLK_XTAL32K_FREQ_APPROX: u32 = 32768; @@ -565,22 +556,4 @@ impl RtcClock { cal_val } - - pub(crate) fn cycles_to_1ms() -> u16 { - let period_13q19 = RtcClock::calibrate( - match RtcClock::slow_freq() { - RtcSlowClock::RtcSlowClockRcSlow => RtcCalSel::RtcCalRtcMux, - RtcSlowClock::RtcSlowClock32kXtal => RtcCalSel::RtcCal32kXtal, - RtcSlowClock::RtcSlowClock32kRc => RtcCalSel::RtcCal32kRc, - RtcSlowClock::RtcSlowOscSlow => RtcCalSel::RtcCal32kOscSlow, - // RtcSlowClock::RtcCalRcFast => RtcCalSel::RtcCalRcFast, - }, - 1024, - ); - - // 100_000_000 is used to get rid of `float` calculations - let period = (100_000_000 * period_13q19 as u64) / (1 << RtcClock::CAL_FRACT); - - (100_000_000 * 1000 / period) as u16 - } } diff --git a/hil-test/tests/init.rs b/hil-test/tests/init.rs index 6d3d6c116..c83cf61cb 100644 --- a/hil-test/tests/init.rs +++ b/hil-test/tests/init.rs @@ -36,10 +36,14 @@ mod tests { let mut wdt0 = timg0.wdt; let delay = Delay::new(); - for _ in 0..4 { + // Loop for more than the timeout of the watchdog. + for _ in 0..6 { wdt0.feed(); - delay.delay(Duration::from_millis(250)); + delay.delay(Duration::from_millis(100)); } + // Disable the watchdog, to prevent accidentally resetting the MCU while the host is setting + // up the next test. + wdt0.disable(); } #[test] @@ -70,10 +74,14 @@ mod tests { let mut wdt1 = timg1.wdt; let delay = Delay::new(); - for _ in 0..4 { + // Loop for more than the timeout of the watchdog. + for _ in 0..6 { wdt1.feed(); - delay.delay(Duration::from_millis(250)); + delay.delay(Duration::from_millis(100)); } + // Disable the watchdog, to prevent accidentally resetting the MCU while the host is setting + // up the next test. + wdt1.disable(); } #[test] @@ -92,27 +100,36 @@ mod tests { let mut wdt0 = timg0.wdt; let delay = Delay::new(); - for _ in 0..4 { + // Loop for more than the timeout of the watchdog. + for _ in 0..6 { wdt0.feed(); - delay.delay(Duration::from_millis(250)); + delay.delay(Duration::from_millis(100)); } + // Disable the watchdog, to prevent accidentally resetting the MCU while the host is setting + // up the next test. + wdt0.disable(); } #[test] - #[timeout(4)] fn test_feeding_rtc_wdt() { let peripherals = esp_hal::init( Config::default().with_watchdog( WatchdogConfig::default() - .with_rwdt(WatchdogStatus::Enabled(Duration::from_millis(3000))), + .with_rwdt(WatchdogStatus::Enabled(Duration::from_millis(500))), ), ); let mut rtc = Rtc::new(peripherals.LPWR); let delay = Delay::new(); - rtc.rwdt.feed(); - delay.delay(Duration::from_millis(2500)); + // Loop for more than the timeout of the watchdog. + for _ in 0..6 { + rtc.rwdt.feed(); + delay.delay(Duration::from_millis(100)); + } + // Disable the watchdog, to prevent accidentally resetting the MCU while the host is setting + // up the next test. + rtc.rwdt.disable(); } #[test] @@ -120,6 +137,6 @@ mod tests { esp_hal::init(Config::default()); let delay = Delay::new(); - delay.delay(Duration::from_millis(2000)); + delay.delay(Duration::from_millis(1000)); } }