spi: SpiDevice transactiontake an operation slice instead of a closure.

embedded-hal-async/src/spi.rs

@@ -1,113 +1,85 @@
 //! SPI master mode traits.
 
-use core::{fmt::Debug, future::Future};
+use core::fmt::Debug;
 
 use embedded_hal::digital::OutputPin;
 use embedded_hal::spi as blocking;
 pub use embedded_hal::spi::{
-    Error, ErrorKind, ErrorType, Mode, Phase, Polarity, MODE_0, MODE_1, MODE_2, MODE_3,
+    Error, ErrorKind, ErrorType, Mode, Operation, Phase, Polarity, MODE_0, MODE_1, MODE_2, MODE_3,
 };
 
-#[macro_export]
+/// SPI read-only device trait
-/// Do an SPI transaction on a bus.
-/// This is a safe wrapper for [SpiDevice::transaction], which handles dereferencing the raw pointer for you.
 ///
-/// # Examples
+/// `SpiDeviceRead` represents ownership over a single SPI device on a (possibly shared) bus, selected
+/// with a CS (Chip Select) pin.
 ///
-/// ```
+/// See (the docs on embedded-hal)[embedded_hal::spi] for important information on SPI Bus vs Device traits.
-/// use embedded_hal_async::spi::{transaction, SpiBus, SpiBusRead, SpiBusWrite, SpiDevice};
 ///
-/// pub async fn transaction_example<SPI>(mut device: SPI) -> Result<u32, SPI::Error>
+/// See the [module-level documentation](self) for important usage information.
-/// where
+pub trait SpiDeviceRead<Word: Copy + 'static = u8>: ErrorType {
-///     SPI: SpiDevice,
+    /// Perform a read transaction against the device.
-///     SPI::Bus: SpiBus,
+    ///
-/// {
+    /// - Locks the bus
-///     transaction!(&mut device, move |bus| async move {
+    /// - Asserts the CS (Chip Select) pin.
-///         // Unlike `SpiDevice::transaction`, we don't need to
+    /// - Performs all the operations.
-///         // manually dereference a pointer in order to use the bus.
+    /// - [Flushes](SpiBusFlush::flush) the bus.
-///         bus.write(&[42]).await?;
+    /// - Deasserts the CS pin.
-///         let mut data = [0; 4];
+    /// - Unlocks the bus.
-///         bus.read(&mut data).await?;
+    ///
-///         Ok(u32::from_be_bytes(data))
+    /// The locking mechanism is implementation-defined. The only requirement is it must prevent two
-///     })
+    /// transactions from executing concurrently against the same bus. Examples of implementations are:
-///     .await
+    /// critical sections, blocking mutexes, returning an error or panicking if the bus is already busy.
-/// }
+    ///
-/// ```
+    /// On bus errors the implementation should try to deassert CS.
-///
+    /// If an error occurs while deasserting CS the bus error should take priority as the return value.
-/// Note that the compiler will prevent you from moving the bus reference outside of the closure
+    async fn read_transaction(&mut self, operations: &mut [&mut [Word]])
-/// ```compile_fail
+        -> Result<(), Self::Error>;
-/// # use embedded_hal_async::spi::{transaction, SpiBus, SpiBusRead, SpiBusWrite, SpiDevice};
+
-/// #
+    /// Do a read within a transaction.
-/// # pub async fn smuggle_test<SPI>(mut device: SPI) -> Result<(), SPI::Error>
+    ///
-/// # where
+    /// This is a convenience method equivalent to `device.read_transaction(&mut [buf])`.
-/// #     SPI: SpiDevice,
+    ///
-/// #     SPI::Bus: SpiBus,
+    /// See also: [`SpiDeviceRead::read_transaction`], [`SpiBusRead::read`]
-/// # {
+    async fn read(&mut self, buf: &mut [Word]) -> Result<(), Self::Error> {
-///     let mut bus_smuggler: Option<&mut SPI::Bus> = None;
+        self.read_transaction(&mut [buf]).await
-///     transaction!(&mut device, move |bus| async move {
+    }
-///         bus_smuggler = Some(bus);
-///         Ok(())
-///     })
-///     .await
-/// # }
-/// ```
-macro_rules! spi_transaction {
-    ($device:expr, move |$bus:ident| async move $closure_body:expr) => {
-        $crate::spi::SpiDevice::transaction($device, move |$bus| {
-            // Safety: Implementers of the `SpiDevice` trait guarantee that the pointer is
-            // valid and dereferencable for the entire duration of the closure.
-            let $bus = unsafe { &mut *$bus };
-            async move {
-                let result = $closure_body;
-                let $bus = $bus; // Ensure that the bus reference was not moved out of the closure
-                let _ = $bus; // Silence the "unused variable" warning from prevous line
-                result
-            }
-        })
-    };
-    ($device:expr, move |$bus:ident| async $closure_body:expr) => {
-        $crate::spi::SpiDevice::transaction($device, move |$bus| {
-            // Safety: Implementers of the `SpiDevice` trait guarantee that the pointer is
-            // valid and dereferencable for the entire duration of the closure.
-            let $bus = unsafe { &mut *$bus };
-            async {
-                let result = $closure_body;
-                let $bus = $bus; // Ensure that the bus reference was not moved out of the closure
-                let _ = $bus; // Silence the "unused variable" warning from prevous line
-                result
-            }
-        })
-    };
-    ($device:expr, |$bus:ident| async move $closure_body:expr) => {
-        $crate::spi::SpiDevice::transaction($device, |$bus| {
-            // Safety: Implementers of the `SpiDevice` trait guarantee that the pointer is
-            // valid and dereferencable for the entire duration of the closure.
-            let $bus = unsafe { &mut *$bus };
-            async move {
-                let result = $closure_body;
-                let $bus = $bus; // Ensure that the bus reference was not moved out of the closure
-                let _ = $bus; // Silence the "unused variable" warning from prevous line
-                result
-            }
-        })
-    };
-    ($device:expr, |$bus:ident| async $closure_body:expr) => {
-        $crate::spi::SpiDevice::transaction($device, |$bus| {
-            // Safety: Implementers of the `SpiDevice` trait guarantee that the pointer is
-            // valid and dereferencable for the entire duration of the closure.
-            let $bus = unsafe { &mut *$bus };
-            async {
-                let result = $closure_body;
-                let $bus = $bus; // Ensure that the bus reference was not moved out of the closure
-                let _ = $bus; // Silence the "unused variable" warning from prevous line
-                result
-            }
-        })
-    };
 }
 
