mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Fabio Baltieri <fabio.baltieri@gmail.com>
Cc: nic_swsd@realtek.com, Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Michael Zimmermann <sigmaepsilon92@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] r8169: add support for RTL8127ATF
Date: Sun, 23 Nov 2025 23:54:41 +0100	[thread overview]
Message-ID: <0cacca03-6302-4e39-a807-06591bf787b1@gmail.com> (raw)
In-Reply-To: <aSNVVoAOQHbleZFF@google.com>

[-- Attachment #1: Type: text/plain, Size: 3707 bytes --]

On 11/23/2025 7:41 PM, Fabio Baltieri wrote:
> Hi Heiner,
> 
> On Sun, Nov 23, 2025 at 04:58:23PM +0100, Heiner Kallweit wrote:
>> This is a version with better integration with phylib, and with 10G support only.
>> Maybe I simplified the PHY/Serdes initialization too much, we'll see.
>> A difference to your version is that via ethtool you now can and have to set autoneg to off.
>>
>> I'd appreciate if you could give it a try and provide a full dmesg log and output of "ethtool <if>".
>>
>> Note: This patch applies on top of net-next and linux-next. However, if you apply it on top
>> of some other recent kernel version, conflicts should be easy to resolve.
> 
> Thanks for the patch, ran some initial tests, I'm on Linus tree for
> other reasons but applied 3dc2a17efc5f, 1479493c91fc, 28c0074fd4b7 and
> the recent suspend fix, then your patch applies cleanly.
> 
> Here's ethtool output:
> 
> # ethtool eth1
> Settings for eth1:
>         Supported ports: [  ]
>         Supported link modes:   10000baseT/Full
>         Supported pause frame use: Symmetric Receive-only
>         Supports auto-negotiation: No
>         Supported FEC modes: Not reported
>         Advertised link modes:  10000baseT/Full
>         Advertised pause frame use: Symmetric Receive-only
>         Advertised auto-negotiation: No
>         Advertised FEC modes: Not reported
>         Speed: 10000Mb/s
>         Duplex: Full
>         Auto-negotiation: off
>         master-slave status: master
>         Port: Twisted Pair
>         PHYAD: 0
>         Transceiver: internal
>         MDI-X: Unknown
>         Supports Wake-on: pumbg
>         Wake-on: d
>         Link detected: yes
> 
> The phy is identified correctly:
> 
> [ 1563.678133] Realtek SFP PHY Mode r8169-1-500:00: attached PHY driver (mii_bus:phy_addr=r8169-1-500:00, irq=MAC)
> 
> That said I've observed two issues with the current patch:
> 
> 1. the link on the other end is flapping, I've seen this while working
> on the original patch and seems to be due to the EEE settings, it is
> addressed by:
> 
> @@ -2439,7 +2439,10 @@ static void rtl8125a_config_eee_mac(struct rtl8169_private *tp)
> 
>  static void rtl8125b_config_eee_mac(struct rtl8169_private *tp)
>  {
> -       r8168_mac_ocp_modify(tp, 0xe040, 0, BIT(1) | BIT(0));
> +       if (tp->sfp_mode)
> +               r8168_mac_ocp_modify(tp, 0xe040, BIT(1) | BIT(0), 0);
> +       else
> +               r8168_mac_ocp_modify(tp, 0xe040, 0, BIT(1) | BIT(0));
>  }
> 
>  static void rtl_rar_exgmac_set(struct rtl8169_private *tp, const u8 *addr)
> 
> 
> 2. the link is lost after a module reload or after an ip link down and
> up, the driver logs "Link is Down" and stays there until the cable is
> unplugged and re-plugged. This seems to be addressed by the code that
> was in rtl8127_sds_phy_reset(), re-adding that code fixes it:
> 
> @@ -2477,6 +2480,13 @@ static void r8127_init_sfp_10g(struct rtl8169_private *tp)
>  {
>         int val;
> 
> +       RTL_W8(tp, 0x2350, RTL_R8(tp, 0x2350) & ~BIT(0));
> +       udelay(1);
> +
> +       RTL_W16(tp, 0x233a, 0x801f);
> +       RTL_W8(tp, 0x2350, RTL_R8(tp, 0x2350) | BIT(0));
> +       udelay(10);
> +
>         RTL_W16(tp, 0x233a, 0x801a);
>         RTL_W16(tp, 0x233e, (RTL_R16(tp, 0x233e) & ~0x3003) | 0x1000);
> 
> Guess the phy needs a reset after all.
> 
> With these two applied it seems to be working fine, tested suspend as
> well.
> 
> Would you integrate these two or want to try me something different?

Thanks a lot for the valuable feedback!
I added the SDS PHY reset to the patch, and improved MAC EEE handling
in a second patch, incl. what you mentioned.
Patches should fully cover your use case now. Please give it a try.

[-- Attachment #2: 0001-r8169-add-support-for-10G-SFP-mode-on-RTL8127ATF.patch --]
[-- Type: text/plain, Size: 6061 bytes --]

From fdd7aa806184f0bc8d8feb80cb2dd2c5397ee134 Mon Sep 17 00:00:00 2001
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 23 Nov 2025 22:29:25 +0100
Subject: [PATCH 1/2] r8169: add support for 10G SFP mode on RTL8127ATF

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 77 +++++++++++++++++++++--
 drivers/net/phy/realtek/realtek_main.c    | 26 ++++++++
 2 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 97dbe8f8933..cbacf1ef87a 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -731,6 +731,7 @@ 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;
@@ -1094,6 +1095,10 @@ 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 0xcbff;
+
 	RTL_W32(tp, GPHY_OCP, reg << 15);
 
 	return rtl_loop_wait_high(tp, &rtl_ocp_gphy_cond, 25, 10) ?
@@ -2305,6 +2310,41 @@ static void rtl8169_get_eth_ctrl_stats(struct net_device *dev,
 		le32_to_cpu(tp->counters->rx_unknown_opcode);
 }
 
+static int rtl8169_set_link_ksettings(struct net_device *ndev,
+				      const struct ethtool_link_ksettings *cmd)
+{
+	struct rtl8169_private *tp = netdev_priv(ndev);
+	struct phy_device *phydev = tp->phydev;
+	int duplex = cmd->base.duplex;
+	int speed = cmd->base.speed;
+
+	if (!tp->sfp_mode)
+		return phy_ethtool_ksettings_set(phydev, cmd);
+
+	if (cmd->base.autoneg != AUTONEG_DISABLE)
+		return -EINVAL;
+
+	if (!phy_check_valid(speed, duplex, phydev->supported))
+		return -EINVAL;
+
+	mutex_lock(&phydev->lock);
+
+	phydev->autoneg = AUTONEG_DISABLE;
+	phydev->speed = speed;
+	phydev->duplex = duplex;
+
+	if (phy_is_started(phydev)) {
+		phydev->state = PHY_UP;
+		phy_trigger_machine(phydev);
+	} else {
+		_phy_start_aneg(phydev);
+	}
+
+	mutex_unlock(&phydev->lock);
+
+	return 0;
+}
+
 static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES,
@@ -2324,7 +2364,7 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.get_eee		= rtl8169_get_eee,
 	.set_eee		= rtl8169_set_eee,
 	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
-	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
+	.set_link_ksettings	= rtl8169_set_link_ksettings,
 	.get_ringparam		= rtl8169_get_ringparam,
 	.get_pause_stats	= rtl8169_get_pause_stats,
 	.get_pauseparam		= rtl8169_get_pauseparam,
@@ -2436,6 +2476,34 @@ static void rtl_schedule_task(struct rtl8169_private *tp, enum rtl_flag flag)
 		clear_bit(flag, tp->wk.flags);
 }
 
+static void r8127_sfp_sds_phy_reset(struct rtl8169_private *tp)
+{
+	RTL_W8(tp, 0x2350, RTL_R8(tp, 0x2350) & ~BIT(0));
+	udelay(1);
+
+	RTL_W16(tp, 0x233a, 0x801f);
+	RTL_W8(tp, 0x2350, RTL_R8(tp, 0x2350) | BIT(0));
+	udelay(10);
+}
+
+static void r8127_sfp_init_10g(struct rtl8169_private *tp)
+{
+	int val;
+
+	r8127_sfp_sds_phy_reset(tp);
+
+	RTL_W16(tp, 0x233a, 0x801a);
+	RTL_W16(tp, 0x233e, (RTL_R16(tp, 0x233e) & ~0x3003) | 0x1000);
+
+	r8168_phy_ocp_write(tp, 0xc40a, 0x0000);
+	r8168_phy_ocp_write(tp, 0xc466, 0x0003);
+	r8168_phy_ocp_write(tp, 0xc808, 0x0000);
+	r8168_phy_ocp_write(tp, 0xc80a, 0x0000);
+
+	val = r8168_phy_ocp_read(tp, 0xc804);
+	r8168_phy_ocp_write(tp, 0xc804, (val & ~0x000f) | 0x000c);
+}
+
 static void rtl8169_init_phy(struct rtl8169_private *tp)
 {
 	r8169_hw_phy_config(tp, tp->phydev, tp->mac_version);
@@ -2452,6 +2520,9 @@ 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)
+		r8127_sfp_init_10g(tp);
+
 	/* We may have called phy_speed_down before */
 	phy_speed_up(tp->phydev);
 
@@ -5460,13 +5531,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 	tp->aspm_manageable = !rc;
 
-	/* Fiber mode on RTL8127AF isn't supported */
 	if (rtl_is_8125(tp)) {
 		u16 data = r8168_mac_ocp_read(tp, 0xd006);
 
 		if ((data & 0xff) == 0x07)
-			return dev_err_probe(&pdev->dev, -ENODEV,
-					     "Fiber mode not supported\n");
+			tp->sfp_mode = true;
 	}
 
 	tp->dash_type = rtl_get_dash_type(tp);
diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 67ecf3d4af2..296559dbc7f 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -1977,6 +1977,18 @@ static irqreturn_t rtl8221b_handle_interrupt(struct phy_device *phydev)
 	return IRQ_HANDLED;
 }
 
+static int rtlgen_sfp_get_features(struct phy_device *phydev)
+{
+	linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+			 phydev->supported);
+	return 0;
+}
+
+static int rtlgen_sfp_config_aneg(struct phy_device *phydev)
+{
+	return 0;
+}
+
 static struct phy_driver realtek_drvs[] = {
 	{
 		PHY_ID_MATCH_EXACT(0x00008201),
@@ -2212,6 +2224,20 @@ 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(0x001ccbff),
+		.name           = "Realtek SFP PHY Mode",
+		.flags		= PHY_IS_INTERNAL,
+		.probe		= rtl822x_probe,
+		.get_features   = rtlgen_sfp_get_features,
+		.config_aneg    = rtlgen_sfp_config_aneg,
+		.read_status    = rtl822x_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",
-- 
2.52.0


[-- Attachment #3: 0002-r8169-improve-MAC-EEE-handling.patch --]
[-- Type: text/plain, Size: 5035 bytes --]

From 9de6ee722512946f9d090bd65070a05dc66e1214 Mon Sep 17 00:00:00 2001
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 23 Nov 2025 23:39:43 +0100
Subject: [PATCH 2/2] r8169: improve MAC EEE handling

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 73 +++++++++++------------
 1 file changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index cbacf1ef87a..33a83bf9035 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -2425,26 +2425,6 @@ void r8169_apply_firmware(struct rtl8169_private *tp)
 	}
 }
 
-static void rtl8168_config_eee_mac(struct rtl8169_private *tp)
-{
-	/* Adjust EEE LED frequency */
-	if (tp->mac_version != RTL_GIGA_MAC_VER_38)
-		RTL_W8(tp, EEE_LED, RTL_R8(tp, EEE_LED) & ~0x07);
-
-	rtl_eri_set_bits(tp, 0x1b0, 0x0003);
-}
-
-static void rtl8125a_config_eee_mac(struct rtl8169_private *tp)
-{
-	r8168_mac_ocp_modify(tp, 0xe040, 0, BIT(1) | BIT(0));
-	r8168_mac_ocp_modify(tp, 0xeb62, 0, BIT(2) | BIT(1));
-}
-
-static void rtl8125b_config_eee_mac(struct rtl8169_private *tp)
-{
-	r8168_mac_ocp_modify(tp, 0xe040, 0, BIT(1) | BIT(0));
-}
-
 static void rtl_rar_exgmac_set(struct rtl8169_private *tp, const u8 *addr)
 {
 	rtl_eri_write(tp, 0xe0, ERIAR_MASK_1111, get_unaligned_le32(addr));
@@ -3253,8 +3233,6 @@ static void rtl_hw_start_8168e_2(struct rtl8169_private *tp)
 
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 
-	rtl8168_config_eee_mac(tp);
-
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) | PFM_EN);
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) | PWM_EN);
 	rtl_mod_config5(tp, Spi_en, 0);
