Merge pull request #3363 from embassy-rs/spi-word-tests

stm32/spi: fix non-u8 word sizes, add tests.

5 files changed, 174 insertions, 130 deletions

diff --git a/embassy-stm32/Cargo.toml b/embassy-stm32/Cargo.toml
index 575c4f20c..8fc8da006 100644
--- a/embassy-stm32/Cargo.toml
+++ b/embassy-stm32/Cargo.toml
@@ -72,7 +72,7 @@ rand_core = "0.6.3"
 sdio-host = "0.5.0"
 critical-section = "1.1"
 #stm32-metapac = { version = "15" }
-stm32-metapac = { git = "https://github.com/embassy-rs/stm32-data-generated", tag = "stm32-data-acaf04256034066bd5b3a8426224ccf3e4cb7d19" }
+stm32-metapac = { git = "https://github.com/embassy-rs/stm32-data-generated", tag = "stm32-data-9b7414490b10ffbd5beb1b0dcf14adb018cbe37f" }
 
 vcell = "0.1.3"
 nb = "1.0.0"
@@ -99,7 +99,7 @@ proc-macro2 = "1.0.36"
 quote = "1.0.15"
 
 #stm32-metapac = { version = "15", default-features = false, features = ["metadata"]}
-stm32-metapac = { git = "https://github.com/embassy-rs/stm32-data-generated", tag = "stm32-data-acaf04256034066bd5b3a8426224ccf3e4cb7d19", default-features = false, features = ["metadata"] }
+stm32-metapac = { git = "https://github.com/embassy-rs/stm32-data-generated", tag = "stm32-data-9b7414490b10ffbd5beb1b0dcf14adb018cbe37f", default-features = false, features = ["metadata"] }
 
 [features]
 default = ["rt"]
diff --git a/embassy-stm32/src/dma/gpdma.rs b/embassy-stm32/src/dma/gpdma.rs
index f9d66ca86..a877bb8d4 100644
--- a/embassy-stm32/src/dma/gpdma.rs
+++ b/embassy-stm32/src/dma/gpdma.rs
@@ -216,7 +216,10 @@ impl<'a> Transfer<'a> {
         data_size: WordSize,
         _options: TransferOptions,
     ) -> Self {
-        assert!(mem_len > 0 && mem_len <= 0xFFFF);
+        // BNDT is specified as bytes, not as number of transfers.
+        let Ok(bndt) = (mem_len * data_size.bytes()).try_into() else {
+            panic!("DMA transfers may not be larger than 65535 bytes.");
+        };
 
         let info = channel.info();
         let ch = info.dma.ch(info.num);
@@ -226,9 +229,6 @@ impl<'a> Transfer<'a> {
 
         let this = Self { channel };
 
-        #[cfg(dmamux)]
-        super::dmamux::configure_dmamux(&*this.channel, request);
-
         ch.cr().write(|w| w.set_reset(true));
         ch.fcr().write(|w| w.0 = 0xFFFF_FFFF); // clear all irqs
         ch.llr().write(|_| {}); // no linked list
@@ -245,10 +245,8 @@ impl<'a> Transfer<'a> {
             });
             w.set_reqsel(request);
         });
