Merge #486

486: Pwm ppi events r=Dirbaio a=jacobrosenthal

More PWM yak shaving. I was going to do some safe pwm ppi events stuff but I just dont think it fits this api design.. ppi is just very low level, im not sure how safe it will be in general

* first we should probably have borrows of handlers for ppi with lifetime of the peripheral?  hal does https://github.com/nrf-rs/nrf-hal/blob/eb4ba6ae4204c7f58fc968e57b2995df15f5ac77/nrf-hal-common/src/pwm.rs#L714-L716
* in general having access to tasks can put the state in some configuration the api doesnt understand anymore. for `SequencePwm` ideally id hand you back either only seq_start0 or seq_start1 because youd only use one based on if your `Times` is even or odd.. but again we only know that with this api AFTER start has been called. I dont think were ready for typestates

SO I figured why not add the pwm ppi events but make them unsafe and commit this example since I started it.

Somewhat related drop IS removing the last duty cycle from the pin correctly, but stop DOES NOT..the only thing that sets the pin back is pin.conf() as far as I can tell, so I tried to document that better and got rid of stop for the `SimplePwm` again since that doesnt need it then. However its ackward we dont have a way to unset the pwm without setting a new sequence of 0s, or dropping the peripheral


Co-authored-by: Jacob Rosenthal <[email protected]>

2 files changed, 206 insertions, 37 deletions

diff --git a/embassy-nrf/src/pwm.rs b/embassy-nrf/src/pwm.rs
index 90cdf69c6..3fdc37ec0 100644
--- a/embassy-nrf/src/pwm.rs
+++ b/embassy-nrf/src/pwm.rs
@@ -9,6 +9,7 @@ use crate::gpio::sealed::Pin as _;
 use crate::gpio::{AnyPin, OptionalPin as GpioOptionalPin};
 use crate::interrupt::Interrupt;
 use crate::pac;
+use crate::ppi::{Event, Task};
 use crate::util::slice_in_ram_or;
 
 /// SimplePwm is the traditional pwm interface you're probably used to, allowing
