From fa4af7e8bea2bb8739a95dbd3ac7096b1f7f2752 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 4 Feb 2025 14:23:01 +0100 Subject: [PATCH 1/6] Shorten test delays --- hil-test/tests/delay_async.rs | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/hil-test/tests/delay_async.rs b/hil-test/tests/delay_async.rs index cb21e1bc8a..56c19ddcb7 100644 --- a/hil-test/tests/delay_async.rs +++ b/hil-test/tests/delay_async.rs @@ -2,6 +2,9 @@ //! //! Specifically tests the various implementations of the //! `embedded_hal_async::delay::DelayNs` trait. +//! +//! This test does not configure embassy, as it doesn't need a timer queue +//! implementation or an embassy time driver. //% CHIPS: esp32 esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3 //% FEATURES: unstable @@ -86,16 +89,16 @@ mod tests { async fn test_systimer_async_delay_ns(ctx: Context) { let alarms = SystemTimer::new(ctx.peripherals.SYSTIMER); - test_async_delay_ns(OneShotTimer::new(alarms.alarm0).into_async(), 10_000_000).await; + test_async_delay_ns(OneShotTimer::new(alarms.alarm0).into_async(), 10_000).await; } #[test] async fn test_timg0_async_delay_ns(ctx: Context) { let timg0 = TimerGroup::new(ctx.peripherals.TIMG0); - test_async_delay_ns(OneShotTimer::new(timg0.timer0).into_async(), 10_000_000).await; + test_async_delay_ns(OneShotTimer::new(timg0.timer0).into_async(), 10_000).await; #[cfg(timg_timer1)] - test_async_delay_ns(OneShotTimer::new(timg0.timer1).into_async(), 10_000_000).await; + test_async_delay_ns(OneShotTimer::new(timg0.timer1).into_async(), 10_000).await; } #[cfg(timg1)] @@ -103,9 +106,9 @@ mod tests { async fn test_timg1_async_delay_ns(ctx: Context) { let timg1 = TimerGroup::new(ctx.peripherals.TIMG1); - test_async_delay_ns(OneShotTimer::new(timg1.timer0).into_async(), 10_000_000).await; + test_async_delay_ns(OneShotTimer::new(timg1.timer0).into_async(), 10_000).await; #[cfg(timg_timer1)] - test_async_delay_ns(OneShotTimer::new(timg1.timer1).into_async(), 10_000_000).await; + test_async_delay_ns(OneShotTimer::new(timg1.timer1).into_async(), 10_000).await; } #[cfg(systimer)] @@ -113,16 +116,16 @@ mod tests { async fn test_systimer_async_delay_us(ctx: Context) { let alarms = SystemTimer::new(ctx.peripherals.SYSTIMER); - test_async_delay_us(OneShotTimer::new(alarms.alarm0).into_async(), 10_000).await; + test_async_delay_us(OneShotTimer::new(alarms.alarm0).into_async(), 10).await; } #[test] async fn test_timg0_async_delay_us(ctx: Context) { let timg0 = TimerGroup::new(ctx.peripherals.TIMG0); - test_async_delay_us(OneShotTimer::new(timg0.timer0).into_async(), 10_000).await; + test_async_delay_us(OneShotTimer::new(timg0.timer0).into_async(), 10).await; #[cfg(timg_timer1)] - test_async_delay_us(OneShotTimer::new(timg0.timer1).into_async(), 10_000).await; + test_async_delay_us(OneShotTimer::new(timg0.timer1).into_async(), 10).await; } #[cfg(timg1)] @@ -130,9 +133,9 @@ mod tests { async fn test_timg1_async_delay_us(ctx: Context) { let timg1 = TimerGroup::new(ctx.peripherals.TIMG1); - test_async_delay_us(OneShotTimer::new(timg1.timer0).into_async(), 10_000).await; + test_async_delay_us(OneShotTimer::new(timg1.timer0).into_async(), 10).await; #[cfg(timg_timer1)] - test_async_delay_us(OneShotTimer::new(timg1.timer1).into_async(), 10_000).await; + test_async_delay_us(OneShotTimer::new(timg1.timer1).into_async(), 10).await; } #[cfg(systimer)] @@ -140,16 +143,16 @@ mod tests { async fn test_systimer_async_delay_ms(ctx: Context) { let alarms = SystemTimer::new(ctx.peripherals.SYSTIMER); - test_async_delay_ms(OneShotTimer::new(alarms.alarm0).into_async(), 10).await; + test_async_delay_ms(OneShotTimer::new(alarms.alarm0).into_async(), 1).await; } #[test] async fn test_timg0_async_delay_ms(ctx: Context) { let timg0 = TimerGroup::new(ctx.peripherals.TIMG0); - test_async_delay_ms(OneShotTimer::new(timg0.timer0).into_async(), 10).await; + test_async_delay_ms(OneShotTimer::new(timg0.timer0).into_async(), 1).await; #[cfg(timg_timer1)] - test_async_delay_ms(OneShotTimer::new(timg0.timer1).into_async(), 10).await; + test_async_delay_ms(OneShotTimer::new(timg0.timer1).into_async(), 1).await; } #[cfg(timg1)] @@ -157,8 +160,8 @@ mod tests { async fn test_timg1_async_delay_ms(ctx: Context) { let timg1 = TimerGroup::new(ctx.peripherals.TIMG1); - test_async_delay_ms(OneShotTimer::new(timg1.timer0).into_async(), 10).await; + test_async_delay_ms(OneShotTimer::new(timg1.timer0).into_async(), 1).await; #[cfg(timg_timer1)] - test_async_delay_ms(OneShotTimer::new(timg1.timer1).into_async(), 10).await; + test_async_delay_ms(OneShotTimer::new(timg1.timer1).into_async(), 1).await; } } From b5ebcb423d2bfce4c4773abc86d4a23ec8631b17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 4 Feb 2025 15:05:59 +0100 Subject: [PATCH 2/6] Fix TIMG --- esp-hal/src/timer/timg.rs | 140 +++++++++++++++++++------------------- 1 file changed, 69 insertions(+), 71 deletions(-) diff --git a/esp-hal/src/timer/timg.rs b/esp-hal/src/timer/timg.rs index edfe8b4100..77b2b836bc 100644 --- a/esp-hal/src/timer/timg.rs +++ b/esp-hal/src/timer/timg.rs @@ -69,6 +69,8 @@ use core::marker::PhantomData; use super::Error; +#[cfg(timg1)] +use crate::peripherals::TIMG1; #[cfg(any(esp32c6, esp32h2))] use crate::soc::constants::TIMG_DEFAULT_CLK_SRC; use crate::{ @@ -78,14 +80,21 @@ use crate::{ peripheral::Peripheral, peripherals::{Interrupt, TIMG0}, private::Sealed, - sync::{lock, RawMutex}, system::PeripheralClockControl, time::{Duration, Instant, Rate}, }; const NUM_TIMG: usize = 1 + cfg!(timg1) as usize; -static INT_ENA_LOCK: [RawMutex; NUM_TIMG] = [const { RawMutex::new() }; NUM_TIMG]; +cfg_if::cfg_if! { + // We need no locks when a TIMG has a single timer, and we don't need locks for ESP32 + // and S2 where the effective interrupt enable register (config) is not shared between + // the timers. + if #[cfg(all(timg_timer1, not(any(esp32, esp32s2))))] { + use crate::sync::{lock, RawMutex}; + static INT_ENA_LOCK: [RawMutex; NUM_TIMG] = [const { RawMutex::new() }; NUM_TIMG]; + } +} /// A timer group consisting of #[cfg_attr(not(timg_timer1), doc = "a general purpose timer")] @@ -300,18 +309,7 @@ impl super::Timer for Timer { } fn enable_interrupt(&self, state: bool) { - // always use level interrupt - #[cfg(any(esp32, esp32s2))] - self.register_block() - .t(self.timer_number().into()) - .config() - .modify(|_, w| w.level_int_en().set_bit()); - - lock(&INT_ENA_LOCK[self.timer_group() as usize], || { - self.register_block() - .int_ena() - .modify(|_, w| w.t(self.timer_number()).bit(state)); - }); + self.set_interrupt_enabled(state); } fn clear_interrupt(&self) { @@ -532,6 +530,27 @@ impl Timer { .t(self.timer) .bit_is_set() } + + fn set_interrupt_enabled(&self, state: bool) { + cfg_if::cfg_if! { + if #[cfg(any(esp32, esp32s2))] { + self.register_block() + .t(self.timer as usize) + .config() + .modify(|_, w| w.level_int_en().bit(state)); + } else if #[cfg(timg_timer1)] { + lock(&INT_ENA_LOCK[self.timer_group() as usize], || { + self.register_block() + .int_ena() + .modify(|_, w| w.t(self.timer_number()).bit(state)); + }); + } else { + self.register_block() + .int_ena() + .modify(|_, w| w.t(0).bit(state)); + } + } + } } fn ticks_to_timeout(ticks: u64, clock: Rate, divider: u32) -> u64 { @@ -793,18 +812,18 @@ mod asynch { use super::*; use crate::asynch::AtomicWaker; - cfg_if::cfg_if! { - if #[cfg(all(timg1, timg_timer1))] { - const NUM_WAKERS: usize = 4; - } else if #[cfg(timg1)] { - const NUM_WAKERS: usize = 2; - } else { - const NUM_WAKERS: usize = 1; - } - } + const NUM_WAKERS: usize = { + let timer_per_group = 1 + cfg!(timg_timer1) as usize; + NUM_TIMG * timer_per_group + }; static WAKERS: [AtomicWaker; NUM_WAKERS] = [const { AtomicWaker::new() }; NUM_WAKERS]; + fn waker(timer: &Timer) -> &AtomicWaker { + let index = (timer.timer_number() << 1) | timer.timer_group(); + &WAKERS[index as usize] + } + pub(crate) struct TimerFuture<'a> { timer: &'a Timer, } @@ -818,13 +837,8 @@ mod asynch { Self { timer } } - fn event_bit_is_clear(&self) -> bool { - self.timer - .register_block() - .int_ena() - .read() - .t(self.timer.timer_number()) - .bit_is_clear() + fn is_done(&self) -> bool { + self.timer.is_interrupt_set() } } @@ -832,10 +846,9 @@ mod asynch { type Output = (); fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll { - let index = (self.timer.timer_number() << 1) | self.timer.timer_group(); - WAKERS[index as usize].register(ctx.waker()); + waker(self.timer).register(ctx.waker()); - if self.event_bit_is_clear() { + if self.is_done() { Poll::Ready(()) } else { Poll::Pending @@ -846,72 +859,57 @@ mod asynch { impl Drop for TimerFuture<'_> { fn drop(&mut self) { self.timer.clear_interrupt(); + self.timer.set_interrupt_enabled(false); } } + #[inline] + fn handle_irq(timer: Timer) { + timer.set_interrupt_enabled(false); + waker(&timer).wake(); + } + // INT_ENA means that when the interrupt occurs, it will show up in the // INT_ST. Clearing INT_ENA that it won't show up on INT_ST but if // interrupt is already there, it won't clear it - that's why we need to // clear the INT_CLR as well. #[handler] pub(crate) fn timg0_timer0_handler() { - lock(&INT_ENA_LOCK[0], || { - crate::peripherals::TIMG0::regs() - .int_ena() - .modify(|_, w| w.t(0).clear_bit()) + handle_irq(Timer { + register_block: TIMG0::regs(), + timer: 0, + tg: 0, }); - - crate::peripherals::TIMG0::regs() - .int_clr() - .write(|w| w.t(0).clear_bit_by_one()); - - WAKERS[0].wake(); } #[cfg(timg1)] #[handler] pub(crate) fn timg1_timer0_handler() { - lock(&INT_ENA_LOCK[1], || { - crate::peripherals::TIMG1::regs() - .int_ena() - .modify(|_, w| w.t(0).clear_bit()) + handle_irq(Timer { + register_block: TIMG1::regs(), + timer: 0, + tg: 1, }); - crate::peripherals::TIMG1::regs() - .int_clr() - .write(|w| w.t(0).clear_bit_by_one()); - - WAKERS[1].wake(); } #[cfg(timg_timer1)] #[handler] pub(crate) fn timg0_timer1_handler() { - lock(&INT_ENA_LOCK[0], || { - crate::peripherals::TIMG0::regs() - .int_ena() - .modify(|_, w| w.t(1).clear_bit()) + handle_irq(Timer { + register_block: TIMG0::regs(), + timer: 1, + tg: 0, }); - crate::peripherals::TIMG0::regs() - .int_clr() - .write(|w| w.t(1).clear_bit_by_one()); - - WAKERS[2].wake(); } #[cfg(all(timg1, timg_timer1))] #[handler] pub(crate) fn timg1_timer1_handler() { - lock(&INT_ENA_LOCK[1], || { - crate::peripherals::TIMG1::regs() - .int_ena() - .modify(|_, w| w.t(1).clear_bit()) + handle_irq(Timer { + register_block: TIMG1::regs(), + timer: 1, + tg: 1, }); - - crate::peripherals::TIMG1::regs() - .int_clr() - .write(|w| w.t(1).clear_bit_by_one()); - - WAKERS[3].wake(); } } From 97a3f8a3fcf015e1adfb4b04ba605b0f775433ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 4 Feb 2025 15:32:13 +0100 Subject: [PATCH 3/6] Fix systimer --- esp-hal/src/timer/systimer.rs | 60 +++++++++++++++++------------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/esp-hal/src/timer/systimer.rs b/esp-hal/src/timer/systimer.rs index 11d9b775ba..2cab86c1dc 100644 --- a/esp-hal/src/timer/systimer.rs +++ b/esp-hal/src/timer/systimer.rs @@ -616,7 +616,7 @@ mod asynch { use procmacros::handler; use super::*; - use crate::{asynch::AtomicWaker, peripherals::SYSTIMER}; + use crate::asynch::AtomicWaker; const NUM_ALARMS: usize = 3; @@ -629,17 +629,15 @@ mod asynch { impl<'a> AlarmFuture<'a> { pub(crate) fn new(alarm: &'a Alarm) -> Self { + // For some reason, on the S2 we need to enable the interrupt before we + // read its status. Doing so in the other order causes the interrupt + // request to never be fired. alarm.enable_interrupt(true); - Self { alarm } } - fn event_bit_is_clear(&self) -> bool { - SYSTIMER::regs() - .int_ena() - .read() - .target(self.alarm.channel()) - .bit_is_clear() + fn is_done(&self) -> bool { + self.alarm.is_interrupt_set() } } @@ -647,9 +645,11 @@ mod asynch { type Output = (); fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll { + // Interrupts are enabled, so we need to register the waker before we check for + // done. Otherwise we might miss the interrupt that would wake us. WAKERS[self.alarm.channel() as usize].register(ctx.waker()); - if self.event_bit_is_clear() { + if self.is_done() { Poll::Ready(()) } else { Poll::Pending @@ -657,37 +657,37 @@ mod asynch { } } + impl Drop for AlarmFuture<'_> { + fn drop(&mut self) { + self.alarm.enable_interrupt(false); + self.alarm.clear_interrupt(); + } + } + + #[inline] + fn handle_alarm(alarm: u8) { + Alarm { + comp: alarm, + unit: Unit::Unit0, + } + .enable_interrupt(false); + + WAKERS[alarm as usize].wake(); + } + #[handler] pub(crate) fn target0_handler() { - lock(&INT_ENA_LOCK, || { - SYSTIMER::regs() - .int_ena() - .modify(|_, w| w.target0().clear_bit()); - }); - - WAKERS[0].wake(); + handle_alarm(0); } #[handler] pub(crate) fn target1_handler() { - lock(&INT_ENA_LOCK, || { - SYSTIMER::regs() - .int_ena() - .modify(|_, w| w.target1().clear_bit()); - }); - - WAKERS[1].wake(); + handle_alarm(1); } #[handler] pub(crate) fn target2_handler() { - lock(&INT_ENA_LOCK, || { - SYSTIMER::regs() - .int_ena() - .modify(|_, w| w.target2().clear_bit()); - }); - - WAKERS[2].wake(); + handle_alarm(2); } } From 3d5fbdc0f3a15aad74dabd8da4da50bf451cc0b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 5 Feb 2025 09:21:24 +0100 Subject: [PATCH 4/6] Changelog --- esp-hal/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index ac319cef4a..a6ddaccf2b 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `DmaDescriptor` is now `#[repr(C)]` (#2988) - Fixed an issue that caused LCD_CAM drivers to turn off their clocks unexpectedly (#3007) - Fixed an issue where DMA-driver peripherals started transferring before the data was ready (#3003) +- Fixed an issue on ESP32 and S2 where short asynchronous Timer delays would never resolve (#3093) ### Removed From 9e093b1b6f83fa9f1873e39372db5f7ca85f4355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 5 Feb 2025 09:52:33 +0100 Subject: [PATCH 5/6] Use a single future implementation --- esp-hal/src/timer/mod.rs | 65 +++++++++++++++++++++++++++++------ esp-hal/src/timer/systimer.rs | 57 ++++-------------------------- esp-hal/src/timer/timg.rs | 55 ++++------------------------- 3 files changed, 68 insertions(+), 109 deletions(-) diff --git a/esp-hal/src/timer/mod.rs b/esp-hal/src/timer/mod.rs index 12e7dcda49..c7b8cebb0c 100644 --- a/esp-hal/src/timer/mod.rs +++ b/esp-hal/src/timer/mod.rs @@ -41,9 +41,14 @@ //! # } //! ``` -use core::marker::PhantomData; +use core::{ + marker::PhantomData, + pin::Pin, + task::{Context, Poll}, +}; use crate::{ + asynch::AtomicWaker, interrupt::{InterruptConfigurable, InterruptHandler}, peripheral::{Peripheral, PeripheralRef}, peripherals::Interrupt, @@ -113,13 +118,6 @@ pub trait Timer: Into + 'static + crate::private::Sealed { /// Has the timer triggered? fn is_interrupt_set(&self) -> bool; - /// Asynchronously wait for the timer interrupt to fire. - /// - /// Requires the correct `InterruptHandler` to be installed to function - /// correctly. - #[doc(hidden)] - async fn wait(&self); - /// Returns the HAL provided async interrupt handler #[doc(hidden)] fn async_interrupt_handler(&self) -> InterruptHandler; @@ -130,6 +128,9 @@ pub trait Timer: Into + 'static + crate::private::Sealed { /// Configures the interrupt handler. #[doc(hidden)] fn set_interrupt_handler(&self, handler: InterruptHandler); + + #[doc(hidden)] + fn waker(&self) -> &AtomicWaker; } /// A one-shot timer. @@ -189,12 +190,56 @@ impl OneShotTimer<'_, Async> { async fn delay_async(&mut self, us: Duration) { unwrap!(self.schedule(us)); - self.inner.wait().await; + + WaitFuture::new(self.inner.reborrow()).await; + self.stop(); self.clear_interrupt(); } } +#[must_use = "futures do nothing unless you `.await` or poll them"] +struct WaitFuture<'d> { + timer: PeripheralRef<'d, AnyTimer>, +} + +impl<'d> WaitFuture<'d> { + fn new(timer: impl Peripheral

+ 'd) -> Self { + crate::into_ref!(timer); + // For some reason, on the S2 we need to enable the interrupt before we + // read its status. Doing so in the other order causes the interrupt + // request to never be fired. + timer.enable_interrupt(true); + Self { timer } + } + + fn is_done(&self) -> bool { + self.timer.is_interrupt_set() + } +} + +impl core::future::Future for WaitFuture<'_> { + type Output = (); + + fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll { + // Interrupts are enabled, so we need to register the waker before we check for + // done. Otherwise we might miss the interrupt that would wake us. + self.timer.waker().register(ctx.waker()); + + if self.is_done() { + Poll::Ready(()) + } else { + Poll::Pending + } + } +} + +impl Drop for WaitFuture<'_> { + fn drop(&mut self) { + self.timer.enable_interrupt(false); + } +} + impl OneShotTimer<'_, Dm> where Dm: DriverMode, @@ -398,10 +443,10 @@ impl Timer for AnyTimer { fn enable_interrupt(&self, state: bool); fn clear_interrupt(&self); fn is_interrupt_set(&self) -> bool; - async fn wait(&self); fn async_interrupt_handler(&self) -> InterruptHandler; fn peripheral_interrupt(&self) -> Interrupt; fn set_interrupt_handler(&self, handler: InterruptHandler); + fn waker(&self) -> &AtomicWaker; } } } diff --git a/esp-hal/src/timer/systimer.rs b/esp-hal/src/timer/systimer.rs index 2cab86c1dc..8c74637519 100644 --- a/esp-hal/src/timer/systimer.rs +++ b/esp-hal/src/timer/systimer.rs @@ -21,6 +21,7 @@ use core::fmt::Debug; use super::{Error, Timer as _}; use crate::{ + asynch::AtomicWaker, interrupt::{self, InterruptHandler}, peripheral::Peripheral, peripherals::{Interrupt, SYSTIMER}, @@ -562,10 +563,6 @@ impl super::Timer for Alarm { .bit_is_set() } - async fn wait(&self) { - asynch::AlarmFuture::new(self).await - } - fn async_interrupt_handler(&self) -> InterruptHandler { match self.channel() { 0 => asynch::target0_handler, @@ -587,6 +584,10 @@ impl super::Timer for Alarm { fn set_interrupt_handler(&self, handler: InterruptHandler) { self.set_interrupt_handler(handler) } + + fn waker(&self) -> &AtomicWaker { + asynch::waker(self) + } } impl Peripheral for Alarm { @@ -608,60 +609,16 @@ static INT_ENA_LOCK: RawMutex = RawMutex::new(); // Async functionality of the system timer. mod asynch { - use core::{ - pin::Pin, - task::{Context, Poll}, - }; - use procmacros::handler; use super::*; use crate::asynch::AtomicWaker; const NUM_ALARMS: usize = 3; - static WAKERS: [AtomicWaker; NUM_ALARMS] = [const { AtomicWaker::new() }; NUM_ALARMS]; - #[must_use = "futures do nothing unless you `.await` or poll them"] - pub(crate) struct AlarmFuture<'a> { - alarm: &'a Alarm, - } - - impl<'a> AlarmFuture<'a> { - pub(crate) fn new(alarm: &'a Alarm) -> Self { - // For some reason, on the S2 we need to enable the interrupt before we - // read its status. Doing so in the other order causes the interrupt - // request to never be fired. - alarm.enable_interrupt(true); - Self { alarm } - } - - fn is_done(&self) -> bool { - self.alarm.is_interrupt_set() - } - } - - impl core::future::Future for AlarmFuture<'_> { - type Output = (); - - fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll { - // Interrupts are enabled, so we need to register the waker before we check for - // done. Otherwise we might miss the interrupt that would wake us. - WAKERS[self.alarm.channel() as usize].register(ctx.waker()); - - if self.is_done() { - Poll::Ready(()) - } else { - Poll::Pending - } - } - } - - impl Drop for AlarmFuture<'_> { - fn drop(&mut self) { - self.alarm.enable_interrupt(false); - self.alarm.clear_interrupt(); - } + pub(super) fn waker(alarm: &Alarm) -> &AtomicWaker { + &WAKERS[alarm.channel() as usize] } #[inline] diff --git a/esp-hal/src/timer/timg.rs b/esp-hal/src/timer/timg.rs index 77b2b836bc..4f1c7687ff 100644 --- a/esp-hal/src/timer/timg.rs +++ b/esp-hal/src/timer/timg.rs @@ -74,6 +74,7 @@ use crate::peripherals::TIMG1; #[cfg(any(esp32c6, esp32h2))] use crate::soc::constants::TIMG_DEFAULT_CLK_SRC; use crate::{ + asynch::AtomicWaker, clock::Clocks, interrupt::{self, InterruptConfigurable, InterruptHandler}, pac::timg0::RegisterBlock, @@ -320,10 +321,6 @@ impl super::Timer for Timer { self.is_interrupt_set() } - async fn wait(&self) { - asynch::TimerFuture::new(self).await - } - fn async_interrupt_handler(&self) -> InterruptHandler { match (self.timer_group(), self.timer_number()) { (0, 0) => asynch::timg0_timer0_handler, @@ -353,6 +350,10 @@ impl super::Timer for Timer { fn set_interrupt_handler(&self, handler: InterruptHandler) { self.set_interrupt_handler(handler) } + + fn waker(&self) -> &AtomicWaker { + asynch::waker(self) + } } impl Peripheral for Timer { @@ -802,11 +803,6 @@ where // Async functionality of the timer groups. mod asynch { - use core::{ - pin::Pin, - task::{Context, Poll}, - }; - use procmacros::handler; use super::*; @@ -819,50 +815,11 @@ mod asynch { static WAKERS: [AtomicWaker; NUM_WAKERS] = [const { AtomicWaker::new() }; NUM_WAKERS]; - fn waker(timer: &Timer) -> &AtomicWaker { + pub(super) fn waker(timer: &Timer) -> &AtomicWaker { let index = (timer.timer_number() << 1) | timer.timer_group(); &WAKERS[index as usize] } - pub(crate) struct TimerFuture<'a> { - timer: &'a Timer, - } - - impl<'a> TimerFuture<'a> { - pub(crate) fn new(timer: &'a Timer) -> Self { - use crate::timer::Timer; - - timer.enable_interrupt(true); - - Self { timer } - } - - fn is_done(&self) -> bool { - self.timer.is_interrupt_set() - } - } - - impl core::future::Future for TimerFuture<'_> { - type Output = (); - - fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll { - waker(self.timer).register(ctx.waker()); - - if self.is_done() { - Poll::Ready(()) - } else { - Poll::Pending - } - } - } - - impl Drop for TimerFuture<'_> { - fn drop(&mut self) { - self.timer.clear_interrupt(); - self.timer.set_interrupt_enabled(false); - } - } - #[inline] fn handle_irq(timer: Timer) { timer.set_interrupt_enabled(false); From feccff8b9f92c01d18005602247a3d59236e842b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 5 Feb 2025 13:49:56 +0100 Subject: [PATCH 6/6] Explain level interrupts --- esp-hal/src/timer/timg.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/esp-hal/src/timer/timg.rs b/esp-hal/src/timer/timg.rs index 4f1c7687ff..0b546876b5 100644 --- a/esp-hal/src/timer/timg.rs +++ b/esp-hal/src/timer/timg.rs @@ -535,6 +535,9 @@ impl Timer { fn set_interrupt_enabled(&self, state: bool) { cfg_if::cfg_if! { if #[cfg(any(esp32, esp32s2))] { + // On ESP32 and S2, the `int_ena` register is ineffective - interrupts fire even + // without int_ena enabling them. We use level interrupts so that we have a status + // bit available. self.register_block() .t(self.timer as usize) .config()