Reset peripherals on driver construction (where missing) (#1893)

* Reset peripherals on driver contruction (where missing) * Don't enable and reset SHA in HMAC ctor * changelog * Don't reset the TIMG0
esp-rs · Aug 13, 2024 · 64a7d49 · 64a7d49
1 parent 23f76b0
commit 64a7d49
Show file tree

Hide file tree

Showing 22 changed files with 110 additions and 3 deletions.
diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added touch pad support for esp32 (#1873)
 - Allow configuration of period updating method for MCPWM timers (#1898)
 - Add self-testing mode for TWAI peripheral. (#1929)
+- Added a `PeripheralClockControl::reset` to the driver constructors where missing (#1893)
 
 ### Changed
 
@@ -21,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - DMA: don't require `Sealed` to implement `ReadBuffer` and `WriteBuffer` (#1921)
 - Allow DMA to/from psram for esp32s3 (#1827)
 - DMA buffers now don't require a static lifetime. Make sure to never `mem::forget` an in-progress DMA transfer (consider using `#[deny(clippy::mem_forget)]`) (#1837)
+- Peripherals (where possible) are now explicitly reset and enabled in their constructors (#1893)
 
 ### Fixed
 

diff --git a/esp-hal/src/aes/mod.rs b/esp-hal/src/aes/mod.rs
@@ -135,6 +135,10 @@ impl<'d> Aes<'d> {
     /// Constructs a new `Aes` instance.
     pub fn new(aes: impl Peripheral<P = AES> + 'd) -> Self {
         crate::into_ref!(aes);
+
+        crate::system::PeripheralClockControl::reset(crate::system::Peripheral::Aes);
+        crate::system::PeripheralClockControl::enable(crate::system::Peripheral::Aes);
+
         let mut ret = Self {
             aes,
             alignment_helper: AlignmentHelper::native_endianess(),

diff --git a/esp-hal/src/analog/adc/riscv.rs b/esp-hal/src/analog/adc/riscv.rs
@@ -408,6 +408,7 @@ where
         adc_instance: impl crate::peripheral::Peripheral<P = ADCI> + 'd,
         config: AdcConfig<ADCI>,
     ) -> Self {
+        PeripheralClockControl::reset(Peripheral::ApbSarAdc);
         PeripheralClockControl::enable(Peripheral::ApbSarAdc);
 
         unsafe { &*APB_SARADC::PTR }.ctrl().modify(|_, w| unsafe {

diff --git a/esp-hal/src/analog/adc/xtensa.rs b/esp-hal/src/analog/adc/xtensa.rs
@@ -6,6 +6,7 @@ use crate::efuse::Efuse;
 use crate::{
     peripheral::PeripheralRef,
     peripherals::{APB_SARADC, SENS},
+    system::{Peripheral, PeripheralClockControl},
 };
 
 mod calibration;
@@ -400,6 +401,9 @@ where
         adc_instance: impl crate::peripheral::Peripheral<P = ADCI> + 'd,
         config: AdcConfig<ADCI>,
     ) -> Self {
+        PeripheralClockControl::reset(Peripheral::ApbSarAdc);
+        PeripheralClockControl::enable(Peripheral::ApbSarAdc);
+
         let sensors = unsafe { &*SENS::ptr() };
 
         // Set attenuation for pins

diff --git a/esp-hal/src/ecc.rs b/esp-hal/src/ecc.rs
@@ -85,6 +85,7 @@ impl<'d> Ecc<'d, crate::Blocking> {
     pub fn new(ecc: impl Peripheral<P = ECC> + 'd) -> Self {
         crate::into_ref!(ecc);
 
+        PeripheralClockControl::reset(PeripheralEnable::Ecc);
         PeripheralClockControl::enable(PeripheralEnable::Ecc);
 
         Self {

diff --git a/esp-hal/src/etm.rs b/esp-hal/src/etm.rs
@@ -149,6 +149,7 @@ macro_rules! create_etm {
                 pub fn new(peripheral: impl Peripheral<P = crate::peripherals::SOC_ETM> + 'd) -> Self {
                     crate::into_ref!(peripheral);
 
+                    PeripheralClockControl::reset(crate::system::Peripheral::Etm);
                     PeripheralClockControl::enable(crate::system::Peripheral::Etm);
 
                     Self {

diff --git a/esp-hal/src/hmac.rs b/esp-hal/src/hmac.rs
@@ -96,7 +96,7 @@ impl<'d> Hmac<'d> {
     pub fn new(hmac: impl Peripheral<P = HMAC> + 'd) -> Self {
         crate::into_ref!(hmac);
 
-        PeripheralClockControl::enable(PeripheralEnable::Sha);
+        PeripheralClockControl::reset(PeripheralEnable::Hmac);
         PeripheralClockControl::enable(PeripheralEnable::Hmac);
 
         Self {

diff --git a/esp-hal/src/i2s.rs b/esp-hal/src/i2s.rs
@@ -330,6 +330,7 @@ where
         // the targets the same and force same configuration for both, TX and RX
 
         channel.tx.init_channel();
+        PeripheralClockControl::reset(I::get_peripheral());
         PeripheralClockControl::enable(I::get_peripheral());
         I::set_clock(calculate_clock(
             sample_rate,

diff --git a/esp-hal/src/lcd_cam/mod.rs b/esp-hal/src/lcd_cam/mod.rs
@@ -28,6 +28,7 @@ impl<'d> LcdCam<'d, crate::Blocking> {
     pub fn new(lcd_cam: impl Peripheral<P = LCD_CAM> + 'd) -> Self {
         crate::into_ref!(lcd_cam);
 
+        PeripheralClockControl::reset(system::Peripheral::LcdCam);
         PeripheralClockControl::enable(system::Peripheral::LcdCam);
 
         Self {

diff --git a/esp-hal/src/ledc/mod.rs b/esp-hal/src/ledc/mod.rs
@@ -114,6 +114,8 @@ impl<'d> Ledc<'d> {
         clock_control_config: &'d Clocks<'d>,
     ) -> Self {
         crate::into_ref!(_instance);
+
+        PeripheralClockControl::reset(PeripheralEnable::Ledc);
         PeripheralClockControl::enable(PeripheralEnable::Ledc);
 
         let ledc = unsafe { &*crate::peripherals::LEDC::ptr() };

diff --git a/esp-hal/src/mcpwm/mod.rs b/esp-hal/src/mcpwm/mod.rs
@@ -132,6 +132,7 @@ impl<'d, PWM: PwmPeripheral> McPwm<'d, PWM> {
     ) -> Self {
         crate::into_ref!(peripheral);
 
+        PWM::reset();
         PWM::enable();
 
         #[cfg(not(esp32c6))]
@@ -312,6 +313,8 @@ pub struct FrequencyError;
 pub trait PwmPeripheral: Deref<Target = RegisterBlock> + crate::private::Sealed {
     /// Enable peripheral
     fn enable();
+    /// Reset peripheral
+    fn reset();
     /// Get a pointer to the peripheral RegisterBlock
     fn block() -> *const RegisterBlock;
     /// Get operator GPIO mux output signal
@@ -324,6 +327,10 @@ impl PwmPeripheral for crate::peripherals::MCPWM0 {
         PeripheralClockControl::enable(PeripheralEnable::Mcpwm0)
     }
 
+    fn reset() {
+        PeripheralClockControl::reset(PeripheralEnable::Mcpwm0)
+    }
+
     fn block() -> *const RegisterBlock {
         Self::PTR
     }
@@ -347,6 +354,10 @@ impl PwmPeripheral for crate::peripherals::MCPWM1 {
         PeripheralClockControl::enable(PeripheralEnable::Mcpwm1)
     }
 
+    fn reset() {
+        PeripheralClockControl::reset(PeripheralEnable::Mcpwm1)
+    }
+
     fn block() -> *const RegisterBlock {
         Self::PTR
     }

diff --git a/esp-hal/src/otg_fs.rs b/esp-hal/src/otg_fs.rs
@@ -66,6 +66,7 @@ impl<'d> Usb<'d> {
         P: UsbDp + Send + Sync,
         M: UsbDm + Send + Sync,
     {
+        PeripheralClockControl::reset(PeripheralEnable::Usb);
         PeripheralClockControl::enable(PeripheralEnable::Usb);
 
         Self {

diff --git a/esp-hal/src/parl_io.rs b/esp-hal/src/parl_io.rs
@@ -1381,6 +1381,7 @@ where
         return Err(Error::UnreachableClockRate);
     }
 
+    PeripheralClockControl::reset(crate::system::Peripheral::ParlIo);
     PeripheralClockControl::enable(crate::system::Peripheral::ParlIo);
 
     let pcr = unsafe { &*crate::peripherals::PCR::PTR };

diff --git a/esp-hal/src/pcnt/mod.rs b/esp-hal/src/pcnt/mod.rs
@@ -61,6 +61,7 @@ impl<'d> Pcnt<'d> {
     /// Return a new PCNT
     pub fn new(_instance: impl Peripheral<P = peripherals::PCNT> + 'd) -> Self {
         crate::into_ref!(_instance);
+
         // Enable the PCNT peripherals clock in the system peripheral
         PeripheralClockControl::reset(crate::system::Peripheral::Pcnt);
         PeripheralClockControl::enable(crate::system::Peripheral::Pcnt);

diff --git a/esp-hal/src/rmt.rs b/esp-hal/src/rmt.rs
@@ -227,6 +227,7 @@ where
             return Err(Error::UnreachableTargetFrequency);
         }
 
+        PeripheralClockControl::reset(crate::system::Peripheral::Rmt);
         PeripheralClockControl::enable(crate::system::Peripheral::Rmt);
 
         #[cfg(not(any(esp32, esp32s2)))]

diff --git a/esp-hal/src/rsa/mod.rs b/esp-hal/src/rsa/mod.rs
@@ -101,6 +101,7 @@ impl<'d, DM: crate::Mode> Rsa<'d, DM> {
     fn new_internal(rsa: impl Peripheral<P = RSA> + 'd) -> Self {
         crate::into_ref!(rsa);
 
+        PeripheralClockControl::reset(PeripheralEnable::Rsa);
         PeripheralClockControl::enable(PeripheralEnable::Rsa);
 
         Self {

diff --git a/esp-hal/src/sha.rs b/esp-hal/src/sha.rs
@@ -136,6 +136,7 @@ impl<'d> Sha<'d, crate::Blocking> {
     pub fn new(sha: impl Peripheral<P = SHA> + 'd, mode: ShaMode) -> Self {
         crate::into_ref!(sha);
 
+        PeripheralClockControl::reset(crate::system::Peripheral::Sha);
         PeripheralClockControl::enable(crate::system::Peripheral::Sha);
 
         // Setup SHA Mode

diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs
@@ -544,6 +544,7 @@ where
         mode: SpiMode,
         clocks: &Clocks<'d>,
     ) -> Spi<'d, T, FullDuplexMode> {
+        spi.reset_peripheral();
         spi.enable_peripheral();
 
         let mut spi = Spi {
@@ -721,6 +722,7 @@ where
         mode: SpiMode,
         clocks: &Clocks<'d>,
     ) -> Spi<'d, T, HalfDuplexMode> {
+        spi.reset_peripheral();
         spi.enable_peripheral();
 
         let mut spi = Spi {
@@ -2295,6 +2297,8 @@ pub trait Instance: private::Sealed {
 
     fn enable_peripheral(&self);
 
+    fn reset_peripheral(&self);
+
     fn spi_num(&self) -> u8;
 
     /// Initialize for full-duplex 1 bit mode
@@ -3255,6 +3259,11 @@ impl Instance for crate::peripherals::SPI2 {
         PeripheralClockControl::enable(crate::system::Peripheral::Spi2);
     }
 
+    #[inline(always)]
+    fn reset_peripheral(&self) {
+        PeripheralClockControl::reset(crate::system::Peripheral::Spi2);
+    }
+
     #[inline(always)]
     fn spi_num(&self) -> u8 {
         2
@@ -3332,6 +3341,11 @@ impl Instance for crate::peripherals::SPI2 {
         PeripheralClockControl::enable(crate::system::Peripheral::Spi2);
     }
 
+    #[inline(always)]
+    fn reset_peripheral(&self) {
+        PeripheralClockControl::reset(crate::system::Peripheral::Spi2);
+    }
+
     #[inline(always)]
     fn spi_num(&self) -> u8 {
         2
@@ -3403,6 +3417,11 @@ impl Instance for crate::peripherals::SPI3 {
         PeripheralClockControl::enable(crate::system::Peripheral::Spi3)
     }
 
+    #[inline(always)]
+    fn reset_peripheral(&self) {
+        PeripheralClockControl::reset(crate::system::Peripheral::Spi3)
+    }
+
     #[inline(always)]
     fn spi_num(&self) -> u8 {
         3
@@ -3447,6 +3466,11 @@ impl Instance for crate::peripherals::SPI2 {
         PeripheralClockControl::enable(crate::system::Peripheral::Spi2)
     }
 
+    #[inline(always)]
+    fn reset_peripheral(&self) {
+        PeripheralClockControl::reset(crate::system::Peripheral::Spi2)
+    }
+
     #[inline(always)]
     fn spi_num(&self) -> u8 {
         2
@@ -3524,6 +3548,11 @@ impl Instance for crate::peripherals::SPI3 {
         PeripheralClockControl::enable(crate::system::Peripheral::Spi3)
     }
 
+    #[inline(always)]
+    fn reset_peripheral(&self) {
+        PeripheralClockControl::reset(crate::system::Peripheral::Spi3)
+    }
+
     #[inline(always)]
     fn spi_num(&self) -> u8 {
         3

diff --git a/esp-hal/src/timer/timg.rs b/esp-hal/src/timer/timg.rs
@@ -106,6 +106,8 @@ pub trait TimerGroupInstance {
     fn id() -> u8;
     fn register_block() -> *const RegisterBlock;
     fn configure_src_clk();
+    fn enable_peripheral();
+    fn reset_peripheral();
     fn configure_wdt_src_clk();
 }
 
@@ -144,6 +146,14 @@ impl TimerGroupInstance for TIMG0 {
         // ESP32 has only APB clock source, do nothing
     }
 
+    fn enable_peripheral() {
+        crate::system::PeripheralClockControl::enable(crate::system::Peripheral::Timg0)
+    }
+
+    fn reset_peripheral() {
+        // for TIMG0 do nothing for now because the reset breaks `current_time`
+    }
+
     #[inline(always)]
     #[cfg(any(esp32c2, esp32c3))]
     fn configure_wdt_src_clk() {
@@ -206,6 +216,16 @@ impl TimerGroupInstance for TIMG1 {
         // ESP32-C2 and ESP32-C3 don't have t1config only t0config, do nothing
     }
 
+    #[inline(always)]
+    fn enable_peripheral() {
+        crate::system::PeripheralClockControl::enable(crate::system::Peripheral::Timg1)
+    }
+
+    #[inline(always)]
+    fn reset_peripheral() {
+        crate::system::PeripheralClockControl::reset(crate::system::Peripheral::Timg1)
+    }
+
     #[inline(always)]
     #[cfg(any(esp32c6, esp32h2))]
     fn configure_wdt_src_clk() {
@@ -230,6 +250,9 @@ where
     pub fn new(_timer_group: impl Peripheral<P = T> + 'd, clocks: &Clocks<'d>) -> Self {
         crate::into_ref!(_timer_group);
 
+        T::reset_peripheral();
+        T::enable_peripheral();
+
         T::configure_src_clk();
 
         // ESP32-H2 is using PLL_48M_CLK source instead of APB_CLK
@@ -269,6 +292,9 @@ where
     pub fn new_async(_timer_group: impl Peripheral<P = T> + 'd, clocks: &Clocks<'d>) -> Self {
         crate::into_ref!(_timer_group);
 
+        T::reset_peripheral();
+        T::enable_peripheral();
+
         T::configure_src_clk();
 
         // ESP32-H2 is using PLL_48M_CLK source instead of APB_CLK

diff --git a/esp-hal/src/trace.rs b/esp-hal/src/trace.rs
@@ -66,6 +66,7 @@ where
     pub fn new(peripheral: impl Peripheral<P = T> + 'd) -> Self {
         crate::into_ref!(peripheral);
 
+        PeripheralClockControl::reset(crate::system::Peripheral::Trace0);
         PeripheralClockControl::enable(crate::system::Peripheral::Trace0);
 
         Self {