From 1afbb8e58a4c19a3c24e7a4b2c62d0f29de78a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Alse=CC=81r?= <> Date: Wed, 24 Jun 2020 19:33:13 +0200 Subject: [PATCH 1/7] Spim transfer: Implicitly copy buffer into RAM if needed when using embedded hal trait --- nrf-hal-common/src/spim.rs | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/nrf-hal-common/src/spim.rs b/nrf-hal-common/src/spim.rs index 83331268..915675e5 100644 --- a/nrf-hal-common/src/spim.rs +++ b/nrf-hal-common/src/spim.rs @@ -41,13 +41,24 @@ where type Error = Error; fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Error> { - // If the slice isn't in RAM, we can't write back to it at all - slice_in_ram_or(words, Error::DMABufferNotInDataMemory)?; - - words.chunks(EASY_DMA_SIZE).try_for_each(|chunk| { - self.do_spi_dma_transfer(DmaSlice::from_slice(chunk), DmaSlice::from_slice(chunk)) - })?; - + if slice_in_ram(words) { + words.chunks(EASY_DMA_SIZE).try_for_each(|chunk| { + self.do_spi_dma_transfer(DmaSlice::from_slice(chunk), DmaSlice::from_slice(chunk)) + })?; + } else { + words + .chunks_mut(FORCE_COPY_BUFFER_SIZE) + .try_for_each(|chunk| { + let mut buf = [0u8; FORCE_COPY_BUFFER_SIZE]; + buf[..chunk.len()].copy_from_slice(chunk); + self.do_spi_dma_transfer( + DmaSlice::from_slice(&buf[..chunk.len()]), + DmaSlice::from_slice(&buf[..chunk.len()]), + )?; + chunk.copy_from_slice(&buf[..chunk.len()]); + Ok(()) + })?; + } Ok(words) } } From aad5fb903f6b3debdeab02551e31f2a356a9db17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Alse=CC=81r?= <> Date: Wed, 24 Jun 2020 21:06:45 +0200 Subject: [PATCH 2/7] Twim write/write_read: Implicitly copy buffer into RAM if needed when using embedded hal traits --- nrf-hal-common/src/twim.rs | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/nrf-hal-common/src/twim.rs b/nrf-hal-common/src/twim.rs index a12d882d..f692f4cc 100644 --- a/nrf-hal-common/src/twim.rs +++ b/nrf-hal-common/src/twim.rs @@ -19,7 +19,8 @@ use crate::target::TWIM1; use crate::{ gpio::{Floating, Input, Pin}, slice_in_ram_or, - target_constants::EASY_DMA_SIZE, + slice_in_ram, + target_constants::{EASY_DMA_SIZE, FORCE_COPY_BUFFER_SIZE}, }; pub use twim0::frequency::FREQUENCY_A as Frequency; @@ -348,7 +349,17 @@ where type Error = Error; fn write<'w>(&mut self, addr: u8, bytes: &'w [u8]) -> Result<(), Error> { - self.write(addr, bytes) + if slice_in_ram(bytes){ + self.write(addr, bytes) + } + else { + let buf = &mut [0; FORCE_COPY_BUFFER_SIZE][..]; + for chunk in bytes.chunks(FORCE_COPY_BUFFER_SIZE) { + buf[..chunk.len()].copy_from_slice(chunk); + self.write(addr, &buf[..chunk.len()])?; + } + Ok(()) + } } } @@ -375,11 +386,22 @@ where bytes: &'w [u8], buffer: &'w mut [u8], ) -> Result<(), Error> { - self.write_then_read(addr, bytes, buffer) + if slice_in_ram(bytes){ + self.write_then_read(addr, bytes, buffer) + } else { + let txi = bytes.chunks(FORCE_COPY_BUFFER_SIZE); + let rxi = buffer.chunks_mut(FORCE_COPY_BUFFER_SIZE); + let tx_buf = &mut [0; FORCE_COPY_BUFFER_SIZE][..]; + txi.zip(rxi).try_for_each(|(tx_chunk, rx_chunk)| { + tx_buf[..tx_chunk.len()].copy_from_slice(tx_chunk); + self.write_then_read(addr, &tx_buf[..tx_chunk.len()], rx_chunk) + })?; + Ok(()) + } } } -/// The pins used by the TWIN peripheral +/// The pins used by the TWIM peripheral /// /// Currently, only P0 pins are supported. pub struct Pins { From 4c935d0b82e2f5ed7582f3c6a1356d299f4087e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Alse=CC=81r?= <> Date: Wed, 24 Jun 2020 22:17:30 +0200 Subject: [PATCH 3/7] Revert copy to RAM in embedded hal spim transfer. The mut slice must already be in RAM, so no check neccessary --- nrf-hal-common/, | 0 nrf-hal-common/src/spim.rs | 25 +++++++------------------ 2 files changed, 7 insertions(+), 18 deletions(-) create mode 100644 nrf-hal-common/, diff --git a/nrf-hal-common/, b/nrf-hal-common/, new file mode 100644 index 00000000..e69de29b diff --git a/nrf-hal-common/src/spim.rs b/nrf-hal-common/src/spim.rs index 915675e5..83331268 100644 --- a/nrf-hal-common/src/spim.rs +++ b/nrf-hal-common/src/spim.rs @@ -41,24 +41,13 @@ where type Error = Error; fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Error> { - if slice_in_ram(words) { - words.chunks(EASY_DMA_SIZE).try_for_each(|chunk| { - self.do_spi_dma_transfer(DmaSlice::from_slice(chunk), DmaSlice::from_slice(chunk)) - })?; - } else { - words - .chunks_mut(FORCE_COPY_BUFFER_SIZE) - .try_for_each(|chunk| { - let mut buf = [0u8; FORCE_COPY_BUFFER_SIZE]; - buf[..chunk.len()].copy_from_slice(chunk); - self.do_spi_dma_transfer( - DmaSlice::from_slice(&buf[..chunk.len()]), - DmaSlice::from_slice(&buf[..chunk.len()]), - )?; - chunk.copy_from_slice(&buf[..chunk.len()]); - Ok(()) - })?; - } + // If the slice isn't in RAM, we can't write back to it at all + slice_in_ram_or(words, Error::DMABufferNotInDataMemory)?; + + words.chunks(EASY_DMA_SIZE).try_for_each(|chunk| { + self.do_spi_dma_transfer(DmaSlice::from_slice(chunk), DmaSlice::from_slice(chunk)) + })?; + Ok(words) } } From 287ea225ed6c3c1ec500ab0d689590b1e4abe2d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Alse=CC=81r?= <> Date: Thu, 25 Jun 2020 02:41:25 +0200 Subject: [PATCH 4/7] cargo fmt --- nrf-hal-common/src/twim.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/nrf-hal-common/src/twim.rs b/nrf-hal-common/src/twim.rs index f692f4cc..2b9fea16 100644 --- a/nrf-hal-common/src/twim.rs +++ b/nrf-hal-common/src/twim.rs @@ -18,8 +18,7 @@ use crate::target::TWIM1; use crate::{ gpio::{Floating, Input, Pin}, - slice_in_ram_or, - slice_in_ram, + slice_in_ram, slice_in_ram_or, target_constants::{EASY_DMA_SIZE, FORCE_COPY_BUFFER_SIZE}, }; @@ -349,10 +348,9 @@ where type Error = Error; fn write<'w>(&mut self, addr: u8, bytes: &'w [u8]) -> Result<(), Error> { - if slice_in_ram(bytes){ + if slice_in_ram(bytes) { self.write(addr, bytes) - } - else { + } else { let buf = &mut [0; FORCE_COPY_BUFFER_SIZE][..]; for chunk in bytes.chunks(FORCE_COPY_BUFFER_SIZE) { buf[..chunk.len()].copy_from_slice(chunk); @@ -386,7 +384,7 @@ where bytes: &'w [u8], buffer: &'w mut [u8], ) -> Result<(), Error> { - if slice_in_ram(bytes){ + if slice_in_ram(bytes) { self.write_then_read(addr, bytes, buffer) } else { let txi = bytes.chunks(FORCE_COPY_BUFFER_SIZE); From 4f64fc22d5ee9d2d11e16f882cfffd1ea71ad942 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Alse=CC=81r?= <> Date: Thu, 25 Jun 2020 08:59:25 +0200 Subject: [PATCH 5/7] Twim write_read for embedded hal: copy data to RAM if needed, write all chunks then read --- nrf-hal-common/src/twim.rs | 130 ++++++++++++++++++++++++++++++++++--- 1 file changed, 120 insertions(+), 10 deletions(-) diff --git a/nrf-hal-common/src/twim.rs b/nrf-hal-common/src/twim.rs index 2b9fea16..1336a97e 100644 --- a/nrf-hal-common/src/twim.rs +++ b/nrf-hal-common/src/twim.rs @@ -133,7 +133,7 @@ where while self.0.events_lasttx.read().bits() == 0 {} self.0.events_lasttx.write(|w| w); // reset event - // Stop read operation + // Stop write operation self.0.tasks_stop.write(|w| // `1` is a valid value to write to task registers. unsafe { w.bits(1) }); @@ -229,7 +229,8 @@ where /// Write data to an I2C slave, then read data from the slave without /// triggering a stop condition between the two /// - /// The buffer must have a length of at most 255 bytes. + /// The buffers must have a length of at most 255 bytes on the nRF52832 + /// and at most 65535 bytes on the nRF52840. pub fn write_then_read( &mut self, address: u8, @@ -333,6 +334,122 @@ where Ok(()) } + /// Copy data into RAM and write to an I2C slave, then read data from the slave without + /// triggering a stop condition between the two + /// + /// The read buffer must have a length of at most 255 bytes on the nRF52832 + /// and at most 65535 bytes on the nRF52840. + pub fn copy_write_then_read( + &mut self, + address: u8, + tx_buffer: &[u8], + rx_buffer: &mut [u8], + ) -> Result<(), Error> { + if rx_buffer.len() > EASY_DMA_SIZE { + return Err(Error::RxBufferTooLong); + } + + // Conservative compiler fence to prevent optimizations that do not + // take in to account actions by DMA. The fence has been placed here, + // before any DMA action has started + compiler_fence(SeqCst); + + self.0 + .address + .write(|w| unsafe { w.address().bits(address) }); + + // Set up the DMA read + self.0.rxd.ptr.write(|w| + // We're giving the register a pointer to the stack. Since we're + // waiting for the I2C transaction to end before this stack pointer + // becomes invalid, there's nothing wrong here. + // + // The PTR field is a full 32 bits wide and accepts the full range + // of values. + unsafe { w.ptr().bits(rx_buffer.as_mut_ptr() as u32) }); + self.0.rxd.maxcnt.write(|w| + // We're giving it the length of the buffer, so no danger of + // accessing invalid memory. We have verified that the length of the + // buffer fits in an `u8`, so the cast to the type of maxcnt + // is also fine. + // + // Note that that nrf52840 maxcnt is a wider + // type than a u8, so we use a `_` cast rather than a `u8` cast. + // The MAXCNT field is thus at least 8 bits wide and accepts the + // full range of values that fit in a `u8`. + unsafe { w.maxcnt().bits(rx_buffer.len() as _) }); + + // Chunk write data + for chunk in tx_buffer.chunks(FORCE_COPY_BUFFER_SIZE) { + // Copy chunk into RAM + let wr_buffer = &mut [0; FORCE_COPY_BUFFER_SIZE][..]; + wr_buffer[..chunk.len()].copy_from_slice(chunk); + + // Set up the DMA write + self.0.txd.ptr.write(|w| + // We're giving the register a pointer to the stack. Since we're + // waiting for the I2C transaction to end before this stack pointer + // becomes invalid, there's nothing wrong here. + // + // The PTR field is a full 32 bits wide and accepts the full range + // of values. + unsafe { w.ptr().bits(wr_buffer.as_ptr() as u32) }); + + self.0.txd.maxcnt.write(|w| + // We're giving it the length of the buffer, so no danger of + // accessing invalid memory. We have verified that the length of the + // buffer fits in an `u8`, so the cast to `u8` is also fine. + // + // The MAXCNT field is 8 bits wide and accepts the full range of + // values. + unsafe { w.maxcnt().bits(wr_buffer.len() as _) }); + + // Start write operation + self.0.tasks_starttx.write(|w| + // `1` is a valid value to write to task registers. + unsafe { w.bits(1) }); + + // Wait until write operation is about to end + while self.0.events_lasttx.read().bits() == 0 {} + self.0.events_lasttx.write(|w| w); // reset event + + // Check for bad writes + if self.0.txd.amount.read().bits() != wr_buffer.len() as u32 { + return Err(Error::Transmit); + } + } + + // Start read operation + self.0.tasks_startrx.write(|w| + // `1` is a valid value to write to task registers. + unsafe { w.bits(1) }); + + // Wait until read operation is about to end + while self.0.events_lastrx.read().bits() == 0 {} + self.0.events_lastrx.write(|w| w); // reset event + + // Stop read operation + self.0.tasks_stop.write(|w| + // `1` is a valid value to write to task registers. + unsafe { w.bits(1) }); + + // Wait until total operation has ended + while self.0.events_stopped.read().bits() == 0 {} + self.0.events_stopped.write(|w| w); // reset event + + // Conservative compiler fence to prevent optimizations that do not + // take in to account actions by DMA. The fence has been placed here, + // after all possible DMA actions have completed + compiler_fence(SeqCst); + + // Check for bad reads + if self.0.rxd.amount.read().bits() != rx_buffer.len() as u32 { + return Err(Error::Receive); + } + + Ok(()) + } + /// Return the raw interface to the underlying TWIM peripheral pub fn free(self) -> T { self.0 @@ -387,14 +504,7 @@ where if slice_in_ram(bytes) { self.write_then_read(addr, bytes, buffer) } else { - let txi = bytes.chunks(FORCE_COPY_BUFFER_SIZE); - let rxi = buffer.chunks_mut(FORCE_COPY_BUFFER_SIZE); - let tx_buf = &mut [0; FORCE_COPY_BUFFER_SIZE][..]; - txi.zip(rxi).try_for_each(|(tx_chunk, rx_chunk)| { - tx_buf[..tx_chunk.len()].copy_from_slice(tx_chunk); - self.write_then_read(addr, &tx_buf[..tx_chunk.len()], rx_chunk) - })?; - Ok(()) + self.copy_write_then_read(addr, bytes, buffer) } } } From 45d9201d6106b90ed3b8c9c32a51fbc5d2789f79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Alse=CC=81r?= <> Date: Thu, 25 Jun 2020 10:36:43 +0200 Subject: [PATCH 6/7] Chunk size is min value of EASY_DMA_SIZE and FORCE_COPY_BUFFER_SIZE --- nrf-hal-common/src/twim.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nrf-hal-common/src/twim.rs b/nrf-hal-common/src/twim.rs index 1336a97e..7f2ba5ad 100644 --- a/nrf-hal-common/src/twim.rs +++ b/nrf-hal-common/src/twim.rs @@ -380,9 +380,9 @@ where unsafe { w.maxcnt().bits(rx_buffer.len() as _) }); // Chunk write data - for chunk in tx_buffer.chunks(FORCE_COPY_BUFFER_SIZE) { + let wr_buffer = &mut [0; FORCE_COPY_BUFFER_SIZE][..]; + for chunk in tx_buffer.chunks(FORCE_COPY_BUFFER_SIZE.min(EASY_DMA_SIZE)) { // Copy chunk into RAM - let wr_buffer = &mut [0; FORCE_COPY_BUFFER_SIZE][..]; wr_buffer[..chunk.len()].copy_from_slice(chunk); // Set up the DMA write From edc316d545895f1e17b88856b0e1a2211ca313c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Alse=CC=81r?= <> Date: Tue, 30 Jun 2020 23:19:05 +0200 Subject: [PATCH 7/7] Compile time assert FORCE_COPY_BUFFER_SIZE <= EASY_DMA_SIZE --- nrf-hal-common/src/lib.rs | 4 ++++ nrf-hal-common/src/twim.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/nrf-hal-common/src/lib.rs b/nrf-hal-common/src/lib.rs index 0ad651b1..eda3fbbd 100644 --- a/nrf-hal-common/src/lib.rs +++ b/nrf-hal-common/src/lib.rs @@ -70,6 +70,8 @@ pub mod target_constants { pub const SRAM_LOWER: usize = 0x2000_0000; pub const SRAM_UPPER: usize = 0x3000_0000; pub const FORCE_COPY_BUFFER_SIZE: usize = 255; + const _CHECK_FORCE_COPY_BUFFER_SIZE: usize = EASY_DMA_SIZE - FORCE_COPY_BUFFER_SIZE; + // ERROR: FORCE_COPY_BUFFER_SIZE must be <= EASY_DMA_SIZE } #[cfg(any(feature = "52840", feature = "52833", feature = "9160"))] pub mod target_constants { @@ -79,6 +81,8 @@ pub mod target_constants { pub const SRAM_LOWER: usize = 0x2000_0000; pub const SRAM_UPPER: usize = 0x3000_0000; pub const FORCE_COPY_BUFFER_SIZE: usize = 1024; + const _CHECK_FORCE_COPY_BUFFER_SIZE: usize = EASY_DMA_SIZE - FORCE_COPY_BUFFER_SIZE; + // ERROR: FORCE_COPY_BUFFER_SIZE must be <= EASY_DMA_SIZE } /// Does this slice reside entirely within RAM? diff --git a/nrf-hal-common/src/twim.rs b/nrf-hal-common/src/twim.rs index 7f2ba5ad..7497b351 100644 --- a/nrf-hal-common/src/twim.rs +++ b/nrf-hal-common/src/twim.rs @@ -381,7 +381,7 @@ where // Chunk write data let wr_buffer = &mut [0; FORCE_COPY_BUFFER_SIZE][..]; - for chunk in tx_buffer.chunks(FORCE_COPY_BUFFER_SIZE.min(EASY_DMA_SIZE)) { + for chunk in tx_buffer.chunks(FORCE_COPY_BUFFER_SIZE) { // Copy chunk into RAM wr_buffer[..chunk.len()].copy_from_slice(chunk);