Thread (25 messages) flat view 25 messages, 5 authors, 2d ago

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

From: netdev-bot+sashiko@kernel.org
Date: 2026-09-11 11:25:28
Also in: linux-arm-kernel, linux-arm-msm, linux-devicetree, lkml

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 hunk ↗ jump to hunk
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 hunk ↗ jump to hunk
@@ -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 hunk ↗ jump to hunk
@@ -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?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-shikra_ethernet-v2-0-bbe3389d0652%40oss.qualcomm.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help