Re: [PATCH net-next v12 4/7] net: stmmac: qcom-ethqos: set serdes mode before powerup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Calling phy_set_mode_ext() with statically cached ethqos->phy_mode ignores dynamic phylink interface changes, leading to incorrect PHY performance state.
--

commit 1aa35b54e2c1e3097800bf61ac8665f69589631f
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

net: stmmac: qcom-ethqos: set serdes mode before powerup

Call phy_set_mode_ext() before phy_power_on() in
qcom_ethqos_serdes_powerup(). This is done ahead of adding support for the
firmware-managed EMAC on Qualcomm sa8255p.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -601,10 +601,19 @@ static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
>  	if (ret)
>  		return ret;
>  
> +	ret = phy_set_mode_ext(ethqos->serdes_phy, PHY_MODE_ETHERNET,
> +			       ethqos->phy_mode);

[Severity: High]
Does this ignore dynamic phylink interface changes by always using the
statically cached ethqos->phy_mode?

At probe, ethqos->phy_mode is statically assigned from the device tree.
On interface up, stmmac_open() calls qcom_ethqos_serdes_powerup(), which
calls phy_set_mode_ext() and then phy_power_on().

Later, when phylink resolves the link and dynamically switches the interface,
it calls ethqos_mac_finish_serdes() which calls phy_set_mode_ext() with the
new dynamically negotiated interface mode. However, because the PHY is already
powered on, the SCMI PHY driver caches the new performance state but never
applies it to the hardware.

If the user subsequently toggles the interface (ip link set eth0 down/up),
qcom_ethqos_serdes_powerup() is called again. Will this pass the stale, static
ethqos->phy_mode instead of the current phylink mode, thereby reverting the PHY
to the wrong configuration and applying the wrong performance state?

> +	if (ret)
> +		goto err_out;
> +
>  	ret = phy_power_on(ethqos->serdes_phy);
>  	if (ret)
> -		phy_exit(ethqos->serdes_phy);
> +		goto err_out;
>  
> +	return 0;
> +
> +err_out:
> +	phy_exit(ethqos->serdes_phy);
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-qcom-sa8255p-emac-v12-0-e3ab1ecf2901@oss.qualcomm.com?part=4