[PATCH net-next v1 1/1] net: phy: dp83tg720: Add cable testing support

[PATCH net-next v1 1/1] net: phy: dp83tg720: Add cable testing support

[Oleksij Rempel]

Introduce cable testing support for the DP83TG720 PHY. This implementation
is based on the "DP83TG720S-Q1: Configuring for Open Alliance Specification
Compliance (Rev. B)" application note.

The feature has been tested with cables of various lengths:
- No cable: 1m till open reported.
- 5 meter cable: reported properly.
- 20 meter cable: reported as 19m.
- 40 meter cable: reported as cable ok.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/dp83tg720.c | 188 ++++++++++++++++++++++++++++++++++++
 1 file changed, 188 insertions(+)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index c706429b225a2..4c0982825bbd8 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2023 Pengutronix, Oleksij Rempel <kernel@pengutronix.de>
  */
 #include <linux/bitfield.h>
+#include <linux/ethtool_netlink.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/phy.h>
@@ -14,6 +15,17 @@
 #define DP83TG720S_STS_MII_INT			BIT(7)
 #define DP83TG720S_LINK_STATUS			BIT(0)
 
+/* TDR Configuration Register (0x1E) */
+#define DP83TG720S_TDR_CFG			0x1e
+/* 1b = TDR start, 0b = No TDR */
+#define DP83TG720S_TDR_START			BIT(15)
+/* 1b = TDR auto on link down, 0b = Manual TDR start */
+#define DP83TG720S_CFG_TDR_AUTO_RUN		BIT(14)
+/* 1b = TDR done, 0b = TDR in progress */
+#define DP83TG720S_TDR_DONE			BIT(1)
+/* 1b = TDR fail, 0b = TDR success */
+#define DP83TG720S_TDR_FAIL			BIT(0)
+
 #define DP83TG720S_PHY_RESET			0x1f
 #define DP83TG720S_HW_RESET			BIT(15)
 
@@ -22,18 +34,192 @@
 /* Power Mode 0 is Normal mode */
 #define DP83TG720S_LPS_CFG3_PWR_MODE_0		BIT(0)
 
+/* TDR Fault Status Register (0x030F) */
+#define DP83TG720S_TDR_FAULT_STATUS		0x30f
+/* TDR fault distance */
+#define DP83TG720S_TDR_FAULT_DISTANCE_MASK	GENMASK(13, 8)
+/* TDR fault type */
+#define DP83TG720S_TDR_FAULT_TYPE_MASK		GENMASK(7, 4)
+#define DP83TG720S_TDR_FAULT_TYPE_SHORT		0x3 /* 0011 = Short */
+#define DP83TG720S_TDR_FAULT_TYPE_OPEN		0x6 /* 0110 = Open */
+#define DP83TG720S_TDR_FAULT_TYPE_NOISE		0x5 /* 0101 = Noise */
+#define DP83TG720S_TDR_FAULT_TYPE_OK		0x7 /* 0111 = Cable OK */
+#define DP83TG720S_TDR_FAULT_TYPE_PROGRESS	0x8 /* 1000 = Test in progress */
+#define DP83TG720S_TDR_FAULT_TYPE_NP		0xD /* 1101 = Test not possible */
+/* TDR readiness and state */
+#define DP83TG720S_TDR_STATE_MASK		GENMASK(1, 0)
+#define DP83TG720S_TDR_STATE_READY		0x1 /* 01 = TDR ready for activation */
+#define DP83TG720S_TDR_STATE_ON			0x2 /* 10 = TDR On */
+
+/* Register 0x0301: TDR Configuration 2 */
+#define DP83TG720S_TDR_CFG2			0x301
+
+/* Register 0x0303: TDR Configuration 3 */
+#define DP83TG720S_TDR_CFG3			0x303
+
+/* Register 0x0304: TDR Configuration 4 */
+#define DP83TG720S_TDR_CFG4			0x304
+
+/* Register 0x0405: Unknown Register */
+#define DP83TG720S_UNKNOWN_0405			0x405
+
+/* Register 0x0576: TDR Master Link Down Control */
+#define DP83TG720S_TDR_MASTER_LINK_DOWN		0x576
+
 #define DP83TG720S_RGMII_DELAY_CTRL		0x602
 /* In RGMII mode, Enable or disable the internal delay for RXD */
 #define DP83TG720S_RGMII_RX_CLK_SEL		BIT(1)
 /* In RGMII mode, Enable or disable the internal delay for TXD */
 #define DP83TG720S_RGMII_TX_CLK_SEL		BIT(0)
 
