Re: [PATCH net-next v1 3/4] net: phy: do not force EEE support

On Mon, Feb 20, 2023 at 02:18:37PM +0000, Russell King (Oracle) wrote:

Hi,

A couple of minor points, but not sufficient not to prevent merging
this.

On Mon, Feb 20, 2023 at 02:56:04PM +0100, Oleksij Rempel wrote:

quoted

@@ -865,7 +864,12 @@ EXPORT_SYMBOL_GPL(genphy_c45_read_eee_abilities);
  */
 int genphy_c45_an_config_eee_aneg(struct phy_device *phydev)
 {
-	return genphy_c45_write_eee_adv(phydev, phydev->supported_eee);
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(adv) = {};

It would be nice to avoid this initialisation in the case where
eee_enabled is true, as this takes CPU cycles. However, not too
bothered about it as this isn't a fast path.

quoted

+
+	if (!phydev->eee_enabled)
+		return genphy_c45_write_eee_adv(phydev, adv);
+
+	return genphy_c45_write_eee_adv(phydev, phydev->advertising_eee);
 }
 
 /**

@@ -1431,17 +1435,17 @@ EXPORT_SYMBOL(genphy_c45_ethtool_get_eee);
 int genphy_c45_ethtool_set_eee(struct phy_device *phydev,
 			       struct ethtool_eee *data)
 {
-	__ETHTOOL_DECLARE_LINK_MODE_MASK(adv) = {};
 	int ret;
 
 	if (data->eee_enabled) {
+		phydev->eee_enabled = true;
 		if (data->advertised)
-			adv[0] = data->advertised;
-		else
-			linkmode_copy(adv, phydev->supported_eee);
+			phydev->advertising_eee[0] = data->advertised;

Is there a reason not to use ethtool_convert_legacy_u32_to_link_mode()?
I'm guessing this will be more efficient.

Or at leas more readable. I'll update it.
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |