[PATCH net-next v2 1/3] ethtool: Add new result codes for TDR diagnostics

[PATCH net-next v2 1/3] ethtool: Add new result codes for TDR diagnostics

[Oleksij Rempel]

Add new result codes to support TDR diagnostics in preparation for
Open Alliance 1000BaseT1 TDR support:

- ETHTOOL_A_CABLE_RESULT_CODE_NOISE: TDR not possible due to high noise
  level.
- ETHTOOL_A_CABLE_RESULT_CODE_RESOLUTION_NOT_POSSIBLE: TDR resolution not
  possible / out of distance.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/uapi/linux/ethtool_netlink.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 6d5bdcc67631a..fc814c8a5ac47 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -556,6 +556,10 @@ enum {
 	 * a regular 100 Ohm cable and a part with the abnormal impedance value
 	 */
 	ETHTOOL_A_CABLE_RESULT_CODE_IMPEDANCE_MISMATCH,
+	/* TDR not possible due to high noise level */
+	ETHTOOL_A_CABLE_RESULT_CODE_NOISE,
+	/* TDR resolution not possible / out of distance */
+	ETHTOOL_A_CABLE_RESULT_CODE_RESOLUTION_NOT_POSSIBLE,
 };
 
 enum {
-- 
2.39.2

[PATCH net-next v2 2/3] phy: Add Open Alliance helpers for the PHY framework

[Oleksij Rempel]

Introduce helper functions specific to Open Alliance diagnostics,
integrating them into the PHY framework. Currently, these helpers
are limited to 1000BaseT1 specific TDR functionality.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/Makefile                |  2 +-
 drivers/net/phy/open_alliance_helpers.c | 70 +++++++++++++++++++++++++
 include/linux/open_alliance_helpers.h   | 47 +++++++++++++++++
 3 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/phy/open_alliance_helpers.c
 create mode 100644 include/linux/open_alliance_helpers.h

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 202ed7f450da6..8a46a04af01a5 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,7 +2,7 @@
 # Makefile for Linux PHY drivers
 
 libphy-y			:= phy.o phy-c45.o phy-core.o phy_device.o \
-				   linkmode.o
+				   linkmode.o open_alliance_helpers.o
 mdio-bus-y			+= mdio_bus.o mdio_device.o
 
 ifdef CONFIG_MDIO_DEVICE
diff --git a/drivers/net/phy/open_alliance_helpers.c b/drivers/net/phy/open_alliance_helpers.c
new file mode 100644
index 0000000000000..eac1004c065ae
--- /dev/null
+++ b/drivers/net/phy/open_alliance_helpers.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * open_alliance_helpers.c - OPEN Alliance specific PHY diagnostic helpers
+ *
+ * This file contains helper functions for implementing advanced diagnostic
+ * features as specified by the OPEN Alliance for automotive Ethernet PHYs.
+ * These helpers include functionality for Time Delay Reflection (TDR), dynamic
+ * channel quality assessment, and other PHY diagnostics.
+ *
+ * For more information on the specifications, refer to the OPEN Alliance
+ * documentation: https://opensig.org/automotive-ethernet-specifications/
+ */
+
+#include <linux/ethtool_netlink.h>
+#include <linux/open_alliance_helpers.h>
+
+/**
+ * oa_1000bt1_get_ethtool_cable_result_code - Convert TDR status to ethtool
+ *					      result code
+ * @reg_value: Value read from the TDR register
+ *
+ * This function takes a register value from the HDD.TDR register and converts
+ * the TDR status to the corresponding ethtool cable test result code.
+ *
+ * Return: The appropriate ethtool result code based on the TDR status
+ */
+int oa_1000bt1_get_ethtool_cable_result_code(u16 reg_value)
+{
+	u8 tdr_status = (reg_value & OA_1000BT1_HDD_TDR_STATUS_MASK) >> 4;
+	u8 dist_val = (reg_value & OA_1000BT1_HDD_TDR_DISTANCE_MASK) >> 8;
+
+	switch (tdr_status) {
+	case OA_1000BT1_HDD_TDR_STATUS_CABLE_OK:
+		return ETHTOOL_A_CABLE_RESULT_CODE_OK;
+	case OA_1000BT1_HDD_TDR_STATUS_OPEN:
+		return ETHTOOL_A_CABLE_RESULT_CODE_OPEN;
+	case OA_1000BT1_HDD_TDR_STATUS_SHORT:
+		return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT;
+	case OA_1000BT1_HDD_TDR_STATUS_NOISE:
+		return ETHTOOL_A_CABLE_RESULT_CODE_NOISE;
+	default:
+		if (dist_val == OA_1000BT1_HDD_TDR_DISTANCE_RESOLUTION_NOT_POSSIBLE)
+			return ETHTOOL_A_CABLE_RESULT_CODE_RESOLUTION_NOT_POSSIBLE;
+		return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC;
+	}
+}
+
+/**
+ * oa_1000bt1_get_tdr_distance - Get distance to the main fault from TDR
+ *				 register value
+ * @reg_value: Value read from the TDR register
+ *
+ * This function takes a register value from the HDD.TDR register and extracts
+ * the distance to the main fault detected by the TDR feature. The distance is
+ * measured in centimeters and ranges from 0 to 3100 centimeters. If the
+ * distance is not available (0x3f), the function returns -ERANGE.
+ *
+ * Return: The distance to the main fault in centimeters, or -ERANGE if the
+ * resolution is not possible.
+ */
+int oa_1000bt1_get_tdr_distance(u16 reg_value)
+{
+	u8 dist_val = (reg_value & OA_1000BT1_HDD_TDR_DISTANCE_MASK) >> 8;
+
+	if (dist_val == OA_1000BT1_HDD_TDR_DISTANCE_RESOLUTION_NOT_POSSIBLE)
+		return -ERANGE;
+
+	return dist_val * 100;
+}
+
diff --git a/include/linux/open_alliance_helpers.h b/include/linux/open_alliance_helpers.h
new file mode 100644
index 0000000000000..8b7d97bc6f186
--- /dev/null
+++ b/include/linux/open_alliance_helpers.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef OPEN_ALLIANCE_HELPERS_H
+#define OPEN_ALLIANCE_HELPERS_H
+
+/*
+ * These defines reflect the TDR (Time Delay Reflection) diagnostic feature
+ * for 1000BASE-T1 automotive Ethernet PHYs as specified by the OPEN Alliance.
+ *
+ * The register values are part of the HDD.TDR register, which provides
+ * information about the cable status and faults. The exact register offset
+ * is device-specific and should be provided by the driver.
+ */
+#define OA_1000BT1_HDD_TDR_ACTIVATION_MASK		GENMASK(1, 0)
+#define OA_1000BT1_HDD_TDR_ACTIVATION_OFF		1
+#define OA_1000BT1_HDD_TDR_ACTIVATION_ON		2
+
+#define OA_1000BT1_HDD_TDR_STATUS_MASK			GENMASK(7, 4)
+#define OA_1000BT1_HDD_TDR_STATUS_SHORT			3
+#define OA_1000BT1_HDD_TDR_STATUS_OPEN			6
+#define OA_1000BT1_HDD_TDR_STATUS_NOISE			5
+#define OA_1000BT1_HDD_TDR_STATUS_CABLE_OK		7
+#define OA_1000BT1_HDD_TDR_STATUS_TEST_IN_PROGRESS	8
+#define OA_1000BT1_HDD_TDR_STATUS_TEST_NOT_POSSIBLE	13
+
+/*
+ * OA_1000BT1_HDD_TDR_DISTANCE_MASK:
+ * This mask is used to extract the distance to the first/main fault
+ * detected by the TDR feature. Each bit represents an approximate distance
+ * of 1 meter, ranging from 0 to 31 meters. The exact interpretation of the
+ * bits may vary, but generally:
+ * 000000 = no error
+ * 000001 = error about 0-1m away
+ * 000010 = error between 1-2m away
+ * ...
+ * 011111 = error about 30-31m away
+ * 111111 = resolution not possible / out of distance
+ */
+#define OA_1000BT1_HDD_TDR_DISTANCE_MASK			GENMASK(13, 8)
+#define OA_1000BT1_HDD_TDR_DISTANCE_NO_ERROR			0
+#define OA_1000BT1_HDD_TDR_DISTANCE_RESOLUTION_NOT_POSSIBLE	0x3f
+
+int oa_1000bt1_get_ethtool_cable_result_code(u16 reg_value);
+int oa_1000bt1_get_tdr_distance(u16 reg_value);
+
+#endif /* OPEN_ALLIANCE_HELPERS_H */
+
-- 
2.39.2

[PATCH net-next v2 3/3] 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>
---
changes v2:
- use open alliance specific helpers for the TDR results

 drivers/net/phy/dp83tg720.c | 153 ++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/drivers/net/phy/dp83tg720.c b/drivers/net/phy/dp83tg720.c
index c706429b225a2..06542fce83c95 100644
--- a/drivers/net/phy/dp83tg720.c
+++ b/drivers/net/phy/dp83tg720.c
@@ -3,8 +3,10 @@
  * 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/open_alliance_helpers.h>
 #include <linux/phy.h>

 #define DP83TG720S_PHY_ID			0x2000a284
@@ -14,6 +16,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 +35,155 @@
 /* Power Mode 0 is Normal mode */
 #define DP83TG720S_LPS_CFG3_PWR_MODE_0		BIT(0)

+/* Open Aliance 1000BaseT1 compatible HDD.TDR Fault Status Register */
+#define DP83TG720S_TDR_FAULT_STATUS		0x30f
+
+/* 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_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 location;
+
+		/* Read fault status */
+		ret = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+				   DP83TG720S_TDR_FAULT_STATUS);
+		if (ret < 0)
+			return ret;
+
+		/* Get fault type */
+		stat = oa_1000bt1_get_ethtool_cable_result_code(ret);
+
+		/* Determine fault location */
+		location = oa_1000bt1_get_tdr_distance(ret);
+		if (location > 0)
+			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;
@@ -195,12 +345,15 @@ static struct phy_driver dp83tg720_driver[] = {
 	PHY_ID_MATCH_MODEL(DP83TG720S_PHY_ID),
 	.name		= "TI DP83TG720S",

+	.flags          = PHY_POLL_CABLE_TEST,
 	.config_aneg	= dp83tg720_config_aneg,
 	.read_status	= dp83tg720_read_status,
 	.get_features	= genphy_c45_pma_read_ext_abilities,
 	.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 v2 2/3] phy: Add Open Alliance helpers for the PHY framework

