2023-11-27 18:45:16

by Vincent Whitchurch

[permalink] [raw]
Subject: [PATCH net-next] net: phy: adin: allow control of Fast Link Down

Add support to allow Fast Link Down (aka "Enhanced link detection") to
be controlled via the ETHTOOL_PHY_FAST_LINK_DOWN tunable. These PHYs
have this feature enabled by default.

Signed-off-by: Vincent Whitchurch <[email protected]>
---
drivers/net/phy/adin.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 134637584a83..2e1a46e121d9 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -68,6 +68,24 @@
#define ADIN1300_EEE_CAP_REG 0x8000
#define ADIN1300_EEE_ADV_REG 0x8001
#define ADIN1300_EEE_LPABLE_REG 0x8002
+
+#define ADIN1300_FLD_EN_REG 0x8E27
+#define ADIN1300_FLD_PCS_ERR_100_EN BIT(7)
+#define ADIN1300_FLD_PCS_ERR_1000_EN BIT(6)
+#define ADIN1300_FLD_SLCR_OUT_STUCK_100_EN BIT(5)
+#define ADIN1300_FLD_SLCR_OUT_STUCK_1000_EN BIT(4)
+#define ADIN1300_FLD_SLCR_IN_ZDET_100_EN BIT(3)
+#define ADIN1300_FLD_SLCR_IN_ZDET_1000_EN BIT(2)
+#define ADIN1300_FLD_SLCR_IN_INVLD_100_EN BIT(1)
+#define ADIN1300_FLD_SLCR_IN_INVLD_1000_EN BIT(0)
+/* These bits are the ones which are enabled by default. */
+#define ADIN1300_FLD_EN_ON \
+ (ADIN1300_FLD_SLCR_OUT_STUCK_100_EN | \
+ ADIN1300_FLD_SLCR_OUT_STUCK_1000_EN | \
+ ADIN1300_FLD_SLCR_IN_ZDET_100_EN | \
+ ADIN1300_FLD_SLCR_IN_ZDET_1000_EN | \
+ ADIN1300_FLD_SLCR_IN_INVLD_1000_EN)
+
#define ADIN1300_CLOCK_STOP_REG 0x9400
#define ADIN1300_LPI_WAKE_ERR_CNT_REG 0xa000

@@ -416,6 +434,37 @@ static int adin_set_edpd(struct phy_device *phydev, u16 tx_interval)
val);
}

