[PATCH net-next v2 0/4] net: dsa: Add SoC-e DSA driver

[PATCH net-next v2 0/4] net: dsa: Add SoC-e DSA driver

[Vasilij Strassheim]

Add support for SoC-e Ethernet switch IP cores synthesized for and
implemented in FPGAs.

The series introduces the SoC-e vendor prefix and device tree binding,
an EtherType-based SDSA tagger, and an initial DSA switch driver.

The driver uses a memory-mapped control interface. During probe, it
detects the core version and the licensed and implemented port counts,
and verifies that the synthesized core provides DSA support. Port
counts from 1 through 31 are supported.

The initial implementation supports Ethernet switching using MII, GMII,
RMII, and RGMII port interfaces. Offloading features are not
implemented.

External MDIO outputs are exposed as separate logical MDIO buses sharing
the integrated MDIO controller.

The driver was tested with a SoC-e MRS 25.01 IP core on a Xilinx ZynqMP
platform.

Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
---
Changes in v2:
- Correctly use net-next prefix
- Rework the binding into a single MMIO-based switch node
- Model hardware MDIO outputs as separate logical MDIO buses
- Detect the switch version, features, and port count from hardware
- Drop incomplete STP, bridge, and FDB offloading
- Derive phylink capabilities from each port's phy-mode
- Harden MDIO access and SDSA receive validation
- Address binding, naming, and coding style review comments

Link to v1: https://patch.msgid.link/20260729-devel-vstrassheim-soce-dsa-ml-v1-0-be569dae1b20@linutronix.de

To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Andrew Lunn <andrew@lunn.ch>
To: Vladimir Oltean <olteanv@gmail.com>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Vasilij Strassheim <v.strassheim@linutronix.de>
To: Simon Horman <horms@kernel.org>
To: Russell King <linux@armlinux.org.uk>
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org

---
Vasilij Strassheim (4):
      dt-bindings: vendor-prefixes: Add soce
      dt-bindings: net: dsa: Add SoC-e SWIP switch
      net: dsa: Add tag handling for SoC-e switches
      net: dsa: soce: Add basic support for SoC-e switch IP cores

 .../devicetree/bindings/net/dsa/soce,swip.yaml     | 161 ++++++++
 .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
 drivers/net/dsa/Kconfig                            |   2 +
 drivers/net/dsa/Makefile                           |   1 +
 drivers/net/dsa/soce/Kconfig                       |  12 +
 drivers/net/dsa/soce/Makefile                      |   4 +
 drivers/net/dsa/soce/soce_dsa.h                    |  44 +++
 drivers/net/dsa/soce/soce_dsa_core.c               | 297 +++++++++++++++
 drivers/net/dsa/soce/soce_mdio.c                   | 411 +++++++++++++++++++++
 drivers/net/dsa/soce/soce_mdio.h                   |  38 ++
 include/net/dsa.h                                  |   2 +
 net/dsa/Kconfig                                    |   6 +
 net/dsa/Makefile                                   |   1 +
 net/dsa/tag_sdsa.c                                 | 123 ++++++
 14 files changed, 1104 insertions(+)
---
base-commit: 7042c8c193e5d634198b7c766bb3a01c8e3ee0e2
change-id: 20260729-devel-vstrassheim-soce-dsa-ml-20d6a5adb838

Best regards,
--  
Vasilij Strassheim <v.strassheim@linutronix.de>

[PATCH net-next v2 1/4] dt-bindings: vendor-prefixes: Add soce

[Vasilij Strassheim]

Document binding for System-On-Chip Engineering, S.L.

Link: https://soc-e.com/
Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index ba2002969373..1c641e1f0ca6 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1581,6 +1581,8 @@ patternProperties:
     description: Standard Microsystems Corporation
   "^snps,.*":
     description: Synopsys, Inc.
+  "^soce,.*":
+    description: System-On-Chip Engineering, S.L.
   "^sochip,.*":
     description: Shenzhen SoChip Technology Co., Ltd.
   "^socionext,.*":

-- 
2.39.5

Re: [PATCH net-next v2 1/4] dt-bindings: vendor-prefixes: Add soce

[Krzysztof Kozlowski]

On Thu, Sep 03, 2026 at 08:10:58PM +0200, Vasilij Strassheim wrote:
> Document binding for System-On-Chip Engineering, S.L.
> 
> Link: https://soc-e.com/
> Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Vasilij Strassheim]

Add a binding for SoC-e FPGA-based Ethernet switch IP cores.

The compatible identifies a register layout characterized by the core
and feature identification registers at the encoded offsets. The
concrete synthesis-time configuration is then autodetected from these
registers.

Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
---
 .../devicetree/bindings/net/dsa/soce,swip.yaml     | 161 +++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml b/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml
new file mode 100644
index 000000000000..508e444e10da
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml
@@ -0,0 +1,161 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/soce,swip.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SoC-e ethernet switch IP core for FPGAs
+
+maintainers:
+  - Vasilij Strassheim <v.strassheim@linutronix.de>
+
+description:
+  SoC-e Ethernet switch IP cores are FPGA-based switches whose features
+  and number of ports are selected at synthesis time. Ports can connect
+  to CPUs, external PHYs, FPGA logic, or other switch cores. The
+  integrated MDIO controller provides multiple independently selectable
+  external outputs.
+
+$ref: dsa.yaml#
+
+properties:
+  compatible:
+    const: soce,swip-00-04-0c-10
+    description:
+      Register layout with the core version register at offset 0x00 and
+      feature identification registers at offsets 0x04, 0x0c, and 0x10.
+      Switch instances using this register layout are autodetected from
+      these registers and use this compatible regardless of their
+      synthesis-time feature and port configuration.
+
+  reg:
+    maxItems: 1
+
+  mdios:
+    type: object
+    additionalProperties: false
+    description:
+      External MDIO buses provided by the integrated MDIO controller.
+      Each child bus selects one controller output.
+
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      '^mdio@[0-9a-f]+$':
+        $ref: /schemas/net/mdio.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          reg:
+            maximum: 30
+            description:
+              MDIO controller output index, which must be lower than the
+              number of implemented switch ports.
+
+        required:
+          - reg
+
+    required:
+      - '#address-cells'
+      - '#size-cells'
+
+patternProperties:
+  '^(ethernet-)?ports$':
+    patternProperties:
+      '^(ethernet-)?port@[0-9a-f]+$':
+        $ref: dsa-port.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          reg:
+            maximum: 30
+            description:
+              Switch port index. Supported switch configurations have
+              up to 31 ports, numbered from 0 through 30.
+
+          phy-mode:
+            enum:
+              - mii
+              - gmii
+              - rmii
+              - rgmii
+              - rgmii-id
+              - rgmii-rxid
+              - rgmii-txid
+
+        required:
+          - phy-mode
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    ethernet-switch@80020000 {
+        compatible = "soce,swip-00-04-0c-10";
+        reg = <0x80020000 0x10000>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                label = "lan0";
+                phy-handle = <&switchphy0>;
+                phy-mode = "rgmii-id";
+            };
+
+            port@1 {
+                reg = <1>;
+                label = "lan1";
+                phy-handle = <&switchphy1>;
+                phy-mode = "rgmii-id";
+            };
+
+            port@2 {
+                reg = <2>;
+                ethernet = <&eth0>;
+                phy-mode = "gmii";
+
+                fixed-link {
+                    speed = <1000>;
+                    full-duplex;
+                };
+            };
+        };
+
+        mdios {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            mdio@0 {
+                reg = <0>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                switchphy0: ethernet-phy@1 {
+                    compatible = "ethernet-phy-ieee802.3-c22";
+                    reg = <1>;
+                };
+            };
+
+            mdio@1 {
+                reg = <1>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                switchphy1: ethernet-phy@1 {
+                    compatible = "ethernet-phy-ieee802.3-c22";
+                    reg = <1>;
+                };
+            };
+        };
+    };

-- 
2.39.5

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Krzysztof Kozlowski]

On Thu, Sep 03, 2026 at 08:10:59PM +0200, Vasilij Strassheim wrote:
> Add a binding for SoC-e FPGA-based Ethernet switch IP cores.
> 
> The compatible identifies a register layout characterized by the core
> and feature identification registers at the encoded offsets. The
> concrete synthesis-time configuration is then autodetected from these
> registers.
> 
> Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
> ---
>  .../devicetree/bindings/net/dsa/soce,swip.yaml     | 161 +++++++++++++++++++++
>  1 file changed, 161 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml b/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml
> new file mode 100644
> index 000000000000..508e444e10da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml
> @@ -0,0 +1,161 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/soce,swip.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SoC-e ethernet switch IP core for FPGAs
> +
> +maintainers:
> +  - Vasilij Strassheim <v.strassheim@linutronix.de>
> +
> +description:
> +  SoC-e Ethernet switch IP cores are FPGA-based switches whose features
> +  and number of ports are selected at synthesis time. Ports can connect
> +  to CPUs, external PHYs, FPGA logic, or other switch cores. The
> +  integrated MDIO controller provides multiple independently selectable
> +  external outputs.
> +
> +$ref: dsa.yaml#
> +
> +properties:
> +  compatible:
> +    const: soce,swip-00-04-0c-10
> +    description:
> +      Register layout with the core version register at offset 0x00 and
> +      feature identification registers at offsets 0x04, 0x0c, and 0x10.
> +      Switch instances using this register layout are autodetected from
> +      these registers and use this compatible regardless of their
> +      synthesis-time feature and port configuration.

There is no traceable and documented version or model name?

> +
> +  reg:
> +    maxItems: 1
> +
> +  mdios:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      External MDIO buses provided by the integrated MDIO controller.
> +      Each child bus selects one controller output.
> +
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      '^mdio@[0-9a-f]+$':
> +        $ref: /schemas/net/mdio.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            maximum: 30
> +            description:
> +              MDIO controller output index, which must be lower than the
> +              number of implemented switch ports.
> +
> +        required:
> +          - reg
> +
> +    required:
> +      - '#address-cells'
> +      - '#size-cells'
> +
> +patternProperties:
> +  '^(ethernet-)?ports$':

ethernet-ports

> +    patternProperties:
> +      '^(ethernet-)?port@[0-9a-f]+$':

ethernet-port

> +        $ref: dsa-port.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            maximum: 30
> +            description:
> +              Switch port index. Supported switch configurations have
> +              up to 31 ports, numbered from 0 through 30.

Best regards,
Krzysztof

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Vasilij Strassheim]

On Mon, 2026-09-07 at 11:21 +0200, Krzysztof Kozlowski wrote:
> On Thu, Sep 03, 2026 at 08:10:59PM +0200, Vasilij Strassheim wrote:
> > Add a binding for SoC-e FPGA-based Ethernet switch IP cores.
> > 
> > The compatible identifies a register layout characterized by the core
> > and feature identification registers at the encoded offsets. The
> > concrete synthesis-time configuration is then autodetected from these
> > registers.
> > 
> > Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
> > ---
> >  .../devicetree/bindings/net/dsa/soce,swip.yaml     | 161 +++++++++++++++++++++
> >  1 file changed, 161 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml b/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml
> > new file mode 100644
> > index 000000000000..508e444e10da
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/dsa/soce,swip.yaml
> > @@ -0,0 +1,161 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/dsa/soce,swip.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SoC-e ethernet switch IP core for FPGAs
> > +
> > +maintainers:
> > +  - Vasilij Strassheim <v.strassheim@linutronix.de>
> > +
> > +description:
> > +  SoC-e Ethernet switch IP cores are FPGA-based switches whose features
> > +  and number of ports are selected at synthesis time. Ports can connect
> > +  to CPUs, external PHYs, FPGA logic, or other switch cores. The
> > +  integrated MDIO controller provides multiple independently selectable
> > +  external outputs.
> > +
> > +$ref: dsa.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: soce,swip-00-04-0c-10
> > +    description:
> > +      Register layout with the core version register at offset 0x00 and
> > +      feature identification registers at offsets 0x04, 0x0c, and 0x10.
> > +      Switch instances using this register layout are autodetected from
> > +      these registers and use this compatible regardless of their
> > +      synthesis-time feature and port configuration.
> 
> There is no traceable and documented version or model name?
> 

tl;dr
There are too many, but they are not specific enough. I think it is not
useful to use this information to define a specific compatible. Using a
set of known registers, the driver can check the active features and
capabilities and adapt accordingly, regardless of marketing names.

SoC-e shows different variants of their IP cores on their website:
https://soc-e.com/soctek-ip-cores/
SocTek is product line that covers different types of IP cores. Ethernet
switches, Timing controllers, and SpaceWire.
For Ethernet switches, there are different variants listed like Managed
Ethernet Switch (MES), Managed Redundant Switch (MRS) and some others.
Most of the variants are build of other variants e.g. MRS = MES+HPS
Think of it as name of a base collection of IP functionality. On top of
it you can license additional features. This do not result in a
different name. If you start with MES and buy HPS later on, you may or
may not end up with MRS.
Additionally, the actual switch is synthesized in FPGA, so the final
implementation can vary even if the same IP core is used e.g. port
number, deactivated features, etc.
This is used to bring down product costs: You can buy the same hardware,
really the same hardware - here a PLC - with a different bitstream.
Depending on the enabled features you have to pay more money for it.


> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  mdios:
> > +    type: object
> > +    additionalProperties: false
> > +    description:
> > +      External MDIO buses provided by the integrated MDIO controller.
> > +      Each child bus selects one controller output.
> > +
> > +    properties:
> > +      '#address-cells':
> > +        const: 1
> > +      '#size-cells':
> > +        const: 0
> > +
> > +    patternProperties:
> > +      '^mdio@[0-9a-f]+$':
> > +        $ref: /schemas/net/mdio.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          reg:
> > +            maximum: 30
> > +            description:
> > +              MDIO controller output index, which must be lower than the
> > +              number of implemented switch ports.
> > +
> > +        required:
> > +          - reg
> > +
> > +    required:
> > +      - '#address-cells'
> > +      - '#size-cells'
> > +
> > +patternProperties:
> > +  '^(ethernet-)?ports$':
> 
> ethernet-ports
> 
> > +    patternProperties:
> > +      '^(ethernet-)?port@[0-9a-f]+$':
> 
> ethernet-port
> 

Is there a reason for such a restriction?
The pattern intentionally follows the generic DSA binding in dsa.yaml,
which permits both ports/port@N and ethernet-ports/ethernet-port@N. The
device-specific pattern is needed here to apply the SoC-e port
constraints to both accepted node naming variants.

> > +        $ref: dsa-port.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          reg:
> > +            maximum: 30
> > +            description:
> > +              Switch port index. Supported switch configurations have
> > +              up to 31 ports, numbered from 0 through 30.

Thanks,
Vasilij

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Andrew Lunn]

> > ethernet-ports
> > 
> > > +    patternProperties:
> > > +      '^(ethernet-)?port@[0-9a-f]+$':
> > 
> > ethernet-port
> > 
> 
> Is there a reason for such a restriction?
> The pattern intentionally follows the generic DSA binding in dsa.yaml,

You need to be careful with the DSA binding. It is old, and uses some
things which are not considered best practices today. But they cannot
be removed because of backwards compatibility.

	Andrew

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Vasilij Strassheim]

On Mon, 2026-09-07 at 20:56 +0200, Andrew Lunn wrote:
> > > ethernet-ports
> > > 
> > > > +    patternProperties:
> > > > +      '^(ethernet-)?port@[0-9a-f]+$':
> > > 
> > > ethernet-port
> > > 
> > 
> > Is there a reason for such a restriction?
> > The pattern intentionally follows the generic DSA binding in dsa.yaml,
> 
> You need to be careful with the DSA binding. It is old, and uses some
> things which are not considered best practices today. But they cannot
> be removed because of backwards compatibility.
> 

Got it. Technically, it makes no difference, so I will just fix it.

Thanks,
Vasilij

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

