[PATCH net-next v2 0/3] Add ALCD Support to Cable Testing Interface

[PATCH net-next v2 0/3] Add ALCD Support to Cable Testing Interface

[Oleksij Rempel]

Hi all,

This patch series introduces support for Active Link Cable Diagnostics
(ALCD) in the ethtool cable testing interface and the DP83TD510 PHY
driver.

Why ALCD?
On a 10BaseT1L interface, TDR (Time Domain Reflectometry) is not
possible if the link partner is active - TDR will fail in these cases
because it requires interrupting the link. Since the link is active, we
already know the cable is functioning, so instead of using TDR, we can
use ALCD.

ALCD lets us measure cable length without disrupting the active link,
which is crucial in environments where network uptime is important. It
provides a way to gather diagnostic data without the need for downtime.

What's in this series:
- Extended the ethtool cable testing interface to specify the source of
  diagnostic results (TDR or ALCD).
- Updated the DP83TD510 PHY driver to use ALCD when the link is
  active, ensuring we can still get cable length info without dropping the
  connection.

Changes are described in separate patches.

Thanks,
Oleksij

Oleksij Rempel (3):
  ethtool: Extend cable testing interface with result source information
  ethtool: Add support for specifying information source in cable test
    results
  phy: dp83td510: Utilize ALCD for cable length measurement when link is
    active

 Documentation/netlink/specs/ethtool.yaml     |  6 ++
 Documentation/networking/ethtool-netlink.rst |  5 ++
 drivers/net/phy/dp83td510.c                  | 83 ++++++++++++++++++--
 include/linux/ethtool_netlink.h              | 29 +++++--
 include/uapi/linux/ethtool_netlink.h         | 11 +++
 net/ethtool/cabletest.c                      | 19 ++++-
 6 files changed, 137 insertions(+), 16 deletions(-)

--
2.39.2

[PATCH net-next v2 1/3] ethtool: Extend cable testing interface with result source information

[Oleksij Rempel]

Extend the ethtool netlink cable testing interface by adding support for
specifying the source of cable testing results. This allows users to
differentiate between results obtained through different diagnostic
methods.

For example, some TI 10BaseT1L PHYs provide two variants of cable
diagnostics: Time Domain Reflectometry (TDR) and Active Link Cable
Diagnostic (ALCD). By introducing `ETHTOOL_A_CABLE_RESULT_SRC` and
`ETHTOOL_A_CABLE_FAULT_LENGTH_SRC` attributes, this update enables
drivers to indicate whether the result was derived from TDR or ALCD,
improving the clarity and utility of diagnostic information.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- update Documentation/netlink/specs/ethtool.yaml
- use u8 instead of u32 for _src
- update comments
---
 Documentation/netlink/specs/ethtool.yaml     |  6 ++++++
 Documentation/networking/ethtool-netlink.rst |  5 +++++
 include/uapi/linux/ethtool_netlink.h         | 11 +++++++++++
 3 files changed, 22 insertions(+)

diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 1bbeaba5c6442..af999ccd0adf8 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -659,6 +659,9 @@ attribute-sets:
       -
         name: code
         type: u8
+      -
+        name: src
+        type: u8
   -
     name: cable-fault-length
     attributes:
@@ -668,6 +671,9 @@ attribute-sets:
       -
         name: cm
         type: u32
+      -
+        name: src
+        type: u8
   -
     name: cable-nest
     attributes:
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 3f6c6880e7c48..295e75619abdf 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1307,12 +1307,17 @@ information.
  +-+-+-----------------------------------------+--------+---------------------+
  | | | ``ETHTOOL_A_CABLE_RESULTS_CODE``        | u8     | result code         |
  +-+-+-----------------------------------------+--------+---------------------+
+ | | | ``ETHTOOL_A_CABLE_RESULT_SRC``          | u8     | information source  |
+ +-+-+-----------------------------------------+--------+---------------------+
  | | ``ETHTOOL_A_CABLE_NEST_FAULT_LENGTH``     | nested | cable length        |
  +-+-+-----------------------------------------+--------+---------------------+
  | | | ``ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR``   | u8     | pair number         |
  +-+-+-----------------------------------------+--------+---------------------+
  | | | ``ETHTOOL_A_CABLE_FAULT_LENGTH_CM``     | u32    | length in cm        |
  +-+-+-----------------------------------------+--------+---------------------+
