Re: [net-next,21/21] net: stmmac: rk: rk3506, rk3528 and kk3588 have rmii_mode in clock register
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-25 22:16:50
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 moves the RMII mode bit configuration from the dedicated set_to_rmii() functions to rk_gmac_powerup() using a new rmii_mode_mask field in the clock register structure.
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; }
For context, prior to this hunk, ret is set by rk_get_phy_intf_sel() to
hold the PHY interface value (e.g., PHY_INTF_SEL_RMII = 4). Then in the
block above (not shown here), if gmac_phy_intf_sel_mask is set, there is:
ret = regmap_write(bsp_priv->grf, bsp_priv->gmac_grf_reg, val);
This overwrites ret with the regmap_write() return value (0 on success).
+ if (bsp_priv->clock.rmii_mode_mask) {
+ val = rk_encode_wm16(ret == PHY_INTF_SEL_RMII,
+ bsp_priv->clock.rmii_mode_mask);For rk3588, gmac_phy_intf_sel_mask is set in rk3588_init(), so the previous block executes and ret becomes 0. Then here, the comparison ret == PHY_INTF_SEL_RMII evaluates to 0 == 4, which is always false. This means the RMII mode bit will never be set for rk3588 in RMII mode. Should this use bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII instead, or should the PHY interface value be saved to a separate variable before it gets overwritten?
+ + ret = rk_write_clock_grf_reg(bsp_priv, val); + if (ret < 0) + return ret; + }
[ ... ]