CLINT rework

Author: romancardenas

riscv-peripheral/CHANGELOG.md

@@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Changed
+
+- Rework of CLINT peripheral to use methods instead of associated functions.
+  This change follows the `svd2rust` pattern, making the ecosystem more consistent.
+- Simplify `clint!` macro using the `Deref` trait.
+
+### Removed
+
+- Removed support for `embedded-hal-async`, as it was not flexible enough to be
+  used in different targets (single HART, multi HART...). Instead, each chip must
+  have its own `chip-hal-async` crate that properly adapts to its specific needs.
+
 ### Fixed
 
 - `clippy` fixes

riscv-peripheral/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "riscv-peripheral"
-version = "0.2.1"
+version = "0.3.0-rc.1"
 edition = "2021"
 rust-version = "1.75"
 repository = "https://github.com/rust-embedded/riscv"
@@ -15,16 +15,10 @@ license = "ISC"
 
 [dependencies]
 embedded-hal = "1.0.0"
-embedded-hal-async = { version = "1.0.0", optional =  true }
+paste = "1.0"
 riscv = { path = "../riscv", version = "0.13.0" }
 riscv-pac = { path = "../riscv-pac", version = "0.2.0" }
 
-[dev-dependencies]
-heapless = "0.8.0"
-
-[features]
-aclint-hal-async = ["embedded-hal-async"]
-
 [package.metadata.docs.rs]
 all-features = true
 default-target = "riscv64imac-unknown-none-elf"

riscv-peripheral/examples/e310x.rs

@@ -139,68 +139,15 @@ unsafe impl PriorityNumber for Priority {
     }
 }
 
-#[cfg(feature = "aclint-hal-async")]
 riscv_peripheral::clint_codegen!(
     base 0x0200_0000,
-    freq 32_768,
-    async_delay,
-    mtimecmps [mtimecmp0=(HartId::H0,"`H0`")],
-    msips [msip0=(HartId::H0,"`H0`")],
-);
-
-#[cfg(not(feature = "aclint-hal-async"))]
-riscv_peripheral::clint_codegen!(
-    base 0x0200_0000,
-    freq 32_768,
-    mtimecmps [mtimecmp0=(HartId::H0,"`H0`")],
-    msips [msip0=(HartId::H0,"`H0`")],
+    mtime_freq 32_768,
+    harts [HartId::H0 => 0],
 );
 
 riscv_peripheral::plic_codegen!(
     base 0x0C00_0000,
     ctxs [ctx0=(HartId::H0,"`H0`")],
 );
 
-#[cfg(feature = "aclint-hal-async")]
-/// extern functions needed by the `riscv-peripheral` crate for the `async` feature.
-///
-/// # Note
-///
-/// The functionality in this module is just to illustrate how to enable the `async` feature
-/// The timer queue used here, while functional, is unsound and should not be used in production.
-/// In this case, you should protect the timer queue with a mutex or critical section.
-/// For a more robust implementation, use proper timer queues such as the ones provided by `embassy-time`
-mod async_no_mangle {
-    use super::CLINT;
-    use heapless::binary_heap::{BinaryHeap, Min};
-    use riscv_peripheral::{aclint::mtimer::MTIMER, hal_async::aclint::Timer};
-
-    const N_TIMERS: usize = 16;
-    static mut TIMER_QUEUE: BinaryHeap<Timer, Min, N_TIMERS> = BinaryHeap::new();
-
-    #[no_mangle]
-    fn _riscv_peripheral_aclint_mtimer() -> MTIMER {
-        CLINT::mtimer()
-    }
-
-    #[no_mangle]
-    fn _riscv_peripheral_aclint_push_timer(t: Timer) -> Result<(), Timer> {
-        unsafe { TIMER_QUEUE.push(t) }
-    }
-
-    #[no_mangle]
-    fn _riscv_peripheral_aclint_wake_timers(current_tick: u64) -> Option<u64> {
-        let mut next_expires = None;
-        while let Some(t) = unsafe { TIMER_QUEUE.peek() } {
-            if t.expires() > current_tick {
-                next_expires = Some(t.expires());
-                break;
-            }
-            let t = unsafe { TIMER_QUEUE.pop() }.unwrap();
-            t.waker().wake_by_ref();
-        }
-        next_expires
-    }
-}
-
 fn main() {}

