Merge #2566

2566: Make SPI return buffers upon error r=hudson-ayers a=alexandruradovici

### Pull Request Overview

This pull request changes the SPI HIL so that function return the buffers to the caller in case of errors. It also adds deferred callbacks to the SPI Mux, similar to what the I2C Mux.

I have two questions that need some feedback:
1. Several drivers now output a warning due to unverified result. The two simple solutions are: use `let _ =` or use `.unwrap()`. I'm not sure which one would be best.
2. The setup functions, like `configure` or `set_rate` do have to return results,  as the Mux might fail to execute them. There is no callback function defined for this. Should I add a new callback function, something like `command_done` or simply remove the return from the functions?

### Testing Strategy

The PR still needs testing and review.

### TODO or Help Wanted

Testing on different boards would be very helpful. I can test on STM32F4 and Adafruit CLUE.

### Documentation Updated

- [ ] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [ ] Ran `make prepush`.


Co-authored-by: Alexandru Radovici <msg4alex@gmail.com>
Co-authored-by: Philip Levis <pal@cs.stanford.edu>

boards/clue_nrf52840/src/main.rs

@@ -306,7 +306,7 @@ pub unsafe fn main() {
     //--------------------------------------------------------------------------
 
     let dynamic_deferred_call_clients =
-        static_init!([DynamicDeferredCallClientState; 4], Default::default());
+        static_init!([DynamicDeferredCallClientState; 5], Default::default());
     let dynamic_deferred_caller = static_init!(
         DynamicDeferredCall,
         DynamicDeferredCall::new(dynamic_deferred_call_clients)
@@ -493,8 +493,9 @@ pub unsafe fn main() {
     // TFT
     //--------------------------------------------------------------------------
 
-    let spi_mux = components::spi::SpiMuxComponent::new(&base_peripherals.spim0)
-        .finalize(components::spi_mux_component_helper!(nrf52840::spi::SPIM));
+    let spi_mux =
+        components::spi::SpiMuxComponent::new(&base_peripherals.spim0, dynamic_deferred_caller)
+            .finalize(components::spi_mux_component_helper!(nrf52840::spi::SPIM));
 
     base_peripherals.spim0.configure(
         nrf52840::pinmux::Pinmux::new(ST7789H2_MOSI as u32),

boards/components/src/bus.rs

@@ -35,6 +35,7 @@ use core::mem::MaybeUninit;
 use kernel::component::Component;
 use kernel::hil::bus8080;
 use kernel::hil::spi;
+use kernel::hil::spi::SpiMasterDevice;
 use kernel::static_init_half;
 
 // Setup static space for the objects.
@@ -127,6 +128,7 @@ impl<S: 'static + spi::SpiMaster> Component for SpiMasterBusComponent<S> {
             SpiMasterBus<'static, VirtualSpiMasterDevice<'static, S>>,
             SpiMasterBus::new(static_buffer.0, static_buffer.2)
         );
+        static_buffer.0.setup();
         static_buffer.0.set_client(bus);
 
         bus

boards/components/src/l3gd20.rs

@@ -23,6 +23,7 @@ use core::mem::MaybeUninit;
 use kernel::capabilities;
 use kernel::component::Component;
 use kernel::hil::spi;
+use kernel::hil::spi::SpiMasterDevice;
 use kernel::{create_capability, static_init_half};
 
 // Setup static space for the objects.
@@ -77,8 +78,11 @@ impl<S: 'static + spi::SpiMaster> Component for L3gd20SpiComponent<S> {
                 grant
             )
         );
+        static_buffer.0.setup();
         static_buffer.0.set_client(l3gd20);
-        l3gd20.configure();
+
+        // TODO verify SPI return value
+        let _ = l3gd20.configure();
 
         l3gd20
     }

boards/components/src/mx25r6435f.rs

@@ -22,6 +22,7 @@ use capsules::virtual_spi::{MuxSpiMaster, VirtualSpiMasterDevice};
 use core::mem::MaybeUninit;
 use kernel::component::Component;
 use kernel::hil;
+use kernel::hil::spi::SpiMasterDevice;
 use kernel::hil::time::Alarm;
 use kernel::static_init_half;
 
@@ -132,6 +133,7 @@ impl<
                 Some(self.hold_pin)
             )
         );
+        mx25r6435f_spi.setup();
         mx25r6435f_spi.set_client(mx25r6435f);
         mx25r6435f_virtual_alarm.set_alarm_client(mx25r6435f);
         mx25r6435f

boards/components/src/spi.rs

@@ -31,10 +31,13 @@ use core::mem::MaybeUninit;
 
 use capsules::spi_controller::{Spi, DEFAULT_READ_BUF_LENGTH, DEFAULT_WRITE_BUF_LENGTH};
 use capsules::spi_peripheral::SpiPeripheral;
