[PATCH net-next 00/11] net: stmmac: multi-interface stmmac

[PATCH net-next 00/11] net: stmmac: multi-interface stmmac

[Russell King (Oracle)]

Hi,

This series adds a callback for platform glue to configure the stmmac
core interface mode depending on the PHY interface mode that is being
used. This is currently only called justbefore the dwmac core is reset
since these signals are latched on reset.

Included in this series are changes to s32 to move its PHY_INTF_SEL_x
definitions out of the way of the dwmac core's signals which has more
entitlement to use this name. We convert dwmac-imx as an example.

Including other platform glue would make this series excessively large,
but once this core code is merged, the individual platform glue updates
can be posted one after another as they will be independent of each
other.

It is hoped that this callback can be used in future to reconfigure the
dwmac core when the interface mode changes to support PHYs that change
their interface mode, but we're nowhere near being able to do that yet.

 drivers/net/ethernet/stmicro/stmmac/common.h      |  10 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c   | 134 +++++++---------------
 drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c   |  10 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |   1 +
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  54 +++++++++
 include/linux/stmmac.h                            |   1 +
 6 files changed, 115 insertions(+), 95 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 01/11] net: stmmac: imx: use phylink's interface mode for set_clk_tx_rate()

[Russell King (Oracle)]

imx_dwmac_set_clk_tx_rate() is passed the interface mode from phylink
which will be the same as plat_dat->phy_interface. Use the passed-in
interface mode rather than plat_dat->phy_interface.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index 4268b9987237..147fa08d5b6e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -195,9 +195,6 @@ static void imx_dwmac_exit(struct platform_device *pdev, void *priv)
 static int imx_dwmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i,
 				     phy_interface_t interface, int speed)
 {
-	struct imx_priv_data *dwmac = bsp_priv;
-
-	interface = dwmac->plat_dat->phy_interface;
 	if (interface == PHY_INTERFACE_MODE_RMII ||
 	    interface == PHY_INTERFACE_MODE_MII)
 		return 0;
-- 
2.47.3

[PATCH net-next 02/11] net: stmmac: s32: move PHY_INTF_SEL_x definitions out of the way

[Russell King (Oracle)]

S32's PHY_INTF_SEL_x definitions conflict with those for the dwmac
cores as they use a different bitmapping. Add a S32 prefix so that
they are unique.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
index ee095ac13203..2b7ad64bfdf7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
@@ -24,10 +24,10 @@
 #define GMAC_INTF_RATE_125M	125000000	/* 125MHz */
 
 /* SoC PHY interface control register */
-#define PHY_INTF_SEL_MII	0x00
-#define PHY_INTF_SEL_SGMII	0x01
-#define PHY_INTF_SEL_RGMII	0x02
-#define PHY_INTF_SEL_RMII	0x08
+#define S32_PHY_INTF_SEL_MII	0x00
+#define S32_PHY_INTF_SEL_SGMII	0x01
+#define S32_PHY_INTF_SEL_RGMII	0x02
+#define S32_PHY_INTF_SEL_RMII	0x08
 
 struct s32_priv_data {
 	void __iomem *ioaddr;
@@ -40,7 +40,7 @@ struct s32_priv_data {
 
 static int s32_gmac_write_phy_intf_select(struct s32_priv_data *gmac)
 {
-	writel(PHY_INTF_SEL_RGMII, gmac->ctrl_sts);
+	writel(S32_PHY_INTF_SEL_RGMII, gmac->ctrl_sts);
 
 	dev_dbg(gmac->dev, "PHY mode set to %s\n", phy_modes(*gmac->intf_mode));
 
-- 
2.47.3

[PATCH net-next 03/11] net: stmmac: add phy_intf_sel and ACTPHYIF definitions

[Russell King (Oracle)]

Add definitions for the active PHY interface found in DMA hardware
feature register 0, and also used to configure the core in multi-
interface designs via phy_intf_sel.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 27083af54568..7395bbb94aea 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -313,6 +313,16 @@ struct stmmac_safety_stats {
 #define DMA_HW_FEAT_ACTPHYIF	0x70000000	/* Active/selected PHY iface */
 #define DEFAULT_DMA_PBL		8
 
+/* phy_intf_sel_i and ACTPHYIF encodings */
+#define PHY_INTF_SEL_GMII_MII	0
+#define PHY_INTF_SEL_RGMII	1
+#define PHY_INTF_SEL_SGMII	2
+#define PHY_INTF_SEL_TBI	3
+#define PHY_INTF_SEL_RMII	4
+#define PHY_INTF_SEL_RTBI	5
+#define PHY_INTF_SEL_SMII	6
+#define PHY_INTF_SEL_REVMII	7
+
 /* MSI defines */
 #define STMMAC_MSI_VEC_MAX	32
 
-- 
2.47.3

[PATCH net-next 04/11] net: stmmac: add stmmac_get_phy_intf_sel()

[Russell King (Oracle)]

Provide a function to translate the PHY interface mode to the
phy_intf_sel pin configuration for dwmac1000 and dwmac4 cores that
support multiple interfaces. We currently handle MII, GMII, RGMII,
SGMII, RMII and REVMII, but not TBI, RTBI nor SMII as drivers do not
appear to use these three and the driver doesn't currently support
these.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  1 +
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 20 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 3ea680cc63d8..0ea74c88a779 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -396,6 +396,7 @@ void stmmac_ptp_register(struct stmmac_priv *priv);
 void stmmac_ptp_unregister(struct stmmac_priv *priv);
 int stmmac_xdp_open(struct net_device *dev);
 void stmmac_xdp_release(struct net_device *dev);
+int stmmac_get_phy_intf_sel(phy_interface_t interface);
 int stmmac_resume(struct device *dev);
 int stmmac_suspend(struct device *dev);
 void stmmac_dvr_remove(struct device *dev);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1e69c1a7dc6c..95d2cd020a0c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3082,6 +3082,26 @@ static void stmmac_check_ether_addr(struct stmmac_priv *priv)
 	}
 }
 
+int stmmac_get_phy_intf_sel(phy_interface_t interface)
+{
+	int phy_intf_sel = -EINVAL;
+
+	if (interface == PHY_INTERFACE_MODE_MII ||
+	    interface == PHY_INTERFACE_MODE_GMII)
+		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
+	else if (phy_interface_mode_is_rgmii(interface))
+		phy_intf_sel = PHY_INTF_SEL_RGMII;
+	else if (interface == PHY_INTERFACE_MODE_SGMII)
+		phy_intf_sel = PHY_INTF_SEL_SGMII;
+	else if (interface == PHY_INTERFACE_MODE_RMII)
+		phy_intf_sel = PHY_INTF_SEL_RMII;
+	else if (interface == PHY_INTERFACE_MODE_REVMII)
+		phy_intf_sel = PHY_INTF_SEL_REVMII;
+
+	return phy_intf_sel;
+}
+EXPORT_SYMBOL_GPL(stmmac_get_phy_intf_sel);
+
 /**
  * stmmac_init_dma_engine - DMA init.
  * @priv: driver private structure
-- 
2.47.3

[PATCH net-next 05/11] net: stmmac: add support for configuring the phy_intf_sel inputs

[Russell King (Oracle)]

When dwmac is synthesised with support for multiple PHY interfaces, the
core provides phy_intf_sel inputs, sampled on reset, to configure the
PHY facing interface. Use stmmac_get_phy_intf_sel() in core code to
determine the dwmac phy_intf_sel input value, and provide a new
platform method called with this value just before we issue a soft
reset to the dwmac core.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 34 +++++++++++++++++++
 include/linux/stmmac.h                        |  1 +
 2 files changed, 35 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 95d2cd020a0c..596995d2bec3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3102,6 +3102,36 @@ int stmmac_get_phy_intf_sel(phy_interface_t interface)
 }
 EXPORT_SYMBOL_GPL(stmmac_get_phy_intf_sel);
 
+static int stmmac_prereset_configure(struct stmmac_priv *priv)
+{
+	struct plat_stmmacenet_data *plat_dat = priv->plat;
+	phy_interface_t interface;
+	int phy_intf_sel, ret;
+
+	if (!plat_dat->set_phy_intf_sel)
+		return 0;
+
+	interface = plat_dat->phy_interface;
+	phy_intf_sel = stmmac_get_phy_intf_sel(interface);
+	if (phy_intf_sel < 0) {
+		netdev_err(priv->dev,
+			   "failed to get phy_intf_sel for %s: %pe\n",
+			   phy_modes(interface), ERR_PTR(phy_intf_sel));
+		return phy_intf_sel;
+	}
+
+	ret = plat_dat->set_phy_intf_sel(plat_dat->bsp_priv, phy_intf_sel);
+	if (ret == -EINVAL)
+		netdev_err(priv->dev, "platform does not support %s\n",
+			   phy_modes(interface));
+	else if (ret < 0)
+		netdev_err(priv->dev,
+			   "platform failed to set interface %s: %pe\n",
+			   phy_modes(interface), ERR_PTR(ret));
+
+	return ret;
+}
+
 /**
  * stmmac_init_dma_engine - DMA init.
  * @priv: driver private structure
@@ -3128,6 +3158,10 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
 	if (priv->extend_desc && (priv->mode == STMMAC_RING_MODE))
 		priv->plat->dma_cfg->atds = 1;
 
+	ret = stmmac_prereset_configure(priv);
+	if (ret)
+		return ret;
+
 	ret = stmmac_reset(priv, priv->ioaddr);
 	if (ret) {
 		netdev_err(priv->dev, "Failed to reset the dma\n");
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 151c81c560c8..48e9f1d4e17e 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -250,6 +250,7 @@ struct plat_stmmacenet_data {
 	struct stmmac_txq_cfg tx_queues_cfg[MTL_MAX_TX_QUEUES];
 	void (*get_interfaces)(struct stmmac_priv *priv, void *bsp_priv,
 			       unsigned long *interfaces);
+	int (*set_phy_intf_sel)(void *priv, u8 phy_intf_sel);
 	int (*set_clk_tx_rate)(void *priv, struct clk *clk_tx_i,
 			       phy_interface_t interface, int speed);
 	void (*fix_mac_speed)(void *priv, int speed, unsigned int mode);
-- 
2.47.3

[PATCH net-next 06/11] net: stmmac: imx: convert to PHY_INTF_SEL_xxx

[Russell King (Oracle)]

Convert dwmac-imx to use the PHY_INTF_SEL_xxx definitions rather than
constants via:
- ensuring that the prefix for the MASK and value definitions is the
  same.
- using FIELD_PREP() to shift the PHY_INTF_SEL_xxx definition to the
  appropriate bitfield.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index 147fa08d5b6e..4fbee59e7337 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -23,18 +23,25 @@
 #include "stmmac_platform.h"
 
 #define GPR_ENET_QOS_INTF_MODE_MASK	GENMASK(21, 16)
-#define GPR_ENET_QOS_INTF_SEL_MII	(0x0 << 16)
-#define GPR_ENET_QOS_INTF_SEL_RMII	(0x4 << 16)
-#define GPR_ENET_QOS_INTF_SEL_RGMII	(0x1 << 16)
+#define GPR_ENET_QOS_INTF_SEL_MASK	GENMASK(20, 16)
+#define GPR_ENET_QOS_INTF_SEL_MII	FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, \
+						   PHY_INTF_SEL_GMII_MII)
+#define GPR_ENET_QOS_INTF_SEL_RMII	FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, \
+						   PHY_INTF_SEL_RMII)
+#define GPR_ENET_QOS_INTF_SEL_RGMII	FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, \
+						   PHY_INTF_SEL_RGMII)
 #define GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 19)
 #define GPR_ENET_QOS_CLK_TX_CLK_SEL	(0x1 << 20)
 #define GPR_ENET_QOS_RGMII_EN		(0x1 << 21)
 
 #define MX93_GPR_ENET_QOS_INTF_MODE_MASK	GENMASK(3, 0)
-#define MX93_GPR_ENET_QOS_INTF_MASK		GENMASK(3, 1)
-#define MX93_GPR_ENET_QOS_INTF_SEL_MII		(0x0 << 1)
-#define MX93_GPR_ENET_QOS_INTF_SEL_RMII		(0x4 << 1)
-#define MX93_GPR_ENET_QOS_INTF_SEL_RGMII	(0x1 << 1)
+#define MX93_GPR_ENET_QOS_INTF_SEL_MASK		GENMASK(3, 1)
+#define MX93_GPR_ENET_QOS_INTF_SEL_MII		FIELD_PREP(MX93_GPR_ENET_QOS_INTF_SEL_MASK, \
+							   PHY_INTF_SEL_GMII_MII)
+#define MX93_GPR_ENET_QOS_INTF_SEL_RMII		FIELD_PREP(MX93_GPR_ENET_QOS_INTF_SEL_MASK, \
+							   PHY_INTF_SEL_RMII)
+#define MX93_GPR_ENET_QOS_INTF_SEL_RGMII	FIELD_PREP(MX93_GPR_ENET_QOS_INTF_SEL_MASK, \
+							   PHY_INTF_SEL_RGMII)
 #define MX93_GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 0)
 #define MX93_GPR_ENET_QOS_CLK_SEL_MASK		BIT_MASK(0)
 #define MX93_GPR_CLK_SEL_OFFSET			(4)
@@ -241,7 +248,7 @@ static void imx93_dwmac_fix_speed(void *priv, int speed, unsigned int mode)
 	if (regmap_read(dwmac->intf_regmap, dwmac->intf_reg_off, &iface))
 		return;
 
-	iface &= MX93_GPR_ENET_QOS_INTF_MASK;
+	iface &= MX93_GPR_ENET_QOS_INTF_SEL_MASK;
 	if (iface != MX93_GPR_ENET_QOS_INTF_SEL_RGMII)
 		return;
 
-- 
2.47.3

[PATCH net-next 07/11] net: stmmac: imx: use FIELD_PREP()/FIELD_GET() for PHY_INTF_SEL_x

[Russell King (Oracle)]

Use FIELD_PREP()/FIELD_GET() in the functions to construct the PHY
interface selection bitfield or to extract its value.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index 4fbee59e7337..f1cfccd4269c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -24,24 +24,12 @@
 
 #define GPR_ENET_QOS_INTF_MODE_MASK	GENMASK(21, 16)
 #define GPR_ENET_QOS_INTF_SEL_MASK	GENMASK(20, 16)
-#define GPR_ENET_QOS_INTF_SEL_MII	FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, \
-						   PHY_INTF_SEL_GMII_MII)
-#define GPR_ENET_QOS_INTF_SEL_RMII	FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, \
-						   PHY_INTF_SEL_RMII)
-#define GPR_ENET_QOS_INTF_SEL_RGMII	FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, \
-						   PHY_INTF_SEL_RGMII)
 #define GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 19)
 #define GPR_ENET_QOS_CLK_TX_CLK_SEL	(0x1 << 20)
 #define GPR_ENET_QOS_RGMII_EN		(0x1 << 21)
 
 #define MX93_GPR_ENET_QOS_INTF_MODE_MASK	GENMASK(3, 0)
 #define MX93_GPR_ENET_QOS_INTF_SEL_MASK		GENMASK(3, 1)
-#define MX93_GPR_ENET_QOS_INTF_SEL_MII		FIELD_PREP(MX93_GPR_ENET_QOS_INTF_SEL_MASK, \
-							   PHY_INTF_SEL_GMII_MII)
-#define MX93_GPR_ENET_QOS_INTF_SEL_RMII		FIELD_PREP(MX93_GPR_ENET_QOS_INTF_SEL_MASK, \
-							   PHY_INTF_SEL_RMII)
-#define MX93_GPR_ENET_QOS_INTF_SEL_RGMII	FIELD_PREP(MX93_GPR_ENET_QOS_INTF_SEL_MASK, \
-							   PHY_INTF_SEL_RGMII)
 #define MX93_GPR_ENET_QOS_CLK_GEN_EN		(0x1 << 0)
 #define MX93_GPR_ENET_QOS_CLK_SEL_MASK		BIT_MASK(0)
 #define MX93_GPR_CLK_SEL_OFFSET			(4)
@@ -77,22 +65,24 @@ struct imx_priv_data {
 static int imx8mp_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 {
 	struct imx_priv_data *dwmac = plat_dat->bsp_priv;
+	u8 phy_intf_sel;
 	int val;
 
 	switch (plat_dat->phy_interface) {
 	case PHY_INTERFACE_MODE_MII:
-		val = GPR_ENET_QOS_INTF_SEL_MII;
+		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
+		val = 0;
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-		val = GPR_ENET_QOS_INTF_SEL_RMII;
-		val |= (dwmac->rmii_refclk_ext ? 0 : GPR_ENET_QOS_CLK_TX_CLK_SEL);
+		phy_intf_sel = PHY_INTF_SEL_RMII;
+		val = dwmac->rmii_refclk_ext ? 0 : GPR_ENET_QOS_CLK_TX_CLK_SEL;
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		val = GPR_ENET_QOS_INTF_SEL_RGMII |
-		      GPR_ENET_QOS_RGMII_EN;
+		phy_intf_sel = PHY_INTF_SEL_RGMII;
+		val = GPR_ENET_QOS_RGMII_EN;
 		break;
 	default:
 		pr_debug("imx dwmac doesn't support %s interface\n",
@@ -100,7 +90,9 @@ static int imx8mp_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 		return -EINVAL;
 	}
 
-	val |= GPR_ENET_QOS_CLK_GEN_EN;
+	val |= FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, phy_intf_sel) |
+	       GPR_ENET_QOS_CLK_GEN_EN;
+
 	return regmap_update_bits(dwmac->intf_regmap, dwmac->intf_reg_off,
 				  GPR_ENET_QOS_INTF_MODE_MASK, val);
 };
@@ -117,11 +109,12 @@ imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 {
 	struct imx_priv_data *dwmac = plat_dat->bsp_priv;
+	u8 phy_intf_sel;
 	int val, ret;
 
 	switch (plat_dat->phy_interface) {
 	case PHY_INTERFACE_MODE_MII:
-		val = MX93_GPR_ENET_QOS_INTF_SEL_MII;
+		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
 		break;
 	case PHY_INTERFACE_MODE_RMII:
 		if (dwmac->rmii_refclk_ext) {
@@ -132,13 +125,13 @@ static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 			if (ret)
 				return ret;
 		}
-		val = MX93_GPR_ENET_QOS_INTF_SEL_RMII;
+		phy_intf_sel = PHY_INTF_SEL_RMII;
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		val = MX93_GPR_ENET_QOS_INTF_SEL_RGMII;
+		phy_intf_sel = PHY_INTF_SEL_RGMII;
 		break;
 	default:
 		dev_dbg(dwmac->dev, "imx dwmac doesn't support %s interface\n",
@@ -146,7 +139,9 @@ static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 		return -EINVAL;
 	}
 
-	val |= MX93_GPR_ENET_QOS_CLK_GEN_EN;
+	val = FIELD_PREP(MX93_GPR_ENET_QOS_INTF_SEL_MASK, phy_intf_sel) |
+	      MX93_GPR_ENET_QOS_CLK_GEN_EN;
+
 	return regmap_update_bits(dwmac->intf_regmap, dwmac->intf_reg_off,
 				  MX93_GPR_ENET_QOS_INTF_MODE_MASK, val);
 };
@@ -248,8 +243,8 @@ static void imx93_dwmac_fix_speed(void *priv, int speed, unsigned int mode)
 	if (regmap_read(dwmac->intf_regmap, dwmac->intf_reg_off, &iface))
 		return;
 
-	iface &= MX93_GPR_ENET_QOS_INTF_SEL_MASK;
-	if (iface != MX93_GPR_ENET_QOS_INTF_SEL_RGMII)
+	if (FIELD_GET(MX93_GPR_ENET_QOS_INTF_SEL_MASK, iface) !=
+	    PHY_INTF_SEL_RGMII)
 		return;
 
 	old_ctrl = readl(dwmac->base_addr + MAC_CTRL_REG);
@@ -262,6 +257,7 @@ static void imx93_dwmac_fix_speed(void *priv, int speed, unsigned int mode)
 	readl(dwmac->base_addr + MAC_CTRL_REG);
 
 	usleep_range(10, 20);
+	iface &= MX93_GPR_ENET_QOS_INTF_SEL_MASK;
 	iface |= MX93_GPR_ENET_QOS_CLK_GEN_EN;
 	regmap_update_bits(dwmac->intf_regmap, dwmac->intf_reg_off,
 			   MX93_GPR_ENET_QOS_INTF_MODE_MASK, iface);
-- 
2.47.3

[PATCH net-next 08/11] net: stmmac: imx: use stmmac_get_phy_intf_sel()

[Russell King (Oracle)]

i.MX implementations other than IMX8DXL involve setting the dwmac core
phy_intf_sel input. Use stmmac_get_phy_intf_sel() to decode the PHY
interface mode to the phy_intf_sel value, validating the result, and
passing it into the implementation specific .set_intf_mode() method
rather than each .set_intf_mode() method doing this.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index f1cfccd4269c..dc28486a7af0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -45,7 +45,8 @@ struct imx_dwmac_ops {
 	bool mac_rgmii_txclk_auto_adj;
 
 	int (*fix_soc_reset)(struct stmmac_priv *priv, void __iomem *ioaddr);
-	int (*set_intf_mode)(struct plat_stmmacenet_data *plat_dat);
+	int (*set_intf_mode)(struct plat_stmmacenet_data *plat_dat,
+			     u8 phy_intf_sel);
 	void (*fix_mac_speed)(void *priv, int speed, unsigned int mode);
 };
 
@@ -62,26 +63,23 @@ struct imx_priv_data {
 	struct plat_stmmacenet_data *plat_dat;
 };
 
-static int imx8mp_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
+static int imx8mp_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
+				u8 phy_intf_sel)
 {
 	struct imx_priv_data *dwmac = plat_dat->bsp_priv;
-	u8 phy_intf_sel;
 	int val;
 
 	switch (plat_dat->phy_interface) {
 	case PHY_INTERFACE_MODE_MII:
-		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
 		val = 0;
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-		phy_intf_sel = PHY_INTF_SEL_RMII;
 		val = dwmac->rmii_refclk_ext ? 0 : GPR_ENET_QOS_CLK_TX_CLK_SEL;
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		phy_intf_sel = PHY_INTF_SEL_RGMII;
 		val = GPR_ENET_QOS_RGMII_EN;
 		break;
 	default:
@@ -98,7 +96,8 @@ static int imx8mp_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 };
 
 static int
-imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
+imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
+		      u8 phy_intf_sel)
 {
 	int ret = 0;
 
@@ -106,16 +105,13 @@ imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 	return ret;
 }
 
-static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
+static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
+			       u8 phy_intf_sel)
 {
 	struct imx_priv_data *dwmac = plat_dat->bsp_priv;
-	u8 phy_intf_sel;
 	int val, ret;
 
 	switch (plat_dat->phy_interface) {
-	case PHY_INTERFACE_MODE_MII:
-		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
-		break;
 	case PHY_INTERFACE_MODE_RMII:
 		if (dwmac->rmii_refclk_ext) {
 			ret = regmap_clear_bits(dwmac->intf_regmap,
@@ -125,13 +121,12 @@ static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat)
 			if (ret)
 				return ret;
 		}
-		phy_intf_sel = PHY_INTF_SEL_RMII;
 		break;
+	case PHY_INTERFACE_MODE_MII:
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		phy_intf_sel = PHY_INTF_SEL_RGMII;
 		break;
 	default:
 		dev_dbg(dwmac->dev, "imx dwmac doesn't support %s interface\n",
@@ -176,12 +171,24 @@ static int imx_dwmac_init(struct platform_device *pdev, void *priv)
 {
 	struct plat_stmmacenet_data *plat_dat;
 	struct imx_priv_data *dwmac = priv;
-	int ret;
-
-	plat_dat = dwmac->plat_dat;
+	phy_interface_t interface;
+	int phy_intf_sel, ret;
 
 	if (dwmac->ops->set_intf_mode) {
-		ret = dwmac->ops->set_intf_mode(plat_dat);
+		plat_dat = dwmac->plat_dat;
+		interface = plat_dat->phy_interface;
+
+		phy_intf_sel = stmmac_get_phy_intf_sel(interface);
+		if (phy_intf_sel != PHY_INTF_SEL_GMII_MII &&
+		    phy_intf_sel != PHY_INTF_SEL_RGMII &&
+		    phy_intf_sel != PHY_INTF_SEL_RMII) {
+			dev_dbg(dwmac->dev,
+				"imx dwmac doesn't support %s interface\n",
+				phy_modes(interface));
+			return phy_intf_sel < 0 ? phy_intf_sel : -EINVAL;
+		}
+
+		ret = dwmac->ops->set_intf_mode(plat_dat, phy_intf_sel);
 		if (ret)
 			return ret;
 	}
-- 
2.47.3

[PATCH net-next 09/11] net: stmmac: imx: simplify set_intf_mode() implementations

[Russell King (Oracle)]

Simplify the set_intf_mode() implementations, testing the phy_intf_sel
value rather than the PHY interface mode.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index dc28486a7af0..d69be9de4468 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -67,29 +67,15 @@ static int imx8mp_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
 				u8 phy_intf_sel)
 {
 	struct imx_priv_data *dwmac = plat_dat->bsp_priv;
-	int val;
-
-	switch (plat_dat->phy_interface) {
-	case PHY_INTERFACE_MODE_MII:
-		val = 0;
-		break;
-	case PHY_INTERFACE_MODE_RMII:
-		val = dwmac->rmii_refclk_ext ? 0 : GPR_ENET_QOS_CLK_TX_CLK_SEL;
-		break;
-	case PHY_INTERFACE_MODE_RGMII:
-	case PHY_INTERFACE_MODE_RGMII_ID:
-	case PHY_INTERFACE_MODE_RGMII_RXID:
-	case PHY_INTERFACE_MODE_RGMII_TXID:
-		val = GPR_ENET_QOS_RGMII_EN;
-		break;
-	default:
-		pr_debug("imx dwmac doesn't support %s interface\n",
-			 phy_modes(plat_dat->phy_interface));
-		return -EINVAL;
-	}
+	unsigned int val;
 
-	val |= FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, phy_intf_sel) |
-	       GPR_ENET_QOS_CLK_GEN_EN;
+	val = FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, phy_intf_sel) |
+	      GPR_ENET_QOS_CLK_GEN_EN;
+
+	if (phy_intf_sel == PHY_INTF_SEL_RMII && !dwmac->rmii_refclk_ext)
+		val |= GPR_ENET_QOS_CLK_TX_CLK_SEL;
+	else if (phy_intf_sel == PHY_INTF_SEL_RGMII)
+		val |= GPR_ENET_QOS_RGMII_EN;
 
 	return regmap_update_bits(dwmac->intf_regmap, dwmac->intf_reg_off,
 				  GPR_ENET_QOS_INTF_MODE_MASK, val);
@@ -99,39 +85,24 @@ static int
 imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
 		      u8 phy_intf_sel)
 {
-	int ret = 0;
-
 	/* TBD: depends on imx8dxl scu interfaces to be upstreamed */
-	return ret;
+	return 0;
 }
 
 static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
 			       u8 phy_intf_sel)
 {
 	struct imx_priv_data *dwmac = plat_dat->bsp_priv;
-	int val, ret;
-
-	switch (plat_dat->phy_interface) {
-	case PHY_INTERFACE_MODE_RMII:
-		if (dwmac->rmii_refclk_ext) {
-			ret = regmap_clear_bits(dwmac->intf_regmap,
-						dwmac->intf_reg_off +
-						MX93_GPR_CLK_SEL_OFFSET,
-						MX93_GPR_ENET_QOS_CLK_SEL_MASK);
-			if (ret)
-				return ret;
-		}
-		break;
-	case PHY_INTERFACE_MODE_MII:
-	case PHY_INTERFACE_MODE_RGMII:
-	case PHY_INTERFACE_MODE_RGMII_ID:
-	case PHY_INTERFACE_MODE_RGMII_RXID:
-	case PHY_INTERFACE_MODE_RGMII_TXID:
-		break;
-	default:
-		dev_dbg(dwmac->dev, "imx dwmac doesn't support %s interface\n",
-			phy_modes(plat_dat->phy_interface));
-		return -EINVAL;
+	unsigned int val;
+	int ret;
+
+	if (phy_intf_sel == PHY_INTF_SEL_RMII && dwmac->rmii_refclk_ext) {
+		ret = regmap_clear_bits(dwmac->intf_regmap,
+					dwmac->intf_reg_off +
+					MX93_GPR_CLK_SEL_OFFSET,
+					MX93_GPR_ENET_QOS_CLK_SEL_MASK);
+		if (ret)
+			return ret;
 	}
 
 	val = FIELD_PREP(MX93_GPR_ENET_QOS_INTF_SEL_MASK, phy_intf_sel) |
-- 
2.47.3

[PATCH net-next 10/11] net: stmmac: imx: cleanup arguments for set_intf_mode() method

[Russell King (Oracle)]

Pass the imx_priv_data instead of the plat_stmmacenet_data into the
set_intf_mode() SoC specific methods.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index d69be9de4468..ae1b73e1bcb2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -39,14 +39,15 @@
 #define RMII_RESET_SPEED		(0x3 << 14)
 #define CTRL_SPEED_MASK			GENMASK(15, 14)
 
+struct imx_priv_data;
+
 struct imx_dwmac_ops {
 	u32 addr_width;
 	u32 flags;
 	bool mac_rgmii_txclk_auto_adj;
 
 	int (*fix_soc_reset)(struct stmmac_priv *priv, void __iomem *ioaddr);
-	int (*set_intf_mode)(struct plat_stmmacenet_data *plat_dat,
-			     u8 phy_intf_sel);
+	int (*set_intf_mode)(struct imx_priv_data *dwmac, u8 phy_intf_sel);
 	void (*fix_mac_speed)(void *priv, int speed, unsigned int mode);
 };
 
@@ -63,10 +64,8 @@ struct imx_priv_data {
 	struct plat_stmmacenet_data *plat_dat;
 };
 
-static int imx8mp_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
-				u8 phy_intf_sel)
+static int imx8mp_set_intf_mode(struct imx_priv_data *dwmac, u8 phy_intf_sel)
 {
-	struct imx_priv_data *dwmac = plat_dat->bsp_priv;
 	unsigned int val;
 
 	val = FIELD_PREP(GPR_ENET_QOS_INTF_SEL_MASK, phy_intf_sel) |
@@ -82,17 +81,14 @@ static int imx8mp_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
 };
 
 static int
-imx8dxl_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
-		      u8 phy_intf_sel)
+imx8dxl_set_intf_mode(struct imx_priv_data *dwmac, u8 phy_intf_sel)
 {
 	/* TBD: depends on imx8dxl scu interfaces to be upstreamed */
 	return 0;
 }
 
-static int imx93_set_intf_mode(struct plat_stmmacenet_data *plat_dat,
-			       u8 phy_intf_sel)
+static int imx93_set_intf_mode(struct imx_priv_data *dwmac, u8 phy_intf_sel)
 {
-	struct imx_priv_data *dwmac = plat_dat->bsp_priv;
 	unsigned int val;
 	int ret;
 
@@ -140,14 +136,12 @@ static int imx_dwmac_clks_config(void *priv, bool enabled)
 
 static int imx_dwmac_init(struct platform_device *pdev, void *priv)
 {
-	struct plat_stmmacenet_data *plat_dat;
 	struct imx_priv_data *dwmac = priv;
 	phy_interface_t interface;
 	int phy_intf_sel, ret;
 
 	if (dwmac->ops->set_intf_mode) {
-		plat_dat = dwmac->plat_dat;
-		interface = plat_dat->phy_interface;
+		interface = dwmac->plat_dat->phy_interface;
 
 		phy_intf_sel = stmmac_get_phy_intf_sel(interface);
 		if (phy_intf_sel != PHY_INTF_SEL_GMII_MII &&
@@ -159,7 +153,7 @@ static int imx_dwmac_init(struct platform_device *pdev, void *priv)
 			return phy_intf_sel < 0 ? phy_intf_sel : -EINVAL;
 		}
 
-		ret = dwmac->ops->set_intf_mode(plat_dat, phy_intf_sel);
+		ret = dwmac->ops->set_intf_mode(dwmac, phy_intf_sel);
 		if (ret)
 			return ret;
 	}
-- 
2.47.3

[PATCH net-next 11/11] net: stmmac: imx: use ->set_phy_intf_sel()

[Russell King (Oracle)]

Rather than placing the phy_intf_sel() setup in the ->init() method,
move it to the new ->set_phy_intf_sel() method.

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

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
index ae1b73e1bcb2..db288fbd5a4d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c
@@ -134,36 +134,19 @@ static int imx_dwmac_clks_config(void *priv, bool enabled)
 	return ret;
 }
 
-static int imx_dwmac_init(struct platform_device *pdev, void *priv)
+static int imx_set_phy_intf_sel(void *bsp_priv, u8 phy_intf_sel)
 {
-	struct imx_priv_data *dwmac = priv;
-	phy_interface_t interface;
-	int phy_intf_sel, ret;
-
-	if (dwmac->ops->set_intf_mode) {
-		interface = dwmac->plat_dat->phy_interface;
-
-		phy_intf_sel = stmmac_get_phy_intf_sel(interface);
-		if (phy_intf_sel != PHY_INTF_SEL_GMII_MII &&
-		    phy_intf_sel != PHY_INTF_SEL_RGMII &&
-		    phy_intf_sel != PHY_INTF_SEL_RMII) {
-			dev_dbg(dwmac->dev,
-				"imx dwmac doesn't support %s interface\n",
-				phy_modes(interface));
-			return phy_intf_sel < 0 ? phy_intf_sel : -EINVAL;
-		}
+	struct imx_priv_data *dwmac = bsp_priv;
 
-		ret = dwmac->ops->set_intf_mode(dwmac, phy_intf_sel);
-		if (ret)
-			return ret;
-	}
+	if (!dwmac->ops->set_intf_mode)
+		return 0;
 
-	return 0;
-}
+	if (phy_intf_sel != PHY_INTF_SEL_GMII_MII &&
+	    phy_intf_sel != PHY_INTF_SEL_RGMII &&
+	    phy_intf_sel != PHY_INTF_SEL_RMII)
+		return -EINVAL;
 
-static void imx_dwmac_exit(struct platform_device *pdev, void *priv)
-{
-	/* nothing to do now */
+	return dwmac->ops->set_intf_mode(dwmac, phy_intf_sel);
 }
 
 static int imx_dwmac_set_clk_tx_rate(void *bsp_priv, struct clk *clk_tx_i,
@@ -342,8 +325,7 @@ static int imx_dwmac_probe(struct platform_device *pdev)
 		plat_dat->tx_queues_cfg[i].tbs_en = 1;
 
 	plat_dat->host_dma_width = dwmac->ops->addr_width;
-	plat_dat->init = imx_dwmac_init;
-	plat_dat->exit = imx_dwmac_exit;
+	plat_dat->set_phy_intf_sel = imx_set_phy_intf_sel;
 	plat_dat->clks_config = imx_dwmac_clks_config;
 	plat_dat->bsp_priv = dwmac;
 	dwmac->plat_dat = plat_dat;
-- 
2.47.3

Re: [PATCH net-next 04/11] net: stmmac: add stmmac_get_phy_intf_sel()

[Maxime Chevallier]

Hi Russell,

On 03/11/2025 12:50, Russell King (Oracle) wrote:
> Provide a function to translate the PHY interface mode to the
> phy_intf_sel pin configuration for dwmac1000 and dwmac4 cores that
> support multiple interfaces. We currently handle MII, GMII, RGMII,
> SGMII, RMII and REVMII, but not TBI, RTBI nor SMII as drivers do not
> appear to use these three and the driver doesn't currently support
> these.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

First, thanks for this work !

[...]

> +int stmmac_get_phy_intf_sel(phy_interface_t interface)
> +{
> +	int phy_intf_sel = -EINVAL;
> +
> +	if (interface == PHY_INTERFACE_MODE_MII ||
> +	    interface == PHY_INTERFACE_MODE_GMII)
> +		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
> +	else if (phy_interface_mode_is_rgmii(interface))
> +		phy_intf_sel = PHY_INTF_SEL_RGMII;
> +	else if (interface == PHY_INTERFACE_MODE_SGMII)
> +		phy_intf_sel = PHY_INTF_SEL_SGMII;
> +	else if (interface == PHY_INTERFACE_MODE_RMII)
> +		phy_intf_sel = PHY_INTF_SEL_RMII;
> +	else if (interface == PHY_INTERFACE_MODE_REVMII)
> +		phy_intf_sel = PHY_INTF_SEL_REVMII;
> +
> +	return phy_intf_sel;
> +}
> +EXPORT_SYMBOL_GPL(stmmac_get_phy_intf_sel);

Nothng wrong with your code, this is out of curiosity.

I'm wondering how we are going to support cases like socfpga (and
probably some other) where the PHY_INTF_SEL_xxx doesn't directly
translate to the phy_interface, i.e.  when you have a PCS or other
IP that serialises the MAC interface ?

for socfpga for example, we need to set the PHY_INTF_SEL to GMII_MII
when we want to use SGMII / 1000BaseX, but we do set it to RGMII when
we need to output RGMII.

Do you have a plan in mind for that ? (maybe a .get_phy_intf_sel() ops ?)

Maxime

Re: [PATCH net-next 02/11] net: stmmac: s32: move PHY_INTF_SEL_x definitions out of the way

[Maxime Chevallier]

On 03/11/2025 12:50, Russell King (Oracle) wrote:
> S32's PHY_INTF_SEL_x definitions conflict with those for the dwmac
> cores as they use a different bitmapping. Add a S32 prefix so that
> they are unique.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

Re: [PATCH net-next 03/11] net: stmmac: add phy_intf_sel and ACTPHYIF definitions

[Maxime Chevallier]

On 03/11/2025 12:50, Russell King (Oracle) wrote:
> Add definitions for the active PHY interface found in DMA hardware
> feature register 0, and also used to configure the core in multi-
> interface designs via phy_intf_sel.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/stmicro/stmmac/common.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 27083af54568..7395bbb94aea 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -313,6 +313,16 @@ struct stmmac_safety_stats {
>  #define DMA_HW_FEAT_ACTPHYIF	0x70000000	/* Active/selected PHY iface */
>  #define DEFAULT_DMA_PBL		8
>  
> +/* phy_intf_sel_i and ACTPHYIF encodings */
> +#define PHY_INTF_SEL_GMII_MII	0
> +#define PHY_INTF_SEL_RGMII	1
> +#define PHY_INTF_SEL_SGMII	2
> +#define PHY_INTF_SEL_TBI	3
> +#define PHY_INTF_SEL_RMII	4
> +#define PHY_INTF_SEL_RTBI	5
> +#define PHY_INTF_SEL_SMII	6
> +#define PHY_INTF_SEL_REVMII	7
> +
>  /* MSI defines */
>  #define STMMAC_MSI_VEC_MAX	32
>  
This matches the docs I have :)

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

Re: [PATCH net-next 04/11] net: stmmac: add stmmac_get_phy_intf_sel()

[Russell King (Oracle)]

On Tue, Nov 04, 2025 at 09:34:31AM +0100, Maxime Chevallier wrote:
> > +int stmmac_get_phy_intf_sel(phy_interface_t interface)
> > +{
> > +	int phy_intf_sel = -EINVAL;
> > +
> > +	if (interface == PHY_INTERFACE_MODE_MII ||
> > +	    interface == PHY_INTERFACE_MODE_GMII)
> > +		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
> > +	else if (phy_interface_mode_is_rgmii(interface))
> > +		phy_intf_sel = PHY_INTF_SEL_RGMII;
> > +	else if (interface == PHY_INTERFACE_MODE_SGMII)
> > +		phy_intf_sel = PHY_INTF_SEL_SGMII;
> > +	else if (interface == PHY_INTERFACE_MODE_RMII)
> > +		phy_intf_sel = PHY_INTF_SEL_RMII;
> > +	else if (interface == PHY_INTERFACE_MODE_REVMII)
> > +		phy_intf_sel = PHY_INTF_SEL_REVMII;
> > +
> > +	return phy_intf_sel;
> > +}
> > +EXPORT_SYMBOL_GPL(stmmac_get_phy_intf_sel);
> 
> Nothng wrong with your code, this is out of curiosity.
> 
> I'm wondering how we are going to support cases like socfpga (and
> probably some other) where the PHY_INTF_SEL_xxx doesn't directly
> translate to the phy_interface, i.e.  when you have a PCS or other
> IP that serialises the MAC interface ?

It also doesn't differentiate between MII and GMII. That's fine for
this - this is about producing the configuration for the dwmac's
phy_intf_sel_i signals. It isn't for configuring the glue hardware
for any other parameters such as RGMII delays.
 
> for socfpga for example, we need to set the PHY_INTF_SEL to GMII_MII
> when we want to use SGMII / 1000BaseX, but we do set it to RGMII when
> we need to output RGMII.

From what I remember for socfpga, you use an external PCS that needs
GMII. This function doesn't take account of external PCS, and thus
platform glue that makes use of an external PCS can't implement
.set_phy_intf_sel() yet. As noted, it also doesn't handle TBI (which,
although we have PHY_INTERFACE_MODE_TBI, Synopsys intended this mode
to be used to connect to a SerDes for 1000BASE-X.)