[Marc Kleine-Budde]

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

On 08.08.2024 15:08:32, Oleksij Rempel wrote:
> Introduce helper functions specific to Open Alliance diagnostics,
> integrating them into the PHY framework. Currently, these helpers
> are limited to 1000BaseT1 specific TDR functionality.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/Makefile                |  2 +-
>  drivers/net/phy/open_alliance_helpers.c | 70 +++++++++++++++++++++++++
>  include/linux/open_alliance_helpers.h   | 47 +++++++++++++++++
>  3 files changed, 118 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/phy/open_alliance_helpers.c
>  create mode 100644 include/linux/open_alliance_helpers.h
> 
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 202ed7f450da6..8a46a04af01a5 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -2,7 +2,7 @@
>  # Makefile for Linux PHY drivers
>  
>  libphy-y			:= phy.o phy-c45.o phy-core.o phy_device.o \
> -				   linkmode.o
> +				   linkmode.o open_alliance_helpers.o
>  mdio-bus-y			+= mdio_bus.o mdio_device.o
>  
>  ifdef CONFIG_MDIO_DEVICE
> diff --git a/drivers/net/phy/open_alliance_helpers.c b/drivers/net/phy/open_alliance_helpers.c
> new file mode 100644
> index 0000000000000..eac1004c065ae
> --- /dev/null
> +++ b/drivers/net/phy/open_alliance_helpers.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * open_alliance_helpers.c - OPEN Alliance specific PHY diagnostic helpers
> + *
> + * This file contains helper functions for implementing advanced diagnostic
> + * features as specified by the OPEN Alliance for automotive Ethernet PHYs.
> + * These helpers include functionality for Time Delay Reflection (TDR), dynamic
> + * channel quality assessment, and other PHY diagnostics.
> + *
> + * For more information on the specifications, refer to the OPEN Alliance
> + * documentation: https://opensig.org/automotive-ethernet-specifications/
> + */
> +
> +#include <linux/ethtool_netlink.h>
> +#include <linux/open_alliance_helpers.h>
> +
> +/**
> + * oa_1000bt1_get_ethtool_cable_result_code - Convert TDR status to ethtool
> + *					      result code
> + * @reg_value: Value read from the TDR register
> + *
> + * This function takes a register value from the HDD.TDR register and converts
> + * the TDR status to the corresponding ethtool cable test result code.
> + *
> + * Return: The appropriate ethtool result code based on the TDR status
> + */
> +int oa_1000bt1_get_ethtool_cable_result_code(u16 reg_value)
> +{
> +	u8 tdr_status = (reg_value & OA_1000BT1_HDD_TDR_STATUS_MASK) >> 4;
> +	u8 dist_val = (reg_value & OA_1000BT1_HDD_TDR_DISTANCE_MASK) >> 8;

can you make use of FIELD_GET() here and in the other functions?

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH net-next v2 2/3] phy: Add Open Alliance helpers for the PHY framework

