From 0dee1a402021870ea20f28a126464248dc805eb6 Mon Sep 17 00:00:00 2001
From: Andrey Zgarbul <zgarbul.andrey@gmail.com>
Date: Sun, 18 Aug 2024 07:39:29 +0300
Subject: [PATCH 1/5] move PllSetup in pll mod

---
 src/rcc/f4/mod.rs | 169 ----------------------------------------------
 src/rcc/f4/pll.rs | 152 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 150 insertions(+), 171 deletions(-)

diff --git a/src/rcc/f4/mod.rs b/src/rcc/f4/mod.rs
index b77731d5..fabf61aa 100644
--- a/src/rcc/f4/mod.rs
+++ b/src/rcc/f4/mod.rs
@@ -6,13 +6,6 @@ use super::{BusClock, BusTimerClock, RccBus};
 use fugit::HertzU32 as Hertz;
 use fugit::RateExtU32;
 
-#[cfg(not(feature = "gpio-f410"))]
-use pll::I2sPll;
-use pll::MainPll;
-#[cfg(feature = "sai")]
-#[cfg(not(feature = "gpio-f413"))]
-use pll::SaiPll;
-
 mod pll;
 
 mod enable;
@@ -449,148 +442,6 @@ impl CFGR {
     }
 }
 
-impl CFGR {
-    #[cfg(feature = "gpio-f410")]
-    #[inline(always)]
-    fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
-        let i2s_clocks = self.i2s_clocks();
-
-        let main_pll = if let Some(i2s_clk) = i2s_clocks.pll_i2s_clk {
-            // The I2S frequency is generated by the main PLL. The frequency needs to be accurate,
-            // so we need an expensive full PLL configuration search.
-            MainPll::setup_with_i2s(
-                pllsrcclk,
-                self.hse.is_some(),
-                pllsysclk,
-                self.pll48clk,
-                i2s_clk,
-            )
-        } else {
-            MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk)
-        };
-
-        PllSetup {
-            use_pll: main_pll.use_pll,
-            pllsysclk: main_pll.pllsysclk,
-            pll48clk: main_pll.pll48clk,
-            i2s: i2s_clocks.real(main_pll.plli2sclk, self.i2s_ckin),
-        }
-    }
-
-    #[cfg(feature = "gpio-f413")]
-    #[inline(always)]
-    fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
-        let rcc = unsafe { &*RCC::ptr() };
-
-        let i2s_clocks = self.i2s_clocks();
-        let sai_clocks = self.sai_clocks();
-
-        let main_pll = MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk);
-
-        let (i2s_pll, real_sai_clk) = if let Some(i2s_clk) = i2s_clocks.pll_i2s_clk {
-            // Currently, we only support generating SAI/PLL clocks with the I2S PLL. This is only
-            // really usable when the frequencies are identical or the I2S frequency is a multiple of
-            // the SAI frequency. Therefore, we just optimize the PLL for the I2S frequency and then
-            // derive the SAI frequency from the I2S frequency.
-            let i2s_pll = I2sPll::setup(pllsrcclk, Some(i2s_clk));
-
-            if let Some(sai_clk) = sai_clocks.pll_sai_clk {
-                let div = u32::min(
-                    u32::max((i2s_pll.plli2sclk.unwrap() + (sai_clk >> 1)) / sai_clk, 1),
-                    31,
-                );
-                rcc.dckcfgr().modify(|_, w| w.plli2sdivr().set(div as u8));
-                let real_sai_clk = sai_clk / div;
-                (i2s_pll, Some(real_sai_clk))
-            } else {
-                (i2s_pll, None)
-            }
-        } else if let Some(pll_sai_clk) = sai_clocks.pll_sai_clk {
-            // We try all divider values to get the best approximation of the requested frequency.
-            // NOTE: STM32F413/423 have a different divider range than other models!
-            let (i2s_pll, real_sai_clk, div) = (1..31)
-                .map(|div| {
-                    let i2s_pll = I2sPll::setup(pllsrcclk, Some(pll_sai_clk * div));
-                    let real_clk = i2s_pll.plli2sclk.unwrap() / div;
-                    (i2s_pll, real_clk, div)
-                })
-                .min_by_key(|(_, real_clk, _)| (*real_clk as i32 - pll_sai_clk as i32).abs())
-                .unwrap();
-            rcc.dckcfgr().modify(|_, w| w.plli2sdivr().set(div as u8));
-            (i2s_pll, Some(real_sai_clk))
-        } else {
-            (I2sPll::unused(), None)
-        };
-
-        PllSetup {
-            use_pll: main_pll.use_pll,
-            use_i2spll: i2s_pll.use_pll,
-            pllsysclk: main_pll.pllsysclk,
-            pll48clk: main_pll.pll48clk,
-            i2s: i2s_clocks.real(i2s_pll.plli2sclk, self.i2s_ckin),
-            sai: sai_clocks.real(real_sai_clk, self.i2s_ckin),
-        }
-    }
-
-    #[cfg(any(feature = "gpio-f411", feature = "gpio-f412", feature = "gpio-f446"))]
-    #[inline(always)]
-    fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
-        let i2s_clocks = self.i2s_clocks();
-        #[cfg(feature = "sai")]
-        let sai_clocks = self.sai_clocks();
-
-        // All PLLs are completely independent.
-        let main_pll = MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk);
-        let i2s_pll = I2sPll::setup(pllsrcclk, i2s_clocks.pll_i2s_clk);
-        #[cfg(feature = "sai")]
-        let sai_pll = SaiPll::setup(pllsrcclk, sai_clocks.pll_sai_clk);
-
-        PllSetup {
-            use_pll: main_pll.use_pll,
-            use_i2spll: i2s_pll.use_pll,
-            #[cfg(feature = "sai")]
-            use_saipll: sai_pll.use_pll,
-            pllsysclk: main_pll.pllsysclk,
-            pll48clk: main_pll.pll48clk,
-            i2s: i2s_clocks.real(i2s_pll.plli2sclk, self.i2s_ckin),
-            #[cfg(feature = "sai")]
-            sai: sai_clocks.real(sai_pll.sai_clk, self.i2s_ckin),
-        }
-    }
-
-    #[cfg(any(
-        feature = "gpio-f401",
-        feature = "gpio-f417",
-        feature = "gpio-f427",
-        feature = "gpio-f469",
-    ))]
-    #[inline(always)]
-    fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
-        let i2s_clocks = self.i2s_clocks();
-        #[cfg(any(feature = "gpio-f427", feature = "gpio-f469"))]
-        let sai_clocks = self.sai_clocks();
-
-        // We have separate PLLs, but they share the "M" divider.
-        let main_pll = MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk);
-        let i2s_pll = I2sPll::setup_shared_m(pllsrcclk, main_pll.m, i2s_clocks.pll_i2s_clk);
-        #[cfg(feature = "sai")]
-        let sai_pll =
-            SaiPll::setup_shared_m(pllsrcclk, main_pll.m.or(i2s_pll.m), sai_clocks.pll_sai_clk);
-
-        PllSetup {
-            use_pll: main_pll.use_pll,
-            use_i2spll: i2s_pll.use_pll,
-            #[cfg(feature = "sai")]
-            use_saipll: sai_pll.use_pll,
-            pllsysclk: main_pll.pllsysclk,
-            pll48clk: main_pll.pll48clk,
-            i2s: i2s_clocks.real(i2s_pll.plli2sclk, self.i2s_ckin),
-            #[cfg(feature = "sai")]
-            sai: sai_clocks.real(sai_pll.sai_clk, self.i2s_ckin),
-        }
-    }
-}
-
 #[cfg(feature = "sai")]
 impl CFGR {
     fn sai_clocks(&self) -> SaiClocks {
@@ -703,7 +554,6 @@ impl CFGR {
     fn freeze_internal(self, unchecked: bool) -> Clocks {
         let rcc = unsafe { &*RCC::ptr() };
 
-        //let (use_pll, sysclk_on_pll, sysclk, pll48clk) = self.pll_setup();
         let pllsrcclk = self.hse.unwrap_or(HSI);
         let sysclk = self.sysclk.unwrap_or(pllsrcclk);
         let sysclk_on_pll = sysclk != pllsrcclk;
@@ -893,25 +743,6 @@ impl CFGR {
     }
 }
 
-#[cfg_attr(feature = "defmt", derive(defmt::Format))]
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
-struct PllSetup {
-    use_pll: bool,
-    #[cfg(not(feature = "gpio-f410"))]
-    use_i2spll: bool,
-    #[cfg(feature = "sai")]
-    #[cfg(not(feature = "gpio-f413"))]
-    use_saipll: bool,
-
-    pllsysclk: Option<u32>,
-    pll48clk: Option<u32>,
-
-    i2s: RealI2sClocks,
-
-    #[cfg(feature = "sai")]
-    sai: RealSaiClocks,
-}
-
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 struct I2sClocks {
diff --git a/src/rcc/f4/pll.rs b/src/rcc/f4/pll.rs
index 84696021..88325cbb 100644
--- a/src/rcc/f4/pll.rs
+++ b/src/rcc/f4/pll.rs
@@ -1,5 +1,152 @@
 use crate::pac::RCC;
 
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub struct PllSetup {
+    pub use_pll: bool,
+    pub pllsysclk: Option<u32>,
+    pub pll48clk: Option<u32>,
+
+    #[cfg(not(feature = "gpio-f410"))]
+    pub use_i2spll: bool,
+    pub(super) i2s: super::RealI2sClocks,
+
+    #[cfg(feature = "sai")]
+    #[cfg(not(feature = "gpio-f413"))]
+    pub use_saipll: bool,
+    #[cfg(feature = "sai")]
+    pub(super) sai: super::RealSaiClocks,
+}
+
+impl super::CFGR {
+    #[cfg(feature = "gpio-f410")]
+    #[inline(always)]
+    pub fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
+        let i2s_clocks = self.i2s_clocks();
+
+        let main_pll = if let Some(i2s_clk) = i2s_clocks.pll_i2s_clk {
+            // The I2S frequency is generated by the main PLL. The frequency needs to be accurate,
+            // so we need an expensive full PLL configuration search.
+            MainPll::setup_with_i2s(
+                pllsrcclk,
+                self.hse.is_some(),
+                pllsysclk,
+                self.pll48clk,
+                i2s_clk,
+            )
+        } else {
+            MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk)
+        };
+
+        PllSetup {
+            use_pll: main_pll.use_pll,
+            pllsysclk: main_pll.pllsysclk,
+            pll48clk: main_pll.pll48clk,
+
+            i2s: i2s_clocks.real(main_pll.plli2sclk, self.i2s_ckin),
+        }
+    }
+
+    #[cfg(feature = "gpio-f413")]
+    #[inline(always)]
+    pub fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
+        let rcc = unsafe { &*RCC::ptr() };
+
+        let i2s_clocks = self.i2s_clocks();
+        let sai_clocks = self.sai_clocks();
+
+        let main_pll = MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk);
+
+        let (i2s_pll, real_sai_clk) = if let Some(i2s_clk) = i2s_clocks.pll_i2s_clk {
+            // Currently, we only support generating SAI/PLL clocks with the I2S PLL. This is only
+            // really usable when the frequencies are identical or the I2S frequency is a multiple of
+            // the SAI frequency. Therefore, we just optimize the PLL for the I2S frequency and then
+            // derive the SAI frequency from the I2S frequency.
+            let i2s_pll = I2sPll::setup(pllsrcclk, Some(i2s_clk));
+
+            if let Some(sai_clk) = sai_clocks.pll_sai_clk {
+                let div = u32::min(
+                    u32::max((i2s_pll.plli2sclk.unwrap() + (sai_clk >> 1)) / sai_clk, 1),
+                    31,
+                );
+                rcc.dckcfgr().modify(|_, w| w.plli2sdivr().set(div as u8));
+                let real_sai_clk = sai_clk / div;
+                (i2s_pll, Some(real_sai_clk))
+            } else {
+                (i2s_pll, None)
+            }
+        } else if let Some(pll_sai_clk) = sai_clocks.pll_sai_clk {
+            // We try all divider values to get the best approximation of the requested frequency.
+            // NOTE: STM32F413/423 have a different divider range than other models!
+            let (i2s_pll, real_sai_clk, div) = (1..31)
+                .map(|div| {
+                    let i2s_pll = I2sPll::setup(pllsrcclk, Some(pll_sai_clk * div));
+                    let real_clk = i2s_pll.plli2sclk.unwrap() / div;
+                    (i2s_pll, real_clk, div)
+                })
+                .min_by_key(|(_, real_clk, _)| (*real_clk as i32 - pll_sai_clk as i32).abs())
+                .unwrap();
+            rcc.dckcfgr().modify(|_, w| w.plli2sdivr().set(div as u8));
+            (i2s_pll, Some(real_sai_clk))
+        } else {
+            (I2sPll::unused(), None)
+        };
+
+        PllSetup {
+            use_pll: main_pll.use_pll,
+            pllsysclk: main_pll.pllsysclk,
+            pll48clk: main_pll.pll48clk,
+
+            use_i2spll: i2s_pll.use_pll,
+            i2s: i2s_clocks.real(i2s_pll.plli2sclk, self.i2s_ckin),
+
+            sai: sai_clocks.real(real_sai_clk, self.i2s_ckin),
+        }
+    }
+
+    #[cfg(not(any(feature = "gpio-f410", feature = "gpio-f413")))]
+    #[inline(always)]
+    pub fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
+        let i2s_clocks = self.i2s_clocks();
+        #[cfg(feature = "sai")]
+        let sai_clocks = self.sai_clocks();
+
+        // All PLLs are completely independent.
+        let main_pll = MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk);
+
+        #[cfg(any(feature = "gpio-f411", feature = "gpio-f412", feature = "gpio-f446"))]
+        let i2s_pll = I2sPll::setup(pllsrcclk, i2s_clocks.pll_i2s_clk);
+        #[cfg(any(
+            feature = "gpio-f401",
+            feature = "gpio-f417",
+            feature = "gpio-f427",
+            feature = "gpio-f469",
+        ))]
+        // We have separate PLLs, but they share the "M" divider.
+        let i2s_pll = I2sPll::setup_shared_m(pllsrcclk, main_pll.m, i2s_clocks.pll_i2s_clk);
+
+        #[cfg(any(feature = "gpio-f446"))]
+        let sai_pll = SaiPll::setup(pllsrcclk, sai_clocks.pll_sai_clk);
+        #[cfg(any(feature = "gpio-f427", feature = "gpio-f469"))]
+        let sai_pll =
+            SaiPll::setup_shared_m(pllsrcclk, main_pll.m.or(i2s_pll.m), sai_clocks.pll_sai_clk);
+
+        PllSetup {
+            use_pll: main_pll.use_pll,
+            pllsysclk: main_pll.pllsysclk,
+            pll48clk: main_pll.pll48clk,
+
+            use_i2spll: i2s_pll.use_pll,
+            i2s: i2s_clocks.real(i2s_pll.plli2sclk, self.i2s_ckin),
+
+            #[cfg(feature = "sai")]
+            use_saipll: sai_pll.use_pll,
+            #[cfg(feature = "sai")]
+            sai: sai_clocks.real(sai_pll.sai_clk, self.i2s_ckin),
+        }
+    }
+}
+
 pub struct MainPll {
     pub use_pll: bool,
     pub pllsysclk: Option<u32>,
@@ -102,8 +249,10 @@ impl MainPll {
             plli2sclk: None,
         }
     }
