Move all SMI functionality into MAC module

Add some docs and make LAN87XXA phy implmementation a bit more clear

Author: Johannes Draaijer

Cargo.toml

@@ -15,7 +15,7 @@ travis-ci = { repository = "astro/stm32-eth", branch = "master" }
 maintenance = { status = "experimental" }
 
 [package.metadata.docs.rs]
-features = ["smoltcp-phy", "stm32f429", "smoltcp/socket-tcp"]
+features = ["smoltcp-phy", "smoltcp/socket-raw", "stm32f429"]
 
 [dependencies]
 volatile-register = "0.2"

src/lib.rs

@@ -28,7 +28,6 @@ use hal::rcc::Clocks;
 use stm32::{Interrupt, ETHERNET_DMA, ETHERNET_MAC, ETHERNET_MMC, NVIC};
 
 mod ring;
-pub mod smi;
 pub use ring::RingEntry;
 mod desc;
 mod mac;
@@ -99,11 +98,11 @@ macro_rules! new {
         /// This method does not initialise the external PHY.
         ///
         /// Interacting with a PHY can be done through a struct that implementes the
-        /// [`smi::StationManagement`] trait.
+        /// [`StationManagement`] trait.
         ///
         /// In the case of `new_borrowed_smi` you may access SMI through the [`EthernetMAC::smi`] function.
         ///
-        /// In the case of `new_no_smi`, the SMI may be accessed by constructing a new [`smi::Smi`] before passing
+        /// In the case of `new_no_smi`, the SMI may be accessed by constructing a new [`Smi`] before passing
         /// the `ETHERNET_MAC` to the `new_no_smi` function.
         pub fn $name<'rx, 'tx, REFCLK, CRS, TXEN, TXD0, TXD1, RXD0, RXD1>(
             eth_mac: ETHERNET_MAC,
@@ -145,7 +144,7 @@ new!(new_borrowed_smi, BorrowedSmi);
 /// [new_unchecked](new_unchecked).
 ///
 /// This method does not initialise the external PHY. The SMI for the external PHY
-/// can be accessed through a struct that implements the [`smi::StationManagement`] trait,
+/// can be accessed through a struct that implements the [`StationManagement`] trait,
 /// which [`EthernetMAC<OwnedSmi>`] does.
 ///
 /// # Note
@@ -178,12 +177,12 @@ where
     TXD1: RmiiTxD1 + AlternateVeryHighSpeed,
     RXD0: RmiiRxD0 + AlternateVeryHighSpeed,
     RXD1: RmiiRxD1 + AlternateVeryHighSpeed,
-    MDIO: smi::MdioPin,
-    MDC: smi::MdcPin,
+    MDIO: MdioPin,
+    MDC: MdcPin,
 {
     pins.setup_pins();
 
-    let eth_mac = EthernetMAC::new_owned(eth_mac, mdio, mdc);
+    let eth_mac = EthernetMAC::<OwnedSmi<MDIO, MDC>>::new(eth_mac, mdio, mdc);
 
     unsafe { new_unchecked(eth_mac, eth_mmc, eth_dma, rx_buffer, tx_buffer, clocks) }
 }
@@ -192,7 +191,7 @@ where
 ///
 /// This method does not initialise the external PHY. However it does return an
 /// [EthernetMAC](EthernetMAC) which implements the
-/// [StationManagement](smi::StationManagement) trait. This can be used to
+/// [StationManagement](StationManagement) trait. This can be used to
 /// communicate with the external PHY.
 ///
 /// # Note

src/mac.rs

@@ -1,7 +1,155 @@
-use crate::smi::{self, read, write, StationManagement};
-use crate::stm32::ETHERNET_MAC;
+use crate::stm32::{
+    ethernet_mac::{MACMIIAR, MACMIIDR},
+    ETHERNET_MAC,
+};
 
-pub struct NoSmi;
+/// MDIO pin types.
+pub unsafe trait MdioPin {}
+/// MDC pin types.
+pub unsafe trait MdcPin {}
+
+pub trait StationManagement {
+    fn read(&self, phy: u8, reg: u8) -> u16;
+    fn write(&mut self, phy: u8, reg: u8, data: u16);
+}
+
+#[inline(always)]
+fn smi_wait_ready(iar: &MACMIIAR) {
+    while iar.read().mb().bit_is_set() {}
+}
+
+#[inline(always)]
+fn smi_write(iar: &MACMIIAR, dr: &MACMIIDR, phy: u8, reg: u8, data: u16) {
+    dr.write(|w| {
+        #[cfg_attr(not(feature = "stm32f107"), allow(unused_unsafe))]
+        unsafe {
+            w.md().bits(data)
+        }
+    });
+
+    iar.modify(|_, w| {
+        #[cfg_attr(not(feature = "stm32f107"), allow(unused_unsafe))]
+        unsafe {
+            w.pa()
+                .bits(phy)
+                .mr()
+                .bits(reg)
+                /* Write operation MW=1*/
+                .mw()
+                .set_bit()
+                .mb()
+                .set_bit()
+        }
+    });
+    smi_wait_ready(iar);
+}
+
+#[inline(always)]
+fn smi_read(iar: &MACMIIAR, dr: &MACMIIDR, phy: u8, reg: u8) -> u16 {
+    iar.modify(|_, w| {
+        #[cfg_attr(not(feature = "stm32f107"), allow(unused_unsafe))]
+        unsafe {
+            w.pa()
+                .bits(phy)
+                .mr()
+                .bits(reg)
+                /* Read operation MW=0 */
+                .mw()
+                .clear_bit()
+                .mb()
+                .set_bit()
+        }
+    });
+    smi_wait_ready(iar);
+
+    // Return value:
+    dr.read().md().bits()
+}
+
+/// Station Management Interface with pins and registers borrowed from [`EthernetMAC`].
+///
+/// Can also be constructed by borrowing from [`ETHERNET_MAC`] and borrowing the pins manually.
+///
+/// Provides access to the MIIM implementation exposed by the MCU's MAC API.
+pub struct Smi<'eth, 'pins, Mdio, Mdc> {
+    macmiiar: &'eth MACMIIAR,
+    macmiidr: &'eth MACMIIDR,
+    _mdio: &'pins mut Mdio,
+    _mdc: &'pins mut Mdc,
+}
+
+impl<'eth, 'pins, Mdio, Mdc> StationManagement for Smi<'eth, 'pins, Mdio, Mdc>
+where
+    Mdio: MdioPin,
+    Mdc: MdcPin,
+{
+    /// Read an SMI register
+    fn read(&self, phy: u8, reg: u8) -> u16 {
+        smi_read(&self.macmiiar, &self.macmiidr, phy, reg)
+    }
+
+    /// Write an SMI register
+    fn write(&mut self, phy: u8, reg: u8, data: u16) {
+        smi_write(&self.macmiiar, &self.macmiidr, phy, reg, data)
+    }
+}
+
+impl<'eth, 'pins, Mdio, Mdc> Smi<'eth, 'pins, Mdio, Mdc>
+where
+    Mdio: MdioPin,
+    Mdc: MdcPin,
+{
+    /// Create the temporary `Smi` instance.
+    ///
+    /// Temporarily take exclusive access to the MDIO and MDC pins to ensure they are not used
+    /// elsewhere for the duration of SMI communication.
+    pub fn new(
+        macmiiar: &'eth MACMIIAR,
+        macmiidr: &'eth MACMIIDR,
+        _mdio: &'pins mut Mdio,
+        _mdc: &'pins mut Mdc,
+    ) -> Self {
+        Self {
+            macmiiar,
+            macmiidr,
+            _mdio,
+            _mdc,
+        }
+    }
+
+    /// Read an SMI register
+    pub fn read(&self, phy: u8, reg: u8) -> u16 {
+        smi_read(&self.macmiiar, &self.macmiidr, phy, reg)
+    }
+}
+
+#[cfg(feature = "stm32f4xx-hal")]
+mod pin_impls {
+    use crate::hal::gpio::{gpioa::PA2, gpioc::PC1, Alternate};
+
+    const AF11: u8 = 11;
+
+    unsafe impl super::MdioPin for PA2<Alternate<AF11>> {}
+    unsafe impl super::MdcPin for PC1<Alternate<AF11>> {}
+}
+
+#[cfg(feature = "stm32f7xx-hal")]
+mod pin_impls {
+    use crate::hal::gpio::{gpioa::PA2, gpioc::PC1, Alternate};
+
+    const AF11: u8 = 11;
+
+    unsafe impl super::MdioPin for PA2<Alternate<AF11>> {}
+    unsafe impl super::MdcPin for PC1<Alternate<AF11>> {}
+}
+
+#[cfg(feature = "stm32f1xx-hal")]
+mod pin_impls {
+    use crate::hal::gpio::{gpioa::PA2, gpioc::PC1, Alternate, PushPull};
+
+    unsafe impl super::MdioPin for PA2<Alternate<PushPull>> {}
+    unsafe impl super::MdcPin for PC1<Alternate<PushPull>> {}
+}
 
 /// Ethernet media access control (MAC).
 pub struct EthernetMAC<SMI> {
@@ -18,12 +166,18 @@ impl EthernetMAC<NoSmi> {
     }
 }
 
+/// The MAC does not have and does not provide access to its SMI
+pub struct NoSmi;
+
+/// Access to the MAC's SMI can be obtained by borrowing [`MdioPin`] and [`MdcPin`] to it. For [`StationManagement`], see [`EthernetMAC::smi`]
 pub struct BorrowedSmi;
 
+/// The SMI (and its pins) are owned. An [`EthernetMAC`] that owns
+/// its SMI is [`StationManagement`]
 pub struct OwnedSmi<MDIO, MDC>
 where
-    MDIO: smi::MdioPin,
-    MDC: smi::MdcPin,
+    MDIO: MdioPin,
+    MDC: MdcPin,
 {
     _mdio: MDIO,
     _mdc: MDC,
@@ -48,21 +202,21 @@ impl EthernetMAC<BorrowedSmi> {
         &'eth mut self,
         mdio: &'pins mut Mdio,
         mdc: &'pins mut Mdc,
-    ) -> smi::Smi<'eth, 'pins, Mdio, Mdc>
+    ) -> Smi<'eth, 'pins, Mdio, Mdc>
     where
-        Mdio: smi::MdioPin,
-        Mdc: smi::MdcPin,
+        Mdio: MdioPin,
+        Mdc: MdcPin,
     {
-        smi::Smi::new(&self.eth_mac.macmiiar, &self.eth_mac.macmiidr, mdio, mdc)
+        Smi::new(&self.eth_mac.macmiiar, &self.eth_mac.macmiidr, mdio, mdc)
     }
 }
 
 impl<MDIO, MDC> EthernetMAC<OwnedSmi<MDIO, MDC>>
 where
-    MDIO: smi::MdioPin,
-    MDC: smi::MdcPin,
+    MDIO: MdioPin,
+    MDC: MdcPin,
 {
-    pub fn new_owned(eth_mac: ETHERNET_MAC, mdio: MDIO, mdc: MDC) -> Self {
+    pub fn new(eth_mac: ETHERNET_MAC, mdio: MDIO, mdc: MDC) -> Self {
         Self {
             eth_mac,
             _state: OwnedSmi {
@@ -75,15 +229,15 @@ where
 
 impl<MDIO, MDC> StationManagement for EthernetMAC<OwnedSmi<MDIO, MDC>>
 where
-    MDIO: smi::MdioPin,
-    MDC: smi::MdcPin,
+    MDIO: MdioPin,
+    MDC: MdcPin,
 {
     fn read(&self, phy: u8, reg: u8) -> u16 {
-        read(&self.eth_mac.macmiiar, &self.eth_mac.macmiidr, phy, reg)
+        smi_read(&self.eth_mac.macmiiar, &self.eth_mac.macmiidr, phy, reg)
     }
 
     fn write(&mut self, phy: u8, reg: u8, data: u16) {
-        write(
+        smi_write(
             &self.eth_mac.macmiiar,
             &self.eth_mac.macmiidr,
             phy,

src/phy/lan87xxa.rs

@@ -3,7 +3,12 @@
 use core::convert::TryFrom;
 use num_enum::{IntoPrimitive, TryFromPrimitive};
 
-use crate::smi::StationManagement;
+use crate::mac::StationManagement;
+
+/// SMSC LAN8720A Ethernet PHY
+pub type LAN8720A<SMI> = LAN87xxA<SMI, false>;
+/// SMSC LAN8742A Ethernet PHY
+pub type LAN8742A<SMI> = LAN87xxA<SMI, true>;
 
 /// The link speeds supported by this PHY
 #[derive(Clone, Copy, Debug, IntoPrimitive, TryFromPrimitive)]
@@ -65,17 +70,22 @@ mod phy_consts {
     pub const PHY_REG_SSR_100BASE_FD: u16 = 0b110 << 2;
 }
 
-/// SMSC LAN8742A Ethernet PHY
-pub struct LAN87xxA<S> {
+/// An SMSC LAN87XXA Ethernet PHY.
+///
+/// EXT_WUCSR_CLEAR is used to determine if the "WU CSR" bit
+/// in extended registers should be cleared
+///
+/// This type should not be used directly. Use [`LAN8720A`] or [`LAN8742A`] instead.
+pub struct LAN87xxA<S, const EXT_WUCSR_CLEAR: bool> {
     phy_addr: u8,
     smi: S,
 }
 
-impl<S> LAN87xxA<S>
+impl<S, const EXT_WUCSR_CLEAR: bool> LAN87xxA<S, EXT_WUCSR_CLEAR>
 where
     S: StationManagement,
 {
-    ///
+    /// Create a new LAN87XXA based PHY
     pub fn new(smi: S, phy_addr: u8) -> Self {
         LAN87xxA { smi, phy_addr }
     }
@@ -84,6 +94,16 @@ where
         self.smi.write(self.phy_addr, reg, data)
     }
 
+    /// Writes a value to an extended PHY register in MMD address space.
+    ///
+    /// Only available in `LAN8742A` PHYs
+    fn smi_write_ext(&mut self, reg_addr: u16, reg_data: u16) {
+        self.write(PHY_REG_CTL, 0x0003); // set address
+        self.write(PHY_REG_ADDAR, reg_addr);
+        self.write(PHY_REG_CTL, 0x4003); // set data
+        self.write(PHY_REG_ADDAR, reg_data);
+    }
+
     fn read(&mut self, reg: u8) -> u16 {
         self.smi.read(self.phy_addr, reg)
     }
@@ -94,11 +114,12 @@ where
         while self.read(PHY_REG_BCR) & PHY_REG_BCR_RESET == PHY_REG_BCR_RESET {}
     }
 
-    /// PHY initialisation.
+    /// Initialize the PHY
     pub fn phy_init(&mut self) {
-        // Clear WU CSR
-        #[cfg(feature = "phy-lan8742a")]
-        self.smi_write_ext(PHY_REG_WUCSR, 0);
+        if EXT_WUCSR_CLEAR {
+            // Clear WU CSR
+            self.smi_write_ext(PHY_REG_WUCSR, 0);
+        }
 
         // Enable auto-negotiation
         self.write(
@@ -147,25 +168,18 @@ where
         LinkSpeed::try_from(link_data).ok()
     }
 
-    ///
+    /// Check if the link is up
     pub fn link_established(&mut self) -> bool {
         self.poll_link()
     }
 
-    ///
+    /// Block until a link is established
     pub fn block_until_link(&mut self) {
         while !self.link_established() {}
     }
 
-    #[cfg_attr(
-        feature = "phy-lan8742a",
-        doc = "Writes a value to an extended PHY register in MMD address space"
-    )]
-    #[cfg(feature = "phy-lan8742a")]
-    fn smi_write_ext(&mut self, reg_addr: u16, reg_data: u16) {
-        self.write(PHY_REG_CTL, 0x0003); // set address
-        self.write(PHY_REG_ADDAR, reg_addr);
-        self.write(PHY_REG_CTL, 0x4003); // set data
-        self.write(PHY_REG_ADDAR, reg_data);
+    /// Release the underlying [`StationManagement`]
+    pub fn release(self) -> S {
+        self.smi
     }
 }