| Message ID | 20240817051939.77735-4-fujita.tomonori@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: add Applied Micro QT2025 PHY driver | expand |
On 17.08.24 07:19, FUJITA Tomonori wrote: > Implement AsRef<kernel::device::Device> trait for Device. A PHY driver > needs a reference to device::Device to call the firmware API. > > Reviewed-by: Alice Ryhl <aliceryhl@google.com> > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> > --- > rust/kernel/net/phy.rs | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs > index 5e8137a1972f..569dd3beef9c 100644 > --- a/rust/kernel/net/phy.rs > +++ b/rust/kernel/net/phy.rs > @@ -7,8 +7,7 @@ > //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h). > > use crate::{error::*, prelude::*, types::Opaque}; > - > -use core::marker::PhantomData; > +use core::{marker::PhantomData, ptr::addr_of_mut}; > > /// PHY state machine states. > /// > @@ -302,6 +301,15 @@ pub fn genphy_read_abilities(&mut self) -> Result { > } > } > > +impl AsRef<kernel::device::Device> for Device { > + fn as_ref(&self) -> &kernel::device::Device { > + let phydev = self.0.get(); > + // SAFETY: The struct invariant ensures that we may access > + // this field without additional synchronization. I don't see this invariant on `phy::Device`. --- Cheers, Benno > + unsafe { kernel::device::Device::as_ref(addr_of_mut!((*phydev).mdio.dev)) } > + } > +} > + > /// Defines certain other features this PHY supports (like interrupts). > /// > /// These flag values are used in [`Driver::FLAGS`]. > -- > 2.34.1 >
On Sat, 17 Aug 2024 13:30:15 +0000 Benno Lossin <benno.lossin@proton.me> wrote: >> +impl AsRef<kernel::device::Device> for Device { >> + fn as_ref(&self) -> &kernel::device::Device { >> + let phydev = self.0.get(); >> + // SAFETY: The struct invariant ensures that we may access >> + // this field without additional synchronization. > > I don't see this invariant on `phy::Device`. You meant that `phy::Device` Invariants says that all methods defined on this struct are safe to call; not about accessing a field so the above SAFETY comment isn't correct, right? > --- > Cheers, > Benno > >> + unsafe { kernel::device::Device::as_ref(addr_of_mut!((*phydev).mdio.dev)) } >> + } >> +} SAFETY: A valid `phy_device` always have a valid `mdio.dev`. Better?
On 18.08.24 04:13, FUJITA Tomonori wrote: > On Sat, 17 Aug 2024 13:30:15 +0000 > Benno Lossin <benno.lossin@proton.me> wrote: > >>> +impl AsRef<kernel::device::Device> for Device { >>> + fn as_ref(&self) -> &kernel::device::Device { >>> + let phydev = self.0.get(); >>> + // SAFETY: The struct invariant ensures that we may access >>> + // this field without additional synchronization. >> >> I don't see this invariant on `phy::Device`. > > You meant that `phy::Device` Invariants says that all methods defined > on this struct are safe to call; not about accessing a field so the > above SAFETY comment isn't correct, right? Correct. >> --- >> Cheers, >> Benno >> >>> + unsafe { kernel::device::Device::as_ref(addr_of_mut!((*phydev).mdio.dev)) } >>> + } >>> +} > > SAFETY: A valid `phy_device` always have a valid `mdio.dev`. > > Better? It would be nice if you could add this on the invariants on `phy::Device` (you will also have to extend the INVAIRANTS comment that creates a `&'a mut Device`) --- Cheers, Benno
On Sun, 18 Aug 2024 06:01:27 +0000 Benno Lossin <benno.lossin@proton.me> wrote: >>>> +impl AsRef<kernel::device::Device> for Device { >>>> + fn as_ref(&self) -> &kernel::device::Device { >>>> + let phydev = self.0.get(); >>>> + // SAFETY: The struct invariant ensures that we may access >>>> + // this field without additional synchronization. >>> >>> I don't see this invariant on `phy::Device`. >> >> You meant that `phy::Device` Invariants says that all methods defined >> on this struct are safe to call; not about accessing a field so the >> above SAFETY comment isn't correct, right? > > Correct. Understood. >>>> + unsafe { kernel::device::Device::as_ref(addr_of_mut!((*phydev).mdio.dev)) } >>>> + } >>>> +} >> >> SAFETY: A valid `phy_device` always have a valid `mdio.dev`. >> >> Better? > > It would be nice if you could add this on the invariants on > `phy::Device` (you will also have to extend the INVAIRANTS comment that > creates a `&'a mut Device`) How about the followings? diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 5e8137a1972f..3e1d6c43ca33 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -7,8 +7,7 @@ //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h). use crate::{error::*, prelude::*, types::Opaque}; - -use core::marker::PhantomData; +use core::{marker::PhantomData, ptr::addr_of_mut}; /// PHY state machine states. /// @@ -60,6 +59,7 @@ pub enum DuplexMode { /// /// Referencing a `phy_device` using this struct asserts that you are in /// a context where all methods defined on this struct are safe to call. +/// This struct always has a valid `mdio.dev`. /// /// [`struct phy_device`]: srctree/include/linux/phy.h // During the calls to most functions in [`Driver`], the C side (`PHYLIB`) holds a lock that is @@ -76,9 +76,9 @@ impl Device { /// /// # Safety /// - /// For the duration of 'a, the pointer must point at a valid `phy_device`, - /// and the caller must be in a context where all methods defined on this struct - /// are safe to call. + /// For the duration of 'a, the pointer must point at a valid `phy_device` with + /// a valid `mdio.dev`, and the caller must be in a context where all methods + /// defined on this struct are safe to call. unsafe fn from_raw<'a>(ptr: *mut bindings::phy_device) -> &'a mut Self { // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::phy_device`. let ptr = ptr.cast::<Self>(); @@ -302,6 +302,14 @@ pub fn genphy_read_abilities(&mut self) -> Result { } } +impl AsRef<kernel::device::Device> 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)) } + } +} + /// Defines certain other features this PHY supports (like interrupts). /// /// These flag values are used in [`Driver::FLAGS`].
On 18.08.24 09:36, FUJITA Tomonori wrote: > On Sun, 18 Aug 2024 06:01:27 +0000 > Benno Lossin <benno.lossin@proton.me> wrote: >>>>> + unsafe { kernel::device::Device::as_ref(addr_of_mut!((*phydev).mdio.dev)) } >>>>> + } >>>>> +} >>> >>> SAFETY: A valid `phy_device` always have a valid `mdio.dev`. >>> >>> Better? >> >> It would be nice if you could add this on the invariants on >> `phy::Device` (you will also have to extend the INVAIRANTS comment that >> creates a `&'a mut Device`) > > How about the followings? > > diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs > index 5e8137a1972f..3e1d6c43ca33 100644 > --- a/rust/kernel/net/phy.rs > +++ b/rust/kernel/net/phy.rs > @@ -7,8 +7,7 @@ > //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h). > > use crate::{error::*, prelude::*, types::Opaque}; > - > -use core::marker::PhantomData; > +use core::{marker::PhantomData, ptr::addr_of_mut}; > > /// PHY state machine states. > /// > @@ -60,6 +59,7 @@ pub enum DuplexMode { > /// > /// Referencing a `phy_device` using this struct asserts that you are in > /// a context where all methods defined on this struct are safe to call. > +/// This struct always has a valid `mdio.dev`. Please turn this into a bullet point list. > /// > /// [`struct phy_device`]: srctree/include/linux/phy.h > // During the calls to most functions in [`Driver`], the C side (`PHYLIB`) holds a lock that is > @@ -76,9 +76,9 @@ impl Device { > /// > /// # Safety > /// > - /// For the duration of 'a, the pointer must point at a valid `phy_device`, > - /// and the caller must be in a context where all methods defined on this struct > - /// are safe to call. > + /// For the duration of 'a, the pointer must point at a valid `phy_device` with > + /// a valid `mdio.dev`, and the caller must be in a context where all methods > + /// defined on this struct are safe to call. Also here. > unsafe fn from_raw<'a>(ptr: *mut bindings::phy_device) -> &'a mut Self { > // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::phy_device`. > let ptr = ptr.cast::<Self>(); > @@ -302,6 +302,14 @@ pub fn genphy_read_abilities(&mut self) -> Result { > } > } > > +impl AsRef<kernel::device::Device> 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)) } > + } Just to be sure: the `phydev.mdio.dev` struct is refcounted and incrementing the refcount is fine, right? --- Cheers, Benno > +} > + > /// Defines certain other features this PHY supports (like interrupts). > /// > /// These flag values are used in [`Driver::FLAGS`].
On Sun, 18 Aug 2024 09:03:01 +0000 Benno Lossin <benno.lossin@proton.me> wrote: >> /// PHY state machine states. >> /// >> @@ -60,6 +59,7 @@ pub enum DuplexMode { >> /// >> /// Referencing a `phy_device` using this struct asserts that you are in >> /// a context where all methods defined on this struct are safe to call. >> +/// This struct always has a valid `mdio.dev`. > > Please turn this into a bullet point list. /// - Referencing a `phy_device` using this struct asserts that you are in /// a context where all methods defined on this struct are safe to call. /// - This struct always has a valid `mdio.dev`. Looks fine? >> /// >> /// [`struct phy_device`]: srctree/include/linux/phy.h >> // During the calls to most functions in [`Driver`], the C side (`PHYLIB`) holds a lock that is >> @@ -76,9 +76,9 @@ impl Device { >> /// >> /// # Safety >> /// >> - /// For the duration of 'a, the pointer must point at a valid `phy_device`, >> - /// and the caller must be in a context where all methods defined on this struct >> - /// are safe to call. >> + /// For the duration of 'a, the pointer must point at a valid `phy_device` with >> + /// a valid `mdio.dev`, and the caller must be in a context where all methods >> + /// defined on this struct are safe to call. > > Also here. /// # Safety /// /// For the duration of 'a, /// - the pointer must point at a valid `phy_device`, and the caller /// must be in a context where all methods defined on this struct /// are safe to call. /// - 'mdio.dev' must be a valid. Better? >> +impl AsRef<kernel::device::Device> 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)) } >> + } > > Just to be sure: the `phydev.mdio.dev` struct is refcounted and > incrementing the refcount is fine, right? phydev.mdio.dev is valid after phydev is initialized. struct phy_device { struct mdio_device mdio; ... struct mdio_device { struct device dev; ...
On 18.08.24 11:15, FUJITA Tomonori wrote: > On Sun, 18 Aug 2024 09:03:01 +0000 > Benno Lossin <benno.lossin@proton.me> wrote: > >>> /// PHY state machine states. >>> /// >>> @@ -60,6 +59,7 @@ pub enum DuplexMode { >>> /// >>> /// Referencing a `phy_device` using this struct asserts that you are in >>> /// a context where all methods defined on this struct are safe to call. >>> +/// This struct always has a valid `mdio.dev`. >> >> Please turn this into a bullet point list. > > /// - Referencing a `phy_device` using this struct asserts that you are in > /// a context where all methods defined on this struct are safe to call. > /// - This struct always has a valid `mdio.dev`. Hmm, I think `self.0.mdio.dev` would be clearer. > > Looks fine? > >>> /// >>> /// [`struct phy_device`]: srctree/include/linux/phy.h >>> // During the calls to most functions in [`Driver`], the C side (`PHYLIB`) holds a lock that is >>> @@ -76,9 +76,9 @@ impl Device { >>> /// >>> /// # Safety >>> /// >>> - /// For the duration of 'a, the pointer must point at a valid `phy_device`, >>> - /// and the caller must be in a context where all methods defined on this struct >>> - /// are safe to call. >>> + /// For the duration of 'a, the pointer must point at a valid `phy_device` with >>> + /// a valid `mdio.dev`, and the caller must be in a context where all methods >>> + /// defined on this struct are safe to call. >> >> Also here. > > /// # Safety > /// > /// For the duration of 'a, > /// - the pointer must point at a valid `phy_device`, and the caller > /// must be in a context where all methods defined on this struct > /// are safe to call. > /// - 'mdio.dev' must be a valid. Also here: `(*ptr).mdio.dev`. --- Cheers, Benno > > Better? > >>> +impl AsRef<kernel::device::Device> 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)) } >>> + } >> >> Just to be sure: the `phydev.mdio.dev` struct is refcounted and >> incrementing the refcount is fine, right? > > phydev.mdio.dev is valid after phydev is initialized. > > struct phy_device { > struct mdio_device mdio; > ... > > struct mdio_device { > struct device dev; > ... >
On Sun, 18 Aug 2024 10:42:32 +0000 Benno Lossin <benno.lossin@proton.me> wrote: >> /// - Referencing a `phy_device` using this struct asserts that you are in >> /// a context where all methods defined on this struct are safe to call. >> /// - This struct always has a valid `mdio.dev`. > > Hmm, I think `self.0.mdio.dev` would be clearer. Sure, fixed. >> /// # Safety >> /// >> /// For the duration of 'a, >> /// - the pointer must point at a valid `phy_device`, and the caller >> /// must be in a context where all methods defined on this struct >> /// are safe to call. >> /// - 'mdio.dev' must be a valid. > > Also here: `(*ptr).mdio.dev`. Thanks, looks better. Here's an updated patch. diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 5e8137a1972f..ec337cbd391b 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -7,8 +7,7 @@ //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h). use crate::{error::*, prelude::*, types::Opaque}; - -use core::marker::PhantomData; +use core::{marker::PhantomData, ptr::addr_of_mut}; /// PHY state machine states. /// @@ -58,8 +57,9 @@ pub enum DuplexMode { /// /// # Invariants /// -/// Referencing a `phy_device` using this struct asserts that you are in -/// a context where all methods defined on this struct are safe to call. +/// - Referencing a `phy_device` using this struct asserts that you are in +/// a context where all methods defined on this struct are safe to call. +/// - This struct always has a valid `self.0.mdio.dev`. /// /// [`struct phy_device`]: srctree/include/linux/phy.h // During the calls to most functions in [`Driver`], the C side (`PHYLIB`) holds a lock that is @@ -76,9 +76,11 @@ impl Device { /// /// # Safety /// - /// For the duration of 'a, the pointer must point at a valid `phy_device`, - /// and the caller must be in a context where all methods defined on this struct - /// are safe to call. + /// For the duration of 'a, + /// - the pointer must point at a valid `phy_device`, and the caller + /// must be in a context where all methods defined on this struct + /// are safe to call. + /// - `(*ptr).mdio.dev` must be a valid. unsafe fn from_raw<'a>(ptr: *mut bindings::phy_device) -> &'a mut Self { // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::phy_device`. let ptr = ptr.cast::<Self>(); @@ -302,6 +304,14 @@ pub fn genphy_read_abilities(&mut self) -> Result { } } +impl AsRef<kernel::device::Device> 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)) } + } +} + /// Defines certain other features this PHY supports (like interrupts). /// /// These flag values are used in [`Driver::FLAGS`].
> Just to be sure: the `phydev.mdio.dev` struct is refcounted and > incrementing the refcount is fine, right? There is nothing special here. PHY devices are just normal Linux devices. The device core is responsible for calling .probe() and .remove() on a device, and these should be the first and last operation on a device. The device core provides refcounting of the struct device, so it should always be valid. So i have to wounder if you are solving this at the correct level. This should be generic to any device, so the Rust concept of a device should be stating these guarantees, not each specific type of device. Andrew
On Sun, Aug 18, 2024 at 05:38:01PM +0200, Andrew Lunn wrote: > > Just to be sure: the `phydev.mdio.dev` struct is refcounted and > > incrementing the refcount is fine, right? > > There is nothing special here. PHY devices are just normal Linux > devices. The device core is responsible for calling .probe() and > .remove() on a device, and these should be the first and last > operation on a device. The device core provides refcounting of the > struct device, so it should always be valid. > > So i have to wounder if you are solving this at the correct > level. This should be generic to any device, so the Rust concept of a > device should be stating these guarantees, not each specific type of > device. It should, why isn't it using the rust binding to Device that we already have: https://rust.docs.kernel.org/kernel/device/struct.Device.html or is it? I'm confused now... thanks, greg k-h
On 18.08.24 17:45, Greg KH wrote: > On Sun, Aug 18, 2024 at 05:38:01PM +0200, Andrew Lunn wrote: >>> Just to be sure: the `phydev.mdio.dev` struct is refcounted and >>> incrementing the refcount is fine, right? >> >> There is nothing special here. PHY devices are just normal Linux >> devices. The device core is responsible for calling .probe() and >> .remove() on a device, and these should be the first and last >> operation on a device. The device core provides refcounting of the >> struct device, so it should always be valid. Thanks that's good to know. >> So i have to wounder if you are solving this at the correct >> level. This should be generic to any device, so the Rust concept of a >> device should be stating these guarantees, not each specific type of >> device. > > It should, why isn't it using the rust binding to Device that we already > have: > https://rust.docs.kernel.org/kernel/device/struct.Device.html > or is it? I'm confused now... It is using that one. I wanted to verify that we can use that one, since using this implementation users can freely increment the refcount of the device (without decrementing it before control is given back to PHYLIB). Since that seems to be the case, all is fine. --- Cheers, Benno
On 18.08.24 13:25, FUJITA Tomonori wrote: > On Sun, 18 Aug 2024 10:42:32 +0000 > Benno Lossin <benno.lossin@proton.me> wrote: > >>> /// - Referencing a `phy_device` using this struct asserts that you are in >>> /// a context where all methods defined on this struct are safe to call. >>> /// - This struct always has a valid `mdio.dev`. >> >> Hmm, I think `self.0.mdio.dev` would be clearer. > > Sure, fixed. > >>> /// # Safety >>> /// >>> /// For the duration of 'a, >>> /// - the pointer must point at a valid `phy_device`, and the caller >>> /// must be in a context where all methods defined on this struct >>> /// are safe to call. >>> /// - 'mdio.dev' must be a valid. >> >> Also here: `(*ptr).mdio.dev`. > > Thanks, looks better. > > Here's an updated patch. > > diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs > index 5e8137a1972f..ec337cbd391b 100644 > --- a/rust/kernel/net/phy.rs > +++ b/rust/kernel/net/phy.rs > @@ -7,8 +7,7 @@ > //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h). > > use crate::{error::*, prelude::*, types::Opaque}; > - > -use core::marker::PhantomData; > +use core::{marker::PhantomData, ptr::addr_of_mut}; > > /// PHY state machine states. > /// > @@ -58,8 +57,9 @@ pub enum DuplexMode { > /// > /// # Invariants > /// > -/// Referencing a `phy_device` using this struct asserts that you are in > -/// a context where all methods defined on this struct are safe to call. > +/// - Referencing a `phy_device` using this struct asserts that you are in > +/// a context where all methods defined on this struct are safe to call. > +/// - This struct always has a valid `self.0.mdio.dev`. > /// > /// [`struct phy_device`]: srctree/include/linux/phy.h > // During the calls to most functions in [`Driver`], the C side (`PHYLIB`) holds a lock that is > @@ -76,9 +76,11 @@ impl Device { > /// > /// # Safety > /// > - /// For the duration of 'a, the pointer must point at a valid `phy_device`, > - /// and the caller must be in a context where all methods defined on this struct > - /// are safe to call. > + /// For the duration of 'a, > + /// - the pointer must point at a valid `phy_device`, and the caller > + /// must be in a context where all methods defined on this struct > + /// are safe to call. > + /// - `(*ptr).mdio.dev` must be a valid. > unsafe fn from_raw<'a>(ptr: *mut bindings::phy_device) -> &'a mut Self { > // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::phy_device`. > let ptr = ptr.cast::<Self>(); > @@ -302,6 +304,14 @@ pub fn genphy_read_abilities(&mut self) -> Result { > } > } > > +impl AsRef<kernel::device::Device> 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)) } > + } > +} > + > /// Defines certain other features this PHY supports (like interrupts). > /// > /// These flag values are used in [`Driver::FLAGS`]. > -- > 2.34.1 > This looks, good, if you want, you can add my reviewed-by on this patch. --- Cheers, Benno
> Thanks that's good to know. > > >> So i have to wounder if you are solving this at the correct > >> level. This should be generic to any device, so the Rust concept of a > >> device should be stating these guarantees, not each specific type of > >> device. > > > > It should, why isn't it using the rust binding to Device that we already > > have: > > https://rust.docs.kernel.org/kernel/device/struct.Device.html > > or is it? I'm confused now... > > It is using that one. > I wanted to verify that we can use that one, since using this > implementation users can freely increment the refcount of the device > (without decrementing it before control is given back to PHYLIB). Since > that seems to be the case, all is fine. Any driver which is not using the device core is broken, and no amount of SAFETY is going to fix it. Andrew
On 18.08.24 18:16, Andrew Lunn wrote: >> Thanks that's good to know. >> >>>> So i have to wounder if you are solving this at the correct >>>> level. This should be generic to any device, so the Rust concept of a >>>> device should be stating these guarantees, not each specific type of >>>> device. >>> >>> It should, why isn't it using the rust binding to Device that we already >>> have: >>> https://rust.docs.kernel.org/kernel/device/struct.Device.html >>> or is it? I'm confused now... >> >> It is using that one. >> I wanted to verify that we can use that one, since using this >> implementation users can freely increment the refcount of the device >> (without decrementing it before control is given back to PHYLIB). Since >> that seems to be the case, all is fine. > > Any driver which is not using the device core is broken, and no amount > of SAFETY is going to fix it. This is what I did not know, and I asked to ensure that we don't introduce miscommunication with the C side. (i.e. can we rely in our SAFETY comments that devices are always used in this way) --- Cheers, Benno
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs index 5e8137a1972f..569dd3beef9c 100644 --- a/rust/kernel/net/phy.rs +++ b/rust/kernel/net/phy.rs @@ -7,8 +7,7 @@ //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h). use crate::{error::*, prelude::*, types::Opaque}; - -use core::marker::PhantomData; +use core::{marker::PhantomData, ptr::addr_of_mut}; /// PHY state machine states. /// @@ -302,6 +301,15 @@ pub fn genphy_read_abilities(&mut self) -> Result { } } +impl AsRef<kernel::device::Device> for Device { + fn as_ref(&self) -> &kernel::device::Device { + let phydev = self.0.get(); + // SAFETY: The struct invariant ensures that we may access + // this field without additional synchronization. + unsafe { kernel::device::Device::as_ref(addr_of_mut!((*phydev).mdio.dev)) } + } +} + /// Defines certain other features this PHY supports (like interrupts). /// /// These flag values are used in [`Driver::FLAGS`].