From c4b88b57812da85b6952300509736fd02a4640fa Mon Sep 17 00:00:00 2001 From: Josh Junon Date: Sat, 29 Jun 2024 01:17:27 +0200 Subject: [PATCH] wiznet: add version check to initialization sequence --- embassy-net-wiznet/src/chip/mod.rs | 7 +++ embassy-net-wiznet/src/chip/w5100s.rs | 3 + embassy-net-wiznet/src/chip/w5500.rs | 3 + embassy-net-wiznet/src/device.rs | 62 ++++++++++++++++++- embassy-net-wiznet/src/lib.rs | 10 +-- .../rp/src/bin/ethernet_w5500_multisocket.rs | 3 +- .../rp/src/bin/ethernet_w5500_tcp_client.rs | 3 +- .../rp/src/bin/ethernet_w5500_tcp_server.rs | 3 +- examples/rp/src/bin/ethernet_w5500_udp.rs | 3 +- examples/stm32f4/src/bin/eth_w5500.rs | 4 +- tests/rp/src/bin/ethernet_w5100s_perf.rs | 3 +- 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 { _phantom: PhantomData, } +/// Error type when initializing a new Wiznet device +pub enum InitError { + /// 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 From for InitError { + fn from(e: SE) -> Self { + InitError::SpiError(e) + } +} + +impl core::fmt::Debug for InitError +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 defmt::Format for InitError +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 WiznetDevice { /// Create and initialize the driver - pub async fn new(spi: SPI, mac_addr: [u8; 6]) -> Result { + pub async fn new(spi: SPI, mac_addr: [u8; 6]) -> Result> { let mut this = Self { spi, _phantom: PhantomData, @@ -35,6 +83,18 @@ impl WiznetDevice { // 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> { // 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> = 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