On Mon Sep 07 2026, Andrew Lunn wrote:
>> > ethernet-ports
>> > 
>> > > +    patternProperties:
>> > > +      '^(ethernet-)?port@[0-9a-f]+$':
>> > 
>> > ethernet-port
>> > 
>> 
>> Is there a reason for such a restriction?
>> The pattern intentionally follows the generic DSA binding in dsa.yaml,
>
> You need to be careful with the DSA binding. It is old, and uses some
> things which are not considered best practices today. But they cannot
> be removed because of backwards compatibility.

Yes, exactly. When we added the dsa.yaml binding, we've had a discussion
about 'port' vs. 'ethernet-port' and had to allow both [1].

Thanks,
Kurt

[1] - https://lore.kernel.org/netdev/20200710090618.28945-1-kurt@linutronix.de/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Krzysztof Kozlowski]

On Mon, Sep 07, 2026 at 04:01:26PM +0200, Vasilij Strassheim wrote:
> > > +patternProperties:
> > > +  '^(ethernet-)?ports$':
> > 
> > ethernet-ports
> > 
> > > +    patternProperties:
> > > +      '^(ethernet-)?port@[0-9a-f]+$':
> > 
> > ethernet-port
> > 
> 
> Is there a reason for such a restriction?
> The pattern intentionally follows the generic DSA binding in dsa.yaml,

dsa.yaml lists ethernet-ports and port.

> which permits both ports/port@N and ethernet-ports/ethernet-port@N. The
> device-specific pattern is needed here to apply the SoC-e port
> constraints to both accepted node naming variants.

ports is legacy (because it conflicts with graph ports), for existing
users, so why insisting on this pattern? If legacy is being added to new
schemas, when are we going to be able to remove that legacy? Never.

Best regards,
Krzysztof

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Vasilij Strassheim]

On Tue, 2026-09-08 at 11:14 +0200, Krzysztof Kozlowski wrote:
> On Mon, Sep 07, 2026 at 04:01:26PM +0200, Vasilij Strassheim wrote:
> > > > +patternProperties:
> > > > +  '^(ethernet-)?ports$':
> > > 
> > > ethernet-ports
> > > 
> > > > +    patternProperties:
> > > > +      '^(ethernet-)?port@[0-9a-f]+$':
> > > 
> > > ethernet-port
> > > 
> > 
> > Is there a reason for such a restriction?
> > The pattern intentionally follows the generic DSA binding in dsa.yaml,
> 
> dsa.yaml lists ethernet-ports and port.
> 
> > which permits both ports/port@N and ethernet-ports/ethernet-port@N. The
> > device-specific pattern is needed here to apply the SoC-e port
> > constraints to both accepted node naming variants.
> 
> ports is legacy (because it conflicts with graph ports), for existing
> users, so why insisting on this pattern? If legacy is being added to new
> schemas, when are we going to be able to remove that legacy? Never.
> 

I will use ethernet-ports and ethernet-port@N instead.

Thanks for the clarification!
I was not insisting on the pattern, just wanted to understand the
reasoning behind it. I had missed that ports is legacy and followed the
generic DSA pattern and the same 10-month-old pattern in Lantiq binding.
Now I see that it is used for compatibility there and should not be
introduced in new bindings.

Best regards,
Vasilij

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Andrew Lunn]

> +    patternProperties:
> +      '^mdio@[0-9a-f]+$':
> +        $ref: /schemas/net/mdio.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            maximum: 30
> +            description:
> +              MDIO controller output index, which must be lower than the
> +              number of implemented switch ports.

Is there a relationship between an MDIO bus and a port?

I'm just wondering if the MDIO bus should be a property of the
port. There are switch which have an MDIO bus per port.

> +patternProperties:
> +  '^(ethernet-)?ports$':
> +    patternProperties:
> +      '^(ethernet-)?port@[0-9a-f]+$':
> +        $ref: dsa-port.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            maximum: 30
> +            description:
> +              Switch port index. Supported switch configurations have
> +              up to 31 ports, numbered from 0 through 30.

30 seems odd. Is 31 something special? 

> +examples:
> +  - |
> +    ethernet-switch@80020000 {
> +        compatible = "soce,swip-00-04-0c-10";
> +        reg = <0x80020000 0x10000>;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                label = "lan0";

label is another thing in DSA which is today not best practice. I
would drop this.

	Andrew

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Andrew Lunn]

> > +examples:
> > +  - |
> > +    ethernet-switch@80020000 {
> > +        compatible = "soce,swip-00-04-0c-10";
> > +        reg = <0x80020000 0x10000>;
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            port@0 {
> > +                reg = <0>;
> > +                label = "lan0";
> 
> label is another thing in DSA which is today not best practice. I
> would drop this.

Humm, actually, since it is a DSA driver, it is O.K. You can keep
it. It is however better practice to allow udev to rename the
interface as it wants to.

	Andrew

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Vasilij Strassheim]

On Mon, 2026-09-07 at 21:04 +0200, Andrew Lunn wrote:
> > +    patternProperties:
> > +      '^mdio@[0-9a-f]+$':
> > +        $ref: /schemas/net/mdio.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          reg:
> > +            maximum: 30
> > +            description:
> > +              MDIO controller output index, which must be lower than the
> > +              number of implemented switch ports.
> 
> Is there a relationship between an MDIO bus and a port?

Yes, there is a one-to-one relationship for this IP. The MDIO bus
selector corresponds to the switch port index.

> 
> I'm just wondering if the MDIO bus should be a property of the
> port. There are switch which have an MDIO bus per port.
> 

I see this model in the new NETC switch binding. I will move the
optional mdio node below the corresponding ethernet-port@N and derive
the selector from the port's reg.

> > +patternProperties:
> > +  '^(ethernet-)?ports$':
> > +    patternProperties:
> > +      '^(ethernet-)?port@[0-9a-f]+$':
> > +        $ref: dsa-port.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          reg:
> > +            maximum: 30
> > +            description:
> > +              Switch port index. Supported switch configurations have
> > +              up to 31 ports, numbered from 0 through 30.
> 
> 30 seems odd. Is 31 something special? 
> 

The hardware stores the port count directly in a 5-bit field, so 31 is
the maximum representable count. Therefore, valid zero-based port
indices range from 0 to 30. The switch documentation is unclear about
the encoding, but I tested a three-port configuration and the field
contained 3.

> > +examples:
> > +  - |
> > +    ethernet-switch@80020000 {
> > +        compatible = "soce,swip-00-04-0c-10";
> > +        reg = <0x80020000 0x10000>;
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            port@0 {
> > +                reg = <0>;
> > +                label = "lan0";
> 
> label is another thing in DSA which is today not best practice. I
> would drop this.

I have no problem removing the label from example. I'll have to
adjust it anyway for the new MDIO bus and port relationship.

> 
> 	Andrew

Thanks,
Vasilij

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Andrew Lunn]

On Tue, Sep 08, 2026 at 08:15:35PM +0200, Vasilij Strassheim wrote:
> On Mon, 2026-09-07 at 21:04 +0200, Andrew Lunn wrote:
> > > +    patternProperties:
> > > +      '^mdio@[0-9a-f]+$':
> > > +        $ref: /schemas/net/mdio.yaml#
> > > +        unevaluatedProperties: false
> > > +
> > > +        properties:
> > > +          reg:
> > > +            maximum: 30
> > > +            description:
> > > +              MDIO controller output index, which must be lower than the
> > > +              number of implemented switch ports.
> > 
> > Is there a relationship between an MDIO bus and a port?
> 
> Yes, there is a one-to-one relationship for this IP. The MDIO bus
> selector corresponds to the switch port index.
> 
> > 
> > I'm just wondering if the MDIO bus should be a property of the
> > port. There are switch which have an MDIO bus per port.
> > 
> 
> I see this model in the new NETC switch binding. I will move the
> optional mdio node below the corresponding ethernet-port@N and derive
> the selector from the port's reg.

Since this is an FPGA, i assume there are no internal PHYs. Mixed mode
is not something FPGAs do. There are sometime "interesting"
relationships between port number and address on the MDIO bus. But
without internal PHYs you don't need to worry about this.

> > > +patternProperties:
> > > +  '^(ethernet-)?ports$':
> > > +    patternProperties:
> > > +      '^(ethernet-)?port@[0-9a-f]+$':
> > > +        $ref: dsa-port.yaml#
> > > +        unevaluatedProperties: false
> > > +
> > > +        properties:
> > > +          reg:
> > > +            maximum: 30
> > > +            description:
> > > +              Switch port index. Supported switch configurations have
> > > +              up to 31 ports, numbered from 0 through 30.
> > 
> > 30 seems odd. Is 31 something special? 
> > 
> 
> The hardware stores the port count directly in a 5-bit field, so 31 is
> the maximum representable count. Therefore, valid zero-based port
> indices range from 0 to 30. The switch documentation is unclear about
> the encoding, but I tested a three-port configuration and the field
> contained 3.

So in theory, a 0 port switch is possible!

   Andrew

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Vasilij Strassheim]

On Tue, 2026-09-08 at 21:10 +0200, Andrew Lunn wrote:
> On Tue, Sep 08, 2026 at 08:15:35PM +0200, Vasilij Strassheim wrote:
> > On Mon, 2026-09-07 at 21:04 +0200, Andrew Lunn wrote:
> > > > +    patternProperties:
> > > > +      '^mdio@[0-9a-f]+$':
> > > > +        $ref: /schemas/net/mdio.yaml#
> > > > +        unevaluatedProperties: false
> > > > +
> > > > +        properties:
> > > > +          reg:
> > > > +            maximum: 30
> > > > +            description:
> > > > +              MDIO controller output index, which must be lower than the
> > > > +              number of implemented switch ports.
> > > 
> > > Is there a relationship between an MDIO bus and a port?
> > 
> > Yes, there is a one-to-one relationship for this IP. The MDIO bus
> > selector corresponds to the switch port index.
> > 
> > > 
> > > I'm just wondering if the MDIO bus should be a property of the
> > > port. There are switch which have an MDIO bus per port.
> > > 
> > 
> > I see this model in the new NETC switch binding. I will move the
> > optional mdio node below the corresponding ethernet-port@N and derive
> > the selector from the port's reg.
> 
> Since this is an FPGA, i assume there are no internal PHYs. Mixed mode
> is not something FPGAs do. There are sometime "interesting"
> relationships between port number and address on the MDIO bus. But
> without internal PHYs you don't need to worry about this.
> 

Correct, the FPGA IP has no internal PHYs. Each port is associated with
a dedicated external MDIO bus.

> > > > +patternProperties:
> > > > +  '^(ethernet-)?ports$':
> > > > +    patternProperties:
> > > > +      '^(ethernet-)?port@[0-9a-f]+$':
> > > > +        $ref: dsa-port.yaml#
> > > > +        unevaluatedProperties: false
> > > > +
> > > > +        properties:
> > > > +          reg:
> > > > +            maximum: 30
> > > > +            description:
> > > > +              Switch port index. Supported switch configurations have
> > > > +              up to 31 ports, numbered from 0 through 30.
> > > 
> > > 30 seems odd. Is 31 something special? 
> > > 
> > 
> > The hardware stores the port count directly in a 5-bit field, so 31 is
> > the maximum representable count. Therefore, valid zero-based port
> > indices range from 0 to 30. The switch documentation is unclear about
> > the encoding, but I tested a three-port configuration and the field
> > contained 3.
> 
> So in theory, a 0 port switch is possible!

In theory, yes, but I prefer to treat it as invalid until it can be
tested.

> 
>    Andrew

Thanks,
Vasilij

Re: [PATCH net-next v2 2/4] dt-bindings: net: dsa: Add SoC-e SWIP switch

[Andrew Lunn]

> > Since this is an FPGA, i assume there are no internal PHYs. Mixed mode
> > is not something FPGAs do. There are sometime "interesting"
> > relationships between port number and address on the MDIO bus. But
> > without internal PHYs you don't need to worry about this.
> > 
> 
> Correct, the FPGA IP has no internal PHYs. Each port is associated with
> a dedicated external MDIO bus.

How is a port and the MDIO bus associated?

Linux, in general, does not care. You have a collection of MDIO
busses, and on those busses you have a collection of PHYs. The MACs
use a phandle, or some other means to point to the PHY it should use.

Given this is a 31 port switch, why would you dedicate 62 pins to
MDIO, two per port, when you can put 32 PHYs on one MDIO bus?

So i doubt there is a strong association between port and MDIO bus.

What i could image is the one real MDIO bus controller is in global
scope within the RTL design. And then the logic to provide a port with
its multiplexor on that shared bus is in the per port RTL design
scope. I've not done much FPGA design, but i assume if you don't
connect the per port MDIO lines to anything, while place and route is
performed, they get optimised out?

> > So in theory, a 0 port switch is possible!
> 
> In theory, yes, but I prefer to treat it as invalid until it can be
> tested.

Yes, i would treat 0, 1 and probably 2 as invalid. 2 ports would
technically work in the DSA setup, but is pretty much pointless. It
only gets interesting with 3 ports or more.

     Andrew

[PATCH net-next v2 3/4] net: dsa: Add tag handling for SoC-e switches

[Vasilij Strassheim]

SoC-e switches exchange frames with the conduit using an 8-byte SDSA
header inserted between the source MAC address and the original
EtherType. The header identifies itself with EtherType 0xdcdc and carries
the frame direction and a 10-bit source or destination port.
Add support for both frame types FROM_CPU and TO_CPU.

SDSA VLAN metadata generation could not be tested on the available
switch configuration yet. Leave these fields clear on transmit and
reject receive headers which mark the metadata as valid.

Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
---
 include/net/dsa.h  |   2 +
 net/dsa/Kconfig    |   6 +++
 net/dsa/Makefile   |   1 +
 net/dsa/tag_sdsa.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 132 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7507d632e7c6..cabe9f94788a 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -61,6 +61,7 @@ struct tc_action;
 #define DSA_TAG_PROTO_NETC_VALUE		33
 #define DSA_TAG_PROTO_KSZ8463_VALUE		34
 #define DSA_TAG_PROTO_MT7628_VALUE		35
+#define DSA_TAG_PROTO_SDSA_VALUE		36
 
 enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NONE		= DSA_TAG_PROTO_NONE_VALUE,
@@ -99,6 +100,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_NETC		= DSA_TAG_PROTO_NETC_VALUE,
 	DSA_TAG_PROTO_KSZ8463		= DSA_TAG_PROTO_KSZ8463_VALUE,
 	DSA_TAG_PROTO_MT7628		= DSA_TAG_PROTO_MT7628_VALUE,
+	DSA_TAG_PROTO_SDSA		= DSA_TAG_PROTO_SDSA_VALUE,
 };
 
 struct dsa_switch;
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 23b4b74004ed..0032ccd3badf 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -188,6 +188,12 @@ config NET_DSA_TAG_RZN1_A5PSW
 	  Renesas RZ/N1 embedded switch that uses an 8 byte tag located after
 	  destination MAC address.
 
+config NET_DSA_TAG_SDSA
+	tristate "Tag driver for SoC-e switches using EtherType SDSA headers"
+	help
+	  Say Y or M if you want to enable support for tagging frames for the
+	  SoC-e switches.
+
 config NET_DSA_TAG_LAN9303
 	tristate "Tag driver for SMSC/Microchip LAN9303 family of switches"
 	help
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index d15bcf5c68f0..2f31ed0e4626 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
 obj-$(CONFIG_NET_DSA_TAG_RTL4_A) += tag_rtl4_a.o
 obj-$(CONFIG_NET_DSA_TAG_RTL8_4) += tag_rtl8_4.o
 obj-$(CONFIG_NET_DSA_TAG_RZN1_A5PSW) += tag_rzn1_a5psw.o
+obj-$(CONFIG_NET_DSA_TAG_SDSA) += tag_sdsa.o
 obj-$(CONFIG_NET_DSA_TAG_SJA1105) += tag_sja1105.o
 obj-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
 obj-$(CONFIG_NET_DSA_TAG_VSC73XX_8021Q) += tag_vsc73xx_8021q.o