+}
 
-    #[cfg(feature = "gpio-f410")]
+#[cfg(feature = "gpio-f410")]
+impl MainPll {
     pub fn setup_with_i2s(
         pllsrcclk: u32,
         use_hse: bool,
@@ -191,7 +340,6 @@ impl MainPll {
         }
     }
 
-    #[cfg(feature = "gpio-f410")]
     fn best_divider(
         vco_out: u32,
         min: u32,

From 6d11419f3cc186964716eca1aed6581757985523 Mon Sep 17 00:00:00 2001
From: Andrey Zgarbul <zgarbul.andrey@gmail.com>
Date: Sun, 18 Aug 2024 08:14:20 +0300
Subject: [PATCH 2/5] const fn best_divider

---
 src/rcc/f4/pll.rs | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/rcc/f4/pll.rs b/src/rcc/f4/pll.rs
index 88325cbb..329fd6e1 100644
--- a/src/rcc/f4/pll.rs
+++ b/src/rcc/f4/pll.rs
@@ -340,7 +340,7 @@ impl MainPll {
         }
     }
 
-    fn best_divider(
+    const fn best_divider(
         vco_out: u32,
         min: u32,
         target: u32,
@@ -349,7 +349,7 @@ impl MainPll {
         max_div: u32,
     ) -> Option<(u32, u32, u32)> {
         let div = (vco_out + target / 2) / target;
-        let min_div = u32::max(
+        let min_div = max_u32(
             min_div,
             if max != 0 {
                 (vco_out + max - 1) / max
@@ -357,17 +357,35 @@ impl MainPll {
                 0
             },
         );
-        let max_div = u32::min(max_div, if min != 0 { vco_out / min } else { u32::MAX });
+        let max_div = min_u32(max_div, if min != 0 { vco_out / min } else { u32::MAX });
         if min_div > max_div {
             return None;
         }
-        let div = u32::min(u32::max(div, min_div), max_div);
+        let div = min_u32(max_u32(div, min_div), max_div);
         let output = vco_out / div;
         let error = (output as i32 - target as i32).abs() as u32;
         Some((div, output, error))
     }
 }
 
+#[cfg(feature = "gpio-f410")]
+const fn max_u32(first: u32, second: u32) -> u32 {
+    if second > first {
+        second
+    } else {
+        first
+    }
+}
+
+#[cfg(feature = "gpio-f410")]
+const fn min_u32(first: u32, second: u32) -> u32 {
+    if second < first {
+        second
+    } else {
+        first
+    }
+}
+
 #[cfg(not(feature = "gpio-f410"))]
 pub struct I2sPll {
     pub use_pll: bool,

From 4f1caaec8939064ab9e45a358e40b7f8ed50d74f Mon Sep 17 00:00:00 2001
From: Andrey Zgarbul <zgarbul.andrey@gmail.com>
Date: Sun, 18 Aug 2024 11:01:24 +0300
Subject: [PATCH 3/5] used/unused

---
 src/fmpi2c.rs     |   7 +-
 src/lib.rs        |  17 +++
 src/rcc/f4/mod.rs |   6 +-
 src/rcc/f4/pll.rs | 303 +++++++++++++++++++++++++---------------------
 4 files changed, 190 insertions(+), 143 deletions(-)

diff --git a/src/fmpi2c.rs b/src/fmpi2c.rs
index efe6ab4b..b5b0ca25 100644
--- a/src/fmpi2c.rs
+++ b/src/fmpi2c.rs
@@ -143,7 +143,6 @@ impl<I2C: Instance> I2c<I2C> {
 impl<I2C: Instance> I2c<I2C> {
     fn i2c_init(&self, mode: impl Into<Mode>) {
         let mode = mode.into();
-        use core::cmp;
 
         // Make sure the I2C unit is disabled so we can configure it
         self.i2c.cr1().modify(|_, w| w.pe().clear_bit());
@@ -167,21 +166,21 @@ impl<I2C: Instance> I2c<I2C> {
         match mode {
             Mode::Standard { frequency } => {
                 presc = 3;
-                scll = cmp::max((((FREQ >> presc) >> 1) / frequency.raw()) - 1, 255) as u8;
+                scll = crate::max_u32((((FREQ >> presc) >> 1) / frequency.raw()) - 1, 255) as u8;
                 sclh = scll - 4;
                 sdadel = 2;
                 scldel = 4;
             }
             Mode::Fast { frequency } => {
                 presc = 1;
-                scll = cmp::max((((FREQ >> presc) >> 1) / frequency.raw()) - 1, 255) as u8;
+                scll = crate::max_u32((((FREQ >> presc) >> 1) / frequency.raw()) - 1, 255) as u8;
                 sclh = scll - 6;
                 sdadel = 2;
                 scldel = 3;
             }
             Mode::FastPlus { frequency } => {
                 presc = 0;
-                scll = cmp::max((((FREQ >> presc) >> 1) / frequency.raw()) - 4, 255) as u8;
+                scll = crate::max_u32((((FREQ >> presc) >> 1) / frequency.raw()) - 4, 255) as u8;
                 sclh = scll - 2;
                 sdadel = 0;
                 scldel = 2;
diff --git a/src/lib.rs b/src/lib.rs
index d9ba82e6..c7503a80 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -227,3 +227,20 @@ pub trait Steal {
     /// no stolen instances are passed to such software.
     unsafe fn steal() -> Self;
 }
+
+#[allow(unused)]
+const fn max_u32(first: u32, second: u32) -> u32 {
+    if second > first {
+        second
+    } else {
+        first
+    }
+}
+
+const fn min_u32(first: u32, second: u32) -> u32 {
+    if second < first {
+        second
+    } else {
+        first
+    }
+}
diff --git a/src/rcc/f4/mod.rs b/src/rcc/f4/mod.rs
index fabf61aa..d7e0c332 100644
--- a/src/rcc/f4/mod.rs
+++ b/src/rcc/f4/mod.rs
@@ -558,7 +558,7 @@ impl CFGR {
         let sysclk = self.sysclk.unwrap_or(pllsrcclk);
         let sysclk_on_pll = sysclk != pllsrcclk;
 
-        let plls = self.pll_setup(pllsrcclk, sysclk_on_pll.then_some(sysclk));
+        let plls = pll::PllSetup::from_cfgr(&self, pllsrcclk, sysclk_on_pll.then_some(sysclk));
         let sysclk = if sysclk_on_pll {
             plls.pllsysclk.unwrap()
         } else {
@@ -586,7 +586,7 @@ impl CFGR {
 
         let pclk1 = self
             .pclk1
-            .unwrap_or_else(|| core::cmp::min(PCLK1_MAX, hclk));
+            .unwrap_or_else(|| crate::min_u32(PCLK1_MAX, hclk));
         let (ppre1_bits, ppre1) = match (hclk + pclk1 - 1) / pclk1 {
             0 => unreachable!(),
             1 => (0b000, 1u8),
@@ -603,7 +603,7 @@ impl CFGR {
 
         let pclk2 = self
             .pclk2
-            .unwrap_or_else(|| core::cmp::min(PCLK2_MAX, hclk));
+            .unwrap_or_else(|| crate::min_u32(PCLK2_MAX, hclk));
         let (ppre2_bits, ppre2) = match (hclk + pclk2 - 1) / pclk2 {
             0 => unreachable!(),
             1 => (0b000, 1u8),
diff --git a/src/rcc/f4/pll.rs b/src/rcc/f4/pll.rs
index 329fd6e1..4eb2ec13 100644
--- a/src/rcc/f4/pll.rs
+++ b/src/rcc/f4/pll.rs
@@ -1,3 +1,4 @@
+use super::CFGR;
 use crate::pac::RCC;
 
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
@@ -18,44 +19,47 @@ pub struct PllSetup {
     pub(super) sai: super::RealSaiClocks,
 }
 
-impl super::CFGR {
+impl PllSetup {
     #[cfg(feature = "gpio-f410")]
     #[inline(always)]
-    pub fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
-        let i2s_clocks = self.i2s_clocks();
+    pub fn from_cfgr(cfgr: &CFGR, pllsrcclk: u32, pllsysclk: Option<u32>) -> Self {
+        let i2s_clocks = cfgr.i2s_clocks();
 
-        let main_pll = if let Some(i2s_clk) = i2s_clocks.pll_i2s_clk {
+        let (main_pll, plli2sclk) = if let Some(i2s_clk) = i2s_clocks.pll_i2s_clk {
             // The I2S frequency is generated by the main PLL. The frequency needs to be accurate,
             // so we need an expensive full PLL configuration search.
             MainPll::setup_with_i2s(
                 pllsrcclk,
-                self.hse.is_some(),
+                cfgr.hse.is_some(),
                 pllsysclk,
-                self.pll48clk,
+                cfgr.pll48clk,
                 i2s_clk,
             )
         } else {
-            MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk)
+            (
+                MainPll::fast_setup(pllsrcclk, cfgr.hse.is_some(), pllsysclk, cfgr.pll48clk),
+                None,
+            )
         };
 
-        PllSetup {
-            use_pll: main_pll.use_pll,
-            pllsysclk: main_pll.pllsysclk,
-            pll48clk: main_pll.pll48clk,
+        Self {
+            use_pll: main_pll.use_pll(),
+            pllsysclk: main_pll.pllsysclk(),
+            pll48clk: main_pll.pll48clk(),
 
-            i2s: i2s_clocks.real(main_pll.plli2sclk, self.i2s_ckin),
+            i2s: i2s_clocks.real(plli2sclk, cfgr.i2s_ckin),
         }
     }
 
     #[cfg(feature = "gpio-f413")]
     #[inline(always)]
-    pub fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
+    pub fn from_cfgr(cfgr: &CFGR, pllsrcclk: u32, pllsysclk: Option<u32>) -> Self {
         let rcc = unsafe { &*RCC::ptr() };
 
-        let i2s_clocks = self.i2s_clocks();
-        let sai_clocks = self.sai_clocks();
+        let i2s_clocks = cfgr.i2s_clocks();
+        let sai_clocks = cfgr.sai_clocks();
 
-        let main_pll = MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk);
+        let main_pll = MainPll::fast_setup(pllsrcclk, cfgr.hse.is_some(), pllsysclk, cfgr.pll48clk);
 
         let (i2s_pll, real_sai_clk) = if let Some(i2s_clk) = i2s_clocks.pll_i2s_clk {
             // Currently, we only support generating SAI/PLL clocks with the I2S PLL. This is only
@@ -65,8 +69,12 @@ impl super::CFGR {
             let i2s_pll = I2sPll::setup(pllsrcclk, Some(i2s_clk));
 
             if let Some(sai_clk) = sai_clocks.pll_sai_clk {
-                let div = u32::min(
-                    u32::max((i2s_pll.plli2sclk.unwrap() + (sai_clk >> 1)) / sai_clk, 1),
+                let div = crate::min_u32(
+                    if let I2sPll::Used { plli2sclk, .. } = i2s_pll {
+                        crate::max_u32((plli2sclk + (sai_clk >> 1)) / sai_clk, 1)
+                    } else {
+                        panic!()
+                    },
                     31,
                 );
                 rcc.dckcfgr().modify(|_, w| w.plli2sdivr().set(div as u8));
@@ -81,7 +89,11 @@ impl super::CFGR {
             let (i2s_pll, real_sai_clk, div) = (1..31)
                 .map(|div| {
                     let i2s_pll = I2sPll::setup(pllsrcclk, Some(pll_sai_clk * div));
-                    let real_clk = i2s_pll.plli2sclk.unwrap() / div;
+                    let real_clk = if let I2sPll::Used { plli2sclk, .. } = i2s_pll {
+                        plli2sclk / div
+                    } else {
+                        panic!()
+                    };
                     (i2s_pll, real_clk, div)
                 })
                 .min_by_key(|(_, real_clk, _)| (*real_clk as i32 - pll_sai_clk as i32).abs())
@@ -89,30 +101,30 @@ impl super::CFGR {
             rcc.dckcfgr().modify(|_, w| w.plli2sdivr().set(div as u8));
             (i2s_pll, Some(real_sai_clk))
         } else {
-            (I2sPll::unused(), None)
+            (I2sPll::Unused, None)
         };
 
-        PllSetup {
-            use_pll: main_pll.use_pll,
-            pllsysclk: main_pll.pllsysclk,
-            pll48clk: main_pll.pll48clk,
+        Self {
+            use_pll: main_pll.use_pll(),
+            pllsysclk: main_pll.pllsysclk(),
+            pll48clk: main_pll.pll48clk(),
 
-            use_i2spll: i2s_pll.use_pll,
-            i2s: i2s_clocks.real(i2s_pll.plli2sclk, self.i2s_ckin),
+            use_i2spll: i2s_pll.use_pll(),
+            i2s: i2s_clocks.real(i2s_pll.plli2sclk(), cfgr.i2s_ckin),
 
-            sai: sai_clocks.real(real_sai_clk, self.i2s_ckin),
+            sai: sai_clocks.real(real_sai_clk, cfgr.i2s_ckin),
         }
     }
 
     #[cfg(not(any(feature = "gpio-f410", feature = "gpio-f413")))]
     #[inline(always)]
-    pub fn pll_setup(&self, pllsrcclk: u32, pllsysclk: Option<u32>) -> PllSetup {
-        let i2s_clocks = self.i2s_clocks();
+    pub fn from_cfgr(cfgr: &CFGR, pllsrcclk: u32, pllsysclk: Option<u32>) -> Self {
+        let i2s_clocks = cfgr.i2s_clocks();
         #[cfg(feature = "sai")]
-        let sai_clocks = self.sai_clocks();
+        let sai_clocks = cfgr.sai_clocks();
 
         // All PLLs are completely independent.
-        let main_pll = MainPll::fast_setup(pllsrcclk, self.hse.is_some(), pllsysclk, self.pll48clk);
+        let main_pll = MainPll::fast_setup(pllsrcclk, cfgr.hse.is_some(), pllsysclk, cfgr.pll48clk);
 
         #[cfg(any(feature = "gpio-f411", feature = "gpio-f412", feature = "gpio-f446"))]
         let i2s_pll = I2sPll::setup(pllsrcclk, i2s_clocks.pll_i2s_clk);
@@ -123,49 +135,71 @@ impl super::CFGR {
             feature = "gpio-f469",
         ))]
         // We have separate PLLs, but they share the "M" divider.
-        let i2s_pll = I2sPll::setup_shared_m(pllsrcclk, main_pll.m, i2s_clocks.pll_i2s_clk);
+        let i2s_pll = I2sPll::setup_shared_m(pllsrcclk, main_pll.m(), i2s_clocks.pll_i2s_clk);
 
         #[cfg(any(feature = "gpio-f446"))]
         let sai_pll = SaiPll::setup(pllsrcclk, sai_clocks.pll_sai_clk);
         #[cfg(any(feature = "gpio-f427", feature = "gpio-f469"))]
-        let sai_pll =
-            SaiPll::setup_shared_m(pllsrcclk, main_pll.m.or(i2s_pll.m), sai_clocks.pll_sai_clk);
+        let sai_pll = SaiPll::setup_shared_m(
+            pllsrcclk,
+            main_pll.m().or(i2s_pll.m()),
+            sai_clocks.pll_sai_clk,
+        );
 
-        PllSetup {
-            use_pll: main_pll.use_pll,
-            pllsysclk: main_pll.pllsysclk,
-            pll48clk: main_pll.pll48clk,
+        Self {
+            use_pll: main_pll.use_pll(),
+            pllsysclk: main_pll.pllsysclk(),
+            pll48clk: main_pll.pll48clk(),
 
-            use_i2spll: i2s_pll.use_pll,
-            i2s: i2s_clocks.real(i2s_pll.plli2sclk, self.i2s_ckin),
+            use_i2spll: i2s_pll.use_pll(),
+            i2s: i2s_clocks.real(i2s_pll.plli2sclk(), cfgr.i2s_ckin),
 
             #[cfg(feature = "sai")]
-            use_saipll: sai_pll.use_pll,
+            use_saipll: sai_pll.use_pll(),
             #[cfg(feature = "sai")]
-            sai: sai_clocks.real(sai_pll.sai_clk, self.i2s_ckin),
+            sai: sai_clocks.real(sai_pll.sai_clk(), cfgr.i2s_ckin),
         }
     }
 }
 
-pub struct MainPll {
-    pub use_pll: bool,
-    pub pllsysclk: Option<u32>,
-    pub pll48clk: Option<u32>,
-    /// "M" divisor, required for the other PLLs on some MCUs.
-    #[allow(unused)]
-    pub m: Option<u32>,
-    /// "R" output, required for I2S on STM32F410.
-    #[allow(unused)]
-    pub plli2sclk: Option<u32>,
+#[derive(Clone, Copy)]
+pub enum MainPll {
+    Used {
+        pllsysclk: Option<u32>,
+        pll48clk: Option<u32>,
+        /// "M" divisor, required for the other PLLs on some MCUs.
+        #[allow(unused)]
+        m: u32,
+    },
+    Unused,
 }
 
 impl MainPll {
-    pub fn fast_setup(
-        pllsrcclk: u32,
-        use_hse: bool,
-        pllsysclk: Option<u32>,
-        pll48clk: bool,
-    ) -> MainPll {
+    fn use_pll(&self) -> bool {
+        matches!(self, Self::Used { .. })
+    }
+    fn pllsysclk(&self) -> Option<u32> {
+        match self {
+            Self::Used { pllsysclk, .. } => *pllsysclk,
+            Self::Unused => None,
+        }
+    }
+    fn pll48clk(&self) -> Option<u32> {
+        match self {
+            Self::Used { pll48clk, .. } => *pll48clk,
+            Self::Unused => None,
+        }
+    }
+    #[allow(unused)]
+    fn m(&self) -> Option<u32> {
+        if let Self::Used { m, .. } = self {
+            Some(*m)
+        } else {
+            None
+        }
+    }
+
+    fn fast_setup(pllsrcclk: u32, use_hse: bool, pllsysclk: Option<u32>, pll48clk: bool) -> Self {
         let sysclk = pllsysclk.unwrap_or(pllsrcclk);
         if pllsysclk.is_none() && !pll48clk {
             // Even if we do not use the main PLL, we still need to set the PLL source as that setting
@@ -174,13 +208,7 @@ impl MainPll {
                 .pllcfgr()
                 .write(|w| w.pllsrc().bit(use_hse));
 
-            return MainPll {
-                use_pll: false,
-                pllsysclk: None,
-                pll48clk: None,
-                m: None,
-                plli2sclk: None,
-            };
+            return Self::Unused;
         }
         // Input divisor from PLL source clock, must result to frequency in
         // the range from 1 to 2 MHz
@@ -188,7 +216,7 @@ impl MainPll {
         let pllm_max = pllsrcclk / 1_000_000;
 
         // Sysclk output divisor must be one of 2, 4, 6 or 8
-        let sysclk_div = core::cmp::min(8, (432_000_000 / sysclk) & !1);
+        let sysclk_div = crate::min_u32(8, (432_000_000 / sysclk) & !1);
 
         let target_freq = if pll48clk {
             48_000_000
@@ -241,25 +269,23 @@ impl MainPll {
 
         let real_pllsysclk = vco_in * plln / sysclk_div;
 
-        MainPll {
-            use_pll: true,
+        Self::Used {
             pllsysclk: Some(real_pllsysclk),
-            pll48clk: if pll48clk { Some(real_pll48clk) } else { None },
-            m: Some(pllm),
-            plli2sclk: None,
+            pll48clk: pll48clk.then_some(real_pll48clk),
+            m: pllm,
         }
     }
 }
 
 #[cfg(feature = "gpio-f410")]
 impl MainPll {
-    pub fn setup_with_i2s(
+    fn setup_with_i2s(
         pllsrcclk: u32,
         use_hse: bool,
         pllsysclk: Option<u32>,
         pll48clk: bool,
         plli2sclk: u32,
-    ) -> MainPll {
+    ) -> (Self, Option<u32>) {
         use super::{SYSCLK_MAX, SYSCLK_MIN};
 
         // Input divisor from PLL source clock, must result to frequency in
@@ -331,13 +357,15 @@ impl MainPll {
         let real_pllsysclk = pllp.map(|pllp| pllsrcclk / pllm * plln / pllp);
         let real_pll48clk = pllq.map(|pllq| pllsrcclk / pllm * plln / pllq);
 
-        MainPll {
-            use_pll: true,
-            pllsysclk: real_pllsysclk,
-            pll48clk: real_pll48clk,
-            m: Some(pllm),
-            plli2sclk: None,
-        }
+        (
+            Self::Used {
+                pllsysclk: real_pllsysclk,
+                pll48clk: real_pll48clk,
+                m: pllm,
+            },
+            // TODO: check this
+            None,
+        )
     }
 
     const fn best_divider(
@@ -349,7 +377,7 @@ impl MainPll {
         max_div: u32,
     ) -> Option<(u32, u32, u32)> {
         let div = (vco_out + target / 2) / target;
-        let min_div = max_u32(
+        let min_div = crate::max_u32(
             min_div,
             if max != 0 {
                 (vco_out + max - 1) / max
@@ -357,58 +385,55 @@ impl MainPll {
                 0
             },
         );
-        let max_div = min_u32(max_div, if min != 0 { vco_out / min } else { u32::MAX });
+        let max_div = crate::min_u32(max_div, if min != 0 { vco_out / min } else { u32::MAX });
         if min_div > max_div {
             return None;
         }
-        let div = min_u32(max_u32(div, min_div), max_div);
+        let div = crate::min_u32(crate::max_u32(div, min_div), max_div);
         let output = vco_out / div;
         let error = (output as i32 - target as i32).abs() as u32;
         Some((div, output, error))
     }
 }
 
-#[cfg(feature = "gpio-f410")]
-const fn max_u32(first: u32, second: u32) -> u32 {
-    if second > first {
-        second
-    } else {
-        first
-    }
+#[cfg(not(feature = "gpio-f410"))]
+#[derive(Clone, Copy)]
+pub enum I2sPll {
+    Used {
+        /// "M" divisor, required for the other PLLs on some MCUs.
+        m: u32,
+        /// PLL I2S clock output.
+        plli2sclk: u32,
+    },
+    Unused,
 }
 
-#[cfg(feature = "gpio-f410")]
-const fn min_u32(first: u32, second: u32) -> u32 {
-    if second < first {
-        second
-    } else {
-        first
+#[cfg(not(feature = "gpio-f410"))]
+impl I2sPll {
+    fn use_pll(&self) -> bool {
+        matches!(self, Self::Used { .. })
     }
-}
 
-#[cfg(not(feature = "gpio-f410"))]
-pub struct I2sPll {
-    pub use_pll: bool,
-    /// "M" divisor, required for the other PLLs on some MCUs.
     #[allow(unused)]
-    pub m: Option<u32>,
-    /// PLL I2S clock output.
-    pub plli2sclk: Option<u32>,
-}
+    fn m(&self) -> Option<u32> {
+        if let Self::Used { m, .. } = self {
+            Some(*m)
+        } else {
+            None
+        }
+    }
 
-#[cfg(not(feature = "gpio-f410"))]
-impl I2sPll {
-    pub fn unused() -> I2sPll {
-        I2sPll {
-            use_pll: false,
-            m: None,
-            plli2sclk: None,
+    fn plli2sclk(&self) -> Option<u32> {
+        if let Self::Used { plli2sclk, .. } = self {
+            Some(*plli2sclk)
+        } else {
+            None
         }
     }
 
-    pub fn setup(pllsrcclk: u32, plli2sclk: Option<u32>) -> I2sPll {
+    fn setup(pllsrcclk: u32, plli2sclk: Option<u32>) -> Self {
         let Some(target) = plli2sclk else {
-            return Self::unused();
+            return Self::Unused;
         };
         // Input divisor from PLL source clock, must result to frequency in
         // the range from 1 to 2 MHz
@@ -428,28 +453,27 @@ impl I2sPll {
         feature = "gpio-f427",
         feature = "gpio-f469",
     ))]
-    pub fn setup_shared_m(pllsrcclk: u32, m: Option<u32>, plli2sclk: Option<u32>) -> I2sPll {
+    fn setup_shared_m(pllsrcclk: u32, m: Option<u32>, plli2sclk: Option<u32>) -> Self {
         // "m" is None if the main PLL is not in use.
         let Some(m) = m else {
             return Self::setup(pllsrcclk, plli2sclk);
         };
         let Some(target) = plli2sclk else {
-            return Self::unused();
+            return Self::Unused;
         };
         let (pll, config, _) = Self::optimize_fixed_m(pllsrcclk, m, target);
         Self::apply_config(config);
         pll
     }
 
-    fn optimize_fixed_m(pllsrcclk: u32, m: u32, plli2sclk: u32) -> (I2sPll, SingleOutputPll, u32) {
+    fn optimize_fixed_m(pllsrcclk: u32, m: u32, plli2sclk: u32) -> (Self, SingleOutputPll, u32) {
         let (config, real_plli2sclk, error) =
             SingleOutputPll::optimize(pllsrcclk, m, plli2sclk, 2, 7)
                 .expect("did not find any valid I2S PLL config");
         (
-            I2sPll {
-                use_pll: true,
-                m: Some(config.m as u32),
-                plli2sclk: Some(real_plli2sclk),
+            Self::Used {
+                m: config.m as u32,
+                plli2sclk: real_plli2sclk,
             },
             config,
             error,
@@ -483,25 +507,33 @@ impl I2sPll {
 
 #[cfg(feature = "sai")]
 #[cfg(not(feature = "gpio-f413"))]
-pub struct SaiPll {
-    pub use_pll: bool,
-    /// SAI clock (PLL output divided by the SAI clock divider).
-    pub sai_clk: Option<u32>,
+#[derive(Clone, Copy)]
+pub enum SaiPll {
+    Used {
+        /// SAI clock (PLL output divided by the SAI clock divider).
+        sai_clk: u32,
+    },
+    Unused,
 }
 
 #[cfg(feature = "sai")]
 #[cfg(not(feature = "gpio-f413"))]
 impl SaiPll {
-    pub fn unused() -> SaiPll {
-        SaiPll {
-            use_pll: false,
-            sai_clk: None,
+    fn use_pll(&self) -> bool {
+        matches!(self, Self::Used { .. })
+    }
+
+    fn sai_clk(&self) -> Option<u32> {
+        if let Self::Used { sai_clk } = self {
+            Some(*sai_clk)
+        } else {
+            None
         }
     }
 
-    pub fn setup(pllsrcclk: u32, sai_clk: Option<u32>) -> SaiPll {
+    fn setup(pllsrcclk: u32, sai_clk: Option<u32>) -> SaiPll {
         let Some(target) = sai_clk else {
-            return Self::unused();
+            return Self::Unused;
         };
         // Input divisor from PLL source clock, must result to frequency in
         // the range from 1 to 2 MHz
@@ -516,13 +548,13 @@ impl SaiPll {
     }
 
     #[cfg(any(feature = "gpio-f427", feature = "gpio-f469"))]
-    pub fn setup_shared_m(pllsrcclk: u32, m: Option<u32>, sai_clk: Option<u32>) -> SaiPll {
+    fn setup_shared_m(pllsrcclk: u32, m: Option<u32>, sai_clk: Option<u32>) -> Self {
         // "m" is None if both other PLLs are not in use.
         let Some(m) = m else {
             return Self::setup(pllsrcclk, sai_clk);
         };
         let Some(target) = sai_clk else {
-            return Self::unused();
+            return Self::Unused;
         };
         let (pll, config, saidiv, _) = Self::optimize_fixed_m(pllsrcclk, m, target);
         Self::apply_config(config, saidiv);
@@ -546,9 +578,8 @@ impl SaiPll {
             .min_by_key(|(_, _, _, error)| *error)
             .expect("no suitable I2S PLL configuration found");
         (
-            SaiPll {
-                use_pll: true,
-                sai_clk: Some(real_sai_clk),
+            Self::Used {
+                sai_clk: real_sai_clk,
             },
             config,
             saidiv,

From 17ee16a0c0858e14a69cf186e9c08d70af7dd98d Mon Sep 17 00:00:00 2001
From: Andrey Zgarbul <zgarbul.andrey@gmail.com>
Date: Sun, 18 Aug 2024 12:37:14 +0300
Subject: [PATCH 4/5] rcc_shared_m & rcc_i2s_apb features

---
 Cargo.toml        | 11 ++++++++
 src/i2s.rs        |  8 +++---
 src/rcc/f4/mod.rs | 66 +++++++++++++++++++++--------------------------
 src/rcc/f4/pll.rs | 44 ++++++++++---------------------
 4 files changed, 57 insertions(+), 72 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 78943481..0061803a 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -167,6 +167,7 @@ gpio-f401 = [
     "tim9",
     "tim10",
     "tim11",
+    "rcc_shared_m"
 ]
 gpio-f410 = [
     "dac",
@@ -230,6 +231,7 @@ gpio-f412 = [
     "tim13",
     "tim14",
     "usart3",
+    "rcc_i2s_apb",
 ]
 gpio-f413 = [
     "gpiod",
@@ -275,6 +277,7 @@ gpio-f413 = [
     "uart8",
     "uart9",
     "uart10",
+    "rcc_i2s_apb",
 ]
 gpio-f417 = [
     "gpiod",
@@ -313,6 +316,7 @@ gpio-f417 = [
     "usart3",
     "uart4",
     "uart5",
+    "rcc_shared_m"
 ]
 gpio-f427 = [
     "gpiod",
@@ -360,6 +364,7 @@ gpio-f427 = [
     "uart5",
     "uart7",
     "uart8",
+    "rcc_shared_m"
 ]
 gpio-f446 = [
     "gpiod",
@@ -401,6 +406,7 @@ gpio-f446 = [
     "usart3",
     "uart4",
     "uart5",
+    "rcc_i2s_apb",
 ]
 gpio-f469 = [
     "gpiod",
@@ -451,6 +457,7 @@ gpio-f469 = [
     "uart5",
     "uart7",
     "uart8",
+    "rcc_shared_m"
 ]
 
 ## Support monotonic timers and other stuff that can be used by [RTICv1 framework](https://crates.io/crates/cortex-m-rtic)
@@ -490,8 +497,12 @@ fsmc_lcd = ["dep:display-interface", "dep:display-interface-04"]
 ## SDIO peripheral support. See [sdio-host](https://crates.io/crates/sdio-host)
 sdio-host = ["dep:sdio-host"]
 
+# Next features are for internal use only!!!
+
 dfsdm = []
 sai = []
+rcc_shared_m = []
+rcc_i2s_apb = []
 
 adc2 = []
 adc3 = []
diff --git a/src/i2s.rs b/src/i2s.rs
index 5cabf0ce..48fde593 100644
--- a/src/i2s.rs
+++ b/src/i2s.rs
@@ -56,11 +56,11 @@ where
 
 impl I2sFreq for rcc::APB1 {
     fn try_i2s_freq(clocks: &Clocks) -> Option<Hertz> {
-        #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+        #[cfg(not(feature = "rcc_i2s_apb"))]
         {
             clocks.i2s_clk()
         }
-        #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+        #[cfg(feature = "rcc_i2s_apb")]
         {
             clocks.i2s_apb1_clk()
         }
@@ -69,11 +69,11 @@ impl I2sFreq for rcc::APB1 {
 
 impl I2sFreq for rcc::APB2 {
     fn try_i2s_freq(clocks: &Clocks) -> Option<Hertz> {
-        #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+        #[cfg(not(feature = "rcc_i2s_apb"))]
         {
             clocks.i2s_clk()
         }
-        #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+        #[cfg(feature = "rcc_i2s_apb")]
         {
             clocks.i2s_apb2_clk()
         }
diff --git a/src/rcc/f4/mod.rs b/src/rcc/f4/mod.rs
index d7e0c332..5b277f6c 100644
--- a/src/rcc/f4/mod.rs
+++ b/src/rcc/f4/mod.rs
@@ -217,15 +217,11 @@ impl RccExt for RCC {
                 pll48clk: false,
                 i2s_ckin: None,
 
-                #[cfg(not(any(
-                    feature = "gpio-f412",
-                    feature = "gpio-f413",
-                    feature = "gpio-f446"
-                )))]
+                #[cfg(not(feature = "rcc_i2s_apb"))]
                 i2s_clk: None,
-                #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+                #[cfg(feature = "rcc_i2s_apb")]
                 i2s_apb1_clk: None,
-                #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+                #[cfg(feature = "rcc_i2s_apb")]
                 i2s_apb2_clk: None,
 
                 #[cfg(feature = "sai")]
@@ -317,11 +313,11 @@ pub struct CFGR {
 
     i2s_ckin: Option<u32>,
 
-    #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+    #[cfg(not(feature = "rcc_i2s_apb"))]
     i2s_clk: Option<u32>,
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     i2s_apb1_clk: Option<u32>,
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     i2s_apb2_clk: Option<u32>,
 
     #[cfg(feature = "sai")]
@@ -387,7 +383,7 @@ impl CFGR {
     }
 }
 
-#[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+#[cfg(not(feature = "rcc_i2s_apb"))]
 impl CFGR {
     /// Selects an I2S clock frequency and enables the I2S clock.
     pub fn i2s_clk(mut self, freq: Hertz) -> Self {
@@ -396,7 +392,7 @@ impl CFGR {
     }
 }
 
-#[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+#[cfg(feature = "rcc_i2s_apb")]
 impl CFGR {
     /// Selects an I2S clock frequency for the first set of I2S instancesand enables the I2S clock.
     pub fn i2s_apb1_clk(mut self, freq: Hertz) -> Self {
@@ -468,7 +464,7 @@ impl CFGR {
 }
 
 impl CFGR {
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     fn i2s_clocks(&self) -> I2sClocks {
         let i2s_apb1_ext = self.i2s_apb1_clk.is_some() && self.i2s_apb1_clk == self.i2s_ckin;
         let i2s_apb2_ext = self.i2s_apb2_clk.is_some() && self.i2s_apb2_clk == self.i2s_ckin;
@@ -492,7 +488,7 @@ impl CFGR {
         }
     }
 
-    #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+    #[cfg(not(feature = "rcc_i2s_apb"))]
     fn i2s_clocks(&self) -> I2sClocks {
         let i2s_ext = self.i2s_clk.is_some() && self.i2s_clk == self.i2s_ckin;
         let pll_i2s_clk = if i2s_ext { None } else { self.i2s_clk };
@@ -716,11 +712,11 @@ impl CFGR {
             sysclk: sysclk.Hz(),
             pll48clk: plls.pll48clk.map(Hertz::from_raw),
 
-            #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+            #[cfg(not(feature = "rcc_i2s_apb"))]
             i2s_clk: plls.i2s.i2s_clk.map(Hertz::from_raw),
-            #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+            #[cfg(feature = "rcc_i2s_apb")]
             i2s_apb1_clk: plls.i2s.apb1.i2s_clk.map(Hertz::from_raw),
-            #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+            #[cfg(feature = "rcc_i2s_apb")]
             i2s_apb2_clk: plls.i2s.apb2.i2s_clk.map(Hertz::from_raw),
 
             #[cfg(feature = "sai")]
@@ -747,13 +743,13 @@ impl CFGR {
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 struct I2sClocks {
     /// True if the clock for the APB1 I2S instances is identical to I2S_CKIN.
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     i2s_apb1_ext: bool,
     /// True if the clock for the APB2 I2S instances is identical to I2S_CKIN.
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     i2s_apb2_ext: bool,
     /// True if the clock for I2S is identical to I2S_CKIN.
-    #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+    #[cfg(not(feature = "rcc_i2s_apb"))]
     i2s_ext: bool,
     /// Target for the I2S PLL output.
     pll_i2s_clk: Option<u32>,
@@ -761,7 +757,7 @@ struct I2sClocks {
 
 impl I2sClocks {
     fn real(&self, pll_i2s_clk: Option<u32>, i2s_ckin: Option<u32>) -> RealI2sClocks {
-        #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+        #[cfg(feature = "rcc_i2s_apb")]
         let clk = RealI2sClocks {
             apb1: RealI2sClock {
                 i2s_ext: self.i2s_apb1_ext,
@@ -780,7 +776,7 @@ impl I2sClocks {
                 },
             },
         };
-        #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+        #[cfg(not(feature = "rcc_i2s_apb"))]
         let clk = RealI2sClocks {
             i2s_ext: self.i2s_ext,
             i2s_clk: if self.i2s_ext { i2s_ckin } else { pll_i2s_clk },
@@ -796,7 +792,7 @@ struct RealI2sClock {
     i2s_clk: Option<u32>,
 }
 
-#[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+#[cfg(feature = "rcc_i2s_apb")]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 struct RealI2sClocks {
@@ -804,19 +800,15 @@ struct RealI2sClocks {
     apb2: RealI2sClock,
 }
 
-#[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+#[cfg(not(feature = "rcc_i2s_apb"))]
 use RealI2sClock as RealI2sClocks;
 
 impl RealI2sClocks {
     fn config_clocksel(&self) {
         let rcc = unsafe { &*RCC::ptr() };
 
-        #[cfg(not(any(
-            feature = "gpio-f410",
-            feature = "gpio-f412",
-            feature = "gpio-f413",
-            feature = "gpio-f446",
-        )))]
+        #[cfg(not(feature = "gpio-f410"))]
+        #[cfg(not(feature = "rcc_i2s_apb"))]
         rcc.cfgr().modify(|_, w| {
             if self.i2s_ext {
                 w.i2ssrc().ckin()
@@ -832,7 +824,7 @@ impl RealI2sClocks {
                 w.i2ssrc().pllclkr()
             }
         });
-        #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+        #[cfg(feature = "rcc_i2s_apb")]
         rcc.dckcfgr().modify(|_, w| {
             if self.apb1.i2s_ext {
                 w.i2s1src().i2s_ckin()
@@ -933,11 +925,11 @@ pub struct Clocks {
     sysclk: Hertz,
     pll48clk: Option<Hertz>,
 
-    #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+    #[cfg(not(feature = "rcc_i2s_apb"))]
     i2s_clk: Option<Hertz>,
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     i2s_apb1_clk: Option<Hertz>,
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     i2s_apb2_clk: Option<Hertz>,
 
     #[cfg(feature = "sai")]
@@ -998,17 +990,17 @@ impl Clocks {
     }
 
     /// Returns the frequency of the I2S clock.
-    #[cfg(not(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446")))]
+    #[cfg(not(feature = "rcc_i2s_apb"))]
     pub fn i2s_clk(&self) -> Option<Hertz> {
         self.i2s_clk
     }
     /// Returns the frequency of the first I2S clock (for the I2S peripherals on APB1).
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     pub fn i2s_apb1_clk(&self) -> Option<Hertz> {
         self.i2s_apb1_clk
     }
     /// Returns the frequency of the second I2S clock (for the I2S peripherals on APB2).
-    #[cfg(any(feature = "gpio-f412", feature = "gpio-f413", feature = "gpio-f446"))]
+    #[cfg(feature = "rcc_i2s_apb")]
     pub fn i2s_apb2_clk(&self) -> Option<Hertz> {
         self.i2s_apb2_clk
     }
diff --git a/src/rcc/f4/pll.rs b/src/rcc/f4/pll.rs
index 4eb2ec13..f4e7b532 100644
--- a/src/rcc/f4/pll.rs
+++ b/src/rcc/f4/pll.rs
@@ -126,20 +126,17 @@ impl PllSetup {
         // All PLLs are completely independent.
         let main_pll = MainPll::fast_setup(pllsrcclk, cfgr.hse.is_some(), pllsysclk, cfgr.pll48clk);
 
-        #[cfg(any(feature = "gpio-f411", feature = "gpio-f412", feature = "gpio-f446"))]
+        #[cfg(not(feature = "rcc_shared_m"))]
         let i2s_pll = I2sPll::setup(pllsrcclk, i2s_clocks.pll_i2s_clk);
-        #[cfg(any(
-            feature = "gpio-f401",
-            feature = "gpio-f417",
-            feature = "gpio-f427",
-            feature = "gpio-f469",
-        ))]
+        #[cfg(feature = "rcc_shared_m")]
         // We have separate PLLs, but they share the "M" divider.
         let i2s_pll = I2sPll::setup_shared_m(pllsrcclk, main_pll.m(), i2s_clocks.pll_i2s_clk);
 
-        #[cfg(any(feature = "gpio-f446"))]
+        #[cfg(feature = "sai")]
+        #[cfg(not(feature = "rcc_shared_m"))]
         let sai_pll = SaiPll::setup(pllsrcclk, sai_clocks.pll_sai_clk);
-        #[cfg(any(feature = "gpio-f427", feature = "gpio-f469"))]
+        #[cfg(feature = "sai")]
+        #[cfg(feature = "rcc_shared_m")]
         let sai_pll = SaiPll::setup_shared_m(
             pllsrcclk,
             main_pll.m().or(i2s_pll.m()),
@@ -391,7 +388,7 @@ impl MainPll {
         }
         let div = crate::min_u32(crate::max_u32(div, min_div), max_div);
         let output = vco_out / div;
-        let error = (output as i32 - target as i32).abs() as u32;
+        let error = (output as i32 - target as i32).unsigned_abs();
         Some((div, output, error))
     }
 }
@@ -447,12 +444,7 @@ impl I2sPll {
         pll
     }
 
-    #[cfg(any(
-        feature = "gpio-f401",
-        feature = "gpio-f417",
-        feature = "gpio-f427",
-        feature = "gpio-f469",
-    ))]
+    #[cfg(feature = "rcc_shared_m")]
     fn setup_shared_m(pllsrcclk: u32, m: Option<u32>, plli2sclk: Option<u32>) -> Self {
         // "m" is None if the main PLL is not in use.
         let Some(m) = m else {
@@ -483,21 +475,11 @@ impl I2sPll {
     fn apply_config(config: SingleOutputPll) {
         let rcc = unsafe { &*RCC::ptr() };
         // "M" may have been written before, but the value is identical.
-        #[cfg(any(
-            feature = "gpio-f401",
-            feature = "gpio-f417",
-            feature = "gpio-f427",
-            feature = "gpio-f469",
-        ))]
+        #[cfg(feature = "rcc_shared_m")]
         rcc.pllcfgr()
             .modify(|_, w| unsafe { w.pllm().bits(config.m) });
         rcc.plli2scfgr().modify(|_, w| unsafe {
-            #[cfg(any(
-                feature = "gpio-f411",
-                feature = "gpio-f412",
-                feature = "gpio-f413",
-                feature = "gpio-f446",
-            ))]
+            #[cfg(not(feature = "rcc_shared_m"))]
             w.plli2sm().bits(config.m);
             w.plli2sn().bits(config.n);
             w.plli2sr().bits(config.outdiv)
@@ -547,7 +529,7 @@ impl SaiPll {
         pll
     }
 
-    #[cfg(any(feature = "gpio-f427", feature = "gpio-f469"))]
+    #[cfg(feature = "rcc_shared_m")]
     fn setup_shared_m(pllsrcclk: u32, m: Option<u32>, sai_clk: Option<u32>) -> Self {
         // "m" is None if both other PLLs are not in use.
         let Some(m) = m else {
@@ -592,11 +574,11 @@ impl SaiPll {
         rcc.dckcfgr()
             .modify(|_, w| w.pllsaidivq().set(saidiv as u8 - 1));
         // "M" may have been written before, but the value is identical.
-        #[cfg(any(feature = "gpio-f427", feature = "gpio-f469"))]
+        #[cfg(feature = "rcc_shared_m")]
         rcc.pllcfgr()
             .modify(|_, w| unsafe { w.pllm().bits(config.m) });
         rcc.pllsaicfgr().modify(|_, w| unsafe {
-            #[cfg(feature = "gpio-f446")]
+            #[cfg(not(feature = "rcc_shared_m"))]
             w.pllsaim().bits(config.m);
             w.pllsain().bits(config.n);
             w.pllsaiq().bits(config.outdiv)

From ea50622199442c30acc7017dc5892078a63f9b41 Mon Sep 17 00:00:00 2001
From: Andrey Zgarbul <zgarbul.andrey@gmail.com>
Date: Sun, 18 Aug 2024 12:50:09 +0300
Subject: [PATCH 5/5] changelog

---
 CHANGELOG.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 42116539..d8245ddc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,7 +30,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
  - Allow different lengths of buffers in hal_1 SpiBus impl [#566]
  - Clean SPI write impls [#774]
  - move `ptr()` to `Ptr` trait [#773]
- - make `I2sFreq` trait similar to `BusClock` [#796]
+ - make `I2sFreq` trait similar to `BusClock`, refactor `rcc::Pll` [#796] [#798]
  - `steal` UART peripheral on `Rx::new` [#768]
 
 [#248]: https://github.com/stm32-rs/stm32f4xx-hal/pull/248
@@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 [#783]: https://github.com/stm32-rs/stm32f4xx-hal/pull/783
 [#785]: https://github.com/stm32-rs/stm32f4xx-hal/pull/785
 [#796]: https://github.com/stm32-rs/stm32f4xx-hal/pull/796
+[#798]: https://github.com/stm32-rs/stm32f4xx-hal/pull/798
 
 ## [v0.21.0] - 2024-05-30