Revert "Own the sequence buffer"

This reverts commit 482389a691.

embassy-nrf/src/pwm.rs

@@ -25,14 +25,14 @@ pub struct SimplePwm<'d, T: Instance> {
 
 /// SequencePwm allows you to offload the updating of a sequence of duty cycles
 /// to up to four channels, as well as repeat that sequence n times.
-pub struct SequencePwm<'d, T: Instance, const S0: usize, const S1: usize> {
+pub struct SequencePwm<'d, T: Instance> {
     phantom: PhantomData<&'d mut T>,
     ch0: Option<AnyPin>,
     ch1: Option<AnyPin>,
     ch2: Option<AnyPin>,
     ch3: Option<AnyPin>,
-    sequence0: Option<Sequence<S0>>,
+    sequence0: Option<Sequence<'d>>,
-    sequence1: Option<Sequence<S1>>,
+    sequence1: Option<Sequence<'d>>,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -43,17 +43,13 @@ pub enum Error {
     SequenceTooLong,
     /// Min Sequence count is 1
     SequenceTimesAtLeastOne,
-    /// Sequence 0 is required, Sequence 1 is NOT required
-    SequenceTimesRequireSeq0Only,
-    /// Sequence 0 is required, Sequence 1 is required
-    SequenceTimesRequireBothSeq0AndSeq1,
     /// EasyDMA can only read from data memory, read only buffers in flash will fail.
     DMABufferNotInDataMemory,
 }
 
 const MAX_SEQUENCE_LEN: usize = 32767;
 