-use capsules::virtual_spi::{MuxSpiMaster, SpiSlaveDevice, VirtualSpiMasterDevice};
+use capsules::virtual_spi;
+use capsules::virtual_spi::{MuxSpiMaster, VirtualSpiMasterDevice};
 use kernel::capabilities;
 use kernel::component::Component;
+use kernel::dynamic_deferred_call::DynamicDeferredCall;
 use kernel::hil::spi;
+use kernel::hil::spi::{SpiMasterDevice, SpiSlaveDevice};
 use kernel::{create_capability, static_init, static_init_half};
 
 // Setup static space for the objects.
@@ -96,6 +99,7 @@ macro_rules! spi_peripheral_component_helper {
 
 pub struct SpiMuxComponent<S: 'static + spi::SpiMaster> {
     spi: &'static S,
+    deferred_caller: &'static DynamicDeferredCall,
 }
 
 pub struct SpiSyscallComponent<S: 'static + spi::SpiMaster> {
@@ -117,8 +121,11 @@ pub struct SpiComponent<S: 'static + spi::SpiMaster> {
 }
 
 impl<S: 'static + spi::SpiMaster> SpiMuxComponent<S> {
-    pub fn new(spi: &'static S) -> Self {
-        SpiMuxComponent { spi: spi }
+    pub fn new(spi: &'static S, deferred_caller: &'static DynamicDeferredCall) -> Self {
+        SpiMuxComponent {
+            spi: spi,
+            deferred_caller: deferred_caller,
+        }
     }
 }
 
@@ -130,11 +137,20 @@ impl<S: 'static + spi::SpiMaster> Component for SpiMuxComponent<S> {
         let mux_spi = static_init_half!(
             static_buffer,
             MuxSpiMaster<'static, S>,
-            MuxSpiMaster::new(self.spi)
+            MuxSpiMaster::new(self.spi, self.deferred_caller)
+        );
+
+        mux_spi.initialize_callback_handle(
+            self.deferred_caller
+                .register(mux_spi)
+                .expect("no deferred call slot available for SPI mux"),
         );
 
         self.spi.set_client(mux_spi);
-        self.spi.init();
+
+        if let Err(error) = self.spi.init() {
+            panic!("SPI init failed ({:?})", error);
+        }
 
         mux_spi
     }
@@ -190,8 +206,8 @@ impl<S: 'static + spi::SpiMaster> Component for SpiSyscallComponent<S> {
         );
 
         spi_syscalls.config_buffers(spi_read_buf, spi_write_buf);
+        syscall_spi_device.setup();
         syscall_spi_device.set_client(spi_syscalls);
-
         spi_syscalls
     }
 }
@@ -212,23 +228,23 @@ impl<S: 'static + spi::SpiSlave> SpiSyscallPComponent<S> {
 
 impl<S: 'static + spi::SpiSlave> Component for SpiSyscallPComponent<S> {
     type StaticInput = (
-        &'static mut MaybeUninit<SpiSlaveDevice<'static, S>>,
-        &'static mut MaybeUninit<SpiPeripheral<'static, SpiSlaveDevice<'static, S>>>,
+        &'static mut MaybeUninit<virtual_spi::SpiSlaveDevice<'static, S>>,
+        &'static mut MaybeUninit<SpiPeripheral<'static, virtual_spi::SpiSlaveDevice<'static, S>>>,
     );
-    type Output = &'static SpiPeripheral<'static, SpiSlaveDevice<'static, S>>;
+    type Output = &'static SpiPeripheral<'static, virtual_spi::SpiSlaveDevice<'static, S>>;
 
     unsafe fn finalize(self, static_buffer: Self::StaticInput) -> Self::Output {
         let grant_cap = create_capability!(capabilities::MemoryAllocationCapability);
 
         let syscallp_spi_device = static_init_half!(
             static_buffer.0,
-            SpiSlaveDevice<'static, S>,
-            SpiSlaveDevice::new(self.spi_slave)
+            virtual_spi::SpiSlaveDevice<'static, S>,
+            virtual_spi::SpiSlaveDevice::new(self.spi_slave)
         );
 
         let spi_syscallsp = static_init_half!(
             static_buffer.1,
-            SpiPeripheral<'static, SpiSlaveDevice<'static, S>>,
+            SpiPeripheral<'static, virtual_spi::SpiSlaveDevice<'static, S>>,
             SpiPeripheral::new(
                 syscallp_spi_device,
                 self.board_kernel.create_grant(self.driver_num, &grant_cap)
@@ -269,7 +285,7 @@ impl<S: 'static + spi::SpiMaster> Component for SpiComponent<S> {
             VirtualSpiMasterDevice<'static, S>,
             VirtualSpiMasterDevice::new(self.spi_mux, self.chip_select)
         );
-
+        spi_device.setup();
         spi_device
     }
 }

