[PATCH net-next v2 3/6] net: dsa: lantiq: allow arbitrary MII registers

Daniel Golle posted 6 patches 3 weeks, 3 days ago
There is a newer version of this series
[PATCH net-next v2 3/6] net: dsa: lantiq: allow arbitrary MII registers
Posted by Daniel Golle 3 weeks, 3 days ago
The Lantiq GSWIP and MaxLinear GSW1xx drivers are currently relying on a
hard-coded mapping of MII ports to their respective MII_CFG and MII_PCDU
registers and only allow applying an offset to the port index.

While this is sufficient for the currently supported hardware, the very
similar Intel GSW150 (aka. Lantiq PEB7084) cannot be described using
this arrangement.

Introduce two arrays to specify the MII_CFG and MII_PCDU registers for
each port, replacing the current bitmap used to safeguard MII ports as
well as the port index offset.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2:
 * introduce GSWIP_MAX_PORTS macro

 drivers/net/dsa/lantiq/lantiq_gswip.c        | 30 ++++++++++++++++----
 drivers/net/dsa/lantiq/lantiq_gswip.h        |  6 ++--
 drivers/net/dsa/lantiq/lantiq_gswip_common.c | 27 +++---------------
 drivers/net/dsa/lantiq/mxl-gsw1xx.c          | 30 ++++++++++++++++----
 4 files changed, 56 insertions(+), 37 deletions(-)

diff --git a/drivers/net/dsa/lantiq/lantiq_gswip.c b/drivers/net/dsa/lantiq/lantiq_gswip.c
index b094001a7c805..4a1be6a1df6fe 100644
--- a/drivers/net/dsa/lantiq/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq/lantiq_gswip.c
@@ -463,10 +463,20 @@ static void gswip_shutdown(struct platform_device *pdev)
 }
 
 static const struct gswip_hw_info gswip_xrx200 = {
-	.max_ports = 7,
+	.max_ports = GSWIP_MAX_PORTS,
 	.allowed_cpu_ports = BIT(6),
-	.mii_ports = BIT(0) | BIT(1) | BIT(5),
-	.mii_port_reg_offset = 0,
+	.mii_cfg = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[0] = GSWIP_MII_CFGp(0),
+		[1] = GSWIP_MII_CFGp(1),
+		[5] = GSWIP_MII_CFGp(5),
+	},
+	.mii_pcdu = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[0] = GSWIP_MII_PCDU0,
+		[1] = GSWIP_MII_PCDU1,
+		[5] = GSWIP_MII_PCDU5,
+	},
 	.phylink_get_caps = gswip_xrx200_phylink_get_caps,
 	.pce_microcode = &gswip_pce_microcode,
 	.pce_microcode_size = ARRAY_SIZE(gswip_pce_microcode),
@@ -474,10 +484,18 @@ static const struct gswip_hw_info gswip_xrx200 = {
 };
 
 static const struct gswip_hw_info gswip_xrx300 = {
-	.max_ports = 7,
+	.max_ports = GSWIP_MAX_PORTS,
 	.allowed_cpu_ports = BIT(6),
-	.mii_ports = BIT(0) | BIT(5),
-	.mii_port_reg_offset = 0,
+	.mii_cfg = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[0] = GSWIP_MII_CFGp(0),
+		[5] = GSWIP_MII_CFGp(5),
+	},
+	.mii_pcdu = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[0] = GSWIP_MII_PCDU0,
+		[5] = GSWIP_MII_PCDU5,
+	},
 	.phylink_get_caps = gswip_xrx300_phylink_get_caps,
 	.pce_microcode = &gswip_pce_microcode,
 	.pce_microcode_size = ARRAY_SIZE(gswip_pce_microcode),
diff --git a/drivers/net/dsa/lantiq/lantiq_gswip.h b/drivers/net/dsa/lantiq/lantiq_gswip.h
index 2e0f2afbadbbc..524289db7c213 100644
--- a/drivers/net/dsa/lantiq/lantiq_gswip.h
+++ b/drivers/net/dsa/lantiq/lantiq_gswip.h
@@ -243,6 +243,8 @@
 
 #define GSWIP_VLAN_UNAWARE_PVID	0
 
