From a0094edaf4920689dd6c26cb14a929ae80e290a0 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 26 Jan 2026 11:45:02 +0000 Subject: [PATCH 1/4] net: stmmac: rk: avoid phy_power_on() In https://lore.kernel.org/netdev/aDne1Ybuvbk0AwG0@shell.armlinux.org.uk/ I requested that a follow-up patch to change the name of dwmac-rk's phy_power_on() function, which clashes with the drivers/phy function of the same name. This can cause confusion when grepping for this function name, or when reviewing code. Thankfully, stmmac doesn't make use of drivers/phy which saves this from compile errors. However, as is the usual case when a request is made as part of a review, if the review leads to successful application of the patch the author doesn't bother following up with any such requests, and so the problem falls back onto the reviewer to address... so here is the solution. Rename dwmac-rk's function to rk_phy_power_ctl(), as the function both powers up and down. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1vkL1i-00000005usD-3lhz@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 0a95f54e725e..de420997e21f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -1498,7 +1498,7 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable) return 0; } -static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable) +static int rk_phy_power_ctl(struct rk_priv_data *bsp_priv, bool enable) { struct regulator *ldo = bsp_priv->regulator; struct device *dev = bsp_priv->dev; @@ -1692,7 +1692,7 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv) dev_err(dev, "NO interface defined!\n"); } - ret = phy_power_on(bsp_priv, true); + ret = rk_phy_power_ctl(bsp_priv, true); if (ret) { gmac_clk_enable(bsp_priv, false); return ret; @@ -1713,7 +1713,7 @@ static void rk_gmac_powerdown(struct rk_priv_data *gmac) pm_runtime_put_sync(gmac->dev); - phy_power_on(gmac, false); + rk_phy_power_ctl(gmac, false); gmac_clk_enable(gmac, false); } From af6eaf70189785c4b616c7bea75c1b46bb8498bc Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 26 Jan 2026 11:45:08 +0000 Subject: [PATCH 2/4] net: stmmac: rk: get rid of rk_phy_power_ctl() It is not worth having a common rk_phy_power_ctl() when the only difference is which regulator function is called. Also, passing true/false is non-descriptive. Split this function, moving the code appropriately into rk_phy_powerup() and rk_phy_powerdown(). Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1vkL1o-00000005usJ-08hy@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- .../net/ethernet/stmicro/stmmac/dwmac-rk.c | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index de420997e21f..0e66252eb5ae 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -1498,23 +1498,26 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable) return 0; } -static int rk_phy_power_ctl(struct rk_priv_data *bsp_priv, bool enable) +static int rk_phy_powerup(struct rk_priv_data *bsp_priv) { struct regulator *ldo = bsp_priv->regulator; - struct device *dev = bsp_priv->dev; int ret; - if (enable) { - ret = regulator_enable(ldo); - if (ret) - dev_err(dev, "fail to enable phy-supply\n"); - } else { - ret = regulator_disable(ldo); - if (ret) - dev_err(dev, "fail to disable phy-supply\n"); - } + ret = regulator_enable(ldo); + if (ret) + dev_err(bsp_priv->dev, "fail to enable phy-supply\n"); - return 0; + return ret; +} + +static void rk_phy_powerdown(struct rk_priv_data *bsp_priv) +{ + struct regulator *ldo = bsp_priv->regulator; + int ret; + + ret = regulator_disable(ldo); + if (ret) + dev_err(bsp_priv->dev, "fail to disable phy-supply\n"); } static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev, @@ -1692,7 +1695,7 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv) dev_err(dev, "NO interface defined!\n"); } - ret = rk_phy_power_ctl(bsp_priv, true); + ret = rk_phy_powerup(bsp_priv); if (ret) { gmac_clk_enable(bsp_priv, false); return ret; @@ -1713,7 +1716,7 @@ static void rk_gmac_powerdown(struct rk_priv_data *gmac) pm_runtime_put_sync(gmac->dev); - rk_phy_power_ctl(gmac, false); + rk_phy_powerdown(gmac); gmac_clk_enable(gmac, false); } From cdb5fdfcf39607823491668bf699024595431d61 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 26 Jan 2026 11:45:13 +0000 Subject: [PATCH 3/4] net: stmmac: rk: convert rk3328 to use bsp_priv->id rk3328 contains two GMAC instances - gmac2io and gmac2phy. While the gmac2io instance may be connected to an external PHY, the gmac2phy instance is permanently connected via RMII to an on-SoC integrated PHY. The driver currently tests for the gmac2phy instance by checking bsp_priv->integrated_phy (determined from the PHY's phy-is-integrated property) and sometimes that the interface mode is RMII. This works because the rk3328.dtsi has: gmac2phy: ethernet@ff550000 { compatible = "rockchip,rk3328-gmac"; phy-mode = "rmii"; phy-handle = <&phy>; mdio { phy: ethernet-phy@0 { phy-is-integrated; }; }; }; The driver contains a mechanism to look up the MMIO address in a table to determine bsp_priv->id, which is used for every other Rockchip device. Switch rk3328 to use this mechanism to determine bsp_priv->id and use that to select which GRF register is used for configuration, similarly to how the other Rockchip SoCs handle such differences. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1vkL1t-00000005usQ-0vjt@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index 0e66252eb5ae..c8b49ed2064a 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -570,8 +570,7 @@ static void rk3328_set_to_rmii(struct rk_priv_data *bsp_priv) { unsigned int reg; - reg = bsp_priv->integrated_phy ? RK3328_GRF_MAC_CON2 : - RK3328_GRF_MAC_CON1; + reg = bsp_priv->id ? RK3328_GRF_MAC_CON2 : RK3328_GRF_MAC_CON1; regmap_write(bsp_priv->grf, reg, RK3328_GMAC_PHY_INTF_SEL(PHY_INTF_SEL_RMII) | @@ -591,10 +590,7 @@ static int rk3328_set_speed(struct rk_priv_data *bsp_priv, { unsigned int reg; - if (interface == PHY_INTERFACE_MODE_RMII && bsp_priv->integrated_phy) - reg = RK3328_GRF_MAC_CON2; - else - reg = RK3328_GRF_MAC_CON1; + reg = bsp_priv->id ? RK3328_GRF_MAC_CON2 : RK3328_GRF_MAC_CON1; return rk_set_reg_speed(bsp_priv, &rk3328_reg_speed_data, reg, interface, speed); @@ -614,6 +610,13 @@ static const struct rk_gmac_ops rk3328_ops = { .set_speed = rk3328_set_speed, .integrated_phy_powerup = rk3328_integrated_phy_powerup, .integrated_phy_powerdown = rk_gmac_integrated_ephy_powerdown, + + .regs_valid = true, + .regs = { + 0xff540000, /* gmac2io */ + 0xff550000, /* gmac2phy */ + 0, /* sentinel */ + }, }; #define RK3366_GRF_SOC_CON6 0x0418 From a7ad67e9745d3347d98d70cbeea8e80a5f950e9e Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 26 Jan 2026 11:45:18 +0000 Subject: [PATCH 4/4] net: stmmac: rk: group MACPHY register offset and fields together Group the MACPHY register offsets and associated bitfields together to become self-documenting which definitions are associated with which register. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1vkL1y-00000005usW-1TKX@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c index c8b49ed2064a..5f8d2031b97c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c @@ -162,15 +162,17 @@ static int rk_set_clk_mac_speed(struct rk_priv_data *bsp_priv, ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE)) #define RK_GRF_MACPHY_CON0 0xb00 -#define RK_GRF_MACPHY_CON1 0xb04 -#define RK_GRF_MACPHY_CON2 0xb08 -#define RK_GRF_MACPHY_CON3 0xb0c - #define RK_MACPHY_ENABLE GRF_BIT(0) #define RK_MACPHY_DISABLE GRF_CLR_BIT(0) #define RK_MACPHY_CFG_CLK_50M GRF_BIT(14) #define RK_GMAC2PHY_RMII_MODE GRF_FIELD(7, 6, 1) + +#define RK_GRF_MACPHY_CON1 0xb04 + +#define RK_GRF_MACPHY_CON2 0xb08 #define RK_GRF_CON2_MACPHY_ID GRF_FIELD(15, 0, 0x1234) + +#define RK_GRF_MACPHY_CON3 0xb0c #define RK_GRF_CON3_MACPHY_ID GRF_FIELD(5, 0, 0x35) static void rk_gmac_integrated_ephy_powerup(struct rk_priv_data *priv)