[PATCH net-next 0/9] net: stmmac: rk: much needed cleanups

[PATCH net-next 0/9] net: stmmac: rk: much needed cleanups

[Russell King (Oracle)]

Hi,

This series starts attacking the reams of fairly identical duplicated
code in dwmac-rk. Every new SoC that comes along seems to need more
code added to this file because e.g. the way the clock is controlled
is different in every SoC.

The first thing to realise is that the driver only supports RMII and
RGMII interface modes. So, the first patch adds a .get_interfaces()
implementation which reports this for phylink's usage, thus ensuring
that we error out during initialisation should something that isn't
supported be specified. Note that there is one case where there are
a pair of interfaces, one supports only RMII the other supports RMII
and RGMII, but we report both anyway - something that the existing
driver allows. A future patch may attempt to fix this.

Rather than writing code, let's realise that there are two major
implementations here:

1. a struct clk that needs to be set.
2. writing a register with settings for RGMII and RMII speeds.

Provide implementations for these, Also realise that as a result
of doing this, we can kill off the .set_rgmii_speed() and
.set_rmii_speed() methods by combining them together - indeed,
this is what later SoCs already do by pointing both these methods
at the same function.

Overall, this patch series shrinks the file LOC by almost 8.7%
by removing 175 lines from over 2000 lines.

Apart from the error reporting changing and restricting interface
modes to those that the driver supports, no functional change is
anticipated with this patch. However, I have no hardware to test
this.

 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 689 +++++++++----------------
 1 file changed, 257 insertions(+), 432 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

[PATCH net-next 1/9] net: stmmac: rk: add get_interfaces() implementation

[Russell King (Oracle)]

RK platforms support RGMII and/or RMII depending on the SoC. Detect
whether support for a SoC exists by whether the interface specific
set_to functions have been populated, and set the appropriate bits in
phylink's bitmap of interfaces.

This assumes all dwmac interfaces on a SoC have identical support,
but it should be noted that this is not true for RK3528 which only
supports RGMII on GMAC1. However, the existing code structure
permits RGMII to be configured on GMAC0 without complaint, so
preserve this behaviour even though it is incorrect to avoid
functional change.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 700858ff6f7c..8006424ab027 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1864,6 +1864,18 @@ static void rk_gmac_powerdown(struct rk_priv_data *gmac)
 	gmac_clk_enable(gmac, false);
 }
 
+static void rk_get_interfaces(struct stmmac_priv *priv, void *bsp_priv,
+			      unsigned long *interfaces)
+{
+	struct rk_priv_data *rk = bsp_priv;
+
+	if (rk->ops->set_to_rgmii)
+		phy_interface_set_rgmii(interfaces);
+
+	if (rk->ops->set_to_rmii)
+		__set_bit(PHY_INTERFACE_MODE_RMII, interfaces);
+}
+
 static int rk_set_clk_tx_rate(void *bsp_priv_, struct clk *clk_tx_i,
 			      phy_interface_t interface, int speed)
 {
@@ -1919,6 +1931,7 @@ static int rk_gmac_probe(struct platform_device *pdev)
 		plat_dat->tx_fifo_size = 2048;
 	}
 
+	plat_dat->get_interfaces = rk_get_interfaces;
 	plat_dat->set_clk_tx_rate = rk_set_clk_tx_rate;
 
 	plat_dat->bsp_priv = rk_gmac_setup(pdev, plat_dat, data);
-- 
2.30.2

Re: [PATCH net-next 1/9] net: stmmac: rk: add get_interfaces() implementation

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:40:41PM +0100, Russell King (Oracle) wrote:
> RK platforms support RGMII and/or RMII depending on the SoC. Detect
> whether support for a SoC exists by whether the interface specific
> set_to functions have been populated, and set the appropriate bits in
> phylink's bitmap of interfaces.
> 
> This assumes all dwmac interfaces on a SoC have identical support,
> but it should be noted that this is not true for RK3528 which only
> supports RGMII on GMAC1. However, the existing code structure
> permits RGMII to be configured on GMAC0 without complaint, so
> preserve this behaviour even though it is incorrect to avoid
> functional change.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 2/9] net: stmmac: rk: simplify set_*_speed()

[Russell King (Oracle)]

Rather than having lots of regmap_write()s to the same register but
with different values depending on the speed, reorganise the
functions to use a local variable for the value, and then have one
regmap_write() call to write it to the register. This reduces the
amount of code and is a step towards further reducing the code size.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 303 ++++++++++--------
 1 file changed, 161 insertions(+), 142 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 8006424ab027..7a1a9f54748d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -264,35 +264,37 @@ static void rk3128_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3128_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con1;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1,
-			     RK3128_GMAC_CLK_2_5M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1,
-			     RK3128_GMAC_CLK_25M);
-	else if (speed == 1000)
-		regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1,
-			     RK3128_GMAC_CLK_125M);
-	else
+	if (speed == 10) {
+		con1 = RK3128_GMAC_CLK_2_5M;
+	} else if (speed == 100) {
+		con1 = RK3128_GMAC_CLK_25M;
+	} else if (speed == 1000) {
+		con1 = RK3128_GMAC_CLK_125M;
+	} else {
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1, con1);
 }
 
 static void rk3128_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con1;
 
 	if (speed == 10) {
-		regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1,
-			     RK3128_GMAC_RMII_CLK_2_5M |
-			     RK3128_GMAC_SPEED_10M);
+		con1 = RK3128_GMAC_RMII_CLK_2_5M | RK3128_GMAC_SPEED_10M;
 	} else if (speed == 100) {
-		regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1,
-			     RK3128_GMAC_RMII_CLK_25M |
-			     RK3128_GMAC_SPEED_100M);
+		con1 = RK3128_GMAC_RMII_CLK_25M | RK3128_GMAC_SPEED_100M;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
 	}
+
+	regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1, con1);
 }
 
 static const struct rk_gmac_ops rk3128_ops = {
@@ -361,34 +363,37 @@ static void rk3228_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3228_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con1;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1,
-			     RK3228_GMAC_CLK_2_5M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1,
-			     RK3228_GMAC_CLK_25M);
-	else if (speed == 1000)
-		regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1,
-			     RK3228_GMAC_CLK_125M);
-	else
+	if (speed == 10) {
+		con1 = RK3228_GMAC_CLK_2_5M;
+	} else if (speed == 100) {
+		con1 = RK3228_GMAC_CLK_25M;
+	} else if (speed == 1000) {
+		con1 = RK3228_GMAC_CLK_125M;
+	} else {
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1, con1);
 }
 
 static void rk3228_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con1;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1,
-			     RK3228_GMAC_RMII_CLK_2_5M |
-			     RK3228_GMAC_SPEED_10M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1,
-			     RK3228_GMAC_RMII_CLK_25M |
-			     RK3228_GMAC_SPEED_100M);
-	else
+	if (speed == 10) {
+		con1 = RK3228_GMAC_RMII_CLK_2_5M | RK3228_GMAC_SPEED_10M;
+	} else if (speed == 100) {
+		con1 = RK3228_GMAC_RMII_CLK_25M | RK3228_GMAC_SPEED_100M;
+	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1, con1);
 }
 
 static void rk3228_integrated_phy_powerup(struct rk_priv_data *priv)
@@ -457,35 +462,37 @@ static void rk3288_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3288_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con1;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
-			     RK3288_GMAC_CLK_2_5M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
-			     RK3288_GMAC_CLK_25M);
-	else if (speed == 1000)
-		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
-			     RK3288_GMAC_CLK_125M);
-	else
+	if (speed == 10) {
+		con1 = RK3288_GMAC_CLK_2_5M;
+	} else if (speed == 100) {
+		con1 = RK3288_GMAC_CLK_25M;
+	} else if (speed == 1000) {
+		con1 = RK3288_GMAC_CLK_125M;
+	} else {
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, con1);
 }
 
 static void rk3288_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con1;
 
 	if (speed == 10) {
-		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
-			     RK3288_GMAC_RMII_CLK_2_5M |
-			     RK3288_GMAC_SPEED_10M);
+		con1 = RK3288_GMAC_RMII_CLK_2_5M | RK3288_GMAC_SPEED_10M;
 	} else if (speed == 100) {
-		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
-			     RK3288_GMAC_RMII_CLK_25M |
-			     RK3288_GMAC_SPEED_100M);
+		con1 = RK3288_GMAC_RMII_CLK_25M | RK3288_GMAC_SPEED_100M;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
 	}
+
+	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, con1);
 }
 
 static const struct rk_gmac_ops rk3288_ops = {
@@ -514,16 +521,18 @@ static void rk3308_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3308_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con0;
 
 	if (speed == 10) {
-		regmap_write(bsp_priv->grf, RK3308_GRF_MAC_CON0,
-			     RK3308_GMAC_SPEED_10M);
+		con0 = RK3308_GMAC_SPEED_10M;
 	} else if (speed == 100) {
-		regmap_write(bsp_priv->grf, RK3308_GRF_MAC_CON0,
-			     RK3308_GMAC_SPEED_100M);
+		con0 = RK3308_GMAC_SPEED_100M;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
 	}
+
+	regmap_write(bsp_priv->grf, RK3308_GRF_MAC_CON0, con0);
 }
 
 static const struct rk_gmac_ops rk3308_ops = {
@@ -593,38 +602,40 @@ static void rk3328_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3328_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con1;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1,
-			     RK3328_GMAC_CLK_2_5M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1,
-			     RK3328_GMAC_CLK_25M);
-	else if (speed == 1000)
-		regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1,
-			     RK3328_GMAC_CLK_125M);
-	else
+	if (speed == 10) {
+		con1 = RK3328_GMAC_CLK_2_5M;
+	} else if (speed == 100) {
+		con1 = RK3328_GMAC_CLK_25M;
+	} else if (speed == 1000) {
+		con1 = RK3328_GMAC_CLK_125M;
+	} else {
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1, con1);
 }
 
 static void rk3328_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int reg;
