Move EXTI management to SysCfg module

Sh3Rm4n · Sh3Rm4n · commit 35949ffe6795 · 2021-07-31T00:01:48.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -91,6 +91,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   - `triggered_events` returns an `EnumSet` of triggered events.
 - Change gpio interrupt API to be more in line with the new serial interrupt
   API. ([#262])
+- Move EXTI interrupt management to SysCfg. ([#262])
+  - Becuase EXTI interrupt confiugration could cancel out, make it more obvious
+  in that SysCfg manages the interrupts, not the pin itself.
+  Change `make_interrupt_source()` to `SysCfg::select_exti_interrupt_source()`.
 
 ## [v0.7.0] - 2021-06-18
 
diff --git a/examples/gpio_interrupts.rs b/examples/gpio_interrupts.rs
@@ -44,7 +44,7 @@ fn main() -> ! {
     let mut user_button = gpioa
         .pa0
         .into_pull_down_input(&mut gpioa.moder, &mut gpioa.pupdr);
-    user_button.make_interrupt_source(&mut syscfg);
+    syscfg.select_exti_interrupt_source(&user_button);
     user_button.trigger_on_edge(&mut exti, Edge::Rising);
     user_button.enable_interrupt(&mut exti);
     let interrupt_num = user_button.nvic(); // hal::pac::Interrupt::EXTI0
diff --git a/src/gpio.rs b/src/gpio.rs
@@ -66,7 +66,6 @@ use crate::{
     hal::digital::v2::OutputPin,
     pac::{Interrupt, EXTI},
     rcc::AHB,
-    syscfg::SysCfg,
     Toggle,
 };
 
@@ -291,8 +290,8 @@ pub enum Edge {
 
 /// Generic pin
 pub struct Pin<Gpio, Index, Mode> {
-    gpio: Gpio,
-    index: Index,
+    pub(crate) gpio: Gpio,
+    pub(crate) index: Index,
     _mode: PhantomData<Mode>,
 }
 
@@ -310,17 +309,6 @@ impl<Gpio, Index, Mode> crate::private::Sealed for Pin<Gpio, Index, Mode> {}
 /// [examples/gpio_erased.rs]: https://github.com/stm32-rs/stm32f3xx-hal/blob/v0.7.0/examples/gpio_erased.rs
 pub type PXx<Mode> = Pin<Gpiox, Ux, Mode>;
 
-/// Modify specific index of array-like register
-macro_rules! modify_at {
-    ($reg:expr, $bitwidth:expr, $index:expr, $value:expr) => {
-        $reg.modify(|r, w| {
-            let mask = !(u32::MAX >> (32 - $bitwidth) << ($bitwidth * $index));
-            let value = $value << ($bitwidth * $index);
-            w.bits(r.bits() & mask | value)
-        });
-    };
-}
-
 impl<Gpio, Mode, const X: u8> Pin<Gpio, U<X>, Mode> {
     /// Erases the pin number from the type
     ///
@@ -593,33 +581,10 @@ where
             #[cfg(not(feature = "svd-f373"))]
             5..=9 => Interrupt::EXTI9_5,
             10..=15 => Interrupt::EXTI15_10,
-            _ => unreachable!(),
+            _ => crate::unreachable!(),
         }
     }
 
-    /// Make corresponding EXTI line sensitive to this pin.
-    ///
-    /// # Note
-    ///
-    /// Only **one** Pin index of all banks can be activated
-    /// for interrupts simultainously.
-    ///
-    /// For example, only on of `PA1`, `PB1`, `PC1`, ... can be activated.
-    pub fn make_interrupt_source(&mut self, syscfg: &mut SysCfg) {
-        const BITWIDTH: u8 = 4;
-        let index = self.index.index() % 4;
-        let extigpionr = self.gpio.port_index() as u32;
-        match self.index.index() {
-            // SAFETY: These are all unguarded writes directly to the register,
-            // without leveraging the safety of stm32f3 generated values.
-            0..=3 => unsafe { modify_at!(syscfg.exticr1, BITWIDTH, index, extigpionr) },
-            4..=7 => unsafe { modify_at!(syscfg.exticr2, BITWIDTH, index, extigpionr) },
-            8..=11 => unsafe { modify_at!(syscfg.exticr3, BITWIDTH, index, extigpionr) },
-            12..=15 => unsafe { modify_at!(syscfg.exticr4, BITWIDTH, index, extigpionr) },
-            _ => unreachable!(),
-        };
-    }
-
     /// Generate interrupt on rising edge, falling edge, or both
     pub fn trigger_on_edge(&mut self, exti: &mut EXTI, edge: Edge) {
         const BITWIDTH: u8 = 1;
@@ -631,15 +596,17 @@ where
         };
         // SAFETY: Unguarded write to the register, but behind a &mut
         unsafe {
-            modify_at!(reg_for_cpu!(exti, rtsr), BITWIDTH, index, rise);
-            modify_at!(reg_for_cpu!(exti, ftsr), BITWIDTH, index, fall);
+            crate::modify_at!(reg_for_cpu!(exti, rtsr), BITWIDTH, index, rise);
+            crate::modify_at!(reg_for_cpu!(exti, ftsr), BITWIDTH, index, fall);
         }
     }
 
     /// Configure external interrupts from this pin
     ///
+    /// # Note
+    ///
     /// Remeber to also configure the interrupt pin on
-    /// the SysCfg site, with [`Pin::make_interrupt_source()`]
+    /// the SysCfg site, with [`crate::syscfg::SysCfg::select_exti_interrupt_source()`]
     pub fn configure_interrupt(&mut self, exti: &mut EXTI, enable: impl Into<Toggle>) {
         const BITWIDTH: u8 = 1;
 
@@ -649,10 +616,15 @@ where
         let index = self.index.index();
         let value = u32::from(enable);
         // SAFETY: Unguarded write to the register, but behind a &mut
-        unsafe { modify_at!(reg_for_cpu!(exti, imr), BITWIDTH, index, value) };
+        unsafe { crate::modify_at!(reg_for_cpu!(exti, imr), BITWIDTH, index, value) };
     }
 
     /// Enable external interrupts from this pin
+    ///
+    /// # Note
+    ///
+    /// Remeber to also configure the interrupt pin on
+    /// the SysCfg site, with [`crate::syscfg::SysCfg::select_exti_interrupt_source()`]
     pub fn enable_interrupt(&mut self, exti: &mut EXTI) {
         self.configure_interrupt(exti, Toggle::On)
     }
@@ -772,7 +744,7 @@ macro_rules! r_trait {
                 #[inline]
                 fn $fn(&mut self, i: u8) {
                     let value = $gpioy::$xr::$enum::$VARIANT as u32;
-                    unsafe { modify_at!((*$GPIOX::ptr()).$xr, $bitwidth, i, value) };
+                    unsafe { crate::modify_at!((*$GPIOX::ptr()).$xr, $bitwidth, i, value) };
                 }
             )+
         }
@@ -913,7 +885,7 @@ macro_rules! gpio {
                     #[inline]
                     fn afx(&mut self, i: u8, x: u8) {
                         const BITWIDTH: u8 = 4;
-                        unsafe { modify_at!((*$GPIOX::ptr()).afrh, BITWIDTH, i - 8, x as u32) };
+                        unsafe { crate::modify_at!((*$GPIOX::ptr()).afrh, BITWIDTH, i - 8, x as u32) };
                     }
                 }
 
@@ -924,7 +896,7 @@ macro_rules! gpio {
                     #[inline]
                     fn afx(&mut self, i: u8, x: u8) {
                         const BITWIDTH: u8 = 4;
-                        unsafe { modify_at!((*$GPIOX::ptr()).afrl, BITWIDTH, i, x as u32) };
+                        unsafe { crate::modify_at!((*$GPIOX::ptr()).afrl, BITWIDTH, i, x as u32) };
                     }
                 }
 
diff --git a/src/lib.rs b/src/lib.rs
@@ -91,6 +91,26 @@ pub use nb::block;
 
 pub use embedded_time as time;
 
+mod private {
+    /// Private sealed trait to seal all GPIO implementations
+    /// which do implement peripheral functionalities.
+    pub trait Sealed {}
+
+    /// Modify specific index of array-like register
+    macro_rules! modify_at {
+        ($reg:expr, $bitwidth:expr, $index:expr, $value:expr) => {
+            $reg.modify(|r, w| {
+                let mask = !(u32::MAX >> (32 - $bitwidth) << ($bitwidth * $index));
+                let value = $value << ($bitwidth * $index);
+                w.bits(r.bits() & mask | value)
+            });
+        };
+    }
+    pub(crate) use modify_at;
+}
+
+pub(crate) use private::modify_at;
+
 /// Peripheral access
 #[cfg(feature = "svd-f301")]
 pub use stm32f3::stm32f301 as pac;
@@ -196,12 +216,6 @@ cfg_if! {
     }
 }
 
-mod private {
-    /// Private sealed trait to seal all GPIO implementations
-    /// which do implement peripheral functionalities.
-    pub trait Sealed {}
-}
-
 /// Toggle something on or off.
 ///
 /// Convenience enum and wrapper around a bool, which more explicit about the intention to enable
diff --git a/src/syscfg.rs b/src/syscfg.rs
@@ -2,6 +2,7 @@
 
 use core::ops::Deref;
 
+use crate::gpio::{marker, Pin};
 use crate::{pac::SYSCFG, rcc::APB2};
 
 /// Extension trait that constrains the `SYSCFG` peripheral
@@ -39,3 +40,38 @@ impl Deref for SysCfg {
         &self.0
     }
 }
+
+impl SysCfg {
+    /// Make corresponding EXTI (external interrupt) line sensitive to the selected pin.
+    ///
+    /// # Note
+    ///
+    /// Only **one** Pin index of all banks can be activated
+    /// for interrupts at the same time.
+    ///
+    /// This means, that only on of `PA1`, `PB1`, `PC1`, ... can be activated.
+    ///
+    /// For example, if first [`crate::gpio::gpioa::PA1`] and than [`crate::gpio::gpiob::PB1`]
+    /// would be configured, the former configuration would be overwritten.
+    ///
+    /// But configuring `PA1` and and `PB2` works!
+    #[doc(alias = "enable_interrupt")]
+    pub fn select_exti_interrupt_source<Gpio, Index, Mode>(&mut self, pin: &Pin<Gpio, Index, Mode>)
+    where
+        Gpio: marker::Gpio,
+        Index: marker::Index,
+    {
+        const BITWIDTH: u8 = 4;
+        let index = pin.index.index() % 4;
+        let extigpionr = pin.gpio.port_index() as u32;
+        match pin.index.index() {
+            // SAFETY: These are all unguarded writes directly to the register,
+            // without leveraging the safety of stm32f3 generated values.
+            0..=3 => unsafe { crate::modify_at!(self.exticr1, BITWIDTH, index, extigpionr) },
+            4..=7 => unsafe { crate::modify_at!(self.exticr2, BITWIDTH, index, extigpionr) },
+            8..=11 => unsafe { crate::modify_at!(self.exticr3, BITWIDTH, index, extigpionr) },
+            12..=15 => unsafe { crate::modify_at!(self.exticr4, BITWIDTH, index, extigpionr) },
+            _ => crate::unreachable!(),
+        };
+    }
+}
