Merge #856

856: embassy-stm32/pwm: Generalize channel selection r=Dirbaio a=bgamari

550da471be7b56927b50b5955a6de0916ebe6b1f previously refactored the STM32
PWM logic in such a way to preclude use of non-contiguous channels (e.g.
channel 2 but not channel 1). Refactor it yet again to yet again allow
this sort of usage.

Co-authored-by: Ben Gamari <[email protected]>
Co-authored-by: Dario Nieuwenhuis <[email protected]>

4 files changed, 52 insertions, 63 deletions

diff --git a/embassy-stm32/src/pwm/simple_pwm.rs b/embassy-stm32/src/pwm/simple_pwm.rs
index 7b2cdbc01..b045a2d78 100644
--- a/embassy-stm32/src/pwm/simple_pwm.rs
+++ b/embassy-stm32/src/pwm/simple_pwm.rs
@@ -1,85 +1,71 @@
+use core::marker::PhantomData;
+
 use embassy_hal_common::{into_ref, PeripheralRef};
 
 use super::*;
 #[allow(unused_imports)]
 use crate::gpio::sealed::{AFType, Pin};
+use crate::gpio::AnyPin;
 use crate::time::Hertz;
 use crate::Peripheral;
 
-pub struct SimplePwm<'d, T> {
-    inner: PeripheralRef<'d, T>,
+pub struct Ch1;
+pub struct Ch2;
+pub struct Ch3;
+pub struct Ch4;
+
+pub struct PwmPin<'d, Perip, Channel> {
+    _pin: PeripheralRef<'d, AnyPin>,
+    phantom: PhantomData<(Perip, Channel)>,
 }
 
