[PATCH net-next v2 0/2] ethtool: add support for Fast Link Down as new PHY tunable

[PATCH net-next v2 0/2] ethtool: add support for Fast Link Down as new PHY tunable

[Heiner Kallweit]

This adds support for Fast Link Down as new PHY tunable.
Fast Link Down reduces the time until a link down event is reported
for 1000BaseT. According to the standard it's 750ms what is too long
for several use cases.

This is the kernel-related series, the ethtool userspace extension
I'd submit once the kernel part has been applied.

v2:
- add describing comment in patch 1

Heiner Kallweit (2):
  ethtool: add PHY Fast Link Down support
  net: phy: marvell: add PHY tunable fast link down support for 88E1540

 drivers/net/phy/marvell.c    | 108 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/ethtool.h |   8 +++
 net/core/ethtool.c           |   2 +
 3 files changed, 118 insertions(+)

-- 
2.21.0

[PATCH net-next v2 1/2] ethtool: add PHY Fast Link Down support

[Heiner Kallweit]

This adds support for Fast Link Down as new PHY tunable.
Fast Link Down reduces the time until a link down event is reported
for 1000BaseT. According to the standard it's 750ms what is too long
for several use cases.

v2:
- add comment describing the constants

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 include/uapi/linux/ethtool.h | 8 ++++++++
 net/core/ethtool.c           | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 3652b239d..50c76f4fa 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -252,9 +252,17 @@ struct ethtool_tunable {
 #define DOWNSHIFT_DEV_DEFAULT_COUNT	0xff
 #define DOWNSHIFT_DEV_DISABLE		0
 
+/* Time in msecs after which link is reported as down
+ * 0 = lowest time supported by the PHY
+ * 0xff = off, link down detection according to standard
+ */
+#define ETHTOOL_PHY_FAST_LINK_DOWN_ON	0
+#define ETHTOOL_PHY_FAST_LINK_DOWN_OFF	0xff
+
 enum phy_tunable_id {
 	ETHTOOL_PHY_ID_UNSPEC,
 	ETHTOOL_PHY_DOWNSHIFT,
+	ETHTOOL_PHY_FAST_LINK_DOWN,
 	/*
 	 * Add your fresh new phy tunable attribute above and remember to update
 	 * phy_tunable_strings[] in net/core/ethtool.c
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index b1eb32419..387d67eb7 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -136,6 +136,7 @@ static const char
 phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
 	[ETHTOOL_ID_UNSPEC]     = "Unspec",
 	[ETHTOOL_PHY_DOWNSHIFT]	= "phy-downshift",
+	[ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down",
 };
 
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
@@ -2432,6 +2433,7 @@ static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
 {
 	switch (tuna->id) {
 	case ETHTOOL_PHY_DOWNSHIFT:
+	case ETHTOOL_PHY_FAST_LINK_DOWN:
 		if (tuna->len != sizeof(u8) ||
 		    tuna->type_id != ETHTOOL_TUNABLE_U8)
 			return -EINVAL;
-- 
2.21.0

[PATCH net-next v2 2/2] net: phy: marvell: add PHY tunable fast link down support for 88E1540

[Heiner Kallweit]

1000BaseT standard requires that a link is reported as down earliest
after 750ms. Several use case however require a much faster detecion
of a broken link. Fast Link Down supports this by intentionally
violating a the standard. This patch exposes the Fast Link Down
feature of 88E1540 and 88E6390. These PHY's can be found as internal
PHY's in several switches: 88E6352, 88E6240, 88E6176, 88E6172,
and 88E6390(X). Fast Link Down and EEE are mutually exclusive.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/marvell.c | 108 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 3ccba37bd..65350186d 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -29,6 +29,7 @@
 #include <linux/ethtool.h>
 #include <linux/phy.h>
 #include <linux/marvell_phy.h>
+#include <linux/bitfield.h>
 #include <linux/of.h>
 
 #include <linux/io.h>
@@ -91,6 +92,14 @@
 #define MII_88E1510_TEMP_SENSOR		0x1b
 #define MII_88E1510_TEMP_SENSOR_MASK	0xff
 
+#define MII_88E1540_COPPER_CTRL3	0x1a
+#define MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_MASK	GENMASK(11, 10)
+#define MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_00MS	0
+#define MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_10MS	1
+#define MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_20MS	2
+#define MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_40MS	3
+#define MII_88E1540_COPPER_CTRL3_FAST_LINK_DOWN		BIT(9)
+
 #define MII_88E6390_MISC_TEST		0x1b
 #define MII_88E6390_MISC_TEST_SAMPLE_1S		0
 #define MII_88E6390_MISC_TEST_SAMPLE_10MS	BIT(14)
@@ -1025,6 +1034,101 @@ static int m88e1145_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int m88e1540_get_fld(struct phy_device *phydev, u8 *msecs)
+{
+	int val;
+
+	val = phy_read(phydev, MII_88E1540_COPPER_CTRL3);
+	if (val < 0)
+		return val;
+
+	if (!(val & MII_88E1540_COPPER_CTRL3_FAST_LINK_DOWN)) {
+		*msecs = ETHTOOL_PHY_FAST_LINK_DOWN_OFF;
+		return 0;
+	}
+
+	val = FIELD_GET(MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_MASK, val);
+
+	switch (val) {
+	case MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_00MS:
+		*msecs = 0;
+		break;
+	case MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_10MS:
+		*msecs = 10;
+		break;
+	case MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_20MS:
+		*msecs = 20;
+		break;
+	case MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_40MS:
+		*msecs = 40;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int m88e1540_set_fld(struct phy_device *phydev, const u8 *msecs)
+{
+	struct ethtool_eee eee;
+	int val, ret;
+
+	if (*msecs == ETHTOOL_PHY_FAST_LINK_DOWN_OFF)
+		return phy_clear_bits(phydev, MII_88E1540_COPPER_CTRL3,
+				      MII_88E1540_COPPER_CTRL3_FAST_LINK_DOWN);
+
+	/* According to the Marvell data sheet EEE must be disabled for
+	 * Fast Link Down detection to work properly
+	 */
+	ret = phy_ethtool_get_eee(phydev, &eee);
+	if (!ret && eee.eee_enabled) {
+		phydev_warn(phydev, "Fast Link Down detection requires EEE to be disabled!\n");
+		return -EBUSY;
+	}
+
+	if (*msecs <= 5)
+		val = MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_00MS;
+	else if (*msecs <= 15)
+		val = MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_10MS;
+	else if (*msecs <= 30)
+		val = MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_20MS;
+	else
+		val = MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_40MS;
+
+	val = FIELD_PREP(MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_MASK, val);
+
+	ret = phy_modify(phydev, MII_88E1540_COPPER_CTRL3,
+			 MII_88E1540_COPPER_CTRL3_LINK_DOWN_DELAY_MASK, val);
+	if (ret)
+		return ret;
+
+	return phy_set_bits(phydev, MII_88E1540_COPPER_CTRL3,
+			    MII_88E1540_COPPER_CTRL3_FAST_LINK_DOWN);
+}
+
+static int m88e1540_get_tunable(struct phy_device *phydev,
+				struct ethtool_tunable *tuna, void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_FAST_LINK_DOWN:
+		return m88e1540_get_fld(phydev, data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int m88e1540_set_tunable(struct phy_device *phydev,
+				struct ethtool_tunable *tuna, const void *data)
+{
+	switch (tuna->id) {
+	case ETHTOOL_PHY_FAST_LINK_DOWN:
+		return m88e1540_set_fld(phydev, data);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 /* The VOD can be out of specification on link up. Poke an
  * undocumented register, in an undocumented page, with a magic value
  * to fix this.
@@ -2247,6 +2351,8 @@ static struct phy_driver marvell_drivers[] = {
 		.get_sset_count = marvell_get_sset_count,
 		.get_strings = marvell_get_strings,
 		.get_stats = marvell_get_stats,
+		.get_tunable = m88e1540_get_tunable,
+		.set_tunable = m88e1540_set_tunable,
 	},
 	{
 		.phy_id = MARVELL_PHY_ID_88E1545,
@@ -2307,6 +2413,8 @@ static struct phy_driver marvell_drivers[] = {
 		.get_sset_count = marvell_get_sset_count,
 		.get_strings = marvell_get_strings,
 		.get_stats = marvell_get_stats,
+		.get_tunable = m88e1540_get_tunable,
+		.set_tunable = m88e1540_set_tunable,
 	},
 };
 
-- 
2.21.0

Re: [PATCH net-next v2 1/2] ethtool: add PHY Fast Link Down support

[Florian Fainelli]

On 3/25/19 11:34 AM, Heiner Kallweit wrote:
> This adds support for Fast Link Down as new PHY tunable.
> Fast Link Down reduces the time until a link down event is reported
> for 1000BaseT. According to the standard it's 750ms what is too long
> for several use cases.
> 
> v2:
> - add comment describing the constants
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH net-next v2 1/2] ethtool: add PHY Fast Link Down support

[Michal Kubecek]

On Mon, Mar 25, 2019 at 07:34:58PM +0100, Heiner Kallweit wrote:
> This adds support for Fast Link Down as new PHY tunable.
> Fast Link Down reduces the time until a link down event is reported
> for 1000BaseT. According to the standard it's 750ms what is too long
> for several use cases.
> 
> v2:
> - add comment describing the constants
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

> ---
>  include/uapi/linux/ethtool.h | 8 ++++++++
>  net/core/ethtool.c           | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index 3652b239d..50c76f4fa 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -252,9 +252,17 @@ struct ethtool_tunable {
>  #define DOWNSHIFT_DEV_DEFAULT_COUNT	0xff
>  #define DOWNSHIFT_DEV_DISABLE		0
>  
> +/* Time in msecs after which link is reported as down
> + * 0 = lowest time supported by the PHY
> + * 0xff = off, link down detection according to standard
> + */
> +#define ETHTOOL_PHY_FAST_LINK_DOWN_ON	0
> +#define ETHTOOL_PHY_FAST_LINK_DOWN_OFF	0xff
> +
>  enum phy_tunable_id {
>  	ETHTOOL_PHY_ID_UNSPEC,
>  	ETHTOOL_PHY_DOWNSHIFT,
> +	ETHTOOL_PHY_FAST_LINK_DOWN,
>  	/*
>  	 * Add your fresh new phy tunable attribute above and remember to update
>  	 * phy_tunable_strings[] in net/core/ethtool.c
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index b1eb32419..387d67eb7 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -136,6 +136,7 @@ static const char
>  phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
>  	[ETHTOOL_ID_UNSPEC]     = "Unspec",
>  	[ETHTOOL_PHY_DOWNSHIFT]	= "phy-downshift",
> +	[ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down",
>  };
>  
>  static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
> @@ -2432,6 +2433,7 @@ static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
>  {
>  	switch (tuna->id) {
>  	case ETHTOOL_PHY_DOWNSHIFT:
> +	case ETHTOOL_PHY_FAST_LINK_DOWN:
>  		if (tuna->len != sizeof(u8) ||
>  		    tuna->type_id != ETHTOOL_TUNABLE_U8)
>  			return -EINVAL;
> -- 
> 2.21.0
> 
> 

Re: [PATCH net-next v2 2/2] net: phy: marvell: add PHY tunable fast link down support for 88E1540

[Florian Fainelli]

On 3/25/19 11:35 AM, Heiner Kallweit wrote:
> 1000BaseT standard requires that a link is reported as down earliest
> after 750ms. Several use case however require a much faster detecion
> of a broken link. Fast Link Down supports this by intentionally
> violating a the standard. This patch exposes the Fast Link Down
> feature of 88E1540 and 88E6390. These PHY's can be found as internal
> PHY's in several switches: 88E6352, 88E6240, 88E6176, 88E6172,
> and 88E6390(X). Fast Link Down and EEE are mutually exclusive.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

This looks fine, just one question though: do not you need to verify
that the phy_interface_t maps to a copper medium somehow?
-- 
Florian

Re: [PATCH net-next v2 2/2] net: phy: marvell: add PHY tunable fast link down support for 88E1540

[Heiner Kallweit]

On 25.03.2019 19:44, Florian Fainelli wrote:
> On 3/25/19 11:35 AM, Heiner Kallweit wrote:
>> 1000BaseT standard requires that a link is reported as down earliest
>> after 750ms. Several use case however require a much faster detecion
>> of a broken link. Fast Link Down supports this by intentionally
>> violating a the standard. This patch exposes the Fast Link Down
>> feature of 88E1540 and 88E6390. These PHY's can be found as internal
>> PHY's in several switches: 88E6352, 88E6240, 88E6176, 88E6172,
>> and 88E6390(X). Fast Link Down and EEE are mutually exclusive.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> This looks fine, just one question though: do not you need to verify
> that the phy_interface_t maps to a copper medium somehow?
> 
Good question. The register we configure is used in copper mode
only (see name: COPPER_CTRL3). So we don't cause any harm at least
if we configure Fast Link Down when being, let's say, in a fiber mode.
Fast Link Down is applicable for 1000BaseT only, so in theory what
you say would apply also to an attempt to configure Fast Link Down
when being in 100BaseT or 10GBaseT copper modes.
Being allowed to configure a feature for 1000BaseT whilst being in
a different mode can been seen as a feature (what I do) or as a bug.
Most likely there is no 100% answer to this question.

Re: [PATCH net-next v2 2/2] net: phy: marvell: add PHY tunable fast link down support for 88E1540

[Florian Fainelli]

On 3/25/19 12:13 PM, Heiner Kallweit wrote:
> On 25.03.2019 19:44, Florian Fainelli wrote:
>> On 3/25/19 11:35 AM, Heiner Kallweit wrote:
>>> 1000BaseT standard requires that a link is reported as down earliest
>>> after 750ms. Several use case however require a much faster detecion
>>> of a broken link. Fast Link Down supports this by intentionally
>>> violating a the standard. This patch exposes the Fast Link Down
>>> feature of 88E1540 and 88E6390. These PHY's can be found as internal
>>> PHY's in several switches: 88E6352, 88E6240, 88E6176, 88E6172,
>>> and 88E6390(X). Fast Link Down and EEE are mutually exclusive.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> This looks fine, just one question though: do not you need to verify
>> that the phy_interface_t maps to a copper medium somehow?
>>
> Good question. The register we configure is used in copper mode
> only (see name: COPPER_CTRL3). So we don't cause any harm at least
> if we configure Fast Link Down when being, let's say, in a fiber mode.
> Fast Link Down is applicable for 1000BaseT only, so in theory what
> you say would apply also to an attempt to configure Fast Link Down
> when being in 100BaseT or 10GBaseT copper modes.
> Being allowed to configure a feature for 1000BaseT whilst being in
> a different mode can been seen as a feature (what I do) or as a bug.
> Most likely there is no 100% answer to this question.
> 

Agree, and the reasoning about the register we are reading/writing to is
indeed not a problem, it is more about allowing the tunable to be
configured or not, and in fact, this is a policy check that can be done
above the PHY driver itself.
-- 
Florian

Re: [PATCH net-next v2 0/2] ethtool: add support for Fast Link Down as new PHY tunable

[David Miller]

From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Mon, 25 Mar 2019 19:33:39 +0100

> This adds support for Fast Link Down as new PHY tunable.
> Fast Link Down reduces the time until a link down event is reported
> for 1000BaseT. According to the standard it's 750ms what is too long
> for several use cases.
> 
> This is the kernel-related series, the ethtool userspace extension
> I'd submit once the kernel part has been applied.
> 
> v2:
> - add describing comment in patch 1

Series applied, thanks Heiner.