* [PATCH v4 0/2] ethtool: implement Energy Detect Powerdown support via phy-tunable @ 2019-09-12 16:28 Alexandru Ardelean 2019-09-12 16:28 ` [PATCH v4 1/2] " Alexandru Ardelean 2019-09-12 16:28 ` [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode " Alexandru Ardelean 0 siblings, 2 replies; 8+ messages in thread From: Alexandru Ardelean @ 2019-09-12 16:28 UTC (permalink / raw) To: netdev, devicetree, linux-kernel Cc: davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, andrew, mkubecek, Alexandru Ardelean This changeset proposes a new control for PHY tunable to control Energy Detect Power Down. 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. Alexandru Ardelean (2): ethtool: implement Energy Detect Powerdown support via phy-tunable net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable drivers/net/phy/adin.c | 61 ++++++++++++++++++++++++++++++++++++ include/uapi/linux/ethtool.h | 22 +++++++++++++ net/core/ethtool.c | 6 ++++ 3 files changed, 89 insertions(+) -- Changelog v3 -> v4: * impose the TX interval unit for EDPD to be milliseconds; the point was raised by Michal; this should allow for intervals: - as small as 1 millisecond, which does not sound like a power-saver - as large as 65 seconds, which sounds like a lot to wait for a link to come up Changelog v2 -> v3: * implement Andrew's review comments: 1. for patch `ethtool: implement Energy Detect Powerdown support via phy-tunable` - ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL == 0xffff - ETHTOOL_PHY_EDPD_NO_TX == 0xfffe - added comment in include/uapi/linux/ethtool.h 2. for patch `net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable` - added comments about interval & units for the ADIN PHY - in `adin_set_edpd()`: add a switch statement of all the valid values - in `adin_get_edpd()`: return `ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL` since the PHY only supports a single TX-interval value (1 second) Changelog v1 -> v2: * initial series was made up of 2 sub-series: 1 for kernel & 1 for ethtool in userspace; v2 contains only the kernel series 2.20.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 1/2] ethtool: implement Energy Detect Powerdown support via phy-tunable 2019-09-12 16:28 [PATCH v4 0/2] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean @ 2019-09-12 16:28 ` Alexandru Ardelean 2019-09-14 15:26 ` Andrew Lunn 2019-09-15 15:08 ` Florian Fainelli 2019-09-12 16:28 ` [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode " Alexandru Ardelean 1 sibling, 2 replies; 8+ messages in thread From: Alexandru Ardelean @ 2019-09-12 16:28 UTC (permalink / raw) To: netdev, devicetree, linux-kernel Cc: davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, andrew, mkubecek, 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_MSECS` 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 As noted by the `ETHTOOL_PHY_EDPD_DFLT_TX_MSECS` the interval unit is 1 millisecond, which should cover a reasonable range of intervals: - from 1 millisecond, which does not sound like much of a power-saver - to ~65 seconds which is quite a lot to wait for a link to come up when plugging a cable Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- include/uapi/linux/ethtool.h | 22 ++++++++++++++++++++++ net/core/ethtool.c | 6 ++++++ 2 files changed, 28 insertions(+) diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h index dd06302aa93e..8938b76c4ee3 100644 --- a/include/uapi/linux/ethtool.h +++ b/include/uapi/linux/ethtool.h @@ -259,10 +259,32 @@ struct ethtool_tunable { #define ETHTOOL_PHY_FAST_LINK_DOWN_ON 0 #define ETHTOOL_PHY_FAST_LINK_DOWN_OFF 0xff +/* Energy Detect Power Down (EDPD) is a feature supported by some PHYs, where + * the PHY's RX & TX blocks are put into a low-power mode when there is no + * link detected (typically cable is un-plugged). For RX, only a minimal + * link-detection is available, and for TX the PHY wakes up to send link pulses + * to avoid any lock-ups in case the peer PHY may also be running in EDPD mode. + * + * Some PHYs may support configuration of the wake-up interval for TX pulses, + * and some PHYs may support only disabling TX pulses entirely. For the latter + * a special value is required (ETHTOOL_PHY_EDPD_NO_TX) so that this can be + * configured from userspace (should the user want it). + * + * The interval units for TX wake-up are in milliseconds, since this should + * cover a reasonable range of intervals: + * - from 1 millisecond, which does not sound like much of a power-saver + * - to ~65 seconds which is quite a lot to wait for a link to come up when + * plugging a cable + */ +#define ETHTOOL_PHY_EDPD_DFLT_TX_MSECS 0xffff +#define ETHTOOL_PHY_EDPD_NO_TX 0xfffe +#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] 8+ messages in thread
* Re: [PATCH v4 1/2] ethtool: implement Energy Detect Powerdown support via phy-tunable 2019-09-12 16:28 ` [PATCH v4 1/2] " Alexandru Ardelean @ 2019-09-14 15:26 ` Andrew Lunn 2019-09-15 15:08 ` Florian Fainelli 1 sibling, 0 replies; 8+ messages in thread From: Andrew Lunn @ 2019-09-14 15:26 UTC (permalink / raw) To: Alexandru Ardelean Cc: netdev, devicetree, linux-kernel, davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, mkubecek On Thu, Sep 12, 2019 at 07:28:11PM +0300, Alexandru Ardelean wrote: > 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_MSECS` 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 > > As noted by the `ETHTOOL_PHY_EDPD_DFLT_TX_MSECS` the interval unit is 1 > millisecond, which should cover a reasonable range of intervals: > - from 1 millisecond, which does not sound like much of a power-saver > - to ~65 seconds which is quite a lot to wait for a link to come up when > plugging a cable > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 1/2] ethtool: implement Energy Detect Powerdown support via phy-tunable 2019-09-12 16:28 ` [PATCH v4 1/2] " Alexandru Ardelean 2019-09-14 15:26 ` Andrew Lunn @ 2019-09-15 15:08 ` Florian Fainelli 1 sibling, 0 replies; 8+ messages in thread From: Florian Fainelli @ 2019-09-15 15:08 UTC (permalink / raw) To: Alexandru Ardelean, netdev, devicetree, linux-kernel Cc: davem, robh+dt, mark.rutland, hkallweit1, andrew, mkubecek On 9/12/2019 9:28 AM, Alexandru Ardelean wrote: > 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_MSECS` 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 > > As noted by the `ETHTOOL_PHY_EDPD_DFLT_TX_MSECS` the interval unit is 1 > millisecond, which should cover a reasonable range of intervals: > - from 1 millisecond, which does not sound like much of a power-saver > - to ~65 seconds which is quite a lot to wait for a link to come up when > plugging a cable > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable 2019-09-12 16:28 [PATCH v4 0/2] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean 2019-09-12 16:28 ` [PATCH v4 1/2] " Alexandru Ardelean @ 2019-09-12 16:28 ` Alexandru Ardelean 2019-09-14 15:29 ` Andrew Lunn 1 sibling, 1 reply; 8+ messages in thread From: Alexandru Ardelean @ 2019-09-12 16:28 UTC (permalink / raw) To: netdev, devicetree, linux-kernel Cc: davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, andrew, mkubecek, 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. The ADIN PHY supports only fixed 1 second intervals; they cannot be configured. That is why the acceptable values are 1, ETHTOOL_PHY_EDPD_DFLT_TX_MSECS and ETHTOOL_PHY_EDPD_NO_TX (which disables TX pulses). Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- drivers/net/phy/adin.c | 61 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c index 4dec83df048d..9afeee67675b 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,62 @@ 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) + /* default is 1 second */ + *tx_interval = ETHTOOL_PHY_EDPD_DFLT_TX_MSECS; + 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; + + switch (tx_interval) { + case 1000: /* 1 second */ + /* fallthrough */ + case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS: + val |= ADIN1300_NRG_PD_TX_EN; + /* fallthrough */ + case ETHTOOL_PHY_EDPD_NO_TX: + break; + default: + return -EINVAL; + } + + 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 +400,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 +425,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] 8+ messages in thread
* Re: [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable 2019-09-12 16:28 ` [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode " Alexandru Ardelean @ 2019-09-14 15:29 ` Andrew Lunn 2019-09-15 15:11 ` Florian Fainelli 0 siblings, 1 reply; 8+ messages in thread From: Andrew Lunn @ 2019-09-14 15:29 UTC (permalink / raw) To: Alexandru Ardelean Cc: netdev, devicetree, linux-kernel, davem, robh+dt, mark.rutland, f.fainelli, hkallweit1, mkubecek On Thu, Sep 12, 2019 at 07:28:12PM +0300, Alexandru Ardelean wrote: > +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; > + > + switch (tx_interval) { > + case 1000: /* 1 second */ > + /* fallthrough */ > + case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS: > + val |= ADIN1300_NRG_PD_TX_EN; > + /* fallthrough */ > + case ETHTOOL_PHY_EDPD_NO_TX: > + break; > + default: > + return -EINVAL; > + } > + > + return phy_modify(phydev, ADIN1300_PHY_CTRL_STATUS2, > + (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN), > + val); > +} > + > > + rc = adin_set_edpd(phydev, 1); > + if (rc < 0) > + return rc; Hi Alexandru Shouldn't this be adin_set_edpd(phydev, 1000); Andrew ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable 2019-09-14 15:29 ` Andrew Lunn @ 2019-09-15 15:11 ` Florian Fainelli 2019-09-16 6:44 ` Ardelean, Alexandru 0 siblings, 1 reply; 8+ messages in thread From: Florian Fainelli @ 2019-09-15 15:11 UTC (permalink / raw) To: Andrew Lunn, Alexandru Ardelean Cc: netdev, devicetree, linux-kernel, davem, robh+dt, mark.rutland, hkallweit1, mkubecek On 9/14/2019 8:29 AM, Andrew Lunn wrote: > On Thu, Sep 12, 2019 at 07:28:12PM +0300, Alexandru Ardelean wrote: > >> +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; >> + >> + switch (tx_interval) { >> + case 1000: /* 1 second */ >> + /* fallthrough */ >> + case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS: >> + val |= ADIN1300_NRG_PD_TX_EN; >> + /* fallthrough */ >> + case ETHTOOL_PHY_EDPD_NO_TX: >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + return phy_modify(phydev, ADIN1300_PHY_CTRL_STATUS2, >> + (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN), >> + val); >> +} >> + > >> >> + rc = adin_set_edpd(phydev, 1); >> + if (rc < 0) >> + return rc; > > Hi Alexandru > > Shouldn't this be adin_set_edpd(phydev, 1000); That does sound like the intended use, or use ETHTOOL_PHY_EDPD_DFLT_TX_MSECS, with that fixed: Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable 2019-09-15 15:11 ` Florian Fainelli @ 2019-09-16 6:44 ` Ardelean, Alexandru 0 siblings, 0 replies; 8+ messages in thread From: Ardelean, Alexandru @ 2019-09-16 6:44 UTC (permalink / raw) To: f.fainelli, andrew Cc: mkubecek, davem, hkallweit1, devicetree, mark.rutland, linux-kernel, netdev, robh+dt On Sun, 2019-09-15 at 08:11 -0700, Florian Fainelli wrote: > [External] > > > > On 9/14/2019 8:29 AM, Andrew Lunn wrote: > > On Thu, Sep 12, 2019 at 07:28:12PM +0300, Alexandru Ardelean wrote: > > > > > +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; > > > + > > > + switch (tx_interval) { > > > + case 1000: /* 1 second */ > > > + /* fallthrough */ > > > + case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS: > > > + val |= ADIN1300_NRG_PD_TX_EN; > > > + /* fallthrough */ > > > + case ETHTOOL_PHY_EDPD_NO_TX: > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + return phy_modify(phydev, ADIN1300_PHY_CTRL_STATUS2, > > > + (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN), > > > + val); > > > +} > > > + > > > > > > + rc = adin_set_edpd(phydev, 1); > > > + if (rc < 0) > > > + return rc; > > > > Hi Alexandru > > > > Shouldn't this be adin_set_edpd(phydev, 1000); > > That does sound like the intended use, or use > ETHTOOL_PHY_EDPD_DFLT_TX_MSECS, with that fixed: Ack. Many thanks for catching this. I missed it when re-spinning. > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-09-16 6:44 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-12 16:28 [PATCH v4 0/2] ethtool: implement Energy Detect Powerdown support via phy-tunable Alexandru Ardelean 2019-09-12 16:28 ` [PATCH v4 1/2] " Alexandru Ardelean 2019-09-14 15:26 ` Andrew Lunn 2019-09-15 15:08 ` Florian Fainelli 2019-09-12 16:28 ` [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode " Alexandru Ardelean 2019-09-14 15:29 ` Andrew Lunn 2019-09-15 15:11 ` Florian Fainelli 2019-09-16 6:44 ` Ardelean, Alexandru
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).