[PATCH net-next] net: phy: motorcomm: Add cable test support for internal YT861x PHYs

[PATCH net-next] net: phy: motorcomm: Add cable test support for internal YT861x PHYs

[David Yang]

The YT921x switches integrate GbE PHYs of the YT861x family (PHY ID
0x01e04281) on the switch-internal MDIO bus exposed by the yt921x DSA
driver. Without a specific driver they bind to the generic PHY driver
and lack the cable diagnostic facility. Add a driver entry for them
with cable test support.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/net/phy/motorcomm.c | 119 +++++++++++++++++++++++++++++++++++-
 1 file changed, 117 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 90a4f86f2758..ef15fef304aa 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Motorcomm 8511/8521/8522/8531/8531S/8821 PHY driver.
+ * Motorcomm 8511/8521/8522/8531/8531S/861x/8821 PHY driver.
  *
  * Author: Peter Geis <pgwipeout@gmail.com>
  * Author: Frank <Frank.Sae@motor-comm.com>
@@ -8,6 +8,7 @@
 
 #include <linux/clk.h>
 #include <linux/etherdevice.h>
+#include <linux/ethtool_netlink.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/phy.h>
@@ -18,7 +19,9 @@
 #define PHY_ID_YT8522		0x4f51e928
 #define PHY_ID_YT8531		0x4f51e91b
 #define PHY_ID_YT8531S		0x4f51e91a
+#define PHY_ID_INT861X		0x01e04281
 #define PHY_ID_YT8821		0x4f51ea19
+
 /* YT8521/YT8531S/YT8821 Register Overview
  *	UTP Register space	|	FIBER Register space
  *  ------------------------------------------------------------
@@ -298,6 +301,19 @@
 #define YT8531_SCR_CLK_SRC_REF_25M		4
 #define YT8531_SCR_CLK_SRC_SSC_25M		5
 
+/* TDR (cable diagnostic) test control */
+#define YT861X_TDR_CTRL_REG			0x80
+#define YT861X_TDR_CTRL_START			BIT(0)
+
+#define YT861X_TDR_STATUS_REG			0x84
+#define YT861X_TDR_STATUS_BUSY			BIT(15)
+#define YT861X_TDR_STATUS_PAIR_OK		0
+#define YT861X_TDR_STATUS_PAIR_UNKNOWN		1
+#define YT861X_TDR_STATUS_PAIR_SHORT		2
+#define YT861X_TDR_STATUS_PAIR_OPEN		3
+
+#define YT861X_TDR_PAIR_LENGTH_REG(n)		(0x87 + (n))	/* in cm */
+
 #define YT8821_SDS_EXT_CSR_CTRL_REG			0x23
 #define YT8821_SDS_EXT_CSR_VCO_LDO_EN			BIT(15)
 #define YT8821_SDS_EXT_CSR_VCO_BIAS_LPF_EN		BIT(8)
@@ -2538,6 +2554,95 @@ static int yt8521_get_features(struct phy_device *phydev)
 	return ret;
 }
 
