net: dsa: lantiq_gswip: Add support for MaxLinear GSW1xx switch family

Move all parts specific for the MMIO/SoC driver into a module of its own to prepare for supporting MDIO-connected switch ICs.
Modify gswip_probe() functions by splitting it into a common function gswip_probe_common() which covers allocating, initializing and registering the DSA switch, while keeping transport-specific regmap initialization as well as PHY firmware loading in the new MMIO/SoC-specific gswip_probe() function.

Signed-off-by Daniel Golle <[email protected]>

---
 drivers/net/dsa/lantiq/Kconfig               |    6 +- drivers/net/dsa/lantiq/Makefile              |    1 + drivers/net/dsa/lantiq/lantiq_gswip.c        | 1617 +---------------- drivers/net/dsa/lantiq/lantiq_gswip.h        |    4 + drivers/net/dsa/lantiq/lantiq_gswip_common.c | 1622 ++++++++++++++++++

 5 files changed, 1658 insertions(+), 1592 deletions(-)

 create mode 100644 drivers/net/dsa/lantiq/lantiq_gswip_common.c

Move all parts specific for the MMIO/SoC driver into a module of its own to prepare for supporting MDIO-connected switch ICs.
Modify gswip_probe() functions by splitting it into a common function gswip_probe_common() which covers allocating, initializing and registering the DSA switch, while keeping transport-specific regmap initialization as well as PHY firmware loading in the new MMIO/SoC-specific gswip_probe() function.

Signed-off-by Daniel Golle <[email protected]>

---

Switch API 2.2 or later supports enabling or disabling learning on each port. Implement support for BR_LEARNING bridge flag and announce support for BR_LEARNING on GSWIP 2.2 or later.

Signed-off-by Daniel Golle <[email protected]>

---
v2: initialize supported flags with 0

On standalone switch ICs the link parameters of the CPU port need to be setup just like user ports. The destinction in the driver to not carry out link parameter setup for the CPU port does make sense for in-SoC switches on which the CPU port is internally connected to the SoC's Ethernet MAC.
Set link parameters also for the CPU port unless it is an internal interface.

Signed-off-by Daniel Golle <[email protected]>

---
 drivers/net/dsa/lantiq/lantiq_gswip_common.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/dsa/lantiq/lantiq_gswip_common.c +0 -0

