[PATCH net-next v1 0/5] r8169: fix RTL8116AF low power and link readiness issues

[PATCH net-next v1 0/5] r8169: fix RTL8116AF low power and link readiness issues

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

This series fixes several RTL8116AF issues in r8169.

RTL8116af operate in a SFP mode where the standard PHY register path does not
provide all required information for normal phylib. In this mode, the driver
needs a dummy PHY ID so that phylib can attach to a dummy Realtek PHY driver,
while selected standard PHY registers are handled through the SerDes register.
And RTL8116af is a multi-function card, all the function should be set to d3hot
and enable aspm to ensure that system can enter C10 and s0ix power saving state,
including invisble function 2 to 7.

Javen Xu (5):
  net: phy: realtek: add support for dummy phy
  r8169: move funcitons forward
  r8169: fix RTL8116af readiness bug
  r8169: add ltr support for RTL8116af
  r8169: fix RTL8116af can not enter s0idle and c10

 drivers/net/ethernet/realtek/r8169_main.c | 217 ++++++++++++++++------
 drivers/net/phy/realtek/realtek_main.c    |  54 ++++++
 include/net/phy/realtek_phy.h             |   1 +
 3 files changed, 214 insertions(+), 58 deletions(-)

-- 
2.43.0

[PATCH net-next v1 1/5] net: phy: realtek: add support for dummy phy

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

Add support for rtl8116af dummy phy driver, match phy id and read link
speed from MII_BMCR.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/phy/realtek/realtek_main.c | 54 ++++++++++++++++++++++++++
 include/net/phy/realtek_phy.h          |  1 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 27268811f564..9b92828c49d9 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -2659,6 +2659,47 @@ static int rtlgen_sfp_get_features(struct phy_device *phydev)
 	return 0;
 }
 
+static int rtl8116af_sfp_get_features(struct phy_device *phydev)
+{
+	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+			 phydev->supported);
+
+	phydev->speed = SPEED_1000;
+	phydev->duplex = DUPLEX_FULL;
+
+	phydev->port = PORT_FIBRE;
+
+	return 0;
+}
+
+static int rtl8116af_sfp_read_status(struct phy_device *phydev)
+{
+	int val, err;
+
+	err = genphy_update_link(phydev);
+	if (err)
+		return err;
+
+	if (!phydev->link)
+		return 0;
+
+	val = phy_read(phydev, MII_BMCR);
+	if (val < 0)
+		return val;
+
+	if (val & BMCR_SPEED1000)
+		phydev->speed = SPEED_1000;
+	else if (val & BMCR_SPEED100)
+		phydev->speed = SPEED_100;
+
+	if (val & BMCR_FULLDPLX)
+		phydev->duplex = DUPLEX_FULL;
+	else
+		phydev->duplex = DUPLEX_HALF;
+
+	return 0;
+}
+
 static int rtlgen_sfp_read_status(struct phy_device *phydev)
 {
 	int val, err;
@@ -2947,6 +2988,19 @@ static struct phy_driver realtek_drvs[] = {
 		.write_page	= rtl821x_write_page,
 		.read_mmd	= rtl822x_read_mmd,
 		.write_mmd	= rtl822x_write_mmd,
+	}, {
+		PHY_ID_MATCH_EXACT(PHY_ID_RTL8116AF_DUMMY),
+		.name		= "RTL8116af PHY Mode",
+		.flags		= PHY_IS_INTERNAL,
+		.get_features	= rtl8116af_sfp_get_features,
+		.config_aneg	= rtlgen_sfp_config_aneg,
+		.read_status	= rtl8116af_sfp_read_status,
+		.suspend	= genphy_suspend,
+		.resume		= rtlgen_resume,
+		.read_page	= rtl821x_read_page,
+		.write_page	= rtl821x_write_page,
+		.read_mmd	= rtl822x_read_mmd,
+		.write_mmd	= rtl822x_write_mmd,
 	}, {
 		PHY_ID_MATCH_EXACT(0x001ccad0),
 		.name		= "RTL8224 2.5Gbps PHY",
diff --git a/include/net/phy/realtek_phy.h b/include/net/phy/realtek_phy.h
index d683bc1b0659..cbf91af0ead6 100644
--- a/include/net/phy/realtek_phy.h
+++ b/include/net/phy/realtek_phy.h
@@ -3,5 +3,6 @@
 #define _REALTEK_PHY_H
 
 #define	PHY_ID_RTL_DUMMY_SFP	0x001ccbff
+#define PHY_ID_RTL8116AF_DUMMY  0x001ccbfe
 
 #endif /* _REALTEK_PHY_H */
-- 
2.43.0

[PATCH net-next v1 2/5] r8169: move funcitons forward

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

Move some functions forward to avoid adding a forward declaration.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/ethernet/realtek/r8169_main.c | 84 +++++++++++------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index ec4fc21fa21f..1e2e4074d343 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -1113,6 +1113,23 @@ DECLARE_RTL_COND(rtl_ocp_gphy_cond)
 	return RTL_R32(tp, GPHY_OCP) & OCPAR_FLAG;
 }
 
+/* Work around a hw issue with RTL8168g PHY, the quirk disables
+ * PHY MCU interrupts before PHY power-down.
+ */
+static void rtl8168g_phy_suspend_quirk(struct rtl8169_private *tp, int value)
+{
+	switch (tp->mac_version) {
+	case RTL_GIGA_MAC_VER_40:
+		if (value & BMCR_RESET || !(value & BMCR_PDOWN))
+			rtl_eri_set_bits(tp, 0x1a8, 0xfc000000);
+		else
+			rtl_eri_clear_bits(tp, 0x1a8, 0xfc000000);
+		break;
+	default:
+		break;
+	}
+};
+
 static void r8168_phy_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 {
 	if (rtl_ocp_reg_failure(reg))
@@ -1123,19 +1140,20 @@ static void r8168_phy_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 	rtl_loop_wait_low(tp, &rtl_ocp_gphy_cond, 25, 10);
 }
 
-static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
+static void r8168g_mdio_write(struct rtl8169_private *tp, int reg, int value)
 {
-	if (rtl_ocp_reg_failure(reg))
-		return 0;
+	if (reg == 0x1f) {
+		tp->ocp_base = value ? value << 4 : OCP_STD_PHY_BASE;
+		return;
+	}
 
-	/* Return dummy MII_PHYSID2 in SFP mode to match SFP PHY driver */
-	if (tp->sfp_mode && reg == (OCP_STD_PHY_BASE + 2 * MII_PHYSID2))
-		return PHY_ID_RTL_DUMMY_SFP & 0xffff;
+	if (tp->ocp_base != OCP_STD_PHY_BASE)
+		reg -= 0x10;
 
-	RTL_W32(tp, GPHY_OCP, reg << 15);
+	if (tp->ocp_base == OCP_STD_PHY_BASE && reg == MII_BMCR)
+		rtl8168g_phy_suspend_quirk(tp, value);
 
-	return rtl_loop_wait_high(tp, &rtl_ocp_gphy_cond, 25, 10) ?
-		(RTL_R32(tp, GPHY_OCP) & 0xffff) : -ETIMEDOUT;
+	r8168_phy_ocp_write(tp, tp->ocp_base + reg * 2, value);
 }
 
 static void __r8168_mac_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
@@ -1177,6 +1195,21 @@ static u16 r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
 	return val;
 }
 