+/**
+ * yt861x_cable_test_start() - start a cable diagnostic (TDR) test
+ * @phydev: a pointer to a &struct phy_device
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt861x_cable_test_start(struct phy_device *phydev)
+{
+	int ret;
+
+	/* auto sleep would abort the TDR test */
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8521_EXTREG_SLEEP_CONTROL1_REG,
+					 YT8521_ESC1R_SLEEP_SW, 0);
+	if (ret)
+		return ret;
+
+	return ytphy_write_ext_with_lock(phydev, YT861X_TDR_CTRL_REG,
+					 YT861X_TDR_CTRL_START);
+}
+
+/**
+ * yt861x_cable_test_get_status() - report cable diagnostic test results
+ * @phydev: a pointer to a &struct phy_device
+ * @finished: set to true when the test is complete
+ *
+ * Returns: 0 or negative errno code
+ */
+static int yt861x_cable_test_get_status(struct phy_device *phydev,
+					bool *finished)
+{
+	int ret;
+
+	*finished = false;
+
+	ret = ytphy_read_ext_with_lock(phydev, YT861X_TDR_STATUS_REG);
+	if (ret < 0)
+		return ret;
+
+	if (ret & YT861X_TDR_STATUS_BUSY)
+		return 0;
+
+	for (int pair = ETHTOOL_A_CABLE_PAIR_A; pair <= ETHTOOL_A_CABLE_PAIR_D;
+	     pair++) {
+		u8 code;
+
+		switch ((ret >> (2 * pair)) & 0x3) {
+		case YT861X_TDR_STATUS_PAIR_OK:
+			code = ETHTOOL_A_CABLE_RESULT_CODE_OK;
+			break;
+		case YT861X_TDR_STATUS_PAIR_SHORT:
+			code = ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
+			break;
+		case YT861X_TDR_STATUS_PAIR_OPEN:
+			code = ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
+			break;
+		default:
+			code = ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+		}
+
+		ethnl_cable_test_result(phydev, pair, code);
+
+		if (code != ETHTOOL_A_CABLE_RESULT_CODE_OK &&
+		    code != ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC) {
+			ret = ytphy_read_ext_with_lock(phydev,
+						       YT861X_TDR_PAIR_LENGTH_REG(pair));
+			if (ret >= 0)
+				ethnl_cable_test_fault_length(phydev, pair,
+							      ret);
+		}
+	}
+
+	/* restore auto sleep and restart the PHY to resume the link */
+	ret = ytphy_modify_ext_with_lock(phydev,
+					 YT8521_EXTREG_SLEEP_CONTROL1_REG,
+					 YT8521_ESC1R_SLEEP_SW,
+					 YT8521_ESC1R_SLEEP_SW);
+	if (ret)
+		return ret;
+
+	ret = genphy_soft_reset(phydev);
+	if (ret)
+		return ret;
+
+	*finished = true;
+
+	return 0;
+}
+
 /**
  * yt8821_get_features - read mmd register to get 2.5G capability
  * @phydev: target phy_device struct
@@ -3173,6 +3278,15 @@ static struct phy_driver motorcomm_phy_drvs[] = {
 		.suspend	= yt8521_suspend,
 		.resume		= yt8521_resume,
 	},
+	{
+		PHY_ID_MATCH_EXACT(PHY_ID_INT861X),
+		.name			= "Internal YT861x Gigabit Ethernet",
+		.soft_reset		= genphy_soft_reset,
+		.suspend		= genphy_suspend,
+		.resume			= genphy_resume,
+		.cable_test_start	= yt861x_cable_test_start,
+		.cable_test_get_status	= yt861x_cable_test_get_status,
+	},
 	{
 		PHY_ID_MATCH_EXACT(PHY_ID_YT8821),
 		.name			= "YT8821 2.5Gbps PHY",
@@ -3194,7 +3308,7 @@ static struct phy_driver motorcomm_phy_drvs[] = {
 
 module_phy_driver(motorcomm_phy_drvs);
 
-MODULE_DESCRIPTION("Motorcomm 8511/8521/8531/8531S/8821 PHY driver");
+MODULE_DESCRIPTION("Motorcomm 8511/8521/8531/8531S/861x/8821 PHY driver");
 MODULE_AUTHOR("Peter Geis");
 MODULE_AUTHOR("Frank");
 MODULE_LICENSE("GPL");
@@ -3205,6 +3319,7 @@ static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8522) },
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531) },
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531S) },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_INT861X) },
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8821) },
 	{ /* sentinel */ }
 };
-- 
2.53.0

Re: [PATCH net-next] net: phy: motorcomm: Add cable test support for internal YT861x PHYs

[Andrew Lunn]

On Mon, Oct 05, 2026 at 02:42:43AM +0800, David Yang wrote:
> The YT921x switches integrate GbE PHYs of the YT861x family (PHY ID
> 0x01e04281) on the switch-internal MDIO bus exposed by the yt921x DSA
> driver. Without a specific driver they bind to the generic PHY driver
> and lack the cable diagnostic facility. Add a driver entry for them
> with cable test support.

