Re: [PATCH net-next 7/9] net: stmmac: qcom-ethqos: set initial RGMII link clock to lowest speed

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware initialization ordering mismatch causes 1-second busy loop and RGMII DLL lock failure on first Gigabit link-up.
--

commit a0ec14dab79ed801bb626d58b6ec17e6c1024077
Author: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>

net: stmmac: qcom-ethqos: set initial RGMII link clock to lowest speed

This patch changes the initial RGMII link clock to 10 Mbps (2.5 MHz) during
probe to save power before a PHY link is established.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 2ca2f03de3017..4f9bef662c579 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -814,7 +814,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
>  				     "Failed to get serdes phy\n");
>  
>  	ethqos_set_clk_tx_rate(ethqos, NULL, plat_dat->phy_interface,
> -			       SPEED_1000);
> +			       SPEED_10);

[Severity: High]
The commit message states that fix_mac_speed updates the rate once a link is
established, but does fix_mac_speed actually update the rate?

Looking at stmmac_mac_link_up() in stmmac_main.c, it calls fix_mac_speed
before set_clk_tx_rate:

	if (priv->plat->fix_mac_speed)
		priv->plat->fix_mac_speed(priv->plat->bsp_priv, interface,
					  speed, mode);
    ...
	if (priv->plat->set_clk_tx_rate) {
		ret = priv->plat->set_clk_tx_rate(priv->plat->bsp_priv,
						priv->plat->clk_tx_i,
						interface, speed);

Because the initial clock is now SPEED_10, when a Gigabit link comes up,
ethqos_fix_mac_speed_rgmii() will wait for DLL lock while the clock is still
running at the slower rate:

		/* wait for DLL LOCK */
		if (read_poll_timeout_atomic(rgmii_readl, val,
					     val & SDC4_STATUS_DLL_LOCK,
					     1000, 1000000, true,
					     ethqos, SDC4_STATUS))
			dev_err(dev, "Timeout while waiting for DLL lock\n");

Will this cause a 1-second timeout and fail to lock the DLL on the first
Gigabit link-up?

>  
>  	qcom_ethqos_set_sgmii_loopback(ethqos, true);
>  	ethqos_set_func_clk_en(ethqos);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-shikra_ethernet-v1-0-a50765996035@oss.qualcomm.com?part=7