* [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable
@ 2023-04-02 9:43 Heiner Kallweit
2023-04-02 9:44 ` [PATCH net-next 1/7] net: phy: smsc: rename flag energy_enable Heiner Kallweit
` (7 more replies)
0 siblings, 8 replies; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 9:43 UTC (permalink / raw)
To: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski
Cc: netdev
This adds support for the EDPD PHY tunable.
Per default EDPD is disabled in interrupt mode, the tunable can be used
to override this, e.g. if the link partner doesn't use EDPD.
The interval to check for energy can be chosen between 1000ms and
2000ms. Note that this value consists of the 1000ms phylib interval
for state machine runs plus the time to wait for energy being detected.
Heiner Kallweit (7):
net: phy: smsc: rename flag energy_enable
net: phy: smsc: add helper smsc_phy_config_edpd
net: phy: smsc: clear edpd_enable if interrupt mode is used
net: phy: smsc: add flag edpd_mode_set_by_user
net: phy: smss: prepare for making edpd wait period configurable
net: phy: smsc: add edpd tunable support
net: phy: smsc: enable edpd tunable support
drivers/net/phy/smsc.c | 138 ++++++++++++++++++++++++++++++++++++----
include/linux/smscphy.h | 4 ++
2 files changed, 131 insertions(+), 11 deletions(-)
--
2.40.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next 1/7] net: phy: smsc: rename flag energy_enable
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
@ 2023-04-02 9:44 ` Heiner Kallweit
2023-04-02 9:45 ` [PATCH net-next 2/7] net: phy: smsc: add helper smsc_phy_config_edpd Heiner Kallweit
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 9:44 UTC (permalink / raw)
To: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski
Cc: netdev
Rename the flag to edpd_enable, as we're not enabling energy but
edpd (energy detect power down) mode. In addition change the
type to a bit field member in preparation of adding further flags.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/smsc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 730964b85..928cf6d8b 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -44,7 +44,7 @@ static struct smsc_hw_stat smsc_hw_stats[] = {
};
struct smsc_phy_priv {
- bool energy_enable;
+ unsigned int edpd_enable:1;
};
static int smsc_phy_ack_interrupt(struct phy_device *phydev)
@@ -102,7 +102,7 @@ int smsc_phy_config_init(struct phy_device *phydev)
{
struct smsc_phy_priv *priv = phydev->priv;
- if (!priv || !priv->energy_enable || phydev->irq != PHY_POLL)
+ if (!priv || !priv->edpd_enable || phydev->irq != PHY_POLL)
return 0;
/* Enable energy detect power down mode */
@@ -198,7 +198,7 @@ int lan87xx_read_status(struct phy_device *phydev)
if (err)
return err;
- if (!phydev->link && priv && priv->energy_enable &&
+ if (!phydev->link && priv && priv->edpd_enable &&
phydev->irq == PHY_POLL) {
/* Disable EDPD to wake up PHY */
int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
@@ -284,10 +284,10 @@ int smsc_phy_probe(struct phy_device *phydev)
if (!priv)
return -ENOMEM;
- priv->energy_enable = true;
+ priv->edpd_enable = true;
if (device_property_present(dev, "smsc,disable-energy-detect"))
- priv->energy_enable = false;
+ priv->edpd_enable = false;
phydev->priv = priv;
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 2/7] net: phy: smsc: add helper smsc_phy_config_edpd
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
2023-04-02 9:44 ` [PATCH net-next 1/7] net: phy: smsc: rename flag energy_enable Heiner Kallweit
@ 2023-04-02 9:45 ` Heiner Kallweit
2023-04-02 9:45 ` [PATCH net-next 3/7] net: phy: smsc: clear edpd_enable if interrupt mode is used Heiner Kallweit
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 9:45 UTC (permalink / raw)
To: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski
Cc: netdev
Add helper smsc_phy_config_edpd() and explicitly clear bit
MII_LAN83C185_EDPWRDOWN is edpd_enable isn't set.
Boot loader may have left whatever value.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/smsc.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 928cf6d8b..1b588366e 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -77,6 +77,18 @@ int smsc_phy_config_intr(struct phy_device *phydev)
}
EXPORT_SYMBOL_GPL(smsc_phy_config_intr);
+static int smsc_phy_config_edpd(struct phy_device *phydev)
+{
+ struct smsc_phy_priv *priv = phydev->priv;
+
+ if (priv->edpd_enable)
+ return phy_set_bits(phydev, MII_LAN83C185_CTRL_STATUS,
+ MII_LAN83C185_EDPWRDOWN);
+ else
+ return phy_clear_bits(phydev, MII_LAN83C185_CTRL_STATUS,
+ MII_LAN83C185_EDPWRDOWN);
+}
+
irqreturn_t smsc_phy_handle_interrupt(struct phy_device *phydev)
{
int irq_status;
@@ -105,9 +117,7 @@ int smsc_phy_config_init(struct phy_device *phydev)
if (!priv || !priv->edpd_enable || phydev->irq != PHY_POLL)
return 0;
- /* Enable energy detect power down mode */
- return phy_set_bits(phydev, MII_LAN83C185_CTRL_STATUS,
- MII_LAN83C185_EDPWRDOWN);
+ return smsc_phy_config_edpd(phydev);
}
EXPORT_SYMBOL_GPL(smsc_phy_config_init);
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 3/7] net: phy: smsc: clear edpd_enable if interrupt mode is used
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
2023-04-02 9:44 ` [PATCH net-next 1/7] net: phy: smsc: rename flag energy_enable Heiner Kallweit
2023-04-02 9:45 ` [PATCH net-next 2/7] net: phy: smsc: add helper smsc_phy_config_edpd Heiner Kallweit
@ 2023-04-02 9:45 ` Heiner Kallweit
2023-04-02 9:46 ` [PATCH net-next 4/7] net: phy: smsc: add flag edpd_mode_set_by_user Heiner Kallweit
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 9:45 UTC (permalink / raw)
To: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski
Cc: netdev
Clear edpd_enable if interrupt mode is used, this avoids
having to check for PHY_POLL multiple times.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/smsc.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 1b588366e..f5ecd8bea 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -114,9 +114,12 @@ int smsc_phy_config_init(struct phy_device *phydev)
{
struct smsc_phy_priv *priv = phydev->priv;
- if (!priv || !priv->edpd_enable || phydev->irq != PHY_POLL)
+ if (!priv)
return 0;
+ if (phydev->irq != PHY_POLL)
+ priv->edpd_enable = false;
+
return smsc_phy_config_edpd(phydev);
}
EXPORT_SYMBOL_GPL(smsc_phy_config_init);
@@ -208,8 +211,7 @@ int lan87xx_read_status(struct phy_device *phydev)
if (err)
return err;
- if (!phydev->link && priv && priv->edpd_enable &&
- phydev->irq == PHY_POLL) {
+ if (!phydev->link && priv && priv->edpd_enable) {
/* Disable EDPD to wake up PHY */
int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 4/7] net: phy: smsc: add flag edpd_mode_set_by_user
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
` (2 preceding siblings ...)
2023-04-02 9:45 ` [PATCH net-next 3/7] net: phy: smsc: clear edpd_enable if interrupt mode is used Heiner Kallweit
@ 2023-04-02 9:46 ` Heiner Kallweit
2023-04-02 9:47 ` [PATCH net-next 5/7] net: phy: smss: prepare for making edpd wait period configurable Heiner Kallweit
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 9:46 UTC (permalink / raw)
To: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski
Cc: netdev
Add flag edpd_mode_set_by_user in preparation of adding edpd phy tunable
support. This flag will allow users to override the default behavior
of edpd being disabled if interrupt mode is used.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/smsc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index f5ecd8bea..25b9cd474 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -45,6 +45,7 @@ static struct smsc_hw_stat smsc_hw_stats[] = {
struct smsc_phy_priv {
unsigned int edpd_enable:1;
+ unsigned int edpd_mode_set_by_user:1;
};
static int smsc_phy_ack_interrupt(struct phy_device *phydev)
@@ -117,7 +118,8 @@ int smsc_phy_config_init(struct phy_device *phydev)
if (!priv)
return 0;
- if (phydev->irq != PHY_POLL)
+ /* don't use EDPD in irq mode except overridden by user */
+ if (!priv->edpd_mode_set_by_user && phydev->irq != PHY_POLL)
priv->edpd_enable = false;
return smsc_phy_config_edpd(phydev);
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 5/7] net: phy: smss: prepare for making edpd wait period configurable
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
` (3 preceding siblings ...)
2023-04-02 9:46 ` [PATCH net-next 4/7] net: phy: smsc: add flag edpd_mode_set_by_user Heiner Kallweit
@ 2023-04-02 9:47 ` Heiner Kallweit
2023-04-02 9:47 ` [PATCH net-next 6/7] net: phy: smsc: add edpd tunable support Heiner Kallweit
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 9:47 UTC (permalink / raw)
To: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski
Cc: netdev
Add a member edpd_max_wait_ms to the private data structure in preparation
of making the wait period configurable by supporting the edpd phy tunable.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/smsc.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 25b9cd474..0cd433f01 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -33,6 +33,8 @@
#define SPECIAL_CTRL_STS_AMDIX_ENABLE_ 0x4000
#define SPECIAL_CTRL_STS_AMDIX_STATE_ 0x2000
+#define EDPD_MAX_WAIT_DFLT 640
+
struct smsc_hw_stat {
const char *string;
u8 reg;
@@ -46,6 +48,7 @@ static struct smsc_hw_stat smsc_hw_stats[] = {
struct smsc_phy_priv {
unsigned int edpd_enable:1;
unsigned int edpd_mode_set_by_user:1;
+ unsigned int edpd_max_wait_ms;
};
static int smsc_phy_ack_interrupt(struct phy_device *phydev)
@@ -213,9 +216,13 @@ int lan87xx_read_status(struct phy_device *phydev)
if (err)
return err;
- if (!phydev->link && priv && priv->edpd_enable) {
+ if (!phydev->link && priv && priv->edpd_enable &&
+ priv->edpd_max_wait_ms) {
+ unsigned int max_wait = priv->edpd_max_wait_ms * 1000;
+ int rc;
+
/* Disable EDPD to wake up PHY */
- int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+ rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
@@ -229,7 +236,7 @@ int lan87xx_read_status(struct phy_device *phydev)
*/
read_poll_timeout(phy_read, rc,
rc & MII_LAN83C185_ENERGYON || rc < 0,
- 10000, 640000, true, phydev,
+ 10000, max_wait, true, phydev,
MII_LAN83C185_CTRL_STATUS);
if (rc < 0)
return rc;
@@ -299,6 +306,7 @@ int smsc_phy_probe(struct phy_device *phydev)
return -ENOMEM;
priv->edpd_enable = true;
+ priv->edpd_max_wait_ms = EDPD_MAX_WAIT_DFLT;
if (device_property_present(dev, "smsc,disable-energy-detect"))
priv->edpd_enable = false;
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 6/7] net: phy: smsc: add edpd tunable support
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
` (4 preceding siblings ...)
2023-04-02 9:47 ` [PATCH net-next 5/7] net: phy: smss: prepare for making edpd wait period configurable Heiner Kallweit
@ 2023-04-02 9:47 ` Heiner Kallweit
2023-04-02 12:08 ` Simon Horman
2023-04-02 9:48 ` [PATCH net-next 7/7] net: phy: smsc: enable " Heiner Kallweit
2023-04-02 14:00 ` [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Florian Fainelli
7 siblings, 1 reply; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 9:47 UTC (permalink / raw)
To: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski
Cc: netdev
This adds support for the EDPD PHY tunable.
Per default EDPD is disabled in interrupt mode, the tunable can be used
to override this, e.g. if the link partner doesn't use EDPD.
The interval to check for energy can be chosen between 1000ms and
2000ms. Note that this value consists of the 1000ms phylib interval
for state machine runs plus the time to wait for energy being detected.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/smsc.c | 82 +++++++++++++++++++++++++++++++++++++++++
include/linux/smscphy.h | 4 ++
2 files changed, 86 insertions(+)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 0cd433f01..cca5bf46f 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -34,6 +34,8 @@
#define SPECIAL_CTRL_STS_AMDIX_STATE_ 0x2000
#define EDPD_MAX_WAIT_DFLT 640
+/* interval between phylib state machine runs in ms */
+#define PHY_STATE_MACH_MS 1000
struct smsc_hw_stat {
const char *string;
@@ -295,6 +297,86 @@ static void smsc_get_stats(struct phy_device *phydev,
data[i] = smsc_get_stat(phydev, i);
}
+static int smsc_phy_get_edpd(struct phy_device *phydev, u16 *edpd)
+{
+ struct smsc_phy_priv *priv = phydev->priv;
+
+ if (!priv)
+ return -EOPNOTSUPP;
+
+ if (!priv->edpd_enable)
+ *edpd = ETHTOOL_PHY_EDPD_DISABLE;
+ else if (!priv->edpd_max_wait_ms)
+ *edpd = ETHTOOL_PHY_EDPD_NO_TX;
+ else
+ *edpd = PHY_STATE_MACH_MS + priv->edpd_max_wait_ms;
+
+ return 0;
+}
+
+static int smsc_phy_set_edpd(struct phy_device *phydev, u16 edpd)
+{
+ struct smsc_phy_priv *priv = phydev->priv;
+ int ret;
+
+ if (!priv)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&phydev->lock);
+
+ switch (edpd) {
+ case ETHTOOL_PHY_EDPD_DISABLE:
+ priv->edpd_enable = false;
+ break;
+ case ETHTOOL_PHY_EDPD_NO_TX:
+ priv->edpd_enable = true;
+ priv->edpd_max_wait_ms = 0;
+ break;
+ case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS:
+ edpd = PHY_STATE_MACH_MS + EDPD_MAX_WAIT_DFLT;
+ fallthrough;
+ default:
+ if (phydev->irq != PHY_POLL)
+ return -EOPNOTSUPP;
+ if (edpd < PHY_STATE_MACH_MS || edpd > PHY_STATE_MACH_MS + 1000)
+ return -EINVAL;
+ priv->edpd_enable = true;
+ priv->edpd_max_wait_ms = edpd - PHY_STATE_MACH_MS;
+ }
+
+ priv->edpd_mode_set_by_user = true;
+
+ ret = smsc_phy_config_edpd(phydev);
+
+ mutex_unlock(&phydev->lock);
+
+ return ret;
+}
+
+int smsc_phy_get_tunable(struct phy_device *phydev,
+ struct ethtool_tunable *tuna, void *data)
+{
+ switch (tuna->id) {
+ case ETHTOOL_PHY_EDPD:
+ return smsc_phy_get_edpd(phydev, data);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+EXPORT_SYMBOL_GPL(smsc_phy_get_tunable);
+
+int smsc_phy_set_tunable(struct phy_device *phydev,
+ struct ethtool_tunable *tuna, const void *data)
+{
+ switch (tuna->id) {
+ case ETHTOOL_PHY_EDPD:
+ return smsc_phy_set_edpd(phydev, *(u16 *)data);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+EXPORT_SYMBOL_GPL(smsc_phy_set_tunable);
+
int smsc_phy_probe(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
index 80f37c1db..e1c886277 100644
--- a/include/linux/smscphy.h
+++ b/include/linux/smscphy.h
@@ -32,6 +32,10 @@ int smsc_phy_config_intr(struct phy_device *phydev);
irqreturn_t smsc_phy_handle_interrupt(struct phy_device *phydev);
int smsc_phy_config_init(struct phy_device *phydev);
int lan87xx_read_status(struct phy_device *phydev);
+int smsc_phy_get_tunable(struct phy_device *phydev,
+ struct ethtool_tunable *tuna, void *data);
+int smsc_phy_set_tunable(struct phy_device *phydev,
+ struct ethtool_tunable *tuna, const void *data);
int smsc_phy_probe(struct phy_device *phydev);
#endif /* __LINUX_SMSCPHY_H__ */
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next 7/7] net: phy: smsc: enable edpd tunable support
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
` (5 preceding siblings ...)
2023-04-02 9:47 ` [PATCH net-next 6/7] net: phy: smsc: add edpd tunable support Heiner Kallweit
@ 2023-04-02 9:48 ` Heiner Kallweit
2023-04-02 14:00 ` [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Florian Fainelli
7 siblings, 0 replies; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 9:48 UTC (permalink / raw)
To: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski
Cc: netdev
Enable EDPD PHY tunable support for all drivers using
lan87xx_read_status.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/smsc.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index cca5bf46f..a85763c02 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -476,6 +476,9 @@ static struct phy_driver smsc_phy_driver[] = {
.get_strings = smsc_get_strings,
.get_stats = smsc_get_stats,
+ .get_tunable = smsc_phy_get_tunable,
+ .set_tunable = smsc_phy_set_tunable,
+
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
@@ -520,6 +523,9 @@ static struct phy_driver smsc_phy_driver[] = {
.get_strings = smsc_get_strings,
.get_stats = smsc_get_stats,
+ .get_tunable = smsc_phy_get_tunable,
+ .set_tunable = smsc_phy_set_tunable,
+
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
@@ -546,6 +552,9 @@ static struct phy_driver smsc_phy_driver[] = {
.get_strings = smsc_get_strings,
.get_stats = smsc_get_stats,
+ .get_tunable = smsc_phy_get_tunable,
+ .set_tunable = smsc_phy_set_tunable,
+
.suspend = genphy_suspend,
.resume = genphy_resume,
}, {
@@ -576,6 +585,9 @@ static struct phy_driver smsc_phy_driver[] = {
.get_strings = smsc_get_strings,
.get_stats = smsc_get_stats,
+ .get_tunable = smsc_phy_get_tunable,
+ .set_tunable = smsc_phy_set_tunable,
+
.suspend = genphy_suspend,
.resume = genphy_resume,
} };
--
2.40.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 6/7] net: phy: smsc: add edpd tunable support
2023-04-02 9:47 ` [PATCH net-next 6/7] net: phy: smsc: add edpd tunable support Heiner Kallweit
@ 2023-04-02 12:08 ` Simon Horman
2023-04-02 13:52 ` Heiner Kallweit
0 siblings, 1 reply; 13+ messages in thread
From: Simon Horman @ 2023-04-02 12:08 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski, netdev
On Sun, Apr 02, 2023 at 11:47:34AM +0200, Heiner Kallweit wrote:
> This adds support for the EDPD PHY tunable.
> Per default EDPD is disabled in interrupt mode, the tunable can be used
> to override this, e.g. if the link partner doesn't use EDPD.
> The interval to check for energy can be chosen between 1000ms and
> 2000ms. Note that this value consists of the 1000ms phylib interval
> for state machine runs plus the time to wait for energy being detected.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/net/phy/smsc.c | 82 +++++++++++++++++++++++++++++++++++++++++
> include/linux/smscphy.h | 4 ++
> 2 files changed, 86 insertions(+)
>
> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> index 0cd433f01..cca5bf46f 100644
> --- a/drivers/net/phy/smsc.c
> +++ b/drivers/net/phy/smsc.c
> @@ -34,6 +34,8 @@
> #define SPECIAL_CTRL_STS_AMDIX_STATE_ 0x2000
>
> #define EDPD_MAX_WAIT_DFLT 640
nit: Maybe this could be EDPD_MAX_WAIT_DFLT_MS for consistency
with PHY_STATE_MACH_MS.
> +/* interval between phylib state machine runs in ms */
> +#define PHY_STATE_MACH_MS 1000
>
> struct smsc_hw_stat {
> const char *string;
> @@ -295,6 +297,86 @@ static void smsc_get_stats(struct phy_device *phydev,
> data[i] = smsc_get_stat(phydev, i);
> }
>
> +static int smsc_phy_get_edpd(struct phy_device *phydev, u16 *edpd)
> +{
> + struct smsc_phy_priv *priv = phydev->priv;
> +
> + if (!priv)
> + return -EOPNOTSUPP;
> +
> + if (!priv->edpd_enable)
> + *edpd = ETHTOOL_PHY_EDPD_DISABLE;
> + else if (!priv->edpd_max_wait_ms)
> + *edpd = ETHTOOL_PHY_EDPD_NO_TX;
> + else
> + *edpd = PHY_STATE_MACH_MS + priv->edpd_max_wait_ms;
> +
> + return 0;
> +}
> +
> +static int smsc_phy_set_edpd(struct phy_device *phydev, u16 edpd)
> +{
> + struct smsc_phy_priv *priv = phydev->priv;
> + int ret;
> +
> + if (!priv)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&phydev->lock);
I am a little confused by this as by my reasoning this code is called via
the first arm of the following in set_phy_tunable(), and phydev->lock is
already held.
if (phy_drv_tunable) {
mutex_lock(&phydev->lock);
ret = phydev->drv->set_tunable(phydev, &tuna, data);
mutex_unlock(&phydev->lock);
} else {
ret = dev->ethtool_ops->set_phy_tunable(dev, &tuna, data);
}
> +
> + switch (edpd) {
> + case ETHTOOL_PHY_EDPD_DISABLE:
> + priv->edpd_enable = false;
> + break;
> + case ETHTOOL_PHY_EDPD_NO_TX:
> + priv->edpd_enable = true;
> + priv->edpd_max_wait_ms = 0;
> + break;
> + case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS:
> + edpd = PHY_STATE_MACH_MS + EDPD_MAX_WAIT_DFLT;
> + fallthrough;
> + default:
> + if (phydev->irq != PHY_POLL)
> + return -EOPNOTSUPP;
This returns without releasing phydev->lock.
Is that intended?
> + if (edpd < PHY_STATE_MACH_MS || edpd > PHY_STATE_MACH_MS + 1000)
> + return -EINVAL;
Ditto.
> + priv->edpd_enable = true;
> + priv->edpd_max_wait_ms = edpd - PHY_STATE_MACH_MS;
> + }
> +
> + priv->edpd_mode_set_by_user = true;
> +
> + ret = smsc_phy_config_edpd(phydev);
> +
> + mutex_unlock(&phydev->lock);
> +
> + return ret;
> +}
> +
> +int smsc_phy_get_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, void *data)
> +{
> + switch (tuna->id) {
> + case ETHTOOL_PHY_EDPD:
> + return smsc_phy_get_edpd(phydev, data);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +EXPORT_SYMBOL_GPL(smsc_phy_get_tunable);
> +
> +int smsc_phy_set_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, const void *data)
> +{
> + switch (tuna->id) {
> + case ETHTOOL_PHY_EDPD:
> + return smsc_phy_set_edpd(phydev, *(u16 *)data);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +EXPORT_SYMBOL_GPL(smsc_phy_set_tunable);
> +
> int smsc_phy_probe(struct phy_device *phydev)
> {
> struct device *dev = &phydev->mdio.dev;
> diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
> index 80f37c1db..e1c886277 100644
> --- a/include/linux/smscphy.h
> +++ b/include/linux/smscphy.h
> @@ -32,6 +32,10 @@ int smsc_phy_config_intr(struct phy_device *phydev);
> irqreturn_t smsc_phy_handle_interrupt(struct phy_device *phydev);
> int smsc_phy_config_init(struct phy_device *phydev);
> int lan87xx_read_status(struct phy_device *phydev);
> +int smsc_phy_get_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, void *data);
> +int smsc_phy_set_tunable(struct phy_device *phydev,
> + struct ethtool_tunable *tuna, const void *data);
> int smsc_phy_probe(struct phy_device *phydev);
>
> #endif /* __LINUX_SMSCPHY_H__ */
> --
> 2.40.0
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 6/7] net: phy: smsc: add edpd tunable support
2023-04-02 12:08 ` Simon Horman
@ 2023-04-02 13:52 ` Heiner Kallweit
0 siblings, 0 replies; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 13:52 UTC (permalink / raw)
To: Simon Horman
Cc: Andrew Lunn, Russell King - ARM Linux, Eric Dumazet, Paolo Abeni,
David Miller, Jakub Kicinski, netdev
On 02.04.2023 14:08, Simon Horman wrote:
> On Sun, Apr 02, 2023 at 11:47:34AM +0200, Heiner Kallweit wrote:
>> This adds support for the EDPD PHY tunable.
>> Per default EDPD is disabled in interrupt mode, the tunable can be used
>> to override this, e.g. if the link partner doesn't use EDPD.
>> The interval to check for energy can be chosen between 1000ms and
>> 2000ms. Note that this value consists of the 1000ms phylib interval
>> for state machine runs plus the time to wait for energy being detected.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/net/phy/smsc.c | 82 +++++++++++++++++++++++++++++++++++++++++
>> include/linux/smscphy.h | 4 ++
>> 2 files changed, 86 insertions(+)
>>
>> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
>> index 0cd433f01..cca5bf46f 100644
>> --- a/drivers/net/phy/smsc.c
>> +++ b/drivers/net/phy/smsc.c
>> @@ -34,6 +34,8 @@
>> #define SPECIAL_CTRL_STS_AMDIX_STATE_ 0x2000
>>
>> #define EDPD_MAX_WAIT_DFLT 640
>
> nit: Maybe this could be EDPD_MAX_WAIT_DFLT_MS for consistency
> with PHY_STATE_MACH_MS.
>
Yes, this would be better.
>> +/* interval between phylib state machine runs in ms */
>> +#define PHY_STATE_MACH_MS 1000
>>
>> struct smsc_hw_stat {
>> const char *string;
>> @@ -295,6 +297,86 @@ static void smsc_get_stats(struct phy_device *phydev,
>> data[i] = smsc_get_stat(phydev, i);
>> }
>>
>> +static int smsc_phy_get_edpd(struct phy_device *phydev, u16 *edpd)
>> +{
>> + struct smsc_phy_priv *priv = phydev->priv;
>> +
>> + if (!priv)
>> + return -EOPNOTSUPP;
>> +
>> + if (!priv->edpd_enable)
>> + *edpd = ETHTOOL_PHY_EDPD_DISABLE;
>> + else if (!priv->edpd_max_wait_ms)
>> + *edpd = ETHTOOL_PHY_EDPD_NO_TX;
>> + else
>> + *edpd = PHY_STATE_MACH_MS + priv->edpd_max_wait_ms;
>> +
>> + return 0;
>> +}
>> +
>> +static int smsc_phy_set_edpd(struct phy_device *phydev, u16 edpd)
>> +{
>> + struct smsc_phy_priv *priv = phydev->priv;
>> + int ret;
>> +
>> + if (!priv)
>> + return -EOPNOTSUPP;
>> +
>> + mutex_lock(&phydev->lock);
>
> I am a little confused by this as by my reasoning this code is called via
> the first arm of the following in set_phy_tunable(), and phydev->lock is
> already held.
>
You're right of course. So we can remove the locking in the driver.
> if (phy_drv_tunable) {
> mutex_lock(&phydev->lock);
> ret = phydev->drv->set_tunable(phydev, &tuna, data);
> mutex_unlock(&phydev->lock);
> } else {
> ret = dev->ethtool_ops->set_phy_tunable(dev, &tuna, data);
> }
>
>> +
>> + switch (edpd) {
>> + case ETHTOOL_PHY_EDPD_DISABLE:
>> + priv->edpd_enable = false;
>> + break;
>> + case ETHTOOL_PHY_EDPD_NO_TX:
>> + priv->edpd_enable = true;
>> + priv->edpd_max_wait_ms = 0;
>> + break;
>> + case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS:
>> + edpd = PHY_STATE_MACH_MS + EDPD_MAX_WAIT_DFLT;
>> + fallthrough;
>> + default:
>> + if (phydev->irq != PHY_POLL)
>> + return -EOPNOTSUPP;
>
> This returns without releasing phydev->lock.
> Is that intended?
>
>> + if (edpd < PHY_STATE_MACH_MS || edpd > PHY_STATE_MACH_MS + 1000)
>> + return -EINVAL;
>
> Ditto.
>
>> + priv->edpd_enable = true;
>> + priv->edpd_max_wait_ms = edpd - PHY_STATE_MACH_MS;
>> + }
>> +
>> + priv->edpd_mode_set_by_user = true;
>> +
>> + ret = smsc_phy_config_edpd(phydev);
>> +
>> + mutex_unlock(&phydev->lock);
>> +
>> + return ret;
>> +}
>> +
>> +int smsc_phy_get_tunable(struct phy_device *phydev,
>> + struct ethtool_tunable *tuna, void *data)
>> +{
>> + switch (tuna->id) {
>> + case ETHTOOL_PHY_EDPD:
>> + return smsc_phy_get_edpd(phydev, data);
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +}
>> +EXPORT_SYMBOL_GPL(smsc_phy_get_tunable);
>> +
>> +int smsc_phy_set_tunable(struct phy_device *phydev,
>> + struct ethtool_tunable *tuna, const void *data)
>> +{
>> + switch (tuna->id) {
>> + case ETHTOOL_PHY_EDPD:
>> + return smsc_phy_set_edpd(phydev, *(u16 *)data);
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +}
>> +EXPORT_SYMBOL_GPL(smsc_phy_set_tunable);
>> +
>> int smsc_phy_probe(struct phy_device *phydev)
>> {
>> struct device *dev = &phydev->mdio.dev;
>> diff --git a/include/linux/smscphy.h b/include/linux/smscphy.h
>> index 80f37c1db..e1c886277 100644
>> --- a/include/linux/smscphy.h
>> +++ b/include/linux/smscphy.h
>> @@ -32,6 +32,10 @@ int smsc_phy_config_intr(struct phy_device *phydev);
>> irqreturn_t smsc_phy_handle_interrupt(struct phy_device *phydev);
>> int smsc_phy_config_init(struct phy_device *phydev);
>> int lan87xx_read_status(struct phy_device *phydev);
>> +int smsc_phy_get_tunable(struct phy_device *phydev,
>> + struct ethtool_tunable *tuna, void *data);
>> +int smsc_phy_set_tunable(struct phy_device *phydev,
>> + struct ethtool_tunable *tuna, const void *data);
>> int smsc_phy_probe(struct phy_device *phydev);
>>
>> #endif /* __LINUX_SMSCPHY_H__ */
>> --
>> 2.40.0
>>
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
` (6 preceding siblings ...)
2023-04-02 9:48 ` [PATCH net-next 7/7] net: phy: smsc: enable " Heiner Kallweit
@ 2023-04-02 14:00 ` Florian Fainelli
2023-04-02 14:58 ` Heiner Kallweit
7 siblings, 1 reply; 13+ messages in thread
From: Florian Fainelli @ 2023-04-02 14:00 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, Russell King - ARM Linux,
Eric Dumazet, Paolo Abeni, David Miller, Jakub Kicinski,
Chris Healy
Cc: netdev
On 4/2/2023 2:43 AM, Heiner Kallweit wrote:
> This adds support for the EDPD PHY tunable.
> Per default EDPD is disabled in interrupt mode, the tunable can be used
> to override this, e.g. if the link partner doesn't use EDPD.
> The interval to check for energy can be chosen between 1000ms and
> 2000ms. Note that this value consists of the 1000ms phylib interval
> for state machine runs plus the time to wait for energy being detected.
AFAIR Chris Healy was trying to get something similar done, maybe you
can CC him on the next version?
--
Florian
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable
2023-04-02 14:00 ` [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Florian Fainelli
@ 2023-04-02 14:58 ` Heiner Kallweit
2023-04-02 23:33 ` Chris Healy
0 siblings, 1 reply; 13+ messages in thread
From: Heiner Kallweit @ 2023-04-02 14:58 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, Russell King - ARM Linux,
Eric Dumazet, Paolo Abeni, David Miller, Jakub Kicinski,
Chris Healy
Cc: netdev
On 02.04.2023 16:00, Florian Fainelli wrote:
>
>
> On 4/2/2023 2:43 AM, Heiner Kallweit wrote:
>> This adds support for the EDPD PHY tunable.
>> Per default EDPD is disabled in interrupt mode, the tunable can be used
>> to override this, e.g. if the link partner doesn't use EDPD.
>> The interval to check for energy can be chosen between 1000ms and
>> 2000ms. Note that this value consists of the 1000ms phylib interval
>> for state machine runs plus the time to wait for energy being detected.
>
> AFAIR Chris Healy was trying to get something similar done, maybe you can CC him on the next version?
Actually this is based on a conversation between him and me. Chris came up
with a first version and I reworked it from scratch to meet mainline standards
and make it usable on SMSC and Amlogic PHY's.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable
2023-04-02 14:58 ` Heiner Kallweit
@ 2023-04-02 23:33 ` Chris Healy
0 siblings, 0 replies; 13+ messages in thread
From: Chris Healy @ 2023-04-02 23:33 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Florian Fainelli, Andrew Lunn, Russell King - ARM Linux,
Eric Dumazet, Paolo Abeni, David Miller, Jakub Kicinski, netdev
On Sun, Apr 2, 2023 at 7:58 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> On 02.04.2023 16:00, Florian Fainelli wrote:
> >
> >
> > On 4/2/2023 2:43 AM, Heiner Kallweit wrote:
> >> This adds support for the EDPD PHY tunable.
> >> Per default EDPD is disabled in interrupt mode, the tunable can be used
> >> to override this, e.g. if the link partner doesn't use EDPD.
> >> The interval to check for energy can be chosen between 1000ms and
> >> 2000ms. Note that this value consists of the 1000ms phylib interval
> >> for state machine runs plus the time to wait for energy being detected.
> >
> > AFAIR Chris Healy was trying to get something similar done, maybe you can CC him on the next version?
>
> Actually this is based on a conversation between him and me. Chris came up
> with a first version and I reworked it from scratch to meet mainline standards
> and make it usable on SMSC and Amlogic PHY's.
>
It's great to see this feature coming together. I wish I would have
had more time to help out.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-04-02 23:33 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-02 9:43 [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Heiner Kallweit
2023-04-02 9:44 ` [PATCH net-next 1/7] net: phy: smsc: rename flag energy_enable Heiner Kallweit
2023-04-02 9:45 ` [PATCH net-next 2/7] net: phy: smsc: add helper smsc_phy_config_edpd Heiner Kallweit
2023-04-02 9:45 ` [PATCH net-next 3/7] net: phy: smsc: clear edpd_enable if interrupt mode is used Heiner Kallweit
2023-04-02 9:46 ` [PATCH net-next 4/7] net: phy: smsc: add flag edpd_mode_set_by_user Heiner Kallweit
2023-04-02 9:47 ` [PATCH net-next 5/7] net: phy: smss: prepare for making edpd wait period configurable Heiner Kallweit
2023-04-02 9:47 ` [PATCH net-next 6/7] net: phy: smsc: add edpd tunable support Heiner Kallweit
2023-04-02 12:08 ` Simon Horman
2023-04-02 13:52 ` Heiner Kallweit
2023-04-02 9:48 ` [PATCH net-next 7/7] net: phy: smsc: enable " Heiner Kallweit
2023-04-02 14:00 ` [PATCH net-next 0/7] net: phy: smsc: add support for edpd tunable Florian Fainelli
2023-04-02 14:58 ` Heiner Kallweit
2023-04-02 23:33 ` Chris Healy
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).