-        ch.br1().write(|w| {
-            // BNDT is specified as bytes, not as number of transfers.
-            w.set_bndt((mem_len * data_size.bytes()) as u16)
-        });
+        ch.tr3().write(|_| {}); // no address offsets.
+        ch.br1().write(|w| w.set_bndt(bndt));
 
         match dir {
             Dir::MemoryToPeripheral => {
diff --git a/embassy-stm32/src/spi/mod.rs b/embassy-stm32/src/spi/mod.rs
index dfac4bc89..d034c028e 100644
--- a/embassy-stm32/src/spi/mod.rs
+++ b/embassy-stm32/src/spi/mod.rs
@@ -311,51 +311,29 @@ impl<'d, M: PeriMode> Spi<'d, M> {
         }
     }
 
+    /// Set SPI word size. Disables SPI if needed, you have to enable it back yourself.
     fn set_word_size(&mut self, word_size: word_impl::Config) {
         if self.current_word_size == word_size {
             return;
         }
 
+        self.info.regs.cr1().modify(|w| {
+            w.set_spe(false);
+        });
+
         #[cfg(any(spi_v1, spi_f1))]
-        {
-            self.info.regs.cr1().modify(|reg| {
-                reg.set_spe(false);
-                reg.set_dff(word_size)
-            });
-            self.info.regs.cr1().modify(|reg| {
-                reg.set_spe(true);
-            });
-        }
+        self.info.regs.cr1().modify(|reg| {
+            reg.set_dff(word_size);
+        });
         #[cfg(spi_v2)]
-        {
-            self.info.regs.cr1().modify(|w| {
-                w.set_spe(false);
-            });
-            self.info.regs.cr2().modify(|w| {
-                w.set_frxth(word_size.1);
-                w.set_ds(word_size.0);
-            });
-            self.info.regs.cr1().modify(|w| {
-                w.set_spe(true);
-            });
-        }
+        self.info.regs.cr2().modify(|w| {
+            w.set_frxth(word_size.1);
+            w.set_ds(word_size.0);
+        });
         #[cfg(any(spi_v3, spi_v4, spi_v5))]
-        {
-            self.info.regs.cr1().modify(|w| {
-                w.set_csusp(true);
-            });
-            while self.info.regs.sr().read().eot() {}
-            self.info.regs.cr1().modify(|w| {
-                w.set_spe(false);
-            });
-            self.info.regs.cfg1().modify(|w| {
-                w.set_dsize(word_size);
-            });
-            self.info.regs.cr1().modify(|w| {
-                w.set_csusp(false);
-                w.set_spe(true);
-            });
-        }
+        self.info.regs.cfg1().modify(|w| {
+            w.set_dsize(word_size);
+        });
 
         self.current_word_size = word_size;
     }
@@ -365,9 +343,9 @@ impl<'d, M: PeriMode> Spi<'d, M> {
         // needed in v3+ to avoid overrun causing the SPI RX state machine to get stuck...?
         #[cfg(any(spi_v3, spi_v4, spi_v5))]
         self.info.regs.cr1().modify(|w| w.set_spe(false));
+        self.set_word_size(W::CONFIG);
         self.info.regs.cr1().modify(|w| w.set_spe(true));
         flush_rx_fifo(self.info.regs);
-        self.set_word_size(W::CONFIG);
         for word in words.iter() {
             // this cannot use `transfer_word` because on SPIv2 and higher,
             // the SPI RX state machine hangs if no physical pin is connected to the SCK AF.
@@ -402,9 +380,9 @@ impl<'d, M: PeriMode> Spi<'d, M> {
         // needed in v3+ to avoid overrun causing the SPI RX state machine to get stuck...?
         #[cfg(any(spi_v3, spi_v4, spi_v5))]
         self.info.regs.cr1().modify(|w| w.set_spe(false));
+        self.set_word_size(W::CONFIG);
         self.info.regs.cr1().modify(|w| w.set_spe(true));
         flush_rx_fifo(self.info.regs);
-        self.set_word_size(W::CONFIG);
         for word in words.iter_mut() {
             *word = transfer_word(self.info.regs, W::default())?;
         }
@@ -418,9 +396,9 @@ impl<'d, M: PeriMode> Spi<'d, M> {
         // needed in v3+ to avoid overrun causing the SPI RX state machine to get stuck...?
         #[cfg(any(spi_v3, spi_v4, spi_v5))]
         self.info.regs.cr1().modify(|w| w.set_spe(false));
+        self.set_word_size(W::CONFIG);
         self.info.regs.cr1().modify(|w| w.set_spe(true));
         flush_rx_fifo(self.info.regs);
-        self.set_word_size(W::CONFIG);
         for word in words.iter_mut() {
             *word = transfer_word(self.info.regs, *word)?;
         }
@@ -437,9 +415,9 @@ impl<'d, M: PeriMode> Spi<'d, M> {
         // needed in v3+ to avoid overrun causing the SPI RX state machine to get stuck...?
         #[cfg(any(spi_v3, spi_v4, spi_v5))]
         self.info.regs.cr1().modify(|w| w.set_spe(false));
+        self.set_word_size(W::CONFIG);
         self.info.regs.cr1().modify(|w| w.set_spe(true));
         flush_rx_fifo(self.info.regs);
-        self.set_word_size(W::CONFIG);
         let len = read.len().max(write.len());
         for i in 0..len {
             let wb = write.get(i).copied().unwrap_or_default();
@@ -648,10 +626,10 @@ impl<'d> Spi<'d, Async> {
             return Ok(());
         }
 
-        self.set_word_size(W::CONFIG);
         self.info.regs.cr1().modify(|w| {
             w.set_spe(false);
         });
+        self.set_word_size(W::CONFIG);
 
         let tx_dst = self.info.regs.tx_ptr();
         let tx_f = unsafe { self.tx_dma.as_mut().unwrap().write(data, tx_dst, Default::default()) };
@@ -685,6 +663,8 @@ impl<'d> Spi<'d, Async> {
             w.set_spe(false);
         });
 
+        self.set_word_size(W::CONFIG);
+
         let comm = regs.cfg2().modify(|w| {
             let prev = w.comm();
             w.set_comm(vals::Comm::RECEIVER);
@@ -707,7 +687,6 @@ impl<'d> Spi<'d, Async> {
         let rx_src = regs.rx_ptr();
 
         for mut chunk in data.chunks_mut(u16::max_value().into()) {
-            self.set_word_size(W::CONFIG);
             set_rxdmaen(regs, true);
 
             let tsize = chunk.len();
@@ -765,12 +744,12 @@ impl<'d> Spi<'d, Async> {
             return Ok(());
         }
 
-        self.set_word_size(W::CONFIG);
-
         self.info.regs.cr1().modify(|w| {
             w.set_spe(false);
         });
 
+        self.set_word_size(W::CONFIG);
+
         // SPIv3 clears rxfifo on SPE=0
         #[cfg(not(any(spi_v3, spi_v4, spi_v5)))]
         flush_rx_fifo(self.info.regs);
@@ -813,11 +792,12 @@ impl<'d> Spi<'d, Async> {
             return Ok(());
         }
 
-        self.set_word_size(W::CONFIG);
         self.info.regs.cr1().modify(|w| {
             w.set_spe(false);
         });
 
+        self.set_word_size(W::CONFIG);
+
         // SPIv3 clears rxfifo on SPE=0
         #[cfg(not(any(spi_v3, spi_v4, spi_v5)))]
         flush_rx_fifo(self.info.regs);
diff --git a/tests/stm32/src/bin/spi.rs b/tests/stm32/src/bin/spi.rs
index 53d44a94a..9712a8c5a 100644
--- a/tests/stm32/src/bin/spi.rs
+++ b/tests/stm32/src/bin/spi.rs
@@ -7,7 +7,8 @@ use common::*;
 use defmt::assert_eq;
 use embassy_executor::Spawner;
 use embassy_stm32::gpio::{Level, Output, Speed};
-use embassy_stm32::spi::{self, Spi};
+use embassy_stm32::mode::Blocking;
+use embassy_stm32::spi::{self, Spi, Word};
 use embassy_stm32::time::Hertz;
 
 #[embassy_executor::main]
@@ -31,11 +32,58 @@ async fn main(_spawner: Spawner) {
         spi_config,
     );
 
-    let data: [u8; 9] = [0x00, 0xFF, 0xAA, 0x55, 0xC0, 0xFF, 0xEE, 0xC0, 0xDE];
+    test_txrx::<u8>(&mut spi);
+    test_txrx::<u16>(&mut spi);
+
+    // Assert the RCC bit gets disabled on drop.
+    #[cfg(feature = "stm32f429zi")]
+    defmt::assert!(embassy_stm32::pac::RCC.apb2enr().read().spi1en());
+    drop(spi);
+    #[cfg(feature = "stm32f429zi")]
+    defmt::assert!(!embassy_stm32::pac::RCC.apb2enr().read().spi1en());
+
+    // test rx-only configuration
+    let mut spi = Spi::new_blocking_rxonly(&mut spi_peri, &mut sck, &mut miso, spi_config);
+    let mut mosi_out = Output::new(&mut mosi, Level::Low, Speed::VeryHigh);
+
+    test_rx::<u8>(&mut spi, &mut mosi_out);
+    test_rx::<u16>(&mut spi, &mut mosi_out);
+    drop(spi);
+    drop(mosi_out);
+
+    let mut spi = Spi::new_blocking_txonly(&mut spi_peri, &mut sck, &mut mosi, spi_config);
+    test_tx::<u8>(&mut spi);
+    test_tx::<u16>(&mut spi);
+    drop(spi);
+
+    let mut spi = Spi::new_blocking_txonly_nosck(&mut spi_peri, &mut mosi, spi_config);
+    test_tx::<u8>(&mut spi);
+    test_tx::<u16>(&mut spi);
+    drop(spi);
+
+    info!("Test OK");
+    cortex_m::asm::bkpt();
+}
+
+fn test_txrx<W: Word + From<u8> + defmt::Format + Eq>(spi: &mut Spi<'_, Blocking>)
+where
+    W: core::ops::Not<Output = W>,
+{
+    let data: [W; 9] = [
+        0x00u8.into(),
+        0xFFu8.into(),
+        0xAAu8.into(),
+        0x55u8.into(),
+        0xC0u8.into(),
+        0xFFu8.into(),
+        0xEEu8.into(),
+        0xC0u8.into(),
+        0xDEu8.into(),
+    ];
 
     // 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 = [0; 9];
+    let mut buf = [W::default(); 9];
     spi.blocking_transfer(&mut buf, &data).unwrap();
     assert_eq!(buf, data);
 
@@ -59,47 +107,33 @@ async fn main(_spawner: Spawner) {
     spi.blocking_transfer_in_place::<u8>(&mut []).unwrap();
     spi.blocking_read::<u8>(&mut []).unwrap();
     spi.blocking_write::<u8>(&[]).unwrap();
+}
 
-    // Assert the RCC bit gets disabled on drop.
-    #[cfg(feature = "stm32f429zi")]
-    defmt::assert!(embassy_stm32::pac::RCC.apb2enr().read().spi1en());
-    drop(spi);
-    #[cfg(feature = "stm32f429zi")]
-    defmt::assert!(!embassy_stm32::pac::RCC.apb2enr().read().spi1en());
+fn test_rx<W: Word + From<u8> + defmt::Format + Eq>(spi: &mut Spi<'_, Blocking>, mosi_out: &mut Output<'_>)
+where
+    W: core::ops::Not<Output = W>,
+{
+    let mut buf = [W::default(); 9];
 
-    // test rx-only configuration
-    let mut spi = Spi::new_blocking_rxonly(&mut spi_peri, &mut sck, &mut miso, spi_config);
-    let mut mosi_out = Output::new(&mut mosi, Level::Low, Speed::VeryHigh);
     mosi_out.set_high();
     spi.blocking_read(&mut buf).unwrap();
-    assert_eq!(buf, [0xff; 9]);
+    assert_eq!(buf, [!W::default(); 9]);
     mosi_out.set_low();
     spi.blocking_read(&mut buf).unwrap();
-    assert_eq!(buf, [0x00; 9]);
+    assert_eq!(buf, [W::default(); 9]);
     spi.blocking_read::<u8>(&mut []).unwrap();
     spi.blocking_read::<u8>(&mut []).unwrap();
-    drop(mosi_out);
-    drop(spi);
+}
+
+fn test_tx<W: Word + From<u8> + defmt::Format + Eq>(spi: &mut Spi<'_, Blocking>)
+where
+    W: core::ops::Not<Output = W>,
+{
+    let buf = [W::default(); 9];
 
     // Test tx-only. Just check it doesn't hang, not much else we can do without using SPI slave.
-    let mut spi = Spi::new_blocking_txonly(&mut spi_peri, &mut sck, &mut mosi, spi_config);
-    spi.blocking_transfer(&mut buf, &data).unwrap();
-    spi.blocking_transfer_in_place(&mut buf).unwrap();
     spi.blocking_write(&buf).unwrap();
-    spi.blocking_read(&mut buf).unwrap();
-    spi.blocking_transfer::<u8>(&mut [], &[]).unwrap();
-    spi.blocking_transfer_in_place::<u8>(&mut []).unwrap();
-    spi.blocking_read::<u8>(&mut []).unwrap();
     spi.blocking_write::<u8>(&[]).unwrap();
-    drop(spi);
-
-    // Test tx-only nosck.
-    let mut spi = Spi::new_blocking_txonly_nosck(&mut spi_peri, &mut mosi, spi_config);
     spi.blocking_write(&buf).unwrap();
     spi.blocking_write::<u8>(&[]).unwrap();
-    spi.blocking_write(&buf).unwrap();
-    drop(spi);
-
-    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 a1cbc0ed1..307409a16 100644
--- a/tests/stm32/src/bin/spi_dma.rs
+++ b/tests/stm32/src/bin/spi_dma.rs
@@ -7,7 +7,8 @@ use common::*;
 use defmt::assert_eq;
 use embassy_executor::Spawner;
 use embassy_stm32::gpio::{Level, Output, Speed};
-use embassy_stm32::spi::{self, Spi};
+use embassy_stm32::mode::Async;
+use embassy_stm32::spi::{self, Spi, Word};
 use embassy_stm32::time::Hertz;
 
 #[embassy_executor::main]
@@ -35,11 +36,61 @@ async fn main(_spawner: Spawner) {
         spi_config,
     );
 
-    let data: [u8; 9] = [0x00, 0xFF, 0xAA, 0x55, 0xC0, 0xFF, 0xEE, 0xC0, 0xDE];
+    test_txrx::<u8>(&mut spi).await;
+    test_txrx::<u16>(&mut spi).await;
+    drop(spi);
+
+    // test rx-only configuration
+    let mut spi = Spi::new_rxonly(
+        &mut spi_peri,
+        &mut sck,
+        &mut miso,
+        // SPIv1/f1 requires txdma even if rxonly.
+        #[cfg(not(feature = "spi-v345"))]
+        &mut tx_dma,
+        &mut rx_dma,
+        spi_config,
+    );
+    let mut mosi_out = Output::new(&mut mosi, Level::Low, Speed::VeryHigh);
+
+    test_rx::<u8>(&mut spi, &mut mosi_out).await;
+    test_rx::<u16>(&mut spi, &mut mosi_out).await;
+    drop(spi);
+    drop(mosi_out);
+
+    let mut spi = Spi::new_txonly(&mut spi_peri, &mut sck, &mut mosi, &mut tx_dma, spi_config);
+    test_tx::<u8>(&mut spi).await;
+    test_tx::<u16>(&mut spi).await;
+    drop(spi);
+
+    let mut spi = Spi::new_txonly_nosck(&mut spi_peri, &mut mosi, &mut tx_dma, spi_config);
+    test_tx::<u8>(&mut spi).await;
+    test_tx::<u16>(&mut spi).await;
+    drop(spi);
+
+    info!("Test OK");
+    cortex_m::asm::bkpt();
+}
+
+async fn test_txrx<W: Word + From<u8> + defmt::Format + Eq>(spi: &mut Spi<'_, Async>)
+where
+    W: core::ops::Not<Output = W>,
+{
+    let data: [W; 9] = [
+        0x00u8.into(),
+        0xFFu8.into(),
+        0xAAu8.into(),
+        0x55u8.into(),
+        0xC0u8.into(),
+        0xFFu8.into(),
+        0xEEu8.into(),
+        0xC0u8.into(),
+        0xDEu8.into(),
+    ];
 
     // 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 = [0; 9];
+    let mut buf = [W::default(); 9];
     spi.transfer(&mut buf, &data).await.unwrap();
     assert_eq!(buf, data);
 
@@ -83,56 +134,41 @@ async fn main(_spawner: Spawner) {
     spi.blocking_write(&buf).unwrap();
     spi.blocking_read(&mut buf).unwrap();
     spi.write(&buf).await.unwrap();
+}
 
-    core::mem::drop(spi);
+async fn test_rx<W: Word + From<u8> + defmt::Format + Eq>(spi: &mut Spi<'_, Async>, mosi_out: &mut Output<'_>)
+where
+    W: core::ops::Not<Output = W>,
+{
+    let mut buf = [W::default(); 9];
 
-    // test rx-only configuration
-    let mut spi = Spi::new_rxonly(
-        &mut spi_peri,
-        &mut sck,
-        &mut miso,
-        // SPIv1/f1 requires txdma even if rxonly.
-        #[cfg(not(feature = "spi-v345"))]
-        &mut tx_dma,
-        &mut rx_dma,
-        spi_config,
-    );
-    let mut mosi_out = Output::new(&mut mosi, Level::Low, Speed::VeryHigh);
     mosi_out.set_high();
     spi.read(&mut buf).await.unwrap();
-    assert_eq!(buf, [0xff; 9]);
+    assert_eq!(buf, [!W::default(); 9]);
     spi.blocking_read(&mut buf).unwrap();
-    assert_eq!(buf, [0xff; 9]);
+    assert_eq!(buf, [!W::default(); 9]);
     spi.read(&mut buf).await.unwrap();
-    assert_eq!(buf, [0xff; 9]);
+    assert_eq!(buf, [!W::default(); 9]);
     spi.read(&mut buf).await.unwrap();
-    assert_eq!(buf, [0xff; 9]);
+    assert_eq!(buf, [!W::default(); 9]);
     spi.blocking_read(&mut buf).unwrap();
-    assert_eq!(buf, [0xff; 9]);
+    assert_eq!(buf, [!W::default(); 9]);
     spi.blocking_read(&mut buf).unwrap();
-    assert_eq!(buf, [0xff; 9]);
+    assert_eq!(buf, [!W::default(); 9]);
     mosi_out.set_low();
     spi.read(&mut buf).await.unwrap();
-    assert_eq!(buf, [0x00; 9]);
+    assert_eq!(buf, [W::default(); 9]);
     spi.read::<u8>(&mut []).await.unwrap();
     spi.blocking_read::<u8>(&mut []).unwrap();
-    drop(mosi_out);
-    drop(spi);
+}
 
-    // Test tx-only. Just check it doesn't hang, not much else we can do without using SPI slave.
-    let mut spi = Spi::new_txonly(&mut spi_peri, &mut sck, &mut mosi, &mut tx_dma, spi_config);
-    spi.blocking_write(&buf).unwrap();
-    spi.write(&buf).await.unwrap();
-    spi.blocking_write(&buf).unwrap();
-    spi.blocking_write(&buf).unwrap();
-    spi.write(&buf).await.unwrap();
-    spi.write(&buf).await.unwrap();
-    spi.write::<u8>(&[]).await.unwrap();
-    spi.blocking_write::<u8>(&[]).unwrap();
-    drop(spi);
+async fn test_tx<W: Word + From<u8> + defmt::Format + Eq>(spi: &mut Spi<'_, Async>)
+where
+    W: core::ops::Not<Output = W>,
+{
+    let buf = [W::default(); 9];
 
-    // Test tx-only nosck.
-    let mut spi = Spi::new_txonly_nosck(&mut spi_peri, &mut mosi, &mut tx_dma, spi_config);
+    // Test tx-only. Just check it doesn't hang, not much else we can do without using SPI slave.
     spi.blocking_write(&buf).unwrap();
     spi.write(&buf).await.unwrap();
     spi.blocking_write(&buf).unwrap();
@@ -141,8 +177,4 @@ async fn main(_spawner: Spawner) {
     spi.write(&buf).await.unwrap();
     spi.write::<u8>(&[]).await.unwrap();
     spi.blocking_write::<u8>(&[]).unwrap();
-    drop(spi);
-
-    info!("Test OK");
-    cortex_m::asm::bkpt();
 }