diff --git a/net/dsa/tag_sdsa.c b/net/dsa/tag_sdsa.c
new file mode 100644
index 000000000000..83bccd65b1b1
--- /dev/null
+++ b/net/dsa/tag_sdsa.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020-2026 System on Chip engineering, S.L.
+ * Copyright (c) 2026 Linutronix GmbH
+ * Author: Vasilij Strassheim <v.strassheim@linutronix.de>
+ */
+
+#include <linux/bitops.h>
+#include <linux/byteorder/generic.h>
+#include <linux/etherdevice.h>
+
+#include "tag.h"
+
+#define SDSA_HLEN	8
+
+#define SDSA_NAME	"sdsa"
+#define ETH_P_SDSA	0xDCDC
+
+/* SDSA tag byte layout (after the 12-byte MAC header):
+ * Bytes 0-1: SDSA EtherType (0xDCDC)
+ * Bytes 2-3: Reserved
+ * Byte 4:    Frame type (bits 7-6), VLAN-info bit (bit 5), port[9:5] (bits 4-0)
+ * Byte 5:    Port[4:0] (bits 7-3)
+ * Bytes 6-7: PCP (bits 7-5) / CFI (bit 4) / VID (bits 3-0 + byte 7), only
+ *            meaningful when the VLAN-info bit is set.
+ */
+#define SDSA_TAG_FRAME_TYPE_MASK	GENMASK(7, 6)
+#define SDSA_TAG_VLAN_BIT		BIT(5)
+#define SDSA_TAG_PORT_HI_MASK		GENMASK(4, 0)
+#define SDSA_TAG_PORT_LO_MASK		GENMASK(7, 3)
+#define SDSA_TAG_PORT_HI_SHIFT		5
+#define SDSA_FRAME_TYPE_TO_CPU		0
+#define SDSA_FRAME_TYPE_FROM_CPU	1
+
+struct sdsa_tag {
+	__be16 ethertype;
+	__be16 reserved;
+	u8 frame_type_port_hi;
+	u8 port_lo;
+	__be16 vlan;
+};
+
+static struct sk_buff *sdsa_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct dsa_port *dp = dsa_user_to_port(dev);
+	struct sdsa_tag *tag;
+
+	BUILD_BUG_ON(sizeof(*tag) != SDSA_HLEN);
+
+	skb_push(skb, SDSA_HLEN);
+	dsa_alloc_etype_header(skb, SDSA_HLEN);
+
+	/* Construct the FROM_CPU DSA tag. */
+	tag = dsa_etype_header_pos_tx(skb);
+	tag->ethertype = cpu_to_be16(ETH_P_SDSA);
+	tag->reserved = 0;
+	tag->frame_type_port_hi =
+		FIELD_PREP(SDSA_TAG_FRAME_TYPE_MASK, SDSA_FRAME_TYPE_FROM_CPU) |
+		FIELD_PREP(SDSA_TAG_PORT_HI_MASK,
+			   dp->index >> SDSA_TAG_PORT_HI_SHIFT);
+	tag->port_lo = FIELD_PREP(SDSA_TAG_PORT_LO_MASK, dp->index);
+	tag->vlan = 0;
+
+	return skb;
+}
+
+static struct sk_buff *sdsa_rcv(struct sk_buff *skb, struct net_device *dev)
+{
+	struct sdsa_tag *tag;
+	int source_port;
+	u8 frame_type;
+
+	if (unlikely(!pskb_may_pull(skb, SDSA_HLEN)))
+		goto out_drop;
+
+	tag = dsa_etype_header_pos_rx(skb);
+	if (unlikely(tag->ethertype != cpu_to_be16(ETH_P_SDSA)))
+		goto out_drop;
+
+	/* Check that the frame type is TO_CPU. */
+	frame_type = FIELD_GET(SDSA_TAG_FRAME_TYPE_MASK,
+			       tag->frame_type_port_hi);
+	if (frame_type != SDSA_FRAME_TYPE_TO_CPU)
+		goto out_drop;
+
+	/* SDSA VLAN information is not supported. */
+	if (tag->frame_type_port_hi & SDSA_TAG_VLAN_BIT)
+		goto out_drop;
+
+	/* Determine the source port from the two port fields. */
+	source_port = FIELD_GET(SDSA_TAG_PORT_HI_MASK,
+				tag->frame_type_port_hi) <<
+		      SDSA_TAG_PORT_HI_SHIFT;
+	source_port |= FIELD_GET(SDSA_TAG_PORT_LO_MASK, tag->port_lo);
+
+	skb->dev = dsa_conduit_find_user(dev, 0, source_port);
+	if (!skb->dev)
+		goto out_drop;
+
+	skb_pull_rcsum(skb, SDSA_HLEN);
+	dsa_strip_etype_header(skb, SDSA_HLEN);
+
+	dsa_default_offload_fwd_mark(skb);
+	return skb;
+
+out_drop:
+	kfree_skb(skb);
+	return NULL;
+}
+
+static const struct dsa_device_ops sdsa_netdev_ops = {
+	.name = SDSA_NAME,
+	.proto = DSA_TAG_PROTO_SDSA,
+	.xmit = sdsa_xmit,
+	.rcv = sdsa_rcv,
+	.needed_headroom = SDSA_HLEN,
+};
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("DSA tag driver for SoC-e SDSA protocol");
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_SDSA, SDSA_NAME);
+
+module_dsa_tag_driver(sdsa_netdev_ops);

-- 
2.39.5

Re: [PATCH net-next v2 3/4] net: dsa: Add tag handling for SoC-e switches

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 1

- [Medium] sdsa_rcv() (net/dsa/tag_sdsa.c, lines 86-87) unconditionally
  destroys every received frame whose SDSA header has the VLAN-info bit…
- [Low] Concern (INVALIDATED by series context): 'the new DSA tag
  protocol DSA_TAG_PROTO_SDSA / CONFIG_NET_DSA_TAG_SDSA has no in-tree…

--

net: dsa: Add tag handling for SoC-e switches

This adds a new DSA tagger for SoC-e switches, which insert an 8-byte SDSA
header with EtherType 0xdcdc between the source MAC address and the original
EtherType.  Both TO_CPU and FROM_CPU directions are handled, while the VLAN
metadata fields in the header are left clear on transmit and rejected on
receive.

> diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
> index 23b4b74004ed5..0032ccd3badf6 100644
> --- a/net/dsa/Kconfig
> +++ b/net/dsa/Kconfig
> @@ -188,6 +188,12 @@ config NET_DSA_TAG_RZN1_A5PSW
>  	  Renesas RZ/N1 embedded switch that uses an 8 byte tag located after
>  	  destination MAC address.
>  
> +config NET_DSA_TAG_SDSA
> +	tristate "Tag driver for SoC-e switches using EtherType SDSA headers"
> +	help
> +	  Say Y or M if you want to enable support for tagging frames for the
> +	  SoC-e switches.
> +

[Severity: Low]
This isn't a bug, just a note so it does not get raised again during review.
Taken on its own, this patch adds a tagger and a protocol id with no in-tree
user.  The next patch in the series, "net: dsa: soce: Add basic support for
SoC-e switch IP cores", adds drivers/net/dsa/soce/ whose Kconfig contains
select NET_DSA_TAG_SDSA and whose soce_get_tag_protocol() returns
DSA_TAG_PROTO_SDSA, so the tagger is reachable by the end of the series.

[ ... ]

> diff --git a/net/dsa/tag_sdsa.c b/net/dsa/tag_sdsa.c
> new file mode 100644
> index 0000000000000..83bccd65b1b1d
> --- /dev/null
> +++ b/net/dsa/tag_sdsa.c
> @@ -0,0 +1,123 @@

[ ... ]

> +/* SDSA tag byte layout (after the 12-byte MAC header):
> + * Bytes 0-1: SDSA EtherType (0xDCDC)
> + * Bytes 2-3: Reserved
> + * Byte 4:    Frame type (bits 7-6), VLAN-info bit (bit 5), port[9:5] (bits 4-0)
> + * Byte 5:    Port[4:0] (bits 7-3)
> + * Bytes 6-7: PCP (bits 7-5) / CFI (bit 4) / VID (bits 3-0 + byte 7), only
> + *            meaningful when the VLAN-info bit is set.
> + */

[ ... ]

