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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230220150840.GB19238@pengutronix.de>
Date:   Mon, 20 Feb 2023 16:08:40 +0100
From:   Oleksij Rempel <o.rempel@...gutronix.de>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, kernel@...gutronix.de,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org
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 |