Thread (31 messages) 31 messages, 3 authors, 2026-01-25

Re: [net-next,18/21] net: stmmac: rk: rk3328: gmac2phy only supports RMII

From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-25 22:16:48
Also in: linux-arm-kernel, linux-rockchip

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: stmmac: rk: rk3506, rk3528 and kk3588 have rmii_mode in clock register

This commit consolidates RMII mode configuration for rk3506, rk3528, and
rk3588 by adding a rmii_mode_mask field to rk_clock_fields, removing the
per-SoC set_to_rmii() functions, and adding common code in rk_gmac_powerup()
to write this field.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index ebbc09ea572a..759deb66ef2a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1412,6 +1385,15 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 			return ret;
 	}
The code above (not shown in the diff, but present in the function) is:

    ret = rk_get_phy_intf_sel(bsp_priv->phy_iface);
    ...
    if (bsp_priv->gmac_phy_intf_sel_mask ||
        bsp_priv->gmac_rmii_mode_mask) {
        ...
        ret = regmap_write(bsp_priv->grf, bsp_priv->gmac_grf_reg, val);
        if (ret < 0)
            return ret;
    }

At this point, if the block above executed, ret holds the return value from
regmap_write() (0 on success), not the interface selector.
+	if (bsp_priv->clock.rmii_mode_mask) {
+		val = rk_encode_wm16(ret == PHY_INTF_SEL_RMII,
+				     bsp_priv->clock.rmii_mode_mask);
For rk3588, both gmac_phy_intf_sel_mask and clock.rmii_mode_mask are set in
rk3588_init(). When the preceding block executes, it overwrites ret with
the regmap_write() return value. Then here, ret == PHY_INTF_SEL_RMII
compares 0 == 4 which is always false, so the rmii_mode bit will never be
set for rk3588 in RMII mode.

Should ret be saved before the preceding block so its original value
(the interface selector) can be used here?
+
+		ret = rk_write_clock_grf_reg(bsp_priv, val);
+		if (ret < 0)
+			return ret;
+	}
[ ... ]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help