> +static struct sk_buff *sdsa_xmit(struct sk_buff *skb, struct net_device *dev)
> +{

[ ... ]

> +	tag->port_lo = FIELD_PREP(SDSA_TAG_PORT_LO_MASK, dp->index);
> +	tag->vlan = 0;
> +
> +	return skb;
> +}
> +
> +static struct sk_buff *sdsa_rcv(struct sk_buff *skb, struct net_device *dev)
> +{

[ ... ]

> +	/* SDSA VLAN information is not supported. */
> +	if (tag->frame_type_port_hi & SDSA_TAG_VLAN_BIT)
> +		goto out_drop;

[Severity: Medium]
Can this drop every VLAN-tagged frame destined for the CPU?

The layout comment in this file states that bytes 6-7 carry PCP/CFI/VID and
are "only meaningful when the VLAN-info bit is set", so the hardware appears
to move the frame's VLAN identity out of the frame body and into the tag.

The SDSA layout looks like a close relative of Marvell (E)DSA: proprietary
EtherType, two reserved bytes, command in bits 7-6 of the first tag byte,
port in bits 7-3 of the second byte, and the VLAN indication in bit 5.  In
tag_dsa.c that bit means the frame arrived VLAN-tagged and the tagger
reconstructs the 802.1Q header rather than dropping the frame:

net/dsa/tag_dsa.c:dsa_rcv_ll() {
	...
	if (dsa_header[0] & 0x20) {
		u8 new_header[4];

		/* Insert 802.1Q ethertype and copy the VLAN-related
		 * fields, but clear the bit that will hold CFI (since
		 * DSA uses that bit location for another purpose).
		 */
		new_header[0] = (ETH_P_8021Q >> 8) & 0xff;
		new_header[1] = ETH_P_8021Q & 0xff;
		new_header[2] = dsa_header[2] & ~0x10;
		new_header[3] = dsa_header[3];
	...
}

If the SoC-e core sets bit 5 for ordinary tagged ingress in any supported
configuration, all such traffic towards the CPU is freed here.  The driver
added later in the series only programs SOCE_TAG_ALL_FRAMES_ENABLE and
SOCE_CUSTOM_RULES_TAGGING_ENABLE in soce_sw_enable_tagging() and implements
no VLAN operations at all, so nothing turns this hardware behaviour off.
Is it known that the core never sets that bit?

The drop itself is a bare kfree_skb() with no drop reason, counter, or
ratelimited message, so from userspace the resulting loss is hard to tell
apart from a link or MAC problem.  Would kfree_skb_reason() be worth using
for these paths?

On the transmit side, sdsa_xmit() writes tag->vlan = 0 with the VLAN-info
bit left clear while any 802.1Q header stays inline in the payload.  If the
switch expects the egress VLAN identity in the tag, as the Marvell-style
parts do in tag_dsa.c:

net/dsa/tag_dsa.c:dsa_xmit_ll() {
	...
	if (skb->protocol == htons(ETH_P_8021Q) &&
	    (!br_dev || br_vlan_enabled(br_dev))) {
		...
		dsa_header[0] = (cmd << 6) | 0x20 | tag_dev;
	...
}

would FROM_CPU frames then be injected as untagged/PVID-assigned with the
inline 802.1Q header treated as opaque payload?

The commit message does say the field could not be tested:

    SDSA VLAN metadata generation could not be tested on the available
    switch configuration yet. Leave these fields clear on transmit and
    reject receive headers which mark the metadata as valid.

Could the receive side pass such frames up (or at least count them) instead
of discarding them, so an untested field does not turn into silent packet
loss?

> +
> +	/* Determine the source port from the two port fields. */
> +	source_port = FIELD_GET(SDSA_TAG_PORT_HI_MASK,
> +				tag->frame_type_port_hi) <<
> +		      SDSA_TAG_PORT_HI_SHIFT;
> +	source_port |= FIELD_GET(SDSA_TAG_PORT_LO_MASK, tag->port_lo);
> +
> +	skb->dev = dsa_conduit_find_user(dev, 0, source_port);
> +	if (!skb->dev)
> +		goto out_drop;

[ ... ]

> +out_drop:
> +	kfree_skb(skb);
> +	return NULL;
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-devel-vstrassheim-soce-dsa-ml-v2-0-fb0587cb466b%40linutronix.de

[PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Vasilij Strassheim]

Add an initial DSA driver for SoC-e FPGA-based Ethernet switch IP cores.

The driver accesses the switch through a memory-mapped register
interface. During probe, it detects the core version and the licensed
and implemented port counts, and verifies that the synthesized core
provides DSA support. Port counts from 1 through 31 are supported.

Enable "DSA custom rules" feature for all frames and derive the phylink
capabilities from each port's phy-mode. Enabling this tagging feature of
the switch, places all user ports into CPU-directed operation: frames
received on user ports are forwarded exclusively to the CPU port and
tagged with their source port. Consequently, standalone ports remain
isolated.

This switch IP core also has an integrated MDIO controller (called MDIO
Bridge). Expose each configured hardware MDIO output as a separate
logical MDIO bus. The buses support Clause 22 and Clause 45 transactions
and serialize access to the shared MDIO controller.

Tested with a SoC-e MRS 25.01 IP core on a Xilinx ZynqMP platform.

Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
---
 drivers/net/dsa/Kconfig              |   2 +
 drivers/net/dsa/Makefile             |   1 +
 drivers/net/dsa/soce/Kconfig         |  12 +
 drivers/net/dsa/soce/Makefile        |   4 +
 drivers/net/dsa/soce/soce_dsa.h      |  44 ++++
 drivers/net/dsa/soce/soce_dsa_core.c | 297 +++++++++++++++++++++++++
 drivers/net/dsa/soce/soce_mdio.c     | 411 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/soce/soce_mdio.h     |  38 ++++
 8 files changed, 809 insertions(+)

diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index 8d8edcf89f10..1422b9c5bf85 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -94,6 +94,8 @@ source "drivers/net/dsa/ocelot/Kconfig"
 
 source "drivers/net/dsa/qca/Kconfig"
 
+source "drivers/net/dsa/soce/Kconfig"
+
 source "drivers/net/dsa/sja1105/Kconfig"
 
 source "drivers/net/dsa/xrs700x/Kconfig"
diff --git a/drivers/net/dsa/Makefile b/drivers/net/dsa/Makefile
index df849cd0e640..b1336d5b3989 100644
--- a/drivers/net/dsa/Makefile
+++ b/drivers/net/dsa/Makefile
@@ -27,4 +27,5 @@ obj-y				+= ocelot/
 obj-y				+= qca/
 obj-y				+= realtek/
 obj-y				+= sja1105/
+obj-y				+= soce/
 obj-y				+= xrs700x/
diff --git a/drivers/net/dsa/soce/Kconfig b/drivers/net/dsa/soce/Kconfig
new file mode 100644
index 000000000000..321da9835402
--- /dev/null
+++ b/drivers/net/dsa/soce/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0
+config NET_DSA_SOCE
+	tristate "SoC-e switches"
+	depends on NET_DSA
+	depends on OF
+	depends on HAS_IOMEM
+	select NET_DSA_TAG_SDSA
+	help
+	  This enables support for switches based on SoC-e IP cores.
+	  Frames are exchanged with the CPU port using the SDSA DSA tag protocol.
+	  The driver supports switch variants whose features and number of ports
+	  are selected at synthesis time and detected at runtime.
diff --git a/drivers/net/dsa/soce/Makefile b/drivers/net/dsa/soce/Makefile
new file mode 100644
index 000000000000..c2635fe19643
--- /dev/null
+++ b/drivers/net/dsa/soce/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_NET_DSA_SOCE) += soce_dsa.o
+soce_dsa-objs := soce_dsa_core.o soce_mdio.o
diff --git a/drivers/net/dsa/soce/soce_dsa.h b/drivers/net/dsa/soce/soce_dsa.h
new file mode 100644
index 000000000000..ef01304209e6
--- /dev/null
+++ b/drivers/net/dsa/soce/soce_dsa.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020-2026 System on Chip engineering, S.L.
+ * Copyright (c) 2026 Linutronix GmbH
+ * Author: Vasilij Strassheim <v.strassheim@linutronix.de>
+ */
+
+#ifndef __SOCE_DSA_H
+#define __SOCE_DSA_H
+
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+#define SOCE_MAX_NUM_PORTS 31
+#define SOCE_MAX_MDIO_ADDR 32
+#define SOCE_MAX_MDIO_OUTPUTS SOCE_MAX_NUM_PORTS
+
+struct dsa_switch;
+
+struct soce_mdio_ops {
+	int (*phy_read)(struct dsa_switch *ds, int mdio_output, int phy_addr,
+			int regnum);
+	int (*phy_write)(struct dsa_switch *ds, int mdio_output, int phy_addr,
+			 int regnum, u16 val);
+	int (*phy_read_c45)(struct dsa_switch *ds, int mdio_output, int phy_addr,
+			    int devad, int regnum);
+	int (*phy_write_c45)(struct dsa_switch *ds, int mdio_output, int phy_addr,
+			     int devad, int regnum, u16 val);
+};
+
+struct soce_dsa_local {
+	void __iomem *base_addr;
+	void __iomem *mdio_master_addr;
+	/* Serializes all logical buses sharing the MDIO master. */
+	struct mutex mdio_lock;
+	const struct soce_mdio_ops *mdio_ops;
+};
+
+struct soce_priv {
+	struct soce_dsa_local local;
+	struct dsa_switch *ds;
+};
+
+#endif /* __SOCE_DSA_H */
diff --git a/drivers/net/dsa/soce/soce_dsa_core.c b/drivers/net/dsa/soce/soce_dsa_core.c
new file mode 100644
index 000000000000..60f9f8c030e8
--- /dev/null
+++ b/drivers/net/dsa/soce/soce_dsa_core.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020-2026 System on Chip engineering, S.L.
+ * Copyright (c) 2026 Linutronix GmbH
+ * Author: Vasilij Strassheim <v.strassheim@linutronix.de>
+ */
+
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_net.h>
+#include <linux/phy.h>
+#include <linux/phylink.h>
+#include <linux/platform_device.h>
+
+#include <net/dsa.h>
+
+#include "soce_dsa.h"
+#include "soce_mdio.h"
+
+#define SOCE_CORE_VERSION_SHIFT			24
+#define SOCE_CORE_SUBVERSION_SHIFT		16
+#define SOCE_LICENSED_NUM_PORTS_MASK		GENMASK(31, 27)
+#define SOCE_IMPLEMENTED_NUM_PORTS_MASK		GENMASK(31, 27)
+#define SOCE_IMPLEMENTED_DSA			BIT(23)
+#define SOCE_DSA_REGS_BASE			0x1200
+#define SOCE_TAG_ALL_FRAMES_CTRL_OFFSET		(SOCE_DSA_REGS_BASE + 0x001c)
+#define SOCE_TAG_ALL_FRAMES_ENABLE		BIT(0)
+#define SOCE_CUSTOM_RULES_TAGGING_OFFSET	(SOCE_DSA_REGS_BASE + 0x0020)
+#define SOCE_CUSTOM_RULES_TAGGING_ENABLE	BIT(0)
+#define SOCE_MIN_CORE_VERSION			0x24
+#define SOCE_MIN_CORE_SUBVERSION		0x01
+
+static void soce_phylink_get_caps(struct dsa_switch *ds, int port,
+				  struct phylink_config *config)
+{
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	phy_interface_t mode;
+	int ret;
+
+	ret = of_get_phy_mode(dp->dn, &mode);
+	if (ret)
+		return;
+
+	if (phy_interface_mode_is_rgmii(mode))
+		phy_interface_set_rgmii(config->supported_interfaces);
+	else
+		__set_bit(mode, config->supported_interfaces);
+
+	config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE;
+
+	switch (mode) {
+	case PHY_INTERFACE_MODE_MII:
+		config->mac_capabilities |= MAC_10 | MAC_100;
+		break;
+	case PHY_INTERFACE_MODE_GMII:
+		config->mac_capabilities |= MAC_10 | MAC_100 | MAC_1000;
+		break;
+	case PHY_INTERFACE_MODE_RMII:
+		config->mac_capabilities |= MAC_10FD | MAC_100FD;
+		break;
+	default:
+		if (phy_interface_mode_is_rgmii(mode))
+			config->mac_capabilities |= MAC_10FD | MAC_100FD |
+						    MAC_1000FD;
+		break;
+	}
+}
+
+static const struct soce_mdio_ops soce_mdio_ops_c22_c45 = {
+	.phy_read	= soce_mdio_23_02_read,
+	.phy_write	= soce_mdio_23_02_write,
+	.phy_read_c45	= soce_mdio_23_02_read_c45,
+	.phy_write_c45	= soce_mdio_23_02_write_c45,
+};
+
+struct soce_probe_desc {
+	u32 core_version_offset;
+	u32 licensed_features_offset;
+	u32 implemented_features0_offset;
+	u32 mdio_master_offset;
+};
+
+static const struct soce_probe_desc soce_probe_desc_swip_00_04_0c_10 = {
+	.core_version_offset		= 0x0000,
+	.licensed_features_offset	= 0x0004,
+	.implemented_features0_offset	= 0x000c,
+	.mdio_master_offset		= 0x0200,
+};
+
+static int soce_sw_validate_core_version(u8 version, u8 subversion)
+{
+	if (version < SOCE_MIN_CORE_VERSION ||
+	    (version == SOCE_MIN_CORE_VERSION &&
+	     subversion < SOCE_MIN_CORE_SUBVERSION))
+		return -ENODEV;
+
+	return 0;
+}
+
+static void soce_sw_read_core_version(struct soce_dsa_local *local,
+				      const struct soce_probe_desc *probe_desc,
+				      u8 *version, u8 *subversion, u16 *revision)
+{
+	u32 regval;
+
+	regval = readl(local->base_addr + probe_desc->core_version_offset);
+	*version = (u8)(regval >> SOCE_CORE_VERSION_SHIFT);
+	*subversion = (u8)(regval >> SOCE_CORE_SUBVERSION_SHIFT);
+	*revision = (u16)regval;
+}
+
+static int soce_sw_detect_features(struct soce_dsa_local *local,
+				   const struct soce_probe_desc *probe_desc,
+				   u32 *numports)
+{
+	u32 implemented_numports;
+	u32 licensed_numports;
+	u32 regval;
+
+	regval = readl(local->base_addr + probe_desc->licensed_features_offset);
+	licensed_numports = FIELD_GET(SOCE_LICENSED_NUM_PORTS_MASK, regval);
+	if (!licensed_numports || licensed_numports > SOCE_MAX_NUM_PORTS)
+		return -EINVAL;
+
+	regval = readl(local->base_addr +
+		       probe_desc->implemented_features0_offset);
+	if (!(regval & SOCE_IMPLEMENTED_DSA))
+		return -ENODEV;
+
+	implemented_numports = FIELD_GET(SOCE_IMPLEMENTED_NUM_PORTS_MASK,
+					 regval);
+	if (!implemented_numports ||
+	    implemented_numports > licensed_numports)
+		return -EINVAL;
+
+	*numports = implemented_numports;
+
+	return 0;
+}
+
+static void soce_sw_enable_tagging(struct soce_dsa_local *local)
+{
+	u32 regval;
+
+	regval = readl(local->base_addr + SOCE_TAG_ALL_FRAMES_CTRL_OFFSET);
+	regval |= SOCE_TAG_ALL_FRAMES_ENABLE;
+	writel(regval, local->base_addr + SOCE_TAG_ALL_FRAMES_CTRL_OFFSET);
+
+	regval = readl(local->base_addr + SOCE_CUSTOM_RULES_TAGGING_OFFSET);
+	regval |= SOCE_CUSTOM_RULES_TAGGING_ENABLE;
+	writel(regval, local->base_addr + SOCE_CUSTOM_RULES_TAGGING_OFFSET);
+}
+
+static int soce_setup(struct dsa_switch *ds)
+{
+	struct soce_priv *priv = ds->priv;
+
+	soce_sw_enable_tagging(&priv->local);
+
+	return 0;
+}
+
+static enum dsa_tag_protocol soce_get_tag_protocol(struct dsa_switch *ds,
+						   int port,
+						   enum dsa_tag_protocol mprop)
+{
+	return DSA_TAG_PROTO_SDSA;
+}
+
+static const struct dsa_switch_ops soce_switch_ops = {
+	.get_tag_protocol	= soce_get_tag_protocol,
+	.setup			= soce_setup,
+	.phylink_get_caps	= soce_phylink_get_caps,
+};
+
+static int soce_sw_probe(struct platform_device *pdev)
+{
+	const struct soce_probe_desc *probe_desc;
+	struct device *dev = &pdev->dev;
+	struct soce_dsa_local *local;
+	struct soce_priv *priv;
+	u8 hw_subversion;
+	u16 hw_revision;
+	u32 hw_numports;
+	u8 hw_version;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->ds = devm_kzalloc(dev, sizeof(*priv->ds), GFP_KERNEL);
+	if (!priv->ds)
+		return -ENOMEM;
+
+	priv->ds->dev = dev;
+	priv->ds->priv = priv;
+	probe_desc = of_device_get_match_data(dev);
+	if (!probe_desc)
+		return -EINVAL;
+
+	local = &priv->local;
+	mutex_init(&local->mdio_lock);
+
+	local->base_addr = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(local->base_addr))
+		return PTR_ERR(local->base_addr);
+
+	soce_sw_read_core_version(local, probe_desc, &hw_version,
+				  &hw_subversion, &hw_revision);
+
+	ret = soce_sw_detect_features(local, probe_desc, &hw_numports);
+	if (ret) {
+		if (ret == -ENODEV)
+			dev_err(dev, "switch core does not implement DSA\n");
+		else
+			dev_err(dev,
+				"invalid licensed or implemented features register\n");
+		return ret;
+	}
+
+	ret = soce_sw_validate_core_version(hw_version, hw_subversion);
+	if (ret) {
+		dev_err(dev, "unsupported switch core version %.2X.%.2X.%.4X\n",
+			hw_version, hw_subversion, hw_revision);
+		return ret;
+	}
+
+	priv->ds->ops = &soce_switch_ops;
+	local->mdio_ops = &soce_mdio_ops_c22_c45;
+	local->mdio_master_addr =
+		local->base_addr + probe_desc->mdio_master_offset;
+
+	priv->ds->num_ports = hw_numports;
+	ret = soce_sw_register_mdio_buses(priv, dev, hw_numports);
+	if (ret)
+		return ret;
+
+	dev_set_drvdata(dev, priv);
+
+	ret = dsa_register_switch(priv->ds);
+	if (ret)
+		return ret;
+
+	dev_info(dev,
+		 "probed soce switch core version %02x.%02x.%04x with %u ports\n",
+		 hw_version, hw_subversion, hw_revision, hw_numports);
+
+	return 0;
+}
+
+static void soce_sw_remove(struct platform_device *pdev)
+{
+	struct soce_priv *priv = platform_get_drvdata(pdev);
+
+	if (!priv)
+		return;
+
+	dsa_unregister_switch(priv->ds);
+	platform_set_drvdata(pdev, NULL);
+}
+
+static void soce_sw_shutdown(struct platform_device *pdev)
+{
+	struct soce_priv *priv = platform_get_drvdata(pdev);
+
+	if (!priv)
+		return;
+
+	dsa_switch_shutdown(priv->ds);
+	platform_set_drvdata(pdev, NULL);
+}
+
+static const struct of_device_id soce_of_match[] = {
+	{ .compatible = "soce,swip-00-04-0c-10",
+	  .data = &soce_probe_desc_swip_00_04_0c_10 },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver soce_driver = {
+	.probe = soce_sw_probe,
+	.remove = soce_sw_remove,
+	.shutdown = soce_sw_shutdown,
+	.driver = {
+		.name = "soce-swip",
+		.of_match_table = soce_of_match,
+	},
+};
+
+module_platform_driver(soce_driver);
+MODULE_DEVICE_TABLE(of, soce_of_match);
+MODULE_AUTHOR("Vasilij Strassheim <v.strassheim@linutronix.de>");
+MODULE_DESCRIPTION("Driver for SoC-e ethernet switch family");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/dsa/soce/soce_mdio.c b/drivers/net/dsa/soce/soce_mdio.c
new file mode 100644
index 000000000000..692dcbe57d1a
--- /dev/null
+++ b/drivers/net/dsa/soce/soce_mdio.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020-2026 System on Chip engineering, S.L.
+ * Copyright (c) 2026 Linutronix GmbH
+ * Author: Vasilij Strassheim <v.strassheim@linutronix.de>
+ */
+
+#include <linux/bits.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/mdio.h>
+#include <linux/of.h>
+#include <linux/of_mdio.h>
+#include <linux/phy.h>
+
+#include <net/dsa.h>
+
+#include "soce_dsa.h"
+#include "soce_mdio.h"
+
+#define SOCE_MDIO_TIMEOUT 1000
+#define SOCE_MDIO_C22_REG_MAX 0x1f
+#define SOCE_MDIO_C45_DEVAD_MAX 0x1f
+#define SOCE_MDIO_C45_REG_MAX 0xffff
+
+static inline bool soce_mdio_output_valid(int mdio_output)
+{
+	return mdio_output >= 0 && mdio_output < SOCE_MAX_MDIO_OUTPUTS;
+}
+
+static inline bool soce_mdio_addr_valid(int phy_addr)
+{
+	return phy_addr >= 0 && phy_addr < SOCE_MAX_MDIO_ADDR;
+}
+
+static inline bool soce_mdio_c22_reg_valid(int regnum)
+{
+	return regnum >= 0 && regnum <= SOCE_MDIO_C22_REG_MAX;
+}
+
+static inline bool soce_mdio_c45_params_valid(int devad, int regnum)
+{
+	return devad >= 0 && devad <= SOCE_MDIO_C45_DEVAD_MAX &&
+	       regnum >= 0 && regnum <= SOCE_MDIO_C45_REG_MAX;
+}
+
+struct soce_mdio_bus {
+	struct dsa_switch *ds;
+	u32 mdio_output;
+};
+
+static int soce_mdio_read(struct mii_bus *bus, int addr, int reg)
+{
+	struct soce_mdio_bus *state = bus->priv;
+	struct soce_dsa_local *local;
+	struct soce_priv *priv;
+	int ret;
+
+	priv = state->ds->priv;
+	local = &priv->local;
+
+	if (!local->mdio_ops || !local->mdio_ops->phy_read)
+		return -EOPNOTSUPP;
+
+	if (!soce_mdio_addr_valid(addr))
+		return -EINVAL;
+
+	mutex_lock(&local->mdio_lock);
+	ret = local->mdio_ops->phy_read(state->ds, state->mdio_output, addr,
+					reg);
+	mutex_unlock(&local->mdio_lock);
+
+	return ret;
+}
+
+static int soce_mdio_write(struct mii_bus *bus, int addr, int reg, u16 val)
+{
+	struct soce_mdio_bus *state = bus->priv;
+	struct soce_dsa_local *local;
+	struct soce_priv *priv;
+	int ret;
+
+	priv = state->ds->priv;
+	local = &priv->local;
+
+	if (!local->mdio_ops || !local->mdio_ops->phy_write)
+		return -EOPNOTSUPP;
+
+	if (!soce_mdio_addr_valid(addr))
+		return -EINVAL;
+
+	mutex_lock(&local->mdio_lock);
+	ret = local->mdio_ops->phy_write(state->ds, state->mdio_output, addr,
+					 reg, val);
+	mutex_unlock(&local->mdio_lock);
+
+	return ret;
+}
+
+static int soce_mdio_read_c45(struct mii_bus *bus, int addr, int devad,
+			      int regnum)
+{
+	struct soce_mdio_bus *state = bus->priv;
+	struct soce_dsa_local *local;
+	struct soce_priv *priv;
+	int ret;
+
+	priv = state->ds->priv;
+	local = &priv->local;
+
+	if (!local->mdio_ops || !local->mdio_ops->phy_read_c45)
+		return -EOPNOTSUPP;
+
+	if (!soce_mdio_addr_valid(addr))
+		return -EINVAL;
+
+	mutex_lock(&local->mdio_lock);
+	ret = local->mdio_ops->phy_read_c45(state->ds, state->mdio_output,
+					    addr, devad, regnum);
+	mutex_unlock(&local->mdio_lock);
+
+	return ret;
+}
+
+static int soce_mdio_write_c45(struct mii_bus *bus, int addr, int devad,
+			       int regnum, u16 val)
+{
+	struct soce_mdio_bus *state = bus->priv;
+	struct soce_dsa_local *local;
+	struct soce_priv *priv;
+	int ret;
+
+	priv = state->ds->priv;
+	local = &priv->local;
+
+	if (!local->mdio_ops || !local->mdio_ops->phy_write_c45)
+		return -EOPNOTSUPP;
+
+	if (!soce_mdio_addr_valid(addr))
+		return -EINVAL;
+
+	mutex_lock(&local->mdio_lock);
+	ret = local->mdio_ops->phy_write_c45(state->ds, state->mdio_output,
+					     addr, devad, regnum, val);
+	mutex_unlock(&local->mdio_lock);
+
+	return ret;
+}
+
+static int soce_register_mdio_bus(struct soce_priv *priv, struct device *dev,
+				  struct device_node *mdio_np,
+				  u32 mdio_output)
+{
+	struct soce_mdio_bus *state;
+	struct mii_bus *bus;
+
+	bus = devm_mdiobus_alloc(dev);
+	if (!bus)
+		return -ENOMEM;
+
+	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	state->ds = priv->ds;
+	state->mdio_output = mdio_output;
+
+	bus->priv = state;
+	bus->name = "soce mdio";
+	bus->read = soce_mdio_read;
+	bus->write = soce_mdio_write;
+	bus->read_c45 = soce_mdio_read_c45;
+	bus->write_c45 = soce_mdio_write_c45;
+	/* ds->dst can be NULL during probe, before dsa_register_switch() */
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mdio-%u", dev_name(dev),
+		 mdio_output);
+	bus->parent = dev;
+
+	return devm_of_mdiobus_register(dev, bus, mdio_np);
+}
+
+int soce_sw_register_mdio_buses(struct soce_priv *priv, struct device *dev,
+				u32 numports)
+{
+	struct device_node *mdios_node;
+	u32 mdio_output;
+	int ret = 0;
+
+	mdios_node = of_get_child_by_name(dev->of_node, "mdios");
+	if (!mdios_node)
+		return 0;
+
+	for_each_available_child_of_node_scoped(mdios_node, mdio_node) {
+		ret = of_property_read_u32(mdio_node, "reg", &mdio_output);
+		if (ret) {
+			dev_err(dev, "missing reg in %pOF\n", mdio_node);
+			goto out_put_mdio;
+		}
+
+		if (mdio_output >= numports) {
+			dev_err(dev,
+				"MDIO output %u in %pOF exceeds hardware limit %u\n",
+				mdio_output, mdio_node, numports - 1);
+			ret = -EINVAL;
+			goto out_put_mdio;
+		}
+
+		ret = soce_register_mdio_bus(priv, dev, mdio_node, mdio_output);
+		if (ret)
+			goto out_put_mdio;
+	}
+
+out_put_mdio:
+	of_node_put(mdios_node);
+	return ret;
+}
+
+static int soce_mdio_23_02_wait_for_idle(struct dsa_switch *ds)
+{
+	struct soce_priv *priv = ds->priv;
+	struct soce_dsa_local *local;
+	void __iomem *ctrl;
+	u32 val;
+
+	local = &priv->local;
+	ctrl = local->mdio_master_addr + SOCE_MDIO_CTRL_OFFSET;
+
+	return readl_poll_timeout(ctrl, val,
+		!(val & (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET)), 10,
+		SOCE_MDIO_TIMEOUT * 1000);
+}
+
+static int soce_mdio_23_02_read_c22(struct dsa_switch *ds, int mdio_output,
+				    int phy_addr, int regnum)
+{
+	void __iomem *ctrl, *params, *read_reg;
+	struct soce_priv *priv = ds->priv;
+	struct soce_dsa_local *local;
+	u32 regvalue;
+	int ret;
+
+	local = &priv->local;
+	ctrl = local->mdio_master_addr + SOCE_MDIO_CTRL_OFFSET;
+	params = local->mdio_master_addr + SOCE_MDIO_23_02_PARAMS_OFFSET;
+	read_reg = local->mdio_master_addr + SOCE_MDIO_23_02_READ_OFFSET;
+
+	regvalue = (regnum << SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET) +
+		   (phy_addr << SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET);
+	writel(regvalue, params);
+
+	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
+		    (0x3 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +
+		    (0x0 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));
+	writel(regvalue, ctrl);
+
+	ret = soce_mdio_23_02_wait_for_idle(ds);
+	if (ret)
+		return ret;
+
+	return readl(read_reg) & 0xffff;
+}
+
+int soce_mdio_23_02_read_c45(struct dsa_switch *ds, int mdio_output,
+			     int phy_addr, int devad, int regnum)
+{
+	void __iomem *ctrl, *params, *read_reg, *write_reg;
+	struct soce_priv *priv = ds->priv;
+	struct soce_dsa_local *local;
+	u32 regvalue;
+	int ret;
+
+	if (!soce_mdio_output_valid(mdio_output) ||
+	    !soce_mdio_addr_valid(phy_addr) ||
+	    !soce_mdio_c45_params_valid(devad, regnum))
+		return -EINVAL;
+
+	local = &priv->local;
+	ctrl = local->mdio_master_addr + SOCE_MDIO_CTRL_OFFSET;
+	params = local->mdio_master_addr + SOCE_MDIO_23_02_PARAMS_OFFSET;
+	write_reg = local->mdio_master_addr + SOCE_MDIO_23_02_WRITE_OFFSET;
+	read_reg = local->mdio_master_addr + SOCE_MDIO_23_02_READ_OFFSET;
+
+	regvalue = (devad << SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET) +
+		   (phy_addr << SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET);
+	writel(regvalue, params);
+
+	writel(regnum, write_reg);
+
+	/* address cycle */
+	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
+		    (0x0 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));
+	writel(regvalue, ctrl);
+
+	ret = soce_mdio_23_02_wait_for_idle(ds);
+	if (ret)
+		return ret;
+
+	/* read cycle */
+	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
+		    (0x3 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));
+	writel(regvalue, ctrl);
+
+	ret = soce_mdio_23_02_wait_for_idle(ds);
+	if (ret)
+		return ret;
+
+	return readl(read_reg) & 0xffff;
+}
+
+int soce_mdio_23_02_read(struct dsa_switch *ds, int mdio_output, int phy_addr,
+			 int regnum)
+{
+	if (!soce_mdio_output_valid(mdio_output) ||
+	    !soce_mdio_addr_valid(phy_addr) ||
+	    !soce_mdio_c22_reg_valid(regnum))
+		return -EINVAL;
+
+	return soce_mdio_23_02_read_c22(ds, mdio_output, phy_addr, regnum);
+}
+
+static int soce_mdio_23_02_write_c22(struct dsa_switch *ds, int mdio_output,
+				     int phy_addr, int regnum, u16 val)
+{
+	void __iomem *ctrl, *params, *write_reg;
+	struct soce_priv *priv = ds->priv;
+	struct soce_dsa_local *local;
+	u32 regvalue;
+
+	local = &priv->local;
+	params = local->mdio_master_addr + SOCE_MDIO_23_02_PARAMS_OFFSET;
+	ctrl = local->mdio_master_addr + SOCE_MDIO_CTRL_OFFSET;
+	write_reg = local->mdio_master_addr + SOCE_MDIO_23_02_WRITE_OFFSET;
+
+	regvalue = (regnum << SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET) +
+		   (phy_addr << SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET);
+	writel(regvalue, params);
+
+	writel(val, write_reg);
+
+	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +
+		    (0x0 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));
+	writel(regvalue, ctrl);
+
+	return soce_mdio_23_02_wait_for_idle(ds);
+}
+
+int soce_mdio_23_02_write_c45(struct dsa_switch *ds, int mdio_output,
+			      int phy_addr, int devad, int regnum, u16 val)
+{
+	void __iomem *ctrl, *params, *write_reg;
+	struct soce_priv *priv = ds->priv;
+	struct soce_dsa_local *local;
+	u32 regvalue;
+	int ret;
+
+	if (!soce_mdio_output_valid(mdio_output) ||
+	    !soce_mdio_addr_valid(phy_addr) ||
+	    !soce_mdio_c45_params_valid(devad, regnum))
+		return -EINVAL;
+
+	local = &priv->local;
+	ctrl = local->mdio_master_addr + SOCE_MDIO_CTRL_OFFSET;
+	params = local->mdio_master_addr + SOCE_MDIO_23_02_PARAMS_OFFSET;
+	write_reg = local->mdio_master_addr + SOCE_MDIO_23_02_WRITE_OFFSET;
+
+	regvalue = (devad << SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET) +
+		   (phy_addr << SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET);
+	writel(regvalue, params);
+
+	writel(regnum, write_reg);
+
+	/* address cycle */
+	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
+		    (0x0 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));
+	writel(regvalue, ctrl);
+
+	ret = soce_mdio_23_02_wait_for_idle(ds);
+	if (ret)
+		return ret;
+
+	writel(val, write_reg);
+
+	/* write cycle */
+	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +
+		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));
+	writel(regvalue, ctrl);
+
+	return soce_mdio_23_02_wait_for_idle(ds);
+}
+
+int soce_mdio_23_02_write(struct dsa_switch *ds, int mdio_output, int phy_addr,
+			  int regnum, u16 val)
+{
+	if (!soce_mdio_output_valid(mdio_output) ||
+	    !soce_mdio_addr_valid(phy_addr) ||
+	    !soce_mdio_c22_reg_valid(regnum))
+		return -EINVAL;
+
+	return soce_mdio_23_02_write_c22(ds, mdio_output, phy_addr, regnum, val);
+}
diff --git a/drivers/net/dsa/soce/soce_mdio.h b/drivers/net/dsa/soce/soce_mdio.h
new file mode 100644
index 000000000000..f2b21fe50d79
--- /dev/null
+++ b/drivers/net/dsa/soce/soce_mdio.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2020-2026 System on Chip engineering, S.L.
+ * Copyright (c) 2026 Linutronix GmbH
+ * Author: Vasilij Strassheim <v.strassheim@linutronix.de>
+ */
+
+#ifndef __SOCE_MDIO_H
+#define __SOCE_MDIO_H
+
+#define SOCE_MDIO_CTRL_OFFSET 0x0000
+
+/* for newer versions */
+#define SOCE_MDIO_23_02_PARAMS_OFFSET		0x0004
+#define SOCE_MDIO_23_02_WRITE_OFFSET		0x0008
+#define SOCE_MDIO_23_02_READ_OFFSET		0x000c
+#define SOCE_MDIO_23_02_CTRL_BUS_OFFSET		16
+#define SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET	3
+#define SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET	1
+#define SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET	0
+
+#define SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET	8
+#define SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET		0
+
+struct device;
+struct soce_priv;
+
+int soce_mdio_23_02_read(struct dsa_switch *ds, int mdio_output, int phy_addr,
+			 int regnum);
+int soce_mdio_23_02_write(struct dsa_switch *ds, int mdio_output, int phy_addr,
+			  int regnum, u16 val);
+int soce_mdio_23_02_read_c45(struct dsa_switch *ds, int mdio_output,
+			     int phy_addr, int devad, int regnum);
+int soce_mdio_23_02_write_c45(struct dsa_switch *ds, int mdio_output,
+			      int phy_addr, int devad, int regnum, u16 val);
+int soce_sw_register_mdio_buses(struct soce_priv *priv, struct device *dev,
+				u32 numports);
+#endif /* __SOCE_MDIO_H */

