From 8ac758bdee793496f842f4c267bda2b6f935a0bb Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Fri, 6 Sep 2024 09:07:29 -0500
Subject: [PATCH 01/10] embassy-stm32: Add SimplePwmChannel

---
 embassy-stm32/src/timer/simple_pwm.rs | 112 ++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index b7771bd642..f94ed1be55 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -51,6 +51,96 @@ channel_impl!(new_ch2, Ch2, Channel2Pin);
 channel_impl!(new_ch3, Ch3, Channel3Pin);
 channel_impl!(new_ch4, Ch4, Channel4Pin);
 
+/// A single channel of a pwm, obtained from [`SimplePwm::split`].
+///
+/// It is not possible to change the pwm frequency because
+/// the frequency configuration is shared with all four channels.
+pub struct SimplePwmChannel<'d, T: GeneralInstance4Channel> {
+    timer: &'d Timer<'d, T>,
+    channel: Channel,
+}
+
+// TODO: check for RMW races
+impl<'d, T: GeneralInstance4Channel> SimplePwmChannel<'d, T> {
+    /// Enable the given channel.
+    pub fn enable(&mut self) {
+        self.timer.enable_channel(self.channel, true);
+    }
+
+    /// Disable the given channel.
+    pub fn disable(&mut self) {
+        self.timer.enable_channel(self.channel, false);
+    }
+
+    /// Check whether given channel is enabled
+    pub fn is_enabled(&self) -> bool {
+        self.timer.get_channel_enable_state(self.channel)
+    }
+
+    /// Get max duty value.
+    ///
+    /// This value depends on the configured frequency and the timer's clock rate from RCC.
+    pub fn get_max_duty(&self) -> u32 {
+        self.timer.get_max_compare_value() + 1
+    }
+
+    /// Set the duty for a given channel.
+    ///
+    /// The value ranges from 0 for 0% duty, to [`get_max_duty`](Self::get_max_duty) for 100% duty, both included.
+    pub fn set_duty(&mut self, duty: u32) {
+        assert!(duty <= self.get_max_duty());
+        self.timer.set_compare_value(self.channel, duty)
+    }
+
+    /// Get the duty for a given channel.
+    ///
+    /// The value ranges from 0 for 0% duty, to [`get_max_duty`](Self::get_max_duty) for 100% duty, both included.
+    pub fn get_duty(&self) -> u32 {
+        self.timer.get_compare_value(self.channel)
+    }
+
+    /// Set the output polarity for a given channel.
+    pub fn set_polarity(&mut self, polarity: OutputPolarity) {
+        self.timer.set_output_polarity(self.channel, polarity);
+    }
+
+    /// Set the output compare mode for a given channel.
+    pub fn set_output_compare_mode(&mut self, mode: OutputCompareMode) {
+        self.timer.set_output_compare_mode(self.channel, mode);
+    }
+}
+
+/// A group of four [`SimplePwmChannel`]s, obtained from [`SimplePwm::split`].
+pub struct SimplePwmChannels<'d, T: GeneralInstance4Channel> {
+    /// Channel 1
+    pub ch1: SimplePwmChannel<'d, T>,
+    /// Channel 2
+    pub ch2: SimplePwmChannel<'d, T>,
+    /// Channel 3
+    pub ch3: SimplePwmChannel<'d, T>,
+    /// Channel 4
+    pub ch4: SimplePwmChannel<'d, T>,
+}
+
+impl<'d, T: GeneralInstance4Channel> embedded_hal_1::pwm::ErrorType for SimplePwmChannel<'d, T> {
+    type Error = core::convert::Infallible;
+}
+
+impl<'d, T: GeneralInstance4Channel> embedded_hal_1::pwm::SetDutyCycle for SimplePwmChannel<'d, T> {
+    fn max_duty_cycle(&self) -> u16 {
+        // TODO: panics if CCR is 0xFFFF
+        // TODO: rename get_max_duty to max_duty_cycle
+        unwrap!(self.get_max_duty().try_into())
+    }
+
+    fn set_duty_cycle(&mut self, duty: u16) -> Result<(), Self::Error> {
+        self.set_duty(duty.into());
+        Ok(())
+    }
+
+    // TODO: default methods?
+}
+
 /// Simple PWM driver.
 pub struct SimplePwm<'d, T: GeneralInstance4Channel> {
     inner: Timer<'d, T>,
@@ -89,6 +179,28 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
         this
     }
 
+    fn channel(&self, channel: Channel) -> SimplePwmChannel<'_, T> {
+        SimplePwmChannel {
+            timer: &self.inner,
+            channel,
+        }
+    }
+
+    /// Splits a [`SimplePwm`] into four pwm channels.
+    ///
+    /// This returns all four channels, including channels that
+    /// aren't configured with a [`PwmPin`].
+    // TODO: I hate the name "split"
+    pub fn split(&mut self) -> SimplePwmChannels<'_, T> {
+        // TODO: pre-enable channels?
+        SimplePwmChannels {
+            ch1: self.channel(Channel::Ch1),
+            ch2: self.channel(Channel::Ch2),
+            ch3: self.channel(Channel::Ch3),
+            ch4: self.channel(Channel::Ch4),
+        }
+    }
+
     /// Enable the given channel.
     pub fn enable(&mut self, channel: Channel) {
         self.inner.enable_channel(channel, true);

From f7f062e0a3f303dfc4f976907b2555f2cfb48621 Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Fri, 6 Sep 2024 10:25:29 -0500
Subject: [PATCH 02/10] Deduplicate SimplePwm's channel methods

---
 embassy-stm32/src/timer/simple_pwm.rs | 119 ++++++++++++++------------
 1 file changed, 63 insertions(+), 56 deletions(-)

diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index f94ed1be55..7179a7a599 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -51,7 +51,8 @@ channel_impl!(new_ch2, Ch2, Channel2Pin);
 channel_impl!(new_ch3, Ch3, Channel3Pin);
 channel_impl!(new_ch4, Ch4, Channel4Pin);
 
-/// A single channel of a pwm, obtained from [`SimplePwm::split`].
+/// A single channel of a pwm, obtained from [`SimplePwm::split`],
+/// [`SimplePwm::channel`], [`SimplePwm::ch1`], etc.
 ///
 /// It is not possible to change the pwm frequency because
 /// the frequency configuration is shared with all four channels.
@@ -179,13 +180,52 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
         this
     }
 
