From 0cb6daf549f99cc978fe4dd531d2426313fc08e4 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Sun, 4 Feb 2024 12:13:02 +0000 Subject: [PATCH 1/6] net: stmmac: remove eee_enabled/eee_active in stmmac_ethtool_op_get_eee() stmmac_ethtool_op_get_eee() sets both eee_enabled and eee_active, and then goes on to call phylink_ethtool_get_eee(). phylink_ethtool_get_eee() will return -EOPNOTSUPP if there is no PHY present, otherwise calling phy_ethtool_get_eee() which in turn will call genphy_c45_ethtool_get_eee(). genphy_c45_ethtool_get_eee() will overwrite eee_enabled and eee_active with its own interpretation from the PHYs settings and negotiation result. Thus, when there is no PHY, stmmac_ethtool_op_get_eee() will fail with -EOPNOTSUPP, meaning eee_enabled and eee_active will not be returned to userspace. When there is a PHY, eee_enabled and eee_active will be overwritten by phylib, making the setting of these members in stmmac_ethtool_op_get_eee() entirely unnecessary. Remove this code, thus simplifying stmmac_ethtool_op_get_eee(). Reviewed-by: Andrew Lunn Reviewed-by: Florian Fainelli Signed-off-by: Russell King (Oracle) Reviewed-by: Serge Semin Link: https://lore.kernel.org/r/E1rWbMs-002cCV-EE@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index bbecb3b89535..411c3ac8cb17 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c @@ -859,8 +859,6 @@ static int stmmac_ethtool_op_get_eee(struct net_device *dev, if (!priv->dma_cap.eee) return -EOPNOTSUPP; - edata->eee_enabled = priv->eee_enabled; - edata->eee_active = priv->eee_active; edata->tx_lpi_timer = priv->tx_lpi_timer; edata->tx_lpi_enabled = priv->tx_lpi_enabled; From d0d8c548789d45391c7c592fb763fab2fee0f632 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Sun, 4 Feb 2024 12:13:07 +0000 Subject: [PATCH 2/6] net: sxgbe: remove eee_enabled/eee_active in sxgbe_get_eee() sxgbe_get_eee() sets edata->eee_active and edata->eee_enabled from its own copy, and then calls phy_ethtool_get_eee() which in turn will call genphy_c45_ethtool_get_eee(). genphy_c45_ethtool_get_eee() will overwrite eee_enabled and eee_active with its own interpretation from the PHYs settings and negotiation result. Therefore, setting these members in sxgbe_get_eee() is redundant. Remove this, and remove the priv->eee_active member which then becomes a write-only variable. Reviewed-by: Andrew Lunn Signed-off-by: Russell King (Oracle) Link: https://lore.kernel.org/r/E1rWbMx-002cCb-IU@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h | 1 - drivers/net/ethernet/samsung/sxgbe/sxgbe_ethtool.c | 2 -- drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c | 1 - 3 files changed, 4 deletions(-) diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h b/drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h index d14e0cfc3a6b..1458939c3bf5 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_common.h @@ -503,7 +503,6 @@ struct sxgbe_priv_data { bool tx_path_in_lpi_mode; int lpi_irq; int eee_enabled; - int eee_active; int tx_lpi_timer; }; diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_ethtool.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_ethtool.c index d93b628b7046..4a439b34114d 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_ethtool.c +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_ethtool.c @@ -140,8 +140,6 @@ static int sxgbe_get_eee(struct net_device *dev, if (!priv->hw_cap.eee) return -EOPNOTSUPP; - edata->eee_enabled = priv->eee_enabled; - edata->eee_active = priv->eee_active; edata->tx_lpi_timer = priv->tx_lpi_timer; return phy_ethtool_get_eee(dev->phydev, edata); diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c index 71439825ea4e..ecbe3994f2b1 100644 --- a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c +++ b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c @@ -130,7 +130,6 @@ bool sxgbe_eee_init(struct sxgbe_priv_data * const priv) if (phy_init_eee(ndev->phydev, true)) return false; - priv->eee_active = 1; timer_setup(&priv->eee_ctrl_timer, sxgbe_eee_ctrl_timer, 0); priv->eee_ctrl_timer.expires = SXGBE_LPI_TIMER(eee_timer); add_timer(&priv->eee_ctrl_timer); From b573cb0a55866830586eb26ce53e1163c1580667 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Sun, 4 Feb 2024 12:13:12 +0000 Subject: [PATCH 3/6] net: fec: remove eee_enabled/eee_active in fec_enet_get_eee() fec_enet_get_eee() sets edata->eee_active and edata->eee_enabled from its own copy, and then calls phy_ethtool_get_eee() which in turn will call genphy_c45_ethtool_get_eee(). genphy_c45_ethtool_get_eee() will overwrite eee_enabled and eee_active with its own interpretation from the PHYs settings and negotiation result. Therefore, setting these members in fec_enet_get_eee() is redundant. Remove this, and remove the setting of fep->eee.eee_active member which becomes a write-only variable. Signed-off-by: Russell King (Oracle) Reviewed-by: Andrew Lunn Link: https://lore.kernel.org/r/E1rWbN2-002cCh-MY@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/freescale/fec_main.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 63707e065141..42bdc01a304e 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -3139,8 +3139,6 @@ static int fec_enet_eee_mode_set(struct net_device *ndev, bool enable) } p->tx_lpi_enabled = enable; - p->eee_enabled = enable; - p->eee_active = enable; writel(sleep_cycle, fep->hwp + FEC_LPI_SLEEP); writel(wake_cycle, fep->hwp + FEC_LPI_WAKE); @@ -3160,8 +3158,6 @@ fec_enet_get_eee(struct net_device *ndev, struct ethtool_keee *edata) if (!netif_running(ndev)) return -ENETDOWN; - edata->eee_enabled = p->eee_enabled; - edata->eee_active = p->eee_active; edata->tx_lpi_timer = p->tx_lpi_timer; edata->tx_lpi_enabled = p->tx_lpi_enabled; From 409359c1c2ef30788477b150c8837d8714117d13 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Sun, 4 Feb 2024 12:13:17 +0000 Subject: [PATCH 4/6] net: bcmgenet: remove eee_enabled/eee_active in bcmgenet_get_eee() bcmgenet_get_eee() sets edata->eee_active and edata->eee_enabled from its own copy, and then calls phy_ethtool_get_eee() which in turn will call genphy_c45_ethtool_get_eee(). genphy_c45_ethtool_get_eee() will overwrite eee_enabled and eee_active with its own interpretation from the PHYs settings and negotiation result. Therefore, setting these members in bcmgenet_get_eee() is redundant, and can be removed. This also makes priv->eee.eee_active unnecessary, so remove this and use a local variable where appropriate. Reviewed-by: Andrew Lunn Reviewed-by: Florian Fainelli Signed-off-by: Russell King (Oracle) Link: https://lore.kernel.org/r/E1rWbN7-002cCn-RO@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/genet/bcmgenet.c | 8 +++----- drivers/net/ethernet/broadcom/genet/bcmmii.c | 5 +++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c index 051c31fb17c2..7396e2823e32 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c @@ -1313,7 +1313,6 @@ void bcmgenet_eee_enable_set(struct net_device *dev, bool enable, } priv->eee.eee_enabled = enable; - priv->eee.eee_active = enable; priv->eee.tx_lpi_enabled = tx_lpi_enabled; } @@ -1328,8 +1327,6 @@ static int bcmgenet_get_eee(struct net_device *dev, struct ethtool_keee *e) if (!dev->phydev) return -ENODEV; - e->eee_enabled = p->eee_enabled; - e->eee_active = p->eee_active; e->tx_lpi_enabled = p->tx_lpi_enabled; e->tx_lpi_timer = bcmgenet_umac_readl(priv, UMAC_EEE_LPI_TIMER); @@ -1340,6 +1337,7 @@ static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_keee *e) { struct bcmgenet_priv *priv = netdev_priv(dev); struct ethtool_keee *p = &priv->eee; + bool active; if (GENET_IS_V1(priv)) return -EOPNOTSUPP; @@ -1352,9 +1350,9 @@ static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_keee *e) if (!p->eee_enabled) { bcmgenet_eee_enable_set(dev, false, false); } else { - p->eee_active = phy_init_eee(dev->phydev, false) >= 0; + active = phy_init_eee(dev->phydev, false) >= 0; bcmgenet_umac_writel(priv, e->tx_lpi_timer, UMAC_EEE_LPI_TIMER); - bcmgenet_eee_enable_set(dev, p->eee_active, e->tx_lpi_enabled); + bcmgenet_eee_enable_set(dev, active, e->tx_lpi_enabled); } return phy_ethtool_set_eee(dev->phydev, e); diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c index 97ea76d443ab..cbbe004621bc 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c @@ -30,6 +30,7 @@ static void bcmgenet_mac_config(struct net_device *dev) struct bcmgenet_priv *priv = netdev_priv(dev); struct phy_device *phydev = dev->phydev; u32 reg, cmd_bits = 0; + bool active; /* speed */ if (phydev->speed == SPEED_1000) @@ -88,9 +89,9 @@ static void bcmgenet_mac_config(struct net_device *dev) } bcmgenet_umac_writel(priv, reg, UMAC_CMD); - priv->eee.eee_active = phy_init_eee(phydev, 0) >= 0; + active = phy_init_eee(phydev, 0) >= 0; bcmgenet_eee_enable_set(dev, - priv->eee.eee_enabled && priv->eee.eee_active, + priv->eee.eee_enabled && active, priv->eee.tx_lpi_enabled); } From 0cbfdfe3fb806e90e9c146974b250d7227f26790 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Sun, 4 Feb 2024 12:13:22 +0000 Subject: [PATCH 5/6] net: bcmasp: remove eee_enabled/eee_active in bcmasp_get_eee() bcmasp_get_eee() sets edata->eee_active and edata->eee_enabled from its own copy, and then calls phy_ethtool_get_eee() which in turn will call genphy_c45_ethtool_get_eee(). genphy_c45_ethtool_get_eee() will overwrite eee_enabled and eee_active with its own interpretation from the PHYs settings and negotiation result. Therefore, setting these members in bcmasp_get_eee() is redundant, and can be removed. This also makes intf->eee.eee_active unnecessary, so remove this and use a local variable where appropriate. Reviewed-by: Andrew Lunn Reviewed-by: Florian Fainelli Signed-off-by: Russell King (Oracle) Link: https://lore.kernel.org/r/E1rWbNC-002cCt-W7@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c | 4 ---- drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 5 +++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c b/drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c index 2851bed153e6..484fc2b5626f 100644 --- a/drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c @@ -360,7 +360,6 @@ void bcmasp_eee_enable_set(struct bcmasp_intf *intf, bool enable) umac_wl(intf, reg, UMC_EEE_CTRL); intf->eee.eee_enabled = enable; - intf->eee.eee_active = enable; } static int bcmasp_get_eee(struct net_device *dev, struct ethtool_keee *e) @@ -371,8 +370,6 @@ static int bcmasp_get_eee(struct net_device *dev, struct ethtool_keee *e) if (!dev->phydev) return -ENODEV; - e->eee_enabled = p->eee_enabled; - e->eee_active = p->eee_active; e->tx_lpi_enabled = p->tx_lpi_enabled; e->tx_lpi_timer = umac_rl(intf, UMC_EEE_LPI_TIMER); @@ -399,7 +396,6 @@ static int bcmasp_set_eee(struct net_device *dev, struct ethtool_keee *e) } umac_wl(intf, e->tx_lpi_timer, UMC_EEE_LPI_TIMER); - intf->eee.eee_active = ret >= 0; intf->eee.tx_lpi_enabled = e->tx_lpi_enabled; bcmasp_eee_enable_set(intf, true); } diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c index 53e542881255..3a15f269c7d1 100644 --- a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c @@ -607,6 +607,7 @@ static void bcmasp_adj_link(struct net_device *dev) struct phy_device *phydev = dev->phydev; u32 cmd_bits = 0, reg; int changed = 0; + bool active; if (intf->old_link != phydev->link) { changed = 1; @@ -658,8 +659,8 @@ static void bcmasp_adj_link(struct net_device *dev) reg |= cmd_bits; umac_wl(intf, reg, UMC_CMD); - intf->eee.eee_active = phy_init_eee(phydev, 0) >= 0; - bcmasp_eee_enable_set(intf, intf->eee.eee_active); + active = phy_init_eee(phydev, 0) >= 0; + bcmasp_eee_enable_set(intf, active); } reg = rgmii_rl(intf, RGMII_OOB_CNTRL); From 3465df5533af94af8e3d3a524329e21a7698618c Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Sun, 4 Feb 2024 12:13:28 +0000 Subject: [PATCH 6/6] net: dsa: b53: remove eee_enabled/eee_active in b53_get_mac_eee() b53_get_mac_eee() sets both eee_enabled and eee_active, and then returns zero. dsa_slave_get_eee(), which calls this function, will then continue to call phylink_ethtool_get_eee(), which will return -EOPNOTSUPP if there is no PHY present, otherwise calling phy_ethtool_get_eee() which in turn will call genphy_c45_ethtool_get_eee(). genphy_c45_ethtool_get_eee() will overwrite eee_enabled and eee_active with its own interpretation from the PHYs settings and negotiation result. Thus, when there is no PHY, dsa_slave_get_eee() will fail with -EOPNOTSUPP, meaning eee_enabled and eee_active will not be returned to userspace. When there is a PHY, eee_enabled and eee_active will be overwritten by phylib, making the setting of these members in b53_get_mac_eee() entirely unnecessary. Remove this code, thus simplifying b53_get_mac_eee(). Reviewed-by: Andrew Lunn Reviewed-by: Florian Fainelli Signed-off-by: Russell King (Oracle) Reviewed-by: Vladimir Oltean Link: https://lore.kernel.org/r/E1rWbNI-002cCz-4x@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/dsa/b53/b53_common.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index adc93abf4551..9e4c9bd6abcc 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -2227,16 +2227,10 @@ EXPORT_SYMBOL(b53_eee_init); int b53_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e) { struct b53_device *dev = ds->priv; - struct ethtool_keee *p = &dev->ports[port].eee; - u16 reg; if (is5325(dev) || is5365(dev)) return -EOPNOTSUPP; - b53_read16(dev, B53_EEE_PAGE, B53_EEE_LPI_INDICATE, ®); - e->eee_enabled = p->eee_enabled; - e->eee_active = !!(reg & BIT(port)); - return 0; } EXPORT_SYMBOL(b53_get_mac_eee);