@@ -101,6 +102,13 @@ impl<'d, T: Instance> SequencePwm<'d, T> {
         // Disable all interrupts
         r.intenclr.write(|w| unsafe { w.bits(0xFFFF_FFFF) });
         r.shorts.reset();
+        r.events_stopped.reset();
+        r.events_loopsdone.reset();
+        r.events_seqend[0].reset();
+        r.events_seqend[1].reset();
+        r.events_pwmperiodend.reset();
+        r.events_seqstarted[0].reset();
+        r.events_seqstarted[1].reset();
 
         r.seq0
             .ptr
@@ -200,12 +208,111 @@ impl<'d, T: Instance> SequencePwm<'d, T> {
         Ok(())
     }
 
-    /// Stop playback.
+    /// Returns reference to `Stopped` event endpoint for PPI.
     #[inline(always)]
-    pub fn stop(&self) {
+    pub fn event_stopped(&self) -> Event {
         let r = T::regs();
 
-        r.enable.write(|w| w.enable().disabled());
+        Event::from_reg(&r.events_stopped)
+    }
+
+    /// Returns reference to `LoopsDone` event endpoint for PPI.
+    #[inline(always)]
+    pub fn event_loops_done(&self) -> Event {
+        let r = T::regs();
+
+        Event::from_reg(&r.events_loopsdone)
+    }
+
+    /// Returns reference to `PwmPeriodEnd` event endpoint for PPI.
+    #[inline(always)]
+    pub fn event_pwm_period_end(&self) -> Event {
+        let r = T::regs();
+
+        Event::from_reg(&r.events_pwmperiodend)
+    }
+
+    /// Returns reference to `Seq0 End` event endpoint for PPI.
+    #[inline(always)]
+    pub fn event_seq_end(&self) -> Event {
+        let r = T::regs();
+
+        Event::from_reg(&r.events_seqend[0])
+    }
+
+    /// Returns reference to `Seq1 End` event endpoint for PPI.
+    #[inline(always)]
+    pub fn event_seq1_end(&self) -> Event {
+        let r = T::regs();
+
+        Event::from_reg(&r.events_seqend[1])
+    }
+
+    /// Returns reference to `Seq0 Started` event endpoint for PPI.
+    #[inline(always)]
+    pub fn event_seq0_started(&self) -> Event {
+        let r = T::regs();
+
+        Event::from_reg(&r.events_seqstarted[0])
+    }
+
+    /// Returns reference to `Seq1 Started` event endpoint for PPI.
+    #[inline(always)]
+    pub fn event_seq1_started(&self) -> Event {
+        let r = T::regs();
+
+        Event::from_reg(&r.events_seqstarted[1])
+    }
+
+    /// Returns reference to `Seq0 Start` task endpoint for PPI.
+    /// # Safety
+    ///
+    /// Interacting with the sequence while it runs puts it in an unknown state
+    #[inline(always)]
+    pub unsafe fn task_start_seq0(&self) -> Task {
+        let r = T::regs();
+
+        Task::from_reg(&r.tasks_seqstart[0])
+    }
+
+    /// Returns reference to `Seq1 Started` task endpoint for PPI.
+    /// # Safety
+    ///
+    /// Interacting with the sequence while it runs puts it in an unknown state
+    #[inline(always)]
+    pub unsafe fn task_start_seq1(&self) -> Task {
+        let r = T::regs();
+
+        Task::from_reg(&r.tasks_seqstart[1])
+    }
+
+    /// Returns reference to `NextStep` task endpoint for PPI.
+    /// # Safety
+    ///
+    /// Interacting with the sequence while it runs puts it in an unknown state
+    #[inline(always)]
+    pub unsafe fn task_next_step(&self) -> Task {
+        let r = T::regs();
+
+        Task::from_reg(&r.tasks_nextstep)
+    }
+
+    /// Returns reference to `Stop` task endpoint for PPI.
+    /// # Safety
+    ///
+    /// Interacting with the sequence while it runs puts it in an unknown state
+    #[inline(always)]
+    pub unsafe fn task_stop(&self) -> Task {
+        let r = T::regs();
+
+        Task::from_reg(&r.tasks_stop)
+    }
+
+    /// Stop playback. Disables the peripheral. Does NOT clear the last duty
+    /// cycle from the pin.
+    #[inline(always)]
+    pub fn stop(&self) {
+        let r = T::regs();
 
         r.shorts.reset();
 
@@ -213,6 +320,8 @@ impl<'d, T: Instance> SequencePwm<'d, T> {
 
         // tasks_stop() doesn't exist in all svds so write its bit instead
         r.tasks_stop.write(|w| unsafe { w.bits(0x01) });
+
+        r.enable.write(|w| w.enable().disabled());
     }
 }
 
@@ -224,23 +333,23 @@ impl<'a, T: Instance> Drop for SequencePwm<'a, T> {
 
         if let Some(pin) = &self.ch0 {
             pin.set_low();
-            pin.conf().write(|w| w);
-            r.psel.out[0].write(|w| unsafe { w.bits(0x80000000) });
+            pin.conf().reset();
+            r.psel.out[0].reset();
         }
         if let Some(pin) = &self.ch1 {
             pin.set_low();
-            pin.conf().write(|w| w);
-            r.psel.out[1].write(|w| unsafe { w.bits(0x80000000) });
+            pin.conf().reset();
+            r.psel.out[1].reset();
         }
         if let Some(pin) = &self.ch2 {
             pin.set_low();
-            pin.conf().write(|w| w);
-            r.psel.out[2].write(|w| unsafe { w.bits(0x80000000) });
+            pin.conf().reset();
+            r.psel.out[2].reset();
         }
         if let Some(pin) = &self.ch3 {
             pin.set_low();
-            pin.conf().write(|w| w);
-            r.psel.out[3].write(|w| unsafe { w.bits(0x80000000) });
+            pin.conf().reset();
+            r.psel.out[3].reset();
         }
     }
 }
@@ -325,8 +434,8 @@ pub enum CounterMode {
 impl<'d, T: Instance> SimplePwm<'d, T> {
     /// Creates the interface to a `SimplePwm`
     ///
-    /// Defaults the freq to 1Mhz, max_duty 1000, duty 0, up mode, and pins low.
-    /// Must be started by calling `set_duty`
+    /// Enables the peripheral, defaults the freq to 1Mhz, max_duty 1000, duty
+    /// 0, up mode, and pins low. Must be started by calling `set_duty`
     ///
     /// # Safety
     ///
@@ -405,19 +514,6 @@ impl<'d, T: Instance> SimplePwm<'d, T> {
         pwm
     }
 
-    /// Stop playback
-    #[inline(always)]
-    pub fn stop(&self) {
-        let r = T::regs();
-
-        r.shorts.reset();
-
-        compiler_fence(Ordering::SeqCst);
-
-        // tasks_stop() doesn't exist in all svds so write its bit instead
-        r.tasks_stop.write(|w| unsafe { w.bits(0x01) });
-    }
-
     /// Enables the PWM generator.
     #[inline(always)]
     pub fn enable(&self) {
@@ -425,7 +521,7 @@ impl<'d, T: Instance> SimplePwm<'d, T> {
         r.enable.write(|w| w.enable().enabled());
     }
 
-    /// Disables the PWM generator.
+    /// Disables the PWM generator. Does NOT clear the last duty cycle from the pin.
     #[inline(always)]
     pub fn disable(&self) {
         let r = T::regs();
@@ -446,13 +542,14 @@ impl<'d, T: Instance> SimplePwm<'d, T> {
         // defensive before seqstart
         compiler_fence(Ordering::SeqCst);
 
+        r.events_seqend[0].reset();
+
         // tasks_seqstart() doesn't exist in all svds so write its bit instead
         r.tasks_seqstart[0].write(|w| unsafe { w.bits(1) });
 
         // defensive wait until waveform is loaded after seqstart so set_duty
         // can't be called again while dma is still reading
         while r.events_seqend[0].read().bits() == 0 {}
-        r.events_seqend[0].write(|w| w);
     }
 
     /// Sets the PWM clock prescaler.
@@ -512,28 +609,27 @@ impl<'a, T: Instance> Drop for SimplePwm<'a, T> {
     fn drop(&mut self) {
         let r = T::regs();
 
-        self.stop();
         self.disable();
 
         if let Some(pin) = &self.ch0 {
             pin.set_low();
-            pin.conf().write(|w| w);
-            r.psel.out[0].write(|w| unsafe { w.bits(0x80000000) });
+            pin.conf().reset();
+            r.psel.out[0].reset();
         }
         if let Some(pin) = &self.ch1 {
             pin.set_low();
-            pin.conf().write(|w| w);
-            r.psel.out[1].write(|w| unsafe { w.bits(0x80000000) });
+            pin.conf().reset();
+            r.psel.out[1].reset();
         }
         if let Some(pin) = &self.ch2 {
             pin.set_low();
-            pin.conf().write(|w| w);
-            r.psel.out[2].write(|w| unsafe { w.bits(0x80000000) });
+            pin.conf().reset();
+            r.psel.out[2].reset();
         }
         if let Some(pin) = &self.ch3 {
             pin.set_low();
-            pin.conf().write(|w| w);
-            r.psel.out[3].write(|w| unsafe { w.bits(0x80000000) });
+            pin.conf().reset();
+            r.psel.out[3].reset();
         }
     }
 }
diff --git a/examples/nrf/src/bin/pwm_sequence_ppi.rs b/examples/nrf/src/bin/pwm_sequence_ppi.rs
new file mode 100644
index 000000000..aaea9ff00
--- /dev/null
+++ b/examples/nrf/src/bin/pwm_sequence_ppi.rs
@@ -0,0 +1,73 @@
+#![no_std]
+#![no_main]
+#![feature(type_alias_impl_trait)]
+#![feature(array_from_fn)]
+
+#[path = "../example_common.rs"]
+mod example_common;
+use core::future::pending;
+use defmt::*;
+use embassy::executor::Spawner;
+use embassy_nrf::gpio::{Input, NoPin, Pull};
+use embassy_nrf::gpiote::{InputChannel, InputChannelPolarity};
+use embassy_nrf::ppi::Ppi;
+use embassy_nrf::pwm::{Prescaler, SequenceConfig, SequenceMode, SequencePwm};
+use embassy_nrf::Peripherals;
+
+#[embassy::main]
+async fn main(_spawner: Spawner, p: Peripherals) {
+    let seq_values: [u16; 5] = [1000, 250, 100, 50, 0];
+
+    let mut config = SequenceConfig::default();
+    config.prescaler = Prescaler::Div128;
+    // 1 period is 1000 * (128/16mhz = 0.000008s = 0.008ms) = 8ms
+    // but say we want to hold the value for 250ms 250ms/8 = 31.25 periods
+    // so round to 31 - 1 (we get the one period for free remember)
+    // thus our sequence takes 5 * 250ms or 1.25 seconds
+    config.refresh = 30;
+
+    let pwm = unwrap!(SequencePwm::new(
+        p.PWM0,
+        p.P0_13,
+        NoPin,
+        NoPin,
+        NoPin,
+        config,
+        &seq_values
+    ));
+
+    let _ = pwm.start(SequenceMode::Times(1));
+    // pwm.stop() deconfigures pins, and then the task_start_seq0 task cant work
+    // so its going to have to start running in order load the configuration
+
+    let button1 = InputChannel::new(
+        p.GPIOTE_CH0,
+        Input::new(p.P0_11, Pull::Up),
+        InputChannelPolarity::HiToLo,
+    );
+
+    let button2 = InputChannel::new(
+        p.GPIOTE_CH1,
+        Input::new(p.P0_12, Pull::Up),
+        InputChannelPolarity::HiToLo,
+    );
+
+    // messing with the pwm tasks is ill advised
+    // Times::Ininite and Times even are seq0, Times odd is seq1
+    let start = unsafe { pwm.task_start_seq0() };
+    let stop = unsafe { pwm.task_stop() };
+
+    let mut ppi = Ppi::new_one_to_one(p.PPI_CH1, button1.event_in(), start);
+    ppi.enable();
+
+    let mut ppi2 = Ppi::new_one_to_one(p.PPI_CH0, button2.event_in(), stop);
+    ppi2.enable();
+
+    info!("PPI setup!");
+    info!("Press button 1 to start LED 1");
+    info!("Press button 2 to stop LED 1");
+    info!("Note! task_stop stops the sequence, but not the pin output");
+
+    // Block forever so the above drivers don't get dropped
+    pending::<()>().await;
+}