+/* Register 0x083F: Unknown Register */
+#define DP83TG720S_UNKNOWN_083F			0x83f
+
 #define DP83TG720S_SQI_REG_1			0x871
 #define DP83TG720S_SQI_OUT_WORST		GENMASK(7, 5)
 #define DP83TG720S_SQI_OUT			GENMASK(3, 1)
 
 #define DP83TG720_SQI_MAX			7
 
+/**
+ * dp83tg720_get_fault_type - Convert TDR fault type to ethtool result code.
+ * @fault_type: Fault type value from the PHY.
+ *
+ * Returns: Corresponding ethtool result code.
+ */
+static int dp83tg720_get_fault_type(int fault_type)
+{
+	switch (fault_type) {
+	case DP83TG720S_TDR_FAULT_TYPE_SHORT:
+		return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
+	case DP83TG720S_TDR_FAULT_TYPE_OPEN:
+		return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
+	case DP83TG720S_TDR_FAULT_TYPE_NOISE:
+		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+	case DP83TG720S_TDR_FAULT_TYPE_OK:
+	default:
+		return ETHTOOL_A_CABLE_RESULT_CODE_OK;
+	}
+}
+
+/**
+ * dp83tg720_cable_test_start - Start the cable test for the DP83TG720 PHY.
+ * @phydev: Pointer to the phy_device structure.
+ *
+ * This sequence is based on the documented procedure for the DP83TG720 PHY.
+ *
+ * Returns: 0 on success, a negative error code on failure.
+ */
+static int dp83tg720_cable_test_start(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Initialize the PHY to run the TDR test as described in the
+	 * "DP83TG720S-Q1: Configuring for Open Alliance Specification
+	 * Compliance (Rev. B)" application note.
+	 * Most of the registers are not documented. Some of register names
+	 * are guessed by comparing the register offsets with the DP83TD510E.
+	 */
+
+	/* Force master link down */
+	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2,
+			       DP83TG720S_TDR_MASTER_LINK_DOWN, 0x0400);
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG2,
+			    0xa008);
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG3,
+			    0x0928);
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG4,
+			    0x0004);
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_UNKNOWN_0405,
+			    0x6400);
+	if (ret)
+		return ret;
+
+	ret = phy_write_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_UNKNOWN_083F,
+			    0x3003);
+	if (ret)
+		return ret;
+
+	/* Start the TDR */
+	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG,
+			       DP83TG720S_TDR_START);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/**
+ * dp83tg720_cable_test_get_status - Get the status of the cable test for the
+ *                                   DP83TG720 PHY.
+ * @phydev: Pointer to the phy_device structure.
+ * @finished: Pointer to a boolean that indicates whether the test is finished.
+ *
+ * The function sets the @finished flag to true if the test is complete.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+static int dp83tg720_cable_test_get_status(struct phy_device *phydev,
+					   bool *finished)
+{
+	int ret, stat;
+
+	*finished = false;
+
+	/* Read the TDR status */
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_TDR_CFG);
+	if (ret < 0)
+		return ret;
+
+	/* Check if the TDR test is done */
+	if (!(ret & DP83TG720S_TDR_DONE))
+		return 0;
+
+	/* Check for TDR test failure */
+	if (!(ret & DP83TG720S_TDR_FAIL)) {
+		int fault_type;
+		int location;
+
+		/* Read fault status */
+		ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+				   DP83TG720S_TDR_FAULT_STATUS);
+		if (ret < 0)
+			return ret;
+
+		/* Get fault type */
+		fault_type = FIELD_GET(DP83TG720S_TDR_FAULT_TYPE_MASK, ret);
+		stat = dp83tg720_get_fault_type(fault_type);
+
+		/* Determine fault location */
+		location = FIELD_GET(DP83TG720S_TDR_FAULT_DISTANCE_MASK,
+				     ret) * 100;
+		ethnl_cable_test_fault_length(phydev, ETHTOOL_A_CABLE_PAIR_A,
+					      location);
+	} else {
+		/* Active link partner or other issues */
+		stat = ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+	}
+
+	*finished = true;
+
+	ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A, stat);
+
+	return phy_init_hw(phydev);
+}
+
+
 static int dp83tg720_config_aneg(struct phy_device *phydev)
 {
 	int ret;
@@ -201,6 +387,8 @@ static struct phy_driver dp83tg720_driver[] = {
 	.config_init	= dp83tg720_config_init,
 	.get_sqi	= dp83tg720_get_sqi,
 	.get_sqi_max	= dp83tg720_get_sqi_max,
+	.cable_test_start = dp83tg720_cable_test_start,
+	.cable_test_get_status = dp83tg720_cable_test_get_status,
 
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
-- 
2.39.2

Re: [PATCH net-next v1 1/1] net: phy: dp83tg720: Add cable testing support

[Oleksij Rempel]

On Wed, Aug 07, 2024 at 11:32:51AM +0200, Oleksij Rempel wrote:
> Introduce cable testing support for the DP83TG720 PHY. This implementation
> is based on the "DP83TG720S-Q1: Configuring for Open Alliance Specification
> Compliance (Rev. B)" application note.
> 
> The feature has been tested with cables of various lengths:
> - No cable: 1m till open reported.
> - 5 meter cable: reported properly.
> - 20 meter cable: reported as 19m.
> - 40 meter cable: reported as cable ok.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
 
There is missing PHY_POLL_CABLE_TEST, i'll send v2 soon.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 1/1] net: phy: dp83tg720: Add cable testing support

[Andrew Lunn]

On Wed, Aug 07, 2024 at 11:32:51AM +0200, Oleksij Rempel wrote:
> Introduce cable testing support for the DP83TG720 PHY. This implementation
> is based on the "DP83TG720S-Q1: Configuring for Open Alliance Specification
> Compliance (Rev. B)" application note.

Does the OA specification describe all these registers? Should we
expect other devices to be identical?

> +/**
> + * dp83tg720_get_fault_type - Convert TDR fault type to ethtool result code.
> + * @fault_type: Fault type value from the PHY.
> + *
> + * Returns: Corresponding ethtool result code.
> + */
> +static int dp83tg720_get_fault_type(int fault_type)
> +{
> +	switch (fault_type) {
> +	case DP83TG720S_TDR_FAULT_TYPE_SHORT:
> +		return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
> +	case DP83TG720S_TDR_FAULT_TYPE_OPEN:
> +		return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
> +	case DP83TG720S_TDR_FAULT_TYPE_NOISE:
> +		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;

You could add a new type for this. I think some implementations will
also re-try a few times to see if a quiet period can be found.

	Andrew

Re: [PATCH net-next v1 1/1] net: phy: dp83tg720: Add cable testing support

[Oleksij Rempel]

On Wed, Aug 07, 2024 at 03:24:52PM +0200, Andrew Lunn wrote:
> On Wed, Aug 07, 2024 at 11:32:51AM +0200, Oleksij Rempel wrote:
> > Introduce cable testing support for the DP83TG720 PHY. This implementation
> > is based on the "DP83TG720S-Q1: Configuring for Open Alliance Specification
> > Compliance (Rev. B)" application note.
> 
> Does the OA specification describe all these registers?

Not all of them.

OA specification provide recommendation for the bit layout for the TDR
status register (HDD.TDR):
https://opensig.org/wp-content/uploads/2024/03/Advanced_PHY_features_for_automotive_Ethernet_v2.0_fin.pdf

> Should we expect other devices to be identical?

Yes. I'll create a new file for OA specific helpers.

> > +/**
> > + * dp83tg720_get_fault_type - Convert TDR fault type to ethtool result code.
> > + * @fault_type: Fault type value from the PHY.
> > + *
> > + * Returns: Corresponding ethtool result code.
> > + */
> > +static int dp83tg720_get_fault_type(int fault_type)
> > +{
> > +	switch (fault_type) {
> > +	case DP83TG720S_TDR_FAULT_TYPE_SHORT:
> > +		return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
> > +	case DP83TG720S_TDR_FAULT_TYPE_OPEN:
> > +		return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
> > +	case DP83TG720S_TDR_FAULT_TYPE_NOISE:
> > +		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
> 
> You could add a new type for this. I think some implementations will
> also re-try a few times to see if a quiet period can be found.

ack.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |