linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable
@ 2019-09-03 16:06 Alexandru Ardelean
  2019-09-03 16:06 ` [PATCH 1/4] " Alexandru Ardelean
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean

This patch series is actually 2 series in 1.

First 2 patches implement the kernel support for controlling Energy Detect
Powerdown support via phy-tunable, and the next 2 patches implement the
ethtool user-space control.
Hopefully, this combination of 2 series is an acceptable approach; if not,
I am fine to re-update it based on feedback.

The `phy_tunable_id` has been named `ETHTOOL_PHY_EDPD` since it looks like
this feature is common across other PHYs (like EEE), and defining
`ETHTOOL_PHY_ENERGY_DETECT_POWER_DOWN` seems too long.
    
The way EDPD works, is that the RX block is put to a lower power mode,
except for link-pulse detection circuits. The TX block is also put to low
power mode, but the PHY wakes-up periodically to send link pulses, to avoid
lock-ups in case the other side is also in EDPD mode.
    
Currently, there are 2 PHY drivers that look like they could use this new
PHY tunable feature: the `adin` && `micrel` PHYs.

This series updates only the `adin` PHY driver to support this new feature,
as this chip has been tested. A change for `micrel` can be proposed after a
discussion of the PHY-tunable API is resolved.

-- 
2.20.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] ethtool: implement Energy Detect Powerdown support via phy-tunable
  2019-09-03 16:06 [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean
@ 2019-09-03 16:06 ` Alexandru Ardelean
  2019-09-03 16:06 ` [PATCH 2/4] net: phy: adin: implement Energy Detect Powerdown mode " Alexandru Ardelean
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean

The `phy_tunable_id` has been named `ETHTOOL_PHY_EDPD` since it looks like
this feature is common across other PHYs (like EEE), and defining
`ETHTOOL_PHY_ENERGY_DETECT_POWER_DOWN` seems too long.

The way EDPD works, is that the RX block is put to a lower power mode,
except for link-pulse detection circuits. The TX block is also put to low
power mode, but the PHY wakes-up periodically to send link pulses, to avoid
lock-ups in case the other side is also in EDPD mode.

Currently, there are 2 PHY drivers that look like they could use this new
PHY tunable feature: the `adin` && `micrel` PHYs.

The ADIN's datasheet mentions that TX pulses are at intervals of 1 second
default each, and they can be disabled. For the Micrel KSZ9031 PHY, the
datasheet does not mention whether they can be disabled, but mentions that
they can modified.

The way this change is structured, is similar to the PHY tunable downshift
control:
* a `ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL` value is exposed to cover a default
  TX interval; some PHYs could specify a certain value that makes sense
* `ETHTOOL_PHY_EDPD_NO_TX` would disable TX when EDPD is enabled
* `ETHTOOL_PHY_EDPD_DISABLE` will disable EDPD

This should allow PHYs to:
* enable EDPD and not enable TX pulses (interval would be 0)
* enable EDPD and configure TX pulse interval; note that TX interval units
  would be PHY specific; we could consider `seconds` as units, but it could
  happen that some PHYs would be prefer 500 milliseconds as a unit;
  a maximum of 32766 units should be sufficient
* disable EDPD

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 include/uapi/linux/ethtool.h | 5 +++++
 net/core/ethtool.c           | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index dd06302aa93e..0349e9c4350f 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -259,10 +259,15 @@ struct ethtool_tunable {
 #define ETHTOOL_PHY_FAST_LINK_DOWN_ON	0
 #define ETHTOOL_PHY_FAST_LINK_DOWN_OFF	0xff
 
+#define ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL	0x7fff
+#define ETHTOOL_PHY_EDPD_NO_TX			0x8000
+#define ETHTOOL_PHY_EDPD_DISABLE		0
+
 enum phy_tunable_id {
 	ETHTOOL_PHY_ID_UNSPEC,
 	ETHTOOL_PHY_DOWNSHIFT,
 	ETHTOOL_PHY_FAST_LINK_DOWN,
+	ETHTOOL_PHY_EDPD,
 	/*
 	 * 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 6288e69e94fc..c763106c73fc 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -133,6 +133,7 @@ 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",
+	[ETHTOOL_PHY_EDPD]	= "phy-energy-detect-power-down",
 };
 
 static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
@@ -2451,6 +2452,11 @@ static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
 		    tuna->type_id != ETHTOOL_TUNABLE_U8)
 			return -EINVAL;
 		break;
+	case ETHTOOL_PHY_EDPD:
+		if (tuna->len != sizeof(u16) ||
+		    tuna->type_id != ETHTOOL_TUNABLE_U16)
+			return -EINVAL;
+		break;
 	default:
 		return -EINVAL;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/4] net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable
  2019-09-03 16:06 [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean
  2019-09-03 16:06 ` [PATCH 1/4] " Alexandru Ardelean
