From a6a83d3bb5d5d92baae11861ee4ba4483eee7f80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 2 Dec 2024 16:35:10 +0100 Subject: [PATCH] Track async GPIOs in memory (#2625) * Track async GPIOs in memory * Add an example * Deduplicate interrupt handling * Changelog * Add gpio_bank_1 symbol * Derive EnumCount * Try to fix issues around manual listen calls and multi-core * Fix test * Update esp-hal/src/gpio/mod.rs Co-authored-by: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com> * Do not prevent pending interrupt from being handled * Remove unnecessary unpin * Add a note about interrupt status flags --------- Co-authored-by: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com> --- esp-hal/CHANGELOG.md | 4 + esp-hal/src/gpio/mod.rs | 403 ++++++++++++++++++++------ esp-hal/src/soc/esp32/gpio.rs | 10 +- esp-metadata/CHANGELOG.md | 1 + esp-metadata/devices/esp32.toml | 1 + esp-metadata/devices/esp32p4.toml | 1 + esp-metadata/devices/esp32s2.toml | 1 + esp-metadata/devices/esp32s3.toml | 1 + hil-test/tests/gpio.rs | 8 + hil-test/tests/gpio_custom_handler.rs | 21 +- 10 files changed, 345 insertions(+), 106 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 5a5b59b1d..0c5fc26f8 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added `I8080::apply_config`, `DPI::apply_config` and `Camera::apply_config` (#2610) - Introduced the `unstable` feature which will be used to restrict stable APIs to a subset of esp-hal. (#2628) - HAL configuration structs now implement the Builder Lite pattern (#2645) +- Added `OutputOpenDrain::unlisten` (#2625) +- Added `{Input, Flex}::wait_for` (#2625) ### Changed @@ -50,10 +52,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Interrupt handling related functions are only provided for Blocking UART. (#2610) - Changed how `Spi`, (split or unsplit) `Uart`, `LpUart`, `I8080`, `Camera`, `DPI` and `I2C` drivers are constructed (#2610) - I8080, camera, DPI: The various standalone configuration options have been merged into `Config` (#2610) +- Dropped GPIO futures stop listening for interrupts (#2625) ### Fixed - Xtensa devices now correctly enable the `esp-hal-procmacros/rtc-slow` feature (#2594) +- User-bound GPIO interrupt handlers should no longer interfere with async pins. (#2625) ### Removed diff --git a/esp-hal/src/gpio/mod.rs b/esp-hal/src/gpio/mod.rs index 65b938234..5baf33bde 100644 --- a/esp-hal/src/gpio/mod.rs +++ b/esp-hal/src/gpio/mod.rs @@ -55,8 +55,9 @@ //! [embedded-hal-async]: https://docs.rs/embedded-hal-async/latest/embedded_hal_async/index.html //! [Inverting TX and RX Pins]: crate::uart#inverting-rx-and-tx-pins -use portable_atomic::{AtomicPtr, Ordering}; +use portable_atomic::{AtomicPtr, AtomicU32, Ordering}; use procmacros::ram; +use strum::EnumCount; #[cfg(any(lp_io, rtc_cntl))] use crate::peripherals::gpio::{handle_rtcio, handle_rtcio_with_resistors}; @@ -454,7 +455,7 @@ pub trait OutputPin: Pin + Into + 'static { gpio.func_out_sel_cfg(self.number() as usize) .modify(|_, w| unsafe { w.out_sel().bits(OutputSignal::GPIO as OutputSignalType) }); - #[cfg(any(esp32c3, esp32s3))] + #[cfg(usb_device)] disable_usb_pads(self.number()); io_mux_reg(self.number()).modify(|_, w| unsafe { @@ -546,16 +547,16 @@ pub trait TouchPin: Pin { } #[doc(hidden)] -#[derive(Clone, Copy)] +#[derive(Clone, Copy, EnumCount)] pub enum GpioRegisterAccess { Bank0, - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Bank1, } impl From for GpioRegisterAccess { fn from(_gpio_num: usize) -> Self { - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] if _gpio_num >= 32 { return GpioRegisterAccess::Bank1; } @@ -565,6 +566,21 @@ impl From for GpioRegisterAccess { } impl GpioRegisterAccess { + fn async_operations(self) -> &'static AtomicU32 { + static FLAGS: [AtomicU32; GpioRegisterAccess::COUNT] = + [const { AtomicU32::new(0) }; GpioRegisterAccess::COUNT]; + + &FLAGS[self as usize] + } + + fn offset(self) -> u8 { + match self { + Self::Bank0 => 0, + #[cfg(gpio_bank_1)] + Self::Bank1 => 32, + } + } + fn write_out_en(self, word: u32, enable: bool) { if enable { self.write_out_en_set(word); @@ -576,7 +592,7 @@ impl GpioRegisterAccess { fn write_out_en_clear(self, word: u32) { match self { Self::Bank0 => Bank0GpioRegisterAccess::write_out_en_clear(word), - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Self::Bank1 => Bank1GpioRegisterAccess::write_out_en_clear(word), } } @@ -584,7 +600,7 @@ impl GpioRegisterAccess { fn write_out_en_set(self, word: u32) { match self { Self::Bank0 => Bank0GpioRegisterAccess::write_out_en_set(word), - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Self::Bank1 => Bank1GpioRegisterAccess::write_out_en_set(word), } } @@ -592,7 +608,7 @@ impl GpioRegisterAccess { fn read_input(self) -> u32 { match self { Self::Bank0 => Bank0GpioRegisterAccess::read_input(), - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Self::Bank1 => Bank1GpioRegisterAccess::read_input(), } } @@ -600,7 +616,7 @@ impl GpioRegisterAccess { fn read_output(self) -> u32 { match self { Self::Bank0 => Bank0GpioRegisterAccess::read_output(), - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Self::Bank1 => Bank1GpioRegisterAccess::read_output(), } } @@ -608,7 +624,7 @@ impl GpioRegisterAccess { fn read_interrupt_status(self) -> u32 { match self { Self::Bank0 => Bank0GpioRegisterAccess::read_interrupt_status(), - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Self::Bank1 => Bank1GpioRegisterAccess::read_interrupt_status(), } } @@ -616,7 +632,7 @@ impl GpioRegisterAccess { fn write_interrupt_status_clear(self, word: u32) { match self { Self::Bank0 => Bank0GpioRegisterAccess::write_interrupt_status_clear(word), - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Self::Bank1 => Bank1GpioRegisterAccess::write_interrupt_status_clear(word), } } @@ -632,7 +648,7 @@ impl GpioRegisterAccess { fn write_output_set(self, word: u32) { match self { Self::Bank0 => Bank0GpioRegisterAccess::write_output_set(word), - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Self::Bank1 => Bank1GpioRegisterAccess::write_output_set(word), } } @@ -640,7 +656,7 @@ impl GpioRegisterAccess { fn write_output_clear(self, word: u32) { match self { Self::Bank0 => Bank0GpioRegisterAccess::write_output_clear(word), - #[cfg(any(esp32, esp32s2, esp32s3))] + #[cfg(gpio_bank_1)] Self::Bank1 => Bank1GpioRegisterAccess::write_output_clear(word), } } @@ -692,10 +708,10 @@ impl Bank0GpioRegisterAccess { } } -#[cfg(any(esp32, esp32s2, esp32s3))] +#[cfg(gpio_bank_1)] struct Bank1GpioRegisterAccess; -#[cfg(any(esp32, esp32s2, esp32s3))] +#[cfg(gpio_bank_1)] impl Bank1GpioRegisterAccess { fn write_out_en_clear(word: u32) { unsafe { GPIO::steal() } @@ -863,14 +879,10 @@ impl InterruptConfigurable for Io { /// Install the given interrupt handler replacing any previously set /// handler. /// - /// ⚠️ Be careful when using this together with the async API: - /// - /// - The async driver will disable any interrupts whose status is not - /// cleared by the user handler. - /// - Clearing the interrupt status in the user handler will prevent the - /// async driver from detecting the interrupt, silently disabling the - /// corresponding pin's async API. - /// - You will not be notified if you make a mistake. + /// Note that when using interrupt handlers registered by this function, + /// we clear the interrupt status register for you. This is NOT the case + /// if you register the interrupt handler directly, by defining a + /// `#[no_mangle] unsafe extern "C" fn GPIO()` function. fn set_interrupt_handler(&mut self, handler: InterruptHandler) { for core in crate::Cpu::other() { crate::interrupt::disable(core, Interrupt::GPIO); @@ -883,22 +895,46 @@ impl InterruptConfigurable for Io { #[ram] extern "C" fn user_gpio_interrupt_handler() { - USER_INTERRUPT_HANDLER.call(); - - default_gpio_interrupt_handler(); + handle_pin_interrupts(|| USER_INTERRUPT_HANDLER.call()); } #[ram] extern "C" fn default_gpio_interrupt_handler() { - handle_pin_interrupts(on_pin_irq); + handle_pin_interrupts(|| ()); } #[ram] -fn on_pin_irq(pin_nr: u8) { - // FIXME: async handlers signal completion by disabling the interrupt, but this - // conflicts with user handlers. - set_int_enable(pin_nr, 0, 0, false); - asynch::PIN_WAKERS[pin_nr as usize].wake(); // wake task +fn handle_pin_interrupts(user_handler: fn()) { + let intrs_bank0 = InterruptStatusRegisterAccess::Bank0.interrupt_status_read(); + + #[cfg(gpio_bank_1)] + let intrs_bank1 = InterruptStatusRegisterAccess::Bank1.interrupt_status_read(); + + user_handler(); + + let banks = [ + (GpioRegisterAccess::Bank0, intrs_bank0), + #[cfg(gpio_bank_1)] + (GpioRegisterAccess::Bank1, intrs_bank1), + ]; + + for (bank, intrs) in banks { + // Get the mask of active async pins and also unmark them in the same go. + let async_pins = bank.async_operations().fetch_and(!intrs, Ordering::Relaxed); + + // Wake up the tasks + let mut intr_bits = intrs & async_pins; + while intr_bits != 0 { + let pin_pos = intr_bits.trailing_zeros(); + intr_bits -= 1 << pin_pos; + + let pin_nr = pin_pos as u8 + bank.offset(); + asynch::PIN_WAKERS[pin_nr as usize].wake(); + set_int_enable(pin_nr, Some(0), 0, false); + } + + bank.write_interrupt_status_clear(intrs); + } } #[doc(hidden)] @@ -1445,7 +1481,74 @@ where self.pin.level() } - /// Listen for interrupts + /// Listen for interrupts. + /// + /// The interrupts will be handled by the handler set using + /// [`Io::set_interrupt_handler`]. All GPIO pins share the same + /// interrupt handler. + /// + /// Note that [`Event::LowLevel`] and [`Event::HighLevel`] are fired + /// continuously when the pin is low or high, respectively. You must use + /// a custom interrupt handler to stop listening for these events, + /// otherwise your program will be stuck in a loop as long as the pin is + /// reading the corresponding level. + /// + /// ## Example: print something when a button is pressed. + /// + /// ```rust, no_run + #[doc = crate::before_snippet!()] + /// use esp_hal::gpio::{Event, Input, Pull, Io}; + /// + /// let mut io = Io::new(peripherals.IO_MUX); + /// io.set_interrupt_handler(handler); + /// + /// // Set up the input and store it in the static variable. + /// // This example uses a push button that is high when not + /// // pressed and low when pressed. + /// let mut button = Input::new(peripherals.GPIO5, Pull::Up); + /// + /// critical_section::with(|cs| { + /// // Here we are listening for a low level to demonstrate + /// // that you need to stop listening for level interrupts, + /// // but usually you'd probably use `FallingEdge`. + /// button.listen(Event::LowLevel); + /// BUTTON.borrow_ref_mut(cs).replace(button); + /// }); + /// # } + /// + /// // Outside of your `main` function: + /// + /// # use esp_hal::gpio::Input; + /// use core::cell::RefCell; + /// use critical_section::Mutex; + /// + /// // You will need to store the `Input` object in a static variable so + /// // that the interrupt handler can access it. + /// static BUTTON: Mutex>> = + /// Mutex::new(RefCell::new(None)); + /// + /// #[handler] + /// fn handler() { + /// critical_section::with(|cs| { + /// let mut button = BUTTON.borrow_ref_mut(cs); + /// let Some(button) = button.as_mut() else { + /// // Some other interrupt has occurred + /// // before the button was set up. + /// return; + /// }; + /// + /// if button.is_interrupt_set() { + /// print!("Button pressed"); + /// + /// // If you want to stop listening for interrupts, you need to + /// // call `unlisten` here. If you comment this line, the + /// // interrupt will fire continuously while the button + /// // is pressed. + /// button.unlisten(); + /// } + /// }); + /// } + /// ``` #[inline] pub fn listen(&mut self, event: Event) { self.pin.listen(event); @@ -1660,12 +1763,20 @@ where self.pin.level() } - /// Listen for interrupts + /// Listen for interrupts. + /// + /// See [`Input::listen`] for more information and an example. #[inline] pub fn listen(&mut self, event: Event) { self.pin.listen(event); } + /// Stop listening for interrupts. + #[inline] + pub fn unlisten(&mut self) { + self.pin.unlisten(); + } + /// Clear the interrupt status bit for this Pin #[inline] pub fn clear_interrupt(&mut self) { @@ -1815,21 +1926,30 @@ where set_int_enable( self.pin.number(), - gpio_intr_enable(int_enable, nmi_enable), + Some(gpio_intr_enable(int_enable, nmi_enable)), event as u8, wake_up_from_light_sleep, ) } - /// Listen for interrupts + /// Listen for interrupts. + /// + /// See [`Input::listen`] for more information and an example. #[inline] pub fn listen(&mut self, event: Event) { self.listen_with_options(event, true, false, false) } - /// Stop listening for interrupts + /// Stop listening for interrupts. + #[inline] pub fn unlisten(&mut self) { - set_int_enable(self.pin.number(), 0, 0, false); + set_int_enable(self.pin.number(), Some(0), 0, false); + } + + /// Check if the pin is listening for interrupts. + #[inline] + pub fn is_listening(&self) -> bool { + is_int_enabled(self.pin.number()) } /// Clear the interrupt status bit for this Pin @@ -2151,56 +2271,39 @@ pub(crate) mod internal { } } -fn is_listening(pin_num: u8) -> bool { - let bits = unsafe { GPIO::steal() } - .pin(pin_num as usize) - .read() - .int_ena() - .bits(); - bits != 0 -} - -fn set_int_enable(gpio_num: u8, int_ena: u8, int_type: u8, wake_up_from_light_sleep: bool) { +/// Set GPIO event listening. +/// +/// - `gpio_num`: the pin to configure +/// - `int_ena`: maskable and non-maskable CPU interrupt bits. None to leave +/// unchanged. +/// - `int_type`: interrupt type, see [Event] (or 0 to disable) +/// - `wake_up_from_light_sleep`: whether to wake up from light sleep +fn set_int_enable(gpio_num: u8, int_ena: Option, int_type: u8, wake_up_from_light_sleep: bool) { unsafe { GPIO::steal() } .pin(gpio_num as usize) .modify(|_, w| unsafe { - w.int_ena().bits(int_ena); + if let Some(int_ena) = int_ena { + w.int_ena().bits(int_ena); + } w.int_type().bits(int_type); w.wakeup_enable().bit(wake_up_from_light_sleep) }); } -#[ram] -fn handle_pin_interrupts(handle: impl Fn(u8)) { - let intrs_bank0 = InterruptStatusRegisterAccess::Bank0.interrupt_status_read(); - - #[cfg(any(esp32, esp32s2, esp32s3))] - let intrs_bank1 = InterruptStatusRegisterAccess::Bank1.interrupt_status_read(); - - let mut intr_bits = intrs_bank0; - while intr_bits != 0 { - let pin_nr = intr_bits.trailing_zeros(); - handle(pin_nr as u8); - intr_bits -= 1 << pin_nr; - } - - // clear interrupt bits - Bank0GpioRegisterAccess::write_interrupt_status_clear(intrs_bank0); - - #[cfg(any(esp32, esp32s2, esp32s3))] - { - let mut intr_bits = intrs_bank1; - while intr_bits != 0 { - let pin_nr = intr_bits.trailing_zeros(); - handle(pin_nr as u8 + 32); - intr_bits -= 1 << pin_nr; - } - Bank1GpioRegisterAccess::write_interrupt_status_clear(intrs_bank1); - } +fn is_int_enabled(gpio_num: u8) -> bool { + unsafe { GPIO::steal() } + .pin(gpio_num as usize) + .read() + .int_ena() + .bits() + != 0 } mod asynch { - use core::task::{Context, Poll}; + use core::{ + future::poll_fn, + task::{Context, Poll}, + }; use super::*; use crate::asynch::AtomicWaker; @@ -2213,34 +2316,85 @@ mod asynch { where P: InputPin, { - async fn wait_for(&mut self, event: Event) { - self.listen(event); - PinFuture::new(self.pin.number()).await + /// Wait until the pin experiences a particular [`Event`]. + /// + /// The GPIO driver will disable listening for the event once it occurs, + /// or if the `Future` is dropped. + /// + /// Note that calling this function will overwrite previous + /// [`listen`][Self::listen] operations for this pin. + #[inline] + pub async fn wait_for(&mut self, event: Event) { + let mut future = PinFuture { + pin: unsafe { self.clone_unchecked() }, + }; + + // Make sure this pin is not being processed by an interrupt handler. + if future.pin.is_listening() { + set_int_enable( + future.pin.number(), + None, // Do not disable handling pending interrupts. + 0, // Disable generating new events + false, + ); + poll_fn(|cx| { + if future.pin.is_interrupt_set() { + cx.waker().wake_by_ref(); + Poll::Pending + } else { + Poll::Ready(()) + } + }) + .await; + } + + // At this point the pin is no longer listening, we can safely + // do our setup. + + // Mark pin as async. + future + .pin + .gpio_bank(private::Internal) + .async_operations() + .fetch_or(future.pin_mask(), Ordering::Relaxed); + + future.pin.listen(event); + + future.await } - /// Wait until the pin is high. If it is already high, return - /// immediately. + /// Wait until the pin is high. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_high(&mut self) { self.wait_for(Event::HighLevel).await } - /// Wait until the pin is low. If it is already low, return immediately. + /// Wait until the pin is low. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_low(&mut self) { self.wait_for(Event::LowLevel).await } /// Wait for the pin to undergo a transition from low to high. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_rising_edge(&mut self) { self.wait_for(Event::RisingEdge).await } /// Wait for the pin to undergo a transition from high to low. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_falling_edge(&mut self) { self.wait_for(Event::FallingEdge).await } /// Wait for the pin to undergo any transition, i.e low to high OR high /// to low. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_any_edge(&mut self) { self.wait_for(Event::AnyEdge).await } @@ -2250,60 +2404,117 @@ mod asynch { where P: InputPin, { - /// Wait until the pin is high. If it is already high, return - /// immediately. + /// Wait until the pin experiences a particular [`Event`]. + /// + /// The GPIO driver will disable listening for the event once it occurs, + /// or if the `Future` is dropped. + /// + /// Note that calling this function will overwrite previous + /// [`listen`][Self::listen] operations for this pin. + #[inline] + pub async fn wait_for(&mut self, event: Event) { + self.pin.wait_for(event).await + } + + /// Wait until the pin is high. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_high(&mut self) { self.pin.wait_for_high().await } - /// Wait until the pin is low. If it is already low, return immediately. + /// Wait until the pin is low. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_low(&mut self) { self.pin.wait_for_low().await } /// Wait for the pin to undergo a transition from low to high. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_rising_edge(&mut self) { self.pin.wait_for_rising_edge().await } /// Wait for the pin to undergo a transition from high to low. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_falling_edge(&mut self) { self.pin.wait_for_falling_edge().await } /// Wait for the pin to undergo any transition, i.e low to high OR high /// to low. + /// + /// See [Self::wait_for] for more information. pub async fn wait_for_any_edge(&mut self) { self.pin.wait_for_any_edge().await } } #[must_use = "futures do nothing unless you `.await` or poll them"] - struct PinFuture { - pin_num: u8, + struct PinFuture<'d, P: InputPin> { + pin: Flex<'d, P>, } - impl PinFuture { - fn new(pin_num: u8) -> Self { - Self { pin_num } + impl<'d, P: InputPin> PinFuture<'d, P> { + fn pin_mask(&self) -> u32 { + let bank = self.pin.gpio_bank(private::Internal); + 1 << (self.pin.number() - bank.offset()) + } + + fn is_done(&self) -> bool { + // Only the interrupt handler should clear the async bit, and only if the + // specific pin is handling an interrupt. + self.pin + .gpio_bank(private::Internal) + .async_operations() + .load(Ordering::Acquire) + & self.pin_mask() + == 0 } } - impl core::future::Future for PinFuture { + impl core::future::Future for PinFuture<'_, P> { type Output = (); fn poll(self: core::pin::Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - PIN_WAKERS[self.pin_num as usize].register(cx.waker()); + PIN_WAKERS[self.pin.number() as usize].register(cx.waker()); - // if pin is no longer listening its been triggered - // therefore the future has resolved - if !is_listening(self.pin_num) { + if self.is_done() { Poll::Ready(()) } else { Poll::Pending } } } + + impl Drop for PinFuture<'_, P> { + fn drop(&mut self) { + // If the pin isn't listening, the future has either been dropped before setup, + // or the interrupt has already been handled. + if self.pin.is_listening() { + // Make sure the future isn't dropped while the interrupt is being handled. + // This prevents tricky drop-and-relisten scenarios. + + set_int_enable( + self.pin.number(), + None, // Do not disable handling pending interrupts. + 0, // Disable generating new events + false, + ); + + while self.pin.is_interrupt_set() {} + + // Unmark pin as async + self.pin + .gpio_bank(private::Internal) + .async_operations() + .fetch_and(!self.pin_mask(), Ordering::Relaxed); + } + } + } } mod embedded_hal_impls { diff --git a/esp-hal/src/soc/esp32/gpio.rs b/esp-hal/src/soc/esp32/gpio.rs index 78156ff5c..2c0e38c4c 100644 --- a/esp-hal/src/soc/esp32/gpio.rs +++ b/esp-hal/src/soc/esp32/gpio.rs @@ -787,14 +787,8 @@ pub(crate) enum InterruptStatusRegisterAccess { impl InterruptStatusRegisterAccess { pub(crate) fn interrupt_status_read(self) -> u32 { match self { - Self::Bank0 => match Cpu::current() { - Cpu::ProCpu => unsafe { GPIO::steal() }.pcpu_int().read().bits(), - Cpu::AppCpu => unsafe { GPIO::steal() }.acpu_int().read().bits(), - }, - Self::Bank1 => match Cpu::current() { - Cpu::ProCpu => unsafe { GPIO::steal() }.pcpu_int1().read().bits(), - Cpu::AppCpu => unsafe { GPIO::steal() }.acpu_int1().read().bits(), - }, + Self::Bank0 => unsafe { GPIO::steal() }.status().read().bits(), + Self::Bank1 => unsafe { GPIO::steal() }.status1().read().bits(), } } } diff --git a/esp-metadata/CHANGELOG.md b/esp-metadata/CHANGELOG.md index d362ae1a2..00db06c63 100644 --- a/esp-metadata/CHANGELOG.md +++ b/esp-metadata/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Introduced the `wifi6` symbol (#2612) +- Introduced the `gpio_bank_1` symbol (#2625) ### Fixed diff --git a/esp-metadata/devices/esp32.toml b/esp-metadata/devices/esp32.toml index e5a70a666..7ae624ea9 100644 --- a/esp-metadata/devices/esp32.toml +++ b/esp-metadata/devices/esp32.toml @@ -62,6 +62,7 @@ symbols = [ "timg_timer1", "touch", "large_intr_status", + "gpio_bank_1", # ROM capabilities "rom_crc_le", diff --git a/esp-metadata/devices/esp32p4.toml b/esp-metadata/devices/esp32p4.toml index 9d745becd..c973e4ff7 100644 --- a/esp-metadata/devices/esp32p4.toml +++ b/esp-metadata/devices/esp32p4.toml @@ -95,4 +95,5 @@ symbols = [ # Additional peripherals defined by us (the developers): "clic", "very_large_intr_status", + "gpio_bank_1", ] diff --git a/esp-metadata/devices/esp32s2.toml b/esp-metadata/devices/esp32s2.toml index 2b556cd00..32db8a323 100644 --- a/esp-metadata/devices/esp32s2.toml +++ b/esp-metadata/devices/esp32s2.toml @@ -58,6 +58,7 @@ symbols = [ "ulp_riscv_core", "timg_timer1", "large_intr_status", + "gpio_bank_1", # ROM capabilities "rom_crc_le", diff --git a/esp-metadata/devices/esp32s3.toml b/esp-metadata/devices/esp32s3.toml index 96fbe5727..31e1f3cf4 100644 --- a/esp-metadata/devices/esp32s3.toml +++ b/esp-metadata/devices/esp32s3.toml @@ -72,6 +72,7 @@ symbols = [ "ulp_riscv_core", "timg_timer1", "very_large_intr_status", + "gpio_bank_1", # ROM capabilities "rom_crc_le", diff --git a/hil-test/tests/gpio.rs b/hil-test/tests/gpio.rs index 72cf65234..6b6888083 100644 --- a/hil-test/tests/gpio.rs +++ b/hil-test/tests/gpio.rs @@ -119,6 +119,14 @@ mod tests { assert_eq!(test_gpio1.is_high(), false); } + #[test] + async fn waiting_for_level_does_not_hang(ctx: Context) { + let mut test_gpio1 = Input::new(ctx.test_gpio1, Pull::Down); + let _test_gpio2 = Output::new(ctx.test_gpio2, Level::High); + + test_gpio1.wait_for_high().await; + } + #[test] fn gpio_output(ctx: Context) { let mut test_gpio2 = Output::new(ctx.test_gpio2, Level::Low); diff --git a/hil-test/tests/gpio_custom_handler.rs b/hil-test/tests/gpio_custom_handler.rs index 480a067b5..c70332067 100644 --- a/hil-test/tests/gpio_custom_handler.rs +++ b/hil-test/tests/gpio_custom_handler.rs @@ -13,7 +13,7 @@ use embassy_time::{Duration, Timer}; use esp_hal::{ - gpio::{AnyPin, Input, Io, Level, Output, Pull}, + gpio::{AnyPin, Flex, Input, Io, Level, Output, Pull}, interrupt::InterruptConfigurable, macros::handler, timer::timg::TimerGroup, @@ -23,7 +23,16 @@ use portable_atomic::{AtomicUsize, Ordering}; #[no_mangle] unsafe extern "C" fn GPIO() { - // do nothing, prevents binding the default handler + // Prevents binding the default handler, but we need to clear the GPIO + // interrupts by hand. + let peripherals = esp_hal::peripherals::Peripherals::steal(); + + let (gpio1, _) = hil_test::common_test_pins!(peripherals); + + // Using flex will not mutate the pin. + let mut gpio1 = Flex::new(gpio1); + + gpio1.clear_interrupt(); } #[handler] @@ -71,6 +80,14 @@ mod tests { let timg0 = TimerGroup::new(peripherals.TIMG0); esp_hal_embassy::init(timg0.timer0); + // We need to enable the GPIO interrupt, otherwise the async Future's + // setup or Drop implementation hangs. + esp_hal::interrupt::enable( + esp_hal::peripherals::Interrupt::GPIO, + esp_hal::interrupt::Priority::Priority1, + ) + .unwrap(); + let counter = drive_pins(gpio1, gpio2).await; // GPIO is bound to something else, so we don't expect the async API to work.