linux-kernel - Re: [PATCH 7/8] net: intel: igb: Use linkmode helpers for EEE

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: <20240206103425.28e64a8f@device-28.home>
Date: Tue, 6 Feb 2024 10:34:25 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
 <pabeni@...hat.com>, Ariel Elior <aelior@...vell.com>, Manish Chopra
 <manishc@...vell.com>, Jesse Brandeburg <jesse.brandeburg@...el.com>, Tony
 Nguyen <anthony.l.nguyen@...el.com>, linux-usb@...r.kernel.org,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 intel-wired-lan@...ts.osuosl.org
Subject: Re: [PATCH 7/8] net: intel: igb: Use linkmode helpers for EEE

Hello Andrew,

On Sun, 04 Feb 2024 17:40:24 -0600
Andrew Lunn <andrew@...n.ch> wrote:

> Make use of the existing linkmode helpers for converting PHY EEE
> register values into links modes, now that ethtool_keee uses link
> modes, rather than u32 values.
> 
> Signed-off-by: Andrew Lunn <andrew@...n.ch>

[...]

> @@ -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;
> @@ -3138,14 +3142,21 @@ 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_100baseT_Full_BIT,

Probably a typo, I think it should be ETHTOOL_LINK_MODE_1000baseT_Full_BIT
here :)

> +			edata->advertised);
>  
>  	} else if (!edata->eee_enabled) {
>  		dev_err(&adapter->pdev->dev,
> @@ -3153,7 +3164,7 @@ static int igb_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._82575.eee_disable != !edata->eee_enabled) {
>  		hw->dev_spec._82575.eee_disable = !edata->eee_enabled;
>  		adapter->flags |= IGB_FLAG_EEE;
> 

Thanks,

Maxime