Merge #582

582: Make advanced timer trait not require general purpose timer trait as … r=Dirbaio a=matoushybl

…the timers are too different.

When developing pwm driver, I originally used T: GeneralPurpose16bitTimer as it could support both GP timers and advanced timer, but advanced timer requires further modifications in registers accessible only in it (BDTR - bit AOE).

This PR makes advanced timers depend on Basic16bitTimer instead, which should hopefully improve type safety and allow for better timer drivers that can distinguish between advanced timers and general purpose ones.

Co-authored-by: Matous Hybl <[email protected]>

2 files changed, 68 insertions, 15 deletions

diff --git a/embassy-stm32/src/pwm/mod.rs b/embassy-stm32/src/pwm/mod.rs
index b62a14668..1cb191a95 100644
--- a/embassy-stm32/src/pwm/mod.rs
+++ b/embassy-stm32/src/pwm/mod.rs
@@ -62,9 +62,7 @@ impl From<OutputCompareMode> for stm32_metapac::timer::vals::Ocm {
 pub(crate) mod sealed {
     use super::*;
 
-    pub trait CaptureCompareCapable16bitInstance:
-        crate::timer::sealed::GeneralPurpose16bitInstance
-    {
+    pub trait CaptureCompareCapable16bitInstance: crate::timer::sealed::Basic16bitInstance {
         unsafe fn set_output_compare_mode(&mut self, channel: Channel, mode: OutputCompareMode);
 
         unsafe fn enable_channel(&mut self, channel: Channel, enable: bool);
@@ -88,7 +86,7 @@ pub(crate) mod sealed {
 }
 
 pub trait CaptureCompareCapable16bitInstance:
-    sealed::CaptureCompareCapable16bitInstance + crate::timer::GeneralPurpose16bitInstance + 'static
+    sealed::CaptureCompareCapable16bitInstance + crate::timer::Basic16bitInstance + 'static
 {
 }
 pub trait CaptureCompareCapable32bitInstance:
@@ -136,7 +134,38 @@ macro_rules! impl_compare_capable_16bit {
 
 crate::pac::interrupts! {
     ($inst:ident, timer, TIM_GP16, UP, $irq:ident) => {
-        impl_compare_capable_16bit!($inst);
+        impl crate::pwm::sealed::CaptureCompareCapable16bitInstance for crate::peripherals::$inst {
+            unsafe fn set_output_compare_mode(
+                &mut self,
+                channel: crate::pwm::Channel,
+                mode: OutputCompareMode,
+            ) {
+                use crate::timer::sealed::GeneralPurpose16bitInstance;
+                let r = self.regs_gp16();
+                let raw_channel: usize = channel.raw();
+                r.ccmr_output(raw_channel / 2)
+                    .modify(|w| w.set_ocm(raw_channel % 2, mode.into()));
+            }
+
+            unsafe fn enable_channel(&mut self, channel: Channel, enable: bool) {
+                use crate::timer::sealed::GeneralPurpose16bitInstance;
+                self.regs_gp16()
+                    .ccer()
+                    .modify(|w| w.set_cce(channel.raw(), enable));
+            }
+
+            unsafe fn set_compare_value(&mut self, channel: Channel, value: u16) {
+                use crate::timer::sealed::GeneralPurpose16bitInstance;
+                self.regs_gp16()
+                    .ccr(channel.raw())
+                    .modify(|w| w.set_ccr(value));
+            }
+
+            unsafe fn get_max_compare_value(&self) -> u16 {
+                use crate::timer::sealed::GeneralPurpose16bitInstance;
+                self.regs_gp16().arr().read().arr()
+            }
+        }
 
         impl CaptureCompareCapable16bitInstance for crate::peripherals::$inst {
 
@@ -180,7 +209,39 @@ crate::pac::interrupts! {
     };
 
     ($inst:ident, timer, TIM_ADV, UP, $irq:ident) => {
-        impl_compare_capable_16bit!($inst);
+        impl crate::pwm::sealed::CaptureCompareCapable16bitInstance for crate::peripherals::$inst {
+            unsafe fn set_output_compare_mode(
+                &mut self,
+                channel: crate::pwm::Channel,
+                mode: OutputCompareMode,
+            ) {
+                use crate::timer::sealed::AdvancedControlInstance;
+                let r = self.regs_advanced();
+                let raw_channel: usize = channel.raw();
+                r.ccmr_output(raw_channel / 2)
+                    .modify(|w| w.set_ocm(raw_channel % 2, mode.into()));
+            }
+
+            unsafe fn enable_channel(&mut self, channel: Channel, enable: bool) {
+                use crate::timer::sealed::AdvancedControlInstance;
+                self.regs_advanced()
+                    .ccer()
+                    .modify(|w| w.set_cce(channel.raw(), enable));
+            }
+
+            unsafe fn set_compare_value(&mut self, channel: Channel, value: u16) {
+                use crate::timer::sealed::AdvancedControlInstance;
+                self.regs_advanced()
+                    .ccr(channel.raw())
+                    .modify(|w| w.set_ccr(value));
+            }
+
+            unsafe fn get_max_compare_value(&self) -> u16 {
+                use crate::timer::sealed::AdvancedControlInstance;
+                self.regs_advanced().arr().read().arr()
+            }
+        }
+
         impl CaptureCompareCapable16bitInstance for crate::peripherals::$inst {
 
         }
diff --git a/embassy-stm32/src/timer/mod.rs b/embassy-stm32/src/timer/mod.rs
index ed2cb2d97..e3389fbad 100644
--- a/embassy-stm32/src/timer/mod.rs
+++ b/embassy-stm32/src/timer/mod.rs
@@ -39,7 +39,7 @@ pub(crate) mod sealed {
         fn set_frequency<F: Into<Hertz>>(&mut self, frequency: F);
     }
 
-    pub trait AdvancedControlInstance: GeneralPurpose16bitInstance {
+    pub trait AdvancedControlInstance: Basic16bitInstance {
         fn regs_advanced(&self) -> crate::pac::timer::TimAdv;
     }
 }
@@ -205,14 +205,6 @@ crate::pac::interrupts! {
         impl Basic16bitInstance for crate::peripherals::$inst {
         }
 
-        impl sealed::GeneralPurpose16bitInstance for crate::peripherals::$inst {
-            fn regs_gp16(&self) -> crate::pac::timer::TimGp16 {
-                crate::pac::timer::TimGp16(crate::pac::$inst.0)
-            }
-        }
-
-        impl GeneralPurpose16bitInstance for crate::peripherals::$inst {
-        }
         impl sealed::AdvancedControlInstance for crate::peripherals::$inst {
             fn regs_advanced(&self) -> crate::pac::timer::TimAdv {
                 crate::pac::$inst