[PATCH v2 net-next 0/3] net: phy: realtek: improve MMD register access for internal PHY's

[PATCH v2 net-next 0/3] net: phy: realtek: improve MMD register access for internal PHY's

[Heiner Kallweit]

The integrated PHYs on chip versions from RTL8168g allow to address
MDIO_MMD_VEND2 registers. All c22 standard registers are mapped to
MDIO_MMD_VEND2 registers. So far the paging mechanism is used to
address PHY registers. Add support for c45 ops to address MDIO_MMD_VEND2
registers directly, w/o the paging.

v2:
- remove superfluous space in patch 1
- make check in r8169_mdio_write_reg_c45 more strict

Heiner Kallweit (3):
  r8169: add PHY c45 ops for MII_MMD_VENDOR2 registers
  net: phy: realtek: improve mmd register access for internal PHY's
  net: phy: realtek: switch from paged to mmd ops in rtl822x functions

 drivers/net/ethernet/realtek/r8169_main.c | 32 ++++++++
 drivers/net/phy/realtek/realtek_main.c    | 90 ++++++++++-------------
 2 files changed, 70 insertions(+), 52 deletions(-)

-- 
2.48.1

[PATCH v2 net-next 1/3] r8169: add PHY c45 ops for MDIO_MMD_VENDOR2 registers

[Heiner Kallweit]

The integrated PHYs on chip versions from RTL8168g allow to address
MDIO_MMD_VEND2 registers. All c22 standard registers are mapped to
MDIO_MMD_VEND2 registers. So far the paging mechanism is used to
address PHY registers. Add support for c45 ops to address MDIO_MMD_VEND2
registers directly, w/o the paging.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- remove superfluous space
- make check in r8169_mdio_write_reg_c45 more strict
---
 drivers/net/ethernet/realtek/r8169_main.c | 32 +++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 9fe53322d..fa339bd8c 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5200,6 +5200,33 @@ static int r8169_mdio_write_reg(struct mii_bus *mii_bus, int phyaddr,
 	return 0;
 }
 
+static int r8169_mdio_read_reg_c45(struct mii_bus *mii_bus, int addr,
+				   int devnum, int regnum)
+{
+	struct rtl8169_private *tp = mii_bus->priv;
+
+	if (addr > 0)
+		return -ENODEV;
+
+	if (devnum == MDIO_MMD_VEND2 && regnum > MDIO_STAT2)
+		return r8168_phy_ocp_read(tp, regnum);
+
+	return 0;
+}
+
+static int r8169_mdio_write_reg_c45(struct mii_bus *mii_bus, int addr,
+				    int devnum, int regnum, u16 val)
+{
+	struct rtl8169_private *tp = mii_bus->priv;
+
+	if (addr > 0 || devnum != MDIO_MMD_VEND2 || regnum <= MDIO_STAT2)
+		return -ENODEV;
+
+	r8168_phy_ocp_write(tp, regnum, val);
+
+	return 0;
+}
+
 static int r8169_mdio_register(struct rtl8169_private *tp)
 {
 	struct pci_dev *pdev = tp->pci_dev;
@@ -5230,6 +5257,11 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
 	new_bus->read = r8169_mdio_read_reg;
 	new_bus->write = r8169_mdio_write_reg;
 
+	if (tp->mac_version >= RTL_GIGA_MAC_VER_40) {
+		new_bus->read_c45 = r8169_mdio_read_reg_c45;
+		new_bus->write_c45 = r8169_mdio_write_reg_c45;
+	}
+
 	ret = devm_mdiobus_register(&pdev->dev, new_bus);
 	if (ret)
 		return ret;
-- 
2.48.1

Re: [PATCH v2 net-next 1/3] r8169: add PHY c45 ops for MDIO_MMD_VENDOR2 registers

[Andrew Lunn]

On Thu, Feb 13, 2025 at 08:15:42PM +0100, Heiner Kallweit wrote:
> The integrated PHYs on chip versions from RTL8168g allow to address
> MDIO_MMD_VEND2 registers. All c22 standard registers are mapped to
> MDIO_MMD_VEND2 registers. So far the paging mechanism is used to
> address PHY registers. Add support for c45 ops to address MDIO_MMD_VEND2
> registers directly, w/o the paging.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v2 net-next 2/3] net: phy: realtek: improve mmd register access for internal PHY's

