Skip to content

Commit

Permalink
Merge branch 'eee-linkmode-bitmaps'
Browse files Browse the repository at this point in the history
Andrew Lunn says:

====================
drivers: net: Convert EEE handling to use linkmode bitmaps

EEE has until recently been limited to lower speeds due to the use of
the legacy u32 for link speeds. This restriction has been lifted, with
the use of linkmode bitmaps, added in the following patches:

1f069de ethtool: add linkmode bitmap support to struct ethtool_keee
1d756ff ethtool: add suffix _u32 to legacy bitmap members of struct ethtool_keee
285cc15 ethtool: adjust struct ethtool_keee to kernel needs
0b3100b ethtool: switch back from ethtool_keee to ethtool_eee for ioctl
d80a523 ethtool: replace struct ethtool_eee with a new struct ethtool_keee on kernel side

This patchset converts the remaining MAC drivers still using the old
_u32 to link modes.

A couple of Intel drivers do odd things with EEE, setting the autoneg
bit. It is unclear why, no other driver does, ethtool does not display
it, and EEE is always negotiated. One patch in this series deletes
this code.

With all users of the legacy _u32 changed to link modes, the _u32
values are removed from keee, and support for them in the ethtool core
is removed.

---
Changes in v5:
- Restore zeroing eee_data.advertised in ax8817_178a
- Fix lp_advertised -> supported in ixgdb
- Link to v4: https://lore.kernel.org/r/20240218-keee-u32-cleanup-v4-0-71f13b7c3e60@lunn.ch

Changes in v4:
- Add missing conversion in igb
- Add missing conversion in r8152
- Add patch to remove now unused _u32 members
- Link to v3: https://lore.kernel.org/r/20240217-keee-u32-cleanup-v3-0-fcf6b62a0c7f@lunn.ch

Changes in v3:
- Add list of commits adding linkmodes to EEE to cover letter
- Fix grammar error in cover letter.
- Add Reviewed-by from Jacob Keller
- Link to v2: https://lore.kernel.org/r/20240214-keee-u32-cleanup-v2-0-4ac534b83d66@lunn.ch

Changes in v2:
- igb: Fix type 100BaseT to 1000BaseT.
- Link to v1: https://lore.kernel.org/r/20240204-keee-u32-cleanup-v1-0-fb6e08329d9a@lunn.ch
====================

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Feb 28, 2024
2 parents 2e26b6d + 292fac4 commit 4ac8289
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 147 deletions.
17 changes: 12 additions & 5 deletions drivers/net/ethernet/intel/e1000e/ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -2223,16 +2223,16 @@ static int e1000e_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
ret_val = e1000_read_emi_reg_locked(hw, cap_addr, &phy_data);
if (ret_val)
goto release;
edata->supported_u32 = mmd_eee_cap_to_ethtool_sup_t(phy_data);
mii_eee_cap1_mod_linkmode_t(edata->supported, phy_data);

/* EEE Advertised */
edata->advertised_u32 = mmd_eee_adv_to_ethtool_adv_t(adapter->eee_advert);
mii_eee_cap1_mod_linkmode_t(edata->advertised, adapter->eee_advert);

/* EEE Link Partner Advertised */
ret_val = e1000_read_emi_reg_locked(hw, lpa_addr, &phy_data);
if (ret_val)
goto release;
edata->lp_advertised_u32 = mmd_eee_adv_to_ethtool_adv_t(phy_data);
mii_eee_cap1_mod_linkmode_t(edata->lp_advertised, phy_data);

/* EEE PCS Status */
ret_val = e1000_read_emi_reg_locked(hw, pcs_stat_addr, &phy_data);
Expand Down Expand Up @@ -2265,6 +2265,8 @@ static int e1000e_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
static int e1000e_set_eee(struct net_device *netdev, struct ethtool_keee *edata)
{
struct e1000_adapter *adapter = netdev_priv(netdev);
__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = {};
__ETHTOOL_DECLARE_LINK_MODE_MASK(tmp) = {};
struct e1000_hw *hw = &adapter->hw;
struct ethtool_keee eee_curr;
s32 ret_val;
Expand All @@ -2283,12 +2285,17 @@ static int e1000e_set_eee(struct net_device *netdev, struct ethtool_keee *edata)
return -EINVAL;
}

if (edata->advertised_u32 & ~(ADVERTISE_100_FULL | ADVERTISE_1000_FULL)) {
linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
supported);
linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
supported);