+	unsigned int reg, con;
 
 	reg = bsp_priv->integrated_phy ? RK3328_GRF_MAC_CON2 :
 		  RK3328_GRF_MAC_CON1;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, reg,
-			     RK3328_GMAC_RMII_CLK_2_5M |
-			     RK3328_GMAC_SPEED_10M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, reg,
-			     RK3328_GMAC_RMII_CLK_25M |
-			     RK3328_GMAC_SPEED_100M);
-	else
+	if (speed == 10) {
+		con = RK3328_GMAC_RMII_CLK_2_5M | RK3328_GMAC_SPEED_10M;
+	} else if (speed == 100) {
+		con = RK3328_GMAC_RMII_CLK_25M | RK3328_GMAC_SPEED_100M;
+	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, reg, con);
 }
 
 static void rk3328_integrated_phy_powerup(struct rk_priv_data *priv)
@@ -693,35 +704,37 @@ static void rk3366_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3366_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con6;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6,
-			     RK3366_GMAC_CLK_2_5M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6,
-			     RK3366_GMAC_CLK_25M);
-	else if (speed == 1000)
-		regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6,
-			     RK3366_GMAC_CLK_125M);
-	else
+	if (speed == 10) {
+		con6 = RK3366_GMAC_CLK_2_5M;
+	} else if (speed == 100) {
+		con6 = RK3366_GMAC_CLK_25M;
+	} else if (speed == 1000) {
+		con6 = RK3366_GMAC_CLK_125M;
+	} else {
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6, con6);
 }
 
 static void rk3366_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con6;
 
 	if (speed == 10) {
-		regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6,
-			     RK3366_GMAC_RMII_CLK_2_5M |
-			     RK3366_GMAC_SPEED_10M);
+		con6 = RK3366_GMAC_RMII_CLK_2_5M | RK3366_GMAC_SPEED_10M;
 	} else if (speed == 100) {
-		regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6,
-			     RK3366_GMAC_RMII_CLK_25M |
-			     RK3366_GMAC_SPEED_100M);
+		con6 = RK3366_GMAC_RMII_CLK_25M | RK3366_GMAC_SPEED_100M;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
 	}
+
+	regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6, con6);
 }
 
 static const struct rk_gmac_ops rk3366_ops = {
@@ -780,35 +793,37 @@ static void rk3368_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3368_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con15;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15,
-			     RK3368_GMAC_CLK_2_5M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15,
-			     RK3368_GMAC_CLK_25M);
-	else if (speed == 1000)
-		regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15,
-			     RK3368_GMAC_CLK_125M);
-	else
+	if (speed == 10) {
+		con15 = RK3368_GMAC_CLK_2_5M;
+	} else if (speed == 100) {
+		con15 = RK3368_GMAC_CLK_25M;
+	} else if (speed == 1000) {
+		con15 = RK3368_GMAC_CLK_125M;
+	} else {
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15, con15);
 }
 
 static void rk3368_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con15;
 
 	if (speed == 10) {
-		regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15,
-			     RK3368_GMAC_RMII_CLK_2_5M |
-			     RK3368_GMAC_SPEED_10M);
+		con15 = RK3368_GMAC_RMII_CLK_2_5M | RK3368_GMAC_SPEED_10M;
 	} else if (speed == 100) {
-		regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15,
-			     RK3368_GMAC_RMII_CLK_25M |
-			     RK3368_GMAC_SPEED_100M);
+		con15 = RK3368_GMAC_RMII_CLK_25M | RK3368_GMAC_SPEED_100M;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
 	}
+
+	regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15, con15);
 }
 
 static const struct rk_gmac_ops rk3368_ops = {
@@ -867,35 +882,37 @@ static void rk3399_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3399_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con5;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5,
-			     RK3399_GMAC_CLK_2_5M);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5,
-			     RK3399_GMAC_CLK_25M);
-	else if (speed == 1000)
-		regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5,
-			     RK3399_GMAC_CLK_125M);
-	else
+	if (speed == 10) {
+		con5 = RK3399_GMAC_CLK_2_5M;
+	} else if (speed == 100) {
+		con5 = RK3399_GMAC_CLK_25M;
+	} else if (speed == 1000) {
+		con5 = RK3399_GMAC_CLK_125M;
+	} else {
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5, con5);
 }
 
 static void rk3399_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con5;
 
 	if (speed == 10) {
-		regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5,
-			     RK3399_GMAC_RMII_CLK_2_5M |
-			     RK3399_GMAC_SPEED_10M);
+		con5 = RK3399_GMAC_RMII_CLK_2_5M | RK3399_GMAC_SPEED_10M;
 	} else if (speed == 100) {
-		regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5,
-			     RK3399_GMAC_RMII_CLK_25M |
-			     RK3399_GMAC_SPEED_100M);
+		con5 = RK3399_GMAC_RMII_CLK_25M | RK3399_GMAC_SPEED_100M;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
 	}
+
+	regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5, con5);
 }
 
 static const struct rk_gmac_ops rk3399_ops = {
@@ -968,18 +985,20 @@ static void rk3528_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rk3528_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con5;
 
-	if (speed == 10)
-		regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5,
-			     RK3528_GMAC1_CLK_RGMII_DIV50);
-	else if (speed == 100)
-		regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5,
-			     RK3528_GMAC1_CLK_RGMII_DIV5);
-	else if (speed == 1000)
-		regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5,
-			     RK3528_GMAC1_CLK_RGMII_DIV1);
-	else
+	if (speed == 10) {
+		con5 = RK3528_GMAC1_CLK_RGMII_DIV50;
+	} else if (speed == 100) {
+		con5 = RK3528_GMAC1_CLK_RGMII_DIV5;
+	} else if (speed == 1000) {
+		con5 = RK3528_GMAC1_CLK_RGMII_DIV1;
+	} else {
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
+		return;
+	}
+
+	regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5, con5);
 }
 
 static void rk3528_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
@@ -987,13 +1006,13 @@ static void rk3528_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 	struct device *dev = &bsp_priv->pdev->dev;
 	unsigned int reg, val;
 
-	if (speed == 10)
+	if (speed == 10) {
 		val = bsp_priv->id == 1 ? RK3528_GMAC1_CLK_RMII_DIV20 :
 					  RK3528_GMAC0_CLK_RMII_DIV20;
-	else if (speed == 100)
+	} else if (speed == 100) {
 		val = bsp_priv->id == 1 ? RK3528_GMAC1_CLK_RMII_DIV2 :
 					  RK3528_GMAC0_CLK_RMII_DIV2;
-	else {
+	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
 		return;
 	}
@@ -1430,18 +1449,18 @@ static void rv1108_set_to_rmii(struct rk_priv_data *bsp_priv)
 static void rv1108_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con0;
 
 	if (speed == 10) {
-		regmap_write(bsp_priv->grf, RV1108_GRF_GMAC_CON0,
-			     RV1108_GMAC_RMII_CLK_2_5M |
-			     RV1108_GMAC_SPEED_10M);
+		con0 = RV1108_GMAC_RMII_CLK_2_5M | RV1108_GMAC_SPEED_10M;
 	} else if (speed == 100) {
-		regmap_write(bsp_priv->grf, RV1108_GRF_GMAC_CON0,
-			     RV1108_GMAC_RMII_CLK_25M |
-			     RV1108_GMAC_SPEED_100M);
+		con0 = RV1108_GMAC_RMII_CLK_25M | RV1108_GMAC_SPEED_100M;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
 	}
+
+	regmap_write(bsp_priv->grf, RV1108_GRF_GMAC_CON0, con0);
 }
 
 static const struct rk_gmac_ops rv1108_ops = {
-- 
2.30.2

Re: [PATCH net-next 2/9] net: stmmac: rk: simplify set_*_speed()

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:40:46PM +0100, Russell King (Oracle) wrote:
> Rather than having lots of regmap_write()s to the same register but
> with different values depending on the speed, reorganise the
> functions to use a local variable for the value, and then have one
> regmap_write() call to write it to the register. This reduces the
> amount of code and is a step towards further reducing the code size.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 3/9] net: stmmac: rk: add struct for programming register based speeds

[Russell King (Oracle)]

There is a common pattern in the driver where many SoCs need to write a
single register with a value dependent on the interface mode and speed.
Rather than having a lot of repeated code, add some common functions
and a struct to contain the values to be written to a register to
select the RGMII and RMII speeds.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 432 +++++++++---------
 1 file changed, 204 insertions(+), 228 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 7a1a9f54748d..7b5e989bb77f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -24,6 +24,15 @@
 #include "stmmac_platform.h"
 
 struct rk_priv_data;
