From dd5bcb850964774319b7cee7a86ebd71df9255cd Mon Sep 17 00:00:00 2001 From: Studiedlist <49450452+Studiedlist@users.noreply.github.com> Date: Tue, 12 Dec 2023 13:09:20 +0300 Subject: [PATCH] Fix `dma_read` and `dma_write` for SPI slave dma driver (#1013) * separate SpiDmaTransfer impl for rx and tx * format code * update spi_slave_dma example * update CHANGELOG * fix changelog entry * add dma_read and dma_write examples for all supported chips --- CHANGELOG.md | 1 + esp-hal-common/src/spi/slave.rs | 99 ++++++++++++++++++++++----- esp32c2-hal/examples/spi_slave_dma.rs | 54 +++++++++++++++ esp32c3-hal/examples/spi_slave_dma.rs | 54 +++++++++++++++ esp32c6-hal/examples/spi_slave_dma.rs | 54 +++++++++++++++ esp32h2-hal/examples/spi_slave_dma.rs | 54 +++++++++++++++ esp32s3-hal/examples/spi_slave_dma.rs | 54 +++++++++++++++ 7 files changed, 353 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a46a66250..bfe9f9290 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - RISC-V: Fix stack allocation (#988) - ESP32-C6: Fix used RAM (#997) - ESP32-H2: Fix used RAM (#1003) +- Fix SPI slave DMA dma\_read and dma\_write (#1013) ### Removed diff --git a/esp-hal-common/src/spi/slave.rs b/esp-hal-common/src/spi/slave.rs index 51a387d2e..4b44dd50d 100644 --- a/esp-hal-common/src/spi/slave.rs +++ b/esp-hal-common/src/spi/slave.rs @@ -293,8 +293,7 @@ pub mod dma { } } - /// An in-progress DMA transfer. - pub struct SpiDmaTransfer<'d, T, C, BUFFER> + pub struct SpiDmaTransferRx<'d, T, C, BUFFER> where T: InstanceDma, C::Rx<'d>>, C: ChannelTypes, @@ -304,7 +303,73 @@ pub mod dma { buffer: BUFFER, } - impl<'d, T, C, BUFFER> DmaTransfer> for SpiDmaTransfer<'d, T, C, BUFFER> + impl<'d, T, C, BUFFER> DmaTransfer> for SpiDmaTransferRx<'d, T, C, BUFFER> + where + T: InstanceDma, C::Rx<'d>>, + C: ChannelTypes, + C::P: SpiPeripheral, + { + /// Wait for the DMA transfer to complete and return the buffers and the + /// SPI instance. + fn wait( + mut self, + ) -> Result<(BUFFER, SpiDma<'d, T, C>), (DmaError, BUFFER, SpiDma<'d, T, C>)> { + while !self.is_done() {} + self.spi_dma.spi.flush().ok(); // waiting for the DMA transfer is not enough + + // `DmaTransfer` needs to have a `Drop` implementation, because we accept + // managed buffers that can free their memory on drop. Because of that + // we can't move out of the `DmaTransfer`'s fields, so we use `ptr::read` + // and `mem::forget`. + // + // NOTE(unsafe) There is no panic branch between getting the resources + // and forgetting `self`. + unsafe { + let buffer = core::ptr::read(&self.buffer); + let payload = core::ptr::read(&self.spi_dma); + let err = (&self).spi_dma.channel.rx.has_error() + || (&self).spi_dma.channel.tx.has_error(); + mem::forget(self); + if err { + Err((DmaError::DescriptorError, buffer, payload)) + } else { + Ok((buffer, payload)) + } + } + } + + /// Check if the DMA transfer is complete + fn is_done(&self) -> bool { + let ch = &self.spi_dma.channel; + ch.rx.is_done() + } + } + + impl<'d, T, C, BUFFER> Drop for SpiDmaTransferRx<'d, T, C, BUFFER> + where + T: InstanceDma, C::Rx<'d>>, + C: ChannelTypes, + C::P: SpiPeripheral, + { + fn drop(&mut self) { + while !self.is_done() {} + self.spi_dma.spi.flush().ok(); // waiting for the DMA transfer is + // not enough + } + } + + /// An in-progress DMA transfer. + pub struct SpiDmaTransferTx<'d, T, C, BUFFER> + where + T: InstanceDma, C::Rx<'d>>, + C: ChannelTypes, + C::P: SpiPeripheral, + { + spi_dma: SpiDma<'d, T, C>, + buffer: BUFFER, + } + + impl<'d, T, C, BUFFER> DmaTransfer> for SpiDmaTransferTx<'d, T, C, BUFFER> where T: InstanceDma, C::Rx<'d>>, C: ChannelTypes, @@ -344,11 +409,11 @@ pub mod dma { /// Check if the DMA transfer is complete fn is_done(&self) -> bool { let ch = &self.spi_dma.channel; - ch.tx.is_done() && ch.rx.is_done() + ch.tx.is_done() } } - impl<'d, T, C, BUFFER> Drop for SpiDmaTransfer<'d, T, C, BUFFER> + impl<'d, T, C, BUFFER> Drop for SpiDmaTransferTx<'d, T, C, BUFFER> where T: InstanceDma, C::Rx<'d>>, C: ChannelTypes, @@ -389,15 +454,15 @@ pub mod dma { { /// Register a buffer for a DMA write. /// - /// This will return a [SpiDmaTransfer] owning the buffer(s) and the SPI - /// instance. The maximum amount of data to be sent is 32736 + /// This will return a [SpiDmaTransferTx] owning the buffer(s) and the + /// SPI instance. The maximum amount of data to be sent is 32736 /// bytes. /// /// The write is driven by the SPI master's sclk signal and cs line. pub fn dma_write( mut self, words: TXBUF, - ) -> Result, Error> + ) -> Result, Error> where TXBUF: ReadBuffer, { @@ -409,7 +474,7 @@ pub mod dma { self.spi .start_write_bytes_dma(ptr, len, &mut self.channel.tx) - .map(move |_| SpiDmaTransfer { + .map(move |_| SpiDmaTransferTx { spi_dma: self, buffer: words, }) @@ -417,15 +482,15 @@ pub mod dma { /// Register a buffer for a DMA read. /// - /// This will return a [SpiDmaTransfer] owning the buffer(s) and the SPI - /// instance. The maximum amount of data to be received is 32736 - /// bytes. + /// This will return a [SpiDmaTransferRx] owning the buffer(s) and the + /// SPI instance. The maximum amount of data to be received is + /// 32736 bytes. /// /// The read is driven by the SPI master's sclk signal and cs line. pub fn dma_read( mut self, mut words: RXBUF, - ) -> Result, Error> + ) -> Result, Error> where RXBUF: WriteBuffer, { @@ -437,7 +502,7 @@ pub mod dma { self.spi .start_read_bytes_dma(ptr, len, &mut self.channel.rx) - .map(move |_| SpiDmaTransfer { + .map(move |_| SpiDmaTransferRx { spi_dma: self, buffer: words, }) @@ -445,9 +510,9 @@ pub mod dma { /// Register buffers for a DMA transfer. /// - /// This will return a [SpiDmaTransfer] owning the buffer(s) and the SPI - /// instance. The maximum amount of data to be sent/received is - /// 32736 bytes. + /// This will return a [SpiDmaTransferRxTx] owning the buffer(s) and the + /// SPI instance. The maximum amount of data to be sent/received + /// is 32736 bytes. /// /// The data transfer is driven by the SPI master's sclk signal and cs /// line. diff --git a/esp32c2-hal/examples/spi_slave_dma.rs b/esp32c2-hal/examples/spi_slave_dma.rs index 3c440b88f..4b23a8fbc 100644 --- a/esp32c2-hal/examples/spi_slave_dma.rs +++ b/esp32c2-hal/examples/spi_slave_dma.rs @@ -147,5 +147,59 @@ fn main() -> ! { ); delay.delay_ms(250u32); + + slave_receive.fill(0xff); + let transfer = spi.dma_read(slave_receive).unwrap(); + master_cs.set_high().unwrap(); + + master_cs.set_low().unwrap(); + for v in master_send.iter() { + let mut b = *v; + for _ in 0..8 { + if b & 128 != 0 { + master_mosi.set_high().unwrap(); + } else { + master_mosi.set_low().unwrap(); + } + b <<= 1; + master_sclk.set_low().unwrap(); + master_sclk.set_high().unwrap(); + } + } + master_cs.set_high().unwrap(); + (slave_receive, spi) = transfer.wait().unwrap(); + println!( + "slave got {:x?} .. {:x?}", + &slave_receive[..10], + &slave_receive[slave_receive.len() - 10..], + ); + + delay.delay_ms(250u32); + let transfer = spi.dma_write(slave_send).unwrap(); + + master_receive.fill(0); + + master_cs.set_low().unwrap(); + for (j, _) in master_send.iter().enumerate() { + let mut rb = 0u8; + for _ in 0..8 { + master_sclk.set_low().unwrap(); + rb <<= 1; + master_sclk.set_high().unwrap(); + if master_miso.is_high().unwrap() { + rb |= 1; + } + } + master_receive[j] = rb; + } + master_cs.set_high().unwrap(); + (slave_send, spi) = transfer.wait().unwrap(); + + println!( + "master got {:x?} .. {:x?}", + &master_receive[..10], + &master_receive[master_receive.len() - 10..], + ); + println!(); } } diff --git a/esp32c3-hal/examples/spi_slave_dma.rs b/esp32c3-hal/examples/spi_slave_dma.rs index 6084cfb28..ec21b620c 100644 --- a/esp32c3-hal/examples/spi_slave_dma.rs +++ b/esp32c3-hal/examples/spi_slave_dma.rs @@ -146,5 +146,59 @@ fn main() -> ! { ); delay.delay_ms(250u32); + + slave_receive.fill(0xff); + let transfer = spi.dma_read(slave_receive).unwrap(); + master_cs.set_high().unwrap(); + + master_cs.set_low().unwrap(); + for v in master_send.iter() { + let mut b = *v; + for _ in 0..8 { + if b & 128 != 0 { + master_mosi.set_high().unwrap(); + } else { + master_mosi.set_low().unwrap(); + } + b <<= 1; + master_sclk.set_low().unwrap(); + master_sclk.set_high().unwrap(); + } + } + master_cs.set_high().unwrap(); + (slave_receive, spi) = transfer.wait().unwrap(); + println!( + "slave got {:x?} .. {:x?}", + &slave_receive[..10], + &slave_receive[slave_receive.len() - 10..], + ); + + delay.delay_ms(250u32); + let transfer = spi.dma_write(slave_send).unwrap(); + + master_receive.fill(0); + + master_cs.set_low().unwrap(); + for (j, _) in master_send.iter().enumerate() { + let mut rb = 0u8; + for _ in 0..8 { + master_sclk.set_low().unwrap(); + rb <<= 1; + master_sclk.set_high().unwrap(); + if master_miso.is_high().unwrap() { + rb |= 1; + } + } + master_receive[j] = rb; + } + master_cs.set_high().unwrap(); + (slave_send, spi) = transfer.wait().unwrap(); + + println!( + "master got {:x?} .. {:x?}", + &master_receive[..10], + &master_receive[master_receive.len() - 10..], + ); + println!(); } } diff --git a/esp32c6-hal/examples/spi_slave_dma.rs b/esp32c6-hal/examples/spi_slave_dma.rs index fe8576e40..6128b16cb 100644 --- a/esp32c6-hal/examples/spi_slave_dma.rs +++ b/esp32c6-hal/examples/spi_slave_dma.rs @@ -146,5 +146,59 @@ fn main() -> ! { ); delay.delay_ms(250u32); + + slave_receive.fill(0xff); + let transfer = spi.dma_read(slave_receive).unwrap(); + master_cs.set_high().unwrap(); + + master_cs.set_low().unwrap(); + for v in master_send.iter() { + let mut b = *v; + for _ in 0..8 { + if b & 128 != 0 { + master_mosi.set_high().unwrap(); + } else { + master_mosi.set_low().unwrap(); + } + b <<= 1; + master_sclk.set_low().unwrap(); + master_sclk.set_high().unwrap(); + } + } + master_cs.set_high().unwrap(); + (slave_receive, spi) = transfer.wait().unwrap(); + println!( + "slave got {:x?} .. {:x?}", + &slave_receive[..10], + &slave_receive[slave_receive.len() - 10..], + ); + + delay.delay_ms(250u32); + let transfer = spi.dma_write(slave_send).unwrap(); + + master_receive.fill(0); + + master_cs.set_low().unwrap(); + for (j, _) in master_send.iter().enumerate() { + let mut rb = 0u8; + for _ in 0..8 { + master_sclk.set_low().unwrap(); + rb <<= 1; + master_sclk.set_high().unwrap(); + if master_miso.is_high().unwrap() { + rb |= 1; + } + } + master_receive[j] = rb; + } + master_cs.set_high().unwrap(); + (slave_send, spi) = transfer.wait().unwrap(); + + println!( + "master got {:x?} .. {:x?}", + &master_receive[..10], + &master_receive[master_receive.len() - 10..], + ); + println!(); } } diff --git a/esp32h2-hal/examples/spi_slave_dma.rs b/esp32h2-hal/examples/spi_slave_dma.rs index 23ad9d9ec..53a93f49e 100644 --- a/esp32h2-hal/examples/spi_slave_dma.rs +++ b/esp32h2-hal/examples/spi_slave_dma.rs @@ -146,5 +146,59 @@ fn main() -> ! { ); delay.delay_ms(250u32); + + slave_receive.fill(0xff); + let transfer = spi.dma_read(slave_receive).unwrap(); + master_cs.set_high().unwrap(); + + master_cs.set_low().unwrap(); + for v in master_send.iter() { + let mut b = *v; + for _ in 0..8 { + if b & 128 != 0 { + master_mosi.set_high().unwrap(); + } else { + master_mosi.set_low().unwrap(); + } + b <<= 1; + master_sclk.set_low().unwrap(); + master_sclk.set_high().unwrap(); + } + } + master_cs.set_high().unwrap(); + (slave_receive, spi) = transfer.wait().unwrap(); + println!( + "slave got {:x?} .. {:x?}", + &slave_receive[..10], + &slave_receive[slave_receive.len() - 10..], + ); + + delay.delay_ms(250u32); + let transfer = spi.dma_write(slave_send).unwrap(); + + master_receive.fill(0); + + master_cs.set_low().unwrap(); + for (j, _) in master_send.iter().enumerate() { + let mut rb = 0u8; + for _ in 0..8 { + master_sclk.set_low().unwrap(); + rb <<= 1; + master_sclk.set_high().unwrap(); + if master_miso.is_high().unwrap() { + rb |= 1; + } + } + master_receive[j] = rb; + } + master_cs.set_high().unwrap(); + (slave_send, spi) = transfer.wait().unwrap(); + + println!( + "master got {:x?} .. {:x?}", + &master_receive[..10], + &master_receive[master_receive.len() - 10..], + ); + println!(); } } diff --git a/esp32s3-hal/examples/spi_slave_dma.rs b/esp32s3-hal/examples/spi_slave_dma.rs index cb4ad323e..dcb4a7be7 100644 --- a/esp32s3-hal/examples/spi_slave_dma.rs +++ b/esp32s3-hal/examples/spi_slave_dma.rs @@ -146,5 +146,59 @@ fn main() -> ! { ); delay.delay_ms(250u32); + + slave_receive.fill(0xff); + let transfer = spi.dma_read(slave_receive).unwrap(); + master_cs.set_high().unwrap(); + + master_cs.set_low().unwrap(); + for v in master_send.iter() { + let mut b = *v; + for _ in 0..8 { + if b & 128 != 0 { + master_mosi.set_high().unwrap(); + } else { + master_mosi.set_low().unwrap(); + } + b <<= 1; + master_sclk.set_low().unwrap(); + master_sclk.set_high().unwrap(); + } + } + master_cs.set_high().unwrap(); + (slave_receive, spi) = transfer.wait().unwrap(); + println!( + "slave got {:x?} .. {:x?}", + &slave_receive[..10], + &slave_receive[slave_receive.len() - 10..], + ); + + delay.delay_ms(250u32); + let transfer = spi.dma_write(slave_send).unwrap(); + + master_receive.fill(0); + + master_cs.set_low().unwrap(); + for (j, _) in master_send.iter().enumerate() { + let mut rb = 0u8; + for _ in 0..8 { + master_sclk.set_low().unwrap(); + rb <<= 1; + master_sclk.set_high().unwrap(); + if master_miso.is_high().unwrap() { + rb |= 1; + } + } + master_receive[j] = rb; + } + master_cs.set_high().unwrap(); + (slave_send, spi) = transfer.wait().unwrap(); + + println!( + "master got {:x?} .. {:x?}", + &master_receive[..10], + &master_receive[master_receive.len() - 10..], + ); + println!(); } }