[PATCH] net: phy: realtek: Add RTL8224 cable testing support

[PATCH] net: phy: realtek: Add RTL8224 cable testing support

[Sven Eckelmann]

From: Issam Hamdi <ih@simonwunderlich.de>

The RTL8224 can detect open pairs and short types (in same pair or some
other pair). The distance to this problem can be estimated. This is done
for each of the 4 pairs separately.

It is not meant to be run while there is an active link partner because
this interferes with the active test pulses.

Output with open 50 m cable:

  Pair A code Open Circuit, source: TDR
  Pair A, fault length: 51.79m, source: TDR
  Pair B code Open Circuit, source: TDR
  Pair B, fault length: 51.28m, source: TDR
  Pair C code Open Circuit, source: TDR
  Pair C, fault length: 50.46m, source: TDR
  Pair D code Open Circuit, source: TDR
  Pair D, fault length: 51.12m, source: TDR

Terminated cable:

  Pair A code OK, source: TDR
  Pair B code OK, source: TDR
  Pair C code OK, source: TDR
  Pair D code OK, source: TDR

Shorted cable (both short types are at roughly the same distance)

  Pair A code Short to another pair, source: TDR
  Pair A, fault length: 2.35m, source: TDR
  Pair B code Short to another pair, source: TDR
  Pair B, fault length: 2.15m, source: TDR
  Pair C code OK, source: TDR
  Pair D code Short within Pair, source: TDR
  Pair D, fault length: 1.94m, source: TDR

Signed-off-by: Issam Hamdi <ih@simonwunderlich.de>
Co-developed-by: Sven Eckelmann <se@simonwunderlich.de>
Signed-off-by: Sven Eckelmann <se@simonwunderlich.de>
---
 drivers/net/phy/realtek/realtek_main.c | 187 +++++++++++++++++++++++++++++++++
 1 file changed, 187 insertions(+)

diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c
index 16a347084293..1fd4b6cf5c1e 100644
--- a/drivers/net/phy/realtek/realtek_main.c
+++ b/drivers/net/phy/realtek/realtek_main.c
@@ -8,6 +8,7 @@
  * Copyright (c) 2004 Freescale Semiconductor, Inc.
  */
 #include <linux/bitops.h>
+#include <linux/ethtool_netlink.h>
 #include <linux/of.h>
 #include <linux/phy.h>
 #include <linux/pm_wakeirq.h>
@@ -127,6 +128,27 @@
  */
 #define RTL822X_VND2_C22_REG(reg)		(0xa400 + 2 * (reg))
 
+#define RTL8224_MII_RTCT			0x11
+#define RTL8224_MII_RTCT_ENABLE			BIT(0)
+#define RTL8224_MII_RTCT_PAIR_A			BIT(4)
+#define RTL8224_MII_RTCT_PAIR_B			BIT(5)
+#define RTL8224_MII_RTCT_PAIR_C			BIT(6)
+#define RTL8224_MII_RTCT_PAIR_D			BIT(7)
+#define RTL8224_MII_RTCT_DONE			BIT(15)
+
+#define RTL8224_MII_SRAM_ADDR			0x1b
+#define RTL8224_MII_SRAM_DATA			0x1c
+
+#define RTL8224_SRAM_RTCT_FAULT(pair)		(0x8026 + (pair) * 4)
+#define RTL8224_SRAM_RTCT_FAULT_BUSY		BIT(0)
+#define RTL8224_SRAM_RTCT_FAULT_OPEN		BIT(3)
+#define RTL8224_SRAM_RTCT_FAULT_SAME_SHORT	BIT(4)
+#define RTL8224_SRAM_RTCT_FAULT_OK		BIT(5)
+#define RTL8224_SRAM_RTCT_FAULT_DONE		BIT(6)
+#define RTL8224_SRAM_RTCT_FAULT_CROSS_SHORT	BIT(7)
+
+#define RTL8224_SRAM_RTCT_LEN(pair)		(0x8028 + (pair) * 4)
+
 #define RTL8366RB_POWER_SAVE			0x15
 #define RTL8366RB_POWER_SAVE_ON			BIT(12)
 
@@ -1453,6 +1475,168 @@ static int rtl822xb_c45_read_status(struct phy_device *phydev)
 	return 0;
 }
 