-    fn channel(&self, channel: Channel) -> SimplePwmChannel<'_, T> {
+    /// Get a single channel
+    ///
+    /// If you need to use multiple channels, use [`Self::split`].
+    pub fn channel(&mut self, channel: Channel) -> SimplePwmChannel<'_, T> {
         SimplePwmChannel {
             timer: &self.inner,
             channel,
         }
     }
 
+    /// Channel 1
+    ///
+    /// This is just a convenience wrapper around [`Self::channel`].
+    ///
+    /// If you need to use multiple channels, use [`Self::split`].
+    pub fn ch1(&mut self) -> SimplePwmChannel<'_, T> {
+        self.channel(Channel::Ch1)
+    }
+
+    /// Channel 2
+    ///
+    /// This is just a convenience wrapper around [`Self::channel`].
+    ///
+    /// If you need to use multiple channels, use [`Self::split`].
+    pub fn ch2(&mut self) -> SimplePwmChannel<'_, T> {
+        self.channel(Channel::Ch2)
+    }
+
+    /// Channel 3
+    ///
+    /// This is just a convenience wrapper around [`Self::channel`].
+    ///
+    /// If you need to use multiple channels, use [`Self::split`].
+    pub fn ch3(&mut self) -> SimplePwmChannel<'_, T> {
+        self.channel(Channel::Ch3)
+    }
+
+    /// Channel 4
+    ///
+    /// This is just a convenience wrapper around [`Self::channel`].
+    ///
+    /// If you need to use multiple channels, use [`Self::split`].
+    pub fn ch4(&mut self) -> SimplePwmChannel<'_, T> {
+        self.channel(Channel::Ch4)
+    }
+
     /// Splits a [`SimplePwm`] into four pwm channels.
     ///
     /// This returns all four channels, including channels that
@@ -193,27 +233,19 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     // TODO: I hate the name "split"
     pub fn split(&mut self) -> SimplePwmChannels<'_, T> {
         // TODO: pre-enable channels?
-        SimplePwmChannels {
-            ch1: self.channel(Channel::Ch1),
-            ch2: self.channel(Channel::Ch2),
-            ch3: self.channel(Channel::Ch3),
-            ch4: self.channel(Channel::Ch4),
-        }
-    }
-
-    /// Enable the given channel.
-    pub fn enable(&mut self, channel: Channel) {
-        self.inner.enable_channel(channel, true);
-    }
 
-    /// Disable the given channel.
-    pub fn disable(&mut self, channel: Channel) {
-        self.inner.enable_channel(channel, false);
-    }
+        // we can't use self.channel() because that takes &mut self
+        let ch = |channel| SimplePwmChannel {
+            timer: &self.inner,
+            channel,
+        };
 
-    /// Check whether given channel is enabled
-    pub fn is_enabled(&self, channel: Channel) -> bool {
-        self.inner.get_channel_enable_state(channel)
+        SimplePwmChannels {
+            ch1: ch(Channel::Ch1),
+            ch2: ch(Channel::Ch2),
+            ch3: ch(Channel::Ch3),
+            ch4: ch(Channel::Ch4),
+        }
     }
 
     /// Set PWM frequency.
@@ -236,31 +268,6 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
         self.inner.get_max_compare_value() + 1
     }
 
-    /// Set the duty for a given channel.
-    ///
-    /// The value ranges from 0 for 0% duty, to [`get_max_duty`](Self::get_max_duty) for 100% duty, both included.
-    pub fn set_duty(&mut self, channel: Channel, duty: u32) {
-        assert!(duty <= self.get_max_duty());
-        self.inner.set_compare_value(channel, duty)
-    }
-
-    /// Get the duty for a given channel.
-    ///
-    /// The value ranges from 0 for 0% duty, to [`get_max_duty`](Self::get_max_duty) for 100% duty, both included.
-    pub fn get_duty(&self, channel: Channel) -> u32 {
-        self.inner.get_compare_value(channel)
-    }
-
-    /// Set the output polarity for a given channel.
-    pub fn set_polarity(&mut self, channel: Channel, polarity: OutputPolarity) {
-        self.inner.set_output_polarity(channel, polarity);
-    }
-
-    /// Set the output compare mode for a given channel.
-    pub fn set_output_compare_mode(&mut self, channel: Channel, mode: OutputCompareMode) {
-        self.inner.set_output_compare_mode(channel, mode);
-    }
-
     /// Generate a sequence of PWM waveform
     ///
     /// Note:  
@@ -276,8 +283,8 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
         #[allow(clippy::let_unit_value)] // eg. stm32f334
         let req = dma.request();
 
