Consistently check slices to prevent odd EasyDMA errors

Author: James Munns

nrf52-hal-common/src/lib.rs

@@ -62,6 +62,16 @@ pub(crate) fn slice_in_ram(slice: &[u8]) -> bool {
     ptr >= target_constants::SRAM_LOWER && (ptr + slice.len()) < target_constants::SRAM_UPPER
 }
 
+/// Return an error if slice is not in RAM
+pub(crate) fn slice_in_ram_or<T>(slice: &[u8], err: T) -> Result<(), T> {
+    if slice_in_ram(slice) {
+        Ok(())
+    } else {
+        Err(err)
+    }
+}
+
+
 /// A handy structure for converting rust slices into ptr and len pairs
 /// for use with EasyDMA. Care must be taken to make sure mutability
 /// guarantees are respected

nrf52-hal-common/src/spim.rs

@@ -20,7 +20,7 @@ use crate::target::{SPIM1, SPIM2};
 
 use crate::gpio::{Floating, Input, Output, Pin, PushPull};
 use crate::target_constants::{EASY_DMA_SIZE, FORCE_COPY_BUFFER_SIZE};
-use crate::{slice_in_ram, DmaSlice};
+use crate::{slice_in_ram, slice_in_ram_or, DmaSlice};
 use embedded_hal::digital::v2::OutputPin;
 
 /// Interface to a SPIM instance
@@ -39,7 +39,7 @@ where
 
     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
-        ram_slice_check(words)?;
+        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))
@@ -242,7 +242,7 @@ where
         chip_select: &mut Pin<Output<PushPull>>,
         buffer: &mut [u8],
     ) -> Result<(), Error> {
-        ram_slice_check(buffer)?;
+        slice_in_ram_or(buffer, Error::DMABufferNotInDataMemory)?;
 
         chip_select.set_low().unwrap();
 
@@ -273,8 +273,9 @@ where
         tx_buffer: &[u8],
         rx_buffer: &mut [u8],
     ) -> Result<(), Error> {
-        ram_slice_check(tx_buffer)?;
-        ram_slice_check(rx_buffer)?;
+        // NOTE: RAM slice check for `rx_buffer` is not necessary, as a mutable
+        // slice can only be built from data located in RAM
+        slice_in_ram_or(tx_buffer, Error::DMABufferNotInDataMemory)?;
 
         let txi = tx_buffer.chunks(EASY_DMA_SIZE);
         let rxi = rx_buffer.chunks_mut(EASY_DMA_SIZE);
@@ -310,8 +311,10 @@ where
         tx_buffer: &[u8],
         rx_buffer: &mut [u8],
     ) -> Result<(), Error> {
-        ram_slice_check(tx_buffer)?;
-        ram_slice_check(rx_buffer)?;
+        // NOTE: RAM slice check for `rx_buffer` is not necessary, as a mutable
+        // slice can only be built from data located in RAM
+        slice_in_ram_or(tx_buffer, Error::DMABufferNotInDataMemory)?;
+
         // For the tx and rx, we want to return Some(chunk)
         // as long as there is data to send. We then chain a repeat to
         // the end so once all chunks have been exhausted, we will keep
@@ -362,7 +365,7 @@ where
         chip_select: &mut Pin<Output<PushPull>>,
         tx_buffer: &[u8],
     ) -> Result<(), Error> {
-        ram_slice_check(tx_buffer)?;
+        slice_in_ram_or(tx_buffer, Error::DMABufferNotInDataMemory)?;
         self.transfer_split_uneven(chip_select, tx_buffer, &mut [0u8; 0])
     }
 
@@ -396,14 +399,6 @@ pub enum Error {
     Receive,
 }
 
-fn ram_slice_check(slice: &[u8]) -> Result<(), Error> {
-    if slice_in_ram(slice) {
-        Ok(())
-    } else {
-        Err(Error::DMABufferNotInDataMemory)
-    }
-}
-
 /// Implemented by all SPIM instances
 pub trait Instance: Deref<Target = spim0::RegisterBlock> {}
 

nrf52-hal-common/src/twim.rs

@@ -16,9 +16,11 @@ use crate::target::{twim0, P0, TWIM0};
 #[cfg(any(feature = "52832", feature = "52840"))]
 use crate::target::TWIM1;
 
-use crate::gpio::{Floating, Input, Pin};
-
-use crate::target_constants::EASY_DMA_SIZE;
+use crate::{
+    gpio::{Floating, Input, Pin},
+    target_constants::EASY_DMA_SIZE,
+    slice_in_ram_or,
+};
 
 pub use twim0::frequency::FREQUENCYW as Frequency;
 
@@ -89,6 +91,8 @@ where
     /// The buffer must have a length of at most 255 bytes on the nRF52832
     /// and at most 65535 bytes on the nRF52840.
     pub fn write(&mut self, address: u8, buffer: &[u8]) -> Result<(), Error> {
+        slice_in_ram_or(buffer, Error::DMABufferNotInDataMemory)?;
+
         if buffer.len() > EASY_DMA_SIZE {
             return Err(Error::TxBufferTooLong);
         }
@@ -155,6 +159,9 @@ where
     /// The buffer must have a length of at most 255 bytes on the nRF52832
     /// and at most 65535 bytes on the nRF52840.
     pub fn read(&mut self, address: u8, buffer: &mut [u8]) -> Result<(), Error> {
+        // NOTE: RAM slice check is not necessary, as a mutable slice can only be
+        // built from data located in RAM
+
         if buffer.len() > EASY_DMA_SIZE {
             return Err(Error::RxBufferTooLong);
         }
@@ -229,6 +236,10 @@ where
         wr_buffer: &[u8],
         rd_buffer: &mut [u8],
     ) -> Result<(), Error> {
+        // NOTE: RAM slice check for `rd_buffer` is not necessary, as a mutable
+        // slice can only be built from data located in RAM
+        slice_in_ram_or(wr_buffer, Error::DMABufferNotInDataMemory)?;
+
         if wr_buffer.len() > EASY_DMA_SIZE {
             return Err(Error::TxBufferTooLong);
         }
@@ -385,6 +396,7 @@ pub enum Error {
     RxBufferTooLong,
     Transmit,
     Receive,
+    DMABufferNotInDataMemory,
 }
 
 /// Implemented by all TWIM instances

nrf52-hal-common/src/uarte.rs

@@ -23,6 +23,7 @@ use crate::gpio::{Floating, Input, Output, Pin, PushPull};
 use crate::prelude::*;
 use crate::target_constants::EASY_DMA_SIZE;
 use crate::timer::{self, Timer};
+use crate::slice_in_ram_or;
 
 // Re-export SVD variants to allow user to directly set values
 pub use uarte0::{baudrate::BAUDRATEW as Baudrate, config::PARITYW as Parity};
@@ -107,9 +108,7 @@ where
         }
 
         // We can only DMA out of RAM
-        if !crate::slice_in_ram(tx_buffer) {
-            return Err(Error::BufferNotInRAM);
-        }
+        slice_in_ram_or(tx_buffer, Error::BufferNotInRAM)?;
 
         // Conservative compiler fence to prevent optimizations that do not
         // take in to account actions by DMA. The fence has been placed here,
@@ -264,6 +263,9 @@ where
             return Err(Error::TxBufferTooLong);
         }
 
+        // NOTE: RAM slice check is not necessary, as a mutable slice can only be
+        // built from data located in RAM
+
         // 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