Pass PHY driver pointer to .match_phy_device OP in addition to phydev.
Having access to the PHY driver struct might be useful to check the
PHY ID of the driver is being matched for in case the PHY ID scanned in
the phydev is not consistent.

A scenario for this is a PHY that change PHY ID after a firmware is
loaded, in such case, the PHY ID stored in PHY device struct is not
valid anymore and PHY will manually scan the ID in the match_phy_device
function.

Having the PHY driver info is also useful for those PHY driver that
implement multiple simple .match_phy_device OP to match specific MMD PHY
ID. With this extra info if the parsing logic is the same, the matching
function can be generalized by using the phy_id in the PHY driver
instead of hardcoding.

Rust bindings are updated to align to the new match_phy_device
arguments.

Suggested-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/phy/bcm87xx.c              |  6 ++++--
 drivers/net/phy/icplus.c               |  6 ++++--
 drivers/net/phy/marvell10g.c           | 12 ++++++++----
 drivers/net/phy/micrel.c               |  6 ++++--
 drivers/net/phy/nxp-c45-tja11xx.c      | 12 ++++++++----
 drivers/net/phy/nxp-tja11xx.c          |  6 ++++--
 drivers/net/phy/phy_device.c           |  2 +-
 drivers/net/phy/realtek/realtek_main.c | 27 +++++++++++++++++---------
 drivers/net/phy/teranetics.c           |  3 ++-
 include/linux/phy.h                    |  3 ++-
 rust/kernel/net/phy.rs                 | 22 ++++++++++++++++++---
 11 files changed, 74 insertions(+), 31 deletions(-)

diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
index e81404bf8994..1e1e2259fc2b 100644
--- a/drivers/net/phy/bcm87xx.c
+++ b/drivers/net/phy/bcm87xx.c
@@ -185,12 +185,14 @@ static irqreturn_t bcm87xx_handle_interrupt(struct phy_device *phydev)
 	return IRQ_HANDLED;
 }
 
-static int bcm8706_match_phy_device(struct phy_device *phydev)
+static int bcm8706_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return phydev->c45_ids.device_ids[4] == PHY_ID_BCM8706;
 }
 
-static int bcm8727_match_phy_device(struct phy_device *phydev)
+static int bcm8727_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return phydev->c45_ids.device_ids[4] == PHY_ID_BCM8727;
 }
diff --git a/drivers/net/phy/icplus.c b/drivers/net/phy/icplus.c
index bbcc7d2b54cd..c0c4f19cfb6a 100644
--- a/drivers/net/phy/icplus.c
+++ b/drivers/net/phy/icplus.c
@@ -520,12 +520,14 @@ static int ip101a_g_match_phy_device(struct phy_device *phydev, bool ip101a)
 	return ip101a == !ret;
 }
 
-static int ip101a_match_phy_device(struct phy_device *phydev)
+static int ip101a_match_phy_device(struct phy_device *phydev,
+				   const struct phy_driver *phydrv)
 {
 	return ip101a_g_match_phy_device(phydev, true);
 }
 
-static int ip101g_match_phy_device(struct phy_device *phydev)
+static int ip101g_match_phy_device(struct phy_device *phydev,
+				   const struct phy_driver *phydrv)
 {
 	return ip101a_g_match_phy_device(phydev, false);
 }
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 5354c8895163..13e81dff42c1 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -1264,7 +1264,8 @@ static int mv3310_get_number_of_ports(struct phy_device *phydev)
 	return ret + 1;
 }
 
-static int mv3310_match_phy_device(struct phy_device *phydev)
+static int mv3310_match_phy_device(struct phy_device *phydev,
+				   const struct phy_driver *phydrv)
 {
 	if ((phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] &
 	     MARVELL_PHY_ID_MASK) != MARVELL_PHY_ID_88X3310)
@@ -1273,7 +1274,8 @@ static int mv3310_match_phy_device(struct phy_device *phydev)
 	return mv3310_get_number_of_ports(phydev) == 1;
 }
 