-macro_rules! config_pins {
-    ($($pin:ident),*) => {
-        into_ref!($($pin),*);
-        // NOTE(unsafe) Exclusive access to the registers
-        critical_section::with(|_| unsafe {
-            $(
-                $pin.set_low();
-                $pin.set_as_af($pin.af_num(), AFType::OutputPushPull);
-                #[cfg(gpio_v2)]
-                $pin.set_speed(crate::gpio::Speed::VeryHigh);
-            )*
-        })
+macro_rules! channel_impl {
+    ($new_chx:ident, $channel:ident, $pin_trait:ident) => {
+        impl<'d, Perip: CaptureCompare16bitInstance> PwmPin<'d, Perip, $channel> {
+            pub fn $new_chx(pin: impl Peripheral<P = impl $pin_trait<Perip>> + 'd) -> Self {
+                into_ref!(pin);
+                critical_section::with(|_| unsafe {
+                    pin.set_low();
+                    pin.set_as_af(pin.af_num(), AFType::OutputPushPull);
+                    #[cfg(gpio_v2)]
+                    pin.set_speed(crate::gpio::Speed::VeryHigh);
+                });
+                PwmPin {
+                    _pin: pin.map_into(),
+                    phantom: PhantomData,
+                }
+            }
+        }
     };
 }
 
-impl<'d, T: CaptureCompare16bitInstance> SimplePwm<'d, T> {
-    pub fn new_1ch(
-        tim: impl Peripheral<P = T> + 'd,
-        ch1: impl Peripheral<P = impl Channel1Pin<T>> + 'd,
-        freq: Hertz,
-    ) -> Self {
-        Self::new_inner(tim, freq, move || {
-            config_pins!(ch1);
-        })
-    }
-
-    pub fn new_2ch(
-        tim: impl Peripheral<P = T> + 'd,
-        ch1: impl Peripheral<P = impl Channel1Pin<T>> + 'd,
-        ch2: impl Peripheral<P = impl Channel2Pin<T>> + 'd,
-        freq: Hertz,
-    ) -> Self {
-        Self::new_inner(tim, freq, move || {
-            config_pins!(ch1, ch2);
-        })
-    }
+channel_impl!(new_ch1, Ch1, Channel1Pin);
+channel_impl!(new_ch2, Ch2, Channel2Pin);
+channel_impl!(new_ch3, Ch3, Channel3Pin);
+channel_impl!(new_ch4, Ch4, Channel4Pin);
 
-    pub fn new_3ch(
-        tim: impl Peripheral<P = T> + 'd,
-        ch1: impl Peripheral<P = impl Channel1Pin<T>> + 'd,
-        ch2: impl Peripheral<P = impl Channel2Pin<T>> + 'd,
-        ch3: impl Peripheral<P = impl Channel3Pin<T>> + 'd,
-        freq: Hertz,
-    ) -> Self {
-        Self::new_inner(tim, freq, move || {
-            config_pins!(ch1, ch2, ch3);
-        })
-    }
+pub struct SimplePwm<'d, T> {
+    inner: PeripheralRef<'d, T>,
+}
 
-    pub fn new_4ch(
+impl<'d, T: CaptureCompare16bitInstance> SimplePwm<'d, T> {
+    pub fn new(
         tim: impl Peripheral<P = T> + 'd,
-        ch1: impl Peripheral<P = impl Channel1Pin<T>> + 'd,
-        ch2: impl Peripheral<P = impl Channel2Pin<T>> + 'd,
-        ch3: impl Peripheral<P = impl Channel3Pin<T>> + 'd,
-        ch4: impl Peripheral<P = impl Channel4Pin<T>> + 'd,
+        _ch1: Option<PwmPin<'d, T, Ch1>>,
+        _ch2: Option<PwmPin<'d, T, Ch2>>,
+        _ch3: Option<PwmPin<'d, T, Ch3>>,
+        _ch4: Option<PwmPin<'d, T, Ch4>>,
         freq: Hertz,
     ) -> Self {
-        Self::new_inner(tim, freq, move || {
-            config_pins!(ch1, ch2, ch3, ch4);
-        })
+        Self::new_inner(tim, freq)
     }
 
-    fn new_inner(tim: impl Peripheral<P = T> + 'd, freq: Hertz, configure_pins: impl FnOnce()) -> Self {
+    fn new_inner(tim: impl Peripheral<P = T> + 'd, freq: Hertz) -> Self {
         into_ref!(tim);
 
         T::enable();
         <T as crate::rcc::sealed::RccPeripheral>::reset();
 
-        configure_pins();
-
         let mut this = Self { inner: tim };
 
         this.inner.set_frequency(freq);
diff --git a/examples/stm32f4/src/bin/pwm.rs b/examples/stm32f4/src/bin/pwm.rs
index c99f3cc26..b39bbbe28 100644
--- a/examples/stm32f4/src/bin/pwm.rs
+++ b/examples/stm32f4/src/bin/pwm.rs
@@ -5,7 +5,7 @@
 use defmt::*;
 use embassy::executor::Spawner;
 use embassy::time::{Duration, Timer};
-use embassy_stm32::pwm::simple_pwm::SimplePwm;
+use embassy_stm32::pwm::simple_pwm::{PwmPin, SimplePwm};
 use embassy_stm32::pwm::Channel;
 use embassy_stm32::time::khz;
 use embassy_stm32::Peripherals;
@@ -15,7 +15,8 @@ use {defmt_rtt as _, panic_probe as _};
 async fn main(_spawner: Spawner, p: Peripherals) {
     info!("Hello World!");
 
-    let mut pwm = SimplePwm::new_1ch(p.TIM1, p.PE9, khz(10));
+    let ch1 = PwmPin::new_ch1(p.PE9);
+    let mut pwm = SimplePwm::new(p.TIM1, Some(ch1), None, None, None, khz(10));
     let max = pwm.get_max_duty();
     pwm.enable(Channel::Ch1);
 
diff --git a/examples/stm32g4/src/bin/pwm.rs b/examples/stm32g4/src/bin/pwm.rs
index 579e289b0..dc4e164ab 100644
--- a/examples/stm32g4/src/bin/pwm.rs
+++ b/examples/stm32g4/src/bin/pwm.rs
@@ -5,7 +5,7 @@
 use defmt::*;
 use embassy::executor::Spawner;
 use embassy::time::{Duration, Timer};
-use embassy_stm32::pwm::simple_pwm::SimplePwm;
+use embassy_stm32::pwm::simple_pwm::{PwmPin, SimplePwm};
 use embassy_stm32::pwm::Channel;
 use embassy_stm32::time::khz;
 use embassy_stm32::Peripherals;
@@ -15,7 +15,8 @@ use {defmt_rtt as _, panic_probe as _};
 async fn main(_spawner: Spawner, p: Peripherals) {
     info!("Hello World!");
 
-    let mut pwm = SimplePwm::new_1ch(p.TIM2, p.PA5, khz(10));
+    let ch1 = PwmPin::new_ch1(p.PA5);
+    let mut pwm = SimplePwm::new(p.TIM2, Some(ch1), None, None, None, khz(10));
     let max = pwm.get_max_duty();
     pwm.enable(Channel::Ch1);
 
diff --git a/examples/stm32h7/src/bin/pwm.rs b/examples/stm32h7/src/bin/pwm.rs
index f072c5375..730f637e9 100644
--- a/examples/stm32h7/src/bin/pwm.rs
+++ b/examples/stm32h7/src/bin/pwm.rs
@@ -5,7 +5,7 @@
 use defmt::*;
 use embassy::executor::Spawner;
 use embassy::time::{Duration, Timer};
-use embassy_stm32::pwm::simple_pwm::SimplePwm;
+use embassy_stm32::pwm::simple_pwm::{PwmPin, SimplePwm};
 use embassy_stm32::pwm::Channel;
 use embassy_stm32::time::{khz, mhz};
 use embassy_stm32::{Config, Peripherals};
@@ -27,7 +27,8 @@ pub fn config() -> Config {
 async fn main(_spawner: Spawner, p: Peripherals) {
     info!("Hello World!");
 
-    let mut pwm = SimplePwm::new_1ch(p.TIM3, p.PA6, khz(10));
+    let ch1 = PwmPin::new_ch1(p.PA6);
+    let mut pwm = SimplePwm::new(p.TIM3, Some(ch1), None, None, None, khz(10));
     let max = pwm.get_max_duty();
     pwm.enable(Channel::Ch1);