On 8/9/24 17:25, Simon Horman wrote:
On Thu, Aug 08, 2024 at 01:22:17PM +0200, Aleksandr Loktionov wrote:
...
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 1d0d2e5..cd7509f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -5641,50 +5641,77 @@ static int i40e_get_module_eeprom(struct net_device
*netdev,
return 0;
}
+static void i40e_eee_capability_to_kedata_supported(__le16 eee_capability_,
+ unsigned long *supported)
+{
+ const int eee_capability = le16_to_cpu(eee_capability_);
Hi Aleksandr,
Maybe u16 would be an appropriate type for eee_capability.
Also, using const seems excessive here.
+ static const int lut[] = {
+ ETHTOOL_LINK_MODE_100baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseT_Full_BIT,
+ ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+ ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+ ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+ };
+
+ linkmode_zero(supported);
+ for (unsigned int i = ARRAY_SIZE(lut); i--; )
+ if (eee_capability & (1 << (i + 1)))
Perhaps:
if (eee_capability & BIT(i + 1))
I would avoid any operations with @i other than using it as index:
lut[i]. We have link mode bits in the table, why to compute that again?
+ linkmode_set_bit(lut[i], supported);
+}
+
static int i40e_get_eee(struct net_device *netdev, struct ethtool_keee *edata)
{
struct i40e_netdev_priv *np = netdev_priv(netdev);
struct i40e_aq_get_phy_abilities_resp phy_cfg;
struct i40e_vsi *vsi = np->vsi;
struct i40e_pf *pf = vsi->back;
struct i40e_hw *hw = &pf->hw;
- int status = 0;
+ int status;
This change seems unrelated to the subject of this patch.
If so, please remove.
Hmm, it's remotely related, trivial, and makes code better;
I intentionally said nothing about this during our internal review ;)
--
// snip,
Aleksandr, please remember to address all issues pointed by Simon