+
+struct rk_reg_speed_data {
+	unsigned int rgmii_10;
+	unsigned int rgmii_100;
+	unsigned int rgmii_1000;
+	unsigned int rmii_10;
+	unsigned int rmii_100;
+};
+
 struct rk_gmac_ops {
 	void (*set_to_rgmii)(struct rk_priv_data *bsp_priv,
 			     int tx_delay, int rx_delay);
@@ -83,6 +92,67 @@ struct rk_priv_data {
 	struct regmap *php_grf;
 };
 
+static int rk_set_reg_speed(struct rk_priv_data *bsp_priv,
+			    const struct rk_reg_speed_data *rsd,
+			    unsigned int reg, phy_interface_t interface,
+			    int speed)
+{
+	unsigned int val;
+
+	if (phy_interface_mode_is_rgmii(interface)) {
+		if (speed == SPEED_10) {
+			val = rsd->rgmii_10;
+		} else if (speed == SPEED_100) {
+			val = rsd->rgmii_100;
+		} else if (speed == SPEED_1000) {
+			val = rsd->rgmii_1000;
+		} else {
+			/* Phylink will not allow inappropriate speeds for
+			 * interface modes, so this should never happen.
+			 */
+			return -EINVAL;
+		}
+	} else if (interface == PHY_INTERFACE_MODE_RMII) {
+		if (speed == SPEED_10) {
+			val = rsd->rmii_10;
+		} else if (speed == SPEED_100) {
+			val = rsd->rmii_100;
+		} else {
+			/* Phylink will not allow inappropriate speeds for
+			 * interface modes, so this should never happen.
+			 */
+			return -EINVAL;
+		}
+	} else {
+		/* This should never happen, as .get_interfaces() limits
+		 * the interface modes that are supported to RGMII and/or
+		 * RMII.
+		 */
+		return -EINVAL;
+	}
+
+	regmap_write(bsp_priv->grf, reg, val);
+
+	return 0;
+
+}
+
+static int rk_set_reg_speed_rgmii(struct rk_priv_data *bsp_priv,
+				  const struct rk_reg_speed_data *rsd,
+				  unsigned int reg, int speed)
+{
+	return rk_set_reg_speed(bsp_priv, rsd, reg, PHY_INTERFACE_MODE_RGMII,
+				speed);
+}
+
+static int rk_set_reg_speed_rmii(struct rk_priv_data *bsp_priv,
+				 const struct rk_reg_speed_data *rsd,
+				 unsigned int reg, int speed)
+{
+	return rk_set_reg_speed(bsp_priv, rsd, reg, PHY_INTERFACE_MODE_RMII,
+				speed);
+}
+
 #define HIWORD_UPDATE(val, mask, shift) \
 		((val) << (shift) | (mask) << ((shift) + 16))
 
@@ -261,40 +331,30 @@ static void rk3128_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3128_GMAC_PHY_INTF_SEL_RMII | RK3128_GMAC_RMII_MODE);
 }
 
+static const struct rk_reg_speed_data rk3128_reg_speed_data = {
+	.rgmii_10 = RK3128_GMAC_CLK_2_5M,
+	.rgmii_100 = RK3128_GMAC_CLK_25M,
+	.rgmii_1000 = RK3128_GMAC_CLK_125M,
+	.rmii_10 = RK3128_GMAC_RMII_CLK_2_5M | RK3128_GMAC_SPEED_10M,
+	.rmii_100 = RK3128_GMAC_RMII_CLK_25M | RK3128_GMAC_SPEED_100M,
+};
+
 static void rk3128_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con1;
 
-	if (speed == 10) {
-		con1 = RK3128_GMAC_CLK_2_5M;
-	} else if (speed == 100) {
-		con1 = RK3128_GMAC_CLK_25M;
-	} else if (speed == 1000) {
-		con1 = RK3128_GMAC_CLK_125M;
-	} else {
+	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3128_reg_speed_data,
+				   RK3128_GRF_MAC_CON1, speed))
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1, con1);
 }
 
 static void rk3128_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con1;
 
-	if (speed == 10) {
-		con1 = RK3128_GMAC_RMII_CLK_2_5M | RK3128_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con1 = RK3128_GMAC_RMII_CLK_25M | RK3128_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rk3128_reg_speed_data,
+				  RK3128_GRF_MAC_CON1, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3128_GRF_MAC_CON1, con1);
 }
 
 static const struct rk_gmac_ops rk3128_ops = {
@@ -360,40 +420,30 @@ static void rk3228_set_to_rmii(struct rk_priv_data *bsp_priv)
 	regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1, GRF_BIT(11));
 }
 
+static const struct rk_reg_speed_data rk3228_reg_speed_data = {
+	.rgmii_10 = RK3228_GMAC_CLK_2_5M,
+	.rgmii_100 = RK3228_GMAC_CLK_25M,
+	.rgmii_1000 = RK3228_GMAC_CLK_125M,
+	.rmii_10 = RK3228_GMAC_RMII_CLK_2_5M | RK3228_GMAC_SPEED_10M,
+	.rmii_100 = RK3228_GMAC_RMII_CLK_25M | RK3228_GMAC_SPEED_100M,
+};
+
 static void rk3228_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con1;
 
-	if (speed == 10) {
-		con1 = RK3228_GMAC_CLK_2_5M;
-	} else if (speed == 100) {
-		con1 = RK3228_GMAC_CLK_25M;
-	} else if (speed == 1000) {
-		con1 = RK3228_GMAC_CLK_125M;
-	} else {
+	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3228_reg_speed_data,
+				   RK3228_GRF_MAC_CON1, speed))
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1, con1);
 }
 
 static void rk3228_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con1;
 
-	if (speed == 10) {
-		con1 = RK3228_GMAC_RMII_CLK_2_5M | RK3228_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con1 = RK3228_GMAC_RMII_CLK_25M | RK3228_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rk3228_reg_speed_data,
+				  RK3228_GRF_MAC_CON1, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3228_GRF_MAC_CON1, con1);
 }
 
 static void rk3228_integrated_phy_powerup(struct rk_priv_data *priv)
@@ -459,40 +509,30 @@ static void rk3288_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3288_GMAC_PHY_INTF_SEL_RMII | RK3288_GMAC_RMII_MODE);
 }
 
+static const struct rk_reg_speed_data rk3288_reg_speed_data = {
+	.rgmii_10 = RK3288_GMAC_CLK_2_5M,
+	.rgmii_100 = RK3288_GMAC_CLK_25M,
+	.rgmii_1000 = RK3288_GMAC_CLK_125M,
+	.rmii_10 = RK3288_GMAC_RMII_CLK_2_5M | RK3288_GMAC_SPEED_10M,
+	.rmii_100 = RK3288_GMAC_RMII_CLK_25M | RK3288_GMAC_SPEED_100M,
+};
+
 static void rk3288_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con1;
 
-	if (speed == 10) {
-		con1 = RK3288_GMAC_CLK_2_5M;
-	} else if (speed == 100) {
-		con1 = RK3288_GMAC_CLK_25M;
-	} else if (speed == 1000) {
-		con1 = RK3288_GMAC_CLK_125M;
-	} else {
+	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3288_reg_speed_data,
+				   RK3288_GRF_SOC_CON1, speed))
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, con1);
 }
 
 static void rk3288_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con1;
 
-	if (speed == 10) {
-		con1 = RK3288_GMAC_RMII_CLK_2_5M | RK3288_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con1 = RK3288_GMAC_RMII_CLK_25M | RK3288_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rk3288_reg_speed_data,
+				  RK3288_GRF_SOC_CON1, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, con1);
 }
 
 static const struct rk_gmac_ops rk3288_ops = {
@@ -518,21 +558,18 @@ static void rk3308_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3308_GMAC_PHY_INTF_SEL_RMII);
 }
 
+static const struct rk_reg_speed_data rk3308_reg_speed_data = {
+	.rmii_10 = RK3308_GMAC_SPEED_10M,
+	.rmii_100 = RK3308_GMAC_SPEED_100M,
+};
+
 static void rk3308_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con0;
 
-	if (speed == 10) {
-		con0 = RK3308_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con0 = RK3308_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rk3308_reg_speed_data,
+				  RK3308_GRF_MAC_CON0, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3308_GRF_MAC_CON0, con0);
 }
 
 static const struct rk_gmac_ops rk3308_ops = {
@@ -599,43 +636,33 @@ static void rk3328_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3328_GMAC_RMII_MODE);
 }
 
+static const struct rk_reg_speed_data rk3328_reg_speed_data = {
+	.rgmii_10 = RK3328_GMAC_CLK_2_5M,
+	.rgmii_100 = RK3328_GMAC_CLK_25M,
+	.rgmii_1000 = RK3328_GMAC_CLK_125M,
+	.rmii_10 = RK3328_GMAC_RMII_CLK_2_5M | RK3328_GMAC_SPEED_10M,
+	.rmii_100 = RK3328_GMAC_RMII_CLK_25M | RK3328_GMAC_SPEED_100M,
+};
+
 static void rk3328_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con1;
 
-	if (speed == 10) {
-		con1 = RK3328_GMAC_CLK_2_5M;
-	} else if (speed == 100) {
-		con1 = RK3328_GMAC_CLK_25M;
-	} else if (speed == 1000) {
-		con1 = RK3328_GMAC_CLK_125M;
-	} else {
+	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3328_reg_speed_data,
+				   RK3328_GRF_MAC_CON1, speed))
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3328_GRF_MAC_CON1, con1);
 }
 
 static void rk3328_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int reg, con;
+	unsigned int reg;
 
 	reg = bsp_priv->integrated_phy ? RK3328_GRF_MAC_CON2 :
 		  RK3328_GRF_MAC_CON1;
 
-	if (speed == 10) {
-		con = RK3328_GMAC_RMII_CLK_2_5M | RK3328_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con = RK3328_GMAC_RMII_CLK_25M | RK3328_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rk3328_reg_speed_data, reg, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, reg, con);
 }
 
 static void rk3328_integrated_phy_powerup(struct rk_priv_data *priv)
@@ -701,40 +728,30 @@ static void rk3366_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3366_GMAC_PHY_INTF_SEL_RMII | RK3366_GMAC_RMII_MODE);
 }
 
