Merge pull request #17 from thalesfragoso/fences

Add fences

Author: thalesfragoso

Cargo.toml

@@ -22,13 +22,15 @@ volatile-register = "0.2"
 aligned = "0.3"
 stm32f7xx-hal = {version = "0.2.0", optional = true}
 stm32f4xx-hal = {version = "0.8.3", optional = true}
-cortex-m = "0.6.0"
+cortex-m = "0.6.2"
 
 smoltcp = { version = "0.6.0", default-features = false, features = ["proto-ipv4", "proto-ipv6", "socket-icmp", "socket-udp", "socket-tcp", "ethernet"], optional = true }
 log = { version = "0.4", optional = true }
 
 [features]
 device-selected = []
+fence = []
+
 stm32f407 = ["stm32f4xx-hal/stm32f407", "device-selected"]
 stm32f417 = ["stm32f4xx-hal/stm32f417", "device-selected"]
 stm32f427 = ["stm32f4xx-hal/stm32f427", "device-selected"]
@@ -38,15 +40,15 @@ stm32f439 = ["stm32f4xx-hal/stm32f439", "device-selected"]
 stm32f469 = ["stm32f4xx-hal/stm32f469", "device-selected"]
 stm32f479 = ["stm32f4xx-hal/stm32f479", "device-selected"]
 
-stm32f745 = ["stm32f7xx-hal/stm32f745", "device-selected"]
-stm32f746 = ["stm32f7xx-hal/stm32f746", "device-selected"]
-stm32f756 = ["stm32f7xx-hal/stm32f756", "device-selected"]
-stm32f765 = ["stm32f7xx-hal/stm32f765", "device-selected"]
-stm32f767 = ["stm32f7xx-hal/stm32f767", "device-selected"]
-stm32f769 = ["stm32f7xx-hal/stm32f769", "device-selected"]
-stm32f777 = ["stm32f7xx-hal/stm32f777", "device-selected"]
-stm32f778 = ["stm32f7xx-hal/stm32f778", "device-selected"]
-stm32f779 = ["stm32f7xx-hal/stm32f779", "device-selected"]
+stm32f745 = ["stm32f7xx-hal/stm32f745", "device-selected", "fence"]
+stm32f746 = ["stm32f7xx-hal/stm32f746", "device-selected", "fence"]
+stm32f756 = ["stm32f7xx-hal/stm32f756", "device-selected", "fence"]
+stm32f765 = ["stm32f7xx-hal/stm32f765", "device-selected", "fence"]
+stm32f767 = ["stm32f7xx-hal/stm32f767", "device-selected", "fence"]
+stm32f769 = ["stm32f7xx-hal/stm32f769", "device-selected", "fence"]
+stm32f777 = ["stm32f7xx-hal/stm32f777", "device-selected", "fence"]
+stm32f778 = ["stm32f7xx-hal/stm32f778", "device-selected", "fence"]
+stm32f779 = ["stm32f7xx-hal/stm32f779", "device-selected", "fence"]
 
 smoltcp-phy = ["smoltcp"]
 smoltcp-log = ["log", "smoltcp/log"]

src/lib.rs

@@ -17,8 +17,6 @@ pub use stm32f4xx_hal::stm32;
 use hal::rcc::Clocks;
 use stm32::{Interrupt, ETHERNET_DMA, ETHERNET_MAC, NVIC};
 
-use cortex_m::asm;
-
 pub mod phy;
 use phy::{Phy, PhyStatus};
 mod ring;
@@ -88,7 +86,7 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
     ///
     /// Make sure that the buffers reside in a memory region that is
     /// accessible by the peripheral. Core-Coupled Memory (CCM) is
-    /// usually not accessible. HCLK must be between 25MHz and 168MHz for STM32F4xx 
+    /// usually not accessible. HCLK must be between 25MHz and 168MHz for STM32F4xx
     /// or 25MHz to 216MHz for STM32F7xx.
     ///
     /// Uses an interrupt free critical section to turn on the ethernet clock for STM32F7xx.
