Refactor SPI-M & add build-time checks

examples/headsail-bsp/Cargo.toml

@@ -44,6 +44,7 @@ good_memory_allocator = { version = "0.1.7", optional = true }
 bit_field = "0.10.2"
 headsail-sysctrl-pac = { git = "https://github.com/soc-hub-fi/headsail-pac", version = "0.1.1", optional = true }
 headsail-hpc-pac = { git = "https://github.com/soc-hub-fi/headsail-pac", version = "0.1.1", optional = true }
+embedded-io = "0.6.1"
 
 [[example]]
 name = "panic"

examples/headsail-bsp/src/lib.rs

@@ -15,6 +15,11 @@ pub mod sysctrl;
 #[cfg(feature = "hpc")]
 pub use hpc::*;
 
+#[cfg(all(feature = "hpc", feature = "sysctrl"))]
+compile_error!(
+    "CPU-specific features \"hpc\" and feature \"sysctrl\" cannot be enabled at the same time. Select the one that matches the current target CPU."
+);
+
 #[cfg(feature = "alloc")]
 pub mod alloc;
 #[cfg(feature = "alloc")]
@@ -50,6 +55,11 @@ pub mod sprintln;
 mod ufmt_panic;
 pub use ufmt;
 
+#[cfg(all(feature = "panic-apb-uart0", feature = "panic-sysctrl-uart"))]
+compile_error!(
+    "Features \"panic-apb-uart0\" and feature \"panic-sysctrl-uart\" cannot be enabled at the same time. Only one panic implementation must exist at a time."
+);
+
 pub mod apb_uart;
 pub mod mmap;
 mod mmio;

examples/headsail-bsp/src/sysctrl/udma/spim.rs

@@ -2,6 +2,7 @@ use core::marker::PhantomData;
 
 use super::{Disabled, Enabled};
 use crate::pac;
+use embedded_io::ErrorType;
 
 pub const SPI_CMD_SOT: u32 = 0x10000000;
 pub const SPI_CMD_EOT: u32 = 0x90000000;
