netdev - Re: [PATCH net v4 2/2] net: phy: aquantia: remove usage of phy_set_max

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: <20240927183756.16d3c6a3@fedora.home>
Date: Fri, 27 Sep 2024 18:37:56 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Abhishek Chauhan <quic_abchauha@...cinc.com>
Cc: "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>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>, kernel@...cinc.com
Subject: Re: [PATCH net v4 2/2] net: phy: aquantia: remove usage of
 phy_set_max_speed

Hi,

On Thu, 26 Sep 2024 18:05:53 -0700
Abhishek Chauhan <quic_abchauha@...cinc.com> 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.
> 
> Instead use get_features to fix up Phy PMA capabilities for
> AQR111, AQR111B0, AQR114C and AQCS109
> 
> 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")
> Link: https://lore.kernel.org/all/20240913011635.1286027-1-quic_abchauha@quicinc.com/T/
> Signed-off-by: Abhishek Chauhan <quic_abchauha@...cinc.com>

[...]

> +static int aqr111_get_features(struct phy_device *phydev)
> +{
> +	unsigned long *supported = phydev->supported;
> +	int ret;
> +
> +	/* Normal feature discovery */
> +	ret = genphy_c45_pma_read_abilities(phydev);
> +	if (ret)
> +		return ret;
> +
> +	/* PHY FIXUP */
> +	/* Although the PHY sets bit 12.18.19, it does not support 10G modes */
> +	linkmode_clear_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, supported);
> +	linkmode_clear_bit(ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT, supported);
> +	linkmode_clear_bit(ETHTOOL_LINK_MODE_10000baseKR_Full_BIT, supported);
> +
> +	/* Phy supports Speeds up to 5G with Autoneg though the phy PMA says otherwise */
> +	linkmode_or(supported, supported, phy_gbit_features);
> +	/* Set the 5G speed if it wasn't set as part of the PMA feature discovery */
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, supported);
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, supported);

As you are moving away from phy_set_max_speed(phydev, 5000), it should mean
that what used to be in the supported bits already contained the
5GBaseT bit, as phy_set_max_speed simply clears the highest speeds.

In such case, calling the newly introduced function from
patch 1 should be enough ?

Thanks,

Maxime