Skip to contentSkip to conversation

net: stmmac: rk: final cleanups part

PATCH Applied Russell King (Oracle) 5 participants 20 messages Feb 4 – Feb 5

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover net: stmmac: rk: final cleanups part

Hi,

This is the last part of my current dwmac-rk cleanups.

 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 361 ++++++++++---------------
 1 file changed, 150 insertions(+), 211 deletions(-)
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#1 lore raw

PATCH 1/6 net: stmmac: rk: introduce flags indicating support for RGMII/RMII

Introduce two boolean flags into struct rk_priv_data indicating whether RGMII and/or RMII is supported for this instance. Use these to configure the supported_interfaces mask for phylink, validate the interface mode. Initialise these from equivalent flags in the rk_gmac_ops or depending on the presence of the ops->set_to_rgmii and ops->set_to_mii methods. Finally, make ops->set_to_* optional.

This will allow us to get rid of empty set_to_rmii() methods.

---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 35 +++++++++++++------
 1 file changed, 25 insertions(+), 10 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +25 -10
index c56cc40683a3..73d471844022 100644
@@ -53,6 +53,8 @@ struct rk_gmac_ops {
 
 	bool gmac_grf_reg_in_php;
 	bool clock_grf_reg_in_php;
+	bool supports_rgmii;
+	bool supports_rmii;
 	bool php_grf_required;
 	bool regs_valid;
 	u32 regs[];
@@ -86,6 +88,8 @@ struct rk_priv_data {
 	bool clk_enabled;
 	bool clock_input;
 	bool integrated_phy;
+	bool supports_rgmii;
+	bool supports_rmii;
 
 	struct clk_bulk_data *clks;
 	int num_clks;
@@ -1415,6 +1419,9 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
 	bsp_priv->clock_grf_reg = ops->clock_grf_reg;
 	bsp_priv->clock = ops->clock;
 
+	bsp_priv->supports_rgmii = ops->supports_rgmii || !!ops->set_to_rgmii;
+	bsp_priv->supports_rmii = ops->supports_rmii || !!ops->set_to_rmii;
+
 	if (ops->init) {
 		ret = ops->init(bsp_priv);
 		if (ret) {
@@ -1434,11 +1441,11 @@ static int rk_gmac_check_ops(struct rk_priv_data *bsp_priv)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		if (!bsp_priv->ops->set_to_rgmii)
+		if (!bsp_priv->supports_rgmii)
 			return -EINVAL;
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-		if (!bsp_priv->ops->set_to_rmii)
+		if (!bsp_priv->supports_rmii)
 			return -EINVAL;
 		break;
 	default:
@@ -1489,24 +1496,32 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 	switch (bsp_priv->phy_iface) {
 	case PHY_INTERFACE_MODE_RGMII:
 		dev_info(dev, "init for RGMII\n");
-		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay,
-					    bsp_priv->rx_delay);
+		if (bsp_priv->ops->set_to_rgmii)
+			bsp_priv->ops->set_to_rgmii(bsp_priv,
+						    bsp_priv->tx_delay,
+						    bsp_priv->rx_delay);
 		break;
 	case PHY_INTERFACE_MODE_RGMII_ID:
 		dev_info(dev, "init for RGMII_ID\n");
-		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, 0);
+		if (bsp_priv->ops->set_to_rgmii)
+			bsp_priv->ops->set_to_rgmii(bsp_priv, 0, 0);
 		break;
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 		dev_info(dev, "init for RGMII_RXID\n");
-		bsp_priv->ops->set_to_rgmii(bsp_priv, bsp_priv->tx_delay, 0);
+		if (bsp_priv->ops->set_to_rgmii)
+			bsp_priv->ops->set_to_rgmii(bsp_priv,
+						    bsp_priv->tx_delay, 0);
 		break;
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		dev_info(dev, "init for RGMII_TXID\n");
-		bsp_priv->ops->set_to_rgmii(bsp_priv, 0, bsp_priv->rx_delay);
+		if (bsp_priv->ops->set_to_rgmii)
+			bsp_priv->ops->set_to_rgmii(bsp_priv,
+						    0, bsp_priv->rx_delay);
 		break;
 	case PHY_INTERFACE_MODE_RMII:
 		dev_info(dev, "init for RMII\n");
-		bsp_priv->ops->set_to_rmii(bsp_priv);
+		if (bsp_priv->ops->set_to_rmii)
+			bsp_priv->ops->set_to_rmii(bsp_priv);
 		break;
 	default:
 		dev_err(dev, "NO interface defined!\n");
@@ -1542,10 +1557,10 @@ static void rk_get_interfaces(struct stmmac_priv *priv, void *bsp_priv,
 {
 	struct rk_priv_data *rk = bsp_priv;
 
-	if (rk->ops->set_to_rgmii)
+	if (rk->supports_rgmii)
 		phy_interface_set_rgmii(interfaces);
 
-	if (rk->ops->set_to_rmii)
+	if (rk->supports_rmii)
 		__set_bit(PHY_INTERFACE_MODE_RMII, interfaces);
 }
 
Signature · 1 line
2.47.3
maintainer Reviewed-by on v1 1/6
#2 lore raw

Re: [PATCH net-next 1/6] net: stmmac: rk: introduce flags indicating support for RGMII/RMII

Hi,

Quoted 10 lines · Russell King (Oracle)

Introduce two boolean flags into struct rk_priv_data indicating whether RGMII and/or RMII is supported for this instance. Use these to configure the supported_interfaces mask for phylink, validate the interface mode. Initialise these from equivalent flags in the rk_gmac_ops or depending on the presence of the ops->set_to_rgmii and ops->set_to_mii methods. Finally, make ops->set_to_* optional.

This will allow us to get rid of empty set_to_rmii() methods.

Maxime

maintainer Reviewed-by Tested-by on v1 1/6
#3 lore raw

Re: [PATCH net-next 1/6] net: stmmac: rk: introduce flags indicating support for RGMII/RMII

In reply to Russell King (Oracle)

Am Mittwoch, 4. Februar 2026, 10:14:17 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 10 lines

Introduce two boolean flags into struct rk_priv_data indicating whether RGMII and/or RMII is supported for this instance. Use these to configure the supported_interfaces mask for phylink, validate the interface mode. Initialise these from equivalent flags in the rk_gmac_ops or depending on the presence of the ops->set_to_rgmii and ops->set_to_mii methods. Finally, make ops->set_to_* optional.

This will allow us to get rid of empty set_to_rmii() methods.

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip

#4 lore raw

PATCH 2/6 net: stmmac: rk: replace empty set_to_rmii() with supports_rmii

In reply to Russell King (Oracle)

Rather than providing a now-empty set_to_rmii() method to indicate that RMII is supported, switch to setting ops->supports_rmii instead.

---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 86 ++++++-------------
 1 file changed, 24 insertions(+), 62 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +24 -62
index 73d471844022..25ad8600e9e2 100644
@@ -268,12 +268,7 @@ static void rk_gmac_integrated_fephy_powerdown(struct rk_priv_data *priv,
 
 #define PX30_GRF_GMAC_CON1		0x0904
 
-static void px30_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops px30_ops = {
-	.set_to_rmii = px30_set_to_rmii,
 	.set_speed = rk_set_clk_mac_speed,
 
 	.gmac_grf_reg = PX30_GRF_GMAC_CON1,
@@ -281,6 +276,8 @@ static const struct rk_gmac_ops px30_ops = {
 
 	.clock_grf_reg = PX30_GRF_GMAC_CON1,
 	.clock.mac_speed_mask = BIT_U16(2),
+
+	.supports_rmii = true,
 };
 
 #define RK3128_GRF_MAC_CON0	0x0168
@@ -307,13 +304,8 @@ static void rk3128_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3128_GMAC_CLK_TX_DL_CFG(tx_delay));
 }
 
-static void rk3128_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops rk3128_ops = {
 	.set_to_rgmii = rk3128_set_to_rgmii,
-	.set_to_rmii = rk3128_set_to_rmii,
 
 	.gmac_grf_reg = RK3128_GRF_MAC_CON1,
 	.gmac_phy_intf_sel_mask = GENMASK_U16(8, 6),
@@ -323,6 +315,8 @@ static const struct rk_gmac_ops rk3128_ops = {
 	.clock.gmii_clk_sel_mask = GENMASK_U16(13, 12),
 	.clock.rmii_clk_sel_mask = BIT_U16(11),
 	.clock.mac_speed_mask = BIT_U16(10),
+
+	.supports_rmii = true,
 };
 
 #define RK3228_GRF_MAC_CON0	0x0900
@@ -410,13 +404,8 @@ static void rk3288_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3288_GMAC_CLK_TX_DL_CFG(tx_delay));
 }
 
-static void rk3288_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops rk3288_ops = {
 	.set_to_rgmii = rk3288_set_to_rgmii,
-	.set_to_rmii = rk3288_set_to_rmii,
 
 	.gmac_grf_reg = RK3288_GRF_SOC_CON1,
 	.gmac_phy_intf_sel_mask = GENMASK_U16(8, 6),
@@ -426,6 +415,8 @@ static const struct rk_gmac_ops rk3288_ops = {
 	.clock.gmii_clk_sel_mask = GENMASK_U16(13, 12),
 	.clock.rmii_clk_sel_mask = BIT_U16(11),
 	.clock.mac_speed_mask = BIT_U16(10),
+
+	.supports_rmii = true,
 };
 
 #define RK3308_GRF_MAC_CON0		0x04a0
@@ -434,18 +425,14 @@ static const struct rk_gmac_ops rk3288_ops = {
 #define RK3308_GMAC_FLOW_CTRL		GRF_BIT(3)
 #define RK3308_GMAC_FLOW_CTRL_CLR	GRF_CLR_BIT(3)
 
-static void rk3308_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops rk3308_ops = {
-	.set_to_rmii = rk3308_set_to_rmii,
-
 	.gmac_grf_reg = RK3308_GRF_MAC_CON0,
 	.gmac_phy_intf_sel_mask = GENMASK_U16(4, 2),
 
 	.clock_grf_reg = RK3308_GRF_MAC_CON0,
 	.clock.mac_speed_mask = BIT_U16(0),
+
+	.supports_rmii = true,
 };
 
 #define RK3328_GRF_MAC_CON0	0x0900
@@ -497,10 +484,6 @@ static void rk3328_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3328_GMAC_CLK_TX_DL_CFG(tx_delay));
 }
 
-static void rk3328_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static void rk3328_integrated_phy_powerup(struct rk_priv_data *priv)
 {
 	regmap_write(priv->grf, RK3328_GRF_MACPHY_CON1,
@@ -512,7 +495,6 @@ static void rk3328_integrated_phy_powerup(struct rk_priv_data *priv)
 static const struct rk_gmac_ops rk3328_ops = {
 	.init = rk3328_init,
 	.set_to_rgmii = rk3328_set_to_rgmii,
-	.set_to_rmii = rk3328_set_to_rmii,
 	.integrated_phy_powerup = rk3328_integrated_phy_powerup,
 	.integrated_phy_powerdown = rk_gmac_integrated_ephy_powerdown,
 
@@ -522,6 +504,8 @@ static const struct rk_gmac_ops rk3328_ops = {
 	.clock.rmii_clk_sel_mask = BIT_U16(7),
 	.clock.mac_speed_mask = BIT_U16(2),
 
+	.supports_rmii = true,
+
 	.regs_valid = true,
 	.regs = {
 		0xff540000, /* gmac2io */
@@ -554,13 +538,8 @@ static void rk3366_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3366_GMAC_CLK_TX_DL_CFG(tx_delay));
 }
 
-static void rk3366_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops rk3366_ops = {
 	.set_to_rgmii = rk3366_set_to_rgmii,
-	.set_to_rmii = rk3366_set_to_rmii,
 
 	.gmac_grf_reg = RK3366_GRF_SOC_CON6,
 	.gmac_phy_intf_sel_mask = GENMASK_U16(11, 9),
@@ -570,6 +549,8 @@ static const struct rk_gmac_ops rk3366_ops = {
 	.clock.gmii_clk_sel_mask = GENMASK_U16(5, 4),
 	.clock.rmii_clk_sel_mask = BIT_U16(3),
 	.clock.mac_speed_mask = BIT_U16(7),
+
+	.supports_rmii = true,
 };
 
 #define RK3368_GRF_SOC_CON15	0x043c
@@ -596,13 +577,8 @@ static void rk3368_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3368_GMAC_CLK_TX_DL_CFG(tx_delay));
 }
 
-static void rk3368_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops rk3368_ops = {
 	.set_to_rgmii = rk3368_set_to_rgmii,
-	.set_to_rmii = rk3368_set_to_rmii,
 
 	.gmac_grf_reg = RK3368_GRF_SOC_CON15,
 	.gmac_phy_intf_sel_mask = GENMASK_U16(11, 9),
@@ -612,6 +588,8 @@ static const struct rk_gmac_ops rk3368_ops = {
 	.clock.gmii_clk_sel_mask = GENMASK_U16(5, 4),
 	.clock.rmii_clk_sel_mask = BIT_U16(3),
 	.clock.mac_speed_mask = BIT_U16(7),
+
+	.supports_rmii = true,
 };
 
 #define RK3399_GRF_SOC_CON5	0xc214
@@ -638,13 +616,8 @@ static void rk3399_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3399_GMAC_CLK_TX_DL_CFG(tx_delay));
 }
 
-static void rk3399_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops rk3399_ops = {
 	.set_to_rgmii = rk3399_set_to_rgmii,
-	.set_to_rmii = rk3399_set_to_rmii,
 
 	.gmac_grf_reg = RK3399_GRF_SOC_CON5,
 	.gmac_phy_intf_sel_mask = GENMASK_U16(11, 9),
@@ -654,6 +627,8 @@ static const struct rk_gmac_ops rk3399_ops = {
 	.clock.gmii_clk_sel_mask = GENMASK_U16(5, 4),
 	.clock.rmii_clk_sel_mask = BIT_U16(3),
 	.clock.mac_speed_mask = BIT_U16(7),
+
+	.supports_rmii = true,
 };
 
 #define RK3506_GRF_SOC_CON8		0x0020
@@ -884,18 +859,15 @@ static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3568_GMAC_TXCLK_DLY_ENABLE);
 }
 
-static void rk3568_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops rk3568_ops = {
 	.init = rk3568_init,
 	.set_to_rgmii = rk3568_set_to_rgmii,
-	.set_to_rmii = rk3568_set_to_rmii,
 	.set_speed = rk_set_clk_mac_speed,
 
 	.gmac_phy_intf_sel_mask = GENMASK_U16(6, 4),
 
+	.supports_rmii = true,
+
 	.regs_valid = true,
 	.regs = {
 		0xfe2a0000, /* gmac0 */
@@ -969,10 +941,6 @@ static void rk3576_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3576_GMAC_CLK_RX_DL_CFG(rx_delay));
 }
 
-static void rk3576_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static void rk3576_set_clock_selection(struct rk_priv_data *bsp_priv, bool input,
 				       bool enable)
 {
@@ -992,7 +960,6 @@ static void rk3576_set_clock_selection(struct rk_priv_data *bsp_priv, bool input
 static const struct rk_gmac_ops rk3576_ops = {
 	.init = rk3576_init,
 	.set_to_rgmii = rk3576_set_to_rgmii,
-	.set_to_rmii = rk3576_set_to_rmii,
 	.set_clock_selection = rk3576_set_clock_selection,
 
 	.gmac_rmii_mode_mask = BIT_U16(3),
@@ -1000,6 +967,8 @@ static const struct rk_gmac_ops rk3576_ops = {
 	.clock.gmii_clk_sel_mask = GENMASK_U16(6, 5),
 	.clock.rmii_clk_sel_mask = BIT_U16(5),
 
+	.supports_rmii = true,
+
 	.php_grf_required = true,
 	.regs_valid = true,
 	.regs = {
@@ -1120,19 +1089,15 @@ static const struct rk_gmac_ops rk3588_ops = {
 #define RV1108_GMAC_FLOW_CTRL		GRF_BIT(3)
 #define RV1108_GMAC_FLOW_CTRL_CLR	GRF_CLR_BIT(3)
 
-static void rv1108_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-}
-
 static const struct rk_gmac_ops rv1108_ops = {
-	.set_to_rmii = rv1108_set_to_rmii,
-
 	.gmac_grf_reg = RV1108_GRF_GMAC_CON0,
 	.gmac_phy_intf_sel_mask = GENMASK_U16(6, 4),
 
 	.clock_grf_reg = RV1108_GRF_GMAC_CON0,
 	.clock.rmii_clk_sel_mask = BIT_U16(7),
 	.clock.mac_speed_mask = BIT_U16(2),
+
+	.supports_rmii = true,
 };
 
 #define RV1126_GRF_GMAC_CON0		0X0070
@@ -1176,17 +1141,14 @@ static void rv1126_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RV1126_GMAC_M1_CLK_TX_DL_CFG(tx_delay));
 }
 
-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_speed = rk_set_clk_mac_speed,
 
 	.gmac_grf_reg = RV1126_GRF_GMAC_CON0,
 	.gmac_phy_intf_sel_mask = GENMASK_U16(6, 4),
+
+	.supports_rmii = true,
 };
 
 static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
Signature · 1 line
2.47.3
maintainer Reviewed-by Tested-by on v1 2/6
#6 lore raw

Re: [PATCH net-next 2/6] net: stmmac: rk: replace empty set_to_rmii() with supports_rmii

In reply to Russell King (Oracle)

Am Mittwoch, 4. Februar 2026, 10:14:22 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 4 lines

Rather than providing a now-empty set_to_rmii() method to indicate that RMII is supported, switch to setting ops->supports_rmii instead.

#7 lore raw

PATCH 3/6 net: stmmac: rk: rk3328: gmac2phy only supports RMII

In reply to Russell King (Oracle)

As detailed in a previous commit ("net: stmmac: rk: convert rk3328 to use bsp_priv->id") rk3328 gmac2phy only supports RMII, whereas gmac2io supports both RMII and RGMII. Clear supports_rgmii for gmac2phy.

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +1 -0
index 25ad8600e9e2..8dd2122fd9b1 100644
@@ -465,6 +465,7 @@ static int rk3328_init(struct rk_priv_data *bsp_priv)
 	case 1: /* gmac2phy */
 		bsp_priv->gmac_grf_reg = RK3328_GRF_MAC_CON2;
 		bsp_priv->clock_grf_reg = RK3328_GRF_MAC_CON2;
+		bsp_priv->supports_rgmii = false;
 		return 0;
 
 	default:
Signature · 1 line
2.47.3
maintainer Reviewed-by on v1 3/6
#8 lore raw

Re: [PATCH net-next 3/6] net: stmmac: rk: rk3328: gmac2phy only supports RMII

Quoted 5 lines · Russell King (Oracle)

As detailed in a previous commit ("net: stmmac: rk: convert rk3328 to use bsp_priv->id") rk3328 gmac2phy only supports RMII, whereas gmac2io supports both RMII and RGMII. Clear supports_rgmii for gmac2phy.

Maxime

maintainer Reviewed-by Tested-by on v1 3/6
#9 lore raw

Re: [PATCH net-next 3/6] net: stmmac: rk: rk3328: gmac2phy only supports RMII

In reply to Russell King (Oracle)

Am Mittwoch, 4. Februar 2026, 10:14:28 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 5 lines

As detailed in a previous commit ("net: stmmac: rk: convert rk3328 to use bsp_priv->id") rk3328 gmac2phy only supports RMII, whereas gmac2io supports both RMII and RGMII. Clear supports_rgmii for gmac2phy.

Sadly the rk3328-board I have only uses the gmac2io controller.

#10 lore raw

PATCH 4/6 net: stmmac: rk: rk3528: gmac0 only supports RMII

In reply to Russell King (Oracle)

RK3528 gmac0 dtsi contains:

                gmac0: ethernet@ffbd0000 {
                        phy-handle = <&rmii0_phy>;
                        phy-mode = "rmii";

                        mdio0: mdio {
                                rmii0_phy: ethernet-phy@2 {
                                        phy-is-integrated;
                                };
                        };
                };

This follows the same pattern as rk3328, where this gmac instance only supports RMII. Disable RGMII in phylink's supported_interfaces mask for this gmac instance.

---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +1 -0
index 8dd2122fd9b1..4099cbc5d0de 100644
@@ -728,6 +728,7 @@ static int rk3528_init(struct rk_priv_data *bsp_priv)
 	case 0:
 		bsp_priv->clock_grf_reg = RK3528_VO_GRF_GMAC_CON;
 		bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(3);
+		bsp_priv->supports_rgmii = false;
 		return 0;
 
 	case 1:
Signature · 1 line
2.47.3
maintainer Reviewed-by on v1 4/6
#11 lore raw

Re: [PATCH net-next 4/6] net: stmmac: rk: rk3528: gmac0 only supports RMII

Quoted 13 lines · Russell King (Oracle)

RK3528 gmac0 dtsi contains:

                gmac0: ethernet@ffbd0000 {
                        phy-handle = <&rmii0_phy>;
                        phy-mode = "rmii";

                        mdio0: mdio {
                                rmii0_phy: ethernet-phy@2 {
                                        phy-is-integrated;
                                };
                        };
                };
Quoted 5 lines

This follows the same pattern as rk3328, where this gmac instance only supports RMII. Disable RGMII in phylink's supported_interfaces mask for this gmac instance.

Maxime

maintainer Reviewed-by on v1 4/6
#12 lore raw

Re: [PATCH net-next 4/6] net: stmmac: rk: rk3528: gmac0 only supports RMII

In reply to Russell King (Oracle)

Am Mittwoch, 4. Februar 2026, 10:14:33 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 13 lines

RK3528 gmac0 dtsi contains:

                gmac0: ethernet@ffbd0000 {
                        phy-handle = <&rmii0_phy>;
                        phy-mode = "rmii";

                        mdio0: mdio {
                                rmii0_phy: ethernet-phy@2 {
                                        phy-is-integrated;
                                };
                        };
                };
Quoted 5 lines

This follows the same pattern as rk3328, where this gmac instance only supports RMII. Disable RGMII in phylink's supported_interfaces mask for this gmac instance.

#13 lore raw

PATCH 5/6 net: stmmac: rk: use rk_encode_wm16() for clock selection

In reply to Russell King (Oracle)

Use rk_encode_wm16() for RMII clock gating control, and also for the io_clksel bit used to select the transmit clock between CRU-derived and IO-derived clock sources.

Both of these were configured via the "set_clock_selection" method in the SoC specific operations, but there is no requirement to change the io_clksel except when enabling clocks.

It is also possible that we don't need to ungate the RMII clock if we are operating in RGMII mode, but this commit makes no change there.

Split up the configuration of these as separate functions, and remove the set_clock_selection() method. Since these clocking bits are in the same register that we call the "speed" register, move the logic for writing that register into rk_write_speed_grf_reg().

---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 174 ++++++++----------
 1 file changed, 75 insertions(+), 99 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +75 -99
index 4099cbc5d0de..ed9adac70f0a 100644
@@ -27,8 +27,17 @@
 struct rk_priv_data;
 
 struct rk_clock_fields {
+	/* io_clksel_cru_mask - io_clksel bit in clock GRF register which,
+	 * when set, selects the tx clock from CRU.
+	 */
+	u16 io_clksel_cru_mask;
+	/* io_clksel_io_mask - io_clksel bit in clock GRF register which,
+	 * when set, selects the tx clock from IO.
+	 */
+	u16 io_clksel_io_mask;
 	u16 gmii_clk_sel_mask;
 	u16 rmii_clk_sel_mask;
+	u16 rmii_gate_en_mask;
 	u16 mac_speed_mask;
 };
 
@@ -39,8 +48,6 @@ struct rk_gmac_ops {
 	void (*set_to_rmii)(struct rk_priv_data *bsp_priv);
 	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);
 	void (*integrated_phy_powerdown)(struct rk_priv_data *bsp_priv);
 
@@ -171,6 +178,54 @@ static int rk_write_clock_grf_reg(struct rk_priv_data *bsp_priv, u32 val)
 	return regmap_write(regmap, bsp_priv->clock_grf_reg, val);
 }
 
+static int rk_set_rmii_gate_en(struct rk_priv_data *bsp_priv, bool state)
+{
+	u32 val;
+
+	if (!bsp_priv->clock.rmii_gate_en_mask)
+		return 0;
+
+	val = rk_encode_wm16(state, bsp_priv->clock.rmii_gate_en_mask);
+
+	return rk_write_clock_grf_reg(bsp_priv, val);
+}
+
+static int rk_ungate_rmii_clock(struct rk_priv_data *bsp_priv)
+{
+	return rk_set_rmii_gate_en(bsp_priv, false);
+}
+
+static int rk_gate_rmii_clock(struct rk_priv_data *bsp_priv)
+{
+	return rk_set_rmii_gate_en(bsp_priv, true);
+}
+
+static int rk_configure_io_clksel(struct rk_priv_data *bsp_priv)
+{
+	bool io, cru;
+	u32 val;
+
+	if (!bsp_priv->clock.io_clksel_io_mask &&
+	    !bsp_priv->clock.io_clksel_cru_mask)
+		return 0;
+
+	io = bsp_priv->clock_input;
+	cru = !io;
+
+	/* The io_clksel configuration can be either:
+	 *  0=CRU, 1=IO (rk3506, rk3520, rk3576) or
+	 *  0=IO, 1=CRU (rk3588)
+	 * where CRU means the transmit clock comes from the CRU and IO
+	 * means the transmit clock comes from IO.
+	 *
+	 * Handle this by having two masks.
+	 */
+	val = rk_encode_wm16(io, bsp_priv->clock.io_clksel_io_mask) |
+	      rk_encode_wm16(cru, bsp_priv->clock.io_clksel_cru_mask);
+
+	return rk_write_clock_grf_reg(bsp_priv, val);
+}
+
 static int rk_set_clk_mac_speed(struct rk_priv_data *bsp_priv,
 				phy_interface_t interface, int speed)
 {
@@ -637,12 +692,6 @@ static const struct rk_gmac_ops rk3399_ops = {
 
 #define RK3506_GMAC_RMII_MODE		GRF_BIT(1)
 
-#define RK3506_GMAC_CLK_SELECT_CRU	GRF_CLR_BIT(5)
-#define RK3506_GMAC_CLK_SELECT_IO	GRF_BIT(5)
-
-#define RK3506_GMAC_CLK_RMII_GATE	GRF_BIT(2)
-#define RK3506_GMAC_CLK_RMII_NOGATE	GRF_CLR_BIT(2)
-
 static int rk3506_init(struct rk_priv_data *bsp_priv)
 {
 	switch (bsp_priv->id) {
@@ -667,26 +716,13 @@ static void rk3506_set_to_rmii(struct rk_priv_data *bsp_priv)
 	regmap_write(bsp_priv->grf, offset, RK3506_GMAC_RMII_MODE);
 }
 
-static void rk3506_set_clock_selection(struct rk_priv_data *bsp_priv,
-				       bool input, bool enable)
-{
-	unsigned int value, offset, id = bsp_priv->id;
-
-	offset = (id == 1) ? RK3506_GRF_SOC_CON11 : RK3506_GRF_SOC_CON8;
-
-	value = input ? RK3506_GMAC_CLK_SELECT_IO :
-			RK3506_GMAC_CLK_SELECT_CRU;
-	value |= enable ? RK3506_GMAC_CLK_RMII_NOGATE :
-			  RK3506_GMAC_CLK_RMII_GATE;
-	regmap_write(bsp_priv->grf, offset, value);
-}
-
 static const struct rk_gmac_ops rk3506_ops = {
 	.init = rk3506_init,
 	.set_to_rmii = rk3506_set_to_rmii,
-	.set_clock_selection = rk3506_set_clock_selection,
 
+	.clock.io_clksel_io_mask = BIT_U16(5),
 	.clock.rmii_clk_sel_mask = BIT_U16(3),
+	.clock.rmii_gate_en_mask = BIT_U16(2),
 
 	.regs_valid = true,
 	.regs = {
@@ -714,27 +750,22 @@ static const struct rk_gmac_ops rk3506_ops = {
 #define RK3528_GMAC1_PHY_INTF_SEL_RGMII	GRF_CLR_BIT(8)
 #define RK3528_GMAC1_PHY_INTF_SEL_RMII	GRF_BIT(8)
 
-#define RK3528_GMAC1_CLK_SELECT_CRU	GRF_CLR_BIT(12)
-#define RK3528_GMAC1_CLK_SELECT_IO	GRF_BIT(12)
-
-#define RK3528_GMAC0_CLK_RMII_GATE	GRF_BIT(2)
-#define RK3528_GMAC0_CLK_RMII_NOGATE	GRF_CLR_BIT(2)
-#define RK3528_GMAC1_CLK_RMII_GATE	GRF_BIT(9)
-#define RK3528_GMAC1_CLK_RMII_NOGATE	GRF_CLR_BIT(9)
-
 static int rk3528_init(struct rk_priv_data *bsp_priv)
 {
 	switch (bsp_priv->id) {
 	case 0:
 		bsp_priv->clock_grf_reg = RK3528_VO_GRF_GMAC_CON;
 		bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(3);
+		bsp_priv->clock.rmii_gate_en_mask = BIT_U16(2);
 		bsp_priv->supports_rgmii = false;
 		return 0;
 
 	case 1:
 		bsp_priv->clock_grf_reg = RK3528_VPU_GRF_GMAC_CON5;
+		bsp_priv->clock.io_clksel_io_mask = BIT_U16(12);
 		bsp_priv->clock.gmii_clk_sel_mask = GENMASK_U16(11, 10);
 		bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(10);
+		bsp_priv->clock.rmii_gate_en_mask = BIT_U16(9);
 		return 0;
 
 	default:
@@ -766,24 +797,6 @@ static void rk3528_set_to_rmii(struct rk_priv_data *bsp_priv)
 			     RK3528_GMAC0_PHY_INTF_SEL_RMII);
 }
 
-static void rk3528_set_clock_selection(struct rk_priv_data *bsp_priv,
-				       bool input, bool enable)
-{
-	unsigned int val;
-
-	if (bsp_priv->id == 1) {
-		val = input ? RK3528_GMAC1_CLK_SELECT_IO :
-			      RK3528_GMAC1_CLK_SELECT_CRU;
-		val |= enable ? RK3528_GMAC1_CLK_RMII_NOGATE :
-				RK3528_GMAC1_CLK_RMII_GATE;
-		regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5, val);
-	} else {
-		val = enable ? RK3528_GMAC0_CLK_RMII_NOGATE :
-			       RK3528_GMAC0_CLK_RMII_GATE;
-		regmap_write(bsp_priv->grf, RK3528_VO_GRF_GMAC_CON, val);
-	}
-}
-
 static void rk3528_integrated_phy_powerup(struct rk_priv_data *bsp_priv)
 {
 	rk_gmac_integrated_fephy_powerup(bsp_priv, RK3528_VO_GRF_MACPHY_CON0);
@@ -798,7 +811,6 @@ static const struct rk_gmac_ops rk3528_ops = {
 	.init = rk3528_init,
 	.set_to_rgmii = rk3528_set_to_rgmii,
 	.set_to_rmii = rk3528_set_to_rmii,
-	.set_clock_selection = rk3528_set_clock_selection,
 	.integrated_phy_powerup = rk3528_integrated_phy_powerup,
 	.integrated_phy_powerdown = rk3528_integrated_phy_powerdown,
 	.regs_valid = true,
@@ -896,12 +908,6 @@ static const struct rk_gmac_ops rk3568_ops = {
 #define RK3576_GRF_GMAC_CON0			0X0020
 #define RK3576_GRF_GMAC_CON1			0X0024
 
-#define RK3576_GMAC_CLK_SELECT_IO		GRF_BIT(7)
-#define RK3576_GMAC_CLK_SELECT_CRU		GRF_CLR_BIT(7)
-
-#define RK3576_GMAC_CLK_RMII_GATE		GRF_BIT(4)
-#define RK3576_GMAC_CLK_RMII_NOGATE		GRF_CLR_BIT(4)
-
 static int rk3576_init(struct rk_priv_data *bsp_priv)
 {
 	switch (bsp_priv->id) {
@@ -943,31 +949,16 @@ static void rk3576_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3576_GMAC_CLK_RX_DL_CFG(rx_delay));
 }
 
-static void rk3576_set_clock_selection(struct rk_priv_data *bsp_priv, bool input,
-				       bool enable)
-{
-	unsigned int val = input ? RK3576_GMAC_CLK_SELECT_IO :
-				   RK3576_GMAC_CLK_SELECT_CRU;
-	unsigned int offset_con;
-
-	val |= enable ? RK3576_GMAC_CLK_RMII_NOGATE :
-			RK3576_GMAC_CLK_RMII_GATE;
-
-	offset_con = bsp_priv->id == 1 ? RK3576_GRF_GMAC_CON1 :
-					 RK3576_GRF_GMAC_CON0;
-
-	regmap_write(bsp_priv->grf, offset_con, val);
-}
-
 static const struct rk_gmac_ops rk3576_ops = {
 	.init = rk3576_init,
 	.set_to_rgmii = rk3576_set_to_rgmii,
-	.set_clock_selection = rk3576_set_clock_selection,
 
 	.gmac_rmii_mode_mask = BIT_U16(3),
 
+	.clock.io_clksel_io_mask = BIT_U16(7),
 	.clock.gmii_clk_sel_mask = GENMASK_U16(6, 5),
 	.clock.rmii_clk_sel_mask = BIT_U16(5),
+	.clock.rmii_gate_en_mask = BIT_U16(4),
 
 	.supports_rmii = true,
 
@@ -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(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;
 
 	default:
@@ -1052,23 +1041,10 @@ static void rk3588_set_to_rmii(struct rk_priv_data *bsp_priv)
 		     RK3588_GMAC_CLK_RMII_MODE(bsp_priv->id));
 }
 
-static void rk3588_set_clock_selection(struct rk_priv_data *bsp_priv, bool input,
-				       bool enable)
-{
-	unsigned int val = input ? RK3588_GMAC_CLK_SELECT_IO(bsp_priv->id) :
-				   RK3588_GMAC_CLK_SELECT_CRU(bsp_priv->id);
-
-	val |= enable ? RK3588_GMAC_CLK_RMII_NOGATE(bsp_priv->id) :
-			RK3588_GMAC_CLK_RMII_GATE(bsp_priv->id);
-
-	regmap_write(bsp_priv->php_grf, RK3588_GRF_CLK_CON1, val);
-}
-
 static const struct rk_gmac_ops rk3588_ops = {
 	.init = rk3588_init,
 	.set_to_rgmii = rk3588_set_to_rgmii,
 	.set_to_rmii = rk3588_set_to_rmii,
-	.set_clock_selection = rk3588_set_clock_selection,
 
 	.gmac_grf_reg_in_php = true,
 	.gmac_grf_reg = RK3588_GRF_GMAC_CON0,
@@ -1216,19 +1192,15 @@ static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
 			if (ret)
 				return ret;
 
-			if (bsp_priv->ops && bsp_priv->ops->set_clock_selection)
-				bsp_priv->ops->set_clock_selection(bsp_priv,
-					       bsp_priv->clock_input, true);
+			rk_configure_io_clksel(bsp_priv);
+			rk_ungate_rmii_clock(bsp_priv);
 
 			mdelay(5);
 			bsp_priv->clk_enabled = true;
 		}
 	} else {
 		if (bsp_priv->clk_enabled) {
-			if (bsp_priv->ops && bsp_priv->ops->set_clock_selection) {
-				bsp_priv->ops->set_clock_selection(bsp_priv,
-					      bsp_priv->clock_input, false);
-			}
+			rk_gate_rmii_clock(bsp_priv);
 
 			clk_bulk_disable_unprepare(bsp_priv->num_clks,
 						   bsp_priv->clks);
@@ -1395,6 +1367,10 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
 		}
 	}
 
+	if (bsp_priv->clock.io_clksel_cru_mask &&
+	    bsp_priv->clock.io_clksel_io_mask)
+		dev_warn(dev, "both CRU and IO io_clksel masks should not be populated - driver may malfunction\n");
+
 	return bsp_priv;
 }
 
Signature · 1 line
2.47.3
maintainer Reviewed-by Tested-by on v1 5/6
#14 lore raw

Re: [PATCH net-next 5/6] net: stmmac: rk: use rk_encode_wm16() for clock selection

Am Mittwoch, 4. Februar 2026, 10:14:38 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 26 lines

Use rk_encode_wm16() for RMII clock gating control, and also for the io_clksel bit used to select the transmit clock between CRU-derived and IO-derived clock sources.

Both of these were configured via the "set_clock_selection" method in the SoC specific operations, but there is no requirement to change the io_clksel except when enabling clocks.

It is also possible that we don't need to ungate the RMII clock if we are operating in RGMII mode, but this commit makes no change there.

Split up the configuration of these as separate functions, and remove the set_clock_selection() method. Since these clocking bits are in the same register that we call the "speed" register, move the logic for writing that register into rk_write_speed_grf_reg().

---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 174 ++++++++----------
 1 file changed, 75 insertions(+), 99 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +0 -0
index 4099cbc5d0de..ed9adac70f0a 100644

drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c line 36

Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +7 -0
@@ -27,8 +27,17 @@
 struct rk_priv_data;
 
 struct rk_clock_fields {
+	/* io_clksel_cru_mask - io_clksel bit in clock GRF register which,
+	 * when set, selects the tx clock from CRU.
+	 */
+	u16 io_clksel_cru_mask;
+	/* io_clksel_io_mask - io_clksel bit in clock GRF register which,
+	 * when set, selects the tx clock from IO.
+	 */

nit: comment stile does not seem to follow the kernel coding style
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n622

Quoted 6 lines
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +2 -0
+	u16 io_clksel_io_mask;
 	u16 gmii_clk_sel_mask;
 	u16 rmii_clk_sel_mask;
+	u16 rmii_gate_en_mask;
 	u16 mac_speed_mask;
 };

[...]

drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c static int rk_write_clock_grf_reg(struct rk_priv_data *bsp_priv, u32 val) · line 222

Quoted 8 lines
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +8 -0
+	/* The io_clksel configuration can be either:
+	 *  0=CRU, 1=IO (rk3506, rk3520, rk3576) or
+	 *  0=IO, 1=CRU (rk3588)
+	 * where CRU means the transmit clock comes from the CRU and IO
+	 * means the transmit clock comes from IO.
+	 *
+	 * Handle this by having two masks.
+	 */

nit: same as above

Other than these 2 nits, this looks good to me.

#15 lore raw

Re: [PATCH net-next 5/6] net: stmmac: rk: use rk_encode_wm16() for clock selection

Quoted 29 lines · Heiko Stübner

Am Mittwoch, 4. Februar 2026, 10:14:38 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 28 lines

Use rk_encode_wm16() for RMII clock gating control, and also for the io_clksel bit used to select the transmit clock between CRU-derived and IO-derived clock sources.

Both of these were configured via the "set_clock_selection" method in the SoC specific operations, but there is no requirement to change the io_clksel except when enabling clocks.

It is also possible that we don't need to ungate the RMII clock if we are operating in RGMII mode, but this commit makes no change there.

Split up the configuration of these as separate functions, and remove the set_clock_selection() method. Since these clocking bits are in the same register that we call the "speed" register, move the logic for writing that register into rk_write_speed_grf_reg().

---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 174 ++++++++----------
 1 file changed, 75 insertions(+), 99 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +0 -0
index 4099cbc5d0de..ed9adac70f0a 100644
@@ -27,8 +27,17 @@
 struct rk_priv_data;
 
Quoted 11 lines

drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c line 36

Quoted 8 lines
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +7 -0
 struct rk_clock_fields {
+	/* io_clksel_cru_mask - io_clksel bit in clock GRF register which,
+	 * when set, selects the tx clock from CRU.
+	 */
+	u16 io_clksel_cru_mask;
+	/* io_clksel_io_mask - io_clksel bit in clock GRF register which,
+	 * when set, selects the tx clock from IO.
+	 */

nit: comment stile does not seem to follow the kernel coding style
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n622

netdev has historically had this style which is separate from the rest of the kernel. The file already uses this comment style, so it is entirely correct to keep to the style which already exists in this file, rather than mixing styles and turning it into a mess.

Quoted 2 lines
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
maintainer
#16 lore raw

Re: [PATCH net-next 5/6] net: stmmac: rk: use rk_encode_wm16() for clock selection

Hi Russell,

Am Mittwoch, 4. Februar 2026, 23:26:21 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 42 lines · Heiko Stübner
Quoted 29 lines

Am Mittwoch, 4. Februar 2026, 10:14:38 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 28 lines

Use rk_encode_wm16() for RMII clock gating control, and also for the io_clksel bit used to select the transmit clock between CRU-derived and IO-derived clock sources.

Both of these were configured via the "set_clock_selection" method in the SoC specific operations, but there is no requirement to change the io_clksel except when enabling clocks.

It is also possible that we don't need to ungate the RMII clock if we are operating in RGMII mode, but this commit makes no change there.

Split up the configuration of these as separate functions, and remove the set_clock_selection() method. Since these clocking bits are in the same register that we call the "speed" register, move the logic for writing that register into rk_write_speed_grf_reg().

---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 174 ++++++++----------
 1 file changed, 75 insertions(+), 99 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +0 -0
index 4099cbc5d0de..ed9adac70f0a 100644
@@ -27,8 +27,17 @@
 struct rk_priv_data;
 
Quoted 11 lines

drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c line 36

Quoted 8 lines
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +7 -0
 struct rk_clock_fields {
+	/* io_clksel_cru_mask - io_clksel bit in clock GRF register which,
+	 * when set, selects the tx clock from CRU.
+	 */
+	u16 io_clksel_cru_mask;
+	/* io_clksel_io_mask - io_clksel bit in clock GRF register which,
+	 * when set, selects the tx clock from IO.
+	 */

nit: comment stile does not seem to follow the kernel coding style
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst#n622

Quoted 4 lines

netdev has historically had this style which is separate from the rest of the kernel. The file already uses this comment style, so it is entirely correct to keep to the style which already exists in this file, rather than mixing styles and turning it into a mess.

thanks for the clarification. Then the comment style is obviously fine.

Regards
Heiko

Quoted 4 lines
Quoted 2 lines
#17 lore raw

PATCH 6/6 net: stmmac: rk: rk3506, rk3528 and rk3588 have rmii_mode in clock register

In reply to Russell King (Oracle)

rk3506, rk3528 and rk3588 have the rmii_mode bit in the clock GRF register rather than the gmac GRF register. Provide a mask for this field in the clock register, and convert these SoCs to use this. Add the necessary code in rk_gmac_powerup() to write this field.

This allows us to get rid of these SoCs set_to_rmii() function. As such, we need to mark these SoCs as supporting RMII mode.

---
 .../net/ethernet/stmicro/stmmac/dwmac-rk.c    | 64 +++++++------------
 1 file changed, 24 insertions(+), 40 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c +24 -40
index ed9adac70f0a..b0441a368cb1 100644
@@ -38,6 +38,7 @@ struct rk_clock_fields {
 	u16 gmii_clk_sel_mask;
 	u16 rmii_clk_sel_mask;
 	u16 rmii_gate_en_mask;
+	u16 rmii_mode_mask;
 	u16 mac_speed_mask;
 };
 
@@ -708,21 +709,15 @@ static int rk3506_init(struct rk_priv_data *bsp_priv)
 	}
 }
 
-static void rk3506_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-	unsigned int id = bsp_priv->id, offset;
-
-	offset = (id == 1) ? RK3506_GRF_SOC_CON11 : RK3506_GRF_SOC_CON8;
-	regmap_write(bsp_priv->grf, offset, RK3506_GMAC_RMII_MODE);
-}
-
 static const struct rk_gmac_ops rk3506_ops = {
 	.init = rk3506_init,
-	.set_to_rmii = rk3506_set_to_rmii,
 
 	.clock.io_clksel_io_mask = BIT_U16(5),
 	.clock.rmii_clk_sel_mask = BIT_U16(3),
 	.clock.rmii_gate_en_mask = BIT_U16(2),
+	.clock.rmii_mode_mask = BIT_U16(1),
+
+	.supports_rmii = true,
 
 	.regs_valid = true,
 	.regs = {
@@ -746,10 +741,6 @@ static const struct rk_gmac_ops rk3506_ops = {
 #define RK3528_GMAC_CLK_RX_DL_CFG(val)	GRF_FIELD(15, 8, val)
 #define RK3528_GMAC_CLK_TX_DL_CFG(val)	GRF_FIELD(7, 0, val)
 
-#define RK3528_GMAC0_PHY_INTF_SEL_RMII	GRF_BIT(1)
-#define RK3528_GMAC1_PHY_INTF_SEL_RGMII	GRF_CLR_BIT(8)
-#define RK3528_GMAC1_PHY_INTF_SEL_RMII	GRF_BIT(8)
-
 static int rk3528_init(struct rk_priv_data *bsp_priv)
 {
 	switch (bsp_priv->id) {
@@ -757,6 +748,7 @@ static int rk3528_init(struct rk_priv_data *bsp_priv)
 		bsp_priv->clock_grf_reg = RK3528_VO_GRF_GMAC_CON;
 		bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(3);
 		bsp_priv->clock.rmii_gate_en_mask = BIT_U16(2);
+		bsp_priv->clock.rmii_mode_mask = BIT_U16(1);
 		bsp_priv->supports_rgmii = false;
 		return 0;
 
@@ -766,6 +758,7 @@ static int rk3528_init(struct rk_priv_data *bsp_priv)
 		bsp_priv->clock.gmii_clk_sel_mask = GENMASK_U16(11, 10);
 		bsp_priv->clock.rmii_clk_sel_mask = BIT_U16(10);
 		bsp_priv->clock.rmii_gate_en_mask = BIT_U16(9);
+		bsp_priv->clock.rmii_mode_mask = BIT_U16(8);
 		return 0;
 
 	default:
@@ -776,9 +769,6 @@ static int rk3528_init(struct rk_priv_data *bsp_priv)
 static void rk3528_set_to_rgmii(struct rk_priv_data *bsp_priv,
 				int tx_delay, int rx_delay)
 {
-	regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5,
-		     RK3528_GMAC1_PHY_INTF_SEL_RGMII);
-
 	regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5,
 		     DELAY_ENABLE(RK3528, tx_delay, rx_delay));
 
@@ -787,16 +777,6 @@ static void rk3528_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3528_GMAC_CLK_TX_DL_CFG(tx_delay));
 }
 
-static void rk3528_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-	if (bsp_priv->id == 1)
-		regmap_write(bsp_priv->grf, RK3528_VPU_GRF_GMAC_CON5,
-			     RK3528_GMAC1_PHY_INTF_SEL_RMII);
-	else
-		regmap_write(bsp_priv->grf, RK3528_VO_GRF_GMAC_CON,
-			     RK3528_GMAC0_PHY_INTF_SEL_RMII);
-}
-
 static void rk3528_integrated_phy_powerup(struct rk_priv_data *bsp_priv)
 {
 	rk_gmac_integrated_fephy_powerup(bsp_priv, RK3528_VO_GRF_MACPHY_CON0);
@@ -810,9 +790,11 @@ static void rk3528_integrated_phy_powerdown(struct rk_priv_data *bsp_priv)
 static const struct rk_gmac_ops rk3528_ops = {
 	.init = rk3528_init,
 	.set_to_rgmii = rk3528_set_to_rgmii,
-	.set_to_rmii = rk3528_set_to_rmii,
 	.integrated_phy_powerup = rk3528_integrated_phy_powerup,
 	.integrated_phy_powerdown = rk3528_integrated_phy_powerdown,
+
+	.supports_rmii = true,
+
 	.regs_valid = true,
 	.regs = {
 		0xffbd0000, /* gmac0 */
@@ -988,9 +970,6 @@ static const struct rk_gmac_ops rk3576_ops = {
 #define RK3588_GRF_GMAC_CON0			0X0008
 #define RK3588_GRF_CLK_CON1			0X0070
 
-#define RK3588_GMAC_CLK_RMII_MODE(id)		GRF_BIT(5 * (id))
-#define RK3588_GMAC_CLK_RGMII_MODE(id)		GRF_CLR_BIT(5 * (id))
-
 static int rk3588_init(struct rk_priv_data *bsp_priv)
 {
 	switch (bsp_priv->id) {
@@ -1000,6 +979,7 @@ static int rk3588_init(struct rk_priv_data *bsp_priv)
 		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);
+		bsp_priv->clock.rmii_mode_mask = BIT_U16(0);
 		return 0;
 
 	case 1:
@@ -1008,6 +988,7 @@ static int rk3588_init(struct rk_priv_data *bsp_priv)
 		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);
+		bsp_priv->clock.rmii_mode_mask = BIT_U16(5);
 		return 0;
 
 	default:
@@ -1023,9 +1004,6 @@ static void rk3588_set_to_rgmii(struct rk_priv_data *bsp_priv,
 	offset_con = bsp_priv->id == 1 ? RK3588_GRF_GMAC_CON9 :
 					 RK3588_GRF_GMAC_CON8;
 
-	regmap_write(bsp_priv->php_grf, RK3588_GRF_CLK_CON1,
-		     RK3588_GMAC_CLK_RGMII_MODE(id));
-
 	regmap_write(bsp_priv->grf, RK3588_GRF_GMAC_CON7,
 		     RK3588_GMAC_RXCLK_DLY_ENABLE(id) |
 		     RK3588_GMAC_TXCLK_DLY_ENABLE(id));
@@ -1035,16 +1013,9 @@ static void rk3588_set_to_rgmii(struct rk_priv_data *bsp_priv,
 		     RK3588_GMAC_CLK_TX_DL_CFG(tx_delay));
 }
 
-static void rk3588_set_to_rmii(struct rk_priv_data *bsp_priv)
-{
-	regmap_write(bsp_priv->php_grf, RK3588_GRF_CLK_CON1,
-		     RK3588_GMAC_CLK_RMII_MODE(bsp_priv->id));
-}
-
 static const struct rk_gmac_ops rk3588_ops = {
 	.init = rk3588_init,
 	.set_to_rgmii = rk3588_set_to_rgmii,
-	.set_to_rmii = rk3588_set_to_rmii,
 
 	.gmac_grf_reg_in_php = true,
 	.gmac_grf_reg = RK3588_GRF_GMAC_CON0,
@@ -1052,6 +1023,8 @@ static const struct rk_gmac_ops rk3588_ops = {
 	.clock_grf_reg_in_php = true,
 	.clock_grf_reg = RK3588_GRF_CLK_CON1,
 
+	.supports_rmii = true,
+
 	.php_grf_required = true,
 	.regs_valid = true,
 	.regs = {
@@ -1432,6 +1405,17 @@ static int rk_gmac_powerup(struct rk_priv_data *bsp_priv)
 		}
 	}
 
+	if (bsp_priv->clock.rmii_mode_mask) {
+		val = rk_encode_wm16(intf == PHY_INTF_SEL_RMII,
+				     bsp_priv->clock.rmii_mode_mask);
+
+		ret = rk_write_clock_grf_reg(bsp_priv, val);
+		if (ret < 0) {
+			gmac_clk_enable(bsp_priv, false);
+			return ret;
+		}
+	}
+
 	/*rmii or rgmii*/
 	switch (bsp_priv->phy_iface) {
 	case PHY_INTERFACE_MODE_RGMII:
Signature · 1 line
2.47.3
maintainer Reviewed-by Tested-by on v1 6/6
#18 lore raw

Re: [PATCH net-next 6/6] net: stmmac: rk: rk3506, rk3528 and rk3588 have rmii_mode in clock register

Am Mittwoch, 4. Februar 2026, 10:14:43 Mitteleuropäische Normalzeit schrieb Russell King (Oracle):

Quoted 9 lines

rk3506, rk3528 and rk3588 have the rmii_mode bit in the clock GRF register rather than the gmac GRF register. Provide a mask for this field in the clock register, and convert these SoCs to use this. Add the necessary code in rk_gmac_powerup() to write this field.

This allows us to get rid of these SoCs set_to_rmii() function. As such, we need to mark these SoCs as supporting RMII mode.

#19 lore raw
In reply to Russell King (Oracle)
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 6 lines · On Wed, 4 Feb 2026 09:14:02 +0000 you wrote:

Hi,

This is the last part of my current dwmac-rk cleanups.

 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 361 ++++++++++---------------
 1 file changed, 150 insertions(+), 211 deletions(-)

Here is the summary with links:
- [net-next,1/6] net: stmmac: rk: introduce flags indicating support for RGMII/RMII

    https://git.kernel.org/netdev/net-next/c/d323769d64f2
  - [net-next,2/6] net: stmmac: rk: replace empty set_to_rmii() with supports_rmii
    https://git.kernel.org/netdev/net-next/c/a9d4aff670ed
  - [net-next,3/6] net: stmmac: rk: rk3328: gmac2phy only supports RMII
    https://git.kernel.org/netdev/net-next/c/f365541bf7b6
  - [net-next,4/6] net: stmmac: rk: rk3528: gmac0 only supports RMII
    https://git.kernel.org/netdev/net-next/c/d7d92037cbd8
  - [net-next,5/6] net: stmmac: rk: use rk_encode_wm16() for clock selection
    https://git.kernel.org/netdev/net-next/c/5c1fc7cb81df
  - [net-next,6/6] net: stmmac: rk: rk3506, rk3528 and rk3588 have rmii_mode in clock register
    https://git.kernel.org/netdev/net-next/c/b10d56b0db3a

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html