Since this does two different things, it should be two patches.

Does the cable test code apply to other devices this driver drive?

> @@ -18,7 +19,9 @@
>  #define PHY_ID_YT8522		0x4f51e928
>  #define PHY_ID_YT8531		0x4f51e91b
>  #define PHY_ID_YT8531S		0x4f51e91a
> +#define PHY_ID_INT861X		0x01e04281

Is this the ID of the switch itself? It does not match the pattern.

>  #define PHY_ID_YT8821		0x4f51ea19

	Andrew

Re: [PATCH net-next] net: phy: motorcomm: Add cable test support for internal YT861x PHYs

[David Yang]

On Mon, Oct 5, 2026 at 4:17 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Oct 05, 2026 at 02:42:43AM +0800, David Yang wrote:
> > The YT921x switches integrate GbE PHYs of the YT861x family (PHY ID
> > 0x01e04281) on the switch-internal MDIO bus exposed by the yt921x DSA
> > driver. Without a specific driver they bind to the generic PHY driver
> > and lack the cable diagnostic facility. Add a driver entry for them
> > with cable test support.
>
> Since this does two different things, it should be two patches.
>
> Does the cable test code apply to other devices this driver drive?

I don't have other devices to test with.

> > @@ -18,7 +19,9 @@
> >  #define PHY_ID_YT8522                0x4f51e928
> >  #define PHY_ID_YT8531                0x4f51e91b
> >  #define PHY_ID_YT8531S               0x4f51e91a
> > +#define PHY_ID_INT861X               0x01e04281
>
> Is this the ID of the switch itself? It does not match the pattern.

It's live captured from the system. By the way the serdes PCS does
show something matching the pattern 0x4f51e899.

> >  #define PHY_ID_YT8821                0x4f51ea19
>
>         Andrew

Re: [PATCH net-next] net: phy: motorcomm: Add cable test support for internal YT861x PHYs

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] yt861x_cable_test_get_status() in drivers/net/phy/motorcomm.c
  uses one variable, `ret`, for two different things.
- [Medium] yt861x_cable_test_start() clears YT8521_ESC1R_SLEEP_SW (auto-
  sleep) in ext register 0x27 at lines 2568-2570.
- [Low] The MOTORCOMM_PHY Kconfig help in drivers/net/phy/Kconfig still
  says "Currently supports YT85xx Gigabit Ethernet PHYs." The patch adds…

--

net: phy: motorcomm: Add cable test support for internal YT861x PHYs

This patch adds a driver entry for the YT861x family PHYs (PHY ID
0x01e04281) built into YT921x switches. Before this, they bound to the
generic PHY driver. The new entry adds TDR cable test start and status
callbacks.

> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> index 90a4f86f2758b..ef15fef304aa9 100644
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c

[ ... ]

> @@ -2538,6 +2554,95 @@ static int yt8521_get_features(struct phy_device *phydev)
>  	return ret;
>  }
>  
> +/**
> + * yt861x_cable_test_start() - start a cable diagnostic (TDR) test
> + * @phydev: a pointer to a &struct phy_device
> + *
> + * Returns: 0 or negative errno code
> + */
> +static int yt861x_cable_test_start(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	/* auto sleep would abort the TDR test */
> +	ret = ytphy_modify_ext_with_lock(phydev,
> +					 YT8521_EXTREG_SLEEP_CONTROL1_REG,
> +					 YT8521_ESC1R_SLEEP_SW, 0);
> +	if (ret)
> +		return ret;
> +
> +	return ytphy_write_ext_with_lock(phydev, YT861X_TDR_CTRL_REG,
> +					 YT861X_TDR_CTRL_START);
> +}

[Severity: Medium]
Can auto-sleep stay disabled if the test does not finish normally?

yt861x_cable_test_start() clears YT8521_ESC1R_SLEEP_SW. The only code
that sets it again is the success path at the end of
yt861x_cable_test_get_status(). These exits skip that restore:

