From dc88cb13e876bd86762e2822c4963a266575c915 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 9 Oct 2024 01:15:21 +0200 Subject: [PATCH] Reimplement SPI DMA operations in terms of transfer (#2308) * Reimplement SPI DMA operations in terms of transfer * Deduplicate reset_dma * Create empty constructors * Introduce EmptyBuf * Fix comment * Undo unnecessary change * Remove unnecessary slicing --- esp-hal/src/dma/buffers.rs | 51 ++++++++ esp-hal/src/spi/master.rs | 250 +++++++++++++------------------------ 2 files changed, 139 insertions(+), 162 deletions(-) diff --git a/esp-hal/src/dma/buffers.rs b/esp-hal/src/dma/buffers.rs index e783645ce..10a76bd03 100644 --- a/esp-hal/src/dma/buffers.rs +++ b/esp-hal/src/dma/buffers.rs @@ -913,3 +913,54 @@ impl DmaRxStreamBufView { } } } + +static mut EMPTY: [DmaDescriptor; 1] = [DmaDescriptor::EMPTY]; + +/// An empty buffer that can be used when you don't need to transfer any data. +pub struct EmptyBuf; + +unsafe impl DmaTxBuffer for EmptyBuf { + type View = EmptyBuf; + + fn prepare(&mut self) -> Preparation { + Preparation { + start: unsafe { EMPTY.as_mut_ptr() }, + block_size: None, + } + } + + fn into_view(self) -> EmptyBuf { + self + } + + fn from_view(view: Self::View) -> Self { + view + } + + fn length(&self) -> usize { + 0 + } +} + +unsafe impl DmaRxBuffer for EmptyBuf { + type View = EmptyBuf; + + fn prepare(&mut self) -> Preparation { + Preparation { + start: unsafe { EMPTY.as_mut_ptr() }, + block_size: None, + } + } + + fn into_view(self) -> EmptyBuf { + self + } + + fn from_view(view: Self::View) -> Self { + view + } + + fn length(&self) -> usize { + 0 + } +} diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 5c2b6b86e..ad5aa335b 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -944,6 +944,7 @@ mod dma { DmaRxBuffer, DmaTxBuf, DmaTxBuffer, + EmptyBuf, Rx, Tx, }, @@ -1121,40 +1122,6 @@ mod dma { .await; } - /// # Safety: - /// - /// The caller must ensure to not access the buffer contents while the - /// transfer is in progress. Moving the buffer itself is allowed. - #[cfg_attr(place_spi_driver_in_ram, ram)] - unsafe fn start_write_bytes_dma( - &mut self, - buffer: &mut TX, - full_duplex: bool, - ) -> Result<(), Error> { - self.tx_transfer_in_progress = buffer.length() > 0; - unsafe { - self.spi - .start_write_bytes_dma(buffer, &mut self.channel.tx, full_duplex) - } - } - - /// # Safety: - /// - /// The caller must ensure to not access the buffer contents while the - /// transfer is in progress. Moving the buffer itself is allowed. - #[cfg_attr(place_spi_driver_in_ram, ram)] - unsafe fn start_read_bytes_dma( - &mut self, - buffer: &mut RX, - full_duplex: bool, - ) -> Result<(), Error> { - self.rx_transfer_in_progress = buffer.length() > 0; - unsafe { - self.spi - .start_read_bytes_dma(buffer, &mut self.channel.rx, full_duplex) - } - } - /// # Safety: /// /// The caller must ensure to not access the buffer contents while the @@ -1162,6 +1129,7 @@ mod dma { #[cfg_attr(place_spi_driver_in_ram, ram)] unsafe fn start_transfer_dma( &mut self, + full_duplex: bool, rx_buffer: &mut RX, tx_buffer: &mut TX, ) -> Result<(), Error> { @@ -1169,6 +1137,7 @@ mod dma { self.tx_transfer_in_progress = tx_buffer.length() > 0; unsafe { self.spi.start_transfer_dma( + full_duplex, rx_buffer, tx_buffer, &mut self.channel.rx, @@ -1203,10 +1172,12 @@ mod dma { self.tx_transfer_in_progress = true; unsafe { - self.spi.start_write_bytes_dma( - &mut self.address_buffer, - &mut self.channel.tx, + self.spi.start_transfer_dma( false, + &mut EmptyBuf, + &mut self.address_buffer, + &mut self.channel.rx, + &mut self.channel.tx, ) } } @@ -1387,7 +1358,13 @@ mod dma { return Err(Error::MaxDmaTransferSizeExceeded); } - self.start_write_bytes_dma(buffer, true) + self.spi.start_transfer_dma( + true, + &mut EmptyBuf, + buffer, + &mut self.channel.rx, + &mut self.channel.tx, + ) } /// Perform a DMA write. @@ -1420,7 +1397,13 @@ mod dma { return Err(Error::MaxDmaTransferSizeExceeded); } - self.start_read_bytes_dma(buffer, true) + self.spi.start_transfer_dma( + false, + buffer, + &mut EmptyBuf, + &mut self.channel.rx, + &mut self.channel.tx, + ) } /// Perform a DMA read. @@ -1458,7 +1441,7 @@ mod dma { return Err(Error::MaxDmaTransferSizeExceeded); } - self.start_transfer_dma(rx_buffer, tx_buffer) + self.start_transfer_dma(true, rx_buffer, tx_buffer) } /// Perform a DMA transfer @@ -1515,7 +1498,13 @@ mod dma { data_mode, ); - self.start_read_bytes_dma(buffer, false) + self.spi.start_transfer_dma( + false, + buffer, + &mut EmptyBuf, + &mut self.channel.rx, + &mut self.channel.tx, + ) } /// Perform a half-duplex read operation using DMA. @@ -1576,7 +1565,13 @@ mod dma { data_mode, ); - self.start_write_bytes_dma(buffer, false) + self.spi.start_transfer_dma( + false, + &mut EmptyBuf, + buffer, + &mut self.channel.rx, + &mut self.channel.tx, + ) } /// Perform a half-duplex write operation using DMA. @@ -1704,7 +1699,11 @@ mod dma { for chunk in words.chunks_mut(self.rx_buf.capacity()) { self.rx_buf.set_length(chunk.len()); - unsafe { self.spi_dma.start_dma_read(&mut self.rx_buf)? }; + unsafe { + self.spi_dma + .start_dma_transfer(&mut self.rx_buf, &mut EmptyBuf)?; + } + self.wait_for_idle(); let bytes_read = self.rx_buf.read_received_data(chunk); @@ -1721,8 +1720,10 @@ mod dma { self.tx_buf.fill(chunk); unsafe { - self.spi_dma.start_dma_write(&mut self.tx_buf)?; + self.spi_dma + .start_dma_transfer(&mut EmptyBuf, &mut self.tx_buf)?; } + self.wait_for_idle(); } @@ -1945,9 +1946,8 @@ mod dma { let mut spi = DropGuard::new(&mut self.spi_dma, |spi| spi.cancel_transfer()); - unsafe { - spi.start_dma_read(&mut self.rx_buf)?; - } + unsafe { spi.start_dma_transfer(&mut self.rx_buf, &mut EmptyBuf)? }; + spi.wait_for_idle_async().await; let bytes_read = self.rx_buf.read_received_data(chunk); @@ -1969,9 +1969,8 @@ mod dma { for chunk in words.chunks(chunk_size) { self.tx_buf.fill(chunk); - unsafe { - spi.start_dma_write(&mut self.tx_buf)?; - } + unsafe { spi.start_dma_transfer(&mut EmptyBuf, &mut self.tx_buf)? }; + spi.wait_for_idle_async().await; } spi.defuse(); @@ -2215,9 +2214,10 @@ mod ehal1 { #[doc(hidden)] pub trait InstanceDma: Instance + DmaEligible { - #[allow(clippy::too_many_arguments)] + #[cfg_attr(place_spi_driver_in_ram, ram)] unsafe fn start_transfer_dma( &mut self, + _full_duplex: bool, rx_buffer: &mut impl DmaRxBuffer, tx_buffer: &mut impl DmaTxBuffer, rx: &mut RX, @@ -2232,102 +2232,40 @@ pub trait InstanceDma: Instance + DmaEligible { reg_block.dma_in_link().write(|w| w.bits(0)); } - self.configure_datalen(rx_buffer.length(), tx_buffer.length()); + let rx_len = rx_buffer.length(); + let tx_len = tx_buffer.length(); + self.configure_datalen(rx_len, tx_len); - // re-enable the MISO and MOSI + // enable the MISO and MOSI if needed reg_block .user() - .modify(|_, w| w.usr_miso().bit(true).usr_mosi().bit(true)); + .modify(|_, w| w.usr_miso().bit(rx_len > 0).usr_mosi().bit(tx_len > 0)); self.enable_dma(); - rx.prepare_transfer(self.dma_peripheral(), rx_buffer) - .and_then(|_| rx.start_transfer())?; - tx.prepare_transfer(self.dma_peripheral(), tx_buffer) - .and_then(|_| tx.start_transfer())?; - - #[cfg(gdma)] - self.reset_dma(); - - self.start_operation(); - - Ok(()) - } - - #[cfg_attr(place_spi_driver_in_ram, ram)] - unsafe fn start_write_bytes_dma( - &mut self, - buffer: &mut impl DmaTxBuffer, - tx: &mut TX, - full_duplex: bool, - ) -> Result<(), Error> { - let reg_block = self.register_block(); - self.configure_datalen(0, buffer.length()); - - // disable MISO and re-enable MOSI (DON'T do it for half-duplex) - if full_duplex { - reg_block - .user() - .modify(|_, w| w.usr_miso().bit(false).usr_mosi().bit(true)); - } - - #[cfg(esp32)] - { - // see https://github.com/espressif/esp-idf/commit/366e4397e9dae9d93fe69ea9d389b5743295886f - // see https://github.com/espressif/esp-idf/commit/0c3653b1fd7151001143451d4aa95dbf15ee8506 - if full_duplex { - reg_block - .dma_in_link() - .modify(|_, w| unsafe { w.inlink_addr().bits(0) }); - reg_block - .dma_in_link() - .modify(|_, w| w.inlink_start().set_bit()); + if rx_len > 0 { + rx.prepare_transfer(self.dma_peripheral(), rx_buffer) + .and_then(|_| rx.start_transfer())?; + } else { + #[cfg(esp32)] + { + // see https://github.com/espressif/esp-idf/commit/366e4397e9dae9d93fe69ea9d389b5743295886f + // see https://github.com/espressif/esp-idf/commit/0c3653b1fd7151001143451d4aa95dbf15ee8506 + if _full_duplex { + reg_block + .dma_in_link() + .modify(|_, w| unsafe { w.inlink_addr().bits(0) }); + reg_block + .dma_in_link() + .modify(|_, w| w.inlink_start().set_bit()); + } } } - - self.enable_dma(); - - tx.prepare_transfer(self.dma_peripheral(), buffer) - .and_then(|_| tx.start_transfer())?; - - #[cfg(gdma)] - self.reset_dma(); - - self.start_operation(); - - Ok(()) - } - - #[cfg_attr(place_spi_driver_in_ram, ram)] - unsafe fn start_read_bytes_dma( - &mut self, - buffer: &mut BUF, - rx: &mut RX, - full_duplex: bool, - ) -> Result<(), Error> { - let reg_block = self.register_block(); - - #[cfg(esp32s2)] - { - // without this a read after a write will fail - reg_block.dma_out_link().write(|w| w.bits(0)); - reg_block.dma_in_link().write(|w| w.bits(0)); + if tx_len > 0 { + tx.prepare_transfer(self.dma_peripheral(), tx_buffer) + .and_then(|_| tx.start_transfer())?; } - self.configure_datalen(buffer.length(), 0); - - // re-enable MISO and disable MOSI (DON'T do it for half-duplex) - if full_duplex { - reg_block - .user() - .modify(|_, w| w.usr_miso().bit(true).usr_mosi().bit(false)); - } - - self.enable_dma(); - - rx.prepare_transfer(self.dma_peripheral(), buffer) - .and_then(|_| rx.start_transfer())?; - #[cfg(gdma)] self.reset_dma(); @@ -2351,36 +2289,24 @@ pub trait InstanceDma: Instance + DmaEligible { } fn reset_dma(&self) { - #[cfg(pdma)] - { - let reg_block = self.register_block(); + fn set_reset_bit(reg_block: &RegisterBlock, bit: bool) { + #[cfg(pdma)] reg_block.dma_conf().modify(|_, w| { - w.out_rst().set_bit(); - w.in_rst().set_bit(); - w.ahbm_fifo_rst().set_bit(); - w.ahbm_rst().set_bit() + w.out_rst().bit(bit); + w.in_rst().bit(bit); + w.ahbm_fifo_rst().bit(bit); + w.ahbm_rst().bit(bit) }); + #[cfg(gdma)] reg_block.dma_conf().modify(|_, w| { - w.out_rst().clear_bit(); - w.in_rst().clear_bit(); - w.ahbm_fifo_rst().clear_bit(); - w.ahbm_rst().clear_bit() - }); - } - #[cfg(gdma)] - { - let reg_block = self.register_block(); - reg_block.dma_conf().modify(|_, w| { - w.rx_afifo_rst().set_bit(); - w.buf_afifo_rst().set_bit(); - w.dma_afifo_rst().set_bit() - }); - reg_block.dma_conf().modify(|_, w| { - w.rx_afifo_rst().clear_bit(); - w.buf_afifo_rst().clear_bit(); - w.dma_afifo_rst().clear_bit() + w.rx_afifo_rst().bit(bit); + w.buf_afifo_rst().bit(bit); + w.dma_afifo_rst().bit(bit) }); } + let reg_block = self.register_block(); + set_reset_bit(reg_block, true); + set_reset_bit(reg_block, false); self.clear_dma_interrupts(); }