diff mbox series

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

Message ID 20260706-qcom-sa8255p-emac-v12-4-e3ab1ecf2901@oss.qualcomm.com (mailing list archive)
State New
Headers
Series net: stmmac: qcom-ethqos: add support for SCMI power domains |

Commit Message

Bartosz Golaszewski July 6, 2026, 1:20 p.m. UTC 
Call phy_set_mode_ext() before phy_power_on() in
qcom_ethqos_serdes_powerup(). This is harmless for existing users but on
SCMI systems this is required for the PHY driver to select the right
performance level - which translates to the link speed. This is done
ahead of adding support for the firmware-managed EMAC on Qualcomm sa8255p.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

sashiko-bot@kernel.org July 7, 2026, 2:11 p.m. UTC | #1 
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;
>  }
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index ac7d6d3e205a1ab5b391def879d6f1033a0961b6..47b70b5e706f221c01f1c0ae3b1acafae6641165 100644
--- 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);
+	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;
 }