@ 2019-09-03 16:06 ` Alexandru Ardelean
  2019-09-03 16:06 ` [PATCH 3/4] [ethtool] ethtool: sync ethtool-copy.h: adds support for EDPD (3rd Sep 2019) Alexandru Ardelean
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean

This driver becomes the first user of the kernel's `ETHTOOL_PHY_EDPD`
phy-tunable feature.
EDPD is also enabled by default on PHY config_init, but can be disabled via
the phy-tunable control.

When enabling EDPD, it's also a good idea (for the ADIN PHYs) to enable TX
periodic pulses, so that in case the other PHY is also on EDPD mode, there
is no lock-up situation where both sides are waiting for the other to
transmit.

Via the phy-tunable control, TX pulses can be disabled if specifying 0
`tx-interval` via ethtool.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/net/phy/adin.c | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 4dec83df048d..742728ab2a5d 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -26,6 +26,11 @@
 
 #define ADIN1300_RX_ERR_CNT			0x0014
 
+#define ADIN1300_PHY_CTRL_STATUS2		0x0015
+#define   ADIN1300_NRG_PD_EN			BIT(3)
+#define   ADIN1300_NRG_PD_TX_EN			BIT(2)
+#define   ADIN1300_NRG_PD_STATUS		BIT(1)
+
 #define ADIN1300_PHY_CTRL2			0x0016
 #define   ADIN1300_DOWNSPEED_AN_100_EN		BIT(11)
 #define   ADIN1300_DOWNSPEED_AN_10_EN		BIT(10)
@@ -328,12 +333,51 @@ static int adin_set_downshift(struct phy_device *phydev, u8 cnt)
 			    ADIN1300_DOWNSPEEDS_EN);
 }
 