- the YT861X_TDR_CTRL_REG write above fails, and the error is returned
  without restoring the sleep bit
- the YT861X_TDR_STATUS_REG read in yt861x_cable_test_get_status() fails,
  so the state machine calls phy_abort_cable_test()
- phy_stop() aborts the test, for example on "ip link set down" while a
  test is running
- the restoring ytphy_modify_ext_with_lock() call itself fails

The abort path is:

phy_stop()
  phy_abort_cable_test()
    ethnl_cable_test_finished()
    phy_init_hw()
      genphy_soft_reset()

The new driver entry has no config_init, so phy_init_hw() never writes
YT8521_EXTREG_SLEEP_CONTROL1_REG again. The success path sets SLEEP_SW
itself before calling genphy_soft_reset(). That suggests a BMCR reset does
not restore this ext register.

The restore also always writes SLEEP_SW=1 instead of the value that was
set before the test.

Could the previous value be saved and restored on every exit path? Or
could the bit be set again from a config_init callback?

> +
> +/**
> + * yt861x_cable_test_get_status() - report cable diagnostic test results
> + * @phydev: a pointer to a &struct phy_device
> + * @finished: set to true when the test is complete
> + *
> + * Returns: 0 or negative errno code
> + */
> +static int yt861x_cable_test_get_status(struct phy_device *phydev,
> +					bool *finished)
> +{
> +	int ret;
> +
> +	*finished = false;
> +
> +	ret = ytphy_read_ext_with_lock(phydev, YT861X_TDR_STATUS_REG);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret & YT861X_TDR_STATUS_BUSY)
> +		return 0;
> +
> +	for (int pair = ETHTOOL_A_CABLE_PAIR_A; pair <= ETHTOOL_A_CABLE_PAIR_D;
> +	     pair++) {
> +		u8 code;
> +
> +		switch ((ret >> (2 * pair)) & 0x3) {

[ ... ]

> +		ethnl_cable_test_result(phydev, pair, code);
> +
> +		if (code != ETHTOOL_A_CABLE_RESULT_CODE_OK &&
> +		    code != ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC) {
> +			ret = ytphy_read_ext_with_lock(phydev,
> +						       YT861X_TDR_PAIR_LENGTH_REG(pair));

[Severity: High]
Does this overwrite the status word in ret while the switch above still
needs it for the remaining pairs?

If a pair is SHORT or OPEN, ret is replaced by that pair's fault length in
cm, or by a negative errno if the read fails. The next pairs are then
decoded from bits of that value, not from YT861X_TDR_STATUS_REG.

Take an unplugged cable, where the status is 0x00ff (all pairs open).
Pair A is reported OPEN. If each pair's length reads back as 300 (0x12c),
pair B decodes as OPEN, pair C as SHORT and pair D as OK.

If the length read returns -EIO, later pairs decode as SHORT or OPEN from
the errno bits. Length registers are then read for faults the hardware
never reported.

Should the status word be kept in a separate variable from the length
read?

> +			if (ret >= 0)
> +				ethnl_cable_test_fault_length(phydev, pair,
> +							      ret);
> +		}
> +	}

[ ... ]

> @@ -3194,7 +3308,7 @@ static struct phy_driver motorcomm_phy_drvs[] = {
>  
>  module_phy_driver(motorcomm_phy_drvs);
>  
> -MODULE_DESCRIPTION("Motorcomm 8511/8521/8531/8531S/8821 PHY driver");
> +MODULE_DESCRIPTION("Motorcomm 8511/8521/8531/8531S/861x/8821 PHY driver");

[Severity: Low]
This isn't a bug, but the MOTORCOMM_PHY help text in
drivers/net/phy/Kconfig still says:

	  Currently supports YT85xx Gigabit Ethernet PHYs.

The header comment and MODULE_DESCRIPTION now mention 861x. Should the
Kconfig help be updated to match? It was already missing YT8821 before
this patch.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261004184246.1260426-1-mmyangfl%40gmail.com