Re: [PATCH net-next v5 6/6] net: phy: air_an8801: ensure maximum available speed link use

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] If the MDIO register write in `an8801r_read_status()` fails, the software state (`phydev->speed`) is left updated while the hardware is not configured for the new speed. This prevents subsequent retries and permanently misconfigures the PHY datapath until the link drops again.
--

commit 755d222adefc1329362882564596b829427a8bc6
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

net: phy: air_an8801: ensure maximum available speed link use

This commit configures the function mode of the Airoha AN8801R PHY for 1G
or 100M/10M operation after link detection. It implements a custom
read_status callback that writes to the link mode register based on the
detected speed.

> diff --git a/drivers/net/phy/air_an8801.c b/drivers/net/phy/air_an8801.c
> index aa24bb182d645..f97c78e430237 100644
> --- a/drivers/net/phy/air_an8801.c
> +++ b/drivers/net/phy/air_an8801.c

[ ... ]

> +static int an8801r_read_status(struct phy_device *phydev)
> +{
> +	int prev_speed, ret;
> +	u32 val;
> +
> +	prev_speed = phydev->speed;
> +
> +	ret = genphy_read_status(phydev);
> +	if (ret)
> +		return ret;
> +
> +	if (!phydev->link) {
> +		phydev->speed = SPEED_UNKNOWN;
> +		return 0;
> +	}
> +
> +	if (prev_speed != phydev->speed) {
> +		/* Ensure that PHY switches to 1G speed when available,
> +		 * by configuring the function mode for either 1G or 100M/10M
> +		 * operation.
> +		 * Therefore, set the link mode register, after read_status
> +		 * determines the link speed.
> +		 */
> +		val = phydev->speed == SPEED_1000 ?
> +		      AN8801_BPBUS_LINK_MODE_1000 : 0;
> +
> +		return an8801_buckpbus_reg_rmw(phydev,
> +					       AN8801_BPBUS_REG_LINK_MODE,
> +					       AN8801_BPBUS_LINK_MODE_1000,
> +					       val);

[Severity: Medium]
If the MDIO register write fails, does this leave the PHY state permanently
misconfigured until the link drops again?

genphy_read_status() successfully updates phydev->speed, but if
an8801_buckpbus_reg_rmw() fails, it returns an error. On the next polling
interval, prev_speed will now match phydev->speed, making the condition
check evaluate to false.

This means the register write will be skipped on subsequent retries,
leaving the MAC thinking the link is up at the new speed while the PHY
hardware remains physically in the incorrect mode.

Could the error path revert phydev->speed to prev_speed on failure, or
could the hardware register state be re-evaluated instead of relying on
the software state transition?

> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-add-airoha-an8801-support-v5-0-01aea8dee69b@collabora.com?part=6