Re: [PATCH net v4 2/2] net: phy: aquantia: remove usage of phy_set_max_speed
From: Maxime Chevallier
Date: Fri Sep 27 2024 - 12:38:16 EST
Hi,
On Thu, 26 Sep 2024 18:05:53 -0700
Abhishek Chauhan <quic_abchauha@xxxxxxxxxxx> 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@xxxxxxxxxxx/T/
> Signed-off-by: Abhishek Chauhan <quic_abchauha@xxxxxxxxxxx>
[...]
> +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