Merge #247

247: Make ethernet::DesRing generic over the number of buffers r=richardeoin a=richardeoin

This allows users to decide how many transmit and receive buffers they wish to have the the `ethernet::DesRing`. Typically this allows a trade-off in performance vs memory usage.

The number of buffers need to be annotated on both `ethernet::DesRing` and `ethernet::EthernetDMA`. Requiring it on both is a slight usability downside. However this is small compared to the previous solution, which was making a local copy of the HAL in order to edit a constant value in `src/ethernet/eth.rs`.

Co-authored-by: Richard Meadows <962920+richardeoin@users.noreply.github.com>

Author: bors[bot]

CHANGELOG.md

@@ -13,6 +13,10 @@
 * Rename the PeripheralREC object for BDMA2 on 7B3, 7B0, 7A3 parts from BDMA to BDMA2
 * pac: Upgrade to stm32-rs v0.14.0
 
+* ethernet: `ethernet::DesRing` and `ethernet::EthernetDMA` require generic
+  constants to specify how many transmit / receive buffers to include in
+  `ethernet::DesRing`. To replicate the previous behaviour, use `DesRing<4, 4>`
+
 ## [v0.10.0] 2021-07-xx
 
 * **Breaking**: Don't reset peripheral in DMA1/2 `StreamsTuple::new()` method #229

examples/ethernet-nucleo-h743zi2.rs

@@ -43,7 +43,7 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];
 
 /// Ethernet descriptor rings are a global singleton
 #[link_section = ".sram3.eth"]
-static mut DES_RING: ethernet::DesRing = ethernet::DesRing::new();
+static mut DES_RING: ethernet::DesRing<4, 4> = ethernet::DesRing::new();
 
 // the program entry point
 #[entry]

examples/ethernet-rtic-stm32h735g-dk.rs

@@ -56,7 +56,7 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];
 
 /// Ethernet descriptor rings are a global singleton
 #[link_section = ".axisram.eth"]
-static mut DES_RING: ethernet::DesRing = ethernet::DesRing::new();
+static mut DES_RING: ethernet::DesRing<4, 4> = ethernet::DesRing::new();
 
 /// Net storage with static initialisation - another global singleton
 pub struct NetStorageStatic<'a> {
@@ -74,13 +74,13 @@ static mut STORE: NetStorageStatic = NetStorageStatic {
 };
 
 pub struct Net<'a> {
