Re: [PATCH net 2/3] net: phy: as21xxx: fix read_status speed handling
From: Maxime Chevallier
Date: Thu Jun 04 2026 - 06:31:36 EST
On 6/1/26 12:53, Christian Marangi wrote:
> With further test with 2.5G NIC it was discovered that
> phy_resolve_aneg_linkmode is not enough to detect speed higher that 1G
> when autoneg is enabled.
>
> Also in the switch case there is a typo where the speed mask is AND with
> VEND1_SPEED_STATUS instead of the correct mask VEND1_SPEED_MASK.
>
> Rework the read_status code to always read the speed from the vendor
> register and parse the generic bit only for the pause frame.
>
> Fixes: 830877d89edc ("net: phy: Add support for Aeonsemi AS21xxx PHYs")
> Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
Reviewed-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>
Maxime
> ---
> drivers/net/phy/as21xxx.c | 96 +++++++++++++++++++++------------------
> 1 file changed, 53 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/phy/as21xxx.c b/drivers/net/phy/as21xxx.c
> index 0db82da8dbdf..97ca37c6929f 100644
> --- a/drivers/net/phy/as21xxx.c
> +++ b/drivers/net/phy/as21xxx.c
> @@ -671,7 +671,7 @@ static int as21xxx_read_link(struct phy_device *phydev, int *bmcr)
>
> static int as21xxx_read_c22_lpa(struct phy_device *phydev)
> {
> - int lpagb;
> + int lpagb, lpa;
>
> /* MII_STAT1000 are only filled in the mapped C22
> * in C45, use that to fill lpagb values and check.
> @@ -698,12 +698,20 @@ static int as21xxx_read_c22_lpa(struct phy_device *phydev)
> mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising,
> lpagb);
>
> + lpa = phy_read_mmd(phydev, MDIO_MMD_AN,
> + AS21XXX_MDIO_AN_C22 + MII_LPA);
> + if (lpa < 0)
> + return lpa;
> +
> + mii_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, lpa);
> +
> return 0;
> }
>
> static int as21xxx_read_status(struct phy_device *phydev)
> {
> int bmcr, old_link = phydev->link;
> + int speed;
> int ret;
>
> ret = as21xxx_read_link(phydev, &bmcr);
> @@ -720,58 +728,60 @@ static int as21xxx_read_status(struct phy_device *phydev)
> phydev->asym_pause = 0;
>
> if (phydev->autoneg == AUTONEG_ENABLE) {
> - ret = genphy_c45_read_lpa(phydev);
> - if (ret)
> - return ret;
> + if (!phydev->autoneg_complete) {
> + mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising,
> + 0);
> + mii_lpa_mod_linkmode_lpa_t(phydev->lp_advertising, 0);
> + return 0;
> + }
>
> ret = as21xxx_read_c22_lpa(phydev);
> if (ret)
> return ret;
> -
> - phy_resolve_aneg_linkmode(phydev);
> } else {
> - int speed;
> -
> linkmode_zero(phydev->lp_advertising);
> + }
>
> - speed = phy_read_mmd(phydev, MDIO_MMD_VEND1,
> - VEND1_SPEED_STATUS);
> - if (speed < 0)
> - return speed;
> -
> - switch (speed & VEND1_SPEED_STATUS) {
> - case VEND1_SPEED_10000:
> - phydev->speed = SPEED_10000;
> - phydev->duplex = DUPLEX_FULL;
> - break;
> - case VEND1_SPEED_5000:
> - phydev->speed = SPEED_5000;
> - phydev->duplex = DUPLEX_FULL;
> - break;
> - case VEND1_SPEED_2500:
> - phydev->speed = SPEED_2500;
> - phydev->duplex = DUPLEX_FULL;
> - break;
> - case VEND1_SPEED_1000:
> - phydev->speed = SPEED_1000;
> - if (bmcr & BMCR_FULLDPLX)
> - phydev->duplex = DUPLEX_FULL;
> - else
> - phydev->duplex = DUPLEX_HALF;
> - break;
> - case VEND1_SPEED_100:
> - phydev->speed = SPEED_100;
> + speed = phy_read_mmd(phydev, MDIO_MMD_VEND1,
> + VEND1_SPEED_STATUS);
> + if (speed < 0)
> + return speed;
> +
> + switch (speed & VEND1_SPEED_MASK) {
> + case VEND1_SPEED_10000:
> + phydev->speed = SPEED_10000;
> + phydev->duplex = DUPLEX_FULL;
> + break;
> + case VEND1_SPEED_5000:
> + phydev->speed = SPEED_5000;
> + phydev->duplex = DUPLEX_FULL;
> + break;
> + case VEND1_SPEED_2500:
> + phydev->speed = SPEED_2500;
> + phydev->duplex = DUPLEX_FULL;
> + break;
> + case VEND1_SPEED_1000:
> + phydev->speed = SPEED_1000;
> + if (bmcr & BMCR_FULLDPLX)
> phydev->duplex = DUPLEX_FULL;
> - break;
> - case VEND1_SPEED_10:
> - phydev->speed = SPEED_10;
> - phydev->duplex = DUPLEX_FULL;
> - break;
> - default:
> - return -EINVAL;
> - }
> + else
> + phydev->duplex = DUPLEX_HALF;
> + break;
> + case VEND1_SPEED_100:
> + phydev->speed = SPEED_100;
> + phydev->duplex = DUPLEX_FULL;
> + break;
> + case VEND1_SPEED_10:
> + phydev->speed = SPEED_10;
> + phydev->duplex = DUPLEX_FULL;
> + break;
> + default:
> + return -EINVAL;
> }
>
> + if (phydev->autoneg == AUTONEG_ENABLE)
> + phy_resolve_aneg_pause(phydev);
> +
> return 0;
> }
>