-        let original_duty_state = self.get_duty(channel);
-        let original_enable_state = self.is_enabled(channel);
+        let original_duty_state = self.channel(channel).get_duty();
+        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 {
@@ -285,7 +292,7 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
         }
 
         if !original_enable_state {
-            self.enable(channel);
+            self.channel(channel).enable();
         }
 
         unsafe {
@@ -313,10 +320,10 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
 
         // restore output compare state
         if !original_enable_state {
-            self.disable(channel);
+            self.channel(channel).disable();
         }
 
-        self.set_duty(channel, original_duty_state);
+        self.channel(channel).set_duty(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.
@@ -346,8 +353,8 @@ macro_rules! impl_waveform_chx {
 
                 let cc_channel = Channel::$cc_ch;
 
-                let original_duty_state = self.get_duty(cc_channel);
-                let original_enable_state = self.is_enabled(cc_channel);
+                let original_duty_state = self.channel(cc_channel).get_duty();
+                let original_enable_state = self.channel(cc_channel).is_enabled();
                 let original_cc_dma_on_update = self.inner.get_cc_dma_selection() == Ccds::ONUPDATE;
                 let original_cc_dma_enabled = self.inner.get_cc_dma_enable_state(cc_channel);
 
@@ -361,7 +368,7 @@ macro_rules! impl_waveform_chx {
                 }
 
                 if !original_enable_state {
-                    self.enable(cc_channel);
+                    self.channel(cc_channel).enable();
                 }
 
                 unsafe {
@@ -389,10 +396,10 @@ macro_rules! impl_waveform_chx {
 
                 // restore output compare state
                 if !original_enable_state {
-                    self.disable(cc_channel);
+                    self.channel(cc_channel).disable();
                 }
 
-                self.set_duty(cc_channel, original_duty_state);
+                self.channel(cc_channel).set_duty(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.

From cfc76cec711447300e2d957439a32d6ad418a58c Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Fri, 6 Sep 2024 13:29:54 -0500
Subject: [PATCH 03/10] Match embedded-hal api

---
 embassy-stm32/src/timer/simple_pwm.rs | 105 ++++++++++++++++++--------
 1 file changed, 72 insertions(+), 33 deletions(-)

diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index 7179a7a599..23c727731c 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -81,23 +81,45 @@ impl<'d, T: GeneralInstance4Channel> SimplePwmChannel<'d, T> {
     /// Get max duty value.
     ///
     /// This value depends on the configured frequency and the timer's clock rate from RCC.
-    pub fn get_max_duty(&self) -> u32 {
-        self.timer.get_max_compare_value() + 1
+    pub fn max_duty_cycle(&self) -> u16 {
+        unwrap!(self.timer.get_max_compare_value().checked_add(1))
     }
 
     /// Set the duty for a given channel.
     ///
-    /// The value ranges from 0 for 0% duty, to [`get_max_duty`](Self::get_max_duty) for 100% duty, both included.
-    pub fn set_duty(&mut self, duty: u32) {
-        assert!(duty <= self.get_max_duty());
-        self.timer.set_compare_value(self.channel, duty)
+    /// The value ranges from 0 for 0% duty, to [`max_duty_cycle`](Self::max_duty_cycle) for 100% duty, both included.
+    pub fn set_duty_cycle(&mut self, duty: u16) {
+        assert!(duty <= (*self).max_duty_cycle());
+        self.timer.set_compare_value(self.channel, duty.into())
+    }
+
+    fn set_duty_cycle_fully_off(&mut self) {
+        self.set_duty_cycle(0);
+    }
+
+    fn set_duty_cycle_fully_on(&mut self) {
+        self.set_duty_cycle((*self).max_duty_cycle());
+    }
+
+    fn set_duty_cycle_fraction(&mut self, num: u16, denom: u16) {
+        assert!(denom != 0);
+        assert!(num <= denom);
+        let duty = u32::from(num) * u32::from(self.max_duty_cycle()) / u32::from(denom);
+
+        // This is safe because we know that `num <= denom`, so `duty <= self.max_duty_cycle()` (u16)
+        #[allow(clippy::cast_possible_truncation)]
+        self.set_duty_cycle(duty as u16);
+    }
+
+    fn set_duty_cycle_percent(&mut self, percent: u8) {
+        self.set_duty_cycle_fraction(u16::from(percent), 100)
     }
 
     /// Get the duty for a given channel.
     ///
-    /// The value ranges from 0 for 0% duty, to [`get_max_duty`](Self::get_max_duty) for 100% duty, both included.
-    pub fn get_duty(&self) -> u32 {
-        self.timer.get_compare_value(self.channel)
+    /// The value ranges from 0 for 0% duty, to [`max_duty_cycle`](Self::max_duty_cycle) for 100% duty, both included.
+    pub fn get_duty(&self) -> u16 {
+        unwrap!(self.timer.get_compare_value(self.channel).try_into())
     }
 
     /// Set the output polarity for a given channel.
@@ -123,25 +145,6 @@ pub struct SimplePwmChannels<'d, T: GeneralInstance4Channel> {
     pub ch4: SimplePwmChannel<'d, T>,
 }
 
-impl<'d, T: GeneralInstance4Channel> embedded_hal_1::pwm::ErrorType for SimplePwmChannel<'d, T> {
-    type Error = core::convert::Infallible;
-}
-
-impl<'d, T: GeneralInstance4Channel> embedded_hal_1::pwm::SetDutyCycle for SimplePwmChannel<'d, T> {
-    fn max_duty_cycle(&self) -> u16 {
-        // TODO: panics if CCR is 0xFFFF
-        // TODO: rename get_max_duty to max_duty_cycle
-        unwrap!(self.get_max_duty().try_into())
-    }
-
-    fn set_duty_cycle(&mut self, duty: u16) -> Result<(), Self::Error> {
-        self.set_duty(duty.into());
-        Ok(())
-    }
-
-    // TODO: default methods?
-}
-
 /// Simple PWM driver.
 pub struct SimplePwm<'d, T: GeneralInstance4Channel> {
     inner: Timer<'d, T>,
@@ -253,6 +256,7 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     /// Note: when you call this, the max duty value changes, so you will have to
     /// call `set_duty` on all channels with the duty calculated based on the new max duty.
     pub fn set_frequency(&mut self, freq: Hertz) {
+        // TODO: prevent ARR = u16::MAX?
         let multiplier = if self.inner.get_counting_mode().is_center_aligned() {
             2u8
         } else {
@@ -264,8 +268,8 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     /// Get max duty value.
     ///
     /// This value depends on the configured frequency and the timer's clock rate from RCC.
-    pub fn get_max_duty(&self) -> u32 {
-        self.inner.get_max_compare_value() + 1
+    pub fn max_duty_cycle(&self) -> u16 {
+        unwrap!(self.inner.get_max_compare_value().checked_add(1))
     }
 
     /// Generate a sequence of PWM waveform
@@ -323,7 +327,7 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
             self.channel(channel).disable();
         }
 
-        self.channel(channel).set_duty(original_duty_state);
+        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.
@@ -399,7 +403,7 @@ macro_rules! impl_waveform_chx {
                     self.channel(cc_channel).disable();
                 }
 
-                self.channel(cc_channel).set_duty(original_duty_state);
+                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.
@@ -423,6 +427,41 @@ impl_waveform_chx!(waveform_ch2, Ch2Dma, Ch2);
 impl_waveform_chx!(waveform_ch3, Ch3Dma, Ch3);
 impl_waveform_chx!(waveform_ch4, Ch4Dma, Ch4);
 
+impl<'d, T: GeneralInstance4Channel> embedded_hal_1::pwm::ErrorType for SimplePwmChannel<'d, T> {
+    type Error = core::convert::Infallible;
+}
+
+impl<'d, T: GeneralInstance4Channel> embedded_hal_1::pwm::SetDutyCycle for SimplePwmChannel<'d, T> {
+    fn max_duty_cycle(&self) -> u16 {
+        self.max_duty_cycle()
+    }
+
+    fn set_duty_cycle(&mut self, duty: u16) -> Result<(), Self::Error> {
+        self.set_duty_cycle(duty);
+        Ok(())
+    }
+
+    fn set_duty_cycle_fully_off(&mut self) -> Result<(), Self::Error> {
+        self.set_duty_cycle_fully_off();
+        Ok(())
+    }
+
+    fn set_duty_cycle_fully_on(&mut self) -> Result<(), Self::Error> {
+        self.set_duty_cycle_fully_on();
+        Ok(())
+    }
+
+    fn set_duty_cycle_fraction(&mut self, num: u16, denom: u16) -> Result<(), Self::Error> {
+        self.set_duty_cycle_fraction(num, denom);
+        Ok(())
+    }
+
+    fn set_duty_cycle_percent(&mut self, percent: u8) -> Result<(), Self::Error> {
+        self.set_duty_cycle_percent(percent);
+        Ok(())
+    }
+}
+
 impl<'d, T: GeneralInstance4Channel> embedded_hal_02::Pwm for SimplePwm<'d, T> {
     type Channel = Channel;
     type Time = Hertz;
@@ -449,7 +488,7 @@ impl<'d, T: GeneralInstance4Channel> embedded_hal_02::Pwm for SimplePwm<'d, T> {
     }
 
     fn set_duty(&mut self, channel: Self::Channel, duty: Self::Duty) {
-        assert!(duty <= self.get_max_duty());
+        assert!(duty <= self.max_duty_cycle() as u32);
         self.inner.set_compare_value(channel, duty)
     }
 

From 1a8977db7837a5635d3c5e0ae8213d1b3181ffb7 Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Fri, 6 Sep 2024 13:53:49 -0500
Subject: [PATCH 04/10] Update examples

---
 examples/stm32f4/src/bin/pwm.rs           | 18 +++++++++---------
 examples/stm32f4/src/bin/ws2812_pwm.rs    |  4 ++--
 examples/stm32g0/src/bin/input_capture.rs |  8 ++++----
 examples/stm32g0/src/bin/pwm_input.rs     |  9 ++++-----
 examples/stm32g4/src/bin/pwm.rs           | 18 +++++++++---------
 examples/stm32h7/src/bin/pwm.rs           | 18 +++++++++---------
 6 files changed, 37 insertions(+), 38 deletions(-)

diff --git a/examples/stm32f4/src/bin/pwm.rs b/examples/stm32f4/src/bin/pwm.rs
index 8844a9f0e3..0cd65a2588 100644
--- a/examples/stm32f4/src/bin/pwm.rs
+++ b/examples/stm32f4/src/bin/pwm.rs
@@ -15,22 +15,22 @@ async fn main(_spawner: Spawner) {
     let p = embassy_stm32::init(Default::default());
     info!("Hello World!");
 
-    let ch1 = PwmPin::new_ch1(p.PE9, OutputType::PushPull);
-    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1), None, None, None, khz(10), Default::default());
-    let max = pwm.get_max_duty();
-    pwm.enable(Channel::Ch1);
+    let ch1_pin = PwmPin::new_ch1(p.PE9, OutputType::PushPull);
+    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1_pin), None, None, None, khz(10), Default::default());
+    let mut ch1 = pwm.ch1();
+    ch1.enable();
 
     info!("PWM initialized");
-    info!("PWM max duty {}", max);
+    info!("PWM max duty {}", ch1.max_duty_cycle());
 
     loop {
-        pwm.set_duty(Channel::Ch1, 0);
+        ch1.set_duty_cycle_fully_off();
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max / 4);
+        ch1.set_duty_cycle_fraction(1, 4);
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max / 2);
+        ch1.set_dutycycle_fraction(1, 2);
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max - 1);
+        ch1.set_duty_cycle(ch1.max_duty_cycle() - 1);
         Timer::after_millis(300).await;
     }
 }
diff --git a/examples/stm32f4/src/bin/ws2812_pwm.rs b/examples/stm32f4/src/bin/ws2812_pwm.rs
index cbaff75fcb..7a9fa302b6 100644
--- a/examples/stm32f4/src/bin/ws2812_pwm.rs
+++ b/examples/stm32f4/src/bin/ws2812_pwm.rs
@@ -61,7 +61,7 @@ async fn main(_spawner: Spawner) {
     // construct ws2812 non-return-to-zero (NRZ) code bit by bit
     // ws2812 only need 24 bits for each LED, but we add one bit more to keep PWM output low
 
-    let max_duty = ws2812_pwm.get_max_duty() as u16;
+    let max_duty = ws2812_pwm.max_duty_cycle();
     let n0 = 8 * max_duty / 25; // ws2812 Bit 0 high level timing
     let n1 = 2 * n0; // ws2812 Bit 1 high level timing
 
@@ -84,7 +84,7 @@ async fn main(_spawner: Spawner) {
     let pwm_channel = Channel::Ch1;
 
     // make sure PWM output keep low on first start
-    ws2812_pwm.set_duty(pwm_channel, 0);
+    ws2812_pwm.channel(pwm_channel).set_duty(0);
 
     // flip color at 2 Hz
     let mut ticker = Ticker::every(Duration::from_millis(500));
diff --git a/examples/stm32g0/src/bin/input_capture.rs b/examples/stm32g0/src/bin/input_capture.rs
index 69fdae96db..bc814cb13c 100644
--- a/examples/stm32g0/src/bin/input_capture.rs
+++ b/examples/stm32g0/src/bin/input_capture.rs
@@ -47,10 +47,10 @@ async fn main(spawner: Spawner) {
     unwrap!(spawner.spawn(blinky(p.PB1)));
 
     // Connect PB1 and PA8 with a 1k Ohm resistor
-    let ch1 = PwmPin::new_ch1(p.PA8, OutputType::PushPull);
-    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1), None, None, None, khz(1), Default::default());
-    pwm.enable(Channel::Ch1);
-    pwm.set_duty(Channel::Ch1, 50);
+    let ch1_pin = PwmPin::new_ch1(p.PA8, OutputType::PushPull);
+    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1_pin), None, None, None, khz(1), Default::default());
+    pwm.ch1().enable();
+    pwm.ch1().set_duty_cycle(50);
 
     let ch1 = CapturePin::new_ch1(p.PA0, Pull::None);
     let mut ic = InputCapture::new(p.TIM2, Some(ch1), None, None, None, Irqs, khz(1000), Default::default());
diff --git a/examples/stm32g0/src/bin/pwm_input.rs b/examples/stm32g0/src/bin/pwm_input.rs
index 152ecda860..983705e2f6 100644
--- a/examples/stm32g0/src/bin/pwm_input.rs
+++ b/examples/stm32g0/src/bin/pwm_input.rs
@@ -43,11 +43,10 @@ async fn main(spawner: Spawner) {
 
     unwrap!(spawner.spawn(blinky(p.PB1)));
     // Connect PA8 and PA6 with a 1k Ohm resistor
-    let ch1 = PwmPin::new_ch1(p.PA8, OutputType::PushPull);
-    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1), None, None, None, khz(1), Default::default());
-    let max = pwm.get_max_duty();
-    pwm.set_duty(Channel::Ch1, max / 4);
-    pwm.enable(Channel::Ch1);
+    let ch1_pin = PwmPin::new_ch1(p.PA8, OutputType::PushPull);
+    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1_pin), None, None, None, khz(1), Default::default());
+    pwm.ch1().set_duty_cycle_fraction(1, 4);
+    pwm.ch1().enable();
 
     let mut pwm_input = PwmInput::new(p.TIM2, p.PA0, Pull::None, khz(1000));
     pwm_input.enable();
diff --git a/examples/stm32g4/src/bin/pwm.rs b/examples/stm32g4/src/bin/pwm.rs
index d4809a481f..3833be58f8 100644
--- a/examples/stm32g4/src/bin/pwm.rs
+++ b/examples/stm32g4/src/bin/pwm.rs
@@ -15,22 +15,22 @@ async fn main(_spawner: Spawner) {
     let p = embassy_stm32::init(Default::default());
     info!("Hello World!");
 
-    let ch1 = PwmPin::new_ch1(p.PC0, OutputType::PushPull);
-    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1), None, None, None, khz(10), Default::default());
-    let max = pwm.get_max_duty();
-    pwm.enable(Channel::Ch1);
+    let ch1_pin = PwmPin::new_ch1(p.PC0, OutputType::PushPull);
+    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1_pin), None, None, None, khz(10), Default::default());
+    let mut ch1 = pwm.ch1();
+    ch1.enable();
 
     info!("PWM initialized");