+static const struct rk_reg_speed_data rk3366_reg_speed_data = {
+	.rgmii_10 = RK3366_GMAC_CLK_2_5M,
+	.rgmii_100 = RK3366_GMAC_CLK_25M,
+	.rgmii_1000 = RK3366_GMAC_CLK_125M,
+	.rmii_10 = RK3366_GMAC_RMII_CLK_2_5M | RK3366_GMAC_SPEED_10M,
+	.rmii_100 = RK3366_GMAC_RMII_CLK_25M | RK3366_GMAC_SPEED_100M,
+};
+
 static void rk3366_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con6;
 
-	if (speed == 10) {
-		con6 = RK3366_GMAC_CLK_2_5M;
-	} else if (speed == 100) {
-		con6 = RK3366_GMAC_CLK_25M;
-	} else if (speed == 1000) {
-		con6 = RK3366_GMAC_CLK_125M;
-	} else {
+	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3366_reg_speed_data,
+				   RK3366_GRF_SOC_CON6, speed))
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6, con6);
 }
 
 static void rk3366_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con6;
 
-	if (speed == 10) {
-		con6 = RK3366_GMAC_RMII_CLK_2_5M | RK3366_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con6 = RK3366_GMAC_RMII_CLK_25M | RK3366_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rk3366_reg_speed_data,
+				  RK3366_GRF_SOC_CON6, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3366_GRF_SOC_CON6, con6);
 }
 
 static const struct rk_gmac_ops rk3366_ops = {
@@ -790,40 +807,30 @@ static void rk3368_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3368_GMAC_PHY_INTF_SEL_RMII | RK3368_GMAC_RMII_MODE);
 }
 
+static const struct rk_reg_speed_data rk3368_reg_speed_data = {
+	.rgmii_10 = RK3368_GMAC_CLK_2_5M,
+	.rgmii_100 = RK3368_GMAC_CLK_25M,
+	.rgmii_1000 = RK3368_GMAC_CLK_125M,
+	.rmii_10 = RK3368_GMAC_RMII_CLK_2_5M | RK3368_GMAC_SPEED_10M,
+	.rmii_100 = RK3368_GMAC_RMII_CLK_25M | RK3368_GMAC_SPEED_100M,
+};
+
 static void rk3368_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con15;
 
-	if (speed == 10) {
-		con15 = RK3368_GMAC_CLK_2_5M;
-	} else if (speed == 100) {
-		con15 = RK3368_GMAC_CLK_25M;
-	} else if (speed == 1000) {
-		con15 = RK3368_GMAC_CLK_125M;
-	} else {
+	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3368_reg_speed_data,
+				   RK3368_GRF_SOC_CON15, speed))
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15, con15);
 }
 
 static void rk3368_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con15;
 
-	if (speed == 10) {
-		con15 = RK3368_GMAC_RMII_CLK_2_5M | RK3368_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con15 = RK3368_GMAC_RMII_CLK_25M | RK3368_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rk3368_reg_speed_data,
+				  RK3368_GRF_SOC_CON15, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3368_GRF_SOC_CON15, con15);
 }
 
 static const struct rk_gmac_ops rk3368_ops = {
@@ -879,40 +886,30 @@ static void rk3399_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3399_GMAC_PHY_INTF_SEL_RMII | RK3399_GMAC_RMII_MODE);
 }
 
+static const struct rk_reg_speed_data rk3399_reg_speed_data = {
+	.rgmii_10 = RK3399_GMAC_CLK_2_5M,
+	.rgmii_100 = RK3399_GMAC_CLK_25M,
+	.rgmii_1000 = RK3399_GMAC_CLK_125M,
+	.rmii_10 = RK3399_GMAC_RMII_CLK_2_5M | RK3399_GMAC_SPEED_10M,
+	.rmii_100 = RK3399_GMAC_RMII_CLK_25M | RK3399_GMAC_SPEED_100M,
+};
+
 static void rk3399_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con5;
 
-	if (speed == 10) {
-		con5 = RK3399_GMAC_CLK_2_5M;
-	} else if (speed == 100) {
-		con5 = RK3399_GMAC_CLK_25M;
-	} else if (speed == 1000) {
-		con5 = RK3399_GMAC_CLK_125M;
-	} else {
+	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3399_reg_speed_data,
+				   RK3399_GRF_SOC_CON5, speed))
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5, con5);
 }
 
 static void rk3399_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con5;
 
-	if (speed == 10) {
-		con5 = RK3399_GMAC_RMII_CLK_2_5M | RK3399_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con5 = RK3399_GMAC_RMII_CLK_25M | RK3399_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rk3399_reg_speed_data,
+				  RK3399_GRF_SOC_CON5, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3399_GRF_SOC_CON5, con5);
 }
 
 static const struct rk_gmac_ops rk3399_ops = {
@@ -982,45 +979,44 @@ static void rk3528_set_to_rmii(struct rk_priv_data *bsp_priv)
 			     RK3528_GMAC0_CLK_RMII_DIV2);
 }
 
+static const struct rk_reg_speed_data rk3528_gmac0_reg_speed_data = {
+	.rmii_10 = RK3528_GMAC0_CLK_RMII_DIV20,
+	.rmii_100 = RK3528_GMAC0_CLK_RMII_DIV2,
+};
+
+static const struct rk_reg_speed_data rk3528_gmac1_reg_speed_data = {
+	.rgmii_10 = RK3528_GMAC1_CLK_RGMII_DIV50,
+	.rgmii_100 = RK3528_GMAC1_CLK_RGMII_DIV5,
+	.rgmii_1000 = RK3528_GMAC1_CLK_RGMII_DIV1,
+	.rmii_10 = RK3528_GMAC1_CLK_RMII_DIV20,
+	.rmii_100 = RK3528_GMAC1_CLK_RMII_DIV2,
+};
+
 static void rk3528_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con5;
 
-	if (speed == 10) {
-		con5 = RK3528_GMAC1_CLK_RGMII_DIV50;
-	} else if (speed == 100) {
-		con5 = RK3528_GMAC1_CLK_RGMII_DIV5;
-	} else if (speed == 1000) {
-		con5 = RK3528_GMAC1_CLK_RGMII_DIV1;
-	} else {
+	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3528_gmac1_reg_speed_data,
+				   RK3528_VPU_GRF_GMAC_CON5, speed))
 		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5, con5);
 }
 
 static void rk3528_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int reg, val;
+	const struct rk_reg_speed_data *rsd;
+	unsigned int reg;
 
-	if (speed == 10) {
-		val = bsp_priv->id == 1 ? RK3528_GMAC1_CLK_RMII_DIV20 :
-					  RK3528_GMAC0_CLK_RMII_DIV20;
-	} else if (speed == 100) {
-		val = bsp_priv->id == 1 ? RK3528_GMAC1_CLK_RMII_DIV2 :
-					  RK3528_GMAC0_CLK_RMII_DIV2;
+	if (bsp_priv->id == 1) {
+		rsd = &rk3528_gmac1_reg_speed_data;
+		reg = RK3528_VPU_GRF_GMAC_CON5;
 	} else {
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
+		rsd = &rk3528_gmac0_reg_speed_data;
+		reg = RK3528_VO_GRF_GMAC_CON;
 	}
 
-	reg = bsp_priv->id == 1 ? RK3528_VPU_GRF_GMAC_CON5 :
-				  RK3528_VO_GRF_GMAC_CON;
-
-	regmap_write(bsp_priv->grf, reg, val);
+	if (rk_set_reg_speed_rmii(bsp_priv, rsd, reg, speed))
+		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
 }
 
 static void rk3528_set_clock_selection(struct rk_priv_data *bsp_priv,
@@ -1224,42 +1220,25 @@ static void rk3576_set_to_rmii(struct rk_priv_data *bsp_priv)
 	regmap_write(bsp_priv->grf, offset_con, RK3576_GMAC_RMII_MODE);
 }
 
+static const struct rk_reg_speed_data rk3578_reg_speed_data = {
+	.rgmii_10 = RK3576_GMAC_CLK_RGMII_DIV50,
+	.rgmii_100 = RK3576_GMAC_CLK_RGMII_DIV5,
+	.rgmii_1000 = RK3576_GMAC_CLK_RGMII_DIV1,
+	.rmii_10 = RK3576_GMAC_CLK_RMII_DIV20,
+	.rmii_100 = RK3576_GMAC_CLK_RMII_DIV2,
+};
+
 static void rk3576_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int val = 0, offset_con;
-
-	switch (speed) {
-	case 10:
-		if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
-			val = RK3576_GMAC_CLK_RMII_DIV20;
-		else
-			val = RK3576_GMAC_CLK_RGMII_DIV50;
-		break;
-	case 100:
-		if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
-			val = RK3576_GMAC_CLK_RMII_DIV2;
-		else
-			val = RK3576_GMAC_CLK_RGMII_DIV5;
-		break;
-	case 1000:
-		if (bsp_priv->phy_iface != PHY_INTERFACE_MODE_RMII)
-			val = RK3576_GMAC_CLK_RGMII_DIV1;
-		else
-			goto err;
-		break;
-	default:
-		goto err;
-	}
+	unsigned int offset_con;
 
 	offset_con = bsp_priv->id == 1 ? RK3576_GRF_GMAC_CON1 :
 					 RK3576_GRF_GMAC_CON0;
 
-	regmap_write(bsp_priv->grf, offset_con, val);
-
-	return;
-err:
-	dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
+	if (rk_set_reg_speed(bsp_priv, &rk3578_reg_speed_data, offset_con,
+			     bsp_priv->phy_iface, speed))
+		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
 }
 
 static void rk3576_set_clock_selection(struct rk_priv_data *bsp_priv, bool input,
@@ -1446,21 +1425,18 @@ static void rv1108_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RV1108_GMAC_PHY_INTF_SEL_RMII);
 }
 
