From 8a6a77bb5a41d5a91c6155e1b902d9f75b5bf9a6 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 16 Feb 2025 22:15:42 +0100 Subject: [PATCH 1/6] net: phy: move definition of phy_is_started before phy_disable_eee_mode In preparation of a follow-up patch, move phy_is_started() to before phy_disable_eee_mode(). Signed-off-by: Heiner Kallweit Reviewed-by: Andrew Lunn Reviewed-by: Russell King (Oracle) Link: https://patch.msgid.link/04d1e7a5-f4c0-42ab-8fa4-88ad26b74813@gmail.com Signed-off-by: Jakub Kicinski --- include/linux/phy.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/include/linux/phy.h b/include/linux/phy.h index 70c632799082..96ea56de71b2 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1340,22 +1340,22 @@ void of_set_phy_timing_role(struct phy_device *phydev); int phy_speed_down_core(struct phy_device *phydev); /** - * phy_disable_eee_mode - Don't advertise an EEE mode. + * phy_is_started - Convenience function to check whether PHY is started * @phydev: The phy_device struct - * @link_mode: The EEE mode to be disabled */ -static inline void phy_disable_eee_mode(struct phy_device *phydev, u32 link_mode) +static inline bool phy_is_started(struct phy_device *phydev) { - linkmode_set_bit(link_mode, phydev->eee_disabled_modes); + return phydev->state >= PHY_UP; } /** - * phy_is_started - Convenience function to check whether PHY is started + * phy_disable_eee_mode - Don't advertise an EEE mode. * @phydev: The phy_device struct + * @link_mode: The EEE mode to be disabled */ -static inline bool phy_is_started(struct phy_device *phydev) +static inline void phy_disable_eee_mode(struct phy_device *phydev, u32 link_mode) { - return phydev->state >= PHY_UP; + linkmode_set_bit(link_mode, phydev->eee_disabled_modes); } void phy_resolve_aneg_pause(struct phy_device *phydev); From a9b6a860d7789d8183530aedbb46cf70f843e40d Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 16 Feb 2025 22:16:34 +0100 Subject: [PATCH 2/6] net: phy: improve phy_disable_eee_mode If a mode is to be disabled, remove it from advertising_eee. Disabling EEE modes shall be done before calling phy_start(), warn if that's not the case. Signed-off-by: Heiner Kallweit Reviewed-by: Andrew Lunn Reviewed-by: Russell King (Oracle) Link: https://patch.msgid.link/92164896-38ff-4474-b98b-e83fc05b9509@gmail.com Signed-off-by: Jakub Kicinski --- include/linux/phy.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/phy.h b/include/linux/phy.h index 96ea56de71b2..0d5da01d275c 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1355,7 +1355,10 @@ static inline bool phy_is_started(struct phy_device *phydev) */ static inline void phy_disable_eee_mode(struct phy_device *phydev, u32 link_mode) { + WARN_ON(phy_is_started(phydev)); + linkmode_set_bit(link_mode, phydev->eee_disabled_modes); + linkmode_clear_bit(link_mode, phydev->advertising_eee); } void phy_resolve_aneg_pause(struct phy_device *phydev); From 7f33fea6bb53d4dcc927a7aca81eb08b5c1fe63a Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 16 Feb 2025 22:17:48 +0100 Subject: [PATCH 3/6] net: phy: remove disabled EEE modes from advertising_eee in phy_probe A PHY driver may populate eee_disabled_modes in its probe or get_features callback, therefore filter the EEE advertisement read from the PHY. Signed-off-by: Heiner Kallweit Reviewed-by: Andrew Lunn Reviewed-by: Russell King (Oracle) Link: https://patch.msgid.link/493f3e2e-9cfc-445d-adbe-58d9c117a489@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/phy/phy_device.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 0d56e7c7f98c..7c4e1ad1864c 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -3580,22 +3580,21 @@ static int phy_probe(struct device *dev) if (err) goto out; - /* There is no "enabled" flag. If PHY is advertising, assume it is - * kind of enabled. - */ - phydev->eee_cfg.eee_enabled = !linkmode_empty(phydev->advertising_eee); + /* Get the EEE modes we want to prohibit. */ + of_set_phy_eee_broken(phydev); /* Some PHYs may advertise, by default, not support EEE modes. So, - * we need to clean them. + * we need to clean them. In addition remove all disabled EEE modes. */ - if (phydev->eee_cfg.eee_enabled) - linkmode_and(phydev->advertising_eee, phydev->supported_eee, - phydev->advertising_eee); + linkmode_and(phydev->advertising_eee, phydev->supported_eee, + phydev->advertising_eee); + linkmode_andnot(phydev->advertising_eee, phydev->advertising_eee, + phydev->eee_disabled_modes); - /* Get the EEE modes we want to prohibit. We will ask - * the PHY stop advertising these mode later on + /* There is no "enabled" flag. If PHY is advertising, assume it is + * kind of enabled. */ - of_set_phy_eee_broken(phydev); + phydev->eee_cfg.eee_enabled = !linkmode_empty(phydev->advertising_eee); /* Get master/slave strap overrides */ of_set_phy_timing_role(phydev); From aa951feb542662342223c8db582897969b64fb59 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 16 Feb 2025 22:18:42 +0100 Subject: [PATCH 4/6] net: phy: c45: Don't silently remove disabled EEE modes any longer when writing advertisement register advertising_eee is adjusted now whenever an EEE mode gets disabled. Therefore we can remove the silent removal of disabled EEE modes here. Signed-off-by: Heiner Kallweit Reviewed-by: Russell King (Oracle) Link: https://patch.msgid.link/e95b9dad-24a7-4e3e-9af9-6f0770cf1520@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/phy/phy-c45.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index dedbdcab83a5..b19055cfced0 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -683,13 +683,10 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_mdix); static int genphy_c45_write_eee_adv(struct phy_device *phydev, unsigned long *adv) { - __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp); int val, changed = 0; - linkmode_andnot(tmp, adv, phydev->eee_disabled_modes); - if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP1_FEATURES)) { - val = linkmode_to_mii_eee_cap1_t(tmp); + val = linkmode_to_mii_eee_cap1_t(adv); /* IEEE 802.3-2018 45.2.7.13 EEE advertisement 1 * (Register 7.60) @@ -707,7 +704,7 @@ static int genphy_c45_write_eee_adv(struct phy_device *phydev, } if (linkmode_intersects(phydev->supported_eee, PHY_EEE_CAP2_FEATURES)) { - val = linkmode_to_mii_eee_cap2_t(tmp); + val = linkmode_to_mii_eee_cap2_t(adv); /* IEEE 802.3-2022 45.2.7.16 EEE advertisement 2 * (Register 7.62) From 199d0ce385adbbf5b8532752a26e12413100c4ea Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 16 Feb 2025 22:19:23 +0100 Subject: [PATCH 5/6] net: phy: c45: use cached EEE advertisement in genphy_c45_ethtool_get_eee Now that disabled EEE modes are considered when populating advertising_eee, we can use this bitmap here instead of reading the PHY register. Signed-off-by: Heiner Kallweit Reviewed-by: Andrew Lunn Reviewed-by: Russell King (Oracle) Link: https://patch.msgid.link/e57ed3d4-d0bc-4f91-83f6-8f48dfb6d7d7@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/phy/phy-c45.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index b19055cfced0..fed5fb0ef321 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -1516,14 +1516,14 @@ int genphy_c45_ethtool_get_eee(struct phy_device *phydev, { int ret; - ret = genphy_c45_eee_is_active(phydev, data->advertised, - data->lp_advertised); + ret = genphy_c45_eee_is_active(phydev, NULL, data->lp_advertised); if (ret < 0) return ret; data->eee_active = phydev->eee_active; linkmode_andnot(data->supported, phydev->supported_eee, phydev->eee_disabled_modes); + linkmode_copy(data->advertised, phydev->advertising_eee); return 0; } EXPORT_SYMBOL(genphy_c45_ethtool_get_eee); From 809265fe96fe3eb7a85a9260356767587c482cb7 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Sun, 16 Feb 2025 22:20:07 +0100 Subject: [PATCH 6/6] net: phy: c45: remove local advertisement parameter from genphy_c45_eee_is_active After the last user has gone, we can remove the local advertisement parameter from genphy_c45_eee_is_active. Signed-off-by: Heiner Kallweit Reviewed-by: Andrew Lunn Reviewed-by: Russell King (Oracle) Link: https://patch.msgid.link/bd121330-9e28-4bc8-8422-794bd54d561f@gmail.com Signed-off-by: Jakub Kicinski --- drivers/net/phy/phy-c45.c | 31 +++++++++---------------------- drivers/net/phy/phy.c | 4 ++-- include/linux/phy.h | 3 +-- 3 files changed, 12 insertions(+), 26 deletions(-) diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index fed5fb0ef321..37c9a344bf4a 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -1464,42 +1464,29 @@ EXPORT_SYMBOL_GPL(genphy_c45_plca_get_status); /** * genphy_c45_eee_is_active - get EEE status * @phydev: target phy_device struct - * @adv: variable to store advertised linkmodes * @lp: variable to store LP advertised linkmodes * - * Description: this function will read local and link partner PHY - * advertisements. Compare them return current EEE state. + * Description: this function will read link partner PHY advertisement + * and compare it to local advertisement to return current EEE state. */ -int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv, - unsigned long *lp) +int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *lp) { - __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_adv) = {}; __ETHTOOL_DECLARE_LINK_MODE_MASK(tmp_lp) = {}; __ETHTOOL_DECLARE_LINK_MODE_MASK(common); - bool eee_active; int ret; - ret = genphy_c45_read_eee_adv(phydev, tmp_adv); - if (ret) - return ret; - ret = genphy_c45_read_eee_lpa(phydev, tmp_lp); if (ret) return ret; - linkmode_and(common, tmp_adv, tmp_lp); - if (!linkmode_empty(tmp_adv) && !linkmode_empty(common)) - eee_active = phy_check_valid(phydev->speed, phydev->duplex, - common); - else - eee_active = false; - - if (adv) - linkmode_copy(adv, tmp_adv); if (lp) linkmode_copy(lp, tmp_lp); - return eee_active; + linkmode_and(common, phydev->advertising_eee, tmp_lp); + if (linkmode_empty(common)) + return 0; + + return phy_check_valid(phydev->speed, phydev->duplex, common); } EXPORT_SYMBOL(genphy_c45_eee_is_active); @@ -1516,7 +1503,7 @@ int genphy_c45_ethtool_get_eee(struct phy_device *phydev, { int ret; - ret = genphy_c45_eee_is_active(phydev, NULL, data->lp_advertised); + ret = genphy_c45_eee_is_active(phydev, data->lp_advertised); if (ret < 0) return ret; diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index f2086b0b7b76..831b36839627 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -1031,7 +1031,7 @@ static int phy_check_link_status(struct phy_device *phydev) if (phydev->link && phydev->state != PHY_RUNNING) { phy_check_downshift(phydev); phydev->state = PHY_RUNNING; - err = genphy_c45_eee_is_active(phydev, NULL, NULL); + err = genphy_c45_eee_is_active(phydev, NULL); phydev->eee_active = err > 0; phydev->enable_tx_lpi = phydev->eee_cfg.tx_lpi_enabled && phydev->eee_active; @@ -1780,7 +1780,7 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable) if (!phydev->drv) return -EIO; - ret = genphy_c45_eee_is_active(phydev, NULL, NULL); + ret = genphy_c45_eee_is_active(phydev, NULL); if (ret < 0) return ret; if (!ret) diff --git a/include/linux/phy.h b/include/linux/phy.h index 0d5da01d275c..584710e084eb 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -2032,8 +2032,7 @@ int genphy_c45_plca_set_cfg(struct phy_device *phydev, const struct phy_plca_cfg *plca_cfg); int genphy_c45_plca_get_status(struct phy_device *phydev, struct phy_plca_status *plca_st); -int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv, - unsigned long *lp); +int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *lp); int genphy_c45_ethtool_get_eee(struct phy_device *phydev, struct ethtool_keee *data); int genphy_c45_ethtool_set_eee(struct phy_device *phydev,