diff --git a/hal/src/sercom/spi.rs b/hal/src/sercom/spi.rs index f9f081cefc2..bda4b47a5b8 100644 --- a/hal/src/sercom/spi.rs +++ b/hal/src/sercom/spi.rs @@ -312,7 +312,6 @@ let (chan0, _, spi, _) = dma_transfer.wait(); " )] -use core::convert::TryFrom; use core::marker::PhantomData; use bitflags::bitflags; @@ -417,15 +416,13 @@ bitflags! { } } -/// Convert [`Status`] flags into the corresponding [`Error`] variants -impl TryFrom for () { - type Error = Error; - #[inline] - fn try_from(status: Status) -> Result<(), Error> { +impl Status { + /// Check the [`Status`] flags for [`Error`] conditions + pub fn check_errors(&self) -> Result<(), Error> { // Buffer overflow has priority - if status.contains(Status::BUFOVF) { + if self.contains(Status::BUFOVF) { Err(Error::Overflow) - } else if status.contains(Status::LENERR) { + } else if self.contains(Status::LENERR) { Err(Error::LengthError) } else { Ok(()) @@ -564,13 +561,24 @@ seq!(N in 1..=4 { // Capability //============================================================================== +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum DynCapability { + Rx, + Tx, + Duplex, +} + +impl Sealed for DynCapability {} + /// Type-level enum representing the simplex or duplex transaction capability /// /// The available, type-level variants are [`Rx`], [`Tx`] and [`Duplex`]. See /// the [type-level enum] documentation for more details. /// /// [type-level enum]: crate::typelevel#type-level-enums -pub trait Capability: Sealed + Default {} +pub trait Capability: Sealed + Default { + const DYN: DynCapability; +} /// Sub-set of [`Capability`] variants that can receive data, i.e. [`Rx`] and /// [`Duplex`] @@ -597,7 +605,9 @@ pub struct Rx { } impl Sealed for Rx {} -impl Capability for Rx {} +impl Capability for Rx { + const DYN: DynCapability = DynCapability::Rx; +} impl Receive for Rx {} /// Type-level variant of the [`Capability`] enum for simplex, [`Transmit`]-only @@ -609,7 +619,9 @@ impl Receive for Rx {} pub struct Tx; impl Sealed for Tx {} -impl Capability for Tx {} +impl Capability for Tx { + const DYN: DynCapability = DynCapability::Tx; +} impl Transmit for Tx {} /// Type-level variant of the [`Capability`] enum for duplex transactions @@ -621,7 +633,9 @@ impl Transmit for Tx {} pub struct Duplex; impl Sealed for Duplex {} -impl Capability for Duplex {} +impl Capability for Duplex { + const DYN: DynCapability = DynCapability::Duplex; +} impl Receive for Duplex {} impl Transmit for Duplex {} @@ -1041,10 +1055,10 @@ where /// [type class]: crate::typelevel#type-classes pub trait AnyConfig: Is> { type Sercom: Sercom; - type Pads: ValidPads; + type Pads: ValidPads; type Capability: Capability; type OpMode: OpMode; - type Size: Size; + type Size: Size; type Word: 'static; } @@ -1304,11 +1318,9 @@ where } #[cfg(feature = "min-samd51g")] -impl Spi, A> +impl Spi where - P: ValidPads, - M: OpMode, - Config: ValidConfig, + C: ValidConfig, A: Capability, { /// Return the current transaction length @@ -1316,7 +1328,7 @@ where /// Read the LENGTH register to determine the current transaction length #[inline] pub fn get_dyn_length(&self) -> u8 { - self.config.get_dyn_length() + self.config.as_ref().get_dyn_length() } /// Set the transaction length @@ -1330,7 +1342,7 @@ where /// `LENGTH`. #[inline] pub fn set_dyn_length(&mut self, length: u8) { - self.config.set_dyn_length(length); + self.config.as_mut().set_dyn_length(length); } } @@ -1358,7 +1370,14 @@ pub trait AnySpi: Is> { type OpMode: OpMode; type Size: Size; type Word: 'static; - type Config: ValidConfig; + type Config: ValidConfig< + Sercom = Self::Sercom, + Pads = Self::Pads, + Capability = Self::Capability, + OpMode = Self::OpMode, + Size = Self::Size, + Word = Self::Word, + >; } /// Type alias to recover the specific [`Spi`] type from an implementation of @@ -1394,14 +1413,10 @@ where { } -impl AnySpi for Spi -where - C: ValidConfig, - A: Capability, -{ +impl AnySpi for Spi { type Sercom = C::Sercom; type Pads = C::Pads; - type Capability = A; + type Capability = C::Capability; type OpMode = C::OpMode; type Size = C::Size; type Word = C::Word; diff --git a/hal/src/sercom/spi/impl_ehal_thumbv7em.rs b/hal/src/sercom/spi/impl_ehal_thumbv7em.rs index dfa8f321ca4..0e67a714726 100644 --- a/hal/src/sercom/spi/impl_ehal_thumbv7em.rs +++ b/hal/src/sercom/spi/impl_ehal_thumbv7em.rs @@ -336,7 +336,9 @@ where #[inline] fn transfer<'w>(&mut self, buf: &'w mut [u8]) -> Result<&'w [u8], Error> { - assert_eq!(buf.len(), L::USIZE); + if buf.len() != L::USIZE { + panic!("Slice length does not equal SPI transfer length"); + } let sercom = unsafe { self.config.as_ref().sercom() }; transfer_slice(sercom, buf) } @@ -352,7 +354,7 @@ where /// [`Transfer`]: blocking::spi::Transfer impl blocking::spi::Transfer for Spi, A> where - Config: ValidConfig, + Config: ValidConfig, P: ValidPads, M: OpMode, A: Receive, @@ -361,7 +363,9 @@ where #[inline] fn transfer<'w>(&mut self, buf: &'w mut [u8]) -> Result<&'w [u8], Error> { - assert_eq!(buf.len(), self.get_dyn_length() as usize); + if buf.len() != self.get_dyn_length() as usize { + panic!("Slice length does not equal SPI transfer length"); + } let sercom = unsafe { self.config.as_ref().sercom() }; transfer_slice(sercom, buf) } @@ -524,7 +528,7 @@ where /// [`Write`]: blocking::spi::Write impl blocking::spi::Write for Spi, Duplex> where - Config: ValidConfig, + Config: ValidConfig, P: ValidPads, M: OpMode, { @@ -553,7 +557,7 @@ where /// [`Write`]: blocking::spi::Write impl blocking::spi::Write for Spi, Tx> where - Config: ValidConfig, + Config: ValidConfig, P: ValidPads, M: OpMode, { @@ -723,8 +727,8 @@ fn transfer_slice<'w>(sercom: &RegisterBlock, buf: &'w mut [u8]) -> Result<&'w [ /// If `duplex` is false, buffer overflow errors are ignored fn write_slice(sercom: &RegisterBlock, buf: &[u8], duplex: bool) -> Result<(), Error> { let mut to_send = buf.iter(); - let mut to_recv: usize = to_send.len(); - while to_recv > 0 { + let mut to_recv: usize = if duplex { to_send.len() } else { 0 }; + loop { let errors = sercom.spim().status.read(); if duplex && errors.bufovf().bit_is_set() { return Err(Error::Overflow); @@ -733,7 +737,6 @@ fn write_slice(sercom: &RegisterBlock, buf: &[u8], duplex: bool) -> Result<(), E return Err(Error::LengthError); } let flags = sercom.spim().intflag.read(); - // Send the word if to_send.len() > 0 && flags.dre().bit_is_set() { let mut bytes = [0; 4]; for byte in &mut bytes { @@ -745,11 +748,14 @@ fn write_slice(sercom: &RegisterBlock, buf: &[u8], duplex: bool) -> Result<(), E let word = u32::from_le_bytes(bytes); sercom.spim().data.write(|w| unsafe { w.data().bits(word) }); } - if duplex && to_recv > to_send.len() && flags.rxc().bit_is_set() { + if to_recv > to_send.len() && flags.rxc().bit_is_set() { sercom.spim().data.read().data().bits(); let diff = to_recv - to_send.len(); to_recv -= if diff < 4 { diff } else { 4 }; } + if to_recv == 0 && to_send.len() == 0 { + break; + } } Ok(()) } diff --git a/hal/src/sercom/spi/reg.rs b/hal/src/sercom/spi/reg.rs index 5a21ee8b2db..372d8eadcd0 100644 --- a/hal/src/sercom/spi/reg.rs +++ b/hal/src/sercom/spi/reg.rs @@ -340,7 +340,7 @@ impl Registers { /// Try to read the interrupt flags, but first check the error status flags. #[inline] pub fn read_flags_errors(&self) -> Result { - self.read_status().try_into()?; + self.read_status().check_errors()?; Ok(self.read_flags()) } } diff --git a/hal/src/sercom/spi_future.rs b/hal/src/sercom/spi_future.rs index 10b40c93160..3b713eea487 100644 --- a/hal/src/sercom/spi_future.rs +++ b/hal/src/sercom/spi_future.rs @@ -30,8 +30,8 @@ //! Create an `SpiFuture` like so //! //! ``` -//! use atsamd_hal::sercom::v2::spi::AnySpi; -//! use atsamd_hal::sercom::v2::spi_future::SpiFuture; +//! use atsamd_hal::sercom::spi::AnySpi; +//! use atsamd_hal::sercom::spi_future::SpiFuture; //! //! fn wake_up() { //! //... @@ -43,9 +43,8 @@ //! } //! ``` //! -//! Like real [`Future`]s, `SpiFuture`s are lazy. Nothing happens until calling -//! [`start`]. Doing so will enable the `DRE` and `RXC` interrupts and begin the -//! transaction. +//! `SpiFuture`s are lazy; nothing happens until calling [`start`]. Doing so +//! will enable the `DRE` and `RXC` interrupts and begin the transaction. //! //! ``` //! use atsamd_hal::sercom::spi::AnySpi; @@ -223,9 +222,10 @@ //! [RTIC]: https://rtic.rs/ //! [`bytemuck`]: https://docs.rs/bytemuck +use core::cmp::min; use core::task::Poll; -use super::spi::{AnySpi, Error, Flags, SpecificSpi}; +use super::spi::{AnySpi, Capability, DynCapability, Error, Flags, SpecificSpi}; #[cfg(any(feature = "samd11", feature = "samd21"))] type Data = u16; @@ -250,7 +250,7 @@ type Data = u32; /// a NOP. /// /// [`Spi`]: super::spi::Spi -/// [`Pin`]: crate::gpio::v2::pin::Pin +/// [`Pin`]: crate::gpio::pin::Pin /// [`assert_ss`]: AsSpi::assert_ss /// [`deassert_ss`]: AsSpi::assert_ss pub trait AsSpi { @@ -313,7 +313,7 @@ pub trait AsSpi { /// } /// ``` /// - /// [`Pin`]: crate::gpio::v2::pin::Pin + /// [`Pin`]: crate::gpio::pin::Pin #[inline] fn assert_ss(&mut self) {} @@ -333,7 +333,7 @@ pub trait AsSpi { /// } /// ``` /// - /// [`Pin`]: crate::gpio::v2::pin::Pin + /// [`Pin`]: crate::gpio::pin::Pin #[inline] fn deassert_ss(&mut self) {} } @@ -424,26 +424,35 @@ where B: AsRef<[u8]> + AsMut<[u8]> + 'static, W: FnOnce() + 'static, { + const CAP: DynCapability = ::Capability::DYN; + #[inline] fn step(&self) -> usize { let len = self.resource.spi().transaction_length() as usize; - if len > 4 { - 4 - } else { - len - } + min(len, 4) } /// Start the [`SpiFuture`] transaction /// /// This will assert the SS pin, if present, and enable the `DRE` and `RXC` /// interrupts. + /// + /// When the [`Spi`] struct is [`Tx`]-only, this will also perform reads of + /// the `DATA` register to clear any accumulated overflow errors. Omitting + /// this step would lead to spurious `RXC` interrupts. #[inline] pub fn start(&mut self) { self.resource.assert_ss(); - self.resource - .spi_mut() - .enable_interrupts(Flags::DRE | Flags::RXC); + let spi = self.resource.spi_mut(); + if Self::CAP == DynCapability::Tx { + // Clear any existing RXC or BUFOVF flags + unsafe { + spi.read_data(); + spi.read_data(); + spi.read_data(); + } + } + spi.enable_interrupts(Flags::DRE | Flags::RXC); } /// Send the next set of bytes from the buffer @@ -456,7 +465,12 @@ where let step = self.step(); let spi = self.resource.spi_mut(); let buf = self.buf.as_ref(); - let _ = spi.read_flags_errors()?; + match spi.read_status().check_errors() { + Ok(()) => {} + // Ignore overflow errors if we are only transmitting + Err(Error::Overflow) if Self::CAP == DynCapability::Tx => {} + Err(err) => return Err(err), + } if let Some(buf) = buf.get(self.sent..) { let mut data = buf.iter(); let mut bytes = [0; 4]; @@ -477,7 +491,7 @@ where Ok(()) } - /// Received the next set of bytes and write them to the buffer + /// Receive the next set of bytes and write them to the buffer /// /// This method should be called from the `RXC` interrupt handler. Once all /// bytes of the transaction have been received, this function will @@ -488,7 +502,12 @@ where let step = self.step(); let spi = self.resource.spi_mut(); let buf = self.buf.as_mut(); - let _ = spi.read_flags_errors()?; + match spi.read_status().check_errors() { + Ok(()) => {} + // Ignore overflow errors if we are only transmitting + Err(Error::Overflow) if Self::CAP == DynCapability::Tx => {} + Err(err) => return Err(err), + } if self.rcvd < self.sent { let buf = unsafe { buf.get_unchecked_mut(self.rcvd..) }; let mut data = buf.into_iter(); @@ -533,7 +552,8 @@ where /// If the transaction is complete, this function will consume the /// [`SpiFuture`] and return the resource and buffer. /// - /// If the transaction is not complete, it will return `Err(self)`. + /// If the transaction is not complete, it will return the future as an + /// `Err`. #[inline] pub fn free(self) -> Result<(R, B), Self> { if self.rcvd >= self.buf.as_ref().len() {