+static const struct rk_reg_speed_data rv1108_reg_speed_data = {
+	.rmii_10 = RV1108_GMAC_RMII_CLK_2_5M | RV1108_GMAC_SPEED_10M,
+	.rmii_100 = RV1108_GMAC_RMII_CLK_25M | RV1108_GMAC_SPEED_100M,
+};
+
 static void rv1108_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned int con0;
 
-	if (speed == 10) {
-		con0 = RV1108_GMAC_RMII_CLK_2_5M | RV1108_GMAC_SPEED_10M;
-	} else if (speed == 100) {
-		con0 = RV1108_GMAC_RMII_CLK_25M | RV1108_GMAC_SPEED_100M;
-	} else {
+	if (rk_set_reg_speed_rmii(bsp_priv, &rv1108_reg_speed_data,
+				  RV1108_GRF_GMAC_CON0, speed))
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
-	}
-
-	regmap_write(bsp_priv->grf, RV1108_GRF_GMAC_CON0, con0);
 }
 
 static const struct rk_gmac_ops rv1108_ops = {
-- 
2.30.2

Re: [PATCH net-next 3/9] net: stmmac: rk: add struct for programming register based speeds

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:40:51PM +0100, Russell King (Oracle) wrote:
> There is a common pattern in the driver where many SoCs need to write a
> single register with a value dependent on the interface mode and speed.
> Rather than having a lot of repeated code, add some common functions
> and a struct to contain the values to be written to a register to
> select the RGMII and RMII speeds.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 4/9] net: stmmac: rk: combine rv1126 set_*_speed() methods

[Russell King (Oracle)]

Just like rk3568, there is no need to have separate RGMII and RMII
methods to set clk_mac_speed() as rgmii_clock() can be used to return
the clock rate for both RGMII and RMII interface modes. Combine these
two methods.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 33 +++----------------
 1 file changed, 4 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 7b5e989bb77f..c7b64f0a2931 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1496,7 +1496,7 @@ static void rv1126_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RV1126_GMAC_PHY_INTF_SEL_RMII);
 }
 
-static void rv1126_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static void rv1126_set_clk_mac_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct clk *clk_mac_speed = bsp_priv->clks[RK_CLK_MAC_SPEED].clk;
 	struct device *dev = &bsp_priv->pdev->dev;
@@ -1505,32 +1505,7 @@ static void rv1126_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
 
 	rate = rgmii_clock(speed);
 	if (rate < 0) {
-		dev_err(dev, "unknown speed value for RGMII speed=%d", speed);
-		return;
-	}
-
-	ret = clk_set_rate(clk_mac_speed, rate);
-	if (ret)
-		dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
-			__func__, rate, ret);
-}
-
-static void rv1126_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct clk *clk_mac_speed = bsp_priv->clks[RK_CLK_MAC_SPEED].clk;
-	struct device *dev = &bsp_priv->pdev->dev;
-	unsigned long rate;
-	int ret;
-
-	switch (speed) {
-	case 10:
-		rate = 2500000;
-		break;
-	case 100:
-		rate = 25000000;
-		break;
-	default:
-		dev_err(dev, "unknown speed value for RGMII speed=%d", speed);
+		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
 		return;
 	}
 
@@ -1543,8 +1518,8 @@ static void rv1126_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 static const struct rk_gmac_ops rv1126_ops = {
 	.set_to_rgmii = rv1126_set_to_rgmii,
 	.set_to_rmii = rv1126_set_to_rmii,
-	.set_rgmii_speed = rv1126_set_rgmii_speed,
-	.set_rmii_speed = rv1126_set_rmii_speed,
+	.set_rgmii_speed = rv1126_set_clk_mac_speed,
+	.set_rmii_speed = rv1126_set_clk_mac_speed,
 };
 
 static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
-- 
2.30.2

Re: [PATCH net-next 4/9] net: stmmac: rk: combine rv1126 set_*_speed() methods

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:40:57PM +0100, Russell King (Oracle) wrote:
> Just like rk3568, there is no need to have separate RGMII and RMII
> methods to set clk_mac_speed() as rgmii_clock() can be used to return
> the clock rate for both RGMII and RMII interface modes. Combine these
> two methods.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 5/9] net: stmmac: rk: combine clk_mac_speed rate setting functions

[Russell King (Oracle)]

rk3568_set_gmac_speed() and rv1126_set_clk_mac_speed() are now
identical. Combine these so we have a single copy of this code.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 65 +++++++------------
 1 file changed, 23 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index c7b64f0a2931..eeef11b60566 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -153,6 +153,25 @@ static int rk_set_reg_speed_rmii(struct rk_priv_data *bsp_priv,
 				speed);
 }
 
+static void rk_set_clk_mac_speed(struct rk_priv_data *bsp_priv, int speed)
+{
+	struct clk *clk_mac_speed = bsp_priv->clks[RK_CLK_MAC_SPEED].clk;
+	struct device *dev = &bsp_priv->pdev->dev;
+	long rate;
+	int ret;
+
+	rate = rgmii_clock(speed);
+	if (rate < 0) {
+		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
+		return;
+	}
+
+	ret = clk_set_rate(clk_mac_speed, rate);
+	if (ret)
+		dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
+			__func__, rate, ret);
+}
+
 #define HIWORD_UPDATE(val, mask, shift) \
 		((val) << (shift) | (mask) << ((shift) + 16))
 
@@ -1113,30 +1132,11 @@ static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
 	regmap_write(bsp_priv->grf, con1, RK3568_GMAC_PHY_INTF_SEL_RMII);
 }
 
-static void rk3568_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct clk *clk_mac_speed = bsp_priv->clks[RK_CLK_MAC_SPEED].clk;
-	struct device *dev = &bsp_priv->pdev->dev;
-	long rate;
-	int ret;
-
-	rate = rgmii_clock(speed);
-	if (rate < 0) {
-		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
-		return;
-	}
-
-	ret = clk_set_rate(clk_mac_speed, rate);
-	if (ret)
-		dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
-			__func__, rate, ret);
-}
-
 static const struct rk_gmac_ops rk3568_ops = {
 	.set_to_rgmii = rk3568_set_to_rgmii,
 	.set_to_rmii = rk3568_set_to_rmii,
-	.set_rgmii_speed = rk3568_set_gmac_speed,
-	.set_rmii_speed = rk3568_set_gmac_speed,
+	.set_rgmii_speed = rk_set_clk_mac_speed,
+	.set_rmii_speed = rk_set_clk_mac_speed,
 	.regs_valid = true,
 	.regs = {
 		0xfe2a0000, /* gmac0 */
@@ -1496,30 +1496,11 @@ static void rv1126_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RV1126_GMAC_PHY_INTF_SEL_RMII);
 }
 
-static void rv1126_set_clk_mac_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct clk *clk_mac_speed = bsp_priv->clks[RK_CLK_MAC_SPEED].clk;
-	struct device *dev = &bsp_priv->pdev->dev;
-	long rate;
-	int ret;
-
-	rate = rgmii_clock(speed);
-	if (rate < 0) {
-		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
-		return;
-	}
-
-	ret = clk_set_rate(clk_mac_speed, rate);
-	if (ret)
-		dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
-			__func__, rate, ret);
-}
-
 static const struct rk_gmac_ops rv1126_ops = {
 	.set_to_rgmii = rv1126_set_to_rgmii,
 	.set_to_rmii = rv1126_set_to_rmii,
-	.set_rgmii_speed = rv1126_set_clk_mac_speed,
-	.set_rmii_speed = rv1126_set_clk_mac_speed,
+	.set_rgmii_speed = rk_set_clk_mac_speed,
+	.set_rmii_speed = rk_set_clk_mac_speed,
 };
 
 static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
-- 
2.30.2

Re: [PATCH net-next 5/9] net: stmmac: rk: combine clk_mac_speed rate setting functions

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:41:02PM +0100, Russell King (Oracle) wrote:
> rk3568_set_gmac_speed() and rv1126_set_clk_mac_speed() are now
> identical. Combine these so we have a single copy of this code.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 6/9] net: stmmac: rk: combine .set_*_speed() methods

[Russell King (Oracle)]

As a result of the previous patches, many of the .set_rgmii_speed()
and .set_rmii_speed() implementations are identical apart from the
interface mode. Add a new .set_speed() function which takes the
interface mode in addition to the speed, and use it to combine the
separate implementations, calling the common rk_set_reg_speed()
function.

Also convert rk_set_clk_mac_speed() to be called by this new method
pointer, rather than having these implementations called from both
.set_*_speed() methods.