-static int mv3340_match_phy_device(struct phy_device *phydev)
+static int mv3340_match_phy_device(struct phy_device *phydev,
+				   const struct phy_driver *phydrv)
 {
 	if ((phydev->c45_ids.device_ids[MDIO_MMD_PMAPMD] &
 	     MARVELL_PHY_ID_MASK) != MARVELL_PHY_ID_88X3310)
@@ -1297,12 +1299,14 @@ static int mv211x_match_phy_device(struct phy_device *phydev, bool has_5g)
 	return !!(val & MDIO_PCS_SPEED_5G) == has_5g;
 }
 
-static int mv2110_match_phy_device(struct phy_device *phydev)
+static int mv2110_match_phy_device(struct phy_device *phydev,
+				   const struct phy_driver *phydrv)
 {
 	return mv211x_match_phy_device(phydev, true);
 }
 
-static int mv2111_match_phy_device(struct phy_device *phydev)
+static int mv2111_match_phy_device(struct phy_device *phydev,
+				   const struct phy_driver *phydrv)
 {
 	return mv211x_match_phy_device(phydev, false);
 }
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 71fb4410c31b..4d8460c93078 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -768,7 +768,8 @@ static int ksz8051_ksz8795_match_phy_device(struct phy_device *phydev,
 		return !ret;
 }
 
-static int ksz8051_match_phy_device(struct phy_device *phydev)
+static int ksz8051_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return ksz8051_ksz8795_match_phy_device(phydev, true);
 }
@@ -888,7 +889,8 @@ static int ksz8061_config_init(struct phy_device *phydev)
 	return kszphy_config_init(phydev);
 }
 
-static int ksz8795_match_phy_device(struct phy_device *phydev)
+static int ksz8795_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return ksz8051_ksz8795_match_phy_device(phydev, false);
 }
diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
index f11dd32494c3..22921b192a8b 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.c
+++ b/drivers/net/phy/nxp-c45-tja11xx.c
@@ -1966,25 +1966,29 @@ static int nxp_c45_macsec_ability(struct phy_device *phydev)
 	return macsec_ability;
 }
 
-static int tja1103_match_phy_device(struct phy_device *phydev)
+static int tja1103_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return phy_id_compare(phydev->phy_id, PHY_ID_TJA_1103, PHY_ID_MASK) &&
 	       !nxp_c45_macsec_ability(phydev);
 }
 
-static int tja1104_match_phy_device(struct phy_device *phydev)
+static int tja1104_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return phy_id_compare(phydev->phy_id, PHY_ID_TJA_1103, PHY_ID_MASK) &&
 	       nxp_c45_macsec_ability(phydev);
 }
 
-static int tja1120_match_phy_device(struct phy_device *phydev)
+static int tja1120_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return phy_id_compare(phydev->phy_id, PHY_ID_TJA_1120, PHY_ID_MASK) &&
 	       !nxp_c45_macsec_ability(phydev);
 }
 
-static int tja1121_match_phy_device(struct phy_device *phydev)
+static int tja1121_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return phy_id_compare(phydev->phy_id, PHY_ID_TJA_1120, PHY_ID_MASK) &&
 	       nxp_c45_macsec_ability(phydev);
diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c
index 07e94a2478ac..3c38a8ddae2f 100644
--- a/drivers/net/phy/nxp-tja11xx.c
+++ b/drivers/net/phy/nxp-tja11xx.c
@@ -651,12 +651,14 @@ static int tja1102_match_phy_device(struct phy_device *phydev, bool port0)
 	return !ret;
 }
 
-static int tja1102_p0_match_phy_device(struct phy_device *phydev)
+static int tja1102_p0_match_phy_device(struct phy_device *phydev,
+				       const struct phy_driver *phydrv)
 {
 	return tja1102_match_phy_device(phydev, true);
 }
 
-static int tja1102_p1_match_phy_device(struct phy_device *phydev)
+static int tja1102_p1_match_phy_device(struct phy_device *phydev,
+				       const struct phy_driver *phydrv)
 {
 	return tja1102_match_phy_device(phydev, false);
 }
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2eb735e68dd8..96a96c0334a7 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -554,7 +554,7 @@ static int phy_bus_match(struct device *dev, const struct device_driver *drv)
 		return 0;
 
 	if (phydrv->match_phy_device)
