[PATCH net-next v1 0/6] add support for phylink

[PATCH net-next v1 0/6] add support for phylink

[javen]

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

This series patch adds support for phylink. RTL8116af is a fiber mode
card, link status and speed can not be read from standard phy reg. So
we read link status and speed from serdes reg by pcs. We have not changed
the loading process of RTL8127atf.

Javen Xu (6):
  r8169: add current_speed in private struct
  r8169: add support for phylink
  r8169: decoupling tp->phydev
  r8169: add support for RTL8116af
  r8169: add ltr support for RTL8116af
  r8169: fix RTL8116af can not enter s0idle and c10

 drivers/net/ethernet/realtek/Kconfig      |   1 +
 drivers/net/ethernet/realtek/r8169_main.c | 447 +++++++++++++++++-----
 2 files changed, 346 insertions(+), 102 deletions(-)

-- 
2.43.0

[PATCH net-next v1 1/6] r8169: add current_speed in private struct

[javen]

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

This patch adds current_speed in private strcut in order to decouple
from phydev in the following patch supporting for phylink.

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

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index ec4fc21fa21f..fdc8c84dd112 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -750,6 +750,7 @@ struct rtl8169_private {
 	u32 irq_mask;
 	int irq;
 	struct clk *clk;
+	int current_speed;
 
 	struct {
 		DECLARE_BITMAP(flags, RTL_FLAG_MAX);
@@ -1673,16 +1674,14 @@ static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp)
 	rtl_pci_commit(tp);
 }
 
-static void rtl_link_chg_patch(struct rtl8169_private *tp)
+static void rtl_link_chg_patch(struct rtl8169_private *tp, int speed)
 {
-	struct phy_device *phydev = tp->phydev;
-
 	if (tp->mac_version == RTL_GIGA_MAC_VER_34 ||
 	    tp->mac_version == RTL_GIGA_MAC_VER_38) {
-		if (phydev->speed == SPEED_1000) {
+		if (speed == SPEED_1000) {
 			rtl_eri_write(tp, 0x1bc, ERIAR_MASK_1111, 0x00000011);
 			rtl_eri_write(tp, 0x1dc, ERIAR_MASK_1111, 0x00000005);
-		} else if (phydev->speed == SPEED_100) {
+		} else if (speed == SPEED_100) {
 			rtl_eri_write(tp, 0x1bc, ERIAR_MASK_1111, 0x0000001f);
 			rtl_eri_write(tp, 0x1dc, ERIAR_MASK_1111, 0x00000005);
 		} else {
@@ -1692,7 +1691,7 @@ static void rtl_link_chg_patch(struct rtl8169_private *tp)
 		rtl_reset_packet_filter(tp);
 	} else if (tp->mac_version == RTL_GIGA_MAC_VER_35 ||
 		   tp->mac_version == RTL_GIGA_MAC_VER_36) {
-		if (phydev->speed == SPEED_1000) {
+		if (speed == SPEED_1000) {
 			rtl_eri_write(tp, 0x1bc, ERIAR_MASK_1111, 0x00000011);
 			rtl_eri_write(tp, 0x1dc, ERIAR_MASK_1111, 0x00000005);
 		} else {
@@ -1700,7 +1699,7 @@ static void rtl_link_chg_patch(struct rtl8169_private *tp)
 			rtl_eri_write(tp, 0x1dc, ERIAR_MASK_1111, 0x0000003f);
 		}
 	} else if (tp->mac_version == RTL_GIGA_MAC_VER_37) {
-		if (phydev->speed == SPEED_10) {
+		if (speed == SPEED_10) {
 			rtl_eri_write(tp, 0x1d0, ERIAR_MASK_0011, 0x4d02);
 			rtl_eri_write(tp, 0x1dc, ERIAR_MASK_0011, 0x0060a);
 		} else {
@@ -2074,11 +2073,11 @@ rtl_coalesce_info(struct rtl8169_private *tp)
 		ci = rtl_coalesce_info_8168_8136;
 
 	/* if speed is unknown assume highest one */
-	if (tp->phydev->speed == SPEED_UNKNOWN)
+	if (tp->current_speed == SPEED_UNKNOWN)
 		return ci;
 
 	for (; ci->speed; ci++) {
-		if (tp->phydev->speed == ci->speed)
+		if (tp->current_speed == ci->speed)
 			return ci;
 	}
 
@@ -2236,7 +2235,7 @@ static void rtl_set_eee_txidle_timer(struct rtl8169_private *tp)
 
 static unsigned int r8169_get_tx_lpi_timer_us(struct rtl8169_private *tp)
 {
-	unsigned int speed = tp->phydev->speed;
+	unsigned int speed = tp->current_speed;
 	unsigned int timer = tp->tx_lpi_timer;
 
 	if (!timer || speed == SPEED_UNKNOWN)
@@ -4968,8 +4967,9 @@ static void r8169_phylink_handler(struct net_device *ndev)
 	struct rtl8169_private *tp = netdev_priv(ndev);
 	struct device *d = tp_to_dev(tp);
 
+	tp->current_speed = tp->phydev->speed;
 	if (netif_carrier_ok(ndev)) {
-		rtl_link_chg_patch(tp);
+		rtl_link_chg_patch(tp, tp->current_speed);
 		rtl_enable_tx_lpi(tp, tp->phydev->enable_tx_lpi);
 		pm_request_resume(d);
 	} else {
@@ -5667,6 +5667,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 				     ext_xid_str, xid);
 	tp->mac_version = chip->mac_version;
 	tp->fw_name = chip->fw_name;
+	tp->current_speed = SPEED_UNKNOWN;
 
 	/* Disable ASPM L1 as that cause random device stop working
 	 * problems as well as full system hangs for some PCIe devices users.
-- 
2.43.0

[PATCH net-next v1 2/6] r8169: add support for phylink

[javen]

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

Transfer old framework to phylink. Phylink can support fiber mode card
which can not get link status or link speed from standard phy registers.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
 drivers/net/ethernet/realtek/Kconfig      |   1 +
 drivers/net/ethernet/realtek/r8169_main.c | 170 ++++++++++++++++------
 2 files changed, 123 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/realtek/Kconfig b/drivers/net/ethernet/realtek/Kconfig
index 9b0f4f9631db..49ac72734225 100644
--- a/drivers/net/ethernet/realtek/Kconfig
+++ b/drivers/net/ethernet/realtek/Kconfig
@@ -88,6 +88,7 @@ config R8169
 	select CRC32
 	select PHYLIB
 	select REALTEK_PHY
+	select PHYLINK
 	help
 	  Say Y here if you have a Realtek Ethernet adapter belonging to
 	  the following families:
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index fdc8c84dd112..97bcd36efdbb 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -26,6 +26,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/pm_runtime.h>
 #include <linux/bitfield.h>
+#include <linux/phylink.h>
 #include <linux/prefetch.h>
 #include <linux/ipv6.h>
 #include <linux/unaligned.h>
@@ -775,6 +776,8 @@ struct rtl8169_private {
 	struct r8169_led_classdev *leds;
 
 	u32 ocp_base;
+	struct phylink *phylink;
+	struct phylink_config phylink_config;
 };
 
 typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
@@ -2554,9 +2557,6 @@ static void rtl8169_init_phy(struct rtl8169_private *tp)
 	if (tp->sfp_mode)
 		rtl_sfp_init(tp);
 
-	/* We may have called phy_speed_down before */
-	phy_speed_up(tp->phydev);
-
 	genphy_soft_reset(tp->phydev);
 }
 
@@ -2658,13 +2658,10 @@ static void rtl_jumbo_config(struct rtl8169_private *tp)
 		pcie_set_readrq(tp->pci_dev, readrq);
 
 	/* Chip doesn't support pause in jumbo mode */
-	if (jumbo) {
-		linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT,
-				   tp->phydev->advertising);
-		linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
-				   tp->phydev->advertising);
-		phy_start_aneg(tp->phydev);
-	}
+	if (jumbo)
+		tp->phylink_config.mac_capabilities &= ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
+	else
+		tp->phylink_config.mac_capabilities |= (MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
 }
 
 DECLARE_RTL_COND(rtl_chipcmd_cond)
@@ -2779,7 +2776,7 @@ static void rtl_prepare_power_down(struct rtl8169_private *tp)
 		rtl_ephy_write(tp, 0x19, 0xff64);
 
 	if (device_may_wakeup(tp_to_dev(tp))) {
-		phy_speed_down(tp->phydev, false);
+		phylink_speed_down(tp->phylink, false);
 		rtl_wol_enable_rx(tp);
 	}
 }
@@ -4139,11 +4136,17 @@ static int rtl8169_change_mtu(struct net_device *dev, int new_mtu)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
 
+	if (netif_running(dev))
+		phylink_stop(tp->phylink);
+
 	WRITE_ONCE(dev->mtu, new_mtu);
 	netdev_update_features(dev);
 	rtl_jumbo_config(tp);
 	rtl_set_eee_txidle_timer(tp);
 
+	if (netif_running(dev))
+		phylink_start(tp->phylink);
+
 	return 0;
 }
 
@@ -4962,41 +4965,15 @@ static void rtl_enable_tx_lpi(struct rtl8169_private *tp, bool enable)
 	}
 }
 
-static void r8169_phylink_handler(struct net_device *ndev)
-{
-	struct rtl8169_private *tp = netdev_priv(ndev);
-	struct device *d = tp_to_dev(tp);
-
-	tp->current_speed = tp->phydev->speed;
-	if (netif_carrier_ok(ndev)) {
-		rtl_link_chg_patch(tp, tp->current_speed);
-		rtl_enable_tx_lpi(tp, tp->phydev->enable_tx_lpi);
-		pm_request_resume(d);
-	} else {
-		pm_runtime_idle(d);
-	}
-
-	phy_print_status(tp->phydev);
-}
-
 static int r8169_phy_connect(struct rtl8169_private *tp)
 {
-	struct phy_device *phydev = tp->phydev;
-	phy_interface_t phy_mode;
 	int ret;
 
-	phy_mode = tp->supports_gmii ? PHY_INTERFACE_MODE_GMII :
-		   PHY_INTERFACE_MODE_MII;
-
-	ret = phy_connect_direct(tp->dev, phydev, r8169_phylink_handler,
-				 phy_mode);
-	if (ret)
+	ret = phylink_connect_phy(tp->phylink, tp->phydev);
+	if (ret) {
+		netdev_err(tp->dev, "failed to connect phy\n");
 		return ret;
-
-	if (!tp->supports_gmii)
-		phy_set_max_speed(phydev, SPEED_100);
-
-	phy_attached_info(phydev);
+	}
 
 	return 0;
 }
@@ -5007,7 +4984,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
 	/* Clear all task flags */
 	bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
 
-	phy_stop(tp->phydev);
+	phylink_stop(tp->phylink);
 
 	/* Reset SerDes PHY to bring down fiber link */
 	if (tp->sfp_mode)
@@ -5035,11 +5012,14 @@ static void rtl8169_up(struct rtl8169_private *tp)
 	phy_init_hw(tp->phydev);
 	phy_resume(tp->phydev);
 	rtl8169_init_phy(tp);
+
+	/* We may have called phy_speed_down before */
+	phylink_speed_up(tp->phylink);
 	napi_enable(&tp->napi);
 	enable_work(&tp->wk.work);
 	rtl_reset_work(tp);
 
-	phy_start(tp->phydev);
+	phylink_start(tp->phylink);
 }
 
 static int rtl8169_close(struct net_device *dev)