@@ -20,7 +21,8 @@ pub struct UdmaSpim<'u, UdmaPeriphState>(
 );
 
 impl<'u> UdmaSpim<'u, Disabled> {
-    #[inline]
+    /// Enables the uDMA clock gate for SPI-M
+    #[inline(always)]
     pub fn enable(self) -> UdmaSpim<'u, Enabled> {
         let spim = &self.0;
 
@@ -32,7 +34,7 @@ impl<'u> UdmaSpim<'u, Disabled> {
 }
 
 impl<'u> UdmaSpim<'u, Enabled> {
-    #[inline]
+    #[inline(always)]
     pub fn disable(self) -> UdmaSpim<'u, Disabled> {
         self.0.ctrl_cfg_cg().modify(|_r, w| w.cg_spim().clear_bit());
         UdmaSpim::<Disabled>(self.0, PhantomData)
@@ -41,13 +43,15 @@ impl<'u> UdmaSpim<'u, Enabled> {
     /// # Safety
     ///
     /// This will not configure the SPI-M in any way.
-    #[inline]
+    #[inline(always)]
     pub unsafe fn steal(udma: &'static pac::sysctrl::Udma) -> Self {
         Self(udma, PhantomData)
     }
 
-    #[inline]
+    #[inline(always)]
     pub fn write_tx(&mut self, buf: &[u8]) {
+        while !self.can_enqueue_tx() {}
+
         // SAFETY: we spin lock on spim_tx_saddr to make sure the transfer is complete before
         // dropping the stack frame.
         unsafe { self.enqueue_tx(buf) };
@@ -58,17 +62,32 @@ impl<'u> UdmaSpim<'u, Enabled> {
         while spim.spim_tx_saddr().read().bits() != 0 {}
     }
 
-    pub fn read_rx(&mut self, buf: &mut [u8]) {
+    /// A potential `impl<'u> embedded_io::Read for UdmaSpim<'u, Enabled> {}`
+    #[deprecated = "UdmaSpim::read method is experimental and untested. If you see this method
+                    does what it's supposed to do useful, remove this deprecation notice from the \
+                    method definition."]
+    #[inline(always)]
+    pub fn read_rx(&mut self, buf: &mut [u8]) -> Result<usize, SpimError> {
+        while !self.can_enqueue_rx() {}
+
         // SAFETY: we spin lock on spim_rx_saddr to make sure the transfer is complete before
         // dropping the stack frame.
         unsafe { self.enqueue_rx(buf) };
 
         // Poll until finished (prevents `buf` leakage)
         let spim = &self.0;
         while spim.spim_rx_saddr().read().bits() != 0 {}
+
+        // TODO: is this a guarantee that we always read exactly `buf.len()`? Can we somehow
+        // identify how many bytes were actually read. This does sound reasonable if we're always
+        // doing block reads like we might do with an SD card.
+        Ok(buf.len())
     }
 
+    #[inline(always)]
     pub fn write_cmd(&mut self, buf: &[u8]) {
+        while !self.can_enqueue_cmd() {}
+
         // SAFETY: we spin lock on spim_cmd_saddr to make sure the transfer is complete before
         // dropping the stack frame.
         unsafe { self.enqueue_cmd(buf) };
@@ -79,19 +98,22 @@ impl<'u> UdmaSpim<'u, Enabled> {
     }
 
     /// Send 'Start Of Transmission' (SOT) command
-    pub fn write_sot(&mut self) {
+    #[inline(always)]
+    pub fn send_sot(&mut self) {
         let sot_cmd: [u8; 4] = SPI_CMD_SOT.to_ne_bytes();
         self.write_cmd(&sot_cmd);
     }
 
     /// Send 'End Of Transmission' (EOT) command
-    pub fn write_eot(&mut self) {
+    #[inline(always)]
+    pub fn send_eot(&mut self) {
         let eot_cmd: [u8; 4] = (SPI_CMD_EOT).to_ne_bytes();
         self.write_cmd(&eot_cmd);
     }
 
     /// This function sends EOT (End Of Transmission) command but keeps the cs asserted.
-    pub fn write_eot_keep_cs(&mut self) {
+    #[inline(always)]
+    pub fn send_eot_keep_cs(&mut self) {
         let eot_cmd: [u8; 4] = (SPI_CMD_EOT | 0x03).to_ne_bytes();
         self.write_cmd(&eot_cmd);
     }
@@ -109,6 +131,7 @@ impl<'u> UdmaSpim<'u, Enabled> {
     ///     pim.send_dummy();
     /// }
     /// ```
+    #[inline(always)]
     pub fn write_dummy(&mut self) {
         let mut buffer: [u8; 4] = [0; 4];
         let cmd_cmd: [u8; 4] = (SPI_CMD_SEND_CMD_BASE | 0xFF).to_ne_bytes();
@@ -128,6 +151,7 @@ impl<'u> UdmaSpim<'u, Enabled> {
     ///   spim.eot();
     ///
     /// ```
+    #[inline(always)]
     pub fn send_data(&mut self, data: &[u8]) {
         let mut cmd_data: [u8; 12] = [0; 12];
 
@@ -156,7 +180,8 @@ impl<'u> UdmaSpim<'u, Enabled> {
     ///   spim.eot();
     ///
     /// ```
-    pub fn receive_data(&mut self, data: &mut [u8]) {
+    #[inline(always)]
+    pub fn receive_data(&mut self, data: &mut [u8]) -> Result<usize, SpimError> {
         let mut cmd_data: [u8; 12] = [0; 12];
 
         cmd_data[0..4].copy_from_slice(
@@ -169,13 +194,42 @@ impl<'u> UdmaSpim<'u, Enabled> {
         );
 
         self.write_cmd(&cmd_data);
-        self.read_rx(data);
+
+        self.read_rx(data)
+    }
+
+    /// Can a new transfer be enqueued to the CMD channel?
+    ///
+    /// Returns 1 if another transfer can be enqueued, 0 otherwise
+    #[inline(always)]
+    fn can_enqueue_cmd(&self) -> bool {
+        let spim = &self.0;
+        spim.spim_cmd_cfg().read().pending().bit_is_clear()
+    }
+
+    /// Can a new transfer be enqueued to the TX channel?
+    ///
+    /// Returns 1 if another transfer can be enqueued, 0 otherwise
+    #[inline(always)]
+    fn can_enqueue_tx(&self) -> bool {
+        let spim = &self.0;
+        spim.spim_tx_cfg().read().pending().bit_is_clear()
+    }
+
+    /// Can a new transfer be enqueued to the RX channel?
+    ///
+    /// Returns 1 if another transfer can be enqueued, 0 otherwise
+    #[inline(always)]
+    fn can_enqueue_rx(&self) -> bool {
+        let spim = &self.0;
+        spim.spim_rx_cfg().read().pending().bit_is_clear()
     }
 
     /// # Safety
     ///
     /// `buf` must outlive the transfer. Call `while spim.spim_*_saddr().read().bits() != 0 {}` or
     /// use an interrupt to determine when `buf` is safe to free.
+    #[inline(always)]
     unsafe fn enqueue_cmd(&mut self, buf: &[u8]) {
         let spim = &self.0;
 
@@ -193,6 +247,7 @@ impl<'u> UdmaSpim<'u, Enabled> {
     ///
     /// `buf` must outlive the transfer. Call `while spim.spim_*_saddr().read().bits() != 0 {}` or
     /// use an interrupt to determine when `buf` is safe to free.
+    #[inline(always)]
     unsafe fn enqueue_tx(&mut self, buf: &[u8]) {
         let spim = &self.0;
 
@@ -210,7 +265,8 @@ impl<'u> UdmaSpim<'u, Enabled> {
     ///
     /// `buf` must outlive the transfer. Call `while spim.spim_*_saddr().read().bits() != 0 {}` or
     /// use an interrupt to determine when `buf` is safe to free.
-    unsafe fn enqueue_rx(&mut self, buf: &[u8]) {
+    #[inline(always)]
+    unsafe fn enqueue_rx(&mut self, buf: &mut [u8]) {
         let spim = &self.0;
 
         // Write buffer location & len
@@ -223,3 +279,15 @@ impl<'u> UdmaSpim<'u, Enabled> {
         spim.spim_rx_cfg().modify(|_, w| w.en().set_bit());
     }
 }
+
+#[derive(Debug)]
+pub struct SpimError;
+impl embedded_io::Error for SpimError {
+    fn kind(&self) -> embedded_io::ErrorKind {
+        todo!()
+    }
+}
+
+impl<'u> ErrorType for UdmaSpim<'u, Enabled> {
+    type Error = SpimError;
+}

examples/headsail-bsp/src/sysctrl/udma/uart.rs

@@ -12,6 +12,8 @@ pub struct UdmaUart<'u, UdmaPeriphState>(
 type UartSetupW = pac::sysctrl::udma::uart_setup::W;
 
 impl<'u> UdmaUart<'u, Disabled> {
+    /// Enables the uDMA clock gate for UART and sets up the peripheral with the provided register
+    /// writer.
     #[inline]
     pub fn enable<F>(self, setup_spec: F) -> UdmaUart<'u, Enabled>
     where

examples/sysctrl/hello-sysctrl/examples/spim0.rs

@@ -34,11 +34,11 @@ fn main() -> ! {
     let tx_data: [u8; 8] = [0x01, 0x42, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08];
     let mut rx_data: [u8; 8] = [0; 8];
 
-    spim.write_sot();
+    spim.send_sot();
 
     // Send 8 bytes
     spim.send_data(&tx_data);
-    spim.write_eot_keep_cs();
+    spim.send_eot_keep_cs();
     sprintln!("Data sent!\n\r");
 
     for _ in 0..10_000 {
@@ -47,7 +47,7 @@ fn main() -> ! {
 
     // Receive 8 bytes
     spim.receive_data(&mut rx_data);
-    spim.write_eot();
+    spim.send_eot();
     sprintln!("Data received!\n\r");
 
     loop {