From ab1a6889a62e86a80af6fc572ffa992cfb9ef960 Mon Sep 17 00:00:00 2001 From: Alex Martens Date: Sun, 18 Sep 2022 12:02:05 -0700 Subject: [PATCH 1/2] rp: fix async SPI read and write --- embassy-rp/src/dma.rs | 38 ++++++++++++++++++++++++++++ embassy-rp/src/spi.rs | 59 ++++++++++++++++++++++++++++++------------- 2 files changed, 79 insertions(+), 18 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index acf33822..b256cc2f 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -56,6 +56,25 @@ pub unsafe fn read<'a, C: Channel, W: Word>( ) } +pub unsafe fn read_repeated<'a, C: Channel, W: Word>( + ch: impl Peripheral

+ 'a, + from: *const W, + len: usize, + dreq: u8, +) -> Transfer<'a, C> { + let mut dummy: u32 = 0; + copy_inner( + ch, + from as *const u32, + &mut dummy as *mut u32, + len, + W::size(), + false, + false, + dreq, + ) +} + pub unsafe fn write<'a, C: Channel, W: Word>( ch: impl Peripheral

+ 'a, from: *const [W], @@ -75,6 +94,25 @@ pub unsafe fn write<'a, C: Channel, W: Word>( ) } +pub unsafe fn write_repeated<'a, C: Channel, W: Word>( + ch: impl Peripheral

+ 'a, + to: *mut W, + len: usize, + dreq: u8, +) -> Transfer<'a, C> { + let dummy: u32 = 0; + copy_inner( + ch, + &dummy as *const u32, + to as *mut u32, + len, + W::size(), + false, + false, + dreq, + ) +} + pub unsafe fn copy<'a, C: Channel, W: Word>( ch: impl Peripheral

+ 'a, from: &[W], diff --git a/embassy-rp/src/spi.rs b/embassy-rp/src/spi.rs index 74f0b04d..e7cd9992 100644 --- a/embassy-rp/src/spi.rs +++ b/embassy-rp/src/spi.rs @@ -325,30 +325,53 @@ impl<'d, T: Instance> Spi<'d, T, Async> { } pub async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { - let ch = self.tx_dma.as_mut().unwrap(); - let transfer = unsafe { - self.inner.regs().dmacr().modify(|reg| { + unsafe { + self.inner.regs().dmacr().write(|reg| { + reg.set_rxdmae(true); reg.set_txdmae(true); - }); + }) + }; + let tx_ch = self.tx_dma.as_mut().unwrap(); + let tx_transfer = unsafe { // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. - crate::dma::write(ch, buffer, self.inner.regs().dr().ptr() as *mut _, T::TX_DREQ) + crate::dma::write(tx_ch, buffer, self.inner.regs().dr().ptr() as *mut _, T::TX_DREQ) }; - transfer.await; + let rx_ch = self.rx_dma.as_mut().unwrap(); + let rx_transfer = unsafe { + // If we don't assign future to a variable, the data register pointer + // is held across an await and makes the future non-Send. + crate::dma::read_repeated( + rx_ch, + self.inner.regs().dr().ptr() as *const u8, + buffer.len(), + T::RX_DREQ, + ) + }; + join(tx_transfer, rx_transfer).await; Ok(()) } pub async fn read(&mut self, buffer: &mut [u8]) -> Result<(), Error> { - let ch = self.rx_dma.as_mut().unwrap(); - let transfer = unsafe { - self.inner.regs().dmacr().modify(|reg| { + unsafe { + self.inner.regs().dmacr().write(|reg| { reg.set_rxdmae(true); - }); + reg.set_txdmae(true); + }) + }; + let tx_ch = self.tx_dma.as_mut().unwrap(); + let tx_transfer = unsafe { // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. - crate::dma::read(ch, self.inner.regs().dr().ptr() as *const _, buffer, T::RX_DREQ) + crate::dma::write_repeated(tx_ch, self.inner.regs().dr().ptr() as *mut u8, buffer.len(), T::TX_DREQ) }; - transfer.await; + let rx_ch = self.rx_dma.as_mut().unwrap(); + let rx_transfer = unsafe { + // If we don't assign future to a variable, the data register pointer + // is held across an await and makes the future non-Send. + crate::dma::read(rx_ch, self.inner.regs().dr().ptr() as *const _, buffer, T::RX_DREQ) + }; + join(tx_transfer, rx_transfer).await; Ok(()) } @@ -364,20 +387,20 @@ impl<'d, T: Instance> Spi<'d, T, Async> { let (_, from_len) = crate::dma::slice_ptr_parts(tx_ptr); let (_, to_len) = crate::dma::slice_ptr_parts_mut(rx_ptr); assert_eq!(from_len, to_len); + unsafe { + self.inner.regs().dmacr().write(|reg| { + reg.set_rxdmae(true); + reg.set_txdmae(true); + }) + }; let tx_ch = self.tx_dma.as_mut().unwrap(); let tx_transfer = unsafe { - self.inner.regs().dmacr().modify(|reg| { - reg.set_txdmae(true); - }); // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. crate::dma::write(tx_ch, tx_ptr, self.inner.regs().dr().ptr() as *mut _, T::TX_DREQ) }; let rx_ch = self.rx_dma.as_mut().unwrap(); let rx_transfer = unsafe { - self.inner.regs().dmacr().modify(|reg| { - reg.set_rxdmae(true); - }); // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. crate::dma::read(rx_ch, self.inner.regs().dr().ptr() as *const _, rx_ptr, T::RX_DREQ) From 295cc997ae8b1468617dd9f430be4e502901f4f2 Mon Sep 17 00:00:00 2001 From: Alex Martens Date: Sun, 18 Sep 2022 12:23:17 -0700 Subject: [PATCH 2/2] rp: let SPI RX overflow during async write --- embassy-rp/src/dma.rs | 19 ------------------- embassy-rp/src/spi.rs | 35 +++++++++++++++++------------------ 2 files changed, 17 insertions(+), 37 deletions(-) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index b256cc2f..7ad1a6bf 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -56,25 +56,6 @@ pub unsafe fn read<'a, C: Channel, W: Word>( ) } -pub unsafe fn read_repeated<'a, C: Channel, W: Word>( - ch: impl Peripheral

