Merge pull request #1796 from embassy-rs/enc28j60-fixes

enc28j60: add HIL test, workaround RX buffer corruption.

embassy-net-enc28j60/src/lib.rs

@@ -106,19 +106,7 @@ where
         // disable CLKOUT output
         self.write_control_register(bank3::Register::ECOCON, 0);
 
-        // RX start
-        // "It is recommended that the ERXST Pointer be programmed with an even address"
-        self.write_control_register(bank0::Register::ERXSTL, RXST.low());
-        self.write_control_register(bank0::Register::ERXSTH, RXST.high());
-
-        // RX read pointer
-        // NOTE Errata #14 so we are using an *odd* address here instead of ERXST
-        self.write_control_register(bank0::Register::ERXRDPTL, RXND.low());
-        self.write_control_register(bank0::Register::ERXRDPTH, RXND.high());
-
-        // RX end
-        self.write_control_register(bank0::Register::ERXNDL, RXND.low());
-        self.write_control_register(bank0::Register::ERXNDH, RXND.high());
+        self.init_rx();
 
         // TX start
         // "It is recommended that an even address be used for ETXST"
@@ -172,12 +160,37 @@ where
         // Set the per packet control byte; we'll always use the value 0
         self.write_buffer_memory(Some(TXST), &mut [0]);
 
+        // Enable reception
+        self.bit_field_set(common::Register::ECON1, common::ECON1::mask().rxen());
+    }
+
+    fn init_rx(&mut self) {
+        // RX start
+        // "It is recommended that the ERXST Pointer be programmed with an even address"
+        self.write_control_register(bank0::Register::ERXSTL, RXST.low());
+        self.write_control_register(bank0::Register::ERXSTH, RXST.high());
+
+        // RX read pointer
+        // NOTE Errata #14 so we are using an *odd* address here instead of ERXST
+        self.write_control_register(bank0::Register::ERXRDPTL, RXND.low());
+        self.write_control_register(bank0::Register::ERXRDPTH, RXND.high());
+
+        // RX end
+        self.write_control_register(bank0::Register::ERXNDL, RXND.low());
+        self.write_control_register(bank0::Register::ERXNDH, RXND.high());
+
         // decrease the packet count to 0
         while self.read_control_register(bank1::Register::EPKTCNT) != 0 {
             self.bit_field_set(common::Register::ECON2, common::ECON2::mask().pktdec());
         }
 
-        // Enable reception
+        self.next_packet = RXST;
+    }
+
+    fn reset_rx(&mut self) {
+        self.bit_field_set(common::Register::ECON1, common::ECON1::mask().rxrst());
+        self.bit_field_clear(common::Register::ECON1, common::ECON1::mask().rxrst());
+        self.init_rx();
         self.bit_field_set(common::Register::ECON1, common::ECON1::mask().rxen());
     }
 
@@ -198,18 +211,17 @@ where
 
         // next packet pointer
         let next_packet = u16::from_parts(temp_buf[0], temp_buf[1]);
-        if next_packet > RXND {
-            panic!("CorruptRxBuffer");
-        }
-
         // status vector
         let status = header::RxStatus(u32::from_le_bytes(temp_buf[2..].try_into().unwrap()));
-        let len = status.byte_count() as u16 - CRC_SZ;
+        let len_with_crc = status.byte_count() as u16;
 