-    iface: EthernetInterface<'a, ethernet::EthernetDMA<'a>>,
+    iface: EthernetInterface<'a, ethernet::EthernetDMA<'a, 4, 4>>,
     sockets: SocketSet<'a>,
 }
 impl<'a> Net<'a> {
     pub fn new(
         store: &'static mut NetStorageStatic<'a>,
-        ethdev: ethernet::EthernetDMA<'a>,
+        ethdev: ethernet::EthernetDMA<'a, 4, 4>,
         ethernet_addr: EthernetAddress,
     ) -> Self {
         // Set IP address

examples/ethernet-rtic-stm32h747i-disco.rs

@@ -56,7 +56,7 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];
 
 /// Ethernet descriptor rings are a global singleton
 #[link_section = ".sram3.eth"]
-static mut DES_RING: ethernet::DesRing = ethernet::DesRing::new();
+static mut DES_RING: ethernet::DesRing<4, 4> = ethernet::DesRing::new();
 
 /// Net storage with static initialisation - another global singleton
 pub struct NetStorageStatic<'a> {
@@ -74,13 +74,13 @@ static mut STORE: NetStorageStatic = NetStorageStatic {
 };
 
 pub struct Net<'a> {
-    iface: EthernetInterface<'a, ethernet::EthernetDMA<'a>>,
+    iface: EthernetInterface<'a, ethernet::EthernetDMA<'a, 4, 4>>,
     sockets: SocketSet<'a>,
 }
 impl<'a> Net<'a> {
     pub fn new(
         store: &'static mut NetStorageStatic<'a>,
-        ethdev: ethernet::EthernetDMA<'a>,
+        ethdev: ethernet::EthernetDMA<'a, 4, 4>,
         ethernet_addr: EthernetAddress,
     ) -> Self {
         // Set IP address

examples/ethernet-stm32h747i-disco.rs

@@ -28,7 +28,7 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];
 
 /// Ethernet descriptor rings are a global singleton
 #[link_section = ".sram3.eth"]
-static mut DES_RING: ethernet::DesRing = ethernet::DesRing::new();
+static mut DES_RING: ethernet::DesRing<4, 4> = ethernet::DesRing::new();
 
 // the program entry point
 #[entry]

src/ethernet/eth.rs

@@ -39,8 +39,6 @@ use crate::ethernet::StationManagement;
 // 6 DMAC, 6 SMAC, 4 q tag, 2 ethernet type II, 1500 ip MTU, 4 CRC, 2
 // padding
 const ETH_BUF_SIZE: usize = 1536;
-const ETH_NUM_TD: usize = 4;
-const ETH_NUM_RD: usize = 4;
 
 /// Transmit and Receive Descriptor fields
 #[allow(dead_code)]
@@ -95,22 +93,22 @@ impl TDes {
 
 /// Store a ring of TDes and associated buffers
 #[repr(C, packed)]
-struct TDesRing {
-    td: [TDes; ETH_NUM_TD],
-    tbuf: [[u32; ETH_BUF_SIZE / 4]; ETH_NUM_TD],
+struct TDesRing<const TD: usize> {
+    td: [TDes; TD],
+    tbuf: [[u32; ETH_BUF_SIZE / 4]; TD],
     tdidx: usize,
 }
 
-impl TDesRing {
+impl<const TD: usize> TDesRing<TD> {
     const fn new() -> Self {
         Self {
             td: [TDes {
                 tdes0: 0,
                 tdes1: 0,
                 tdes2: 0,
                 tdes3: 0,
-            }; ETH_NUM_TD],
-            tbuf: [[0; ETH_BUF_SIZE / 4]; ETH_NUM_TD],
+            }; TD],
+            tbuf: [[0; ETH_BUF_SIZE / 4]; TD],
             tdidx: 0,
         }
     }
@@ -121,8 +119,8 @@ impl TDesRing {
     /// will be stored in the descriptors, so ensure the TDesRing is
     /// not moved after initialisation.
     pub fn init(&mut self) {
-        for td in self.td.iter_mut() {
-            td.init();
+        for x in 0..TD {
+            self.td[x].init();
         }
         self.tdidx = 0;
 
@@ -131,9 +129,8 @@ impl TDesRing {
         unsafe {
             let dma = &*stm32::ETHERNET_DMA::ptr();
             dma.dmactx_dlar
-                .write(|w| w.bits(&self.td as *const _ as u32));
-            dma.dmactx_rlr
-                .write(|w| w.tdrl().bits(self.td.len() as u16 - 1));
+                .write(|w| w.bits(&self.td[0] as *const _ as u32));
+            dma.dmactx_rlr.write(|w| w.tdrl().bits(TD as u16 - 1));
             dma.dmactx_dtpr
                 .write(|w| w.bits(&self.td[0] as *const _ as u32));
         }
@@ -166,7 +163,7 @@ impl TDesRing {
         cortex_m::asm::dsb();
 
         // Move the tail pointer (TPR) to the next descriptor
-        let x = (x + 1) % ETH_NUM_TD;
+        let x = (x + 1) % TD;
         unsafe {
             let dma = &*stm32::ETHERNET_DMA::ptr();
             dma.dmactx_dtpr
@@ -241,22 +238,22 @@ impl RDes {
 
 /// Store a ring of RDes and associated buffers
 #[repr(C, packed)]
-struct RDesRing {
-    rd: [RDes; ETH_NUM_RD],
-    rbuf: [[u32; ETH_BUF_SIZE / 4]; ETH_NUM_RD],
+struct RDesRing<const RD: usize> {
+    rd: [RDes; RD],
+    rbuf: [[u32; ETH_BUF_SIZE / 4]; RD],
     rdidx: usize,
 }
 
-impl RDesRing {
+impl<const RD: usize> RDesRing<RD> {
     const fn new() -> Self {
         Self {
             rd: [RDes {
                 rdes0: 0,
                 rdes1: 0,
                 rdes2: 0,
                 rdes3: 0,
-            }; ETH_NUM_RD],
-            rbuf: [[0; ETH_BUF_SIZE / 4]; ETH_NUM_RD],
+            }; RD],
+            rbuf: [[0; ETH_BUF_SIZE / 4]; RD],
             rdidx: 0,
         }
     }
@@ -267,18 +264,17 @@ impl RDesRing {
     /// will be stored in the descriptors, so ensure the RDesRing is
     /// not moved after initialisation.
     pub fn init(&mut self) {
-        for rd in self.rd.iter_mut() {
-            rd.init();
+        for x in 0..RD {
+            self.rd[x].init();
         }
         self.rdidx = 0;
 
         // Initialise pointers in the DMA engine
         unsafe {
             let dma = &*stm32::ETHERNET_DMA::ptr();
             dma.dmacrx_dlar
-                .write(|w| w.bits(&self.rd as *const _ as u32));
-            dma.dmacrx_rlr
-                .write(|w| w.rdrl().bits(self.rd.len() as u16 - 1));
+                .write(|w| w.bits(&self.rd[0] as *const _ as u32));
+            dma.dmacrx_rlr.write(|w| w.rdrl().bits(RD as u16 - 1));
         }
 
         // Release descriptors to the DMA engine
@@ -325,7 +321,7 @@ impl RDesRing {
         }
 
         // Update active descriptor
-        self.rdidx = (x + 1) % ETH_NUM_RD;
+        self.rdidx = (x + 1) % RD;
     }
 
     /// Access the buffer pointed to by the next RDes
@@ -347,24 +343,29 @@ impl RDesRing {
     }
 }
 
-pub struct DesRing {
-    tx: TDesRing,
-    rx: RDesRing,
+pub struct DesRing<const TD: usize, const RD: usize> {
+    tx: TDesRing<TD>,
+    rx: RDesRing<RD>,
 }
-impl DesRing {
-    pub const fn new() -> DesRing {
+impl<const TD: usize, const RD: usize> DesRing<TD, RD> {
+    pub const fn new() -> Self {
         DesRing {
             tx: TDesRing::new(),
             rx: RDesRing::new(),
         }
     }
 }
+impl<const TD: usize, const RD: usize> Default for DesRing<TD, RD> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
 
 ///
 /// Ethernet DMA
 ///
-pub struct EthernetDMA<'a> {
-    ring: &'a mut DesRing,
+pub struct EthernetDMA<'a, const TD: usize, const RD: usize> {
+    ring: &'a mut DesRing<TD, RD>,
     eth_dma: stm32::ETHERNET_DMA,
 }
 
@@ -393,15 +394,15 @@ pub struct EthernetMAC {
 /// # Safety
 ///
 /// `EthernetDMA` shall not be moved as it is initialised here
-pub unsafe fn new_unchecked<'a>(
+pub unsafe fn new_unchecked<'a, const TD: usize, const RD: usize>(
     eth_mac: stm32::ETHERNET_MAC,
     eth_mtl: stm32::ETHERNET_MTL,
     eth_dma: stm32::ETHERNET_DMA,
-    ring: &'a mut DesRing,
+    ring: &'a mut DesRing<TD, RD>,
     mac_addr: EthernetAddress,
     prec: rec::Eth1Mac,
     clocks: &CoreClocks,
-) -> (EthernetDMA<'a>, EthernetMAC) {
+) -> (EthernetDMA<'a, TD, RD>, EthernetMAC) {
     // RCC
     {
         let rcc = &*stm32::RCC::ptr();
@@ -713,9 +714,9 @@ impl StationManagement for EthernetMAC {
 }
 
 /// Define TxToken type and implement consume method
-pub struct TxToken<'a>(&'a mut TDesRing);
+pub struct TxToken<'a, const TD: usize>(&'a mut TDesRing<TD>);
 
-impl<'a> phy::TxToken for TxToken<'a> {
+impl<'a, const TD: usize> phy::TxToken for TxToken<'a, TD> {
     fn consume<R, F>(
         self,
         _timestamp: Instant,
@@ -734,9 +735,9 @@ impl<'a> phy::TxToken for TxToken<'a> {
 }
 
 /// Define RxToken type and implement consume method
-pub struct RxToken<'a>(&'a mut RDesRing);
+pub struct RxToken<'a, const RD: usize>(&'a mut RDesRing<RD>);
 
-impl<'a> phy::RxToken for RxToken<'a> {
+impl<'a, const RD: usize> phy::RxToken for RxToken<'a, RD> {
     fn consume<R, F>(self, _timestamp: Instant, f: F) -> smoltcp::Result<R>
     where
         F: FnOnce(&mut [u8]) -> smoltcp::Result<R>,
@@ -748,9 +749,11 @@ impl<'a> phy::RxToken for RxToken<'a> {
 }
 
 /// Implement the smoltcp Device interface
-impl<'a> phy::Device<'a> for EthernetDMA<'_> {
-    type RxToken = RxToken<'a>;
-    type TxToken = TxToken<'a>;
+impl<'a, const TD: usize, const RD: usize> phy::Device<'a>
+    for EthernetDMA<'_, TD, RD>
+{
+    type RxToken = RxToken<'a, RD>;
+    type TxToken = TxToken<'a, TD>;
 
     // Clippy false positive because DeviceCapabilities is non-exhaustive
     #[allow(clippy::field_reassign_with_default)]
@@ -759,11 +762,11 @@ impl<'a> phy::Device<'a> for EthernetDMA<'_> {
         // ethernet frame type II (6 smac, 6 dmac, 2 ethertype),
         // sans CRC (4), 1500 IP MTU
         caps.max_transmission_unit = 1514;
-        caps.max_burst_size = Some(core::cmp::min(ETH_NUM_TD, ETH_NUM_RD));
+        caps.max_burst_size = Some(core::cmp::min(TD, RD));
         caps
     }
 
-    fn receive(&mut self) -> Option<(RxToken, TxToken)> {
+    fn receive(&mut self) -> Option<(RxToken<RD>, TxToken<TD>)> {
         // Skip all queued packets with errors.
         while self.ring.rx.available() && !self.ring.rx.valid() {
             self.ring.rx.release()
@@ -776,7 +779,7 @@ impl<'a> phy::Device<'a> for EthernetDMA<'_> {
         }
     }
 
-    fn transmit(&mut self) -> Option<TxToken> {
+    fn transmit(&mut self) -> Option<TxToken<TD>> {
         if self.ring.tx.available() {
             Some(TxToken(&mut self.ring.tx))
         } else {
@@ -785,7 +788,7 @@ impl<'a> phy::Device<'a> for EthernetDMA<'_> {
     }
 }
 
-impl EthernetDMA<'_> {
+impl<const TD: usize, const RD: usize> EthernetDMA<'_, TD, RD> {
     /// Return the number of packets dropped since this method was
     /// last called
     pub fn number_packets_dropped(&self) -> u32 {