| Message ID | 20240820101256.1506460-4-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add ALCD Support to Cable Testing Interface | expand |
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
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?
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.
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(-)