@@ -5055,7 +5035,7 @@ static int rtl8169_close(struct net_device *dev)
 
 	free_irq(tp->irq, tp);
 
-	phy_disconnect(tp->phydev);
+	phylink_disconnect_phy(tp->phylink);
 
 	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
 			  tp->RxPhyAddr);
@@ -5288,6 +5268,8 @@ static void rtl_remove_one(struct pci_dev *pdev)
 		r8169_remove_leds(tp->leds);
 
 	unregister_netdev(tp->dev);
+	if (tp->phylink)
+		phylink_destroy(tp->phylink);
 
 	if (tp->dash_type != RTL_DASH_NONE)
 		rtl8168_driver_stop(tp);
@@ -5474,10 +5456,8 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
 		return -EUNATCH;
 	}
 
-	tp->phydev->mac_managed_pm = true;
 	if (rtl_supports_eee(tp))
 		phy_support_eee(tp->phydev);
-	phy_support_asym_pause(tp->phydev);
 
 	/* mimic behavior of r8125/r8126 vendor drivers */
 	if (tp->mac_version == RTL_GIGA_MAC_VER_61)
@@ -5599,6 +5579,92 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
 	return false;
 }
 
+static void rtl_mac_link_down(struct phylink_config *config, unsigned int mode,
+			      phy_interface_t interface)
+{
+	struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
+
+	tp->current_speed = SPEED_UNKNOWN;
+	pm_runtime_idle(tp_to_dev(tp));
+}
+
+static void rtl_mac_link_up(struct phylink_config *config, struct phy_device *phydev,
+			    unsigned int mode, phy_interface_t interface,
+			    int speed, int duplex, bool tx_pause, bool rx_pause)
+{
+	struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
+
+	struct device *d = tp_to_dev(tp);
+
+	tp->current_speed = speed;
+	rtl_link_chg_patch(tp, speed);
+
+	if (phydev)
+		rtl_enable_tx_lpi(tp, phydev->enable_tx_lpi);
+
+	pm_request_resume(d);
+}
+
+static struct phylink_pcs *rtl_mac_select_pcs(struct phylink_config *config,
+					      phy_interface_t interface)
+{
+	return NULL;
+}
+
+static void rtl_mac_config(struct phylink_config *config, unsigned int mode,
+			   const struct phylink_link_state *state)
+{
+}
+
+static const struct phylink_mac_ops rtl_phylink_mac_ops = {
+	.mac_select_pcs = rtl_mac_select_pcs,
+	.mac_config = rtl_mac_config,
+	.mac_link_down  = rtl_mac_link_down,
+	.mac_link_up    = rtl_mac_link_up,
+};
+
+static int rtl_init_phylink(struct rtl8169_private *tp)
+{
+	struct phylink *pl;
+	phy_interface_t phy_mode;
+
+	tp->phylink_config.dev = &tp->dev->dev;
+	tp->phylink_config.type = PHYLINK_NETDEV;
+	tp->phylink_config.mac_managed_pm = true;
+
+	tp->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
+
+	if (tp->sfp_mode) {
+		phy_mode = PHY_INTERFACE_MODE_INTERNAL;
+		tp->phylink_config.mac_capabilities |= MAC_10000FD;
+	} else {
+		tp->phylink_config.mac_capabilities |= MAC_10 | MAC_100;
+		phy_mode = PHY_INTERFACE_MODE_INTERNAL;
+
+		if (tp->mac_version == RTL_GIGA_MAC_VER_80)
+			tp->phylink_config.mac_capabilities |= MAC_1000FD | MAC_2500FD |
+							       MAC_5000FD | MAC_10000FD;
+		else if (tp->mac_version == RTL_GIGA_MAC_VER_70)
+			tp->phylink_config.mac_capabilities |= MAC_1000FD |
+							       MAC_2500FD | MAC_5000FD;
+		else if (tp->mac_version >= RTL_GIGA_MAC_VER_61)
+			tp->phylink_config.mac_capabilities |= MAC_1000FD | MAC_2500FD;
+		else
+			if (tp->supports_gmii)
+				tp->phylink_config.mac_capabilities |= MAC_1000FD;
+	}
+
+	__set_bit(phy_mode, tp->phylink_config.supported_interfaces);
+	pl = phylink_create(&tp->phylink_config, tp_to_dev(tp)->fwnode,
+			    phy_mode, &rtl_phylink_mac_ops);
+	if (IS_ERR(pl))
+		return PTR_ERR(pl);
+
+	tp->phylink = pl;
+
+	return 0;
+}
+
 static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	const struct rtl_chip_info *chip;
@@ -5789,13 +5855,21 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	pci_set_drvdata(pdev, tp);
 
-	rc = r8169_mdio_register(tp);
+	rc = rtl_init_phylink(tp);
 	if (rc)
 		return rc;
 
+	rc = r8169_mdio_register(tp);
+	if (rc) {
+		phylink_destroy(tp->phylink);
+		return rc;
+	}
+
 	rc = register_netdev(dev);