> Do you have a plan in mind for that ? (maybe a .get_phy_intf_sel() ops ?)

Yes, there will need to be a way to override this when an external
PCS is being used. I suspect that all external 1G PCS will use GMII,
thus we can probably work it out in core code.

Note, however, that socfpga doesn't use the phy_intf_sel encoding:

#define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII 0x0
#define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RGMII 0x1
#define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RMII 0x2

#define PHY_INTF_SEL_GMII_MII   0
#define PHY_INTF_SEL_RGMII      1
#define PHY_INTF_SEL_RMII       4

It's close, but it isn't the phy_intf_sel_i[2:0] signal values.

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

Re: [PATCH net-next 04/11] net: stmmac: add stmmac_get_phy_intf_sel()

[Maxime Chevallier]

On 04/11/2025 09:49, Russell King (Oracle) wrote:
> On Tue, Nov 04, 2025 at 09:34:31AM +0100, Maxime Chevallier wrote:
>>> +int stmmac_get_phy_intf_sel(phy_interface_t interface)
>>> +{
>>> +	int phy_intf_sel = -EINVAL;
>>> +
>>> +	if (interface == PHY_INTERFACE_MODE_MII ||
>>> +	    interface == PHY_INTERFACE_MODE_GMII)
>>> +		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
>>> +	else if (phy_interface_mode_is_rgmii(interface))
>>> +		phy_intf_sel = PHY_INTF_SEL_RGMII;
>>> +	else if (interface == PHY_INTERFACE_MODE_SGMII)
>>> +		phy_intf_sel = PHY_INTF_SEL_SGMII;
>>> +	else if (interface == PHY_INTERFACE_MODE_RMII)
>>> +		phy_intf_sel = PHY_INTF_SEL_RMII;
>>> +	else if (interface == PHY_INTERFACE_MODE_REVMII)
>>> +		phy_intf_sel = PHY_INTF_SEL_REVMII;
>>> +
>>> +	return phy_intf_sel;
>>> +}
>>> +EXPORT_SYMBOL_GPL(stmmac_get_phy_intf_sel);
>>
>> Nothng wrong with your code, this is out of curiosity.
>>
>> I'm wondering how we are going to support cases like socfpga (and
>> probably some other) where the PHY_INTF_SEL_xxx doesn't directly
>> translate to the phy_interface, i.e.  when you have a PCS or other
>> IP that serialises the MAC interface ?
> 
> It also doesn't differentiate between MII and GMII. That's fine for
> this - this is about producing the configuration for the dwmac's
> phy_intf_sel_i signals. It isn't for configuring the glue hardware
> for any other parameters such as RGMII delays.
>  
>> for socfpga for example, we need to set the PHY_INTF_SEL to GMII_MII
>> when we want to use SGMII / 1000BaseX, but we do set it to RGMII when
>> we need to output RGMII.
> 
> From what I remember for socfpga, you use an external PCS that needs
> GMII. This function doesn't take account of external PCS, and thus
> platform glue that makes use of an external PCS can't implement
> .set_phy_intf_sel() yet.