riscv-peripheral/src/aclint.rs

@@ -1,7 +1,7 @@
 //! Devices for the Core Local Interruptor (CLINT) and Advanced CLINT (ACLINT) peripherals.
 //!
 //! CLINT pecification: <https://github.com/pulp-platform/clint>
-//! ACLINT Specification: <https://chromitem-soc.readthedocs.io/en/latest/clint.html>
+//! ACLINT Specification: <https://github.com/riscvarchive/riscv-aclint/blob/main/riscv-aclint.adoc>
 
 pub mod mswi;
 pub mod mtimer;
@@ -15,48 +15,78 @@ pub use riscv_pac::HartIdNumber; // re-export useful riscv-pac traits
 ///
 /// * This trait must only be implemented on a PAC of a target with a CLINT peripheral.
 /// * The CLINT peripheral base address `BASE` must be valid for the target device.
+/// * The CLINT peripheral clock frequency `MTIME_FREQ` must be valid for the target device.
 pub unsafe trait Clint: Copy {
     /// Base address of the CLINT peripheral.
     const BASE: usize;
+    /// Clock frequency of the CLINT's [`MTIME`](mtimer::MTIME) register.
+    const MTIME_FREQ: usize;
 }
 
 /// Interface for a CLINT peripheral.
 ///
 /// The RISC-V standard does not specify a fixed location for the CLINT.
 /// Thus, each platform must specify the base address of the CLINT on the platform.