Remove all the error messages from the .set_speed() methods, as these
return an error code which is propagated up to stmmac_mac_link_up()
which will print the error.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 274 +++++-------------
 1 file changed, 79 insertions(+), 195 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index eeef11b60566..8ad6b3b0e282 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -39,6 +39,8 @@ struct rk_gmac_ops {
 	void (*set_to_rmii)(struct rk_priv_data *bsp_priv);
 	void (*set_rgmii_speed)(struct rk_priv_data *bsp_priv, int speed);
 	void (*set_rmii_speed)(struct rk_priv_data *bsp_priv, int speed);
+	int (*set_speed)(struct rk_priv_data *bsp_priv,
+			 phy_interface_t interface, int speed);
 	void (*set_clock_selection)(struct rk_priv_data *bsp_priv, bool input,
 				    bool enable);
 	void (*integrated_phy_powerup)(struct rk_priv_data *bsp_priv);
@@ -137,39 +139,17 @@ static int rk_set_reg_speed(struct rk_priv_data *bsp_priv,
 
 }
 
-static int rk_set_reg_speed_rgmii(struct rk_priv_data *bsp_priv,
-				  const struct rk_reg_speed_data *rsd,
-				  unsigned int reg, int speed)
-{
-	return rk_set_reg_speed(bsp_priv, rsd, reg, PHY_INTERFACE_MODE_RGMII,
-				speed);
-}
-
-static int rk_set_reg_speed_rmii(struct rk_priv_data *bsp_priv,
-				 const struct rk_reg_speed_data *rsd,
-				 unsigned int reg, int speed)
-{
-	return rk_set_reg_speed(bsp_priv, rsd, reg, PHY_INTERFACE_MODE_RMII,
-				speed);
-}
-
-static void rk_set_clk_mac_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk_set_clk_mac_speed(struct rk_priv_data *bsp_priv,
+				phy_interface_t interface, int speed)
 {
 	struct clk *clk_mac_speed = bsp_priv->clks[RK_CLK_MAC_SPEED].clk;
-	struct device *dev = &bsp_priv->pdev->dev;
 	long rate;
-	int ret;
 
 	rate = rgmii_clock(speed);
-	if (rate < 0) {
-		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
-		return;
-	}
+	if (rate < 0)
+		return rate;
 
-	ret = clk_set_rate(clk_mac_speed, rate);
-	if (ret)
-		dev_err(dev, "%s: set clk_mac_speed rate %ld failed %d\n",
-			__func__, rate, ret);
+	return clk_set_rate(clk_mac_speed, rate);
 }
 
 #define HIWORD_UPDATE(val, mask, shift) \
@@ -358,29 +338,17 @@ static const struct rk_reg_speed_data rk3128_reg_speed_data = {
 	.rmii_100 = RK3128_GMAC_RMII_CLK_25M | RK3128_GMAC_SPEED_100M,
 };
 
-static void rk3128_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3128_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3128_reg_speed_data,
-				   RK3128_GRF_MAC_CON1, speed))
-		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-}
-
-static void rk3128_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rmii(bsp_priv, &rk3128_reg_speed_data,
-				  RK3128_GRF_MAC_CON1, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3128_reg_speed_data,
+				RK3128_GRF_MAC_CON1, interface, speed);
 }
 
 static const struct rk_gmac_ops rk3128_ops = {
 	.set_to_rgmii = rk3128_set_to_rgmii,
 	.set_to_rmii = rk3128_set_to_rmii,
-	.set_rgmii_speed = rk3128_set_rgmii_speed,
-	.set_rmii_speed = rk3128_set_rmii_speed,
+	.set_speed = rk3128_set_speed,
 };
 
 #define RK3228_GRF_MAC_CON0	0x0900
@@ -447,22 +415,11 @@ static const struct rk_reg_speed_data rk3228_reg_speed_data = {
 	.rmii_100 = RK3228_GMAC_RMII_CLK_25M | RK3228_GMAC_SPEED_100M,
 };
 
-static void rk3228_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3228_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3228_reg_speed_data,
-				   RK3228_GRF_MAC_CON1, speed))
-		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-}
-
-static void rk3228_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rmii(bsp_priv, &rk3228_reg_speed_data,
-				  RK3228_GRF_MAC_CON1, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3228_reg_speed_data,
+				RK3228_GRF_MAC_CON1, interface, speed);
 }
 
 static void rk3228_integrated_phy_powerup(struct rk_priv_data *priv)
@@ -476,8 +433,7 @@ static void rk3228_integrated_phy_powerup(struct rk_priv_data *priv)
 static const struct rk_gmac_ops rk3228_ops = {
 	.set_to_rgmii = rk3228_set_to_rgmii,
 	.set_to_rmii = rk3228_set_to_rmii,
-	.set_rgmii_speed = rk3228_set_rgmii_speed,
-	.set_rmii_speed = rk3228_set_rmii_speed,
+	.set_speed = rk3228_set_speed,
 	.integrated_phy_powerup = rk3228_integrated_phy_powerup,
 	.integrated_phy_powerdown = rk_gmac_integrated_ephy_powerdown,
 };
@@ -536,29 +492,17 @@ static const struct rk_reg_speed_data rk3288_reg_speed_data = {
 	.rmii_100 = RK3288_GMAC_RMII_CLK_25M | RK3288_GMAC_SPEED_100M,
 };
 
-static void rk3288_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3288_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3288_reg_speed_data,
-				   RK3288_GRF_SOC_CON1, speed))
-		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-}
-
-static void rk3288_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rmii(bsp_priv, &rk3288_reg_speed_data,
-				  RK3288_GRF_SOC_CON1, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3288_reg_speed_data,
+				RK3288_GRF_SOC_CON1, interface, speed);
 }
 
 static const struct rk_gmac_ops rk3288_ops = {
 	.set_to_rgmii = rk3288_set_to_rgmii,
 	.set_to_rmii = rk3288_set_to_rmii,
-	.set_rgmii_speed = rk3288_set_rgmii_speed,
-	.set_rmii_speed = rk3288_set_rmii_speed,
+	.set_speed = rk3288_set_speed,
 };
 
 #define RK3308_GRF_MAC_CON0		0x04a0
@@ -582,18 +526,16 @@ static const struct rk_reg_speed_data rk3308_reg_speed_data = {
 	.rmii_100 = RK3308_GMAC_SPEED_100M,
 };
 
-static void rk3308_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3308_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rmii(bsp_priv, &rk3308_reg_speed_data,
-				  RK3308_GRF_MAC_CON0, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3308_reg_speed_data,
+				RK3308_GRF_MAC_CON0, interface, speed);
 }
 
 static const struct rk_gmac_ops rk3308_ops = {
 	.set_to_rmii = rk3308_set_to_rmii,
-	.set_rmii_speed = rk3308_set_rmii_speed,
+	.set_speed = rk3308_set_speed,
 };
 
 #define RK3328_GRF_MAC_CON0	0x0900
@@ -663,25 +605,18 @@ static const struct rk_reg_speed_data rk3328_reg_speed_data = {
 	.rmii_100 = RK3328_GMAC_RMII_CLK_25M | RK3328_GMAC_SPEED_100M,
 };
 
-static void rk3328_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3328_reg_speed_data,
-				   RK3328_GRF_MAC_CON1, speed))
-		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-}
-
-static void rk3328_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3328_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
 	unsigned int reg;
 
-	reg = bsp_priv->integrated_phy ? RK3328_GRF_MAC_CON2 :
-		  RK3328_GRF_MAC_CON1;
+	if (interface == PHY_INTERFACE_MODE_RMII && bsp_priv->integrated_phy)
+		reg = RK3328_GRF_MAC_CON2;
+	else
+		reg = RK3328_GRF_MAC_CON1;
 
-	if (rk_set_reg_speed_rmii(bsp_priv, &rk3328_reg_speed_data, reg, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3328_reg_speed_data, reg,
+				interface, speed);
 }
 
 static void rk3328_integrated_phy_powerup(struct rk_priv_data *priv)
@@ -695,8 +630,7 @@ static void rk3328_integrated_phy_powerup(struct rk_priv_data *priv)
 static const struct rk_gmac_ops rk3328_ops = {
 	.set_to_rgmii = rk3328_set_to_rgmii,
 	.set_to_rmii = rk3328_set_to_rmii,
-	.set_rgmii_speed = rk3328_set_rgmii_speed,
-	.set_rmii_speed = rk3328_set_rmii_speed,
+	.set_speed = rk3328_set_speed,
 	.integrated_phy_powerup = rk3328_integrated_phy_powerup,
 	.integrated_phy_powerdown = rk_gmac_integrated_ephy_powerdown,
 };
@@ -755,29 +689,17 @@ static const struct rk_reg_speed_data rk3366_reg_speed_data = {
 	.rmii_100 = RK3366_GMAC_RMII_CLK_25M | RK3366_GMAC_SPEED_100M,
 };
 
-static void rk3366_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3366_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3366_reg_speed_data,
-				   RK3366_GRF_SOC_CON6, speed))
-		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-}
-
-static void rk3366_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rmii(bsp_priv, &rk3366_reg_speed_data,
-				  RK3366_GRF_SOC_CON6, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3366_reg_speed_data,
+				RK3366_GRF_SOC_CON6, interface, speed);
 }
 
 static const struct rk_gmac_ops rk3366_ops = {
 	.set_to_rgmii = rk3366_set_to_rgmii,
 	.set_to_rmii = rk3366_set_to_rmii,
-	.set_rgmii_speed = rk3366_set_rgmii_speed,
-	.set_rmii_speed = rk3366_set_rmii_speed,
+	.set_speed = rk3366_set_speed,
 };
 
 #define RK3368_GRF_SOC_CON15	0x043c
@@ -834,29 +756,17 @@ static const struct rk_reg_speed_data rk3368_reg_speed_data = {
 	.rmii_100 = RK3368_GMAC_RMII_CLK_25M | RK3368_GMAC_SPEED_100M,
 };
 
-static void rk3368_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3368_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3368_reg_speed_data,
-				   RK3368_GRF_SOC_CON15, speed))
-		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-}
-
-static void rk3368_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rmii(bsp_priv, &rk3368_reg_speed_data,
-				  RK3368_GRF_SOC_CON15, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3368_reg_speed_data,
+				RK3368_GRF_SOC_CON15, interface, speed);
 }
 
 static const struct rk_gmac_ops rk3368_ops = {
 	.set_to_rgmii = rk3368_set_to_rgmii,
 	.set_to_rmii = rk3368_set_to_rmii,
-	.set_rgmii_speed = rk3368_set_rgmii_speed,
-	.set_rmii_speed = rk3368_set_rmii_speed,
+	.set_speed = rk3368_set_speed,
 };
 
 #define RK3399_GRF_SOC_CON5	0xc214
