From 232b3e8c39198226f3b30afdbce5a928e20fbf0d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 27 Apr 2026 10:04:27 +0200 Subject: [PATCH] possible bugfix for asynch GPIO --- va108xx/examples/embassy/Cargo.toml | 9 +-- .../examples/embassy/src/bin/async-gpio.rs | 58 ++++++++++--------- .../examples/embassy/src/bin/async-uart-rx.rs | 4 +- va108xx/vorago-reb1/src/button.rs | 4 +- vorago-shared-hal/CHANGELOG.md | 2 + vorago-shared-hal/Cargo.toml | 2 +- vorago-shared-hal/src/gpio/asynch.rs | 47 +++++++-------- vorago-shared-hal/src/gpio/ll.rs | 45 ++++++++++---- vorago-shared-hal/src/gpio/mod.rs | 23 ++++++-- vorago-shared-hal/src/gpio/regs.rs | 3 +- vorago-shared-hal/src/lib.rs | 2 +- 11 files changed, 118 insertions(+), 81 deletions(-) diff --git a/va108xx/examples/embassy/Cargo.toml b/va108xx/examples/embassy/Cargo.toml index e9bdfbc..257761d 100644 --- a/va108xx/examples/embassy/Cargo.toml +++ b/va108xx/examples/embassy/Cargo.toml @@ -4,8 +4,9 @@ version = "0.1.0" edition = "2021" [dependencies] -cfg-if = "1" +cortex-m = { version = "0.7", features = ["critical-section-single-core"] } cortex-m-rt = "0.7" +cfg-if = "1" embedded-hal-async = "1" embedded-io = "0.7" embedded-io-async = "0.7" @@ -18,10 +19,10 @@ panic-probe = { version = "1", features = ["print-defmt"] } critical-section = "1" -embassy-sync = "0.7" +embassy-sync = "0.8" embassy-time = "0.5" -embassy-executor = { version = "0.9", features = [ - "arch-cortex-m", +embassy-executor = { version = "0.10", features = [ + "platform-cortex-m", "executor-thread", "executor-interrupt" ]} diff --git a/va108xx/examples/embassy/src/bin/async-gpio.rs b/va108xx/examples/embassy/src/bin/async-gpio.rs index 537ccd4..c381cf9 100644 --- a/va108xx/examples/embassy/src/bin/async-gpio.rs +++ b/va108xx/examples/embassy/src/bin/async-gpio.rs @@ -62,6 +62,9 @@ async fn main(spawner: Spawner) { // Safety: Only called once here. va108xx_embassy::init(dp.tim23, dp.tim22, SYSCLK_FREQ); + unsafe { + cortex_m::interrupt::enable(); + } let porta = PinsA::new(dp.porta); let portb = PinsB::new(dp.portb); @@ -71,36 +74,35 @@ async fn main(spawner: Spawner) { let out_pb22 = Output::new(portb.pb22, PinState::Low); let in_pb23 = Input::new_floating(portb.pb23); - let mut in_pa1_async = InputPinAsync::new(in_pa1, pac::Interrupt::OC10); - let mut in_pb23_async = InputPinAsync::new(in_pb23, PB22_TO_PB23_IRQ); + let mut in_pa1_async = InputPinAsync::new( + in_pa1, + va108xx_hal::InterruptConfig::new(pac::Interrupt::OC10, true, true), + ); + let mut in_pb23_async = InputPinAsync::new( + in_pb23, + va108xx_hal::InterruptConfig::new(PB22_TO_PB23_IRQ, true, true), + ); - spawner - .spawn(output_task( - "PA0 to PA1", - out_pa0, - CHANNEL_PA0_PA1.receiver(), - )) - .unwrap(); - spawner - .spawn(output_task( - "PB22 to PB23", - out_pb22, - CHANNEL_PB22_TO_PB23.receiver(), - )) - .unwrap(); + spawner.spawn(output_task("PA0 to PA1", out_pa0, CHANNEL_PA0_PA1.receiver()).unwrap()); + spawner.spawn(output_task("PB22 to PB23", out_pb22, CHANNEL_PB22_TO_PB23.receiver()).unwrap()); - if CHECK_PA0_TO_PA1 { - check_pin_to_pin_async_ops("PA0 to PA1", CHANNEL_PA0_PA1.sender(), &mut in_pa1_async).await; - defmt::info!("Example PA0 to PA1 done"); - } - if CHECK_PB22_TO_PB23 { - check_pin_to_pin_async_ops( - "PB22 to PB23", - CHANNEL_PB22_TO_PB23.sender(), - &mut in_pb23_async, - ) - .await; - defmt::info!("Example PB22 to PB23 done"); + for i in 0..3 { + defmt::info!("Starting async GPIO operations check {}", i); + if CHECK_PA0_TO_PA1 { + check_pin_to_pin_async_ops("PA0 to PA1", CHANNEL_PA0_PA1.sender(), &mut in_pa1_async) + .await; + defmt::info!("Example PA0 to PA1 done"); + } + if CHECK_PB22_TO_PB23 { + check_pin_to_pin_async_ops( + "PB22 to PB23", + CHANNEL_PB22_TO_PB23.sender(), + &mut in_pb23_async, + ) + .await; + defmt::info!("Example PB22 to PB23 done"); + } + Timer::after(Duration::from_millis(500)).await; } defmt::info!("Example done, toggling LED0"); diff --git a/va108xx/examples/embassy/src/bin/async-uart-rx.rs b/va108xx/examples/embassy/src/bin/async-uart-rx.rs index 3dd48bc..3c9799e 100644 --- a/va108xx/examples/embassy/src/bin/async-uart-rx.rs +++ b/va108xx/examples/embassy/src/bin/async-uart-rx.rs @@ -97,9 +97,7 @@ async fn main(spawner: Spawner) { }); let mut async_rx_uart_a = RxAsync::new(rx_uart_a, cons_uart_a); let async_rx_uart_b = RxAsyncOverwriting::new(rx_uart_b, &CONSUMER_UART_B); - spawner - .spawn(uart_b_task(async_rx_uart_b, tx_uart_b)) - .unwrap(); + spawner.spawn(uart_b_task(async_rx_uart_b, tx_uart_b).unwrap()); let mut buf = [0u8; 256]; loop { defmt::info!("Current time UART A: {}", Instant::now().as_secs()); diff --git a/va108xx/vorago-reb1/src/button.rs b/va108xx/vorago-reb1/src/button.rs index c9434d8..17e9b9c 100644 --- a/va108xx/vorago-reb1/src/button.rs +++ b/va108xx/vorago-reb1/src/button.rs @@ -36,7 +36,7 @@ impl Button { irq_cfg: InterruptConfig, ) { self.0.configure_edge_interrupt(edge_type); - self.0.enable_interrupt(irq_cfg); + self.0.enable_interrupt(irq_cfg, true); } /// Configures an IRQ on level. @@ -46,7 +46,7 @@ impl Button { irq_cfg: InterruptConfig, ) { self.0.configure_level_interrupt(level); - self.0.enable_interrupt(irq_cfg); + self.0.enable_interrupt(irq_cfg, true); } /// Configures a filter on the button. This can be useful for debouncing the switch. diff --git a/vorago-shared-hal/CHANGELOG.md b/vorago-shared-hal/CHANGELOG.md index fadbc19..c799b28 100644 --- a/vorago-shared-hal/CHANGELOG.md +++ b/vorago-shared-hal/CHANGELOG.md @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added missing `AnyPin` trait impl for Multi HW CS pins. - Expose inner `Input` pin for `InputPinAsync`. - Bugfix for UART clock calculation with 8x baud mode. +- Possible bugfix for Asynch GPIO where the interrupt handler could become stuck in a loop. +- Robustness improvements for the Asynch GPIO driver code. ## [v0.2.0] 2025-09-03 diff --git a/vorago-shared-hal/Cargo.toml b/vorago-shared-hal/Cargo.toml index 71b9662..fc37222 100644 --- a/vorago-shared-hal/Cargo.toml +++ b/vorago-shared-hal/Cargo.toml @@ -29,7 +29,7 @@ fugit = "0.3" defmt = { version = "1", optional = true } va108xx = { version = "0.6", path = "../va108xx/va108xx", default-features = false, optional = true } va416xx = { version = "0.5", path = "../va416xx/va416xx", default-features = false, optional = true } -embassy-sync = "0.7" +embassy-sync = "0.8" embassy-time-driver = "0.2" embassy-time-queue-utils = "0.3" once_cell = { version = "1", default-features = false, features = [ diff --git a/vorago-shared-hal/src/gpio/asynch.rs b/vorago-shared-hal/src/gpio/asynch.rs index 5531bb5..6421a6b 100644 --- a/vorago-shared-hal/src/gpio/asynch.rs +++ b/vorago-shared-hal/src/gpio/asynch.rs @@ -21,9 +21,6 @@ use crate::{InterruptConfig, NUM_PORT_A, NUM_PORT_B}; #[cfg(feature = "vor4x")] use super::ll::PortDoesNotSupportInterrupts; -#[cfg(feature = "vor1x")] -use va108xx as pac; - pub use super::ll::InterruptEdge; use super::{ Input, Port, @@ -118,7 +115,7 @@ pub fn on_interrupt_for_async_gpio_for_port( } fn on_interrupt_for_async_gpio_for_port_generic(port: Port) { - let gpio = unsafe { port.steal_gpio() }; + let mut gpio = unsafe { port.steal_regs() }; let irq_enb = gpio.read_irq_enable(); let edge_status = gpio.read_edge_status(); @@ -134,18 +131,19 @@ fn on_interrupt_for_port( wakers: &'static [AtomicWaker], edge_detection: &'static [AtomicBool], ) { + // Check all enabled interrupts. while irq_enb != 0 { + // For all enabled interrupts, check whether the corresponding edge detection has + // triggered. let bit_pos = irq_enb.trailing_zeros() as usize; let bit_mask = 1 << bit_pos; - wakers[bit_pos].wake(); - if edge_status & bit_mask != 0 { edge_detection[bit_pos].store(true, core::sync::atomic::Ordering::Relaxed); - - // Clear the processed bit - irq_enb &= !bit_mask; + wakers[bit_pos].wake(); } + // Clear the processed bit + irq_enb &= !bit_mask; } } @@ -163,13 +161,12 @@ pub struct InputPinFuture { impl InputPinFuture { /// Create a new input pin future from mutable reference to an [Input] pin. #[cfg(feature = "vor1x")] - pub fn new_with_input_pin(pin: &mut Input, irq: pac::Interrupt, edge: InterruptEdge) -> Self { + pub fn new_with_input_pin(pin: &mut Input, edge: InterruptEdge) -> Self { let (waker_group, edge_detection_group) = pin_group_to_waker_and_edge_detection_group(pin.id().port()); edge_detection_group[pin.id().offset()].store(false, core::sync::atomic::Ordering::Relaxed); pin.configure_edge_interrupt(edge); - #[cfg(feature = "vor1x")] - pin.enable_interrupt(InterruptConfig::new(irq, true, true)); + pin.enable_interrupt_gpio_only(); Self { id: pin.id(), waker_group, @@ -186,7 +183,7 @@ impl InputPinFuture { let (waker_group, edge_detection_group) = pin_group_to_waker_and_edge_detection_group(pin.id().port()); pin.configure_edge_interrupt(edge); - pin.enable_interrupt(true)?; + pin.enable_interrupt_gpio_only(); Ok(Self { id: pin.id(), waker_group, @@ -223,8 +220,6 @@ impl Future for InputPinFuture { /// Input pin which has additional asynchronous support. pub struct InputPinAsync { pin: Input, - #[cfg(feature = "vor1x")] - irq: va108xx::Interrupt, } impl InputPinAsync { @@ -235,8 +230,10 @@ impl InputPinAsync { /// generic [on_interrupt_for_async_gpio_for_port] function must be called inside that function /// for the asynchronous functionality to work. #[cfg(feature = "vor1x")] - pub fn new(pin: Input, irq: va108xx::Interrupt) -> Self { - Self { pin, irq } + pub fn new(mut pin: Input, irq_config: InterruptConfig) -> Self { + // Do not enable GPIO interrupt bit yet. + pin.enable_interrupt(irq_config, false); + Self { pin } } /// Create a new asynchronous input pin from an [Input] pin. The interrupt ID to be used must be @@ -246,10 +243,12 @@ impl InputPinAsync { /// generic [on_interrupt_for_async_gpio_for_port] function must be called inside that function /// for the asynchronous functionality to work. #[cfg(feature = "vor4x")] - pub fn new(pin: Input) -> Result { + pub fn new(mut pin: Input) -> Result { if pin.id().port() == Port::G { return Err(PortDoesNotSupportInterrupts); } + // Do not enable GPIO interrupt bit yet. + pin.enable_interrupt(true, false)?; Ok(Self { pin }) } @@ -259,8 +258,7 @@ impl InputPinAsync { pub async fn wait_for_high(&mut self) { // Unwrap okay, checked pin in constructor. #[cfg(feature = "vor1x")] - let fut = - InputPinFuture::new_with_input_pin(&mut self.pin, self.irq, InterruptEdge::LowToHigh); + let fut = InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::LowToHigh); #[cfg(feature = "vor4x")] let fut = InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::LowToHigh).unwrap(); @@ -300,8 +298,7 @@ impl InputPinAsync { pub async fn wait_for_low(&mut self) { // Unwrap okay, checked pin in constructor. #[cfg(feature = "vor1x")] - let fut = - InputPinFuture::new_with_input_pin(&mut self.pin, self.irq, InterruptEdge::HighToLow); + let fut = InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::HighToLow); #[cfg(feature = "vor4x")] let fut = InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::HighToLow).unwrap(); @@ -315,7 +312,7 @@ impl InputPinAsync { pub async fn wait_for_falling_edge(&mut self) { // Unwrap okay, checked pin in constructor. #[cfg(feature = "vor1x")] - InputPinFuture::new_with_input_pin(&mut self.pin, self.irq, InterruptEdge::HighToLow).await; + InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::HighToLow).await; #[cfg(feature = "vor4x")] InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::HighToLow) .unwrap() @@ -326,14 +323,14 @@ impl InputPinAsync { pub async fn wait_for_rising_edge(&mut self) { // Unwrap okay, checked pin in constructor. #[cfg(feature = "vor1x")] - InputPinFuture::new_with_input_pin(&mut self.pin, self.irq, InterruptEdge::LowToHigh).await; + InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::LowToHigh).await; } /// Asynchronously wait until the pin sees any edge (either rising or falling). pub async fn wait_for_any_edge(&mut self) { // Unwrap okay, checked pin in constructor. #[cfg(feature = "vor1x")] - InputPinFuture::new_with_input_pin(&mut self.pin, self.irq, InterruptEdge::BothEdges).await; + InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::BothEdges).await; #[cfg(feature = "vor4x")] InputPinFuture::new_with_input_pin(&mut self.pin, InterruptEdge::BothEdges) .unwrap() diff --git a/vorago-shared-hal/src/gpio/ll.rs b/vorago-shared-hal/src/gpio/ll.rs index 0389cb2..92310f3 100644 --- a/vorago-shared-hal/src/gpio/ll.rs +++ b/vorago-shared-hal/src/gpio/ll.rs @@ -384,32 +384,53 @@ impl LowLevelGpio { self.gpio.write_tog_out(self.mask_32()); } - #[cfg(feature = "vor1x")] - pub fn enable_interrupt(&mut self, irq_cfg: crate::InterruptConfig) { - if irq_cfg.route { - self.configure_irqsel(irq_cfg.id); - } - if irq_cfg.enable_in_nvic { - unsafe { crate::enable_nvic_interrupt(irq_cfg.id) }; - } + /// Only enabled GPIO peripheral interrupt bit without enabling the interrupt in NVIC + /// or routing it in the IRQSEL peripheral for VA108xx devices. + #[inline] + pub fn enable_interrupt_gpio_only(&mut self) { self.gpio.modify_irq_enable(|mut value| { value |= 1 << self.id.offset; value }); } + /// Depending on the configuration parameters, does the following: + /// + /// - Routes the interrupt in the IRQSEL peripheral + /// - Enables the interrupt in the NVIC, + /// - Enable the GPIO peripheral interrupt bit for this pin if configured. + #[cfg(feature = "vor1x")] + pub fn enable_interrupt(&mut self, irq_cfg: crate::InterruptConfig, gpio: bool) { + if irq_cfg.route { + self.configure_irqsel(irq_cfg.id); + } + if irq_cfg.enable_in_nvic { + unsafe { crate::enable_nvic_interrupt(irq_cfg.id) }; + } + if gpio { + self.enable_interrupt_gpio_only(); + } + } + + /// Depending on the configuration parameters, does the following: + /// + /// - Enables the interrupt in the NVIC, + /// - Enable the GPIO peripheral interrupt bit for this pin if configured. #[cfg(feature = "vor4x")] pub fn enable_interrupt( &mut self, enable_in_nvic: bool, + gpio: bool, ) -> Result<(), PortDoesNotSupportInterrupts> { + if self.id().port() == Port::G { + return Err(PortDoesNotSupportInterrupts); + } if enable_in_nvic { unsafe { crate::enable_nvic_interrupt(self.id().irq_unchecked()) }; } - self.gpio.modify_irq_enable(|mut value| { - value |= 1 << self.id.offset; - value - }); + if gpio { + self.enable_interrupt_gpio_only(); + } Ok(()) } diff --git a/vorago-shared-hal/src/gpio/mod.rs b/vorago-shared-hal/src/gpio/mod.rs index 286b22b..abe5934 100644 --- a/vorago-shared-hal/src/gpio/mod.rs +++ b/vorago-shared-hal/src/gpio/mod.rs @@ -132,19 +132,34 @@ impl Input { self.0.id() } - #[cfg(feature = "vor1x")] #[inline] - pub fn enable_interrupt(&mut self, irq_cfg: crate::InterruptConfig) { - self.0.enable_interrupt(irq_cfg); + pub fn enable_interrupt_gpio_only(&mut self) { + self.0.enable_interrupt_gpio_only(); } + /// Depending on the configuration parameters, does the following: + /// + /// - Routes the interrupt in the IRQSEL peripheral + /// - Enables the interrupt in the NVIC, + /// - Enable the GPIO peripheral interrupt bit for this pin if configured. + #[cfg(feature = "vor1x")] + #[inline] + pub fn enable_interrupt(&mut self, irq_cfg: crate::InterruptConfig, gpio: bool) { + self.0.enable_interrupt(irq_cfg, gpio); + } + + /// Depending on the configuration parameters, does the following: + /// + /// - Enables the interrupt in the NVIC, + /// - Enable the GPIO peripheral interrupt bit for this pin if configured. #[cfg(feature = "vor4x")] #[inline] pub fn enable_interrupt( &mut self, enable_in_nvic: bool, + gpio: bool, ) -> Result<(), ll::PortDoesNotSupportInterrupts> { - self.0.enable_interrupt(enable_in_nvic) + self.0.enable_interrupt(enable_in_nvic, gpio) } #[inline] diff --git a/vorago-shared-hal/src/gpio/regs.rs b/vorago-shared-hal/src/gpio/regs.rs index e0ae9db..4d4dbfe 100644 --- a/vorago-shared-hal/src/gpio/regs.rs +++ b/vorago-shared-hal/src/gpio/regs.rs @@ -58,7 +58,8 @@ pub struct Gpio { /// Read-only register which shows enabled and active interrupts. Called IRQ_end by Vorago. #[mmio(PureRead)] irq_status: u32, - #[mmio(PureRead)] + // Reading this register clears it. + #[mmio(Read)] edge_status: u32, #[cfg(feature = "vor1x")] diff --git a/vorago-shared-hal/src/lib.rs b/vorago-shared-hal/src/lib.rs index 36d316e..b188c7d 100644 --- a/vorago-shared-hal/src/lib.rs +++ b/vorago-shared-hal/src/lib.rs @@ -137,7 +137,7 @@ impl Port { /// # Safety /// /// Circumvents ownership and safety guarantees by the HAL. - pub unsafe fn steal_gpio(&self) -> gpio::regs::MmioGpio<'static> { + pub unsafe fn steal_regs(&self) -> gpio::regs::MmioGpio<'static> { gpio::regs::Gpio::new_mmio(*self) } } -- 2.43.0