[Russell King (Oracle)]

On Thu, Aug 08, 2024 at 03:08:32PM +0200, Oleksij Rempel wrote:
> Introduce helper functions specific to Open Alliance diagnostics,
> integrating them into the PHY framework. Currently, these helpers
> are limited to 1000BaseT1 specific TDR functionality.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/Makefile                |  2 +-
>  drivers/net/phy/open_alliance_helpers.c | 70 +++++++++++++++++++++++++
>  include/linux/open_alliance_helpers.h   | 47 +++++++++++++++++
>  3 files changed, 118 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/phy/open_alliance_helpers.c
>  create mode 100644 include/linux/open_alliance_helpers.h
> 
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 202ed7f450da6..8a46a04af01a5 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -2,7 +2,7 @@
>  # Makefile for Linux PHY drivers
>  
>  libphy-y			:= phy.o phy-c45.o phy-core.o phy_device.o \
> -				   linkmode.o
> +				   linkmode.o open_alliance_helpers.o

Given that most PHY drivers aren't open alliance, would it be better
to have a config symbol that can be selected by PHY drivers that need
it?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/3] phy: Add Open Alliance helpers for the PHY framework

[Andrew Lunn]

On Thu, Aug 08, 2024 at 03:08:32PM +0200, Oleksij Rempel wrote:
> Introduce helper functions specific to Open Alliance diagnostics,
> integrating them into the PHY framework. Currently, these helpers
> are limited to 1000BaseT1 specific TDR functionality.

