Re: [net-next,v2,21/22] net: stmmac: rk: use rk_encode_wm16() for clock selection
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-27 00:41:14
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: use rk_encode_wm16() for clock selection This commit refactors the per-SoC set_clock_selection() method into unified helper functions using rk_encode_wm16(), with mask-based configuration stored in struct rk_clock_fields.
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 26fc93854cb0..443d3bd62cae 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1000,25 +991,23 @@ static const struct rk_gmac_ops rk3576_ops = { #define RK3588_GMAC_CLK_RMII_MODE(id) GRF_BIT(5 * (id)) #define RK3588_GMAC_CLK_RGMII_MODE(id) GRF_CLR_BIT(5 * (id)) -#define RK3588_GMAC_CLK_SELECT_CRU(id) GRF_BIT(5 * (id) + 4) -#define RK3588_GMAC_CLK_SELECT_IO(id) GRF_CLR_BIT(5 * (id) + 4) - -#define RK3588_GMAC_CLK_RMII_GATE(id) GRF_BIT(5 * (id) + 1) -#define RK3588_GMAC_CLK_RMII_NOGATE(id) GRF_CLR_BIT(5 * (id) + 1) - static int rk3588_init(struct rk_priv_data *bsp_priv) { switch (bsp_priv->id) { case 0: bsp_priv->gmac_phy_intf_sel_mask = GENMASK_U16(5, 3); + bsp_priv->clock.io_clksel_cru_mask = BIT_U16(4); bsp_priv->clock.gmii_clk_sel_mask = GENMASK_U16(3, 2); bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(2); + bsp_priv->clock.rmii_gate_en_mask = BIT_U16(1); return 0; case 1: bsp_priv->gmac_phy_intf_sel_mask = GENMASK_U16(11, 9); + bsp_priv->clock.io_clksel_cru_mask = BIT_U16(10);
^^
Should this be BIT_U16(9) instead of BIT_U16(10)?
The removed macro RK3588_GMAC_CLK_SELECT_CRU(id) used the formula
GRF_BIT(5 * (id) + 4), which for id=1 gives 5*1+4 = bit 9.
The other masks for id=1 follow the 5*(id) pattern correctly:
- rmii_gate_en_mask = BIT_U16(6) matches 5*1+1 = 6
- gmii_clk_sel_mask = GENMASK_U16(8, 7) matches 5*1+{2,3}
- rmii_clk_sel_mask = BIT_U16(7)
But io_clksel_cru_mask = BIT_U16(10) does not match 5*1+4 = 9.
bsp_priv->clock.gmii_clk_sel_mask = GENMASK_U16(8, 7); bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(7); + bsp_priv->clock.rmii_gate_en_mask = BIT_U16(6); return 0;
[ ... ]