Fix SPI async busy wait

examples/hpm5300evk/src/bin/async_spi_bug.rs

@@ -0,0 +1,50 @@
+#![no_main]
+#![no_std]
+#![feature(type_alias_impl_trait)]
+#![feature(impl_trait_in_assoc_type)]
+#![feature(abi_riscv_interrupt)]
+
+use embassy_time::{Duration, Timer};
+use hal::gpio::{Level, Output};
+use hal::peripherals;
+use hpm_hal::bind_interrupts;
+use hpm_hal::time::Hertz;
+use {defmt_rtt as _, hpm_hal as hal};
+
+bind_interrupts!(struct Irqs {
+    SPI1 => hal::spi::InterruptHandler<peripherals::SPI1>;
+});
+
+#[embassy_executor::main(entry = "hpm_hal::entry")]
+async fn main(_spawner: embassy_executor::Spawner) -> ! {
+    let config = hal::Config::default();
+    let p = hal::init(config);
+
+    let mut config = hal::spi::Config::default();
+    config.frequency = Hertz::mhz(2);
+    let mut spi = hal::spi::Spi::new_txonly(p.SPI1, p.PA27, p.PA29, Irqs, p.HDMA_CH0, config);
+
+    defmt::info!("go !");
+
+    spi.write(&[0xaa_u8; 1]).await.unwrap(); // lower values fail. larger(10 or so) values work
+
+    // The following lines are never reached
+
+    defmt::println!("bytes sent");
+
+    let mut led = Output::new(p.PA23, Level::Low, Default::default());
+    loop {
+        led.set_high();
+        Timer::after(Duration::from_millis(500)).await;
+        led.set_low();
+        Timer::after(Duration::from_millis(500)).await;
+        defmt::println!("tick");
+    }
+}
+
+#[panic_handler]
+unsafe fn panic(info: &core::panic::PanicInfo) -> ! {
+    defmt::println!("panic!\n {}", defmt::Debug2Format(info));
+
+    loop {}
+}

src/dma/v2.rs