[Heiner Kallweit]

r8169 provides the MDIO bus for the internal PHY's. It has been extended
with c45 access functions for addressing MDIO_MMD_VEND2 registers.
So we can switch from paged access to directly addressing the
MDIO_MMD_VEND2 registers.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/realtek/realtek_main.c | 79 +++++++++++---------------
 1 file changed, 33 insertions(+), 46 deletions(-)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 210fefac4..2e2c5353c 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -735,29 +735,31 @@ static int rtlgen_read_status(struct phy_device *phydev)
 	return 0;
 }
 
+static int rtlgen_read_vend2(struct phy_device *phydev, int regnum)
+{
+	return __mdiobus_c45_read(phydev->mdio.bus, 0, MDIO_MMD_VEND2, regnum);
+}
+
+static int rtlgen_write_vend2(struct phy_device *phydev, int regnum, u16 val)
+{
+	return __mdiobus_c45_write(phydev->mdio.bus, 0, MDIO_MMD_VEND2, regnum,
+				   val);
+}
+
 static int rtlgen_read_mmd(struct phy_device *phydev, int devnum, u16 regnum)
 {
 	int ret;
 
-	if (devnum == MDIO_MMD_VEND2) {
-		rtl821x_write_page(phydev, regnum >> 4);
-		ret = __phy_read(phydev, 0x10 + ((regnum & 0xf) >> 1));
-		rtl821x_write_page(phydev, 0);
-	} else if (devnum == MDIO_MMD_PCS && regnum == MDIO_PCS_EEE_ABLE) {
-		rtl821x_write_page(phydev, 0xa5c);
-		ret = __phy_read(phydev, 0x12);
-		rtl821x_write_page(phydev, 0);
-	} else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV) {
-		rtl821x_write_page(phydev, 0xa5d);
-		ret = __phy_read(phydev, 0x10);
-		rtl821x_write_page(phydev, 0);
-	} else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_LPABLE) {
-		rtl821x_write_page(phydev, 0xa5d);
-		ret = __phy_read(phydev, 0x11);
-		rtl821x_write_page(phydev, 0);
-	} else {
+	if (devnum == MDIO_MMD_VEND2)
+		ret = rtlgen_read_vend2(phydev, regnum);
+	else if (devnum == MDIO_MMD_PCS && regnum == MDIO_PCS_EEE_ABLE)
+		ret = rtlgen_read_vend2(phydev, 0xa5c4);
+	else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV)
+		ret = rtlgen_read_vend2(phydev, 0xa5d0);
+	else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_LPABLE)
+		ret = rtlgen_read_vend2(phydev, 0xa5d2);
+	else
 		ret = -EOPNOTSUPP;
-	}
 
 	return ret;
 }
@@ -767,17 +769,12 @@ static int rtlgen_write_mmd(struct phy_device *phydev, int devnum, u16 regnum,
 {
 	int ret;
 
-	if (devnum == MDIO_MMD_VEND2) {
-		rtl821x_write_page(phydev, regnum >> 4);
-		ret = __phy_write(phydev, 0x10 + ((regnum & 0xf) >> 1), val);
-		rtl821x_write_page(phydev, 0);
-	} else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV) {
-		rtl821x_write_page(phydev, 0xa5d);
-		ret = __phy_write(phydev, 0x10, val);
-		rtl821x_write_page(phydev, 0);
-	} else {
+	if (devnum == MDIO_MMD_VEND2)
+		ret = rtlgen_write_vend2(phydev, regnum, val);
+	else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV)
+		ret = rtlgen_write_vend2(phydev, regnum, 0xa5d0);
+	else
 		ret = -EOPNOTSUPP;
-	}
 
 	return ret;
 }