-    info!("PWM max duty {}", max);
+    info!("PWM max duty {}", ch1.max_duty_cycle());
 
     loop {
-        pwm.set_duty(Channel::Ch1, 0);
+        ch1.set_duty_cycle_fully_off();
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max / 4);
+        ch1.set_duty_cycle_fraction(1, 4);
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max / 2);
+        ch1.set_dutycycle_fraction(1, 2);
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max - 1);
+        ch1.set_duty_cycle(ch1.max_duty_cycle() - 1);
         Timer::after_millis(300).await;
     }
 }
diff --git a/examples/stm32h7/src/bin/pwm.rs b/examples/stm32h7/src/bin/pwm.rs
index 1e48ba67b5..1d0b89e973 100644
--- a/examples/stm32h7/src/bin/pwm.rs
+++ b/examples/stm32h7/src/bin/pwm.rs
@@ -37,22 +37,22 @@ async fn main(_spawner: Spawner) {
     let p = embassy_stm32::init(config);
     info!("Hello World!");
 
-    let ch1 = PwmPin::new_ch1(p.PA6, OutputType::PushPull);
-    let mut pwm = SimplePwm::new(p.TIM3, Some(ch1), None, None, None, khz(10), Default::default());
-    let max = pwm.get_max_duty();
-    pwm.enable(Channel::Ch1);
+    let ch1_pin = PwmPin::new_ch1(p.PA6, OutputType::PushPull);
+    let mut pwm = SimplePwm::new(p.TIM3, Some(ch1_pin), None, None, None, khz(10), Default::default());
+    let mut ch1 = pwm.ch1;
+    ch1.enable();
 
     info!("PWM initialized");
-    info!("PWM max duty {}", max);
+    info!("PWM max duty {}", ch1.max_duty_cycle());
 
     loop {
-        pwm.set_duty(Channel::Ch1, 0);
+        ch1.set_duty_cycle_fully_off();
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max / 4);
+        ch1.set_duty_cycle_fraction(1, 4);
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max / 2);
+        ch1.set_dutycycle_fraction(1, 2);
         Timer::after_millis(300).await;
