[RFC PATCH v4 0/2] net::phy add unified API for C22 and C45 registers

[RFC PATCH v4 0/2] net::phy add unified API for C22 and C45 registers

[FUJITA Tomonori]

add unified API for C22 and C45, reading/writing registers and
genphy_read_status().

We could add methods specifically C45 like read/write_c45 and
genphy_c45_read_status. Instead, this implements the unified API for
C22 and C45 with a new reg module.

v4:
- make reg module sealed
- improve the comments
- add examples
- use error::to_result() to simplify some functions
v3: https://lore.kernel.org/all/20240603.233954.1644803151268035787.fujita.tomonori@gmail.com/T/
- move reg module to a new file (with MAINTAINERS updated)
- rewrite commit log and subjects
- rename Access trait to Register
- add build error message for build_assert!
- define the complete MMD list
- drop new func for Mmd
- remove trait GenPhyOps
v2: https://lore.kernel.org/all/20240601043535.53545-2-fujita.tomonori@gmail.com/T/
- add genphy helper support and split the patch
- create a 'reg' module
- define C22 according to the specs
- handle C22 vendor specific registers
- add const for C45 MMD
v1: https://lore.kernel.org/all/20240530.145258.456915814420167538.fujita.tomonori@gmail.com/T/

FUJITA Tomonori (2):
  rust: net::phy unified read/write API for C22 and C45 registers
  rust: net::phy unified genphy_read_status function for C22 and C45
    registers

 MAINTAINERS                      |   1 +
 drivers/net/phy/ax88796b_rust.rs |   7 +-
 rust/kernel/net/phy.rs           |  44 ++-----
 rust/kernel/net/phy/reg.rs       | 219 +++++++++++++++++++++++++++++++
 rust/uapi/uapi_helper.h          |   1 +
 5 files changed, 234 insertions(+), 38 deletions(-)
 create mode 100644 rust/kernel/net/phy/reg.rs


base-commit: c790275b5edf5d8280ae520bda7c1f37da460c00
-- 
2.34.1

[RFC PATCH v4 1/2] rust: net::phy unified read/write API for C22 and C45 registers

[FUJITA Tomonori]

Add the unified read/write API for C22 and C45 registers. The
abstractions support access to only C22 registers now. Instead of
adding read/write_c45 methods specifically for C45, a new reg module
supports the unified API to access C22 and C45 registers with trait,
by calling an appropriate phylib functions.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 MAINTAINERS                      |   1 +
 drivers/net/phy/ax88796b_rust.rs |   7 +-
 rust/kernel/net/phy.rs           |  32 ++---
 rust/kernel/net/phy/reg.rs       | 193 +++++++++++++++++++++++++++++++
 rust/uapi/uapi_helper.h          |   1 +
 5 files changed, 206 insertions(+), 28 deletions(-)
 create mode 100644 rust/kernel/net/phy/reg.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 7538152be2f1..4c7d3c303dbc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8210,6 +8210,7 @@ L:	netdev@vger.kernel.org
 L:	rust-for-linux@vger.kernel.org
 S:	Maintained
 F:	rust/kernel/net/phy.rs
+F:	rust/kernel/net/phy/reg.rs
 
 EXEC & BINFMT API, ELF
 R:	Eric Biederman <ebiederm@xmission.com>
diff --git a/drivers/net/phy/ax88796b_rust.rs b/drivers/net/phy/ax88796b_rust.rs
index 5c92572962dc..8c7eb009d9fc 100644
--- a/drivers/net/phy/ax88796b_rust.rs
+++ b/drivers/net/phy/ax88796b_rust.rs
@@ -6,7 +6,7 @@
 //! C version of this driver: [`drivers/net/phy/ax88796b.c`](./ax88796b.c)
 use kernel::{
     c_str,
-    net::phy::{self, DeviceId, Driver},
+    net::phy::{self, reg::C22, DeviceId, Driver},
     prelude::*,
     uapi,
 };
@@ -24,7 +24,6 @@
     license: "GPL",
 }
 
-const MII_BMCR: u16 = uapi::MII_BMCR as u16;
 const BMCR_SPEED100: u16 = uapi::BMCR_SPEED100 as u16;
 const BMCR_FULLDPLX: u16 = uapi::BMCR_FULLDPLX as u16;
 
