Merge pull request #2735 from eZioPan/stm32-timer-api-leak

stm32: fix timer api leaking

2 files changed, 115 insertions, 65 deletions

diff --git a/embassy-stm32/src/time_driver.rs b/embassy-stm32/src/time_driver.rs
index cc8161276..e592fbf7d 100644
--- a/embassy-stm32/src/time_driver.rs
+++ b/embassy-stm32/src/time_driver.rs
@@ -15,9 +15,7 @@ use crate::pac::timer::vals;
 use crate::rcc::SealedRccPeripheral;
 #[cfg(feature = "low-power")]
 use crate::rtc::Rtc;
-#[cfg(any(time_driver_tim1, time_driver_tim8, time_driver_tim20))]
-use crate::timer::AdvancedInstance1Channel;
-use crate::timer::CoreInstance;
+use crate::timer::{CoreInstance, GeneralInstance1Channel};
 use crate::{interrupt, peripherals};
 
 // NOTE regarding ALARM_COUNT:
@@ -69,7 +67,7 @@ type T = peripherals::TIM23;
 type T = peripherals::TIM24;
 
 foreach_interrupt! {
-    (TIM1, timer, $block:ident, UP, $irq:ident) => {
+    (TIM1, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim1)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -85,7 +83,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM2, timer, $block:ident, UP, $irq:ident) => {
+    (TIM2, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim2)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -93,7 +91,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM3, timer, $block:ident, UP, $irq:ident) => {
+    (TIM3, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim3)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -101,7 +99,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM4, timer, $block:ident, UP, $irq:ident) => {
+    (TIM4, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim4)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -109,7 +107,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM5, timer, $block:ident, UP, $irq:ident) => {
+    (TIM5, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim5)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -117,7 +115,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM8, timer, $block:ident, UP, $irq:ident) => {
+    (TIM8, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim8)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -133,7 +131,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM9, timer, $block:ident, UP, $irq:ident) => {
+    (TIM9, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim9)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -141,7 +139,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM12, timer, $block:ident, UP, $irq:ident) => {
+    (TIM12, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim12)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -149,7 +147,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM15, timer, $block:ident, UP, $irq:ident) => {
+    (TIM15, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim15)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -157,7 +155,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM20, timer, $block:ident, UP, $irq:ident) => {
+    (TIM20, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim20)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -173,7 +171,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM21, timer, $block:ident, UP, $irq:ident) => {
+    (TIM21, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim21)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -181,7 +179,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM22, timer, $block:ident, UP, $irq:ident) => {
+    (TIM22, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim22)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -189,7 +187,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM23, timer, $block:ident, UP, $irq:ident) => {
+    (TIM23, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim23)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -197,7 +195,7 @@ foreach_interrupt! {
             DRIVER.on_interrupt()
         }
     };
-    (TIM24, timer, $block:ident, UP, $irq:ident) => {
+    (TIM24, timer, $block:ident, CC, $irq:ident) => {
         #[cfg(time_driver_tim24)]
         #[cfg(feature = "rt")]
         #[interrupt]
@@ -263,6 +261,7 @@ pub(crate) struct RtcDriver {
     rtc: Mutex<CriticalSectionRawMutex, Cell<Option<&'static Rtc>>>,
 }
 
+#[allow(clippy::declare_interior_mutable_const)]
 const ALARM_STATE_NEW: AlarmState = AlarmState::new();
 
 embassy_time_driver::time_driver_impl!(static DRIVER: RtcDriver = RtcDriver {
@@ -307,16 +306,8 @@ impl RtcDriver {
             w.set_ccie(0, true);
         });
 
-        <T as CoreInstance>::Interrupt::unpend();
-        unsafe { <T as CoreInstance>::Interrupt::enable() };
-
-        #[cfg(any(time_driver_tim1, time_driver_tim8, time_driver_tim20))]
-        {
-            <T as AdvancedInstance1Channel>::CaptureCompareInterrupt::unpend();
-            unsafe {
-                <T as AdvancedInstance1Channel>::CaptureCompareInterrupt::enable();
-            }
-        }
+        <T as GeneralInstance1Channel>::CaptureCompareInterrupt::unpend();
+        unsafe { <T as GeneralInstance1Channel>::CaptureCompareInterrupt::enable() };
 
         r.cr1().modify(|w| w.set_cen(true));
     }
diff --git a/embassy-stm32/src/timer/mod.rs b/embassy-stm32/src/timer/mod.rs
index 2ba6b3f11..346127005 100644
--- a/embassy-stm32/src/timer/mod.rs
+++ b/embassy-stm32/src/timer/mod.rs
@@ -47,8 +47,8 @@ pub enum TimerBits {
 
 /// Core timer instance.
 pub trait CoreInstance: RccPeripheral + 'static {
-    /// Interrupt for this timer.
-    type Interrupt: interrupt::typelevel::Interrupt;
+    /// Update Interrupt for this timer.
+    type UpdateInterrupt: interrupt::typelevel::Interrupt;
 
     /// Amount of bits this timer has.
     const BITS: TimerBits;
@@ -64,29 +64,46 @@ pub trait BasicNoCr2Instance: CoreInstance {}
 pub trait BasicInstance: BasicNoCr2Instance {}
 
 /// General-purpose 16-bit timer with 1 channel instance.
-pub trait GeneralInstance1Channel: CoreInstance {}
+pub trait GeneralInstance1Channel: CoreInstance {
+    /// Capture compare interrupt for this timer.
+    type CaptureCompareInterrupt: interrupt::typelevel::Interrupt;
+}
 
 /// General-purpose 16-bit timer with 2 channels instance.
-pub trait GeneralInstance2Channel: GeneralInstance1Channel {}
+pub trait GeneralInstance2Channel: GeneralInstance1Channel {
+    /// Trigger event interrupt for this timer.
+    type TriggerInterrupt: interrupt::typelevel::Interrupt;
+}
 
-/// General-purpose 16-bit timer with 4 channels instance.
-pub trait GeneralInstance4Channel: BasicInstance + GeneralInstance2Channel {
+// This trait add *extra* methods to GeneralInstance4Channel,
+// that GeneralInstance4Channel doesn't use, but the "AdvancedInstance"s need.
+// And it's a private trait, so it's content won't leak to outer namespace.
+//
+// If you want to add a new method to it, please leave a detail comment to explain it.
+trait General4ChBlankSealed {
     // SimplePwm<'d, T> is implemented for T: GeneralInstance4Channel
     // Advanced timers implement this trait, but the output needs to be
     // enabled explicitly.
     // To support general-purpose and advanced timers, this function is added
     // here defaulting to noop and overwritten for advanced timers.
-    /// Enable timer outputs.
+    //
+    // Enable timer outputs.
     fn enable_outputs(&self) {}
 }
 
+/// General-purpose 16-bit timer with 4 channels instance.
+#[allow(private_bounds)]
+pub trait GeneralInstance4Channel: BasicInstance + GeneralInstance2Channel + General4ChBlankSealed {}
+
 /// General-purpose 32-bit timer with 4 channels instance.
 pub trait GeneralInstance32bit4Channel: GeneralInstance4Channel {}
 
 /// Advanced 16-bit timer with 1 channel instance.
 pub trait AdvancedInstance1Channel: BasicNoCr2Instance + GeneralInstance1Channel {
-    /// Capture compare interrupt for this timer.
-    type CaptureCompareInterrupt: interrupt::typelevel::Interrupt;
+    /// Communication interrupt for this timer.
+    type CommunicationInterrupt: interrupt::typelevel::Interrupt;
+    /// Break input interrupt for this timer.
+    type BreakInputInterrupt: interrupt::typelevel::Interrupt;
 }
 /// Advanced 16-bit timer with 2 channels instance.
 
@@ -127,7 +144,7 @@ dma_trait!(Ch4Dma, GeneralInstance4Channel);
 macro_rules! impl_core_timer {
     ($inst:ident, $bits:expr) => {
         impl CoreInstance for crate::peripherals::$inst {
-            type Interrupt = crate::_generated::peripheral_interrupts::$inst::UP;
+            type UpdateInterrupt = crate::_generated::peripheral_interrupts::$inst::UP;
 
             const BITS: TimerBits = $bits;
 
@@ -138,6 +155,49 @@ macro_rules! impl_core_timer {
     };
 }
 
+#[allow(unused)]
+macro_rules! impl_general_1ch {
+    ($inst:ident) => {
+        impl GeneralInstance1Channel for crate::peripherals::$inst {
+            type CaptureCompareInterrupt = crate::_generated::peripheral_interrupts::$inst::CC;
+        }
+    };
+}
+
+#[allow(unused)]
+macro_rules! impl_general_2ch {
+    ($inst:ident) => {
+        impl GeneralInstance2Channel for crate::peripherals::$inst {
+            type TriggerInterrupt = crate::_generated::peripheral_interrupts::$inst::TRG;
+        }
+    };
+}
+
+#[allow(unused)]
+macro_rules! impl_advanced_1ch {
+    ($inst:ident) => {
+        impl AdvancedInstance1Channel for crate::peripherals::$inst {
+            type CommunicationInterrupt = crate::_generated::peripheral_interrupts::$inst::COM;
+            type BreakInputInterrupt = crate::_generated::peripheral_interrupts::$inst::BRK;
+        }
+    };
+}
+
+// This macro only apply to "AdvancedInstance(s)",
+// not "GeneralInstance4Channel" itself.
+#[allow(unused)]
+macro_rules! impl_general_4ch_blank_sealed {
+    ($inst:ident) => {
+        impl General4ChBlankSealed for crate::peripherals::$inst {
+            fn enable_outputs(&self) {
+                unsafe { crate::pac::timer::Tim1chCmp::from_ptr(Self::regs()) }
+                    .bdtr()
+                    .modify(|w| w.set_moe(true));
+            }
+        }
+    };
+}
+
 foreach_interrupt! {
     ($inst:ident, timer, TIM_BASIC, UP, $irq:ident) => {
         impl_core_timer!($inst, TimerBits::Bits16);
@@ -149,47 +209,52 @@ foreach_interrupt! {
         impl_core_timer!($inst, TimerBits::Bits16);
         impl BasicNoCr2Instance for crate::peripherals::$inst {}
         impl BasicInstance for crate::peripherals::$inst {}
-        impl GeneralInstance1Channel for crate::peripherals::$inst {}
-        impl GeneralInstance2Channel for crate::peripherals::$inst {}
+        impl_general_1ch!($inst);
+        impl_general_2ch!($inst);
         impl GeneralInstance4Channel for crate::peripherals::$inst {}
+        impl General4ChBlankSealed for crate::peripherals::$inst {}
     };
 
     ($inst:ident, timer, TIM_2CH, UP, $irq:ident) => {
         impl_core_timer!($inst, TimerBits::Bits16);
         impl BasicNoCr2Instance for crate::peripherals::$inst {}
         impl BasicInstance for crate::peripherals::$inst {}
-        impl GeneralInstance1Channel for crate::peripherals::$inst {}
-        impl GeneralInstance2Channel for crate::peripherals::$inst {}
+        impl_general_1ch!($inst);
+        impl_general_2ch!($inst);
         impl GeneralInstance4Channel for crate::peripherals::$inst {}
+        impl General4ChBlankSealed for crate::peripherals::$inst {}
     };
 
     ($inst:ident, timer, TIM_GP16, UP, $irq:ident) => {
         impl_core_timer!($inst, TimerBits::Bits16);
         impl BasicNoCr2Instance for crate::peripherals::$inst {}
         impl BasicInstance for crate::peripherals::$inst {}
-        impl GeneralInstance1Channel for crate::peripherals::$inst {}
-        impl GeneralInstance2Channel for crate::peripherals::$inst {}
+        impl_general_1ch!($inst);
+        impl_general_2ch!($inst);
         impl GeneralInstance4Channel for crate::peripherals::$inst {}
+        impl General4ChBlankSealed for crate::peripherals::$inst {}
     };
 
     ($inst:ident, timer, TIM_GP32, UP, $irq:ident) => {
         impl_core_timer!($inst, TimerBits::Bits32);
         impl BasicNoCr2Instance for crate::peripherals::$inst {}
         impl BasicInstance for crate::peripherals::$inst {}
-        impl GeneralInstance1Channel for crate::peripherals::$inst {}
-        impl GeneralInstance2Channel for crate::peripherals::$inst {}
+        impl_general_1ch!($inst);
+        impl_general_2ch!($inst);
         impl GeneralInstance4Channel for crate::peripherals::$inst {}
         impl GeneralInstance32bit4Channel for crate::peripherals::$inst {}
+        impl General4ChBlankSealed for crate::peripherals::$inst {}
     };
 
     ($inst:ident, timer, TIM_1CH_CMP, UP, $irq:ident) => {
         impl_core_timer!($inst, TimerBits::Bits16);
         impl BasicNoCr2Instance for crate::peripherals::$inst {}
         impl BasicInstance for crate::peripherals::$inst {}
-        impl GeneralInstance1Channel for crate::peripherals::$inst {}
-        impl GeneralInstance2Channel for crate::peripherals::$inst {}
-        impl GeneralInstance4Channel for crate::peripherals::$inst { fn enable_outputs(&self) { set_moe::<Self>() }}
-        impl AdvancedInstance1Channel for crate::peripherals::$inst { type CaptureCompareInterrupt = crate::_generated::peripheral_interrupts::$inst::CC; }
+        impl_general_1ch!($inst);
+        impl_general_2ch!($inst);
+        impl GeneralInstance4Channel for crate::peripherals::$inst {}
+        impl_general_4ch_blank_sealed!($inst);
+        impl_advanced_1ch!($inst);
         impl AdvancedInstance2Channel for crate::peripherals::$inst {}
         impl AdvancedInstance4Channel for crate::peripherals::$inst {}
     };
@@ -198,10 +263,11 @@ foreach_interrupt! {
         impl_core_timer!($inst, TimerBits::Bits16);
         impl BasicNoCr2Instance for crate::peripherals::$inst {}
         impl BasicInstance for crate::peripherals::$inst {}
-        impl GeneralInstance1Channel for crate::peripherals::$inst {}
-        impl GeneralInstance2Channel for crate::peripherals::$inst {}
-        impl GeneralInstance4Channel for crate::peripherals::$inst { fn enable_outputs(&self) { set_moe::<Self>() }}
-        impl AdvancedInstance1Channel for crate::peripherals::$inst { type CaptureCompareInterrupt = crate::_generated::peripheral_interrupts::$inst::CC; }
+        impl_general_1ch!($inst);
+        impl_general_2ch!($inst);
+        impl GeneralInstance4Channel for crate::peripherals::$inst {}
+        impl_general_4ch_blank_sealed!($inst);
+        impl_advanced_1ch!($inst);
         impl AdvancedInstance2Channel for crate::peripherals::$inst {}
         impl AdvancedInstance4Channel for crate::peripherals::$inst {}
     };
@@ -210,19 +276,12 @@ foreach_interrupt! {
         impl_core_timer!($inst, TimerBits::Bits16);
         impl BasicNoCr2Instance for crate::peripherals::$inst {}
         impl BasicInstance for crate::peripherals::$inst {}
-        impl GeneralInstance1Channel for crate::peripherals::$inst {}
-        impl GeneralInstance2Channel for crate::peripherals::$inst {}
-        impl GeneralInstance4Channel for crate::peripherals::$inst { fn enable_outputs(&self) { set_moe::<Self>() }}
-        impl AdvancedInstance1Channel for crate::peripherals::$inst { type CaptureCompareInterrupt = crate::_generated::peripheral_interrupts::$inst::CC; }
+        impl_general_1ch!($inst);
+        impl_general_2ch!($inst);
+        impl GeneralInstance4Channel for crate::peripherals::$inst {}
+        impl_general_4ch_blank_sealed!($inst);
+        impl_advanced_1ch!($inst);
         impl AdvancedInstance2Channel for crate::peripherals::$inst {}
         impl AdvancedInstance4Channel for crate::peripherals::$inst {}
     };
 }
-
-#[cfg(not(stm32l0))]
-#[allow(unused)]
-fn set_moe<T: GeneralInstance4Channel>() {
-    unsafe { crate::pac::timer::Tim1chCmp::from_ptr(T::regs()) }
-        .bdtr()
-        .modify(|w| w.set_moe(true));
-}