Thanks for generalising this code.

> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/Makefile                |  2 +-
>  drivers/net/phy/open_alliance_helpers.c | 70 +++++++++++++++++++++++++
>  include/linux/open_alliance_helpers.h   | 47 +++++++++++++++++

We don't have any PHY drivers outside of drivers/net/phy, and i don't
see any reason why we should allow them anywhere else. So please
consider moving the header file into that directory, rather than
having it global.

>  ifdef CONFIG_MDIO_DEVICE
> diff --git a/drivers/net/phy/open_alliance_helpers.c b/drivers/net/phy/open_alliance_helpers.c
> new file mode 100644
> index 0000000000000..eac1004c065ae
> --- /dev/null
> +++ b/drivers/net/phy/open_alliance_helpers.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * open_alliance_helpers.c - OPEN Alliance specific PHY diagnostic helpers
> + *
> + * This file contains helper functions for implementing advanced diagnostic
> + * features as specified by the OPEN Alliance for automotive Ethernet PHYs.
> + * These helpers include functionality for Time Delay Reflection (TDR), dynamic
> + * channel quality assessment, and other PHY diagnostics.
> + *
> + * For more information on the specifications, refer to the OPEN Alliance
> + * documentation: https://opensig.org/automotive-ethernet-specifications/

Please could you give a reference to the exact standard. I think this
is "Advanced diagnostic features for 1000BASE-T1 automotive Ethernet
PHYs TC12 - advanced PHY features" ?