-		return phydrv->match_phy_device(phydev);
+		return phydrv->match_phy_device(phydev, phydrv);
 
 	if (phydev->is_c45) {
 		for (i = 1; i < num_ids; i++) {
diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 301fbe141b9b..6b655d3c7e1c 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -1314,13 +1314,15 @@ static bool rtlgen_supports_mmd(struct phy_device *phydev)
 	return val > 0;
 }
 
-static int rtlgen_match_phy_device(struct phy_device *phydev)
+static int rtlgen_match_phy_device(struct phy_device *phydev,
+				   const struct phy_driver *phydrv)
 {
 	return phydev->phy_id == RTL_GENERIC_PHYID &&
 	       !rtlgen_supports_2_5gbps(phydev);
 }
 
-static int rtl8226_match_phy_device(struct phy_device *phydev)
+static int rtl8226_match_phy_device(struct phy_device *phydev,
+				    const struct phy_driver *phydrv)
 {
 	return phydev->phy_id == RTL_GENERIC_PHYID &&
 	       rtlgen_supports_2_5gbps(phydev) &&
@@ -1336,32 +1338,38 @@ static int rtlgen_is_c45_match(struct phy_device *phydev, unsigned int id,
 		return !is_c45 && (id == phydev->phy_id);
 }
 
-static int rtl8221b_match_phy_device(struct phy_device *phydev)
+static int rtl8221b_match_phy_device(struct phy_device *phydev,
+				     const struct phy_driver *phydrv)
 {
 	return phydev->phy_id == RTL_8221B && rtlgen_supports_mmd(phydev);
 }
 
-static int rtl8221b_vb_cg_c22_match_phy_device(struct phy_device *phydev)
+static int rtl8221b_vb_cg_c22_match_phy_device(struct phy_device *phydev,
+					       const struct phy_driver *phydrv)
 {
 	return rtlgen_is_c45_match(phydev, RTL_8221B_VB_CG, false);
 }
 
-static int rtl8221b_vb_cg_c45_match_phy_device(struct phy_device *phydev)
+static int rtl8221b_vb_cg_c45_match_phy_device(struct phy_device *phydev,
+					       const struct phy_driver *phydrv)
 {
 	return rtlgen_is_c45_match(phydev, RTL_8221B_VB_CG, true);
 }
 
-static int rtl8221b_vn_cg_c22_match_phy_device(struct phy_device *phydev)
+static int rtl8221b_vn_cg_c22_match_phy_device(struct phy_device *phydev,
+					       const struct phy_driver *phydrv)
 {
 	return rtlgen_is_c45_match(phydev, RTL_8221B_VN_CG, false);
 }
 
-static int rtl8221b_vn_cg_c45_match_phy_device(struct phy_device *phydev)
+static int rtl8221b_vn_cg_c45_match_phy_device(struct phy_device *phydev,
+					       const struct phy_driver *phydrv)
 {
 	return rtlgen_is_c45_match(phydev, RTL_8221B_VN_CG, true);
 }
 
-static int rtl_internal_nbaset_match_phy_device(struct phy_device *phydev)
+static int rtl_internal_nbaset_match_phy_device(struct phy_device *phydev,
+						const struct phy_driver *phydrv)
 {
 	if (phydev->is_c45)
 		return false;
@@ -1379,7 +1387,8 @@ static int rtl_internal_nbaset_match_phy_device(struct phy_device *phydev)
 	return rtlgen_supports_2_5gbps(phydev) && !rtlgen_supports_mmd(phydev);
 }
 
-static int rtl8251b_c45_match_phy_device(struct phy_device *phydev)
+static int rtl8251b_c45_match_phy_device(struct phy_device *phydev,
+					 const struct phy_driver *phydrv)
 {
 	return rtlgen_is_c45_match(phydev, RTL_8251B, true);
 }
diff --git a/drivers/net/phy/teranetics.c b/drivers/net/phy/teranetics.c
index 752d4bf7bb99..46c5ff7d7b56 100644
--- a/drivers/net/phy/teranetics.c
+++ b/drivers/net/phy/teranetics.c
@@ -67,7 +67,8 @@ static int teranetics_read_status(struct phy_device *phydev)
 	return 0;
 }
 
-static int teranetics_match_phy_device(struct phy_device *phydev)
+static int teranetics_match_phy_device(struct phy_device *phydev,
+				       const struct phy_driver *phydrv)
 {
 	return phydev->c45_ids.device_ids[3] == PHY_ID_TN2020;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c29d346d4b3..34ed85686b83 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -990,7 +990,8 @@ struct phy_driver {
 	 * driver for the given phydev.	 If NULL, matching is based on
 	 * phy_id and phy_id_mask.
 	 */
-	int (*match_phy_device)(struct phy_device *phydev);
+	int (*match_phy_device)(struct phy_device *phydev,
+				const struct phy_driver *phydrv);
 
 	/**
 	 * @set_wol: Some devices (e.g. qnap TS-119P II) require PHY
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index a59469c785e3..079a0f884887 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -418,15 +418,18 @@ impl<T: Driver> Adapter<T> {
 
     /// # Safety
     ///
-    /// `phydev` must be passed by the corresponding callback in `phy_driver`.
+    /// `phydev` and `phydrv` must be passed by the corresponding callback in
+    //  `phy_driver`.
     unsafe extern "C" fn match_phy_device_callback(
         phydev: *mut bindings::phy_device,
+        phydrv: *const bindings::phy_driver,
     ) -> crate::ffi::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.
         let dev = unsafe { Device::from_raw(phydev) };
-        T::match_phy_device(dev) as i32
+        let drv = unsafe { T::from_raw(phydrv) };
+        T::match_phy_device(dev, drv) as i32
     }
 
     /// # Safety
@@ -574,6 +577,19 @@ pub const fn create_phy_driver<T: Driver>() -> DriverVTable {
 /// This trait is used to create a [`DriverVTable`].
 #[vtable]
 pub trait Driver {
+    /// # Safety
+    ///
+    /// For the duration of `'a`, the pointer must point at a valid
+    /// `phy_driver`, 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: *const bindings::phy_driver) -> &'a DriverVTable {
+        // CAST: `DriverVTable` is a `repr(transparent)` wrapper around `bindings::phy_driver`.
+        let ptr = ptr.cast::<DriverVTable>();
+        // SAFETY: by the function requirements the pointer is const and is
+        // always valid to access for the duration of `'a`.
+        unsafe { &*ptr }
+    }
+
     /// Defines certain other features this PHY supports.
     /// It is a combination of the flags in the [`flags`] module.
     const FLAGS: u32 = 0;
@@ -602,7 +618,7 @@ fn get_features(_dev: &mut Device) -> Result {
 
     /// Returns true if this is a suitable driver for the given phydev.
     /// If not implemented, matching is based on [`Driver::PHY_DEVICE_ID`].
-    fn match_phy_device(_dev: &Device) -> bool {
+    fn match_phy_device(_dev: &mut Device, _drv: &DriverVTable) -> bool {
         false
     }
 
-- 
2.48.1

On Fri May 16, 2025 at 11:23 PM CEST, Christian Marangi wrote:
> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
> index a59469c785e3..079a0f884887 100644
> --- a/rust/kernel/net/phy.rs
> +++ b/rust/kernel/net/phy.rs
> @@ -418,15 +418,18 @@ impl<T: Driver> Adapter<T> {
>  
>      /// # Safety
>      ///
> -    /// `phydev` must be passed by the corresponding callback in `phy_driver`.
> +    /// `phydev` and `phydrv` must be passed by the corresponding callback in
> +    //  `phy_driver`.
>      unsafe extern "C" fn match_phy_device_callback(
>          phydev: *mut bindings::phy_device,
> +        phydrv: *const bindings::phy_driver,
>      ) -> crate::ffi::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.
>          let dev = unsafe { Device::from_raw(phydev) };
> -        T::match_phy_device(dev) as i32
> +        let drv = unsafe { T::from_raw(phydrv) };
> +        T::match_phy_device(dev, drv) as i32
>      }
>  
>      /// # Safety
> @@ -574,6 +577,19 @@ pub const fn create_phy_driver<T: Driver>() -> DriverVTable {
>  /// This trait is used to create a [`DriverVTable`].
>  #[vtable]
>  pub trait Driver {
> +    /// # Safety
> +    ///
> +    /// For the duration of `'a`, the pointer must point at a valid
> +    /// `phy_driver`, 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: *const bindings::phy_driver) -> &'a DriverVTable {
> +        // CAST: `DriverVTable` is a `repr(transparent)` wrapper around `bindings::phy_driver`.
> +        let ptr = ptr.cast::<DriverVTable>();
> +        // SAFETY: by the function requirements the pointer is const and is
> +        // always valid to access for the duration of `'a`.
> +        unsafe { &*ptr }
> +    }

If we go the way of supplying a `&DriverVTable` in the
`match_phy_device` function, then this should be a function in the impl
block of `DriverVTable` and not in `Driver`.

See my reply to Fujita on the previous version, I don't think that we
need to add the `DriverVTable` to the `match_phy_device` function if we
don't provide accessor methods. Currently that isn't needed, so you only
need the hunks above this one. (I'd wait for Fujita's reply though).

---
Cheers,
Benno

> +
>      /// Defines certain other features this PHY supports.
>      /// It is a combination of the flags in the [`flags`] module.
>      const FLAGS: u32 = 0;
> @@ -602,7 +618,7 @@ fn get_features(_dev: &mut Device) -> Result {
>  
>      /// Returns true if this is a suitable driver for the given phydev.
>      /// If not implemented, matching is based on [`Driver::PHY_DEVICE_ID`].
> -    fn match_phy_device(_dev: &Device) -> bool {
> +    fn match_phy_device(_dev: &mut Device, _drv: &DriverVTable) -> bool {
>          false
>      }
>  

On Sat, 17 May 2025 10:09:53 +0200
"Benno Lossin" <lossin@kernel.org> wrote:

> On Fri May 16, 2025 at 11:23 PM CEST, Christian Marangi wrote:
>> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
>> index a59469c785e3..079a0f884887 100644
>> --- a/rust/kernel/net/phy.rs
>> +++ b/rust/kernel/net/phy.rs
>> @@ -418,15 +418,18 @@ impl<T: Driver> Adapter<T> {
>>  
>>      /// # Safety
>>      ///
>> -    /// `phydev` must be passed by the corresponding callback in `phy_driver`.
>> +    /// `phydev` and `phydrv` must be passed by the corresponding callback in
>> +    //  `phy_driver`.
>>      unsafe extern "C" fn match_phy_device_callback(
>>          phydev: *mut bindings::phy_device,
>> +        phydrv: *const bindings::phy_driver,
>>      ) -> crate::ffi::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.
>>          let dev = unsafe { Device::from_raw(phydev) };
>> -        T::match_phy_device(dev) as i32
>> +        let drv = unsafe { T::from_raw(phydrv) };
>> +        T::match_phy_device(dev, drv) as i32
>>      }
>>  
>>      /// # Safety
>> @@ -574,6 +577,19 @@ pub const fn create_phy_driver<T: Driver>() -> DriverVTable {
>>  /// This trait is used to create a [`DriverVTable`].
>>  #[vtable]
>>  pub trait Driver {
>> +    /// # Safety
>> +    ///
>> +    /// For the duration of `'a`, the pointer must point at a valid
>> +    /// `phy_driver`, 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: *const bindings::phy_driver) -> &'a DriverVTable {
>> +        // CAST: `DriverVTable` is a `repr(transparent)` wrapper around `bindings::phy_driver`.
>> +        let ptr = ptr.cast::<DriverVTable>();
>> +        // SAFETY: by the function requirements the pointer is const and is
>> +        // always valid to access for the duration of `'a`.
>> +        unsafe { &*ptr }
>> +    }
> 
> If we go the way of supplying a `&DriverVTable` in the
> `match_phy_device` function, then this should be a function in the impl
> block of `DriverVTable` and not in `Driver`.

Yeah.

> See my reply to Fujita on the previous version, I don't think that we
> need to add the `DriverVTable` to the `match_phy_device` function if we
> don't provide accessor methods. Currently that isn't needed, so you only
> need the hunks above this one. (I'd wait for Fujita's reply though).

Agreed, to make DriverVTable actually useful in match_phy_device(),
further changes would be needed. I think it's sufficient to simply
make the Rust code compile, as shown in the patch below.

I can take care of making sure Rust uses DriverVTable correctly later.

diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index a59469c785e3..32ea43ece646 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -421,6 +421,7 @@ impl<T: Driver> Adapter<T> {
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
     unsafe extern "C" fn match_phy_device_callback(
         phydev: *mut bindings::phy_device,
+        _phydrv: *const bindings::phy_driver,
     ) -> crate::ffi::c_int {
         // SAFETY: This callback is called only in contexts
         // where we hold `phy_device->lock`, so the accessors on

On Sat, May 17, 2025 at 10:28:04PM +0900, FUJITA Tomonori wrote:
> On Sat, 17 May 2025 10:09:53 +0200
> "Benno Lossin" <lossin@kernel.org> wrote:
> 
> > On Fri May 16, 2025 at 11:23 PM CEST, Christian Marangi wrote:
> >> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
> >> index a59469c785e3..079a0f884887 100644
> >> --- a/rust/kernel/net/phy.rs
> >> +++ b/rust/kernel/net/phy.rs
> >> @@ -418,15 +418,18 @@ impl<T: Driver> Adapter<T> {
> >>  
> >>      /// # Safety
> >>      ///
> >> -    /// `phydev` must be passed by the corresponding callback in `phy_driver`.
> >> +    /// `phydev` and `phydrv` must be passed by the corresponding callback in
> >> +    //  `phy_driver`.
> >>      unsafe extern "C" fn match_phy_device_callback(
> >>          phydev: *mut bindings::phy_device,
> >> +        phydrv: *const bindings::phy_driver,
> >>      ) -> crate::ffi::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.
> >>          let dev = unsafe { Device::from_raw(phydev) };
> >> -        T::match_phy_device(dev) as i32
> >> +        let drv = unsafe { T::from_raw(phydrv) };
> >> +        T::match_phy_device(dev, drv) as i32
> >>      }
> >>  
> >>      /// # Safety
> >> @@ -574,6 +577,19 @@ pub const fn create_phy_driver<T: Driver>() -> DriverVTable {
> >>  /// This trait is used to create a [`DriverVTable`].
> >>  #[vtable]
> >>  pub trait Driver {
> >> +    /// # Safety
> >> +    ///
> >> +    /// For the duration of `'a`, the pointer must point at a valid
> >> +    /// `phy_driver`, 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: *const bindings::phy_driver) -> &'a DriverVTable {
> >> +        // CAST: `DriverVTable` is a `repr(transparent)` wrapper around `bindings::phy_driver`.
> >> +        let ptr = ptr.cast::<DriverVTable>();
> >> +        // SAFETY: by the function requirements the pointer is const and is
> >> +        // always valid to access for the duration of `'a`.
> >> +        unsafe { &*ptr }
> >> +    }
> > 
> > If we go the way of supplying a `&DriverVTable` in the
> > `match_phy_device` function, then this should be a function in the impl
> > block of `DriverVTable` and not in `Driver`.
> 
> Yeah.
> 
> > See my reply to Fujita on the previous version, I don't think that we
> > need to add the `DriverVTable` to the `match_phy_device` function if we
> > don't provide accessor methods. Currently that isn't needed, so you only
> > need the hunks above this one. (I'd wait for Fujita's reply though).
> 
> Agreed, to make DriverVTable actually useful in match_phy_device(),
> further changes would be needed. I think it's sufficient to simply
> make the Rust code compile, as shown in the patch below.
> 
> I can take care of making sure Rust uses DriverVTable correctly later.
>

Thanks a lot, I'm sending new revision with the below change as
suggested.

> diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
> index a59469c785e3..32ea43ece646 100644
> --- a/rust/kernel/net/phy.rs
> +++ b/rust/kernel/net/phy.rs
> @@ -421,6 +421,7 @@ impl<T: Driver> Adapter<T> {
>      /// `phydev` must be passed by the corresponding callback in `phy_driver`.
>      unsafe extern "C" fn match_phy_device_callback(
>          phydev: *mut bindings::phy_device,
> +        _phydrv: *const bindings::phy_driver,
>      ) -> crate::ffi::c_int {
>          // SAFETY: This callback is called only in contexts
>          // where we hold `phy_device->lock`, so the accessors on

-- 
	Ansuel