@@ -314,8 +312,6 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
         f: F,
     ) -> Result<R, TxError> {
         let result = self.tx_ring.send(length, f);
-        //Make sure the memory write occurs before triggering DMA
-        asm::dmb();
         self.tx_ring.demand_poll(&self.eth_dma);
         result
     }

src/rx.rs

@@ -5,8 +5,11 @@ use stm32f7xx_hal::device as stm32;
 
 use stm32::ETHERNET_DMA;
 
-use core::default::Default;
-use core::ops::{Deref, DerefMut};
+use core::{
+    default::Default,
+    ops::{Deref, DerefMut},
+    sync::atomic::{self, Ordering},
+};
 
 use crate::{
     desc::Descriptor,
@@ -63,9 +66,19 @@ impl RxDescriptor {
 
     /// Pass ownership to the DMA engine
     fn set_owned(&mut self) {
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "fence")]
+        atomic::fence(Ordering::Release);
+
+        atomic::compiler_fence(Ordering::Release);
         unsafe {
             self.desc.modify(0, |w| w | RXDESC_0_OWN);
         }
+
+        // Used to flush the store buffer as fast as possible to make the buffer available for the
+        // DMA.
+        #[cfg(feature = "fence")]
+        atomic::fence(Ordering::SeqCst);
     }
 
     fn has_error(&self) -> bool {
@@ -136,6 +149,9 @@ impl RxRingEntry {
         } else if self.desc().is_first() && self.desc().is_last() {
             let frame_len = self.desc().get_frame_len();
 
+            // "Subsequent reads and writes cannot be moved ahead of preceding reads."
+            atomic::compiler_fence(Ordering::Acquire);
+
             // TODO: obtain ethernet frame type (RDESC_1_FT)
             let pkt = RxPacket {
                 entry: self,
@@ -216,6 +232,8 @@ impl<'a> RxRing<'a> {
         // Register RxDescriptor
         eth_dma.dmardlar.write(|w| w.srl().bits(ring_ptr as u32));
 
+        // We already have fences in `set_owned`, which is called in `setup`
+
         // Start receive
         eth_dma.dmaomr.modify(|_, w| w.sr().set_bit());
 

src/tx.rs

@@ -5,7 +5,10 @@ use stm32f7xx_hal::device as stm32;
 
 use stm32::ETHERNET_DMA;
 
-use core::ops::{Deref, DerefMut};
+use core::{
+    ops::{Deref, DerefMut},
+    sync::atomic::{self, Ordering},
+};
 
 use crate::{
     desc::Descriptor,
@@ -60,9 +63,19 @@ impl TxDescriptor {
 
     /// Pass ownership to the DMA engine
     fn set_owned(&mut self) {
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "fence")]
+        atomic::fence(Ordering::Release);
+
+        atomic::compiler_fence(Ordering::Release);
         unsafe {
             self.desc.modify(0, |w| w | TXDESC_0_OWN);
         }
+
+        // Used to flush the store buffer as fast as possible to make the buffer available for the
+        // DMA.
+        #[cfg(feature = "fence")]
+        atomic::fence(Ordering::SeqCst);
     }
 
     #[allow(unused)]
@@ -193,6 +206,13 @@ impl<'a> TxRing<'a> {
         // Register TxDescriptor
         eth_dma.dmatdlar.write(|w| w.stl().bits(ring_ptr as u32));
 
+        // "Preceding reads and writes cannot be moved past subsequent writes."
+        #[cfg(feature = "fence")]
+        atomic::fence(Ordering::Release);
+
+        // We don't need a compiler fence here because all interactions with `Descriptor` are
+        // volatiles
+
         // Start transmission
         eth_dma.dmaomr.modify(|_, w| w.st().set_bit());
     }