From 7864875b9908a2adac5f56f14cfd607f0f2caa98 Mon Sep 17 00:00:00 2001 From: Andelf Date: Sat, 28 Sep 2024 08:37:23 +0800 Subject: [PATCH 1/3] refactor(uart): refine fn arg order - Irqs always come after pins --- src/uart/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uart/mod.rs b/src/uart/mod.rs index cdd988f..49beb8c 100644 --- a/src/uart/mod.rs +++ b/src/uart/mod.rs @@ -495,9 +495,9 @@ impl<'d> UartRx<'d, Async> { /// Create a new rx-only UART with a request-to-send pin pub fn new_with_rts( peri: impl Peripheral

+ 'd, - _irq: impl interrupt::typelevel::Binding> + 'd, rx: impl Peripheral

> + 'd, rts: impl Peripheral

> + 'd, + _irq: impl interrupt::typelevel::Binding> + 'd, rx_dma: impl Peripheral

> + 'd, config: Config, ) -> Result { @@ -837,9 +837,9 @@ impl<'d> Uart<'d, Async> { peri: impl Peripheral

+ 'd, rx: impl Peripheral

> + 'd, tx: impl Peripheral

> + 'd, - _irq: impl interrupt::typelevel::Binding> + 'd, rts: impl Peripheral

> + 'd, cts: impl Peripheral

> + 'd, + _irq: impl interrupt::typelevel::Binding> + 'd, tx_dma: impl Peripheral

> + 'd, rx_dma: impl Peripheral

> + 'd, config: Config, @@ -871,8 +871,8 @@ impl<'d> Uart<'d, Async> { peri: impl Peripheral

+ 'd, rx: impl Peripheral

> + 'd, tx: impl Peripheral

> + 'd, - _irq: impl interrupt::typelevel::Binding> + 'd, de: impl Peripheral

> + 'd, + _irq: impl interrupt::typelevel::Binding> + 'd, tx_dma: impl Peripheral

> + 'd, rx_dma: impl Peripheral

> + 'd, config: Config, From 1426f813e2342e997f4144b8d6322b1cdd118dbe Mon Sep 17 00:00:00 2001 From: Andelf Date: Sat, 28 Sep 2024 08:38:19 +0800 Subject: [PATCH 2/3] wip(spi): add async irq done wait - Not working, it seems SPI finish irq is never triggered under DMA mode --- src/spi/mod.rs | 116 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 35 deletions(-) diff --git a/src/spi/mod.rs b/src/spi/mod.rs index ed2a618..7a8f2cf 100644 --- a/src/spi/mod.rs +++ b/src/spi/mod.rs @@ -5,21 +5,26 @@ //! - SPI_CS_SELECT: v53, v68, //! - SPI_SUPPORT_DIRECTIO: v53, v68 +use core::future::poll_fn; use core::marker::PhantomData; use core::ptr; +use core::sync::atomic::{compiler_fence, Ordering}; +use core::task::Poll; use embassy_futures::join::join; use embassy_hal_internal::{into_ref, Peripheral, PeripheralRef}; use embassy_sync::waitqueue::AtomicWaker; // re-export pub use embedded_hal::spi::{Mode, MODE_0, MODE_1, MODE_2, MODE_3}; -pub use hpm_metapac::spi::vals::{AddrLen, AddrPhaseFormat, DataPhaseFormat, TransMode}; use self::consts::*; use crate::dma::{self, word, ChannelAndRequest}; use crate::gpio::AnyPin; +use crate::interrupt::typelevel::Interrupt as _; use crate::mode::{Async, Blocking, Mode as PeriMode}; +pub use crate::pac::spi::vals::{AddrLen, AddrPhaseFormat, DataPhaseFormat, TransMode}; use crate::time::Hertz; +use crate::{interrupt, pac}; #[cfg(any(hpm53, hpm68, hpm6e))] mod consts { @@ -32,8 +37,36 @@ mod consts { pub const FIFO_SIZE: usize = 4; } -// ========== -// Helper enums +// - MARK: interrupt handler + +/// Interrupt handler. +pub struct InterruptHandler { + _phantom: PhantomData, +} + +impl interrupt::typelevel::Handler for InterruptHandler { + unsafe fn on_interrupt() { + on_interrupt(T::info().regs, T::state()); + + // PLIC ack is handled by typelevel Handler + } +} + +unsafe fn on_interrupt(r: pac::spi::Spi, s: &'static State) { + let status = r.intr_st().read(); + + defmt::info!("in irq"); + + if status.endint() { + s.waker.wake(); + + r.intr_en().modify(|w| w.set_endinten(false)); + } + + r.intr_st().write_value(status); // W1C +} + +// - MARK: Helper enums /// Time between CS active and SCLK edge. #[derive(Copy, Clone)] @@ -100,8 +133,7 @@ impl Into for CsHighTime { } } -// ========== -// Config +// - MARK: Config #[derive(Clone, Copy)] pub struct Timings { @@ -200,8 +232,7 @@ pub enum Error { FifoFull, } -// ========== -// SPI driver +// - MARK: SPI driver /// SPI driver. #[allow(unused)] @@ -351,9 +382,11 @@ impl<'d> Spi<'d, Blocking> { d2.set_as_alt(d2.alt_num()); d3.set_as_alt(d3.alt_num()); - let cs_index = cs.cs_index(); #[cfg(ip_feature_spi_cs_select)] - T::info().regs.ctrl().modify(|w| w.set_cs_en(cs_index)); + { + let cs_index = cs.cs_index(); + T::info().regs.ctrl().modify(|w| w.set_cs_en(cs_index)); + } Self::new_inner( peri, @@ -376,6 +409,7 @@ impl<'d> Spi<'d, Async> { sclk: impl Peripheral

> + 'd, mosi: impl Peripheral

> + 'd, miso: impl Peripheral

> + 'd, + _irq: impl interrupt::typelevel::Binding> + 'd, tx_dma: impl Peripheral

> + 'd, rx_dma: impl Peripheral

> + 'd, config: Config, @@ -408,6 +442,7 @@ impl<'d> Spi<'d, Async> { peri: impl Peripheral

+ 'd, sclk: impl Peripheral

> + 'd, miso: impl Peripheral

> + 'd, + _irq: impl interrupt::typelevel::Binding> + 'd, rx_dma: impl Peripheral

> + 'd, config: Config, ) -> Self { @@ -439,6 +474,7 @@ impl<'d> Spi<'d, Async> { peri: impl Peripheral

+ 'd, sclk: impl Peripheral

> + 'd, mosi: impl Peripheral

> + 'd, + _irq: impl interrupt::typelevel::Binding> + 'd, tx_dma: impl Peripheral

> + 'd, config: Config, ) -> Self { @@ -497,11 +533,19 @@ impl<'d> Spi<'d, Async> { return Ok(()); } + defmt::info!("write spi"); + let r = self.info.regs; + let s = self.state; self.set_word_size(W::CONFIG); + self.configure_transfer(data.len(), 0, &TransferConfig::default())?; + r.intr_en().modify(|w| { + w.set_endinten(true); + }); + r.ctrl().modify(|w| w.set_txdmaen(true)); let tx_dst = r.data().as_ptr() as *mut W; @@ -511,10 +555,17 @@ impl<'d> Spi<'d, Async> { tx_f.await; - r.ctrl().modify(|w| w.set_txdmaen(false)); + poll_fn(move |cx| { + if r.intr_en().read().endinten() { + return Poll::Pending; + } else { + s.waker.register(cx.waker()); + return Poll::Ready(()); + } + }) + .await; - // TODO: should wait tx done via INTRST - // In embassy-stm32 this is a busy wait + r.ctrl().modify(|w| w.set_txdmaen(false)); Ok(()) } @@ -627,6 +678,11 @@ impl<'d, M: PeriMode> Spi<'d, M> { }; this.enable_and_configure(&config).unwrap(); + + unsafe { + T::Interrupt::enable(); + } + this } @@ -797,6 +853,10 @@ impl<'d, M: PeriMode> Spi<'d, M> { // In blocking mode, the final speed is not faster than normal mode pub fn blocking_datamerge_write(&mut self, data: &[u8], config: &TransferConfig) -> Result<(), Error> { + if data.is_empty() { + return Ok(()); + } + let r = self.info.regs; flush_rx_fifo(r); @@ -831,6 +891,10 @@ impl<'d, M: PeriMode> Spi<'d, M> { // Write in master mode pub fn blocking_write(&mut self, data: &[W]) -> Result<(), Error> { + if data.is_empty() { + return Ok(()); + } + let r = self.info.regs; let config = TransferConfig::default(); @@ -852,6 +916,10 @@ impl<'d, M: PeriMode> Spi<'d, M> { } pub fn blocking_read(&mut self, data: &mut [W]) -> Result<(), Error> { + if data.is_empty() { + return Ok(()); + } + let r = self.info.regs; let mut config = TransferConfig::default(); config.transfer_mode = TransMode::READ_ONLY; @@ -939,7 +1007,7 @@ impl<'d, M: PeriMode> Spi<'d, M> { } // ========== -// Helper functions +// - MARK: Helper types and functions fn flush_rx_fifo(r: crate::pac::spi::Spi) { while !r.status().read().rxempty() { @@ -947,28 +1015,6 @@ fn flush_rx_fifo(r: crate::pac::spi::Spi) { } } -// ========== -// Interrupt handler - -/// SPI Interrupt handler. -pub struct InterruptHandler { - _phantom: PhantomData, -} - -impl crate::interrupt::typelevel::Handler for InterruptHandler { - unsafe fn on_interrupt() { - on_interrupt(T::info().regs, T::state()) - } -} - -unsafe fn on_interrupt(r: crate::pac::spi::Spi, s: &'static State) { - let _ = (r, s); - todo!() -} - -// ========== -// Helper types and functions - struct State { #[allow(unused)] waker: AtomicWaker, From a6e2dfcc627f0bed32c017ae01af9b480cc2b763 Mon Sep 17 00:00:00 2001 From: Andelf Date: Sat, 28 Sep 2024 08:48:48 +0800 Subject: [PATCH 3/3] fix(spi): add busy loop waiting for tx finish See-also #33 --- src/spi/mod.rs | 70 ++++++++++---------------------------------------- 1 file changed, 13 insertions(+), 57 deletions(-) diff --git a/src/spi/mod.rs b/src/spi/mod.rs index 7a8f2cf..80e5f3a 100644 --- a/src/spi/mod.rs +++ b/src/spi/mod.rs @@ -5,13 +5,11 @@ //! - SPI_CS_SELECT: v53, v68, //! - SPI_SUPPORT_DIRECTIO: v53, v68 -use core::future::poll_fn; use core::marker::PhantomData; use core::ptr; -use core::sync::atomic::{compiler_fence, Ordering}; -use core::task::Poll; use embassy_futures::join::join; +use embassy_futures::yield_now; use embassy_hal_internal::{into_ref, Peripheral, PeripheralRef}; use embassy_sync::waitqueue::AtomicWaker; // re-export @@ -20,11 +18,9 @@ pub use embedded_hal::spi::{Mode, MODE_0, MODE_1, MODE_2, MODE_3}; use self::consts::*; use crate::dma::{self, word, ChannelAndRequest}; use crate::gpio::AnyPin; -use crate::interrupt::typelevel::Interrupt as _; use crate::mode::{Async, Blocking, Mode as PeriMode}; pub use crate::pac::spi::vals::{AddrLen, AddrPhaseFormat, DataPhaseFormat, TransMode}; use crate::time::Hertz; -use crate::{interrupt, pac}; #[cfg(any(hpm53, hpm68, hpm6e))] mod consts { @@ -37,34 +33,7 @@ mod consts { pub const FIFO_SIZE: usize = 4; } -// - MARK: interrupt handler - -/// Interrupt handler. -pub struct InterruptHandler { - _phantom: PhantomData, -} - -impl interrupt::typelevel::Handler for InterruptHandler { - unsafe fn on_interrupt() { - on_interrupt(T::info().regs, T::state()); - - // PLIC ack is handled by typelevel Handler - } -} - -unsafe fn on_interrupt(r: pac::spi::Spi, s: &'static State) { - let status = r.intr_st().read(); - - defmt::info!("in irq"); - - if status.endint() { - s.waker.wake(); - - r.intr_en().modify(|w| w.set_endinten(false)); - } - - r.intr_st().write_value(status); // W1C -} +// NOTE: SPI end interrupt is not working under DMA mode // - MARK: Helper enums @@ -409,7 +378,6 @@ impl<'d> Spi<'d, Async> { sclk: impl Peripheral

