From 152d31ad62dc4e33c4d52631982ea685f6cca991 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Thu, 22 Aug 2024 11:55:34 +0100 Subject: [PATCH 01/15] use `State` for both blocking and async operations, remove async version of SpiDmaBus in favour of being generic over the mode --- esp-hal/src/spi/master.rs | 283 +++++++++++--------------- hil-test/tests/spi_full_duplex_dma.rs | 1 + 2 files changed, 121 insertions(+), 163 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 1c479a532d2..1926a8bdb13 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -968,7 +968,6 @@ pub mod dma { SpiPeripheral, TxPrivate, }, - Blocking, InterruptConfigurable, Mode, }; @@ -1137,11 +1136,12 @@ pub mod dma { } } - impl<'d, T, C> SpiDma<'d, T, C, FullDuplexMode, Blocking> + impl<'d, T, C, M> SpiDma<'d, T, C, FullDuplexMode, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + M: Mode, { /// Configures the DMA buffers for the SPI instance. /// @@ -1152,32 +1152,11 @@ pub mod dma { self, dma_tx_buf: DmaTxBuf, dma_rx_buf: DmaRxBuf, - ) -> SpiDmaBus<'d, T, C> { + ) -> SpiDmaBus<'d, T, C, M> { SpiDmaBus::new(self, dma_tx_buf, dma_rx_buf) } } - #[cfg(feature = "async")] - impl<'d, T, C> SpiDma<'d, T, C, FullDuplexMode, crate::Async> - where - T: InstanceDma, - C: DmaChannel, - C::P: SpiPeripheral, - { - /// Configures the DMA buffers for asynchronous SPI communication. - /// - /// This method sets up both TX and RX buffers for DMA transfers. - /// It eturns an instance of `SpiDmaAsyncBus` to be used for - /// asynchronous SPI operations. - pub fn with_buffers( - self, - dma_tx_buf: DmaTxBuf, - dma_rx_buf: DmaRxBuf, - ) -> asynch::SpiDmaAsyncBus<'d, T, C> { - asynch::SpiDmaAsyncBus::new(self, dma_tx_buf, dma_rx_buf) - } - } - /// A structure representing a DMA transfer for SPI. /// /// This structure holds references to the SPI instance, DMA buffers, and @@ -1565,43 +1544,83 @@ pub mod dma { } } - /// A DMA-capable SPI bus that handles full-duplex transfers. + #[derive(Default)] + enum State<'d, T, C, M> + where + T: InstanceDma, + C: DmaChannel, + C::P: SpiPeripheral, + M: Mode, + { + Idle(SpiDma<'d, T, C, FullDuplexMode, M>, DmaTxBuf, DmaRxBuf), + Reading( + SpiDmaTransfer<'d, T, C, FullDuplexMode, M, DmaRxBuf>, + DmaTxBuf, + ), + Writing( + SpiDmaTransfer<'d, T, C, FullDuplexMode, M, DmaTxBuf>, + DmaRxBuf, + ), + Transferring(SpiDmaTransfer<'d, T, C, FullDuplexMode, M, (DmaTxBuf, DmaRxBuf)>), + #[default] + InUse, + } + + /// A DMA-capable SPI bus. /// /// This structure is responsible for managing SPI transfers using DMA /// buffers. - pub struct SpiDmaBus<'d, T, C> + pub struct SpiDmaBus<'d, T, C, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + M: Mode, { - spi_dma: Option>, - buffers: Option<(DmaTxBuf, DmaRxBuf)>, + state: State<'d, T, C, M>, } - impl<'d, T, C> SpiDmaBus<'d, T, C> + impl<'d, T, C, M> SpiDmaBus<'d, T, C, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + M: Mode, { /// Creates a new `SpiDmaBus` with the specified SPI instance and DMA /// buffers. pub fn new( - spi_dma: SpiDma<'d, T, C, FullDuplexMode, crate::Blocking>, - tx_buffer: DmaTxBuf, - rx_buffer: DmaRxBuf, + spi: SpiDma<'d, T, C, FullDuplexMode, M>, + dma_tx_buf: DmaTxBuf, + dma_rx_buf: DmaRxBuf, ) -> Self { Self { - spi_dma: Some(spi_dma), - buffers: Some((tx_buffer, rx_buffer)), + state: State::Idle(spi, dma_tx_buf, dma_rx_buf), + } + } + + fn wait_for_idle(&mut self) -> (SpiDma<'d, T, C, FullDuplexMode, M>, DmaTxBuf, DmaRxBuf) { + match core::mem::take(&mut self.state) { + State::Idle(spi, tx_buf, rx_buf) => (spi, tx_buf, rx_buf), + State::Reading(transfer, tx_buf) => { + let (spi, rx_buf) = transfer.wait(); + (spi, tx_buf, rx_buf) + } + State::Writing(transfer, rx_buf) => { + let (spi, tx_buf) = transfer.wait(); + (spi, tx_buf, rx_buf) + } + State::Transferring(transfer) => { + let (spi, (tx_buf, rx_buf)) = transfer.wait(); + (spi, tx_buf, rx_buf) + } + State::InUse => unreachable!(), } } /// Reads data from the SPI bus using DMA. pub fn read(&mut self, words: &mut [u8]) -> Result<(), Error> { - let mut spi_dma = self.spi_dma.take().unwrap(); - let (tx_buf, mut rx_buf) = self.buffers.take().unwrap(); + let (mut spi_dma, tx_buf, mut rx_buf) = self.wait_for_idle(); for chunk in words.chunks_mut(rx_buf.capacity()) { rx_buf.set_length(chunk.len()); @@ -1609,8 +1628,7 @@ pub mod dma { let transfer = match spi_dma.dma_read(rx_buf) { Ok(transfer) => transfer, Err((e, spi, rx)) => { - self.spi_dma = Some(spi); - self.buffers = Some((tx_buf, rx)); + self.state = State::Idle(spi, tx_buf, rx); return Err(e); } }; @@ -1620,16 +1638,14 @@ pub mod dma { debug_assert_eq!(bytes_read, chunk.len()); } - self.spi_dma = Some(spi_dma); - self.buffers = Some((tx_buf, rx_buf)); + self.state = State::Idle(spi_dma, tx_buf, rx_buf); Ok(()) } /// Writes data to the SPI bus using DMA. pub fn write(&mut self, words: &[u8]) -> Result<(), Error> { - let mut spi_dma = self.spi_dma.take().unwrap(); - let (mut tx_buf, rx_buf) = self.buffers.take().unwrap(); + let (mut spi_dma, mut tx_buf, rx_buf) = self.wait_for_idle(); for chunk in words.chunks(tx_buf.capacity()) { tx_buf.fill(chunk); @@ -1637,24 +1653,21 @@ pub mod dma { let transfer = match spi_dma.dma_write(tx_buf) { Ok(transfer) => transfer, Err((e, spi, tx)) => { - self.spi_dma = Some(spi); - self.buffers = Some((tx, rx_buf)); + self.state = State::Idle(spi, tx, rx_buf); return Err(e); } }; (spi_dma, tx_buf) = transfer.wait(); } - self.spi_dma = Some(spi_dma); - self.buffers = Some((tx_buf, rx_buf)); + self.state = State::Idle(spi_dma, tx_buf, rx_buf); Ok(()) } /// Transfers data to and from the SPI bus simultaneously using DMA. pub fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Error> { - let mut spi_dma = self.spi_dma.take().unwrap(); - let (mut tx_buf, mut rx_buf) = self.buffers.take().unwrap(); + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); let chunk_size = min(tx_buf.capacity(), rx_buf.capacity()); @@ -1672,8 +1685,7 @@ pub mod dma { let transfer = match spi_dma.dma_transfer(tx_buf, rx_buf) { Ok(transfer) => transfer, Err((e, spi, tx, rx)) => { - self.spi_dma = Some(spi); - self.buffers = Some((tx, rx)); + self.state = State::Idle(spi, tx, rx); return Err(e); } }; @@ -1683,8 +1695,7 @@ pub mod dma { debug_assert_eq!(bytes_read, read_chunk.len()); } - self.spi_dma = Some(spi_dma); - self.buffers = Some((tx_buf, rx_buf)); + self.state = State::Idle(spi_dma, tx_buf, rx_buf); if !read_remainder.is_empty() { self.read(read_remainder) @@ -1697,8 +1708,7 @@ pub mod dma { /// Transfers data in place on the SPI bus using DMA. pub fn transfer_in_place(&mut self, words: &mut [u8]) -> Result<(), Error> { - let mut spi_dma = self.spi_dma.take().unwrap(); - let (mut tx_buf, mut rx_buf) = self.buffers.take().unwrap(); + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); let chunk_size = min(tx_buf.capacity(), rx_buf.capacity()); @@ -1709,8 +1719,7 @@ pub mod dma { let transfer = match spi_dma.dma_transfer(tx_buf, rx_buf) { Ok(transfer) => transfer, Err((e, spi, tx, rx)) => { - self.spi_dma = Some(spi); - self.buffers = Some((tx, rx)); + self.state = State::Idle(spi, tx, rx); return Err(e); } }; @@ -1720,15 +1729,13 @@ pub mod dma { debug_assert_eq!(bytes_read, chunk.len()); } - self.spi_dma = Some(spi_dma); - self.buffers = Some((tx_buf, rx_buf)); - + self.state = State::Idle(spi_dma, tx_buf, rx_buf); Ok(()) } } #[cfg(feature = "embedded-hal-02")] - impl<'d, T, C> embedded_hal_02::blocking::spi::Transfer for SpiDmaBus<'d, T, C> + impl<'d, T, C> embedded_hal_02::blocking::spi::Transfer for SpiDmaBus<'d, T, C, crate::Blocking> where T: InstanceDma, C: DmaChannel, @@ -1743,7 +1750,7 @@ pub mod dma { } #[cfg(feature = "embedded-hal-02")] - impl<'d, T, C> embedded_hal_02::blocking::spi::Write for SpiDmaBus<'d, T, C> + impl<'d, T, C> embedded_hal_02::blocking::spi::Write for SpiDmaBus<'d, T, C, crate::Blocking> where T: InstanceDma, C: DmaChannel, @@ -1762,73 +1769,15 @@ pub mod dma { pub mod asynch { use core::{cmp::min, mem::take}; - use embedded_hal::spi::ErrorType; - use super::*; - #[derive(Default)] - enum State<'d, T, C> + impl<'d, T, C> SpiDmaBus<'d, T, C, crate::Async> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, { - Idle( - SpiDma<'d, T, C, FullDuplexMode, crate::Async>, - DmaTxBuf, - DmaRxBuf, - ), - Reading( - SpiDmaTransfer<'d, T, C, FullDuplexMode, crate::Async, DmaRxBuf>, - DmaTxBuf, - ), - Writing( - SpiDmaTransfer<'d, T, C, FullDuplexMode, crate::Async, DmaTxBuf>, - DmaRxBuf, - ), - Transferring( - SpiDmaTransfer<'d, T, C, FullDuplexMode, crate::Async, (DmaTxBuf, DmaRxBuf)>, - ), - #[default] - InUse, - } - - /// An asynchronous DMA-capable SPI bus for full-duplex operations. - /// - /// This struct provides an interface for SPI operations using DMA in an - /// asynchronous way. - pub struct SpiDmaAsyncBus<'d, T, C> - where - T: InstanceDma, - C: DmaChannel, - C::P: SpiPeripheral, - { - state: State<'d, T, C>, - } - - impl<'d, T, C> SpiDmaAsyncBus<'d, T, C> - where - T: InstanceDma, - C: DmaChannel, - C::P: SpiPeripheral, - { - /// Creates a new asynchronous DMA SPI bus instance. - /// - /// Initializes the bus with the provided SPI instance and DMA - /// buffers for transmit and receive operations. - pub fn new( - spi: SpiDma<'d, T, C, FullDuplexMode, crate::Async>, - dma_tx_buf: DmaTxBuf, - dma_rx_buf: DmaRxBuf, - ) -> Self { - Self { - state: State::Idle(spi, dma_tx_buf, dma_rx_buf), - } - } - - /// Waits for the current SPI DMA transfer to complete, ensuring the - /// bus is idle. - async fn wait_for_idle( + async fn wait_for_idle_async( &mut self, ) -> ( SpiDma<'d, T, C, FullDuplexMode, crate::Async>, @@ -1859,28 +1808,10 @@ pub mod dma { State::InUse => unreachable!(), } } - } - impl<'d, T, C> ErrorType for SpiDmaAsyncBus<'d, T, C> - where - T: InstanceDma, - C: DmaChannel, - C::P: SpiPeripheral, - { - type Error = Error; - } - - impl<'d, T, C> embedded_hal_async::spi::SpiBus for SpiDmaAsyncBus<'d, T, C> - where - T: InstanceDma, - C: DmaChannel, - C::P: SpiPeripheral, - { - /// Asynchronously reads data from the SPI bus into the provided - /// buffer. - async fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { + async fn read_async(&mut self, words: &mut [u8]) -> Result<(), super::Error> { // Get previous transfer. - let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle().await; + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle_async().await; for chunk in words.chunks_mut(rx_buf.capacity()) { rx_buf.set_length(chunk.len()); @@ -1916,11 +1847,9 @@ pub mod dma { Ok(()) } - /// Asynchronously writes data to the SPI bus from the provided - /// buffer. - async fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { + async fn write_async(&mut self, words: &[u8]) -> Result<(), super::Error> { // Get previous transfer. - let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle().await; + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle_async().await; for chunk in words.chunks(tx_buf.capacity()) { tx_buf.fill(chunk); @@ -1954,15 +1883,13 @@ pub mod dma { Ok(()) } - /// Asynchronously performs a full-duplex transfer over the SPI bus. - /// - /// This method splits the transfer operation into chunks and - /// processes it asynchronously. It simultaneously - /// writes data from the `write` buffer and reads data into the - /// `read` buffer. - async fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> { + async fn transfer_async( + &mut self, + read: &mut [u8], + write: &[u8], + ) -> Result<(), super::Error> { // Get previous transfer. - let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle().await; + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle_async().await; let chunk_size = min(tx_buf.capacity(), rx_buf.capacity()); @@ -2007,19 +1934,20 @@ pub mod dma { self.state = State::Idle(spi_dma, tx_buf, rx_buf); if !read_remainder.is_empty() { - self.read(read_remainder).await + self.read_async(read_remainder).await } else if !write_remainder.is_empty() { - self.write(write_remainder).await + self.write_async(write_remainder).await } else { Ok(()) } } - /// Asynchronously performs an in-place full-duplex transfer over - /// the SPI bus. - async fn transfer_in_place(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { + async fn transfer_in_place_async( + &mut self, + words: &mut [u8], + ) -> Result<(), super::Error> { // Get previous transfer. - let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle().await; + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle_async().await; for chunk in words.chunks_mut(tx_buf.capacity()) { tx_buf.fill(chunk); @@ -2057,13 +1985,40 @@ pub mod dma { Ok(()) } - async fn flush(&mut self) -> Result<(), Self::Error> { + async fn flush_async(&mut self) -> Result<(), super::Error> { // Get previous transfer. - let (spi_dma, tx_buf, rx_buf) = self.wait_for_idle().await; + let (spi_dma, tx_buf, rx_buf) = self.wait_for_idle_async().await; self.state = State::Idle(spi_dma, tx_buf, rx_buf); Ok(()) } } + + impl<'d, T, C> embedded_hal_async::spi::SpiBus for SpiDmaBus<'d, T, C, crate::Async> + where + T: InstanceDma, + C: DmaChannel, + C::P: SpiPeripheral, + { + async fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { + self.read_async(words).await + } + + async fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { + self.write_async(words).await + } + + async fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> { + self.transfer_async(read, write).await + } + + async fn transfer_in_place(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { + self.transfer_in_place_async(words).await + } + + async fn flush(&mut self) -> Result<(), Self::Error> { + self.flush_async().await + } + } } #[cfg(feature = "embedded-hal")] @@ -2072,20 +2027,22 @@ pub mod dma { use super::*; - impl<'d, T, C> ErrorType for SpiDmaBus<'d, T, C> + impl<'d, T, C, M> ErrorType for SpiDmaBus<'d, T, C, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + M: Mode, { type Error = Error; } - impl<'d, T, C> SpiBus for SpiDmaBus<'d, T, C> + impl<'d, T, C, M> SpiBus for SpiDmaBus<'d, T, C, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + M: Mode, { fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { self.read(words) diff --git a/hil-test/tests/spi_full_duplex_dma.rs b/hil-test/tests/spi_full_duplex_dma.rs index 3988d59f249..b530e2e1a1f 100644 --- a/hil-test/tests/spi_full_duplex_dma.rs +++ b/hil-test/tests/spi_full_duplex_dma.rs @@ -49,6 +49,7 @@ struct Context { #[embedded_test::tests] mod tests { use defmt::assert_eq; + use esp_hal::dma::{DmaRxBuf, DmaTxBuf}; use super::*; From 16425466f23c9e2d99067df71ec73fb2e71f9b85 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Thu, 22 Aug 2024 12:18:06 +0100 Subject: [PATCH 02/15] reuse wait_for_idle more --- esp-hal/src/spi/master.rs | 74 +++++++++++---------------------------- 1 file changed, 21 insertions(+), 53 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 1926a8bdb13..9ef4293c2f9 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -1620,19 +1620,19 @@ pub mod dma { /// Reads data from the SPI bus using DMA. pub fn read(&mut self, words: &mut [u8]) -> Result<(), Error> { - let (mut spi_dma, tx_buf, mut rx_buf) = self.wait_for_idle(); + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); for chunk in words.chunks_mut(rx_buf.capacity()) { rx_buf.set_length(chunk.len()); - let transfer = match spi_dma.dma_read(rx_buf) { - Ok(transfer) => transfer, + match spi_dma.dma_read(rx_buf) { + Ok(transfer) => self.state = State::Reading(transfer, tx_buf), Err((e, spi, rx)) => { self.state = State::Idle(spi, tx_buf, rx); return Err(e); } - }; - (spi_dma, rx_buf) = transfer.wait(); + } + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); let bytes_read = rx_buf.read_received_data(chunk); debug_assert_eq!(bytes_read, chunk.len()); @@ -1645,19 +1645,19 @@ pub mod dma { /// Writes data to the SPI bus using DMA. pub fn write(&mut self, words: &[u8]) -> Result<(), Error> { - let (mut spi_dma, mut tx_buf, rx_buf) = self.wait_for_idle(); + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); for chunk in words.chunks(tx_buf.capacity()) { tx_buf.fill(chunk); - let transfer = match spi_dma.dma_write(tx_buf) { - Ok(transfer) => transfer, + match spi_dma.dma_write(tx_buf) { + Ok(transfer) => self.state = State::Writing(transfer, rx_buf), Err((e, spi, tx)) => { self.state = State::Idle(spi, tx, rx_buf); return Err(e); } - }; - (spi_dma, tx_buf) = transfer.wait(); + } + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); } self.state = State::Idle(spi_dma, tx_buf, rx_buf); @@ -1682,14 +1682,14 @@ pub mod dma { tx_buf.fill(write_chunk); rx_buf.set_length(read_chunk.len()); - let transfer = match spi_dma.dma_transfer(tx_buf, rx_buf) { - Ok(transfer) => transfer, + match spi_dma.dma_transfer(tx_buf, rx_buf) { + Ok(transfer) => self.state = State::Transferring(transfer), Err((e, spi, tx, rx)) => { self.state = State::Idle(spi, tx, rx); return Err(e); } - }; - (spi_dma, (tx_buf, rx_buf)) = transfer.wait(); + } + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); let bytes_read = rx_buf.read_received_data(read_chunk); debug_assert_eq!(bytes_read, read_chunk.len()); @@ -1716,14 +1716,14 @@ pub mod dma { tx_buf.fill(chunk); rx_buf.set_length(chunk.len()); - let transfer = match spi_dma.dma_transfer(tx_buf, rx_buf) { - Ok(transfer) => transfer, + match spi_dma.dma_transfer(tx_buf, rx_buf) { + Ok(transfer) => self.state = State::Transferring(transfer), Err((e, spi, tx, rx)) => { self.state = State::Idle(spi, tx, rx); return Err(e); } - }; - (spi_dma, (tx_buf, rx_buf)) = transfer.wait(); + } + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); let bytes_read = rx_buf.read_received_data(chunk); debug_assert_eq!(bytes_read, chunk.len()); @@ -1826,17 +1826,7 @@ pub mod dma { } }; - match &mut self.state { - State::Reading(transfer, _) => transfer.wait_for_done().await, - _ => unreachable!(), - }; - (spi_dma, tx_buf, rx_buf) = match take(&mut self.state) { - State::Reading(transfer, tx_buf) => { - let (spi, rx_buf) = transfer.wait(); - (spi, tx_buf, rx_buf) - } - _ => unreachable!(), - }; + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle_async().await; let bytes_read = rx_buf.read_received_data(chunk); debug_assert_eq!(bytes_read, chunk.len()); @@ -1864,18 +1854,7 @@ pub mod dma { } }; - match &mut self.state { - State::Writing(transfer, _) => transfer.wait_for_done().await, - _ => unreachable!(), - }; - - (spi_dma, tx_buf, rx_buf) = match take(&mut self.state) { - State::Writing(transfer, rx_buf) => { - let (spi, tx_buf) = transfer.wait(); - (spi, tx_buf, rx_buf) - } - _ => unreachable!(), - }; + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle_async().await; } self.state = State::Idle(spi_dma, tx_buf, rx_buf); @@ -1914,18 +1893,7 @@ pub mod dma { } }; - match &mut self.state { - State::Transferring(transfer) => transfer.wait_for_done().await, - _ => unreachable!(), - }; - - (spi_dma, tx_buf, rx_buf) = match take(&mut self.state) { - State::Transferring(transfer) => { - let (spi, (tx_buf, rx_buf)) = transfer.wait(); - (spi, tx_buf, rx_buf) - } - _ => unreachable!(), - }; + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle_async().await; let bytes_read = rx_buf.read_received_data(read_chunk); assert_eq!(bytes_read, read_chunk.len()); From ecf793164c2072d718f294ec7b7b1d2fa731ca9f Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Thu, 22 Aug 2024 12:50:34 +0100 Subject: [PATCH 03/15] changelog --- esp-hal/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index c3e38928585..0e68dd69778 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Introduce DMA buffer objects (#1856) +- Introduce DMA buffer objects (#1856, #1985) - Added new `Io::new_no_bind_interrupt` constructor (#1861) - Added touch pad support for esp32 (#1873, #1956) - Allow configuration of period updating method for MCPWM timers (#1898) @@ -23,7 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Peripheral driver constructors don't take `InterruptHandler`s anymore. Use `set_interrupt_handler` to explicitly set the interrupt handler now. (#1819) -- Migrate SPI driver to use DMA buffer objects (#1856) +- Migrate SPI driver to use DMA buffer objects (#1856, #1985) - Use the peripheral ref pattern for `OneShotTimer` and `PeriodicTimer` (#1855) - Improve SYSTIMER API (#1871) - DMA buffers now don't require a static lifetime. Make sure to never `mem::forget` an in-progress DMA transfer (consider using `#[deny(clippy::mem_forget)]`) (#1837) From 4b96d9b039dbde0deab5cc0fb7509cc7e3ee791b Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Thu, 22 Aug 2024 13:27:07 +0100 Subject: [PATCH 04/15] rename generic params for consistency --- esp-hal/src/spi/master.rs | 84 +++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 9ef4293c2f9..97ab3c35564 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -1030,24 +1030,24 @@ pub mod dma { } /// A DMA capable SPI instance. - pub struct SpiDma<'d, T, C, M, DmaMode> + pub struct SpiDma<'d, T, C, D, M> where C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, - DmaMode: Mode, + D: DuplexMode, + M: Mode, { pub(crate) spi: PeripheralRef<'d, T>, - pub(crate) channel: Channel<'d, C, DmaMode>, - _mode: PhantomData, + pub(crate) channel: Channel<'d, C, M>, + _mode: PhantomData, } - impl<'d, T, C, M, DmaMode> core::fmt::Debug for SpiDma<'d, T, C, M, DmaMode> + impl<'d, T, C, D, M> core::fmt::Debug for SpiDma<'d, T, C, D, M> where C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, - DmaMode: Mode, + D: DuplexMode, + M: Mode, { /// Formats the `SpiDma` instance for debugging purposes. /// @@ -1058,13 +1058,13 @@ pub mod dma { } } - impl<'d, T, C, M, DmaMode> SpiDma<'d, T, C, M, DmaMode> + impl<'d, T, C, D, M> SpiDma<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, - DmaMode: Mode, + D: DuplexMode, + M: Mode, { /// Sets the interrupt handler /// @@ -1098,23 +1098,23 @@ pub mod dma { } } - impl<'d, T, C, M, DmaMode> crate::private::Sealed for SpiDma<'d, T, C, M, DmaMode> + impl<'d, T, C, D, M> crate::private::Sealed for SpiDma<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, - DmaMode: Mode, + D: DuplexMode, + M: Mode, { } - impl<'d, T, C, M, DmaMode> InterruptConfigurable for SpiDma<'d, T, C, M, DmaMode> + impl<'d, T, C, D, M> InterruptConfigurable for SpiDma<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, - DmaMode: Mode, + D: DuplexMode, + M: Mode, { /// Configures the interrupt handler for the DMA-enabled SPI instance. fn set_interrupt_handler(&mut self, handler: crate::interrupt::InterruptHandler) { @@ -1122,13 +1122,13 @@ pub mod dma { } } - impl<'d, T, C, M, DmaMode> SpiDma<'d, T, C, M, DmaMode> + impl<'d, T, C, D, M> SpiDma<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, - DmaMode: Mode, + D: DuplexMode, + M: Mode, { /// Changes the SPI bus frequency for the DMA-enabled SPI instance. pub fn change_bus_frequency(&mut self, frequency: HertzU32, clocks: &Clocks<'d>) { @@ -1161,14 +1161,14 @@ pub mod dma { /// /// This structure holds references to the SPI instance, DMA buffers, and /// transfer status. - pub struct SpiDmaTransfer<'d, T, C, M, DmaMode, Buf> + pub struct SpiDmaTransfer<'d, T, C, D, M, Buf> where C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, - DmaMode: Mode, + D: DuplexMode, + M: Mode, { - spi_dma: SpiDma<'d, T, C, M, DmaMode>, + spi_dma: SpiDma<'d, T, C, D, M>, dma_buf: Buf, is_rx: bool, is_tx: bool, @@ -1177,16 +1177,16 @@ pub mod dma { tx_future_awaited: bool, } - impl<'d, T, C, M, DmaMode, Buf> SpiDmaTransfer<'d, T, C, M, DmaMode, Buf> + impl<'d, T, C, D, M, Buf> SpiDmaTransfer<'d, T, C, D, M, Buf> where T: Instance, C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, - DmaMode: Mode, + D: DuplexMode, + M: Mode, { fn new( - spi_dma: SpiDma<'d, T, C, M, DmaMode>, + spi_dma: SpiDma<'d, T, C, D, M>, dma_buf: Buf, is_rx: bool, is_tx: bool, @@ -1233,7 +1233,7 @@ pub mod dma { /// /// This method blocks until the transfer is finished and returns the /// `SpiDma` instance and the associated buffer. - pub fn wait(mut self) -> (SpiDma<'d, T, C, M, DmaMode>, Buf) { + pub fn wait(mut self) -> (SpiDma<'d, T, C, D, M>, Buf) { self.spi_dma.spi.flush().ok(); fence(Ordering::Acquire); (self.spi_dma, self.dma_buf) @@ -1241,12 +1241,12 @@ pub mod dma { } #[cfg(feature = "async")] - impl<'d, T, C, M, Buf> SpiDmaTransfer<'d, T, C, M, crate::Async, Buf> + impl<'d, T, C, D, Buf> SpiDmaTransfer<'d, T, C, D, crate::Async, Buf> where T: Instance, C: DmaChannel, C::P: SpiPeripheral, - M: DuplexMode, + D: DuplexMode, { /// Waits for the DMA transfer to complete asynchronously. /// @@ -1266,13 +1266,13 @@ pub mod dma { } } - impl<'d, T, C, M, DmaMode> SpiDma<'d, T, C, M, DmaMode> + impl<'d, T, C, D, M> SpiDma<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - M: IsFullDuplex, - DmaMode: Mode, + D: IsFullDuplex, + M: Mode, { /// Perform a DMA write. /// @@ -1284,7 +1284,7 @@ pub mod dma { pub fn dma_write( mut self, buffer: DmaTxBuf, - ) -> Result, (Error, Self, DmaTxBuf)> + ) -> Result, (Error, Self, DmaTxBuf)> { let bytes_to_write = buffer.len(); if bytes_to_write > MAX_DMA_SIZE { @@ -1312,7 +1312,7 @@ pub mod dma { pub fn dma_read( mut self, buffer: DmaRxBuf, - ) -> Result, (Error, Self, DmaRxBuf)> + ) -> Result, (Error, Self, DmaRxBuf)> { let bytes_to_read = buffer.len(); if bytes_to_read > MAX_DMA_SIZE { @@ -1341,7 +1341,7 @@ pub mod dma { tx_buffer: DmaTxBuf, rx_buffer: DmaRxBuf, ) -> Result< - SpiDmaTransfer<'d, T, C, M, DmaMode, (DmaTxBuf, DmaRxBuf)>, + SpiDmaTransfer<'d, T, C, D, M, (DmaTxBuf, DmaRxBuf)>, (Error, Self, DmaTxBuf, DmaRxBuf), > { let bytes_to_write = tx_buffer.len(); @@ -1379,13 +1379,13 @@ pub mod dma { } } - impl<'d, T, C, M, DmaMode> SpiDma<'d, T, C, M, DmaMode> + impl<'d, T, C, D, M> SpiDma<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - M: IsHalfDuplex, - DmaMode: Mode, + D: IsHalfDuplex, + M: Mode, { /// Perform a half-duplex read operation using DMA. #[allow(clippy::type_complexity)] @@ -1397,7 +1397,7 @@ pub mod dma { address: Address, dummy: u8, buffer: DmaRxBuf, - ) -> Result, (Error, Self, DmaRxBuf)> + ) -> Result, (Error, Self, DmaRxBuf)> { let bytes_to_read = buffer.len(); if bytes_to_read > MAX_DMA_SIZE { @@ -1475,7 +1475,7 @@ pub mod dma { address: Address, dummy: u8, buffer: DmaTxBuf, - ) -> Result, (Error, Self, DmaTxBuf)> + ) -> Result, (Error, Self, DmaTxBuf)> { let bytes_to_write = buffer.len(); if bytes_to_write > MAX_DMA_SIZE { From cc2e53246fe935371095b924d3d56ac2c26f84d2 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Thu, 22 Aug 2024 13:33:54 +0100 Subject: [PATCH 05/15] Add duplex mode to SpiDmaBus --- esp-hal/src/spi/master.rs | 73 +++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 97ab3c35564..ee66e031bc3 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -1152,7 +1152,7 @@ pub mod dma { self, dma_tx_buf: DmaTxBuf, dma_rx_buf: DmaRxBuf, - ) -> SpiDmaBus<'d, T, C, M> { + ) -> SpiDmaBus<'d, T, C, FullDuplexMode, M> { SpiDmaBus::new(self, dma_tx_buf, dma_rx_buf) } } @@ -1185,12 +1185,7 @@ pub mod dma { D: DuplexMode, M: Mode, { - fn new( - spi_dma: SpiDma<'d, T, C, D, M>, - dma_buf: Buf, - is_rx: bool, - is_tx: bool, - ) -> Self { + fn new(spi_dma: SpiDma<'d, T, C, D, M>, dma_buf: Buf, is_rx: bool, is_tx: bool) -> Self { Self { spi_dma, dma_buf, @@ -1284,8 +1279,7 @@ pub mod dma { pub fn dma_write( mut self, buffer: DmaTxBuf, - ) -> Result, (Error, Self, DmaTxBuf)> - { + ) -> Result, (Error, Self, DmaTxBuf)> { let bytes_to_write = buffer.len(); if bytes_to_write > MAX_DMA_SIZE { return Err((Error::MaxDmaTransferSizeExceeded, self, buffer)); @@ -1312,8 +1306,7 @@ pub mod dma { pub fn dma_read( mut self, buffer: DmaRxBuf, - ) -> Result, (Error, Self, DmaRxBuf)> - { + ) -> Result, (Error, Self, DmaRxBuf)> { let bytes_to_read = buffer.len(); if bytes_to_read > MAX_DMA_SIZE { return Err((Error::MaxDmaTransferSizeExceeded, self, buffer)); @@ -1397,8 +1390,7 @@ pub mod dma { address: Address, dummy: u8, buffer: DmaRxBuf, - ) -> Result, (Error, Self, DmaRxBuf)> - { + ) -> Result, (Error, Self, DmaRxBuf)> { let bytes_to_read = buffer.len(); if bytes_to_read > MAX_DMA_SIZE { return Err((Error::MaxDmaTransferSizeExceeded, self, buffer)); @@ -1475,8 +1467,7 @@ pub mod dma { address: Address, dummy: u8, buffer: DmaTxBuf, - ) -> Result, (Error, Self, DmaTxBuf)> - { + ) -> Result, (Error, Self, DmaTxBuf)> { let bytes_to_write = buffer.len(); if bytes_to_write > MAX_DMA_SIZE { return Err((Error::MaxDmaTransferSizeExceeded, self, buffer)); @@ -1545,23 +1536,18 @@ pub mod dma { } #[derive(Default)] - enum State<'d, T, C, M> + enum State<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + D: DuplexMode, M: Mode, { - Idle(SpiDma<'d, T, C, FullDuplexMode, M>, DmaTxBuf, DmaRxBuf), - Reading( - SpiDmaTransfer<'d, T, C, FullDuplexMode, M, DmaRxBuf>, - DmaTxBuf, - ), - Writing( - SpiDmaTransfer<'d, T, C, FullDuplexMode, M, DmaTxBuf>, - DmaRxBuf, - ), - Transferring(SpiDmaTransfer<'d, T, C, FullDuplexMode, M, (DmaTxBuf, DmaRxBuf)>), + Idle(SpiDma<'d, T, C, D, M>, DmaTxBuf, DmaRxBuf), + Reading(SpiDmaTransfer<'d, T, C, D, M, DmaRxBuf>, DmaTxBuf), + Writing(SpiDmaTransfer<'d, T, C, D, M, DmaTxBuf>, DmaRxBuf), + Transferring(SpiDmaTransfer<'d, T, C, D, M, (DmaTxBuf, DmaRxBuf)>), #[default] InUse, } @@ -1570,27 +1556,29 @@ pub mod dma { /// /// This structure is responsible for managing SPI transfers using DMA /// buffers. - pub struct SpiDmaBus<'d, T, C, M> + pub struct SpiDmaBus<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + D: DuplexMode, M: Mode, { - state: State<'d, T, C, M>, + state: State<'d, T, C, D, M>, } - impl<'d, T, C, M> SpiDmaBus<'d, T, C, M> + impl<'d, T, C, D, M> SpiDmaBus<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + D: DuplexMode, M: Mode, { /// Creates a new `SpiDmaBus` with the specified SPI instance and DMA /// buffers. pub fn new( - spi: SpiDma<'d, T, C, FullDuplexMode, M>, + spi: SpiDma<'d, T, C, D, M>, dma_tx_buf: DmaTxBuf, dma_rx_buf: DmaRxBuf, ) -> Self { @@ -1599,7 +1587,7 @@ pub mod dma { } } - fn wait_for_idle(&mut self) -> (SpiDma<'d, T, C, FullDuplexMode, M>, DmaTxBuf, DmaRxBuf) { + fn wait_for_idle(&mut self) -> (SpiDma<'d, T, C, D, M>, DmaTxBuf, DmaRxBuf) { match core::mem::take(&mut self.state) { State::Idle(spi, tx_buf, rx_buf) => (spi, tx_buf, rx_buf), State::Reading(transfer, tx_buf) => { @@ -1617,7 +1605,16 @@ pub mod dma { State::InUse => unreachable!(), } } + } + impl<'d, T, C, D, M> SpiDmaBus<'d, T, C, D, M> + where + T: InstanceDma, + C: DmaChannel, + C::P: SpiPeripheral, + D: IsFullDuplex, + M: Mode, + { /// Reads data from the SPI bus using DMA. pub fn read(&mut self, words: &mut [u8]) -> Result<(), Error> { let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); @@ -1735,7 +1732,8 @@ pub mod dma { } #[cfg(feature = "embedded-hal-02")] - impl<'d, T, C> embedded_hal_02::blocking::spi::Transfer for SpiDmaBus<'d, T, C, crate::Blocking> + impl<'d, T, C> embedded_hal_02::blocking::spi::Transfer + for SpiDmaBus<'d, T, C, FullDuplexMode, crate::Blocking> where T: InstanceDma, C: DmaChannel, @@ -1750,7 +1748,8 @@ pub mod dma { } #[cfg(feature = "embedded-hal-02")] - impl<'d, T, C> embedded_hal_02::blocking::spi::Write for SpiDmaBus<'d, T, C, crate::Blocking> + impl<'d, T, C> embedded_hal_02::blocking::spi::Write + for SpiDmaBus<'d, T, C, FullDuplexMode, crate::Blocking> where T: InstanceDma, C: DmaChannel, @@ -1771,7 +1770,7 @@ pub mod dma { use super::*; - impl<'d, T, C> SpiDmaBus<'d, T, C, crate::Async> + impl<'d, T, C> SpiDmaBus<'d, T, C, FullDuplexMode, crate::Async> where T: InstanceDma, C: DmaChannel, @@ -1961,7 +1960,7 @@ pub mod dma { } } - impl<'d, T, C> embedded_hal_async::spi::SpiBus for SpiDmaBus<'d, T, C, crate::Async> + impl<'d, T, C> embedded_hal_async::spi::SpiBus for SpiDmaBus<'d, T, C, FullDuplexMode, crate::Async> where T: InstanceDma, C: DmaChannel, @@ -1995,7 +1994,7 @@ pub mod dma { use super::*; - impl<'d, T, C, M> ErrorType for SpiDmaBus<'d, T, C, M> + impl<'d, T, C, M> ErrorType for SpiDmaBus<'d, T, C, FullDuplexMode, M> where T: InstanceDma, C: DmaChannel, @@ -2005,7 +2004,7 @@ pub mod dma { type Error = Error; } - impl<'d, T, C, M> SpiBus for SpiDmaBus<'d, T, C, M> + impl<'d, T, C, M> SpiBus for SpiDmaBus<'d, T, C, FullDuplexMode, M> where T: InstanceDma, C: DmaChannel, From 674f3c89420fabef338e255bae07fd0a00353924 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Thu, 22 Aug 2024 14:02:29 +0100 Subject: [PATCH 06/15] implement HalfDuplexReadWrite for SpiDmaBus --- esp-hal/src/dma/mod.rs | 20 +++++++ esp-hal/src/spi/master.rs | 79 ++++++++++++++++++++++++- esp-hal/src/spi/mod.rs | 2 +- hil-test/tests/spi_half_duplex_read.rs | 61 +++++++++++++++++++ hil-test/tests/spi_half_duplex_write.rs | 64 ++++++++++++++++++++ 5 files changed, 223 insertions(+), 3 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 270f8236e45..a514fc3e849 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -1947,6 +1947,16 @@ impl DmaTxBuf { Ok(buf) } + /// Create an empty DmaTxBuf. + /// + /// This has no ability to transmit data. + pub fn empty() -> Self { + Self { + descriptors: &mut [], + buffer: &mut [], + } + } + /// Consume the buf, returning the descriptors and buffer. pub fn split(self) -> (&'static mut [DmaDescriptor], &'static mut [u8]) { (self.descriptors, self.buffer) @@ -2082,6 +2092,16 @@ impl DmaRxBuf { Ok(buf) } + /// Create an empty DmaRxBuf. + /// + /// This has no ability to recieve data. + pub fn empty() -> Self { + Self { + descriptors: &mut [], + buffer: &mut [], + } + } + /// Consume the buf, returning the descriptors and buffer. pub fn split(self) -> (&'static mut [DmaDescriptor], &'static mut [u8]) { (self.descriptors, self.buffer) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index ee66e031bc3..6da41e5a745 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -123,6 +123,7 @@ const MAX_DMA_SIZE: usize = 32736; /// /// Used to define specific commands sent over the SPI bus. /// Can be [Command::None] if command phase should be suppressed. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum Command { /// No command is sent. None, @@ -236,6 +237,7 @@ impl Command { /// /// This can be used to specify the address phase of SPI transactions. /// Can be [Address::None] if address phase should be suppressed. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum Address { /// No address phase. None, @@ -1136,11 +1138,12 @@ pub mod dma { } } - impl<'d, T, C, M> SpiDma<'d, T, C, FullDuplexMode, M> + impl<'d, T, C, D, M> SpiDma<'d, T, C, D, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, + D: DuplexMode, M: Mode, { /// Configures the DMA buffers for the SPI instance. @@ -1152,7 +1155,7 @@ pub mod dma { self, dma_tx_buf: DmaTxBuf, dma_rx_buf: DmaRxBuf, - ) -> SpiDmaBus<'d, T, C, FullDuplexMode, M> { + ) -> SpiDmaBus<'d, T, C, D, M> { SpiDmaBus::new(self, dma_tx_buf, dma_rx_buf) } } @@ -1731,6 +1734,78 @@ pub mod dma { } } + impl<'d, T, C, D, M> HalfDuplexReadWrite for SpiDmaBus<'d, T, C, D, M> + where + T: InstanceDma, + C: DmaChannel, + C::P: SpiPeripheral, + D: IsHalfDuplex, + M: Mode, + { + type Error = super::Error; + + /// Half-duplex read. + fn read( + &mut self, + data_mode: SpiDataMode, + cmd: Command, + address: Address, + dummy: u8, + buffer: &mut [u8], + ) -> Result<(), Self::Error> { + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); + + for chunk in buffer.chunks_mut(rx_buf.capacity()) { + rx_buf.set_length(chunk.len()); + + match spi_dma.read(data_mode, cmd, address, dummy, rx_buf) { + Ok(transfer) => self.state = State::Reading(transfer, tx_buf), + Err((e, spi, rx)) => { + self.state = State::Idle(spi, tx_buf, rx); + return Err(e); + } + } + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); + + let bytes_read = rx_buf.read_received_data(chunk); + debug_assert_eq!(bytes_read, chunk.len()); + } + + self.state = State::Idle(spi_dma, tx_buf, rx_buf); + + Ok(()) + } + + /// Half-duplex write. + fn write( + &mut self, + data_mode: SpiDataMode, + cmd: Command, + address: Address, + dummy: u8, + buffer: &[u8], + ) -> Result<(), Self::Error> { + let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); + + for chunk in buffer.chunks(tx_buf.capacity()) { + tx_buf.fill(chunk); + + match spi_dma.write(data_mode, cmd, address, dummy, tx_buf) { + Ok(transfer) => self.state = State::Writing(transfer, rx_buf), + Err((e, spi, tx)) => { + self.state = State::Idle(spi, tx, rx_buf); + return Err(e); + } + } + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); + } + + self.state = State::Idle(spi_dma, tx_buf, rx_buf); + + Ok(()) + } + } + #[cfg(feature = "embedded-hal-02")] impl<'d, T, C> embedded_hal_02::blocking::spi::Transfer for SpiDmaBus<'d, T, C, FullDuplexMode, crate::Blocking> diff --git a/esp-hal/src/spi/mod.rs b/esp-hal/src/spi/mod.rs index 9f30272438a..50b4e14d061 100644 --- a/esp-hal/src/spi/mod.rs +++ b/esp-hal/src/spi/mod.rs @@ -86,7 +86,7 @@ pub trait IsFullDuplex: DuplexMode {} pub trait IsHalfDuplex: DuplexMode {} /// SPI data mode -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum SpiDataMode { /// `Single` Data Mode - 1 bit, 2 wires. diff --git a/hil-test/tests/spi_half_duplex_read.rs b/hil-test/tests/spi_half_duplex_read.rs index ab392a520b3..51c52fc54a2 100644 --- a/hil-test/tests/spi_half_duplex_read.rs +++ b/hil-test/tests/spi_half_duplex_read.rs @@ -129,4 +129,65 @@ mod tests { assert_eq!(dma_rx_buf.as_slice(), &[0xFF; DMA_BUFFER_SIZE]); } + + #[test] + #[timeout(3)] + fn test_spidmabus_reads_correctly_from_gpio_pin() { + const DMA_BUFFER_SIZE: usize = 4; + + let peripherals = Peripherals::take(); + let system = SystemControl::new(peripherals.SYSTEM); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); + let sclk = io.pins.gpio0; + let miso = io.pins.gpio2; + + let mut miso_mirror = Output::new(io.pins.gpio3, Level::High); + + let dma = Dma::new(peripherals.DMA); + + #[cfg(any(feature = "esp32", feature = "esp32s2"))] + let dma_channel = dma.spi2channel; + #[cfg(not(any(feature = "esp32", feature = "esp32s2")))] + let dma_channel = dma.channel0; + + let (buffer, descriptors, _, _) = dma_buffers!(DMA_BUFFER_SIZE, 0); + let dma_rx_buf = DmaRxBuf::new(descriptors, buffer).unwrap(); + + let mut spi = Spi::new_half_duplex(peripherals.SPI2, 100.kHz(), SpiMode::Mode0, &clocks) + .with_sck(sclk) + .with_miso(miso) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)) + .with_buffers(DmaTxBuf::empty(), dma_rx_buf); + + // SPI should read '0's from the MISO pin + miso_mirror.set_low(); + + let mut buffer = [0xAA; DMA_BUFFER_SIZE]; + spi.read( + SpiDataMode::Single, + Command::None, + Address::None, + 0, + &mut buffer, + ) + .unwrap(); + + assert_eq!(buffer.as_slice(), &[0x00; DMA_BUFFER_SIZE]); + + // SPI should read '1's from the MISO pin + miso_mirror.set_high(); + + spi.read( + SpiDataMode::Single, + Command::None, + Address::None, + 0, + &mut buffer, + ) + .unwrap(); + + assert_eq!(buffer.as_slice(), &[0xFF; DMA_BUFFER_SIZE]); + } } diff --git a/hil-test/tests/spi_half_duplex_write.rs b/hil-test/tests/spi_half_duplex_write.rs index 1acd34b17ce..6b7364f965e 100644 --- a/hil-test/tests/spi_half_duplex_write.rs +++ b/hil-test/tests/spi_half_duplex_write.rs @@ -143,4 +143,68 @@ mod tests { assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _); } + + #[test] + #[timeout(3)] + fn test_spidmabus_writes_are_correctly_by_pcnt() { + const DMA_BUFFER_SIZE: usize = 4; + + let peripherals = Peripherals::take(); + let system = SystemControl::new(peripherals.SYSTEM); + let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); + + let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); + let pcnt = Pcnt::new(peripherals.PCNT); + let dma = Dma::new(peripherals.DMA); + + let sclk = io.pins.gpio0; + let mosi = io.pins.gpio2; + let mosi_mirror = io.pins.gpio3; + + #[cfg(any(feature = "esp32", feature = "esp32s2"))] + let dma_channel = dma.spi2channel; + #[cfg(not(any(feature = "esp32", feature = "esp32s2")))] + let dma_channel = dma.channel0; + + let (buffer, descriptors, _, _) = dma_buffers!(DMA_BUFFER_SIZE, 0); + let dma_tx_buf = DmaTxBuf::new(descriptors, buffer).unwrap(); + + let mut spi = Spi::new_half_duplex(peripherals.SPI2, 100.kHz(), SpiMode::Mode0, &clocks) + .with_sck(sclk) + .with_mosi(mosi) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)) + .with_buffers(dma_tx_buf, DmaRxBuf::empty()); + + let unit = pcnt.unit0; + unit.channel0.set_edge_signal(PcntSource::from_pin( + mosi_mirror, + PcntInputConfig { pull: Pull::Down }, + )); + unit.channel0 + .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); + + let buffer = [0b0110_1010; DMA_BUFFER_SIZE]; + // Write the buffer where each byte has 3 pos edges. + spi.write( + SpiDataMode::Single, + Command::None, + Address::None, + 0, + &buffer, + ) + .unwrap(); + + assert_eq!(unit.get_value(), (3 * DMA_BUFFER_SIZE) as _); + + spi.write( + SpiDataMode::Single, + Command::None, + Address::None, + 0, + &buffer, + ) + .unwrap(); + + assert_eq!(unit.get_value(), (6 * DMA_BUFFER_SIZE) as _); + } } From fad8efa8a6287ec1d09b7fc8de831c08e85967e4 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Thu, 22 Aug 2024 14:07:41 +0100 Subject: [PATCH 07/15] Docs on new async APIs --- esp-hal/src/spi/master.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 6da41e5a745..c30a8c2effb 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -1883,7 +1883,8 @@ pub mod dma { } } - async fn read_async(&mut self, words: &mut [u8]) -> Result<(), super::Error> { + /// Fill the given buffer with data from the bus. + pub async fn read_async(&mut self, words: &mut [u8]) -> Result<(), super::Error> { // Get previous transfer. let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle_async().await; @@ -1911,7 +1912,8 @@ pub mod dma { Ok(()) } - async fn write_async(&mut self, words: &[u8]) -> Result<(), super::Error> { + /// Transmit the given buffer to the bus. + pub async fn write_async(&mut self, words: &[u8]) -> Result<(), super::Error> { // Get previous transfer. let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle_async().await; @@ -1936,7 +1938,9 @@ pub mod dma { Ok(()) } - async fn transfer_async( + /// Transfer by writing out a buffer and reading the response from + /// the bus into another buffer. + pub async fn transfer_async( &mut self, read: &mut [u8], write: &[u8], @@ -1984,7 +1988,9 @@ pub mod dma { } } - async fn transfer_in_place_async( + /// Transfer by writing out a buffer and reading the response from + /// the bus into the same buffer. + pub async fn transfer_in_place_async( &mut self, words: &mut [u8], ) -> Result<(), super::Error> { @@ -2027,7 +2033,8 @@ pub mod dma { Ok(()) } - async fn flush_async(&mut self) -> Result<(), super::Error> { + /// Flush any pending data in the SPI peripheral. + pub async fn flush_async(&mut self) -> Result<(), super::Error> { // Get previous transfer. let (spi_dma, tx_buf, rx_buf) = self.wait_for_idle_async().await; self.state = State::Idle(spi_dma, tx_buf, rx_buf); From 75c72d4c8b9b7ce538208e4fd93eb7a47b646813 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Thu, 22 Aug 2024 16:01:28 +0100 Subject: [PATCH 08/15] Limit half duplex transfers to the capacity of the DmaBuf --- esp-hal/src/dma/mod.rs | 20 ----------- esp-hal/src/spi/master.rs | 45 +++++++++++++------------ hil-test/tests/spi_half_duplex_read.rs | 5 +-- hil-test/tests/spi_half_duplex_write.rs | 5 +-- 4 files changed, 30 insertions(+), 45 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index a514fc3e849..270f8236e45 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -1947,16 +1947,6 @@ impl DmaTxBuf { Ok(buf) } - /// Create an empty DmaTxBuf. - /// - /// This has no ability to transmit data. - pub fn empty() -> Self { - Self { - descriptors: &mut [], - buffer: &mut [], - } - } - /// Consume the buf, returning the descriptors and buffer. pub fn split(self) -> (&'static mut [DmaDescriptor], &'static mut [u8]) { (self.descriptors, self.buffer) @@ -2092,16 +2082,6 @@ impl DmaRxBuf { Ok(buf) } - /// Create an empty DmaRxBuf. - /// - /// This has no ability to recieve data. - pub fn empty() -> Self { - Self { - descriptors: &mut [], - buffer: &mut [], - } - } - /// Consume the buf, returning the descriptors and buffer. pub fn split(self) -> (&'static mut [DmaDescriptor], &'static mut [u8]) { (self.descriptors, self.buffer) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index c30a8c2effb..63be41d968c 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -67,6 +67,7 @@ use fugit::HertzU32; use procmacros::ram; use super::{ + DmaError, DuplexMode, Error, FullDuplexMode, @@ -1754,22 +1755,23 @@ pub mod dma { buffer: &mut [u8], ) -> Result<(), Self::Error> { let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); + if buffer.len() > rx_buf.capacity() { + return Err(super::Error::DmaError(DmaError::Overflow)); + } - for chunk in buffer.chunks_mut(rx_buf.capacity()) { - rx_buf.set_length(chunk.len()); + rx_buf.set_length(buffer.len()); - match spi_dma.read(data_mode, cmd, address, dummy, rx_buf) { - Ok(transfer) => self.state = State::Reading(transfer, tx_buf), - Err((e, spi, rx)) => { - self.state = State::Idle(spi, tx_buf, rx); - return Err(e); - } + match spi_dma.read(data_mode, cmd, address, dummy, rx_buf) { + Ok(transfer) => self.state = State::Reading(transfer, tx_buf), + Err((e, spi, rx)) => { + self.state = State::Idle(spi, tx_buf, rx); + return Err(e); } - (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); - - let bytes_read = rx_buf.read_received_data(chunk); - debug_assert_eq!(bytes_read, chunk.len()); } + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); + + let bytes_read = rx_buf.read_received_data(buffer); + debug_assert_eq!(bytes_read, buffer.len()); self.state = State::Idle(spi_dma, tx_buf, rx_buf); @@ -1786,19 +1788,20 @@ pub mod dma { buffer: &[u8], ) -> Result<(), Self::Error> { let (mut spi_dma, mut tx_buf, mut rx_buf) = self.wait_for_idle(); + if buffer.len() > tx_buf.capacity() { + return Err(super::Error::DmaError(DmaError::Overflow)); + } - for chunk in buffer.chunks(tx_buf.capacity()) { - tx_buf.fill(chunk); + tx_buf.fill(buffer); - match spi_dma.write(data_mode, cmd, address, dummy, tx_buf) { - Ok(transfer) => self.state = State::Writing(transfer, rx_buf), - Err((e, spi, tx)) => { - self.state = State::Idle(spi, tx, rx_buf); - return Err(e); - } + match spi_dma.write(data_mode, cmd, address, dummy, tx_buf) { + Ok(transfer) => self.state = State::Writing(transfer, rx_buf), + Err((e, spi, tx)) => { + self.state = State::Idle(spi, tx, rx_buf); + return Err(e); } - (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); } + (spi_dma, tx_buf, rx_buf) = self.wait_for_idle(); self.state = State::Idle(spi_dma, tx_buf, rx_buf); diff --git a/hil-test/tests/spi_half_duplex_read.rs b/hil-test/tests/spi_half_duplex_read.rs index 51c52fc54a2..3f28106eb00 100644 --- a/hil-test/tests/spi_half_duplex_read.rs +++ b/hil-test/tests/spi_half_duplex_read.rs @@ -152,14 +152,15 @@ mod tests { #[cfg(not(any(feature = "esp32", feature = "esp32s2")))] let dma_channel = dma.channel0; - let (buffer, descriptors, _, _) = dma_buffers!(DMA_BUFFER_SIZE, 0); + let (buffer, descriptors, tx, txd) = dma_buffers!(DMA_BUFFER_SIZE, 1); let dma_rx_buf = DmaRxBuf::new(descriptors, buffer).unwrap(); + let dma_tx_buf = DmaTxBuf::new(txd, tx).unwrap(); let mut spi = Spi::new_half_duplex(peripherals.SPI2, 100.kHz(), SpiMode::Mode0, &clocks) .with_sck(sclk) .with_miso(miso) .with_dma(dma_channel.configure(false, DmaPriority::Priority0)) - .with_buffers(DmaTxBuf::empty(), dma_rx_buf); + .with_buffers(dma_tx_buf, dma_rx_buf); // SPI should read '0's from the MISO pin miso_mirror.set_low(); diff --git a/hil-test/tests/spi_half_duplex_write.rs b/hil-test/tests/spi_half_duplex_write.rs index 6b7364f965e..12a0923bef4 100644 --- a/hil-test/tests/spi_half_duplex_write.rs +++ b/hil-test/tests/spi_half_duplex_write.rs @@ -166,14 +166,15 @@ mod tests { #[cfg(not(any(feature = "esp32", feature = "esp32s2")))] let dma_channel = dma.channel0; - let (buffer, descriptors, _, _) = dma_buffers!(DMA_BUFFER_SIZE, 0); + let (buffer, descriptors, rx, rxd) = dma_buffers!(DMA_BUFFER_SIZE, 1); let dma_tx_buf = DmaTxBuf::new(descriptors, buffer).unwrap(); + let dma_rx_buf = DmaRxBuf::new(rxd, rx).unwrap(); let mut spi = Spi::new_half_duplex(peripherals.SPI2, 100.kHz(), SpiMode::Mode0, &clocks) .with_sck(sclk) .with_mosi(mosi) .with_dma(dma_channel.configure(false, DmaPriority::Priority0)) - .with_buffers(dma_tx_buf, DmaRxBuf::empty()); + .with_buffers(dma_tx_buf, dma_rx_buf); let unit = pcnt.unit0; unit.channel0.set_edge_signal(PcntSource::from_pin( From d126cc200dccb26ce671b47329c3c08dfdfc8869 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Fri, 23 Aug 2024 14:27:54 +0100 Subject: [PATCH 09/15] docs --- esp-hal/src/spi/master.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 63be41d968c..eef7e901fc6 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -1033,6 +1033,12 @@ pub mod dma { } /// A DMA capable SPI instance. + /// + /// Using `SpiDma` is not recommended unless you wish + /// to manage buffers yourself. It's recommended to use + /// [`SpiDmaBus`] via `with_buffers` to get access + /// to a DMA capable SPI bus that implements the + /// embedded-hal traits. pub struct SpiDma<'d, T, C, D, M> where C: DmaChannel, @@ -1559,7 +1565,7 @@ pub mod dma { /// A DMA-capable SPI bus. /// /// This structure is responsible for managing SPI transfers using DMA - /// buffers. + /// buffers. Implements the embedded-hal traits. pub struct SpiDmaBus<'d, T, C, D, M> where T: InstanceDma, From 70ce61e64fc187c11b0c0779c22bb16662edc2cf Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Tue, 27 Aug 2024 12:50:14 +0100 Subject: [PATCH 10/15] rebase tests --- hil-test/tests/spi_half_duplex_read.rs | 33 +++++-------------------- hil-test/tests/spi_half_duplex_write.rs | 33 +++++-------------------- 2 files changed, 12 insertions(+), 54 deletions(-) diff --git a/hil-test/tests/spi_half_duplex_read.rs b/hil-test/tests/spi_half_duplex_read.rs index 3f28106eb00..330c8ff8054 100644 --- a/hil-test/tests/spi_half_duplex_read.rs +++ b/hil-test/tests/spi_half_duplex_read.rs @@ -15,13 +15,13 @@ use esp_hal::{ clock::ClockControl, - dma::{Dma, DmaPriority, DmaRxBuf}, + dma::{Dma, DmaPriority, DmaRxBuf, DmaTxBuf}, dma_buffers, gpio::{GpioPin, Io, Level, Output}, peripherals::{Peripherals, SPI2}, prelude::*, spi::{ - master::{dma::SpiDma, Address, Command, Spi}, + master::{dma::SpiDma, Address, Command, HalfDuplexReadWrite, Spi}, HalfDuplexMode, SpiDataMode, SpiMode, @@ -132,38 +132,17 @@ mod tests { #[test] #[timeout(3)] - fn test_spidmabus_reads_correctly_from_gpio_pin() { + fn test_spidmabus_reads_correctly_from_gpio_pin(mut ctx: Context) { const DMA_BUFFER_SIZE: usize = 4; - let peripherals = Peripherals::take(); - let system = SystemControl::new(peripherals.SYSTEM); - let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); - - let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); - let sclk = io.pins.gpio0; - let miso = io.pins.gpio2; - - let mut miso_mirror = Output::new(io.pins.gpio3, Level::High); - - let dma = Dma::new(peripherals.DMA); - - #[cfg(any(feature = "esp32", feature = "esp32s2"))] - let dma_channel = dma.spi2channel; - #[cfg(not(any(feature = "esp32", feature = "esp32s2")))] - let dma_channel = dma.channel0; - let (buffer, descriptors, tx, txd) = dma_buffers!(DMA_BUFFER_SIZE, 1); let dma_rx_buf = DmaRxBuf::new(descriptors, buffer).unwrap(); let dma_tx_buf = DmaTxBuf::new(txd, tx).unwrap(); - let mut spi = Spi::new_half_duplex(peripherals.SPI2, 100.kHz(), SpiMode::Mode0, &clocks) - .with_sck(sclk) - .with_miso(miso) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)) - .with_buffers(dma_tx_buf, dma_rx_buf); + let mut spi = ctx.spi.with_buffers(dma_tx_buf, dma_rx_buf); // SPI should read '0's from the MISO pin - miso_mirror.set_low(); + ctx.miso_mirror.set_low(); let mut buffer = [0xAA; DMA_BUFFER_SIZE]; spi.read( @@ -178,7 +157,7 @@ mod tests { assert_eq!(buffer.as_slice(), &[0x00; DMA_BUFFER_SIZE]); // SPI should read '1's from the MISO pin - miso_mirror.set_high(); + ctx.miso_mirror.set_high(); spi.read( SpiDataMode::Single, diff --git a/hil-test/tests/spi_half_duplex_write.rs b/hil-test/tests/spi_half_duplex_write.rs index 12a0923bef4..22e363d25b3 100644 --- a/hil-test/tests/spi_half_duplex_write.rs +++ b/hil-test/tests/spi_half_duplex_write.rs @@ -15,7 +15,7 @@ use esp_hal::{ clock::ClockControl, - dma::{Dma, DmaPriority, DmaTxBuf}, + dma::{Dma, DmaPriority, DmaRxBuf, DmaTxBuf}, dma_buffers, gpio::{GpioPin, Io, Pull}, pcnt::{ @@ -26,7 +26,7 @@ use esp_hal::{ peripherals::{Peripherals, SPI2}, prelude::*, spi::{ - master::{dma::SpiDma, Address, Command, Spi}, + master::{dma::SpiDma, Address, Command, HalfDuplexReadWrite, Spi}, HalfDuplexMode, SpiDataMode, SpiMode, @@ -146,39 +146,18 @@ mod tests { #[test] #[timeout(3)] - fn test_spidmabus_writes_are_correctly_by_pcnt() { + fn test_spidmabus_writes_are_correctly_by_pcnt(ctx: Context) { const DMA_BUFFER_SIZE: usize = 4; - let peripherals = Peripherals::take(); - let system = SystemControl::new(peripherals.SYSTEM); - let clocks = ClockControl::boot_defaults(system.clock_control).freeze(); - - let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); - let pcnt = Pcnt::new(peripherals.PCNT); - let dma = Dma::new(peripherals.DMA); - - let sclk = io.pins.gpio0; - let mosi = io.pins.gpio2; - let mosi_mirror = io.pins.gpio3; - - #[cfg(any(feature = "esp32", feature = "esp32s2"))] - let dma_channel = dma.spi2channel; - #[cfg(not(any(feature = "esp32", feature = "esp32s2")))] - let dma_channel = dma.channel0; - let (buffer, descriptors, rx, rxd) = dma_buffers!(DMA_BUFFER_SIZE, 1); let dma_tx_buf = DmaTxBuf::new(descriptors, buffer).unwrap(); let dma_rx_buf = DmaRxBuf::new(rxd, rx).unwrap(); - let mut spi = Spi::new_half_duplex(peripherals.SPI2, 100.kHz(), SpiMode::Mode0, &clocks) - .with_sck(sclk) - .with_mosi(mosi) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)) - .with_buffers(dma_tx_buf, dma_rx_buf); + let unit = ctx.pcnt_unit; + let mut spi = ctx.spi.with_buffers(dma_tx_buf, dma_rx_buf); - let unit = pcnt.unit0; unit.channel0.set_edge_signal(PcntSource::from_pin( - mosi_mirror, + ctx.mosi_mirror, PcntInputConfig { pull: Pull::Down }, )); unit.channel0 From a1e111dce16aa414e9508dfccc06818c0f09cffd Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Tue, 27 Aug 2024 12:52:58 +0100 Subject: [PATCH 11/15] address review comments --- esp-hal/src/spi/master.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index eef7e901fc6..bcb6330ed6a 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -100,6 +100,7 @@ pub mod prelude { /// Enumeration of possible SPI interrupt events. #[cfg(not(any(esp32, esp32s2)))] #[derive(EnumSetType)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum SpiInterrupt { /// Indicates that the SPI transaction has completed successfully. /// @@ -125,6 +126,7 @@ const MAX_DMA_SIZE: usize = 32736; /// Used to define specific commands sent over the SPI bus. /// Can be [Command::None] if command phase should be suppressed. #[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Command { /// No command is sent. None, @@ -239,6 +241,7 @@ impl Command { /// This can be used to specify the address phase of SPI transactions. /// Can be [Address::None] if address phase should be suppressed. #[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum Address { /// No address phase. None, @@ -1565,7 +1568,7 @@ pub mod dma { /// A DMA-capable SPI bus. /// /// This structure is responsible for managing SPI transfers using DMA - /// buffers. Implements the embedded-hal traits. + /// buffers. pub struct SpiDmaBus<'d, T, C, D, M> where T: InstanceDma, From 549cb57e9258cb3c4597863cd6b695d5bc998e77 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Tue, 27 Aug 2024 13:00:48 +0100 Subject: [PATCH 12/15] remove duplex traits from spi --- esp-hal/CHANGELOG.md | 1 + esp-hal/src/spi/master.rs | 49 ++++++++++++++++----------------------- esp-hal/src/spi/mod.rs | 10 +++----- 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 0e68dd69778..61c68eed5f6 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -51,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `AesFlavour` trait no longer has the `ENCRYPT_MODE`/`DECRYPT_MODE` associated constants (#1849) - Removed `FlashSafeDma` (#1856) - Remove redundant WithDmaSpi traits (#1975) +- `IsFullDuplex` and `IsHalfDuplex` traits (#1985) ## [0.19.0] - 2024-07-15 diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index bcb6330ed6a..1bdc519de1a 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -72,8 +72,6 @@ use super::{ Error, FullDuplexMode, HalfDuplexMode, - IsFullDuplex, - IsHalfDuplex, SpiBitOrder, SpiDataMode, SpiMode, @@ -464,10 +462,9 @@ pub struct Spi<'d, T, M> { _mode: PhantomData, } -impl<'d, T, M> Spi<'d, T, M> +impl<'d, T> Spi<'d, T, FullDuplexMode> where T: Instance, - M: IsFullDuplex, { /// Read bytes from SPI. /// @@ -860,10 +857,9 @@ where } } -impl HalfDuplexReadWrite for Spi<'_, T, M> +impl HalfDuplexReadWrite for Spi<'_, T, HalfDuplexMode> where T: Instance, - M: IsHalfDuplex, { type Error = Error; @@ -908,10 +904,9 @@ where } #[cfg(feature = "embedded-hal-02")] -impl embedded_hal_02::spi::FullDuplex for Spi<'_, T, M> +impl embedded_hal_02::spi::FullDuplex for Spi<'_, T, FullDuplexMode> where T: Instance, - M: IsFullDuplex, { type Error = Error; @@ -925,10 +920,9 @@ where } #[cfg(feature = "embedded-hal-02")] -impl embedded_hal_02::blocking::spi::Transfer for Spi<'_, T, M> +impl embedded_hal_02::blocking::spi::Transfer for Spi<'_, T, FullDuplexMode> where T: Instance, - M: IsFullDuplex, { type Error = Error; @@ -938,10 +932,9 @@ where } #[cfg(feature = "embedded-hal-02")] -impl embedded_hal_02::blocking::spi::Write for Spi<'_, T, M> +impl embedded_hal_02::blocking::spi::Write for Spi<'_, T, FullDuplexMode> where T: Instance, - M: IsFullDuplex, { type Error = Error; @@ -1274,12 +1267,11 @@ pub mod dma { } } - impl<'d, T, C, D, M> SpiDma<'d, T, C, D, M> + impl<'d, T, C, M> SpiDma<'d, T, C, FullDuplexMode, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - D: IsFullDuplex, M: Mode, { /// Perform a DMA write. @@ -1292,7 +1284,8 @@ pub mod dma { pub fn dma_write( mut self, buffer: DmaTxBuf, - ) -> Result, (Error, Self, DmaTxBuf)> { + ) -> Result, (Error, Self, DmaTxBuf)> + { let bytes_to_write = buffer.len(); if bytes_to_write > MAX_DMA_SIZE { return Err((Error::MaxDmaTransferSizeExceeded, self, buffer)); @@ -1319,7 +1312,8 @@ pub mod dma { pub fn dma_read( mut self, buffer: DmaRxBuf, - ) -> Result, (Error, Self, DmaRxBuf)> { + ) -> Result, (Error, Self, DmaRxBuf)> + { let bytes_to_read = buffer.len(); if bytes_to_read > MAX_DMA_SIZE { return Err((Error::MaxDmaTransferSizeExceeded, self, buffer)); @@ -1347,7 +1341,7 @@ pub mod dma { tx_buffer: DmaTxBuf, rx_buffer: DmaRxBuf, ) -> Result< - SpiDmaTransfer<'d, T, C, D, M, (DmaTxBuf, DmaRxBuf)>, + SpiDmaTransfer<'d, T, C, FullDuplexMode, M, (DmaTxBuf, DmaRxBuf)>, (Error, Self, DmaTxBuf, DmaRxBuf), > { let bytes_to_write = tx_buffer.len(); @@ -1385,12 +1379,11 @@ pub mod dma { } } - impl<'d, T, C, D, M> SpiDma<'d, T, C, D, M> + impl<'d, T, C, M> SpiDma<'d, T, C, HalfDuplexMode, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - D: IsHalfDuplex, M: Mode, { /// Perform a half-duplex read operation using DMA. @@ -1403,7 +1396,8 @@ pub mod dma { address: Address, dummy: u8, buffer: DmaRxBuf, - ) -> Result, (Error, Self, DmaRxBuf)> { + ) -> Result, (Error, Self, DmaRxBuf)> + { let bytes_to_read = buffer.len(); if bytes_to_read > MAX_DMA_SIZE { return Err((Error::MaxDmaTransferSizeExceeded, self, buffer)); @@ -1480,7 +1474,8 @@ pub mod dma { address: Address, dummy: u8, buffer: DmaTxBuf, - ) -> Result, (Error, Self, DmaTxBuf)> { + ) -> Result, (Error, Self, DmaTxBuf)> + { let bytes_to_write = buffer.len(); if bytes_to_write > MAX_DMA_SIZE { return Err((Error::MaxDmaTransferSizeExceeded, self, buffer)); @@ -1620,12 +1615,11 @@ pub mod dma { } } - impl<'d, T, C, D, M> SpiDmaBus<'d, T, C, D, M> + impl<'d, T, C, M> SpiDmaBus<'d, T, C, FullDuplexMode, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - D: IsFullDuplex, M: Mode, { /// Reads data from the SPI bus using DMA. @@ -1744,12 +1738,11 @@ pub mod dma { } } - impl<'d, T, C, D, M> HalfDuplexReadWrite for SpiDmaBus<'d, T, C, D, M> + impl<'d, T, C, M> HalfDuplexReadWrite for SpiDmaBus<'d, T, C, HalfDuplexMode, M> where T: InstanceDma, C: DmaChannel, C::P: SpiPeripheral, - D: IsHalfDuplex, M: Mode, { type Error = super::Error; @@ -2140,10 +2133,9 @@ mod ehal1 { type Error = super::Error; } - impl FullDuplex for Spi<'_, T, M> + impl FullDuplex for Spi<'_, T, FullDuplexMode> where T: Instance, - M: IsFullDuplex, { fn read(&mut self) -> nb::Result { self.spi.read_byte() @@ -2154,10 +2146,9 @@ mod ehal1 { } } - impl SpiBus for Spi<'_, T, M> + impl SpiBus for Spi<'_, T, FullDuplexMode> where T: Instance, - M: IsFullDuplex, { fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { self.spi.read_bytes(words) diff --git a/esp-hal/src/spi/mod.rs b/esp-hal/src/spi/mod.rs index 50b4e14d061..3c73581c159 100644 --- a/esp-hal/src/spi/mod.rs +++ b/esp-hal/src/spi/mod.rs @@ -79,11 +79,7 @@ pub enum SpiBitOrder { } /// Trait marker for defining SPI duplex modes. -pub trait DuplexMode {} -/// Trait marker for SPI full-duplex mode. -pub trait IsFullDuplex: DuplexMode {} -/// Trait marker for SPI half-duplex mode. -pub trait IsHalfDuplex: DuplexMode {} +pub trait DuplexMode: crate::private::Sealed {} /// SPI data mode #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -100,9 +96,9 @@ pub enum SpiDataMode { /// Full-duplex operation pub struct FullDuplexMode {} impl DuplexMode for FullDuplexMode {} -impl IsFullDuplex for FullDuplexMode {} +impl crate::private::Sealed for FullDuplexMode {} /// Half-duplex operation pub struct HalfDuplexMode {} impl DuplexMode for HalfDuplexMode {} -impl IsHalfDuplex for HalfDuplexMode {} +impl crate::private::Sealed for HalfDuplexMode {} From bcdbbd8b77a673e42c471a7dd78bfac5f02448e1 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Tue, 27 Aug 2024 13:04:02 +0100 Subject: [PATCH 13/15] fix tests --- hil-test/tests/spi_full_duplex_dma_async.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hil-test/tests/spi_full_duplex_dma_async.rs b/hil-test/tests/spi_full_duplex_dma_async.rs index 75fee9b2d00..4a392c257e9 100644 --- a/hil-test/tests/spi_full_duplex_dma_async.rs +++ b/hil-test/tests/spi_full_duplex_dma_async.rs @@ -34,10 +34,12 @@ use esp_hal::{ peripherals::{Peripherals, SPI2}, prelude::*, spi::{ - master::{dma::asynch::SpiDmaAsyncBus, Spi}, + master::{dma::SpiDmaBus, Spi}, + FullDuplexMode, SpiMode, }, system::SystemControl, + Async, }; use hil_test as _; @@ -55,7 +57,7 @@ cfg_if::cfg_if! { const DMA_BUFFER_SIZE: usize = 5; struct Context { - spi: SpiDmaAsyncBus<'static, SPI2, DmaChannel0>, + spi: SpiDmaBus<'static, SPI2, DmaChannel0, FullDuplexMode, Async>, pcnt_unit: Unit<'static, 0>, out_pin: Output<'static, GpioPin<5>>, mosi_mirror: GpioPin<2>, From 419868c8ecf7b414e30464cf19a862168910b478 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Tue, 27 Aug 2024 13:55:24 +0100 Subject: [PATCH 14/15] spi docs rejig --- esp-hal/src/dma/mod.rs | 2 +- esp-hal/src/spi/master.rs | 14 +++----------- hil-test/tests/spi_full_duplex_dma.rs | 2 +- hil-test/tests/spi_full_duplex_dma_async.rs | 2 +- hil-test/tests/spi_full_duplex_dma_pcnt.rs | 2 +- hil-test/tests/spi_half_duplex_read.rs | 2 +- hil-test/tests/spi_half_duplex_write.rs | 2 +- 7 files changed, 9 insertions(+), 17 deletions(-) diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index 270f8236e45..c0cc6ca1181 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -16,7 +16,7 @@ #![doc = crate::before_snippet!()] //! # use esp_hal::dma_buffers; //! # use esp_hal::gpio::Io; -//! # use esp_hal::spi::{master::{Spi, prelude::*}, SpiMode}; +//! # use esp_hal::spi::{master::Spi, SpiMode}; //! # use esp_hal::dma::{Dma, DmaPriority}; //! # use crate::esp_hal::prelude::_fugit_RateExtU32; //! let dma = Dma::new(peripherals.DMA); diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 1bdc519de1a..4a610c1dfaa 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -58,6 +58,7 @@ use core::marker::PhantomData; +pub use dma::*; #[cfg(not(any(esp32, esp32s2)))] use enumset::EnumSet; #[cfg(not(any(esp32, esp32s2)))] @@ -87,14 +88,6 @@ use crate::{ system::PeripheralClockControl, }; -/// Prelude for the SPI (Master) driver -pub mod prelude { - pub use super::{ - Instance as _esp_hal_spi_master_Instance, - InstanceDma as _esp_hal_spi_master_InstanceDma, - }; -} - /// Enumeration of possible SPI interrupt events. #[cfg(not(any(esp32, esp32s2)))] #[derive(EnumSetType)] @@ -944,8 +937,7 @@ where } } -/// DMA (Direct Memory Access) funtionality (Master). -pub mod dma { +mod dma { use core::{ cmp::min, sync::atomic::{fence, Ordering}, @@ -1845,7 +1837,7 @@ pub mod dma { /// Async functionality #[cfg(feature = "async")] - pub mod asynch { + mod asynch { use core::{cmp::min, mem::take}; use super::*; diff --git a/hil-test/tests/spi_full_duplex_dma.rs b/hil-test/tests/spi_full_duplex_dma.rs index b530e2e1a1f..7810c4de7b2 100644 --- a/hil-test/tests/spi_full_duplex_dma.rs +++ b/hil-test/tests/spi_full_duplex_dma.rs @@ -21,7 +21,7 @@ use esp_hal::{ peripherals::{Peripherals, SPI2}, prelude::*, spi::{ - master::{dma::SpiDma, Spi}, + master::{Spi, SpiDma}, FullDuplexMode, SpiMode, }, diff --git a/hil-test/tests/spi_full_duplex_dma_async.rs b/hil-test/tests/spi_full_duplex_dma_async.rs index 4a392c257e9..3dd9bc45998 100644 --- a/hil-test/tests/spi_full_duplex_dma_async.rs +++ b/hil-test/tests/spi_full_duplex_dma_async.rs @@ -34,7 +34,7 @@ use esp_hal::{ peripherals::{Peripherals, SPI2}, prelude::*, spi::{ - master::{dma::SpiDmaBus, Spi}, + master::{Spi, SpiDmaBus}, FullDuplexMode, SpiMode, }, diff --git a/hil-test/tests/spi_full_duplex_dma_pcnt.rs b/hil-test/tests/spi_full_duplex_dma_pcnt.rs index e304784240d..50e78fea918 100644 --- a/hil-test/tests/spi_full_duplex_dma_pcnt.rs +++ b/hil-test/tests/spi_full_duplex_dma_pcnt.rs @@ -30,7 +30,7 @@ use esp_hal::{ peripherals::{Peripherals, SPI2}, prelude::*, spi::{ - master::{dma::SpiDma, Spi}, + master::{Spi, SpiDma}, FullDuplexMode, SpiMode, }, diff --git a/hil-test/tests/spi_half_duplex_read.rs b/hil-test/tests/spi_half_duplex_read.rs index 330c8ff8054..eda40ba5e55 100644 --- a/hil-test/tests/spi_half_duplex_read.rs +++ b/hil-test/tests/spi_half_duplex_read.rs @@ -21,7 +21,7 @@ use esp_hal::{ peripherals::{Peripherals, SPI2}, prelude::*, spi::{ - master::{dma::SpiDma, Address, Command, HalfDuplexReadWrite, Spi}, + master::{Address, Command, HalfDuplexReadWrite, Spi, SpiDma}, HalfDuplexMode, SpiDataMode, SpiMode, diff --git a/hil-test/tests/spi_half_duplex_write.rs b/hil-test/tests/spi_half_duplex_write.rs index 22e363d25b3..e62ecd94d3d 100644 --- a/hil-test/tests/spi_half_duplex_write.rs +++ b/hil-test/tests/spi_half_duplex_write.rs @@ -26,7 +26,7 @@ use esp_hal::{ peripherals::{Peripherals, SPI2}, prelude::*, spi::{ - master::{dma::SpiDma, Address, Command, HalfDuplexReadWrite, Spi}, + master::{Address, Command, HalfDuplexReadWrite, Spi, SpiDma}, HalfDuplexMode, SpiDataMode, SpiMode, From f814ef9fb5562ff8cfc4b6f1a66732fb431618a2 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Tue, 27 Aug 2024 16:58:06 +0100 Subject: [PATCH 15/15] s/InUse/TemporarilyRemoved/g --- esp-hal/src/spi/master.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 4a610c1dfaa..ddbfa3250cf 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -1549,7 +1549,7 @@ mod dma { Writing(SpiDmaTransfer<'d, T, C, D, M, DmaTxBuf>, DmaRxBuf), Transferring(SpiDmaTransfer<'d, T, C, D, M, (DmaTxBuf, DmaRxBuf)>), #[default] - InUse, + TemporarilyRemoved, } /// A DMA-capable SPI bus. @@ -1602,7 +1602,7 @@ mod dma { let (spi, (tx_buf, rx_buf)) = transfer.wait(); (spi, tx_buf, rx_buf) } - State::InUse => unreachable!(), + State::TemporarilyRemoved => unreachable!(), } } } @@ -1860,7 +1860,7 @@ mod dma { State::Reading(transfer, _) => transfer.wait_for_done().await, State::Writing(transfer, _) => transfer.wait_for_done().await, State::Transferring(transfer) => transfer.wait_for_done().await, - State::InUse => unreachable!(), + State::TemporarilyRemoved => unreachable!(), } match take(&mut self.state) { State::Idle(spi, tx_buf, rx_buf) => (spi, tx_buf, rx_buf), @@ -1876,7 +1876,7 @@ mod dma { let (spi, (tx_buf, rx_buf)) = transfer.wait(); (spi, tx_buf, rx_buf) } - State::InUse => unreachable!(), + State::TemporarilyRemoved => unreachable!(), } }