@@ -3279,8 +3257,6 @@ static void rtl_hw_start_8168f(struct rtl8169_private *tp)
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) | PFM_EN);
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) | PWM_EN);
 	rtl_mod_config5(tp, Spi_en, 0);
-
-	rtl8168_config_eee_mac(tp);
 }
 
 static void rtl_hw_start_8168f_1(struct rtl8169_private *tp)
@@ -3330,8 +3306,6 @@ static void rtl_hw_start_8168g(struct rtl8169_private *tp)
 	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000);
 	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000);
 
-	rtl8168_config_eee_mac(tp);
-
 	rtl_w0w1_eri(tp, 0x2fc, 0x01, 0x06);
 	rtl_eri_clear_bits(tp, 0x1b0, BIT(12));
 
@@ -3472,8 +3446,6 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000);
 	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000);
 
-	rtl8168_config_eee_mac(tp);
-
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN);
 	RTL_W8(tp, MISC_1, RTL_R8(tp, MISC_1) & ~PFM_D3COLD_EN);
 
@@ -3521,8 +3493,6 @@ static void rtl_hw_start_8168ep(struct rtl8169_private *tp)
 	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000);
 	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000);
 
-	rtl8168_config_eee_mac(tp);
-
 	rtl_w0w1_eri(tp, 0x2fc, 0x01, 0x06);
 
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~TX_10M_PS_EN);
@@ -3578,8 +3548,6 @@ static void rtl_hw_start_8117(struct rtl8169_private *tp)
 	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000);
 	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000);
 
-	rtl8168_config_eee_mac(tp);
-
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN);
 	RTL_W8(tp, MISC_1, RTL_R8(tp, MISC_1) & ~PFM_D3COLD_EN);
 
@@ -3820,11 +3788,6 @@ static void rtl_hw_start_8125_common(struct rtl8169_private *tp)
 
 	rtl_loop_wait_low(tp, &rtl_mac_ocp_e00e_cond, 1000, 10);
 
-	if (tp->mac_version == RTL_GIGA_MAC_VER_61)
-		rtl8125a_config_eee_mac(tp);
-	else
-		rtl8125b_config_eee_mac(tp);
-
 	rtl_disable_rxdvgate(tp);
 }
 
@@ -4827,6 +4790,41 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
+static void rtl_enable_tx_lpi(struct rtl8169_private *tp, bool enable)
+{
+	if (!rtl_supports_eee(tp))
+		return;
+
+	switch (tp->mac_version) {
+	case RTL_GIGA_MAC_VER_34 ... RTL_GIGA_MAC_VER_52:
+		/* Adjust EEE LED frequency */
+		if (tp->mac_version != RTL_GIGA_MAC_VER_38)
+			RTL_W8(tp, EEE_LED, RTL_R8(tp, EEE_LED) & ~0x07);
+		if (enable)
+			rtl_eri_set_bits(tp, 0x1b0, 0x0003);
+		else
+			rtl_eri_clear_bits(tp, 0x1b0, 0x0003);
+		break;
+	case RTL_GIGA_MAC_VER_61:
+		if (enable) {
+			r8168_mac_ocp_modify(tp, 0xe040, 0, 0x0003);
+			r8168_mac_ocp_modify(tp, 0xeb62, 0, 0x0006);
+		} else {
+			r8168_mac_ocp_modify(tp, 0xe040, 0x0003, 0);
+			r8168_mac_ocp_modify(tp, 0xeb62, 0x0006, 0);
+		}
+		break;
+	case RTL_GIGA_MAC_VER_63 ... RTL_GIGA_MAC_VER_LAST:
+		if (enable)
+			r8168_mac_ocp_modify(tp, 0xe040, 0, 0x0003);
+		else
+			r8168_mac_ocp_modify(tp, 0xe040, 0x0003, 0);
+		break;
+	default:
+		break;
+	}
+}
+
 static void r8169_phylink_handler(struct net_device *ndev)
 {
 	struct rtl8169_private *tp = netdev_priv(ndev);
@@ -4834,6 +4832,7 @@ static void r8169_phylink_handler(struct net_device *ndev)
 
 	if (netif_carrier_ok(ndev)) {
 		rtl_link_chg_patch(tp);
+		rtl_enable_tx_lpi(tp, tp->phydev->enable_tx_lpi);
 		pm_request_resume(d);
 	} else {
 		pm_runtime_idle(d);
-- 
2.52.0


  reply	other threads:[~2025-11-23 22:54 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-20 19:50 Fabio Baltieri
2025-11-20 20:55 ` Heiner Kallweit
2025-11-21 20:28   ` Fabio Baltieri
2025-11-23 15:58     ` Heiner Kallweit
2025-11-23 16:26       ` Michael Zimmermann
2025-11-23 16:59         ` Heiner Kallweit
2025-11-23 18:41       ` Fabio Baltieri
2025-11-23 22:54         ` Heiner Kallweit [this message]
2025-11-24  0:00           ` Fabio Baltieri
2025-12-03 22:11             ` Fabio Baltieri
2025-12-04  6:32               ` Heiner Kallweit
2025-12-29  4:18           ` FUKAUMI Naoki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0cacca03-6302-4e39-a807-06591bf787b1@gmail.com \
    --to=hkallweit1@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fabio.baltieri@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nic_swsd@realtek.com \
    --cc=pabeni@redhat.com \
    --cc=sigmaepsilon92@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®