From 090b8b84a09cddeb2922faf58e78f1b844ad4948 Mon Sep 17 00:00:00 2001 From: Noah Friedman Date: Wed, 27 Nov 2024 00:39:49 -0500 Subject: [PATCH] Implement `esp_hal::try_init` API --- esp-hal/CHANGELOG.md | 4 ++++ esp-hal/src/lib.rs | 40 +++++++++++++++++++++++++++++---------- esp-hal/src/peripheral.rs | 23 +++++++++++----------- hil-test/tests/init.rs | 15 +++++++++++++++ 4 files changed, 61 insertions(+), 21 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 360c1427d71..d80d1f0ff60 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -22,6 +22,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `esp_hal::psram::psram_raw_parts` (#2546) - The timer drivers `OneShotTimer` & `PeriodicTimer` have `into_async` and `new_typed` methods (#2586) - `timer::Timer` trait has three new methods, `wait`, `async_interrupt_handler` and `peripheral_interrupt` (#2586) +- Configuration structs in the I2C, SPI, and UART drivers now implement the Builder Lite pattern (#2614) +- Improve safety for verifying `init` is only called once, + add `try_init` function for safely initializing peripheral when they may already have been (#2618) ### Changed @@ -42,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The timer drivers `OneShotTimer` & `PeriodicTimer` now have a `Mode` parameter and type erase the underlying driver by default (#2586) - `timer::Timer` has new trait requirements of `Into`, `'static` and `InterruptConfigurable` (#2586) - `systimer::etm::Event` no longer borrows the alarm indefinitely (#2586) +- A number of public enums and structs in the I2C, SPI, and UART drivers have been marked with `#[non_exhaustive]` (#2614) ### Fixed diff --git a/esp-hal/src/lib.rs b/esp-hal/src/lib.rs index aafe8b21ea8..93da7b93764 100644 --- a/esp-hal/src/lib.rs +++ b/esp-hal/src/lib.rs @@ -494,15 +494,7 @@ pub struct Config { pub psram: psram::PsramConfig, } -/// Initialize the system. -/// -/// This function sets up the CPU clock and watchdog, then, returns the -/// peripherals and clocks. -pub fn init(config: Config) -> Peripherals { - system::disable_peripherals(); - - let mut peripherals = Peripherals::take(); - +fn init_internal<'a>(peripherals: &'a mut Peripherals, config: Config) { // RTC domain must be enabled before we try to disable let mut rtc = crate::rtc_cntl::Rtc::new(&mut peripherals.LPWR); @@ -556,6 +548,34 @@ pub fn init(config: Config) -> Peripherals { #[cfg(any(feature = "quad-psram", feature = "octal-psram"))] crate::psram::init_psram(config.psram); +} - peripherals +/// Initialize the system. +/// +/// This function sets up the CPU clock and watchdog, then, returns the +/// peripherals and clocks. +pub fn init(config: Config) -> Peripherals { + system::disable_peripherals(); + + if let Some(mut peripherals) = Peripherals::try_take() { + init_internal(&mut peripherals, config); + peripherals + } else { + panic!("init called more than once!"); + } +} + +/// Initializes the system only if necessary, then returns the peripherals. +/// +/// If [`init`] has already been executed, simply steals and returns the +/// peripherals. (This check uses [`AtomicBool`](core::sync::atomic::AtomicBool) +/// to be as thread-safe as possible) +/// +/// Otherwise, the initialization process is executed (and then the peripherals +/// are stolen and handed off to you). +pub unsafe fn try_init(config: Config) -> Peripherals { + if let Some(mut peripherals) = Peripherals::try_take() { + init_internal(&mut peripherals, config); + } + Peripherals::steal() } diff --git a/esp-hal/src/peripheral.rs b/esp-hal/src/peripheral.rs index 4a817111a84..6c6f7fe293b 100644 --- a/esp-hal/src/peripheral.rs +++ b/esp-hal/src/peripheral.rs @@ -257,9 +257,14 @@ mod peripheral_macros { ),* $(,)? ] ) => { + use portable_atomic::Ordering; + /// Contains the generated peripherals which implement [`Peripheral`] mod peripherals { + use portable_atomic::AtomicBool; + pub(crate) static INITIALIZED: AtomicBool = AtomicBool::new(false); + pub use super::pac::*; $( $crate::create_peripheral!($name <= $from_pac); @@ -295,17 +300,13 @@ mod peripheral_macros { impl Peripherals { /// Returns all the peripherals *once* #[inline] - pub(crate) fn take() -> Self { - #[no_mangle] - static mut _ESP_HAL_DEVICE_PERIPHERALS: bool = false; - - critical_section::with(|_| unsafe { - if _ESP_HAL_DEVICE_PERIPHERALS { - panic!("init called more than once!") - } - _ESP_HAL_DEVICE_PERIPHERALS = true; - Self::steal() - }) + pub(crate) fn try_take() -> Option { + if let Ok(_) = peripherals::INITIALIZED.compare_exchange(false, true, + Ordering::Acquire, Ordering::Relaxed) { + Some(unsafe { Self::steal() }) + } else { + None + } } /// Unsafely create an instance of this peripheral out of thin air. diff --git a/hil-test/tests/init.rs b/hil-test/tests/init.rs index cec885c7319..533b2787e9d 100644 --- a/hil-test/tests/init.rs +++ b/hil-test/tests/init.rs @@ -107,4 +107,19 @@ mod tests { let delay = Delay::new(); delay.delay(2000.millis()); } + + #[test] + #[timeout(3)] + fn test_init_then_try_init() { + esp_hal::init(Config::default()); + + esp_hal::try_init(Config::default()); + } + + #[test] + #[should_panic] + fn test_try_init_then_init() { + esp_hal::try_init(Config::default()); + esp_hal::init(Config::default()); + } }