-/// The base address, as well as all the associated types, are defined in the [`Clint`] trait.
+/// The base address and clock frequency are defined in the [`Clint`] trait.
 ///
 /// The CLINT standard allows up to 4_095 different HARTs connected to the CLINT.
 /// Each HART has an assigned index starting from 0 to up to 4_094.
 /// In this way, each HART's timer and software interrupts can be independently configured.
 #[allow(clippy::upper_case_acronyms)]
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub struct CLINT<C: Clint> {
+pub struct CLINT<C> {
     _marker: core::marker::PhantomData<C>,
 }
 
 impl<C: Clint> CLINT<C> {
-    const MTIMECMP_OFFSET: usize = 0x4000;
+    #[inline]
+    /// Creates a new `CLINT` peripheral.
+    pub const fn new() -> Self {
+        Self {
+            _marker: core::marker::PhantomData,
+        }
+    }
 
-    const MTIME_OFFSET: usize = 0xBFF8;
+    /// Returns the [`MSWI`](mswi::MSWI) device.
+    #[inline]
+    pub const fn mswi(&self) -> mswi::MSWI<C> {
+        mswi::MSWI::new()
+    }
 
-    /// Returns the `MSWI` peripheral.
+    /// Returns the [`MTIMER`](mtimer::MTIMER) device.
     #[inline]
-    pub const fn mswi() -> mswi::MSWI {
-        // SAFETY: valid base address
-        unsafe { mswi::MSWI::new(C::BASE) }
+    pub const fn mtimer(&self) -> mtimer::MTIMER<C> {
+        mtimer::MTIMER::new()
     }
 
-    /// Returns the `MTIMER` peripheral.
+    /// Returns `true` if a machine timer **OR** software interrupt is pending.
     #[inline]
-    pub const fn mtimer() -> mtimer::MTIMER {
-        // SAFETY: valid base address
-        unsafe {
-            mtimer::MTIMER::new(
-                C::BASE + Self::MTIMECMP_OFFSET,
-                C::BASE + Self::MTIME_OFFSET,
-            )
-        }
+    pub fn is_interrupting(&self) -> bool {
+        self.mswi().is_interrupting() || self.mtimer().is_interrupting()
+    }
+
+    /// Returns `true` if machine timer **OR** software interrupts are enabled.
+    #[inline]
+    pub fn is_enabled(&self) -> bool {
+        self.mswi().is_enabled() || self.mtimer().is_enabled()
+    }
+
+    /// Enables machine timer **AND** software interrupts to allow the CLINT to trigger interrupts.
+    ///
+    /// # Safety
+    ///
+    /// Enabling the `CLINT` may break mask-based critical sections.
+    #[inline]
+    pub unsafe fn enable(&self) {
+        self.mswi().enable();
+        self.mtimer().enable();
+    }
+
+    /// Disables machine timer **AND** software interrupts to prevent the CLINT from triggering interrupts.
+    #[inline]
+    pub fn disable(&self) {
+        self.mswi().disable();
+        self.mtimer().disable();
     }
 }
 
@@ -111,16 +141,22 @@ pub(crate) mod test {
         // Call CLINT macro with a base address and a list of mtimecmps for easing access to per-HART mtimecmp regs.
         crate::clint_codegen!(
             base 0x0200_0000,
-            mtimecmps [mtimecmp0=(HartId::H0,"`H0`"), mtimecmp1=(HartId::H1,"`H1`"), mtimecmp2=(HartId::H2,"`H2`")],
-            msips [msip0=(HartId::H0,"`H0`"), msip1=(HartId::H1,"`H1`"), msip2=(HartId::H2,"`H2`")],
+            mtime_freq 32_768,
+            harts [HartId::H0 => 0, HartId::H1 => 1, HartId::H2 => 2],
         );
 
-        let mswi = CLINT::mswi();
-        let mtimer = CLINT::mtimer();
+        let clint = CLINT::new();
+
+        let mswi = clint.mswi();
+        let mtimer = clint.mtimer();
 
-        assert_eq!(mswi.msip0.get_ptr() as usize, 0x0200_0000);
-        assert_eq!(mtimer.mtimecmp0.get_ptr() as usize, 0x0200_4000);
-        assert_eq!(mtimer.mtime.get_ptr() as usize, 0x0200_bff8);
+        let msip0 = mswi.msip(HartId::H0);
+        let msip1 = mswi.msip(HartId::H1);
+        let msip2 = mswi.msip(HartId::H2);
+
+        assert_eq!(msip0.get_ptr() as usize, 0x0200_0000);
+        assert_eq!(msip1.get_ptr() as usize, 0x0200_0000 + 4); // 4 bytes per register
+        assert_eq!(msip2.get_ptr() as usize, 0x0200_0000 + 2 * 4);
 
         let mtimecmp0 = mtimer.mtimecmp(HartId::H0);
         let mtimecmp1 = mtimer.mtimecmp(HartId::H1);
@@ -130,13 +166,15 @@ pub(crate) mod test {
         assert_eq!(mtimecmp1.get_ptr() as usize, 0x0200_4000 + 8); // 8 bytes per register
         assert_eq!(mtimecmp2.get_ptr() as usize, 0x0200_4000 + 2 * 8);
 
-        assert_eq!(CLINT::mtime(), mtimer.mtime);
-        assert_eq!(CLINT::mtimecmp0(), mtimer.mtimecmp(HartId::H0));
-        assert_eq!(CLINT::mtimecmp1(), mtimer.mtimecmp(HartId::H1));
-        assert_eq!(CLINT::mtimecmp2(), mtimer.mtimecmp(HartId::H2));
+        let mtime = mtimer.mtime();
+        assert_eq!(mtime.get_ptr() as usize, 0x0200_bff8);
+
+        assert_eq!(clint.mtimecmp0(), mtimer.mtimecmp(HartId::H0));
+        assert_eq!(clint.mtimecmp1(), mtimer.mtimecmp(HartId::H1));
+        assert_eq!(clint.mtimecmp2(), mtimer.mtimecmp(HartId::H2));
 
-        assert_eq!(CLINT::msip0(), mswi.msip(HartId::H0));
-        assert_eq!(CLINT::msip1(), mswi.msip(HartId::H1));
-        assert_eq!(CLINT::msip2(), mswi.msip(HartId::H2));
+        assert_eq!(clint.msip0(), mswi.msip(HartId::H0));
+        assert_eq!(clint.msip1(), mswi.msip(HartId::H1));
+        assert_eq!(clint.msip2(), mswi.msip(HartId::H2));
     }
 }