+#define GSWIP_MAX_PORTS		7
+
 struct gswip_pce_microcode {
 	u16 val_3;
 	u16 val_2;
@@ -253,8 +255,8 @@ struct gswip_pce_microcode {
 struct gswip_hw_info {
 	int max_ports;
 	unsigned int allowed_cpu_ports;
-	unsigned int mii_ports;
-	int mii_port_reg_offset;
+	s16 mii_cfg[GSWIP_MAX_PORTS];
+	s16 mii_pcdu[GSWIP_MAX_PORTS];
 	bool supports_2500m;
 	const struct gswip_pce_microcode (*pce_microcode)[];
 	size_t pce_microcode_size;
diff --git a/drivers/net/dsa/lantiq/lantiq_gswip_common.c b/drivers/net/dsa/lantiq/lantiq_gswip_common.c
index e790f2ef75884..05b28b540661a 100644
--- a/drivers/net/dsa/lantiq/lantiq_gswip_common.c
+++ b/drivers/net/dsa/lantiq/lantiq_gswip_common.c
@@ -118,15 +118,11 @@ static u32 gswip_switch_r_timeout(struct gswip_priv *priv, u32 offset,
 static void gswip_mii_mask_cfg(struct gswip_priv *priv, u32 mask, u32 set,
 			       int port)
 {
-	int reg_port;
-
 	/* MII_CFG register only exists for MII ports */
-	if (!(priv->hw_info->mii_ports & BIT(port)))
+	if (priv->hw_info->mii_cfg[port] == -1)
 		return;
 
-	reg_port = port + priv->hw_info->mii_port_reg_offset;
-
-	regmap_write_bits(priv->mii, GSWIP_MII_CFGp(reg_port), mask,
+	regmap_write_bits(priv->mii, priv->hw_info->mii_cfg[port], mask,
 			  set);
 }
 
@@ -604,28 +600,13 @@ static void gswip_mii_delay_setup(struct gswip_priv *priv, struct dsa_port *dp,
 	u32 tx_delay = GSWIP_MII_PCDU_TXDLY_DEFAULT;
 	u32 rx_delay = GSWIP_MII_PCDU_RXDLY_DEFAULT;
 	struct device_node *port_dn = dp->dn;
-	u16 mii_pcdu_reg;
 
 	/* As MII_PCDU registers only exist for MII ports, silently return
 	 * unless the port is an MII port
 	 */
-	if (!(priv->hw_info->mii_ports & BIT(dp->index)))
+	if (priv->hw_info->mii_pcdu[dp->index] == -1)
 		return;
 
-	switch (dp->index + priv->hw_info->mii_port_reg_offset) {
-	case 0:
-		mii_pcdu_reg = GSWIP_MII_PCDU0;
-		break;
-	case 1:
-		mii_pcdu_reg = GSWIP_MII_PCDU1;
-		break;
-	case 5:
-		mii_pcdu_reg = GSWIP_MII_PCDU5;
-		break;
-	default:
-		return;
-	}
-
 	/* legacy code to set default delays according to the interface mode */
 	switch (interface) {
 	case PHY_INTERFACE_MODE_RGMII_ID:
@@ -646,7 +627,7 @@ static void gswip_mii_delay_setup(struct gswip_priv *priv, struct dsa_port *dp,
 	of_property_read_u32(port_dn, "rx-internal-delay-ps", &rx_delay);
 	of_property_read_u32(port_dn, "tx-internal-delay-ps", &tx_delay);
 
-	regmap_write_bits(priv->mii, mii_pcdu_reg,
+	regmap_write_bits(priv->mii, priv->hw_info->mii_pcdu[dp->index],
 			  GSWIP_MII_PCDU_TXDLY_MASK |
 			  GSWIP_MII_PCDU_RXDLY_MASK,
 			  GSWIP_MII_PCDU_TXDLY(tx_delay) |
diff --git a/drivers/net/dsa/lantiq/mxl-gsw1xx.c b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
index f8ff8a604bf53..b74edb85e6d8e 100644
--- a/drivers/net/dsa/lantiq/mxl-gsw1xx.c
+++ b/drivers/net/dsa/lantiq/mxl-gsw1xx.c
@@ -702,8 +702,14 @@ static void gsw1xx_shutdown(struct mdio_device *mdiodev)
 static const struct gswip_hw_info gsw12x_data = {
 	.max_ports		= GSW1XX_PORTS,
 	.allowed_cpu_ports	= BIT(GSW1XX_MII_PORT) | BIT(GSW1XX_SGMII_PORT),
-	.mii_ports		= BIT(GSW1XX_MII_PORT),
-	.mii_port_reg_offset	= -GSW1XX_MII_PORT,
+	.mii_cfg = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[GSW1XX_MII_PORT] = GSWIP_MII_CFGp(0),
+	},
+	.mii_pcdu = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[GSW1XX_MII_PORT] = GSWIP_MII_PCDU0,
+	},
 	.mac_select_pcs		= gsw1xx_phylink_mac_select_pcs,
 	.phylink_get_caps	= &gsw1xx_phylink_get_caps,
 	.supports_2500m		= true,
@@ -715,8 +721,14 @@ static const struct gswip_hw_info gsw12x_data = {
 static const struct gswip_hw_info gsw140_data = {
 	.max_ports		= GSW1XX_PORTS,
 	.allowed_cpu_ports	= BIT(GSW1XX_MII_PORT) | BIT(GSW1XX_SGMII_PORT),
-	.mii_ports		= BIT(GSW1XX_MII_PORT),
-	.mii_port_reg_offset	= -GSW1XX_MII_PORT,
+	.mii_cfg = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[GSW1XX_MII_PORT] = GSWIP_MII_CFGp(0),
+	},
+	.mii_pcdu = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[GSW1XX_MII_PORT] = GSWIP_MII_PCDU0,
+	},
 	.mac_select_pcs		= gsw1xx_phylink_mac_select_pcs,
 	.phylink_get_caps	= &gsw1xx_phylink_get_caps,
 	.supports_2500m		= true,
@@ -728,8 +740,14 @@ static const struct gswip_hw_info gsw140_data = {
 static const struct gswip_hw_info gsw141_data = {
 	.max_ports		= GSW1XX_PORTS,
 	.allowed_cpu_ports	= BIT(GSW1XX_MII_PORT) | BIT(GSW1XX_SGMII_PORT),
-	.mii_ports		= BIT(GSW1XX_MII_PORT),
-	.mii_port_reg_offset	= -GSW1XX_MII_PORT,
+	.mii_cfg = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[GSW1XX_MII_PORT] = GSWIP_MII_CFGp(0),
+	},
+	.mii_pcdu = {
+		[0 ... GSWIP_MAX_PORTS - 1] = -1,
+		[GSW1XX_MII_PORT] = GSWIP_MII_PCDU0,
+	},
 	.mac_select_pcs		= gsw1xx_phylink_mac_select_pcs,
 	.phylink_get_caps	= gsw1xx_phylink_get_caps,
 	.pce_microcode		= &gsw1xx_pce_microcode,
-- 
2.52.0
Re: [PATCH net-next v2 3/6] net: dsa: lantiq: allow arbitrary MII registers
Posted by Daniel Golle 3 weeks, 3 days ago
On Thu, Jan 15, 2026 at 12:57:07AM +0000, Daniel Golle wrote:
> The Lantiq GSWIP and MaxLinear GSW1xx drivers are currently relying on a
> hard-coded mapping of MII ports to their respective MII_CFG and MII_PCDU
> registers and only allow applying an offset to the port index.
> 
> While this is sufficient for the currently supported hardware, the very
> similar Intel GSW150 (aka. Lantiq PEB7084) cannot be described using
> this arrangement.
> 
> Introduce two arrays to specify the MII_CFG and MII_PCDU registers for
> each port, replacing the current bitmap used to safeguard MII ports as
> well as the port index offset.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> v2:
>  * introduce GSWIP_MAX_PORTS macro
> 
>  drivers/net/dsa/lantiq/lantiq_gswip.c        | 30 ++++++++++++++++----
>  drivers/net/dsa/lantiq/lantiq_gswip.h        |  6 ++--
>  drivers/net/dsa/lantiq/lantiq_gswip_common.c | 27 +++---------------
>  drivers/net/dsa/lantiq/mxl-gsw1xx.c          | 30 ++++++++++++++++----
>  4 files changed, 56 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/net/dsa/lantiq/lantiq_gswip.c b/drivers/net/dsa/lantiq/lantiq_gswip.c
> index b094001a7c805..4a1be6a1df6fe 100644
> --- a/drivers/net/dsa/lantiq/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq/lantiq_gswip.c
> @@ -463,10 +463,20 @@ static void gswip_shutdown(struct platform_device *pdev)
>  }
>  
>  static const struct gswip_hw_info gswip_xrx200 = {
> -	.max_ports = 7,
> +	.max_ports = GSWIP_MAX_PORTS,
>  	.allowed_cpu_ports = BIT(6),
> -	.mii_ports = BIT(0) | BIT(1) | BIT(5),
> -	.mii_port_reg_offset = 0,
> +	.mii_cfg = {
> +		[0 ... GSWIP_MAX_PORTS - 1] = -1,
> +		[0] = GSWIP_MII_CFGp(0),
> +		[1] = GSWIP_MII_CFGp(1),
> +		[5] = GSWIP_MII_CFGp(5),
> +	},

Kernel CI trips with
warning: initialized field overwritten [-Woverride-init]

Would it be ok to enclose the gswip_hw_info initializers with
__diag_push();
__diag_ignore_all("-Woverride-init",
		  "logic to initialize all and then override some is OK");

like it is done in drivers/net/ethernet/renesas/sh_eth.c?

Or should I rather keep the .mii_ports bitmap in addition to the array
to indicate the indexes with valid values?
Re: [PATCH net-next v2 3/6] net: dsa: lantiq: allow arbitrary MII registers
Posted by Vladimir Oltean 3 weeks, 2 days ago
On Thu, Jan 15, 2026 at 01:40:50AM +0000, Daniel Golle wrote:
> On Thu, Jan 15, 2026 at 12:57:07AM +0000, Daniel Golle wrote:
> > The Lantiq GSWIP and MaxLinear GSW1xx drivers are currently relying on a
> > hard-coded mapping of MII ports to their respective MII_CFG and MII_PCDU
> > registers and only allow applying an offset to the port index.
> > 
> > While this is sufficient for the currently supported hardware, the very
> > similar Intel GSW150 (aka. Lantiq PEB7084) cannot be described using
> > this arrangement.
> > 
> > Introduce two arrays to specify the MII_CFG and MII_PCDU registers for
> > each port, replacing the current bitmap used to safeguard MII ports as
> > well as the port index offset.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> > v2:
> >  * introduce GSWIP_MAX_PORTS macro
> > 
> >  drivers/net/dsa/lantiq/lantiq_gswip.c        | 30 ++++++++++++++++----
> >  drivers/net/dsa/lantiq/lantiq_gswip.h        |  6 ++--
> >  drivers/net/dsa/lantiq/lantiq_gswip_common.c | 27 +++---------------
> >  drivers/net/dsa/lantiq/mxl-gsw1xx.c          | 30 ++++++++++++++++----
> >  4 files changed, 56 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/net/dsa/lantiq/lantiq_gswip.c b/drivers/net/dsa/lantiq/lantiq_gswip.c
> > index b094001a7c805..4a1be6a1df6fe 100644
> > --- a/drivers/net/dsa/lantiq/lantiq_gswip.c
> > +++ b/drivers/net/dsa/lantiq/lantiq_gswip.c
> > @@ -463,10 +463,20 @@ static void gswip_shutdown(struct platform_device *pdev)
> >  }
> >  
> >  static const struct gswip_hw_info gswip_xrx200 = {
> > -	.max_ports = 7,
> > +	.max_ports = GSWIP_MAX_PORTS,
> >  	.allowed_cpu_ports = BIT(6),
> > -	.mii_ports = BIT(0) | BIT(1) | BIT(5),
> > -	.mii_port_reg_offset = 0,
> > +	.mii_cfg = {
> > +		[0 ... GSWIP_MAX_PORTS - 1] = -1,
> > +		[0] = GSWIP_MII_CFGp(0),
> > +		[1] = GSWIP_MII_CFGp(1),
> > +		[5] = GSWIP_MII_CFGp(5),
> > +	},
> 
> Kernel CI trips with
> warning: initialized field overwritten [-Woverride-init]
> 
> Would it be ok to enclose the gswip_hw_info initializers with
> __diag_push();
> __diag_ignore_all("-Woverride-init",
> 		  "logic to initialize all and then override some is OK");
> 
> like it is done in drivers/net/ethernet/renesas/sh_eth.c?
> 
> Or should I rather keep the .mii_ports bitmap in addition to the array
> to indicate the indexes with valid values?

This is new syntax for me. I don't have an issue either way. It seems
netdev maintainers did already accept it for another Ethernet driver, so
I suppose you could go with __diag_ignore_all().

FWIW, when I had to handle similar things, I opted for the simpler
"spell them out one by one" approach, see sja1110_regs in
drivers/net/dsa/sja1105/sja1105_spi.c. But that shouldn't necessarily be
the only way.