+static int rtl8224_cable_test_start(struct phy_device *phydev)
+{
+	u32 val;
+	int ret;
+
+	/* disable auto-negotiation and force 1000/Full */
+	ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2,
+			     RTL822X_VND2_C22_REG(MII_BMCR),
+			     BMCR_ANENABLE | BMCR_SPEED100 | BMCR_SPEED10,
+			     BMCR_SPEED1000 | BMCR_FULLDPLX);
+	if (ret)
+		return ret;
+
+	mdelay(500);
+
+	/* trigger cable test */
+	val = RTL8224_MII_RTCT_ENABLE;
+	val |= RTL8224_MII_RTCT_PAIR_A;
+	val |= RTL8224_MII_RTCT_PAIR_B;
+	val |= RTL8224_MII_RTCT_PAIR_C;
+	val |= RTL8224_MII_RTCT_PAIR_D;
+
+	return phy_modify_mmd(phydev, MDIO_MMD_VEND2,
+			      RTL822X_VND2_C22_REG(RTL8224_MII_RTCT),
+			      RTL8224_MII_RTCT_DONE, val);
+}
+
+static int rtl8224_sram_read(struct phy_device *phydev, u32 reg)
+{
+	int ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
+			    RTL822X_VND2_C22_REG(RTL8224_MII_SRAM_ADDR),
+			    reg);
+	if (ret)
+		return ret;
+
+	return phy_read_mmd(phydev, MDIO_MMD_VEND2,
+			    RTL822X_VND2_C22_REG(RTL8224_MII_SRAM_DATA));
+}
+
+static int rtl8224_pair_len_get(struct phy_device *phydev, u32 pair)
+{
+	int cable_len;
+	u32 reg_len;
+	int ret;
+	u32 cm;
+
+	reg_len = RTL8224_SRAM_RTCT_LEN(pair);
+
+	ret = rtl8224_sram_read(phydev, reg_len);
+	if (ret < 0)
+		return ret;
+
+	cable_len = ret & 0xff00;
+
+	ret = rtl8224_sram_read(phydev, reg_len + 1);
+	if (ret < 0)
+		return ret;
+
+	cable_len |= (ret & 0xff00) >> 8;
+
+	cable_len -= 620;
+	cable_len = max(cable_len, 0);
+
+	cm = cable_len * 100 / 78;
+
+	return cm;
+}
+
+static int rtl8224_cable_test_result_trans(u32 result)
+{
+	if (!(result & RTL8224_SRAM_RTCT_FAULT_DONE))
+		return -EBUSY;
+
+	if (result & RTL8224_SRAM_RTCT_FAULT_OK)
+		return ETHTOOL_A_CABLE_RESULT_CODE_OK;
+
+	if (result & RTL8224_SRAM_RTCT_FAULT_OPEN)
+		return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
+
+	if (result & RTL8224_SRAM_RTCT_FAULT_SAME_SHORT)
+		return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
+
+	if (result & RTL8224_SRAM_RTCT_FAULT_BUSY)
+		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+
+	if (result & RTL8224_SRAM_RTCT_FAULT_CROSS_SHORT)
+		return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT;
+
+	return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+}
+
+static int rtl8224_cable_test_report_pair(struct phy_device *phydev, unsigned int pair)
+{
+	int fault_rslt;
+	int ret;
+
+	ret = rtl8224_sram_read(phydev, RTL8224_SRAM_RTCT_FAULT(pair));
+	if (ret < 0)
+		return ret;
+
+	fault_rslt = rtl8224_cable_test_result_trans(ret);
+	if (fault_rslt < 0)
+		return 0;
+
+	ret = ethnl_cable_test_result(phydev, pair, fault_rslt);
+	if (ret < 0)
+		return ret;
+
+	switch (fault_rslt) {
+	case ETHTOOL_A_CABLE_RESULT_CODE_OPEN:
+	case ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT:
+	case ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT:
+		ret = rtl8224_pair_len_get(phydev, pair);
+		if (ret < 0)
+			return ret;
+
+		return ethnl_cable_test_fault_length(phydev, pair, ret);
+	default:
+		return  0;
+	}
+}
+
+static int rtl8224_cable_test_report(struct phy_device *phydev, bool *finished)
+{
+	unsigned int pair;
+	int ret;
+
+	for (pair = ETHTOOL_A_CABLE_PAIR_A; pair <= ETHTOOL_A_CABLE_PAIR_D; pair++) {
+		ret = rtl8224_cable_test_report_pair(phydev, pair);
+		if (ret == -EBUSY) {
+			*finished = false;
+			return 0;
+		}
+
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int rtl8224_cable_test_get_status(struct phy_device *phydev, bool *finished)
+{
+	int ret;
+
+	*finished = false;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+			   RTL822X_VND2_C22_REG(RTL8224_MII_RTCT));
+	if (ret < 0)
+		return ret;
+
+	if (!(ret & RTL8224_MII_RTCT_DONE))
+		return 0;
+
+	*finished = true;
+
+	return rtl8224_cable_test_report(phydev, finished);
+}
+
 static bool rtlgen_supports_2_5gbps(struct phy_device *phydev)
 {
 	int val;
@@ -1930,11 +2114,14 @@ static struct phy_driver realtek_drvs[] = {
 	}, {
 		PHY_ID_MATCH_EXACT(0x001ccad0),
 		.name		= "RTL8224 2.5Gbps PHY",
+		.flags		= PHY_POLL_CABLE_TEST,
 		.get_features   = rtl822x_c45_get_features,
 		.config_aneg    = rtl822x_c45_config_aneg,
 		.read_status    = rtl822x_c45_read_status,
 		.suspend        = genphy_c45_pma_suspend,
 		.resume         = rtlgen_c45_resume,
+		.cable_test_start = rtl8224_cable_test_start,
+		.cable_test_get_status = rtl8224_cable_test_get_status,
 	}, {
 		PHY_ID_MATCH_EXACT(0x001cc961),
 		.name		= "RTL8366RB Gigabit Ethernet",

---
base-commit: f0a24b2547cfdd5ec85a131e386a2ce4ff9179cb
change-id: 20251024-rtl8224-cable-test-c45eae2f6974

Best regards,
-- 
Sven Eckelmann <se@simonwunderlich.de>

Re: [PATCH] net: phy: realtek: Add RTL8224 cable testing support

[Andrew Lunn]

> +#define RTL8224_SRAM_RTCT_FAULT_BUSY		BIT(0)
> +#define RTL8224_SRAM_RTCT_FAULT_OPEN		BIT(3)
> +#define RTL8224_SRAM_RTCT_FAULT_SAME_SHORT	BIT(4)
> +#define RTL8224_SRAM_RTCT_FAULT_OK		BIT(5)
> +#define RTL8224_SRAM_RTCT_FAULT_DONE		BIT(6)
> +#define RTL8224_SRAM_RTCT_FAULT_CROSS_SHORT	BIT(7)

It is unusual these are bits. Does the datasheet say what happens if
the cable is both same short and cross short?

> +static int rtl8224_cable_test_result_trans(u32 result)
> +{
> +	if (result & RTL8224_SRAM_RTCT_FAULT_SAME_SHORT)
> +		return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
> +
> +	if (result & RTL8224_SRAM_RTCT_FAULT_BUSY)
> +		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
> +
> +	if (result & RTL8224_SRAM_RTCT_FAULT_CROSS_SHORT)
> +		return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT;

I don't remember seeing a PHY able to report both same short and cross
short at the same time. Maybe there has been, but there is no code for
it. We could add such a code.

	Andrew

Re: [PATCH] net: phy: realtek: Add RTL8224 cable testing support

[Sven Eckelmann]

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

On Monday, 27 October 2025 01:16:12 CET Andrew Lunn wrote:
> > +#define RTL8224_SRAM_RTCT_FAULT_BUSY		BIT(0)
> > +#define RTL8224_SRAM_RTCT_FAULT_OPEN		BIT(3)
> > +#define RTL8224_SRAM_RTCT_FAULT_SAME_SHORT	BIT(4)
> > +#define RTL8224_SRAM_RTCT_FAULT_OK		BIT(5)
> > +#define RTL8224_SRAM_RTCT_FAULT_DONE		BIT(6)
> > +#define RTL8224_SRAM_RTCT_FAULT_CROSS_SHORT	BIT(7)
> 
> It is unusual these are bits. Does the datasheet say what happens if
> the cable is both same short and cross short?

Unfortunately, the datasheet doesn't say anything about cable tests.

> 
> > +static int rtl8224_cable_test_result_trans(u32 result)
> > +{
> > +	if (result & RTL8224_SRAM_RTCT_FAULT_SAME_SHORT)
> > +		return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
> > +
> > +	if (result & RTL8224_SRAM_RTCT_FAULT_BUSY)
> > +		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
> > +
> > +	if (result & RTL8224_SRAM_RTCT_FAULT_CROSS_SHORT)
> > +		return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT;
> 
> I don't remember seeing a PHY able to report both same short and cross
> short at the same time. Maybe there has been, but there is no code for
> it. We could add such a code.

I've tried it a couple of times (with shorts at different lengths) but was not 
able to do this. For me, it looks like the RTL8224 can represent these faults 
in parallel but not actual detect them in parallel. I also played around with
open + shorts. At the end, I never saw any parallel detections and left it to 
the bit prioritization (during decoding) from RTLSDK.

RTL8226 in RTLSDK doesn't use (in contrast to RTL8224 in RTLSDK) the bits - 
but there seems to be at least some connections. The codes for RTL8226 
(according to RTLSDK) are:

* 0x60 RTL8226_SRAM_RTCT_FAULT_OK (would be RTL8224 OK+DONE)
* 0x48 RTL8226_SRAM_RTCT_FAULT_OPEN (would be RTL8224 OPEN+DONE)
* 0x50 RTL8226_SRAM_RTCT_FAULT_SHORT (would be RTL8224 SAME_SHORT + DONE)
* 0x42 RTL8226_SRAM_RTCT_FAULT_MISMATCH_OPEN (would be RTL8224 DONE + ????)
* 0x44 RTL8226_SRAM_RTCT_FAULT_MISMATCH_SHORT (would be RTL8224 DONE + ????)
* else: RTL8226_SRAM_RTCT_FAULT_UNKNOWN

It seems like these decoding would not even have cross shorts. Don't ask me 
what this mismatch is - couldn't find anything like this in RTL8224. The bits 
1 + 2 are completely unused there.

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] net: phy: realtek: Add RTL8224 cable testing support

[Andrew Lunn]

On Mon, Oct 27, 2025 at 08:49:36AM +0100, Sven Eckelmann wrote:
> On Monday, 27 October 2025 01:16:12 CET Andrew Lunn wrote:
> > > +#define RTL8224_SRAM_RTCT_FAULT_BUSY		BIT(0)
> > > +#define RTL8224_SRAM_RTCT_FAULT_OPEN		BIT(3)
> > > +#define RTL8224_SRAM_RTCT_FAULT_SAME_SHORT	BIT(4)
> > > +#define RTL8224_SRAM_RTCT_FAULT_OK		BIT(5)
> > > +#define RTL8224_SRAM_RTCT_FAULT_DONE		BIT(6)
> > > +#define RTL8224_SRAM_RTCT_FAULT_CROSS_SHORT	BIT(7)
> > 
> > It is unusual these are bits. Does the datasheet say what happens if
> > the cable is both same short and cross short?
> 
> Unfortunately, the datasheet doesn't say anything about cable tests.
> 
> > 
> > > +static int rtl8224_cable_test_result_trans(u32 result)
> > > +{
> > > +	if (result & RTL8224_SRAM_RTCT_FAULT_SAME_SHORT)
> > > +		return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
> > > +
> > > +	if (result & RTL8224_SRAM_RTCT_FAULT_BUSY)
> > > +		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
> > > +
> > > +	if (result & RTL8224_SRAM_RTCT_FAULT_CROSS_SHORT)
> > > +		return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT;
> > 
> > I don't remember seeing a PHY able to report both same short and cross
> > short at the same time. Maybe there has been, but there is no code for
> > it. We could add such a code.
> 
> I've tried it a couple of times (with shorts at different lengths) but was not 
> able to do this. For me, it looks like the RTL8224 can represent these faults 
> in parallel but not actual detect them in parallel.

O.K. Thanks for the explanation. The code is good as it is.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH] net: phy: realtek: Add RTL8224 cable testing support

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 24 Oct 2025 11:49:00 +0200 you wrote:
> From: Issam Hamdi <ih@simonwunderlich.de>
> 
> The RTL8224 can detect open pairs and short types (in same pair or some
> other pair). The distance to this problem can be estimated. This is done
> for each of the 4 pairs separately.
> 
> It is not meant to be run while there is an active link partner because
> this interferes with the active test pulses.
> 
> [...]

Here is the summary with links:
  - net: phy: realtek: Add RTL8224 cable testing support
    https://git.kernel.org/netdev/net-next/c/61958b33ef0b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html