-        if len > RXND {
-            panic!("CorruptRxBuffer 2");
+        if len_with_crc < CRC_SZ || len_with_crc > 1600 || next_packet > RXND {
+            warn!("RX buffer corrupted, resetting RX logic to recover...");
+            self.reset_rx();
+            return None;
         }
 
+        let len = len_with_crc - CRC_SZ;
         self.read_buffer_memory(None, &mut buf[..len as usize]);
 
         // update ERXRDPT
@@ -381,8 +393,6 @@ where
     }
 
     fn read_phy_register(&mut self, register: phy::Register) -> u16 {
-        embassy_time::block_for(Duration::from_millis(1));
-
         // set PHY register address
         self.write_control_register(bank2::Register::MIREGADR, register.addr());
 

tests/nrf/Cargo.toml

@@ -15,6 +15,7 @@ embassy-nrf = { version = "0.1.0", path = "../../embassy-nrf", features = ["defm
 embedded-io-async = { version = "0.5.0" }
 embassy-net = { version = "0.1.0", path = "../../embassy-net", features = ["defmt", "tcp", "dhcpv4", "medium-ethernet", "nightly"] }
 embassy-net-esp-hosted = { version = "0.1.0", path = "../../embassy-net-esp-hosted", features = ["defmt"] }
+embassy-net-enc28j60 = { version = "0.1.0", path = "../../embassy-net-enc28j60", features = ["defmt"] }
 embedded-hal-async = { version = "1.0.0-rc.1" }
 embedded-hal-bus = { version = "0.1.0-rc.1", features = ["async"] }
 static_cell = { version = "1.1", features = [ "nightly" ] }

tests/nrf/src/bin/buffered_uart.rs

@@ -1,8 +1,7 @@
 #![no_std]
 #![no_main]
 #![feature(type_alias_impl_trait)]
-#[path = "../common.rs"]
-mod common;
+teleprobe_meta::target!(b"nrf52840-dk");
 
 use defmt::{assert_eq, *};
 use embassy_executor::Spawner;

tests/nrf/src/bin/buffered_uart_spam.rs

@@ -1,8 +1,7 @@
 #![no_std]
 #![no_main]
 #![feature(type_alias_impl_trait)]
-#[path = "../common.rs"]
-mod common;
+teleprobe_meta::target!(b"nrf52840-dk");
 
 use core::mem;
 use core::ptr::NonNull;

tests/nrf/src/bin/ethernet_enc28j60_perf.rs

@@ -0,0 +1,250 @@
+#![no_std]
+#![no_main]
+#![feature(type_alias_impl_trait)]
+teleprobe_meta::target!(b"ak-gwe-r7");
+teleprobe_meta::timeout!(120);
+
+use defmt::{error, info, unwrap};
+use embassy_executor::Spawner;
+use embassy_futures::join::join;
+use embassy_net::tcp::TcpSocket;
+use embassy_net::{Ipv4Address, Stack, StackResources};
+use embassy_net_enc28j60::Enc28j60;
+use embassy_nrf::gpio::{Level, Output, OutputDrive};
+use embassy_nrf::rng::Rng;
+use embassy_nrf::spim::{self, Spim};
+use embassy_nrf::{bind_interrupts, peripherals};
+use embassy_time::{with_timeout, Delay, Duration, Timer};
+use embedded_hal_bus::spi::ExclusiveDevice;
+use static_cell::make_static;
+use {defmt_rtt as _, panic_probe as _};
+
+bind_interrupts!(struct Irqs {
+    SPIM3 => spim::InterruptHandler<peripherals::SPI3>;
+    RNG => embassy_nrf::rng::InterruptHandler<peripherals::RNG>;
+});
+
+type MyDriver = Enc28j60<
+    ExclusiveDevice<Spim<'static, peripherals::SPI3>, Output<'static, peripherals::P0_15>, Delay>,
+    Output<'static, peripherals::P0_13>,
+>;
+
+#[embassy_executor::task]
+async fn net_task(stack: &'static Stack<MyDriver>) -> ! {
+    stack.run().await
+}
+
+#[embassy_executor::main]
+async fn main(spawner: Spawner) {
+    let p = embassy_nrf::init(Default::default());
+    info!("running!");
+
+    let eth_sck = p.P0_20;
+    let eth_mosi = p.P0_22;
+    let eth_miso = p.P0_24;
+    let eth_cs = p.P0_15;
+    let eth_rst = p.P0_13;
+    let _eth_irq = p.P0_12;
+
+    let mut config = spim::Config::default();
+    config.frequency = spim::Frequency::M16;
+    let spi = spim::Spim::new(p.SPI3, Irqs, eth_sck, eth_miso, eth_mosi, config);
+    let cs = Output::new(eth_cs, Level::High, OutputDrive::Standard);
+    let spi = ExclusiveDevice::new(spi, cs, Delay);
+
+    let rst = Output::new(eth_rst, Level::High, OutputDrive::Standard);
+    let mac_addr = [2, 3, 4, 5, 6, 7];
+    let device = Enc28j60::new(spi, Some(rst), mac_addr);
+
+    let config = embassy_net::Config::dhcpv4(Default::default());
+    // let config = embassy_net::Config::ipv4_static(embassy_net::StaticConfigV4 {
+    //    address: Ipv4Cidr::new(Ipv4Address::new(10, 42, 0, 61), 24),
+    //    dns_servers: Vec::new(),
+    //    gateway: Some(Ipv4Address::new(10, 42, 0, 1)),
+    // });
+
+    // Generate random seed
+    let mut rng = Rng::new(p.RNG, Irqs);
+    let mut seed = [0; 8];
+    rng.blocking_fill_bytes(&mut seed);
+    let seed = u64::from_le_bytes(seed);
+
+    // Init network stack
+    let stack = &*make_static!(Stack::new(
+        device,
+        config,
+        make_static!(StackResources::<2>::new()),
+        seed
+    ));
+
+    unwrap!(spawner.spawn(net_task(stack)));
+
+    info!("Waiting for DHCP up...");
+    while stack.config_v4().is_none() {
+        Timer::after(Duration::from_millis(100)).await;
+    }
+    info!("IP addressing up!");
+
+    let down = test_download(stack).await;
+    let up = test_upload(stack).await;
+    let updown = test_upload_download(stack).await;
+
+    assert!(down > TEST_EXPECTED_DOWNLOAD_KBPS);
+    assert!(up > TEST_EXPECTED_UPLOAD_KBPS);
+    assert!(updown > TEST_EXPECTED_UPLOAD_DOWNLOAD_KBPS);
+
+    info!("Test OK");
+    cortex_m::asm::bkpt();
+}
+
+const TEST_DURATION: usize = 10;
+const TEST_EXPECTED_DOWNLOAD_KBPS: usize = 200;
+const TEST_EXPECTED_UPLOAD_KBPS: usize = 200;
+const TEST_EXPECTED_UPLOAD_DOWNLOAD_KBPS: usize = 150;
+const RX_BUFFER_SIZE: usize = 4096;
+const TX_BUFFER_SIZE: usize = 4096;
+const SERVER_ADDRESS: Ipv4Address = Ipv4Address::new(192, 168, 2, 2);
+const DOWNLOAD_PORT: u16 = 4321;
+const UPLOAD_PORT: u16 = 4322;
+const UPLOAD_DOWNLOAD_PORT: u16 = 4323;
+
+async fn test_download(stack: &'static Stack<MyDriver>) -> usize {
+    info!("Testing download...");
+
+    let mut rx_buffer = [0; RX_BUFFER_SIZE];
+    let mut tx_buffer = [0; TX_BUFFER_SIZE];
+    let mut socket = TcpSocket::new(stack, &mut rx_buffer, &mut tx_buffer);
+    socket.set_timeout(Some(Duration::from_secs(10)));
+
+    info!("connecting to {:?}:{}...", SERVER_ADDRESS, DOWNLOAD_PORT);
+    if let Err(e) = socket.connect((SERVER_ADDRESS, DOWNLOAD_PORT)).await {
+        error!("connect error: {:?}", e);
+        return 0;
+    }
+    info!("connected, testing...");
+
+    let mut rx_buf = [0; 4096];
+    let mut total: usize = 0;
+    with_timeout(Duration::from_secs(TEST_DURATION as _), async {
+        loop {
+            match socket.read(&mut rx_buf).await {
+                Ok(0) => {
+                    error!("read EOF");
+                    return 0;
+                }
+                Ok(n) => total += n,
+                Err(e) => {
+                    error!("read error: {:?}", e);
+                    return 0;
+                }
+            }
+        }
+    })
+    .await
+    .ok();
+
+    let kbps = (total + 512) / 1024 / TEST_DURATION;
+    info!("download: {} kB/s", kbps);
+    kbps
+}
+
+async fn test_upload(stack: &'static Stack<MyDriver>) -> usize {
+    info!("Testing upload...");
+
+    let mut rx_buffer = [0; RX_BUFFER_SIZE];
+    let mut tx_buffer = [0; TX_BUFFER_SIZE];
+    let mut socket = TcpSocket::new(stack, &mut rx_buffer, &mut tx_buffer);
+    socket.set_timeout(Some(Duration::from_secs(10)));
+
+    info!("connecting to {:?}:{}...", SERVER_ADDRESS, UPLOAD_PORT);
+    if let Err(e) = socket.connect((SERVER_ADDRESS, UPLOAD_PORT)).await {
+        error!("connect error: {:?}", e);
+        return 0;
+    }
+    info!("connected, testing...");
+
+    let buf = [0; 4096];
+    let mut total: usize = 0;
+    with_timeout(Duration::from_secs(TEST_DURATION as _), async {
+        loop {
+            match socket.write(&buf).await {
+                Ok(0) => {
+                    error!("write zero?!??!?!");
+                    return 0;
+                }
+                Ok(n) => total += n,
+                Err(e) => {
+                    error!("write error: {:?}", e);
+                    return 0;
+                }
+            }
+        }
+    })
+    .await
+    .ok();
+
+    let kbps = (total + 512) / 1024 / TEST_DURATION;
+    info!("upload: {} kB/s", kbps);
+    kbps
+}
+
+async fn test_upload_download(stack: &'static Stack<MyDriver>) -> usize {
+    info!("Testing upload+download...");
+
+    let mut rx_buffer = [0; RX_BUFFER_SIZE];
+    let mut tx_buffer = [0; TX_BUFFER_SIZE];
+    let mut socket = TcpSocket::new(stack, &mut rx_buffer, &mut tx_buffer);
+    socket.set_timeout(Some(Duration::from_secs(10)));
+
+    info!("connecting to {:?}:{}...", SERVER_ADDRESS, UPLOAD_DOWNLOAD_PORT);
+    if let Err(e) = socket.connect((SERVER_ADDRESS, UPLOAD_DOWNLOAD_PORT)).await {
+        error!("connect error: {:?}", e);
+        return 0;
+    }
+    info!("connected, testing...");
+
+    let (mut reader, mut writer) = socket.split();
+
+    let tx_buf = [0; 4096];
+    let mut rx_buf = [0; 4096];
+    let mut total: usize = 0;
+    let tx_fut = async {
+        loop {
+            match writer.write(&tx_buf).await {
+                Ok(0) => {
+                    error!("write zero?!??!?!");
+                    return 0;
+                }
+                Ok(_) => {}
+                Err(e) => {
+                    error!("write error: {:?}", e);
+                    return 0;
+                }
+            }
+        }
+    };
+
+    let rx_fut = async {
+        loop {
+            match reader.read(&mut rx_buf).await {
+                Ok(0) => {
+                    error!("read EOF");
+                    return 0;
+                }
+                Ok(n) => total += n,
+                Err(e) => {
+                    error!("read error: {:?}", e);
+                    return 0;
+                }
+            }
+        }
+    };
+
+    with_timeout(Duration::from_secs(TEST_DURATION as _), join(tx_fut, rx_fut))
+        .await
+        .ok();
+
+    let kbps = (total + 512) / 1024 / TEST_DURATION;
+    info!("upload+download: {} kB/s", kbps);
+    kbps
+}

tests/nrf/src/bin/timer.rs

@@ -1,8 +1,7 @@
 #![no_std]
 #![no_main]
 #![feature(type_alias_impl_trait)]
-#[path = "../common.rs"]
-mod common;
+teleprobe_meta::target!(b"nrf52840-dk");
 
 use defmt::{assert, info};
 use embassy_executor::Spawner;

tests/nrf/src/bin/wifi_esp_hosted_perf.rs

@@ -1,9 +1,8 @@
 #![no_std]
 #![no_main]
 #![feature(type_alias_impl_trait)]
-
-#[path = "../common.rs"]
-mod common;
+teleprobe_meta::target!(b"nrf52840-dk");
+teleprobe_meta::timeout!(120);
 
 use defmt::{error, info, unwrap};
 use embassy_executor::Spawner;
@@ -19,8 +18,6 @@ use embedded_hal_bus::spi::ExclusiveDevice;
 use static_cell::make_static;
 use {defmt_rtt as _, embassy_net_esp_hosted as hosted, panic_probe as _};
 
-teleprobe_meta::timeout!(120);
-
 bind_interrupts!(struct Irqs {
     SPIM3 => spim::InterruptHandler<peripherals::SPI3>;
     RNG => embassy_nrf::rng::InterruptHandler<peripherals::RNG>;

tests/nrf/src/common.rs

@@ -1 +0,0 @@
-teleprobe_meta::target!(b"nrf52840-dk");