Merge #34

34: Use volatile reads and writes for rx and tx buffers r=richardeoin a=Abestanis

Hi, I'm using this library to drive FDCAN on an Stm32H7 microprocessor and I was able to send CAN messages with the correct header and id, but all data bytes were transmitted as zero bytes.

Looking into it, it appears that the ready to transmit bit (`txbar`) is set before the bytes are actually written to the transmit buffer.
There was already a todo at the transmit buffer hinting at this problem, and indeed using volatile writes fixes the problem for me.

I decided to also wrap the rx buffer in a volatile container, just to be safe, even though that receiving seems to also work without it.

I used the `volatile-register` crate, because it is also used by the `cortex-m` crate.

Co-authored-by: Sebastian Scholz <[email protected]>

Cargo.toml

@@ -28,6 +28,7 @@ paste = "1.0"
 vcell = "0.1.3"
 nb = "1.0.0"
 static_assertions = "1.1"
+volatile-register = "0.2.1"
 
 [dependencies.embedded-can-03]
 version = "0.3"

src/lib.rs

@@ -1331,19 +1331,20 @@ where
         tx_element.header.merge(tx_header);
 
         let mut lbuffer = [0_u32; 16];
-
-        let mut data = unsafe {
+        let data = unsafe {
             slice::from_raw_parts_mut(
                 lbuffer.as_mut_ptr() as *mut u8,
                 tx_header.len as usize,
             )
         };
         data[..tx_header.len as usize]
             .copy_from_slice(&buffer[..tx_header.len as usize]);
-
         let data_len = ((tx_header.len as usize) + 3) / 4;
-
-        tx_element.data[..data_len].copy_from_slice(&lbuffer[..data_len]);
+        for (register, byte) in
+            tx_element.data.iter_mut().zip(lbuffer[..data_len].iter())
+        {
+            unsafe { register.write(*byte) };
+        }
 
         // Set <idx as Mailbox> as ready to transmit
         self.registers()
@@ -1361,7 +1362,13 @@ where
 
             //read back header section
             let header = (&tx_ram.tbsa[idx as usize].header).into();
-            Some(pending(idx, header, &tx_ram.tbsa[idx as usize].data))
+            let mut data = [0u32; 16];
+            for (byte, register) in
+                data.iter_mut().zip(tx_ram.tbsa[idx as usize].data.iter())
+            {
+                *byte = register.read();
+            }
+            Some(pending(idx, header, &data))
         } else {
             // Abort request failed because the frame was already sent (or being sent) on
             // the bus. All mailboxes are now free. This can happen for small prescaler
@@ -1525,13 +1532,19 @@ where
             let mailbox: &RxFifoElement = &self.rx_msg_ram().fxsa[idx];
 
             let header: RxFrameInfo = (&mailbox.header).into();
-            let data = unsafe {
-                slice::from_raw_parts(
-                    mailbox.data.as_ptr() as *const u8,
-                    header.len as usize,
-                )
-            };
-            buffer[..header.len as usize].copy_from_slice(data);
+            for (i, register) in mailbox.data.iter().enumerate() {
+                let register_value = register.read();
+                let register_bytes = unsafe {
+                    slice::from_raw_parts(&register_value as *const u32 as *const u8, 4)
+                };
+                let num_bytes = (header.len as usize) - i * 4;
+                if num_bytes <= 4 {
+                    buffer[i * 4..i * 4 + num_bytes]
+                        .copy_from_slice(&register_bytes[..num_bytes]);
+                    break;
+                }
+                buffer[i * 4..(i + 1) * 4].copy_from_slice(register_bytes);
+            }
             self.release_mailbox(mbox);
 
             if self.has_overrun() {

src/message_ram.rs

@@ -1,3 +1,5 @@
+use volatile_register::RW;
+
 pub(crate) mod common;
 pub(crate) mod enums;
 pub(crate) mod generic;
@@ -100,12 +102,14 @@ pub(crate) mod rxfifo_element;
 #[repr(C)]
 pub(crate) struct RxFifoElement {
     pub(crate) header: RxFifoElementHeader,
-    pub(crate) data: [u32; 16], // TODO: Does this need to be volatile?
+    pub(crate) data: [RW<u32>; 16],
 }
 impl RxFifoElement {
     pub(crate) fn reset(&mut self) {
         self.header.reset();
-        self.data = [0; 16];
+        for byte in self.data.iter_mut() {
+            unsafe { byte.write(0) };
+        }
     }
 }
 pub(crate) type RxFifoElementHeaderType = [u32; 2];
@@ -119,12 +123,14 @@ pub(crate) mod txbuffer_element;
 #[repr(C)]
 pub(crate) struct TxBufferElement {
     pub(crate) header: TxBufferElementHeader,
-    pub(crate) data: [u32; 16], // TODO: Does this need to be volatile?
+    pub(crate) data: [RW<u32>; 16],
 }
 impl TxBufferElement {
     pub(crate) fn reset(&mut self) {
         self.header.reset();
-        self.data = [0; 16];
+        for byte in self.data.iter_mut() {
+            unsafe { byte.write(0) };
+        }
     }
 }
 pub(crate) type TxBufferElementHeader =