-	if (rc)
+	if (rc) {
+		phylink_destroy(tp->phylink);
 		return rc;
+	}
 
 	if (IS_ENABLED(CONFIG_R8169_LEDS)) {
 		if (rtl_is_8125(tp))
-- 
2.43.0

[PATCH net-next v1 3/6] r8169: decoupling tp->phydev

[javen]

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

Some functions in ethtool are bound to tp->phydev. But in phylink, we
should replace it with functions which are bound to to->phylink.

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

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 97bcd36efdbb..c46117e3a643 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -2256,7 +2256,7 @@ static int rtl8169_get_eee(struct net_device *dev, struct ethtool_keee *data)
 	if (!rtl_supports_eee(tp))
 		return -EOPNOTSUPP;
 
-	ret = phy_ethtool_get_eee(tp->phydev, data);
+	ret = phylink_ethtool_get_eee(tp->phylink, data);
 	if (ret)
 		return ret;
 
@@ -2272,7 +2272,7 @@ static int rtl8169_set_eee(struct net_device *dev, struct ethtool_keee *data)
 	if (!rtl_supports_eee(tp))
 		return -EOPNOTSUPP;
 
-	return phy_ethtool_set_eee(tp->phydev, data);
+	return phylink_ethtool_set_eee(tp->phylink, data);
 }
 
 static void rtl8169_get_ringparam(struct net_device *dev,
@@ -2303,13 +2303,8 @@ static void rtl8169_get_pauseparam(struct net_device *dev,
 				   struct ethtool_pauseparam *data)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
-	bool tx_pause, rx_pause;
 
-	phy_get_pause(tp->phydev, &tx_pause, &rx_pause);
-
-	data->autoneg = tp->phydev->autoneg;
-	data->tx_pause = tx_pause ? 1 : 0;
-	data->rx_pause = rx_pause ? 1 : 0;
+	phylink_ethtool_get_pauseparam(tp->phylink, data);
 }
 
 static int rtl8169_set_pauseparam(struct net_device *dev,
@@ -2320,9 +2315,7 @@ static int rtl8169_set_pauseparam(struct net_device *dev,
 	if (dev->mtu > ETH_DATA_LEN)
 		return -EOPNOTSUPP;
 
-	phy_set_asym_pause(tp->phydev, data->rx_pause, data->tx_pause);
-
-	return 0;
+	return phylink_ethtool_set_pauseparam(tp->phylink, data);
 }
 
 static void rtl8169_get_eth_mac_stats(struct net_device *dev,
@@ -2388,6 +2381,14 @@ static void rtl8169_get_eth_ctrl_stats(struct net_device *dev,
 		le32_to_cpu(tp->counters->rx_unknown_opcode);
 }
 
+static int rtl8169_get_link_ksettings(struct net_device *ndev,
+				      struct ethtool_link_ksettings *cmd)
+{
+	struct rtl8169_private *tp = netdev_priv(ndev);
+
+	return phylink_ethtool_ksettings_get(tp->phylink, cmd);
+}
+
 static int rtl8169_set_link_ksettings(struct net_device *ndev,
 				      const struct ethtool_link_ksettings *cmd)
 {
@@ -2397,7 +2398,7 @@ static int rtl8169_set_link_ksettings(struct net_device *ndev,
 	int speed = cmd->base.speed;
 
 	if (!tp->sfp_mode)
-		return phy_ethtool_ksettings_set(phydev, cmd);
+		return phylink_ethtool_ksettings_set(tp->phylink, cmd);
 
 	if (cmd->base.autoneg != AUTONEG_DISABLE)
 		return -EINVAL;
@@ -2418,6 +2419,13 @@ static int rtl8169_set_link_ksettings(struct net_device *ndev,
 	return 0;
 }
 
+static int rtl8169_nway_reset(struct net_device *dev)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	return phylink_ethtool_nway_reset(tp->phylink);
+}
+
 static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES,
@@ -2433,10 +2441,10 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.get_sset_count		= rtl8169_get_sset_count,
 	.get_ethtool_stats	= rtl8169_get_ethtool_stats,
 	.get_ts_info		= ethtool_op_get_ts_info,
-	.nway_reset		= phy_ethtool_nway_reset,
+	.nway_reset		= rtl8169_nway_reset,
 	.get_eee		= rtl8169_get_eee,
 	.set_eee		= rtl8169_set_eee,
-	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
+	.get_link_ksettings	= rtl8169_get_link_ksettings,
 	.set_link_ksettings	= rtl8169_set_link_ksettings,
 	.get_ringparam		= rtl8169_get_ringparam,
 	.get_pause_stats	= rtl8169_get_pause_stats,
-- 
2.43.0

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

[javen]

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

RTL8116af is sfp mode. Phylink uses pcs to get the link status and speed
from its serdes reg, instead of standard phy reg. Also, RTL8116af doesn't
have internal phy, so we add some checks to ensure that tp->phydev is
not empty when we need it.

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

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index c46117e3a643..7b79305635fc 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -97,6 +97,12 @@
 #define JUMBO_9K	(9 * SZ_1K - VLAN_ETH_HLEN - ETH_FCS_LEN)
 #define JUMBO_16K	(SZ_16K - VLAN_ETH_HLEN - ETH_FCS_LEN)
 
+#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
+
 static const struct rtl_chip_info {
 	u32 mask;
 	u32 val;
@@ -729,6 +735,12 @@ enum rtl_dash_type {
 	RTL_DASH_25_BP,
 };
 
+enum rtl_sfp_mode {
+	RTL_SFP_NONE,
+	RTL_SFP_8168_AF,
+	RTL_SFP_8127_ATF,
+};
+
 struct rtl8169_private {
 	void __iomem *mmio_addr;	/* memory map physical address */
 	struct pci_dev *pci_dev;
@@ -737,6 +749,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;
@@ -764,7 +777,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;
@@ -778,6 +790,7 @@ struct rtl8169_private {
 	u32 ocp_base;
 	struct phylink *phylink;
 	struct phylink_config phylink_config;
+	struct phylink_pcs pcs;
 };
 
 typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
@@ -1133,7 +1146,7 @@ static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 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))
+	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);
@@ -1287,6 +1300,13 @@ static void mac_mcu_write(struct rtl8169_private *tp, int reg, int value)
 	r8168_mac_ocp_write(tp, tp->ocp_base + reg, value);
 }
 
+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 mac_mcu_read(struct rtl8169_private *tp, int reg)
 {
 	return r8168_mac_ocp_read(tp, tp->ocp_base + reg);
@@ -1582,6 +1602,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_8168_AF;
+	}
+
+	return RTL_SFP_NONE;
+}
+
 static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private *tp)
 {
 	switch (tp->mac_version) {
@@ -2397,7 +2431,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 phylink_ethtool_ksettings_set(tp->phylink, cmd);
 
 	if (cmd->base.autoneg != AUTONEG_DISABLE)
@@ -2509,9 +2543,10 @@ void r8169_apply_firmware(struct rtl8169_private *tp)
 		tp->ocp_base = OCP_STD_PHY_BASE;
 
 		/* PHY soft reset may still be in progress */
-		phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
-				      !(val & BMCR_RESET),
-				      50000, 600000, true);
+		if (tp->phydev)
+			phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
+					      !(val & BMCR_RESET),
+					      50000, 600000, true);
 	}
 }
 
@@ -2562,7 +2597,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);
 
 	genphy_soft_reset(tp->phydev);
@@ -3697,12 +3732,14 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp)
 
 	rtl_pcie_state_l2l3_disable(tp);
 
-	rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
-	if (rg_saw_cnt > 0) {
-		u16 sw_cnt_1ms_ini;
+	if (tp->phydev) {
+		rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
+		if (rg_saw_cnt > 0) {
+			u16 sw_cnt_1ms_ini;
 
-		sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
-		r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
+			sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
+			r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
+		}
 	}
 
 	r8168_mac_ocp_modify(tp, 0xe056, 0x00f0, 0x0000);
@@ -4879,7 +4916,7 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 		goto out;
 	}
 
-	if (status & LinkChg)
+	if (status & LinkChg && tp->phydev)
 		phy_mac_interrupt(tp->phydev);
 
 	rtl_irq_disable(tp);