+static int adin_get_edpd(struct phy_device *phydev, u16 *tx_interval)
+{
+	int val;
+
+	val = phy_read(phydev, ADIN1300_PHY_CTRL_STATUS2);
+	if (val < 0)
+		return val;
+
+	if (ADIN1300_NRG_PD_EN & val) {
+		if (val & ADIN1300_NRG_PD_TX_EN)
+			*tx_interval = 1;
+		else
+			*tx_interval = ETHTOOL_PHY_EDPD_NO_TX;
+	} else {
+		*tx_interval = ETHTOOL_PHY_EDPD_DISABLE;
+	}
+
+	return 0;
+}
+
+static int adin_set_edpd(struct phy_device *phydev, u16 tx_interval)
+{
+	u16 val;
+
+	if (tx_interval == ETHTOOL_PHY_EDPD_DISABLE)
+		return phy_clear_bits(phydev, ADIN1300_PHY_CTRL_STATUS2,
+				(ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN));
+
+	val = ADIN1300_NRG_PD_EN;
+	if (tx_interval != ETHTOOL_PHY_EDPD_NO_TX)
+		val |= ADIN1300_NRG_PD_TX_EN;
+
+	return phy_modify(phydev, ADIN1300_PHY_CTRL_STATUS2,
+			  (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN),
+			  val);
+}
+
 static int adin_get_tunable(struct phy_device *phydev,
 			    struct ethtool_tunable *tuna, void *data)
 {
 	switch (tuna->id) {
 	case ETHTOOL_PHY_DOWNSHIFT:
 		return adin_get_downshift(phydev, data);
+	case ETHTOOL_PHY_EDPD:
+		return adin_get_edpd(phydev, data);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -345,6 +389,8 @@ static int adin_set_tunable(struct phy_device *phydev,
 	switch (tuna->id) {
 	case ETHTOOL_PHY_DOWNSHIFT:
 		return adin_set_downshift(phydev, *(const u8 *)data);
+	case ETHTOOL_PHY_EDPD:
+		return adin_set_edpd(phydev, *(const u16 *)data);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -368,6 +414,10 @@ static int adin_config_init(struct phy_device *phydev)
 	if (rc < 0)
 		return rc;
 
+	rc = adin_set_edpd(phydev, 1);
+	if (rc < 0)
+		return rc;
+
 	phydev_dbg(phydev, "PHY is using mode '%s'\n",
 		   phy_modes(phydev->interface));
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/4] [ethtool] ethtool: sync ethtool-copy.h: adds support for EDPD (3rd Sep 2019)
  2019-09-03 16:06 [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean
  2019-09-03 16:06 ` [PATCH 1/4] " Alexandru Ardelean
  2019-09-03 16:06 ` [PATCH 2/4] net: phy: adin: implement Energy Detect Powerdown mode " Alexandru Ardelean
@ 2019-09-03 16:06 ` Alexandru Ardelean
  2019-09-03 16:06 ` [PATCH 4/4] [ethtool] ethtool: implement support for Energy Detect Power Down Alexandru Ardelean
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean

This change syncs the `ethtool-copy.h` file with Linux net-next to add
support for Energy Detect Powerdown control via phy tunable.

Some formatting also changes with this sync.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 ethtool-copy.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index ad16e8f..8b9a87d 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -257,10 +257,15 @@ struct ethtool_tunable {
 #define ETHTOOL_PHY_FAST_LINK_DOWN_ON	0
 #define ETHTOOL_PHY_FAST_LINK_DOWN_OFF	0xff
 
+#define ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL	0x7fff
+#define ETHTOOL_PHY_EDPD_NO_TX			0x8000
+#define ETHTOOL_PHY_EDPD_DISABLE		0
+
 enum phy_tunable_id {
 	ETHTOOL_PHY_ID_UNSPEC,
 	ETHTOOL_PHY_DOWNSHIFT,
 	ETHTOOL_PHY_FAST_LINK_DOWN,
+	ETHTOOL_PHY_EDPD,
 	/*
 	 * Add your fresh new phy tunable attribute above and remember to update
 	 * phy_tunable_strings[] in net/core/ethtool.c
@@ -1481,8 +1486,8 @@ enum ethtool_link_mode_bit_indices {
 	ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT = 64,
 	ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT	 = 65,
 	ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT	 = 66,
-	ETHTOOL_LINK_MODE_100baseT1_Full_BIT             = 67,
-	ETHTOOL_LINK_MODE_1000baseT1_Full_BIT            = 68,
+	ETHTOOL_LINK_MODE_100baseT1_Full_BIT		 = 67,
+	ETHTOOL_LINK_MODE_1000baseT1_Full_BIT		 = 68,
 
 	/* must be last entry */
 	__ETHTOOL_LINK_MODE_MASK_NBITS
@@ -1712,8 +1717,8 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
 #define ETH_MODULE_SFF_8436		0x4
 #define ETH_MODULE_SFF_8436_LEN		256
 
-#define ETH_MODULE_SFF_8636_MAX_LEN	640
-#define ETH_MODULE_SFF_8436_MAX_LEN	640
+#define ETH_MODULE_SFF_8636_MAX_LEN     640
+#define ETH_MODULE_SFF_8436_MAX_LEN     640
 
 /* Reset flags */
 /* The reset() operation must clear the flags for the components which
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/4] [ethtool] ethtool: implement support for Energy Detect Power Down
  2019-09-03 16:06 [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean
                   ` (2 preceding siblings ...)
  2019-09-03 16:06 ` [PATCH 3/4] [ethtool] ethtool: sync ethtool-copy.h: adds support for EDPD (3rd Sep 2019) Alexandru Ardelean
@ 2019-09-03 16:06 ` Alexandru Ardelean
  2019-09-03 17:50 ` [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable John W. Linville
  2019-09-03 22:41 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean

This change adds control for enabling/disabling Energy Detect Power Down
mode, as well as configuring wake-up intervals for TX pulses, via the new
ETHTOOL_PHY_EDPD control added in PHY tunable, in the kernel.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 ethtool.8.in | 28 +++++++++++++++++
 ethtool.c    | 87 +++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index cd3be91..a32d48b 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -362,11 +362,17 @@ ethtool \- query or control network driver and hardware settings
 .A1 on off
 .BN msecs
 .RB ]
+.RB [
+.B energy\-detect\-power\-down
+.A1 on off
+.BN tx-interval
+.RB ]
 .HP
 .B ethtool \-\-get\-phy\-tunable
 .I devname
 .RB [ downshift ]
 .RB [ fast-link-down ]
+.RB [ energy-detect-power-down ]
 .HP
 .B ethtool \-\-reset
 .I devname
@@ -1100,6 +1106,24 @@ lB	l.
 	Sets the period after which the link is reported as down. Note that the PHY may choose
 	the closest supported value. Only on reading back the tunable do you get the actual value.
 .TE
+.TP
+.A2 energy-detect-power-down on off
+Specifies whether Energy Detect Power Down (EDPD) should be enabled (if supported).
+This will put the RX and TX circuit blocks into a low power mode, and the PHY will
+wake up periodically to send link pulses to avoid any lock-up situation with a peer
+PHY that may also have EDPD enabled. By default, this setting will also enable the
+periodic transmission of TX pulses.
+.TS
+nokeep;
+lB	l.
+.BI tx-interval \ N
+	Some PHYs support configuration of the wake-up interval to send TX pulses.
+	This setting allows the control of this interval, and 0 disables TX pulses
+	if the PHY supports this. Disabling TX pulses can create a lock-up situation
+	where neither of the PHYs wakes the other one. If the PHY supports only
+	a single interval, any non-zero value will enable this.
+.TE
+.TP
 .PD
 .RE
 .TP
@@ -1122,6 +1146,10 @@ Some PHYs support a Fast Link Down Feature and may allow configuration of the de
 before a broken link is reported as being down.
 
 Gets the PHY Fast Link Down status / period.
+.TP
+.B energy\-detect\-power\-down
+Gets the current configured setting for Energy Detect Power Down (if supported).
+
 .RE
 .TP
 .B \-\-reset
diff --git a/ethtool.c b/ethtool.c
index c0e2903..c0a18f8 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4897,6 +4897,30 @@ static int do_get_phy_tunable(struct cmd_context *ctx)
 		else
 			fprintf(stdout, "Fast Link Down enabled, %d msecs\n",
 				cont.msecs);
+	} else if (!strcmp(argp[0], "energy-detect-power-down")) {
+		struct {
+			struct ethtool_tunable ds;
+			u16 tx_interval;
+		} cont;
+
+		cont.ds.cmd = ETHTOOL_PHY_GTUNABLE;
+		cont.ds.id = ETHTOOL_PHY_EDPD;
+		cont.ds.type_id = ETHTOOL_TUNABLE_U16;
+		cont.ds.len = 2;
+		if (send_ioctl(ctx, &cont.ds) < 0) {
+			perror("Cannot Get PHY Energy Detect Power Down value");
+			return 87;
+		}
+
+		if (cont.tx_interval == ETHTOOL_PHY_EDPD_DISABLE)
+			fprintf(stdout, "Energy Detect Power Down: disabled\n");
+		else if (cont.tx_interval == ETHTOOL_PHY_EDPD_NO_TX)
+			fprintf(stdout,
+				"Energy Detect Power Down: enabled, TX disabled\n");
+		else
+			fprintf(stdout,
+				"Energy Detect Power Down: enabled, TX %u intervals\n",
+				cont.tx_interval);
 	} else {
 		exit_bad_args();
 	}
@@ -5018,7 +5042,8 @@ static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
 	return 1;
 }
 
-static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+static int parse_named_uint(struct cmd_context *ctx, const char *name,
+			    void *val, enum tunable_type_id type_id)
 {
 	if (ctx->argc < 2)
 		return 0;
@@ -5026,7 +5051,16 @@ static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
 	if (strcmp(*ctx->argp, name))
 		return 0;
 
-	*val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+	switch (type_id) {
+	case ETHTOOL_TUNABLE_U8:
+		*(u8 *)val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+		break;
+	case ETHTOOL_TUNABLE_U16:
+		*(u16 *)val = get_uint_range(*(ctx->argp + 1), 0, 0xffff);
+		break;
+	default:
+		return 0;
+	}
 
 	ctx->argc -= 2;
 	ctx->argp += 2;
@@ -5034,6 +5068,16 @@ static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
 	return 1;
 }
 
+static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+{
+	return parse_named_uint(ctx, name, val, ETHTOOL_TUNABLE_U8);
+}
+
+static int parse_named_u16(struct cmd_context *ctx, const char *name, u16 *val)
+{
+	return parse_named_uint(ctx, name, val, ETHTOOL_TUNABLE_U16);
+}
+
 static int do_set_phy_tunable(struct cmd_context *ctx)
 {
 	int err = 0;
@@ -5041,6 +5085,8 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
 	u8 ds_changed = 0, ds_has_cnt = 0, ds_enable = 0;
 	u8 fld_changed = 0, fld_enable = 0;
 	u8 fld_msecs = ETHTOOL_PHY_FAST_LINK_DOWN_ON;
+	u8 edpd_changed = 0, edpd_enable = 0;
+	u16 edpd_tx_interval = ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL;
 
 	/* Parse arguments */
 	if (parse_named_bool(ctx, "downshift", &ds_enable)) {
@@ -5050,6 +5096,11 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
 		fld_changed = 1;
 		if (fld_enable)
 			parse_named_u8(ctx, "msecs", &fld_msecs);
+	} else if (parse_named_bool(ctx, "energy-detect-power-down",
+				    &edpd_enable)) {
+		edpd_changed = 1;
+		if (edpd_enable)
+			parse_named_u16(ctx, "tx-interval", &edpd_tx_interval);
 	} else {
 		exit_bad_args();
 	}
@@ -5074,6 +5125,16 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
 			fld_msecs = ETHTOOL_PHY_FAST_LINK_DOWN_OFF;
 		else if (fld_msecs == ETHTOOL_PHY_FAST_LINK_DOWN_OFF)
 			exit_bad_args();
+	} else if (edpd_changed) {
+		if (!edpd_enable)
+			edpd_tx_interval = ETHTOOL_PHY_EDPD_DISABLE;
+		else if (edpd_tx_interval == 0)
+			edpd_tx_interval = ETHTOOL_PHY_EDPD_NO_TX;
+		else if (edpd_tx_interval > ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL) {
+			fprintf(stderr, "'tx-interval' max value is %d.\n",
+				(ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL - 1));
+			exit_bad_args();
+		}
 	}
 
 	/* Do it */
@@ -5109,6 +5170,22 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
 			perror("Cannot Set PHY Fast Link Down value");
 			err = 87;
 		}
+	} else if (edpd_changed) {
+		struct {
+			struct ethtool_tunable fld;
+			u16 tx_interval;
+		} cont;
+
+		cont.fld.cmd = ETHTOOL_PHY_STUNABLE;
+		cont.fld.id = ETHTOOL_PHY_EDPD;
+		cont.fld.type_id = ETHTOOL_TUNABLE_U16;
+		cont.fld.len = 2;
+		cont.tx_interval = edpd_tx_interval;
+		err = send_ioctl(ctx, &cont.fld);
+		if (err < 0) {
+			perror("Cannot Set PHY Energy Detect Power Down");
+			err = 87;
+		}
 	}
 
 	return err;
@@ -5361,10 +5438,12 @@ static const struct option {
 	  "		[ tx-timer %d ]\n"},
 	{ "--set-phy-tunable", 1, do_set_phy_tunable, "Set PHY tunable",
 	  "		[ downshift on|off [count N] ]\n"
-	  "		[ fast-link-down on|off [msecs N] ]\n"},
+	  "		[ fast-link-down on|off [msecs N] ]\n"
+	  "		[ energy-detect-power-down on|off [tx-interval N] ]\n"},
 	{ "--get-phy-tunable", 1, do_get_phy_tunable, "Get PHY tunable",
 	  "		[ downshift ]\n"
-	  "		[ fast-link-down ]\n"},
+	  "		[ fast-link-down ]\n"
+	  "		[ energy-detect-power-down ]\n"},
 	{ "--reset", 1, do_reset, "Reset components",
 	  "		[ flags %x ]\n"
 	  "		[ mgmt ]\n"
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable
  2019-09-03 16:06 [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean
                   ` (3 preceding siblings ...)
  2019-09-03 16:06 ` [PATCH 4/4] [ethtool] ethtool: implement support for Energy Detect Power Down Alexandru Ardelean
@ 2019-09-03 17:50 ` John W. Linville
  2019-09-03 22:41 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: John W. Linville @ 2019-09-03 17:50 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: netdev, linux-kernel, andrew, f.fainelli, hkallweit1, davem

On Tue, Sep 03, 2019 at 07:06:22PM +0300, Alexandru Ardelean wrote:
> This patch series is actually 2 series in 1.
> 
> First 2 patches implement the kernel support for controlling Energy Detect
> Powerdown support via phy-tunable, and the next 2 patches implement the
> ethtool user-space control.
> Hopefully, this combination of 2 series is an acceptable approach; if not,
> I am fine to re-update it based on feedback.

I understand your reasoning, but do keep in mind that userland ethtool
and the kernel are managed in different git trees. Seperate patchsets
would be preferable in general, although in some cases having an
initial userland implementation to show against proposed kernel
changes could be helpful.

It would not be unusual for someone to ask for changes on the kernel
patches. If that happens, just repost the kernel changes until you get
a final merge. Once that happens, then repost the userland patches as
a seperate patchset. But I'll keep an eye here -- if Dave merges the
existing kernel patches as-is, I can take the already posted patchs
(unless problems are found in code review).

John
 
> The `phy_tunable_id` has been named `ETHTOOL_PHY_EDPD` since it looks like
> this feature is common across other PHYs (like EEE), and defining
> `ETHTOOL_PHY_ENERGY_DETECT_POWER_DOWN` seems too long.
>     
> The way EDPD works, is that the RX block is put to a lower power mode,
> except for link-pulse detection circuits. The TX block is also put to low
> power mode, but the PHY wakes-up periodically to send link pulses, to avoid
> lock-ups in case the other side is also in EDPD mode.
>     
> Currently, there are 2 PHY drivers that look like they could use this new
> PHY tunable feature: the `adin` && `micrel` PHYs.
> 
> This series updates only the `adin` PHY driver to support this new feature,
> as this chip has been tested. A change for `micrel` can be proposed after a
> discussion of the PHY-tunable API is resolved.
> 
> -- 
> 2.20.1
> 
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable
  2019-09-03 16:06 [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean
                   ` (4 preceding siblings ...)
  2019-09-03 17:50 ` [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable John W. Linville
@ 2019-09-03 22:41 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2019-09-03 22:41 UTC (permalink / raw)
  To: alexandru.ardelean; +Cc: netdev, linux-kernel, andrew, f.fainelli, hkallweit1

From: Alexandru Ardelean <alexandru.ardelean@analog.com>
Date: Tue, 3 Sep 2019 19:06:22 +0300

> First 2 patches implement the kernel support for controlling Energy Detect
> Powerdown support via phy-tunable, and the next 2 patches implement the
> ethtool user-space control.

You should do this as two separate patch series, one for the kernel
portion and one for the ethtool part.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-09-03 22:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 16:06 [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean
2019-09-03 16:06 ` [PATCH 1/4] " Alexandru Ardelean
2019-09-03 16:06 ` [PATCH 2/4] net: phy: adin: implement Energy Detect Powerdown mode " Alexandru Ardelean
2019-09-03 16:06 ` [PATCH 3/4] [ethtool] ethtool: sync ethtool-copy.h: adds support for EDPD (3rd Sep 2019) Alexandru Ardelean
2019-09-03 16:06 ` [PATCH 4/4] [ethtool] ethtool: implement support for Energy Detect Power Down Alexandru Ardelean
2019-09-03 17:50 ` [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable John W. Linville
2019-09-03 22:41 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).