-#[doc(inline)]
+/// SPI write-only device trait
-pub use spi_transaction as transaction;
+///
+/// `SpiDeviceWrite` represents ownership over a single SPI device on a (possibly shared) bus, selected
+/// with a CS (Chip Select) pin.
+///
+/// See (the docs on embedded-hal)[embedded_hal::spi] for important information on SPI Bus vs Device traits.
+///
+/// See the [module-level documentation](self) for important usage information.
+pub trait SpiDeviceWrite<Word: Copy + 'static = u8>: ErrorType {
+    /// Perform a write transaction against the device.
+    ///
+    /// - Locks the bus
+    /// - Asserts the CS (Chip Select) pin.
+    /// - Performs all the operations.
+    /// - [Flushes](SpiBusFlush::flush) the bus.
+    /// - Deasserts the CS pin.
+    /// - Unlocks the bus.
+    ///
+    /// The locking mechanism is implementation-defined. The only requirement is it must prevent two
+    /// transactions from executing concurrently against the same bus. Examples of implementations are:
+    /// critical sections, blocking mutexes, returning an error or panicking if the bus is already busy.
+    ///
+    /// On bus errors the implementation should try to deassert CS.
+    /// If an error occurs while deasserting CS the bus error should take priority as the return value.
+    async fn write_transaction(&mut self, operations: &[&[Word]]) -> Result<(), Self::Error>;
+
+    /// Do a write within a transaction.
+    ///
+    /// This is a convenience method equivalent to `device.write_transaction(&mut [buf])`.
+    ///
+    /// See also: [`SpiDeviceWrite::write_transaction`], [`SpiBusWrite::write`]
+    async fn write(&mut self, buf: &[Word]) -> Result<(), Self::Error> {
+        self.write_transaction(&[buf]).await
+    }
+}
 
 /// SPI device trait
 ///
@@ -116,90 +88,38 @@ pub use spi_transaction as transaction;
 ///
 /// See (the docs on embedded-hal)[embedded_hal::spi] for important information on SPI Bus vs Device traits.
 ///