-- 
2.39.5

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Andrew Lunn]

> +#define SOCE_MAX_NUM_PORTS 31
> +#define SOCE_MAX_MDIO_ADDR 32

Given what the binding says, that looks odd.

> +#define SOCE_MAX_MDIO_OUTPUTS SOCE_MAX_NUM_PORTS
> +
> +struct dsa_switch;
> +
> +struct soce_mdio_ops {
> +	int (*phy_read)(struct dsa_switch *ds, int mdio_output, int phy_addr,
> +			int regnum);
> +	int (*phy_write)(struct dsa_switch *ds, int mdio_output, int phy_addr,
> +			 int regnum, u16 val);
> +	int (*phy_read_c45)(struct dsa_switch *ds, int mdio_output, int phy_addr,
> +			    int devad, int regnum);
> +	int (*phy_write_c45)(struct dsa_switch *ds, int mdio_output, int phy_addr,
> +			     int devad, int regnum, u16 val);
> +};
> +
> +struct soce_dsa_local {
> +	void __iomem *base_addr;
> +	void __iomem *mdio_master_addr;
> +	/* Serializes all logical buses sharing the MDIO master. */
> +	struct mutex mdio_lock;

What is the MDIO master?

> +static const struct soce_mdio_ops soce_mdio_ops_c22_c45 = {
> +	.phy_read	= soce_mdio_23_02_read,
> +	.phy_write	= soce_mdio_23_02_write,
> +	.phy_read_c45	= soce_mdio_23_02_read_c45,
> +	.phy_write_c45	= soce_mdio_23_02_write_c45,
> +};

It seems like this is the only struct soce_mdio_ops. Does the IP
support different MDIO buses? Is this level of abstraction actually
needed?

> +static inline bool soce_mdio_output_valid(int mdio_output)
> +{
> +	return mdio_output >= 0 && mdio_output < SOCE_MAX_MDIO_OUTPUTS;
> +}

No inline functions in .c files. Let the compile decide.

> +static inline bool soce_mdio_addr_valid(int phy_addr)
> +{
> +	return phy_addr >= 0 && phy_addr < SOCE_MAX_MDIO_ADDR;
> +}
> +
> +static inline bool soce_mdio_c22_reg_valid(int regnum)
> +{
> +	return regnum >= 0 && regnum <= SOCE_MDIO_C22_REG_MAX;
> +}
> +
> +static inline bool soce_mdio_c45_params_valid(int devad, int regnum)
> +{
> +	return devad >= 0 && devad <= SOCE_MDIO_C45_DEVAD_MAX &&
> +	       regnum >= 0 && regnum <= SOCE_MDIO_C45_REG_MAX;
> +}

Do you see any other MDIO driver doing this sort of checking?

> +static int soce_mdio_read(struct mii_bus *bus, int addr, int reg)
> +{
> +	struct soce_mdio_bus *state = bus->priv;
> +	struct soce_dsa_local *local;
> +	struct soce_priv *priv;
> +	int ret;
> +
> +	priv = state->ds->priv;
> +	local = &priv->local;
> +
> +	if (!local->mdio_ops || !local->mdio_ops->phy_read)
> +		return -EOPNOTSUPP;
> +
> +	if (!soce_mdio_addr_valid(addr))
> +		return -EINVAL;
> +
> +	mutex_lock(&local->mdio_lock);
> +	ret = local->mdio_ops->phy_read(state->ds, state->mdio_output, addr,
> +					reg);
> +	mutex_unlock(&local->mdio_lock);

What is the lock protecting?

> +static int soce_register_mdio_bus(struct soce_priv *priv, struct device *dev,
> +				  struct device_node *mdio_np,
> +				  u32 mdio_output)
> +{
> +	struct soce_mdio_bus *state;
> +	struct mii_bus *bus;
> +
> +	bus = devm_mdiobus_alloc(dev);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	state->ds = priv->ds;
> +	state->mdio_output = mdio_output;

Can you think of a better name than mdio_output. It seems to be the
bus number?

> +
> +	bus->priv = state;
> +	bus->name = "soce mdio";
> +	bus->read = soce_mdio_read;
> +	bus->write = soce_mdio_write;
> +	bus->read_c45 = soce_mdio_read_c45;
> +	bus->write_c45 = soce_mdio_write_c45;
> +	/* ds->dst can be NULL during probe, before dsa_register_switch() */
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mdio-%u", dev_name(dev),
> +		 mdio_output);
> +	bus->parent = dev;
> +
> +	return devm_of_mdiobus_register(dev, bus, mdio_np);
> +}
> +int soce_mdio_23_02_read_c45(struct dsa_switch *ds, int mdio_output,
> +			     int phy_addr, int devad, int regnum)
> +{
> +	void __iomem *ctrl, *params, *read_reg, *write_reg;
> +	struct soce_priv *priv = ds->priv;
> +	struct soce_dsa_local *local;
> +	u32 regvalue;
> +	int ret;
> +
> +	if (!soce_mdio_output_valid(mdio_output) ||
> +	    !soce_mdio_addr_valid(phy_addr) ||
> +	    !soce_mdio_c45_params_valid(devad, regnum))
> +		return -EINVAL;

Hasn't this already been checked once?

	Andrew

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Vasilij Strassheim]

On Mon, 2026-09-07 at 21:28 +0200, Andrew Lunn wrote:
> > +#define SOCE_MAX_NUM_PORTS 31
> > +#define SOCE_MAX_MDIO_ADDR 32
> 
> Given what the binding says, that looks odd.
> 

The checks using these constants are unnecessary. I will remove them and
clarify the 31-port hardware limit.

> > +#define SOCE_MAX_MDIO_OUTPUTS SOCE_MAX_NUM_PORTS
> > +
> > +struct dsa_switch;
> > +
> > +struct soce_mdio_ops {
> > +	int (*phy_read)(struct dsa_switch *ds, int mdio_output, int phy_addr,
> > +			int regnum);
> > +	int (*phy_write)(struct dsa_switch *ds, int mdio_output, int phy_addr,
> > +			 int regnum, u16 val);
> > +	int (*phy_read_c45)(struct dsa_switch *ds, int mdio_output, int phy_addr,
> > +			    int devad, int regnum);
> > +	int (*phy_write_c45)(struct dsa_switch *ds, int mdio_output, int phy_addr,
> > +			     int devad, int regnum, u16 val);
> > +};
> > +
> > +struct soce_dsa_local {
> > +	void __iomem *base_addr;
> > +	void __iomem *mdio_master_addr;
> > +	/* Serializes all logical buses sharing the MDIO master. */
> > +	struct mutex mdio_lock;
> 
> What is the MDIO master?

It refers to the switch-integrated MDIO controller: one shared set of
MMIO transaction registers serving multiple selectable MDIO buses. The
switch documentation calls it an MDIO bridge. I will rename "master" to
"controller".

> 
> > +static const struct soce_mdio_ops soce_mdio_ops_c22_c45 = {
> > +	.phy_read	= soce_mdio_23_02_read,
> > +	.phy_write	= soce_mdio_23_02_write,
> > +	.phy_read_c45	= soce_mdio_23_02_read_c45,
> > +	.phy_write_c45	= soce_mdio_23_02_write_c45,
> > +};
> 
> It seems like this is the only struct soce_mdio_ops. Does the IP
> support different MDIO buses? Is this level of abstraction actually
> needed?
> 

Older IP versions used a different register layout. Since this driver
currently supports only one variant, I will remove the abstraction. It
can be added back when another variant is supported.

> > +static inline bool soce_mdio_output_valid(int mdio_output)
> > +{
> > +	return mdio_output >= 0 && mdio_output < SOCE_MAX_MDIO_OUTPUTS;
> > +}
> 
> No inline functions in .c files. Let the compile decide.

Yes, I will fix this along with some other issues reported by the
patchwork checks.

> 
> > +static inline bool soce_mdio_addr_valid(int phy_addr)
> > +{
> > +	return phy_addr >= 0 && phy_addr < SOCE_MAX_MDIO_ADDR;
> > +}
> > +
> > +static inline bool soce_mdio_c22_reg_valid(int regnum)
> > +{
> > +	return regnum >= 0 && regnum <= SOCE_MDIO_C22_REG_MAX;
> > +}
> > +
> > +static inline bool soce_mdio_c45_params_valid(int devad, int regnum)
> > +{
> > +	return devad >= 0 && devad <= SOCE_MDIO_C45_DEVAD_MAX &&
> > +	       regnum >= 0 && regnum <= SOCE_MDIO_C45_REG_MAX;
> > +}
> 
> Do you see any other MDIO driver doing this sort of checking?

No. I will remove all redundant checks. 

> 
> > +static int soce_mdio_read(struct mii_bus *bus, int addr, int reg)
> > +{
> > +	struct soce_mdio_bus *state = bus->priv;
> > +	struct soce_dsa_local *local;
> > +	struct soce_priv *priv;
> > +	int ret;
> > +
> > +	priv = state->ds->priv;
> > +	local = &priv->local;
> > +
> > +	if (!local->mdio_ops || !local->mdio_ops->phy_read)
> > +		return -EOPNOTSUPP;
> > +
> > +	if (!soce_mdio_addr_valid(addr))
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&local->mdio_lock);
> > +	ret = local->mdio_ops->phy_read(state->ds, state->mdio_output, addr,
> > +					reg);
> > +	mutex_unlock(&local->mdio_lock);
> 
> What is the lock protecting?

It serializes accesses to the shared MDIO controller transaction
registers across all MDIO buses.

> 
> > +static int soce_register_mdio_bus(struct soce_priv *priv, struct device *dev,
> > +				  struct device_node *mdio_np,
> > +				  u32 mdio_output)
> > +{
> > +	struct soce_mdio_bus *state;
> > +	struct mii_bus *bus;
> > +
> > +	bus = devm_mdiobus_alloc(dev);
> > +	if (!bus)
> > +		return -ENOMEM;
> > +
> > +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> > +	if (!state)
> > +		return -ENOMEM;
> > +
> > +	state->ds = priv->ds;
> > +	state->mdio_output = mdio_output;
> 
> Can you think of a better name than mdio_output. It seems to be the
> bus number?
> 

Yes, it is the bus selector. I will rename it to mdio_bus_id.

> > +
> > +	bus->priv = state;
> > +	bus->name = "soce mdio";
> > +	bus->read = soce_mdio_read;
> > +	bus->write = soce_mdio_write;
> > +	bus->read_c45 = soce_mdio_read_c45;
> > +	bus->write_c45 = soce_mdio_write_c45;
> > +	/* ds->dst can be NULL during probe, before dsa_register_switch() */
> > +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mdio-%u", dev_name(dev),
> > +		 mdio_output);
> > +	bus->parent = dev;
> > +
> > +	return devm_of_mdiobus_register(dev, bus, mdio_np);
> > +}
> > +int soce_mdio_23_02_read_c45(struct dsa_switch *ds, int mdio_output,
> > +			     int phy_addr, int devad, int regnum)
> > +{
> > +	void __iomem *ctrl, *params, *read_reg, *write_reg;
> > +	struct soce_priv *priv = ds->priv;
> > +	struct soce_dsa_local *local;
> > +	u32 regvalue;
> > +	int ret;
> > +
> > +	if (!soce_mdio_output_valid(mdio_output) ||
> > +	    !soce_mdio_addr_valid(phy_addr) ||
> > +	    !soce_mdio_c45_params_valid(devad, regnum))
> > +		return -EINVAL;
> 
> Hasn't this already been checked once?

Yes, I will remove this and other redundant checks. 

> 
> 	Andrew

Thanks,
Vasilij

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Andrew Lunn]

> > What is the MDIO master?
> 
> It refers to the switch-integrated MDIO controller: one shared set of
> MMIO transaction registers serving multiple selectable MDIO buses. The
> switch documentation calls it an MDIO bridge. I will rename "master" to
> "controller".

In Linux, we have the concept of MDIO muxes. You have one true MDIO
bus master, and then logic gates to connect it to many MDIO
busses. Generally, the MDIO data line has a set of analogue switches
to mux it to one of the many MDIO busses.

Having only one true MDIO bus master means you cannot do transactions
in parallel.

It seems like this is how this hardware works. The question is, should
we model it this way. Take a look at mdio-mux-mmioreg.c. Maybe the
MDIO bus driver can be placed into drivers/net/mdio, and a
mdio-mux-mmioreg stacked on top of it to provide the per port MDIO
busses. That will make the locking simpler, and remove a lot of code.

	Andrew

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Vasilij Strassheim]

