From 38c5545e07ff29764cf69a2cd1690d31055fe377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Thu, 16 Jan 2025 13:29:54 +0100 Subject: [PATCH 1/2] Actually add inherent set_interrupt_handler fns --- esp-hal/src/i2c/master/mod.rs | 21 ++++++++++++++++++++- esp-hal/src/i2s/master.rs | 20 ++++++++++++++++++-- esp-hal/src/interrupt/mod.rs | 17 ++++++++++++----- esp-hal/src/spi/master.rs | 27 +++++++++++++++++++++++---- esp-hal/src/uart.rs | 22 +++++++++++++++++++++- 5 files changed, 94 insertions(+), 13 deletions(-) diff --git a/esp-hal/src/i2c/master/mod.rs b/esp-hal/src/i2c/master/mod.rs index be4456d03d6..acec9518145 100644 --- a/esp-hal/src/i2c/master/mod.rs +++ b/esp-hal/src/i2c/master/mod.rs @@ -601,6 +601,25 @@ impl<'d> I2c<'d, Blocking> { Ok(i2c) } + #[cfg_attr( + not(multi_core), + doc = "Registers an interrupt handler for the peripheral." + )] + #[cfg_attr( + multi_core, + doc = "Registers an interrupt handler for the peripheral on the current core." + )] + #[doc = ""] + /// Note that this will replace any previously registered interrupt + /// handlers. + /// + /// You can restore the default/unhandled interrupt handler by using + /// [crate::DEFAULT_INTERRUPT_HANDLER] + #[instability::unstable] + fn set_interrupt_handler(&mut self, handler: InterruptHandler) { + self.i2c.info().set_interrupt_handler(handler); + } + /// Listen for the given interrupts #[instability::unstable] pub fn listen(&mut self, interrupts: impl Into>) { @@ -707,7 +726,7 @@ impl<'d> I2c<'d, Blocking> { impl private::Sealed for I2c<'_, Blocking> {} impl InterruptConfigurable for I2c<'_, Blocking> { - fn set_interrupt_handler(&mut self, handler: crate::interrupt::InterruptHandler) { + fn set_interrupt_handler(&mut self, handler: InterruptHandler) { self.i2c.info().set_interrupt_handler(handler); } } diff --git a/esp-hal/src/i2s/master.rs b/esp-hal/src/i2s/master.rs index 47eb5efe75a..2f9db2e307d 100644 --- a/esp-hal/src/i2s/master.rs +++ b/esp-hal/src/i2s/master.rs @@ -264,33 +264,49 @@ impl I2s<'_, Dm> where Dm: DriverMode, { - /// Sets the interrupt handler + #[cfg_attr( + not(multi_core), + doc = "Registers an interrupt handler for the peripheral." + )] + #[cfg_attr( + multi_core, + doc = "Registers an interrupt handler for the peripheral on the current core." + )] + #[doc = ""] + /// Note that this will replace any previously registered interrupt + /// handlers. /// - /// Interrupts are not enabled at the peripheral level here. + /// You can restore the default/unhandled interrupt handler by using + /// [crate::interrupt::DEFAULT_INTERRUPT_HANDLER] + #[instability::unstable] pub fn set_interrupt_handler(&mut self, handler: InterruptHandler) { // tx.i2s and rx.i2s is the same, we could use either one self.i2s_tx.i2s.set_interrupt_handler(handler); } /// Listen for the given interrupts + #[instability::unstable] pub fn listen(&mut self, interrupts: impl Into>) { // tx.i2s and rx.i2s is the same, we could use either one self.i2s_tx.i2s.enable_listen(interrupts.into(), true); } /// Unlisten the given interrupts + #[instability::unstable] pub fn unlisten(&mut self, interrupts: impl Into>) { // tx.i2s and rx.i2s is the same, we could use either one self.i2s_tx.i2s.enable_listen(interrupts.into(), false); } /// Gets asserted interrupts + #[instability::unstable] pub fn interrupts(&mut self) -> EnumSet { // tx.i2s and rx.i2s is the same, we could use either one self.i2s_tx.i2s.interrupts() } /// Resets asserted interrupts + #[instability::unstable] pub fn clear_interrupts(&mut self, interrupts: impl Into>) { // tx.i2s and rx.i2s is the same, we could use either one self.i2s_tx.i2s.clear_interrupts(interrupts.into()); diff --git a/esp-hal/src/interrupt/mod.rs b/esp-hal/src/interrupt/mod.rs index d5f37bd9024..797b6e855d3 100644 --- a/esp-hal/src/interrupt/mod.rs +++ b/esp-hal/src/interrupt/mod.rs @@ -102,11 +102,18 @@ pub const DEFAULT_INTERRUPT_HANDLER: InterruptHandler = InterruptHandler::new( /// Trait implemented by drivers which allow the user to set an /// [InterruptHandler] pub trait InterruptConfigurable: crate::private::Sealed { - /// Set the interrupt handler - /// - /// Note that this will replace any previously registered interrupt handler. - /// Some peripherals offer a shared interrupt handler for multiple purposes. - /// It's the users duty to honor this. + #[cfg_attr( + not(multi_core), + doc = "Registers an interrupt handler for the peripheral." + )] + #[cfg_attr( + multi_core, + doc = "Registers an interrupt handler for the peripheral on the current core." + )] + #[doc = ""] + /// Note that this will replace any previously registered interrupt + /// handlers. Some peripherals offer a shared interrupt handler for + /// multiple purposes. It's the users duty to honor this. /// /// You can restore the default/unhandled interrupt handler by using /// [DEFAULT_INTERRUPT_HANDLER] diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index d4a9e904688..b6b7797b272 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -583,12 +583,22 @@ impl<'d> Spi<'d, Blocking> { { SpiDma::new(self.spi, channel.map(|ch| ch.degrade()).into_ref()) } -} -impl InterruptConfigurable for Spi<'_, Blocking> { - /// Sets the interrupt handler + #[cfg_attr( + not(multi_core), + doc = "Registers an interrupt handler for the peripheral." + )] + #[cfg_attr( + multi_core, + doc = "Registers an interrupt handler for the peripheral on the current core." + )] + #[doc = ""] + /// Note that this will replace any previously registered interrupt + /// handlers. /// - /// Interrupts are not enabled at the peripheral level here. + /// You can restore the default/unhandled interrupt handler by using + /// [crate::interrupt::DEFAULT_INTERRUPT_HANDLER] + #[instability::unstable] fn set_interrupt_handler(&mut self, handler: InterruptHandler) { let interrupt = self.driver().info.interrupt; for core in Cpu::other() { @@ -599,6 +609,15 @@ impl InterruptConfigurable for Spi<'_, Blocking> { } } +impl InterruptConfigurable for Spi<'_, Blocking> { + /// Sets the interrupt handler + /// + /// Interrupts are not enabled at the peripheral level here. + fn set_interrupt_handler(&mut self, handler: InterruptHandler) { + Spi::set_interrupt_handler(self, handler); + } +} + impl<'d> Spi<'d, Async> { /// Converts the SPI instance into blocking mode. pub fn into_blocking(self) -> Spi<'d, Blocking> { diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index deeb3f8eeb9..d7f223812d5 100644 --- a/esp-hal/src/uart.rs +++ b/esp-hal/src/uart.rs @@ -1276,13 +1276,33 @@ where impl crate::private::Sealed for Uart<'_, Blocking> {} impl InterruptConfigurable for Uart<'_, Blocking> { - fn set_interrupt_handler(&mut self, handler: crate::interrupt::InterruptHandler) { + fn set_interrupt_handler(&mut self, handler: InterruptHandler) { // `self.tx.uart` and `self.rx.uart` are the same self.tx.uart.info().set_interrupt_handler(handler); } } impl Uart<'_, Blocking> { + #[cfg_attr( + not(multi_core), + doc = "Registers an interrupt handler for the peripheral." + )] + #[cfg_attr( + multi_core, + doc = "Registers an interrupt handler for the peripheral on the current core." + )] + #[doc = ""] + /// Note that this will replace any previously registered interrupt + /// handlers. + /// + /// You can restore the default/unhandled interrupt handler by using + /// [crate::interrupt::DEFAULT_INTERRUPT_HANDLER] + #[instability::unstable] + pub fn set_interrupt_handler(&mut self, handler: InterruptHandler) { + // `self.tx.uart` and `self.rx.uart` are the same + self.tx.uart.info().set_interrupt_handler(handler); + } + /// Listen for the given interrupts #[instability::unstable] pub fn listen(&mut self, interrupts: impl Into>) { From b34de84450f25645d2f6ebf6a946d73d6b8d9fb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Thu, 16 Jan 2025 13:30:21 +0100 Subject: [PATCH 2/2] Mark Async drivers !Send --- esp-hal/CHANGELOG.md | 2 ++ esp-hal/MIGRATING-0.23.md | 23 ++++++++++++++++++++- esp-hal/src/lib.rs | 6 +++++- hil-test/tests/embassy_interrupt_spi_dma.rs | 20 +++++++----------- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index b8ac2e40692..d108a1a57d4 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Rng` and `Trng` now implement `Peripheral

