From b1245858f355e764a17eda819198f68ad83883ab Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Sun, 26 Jan 2025 22:42:13 +0100 Subject: [PATCH] stm32/eth: rename PHY->Phy, GenericSMI -> GenericPhy. Remove unneeded unsafes. We shouldn't use `unsafe` to mark merely "dangerous" actions, only actions that actually cause UB. --- .../eth/{generic_smi.rs => generic_phy.rs} | 10 ++--- embassy-stm32/src/eth/mod.rs | 38 +++++++++---------- embassy-stm32/src/eth/v1/mod.rs | 10 ++--- embassy-stm32/src/eth/v2/mod.rs | 8 ++-- examples/stm32f4/src/bin/eth.rs | 7 ++-- .../stm32f4/src/bin/eth_compliance_test.rs | 7 ++-- examples/stm32f7/src/bin/eth.rs | 7 ++-- examples/stm32h5/src/bin/eth.rs | 7 ++-- examples/stm32h7/src/bin/eth.rs | 7 ++-- examples/stm32h7/src/bin/eth_client.rs | 7 ++-- examples/stm32h7/src/bin/eth_client_mii.rs | 7 ++-- tests/stm32/src/bin/eth.rs | 7 ++-- 12 files changed, 56 insertions(+), 66 deletions(-) rename embassy-stm32/src/eth/{generic_smi.rs => generic_phy.rs} (97%) diff --git a/embassy-stm32/src/eth/generic_smi.rs b/embassy-stm32/src/eth/generic_phy.rs similarity index 97% rename from embassy-stm32/src/eth/generic_smi.rs rename to embassy-stm32/src/eth/generic_phy.rs index 239c52634c..774beef802 100644 --- a/embassy-stm32/src/eth/generic_smi.rs +++ b/embassy-stm32/src/eth/generic_phy.rs @@ -7,7 +7,7 @@ use embassy_time::{Duration, Timer}; #[cfg(feature = "time")] use futures_util::FutureExt; -use super::{StationManagement, PHY}; +use super::{Phy, StationManagement}; #[allow(dead_code)] mod phy_consts { @@ -43,13 +43,13 @@ mod phy_consts { use self::phy_consts::*; /// Generic SMI Ethernet PHY implementation -pub struct GenericSMI { +pub struct GenericPhy { phy_addr: u8, #[cfg(feature = "time")] poll_interval: Duration, } -impl GenericSMI { +impl GenericPhy { /// Construct the PHY. It assumes the address `phy_addr` in the SMI communication /// /// # Panics @@ -89,7 +89,7 @@ fn blocking_delay_us(us: u32) { } } -unsafe impl PHY for GenericSMI { +impl Phy for GenericPhy { fn phy_reset(&mut self, sm: &mut S) { // Detect SMI address if self.phy_addr == 0xFF { @@ -148,7 +148,7 @@ unsafe impl PHY for GenericSMI { } /// Public functions for the PHY -impl GenericSMI { +impl GenericPhy { /// Set the SMI polling interval. #[cfg(feature = "time")] pub fn set_poll_interval(&mut self, poll_interval: Duration) { diff --git a/embassy-stm32/src/eth/mod.rs b/embassy-stm32/src/eth/mod.rs index 773452bf2a..109ceeeb3a 100644 --- a/embassy-stm32/src/eth/mod.rs +++ b/embassy-stm32/src/eth/mod.rs @@ -4,7 +4,7 @@ #[cfg_attr(any(eth_v1a, eth_v1b, eth_v1c), path = "v1/mod.rs")] #[cfg_attr(eth_v2, path = "v2/mod.rs")] mod _version; -pub mod generic_smi; +mod generic_phy; use core::mem::MaybeUninit; use core::task::Context; @@ -13,6 +13,7 @@ use embassy_net_driver::{Capabilities, HardwareAddress, LinkState}; use embassy_sync::waitqueue::AtomicWaker; pub use self::_version::{InterruptHandler, *}; +pub use self::generic_phy::*; use crate::rcc::RccPeripheral; #[allow(unused)] @@ -71,7 +72,7 @@ impl PacketQueue { static WAKER: AtomicWaker = AtomicWaker::new(); -impl<'d, T: Instance, P: PHY> embassy_net_driver::Driver for Ethernet<'d, T, P> { +impl<'d, T: Instance, P: Phy> embassy_net_driver::Driver for Ethernet<'d, T, P> { type RxToken<'a> = RxToken<'a, 'd> where @@ -156,23 +157,15 @@ impl<'a, 'd> embassy_net_driver::TxToken for TxToken<'a, 'd> { } /// Station Management Interface (SMI) on an ethernet PHY -/// -/// # Safety -/// -/// The methods cannot move out of self -pub unsafe trait StationManagement { +pub trait StationManagement { /// Read a register over SMI. fn smi_read(&mut self, phy_addr: u8, reg: u8) -> u16; /// Write a register over SMI. fn smi_write(&mut self, phy_addr: u8, reg: u8, val: u16); } -/// Traits for an Ethernet PHY -/// -/// # Safety -/// -/// The methods cannot move S -pub unsafe trait PHY { +/// Trait for an Ethernet PHY +pub trait Phy { /// Reset PHY and wait for it to come out of reset. fn phy_reset(&mut self, sm: &mut S); /// PHY initialisation. @@ -181,18 +174,23 @@ pub unsafe trait PHY { fn poll_link(&mut self, sm: &mut S, cx: &mut Context) -> bool; } -impl<'d, T: Instance, P: PHY> Ethernet<'d, T, P> { +impl<'d, T: Instance, P: Phy> Ethernet<'d, T, P> { /// Directly expose the SMI interface used by the Ethernet driver. /// /// This can be used to for example configure special PHY registers for compliance testing. - /// - /// # Safety - /// - /// Revert any temporary PHY register changes such as to enable test modes before handing - /// the Ethernet device over to the networking stack otherwise things likely won't work. - pub unsafe fn station_management(&mut self) -> &mut impl StationManagement { + pub fn station_management(&mut self) -> &mut impl StationManagement { &mut self.station_management } + + /// Access the user-supplied `Phy`. + pub fn phy(&self) -> &P { + &self.phy + } + + /// Mutably access the user-supplied `Phy`. + pub fn phy_mut(&mut self) -> &mut P { + &mut self.phy + } } trait SealedInstance { diff --git a/embassy-stm32/src/eth/v1/mod.rs b/embassy-stm32/src/eth/v1/mod.rs index 438b28020a..e12ac2fef4 100644 --- a/embassy-stm32/src/eth/v1/mod.rs +++ b/embassy-stm32/src/eth/v1/mod.rs @@ -46,7 +46,7 @@ impl interrupt::typelevel::Handler for InterruptHandl } /// Ethernet driver. -pub struct Ethernet<'d, T: Instance, P: PHY> { +pub struct Ethernet<'d, T: Instance, P: Phy> { _peri: PeripheralRef<'d, T>, pub(crate) tx: TDesRing<'d>, pub(crate) rx: RDesRing<'d>, @@ -91,7 +91,7 @@ macro_rules! config_pins { }; } -impl<'d, T: Instance, P: PHY> Ethernet<'d, T, P> { +impl<'d, T: Instance, P: Phy> Ethernet<'d, T, P> { /// safety: the returned instance is not leak-safe pub fn new( queue: &'d mut PacketQueue, @@ -272,12 +272,12 @@ impl<'d, T: Instance, P: PHY> Ethernet<'d, T, P> { } /// Ethernet station management interface. -pub struct EthernetStationManagement { +pub(crate) struct EthernetStationManagement { peri: PhantomData, clock_range: Cr, } -unsafe impl StationManagement for EthernetStationManagement { +impl StationManagement for EthernetStationManagement { fn smi_read(&mut self, phy_addr: u8, reg: u8) -> u16 { let mac = T::regs().ethernet_mac(); @@ -307,7 +307,7 @@ unsafe impl StationManagement for EthernetStationManagement { } } -impl<'d, T: Instance, P: PHY> Drop for Ethernet<'d, T, P> { +impl<'d, T: Instance, P: Phy> Drop for Ethernet<'d, T, P> { fn drop(&mut self) { let dma = T::regs().ethernet_dma(); let mac = T::regs().ethernet_mac(); diff --git a/embassy-stm32/src/eth/v2/mod.rs b/embassy-stm32/src/eth/v2/mod.rs index 9dd7f7d958..26e4eeb634 100644 --- a/embassy-stm32/src/eth/v2/mod.rs +++ b/embassy-stm32/src/eth/v2/mod.rs @@ -36,7 +36,7 @@ impl interrupt::typelevel::Handler for InterruptHandl } /// Ethernet driver. -pub struct Ethernet<'d, T: Instance, P: PHY> { +pub struct Ethernet<'d, T: Instance, P: Phy> { _peri: PeripheralRef<'d, T>, pub(crate) tx: TDesRing<'d>, pub(crate) rx: RDesRing<'d>, @@ -63,7 +63,7 @@ macro_rules! config_pins { }; } -impl<'d, T: Instance, P: PHY> Ethernet<'d, T, P> { +impl<'d, T: Instance, P: Phy> Ethernet<'d, T, P> { /// Create a new RMII ethernet driver using 9 pins. pub fn new( queue: &'d mut PacketQueue, @@ -304,7 +304,7 @@ pub struct EthernetStationManagement { clock_range: u8, } -unsafe impl StationManagement for EthernetStationManagement { +impl StationManagement for EthernetStationManagement { fn smi_read(&mut self, phy_addr: u8, reg: u8) -> u16 { let mac = T::regs().ethernet_mac(); @@ -334,7 +334,7 @@ unsafe impl StationManagement for EthernetStationManagement { } } -impl<'d, T: Instance, P: PHY> Drop for Ethernet<'d, T, P> { +impl<'d, T: Instance, P: Phy> Drop for Ethernet<'d, T, P> { fn drop(&mut self) { let dma = T::regs().ethernet_dma(); let mac = T::regs().ethernet_mac(); diff --git a/examples/stm32f4/src/bin/eth.rs b/examples/stm32f4/src/bin/eth.rs index a3af8f75c0..634d8e2c69 100644 --- a/examples/stm32f4/src/bin/eth.rs +++ b/examples/stm32f4/src/bin/eth.rs @@ -5,8 +5,7 @@ use defmt::*; use embassy_executor::Spawner; use embassy_net::tcp::TcpSocket; use embassy_net::{Ipv4Address, StackResources}; -use embassy_stm32::eth::generic_smi::GenericSMI; -use embassy_stm32::eth::{Ethernet, PacketQueue}; +use embassy_stm32::eth::{Ethernet, GenericPhy, PacketQueue}; use embassy_stm32::peripherals::ETH; use embassy_stm32::rng::Rng; use embassy_stm32::time::Hertz; @@ -21,7 +20,7 @@ bind_interrupts!(struct Irqs { HASH_RNG => rng::InterruptHandler; }); -type Device = Ethernet<'static, ETH, GenericSMI>; +type Device = Ethernet<'static, ETH, GenericPhy>; #[embassy_executor::task] async fn net_task(mut runner: embassy_net::Runner<'static, Device>) -> ! { @@ -76,7 +75,7 @@ async fn main(spawner: Spawner) -> ! { p.PG13, p.PB13, p.PG11, - GenericSMI::new_auto(), + GenericPhy::new_auto(), mac_addr, ); diff --git a/examples/stm32f4/src/bin/eth_compliance_test.rs b/examples/stm32f4/src/bin/eth_compliance_test.rs index 5946fed791..52f9d57f63 100644 --- a/examples/stm32f4/src/bin/eth_compliance_test.rs +++ b/examples/stm32f4/src/bin/eth_compliance_test.rs @@ -3,8 +3,7 @@ use defmt::*; use embassy_executor::Spawner; -use embassy_stm32::eth::generic_smi::GenericSMI; -use embassy_stm32::eth::{Ethernet, PacketQueue, StationManagement}; +use embassy_stm32::eth::{Ethernet, GenericPhy, PacketQueue, StationManagement}; use embassy_stm32::time::Hertz; use embassy_stm32::{bind_interrupts, eth, peripherals, rng, Config}; use embassy_time::Timer; @@ -59,11 +58,11 @@ async fn main(_spawner: Spawner) -> ! { p.PG13, p.PB13, p.PG11, - GenericSMI::new(PHY_ADDR), + GenericPhy::new(PHY_ADDR), mac_addr, ); - let sm = unsafe { device.station_management() }; + let sm = device.station_management(); // Just an example. Exact register settings depend on the specific PHY and test. sm.smi_write(PHY_ADDR, 0, 0x2100); diff --git a/examples/stm32f7/src/bin/eth.rs b/examples/stm32f7/src/bin/eth.rs index f353af6741..17ab7fc005 100644 --- a/examples/stm32f7/src/bin/eth.rs +++ b/examples/stm32f7/src/bin/eth.rs @@ -5,8 +5,7 @@ use defmt::*; use embassy_executor::Spawner; use embassy_net::tcp::TcpSocket; use embassy_net::{Ipv4Address, StackResources}; -use embassy_stm32::eth::generic_smi::GenericSMI; -use embassy_stm32::eth::{Ethernet, PacketQueue}; +use embassy_stm32::eth::{Ethernet, GenericPhy, PacketQueue}; use embassy_stm32::peripherals::ETH; use embassy_stm32::rng::Rng; use embassy_stm32::time::Hertz; @@ -22,7 +21,7 @@ bind_interrupts!(struct Irqs { HASH_RNG => rng::InterruptHandler; }); -type Device = Ethernet<'static, ETH, GenericSMI>; +type Device = Ethernet<'static, ETH, GenericPhy>; #[embassy_executor::task] async fn net_task(mut runner: embassy_net::Runner<'static, Device>) -> ! { @@ -77,7 +76,7 @@ async fn main(spawner: Spawner) -> ! { p.PG13, p.PB13, p.PG11, - GenericSMI::new_auto(), + GenericPhy::new_auto(), mac_addr, ); diff --git a/examples/stm32h5/src/bin/eth.rs b/examples/stm32h5/src/bin/eth.rs index ead3467413..4034b552c2 100644 --- a/examples/stm32h5/src/bin/eth.rs +++ b/examples/stm32h5/src/bin/eth.rs @@ -5,8 +5,7 @@ use defmt::*; use embassy_executor::Spawner; use embassy_net::tcp::TcpSocket; use embassy_net::{Ipv4Address, StackResources}; -use embassy_stm32::eth::generic_smi::GenericSMI; -use embassy_stm32::eth::{Ethernet, PacketQueue}; +use embassy_stm32::eth::{Ethernet, GenericPhy, PacketQueue}; use embassy_stm32::peripherals::ETH; use embassy_stm32::rcc::{ AHBPrescaler, APBPrescaler, Hse, HseMode, Pll, PllDiv, PllMul, PllPreDiv, PllSource, Sysclk, VoltageScale, @@ -25,7 +24,7 @@ bind_interrupts!(struct Irqs { RNG => rng::InterruptHandler; }); -type Device = Ethernet<'static, ETH, GenericSMI>; +type Device = Ethernet<'static, ETH, GenericPhy>; #[embassy_executor::task] async fn net_task(mut runner: embassy_net::Runner<'static, Device>) -> ! { @@ -80,7 +79,7 @@ async fn main(spawner: Spawner) -> ! { p.PG13, p.PB15, p.PG11, - GenericSMI::new_auto(), + GenericPhy::new_auto(), mac_addr, ); diff --git a/examples/stm32h7/src/bin/eth.rs b/examples/stm32h7/src/bin/eth.rs index 6665cd1d0a..da7aa4af51 100644 --- a/examples/stm32h7/src/bin/eth.rs +++ b/examples/stm32h7/src/bin/eth.rs @@ -5,8 +5,7 @@ use defmt::*; use embassy_executor::Spawner; use embassy_net::tcp::TcpSocket; use embassy_net::{Ipv4Address, StackResources}; -use embassy_stm32::eth::generic_smi::GenericSMI; -use embassy_stm32::eth::{Ethernet, PacketQueue}; +use embassy_stm32::eth::{Ethernet, GenericPhy, PacketQueue}; use embassy_stm32::peripherals::ETH; use embassy_stm32::rng::Rng; use embassy_stm32::{bind_interrupts, eth, peripherals, rng, Config}; @@ -21,7 +20,7 @@ bind_interrupts!(struct Irqs { RNG => rng::InterruptHandler; }); -type Device = Ethernet<'static, ETH, GenericSMI>; +type Device = Ethernet<'static, ETH, GenericPhy>; #[embassy_executor::task] async fn net_task(mut runner: embassy_net::Runner<'static, Device>) -> ! { @@ -79,7 +78,7 @@ async fn main(spawner: Spawner) -> ! { p.PG13, // TX_D0: Transmit Bit 0 p.PB13, // TX_D1: Transmit Bit 1 p.PG11, // TX_EN: Transmit Enable - GenericSMI::new_auto(), + GenericPhy::new_auto(), mac_addr, ); diff --git a/examples/stm32h7/src/bin/eth_client.rs b/examples/stm32h7/src/bin/eth_client.rs index 4fbe10f318..10485109ad 100644 --- a/examples/stm32h7/src/bin/eth_client.rs +++ b/examples/stm32h7/src/bin/eth_client.rs @@ -7,8 +7,7 @@ use defmt::*; use embassy_executor::Spawner; use embassy_net::tcp::client::{TcpClient, TcpClientState}; use embassy_net::StackResources; -use embassy_stm32::eth::generic_smi::GenericSMI; -use embassy_stm32::eth::{Ethernet, PacketQueue}; +use embassy_stm32::eth::{Ethernet, GenericPhy, PacketQueue}; use embassy_stm32::peripherals::ETH; use embassy_stm32::rng::Rng; use embassy_stm32::{bind_interrupts, eth, peripherals, rng, Config}; @@ -24,7 +23,7 @@ bind_interrupts!(struct Irqs { RNG => rng::InterruptHandler; }); -type Device = Ethernet<'static, ETH, GenericSMI>; +type Device = Ethernet<'static, ETH, GenericPhy>; #[embassy_executor::task] async fn net_task(mut runner: embassy_net::Runner<'static, Device>) -> ! { @@ -81,7 +80,7 @@ async fn main(spawner: Spawner) -> ! { p.PG13, p.PB13, p.PG11, - GenericSMI::new_auto(), + GenericPhy::new_auto(), mac_addr, ); diff --git a/examples/stm32h7/src/bin/eth_client_mii.rs b/examples/stm32h7/src/bin/eth_client_mii.rs index 53f86ac80f..8491736151 100644 --- a/examples/stm32h7/src/bin/eth_client_mii.rs +++ b/examples/stm32h7/src/bin/eth_client_mii.rs @@ -7,8 +7,7 @@ use defmt::*; use embassy_executor::Spawner; use embassy_net::tcp::client::{TcpClient, TcpClientState}; use embassy_net::StackResources; -use embassy_stm32::eth::generic_smi::GenericSMI; -use embassy_stm32::eth::{Ethernet, PacketQueue}; +use embassy_stm32::eth::{Ethernet, GenericPhy, PacketQueue}; use embassy_stm32::peripherals::ETH; use embassy_stm32::rng::Rng; use embassy_stm32::{bind_interrupts, eth, peripherals, rng, Config}; @@ -24,7 +23,7 @@ bind_interrupts!(struct Irqs { RNG => rng::InterruptHandler; }); -type Device = Ethernet<'static, ETH, GenericSMI>; +type Device = Ethernet<'static, ETH, GenericPhy>; #[embassy_executor::task] async fn net_task(mut runner: embassy_net::Runner<'static, Device>) -> ! { @@ -86,7 +85,7 @@ async fn main(spawner: Spawner) -> ! { p.PC2, p.PE2, p.PG11, - GenericSMI::new_auto(), + GenericPhy::new_auto(), mac_addr, ); info!("Device created"); diff --git a/tests/stm32/src/bin/eth.rs b/tests/stm32/src/bin/eth.rs index 4ab6e234f2..a7e76fd8e8 100644 --- a/tests/stm32/src/bin/eth.rs +++ b/tests/stm32/src/bin/eth.rs @@ -7,8 +7,7 @@ mod common; use common::*; use embassy_executor::Spawner; use embassy_net::StackResources; -use embassy_stm32::eth::generic_smi::GenericSMI; -use embassy_stm32::eth::{Ethernet, PacketQueue}; +use embassy_stm32::eth::{Ethernet, GenericPhy, PacketQueue}; use embassy_stm32::peripherals::ETH; use embassy_stm32::rng::Rng; use embassy_stm32::{bind_interrupts, eth, peripherals, rng}; @@ -29,7 +28,7 @@ bind_interrupts!(struct Irqs { RNG => rng::InterruptHandler; }); -type Device = Ethernet<'static, ETH, GenericSMI>; +type Device = Ethernet<'static, ETH, GenericPhy>; #[embassy_executor::task] async fn net_task(mut runner: embassy_net::Runner<'static, Device>) -> ! { @@ -87,7 +86,7 @@ async fn main(spawner: Spawner) { #[cfg(feature = "stm32h563zi")] p.PB15, p.PG11, - GenericSMI::new_auto(), + GenericPhy::new_auto(), mac_addr, );