@@ -789,19 +786,12 @@ static int rtl822x_read_mmd(struct phy_device *phydev, int devnum, u16 regnum)
 	if (ret != -EOPNOTSUPP)
 		return ret;
 
-	if (devnum == MDIO_MMD_PCS && regnum == MDIO_PCS_EEE_ABLE2) {
-		rtl821x_write_page(phydev, 0xa6e);
-		ret = __phy_read(phydev, 0x16);
-		rtl821x_write_page(phydev, 0);
-	} else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV2) {
-		rtl821x_write_page(phydev, 0xa6d);
-		ret = __phy_read(phydev, 0x12);
-		rtl821x_write_page(phydev, 0);
-	} else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_LPABLE2) {
-		rtl821x_write_page(phydev, 0xa6d);
-		ret = __phy_read(phydev, 0x10);
-		rtl821x_write_page(phydev, 0);
-	}
+	if (devnum == MDIO_MMD_PCS && regnum == MDIO_PCS_EEE_ABLE2)
+		ret = rtlgen_read_vend2(phydev, 0xa6ec);
+	else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV2)
+		ret = rtlgen_read_vend2(phydev, 0xa6d4);
+	else if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_LPABLE2)
+		ret = rtlgen_read_vend2(phydev, 0xa6d0);
 
 	return ret;
 }
@@ -814,11 +804,8 @@ static int rtl822x_write_mmd(struct phy_device *phydev, int devnum, u16 regnum,
 	if (ret != -EOPNOTSUPP)
 		return ret;
 
-	if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV2) {
-		rtl821x_write_page(phydev, 0xa6d);
-		ret = __phy_write(phydev, 0x12, val);
-		rtl821x_write_page(phydev, 0);
-	}
+	if (devnum == MDIO_MMD_AN && regnum == MDIO_AN_EEE_ADV2)
+		ret = rtlgen_write_vend2(phydev, 0xa6d4, val);
 
 	return ret;
 }
-- 
2.48.1

Re: [PATCH v2 net-next 2/3] net: phy: realtek: improve mmd register access for internal PHY's

[Andrew Lunn]

On Thu, Feb 13, 2025 at 08:18:17PM +0100, Heiner Kallweit wrote:
> r8169 provides the MDIO bus for the internal PHY's. It has been extended
> with c45 access functions for addressing MDIO_MMD_VEND2 registers.
> So we can switch from paged access to directly addressing the
> MDIO_MMD_VEND2 registers.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH v2 net-next 3/3] net: phy: realtek: switch from paged to MMD ops in rtl822x functions

[Heiner Kallweit]

The MDIO bus provided by r8169 for the internal PHY's now supports
c45 ops for the MDIO_MMD_VEND2 device. So we can switch to standard
MMD ops here.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/realtek/realtek_main.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 2e2c5353c..34be1d752 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -901,7 +901,7 @@ static int rtl822x_get_features(struct phy_device *phydev)
 {
 	int val;
 
-	val = phy_read_paged(phydev, 0xa61, 0x13);
+	val = phy_read_mmd(phydev, MDIO_MMD_VEND2, 0xa616);
 	if (val < 0)
 		return val;
 
@@ -922,10 +922,9 @@ static int rtl822x_config_aneg(struct phy_device *phydev)
 	if (phydev->autoneg == AUTONEG_ENABLE) {
 		u16 adv = linkmode_adv_to_mii_10gbt_adv_t(phydev->advertising);
 
-		ret = phy_modify_paged_changed(phydev, 0xa5d, 0x12,
-					       MDIO_AN_10GBT_CTRL_ADV2_5G |
-					       MDIO_AN_10GBT_CTRL_ADV5G,
-					       adv);
+		ret = phy_modify_mmd_changed(phydev, MDIO_MMD_VEND2, 0xa5d4,
+					     MDIO_AN_10GBT_CTRL_ADV2_5G |
+					     MDIO_AN_10GBT_CTRL_ADV5G, adv);
 		if (ret < 0)
 			return ret;
 	}
@@ -969,7 +968,7 @@ static int rtl822x_read_status(struct phy_device *phydev)
 	    !phydev->autoneg_complete)
 		return 0;
 