+static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
+{
+	if (rtl_ocp_reg_failure(reg))
+		return 0;
+
+	/* Return dummy MII_PHYSID2 in SFP mode to match SFP PHY driver */
+	if (tp->sfp_mode && reg == (OCP_STD_PHY_BASE + 2 * MII_PHYSID2))
+		return PHY_ID_RTL_DUMMY_SFP & 0xffff;
+
+	RTL_W32(tp, GPHY_OCP, reg << 15);
+
+	return rtl_loop_wait_high(tp, &rtl_ocp_gphy_cond, 25, 10) ?
+		(RTL_R32(tp, GPHY_OCP) & 0xffff) : -ETIMEDOUT;
+}
+
 static void r8168_mac_ocp_modify(struct rtl8169_private *tp, u32 reg, u16 mask,
 				 u16 set)
 {
@@ -1229,39 +1262,6 @@ static void rtl_sfp_reset(struct rtl8169_private *tp)
 		r8127_sfp_sds_phy_reset(tp);
 }
 
-/* Work around a hw issue with RTL8168g PHY, the quirk disables
- * PHY MCU interrupts before PHY power-down.
- */
-static void rtl8168g_phy_suspend_quirk(struct rtl8169_private *tp, int value)
-{
-	switch (tp->mac_version) {
-	case RTL_GIGA_MAC_VER_40:
-		if (value & BMCR_RESET || !(value & BMCR_PDOWN))
-			rtl_eri_set_bits(tp, 0x1a8, 0xfc000000);
-		else
-			rtl_eri_clear_bits(tp, 0x1a8, 0xfc000000);
-		break;
-	default:
-		break;
-	}
-};
-
-static void r8168g_mdio_write(struct rtl8169_private *tp, int reg, int value)
-{
-	if (reg == 0x1f) {
-		tp->ocp_base = value ? value << 4 : OCP_STD_PHY_BASE;
-		return;
-	}
-
-	if (tp->ocp_base != OCP_STD_PHY_BASE)
-		reg -= 0x10;
-
-	if (tp->ocp_base == OCP_STD_PHY_BASE && reg == MII_BMCR)
-		rtl8168g_phy_suspend_quirk(tp, value);
-
-	r8168_phy_ocp_write(tp, tp->ocp_base + reg * 2, value);
-}
-
 static int r8168g_mdio_read(struct rtl8169_private *tp, int reg)
 {
 	if (reg == 0x1f)
-- 
2.43.0

[PATCH net-next v1 3/5] r8169: fix RTL8116af link readiness bug

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

For saving power, RTL8116af can not read link status from standard phy
register. Instead, we should read link status through mac register.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/ethernet/realtek/r8169_main.c | 73 +++++++++++++++++++----
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 1e2e4074d343..56653608633a 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -454,6 +454,12 @@ enum rtl8125_registers {
 
 #define RX_FETCH_DFLT_8125	(8 << 27)
 
+#define OCP_SDS_ADDR_REG	0xEB10
+#define OCP_SDS_CMD_REG		0xEB0E
+#define OCP_SDS_DATA_REG	0xEB14
+#define SDS_CMD_READ		0x0001
+#define RTL_SDS_C22_BASE	0x40
+
 enum rtl_register_content {
 	/* InterruptStatusBits */
 	SYSErr		= 0x8000,
@@ -728,6 +734,12 @@ enum rtl_dash_type {
 	RTL_DASH_25_BP,
 };
 
+enum rtl_sfp_mode {
+	RTL_SFP_NONE,
+	RTL_SFP_8116_AF,
+	RTL_SFP_8127_ATF,
+};
+
 struct rtl8169_private {
 	void __iomem *mmio_addr;	/* memory map physical address */
 	struct pci_dev *pci_dev;
@@ -736,6 +748,7 @@ struct rtl8169_private {
 	struct napi_struct napi;
 	enum mac_version mac_version;
 	enum rtl_dash_type dash_type;
+	enum rtl_sfp_mode sfp_mode;
 	u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
 	u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
 	u32 dirty_tx;
@@ -762,7 +775,6 @@ struct rtl8169_private {
 	unsigned supports_gmii:1;
 	unsigned aspm_manageable:1;
 	unsigned dash_enabled:1;
-	bool sfp_mode:1;
 	dma_addr_t counters_phys_addr;
 	struct rtl8169_counters *counters;
 	struct rtl8169_tc_offsets tc_offset;
@@ -1195,13 +1207,42 @@ static u16 r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
 	return val;
 }
 
+static u16 rtl8116af_sds_read(struct rtl8169_private *tp, u16 sds_reg)
+{
+	r8168_mac_ocp_write(tp, OCP_SDS_ADDR_REG, sds_reg);
+	r8168_mac_ocp_write(tp, OCP_SDS_CMD_REG, SDS_CMD_READ);
+	return r8168_mac_ocp_read(tp, OCP_SDS_DATA_REG);
+}
+
+static bool rtl_is_8116af(struct rtl8169_private *tp)
+{
+	return tp->mac_version == RTL_GIGA_MAC_VER_52 &&
+		(r8168_mac_ocp_read(tp, 0xdc00) & 0x0078) == 0x0030 &&
+		(r8168_mac_ocp_read(tp, 0xd006) & 0x00ff) == 0x0000;
+}
+
 static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
 {
 	if (rtl_ocp_reg_failure(reg))
 		return 0;
 
+	if (tp->sfp_mode == RTL_SFP_8116_AF) {
+		switch (reg) {
+		case OCP_STD_PHY_BASE + 2 * MII_PHYSID1:
+			return upper_16_bits(PHY_ID_RTL8116AF_DUMMY);
+		case OCP_STD_PHY_BASE + 2 * MII_PHYSID2:
+			return lower_16_bits(PHY_ID_RTL8116AF_DUMMY);
+		case OCP_STD_PHY_BASE + 2 * MII_BMSR:
+			return rtl8116af_sds_read(tp, RTL_SDS_C22_BASE + MII_BMSR);
+		case OCP_STD_PHY_BASE + 2 * MII_BMCR:
+			return rtl8116af_sds_read(tp, RTL_SDS_C22_BASE + MII_BMCR);
+		default:
+			break;
+		}
+	}
+
 	/* Return dummy MII_PHYSID2 in SFP mode to match SFP PHY driver */
-	if (tp->sfp_mode && reg == (OCP_STD_PHY_BASE + 2 * MII_PHYSID2))
+	if (tp->sfp_mode == RTL_SFP_8127_ATF && reg == (OCP_STD_PHY_BASE + 2 * MII_PHYSID2))
 		return PHY_ID_RTL_DUMMY_SFP & 0xffff;
 
 	RTL_W32(tp, GPHY_OCP, reg << 15);
@@ -1578,6 +1619,20 @@ static bool rtl_dash_is_enabled(struct rtl8169_private *tp)
 	}
 }
 
+static enum rtl_sfp_mode rtl_get_sfp_mode(struct rtl8169_private *tp)
+{
+	if (rtl_is_8125(tp)) {
+		u16 data = r8168_mac_ocp_read(tp, 0xd006);
+
+		if ((data & 0xff) == 0x07)
+			return RTL_SFP_8127_ATF;
+	} else if (rtl_is_8116af(tp)) {
+		return RTL_SFP_8116_AF;
+	}
+
+	return RTL_SFP_NONE;
+}
+
 static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private *tp)
 {
 	switch (tp->mac_version) {
@@ -2394,7 +2449,7 @@ static int rtl8169_set_link_ksettings(struct net_device *ndev,
 	int duplex = cmd->base.duplex;
 	int speed = cmd->base.speed;
 
-	if (!tp->sfp_mode)
+	if (tp->sfp_mode != RTL_SFP_8127_ATF)
 		return phy_ethtool_ksettings_set(phydev, cmd);
 
 	if (cmd->base.autoneg != AUTONEG_DISABLE)
@@ -2552,7 +2607,7 @@ static void rtl8169_init_phy(struct rtl8169_private *tp)
 	    tp->pci_dev->subsystem_device == 0xe000)
 		phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b);
 
-	if (tp->sfp_mode)
+	if (tp->sfp_mode == RTL_SFP_8127_ATF)
 		rtl_sfp_init(tp);
 
 	/* We may have called phy_speed_down before */
@@ -5010,7 +5065,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
 	phy_stop(tp->phydev);
 
 	/* Reset SerDes PHY to bring down fiber link */
-	if (tp->sfp_mode)
+	if (tp->sfp_mode == RTL_SFP_8127_ATF)
 		rtl_sfp_reset(tp);
 
 	rtl8169_update_counters(tp);
@@ -5679,13 +5734,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 	tp->aspm_manageable = !rc;
 
-	if (rtl_is_8125(tp)) {
-		u16 data = r8168_mac_ocp_read(tp, 0xd006);
-
-		if ((data & 0xff) == 0x07)
-			tp->sfp_mode = true;
-	}
-
+	tp->sfp_mode = rtl_get_sfp_mode(tp);
 	tp->dash_type = rtl_get_dash_type(tp);
 	tp->dash_enabled = rtl_dash_is_enabled(tp);
 
-- 
2.43.0

[PATCH net-next v1 4/5] r8169: add ltr support for RTL8116af

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

This patch adds ltr support for RTL8116af, enables RTL8116af enter l1.2
state. This makes sense for the system to enter c10 state.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/ethernet/realtek/r8169_main.c | 31 +++++++++++++++++++----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 56653608633a..5db15137f216 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -334,11 +334,13 @@ enum rtl_registers {
 	ALDPS_LTR	= 0xe0a2,
 	LTR_OBFF_LOCK	= 0xe032,
 	LTR_SNOOP	= 0xe034,
+	SEND_LTR_MSG	= 0xe038,
 
 #define ALDPS_LTR_EN			BIT(0)
 #define LTR_OBFF_LOCK_EN		BIT(0)
 #define LINK_SPEED_CHANGE_EN		BIT(14)
 #define LTR_SNOOP_EN			GENMASK(15, 14)
+#define LTR_MSG_EN			BIT(0)
 };
 
 enum rtl8168_8101_registers {
@@ -3153,8 +3155,22 @@ static void rtl_enable_ltr(struct rtl8169_private *tp)
 		r8168_mac_ocp_write(tp, 0xcdf2, 0x9003);
 		r8168_mac_ocp_modify(tp, LTR_OBFF_LOCK, 0x0000, LINK_SPEED_CHANGE_EN);
 		break;
-	case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
 	case RTL_GIGA_MAC_VER_52:
+		r8168_mac_ocp_write(tp, 0xcdd0, 0x9003);
+		r8168_mac_ocp_modify(tp, LTR_SNOOP, 0x0000, LTR_SNOOP_EN);
+		r8168_mac_ocp_write(tp, 0xe02c, 0x1880);
+		r8168_mac_ocp_write(tp, 0xe02e, 0x4880);
+		r8168_mac_ocp_modify(tp, ALDPS_LTR, 0x0000, ALDPS_LTR_EN);
+		r8168_mac_ocp_write(tp, 0xcdd8, 0x9003);
+		r8168_mac_ocp_write(tp, 0xcdda, 0x9003);
+		r8168_mac_ocp_write(tp, 0xcddc, 0x9003);
+		r8168_mac_ocp_write(tp, 0xcdd2, 0x883c);
+		r8168_mac_ocp_write(tp, 0xcdd4, 0x8c12);
+		r8168_mac_ocp_write(tp, 0xcdd6, 0x9003);
+		r8168_mac_ocp_write(tp, 0xe0a6, 0x9003);
+		r8168_mac_ocp_write(tp, 0xe0a8, 0x9003);
+		break;
+	case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
 		r8168_mac_ocp_modify(tp, ALDPS_LTR, 0x0000, ALDPS_LTR_EN);
 		RTL_W8(tp, COMBO_LTR_EXTEND, RTL_R8(tp, COMBO_LTR_EXTEND) | COMBO_LTR_EXTEND_EN);
 		fallthrough;
@@ -3174,6 +3190,7 @@ static void rtl_enable_ltr(struct rtl8169_private *tp)
 	}
 	/* chip can trigger LTR */
 	r8168_mac_ocp_modify(tp, LTR_OBFF_LOCK, 0x0003, LTR_OBFF_LOCK_EN);
+	r8168_mac_ocp_modify(tp, SEND_LTR_MSG, 0x0000, LTR_MSG_EN);
 }
 
 static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
@@ -3207,6 +3224,7 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
 		rtl_enable_ltr(tp);
 		switch (tp->mac_version) {
 		case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
+		case RTL_GIGA_MAC_VER_52:
 		case RTL_GIGA_MAC_VER_61 ... RTL_GIGA_MAC_VER_LAST:
 			/* reset ephy tx/rx disable timer */
 			r8168_mac_ocp_modify(tp, 0xe094, 0xff00, 0);
@@ -3219,6 +3237,7 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
 	} else {
 		switch (tp->mac_version) {
 		case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
+		case RTL_GIGA_MAC_VER_52:
 		case RTL_GIGA_MAC_VER_61 ... RTL_GIGA_MAC_VER_LAST:
 			r8168_mac_ocp_modify(tp, 0xe092, 0x00ff, 0);
 			break;
@@ -3732,7 +3751,9 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp)
 
 	rtl_eri_set_bits(tp, 0xd4, 0x0010);
 
-	rtl_eri_write(tp, 0x5f0, ERIAR_MASK_0011, 0x4f87);
+	rtl_eri_write(tp, 0x5f0, ERIAR_MASK_0011, 0x4000);
+
+	r8168_mac_ocp_write(tp, 0xe098, 0xc302);
 
 	rtl_disable_rxdvgate(tp);
 
@@ -3757,9 +3778,9 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp)
 	}
 
 	r8168_mac_ocp_modify(tp, 0xe056, 0x00f0, 0x0000);
-	r8168_mac_ocp_write(tp, 0xea80, 0x0003);
-	r8168_mac_ocp_modify(tp, 0xe052, 0x0000, 0x0009);
-	r8168_mac_ocp_modify(tp, 0xd420, 0x0fff, 0x047f);
+	r8168_mac_ocp_write(tp, 0xea80, 0x0000);
+	r8168_mac_ocp_modify(tp, 0xe052, 0x0009, 0x0000);
+	r8168_mac_ocp_modify(tp, 0xd420, 0x0fff, 0x045f);
 
 	r8168_mac_ocp_write(tp, 0xe63e, 0x0001);
 	r8168_mac_ocp_write(tp, 0xe63e, 0x0000);
-- 
2.43.0

[PATCH net-next v1 5/5] r8169: fix RTL8116af can not enter s0idle and c10

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

RTL8116AF is a multi-function device. Functions 2 to 7 are hidden from
the PCI core and return an all-ones response when their vendor ID is read,
so they are not enumerated as normal PCI functions.

However, these hidden functions can still affect platform power
management. If they are left in D0 or keep ASPM disabled, the platform may
fail to enter the low-power s0ix state and the CPU package may fail to
enter Package C10.

Put functions 2 to 7 into D3hot and enable ASPM on their PCIe link control
register. Since these functions are hidden, access their configuration
space through pci_bus_read_config_dword() / pci_bus_write_config_dword()
using the same slot and the target function numbers.

Ignore functions that return a PCI error response when reading their
configuration space.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/ethernet/realtek/r8169_main.c | 31 +++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 5db15137f216..cf73915c48bc 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -341,6 +341,9 @@ enum rtl_registers {
 #define LINK_SPEED_CHANGE_EN		BIT(14)
 #define LTR_SNOOP_EN			GENMASK(15, 14)
 #define LTR_MSG_EN			BIT(0)
+#define RTL8116AF_FUNC_PM_CSR		0x80
+#define RTL8116AF_FUNC_EXP_LNKCTL	0x44
+#define RTL_PM_D3HOT			GENMASK(1, 0)
 };
 
 enum rtl8168_8101_registers {
@@ -3731,6 +3734,33 @@ static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
 	r8168_mac_ocp_modify(tp, 0xe860, 0x0000, 0x0080);
 }
 
+static void rtl_disable_hidden_function(struct pci_dev *pdev)
+{
+	unsigned int slot = PCI_SLOT(pdev->devfn);
+	struct pci_bus *bus = pdev->bus;
+	unsigned int devfn;
+	int func;
+	int ret;
+	u32 val;
+
+	for (func = 2; func < 8; func++) {
+		devfn = PCI_DEVFN(slot, func);
+
+		ret = pci_bus_read_config_dword(bus, devfn, RTL8116AF_FUNC_PM_CSR, &val);
+		if (!ret && !PCI_POSSIBLE_ERROR(val)) {
+			val &= ~(PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_PME_ENABLE);
+			val |= (RTL_PM_D3HOT | PCI_PM_CTRL_PME_ENABLE);
+			pci_bus_write_config_dword(bus, devfn, RTL8116AF_FUNC_PM_CSR, val);
+		}
+
+		ret = pci_bus_read_config_dword(bus, devfn, RTL8116AF_FUNC_EXP_LNKCTL, &val);
+		if (!ret && !PCI_POSSIBLE_ERROR(val)) {
+			val |= PCI_EXP_LNKCTL_ASPMC;
+			pci_bus_write_config_dword(bus, devfn, RTL8116AF_FUNC_EXP_LNKCTL, val);
+		}
+	}
+}
+
 static void rtl_hw_start_8117(struct rtl8169_private *tp)
 {
 	static const struct ephy_info e_info_8117[] = {
@@ -3787,6 +3817,7 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp)
 	r8168_mac_ocp_write(tp, 0xc094, 0x0000);
 	r8168_mac_ocp_write(tp, 0xc09e, 0x0000);
 
+	rtl_disable_hidden_function(tp->pci_dev);
 	/* firmware is for MAC only */
 	r8169_apply_firmware(tp);
 }
-- 
2.43.0

Re: [PATCH net-next v1 1/5] net: phy: realtek: add support for dummy phy

[Maxime Chevallier]

On 6/3/26 08:59, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
> 
> Add support for rtl8116af dummy phy driver, match phy id and read link
> speed from MII_BMCR.
> 
> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>

Can you elaborate more on why this is needed ?

The cover says :
"
In this mode, the driver
needs a dummy PHY ID so that phylib can attach to a dummy Realtek PHY driver,
while selected standard PHY registers are handled through the SerDes register.
"

Why can't you use the SerDes registers for your PHY driver ? The phrase above
suggests that the "SerDes registers" have the typical C22/45 layout. There are PHYs
and PCSs out there that aren't accessed through regular MDIO with regmap being
used to translate the mdio accesses done by phylib into the actual register
access method used.

Maxime

> ---
>  drivers/net/phy/realtek/realtek_main.c | 54 ++++++++++++++++++++++++++
>  include/net/phy/realtek_phy.h          |  1 +
>  2 files changed, 55 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
> index 27268811f564..9b92828c49d9 100644
> --- a/drivers/net/phy/realtek/realtek_main.c
> +++ b/drivers/net/phy/realtek/realtek_main.c
> @@ -2659,6 +2659,47 @@ static int rtlgen_sfp_get_features(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static int rtl8116af_sfp_get_features(struct phy_device *phydev)
> +{
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
> +			 phydev->supported);
> +
> +	phydev->speed = SPEED_1000;
> +	phydev->duplex = DUPLEX_FULL;
> +
> +	phydev->port = PORT_FIBRE;
> +
> +	return 0;
> +}
> +
> +static int rtl8116af_sfp_read_status(struct phy_device *phydev)
> +{
> +	int val, err;
> +
> +	err = genphy_update_link(phydev);
> +	if (err)
> +		return err;
> +
> +	if (!phydev->link)
> +		return 0;
> +
> +	val = phy_read(phydev, MII_BMCR);
> +	if (val < 0)
> +		return val;
> +
> +	if (val & BMCR_SPEED1000)
> +		phydev->speed = SPEED_1000;
> +	else if (val & BMCR_SPEED100)
> +		phydev->speed = SPEED_100;
> +
> +	if (val & BMCR_FULLDPLX)
> +		phydev->duplex = DUPLEX_FULL;
> +	else
> +		phydev->duplex = DUPLEX_HALF;
> +
> +	return 0;
> +}
> +
>  static int rtlgen_sfp_read_status(struct phy_device *phydev)
>  {
>  	int val, err;
> @@ -2947,6 +2988,19 @@ static struct phy_driver realtek_drvs[] = {
>  		.write_page	= rtl821x_write_page,
>  		.read_mmd	= rtl822x_read_mmd,
>  		.write_mmd	= rtl822x_write_mmd,
> +	}, {
> +		PHY_ID_MATCH_EXACT(PHY_ID_RTL8116AF_DUMMY),
> +		.name		= "RTL8116af PHY Mode",
> +		.flags		= PHY_IS_INTERNAL,
> +		.get_features	= rtl8116af_sfp_get_features,
> +		.config_aneg	= rtlgen_sfp_config_aneg,
> +		.read_status	= rtl8116af_sfp_read_status,
> +		.suspend	= genphy_suspend,
> +		.resume		= rtlgen_resume,
> +		.read_page	= rtl821x_read_page,
> +		.write_page	= rtl821x_write_page,
> +		.read_mmd	= rtl822x_read_mmd,
> +		.write_mmd	= rtl822x_write_mmd,
>  	}, {
>  		PHY_ID_MATCH_EXACT(0x001ccad0),
>  		.name		= "RTL8224 2.5Gbps PHY",
> diff --git a/include/net/phy/realtek_phy.h b/include/net/phy/realtek_phy.h
> index d683bc1b0659..cbf91af0ead6 100644
> --- a/include/net/phy/realtek_phy.h
> +++ b/include/net/phy/realtek_phy.h
> @@ -3,5 +3,6 @@
>  #define _REALTEK_PHY_H
>  
>  #define	PHY_ID_RTL_DUMMY_SFP	0x001ccbff
> +#define PHY_ID_RTL8116AF_DUMMY  0x001ccbfe
>  
>  #endif /* _REALTEK_PHY_H */

RE: [PATCH net-next v1 1/5] net: phy: realtek: add support for dummy phy

[Javen]

>On 6/3/26 08:59, javen wrote:
>> From: Javen Xu <javen_xu@realsil.com.cn>
>>
>> Add support for rtl8116af dummy phy driver, match phy id and read link
>> speed from MII_BMCR.
>>
>> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
>
>Can you elaborate more on why this is needed ?
>
>The cover says :
>"
>In this mode, the driver
>needs a dummy PHY ID so that phylib can attach to a dummy Realtek PHY
>driver, while selected standard PHY registers are handled through the SerDes
>register.
>"
>
>Why can't you use the SerDes registers for your PHY driver ? The phrase above
>suggests that the "SerDes registers" have the typical C22/45 layout. There are
>PHYs and PCSs out there that aren't accessed through regular MDIO with
>regmap being used to translate the mdio accesses done by phylib into the
>actual register access method used.
>
>Maxime
>

Thanks for your review.
Maybe I shouldn't call it a "dummy" PHY. It is actually a real, dedicated PHY driver for the RTL8116af operating in Fiber/SerDes mode. The reason we cannot use the standard generic PHY driver is that hardware does not correctly populate the standard Gigabit Status Register (MII_STAT1000, Reg 0x0a). 
Here is a snippet of our MDIO read trace during link up:
r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x0a, value=0x0000
r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x05, value=0x41a0
Reg 0x0a returns 0x0000, the phylib generic status parser fails to resolve the 1000Mbps link, causing the driver to incorrectly fall back to a forced 10Mbps, even though the physical link is actually UP at 1000Mbps. Therefore, we need to register a custom PHY driver to provide a specific .read_status callback that parses MII_BMCR instead of relying on MII_STAT1000.

BRs,
Javen

>> ---
>>  drivers/net/phy/realtek/realtek_main.c | 54 ++++++++++++++++++++++++++
>>  include/net/phy/realtek_phy.h          |  1 +
>>  2 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/net/phy/realtek/realtek_main.c
>> b/drivers/net/phy/realtek/realtek_main.c
>> index 27268811f564..9b92828c49d9 100644
>> --- a/drivers/net/phy/realtek/realtek_main.c
>> +++ b/drivers/net/phy/realtek/realtek_main.c
>> @@ -2659,6 +2659,47 @@ static int rtlgen_sfp_get_features(struct
>phy_device *phydev)
>>       return 0;
>>  }
>>
>> +static int rtl8116af_sfp_get_features(struct phy_device *phydev) {
>> +     linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
>> +                      phydev->supported);
>> +
>> +     phydev->speed = SPEED_1000;
>> +     phydev->duplex = DUPLEX_FULL;
>> +
>> +     phydev->port = PORT_FIBRE;
>> +
>> +     return 0;
>> +}
>> +
>> +static int rtl8116af_sfp_read_status(struct phy_device *phydev) {
>> +     int val, err;
>> +
>> +     err = genphy_update_link(phydev);
>> +     if (err)
>> +             return err;
>> +
>> +     if (!phydev->link)
>> +             return 0;
>> +
>> +     val = phy_read(phydev, MII_BMCR);
>> +     if (val < 0)
>> +             return val;
>> +
>> +     if (val & BMCR_SPEED1000)
>> +             phydev->speed = SPEED_1000;
>> +     else if (val & BMCR_SPEED100)
>> +             phydev->speed = SPEED_100;
>> +
>> +     if (val & BMCR_FULLDPLX)
>> +             phydev->duplex = DUPLEX_FULL;
>> +     else
>> +             phydev->duplex = DUPLEX_HALF;
>> +
>> +     return 0;
>> +}
>> +
>>  static int rtlgen_sfp_read_status(struct phy_device *phydev)  {
>>       int val, err;
>> @@ -2947,6 +2988,19 @@ static struct phy_driver realtek_drvs[] = {
>>               .write_page     = rtl821x_write_page,
>>               .read_mmd       = rtl822x_read_mmd,
>>               .write_mmd      = rtl822x_write_mmd,
>> +     }, {
>> +             PHY_ID_MATCH_EXACT(PHY_ID_RTL8116AF_DUMMY),
>> +             .name           = "RTL8116af PHY Mode",
>> +             .flags          = PHY_IS_INTERNAL,
>> +             .get_features   = rtl8116af_sfp_get_features,
>> +             .config_aneg    = rtlgen_sfp_config_aneg,
>> +             .read_status    = rtl8116af_sfp_read_status,
>> +             .suspend        = genphy_suspend,
>> +             .resume         = rtlgen_resume,
>> +             .read_page      = rtl821x_read_page,
>> +             .write_page     = rtl821x_write_page,
>> +             .read_mmd       = rtl822x_read_mmd,
>> +             .write_mmd      = rtl822x_write_mmd,
>>       }, {
>>               PHY_ID_MATCH_EXACT(0x001ccad0),
>>               .name           = "RTL8224 2.5Gbps PHY",
>> diff --git a/include/net/phy/realtek_phy.h
>> b/include/net/phy/realtek_phy.h index d683bc1b0659..cbf91af0ead6
>> 100644
>> --- a/include/net/phy/realtek_phy.h
>> +++ b/include/net/phy/realtek_phy.h
>> @@ -3,5 +3,6 @@
>>  #define _REALTEK_PHY_H
>>
>>  #define      PHY_ID_RTL_DUMMY_SFP    0x001ccbff
>> +#define PHY_ID_RTL8116AF_DUMMY  0x001ccbfe
>>
>>  #endif /* _REALTEK_PHY_H */

Re: [PATCH net-next v1 1/5] net: phy: realtek: add support for dummy phy

[Maxime Chevallier]

Hi,

On 6/3/26 10:00, Javen wrote:
>> On 6/3/26 08:59, javen wrote:
>>> From: Javen Xu <javen_xu@realsil.com.cn>
>>>
>>> Add support for rtl8116af dummy phy driver, match phy id and read link
>>> speed from MII_BMCR.
>>>
>>> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
>>
>> Can you elaborate more on why this is needed ?
>>
>> The cover says :
>> "
>> In this mode, the driver
>> needs a dummy PHY ID so that phylib can attach to a dummy Realtek PHY
>> driver, while selected standard PHY registers are handled through the SerDes
>> register.
>> "
>>
>> Why can't you use the SerDes registers for your PHY driver ? The phrase above
>> suggests that the "SerDes registers" have the typical C22/45 layout. There are
>> PHYs and PCSs out there that aren't accessed through regular MDIO with
>> regmap being used to translate the mdio accesses done by phylib into the
>> actual register access method used.
>>
>> Maxime
>>
> 
> Thanks for your review.
> Maybe I shouldn't call it a "dummy" PHY. It is actually a real, dedicated PHY driver for the RTL8116af operating in Fiber/SerDes mode. The reason we cannot use the standard generic PHY driver is that hardware does not correctly populate the standard Gigabit Status Register (MII_STAT1000, Reg 0x0a). 
> Here is a snippet of our MDIO read trace during link up:
> r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x0a, value=0x0000
> r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x05, value=0x41a0
> Reg 0x0a returns 0x0000, the phylib generic status parser fails to resolve the 1000Mbps link, causing the driver to incorrectly fall back to a forced 10Mbps, even though the physical link is actually UP at 1000Mbps. Therefore, we need to register a custom PHY driver to provide a specific .read_status callback that parses MII_BMCR instead of relying on MII_STAT1000.

This behaviour of not getting proper information through the standard
register happens on multiple different PHYs, so having a dedicated driver
is the way to go indeed.

What does this PHY do exactly, does it connect to an SFP cage as the name
seems to imply ? If so, what kind of mode can it support ? 1000BaseX/100BaseX,
maybe SGMII as well ?

What I don't quite understand then is why you have the need to return a
custom PHY ID that matches that "dummy" driver, in the next patch.

If this is a regular PHY with C22 registers, what's the original content of
PHYSID1/2 ? M

Can you explain what the hardware looks like exactly ? What are the different
components involved, etc ?

Maxime

RE: [PATCH net-next v1 1/5] net: phy: realtek: add support for dummy phy

[Javen]

>Hi,
>
>On 6/3/26 10:00, Javen wrote:
>>> On 6/3/26 08:59, javen wrote:
>>>> From: Javen Xu <javen_xu@realsil.com.cn>
>>>>
>>>> Add support for rtl8116af dummy phy driver, match phy id and read
>>>> link speed from MII_BMCR.
>>>>
>>>> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
>>>
>>> Can you elaborate more on why this is needed ?
>>>
>>> The cover says :
>>> "
>>> In this mode, the driver
>>> needs a dummy PHY ID so that phylib can attach to a dummy Realtek PHY
>>> driver, while selected standard PHY registers are handled through the
>>> SerDes register.
>>> "
>>>
>>> Why can't you use the SerDes registers for your PHY driver ? The
>>> phrase above suggests that the "SerDes registers" have the typical
>>> C22/45 layout. There are PHYs and PCSs out there that aren't accessed
>>> through regular MDIO with regmap being used to translate the mdio
>>> accesses done by phylib into the actual register access method used.
>>>
>>> Maxime
>>>
>>
>> Thanks for your review.
>> Maybe I shouldn't call it a "dummy" PHY. It is actually a real, dedicated PHY
>driver for the RTL8116af operating in Fiber/SerDes mode. The reason we
>cannot use the standard generic PHY driver is that hardware does not
>correctly populate the standard Gigabit Status Register (MII_STAT1000, Reg
>0x0a).
>> Here is a snippet of our MDIO read trace during link up:
>> r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x0a,
>> value=0x0000
>> r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x05,
>> value=0x41a0 Reg 0x0a returns 0x0000, the phylib generic status parser fails
>to resolve the 1000Mbps link, causing the driver to incorrectly fall back to a
>forced 10Mbps, even though the physical link is actually UP at 1000Mbps.
>Therefore, we need to register a custom PHY driver to provide a
>specific .read_status callback that parses MII_BMCR instead of relying on
>MII_STAT1000.
>
>This behaviour of not getting proper information through the standard
>register happens on multiple different PHYs, so having a dedicated driver is
>the way to go indeed.
>
>What does this PHY do exactly, does it connect to an SFP cage as the name
>seems to imply ? If so, what kind of mode can it support ?
>1000BaseX/100BaseX, maybe SGMII as well ?
>
>What I don't quite understand then is why you have the need to return a
>custom PHY ID that matches that "dummy" driver, in the next patch.
>
>If this is a regular PHY with C22 registers, what's the original content of
>PHYSID1/2 ? M
>
>Can you explain what the hardware looks like exactly ? What are the different
>components involved, etc ?
>
>Maxime

It supports 1000BaseX mode.
The datapath in Fiber mode is strictly and simply: MAC -> PCS -> SerDes. MAC accesses the PCS registers via MAC OCP channel. Although it is accessed via MAC OCP, the register definitions at those addresses are exactly the same as the IEEE 802.3 Clause 22 PHY standard registers.
Reading Reg 0x02 and 0x03 via this MAC OCP interface simply returns 0x00000000.
[ 1671.299837] r8169 0000:85:00.1: 8116af read: reg=0x02, value=0x0000
[ 1671.299906] r8169 0000:85:00.1: 8116af read: reg=0x03, value=0x0000
So we need a dummy phy id to attach phy driver.

BRs,
Javen

Re: [PATCH net-next v1 1/5] net: phy: realtek: add support for dummy phy

[Maxime Chevallier]

Hi,

On 6/3/26 11:13, Javen wrote:
>> Hi,
>>
>> On 6/3/26 10:00, Javen wrote:
>>>> On 6/3/26 08:59, javen wrote:
>>>>> From: Javen Xu <javen_xu@realsil.com.cn>
>>>>>
>>>>> Add support for rtl8116af dummy phy driver, match phy id and read
>>>>> link speed from MII_BMCR.
>>>>>
>>>>> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
>>>>
>>>> Can you elaborate more on why this is needed ?
>>>>
>>>> The cover says :
>>>> "
>>>> In this mode, the driver
>>>> needs a dummy PHY ID so that phylib can attach to a dummy Realtek PHY
>>>> driver, while selected standard PHY registers are handled through the
>>>> SerDes register.
>>>> "
>>>>
>>>> Why can't you use the SerDes registers for your PHY driver ? The
>>>> phrase above suggests that the "SerDes registers" have the typical
>>>> C22/45 layout. There are PHYs and PCSs out there that aren't accessed
>>>> through regular MDIO with regmap being used to translate the mdio
>>>> accesses done by phylib into the actual register access method used.
>>>>
>>>> Maxime
>>>>
>>>
>>> Thanks for your review.
>>> Maybe I shouldn't call it a "dummy" PHY. It is actually a real, dedicated PHY
>> driver for the RTL8116af operating in Fiber/SerDes mode. The reason we
>> cannot use the standard generic PHY driver is that hardware does not
>> correctly populate the standard Gigabit Status Register (MII_STAT1000, Reg
>> 0x0a).
>>> Here is a snippet of our MDIO read trace during link up:
>>> r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x0a,
>>> value=0x0000
>>> r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x05,
>>> value=0x41a0 Reg 0x0a returns 0x0000, the phylib generic status parser fails
>> to resolve the 1000Mbps link, causing the driver to incorrectly fall back to a
>> forced 10Mbps, even though the physical link is actually UP at 1000Mbps.
>> Therefore, we need to register a custom PHY driver to provide a
>> specific .read_status callback that parses MII_BMCR instead of relying on
>> MII_STAT1000.
>>
>> This behaviour of not getting proper information through the standard
>> register happens on multiple different PHYs, so having a dedicated driver is
>> the way to go indeed.
>>
>> What does this PHY do exactly, does it connect to an SFP cage as the name
>> seems to imply ? If so, what kind of mode can it support ?
>> 1000BaseX/100BaseX, maybe SGMII as well ?
>>
>> What I don't quite understand then is why you have the need to return a
>> custom PHY ID that matches that "dummy" driver, in the next patch.
>>
>> If this is a regular PHY with C22 registers, what's the original content of
>> PHYSID1/2 ? M
>>
>> Can you explain what the hardware looks like exactly ? What are the different
>> components involved, etc ?
>>
>> Maxime
> 
> It supports 1000BaseX mode.

Ok, but the driver for the "dummy" phy seems to be able to report SPEED_100, which
you can't achieve with 1000BaseX. Either this is a stray check in the PHY driver,
or there's more to it than 1000BaseX :)

Also, if this is 1000BaseX maybe you need to use the C37 helpers to read the link
status :

https://elixir.bootlin.com/linux/v7.1-rc5/source/drivers/net/phy/phy_device.c#L2607

> The datapath in Fiber mode is strictly and simply: MAC -> PCS -> SerDes. MAC accesses the PCS registers via MAC OCP channel. Although it is accessed via MAC OCP, the register definitions at those addresses are exactly the same as the IEEE 802.3 Clause 22 PHY standard registers.
> Reading Reg 0x02 and 0x03 via this MAC OCP interface simply returns 0x00000000.
> [ 1671.299837] r8169 0000:85:00.1: 8116af read: reg=0x02, value=0x0000
> [ 1671.299906] r8169 0000:85:00.1: 8116af read: reg=0x03, value=0x0000
> So we need a dummy phy id to attach phy driver.

So this PHY actually doesn't exist ? what you have is a PCS that you access
over MDIO, right ?

It seems it all boils down to how the realtek MAC driver deals with PHYs and
PCSs and SFP, that is without using phylink or the SFP infrastructure at all :(

Can you check for the C37 thing and the SPEED_100 reporting ?

Maxime



> 
> BRs,
> Javen
> 

RE: [PATCH net-next v1 1/5] net: phy: realtek: add support for dummy phy

[Javen]

>Hi,
>
>On 6/3/26 11:13, Javen wrote:
>>> Hi,
>>>
>>> On 6/3/26 10:00, Javen wrote:
>>>>> On 6/3/26 08:59, javen wrote:
>>>>>> From: Javen Xu <javen_xu@realsil.com.cn>
>>>>>>
>>>>>> Add support for rtl8116af dummy phy driver, match phy id and read
>>>>>> link speed from MII_BMCR.
>>>>>>
>>>>>> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
>>>>>
>>>>> Can you elaborate more on why this is needed ?
>>>>>
>>>>> The cover says :
>>>>> "
>>>>> In this mode, the driver
>>>>> needs a dummy PHY ID so that phylib can attach to a dummy Realtek
>>>>> PHY driver, while selected standard PHY registers are handled
>>>>> through the SerDes register.
>>>>> "
>>>>>
>>>>> Why can't you use the SerDes registers for your PHY driver ? The
>>>>> phrase above suggests that the "SerDes registers" have the typical
>>>>> C22/45 layout. There are PHYs and PCSs out there that aren't
>>>>> accessed through regular MDIO with regmap being used to translate
>>>>> the mdio accesses done by phylib into the actual register access method
>used.
>>>>>
>>>>> Maxime
>>>>>
>>>>
>>>> Thanks for your review.
>>>> Maybe I shouldn't call it a "dummy" PHY. It is actually a real,
>>>> dedicated PHY
>>> driver for the RTL8116af operating in Fiber/SerDes mode. The reason
>>> we cannot use the standard generic PHY driver is that hardware does
>>> not correctly populate the standard Gigabit Status Register
>>> (MII_STAT1000, Reg 0x0a).
>>>> Here is a snippet of our MDIO read trace during link up:
>>>> r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x0a,
>>>> value=0x0000
>>>> r8169 0000:85:00.1: MDIO read: tp->ocp_base=0xa400, reg=0x05,
>>>> value=0x41a0 Reg 0x0a returns 0x0000, the phylib generic status
>>>> parser fails
>>> to resolve the 1000Mbps link, causing the driver to incorrectly fall
>>> back to a forced 10Mbps, even though the physical link is actually UP at
>1000Mbps.
>>> Therefore, we need to register a custom PHY driver to provide a
>>> specific .read_status callback that parses MII_BMCR instead of
>>> relying on MII_STAT1000.
>>>
>>> This behaviour of not getting proper information through the standard
>>> register happens on multiple different PHYs, so having a dedicated
>>> driver is the way to go indeed.
>>>
>>> What does this PHY do exactly, does it connect to an SFP cage as the
>>> name seems to imply ? If so, what kind of mode can it support ?
>>> 1000BaseX/100BaseX, maybe SGMII as well ?
>>>
>>> What I don't quite understand then is why you have the need to return
>>> a custom PHY ID that matches that "dummy" driver, in the next patch.
>>>
>>> If this is a regular PHY with C22 registers, what's the original
>>> content of
>>> PHYSID1/2 ? M
>>>
>>> Can you explain what the hardware looks like exactly ? What are the
>>> different components involved, etc ?
>>>
>>> Maxime
>>
>> It supports 1000BaseX mode.
>
>Ok, but the driver for the "dummy" phy seems to be able to report SPEED_100,
>which you can't achieve with 1000BaseX. Either this is a stray check in the PHY
>driver, or there's more to it than 1000BaseX :)
>
>Also, if this is 1000BaseX maybe you need to use the C37 helpers to read the
>link status :
>
>https://elixir.bootlin.com/linux/v7.1-
>rc5/source/drivers/net/phy/phy_device.c#L2607
>
>> The datapath in Fiber mode is strictly and simply: MAC -> PCS -> SerDes.
>MAC accesses the PCS registers via MAC OCP channel. Although it is accessed
>via MAC OCP, the register definitions at those addresses are exactly the same
>as the IEEE 802.3 Clause 22 PHY standard registers.
>> Reading Reg 0x02 and 0x03 via this MAC OCP interface simply returns
>0x00000000.
>> [ 1671.299837] r8169 0000:85:00.1: 8116af read: reg=0x02, value=0x0000
>> [ 1671.299906] r8169 0000:85:00.1: 8116af read: reg=0x03, value=0x0000
>> So we need a dummy phy id to attach phy driver.
>
>So this PHY actually doesn't exist ? what you have is a PCS that you access over
>MDIO, right ?
>
>It seems it all boils down to how the realtek MAC driver deals with PHYs and
>PCSs and SFP, that is without using phylink or the SFP infrastructure at all :(
>
>Can you check for the C37 thing and the SPEED_100 reporting ?
>
>Maxime
>
You are exactly right. The PHY doesn't actually exist; what we have is an internal PCS.  I will try using the C37 helpers (genphy_c37_read_status()) to read the link status and test how it behaves with our hardware. I will get back to you with the results soon. And check SPEED_100 too.
I actually tried to migrate it to phylink before. However, the current driver codebase is deeply coupled with tp->phydev in many places. Decoupling it may introduce bugs for numerous existing devices. Therefore, I want to try to expose the internal PCS as a dummy PHY device.
Anyway, thanks for your review.

BRs,
Javen

Re: [PATCH net-next v1 1/5] net: phy: realtek: add support for dummy phy

[Andrew Lunn]

> You are exactly right. The PHY doesn't actually exist; what we have
> is an internal PCS.

...

> I actually tried to migrate it to
> phylink before. However, the current driver codebase is deeply
> coupled with tp->phydev in many places. Decoupling it may introduce
> bugs for numerous existing devices. Therefore, I want to try to
> expose the internal PCS as a dummy PHY device.

Architecturally, you are going down a dead end. You really should be
using phylink.

I don't see anything too scary with tp->phydev in the driver. Are
there parts you think are going to be a problem with phylink?

      Andrew