-        pwm.set_duty(Channel::Ch1, max - 1);
+        ch1.set_duty_cycle(ch1.max_duty_cycle() - 1);
         Timer::after_millis(300).await;
     }
 }

From 71e49839fc350c19c2fd7eebda0fe76afbfdb157 Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Fri, 6 Sep 2024 14:01:10 -0500
Subject: [PATCH 05/10] oops

---
 embassy-stm32/src/timer/simple_pwm.rs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index 23c727731c..885abd23f2 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -82,7 +82,9 @@ impl<'d, T: GeneralInstance4Channel> SimplePwmChannel<'d, T> {
     ///
     /// This value depends on the configured frequency and the timer's clock rate from RCC.
     pub fn max_duty_cycle(&self) -> u16 {
-        unwrap!(self.timer.get_max_compare_value().checked_add(1))
+        let max = self.timer.get_max_compare_value();
+        assert!(max < u16::MAX as u32);
+        max as u16 + 1
     }
 
     /// Set the duty for a given channel.
@@ -269,7 +271,9 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     ///
     /// This value depends on the configured frequency and the timer's clock rate from RCC.
     pub fn max_duty_cycle(&self) -> u16 {
-        unwrap!(self.inner.get_max_compare_value().checked_add(1))
+        let max = self.inner.get_max_compare_value();
+        assert!(max < u16::MAX as u32);
+        max as u16 + 1
     }
 
     /// Generate a sequence of PWM waveform

From f571ab9d600cdb6e3f146a6b2bf464fb817065af Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Fri, 6 Sep 2024 14:04:58 -0500
Subject: [PATCH 06/10] oops again

---
 embassy-stm32/src/timer/simple_pwm.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index 885abd23f2..8673bce545 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -95,15 +95,15 @@ impl<'d, T: GeneralInstance4Channel> SimplePwmChannel<'d, T> {
         self.timer.set_compare_value(self.channel, duty.into())
     }
 
-    fn set_duty_cycle_fully_off(&mut self) {
+    pub fn set_duty_cycle_fully_off(&mut self) {
         self.set_duty_cycle(0);
     }
 
-    fn set_duty_cycle_fully_on(&mut self) {
+    pub fn set_duty_cycle_fully_on(&mut self) {
         self.set_duty_cycle((*self).max_duty_cycle());
     }
 
-    fn set_duty_cycle_fraction(&mut self, num: u16, denom: u16) {
+    pub fn set_duty_cycle_fraction(&mut self, num: u16, denom: u16) {
         assert!(denom != 0);
         assert!(num <= denom);
         let duty = u32::from(num) * u32::from(self.max_duty_cycle()) / u32::from(denom);
@@ -113,7 +113,7 @@ impl<'d, T: GeneralInstance4Channel> SimplePwmChannel<'d, T> {
         self.set_duty_cycle(duty as u16);
     }
 
-    fn set_duty_cycle_percent(&mut self, percent: u8) {
+    pub fn set_duty_cycle_percent(&mut self, percent: u8) {
         self.set_duty_cycle_fraction(u16::from(percent), 100)
     }
 

From d24c47a3ff4a82786b67f06d876bafb1bdabc163 Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Fri, 6 Sep 2024 14:08:22 -0500
Subject: [PATCH 07/10] Missing docs

---
 embassy-stm32/src/timer/simple_pwm.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index 8673bce545..47188053c4 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -95,14 +95,20 @@ impl<'d, T: GeneralInstance4Channel> SimplePwmChannel<'d, T> {
         self.timer.set_compare_value(self.channel, duty.into())
     }
 
+    /// Set the duty cycle to 0%, or always inactive.
     pub fn set_duty_cycle_fully_off(&mut self) {
         self.set_duty_cycle(0);
     }
 
+    /// Set the duty cycle to 100%, or always active.
     pub fn set_duty_cycle_fully_on(&mut self) {
         self.set_duty_cycle((*self).max_duty_cycle());
     }
 
+    /// Set the duty cycle to `num / denom`.
+    ///
+    /// The caller is responsible for ensuring that `num` is less than or equal to `denom`,
+    /// and that `denom` is not zero.
     pub fn set_duty_cycle_fraction(&mut self, num: u16, denom: u16) {
         assert!(denom != 0);
         assert!(num <= denom);
@@ -113,6 +119,9 @@ impl<'d, T: GeneralInstance4Channel> SimplePwmChannel<'d, T> {
         self.set_duty_cycle(duty as u16);
     }
 
+    /// Set the duty cycle to `percent / 100`
+    ///
+    /// The caller is responsible for ensuring that `percent` is less than or equal to 100.
     pub fn set_duty_cycle_percent(&mut self, percent: u8) {
         self.set_duty_cycle_fraction(u16::from(percent), 100)
     }

From b8beaba6df08c4455f55780a6e13191d95ad9eec Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Fri, 6 Sep 2024 15:08:58 -0500
Subject: [PATCH 08/10] last oops I promise

---
 embassy-stm32/src/timer/simple_pwm.rs  | 6 +++---
 examples/stm32f4/src/bin/pwm.rs        | 3 +--
 examples/stm32f4/src/bin/ws2812_pwm.rs | 2 +-
 examples/stm32g0/src/bin/pwm_input.rs  | 1 -
 examples/stm32g4/src/bin/pwm.rs        | 3 +--
 examples/stm32h7/src/bin/pwm.rs        | 5 ++---
 6 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index 47188053c4..7e2e9c2023 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -129,7 +129,7 @@ impl<'d, T: GeneralInstance4Channel> SimplePwmChannel<'d, T> {
     /// Get the duty for a given channel.
     ///
     /// The value ranges from 0 for 0% duty, to [`max_duty_cycle`](Self::max_duty_cycle) for 100% duty, both included.
-    pub fn get_duty(&self) -> u16 {
+    pub fn current_duty_cycle(&self) -> u16 {
         unwrap!(self.timer.get_compare_value(self.channel).try_into())
     }
 
@@ -300,7 +300,7 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
         #[allow(clippy::let_unit_value)] // eg. stm32f334
         let req = dma.request();
 
-        let original_duty_state = self.channel(channel).get_duty();
+        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();
 
@@ -370,7 +370,7 @@ macro_rules! impl_waveform_chx {
 
                 let cc_channel = Channel::$cc_ch;
 
-                let original_duty_state = self.channel(cc_channel).get_duty();
+                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::ONUPDATE;
                 let original_cc_dma_enabled = self.inner.get_cc_dma_enable_state(cc_channel);
diff --git a/examples/stm32f4/src/bin/pwm.rs b/examples/stm32f4/src/bin/pwm.rs
index 0cd65a2588..04811162bb 100644
--- a/examples/stm32f4/src/bin/pwm.rs
+++ b/examples/stm32f4/src/bin/pwm.rs
@@ -6,7 +6,6 @@ use embassy_executor::Spawner;
 use embassy_stm32::gpio::OutputType;
 use embassy_stm32::time::khz;
 use embassy_stm32::timer::simple_pwm::{PwmPin, SimplePwm};
-use embassy_stm32::timer::Channel;
 use embassy_time::Timer;
 use {defmt_rtt as _, panic_probe as _};
 
@@ -28,7 +27,7 @@ async fn main(_spawner: Spawner) {
         Timer::after_millis(300).await;
         ch1.set_duty_cycle_fraction(1, 4);
         Timer::after_millis(300).await;
-        ch1.set_dutycycle_fraction(1, 2);
+        ch1.set_duty_cycle_fraction(1, 2);
         Timer::after_millis(300).await;
         ch1.set_duty_cycle(ch1.max_duty_cycle() - 1);
         Timer::after_millis(300).await;
diff --git a/examples/stm32f4/src/bin/ws2812_pwm.rs b/examples/stm32f4/src/bin/ws2812_pwm.rs
index 7a9fa302b6..3ab93d6e0a 100644
--- a/examples/stm32f4/src/bin/ws2812_pwm.rs
+++ b/examples/stm32f4/src/bin/ws2812_pwm.rs
@@ -84,7 +84,7 @@ async fn main(_spawner: Spawner) {
     let pwm_channel = Channel::Ch1;
 
     // make sure PWM output keep low on first start
-    ws2812_pwm.channel(pwm_channel).set_duty(0);
+    ws2812_pwm.channel(pwm_channel).set_duty_cycle(0);
 
     // flip color at 2 Hz
     let mut ticker = Ticker::every(Duration::from_millis(500));
diff --git a/examples/stm32g0/src/bin/pwm_input.rs b/examples/stm32g0/src/bin/pwm_input.rs
index 983705e2f6..db9cf4f8a1 100644
--- a/examples/stm32g0/src/bin/pwm_input.rs
+++ b/examples/stm32g0/src/bin/pwm_input.rs
@@ -14,7 +14,6 @@ use embassy_stm32::gpio::{Level, Output, OutputType, Pull, Speed};
 use embassy_stm32::time::khz;
 use embassy_stm32::timer::pwm_input::PwmInput;
 use embassy_stm32::timer::simple_pwm::{PwmPin, SimplePwm};
-use embassy_stm32::timer::Channel;
 use embassy_stm32::{bind_interrupts, peripherals, timer};
 use embassy_time::Timer;
 use {defmt_rtt as _, panic_probe as _};
diff --git a/examples/stm32g4/src/bin/pwm.rs b/examples/stm32g4/src/bin/pwm.rs
index 3833be58f8..6c965012c8 100644
--- a/examples/stm32g4/src/bin/pwm.rs
+++ b/examples/stm32g4/src/bin/pwm.rs
@@ -6,7 +6,6 @@ use embassy_executor::Spawner;
 use embassy_stm32::gpio::OutputType;
 use embassy_stm32::time::khz;
 use embassy_stm32::timer::simple_pwm::{PwmPin, SimplePwm};
-use embassy_stm32::timer::Channel;
 use embassy_time::Timer;
 use {defmt_rtt as _, panic_probe as _};
 
@@ -28,7 +27,7 @@ async fn main(_spawner: Spawner) {
         Timer::after_millis(300).await;
         ch1.set_duty_cycle_fraction(1, 4);
         Timer::after_millis(300).await;
-        ch1.set_dutycycle_fraction(1, 2);
+        ch1.set_duty_cycle_fraction(1, 2);
         Timer::after_millis(300).await;
         ch1.set_duty_cycle(ch1.max_duty_cycle() - 1);
         Timer::after_millis(300).await;
diff --git a/examples/stm32h7/src/bin/pwm.rs b/examples/stm32h7/src/bin/pwm.rs
index 1d0b89e973..a1c53fc3f7 100644
--- a/examples/stm32h7/src/bin/pwm.rs
+++ b/examples/stm32h7/src/bin/pwm.rs
@@ -6,7 +6,6 @@ use embassy_executor::Spawner;
 use embassy_stm32::gpio::OutputType;
 use embassy_stm32::time::khz;
 use embassy_stm32::timer::simple_pwm::{PwmPin, SimplePwm};
-use embassy_stm32::timer::Channel;
 use embassy_stm32::Config;
 use embassy_time::Timer;
 use {defmt_rtt as _, panic_probe as _};
@@ -39,7 +38,7 @@ async fn main(_spawner: Spawner) {
 
     let ch1_pin = PwmPin::new_ch1(p.PA6, OutputType::PushPull);
     let mut pwm = SimplePwm::new(p.TIM3, Some(ch1_pin), None, None, None, khz(10), Default::default());
-    let mut ch1 = pwm.ch1;
+    let mut ch1 = pwm.ch1();
     ch1.enable();
 
     info!("PWM initialized");
@@ -50,7 +49,7 @@ async fn main(_spawner: Spawner) {
         Timer::after_millis(300).await;
         ch1.set_duty_cycle_fraction(1, 4);
         Timer::after_millis(300).await;
-        ch1.set_dutycycle_fraction(1, 2);
+        ch1.set_duty_cycle_fraction(1, 2);
         Timer::after_millis(300).await;
         ch1.set_duty_cycle(ch1.max_duty_cycle() - 1);
         Timer::after_millis(300).await;

From df06c2bbfe51e22e0d3eda3d760839f617ffbd96 Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Sat, 7 Sep 2024 11:13:18 -0500
Subject: [PATCH 09/10] wip: split by value

---
 embassy-stm32/src/timer/low_level.rs  |  9 +++++++++
 embassy-stm32/src/timer/mod.rs        |  1 +
 embassy-stm32/src/timer/simple_pwm.rs | 17 +++++++++++------
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/embassy-stm32/src/timer/low_level.rs b/embassy-stm32/src/timer/low_level.rs
index e643722aaa..6377054c5d 100644
--- a/embassy-stm32/src/timer/low_level.rs
+++ b/embassy-stm32/src/timer/low_level.rs
@@ -6,6 +6,8 @@
 //!
 //! The available functionality depends on the timer type.
 
+use core::mem::ManuallyDrop;
+
 use embassy_hal_internal::{into_ref, Peripheral, PeripheralRef};
 // Re-export useful enums
 pub use stm32_metapac::timer::vals::{FilterValue, Sms as SlaveMode, Ts as TriggerSource};
@@ -198,6 +200,13 @@ impl<'d, T: CoreInstance> Timer<'d, T> {
         Self { tim }
     }
 
+    pub(crate) unsafe fn clone_unchecked(&self) -> ManuallyDrop<Self> {
+        // this doesn't work for some reason
+        // let tim = unsafe { self.tim.clone_unchecked() };
+        let tim = todo!();
+        ManuallyDrop::new(Self { tim })
+    }
+
     /// Get access to the virutal core 16bit timer registers.
     ///
     /// Note: This works even if the timer is more capable, because registers
diff --git a/embassy-stm32/src/timer/mod.rs b/embassy-stm32/src/timer/mod.rs
index 25782ee138..6cf22689b9 100644
--- a/embassy-stm32/src/timer/mod.rs
+++ b/embassy-stm32/src/timer/mod.rs
@@ -2,6 +2,7 @@
 
 use core::marker::PhantomData;
 
+use embassy_hal_internal::Peripheral;
 use embassy_sync::waitqueue::AtomicWaker;
 
 #[cfg(not(stm32l0))]
diff --git a/embassy-stm32/src/timer/simple_pwm.rs b/embassy-stm32/src/timer/simple_pwm.rs
index 7e2e9c2023..9e4a09095f 100644
--- a/embassy-stm32/src/timer/simple_pwm.rs
+++ b/embassy-stm32/src/timer/simple_pwm.rs
@@ -1,6 +1,7 @@
 //! Simple PWM driver.
 
 use core::marker::PhantomData;
+use core::mem::ManuallyDrop;
 
 use embassy_hal_internal::{into_ref, PeripheralRef};
 
@@ -57,7 +58,7 @@ channel_impl!(new_ch4, Ch4, Channel4Pin);
 /// It is not possible to change the pwm frequency because
 /// the frequency configuration is shared with all four channels.
 pub struct SimplePwmChannel<'d, T: GeneralInstance4Channel> {
-    timer: &'d Timer<'d, T>,
+    timer: ManuallyDrop<Timer<'d, T>>,
     channel: Channel,
 }
 
@@ -199,7 +200,7 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     /// If you need to use multiple channels, use [`Self::split`].
     pub fn channel(&mut self, channel: Channel) -> SimplePwmChannel<'_, T> {
         SimplePwmChannel {
-            timer: &self.inner,
+            timer: unsafe { self.inner.clone_unchecked() },
             channel,
         }
     }
@@ -245,12 +246,16 @@ impl<'d, T: GeneralInstance4Channel> SimplePwm<'d, T> {
     /// This returns all four channels, including channels that
     /// aren't configured with a [`PwmPin`].
     // TODO: I hate the name "split"
-    pub fn split(&mut self) -> SimplePwmChannels<'_, T> {
-        // TODO: pre-enable channels?
+    pub fn split(self) -> SimplePwmChannels<'static, T>
+    where
+        // must be static because the timer will never be dropped/disabled
+        'd: 'static,
+    {
+        // without this, the timer would be disabled at the end of this function
+        let timer = ManuallyDrop::new(self.inner);
 
-        // we can't use self.channel() because that takes &mut self
         let ch = |channel| SimplePwmChannel {
-            timer: &self.inner,
+            timer: unsafe { timer.clone_unchecked() },
             channel,
         };
 

From f2646b29a6b0a741fc424f88c5ca3dc25fce9369 Mon Sep 17 00:00:00 2001
From: Grant Miller <GrantM11235@gmail.com>
Date: Sat, 21 Sep 2024 07:52:54 -0500
Subject: [PATCH 10/10] Make clone_unchecked work

---
 embassy-stm32/src/timer/low_level.rs | 4 +---
 embassy-stm32/src/timer/mod.rs       | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/embassy-stm32/src/timer/low_level.rs b/embassy-stm32/src/timer/low_level.rs
index 6377054c5d..3136ea4e99 100644
--- a/embassy-stm32/src/timer/low_level.rs
+++ b/embassy-stm32/src/timer/low_level.rs
@@ -201,9 +201,7 @@ impl<'d, T: CoreInstance> Timer<'d, T> {
     }
 
     pub(crate) unsafe fn clone_unchecked(&self) -> ManuallyDrop<Self> {
-        // this doesn't work for some reason
-        // let tim = unsafe { self.tim.clone_unchecked() };
-        let tim = todo!();
+        let tim = unsafe { self.tim.clone_unchecked() };
         ManuallyDrop::new(Self { tim })
     }
 
diff --git a/embassy-stm32/src/timer/mod.rs b/embassy-stm32/src/timer/mod.rs
index 6cf22689b9..aa9dd91d9b 100644
--- a/embassy-stm32/src/timer/mod.rs
+++ b/embassy-stm32/src/timer/mod.rs
@@ -67,7 +67,7 @@ impl State {
     }
 }
 
-trait SealedInstance: RccPeripheral {
+trait SealedInstance: RccPeripheral + Peripheral<P = Self> {
     /// Async state for this timer
     fn state() -> &'static State;
 }