* [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2)
@ 2024-12-10 14:25 Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 1/7] net: dsa: ksz: remove setting of tx_lpi parameters Russell King (Oracle)
` (7 more replies)
0 siblings, 8 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2024-12-10 14:25 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
netdev, Paolo Abeni, Simon Horman, UNGLinuxDriver,
Vladimir Oltean, Woojung Huh
This is part 2 of the DSA EEE cleanups, and is being sent out becaues it
is relevant for the review of part 1, but would make part 1 too large.
Patch 1 removes the useless setting of tx_lpi parameters in the
ksz driver.
Patch 2 removes the DSA core code that calls the get_mac_eee() operation.
This needs to be done before removing the implementations because doing
otherwise would cause dsa_user_get_eee() to return -EOPNOTSUPP.
Patches 3..6 remove the trivial get_mac_eee() implementations from DSA
drivers.
Patch 7 finally removes the get_mac_eee() method from struct
dsa_switch_ops.
For part 2:
drivers/net/dsa/b53/b53_common.c | 7 -------
drivers/net/dsa/b53/b53_priv.h | 1 -
drivers/net/dsa/bcm_sf2.c | 1 -
drivers/net/dsa/microchip/ksz_common.c | 15 ---------------
drivers/net/dsa/mv88e6xxx/chip.c | 8 --------
drivers/net/dsa/qca/qca8k-8xxx.c | 1 -
drivers/net/dsa/qca/qca8k-common.c | 7 -------
drivers/net/dsa/qca/qca8k.h | 1 -
include/net/dsa.h | 2 --
net/dsa/user.c | 12 ------------
10 files changed, 55 deletions(-)
For part 1 and part 2 combined results in a net reduction of 33 LOC:
drivers/net/dsa/b53/b53_common.c | 14 ++++----------
drivers/net/dsa/b53/b53_priv.h | 2 +-
drivers/net/dsa/bcm_sf2.c | 2 +-
drivers/net/dsa/microchip/ksz_common.c | 35 +++++-----------------------------
drivers/net/dsa/mt7530.c | 1 +
drivers/net/dsa/mv88e6xxx/chip.c | 9 +--------
drivers/net/dsa/qca/qca8k-8xxx.c | 2 +-
drivers/net/dsa/qca/qca8k-common.c | 7 -------
drivers/net/dsa/qca/qca8k.h | 1 -
include/net/dsa.h | 4 ++--
net/dsa/port.c | 16 ++++++++++++++++
net/dsa/user.c | 18 +++++++----------
12 files changed, 39 insertions(+), 72 deletions(-)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH RFC net-next 1/7] net: dsa: ksz: remove setting of tx_lpi parameters
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
@ 2024-12-10 14:26 ` Russell King (Oracle)
2024-12-12 19:38 ` Vladimir Oltean
2024-12-10 14:26 ` [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee() Russell King (Oracle)
` (6 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Russell King (Oracle) @ 2024-12-10 14:26 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
netdev, Paolo Abeni, Simon Horman, UNGLinuxDriver,
Vladimir Oltean, Woojung Huh
dsa_user_get_eee() calls the DSA switch get_mac_eee() method followed
by phylink_ethtool_get_eee(), which goes on to call
phy_ethtool_get_eee(). This overwrites all members of the passed
ethtool_keee, which means anything written by the DSA switch
get_mac_eee() method will discarded.
Remove setting any members in ksz_get_mac_eee().
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/microchip/ksz_common.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 94f9aa983ff6..9a48b4438a6d 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3480,14 +3480,6 @@ static bool ksz_support_eee(struct dsa_switch *ds, int port)
static int ksz_get_mac_eee(struct dsa_switch *ds, int port,
struct ethtool_keee *e)
{
- /* There is no documented control of Tx LPI configuration. */
- e->tx_lpi_enabled = true;
-
- /* There is no documented control of Tx LPI timer. According to tests
- * Tx LPI timer seems to be set by default to minimal value.
- */
- e->tx_lpi_timer = 0;
-
return 0;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee()
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 1/7] net: dsa: ksz: remove setting of tx_lpi parameters Russell King (Oracle)
@ 2024-12-10 14:26 ` Russell King (Oracle)
2024-12-12 19:44 ` Vladimir Oltean
2024-12-10 14:26 ` [PATCH RFC net-next 3/7] net: dsa: b53/bcm_sf2: remove b53_get_mac_eee() Russell King (Oracle)
` (5 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Russell King (Oracle) @ 2024-12-10 14:26 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
netdev, Paolo Abeni, Simon Horman, UNGLinuxDriver,
Vladimir Oltean, Woojung Huh
All implementations of get_mac_eee() now just return zero without doing
anything useful. Remove the call to this method in preparation to
removing the method from each DSA driver.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
net/dsa/user.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 4239083c18bf..fb38543b29db 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -1250,23 +1250,11 @@ static int dsa_user_get_eee(struct net_device *dev, struct ethtool_keee *e)
{
struct dsa_port *dp = dsa_user_to_port(dev);
struct dsa_switch *ds = dp->ds;
- int ret;
/* Check whether the switch supports EEE */
if (!ds->ops->support_eee || !ds->ops->support_eee(ds, dp->index))
return -EOPNOTSUPP;
- /* Port's PHY and MAC both need to be EEE capable */
- if (!dev->phydev)
- return -ENODEV;
-
- if (!ds->ops->get_mac_eee)
- return -EOPNOTSUPP;
-
- ret = ds->ops->get_mac_eee(ds, dp->index, e);
- if (ret)
- return ret;
-
return phylink_ethtool_get_eee(dp->pl, e);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH RFC net-next 3/7] net: dsa: b53/bcm_sf2: remove b53_get_mac_eee()
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 1/7] net: dsa: ksz: remove setting of tx_lpi parameters Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee() Russell King (Oracle)
@ 2024-12-10 14:26 ` Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 4/7] net: dsa: ksz: remove ksz_get_mac_eee() Russell King (Oracle)
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2024-12-10 14:26 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
netdev, Paolo Abeni, Simon Horman, UNGLinuxDriver,
Vladimir Oltean, Woojung Huh
b53_get_mac_eee() is no longer called by the core DSA code. Remove it.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/b53/b53_common.c | 7 -------
drivers/net/dsa/b53/b53_priv.h | 1 -
drivers/net/dsa/bcm_sf2.c | 1 -
3 files changed, 9 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 0561b60f668f..79dc77835681 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -2232,12 +2232,6 @@ bool b53_support_eee(struct dsa_switch *ds, int port)
}
EXPORT_SYMBOL(b53_support_eee);
-int b53_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e)
-{
- return 0;
-}
-EXPORT_SYMBOL(b53_get_mac_eee);
-
int b53_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e)
{
struct b53_device *dev = ds->priv;
@@ -2299,7 +2293,6 @@ static const struct dsa_switch_ops b53_switch_ops = {
.port_enable = b53_enable_port,
.port_disable = b53_disable_port,
.support_eee = b53_support_eee,
- .get_mac_eee = b53_get_mac_eee,
.set_mac_eee = b53_set_mac_eee,
.port_bridge_join = b53_br_join,
.port_bridge_leave = b53_br_leave,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 99e5cfc98ae8..9e9b5bc0c5d6 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -385,7 +385,6 @@ void b53_disable_port(struct dsa_switch *ds, int port);
void b53_brcm_hdr_setup(struct dsa_switch *ds, int port);
int b53_eee_init(struct dsa_switch *ds, int port, struct phy_device *phy);
bool b53_support_eee(struct dsa_switch *ds, int port);
-int b53_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e);
int b53_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e);
#endif
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index a53fb6191e6b..fa2bf3fa9019 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -1233,7 +1233,6 @@ static const struct dsa_switch_ops bcm_sf2_ops = {
.port_enable = bcm_sf2_port_setup,
.port_disable = bcm_sf2_port_disable,
.support_eee = b53_support_eee,
- .get_mac_eee = b53_get_mac_eee,
.set_mac_eee = b53_set_mac_eee,
.port_bridge_join = b53_br_join,
.port_bridge_leave = b53_br_leave,
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH RFC net-next 4/7] net: dsa: ksz: remove ksz_get_mac_eee()
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
` (2 preceding siblings ...)
2024-12-10 14:26 ` [PATCH RFC net-next 3/7] net: dsa: b53/bcm_sf2: remove b53_get_mac_eee() Russell King (Oracle)
@ 2024-12-10 14:26 ` Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 5/7] net: dsa: mv88e6xxx: remove mv88e6xxx_get_mac_eee() Russell King (Oracle)
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2024-12-10 14:26 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
netdev, Paolo Abeni, Simon Horman, UNGLinuxDriver,
Vladimir Oltean, Woojung Huh
ksz_get_mac_eee() is no longer called by the core DSA code. Remove it.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/microchip/ksz_common.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 9a48b4438a6d..807a37112a00 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3477,12 +3477,6 @@ static bool ksz_support_eee(struct dsa_switch *ds, int port)
return false;
}
-static int ksz_get_mac_eee(struct dsa_switch *ds, int port,
- struct ethtool_keee *e)
-{
- return 0;
-}
-
static int ksz_set_mac_eee(struct dsa_switch *ds, int port,
struct ethtool_keee *e)
{
@@ -4633,7 +4627,6 @@ static const struct dsa_switch_ops ksz_switch_ops = {
.cls_flower_del = ksz_cls_flower_del,
.port_setup_tc = ksz_setup_tc,
.support_eee = ksz_support_eee,
- .get_mac_eee = ksz_get_mac_eee,
.set_mac_eee = ksz_set_mac_eee,
.port_get_default_prio = ksz_port_get_default_prio,
.port_set_default_prio = ksz_port_set_default_prio,
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH RFC net-next 5/7] net: dsa: mv88e6xxx: remove mv88e6xxx_get_mac_eee()
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
` (3 preceding siblings ...)
2024-12-10 14:26 ` [PATCH RFC net-next 4/7] net: dsa: ksz: remove ksz_get_mac_eee() Russell King (Oracle)
@ 2024-12-10 14:26 ` Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 6/7] net: dsa: qca: remove qca8k_get_mac_eee() Russell King (Oracle)
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2024-12-10 14:26 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
netdev, Paolo Abeni, Simon Horman, UNGLinuxDriver,
Vladimir Oltean, Woojung Huh
mv88e6xxx_get_mac_eee() is no longer called by the core DSA code.
Remove it.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/mv88e6xxx/chip.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 1d78974ea2a3..ddb70047098f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1522,13 +1522,6 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int port,
mv88e6xxx_reg_unlock(chip);
}
-static int mv88e6xxx_get_mac_eee(struct dsa_switch *ds, int port,
- struct ethtool_keee *e)
-{
- /* Nothing to do on the port's MAC */
- return 0;
-}
-
static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, int port,
struct ethtool_keee *e)
{
@@ -7075,7 +7068,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
.port_max_mtu = mv88e6xxx_get_max_mtu,
.port_change_mtu = mv88e6xxx_change_mtu,
.support_eee = dsa_supports_eee,
- .get_mac_eee = mv88e6xxx_get_mac_eee,
.set_mac_eee = mv88e6xxx_set_mac_eee,
.get_eeprom_len = mv88e6xxx_get_eeprom_len,
.get_eeprom = mv88e6xxx_get_eeprom,
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH RFC net-next 6/7] net: dsa: qca: remove qca8k_get_mac_eee()
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
` (4 preceding siblings ...)
2024-12-10 14:26 ` [PATCH RFC net-next 5/7] net: dsa: mv88e6xxx: remove mv88e6xxx_get_mac_eee() Russell King (Oracle)
@ 2024-12-10 14:26 ` Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 7/7] net: dsa: remove get_mac_eee() method Russell King (Oracle)
2024-12-12 19:48 ` [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Vladimir Oltean
7 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2024-12-10 14:26 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
netdev, Paolo Abeni, Simon Horman, UNGLinuxDriver,
Vladimir Oltean, Woojung Huh
qca8k_get_mac_eee() is no longer called by the core DSA code. Remove it.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/dsa/qca/qca8k-8xxx.c | 1 -
drivers/net/dsa/qca/qca8k-common.c | 7 -------
drivers/net/dsa/qca/qca8k.h | 1 -
3 files changed, 9 deletions(-)
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index ec74e3c2b0e9..4c68524ad8af 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -2017,7 +2017,6 @@ static const struct dsa_switch_ops qca8k_switch_ops = {
.get_sset_count = qca8k_get_sset_count,
.set_ageing_time = qca8k_set_ageing_time,
.support_eee = dsa_supports_eee,
- .get_mac_eee = qca8k_get_mac_eee,
.set_mac_eee = qca8k_set_mac_eee,
.port_enable = qca8k_port_enable,
.port_disable = qca8k_port_disable,
diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c
index 560c74c4ac3d..13005f10edb7 100644
--- a/drivers/net/dsa/qca/qca8k-common.c
+++ b/drivers/net/dsa/qca/qca8k-common.c
@@ -557,13 +557,6 @@ int qca8k_set_mac_eee(struct dsa_switch *ds, int port,
return ret;
}
-int qca8k_get_mac_eee(struct dsa_switch *ds, int port,
- struct ethtool_keee *e)
-{
- /* Nothing to do on the port's MAC */
- return 0;
-}
-
static int qca8k_port_configure_learning(struct dsa_switch *ds, int port,
bool learning)
{
diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h
index 3664a2e2f1f6..961d14f0336d 100644
--- a/drivers/net/dsa/qca/qca8k.h
+++ b/drivers/net/dsa/qca/qca8k.h
@@ -520,7 +520,6 @@ int qca8k_get_sset_count(struct dsa_switch *ds, int port, int sset);
/* Common eee function */
int qca8k_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *eee);
-int qca8k_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e);
/* Common bridge function */
void qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state);
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH RFC net-next 7/7] net: dsa: remove get_mac_eee() method
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
` (5 preceding siblings ...)
2024-12-10 14:26 ` [PATCH RFC net-next 6/7] net: dsa: qca: remove qca8k_get_mac_eee() Russell King (Oracle)
@ 2024-12-10 14:26 ` Russell King (Oracle)
2024-12-12 19:48 ` [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Vladimir Oltean
7 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2024-12-10 14:26 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit
Cc: David S. Miller, Eric Dumazet, Florian Fainelli, Jakub Kicinski,
netdev, Paolo Abeni, Simon Horman, UNGLinuxDriver,
Vladimir Oltean, Woojung Huh
The get_mac_eee() is no longer called by the core DSA code, nor are
there any implementations of this method. Remove it.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
include/net/dsa.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4aeedb296d67..9640d5c67f56 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -991,8 +991,6 @@ struct dsa_switch_ops {
bool (*support_eee)(struct dsa_switch *ds, int port);
int (*set_mac_eee)(struct dsa_switch *ds, int port,
struct ethtool_keee *e);
- int (*get_mac_eee)(struct dsa_switch *ds, int port,
- struct ethtool_keee *e);
/* EEPROM access */
int (*get_eeprom_len)(struct dsa_switch *ds);
--
2.30.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH RFC net-next 1/7] net: dsa: ksz: remove setting of tx_lpi parameters
2024-12-10 14:26 ` [PATCH RFC net-next 1/7] net: dsa: ksz: remove setting of tx_lpi parameters Russell King (Oracle)
@ 2024-12-12 19:38 ` Vladimir Oltean
0 siblings, 0 replies; 13+ messages in thread
From: Vladimir Oltean @ 2024-12-12 19:38 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Florian Fainelli, Jakub Kicinski, netdev, Paolo Abeni,
Simon Horman, UNGLinuxDriver, Woojung Huh
On Tue, Dec 10, 2024 at 02:26:14PM +0000, Russell King (Oracle) wrote:
> dsa_user_get_eee() calls the DSA switch get_mac_eee() method followed
> by phylink_ethtool_get_eee(), which goes on to call
> phy_ethtool_get_eee(). This overwrites all members of the passed
> ethtool_keee, which means anything written by the DSA switch
> get_mac_eee() method will discarded.
nitpick: will _be_ discarded.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee()
2024-12-10 14:26 ` [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee() Russell King (Oracle)
@ 2024-12-12 19:44 ` Vladimir Oltean
2025-01-02 17:56 ` Russell King (Oracle)
0 siblings, 1 reply; 13+ messages in thread
From: Vladimir Oltean @ 2024-12-12 19:44 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Florian Fainelli, Jakub Kicinski, netdev, Paolo Abeni,
Simon Horman, UNGLinuxDriver, Woojung Huh
On Tue, Dec 10, 2024 at 02:26:19PM +0000, Russell King (Oracle) wrote:
> All implementations of get_mac_eee() now just return zero without doing
> anything useful. Remove the call to this method in preparation to
> removing the method from each DSA driver.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
> net/dsa/user.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
> diff --git a/net/dsa/user.c b/net/dsa/user.c
> index 4239083c18bf..fb38543b29db 100644
> --- a/net/dsa/user.c
> +++ b/net/dsa/user.c
> @@ -1250,23 +1250,11 @@ static int dsa_user_get_eee(struct net_device *dev, struct ethtool_keee *e)
> {
> struct dsa_port *dp = dsa_user_to_port(dev);
> struct dsa_switch *ds = dp->ds;
> - int ret;
>
> /* Check whether the switch supports EEE */
> if (!ds->ops->support_eee || !ds->ops->support_eee(ds, dp->index))
> return -EOPNOTSUPP;
>
> - /* Port's PHY and MAC both need to be EEE capable */
> - if (!dev->phydev)
> - return -ENODEV;
It may well be that removing this test is ok given the later call to
phylink_ethtool_get_eee() which will fail with the same return code,
but this change does not logically pertain to a patch titled
"no longer call ds->ops->get_mac_eee()", and no justification is brought
for it in the commit message (my previous sentence should be sufficient).
Please move this to a separate patch, for traceability purposes.
> -
> - if (!ds->ops->get_mac_eee)
> - return -EOPNOTSUPP;
> -
> - ret = ds->ops->get_mac_eee(ds, dp->index, e);
> - if (ret)
> - return ret;
> -
> return phylink_ethtool_get_eee(dp->pl, e);
> }
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2)
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
` (6 preceding siblings ...)
2024-12-10 14:26 ` [PATCH RFC net-next 7/7] net: dsa: remove get_mac_eee() method Russell King (Oracle)
@ 2024-12-12 19:48 ` Vladimir Oltean
2025-01-02 18:18 ` Russell King (Oracle)
7 siblings, 1 reply; 13+ messages in thread
From: Vladimir Oltean @ 2024-12-12 19:48 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Florian Fainelli, Jakub Kicinski, netdev, Paolo Abeni,
Simon Horman, UNGLinuxDriver, Woojung Huh
On Tue, Dec 10, 2024 at 02:25:44PM +0000, Russell King (Oracle) wrote:
> This is part 2 of the DSA EEE cleanups, and is being sent out becaues it
> is relevant for the review of part 1, but would make part 1 too large.
>
> Patch 1 removes the useless setting of tx_lpi parameters in the
> ksz driver.
>
> Patch 2 removes the DSA core code that calls the get_mac_eee() operation.
> This needs to be done before removing the implementations because doing
> otherwise would cause dsa_user_get_eee() to return -EOPNOTSUPP.
>
> Patches 3..6 remove the trivial get_mac_eee() implementations from DSA
> drivers.
>
> Patch 7 finally removes the get_mac_eee() method from struct
> dsa_switch_ops.
I appreciate the splitting of the get_mac_eee() removal into multiple
patches per driver and 2 for the DSA framework. It should help BSP
backporters which target only a subset of DSA drivers. Monolithic
patches are harder to digest, and may have trivial context conflicts due
to unrelated changes.
The set looks good, please don't forget to also update the documentation.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee()
2024-12-12 19:44 ` Vladimir Oltean
@ 2025-01-02 17:56 ` Russell King (Oracle)
0 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2025-01-02 17:56 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Florian Fainelli, Jakub Kicinski, netdev, Paolo Abeni,
Simon Horman, UNGLinuxDriver, Woojung Huh
On Thu, Dec 12, 2024 at 09:44:19PM +0200, Vladimir Oltean wrote:
> On Tue, Dec 10, 2024 at 02:26:19PM +0000, Russell King (Oracle) wrote:
> > /* Check whether the switch supports EEE */
> > if (!ds->ops->support_eee || !ds->ops->support_eee(ds, dp->index))
> > return -EOPNOTSUPP;
> >
> > - /* Port's PHY and MAC both need to be EEE capable */
> > - if (!dev->phydev)
> > - return -ENODEV;
>
> It may well be that removing this test is ok given the later call to
> phylink_ethtool_get_eee() which will fail with the same return code,
> but this change does not logically pertain to a patch titled
> "no longer call ds->ops->get_mac_eee()", and no justification is brought
> for it in the commit message (my previous sentence should be sufficient).
> Please move this to a separate patch, for traceability purposes.
Let's say we split this from the patch, and leave the check in place for
this patch.
We then end up with:
/* Port's PHY and MAC both need to be EEE capable */
if (!dev->phydev)
return -ENODEV;
return phylink_ethtool_get_eee(dp->pl, e);
here.
At this point, we end up with this code:
if (!dev->phydev)
return -ENODEV;
followed by:
ret = -EOPNOTSUPP;
if (pl->phydev)
ret = phy_ethtool_get_eee(pl->phydev, eee);
return ret;
You seem to want that, so I'll drop the removal of this from the patch
series, especially as it changes the error that userspace sees - even
though it's different from what other ethernet drivers do. If we want
to address the !phydev return code issue, that can be done some other
time.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2)
2024-12-12 19:48 ` [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Vladimir Oltean
@ 2025-01-02 18:18 ` Russell King (Oracle)
0 siblings, 0 replies; 13+ messages in thread
From: Russell King (Oracle) @ 2025-01-02 18:18 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, Eric Dumazet,
Florian Fainelli, Jakub Kicinski, netdev, Paolo Abeni,
Simon Horman, UNGLinuxDriver, Woojung Huh
On Thu, Dec 12, 2024 at 09:48:53PM +0200, Vladimir Oltean wrote:
> On Tue, Dec 10, 2024 at 02:25:44PM +0000, Russell King (Oracle) wrote:
> > This is part 2 of the DSA EEE cleanups, and is being sent out becaues it
> > is relevant for the review of part 1, but would make part 1 too large.
> >
> > Patch 1 removes the useless setting of tx_lpi parameters in the
> > ksz driver.
> >
> > Patch 2 removes the DSA core code that calls the get_mac_eee() operation.
> > This needs to be done before removing the implementations because doing
> > otherwise would cause dsa_user_get_eee() to return -EOPNOTSUPP.
> >
> > Patches 3..6 remove the trivial get_mac_eee() implementations from DSA
> > drivers.
> >
> > Patch 7 finally removes the get_mac_eee() method from struct
> > dsa_switch_ops.
>
> I appreciate the splitting of the get_mac_eee() removal into multiple
> patches per driver and 2 for the DSA framework. It should help BSP
> backporters which target only a subset of DSA drivers. Monolithic
> patches are harder to digest, and may have trivial context conflicts due
> to unrelated changes.
>
> The set looks good, please don't forget to also update the documentation.
Sorry, but which documentation are you referring to?
$ grep get_mac_eee Documentation/networking/ drivers/net include/ net/ -r
$
No references to get_mac_eee() anywhere (except in the mt7530 driver
which I've missed - patches now included, will be in v2.)
Even looking for "et_mac_eee" in case of Documentation using
[sg]et_mac_eee() reveals nothing.
So, I don't think there's any documentation that these patches change.
There is this in Documentation/networking/dsa/dsa.rst:
- ``get_eee``: ethtool function which is used to query a switch port EEE settings,
this function should return the EEE state of the switch port MAC controller
and data-processing logic as well as query the PHY for its currently configured
EEE settings
First, realise that my patch set actually changes nothing - all these
implementations were entirely useless because everything they have been
doing has been overwritten by phylib since phylib's EEE support was
changed. Therefore, the above as wrong both before and after this
patch set.
Second, when phylink managed EEE gets merged, the above becomes true
again.
Think of the current phylib behaviour as a user visible regression
(because that's exactly what it is), and this patch set is part of a
move towards resolving that regression.
IMHO, phylib managed EEE support should never have been merged - not
in the way it was. There should have been either a way to transition
drivers to it without causing all this breakage, or there should have
been a commitment to getting this fixed in a timely manner (in the
same cycle that it was merged.)
What we have right now is a total trainwreck.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-01-02 18:18 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 1/7] net: dsa: ksz: remove setting of tx_lpi parameters Russell King (Oracle)
2024-12-12 19:38 ` Vladimir Oltean
2024-12-10 14:26 ` [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee() Russell King (Oracle)
2024-12-12 19:44 ` Vladimir Oltean
2025-01-02 17:56 ` Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 3/7] net: dsa: b53/bcm_sf2: remove b53_get_mac_eee() Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 4/7] net: dsa: ksz: remove ksz_get_mac_eee() Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 5/7] net: dsa: mv88e6xxx: remove mv88e6xxx_get_mac_eee() Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 6/7] net: dsa: qca: remove qca8k_get_mac_eee() Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 7/7] net: dsa: remove get_mac_eee() method Russell King (Oracle)
2024-12-12 19:48 ` [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Vladimir Oltean
2025-01-02 18:18 ` Russell King (Oracle)
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).