Merge pull request #3132 from oro-os/wiznet-version-check

wiznet: add version check to initialization sequence

11 files changed, 93 insertions, 11 deletions

diff --git a/embassy-net-wiznet/src/chip/mod.rs b/embassy-net-wiznet/src/chip/mod.rs
index e1f963d95..2e7a9ed6c 100644
--- a/embassy-net-wiznet/src/chip/mod.rs
+++ b/embassy-net-wiznet/src/chip/mod.rs
@@ -8,10 +8,17 @@ pub use w5100s::W5100S;
 pub(crate) trait SealedChip {
     type Address;
 
+    /// The version of the chip as reported by the VERSIONR register.
+    /// This is used to verify that the chip is supported by the driver,
+    /// and that SPI communication is working.
+    const CHIP_VERSION: u8;
+
     const COMMON_MODE: Self::Address;
     const COMMON_MAC: Self::Address;
     const COMMON_SOCKET_INTR: Self::Address;
     const COMMON_PHY_CFG: Self::Address;
+    const COMMON_VERSION: Self::Address;
+
     const SOCKET_MODE: Self::Address;
     const SOCKET_COMMAND: Self::Address;
     const SOCKET_RXBUF_SIZE: Self::Address;
diff --git a/embassy-net-wiznet/src/chip/w5100s.rs b/embassy-net-wiznet/src/chip/w5100s.rs
index 23ce3ed83..4c4b7ab16 100644
--- a/embassy-net-wiznet/src/chip/w5100s.rs
+++ b/embassy-net-wiznet/src/chip/w5100s.rs
@@ -11,10 +11,13 @@ impl super::Chip for W5100S {}
 impl super::SealedChip for W5100S {
     type Address = u16;
 
+    const CHIP_VERSION: u8 = 0x51;
+
     const COMMON_MODE: Self::Address = 0x00;
     const COMMON_MAC: Self::Address = 0x09;
     const COMMON_SOCKET_INTR: Self::Address = 0x16;
     const COMMON_PHY_CFG: Self::Address = 0x3c;
+    const COMMON_VERSION: Self::Address = 0x80;
 
     const SOCKET_MODE: Self::Address = SOCKET_BASE + 0x00;
     const SOCKET_COMMAND: Self::Address = SOCKET_BASE + 0x01;
diff --git a/embassy-net-wiznet/src/chip/w5500.rs b/embassy-net-wiznet/src/chip/w5500.rs
index 12e610ea2..5cfcb94e4 100644
--- a/embassy-net-wiznet/src/chip/w5500.rs
+++ b/embassy-net-wiznet/src/chip/w5500.rs
@@ -15,10 +15,13 @@ impl super::Chip for W5500 {}
 impl super::SealedChip for W5500 {
     type Address = (RegisterBlock, u16);
 
+    const CHIP_VERSION: u8 = 0x04;
+
     const COMMON_MODE: Self::Address = (RegisterBlock::Common, 0x00);
     const COMMON_MAC: Self::Address = (RegisterBlock::Common, 0x09);
     const COMMON_SOCKET_INTR: Self::Address = (RegisterBlock::Common, 0x18);
     const COMMON_PHY_CFG: Self::Address = (RegisterBlock::Common, 0x2E);
+    const COMMON_VERSION: Self::Address = (RegisterBlock::Common, 0x39);
 
     const SOCKET_MODE: Self::Address = (RegisterBlock::Socket0, 0x00);
     const SOCKET_COMMAND: Self::Address = (RegisterBlock::Socket0, 0x01);
diff --git a/embassy-net-wiznet/src/device.rs b/embassy-net-wiznet/src/device.rs
index 43f9512a3..d2b6bb0c3 100644
--- a/embassy-net-wiznet/src/device.rs
+++ b/embassy-net-wiznet/src/device.rs
@@ -24,9 +24,57 @@ pub(crate) struct WiznetDevice<C, SPI> {
     _phantom: PhantomData<C>,
 }
 
+/// Error type when initializing a new Wiznet device
+pub enum InitError<SE> {
+    /// Error occurred when sending or receiving SPI data
+    SpiError(SE),
+    /// The chip returned a version that isn't expected or supported
+    InvalidChipVersion {
+        /// The version that is supported
+        expected: u8,
+        /// The version that was returned by the chip
+        actual: u8,
+    },
+}
+
+impl<SE> From<SE> for InitError<SE> {
+    fn from(e: SE) -> Self {
+        InitError::SpiError(e)
+    }
+}
+
+impl<SE> core::fmt::Debug for InitError<SE>
+where
+    SE: core::fmt::Debug,
+{
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            InitError::SpiError(e) => write!(f, "SpiError({:?})", e),
+            InitError::InvalidChipVersion { expected, actual } => {
+                write!(f, "InvalidChipVersion {{ expected: {}, actual: {} }}", expected, actual)
+            }
+        }
+    }
+}
+
+#[cfg(feature = "defmt")]
+impl<SE> defmt::Format for InitError<SE>
+where
+    SE: defmt::Format,
+{
+    fn format(&self, f: defmt::Formatter) {
+        match self {
+            InitError::SpiError(e) => defmt::write!(f, "SpiError({})", e),
+            InitError::InvalidChipVersion { expected, actual } => {
+                defmt::write!(f, "InvalidChipVersion {{ expected: {}, actual: {} }}", expected, actual)
+            }
+        }
+    }
+}
+
 impl<C: Chip, SPI: SpiDevice> WiznetDevice<C, SPI> {
     /// Create and initialize the driver
-    pub async fn new(spi: SPI, mac_addr: [u8; 6]) -> Result<Self, SPI::Error> {
+    pub async fn new(spi: SPI, mac_addr: [u8; 6]) -> Result<Self, InitError<SPI::Error>> {
         let mut this = Self {
             spi,
             _phantom: PhantomData,
@@ -35,6 +83,18 @@ impl<C: Chip, SPI: SpiDevice> WiznetDevice<C, SPI> {
         // Reset device
         this.bus_write(C::COMMON_MODE, &[0x80]).await?;
 
+        // Check the version of the chip
+        let mut version = [0];
+        this.bus_read(C::COMMON_VERSION, &mut version).await?;
+        if version[0] != C::CHIP_VERSION {
+            #[cfg(feature = "defmt")]
+            defmt::error!("invalid chip version: {} (expected {})", version[0], C::CHIP_VERSION);
+            return Err(InitError::InvalidChipVersion {
+                actual: version[0],
+                expected: C::CHIP_VERSION,
+            });
+        }
+
         // Enable interrupt pin
         this.bus_write(C::COMMON_SOCKET_INTR, &[0x01]).await?;
         // Enable receive interrupt
diff --git a/embassy-net-wiznet/src/lib.rs b/embassy-net-wiznet/src/lib.rs
index 90102196a..3fbd4c741 100644
--- a/embassy-net-wiznet/src/lib.rs
+++ b/embassy-net-wiznet/src/lib.rs
@@ -15,6 +15,7 @@ use embedded_hal_async::digital::Wait;
 use embedded_hal_async::spi::SpiDevice;
 
 use crate::chip::Chip;
+pub use crate::device::InitError;
 use crate::device::WiznetDevice;
 
 // If you change this update the docs of State
@@ -105,7 +106,7 @@ pub async fn new<'a, const N_RX: usize, const N_TX: usize, C: Chip, SPI: SpiDevi
     spi_dev: SPI,
     int: INT,
     mut reset: RST,
-) -> (Device<'a>, Runner<'a, C, SPI, INT, RST>) {
+) -> Result<(Device<'a>, Runner<'a, C, SPI, INT, RST>), InitError<SPI::Error>> {
     // Reset the chip.
     reset.set_low().ok();
     // Ensure the reset is registered.
@@ -116,10 +117,11 @@ pub async fn new<'a, const N_RX: usize, const N_TX: usize, C: Chip, SPI: SpiDevi
     // Slowest is w5100s which is 100ms, so let's just wait that.
     Timer::after_millis(100).await;
 
-    let mac = WiznetDevice::new(spi_dev, mac_addr).await.unwrap();
+    let mac = WiznetDevice::new(spi_dev, mac_addr).await?;
 
     let (runner, device) = ch::new(&mut state.ch_state, ch::driver::HardwareAddress::Ethernet(mac_addr));
-    (
+
+    Ok((
         device,
         Runner {
             ch: runner,
@@ -127,5 +129,5 @@ pub async fn new<'a, const N_RX: usize, const N_TX: usize, C: Chip, SPI: SpiDevi
             int,
             _reset: reset,
         },
-    )
+    ))
 }
diff --git a/examples/rp/src/bin/ethernet_w5500_multisocket.rs b/examples/rp/src/bin/ethernet_w5500_multisocket.rs
index bd52cadca..def26b53d 100644
--- a/examples/rp/src/bin/ethernet_w5500_multisocket.rs
+++ b/examples/rp/src/bin/ethernet_w5500_multisocket.rs
@@ -63,7 +63,8 @@ async fn main(spawner: Spawner) {
         w5500_int,
         w5500_reset,
     )
-    .await;
+    .await
+    .unwrap();
     unwrap!(spawner.spawn(ethernet_task(runner)));
 
     // Generate random seed
diff --git a/examples/rp/src/bin/ethernet_w5500_tcp_client.rs b/examples/rp/src/bin/ethernet_w5500_tcp_client.rs
index 3e4fbd2e6..6c4a78361 100644
--- a/examples/rp/src/bin/ethernet_w5500_tcp_client.rs
+++ b/examples/rp/src/bin/ethernet_w5500_tcp_client.rs
@@ -66,7 +66,8 @@ async fn main(spawner: Spawner) {
         w5500_int,
         w5500_reset,
     )
-    .await;
+    .await
+    .unwrap();
     unwrap!(spawner.spawn(ethernet_task(runner)));
 
     // Generate random seed
diff --git a/examples/rp/src/bin/ethernet_w5500_tcp_server.rs b/examples/rp/src/bin/ethernet_w5500_tcp_server.rs
index 5532851f3..30a3a7463 100644
--- a/examples/rp/src/bin/ethernet_w5500_tcp_server.rs
+++ b/examples/rp/src/bin/ethernet_w5500_tcp_server.rs
@@ -65,7 +65,8 @@ async fn main(spawner: Spawner) {
         w5500_int,
         w5500_reset,
     )
-    .await;
+    .await
+    .unwrap();
     unwrap!(spawner.spawn(ethernet_task(runner)));
 
     // Generate random seed
diff --git a/examples/rp/src/bin/ethernet_w5500_udp.rs b/examples/rp/src/bin/ethernet_w5500_udp.rs
index adb1d8941..1613ed887 100644
--- a/examples/rp/src/bin/ethernet_w5500_udp.rs
+++ b/examples/rp/src/bin/ethernet_w5500_udp.rs
@@ -63,7 +63,8 @@ async fn main(spawner: Spawner) {
         w5500_int,
         w5500_reset,
     )
-    .await;
+    .await
+    .unwrap();
     unwrap!(spawner.spawn(ethernet_task(runner)));
 
     // Generate random seed
diff --git a/examples/stm32f4/src/bin/eth_w5500.rs b/examples/stm32f4/src/bin/eth_w5500.rs
index c51111110..3c770a873 100644
--- a/examples/stm32f4/src/bin/eth_w5500.rs
+++ b/examples/stm32f4/src/bin/eth_w5500.rs
@@ -80,7 +80,9 @@ async fn main(spawner: Spawner) -> ! {
     let mac_addr = [0x02, 234, 3, 4, 82, 231];
     static STATE: StaticCell<State<2, 2>> = StaticCell::new();
     let state = STATE.init(State::<2, 2>::new());
-    let (device, runner) = embassy_net_wiznet::new(mac_addr, state, spi, w5500_int, w5500_reset).await;
+    let (device, runner) = embassy_net_wiznet::new(mac_addr, state, spi, w5500_int, w5500_reset)
+        .await
+        .unwrap();
     unwrap!(spawner.spawn(ethernet_task(runner)));
 
     let config = embassy_net::Config::dhcpv4(Default::default());
diff --git a/tests/rp/src/bin/ethernet_w5100s_perf.rs b/tests/rp/src/bin/ethernet_w5100s_perf.rs
index 5d5547773..4b04571bd 100644
--- a/tests/rp/src/bin/ethernet_w5100s_perf.rs
+++ b/tests/rp/src/bin/ethernet_w5100s_perf.rs
@@ -59,7 +59,8 @@ async fn main(spawner: Spawner) {
         w5500_int,
         w5500_reset,
     )
-    .await;
+    .await
+    .unwrap();
     unwrap!(spawner.spawn(ethernet_task(runner)));
 
     // Generate random seed