Skip to content

Commit

Permalink
Make multiple updates to the spi module
Browse files Browse the repository at this point in the history
- Make conversion from `Status` to `Error` an inherent function of
  `Status` rather than an implementation of `TryFrom`
- Add a `DynCapability` enum to allow run-time branching on `Capability`
- Improve type-level consistency for the `AnyConfig` and `AnySpi` traits
- Change how the `.get_dyn_length()` and `.set_dyn_length()` methods are
  restricted to only those structs with `DynLength`
- Remove calls to `assert_eq!` to help limit formatting code
- Fix bugs related to `Tx` `Capability`
  • Loading branch information
bradleyharden committed May 22, 2022
1 parent bd6eea2 commit 8139d7a
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 58 deletions.
69 changes: 42 additions & 27 deletions hal/src/sercom/spi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,6 @@ let (chan0, _, spi, _) = dma_transfer.wait();
"
)]

use core::convert::TryFrom;
use core::marker::PhantomData;

use bitflags::bitflags;
Expand Down Expand Up @@ -417,15 +416,13 @@ bitflags! {
}
}

/// Convert [`Status`] flags into the corresponding [`Error`] variants
impl TryFrom<Status> 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(())
Expand Down Expand Up @@ -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`]
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {}

Expand Down Expand Up @@ -1041,10 +1055,10 @@ where
/// [type class]: crate::typelevel#type-classes
pub trait AnyConfig: Is<Type = SpecificConfig<Self>> {
type Sercom: Sercom;
type Pads: ValidPads<Sercom = Self::Sercom>;
type Pads: ValidPads<Sercom = Self::Sercom, Capability = Self::Capability>;
type Capability: Capability;
type OpMode: OpMode;
type Size: Size;
type Size: Size<Word = Self::Word>;
type Word: 'static;
}

Expand Down Expand Up @@ -1304,19 +1318,17 @@ where
}

#[cfg(feature = "min-samd51g")]
impl<P, M, A> Spi<Config<P, M, DynLength>, A>
impl<C, A> Spi<C, A>
where
P: ValidPads,
M: OpMode,
Config<P, M, DynLength>: ValidConfig,
C: ValidConfig<Size = DynLength>,
A: Capability,
{
/// Return the current transaction length
///
/// 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
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -1358,7 +1370,14 @@ pub trait AnySpi: Is<Type = SpecificSpi<Self>> {
type OpMode: OpMode;
type Size: Size;
type Word: 'static;
type Config: ValidConfig<Sercom = Self::Sercom>;
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
Expand Down Expand Up @@ -1394,14 +1413,10 @@ where
{
}

impl<C, A> AnySpi for Spi<C, A>
where
C: ValidConfig,
A: Capability,
{
impl<C: ValidConfig> AnySpi for Spi<C, C::Capability> {
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;
Expand Down
24 changes: 15 additions & 9 deletions hal/src/sercom/spi/impl_ehal_thumbv7em.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -352,7 +354,7 @@ where
/// [`Transfer`]: blocking::spi::Transfer
impl<P, M, A> blocking::spi::Transfer<u8> for Spi<Config<P, M, DynLength>, A>
where
Config<P, M, DynLength>: ValidConfig,
Config<P, M, DynLength>: ValidConfig<Size = DynLength>,
P: ValidPads,
M: OpMode,
A: Receive,
Expand All @@ -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)
}
Expand Down Expand Up @@ -524,7 +528,7 @@ where
/// [`Write`]: blocking::spi::Write
impl<P, M> blocking::spi::Write<u8> for Spi<Config<P, M, DynLength>, Duplex>
where
Config<P, M, DynLength>: ValidConfig,
Config<P, M, DynLength>: ValidConfig<Size = DynLength>,
P: ValidPads,
M: OpMode,
{
Expand Down Expand Up @@ -553,7 +557,7 @@ where
/// [`Write`]: blocking::spi::Write
impl<P, M> blocking::spi::Write<u8> for Spi<Config<P, M, DynLength>, Tx>
where
Config<P, M, DynLength>: ValidConfig,
Config<P, M, DynLength>: ValidConfig<Size = DynLength>,
P: ValidPads,
M: OpMode,
{
Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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(())
}
2 changes: 1 addition & 1 deletion hal/src/sercom/spi/reg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ impl<S: Sercom> Registers<S> {
/// Try to read the interrupt flags, but first check the error status flags.
#[inline]
pub fn read_flags_errors(&self) -> Result<Flags, Error> {
self.read_status().try_into()?;
self.read_status().check_errors()?;
Ok(self.read_flags())
}
}
Loading

0 comments on commit 8139d7a

Please # to comment.