The standard seem open, so you could include a URL:

https://opensig.org/wp-content/uploads/2024/03/Advanced_PHY_features_for_automotive_Ethernet_v2.0_fin.pdf

	Andrew

Re: [PATCH net-next v2 3/3] net: phy: dp83tg720: Add cable testing support

[kernel test robot]

Hi Oleksij,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Oleksij-Rempel/phy-Add-Open-Alliance-helpers-for-the-PHY-framework/20240808-211730
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240808130833.2083875-3-o.rempel%40pengutronix.de
patch subject: [PATCH net-next v2 3/3] net: phy: dp83tg720: Add cable testing support
config: i386-buildonly-randconfig-001-20240809 (https://download.01.org/0day-ci/archive/20240809/202408090754.e4G2nq88-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240809/202408090754.e4G2nq88-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408090754.e4G2nq88-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in kernel/locking/test-ww_mutex.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fpga/tests/fpga-mgr-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fpga/tests/fpga-bridge-test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/fpga/tests/fpga-region-test.o
>> ERROR: modpost: "oa_1000bt1_get_ethtool_cable_result_code" [drivers/net/phy/dp83tg720.ko] undefined!
>> ERROR: modpost: "oa_1000bt1_get_tdr_distance" [drivers/net/phy/dp83tg720.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v2 2/3] phy: Add Open Alliance helpers for the PHY framework

[Oleksij Rempel]

On Thu, Aug 08, 2024 at 03:54:06PM +0200, Andrew Lunn wrote:

> Please could you give a reference to the exact standard. I think this
> is "Advanced diagnostic features for 1000BASE-T1 automotive Ethernet
> PHYs TC12 - advanced PHY features" ?
> 
> The standard seem open, so you could include a URL:
> 
> https://opensig.org/wp-content/uploads/2024/03/Advanced_PHY_features_for_automotive_Ethernet_v2.0_fin.pdf

I already started to implement other diagnostic features supported by the
TI DP83TG720 PHY. For example following can be implemented too:
6.3 Link quality – start-up time and link losses (LQ)
6.3.1 Link training time (LTT)
6.3.2 Local receiver time (LRT)
6.3.3 Remote receiver time (RRT)
6.3.4 Link Failures and Losses (LFL)
6.3.5 Communication ready status (COM)
6.4 Polarity Detection and Correction (POL)
6.4.1 Polarity Detection (DET)
6.4.2 Polarity Correction (COR)

What is the best way to proceed with them? Export them over phy-statistics
interface or extending the netlink interface?

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 v2 2/3] phy: Add Open Alliance helpers for the PHY framework

[Andrew Lunn]

On Fri, Aug 09, 2024 at 07:17:50AM +0200, Oleksij Rempel wrote:
> On Thu, Aug 08, 2024 at 03:54:06PM +0200, Andrew Lunn wrote:
> 
> > Please could you give a reference to the exact standard. I think this
> > is "Advanced diagnostic features for 1000BASE-T1 automotive Ethernet
> > PHYs TC12 - advanced PHY features" ?
> > 
> > The standard seem open, so you could include a URL:
> > 
> > https://opensig.org/wp-content/uploads/2024/03/Advanced_PHY_features_for_automotive_Ethernet_v2.0_fin.pdf
> 
> I already started to implement other diagnostic features supported by the
> TI DP83TG720 PHY. For example following can be implemented too:
> 6.3 Link quality – start-up time and link losses (LQ)
> 6.3.1 Link training time (LTT)
> 6.3.2 Local receiver time (LRT)
> 6.3.3 Remote receiver time (RRT)