index 092187603dea..0ac87eb23bb5 100644
@@ -1459,7 +1459,7 @@ static void gswip_phylink_mac_link_up(struct phylink_config *config,
  	struct gswip_priv *priv = dp->ds->priv;
  	int port = dp->index;

drivers/net/dsa/lantiq/lantiq_gswip_common.c +1 -1

-	if (!dsa_port_is_cpu(dp)) {
+	if (!dsa_port_is_cpu(dp) || interface != PHY_INTERFACE_MODE_INTERNAL) {

  gswip_port_set_link(priv, port, true); gswip_port_set_speed(priv, port, speed, interface); gswip_port_set_duplex(priv, port, duplex);

On standalone switch ICs the link parameters of the CPU port need to be setup just like user ports. The destinction in the driver to not carry out link parameter setup for the CPU port does make sense for in-SoC switches on which the CPU port is internally connected to the SoC's Ethernet MAC.
Set link parameters also for the CPU port unless it is an internal interface.

Signed-off-by Daniel Golle <[email protected]>

---
 drivers/net/dsa/lantiq/lantiq_gswip_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/dsa/lantiq/lantiq_gswip_common.c +1 -1

index 092187603dea..0ac87eb23bb5 100644
@@ -1459,7 +1459,7 @@ static void gswip_phylink_mac_link_up(struct phylink_config *config,
 	struct gswip_priv *priv = dp->ds->priv;
 	int port = dp->index;
 
-	if (!dsa_port_is_cpu(dp)) {
+	if (!dsa_port_is_cpu(dp) || interface != PHY_INTERFACE_MODE_INTERNAL) {
 		gswip_port_set_link(priv, port, true);
 		gswip_port_set_speed(priv, port, speed, interface);
 		gswip_port_set_duplex(priv, port, duplex);

Signature · 1 line

2.51.1

When adding FDB entries to the MAC bridge table it is needed to set an (undocumented) bit to mark the entry as valid. If this bit isn't set for entries in the MAC bridge table, then those entries won't be considered as valid MAC addresses.

Signed-off-by Daniel Golle <[email protected]>

---
 drivers/net/dsa/lantiq/lantiq_gswip.h        | 1 +
 drivers/net/dsa/lantiq/lantiq_gswip_common.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

drivers/net/dsa/lantiq/lantiq_gswip.h +1 -0

index 56de869fc472..42000954d842 100644
@@ -224,6 +224,7 @@
 #define  GSWIP_TABLE_MAC_BRIDGE_KEY3_FID	GENMASK(5, 0)	/* Filtering identifier */
 #define  GSWIP_TABLE_MAC_BRIDGE_VAL0_PORT	GENMASK(7, 4)	/* Port on learned entries */
 #define  GSWIP_TABLE_MAC_BRIDGE_VAL1_STATIC	BIT(0)		/* Static, non-aging entry */
+#define  GSWIP_TABLE_MAC_BRIDGE_VAL1_VALID	BIT(1)		/* Valid bit */
 
 #define XRX200_GPHY_FW_ALIGN	(16 * 1024)
 

drivers/net/dsa/lantiq/lantiq_gswip_common.c +2 -1

index 0ac87eb23bb5..94b187899db6 100644
@@ -1149,7 +1149,8 @@ static int gswip_port_fdb(struct dsa_switch *ds, int port,
 	mac_bridge.key[2] = addr[1] | (addr[0] << 8);
 	mac_bridge.key[3] = FIELD_PREP(GSWIP_TABLE_MAC_BRIDGE_KEY3_FID, fid);
 	mac_bridge.val[0] = add ? BIT(port) : 0; /* port map */
-	mac_bridge.val[1] = GSWIP_TABLE_MAC_BRIDGE_VAL1_STATIC;
+	mac_bridge.val[1] = add ? (GSWIP_TABLE_MAC_BRIDGE_VAL1_STATIC |
+				   GSWIP_TABLE_MAC_BRIDGE_VAL1_VALID) : 0;
 	mac_bridge.valid = add;
 
 	err = gswip_pce_table_entry_write(priv, &mac_bridge);

Signature · 1 line

2.51.1

Add support for standard tx-internal-delay-ps and rx-internal-delay-ps properties on port nodes to allow fine-tuning of RGMII clock delays.

The GSWIP switch hardware supports delay values in 500 picosecond increments from 0 to 3500 picoseconds, with a default of 2000 picoseconds for both TX and RX delays.

This corresponds to the driver changes that allow adjusting MII delays using Device Tree properties instead of relying solely on the PHY interface mode.

Signed-off-by Daniel Golle <[email protected]>

---
v3:
* redefine ports node so properties are defined actually apply
* RGMII port with 2ps delay is 'rgmii-id' mode

 .../bindings/net/dsa/lantiq,gswip.yaml        | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +4 -2

index f3154b19af78..b0227b80716c 100644
@@ -6,8 +6,29 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Lantiq GSWIP Ethernet switches
 
-allOf:
-  - $ref: dsa.yaml#/$defs/ethernet-ports
+$ref: dsa.yaml#
+
+patternProperties:
+  "^(ethernet-)?ports$":

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +12 -1

+            description:
+              RGMII TX Clock Delay defined in pico seconds.
+              The delay lines adjust the MII clock vs. data timing.
+          rx-internal-delay-ps:
+            enum: [0, 500, 1000, 1500, 2000, 2500, 3000, 3500]
+            default: 2000
+            description:
+              RGMII RX Clock Delay defined in pico seconds.
+              The delay lines adjust the MII clock vs. data timing.
 
 maintainers:
   - Hauke Mehrtens <[email protected]>

@@ -113,8 +134,10 @@ examples:
                     port@0 {
                             reg = <0>;
                             label = "lan3";
-                            phy-mode = "rgmii";
+                            phy-mode = "rgmii-id";
                             phy-handle = <&phy0>;
+                            tx-internal-delay-ps = <2000>;
+                            rx-internal-delay-ps = <2000>;
                     };
 
                     port@1 {

Signature · 1 line

2.51.1

Quoted 34 lines · Daniel Golle

Add support for standard tx-internal-delay-ps and rx-internal-delay-ps properties on port nodes to allow fine-tuning of RGMII clock delays.

The GSWIP switch hardware supports delay values in 500 picosecond increments from 0 to 3500 picoseconds, with a default of 2000 picoseconds for both TX and RX delays.

This corresponds to the driver changes that allow adjusting MII delays using Device Tree properties instead of relying solely on the PHY interface mode.

Signed-off-by Daniel Golle <[email protected]>

---
v3:
* redefine ports node so properties are defined actually apply
* RGMII port with 2ps delay is 'rgmii-id' mode

 .../bindings/net/dsa/lantiq,gswip.yaml        | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +4 -2

index f3154b19af78..b0227b80716c 100644
@@ -6,8 +6,29 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Lantiq GSWIP Ethernet switches
 
-allOf:
-  - $ref: dsa.yaml#/$defs/ethernet-ports
+$ref: dsa.yaml#
+
+patternProperties:
+  "^(ethernet-)?ports$":

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml $schema: http://devicetree.org/meta-schemas/core.yaml# · line 22

Quoted 10 lines

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +10 -0

+    type: object
+    patternProperties:
+      "^(ethernet-)?port@[0-6]$":
+        $ref: dsa-port.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          tx-internal-delay-ps:
+            enum: [0, 500, 1000, 1500, 2000, 2500, 3000, 3500]
+            default: 2000

No. This is confusing and wrong. I looked at the driver implementation code, wanting to note that it has the potential of being a breaking change for device trees without the "tx-internal-delay-ps" and "rx-internal-delay-ps" properties.

But then I saw that the driver implementation is subtly different. "tx-internal-delay-ps" defaults to 2000 only if "rx-internal-delay-ps" is set, and "rx-internal-delay-ps" defaults to 2000 only if "tx-internal-delay-ps" is set.

So when implemented in this way, it won't cause the regressions I was concerned about, but it is misrepresented in the schema.

Why overcomplicate this and just not set a default? Modify the RX clock skew if set, and the TX clock skew if set.

Quoted 55 lines · Vladimir Oltean

Quoted 34 lines · Daniel Golle

Add support for standard tx-internal-delay-ps and rx-internal-delay-ps properties on port nodes to allow fine-tuning of RGMII clock delays.

The GSWIP switch hardware supports delay values in 500 picosecond increments from 0 to 3500 picoseconds, with a default of 2000 picoseconds for both TX and RX delays.

This corresponds to the driver changes that allow adjusting MII delays using Device Tree properties instead of relying solely on the PHY interface mode.

Signed-off-by Daniel Golle <[email protected]>

---
v3:
* redefine ports node so properties are defined actually apply
* RGMII port with 2ps delay is 'rgmii-id' mode

 .../bindings/net/dsa/lantiq,gswip.yaml        | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +4 -2

index f3154b19af78..b0227b80716c 100644
@@ -6,8 +6,29 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Lantiq GSWIP Ethernet switches
 
-allOf:
-  - $ref: dsa.yaml#/$defs/ethernet-ports
+$ref: dsa.yaml#
+
+patternProperties:
+  "^(ethernet-)?ports$":

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml $schema: http://devicetree.org/meta-schemas/core.yaml# · line 22

Quoted 10 lines

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +10 -0

+    type: object
+    patternProperties:
+      "^(ethernet-)?port@[0-6]$":
+        $ref: dsa-port.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          tx-internal-delay-ps:
+            enum: [0, 500, 1000, 1500, 2000, 2500, 3000, 3500]
+            default: 2000

No. This is confusing and wrong. I looked at the driver implementation code, wanting to note that it has the potential of being a breaking change for device trees without the "tx-internal-delay-ps" and "rx-internal-delay-ps" properties.

But then I saw that the driver implementation is subtly different. "tx-internal-delay-ps" defaults to 2000 only if "rx-internal-delay-ps" is set, and "rx-internal-delay-ps" defaults to 2000 only if "tx-internal-delay-ps" is set.

Quoted 5 lines

So when implemented in this way, it won't cause the regressions I was concerned about, but it is misrepresented in the schema.

Why overcomplicate this and just not set a default? Modify the RX clock skew if set, and the TX clock skew if set.

The problem is that before adding support for both *-internal-delay-ps properties the internal delays would be set exclusively based on the interface mode -- and are inverted logic:

```
         switch (state->interface) {
         case PHY_INTERFACE_MODE_RGMII_ID:
                 gswip_mii_mask_pcdu(priv, GSWIP_MII_PCDU_TXDLY_MASK |
                                           GSWIP_MII_PCDU_RXDLY_MASK, 0, port);
                 break;
         case PHY_INTERFACE_MODE_RGMII_RXID:
                 gswip_mii_mask_pcdu(priv, GSWIP_MII_PCDU_RXDLY_MASK, 0, port);
                 break;
         case PHY_INTERFACE_MODE_RGMII_TXID:
                 gswip_mii_mask_pcdu(priv, GSWIP_MII_PCDU_TXDLY_MASK, 0, port);
                 break;
         default:
                 break;
         }
```

As you can see the delays are set to 0 in case of the interface mode being RGMII_ID (and the same for RGMII_RXID and RGMII_TXID respectively).

This is probably the result of the delays being initialized to 2000ps by default, and if the **PHY connected to the switch port** is set to take care of the clk/data delay then the switch port RGMII interface doesn't have to do it.

From my understanding this is a bit awkward as "internal delay" usually means the delay is taken care of by the PHY rather than by discrete parts of the board design. Here, however, it is *never* part of the board design and always handled by either the switch RGMII interface (MAC side) or the connected PHY.

So in order to not break existing board device trees expecting this behavior I've decided to only fall-back to adjust the delay based on the interface mode in case both properties are missing.

Please correct me if that's the wrong thing to do or if my understanding is flawed in any way.

Quoted 98 lines · Daniel Golle

Quoted 55 lines · Vladimir Oltean

Quoted 34 lines · Daniel Golle

Add support for standard tx-internal-delay-ps and rx-internal-delay-ps properties on port nodes to allow fine-tuning of RGMII clock delays.

The GSWIP switch hardware supports delay values in 500 picosecond increments from 0 to 3500 picoseconds, with a default of 2000 picoseconds for both TX and RX delays.

This corresponds to the driver changes that allow adjusting MII delays using Device Tree properties instead of relying solely on the PHY interface mode.

Signed-off-by Daniel Golle <[email protected]>

---
v3:
* redefine ports node so properties are defined actually apply
* RGMII port with 2ps delay is 'rgmii-id' mode

 .../bindings/net/dsa/lantiq,gswip.yaml        | 29 +++++++++++++++++--
 1 file changed, 26 insertions(+), 3 deletions(-)

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +4 -2

index f3154b19af78..b0227b80716c 100644
@@ -6,8 +6,29 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Lantiq GSWIP Ethernet switches
 
-allOf:
-  - $ref: dsa.yaml#/$defs/ethernet-ports
+$ref: dsa.yaml#
+
+patternProperties:
+  "^(ethernet-)?ports$":

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml $schema: http://devicetree.org/meta-schemas/core.yaml# · line 22

Quoted 10 lines

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +10 -0

+    type: object
+    patternProperties:
+      "^(ethernet-)?port@[0-6]$":
+        $ref: dsa-port.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          tx-internal-delay-ps:
+            enum: [0, 500, 1000, 1500, 2000, 2500, 3000, 3500]
+            default: 2000

No. This is confusing and wrong. I looked at the driver implementation code, wanting to note that it has the potential of being a breaking change for device trees without the "tx-internal-delay-ps" and "rx-internal-delay-ps" properties.

But then I saw that the driver implementation is subtly different. "tx-internal-delay-ps" defaults to 2000 only if "rx-internal-delay-ps" is set, and "rx-internal-delay-ps" defaults to 2000 only if "tx-internal-delay-ps" is set.

Quoted 5 lines

So when implemented in this way, it won't cause the regressions I was concerned about, but it is misrepresented in the schema.

Why overcomplicate this and just not set a default? Modify the RX clock skew if set, and the TX clock skew if set.

The problem is that before adding support for both *-internal-delay-ps properties the internal delays would be set exclusively based on the interface mode -- and are inverted logic:

```
         switch (state->interface) {
         case PHY_INTERFACE_MODE_RGMII_ID:
                 gswip_mii_mask_pcdu(priv, GSWIP_MII_PCDU_TXDLY_MASK |
                                           GSWIP_MII_PCDU_RXDLY_MASK, 0, port);
                 break;
         case PHY_INTERFACE_MODE_RGMII_RXID:
                 gswip_mii_mask_pcdu(priv, GSWIP_MII_PCDU_RXDLY_MASK, 0, port);
                 break;
         case PHY_INTERFACE_MODE_RGMII_TXID:
                 gswip_mii_mask_pcdu(priv, GSWIP_MII_PCDU_TXDLY_MASK, 0, port);
                 break;
         default:
                 break;
         }
```

As you can see the delays are set to 0 in case of the interface mode being RGMII_ID (and the same for RGMII_RXID and RGMII_TXID respectively).

This is probably the result of the delays being initialized to 2000ps by default, and if the **PHY connected to the switch port** is set to take care of the clk/data delay then the switch port RGMII interface doesn't have to do it.

From my understanding this is a bit awkward as "internal delay" usually means the delay is taken care of by the PHY rather than by discrete parts of the board design. Here, however, it is *never* part of the board design and always handled by either the switch RGMII interface (MAC side) or the connected PHY.

Quoted 6 lines

So in order to not break existing board device trees expecting this behavior I've decided to only fall-back to adjust the delay based on the interface mode in case both properties are missing.

Please correct me if that's the wrong thing to do or if my understanding is flawed in any way.

Ok, I missed the fact that there's RGMII delay handling outside of gswip_mii_delay_setup() too (a bit bizarre).

So "why overcomplicate this" has a good reason. You have legacy to maintain for xrx200, xrx300, xrx330 - essentially the same situation as documented in sja1105_parse_rgmii_delays(). But no legacy for the newly introduced switches though? I don't see why you'd opt them into this behaviour of applying MAC delays based on phy-mode.

Also, the point still stands that your documented default delay value is incorrect. What happens in lack of one property depends on the presence of the other, and on the phy-mode. I think deleting the default value from the schema is much better than having wrong documentation for it.

Add support for the maxlinear,rmii-refclk-out boolean property on port nodes to configure the RMII reference clock to be an output rather than an input.

This property is only applicable for ports using RMII and allows the switch to provide the reference clock for RMII-connected PHYs instead of requiring an external clock source.

Signed-off-by Daniel Golle <[email protected]>

---
 Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml | 5 +++++

 1 file changed, 5 insertions(+)

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +0 -0

index b0227b80716c..dd3858bad8ca 100644
@@ -29,6 +29,11 @@ patternProperties:

             description:
               RGMII RX Clock Delay defined in pico seconds. The delay lines adjust the MII clock vs. data timing. +          maxlinear,rmii-refclk-out:
+            type: boolean
+            description:
+              Configure the RMII reference clock to be a clock output +              rather than an input. Only applicable for RMII mode.

Read boolean Device Tree property "maxlinear,rmii-refclk-out" and switch the RMII reference clock to be a clock output rather than an input if it is set.

Signed-off-by Daniel Golle <[email protected]>

---
 drivers/net/dsa/lantiq/lantiq_gswip_common.c | 4 ++++

 1 file changed, 4 insertions(+)

drivers/net/dsa/lantiq/lantiq_gswip_common.c +0 -0

index 60a83093cd10..bf38ecc13f76 100644
@@ -1442,6 +1442,10 @@ static void gswip_phylink_mac_config(struct phylink_config *config,
  		return;
  	}

drivers/net/dsa/lantiq/lantiq_gswip_common.c +4 -0

+	if (of_property_read_bool(dp->dn, "maxlinear,rmii-refclk-out") &&
+	    !(miicfg & GSWIP_MII_CFG_MODE_RGMII))
+		miicfg |= GSWIP_MII_CFG_RMII_CLK;
+

Read boolean Device Tree property "maxlinear,rmii-refclk-out" and switch the RMII reference clock to be a clock output rather than an input if it is set.

Signed-off-by Daniel Golle <[email protected]>

---
 drivers/net/dsa/lantiq/lantiq_gswip_common.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/net/dsa/lantiq/lantiq_gswip_common.c +0 -0

index 60a83093cd10..bf38ecc13f76 100644

drivers/net/dsa/lantiq/lantiq_gswip_common.c +0 -0

 	gswip_mii_mask_cfg(priv,
 			   GSWIP_MII_CFG_MODE_MASK | GSWIP_MII_CFG_RMII_CLK |
 			   GSWIP_MII_CFG_RGMII_IBS | GSWIP_MII_CFG_LDCLKDIS,

Signature · 1 line

2.51.1

Quoted 17 lines · Daniel Golle

Read boolean Device Tree property "maxlinear,rmii-refclk-out" and switch the RMII reference clock to be a clock output rather than an input if it is set.

Signed-off-by Daniel Golle <[email protected]>

---
 drivers/net/dsa/lantiq/lantiq_gswip_common.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/net/dsa/lantiq/lantiq_gswip_common.c +0 -0

index 60a83093cd10..bf38ecc13f76 100644
@@ -1442,6 +1442,10 @@ static void gswip_phylink_mac_config(struct phylink_config *config,
 		return;
 	}
 

drivers/net/dsa/lantiq/lantiq_gswip_common.c static void gswip_phylink_mac_config(struct phylink_config *config, · line 1448

Quoted 4 lines

drivers/net/dsa/lantiq/lantiq_gswip_common.c +4 -0

+	if (of_property_read_bool(dp->dn, "maxlinear,rmii-refclk-out") &&
+	    !(miicfg & GSWIP_MII_CFG_MODE_RGMII))
+		miicfg |= GSWIP_MII_CFG_RMII_CLK;
+

What did you mean with the !(miicfg & GSWIP_MII_CFG_MODE_RGMII) test? If the schema says "Only applicable for RMII mode.", what's the purpose of this extra condition? For example, GSWIP_MII_CFG_MODE_GMII also has the "GSWIP_MII_CFG_MODE_RGMII" bit (0x4) unset. Does this have any significance?

Quoted 18 lines · Vladimir Oltean

Quoted 17 lines · Daniel Golle

Read boolean Device Tree property "maxlinear,rmii-refclk-out" and switch the RMII reference clock to be a clock output rather than an input if it is set.

Signed-off-by Daniel Golle <[email protected]>

---
 drivers/net/dsa/lantiq/lantiq_gswip_common.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/net/dsa/lantiq/lantiq_gswip_common.c +0 -0

index 60a83093cd10..bf38ecc13f76 100644
@@ -1442,6 +1442,10 @@ static void gswip_phylink_mac_config(struct phylink_config *config,
 		return;
 	}
 

Quoted 9 lines

drivers/net/dsa/lantiq/lantiq_gswip_common.c static void gswip_phylink_mac_config(struct phylink_config *config, · line 1448

Quoted 4 lines

drivers/net/dsa/lantiq/lantiq_gswip_common.c +4 -0

+	if (of_property_read_bool(dp->dn, "maxlinear,rmii-refclk-out") &&
+	    !(miicfg & GSWIP_MII_CFG_MODE_RGMII))
+		miicfg |= GSWIP_MII_CFG_RMII_CLK;
+

What did you mean with the !(miicfg & GSWIP_MII_CFG_MODE_RGMII) test? If the schema says "Only applicable for RMII mode.", what's the purpose of this extra condition? For example, GSWIP_MII_CFG_MODE_GMII also has the "GSWIP_MII_CFG_MODE_RGMII" bit (0x4) unset. Does this have any significance?

You are right, probably the best would be to test (if at all) that (miicfg == GSWIP_MII_CFG_MODE_RMIIM || miicfg == GSWIP_MII_CFG_MODE_RMIIP) and only in this case allow setting the GSWIP_MII_CFG_RMII_CLK bit.

I forgot that there is older hardware which supports "full" MII, and MII MAC as well as MII PHY modes also shouldn't allow to set the GSWIP_MII_CFG_RMII_CLK bit to not end up with undefined behavior.

Extend the Lantiq GSWIP device tree binding to also cover MaxLinear GSW1xx switches which are based on the same hardware IP but connected via MDIO instead of being memory-mapped.

Add compatible strings for MaxLinear GSW120, GSW125, GSW140, GSW141, and GSW145 switches and adjust the schema to handle the different connection methods with conditional properties.

Add MaxLinear GSW125 example showing MDIO-connected configuration.

Signed-off-by Daniel Golle <[email protected]>

---
v3:
* add maxlinear,rx-inverted and maxlinear,tx-inverted properties

v2:
* remove git conflict left-overs which somehow creeped in
* indent example with 4 spaces instead of tabs

 .../bindings/net/dsa/lantiq,gswip.yaml        | 275 +++++++++++++-----
 1 file changed, 202 insertions(+), 73 deletions(-)

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +6 -1

index dd3858bad8ca..1148fdd0b6bc 100644
@@ -4,7 +4,12 @@
 $id: http://devicetree.org/schemas/net/dsa/lantiq,gswip.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Lantiq GSWIP Ethernet switches
+title: Lantiq GSWIP and MaxLinear GSW1xx Ethernet switches
+
+description:
+  Lantiq GSWIP and MaxLinear GSW1xx switches share the same hardware IP.
+  Lantiq switches are embedded in SoCs and accessed via memory-mapped I/O,
+  while MaxLinear switches are standalone ICs connected via MDIO.
 
 $ref: dsa.yaml#
 

@@ -34,6 +39,108 @@ patternProperties:
             description:

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +81 -0

+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - lantiq,xrx200-gswip
+              - lantiq,xrx300-gswip
+              - lantiq,xrx330-gswip
+    then:
+      properties:
+        reg:
+          minItems: 3
+          maxItems: 3
+          description: Memory-mapped register regions (switch, mdio, mii)
+        reg-names:
+          items:
+            - const: switch
+            - const: mdio
+            - const: mii
+        mdio:
+          $ref: /schemas/net/mdio.yaml#
+          unevaluatedProperties: false
+
+          properties:
+            compatible:
+              const: lantiq,xrx200-mdio
+
+          required:
+            - compatible
+        gphy-fw:
+          type: object
+          properties:
+            '#address-cells':
+              const: 1
+
+            '#size-cells':
+              const: 0
+
+            compatible:
+              items:
+                - enum:
+                    - lantiq,xrx200-gphy-fw
+                    - lantiq,xrx300-gphy-fw
+                    - lantiq,xrx330-gphy-fw
+                - const: lantiq,gphy-fw
+
+            lantiq,rcu:
+              $ref: /schemas/types.yaml#/definitions/phandle
+              description: phandle to the RCU syscon
+
+          patternProperties:
+            "^gphy@[0-9a-f]{1,2}$":
+              type: object
+
+              additionalProperties: false
+
+              properties:
+                reg:
+                  minimum: 0
+                  maximum: 255
+                  description:
+                    Offset of the GPHY firmware register in the RCU register
+                    range
+
+                resets:
+                  items:
+                    - description: GPHY reset line
+
+                reset-names:
+                  items:
+                    - const: gphy
+
+              required:
+                - reg
+
+          required:
+            - compatible
+            - lantiq,rcu
+

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +97 -72

+        mdio:
+          $ref: /schemas/net/mdio.yaml#
+          unevaluatedProperties: false
 
 maintainers:
   - Hauke Mehrtens <[email protected]>

@@ -44,78 +151,11 @@ properties:
       - lantiq,xrx200-gswip
       - lantiq,xrx300-gswip
       - lantiq,xrx330-gswip
-
-  reg:
-    minItems: 3
-    maxItems: 3
-
-  reg-names:
-    items:
-      - const: switch
-      - const: mdio
-      - const: mii
-
-  mdio:
-    $ref: /schemas/net/mdio.yaml#
-    unevaluatedProperties: false
-
-    properties:
-      compatible:
-        const: lantiq,xrx200-mdio
-
-    required:
-      - compatible
-
-  gphy-fw:
-    type: object
-    properties:
-      '#address-cells':
-        const: 1
-
-      '#size-cells':
-        const: 0
-
-      compatible:
-        items:
-          - enum:
-              - lantiq,xrx200-gphy-fw
-              - lantiq,xrx300-gphy-fw
-              - lantiq,xrx330-gphy-fw
-          - const: lantiq,gphy-fw
-
-      lantiq,rcu:
-        $ref: /schemas/types.yaml#/definitions/phandle
-        description: phandle to the RCU syscon
-
-    patternProperties:
-      "^gphy@[0-9a-f]{1,2}$":
-        type: object
-
-        additionalProperties: false
-
-        properties:
-          reg:
-            minimum: 0
-            maximum: 255
-            description:
-              Offset of the GPHY firmware register in the RCU register range
-
-          resets:
-            items:
-              - description: GPHY reset line
-
-          reset-names:
-            items:
-              - const: gphy
-
-        required:
-          - reg
-
-    required:
-      - compatible
-      - lantiq,rcu
-
-    additionalProperties: false
+      - maxlinear,gsw120
+      - maxlinear,gsw125
+      - maxlinear,gsw140
+      - maxlinear,gsw141
+      - maxlinear,gsw145
 
 required:
   - compatible

@@ -130,6 +170,7 @@ examples:
             reg = <0xe108000 0x3100>,  /* switch */
                   <0xe10b100 0xd8>,    /* mdio */
                   <0xe10b1d8 0x130>;   /* mii */
+            reg-names = "switch", "mdio", "mii";
             dsa,member = <0 0>;
 
             ports {

@@ -228,3 +269,91 @@ examples:
                     };
             };
     };
+
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        switch@1f {
+            compatible = "maxlinear,gsw125";
+            reg = <0x1f>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    label = "lan0";
+                    phy-handle = <&switchphy0>;
+                    phy-mode = "internal";
+                };
+
+                port@1 {
+                    reg = <1>;
+                    label = "lan1";
+                    phy-handle = <&switchphy1>;
+                    phy-mode = "internal";
+                };
+
+                port@4 {
+                    reg = <4>;
+                    label = "wan";
+                    phy-mode = "1000base-x";
+                    maxlinear,rx-inverted;
+                    managed = "in-band-status";
+                };
+
+                port@5 {
+                    reg = <5>;
+                    phy-mode = "rgmii-id";
+                    tx-internal-delay-ps = <2000>;
+                    rx-internal-delay-ps = <2000>;
+                    ethernet = <&eth0>;
+
+                    fixed-link {
+                        speed = <1000>;
+                        full-duplex;
+                    };
+                };
+            };
+
+            mdio {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                switchphy0: switchphy@0 {
+                    reg = <0>;
+
+                    leds {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+
+                        led@0 {
+                            reg = <0>;
+                            color = <LED_COLOR_ID_GREEN>;
+                            function = LED_FUNCTION_LAN;
+                        };
+                    };
+                };
+
+                switchphy1: switchphy@1 {
+                    reg = <1>;
+
+                    leds {
+                        #address-cells = <1>;
+                        #size-cells = <0>;
+
+                        led@0 {
+                            reg = <0>;
+                            color = <LED_COLOR_ID_GREEN>;
+                            function = LED_FUNCTION_LAN;
+                        };
+                    };
+                };
+            };
+        };
+    };

Signature · 1 line

2.51.1

Quoted 40 lines · Daniel Golle

Extend the Lantiq GSWIP device tree binding to also cover MaxLinear GSW1xx switches which are based on the same hardware IP but connected via MDIO instead of being memory-mapped.

Add compatible strings for MaxLinear GSW120, GSW125, GSW140, GSW141, and GSW145 switches and adjust the schema to handle the different connection methods with conditional properties.

Add MaxLinear GSW125 example showing MDIO-connected configuration.

Signed-off-by Daniel Golle <[email protected]>

---
v3:
* add maxlinear,rx-inverted and maxlinear,tx-inverted properties

v2:
* remove git conflict left-overs which somehow creeped in
* indent example with 4 spaces instead of tabs

 .../bindings/net/dsa/lantiq,gswip.yaml        | 275 +++++++++++++-----
 1 file changed, 202 insertions(+), 73 deletions(-)

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +6 -1

index dd3858bad8ca..1148fdd0b6bc 100644
@@ -4,7 +4,12 @@
 $id: http://devicetree.org/schemas/net/dsa/lantiq,gswip.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Lantiq GSWIP Ethernet switches
+title: Lantiq GSWIP and MaxLinear GSW1xx Ethernet switches
+
+description:
+  Lantiq GSWIP and MaxLinear GSW1xx switches share the same hardware IP.
+  Lantiq switches are embedded in SoCs and accessed via memory-mapped I/O,
+  while MaxLinear switches are standalone ICs connected via MDIO.
 
 $ref: dsa.yaml#
 

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml patternProperties: · line 49

Quoted 12 lines

Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml +8 -0

@@ -34,6 +39,108 @@ patternProperties:
             description:
               Configure the RMII reference clock to be a clock output
               rather than an input. Only applicable for RMII mode.
+          maxlinear,rx-inverted:
+            type: boolean
+            description:
+              Enable RX polarity inversion for SerDes port.
+          maxlinear,tx-inverted:
+            type: boolean
+            description:
+              Enable TX polarity inversion for SerDes port.

How urgently do you need these two properties? They are truly general, not vendor-specific, and while I wanted to add such support to the Synopsys XPCS, I started working on some generic variants.

There's some cleanup and consolidation to do. "st,pcie-tx-pol-inv" and "st,sata-tx-pol-inv" are defined in .txt bindings but not implemented. Then we have "st,px_rx_pol_inv" and "mediatek,pnswap" which would also need deprecating and converted to the new formats.

Where I left things was that I haven't decided if there's any value in defining the polarity per SerDes protocol (like Documentation/devicetree/bindings/phy/transmit-amplitude.yaml) or if a global value is fine. I.e. if the polarity is inverted for SATA, it's normal for PCIe, or something like that. The existence of the independent "st,pcie-tx-pol-inv" and "st,sata-tx-pol-inv" properties would suggest yes, but the lack of an implementation casts some doubt on that.

Anyway, I do have some prototype patches that add something like this:

    phy: phy {
      #phy-cells = <1>;
      tx-p2p-microvolt = <915000>, <1100000>, <1200000>;
      tx-p2p-microvolt-names = "2500base-x", "usb-hs", "usb-ss";

      /* RX polarity is inverted for usb-hs, normal for usb-ss */
      rx-polarity = <PHY_POL_INVERT>, <PHY_POL_NORMAL>;
      rx-polarity-names = "usb-hs", "usb-ss";

      /* TX polarity is normal for all modes */
      tx-polarity = <PHY_POL_NORMAL>;
      tx-polarity-names = "default";
    };

and a new drivers/phy/phy-common-props.c file (yes, outside of netdev) with two exported API functions:

int phy_get_rx_polarity(struct fwnode_handle *fwnode, const char *mode_name); int phy_get_tx_polarity(struct fwnode_handle *fwnode, const char *mode_name);

If you can split this up from the rest of the MDIO discrete switch introduction series, I can accelerate work on these common properties in the following weeks.

Add support for a new DSA tagging protocol driver for the MaxLinear GSW1xx switch family. The GSW1xx switches use a proprietary 8-byte special tag inserted between the source MAC address and the EtherType field to indicate the source and destination ports for frames traversing the CPU port.

Implement the tag handling logic to insert the special tag on transmit and parse it on receive.

Signed-off-by Daniel Golle <[email protected]>

---
since RFC:
 * use dsa etype header macros instead of open coding them
 * maintain alphabetic order in Kconfig and Makefile

 MAINTAINERS              |   3 +-
 include/net/dsa.h        |   2 +
 net/dsa/Kconfig          |   8 +++
 net/dsa/Makefile         |   1 +
 net/dsa/tag_mxl-gsw1xx.c | 141 +++++++++++++++++++++++++++++++++++++++

 5 files changed, 154 insertions(+), 1 deletion(-)

Add support for a new DSA tagging protocol driver for the MaxLinear GSW1xx switch family. The GSW1xx switches use a proprietary 8-byte special tag inserted between the source MAC address and the EtherType field to indicate the source and destination ports for frames traversing the CPU port.

Implement the tag handling logic to insert the special tag on transmit and parse it on receive.

Signed-off-by Daniel Golle <[email protected]>

---
since RFC:
* use dsa etype header macros instead of open coding them
* maintain alphabetic order in Kconfig and Makefile

 MAINTAINERS              |   3 +-
 include/net/dsa.h        |   2 +
 net/dsa/Kconfig          |   8 +++
 net/dsa/Makefile         |   1 +
 net/dsa/tag_mxl-gsw1xx.c | 141 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 154 insertions(+), 1 deletion(-)
 create mode 100644 net/dsa/tag_mxl-gsw1xx.c

MAINTAINERS +2 -1

index d652f4f27756..4ddff0b0a547 100644
@@ -14038,7 +14038,7 @@ F:	tools/testing/selftests/landlock/
 K:	landlock
 K:	LANDLOCK
 
-LANTIQ / INTEL Ethernet drivers
+LANTIQ / MAXLINEAR / INTEL Ethernet DSA drivers
 M:	Hauke Mehrtens <[email protected]>
 L:	[email protected]
 S:	Maintained

@@ -14046,6 +14046,7 @@ F:	Documentation/devicetree/bindings/net/dsa/lantiq,gswip.yaml
 F:	drivers/net/dsa/lantiq/*
 F:	drivers/net/ethernet/lantiq_xrx200.c
 F:	net/dsa/tag_gswip.c
+F:	net/dsa/tag_mxl-gsw1xx.c
 
 LANTIQ MIPS ARCHITECTURE
 M:	John Crispin <[email protected]>

include/net/dsa.h +2 -0

index 67762fdaf3c7..2df2e2ead9a8 100644
@@ -56,6 +56,7 @@ struct tc_action;
 #define DSA_TAG_PROTO_VSC73XX_8021Q_VALUE	28
 #define DSA_TAG_PROTO_BRCM_LEGACY_FCS_VALUE	29
 #define DSA_TAG_PROTO_YT921X_VALUE		30
+#define DSA_TAG_PROTO_MXL_GSW1XX_VALUE		31
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,

@@ -89,6 +90,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_LAN937X		= DSA_TAG_PROTO_LAN937X_VALUE,
 	DSA_TAG_PROTO_VSC73XX_8021Q	= DSA_TAG_PROTO_VSC73XX_8021Q_VALUE,
 	DSA_TAG_PROTO_YT921X		= DSA_TAG_PROTO_YT921X_VALUE,
+	DSA_TAG_PROTO_MXL_GSW1XX	= DSA_TAG_PROTO_MXL_GSW1XX_VALUE,
 };
 
 struct dsa_switch;

net/dsa/Kconfig +8 -0

index 6b94028b1fcc..f86b30742122 100644
@@ -104,6 +104,14 @@ config NET_DSA_TAG_MTK
 	  Say Y or M if you want to enable support for tagging frames for
 	  Mediatek switches.
 
+config NET_DSA_TAG_MXL_GSW1XX
+	tristate "Tag driver for MaxLinear GSW1xx switches"
+	help
+	  The GSW1xx family of switches supports an 8-byte special tag which
+	  can be used on the CPU port of the switch.
+	  Say Y or M if you want to enable support for tagging frames for
+	  MaxLinear GSW1xx switches.
+
 config NET_DSA_TAG_KSZ
 	tristate "Tag driver for Microchip 8795/937x/9477/9893 families of switches"
 	help

net/dsa/Makefile +1 -0

index 4b011a1d5c87..42d173f5a701 100644
@@ -28,6 +28,7 @@ obj-$(CONFIG_NET_DSA_TAG_HELLCREEK) += tag_hellcreek.o
 obj-$(CONFIG_NET_DSA_TAG_KSZ) += tag_ksz.o
 obj-$(CONFIG_NET_DSA_TAG_LAN9303) += tag_lan9303.o
 obj-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
+obj-$(CONFIG_NET_DSA_TAG_MXL_GSW1XX) += tag_mxl-gsw1xx.o
 obj-$(CONFIG_NET_DSA_TAG_NONE) += tag_none.o
 obj-$(CONFIG_NET_DSA_TAG_OCELOT) += tag_ocelot.o
 obj-$(CONFIG_NET_DSA_TAG_OCELOT_8021Q) += tag_ocelot_8021q.o

net/dsa/tag_mxl-gsw1xx.c new file +28 -0

new file mode 100644
index 000000000000..9efec6deb494
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * DSA driver Special Tag support for MaxLinear GSW1xx switch chips
+ *
+ * Copyright (C) 2025 Daniel Golle <[email protected]>
+ * Copyright (C) 2023 - 2024 MaxLinear Inc.
+ */
+
+#include <linux/bitops.h>
+#include <linux/etherdevice.h>
+#include <linux/skbuff.h>
+#include <net/dsa.h>
+
+#include "tag.h"
+
+/* To define the outgoing port and to discover the incoming port a special
+ * tag is used by the GSW1xx.
+ *
+ *       Dest MAC       Src MAC    special TAG        EtherType
+ * ...| 1 2 3 4 5 6 | 1 2 3 4 5 6 | 1 2 3 4 5 6 7 8 | 1 2 |...
+ *                                |<--------------->|
+ */
+
+#define GSW1XX_TAG_NAME		"gsw1xx"
+
+/* special tag in TX path header */
+#define GSW1XX_TX_HEADER_LEN	8
+

net/dsa/tag_mxl-gsw1xx.c new file +16 -0

+
+/* special tag in RX path header */
+/* Byte 4 */
+#define GSW1XX_RX_PORT_MAP_LOW_SHIFT	0
+#define GSW1XX_RX_PORT_MAP_LOW_MASK	GENMASK(7, 0)
+
+/* Byte 5 */
+#define GSW1XX_RX_PORT_MAP_HIGH_SHIFT	0
+#define GSW1XX_RX_PORT_MAP_HIGH_MASK	GENMASK(7, 0)
+
+static struct sk_buff *gsw1xx_tag_xmit(struct sk_buff *skb,
+				       struct net_device *dev)
+{
+	struct dsa_port *dp = dsa_user_to_port(dev);
+	u8 *gsw1xx_tag;
+

net/dsa/tag_mxl-gsw1xx.c new file +20 -0

+	gsw1xx_tag[2] = GSW1XX_TX_PORT_MAP_EN | GSW1XX_TX_LRN_DIS;
+	gsw1xx_tag[3] = BIT(dp->index + GSW1XX_TX_PORT_MAP_LOW_SHIFT) &
+			GSW1XX_TX_PORT_MAP_LOW_MASK;
+	gsw1xx_tag[4] = 0;
+	gsw1xx_tag[5] = 0;
+	gsw1xx_tag[6] = 0;
+	gsw1xx_tag[7] = 0;
+
+	return skb;
+}
+
+static struct sk_buff *gsw1xx_tag_rcv(struct sk_buff *skb,
+				      struct net_device *dev)
+{
+	int port;
+	u8 *gsw1xx_tag;
+
+	if (unlikely(!pskb_may_pull(skb, GSW1XX_RX_HEADER_LEN))) {
+		dev_warn_ratelimited(&dev->dev, "Dropping packet, cannot pull SKB\n");
+		return NULL;

net/dsa/tag_mxl-gsw1xx.c new file +10 -0

+		return NULL;
+	}
+
+	/* Get source port information */
+	port = (gsw1xx_tag[2] & GSW1XX_RX_PORT_MAP_LOW_MASK) >> GSW1XX_RX_PORT_MAP_LOW_SHIFT;
+	skb->dev = dsa_conduit_find_user(dev, 0, port);
+	if (!skb->dev) {
+		dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid source port\n");
+		dev_warn_ratelimited(&dev->dev,
+				     "Tag: 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x\n",

net/dsa/tag_mxl-gsw1xx.c new file +17 -0

+
+	return skb;
+}
+
+static const struct dsa_device_ops gsw1xx_netdev_ops = {
+	.name = GSW1XX_TAG_NAME,
+	.proto	= DSA_TAG_PROTO_MXL_GSW1XX,
+	.xmit = gsw1xx_tag_xmit,
+	.rcv = gsw1xx_tag_rcv,
+	.needed_headroom = GSW1XX_RX_HEADER_LEN,
+};
+
+MODULE_DESCRIPTION("DSA tag driver for MaxLinear GSW1xx 8 byte protocol");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_MXL_GSW1XX, GSW1XX_TAG_NAME);
+
+module_dsa_tag_driver(gsw1xx_netdev_ops);

Signature · 1 line

2.51.1

Quoted 6 lines · Daniel Golle

Add support for a new DSA tagging protocol driver for the MaxLinear GSW1xx switch family. The GSW1xx switches use a proprietary 8-byte special tag inserted between the source MAC address and the EtherType field to indicate the source and destination ports for frames traversing the CPU port.

net/dsa/tag_mxl-gsw1xx.c line 38

Quoted 8 lines

Implement the tag handling logic to insert the special tag on transmit and parse it on receive.
[...]

net/dsa/tag_mxl-gsw1xx.c new file +2 -0

[...]
+#define GSW1XX_TX_CLASS_SHIFT		0
+#define GSW1XX_TX_CLASS_MASK		GENMASK(3, 0)

Using FIELD_PREP() would eliminate these _SHIFT definitions and _MASK would also go away from the macro names.

net/dsa/tag_mxl-gsw1xx.c line 48

Quoted 11 lines

net/dsa/tag_mxl-gsw1xx.c new file +10 -0

+
+/* Byte 3 */
+#define GSW1XX_TX_PORT_MAP_LOW_SHIFT	0
+#define GSW1XX_TX_PORT_MAP_LOW_MASK	GENMASK(7, 0)
+
+/* Byte 4 */
+#define GSW1XX_TX_PORT_MAP_HIGH_SHIFT	0
+#define GSW1XX_TX_PORT_MAP_HIGH_MASK	GENMASK(7, 0)
+
+#define GSW1XX_RX_HEADER_LEN		8

Usually you use two separate macros when the lengths are not equal, and you set .needed_headroom to the largest value.

Quoted 10 lines

[...]

net/dsa/tag_mxl-gsw1xx.c new file +9 -0

+	u8 *gsw1xx_tag;
+
+	/* provide additional space 'GSW1XX_TX_HEADER_LEN' bytes */
+	skb_push(skb, GSW1XX_TX_HEADER_LEN);
+
+	/* add space between MAC address and Ethertype */
+	dsa_alloc_etype_header(skb, GSW1XX_TX_HEADER_LEN);
+
+	/* special tag ingress */

net/dsa/tag_mxl-gsw1xx.c line 74

Quoted 3 lines

net/dsa/tag_mxl-gsw1xx.c new file +3 -0

+	gsw1xx_tag = dsa_etype_header_pos_tx(skb);
+	gsw1xx_tag[0] = 0x88;
+	gsw1xx_tag[1] = 0xc3;

Could you write this as a u16 pointer, to make it obvious to everyone it's an EtherType, and define the EtherType constant in include/uapi/linux/if_ether.h, to make it a bit more visible that it's in use?

net/dsa/tag_mxl-gsw1xx.c line 104

Quoted 7 lines

[...]

net/dsa/tag_mxl-gsw1xx.c new file +6 -0

+	if (gsw1xx_tag[0] != 0x88 && gsw1xx_tag[1] != 0xc3) {
+		dev_warn_ratelimited(&dev->dev, "Dropping packet due to invalid special tag\n");
+		dev_warn_ratelimited(&dev->dev,
+				     "Tag: 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x, 0x%x\n",
+				     gsw1xx_tag[0], gsw1xx_tag[1], gsw1xx_tag[2], gsw1xx_tag[3],
+				     gsw1xx_tag[4], gsw1xx_tag[5], gsw1xx_tag[6], gsw1xx_tag[7]);

I think you could print the tag with %*ph, according to
https://elixir.bootlin.com/linux/v6.17.5/source/lib/vsprintf.c#L2453
(needs testing)

Quoted 4 lines

[...]

net/dsa/tag_mxl-gsw1xx.c new file +3 -0

+	/* remove the GSW1xx special tag between MAC addresses and the current
+	 * ethertype field.
+	 */

net/dsa/tag_mxl-gsw1xx.c line 124

Quoted 2 lines

net/dsa/tag_mxl-gsw1xx.c new file +2 -0

+	skb_pull_rcsum(skb, GSW1XX_RX_HEADER_LEN);
+	dsa_strip_etype_header(skb, GSW1XX_RX_HEADER_LEN);

You're not setting skb->offload_fwd_mark but you implement port_bridge_join() so you offload L2 switching. If a packet gets flooded from port A to the CPU and also to port B, don't you see that the software bridge also creates a packet copy that it sends to port B a second time?

Add driver for the MaxLinear GSW1xx family of Ethernet switch ICs which are based on the same IP as the Lantiq/Intel GSWIP found in the Lantiq VR9 and Intel GRX MIPS router SoCs. The main difference is that instead of using memory-mapped I/O to communicate with the host CPU these ICs are connected via MDIO (or SPI, which isn't supported by this driver). Implement the regmap API to access the switch registers over MDIO to allow reusing lantiq_gswip_common for all core functionality.

The GSW1xx also comes with a SerDes port capable of 1000Base-X, SGMII and 2500Base-X, which can either be used to connect an external PHY or SFP cage, or as the CPU port. Support for the SerDes interface is implemented in this driver using the phylink_pcs interface.

drivers/net/dsa/lantiq/mxl-gsw1xx.c new file +23 -0

+
+static int gsw1xx_pcs_phy_xaui_write(struct gsw1xx_priv *priv, u16 addr,
+				     u16 data)
+{
+	int ret, val;
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_D, data);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_A, addr);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_C,
+			   GSW1XX_SGMII_PHY_WRITE |
+			   GSW1XX_SGMII_PHY_RESET_N);
+	if (ret < 0)
+		return ret;
+
+	return regmap_read_poll_timeout(priv->sgmii, GSW1XX_SGMII_PHY_C,
+					val, val & GSW1XX_SGMII_PHY_STATUS,
+					1000, 100000);

drivers/net/dsa/lantiq/mxl-gsw1xx.c new file +182 -0

+	struct dsa_port *dp = dsa_to_port(priv->gswip.ds,
+					  GSW1XX_SGMII_PORT);
+	u16 txaneg, anegctl, val, nco_ctrl;
+	bool reconf = false;
+	int ret;
+
+	/* do not unnecessarily disrupt link and skip resetting the hardware in
+	 * case the PCS has previously been successfully configured for this
+	 * interface mode
+	 */
+	if (priv->tbi_interface == interface)
+		reconf = true;
+
+	/* mark PCS configuration as incomplete */
+	priv->tbi_interface = PHY_INTERFACE_MODE_NA;
+
+	if (reconf)
+		goto skip_init_reset;
+
+	/* Assert and deassert SGMII shell reset */
+	ret = regmap_set_bits(priv->shell, GSW1XX_SHELL_RST_REQ,
+			      GSW1XX_RST_REQ_SGMII_SHELL);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_clear_bits(priv->shell, GSW1XX_SHELL_RST_REQ,
+				GSW1XX_RST_REQ_SGMII_SHELL);
+	if (ret < 0)
+		return ret;
+
+	/* Hardware Bringup FSM Enable  */
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_HWBU_CTRL,
+			   GSW1XX_SGMII_PHY_HWBU_CTRL_EN_HWBU_FSM |
+			   GSW1XX_SGMII_PHY_HWBU_CTRL_HW_FSM_EN);
+	if (ret < 0)
+		return ret;
+
+	/* Configure SGMII PHY Receiver */
+	val = FIELD_PREP(GSW1XX_SGMII_PHY_RX0_CFG2_EQ,
+			 GSW1XX_SGMII_PHY_RX0_CFG2_EQ_DEF) |
+	      GSW1XX_SGMII_PHY_RX0_CFG2_LOS_EN |
+	      GSW1XX_SGMII_PHY_RX0_CFG2_TERM_EN |
+	      FIELD_PREP(GSW1XX_SGMII_PHY_RX0_CFG2_FILT_CNT,
+			 GSW1XX_SGMII_PHY_RX0_CFG2_FILT_CNT_DEF);
+
+	if (of_property_read_bool(dp->dn, "maxlinear,rx-inverted"))
+		val |= GSW1XX_SGMII_PHY_RX0_CFG2_INVERT;
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_RX0_CFG2, val);
+	if (ret < 0)
+		return ret;
+
+	val = FIELD_PREP(GSW1XX_SGMII_PHY_TX0_CFG3_VBOOST_LEVEL,
+			 GSW1XX_SGMII_PHY_TX0_CFG3_VBOOST_LEVEL_DEF);
+
+	if (of_property_read_bool(dp->dn, "maxlinear,tx-inverted"))
+		val |= GSW1XX_SGMII_PHY_TX0_CFG3_INVERT;
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_TX0_CFG3, val);
+	if (ret < 0)
+		return ret;
+
+	/* Reset and Release TBI */
+	val = GSW1XX_SGMII_TBI_TBICTL_INITTBI | GSW1XX_SGMII_TBI_TBICTL_ENTBI |
+	      GSW1XX_SGMII_TBI_TBICTL_CRSTRR | GSW1XX_SGMII_TBI_TBICTL_CRSOFF;
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_TBI_TBICTL, val);
+	if (ret < 0)
+		return ret;
+	val &= ~GSW1XX_SGMII_TBI_TBICTL_INITTBI;
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_TBI_TBICTL, val);
+	if (ret < 0)
+		return ret;
+
+	/* Release Tx Data Buffers */
+	ret = regmap_set_bits(priv->sgmii, GSW1XX_SGMII_PCS_TXB_CTL,
+			      GSW1XX_SGMII_PCS_TXB_CTL_INIT_TX_TXB);
+	if (ret < 0)
+		return ret;
+	ret = regmap_clear_bits(priv->sgmii, GSW1XX_SGMII_PCS_TXB_CTL,
+				GSW1XX_SGMII_PCS_TXB_CTL_INIT_TX_TXB);
+	if (ret < 0)
+		return ret;
+
+	/* Release Rx Data Buffers */
+	ret = regmap_set_bits(priv->sgmii, GSW1XX_SGMII_PCS_RXB_CTL,
+			      GSW1XX_SGMII_PCS_RXB_CTL_INIT_RX_RXB);
+	if (ret < 0)
+		return ret;
+	ret = regmap_clear_bits(priv->sgmii, GSW1XX_SGMII_PCS_RXB_CTL,
+				GSW1XX_SGMII_PCS_RXB_CTL_INIT_RX_RXB);
+	if (ret < 0)
+		return ret;
+
+skip_init_reset:
+	/* override bootstrap pin settings
+	 * OVRANEG sets ANEG Mode, Enable ANEG and restart ANEG to be
+	 * taken from bits ANMODE, ANEGEN, RANEG of the ANEGCTL register.
+	 * OVERABL sets ability bits in tx_config_reg to be taken from
+	 * the TXANEGH and TXANEGL registers.
+	 */
+	anegctl = GSW1XX_SGMII_TBI_ANEGCTL_OVRANEG |
+		  GSW1XX_SGMII_TBI_ANEGCTL_OVRABL;
+
+	switch (phylink_get_link_timer_ns(interface)) {
+	case 10000:
+		anegctl |= FIELD_PREP(GSW1XX_SGMII_TBI_ANEGCTL_LT,
+				      GSW1XX_SGMII_TBI_ANEGCTL_LT_10US);
+		break;
+	case 1600000:
+		anegctl |= FIELD_PREP(GSW1XX_SGMII_TBI_ANEGCTL_LT,
+				      GSW1XX_SGMII_TBI_ANEGCTL_LT_1_6MS);
+		break;
+	case 5000000:
+		anegctl |= FIELD_PREP(GSW1XX_SGMII_TBI_ANEGCTL_LT,
+				      GSW1XX_SGMII_TBI_ANEGCTL_LT_5MS);
+		break;
+	case 10000000:
+		anegctl |= FIELD_PREP(GSW1XX_SGMII_TBI_ANEGCTL_LT,
+				      GSW1XX_SGMII_TBI_ANEGCTL_LT_10MS);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (neg_mode & PHYLINK_PCS_NEG_INBAND)
+		anegctl |= GSW1XX_SGMII_TBI_ANEGCTL_ANEGEN;
+
+	if (interface == PHY_INTERFACE_MODE_SGMII) {
+		if (sgmii_mac_mode) {
+			anegctl |= FIELD_PREP(GSW1XX_SGMII_TBI_ANEGCTL_ANMODE,
+					      GSW1XX_SGMII_TBI_ANEGCTL_ANMODE_SGMII_MAC);
+			txaneg = ADVERTISE_SGMII | ADVERTISE_LPACK;
+		} else {
+			/* lacking a defined reverse-SGMII interface mode this
+			 * driver decides whether SGMII (MAC side) or SGMII (PHY side)
+			 * is being used based on the port being a user port.
+			 */
+			anegctl |= FIELD_PREP(GSW1XX_SGMII_TBI_ANEGCTL_ANMODE,
+					      GSW1XX_SGMII_TBI_ANEGCTL_ANMODE_SGMII_PHY);
+			txaneg = LPA_SGMII | LPA_SGMII_1000FULL;
+		}
+	} else if (interface == PHY_INTERFACE_MODE_1000BASEX ||
+		   interface == PHY_INTERFACE_MODE_2500BASEX) {
+		anegctl |= FIELD_PREP(GSW1XX_SGMII_TBI_ANEGCTL_ANMODE,
+				      GSW1XX_SGMII_TBI_ANEGCTL_ANMODE_1000BASEX);
+		txaneg = phylink_mii_c22_pcs_encode_advertisement(interface,
+								  advertising);
+	} else {
+		dev_err(priv->gswip.dev, "%s: wrong interface mode %s\n",
+			__func__, phy_modes(interface));
+		return -EINVAL;
+	}
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_TBI_TXANEGH,
+			   FIELD_GET(GENMASK(15, 8), txaneg));
+	if (ret < 0)
+		return ret;
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_TBI_TXANEGL,
+			   FIELD_GET(GENMASK(7, 0), txaneg));
+	if (ret < 0)
+		return ret;
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_TBI_ANEGCTL, anegctl);
+	if (ret < 0)
+		return ret;
+
+	if (!reconf) {
+		/* setup SerDes clock speed */
+		if (interface == PHY_INTERFACE_MODE_2500BASEX)
+			nco_ctrl = GSW1XX_SGMII_2G5 | GSW1XX_SGMII_2G5_NCO2;
+		else
+			nco_ctrl = GSW1XX_SGMII_1G | GSW1XX_SGMII_1G_NCO1;
+
+		ret = regmap_update_bits(priv->clk, GSW1XX_CLK_NCO_CTRL,
+					 GSW1XX_SGMII_HSP_MASK |
+					 GSW1XX_SGMII_SEL,
+					 nco_ctrl);
+		if (ret)
+			return ret;
+
+		ret = gsw1xx_pcs_phy_xaui_write(priv, 0x30, 0x80);
+		if (ret)
+			return ret;

drivers/net/dsa/lantiq/mxl-gsw1xx.c new file +60 -0

+static int gsw1xx_probe(struct mdio_device *mdiodev)
+{
+	struct device *dev = &mdiodev->dev;
+	struct gsw1xx_priv *priv;
+	u32 version;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->mdio_dev = mdiodev;
+	priv->smdio_badr = GSW1XX_SMDIO_BADR_UNKNOWN;
+
+	priv->gswip.dev = dev;
+	priv->gswip.hw_info = of_device_get_match_data(dev);
+	if (!priv->gswip.hw_info)
+		return -EINVAL;
+
+	priv->gswip.gswip = gsw1xx_regmap_init(priv, "switch",
+					       GSW1XX_SWITCH_BASE, 0xfff);
+	if (IS_ERR(priv->gswip.gswip))
+		return PTR_ERR(priv->gswip.gswip);
+
+	priv->gswip.mdio = gsw1xx_regmap_init(priv, "mdio", GSW1XX_MMDIO_BASE,
+					      0xff);
+	if (IS_ERR(priv->gswip.mdio))
+		return PTR_ERR(priv->gswip.mdio);
+
+	priv->gswip.mii = gsw1xx_regmap_init(priv, "mii", GSW1XX_RGMII_BASE,
+					     0xff);
+	if (IS_ERR(priv->gswip.mii))
+		return PTR_ERR(priv->gswip.mii);
+
+	priv->sgmii = gsw1xx_regmap_init(priv, "sgmii", GSW1XX_SGMII_BASE,
+					 0xfff);
+	if (IS_ERR(priv->sgmii))
+		return PTR_ERR(priv->sgmii);
+
+	priv->gpio = gsw1xx_regmap_init(priv, "gpio", GSW1XX_GPIO_BASE, 0xff);
+	if (IS_ERR(priv->gpio))
+		return PTR_ERR(priv->gpio);
+
+	priv->clk = gsw1xx_regmap_init(priv, "clk", GSW1XX_CLK_BASE, 0xff);
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	priv->shell = gsw1xx_regmap_init(priv, "shell", GSW1XX_SHELL_BASE,
+					 0xff);
+	if (IS_ERR(priv->shell))
+		return PTR_ERR(priv->shell);
+
+	priv->pcs.ops = &gsw1xx_pcs_ops;
+	priv->pcs.poll = true;
+	__set_bit(PHY_INTERFACE_MODE_SGMII,
+		  priv->pcs.supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_1000BASEX,
+		  priv->pcs.supported_interfaces);
+	if (priv->gswip.hw_info->supports_2500m)
+		__set_bit(PHY_INTERFACE_MODE_2500BASEX,

drivers/net/dsa/lantiq/mxl-gsw1xx.c new file +17 -0

+	 * SGMII or RGMII as slave interface
+	 */
+	regmap_set_bits(priv->gpio, GPIO_ALTSEL0, 3);
+	regmap_set_bits(priv->gpio, GPIO_ALTSEL1, 3);
+
+	ret = regmap_read(priv->gswip.gswip, GSWIP_VERSION, &version);
+	if (ret)
+		return ret;
+
+	ret = gswip_probe_common(&priv->gswip, version);
+	if (ret)
+		return ret;
+
+	dev_set_drvdata(dev, &priv->gswip);
+
+	return 0;
+}

Quoted 8 lines · Daniel Golle

Add driver for the MaxLinear GSW1xx family of Ethernet switch ICs which are based on the same IP as the Lantiq/Intel GSWIP found in the Lantiq VR9 and Intel GRX MIPS router SoCs. The main difference is that instead of using memory-mapped I/O to communicate with the host CPU these ICs are connected via MDIO (or SPI, which isn't supported by this driver). Implement the regmap API to access the switch registers over MDIO to allow reusing lantiq_gswip_common for all core functionality.

Quoted 4 lines

The GSW1xx also comes with a SerDes port capable of 1000Base-X, SGMII and 2500Base-X, which can either be used to connect an external PHY or SFP cage, or as the CPU port. Support for the SerDes interface is implemented in this driver using the phylink_pcs interface.

I opened the GSW145 datasheet and it seems borderline in terms of what I'd suggest to implement via MFD, keeping the DSA driver to be just for the switch fabric, vs implementing everything in the DSA driver.

Just to know what to expect in the future. Are there higher-spec'd switches with an embedded CPU, waiting to be supported by Linux?

Linux running outside, but also potentially inside?

Maybe you'll need full-fledged clock, pinmux, GPIO drivers, due to IPs reused in other parts? Interrupt controller support? The SGMII "PHY" block also seems distinct from the "PCS" block, more like a driver in drivers/phy/ would control.

Quoted 29 lines

drivers/net/dsa/lantiq/mxl-gsw1xx.c new file +28 -0

+
+static int gsw1xx_pcs_phy_xaui_write(struct gsw1xx_priv *priv, u16 addr,
+				     u16 data)
+{
+	int ret, val;
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_D, data);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_A, addr);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(priv->sgmii, GSW1XX_SGMII_PHY_C,
+			   GSW1XX_SGMII_PHY_WRITE |
+			   GSW1XX_SGMII_PHY_RESET_N);
+	if (ret < 0)
+		return ret;
+
+	return regmap_read_poll_timeout(priv->sgmii, GSW1XX_SGMII_PHY_C,
+					val, val & GSW1XX_SGMII_PHY_STATUS,
+					1000, 100000);
+}
+
+static int gsw1xx_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
+			     phy_interface_t interface,
+			     const unsigned long *advertising,

drivers/net/dsa/lantiq/mxl-gsw1xx.c line 234

Quoted 5 lines

drivers/net/dsa/lantiq/mxl-gsw1xx.c new file +5 -0

+			     bool permit_pause_to_mac)
+{
+	struct gsw1xx_priv *priv = pcs_to_gsw1xx(pcs);
+	bool sgmii_mac_mode = dsa_is_user_port(priv->gswip.ds,
+					       GSW1XX_SGMII_PORT);

In lack of the phy-mode = "revsgmii" that you also mention, can we just assume that any port with phy-mode = "sgmii" is in "MAC mode"?

Quoted 1 line

[...]

Can you split up this function in multiple smaller logical blocks? The control flow with "reconf" and "skip_init_reset" is a bit difficult to follow. I can't say I understood what's going on. Ideally gsw1xx_pcs_config() fits in one-two screen.

drivers/net/dsa/lantiq/mxl-gsw1xx.c

Quoted 5 lines

drivers/net/dsa/lantiq/mxl-gsw1xx.c new file +3 -0

+static int gsw1xx_probe(struct mdio_device *mdiodev)
+{
[...]
+	/* configure GPIO pin-mux for MMDIO in case of external PHY connected to

Can you explain that MMDIO stands for MDIO master interface? On first sight it looks like a typo.