diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 524c1ec036e..810549339f1 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - ESP32C6: Make ADC usable after TRNG deinicialization (#1945) - We should no longer generate 1GB .elf files for ESP32C2 and ESP32C3 (#1962) - Reset peripherals in driver constructors where missing (#1893, #1961) +- Fixed ESP32-S2 systimer interrupts (#1979) ### Removed diff --git a/esp-hal/src/interrupt/mod.rs b/esp-hal/src/interrupt/mod.rs index e8ef8d64bc0..b2d05788cdb 100644 --- a/esp-hal/src/interrupt/mod.rs +++ b/esp-hal/src/interrupt/mod.rs @@ -225,10 +225,6 @@ impl Iterator for InterruptStatusIterator { type Item = u8; fn next(&mut self) -> Option { - if self.idx == usize::MAX { - return None; - } - for i in self.idx..STATUS_WORDS { if self.status.status[i] != 0 { let bit = self.status.status[i].trailing_zeros(); diff --git a/esp-hal/src/interrupt/xtensa.rs b/esp-hal/src/interrupt/xtensa.rs index 23382b03a63..04dc1789797 100644 --- a/esp-hal/src/interrupt/xtensa.rs +++ b/esp-hal/src/interrupt/xtensa.rs @@ -1,6 +1,6 @@ //! Interrupt handling -use xtensa_lx::interrupt::{self, InterruptNumber}; +use xtensa_lx::interrupt; use xtensa_lx_rt::exception::Context; pub use self::vectored::*; @@ -502,7 +502,7 @@ mod vectored { fn EspDefaultHandler(level: u32, interrupt: Interrupt); } - let handler = peripherals::__INTERRUPTS[interrupt.number() as usize]._handler; + let handler = peripherals::__INTERRUPTS[interrupt as usize]._handler; if core::ptr::eq( handler as *const _, EspDefaultHandler as *const unsafe extern "C" fn(), @@ -520,9 +520,9 @@ mod vectored { mod chip_specific { use super::*; pub const INTERRUPT_EDGE: InterruptStatus = InterruptStatus::from( - 0b0000_0000_0000_0000_0000_0000_0000_0011, - 0b1111_1100_0000_0000_0000_0000_0000_0000, 0b0000_0000_0000_0000_0000_0000_0000_0000, + 0b1111_1100_0000_0000_0000_0000_0000_0000, + 0b0000_0000_0000_0000_0000_0000_0000_0011, ); #[inline] pub fn interrupt_is_edge(interrupt: Interrupt) -> bool { @@ -541,14 +541,13 @@ mod vectored { } } - #[allow(clippy::unusual_byte_groupings)] #[cfg(esp32s2)] mod chip_specific { use super::*; pub const INTERRUPT_EDGE: InterruptStatus = InterruptStatus::from( - 0b0000_0000_0000_0000_0000_0011_1011_1111, - 0b1100_0000_0000_0000_0000_0000_0000_0000, 0b0000_0000_0000_0000_0000_0000_0000_0000, + 0b1100_0000_0000_0000_0000_0000_0000_0000, + 0b0000_0000_0000_0000_0000_0011_1011_1111, ); #[inline] pub fn interrupt_is_edge(interrupt: Interrupt) -> bool { diff --git a/esp-hal/src/timer/systimer.rs b/esp-hal/src/timer/systimer.rs index 43df2e8e494..60745c4118b 100644 --- a/esp-hal/src/timer/systimer.rs +++ b/esp-hal/src/timer/systimer.rs @@ -396,20 +396,20 @@ pub trait Comparator { fn set_enable(&self, enable: bool) { let systimer = unsafe { &*SYSTIMER::ptr() }; - #[cfg(not(esp32s2))] critical_section::with(|_| { + #[cfg(not(esp32s2))] systimer.conf().modify(|_, w| match self.channel() { 0 => w.target0_work_en().bit(enable), 1 => w.target1_work_en().bit(enable), 2 => w.target2_work_en().bit(enable), _ => unreachable!(), }); - }); - #[cfg(esp32s2)] - systimer - .target_conf(self.channel() as usize) - .modify(|_r, w| w.work_en().bit(enable)); + #[cfg(esp32s2)] + systimer + .target_conf(self.channel() as usize) + .modify(|_r, w| w.work_en().bit(enable)); + }); } /// Returns true if the comparator has been enabled. This means @@ -524,9 +524,48 @@ pub trait Comparator { 2 => Interrupt::SYSTIMER_TARGET2, _ => unreachable!(), }; + + #[cfg(not(esp32s2))] unsafe { interrupt::bind_interrupt(interrupt, handler.handler()); } + + #[cfg(esp32s2)] + { + // ESP32-S2 Systimer interrupts are edge triggered. Our interrupt + // handler calls each of the handlers, regardless of which one triggered the + // interrupt. This mess registers an intermediate handler that + // checks if an interrupt is active before calling the associated + // handler functions. + + static mut HANDLERS: [Option; 3] = [None, None, None]; + + #[crate::prelude::ram] + unsafe extern "C" fn _handle_interrupt() { + if unsafe { &*SYSTIMER::PTR } + .int_raw() + .read() + .target(CH) + .bit_is_set() + { + let handler = unsafe { HANDLERS[CH as usize] }; + if let Some(handler) = handler { + handler(); + } + } + } + + unsafe { + HANDLERS[self.channel() as usize] = Some(handler.handler()); + let handler = match self.channel() { + 0 => _handle_interrupt::<0>, + 1 => _handle_interrupt::<1>, + 2 => _handle_interrupt::<2>, + _ => unreachable!(), + }; + interrupt::bind_interrupt(interrupt, handler); + } + } unwrap!(interrupt::enable(interrupt, handler.priority())); } } diff --git a/examples/src/bin/systimer.rs b/examples/src/bin/systimer.rs index 455af18be48..15e479d8d32 100644 --- a/examples/src/bin/systimer.rs +++ b/examples/src/bin/systimer.rs @@ -14,8 +14,7 @@ use esp_backtrace as _; use esp_hal::{ clock::ClockControl, delay::Delay, - interrupt::{self, Priority}, - peripherals::{Interrupt, Peripherals}, + peripherals::Peripherals, prelude::*, system::SystemControl, timer::systimer::{ @@ -87,10 +86,6 @@ fn main() -> ! { ALARM2.borrow_ref_mut(cs).replace(alarm2); }); - interrupt::enable(Interrupt::SYSTIMER_TARGET0, Priority::Priority1).unwrap(); - interrupt::enable(Interrupt::SYSTIMER_TARGET1, Priority::Priority3).unwrap(); - interrupt::enable(Interrupt::SYSTIMER_TARGET2, Priority::Priority3).unwrap(); - let delay = Delay::new(&clocks); loop { diff --git a/hil-test/Cargo.toml b/hil-test/Cargo.toml index b8779724d4e..c8444811cba 100644 --- a/hil-test/Cargo.toml +++ b/hil-test/Cargo.toml @@ -87,6 +87,10 @@ harness = false name = "spi_half_duplex_write" harness = false +[[test]] +name = "systimer" +harness = false + [[test]] name = "pcnt" harness = false @@ -163,6 +167,7 @@ elliptic-curve = { version = "0.13.8", default-features = false, features = embassy-executor = { version = "0.6.0", default-features = false } # Add the `embedded-test/defmt` feature for more verbose testing embedded-test = { version = "0.4.0", default-features = false } +fugit = "0.3.7" hex-literal = "0.4.1" nb = "1.1.0" p192 = { version = "0.13.0", default-features = false, features = ["arithmetic"] } diff --git a/hil-test/tests/systimer.rs b/hil-test/tests/systimer.rs new file mode 100644 index 00000000000..9e3fe53dc94 --- /dev/null +++ b/hil-test/tests/systimer.rs @@ -0,0 +1,195 @@ +//! System Timer Test + +// esp32 disabled as it does not have a systimer +//% CHIPS: esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3 + +#![no_std] +#![no_main] + +use core::cell::RefCell; + +use critical_section::Mutex; +use embedded_hal::delay::DelayNs; +use esp_hal::{ + clock::{ClockControl, Clocks}, + delay::Delay, + peripherals::Peripherals, + prelude::*, + system::SystemControl, + timer::systimer::{ + Alarm, + FrozenUnit, + Periodic, + SpecificComparator, + SpecificUnit, + SystemTimer, + Target, + }, + Blocking, +}; +use fugit::ExtU32; +use hil_test as _; +use portable_atomic::{AtomicUsize, Ordering}; +use static_cell::StaticCell; + +type TestAlarm = + Alarm<'static, M, Blocking, SpecificComparator<'static, C>, SpecificUnit<'static, 0>>; + +static ALARM_TARGET: Mutex>>> = Mutex::new(RefCell::new(None)); +static ALARM_PERIODIC: Mutex>>> = + Mutex::new(RefCell::new(None)); + +struct Context { + unit: FrozenUnit<'static, SpecificUnit<'static, 0>>, + comparator0: SpecificComparator<'static, 0>, + comparator1: SpecificComparator<'static, 1>, + clocks: Clocks<'static>, +} + +impl Context { + pub fn init() -> Self { + let peripherals = Peripherals::take(); + let system = SystemControl::new(peripherals.SYSTEM); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let systimer = SystemTimer::new(peripherals.SYSTIMER); + static UNIT0: StaticCell> = StaticCell::new(); + + let unit0 = UNIT0.init(systimer.unit0); + let frozen_unit = FrozenUnit::new(unit0); + + Context { + clocks, + unit: frozen_unit, + comparator0: systimer.comparator0, + comparator1: systimer.comparator1, + } + } +} + +#[handler(priority = esp_hal::interrupt::Priority::min())] +fn pass_test_if_called() { + critical_section::with(|cs| { + ALARM_TARGET + .borrow_ref_mut(cs) + .as_mut() + .unwrap() + .clear_interrupt() + }); + embedded_test::export::check_outcome(()); +} + +#[handler(priority = esp_hal::interrupt::Priority::min())] +fn handle_periodic_interrupt() { + critical_section::with(|cs| { + ALARM_PERIODIC + .borrow_ref_mut(cs) + .as_mut() + .unwrap() + .clear_interrupt() + }); +} + +static COUNTER: AtomicUsize = AtomicUsize::new(0); + +#[handler(priority = esp_hal::interrupt::Priority::min())] +fn pass_test_if_called_twice() { + critical_section::with(|cs| { + ALARM_PERIODIC + .borrow_ref_mut(cs) + .as_mut() + .unwrap() + .clear_interrupt() + }); + COUNTER.fetch_add(1, Ordering::Relaxed); + if COUNTER.load(Ordering::Relaxed) == 2 { + embedded_test::export::check_outcome(()); + } +} + +#[handler(priority = esp_hal::interrupt::Priority::min())] +fn target_fail_test_if_called_twice() { + critical_section::with(|cs| { + ALARM_TARGET + .borrow_ref_mut(cs) + .as_mut() + .unwrap() + .clear_interrupt() + }); + COUNTER.fetch_add(1, Ordering::Relaxed); + assert!(COUNTER.load(Ordering::Relaxed) != 2); +} + +#[cfg(test)] +#[embedded_test::tests] +mod tests { + use super::*; + + #[init] + fn init() -> Context { + Context::init() + } + + #[test] + #[timeout(3)] + fn target_interrupt_is_handled(ctx: Context) { + let alarm0 = Alarm::new(ctx.comparator0, &ctx.unit); + + critical_section::with(|cs| { + alarm0.set_interrupt_handler(pass_test_if_called); + alarm0.set_target(SystemTimer::now() + SystemTimer::TICKS_PER_SECOND / 10); + alarm0.enable_interrupt(true); + + ALARM_TARGET.borrow_ref_mut(cs).replace(alarm0); + }); + + // We'll end the test in the interrupt handler. + loop {} + } + + #[test] + #[timeout(3)] + fn target_interrupt_is_handled_once(ctx: Context) { + let alarm0 = Alarm::new(ctx.comparator0, &ctx.unit); + let alarm1 = Alarm::new(ctx.comparator1, &ctx.unit); + + COUNTER.store(0, Ordering::Relaxed); + + critical_section::with(|cs| { + alarm0.set_interrupt_handler(target_fail_test_if_called_twice); + alarm0.set_target(SystemTimer::now() + SystemTimer::TICKS_PER_SECOND / 10); + alarm0.enable_interrupt(true); + + let alarm1 = alarm1.into_periodic(); + alarm1.set_interrupt_handler(handle_periodic_interrupt); + alarm1.set_period(100u32.millis()); + alarm1.enable_interrupt(true); + + ALARM_TARGET.borrow_ref_mut(cs).replace(alarm0); + ALARM_PERIODIC.borrow_ref_mut(cs).replace(alarm1); + }); + + let mut delay = Delay::new(&ctx.clocks); + delay.delay_ms(300); + } + + #[test] + #[timeout(3)] + fn periodic_interrupt_is_handled(ctx: Context) { + let alarm1 = Alarm::new(ctx.comparator1, &ctx.unit); + + COUNTER.store(0, Ordering::Relaxed); + + critical_section::with(|cs| { + let alarm1 = alarm1.into_periodic(); + alarm1.set_interrupt_handler(pass_test_if_called_twice); + alarm1.set_period(100u32.millis()); + alarm1.enable_interrupt(true); + + ALARM_PERIODIC.borrow_ref_mut(cs).replace(alarm1); + }); + + // We'll end the test in the interrupt handler. + loop {} + } +}