Merge #672

672: Reset peripherals on enable r=chemicstry a=chemicstry

Add reset on initialization to peripherals that did not have it before. This fixes problems when same peripheral is reinitialized at runtime multiple times.

Some exceptions:
- ADC: all ADCs share a single reset
- DCMI: does reset before enable - couldn't find anything about the order in the reference manual. Just keep it if it works?

I also fixed safety issues where global RCC registers where accessed without critical section.

Co-authored-by: chemicstry <[email protected]>

6 files changed, 46 insertions, 23 deletions

diff --git a/embassy-stm32/src/adc/v2.rs b/embassy-stm32/src/adc/v2.rs
index e79680493..d2429b111 100644
--- a/embassy-stm32/src/adc/v2.rs
+++ b/embassy-stm32/src/adc/v2.rs
@@ -7,16 +7,18 @@ use embedded_hal_02::blocking::delay::DelayUs;
 pub const VDDA_CALIB_MV: u32 = 3000;
 
 #[cfg(not(rcc_f4))]
-unsafe fn enable() {
+fn enable() {
     todo!()
 }
 
 #[cfg(rcc_f4)]
-unsafe fn enable() {
-    // TODO do not enable all adc clocks if not needed
-    crate::pac::RCC.apb2enr().modify(|w| w.set_adc1en(true));
-    crate::pac::RCC.apb2enr().modify(|w| w.set_adc2en(true));
-    crate::pac::RCC.apb2enr().modify(|w| w.set_adc3en(true));
+fn enable() {
+    critical_section::with(|_| unsafe {
+        // TODO do not enable all adc clocks if not needed
+        crate::pac::RCC.apb2enr().modify(|w| w.set_adc1en(true));
+        crate::pac::RCC.apb2enr().modify(|w| w.set_adc2en(true));
+        crate::pac::RCC.apb2enr().modify(|w| w.set_adc3en(true));
+    });
 }
 
 pub enum Resolution {
@@ -125,8 +127,8 @@ where
 {
     pub fn new(_peri: impl Unborrow<Target = T> + 'd, delay: &mut impl DelayUs<u32>) -> Self {
         unborrow!(_peri);
+        enable();
         unsafe {
-            enable();
             // disable before config is set
             T::regs().cr2().modify(|reg| {
                 reg.set_adon(crate::pac::adc::vals::Adon::DISABLED);
diff --git a/embassy-stm32/src/adc/v3.rs b/embassy-stm32/src/adc/v3.rs
index 387b62470..3221cf5ef 100644
--- a/embassy-stm32/src/adc/v3.rs
+++ b/embassy-stm32/src/adc/v3.rs
@@ -8,13 +8,15 @@ pub const VDDA_CALIB_MV: u32 = 3000;
 
 /// Sadly we cannot use `RccPeripheral::enable` since devices are quite inconsistent ADC clock
 /// configuration.
-unsafe fn enable() {
-    #[cfg(stm32h7)]
-    crate::pac::RCC.apb2enr().modify(|w| w.set_adcen(true));
-    #[cfg(stm32g0)]
-    crate::pac::RCC.apbenr2().modify(|w| w.set_adcen(true));
-    #[cfg(stm32l4)]
-    crate::pac::RCC.ahb2enr().modify(|w| w.set_adcen(true));
+fn enable() {
+    critical_section::with(|_| unsafe {
+        #[cfg(stm32h7)]
+        crate::pac::RCC.apb2enr().modify(|w| w.set_adcen(true));
+        #[cfg(stm32g0)]
+        crate::pac::RCC.apbenr2().modify(|w| w.set_adcen(true));
+        #[cfg(any(stm32l4, stm32wb))]
+        crate::pac::RCC.ahb2enr().modify(|w| w.set_adcen(true));
+    });
 }
 
 pub enum Resolution {
@@ -206,8 +208,8 @@ pub struct Adc<'d, T: Instance> {
 impl<'d, T: Instance> Adc<'d, T> {
     pub fn new(_peri: impl Unborrow<Target = T> + 'd, delay: &mut impl DelayUs<u32>) -> Self {
         unborrow!(_peri);
+        enable();
         unsafe {
-            enable();
             T::regs().cr().modify(|reg| {
                 #[cfg(not(adc_g0))]
                 reg.set_deeppwd(false);
diff --git a/embassy-stm32/src/dac/v2.rs b/embassy-stm32/src/dac/v2.rs
index 9fb01fa94..35facdd25 100644
--- a/embassy-stm32/src/dac/v2.rs
+++ b/embassy-stm32/src/dac/v2.rs
@@ -91,6 +91,20 @@ pub struct Dac<'d, T: Instance> {
     phantom: PhantomData<&'d mut T>,
 }
 
+macro_rules! enable {
+    ($enable_reg:ident, $enable_field:ident, $reset_reg:ident, $reset_field:ident) => {
+        crate::pac::RCC
+            .$enable_reg()
+            .modify(|w| w.$enable_field(true));
+        crate::pac::RCC
+            .$reset_reg()
+            .modify(|w| w.$reset_field(true));
+        crate::pac::RCC
+            .$reset_reg()
+            .modify(|w| w.$reset_field(false));
+    };
+}
+
 impl<'d, T: Instance> Dac<'d, T> {
     pub fn new_1ch(
         peri: impl Unborrow<Target = T> + 'd,
@@ -113,14 +127,16 @@ impl<'d, T: Instance> Dac<'d, T> {
         unsafe {
             // Sadly we cannot use `RccPeripheral::enable` since devices are quite inconsistent DAC clock
             // configuration.
-            #[cfg(rcc_h7)]
-            crate::pac::RCC.apb1lenr().modify(|w| w.set_dac12en(true));
-            #[cfg(rcc_h7ab)]
-            crate::pac::RCC.apb1lenr().modify(|w| w.set_dac1en(true));
-            #[cfg(stm32g0)]
-            crate::pac::RCC.apbenr1().modify(|w| w.set_dac1en(true));
-            #[cfg(stm32l4)]
-            crate::pac::RCC.apb1enr1().modify(|w| w.set_dac1en(true));
+            critical_section::with(|_| {
+                #[cfg(rcc_h7)]
+                enable!(apb1lenr, set_dac12en, apb1lrstr, set_dac12rst);
+                #[cfg(rcc_h7ab)]
+                enable!(apb1lenr, set_dac1en, apb1lrstr, set_dac1rst);
+                #[cfg(stm32g0)]
+                enable!(apbenr1, set_dac1en, apbrstr1, set_dac1rst);
+                #[cfg(stm32l4)]
+                enable!(apb1enr1, set_dac1en, apb1rstr1, set_dac1rst);
+            });
 
             if channels >= 1 {
                 T::regs().cr().modify(|reg| {
diff --git a/embassy-stm32/src/i2c/v1.rs b/embassy-stm32/src/i2c/v1.rs
index f280187e5..e0b151142 100644
--- a/embassy-stm32/src/i2c/v1.rs
+++ b/embassy-stm32/src/i2c/v1.rs
@@ -32,6 +32,7 @@ impl<'d, T: Instance> I2c<'d, T> {
         unborrow!(scl, sda);
 
         T::enable();
+        T::reset();
 
         unsafe {
             scl.set_as_af(scl.af_num(), AFType::OutputOpenDrain);
diff --git a/embassy-stm32/src/i2c/v2.rs b/embassy-stm32/src/i2c/v2.rs
index 5e9de8fd3..f3714cbcb 100644
--- a/embassy-stm32/src/i2c/v2.rs
+++ b/embassy-stm32/src/i2c/v2.rs
@@ -53,6 +53,7 @@ impl<'d, T: Instance, TXDMA, RXDMA> I2c<'d, T, TXDMA, RXDMA> {
         unborrow!(irq, scl, sda, tx_dma, rx_dma);
 
         T::enable();
+        T::reset();
 
         unsafe {
             scl.set_as_af(scl.af_num(), AFType::OutputOpenDrain);
diff --git a/embassy-stm32/src/usart/mod.rs b/embassy-stm32/src/usart/mod.rs
index 0466065f1..caea8d71b 100644
--- a/embassy-stm32/src/usart/mod.rs
+++ b/embassy-stm32/src/usart/mod.rs
@@ -200,6 +200,7 @@ impl<'d, T: Instance, TxDma, RxDma> Uart<'d, T, TxDma, RxDma> {
         unborrow!(_inner, rx, tx, tx_dma, rx_dma);
 
         T::enable();
+        T::reset();
         let pclk_freq = T::frequency();
 
         // TODO: better calculation, including error checking and OVER8 if possible.