@@ -33,7 +32,7 @@
 // Toggle BMCR_RESET bit off to accommodate broken AX8796B PHY implementation
 // such as used on the Individual Computers' X-Surf 100 Zorro card.
 fn asix_soft_reset(dev: &mut phy::Device) -> Result {
-    dev.write(uapi::MII_BMCR as u16, 0)?;
+    dev.write(C22::BMCR, 0)?;
     dev.genphy_soft_reset()
 }
 
@@ -55,7 +54,7 @@ fn read_status(dev: &mut phy::Device) -> Result<u16> {
         }
         // If MII_LPA is 0, phy_resolve_aneg_linkmode() will fail to resolve
         // linkmode so use MII_BMCR as default values.
-        let ret = dev.read(MII_BMCR)?;
+        let ret = dev.read(C22::BMCR)?;
 
         if ret & BMCR_SPEED100 != 0 {
             dev.set_speed(uapi::SPEED_100);
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index fd40b703d224..9b4c4d24b231 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -7,9 +7,10 @@
 //! C headers: [`include/linux/phy.h`](srctree/include/linux/phy.h).
 
 use crate::{error::*, prelude::*, types::Opaque};
-
 use core::marker::PhantomData;
 
+pub mod reg;
+
 /// PHY state machine states.
 ///
 /// Corresponds to the kernel's [`enum phy_state`].
@@ -175,32 +176,15 @@ pub fn set_duplex(&mut self, mode: DuplexMode) {
         unsafe { (*phydev).duplex = v };
     }
 
-    /// Reads a given C22 PHY register.
+    /// Reads a PHY register.
     // This function reads a hardware register and updates the stats so takes `&mut self`.
-    pub fn read(&mut self, regnum: u16) -> Result<u16> {
-        let phydev = self.0.get();
-        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`.
-        // So it's just an FFI call, open code of `phy_read()` with a valid `phy_device` pointer
-        // `phydev`.
-        let ret = unsafe {
-            bindings::mdiobus_read((*phydev).mdio.bus, (*phydev).mdio.addr, regnum.into())
-        };
-        if ret < 0 {
-            Err(Error::from_errno(ret))
-        } else {
-            Ok(ret as u16)
-        }
+    pub fn read<R: reg::Register>(&mut self, reg: R) -> Result<u16> {
+        reg.read(self)
     }
 
-    /// Writes a given C22 PHY register.
-    pub fn write(&mut self, regnum: u16, val: u16) -> Result {
-        let phydev = self.0.get();
-        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`.
-        // So it's just an FFI call, open code of `phy_write()` with a valid `phy_device` pointer
-        // `phydev`.
-        to_result(unsafe {
-            bindings::mdiobus_write((*phydev).mdio.bus, (*phydev).mdio.addr, regnum.into(), val)
-        })
+    /// Writes a PHY register.
+    pub fn write<R: reg::Register>(&mut self, reg: R, val: u16) -> Result {
+        reg.write(self, val)
     }
 
     /// Reads a paged register.
diff --git a/rust/kernel/net/phy/reg.rs b/rust/kernel/net/phy/reg.rs
new file mode 100644
index 000000000000..91f73179315e
--- /dev/null
+++ b/rust/kernel/net/phy/reg.rs
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 FUJITA Tomonori <fujita.tomonori@gmail.com>
+
+//! PHY register interfaces.
+//!
+//! This module provides support for accessing PHY registers via Ethernet
+//! management interface clause 22 and 45, as defined in IEEE 802.3.
+
+use super::Device;
+use crate::build_assert;
+use crate::error::*;
+use crate::uapi;
+
+mod private {
+    /// Marker that a trait cannot be implemented outside of this crate
+    pub trait Sealed {}
+}
+
+/// Accesses PHY registers.
+///
+/// This trait is used to implement the unified interface to access
+/// C22 and C45 PHY registers.
+///
+/// # Examples
+///
+/// ```ignore
+/// fn link_change_notify(dev: &mut Device) {
+///     // read C22 BMCR register
+///     dev.read(C22::BMCR);
+///     // read C45 PMA/PMD control 1 register
+///     dev.read(C45::new(Mmd::PMAPMD, 0));
+/// }
+/// ```
+pub trait Register: private::Sealed {
+    /// Reads a PHY register.
+    fn read(&self, dev: &mut Device) -> Result<u16>;
+
+    /// Writes a PHY register.
+    fn write(&self, dev: &mut Device, val: u16) -> Result;
+}
+
+/// A single MDIO clause 22 register address (5 bits).
+pub struct C22(u8);
+
+impl C22 {
+    /// Basic mode control.
+    pub const BMCR: Self = C22(0x00);
+    /// Basic mode status.
+    pub const BMSR: Self = C22(0x01);
+    /// PHY identifier 1.
+    pub const PHYSID1: Self = C22(0x02);
+    /// PHY identifier 2.
+    pub const PHYSID2: Self = C22(0x03);
+    /// Auto-negotiation advertisement.
+    pub const ADVERTISE: Self = C22(0x04);
+    /// Auto-negotiation link partner base page ability.
+    pub const LPA: Self = C22(0x05);
+    /// Auto-negotiation expansion.
+    pub const EXPANSION: Self = C22(0x06);
+    /// Auto-negotiation next page transmit.
+    pub const NEXT_PAGE_TRANSMIT: Self = C22(0x07);
+    /// Auto-negotiation link partner received next page.
+    pub const LP_RECEIVED_NEXT_PAGE: Self = C22(0x08);
+    /// Master-slave control.
+    pub const MASTER_SLAVE_CONTROL: Self = C22(0x09);
+    /// Master-slave status.
+    pub const MASTER_SLAVE_STATUS: Self = C22(0x0a);
+    /// PSE Control.
+    pub const PSE_CONTROL: Self = C22(0x0b);
+    /// PSE Status.
+    pub const PSE_STATUS: Self = C22(0x0c);
+    /// MMD Register control.
+    pub const MMD_CONTROL: Self = C22(0x0d);
+    /// MMD Register address data.
+    pub const MMD_DATA: Self = C22(0x0e);
+    /// Extended status.
+    pub const EXTENDED_STATUS: Self = C22(0x0f);
+
+    /// Creates a new instance of `C22` with a vendor specific register.
+    pub const fn vendor_specific<const N: u8>() -> Self {
+        build_assert!(
+            N > 0x0f && N < 0x20,
+            "Vendor-specific register address must be between 16 and 31"
+        );
+        C22(N)
+    }
+}
+
+impl private::Sealed for C22 {}
+
+impl Register for C22 {
+    fn read(&self, dev: &mut Device) -> Result<u16> {
+        let phydev = dev.0.get();
+        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Device`.
+        // So it's just an FFI call, open code of `phy_read()` with a valid `phy_device` pointer
+        // `phydev`.
+        let ret = unsafe {
+            bindings::mdiobus_read((*phydev).mdio.bus, (*phydev).mdio.addr, self.0.into())
+        };
+        to_result(ret)?;
+        Ok(ret as u16)
+    }
+
+    fn write(&self, dev: &mut Device, val: u16) -> Result {
+        let phydev = dev.0.get();
+        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Device`.
+        // So it's just an FFI call, open code of `phy_write()` with a valid `phy_device` pointer
+        // `phydev`.
+        to_result(unsafe {
+            bindings::mdiobus_write((*phydev).mdio.bus, (*phydev).mdio.addr, self.0.into(), val)
+        })
+    }
+}
+
+/// A single MDIO clause 45 register device and address.
+pub struct Mmd(u8);
+
+impl Mmd {
+    /// Physical Medium Attachment/Dependent.
+    pub const PMAPMD: Self = Mmd(uapi::MDIO_MMD_PMAPMD as u8);
+    /// WAN interface sublayer.
+    pub const WIS: Self = Mmd(uapi::MDIO_MMD_WIS as u8);
+    /// Physical coding sublayer.
+    pub const PCS: Self = Mmd(uapi::MDIO_MMD_PCS as u8);
+    /// PHY Extender sublayer.
+    pub const PHYXS: Self = Mmd(uapi::MDIO_MMD_PHYXS as u8);
+    /// DTE Extender sublayer.
+    pub const DTEXS: Self = Mmd(uapi::MDIO_MMD_DTEXS as u8);
+    /// Transmission convergence.
+    pub const TC: Self = Mmd(uapi::MDIO_MMD_TC as u8);
+    /// Auto negotiation.
+    pub const AN: Self = Mmd(uapi::MDIO_MMD_AN as u8);
+    /// Separated PMA (1).
+    pub const SEPARATED_PMA1: Self = Mmd(8);
+    /// Separated PMA (2).
+    pub const SEPARATED_PMA2: Self = Mmd(9);
+    /// Separated PMA (3).
+    pub const SEPARATED_PMA3: Self = Mmd(10);
+    /// Separated PMA (4).
+    pub const SEPARATED_PMA4: Self = Mmd(11);
+    /// OFDM PMA/PMD.
+    pub const OFDM_PMAPMD: Self = Mmd(12);
+    /// Power unit.
+    pub const POWER_UNIT: Self = Mmd(13);
+    /// Clause 22 extension.
+    pub const C22_EXT: Self = Mmd(uapi::MDIO_MMD_C22EXT as u8);
+    /// Vendor specific 1.
+    pub const VEND1: Self = Mmd(uapi::MDIO_MMD_VEND1 as u8);
+    /// Vendor specific 2.
+    pub const VEND2: Self = Mmd(uapi::MDIO_MMD_VEND2 as u8);
+}
+
+/// A single MDIO clause 45 register device and address.
+///
+/// Clause 45 uses a 5-bit device address to access a specific MMD within
+/// a port, then a 16-bit register address to access a location within
+/// that device. `C45` represents this by storing a [`Mmd`] and
+/// a register number.
+pub struct C45 {
+    devad: Mmd,
+    regnum: u16,
+}
+
+impl C45 {
+    /// Creates a new instance of `C45`.
+    pub fn new(devad: Mmd, regnum: u16) -> Self {
+        Self { devad, regnum }
+    }
+}
+
+impl private::Sealed for C45 {}
+
+impl Register for C45 {
+    fn read(&self, dev: &mut Device) -> Result<u16> {
+        let phydev = dev.0.get();
+        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Device`.
+        // So it's just an FFI call.
+        let ret =
+            unsafe { bindings::phy_read_mmd(phydev, self.devad.0.into(), self.regnum.into()) };
+        to_result(ret)?;
+        Ok(ret as u16)
+    }
+
+    fn write(&self, dev: &mut Device, val: u16) -> Result {
+        let phydev = dev.0.get();
+        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Device`.
+        // So it's just an FFI call.
+        to_result(unsafe {
+            bindings::phy_write_mmd(phydev, self.devad.0.into(), self.regnum.into(), val)
+        })
+    }
+}
diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h
index 08f5e9334c9e..76d3f103e764 100644
--- a/rust/uapi/uapi_helper.h
+++ b/rust/uapi/uapi_helper.h
@@ -7,5 +7,6 @@
  */
 
 #include <uapi/asm-generic/ioctl.h>
+#include <uapi/linux/mdio.h>
 #include <uapi/linux/mii.h>
 #include <uapi/linux/ethtool.h>
-- 
2.34.1

[RFC PATCH v4 2/2] rust: net::phy unified genphy_read_status function for C22 and C45 registers

[FUJITA Tomonori]

Add unified genphy_read_status function for C22 and C45
registers. Instead of having genphy_c22 and genphy_c45 methods, this
unifies genphy_read_status functions for C22 and C45.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/net/phy.rs     | 12 ++----------
 rust/kernel/net/phy/reg.rs | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index 9b4c4d24b231..ff16d44c7bb0 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -249,16 +249,8 @@ pub fn genphy_suspend(&mut self) -> Result {
     }
 
     /// Checks the link status and updates current link state.
-    pub fn genphy_read_status(&mut self) -> Result<u16> {
-        let phydev = self.0.get();
-        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`.
-        // So it's just an FFI call.
-        let ret = unsafe { bindings::genphy_read_status(phydev) };
-        if ret < 0 {
-            Err(Error::from_errno(ret))
-        } else {
-            Ok(ret as u16)
-        }
+    pub fn genphy_read_status<R: reg::Register>(&mut self) -> Result<u16> {
+        R::read_status(self)
     }
 
     /// Updates the link status.
diff --git a/rust/kernel/net/phy/reg.rs b/rust/kernel/net/phy/reg.rs
index 91f73179315e..4cf47335539b 100644
--- a/rust/kernel/net/phy/reg.rs
+++ b/rust/kernel/net/phy/reg.rs
@@ -30,6 +30,11 @@ pub trait Sealed {}
 ///     dev.read(C22::BMCR);
 ///     // read C45 PMA/PMD control 1 register
 ///     dev.read(C45::new(Mmd::PMAPMD, 0));
+///
+///     // Checks the link status and updates current link state via C22.
+///     dev.genphy_read_status::<phy::C22>();
+///     // Checks the link status and updates current link state via C45.
+///     dev.genphy_read_status::<phy::C45>();
 /// }
 /// ```
 pub trait Register: private::Sealed {
@@ -38,6 +43,9 @@ pub trait Register: private::Sealed {
 
     /// Writes a PHY register.
     fn write(&self, dev: &mut Device, val: u16) -> Result;
+
+    /// Checks the link status and updates current link state.
+    fn read_status(dev: &mut Device) -> Result<u16>;
 }
 
 /// A single MDIO clause 22 register address (5 bits).
@@ -111,6 +119,15 @@ fn write(&self, dev: &mut Device, val: u16) -> Result {
             bindings::mdiobus_write((*phydev).mdio.bus, (*phydev).mdio.addr, self.0.into(), val)
         })
     }
+
+    fn read_status(dev: &mut Device) -> Result<u16> {
+        let phydev = dev.0.get();
+        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`.
+        // So it's just an FFI call.
+        let ret = unsafe { bindings::genphy_read_status(phydev) };
+        to_result(ret)?;
+        Ok(ret as u16)
+    }
 }
 
 /// A single MDIO clause 45 register device and address.
@@ -190,4 +207,13 @@ fn write(&self, dev: &mut Device, val: u16) -> Result {
             bindings::phy_write_mmd(phydev, self.devad.0.into(), self.regnum.into(), val)
         })
     }
+
+    fn read_status(dev: &mut Device) -> Result<u16> {
+        let phydev = dev.0.get();
+        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`.
+        // So it's just an FFI call.
+        let ret = unsafe { bindings::genphy_c45_read_status(phydev) };
+        to_result(ret)?;
+        Ok(ret as u16)
+    }
 }
-- 
2.34.1

Re: [RFC PATCH v4 1/2] rust: net::phy unified read/write API for C22 and C45 registers

[Trevor Gross]

On Fri, Jun 7, 2024 at 12:22 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Add the unified read/write API for C22 and C45 registers. The
> abstractions support access to only C22 registers now. Instead of
> adding read/write_c45 methods specifically for C45, a new reg module
> supports the unified API to access C22 and C45 registers with trait,
> by calling an appropriate phylib functions.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

This new API looks good to me:

Reviewed-by: Trevor Gross <tmgross@umich.edu>

It should probably also get Benno's review since he suggested the API.

Re: [RFC PATCH v4 2/2] rust: net::phy unified genphy_read_status function for C22 and C45 registers

[Trevor Gross]

On Fri, Jun 7, 2024 at 12:22 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Add unified genphy_read_status function for C22 and C45
> registers. Instead of having genphy_c22 and genphy_c45 methods, this
> unifies genphy_read_status functions for C22 and C45.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

Re: [RFC PATCH v4 1/2] rust: net::phy unified read/write API for C22 and C45 registers

[FUJITA Tomonori]

On Thu, 13 Jun 2024 12:59:52 -0500
Trevor Gross <tmgross@umich.edu> wrote:

>> Add the unified read/write API for C22 and C45 registers. The
>> abstractions support access to only C22 registers now. Instead of
>> adding read/write_c45 methods specifically for C45, a new reg module
>> supports the unified API to access C22 and C45 registers with trait,
>> by calling an appropriate phylib functions.
>>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> This new API looks good to me:
> 
> Reviewed-by: Trevor Gross <tmgross@umich.edu>

Thanks a lot!

> It should probably also get Benno's review since he suggested the API.

Indeed. Benno, that patchset looks good?

Re: [RFC PATCH v4 1/2] rust: net::phy unified read/write API for C22 and C45 registers

[Benno Lossin]

On 14.06.24 01:32, FUJITA Tomonori wrote:
> On Thu, 13 Jun 2024 12:59:52 -0500
> Trevor Gross <tmgross@umich.edu> wrote:
> 
>>> Add the unified read/write API for C22 and C45 registers. The
>>> abstractions support access to only C22 registers now. Instead of
>>> adding read/write_c45 methods specifically for C45, a new reg module
>>> supports the unified API to access C22 and C45 registers with trait,
>>> by calling an appropriate phylib functions.
>>>
>>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>
>> This new API looks good to me:
>>
>> Reviewed-by: Trevor Gross <tmgross@umich.edu>
> 
> Thanks a lot!
> 
>> It should probably also get Benno's review since he suggested the API.
> 
> Indeed. Benno, that patchset looks good?

I haven't had a lot of time recently, I can try to take a look today or
the next few days.

I am a bit confused though, why is this marked as RFC? The previous
patches weren't RFCs.

---
Cheers,
Benno

Re: [RFC PATCH v4 1/2] rust: net::phy unified read/write API for C22 and C45 registers

[FUJITA Tomonori]

On Fri, 14 Jun 2024 08:30:56 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

>>> It should probably also get Benno's review since he suggested the API.
>> 
>> Indeed. Benno, that patchset looks good?
> 
> I haven't had a lot of time recently, I can try to take a look today or
> the next few days.

Sorry, no need to rust.

> I am a bit confused though, why is this marked as RFC? The previous
> patches weren't RFCs.

Oops, my bad. Shouldn't have benn marked as RFC.

Re: [RFC PATCH v4 0/2] net::phy add unified API for C22 and C45 registers

[Benno Lossin]

On 07.06.24 07:21, FUJITA Tomonori wrote:
> add unified API for C22 and C45, reading/writing registers and
> genphy_read_status().
> 
> We could add methods specifically C45 like read/write_c45 and
> genphy_c45_read_status. Instead, this implements the unified API for
> C22 and C45 with a new reg module.
> 
> v4:
> - make reg module sealed
> - improve the comments
> - add examples
> - use error::to_result() to simplify some functions
> v3: https://lore.kernel.org/all/20240603.233954.1644803151268035787.fujita.tomonori@gmail.com/T/
> - move reg module to a new file (with MAINTAINERS updated)
> - rewrite commit log and subjects
> - rename Access trait to Register
> - add build error message for build_assert!
> - define the complete MMD list
> - drop new func for Mmd
> - remove trait GenPhyOps
> v2: https://lore.kernel.org/all/20240601043535.53545-2-fujita.tomonori@gmail.com/T/
> - add genphy helper support and split the patch
> - create a 'reg' module
> - define C22 according to the specs
> - handle C22 vendor specific registers
> - add const for C45 MMD
> v1: https://lore.kernel.org/all/20240530.145258.456915814420167538.fujita.tomonori@gmail.com/T/
> 
> FUJITA Tomonori (2):
>   rust: net::phy unified read/write API for C22 and C45 registers
>   rust: net::phy unified genphy_read_status function for C22 and C45
>     registers
> 
>  MAINTAINERS                      |   1 +
>  drivers/net/phy/ax88796b_rust.rs |   7 +-
>  rust/kernel/net/phy.rs           |  44 ++-----
>  rust/kernel/net/phy/reg.rs       | 219 +++++++++++++++++++++++++++++++
>  rust/uapi/uapi_helper.h          |   1 +
>  5 files changed, 234 insertions(+), 38 deletions(-)
>  create mode 100644 rust/kernel/net/phy/reg.rs
> 
> 
> base-commit: c790275b5edf5d8280ae520bda7c1f37da460c00
> --
> 2.34.1
> 

Both patches:

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

---
Cheers,
Benno

Re: [RFC PATCH v4 0/2] net::phy add unified API for C22 and C45 registers

[FUJITA Tomonori]

On Fri, 14 Jun 2024 12:53:33 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

>> FUJITA Tomonori (2):
>>   rust: net::phy unified read/write API for C22 and C45 registers
>>   rust: net::phy unified genphy_read_status function for C22 and C45
>>     registers
>> 
>>  MAINTAINERS                      |   1 +
>>  drivers/net/phy/ax88796b_rust.rs |   7 +-
>>  rust/kernel/net/phy.rs           |  44 ++-----
>>  rust/kernel/net/phy/reg.rs       | 219 +++++++++++++++++++++++++++++++
>>  rust/uapi/uapi_helper.h          |   1 +
>>  5 files changed, 234 insertions(+), 38 deletions(-)
>>  create mode 100644 rust/kernel/net/phy/reg.rs
>> 
>> 
>> base-commit: c790275b5edf5d8280ae520bda7c1f37da460c00
>> --
>> 2.34.1
>> 
> 
> Both patches:
> 
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>

Thanks, much appreciated.

Thanks to your suggestions, the API has become much better than my
initial version.