ppi: simplify driver creation.

Moving `new_*` to the version-specific mod allows doing the correct
register writes right there in `new`, without needing abstractions
like `enable_all`/`disable_all`.

embassy-nrf/src/ppi/dppi.rs

@@ -1,61 +1,74 @@
-use super::{Channel, Event, Ppi, Task};
+use core::marker::PhantomData;
+
+use embassy::util::Unborrow;
+use embassy_hal_common::unborrow;
+
+use super::{Channel, ConfigurableChannel, Event, Ppi, Task};
 
 const DPPI_ENABLE_BIT: u32 = 0x8000_0000;
 const DPPI_CHANNEL_MASK: u32 = 0x0000_00FF;
 
-impl<'d, C: Channel + 'd, const EVENT_COUNT: usize, const TASK_COUNT: usize>
+impl<'d, C: ConfigurableChannel> Ppi<'d, C, 1, 1> {
+    pub fn new_one_to_one(ch: impl Unborrow<Target = C> + 'd, event: Event, task: Task) -> Self {
+        Ppi::new_many_to_many(ch, [event], [task])
+    }
+}
+
+impl<'d, C: ConfigurableChannel> Ppi<'d, C, 1, 2> {
+    pub fn new_one_to_two(
+        ch: impl Unborrow<Target = C> + 'd,
+        event: Event,
+        task1: Task,
+        task2: Task,
+    ) -> Self {
+        Ppi::new_many_to_many(ch, [event], [task1, task2])
+    }
+}
+
+impl<'d, C: ConfigurableChannel, const EVENT_COUNT: usize, const TASK_COUNT: usize>
     Ppi<'d, C, EVENT_COUNT, TASK_COUNT>
 {
-    pub(super) fn enable_task(task: &Task, channel: &C, _index: usize) {
+    pub fn new_many_to_many(
-        unsafe {
+        ch: impl Unborrow<Target = C> + 'd,
-            if task.subscribe_reg().read_volatile() != 0 {
+        events: [Event; EVENT_COUNT],
+        tasks: [Task; TASK_COUNT],
+    ) -> Self {
+        unborrow!(ch);
+
+        let val = DPPI_ENABLE_BIT | (ch.number() as u32 & DPPI_CHANNEL_MASK);
+        for task in tasks {
+            if unsafe { task.subscribe_reg().read_volatile() } != 0 {
                 panic!("Task is already in use");
             }
-            task.subscribe_reg()
+            unsafe { task.subscribe_reg().write_volatile(val) }
-                .write_volatile(DPPI_ENABLE_BIT | (channel.number() as u32 & DPPI_CHANNEL_MASK));
+        }
+        for event in events {
+            if unsafe { event.publish_reg().read_volatile() } != 0 {
+                panic!("Event is already in use");
+            }
+            unsafe { event.publish_reg().write_volatile(val) }
+        }
+
+        Self {
+            ch,
+            events,
+            tasks,
+            phantom: PhantomData,
+        }
     }
 }
 
-    pub(super) fn disable_task(task: &Task, _channel: &C, _index: usize) {
+impl<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> Drop
-        unsafe {
+    for Ppi<'d, C, EVENT_COUNT, TASK_COUNT>
-            task.subscribe_reg().write_volatile(0);
+{
-        }
+    fn drop(&mut self) {
-    }
+        self.disable();
 
-    pub(super) fn enable_event(event: &Event, channel: &C, _index: usize) {
+        for task in self.tasks {
-        unsafe {
+            unsafe { task.subscribe_reg().write_volatile(0) }
-            if event.publish_reg().read_volatile() != 0 {
-                panic!("Task is already in use");
         }
-            event
+        for event in self.events {
-                .publish_reg()
+            unsafe { event.publish_reg().write_volatile(0) }
-                .write_volatile(DPPI_ENABLE_BIT | (channel.number() as u32 & DPPI_CHANNEL_MASK));
-        }
-    }
-
-    pub(super) fn disable_event(event: &Event, _channel: &C, _index: usize) {
-        unsafe {
-            event.publish_reg().write_volatile(0);
-        }
-    }
-
-    /// Enables all tasks and events
-    pub(super) fn enable_all(tasks: &[Task], events: &[Event], channel: &C) {
-        for (index, task) in tasks.iter().enumerate() {
-            Self::enable_task(task, channel, index);
-        }
-        for (index, event) in events.iter().enumerate() {
-            Self::enable_event(event, channel, index);
-        }
-    }
-
-    /// Disable all tasks and events
-    pub(super) fn disable_all(&self) {
-        for (index, task) in self.tasks.iter().enumerate() {
-            Self::disable_task(task, &self.ch, index);
-        }
-        for (index, event) in self.events.iter().enumerate() {
-            Self::disable_event(event, &self.ch, index);
         }
     }
 }

embassy-nrf/src/ppi/mod.rs

@@ -19,7 +19,7 @@ use crate::{pac, peripherals};
 use core::marker::PhantomData;
 use core::ptr::NonNull;
 use embassy::util::Unborrow;
-use embassy_hal_common::{unborrow, unsafe_impl_unborrow};
+use embassy_hal_common::unsafe_impl_unborrow;
 
 #[cfg(feature = "_dppi")]
 mod dppi;
@@ -53,106 +53,6 @@ impl<'d, C: Channel + 'd, const EVENT_COUNT: usize, const TASK_COUNT: usize>
     }
 }
 
-impl<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> Drop
-    for Ppi<'d, C, EVENT_COUNT, TASK_COUNT>
-{
-    fn drop(&mut self) {
-        self.disable();
-        self.disable_all();
-    }
-}
-
-#[cfg(not(feature = "nrf51"))] // Not for nrf51 because of the fork task
-impl<'d, C: StaticChannel> Ppi<'d, C, 0, 1> {
-    pub fn new_zero_to_one(ch: impl Unborrow<Target = C> + 'd, task: Task) -> Self {
-        unborrow!(ch);
-
-        let events = [];
-        let tasks = [task];
-
-        Self::enable_all(&tasks, &events, &ch);
-
-        Self {
-            ch,
-            #[cfg(feature = "_dppi")]
-            events,
-            #[cfg(feature = "_dppi")]
-            tasks,
-            phantom: PhantomData,
-        }
-    }
-}
-
-impl<'d, C: ConfigurableChannel> Ppi<'d, C, 1, 1> {
-    pub fn new_one_to_one(ch: impl Unborrow<Target = C> + 'd, event: Event, task: Task) -> Self {
-        unborrow!(ch);
-
-        let events = [event];
-        let tasks = [task];
-
-        Self::enable_all(&tasks, &events, &ch);
-
-        Self {
-            ch,
-            #[cfg(feature = "_dppi")]
-            events,
-            #[cfg(feature = "_dppi")]
-            tasks,
-            phantom: PhantomData,
-        }
-    }
-}
-
-#[cfg(not(feature = "nrf51"))] // Not for nrf51 because of the fork task
-impl<'d, C: ConfigurableChannel> Ppi<'d, C, 1, 2> {
-    pub fn new_one_to_two(
-        ch: impl Unborrow<Target = C> + 'd,
-        event: Event,
-        task1: Task,
-        task2: Task,
-    ) -> Self {
-        unborrow!(ch);
-
-        let events = [event];
-        let tasks = [task1, task2];
-
-        Self::enable_all(&tasks, &events, &ch);
-
-        Self {
-            ch,
-            #[cfg(feature = "_dppi")]
-            events,
-            #[cfg(feature = "_dppi")]
-            tasks,
-            phantom: PhantomData,
-        }
-    }
-}
-
-#[cfg(feature = "_dppi")]
-impl<'d, C: ConfigurableChannel, const EVENT_COUNT: usize, const TASK_COUNT: usize>
-    Ppi<'d, C, EVENT_COUNT, TASK_COUNT>
-{
-    pub fn new_many_to_many(
-        ch: impl Unborrow<Target = C> + 'd,
-        events: [Event; EVENT_COUNT],
-        tasks: [Task; TASK_COUNT],
-    ) -> Self {
-        unborrow!(ch);
-
-        Self::enable_all(&tasks, &events, &ch);
-
-        Self {
-            ch,
-            #[cfg(feature = "_dppi")]
-            events,
-            #[cfg(feature = "_dppi")]
-            tasks,
-            phantom: PhantomData,
-        }
-    }
-}
-
 const REGISTER_DPPI_CONFIG_OFFSET: usize = 0x80 / core::mem::size_of::<u32>();
 
 /// Represents a task that a peripheral can do.
@@ -209,7 +109,6 @@ pub(crate) mod sealed {
 pub trait Channel: sealed::Channel + Unborrow<Target = Self> + Sized {
     /// Returns the number of the channel
     fn number(&self) -> usize;
-    fn configurable() -> bool;
 }
 
 pub trait ConfigurableChannel: Channel {
@@ -243,10 +142,6 @@ impl Channel for AnyStaticChannel {
     fn number(&self) -> usize {
         self.number as usize
     }
-
-    fn configurable() -> bool {
-        false
-    }
 }
 impl StaticChannel for AnyStaticChannel {
     fn degrade(self) -> AnyStaticChannel {
@@ -265,10 +160,6 @@ impl Channel for AnyConfigurableChannel {
     fn number(&self) -> usize {
         self.number as usize
     }
-
-    fn configurable() -> bool {
-        true
-    }
 }
 impl ConfigurableChannel for AnyConfigurableChannel {
     fn degrade(self) -> AnyConfigurableChannel {
@@ -277,20 +168,16 @@ impl ConfigurableChannel for AnyConfigurableChannel {
 }
 
 macro_rules! impl_ppi_channel {
-    ($type:ident, $number:expr, $configurability:expr) => {
+    ($type:ident, $number:expr) => {
         impl crate::ppi::sealed::Channel for peripherals::$type {}
         impl crate::ppi::Channel for peripherals::$type {
             fn number(&self) -> usize {
                 $number
             }
-
-            fn configurable() -> bool {
-                $configurability
-            }
         }
     };
     ($type:ident, $number:expr => static) => {
-        impl_ppi_channel!($type, $number, false);
+        impl_ppi_channel!($type, $number);
         impl crate::ppi::StaticChannel for peripherals::$type {
             fn degrade(self) -> crate::ppi::AnyStaticChannel {
                 use crate::ppi::Channel;
@@ -301,7 +188,7 @@ macro_rules! impl_ppi_channel {
         }
     };
     ($type:ident, $number:expr => configurable) => {
-        impl_ppi_channel!($type, $number, true);
+        impl_ppi_channel!($type, $number);
         impl crate::ppi::ConfigurableChannel for peripherals::$type {
             fn degrade(self) -> crate::ppi::AnyConfigurableChannel {
                 use crate::ppi::Channel;

embassy-nrf/src/ppi/ppi.rs

@@ -1,81 +1,87 @@
-use super::{Channel, Event, Ppi, Task};
+use core::marker::PhantomData;
+
+use embassy::util::Unborrow;
+use embassy_hal_common::unborrow;
+
+use super::{Channel, ConfigurableChannel, Event, Ppi, StaticChannel, Task};
 use crate::pac;
 
-impl<'d, C: Channel + 'd, const EVENT_COUNT: usize, const TASK_COUNT: usize>
+impl Task {
-    Ppi<'d, C, EVENT_COUNT, TASK_COUNT>
+    fn reg_val(&self) -> u32 {
+        self.0.as_ptr() as _
+    }
+}
+impl Event {
+    fn reg_val(&self) -> u32 {
+        self.0.as_ptr() as _
+    }
+}
+
+#[cfg(not(feature = "nrf51"))] // Not for nrf51 because of the fork task
+impl<'d, C: StaticChannel> Ppi<'d, C, 0, 1> {
+    pub fn new_zero_to_one(ch: impl Unborrow<Target = C> + 'd, task: Task) -> Self {
+        unborrow!(ch);
+
+        let r = unsafe { &*pac::PPI::ptr() };
+        let n = ch.number();
+        r.fork[n].tep.write(|w| unsafe { w.bits(task.reg_val()) });
+
+        Self {
+            ch,
+            phantom: PhantomData,
+        }
+    }
+}
+
+impl<'d, C: ConfigurableChannel> Ppi<'d, C, 1, 1> {
+    pub fn new_one_to_one(ch: impl Unborrow<Target = C> + 'd, event: Event, task: Task) -> Self {
+        unborrow!(ch);
+
+        let r = unsafe { &*pac::PPI::ptr() };
+        let n = ch.number();
+        r.ch[n].eep.write(|w| unsafe { w.bits(event.reg_val()) });
+        r.ch[n].tep.write(|w| unsafe { w.bits(task.reg_val()) });
+
+        Self {
+            ch,
+            phantom: PhantomData,
+        }
+    }
+}
+
+#[cfg(not(feature = "nrf51"))] // Not for nrf51 because of the fork task
+impl<'d, C: ConfigurableChannel> Ppi<'d, C, 1, 2> {
+    pub fn new_one_to_two(
+        ch: impl Unborrow<Target = C> + 'd,
+        event: Event,
+        task1: Task,
+        task2: Task,
+    ) -> Self {
+        unborrow!(ch);
+
+        let r = unsafe { &*pac::PPI::ptr() };
+        let n = ch.number();
+        r.ch[n].eep.write(|w| unsafe { w.bits(event.reg_val()) });
+        r.ch[n].tep.write(|w| unsafe { w.bits(task1.reg_val()) });
+        r.fork[n].tep.write(|w| unsafe { w.bits(task2.reg_val()) });
+
+        Self {
+            ch,
+            phantom: PhantomData,
+        }
+    }
+}
+
+impl<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> Drop
+    for Ppi<'d, C, EVENT_COUNT, TASK_COUNT>
 {
-    fn set_main_task(task: Option<&Task>, channel: usize) {
+    fn drop(&mut self) {
+        self.disable();
+
         let r = unsafe { &*pac::PPI::ptr() };
-        if let Some(task) = task {
+        let n = self.ch.number();
-            r.ch[channel]
+        r.ch[n].eep.write(|w| unsafe { w.bits(0) });
-                .tep
+        r.ch[n].tep.write(|w| unsafe { w.bits(0) });
-                .write(|w| unsafe { w.bits(task.0.as_ptr() as u32) })
+        r.fork[n].tep.write(|w| unsafe { w.bits(0) });
-        } else {
-            r.ch[channel].tep.write(|w| unsafe { w.bits(0) })
-        }
-    }
-
-    #[cfg(not(feature = "nrf51"))]
-    fn set_fork_task(task: Option<&Task>, channel: usize) {
-        let r = unsafe { &*pac::PPI::ptr() };
-        if let Some(task) = task {
-            r.fork[channel]
-                .tep
-                .write(|w| unsafe { w.bits(task.0.as_ptr() as u32) })
-        } else {
-            r.fork[channel].tep.write(|w| unsafe { w.bits(0) })
-        }
-    }
-
-    fn set_event(event: Option<&Event>, channel: usize) {
-        let r = unsafe { &*pac::PPI::ptr() };
-        if let Some(event) = event {
-            r.ch[channel]
-                .eep
-                .write(|w| unsafe { w.bits(event.0.as_ptr() as u32) })
-        } else {
-            r.ch[channel].eep.write(|w| unsafe { w.bits(0) })
-        }
-    }
-
-    /// Enables all tasks and events
-    pub(super) fn enable_all(tasks: &[Task], events: &[Event], channel: &C) {
-        // One configurable task, no fork
-        if C::configurable() && TASK_COUNT == 1 {
-            Self::set_main_task(Some(&tasks[0]), channel.number());
-        }
-
-        // One configurable task, as fork
-        #[cfg(not(feature = "nrf51"))]
-        if !C::configurable() && TASK_COUNT == 1 {
-            Self::set_fork_task(Some(&tasks[0]), channel.number());
-        }
-
-        // Two configurable tasks (main + fork)
-        #[cfg(not(feature = "nrf51"))]
-        if TASK_COUNT == 2 {
-            Self::set_main_task(Some(&tasks[0]), channel.number());
-            Self::set_fork_task(Some(&tasks[1]), channel.number());
-        }
-
-        if EVENT_COUNT == 1 {
-            Self::set_event(Some(&events[0]), channel.number());
-        }
-    }
-
-    /// Disable all tasks and events
-    pub(super) fn disable_all(&self) {
-        if C::configurable() {
-            Self::set_main_task(None, self.ch.number());
-        }
-
-        #[cfg(not(feature = "nrf51"))]
-        if TASK_COUNT == 1 && !C::configurable() || TASK_COUNT == 2 {
-            Self::set_fork_task(None, self.ch.number());
-        }
-
-        if EVENT_COUNT == 1 {
-            Self::set_event(None, self.ch.number());
-        }
     }
 }