Makes sense

> As noted, it also doesn't handle TBI (which,
> although we have PHY_INTERFACE_MODE_TBI, Synopsys intended this mode
> to be used to connect to a SerDes for 1000BASE-X.)

That's fine by me, thanks for the clarifications :)

> 
>> Do you have a plan in mind for that ? (maybe a .get_phy_intf_sel() ops ?)
> 
> Yes, there will need to be a way to override this when an external
> PCS is being used. I suspect that all external 1G PCS will use GMII,
> thus we can probably work it out in core code.
> 
> Note, however, that socfpga doesn't use the phy_intf_sel encoding:
> 
> #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII 0x0
> #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RGMII 0x1
> #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RMII 0x2
> 
> #define PHY_INTF_SEL_GMII_MII   0
> #define PHY_INTF_SEL_RGMII      1
> #define PHY_INTF_SEL_RMII       4
> 
> It's close, but it isn't the phy_intf_sel_i[2:0] signal values.

> 
Yeah :/

I'll give this series a try on dwmac-imx once I get a bit of time then !

thanks,

Maxime

Re: [PATCH net-next 02/11] net: stmmac: s32: move PHY_INTF_SEL_x definitions out of the way

[Jan Petrous]

On Mon, Nov 03, 2025 at 11:50:00AM +0000, Russell King (Oracle) wrote:
> S32's PHY_INTF_SEL_x definitions conflict with those for the dwmac
> cores as they use a different bitmapping. Add a S32 prefix so that
> they are unique.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> index ee095ac13203..2b7ad64bfdf7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c
> @@ -24,10 +24,10 @@
>  #define GMAC_INTF_RATE_125M	125000000	/* 125MHz */
>  
>  /* SoC PHY interface control register */
> -#define PHY_INTF_SEL_MII	0x00
> -#define PHY_INTF_SEL_SGMII	0x01
> -#define PHY_INTF_SEL_RGMII	0x02
> -#define PHY_INTF_SEL_RMII	0x08
> +#define S32_PHY_INTF_SEL_MII	0x00
> +#define S32_PHY_INTF_SEL_SGMII	0x01
> +#define S32_PHY_INTF_SEL_RGMII	0x02
> +#define S32_PHY_INTF_SEL_RMII	0x08
>  
>  struct s32_priv_data {
>  	void __iomem *ioaddr;
> @@ -40,7 +40,7 @@ struct s32_priv_data {
>  
>  static int s32_gmac_write_phy_intf_select(struct s32_priv_data *gmac)
>  {
> -	writel(PHY_INTF_SEL_RGMII, gmac->ctrl_sts);
> +	writel(S32_PHY_INTF_SEL_RGMII, gmac->ctrl_sts);
>  
>  	dev_dbg(gmac->dev, "PHY mode set to %s\n", phy_modes(*gmac->intf_mode));
>  
> -- 
> 2.47.3
> 

Reviewed-by: Jan Petrous (OSS) <jan.petrous@oss.nxp.com>

Thanks.
/Jan

Re: [PATCH net-next 02/11] net: stmmac: s32: move PHY_INTF_SEL_x definitions out of the way

[Russell King (Oracle)]

On Tue, Nov 04, 2025 at 10:37:10AM +0100, Jan Petrous wrote:
> On Mon, Nov 03, 2025 at 11:50:00AM +0000, Russell King (Oracle) wrote:
> >  /* SoC PHY interface control register */
> > -#define PHY_INTF_SEL_MII	0x00
> > -#define PHY_INTF_SEL_SGMII	0x01
> > -#define PHY_INTF_SEL_RGMII	0x02
> > -#define PHY_INTF_SEL_RMII	0x08
> > +#define S32_PHY_INTF_SEL_MII	0x00
> > +#define S32_PHY_INTF_SEL_SGMII	0x01
> > +#define S32_PHY_INTF_SEL_RGMII	0x02
> > +#define S32_PHY_INTF_SEL_RMII	0x08
> 
> Reviewed-by: Jan Petrous (OSS) <jan.petrous@oss.nxp.com>

Thanks. One question: is it possible that bits 3:1 are the dwmac
phy_intf_sel_i inputs, and bit 0 selects an external PCS which
is connected to the dwmac using GMII (and thus would be set bits
3:1 to zero) ?

It's not really relevant as the driver only appears to support
RGMII.

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

Re: [PATCH net-next 02/11] net: stmmac: s32: move PHY_INTF_SEL_x definitions out of the way

[Jan Petrous]

On Tue, Nov 04, 2025 at 09:55:25AM +0000, Russell King (Oracle) wrote:
> On Tue, Nov 04, 2025 at 10:37:10AM +0100, Jan Petrous wrote:
> > On Mon, Nov 03, 2025 at 11:50:00AM +0000, Russell King (Oracle) wrote:
> > >  /* SoC PHY interface control register */
> > > -#define PHY_INTF_SEL_MII	0x00
> > > -#define PHY_INTF_SEL_SGMII	0x01
> > > -#define PHY_INTF_SEL_RGMII	0x02
> > > -#define PHY_INTF_SEL_RMII	0x08
> > > +#define S32_PHY_INTF_SEL_MII	0x00
> > > +#define S32_PHY_INTF_SEL_SGMII	0x01
> > > +#define S32_PHY_INTF_SEL_RGMII	0x02
> > > +#define S32_PHY_INTF_SEL_RMII	0x08
> > 
> > Reviewed-by: Jan Petrous (OSS) <jan.petrous@oss.nxp.com>
> 
> Thanks. One question: is it possible that bits 3:1 are the dwmac
> phy_intf_sel_i inputs, and bit 0 selects an external PCS which
> is connected to the dwmac using GMII (and thus would be set bits
> 3:1 to zero) ?

I guess so, as the S32G3 Reference Manual says regarding
GMAC_0_CTRL_STS register bits the following:

[3-1] PHY_INTF_SEL: PHY Interface Select
      Selects the PHY interface. These values are valid only
      for PHY_MODE=0.

      000b - MII
      001b - RGMII
      100b - RMII

[0-0] PHY_MODE: Select the PHY mode.
      Selects the PHY mode.

      0b - Other PHY modes (for ex. RGMII, RMII, ...)
      1b - SGMII mode

> 
> It's not really relevant as the driver only appears to support
> RGMII.

Yes. The RGMII was the simplest way to upstream review, so
I decided to stick on it.

The SGMII support is ongoing.

/Jan

Re: [PATCH net-next 00/11] net: stmmac: multi-interface stmmac

[patchwork-bot+netdevbpf]

Hello:

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

On Mon, 3 Nov 2025 11:49:35 +0000 you wrote:
> Hi,
> 
> This series adds a callback for platform glue to configure the stmmac
> core interface mode depending on the PHY interface mode that is being
> used. This is currently only called justbefore the dwmac core is reset
> since these signals are latched on reset.
> 
> [...]

Here is the summary with links:
  - [net-next,01/11] net: stmmac: imx: use phylink's interface mode for set_clk_tx_rate()
    https://git.kernel.org/netdev/net-next/c/42190a188fdb
  - [net-next,02/11] net: stmmac: s32: move PHY_INTF_SEL_x definitions out of the way
    https://git.kernel.org/netdev/net-next/c/d8df08b0df02
  - [net-next,03/11] net: stmmac: add phy_intf_sel and ACTPHYIF definitions
    https://git.kernel.org/netdev/net-next/c/38e8c0fb0fc3
  - [net-next,04/11] net: stmmac: add stmmac_get_phy_intf_sel()
    https://git.kernel.org/netdev/net-next/c/2f2a7b907446
  - [net-next,05/11] net: stmmac: add support for configuring the phy_intf_sel inputs
    https://git.kernel.org/netdev/net-next/c/15ade7dbf64f
  - [net-next,06/11] net: stmmac: imx: convert to PHY_INTF_SEL_xxx
    https://git.kernel.org/netdev/net-next/c/8088ca0d88f8
  - [net-next,07/11] net: stmmac: imx: use FIELD_PREP()/FIELD_GET() for PHY_INTF_SEL_x
    https://git.kernel.org/netdev/net-next/c/27db57875c08
  - [net-next,08/11] net: stmmac: imx: use stmmac_get_phy_intf_sel()
    https://git.kernel.org/netdev/net-next/c/cb09d1b9582a
  - [net-next,09/11] net: stmmac: imx: simplify set_intf_mode() implementations
    https://git.kernel.org/netdev/net-next/c/8fc75fe5948d
  - [net-next,10/11] net: stmmac: imx: cleanup arguments for set_intf_mode() method
    https://git.kernel.org/netdev/net-next/c/fb526d0c16c1
  - [net-next,11/11] net: stmmac: imx: use ->set_phy_intf_sel()
    https://git.kernel.org/netdev/net-next/c/da836959c78a

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