Merge pull request #4892 from Dectron-AB/cleanup-timer

stm32: Cleanup timers

4 files changed, 271 insertions, 262 deletions

diff --git a/embassy-stm32/CHANGELOG.md b/embassy-stm32/CHANGELOG.md
index 72fe1c7a8..2c3dfb3d3 100644
--- a/embassy-stm32/CHANGELOG.md
+++ b/embassy-stm32/CHANGELOG.md
@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased - ReleaseDate
 
+- feat: Add waveform methods to ComplementaryPwm
 - chore: cleanup low-power add time
 - fix: Allow setting SAI peripheral `frame_length` to `256`
 - fix: flash erase on dual-bank STM32Gxxx
diff --git a/embassy-stm32/src/timer/complementary_pwm.rs b/embassy-stm32/src/timer/complementary_pwm.rs
index 9a56a41fb..76cbbe91d 100644
--- a/embassy-stm32/src/timer/complementary_pwm.rs
+++ b/embassy-stm32/src/timer/complementary_pwm.rs
@@ -218,60 +218,57 @@ impl<'d, T: AdvancedInstance4Channel> ComplementaryPwm<'d, T> {
     ///
     /// Note:
     /// you will need to provide corresponding TIMx_UP DMA channel to use this method.
+    #[inline(always)]
     pub async fn waveform_up(&mut self, dma: Peri<'_, impl super::UpDma<T>>, channel: Channel, duty: &[u16]) {
-        #[allow(clippy::let_unit_value)] // eg. stm32f334
-        let req = dma.request();
-
-        let original_duty_state = self.inner.get_compare_value(channel);
-        let original_enable_state = self.inner.get_channel_enable_state(channel);
-        let original_update_dma_state = self.inner.get_update_dma_state();
-
-        if !original_update_dma_state {
-            self.inner.enable_update_dma(true);
-        }
-
-        if !original_enable_state {
-            self.inner.enable_channel(channel, true);
-        }
-
-        unsafe {
-            #[cfg(not(any(bdma, gpdma)))]
-            use crate::dma::{Burst, FifoThreshold};
-            use crate::dma::{Transfer, TransferOptions};
-
-            let dma_transfer_option = TransferOptions {
-                #[cfg(not(any(bdma, gpdma)))]
-                fifo_threshold: Some(FifoThreshold::Full),
-                #[cfg(not(any(bdma, gpdma)))]
-                mburst: Burst::Incr8,
-                ..Default::default()
-            };
-
-            Transfer::new_write(
-                dma,
-                req,
-                duty,
-                self.inner.regs_gp16().ccr(channel.index()).as_ptr() as *mut u16,
-                dma_transfer_option,
-            )
-            .await
-        };
-
-        // restore output compare state
-        if !original_enable_state {
-            self.inner.enable_channel(channel, false);
-        }
+        self.inner.waveform_up(dma, channel, duty).await
+    }
 
-        self.inner.set_compare_value(channel, original_duty_state);
+    /// Generate a multichannel sequence of PWM waveforms using DMA triggered by timer update events.
+    ///
+    /// This method utilizes the timer's DMA burst transfer capability to update multiple CCRx registers
+    /// in sequence on each update event (UEV). The data is written via the DMAR register using the
+    /// DMA base address (DBA) and burst length (DBL) configured in the DCR register.
+    ///
+    /// The `duty` buffer must be structured as a flattened 2D array in row-major order, where each row
+    /// represents a single update event and each column corresponds to a specific timer channel (starting
+    /// from `starting_channel` up to and including `ending_channel`).
+    ///
+    /// For example, if using channels 1 through 4, a buffer of 4 update steps might look like:
+    ///
+    /// ```rust,ignore
+    /// let dma_buf: [u16; 16] = [
+    ///     ch1_duty_1, ch2_duty_1, ch3_duty_1, ch4_duty_1, // update 1
+    ///     ch1_duty_2, ch2_duty_2, ch3_duty_2, ch4_duty_2, // update 2
+    ///     ch1_duty_3, ch2_duty_3, ch3_duty_3, ch4_duty_3, // update 3
+    ///     ch1_duty_4, ch2_duty_4, ch3_duty_4, ch4_duty_4, // update 4
+    /// ];
+    /// ```
+    ///
+    /// Each group of `N` values (where `N` is number of channels) is transferred on one update event,
+    /// updating the duty cycles of all selected channels simultaneously.
+    ///
+    /// Note:
+    /// You will need to provide corresponding `TIMx_UP` DMA channel to use this method.
+    /// Also be aware that embassy timers use one of timers internally. It is possible to
+    /// switch this timer by using `time-driver-timX` feature.
+    ///
+    #[inline(always)]
+    pub async fn waveform_up_multi_channel(
+        &mut self,
+        dma: Peri<'_, impl super::UpDma<T>>,
+        starting_channel: Channel,
+        ending_channel: Channel,
+        duty: &[u16],
+    ) {
+        self.inner
+            .waveform_up_multi_channel(dma, starting_channel, ending_channel, duty)
+            .await;
+    }
 
-        // Since DMA is closed before timer update event trigger DMA is turn off,
-        // this can almost always trigger a DMA FIFO error.
-        //
-        // optional TODO:
-        // clean FEIF after disable UDE
-        if !original_update_dma_state {
-            self.inner.enable_update_dma(false);
-        }
+    /// Generate a sequence of PWM waveform
+    #[inline(always)]
+    pub async fn waveform<C: TimerChannel>(&mut self, dma: Peri<'_, impl super::Dma<T, C>>, duty: &[u16]) {
+        self.inner.waveform(dma, duty).await;
     }
 }
 
