Remove SMI and PHY management assumptions, narrow scope to Ethernet

*@thalesfragoso feel free to take or leave this! This direction for
`stm32-eth` has been motivated by my use of the crate over the past few
months for this installation. We currently use a branch that combines a
variation on these changes along with the `stm32f1` branch (#23).*

This PR introduces a couple of significant breaking changes, primarily:

1. **Remove the assumption that the MCU must be associated with a single
   PHY.** MCUs are capable of Ethernet communication without requiring
   knowledge of the PHYs involved at all. In cases where a MCU is
   connected directly to a switch IC via MII or RMII (e.g. using
   something like the KSZ8863), association with a single PHY does not
   make meaningful sense, and an API that expects one can be a little
   frustrating to work around.
2. **Refactor the `smi` module and expose the `Smi` type.** The idea
   behind this change is to narrow the scope of `stm32-eth` to the
   fundamentals of enabling Ethernet communication, while exposing the
   necessary components to allow PHY management to be handled externally
   (e.g. via [`mdio`](https://crates.io/crates/mdio) or device-specific
   crate like [`ksz8863`](https://crates.io/crates/ksz8863)).
   Further, by not requiring that the `Eth` constructor take ownership
   over the `MDIO` and `MDC` pins, we allow the user to use them for
   custom MDIO-based protocols. E.g. the KSZ8863 requires setting the
   R/W OP code bits to `0b00`, however this is not possible to configure
   using the STM32 MAC's limited SMI interface (`MACMIIAR`), so a
   bit-banged implementation of MDIO is required. Without these changes,
   it is not possible to use `stm32-eth` alongside a custom bit-banged
   implementation on the MDIO and MDC pins.

Here's roughly how usage of the new SMI API should look:

```rust
// Borrows MACMIIAR and MACMIIDR from `Eth` along with the MDIO and MDC pins.
let mut smi = eth.smi(&mut mdc, &mut mdio);

// Do stuff with `smi` in scope.
let data = smi.read(phy_addr, reg_addr);
smi.write(phy_addr, reg_addr, data);

// `Smi` instance is dropped at end of scope and borrows are released.
```

Some uses might be as simple as:

```rust
let data = eth.smi(&mut mdc, &mut mdio).read(phy_addr, reg_addr);
```

The `pktgen.rs` example has been updated to demonstrate.

**Follow-up**

- Update `stm32f1` PR for removal of MDIO and MDC pin ownership.

Author: mitchmindtree

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" ]
+features = [ "smi", "smoltcp-phy", "stm32f429" ]
 
 [dependencies]
 volatile-register = "0.2"
@@ -34,6 +34,7 @@ optional = true
 [features]
 device-selected = []
 fence = []
+smi = []
 
 stm32f407 = ["stm32f4xx-hal/stm32f407", "device-selected"]
 stm32f417 = ["stm32f4xx-hal/stm32f417", "device-selected"]
@@ -65,7 +66,7 @@ stm32f4xx-hal = {version = "0.8.3", features = ["rt"] }
 
 [[example]]
 name = "pktgen"
-required-features = ["stm32f429"]
+required-features = ["smi", "stm32f429"]
 
 [[example]]
 name = "ip"

README.md

@@ -70,7 +70,6 @@ fn main() {
         p.ETHERNET_DMA,
         &mut rx_ring[..],
         &mut tx_ring[..],
-        PhyAddress::_0,
         clocks,
         eth_pins,
     )

examples/ip.rs

@@ -24,7 +24,7 @@ use smoltcp::socket::{SocketSet, TcpSocket, TcpSocketBuffer};
 use smoltcp::time::Instant;
 use smoltcp::wire::{EthernetAddress, IpAddress, IpCidr, Ipv4Address};
 
-use stm32_eth::{Eth, EthPins, PhyAddress, RingEntry};
+use stm32_eth::{Eth, EthPins, RingEntry};
 
 static mut LOGGER: HioLogger = HioLogger {};
 
@@ -75,8 +75,6 @@ fn main() -> ! {
 
     let eth_pins = EthPins {
         ref_clk: gpioa.pa1,
-        md_io: gpioa.pa2,
-        md_clk: gpioc.pc1,
         crs: gpioa.pa7,
         tx_en: gpiog.pg11,
         tx_d0: gpiog.pg13,
@@ -92,7 +90,6 @@ fn main() -> ! {
         p.ETHERNET_DMA,
         &mut rx_ring[..],
         &mut tx_ring[..],
-        PhyAddress::_0,
         clocks,
         eth_pins,
     )

examples/pktgen.rs

@@ -1,3 +1,7 @@
+//! Example assumes use of an STM32F429 connected to a network via a single PHY whose address is
+//! `0`. The PHY is expected to be accessible via SMI with an implementation of the standard basic
+//! status register as described in the IEEE 802.3 Ethernet standard.
+
 #![no_std]
 #![no_main]
 
@@ -10,20 +14,22 @@ use cortex_m_rt::{entry, exception};
 use cortex_m::asm;
 use cortex_m::interrupt::Mutex;
 use stm32_eth::{
-    hal::gpio::GpioExt,
+    hal::gpio::{GpioExt, Speed},
     hal::rcc::RccExt,
     hal::time::U32Ext,
+    smi,
     stm32::{interrupt, CorePeripherals, Peripherals, SYST},
 };
 
 use core::fmt::Write;
 use cortex_m_semihosting::hio;
 
-use stm32_eth::{Eth, EthPins, PhyAddress, RingEntry, TxError};
+use stm32_eth::{Eth, EthPins, RingEntry, TxError};
 
 const SRC_MAC: [u8; 6] = [0x00, 0x00, 0xDE, 0xAD, 0xBE, 0xEF];
 const DST_MAC: [u8; 6] = [0x00, 0x00, 0xBE, 0xEF, 0xDE, 0xAD];
 const ETH_TYPE: [u8; 2] = [0x80, 0x00];
+const PHY_ADDR: u8 = 0;
 
 static TIME: Mutex<RefCell<usize>> = Mutex::new(RefCell::new(0));
 static ETH_PENDING: Mutex<RefCell<bool>> = Mutex::new(RefCell::new(false));
@@ -49,8 +55,6 @@ fn main() -> ! {
 
     let eth_pins = EthPins {
         ref_clk: gpioa.pa1,
-        md_io: gpioa.pa2,
-        md_clk: gpioc.pc1,
         crs: gpioa.pa7,
         tx_en: gpiog.pg11,
         tx_d0: gpiog.pg13,
@@ -59,14 +63,16 @@ fn main() -> ! {
         rx_d1: gpioc.pc5,
     };
 
+    let mut mdio = gpioa.pa2.into_alternate_af11().set_speed(Speed::VeryHigh);
+    let mut mdc = gpioc.pc1.into_alternate_af11().set_speed(Speed::VeryHigh);
+
     let mut rx_ring: [RingEntry<_>; 16] = Default::default();
     let mut tx_ring: [RingEntry<_>; 8] = Default::default();
     let mut eth = Eth::new(
         p.ETHERNET_MAC,
         p.ETHERNET_DMA,
         &mut rx_ring[..],
         &mut tx_ring[..],
-        PhyAddress::_0,
         clocks,
         eth_pins,
     )
@@ -79,7 +85,7 @@ fn main() -> ! {
     let mut rx_pkts = 0usize;
     let mut tx_bytes = 0usize;
     let mut tx_pkts = 0usize;
-    let mut last_status = None;
+    let mut last_link_up = false;
 
     loop {
         let time: usize = cortex_m::interrupt::free(|cs| *TIME.borrow(cs).borrow());
@@ -106,28 +112,14 @@ fn main() -> ! {
         }
 
         // Link change detection
-        let status = eth.status();
-        if last_status
-            .map(|last_status| last_status != status)
-            .unwrap_or(true)
-        {
-            if !status.link_detected() {
+        let link_up = link_detected(eth.smi(&mut mdio, &mut mdc));
+        if link_up != last_link_up {
+            if link_up {
                 writeln!(stdout, "Ethernet: no link detected").unwrap();
             } else {
-                writeln!(
-                    stdout,
-                    "Ethernet: link detected with {} Mbps/{}",
-                    status.speed(),
-                    match status.is_full_duplex() {
-                        Some(true) => "FD",
-                        Some(false) => "HD",
-                        None => "?",
-                    }
-                )
-                .unwrap();
+                writeln!(stdout, "Ethernet: link detected!").unwrap();
             }
-
-            last_status = Some(status);
+            last_link_up = link_up;
         }
 
         cortex_m::interrupt::free(|cs| {
@@ -156,7 +148,7 @@ fn main() -> ! {
 
         // fill tx queue
         const SIZE: usize = 1500;
-        if status.link_detected() {
+        if link_detected(eth.smi(&mut mdio, &mut mdc)) {
             'egress: loop {
                 let r = eth.send(SIZE, |buf| {
                     buf[0..6].copy_from_slice(&DST_MAC);
@@ -218,3 +210,14 @@ fn ETH() {
     let p = unsafe { Peripherals::steal() };
     stm32_eth::eth_interrupt_handler(&p.ETHERNET_DMA);
 }
+
+fn link_detected<Mdio, Mdc>(smi: smi::Smi<Mdio, Mdc>) -> bool
+where
+    Mdio: smi::MdioPin,
+    Mdc: smi::MdcPin,
+{
+    const STATUS_REG_ADDR: u8 = 0x3;
+    const STATUS_REG_UP_MASK: u16 = 1 << 2;
+    let status = smi.read(PHY_ADDR, STATUS_REG_ADDR);
+    (status & STATUS_REG_UP_MASK) == STATUS_REG_UP_MASK
+}

src/lib.rs

@@ -17,10 +17,9 @@ pub use stm32f4xx_hal::stm32;
 use hal::rcc::Clocks;
 use stm32::{Interrupt, ETHERNET_DMA, ETHERNET_MAC, NVIC};
 
-pub mod phy;
-use phy::{Phy, PhyStatus};
 mod ring;
-mod smi;
+#[cfg(feature = "smi")]
+pub mod smi;
 pub use ring::RingEntry;
 mod desc;
 mod rx;
@@ -33,7 +32,7 @@ pub mod setup;
 pub use setup::EthPins;
 use setup::{
     AlternateVeryHighSpeed, RmiiCrsDv, RmiiRefClk, RmiiRxD0, RmiiRxD1, RmiiTxD0, RmiiTxD1,
-    RmiiTxEN, MDC, MDIO,
+    RmiiTxEN,
 };
 
 #[cfg(feature = "smoltcp-phy")]
@@ -64,21 +63,12 @@ use self::consts::*;
 #[derive(Debug)]
 pub struct WrongClock;
 
-/// Initial PHY address, must be zero or one.
-#[derive(Copy, Clone, Debug)]
-#[repr(u8)]
-pub enum PhyAddress {
-    _0 = 0,
-    _1 = 1,
-}
-
-/// Ethernet driver for *STM32* chips with a RMII [`Phy`](phy/struct.Phy.html).
+/// Ethernet driver for *STM32* chips.
 pub struct Eth<'rx, 'tx> {
     eth_mac: ETHERNET_MAC,
     eth_dma: ETHERNET_DMA,
     rx_ring: RxRing<'rx>,
     tx_ring: TxRing<'tx>,
-    phy_address: PhyAddress,
 }
 
 impl<'rx, 'tx> Eth<'rx, 'tx> {
@@ -94,19 +84,16 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
     /// Other than that, initializes and starts the Ethernet hardware
     /// so that you can [`send()`](#method.send) and
     /// [`recv_next()`](#method.recv_next).
-    pub fn new<REFCLK, IO, CLK, CRS, TXEN, TXD0, TXD1, RXD0, RXD1>(
+    pub fn new<REFCLK, CRS, TXEN, TXD0, TXD1, RXD0, RXD1>(
         eth_mac: ETHERNET_MAC,
         eth_dma: ETHERNET_DMA,
         rx_buffer: &'rx mut [RxRingEntry],
         tx_buffer: &'tx mut [TxRingEntry],
-        phy_address: PhyAddress,
         clocks: Clocks,
-        pins: EthPins<REFCLK, IO, CLK, CRS, TXEN, TXD0, TXD1, RXD0, RXD1>,
+        pins: EthPins<REFCLK, CRS, TXEN, TXD0, TXD1, RXD0, RXD1>,
     ) -> Result<Self, WrongClock>
     where
         REFCLK: RmiiRefClk + AlternateVeryHighSpeed,
-        IO: MDIO + AlternateVeryHighSpeed,
-        CLK: MDC + AlternateVeryHighSpeed,
         CRS: RmiiCrsDv + AlternateVeryHighSpeed,
         TXEN: RmiiTxEN + AlternateVeryHighSpeed,
         TXD0: RmiiTxD0 + AlternateVeryHighSpeed,
@@ -121,7 +108,6 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
             eth_dma,
             rx_ring: RxRing::new(rx_buffer),
             tx_ring: TxRing::new(tx_buffer),
-            phy_address,
         };
         eth.init(clocks)?;
         eth.rx_ring.start(&eth.eth_dma);
@@ -145,8 +131,6 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
             .macmiiar
             .modify(|_, w| unsafe { w.cr().bits(clock_range) });
 
-        self.get_phy().reset().set_autoneg();
-
         // Configuration Register
         self.eth_mac.maccr.modify(|_, w| {
             // CRC stripping for Type frames
@@ -269,20 +253,6 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
         eth_interrupt_handler(&self.eth_dma);
     }
 
-    /// Construct a PHY driver
-    pub fn get_phy(&self) -> Phy {
-        Phy::new(
-            &self.eth_mac.macmiiar,
-            &self.eth_mac.macmiidr,
-            self.phy_address as u8,
-        )
-    }
-
-    /// Obtain PHY status
-    pub fn status(&self) -> PhyStatus {
-        self.get_phy().status()
-    }
-
     /// Is Rx DMA currently running?
     ///
     /// It stops if the ring is full. Call `recv_next()` to free an
@@ -312,6 +282,26 @@ impl<'rx, 'tx> Eth<'rx, 'tx> {
         self.tx_ring.demand_poll(&self.eth_dma);
         result
     }
+
+    #[cfg(feature = "smi")]
+    /// Borrow access to the MAC's SMI.
+    ///
+    /// Allows for controlling and monitoring any PHYs that may be accessible via the MDIO/MDC
+    /// pins.
+    ///
+    /// Exclusive access to the `MDIO` and `MDC` is required to ensure that are not used elsewhere
+    /// for the duration of SMI communication.
+    pub fn smi<'eth, 'pins, Mdio, Mdc>(
+        &'eth mut self,
+        mdio: &'pins mut Mdio,
+        mdc: &'pins mut Mdc,
+    ) -> smi::Smi<'eth, 'pins, Mdio, Mdc>
+    where
+        Mdio: smi::MdioPin,
+        Mdc: smi::MdcPin,
+    {
+        smi::Smi::new(&self.eth_mac.macmiiar, &self.eth_mac.macmiidr, mdio, mdc)
+    }
 }
 
 /// Call in interrupt handler to clear interrupt reason, when