These three are the time it takes for some action. Not really a
statistic in the normal netdev sense, since it does not count up. But
they are kind of statistics, so it is probably not abusing statistics
too much, so maybe O.K.

> 6.3.4 Link Failures and Losses (LFL)

This is a count, so does fit statistics. 

> 6.3.5 Communication ready status (COM)

Similar to the BMSR link bit. Do it add anything useful?

> 6.4 Polarity Detection and Correction (POL)
> 6.4.1 Polarity Detection (DET)
> 6.4.2 Polarity Correction (COR)

Could these be mapped to ETH_TP_MDI* ? 

      Andrew

Re: [PATCH net-next v2 2/3] phy: Add Open Alliance helpers for the PHY framework

[Oleksij Rempel]

On Fri, Aug 09, 2024 at 04:00:53PM +0200, Andrew Lunn wrote:
> On Fri, Aug 09, 2024 at 07:17:50AM +0200, Oleksij Rempel wrote:
> > On Thu, Aug 08, 2024 at 03:54:06PM +0200, Andrew Lunn wrote:
> > 
> > > Please could you give a reference to the exact standard. I think this
> > > is "Advanced diagnostic features for 1000BASE-T1 automotive Ethernet
> > > PHYs TC12 - advanced PHY features" ?
> > > 
> > > The standard seem open, so you could include a URL:
> > > 
> > > https://opensig.org/wp-content/uploads/2024/03/Advanced_PHY_features_for_automotive_Ethernet_v2.0_fin.pdf
> > 
> > I already started to implement other diagnostic features supported by the
> > TI DP83TG720 PHY. For example following can be implemented too:
> > 6.3 Link quality – start-up time and link losses (LQ)
> > 6.3.1 Link training time (LTT)
> > 6.3.2 Local receiver time (LRT)
> > 6.3.3 Remote receiver time (RRT)
> 
> These three are the time it takes for some action. Not really a
> statistic in the normal netdev sense, since it does not count up. But
> they are kind of statistics, so it is probably not abusing statistics
> too much, so maybe O.K.
> 
> > 6.3.4 Link Failures and Losses (LFL)
> 
> This is a count, so does fit statistics. 
> 
> > 6.3.5 Communication ready status (COM)
> 
> Similar to the BMSR link bit. Do it add anything useful?

Probably. I can leave it for now

> > 6.4 Polarity Detection and Correction (POL)
> > 6.4.1 Polarity Detection (DET)
> > 6.4.2 Polarity Correction (COR)
> 
> Could these be mapped to ETH_TP_MDI* ? 

Yes, but it will look confusing in the user space. To make better
representation in ethtool we will probably need a new port type. For
example instead of PORT_TP it will be PORT_STP (single twiste pair) or
PORT_SPE (single pair ethernet). What do you think?

Beside, there are some not standard specific fail indicators. It can show RGMII
and SGMII specific errors. For example R/S_GMII FIFO full/empty errors. If
i see it correctly, it will not drop MDI link, so I won't be able to
return a link fail reason for this case.

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 v2 2/3] phy: Add Open Alliance helpers for the PHY framework

[Andrew Lunn]

> > > 6.4 Polarity Detection and Correction (POL)
> > > 6.4.1 Polarity Detection (DET)
> > > 6.4.2 Polarity Correction (COR)
> > 
> > Could these be mapped to ETH_TP_MDI* ? 
> 
> Yes, but it will look confusing in the user space. To make better
> representation in ethtool we will probably need a new port type. For
> example instead of PORT_TP it will be PORT_STP (single twiste pair) or
> PORT_SPE (single pair ethernet). What do you think?

Thinking about this some more....

The Marvell PHYs can indicate per pair if the polarity is wrong and
has been corrected, for 10BaseT and 1000BaseT. The datasheet indicates
that for 100BaseTX polarity does not matter.

So i agree, MDI crossover is the wrong way to indicate this. We should
add polarity indication, for up to 4 pairs.

	Andrew