From 7ad76f5f603078e7be300ae6fb08c6f6bf23f6c5 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 13 Jun 2024 20:41:08 +0200 Subject: [PATCH] Use raw slices .len() method instead of unsafe hacks. Stabilized in 1.79. --- embassy-nrf/src/i2s.rs | 7 +++---- embassy-nrf/src/spim.rs | 12 +++++------ embassy-nrf/src/spis.rs | 16 +++++++------- embassy-nrf/src/util.rs | 35 +++++++------------------------ embassy-rp/src/dma.rs | 29 +++++++------------------ embassy-rp/src/spi.rs | 15 ++++++------- embassy-stm32/src/dma/dma_bdma.rs | 16 ++++++-------- embassy-stm32/src/dma/gpdma.rs | 16 ++++++-------- embassy-stm32/src/dma/mod.rs | 13 ------------ embassy-stm32/src/spi/mod.rs | 8 +++---- 10 files changed, 51 insertions(+), 116 deletions(-) diff --git a/embassy-nrf/src/i2s.rs b/embassy-nrf/src/i2s.rs index 966271ed9..5f565a9b7 100644 --- a/embassy-nrf/src/i2s.rs +++ b/embassy-nrf/src/i2s.rs @@ -16,7 +16,7 @@ use embassy_sync::waitqueue::AtomicWaker; use crate::gpio::{AnyPin, Pin as GpioPin}; use crate::interrupt::typelevel::Interrupt; use crate::pac::i2s::RegisterBlock; -use crate::util::{slice_in_ram_or, slice_ptr_parts}; +use crate::util::slice_in_ram_or; use crate::{interrupt, Peripheral, EASY_DMA_SIZE}; /// Type alias for `MultiBuffering` with 2 buffers. @@ -1028,9 +1028,8 @@ impl Device { } fn validated_dma_parts(buffer_ptr: *const [S]) -> Result<(u32, u32), Error> { - let (ptr, len) = slice_ptr_parts(buffer_ptr); - let ptr = ptr as u32; - let bytes_len = len * size_of::(); + let ptr = buffer_ptr as *const S as u32; + let bytes_len = buffer_ptr.len() * size_of::(); let maxcnt = (bytes_len / size_of::()) as u32; trace!("PTR={}, MAXCNT={}", ptr, maxcnt); diff --git a/embassy-nrf/src/spim.rs b/embassy-nrf/src/spim.rs index 373f22642..52660711a 100644 --- a/embassy-nrf/src/spim.rs +++ b/embassy-nrf/src/spim.rs @@ -19,7 +19,7 @@ pub use pac::spim0::frequency::FREQUENCY_A as Frequency; use crate::chip::{EASY_DMA_SIZE, FORCE_COPY_BUFFER_SIZE}; use crate::gpio::{self, convert_drive, AnyPin, OutputDrive, Pin as GpioPin, PselBits, SealedPin as _}; use crate::interrupt::typelevel::Interrupt; -use crate::util::{slice_in_ram_or, slice_ptr_len, slice_ptr_parts, slice_ptr_parts_mut}; +use crate::util::slice_in_ram_or; use crate::{interrupt, pac, Peripheral}; /// SPIM error @@ -240,14 +240,12 @@ impl<'d, T: Instance> Spim<'d, T> { } // Set up the DMA read. - let (ptr, len) = slice_ptr_parts_mut(rx); - let (rx_ptr, rx_len) = xfer_params(ptr as _, len as _, offset, length); + let (rx_ptr, rx_len) = xfer_params(rx as *mut u8 as _, rx.len() as _, offset, length); r.rxd.ptr.write(|w| unsafe { w.ptr().bits(rx_ptr) }); r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(rx_len as _) }); // Set up the DMA write. - let (ptr, len) = slice_ptr_parts(tx); - let (tx_ptr, tx_len) = xfer_params(ptr as _, len as _, offset, length); + let (tx_ptr, tx_len) = xfer_params(tx as *const u8 as _, tx.len() as _, offset, length); r.txd.ptr.write(|w| unsafe { w.ptr().bits(tx_ptr) }); r.txd.maxcnt.write(|w| unsafe { w.maxcnt().bits(tx_len as _) }); @@ -302,7 +300,7 @@ impl<'d, T: Instance> Spim<'d, T> { // NOTE: RAM slice check for rx is not necessary, as a mutable // slice can only be built from data located in RAM. - let xfer_len = core::cmp::max(slice_ptr_len(rx), slice_ptr_len(tx)); + let xfer_len = core::cmp::max(rx.len(), tx.len()); for offset in (0..xfer_len).step_by(EASY_DMA_SIZE) { let length = core::cmp::min(xfer_len - offset, EASY_DMA_SIZE); self.blocking_inner_from_ram_chunk(rx, tx, offset, length); @@ -356,7 +354,7 @@ impl<'d, T: Instance> Spim<'d, T> { // NOTE: RAM slice check for rx is not necessary, as a mutable // slice can only be built from data located in RAM. - let xfer_len = core::cmp::max(slice_ptr_len(rx), slice_ptr_len(tx)); + let xfer_len = core::cmp::max(rx.len(), tx.len()); for offset in (0..xfer_len).step_by(EASY_DMA_SIZE) { let length = core::cmp::min(xfer_len - offset, EASY_DMA_SIZE); self.async_inner_from_ram_chunk(rx, tx, offset, length).await; diff --git a/embassy-nrf/src/spis.rs b/embassy-nrf/src/spis.rs index 47bbeaf77..e98b34369 100644 --- a/embassy-nrf/src/spis.rs +++ b/embassy-nrf/src/spis.rs @@ -15,7 +15,7 @@ pub use pac::spis0::config::ORDER_A as BitOrder; use crate::chip::{EASY_DMA_SIZE, FORCE_COPY_BUFFER_SIZE}; use crate::gpio::{self, AnyPin, Pin as GpioPin, SealedPin as _}; use crate::interrupt::typelevel::Interrupt; -use crate::util::{slice_in_ram_or, slice_ptr_parts, slice_ptr_parts_mut}; +use crate::util::slice_in_ram_or; use crate::{interrupt, pac, Peripheral}; /// SPIS error @@ -226,20 +226,18 @@ impl<'d, T: Instance> Spis<'d, T> { let r = T::regs(); // Set up the DMA write. - let (ptr, len) = slice_ptr_parts(tx); - if len > EASY_DMA_SIZE { + if tx.len() > EASY_DMA_SIZE { return Err(Error::TxBufferTooLong); } - r.txd.ptr.write(|w| unsafe { w.ptr().bits(ptr as _) }); - r.txd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) }); + r.txd.ptr.write(|w| unsafe { w.ptr().bits(tx as *const u8 as _) }); + r.txd.maxcnt.write(|w| unsafe { w.maxcnt().bits(tx.len() as _) }); // Set up the DMA read. - let (ptr, len) = slice_ptr_parts_mut(rx); - if len > EASY_DMA_SIZE { + if rx.len() > EASY_DMA_SIZE { return Err(Error::RxBufferTooLong); } - r.rxd.ptr.write(|w| unsafe { w.ptr().bits(ptr as _) }); - r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(len as _) }); + r.rxd.ptr.write(|w| unsafe { w.ptr().bits(rx as *mut u8 as _) }); + r.rxd.maxcnt.write(|w| unsafe { w.maxcnt().bits(rx.len() as _) }); // Reset end event. r.events_end.reset(); diff --git a/embassy-nrf/src/util.rs b/embassy-nrf/src/util.rs index 13aba7dec..78f71719f 100644 --- a/embassy-nrf/src/util.rs +++ b/embassy-nrf/src/util.rs @@ -1,42 +1,21 @@ #![allow(dead_code)] -use core::mem; const SRAM_LOWER: usize = 0x2000_0000; const SRAM_UPPER: usize = 0x3000_0000; -// #![feature(const_slice_ptr_len)] -// https://github.com/rust-lang/rust/issues/71146 -pub(crate) fn slice_ptr_len(ptr: *const [T]) -> usize { - use core::ptr::NonNull; - let ptr = ptr.cast_mut(); - if let Some(ptr) = NonNull::new(ptr) { - ptr.len() - } else { - // We know ptr is null, so we know ptr.wrapping_byte_add(1) is not null. - NonNull::new(ptr.wrapping_byte_add(1)).unwrap().len() - } -} - -// TODO: replace transmutes with core::ptr::metadata once it's stable -pub(crate) fn slice_ptr_parts(slice: *const [T]) -> (*const T, usize) { - unsafe { mem::transmute(slice) } -} - -pub(crate) fn slice_ptr_parts_mut(slice: *mut [T]) -> (*mut T, usize) { - unsafe { mem::transmute(slice) } -} - /// Does this slice reside entirely within RAM? pub(crate) fn slice_in_ram(slice: *const [T]) -> bool { - let (ptr, len) = slice_ptr_parts(slice); - let ptr = ptr as usize; - ptr >= SRAM_LOWER && (ptr + len * core::mem::size_of::()) < SRAM_UPPER + if slice.is_empty() { + return true; + } + + let ptr = slice as *const T as usize; + ptr >= SRAM_LOWER && (ptr + slice.len() * core::mem::size_of::()) < SRAM_UPPER } /// Return an error if slice is not in RAM. Skips check if slice is zero-length. pub(crate) fn slice_in_ram_or(slice: *const [T], err: E) -> Result<(), E> { - let (_, len) = slice_ptr_parts(slice); - if len == 0 || slice_in_ram(slice) { + if slice_in_ram(slice) { Ok(()) } else { Err(err) diff --git a/embassy-rp/src/dma.rs b/embassy-rp/src/dma.rs index e6374a86c..8c04b43a1 100644 --- a/embassy-rp/src/dma.rs +++ b/embassy-rp/src/dma.rs @@ -47,12 +47,11 @@ pub unsafe fn read<'a, C: Channel, W: Word>( to: *mut [W], dreq: u8, ) -> Transfer<'a, C> { - let (to_ptr, len) = crate::dma::slice_ptr_parts(to); copy_inner( ch, from as *const u32, - to_ptr as *mut u32, - len, + to as *mut W as *mut u32, + to.len(), W::size(), false, true, @@ -69,12 +68,11 @@ pub unsafe fn write<'a, C: Channel, W: Word>( to: *mut W, dreq: u8, ) -> Transfer<'a, C> { - let (from_ptr, len) = crate::dma::slice_ptr_parts(from); copy_inner( ch, - from_ptr as *const u32, + from as *const W as *const u32, to as *mut u32, - len, + from.len(), W::size(), true, false, @@ -114,13 +112,13 @@ pub unsafe fn copy<'a, C: Channel, W: Word>( from: &[W], to: &mut [W], ) -> Transfer<'a, C> { - let (from_ptr, from_len) = crate::dma::slice_ptr_parts(from); - let (to_ptr, to_len) = crate::dma::slice_ptr_parts_mut(to); + let from_len = from.len(); + let to_len = to.len(); assert_eq!(from_len, to_len); copy_inner( ch, - from_ptr as *const u32, - to_ptr as *mut u32, + from.as_ptr() as *const u32, + to.as_mut_ptr() as *mut u32, from_len, W::size(), true, @@ -287,17 +285,6 @@ macro_rules! channel { }; } -// TODO: replace transmutes with core::ptr::metadata once it's stable -#[allow(unused)] -pub(crate) fn slice_ptr_parts(slice: *const [T]) -> (usize, usize) { - unsafe { core::mem::transmute(slice) } -} - -#[allow(unused)] -pub(crate) fn slice_ptr_parts_mut(slice: *mut [T]) -> (usize, usize) { - unsafe { core::mem::transmute(slice) } -} - channel!(DMA_CH0, 0); channel!(DMA_CH1, 1); channel!(DMA_CH2, 2); diff --git a/embassy-rp/src/spi.rs b/embassy-rp/src/spi.rs index ef4c644ae..1617c144c 100644 --- a/embassy-rp/src/spi.rs +++ b/embassy-rp/src/spi.rs @@ -394,17 +394,14 @@ impl<'d, T: Instance> Spi<'d, T, Async> { self.transfer_inner(words, words).await } - async fn transfer_inner(&mut self, rx_ptr: *mut [u8], tx_ptr: *const [u8]) -> Result<(), Error> { - let (_, tx_len) = crate::dma::slice_ptr_parts(tx_ptr); - let (_, rx_len) = crate::dma::slice_ptr_parts_mut(rx_ptr); - + async fn transfer_inner(&mut self, rx: *mut [u8], tx: *const [u8]) -> Result<(), Error> { // Start RX first. Transfer starts when TX starts, if RX // is not started yet we might lose bytes. 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().as_ptr() as *const _, rx_ptr, T::RX_DREQ) + crate::dma::read(rx_ch, self.inner.regs().dr().as_ptr() as *const _, rx, T::RX_DREQ) }; let mut tx_ch = self.tx_dma.as_mut().unwrap(); @@ -413,10 +410,10 @@ impl<'d, T: Instance> Spi<'d, T, Async> { let tx_transfer = async { let p = self.inner.regs(); unsafe { - crate::dma::write(&mut tx_ch, tx_ptr, p.dr().as_ptr() as *mut _, T::TX_DREQ).await; + crate::dma::write(&mut tx_ch, tx, p.dr().as_ptr() as *mut _, T::TX_DREQ).await; - if rx_len > tx_len { - let write_bytes_len = rx_len - tx_len; + if rx.len() > tx.len() { + let write_bytes_len = rx.len() - tx.len(); // write dummy data // this will disable incrementation of the buffers crate::dma::write_repeated(tx_ch, p.dr().as_ptr() as *mut u8, write_bytes_len, T::TX_DREQ).await @@ -426,7 +423,7 @@ impl<'d, T: Instance> Spi<'d, T, Async> { join(tx_transfer, rx_transfer).await; // if tx > rx we should clear any overflow of the FIFO SPI buffer - if tx_len > rx_len { + if tx.len() > rx.len() { let p = self.inner.regs(); while p.sr().read().bsy() {} diff --git a/embassy-stm32/src/dma/dma_bdma.rs b/embassy-stm32/src/dma/dma_bdma.rs index a6344cf06..a462e317f 100644 --- a/embassy-stm32/src/dma/dma_bdma.rs +++ b/embassy-stm32/src/dma/dma_bdma.rs @@ -565,16 +565,13 @@ impl<'a> Transfer<'a> { ) -> Self { into_ref!(channel); - let (ptr, len) = super::slice_ptr_parts_mut(buf); - assert!(len > 0 && len <= 0xFFFF); - Self::new_inner( channel.map_into(), request, Dir::PeripheralToMemory, peri_addr as *const u32, - ptr as *mut u32, - len, + buf as *mut W as *mut u32, + buf.len(), true, W::size(), options, @@ -602,16 +599,13 @@ impl<'a> Transfer<'a> { ) -> Self { into_ref!(channel); - let (ptr, len) = super::slice_ptr_parts(buf); - assert!(len > 0 && len <= 0xFFFF); - Self::new_inner( channel.map_into(), request, Dir::MemoryToPeripheral, peri_addr as *const u32, - ptr as *mut u32, - len, + buf as *const W as *mut u32, + buf.len(), true, W::size(), options, @@ -653,6 +647,8 @@ impl<'a> Transfer<'a> { data_size: WordSize, options: TransferOptions, ) -> Self { + assert!(mem_len > 0 && mem_len <= 0xFFFF); + channel.configure( _request, dir, peri_addr, mem_addr, mem_len, incr_mem, data_size, options, ); diff --git a/embassy-stm32/src/dma/gpdma.rs b/embassy-stm32/src/dma/gpdma.rs index a3717e67b..13d5d15be 100644 --- a/embassy-stm32/src/dma/gpdma.rs +++ b/embassy-stm32/src/dma/gpdma.rs @@ -125,16 +125,13 @@ impl<'a> Transfer<'a> { ) -> Self { into_ref!(channel); - let (ptr, len) = super::slice_ptr_parts_mut(buf); - assert!(len > 0 && len <= 0xFFFF); - Self::new_inner( channel.map_into(), request, Dir::PeripheralToMemory, peri_addr as *const u32, - ptr as *mut u32, - len, + buf as *mut W as *mut u32, + buf.len(), true, W::size(), options, @@ -162,16 +159,13 @@ impl<'a> Transfer<'a> { ) -> Self { into_ref!(channel); - let (ptr, len) = super::slice_ptr_parts(buf); - assert!(len > 0 && len <= 0xFFFF); - Self::new_inner( channel.map_into(), request, Dir::MemoryToPeripheral, peri_addr as *const u32, - ptr as *mut u32, - len, + buf as *const W as *mut u32, + buf.len(), true, W::size(), options, @@ -213,6 +207,8 @@ impl<'a> Transfer<'a> { data_size: WordSize, _options: TransferOptions, ) -> Self { + assert!(mem_len > 0 && mem_len <= 0xFFFF); + let info = channel.info(); let ch = info.dma.ch(info.num); diff --git a/embassy-stm32/src/dma/mod.rs b/embassy-stm32/src/dma/mod.rs index 3f5687a62..66c4aa53c 100644 --- a/embassy-stm32/src/dma/mod.rs +++ b/embassy-stm32/src/dma/mod.rs @@ -22,8 +22,6 @@ pub(crate) use util::*; pub(crate) mod ringbuffer; pub mod word; -use core::mem; - use embassy_hal_internal::{impl_peripheral, Peripheral}; use crate::interrupt; @@ -121,17 +119,6 @@ pub struct NoDma; impl_peripheral!(NoDma); -// TODO: replace transmutes with core::ptr::metadata once it's stable -#[allow(unused)] -pub(crate) fn slice_ptr_parts(slice: *const [T]) -> (usize, usize) { - unsafe { mem::transmute(slice) } -} - -#[allow(unused)] -pub(crate) fn slice_ptr_parts_mut(slice: *mut [T]) -> (usize, usize) { - unsafe { mem::transmute(slice) } -} - // safety: must be called only once at startup pub(crate) unsafe fn init( cs: critical_section::CriticalSection, diff --git a/embassy-stm32/src/spi/mod.rs b/embassy-stm32/src/spi/mod.rs index 03c908db8..2e5f82dcb 100644 --- a/embassy-stm32/src/spi/mod.rs +++ b/embassy-stm32/src/spi/mod.rs @@ -9,7 +9,7 @@ use embassy_futures::join::join; use embassy_hal_internal::PeripheralRef; pub use embedded_hal_02::spi::{Mode, Phase, Polarity, MODE_0, MODE_1, MODE_2, MODE_3}; -use crate::dma::{slice_ptr_parts, word, ChannelAndRequest}; +use crate::dma::{word, ChannelAndRequest}; use crate::gpio::{AFType, AnyPin, Pull, SealedPin as _, Speed}; use crate::mode::{Async, Blocking, Mode as PeriMode}; use crate::pac::spi::{regs, vals, Spi as Regs}; @@ -798,10 +798,8 @@ impl<'d> Spi<'d, Async> { } async fn transfer_inner(&mut self, read: *mut [W], write: *const [W]) -> Result<(), Error> { - let (_, rx_len) = slice_ptr_parts(read); - let (_, tx_len) = slice_ptr_parts(write); - assert_eq!(rx_len, tx_len); - if rx_len == 0 { + assert_eq!(read.len(), write.len()); + if read.len() == 0 { return Ok(()); }