if (linkmode_andnot(tmp, edata->advertised, supported)) {
e_err("EEE advertisement supports only 100TX and/or 1000T full-duplex\n");
return -EINVAL;
}

adapter->eee_advert = ethtool_adv_to_mmd_eee_adv_t(edata->advertised_u32);
adapter->eee_advert = linkmode_to_mii_eee_cap1_t(edata->advertised);

hw->dev_spec.ich8lan.eee_disable = !edata->eee_enabled;

Expand Down
7 changes: 1 addition & 6 deletions drivers/net/ethernet/intel/i40e/i40e_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -5664,16 +5664,12 @@ static int i40e_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
if (phy_cfg.eee_capability == 0)
return -EOPNOTSUPP;

edata->supported_u32 = SUPPORTED_Autoneg;
edata->lp_advertised_u32 = edata->supported_u32;

/* Get current configuration */
status = i40e_aq_get_phy_capabilities(hw, false, false, &phy_cfg, NULL);
if (status)
return -EAGAIN;

edata->advertised_u32 = phy_cfg.eee_capability ? SUPPORTED_Autoneg : 0U;
edata->eee_enabled = !!edata->advertised_u32;
edata->eee_enabled = !!phy_cfg.eee_capability;
edata->tx_lpi_enabled = pf->stats.tx_lpi_status;

edata->eee_active = pf->stats.tx_lpi_status && pf->stats.rx_lpi_status;
Expand All @@ -5691,7 +5687,6 @@ static int i40e_is_eee_param_supported(struct net_device *netdev,
u32 value;
const char *name;
} param[] = {
{edata->advertised_u32 & ~SUPPORTED_Autoneg, "advertise"},
{edata->tx_lpi_timer, "tx-timer"},
{edata->tx_lpi_enabled != pf->stats.tx_lpi_status, "tx-lpi"}
};
Expand Down
35 changes: 23 additions & 12 deletions drivers/net/ethernet/intel/igb/igb_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -3038,11 +3038,13 @@ static int igb_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
(hw->phy.media_type != e1000_media_type_copper))
return -EOPNOTSUPP;

edata->supported_u32 = (SUPPORTED_1000baseT_Full |
SUPPORTED_100baseT_Full);
linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
edata->supported);
linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
edata->supported);
if (!hw->dev_spec._82575.eee_disable)
edata->advertised_u32 =
mmd_eee_adv_to_ethtool_adv_t(adapter->eee_advert);
mii_eee_cap1_mod_linkmode_t(edata->advertised,
adapter->eee_advert);

/* The IPCNFG and EEER registers are not supported on I354. */
if (hw->mac.type == e1000_i354) {
Expand All @@ -3068,7 +3070,7 @@ static int igb_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
if (ret_val)
return -ENODATA;

edata->lp_advertised_u32 = mmd_eee_adv_to_ethtool_adv_t(phy_data);
mii_eee_cap1_mod_linkmode_t(edata->lp_advertised, phy_data);
break;
case e1000_i354:
case e1000_i210:
Expand All @@ -3079,7 +3081,7 @@ static int igb_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
if (ret_val)
return -ENODATA;

edata->lp_advertised_u32 = mmd_eee_adv_to_ethtool_adv_t(phy_data);
mii_eee_cap1_mod_linkmode_t(edata->lp_advertised, phy_data);

break;
default:
Expand All @@ -3099,7 +3101,7 @@ static int igb_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
edata->eee_enabled = false;
edata->eee_active = false;
edata->tx_lpi_enabled = false;
edata->advertised_u32 &= ~edata->advertised_u32;
linkmode_zero(edata->advertised);
}

return 0;
Expand All @@ -3109,6 +3111,8 @@ static int igb_set_eee(struct net_device *netdev,
struct ethtool_keee *edata)
{
struct igb_adapter *adapter = netdev_priv(netdev);
__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = {};
__ETHTOOL_DECLARE_LINK_MODE_MASK(tmp) = {};
struct e1000_hw *hw = &adapter->hw;
struct ethtool_keee eee_curr;
bool adv1g_eee = true, adv100m_eee = true;
Expand Down Expand Up @@ -3138,22 +3142,29 @@ static int igb_set_eee(struct net_device *netdev,
return -EINVAL;
}

if (!edata->advertised_u32 || (edata->advertised_u32 &
~(ADVERTISE_100_FULL | ADVERTISE_1000_FULL))) {
linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
supported);
linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
supported);
if (linkmode_andnot(tmp, edata->advertised, supported)) {
dev_err(&adapter->pdev->dev,
"EEE Advertisement supports only 100Tx and/or 100T full duplex\n");
return -EINVAL;
}
adv100m_eee = !!(edata->advertised_u32 & ADVERTISE_100_FULL);
adv1g_eee = !!(edata->advertised_u32 & ADVERTISE_1000_FULL);
adv100m_eee = linkmode_test_bit(
ETHTOOL_LINK_MODE_100baseT_Full_BIT,
edata->advertised);
adv1g_eee = linkmode_test_bit(
ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
edata->advertised);

} else if (!edata->eee_enabled) {
dev_err(&adapter->pdev->dev,
"Setting EEE options are not supported with EEE disabled\n");
return -EINVAL;
}