` (#2992) +- `Async` drivers are no longer `Send` (#2980) + ### Fixed - `DmaDescriptor` is now `#[repr(C)]` (#2988) diff --git a/esp-hal/MIGRATING-0.23.md b/esp-hal/MIGRATING-0.23.md index c57ff7193a7..665092650b4 100644 --- a/esp-hal/MIGRATING-0.23.md +++ b/esp-hal/MIGRATING-0.23.md @@ -1,4 +1,25 @@ -# Migration Guide from v0.23.x to v?.??.? +# Migration Guide from v0.23.x to v1.0.0-beta.0 + +## `Async` drivers can no longer be sent between cores and executors + +To work around this limitation, send the blocking driver, and configure it into `Async` mode +in the target context. + +```diff + #[embassy_executor::task] +-async fn interrupt_driven_task(mut spi: Spi<'static, Async>) { ++async fn interrupt_driven_task(spi: Spi<'static, Blocking>) { ++ let mut spi = spi.into_async(); + ... + } + + let spi = Spi::new(...) + .unwrap() + // ... +- .into_async(); + + send_spawner.spawn(interrupt_driven_task(spi)).unwrap(); +``` ## RMT changes diff --git a/esp-hal/src/lib.rs b/esp-hal/src/lib.rs index 89640e0bba3..f6a44e3bce5 100644 --- a/esp-hal/src/lib.rs +++ b/esp-hal/src/lib.rs @@ -143,6 +143,8 @@ // MUST be the first module mod fmt; +use core::marker::PhantomData; + #[cfg(riscv)] #[cfg_attr(docsrs, doc(cfg(feature = "unstable")))] #[cfg_attr(not(feature = "unstable"), doc(hidden))] @@ -321,7 +323,9 @@ pub struct Blocking; /// Driver initialized in async mode. #[derive(Debug)] -pub struct Async; +pub struct Async(PhantomData<*const ()>); + +unsafe impl Sync for Async {} impl crate::DriverMode for Blocking {} impl crate::DriverMode for Async {} diff --git a/hil-test/tests/embassy_interrupt_spi_dma.rs b/hil-test/tests/embassy_interrupt_spi_dma.rs index e6505fe94bc..a751fe5581f 100644 --- a/hil-test/tests/embassy_interrupt_spi_dma.rs +++ b/hil-test/tests/embassy_interrupt_spi_dma.rs @@ -18,7 +18,7 @@ use esp_hal::{ }, time::RateExtU32, timer::AnyTimer, - Async, + Blocking, }; use esp_hal_embassy::InterruptExecutor; use hil_test as _; @@ -37,14 +37,14 @@ static INTERRUPT_TASK_WORKING: AtomicBool = AtomicBool::new(false); #[cfg(any(esp32, esp32s2, esp32s3))] #[embassy_executor::task] -async fn interrupt_driven_task(spi: esp_hal::spi::master::SpiDma<'static, Async>) { +async fn interrupt_driven_task(spi: esp_hal::spi::master::SpiDma<'static, Blocking>) { let mut ticker = Ticker::every(Duration::from_millis(1)); let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(128); let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); let dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap(); - let mut spi = spi.with_buffers(dma_rx_buf, dma_tx_buf); + let mut spi = spi.with_buffers(dma_rx_buf, dma_tx_buf).into_async(); loop { let mut buffer: [u8; 8] = [0; 8]; @@ -59,9 +59,11 @@ async fn interrupt_driven_task(spi: esp_hal::spi::master::SpiDma<'static, Async> #[cfg(not(any(esp32, esp32s2, esp32s3)))] #[embassy_executor::task] -async fn interrupt_driven_task(mut i2s_tx: esp_hal::i2s::master::I2sTx<'static, Async>) { +async fn interrupt_driven_task(i2s_tx: esp_hal::i2s::master::I2s<'static, Blocking>) { let mut ticker = Ticker::every(Duration::from_millis(1)); + let mut i2s_tx = i2s_tx.into_async().i2s_tx.build(); + loop { let mut buffer: [u8; 8] = [0; 8]; @@ -137,14 +139,13 @@ mod test { .with_mode(Mode::_0), ) .unwrap() - .with_dma(dma_channel2) - .into_async(); + .with_dma(dma_channel2); #[cfg(not(any(esp32, esp32s2, esp32s3)))] let other_peripheral = { let (_, rx_descriptors, _, tx_descriptors) = dma_buffers!(128); - let other_peripheral = esp_hal::i2s::master::I2s::new( + esp_hal::i2s::master::I2s::new( peripherals.I2S0, esp_hal::i2s::master::Standard::Philips, esp_hal::i2s::master::DataFormat::Data8Channel8, @@ -153,11 +154,6 @@ mod test { rx_descriptors, tx_descriptors, ) - .into_async() - .i2s_tx - .build(); - - other_peripheral }; let sw_ints = SoftwareInterruptControl::new(peripherals.SW_INTERRUPT);