-/// # Safety
+/// See the [module-level documentation](self) for important usage information.
-///
+pub trait SpiDevice<Word: Copy + 'static = u8>:
-/// See [`SpiDevice::transaction`] for details.
+    SpiDeviceRead<Word> + SpiDeviceWrite<Word> + ErrorType
-pub unsafe trait SpiDevice: ErrorType {
+{
-    /// SPI Bus type for this device.
-    type Bus: ErrorType;
-
     /// Perform a transaction against the device.
     ///
-    /// **NOTE:**
-    /// It is not recommended to use this method directly, because it requires `unsafe` code to dereference the raw pointer.
-    /// Instead, the [`transaction!`] macro should be used, which handles this safely inside the macro.
-    ///
     /// - Locks the bus
     /// - Asserts the CS (Chip Select) pin.
-    /// - Calls `f` with an exclusive reference to the bus, which can then be used to do transfers against the device.
+    /// - Performs all the operations.
     /// - [Flushes](SpiBusFlush::flush) the bus.
     /// - Deasserts the CS pin.
     /// - Unlocks the bus.
     ///
     /// The locking mechanism is implementation-defined. The only requirement is it must prevent two
     /// transactions from executing concurrently against the same bus. Examples of implementations are:
-    /// critical sections, blocking mutexes, async mutexes, returning an error or panicking if the bus is already busy.
+    /// critical sections, blocking mutexes, returning an error or panicking if the bus is already busy.
     ///
     /// On bus errors the implementation should try to deassert CS.
     /// If an error occurs while deasserting CS the bus error should take priority as the return value.
-    ///
+    async fn transaction(
-    /// # Safety
+        &mut self,
-    ///
+        operations: &mut [Operation<'_, Word>],
-    /// The current state of the Rust typechecker doesn't allow expressing the necessary lifetime constraints, so
+    ) -> Result<(), Self::Error>;
-    /// the `f` closure receives a lifetime-less `*mut Bus` raw pointer instead.
-    ///
-    /// Implementers of the `SpiDevice` trait must guarantee that the pointer is valid and dereferencable
-    /// for the entire duration of the closure.
-    async fn transaction<R, F, Fut>(&mut self, f: F) -> Result<R, Self::Error>
-    where
-        F: FnOnce(*mut Self::Bus) -> Fut,
-        Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>>;
-
-    /// Do a read within a transaction.
-    ///
-    /// This is a convenience method equivalent to `device.transaction(|bus| bus.read(buf))`.
-    ///
-    /// See also: [`SpiDevice::transaction`], [`SpiBusRead::read`]
-    async fn read<'a, Word>(&'a mut self, buf: &'a mut [Word]) -> Result<(), Self::Error>
-    where
-        Self::Bus: SpiBusRead<Word>,
-        Word: Copy + 'static,
-    {
-        transaction!(self, move |bus| async move { bus.read(buf).await }).await
-    }
-
-    /// Do a write within a transaction.
-    ///
-    /// This is a convenience method equivalent to `device.transaction(|bus| bus.write(buf))`.
-    ///
-    /// See also: [`SpiDevice::transaction`], [`SpiBusWrite::write`]
-    async fn write<'a, Word>(&'a mut self, buf: &'a [Word]) -> Result<(), Self::Error>
-    where
-        Self::Bus: SpiBusWrite<Word>,
-        Word: Copy + 'static,
-    {
-        transaction!(self, move |bus| async move { bus.write(buf).await }).await
-    }
 
     /// Do a transfer within a transaction.
     ///
     /// This is a convenience method equivalent to `device.transaction(|bus| bus.transfer(read, write))`.
     ///
     /// See also: [`SpiDevice::transaction`], [`SpiBus::transfer`]
-    async fn transfer<'a, Word>(
+    async fn transfer(&mut self, read: &mut [Word], write: &[Word]) -> Result<(), Self::Error> {
-        &'a mut self,
+        self.transaction(&mut [Operation::Transfer(read, write)])
-        read: &'a mut [Word],
+            .await
-        write: &'a [Word],
-    ) -> Result<(), Self::Error>
-    where
-        Self::Bus: SpiBus<Word>,
-        Word: Copy + 'static,
-    {
-        transaction!(
-            self,
-            move |bus| async move { bus.transfer(read, write).await }
-        )
-        .await
     }
 
     /// Do an in-place transfer within a transaction.
@@ -207,31 +127,49 @@ pub unsafe trait SpiDevice: ErrorType {
     /// This is a convenience method equivalent to `device.transaction(|bus| bus.transfer_in_place(buf))`.
     ///
     /// See also: [`SpiDevice::transaction`], [`SpiBus::transfer_in_place`]
-    async fn transfer_in_place<'a, Word>(
+    async fn transfer_in_place(&mut self, buf: &mut [Word]) -> Result<(), Self::Error> {
-        &'a mut self,
+        self.transaction(&mut [Operation::TransferInPlace(buf)])
-        buf: &'a mut [Word],
+            .await
-    ) -> Result<(), Self::Error>
-    where
-        Self::Bus: SpiBus<Word>,
-        Word: Copy + 'static,
-    {
-        transaction!(
-            self,
-            move |bus| async move { bus.transfer_in_place(buf).await }
-        )
-        .await
     }
 }
 
-unsafe impl<T: SpiDevice> SpiDevice for &mut T {
+impl<Word: Copy + 'static, T: SpiDeviceRead<Word>> SpiDeviceRead<Word> for &mut T {
-    type Bus = T::Bus;
+    async fn read_transaction(
+        &mut self,
+        operations: &mut [&mut [Word]],
+    ) -> Result<(), Self::Error> {
+        T::read_transaction(self, operations).await
+    }
 
-    async fn transaction<R, F, Fut>(&mut self, f: F) -> Result<R, Self::Error>
+    async fn read(&mut self, buf: &mut [Word]) -> Result<(), Self::Error> {
-    where
+        T::read(self, buf).await
-        F: FnOnce(*mut Self::Bus) -> Fut,
+    }
-        Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>>,
+}
-    {
+
-        T::transaction(self, f).await
+impl<Word: Copy + 'static, T: SpiDeviceWrite<Word>> SpiDeviceWrite<Word> for &mut T {
+    async fn write_transaction(&mut self, operations: &[&[Word]]) -> Result<(), Self::Error> {
+        T::write_transaction(self, operations).await
+    }
+
+    async fn write(&mut self, buf: &[Word]) -> Result<(), Self::Error> {
+        T::write(self, buf).await
+    }
+}
+
+impl<Word: Copy + 'static, T: SpiDevice<Word>> SpiDevice<Word> for &mut T {
+    async fn transaction(
+        &mut self,
+        operations: &mut [Operation<'_, Word>],
+    ) -> Result<(), Self::Error> {
+        T::transaction(self, operations).await
+    }
+
+    async fn transfer(&mut self, read: &mut [Word], write: &[Word]) -> Result<(), Self::Error> {
+        T::transfer(self, read, write).await
+    }
+
+    async fn transfer_in_place(&mut self, buf: &mut [Word]) -> Result<(), Self::Error> {
+        T::transfer_in_place(self, buf).await
     }
 }
 
@@ -374,57 +312,194 @@ where
     type Error = ExclusiveDeviceError<BUS::Error, CS::Error>;
 }
 
-impl<BUS, CS> blocking::SpiDevice for ExclusiveDevice<BUS, CS>
+impl<Word: Copy + 'static, BUS, CS> blocking::SpiDeviceRead<Word> for ExclusiveDevice<BUS, CS>
 where
-    BUS: blocking::SpiBusFlush,
+    BUS: blocking::SpiBusRead<Word>,
     CS: OutputPin,
 {
-    type Bus = BUS;
+    fn read_transaction(&mut self, operations: &mut [&mut [Word]]) -> Result<(), Self::Error> {
-
-    fn transaction<R>(
-        &mut self,
-        f: impl FnOnce(&mut Self::Bus) -> Result<R, <Self::Bus as ErrorType>::Error>,
-    ) -> Result<R, Self::Error> {
         self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
 
-        let f_res = f(&mut self.bus);
+        let mut op_res = Ok(());
+
+        for buf in operations {
+            if let Err(e) = self.bus.read(buf) {
+                op_res = Err(e);
+                break;
+            }
+        }
 
         // On failure, it's important to still flush and deassert CS.
         let flush_res = self.bus.flush();
         let cs_res = self.cs.set_high();
 
-        let f_res = f_res.map_err(ExclusiveDeviceError::Spi)?;
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
         flush_res.map_err(ExclusiveDeviceError::Spi)?;
         cs_res.map_err(ExclusiveDeviceError::Cs)?;
 
-        Ok(f_res)
+        Ok(())
     }
 }
 
-unsafe impl<BUS, CS> SpiDevice for ExclusiveDevice<BUS, CS>
+impl<Word: Copy + 'static, BUS, CS> blocking::SpiDeviceWrite<Word> for ExclusiveDevice<BUS, CS>
 where
-    BUS: SpiBusFlush,
+    BUS: blocking::SpiBusWrite<Word>,
     CS: OutputPin,
 {
-    type Bus = BUS;
+    fn write_transaction(&mut self, operations: &[&[Word]]) -> Result<(), Self::Error> {
-
-    async fn transaction<R, F, Fut>(&mut self, f: F) -> Result<R, Self::Error>
-    where
-        F: FnOnce(*mut Self::Bus) -> Fut,
-        Fut: Future<Output = Result<R, <Self::Bus as ErrorType>::Error>>,
-    {
         self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
 
-        let f_res = f(&mut self.bus).await;
+        let mut op_res = Ok(());
+
+        for buf in operations {
+            if let Err(e) = self.bus.write(buf) {
+                op_res = Err(e);
+                break;
+            }
+        }
+
+        // On failure, it's important to still flush and deassert CS.
+        let flush_res = self.bus.flush();
+        let cs_res = self.cs.set_high();
+
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
+        flush_res.map_err(ExclusiveDeviceError::Spi)?;
+        cs_res.map_err(ExclusiveDeviceError::Cs)?;
+
+        Ok(())
+    }
+}
+
+impl<Word: Copy + 'static, BUS, CS> blocking::SpiDevice<Word> for ExclusiveDevice<BUS, CS>
+where
+    BUS: blocking::SpiBus<Word>,
+    CS: OutputPin,
+{
+    fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error> {
+        self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
+
+        let op_res = 'ops: {
+            for op in operations {
+                let res = match op {
+                    Operation::Read(buf) => self.bus.read(buf),
+                    Operation::Write(buf) => self.bus.write(buf),
+                    Operation::Transfer(read, write) => self.bus.transfer(read, write),
+                    Operation::TransferInPlace(buf) => self.bus.transfer_in_place(buf),
+                };
+                if let Err(e) = res {
+                    break 'ops Err(e);
+                }
+            }
+            Ok(())
+        };
+
+        // On failure, it's important to still flush and deassert CS.
+        let flush_res = self.bus.flush();
+        let cs_res = self.cs.set_high();
+
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
+        flush_res.map_err(ExclusiveDeviceError::Spi)?;
+        cs_res.map_err(ExclusiveDeviceError::Cs)?;
+
+        Ok(())
+    }
+}
+
+impl<Word: Copy + 'static, BUS, CS> SpiDeviceRead<Word> for ExclusiveDevice<BUS, CS>
+where
+    BUS: SpiBusRead<Word>,
+    CS: OutputPin,
+{
+    async fn read_transaction(
+        &mut self,
+        operations: &mut [&mut [Word]],
+    ) -> Result<(), Self::Error> {
+        self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
+
+        let mut op_res = Ok(());
+
+        for buf in operations {
+            if let Err(e) = self.bus.read(buf).await {
+                op_res = Err(e);
+                break;
+            }
+        }
 
         // On failure, it's important to still flush and deassert CS.
         let flush_res = self.bus.flush().await;
         let cs_res = self.cs.set_high();
 
-        let f_res = f_res.map_err(ExclusiveDeviceError::Spi)?;
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
         flush_res.map_err(ExclusiveDeviceError::Spi)?;
         cs_res.map_err(ExclusiveDeviceError::Cs)?;
 
-        Ok(f_res)
+        Ok(())
+    }
+}
+
+impl<Word: Copy + 'static, BUS, CS> SpiDeviceWrite<Word> for ExclusiveDevice<BUS, CS>
+where
+    BUS: SpiBusWrite<Word>,
+    CS: OutputPin,
+{
+    async fn write_transaction(&mut self, operations: &[&[Word]]) -> Result<(), Self::Error> {
+        self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
+
+        let mut op_res = Ok(());
+
+        for buf in operations {
+            if let Err(e) = self.bus.write(buf).await {
+                op_res = Err(e);
+                break;
+            }
+        }
+
+        // On failure, it's important to still flush and deassert CS.
+        let flush_res = self.bus.flush().await;
+        let cs_res = self.cs.set_high();
+
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
+        flush_res.map_err(ExclusiveDeviceError::Spi)?;
+        cs_res.map_err(ExclusiveDeviceError::Cs)?;
+
+        Ok(())
+    }
+}
+
+impl<Word: Copy + 'static, BUS, CS> SpiDevice<Word> for ExclusiveDevice<BUS, CS>
+where
+    BUS: SpiBus<Word>,
+    CS: OutputPin,
+{
+    async fn transaction(
+        &mut self,
+        operations: &mut [Operation<'_, Word>],
+    ) -> Result<(), Self::Error> {
+        self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
+
+        let op_res = 'ops: {
+            for op in operations {
+                let res = match op {
+                    Operation::Read(buf) => self.bus.read(buf).await,
+                    Operation::Write(buf) => self.bus.write(buf).await,
+                    Operation::Transfer(read, write) => self.bus.transfer(read, write).await,
+                    Operation::TransferInPlace(buf) => self.bus.transfer_in_place(buf).await,
+                };
+                if let Err(e) = res {
+                    break 'ops Err(e);
+                }
+            }
+            Ok(())
+        };
+
+        // On failure, it's important to still flush and deassert CS.
+        let flush_res = self.bus.flush().await;
+        let cs_res = self.cs.set_high();
+
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
+        flush_res.map_err(ExclusiveDeviceError::Spi)?;
+        cs_res.map_err(ExclusiveDeviceError::Cs)?;
+
+        Ok(())
     }
 }

embedded-hal-bus/src/spi.rs

@@ -2,7 +2,10 @@
 
 use core::fmt::Debug;
 use embedded_hal::digital::OutputPin;
-use embedded_hal::spi::{Error, ErrorKind, ErrorType, SpiBusFlush, SpiDevice};
+use embedded_hal::spi::{
+    Error, ErrorKind, ErrorType, Operation, SpiBus, SpiBusRead, SpiBusWrite, SpiDevice,
+    SpiDeviceRead, SpiDeviceWrite,
+};
 
 /// Error type for [`ExclusiveDevice`] operations.
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
@@ -50,29 +53,111 @@ where
     type Error = ExclusiveDeviceError<BUS::Error, CS::Error>;
 }
 
-impl<BUS, CS> SpiDevice for ExclusiveDevice<BUS, CS>
+impl<Word: Copy + 'static, BUS, CS> SpiDeviceRead<Word> for ExclusiveDevice<BUS, CS>
 where
-    BUS: SpiBusFlush,
+    BUS: SpiBusRead<Word>,
     CS: OutputPin,
 {
-    type Bus = BUS;
+    fn read_transaction(&mut self, operations: &mut [&mut [Word]]) -> Result<(), Self::Error> {
-
-    fn transaction<R>(
-        &mut self,
-        f: impl FnOnce(&mut Self::Bus) -> Result<R, <Self::Bus as ErrorType>::Error>,
-    ) -> Result<R, Self::Error> {
         self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
 
-        let f_res = f(&mut self.bus);
+        let mut op_res = Ok(());
+
+        for buf in operations {
+            if let Err(e) = self.bus.read(buf) {
+                op_res = Err(e);
+                break;
+            }
+        }
 
         // On failure, it's important to still flush and deassert CS.
         let flush_res = self.bus.flush();
         let cs_res = self.cs.set_high();
 
-        let f_res = f_res.map_err(ExclusiveDeviceError::Spi)?;
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
         flush_res.map_err(ExclusiveDeviceError::Spi)?;
         cs_res.map_err(ExclusiveDeviceError::Cs)?;
 
-        Ok(f_res)
+        Ok(())
+    }
+}
+
+impl<Word: Copy + 'static, BUS, CS> SpiDeviceWrite<Word> for ExclusiveDevice<BUS, CS>
+where
+    BUS: SpiBusWrite<Word>,
+    CS: OutputPin,
+{
+    fn write_transaction(&mut self, operations: &[&[Word]]) -> Result<(), Self::Error> {
+        self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
+
+        let mut op_res = Ok(());
+
+        for buf in operations {
+            if let Err(e) = self.bus.write(buf) {
+                op_res = Err(e);
+                break;
+            }
+        }
+
+        // On failure, it's important to still flush and deassert CS.
+        let flush_res = self.bus.flush();
+        let cs_res = self.cs.set_high();
+
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
+        flush_res.map_err(ExclusiveDeviceError::Spi)?;
+        cs_res.map_err(ExclusiveDeviceError::Cs)?;
+
+        Ok(())
+    }
+}
+
+impl<Word: Copy + 'static, BUS, CS> SpiDevice<Word> for ExclusiveDevice<BUS, CS>
+where
+    BUS: SpiBus<Word>,
+    CS: OutputPin,
+{
+    fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error> {
+        self.cs.set_low().map_err(ExclusiveDeviceError::Cs)?;
+
+        let mut op_res = Ok(());
+
+        for op in operations {
+            match op {
+                Operation::Read(buf) => {
+                    if let Err(e) = self.bus.read(buf) {
+                        op_res = Err(e);
+                        break;
+                    }
+                }
+                Operation::Write(buf) => {
+                    if let Err(e) = self.bus.write(buf) {
+                        op_res = Err(e);
+                        break;
+                    }
+                }
+                Operation::Transfer(read, write) => {
+                    if let Err(e) = self.bus.transfer(read, write) {
+                        op_res = Err(e);
+                        break;
+                    }
+                }
+                Operation::TransferInPlace(buf) => {
+                    if let Err(e) = self.bus.transfer_in_place(buf) {
+                        op_res = Err(e);
+                        break;
+                    }
+                }
+            }
+        }
+
+        // On failure, it's important to still flush and deassert CS.
+        let flush_res = self.bus.flush();
+        let cs_res = self.cs.set_high();
+
+        op_res.map_err(ExclusiveDeviceError::Spi)?;
+        flush_res.map_err(ExclusiveDeviceError::Spi)?;
+        cs_res.map_err(ExclusiveDeviceError::Cs)?;
+
+        Ok(())
     }
 }

embedded-hal/src/spi.rs

@@ -46,20 +46,19 @@
 //! consists of asserting CS, then doing one or more transfers, then deasserting CS. For the entire duration of the transaction, the [`SpiDevice`]
 //! implementation will ensure no other transaction can be opened on the same bus. This is the key that allows correct sharing of the bus.
 //!
-//! The capabilities of the bus (read-write, read-only or write-only) are determined by which of the [`SpiBus`], [`SpiBusRead`] [`SpiBusWrite`] traits
+//! For read-only or write-only SPI devices, the [`SpiDeviceRead`] and [`SpiDeviceWrite`] are available.
-//! are implemented for the [`Bus`](SpiDevice::Bus) associated type.
 //!
 //! # For driver authors
 //!
 //! When implementing a driver, it's crucial to pick the right trait, to ensure correct operation
 //! with maximum interoperability. Here are some guidelines depending on the device you're implementing a driver for:
 //!
-//! If your device **has a CS pin**, use [`SpiDevice`]. Do not manually manage the CS pin, the [`SpiDevice`] implementation will do it for you.
+//! If your device **has a CS pin**, use [`SpiDevice`] (or [`SpiDeviceRead`]/[`SpiDeviceWrite`]). Do not manually
-//! Add bounds like `where T::Bus: SpiBus`, `where T::Bus: SpiBusRead`, `where T::Bus: SpiBusWrite` to specify the kind of access you need.
+//! manage the CS pin, the [`SpiDevice`] implementation will do it for you.
 //! By using [`SpiDevice`], your driver will cooperate nicely with other drivers for other devices in the same shared SPI bus.
 //!
 //! ```
-//! # use embedded_hal::spi::{SpiBus, SpiBusRead, SpiBusWrite, SpiDevice};
+//! # use embedded_hal::spi::{SpiBus, SpiBusRead, SpiBusWrite, SpiDevice, Operation};
 //! pub struct MyDriver<SPI> {
 //!     spi: SPI,
 //! }
@@ -67,7 +66,6 @@
 //! impl<SPI> MyDriver<SPI>
 //! where
 //!     SPI: SpiDevice,
-//!     SPI::Bus: SpiBus, // or SpiBusRead/SpiBusWrite if you only need to read or only write.
 //! {
 //!     pub fn new(spi: SPI) -> Self {
 //!         Self { spi }
@@ -77,10 +75,10 @@
 //!         let mut buf = [0; 2];
 //!
 //!         // `transaction` asserts and deasserts CS for us. No need to do it manually!
-//!         self.spi.transaction(|bus| {
+//!         self.spi.transaction(&mut [
-//!             bus.write(&[0x90])?;
+//!             Operation::Write(&[0x90]),
-//!             bus.read(&mut buf)
+//!             Operation::Read(&mut buf),
-//!         }).map_err(MyError::Spi)?;
+//!         ]).map_err(MyError::Spi)?;
 //!
 //!         Ok(buf)
 //!     }
@@ -298,21 +296,41 @@ impl<T: ErrorType> ErrorType for &mut T {
     type Error = T::Error;
 }
 
-/// SPI device trait
+/// SPI transaction operation.
 ///
-/// `SpiDevice` represents ownership over a single SPI device on a (possibly shared) bus, selected
+/// This allows composition of SPI operations into a single bus transaction
+#[derive(Debug, PartialEq)]
+pub enum Operation<'a, Word: 'static> {
+    /// Read data into the provided buffer.
+    ///
+    /// Equivalent to [`SpiBusRead::read`].
+    Read(&'a mut [Word]),
+    /// Write data from the provided buffer, discarding read data
+    ///
+    /// Equivalent to [`SpiBusWrite::write`].
+    Write(&'a [Word]),
+    /// Read data into the first buffer, while writing data from the second buffer.
+    ///
+    /// Equivalent to [`SpiBus::transfer`].
+    Transfer(&'a mut [Word], &'a [Word]),
+    /// Write data out while reading data into the provided buffer
+    ///
+    /// Equivalent to [`SpiBus::transfer_in_place`].
+    TransferInPlace(&'a mut [Word]),
+}
+
+/// SPI read-only device trait
+///
+/// `SpiDeviceRead` represents ownership over a single SPI device on a (possibly shared) bus, selected
 /// with a CS (Chip Select) pin.
 ///
 /// See the [module-level documentation](self) for important usage information.
-pub trait SpiDevice: ErrorType {
+pub trait SpiDeviceRead<Word: Copy + 'static = u8>: ErrorType {
-    /// SPI Bus type for this device.
+    /// Perform a read transaction against the device.
-    type Bus: ErrorType;
-
-    /// Perform a transaction against the device.
     ///
     /// - Locks the bus
     /// - Asserts the CS (Chip Select) pin.
-    /// - Calls `f` with an exclusive reference to the bus, which can then be used to do transfers against the device.
+    /// - Performs all the operations.
     /// - [Flushes](SpiBusFlush::flush) the bus.
     /// - Deasserts the CS pin.
     /// - Unlocks the bus.
@@ -323,71 +341,128 @@ pub trait SpiDevice: ErrorType {
     ///
     /// On bus errors the implementation should try to deassert CS.
     /// If an error occurs while deasserting CS the bus error should take priority as the return value.
-    fn transaction<R>(
+    fn read_transaction(&mut self, operations: &mut [&mut [Word]]) -> Result<(), Self::Error>;
-        &mut self,
-        f: impl FnOnce(&mut Self::Bus) -> Result<R, <Self::Bus as ErrorType>::Error>,
-    ) -> Result<R, Self::Error>;
-
-    /// Do a write within a transaction.
-    ///
-    /// This is a convenience method equivalent to `device.transaction(|bus| bus.write(buf))`.
-    ///
-    /// See also: [`SpiDevice::transaction`], [`SpiBusWrite::write`]
-    fn write<Word>(&mut self, buf: &[Word]) -> Result<(), Self::Error>
-    where
-        Self::Bus: SpiBusWrite<Word>,
-        Word: Copy,
-    {
-        self.transaction(|bus| bus.write(buf))
-    }
 
     /// Do a read within a transaction.
     ///
-    /// This is a convenience method equivalent to `device.transaction(|bus| bus.read(buf))`.
+    /// This is a convenience method equivalent to `device.read_transaction(&mut [buf])`.
     ///
-    /// See also: [`SpiDevice::transaction`], [`SpiBusRead::read`]
+    /// See also: [`SpiDeviceRead::read_transaction`], [`SpiBusRead::read`]
-    fn read<Word>(&mut self, buf: &mut [Word]) -> Result<(), Self::Error>
+    fn read(&mut self, buf: &mut [Word]) -> Result<(), Self::Error> {
-    where
+        self.read_transaction(&mut [buf])
-        Self::Bus: SpiBusRead<Word>,
-        Word: Copy,
-    {
-        self.transaction(|bus| bus.read(buf))
     }
+}
+
+/// SPI write-only device trait
+///
+/// `SpiDeviceWrite` represents ownership over a single SPI device on a (possibly shared) bus, selected
+/// with a CS (Chip Select) pin.
+///
+/// See the [module-level documentation](self) for important usage information.
+pub trait SpiDeviceWrite<Word: Copy + 'static = u8>: ErrorType {
+    /// Perform a write transaction against the device.
+    ///
+    /// - Locks the bus
+    /// - Asserts the CS (Chip Select) pin.
+    /// - Performs all the operations.
+    /// - [Flushes](SpiBusFlush::flush) the bus.
+    /// - Deasserts the CS pin.
+    /// - Unlocks the bus.
+    ///
+    /// The locking mechanism is implementation-defined. The only requirement is it must prevent two
+    /// transactions from executing concurrently against the same bus. Examples of implementations are:
+    /// critical sections, blocking mutexes, returning an error or panicking if the bus is already busy.
+    ///
+    /// On bus errors the implementation should try to deassert CS.
+    /// If an error occurs while deasserting CS the bus error should take priority as the return value.
+    fn write_transaction(&mut self, operations: &[&[Word]]) -> Result<(), Self::Error>;
+
+    /// Do a write within a transaction.
+    ///
+    /// This is a convenience method equivalent to `device.write_transaction(&mut [buf])`.
+    ///
+    /// See also: [`SpiDeviceWrite::write_transaction`], [`SpiBusWrite::write`]
+    fn write(&mut self, buf: &[Word]) -> Result<(), Self::Error> {
+        self.write_transaction(&[buf])
+    }
+}
+
+/// SPI device trait
+///
+/// `SpiDevice` represents ownership over a single SPI device on a (possibly shared) bus, selected
+/// with a CS (Chip Select) pin.
+///
+/// See the [module-level documentation](self) for important usage information.
+pub trait SpiDevice<Word: Copy + 'static = u8>:
+    SpiDeviceRead<Word> + SpiDeviceWrite<Word> + ErrorType
+{
+    /// Perform a transaction against the device.
+    ///
+    /// - Locks the bus
+    /// - Asserts the CS (Chip Select) pin.
+    /// - Performs all the operations.
+    /// - [Flushes](SpiBusFlush::flush) the bus.
+    /// - Deasserts the CS pin.
+    /// - Unlocks the bus.
+    ///
+    /// The locking mechanism is implementation-defined. The only requirement is it must prevent two
+    /// transactions from executing concurrently against the same bus. Examples of implementations are:
+    /// critical sections, blocking mutexes, returning an error or panicking if the bus is already busy.
+    ///
+    /// On bus errors the implementation should try to deassert CS.
+    /// If an error occurs while deasserting CS the bus error should take priority as the return value.
+    fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error>;
 
     /// Do a transfer within a transaction.
     ///
-    /// This is a convenience method equivalent to `device.transaction(|bus| bus.transfer(read, write))`.
+    /// This is a convenience method equivalent to `device.transaction(&mut [Operation::Transfer(read, write)]`.
     ///
     /// See also: [`SpiDevice::transaction`], [`SpiBus::transfer`]
-    fn transfer<Word>(&mut self, read: &mut [Word], write: &[Word]) -> Result<(), Self::Error>
+    fn transfer(&mut self, read: &mut [Word], write: &[Word]) -> Result<(), Self::Error> {
-    where
+        self.transaction(&mut [Operation::Transfer(read, write)])
-        Self::Bus: SpiBus<Word>,
-        Word: Copy,
-    {
-        self.transaction(|bus| bus.transfer(read, write))
     }
 
     /// Do an in-place transfer within a transaction.
     ///
-    /// This is a convenience method equivalent to `device.transaction(|bus| bus.transfer_in_place(buf))`.
+    /// This is a convenience method equivalent to `device.transaction([Operation::TransferInPlace(buf)]`.
     ///
     /// See also: [`SpiDevice::transaction`], [`SpiBus::transfer_in_place`]
-    fn transfer_in_place<Word>(&mut self, buf: &mut [Word]) -> Result<(), Self::Error>
+    fn transfer_in_place(&mut self, buf: &mut [Word]) -> Result<(), Self::Error> {
-    where
+        self.transaction(&mut [Operation::TransferInPlace(buf)])
-        Self::Bus: SpiBus<Word>,
-        Word: Copy,
-    {
-        self.transaction(|bus| bus.transfer_in_place(buf))
     }
 }
 
-impl<T: SpiDevice> SpiDevice for &mut T {
+impl<Word: Copy + 'static, T: SpiDeviceRead<Word>> SpiDeviceRead<Word> for &mut T {
-    type Bus = T::Bus;
+    fn read_transaction(&mut self, operations: &mut [&mut [Word]]) -> Result<(), Self::Error> {
-    fn transaction<R>(
+        T::read_transaction(self, operations)
-        &mut self,
+    }
-        f: impl FnOnce(&mut Self::Bus) -> Result<R, <Self::Bus as ErrorType>::Error>,
+
-    ) -> Result<R, Self::Error> {
+    fn read(&mut self, buf: &mut [Word]) -> Result<(), Self::Error> {
-        T::transaction(self, f)
+        T::read(self, buf)
+    }
+}
+
+impl<Word: Copy + 'static, T: SpiDeviceWrite<Word>> SpiDeviceWrite<Word> for &mut T {
+    fn write_transaction(&mut self, operations: &[&[Word]]) -> Result<(), Self::Error> {
+        T::write_transaction(self, operations)
+    }
+
+    fn write(&mut self, buf: &[Word]) -> Result<(), Self::Error> {
+        T::write(self, buf)
+    }
+}
+
+impl<Word: Copy + 'static, T: SpiDevice<Word>> SpiDevice<Word> for &mut T {
+    fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error> {
+        T::transaction(self, operations)
+    }
+
+    fn transfer(&mut self, read: &mut [Word], write: &[Word]) -> Result<(), Self::Error> {
+        T::transfer(self, read, write)
+    }
+
+    fn transfer_in_place(&mut self, buf: &mut [Word]) -> Result<(), Self::Error> {
+        T::transfer_in_place(self, buf)
     }
 }
 
@@ -406,7 +481,7 @@ impl<T: SpiBusFlush> SpiBusFlush for &mut T {
 }
 
 /// Read-only SPI bus
-pub trait SpiBusRead<Word: Copy = u8>: SpiBusFlush {
+pub trait SpiBusRead<Word: Copy + 'static = u8>: SpiBusFlush {
     /// Read `words` from the slave.
     ///
     /// The word value sent on MOSI during reading is implementation-defined,
@@ -417,14 +492,14 @@ pub trait SpiBusRead<Word: Copy = u8>: SpiBusFlush {
     fn read(&mut self, words: &mut [Word]) -> Result<(), Self::Error>;
 }
 
-impl<T: SpiBusRead<Word>, Word: Copy> SpiBusRead<Word> for &mut T {
+impl<T: SpiBusRead<Word>, Word: Copy + 'static> SpiBusRead<Word> for &mut T {
     fn read(&mut self, words: &mut [Word]) -> Result<(), Self::Error> {
         T::read(self, words)
     }
 }
 
 /// Write-only SPI bus
-pub trait SpiBusWrite<Word: Copy = u8>: SpiBusFlush {
+pub trait SpiBusWrite<Word: Copy + 'static = u8>: SpiBusFlush {
     /// Write `words` to the slave, ignoring all the incoming words
     ///
     /// Implementations are allowed to return before the operation is
@@ -432,7 +507,7 @@ pub trait SpiBusWrite<Word: Copy = u8>: SpiBusFlush {
     fn write(&mut self, words: &[Word]) -> Result<(), Self::Error>;
 }
 
-impl<T: SpiBusWrite<Word>, Word: Copy> SpiBusWrite<Word> for &mut T {
+impl<T: SpiBusWrite<Word>, Word: Copy + 'static> SpiBusWrite<Word> for &mut T {
     fn write(&mut self, words: &[Word]) -> Result<(), Self::Error> {
         T::write(self, words)
     }
@@ -443,7 +518,7 @@ impl<T: SpiBusWrite<Word>, Word: Copy> SpiBusWrite<Word> for &mut T {
 /// `SpiBus` represents **exclusive ownership** over the whole SPI bus, with SCK, MOSI and MISO pins.
 ///
 /// See the [module-level documentation](self) for important information on SPI Bus vs Device traits.
-pub trait SpiBus<Word: Copy = u8>: SpiBusRead<Word> + SpiBusWrite<Word> {
+pub trait SpiBus<Word: Copy + 'static = u8>: SpiBusRead<Word> + SpiBusWrite<Word> {
     /// Write and read simultaneously. `write` is written to the slave on MOSI and
     /// words received on MISO are stored in `read`.
     ///
@@ -466,7 +541,7 @@ pub trait SpiBus<Word: Copy = u8>: SpiBusRead<Word> + SpiBusWrite<Word> {
     fn transfer_in_place(&mut self, words: &mut [Word]) -> Result<(), Self::Error>;
 }
 
-impl<T: SpiBus<Word>, Word: Copy> SpiBus<Word> for &mut T {
+impl<T: SpiBus<Word>, Word: Copy + 'static> SpiBus<Word> for &mut T {
     fn transfer(&mut self, read: &mut [Word], write: &[Word]) -> Result<(), Self::Error> {
         T::transfer(self, read, write)
     }