On Tue, 2026-09-08 at 21:20 +0200, Andrew Lunn wrote:
> > > What is the MDIO master?
> > 
> > It refers to the switch-integrated MDIO controller: one shared set of
> > MMIO transaction registers serving multiple selectable MDIO buses. The
> > switch documentation calls it an MDIO bridge. I will rename "master" to
> > "controller".
> 
> In Linux, we have the concept of MDIO muxes. You have one true MDIO
> bus master, and then logic gates to connect it to many MDIO
> busses. Generally, the MDIO data line has a set of analogue switches
> to mux it to one of the many MDIO busses.
> 
> Having only one true MDIO bus master means you cannot do transactions
> in parallel.

That's true. I had already considered using an MDIO mux for v2, but
decided to retain the existing implementation while addressing the other
feedback first.

> It seems like this is how this hardware works. The question is, should
> we model it this way. Take a look at mdio-mux-mmioreg.c. Maybe the
> MDIO bus driver can be placed into drivers/net/mdio, and a
> mdio-mux-mmioreg stacked on top of it to provide the per port MDIO
> busses. That will make the locking simpler, and remove a lot of code.
> 

For the next revision, I will try to implement this model. The MDIO
controller will become a standalone driver under mdio and register a
single parent mii_bus. A generic mdio-mux-mmioreg instance will then
provide the per-port child buses. Something like this:

ethernet-switch@xy {
    /* ... */

    mdio_parent: mdio@204 {
        compatible = "soce,swip-mdio-23-02";
        reg = <0x204 0xc>, <0x200 0x4>;
        reg-names = "data", "control";
    };

    mdio-mux@200 {
        compatible = "mdio-mux-mmioreg", "mdio-mux";
        reg = <0x200 0x4>;
        mux-mask = <0x1f0000>;
        mdio-parent-bus = <&mdio_parent>;
        #address-cells = <1>;
        #size-cells = <0>;

        mdio@0 {
            reg = <0>;
            /* PHYs connected to port 0 */
        };

        mdio@10000 {
            reg = <0x10000>;
            /* PHYs connected to port 1 */
        };
    };

    /* ethernet-ports ... */
};

This requires a separate binding and compatible string for the MDIO
controller. I currently have soce,swip-mdio.yaml and the
register-layout-specific soce,swip-mdio-23-02 in mind, although I still
need to consider the naming.

> 	Andrew

Thanks,
Vasilij

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Andrew Lunn]