adapter->eee_advert = ethtool_adv_to_mmd_eee_adv_t(edata->advertised_u32);
adapter->eee_advert = linkmode_to_mii_eee_cap1_t(edata->advertised);
if (hw->dev_spec._82575.eee_disable != !edata->eee_enabled) {
hw->dev_spec._82575.eee_disable = !edata->eee_enabled;
adapter->flags |= IGB_FLAG_EEE;
Expand Down
13 changes: 5 additions & 8 deletions drivers/net/ethernet/intel/igc/igc_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -1630,11 +1630,10 @@ static int igc_ethtool_get_eee(struct net_device *netdev,
u32 eeer;

if (hw->dev_spec._base.eee_enable)
edata->advertised_u32 =
mmd_eee_adv_to_ethtool_adv_t(adapter->eee_advert);
mii_eee_cap1_mod_linkmode_t(edata->advertised,
adapter->eee_advert);

*edata = adapter->eee;
edata->supported_u32 = SUPPORTED_Autoneg;

eeer = rd32(IGC_EEER);

Expand All @@ -1647,17 +1646,14 @@ static int igc_ethtool_get_eee(struct net_device *netdev,

edata->eee_enabled = hw->dev_spec._base.eee_enable;

edata->advertised_u32 = SUPPORTED_Autoneg;
edata->lp_advertised_u32 = SUPPORTED_Autoneg;

/* Report correct negotiated EEE status for devices that
* wrongly report EEE at half-duplex
*/
if (adapter->link_duplex == HALF_DUPLEX) {
edata->eee_enabled = false;
edata->eee_active = false;
edata->tx_lpi_enabled = false;
edata->advertised_u32 &= ~edata->advertised_u32;
linkmode_zero(edata->advertised);
}

return 0;
Expand Down Expand Up @@ -1699,7 +1695,8 @@ static int igc_ethtool_set_eee(struct net_device *netdev,
return -EINVAL;
}

adapter->eee_advert = ethtool_adv_to_mmd_eee_adv_t(edata->advertised_u32);
adapter->eee_advert = linkmode_to_mii_eee_cap1_t(edata->advertised);

if (hw->dev_spec._base.eee_enable != edata->eee_enabled) {
hw->dev_spec._base.eee_enable = edata->eee_enabled;
adapter->flags |= IGC_FLAG_EEE;
Expand Down
48 changes: 25 additions & 23 deletions drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -3403,30 +3403,31 @@ static int ixgbe_get_module_eeprom(struct net_device *dev,

static const struct {
ixgbe_link_speed mac_speed;
u32 supported;
u32 link_mode;
} ixgbe_ls_map[] = {
{ IXGBE_LINK_SPEED_10_FULL, SUPPORTED_10baseT_Full },
{ IXGBE_LINK_SPEED_100_FULL, SUPPORTED_100baseT_Full },
{ IXGBE_LINK_SPEED_1GB_FULL, SUPPORTED_1000baseT_Full },
{ IXGBE_LINK_SPEED_2_5GB_FULL, SUPPORTED_2500baseX_Full },
{ IXGBE_LINK_SPEED_10GB_FULL, SUPPORTED_10000baseT_Full },
{ IXGBE_LINK_SPEED_10_FULL, ETHTOOL_LINK_MODE_10baseT_Full_BIT },
{ IXGBE_LINK_SPEED_100_FULL, ETHTOOL_LINK_MODE_100baseT_Full_BIT },
{ IXGBE_LINK_SPEED_1GB_FULL, ETHTOOL_LINK_MODE_1000baseT_Full_BIT },
{ IXGBE_LINK_SPEED_2_5GB_FULL, ETHTOOL_LINK_MODE_2500baseX_Full_BIT },
{ IXGBE_LINK_SPEED_10GB_FULL, ETHTOOL_LINK_MODE_10000baseT_Full_BIT },
};

static const struct {
u32 lp_advertised;
u32 mac_speed;
u32 link_mode;
} ixgbe_lp_map[] = {
{ FW_PHY_ACT_UD_2_100M_TX_EEE, SUPPORTED_100baseT_Full },
{ FW_PHY_ACT_UD_2_1G_T_EEE, SUPPORTED_1000baseT_Full },
{ FW_PHY_ACT_UD_2_10G_T_EEE, SUPPORTED_10000baseT_Full },
{ FW_PHY_ACT_UD_2_1G_KX_EEE, SUPPORTED_1000baseKX_Full },
{ FW_PHY_ACT_UD_2_10G_KX4_EEE, SUPPORTED_10000baseKX4_Full },
{ FW_PHY_ACT_UD_2_10G_KR_EEE, SUPPORTED_10000baseKR_Full},
{ FW_PHY_ACT_UD_2_100M_TX_EEE, ETHTOOL_LINK_MODE_100baseT_Full_BIT },
{ FW_PHY_ACT_UD_2_1G_T_EEE, ETHTOOL_LINK_MODE_1000baseT_Full_BIT },
{ FW_PHY_ACT_UD_2_10G_T_EEE, ETHTOOL_LINK_MODE_10000baseT_Full_BIT },
{ FW_PHY_ACT_UD_2_1G_KX_EEE, ETHTOOL_LINK_MODE_1000baseKX_Full_BIT },
{ FW_PHY_ACT_UD_2_10G_KX4_EEE, ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT },
{ FW_PHY_ACT_UD_2_10G_KR_EEE, ETHTOOL_LINK_MODE_10000baseKR_Full_BIT},
};

static int
ixgbe_get_eee_fw(struct ixgbe_adapter *adapter, struct ethtool_keee *edata)
{
__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
u32 info[FW_PHY_ACT_DATA_COUNT] = { 0 };
struct ixgbe_hw *hw = &adapter->hw;
int rc;
Expand All @@ -3436,28 +3437,29 @@ ixgbe_get_eee_fw(struct ixgbe_adapter *adapter, struct ethtool_keee *edata)
if (rc)
return rc;

edata->lp_advertised_u32 = 0;
for (i = 0; i < ARRAY_SIZE(ixgbe_lp_map); ++i) {
if (info[0] & ixgbe_lp_map[i].lp_advertised)
edata->lp_advertised_u32 |= ixgbe_lp_map[i].mac_speed;
linkmode_set_bit(ixgbe_lp_map[i].link_mode,
edata->lp_advertised);
}

edata->supported_u32 = 0;
for (i = 0; i < ARRAY_SIZE(ixgbe_ls_map); ++i) {
if (hw->phy.eee_speeds_supported & ixgbe_ls_map[i].mac_speed)
edata->supported_u32 |= ixgbe_ls_map[i].supported;
linkmode_set_bit(ixgbe_lp_map[i].link_mode,
edata->supported);
}

edata->advertised_u32 = 0;
for (i = 0; i < ARRAY_SIZE(ixgbe_ls_map); ++i) {
if (hw->phy.eee_speeds_advertised & ixgbe_ls_map[i].mac_speed)
edata->advertised_u32 |= ixgbe_ls_map[i].supported;
linkmode_set_bit(ixgbe_lp_map[i].link_mode,
edata->advertised);
}

edata->eee_enabled = !!edata->advertised_u32;
edata->eee_enabled = !linkmode_empty(edata->advertised);
edata->tx_lpi_enabled = edata->eee_enabled;
if (edata->advertised_u32 & edata->lp_advertised_u32)
edata->eee_active = true;

linkmode_and(common, edata->advertised, edata->lp_advertised);
edata->eee_active = !linkmode_empty(common);

return 0;
}
Expand Down Expand Up @@ -3504,7 +3506,7 @@ static int ixgbe_set_eee(struct net_device *netdev, struct ethtool_keee *edata)
return -EINVAL;
}

if (eee_data.advertised_u32 != edata->advertised_u32) {
if (!linkmode_equal(eee_data.advertised, edata->advertised)) {
e_err(drv,
"Setting EEE advertised speeds is not supported\n");
return -EINVAL;
Expand Down
60 changes: 38 additions & 22 deletions drivers/net/ethernet/qlogic/qede/qede_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -1789,18 +1789,26 @@ static int qede_get_eee(struct net_device *dev, struct ethtool_keee *edata)
return -EOPNOTSUPP;
}

if (current_link.eee.adv_caps & QED_EEE_1G_ADV)
edata->advertised_u32 = ADVERTISED_1000baseT_Full;
if (current_link.eee.adv_caps & QED_EEE_10G_ADV)
edata->advertised_u32 |= ADVERTISED_10000baseT_Full;
if (current_link.sup_caps & QED_EEE_1G_ADV)
edata->supported_u32 = ADVERTISED_1000baseT_Full;
if (current_link.sup_caps & QED_EEE_10G_ADV)
edata->supported_u32 |= ADVERTISED_10000baseT_Full;
if (current_link.eee.lp_adv_caps & QED_EEE_1G_ADV)
edata->lp_advertised_u32 = ADVERTISED_1000baseT_Full;
if (current_link.eee.lp_adv_caps & QED_EEE_10G_ADV)
edata->lp_advertised_u32 |= ADVERTISED_10000baseT_Full;
linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
edata->advertised,
current_link.eee.adv_caps & QED_EEE_1G_ADV);
linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
edata->advertised,
current_link.eee.adv_caps & QED_EEE_10G_ADV);

linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
edata->supported,
current_link.sup_caps & QED_EEE_1G_ADV);
linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
edata->supported,
current_link.sup_caps & QED_EEE_10G_ADV);

linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
edata->lp_advertised,
current_link.eee.lp_adv_caps & QED_EEE_1G_ADV);
linkmode_mod_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
edata->lp_advertised,
current_link.eee.lp_adv_caps & QED_EEE_10G_ADV);