@@ -4995,7 +5032,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
 	phylink_stop(tp->phylink);
 
 	/* 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);
@@ -5017,9 +5054,11 @@ static void rtl8169_up(struct rtl8169_private *tp)
 		rtl8168_driver_start(tp);
 
 	pci_set_master(tp->pci_dev);
-	phy_init_hw(tp->phydev);
-	phy_resume(tp->phydev);
-	rtl8169_init_phy(tp);
+	if (tp->phydev) {
+		phy_init_hw(tp->phydev);
+		phy_resume(tp->phydev);
+		rtl8169_init_phy(tp);
+	}
 
 	/* We may have called phy_speed_down before */
 	phylink_speed_up(tp->phylink);
@@ -5100,9 +5139,11 @@ static int rtl_open(struct net_device *dev)
 	if (retval < 0)
 		goto err_release_fw_2;
 
-	retval = r8169_phy_connect(tp);
-	if (retval)
-		goto err_free_irq;
+	if (tp->phydev) {
+		retval = r8169_phy_connect(tp);
+		if (retval)
+			goto err_free_irq;
+	}
 
 	rtl8169_up(tp);
 	rtl8169_init_counter_offsets(tp);
@@ -5616,6 +5657,10 @@ static void rtl_mac_link_up(struct phylink_config *config, struct phy_device *ph
 static struct phylink_pcs *rtl_mac_select_pcs(struct phylink_config *config,
 					      phy_interface_t interface)
 {
+	struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
+
+	if (interface == PHY_INTERFACE_MODE_1000BASEX || interface == PHY_INTERFACE_MODE_SGMII)
+		return &tp->pcs;
 	return NULL;
 }
 
@@ -5624,6 +5669,55 @@ static void rtl_mac_config(struct phylink_config *config, unsigned int mode,
 {
 }
 
+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 void rtl8169_pcs_get_state(struct phylink_pcs *pcs,
+				  unsigned int neg_mode,
+				  struct phylink_link_state *state)
+{
+	struct rtl8169_private *tp = container_of(pcs, struct rtl8169_private, pcs);
+	u16 bmsr, lpa;
+
+	bmsr = rtl8116af_sds_read(tp, RTL_SDS_C22_BASE + MII_BMSR);
+	lpa = rtl8116af_sds_read(tp, RTL_SDS_C22_BASE + MII_LPA);
+
+	state->link = !!(bmsr & BMSR_LSTATUS);
+	state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE);
+	if (state->link) {
+		state->speed = SPEED_1000;
+		state->duplex = DUPLEX_FULL;
+	} else {
+		state->speed = SPEED_UNKNOWN;
+		state->duplex = DUPLEX_UNKNOWN;
+	}
+
+	if (lpa & LPA_1000XPAUSE)
+		state->pause |= MLO_PAUSE_RX | MLO_PAUSE_TX;
+}
+
+static int rtl8169_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
+			      phy_interface_t interface,
+			      const unsigned long *advertising,
+			      bool permit_pause_to_mac)
+{
+	return 0;
+}
+
+static int rtl8169_pcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
+				const struct phylink_link_state *state)
+{
+	return 0;
+}
+
+static void rtl8169_pcs_an_restart(struct phylink_pcs *pcs)
+{
+}
+
 static const struct phylink_mac_ops rtl_phylink_mac_ops = {
 	.mac_select_pcs = rtl_mac_select_pcs,
 	.mac_config = rtl_mac_config,
@@ -5631,6 +5725,13 @@ static const struct phylink_mac_ops rtl_phylink_mac_ops = {
 	.mac_link_up    = rtl_mac_link_up,
 };
 
+static const struct phylink_pcs_ops r8169_pcs_ops = {
+	.pcs_validate = rtl8169_pcs_validate,
+	.pcs_get_state = rtl8169_pcs_get_state,
+	.pcs_config = rtl8169_pcs_config,
+	.pcs_an_restart = rtl8169_pcs_an_restart,
+};
+
 static int rtl_init_phylink(struct rtl8169_private *tp)
 {
 	struct phylink *pl;
@@ -5642,10 +5743,19 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
 
 	tp->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
 
-	if (tp->sfp_mode) {
+	switch (tp->sfp_mode) {
+	case RTL_SFP_8168_AF:
+		tp->pcs.ops = &r8169_pcs_ops;
+		tp->pcs.poll = true;
+		tp->phylink_config.default_an_inband = true;
+		phy_mode = PHY_INTERFACE_MODE_1000BASEX;
+		tp->phylink_config.mac_capabilities |= MAC_1000FD;
+		break;
+	case RTL_SFP_8127_ATF:
 		phy_mode = PHY_INTERFACE_MODE_INTERNAL;
 		tp->phylink_config.mac_capabilities |= MAC_10000FD;
-	} else {
+		break;
+	default:
 		tp->phylink_config.mac_capabilities |= MAC_10 | MAC_100;
 		phy_mode = PHY_INTERFACE_MODE_INTERNAL;
 
@@ -5660,6 +5770,7 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
 		else
 			if (tp->supports_gmii)
 				tp->phylink_config.mac_capabilities |= MAC_1000FD;
+		break;
 	}
 
 	__set_bit(phy_mode, tp->phylink_config.supported_interfaces);
@@ -5754,12 +5865,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);
@@ -5867,10 +5973,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc)
 		return rc;
 
-	rc = r8169_mdio_register(tp);
-	if (rc) {
-		phylink_destroy(tp->phylink);
-		return rc;
+	if (tp->sfp_mode != RTL_SFP_8168_AF) {
+		rc = r8169_mdio_register(tp);
+		if (rc) {
+			phylink_destroy(tp->phylink);
+			return rc;
+		}
 	}
 
 	rc = register_netdev(dev);
-- 
2.43.0

[PATCH net-next v1 5/6] 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 7b79305635fc..b9c071b7b081 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -341,11 +341,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 {
@@ -3137,8 +3139,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;
@@ -3158,6 +3174,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)
@@ -3191,6 +3208,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);
@@ -3203,6 +3221,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;
@@ -3716,7 +3735,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);
 