@@ -40,7 +40,7 @@ pub struct TransferOptions {
 impl Default for TransferOptions {
     fn default() -> Self {
         Self {
-            burst: Burst::Liner(0),
+            burst: Burst::Exponential(0), // 1 transfer
             priority: false,
             circular: false,
             half_transfer_irq: false,
@@ -53,8 +53,6 @@ impl Default for TransferOptions {
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub enum Burst {
-    // 0:1transfer; 0xf: 16 transfer
-    Liner(u8),
     /*
     0x0: 1 transfer
     0x1: 2 transfers
@@ -69,6 +67,9 @@ pub enum Burst {
     0xa: 1024 transfers
     */
     Exponential(u8),
+    // For BURSTOPT = 1
+    // 0:1transfer; 0xf: 16 transfer
+    Liner(u8),
 }
 
 impl Burst {
@@ -273,10 +274,12 @@ impl AnyChannel {
             w.set_infiniteloop(options.circular);
             // false: Use burst mode
             // true:  Send all data at once
-            w.set_handshakeopt(false);
+            w.set_handshakeopt(false); // always false in sdk
 
             w.set_burstopt(options.burst.burstopt());
             w.set_priority(options.priority);
+
+            // In DMA handshake case, source burst size must be 1 transfer, that is 0
             w.set_srcburstsize(options.burst.burstsize());
             w.set_srcwidth(src_width.width());
             w.set_dstwidth(dst_width.width());

src/spi/mod.rs

@@ -5,22 +5,27 @@
 //! - 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;
+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
 pub use embedded_hal::spi::{Mode, MODE_0, MODE_1, MODE_2, MODE_3};
+use futures_util::future::{select, Either};
 
 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 {
@@ -33,7 +38,32 @@ mod consts {
     pub const FIFO_SIZE: usize = 4;
 }
 
-// NOTE: SPI end interrupt is not working under DMA mode
+// - MARK: interrupt handler
+
+/// Interrupt handler.
+pub struct InterruptHandler<T: Instance> {
+    _phantom: PhantomData<T>,
+}
+
+impl<T: Instance> interrupt::typelevel::Handler<T::Interrupt> for InterruptHandler<T> {
+    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();
+
+    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
 
@@ -378,6 +408,7 @@ impl<'d> Spi<'d, Async> {
         sclk: impl Peripheral<P = impl SclkPin<T>> + 'd,
         mosi: impl Peripheral<P = impl MosiPin<T>> + 'd,
         miso: impl Peripheral<P = impl MisoPin<T>> + 'd,
+        _irq: impl interrupt::typelevel::Binding<T::Interrupt, InterruptHandler<T>> + 'd,
         tx_dma: impl Peripheral<P = impl TxDma<T>> + 'd,
         rx_dma: impl Peripheral<P = impl RxDma<T>> + 'd,
         config: Config,
@@ -410,6 +441,7 @@ impl<'d> Spi<'d, Async> {
         peri: impl Peripheral<P = T> + 'd,
         sclk: impl Peripheral<P = impl SclkPin<T>> + 'd,
         miso: impl Peripheral<P = impl MisoPin<T>> + 'd,
+        _irq: impl interrupt::typelevel::Binding<T::Interrupt, InterruptHandler<T>> + 'd,
         rx_dma: impl Peripheral<P = impl RxDma<T>> + 'd,
         config: Config,
     ) -> Self {
@@ -441,6 +473,7 @@ impl<'d> Spi<'d, Async> {
         peri: impl Peripheral<P = T> + 'd,
         sclk: impl Peripheral<P = impl SclkPin<T>> + 'd,
         mosi: impl Peripheral<P = impl MosiPin<T>> + 'd,
+        _irq: impl interrupt::typelevel::Binding<T::Interrupt, InterruptHandler<T>> + 'd,
         tx_dma: impl Peripheral<P = impl TxDma<T>> + 'd,
         config: Config,
     ) -> Self {
@@ -500,27 +533,41 @@ impl<'d> Spi<'d, Async> {
         }
 
         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;
-        let mut opts = dma::TransferOptions::default();
-        opts.burst = dma::Burst::from_size(FIFO_SIZE / 2);
-        let tx_f = unsafe { self.tx_dma.as_mut().unwrap().write(data, tx_dst, opts) };
+        let tx_f = unsafe {
+            self.tx_dma
+                .as_mut()
+                .unwrap()
+                .write(data, tx_dst, dma::TransferOptions::default())
+        };
 
-        tx_f.await;
+        let end_f = poll_fn(move |cx| {
+            s.waker.register(cx.waker());
+            if r.intr_en().read().endinten() {
+                return Poll::Pending;
+            } else {
+                return Poll::Ready(());
+            }
+        });
 
-        r.ctrl().modify(|w| w.set_txdmaen(false));
+        end_f.await;
 
-        // 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;
-        }
+        compiler_fence(core::sync::atomic::Ordering::SeqCst);
+
+        r.ctrl().modify(|w| w.set_txdmaen(false));
+        drop(tx_f);
 
         Ok(())
     }
@@ -584,11 +631,6 @@ impl<'d> Spi<'d, Async> {
             w.set_txdmaen(false);
         });
 
-        // See `write`
-        while r.status().read().spiactive() {
-            yield_now().await;
-        }
-
         Ok(())
     }
 
@@ -605,8 +647,10 @@ impl<'d> Spi<'d, Async> {
     /// In-place bidirectional transfer, using DMA.
     ///
     /// This writes the contents of `data` on MOSI, and puts the received data on MISO in `data`, at the same time.
-    pub async fn transfer_in_place<W: Word>(&mut self, data: &mut [W], config: &TransferConfig) -> Result<(), Error> {
-        self.transfer_inner(data, data, config).await
+    pub async fn transfer_in_place<W: Word>(&mut self, data: &mut [W]) -> Result<(), Error> {
+        let mut config = TransferConfig::default();
+        config.transfer_mode = TransMode::WRITE_READ_TOGETHER;
+        self.transfer_inner(data, data, &config).await
     }
 }
 
@@ -639,6 +683,11 @@ impl<'d, M: PeriMode> Spi<'d, M> {
 
         this.enable_and_configure(&config).unwrap();
 
+        T::Interrupt::set_priority(interrupt::Priority::P1);
+        unsafe {
+            T::Interrupt::enable();
+        }
+
         this
     }
 
@@ -727,6 +776,10 @@ impl<'d, M: PeriMode> Spi<'d, M> {
             return Err(Error::InvalidArgument);
         }
 
+        if config.transfer_mode == TransMode::WRITE_READ_TOGETHER && write_len != read_len {
+            return Err(Error::InvalidArgument);
+        }
+
         let r = self.info.regs;
 
         // SPI format init
@@ -745,18 +798,24 @@ impl<'d, M: PeriMode> Spi<'d, M> {
             w.set_addrfmt(config.addr_phase);
             w.set_dualquad(config.data_phase);
             w.set_tokenen(false);
-            #[cfg(not(ip_feature_spi_new_trans_count))]
+            // #[cfg(not(ip_feature_spi_new_trans_count))]
             match config.transfer_mode {
                 TransMode::WRITE_READ_TOGETHER
                 | TransMode::READ_DUMMY_WRITE
                 | TransMode::WRITE_DUMMY_READ
                 | TransMode::READ_WRITE
                 | TransMode::WRITE_READ => {
+                    w.set_wrtrancnt((write_len as u16 - 1) & 0x1ff);
+                    w.set_rdtrancnt((read_len as u16 - 1) & 0x1ff);
+                }
+                TransMode::WRITE_ONLY | TransMode::DUMMY_WRITE => {
                     w.set_wrtrancnt(write_len as u16 - 1);
+                    w.set_rdtrancnt(0x1ff);
+                }
+                TransMode::READ_ONLY | TransMode::DUMMY_READ => {
                     w.set_rdtrancnt(read_len as u16 - 1);
+                    w.set_wrtrancnt(0x1ff);
                 }
-                TransMode::WRITE_ONLY | TransMode::DUMMY_WRITE => w.set_wrtrancnt(write_len as u16 - 1),
-                TransMode::READ_ONLY | TransMode::DUMMY_READ => w.set_rdtrancnt(read_len as u16 - 1),
                 TransMode::NO_DATA => (),
                 _ => (),
             }
@@ -1152,8 +1211,6 @@ impl<'d, W: Word> embedded_hal_async::spi::SpiBus<W> for Spi<'d, Async> {
     }
 
     async fn transfer_in_place(&mut self, words: &mut [W]) -> Result<(), Self::Error> {
-        let mut options = TransferConfig::default();
-        options.transfer_mode = TransMode::WRITE_READ_TOGETHER;
-        self.transfer_in_place(words, &options).await
+        self.transfer_in_place(words).await
     }
 }