netdev - Re: [PATCH net v2] net: phy: aquantia: Introduce custom get

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: <5b4110c2-ebd2-4beb-bd52-f2e15c5bb88b@intel.com>
Date: Tue, 24 Sep 2024 10:46:14 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Abhishek Chauhan <quic_abchauha@...cinc.com>
CC: <kernel@...cinc.com>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Andrew Halaney <ahalaney@...hat.com>,
	"Russell King (Oracle)" <linux@...linux.org.uk>, Andrew Lunn
	<andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, Bartosz Golaszewski
	<bartosz.golaszewski@...aro.org>, "linux-tegra@...r.kernel.org"
	<linux-tegra@...r.kernel.org>, Brad Griffis <bgriffis@...dia.com>, "Vladimir
 Oltean" <vladimir.oltean@....com>, Jon Hunter <jonathanh@...dia.com>, "Maxime
 Chevallier" <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net v2] net: phy: aquantia: Introduce custom get_features

On 9/24/24 07:52, Abhishek Chauhan wrote:
> Remove the use of phy_set_max_speed in phy driver as the
> function is mainly used in MAC driver to set the max
> speed.
> 
> Introduce custom get_features for AQR family of chipsets
> 
> 1. such as AQR111/B0/114c which supports speeds up to 5Gbps
> 2. such as AQR115c/AQCS109 which supports speeds up to 2.5Gbps
> 
> Fixes: 038ba1dc4e54 ("net: phy: aquantia: add AQR111 and AQR111B0 PHY ID")
> Fixes: 0974f1f03b07 ("net: phy: aquantia: remove false 5G and 10G speed ability for AQCS109")
> Fixes: c278ec644377 ("net: phy: aquantia: add support for AQR114C PHY ID")
> Fixes: 0ebc581f8a4b ("net: phy: aquantia: add support for aqr115c")
> Link: https://lore.kernel.org/all/20240913011635.1286027-1-quic_abchauha@quicinc.com/T/
> Signed-off-by: Abhishek Chauhan <quic_abchauha@...cinc.com>

I'm not sure if this patch is -net material

> +static void aqr_supported_speed(struct phy_device *phydev, u32 max_speed)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(supported) = { 0, };
> +
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Pause_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_FIBRE_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_TP_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, supported);
> +
> +	if (max_speed == SPEED_2500) {
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseX_Full_BIT, supported);
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
> +	} else if (max_speed == SPEED_5000) {
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseX_Full_BIT, supported);
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
> +		linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, supported);
> +	}

instead of duplicating, just make the lists incremental:

	if (max_speed >= SPEED_2500) {
		linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseX_Full_BIT, supported);
		linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);
	}
	if (max_speed >= SPEED_5000)
		linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, supported);

> +
> +	linkmode_copy(phydev->supported, supported);
> +}