+static int adin_get_fast_down(struct phy_device *phydev, u8 *msecs)
+{
+ int reg;
+
+ reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_FLD_EN_REG);
+ if (reg < 0)
+ return reg;
+
+ if (reg & ADIN1300_FLD_EN_ON)
+ *msecs = ETHTOOL_PHY_FAST_LINK_DOWN_ON;
+ else
+ *msecs = ETHTOOL_PHY_FAST_LINK_DOWN_OFF;
+
+ return 0;
+}
+
+static int adin_set_fast_down(struct phy_device *phydev, const u8 *msecs)
+{
+ if (*msecs == ETHTOOL_PHY_FAST_LINK_DOWN_ON)
+ return phy_set_bits_mmd(phydev, MDIO_MMD_VEND1,
+ ADIN1300_FLD_EN_REG,
+ ADIN1300_FLD_EN_ON);
+
+ if (*msecs == ETHTOOL_PHY_FAST_LINK_DOWN_OFF)
+ return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1,
+ ADIN1300_FLD_EN_REG,
+ ADIN1300_FLD_EN_ON);
+
+ return -EINVAL;
+}
+
static int adin_get_tunable(struct phy_device *phydev,
struct ethtool_tunable *tuna, void *data)
{
@@ -424,6 +473,8 @@ static int adin_get_tunable(struct phy_device *phydev,
return adin_get_downshift(phydev, data);
case ETHTOOL_PHY_EDPD:
return adin_get_edpd(phydev, data);
+ case ETHTOOL_PHY_FAST_LINK_DOWN:
+ return adin_get_fast_down(phydev, data);
default:
return -EOPNOTSUPP;
}
@@ -437,6 +488,8 @@ static int adin_set_tunable(struct phy_device *phydev,
return adin_set_downshift(phydev, *(const u8 *)data);
case ETHTOOL_PHY_EDPD:
return adin_set_edpd(phydev, *(const u16 *)data);
+ case ETHTOOL_PHY_FAST_LINK_DOWN:
+ return adin_set_fast_down(phydev, data);
default:
return -EOPNOTSUPP;
}

---
base-commit: e1df5202e879bce09845ac62bae30206e1edfb80
change-id: 20231127-adin-fld-c072c3b79a5a

Best regards,
--
Vincent Whitchurch <[email protected]>


2023-11-28 09:06:37

by Nuno Sá

[permalink] [raw]
Subject: Re: [PATCH net-next] net: phy: adin: allow control of Fast Link Down

On Mon, 2023-11-27 at 16:31 +0100, Vincent Whitchurch wrote:
> Add support to allow Fast Link Down (aka "Enhanced link detection") to
> be controlled via the ETHTOOL_PHY_FAST_LINK_DOWN tunable.  These PHYs
> have this feature enabled by default.
>
> Signed-off-by: Vincent Whitchurch <[email protected]>
> ---

LGTM,

Acked-by: Nuno Sa <[email protected]>

Thanks!
- Nuno Sá

>  drivers/net/phy/adin.c | 53
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
>
> diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
> index 134637584a83..2e1a46e121d9 100644
> --- a/drivers/net/phy/adin.c
> +++ b/drivers/net/phy/adin.c
> @@ -68,6 +68,24 @@
>  #define ADIN1300_EEE_CAP_REG 0x8000
>  #define ADIN1300_EEE_ADV_REG 0x8001
>  #define ADIN1300_EEE_LPABLE_REG 0x8002
> +
> +#define ADIN1300_FLD_EN_REG 0x8E27
> +#define   ADIN1300_FLD_PCS_ERR_100_EN BIT(7)
> +#define   ADIN1300_FLD_PCS_ERR_1000_EN BIT(6)
> +#define   ADIN1300_FLD_SLCR_OUT_STUCK_100_EN BIT(5)
> +#define   ADIN1300_FLD_SLCR_OUT_STUCK_1000_EN BIT(4)
> +#define   ADIN1300_FLD_SLCR_IN_ZDET_100_EN BIT(3)
> +#define   ADIN1300_FLD_SLCR_IN_ZDET_1000_EN BIT(2)
> +#define   ADIN1300_FLD_SLCR_IN_INVLD_100_EN BIT(1)
> +#define   ADIN1300_FLD_SLCR_IN_INVLD_1000_EN BIT(0)
> +/* These bits are the ones which are enabled by default. */
> +#define ADIN1300_FLD_EN_ON \
> + (ADIN1300_FLD_SLCR_OUT_STUCK_100_EN | \
> + ADIN1300_FLD_SLCR_OUT_STUCK_1000_EN | \
> + ADIN1300_FLD_SLCR_IN_ZDET_100_EN | \
> + ADIN1300_FLD_SLCR_IN_ZDET_1000_EN | \
> + ADIN1300_FLD_SLCR_IN_INVLD_1000_EN)
> +
>  #define ADIN1300_CLOCK_STOP_REG 0x9400
>  #define ADIN1300_LPI_WAKE_ERR_CNT_REG 0xa000
>  
> @@ -416,6 +434,37 @@ static int adin_set_edpd(struct phy_device *phydev, u16
> tx_interval)
>     val);
>  }
>  
> +static int adin_get_fast_down(struct phy_device *phydev, u8 *msecs)
> +{
> + int reg;
> +
> + reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_FLD_EN_REG);
> + if (reg < 0)
> + return reg;
> +
> + if (reg & ADIN1300_FLD_EN_ON)
> + *msecs = ETHTOOL_PHY_FAST_LINK_DOWN_ON;
> + else
> + *msecs = ETHTOOL_PHY_FAST_LINK_DOWN_OFF;
> +
> + return 0;
> +}
> +
> +static int adin_set_fast_down(struct phy_device *phydev, const u8 *msecs)
> +{
> + if (*msecs == ETHTOOL_PHY_FAST_LINK_DOWN_ON)
> + return phy_set_bits_mmd(phydev, MDIO_MMD_VEND1,
> + ADIN1300_FLD_EN_REG,
> + ADIN1300_FLD_EN_ON);
> +
> + if (*msecs == ETHTOOL_PHY_FAST_LINK_DOWN_OFF)
> + return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1,
> +   ADIN1300_FLD_EN_REG,
> +   ADIN1300_FLD_EN_ON);
> +
> + return -EINVAL;
> +}
> +
>  static int adin_get_tunable(struct phy_device *phydev,
>       struct ethtool_tunable *tuna, void *data)
>  {
> @@ -424,6 +473,8 @@ static int adin_get_tunable(struct phy_device *phydev,
>   return adin_get_downshift(phydev, data);
>   case ETHTOOL_PHY_EDPD:
>   return adin_get_edpd(phydev, data);
> + case ETHTOOL_PHY_FAST_LINK_DOWN:
> + return adin_get_fast_down(phydev, data);
>   default:
>   return -EOPNOTSUPP;
>   }
> @@ -437,6 +488,8 @@ static int adin_set_tunable(struct phy_device *phydev,
>   return adin_set_downshift(phydev, *(const u8 *)data);
>   case ETHTOOL_PHY_EDPD:
>   return adin_set_edpd(phydev, *(const u16 *)data);
> + case ETHTOOL_PHY_FAST_LINK_DOWN:
> + return adin_set_fast_down(phydev, data);
>   default:
>   return -EOPNOTSUPP;
>   }
>
> ---
> base-commit: e1df5202e879bce09845ac62bae30206e1edfb80
> change-id: 20231127-adin-fld-c072c3b79a5a
>
> Best regards,

2023-11-28 20:18:35

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next] net: phy: adin: allow control of Fast Link Down