edata->tx_lpi_timer = current_link.eee.tx_lpi_timer;
edata->eee_enabled = current_link.eee.enable;
Expand All @@ -1812,9 +1820,12 @@ static int qede_get_eee(struct net_device *dev, struct ethtool_keee *edata)

static int qede_set_eee(struct net_device *dev, struct ethtool_keee *edata)
{
__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = {};
__ETHTOOL_DECLARE_LINK_MODE_MASK(tmp) = {};
struct qede_dev *edev = netdev_priv(dev);
struct qed_link_output current_link;
struct qed_link_params params;
bool unsupp;

if (!edev->ops->common->can_link_change(edev->cdev)) {
DP_INFO(edev, "Link settings are not allowed to be changed\n");
Expand All @@ -1832,21 +1843,26 @@ static int qede_set_eee(struct net_device *dev, struct ethtool_keee *edata)
memset(&params, 0, sizeof(params));
params.override_flags |= QED_LINK_OVERRIDE_EEE_CONFIG;

if (!(edata->advertised_u32 & (ADVERTISED_1000baseT_Full |
ADVERTISED_10000baseT_Full)) ||
((edata->advertised_u32 & (ADVERTISED_1000baseT_Full |
ADVERTISED_10000baseT_Full)) !=
edata->advertised_u32)) {
linkmode_set_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
supported);
linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
supported);

unsupp = linkmode_andnot(tmp, edata->advertised, supported);
if (unsupp) {
DP_VERBOSE(edev, QED_MSG_DEBUG,
"Invalid advertised capabilities %d\n",
edata->advertised_u32);
"Invalid advertised capabilities %*pb\n",
__ETHTOOL_LINK_MODE_MASK_NBITS, edata->advertised);
return -EINVAL;
}

if (edata->advertised_u32 & ADVERTISED_1000baseT_Full)
if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
edata->advertised))
params.eee.adv_caps = QED_EEE_1G_ADV;
if (edata->advertised_u32 & ADVERTISED_10000baseT_Full)
params.eee.adv_caps |= QED_EEE_10G_ADV;
if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
edata->advertised))
params.eee.adv_caps = QED_EEE_10G_ADV;

params.eee.enable = edata->eee_enabled;
params.eee.tx_lpi_enable = edata->tx_lpi_enabled;
params.eee.tx_lpi_timer = edata->tx_lpi_timer;
Expand Down
1 change: 1 addition & 0 deletions drivers/net/usb/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ config USB_RTL8150
config USB_RTL8152
tristate "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
select MII
select PHYLIB
select CRC32
select CRYPTO
select CRYPTO_HASH
Expand Down
Loading

0 comments on commit 4ac8289

Please sign in to comment.