Merge pull request #165 from kalkyl/twim_copy

Twim: Implicitly copy buffer into RAM if needed when using embedded hal traits

Author: jonas-schievink

nrf-hal-common/,

@@ -74,6 +74,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 {
@@ -83,6 +85,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?

nrf-hal-common/src/lib.rs

@@ -18,8 +18,8 @@ use crate::target::TWIM1;
 
 use crate::{
     gpio::{Floating, Input, Pin},
-    slice_in_ram_or,
-    target_constants::EASY_DMA_SIZE,
+    slice_in_ram, slice_in_ram_or,
+    target_constants::{EASY_DMA_SIZE, FORCE_COPY_BUFFER_SIZE},
 };
 
 pub use twim0::frequency::FREQUENCY_A as Frequency;
@@ -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
+        let wr_buffer = &mut [0; FORCE_COPY_BUFFER_SIZE][..];
+        for chunk in tx_buffer.chunks(FORCE_COPY_BUFFER_SIZE) {
+            // Copy chunk into RAM
+            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
@@ -348,7 +465,16 @@ 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 +501,15 @@ 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 {
+            self.copy_write_then_read(addr, bytes, buffer)
+        }
     }
 }
 
-/// The pins used by the TWIN peripheral
+/// The pins used by the TWIM peripheral
 ///
 /// Currently, only P0 pins are supported.
 pub struct Pins {