From b7c8d7a8d251ab63fba3cc964f1928a216c28081 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sun, 15 Jun 2025 16:55:09 -0400 Subject: rust: enable `clippy::cast_lossless` lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before Rust 1.29.0, Clippy introduced the `cast_lossless` lint [1]: > Rust’s `as` keyword will perform many kinds of conversions, including > silently lossy conversions. Conversion functions such as `i32::from` > will only perform lossless conversions. Using the conversion functions > prevents conversions from becoming silently lossy if the input types > ever change, and makes it clear for people reading the code that the > conversion is lossless. While this doesn't eliminate unchecked `as` conversions, it makes such conversions easier to scrutinize. It also has the slight benefit of removing a degree of freedom on which to bikeshed. Thus apply the changes and enable the lint -- no functional change intended. Link: https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless [1] Suggested-by: Benno Lossin Link: https://lore.kernel.org/all/D8ORTXSUTKGL.1KOJAGBM8F8TN@proton.me/ Reviewed-by: Benno Lossin Reviewed-by: Boqun Feng Signed-off-by: Tamir Duberstein Acked-by: FUJITA Tomonori Acked-by: Jocelyn Falempe Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250615-ptr-as-ptr-v12-5-f43b024581e8@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/net/phy.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel/net/phy.rs') diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 32ea43ece646..65ac4d59ad77 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -142,7 +142,7 @@ impl Device { // SAFETY: The struct invariant ensures that we may access // this field without additional synchronization. let bit_field = unsafe { &(*self.0.get())._bitfield_1 }; - bit_field.get(13, 1) == bindings::AUTONEG_ENABLE as u64 + bit_field.get(13, 1) == u64::from(bindings::AUTONEG_ENABLE) } /// Gets the current auto-negotiation state. @@ -427,7 +427,7 @@ impl Adapter { // where we hold `phy_device->lock`, so the accessors on // `Device` are okay to call. let dev = unsafe { Device::from_raw(phydev) }; - T::match_phy_device(dev) as i32 + T::match_phy_device(dev).into() } /// # Safety -- cgit From 22955d942f281e476f1db997c1fd4f24c6e3b693 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 25 Jun 2025 05:25:38 -0700 Subject: Use unqualified references to ffi types Remove unnecessary qualifications; `kernel::ffi::*` is included in `kernel::prelude`. Signed-off-by: Tamir Duberstein Reviewed-by: FUJITA Tomonori Link: https://patch.msgid.link/20250625-correct-type-cast-v2-1-6f2c29729e69@gmail.com Signed-off-by: Paolo Abeni --- rust/kernel/net/phy.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) (limited to 'rust/kernel/net/phy.rs') diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 32ea43ece646..0ea70b4c4ed9 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -312,9 +312,7 @@ impl Adapter { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn soft_reset_callback( - phydev: *mut bindings::phy_device, - ) -> crate::ffi::c_int { + unsafe extern "C" fn soft_reset_callback(phydev: *mut bindings::phy_device) -> c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on @@ -328,7 +326,7 @@ impl Adapter { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn probe_callback(phydev: *mut bindings::phy_device) -> crate::ffi::c_int { + unsafe extern "C" fn probe_callback(phydev: *mut bindings::phy_device) -> c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we can exclusively access `phy_device` because @@ -343,9 +341,7 @@ impl Adapter { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn get_features_callback( - phydev: *mut bindings::phy_device, - ) -> crate::ffi::c_int { + unsafe extern "C" fn get_features_callback(phydev: *mut bindings::phy_device) -> c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on @@ -359,7 +355,7 @@ impl Adapter { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn suspend_callback(phydev: *mut bindings::phy_device) -> crate::ffi::c_int { + unsafe extern "C" fn suspend_callback(phydev: *mut bindings::phy_device) -> c_int { from_result(|| { // SAFETY: The C core code ensures that the accessors on // `Device` are okay to call even though `phy_device->lock` @@ -373,7 +369,7 @@ impl Adapter { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn resume_callback(phydev: *mut bindings::phy_device) -> crate::ffi::c_int { + unsafe extern "C" fn resume_callback(phydev: *mut bindings::phy_device) -> c_int { from_result(|| { // SAFETY: The C core code ensures that the accessors on // `Device` are okay to call even though `phy_device->lock` @@ -387,9 +383,7 @@ impl Adapter { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn config_aneg_callback( - phydev: *mut bindings::phy_device, - ) -> crate::ffi::c_int { + unsafe extern "C" fn config_aneg_callback(phydev: *mut bindings::phy_device) -> c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on @@ -403,9 +397,7 @@ impl Adapter { /// # Safety /// /// `phydev` must be passed by the corresponding callback in `phy_driver`. - unsafe extern "C" fn read_status_callback( - phydev: *mut bindings::phy_device, - ) -> crate::ffi::c_int { + unsafe extern "C" fn read_status_callback(phydev: *mut bindings::phy_device) -> c_int { from_result(|| { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on @@ -422,7 +414,7 @@ impl Adapter { unsafe extern "C" fn match_phy_device_callback( phydev: *mut bindings::phy_device, _phydrv: *const bindings::phy_driver, - ) -> crate::ffi::c_int { + ) -> c_int { // SAFETY: This callback is called only in contexts // where we hold `phy_device->lock`, so the accessors on // `Device` are okay to call. -- cgit From c9a7bcd2c016ac814fd56e5a2b0946fb14960de4 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 25 Jun 2025 05:25:39 -0700 Subject: Cast to the proper type Use the ffi type rather than the resolved underlying type. Acked-by: FUJITA Tomonori Signed-off-by: Tamir Duberstein Link: https://patch.msgid.link/20250625-correct-type-cast-v2-2-6f2c29729e69@gmail.com Signed-off-by: Paolo Abeni --- rust/kernel/net/phy.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'rust/kernel/net/phy.rs') diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 0ea70b4c4ed9..b37de09651cb 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -163,20 +163,20 @@ impl Device { let phydev = self.0.get(); // SAFETY: The struct invariant ensures that we may access // this field without additional synchronization. - unsafe { (*phydev).speed = speed as i32 }; + unsafe { (*phydev).speed = speed as c_int }; } /// Sets duplex mode. pub fn set_duplex(&mut self, mode: DuplexMode) { let phydev = self.0.get(); let v = match mode { - DuplexMode::Full => bindings::DUPLEX_FULL as i32, - DuplexMode::Half => bindings::DUPLEX_HALF as i32, - DuplexMode::Unknown => bindings::DUPLEX_UNKNOWN as i32, + DuplexMode::Full => bindings::DUPLEX_FULL, + DuplexMode::Half => bindings::DUPLEX_HALF, + DuplexMode::Unknown => bindings::DUPLEX_UNKNOWN, }; // SAFETY: The struct invariant ensures that we may access // this field without additional synchronization. - unsafe { (*phydev).duplex = v }; + unsafe { (*phydev).duplex = v as c_int }; } /// Reads a PHY register. -- cgit From 2f5606afa4c2bcabd45cb34c92faf93ca5ffe75e Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 11 Jul 2025 08:04:37 +0000 Subject: device: rust: rename Device::as_ref() to Device::from_raw() The prefix as_* should not be used for a constructor. Constructors usually use the prefix from_* instead. Some prior art in the stdlib: Box::from_raw, CString::from_raw, Rc::from_raw, Arc::from_raw, Waker::from_raw, File::from_raw_fd. There is also prior art in the kernel crate: cpufreq::Policy::from_raw, fs::File::from_raw_file, Kuid::from_raw, ARef::from_raw, SeqFile::from_raw, VmaNew::from_raw, Io::from_raw. Link: https://lore.kernel.org/r/aCd8D5IA0RXZvtcv@pollux Signed-off-by: Alice Ryhl Reviewed-by: Benno Lossin Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250711-device-as-ref-v2-1-1b16ab6402d7@google.com Signed-off-by: Danilo Krummrich --- rust/kernel/net/phy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel/net/phy.rs') diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 32ea43ece646..bd43a726f7d3 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -285,7 +285,7 @@ impl AsRef for Device { fn as_ref(&self) -> &kernel::device::Device { let phydev = self.0.get(); // SAFETY: The struct invariant ensures that `mdio.dev` is valid. - unsafe { kernel::device::Device::as_ref(addr_of_mut!((*phydev).mdio.dev)) } + unsafe { kernel::device::Device::from_raw(addr_of_mut!((*phydev).mdio.dev)) } } } -- cgit From f65a3218fd92cbe3e00f25427e1c9255b8973b31 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Fri, 11 Jul 2025 13:09:46 +0900 Subject: rust: net::phy represent DeviceId as transparent wrapper over mdio_device_id Refactor the DeviceId struct to be a #[repr(transparent)] wrapper around the C struct bindings::mdio_device_id. This refactoring is a preparation for enabling the PHY abstractions to use the RawDeviceId trait. Acked-by: Jakub Kicinski Reviewed-by: Trevor Gross Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250711040947.1252162-3-fujita.tomonori@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/net/phy.rs | 55 +++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 27 deletions(-) (limited to 'rust/kernel/net/phy.rs') diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index bd43a726f7d3..11a6809f3bea 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -507,7 +507,7 @@ pub const fn create_phy_driver() -> DriverVTable { DriverVTable(Opaque::new(bindings::phy_driver { name: T::NAME.as_char_ptr().cast_mut(), flags: T::FLAGS, - phy_id: T::PHY_DEVICE_ID.id, + phy_id: T::PHY_DEVICE_ID.id(), phy_id_mask: T::PHY_DEVICE_ID.mask_as_int(), soft_reset: if T::HAS_SOFT_RESET { Some(Adapter::::soft_reset_callback) @@ -691,42 +691,41 @@ impl Drop for Registration { /// /// Represents the kernel's `struct mdio_device_id`. This is used to find an appropriate /// PHY driver. -pub struct DeviceId { - id: u32, - mask: DeviceMask, -} +#[repr(transparent)] +#[derive(Clone, Copy)] +pub struct DeviceId(bindings::mdio_device_id); impl DeviceId { /// Creates a new instance with the exact match mask. pub const fn new_with_exact_mask(id: u32) -> Self { - DeviceId { - id, - mask: DeviceMask::Exact, - } + Self(bindings::mdio_device_id { + phy_id: id, + phy_id_mask: DeviceMask::Exact.as_int(), + }) } /// Creates a new instance with the model match mask. pub const fn new_with_model_mask(id: u32) -> Self { - DeviceId { - id, - mask: DeviceMask::Model, - } + Self(bindings::mdio_device_id { + phy_id: id, + phy_id_mask: DeviceMask::Model.as_int(), + }) } /// Creates a new instance with the vendor match mask. pub const fn new_with_vendor_mask(id: u32) -> Self { - DeviceId { - id, - mask: DeviceMask::Vendor, - } + Self(bindings::mdio_device_id { + phy_id: id, + phy_id_mask: DeviceMask::Vendor.as_int(), + }) } /// Creates a new instance with a custom match mask. pub const fn new_with_custom_mask(id: u32, mask: u32) -> Self { - DeviceId { - id, - mask: DeviceMask::Custom(mask), - } + Self(bindings::mdio_device_id { + phy_id: id, + phy_id_mask: DeviceMask::Custom(mask).as_int(), + }) } /// Creates a new instance from [`Driver`]. @@ -734,18 +733,20 @@ impl DeviceId { T::PHY_DEVICE_ID } - /// Get a `mask` as u32. + /// Get the MDIO device's PHY ID. + pub const fn id(&self) -> u32 { + self.0.phy_id + } + + /// Get the MDIO device's match mask. pub const fn mask_as_int(&self) -> u32 { - self.mask.as_int() + self.0.phy_id_mask } // macro use only #[doc(hidden)] pub const fn mdio_device_id(&self) -> bindings::mdio_device_id { - bindings::mdio_device_id { - phy_id: self.id, - phy_id_mask: self.mask.as_int(), - } + self.0 } } -- cgit From 9a8682f0875b8cedad42bdfe601e6ab204fad06d Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Fri, 11 Jul 2025 13:09:47 +0900 Subject: rust: net::phy Change module_phy_driver macro to use module_device_table macro Change module_phy_driver macro to build device tables which are exported to userspace by using module_device_table macro. Acked-by: Jakub Kicinski Reviewed-by: Trevor Gross Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250711040947.1252162-4-fujita.tomonori@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/net/phy.rs | 51 ++++++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 27 deletions(-) (limited to 'rust/kernel/net/phy.rs') diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 11a6809f3bea..95bd17d3e927 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -6,7 +6,7 @@ //! //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h). -use crate::{error::*, prelude::*, types::Opaque}; +use crate::{device_id::RawDeviceId, error::*, prelude::*, types::Opaque}; use core::{marker::PhantomData, ptr::addr_of_mut}; pub mod reg; @@ -750,6 +750,12 @@ impl DeviceId { } } +// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct mdio_device_id` +// and does not add additional invariants, so it's safe to transmute to `RawType`. +unsafe impl RawDeviceId for DeviceId { + type RawType = bindings::mdio_device_id; +} + enum DeviceMask { Exact, Model, @@ -850,19 +856,18 @@ impl DeviceMask { /// } /// }; /// -/// const _DEVICE_TABLE: [::kernel::bindings::mdio_device_id; 2] = [ -/// ::kernel::bindings::mdio_device_id { -/// phy_id: 0x00000001, -/// phy_id_mask: 0xffffffff, -/// }, -/// ::kernel::bindings::mdio_device_id { -/// phy_id: 0, -/// phy_id_mask: 0, -/// }, -/// ]; -/// #[cfg(MODULE)] -/// #[no_mangle] -/// static __mod_device_table__mdio__phydev: [::kernel::bindings::mdio_device_id; 2] = _DEVICE_TABLE; +/// const N: usize = 1; +/// +/// const TABLE: ::kernel::device_id::IdArray<::kernel::net::phy::DeviceId, (), N> = +/// ::kernel::device_id::IdArray::new_without_index([ +/// ::kernel::net::phy::DeviceId( +/// ::kernel::bindings::mdio_device_id { +/// phy_id: 0x00000001, +/// phy_id_mask: 0xffffffff, +/// }), +/// ]); +/// +/// ::kernel::module_device_table!("mdio", phydev, TABLE); /// ``` #[macro_export] macro_rules! module_phy_driver { @@ -873,20 +878,12 @@ macro_rules! module_phy_driver { }; (@device_table [$($dev:expr),+]) => { - // SAFETY: C will not read off the end of this constant since the last element is zero. - const _DEVICE_TABLE: [$crate::bindings::mdio_device_id; - $crate::module_phy_driver!(@count_devices $($dev),+) + 1] = [ - $($dev.mdio_device_id()),+, - $crate::bindings::mdio_device_id { - phy_id: 0, - phy_id_mask: 0 - } - ]; + const N: usize = $crate::module_phy_driver!(@count_devices $($dev),+); + + const TABLE: $crate::device_id::IdArray<$crate::net::phy::DeviceId, (), N> = + $crate::device_id::IdArray::new_without_index([ $(($dev,())),+, ]); - #[cfg(MODULE)] - #[no_mangle] - static __mod_device_table__mdio__phydev: [$crate::bindings::mdio_device_id; - $crate::module_phy_driver!(@count_devices $($dev),+) + 1] = _DEVICE_TABLE; + $crate::module_device_table!("mdio", phydev, TABLE); }; (drivers: [$($driver:ident),+ $(,)?], device_table: [$($dev:expr),+ $(,)?], $($f:tt)*) => { -- cgit