From a24eff9bf96be4a72cafa23f251c34a476fd42d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 25 Oct 2023 12:50:18 +0200 Subject: [PATCH] Fix async SPI/DMA race condition (#869) --- CHANGELOG.md | 1 + esp-hal-common/src/dma/mod.rs | 2 -- esp-hal-common/src/i2s.rs | 12 ++++++++---- esp-hal-common/src/spi/master.rs | 30 +++++++++++++++++++++++++----- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1c83c58f..3c3ca4331 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - UART/ESP32: fix calculating FIFO counter with `get_rx_fifo_count()` (#804) - Xtensa targets: Use ESP32Reset - not Reset (#823) - Examples should now work with the `defmt` feature (#810) +- Fixed a race condition causing SpiDma to stop working unexpectedly (#869) ### Removed diff --git a/esp-hal-common/src/dma/mod.rs b/esp-hal-common/src/dma/mod.rs index 3a5ab9ed7..b1cff8d38 100644 --- a/esp-hal-common/src/dma/mod.rs +++ b/esp-hal-common/src/dma/mod.rs @@ -1097,7 +1097,6 @@ pub(crate) mod asynch { TX: Tx, { pub fn new(tx: &'a mut TX) -> Self { - tx.listen_eof(); Self { tx, _a: () } } } @@ -1131,7 +1130,6 @@ pub(crate) mod asynch { RX: Rx, { pub fn new(rx: &'a mut RX) -> Self { - rx.listen_eof(); Self { rx, _a: () } } } diff --git a/esp-hal-common/src/i2s.rs b/esp-hal-common/src/i2s.rs index e512c6169..746a81e53 100644 --- a/esp-hal-common/src/i2s.rs +++ b/esp-hal-common/src/i2s.rs @@ -2256,10 +2256,12 @@ pub mod asynch { { async fn write_dma_async(&mut self, words: &mut [u8]) -> Result<(), Error> { let (ptr, len) = (words.as_ptr(), words.len()); + + self.tx_channel.listen_eof(); + self.start_tx_transfer_async(ptr, len, false)?; - let future = DmaTxFuture::new(&mut self.tx_channel); - future.await; + DmaTxFuture::new(&mut self.tx_channel).await; self.register_access.reset_tx(); @@ -2352,10 +2354,12 @@ pub mod asynch { { async fn read_dma_async(&mut self, words: &mut [u8]) -> Result<(), Error> { let (ptr, len) = (words.as_mut_ptr(), words.len()); + + self.rx_channel.listen_eof(); + self.start_rx_transfer_async(ptr, len, false)?; - let future = DmaRxFuture::new(&mut self.rx_channel); - future.await; + DmaRxFuture::new(&mut self.rx_channel).await; // ??? self.register_access.reset_tx(); diff --git a/esp-hal-common/src/spi/master.rs b/esp-hal-common/src/spi/master.rs index 30ca2fbc4..cb4c73665 100644 --- a/esp-hal-common/src/spi/master.rs +++ b/esp-hal-common/src/spi/master.rs @@ -1000,7 +1000,7 @@ pub mod dma { } self.spi - .start_write_bytes_dma(ptr, len, &mut self.channel.tx)?; + .start_write_bytes_dma(ptr, len, &mut self.channel.tx, false)?; Ok(SpiDmaTransfer { spi_dma: self, buffer: words, @@ -1026,7 +1026,7 @@ pub mod dma { } self.spi - .start_read_bytes_dma(ptr, len, &mut self.channel.rx)?; + .start_read_bytes_dma(ptr, len, &mut self.channel.rx, false)?; Ok(SpiDmaTransfer { spi_dma: self, buffer: words, @@ -1061,6 +1061,7 @@ pub mod dma { read_len, &mut self.channel.tx, &mut self.channel.rx, + false, )?; Ok(SpiDmaTransferRxTx { spi_dma: self, @@ -1143,7 +1144,7 @@ pub mod dma { } self.spi - .start_read_bytes_dma(ptr, len, &mut self.channel.rx)?; + .start_read_bytes_dma(ptr, len, &mut self.channel.rx, false)?; Ok(SpiDmaTransfer { spi_dma: self, buffer: buffer, @@ -1216,7 +1217,7 @@ pub mod dma { } self.spi - .start_write_bytes_dma(ptr, len, &mut self.channel.tx)?; + .start_write_bytes_dma(ptr, len, &mut self.channel.tx, false)?; Ok(SpiDmaTransfer { spi_dma: self, buffer: buffer, @@ -1322,6 +1323,7 @@ pub mod dma { words.as_mut_ptr(), words.len(), &mut self.channel.rx, + true, )?; crate::dma::asynch::DmaRxFuture::new(&mut self.channel.rx).await; @@ -1335,6 +1337,7 @@ pub mod dma { chunk.as_ptr(), chunk.len(), &mut self.channel.tx, + true, )?; crate::dma::asynch::DmaTxFuture::new(&mut self.channel.tx).await; @@ -1363,6 +1366,7 @@ pub mod dma { read_len, &mut self.channel.tx, &mut self.channel.rx, + true, )?; embassy_futures::join::join( @@ -1393,6 +1397,7 @@ pub mod dma { chunk.len(), &mut self.channel.tx, &mut self.channel.rx, + true, )?; embassy_futures::join::join( @@ -1791,6 +1796,7 @@ where chunk.len(), tx, rx, + false, )?; while !tx.is_done() && !rx.is_done() {} @@ -1822,6 +1828,7 @@ where read_len, tx, rx, + false, )?; while !tx.is_done() && !rx.is_done() {} @@ -1844,6 +1851,7 @@ where read_buffer_len: usize, tx: &mut TX, rx: &mut RX, + listen: bool, ) -> Result<(), Error> { let reg_block = self.register_block(); self.configure_datalen(usize::max(read_buffer_len, write_buffer_len) as u32 * 8); @@ -1873,6 +1881,10 @@ where self.clear_dma_interrupts(); reset_dma_before_usr_cmd(reg_block); + if listen { + tx.listen_eof(); + rx.listen_eof(); + } reg_block.cmd.modify(|_, w| w.usr().set_bit()); Ok(()) @@ -1880,7 +1892,7 @@ where fn write_bytes_dma<'w>(&mut self, words: &'w [u8], tx: &mut TX) -> Result<&'w [u8], Error> { for chunk in words.chunks(MAX_DMA_SIZE) { - self.start_write_bytes_dma(chunk.as_ptr(), chunk.len(), tx)?; + self.start_write_bytes_dma(chunk.as_ptr(), chunk.len(), tx, false)?; while !tx.is_done() {} self.flush().unwrap(); // seems "is_done" doesn't work as intended? @@ -1894,6 +1906,7 @@ where ptr: *const u8, len: usize, tx: &mut TX, + listen: bool, ) -> Result<(), Error> { let reg_block = self.register_block(); self.configure_datalen(len as u32 * 8); @@ -1910,6 +1923,9 @@ where self.clear_dma_interrupts(); reset_dma_before_usr_cmd(reg_block); + if listen { + tx.listen_eof(); + } reg_block.cmd.modify(|_, w| w.usr().set_bit()); return Ok(()); @@ -1920,6 +1936,7 @@ where ptr: *mut u8, len: usize, rx: &mut RX, + listen: bool, ) -> Result<(), Error> { let reg_block = self.register_block(); self.configure_datalen(len as u32 * 8); @@ -1936,6 +1953,9 @@ where self.clear_dma_interrupts(); reset_dma_before_usr_cmd(reg_block); + if listen { + rx.listen_eof(); + } reg_block.cmd.modify(|_, w| w.usr().set_bit()); return Ok(());