Merge pull request #267 from Sh3Rm4n/timer

Rework Timer

Author: Sh3Rm4n

CHANGELOG.md

@@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Added
 
 - Readd MonoTimer. This was accidentally removed before. ([#247])
+  - Different to before, frequency() and now() now do not consume the MonoTimer.
+    `&self` is used instead. ([#267])
 - Basic serial implementation also available for UART4 and UART5 ([#246])
 - Implement serial DMA also for Serial ([#246])
 - Add [`enumset`][] as a optional dependency, which allow more ergonomic functions
@@ -36,7 +38,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   for configuration purposes. ([#253])
 - Add an associated const to `serial::Instance` to return the corresponding
   `pac::Interrupt`-number, which is useful to `unmask()` interrupts.
-  An `nvic()` function to `Serial` was also added for convinience. ([#253])
+  An `interrupt()` function to `Serial` was also added for convinience. ([#253])
 - Add a `Serial::is_busy()` function to check, if the serial device is busy.
   Useful to block on this function, e.g. `while serial.is_busy() {}`. ([#253])
 - Add `BaudTable` a convinience wrapper around `Baud` to configure the `Serial`
@@ -51,6 +53,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   Almost all important types should now be supported. ([#265])
 - Add a `free()` function to the RTC implementation to retrieve the passed-in
   peripheral. ([#266])
+- Implement an API to return the corresponding interrupt number of timer through
+  the newly introduced `InterruptNumber` trait, where the `Interrupt` is held
+  as an associated const. ([#267])
+  - Depending on the timer one `Interrupt` or a bundle of `InterruptTypes` is
+    returned.
+  - On the bases of these interrupts, the interrupt controller (NVIC) can
+    be set to mask or unmask these interrupts.
+- Implement the `embedded-hal::timer::Cancel` trait for timers. ([#267])
+- Add `use_pll` to `CFGR` - the clock configuration - to force to use the PLL
+  source for the systemclock. Also `Clocks::pllclk()` was introduced to be able
+  to check, whether PLL is used.
 
 [`enumset`]: https://crates.io/crates/enumset
 
@@ -117,6 +130,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Rename `CkMode` to `ClockMode` ([#266])
 - Rename `stm32-usbd` feature to `usb`. `stm32-usbd` is still used as a
   dependency. ([#271])
+- Rework the timer implementation: ([#267])
+  - `PclkSrc` trait was removed in favor of generic `Instance` trait, which
+    is a common ground for all available timers.
+  - `Timer::tim1` and so on are renamed to `Timer::new`
+  - The implementation of the timers are corrected to better represent the
+    avaibilities of timers of the hardware.
+  - `Timer::new` now does not take a timeout value. This means, that the
+    timer will now not be started automatically and one has to explicitly call
+    `start()`.
 
 ## [v0.7.0] - 2021-06-18
 
@@ -444,6 +466,7 @@ let clocks = rcc
 
 [#271]: https://github.com/stm32-rs/stm32f3xx-hal/pull/271
 [#270]: https://github.com/stm32-rs/stm32f3xx-hal/pull/270
+[#267]: https://github.com/stm32-rs/stm32f3xx-hal/pull/267
 [#266]: https://github.com/stm32-rs/stm32f3xx-hal/pull/266
 [#265]: https://github.com/stm32-rs/stm32f3xx-hal/pull/265
 [#264]: https://github.com/stm32-rs/stm32f3xx-hal/pull/264

examples/gpio_interrupts.rs

@@ -47,7 +47,7 @@ fn main() -> ! {
     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
+    let interrupt_num = user_button.interrupt(); // hal::pac::Interrupt::EXTI0
 
     // Moving ownership to the global BUTTON so we can clear the interrupt pending bit.
     cortex_m::interrupt::free(|cs| *BUTTON.borrow(cs).borrow_mut() = Some(user_button));

src/gpio.rs

@@ -592,7 +592,7 @@ where
     /// This is also useful for all other [`cortex_m::peripheral::NVIC`] functions.
     // TODO(Sh3rm4n): It would be cool to have this either const or have a const function.
     // But this is currenlty not possible, because index() is runtime defined.
-    pub fn nvic(&self) -> Interrupt {
+    pub fn interrupt(&self) -> Interrupt {
         match self.index.index() {
             0 => Interrupt::EXTI0,
             1 => Interrupt::EXTI1,

src/interrupts.rs

@@ -0,0 +1,16 @@
+//! Common Interrupt interface defintions shared between peipherals.
+
+/// A common interrupt number interface, which returns the associated interrupt
+/// of the peripheral.
+///
+/// Used to unmask / enable the interrupt with [`cortex_m::peripheral::NVIC::unmask()`].
+/// This is useful for all `cortex_m::peripheral::INTERRUPT` functions.
+pub trait InterruptNumber {
+    /// The type used to represent the Interrupt Number.
+    ///
+    /// This type of interrupt should be compatible with [`cortex_m::peripheral::NVIC`].
+    type Interrupt;
+
+    /// The assocaited constant of the interrupt
+    const INTERRUPT: Self::Interrupt;
+}

src/lib.rs

@@ -176,6 +176,7 @@ pub mod dma;
 pub mod flash;
 pub mod gpio;
 pub mod i2c;
+pub mod interrupts;
 pub mod prelude;
 pub mod pwm;
 pub mod rcc;

src/prelude.rs

@@ -8,7 +8,6 @@
 //! use stm32f3xx-hal::prelude::*;
 //! ```
 
-#[cfg(any(feature = "stm32f302", feature = "stm32f303"))]
 pub use crate::dma::DmaExt as _stm32f3xx_hal_dma_DmaExt;
 pub use crate::flash::FlashExt as _stm32f3xx_hal_flash_FlashExt;
 pub use crate::gpio::GpioExt as _stm32f3xx_hal_gpio_GpioExt;

src/rcc.rs

@@ -261,17 +261,32 @@ impl BDCR {
 /// let rcc = dp.RCC.constrain();
 /// use_cfgr(&mut rcc.cfgr)
 /// ```
-#[derive(Default)]
 pub struct CFGR {
     hse: Option<u32>,
     hse_bypass: bool,
+    pll_bypass: bool,
     css: bool,
     hclk: Option<u32>,
     pclk1: Option<u32>,
     pclk2: Option<u32>,
     sysclk: Option<u32>,
 }
 
+impl Default for CFGR {
+    fn default() -> Self {
+        Self {
+            hse: None,
+            hse_bypass: false,
+            pll_bypass: true,
+            css: false,
+            hclk: None,
+            pclk1: None,
+            pclk2: None,
+            sysclk: None,
+        }
+    }
+}
+
 pub(crate) struct PllConfig {
     src: cfgr::PLLSRC_A,
     mul: cfgr::PLLMUL_A,
@@ -350,6 +365,12 @@ impl CFGR {
         self
     }
 
+    /// Set this to disallow bypass the PLLCLK for the systemclock generation.
+    pub fn use_pll(mut self) -> Self {
+        self.pll_bypass = false;
+        self
+    }
+
     /// Enable `HSE` bypass.
     ///
     /// Uses user provided clock signal instead of an external oscillator.
@@ -464,13 +485,7 @@ impl CFGR {
     ///
     /// `HSI` is simpler to calculate, but the possible system clocks are less than `HSE`, because the
     /// division is not configurable.
-    #[cfg(not(any(
-        feature = "stm32f302xd",
-        feature = "stm32f302xe",
-        feature = "stm32f303xd",
-        feature = "stm32f303xe",
-        feature = "stm32f398"
-    )))]
+    #[cfg(not(feature = "gpio-f303e"))]
     fn calc_pll(&self, sysclk: u32) -> (u32, PllConfig) {
         let pllsrcclk = self.hse.unwrap_or(HSI.integer() / 2);
         // Get the optimal value for the pll divisor (PLL_DIV) and multiplier (PLL_MUL)
@@ -540,13 +555,7 @@ impl CFGR {
     ///
     /// To determine the optimal values, the greatest common divisor is calculated and the
     /// limitations of the possible values are taken into considiration.
-    #[cfg(any(
-        feature = "stm32f302xd",
-        feature = "stm32f302xe",
-        feature = "stm32f303xd",
-        feature = "stm32f303xe",
-        feature = "stm32f398",
-    ))]
+    #[cfg(feature = "gpio-f303e")]
     fn calc_pll(&self, sysclk: u32) -> (u32, PllConfig) {
         let pllsrcclk = self.hse.unwrap_or(HSI.integer());
 
@@ -614,9 +623,11 @@ impl CFGR {
             // because the two valid USB clocks, 72 Mhz and 48 Mhz, can't be generated
             // directly from neither the internal rc (8 Mhz)  nor the external
             // Oscillator (max 32 Mhz), without using the PLL.
-            (Some(sysclk), Some(hse)) if sysclk == hse => (hse, cfgr::SW_A::HSE, None),
+            (Some(sysclk), Some(hse)) if sysclk == hse && self.pll_bypass => {
+                (hse, cfgr::SW_A::HSE, None)
+            }
             // No need to use the PLL
-            (Some(sysclk), None) if sysclk == HSI.integer() => {
+            (Some(sysclk), None) if sysclk == HSI.integer() && self.pll_bypass => {
                 (HSI.integer(), cfgr::SW_A::HSI, None)
             }
             (Some(sysclk), _) => {
@@ -770,6 +781,7 @@ impl CFGR {
             ppre2,
             sysclk: Hertz(sysclk),
             usbclk_valid,
+            pll_bypass: self.pll_bypass,
         }
     }
 }
@@ -779,7 +791,6 @@ impl CFGR {
 /// The existence of this value indicates that the clock configuration can no longer be changed.
 /// This struct can be obtained via the [freeze](CFGR::freeze) method of the [CFGR](CFGR) struct.
 #[derive(Debug, Clone, Copy)]
-// #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct Clocks {
     hclk: Hertz,
     pclk1: Hertz,
@@ -788,6 +799,7 @@ pub struct Clocks {
     ppre2: u8,
     sysclk: Hertz,
     usbclk_valid: bool,
+    pll_bypass: bool,
 }
 
 // TODO(Sh3Rm4n) Add defmt support for embedded-time!
@@ -797,18 +809,22 @@ impl defmt::Format for Clocks {
         // Format as hexadecimal.
         defmt::write!(
             f,
-            "Clocks {{ hclk: {} Hz, pclk1: {} Hz, pclk2: {} Hz, ppre1: {:b}, ppre2: {:b}, sysclk: {} Hz, usbclk_valid: {} }}",
+            "Clocks {{ hclk: {} Hz, pclk1: {} Hz, pclk2: {} Hz, ppre1: {:b}, ppre2: {:b}, sysclk: {} Hz, usbclk_valid: {}, pll_bypass: {} }}",
             self.hclk.integer(),
             self.pclk1.integer(),
             self.pclk2.integer(),
             self.ppre1,
             self.ppre2,
             self.sysclk.integer(),
             self.usbclk_valid,
+            self.pll_bypass,
         );
     }
 }
 
+// TODO(Sh3Rm4n): Think of some way to generlize APB1 and APB2 as types to then implement a method,
+// with which the ppre or pclk can be obtained by passing in the type of APB.
+// With that in place, some places of macro magic are not needed anymore.
 impl Clocks {
     /// Returns the frequency of the AHB
     pub fn hclk(&self) -> Hertz {
@@ -840,6 +856,19 @@ impl Clocks {
         self.sysclk
     }
 
+    /// Returns the PLL clock if configured, else it returns `None`.
+    ///
+    /// The PLL clock is a source of the system clock, but it is not necessarily configured to be one.
+    pub fn pllclk(&self) -> Option<Hertz> {
+        if self.pll_bypass {
+            None
+        } else {
+            // The PLLCLK is the same as the sysclk, beccause
+            // the sysclk is using it as a source.
+            Some(self.sysclk())
+        }
+    }
+
     /// Returns whether the USBCLK clock frequency is valid for the USB peripheral
     ///
     /// If the microcontroller does support USB, 48 Mhz or 72 Mhz have to be used

src/rtc.rs

@@ -86,6 +86,7 @@ impl Rtc {
 
     /// Release the RTC peripheral
     pub fn free(self) -> RTC {
+        // TODO(Sh3Rm4n): Disable peripheral before releasing it.
         self.regs
     }
 

src/serial.rs

@@ -722,9 +722,10 @@ where
         self.usart.isr.read().busy().bit_is_set()
     }
 
-    /// Obtain the assocated intterupt number for the serial peripheral.
+    /// Obtain the associated interrupt number for the serial peripheral.
     ///
-    /// Used to unmask / enable the interrupt with [`cortex_m::peripheral::NVIC::unmask()`]
+    ///
+    /// Used to unmask / enable the interrupt with [`cortex_m::peripheral::NVIC::unmask()`].
     /// This is useful for all `cortex_m::peripheral::INTERRUPT` functions.
     ///
     /// # Note
@@ -734,15 +735,15 @@ where
     /// ```
     /// use cortex_m::peripheral::INTERRUPT;
     /// use stm32f3xx_hal::pac::USART1;
-    /// use stm32f3xx_hal::serial::Instance;
+    /// use stm32f3xx_hal::interrupt::InterruptNumber;
     ///
-    /// const interrupt: INTERRUPT = <USART1 as Instance>::INTERRUPT;
+    /// const INTERRUPT: Interrupt = <USART1 as InterruptNumber>::INTERRUPT;
     /// ```
     ///
     /// though this function can not be used in a const context.
     #[doc(alias = "unmask")]
-    pub fn nvic() -> Interrupt {
-        <Usart as Instance>::INTERRUPT
+    pub fn interrupt(&self) -> <Usart as crate::interrupts::InterruptNumber>::Interrupt {
+        <Usart as crate::interrupts::InterruptNumber>::INTERRUPT
     }
 
     /// Enable the interrupt for the specified [`Event`].
@@ -759,7 +760,7 @@ where
 
     /// Enable or disable the interrupt for the specified [`Event`].
     #[inline]
-    pub fn configure_interrupt(&mut self, event: Event, enable: impl Into<Toggle>) -> &mut Self {
+    pub fn configure_interrupt(&mut self, event: Event, enable: impl Into<Toggle>) {
         // Do a round way trip to be convert Into<Toggle> -> bool
         let enable: Toggle = enable.into();
         let enable: bool = enable.into();
@@ -781,7 +782,6 @@ where
             // Event::EndOfBlock => self.usart.cr1.modify(|_, w| w.eobie().bit(enable)),
             // Event::WakeupFromStopMode => self.usart.cr3.modify(|_, w| w.wufie().bit(enable)),
         };
-        self
     }
 
     /// Enable or disable interrupt for the specified [`Event`]s.
@@ -791,15 +791,13 @@ where
     /// **disabled**.
     #[cfg(feature = "enumset")]
     #[cfg_attr(docsrs, doc(cfg(feature = "enumset")))]
-    pub fn configure_interrupts(&mut self, events: EnumSet<Event>) -> &mut Self {
+    pub fn configure_interrupts(&mut self, events: EnumSet<Event>) {
         for event in events.complement().iter() {
             self.configure_interrupt(event, false);
         }
         for event in events.iter() {
             self.configure_interrupt(event, true);
         }
-
-        self
     }
 
     /// Check if an interrupt event happend.
@@ -1293,14 +1291,12 @@ impl ReceiverTimeoutExt for USART2 {}
 impl ReceiverTimeoutExt for USART3 {}
 
 /// UART instance
-pub trait Instance: Deref<Target = RegisterBlock> + crate::private::Sealed {
+pub trait Instance:
+    Deref<Target = RegisterBlock> + crate::interrupts::InterruptNumber + crate::private::Sealed
+{
     /// Peripheral bus instance which is responsible for the peripheral
     type APB;
 
-    /// The associated interrupt number, used to unmask / enable the interrupt
-    /// with [`cortex_m::peripheral::NVIC::unmask()`]
-    const INTERRUPT: Interrupt;
-
     #[doc(hidden)]
     fn enable_clock(apb1: &mut Self::APB);
     #[doc(hidden)]
@@ -1323,9 +1319,13 @@ macro_rules! usart {
     ) => {
         $(
             impl crate::private::Sealed for $USARTX {}
+            impl crate::interrupts::InterruptNumber for $USARTX {
+                type Interrupt = Interrupt;
+                const INTERRUPT: Interrupt = $INTERRUPT;
+            }
+
             impl Instance for $USARTX {
                 type APB = $APB;
-                const INTERRUPT: Interrupt = $INTERRUPT;
                 fn enable_clock(apb: &mut Self::APB) {
                     apb.enr().modify(|_, w| w.$usartXen().enabled());
                     apb.rstr().modify(|_, w| w.$usartXrst().reset());