Re: [PATCH net-next v1 1/2] net: phy: realtek: add support for RTL8261
From: Nicolai Buchwitz
Date: Thu May 28 2026 - 05:49:22 EST
Hi Javen
On 28.5.2026 09:52, javen wrote:
From: Javen Xu <javen_xu@xxxxxxxxxxxxxx>
This patch adds support for Realtek phy chip RTL8261. Its PHY id is
0x001cc898 and 0x001cc899.
Signed-off-by: Javen Xu <javen_xu@xxxxxxxxxxxxxx>
---
drivers/net/phy/realtek/realtek_main.c | 315 +++++++++++++++++++++++++
1 file changed, 315 insertions(+)
diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 27268811f564..fe743fd0421b 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -22,6 +22,9 @@
#include "../phylib.h"
#include "realtek.h"
+#define RTL_8261C_CG 0x001cc898
+#define RTL_8261CE_CG 0x001cc899
Imho these should be added below RTL_8261C and not at the top.
+
#define RTL8201F_IER_PAGE 0x07
#define RTL8201F_IER 0x13
#define RTL8201F_IER_LINK BIT(13)
@@ -141,6 +144,10 @@
#define RTL8211F_PHYSICAL_ADDR_WORD1 17
#define RTL8211F_PHYSICAL_ADDR_WORD2 18
+#define RTL8261X_EXT_ADDR_REG 0xa436
+#define RTL8261X_EXT_DATA_REG 0xa438
+#define RTL_8261X_SUB_PHY_ID_ADDR 0x801d
+
#define RTL822X_VND1_SERDES_OPTION 0x697a
#define RTL822X_VND1_SERDES_OPTION_MODE_MASK GENMASK(5, 0)
#define RTL822X_VND1_SERDES_OPTION_MODE_2500BASEX_SGMII 0
@@ -252,6 +259,57 @@
#define RTL_8251B 0x001cc862
#define RTL_8261C 0x001cc890
+#define RTL8261C_CE_MODEL 0x00
+#define RTL8261D_MODEL 0x81
+#define RTL8261X_PHYSR_REG 0xa434
+#define RTL8261X_GBCR_REG 0xa412
+#define RTL8261X_IMR 0xa4d2
+#define RTL8261X_ISR 0xa4d4
+#define RTL8261X_INT_AUTONEG_ERROR BIT(0)
+#define RTL8261X_INT_PAGE_RECV BIT(2)
+#define RTL8261X_INT_AUTONEG_DONE BIT(3)
+#define RTL8261X_INT_LINK_CHG BIT(4)
+#define RTL8261X_INT_PHY_REG_ACCESS BIT(5)
+#define RTL8261X_INT_PME BIT(7)
+#define RTL8261X_INT_ALDPS_CHG BIT(9)
+#define RTL8261X_INT_JABBER BIT(10)
+#define RTL8261X_PHYSR_LINK BIT(2)
+#define RTL8261X_PHYSR_DUPLEX BIT(3)
+#define RTL8261X_PHYSR_SPEED_L GENMASK(5, 4)
+#define RTL8261X_PHYSR_SPEED_H GENMASK(10, 9)
+
+/* Concatenated 4-bit speed code values (SPD_H << 2 | SPD_L) */
+#define RTL8261X_SPEED_CODE_500M 0x3 /* H=0, L=3 */
+#define RTL8261X_SPEED_CODE_1000M 0x7 /* H=1, L=3 */
+#define RTL8261X_SPEED_CODE_2500M 0x8 /* H=2, L=0 */
+#define RTL8261X_SPEED_CODE_5000M 0x9 /* H=2, L=1 */
+#define RTL8261X_SPEED_500 500
500 MBit/s isnt a standard rate which also isnt represented in ethtool.
Is this just for documentation or intended to be used? If yes, how?
[...]
+
+static int rtl8261x_get_features(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = genphy_c45_pma_read_abilities(phydev);
+ if (ret)
+ return ret;
+ /*
+ * Supplement Multi-Gig speeds that may not be automatically detected
+ * RTL8261X supports 2.5G/5G in addition to standard 10G
+ */
+ linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
+ phydev->supported);
+ linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
+ phydev->supported);
Feature bits for 2.5 and 5 GBit/s are set unconditionally - even for the generic variant.
Do all variants have these capabilities? If not better add check for at least the generic one.
+
+ return 0;
+}
+
+static int rtl8261x_config_master_slave(struct phy_device *phydev)
+{
+ u16 val;
+ /*
+ * Configure bits 15:14 of MMD 7.0x0020
+ *
+ * Bit 15 (Enable) | Bit 14 (Value) | Mode
+ * ----------------|----------------|-------------
+ * 0 | 0 | Auto (disabled)
+ * 1 | 0 | Force Slave
+ * 1 | 1 | Force Master
+ */
+ switch (phydev->master_slave_set) {
+ case MASTER_SLAVE_CFG_MASTER_FORCE:
+ val = RTL8261X_MS_MASTER;
+ break;
+ case MASTER_SLAVE_CFG_SLAVE_FORCE:
+ val = RTL8261X_MS_SLAVE;
+ break;
+ case MASTER_SLAVE_CFG_UNKNOWN:
+ case MASTER_SLAVE_CFG_MASTER_PREFERRED:
+ case MASTER_SLAVE_CFG_SLAVE_PREFERRED:
+ default:
+ val = RTL8261X_MS_AUTO;
+ break;
+ }
+
+ return phy_modify_mmd(phydev, MDIO_MMD_AN, RTL8261X_MULTIG_CTRL,
+ RTL8261X_MASTER_SLAVE_MASK, val);
+}
RTL8261X_MULTIG_CTRL is a IEEE-defined register, so it can use MDIO_AN_10GBT_CTRL instead.
Also the define for RTL8261X_MULTIG_CTRL can be dropped.
While at it I would suggest to add MDIO_AN_10GBT_CTRL_MS_ENABLE (bit 15) and
MDIO_AN_10GBT_CTRL_MS_VALUE (bit 14) to include/uapi/linux/mdio.h and also make use of it.
RTL8261X_MS_AUTO would become
val = MDIO_AN_10GBT_CTRL_MS_ENABLE | MDIO_AN_10GBT_CTRL_MS_VALUE;
[...]
+
+static int rtl8261x_read_status(struct phy_device *phydev)
+{
+ int ret, val, speed_code;
+
+ ret = genphy_c45_read_status(phydev);
+ if (ret < 0)
+ return ret;
+
+ val = phy_read_mmd(phydev, MDIO_MMD_VEND2, RTL8261X_PHYSR_REG);
+ if (val < 0)
+ return val;
+
+ phydev->link = !!(val & RTL8261X_PHYSR_LINK);
+ if (!phydev->link) {
+ phydev->speed = SPEED_UNKNOWN;
+ phydev->duplex = DUPLEX_UNKNOWN;
+ return 0;
+ }
Why does it need to overreide the values from genphy_c45_read_status() ?
+
+ phydev->duplex = (val & RTL8261X_PHYSR_DUPLEX) ? DUPLEX_FULL : DUPLEX_HALF;
+ speed_code = (FIELD_GET(RTL8261X_PHYSR_SPEED_H, val) << 2) |
+ FIELD_GET(RTL8261X_PHYSR_SPEED_L, val);
+ switch (speed_code) {
+ case RTL8261X_SPEED_CODE_500M:
+ phydev->speed = RTL8261X_SPEED_500;
See question above (defines).
+ break;
+ case RTL8261X_SPEED_CODE_1000M:
+ phydev->speed = SPEED_1000;
+ break;
+ case RTL8261X_SPEED_CODE_2500M:
+ phydev->speed = SPEED_2500;
+ break;
+ case RTL8261X_SPEED_CODE_5000M:
+ phydev->speed = SPEED_5000;
+ break;
+ default:
+ phydev_warn(phydev, "unknown speed code 0x%x (PHYSR=0x%04x)\n", speed_code, val);
+ phydev->speed = SPEED_UNKNOWN;
+ break;
+ }
+
+ return 0;
+}
+
+static int rtl8261x_config_aneg(struct phy_device *phydev)
+{
+ u16 adv_1g = 0;
+ int ret;
+
+ if (phydev->autoneg == AUTONEG_DISABLE)
+ return genphy_c45_pma_setup_forced(phydev);
+
+ ret = rtl8261x_config_master_slave(phydev);
+ if (ret < 0)
+ return ret;
+
+ ret = genphy_c45_config_aneg(phydev);
+ if (ret < 0)
+ return ret;
+
+ if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ phydev->advertising))
+ adv_1g = BIT(9);
+
+ ret = phy_modify_mmd_changed(phydev, MDIO_MMD_VEND2,
+ RTL8261X_GBCR_REG,
+ BIT(9), adv_1g);
+ if (ret < 0)
+ return ret;
+
+ if (ret > 0)
+ return genphy_c45_restart_aneg(phydev);
This does not take rtl8261x_config_master_slave() into account. So maybe change rtl8261x_config_master_slave
return phy_modify_mmd_changed ...
and keep the result in a bool and also trigger auto nego restart if needed.
+
+ return 0;
+}
+
static int rtl821x_probe(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
@@ -3001,6 +3292,30 @@ static struct phy_driver realtek_drvs[] = {
.resume = genphy_resume,
.read_mmd = genphy_read_mmd_unsupported,
.write_mmd = genphy_write_mmd_unsupported,
+ }, {
+ PHY_ID_MATCH_EXACT(RTL_8261C_CG),
+ .name = "Realtek RTL8261C_RTL8261D 10Gbps PHY",
+ .probe = rtl8261x_probe,
RTL8261C/RTL8261D? Or just the c variant as the phy id suggests?
+ .get_features = rtl8261x_get_features,
+ .config_aneg = rtl8261x_config_aneg,
+ .read_status = rtl8261x_read_status,
+ .config_intr = rtl8261x_config_intr,
+ .handle_interrupt = rtl8261x_handle_interrupt,
+ .soft_reset = rtl8261x_soft_reset,
+ .suspend = genphy_c45_pma_suspend,
+ .resume = genphy_c45_pma_resume,
+ }, {
+ PHY_ID_MATCH_EXACT(RTL_8261CE_CG),
+ .name = "Realtek RTL8261CE 10Gbps PHY",
+ .probe = rtl8261x_probe,
+ .get_features = rtl8261x_get_features,
+ .config_aneg = rtl8261x_config_aneg,
+ .read_status = rtl8261x_read_status,
+ .config_intr = rtl8261x_config_intr,
+ .handle_interrupt = rtl8261x_handle_interrupt,
+ .soft_reset = rtl8261x_soft_reset,
+ .suspend = genphy_c45_pma_suspend,
+ .resume = genphy_c45_pma_resume,
},
};
Please also make sure for both patches that checkpatch has no further complaints. Thanks.
Nicolai