> ethernet-switch@xy {
>     /* ... */
> 
>     mdio_parent: mdio@204 {
>         compatible = "soce,swip-mdio-23-02";
>         reg = <0x204 0xc>, <0x200 0x4>;
>         reg-names = "data", "control";
>     };
> 
>     mdio-mux@200 {
>         compatible = "mdio-mux-mmioreg", "mdio-mux";
>         reg = <0x200 0x4>;
>         mux-mask = <0x1f0000>;
>         mdio-parent-bus = <&mdio_parent>;
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         mdio@0 {
>             reg = <0>;
>             /* PHYs connected to port 0 */
>         };
> 
>         mdio@10000 {
>             reg = <0x10000>;
>             /* PHYs connected to port 1 */

These comments are probably wrong, as i suggested in my previous
reply. There is no reason that port 0 PHY could not be on bus 10000.

I've no idea if it will work, but maybe try:

>         reg = <0x203 0x1>;
>         mux-mask = <0x1f>;

and then you get better bus numbers?

    Andrew

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Andrew Lunn]

> +#define SOCE_CORE_VERSION_SHIFT			24
> +#define SOCE_CORE_SUBVERSION_SHIFT		16
> +#define SOCE_LICENSED_NUM_PORTS_MASK		GENMASK(31, 27)
> +#define SOCE_IMPLEMENTED_NUM_PORTS_MASK		GENMASK(31, 27)
> +#define SOCE_IMPLEMENTED_DSA			BIT(23)
> +#define SOCE_DSA_REGS_BASE			0x1200
> +#define SOCE_TAG_ALL_FRAMES_CTRL_OFFSET		(SOCE_DSA_REGS_BASE + 0x001c)
> +#define SOCE_TAG_ALL_FRAMES_ENABLE		BIT(0)
> +#define SOCE_CUSTOM_RULES_TAGGING_OFFSET	(SOCE_DSA_REGS_BASE + 0x0020)
> +#define SOCE_CUSTOM_RULES_TAGGING_ENABLE	BIT(0)
> +#define SOCE_MIN_CORE_VERSION			0x24
> +#define SOCE_MIN_CORE_SUBVERSION		0x01

Could you fully describe the feature registers, so we have an idea
what is actually there.

> +struct soce_probe_desc {
> +	u32 core_version_offset;
> +	u32 licensed_features_offset;
> +	u32 implemented_features0_offset;
> +	u32 mdio_master_offset;
> +};
> +
> +static const struct soce_probe_desc soce_probe_desc_swip_00_04_0c_10 = {
> +	.core_version_offset		= 0x0000,
> +	.licensed_features_offset	= 0x0004,
> +	.implemented_features0_offset	= 0x000c,
> +	.mdio_master_offset		= 0x0200,
> +};

How fixed/variable are these? I'm just thinking there may be too much
abstraction here. To some extent, we leave abstractions out until they
are needed. If you know there are other devices out there which have
these at other addresses, then O.K. But if not, i would keep it KISS.

> +static int soce_sw_detect_features(struct soce_dsa_local *local,
> +				   const struct soce_probe_desc *probe_desc,
> +				   u32 *numports)
> +{
> +	u32 implemented_numports;
> +	u32 licensed_numports;
> +	u32 regval;
> +
> +	regval = readl(local->base_addr + probe_desc->licensed_features_offset);
> +	licensed_numports = FIELD_GET(SOCE_LICENSED_NUM_PORTS_MASK, regval);
> +	if (!licensed_numports || licensed_numports > SOCE_MAX_NUM_PORTS)
> +		return -EINVAL;
> +
> +	regval = readl(local->base_addr +
> +		       probe_desc->implemented_features0_offset);
> +	if (!(regval & SOCE_IMPLEMENTED_DSA))
> +		return -ENODEV;

I find it useful to have macros like SOCE_IMPLEMENTED_DSA indicate
they apply to features0. It makes it easier to spot dumb typos when
you apply it to features1, not features0.

I also wounder at the name. Does the data sheet really call it DSA?

> +
> +	implemented_numports = FIELD_GET(SOCE_IMPLEMENTED_NUM_PORTS_MASK,
> +					 regval);
> +	if (!implemented_numports ||
> +	    implemented_numports > licensed_numports)
> +		return -EINVAL;
> +
> +	*numports = implemented_numports;

How is this going to scale when you need to look at more bits in these
registers? It seems like at some point you are going to need to pass a
structure to be filled in.

> +static int soce_register_mdio_bus(struct soce_priv *priv, struct device *dev,
> +				  struct device_node *mdio_np,
> +				  u32 mdio_output)
> +{
> +	struct soce_mdio_bus *state;
> +	struct mii_bus *bus;
> +
> +	bus = devm_mdiobus_alloc(dev);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	state->ds = priv->ds;
> +	state->mdio_output = mdio_output;
> +
> +	bus->priv = state;
> +	bus->name = "soce mdio";
> +	bus->read = soce_mdio_read;
> +	bus->write = soce_mdio_write;
> +	bus->read_c45 = soce_mdio_read_c45;
> +	bus->write_c45 = soce_mdio_write_c45;
> +	/* ds->dst can be NULL during probe, before dsa_register_switch() */
> +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mdio-%u", dev_name(dev),
> +		 mdio_output);

Why the comment. How is it relevant?

> +static int soce_mdio_23_02_read_c22(struct dsa_switch *ds, int mdio_output,
> +				    int phy_addr, int regnum)
> +{
> +	void __iomem *ctrl, *params, *read_reg;
> +	struct soce_priv *priv = ds->priv;
> +	struct soce_dsa_local *local;
> +	u32 regvalue;
> +	int ret;
> +
> +	local = &priv->local;
> +	ctrl = local->mdio_master_addr + SOCE_MDIO_CTRL_OFFSET;
> +	params = local->mdio_master_addr + SOCE_MDIO_23_02_PARAMS_OFFSET;
> +	read_reg = local->mdio_master_addr + SOCE_MDIO_23_02_READ_OFFSET;
> +
> +	regvalue = (regnum << SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET) +
> +		   (phy_addr << SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET);
> +	writel(regvalue, params);
> +
> +	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
> +		    (0x3 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +

0x3 means read?

> +		    (0x0 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +

0x0 means C22

> +		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));

What does 0x1 mean? Please add some #defines for theses to act as
documentation.

	Andrew

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Vasilij Strassheim]

On Tue, 2026-09-08 at 02:37 +0200, Andrew Lunn wrote:
> > +#define SOCE_CORE_VERSION_SHIFT			24
> > +#define SOCE_CORE_SUBVERSION_SHIFT		16
> > +#define SOCE_LICENSED_NUM_PORTS_MASK		GENMASK(31, 27)
> > +#define SOCE_IMPLEMENTED_NUM_PORTS_MASK		GENMASK(31, 27)
> > +#define SOCE_IMPLEMENTED_DSA			BIT(23)
> > +#define SOCE_DSA_REGS_BASE			0x1200
> > +#define SOCE_TAG_ALL_FRAMES_CTRL_OFFSET		(SOCE_DSA_REGS_BASE + 0x001c)
> > +#define SOCE_TAG_ALL_FRAMES_ENABLE		BIT(0)
> > +#define SOCE_CUSTOM_RULES_TAGGING_OFFSET	(SOCE_DSA_REGS_BASE + 0x0020)
> > +#define SOCE_CUSTOM_RULES_TAGGING_ENABLE	BIT(0)
> > +#define SOCE_MIN_CORE_VERSION			0x24
> > +#define SOCE_MIN_CORE_SUBVERSION		0x01
> 
> Could you fully describe the feature registers, so we have an idea
> what is actually there.
> 

Unfortunately, I cannot share the complete vendor documentation, but I
can provide an overview of the feature register fields and bits used by
the driver, as well as the surrounding registers and bits. This should
help to get an idea.
Each synthesized optional feature has its own register block. The
additional description focuses on the MDIO, and DSA blocks which are
relevant to this driver.

---
Global register block at 0x0000 (three registers currently used)
	0x00 Core Version - Major and minor revision number (RO)
	0x04 Licensed Features (RO)
	0x08 Reserved
	0x0C GUI Configuration - Implemented Features 0 (RO)
	0x10 GUI Configuration - Implemented Features 1 (RO)
	0x14 Reserved
	0x18 Node Address - MAC Address (31-0)
	0x1C Node Address - MAC Address (47-32)
	0x20 Node Address - IPv4 Address
	0x24 Node Address - IPv6 Address (31-0)
	0x28 Node Address - IPv6 Address (63-32)
	0x2C Node Address - IPv6 Address (95-64)
	0x30 Node Address - IPv6 Address (127-96)
	0x34 Switch Configuration - Maximum Frame Length
	0x38 Reserved
	0x3C Switch Configuration - Input Timestamp Addition
	0x40 Reserved
	0x44 Switch Configuration - Input Timestamp Removal
	0x48 Reserved
	0x4C Switch Configuration - Architecture Selection

	Core Version (RO)
	[31:24] - Major version number
	[23:16] - Minor version number
	[15:8] - Major revision number
	[7:0] - Minor revision number

	Licensed Features Register (RO) (Bits specify if the given
	feature is enabled by license):
	[31:27] - Maximum Number of Ports: These bits specify the maximum
	number of ports licensed
	[26:15] - Reserved
	- ACL: Access Control List (ACL) feature
	- LLDP: LLDP feature
	- IEEE 802.1X
	- IEEE 802.1AX-2008 - Link Aggregation
	- IEEE 802.3br/IEEE 802.1Qbu - Preemption
	- IEC 61158-4-2 - DLR
	- IEC 62439-2 - MRP
	- IGMP Snooping
	- Port mirroring
	- IEEE 802.1s - MSTP
	- IEEE 802.1w - RSTP
	- IEEE 1588 - Transparent Clock
	- Switch mode: managed or unmanaged
	- Secure Chip
	- Time Limitation

	Implemented Features 0 Register (RO, Default value from GUI,
	Bits specify if the given feature is synthesized):
	[31:27] - Number of Ports: These bits specify the number of
	ports implemented.
	- IEEE 802.1X
	- IEEE 802.1AX-2008 - Link Aggregation
	- IEEE 802.3br/IEEE 802.1Qbu - Preemption
	[23] - DSA: This bit specifies if the DSA functionality is
	implemented.
	- Port Mirroring:
	- IEEE 802.1s - MSTP
	- IEEE 802.1w - RSTP
	- MAC Level Ingress Frame Filtering
	- Egress Frame Rate Limiting
	- Egress Throughput Rate Limiting
	- MAC Ingress Frame Rate Limiting
	- Ingress Throughput Rate Limiting
	- IGMP Snooping
	- Multicast Filtering
	- MAC Table Type: (SVL/IVL/SVL+IVL)
	- Static Filtering Database
	- Port-based VLAN
	- IEEE 1588 - Transparent Clock Layer 3
	- IEEE 1588 - Transparent Clock P2P calculation
	- IEEE 1588 - Transparent Clock
	- Jumbo Frame
	- QoS - Priorities
	- Statistic Counters

	Implemented Features 1 Register (RO, Default value from GUI,
	Bits specify if the given feature is synthesized):
	- Redundancy Protocol (PRP/HSR/HSR+PRP)
	- IEC 61158-4-2 - DLR
	- IEC 62439-2 - MRP
	- ACL: Access Control List
	- LLDP

---
MDIO Bridge Registers, base address 0x0200
	0x00 MDIO - Control
	0x04 MDIO - Address / Device Type
	0x08 MDIO - Register Address / Write Data
	0x0C MDIO - Read Data

	MDIO - Control
	[26:16] - Bus selection: These bits specify the port number over
	which MDIO operation is performed.
	[4:3] - Transaction Type: These bits specify the type of MDIO
	transaction to be performed.
		0x0 - MDIO address operation (only valid when Clause 45
		is selected).
		0x1 - MDIO write operation.
		0x3 - MDIO read operation.
	[1] - MDIO Clause: If '0', Clause 22, if '1', Clause 45
	[0] - Operation Status: 0: Ready / 1: Operation in progress. Set
	to '1' to start a new operation.

	MDIO - Address / Device Type
	[12:8] - Register Address (if Clause 22)/Device Type (if Clause45)
	[4:0] - PHY Address: These bits specify the PHY address

	MDIO - Register Address / Write Data
	[15:0] - Register Address / Write Data: Depending on Clause 22/45
	and transaction type.

	MDIO - Read Data
	[15:0] - Read Data: Data sampled after a MDIO read transaction.

---
DSA Frame Tagging Registers, base address 0x1200 (Mostly R/W access if
the feature is available. Default values 0 or from synthesis GUI.)
	0x00 CPU Ports (RO)
	0x04 DSA Predefined Rules - Control
	0x08 DSA Predefined Rules - Index
	0x0C DSA Predefined Rules - CPU Port List
	0x10-0x18 Reserved
	0x1C DSA Tag All - Control ([0] - If '1', all frames
	transmitted to the CPU port(s) will be DSA tagged. If '0', only
	those frames identified as DSA will be tagged.)
	0x20 DSA Custom Rules - Enable ([0] - Globally enable or
	disable the Custom DSA Tagging functionality)
	0x24 DSA Custom Rules - Control
	0x28 DSA Custom Rules - Index
	0x2C DSA Custom Rules - Destination MAC Address LO
	0x30 DSA Custom Rules - Destination MAC Address HI
	0x34 DSA Custom Rules - Ethertype
	0x38 DSA Custom Rules - Parameter Mask
	0x3C DSA Custom Rules - CPU Port List
	0x40 DSA Custom Rules - Trap/Mirror

While preparing this description I found a mismatch with MDIO Bus
selection Bits 26:16 in code. I will fix this for next version.


> > +struct soce_probe_desc {
> > +	u32 core_version_offset;
> > +	u32 licensed_features_offset;
> > +	u32 implemented_features0_offset;
> > +	u32 mdio_master_offset;
> > +};
> > +
> > +static const struct soce_probe_desc soce_probe_desc_swip_00_04_0c_10 = {
> > +	.core_version_offset		= 0x0000,
> > +	.licensed_features_offset	= 0x0004,
> > +	.implemented_features0_offset	= 0x000c,
> > +	.mdio_master_offset		= 0x0200,
> > +};
> 
> How fixed/variable are these? I'm just thinking there may be too much
> abstraction here. To some extent, we leave abstractions out until they
> are needed. If you know there are other devices out there which have
> these at other addresses, then O.K. But if not, i would keep it KISS.
> 

Right. I introduced the descriptor to make potential future
register-layout variants easier to support, but there is currently no
concrete variant that requires it.
Will fix this.

> > +static int soce_sw_detect_features(struct soce_dsa_local *local,
> > +				   const struct soce_probe_desc *probe_desc,
> > +				   u32 *numports)
> > +{
> > +	u32 implemented_numports;
> > +	u32 licensed_numports;
> > +	u32 regval;
> > +
> > +	regval = readl(local->base_addr + probe_desc->licensed_features_offset);
> > +	licensed_numports = FIELD_GET(SOCE_LICENSED_NUM_PORTS_MASK, regval);
> > +	if (!licensed_numports || licensed_numports > SOCE_MAX_NUM_PORTS)
> > +		return -EINVAL;
> > +
> > +	regval = readl(local->base_addr +
> > +		       probe_desc->implemented_features0_offset);
> > +	if (!(regval & SOCE_IMPLEMENTED_DSA))
> > +		return -ENODEV;
> 
> I find it useful to have macros like SOCE_IMPLEMENTED_DSA indicate
> they apply to features0. It makes it easier to spot dumb typos when
> you apply it to features1, not features0.
> 

Agreed. I will do it for the next version.

> I also wounder at the name. Does the data sheet really call it DSA?
> 

Yes, there is even a chapter describing Distributed Switch Architecture
(DSA) as "a subsystem that was primarily designed to support specific
vendor switches using Linux, ..." followed by information about "custom
DSA tagging".

> > +
> > +	implemented_numports = FIELD_GET(SOCE_IMPLEMENTED_NUM_PORTS_MASK,
> > +					 regval);
> > +	if (!implemented_numports ||
> > +	    implemented_numports > licensed_numports)
> > +		return -EINVAL;
> > +
> > +	*numports = implemented_numports;
> 
> How is this going to scale when you need to look at more bits in these
> registers? It seems like at some point you are going to need to pass a
> structure to be filled in.
> 

Yes, I agree that a feature structure will be needed once more fields
are used. I would prefer to keep the current interface for now and
introduce struct soce_features when another detected feature needs to be
retained. At the moment, it would contain only a single member.

> > +static int soce_register_mdio_bus(struct soce_priv *priv, struct device *dev,
> > +				  struct device_node *mdio_np,
> > +				  u32 mdio_output)
> > +{
> > +	struct soce_mdio_bus *state;
> > +	struct mii_bus *bus;
> > +
> > +	bus = devm_mdiobus_alloc(dev);
> > +	if (!bus)
> > +		return -ENOMEM;
> > +
> > +	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
> > +	if (!state)
> > +		return -ENOMEM;
> > +
> > +	state->ds = priv->ds;
> > +	state->mdio_output = mdio_output;
> > +
> > +	bus->priv = state;
> > +	bus->name = "soce mdio";
> > +	bus->read = soce_mdio_read;
> > +	bus->write = soce_mdio_write;
> > +	bus->read_c45 = soce_mdio_read_c45;
> > +	bus->write_c45 = soce_mdio_write_c45;
> > +	/* ds->dst can be NULL during probe, before dsa_register_switch() */
> > +	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mdio-%u", dev_name(dev),
> > +		 mdio_output);
> 
> Why the comment. How is it relevant?

I missed to update or remove comments here and on other places while
rewriting the code. I will fix all of them for next version.

> 
> > +static int soce_mdio_23_02_read_c22(struct dsa_switch *ds, int mdio_output,
> > +				    int phy_addr, int regnum)
> > +{
> > +	void __iomem *ctrl, *params, *read_reg;
> > +	struct soce_priv *priv = ds->priv;
> > +	struct soce_dsa_local *local;
> > +	u32 regvalue;
> > +	int ret;
> > +
> > +	local = &priv->local;
> > +	ctrl = local->mdio_master_addr + SOCE_MDIO_CTRL_OFFSET;
> > +	params = local->mdio_master_addr + SOCE_MDIO_23_02_PARAMS_OFFSET;
> > +	read_reg = local->mdio_master_addr + SOCE_MDIO_23_02_READ_OFFSET;
> > +
> > +	regvalue = (regnum << SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET) +
> > +		   (phy_addr << SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET);
> > +	writel(regvalue, params);
> > +
> > +	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
> > +		    (0x3 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +
> 
> 0x3 means read?
> 
> > +		    (0x0 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +
> 
> 0x0 means C22
> 
> > +		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));
> 
> What does 0x1 mean? Please add some #defines for theses to act as
> documentation.

This code will be replaced as part of mentioned MDIO controller and mux
rework. I will address this there by adding descriptive definitions.

> 
> 	Andrew

Thanks,
Vasilij

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Andrew Lunn]

Thanks for this list.

> ---
> Global register block at 0x0000 (three registers currently used)
> 	0x00 Core Version - Major and minor revision number (RO)
> 	0x04 Licensed Features (RO)
> 	0x08 Reserved
> 	0x0C GUI Configuration - Implemented Features 0 (RO)
> 	0x10 GUI Configuration - Implemented Features 1 (RO)

GUI? I assume this is not Graphical User Interface, but something
else?

> 	Licensed Features Register (RO) (Bits specify if the given
> 	feature is enabled by license):
> 	[31:27] - Maximum Number of Ports: These bits specify the maximum
> 	number of ports licensed
> 	[26:15] - Reserved
> 	- ACL: Access Control List (ACL) feature
> 	- LLDP: LLDP feature
> 	- IEEE 802.1X
> 	- IEEE 802.1AX-2008 - Link Aggregation
> 	- IEEE 802.3br/IEEE 802.1Qbu - Preemption
> 	- IEC 61158-4-2 - DLR
> 	- IEC 62439-2 - MRP
> 	- IGMP Snooping
> 	- Port mirroring
> 	- IEEE 802.1s - MSTP
> 	- IEEE 802.1w - RSTP
> 	- IEEE 1588 - Transparent Clock
> 	- Switch mode: managed or unmanaged

This is probably something which should be tested in the probe
method. No point loading the driver if it cannot be managed.

> 	- Secure Chip
> 	- Time Limitation

What do these two mean?

> 	MDIO - Control
> 	[26:16] - Bus selection: These bits specify the port number over
> 	which MDIO operation is performed.

10 bits, which allows more MDIO busses than ports!

> > I also wounder at the name. Does the data sheet really call it DSA?
> > 
> 
> Yes, there is even a chapter describing Distributed Switch Architecture
> (DSA) as "a subsystem that was primarily designed to support specific
> vendor switches using Linux, ..." followed by information about "custom
> DSA tagging".

Does the switch support the D in DSA?

> Yes, I agree that a feature structure will be needed once more fields
> are used. I would prefer to keep the current interface for now and
> introduce struct soce_features when another detected feature needs to be
> retained. At the moment, it would contain only a single member.

O.K.

	Andrew

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Vasilij Strassheim]

On Thu, 2026-09-10 at 17:07 +0200, Andrew Lunn wrote:
> Thanks for this list.
> 
> > ---
> > Global register block at 0x0000 (three registers currently used)
> > 	0x00 Core Version - Major and minor revision number (RO)
> > 	0x04 Licensed Features (RO)
> > 	0x08 Reserved
> > 	0x0C GUI Configuration - Implemented Features 0 (RO)
> > 	0x10 GUI Configuration - Implemented Features 1 (RO)
> 
> GUI? I assume this is not Graphical User Interface, but something
> else?
> 

It actually stands for Graphical User Interface. This refers to
configuration in a graphical IDE, where the bitstream is customised and
synthesized.
"Implemented Features" can be enabled/disabled and configured there by
clicking on the respective checkboxes or options within the graphical
interface.

> > 	Licensed Features Register (RO) (Bits specify if the given
> > 	feature is enabled by license):
> > 	[31:27] - Maximum Number of Ports: These bits specify the maximum
> > 	number of ports licensed
> > 	[26:15] - Reserved
> > 	- ACL: Access Control List (ACL) feature
> > 	- LLDP: LLDP feature
> > 	- IEEE 802.1X
> > 	- IEEE 802.1AX-2008 - Link Aggregation
> > 	- IEEE 802.3br/IEEE 802.1Qbu - Preemption
> > 	- IEC 61158-4-2 - DLR
> > 	- IEC 62439-2 - MRP
> > 	- IGMP Snooping
> > 	- Port mirroring
> > 	- IEEE 802.1s - MSTP
> > 	- IEEE 802.1w - RSTP
> > 	- IEEE 1588 - Transparent Clock
> > 	- Switch mode: managed or unmanaged
> 
> This is probably something which should be tested in the probe
> method. No point loading the driver if it cannot be managed.
> 

"DSA" functionality is already tested and according to the
documentation, this excludes unmanaged variants. Therefore, I wouldn't
add an additional test for this.

> > 	- Secure Chip
> > 	- Time Limitation
> 
> What do these two mean?
> 

Took me some time to find this info. These are license-protection
features. "Security Chip" authenticates the IP license using an external
1-Wire security device, while "Time Limitation" restricts evaluation
licenses to a configured operating period. Neither is relevant to the
driver.

> > 	MDIO - Control
> > 	[26:16] - Bus selection: These bits specify the port number over
> > 	which MDIO operation is performed.
> 
> 10 bits, which allows more MDIO busses than ports!

Actually 11 bits. However, too much. This may be a documentation error,
since the original driver also used only bits 20:16. I will clarify
this and if it's true, somehow address it as part of the MDIO/mux
rework.

> 
> > > I also wounder at the name. Does the data sheet really call it DSA?
> > > 
> > 
> > Yes, there is even a chapter describing Distributed Switch Architecture
> > (DSA) as "a subsystem that was primarily designed to support specific
> > vendor switches using Linux, ..." followed by information about "custom
> > DSA tagging".
> 
> Does the switch support the D in DSA?
> 

Not as far as I understand. The hardware provides custom frame tagging
between the CPU and a single switch. I found no indication that it
supports cascaded switch trees or encodes a target switch identity in
the tag.

> 
Thanks,
Vasilij

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

On Thu Sep 03 2026, Vasilij Strassheim wrote:
> +static const struct of_device_id soce_of_match[] = {
> +	{ .compatible = "soce,swip-00-04-0c-10",
> +	  .data = &soce_probe_desc_swip_00_04_0c_10 },
> +	{ /* sentinel */ },
> +};

In case you repost, remove the comma ;-). See

 https://lore.kernel.org/netdev/20260904043724.2955444-1-s-vadapalli@ti.com/

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[Vasilij Strassheim]

On Tue, 2026-09-08 at 10:25 +0200, Kurt Kanzenbach wrote:
> On Thu Sep 03 2026, Vasilij Strassheim wrote:
> > +static const struct of_device_id soce_of_match[] = {
> > +	{ .compatible = "soce,swip-00-04-0c-10",
> > +	  .data = &soce_probe_desc_swip_00_04_0c_10 },
> > +	{ /* sentinel */ },
> > +};
> 
> In case you repost, remove the comma ;-). See
> 
>  https://lore.kernel.org/netdev/20260904043724.2955444-1-s-vadapalli@ti.com/
> 

I'll do that.
Thanks for pointing that out, I would have probably missed it.

Cheers,
Vasilij

Re: [PATCH net-next v2 4/4] net: dsa: soce: Add basic support for SoC-e switch IP cores

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 2 · Low: 1

- [Medium] The MDIO master transaction sequence never verifies that the
  shared controller is idle before programming PARAMS/WRITE and kicking…
- [Medium] soce_setup() arms the switch's "tag all frames" and "DSA
  custom rules tagging" bits, but the driver provides no .teardown…
- [Low] The compatible string and its binding description declare a
  register layout with "the core version register at offset 0x00 and…

--

net: dsa: soce: Add basic support for SoC-e switch IP cores

This adds a new DSA driver for SoC-e FPGA switch IP cores, detecting the
core version and port counts from memory-mapped registers at probe time
and enabling the SDSA tagging feature. It also exposes each hardware MDIO
output of the integrated MDIO bridge as a separate logical mii_bus.

A few questions about the register descriptor, the MDIO timeout handling
and the missing teardown path.

> diff --git a/drivers/net/dsa/soce/soce_dsa_core.c b/drivers/net/dsa/soce/soce_dsa_core.c
> new file mode 100644
> index 0000000000000..60f9f8c030e88
> --- /dev/null
> +++ b/drivers/net/dsa/soce/soce_dsa_core.c

[ ... ]

> +struct soce_probe_desc {
> +	u32 core_version_offset;
> +	u32 licensed_features_offset;
> +	u32 implemented_features0_offset;
> +	u32 mdio_master_offset;
> +};
> +
> +static const struct soce_probe_desc soce_probe_desc_swip_00_04_0c_10 = {
> +	.core_version_offset		= 0x0000,
> +	.licensed_features_offset	= 0x0004,
> +	.implemented_features0_offset	= 0x000c,
> +	.mdio_master_offset		= 0x0200,
> +};

[Severity: Low]
The compatible name soce,swip-00-04-0c-10 and the binding text describe a
register layout with "the core version register at offset 0x00 and feature
identification registers at offsets 0x04, 0x0c, and 0x10", but this
descriptor only models 0x00, 0x04 and 0x0c.

Is the 0x10 register meant to be read somewhere?  soce_sw_detect_features()
reads only licensed_features_offset and implemented_features0_offset:

	regval = readl(local->base_addr + probe_desc->licensed_features_offset);
	...
	regval = readl(local->base_addr +
		       probe_desc->implemented_features0_offset);

The "features0" suffix also suggests a features1 register at 0x10 that is
currently ignored.

Going the other way, mdio_master_offset = 0x0200 is a register block the
driver depends on for every MDIO transaction, but the binding does not
mention it at all.  Should the binding describe that block too, or should
the offset list in the compatible description match what the driver
actually models?

[ ... ]

> +static void soce_sw_enable_tagging(struct soce_dsa_local *local)
> +{
> +	u32 regval;
> +
> +	regval = readl(local->base_addr + SOCE_TAG_ALL_FRAMES_CTRL_OFFSET);
> +	regval |= SOCE_TAG_ALL_FRAMES_ENABLE;
> +	writel(regval, local->base_addr + SOCE_TAG_ALL_FRAMES_CTRL_OFFSET);
> +
> +	regval = readl(local->base_addr + SOCE_CUSTOM_RULES_TAGGING_OFFSET);
> +	regval |= SOCE_CUSTOM_RULES_TAGGING_ENABLE;
> +	writel(regval, local->base_addr + SOCE_CUSTOM_RULES_TAGGING_OFFSET);
> +}
> +
> +static int soce_setup(struct dsa_switch *ds)
> +{
> +	struct soce_priv *priv = ds->priv;
> +
> +	soce_sw_enable_tagging(&priv->local);
> +
> +	return 0;
> +}

[ ... ]

> +static const struct dsa_switch_ops soce_switch_ops = {
> +	.get_tag_protocol	= soce_get_tag_protocol,
> +	.setup			= soce_setup,
> +	.phylink_get_caps	= soce_phylink_get_caps,
> +};

[Severity: Medium]
Should there be a .teardown counterpart that clears what soce_setup()
armed?

soce_sw_enable_tagging() sets SOCE_TAG_ALL_FRAMES_ENABLE and
SOCE_CUSTOM_RULES_TAGGING_ENABLE, and nothing in the driver ever clears
them again.  There is no .teardown in soce_switch_ops, and the platform
callbacks only unregister:

soce_sw_remove()
	dsa_unregister_switch(priv->ds);

soce_sw_shutdown()
	dsa_switch_shutdown(priv->ds);

After an unbind or rmmod, does the core keep inserting SDSA source-port
tags into every frame delivered to the CPU port, with the tagger that
would strip them gone?  As described in the commit message, the same bits
also keep the user ports in CPU-directed operation:

    "Enabling this tagging feature of the switch, places all user ports
    into CPU-directed operation"

so the switch would also stay locked out of normal port-to-port
forwarding after the driver is removed.

> +static void soce_sw_remove(struct platform_device *pdev)
> +{
> +	struct soce_priv *priv = platform_get_drvdata(pdev);
> +
> +	if (!priv)
> +		return;
> +
> +	dsa_unregister_switch(priv->ds);
> +	platform_set_drvdata(pdev, NULL);
> +}

[ ... ]

> diff --git a/drivers/net/dsa/soce/soce_mdio.c b/drivers/net/dsa/soce/soce_mdio.c
> new file mode 100644
> index 0000000000000..692dcbe57d1a6
> --- /dev/null
> +++ b/drivers/net/dsa/soce/soce_mdio.c

[ ... ]

> +static int soce_mdio_read(struct mii_bus *bus, int addr, int reg)
> +{

[ ... ]

> +	mutex_lock(&local->mdio_lock);
> +	ret = local->mdio_ops->phy_read(state->ds, state->mdio_output, addr,
> +					reg);
> +	mutex_unlock(&local->mdio_lock);
> +
> +	return ret;
> +}

[ ... ]

> +static int soce_mdio_23_02_wait_for_idle(struct dsa_switch *ds)
> +{
> +	struct soce_priv *priv = ds->priv;
> +	struct soce_dsa_local *local;
> +	void __iomem *ctrl;
> +	u32 val;
> +
> +	local = &priv->local;
> +	ctrl = local->mdio_master_addr + SOCE_MDIO_CTRL_OFFSET;
> +
> +	return readl_poll_timeout(ctrl, val,
> +		!(val & (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET)), 10,
> +		SOCE_MDIO_TIMEOUT * 1000);
> +}

[Severity: Medium]
What happens to the hardware state when this poll times out?

On -ETIMEDOUT the OPSTATUS bit is still set, meaning the operation is
still in flight, and the callers just propagate the error without aborting
or resetting the master:

	ret = soce_mdio_23_02_wait_for_idle(ds);
	if (ret)
		return ret;

The bus wrappers soce_mdio_read(), soce_mdio_write(), soce_mdio_read_c45()
and soce_mdio_write_c45() then drop mdio_lock unconditionally, so the
software lock protecting the shared master is released while the hardware
is still busy.

None of the transaction helpers check for idle before programming the
registers either.  In soce_mdio_23_02_read_c22():

	regvalue = (regnum << SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET) +
		   (phy_addr << SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET);
	writel(regvalue, params);

	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) + ...