> + 'd, mosi: impl Peripheral

> + 'd, miso: impl Peripheral

> + 'd, - _irq: impl interrupt::typelevel::Binding> + 'd, tx_dma: impl Peripheral

> + 'd, rx_dma: impl Peripheral

> + 'd, config: Config, @@ -442,7 +410,6 @@ impl<'d> Spi<'d, Async> { peri: impl Peripheral

+ 'd, sclk: impl Peripheral

> + 'd, miso: impl Peripheral

> + 'd, - _irq: impl interrupt::typelevel::Binding> + 'd, rx_dma: impl Peripheral

> + 'd, config: Config, ) -> Self { @@ -474,7 +441,6 @@ impl<'d> Spi<'d, Async> { peri: impl Peripheral

+ 'd, sclk: impl Peripheral

> + 'd, mosi: impl Peripheral

> + 'd, - _irq: impl interrupt::typelevel::Binding> + 'd, tx_dma: impl Peripheral

> + 'd, config: Config, ) -> Self { @@ -533,19 +499,12 @@ impl<'d> Spi<'d, Async> { return Ok(()); } - defmt::info!("write spi"); - let r = self.info.regs; - let s = self.state; self.set_word_size(W::CONFIG); self.configure_transfer(data.len(), 0, &TransferConfig::default())?; - r.intr_en().modify(|w| { - w.set_endinten(true); - }); - r.ctrl().modify(|w| w.set_txdmaen(true)); let tx_dst = r.data().as_ptr() as *mut W; @@ -555,18 +514,14 @@ impl<'d> Spi<'d, Async> { tx_f.await; - poll_fn(move |cx| { - if r.intr_en().read().endinten() { - return Poll::Pending; - } else { - s.waker.register(cx.waker()); - return Poll::Ready(()); - } - }) - .await; - r.ctrl().modify(|w| w.set_txdmaen(false)); + // NOTE: SPI end(finish) interrupt is not working under DMA mode, a busy-loop wait is necessary for TX mode. + // The same goes for `transfer` fn. + while r.status().read().spiactive() { + yield_now().await; + } + Ok(()) } @@ -629,6 +584,11 @@ impl<'d> Spi<'d, Async> { w.set_txdmaen(false); }); + // See `write` + while r.status().read().spiactive() { + yield_now().await; + } + Ok(()) } @@ -679,10 +639,6 @@ impl<'d, M: PeriMode> Spi<'d, M> { this.enable_and_configure(&config).unwrap(); - unsafe { - T::Interrupt::enable(); - } - this }