[PATCH v2 0/5] net: phy: realtek: simplify and reunify C22/C45 drivers

[PATCH v2 0/5] net: phy: realtek: simplify and reunify C22/C45 drivers

[Daniel Golle]

The RTL8221B PHY variants (VB-CG and VM-CG) were previously split into
separate C22 and C45 driver instances to support copper SFP modules
using the RollBall MDIO-over-I2C protocol, which only supports Clause-45
access. However, this split created significant code duplication and
complexity.

Commit 8af2136e77989 ("net: phy: realtek: add helper
RTL822X_VND2_C22_REG") exposed that RealTek PHYs map all standard
Clause-22 registers into MDIO_MMD_VEND2 at offset 0xa400.

With commit 1850ec20d6e71 ("net: phy: realtek: use paged access for
MDIO_MMD_VEND2 in C22 mode") it is now possible to access all MMD
registers transparently, regardless of whether the PHY is accessed via
C22 or C45 MDIO.

Further improve the translation logic for this register mapping, so a
single unified driver works efficiently with both access methods,
reducing code duplication.

The series also includes cleanup to remove unnecessary paged operations
on registers that aren't actually affected by page selection.

Testing was done on RTL8211F and RTL8221B-VB-CG (the latter in both
C22 and C45 modes).

Changes since initial submission:
 - rebase on top of
   "net: phy: realtek: add dummy PHY driver for RTL8127ATF"
 - improve commit messages

Daniel Golle (5):
  net: phy: realtek: support interrupt also for C22 variants
  net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2
  net: phy: realtek: reunify C22 and C45 drivers
  net: phy: realtek: demystify PHYSR register location
  net: phy: realtek: simplify bogus paged operations

 drivers/net/phy/realtek/realtek_main.c | 124 ++++++++++---------------
 1 file changed, 48 insertions(+), 76 deletions(-)

-- 
2.52.0

[PATCH v2 1/5] net: phy: realtek: support interrupt also for C22 variants

[Daniel Golle]

Now that access to MDIO_MMD_VEND2 works transparently also in Clause-22
mode, add interrupt support also for the C22 variants of the
RTL8221B-VB-CG and RTL8221B-VM-CG. This results in the C22 and C45
driver instances now having all the same features implemented.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: no changes

 drivers/net/phy/realtek/realtek_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 5a7f472bf58e1..18eea6b4b59a6 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -2315,6 +2315,8 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		.match_phy_device = rtl8221b_vb_cg_c22_match_phy_device,
 		.name		= "RTL8221B-VB-CG 2.5Gbps PHY (C22)",
+		.config_intr	= rtl8221b_config_intr,
+		.handle_interrupt = rtl8221b_handle_interrupt,
 		.probe		= rtl822x_probe,
 		.get_features	= rtl822x_get_features,
 		.config_aneg	= rtl822x_config_aneg,
@@ -2347,6 +2349,8 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		.match_phy_device = rtl8221b_vm_cg_c22_match_phy_device,
 		.name		= "RTL8221B-VM-CG 2.5Gbps PHY (C22)",
+		.config_intr	= rtl8221b_config_intr,
+		.handle_interrupt = rtl8221b_handle_interrupt,
 		.probe		= rtl822x_probe,
 		.get_features	= rtl822x_get_features,
 		.config_aneg	= rtl822x_config_aneg,
-- 
2.52.0

[PATCH v2 2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

[Daniel Golle]

RealTek 2.5GE PHYs have all standard Clause-22 registers mapped also
inside MDIO_MMD_VEND2 at offset 0xa400. This is used mainly in case the
PHY is connected to a Clause-45-only bus. The RTL8221B is frequently
used in copper SFP module which uses the RollBall MDIO-over-I2C
method which *only* supports Clause-45, for example.

In order to support using the PHY on Clause-45-only busses, the PHY
driver has previously been split into a C22-only and C45-only instances,
creating quite a bit of redundancy and confusion.

In preparation of reunifying the two driver instances, add support for
translating MDIO_MMD_VEND2 registers 0xa400 to 0xa43c back to Clause-22
registers 0 to 30 in case the PHY is accessed on a Clause-22 bus.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: improve commit message

 drivers/net/phy/realtek/realtek_main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 18eea6b4b59a6..74980b2d66157 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -144,6 +144,7 @@
 
 #define RTL822X_VND2_TO_PAGE(reg)		((reg) >> 4)
 #define RTL822X_VND2_TO_PAGE_REG(reg)		(16 + (((reg) & GENMASK(3, 0)) >> 1))
+#define RTL822X_VND2_TO_C22_REG(reg)		(((reg) - 0xa400) / 2)
 #define RTL822X_VND2_C22_REG(reg)		(0xa400 + 2 * (reg))
 
 #define RTL8221B_VND2_INER			0xa4d2
@@ -1265,6 +1266,11 @@ static int rtl822xb_read_mmd(struct phy_device *phydev, int devnum, u16 reg)
 		return mmd_phy_read(phydev->mdio.bus, phydev->mdio.addr,
 				    phydev->is_c45, devnum, reg);
 
+	/* Simplify access to C22-registers addressed inside MDIO_MMD_VEND2 */
+	if (reg >= RTL822X_VND2_C22_REG(0) &&
+	    reg <= RTL822X_VND2_C22_REG(30))
+		return __phy_read(phydev, RTL822X_VND2_TO_C22_REG(reg));
+
 	/* Use paged access for MDIO_MMD_VEND2 over Clause-22 */
 	page = RTL822X_VND2_TO_PAGE(reg);
 	oldpage = __phy_read(phydev, RTL821x_PAGE_SELECT);
@@ -1300,6 +1306,11 @@ static int rtl822xb_write_mmd(struct phy_device *phydev, int devnum, u16 reg,
 		return mmd_phy_write(phydev->mdio.bus, phydev->mdio.addr,
 				     phydev->is_c45, devnum, reg, val);
 
+	/* Simplify access to C22-registers addressed inside MDIO_MMD_VEND2 */
+	if (reg >= RTL822X_VND2_C22_REG(0) &&
+	    reg <= RTL822X_VND2_C22_REG(30))
+		return __phy_write(phydev, RTL822X_VND2_TO_C22_REG(reg), val);
+
 	/* Use paged access for MDIO_MMD_VEND2 over Clause-22 */
 	page = RTL822X_VND2_TO_PAGE(reg);
 	oldpage = __phy_read(phydev, RTL821x_PAGE_SELECT);
-- 
2.52.0

[PATCH v2 3/5] net: phy: realtek: reunify C22 and C45 drivers

[Daniel Golle]

Reunify the split C22/C45 drivers for the RTL8221B-VB-CG 2.5Gbps and
RTL8221B-VM-CG 2.5Gbps PHYs back into a single driver.

This is possible now by using all the driver operations previously used
by the C45 driver, as transparent access to all MMDs including
MDIO_MMD_VEND2 is now possible also over Clause-22 MDIO.

The unified driver will still only use Clause-45 access on any Clause-45
capable busses while still working fine on Clause-22 busses.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: improve commit message

 drivers/net/phy/realtek/realtek_main.c | 72 ++++++--------------------
 1 file changed, 16 insertions(+), 56 deletions(-)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 74980b2d66157..4512fad3f64b8 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -1880,28 +1880,18 @@ static int rtl8221b_match_phy_device(struct phy_device *phydev,
 	return phydev->phy_id == RTL_8221B && rtlgen_supports_mmd(phydev);
 }
 
-static int rtl8221b_vb_cg_c22_match_phy_device(struct phy_device *phydev,
-					       const struct phy_driver *phydrv)
+static int rtl8221b_vb_cg_match_phy_device(struct phy_device *phydev,
+					   const struct phy_driver *phydrv)
 {
-	return rtlgen_is_c45_match(phydev, RTL_8221B_VB_CG, false);
+	return rtlgen_is_c45_match(phydev, RTL_8221B_VB_CG, true) ||
+	       rtlgen_is_c45_match(phydev, RTL_8221B_VB_CG, false);
 }
 
-static int rtl8221b_vb_cg_c45_match_phy_device(struct phy_device *phydev,
-					       const struct phy_driver *phydrv)
+static int rtl8221b_vm_cg_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_vm_cg_c22_match_phy_device(struct phy_device *phydev,
-					       const struct phy_driver *phydrv)
-{
-	return rtlgen_is_c45_match(phydev, RTL_8221B_VM_CG, false);
-}
-
-static int rtl8221b_vm_cg_c45_match_phy_device(struct phy_device *phydev,
-					       const struct phy_driver *phydrv)
-{
-	return rtlgen_is_c45_match(phydev, RTL_8221B_VM_CG, true);
+	return rtlgen_is_c45_match(phydev, RTL_8221B_VM_CG, true) ||
+	       rtlgen_is_c45_match(phydev, RTL_8221B_VM_CG, false);
 }
 
 static int rtl_internal_nbaset_match_phy_device(struct phy_device *phydev,
@@ -2324,27 +2314,8 @@ static struct phy_driver realtek_drvs[] = {
 		.read_mmd	= rtl822xb_read_mmd,
 		.write_mmd	= rtl822xb_write_mmd,
 	}, {
-		.match_phy_device = rtl8221b_vb_cg_c22_match_phy_device,
-		.name		= "RTL8221B-VB-CG 2.5Gbps PHY (C22)",
-		.config_intr	= rtl8221b_config_intr,
-		.handle_interrupt = rtl8221b_handle_interrupt,
-		.probe		= rtl822x_probe,
-		.get_features	= rtl822x_get_features,
-		.config_aneg	= rtl822x_config_aneg,
-		.config_init	= rtl822xb_config_init,
-		.inband_caps	= rtl822x_inband_caps,
-		.config_inband	= rtl822x_config_inband,
-		.get_rate_matching = rtl822xb_get_rate_matching,
-		.read_status	= rtl822xb_read_status,
-		.suspend	= genphy_suspend,
-		.resume		= rtlgen_resume,
-		.read_page	= rtl821x_read_page,
-		.write_page	= rtl821x_write_page,
-		.read_mmd	= rtl822xb_read_mmd,
-		.write_mmd	= rtl822xb_write_mmd,
-	}, {
-		.match_phy_device = rtl8221b_vb_cg_c45_match_phy_device,
-		.name		= "RTL8221B-VB-CG 2.5Gbps PHY (C45)",
+		.match_phy_device = rtl8221b_vb_cg_match_phy_device,
+		.name		= "RTL8221B-VB-CG 2.5Gbps PHY",
 		.config_intr	= rtl8221b_config_intr,
 		.handle_interrupt = rtl8221b_handle_interrupt,
 		.probe		= rtl822x_probe,
@@ -2357,28 +2328,13 @@ static struct phy_driver realtek_drvs[] = {
 		.read_status	= rtl822xb_c45_read_status,
 		.suspend	= genphy_c45_pma_suspend,
 		.resume		= rtlgen_c45_resume,
-	}, {
-		.match_phy_device = rtl8221b_vm_cg_c22_match_phy_device,
-		.name		= "RTL8221B-VM-CG 2.5Gbps PHY (C22)",
-		.config_intr	= rtl8221b_config_intr,
-		.handle_interrupt = rtl8221b_handle_interrupt,
-		.probe		= rtl822x_probe,
-		.get_features	= rtl822x_get_features,
-		.config_aneg	= rtl822x_config_aneg,
-		.config_init	= rtl822xb_config_init,
-		.inband_caps	= rtl822x_inband_caps,
-		.config_inband	= rtl822x_config_inband,
-		.get_rate_matching = rtl822xb_get_rate_matching,
-		.read_status	= rtl822xb_read_status,
-		.suspend	= genphy_suspend,
-		.resume		= rtlgen_resume,
 		.read_page	= rtl821x_read_page,
 		.write_page	= rtl821x_write_page,
 		.read_mmd	= rtl822xb_read_mmd,
 		.write_mmd	= rtl822xb_write_mmd,
 	}, {
-		.match_phy_device = rtl8221b_vm_cg_c45_match_phy_device,
-		.name		= "RTL8221B-VM-CG 2.5Gbps PHY (C45)",
+		.match_phy_device = rtl8221b_vm_cg_match_phy_device,
+		.name		= "RTL8221B-VM-CG 2.5Gbps PHY",
 		.config_intr	= rtl8221b_config_intr,
 		.handle_interrupt = rtl8221b_handle_interrupt,
 		.probe		= rtl822x_probe,
@@ -2391,6 +2347,10 @@ static struct phy_driver realtek_drvs[] = {
 		.read_status	= rtl822xb_c45_read_status,
 		.suspend	= genphy_c45_pma_suspend,
 		.resume		= rtlgen_c45_resume,
+		.read_page	= rtl821x_read_page,
+		.write_page	= rtl821x_write_page,
+		.read_mmd	= rtl822xb_read_mmd,
+		.write_mmd	= rtl822xb_write_mmd,
 	}, {
 		.match_phy_device = rtl8251b_c45_match_phy_device,
 		.name		= "RTL8251B 5Gbps PHY",
-- 
2.52.0

[PATCH v2 4/5] net: phy: realtek: demystify PHYSR register location

[Daniel Golle]

Turns out that register address RTL_VND2_PHYSR (0xa434) maps to
Clause-22 register MII_RESV2. Use that to get rid of yet another magic
number, and rename access macros accordingly.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: rebase on top of recent commit
    "net: phy: realtek: add dummy PHY driver for RTL8127ATF"
    replacing newly added reference to RTL_VND2_PHYSR

 drivers/net/phy/realtek/realtek_main.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 4512fad3f64b8..8f1c8424e7f94 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -179,12 +179,12 @@
 #define RTL9000A_GINMR				0x14
 #define RTL9000A_GINMR_LINK_STATUS		BIT(4)
 
-#define RTL_VND2_PHYSR				0xa434
-#define RTL_VND2_PHYSR_DUPLEX			BIT(3)
-#define RTL_VND2_PHYSR_SPEEDL			GENMASK(5, 4)
-#define RTL_VND2_PHYSR_SPEEDH			GENMASK(10, 9)
-#define RTL_VND2_PHYSR_MASTER			BIT(11)
-#define RTL_VND2_PHYSR_SPEED_MASK		(RTL_VND2_PHYSR_SPEEDL | RTL_VND2_PHYSR_SPEEDH)
+#define RTL_PHYSR				MII_RESV2
+#define RTL_PHYSR_DUPLEX			BIT(3)
+#define RTL_PHYSR_SPEEDL			GENMASK(5, 4)
+#define RTL_PHYSR_SPEEDH			GENMASK(10, 9)
+#define RTL_PHYSR_MASTER			BIT(11)
+#define RTL_PHYSR_SPEED_MASK			(RTL_PHYSR_SPEEDL | RTL_PHYSR_SPEEDH)
 
 #define	RTL_MDIO_PCS_EEE_ABLE			0xa5c4
 #define	RTL_MDIO_AN_EEE_ADV			0xa5d0
@@ -1103,12 +1103,12 @@ static void rtlgen_decode_physr(struct phy_device *phydev, int val)
 	 * 0: Half Duplex
 	 * 1: Full Duplex
 	 */
-	if (val & RTL_VND2_PHYSR_DUPLEX)
+	if (val & RTL_PHYSR_DUPLEX)
 		phydev->duplex = DUPLEX_FULL;
 	else
 		phydev->duplex = DUPLEX_HALF;
 
-	switch (val & RTL_VND2_PHYSR_SPEED_MASK) {
+	switch (val & RTL_PHYSR_SPEED_MASK) {
 	case 0x0000:
 		phydev->speed = SPEED_10;
 		break;
@@ -1136,7 +1136,7 @@ static void rtlgen_decode_physr(struct phy_device *phydev, int val)
 	 * 1: Master Mode
 	 */
 	if (phydev->speed >= 1000) {
-		if (val & RTL_VND2_PHYSR_MASTER)
+		if (val & RTL_PHYSR_MASTER)
 			phydev->master_slave_state = MASTER_SLAVE_STATE_MASTER;
 		else
 			phydev->master_slave_state = MASTER_SLAVE_STATE_SLAVE;
@@ -1156,8 +1156,7 @@ static int rtlgen_read_status(struct phy_device *phydev)
 	if (!phydev->link)
 		return 0;
 
-	val = phy_read_paged(phydev, RTL822X_VND2_TO_PAGE(RTL_VND2_PHYSR),
-			     RTL822X_VND2_TO_PAGE_REG(RTL_VND2_PHYSR));
+	val = phy_read(phydev, RTL_PHYSR);
 	if (val < 0)
 		return val;
 
@@ -1623,7 +1622,8 @@ static int rtl822x_c45_read_status(struct phy_device *phydev)
 	}
 
 	/* Read actual speed from vendor register. */
-	val = phy_read_mmd(phydev, MDIO_MMD_VEND2, RTL_VND2_PHYSR);
+	val = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+			   RTL822X_VND2_C22_REG(RTL_PHYSR));
 	if (val < 0)
 		return val;
 
@@ -2127,7 +2127,7 @@ static int rtlgen_sfp_read_status(struct phy_device *phydev)
 	if (!phydev->link)
 		return 0;
 
-	val = rtlgen_read_vend2(phydev, RTL_VND2_PHYSR);
+	val = phy_read(phydev, RTL_PHYSR);
 	if (val < 0)
 		return val;
 
-- 
2.52.0

[PATCH v2 5/5] net: phy: realtek: simplify bogus paged operations

[Daniel Golle]

Only registers 0x10~0x17 are affected by the value in the page
selection register 0x1f. Hence there is no point in using paged
operations when accessing any other registers.
Simplify the driver by using the normal phy_read and phy_write
operations for registers which are anyway not affected by paging.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2: no changes

 drivers/net/phy/realtek/realtek_main.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 8f1c8424e7f94..1b0d0602ee40d 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -68,7 +68,6 @@
 #define RTL8211E_DELAY_MASK			GENMASK(13, 11)
 
 /* RTL8211F PHY configuration */
-#define RTL8211F_PHYCR_PAGE			0xa43
 #define RTL8211F_PHYCR1				0x18
 #define RTL8211F_ALDPS_PLL_OFF			BIT(1)
 #define RTL8211F_ALDPS_ENABLE			BIT(2)
@@ -78,7 +77,6 @@
 #define RTL8211F_CLKOUT_EN			BIT(0)
 #define RTL8211F_PHYCR2_PHY_EEE_ENABLE		BIT(5)
 
-#define RTL8211F_INSR_PAGE			0xa43
 #define RTL8211F_INSR				0x1d
 
 /* RTL8211F LED configuration */
@@ -333,7 +331,7 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
 {
 	int err;
 
-	err = phy_read_paged(phydev, RTL8211F_INSR_PAGE, RTL8211F_INSR);
+	err = phy_read(phydev, RTL8211F_INSR);
 
 	return (err < 0) ? err : 0;
 }
@@ -479,7 +477,7 @@ static irqreturn_t rtl8211f_handle_interrupt(struct phy_device *phydev)
 {
 	int irq_status;
 
-	irq_status = phy_read_paged(phydev, RTL8211F_INSR_PAGE, RTL8211F_INSR);
+	irq_status = phy_read(phydev, RTL8211F_INSR);
 	if (irq_status < 0) {
 		phy_error(phydev);
 		return IRQ_NONE;
@@ -670,8 +668,8 @@ static int rtl8211f_config_clk_out(struct phy_device *phydev)
 				       RTL8211FVD_CLKOUT_REG,
 				       RTL8211FVD_CLKOUT_EN, 0);
 	else
-		ret = phy_modify_paged(phydev, RTL8211F_PHYCR_PAGE,
-				       RTL8211F_PHYCR2, RTL8211F_CLKOUT_EN, 0);
+		ret = phy_modify(phydev, RTL8211F_PHYCR2, RTL8211F_CLKOUT_EN,
+				 0);
 	if (ret)
 		return ret;
 
@@ -696,15 +694,14 @@ static int rtl8211f_config_aldps(struct phy_device *phydev)
 	if (!priv->enable_aldps)
 		return 0;
 
-	return phy_modify_paged(phydev, RTL8211F_PHYCR_PAGE, RTL8211F_PHYCR1,
-				mask, mask);
+	return phy_modify(phydev, RTL8211F_PHYCR1, mask, mask);
 }
 
 static int rtl8211f_config_phy_eee(struct phy_device *phydev)
 {
 	/* Disable PHY-mode EEE so LPI is passed to the MAC */
-	return phy_modify_paged(phydev, RTL8211F_PHYCR_PAGE, RTL8211F_PHYCR2,
-				RTL8211F_PHYCR2_PHY_EEE_ENABLE, 0);
+	return phy_modify(phydev, RTL8211F_PHYCR2,
+			  RTL8211F_PHYCR2_PHY_EEE_ENABLE, 0);
 }
 
 static int rtl8211f_config_init(struct phy_device *phydev)
@@ -770,7 +767,7 @@ static int rtl8211f_suspend(struct phy_device *phydev)
 			goto err;
 
 		/* Read the INSR to clear any pending interrupt */
-		phy_read_paged(phydev, RTL8211F_INSR_PAGE, RTL8211F_INSR);
+		phy_read(phydev, RTL8211F_INSR);
 
 		/* Reset the WoL to ensure that an event is picked up.
 		 * Unless we do this, even if we receive another packet,
-- 
2.52.0

Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

This commit adds support for translating MDIO_MMD_VEND2 registers back
to Clause-22 registers for PHYs on a C22-only bus. It also renames
RTL_PHYSR to RTL_VND2_PHYSR and changes the register access method in
several read_status functions.

> diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> index a593150b6cbb..40cae9968d20 100644
> --- a/drivers/net/phy/realtek/realtek_main.c
> +++ b/drivers/net/phy/realtek/realtek_main.c

[ ... ]

> @@ -1156,7 +1156,8 @@ static int rtlgen_read_status(struct phy_device *phydev)
>  	if (!phydev->link)
>  		return 0;
>
> -	val = phy_read(phydev, RTL_PHYSR);
> +	val = phy_read_paged(phydev, RTL822X_VND2_TO_PAGE(RTL_VND2_PHYSR),
> +			     RTL822X_VND2_TO_PAGE_REG(RTL_VND2_PHYSR));

This changes rtlgen_read_status() from reading C22 register MII_RESV2
(0x1a) directly to using paged access at page 0xa43, register 18.

rtlgen_read_status() is used by RTL8211F, RTL8211F-VD, and Generic FE-GE
Realtek PHY drivers. Are these Gigabit-only PHYs guaranteed to have the
PHYSR at the same paged location as the RTL822x 2.5GbE PHYs?

I notice this change is reverted in a later commit in the series:
  efef1a8042ec ("net: phy: realtek: demystify PHYSR register location")
which changes back to:
  val = phy_read(phydev, RTL_PHYSR);

with the note "Turns out that register address RTL_VND2_PHYSR (0xa434)
maps to Clause-22 register MII_RESV2".

Is the paged access in this intermediate commit correct, or could it
cause issues during bisection?

[ ... ]

> @@ -2127,7 +2137,7 @@ static int rtlgen_sfp_read_status(struct phy_device *phydev)
>  	if (!phydev->link)
>  		return 0;
>
> -	val = phy_read(phydev, RTL_PHYSR);
> +	val = rtlgen_read_vend2(phydev, RTL_VND2_PHYSR);

Similarly, this changes from C22 register read to C45 MDIO_MMD_VEND2
access via rtlgen_read_vend2(). The SFP PHY mode uses PHY_IS_INTERNAL
and is used by r8169 for 2.5GbE chips in SFP mode.

This is also reverted in the same later commit to:
  val = phy_read(phydev, RTL_PHYSR);

Does rtlgen_read_vend2() work correctly for all PHYs that could be in
SFP mode?
-- 
pw-bot: cr

Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

[Daniel Golle]

On Sat, Jan 17, 2026 at 03:20:06PM -0800, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2
> 
> This commit adds support for translating MDIO_MMD_VEND2 registers back
> to Clause-22 registers for PHYs on a C22-only bus. It also renames
> RTL_PHYSR to RTL_VND2_PHYSR and changes the register access method in
> several read_status functions.
> 
> > diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> > index a593150b6cbb..40cae9968d20 100644
> > --- a/drivers/net/phy/realtek/realtek_main.c
> > +++ b/drivers/net/phy/realtek/realtek_main.c
> 
> [ ... ]
> 
> > @@ -1156,7 +1156,8 @@ static int rtlgen_read_status(struct phy_device *phydev)
> >  	if (!phydev->link)
> >  		return 0;
> >
> > -	val = phy_read(phydev, RTL_PHYSR);
> > +	val = phy_read_paged(phydev, RTL822X_VND2_TO_PAGE(RTL_VND2_PHYSR),
> > +			     RTL822X_VND2_TO_PAGE_REG(RTL_VND2_PHYSR));
> 
> This changes rtlgen_read_status() from reading C22 register MII_RESV2
> (0x1a) directly to using paged access at page 0xa43, register 18.

Yeah. Just that this is not part of the series submitted.
It's rather a (halucinated) partial revert of
[v2,4/5] net: phy: realtek: demystify PHYSR register location


> 
> rtlgen_read_status() is used by RTL8211F, RTL8211F-VD, and Generic FE-GE
> Realtek PHY drivers. Are these Gigabit-only PHYs guaranteed to have the
> PHYSR at the same paged location as the RTL822x 2.5GbE PHYs?
> 
> I notice this change is reverted in a later commit in the series:
>   efef1a8042ec ("net: phy: realtek: demystify PHYSR register location")
> which changes back to:
>   val = phy_read(phydev, RTL_PHYSR);
> 
> with the note "Turns out that register address RTL_VND2_PHYSR (0xa434)
> maps to Clause-22 register MII_RESV2".
> 
> Is the paged access in this intermediate commit correct, or could it
> cause issues during bisection?
> 
> [ ... ]
> 
> > @@ -2127,7 +2137,7 @@ static int rtlgen_sfp_read_status(struct phy_device *phydev)
> >  	if (!phydev->link)
> >  		return 0;
> >
> > -	val = phy_read(phydev, RTL_PHYSR);
> > +	val = rtlgen_read_vend2(phydev, RTL_VND2_PHYSR);
> 
> Similarly, this changes from C22 register read to C45 MDIO_MMD_VEND2
> access via rtlgen_read_vend2(). The SFP PHY mode uses PHY_IS_INTERNAL
> and is used by r8169 for 2.5GbE chips in SFP mode.

Just that, again, this is the revert, and not the actual patch I'm
submitting. And yes, it has been tested on SFP NICs.

> 
> This is also reverted in the same later commit to:
>   val = phy_read(phydev, RTL_PHYSR);
> 
> Does rtlgen_read_vend2() work correctly for all PHYs that could be in
> SFP mode?
> -- 
> pw-bot: cr

Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

[Jakub Kicinski]

On Sat, 17 Jan 2026 23:40:36 +0000 Daniel Golle wrote:
> > > @@ -1156,7 +1156,8 @@ static int rtlgen_read_status(struct phy_device *phydev)
> > >  	if (!phydev->link)
> > >  		return 0;
> > >
> > > -	val = phy_read(phydev, RTL_PHYSR);
> > > +	val = phy_read_paged(phydev, RTL822X_VND2_TO_PAGE(RTL_VND2_PHYSR),
> > > +			     RTL822X_VND2_TO_PAGE_REG(RTL_VND2_PHYSR));  
> > 
> > This changes rtlgen_read_status() from reading C22 register MII_RESV2
> > (0x1a) directly to using paged access at page 0xa43, register 18.  
> 
> Yeah. Just that this is not part of the series submitted.
> It's rather a (halucinated) partial revert of
> [v2,4/5] net: phy: realtek: demystify PHYSR register location

Oh wow, that's a first. No idea how this happened. Is the chunk if
hallucinated from another WIP patch set?

Chris, FWIW this is before we added lore indexing so I don't think 
it got it from the list. Is it possible that semcode index is polluted
by previous submissions? Still, even if, it's weird that it'd
hallucinate a chunk of a patch.

Link to the review:
https://netdev-ai.bots.linux.dev/ai-review.html?id=67c40fdf-dd15-4ac1-8571-9425d9a950b4
-- 
pw-bot: under-review

Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

[Daniel Golle]

On Sat, Jan 17, 2026 at 03:55:15PM -0800, Jakub Kicinski wrote:
> On Sat, 17 Jan 2026 23:40:36 +0000 Daniel Golle wrote:
> > > > @@ -1156,7 +1156,8 @@ static int rtlgen_read_status(struct phy_device *phydev)
> > > >  	if (!phydev->link)
> > > >  		return 0;
> > > >
> > > > -	val = phy_read(phydev, RTL_PHYSR);
> > > > +	val = phy_read_paged(phydev, RTL822X_VND2_TO_PAGE(RTL_VND2_PHYSR),
> > > > +			     RTL822X_VND2_TO_PAGE_REG(RTL_VND2_PHYSR));  
> > > 
> > > This changes rtlgen_read_status() from reading C22 register MII_RESV2
> > > (0x1a) directly to using paged access at page 0xa43, register 18.  
> > 
> > Yeah. Just that this is not part of the series submitted.
> > It's rather a (halucinated) partial revert of
> > [v2,4/5] net: phy: realtek: demystify PHYSR register location
> 
> Oh wow, that's a first. No idea how this happened. Is the chunk if
> hallucinated from another WIP patch set?

No, it's a partial revert of a later patch in the same series.

> 
> Chris, FWIW this is before we added lore indexing so I don't think 
> it got it from the list. Is it possible that semcode index is polluted
> by previous submissions? Still, even if, it's weird that it'd
> hallucinate a chunk of a patch.

I don't think that as it is literally the revert of a later patch
in the same series. I don't think this has ever been submitted as
forward-patch, because it wouldn't ever apply throughout the history
of the driver. But it somehow superficially resembles

https://lore.kernel.org/netdev/a53d4577335fdda4d363db9bc4bf614fd3a56c9b.1767630451.git.daniel@makrotopia.org/

(which is pretty recent and was applied)

Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

[Chris Mason]

On 1/17/26 6:55 PM, Jakub Kicinski wrote:
> On Sat, 17 Jan 2026 23:40:36 +0000 Daniel Golle wrote:
>>>> @@ -1156,7 +1156,8 @@ static int rtlgen_read_status(struct phy_device *phydev)
>>>>  	if (!phydev->link)
>>>>  		return 0;
>>>>
>>>> -	val = phy_read(phydev, RTL_PHYSR);
>>>> +	val = phy_read_paged(phydev, RTL822X_VND2_TO_PAGE(RTL_VND2_PHYSR),
>>>> +			     RTL822X_VND2_TO_PAGE_REG(RTL_VND2_PHYSR));  
>>>
>>> This changes rtlgen_read_status() from reading C22 register MII_RESV2
>>> (0x1a) directly to using paged access at page 0xa43, register 18.  
>>
>> Yeah. Just that this is not part of the series submitted.
>> It's rather a (halucinated) partial revert of
>> [v2,4/5] net: phy: realtek: demystify PHYSR register location
> 
> Oh wow, that's a first. No idea how this happened. Is the chunk if
> hallucinated from another WIP patch set?
> 
> Chris, FWIW this is before we added lore indexing so I don't think 
> it got it from the list. Is it possible that semcode index is polluted
> by previous submissions? Still, even if, it's weird that it'd
> hallucinate a chunk of a patch.

We've definitely had it mix up hunks from other commits, but not since I
changed the prompts to make it re-read the files before writing
review-inline.txt.

But, it absolutely does ignore parts of the prompt sometimes.  I'll give
the logs a read and see if there are any clues, thanks for flagging it.

-chris

Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

[Jakub Kicinski]

On Sat, 17 Jan 2026 19:10:15 -0500 Chris Mason wrote:
> >> Yeah. Just that this is not part of the series submitted.
> >> It's rather a (halucinated) partial revert of
> >> [v2,4/5] net: phy: realtek: demystify PHYSR register location  
> > 
> > Oh wow, that's a first. No idea how this happened. Is the chunk if
> > hallucinated from another WIP patch set?
> > 
> > Chris, FWIW this is before we added lore indexing so I don't think 
> > it got it from the list. Is it possible that semcode index is polluted
> > by previous submissions? Still, even if, it's weird that it'd
> > hallucinate a chunk of a patch.  
> 
> We've definitely had it mix up hunks from other commits, but not since 
> I changed the prompts to make it re-read the files before writing
> review-inline.txt.

To be clear as Daniel mentioned the chunk in patch 4 is the other way,
so it "reverted" the direction too. At least we have a chance to use
the "mark as false positive" in the system :)

Daniel, series applied, thanks! The pw-bot is down, I think K is
repacking repos so expect a delay in the official "applied" msg.

Re: [v2,2/5] net: phy: realtek: simplify C22 reg access via MDIO_MMD_VEND2

[Chris Mason]

On 1/17/26 7:17 PM, Jakub Kicinski wrote:
> On Sat, 17 Jan 2026 19:10:15 -0500 Chris Mason wrote:
>>>> Yeah. Just that this is not part of the series submitted.
>>>> It's rather a (halucinated) partial revert of
>>>> [v2,4/5] net: phy: realtek: demystify PHYSR register location  
>>>
>>> Oh wow, that's a first. No idea how this happened. Is the chunk if
>>> hallucinated from another WIP patch set?
>>>
>>> Chris, FWIW this is before we added lore indexing so I don't think 
>>> it got it from the list. Is it possible that semcode index is polluted
>>> by previous submissions? Still, even if, it's weird that it'd
>>> hallucinate a chunk of a patch.  
>>
>> We've definitely had it mix up hunks from other commits, but not since 
>> I changed the prompts to make it re-read the files before writing
>> review-inline.txt.
> 
> To be clear as Daniel mentioned the chunk in patch 4 is the other way,
> so it "reverted" the direction too. At least we have a chance to use
> the "mark as false positive" in the system :)
> 
> Daniel, series applied, thanks! The pw-bot is down, I think K is
> repacking repos so expect a delay in the official "applied" msg.

Yeah, it looked forward in the series and noticed the later patch
reverting those hunks.  If you read the logs, it goes back and forth
trying to decide if an issue it found was worth reporting given that it
was changed in the later commit.

And then, I think it just forgot which hunks were which.  I'll try to
beef up the section double checking diff contents.

-chris