Skip to content

Commit

Permalink
Merge branch 'net-phylib-eee-cleanups'
Browse files Browse the repository at this point in the history
Russell King says:

====================
net: phylib EEE cleanups

Clean up phylib's EEE support. Patches previously posted as RFC as part
of the phylink EEE series.

Patch 1 changes the Marvell driver to use the state we store in
struct phy_device, rather than manually calling
phydev->eee_cfg.eee_enabled.

Patch 2 avoids genphy_c45_ethtool_get_eee() setting ->eee_enabled, as
we copy that from phydev->eee_cfg.eee_enabled later, and after patch 3
mo one uses this after calling genphy_c45_ethtool_get_eee(). In fact,
the only caller of this function now is phy_ethtool_get_eee().

As all callers to genphy_c45_eee_is_active() now pass NULL as its
is_enabled flag, this is no longer useful. Remove the argument in
patch 3.

Patch 4 updates the phylib documentation to make it absolutely clear
that phy_ethtool_get_eee() now fills in all members of struct
ethtool_keee, which is why we now have so many buggy network drivers.
====================

Link: https://patch.msgid.link/Z1GDZlFyF2fsFa3S@shell.armlinux.org.uk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Dec 7, 2024
2 parents 48697bd + f899c59 commit 7a2716a
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 19 deletions.
4 changes: 1 addition & 3 deletions drivers/net/phy/marvell.c
Original file line number Diff line number Diff line change
Expand Up @@ -1550,7 +1550,6 @@ static int m88e1540_get_fld(struct phy_device *phydev, u8 *msecs)

static int m88e1540_set_fld(struct phy_device *phydev, const u8 *msecs)
{
struct ethtool_keee eee;
int val, ret;

if (*msecs == ETHTOOL_PHY_FAST_LINK_DOWN_OFF)
Expand All @@ -1560,8 +1559,7 @@ static int m88e1540_set_fld(struct phy_device *phydev, const u8 *msecs)
/* According to the Marvell data sheet EEE must be disabled for
* Fast Link Down detection to work properly
*/
ret = genphy_c45_ethtool_get_eee(phydev, &eee);
if (!ret && eee.eee_enabled) {
if (phydev->eee_cfg.eee_enabled) {
phydev_warn(phydev, "Fast Link Down detection requires EEE to be disabled!\n");
return -EBUSY;
}
Expand Down
14 changes: 4 additions & 10 deletions drivers/net/phy/phy-c45.c
Original file line number Diff line number Diff line change
Expand Up @@ -1469,18 +1469,17 @@ EXPORT_SYMBOL_GPL(genphy_c45_plca_get_status);
* @phydev: target phy_device struct
* @adv: variable to store advertised linkmodes
* @lp: variable to store LP advertised linkmodes
* @is_enabled: variable to store EEE enabled/disabled configuration value
*
* Description: this function will read local and link partner PHY
* advertisements. Compare them return current EEE state.
*/
int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv,
unsigned long *lp, bool *is_enabled)
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_enabled, eee_active;
bool eee_active;
int ret;

ret = genphy_c45_read_eee_adv(phydev, tmp_adv);
Expand All @@ -1491,9 +1490,8 @@ int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv,
if (ret)
return ret;

eee_enabled = !linkmode_empty(tmp_adv);
linkmode_and(common, tmp_adv, tmp_lp);
if (eee_enabled && !linkmode_empty(common))
if (!linkmode_empty(tmp_adv) && !linkmode_empty(common))
eee_active = phy_check_valid(phydev->speed, phydev->duplex,
common);
else
Expand All @@ -1503,8 +1501,6 @@ int genphy_c45_eee_is_active(struct phy_device *phydev, unsigned long *adv,
linkmode_copy(adv, tmp_adv);
if (lp)
linkmode_copy(lp, tmp_lp);
if (is_enabled)
*is_enabled = eee_enabled;

return eee_active;
}
Expand All @@ -1521,15 +1517,13 @@ EXPORT_SYMBOL(genphy_c45_eee_is_active);
int genphy_c45_ethtool_get_eee(struct phy_device *phydev,
struct ethtool_keee *data)
{
bool is_enabled;
int ret;

ret = genphy_c45_eee_is_active(phydev, data->advertised,
data->lp_advertised, &is_enabled);
data->lp_advertised);
if (ret < 0)
return ret;

data->eee_enabled = is_enabled;
data->eee_active = phydev->eee_active;
linkmode_copy(data->supported, phydev->supported_eee);

Expand Down
9 changes: 4 additions & 5 deletions drivers/net/phy/phy.c
Original file line number Diff line number Diff line change
Expand Up @@ -988,8 +988,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, NULL);
err = genphy_c45_eee_is_active(phydev, NULL, NULL);
phydev->eee_active = err > 0;
phydev->enable_tx_lpi = phydev->eee_cfg.tx_lpi_enabled &&
phydev->eee_active;
Expand Down Expand Up @@ -1658,7 +1657,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, NULL);
ret = genphy_c45_eee_is_active(phydev, NULL, NULL);
if (ret < 0)
return ret;
if (!ret)
Expand Down Expand Up @@ -1702,8 +1701,8 @@ EXPORT_SYMBOL(phy_get_eee_err);
* @phydev: target phy_device struct
* @data: ethtool_keee data
*
* Description: reports the Supported/Advertisement/LP Advertisement
* capabilities, etc.
* Description: get the current EEE settings, filling in all members of
* @data.
*/
int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_keee *data)
{
Expand Down
2 changes: 1 addition & 1 deletion include/linux/phy.h
Original file line number Diff line number Diff line change
Expand Up @@ -1991,7 +1991,7 @@ int genphy_c45_plca_set_cfg(struct phy_device *phydev,
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, bool *is_enabled);
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,
Expand Down

0 comments on commit 7a2716a

Please sign in to comment.