From 306110f56e4614cc51f6c3d3e9ff96b5fe2ced6f Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 15 Mar 2022 00:31:36 +0100 Subject: [PATCH 1/5] stm32/spi: implement async trasnfer_in_place --- embassy-stm32/src/spi/mod.rs | 35 ++++++++++++++++++++++++++-------- tests/stm32/src/bin/spi_dma.rs | 3 +++ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/embassy-stm32/src/spi/mod.rs b/embassy-stm32/src/spi/mod.rs index 5271d941c..7fa30cb61 100644 --- a/embassy-stm32/src/spi/mod.rs +++ b/embassy-stm32/src/spi/mod.rs @@ -7,7 +7,7 @@ use embassy_hal_common::unborrow; use futures::future::join; use self::sealed::WordSize; -use crate::dma::{NoDma, Transfer}; +use crate::dma::{slice_ptr_parts, NoDma, Transfer}; use crate::gpio::sealed::{AFType, Pin as _}; use crate::gpio::AnyPin; use crate::pac::spi::Spi as Regs; @@ -501,14 +501,19 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> { Ok(()) } - pub async fn transfer(&mut self, read: &mut [W], write: &[W]) -> Result<(), Error> + async fn transfer_inner( + &mut self, + read: *mut [W], + write: *const [W], + ) -> Result<(), Error> where Tx: TxDma, Rx: RxDma, { - assert_eq!(read.len(), write.len()); - - if read.len() == 0 { + let (_, rx_len) = slice_ptr_parts(read); + let (_, tx_len) = slice_ptr_parts(write); + assert_eq!(rx_len, tx_len); + if rx_len == 0 { return Ok(()); } @@ -552,6 +557,22 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> { Ok(()) } + pub async fn transfer(&mut self, read: &mut [W], write: &[W]) -> Result<(), Error> + where + Tx: TxDma, + Rx: RxDma, + { + self.transfer_inner(read, write).await + } + + pub async fn transfer_in_place(&mut self, data: &mut [W]) -> Result<(), Error> + where + Tx: TxDma, + Rx: RxDma, + { + self.transfer_inner(data, data).await + } + pub fn blocking_write(&mut self, words: &[W]) -> Result<(), Error> { self.set_word_size(W::WORDSIZE); for word in words.iter() { @@ -935,9 +956,7 @@ cfg_if::cfg_if! { &'a mut self, words: &'a mut [W], ) -> Self::TransferInPlaceFuture<'a> { - // TODO: Implement async version - let result = self.blocking_transfer_in_place(words); - async move { result } + self.transfer_in_place(words) } } } diff --git a/tests/stm32/src/bin/spi_dma.rs b/tests/stm32/src/bin/spi_dma.rs index 59a5bcd0c..ce80bde74 100644 --- a/tests/stm32/src/bin/spi_dma.rs +++ b/tests/stm32/src/bin/spi_dma.rs @@ -47,6 +47,9 @@ async fn main(_spawner: Spawner, p: Peripherals) { spi.transfer(&mut buf, &data).await.unwrap(); assert_eq!(buf, data); + spi.transfer_in_place(&mut buf).await.unwrap(); + assert_eq!(buf, data); + info!("Test OK"); cortex_m::asm::bkpt(); } From 06f35c25176795c8e856ea7b41a52dba4b160f66 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 15 Mar 2022 00:46:18 +0100 Subject: [PATCH 2/5] stm32/spi: more exhaustive test. --- tests/stm32/src/bin/spi.rs | 15 +++++++++++++++ tests/stm32/src/bin/spi_dma.rs | 12 ++++++++++++ 2 files changed, 27 insertions(+) diff --git a/tests/stm32/src/bin/spi.rs b/tests/stm32/src/bin/spi.rs index 47d0017ac..6151058b7 100644 --- a/tests/stm32/src/bin/spi.rs +++ b/tests/stm32/src/bin/spi.rs @@ -37,9 +37,24 @@ async fn main(_spawner: Spawner, p: Peripherals) { // Arduino pins D11 and D12 (MOSI-MISO) are connected together with a 1K resistor. // so we should get the data we sent back. let mut buf = data; + spi.blocking_transfer(&mut buf, &data).unwrap(); + assert_eq!(buf, data); + spi.blocking_transfer_in_place(&mut buf).unwrap(); assert_eq!(buf, data); + // Check read/write don't hang. We can't check they transfer the right data + // without fancier test mechanisms. + spi.blocking_write(&buf).unwrap(); + spi.blocking_read(&mut buf).unwrap(); + spi.blocking_write(&buf).unwrap(); + spi.blocking_read(&mut buf).unwrap(); + spi.blocking_write(&buf).unwrap(); + + // Check transfer doesn't break after having done a write, due to garbage in the FIFO + spi.blocking_transfer(&mut buf, &data).unwrap(); + assert_eq!(buf, data); + info!("Test OK"); cortex_m::asm::bkpt(); } diff --git a/tests/stm32/src/bin/spi_dma.rs b/tests/stm32/src/bin/spi_dma.rs index ce80bde74..67785778a 100644 --- a/tests/stm32/src/bin/spi_dma.rs +++ b/tests/stm32/src/bin/spi_dma.rs @@ -50,6 +50,18 @@ async fn main(_spawner: Spawner, p: Peripherals) { spi.transfer_in_place(&mut buf).await.unwrap(); assert_eq!(buf, data); + // Check read/write don't hang. We can't check they transfer the right data + // without fancier test mechanisms. + spi.write(&buf).await.unwrap(); + spi.read(&mut buf).await.unwrap(); + spi.write(&buf).await.unwrap(); + spi.read(&mut buf).await.unwrap(); + spi.write(&buf).await.unwrap(); + + // Check transfer doesn't break after having done a write, due to garbage in the FIFO + spi.transfer(&mut buf, &data).await.unwrap(); + assert_eq!(buf, data); + info!("Test OK"); cortex_m::asm::bkpt(); } From 3d6592d22d37402509bd43d88a0a979683f74b04 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 15 Mar 2022 00:48:11 +0100 Subject: [PATCH 3/5] stm32/spi: check zero-length trasnfers. --- tests/stm32/src/bin/spi.rs | 6 ++++++ tests/stm32/src/bin/spi_dma.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/stm32/src/bin/spi.rs b/tests/stm32/src/bin/spi.rs index 6151058b7..b079472d5 100644 --- a/tests/stm32/src/bin/spi.rs +++ b/tests/stm32/src/bin/spi.rs @@ -55,6 +55,12 @@ async fn main(_spawner: Spawner, p: Peripherals) { spi.blocking_transfer(&mut buf, &data).unwrap(); assert_eq!(buf, data); + // Check zero-length operations, these should be noops. + spi.blocking_transfer::(&mut [], &[]).unwrap(); + spi.blocking_transfer_in_place::(&mut []).unwrap(); + spi.blocking_read::(&mut []).unwrap(); + spi.blocking_write::(&[]).unwrap(); + info!("Test OK"); cortex_m::asm::bkpt(); } diff --git a/tests/stm32/src/bin/spi_dma.rs b/tests/stm32/src/bin/spi_dma.rs index 67785778a..3e9521ae7 100644 --- a/tests/stm32/src/bin/spi_dma.rs +++ b/tests/stm32/src/bin/spi_dma.rs @@ -62,6 +62,12 @@ async fn main(_spawner: Spawner, p: Peripherals) { spi.transfer(&mut buf, &data).await.unwrap(); assert_eq!(buf, data); + // Check zero-length operations, these should be noops. + spi.transfer::(&mut [], &[]).await.unwrap(); + spi.transfer_in_place::(&mut []).await.unwrap(); + spi.read::(&mut []).await.unwrap(); + spi.write::(&[]).await.unwrap(); + info!("Test OK"); cortex_m::asm::bkpt(); } From 4579192832e09efaa657fea1fc975d0548499d2a Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 15 Mar 2022 02:36:34 +0100 Subject: [PATCH 4/5] stm32/spi: fix hang in SPIv3 by not waiting for rxfifo empty in finish_dma. --- embassy-stm32/src/spi/mod.rs | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/embassy-stm32/src/spi/mod.rs b/embassy-stm32/src/spi/mod.rs index 7fa30cb61..1806fb369 100644 --- a/embassy-stm32/src/spi/mod.rs +++ b/embassy-stm32/src/spi/mod.rs @@ -440,9 +440,6 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> { tx_f.await; - // flush here otherwise `finish_dma` hangs waiting for the rx fifo to empty - flush_rx_fifo(T::REGS); - finish_dma(T::REGS); Ok(()) @@ -726,26 +723,6 @@ fn spin_until_rx_ready(regs: Regs) -> Result<(), Error> { } } -fn spin_until_idle(regs: Regs) { - #[cfg(any(spi_v1, spi_f1))] - unsafe { - while regs.sr().read().bsy() {} - } - - #[cfg(spi_v2)] - unsafe { - while regs.sr().read().ftlvl() > 0 {} - while regs.sr().read().frlvl() > 0 {} - while regs.sr().read().bsy() {} - } - - #[cfg(spi_v3)] - unsafe { - while !regs.sr().read().txc() {} - while regs.sr().read().rxplvl().0 > 0 {} - } -} - fn flush_rx_fifo(regs: Regs) { unsafe { #[cfg(not(spi_v3))] @@ -786,9 +763,15 @@ fn set_rxdmaen(regs: Regs, val: bool) { } fn finish_dma(regs: Regs) { - spin_until_idle(regs); - unsafe { + #[cfg(spi_v2)] + while regs.sr().read().ftlvl() > 0 {} + + #[cfg(spi_v3)] + while !regs.sr().read().txc() {} + #[cfg(not(spi_v3))] + while regs.sr().read().bsy() {} + regs.cr1().modify(|w| { w.set_spe(false); }); From 059b16423458a80c5cb4e1630260d6c564a88e84 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 15 Mar 2022 02:37:08 +0100 Subject: [PATCH 5/5] stm32/spi: do not clear rxfifo in SPIv3, the hw already does it. --- embassy-stm32/src/spi/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/embassy-stm32/src/spi/mod.rs b/embassy-stm32/src/spi/mod.rs index 1806fb369..d8ffabb11 100644 --- a/embassy-stm32/src/spi/mod.rs +++ b/embassy-stm32/src/spi/mod.rs @@ -462,6 +462,10 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> { set_rxdmaen(T::REGS, true); } + // SPIv3 clears rxfifo on SPE=0 + #[cfg(not(spi_v3))] + flush_rx_fifo(T::REGS); + let clock_byte_count = data.len(); let rx_request = self.rxdma.request(); @@ -522,8 +526,8 @@ impl<'d, T: Instance, Tx, Rx> Spi<'d, T, Tx, Rx> { set_rxdmaen(T::REGS, true); } - // TODO: This is unnecessary in some versions because - // clearing SPE automatically clears the fifos + // SPIv3 clears rxfifo on SPE=0 + #[cfg(not(spi_v3))] flush_rx_fifo(T::REGS); let rx_request = self.rxdma.request(); @@ -723,6 +727,7 @@ fn spin_until_rx_ready(regs: Regs) -> Result<(), Error> { } } +#[cfg(not(spi_v3))] fn flush_rx_fifo(regs: Regs) { unsafe { #[cfg(not(spi_v3))]