boards/hail/src/main.rs

@@ -247,7 +247,7 @@ pub unsafe fn main() {
     );
 
     let dynamic_deferred_call_clients =
-        static_init!([DynamicDeferredCallClientState; 2], Default::default());
+        static_init!([DynamicDeferredCallClientState; 3], Default::default());
     let dynamic_deferred_caller = static_init!(
         DynamicDeferredCall,
         DynamicDeferredCall::new(dynamic_deferred_call_clients)
@@ -355,7 +355,7 @@ pub unsafe fn main() {
 
     // SPI
     // Set up a SPI MUX, so there can be multiple clients.
-    let mux_spi = components::spi::SpiMuxComponent::new(&peripherals.spi)
+    let mux_spi = components::spi::SpiMuxComponent::new(&peripherals.spi, dynamic_deferred_caller)
         .finalize(components::spi_mux_component_helper!(sam4l::spi::SpiHw));
     // Create the SPI system call capsule.
     let spi_syscalls = components::spi::SpiSyscallComponent::new(

boards/imix/src/imix_components/rf233.rs

@@ -19,6 +19,7 @@ use capsules::rf233::RF233;
 use capsules::virtual_spi::VirtualSpiMasterDevice;
 use kernel::component::Component;
 use kernel::hil;
+use kernel::hil::spi::SpiMasterDevice;
 use kernel::static_init;
 
 pub struct RF233Component {

boards/imix/src/main.rs

@@ -335,7 +335,7 @@ pub unsafe fn main() {
     let board_kernel = static_init!(kernel::Kernel, kernel::Kernel::new(&PROCESSES));
 
     let dynamic_deferred_call_clients =
-        static_init!([DynamicDeferredCallClientState; 4], Default::default());
+        static_init!([DynamicDeferredCallClientState; 5], Default::default());
     let dynamic_deferred_caller = static_init!(
         DynamicDeferredCall,
         DynamicDeferredCall::new(dynamic_deferred_call_clients)
@@ -403,7 +403,7 @@ pub unsafe fn main() {
     .finalize(());
 
     // SPI MUX, SPI syscall driver and RF233 radio
-    let mux_spi = components::spi::SpiMuxComponent::new(&peripherals.spi)
+    let mux_spi = components::spi::SpiMuxComponent::new(&peripherals.spi, dynamic_deferred_caller)
         .finalize(components::spi_mux_component_helper!(sam4l::spi::SpiHw));
 
     let spi_syscalls = SpiSyscallComponent::new(

boards/imix/src/test/mod.rs

@@ -7,6 +7,7 @@ pub(crate) mod linear_log_test;
 pub(crate) mod log_test;
 pub(crate) mod rng_test;
 pub(crate) mod spi_dummy;
+pub(crate) mod spi_loopback;
 pub(crate) mod udp_lowpan_test;
 pub(crate) mod virtual_aes_ccm_test;
 pub(crate) mod virtual_uart_rx_test;

boards/imix/src/test/spi_dummy.rs

@@ -2,6 +2,7 @@
 
 use kernel::hil::gpio::Configure;
 use kernel::hil::spi::{self, SpiMaster};
+use kernel::ErrorCode;
 
 #[allow(unused_variables, dead_code)]
 pub struct DummyCB {
@@ -30,9 +31,11 @@ impl spi::SpiMasterClient for DummyCB {
         write: &'static mut [u8],
         read: Option<&'static mut [u8]>,
         len: usize,
+        _status: Result<(), ErrorCode>,
     ) {
         unsafe {
             // do actual stuff
+            // TODO verify SPI return value
             let _ = self.spi.read_write_bytes(&mut A5, None, A5.len());
 
             // FLOP = !FLOP;
@@ -74,13 +77,14 @@ pub unsafe fn spi_dummy_test(spi: &'static sam4l::spi::SpiHw) {
     let spicb = kernel::static_init!(DummyCB, DummyCB::new(spi));
     spi.set_active_peripheral(sam4l::spi::Peripheral::Peripheral0);
     spi.set_client(spicb);
-    spi.init();
+
+    spi.init().unwrap();
     spi.set_baud_rate(200000);
 
     let len = BUF2.len();
     if spi.read_write_bytes(&mut BUF2, Some(&mut BUF1), len) != Ok(()) {
         loop {
-            spi.write_byte(0xA5);
+            spi.write_byte(0xA5).unwrap();
         }
     }