@@ -913,29 +823,17 @@ static const struct rk_reg_speed_data rk3399_reg_speed_data = {
 	.rmii_100 = RK3399_GMAC_RMII_CLK_25M | RK3399_GMAC_SPEED_100M,
 };
 
-static void rk3399_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3399_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3399_reg_speed_data,
-				   RK3399_GRF_SOC_CON5, speed))
-		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-}
-
-static void rk3399_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rmii(bsp_priv, &rk3399_reg_speed_data,
-				  RK3399_GRF_SOC_CON5, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3399_reg_speed_data,
+				RK3399_GRF_SOC_CON5, interface, speed);
 }
 
 static const struct rk_gmac_ops rk3399_ops = {
 	.set_to_rgmii = rk3399_set_to_rgmii,
 	.set_to_rmii = rk3399_set_to_rmii,
-	.set_rgmii_speed = rk3399_set_rgmii_speed,
-	.set_rmii_speed = rk3399_set_rmii_speed,
+	.set_speed = rk3399_set_speed,
 };
 
 #define RK3528_VO_GRF_GMAC_CON		0x0018
@@ -1011,18 +909,9 @@ static const struct rk_reg_speed_data rk3528_gmac1_reg_speed_data = {
 	.rmii_100 = RK3528_GMAC1_CLK_RMII_DIV2,
 };
 
-static void rk3528_set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3528_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface,int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rgmii(bsp_priv, &rk3528_gmac1_reg_speed_data,
-				   RK3528_VPU_GRF_GMAC_CON5, speed))
-		dev_err(dev, "unknown speed value for RGMII! speed=%d", speed);
-}
-
-static void rk3528_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
-{
-	struct device *dev = &bsp_priv->pdev->dev;
 	const struct rk_reg_speed_data *rsd;
 	unsigned int reg;
 
@@ -1034,8 +923,7 @@ static void rk3528_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 		reg = RK3528_VO_GRF_GMAC_CON;
 	}
 