-	lpadv = phy_read_paged(phydev, 0xa5d, 0x13);
+	lpadv = phy_read_mmd(phydev, MDIO_MMD_VEND2, 0xa5d6);
 	if (lpadv < 0)
 		return lpadv;
 
-- 
2.48.1

Re: [PATCH v2 net-next 3/3] net: phy: realtek: switch from paged to MMD ops in rtl822x functions

[Andrew Lunn]

On Thu, Feb 13, 2025 at 08:19:14PM +0100, Heiner Kallweit wrote:
> The MDIO bus provided by r8169 for the internal PHY's now supports
> c45 ops for the MDIO_MMD_VEND2 device. So we can switch to standard
> MMD ops here.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>  {
>  	int val;
>  
> -	val = phy_read_paged(phydev, 0xa61, 0x13);
> +	val = phy_read_mmd(phydev, MDIO_MMD_VEND2, 0xa616);

It is nice to see some magic numbers gone. Maybe as a followup add
#defines for these registers? Are they standard registers, just in odd
places? So you could base there name on the standard register name,
but with a vendor prefix?

Thanks
	Andrew

Re: [PATCH v2 net-next 3/3] net: phy: realtek: switch from paged to MMD ops in rtl822x functions

[Heiner Kallweit]

On 14.02.2025 16:46, Andrew Lunn wrote:
> On Thu, Feb 13, 2025 at 08:19:14PM +0100, Heiner Kallweit wrote:
>> The MDIO bus provided by r8169 for the internal PHY's now supports
>> c45 ops for the MDIO_MMD_VEND2 device. So we can switch to standard
>> MMD ops here.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>>  {
>>  	int val;
>>  
>> -	val = phy_read_paged(phydev, 0xa61, 0x13);
>> +	val = phy_read_mmd(phydev, MDIO_MMD_VEND2, 0xa616);
> 
> It is nice to see some magic numbers gone. Maybe as a followup add
> #defines for these registers? Are they standard registers, just in odd
> places? So you could base there name on the standard register name,
> but with a vendor prefix?
> 

Most of the registers are standard registers which are shadowed in
VEND2 device. E.g. 0xa616 is 45.2.1.4 (PMA/PMD speed ability).
So yes, it would make sense to add defines based on the standard
register name.


> Thanks
> 	Andrew
> 
Heiner

Re: [PATCH v2 net-next 0/3] net: phy: realtek: improve MMD register access for internal PHY's

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 13 Feb 2025 20:14:09 +0100 you wrote:
> The integrated PHYs on chip versions from RTL8168g allow to address
> MDIO_MMD_VEND2 registers. All c22 standard registers are mapped to
> MDIO_MMD_VEND2 registers. So far the paging mechanism is used to
> address PHY registers. Add support for c45 ops to address MDIO_MMD_VEND2
> registers directly, w/o the paging.
> 
> v2:
> - remove superfluous space in patch 1
> - make check in r8169_mdio_write_reg_c45 more strict
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/3] r8169: add PHY c45 ops for MDIO_MMD_VENDOR2 registers
    https://git.kernel.org/netdev/net-next/c/853e80369cfc
  - [v2,net-next,2/3] net: phy: realtek: improve mmd register access for internal PHY's
    https://git.kernel.org/netdev/net-next/c/da681ed73fb9
  - [v2,net-next,3/3] net: phy: realtek: switch from paged to MMD ops in rtl822x functions
    https://git.kernel.org/netdev/net-next/c/02d3b306ac2f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html