From a6c84cb91552fc0442a28126d3fae60031aa6622 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Tue, 19 Oct 2021 10:13:08 +0200 Subject: [PATCH] - Interconnect is now PPI again - Scary pointer math is now contained in the tasks and events - ppi now sets the tasks and events immediately and the struct is now zero-sized - StaticToOne is renamed to ZeroToOne - Used DPPI tasks and events now panic when enabled twice --- embassy-nrf/src/buffered_uarte.rs | 2 +- embassy-nrf/src/gpiote.rs | 2 +- embassy-nrf/src/interconnect/dppi.rs | 47 ------- embassy-nrf/src/interconnect/ppi.rs | 65 ---------- embassy-nrf/src/lib.rs | 2 +- embassy-nrf/src/ppi/dppi.rs | 61 +++++++++ embassy-nrf/src/{interconnect => ppi}/mod.rs | 125 ++++++++++--------- embassy-nrf/src/ppi/ppi.rs | 77 ++++++++++++ embassy-nrf/src/timer.rs | 2 +- embassy-nrf/src/uarte.rs | 2 +- 10 files changed, 209 insertions(+), 176 deletions(-) delete mode 100644 embassy-nrf/src/interconnect/dppi.rs delete mode 100644 embassy-nrf/src/interconnect/ppi.rs create mode 100644 embassy-nrf/src/ppi/dppi.rs rename embassy-nrf/src/{interconnect => ppi}/mod.rs (79%) create mode 100644 embassy-nrf/src/ppi/ppi.rs diff --git a/embassy-nrf/src/buffered_uarte.rs b/embassy-nrf/src/buffered_uarte.rs index 7c9a2344e..1dc04f4f6 100644 --- a/embassy-nrf/src/buffered_uarte.rs +++ b/embassy-nrf/src/buffered_uarte.rs @@ -14,8 +14,8 @@ use embassy_hal_common::{low_power_wait_until, unborrow}; use crate::gpio::sealed::Pin as _; use crate::gpio::{OptionalPin as GpioOptionalPin, Pin as GpioPin}; -use crate::interconnect::{AnyChannel, Event, OneToOneChannel, OneToTwoChannel, Ppi, Task}; use crate::pac; +use crate::ppi::{AnyChannel, Event, OneToOneChannel, OneToTwoChannel, Ppi, Task}; use crate::timer::Instance as TimerInstance; use crate::timer::{Frequency, Timer}; use crate::uarte::{Config, Instance as UarteInstance}; diff --git a/embassy-nrf/src/gpiote.rs b/embassy-nrf/src/gpiote.rs index 6703bfc60..7ec072ac8 100644 --- a/embassy-nrf/src/gpiote.rs +++ b/embassy-nrf/src/gpiote.rs @@ -11,8 +11,8 @@ use futures::future::poll_fn; use crate::gpio::sealed::Pin as _; use crate::gpio::{AnyPin, Input, Output, Pin as GpioPin}; -use crate::interconnect::{Event, Task}; use crate::pac; +use crate::ppi::{Event, Task}; use crate::{interrupt, peripherals}; pub const CHANNEL_COUNT: usize = 8; diff --git a/embassy-nrf/src/interconnect/dppi.rs b/embassy-nrf/src/interconnect/dppi.rs deleted file mode 100644 index 60f19fca0..000000000 --- a/embassy-nrf/src/interconnect/dppi.rs +++ /dev/null @@ -1,47 +0,0 @@ -use super::{Channel, Event, Ppi, Task}; - -const DPPI_ENABLE_BIT: u32 = 0x8000_0000; -const DPPI_CHANNEL_MASK: u32 = 0x0000_00FF; -const REGISTER_DPPI_CONFIG_OFFSET: usize = 0x80 / core::mem::size_of::(); - -impl<'d, C: Channel, 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) { - unsafe { - task.0 - .as_ptr() - .add(REGISTER_DPPI_CONFIG_OFFSET) - .write_volatile(DPPI_ENABLE_BIT | (channel.number() as u32 & DPPI_CHANNEL_MASK)); - } - } - - pub(super) fn disable_task(task: &Task, _channel: &C, _index: usize) { - unsafe { - task.0 - .as_ptr() - .add(REGISTER_DPPI_CONFIG_OFFSET) - .write_volatile(0); - } - } - - pub(super) fn enable_event(event: &Event, channel: &C, _index: usize) { - unsafe { - event - .0 - .as_ptr() - .add(REGISTER_DPPI_CONFIG_OFFSET) - .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 - .0 - .as_ptr() - .add(REGISTER_DPPI_CONFIG_OFFSET) - .write_volatile(0); - } - } -} diff --git a/embassy-nrf/src/interconnect/ppi.rs b/embassy-nrf/src/interconnect/ppi.rs deleted file mode 100644 index 91ff811fe..000000000 --- a/embassy-nrf/src/interconnect/ppi.rs +++ /dev/null @@ -1,65 +0,0 @@ -use super::{Channel, Event, Ppi, Task}; -use crate::pac; - -impl<'d, C: Channel + 'd, 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) { - match (index, channel.is_task_configurable()) { - (0, false) => Self::set_fork_task(Some(task), channel.number()), // Static channel with fork - (0, true) => Self::set_main_task(Some(task), channel.number()), // Configurable channel without fork - (1, true) => Self::set_fork_task(Some(task), channel.number()), // Configurable channel with fork - _ => unreachable!("{}, {}", index, channel.is_task_configurable()), // Not available with the PPI, so should not be constructable - } - } - - pub(super) fn disable_task(_task: &Task, channel: &C, index: usize) { - match (index, channel.is_task_configurable()) { - (0, false) => Self::set_fork_task(None, channel.number()), // Static channel with fork - (0, true) => Self::set_main_task(None, channel.number()), // Configurable channel without fork - (1, true) => Self::set_fork_task(None, channel.number()), // Configurable channel with fork - _ => unreachable!(), // Not available with the PPI, so should not be constructable - } - } - - pub(super) fn enable_event(event: &Event, channel: &C, _index: usize) { - Self::set_event(Some(event), channel.number()) - } - - pub(super) fn disable_event(_event: &Event, channel: &C, _index: usize) { - Self::set_event(None, channel.number()) - } - - fn set_main_task(task: Option<&Task>, channel: usize) { - let r = unsafe { &*pac::PPI::ptr() }; - if let Some(task) = task { - r.ch[channel] - .tep - .write(|w| unsafe { w.bits(task.0.as_ptr() as u32) }) - } else { - r.ch[channel].tep.write(|w| unsafe { w.bits(0) }) - } - } - - 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) }) - } - } -} diff --git a/embassy-nrf/src/lib.rs b/embassy-nrf/src/lib.rs index 39f3e4a4a..c21c4d398 100644 --- a/embassy-nrf/src/lib.rs +++ b/embassy-nrf/src/lib.rs @@ -31,7 +31,7 @@ pub mod gpio; pub mod gpiote; #[cfg(not(feature = "nrf9160"))] pub mod nvmc; -pub mod interconnect; +pub mod ppi; #[cfg(not(any(feature = "nrf52805", feature = "nrf52820")))] pub mod pwm; #[cfg(feature = "nrf52840")] diff --git a/embassy-nrf/src/ppi/dppi.rs b/embassy-nrf/src/ppi/dppi.rs new file mode 100644 index 000000000..083ec858a --- /dev/null +++ b/embassy-nrf/src/ppi/dppi.rs @@ -0,0 +1,61 @@ +use super::{Channel, 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> + Ppi<'d, C, EVENT_COUNT, TASK_COUNT> +{ + pub(super) fn enable_task(task: &Task, channel: &C, _index: usize) { + unsafe { + if task.subscribe_reg().read_volatile() != 0 { + panic!("Task is already in use"); + } + task.subscribe_reg() + .write_volatile(DPPI_ENABLE_BIT | (channel.number() as u32 & DPPI_CHANNEL_MASK)); + } + } + + pub(super) fn disable_task(task: &Task, _channel: &C, _index: usize) { + unsafe { + task.subscribe_reg().write_volatile(0); + } + } + + pub(super) fn enable_event(event: &Event, channel: &C, _index: usize) { + unsafe { + if event.publish_reg().read_volatile() != 0 { + panic!("Task is already in use"); + } + event + .publish_reg() + .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); + } + } +} diff --git a/embassy-nrf/src/interconnect/mod.rs b/embassy-nrf/src/ppi/mod.rs similarity index 79% rename from embassy-nrf/src/interconnect/mod.rs rename to embassy-nrf/src/ppi/mod.rs index 8cb505bdc..ffb6af020 100644 --- a/embassy-nrf/src/interconnect/mod.rs +++ b/embassy-nrf/src/ppi/mod.rs @@ -28,12 +28,14 @@ mod ppi; pub struct Ppi<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> { ch: C, + #[cfg(feature = "_dppi")] events: [Event; EVENT_COUNT], + #[cfg(feature = "_dppi")] tasks: [Task; TASK_COUNT], phantom: PhantomData<&'d mut C>, } -impl<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> +impl<'d, C: Channel + 'd, const EVENT_COUNT: usize, const TASK_COUNT: usize> Ppi<'d, C, EVENT_COUNT, TASK_COUNT> { pub fn degrade(self) -> Ppi<'d, AnyChannel, EVENT_COUNT, TASK_COUNT> { @@ -43,7 +45,9 @@ impl<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> #[cfg(feature = "_ppi")] has_configurable_task: self.ch.is_task_configurable(), }, + #[cfg(feature = "_dppi")] events: self.events, + #[cfg(feature = "_dppi")] tasks: self.tasks, phantom: PhantomData, } @@ -62,26 +66,6 @@ impl<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> r.chenclr .write(|w| unsafe { w.bits(1 << self.ch.number()) }); } - - /// Enables all tasks and events - fn enable_all(&self) { - for (index, task) in self.tasks.iter().enumerate() { - Self::enable_task(task, &self.ch, index); - } - for (index, event) in self.events.iter().enumerate() { - Self::enable_event(event, &self.ch, index); - } - } - - /// Disable all tasks and events - 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); - } - } } impl<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> Drop @@ -93,19 +77,23 @@ impl<'d, C: Channel, const EVENT_COUNT: usize, const TASK_COUNT: usize> Drop } } -impl<'d, C: StaticToOneChannel> Ppi<'d, C, 0, 1> { +impl<'d, C: ZeroToOneChannel> Ppi<'d, C, 0, 1> { pub fn new_static_to_one(ch: impl Unborrow + 'd, task: Task) -> Self { unborrow!(ch); - let s = Self { - ch, - events: [], - tasks: [task], - phantom: PhantomData, - }; + let events = []; + let tasks = [task]; - s.enable_all(); - s + Self::enable_all(&tasks, &events, &ch); + + Self { + ch, + #[cfg(feature = "_dppi")] + events, + #[cfg(feature = "_dppi")] + tasks, + phantom: PhantomData, + } } } @@ -113,15 +101,19 @@ impl<'d, C: OneToOneChannel> Ppi<'d, C, 1, 1> { pub fn new_one_to_one(ch: impl Unborrow + 'd, event: Event, task: Task) -> Self { unborrow!(ch); - let s = Self { - ch, - events: [event], - tasks: [task], - phantom: PhantomData, - }; + let events = [event]; + let tasks = [task]; - s.enable_all(); - s + Self::enable_all(&tasks, &events, &ch); + + Self { + ch, + #[cfg(feature = "_dppi")] + events, + #[cfg(feature = "_dppi")] + tasks, + phantom: PhantomData, + } } } @@ -134,15 +126,19 @@ impl<'d, C: OneToTwoChannel> Ppi<'d, C, 1, 2> { ) -> Self { unborrow!(ch); - let s = Self { - ch, - events: [event], - tasks: [task1, task2], - phantom: PhantomData, - }; + let events = [event]; + let tasks = [task1, task2]; - s.enable_all(); - s + Self::enable_all(&tasks, &events, &ch); + + Self { + ch, + #[cfg(feature = "_dppi")] + events, + #[cfg(feature = "_dppi")] + tasks, + phantom: PhantomData, + } } } @@ -156,18 +152,21 @@ impl<'d, C: ManyToManyChannel, const EVENT_COUNT: usize, const TASK_COUNT: usize ) -> Self { unborrow!(ch); - let s = Self { + Self::enable_all(&tasks, &events, &ch); + + Self { ch, + #[cfg(feature = "_dppi")] events, + #[cfg(feature = "_dppi")] tasks, phantom: PhantomData, - }; - - s.enable_all(); - s + } } } +const REGISTER_DPPI_CONFIG_OFFSET: usize = 0x80 / core::mem::size_of::(); + /// Represents a task that a peripheral can do. /// When a task is subscribed to a PPI channel it will run when the channel is triggered by /// a published event. @@ -179,6 +178,10 @@ impl Task { pub(crate) fn from_reg(reg: &T) -> Self { Self(unsafe { NonNull::new_unchecked(reg as *const _ as *mut _) }) } + + pub fn subscribe_reg(&self) -> *mut u32 { + unsafe { self.0.as_ptr().add(REGISTER_DPPI_CONFIG_OFFSET) } + } } /// # Safety @@ -196,6 +199,10 @@ impl Event { pub(crate) fn from_reg(reg: &T) -> Self { Self(unsafe { NonNull::new_unchecked(reg as *const _ as *mut _) }) } + + pub fn publish_reg(&self) -> *mut u32 { + unsafe { self.0.as_ptr().add(REGISTER_DPPI_CONFIG_OFFSET) } + } } /// # Safety @@ -218,8 +225,8 @@ pub trait Channel: sealed::Channel + Unborrow + Sized { fn is_task_configurable(&self) -> bool; } -pub trait StaticToOneChannel: Channel {} -pub trait OneToOneChannel: StaticToOneChannel {} +pub trait ZeroToOneChannel: Channel {} +pub trait OneToOneChannel: ZeroToOneChannel {} pub trait OneToTwoChannel: OneToOneChannel {} pub trait ManyToManyChannel: OneToTwoChannel {} @@ -250,8 +257,8 @@ impl Channel for AnyChannel { macro_rules! impl_ppi_channel { ($type:ident, $number:expr, $has_configurable_task:expr) => { - impl crate::interconnect::sealed::Channel for peripherals::$type {} - impl crate::interconnect::Channel for peripherals::$type { + impl crate::ppi::sealed::Channel for peripherals::$type {} + impl crate::ppi::Channel for peripherals::$type { fn number(&self) -> usize { $number } @@ -267,19 +274,19 @@ macro_rules! impl_ppi_channel { }; ($type:ident, $number:expr, $has_configurable_task:expr, 0, 1) => { impl_ppi_channel!($type, $number, $has_configurable_task, 0, 0); - impl crate::interconnect::StaticToOneChannel for peripherals::$type {} + impl crate::ppi::ZeroToOneChannel for peripherals::$type {} }; ($type:ident, $number:expr, $has_configurable_task:expr, 1, 1) => { impl_ppi_channel!($type, $number, $has_configurable_task, 0, 1); - impl crate::interconnect::OneToOneChannel for peripherals::$type {} + impl crate::ppi::OneToOneChannel for peripherals::$type {} }; ($type:ident, $number:expr, $has_configurable_task:expr, 1, 2) => { impl_ppi_channel!($type, $number, $has_configurable_task, 1, 1); - impl crate::interconnect::OneToTwoChannel for peripherals::$type {} + impl crate::ppi::OneToTwoChannel for peripherals::$type {} }; ($type:ident, $number:expr, $has_configurable_task:expr, many, many) => { impl_ppi_channel!($type, $number, $has_configurable_task, 1, 2); - impl crate::interconnect::ManyToManyChannel for peripherals::$type {} + impl crate::ppi::ManyToManyChannel for peripherals::$type {} }; } diff --git a/embassy-nrf/src/ppi/ppi.rs b/embassy-nrf/src/ppi/ppi.rs new file mode 100644 index 000000000..67d2086c3 --- /dev/null +++ b/embassy-nrf/src/ppi/ppi.rs @@ -0,0 +1,77 @@ +use super::{Channel, Event, Ppi, Task}; +use crate::pac; + +impl<'d, C: Channel + 'd, const EVENT_COUNT: usize, const TASK_COUNT: usize> + Ppi<'d, C, EVENT_COUNT, TASK_COUNT> +{ + fn set_main_task(task: Option<&Task>, channel: usize) { + let r = unsafe { &*pac::PPI::ptr() }; + if let Some(task) = task { + r.ch[channel] + .tep + .write(|w| unsafe { w.bits(task.0.as_ptr() as u32) }) + } else { + r.ch[channel].tep.write(|w| unsafe { w.bits(0) }) + } + } + + 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 channel.is_task_configurable() && TASK_COUNT == 1 { + Self::set_main_task(Some(&tasks[0]), channel.number()); + } + + // One configurable task, as fork + if !channel.is_task_configurable() && TASK_COUNT == 1 { + Self::set_fork_task(Some(&tasks[0]), channel.number()); + } + + // Two configurable tasks (main + fork) + 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 self.ch.is_task_configurable() { + Self::set_main_task(None, self.ch.number()); + } + + if TASK_COUNT == 1 && !self.ch.is_task_configurable() || TASK_COUNT == 2 { + Self::set_fork_task(None, self.ch.number()); + } + + if EVENT_COUNT == 1 { + Self::set_event(None, self.ch.number()); + } + } +} diff --git a/embassy-nrf/src/timer.rs b/embassy-nrf/src/timer.rs index 27f8e715e..9173338b6 100644 --- a/embassy-nrf/src/timer.rs +++ b/embassy-nrf/src/timer.rs @@ -11,8 +11,8 @@ use embassy_hal_common::drop::OnDrop; use embassy_hal_common::unborrow; use futures::future::poll_fn; -use crate::interconnect::{Event, Task}; use crate::pac; +use crate::ppi::{Event, Task}; pub(crate) mod sealed { diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs index 17e60ce22..36cf65d89 100644 --- a/embassy-nrf/src/uarte.rs +++ b/embassy-nrf/src/uarte.rs @@ -16,9 +16,9 @@ use futures::future::poll_fn; use crate::chip::EASY_DMA_SIZE; use crate::gpio::sealed::Pin as _; use crate::gpio::{self, OptionalPin as GpioOptionalPin, Pin as GpioPin}; -use crate::interconnect::{AnyChannel, Event, OneToOneChannel, OneToTwoChannel, Ppi, Task}; use crate::interrupt::Interrupt; use crate::pac; +use crate::ppi::{AnyChannel, Event, OneToOneChannel, OneToTwoChannel, Ppi, Task}; use crate::timer::Instance as TimerInstance; use crate::timer::{Frequency, Timer};