diff --git a/embassy-stm32/src/timer/low_level.rs b/embassy-stm32/src/timer/low_level.rs
index 0122fe4f7..c574277e7 100644
--- a/embassy-stm32/src/timer/low_level.rs
+++ b/embassy-stm32/src/timer/low_level.rs
@@ -14,8 +14,8 @@ pub use stm32_metapac::timer::vals::{FilterValue, Mms as MasterMode, Sms as Slav
 
 use super::*;
 use crate::pac::timer::vals;
-use crate::rcc;
 use crate::time::Hertz;
+use crate::{dma, rcc};
 
 /// Input capture mode.
 #[derive(Clone, Copy)]
@@ -656,6 +656,219 @@ impl<'d, T: GeneralInstance4Channel> Timer<'d, T> {
         }
     }
 
+    /// Generate a sequence of PWM waveform
+    ///
+    /// Note:
+    /// you will need to provide corresponding TIMx_UP DMA channel to use this method.
+    pub async fn waveform_up(&mut self, dma: Peri<'_, impl super::UpDma<T>>, channel: Channel, duty: &[u16]) {
+        #[allow(clippy::let_unit_value)] // eg. stm32f334
+        let req = dma.request();
+
+        let original_update_dma_state = self.get_update_dma_state();
+
+        if !original_update_dma_state {
+            self.enable_update_dma(true);
+        }
+
+        self.waveform_helper(dma, req, channel, duty).await;
+
+        // Since DMA is closed before timer update event trigger DMA is turn off,
+        // this can almost always trigger a DMA FIFO error.
+        //
+        // optional TODO:
+        // clean FEIF after disable UDE
+        if !original_update_dma_state {
+            self.enable_update_dma(false);
+        }
+    }
+
+    /// Generate a multichannel sequence of PWM waveforms using DMA triggered by timer update events.
+    ///
+    /// This method utilizes the timer's DMA burst transfer capability to update multiple CCRx registers
+    /// in sequence on each update event (UEV). The data is written via the DMAR register using the
+    /// DMA base address (DBA) and burst length (DBL) configured in the DCR register.
+    ///
+    /// The `duty` buffer must be structured as a flattened 2D array in row-major order, where each row
+    /// represents a single update event and each column corresponds to a specific timer channel (starting
+    /// from `starting_channel` up to and including `ending_channel`).
+    ///
+    /// For example, if using channels 1 through 4, a buffer of 4 update steps might look like:
+    ///
+    /// ```rust,ignore
+    /// let dma_buf: [u16; 16] = [
+    ///     ch1_duty_1, ch2_duty_1, ch3_duty_1, ch4_duty_1, // update 1
+    ///     ch1_duty_2, ch2_duty_2, ch3_duty_2, ch4_duty_2, // update 2
+    ///     ch1_duty_3, ch2_duty_3, ch3_duty_3, ch4_duty_3, // update 3
+    ///     ch1_duty_4, ch2_duty_4, ch3_duty_4, ch4_duty_4, // update 4
+    /// ];
+    /// ```
+    ///
+    /// Each group of `N` values (where `N` is number of channels) is transferred on one update event,
+    /// updating the duty cycles of all selected channels simultaneously.
+    ///
+    /// Note:
+    /// You will need to provide corresponding `TIMx_UP` DMA channel to use this method.
+    /// Also be aware that embassy timers use one of timers internally. It is possible to
+    /// switch this timer by using `time-driver-timX` feature.
+    ///
+    pub async fn waveform_up_multi_channel(
+        &mut self,
+        dma: Peri<'_, impl super::UpDma<T>>,
+        starting_channel: Channel,
+        ending_channel: Channel,
+        duty: &[u16],
+    ) {
+        let cr1_addr = self.regs_gp16().cr1().as_ptr() as u32;
+        let start_ch_index = starting_channel.index();
+        let end_ch_index = ending_channel.index();
+
+        assert!(start_ch_index <= end_ch_index);
+
+        let ccrx_addr = self.regs_gp16().ccr(start_ch_index).as_ptr() as u32;
+        self.regs_gp16()
+            .dcr()
+            .modify(|w| w.set_dba(((ccrx_addr - cr1_addr) / 4) as u8));
+        self.regs_gp16()
+            .dcr()
+            .modify(|w| w.set_dbl((end_ch_index - start_ch_index) as u8));
+
+        #[allow(clippy::let_unit_value)] // eg. stm32f334
+        let req = dma.request();
+
+        let original_update_dma_state = self.get_update_dma_state();
+        if !original_update_dma_state {
+            self.enable_update_dma(true);
+        }
+
+        unsafe {
+            #[cfg(not(any(bdma, gpdma)))]
+            use crate::dma::{Burst, FifoThreshold};
+            use crate::dma::{Transfer, TransferOptions};
+
+            let dma_transfer_option = TransferOptions {
+                #[cfg(not(any(bdma, gpdma)))]
+                fifo_threshold: Some(FifoThreshold::Full),
+                #[cfg(not(any(bdma, gpdma)))]
+                mburst: Burst::Incr4,
+                ..Default::default()
+            };
+
+            Transfer::new_write(
+                dma,
+                req,
+                duty,
+                self.regs_gp16().dmar().as_ptr() as *mut u16,
+                dma_transfer_option,
+            )
+            .await
+        };
+
+        if !original_update_dma_state {
+            self.enable_update_dma(false);
+        }
+    }
+
+    /// Generate a sequence of PWM waveform
+    pub async fn waveform<C: TimerChannel>(&mut self, dma: Peri<'_, impl super::Dma<T, C>>, duty: &[u16]) {
+        use crate::pac::timer::vals::Ccds;
+
+        #[allow(clippy::let_unit_value)] // eg. stm32f334
+        let req = dma.request();
+
+        let cc_channel = C::CHANNEL;
+
+        let original_cc_dma_on_update = self.get_cc_dma_selection() == Ccds::ON_UPDATE;
+        let original_cc_dma_enabled = self.get_cc_dma_enable_state(cc_channel);
+
+        // redirect CC DMA request onto Update Event
+        if !original_cc_dma_on_update {
+            self.set_cc_dma_selection(Ccds::ON_UPDATE)
+        }
+
+        if !original_cc_dma_enabled {
+            self.set_cc_dma_enable_state(cc_channel, true);
+        }
+
+        self.waveform_helper(dma, req, cc_channel, duty).await;
+
+        // Since DMA is closed before timer Capture Compare Event trigger DMA is turn off,
+        // this can almost always trigger a DMA FIFO error.
+        //
+        // optional TODO:
+        // clean FEIF after disable UDE
+        if !original_cc_dma_enabled {
+            self.set_cc_dma_enable_state(cc_channel, false);
+        }
+
+        if !original_cc_dma_on_update {
+            self.set_cc_dma_selection(Ccds::ON_COMPARE)
+        }
+    }
+
+    async fn waveform_helper(
+        &mut self,
+        dma: Peri<'_, impl dma::Channel>,
+        req: dma::Request,
+        channel: Channel,
+        duty: &[u16],
+    ) {
+        let original_duty_state = self.get_compare_value(channel);
+        let original_enable_state = self.get_channel_enable_state(channel);
+
+        if !original_enable_state {
+            self.enable_channel(channel, true);
+        }
+
+        unsafe {
+            #[cfg(not(any(bdma, gpdma)))]
+            use crate::dma::{Burst, FifoThreshold};
+            use crate::dma::{Transfer, TransferOptions};
+
+            let dma_transfer_option = TransferOptions {
+                #[cfg(not(any(bdma, gpdma)))]
+                fifo_threshold: Some(FifoThreshold::Full),
+                #[cfg(not(any(bdma, gpdma)))]
+                mburst: Burst::Incr8,
+                ..Default::default()
+            };
+
+            match self.bits() {
+                TimerBits::Bits16 => {
+                    Transfer::new_write(
+                        dma,
+                        req,
+                        duty,
+                        self.regs_1ch().ccr(channel.index()).as_ptr() as *mut u16,
+                        dma_transfer_option,
+                    )
+                    .await
+                }
+                #[cfg(not(any(stm32l0)))]
+                TimerBits::Bits32 => {
+                    #[cfg(not(any(bdma, gpdma)))]
+                    panic!("unsupported timer bits");
+
+                    #[cfg(any(bdma, gpdma))]
+                    Transfer::new_write(
+                        dma,
+                        req,
+                        duty,
+                        self.regs_1ch().ccr(channel.index()).as_ptr() as *mut u32,
+                        dma_transfer_option,
+                    )
+                    .await
+                }
+            };
+        };
+
+        // restore output compare state
+        if !original_enable_state {
+            self.enable_channel(channel, false);
+        }
+
+        self.set_compare_value(channel, original_duty_state);
+    }
+
     /// Get capture value for a channel.
     pub fn get_capture_value(&self, channel: Channel) -> u32 {
         self.get_compare_value(channel)
diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index c338b0fd4..19a0b38d1 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -4,7 +4,7 @@ use core::marker::PhantomData;
 use core::mem::ManuallyDrop;
 
 use super::low_level::{CountingMode, OutputCompareMode, OutputPolarity, Timer};
-use super::{Ch1, Ch2, Ch3, Ch4, Channel, GeneralInstance4Channel, TimerBits, TimerChannel, TimerPin};
+use super::{Ch1, Ch2, Ch3, Ch4, Channel, GeneralInstance4Channel, TimerChannel, TimerPin};
 use crate::Peri;
 #[cfg(gpio_v2)]
 use crate::gpio::Pull;
@@ -312,79 +312,9 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     /// You will need to provide corresponding `TIMx_UP` DMA channel to use this method.
     /// Also be aware that embassy timers use one of timers internally. It is possible to
     /// switch this timer by using `time-driver-timX` feature.
+    #[inline(always)]
     pub async fn waveform_up(&mut self, dma: Peri<'_, impl super::UpDma<T>>, channel: Channel, duty: &[u16]) {
-        #[allow(clippy::let_unit_value)] // eg. stm32f334
-        let req = dma.request();
-
-        let original_duty_state = self.channel(channel).current_duty_cycle();
-        let original_enable_state = self.channel(channel).is_enabled();
-        let original_update_dma_state = self.inner.get_update_dma_state();
-
-        if !original_update_dma_state {
-            self.inner.enable_update_dma(true);
-        }
-
-        if !original_enable_state {
-            self.channel(channel).enable();
-        }
-
-        unsafe {
-            #[cfg(not(any(bdma, gpdma)))]
-            use crate::dma::{Burst, FifoThreshold};
-            use crate::dma::{Transfer, TransferOptions};
-
-            let dma_transfer_option = TransferOptions {
-                #[cfg(not(any(bdma, gpdma)))]
-                fifo_threshold: Some(FifoThreshold::Full),
-                #[cfg(not(any(bdma, gpdma)))]
-                mburst: Burst::Incr8,
-                ..Default::default()
-            };
-
-            match self.inner.bits() {
-                TimerBits::Bits16 => {
-                    Transfer::new_write(
-                        dma,
-                        req,
-                        duty,
-                        self.inner.regs_1ch().ccr(channel.index()).as_ptr() as *mut u16,
-                        dma_transfer_option,
-                    )
-                    .await
-                }
-                #[cfg(not(any(stm32l0)))]
-                TimerBits::Bits32 => {
-                    #[cfg(not(any(bdma, gpdma)))]
-                    panic!("unsupported timer bits");
-
-                    #[cfg(any(bdma, gpdma))]
-                    Transfer::new_write(
-                        dma,
-                        req,
-                        duty,
-                        self.inner.regs_1ch().ccr(channel.index()).as_ptr() as *mut u32,
-                        dma_transfer_option,
-                    )
-                    .await
-                }
-            };
-        };
-
-        // restore output compare state
-        if !original_enable_state {
-            self.channel(channel).disable();
-        }
-
-        self.channel(channel).set_duty_cycle(original_duty_state);
-
-        // Since DMA is closed before timer update event trigger DMA is turn off,
-        // this can almost always trigger a DMA FIFO error.
-        //
-        // optional TODO:
-        // clean FEIF after disable UDE
-        if !original_update_dma_state {
-            self.inner.enable_update_dma(false);
-        }
+        self.inner.waveform_up(dma, channel, duty).await;
     }
 
     /// Generate a multichannel sequence of PWM waveforms using DMA triggered by timer update events.
@@ -416,6 +346,7 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     /// Also be aware that embassy timers use one of timers internally. It is possible to
     /// switch this timer by using `time-driver-timX` feature.
     ///
+    #[inline(always)]
     pub async fn waveform_up_multi_channel(
         &mut self,
         dma: Peri<'_, impl super::UpDma<T>>,
@@ -423,148 +354,15 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
         ending_channel: Channel,
         duty: &[u16],
     ) {
-        let cr1_addr = self.inner.regs_gp16().cr1().as_ptr() as u32;
-        let start_ch_index = starting_channel.index();
-        let end_ch_index = ending_channel.index();
-
-        assert!(start_ch_index <= end_ch_index);
-
-        let ccrx_addr = self.inner.regs_gp16().ccr(start_ch_index).as_ptr() as u32;
         self.inner
-            .regs_gp16()
-            .dcr()
-            .modify(|w| w.set_dba(((ccrx_addr - cr1_addr) / 4) as u8));
-        self.inner
-            .regs_gp16()
-            .dcr()
-            .modify(|w| w.set_dbl((end_ch_index - start_ch_index) as u8));
-
-        #[allow(clippy::let_unit_value)] // eg. stm32f334
-        let req = dma.request();
-
-        let original_update_dma_state = self.inner.get_update_dma_state();
-        if !original_update_dma_state {
-            self.inner.enable_update_dma(true);
-        }
-
-        unsafe {
-            #[cfg(not(any(bdma, gpdma)))]
-            use crate::dma::{Burst, FifoThreshold};
-            use crate::dma::{Transfer, TransferOptions};
-
-            let dma_transfer_option = TransferOptions {
-                #[cfg(not(any(bdma, gpdma)))]
-                fifo_threshold: Some(FifoThreshold::Full),
-                #[cfg(not(any(bdma, gpdma)))]
-                mburst: Burst::Incr4,
-                ..Default::default()
-            };
-
-            Transfer::new_write(
-                dma,
-                req,
-                duty,
-                self.inner.regs_gp16().dmar().as_ptr() as *mut u16,
-                dma_transfer_option,
-            )
-            .await
-        };
-
-        if !original_update_dma_state {
-            self.inner.enable_update_dma(false);
-        }
+            .waveform_up_multi_channel(dma, starting_channel, ending_channel, duty)
+            .await;
     }
-}
 
-impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     /// Generate a sequence of PWM waveform
+    #[inline(always)]
     pub async fn waveform<C: TimerChannel>(&mut self, dma: Peri<'_, impl super::Dma<T, C>>, duty: &[u16]) {
-        use crate::pac::timer::vals::Ccds;
-
-        #[allow(clippy::let_unit_value)] // eg. stm32f334
-        let req = dma.request();
-
-        let cc_channel = C::CHANNEL;
-
-        let original_duty_state = self.channel(cc_channel).current_duty_cycle();
-        let original_enable_state = self.channel(cc_channel).is_enabled();
-        let original_cc_dma_on_update = self.inner.get_cc_dma_selection() == Ccds::ON_UPDATE;
-        let original_cc_dma_enabled = self.inner.get_cc_dma_enable_state(cc_channel);
-
-        // redirect CC DMA request onto Update Event
-        if !original_cc_dma_on_update {
-            self.inner.set_cc_dma_selection(Ccds::ON_UPDATE)
-        }
-
-        if !original_cc_dma_enabled {
-            self.inner.set_cc_dma_enable_state(cc_channel, true);
-        }
-
-        if !original_enable_state {
-            self.channel(cc_channel).enable();
-        }
-
-        unsafe {
-            #[cfg(not(any(bdma, gpdma)))]
-            use crate::dma::{Burst, FifoThreshold};
-            use crate::dma::{Transfer, TransferOptions};
-
-            let dma_transfer_option = TransferOptions {
-                #[cfg(not(any(bdma, gpdma)))]
-                fifo_threshold: Some(FifoThreshold::Full),
-                #[cfg(not(any(bdma, gpdma)))]
-                mburst: Burst::Incr8,
-                ..Default::default()
-            };
-
-            match self.inner.bits() {
-                TimerBits::Bits16 => {
-                    Transfer::new_write(
-                        dma,
-                        req,
-                        duty,
-                        self.inner.regs_gp16().ccr(cc_channel.index()).as_ptr() as *mut u16,
-                        dma_transfer_option,
-                    )
-                    .await
-                }
-                #[cfg(not(any(stm32l0)))]
-                TimerBits::Bits32 => {
-                    #[cfg(not(any(bdma, gpdma)))]
-                    panic!("unsupported timer bits");
-
-                    #[cfg(any(bdma, gpdma))]
-                    Transfer::new_write(
-                        dma,
-                        req,
-                        duty,
-                        self.inner.regs_gp16().ccr(cc_channel.index()).as_ptr() as *mut u32,
-                        dma_transfer_option,
-                    )
-                    .await
-                }
-            };
-        };
-
-        // restore output compare state
-        if !original_enable_state {
-            self.channel(cc_channel).disable();
-        }
-
-        self.channel(cc_channel).set_duty_cycle(original_duty_state);
-
-        // Since DMA is closed before timer Capture Compare Event trigger DMA is turn off,
-        // this can almost always trigger a DMA FIFO error.
-        //
-        // optional TODO:
-        // clean FEIF after disable UDE
-        if !original_cc_dma_enabled {
-            self.inner.set_cc_dma_enable_state(cc_channel, false);
-        }
-
-        if !original_cc_dma_on_update {
-            self.inner.set_cc_dma_selection(Ccds::ON_COMPARE)
-        }
+        self.inner.waveform(dma, duty).await;
     }
 }