Can the next transaction, possibly issued through a different logical bus
sharing the same master, reprogram PARAMS/WRITE and kick CTRL underneath
a still-running operation?

> +int soce_mdio_23_02_read_c45(struct dsa_switch *ds, int mdio_output,
> +			     int phy_addr, int devad, int regnum)
> +{

[ ... ]

> +	regvalue = (devad << SOCE_MDIO_23_02_CTRL_REGADDRDEVTYPE_OFFSET) +
> +		   (phy_addr << SOCE_MDIO_23_02_CTRL_PHYADDR_OFFSET);
> +	writel(regvalue, params);
> +
> +	writel(regnum, write_reg);
> +
> +	/* address cycle */
> +	regvalue = ((mdio_output << SOCE_MDIO_23_02_CTRL_BUS_OFFSET) +
> +		    (0x0 << SOCE_MDIO_23_02_CTRL_TRANSTYPE_OFFSET) +
> +		    (0x1 << SOCE_MDIO_23_02_CTRL_CLAUSE_OFFSET) +
> +		    (0x1 << SOCE_MDIO_23_02_CTRL_OPSTATUS_OFFSET));
> +	writel(regvalue, ctrl);
> +
> +	ret = soce_mdio_23_02_wait_for_idle(ds);
> +	if (ret)
> +		return ret;
> +
> +	/* read cycle */

[Severity: Medium]
For Clause 45 the address cycle and the data cycle are separate hardware
operations.  If the address cycle times out here and is never aborted, can
a later data cycle land on a stale address, and can the following

	return readl(read_reg) & 0xffff;

hand the previous transaction's result back to phylib as a valid register
value?

Would it make sense to poll for idle before programming PARAMS/WRITE, and
to clear or reset the master on the timeout path before releasing
mdio_lock?  The same applies to soce_mdio_23_02_write_c22() and
soce_mdio_23_02_write_c45(), where a corrupted transaction would write to
an unintended PHY register.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-devel-vstrassheim-soce-dsa-ml-v2-0-fb0587cb466b%40linutronix.de

Re: [PATCH net-next v2 0/4] net: dsa: Add SoC-e DSA driver

[Krzysztof Kozlowski]

On Thu, Sep 03, 2026 at 08:10:57PM +0200, Vasilij Strassheim wrote:
> Add support for SoC-e Ethernet switch IP cores synthesized for and
> implemented in FPGAs.
> 
> The series introduces the SoC-e vendor prefix and device tree binding,
> an EtherType-based SDSA tagger, and an initial DSA switch driver.
> 
> The driver uses a memory-mapped control interface. During probe, it
> detects the core version and the licensed and implemented port counts,
> and verifies that the synthesized core provides DSA support. Port
> counts from 1 through 31 are supported.
> 
> The initial implementation supports Ethernet switching using MII, GMII,
> RMII, and RGMII port interfaces. Offloading features are not
> implemented.
> 
> External MDIO outputs are exposed as separate logical MDIO buses sharing
> the integrated MDIO controller.
> 
> The driver was tested with a SoC-e MRS 25.01 IP core on a Xilinx ZynqMP
> platform.
> 
> Signed-off-by: Vasilij Strassheim <v.strassheim@linutronix.de>
> ---
> Changes in v2:
> - Correctly use net-next prefix
> - Rework the binding into a single MMIO-based switch node
> - Model hardware MDIO outputs as separate logical MDIO buses
> - Detect the switch version, features, and port count from hardware
> - Drop incomplete STP, bridge, and FDB offloading
> - Derive phylink capabilities from each port's phy-mode
> - Harden MDIO access and SDSA receive validation
> - Address binding, naming, and coding style review comments

This is not specific enough. Every contributor can say that for every
patch...

Best regards,
Krzysztof