@@ -3743,9 +3764,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 6/6] 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 b9c071b7b081..9c0a76f70d31 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -348,6 +348,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 {
@@ -3715,6 +3718,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[] = {
@@ -3773,6 +3803,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/6] r8169: add current_speed in private struct

[Andrew Lunn]

On Fri, Jun 05, 2026 at 06:39:00PM +0800, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
> 
> This patch adds current_speed in private strcut in order to decouple

struct.

> from phydev in the following patch supporting for phylink.
> 
> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index ec4fc21fa21f..fdc8c84dd112 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -750,6 +750,7 @@ struct rtl8169_private {
>  	u32 irq_mask;
>  	int irq;
>  	struct clk *clk;
> +	int current_speed;

Maybe just speed? The code would then be more consistent?


    Andrew

---
pw-bot: cr

Re: [PATCH net-next v1 2/6] r8169: add support for phylink

[Maxime Chevallier]

Hi,

On 6/5/26 12:39, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
> 
> Transfer old framework to phylink. Phylink can support fiber mode card
> which can not get link status or link speed from standard phy registers.

This is a good start, but you need to go deeper than that. Looking at the
end result, you still access tp->phydev a lot in this driver :

Looking at r8169_mdio_register() for example :

 -> don't configure the PHY eee support with phy_support_eee() and
    phy_disable_eee_mode(), let phylink to that for you

All over the driver, there's still a lot of manual control of the PHY
with phylib, look at the calls for phy_init_hw(), phy_resume(), and so on,
you need to assume that with phylink you may not have a PHY.

The problem then is that there are places in the code where PHY registers
are directly accessed :

static void rtl8169_init_phy(struct rtl8169_private *tp)
{
[...]

	if (tp->mac_version == RTL_GIGA_MAC_VER_05 &&
	    tp->pci_dev->subsystem_vendor == PCI_VENDOR_ID_GIGABYTE &&
	    tp->pci_dev->subsystem_device == 0xe000)
		phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b);
[...]
	genphy_soft_reset(tp->phydev);
}

In the end, you shouldn't even need to use tp->phydev at all.

It's hard to know what this is all about, but it seems like something
a PHY driver should have to do, not a MAC driver :(

Also, I'd merge the next commit with this one to have one single commit
doing the phylink conversion.

Maxime

> 
> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
> ---
>  drivers/net/ethernet/realtek/Kconfig      |   1 +
>  drivers/net/ethernet/realtek/r8169_main.c | 170 ++++++++++++++++------
>  2 files changed, 123 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/Kconfig b/drivers/net/ethernet/realtek/Kconfig
> index 9b0f4f9631db..49ac72734225 100644
> --- a/drivers/net/ethernet/realtek/Kconfig
> +++ b/drivers/net/ethernet/realtek/Kconfig
> @@ -88,6 +88,7 @@ config R8169
>  	select CRC32
>  	select PHYLIB
>  	select REALTEK_PHY
> +	select PHYLINK
>  	help
>  	  Say Y here if you have a Realtek Ethernet adapter belonging to
>  	  the following families:
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index fdc8c84dd112..97bcd36efdbb 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -26,6 +26,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/bitfield.h>
> +#include <linux/phylink.h>
>  #include <linux/prefetch.h>
>  #include <linux/ipv6.h>
>  #include <linux/unaligned.h>
> @@ -775,6 +776,8 @@ struct rtl8169_private {
>  	struct r8169_led_classdev *leds;
>  
>  	u32 ocp_base;
> +	struct phylink *phylink;
> +	struct phylink_config phylink_config;
>  };
>  
>  typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
> @@ -2554,9 +2557,6 @@ static void rtl8169_init_phy(struct rtl8169_private *tp)
>  	if (tp->sfp_mode)
>  		rtl_sfp_init(tp);
>  
> -	/* We may have called phy_speed_down before */
> -	phy_speed_up(tp->phydev);
> -
>  	genphy_soft_reset(tp->phydev);
>  }
>  
> @@ -2658,13 +2658,10 @@ static void rtl_jumbo_config(struct rtl8169_private *tp)
>  		pcie_set_readrq(tp->pci_dev, readrq);
>  
>  	/* Chip doesn't support pause in jumbo mode */
> -	if (jumbo) {
> -		linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> -				   tp->phydev->advertising);
> -		linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> -				   tp->phydev->advertising);
> -		phy_start_aneg(tp->phydev);
> -	}
> +	if (jumbo)
> +		tp->phylink_config.mac_capabilities &= ~(MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
> +	else
> +		tp->phylink_config.mac_capabilities |= (MAC_SYM_PAUSE | MAC_ASYM_PAUSE);
>  }
>  
>  DECLARE_RTL_COND(rtl_chipcmd_cond)
> @@ -2779,7 +2776,7 @@ static void rtl_prepare_power_down(struct rtl8169_private *tp)
>  		rtl_ephy_write(tp, 0x19, 0xff64);
>  
>  	if (device_may_wakeup(tp_to_dev(tp))) {
> -		phy_speed_down(tp->phydev, false);
> +		phylink_speed_down(tp->phylink, false);
>  		rtl_wol_enable_rx(tp);
>  	}
>  }
> @@ -4139,11 +4136,17 @@ static int rtl8169_change_mtu(struct net_device *dev, int new_mtu)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
>  
> +	if (netif_running(dev))
> +		phylink_stop(tp->phylink);
> +
>  	WRITE_ONCE(dev->mtu, new_mtu);
>  	netdev_update_features(dev);
>  	rtl_jumbo_config(tp);
>  	rtl_set_eee_txidle_timer(tp);
>  
> +	if (netif_running(dev))
> +		phylink_start(tp->phylink);
> +
>  	return 0;
>  }
>  
> @@ -4962,41 +4965,15 @@ static void rtl_enable_tx_lpi(struct rtl8169_private *tp, bool enable)
>  	}
>  }
>  
> -static void r8169_phylink_handler(struct net_device *ndev)
> -{
> -	struct rtl8169_private *tp = netdev_priv(ndev);
> -	struct device *d = tp_to_dev(tp);
> -
> -	tp->current_speed = tp->phydev->speed;
> -	if (netif_carrier_ok(ndev)) {
> -		rtl_link_chg_patch(tp, tp->current_speed);
> -		rtl_enable_tx_lpi(tp, tp->phydev->enable_tx_lpi);
> -		pm_request_resume(d);
> -	} else {
> -		pm_runtime_idle(d);
> -	}
> -
> -	phy_print_status(tp->phydev);
> -}
> -
>  static int r8169_phy_connect(struct rtl8169_private *tp)
>  {
> -	struct phy_device *phydev = tp->phydev;
> -	phy_interface_t phy_mode;
>  	int ret;
>  
> -	phy_mode = tp->supports_gmii ? PHY_INTERFACE_MODE_GMII :
> -		   PHY_INTERFACE_MODE_MII;
> -
> -	ret = phy_connect_direct(tp->dev, phydev, r8169_phylink_handler,
> -				 phy_mode);
> -	if (ret)
> +	ret = phylink_connect_phy(tp->phylink, tp->phydev);
> +	if (ret) {
> +		netdev_err(tp->dev, "failed to connect phy\n");
>  		return ret;
> -
> -	if (!tp->supports_gmii)
> -		phy_set_max_speed(phydev, SPEED_100);
> -
> -	phy_attached_info(phydev);
> +	}
>  
>  	return 0;
>  }
> @@ -5007,7 +4984,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
>  	/* Clear all task flags */
>  	bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
>  
> -	phy_stop(tp->phydev);
> +	phylink_stop(tp->phylink);
>  
>  	/* Reset SerDes PHY to bring down fiber link */
>  	if (tp->sfp_mode)
> @@ -5035,11 +5012,14 @@ static void rtl8169_up(struct rtl8169_private *tp)
>  	phy_init_hw(tp->phydev);
>  	phy_resume(tp->phydev);
>  	rtl8169_init_phy(tp);
> +
> +	/* We may have called phy_speed_down before */
> +	phylink_speed_up(tp->phylink);
>  	napi_enable(&tp->napi);
>  	enable_work(&tp->wk.work);
>  	rtl_reset_work(tp);
>  
> -	phy_start(tp->phydev);
> +	phylink_start(tp->phylink);
>  }
>  
>  static int rtl8169_close(struct net_device *dev)
> @@ -5055,7 +5035,7 @@ static int rtl8169_close(struct net_device *dev)
>  
>  	free_irq(tp->irq, tp);
>  
> -	phy_disconnect(tp->phydev);
> +	phylink_disconnect_phy(tp->phylink);
>  
>  	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
>  			  tp->RxPhyAddr);
> @@ -5288,6 +5268,8 @@ static void rtl_remove_one(struct pci_dev *pdev)
>  		r8169_remove_leds(tp->leds);
>  
>  	unregister_netdev(tp->dev);
> +	if (tp->phylink)
> +		phylink_destroy(tp->phylink);
>  
>  	if (tp->dash_type != RTL_DASH_NONE)
>  		rtl8168_driver_stop(tp);
> @@ -5474,10 +5456,8 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
>  		return -EUNATCH;
>  	}
>  
> -	tp->phydev->mac_managed_pm = true;
>  	if (rtl_supports_eee(tp))
>  		phy_support_eee(tp->phydev);
> -	phy_support_asym_pause(tp->phydev);
>  
>  	/* mimic behavior of r8125/r8126 vendor drivers */
>  	if (tp->mac_version == RTL_GIGA_MAC_VER_61)
> @@ -5599,6 +5579,92 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
>  	return false;
>  }
>  
> +static void rtl_mac_link_down(struct phylink_config *config, unsigned int mode,
> +			      phy_interface_t interface)
> +{
> +	struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
> +
> +	tp->current_speed = SPEED_UNKNOWN;
> +	pm_runtime_idle(tp_to_dev(tp));
> +}
> +
> +static void rtl_mac_link_up(struct phylink_config *config, struct phy_device *phydev,
> +			    unsigned int mode, phy_interface_t interface,
> +			    int speed, int duplex, bool tx_pause, bool rx_pause)
> +{
> +	struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
> +
> +	struct device *d = tp_to_dev(tp);
> +
> +	tp->current_speed = speed;
> +	rtl_link_chg_patch(tp, speed);
> +
> +	if (phydev)
> +		rtl_enable_tx_lpi(tp, phydev->enable_tx_lpi);
> +
> +	pm_request_resume(d);
> +}
> +
> +static struct phylink_pcs *rtl_mac_select_pcs(struct phylink_config *config,
> +					      phy_interface_t interface)
> +{
> +	return NULL;
> +}
> +
> +static void rtl_mac_config(struct phylink_config *config, unsigned int mode,
> +			   const struct phylink_link_state *state)
> +{
> +}
> +
> +static const struct phylink_mac_ops rtl_phylink_mac_ops = {
> +	.mac_select_pcs = rtl_mac_select_pcs,
> +	.mac_config = rtl_mac_config,
> +	.mac_link_down  = rtl_mac_link_down,
> +	.mac_link_up    = rtl_mac_link_up,
> +};
> +
> +static int rtl_init_phylink(struct rtl8169_private *tp)
> +{
> +	struct phylink *pl;
> +	phy_interface_t phy_mode;
> +
> +	tp->phylink_config.dev = &tp->dev->dev;
> +	tp->phylink_config.type = PHYLINK_NETDEV;
> +	tp->phylink_config.mac_managed_pm = true;
> +
> +	tp->phylink_config.mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE;
> +
> +	if (tp->sfp_mode) {
> +		phy_mode = PHY_INTERFACE_MODE_INTERNAL;
> +		tp->phylink_config.mac_capabilities |= MAC_10000FD;
> +	} else {
> +		tp->phylink_config.mac_capabilities |= MAC_10 | MAC_100;
> +		phy_mode = PHY_INTERFACE_MODE_INTERNAL;
> +
> +		if (tp->mac_version == RTL_GIGA_MAC_VER_80)
> +			tp->phylink_config.mac_capabilities |= MAC_1000FD | MAC_2500FD |
> +							       MAC_5000FD | MAC_10000FD;
> +		else if (tp->mac_version == RTL_GIGA_MAC_VER_70)
> +			tp->phylink_config.mac_capabilities |= MAC_1000FD |
> +							       MAC_2500FD | MAC_5000FD;
> +		else if (tp->mac_version >= RTL_GIGA_MAC_VER_61)
> +			tp->phylink_config.mac_capabilities |= MAC_1000FD | MAC_2500FD;
> +		else
> +			if (tp->supports_gmii)
> +				tp->phylink_config.mac_capabilities |= MAC_1000FD;
> +	}
> +
> +	__set_bit(phy_mode, tp->phylink_config.supported_interfaces);
> +	pl = phylink_create(&tp->phylink_config, tp_to_dev(tp)->fwnode,
> +			    phy_mode, &rtl_phylink_mac_ops);
> +	if (IS_ERR(pl))
> +		return PTR_ERR(pl);
> +
> +	tp->phylink = pl;
> +
> +	return 0;
> +}
> +
>  static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	const struct rtl_chip_info *chip;
> @@ -5789,13 +5855,21 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  
>  	pci_set_drvdata(pdev, tp);
>  
> -	rc = r8169_mdio_register(tp);
> +	rc = rtl_init_phylink(tp);
>  	if (rc)
>  		return rc;
>  
> +	rc = r8169_mdio_register(tp);
> +	if (rc) {
> +		phylink_destroy(tp->phylink);
> +		return rc;
> +	}
> +
>  	rc = register_netdev(dev);
> -	if (rc)
> +	if (rc) {
> +		phylink_destroy(tp->phylink);
>  		return rc;
> +	}
>  
>  	if (IS_ENABLED(CONFIG_R8169_LEDS)) {
>  		if (rtl_is_8125(tp))

Re: [PATCH net-next v1 2/6] r8169: add support for phylink

[Andrew Lunn]

> +static void rtl_mac_link_up(struct phylink_config *config, struct phy_device *phydev,
> +			    unsigned int mode, phy_interface_t interface,
> +			    int speed, int duplex, bool tx_pause, bool rx_pause)
> +{
> +	struct rtl8169_private *tp = container_of(config, struct rtl8169_private, phylink_config);
> +
> +	struct device *d = tp_to_dev(tp);
> +
> +	tp->current_speed = speed;
> +	rtl_link_chg_patch(tp, speed);
> +
> +	if (phydev)
> +		rtl_enable_tx_lpi(tp, phydev->enable_tx_lpi);

This is not correct. You should implement .mac_disable_tx_lpi and
.mac_enable_tx_lpi in phylink_mac_ops.

	Andrew

Re: [PATCH net-next v1 4/6] r8169: add support for RTL8116af

[Andrew Lunn]

> +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;

Do we know what these magic numbers mean?

>  static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private *tp)
>  {
>  	switch (tp->mac_version) {
> @@ -2397,7 +2431,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 phylink_ethtool_ksettings_set(tp->phylink, cmd);

Is this even needed? phylink should be able to handle sfp and copper
in the same way.

> @@ -2509,9 +2543,10 @@ void r8169_apply_firmware(struct rtl8169_private *tp)
>  		tp->ocp_base = OCP_STD_PHY_BASE;
>  
>  		/* PHY soft reset may still be in progress */
> -		phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
> -				      !(val & BMCR_RESET),
> -				      50000, 600000, true);
> +		if (tp->phydev)
> +			phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
> +					      !(val & BMCR_RESET),
> +					      50000, 600000, true);

Maybe this all needs to move into the PHY driver?

> -	rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
> -	if (rg_saw_cnt > 0) {
> -		u16 sw_cnt_1ms_ini;
> +	if (tp->phydev) {
> +		rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
> +		if (rg_saw_cnt > 0) {
> +			u16 sw_cnt_1ms_ini;
>  
> -		sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
> -		r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
> +			sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
> +			r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
> +		}

Can this move into the PHY driver?

> @@ -5017,9 +5054,11 @@ static void rtl8169_up(struct rtl8169_private *tp)
>  		rtl8168_driver_start(tp);
>  
>  	pci_set_master(tp->pci_dev);
> -	phy_init_hw(tp->phydev);
> -	phy_resume(tp->phydev);
> -	rtl8169_init_phy(tp);
> +	if (tp->phydev) {
> +		phy_init_hw(tp->phydev);
> +		phy_resume(tp->phydev);
> +		rtl8169_init_phy(tp);
> +	}

Why is this needed?

	Andrew

RE: [PATCH net-next v1 4/6] r8169: add support for RTL8116af

[Javen]

>> +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;
>
>Do we know what these magic numbers mean?

0xdc00 is a package-detect field. 0xd006 is internal HW id. RTL8116AF shares the same RTL_GIGA_MAC_VER_52 mac_version with other variants. 

>
>>  static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private
>> *tp)  {
>>       switch (tp->mac_version) {
>> @@ -2397,7 +2431,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 phylink_ethtool_ksettings_set(tp->phylink, cmd);
>
>Is this even needed? phylink should be able to handle sfp and copper in the
>same way.

I will try to handle this.

>
>> @@ -2509,9 +2543,10 @@ void r8169_apply_firmware(struct
>rtl8169_private *tp)
>>               tp->ocp_base = OCP_STD_PHY_BASE;
>>
>>               /* PHY soft reset may still be in progress */
>> -             phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
>> -                                   !(val & BMCR_RESET),
>> -                                   50000, 600000, true);
>> +             if (tp->phydev)
>> +                     phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
>> +                                           !(val & BMCR_RESET),
>> +                                           50000, 600000, true);
>
>Maybe this all needs to move into the PHY driver?

This is after firmware application. And PHY_MDIO_CHG opcode switches the access callbacks between PHY and MAC accessors.
data == 0: phy_read/phy_write
data != 0: mac_mcu_read/mac_mcu_write
So the firmware may contain mixed PHY and MAC.

>
>> -     rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
>> -     if (rg_saw_cnt > 0) {
>> -             u16 sw_cnt_1ms_ini;
>> +     if (tp->phydev) {
>> +             rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
>> +             if (rg_saw_cnt > 0) {
>> +                     u16 sw_cnt_1ms_ini;
>>
>> -             sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
>> -             r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
>> +                     sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
>> +                     r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
>> +             }
>
>Can this move into the PHY driver?

It reads a counter from PHY, but the calculated value is programmed into MAC OCP register via r8168_mac_ocp_modify, which accesses r8169 through tp->mmio_addr. So I think this can not be moved.

>
>> @@ -5017,9 +5054,11 @@ static void rtl8169_up(struct rtl8169_private *tp)
>>               rtl8168_driver_start(tp);
>>
>>       pci_set_master(tp->pci_dev);
>> -     phy_init_hw(tp->phydev);
>> -     phy_resume(tp->phydev);
>> -     rtl8169_init_phy(tp);
>> +     if (tp->phydev) {
>> +             phy_init_hw(tp->phydev);
>> +             phy_resume(tp->phydev);
>> +             rtl8169_init_phy(tp);
>> +     }
>
>Why is this needed?

I will try to remove this.

Thanks for your review.

BRs,
Javen

Re: [PATCH net-next v1 4/6] r8169: add support for RTL8116af

[Andrew Lunn]

On Mon, Jun 08, 2026 at 06:32:19AM +0000, Javen wrote:
> >> +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;
> >
> >Do we know what these magic numbers mean?
> 
> 0xdc00 is a package-detect field. 0xd006 is internal HW id. RTL8116AF shares the same RTL_GIGA_MAC_VER_52 mac_version with other variants. 

Since you know what they are, please add #defines.

> >> @@ -2509,9 +2543,10 @@ void r8169_apply_firmware(struct
> >rtl8169_private *tp)
> >>               tp->ocp_base = OCP_STD_PHY_BASE;
> >>
> >>               /* PHY soft reset may still be in progress */
> >> -             phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
> >> -                                   !(val & BMCR_RESET),
> >> -                                   50000, 600000, true);
> >> +             if (tp->phydev)
> >> +                     phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
> >> +                                           !(val & BMCR_RESET),
> >> +                                           50000, 600000, true);
> >
> >Maybe this all needs to move into the PHY driver?
> 
> This is after firmware application. And PHY_MDIO_CHG opcode switches the access callbacks between PHY and MAC accessors.
> data == 0: phy_read/phy_write
> data != 0: mac_mcu_read/mac_mcu_write
> So the firmware may contain mixed PHY and MAC.

Normally, the MAC does not touch the PHY, it only calls phylib/phylink
API methods. If standard MII interfaces are used, you can connect any
MAC to any PHY.

If it is all integrated into silicon, then there is no choice, you
know the MAC/PHY relationship. However, it is still good practice to
keep MAC code in the MAC driver and PHY code in the PHY driver.

Can firmware programming be moved into the PHY driver?

> >> -     rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
> >> -     if (rg_saw_cnt > 0) {
> >> -             u16 sw_cnt_1ms_ini;
> >> +     if (tp->phydev) {
> >> +             rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
> >> +             if (rg_saw_cnt > 0) {
> >> +                     u16 sw_cnt_1ms_ini;
> >>
> >> -             sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
> >> -             r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
> >> +                     sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
> >> +                     r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
> >> +             }
> >
> >Can this move into the PHY driver?
> 
> It reads a counter from PHY, but the calculated value is programmed into MAC OCP register via r8168_mac_ocp_modify, which accesses r8169 through tp->mmio_addr. So I think this can not be moved.

So, big picture, what is this doing? What is rg_saw_cnt?

	Andrew

RE: [PATCH net-next v1 2/6] r8169: add support for phylink

[Javen]

>
>Hi,
>
>On 6/5/26 12:39, javen wrote:
>> From: Javen Xu <javen_xu@realsil.com.cn>
>>
>> Transfer old framework to phylink. Phylink can support fiber mode card
>> which can not get link status or link speed from standard phy registers.
>
>This is a good start, but you need to go deeper than that. Looking at the end
>result, you still access tp->phydev a lot in this driver :
>
>Looking at r8169_mdio_register() for example :
>
> -> don't configure the PHY eee support with phy_support_eee() and
>    phy_disable_eee_mode(), let phylink to that for you
>
>All over the driver, there's still a lot of manual control of the PHY with phylib,
>look at the calls for phy_init_hw(), phy_resume(), and so on, you need to
>assume that with phylink you may not have a PHY.
>
>The problem then is that there are places in the code where PHY registers are
>directly accessed :
>
>static void rtl8169_init_phy(struct rtl8169_private *tp) { [...]
>
>        if (tp->mac_version == RTL_GIGA_MAC_VER_05 &&
>            tp->pci_dev->subsystem_vendor == PCI_VENDOR_ID_GIGABYTE &&
>            tp->pci_dev->subsystem_device == 0xe000)
>                phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b); [...]
>        genphy_soft_reset(tp->phydev);
>}
>
>In the end, you shouldn't even need to use tp->phydev at all.
>
>It's hard to know what this is all about, but it seems like something a PHY
>driver should have to do, not a MAC driver :(
>
>Also, I'd merge the next commit with this one to have one single commit doing
>the phylink conversion.
>
>Maxime
>
Hi Maxime,

I have one question about the existing link-change interrupt model.

For most r8169 chips, the PHY exists and the current driver uses the MAC LinkChg interrupt to notify phylib:
        if (status & LinkChg)
                phy_mac_interrupt(tp->phydev);

After converting the driver to phylink, my understanding is that the MAC driver should avoid keeping a private tp->phydev pointer. The PHY can still be found during setup and passed to phylink_connect_phy(), but then I am not sure how the MAC-routed PHY interrupt should be handled.

Would switching the internal PHY to PHY_POLL be acceptable , or would you prefer to keep the existing interrupt-driven behaviour? If the latter, what would be the preferred way to notify phylib without storing tp->phydev in the driver?

I also have a related question about the existing firmware handling and sw_cnt_1ms_ini logic. Some of this code touches MAC MMIO registers, so it does not seem suitable to move it entirely into the Realtek PHY driver. The current code uses tp->phydev in this area.

What would be the preferred phylink-compatible way to handle this? Should the driver still keep the tp->phydev, or is there another recommended approach?

Link: https://lore.kernel.org/netdev/b54b5f3a6703498fbc17e64548550ba0@realsil.com.cn/

Thanks,
BRs,
Javen

Re: [PATCH net-next v1 2/6] r8169: add support for phylink

[Maxime Chevallier]

On 6/8/26 09:40, Javen wrote:
>>
>> Hi,
>>
>> On 6/5/26 12:39, javen wrote:
>>> From: Javen Xu <javen_xu@realsil.com.cn>
>>>
>>> Transfer old framework to phylink. Phylink can support fiber mode card
>>> which can not get link status or link speed from standard phy registers.
>>
>> This is a good start, but you need to go deeper than that. Looking at the end
>> result, you still access tp->phydev a lot in this driver :
>>
>> Looking at r8169_mdio_register() for example :
>>
>> -> don't configure the PHY eee support with phy_support_eee() and
>>    phy_disable_eee_mode(), let phylink to that for you
>>
>> All over the driver, there's still a lot of manual control of the PHY with phylib,
>> look at the calls for phy_init_hw(), phy_resume(), and so on, you need to
>> assume that with phylink you may not have a PHY.
>>
>> The problem then is that there are places in the code where PHY registers are
>> directly accessed :
>>
>> static void rtl8169_init_phy(struct rtl8169_private *tp) { [...]
>>
>>        if (tp->mac_version == RTL_GIGA_MAC_VER_05 &&
>>            tp->pci_dev->subsystem_vendor == PCI_VENDOR_ID_GIGABYTE &&
>>            tp->pci_dev->subsystem_device == 0xe000)
>>                phy_write_paged(tp->phydev, 0x0001, 0x10, 0xf01b); [...]
>>        genphy_soft_reset(tp->phydev);
>> }
>>
>> In the end, you shouldn't even need to use tp->phydev at all.
>>
>> It's hard to know what this is all about, but it seems like something a PHY
>> driver should have to do, not a MAC driver :(
>>
>> Also, I'd merge the next commit with this one to have one single commit doing
>> the phylink conversion.
>>
>> Maxime
>>
> Hi Maxime,
> 
> I have one question about the existing link-change interrupt model.
> 
> For most r8169 chips, the PHY exists and the current driver uses the MAC LinkChg interrupt to notify phylib:
>         if (status & LinkChg)
>                 phy_mac_interrupt(tp->phydev);

This is the first time there's an attempt to port a driver that
typically deals with an embedded PHY to phylink, so we need to
sort this out.

The main issue is that this assumes the phy device is an integrated
PHY. But what if the PHY isn't integrated and can deal itself with the
interrupt ? or there's just no PHY ?

I think we could imagine adding a phylink helper that would call
phy_mac_interrupt(pl->phydev), however we need to make sure that
this is only done when the MAC is handling the interrupt on behalf
of the integrated PHY, and not any PHY.

> 
> After converting the driver to phylink, my understanding is that the MAC driver should avoid keeping a private tp->phydev pointer. The PHY can still be found during setup and passed to phylink_connect_phy(), but then I am not sure how the MAC-routed PHY interrupt should be handled.
> 
> Would switching the internal PHY to PHY_POLL be acceptable , or would you prefer to keep the existing interrupt-driven behaviour? If the latter, what would be the preferred way to notify phylib without storing tp->phydev in the driver?

I think we need to address this instead of falling to POLL :)

> 
> I also have a related question about the existing firmware handling and sw_cnt_1ms_ini logic. Some of this code touches MAC MMIO registers, so it does not seem suitable to move it entirely into the Realtek PHY driver. The current code uses tp->phydev in this area.

I'll continue looking at this deeper in the upcoming days :)

Maxime






> 
> What would be the preferred phylink-compatible way to handle this? Should the driver still keep the tp->phydev, or is there another recommended approach?
> 
> Link: https://lore.kernel.org/netdev/b54b5f3a6703498fbc17e64548550ba0@realsil.com.cn/
> 
> Thanks,
> BRs,
> Javen
> 
> 

Re: [PATCH net-next v1 2/6] r8169: add support for phylink

[Andrew Lunn]

> > Hi Maxime,
> > 
> > I have one question about the existing link-change interrupt model.
> > 
> > For most r8169 chips, the PHY exists and the current driver uses the MAC LinkChg interrupt to notify phylib:
> >         if (status & LinkChg)
> >                 phy_mac_interrupt(tp->phydev);
> 
> This is the first time there's an attempt to port a driver that
> typically deals with an embedded PHY to phylink, so we need to
> sort this out.

Well, mv88e6xxx has something similar, for its embedded PHYs. For
that, we export the interrupt controller as a linux interrupt
controller. We then allow two different things:

1) The PHY has a phandle to the interrupt. That causes phylib to
request the interrupt.

2) The interrupt controller code fills mdiobus->irq[addr] with the
interrupt number, so that phylib will request the interrupt.

	  Andrew

RE: [PATCH net-next v1 4/6] r8169: add support for RTL8116af

[Javen]

Hi, Andrew, Maxime

>On Mon, Jun 08, 2026 at 06:32:19AM +0000, Javen wrote:
>> >> +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;
>> >
>> >Do we know what these magic numbers mean?
>>
>> 0xdc00 is a package-detect field. 0xd006 is internal HW id. RTL8116AF shares
>the same RTL_GIGA_MAC_VER_52 mac_version with other variants.
>
>Since you know what they are, please add #defines.
>
>> >> @@ -2509,9 +2543,10 @@ void r8169_apply_firmware(struct
>> >rtl8169_private *tp)
>> >>               tp->ocp_base = OCP_STD_PHY_BASE;
>> >>
>> >>               /* PHY soft reset may still be in progress */
>> >> -             phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
>> >> -                                   !(val & BMCR_RESET),
>> >> -                                   50000, 600000, true);
>> >> +             if (tp->phydev)
>> >> +                     phy_read_poll_timeout(tp->phydev, MII_BMCR, val,
>> >> +                                           !(val & BMCR_RESET),
>> >> +                                           50000, 600000, true);
>> >
>> >Maybe this all needs to move into the PHY driver?
>>
>> This is after firmware application. And PHY_MDIO_CHG opcode switches the
>access callbacks between PHY and MAC accessors.
>> data == 0: phy_read/phy_write
>> data != 0: mac_mcu_read/mac_mcu_write
>> So the firmware may contain mixed PHY and MAC.
>
>Normally, the MAC does not touch the PHY, it only calls phylib/phylink API
>methods. If standard MII interfaces are used, you can connect any MAC to any
>PHY.
>
>If it is all integrated into silicon, then there is no choice, you know the
>MAC/PHY relationship. However, it is still good practice to keep MAC code in
>the MAC driver and PHY code in the PHY driver.
>
>Can firmware programming be moved into the PHY driver?

For our PCIe nics, they are single chips. r8169 driver is for the entire integrated chip. Some internal phy shared the same PHY id, but the required PHY parameters differ depending on the specific MAC/Chip it is integrated with. So I think firmware here can not be moved.

>
>> >> -     rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
>> >> -     if (rg_saw_cnt > 0) {
>> >> -             u16 sw_cnt_1ms_ini;
>> >> +     if (tp->phydev) {
>> >> +             rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) &
>0x3fff;
>> >> +             if (rg_saw_cnt > 0) {
>> >> +                     u16 sw_cnt_1ms_ini;
>> >>
>> >> -             sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
>> >> -             r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
>> >> +                     sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
>> >> +                     r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
>> >> +             }
>> >
>> >Can this move into the PHY driver?
>>
>> It reads a counter from PHY, but the calculated value is programmed into
>MAC OCP register via r8168_mac_ocp_modify, which accesses r8169 through
>tp->mmio_addr. So I think this can not be moved.
>
>So, big picture, what is this doing? What is rg_saw_cnt?

UPS mode TX-link-pulse timing calibration. We read rg_saw_cnt from the GPHY page 0x0C42, register 0x13. This value is the SAW calibration result burned during mass production. If rg_saw_cnt == 0, it means the MP did not burn the calibration result, so we do nothing. If calibrated (rg_saw_cnt > 0), we calculate the 1ms timer counter: sw_cnt_1ms_ini = (16000000 / rg_saw_cnt). And then write it into the MAC OCP register (0xD412). This requires reading a hardware-specific value from the PHY to configure a timer register in the MAC. So I think it can not be moved too.

How do you recommend handling this kind of SoC-specific tight coupling within the phylink framework? Is it acceptable to retain a local phydev pointer in the MAC driver solely for these hardware-specific initialization quirks, or is there a preferred approach for single-chip devices?

Any guidance for the migration would be greatly appreciated!

Thanks,
BRs,
Javen
>
>        Andrew

Re: [PATCH net-next v1 4/6] r8169: add support for RTL8116af

[Andrew Lunn]

> >Can firmware programming be moved into the PHY driver?
> 

> For our PCIe nics, they are single chips. r8169 driver is for the
> entire integrated chip. Some internal phy shared the same PHY id,
> but the required PHY parameters differ depending on the specific
> MAC/Chip it is integrated with. So I think firmware here can not be
> moved.

Can you tell them part only using PHY registers? It is a bad design if
you cannot. You can populate the .match_phy_device call with a
function which can peak into registers to determine if the hardware
matches the entry in the PHY driver list.

> >> >> -     rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) & 0x3fff;
> >> >> -     if (rg_saw_cnt > 0) {
> >> >> -             u16 sw_cnt_1ms_ini;
> >> >> +     if (tp->phydev) {
> >> >> +             rg_saw_cnt = phy_read_paged(tp->phydev, 0x0c42, 0x13) &
> >0x3fff;
> >> >> +             if (rg_saw_cnt > 0) {
> >> >> +                     u16 sw_cnt_1ms_ini;
> >> >>
> >> >> -             sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
> >> >> -             r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
> >> >> +                     sw_cnt_1ms_ini = (16000000 / rg_saw_cnt) & 0x0fff;
> >> >> +                     r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
> >> >> +             }
> >> >
> >> >Can this move into the PHY driver?
> >>
> >> It reads a counter from PHY, but the calculated value is programmed into
> >MAC OCP register via r8168_mac_ocp_modify, which accesses r8169 through
> >tp->mmio_addr. So I think this can not be moved.
> >
> >So, big picture, what is this doing? What is rg_saw_cnt?
> 
> UPS mode TX-link-pulse timing calibration. We read rg_saw_cnt from
> the GPHY page 0x0C42, register 0x13. This value is the SAW
> calibration result burned during mass production. If rg_saw_cnt ==
> 0, it means the MP did not burn the calibration result, so we do
> nothing. If calibrated (rg_saw_cnt > 0), we calculate the 1ms timer
> counter: sw_cnt_1ms_ini = (16000000 / rg_saw_cnt). And then write it
> into the MAC OCP register (0xD412). This requires reading a
> hardware-specific value from the PHY to configure a timer register
> in the MAC. So I think it can not be moved too.

> How do you recommend handling this kind of SoC-specific tight
> coupling within the phylink framework? Is it acceptable to retain a
> local phydev pointer in the MAC driver solely for these
> hardware-specific initialization quirks, or is there a preferred
> approach for single-chip devices?

Step one is to not tightly couple hardware like this when you design
it. It just causes problems when you need to make layering violations.

However, the hardware is designed like this, so you have to live with
it. At least for this generation. I hope the next generation is better
designed.

So yes, you can reference the phydev.

	Andrew




> 
> Any guidance for the migration would be greatly appreciated!
> 
> Thanks,
> BRs,
> Javen
> >
> >        Andrew