On Mon, Nov 27, 2023 at 04:31:39PM +0100, Vincent Whitchurch wrote:
> Add support to allow Fast Link Down (aka "Enhanced link detection") to
> be controlled via the ETHTOOL_PHY_FAST_LINK_DOWN tunable. These PHYs
> have this feature enabled by default.
>
> Signed-off-by: Vincent Whitchurch <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Is there anything in the datasheet about how fast it is? It would be
nice to return the number of milliseconds, if its known.

Andrew

2023-11-28 20:24:03

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next] net: phy: adin: allow control of Fast Link Down

On Tue, Nov 28, 2023 at 09:18:00PM +0100, Andrew Lunn wrote:
> On Mon, Nov 27, 2023 at 04:31:39PM +0100, Vincent Whitchurch wrote:
> > Add support to allow Fast Link Down (aka "Enhanced link detection") to
> > be controlled via the ETHTOOL_PHY_FAST_LINK_DOWN tunable. These PHYs
> > have this feature enabled by default.
> >
> > Signed-off-by: Vincent Whitchurch <[email protected]>
>
> Reviewed-by: Andrew Lunn <[email protected]>
>
> Is there anything in the datasheet about how fast it is? It would be
> nice to return the number of milliseconds, if its known.

Datasheet says:

If enhanced link detection is enabled (it is enabled by default), the
ADIN1300 typically reacts to a break in the cable within 10 μs and
indicates link down via the LINK_ST pin. If enhanced link detection
is not enabled, the ADIN1300 follows the IEEE standard, and in
100BASE-TX, it can take more than either 350 ms or 750 ms in
1000BASE-T, depending if the PHY is 1000BASE-T master or 1000BASE-T
slave.

10uS is closer to 0ms and 1ms, so ETHTOOL_PHY_FAST_LINK_DOWN_ON == 0
is right.

Andrew

2023-11-30 04:10:42

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net-next] net: phy: adin: allow control of Fast Link Down

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <[email protected]>:

On Mon, 27 Nov 2023 16:31:39 +0100 you wrote:
> Add support to allow Fast Link Down (aka "Enhanced link detection") to
> be controlled via the ETHTOOL_PHY_FAST_LINK_DOWN tunable. These PHYs
> have this feature enabled by default.
>
> Signed-off-by: Vincent Whitchurch <[email protected]>
> ---
> drivers/net/phy/adin.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> [...]

Here is the summary with links:
- [net-next] net: phy: adin: allow control of Fast Link Down
https://git.kernel.org/netdev/net-next/c/cb2f01b856ea

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html