Re: [net-next,v2,07/22] net: stmmac: rk: convert to mask-based interface mode configuration
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-27 00:41:10
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: convert to mask-based interface mode configuration This commit consolidates PHY interface mode configuration into common infrastructure by introducing mask fields for phy_intf_sel and rmii_mode, moving the register writes from SoC-specific functions into the common rk_gmac_powerup() function.
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 4117f6863ff0..35836ffdfc1d 100644
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1654,7 +1685,9 @@ static int rk_gmac_check_ops(struct rk_priv_data *bsp_priv) static int rk_gmac_powerup(struct rk_priv_data *bsp_priv) { struct device *dev = bsp_priv->dev; + u32 val; int ret; + u8 intf; ret = rk_gmac_check_ops(bsp_priv); if (ret)@@ -1664,6 +1697,26 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv) if (ret) return ret; + ret = rk_get_phy_intf_sel(bsp_priv->phy_iface); + if (ret < 0) + return ret;
Should this error path call gmac_clk_enable(bsp_priv, false) before
returning? The clocks were enabled just above, and the existing error
handling for rk_phy_powerup() later in this function disables the
clocks on failure:
ret = rk_phy_powerup(bsp_priv);
if (ret) {
gmac_clk_enable(bsp_priv, false);
return ret;
}
+
+ intf = ret;
+
+ if (bsp_priv->gmac_phy_intf_sel_mask ||
+ bsp_priv->gmac_rmii_mode_mask) {
+ /* If defined, encode the phy_intf_sel value */
+ val = rk_encode_wm16(intf, bsp_priv->gmac_phy_intf_sel_mask);
+
+ /* If defined, encode the RMII mode mask setting. */
+ val |= rk_encode_wm16(intf == PHY_INTF_SEL_RMII,
+ bsp_priv->gmac_rmii_mode_mask);
+
+ ret = regmap_write(bsp_priv->grf, bsp_priv->gmac_grf_reg, val);
+ if (ret < 0)
+ return ret;Same question here - should this also disable clocks before returning?
+ }