+ 'a, - from: *const W, - len: usize, - dreq: u8, -) -> Transfer<'a, C> { - let mut dummy: u32 = 0; - copy_inner( - ch, - from as *const u32, - &mut dummy as *mut u32, - len, - W::size(), - false, - false, - dreq, - ) -} - pub unsafe fn write<'a, C: Channel, W: Word>( ch: impl Peripheral

+ 'a, from: *const [W], diff --git a/embassy-rp/src/spi.rs b/embassy-rp/src/spi.rs index e7cd9992..3cf82357 100644 --- a/embassy-rp/src/spi.rs +++ b/embassy-rp/src/spi.rs @@ -325,30 +325,29 @@ impl<'d, T: Instance> Spi<'d, T, Async> { } pub async fn write(&mut self, buffer: &[u8]) -> Result<(), Error> { - unsafe { - self.inner.regs().dmacr().write(|reg| { - reg.set_rxdmae(true); - reg.set_txdmae(true); - }) - }; let tx_ch = self.tx_dma.as_mut().unwrap(); let tx_transfer = unsafe { + self.inner.regs().dmacr().modify(|reg| { + reg.set_txdmae(true); + }); // If we don't assign future to a variable, the data register pointer // is held across an await and makes the future non-Send. crate::dma::write(tx_ch, buffer, self.inner.regs().dr().ptr() as *mut _, T::TX_DREQ) }; - let rx_ch = self.rx_dma.as_mut().unwrap(); - let rx_transfer = unsafe { - // If we don't assign future to a variable, the data register pointer - // is held across an await and makes the future non-Send. - crate::dma::read_repeated( - rx_ch, - self.inner.regs().dr().ptr() as *const u8, - buffer.len(), - T::RX_DREQ, - ) - }; - join(tx_transfer, rx_transfer).await; + tx_transfer.await; + + let p = self.inner.regs(); + unsafe { + while p.sr().read().bsy() {} + + // clear RX FIFO contents to prevent stale reads + while p.sr().read().rne() { + let _: u16 = p.dr().read().data(); + } + // clear RX overrun interrupt + p.icr().write(|w| w.set_roric(true)); + } + Ok(()) }