-impl<'d, T: Instance, const S0: usize, const S1: usize> SequencePwm<'d, T, S0, S1> {
+impl<'d, T: Instance> SequencePwm<'d, T> {
     /// Creates the interface to a `SequencePwm`.
     ///
     /// Must be started by calling `start`
@@ -72,10 +68,6 @@ impl<'d, T: Instance, const S0: usize, const S1: usize> SequencePwm<'d, T, S0, S
         ch3: impl Unborrow<Target = impl GpioOptionalPin> + 'd,
         config: Config,
     ) -> Result<Self, Error> {
-        if S0 > MAX_SEQUENCE_LEN || S1 > MAX_SEQUENCE_LEN {
-            return Err(Error::SequenceTooLong);
-        }
-
         unborrow!(ch0, ch1, ch2, ch3);
 
         let r = T::regs();
@@ -141,49 +133,31 @@ impl<'d, T: Instance, const S0: usize, const S1: usize> SequencePwm<'d, T, S0, S
     }
 
     /// Start or restart playback. Takes at least one sequence along with its
-    /// configuration. A second sequence must be provided when looping i.e.
+    /// configuration. Optionally takes a second sequence and its configuration.
-    /// when the sequence mode is anything other than Times(1).
+    /// In the case where no second sequence is provided then the first sequence
+    /// is used. The sequence mode applies to both sequences combined as one.
     #[inline(always)]
     pub fn start(
         &mut self,
-        sequence0: Sequence<S0>,
+        sequence0: Sequence<'d>,
-        sequence1: Sequence<S1>,
+        sequence1: Option<Sequence<'d>>,
         times: SequenceMode,
     ) -> Result<(), Error> {
-        slice_in_ram_or(&sequence0.words, Error::DMABufferNotInDataMemory)?;
+        let alt_sequence = sequence1.as_ref().unwrap_or(&sequence0);
-        slice_in_ram_or(&sequence1.words, Error::DMABufferNotInDataMemory)?;
 
-        let seq_0_word_count = sequence0.word_count.unwrap_or(S0);
+        slice_in_ram_or(sequence0.words, Error::DMABufferNotInDataMemory)?;
-        let seq_1_word_count = sequence0.word_count.unwrap_or(S1);
+        slice_in_ram_or(alt_sequence.words, Error::DMABufferNotInDataMemory)?;
-        if seq_0_word_count > S0 || seq_1_word_count > S1 {
+
+        if sequence0.words.len() > MAX_SEQUENCE_LEN || alt_sequence.words.len() > MAX_SEQUENCE_LEN {
             return Err(Error::SequenceTooLong);
         }
 
-        match times {
+        if let SequenceMode::Times(0) = times {
-            SequenceMode::Times(0) => return Err(Error::SequenceTimesAtLeastOne),
+            return Err(Error::SequenceTimesAtLeastOne);
-            SequenceMode::Times(1) if seq_0_word_count == 0 || seq_1_word_count != 0 => {
-                return Err(Error::SequenceTimesRequireSeq0Only)
-            }
-            SequenceMode::Times(1) => (),
-            SequenceMode::Times(_) | SequenceMode::Infinite
-                if seq_0_word_count == 0 || seq_1_word_count == 0 =>
-            {
-                return Err(Error::SequenceTimesRequireBothSeq0AndSeq1)
-            }
-            SequenceMode::Times(_) | SequenceMode::Infinite => (),
         }
 
         let _ = self.stop();
 
-        // We now own these sequences and they will be moved. We want
-        // the peripheral to point at the right bits of memory hence
-        // moving the sequences early.
-        self.sequence0 = Some(sequence0);
-        self.sequence1 = Some(sequence1);
-
-        let sequence0 = self.sequence0.as_ref().unwrap();
-        let sequence1 = self.sequence1.as_ref().unwrap();
-
         let r = T::regs();
 
         r.seq0
@@ -197,20 +171,20 @@ impl<'d, T: Instance, const S0: usize, const S1: usize> SequencePwm<'d, T, S0, S
             .write(|w| unsafe { w.bits(sequence0.words.as_ptr() as u32) });
         r.seq0
             .cnt
-            .write(|w| unsafe { w.bits(seq_0_word_count as u32) });
+            .write(|w| unsafe { w.bits(sequence0.words.len() as u32) });
 
         r.seq1
             .refresh
-            .write(|w| unsafe { w.bits(sequence1.config.refresh) });
+            .write(|w| unsafe { w.bits(alt_sequence.config.refresh) });
         r.seq1
             .enddelay
-            .write(|w| unsafe { w.bits(sequence1.config.end_delay) });
+            .write(|w| unsafe { w.bits(alt_sequence.config.end_delay) });
         r.seq1
             .ptr
-            .write(|w| unsafe { w.bits(sequence1.words.as_ptr() as u32) });
+            .write(|w| unsafe { w.bits(alt_sequence.words.as_ptr() as u32) });
         r.seq1
             .cnt
-            .write(|w| unsafe { w.bits(seq_1_word_count as u32) });
+            .write(|w| unsafe { w.bits(alt_sequence.words.len() as u32) });
 
         r.enable.write(|w| w.enable().enabled());
 
@@ -252,6 +226,9 @@ impl<'d, T: Instance, const S0: usize, const S1: usize> SequencePwm<'d, T, S0, S
             }
         }
 
+        self.sequence0 = Some(sequence0);
+        self.sequence1 = sequence1;
+
         Ok(())
     }
 
@@ -359,7 +336,7 @@ impl<'d, T: Instance, const S0: usize, const S1: usize> SequencePwm<'d, T, S0, S
     /// cycle from the pin. Returns any sequences previously provided to
     /// `start` so that they may be further mutated.
     #[inline(always)]
-    pub fn stop(&mut self) -> (Option<Sequence<S0>>, Option<Sequence<S1>>) {
+    pub fn stop(&mut self) -> (Option<Sequence<'d>>, Option<Sequence<'d>>) {
         let r = T::regs();
 
         r.shorts.reset();
@@ -375,7 +352,7 @@ impl<'d, T: Instance, const S0: usize, const S1: usize> SequencePwm<'d, T, S0, S
     }
 }
 
-impl<'a, T: Instance, const S0: usize, const S1: usize> Drop for SequencePwm<'a, T, S0, S1> {
+impl<'a, T: Instance> Drop for SequencePwm<'a, T> {
     fn drop(&mut self) {
         let r = T::regs();
 
@@ -404,7 +381,6 @@ impl<'a, T: Instance, const S0: usize, const S1: usize> Drop for SequencePwm<'a,
     }
 }
 
-/// Configuration for the PWM as a whole.
 #[non_exhaustive]
 pub struct Config {
     /// Selects up mode or up-and-down mode for the counter
@@ -428,7 +404,6 @@ impl Default for Config {
     }
 }
 
-/// Configuration per sequence
 #[non_exhaustive]
 #[derive(Clone)]
 pub struct SequenceConfig {
@@ -447,39 +422,20 @@ impl Default for SequenceConfig {
     }
 }
 
-/// A composition of a sequence buffer and its configuration.
 #[non_exhaustive]
-#[derive(Clone)]
+pub struct Sequence<'d> {
-pub struct Sequence<const S: usize> {
     /// The words comprising the sequence. Must not exceed 32767 words.
-    pub words: [u16; S],
+    pub words: &'d mut [u16],
-    /// The count of words to use. If None the S will be used.
-    pub word_count: Option<usize>,
     /// Configuration associated with the sequence.
     pub config: SequenceConfig,
 }
 
-impl<const S: usize> Sequence<S> {
+impl<'d> Sequence<'d> {
-    pub const fn new(words: [u16; S], config: SequenceConfig) -> Self {
+    pub fn new(words: &'d mut [u16], config: SequenceConfig) -> Self {
-        Self {
+        Self { words, config }
-            words,
-            word_count: None,
-            config,
-        }
     }
 }
 
-/// Declares an empty sequence which will cause it to be disabled.
-/// Note that any looping i.e. !Times(1), will require a second
-/// sequence given the way the PWM peripheral works.
-pub const EMPTY_SEQ: Sequence<0> = Sequence::new(
-    [],
-    SequenceConfig {
-        refresh: 0,
-        end_delay: 0,
-    },
-);
-
 /// How many times to run the sequence
 #[derive(Debug, Eq, PartialEq, Clone, Copy)]
 pub enum SequenceMode {
@@ -490,7 +446,7 @@ pub enum SequenceMode {
     /// 5 to 6 = Run sequence 0, sequence 1, sequence 0, sequence 1, sequence 0 and then sequence 1
     /// i.e the when >= 2 the loop count is determined by dividing by 2 and rounding up
     Times(u16),
-    /// Repeat until `stop` is called. Both sequences must be provided.
+    /// Repeat until `stop` is called.
     Infinite,
 }
 

examples/nrf/src/bin/pwm_sequence.rs

@@ -8,13 +8,14 @@ use defmt::*;
 use embassy::executor::Spawner;
 use embassy::time::{Duration, Timer};
 use embassy_nrf::gpio::NoPin;
-use embassy_nrf::pwm::{
+use embassy_nrf::pwm::{Config, Prescaler, Sequence, SequenceConfig, SequenceMode, SequencePwm};
-    Config, Prescaler, Sequence, SequenceConfig, SequenceMode, SequencePwm, EMPTY_SEQ,
-};
 use embassy_nrf::Peripherals;
 
 #[embassy::main]
 async fn main(_spawner: Spawner, p: Peripherals) {
+    let mut seq_words_1: [u16; 5] = [1000, 250, 100, 50, 0];
+    let mut seq_words_2: [u16; 5] = [0, 50, 100, 250, 1000];
+
     let mut config = Config::default();
     config.prescaler = Prescaler::Div128;
     // 1 period is 1000 * (128/16mhz = 0.000008s = 0.008ms) = 8us
@@ -25,20 +26,25 @@ async fn main(_spawner: Spawner, p: Peripherals) {
     seq_config.refresh = 624;
     // thus our sequence takes 5 * 5000ms or 25 seconds
 
-    let seq_1 = Sequence::new([1000, 250, 100, 50, 0], seq_config.clone());
-    let seq_2 = Sequence::new([0, 50, 100, 250, 1000], seq_config);
-
     let mut pwm = unwrap!(SequencePwm::new(
         p.PWM0, p.P0_13, NoPin, NoPin, NoPin, config,
     ));
-    unwrap!(pwm.start(seq_1, EMPTY_SEQ, SequenceMode::Times(1)));
+    let _ = pwm.start(
+        Sequence::new(&mut seq_words_1, seq_config.clone()),
+        None,
+        SequenceMode::Infinite,
+    );
 
     info!("pwm started!");
 
     Timer::after(Duration::from_millis(20000)).await;
     info!("pwm starting with another sequence!");
 
-    unwrap!(pwm.start(seq_2, EMPTY_SEQ, SequenceMode::Times(1)));
+    let _ = pwm.start(
+        Sequence::new(&mut seq_words_2, seq_config),
+        None,
+        SequenceMode::Infinite,
+    );
 
     // we can abort a sequence if we need to before its complete with pwm.stop()
     // or stop is also implicitly called when the pwm peripheral is dropped

examples/nrf/src/bin/pwm_sequence_ppi.rs

@@ -16,7 +16,7 @@ use embassy_nrf::Peripherals;
 
 #[embassy::main]
 async fn main(_spawner: Spawner, p: Peripherals) {
-    let seq_words: [u16; 5] = [1000, 250, 100, 50, 0];
+    let mut seq_words: [u16; 5] = [1000, 250, 100, 50, 0];
 
     let mut config = Config::default();
     config.prescaler = Prescaler::Div128;
@@ -31,12 +31,11 @@ async fn main(_spawner: Spawner, p: Peripherals) {
         p.PWM0, p.P0_13, NoPin, NoPin, NoPin, config,
     ));
 
-    // If we loop in any way i.e. not Times(1), then we must provide
+    let _ = pwm.start(
-    // the PWM peripheral with two sequences.
+        Sequence::new(&mut seq_words, seq_config),
-    let seq_0 = Sequence::new(seq_words, seq_config);
+        None,
-    let seq_1 = seq_0.clone();
+        SequenceMode::Infinite,
-
+    );
-    unwrap!(pwm.start(seq_0, seq_1, SequenceMode::Infinite));
     // 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
 

examples/nrf/src/bin/pwm_sequence_ws2812b.rs

@@ -30,6 +30,19 @@ const RES: u16 = 0x8000;
 // line is assumed to be P1_05.
 #[embassy::main]
 async fn main(_spawner: Spawner, p: Peripherals) {
+    // Declare the bits of 24 bits
+    let mut color_seq_words = [
+        T0H, T0H, T0H, T0H, T0H, T0H, T0H, T0H, // G
+        T0H, T0H, T0H, T0H, T0H, T0H, T0H, T0H, // R
+        T1H, T1H, T1H, T1H, T1H, T1H, T1H, T1H, // B
+    ];
+    let color_seq = Sequence::new(&mut color_seq_words, SequenceConfig::default());
+
+    let mut reset_seq_words = [RES; 1];
+    let mut reset_seq_config = SequenceConfig::default();
+    reset_seq_config.end_delay = 799; // 50us (20 ticks * 40) - 1 tick because we've already got one RES;
+    let reset_seq = Sequence::new(&mut reset_seq_words, reset_seq_config);
+
     let mut config = Config::default();
     config.sequence_load = SequenceLoad::Common;
     config.prescaler = Prescaler::Div1;
@@ -38,21 +51,7 @@ async fn main(_spawner: Spawner, p: Peripherals) {
         p.PWM0, p.P1_05, NoPin, NoPin, NoPin, config,
     ));
 
-    // Declare the bits of 24 bits
+    unwrap!(pwm.start(color_seq, Some(reset_seq), SequenceMode::Times(2)));
-    let color_seq = Sequence::new(
-        [
-            T0H, T0H, T0H, T0H, T0H, T0H, T0H, T0H, // G
-            T0H, T0H, T0H, T0H, T0H, T0H, T0H, T0H, // R
-            T1H, T1H, T1H, T1H, T1H, T1H, T1H, T1H, // B
-        ],
-        SequenceConfig::default(),
-    );
-
-    let mut reset_seq_config = SequenceConfig::default();
-    reset_seq_config.end_delay = 799; // 50us (20 ticks * 40) - 1 tick because we've already got one RES;
-    let reset_seq = Sequence::new([RES], reset_seq_config);
-
-    unwrap!(pwm.start(color_seq, reset_seq, SequenceMode::Times(2)));
 
     Timer::after(Duration::from_millis(1000)).await;
 
@@ -60,9 +59,9 @@ async fn main(_spawner: Spawner, p: Peripherals) {
     let mut bit_value = T0H;
 
     loop {
-        if let (Some(mut color_seq), Some(reset_seq)) = pwm.stop() {
+        if let (Some(color_seq), Some(reset_seq)) = pwm.stop() {
             color_seq.words[color_bit] = bit_value;
-            unwrap!(pwm.start(color_seq, reset_seq, SequenceMode::Times(2)));
+            unwrap!(pwm.start(color_seq, Some(reset_seq), SequenceMode::Times(2)));
         }
 
         Timer::after(Duration::from_millis(50)).await;