-	if (rk_set_reg_speed_rmii(bsp_priv, rsd, reg, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, rsd, reg, interface, speed);
 }
 
 static void rk3528_set_clock_selection(struct rk_priv_data *bsp_priv,
@@ -1069,8 +957,7 @@ static void rk3528_integrated_phy_powerdown(struct rk_priv_data *bsp_priv)
 static const struct rk_gmac_ops rk3528_ops = {
 	.set_to_rgmii = rk3528_set_to_rgmii,
 	.set_to_rmii = rk3528_set_to_rmii,
-	.set_rgmii_speed = rk3528_set_rgmii_speed,
-	.set_rmii_speed = rk3528_set_rmii_speed,
+	.set_speed = rk3528_set_speed,
 	.set_clock_selection = rk3528_set_clock_selection,
 	.integrated_phy_powerup = rk3528_integrated_phy_powerup,
 	.integrated_phy_powerdown = rk3528_integrated_phy_powerdown,
@@ -1135,8 +1022,7 @@ static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
 static const struct rk_gmac_ops rk3568_ops = {
 	.set_to_rgmii = rk3568_set_to_rgmii,
 	.set_to_rmii = rk3568_set_to_rmii,
-	.set_rgmii_speed = rk_set_clk_mac_speed,
-	.set_rmii_speed = rk_set_clk_mac_speed,
+	.set_speed = rk_set_clk_mac_speed,
 	.regs_valid = true,
 	.regs = {
 		0xfe2a0000, /* gmac0 */
@@ -1228,17 +1114,16 @@ static const struct rk_reg_speed_data rk3578_reg_speed_data = {
 	.rmii_100 = RK3576_GMAC_CLK_RMII_DIV2,
 };
 
-static void rk3576_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3576_set_gmac_speed(struct rk_priv_data *bsp_priv,
+				 phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
 	unsigned int offset_con;
 
 	offset_con = bsp_priv->id == 1 ? RK3576_GRF_GMAC_CON1 :
 					 RK3576_GRF_GMAC_CON0;
 
-	if (rk_set_reg_speed(bsp_priv, &rk3578_reg_speed_data, offset_con,
-			     bsp_priv->phy_iface, speed))
-		dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rk3578_reg_speed_data, offset_con,
+				interface, speed);
 }
 
 static void rk3576_set_clock_selection(struct rk_priv_data *bsp_priv, bool input,
@@ -1260,8 +1145,7 @@ static void rk3576_set_clock_selection(struct rk_priv_data *bsp_priv, bool input
 static const struct rk_gmac_ops rk3576_ops = {
 	.set_to_rgmii = rk3576_set_to_rgmii,
 	.set_to_rmii = rk3576_set_to_rmii,
-	.set_rgmii_speed = rk3576_set_gmac_speed,
-	.set_rmii_speed = rk3576_set_gmac_speed,
+	.set_speed = rk3576_set_gmac_speed,
 	.set_clock_selection = rk3576_set_clock_selection,
 	.php_grf_required = true,
 	.regs_valid = true,
@@ -1345,26 +1229,26 @@ static void rk3588_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3588_GMAC_CLK_RMII_MODE(bsp_priv->id));
 }
 
-static void rk3588_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rk3588_set_gmac_speed(struct rk_priv_data *bsp_priv,
+				 phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
 	unsigned int val = 0, id = bsp_priv->id;
 
 	switch (speed) {
 	case 10:
-		if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
+		if (interface == PHY_INTERFACE_MODE_RMII)
 			val = RK3588_GMA_CLK_RMII_DIV20(id);
 		else
 			val = RK3588_GMAC_CLK_RGMII_DIV50(id);
 		break;
 	case 100:
-		if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
+		if (interface == PHY_INTERFACE_MODE_RMII)
 			val = RK3588_GMA_CLK_RMII_DIV2(id);
 		else
 			val = RK3588_GMAC_CLK_RGMII_DIV5(id);
 		break;
 	case 1000:
-		if (bsp_priv->phy_iface != PHY_INTERFACE_MODE_RMII)
+		if (interface != PHY_INTERFACE_MODE_RMII)
 			val = RK3588_GMAC_CLK_RGMII_DIV1(id);
 		else
 			goto err;
@@ -1375,9 +1259,9 @@ static void rk3588_set_gmac_speed(struct rk_priv_data *bsp_priv, int speed)
 
 	regmap_write(bsp_priv->php_grf, RK3588_GRF_CLK_CON1, val);
 
-	return;
+	return 0;
 err:
-	dev_err(dev, "unknown speed value for GMAC speed=%d", speed);
+	return -EINVAL;
 }
 
 static void rk3588_set_clock_selection(struct rk_priv_data *bsp_priv, bool input,
@@ -1395,8 +1279,7 @@ static void rk3588_set_clock_selection(struct rk_priv_data *bsp_priv, bool input
 static const struct rk_gmac_ops rk3588_ops = {
 	.set_to_rgmii = rk3588_set_to_rgmii,
 	.set_to_rmii = rk3588_set_to_rmii,
-	.set_rgmii_speed = rk3588_set_gmac_speed,
-	.set_rmii_speed = rk3588_set_gmac_speed,
+	.set_speed = rk3588_set_gmac_speed,
 	.set_clock_selection = rk3588_set_clock_selection,
 	.php_grf_required = true,
 	.regs_valid = true,
@@ -1430,18 +1313,16 @@ static const struct rk_reg_speed_data rv1108_reg_speed_data = {
 	.rmii_100 = RV1108_GMAC_RMII_CLK_25M | RV1108_GMAC_SPEED_100M,
 };
 
-static void rv1108_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int rv1108_set_speed(struct rk_priv_data *bsp_priv,
+			    phy_interface_t interface, int speed)
 {
-	struct device *dev = &bsp_priv->pdev->dev;
-
-	if (rk_set_reg_speed_rmii(bsp_priv, &rv1108_reg_speed_data,
-				  RV1108_GRF_GMAC_CON0, speed))
-		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+	return rk_set_reg_speed(bsp_priv, &rv1108_reg_speed_data,
+				RV1108_GRF_GMAC_CON0, interface, speed);
 }
 
 static const struct rk_gmac_ops rv1108_ops = {
 	.set_to_rmii = rv1108_set_to_rmii,
-	.set_rmii_speed = rv1108_set_rmii_speed,
+	.set_speed = rv1108_set_speed,
 };
 
 #define RV1126_GRF_GMAC_CON0		0X0070
@@ -1499,8 +1380,7 @@ static void rv1126_set_to_rmii(struct rk_priv_data *bsp_priv)
 static const struct rk_gmac_ops rv1126_ops = {
 	.set_to_rgmii = rv1126_set_to_rgmii,
 	.set_to_rmii = rv1126_set_to_rmii,
-	.set_rgmii_speed = rk_set_clk_mac_speed,
-	.set_rmii_speed = rk_set_clk_mac_speed,
+	.set_speed = rk_set_clk_mac_speed,
 };
 
 static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
@@ -1833,6 +1713,10 @@ static int rk_set_clk_tx_rate(void *bsp_priv_, struct clk *clk_tx_i,
 	struct rk_priv_data *bsp_priv = bsp_priv_;
 	struct device *dev = &bsp_priv->pdev->dev;
 
+	if (bsp_priv->ops->set_speed)
+		return bsp_priv->ops->set_speed(bsp_priv, bsp_priv->phy_iface,
+						speed);
+
 	switch (bsp_priv->phy_iface) {
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
-- 
2.30.2

Re: [PATCH net-next 6/9] net: stmmac: rk: combine .set_*_speed() methods

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:41:07PM +0100, Russell King (Oracle) wrote:
> As a result of the previous patches, many of the .set_rgmii_speed()
> and .set_rmii_speed() implementations are identical apart from the
> interface mode. Add a new .set_speed() function which takes the
> interface mode in addition to the speed, and use it to combine the
> separate implementations, calling the common rk_set_reg_speed()
> function.
> 
> Also convert rk_set_clk_mac_speed() to be called by this new method
> pointer, rather than having these implementations called from both
> .set_*_speed() methods.
> 
> Remove all the error messages from the .set_speed() methods, as these
> return an error code which is propagated up to stmmac_mac_link_up()
> which will print the error.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 7/9] net: stmmac: rk: simplify px30_set_rmii_speed()

[Russell King (Oracle)]

px30_set_rmii_speed() doesn't need to be as verbose as it is - it
merely needs the values for the register and clock rate which depend
on the speed, and then call the appropriate functions. Rewrite the
function to make it so.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 29 +++++++++----------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 8ad6b3b0e282..72f2b80bf3bb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -250,6 +250,8 @@ static void px30_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 {
 	struct clk *clk_mac_speed = bsp_priv->clks[RK_CLK_MAC_SPEED].clk;
 	struct device *dev = &bsp_priv->pdev->dev;
+	unsigned int con1;
+	long rate;
 	int ret;
 
 	if (!clk_mac_speed) {
@@ -258,25 +260,22 @@ static void px30_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 	}
 
 	if (speed == 10) {
-		regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
-			     PX30_GMAC_SPEED_10M);
-
-		ret = clk_set_rate(clk_mac_speed, 2500000);
-		if (ret)
-			dev_err(dev, "%s: set clk_mac_speed rate 2500000 failed: %d\n",
-				__func__, ret);
+		con1 = PX30_GMAC_SPEED_10M;
+		rate = 2500000;
 	} else if (speed == 100) {
-		regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1,
-			     PX30_GMAC_SPEED_100M);
-
-		ret = clk_set_rate(clk_mac_speed, 25000000);
-		if (ret)
-			dev_err(dev, "%s: set clk_mac_speed rate 25000000 failed: %d\n",
-				__func__, ret);
-
+		con1 = PX30_GMAC_SPEED_100M;
+		rate = 25000000;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
+		return;
 	}
+
+	regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1, con1);
+
+	ret = clk_set_rate(clk_mac_speed, rate);
+	if (ret)
+		dev_err(dev, "%s: set clk_mac_speed rate %ld failed: %d\n",
+			__func__, rate, ret);
 }
 
 static const struct rk_gmac_ops px30_ops = {
-- 
2.30.2

Re: [PATCH net-next 7/9] net: stmmac: rk: simplify px30_set_rmii_speed()

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:41:12PM +0100, Russell King (Oracle) wrote:
> px30_set_rmii_speed() doesn't need to be as verbose as it is - it
> merely needs the values for the register and clock rate which depend
> on the speed, and then call the appropriate functions. Rewrite the
> function to make it so.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 8/9] net: stmmac: rk: convert px30_set_rmii_speed() to .set_speed()

[Russell King (Oracle)]

Convert px30_set_rmii_speed() to use the common .set_speed() method,
which eliminates another user of the older .set_*_speed() methods.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 72f2b80bf3bb..7cdb09037da0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -246,17 +246,17 @@ static void px30_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     PX30_GMAC_PHY_INTF_SEL_RMII);
 }
 
-static void px30_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
+static int px30_set_speed(struct rk_priv_data *bsp_priv,
+			  phy_interface_t interface, int speed)
 {
 	struct clk *clk_mac_speed = bsp_priv->clks[RK_CLK_MAC_SPEED].clk;
 	struct device *dev = &bsp_priv->pdev->dev;
 	unsigned int con1;
 	long rate;
-	int ret;
 
 	if (!clk_mac_speed) {
 		dev_err(dev, "%s: Missing clk_mac_speed clock\n", __func__);
-		return;
+		return -EINVAL;
 	}
 
 	if (speed == 10) {
@@ -267,20 +267,17 @@ static void px30_set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
 		rate = 25000000;
 	} else {
 		dev_err(dev, "unknown speed value for RMII! speed=%d", speed);
-		return;
+		return -EINVAL;
 	}
 
 	regmap_write(bsp_priv->grf, PX30_GRF_GMAC_CON1, con1);
 
-	ret = clk_set_rate(clk_mac_speed, rate);
-	if (ret)
-		dev_err(dev, "%s: set clk_mac_speed rate %ld failed: %d\n",
-			__func__, rate, ret);
+	return clk_set_rate(clk_mac_speed, rate);
 }
 
 static const struct rk_gmac_ops px30_ops = {
 	.set_to_rmii = px30_set_to_rmii,
-	.set_rmii_speed = px30_set_rmii_speed,
+	.set_speed = px30_set_speed,
 };
 
 #define RK3128_GRF_MAC_CON0	0x0168
-- 
2.30.2

Re: [PATCH net-next 8/9] net: stmmac: rk: convert px30_set_rmii_speed() to .set_speed()

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:41:17PM +0100, Russell King (Oracle) wrote:
> Convert px30_set_rmii_speed() to use the common .set_speed() method,
> which eliminates another user of the older .set_*_speed() methods.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next 9/9] net: stmmac: rk: remove obsolete .set_*_speed() methods

[Russell King (Oracle)]

Now that no SoC implements the .set_*_speed() methods, we can get rid
of these methods and the now unused code in rk_set_clk_tx_rate().
Arrange for the function to return an error when the .set_speed()
method is not implemented.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 21 +------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 7cdb09037da0..18ae12df4a85 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -37,8 +37,6 @@ struct rk_gmac_ops {
 	void (*set_to_rgmii)(struct rk_priv_data *bsp_priv,
 			     int tx_delay, int rx_delay);
 	void (*set_to_rmii)(struct rk_priv_data *bsp_priv);
-	void (*set_rgmii_speed)(struct rk_priv_data *bsp_priv, int speed);
-	void (*set_rmii_speed)(struct rk_priv_data *bsp_priv, int speed);
 	int (*set_speed)(struct rk_priv_data *bsp_priv,
 			 phy_interface_t interface, int speed);
 	void (*set_clock_selection)(struct rk_priv_data *bsp_priv, bool input,
@@ -1707,29 +1705,12 @@ static int rk_set_clk_tx_rate(void *bsp_priv_, struct clk *clk_tx_i,
 			      phy_interface_t interface, int speed)
 {
 	struct rk_priv_data *bsp_priv = bsp_priv_;
-	struct device *dev = &bsp_priv->pdev->dev;
 
 	if (bsp_priv->ops->set_speed)
 		return bsp_priv->ops->set_speed(bsp_priv, bsp_priv->phy_iface,
 						speed);
 
-	switch (bsp_priv->phy_iface) {
-	case PHY_INTERFACE_MODE_RGMII:
-	case PHY_INTERFACE_MODE_RGMII_ID:
-	case PHY_INTERFACE_MODE_RGMII_RXID:
-	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (bsp_priv->ops->set_rgmii_speed)
-			bsp_priv->ops->set_rgmii_speed(bsp_priv, speed);
-		break;
-	case PHY_INTERFACE_MODE_RMII:
-		if (bsp_priv->ops->set_rmii_speed)
-			bsp_priv->ops->set_rmii_speed(bsp_priv, speed);
-		break;
-	default:
-		dev_err(dev, "unsupported interface %d", bsp_priv->phy_iface);
-	}
-
-	return 0;
+	return -EINVAL;
 }
 
 static int rk_gmac_probe(struct platform_device *pdev)
-- 
2.30.2

Re: [PATCH net-next 9/9] net: stmmac: rk: remove obsolete .set_*_speed() methods

[Andrew Lunn]

On Thu, Jun 12, 2025 at 04:41:22PM +0100, Russell King (Oracle) wrote:
> Now that no SoC implements the .set_*_speed() methods, we can get rid
> of these methods and the now unused code in rk_set_clk_tx_rate().
> Arrange for the function to return an error when the .set_speed()
> method is not implemented.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 0/9] net: stmmac: rk: much needed cleanups

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 12 Jun 2025 16:40:54 +0100 you wrote:
> Hi,
> 
> This series starts attacking the reams of fairly identical duplicated
> code in dwmac-rk. Every new SoC that comes along seems to need more
> code added to this file because e.g. the way the clock is controlled
> is different in every SoC.
> 
> [...]

Here is the summary with links:
  - [net-next,1/9] net: stmmac: rk: add get_interfaces() implementation
    https://git.kernel.org/netdev/net-next/c/1f59e30403a7
  - [net-next,2/9] net: stmmac: rk: simplify set_*_speed()
    https://git.kernel.org/netdev/net-next/c/e6e9e837d312
  - [net-next,3/9] net: stmmac: rk: add struct for programming register based speeds
    https://git.kernel.org/netdev/net-next/c/3de607d13b6b
  - [net-next,4/9] net: stmmac: rk: combine rv1126 set_*_speed() methods
    https://git.kernel.org/netdev/net-next/c/29f0aca13914
  - [net-next,5/9] net: stmmac: rk: combine clk_mac_speed rate setting functions
    https://git.kernel.org/netdev/net-next/c/d8d6096f8161
  - [net-next,6/9] net: stmmac: rk: combine .set_*_speed() methods
    https://git.kernel.org/netdev/net-next/c/3930c2cca657
  - [net-next,7/9] net: stmmac: rk: simplify px30_set_rmii_speed()
    https://git.kernel.org/netdev/net-next/c/c5cddcdbd2af
  - [net-next,8/9] net: stmmac: rk: convert px30_set_rmii_speed() to .set_speed()
    https://git.kernel.org/netdev/net-next/c/9165487d21a4
  - [net-next,9/9] net: stmmac: rk: remove obsolete .set_*_speed() methods
    https://git.kernel.org/netdev/net-next/c/0f3a079786ba

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html