+ | | | ``ETHTOOL_A_CABLE_FAULT_LENGTH_SRC``    | u8     | information source  |
+ +-+-+-----------------------------------------+--------+---------------------+
+
 
 CABLE_TEST TDR
 ==============
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 9074fa309bd6d..445e2d434686f 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -569,10 +569,20 @@ enum {
 	ETHTOOL_A_CABLE_PAIR_D,
 };
 
+/* Information source for specific results. */
+enum {
+	ETHTOOL_A_CABLE_INF_SRC_UNSPEC,
+	/* Results provided by the Time Domain Reflectometry (TDR) */
+	ETHTOOL_A_CABLE_INF_SRC_TDR,
+	/* Results provided by the Active Link Cable Diagnostic (ALCD) */
+	ETHTOOL_A_CABLE_INF_SRC_ALCD,
+};
+
 enum {
 	ETHTOOL_A_CABLE_RESULT_UNSPEC,
 	ETHTOOL_A_CABLE_RESULT_PAIR,		/* u8 ETHTOOL_A_CABLE_PAIR_ */
 	ETHTOOL_A_CABLE_RESULT_CODE,		/* u8 ETHTOOL_A_CABLE_RESULT_CODE_ */
+	ETHTOOL_A_CABLE_RESULT_SRC,		/* u8 ETHTOOL_A_CABLE_INF_SRC_ */
 
 	__ETHTOOL_A_CABLE_RESULT_CNT,
 	ETHTOOL_A_CABLE_RESULT_MAX = (__ETHTOOL_A_CABLE_RESULT_CNT - 1)
@@ -582,6 +592,7 @@ enum {
 	ETHTOOL_A_CABLE_FAULT_LENGTH_UNSPEC,
 	ETHTOOL_A_CABLE_FAULT_LENGTH_PAIR,	/* u8 ETHTOOL_A_CABLE_PAIR_ */
 	ETHTOOL_A_CABLE_FAULT_LENGTH_CM,	/* u32 */
+	ETHTOOL_A_CABLE_FAULT_LENGTH_SRC,	/* u8 ETHTOOL_A_CABLE_INF_SRC_ */
 
 	__ETHTOOL_A_CABLE_FAULT_LENGTH_CNT,
 	ETHTOOL_A_CABLE_FAULT_LENGTH_MAX = (__ETHTOOL_A_CABLE_FAULT_LENGTH_CNT - 1)
-- 
2.39.2

[PATCH net-next v2 2/3] ethtool: Add support for specifying information source in cable test results

[Oleksij Rempel]

Enhance the ethtool cable test interface by introducing the ability to
specify the source of the diagnostic information for cable test results.
This is particularly useful for PHYs that offer multiple diagnostic
methods, such as Time Domain Reflectometry (TDR) and Active Link Cable
Diagnostic (ALCD).

Key changes:
- Added `ethnl_cable_test_result_with_src` and
  `ethnl_cable_test_fault_length_with_src` functions to allow specifying
  the information source when reporting cable test results.
- Updated existing `ethnl_cable_test_result` and
  `ethnl_cable_test_fault_length` functions to use TDR as the default
  source, ensuring backward compatibility.
- Modified the UAPI to support these new attributes, enabling drivers to
  provide more detailed diagnostic information.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- use u8 instead of u32 for src
- do not send src field if src is ETHTOOL_A_CABLE_INF_SRC_UNSPEC
---
 include/linux/ethtool_netlink.h | 29 +++++++++++++++++++++++------
 net/ethtool/cabletest.c         | 19 +++++++++++++++----
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/include/linux/ethtool_netlink.h b/include/linux/ethtool_netlink.h
index fae0dfb9a9c82..a21e5c7b9435f 100644
--- a/include/linux/ethtool_netlink.h
+++ b/include/linux/ethtool_netlink.h
@@ -23,8 +23,10 @@ struct phy_device;
 int ethnl_cable_test_alloc(struct phy_device *phydev, u8 cmd);
 void ethnl_cable_test_free(struct phy_device *phydev);
 void ethnl_cable_test_finished(struct phy_device *phydev);
-int ethnl_cable_test_result(struct phy_device *phydev, u8 pair, u8 result);
-int ethnl_cable_test_fault_length(struct phy_device *phydev, u8 pair, u32 cm);
+int ethnl_cable_test_result_with_src(struct phy_device *phydev, u8 pair,
+				     u8 result, u8 src);
+int ethnl_cable_test_fault_length_with_src(struct phy_device *phydev, u8 pair,
+					   u32 cm, u8 src);
 int ethnl_cable_test_amplitude(struct phy_device *phydev, u8 pair, s16 mV);
 int ethnl_cable_test_pulse(struct phy_device *phydev, u16 mV);
 int ethnl_cable_test_step(struct phy_device *phydev, u32 first, u32 last,
@@ -54,14 +56,14 @@ static inline void ethnl_cable_test_free(struct phy_device *phydev)
 static inline void ethnl_cable_test_finished(struct phy_device *phydev)
 {
 }
-static inline int ethnl_cable_test_result(struct phy_device *phydev, u8 pair,
-					  u8 result)
+static inline int ethnl_cable_test_result_with_src(struct phy_device *phydev,
+						   u8 pair, u8 result, u8 src)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline int ethnl_cable_test_fault_length(struct phy_device *phydev,
-						u8 pair, u32 cm)
+static inline int ethnl_cable_test_fault_length_with_src(struct phy_device *phydev,
+							 u8 pair, u32 cm, u8 src)
 {
 	return -EOPNOTSUPP;
 }
@@ -119,4 +121,19 @@ static inline bool ethtool_dev_mm_supported(struct net_device *dev)
 }
 
 #endif /* IS_ENABLED(CONFIG_ETHTOOL_NETLINK) */
+
+static inline int ethnl_cable_test_result(struct phy_device *phydev, u8 pair,
+					  u8 result)
+{
+	return ethnl_cable_test_result_with_src(phydev, pair, result,
+						ETHTOOL_A_CABLE_INF_SRC_TDR);
+}
+
+static inline int ethnl_cable_test_fault_length(struct phy_device *phydev,
+						u8 pair, u32 cm)
+{
+	return ethnl_cable_test_fault_length_with_src(phydev, pair, cm,
+						      ETHTOOL_A_CABLE_INF_SRC_TDR);
+}
+
 #endif /* _LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/net/ethtool/cabletest.c b/net/ethtool/cabletest.c
index f6f136ec7ddfc..97ef7895997e2 100644
--- a/net/ethtool/cabletest.c
+++ b/net/ethtool/cabletest.c
@@ -160,7 +160,8 @@ void ethnl_cable_test_finished(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(ethnl_cable_test_finished);
 
-int ethnl_cable_test_result(struct phy_device *phydev, u8 pair, u8 result)
+int ethnl_cable_test_result_with_src(struct phy_device *phydev, u8 pair,
+				     u8 result, u8 src)
 {
 	struct nlattr *nest;
 	int ret = -EMSGSIZE;
@@ -173,6 +174,10 @@ int ethnl_cable_test_result(struct phy_device *phydev, u8 pair, u8 result)
 		goto err;
 	if (nla_put_u8(phydev->skb, ETHTOOL_A_CABLE_RESULT_CODE, result))
 		goto err;
+	if (src != ETHTOOL_A_CABLE_INF_SRC_UNSPEC) {
+		if (nla_put_u8(phydev->skb, ETHTOOL_A_CABLE_RESULT_SRC, src))
+			goto err;
+	}
 
 	nla_nest_end(phydev->skb, nest);
 	return 0;
@@ -181,9 +186,10 @@ int ethnl_cable_test_result(struct phy_device *phydev, u8 pair, u8 result)
 	nla_nest_cancel(phydev->skb, nest);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(ethnl_cable_test_result);
+EXPORT_SYMBOL_GPL(ethnl_cable_test_result_with_src);
 
-int ethnl_cable_test_fault_length(struct phy_device *phydev, u8 pair, u32 cm)
+int ethnl_cable_test_fault_length_with_src(struct phy_device *phydev, u8 pair,
+					   u32 cm, u8 src)
 {
 	struct nlattr *nest;
 	int ret = -EMSGSIZE;
@@ -197,6 +203,11 @@ int ethnl_cable_test_fault_length(struct phy_device *phydev, u8 pair, u32 cm)
 		goto err;
 	if (nla_put_u32(phydev->skb, ETHTOOL_A_CABLE_FAULT_LENGTH_CM, cm))
 		goto err;
+	if (src != ETHTOOL_A_CABLE_INF_SRC_UNSPEC) {
+		if (nla_put_u8(phydev->skb, ETHTOOL_A_CABLE_FAULT_LENGTH_SRC,
+			       src))
+			goto err;
+	}
 
 	nla_nest_end(phydev->skb, nest);
 	return 0;
@@ -205,7 +216,7 @@ int ethnl_cable_test_fault_length(struct phy_device *phydev, u8 pair, u32 cm)
 	nla_nest_cancel(phydev->skb, nest);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(ethnl_cable_test_fault_length);
+EXPORT_SYMBOL_GPL(ethnl_cable_test_fault_length_with_src);
 
 static const struct nla_policy cable_test_tdr_act_cfg_policy[] = {
 	[ETHTOOL_A_CABLE_TEST_TDR_CFG_FIRST]	= { .type = NLA_U32 },
-- 
2.39.2

[PATCH net-next v2 3/3] phy: dp83td510: Utilize ALCD for cable length measurement when link is active

[Oleksij Rempel]

In industrial environments where 10BaseT1L PHYs are replacing existing
field bus systems like CAN, it's often essential to retain the existing
cable infrastructure. After installation, collecting metrics such as
cable length is crucial for assessing the quality of the infrastructure.
Traditionally, TDR (Time Domain Reflectometry) is used for this purpose.
However, TDR requires interrupting the link, and if the link partner
remains active, the TDR measurement will fail.

Unlike multi-pair systems, where TDR can be attempted during the MDI-X
switching window, 10BaseT1L systems face greater challenges. The TDR
sequence on 10BaseT1L is longer and coincides with uninterrupted
autonegotiation pulses, making TDR impossible when the link partner is
active.

The DP83TD510 PHY provides an alternative through ALCD (Active Link
Cable Diagnostics), which allows for cable length measurement without
disrupting an active link. Since a live link indicates no short or open
cable states, ALCD can be used effectively to gather cable length
information.

Enhance the dp83td510 driver by:
- Leveraging ALCD to measure cable length when the link is active.
- Bypassing TDR when a link is detected, as ALCD provides the required
  information without disruption.

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

diff --git a/drivers/net/phy/dp83td510.c b/drivers/net/phy/dp83td510.c
index 551e37786c2da..10a46354ad2b8 100644
--- a/drivers/net/phy/dp83td510.c
+++ b/drivers/net/phy/dp83td510.c
@@ -169,6 +169,10 @@ static const u16 dp83td510_mse_sqi_map[] = {
 #define DP83TD510E_UNKN_030E				0x30e
 #define DP83TD510E_030E_VAL				0x2520
 
+#define DP83TD510E_ALCD_STAT				0xa9f
+#define DP83TD510E_ALCD_COMPLETE			BIT(15)
+#define DP83TD510E_ALCD_CABLE_LENGTH			GENMASK(10, 0)
+
 static int dp83td510_config_intr(struct phy_device *phydev)
 {
 	int ret;
@@ -327,6 +331,16 @@ static int dp83td510_cable_test_start(struct phy_device *phydev)
 {
 	int ret;
 
+	/* If link partner is active, we won't be able to use TDR, since
+	 * we can't force link partner to be silent. The autonegotiation
+	 * pulses will be too frequent and the TDR sequence will be
+	 * too long. So, TDR will always fail. Since the link is established
+	 * we already know that the cable is working, so we can get some
+	 * extra information line the cable length using ALCD.
+	 */
+	if (phydev->link)
+		return 0;
+
 	ret = phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_CTRL,
 			       DP83TD510E_CTRL_HW_RESET);
 	if (ret)
@@ -402,8 +416,8 @@ static int dp83td510_cable_test_start(struct phy_device *phydev)
 }
 
 /**
- * dp83td510_cable_test_get_status - Get the status of the cable test for the
- *                                   DP83TD510 PHY.
+ * dp83td510_cable_test_get_tdr_status - Get the status of the TDR test for the
+ *                                       DP83TD510 PHY.
  * @phydev: Pointer to the phy_device structure.
  * @finished: Pointer to a boolean that indicates whether the test is finished.
  *
@@ -411,13 +425,11 @@ static int dp83td510_cable_test_start(struct phy_device *phydev)
  *
  * Returns: 0 on success or a negative error code on failure.
  */
-static int dp83td510_cable_test_get_status(struct phy_device *phydev,
-					   bool *finished)
+static int dp83td510_cable_test_get_tdr_status(struct phy_device *phydev,
+					       bool *finished)
 {
 	int ret, stat;
 
-	*finished = false;
-
 	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_TDR_CFG);
 	if (ret < 0)
 		return ret;
@@ -459,6 +471,65 @@ static int dp83td510_cable_test_get_status(struct phy_device *phydev,
 	return phy_init_hw(phydev);
 }
 
+/**
+ * dp83td510_cable_test_get_alcd_status - Get the status of the ALCD test for the
+ *                                        DP83TD510 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.
+ * The function reads the cable length and reports it to the user.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+static int dp83td510_cable_test_get_alcd_status(struct phy_device *phydev,
+						bool *finished)
+{
+	unsigned int location;
+	int ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_ALCD_STAT);
+	if (ret < 0)
+		return ret;
+
+	if (!(ret & DP83TD510E_ALCD_COMPLETE))
+		return 0;
+
+	location = FIELD_GET(DP83TD510E_ALCD_CABLE_LENGTH, ret) * 100;
+
+	ethnl_cable_test_fault_length_with_src(phydev, ETHTOOL_A_CABLE_PAIR_A,
+					       location,
+					       ETHTOOL_A_CABLE_INF_SRC_ALCD);
+
+	ethnl_cable_test_result_with_src(phydev, ETHTOOL_A_CABLE_PAIR_A,
+					 ETHTOOL_A_CABLE_RESULT_CODE_OK,
+					 ETHTOOL_A_CABLE_INF_SRC_ALCD);
+	*finished = true;
+
+	return 0;
+}
+
+/**
+ * dp83td510_cable_test_get_status - Get the status of the cable test for the
+ *                                   DP83TD510 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 dp83td510_cable_test_get_status(struct phy_device *phydev,
+					   bool *finished)
+{
+	*finished = false;
+
+	if (!phydev->link)
+		return dp83td510_cable_test_get_tdr_status(phydev, finished);
+
+	return dp83td510_cable_test_get_alcd_status(phydev, finished);
+}
+
 static int dp83td510_get_features(struct phy_device *phydev)
 {
 	/* This PHY can't respond on MDIO bus if no RMII clock is enabled.
-- 
2.39.2

Re: [PATCH net-next v2 1/3] ethtool: Extend cable testing interface with result source information

[Andrew Lunn]

On Tue, Aug 20, 2024 at 12:12:54PM +0200, Oleksij Rempel wrote:
> Extend the ethtool netlink cable testing interface by adding support for
> specifying the source of cable testing results. This allows users to
> differentiate between results obtained through different diagnostic
> methods.
> 
> For example, some TI 10BaseT1L PHYs provide two variants of cable
> diagnostics: Time Domain Reflectometry (TDR) and Active Link Cable
> Diagnostic (ALCD). By introducing `ETHTOOL_A_CABLE_RESULT_SRC` and
> `ETHTOOL_A_CABLE_FAULT_LENGTH_SRC` attributes, this update enables
> drivers to indicate whether the result was derived from TDR or ALCD,
> improving the clarity and utility of diagnostic information.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

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

    Andrew

Re: [PATCH net-next v2 2/3] ethtool: Add support for specifying information source in cable test results

[Andrew Lunn]

On Tue, Aug 20, 2024 at 12:12:55PM +0200, Oleksij Rempel wrote:
> Enhance the ethtool cable test interface by introducing the ability to
> specify the source of the diagnostic information for cable test results.
> This is particularly useful for PHYs that offer multiple diagnostic
> methods, such as Time Domain Reflectometry (TDR) and Active Link Cable
> Diagnostic (ALCD).
> 
> Key changes:
> - Added `ethnl_cable_test_result_with_src` and
>   `ethnl_cable_test_fault_length_with_src` functions to allow specifying
>   the information source when reporting cable test results.
> - Updated existing `ethnl_cable_test_result` and
>   `ethnl_cable_test_fault_length` functions to use TDR as the default
>   source, ensuring backward compatibility.
> - Modified the UAPI to support these new attributes, enabling drivers to
>   provide more detailed diagnostic information.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

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

    Andrew

Re: [PATCH net-next v2 3/3] phy: dp83td510: Utilize ALCD for cable length measurement when link is active

[Andrew Lunn]

On Tue, Aug 20, 2024 at 12:12:56PM +0200, Oleksij Rempel wrote:
> In industrial environments where 10BaseT1L PHYs are replacing existing
> field bus systems like CAN, it's often essential to retain the existing
> cable infrastructure. After installation, collecting metrics such as
> cable length is crucial for assessing the quality of the infrastructure.
> Traditionally, TDR (Time Domain Reflectometry) is used for this purpose.
> However, TDR requires interrupting the link, and if the link partner
> remains active, the TDR measurement will fail.
> 
> Unlike multi-pair systems, where TDR can be attempted during the MDI-X
> switching window, 10BaseT1L systems face greater challenges. The TDR
> sequence on 10BaseT1L is longer and coincides with uninterrupted
> autonegotiation pulses, making TDR impossible when the link partner is
> active.
> 
> The DP83TD510 PHY provides an alternative through ALCD (Active Link
> Cable Diagnostics), which allows for cable length measurement without
> disrupting an active link. Since a live link indicates no short or open
> cable states, ALCD can be used effectively to gather cable length
> information.
> 
> Enhance the dp83td510 driver by:
> - Leveraging ALCD to measure cable length when the link is active.
> - Bypassing TDR when a link is detected, as ALCD provides the required
>   information without disruption.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/dp83td510.c | 83 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 77 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/phy/dp83td510.c b/drivers/net/phy/dp83td510.c
> index 551e37786c2da..10a46354ad2b8 100644
> --- a/drivers/net/phy/dp83td510.c
> +++ b/drivers/net/phy/dp83td510.c
> @@ -169,6 +169,10 @@ static const u16 dp83td510_mse_sqi_map[] = {
>  #define DP83TD510E_UNKN_030E				0x30e
>  #define DP83TD510E_030E_VAL				0x2520
>  
> +#define DP83TD510E_ALCD_STAT				0xa9f
> +#define DP83TD510E_ALCD_COMPLETE			BIT(15)
> +#define DP83TD510E_ALCD_CABLE_LENGTH			GENMASK(10, 0)
> +
>  static int dp83td510_config_intr(struct phy_device *phydev)
>  {
>  	int ret;
> @@ -327,6 +331,16 @@ static int dp83td510_cable_test_start(struct phy_device *phydev)
>  {
>  	int ret;
>  
> +	/* If link partner is active, we won't be able to use TDR, since
> +	 * we can't force link partner to be silent. The autonegotiation
> +	 * pulses will be too frequent and the TDR sequence will be
> +	 * too long. So, TDR will always fail. Since the link is established
> +	 * we already know that the cable is working, so we can get some
> +	 * extra information line the cable length using ALCD.
> +	 */
> +	if (phydev->link)
> +		return 0;
> +

> +static int dp83td510_cable_test_get_status(struct phy_device *phydev,
> +					   bool *finished)
> +{
> +	*finished = false;
> +
> +	if (!phydev->link)
> +		return dp83td510_cable_test_get_tdr_status(phydev, finished);
> +
> +	return dp83td510_cable_test_get_alcd_status(phydev, finished);

Sorry, missed this earlier. It seems like there is a race here. It
could be the cable test was started without link, but when phylib
polls a few seconds later link could of established. Will valid ALCD
results be returned?

	Andrew

Re: [PATCH net-next v2 3/3] phy: dp83td510: Utilize ALCD for cable length measurement when link is active

[Oleksij Rempel]

On Tue, Aug 20, 2024 at 06:49:00PM +0200, Andrew Lunn wrote:
> > +static int dp83td510_cable_test_get_status(struct phy_device *phydev,
> > +					   bool *finished)
> > +{
> > +	*finished = false;
> > +
> > +	if (!phydev->link)
> > +		return dp83td510_cable_test_get_tdr_status(phydev, finished);
> > +
> > +	return dp83td510_cable_test_get_alcd_status(phydev, finished);
> 
> Sorry, missed this earlier. It seems like there is a race here. It
> could be the cable test was started without link, but when phylib
> polls a few seconds later link could of established. Will valid ALCD
> results be returned?

Hm.. probably not. In this case the best option I have is to store
results in the priv. Will it be acceptable?

-- 
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 1/3] ethtool: Extend cable testing interface with result source information

[Jakub Kicinski]

On Tue, 20 Aug 2024 12:12:54 +0200 Oleksij Rempel wrote:
> +      -
> +        name: src
> +        type: u8

If you have to respin please s/u8/u32/

Netlink rounds up all fields sizes to u32, u8 ends up being the same
size as u32 "on the wire" but 3 out of the 4 bytes are unusable.