Re: [PATCH net-next v2 5/9] net: stmmac: qcom-ethqos: fix RGMII_ID mode to use DLL bypass

On Fri, Sep 11, 2026 at 11:25:14AM +0000, netdev-bot+sashiko@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] The patch is worded as a bug fix ("fix RGMII_ID mode to use
  DLL bypass", "Also fix ethqos_set_clk_tx_rate()") for two user-visible…

--

net: stmmac: qcom-ethqos: fix RGMII_ID mode to use DLL bypass

For phy-mode = "rgmii-id" the driver now powers down the DLL, sets DDR
bypass and programs the IO_MACRO through a new
ethqos_rgmii_id_macro_init() helper instead of running the generic DLL
init.  It also stops doubling the link clock rate at 100M/10M in that
mode, and drops RGMII_ID from the TX phase-shift suppression in
ethqos_rgmii_macro_init().

quoted

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 44270c25d874f..53fb9de4d92c7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c

[ ... ]

quoted

@@ -183,7 +186,15 @@ static int ethqos_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i,
 	if (rate < 0)
 		return rate;
 
-	return clk_set_rate(ethqos->link_clk, rate * 2);
+	/* Clock Rate Requirements:
+	 * MAC added delay: 250/50/5 Mhz for 1G/100M/10M
+	 * No MAC delay (DLL bypass): 250/25/2.5 Mhz for 1G/100M/10M
+	 */
+	if (ethqos->phy_mode != PHY_INTERFACE_MODE_RGMII_ID ||
+	    speed == SPEED_1000)
+		rate *= 2;
+
+	return clk_set_rate(ethqos->link_clk, rate);
 }

[ ... ]

quoted

@@ -493,6 +539,21 @@ static void ethqos_fix_mac_speed_rgmii(void *bsp_priv,
 
 	ethqos_set_func_clk_en(ethqos);
 
+	/* For rgmii-id mode, the PHY should add the required delays.
+	 * Therefore, power down the DLL and program it in bypass mode.
+	 * Program the IO_MACRO as per the settings recommended by the
+	 * programming guide for bypass mode. This will ensure that the
+	 * MAC core doesn't add any additional delays.
+	 */
+	if (ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_ID) {
+		rgmii_setmask(ethqos, SDCC_DLL_CONFIG_PDN, SDCC_HC_REG_DLL_CONFIG);
+		rgmii_setmask(ethqos, SDCC_USR_CTL_DDR_BYPASS, SDCC_USR_CTL);
+
+		ethqos_rgmii_id_macro_init(ethqos, speed);
+
+		return;
+	}
+

[Severity: Medium]
The changelog describes this as two functional corrections, "fix RGMII_ID
mode to use DLL bypass" and "Also fix ethqos_set_clk_tx_rate() to not
double the clock rate in bypass mode at 100M/10M", but there is no Fixes:
tag.  Should one be added?

Both defects look reachable before this patch.  qcom_ethqos_probe() accepts
PHY_INTERFACE_MODE_RGMII_ID and installs the affected callback:

	case PHY_INTERFACE_MODE_RGMII_ID:
	case PHY_INTERFACE_MODE_RGMII_RXID:
	case PHY_INTERFACE_MODE_RGMII_TXID:
		plat_dat->fix_mac_speed = ethqos_fix_mac_speed_rgmii;

so an rgmii-id link previously ran the full DLL init and had the RX delay
programmed by ethqos_rgmii_macro_init() through
SDCC_DDR_CONFIG_PRG_RCLK_DLY / SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY, and also
got rate * 2 from ethqos_set_clk_tx_rate().

The partial mitigation for the TX side came in with 164a9ebe9742 ("net:
stmmac: dwmac-qcom-ethqos: Respect phy-mode and TX delay"), which only
suppressed RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN and left the RX delay and
the doubled rate in place.  Would that commit, or the original driver
commit, be the right Fixes: target here?

Related question: which platform reproduces the misprogramming, and on
which hardware was the new bypass register sequence in
ethqos_rgmii_id_macro_init() validated?  Grepping arch/arm64/boot/dts/qcom
turns up no ethqos node using phy-mode = "rgmii-id" (only sa8540p-ride.dts,
which uses "rgmii-txid"), so it is hard to tell from the changelog who is
affected and what the backport risk is.  Could that be stated in the commit
message?

Since there are no current ethqos users of "rgmii-id", nothing is really
broken in the current tree, so a Fixes: tag might not be required.

	Ayaan