Messages in this thread Patch in this message | | | Date | Mon, 20 Feb 2023 16:08:40 +0100 | From | Oleksij Rempel <> | Subject | 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: > > @@ -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. > > > + > > + 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 |
| |