Implement embedded_hal_async::delay::DelayNs for TIMGx timers (es…

…p-rs#2084)

* Update `hil-test` package dependencies, add simple test for async delay with `SYSTIMER`

* Implement `embedded_hal_async::delay::DelayNs` for the `TIMGx` timers

* Improve tests slightly

* Update `CHANGELOG.md`

* Enable `delay` and `delay_async` tests for the ESP32-H2

* Fix error in `delay_async` test after rebasing

* ESP32 does not have `SYSTIMER`, so don't try to test it :)

* Protect int_ena modifications with INT_ENA_LOCK, clear int_clr in ISRs, move interrupt binds from Future constructor into new_async constructor

* Fix wrong imports

* Address reviews: Remove duplicated/useless code and add HIL test for delay_us and delay_ms

* Implement DelayNs on Target instead of Periodic

* clean dead code

* fix after rebase

* fix build errors

* More accurate nanos to ticks calculation

* Fix wrong handler passed to set_interrupt_handler()

* Update esp-hal/src/timer/timg.rs

Co-authored-by: Dániel Buga <bugadani@gmail.com>

* cleanup left over

---------

Co-authored-by: Juraj Sadel <juraj.sadel@espressif.com>
Co-authored-by: Juraj Sadel <jurajsadel@gmail.com>
Co-authored-by: Dániel Buga <bugadani@gmail.com>

esp-hal/CHANGELOG.md

@@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Expose `RtcClock::get_xtal_freq` and `RtcClock::get_slow_freq` publically for all chips (#2183)
 - TWAI support for ESP32-H2 (#2199)
 - Added a way to configure watchdogs in `esp_hal::init` (#2180)
+- Implement `embedded_hal_async::delay::DelayNs` for `TIMGx` timers (#2084)
 
 ### Changed
 

esp-hal/src/timer/systimer.rs

@@ -1029,11 +1029,11 @@ mod asynch {
 
     #[must_use = "futures do nothing unless you `.await` or poll them"]
     pub(crate) struct AlarmFuture<'a, COMP: Comparator, UNIT: Unit> {
-        alarm: &'a Alarm<'a, Periodic, crate::Async, COMP, UNIT>,
+        alarm: &'a Alarm<'a, Target, crate::Async, COMP, UNIT>,
     }
 
     impl<'a, COMP: Comparator, UNIT: Unit> AlarmFuture<'a, COMP, UNIT> {
-        pub(crate) fn new(alarm: &'a Alarm<'a, Periodic, crate::Async, COMP, UNIT>) -> Self {
+        pub(crate) fn new(alarm: &'a Alarm<'a, Target, crate::Async, COMP, UNIT>) -> Self {
             alarm.clear_interrupt();
 
             let (interrupt, handler) = match alarm.comparator.channel() {
@@ -1047,6 +1047,8 @@ mod asynch {
                 interrupt::enable(interrupt, handler.priority()).unwrap();
             }
 
+            alarm.set_interrupt_handler(handler);
+
             alarm.enable_interrupt(true);
 
             Self { alarm }
@@ -1076,11 +1078,13 @@ mod asynch {
     }
 
     impl<'d, COMP: Comparator, UNIT: Unit> embedded_hal_async::delay::DelayNs
-        for Alarm<'d, Periodic, crate::Async, COMP, UNIT>
+        for Alarm<'d, Target, crate::Async, COMP, UNIT>
     {
-        async fn delay_ns(&mut self, ns: u32) {
-            let period = MicrosDurationU32::from_ticks(ns / 1000);
-            self.set_period(period);
+        async fn delay_ns(&mut self, nanos: u32) {
+            self.set_target(
+                self.unit.read_count()
+                    + (nanos as u64 * SystemTimer::ticks_per_second()).div_ceil(1_000_000_000),
+            );
 
             AlarmFuture::new(self).await;
         }

esp-hal/src/timer/timg.rs

@@ -304,6 +304,57 @@ where
 {
     /// Construct a new instance of [`TimerGroup`] in asynchronous mode
     pub fn new_async(_timer_group: impl Peripheral<P = T> + 'd) -> Self {
+        match T::id() {
+            0 => {
+                use crate::timer::timg::asynch::timg0_timer0_handler;
+                unsafe {
+                    interrupt::bind_interrupt(
+                        Interrupt::TG0_T0_LEVEL,
+                        timg0_timer0_handler.handler(),
+                    );
+                    interrupt::enable(Interrupt::TG0_T0_LEVEL, timg0_timer0_handler.priority())
+                        .unwrap();
+
+                    #[cfg(timg_timer1)]
+                    {
+                        use crate::timer::timg::asynch::timg0_timer1_handler;
+
+                        interrupt::bind_interrupt(
+                            Interrupt::TG0_T1_LEVEL,
+                            timg0_timer1_handler.handler(),
+                        );
+                        interrupt::enable(Interrupt::TG0_T1_LEVEL, timg0_timer1_handler.priority())
+                            .unwrap();
+                    }
+                }
+            }
+            #[cfg(timg1)]
+            1 => {
+                use crate::timer::timg::asynch::timg1_timer0_handler;
+                unsafe {
+                    {
+                        interrupt::bind_interrupt(
+                            Interrupt::TG1_T0_LEVEL,
+                            timg1_timer0_handler.handler(),
+                        );
+                        interrupt::enable(Interrupt::TG1_T0_LEVEL, timg1_timer0_handler.priority())
+                            .unwrap();
+                    }
+                    #[cfg(timg_timer1)]
+                    {
+                        use crate::timer::timg::asynch::timg1_timer1_handler;
+                        interrupt::bind_interrupt(
+                            Interrupt::TG1_T1_LEVEL,
+                            timg1_timer1_handler.handler(),
+                        );
+                        interrupt::enable(Interrupt::TG1_T1_LEVEL, timg1_timer1_handler.priority())
+                            .unwrap();
+                    }
+                }
+            }
+            _ => unreachable!(),
+        }
+
         Self::new_inner(_timer_group)
     }
 }
@@ -1040,6 +1091,168 @@ where
     }
 }
 
+// Async functionality of the timer groups.
+mod asynch {
+    use core::{
+        pin::Pin,
+        task::{Context, Poll},
+    };
+
+    use embassy_sync::waitqueue::AtomicWaker;
+    use procmacros::handler;
+
+    use super::*;
+
+    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;
+        }
+    }
+
+    static WAKERS: [AtomicWaker; NUM_WAKERS] = [const { AtomicWaker::new() }; NUM_WAKERS];
+
+    pub(crate) struct TimerFuture<'a, T>
+    where
+        T: Instance,
+    {
+        timer: &'a Timer<T, crate::Async>,
+    }
+
+    impl<'a, T> TimerFuture<'a, T>
+    where
+        T: Instance,
+    {
+        pub(crate) fn new(timer: &'a Timer<T, crate::Async>) -> Self {
+            use crate::timer::Timer;
+
+            timer.enable_interrupt(true);
+
+            Self { timer }
+        }
+
+        fn event_bit_is_clear(&self) -> bool {
+            self.timer
+                .register_block()
+                .int_ena()
+                .read()
+                .t(self.timer.timer_number())
+                .bit_is_clear()
+        }
+    }
+
+    impl<'a, T> core::future::Future for TimerFuture<'a, T>
+    where
+        T: Instance,
+    {
+        type Output = ();
+
+        fn poll(self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll<Self::Output> {
+            let index = (self.timer.timer_number() << 1) | self.timer.timer_group();
+            WAKERS[index as usize].register(ctx.waker());
+
+            if self.event_bit_is_clear() {
+                Poll::Ready(())
+            } else {
+                Poll::Pending
+            }
+        }
+    }
+
+    impl<'a, T> Drop for TimerFuture<'a, T>
+    where
+        T: Instance,
+    {
+        fn drop(&mut self) {
+            self.timer.clear_interrupt();
+        }
+    }
+
+    impl<T> embedded_hal_async::delay::DelayNs for Timer<T, crate::Async>
+    where
+        T: Instance,
+    {
+        async fn delay_ns(&mut self, ns: u32) {
+            use crate::timer::Timer as _;
+
+            let period = MicrosDurationU64::from_ticks(ns.div_ceil(1000) as u64);
+            self.load_value(period).unwrap();
+            self.start();
+            self.listen();
+
+            TimerFuture::new(self).await;
+        }
+    }
+
+    // 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], || {
+            unsafe { &*crate::peripherals::TIMG0::PTR }
+                .int_ena()
+                .modify(|_, w| w.t(0).clear_bit())
+        });
+
+        unsafe { &*crate::peripherals::TIMG0::PTR }
+            .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], || {
+            unsafe { &*crate::peripherals::TIMG1::PTR }
+                .int_ena()
+                .modify(|_, w| w.t(0).clear_bit())
+        });
+        unsafe { &*crate::peripherals::TIMG1::PTR }
+            .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], || {
+            unsafe { &*crate::peripherals::TIMG0::PTR }
+                .int_ena()
+                .modify(|_, w| w.t(1).clear_bit())
+        });
+        unsafe { &*crate::peripherals::TIMG0::PTR }
+            .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], || {
+            unsafe { &*crate::peripherals::TIMG1::PTR }
+                .int_ena()
+                .modify(|_, w| w.t(1).clear_bit())
+        });
+
+        unsafe { &*crate::peripherals::TIMG1::PTR }
+            .int_clr()
+            .write(|w| w.t(1).clear_bit_by_one());
+
+        WAKERS[3].wake();
+    }
+}
+
 /// Event Task Matrix
 #[cfg(soc_etm)]
 pub mod etm {

hil-test/Cargo.toml

@@ -27,6 +27,10 @@ harness = false
 name    = "delay"
 harness = false
 
+[[test]]
+name    = "delay_async"
+harness = false
+
 [[test]]
 name    = "dma_macros"
 harness = false
@@ -161,20 +165,20 @@ harness = false
 
 [dependencies]
 cfg-if             = "1.0.0"
-critical-section   = "1.1.2"
+critical-section   = "1.1.3"
 defmt              = "0.3.8"
 defmt-rtt          = { version = "0.4.1", optional = true }
 embassy-futures    = "0.1.1"
 embassy-sync       = "0.6.0"
-embassy-time       = { version = "0.3.1" }
+embassy-time       = "0.3.2"
 embedded-hal       = "1.0.0"
 embedded-hal-02    = { version = "0.2.7", package = "embedded-hal", features = ["unproven"] }
 embedded-hal-async = "1.0.0"
 embedded-hal-nb    = { version = "1.0.0", optional = true }
 esp-backtrace      = { path = "../esp-backtrace", default-features = false, features = ["exception-handler", "panic-handler", "defmt", "semihosting"] }
 esp-hal            = { path = "../esp-hal", features = ["defmt", "digest"], optional = true }
 esp-hal-embassy    = { path = "../esp-hal-embassy", optional = true }
-portable-atomic    = "1.6.0"
+portable-atomic    = "1.7.0"
 static_cell        = { version = "2.1.0", features = ["nightly"] }
 
 [dev-dependencies]
@@ -193,8 +197,8 @@ sha1                = { version = "0.10.6", default-features = false }
 sha2                = { version = "0.10.8", default-features = false }
 
 [build-dependencies]
-esp-build    = { version = "0.1.0", path = "../esp-build" }
-esp-metadata = { version = "0.3.0", path = "../esp-metadata" }
+esp-build    = { path = "../esp-build" }
+esp-metadata = { path = "../esp-metadata" }
 
 [features]
 default = ["embassy"]