From 0406dbea6e4102297b13e7208ec4bf60d3be4c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 1 Nov 2024 21:27:38 +0100 Subject: [PATCH 1/8] Remove hidden public SPI API --- esp-hal/src/spi/master.rs | 745 ++++++++++++++++++-------------------- 1 file changed, 355 insertions(+), 390 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index cd906e3fab..55d65ee4a0 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -444,7 +444,7 @@ pub struct Spi<'d, M, T = AnySpi> { impl<'d, M, T> Spi<'d, M, T> where M: Mode, - T: InstanceDma, + T: Instance, { /// Configures the SPI instance to use DMA with the specified channel. /// @@ -465,18 +465,22 @@ impl Spi<'_, M, T> where T: Instance, { + fn driver(&self) -> &'static Info { + self.spi.info() + } + /// Read a byte from SPI. /// /// Sends out a stuffing byte for every byte to read. This function doesn't /// perform flushing. If you want to read the response to something you /// have written before, consider using [`Self::transfer`] instead. pub fn read_byte(&mut self) -> nb::Result { - self.spi.read_byte() + self.driver().read_byte() } /// Write a byte to SPI. pub fn write_byte(&mut self, word: u8) -> nb::Result<(), Error> { - self.spi.write_byte(word) + self.driver().write_byte(word) } /// Write bytes to SPI. @@ -485,15 +489,15 @@ where /// transmission FIFO. If `words` is longer than 64 bytes, multiple /// sequential transfers are performed. pub fn write_bytes(&mut self, words: &[u8]) -> Result<(), Error> { - self.spi.write_bytes(words)?; - self.spi.flush()?; + self.driver().write_bytes(words)?; + self.driver().flush()?; Ok(()) } /// Sends `words` to the slave. Returns the `words` received from the slave pub fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Error> { - self.spi.transfer(words) + self.driver().transfer(words) } } @@ -538,45 +542,43 @@ where ) -> Spi<'d, M, T> { crate::into_ref!(spi); - let mut spi = Spi { + let this = Spi { spi, _mode: PhantomData, }; - spi.spi.reset_peripheral(); - spi.spi.enable_peripheral(); - spi.spi.setup(frequency); - spi.spi.init(); - spi.spi.set_data_mode(mode); - let spi = spi + PeripheralClockControl::enable(this.spi.peripheral()); + PeripheralClockControl::reset(this.spi.peripheral()); + + this.driver().setup(frequency); + this.driver().init(); + this.driver().set_data_mode(mode); + + let this = this .with_mosi(NoPin) .with_miso(NoPin) .with_sck(NoPin) .with_cs(NoPin); - let is_qspi = spi.spi.sio2_input_signal().is_some(); + let is_qspi = this.driver().sio2_input.is_some(); if is_qspi { let mut signal = OutputConnection::from(NoPin); - signal.connect_input_to_peripheral( - unwrap!(spi.spi.sio2_input_signal()), - private::Internal, - ); + signal + .connect_input_to_peripheral(unwrap!(this.driver().sio2_input), private::Internal); signal.connect_peripheral_to_output( - unwrap!(spi.spi.sio2_output_signal()), - private::Internal, - ); - signal.connect_input_to_peripheral( - unwrap!(spi.spi.sio3_input_signal()), + unwrap!(this.driver().sio2_output), private::Internal, ); + signal + .connect_input_to_peripheral(unwrap!(this.driver().sio3_input), private::Internal); signal.connect_peripheral_to_output( - unwrap!(spi.spi.sio3_output_signal()), + unwrap!(this.driver().sio3_output), private::Internal, ); } - spi + this } /// Assign the MOSI (Master Out Slave In) pin for the SPI instance. @@ -586,10 +588,10 @@ where pub fn with_mosi(self, mosi: impl Peripheral

+ 'd) -> Self { crate::into_mapped_ref!(mosi); mosi.enable_output(true, private::Internal); - mosi.connect_peripheral_to_output(self.spi.mosi_signal(), private::Internal); + mosi.connect_peripheral_to_output(self.driver().mosi, private::Internal); mosi.enable_input(true, private::Internal); - mosi.connect_input_to_peripheral(self.spi.sio0_input_signal(), private::Internal); + mosi.connect_input_to_peripheral(self.driver().sio0_input, private::Internal); self } @@ -601,10 +603,10 @@ where pub fn with_miso(self, miso: impl Peripheral

+ 'd) -> Self { crate::into_mapped_ref!(miso); miso.enable_input(true, private::Internal); - miso.connect_input_to_peripheral(self.spi.miso_signal(), private::Internal); + miso.connect_input_to_peripheral(self.driver().miso, private::Internal); miso.enable_output(true, private::Internal); - miso.connect_peripheral_to_output(self.spi.sio1_output_signal(), private::Internal); + miso.connect_peripheral_to_output(self.driver().sio1_output, private::Internal); self } @@ -616,7 +618,7 @@ where pub fn with_sck(self, sclk: impl Peripheral

+ 'd) -> Self { crate::into_mapped_ref!(sclk); sclk.set_to_push_pull_output(private::Internal); - sclk.connect_peripheral_to_output(self.spi.sclk_signal(), private::Internal); + sclk.connect_peripheral_to_output(self.driver().sclk, private::Internal); self } @@ -628,7 +630,7 @@ where pub fn with_cs(self, cs: impl Peripheral

+ 'd) -> Self { crate::into_mapped_ref!(cs); cs.set_to_push_pull_output(private::Internal); - cs.connect_peripheral_to_output(self.spi.cs_signal(), private::Internal); + cs.connect_peripheral_to_output(self.driver().cs, private::Internal); self } @@ -638,14 +640,14 @@ where /// This method allows you to update the bus frequency for the SPI /// communication after the instance has been created. pub fn change_bus_frequency(&mut self, frequency: HertzU32) { - self.spi.ch_bus_freq(frequency); + self.driver().ch_bus_freq(frequency); } /// Set the bit order for the SPI instance. /// /// The default is MSB first for both read and write. - pub fn with_bit_order(mut self, read_order: SpiBitOrder, write_order: SpiBitOrder) -> Self { - self.spi.set_bit_order(read_order, write_order); + pub fn with_bit_order(self, read_order: SpiBitOrder, write_order: SpiBitOrder) -> Self { + self.driver().set_bit_order(read_order, write_order); self } } @@ -666,11 +668,8 @@ where sio2.enable_input(true, private::Internal); sio2.enable_output(true, private::Internal); - sio2.connect_input_to_peripheral(unwrap!(self.spi.sio2_input_signal()), private::Internal); - sio2.connect_peripheral_to_output( - unwrap!(self.spi.sio2_output_signal()), - private::Internal, - ); + sio2.connect_input_to_peripheral(unwrap!(self.driver().sio2_input), private::Internal); + sio2.connect_peripheral_to_output(unwrap!(self.driver().sio2_output), private::Internal); self } @@ -687,11 +686,8 @@ where sio3.enable_input(true, private::Internal); sio3.enable_output(true, private::Internal); - sio3.connect_input_to_peripheral(unwrap!(self.spi.sio3_input_signal()), private::Internal); - sio3.connect_peripheral_to_output( - unwrap!(self.spi.sio3_output_signal()), - private::Internal, - ); + sio3.connect_input_to_peripheral(unwrap!(self.driver().sio3_input), private::Internal); + sio3.connect_peripheral_to_output(unwrap!(self.driver().sio3_output), private::Internal); self } @@ -718,7 +714,7 @@ where return Err(Error::Unsupported); } - self.spi.setup_half_duplex( + self.driver().setup_half_duplex( false, cmd, address, @@ -728,10 +724,10 @@ where data_mode, ); - self.spi.configure_datalen(buffer.len(), 0); - self.spi.start_operation(); - self.spi.flush()?; - self.spi.read_bytes_from_fifo(buffer) + self.driver().configure_datalen(buffer.len(), 0); + self.driver().start_operation(); + self.driver().flush()?; + self.driver().read_bytes_from_fifo(buffer) } /// Half-duplex write. @@ -767,7 +763,7 @@ where } } - self.spi.setup_half_duplex( + self.driver().setup_half_duplex( true, cmd, address, @@ -779,12 +775,12 @@ where if !buffer.is_empty() { // re-using the full-duplex write here - self.spi.write_bytes(buffer)?; + self.driver().write_bytes(buffer)?; } else { - self.spi.start_operation(); + self.driver().start_operation(); } - self.spi.flush() + self.driver().flush() } } @@ -822,7 +818,7 @@ where fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { self.write_bytes(words)?; - self.spi.flush() + self.driver().flush() } } @@ -858,7 +854,7 @@ mod dma { /// embedded-hal traits. pub struct SpiDma<'d, M, T = AnySpi> where - T: InstanceDma, + T: Instance, M: Mode, { pub(crate) spi: PeripheralRef<'d, T>, @@ -871,7 +867,7 @@ mod dma { impl<'d, T> SpiDma<'d, Blocking, T> where - T: InstanceDma, + T: Instance, { /// Converts the SPI instance into async mode. pub fn into_async(self) -> SpiDma<'d, Async, T> { @@ -888,7 +884,7 @@ mod dma { impl<'d, T> SpiDma<'d, Async, T> where - T: InstanceDma, + T: Instance, { /// Converts the SPI instance into async mode. pub fn into_blocking(self) -> SpiDma<'d, Blocking, T> { @@ -906,14 +902,14 @@ mod dma { #[cfg(all(esp32, spi_address_workaround))] unsafe impl<'d, M, T> Send for SpiDma<'d, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { } impl core::fmt::Debug for SpiDma<'_, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { /// Formats the `SpiDma` instance for debugging purposes. @@ -927,13 +923,13 @@ mod dma { impl InterruptConfigurable for SpiDma<'_, Blocking, T> where - T: InstanceDma, + T: Instance, { /// Sets the interrupt handler /// /// Interrupts are not enabled at the peripheral level here. fn set_interrupt_handler(&mut self, handler: InterruptHandler) { - let interrupt = self.spi.interrupt(); + let interrupt = self.driver().interrupt; for core in crate::Cpu::other() { crate::interrupt::disable(core, interrupt); } @@ -945,32 +941,32 @@ mod dma { #[cfg(gdma)] impl SpiDma<'_, Blocking, T> where - T: InstanceDma, + T: Instance, { /// Listen for the given interrupts pub fn listen(&mut self, interrupts: impl Into>) { - self.spi.enable_listen(interrupts.into(), true); + self.driver().enable_listen(interrupts.into(), true); } /// Unlisten the given interrupts pub fn unlisten(&mut self, interrupts: impl Into>) { - self.spi.enable_listen(interrupts.into(), false); + self.driver().enable_listen(interrupts.into(), false); } /// Gets asserted interrupts pub fn interrupts(&mut self) -> EnumSet { - self.spi.interrupts() + self.driver().interrupts() } /// Resets asserted interrupts pub fn clear_interrupts(&mut self, interrupts: impl Into>) { - self.spi.clear_interrupts(interrupts.into()); + self.driver().clear_interrupts(interrupts.into()); } } impl<'d, M, T> SpiDma<'d, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { pub(super) fn new(spi: PeripheralRef<'d, T>, channel: Channel<'d, CH, M>) -> Self @@ -986,7 +982,11 @@ mod dma { [[DmaDescriptor::EMPTY]; SPI_NUM]; static mut BUFFERS: [[u32; 1]; SPI_NUM] = [[0; 1]; SPI_NUM]; - let id = spi.spi_num() as usize - 2; + let id = if spi.info() == unsafe { crate::peripherals::SPI2::steal().info() } { + 0 + } else { + 1 + }; unwrap!(DmaTxBuf::new( unsafe { &mut DESCRIPTORS[id] }, @@ -1004,11 +1004,15 @@ mod dma { } } + fn driver(&self) -> &'static Info { + self.spi.info() + } + fn is_done(&self) -> bool { if self.tx_transfer_in_progress && !self.channel.tx.is_done() { return false; } - if self.spi.busy() { + if self.driver().busy() { return false; } if self.rx_transfer_in_progress { @@ -1073,7 +1077,7 @@ mod dma { self.rx_transfer_in_progress = rx_buffer.length() > 0; self.tx_transfer_in_progress = tx_buffer.length() > 0; unsafe { - self.spi.start_transfer_dma( + self.driver().start_transfer_dma( full_duplex, rx_buffer, tx_buffer, @@ -1097,7 +1101,7 @@ mod dma { let addr_bytes = &addr_bytes[4 - bytes_to_write..][..bytes_to_write]; self.address_buffer.fill(addr_bytes); - self.spi.setup_half_duplex( + self.driver().setup_half_duplex( true, cmd, Address::None, @@ -1109,7 +1113,7 @@ mod dma { self.tx_transfer_in_progress = true; unsafe { - self.spi.start_transfer_dma( + self.driver().start_transfer_dma( false, &mut EmptyBuf, &mut self.address_buffer, @@ -1127,8 +1131,8 @@ mod dma { // 1 seems to stop after transmitting the current byte which is somewhat less // impolite. if self.tx_transfer_in_progress || self.rx_transfer_in_progress { - self.spi.configure_datalen(1, 1); - self.spi.update(); + self.driver().configure_datalen(1, 1); + self.driver().update(); // We need to stop the DMA transfer, too. if self.tx_transfer_in_progress { @@ -1145,19 +1149,19 @@ mod dma { impl crate::private::Sealed for SpiDma<'_, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { } impl<'d, M, T> SpiDma<'d, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { /// Changes the SPI bus frequency for the DMA-enabled SPI instance. pub fn change_bus_frequency(&mut self, frequency: HertzU32) { - self.spi.ch_bus_freq(frequency); + self.driver().ch_bus_freq(frequency); } /// Configures the DMA buffers for the SPI instance. @@ -1180,7 +1184,7 @@ mod dma { /// transfer status. pub struct SpiDmaTransfer<'d, M, Buf, T = AnySpi> where - T: InstanceDma, + T: Instance, M: Mode, { spi_dma: ManuallyDrop>, @@ -1189,7 +1193,7 @@ mod dma { impl<'d, M, T, Buf> SpiDmaTransfer<'d, M, Buf, T> where - T: InstanceDma, + T: Instance, M: Mode, { fn new(spi_dma: SpiDma<'d, M, T>, dma_buf: Buf) -> Self { @@ -1233,7 +1237,7 @@ mod dma { impl Drop for SpiDmaTransfer<'_, M, Buf, T> where - T: InstanceDma, + T: Instance, M: Mode, { fn drop(&mut self) { @@ -1251,7 +1255,7 @@ mod dma { impl SpiDmaTransfer<'_, Async, Buf, T> where - T: InstanceDma, + T: Instance, { /// Waits for the DMA transfer to complete asynchronously. /// @@ -1263,7 +1267,7 @@ mod dma { impl<'d, M, T> SpiDma<'d, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { /// # Safety: @@ -1277,7 +1281,7 @@ mod dma { return Err(Error::MaxDmaTransferSizeExceeded); } - self.spi.start_transfer_dma( + self.driver().start_transfer_dma( true, &mut EmptyBuf, buffer, @@ -1316,7 +1320,7 @@ mod dma { return Err(Error::MaxDmaTransferSizeExceeded); } - self.spi.start_transfer_dma( + self.driver().start_transfer_dma( false, buffer, &mut EmptyBuf, @@ -1385,7 +1389,7 @@ mod dma { impl<'d, M, T> SpiDma<'d, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { /// # Safety: @@ -1406,7 +1410,7 @@ mod dma { return Err(Error::MaxDmaTransferSizeExceeded); } - self.spi.setup_half_duplex( + self.driver().setup_half_duplex( false, cmd, address, @@ -1416,7 +1420,7 @@ mod dma { data_mode, ); - self.spi.start_transfer_dma( + self.driver().start_transfer_dma( false, buffer, &mut EmptyBuf, @@ -1473,7 +1477,7 @@ mod dma { } } - self.spi.setup_half_duplex( + self.driver().setup_half_duplex( true, cmd, address, @@ -1483,7 +1487,7 @@ mod dma { data_mode, ); - self.spi.start_transfer_dma( + self.driver().start_transfer_dma( false, &mut EmptyBuf, buffer, @@ -1520,7 +1524,7 @@ mod dma { /// buffers. pub struct SpiDmaBus<'d, M, T = AnySpi> where - T: InstanceDma, + T: Instance, M: Mode, { @@ -1531,7 +1535,7 @@ mod dma { impl<'d, T> SpiDmaBus<'d, Blocking, T> where - T: InstanceDma, + T: Instance, { /// Converts the SPI instance into async mode. pub fn into_async(self) -> SpiDmaBus<'d, Async, T> { @@ -1545,7 +1549,7 @@ mod dma { impl<'d, T> SpiDmaBus<'d, Async, T> where - T: InstanceDma, + T: Instance, { /// Converts the SPI instance into async mode. pub fn into_blocking(self) -> SpiDmaBus<'d, Blocking, T> { @@ -1559,7 +1563,7 @@ mod dma { impl<'d, M, T> SpiDmaBus<'d, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { /// Creates a new `SpiDmaBus` with the specified SPI instance and DMA @@ -1584,7 +1588,7 @@ mod dma { impl InterruptConfigurable for SpiDmaBus<'_, Blocking, T> where - T: InstanceDma, + T: Instance, { /// Sets the interrupt handler /// @@ -1597,7 +1601,7 @@ mod dma { #[cfg(gdma)] impl SpiDmaBus<'_, Blocking, T> where - T: InstanceDma, + T: Instance, { /// Listen for the given interrupts pub fn listen(&mut self, interrupts: impl Into>) { @@ -1622,14 +1626,14 @@ mod dma { impl crate::private::Sealed for SpiDmaBus<'_, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { } impl SpiDmaBus<'_, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { /// Reads data from the SPI bus using DMA. @@ -1729,7 +1733,7 @@ mod dma { impl SpiDmaBus<'_, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { /// Half-duplex read. @@ -1798,7 +1802,7 @@ mod dma { impl embedded_hal_02::blocking::spi::Transfer for SpiDmaBus<'_, Blocking, T> where - T: InstanceDma, + T: Instance, { type Error = Error; @@ -1810,7 +1814,7 @@ mod dma { impl embedded_hal_02::blocking::spi::Write for SpiDmaBus<'_, Blocking, T> where - T: InstanceDma, + T: Instance, { type Error = Error; @@ -1870,7 +1874,7 @@ mod dma { impl SpiDmaBus<'_, Async, T> where - T: InstanceDma, + T: Instance, { /// Fill the given buffer with data from the bus. pub async fn read_async(&mut self, words: &mut [u8]) -> Result<(), Error> { @@ -1984,7 +1988,7 @@ mod dma { impl embedded_hal_async::spi::SpiBus for SpiDmaBus<'_, Async, T> where - T: InstanceDma, + T: Instance, { async fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { self.read_async(words).await @@ -2016,7 +2020,7 @@ mod dma { impl ErrorType for SpiDmaBus<'_, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { type Error = Error; @@ -2024,7 +2028,7 @@ mod dma { impl SpiBus for SpiDmaBus<'_, M, T> where - T: InstanceDma, + T: Instance, M: Mode, { fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { @@ -2066,11 +2070,11 @@ mod ehal1 { T: Instance, { fn read(&mut self) -> nb::Result { - self.spi.read_byte() + self.driver().read_byte() } fn write(&mut self, word: u8) -> nb::Result<(), Self::Error> { - self.spi.write_byte(word) + self.driver().write_byte(word) } } @@ -2079,11 +2083,11 @@ mod ehal1 { T: Instance, { fn read(&mut self, words: &mut [u8]) -> Result<(), Self::Error> { - self.spi.read_bytes(words) + self.driver().read_bytes(words) } fn write(&mut self, words: &[u8]) -> Result<(), Self::Error> { - self.spi.write_bytes(words) + self.driver().write_bytes(words) } fn transfer(&mut self, read: &mut [u8], write: &[u8]) -> Result<(), Self::Error> { @@ -2122,7 +2126,7 @@ mod ehal1 { if read_inc > 0 { SpiBus::flush(self)?; - self.spi + self.driver() .read_bytes_from_fifo(&mut read[read_from..read_to])?; } @@ -2137,181 +2141,83 @@ mod ehal1 { for chunk in words.chunks_mut(FIFO_SIZE) { SpiBus::write(self, chunk)?; SpiBus::flush(self)?; - self.spi.read_bytes_from_fifo(chunk)?; + self.driver().read_bytes_from_fifo(chunk)?; } Ok(()) } fn flush(&mut self) -> Result<(), Self::Error> { - self.spi.flush() + self.driver().flush() } } } -#[doc(hidden)] -pub trait InstanceDma: Instance + DmaEligible { - #[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, - tx: &mut TX, - ) -> Result<(), Error> { - let reg_block = self.register_block(); - - #[cfg(esp32s2)] - { - // without this a transfer 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)); - } - - let rx_len = rx_buffer.length(); - let tx_len = tx_buffer.length(); - self.configure_datalen(rx_len, tx_len); - - // enable the MISO and MOSI if needed - reg_block - .user() - .modify(|_, w| w.usr_miso().bit(rx_len > 0).usr_mosi().bit(tx_len > 0)); - - self.enable_dma(); - - 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()); - } - } - } - if tx_len > 0 { - tx.prepare_transfer(self.dma_peripheral(), tx_buffer) - .and_then(|_| tx.start_transfer())?; - } +/// SPI peripheral instance. +pub trait Instance: + private::Sealed + PeripheralMarker + Into + DmaEligible + 'static +{ + /// Returns the peripheral data describing this SPI instance. + fn info(&self) -> &'static Info; +} - #[cfg(gdma)] - self.reset_dma(); +/// Marker trait for QSPI-capable SPI peripherals. +pub trait QspiInstance: Instance {} - self.start_operation(); +/// Peripheral data describing a particular SPI instance. +pub struct Info { + /// Pointer to the register block for this SPI instance. + /// + /// Use [Self::register_block] to access the register block. + pub register_block: *const RegisterBlock, - Ok(()) - } + /// Interrupt for this SPI instance. + pub interrupt: crate::peripherals::Interrupt, - fn enable_dma(&self) { - #[cfg(gdma)] - { - // for non GDMA this is done in `assign_tx_device` / `assign_rx_device` - let reg_block = self.register_block(); - reg_block.dma_conf().modify(|_, w| { - w.dma_tx_ena().set_bit(); - w.dma_rx_ena().set_bit() - }); - } - #[cfg(pdma)] - self.reset_dma(); - } + /// DMA peripheral for this SPI instance. + // FIXME: we have this through DmaEligible, too. + pub dma_peripheral: crate::dma::DmaPeripheral, - fn reset_dma(&self) { - fn set_reset_bit(reg_block: &RegisterBlock, bit: bool) { - #[cfg(pdma)] - reg_block.dma_conf().modify(|_, w| { - 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.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(); - } + /// SCLK signal. + pub sclk: OutputSignal, - #[cfg(gdma)] - fn clear_dma_interrupts(&self) { - let reg_block = self.register_block(); - reg_block.dma_int_clr().write(|w| { - w.dma_infifo_full_err().clear_bit_by_one(); - w.dma_outfifo_empty_err().clear_bit_by_one(); - w.trans_done().clear_bit_by_one(); - w.mst_rx_afifo_wfull_err().clear_bit_by_one(); - w.mst_tx_afifo_rempty_err().clear_bit_by_one() - }); - } + /// MOSI signal. + pub mosi: OutputSignal, - #[cfg(pdma)] - fn clear_dma_interrupts(&self) { - let reg_block = self.register_block(); - reg_block.dma_int_clr().write(|w| { - w.inlink_dscr_empty().clear_bit_by_one(); - w.outlink_dscr_error().clear_bit_by_one(); - w.inlink_dscr_error().clear_bit_by_one(); - w.in_done().clear_bit_by_one(); - w.in_err_eof().clear_bit_by_one(); - w.in_suc_eof().clear_bit_by_one(); - w.out_done().clear_bit_by_one(); - w.out_eof().clear_bit_by_one(); - w.out_total_eof().clear_bit_by_one() - }); - } -} + /// MISO signal. + pub miso: InputSignal, -impl InstanceDma for crate::peripherals::SPI2 {} + /// Chip select signal. + pub cs: OutputSignal, -#[cfg(spi3)] -impl InstanceDma for crate::peripherals::SPI3 {} + /// SIO0 (MOSI) input signal for half-duplex mode. + pub sio0_input: InputSignal, -#[doc(hidden)] -pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static { - fn register_block(&self) -> &RegisterBlock; + /// SIO1 (MISO) output signal for half-duplex mode. + pub sio1_output: OutputSignal, - fn sclk_signal(&self) -> OutputSignal; - fn mosi_signal(&self) -> OutputSignal; - fn miso_signal(&self) -> InputSignal; - fn cs_signal(&self) -> OutputSignal; + /// SIO2 output signal for QSPI mode. + pub sio2_output: Option, - fn sio0_input_signal(&self) -> InputSignal; - fn sio1_output_signal(&self) -> OutputSignal; - fn sio2_output_signal(&self) -> Option; - fn sio2_input_signal(&self) -> Option; - fn sio3_output_signal(&self) -> Option; - fn sio3_input_signal(&self) -> Option; + /// SIO2 input signal for QSPI mode. + pub sio2_input: Option, - fn interrupt(&self) -> crate::peripherals::Interrupt; + /// SIO3 output signal for QSPI mode. + pub sio3_output: Option, - #[inline(always)] - fn enable_peripheral(&self) { - PeripheralClockControl::enable(self.peripheral()); - } + /// SIO3 input signal for QSPI mode. + pub sio3_input: Option, +} - #[inline(always)] - fn reset_peripheral(&self) { - PeripheralClockControl::reset(self.peripheral()); +// private implementation bits +// FIXME: split this up into peripheral versions +impl Info { + /// Returns the register block for this SPI instance. + pub fn register_block(&self) -> &RegisterBlock { + unsafe { &*self.register_block } } - fn spi_num(&self) -> u8; - /// Initialize for full-duplex 1 bit mode - fn init(&mut self) { + fn init(&self) { let reg_block = self.register_block(); reg_block.user().modify(|_, w| { w.usr_miso_highpart().clear_bit(); @@ -2336,7 +2242,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static unsafe { // use default clock source PLL_F80M_CLK (ESP32-C6) and // PLL_F48M_CLK (ESP32-H2) - (*crate::peripherals::PCR::PTR) + crate::peripherals::PCR::steal() .spi2_clkm_conf() .modify(|_, w| w.spi2_clkm_sel().bits(1)); } @@ -2366,7 +2272,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static #[cfg(not(esp32))] fn init_spi_data_mode( - &mut self, + &self, cmd_mode: SpiDataMode, address_mode: SpiDataMode, data_mode: SpiDataMode, @@ -2388,7 +2294,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static #[cfg(esp32)] fn init_spi_data_mode( - &mut self, + &self, cmd_mode: SpiDataMode, address_mode: SpiDataMode, data_mode: SpiDataMode, @@ -2436,7 +2342,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static } // taken from https://github.com/apache/incubator-nuttx/blob/8267a7618629838231256edfa666e44b5313348e/arch/risc-v/src/esp32c3/esp32c3_spi.c#L496 - fn setup(&mut self, frequency: HertzU32) { + fn setup(&self, frequency: HertzU32) { let clocks = Clocks::get(); cfg_if::cfg_if! { if #[cfg(esp32h2)] { @@ -2525,7 +2431,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static /// Enable or disable listening for the given interrupts. #[cfg(gdma)] - fn enable_listen(&mut self, interrupts: EnumSet, enable: bool) { + fn enable_listen(&self, interrupts: EnumSet, enable: bool) { let reg_block = self.register_block(); reg_block.dma_int_ena().modify(|_, w| { @@ -2540,7 +2446,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static /// Gets asserted interrupts #[cfg(gdma)] - fn interrupts(&mut self) -> EnumSet { + fn interrupts(&self) -> EnumSet { let mut res = EnumSet::new(); let reg_block = self.register_block(); @@ -2555,7 +2461,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static /// Resets asserted interrupts #[cfg(gdma)] - fn clear_interrupts(&mut self, interrupts: EnumSet) { + fn clear_interrupts(&self, interrupts: EnumSet) { let reg_block = self.register_block(); reg_block.dma_int_clr().write(|w| { @@ -2568,7 +2474,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static }); } - fn set_data_mode(&mut self, data_mode: SpiMode) -> &mut Self { + fn set_data_mode(&self, data_mode: SpiMode) { let reg_block = self.register_block(); cfg_if::cfg_if! { @@ -2587,11 +2493,9 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static w.ck_out_edge() .bit(matches!(data_mode, SpiMode::Mode1 | SpiMode::Mode2)) }); - - self } - fn ch_bus_freq(&mut self, frequency: HertzU32) { + fn ch_bus_freq(&self, frequency: HertzU32) { fn enable_clocks(_reg_block: &RegisterBlock, _enable: bool) { #[cfg(gdma)] _reg_block.clk_gate().modify(|_, w| { @@ -2610,7 +2514,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static } #[cfg(not(any(esp32, esp32c3, esp32s2)))] - fn set_bit_order(&mut self, read_order: SpiBitOrder, write_order: SpiBitOrder) { + fn set_bit_order(&self, read_order: SpiBitOrder, write_order: SpiBitOrder) { let reg_block = self.register_block(); let read_value = match read_order { @@ -2627,8 +2531,9 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static w }); } + #[cfg(any(esp32, esp32c3, esp32s2))] - fn set_bit_order(&mut self, read_order: SpiBitOrder, write_order: SpiBitOrder) { + fn set_bit_order(&self, read_order: SpiBitOrder, write_order: SpiBitOrder) { let reg_block = self.register_block(); let read_value = match read_order { @@ -2646,7 +2551,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static }); } - fn read_byte(&mut self) -> nb::Result { + fn read_byte(&self) -> nb::Result { if self.busy() { return Err(nb::Error::WouldBlock); } @@ -2655,7 +2560,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static Ok(u32::try_into(reg_block.w(0).read().bits()).unwrap_or_default()) } - fn write_byte(&mut self, word: u8) -> nb::Result<(), Error> { + fn write_byte(&self, word: u8) -> nb::Result<(), Error> { if self.busy() { return Err(nb::Error::WouldBlock); } @@ -2679,7 +2584,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static /// you must ensure that the whole messages was written correctly, use /// [`Self::flush`]. #[cfg_attr(place_spi_driver_in_ram, ram)] - fn write_bytes(&mut self, words: &[u8]) -> Result<(), Error> { + fn write_bytes(&self, words: &[u8]) -> Result<(), Error> { let num_chunks = words.len() / FIFO_SIZE; // Flush in case previous writes have not completed yet, required as per @@ -2736,7 +2641,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static /// perform flushing. If you want to read the response to something you /// have written before, consider using [`Self::transfer`] instead. #[cfg_attr(place_spi_driver_in_ram, ram)] - fn read_bytes(&mut self, words: &mut [u8]) -> Result<(), Error> { + fn read_bytes(&self, words: &mut [u8]) -> Result<(), Error> { let empty_array = [EMPTY_WRITE_PAD; FIFO_SIZE]; for chunk in words.chunks_mut(FIFO_SIZE) { @@ -2754,7 +2659,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static /// something you have written before, consider using [`Self::transfer`] /// instead. #[cfg_attr(place_spi_driver_in_ram, ram)] - fn read_bytes_from_fifo(&mut self, words: &mut [u8]) -> Result<(), Error> { + fn read_bytes_from_fifo(&self, words: &mut [u8]) -> Result<(), Error> { let reg_block = self.register_block(); for chunk in words.chunks_mut(FIFO_SIZE) { @@ -2778,7 +2683,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static } // Check if the bus is busy and if it is wait for it to be idle - fn flush(&mut self) -> Result<(), Error> { + fn flush(&self) -> Result<(), Error> { while self.busy() { // wait for bus to be clear } @@ -2786,7 +2691,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static } #[cfg_attr(place_spi_driver_in_ram, ram)] - fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Error> { + fn transfer<'w>(&self, words: &'w mut [u8]) -> Result<&'w [u8], Error> { for chunk in words.chunks_mut(FIFO_SIZE) { self.write_bytes(chunk)?; self.flush()?; @@ -2804,7 +2709,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static #[allow(clippy::too_many_arguments)] fn setup_half_duplex( - &mut self, + &self, is_write: bool, cmd: Command, address: Address, @@ -2838,7 +2743,7 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static #[cfg(any(esp32c6, esp32h2))] unsafe { - let pcr = &*crate::peripherals::PCR::PTR; + let pcr = crate::peripherals::PCR::steal(); // use default clock source PLL_F80M_CLK pcr.spi2_clkm_conf() @@ -2853,7 +2758,37 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static self.update(); // set cmd, address, dummy cycles - set_up_common_phases(reg_block, cmd, address, dummy); + self.set_up_common_phases(cmd, address, dummy); + } + + fn set_up_common_phases(&self, cmd: Command, address: Address, dummy: u8) { + let reg_block = self.register_block(); + if !cmd.is_none() { + reg_block.user2().modify(|_, w| unsafe { + w.usr_command_bitlen().bits((cmd.width() - 1) as u8); + w.usr_command_value().bits(cmd.value()) + }); + } + + if !address.is_none() { + reg_block + .user1() + .modify(|_, w| unsafe { w.usr_addr_bitlen().bits((address.width() - 1) as u8) }); + + let addr = address.value() << (32 - address.width()); + #[cfg(not(esp32))] + reg_block + .addr() + .write(|w| unsafe { w.usr_addr_value().bits(addr) }); + #[cfg(esp32)] + reg_block.addr().write(|w| unsafe { w.bits(addr) }); + } + + if dummy > 0 { + reg_block + .user1() + .modify(|_, w| unsafe { w.usr_dummy_cyclelen().bits(dummy - 1) }); + } } fn update(&self) { @@ -2909,109 +2844,165 @@ pub trait Instance: private::Sealed + PeripheralMarker + Into + 'static } } -} -#[doc(hidden)] -pub trait QspiInstance: Instance {} - -fn set_up_common_phases(reg_block: &RegisterBlock, cmd: Command, address: Address, dummy: u8) { - if !cmd.is_none() { - reg_block.user2().modify(|_, w| unsafe { - w.usr_command_bitlen().bits((cmd.width() - 1) as u8); - w.usr_command_value().bits(cmd.value()) - }); - } + #[cfg_attr(place_spi_driver_in_ram, ram)] + unsafe fn start_transfer_dma( + &self, + _full_duplex: bool, + rx_buffer: &mut impl DmaRxBuffer, + tx_buffer: &mut impl DmaTxBuffer, + rx: &mut RX, + tx: &mut TX, + ) -> Result<(), Error> { + let reg_block = self.register_block(); - if !address.is_none() { - reg_block - .user1() - .modify(|_, w| unsafe { w.usr_addr_bitlen().bits((address.width() - 1) as u8) }); + #[cfg(esp32s2)] + { + // without this a transfer 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)); + } - let addr = address.value() << (32 - address.width()); - #[cfg(not(esp32))] - reg_block - .addr() - .write(|w| unsafe { w.usr_addr_value().bits(addr) }); - #[cfg(esp32)] - reg_block.addr().write(|w| unsafe { w.bits(addr) }); - } + let rx_len = rx_buffer.length(); + let tx_len = tx_buffer.length(); + self.configure_datalen(rx_len, tx_len); - if dummy > 0 { + // enable the MISO and MOSI if needed reg_block - .user1() - .modify(|_, w| unsafe { w.usr_dummy_cyclelen().bits(dummy - 1) }); - } -} - -macro_rules! spi_instance { - (@ignore $sio2:ident) => {}; + .user() + .modify(|_, w| w.usr_miso().bit(rx_len > 0).usr_mosi().bit(tx_len > 0)); - ($num:literal, $sclk:ident, $mosi:ident, $miso:ident, $cs:ident $(, $sio2:ident, $sio3:ident)?) => { - paste::paste! { - impl Instance for crate::peripherals::[] { - #[inline(always)] - fn register_block(&self) -> &RegisterBlock { - self - } + self.enable_dma(); - #[inline(always)] - fn spi_num(&self) -> u8 { - $num + 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()); } + } + } + if tx_len > 0 { + tx.prepare_transfer(self.dma_peripheral, tx_buffer) + .and_then(|_| tx.start_transfer())?; + } - #[inline(always)] - fn interrupt(&self) -> crate::peripherals::Interrupt { - crate::peripherals::Interrupt::[] - } + #[cfg(gdma)] + self.reset_dma(); - #[inline(always)] - fn sclk_signal(&self) -> OutputSignal { - OutputSignal::$sclk - } + self.start_operation(); - #[inline(always)] - fn mosi_signal(&self) -> OutputSignal { - OutputSignal::$mosi - } + Ok(()) + } - #[inline(always)] - fn miso_signal(&self) -> InputSignal { - InputSignal::$miso - } + fn enable_dma(&self) { + #[cfg(gdma)] + { + // for non GDMA this is done in `assign_tx_device` / `assign_rx_device` + let reg_block = self.register_block(); + reg_block.dma_conf().modify(|_, w| { + w.dma_tx_ena().set_bit(); + w.dma_rx_ena().set_bit() + }); + } + #[cfg(pdma)] + self.reset_dma(); + } - #[inline(always)] - fn cs_signal(&self) -> OutputSignal { - OutputSignal::$cs - } + fn reset_dma(&self) { + fn set_reset_bit(reg_block: &RegisterBlock, bit: bool) { + #[cfg(pdma)] + reg_block.dma_conf().modify(|_, w| { + 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.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(); + } - #[inline(always)] - fn sio0_input_signal(&self) -> InputSignal { - InputSignal::$mosi - } + #[cfg(gdma)] + fn clear_dma_interrupts(&self) { + let reg_block = self.register_block(); + reg_block.dma_int_clr().write(|w| { + w.dma_infifo_full_err().clear_bit_by_one(); + w.dma_outfifo_empty_err().clear_bit_by_one(); + w.trans_done().clear_bit_by_one(); + w.mst_rx_afifo_wfull_err().clear_bit_by_one(); + w.mst_tx_afifo_rempty_err().clear_bit_by_one() + }); + } - #[inline(always)] - fn sio1_output_signal(&self) -> OutputSignal { - OutputSignal::$miso - } + #[cfg(pdma)] + fn clear_dma_interrupts(&self) { + let reg_block = self.register_block(); + reg_block.dma_int_clr().write(|w| { + w.inlink_dscr_empty().clear_bit_by_one(); + w.outlink_dscr_error().clear_bit_by_one(); + w.inlink_dscr_error().clear_bit_by_one(); + w.in_done().clear_bit_by_one(); + w.in_err_eof().clear_bit_by_one(); + w.in_suc_eof().clear_bit_by_one(); + w.out_done().clear_bit_by_one(); + w.out_eof().clear_bit_by_one(); + w.out_total_eof().clear_bit_by_one() + }); + } +} - #[inline(always)] - fn sio2_output_signal(&self) -> Option { - if_set!($(Some(OutputSignal::$sio2))?, None) - } +impl PartialEq for Info { + fn eq(&self, other: &Self) -> bool { + self.register_block == other.register_block + } +} - #[inline(always)] - fn sio2_input_signal(&self) -> Option { - if_set!($(Some(InputSignal::$sio2))?, None) - } +unsafe impl Sync for Info {} - #[inline(always)] - fn sio3_output_signal(&self) -> Option { - if_set!($(Some(OutputSignal::$sio3))?, None) - } +macro_rules! spi_instance { + (@ignore $sio2:ident) => {}; + ($num:literal, $sclk:ident, $mosi:ident, $miso:ident, $cs:ident $(, $sio2:ident, $sio3:ident)?) => { + paste::paste! { + impl Instance for crate::peripherals::[] { #[inline(always)] - fn sio3_input_signal(&self) -> Option { - if_set!($(Some(InputSignal::$sio3))?, None) + fn info(&self) -> &'static Info { + static INFO: Info = Info { + register_block: crate::peripherals::[]::PTR, + interrupt: crate::peripherals::Interrupt::[], + dma_peripheral: crate::dma::DmaPeripheral::[], + sclk: OutputSignal::$sclk, + mosi: OutputSignal::$mosi, + miso: InputSignal::$miso, + cs: OutputSignal::$cs, + sio0_input: InputSignal::$mosi, + sio1_output: OutputSignal::$miso, + sio2_output: if_set!($(Some(OutputSignal::$sio2))?, None), + sio2_input: if_set!($(Some(InputSignal::$sio2))?, None), + sio3_output: if_set!($(Some(OutputSignal::$sio3))?, None), + sio3_input: if_set!($(Some(InputSignal::$sio3))?, None), + }; + + &INFO } } @@ -3064,35 +3055,9 @@ impl Instance for super::AnySpi { #[cfg(spi3)] super::AnySpiInner::Spi3(spi) => spi, } { - fn register_block(&self) -> &RegisterBlock; - fn spi_num(&self) -> u8; - fn sclk_signal(&self) -> OutputSignal; - fn mosi_signal(&self) -> OutputSignal; - fn miso_signal(&self) -> InputSignal; - fn cs_signal(&self) -> OutputSignal; - - fn sio0_input_signal(&self) -> InputSignal; - fn sio1_output_signal(&self) -> OutputSignal; - fn sio2_output_signal(&self) -> Option; - fn sio2_input_signal(&self) -> Option; - fn sio3_output_signal(&self) -> Option; - fn sio3_input_signal(&self) -> Option; - - fn interrupt(&self) -> crate::peripherals::Interrupt; + fn info(&self) -> &'static Info; } } } impl QspiInstance for super::AnySpi {} - -impl InstanceDma for super::AnySpi { - delegate::delegate! { - to match &self.0 { - super::AnySpiInner::Spi2(spi) => spi, - #[cfg(spi3)] - super::AnySpiInner::Spi3(spi) => spi, - } { - fn clear_dma_interrupts(&self); - } - } -} From 7b747f4e0f2acd29595fdd19c09b0de9e8f17a62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 1 Nov 2024 23:26:17 +0100 Subject: [PATCH 2/8] Fix in_progress flags not being set --- esp-hal/src/spi/master.rs | 207 +++++++++++++++++--------------------- 1 file changed, 93 insertions(+), 114 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 55d65ee4a0..79728a8eba 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -1008,6 +1008,13 @@ mod dma { self.spi.info() } + fn dma_driver(&self) -> DmaDriver { + DmaDriver { + info: self.driver(), + dma_peripheral: self.spi.dma_peripheral(), + } + } + fn is_done(&self) -> bool { if self.tx_transfer_in_progress && !self.channel.tx.is_done() { return false; @@ -1074,10 +1081,19 @@ mod dma { rx_buffer: &mut RX, tx_buffer: &mut TX, ) -> Result<(), Error> { - self.rx_transfer_in_progress = rx_buffer.length() > 0; - self.tx_transfer_in_progress = tx_buffer.length() > 0; + let bytes_to_read = rx_buffer.length(); + if bytes_to_read > MAX_DMA_SIZE { + return Err(Error::MaxDmaTransferSizeExceeded); + } + let bytes_to_write = tx_buffer.length(); + if bytes_to_write > MAX_DMA_SIZE { + return Err(Error::MaxDmaTransferSizeExceeded); + } + + self.rx_transfer_in_progress = bytes_to_read > 0; + self.tx_transfer_in_progress = bytes_to_write > 0; unsafe { - self.driver().start_transfer_dma( + self.dma_driver().start_transfer_dma( full_duplex, rx_buffer, tx_buffer, @@ -1111,9 +1127,11 @@ mod dma { address.mode(), ); + // FIXME: we could use self.start_transfer_dma if the address buffer was part of + // the (yet-to-be-created) State struct. self.tx_transfer_in_progress = true; unsafe { - self.driver().start_transfer_dma( + self.dma_driver().start_transfer_dma( false, &mut EmptyBuf, &mut self.address_buffer, @@ -1276,18 +1294,7 @@ mod dma { /// transfer is in progress. Moving the buffers is allowed. #[cfg_attr(place_spi_driver_in_ram, ram)] unsafe fn start_dma_write(&mut self, buffer: &mut impl DmaTxBuffer) -> Result<(), Error> { - let bytes_to_write = buffer.length(); - if bytes_to_write > MAX_DMA_SIZE { - return Err(Error::MaxDmaTransferSizeExceeded); - } - - self.driver().start_transfer_dma( - true, - &mut EmptyBuf, - buffer, - &mut self.channel.rx, - &mut self.channel.tx, - ) + self.start_transfer_dma(true, &mut EmptyBuf, buffer) } /// Perform a DMA write. @@ -1315,18 +1322,7 @@ mod dma { /// transfer is in progress. Moving the buffers is allowed. #[cfg_attr(place_spi_driver_in_ram, ram)] unsafe fn start_dma_read(&mut self, buffer: &mut impl DmaRxBuffer) -> Result<(), Error> { - let bytes_to_read = buffer.length(); - if bytes_to_read > MAX_DMA_SIZE { - return Err(Error::MaxDmaTransferSizeExceeded); - } - - self.driver().start_transfer_dma( - false, - buffer, - &mut EmptyBuf, - &mut self.channel.rx, - &mut self.channel.tx, - ) + self.start_transfer_dma(false, buffer, &mut EmptyBuf) } /// Perform a DMA read. @@ -1357,13 +1353,6 @@ mod dma { rx_buffer: &mut impl DmaRxBuffer, tx_buffer: &mut impl DmaTxBuffer, ) -> Result<(), Error> { - let bytes_to_read = rx_buffer.length(); - let bytes_to_write = tx_buffer.length(); - - if bytes_to_write > MAX_DMA_SIZE || bytes_to_read > MAX_DMA_SIZE { - return Err(Error::MaxDmaTransferSizeExceeded); - } - self.start_transfer_dma(true, rx_buffer, tx_buffer) } @@ -1420,13 +1409,7 @@ mod dma { data_mode, ); - self.driver().start_transfer_dma( - false, - buffer, - &mut EmptyBuf, - &mut self.channel.rx, - &mut self.channel.tx, - ) + self.start_transfer_dma(false, buffer, &mut EmptyBuf) } /// Perform a half-duplex read operation using DMA. @@ -1487,13 +1470,7 @@ mod dma { data_mode, ); - self.driver().start_transfer_dma( - false, - &mut EmptyBuf, - buffer, - &mut self.channel.rx, - &mut self.channel.tx, - ) + self.start_transfer_dma(false, &mut EmptyBuf, buffer) } /// Perform a half-duplex write operation using DMA. @@ -2173,10 +2150,6 @@ pub struct Info { /// Interrupt for this SPI instance. pub interrupt: crate::peripherals::Interrupt, - /// DMA peripheral for this SPI instance. - // FIXME: we have this through DmaEligible, too. - pub dma_peripheral: crate::dma::DmaPeripheral, - /// SCLK signal. pub sclk: OutputSignal, @@ -2208,6 +2181,73 @@ pub struct Info { pub sio3_input: Option, } +struct DmaDriver { + info: &'static Info, + dma_peripheral: crate::dma::DmaPeripheral, +} + +impl DmaDriver { + #[cfg_attr(place_spi_driver_in_ram, ram)] + unsafe fn start_transfer_dma( + &self, + _full_duplex: bool, + rx_buffer: &mut impl DmaRxBuffer, + tx_buffer: &mut impl DmaTxBuffer, + rx: &mut RX, + tx: &mut TX, + ) -> Result<(), Error> { + let reg_block = self.info.register_block(); + + #[cfg(esp32s2)] + { + // without this a transfer 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)); + } + + let rx_len = rx_buffer.length(); + let tx_len = tx_buffer.length(); + self.info.configure_datalen(rx_len, tx_len); + + // enable the MISO and MOSI if needed + reg_block + .user() + .modify(|_, w| w.usr_miso().bit(rx_len > 0).usr_mosi().bit(tx_len > 0)); + + self.info.enable_dma(); + + 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()); + } + } + } + if tx_len > 0 { + tx.prepare_transfer(self.dma_peripheral, tx_buffer) + .and_then(|_| tx.start_transfer())?; + } + + #[cfg(gdma)] + self.info.reset_dma(); + + self.info.start_operation(); + + Ok(()) + } +} + // private implementation bits // FIXME: split this up into peripheral versions impl Info { @@ -2845,66 +2885,6 @@ impl Info { } } - #[cfg_attr(place_spi_driver_in_ram, ram)] - unsafe fn start_transfer_dma( - &self, - _full_duplex: bool, - rx_buffer: &mut impl DmaRxBuffer, - tx_buffer: &mut impl DmaTxBuffer, - rx: &mut RX, - tx: &mut TX, - ) -> Result<(), Error> { - let reg_block = self.register_block(); - - #[cfg(esp32s2)] - { - // without this a transfer 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)); - } - - let rx_len = rx_buffer.length(); - let tx_len = tx_buffer.length(); - self.configure_datalen(rx_len, tx_len); - - // enable the MISO and MOSI if needed - reg_block - .user() - .modify(|_, w| w.usr_miso().bit(rx_len > 0).usr_mosi().bit(tx_len > 0)); - - self.enable_dma(); - - 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()); - } - } - } - if tx_len > 0 { - tx.prepare_transfer(self.dma_peripheral, tx_buffer) - .and_then(|_| tx.start_transfer())?; - } - - #[cfg(gdma)] - self.reset_dma(); - - self.start_operation(); - - Ok(()) - } - fn enable_dma(&self) { #[cfg(gdma)] { @@ -2989,7 +2969,6 @@ macro_rules! spi_instance { static INFO: Info = Info { register_block: crate::peripherals::[]::PTR, interrupt: crate::peripherals::Interrupt::[], - dma_peripheral: crate::dma::DmaPeripheral::[], sclk: OutputSignal::$sclk, mosi: OutputSignal::$mosi, miso: InputSignal::$miso, From 38366344c556f430fd030f5201972bd5cce09197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Sat, 2 Nov 2024 10:40:18 +0100 Subject: [PATCH 3/8] Remove redundant checks, fix full-duplex flag --- esp-hal/src/spi/master.rs | 162 ++++++++++++++++++-------------------- 1 file changed, 76 insertions(+), 86 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 79728a8eba..1ae10753c2 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -1082,11 +1082,9 @@ mod dma { tx_buffer: &mut TX, ) -> Result<(), Error> { let bytes_to_read = rx_buffer.length(); - if bytes_to_read > MAX_DMA_SIZE { - return Err(Error::MaxDmaTransferSizeExceeded); - } let bytes_to_write = tx_buffer.length(); - if bytes_to_write > MAX_DMA_SIZE { + + if bytes_to_read > MAX_DMA_SIZE || bytes_to_write > MAX_DMA_SIZE { return Err(Error::MaxDmaTransferSizeExceeded); } @@ -1149,8 +1147,7 @@ mod dma { // 1 seems to stop after transmitting the current byte which is somewhat less // impolite. if self.tx_transfer_in_progress || self.rx_transfer_in_progress { - self.driver().configure_datalen(1, 1); - self.driver().update(); + self.dma_driver().abort_transfer(); // We need to stop the DMA transfer, too. if self.tx_transfer_in_progress { @@ -1294,7 +1291,7 @@ mod dma { /// transfer is in progress. Moving the buffers is allowed. #[cfg_attr(place_spi_driver_in_ram, ram)] unsafe fn start_dma_write(&mut self, buffer: &mut impl DmaTxBuffer) -> Result<(), Error> { - self.start_transfer_dma(true, &mut EmptyBuf, buffer) + self.start_dma_transfer(&mut EmptyBuf, buffer) } /// Perform a DMA write. @@ -1322,7 +1319,7 @@ mod dma { /// transfer is in progress. Moving the buffers is allowed. #[cfg_attr(place_spi_driver_in_ram, ram)] unsafe fn start_dma_read(&mut self, buffer: &mut impl DmaRxBuffer) -> Result<(), Error> { - self.start_transfer_dma(false, buffer, &mut EmptyBuf) + self.start_dma_transfer(buffer, &mut EmptyBuf) } /// Perform a DMA read. @@ -1374,13 +1371,7 @@ mod dma { Err(e) => Err((e, self, rx_buffer, tx_buffer)), } } - } - impl<'d, M, T> SpiDma<'d, M, T> - where - T: Instance, - M: Mode, - { /// # Safety: /// /// The caller must ensure that the buffers are not accessed while the @@ -1395,9 +1386,6 @@ mod dma { buffer: &mut impl DmaRxBuffer, ) -> Result<(), Error> { let bytes_to_read = buffer.length(); - if bytes_to_read > MAX_DMA_SIZE { - return Err(Error::MaxDmaTransferSizeExceeded); - } self.driver().setup_half_duplex( false, @@ -1447,9 +1435,6 @@ mod dma { buffer: &mut impl DmaTxBuffer, ) -> Result<(), Error> { let bytes_to_write = buffer.length(); - if bytes_to_write > MAX_DMA_SIZE { - return Err(Error::MaxDmaTransferSizeExceeded); - } #[cfg(all(esp32, spi_address_workaround))] { @@ -2187,6 +2172,11 @@ struct DmaDriver { } impl DmaDriver { + fn abort_transfer(&self) { + self.info.configure_datalen(1, 1); + self.info.update(); + } + #[cfg_attr(place_spi_driver_in_ram, ram)] unsafe fn start_transfer_dma( &self, @@ -2214,7 +2204,7 @@ impl DmaDriver { .user() .modify(|_, w| w.usr_miso().bit(rx_len > 0).usr_mosi().bit(tx_len > 0)); - self.info.enable_dma(); + self.enable_dma(); if rx_len > 0 { rx.prepare_transfer(self.dma_peripheral, rx_buffer) @@ -2240,12 +2230,76 @@ impl DmaDriver { } #[cfg(gdma)] - self.info.reset_dma(); + self.reset_dma(); self.info.start_operation(); Ok(()) } + + fn enable_dma(&self) { + #[cfg(gdma)] + { + // for non GDMA this is done in `assign_tx_device` / `assign_rx_device` + let reg_block = self.info.register_block(); + reg_block.dma_conf().modify(|_, w| { + w.dma_tx_ena().set_bit(); + w.dma_rx_ena().set_bit() + }); + } + #[cfg(pdma)] + self.reset_dma(); + } + + fn reset_dma(&self) { + fn set_reset_bit(reg_block: &RegisterBlock, bit: bool) { + #[cfg(pdma)] + reg_block.dma_conf().modify(|_, w| { + 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.rx_afifo_rst().bit(bit); + w.buf_afifo_rst().bit(bit); + w.dma_afifo_rst().bit(bit) + }); + } + let reg_block = self.info.register_block(); + set_reset_bit(reg_block, true); + set_reset_bit(reg_block, false); + self.clear_dma_interrupts(); + } + + #[cfg(gdma)] + fn clear_dma_interrupts(&self) { + let reg_block = self.info.register_block(); + reg_block.dma_int_clr().write(|w| { + w.dma_infifo_full_err().clear_bit_by_one(); + w.dma_outfifo_empty_err().clear_bit_by_one(); + w.trans_done().clear_bit_by_one(); + w.mst_rx_afifo_wfull_err().clear_bit_by_one(); + w.mst_tx_afifo_rempty_err().clear_bit_by_one() + }); + } + + #[cfg(pdma)] + fn clear_dma_interrupts(&self) { + let reg_block = self.info.register_block(); + reg_block.dma_int_clr().write(|w| { + w.inlink_dscr_empty().clear_bit_by_one(); + w.outlink_dscr_error().clear_bit_by_one(); + w.inlink_dscr_error().clear_bit_by_one(); + w.in_done().clear_bit_by_one(); + w.in_err_eof().clear_bit_by_one(); + w.in_suc_eof().clear_bit_by_one(); + w.out_done().clear_bit_by_one(); + w.out_eof().clear_bit_by_one(); + w.out_total_eof().clear_bit_by_one() + }); + } } // private implementation bits @@ -2884,70 +2938,6 @@ impl Info { } } - - fn enable_dma(&self) { - #[cfg(gdma)] - { - // for non GDMA this is done in `assign_tx_device` / `assign_rx_device` - let reg_block = self.register_block(); - reg_block.dma_conf().modify(|_, w| { - w.dma_tx_ena().set_bit(); - w.dma_rx_ena().set_bit() - }); - } - #[cfg(pdma)] - self.reset_dma(); - } - - fn reset_dma(&self) { - fn set_reset_bit(reg_block: &RegisterBlock, bit: bool) { - #[cfg(pdma)] - reg_block.dma_conf().modify(|_, w| { - 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.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(); - } - - #[cfg(gdma)] - fn clear_dma_interrupts(&self) { - let reg_block = self.register_block(); - reg_block.dma_int_clr().write(|w| { - w.dma_infifo_full_err().clear_bit_by_one(); - w.dma_outfifo_empty_err().clear_bit_by_one(); - w.trans_done().clear_bit_by_one(); - w.mst_rx_afifo_wfull_err().clear_bit_by_one(); - w.mst_tx_afifo_rempty_err().clear_bit_by_one() - }); - } - - #[cfg(pdma)] - fn clear_dma_interrupts(&self) { - let reg_block = self.register_block(); - reg_block.dma_int_clr().write(|w| { - w.inlink_dscr_empty().clear_bit_by_one(); - w.outlink_dscr_error().clear_bit_by_one(); - w.inlink_dscr_error().clear_bit_by_one(); - w.in_done().clear_bit_by_one(); - w.in_err_eof().clear_bit_by_one(); - w.in_suc_eof().clear_bit_by_one(); - w.out_done().clear_bit_by_one(); - w.out_eof().clear_bit_by_one(); - w.out_total_eof().clear_bit_by_one() - }); - } } impl PartialEq for Info { From 233bd44e3737798dbd8860a07da695515fe4ea21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 4 Nov 2024 13:11:58 +0100 Subject: [PATCH 4/8] Remove now-redundant Send impl --- esp-hal/src/spi/master.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 1ae10753c2..e08a974630 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -899,14 +899,6 @@ mod dma { } } - #[cfg(all(esp32, spi_address_workaround))] - unsafe impl<'d, M, T> Send for SpiDma<'d, M, T> - where - T: Instance, - M: Mode, - { - } - impl core::fmt::Debug for SpiDma<'_, M, T> where T: Instance, From 7408f9c9b2a8233e33cfb836cd06df5e5e0ef427 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 4 Nov 2024 13:29:19 +0100 Subject: [PATCH 5/8] apply_config --- esp-hal/CHANGELOG.md | 3 + esp-hal/MIGRATING-0.21.md | 12 +++- esp-hal/src/spi/master.rs | 143 +++++++++++++++++++++----------------- 3 files changed, 95 insertions(+), 63 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index af6fddbed6..1abb466d3c 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `{Uart, UartRx, UartTx}::apply_config()` (#2449) - `{Uart, UartRx, UartTx}` now implement `embassy_embedded_hal::SetConfig` (#2449) - GPIO ETM tasks and events now accept `InputSignal` and `OutputSignal` (#2427) +- `spi::master::Config` and `{Spi, SpiDma, SpiDmaBus}::apply_config` (#2448) ### Changed @@ -50,6 +51,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `rmt::asynch::RxChannelAsync` and `rmt::asynch::TxChannelAsync` traits have been moved to `rmt` (#2430) - Calling `AnyPin::output_signals` on an input-only pin (ESP32 GPIO 34-39) will now result in a panic. (#2418) - UART configuration types have been moved to `esp_hal::uart` (#2449) +- `spi::master::Spi::new()` no longer takes `frequency` and `mode` as a parameter. (#2448) ### Fixed @@ -85,6 +87,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `SysTimerEtm` prefix has been removed from `timer::systimer::etm` types (#2427) - The `GpioEtmEventRising`, `GpioEtmEventFalling`, `GpioEtmEventAny` types have been replaced with `Event` (#2427) - The `TaskSet`, `TaskClear`, `TaskToggle` types have been replaced with `Task` (#2427) +- `{Spi, SpiDma, SpiDmaBus}` configuration methods (#2448) ## [0.21.1] diff --git a/esp-hal/MIGRATING-0.21.md b/esp-hal/MIGRATING-0.21.md index 9391c1b324..c8c9b695dd 100644 --- a/esp-hal/MIGRATING-0.21.md +++ b/esp-hal/MIGRATING-0.21.md @@ -251,4 +251,14 @@ The module's contents have been moved into `uart`. ``` If you work with multiple configurable peripherals, you may want to import the `uart` module and -refer to the `Config` struct as `uart::Config`. \ No newline at end of file +refer to the `Config` struct as `uart::Config`. + +### SPI drivers can now be configured using `spi::master::Config` + +- The old methods to change configuration have been removed. +- The `new` and `new_typed` constructor no longer takes `frequency` and `mode`. +- The default configuration is now: + - bus frequency: 1 MHz + - bit order: MSB first + - mode: SPI mode 0 +- There are new constructors (`new_with_config`, `new_typed_with_config`) and a new `apply_config` method to apply custom configuration. diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index e08a974630..d66fd7f3a0 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -435,6 +435,35 @@ impl Address { } } +/// SPI peripheral configuration +#[derive(Clone, Copy, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub struct Config { + /// SPI clock frequency + pub frequency: HertzU32, + + /// SPI mode + pub mode: SpiMode, + + /// Bit order of the read data. + pub read_bit_order: SpiBitOrder, + + /// Bit order of the written data. + pub write_bit_order: SpiBitOrder, +} + +impl Default for Config { + fn default() -> Self { + use fugit::RateExtU32; + Config { + frequency: 1_u32.MHz(), + mode: SpiMode::Mode0, + read_bit_order: SpiBitOrder::MSBFirst, + write_bit_order: SpiBitOrder::MSBFirst, + } + } +} + /// SPI peripheral driver pub struct Spi<'d, M, T = AnySpi> { spi: PeripheralRef<'d, T>, @@ -503,12 +532,16 @@ where impl<'d> Spi<'d, Blocking> { /// Constructs an SPI instance in 8bit dataframe mode. - pub fn new( + pub fn new(spi: impl Peripheral

+ 'd) -> Spi<'d, Blocking> { + Self::new_with_config(spi, Config::default()) + } + + /// Constructs an SPI instance in 8bit dataframe mode. + pub fn new_with_config( spi: impl Peripheral

+ 'd, - frequency: HertzU32, - mode: SpiMode, + config: Config, ) -> Spi<'d, Blocking> { - Self::new_typed(spi.map_into(), frequency, mode) + Self::new_typed_with_config(spi.map_into(), config) } /// Converts the SPI instance into async mode. @@ -535,14 +568,18 @@ where T: Instance, { /// Constructs an SPI instance in 8bit dataframe mode. - pub fn new_typed( + pub fn new_typed(spi: impl Peripheral

+ 'd) -> Spi<'d, M, T> { + Self::new_typed_with_config(spi, Config::default()) + } + + /// Constructs an SPI instance in 8bit dataframe mode. + pub fn new_typed_with_config( spi: impl Peripheral

+ 'd, - frequency: HertzU32, - mode: SpiMode, + config: Config, ) -> Spi<'d, M, T> { crate::into_ref!(spi); - let this = Spi { + let mut this = Spi { spi, _mode: PhantomData, }; @@ -550,9 +587,8 @@ where PeripheralClockControl::enable(this.spi.peripheral()); PeripheralClockControl::reset(this.spi.peripheral()); - this.driver().setup(frequency); this.driver().init(); - this.driver().set_data_mode(mode); + this.apply_config(&config); let this = this .with_mosi(NoPin) @@ -635,20 +671,9 @@ where self } - /// Change the bus frequency of the SPI instance in half-duplex mode. - /// - /// This method allows you to update the bus frequency for the SPI - /// communication after the instance has been created. - pub fn change_bus_frequency(&mut self, frequency: HertzU32) { - self.driver().ch_bus_freq(frequency); - } - - /// Set the bit order for the SPI instance. - /// - /// The default is MSB first for both read and write. - pub fn with_bit_order(self, read_order: SpiBitOrder, write_order: SpiBitOrder) -> Self { - self.driver().set_bit_order(read_order, write_order); - self + /// Change the bus configuration. + pub fn apply_config(&mut self, config: &Config) { + self.driver().apply_config(config); } } @@ -865,6 +890,13 @@ mod dma { address_buffer: DmaTxBuf, } + impl crate::private::Sealed for SpiDma<'_, M, T> + where + T: Instance, + M: Mode, + { + } + impl<'d, T> SpiDma<'d, Blocking, T> where T: Instance, @@ -1152,23 +1184,10 @@ mod dma { } } } - } - impl crate::private::Sealed for SpiDma<'_, M, T> - where - T: Instance, - M: Mode, - { - } - - impl<'d, M, T> SpiDma<'d, M, T> - where - T: Instance, - M: Mode, - { - /// Changes the SPI bus frequency for the DMA-enabled SPI instance. - pub fn change_bus_frequency(&mut self, frequency: HertzU32) { - self.driver().ch_bus_freq(frequency); + /// Change the bus configuration. + pub fn apply_config(&mut self, config: &Config) { + self.driver().apply_config(config); } /// Configures the DMA buffers for the SPI instance. @@ -1487,6 +1506,13 @@ mod dma { tx_buf: DmaTxBuf, } + impl crate::private::Sealed for SpiDmaBus<'_, M, T> + where + T: Instance, + M: Mode, + { + } + impl<'d, T> SpiDmaBus<'d, Blocking, T> where T: Instance, @@ -1529,15 +1555,6 @@ mod dma { tx_buf, } } - - fn wait_for_idle(&mut self) { - self.spi_dma.wait_for_idle(); - } - - /// Changes the SPI bus frequency for the DMA-enabled SPI instance. - pub fn change_bus_frequency(&mut self, frequency: HertzU32) { - self.spi_dma.change_bus_frequency(frequency); - } } impl InterruptConfigurable for SpiDmaBus<'_, Blocking, T> @@ -1578,18 +1595,20 @@ mod dma { } } - impl crate::private::Sealed for SpiDmaBus<'_, M, T> - where - T: Instance, - M: Mode, - { - } - impl SpiDmaBus<'_, M, T> where T: Instance, M: Mode, { + fn wait_for_idle(&mut self) { + self.spi_dma.wait_for_idle(); + } + + /// Change the bus configuration. + pub fn apply_config(&mut self, config: &Config) { + self.spi_dma.apply_config(config); + } + /// Reads data from the SPI bus using DMA. pub fn read(&mut self, words: &mut [u8]) -> Result<(), Error> { self.wait_for_idle(); @@ -1683,13 +1702,7 @@ mod dma { Ok(()) } - } - impl SpiDmaBus<'_, M, T> - where - T: Instance, - M: Mode, - { /// Half-duplex read. pub fn half_duplex_read( &mut self, @@ -2560,6 +2573,12 @@ impl Info { }); } + fn apply_config(&self, config: &Config) { + self.ch_bus_freq(config.frequency); + self.set_bit_order(config.read_bit_order, config.write_bit_order); + self.set_data_mode(config.mode); + } + fn set_data_mode(&self, data_mode: SpiMode) { let reg_block = self.register_block(); From da32819df1b901d38b930e64bb36bb3051c9d4f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 4 Nov 2024 13:38:29 +0100 Subject: [PATCH 6/8] SetConfig --- esp-hal/CHANGELOG.md | 1 + esp-hal/MIGRATING-0.21.md | 14 +++++ esp-hal/src/dma/mod.rs | 11 ++-- esp-hal/src/spi/master.rs | 61 ++++++++++++++++--- esp-hal/src/spi/slave.rs | 3 +- examples/src/bin/embassy_spi.rs | 28 ++++++--- examples/src/bin/qspi_flash.rs | 25 +++++--- .../spi_halfduplex_read_manufacturer_id.rs | 23 ++++--- examples/src/bin/spi_loopback.rs | 22 +++++-- examples/src/bin/spi_loopback_dma.rs | 24 +++++--- examples/src/bin/spi_loopback_dma_psram.rs | 24 +++++--- hil-test/tests/embassy_interrupt_spi_dma.rs | 55 +++++++++++------ hil-test/tests/qspi.rs | 59 ++++++++---------- hil-test/tests/spi_full_duplex.rs | 31 +++++++--- hil-test/tests/spi_half_duplex_read.rs | 17 ++++-- hil-test/tests/spi_half_duplex_write.rs | 17 ++++-- hil-test/tests/spi_half_duplex_write_psram.rs | 17 ++++-- 17 files changed, 301 insertions(+), 131 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index 1abb466d3c..1733b3b9b7 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `{Uart, UartRx, UartTx}` now implement `embassy_embedded_hal::SetConfig` (#2449) - GPIO ETM tasks and events now accept `InputSignal` and `OutputSignal` (#2427) - `spi::master::Config` and `{Spi, SpiDma, SpiDmaBus}::apply_config` (#2448) +- `embassy_embedded_hal::SetConfig` is now implemented for `{Spi, SpiDma, SpiDmaBus}` (#2448) ### Changed diff --git a/esp-hal/MIGRATING-0.21.md b/esp-hal/MIGRATING-0.21.md index c8c9b695dd..d490ce1677 100644 --- a/esp-hal/MIGRATING-0.21.md +++ b/esp-hal/MIGRATING-0.21.md @@ -262,3 +262,17 @@ refer to the `Config` struct as `uart::Config`. - bit order: MSB first - mode: SPI mode 0 - There are new constructors (`new_with_config`, `new_typed_with_config`) and a new `apply_config` method to apply custom configuration. + +```diff +-use esp_hal::spi::{master::Spi, SpiMode}; ++use esp_hal::spi::{master::{Config, Spi}, SpiMode}; +-Spi::new(SPI2, 100.kHz(), SpiMode::Mode1); ++Spi::new_with_config( ++ SPI2, ++ Config { ++ frequency: 100.kHz(), ++ mode: SpiMode::Mode0, ++ ..Config::default() ++ }, ++) +``` \ No newline at end of file diff --git a/esp-hal/src/dma/mod.rs b/esp-hal/src/dma/mod.rs index df652f9ff2..f91b18d1da 100644 --- a/esp-hal/src/dma/mod.rs +++ b/esp-hal/src/dma/mod.rs @@ -19,7 +19,7 @@ #![doc = crate::before_snippet!()] //! # use esp_hal::dma_buffers; //! # use esp_hal::gpio::Io; -//! # use esp_hal::spi::{master::Spi, SpiMode}; +//! # use esp_hal::spi::{master::{Config, Spi}, SpiMode}; //! # use esp_hal::dma::{Dma, DmaPriority}; //! let dma = Dma::new(peripherals.DMA); #![cfg_attr(any(esp32, esp32s2), doc = "let dma_channel = dma.spi2channel;")] @@ -30,10 +30,13 @@ //! let mosi = io.pins.gpio4; //! let cs = io.pins.gpio5; //! -//! let mut spi = Spi::new( +//! let mut spi = Spi::new_with_config( //! peripherals.SPI2, -//! 100.kHz(), -//! SpiMode::Mode0, +//! Config { +//! frequency: 100.kHz(), +//! mode: SpiMode::Mode0, +//! ..Config::default() +//! }, //! ) //! .with_sck(sclk) //! .with_mosi(mosi) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index d66fd7f3a0..39fba48009 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -24,8 +24,9 @@ //! ### Shared SPI access //! //! If you have multiple devices on the same SPI bus that each have their own CS -//! line, you may want to have a look at the implementations provided by -//! [`embedded-hal-bus`] and [`embassy-embedded-hal`]. +//! line (and optionally, configuration), you may want to have a look at the +//! implementations provided by [`embedded-hal-bus`] and +//! [`embassy-embedded-hal`]. //! //! ## Usage //! @@ -38,7 +39,7 @@ //! ```rust, no_run #![doc = crate::before_snippet!()] //! # use esp_hal::spi::SpiMode; -//! # use esp_hal::spi::master::Spi; +//! # use esp_hal::spi::master::{Config, Spi}; //! # use esp_hal::gpio::Io; //! # let io = Io::new(peripherals.GPIO, peripherals.IO_MUX); //! let sclk = io.pins.gpio0; @@ -46,10 +47,13 @@ //! let mosi = io.pins.gpio1; //! let cs = io.pins.gpio5; //! -//! let mut spi = Spi::new( +//! let mut spi = Spi::new_with_config( //! peripherals.SPI2, -//! 100.kHz(), -//! SpiMode::Mode0, +//! Config { +//! frequency: 100.kHz(), +//! mode: SpiMode::Mode0, +//! ..Config::default() +//! }, //! ) //! .with_sck(sclk) //! .with_mosi(mosi) @@ -61,9 +65,10 @@ //! [`embedded-hal-bus`]: https://docs.rs/embedded-hal-bus/latest/embedded_hal_bus/spi/index.html //! [`embassy-embedded-hal`]: https://docs.embassy.dev/embassy-embedded-hal/git/default/shared_bus/index.html -use core::marker::PhantomData; +use core::{convert::Infallible, marker::PhantomData}; pub use dma::*; +use embassy_embedded_hal::SetConfig; #[cfg(gdma)] use enumset::EnumSet; #[cfg(gdma)] @@ -677,6 +682,20 @@ where } } +impl SetConfig for Spi<'_, M, T> +where + T: Instance, + M: Mode, +{ + type Config = Config; + type ConfigError = Infallible; + + fn set_config(&mut self, config: &Self::Config) -> Result<(), Self::ConfigError> { + self.apply_config(config); + Ok(()) + } +} + impl<'d, M, T> Spi<'d, M, T> where T: QspiInstance, @@ -1204,6 +1223,20 @@ mod dma { } } + impl SetConfig for SpiDma<'_, M, T> + where + T: Instance, + M: Mode, + { + type Config = Config; + type ConfigError = Infallible; + + fn set_config(&mut self, config: &Self::Config) -> Result<(), Self::ConfigError> { + self.apply_config(config); + Ok(()) + } + } + /// A structure representing a DMA transfer for SPI. /// /// This structure holds references to the SPI instance, DMA buffers, and @@ -1767,6 +1800,20 @@ mod dma { } } + impl SetConfig for SpiDmaBus<'_, M, T> + where + T: Instance, + M: Mode, + { + type Config = Config; + type ConfigError = Infallible; + + fn set_config(&mut self, config: &Self::Config) -> Result<(), Self::ConfigError> { + self.apply_config(config); + Ok(()) + } + } + impl embedded_hal_02::blocking::spi::Transfer for SpiDmaBus<'_, Blocking, T> where T: Instance, diff --git a/esp-hal/src/spi/slave.rs b/esp-hal/src/spi/slave.rs index 8bea967bda..a4e190ecd5 100644 --- a/esp-hal/src/spi/slave.rs +++ b/esp-hal/src/spi/slave.rs @@ -31,7 +31,8 @@ //! let cs = io.pins.gpio3; //! //! let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = -//! dma_buffers!(32000); let mut spi = Spi::new( +//! dma_buffers!(32000); +//! let mut spi = Spi::new( //! peripherals.SPI2, //! sclk, //! mosi, diff --git a/examples/src/bin/embassy_spi.rs b/examples/src/bin/embassy_spi.rs index 2ae0afe977..702b30046d 100644 --- a/examples/src/bin/embassy_spi.rs +++ b/examples/src/bin/embassy_spi.rs @@ -26,7 +26,10 @@ use esp_hal::{ dma_buffers, gpio::Io, prelude::*, - spi::{master::Spi, SpiMode}, + spi::{ + master::{Config, Spi}, + SpiMode, + }, timer::timg::TimerGroup, }; @@ -58,14 +61,21 @@ async fn main(_spawner: Spawner) { let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); let dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap(); - let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_mosi(mosi) - .with_miso(miso) - .with_cs(cs) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)) - .with_buffers(dma_rx_buf, dma_tx_buf) - .into_async(); + let mut spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_mosi(mosi) + .with_miso(miso) + .with_cs(cs) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)) + .with_buffers(dma_rx_buf, dma_tx_buf) + .into_async(); let send_buffer = [0, 1, 2, 3, 4, 5, 6, 7]; loop { diff --git a/examples/src/bin/qspi_flash.rs b/examples/src/bin/qspi_flash.rs index 687b28ba18..cd901df7c3 100644 --- a/examples/src/bin/qspi_flash.rs +++ b/examples/src/bin/qspi_flash.rs @@ -35,7 +35,7 @@ use esp_hal::{ gpio::Io, prelude::*, spi::{ - master::{Address, Command, Spi}, + master::{Address, Command, Config, Spi}, SpiDataMode, SpiMode, }, @@ -79,14 +79,21 @@ fn main() -> ! { let mut dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); let mut dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap(); - let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_mosi(mosi) - .with_miso(miso) - .with_sio2(sio2) - .with_sio3(sio3) - .with_cs(cs) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); + let mut spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_mosi(mosi) + .with_miso(miso) + .with_sio2(sio2) + .with_sio3(sio3) + .with_cs(cs) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); let delay = Delay::new(); diff --git a/examples/src/bin/spi_halfduplex_read_manufacturer_id.rs b/examples/src/bin/spi_halfduplex_read_manufacturer_id.rs index c6dba002af..28266b67e9 100644 --- a/examples/src/bin/spi_halfduplex_read_manufacturer_id.rs +++ b/examples/src/bin/spi_halfduplex_read_manufacturer_id.rs @@ -33,7 +33,7 @@ use esp_hal::{ gpio::Io, prelude::*, spi::{ - master::{Address, Command, Spi}, + master::{Address, Command, Config, Spi}, SpiDataMode, SpiMode, }, @@ -63,13 +63,20 @@ fn main() -> ! { } } - let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_mosi(mosi) - .with_miso(miso) - .with_sio2(sio2) - .with_sio3(sio3) - .with_cs(cs); + let mut spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_mosi(mosi) + .with_miso(miso) + .with_sio2(sio2) + .with_sio3(sio3) + .with_cs(cs); let delay = Delay::new(); diff --git a/examples/src/bin/spi_loopback.rs b/examples/src/bin/spi_loopback.rs index 0900d031d9..38e9d6f1ed 100644 --- a/examples/src/bin/spi_loopback.rs +++ b/examples/src/bin/spi_loopback.rs @@ -21,7 +21,10 @@ use esp_hal::{ gpio::Io, peripheral::Peripheral, prelude::*, - spi::{master::Spi, SpiMode}, + spi::{ + master::{Config, Spi}, + SpiMode, + }, }; use esp_println::println; @@ -36,11 +39,18 @@ fn main() -> ! { let miso = unsafe { miso_mosi.clone_unchecked() }; - let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_mosi(miso_mosi) - .with_miso(miso) - .with_cs(cs); + let mut spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_mosi(miso_mosi) + .with_miso(miso) + .with_cs(cs); let delay = Delay::new(); diff --git a/examples/src/bin/spi_loopback_dma.rs b/examples/src/bin/spi_loopback_dma.rs index f081478da3..e9bc827ff7 100644 --- a/examples/src/bin/spi_loopback_dma.rs +++ b/examples/src/bin/spi_loopback_dma.rs @@ -25,7 +25,10 @@ use esp_hal::{ dma_buffers, gpio::Io, prelude::*, - spi::{master::Spi, SpiMode}, + spi::{ + master::{Config, Spi}, + SpiMode, + }, }; use esp_println::println; @@ -53,12 +56,19 @@ fn main() -> ! { let mut dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); let mut dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap(); - let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_mosi(mosi) - .with_miso(miso) - .with_cs(cs) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); + let mut spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_mosi(mosi) + .with_miso(miso) + .with_cs(cs) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); let delay = Delay::new(); diff --git a/examples/src/bin/spi_loopback_dma_psram.rs b/examples/src/bin/spi_loopback_dma_psram.rs index 1ae3040851..0ee76893cd 100644 --- a/examples/src/bin/spi_loopback_dma_psram.rs +++ b/examples/src/bin/spi_loopback_dma_psram.rs @@ -29,7 +29,10 @@ use esp_hal::{ gpio::Io, peripheral::Peripheral, prelude::*, - spi::{master::Spi, SpiMode}, + spi::{ + master::{Config, Spi}, + SpiMode, + }, }; extern crate alloc; use log::*; @@ -90,12 +93,19 @@ fn main() -> ! { let mut dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); // Need to set miso first so that mosi can overwrite the // output connection (because we are using the same pin to loop back) - let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_miso(miso) - .with_mosi(mosi) - .with_cs(cs) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); + let mut spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_miso(miso) + .with_mosi(mosi) + .with_cs(cs) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); delay.delay_millis(100); // delay to let the above messages display diff --git a/hil-test/tests/embassy_interrupt_spi_dma.rs b/hil-test/tests/embassy_interrupt_spi_dma.rs index fc6580a12f..85dc6edacf 100644 --- a/hil-test/tests/embassy_interrupt_spi_dma.rs +++ b/hil-test/tests/embassy_interrupt_spi_dma.rs @@ -14,7 +14,7 @@ use esp_hal::{ interrupt::{software::SoftwareInterruptControl, Priority}, prelude::*, spi::{ - master::{Spi, SpiDma}, + master::{Config, Spi, SpiDma}, SpiMode, }, timer::AnyTimer, @@ -94,14 +94,28 @@ mod test { let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); let dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap(); - let mut spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_dma(dma_channel1.configure(false, DmaPriority::Priority0)) - .with_buffers(dma_rx_buf, dma_tx_buf) - .into_async(); - - let spi2 = Spi::new(peripherals.SPI3, 100.kHz(), SpiMode::Mode0) - .with_dma(dma_channel2.configure(false, DmaPriority::Priority0)) - .into_async(); + let mut spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_dma(dma_channel1.configure(false, DmaPriority::Priority0)) + .with_buffers(dma_rx_buf, dma_tx_buf) + .into_async(); + + let spi2 = Spi::new_with_config( + peripherals.SPI3, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_dma(dma_channel2.configure(false, DmaPriority::Priority0)) + .into_async(); let sw_ints = SoftwareInterruptControl::new(peripherals.SW_INTERRUPT); @@ -156,14 +170,21 @@ mod test { let dma_rx_buf = DmaRxBuf::new(rx_descriptors, rx_buffer).unwrap(); let dma_tx_buf = DmaTxBuf::new(tx_descriptors, tx_buffer).unwrap(); - let mut spi = Spi::new(peripherals.spi, 100.kHz(), SpiMode::Mode0) - .with_dma( - peripherals - .dma_channel - .configure(false, DmaPriority::Priority0), - ) - .with_buffers(dma_rx_buf, dma_tx_buf) - .into_async(); + let mut spi = Spi::new_with_config( + peripherals.spi, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_dma( + peripherals + .dma_channel + .configure(false, DmaPriority::Priority0), + ) + .with_buffers(dma_rx_buf, dma_tx_buf) + .into_async(); let send_buffer = mk_static!([u8; BUFFER_SIZE], [0u8; BUFFER_SIZE]); loop { diff --git a/hil-test/tests/qspi.rs b/hil-test/tests/qspi.rs index 9322a7b052..8b61a26dd5 100644 --- a/hil-test/tests/qspi.rs +++ b/hil-test/tests/qspi.rs @@ -13,7 +13,7 @@ use esp_hal::{ gpio::{AnyPin, Input, Io, Level, Output, Pull}, prelude::*, spi::{ - master::{Address, Command, Spi, SpiDma}, + master::{Address, Command, Config, Spi, SpiDma}, SpiDataMode, SpiMode, }, @@ -40,7 +40,7 @@ cfg_if::cfg_if! { type SpiUnderTest = SpiDma<'static, Blocking>; struct Context { - spi: esp_hal::peripherals::SPI2, + spi: Spi<'static, Blocking>, #[cfg(pcnt)] pcnt: esp_hal::peripherals::PCNT, dma_channel: Channel<'static, DmaChannel0, Blocking>, @@ -205,9 +205,17 @@ mod tests { } let dma_channel = dma_channel.configure(false, DmaPriority::Priority0); + let spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ); Context { - spi: peripherals.SPI2, + spi, #[cfg(pcnt)] pcnt: peripherals.PCNT, dma_channel, @@ -225,9 +233,7 @@ mod tests { let [pin, pin_mirror, _] = ctx.gpios; let pin_mirror = Output::new(pin_mirror, Level::High); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_mosi(pin) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_mosi(pin).with_dma(ctx.dma_channel); super::execute_read(spi, pin_mirror, 0b0001_0001); } @@ -238,9 +244,7 @@ mod tests { let [pin, pin_mirror, _] = ctx.gpios; let pin_mirror = Output::new(pin_mirror, Level::High); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_miso(pin) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_miso(pin).with_dma(ctx.dma_channel); super::execute_read(spi, pin_mirror, 0b0010_0010); } @@ -251,9 +255,7 @@ mod tests { let [pin, pin_mirror, _] = ctx.gpios; let pin_mirror = Output::new(pin_mirror, Level::High); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_sio2(pin) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_sio2(pin).with_dma(ctx.dma_channel); super::execute_read(spi, pin_mirror, 0b0100_0100); } @@ -264,9 +266,7 @@ mod tests { let [pin, pin_mirror, _] = ctx.gpios; let pin_mirror = Output::new(pin_mirror, Level::High); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_sio3(pin) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_sio3(pin).with_dma(ctx.dma_channel); super::execute_read(spi, pin_mirror, 0b1000_1000); } @@ -277,9 +277,7 @@ mod tests { let [pin, pin_mirror, _] = ctx.gpios; let pin_mirror = Output::new(pin_mirror, Level::High); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_mosi(pin) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_mosi(pin).with_dma(ctx.dma_channel); super::execute_write_read(spi, pin_mirror, 0b0001_0001); } @@ -290,9 +288,7 @@ mod tests { let [pin, pin_mirror, _] = ctx.gpios; let pin_mirror = Output::new(pin_mirror, Level::High); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_miso(pin) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_miso(pin).with_dma(ctx.dma_channel); super::execute_write_read(spi, pin_mirror, 0b0010_0010); } @@ -303,9 +299,7 @@ mod tests { let [pin, pin_mirror, _] = ctx.gpios; let pin_mirror = Output::new(pin_mirror, Level::High); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_sio2(pin) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_sio2(pin).with_dma(ctx.dma_channel); super::execute_write_read(spi, pin_mirror, 0b0100_0100); } @@ -316,9 +310,7 @@ mod tests { let [pin, pin_mirror, _] = ctx.gpios; let pin_mirror = Output::new(pin_mirror, Level::High); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_sio3(pin) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_sio3(pin).with_dma(ctx.dma_channel); super::execute_write_read(spi, pin_mirror, 0b1000_1000); } @@ -342,9 +334,7 @@ mod tests { .channel0 .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) - .with_mosi(mosi) - .with_dma(ctx.dma_channel); + let spi = ctx.spi.with_mosi(mosi).with_dma(ctx.dma_channel); super::execute_write(unit0, unit1, spi, 0b0000_0001, false); } @@ -374,7 +364,8 @@ mod tests { .channel0 .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) + let spi = ctx + .spi .with_mosi(mosi) .with_miso(gpio) .with_dma(ctx.dma_channel); @@ -407,7 +398,8 @@ mod tests { .channel0 .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) + let spi = ctx + .spi .with_mosi(mosi) .with_sio2(gpio) .with_dma(ctx.dma_channel); @@ -440,7 +432,8 @@ mod tests { .channel0 .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); - let spi = Spi::new(ctx.spi, 100.kHz(), SpiMode::Mode0) + let spi = ctx + .spi .with_mosi(mosi) .with_sio3(gpio) .with_dma(ctx.dma_channel); diff --git a/hil-test/tests/spi_full_duplex.rs b/hil-test/tests/spi_full_duplex.rs index 2d465a25b8..a4211f7199 100644 --- a/hil-test/tests/spi_full_duplex.rs +++ b/hil-test/tests/spi_full_duplex.rs @@ -17,7 +17,7 @@ use esp_hal::{ gpio::{Io, Level, NoPin}, peripheral::Peripheral, prelude::*, - spi::{master::Spi, SpiMode}, + spi::master::{Config, Spi}, Blocking, }; #[cfg(pcnt)] @@ -76,10 +76,16 @@ mod tests { let (mosi_loopback_pcnt, mosi) = mosi.split(); // Need to set miso first so that mosi can overwrite the // output connection (because we are using the same pin to loop back) - let spi = Spi::new(peripherals.SPI2, 10000.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_miso(unsafe { mosi.clone_unchecked() }) - .with_mosi(mosi); + let spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 10.MHz(), + ..Config::default() + }, + ) + .with_sck(sclk) + .with_miso(unsafe { mosi.clone_unchecked() }) + .with_mosi(mosi); let (rx_buffer, rx_descriptors, tx_buffer, tx_descriptors) = dma_buffers!(32000); @@ -491,7 +497,10 @@ mod tests { // Slow down. At 80kHz, the transfer is supposed to take a bit over 3 seconds. // This means that without working cancellation, the test case should // fail. - ctx.spi.change_bus_frequency(80.kHz()); + ctx.spi.apply_config(&Config { + frequency: 80.kHz(), + ..Config::default() + }); // Set up a large buffer that would trigger a timeout let dma_rx_buf = DmaRxBuf::new(ctx.rx_descriptors, ctx.rx_buffer).unwrap(); @@ -514,7 +523,10 @@ mod tests { #[timeout(3)] fn can_transmit_after_cancel(mut ctx: Context) { // Slow down. At 80kHz, the transfer is supposed to take a bit over 3 seconds. - ctx.spi.change_bus_frequency(80.kHz()); + ctx.spi.apply_config(&Config { + frequency: 80.kHz(), + ..Config::default() + }); // Set up a large buffer that would trigger a timeout let mut dma_rx_buf = DmaRxBuf::new(ctx.rx_descriptors, ctx.rx_buffer).unwrap(); @@ -532,7 +544,10 @@ mod tests { transfer.cancel(); (spi, (dma_rx_buf, dma_tx_buf)) = transfer.wait(); - spi.change_bus_frequency(10000.kHz()); + spi.apply_config(&Config { + frequency: 10.MHz(), + ..Config::default() + }); let transfer = spi .transfer(dma_rx_buf, dma_tx_buf) diff --git a/hil-test/tests/spi_half_duplex_read.rs b/hil-test/tests/spi_half_duplex_read.rs index a50fa0dc80..dc83a638ce 100644 --- a/hil-test/tests/spi_half_duplex_read.rs +++ b/hil-test/tests/spi_half_duplex_read.rs @@ -11,7 +11,7 @@ use esp_hal::{ gpio::{Io, Level, Output}, prelude::*, spi::{ - master::{Address, Command, Spi, SpiDma}, + master::{Address, Command, Config, Spi, SpiDma}, SpiDataMode, SpiMode, }, @@ -49,10 +49,17 @@ mod tests { } } - let spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_miso(miso) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); + let spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_miso(miso) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); Context { spi, miso_mirror } } diff --git a/hil-test/tests/spi_half_duplex_write.rs b/hil-test/tests/spi_half_duplex_write.rs index f06a80f3ea..88a51641e4 100644 --- a/hil-test/tests/spi_half_duplex_write.rs +++ b/hil-test/tests/spi_half_duplex_write.rs @@ -12,7 +12,7 @@ use esp_hal::{ pcnt::{channel::EdgeMode, unit::Unit, Pcnt}, prelude::*, spi::{ - master::{Address, Command, Spi, SpiDma}, + master::{Address, Command, Config, Spi, SpiDma}, SpiDataMode, SpiMode, }, @@ -52,10 +52,17 @@ mod tests { let (mosi_loopback, mosi) = mosi.split(); - let spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_mosi(mosi) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); + let spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_mosi(mosi) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); Context { spi, diff --git a/hil-test/tests/spi_half_duplex_write_psram.rs b/hil-test/tests/spi_half_duplex_write_psram.rs index f40f81dc2d..aaf9fb7b97 100644 --- a/hil-test/tests/spi_half_duplex_write_psram.rs +++ b/hil-test/tests/spi_half_duplex_write_psram.rs @@ -14,7 +14,7 @@ use esp_hal::{ pcnt::{channel::EdgeMode, unit::Unit, Pcnt}, prelude::*, spi::{ - master::{Address, Command, Spi, SpiDma}, + master::{Address, Command, Config, Spi, SpiDma}, SpiDataMode, SpiMode, }, @@ -64,10 +64,17 @@ mod tests { let (mosi_loopback, mosi) = mosi.split(); - let spi = Spi::new(peripherals.SPI2, 100.kHz(), SpiMode::Mode0) - .with_sck(sclk) - .with_mosi(mosi) - .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); + let spi = Spi::new_with_config( + peripherals.SPI2, + Config { + frequency: 100.kHz(), + mode: SpiMode::Mode0, + ..Config::default() + }, + ) + .with_sck(sclk) + .with_mosi(mosi) + .with_dma(dma_channel.configure(false, DmaPriority::Priority0)); Context { spi, From b9762e85b1dbc9de445d4164ed6eb3331a247b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 5 Nov 2024 17:41:40 +0100 Subject: [PATCH 7/8] Return ConfigError --- esp-hal/src/spi/master.rs | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index 39fba48009..ff045d7d70 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -65,7 +65,7 @@ //! [`embedded-hal-bus`]: https://docs.rs/embedded-hal-bus/latest/embedded_hal_bus/spi/index.html //! [`embassy-embedded-hal`]: https://docs.embassy.dev/embassy-embedded-hal/git/default/shared_bus/index.html -use core::{convert::Infallible, marker::PhantomData}; +use core::marker::PhantomData; pub use dma::*; use embassy_embedded_hal::SetConfig; @@ -469,6 +469,11 @@ impl Default for Config { } } +/// Configuration errors. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum ConfigError {} + /// SPI peripheral driver pub struct Spi<'d, M, T = AnySpi> { spi: PeripheralRef<'d, T>, @@ -677,8 +682,8 @@ where } /// Change the bus configuration. - pub fn apply_config(&mut self, config: &Config) { - self.driver().apply_config(config); + pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { + self.driver().apply_config(config) } } @@ -688,11 +693,10 @@ where M: Mode, { type Config = Config; - type ConfigError = Infallible; + type ConfigError = ConfigError; fn set_config(&mut self, config: &Self::Config) -> Result<(), Self::ConfigError> { - self.apply_config(config); - Ok(()) + self.apply_config(config) } } @@ -1205,8 +1209,8 @@ mod dma { } /// Change the bus configuration. - pub fn apply_config(&mut self, config: &Config) { - self.driver().apply_config(config); + pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { + self.driver().apply_config(config) } /// Configures the DMA buffers for the SPI instance. @@ -1229,11 +1233,10 @@ mod dma { M: Mode, { type Config = Config; - type ConfigError = Infallible; + type ConfigError = ConfigError; fn set_config(&mut self, config: &Self::Config) -> Result<(), Self::ConfigError> { - self.apply_config(config); - Ok(()) + self.apply_config(config) } } @@ -1638,8 +1641,8 @@ mod dma { } /// Change the bus configuration. - pub fn apply_config(&mut self, config: &Config) { - self.spi_dma.apply_config(config); + pub fn apply_config(&mut self, config: &Config) -> Result<(), ConfigError> { + self.spi_dma.apply_config(config) } /// Reads data from the SPI bus using DMA. @@ -1806,11 +1809,10 @@ mod dma { M: Mode, { type Config = Config; - type ConfigError = Infallible; + type ConfigError = ConfigError; fn set_config(&mut self, config: &Self::Config) -> Result<(), Self::ConfigError> { - self.apply_config(config); - Ok(()) + self.apply_config(config) } } @@ -2620,10 +2622,11 @@ impl Info { }); } - fn apply_config(&self, config: &Config) { + fn apply_config(&self, config: &Config) -> Result<(), ConfigError> { self.ch_bus_freq(config.frequency); self.set_bit_order(config.read_bit_order, config.write_bit_order); self.set_data_mode(config.mode); + Ok(()) } fn set_data_mode(&self, data_mode: SpiMode) { From 46b49677aa887fcd7dd31e778981f5cada2e8e8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Tue, 5 Nov 2024 18:27:53 +0100 Subject: [PATCH 8/8] Unwrap config result in ctor --- esp-hal/src/spi/master.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esp-hal/src/spi/master.rs b/esp-hal/src/spi/master.rs index ff045d7d70..bab494fa82 100644 --- a/esp-hal/src/spi/master.rs +++ b/esp-hal/src/spi/master.rs @@ -598,7 +598,7 @@ where PeripheralClockControl::reset(this.spi.peripheral()); this.driver().init(); - this.apply_config(&config); + unwrap!(this.apply_config(&config)); // FIXME: update based on the resolution of https://github.com/esp-rs/esp-hal/issues/2416 let this = this .with_mosi(NoPin)