[PATCH v3 0/4] net: mtip: Add support for MTIP imx287 L2 switch driver

[PATCH v3 0/4] net: mtip: Add support for MTIP imx287 L2 switch driver

[Lukasz Majewski]

This patch series adds support for More Than IP's L2 switch driver embedded
in some NXP's SoCs. This one has been tested on imx287, but is also available
in the vf610.

In the past there has been performed some attempts to upstream this driver:
1. The 4.19-cip based one [1]
2. DSA based one for 5.12 [2] - i.e. the switch itself was treat as a DSA switch
   with NO tag appended.
3. The extension for FEC driver for 5.12 [3] - the trick here was to fully reuse
   FEC when the in-HW switching is disabled. When bridge offloading is enabled,
   the driver uses already configured MAC and PHY to also configure PHY.

All three approaches were not accepted as eligible for upstreaming.

The driver from this series has floowing features:

1. It is fully separated from fec_main - i.e. can be used interchangeable
   with it. To be more specific - one can build them as modules and
   if required switch between them when e.g. bridge offloading is required.

   To be more specific:
        - Use FEC_MAIN: When one needs support for two ETH ports with separate
          uDMAs used for both and bridging can be realized in SW.

        - Use MTIPL2SW: When it is enough to support two ports with only uDMA0
          attached to switch and bridging shall be offloaded to HW. 

2. This driver uses MTIP's L2 switch internal VLAN feature to provide port
   separation at boot time. Port separation is disabled when bridging is
   required.

3. Example usage:
        Configuration:
        ip link set lan0 up; sleep 1;
        ip link set lan1 up; sleep 1;
        ip link add name br0 type bridge;
        ip link set br0 up; sleep 1;
        ip link set lan0 master br0;
        ip link set lan1 master br0;
        bridge link;
        ip addr add 192.168.2.17/24 dev br0;
        ping -c 5 192.168.2.222

        Removal:
        ip link set br0 down;
        ip link delete br0 type bridge;
        ip link set dev lan1 down
        ip link set dev lan0 down

4. Limitations:
        - Driver enables and disables switch operation with learning and ageing.
        - Missing is the advanced configuration (e.g. adding entries to FBD). This is
          on purpose, as up till now we didn't had consensus about how the driver
          shall be added to Linux.

Links:
[1] - https://github.com/lmajewski/linux-imx28-l2switch/commits/master
[2] - https://github.com/lmajewski/linux-imx28-l2switch/tree/imx28-v5.12-L2-upstream-RFC_v1
[3] - https://source.denx.de/linux/linux-imx28-l2switch/-/tree/imx28-v5.12-L2-upstream-switchdev-RFC_v1?ref_type=heads

Lukasz Majewski (4):
  dt-bindings: net: Add MTIP L2 switch description
  ARM: dts: nxp: mxs: Adjust the imx28.dtsi L2 switch description
  ARM: dts: nxp: mxs: Adjust XEA board's DTS to support L2 switch
  net: mtip: The L2 switch driver for imx287

 .../bindings/net/nxp,imx287-mtip-switch.yaml  |  154 ++
 MAINTAINERS                                   |    7 +
 arch/arm/boot/dts/nxp/mxs/imx28-xea.dts       |   54 +
 arch/arm/boot/dts/nxp/mxs/imx28.dtsi          |    8 +-
 drivers/net/ethernet/freescale/Kconfig        |    1 +
 drivers/net/ethernet/freescale/Makefile       |    1 +
 drivers/net/ethernet/freescale/mtipsw/Kconfig |   13 +
 .../net/ethernet/freescale/mtipsw/Makefile    |    3 +
 .../net/ethernet/freescale/mtipsw/mtipl2sw.c  | 1964 +++++++++++++++++
 .../net/ethernet/freescale/mtipsw/mtipl2sw.h  |  780 +++++++
 .../ethernet/freescale/mtipsw/mtipl2sw_br.c   |  113 +
 .../ethernet/freescale/mtipsw/mtipl2sw_mgnt.c |  449 ++++
 12 files changed, 3545 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
 create mode 100644 drivers/net/ethernet/freescale/mtipsw/Kconfig
 create mode 100644 drivers/net/ethernet/freescale/mtipsw/Makefile
 create mode 100644 drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
 create mode 100644 drivers/net/ethernet/freescale/mtipsw/mtipl2sw.h
 create mode 100644 drivers/net/ethernet/freescale/mtipsw/mtipl2sw_br.c
 create mode 100644 drivers/net/ethernet/freescale/mtipsw/mtipl2sw_mgnt.c

-- 
2.39.5

[PATCH v3 1/4] dt-bindings: net: Add MTIP L2 switch description

[Lukasz Majewski]

This patch provides description of the MTIP L2 switch available in some
NXP's SOCs - e.g. imx287.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
Changes for v2:
- Rename the file to match exactly the compatible
  (nxp,imx287-mtip-switch)

Changes for v3:
- Remove '-' from const:'nxp,imx287-mtip-switch'
- Use '^port@[12]+$' for port patternProperties
- Drop status = "okay";
- Provide proper indentation for 'example' binding (replace 8
  spaces with 4 spaces)
- Remove smsc,disable-energy-detect; property
- Remove interrupt-parent and interrupts properties as not required
- Remove #address-cells and #size-cells from required properties check
- remove description from reg:
- Add $ref: ethernet-switch.yaml#
---
 .../bindings/net/nxp,imx287-mtip-switch.yaml  | 154 ++++++++++++++++++
 1 file changed, 154 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml

diff --git a/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
new file mode 100644
index 000000000000..98eba3665f32
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/nxp,imx287-mtip-switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP SoC Ethernet Switch Controller (L2 MoreThanIP switch)
+
+maintainers:
+  - Lukasz Majewski <lukma@denx.de>
+
+description:
+  The 2-port switch ethernet subsystem provides ethernet packet (L2)
+  communication and can be configured as an ethernet switch. It provides the
+  reduced media independent interface (RMII), the management data input
+  output (MDIO) for physical layer device (PHY) management.
+
+$ref: ethernet-switch.yaml#
+
+properties:
+  compatible:
+    const: nxp,imx287-mtip-switch
+
+  reg:
+    maxItems: 1
+
+  phy-supply:
+    description:
+      Regulator that powers Ethernet PHYs.
+
+  clocks:
+    items:
+      - description: Register accessing clock
+      - description: Bus access clock
+      - description: Output clock for external device - e.g. PHY source clock
+      - description: IEEE1588 timer clock
+
+  clock-names:
+    items:
+      - const: ipg
+      - const: ahb
+      - const: enet_out
+      - const: ptp
+
+  interrupts:
+    items:
+      - description: Switch interrupt
+      - description: ENET0 interrupt
+      - description: ENET1 interrupt
+
+  pinctrl-names: true
+
+  ethernet-ports:
+    type: object
+    additionalProperties: false
+
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      '^port@[12]+$':
+        type: object
+        description: MTIP L2 switch external ports
+
+        $ref: ethernet-controller.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          reg:
+            items:
+              - enum: [1, 2]
+            description: MTIP L2 switch port number
+
+          label:
+            description: Label associated with this port
+
+        required:
+          - reg
+          - label
+          - phy-mode
+          - phy-handle
+
+  mdio:
+    type: object
+    $ref: mdio.yaml#
+    unevaluatedProperties: false
+    description:
+      Specifies the mdio bus in the switch, used as a container for phy nodes.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - mdio
+  - ethernet-ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include<dt-bindings/interrupt-controller/irq.h>
+    switch@800f0000 {
+        compatible = "nxp,imx287-mtip-switch";
+        reg = <0x800f0000 0x20000>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&mac0_pins_a>, <&mac1_pins_a>;
+        phy-supply = <&reg_fec_3v3>;
+        interrupts = <100>, <101>, <102>;
+        clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>;
+        clock-names = "ipg", "ahb", "enet_out", "ptp";
+
+        ethernet-ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            mtip_port1: port@1 {
+                reg = <1>;
+                label = "lan0";
+                local-mac-address = [ 00 00 00 00 00 00 ];
+                phy-mode = "rmii";
+                phy-handle = <&ethphy0>;
+            };
+
+            mtip_port2: port@2 {
+                reg = <2>;
+                label = "lan1";
+                local-mac-address = [ 00 00 00 00 00 00 ];
+                phy-mode = "rmii";
+                phy-handle = <&ethphy1>;
+            };
+        };
+
+        mdio_sw: mdio {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            reset-gpios = <&gpio2 13 0>;
+            reset-delay-us = <25000>;
+            reset-post-delay-us = <10000>;
+
+            ethphy0: ethernet-phy@0 {
+                reg = <0>;
+            };
+
+            ethphy1: ethernet-phy@1 {
+                reg = <1>;
+            };
+        };
+    };
-- 
2.39.5

[PATCH v3 2/4] ARM: dts: nxp: mxs: Adjust the imx28.dtsi L2 switch description

[Lukasz Majewski]

The current range of 'reg' property is too small to allow full control
of the L2 switch on imx287.

As this IP block also uses ENET-MAC blocks for its operation, the address
range for it must be included as well.

Moreover, some SoC common properties (like compatible, clocks, interrupts
numbers) have been moved to this node.

Signed-off-by: Lukasz Majewski <lukma@denx.de>

---
Changes for v2:
- adding extra properties (like compatible, clocks, interupts)

Changes for v3:
- None
---
 arch/arm/boot/dts/nxp/mxs/imx28.dtsi | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/nxp/mxs/imx28.dtsi b/arch/arm/boot/dts/nxp/mxs/imx28.dtsi
index bbea8b77386f..4117a5003b36 100644
--- a/arch/arm/boot/dts/nxp/mxs/imx28.dtsi
+++ b/arch/arm/boot/dts/nxp/mxs/imx28.dtsi
@@ -1321,8 +1321,12 @@ mac1: ethernet@800f4000 {
 			status = "disabled";
 		};
 
-		eth_switch: switch@800f8000 {
-			reg = <0x800f8000 0x8000>;
+		eth_switch: switch@800f0000 {
+			compatible = "nxp,imx287-mtip-switch";
+			reg = <0x800f0000 0x20000>;
+			interrupts = <100>, <101>, <102>;
+			clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>;
+			clock-names = "ipg", "ahb", "enet_out", "ptp";
 			status = "disabled";
 		};
 	};
-- 
2.39.5

[PATCH v3 3/4] ARM: dts: nxp: mxs: Adjust XEA board's DTS to support L2 switch

[Lukasz Majewski]

The description is similar to the one used with the new CPSW driver.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
Changes for v2:
- Remove properties which are common for the imx28(7) SoC
- Use mdio properties to perform L2 switch reset (avoid using
  deprecated properties)

Changes for v3:
- Replace IRQ_TYPE_EDGE_FALLING with IRQ_TYPE_LEVEL_LOW
- Update comment regarding PHY interrupts s/AND/OR/g
---
 arch/arm/boot/dts/nxp/mxs/imx28-xea.dts | 54 +++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts b/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts
index 6c5e6856648a..8642578fddf3 100644
--- a/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts
+++ b/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts
@@ -5,6 +5,7 @@
  */
 
 /dts-v1/;
+#include<dt-bindings/interrupt-controller/irq.h>
 #include "imx28-lwe.dtsi"
 
 / {
@@ -90,6 +91,59 @@ &reg_usb_5v {
 	gpio = <&gpio0 2 0>;
 };
 
+&eth_switch {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mac0_pins_a>, <&mac1_pins_a>;
+	phy-supply = <&reg_fec_3v3>;
+	status = "okay";
+
+	ethernet-ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mtip_port1: port@1 {
+			reg = <1>;
+			label = "lan0";
+			local-mac-address = [ 00 00 00 00 00 00 ];
+			phy-mode = "rmii";
+			phy-handle = <&ethphy0>;
+		};
+
+		mtip_port2: port@2 {
+			reg = <2>;
+			label = "lan1";
+			local-mac-address = [ 00 00 00 00 00 00 ];
+			phy-mode = "rmii";
+			phy-handle = <&ethphy1>;
+		};
+	};
+
+	mdio_sw: mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		reset-gpios = <&gpio3 21 0>;
+		reset-delay-us = <25000>;
+		reset-post-delay-us = <10000>;
+
+		ethphy0: ethernet-phy@0 {
+			reg = <0>;
+			smsc,disable-energy-detect;
+			/* Both PHYs (i.e. 0,1) have the same, single GPIO, */
+			/* line to handle both, their interrupts (OR'ed) */
+			interrupt-parent = <&gpio4>;
+			interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
+		};
+
+		ethphy1: ethernet-phy@1 {
+			reg = <1>;
+			smsc,disable-energy-detect;
+			interrupt-parent = <&gpio4>;
+			interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
+		};
+	};
+};
+
 &spi2_pins_a {
 	fsl,pinmux-ids = <
 		MX28_PAD_SSP2_SCK__SSP2_SCK
-- 
2.39.5

Re: [PATCH v3 0/4] net: mtip: Add support for MTIP imx287 L2 switch driver

[Jakub Kicinski]

On Mon, 31 Mar 2025 12:31:12 +0200 Lukasz Majewski wrote:
> This patch series adds support for More Than IP's L2 switch driver embedded
> in some NXP's SoCs. This one has been tested on imx287, but is also available
> in the vf610.

Lukasz, please post with RFC in the subject tags during the merge
window. As I already said net-next is closed.

Re: [PATCH v3 1/4] dt-bindings: net: Add MTIP L2 switch description

[Fabio Estevam]

Hi Lukasz,

On Mon, Mar 31, 2025 at 7:31 AM Lukasz Majewski <lukma@denx.de> wrote:

> +properties:
> +  compatible:
> +    const: nxp,imx287-mtip-switch

For consistency, please use "nxp,imx28-mtip-switch" instead.

imx287 is not used anywhere in the kernel.

Re: [PATCH v3 0/4] net: mtip: Add support for MTIP imx287 L2 switch driver

[Lukasz Majewski]

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

Hi Jakub,

> On Mon, 31 Mar 2025 12:31:12 +0200 Lukasz Majewski wrote:
> > This patch series adds support for More Than IP's L2 switch driver
> > embedded in some NXP's SoCs. This one has been tested on imx287,
> > but is also available in the vf610.  
> 
> Lukasz, please post with RFC in the subject tags during the merge
> window. As I already said net-next is closed.

Ach, Ok.

I hope, that we will finish all reviews till 07.04, so v4 would be the
final version.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 1/4] dt-bindings: net: Add MTIP L2 switch description

[Lukasz Majewski]

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

Hi Fabio,

> Hi Lukasz,
> 
> On Mon, Mar 31, 2025 at 7:31 AM Lukasz Majewski <lukma@denx.de> wrote:
> 
> > +properties:
> > +  compatible:
> > +    const: nxp,imx287-mtip-switch  
> 
> For consistency, please use "nxp,imx28-mtip-switch" instead.
> 
> imx287 is not used anywhere in the kernel.

Ok. Thanks for the info.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 0/4] net: mtip: Add support for MTIP imx287 L2 switch driver

[Stefan Wahren]

Hi Lukasz,

Am 31.03.25 um 21:11 schrieb Lukasz Majewski:
> Hi Jakub,
>
>> On Mon, 31 Mar 2025 12:31:12 +0200 Lukasz Majewski wrote:
>>> This patch series adds support for More Than IP's L2 switch driver
>>> embedded in some NXP's SoCs. This one has been tested on imx287,
>>> but is also available in the vf610.
>> Lukasz, please post with RFC in the subject tags during the merge
>> window. As I already said net-next is closed.
> Ach, Ok.
>
> I hope, that we will finish all reviews till 07.04, so v4 would be the
> final version.
well i appreciate your work on this driver, but this is not how it's
going to work. No version of this patch series had a review time of a
week. It's about quality not about deadlines.

Regards

Re: [PATCH v3 3/4] ARM: dts: nxp: mxs: Adjust XEA board's DTS to support L2 switch

[Stefan Wahren]

Hi,

Am 31.03.25 um 12:31 schrieb Lukasz Majewski:
> The description is similar to the one used with the new CPSW driver.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
> Changes for v2:
> - Remove properties which are common for the imx28(7) SoC
> - Use mdio properties to perform L2 switch reset (avoid using
>    deprecated properties)
>
> Changes for v3:
> - Replace IRQ_TYPE_EDGE_FALLING with IRQ_TYPE_LEVEL_LOW
> - Update comment regarding PHY interrupts s/AND/OR/g
> ---
>   arch/arm/boot/dts/nxp/mxs/imx28-xea.dts | 54 +++++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
>
> diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts b/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts
> index 6c5e6856648a..8642578fddf3 100644
> --- a/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts
> +++ b/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts
> @@ -5,6 +5,7 @@
>    */
>
>   /dts-v1/;
> +#include<dt-bindings/interrupt-controller/irq.h>
>   #include "imx28-lwe.dtsi"
>
>   / {
> @@ -90,6 +91,59 @@ &reg_usb_5v {
>   	gpio = <&gpio0 2 0>;
>   };
>
> +&eth_switch {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mac0_pins_a>, <&mac1_pins_a>;
> +	phy-supply = <&reg_fec_3v3>;
> +	status = "okay";
> +
> +	ethernet-ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		mtip_port1: port@1 {
> +			reg = <1>;
> +			label = "lan0";
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			phy-mode = "rmii";
> +			phy-handle = <&ethphy0>;
> +		};
> +
> +		mtip_port2: port@2 {
> +			reg = <2>;
> +			label = "lan1";
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			phy-mode = "rmii";
> +			phy-handle = <&ethphy1>;
> +		};
> +	};
> +
> +	mdio_sw: mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		reset-gpios = <&gpio3 21 0>;
i'm a huge fan of the polarity defines, which makes it easier to understand.

Btw since you introduced the compatible in the DTS of a i.MX28 board, it
would be nice to also enable the driver in mxs_defconfig.

Regards
> +		reset-delay-us = <25000>;
> +		reset-post-delay-us = <10000>;
> +
> +		ethphy0: ethernet-phy@0 {
> +			reg = <0>;
> +			smsc,disable-energy-detect;
> +			/* Both PHYs (i.e. 0,1) have the same, single GPIO, */
> +			/* line to handle both, their interrupts (OR'ed) */
> +			interrupt-parent = <&gpio4>;
> +			interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
> +		};
> +
> +		ethphy1: ethernet-phy@1 {
> +			reg = <1>;
> +			smsc,disable-energy-detect;
> +			interrupt-parent = <&gpio4>;
> +			interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
> +		};
> +	};
> +};
> +
>   &spi2_pins_a {
>   	fsl,pinmux-ids = <
>   		MX28_PAD_SSP2_SCK__SSP2_SCK

Re: [PATCH v3 0/4] net: mtip: Add support for MTIP imx287 L2 switch driver

[Lukasz Majewski]

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

Hi Stefan,

> Hi Lukasz,
> 
> Am 31.03.25 um 21:11 schrieb Lukasz Majewski:
> > Hi Jakub,
> >  
> >> On Mon, 31 Mar 2025 12:31:12 +0200 Lukasz Majewski wrote:  
> >>> This patch series adds support for More Than IP's L2 switch driver
> >>> embedded in some NXP's SoCs. This one has been tested on imx287,
> >>> but is also available in the vf610.  
> >> Lukasz, please post with RFC in the subject tags during the merge
> >> window. As I already said net-next is closed.  
> > Ach, Ok.
> >
> > I hope, that we will finish all reviews till 07.04, so v4 would be
> > the final version.  
> well i appreciate your work on this driver,

Do you maintain some vf610 or imx28 devices, which would like to use L2
switch?

> but this is not how it's
> going to work. No version of this patch series had a review time of a
> week.

I just pointed out that patches would not be pulled (or anything else
would be done with them) until the merge window is re-opened.

> It's about quality not about deadlines.
> 

:-D

> Regards

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 3/4] ARM: dts: nxp: mxs: Adjust XEA board's DTS to support L2 switch

[Lukasz Majewski]

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

Hi Stefan,

> Hi,
> 
> Am 31.03.25 um 12:31 schrieb Lukasz Majewski:
> > The description is similar to the one used with the new CPSW driver.
> >
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> > Changes for v2:
> > - Remove properties which are common for the imx28(7) SoC
> > - Use mdio properties to perform L2 switch reset (avoid using
> >    deprecated properties)
> >
> > Changes for v3:
> > - Replace IRQ_TYPE_EDGE_FALLING with IRQ_TYPE_LEVEL_LOW
> > - Update comment regarding PHY interrupts s/AND/OR/g
> > ---
> >   arch/arm/boot/dts/nxp/mxs/imx28-xea.dts | 54
> > +++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts
> > b/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts index
> > 6c5e6856648a..8642578fddf3 100644 ---
> > a/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts +++
> > b/arch/arm/boot/dts/nxp/mxs/imx28-xea.dts @@ -5,6 +5,7 @@
> >    */
> >
> >   /dts-v1/;
> > +#include<dt-bindings/interrupt-controller/irq.h>
> >   #include "imx28-lwe.dtsi"
> >
> >   / {
> > @@ -90,6 +91,59 @@ &reg_usb_5v {
> >   	gpio = <&gpio0 2 0>;
> >   };
> >
> > +&eth_switch {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&mac0_pins_a>, <&mac1_pins_a>;
> > +	phy-supply = <&reg_fec_3v3>;
> > +	status = "okay";
> > +
> > +	ethernet-ports {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		mtip_port1: port@1 {
> > +			reg = <1>;
> > +			label = "lan0";
> > +			local-mac-address = [ 00 00 00 00 00 00 ];
> > +			phy-mode = "rmii";
> > +			phy-handle = <&ethphy0>;
> > +		};
> > +
> > +		mtip_port2: port@2 {
> > +			reg = <2>;
> > +			label = "lan1";
> > +			local-mac-address = [ 00 00 00 00 00 00 ];
> > +			phy-mode = "rmii";
> > +			phy-handle = <&ethphy1>;
> > +		};
> > +	};
> > +
> > +	mdio_sw: mdio {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		reset-gpios = <&gpio3 21 0>;  
> i'm a huge fan of the polarity defines, which makes it easier to
> understand.
> 

Ok.

> Btw since you introduced the compatible in the DTS of a i.MX28 board,
> it would be nice to also enable the driver in mxs_defconfig.

Yes, thanks for the tip.

> 
> Regards
> > +		reset-delay-us = <25000>;
> > +		reset-post-delay-us = <10000>;
> > +
> > +		ethphy0: ethernet-phy@0 {
> > +			reg = <0>;
> > +			smsc,disable-energy-detect;
> > +			/* Both PHYs (i.e. 0,1) have the same,
> > single GPIO, */
> > +			/* line to handle both, their interrupts
> > (OR'ed) */
> > +			interrupt-parent = <&gpio4>;
> > +			interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
> > +		};
> > +
> > +		ethphy1: ethernet-phy@1 {
> > +			reg = <1>;
> > +			smsc,disable-energy-detect;
> > +			interrupt-parent = <&gpio4>;
> > +			interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
> > +		};
> > +	};
> > +};
> > +
> >   &spi2_pins_a {
> >   	fsl,pinmux-ids = <
> >   		MX28_PAD_SSP2_SCK__SSP2_SCK  
> 


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 1/4] dt-bindings: net: Add MTIP L2 switch description

[Rob Herring]

On Mon, Mar 31, 2025 at 12:31:13PM +0200, Lukasz Majewski wrote:
> This patch provides description of the MTIP L2 switch available in some
> NXP's SOCs - e.g. imx287.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
> Changes for v2:
> - Rename the file to match exactly the compatible
>   (nxp,imx287-mtip-switch)
> 
> Changes for v3:
> - Remove '-' from const:'nxp,imx287-mtip-switch'
> - Use '^port@[12]+$' for port patternProperties
> - Drop status = "okay";
> - Provide proper indentation for 'example' binding (replace 8
>   spaces with 4 spaces)
> - Remove smsc,disable-energy-detect; property
> - Remove interrupt-parent and interrupts properties as not required
> - Remove #address-cells and #size-cells from required properties check
> - remove description from reg:
> - Add $ref: ethernet-switch.yaml#
> ---
>  .../bindings/net/nxp,imx287-mtip-switch.yaml  | 154 ++++++++++++++++++
>  1 file changed, 154 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> new file mode 100644
> index 000000000000..98eba3665f32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/nxp,imx287-mtip-switch.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP SoC Ethernet Switch Controller (L2 MoreThanIP switch)
> +
> +maintainers:
> +  - Lukasz Majewski <lukma@denx.de>
> +
> +description:
> +  The 2-port switch ethernet subsystem provides ethernet packet (L2)
> +  communication and can be configured as an ethernet switch. It provides the
> +  reduced media independent interface (RMII), the management data input
> +  output (MDIO) for physical layer device (PHY) management.
> +
> +$ref: ethernet-switch.yaml#

This needs to be: ethernet-switch.yaml#/$defs/ethernet-ports

With that, you can drop much of the below part. More below...

> +
> +properties:
> +  compatible:
> +    const: nxp,imx287-mtip-switch
> +
> +  reg:
> +    maxItems: 1
> +
> +  phy-supply:
> +    description:
> +      Regulator that powers Ethernet PHYs.
> +
> +  clocks:
> +    items:
> +      - description: Register accessing clock
> +      - description: Bus access clock
> +      - description: Output clock for external device - e.g. PHY source clock
> +      - description: IEEE1588 timer clock
> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: ahb
> +      - const: enet_out
> +      - const: ptp
> +
> +  interrupts:
> +    items:
> +      - description: Switch interrupt
> +      - description: ENET0 interrupt
> +      - description: ENET1 interrupt
> +
> +  pinctrl-names: true
> +

> +  ethernet-ports:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      '^port@[12]+$':

Note that 'ethernet-port' is the preferred node name though 'port' is 
allowed.

> +        type: object
> +        description: MTIP L2 switch external ports
> +
> +        $ref: ethernet-controller.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            items:
> +              - enum: [1, 2]
> +            description: MTIP L2 switch port number
> +
> +          label:
> +            description: Label associated with this port
> +
> +        required:
> +          - reg
> +          - label
> +          - phy-mode
> +          - phy-handle

All the above under 'ethernet-ports' can be dropped though you might 
want to keep 'required' part.

> +
> +  mdio:
> +    type: object
> +    $ref: mdio.yaml#
> +    unevaluatedProperties: false
> +    description:
> +      Specifies the mdio bus in the switch, used as a container for phy nodes.
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - mdio
> +  - ethernet-ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include<dt-bindings/interrupt-controller/irq.h>
> +    switch@800f0000 {
> +        compatible = "nxp,imx287-mtip-switch";
> +        reg = <0x800f0000 0x20000>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&mac0_pins_a>, <&mac1_pins_a>;
> +        phy-supply = <&reg_fec_3v3>;
> +        interrupts = <100>, <101>, <102>;
> +        clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>;
> +        clock-names = "ipg", "ahb", "enet_out", "ptp";
> +
> +        ethernet-ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            mtip_port1: port@1 {
> +                reg = <1>;
> +                label = "lan0";
> +                local-mac-address = [ 00 00 00 00 00 00 ];
> +                phy-mode = "rmii";
> +                phy-handle = <&ethphy0>;
> +            };
> +
> +            mtip_port2: port@2 {
> +                reg = <2>;
> +                label = "lan1";
> +                local-mac-address = [ 00 00 00 00 00 00 ];
> +                phy-mode = "rmii";
> +                phy-handle = <&ethphy1>;
> +            };
> +        };
> +
> +        mdio_sw: mdio {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            reset-gpios = <&gpio2 13 0>;
> +            reset-delay-us = <25000>;
> +            reset-post-delay-us = <10000>;
> +
> +            ethphy0: ethernet-phy@0 {
> +                reg = <0>;
> +            };
> +
> +            ethphy1: ethernet-phy@1 {
> +                reg = <1>;
> +            };
> +        };
> +    };
> -- 
> 2.39.5
> 

Re: [PATCH v3 1/4] dt-bindings: net: Add MTIP L2 switch description

[Lukasz Majewski]

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

Hi Rob,

> On Mon, Mar 31, 2025 at 12:31:13PM +0200, Lukasz Majewski wrote:
> > This patch provides description of the MTIP L2 switch available in
> > some NXP's SOCs - e.g. imx287.
> > 
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> > Changes for v2:
> > - Rename the file to match exactly the compatible
> >   (nxp,imx287-mtip-switch)
> > 
> > Changes for v3:
> > - Remove '-' from const:'nxp,imx287-mtip-switch'
> > - Use '^port@[12]+$' for port patternProperties
> > - Drop status = "okay";
> > - Provide proper indentation for 'example' binding (replace 8
> >   spaces with 4 spaces)
> > - Remove smsc,disable-energy-detect; property
> > - Remove interrupt-parent and interrupts properties as not required
> > - Remove #address-cells and #size-cells from required properties
> > check
> > - remove description from reg:
> > - Add $ref: ethernet-switch.yaml#
> > ---
> >  .../bindings/net/nxp,imx287-mtip-switch.yaml  | 154
> > ++++++++++++++++++ 1 file changed, 154 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > new file mode 100644 index 000000000000..98eba3665f32 --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > @@ -0,0 +1,154 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR
> > BSD-2-Clause) +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/nxp,imx287-mtip-switch.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP SoC Ethernet Switch Controller (L2 MoreThanIP switch)
> > +
> > +maintainers:
> > +  - Lukasz Majewski <lukma@denx.de>
> > +
> > +description:
> > +  The 2-port switch ethernet subsystem provides ethernet packet
> > (L2)
> > +  communication and can be configured as an ethernet switch. It
> > provides the
> > +  reduced media independent interface (RMII), the management data
> > input
> > +  output (MDIO) for physical layer device (PHY) management.
> > +
> > +$ref: ethernet-switch.yaml#  
> 
> This needs to be: ethernet-switch.yaml#/$defs/ethernet-ports
> 
> With that, you can drop much of the below part. More below...
> 
> > +
> > +properties:

So it shall be after the "properties:"

$ref: ethernet-switch.yaml#/$defs/ethernet-ports   [*]


> > +  compatible:
> > +    const: nxp,imx287-mtip-switch
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  phy-supply:
> > +    description:
> > +      Regulator that powers Ethernet PHYs.
> > +
> > +  clocks:
> > +    items:
> > +      - description: Register accessing clock
> > +      - description: Bus access clock
> > +      - description: Output clock for external device - e.g. PHY
> > source clock
> > +      - description: IEEE1588 timer clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: ipg
> > +      - const: ahb
> > +      - const: enet_out
> > +      - const: ptp
> > +
> > +  interrupts:
> > +    items:
> > +      - description: Switch interrupt
> > +      - description: ENET0 interrupt
> > +      - description: ENET1 interrupt
> > +
> > +  pinctrl-names: true
> > +  
> 
> > +  ethernet-ports:

And then this "node" can be removed as it has been referenced above [*]?

(I shall only keep the properties, which are not standard to [*] if
any).

> > +    type: object
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      '#address-cells':
> > +        const: 1
> > +      '#size-cells':
> > +        const: 0
> > +
> > +    patternProperties:
> > +      '^port@[12]+$':  
> 
> Note that 'ethernet-port' is the preferred node name though 'port' is 
> allowed.

Ok. That would be the correct define:

ethernet-ports {
     mtip_port1: ethernet-port@1 {
               reg = <1>;
               label = "lan0";
               local-mac-address = [ 00 00 00 00 00 00 ];
               phy-mode = "rmii";
               phy-handle = <&ethphy0>;
	       };

               mtip_port2: port@2 {
		....
	       };

> 
> > +        type: object
> > +        description: MTIP L2 switch external ports
> > +
> > +        $ref: ethernet-controller.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          reg:
> > +            items:
> > +              - enum: [1, 2]
> > +            description: MTIP L2 switch port number
> > +
> > +          label:
> > +            description: Label associated with this port
> > +
> > +        required:
> > +          - reg
> > +          - label
> > +          - phy-mode
> > +          - phy-handle  
> 
> All the above under 'ethernet-ports' can be dropped though you might 
> want to keep 'required' part.

Ok, I will keep it.

> 
> > +
> > +  mdio:
> > +    type: object
> > +    $ref: mdio.yaml#
> > +    unevaluatedProperties: false
> > +    description:
> > +      Specifies the mdio bus in the switch, used as a container
> > for phy nodes. +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - interrupts
> > +  - mdio
> > +  - ethernet-ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include<dt-bindings/interrupt-controller/irq.h>
> > +    switch@800f0000 {
> > +        compatible = "nxp,imx287-mtip-switch";
> > +        reg = <0x800f0000 0x20000>;
> > +        pinctrl-names = "default";
> > +        pinctrl-0 = <&mac0_pins_a>, <&mac1_pins_a>;
> > +        phy-supply = <&reg_fec_3v3>;
> > +        interrupts = <100>, <101>, <102>;
> > +        clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>;
> > +        clock-names = "ipg", "ahb", "enet_out", "ptp";
> > +
> > +        ethernet-ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            mtip_port1: port@1 {
> > +                reg = <1>;
> > +                label = "lan0";
> > +                local-mac-address = [ 00 00 00 00 00 00 ];
> > +                phy-mode = "rmii";
> > +                phy-handle = <&ethphy0>;
> > +            };
> > +
> > +            mtip_port2: port@2 {
> > +                reg = <2>;
> > +                label = "lan1";
> > +                local-mac-address = [ 00 00 00 00 00 00 ];
> > +                phy-mode = "rmii";
> > +                phy-handle = <&ethphy1>;
> > +            };
> > +        };
> > +
> > +        mdio_sw: mdio {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            reset-gpios = <&gpio2 13 0>;
> > +            reset-delay-us = <25000>;
> > +            reset-post-delay-us = <10000>;
> > +
> > +            ethphy0: ethernet-phy@0 {
> > +                reg = <0>;
> > +            };
> > +
> > +            ethphy1: ethernet-phy@1 {
> > +                reg = <1>;
> > +            };
> > +        };
> > +    };
> > -- 
> > 2.39.5
> >   

Thanks for the hint.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 0/4] net: mtip: Add support for MTIP imx287 L2 switch driver

[Stefan Wahren]

Am 31.03.25 um 23:42 schrieb Lukasz Majewski:
> Hi Stefan,
>
>> Hi Lukasz,
>>
>> Am 31.03.25 um 21:11 schrieb Lukasz Majewski:
>>> Hi Jakub,
>>>
>>>> On Mon, 31 Mar 2025 12:31:12 +0200 Lukasz Majewski wrote:
>>>>> This patch series adds support for More Than IP's L2 switch driver
>>>>> embedded in some NXP's SoCs. This one has been tested on imx287,
>>>>> but is also available in the vf610.
>>>> Lukasz, please post with RFC in the subject tags during the merge
>>>> window. As I already said net-next is closed.
>>> Ach, Ok.
>>>
>>> I hope, that we will finish all reviews till 07.04, so v4 would be
>>> the final version.
>> well i appreciate your work on this driver,
> Do you maintain some vf610 or imx28 devices, which would like to use L2
> switch?
I've have been working with some i.MX28 devices and still "maintain" one
i.MX28 platform of my employer. But it doesn't have a L2 switch.

Many years again, I tried to mainline a driver for the on-chip regulator
of i.MX28 SoC with cpufreq support at the end.

https://github.com/lategoodbye/linux-mxs-power

Regards

Re: [PATCH v3 1/4] dt-bindings: net: Add MTIP L2 switch description

[Rob Herring]

On Tue, Apr 01, 2025 at 12:35:07PM +0200, Lukasz Majewski wrote:
> Hi Rob,
> 
> > On Mon, Mar 31, 2025 at 12:31:13PM +0200, Lukasz Majewski wrote:
> > > This patch provides description of the MTIP L2 switch available in
> > > some NXP's SOCs - e.g. imx287.
> > > 
> > > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > > ---
> > > Changes for v2:
> > > - Rename the file to match exactly the compatible
> > >   (nxp,imx287-mtip-switch)
> > > 
> > > Changes for v3:
> > > - Remove '-' from const:'nxp,imx287-mtip-switch'
> > > - Use '^port@[12]+$' for port patternProperties
> > > - Drop status = "okay";
> > > - Provide proper indentation for 'example' binding (replace 8
> > >   spaces with 4 spaces)
> > > - Remove smsc,disable-energy-detect; property
> > > - Remove interrupt-parent and interrupts properties as not required
> > > - Remove #address-cells and #size-cells from required properties
> > > check
> > > - remove description from reg:
> > > - Add $ref: ethernet-switch.yaml#
> > > ---
> > >  .../bindings/net/nxp,imx287-mtip-switch.yaml  | 154
> > > ++++++++++++++++++ 1 file changed, 154 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > > b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > > new file mode 100644 index 000000000000..98eba3665f32 --- /dev/null
> > > +++
> > > b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > > @@ -0,0 +1,154 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR
> > > BSD-2-Clause) +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/nxp,imx287-mtip-switch.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: NXP SoC Ethernet Switch Controller (L2 MoreThanIP switch)
> > > +
> > > +maintainers:
> > > +  - Lukasz Majewski <lukma@denx.de>
> > > +
> > > +description:
> > > +  The 2-port switch ethernet subsystem provides ethernet packet
> > > (L2)
> > > +  communication and can be configured as an ethernet switch. It
> > > provides the
> > > +  reduced media independent interface (RMII), the management data
> > > input
> > > +  output (MDIO) for physical layer device (PHY) management.
> > > +
> > > +$ref: ethernet-switch.yaml#  
> > 
> > This needs to be: ethernet-switch.yaml#/$defs/ethernet-ports
> > 
> > With that, you can drop much of the below part. More below...
> > 
> > > +
> > > +properties:
> 
> So it shall be after the "properties:"
> 
> $ref: ethernet-switch.yaml#/$defs/ethernet-ports   [*]

It can stay where it is, just add "/$defs/ethernet-ports"


> > > +  compatible:
> > > +    const: nxp,imx287-mtip-switch
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  phy-supply:
> > > +    description:
> > > +      Regulator that powers Ethernet PHYs.
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: Register accessing clock
> > > +      - description: Bus access clock
> > > +      - description: Output clock for external device - e.g. PHY
> > > source clock
> > > +      - description: IEEE1588 timer clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: ipg
> > > +      - const: ahb
> > > +      - const: enet_out
> > > +      - const: ptp
> > > +
> > > +  interrupts:
> > > +    items:
> > > +      - description: Switch interrupt
> > > +      - description: ENET0 interrupt
> > > +      - description: ENET1 interrupt
> > > +
> > > +  pinctrl-names: true
> > > +  
> > 
> > > +  ethernet-ports:
> 
> And then this "node" can be removed as it has been referenced above [*]?

Well, you have to keep it to have 'required' in the child nodes.

> 
> (I shall only keep the properties, which are not standard to [*] if
> any).
> 
> > > +    type: object
> > > +    additionalProperties: false
> > > +
> > > +    properties:
> > > +      '#address-cells':
> > > +        const: 1
> > > +      '#size-cells':
> > > +        const: 0
> > > +
> > > +    patternProperties:
> > > +      '^port@[12]+$':  
> > 
> > Note that 'ethernet-port' is the preferred node name though 'port' is 
> > allowed.
> 
> Ok. That would be the correct define:
> 
> ethernet-ports {
>      mtip_port1: ethernet-port@1 {
>                reg = <1>;
>                label = "lan0";
>                local-mac-address = [ 00 00 00 00 00 00 ];
>                phy-mode = "rmii";
>                phy-handle = <&ethphy0>;
> 	       };
> 
>                mtip_port2: port@2 {

And here...

> 		....
> 	       };
> 
> > 
> > > +        type: object
> > > +        description: MTIP L2 switch external ports
> > > +
> > > +        $ref: ethernet-controller.yaml#
> > > +        unevaluatedProperties: false
> > > +
> > > +        properties:
> > > +          reg:
> > > +            items:
> > > +              - enum: [1, 2]
> > > +            description: MTIP L2 switch port number
> > > +
> > > +          label:
> > > +            description: Label associated with this port
> > > +
> > > +        required:
> > > +          - reg
> > > +          - label
> > > +          - phy-mode
> > > +          - phy-handle  
> > 
> > All the above under 'ethernet-ports' can be dropped though you might 
> > want to keep 'required' part.
> 
> Ok, I will keep it.

So I think you just want this:

ethernet-ports:
  type: object
  additionalProperties: true  # Check if you need this

  patternProperties:
    '^ethernet-port@[12]$':
      required:
        - label
        - phy-mode
        - phy-handle

Rob

Re: [PATCH v3 1/4] dt-bindings: net: Add MTIP L2 switch description

[Lukasz Majewski]

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

On Tue, 1 Apr 2025 18:03:46 -0500
Rob Herring <robh@kernel.org> wrote:

> On Tue, Apr 01, 2025 at 12:35:07PM +0200, Lukasz Majewski wrote:
> > Hi Rob,
> >   
> > > On Mon, Mar 31, 2025 at 12:31:13PM +0200, Lukasz Majewski wrote:  
> > > > This patch provides description of the MTIP L2 switch available
> > > > in some NXP's SOCs - e.g. imx287.
> > > > 
> > > > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > > > ---
> > > > Changes for v2:
> > > > - Rename the file to match exactly the compatible
> > > >   (nxp,imx287-mtip-switch)
> > > > 
> > > > Changes for v3:
> > > > - Remove '-' from const:'nxp,imx287-mtip-switch'
> > > > - Use '^port@[12]+$' for port patternProperties
> > > > - Drop status = "okay";
> > > > - Provide proper indentation for 'example' binding (replace 8
> > > >   spaces with 4 spaces)
> > > > - Remove smsc,disable-energy-detect; property
> > > > - Remove interrupt-parent and interrupts properties as not
> > > > required
> > > > - Remove #address-cells and #size-cells from required properties
> > > > check
> > > > - remove description from reg:
> > > > - Add $ref: ethernet-switch.yaml#
> > > > ---
> > > >  .../bindings/net/nxp,imx287-mtip-switch.yaml  | 154
> > > > ++++++++++++++++++ 1 file changed, 154 insertions(+)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > > > b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > > > new file mode 100644 index 000000000000..98eba3665f32 ---
> > > > /dev/null +++
> > > > b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml
> > > > @@ -0,0 +1,154 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR
> > > > BSD-2-Clause) +%YAML 1.2
> > > > +---
> > > > +$id:
> > > > http://devicetree.org/schemas/net/nxp,imx287-mtip-switch.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# +
> > > > +title: NXP SoC Ethernet Switch Controller (L2 MoreThanIP
> > > > switch) +
> > > > +maintainers:
> > > > +  - Lukasz Majewski <lukma@denx.de>
> > > > +
> > > > +description:
> > > > +  The 2-port switch ethernet subsystem provides ethernet packet
> > > > (L2)
> > > > +  communication and can be configured as an ethernet switch. It
> > > > provides the
> > > > +  reduced media independent interface (RMII), the management
> > > > data input
> > > > +  output (MDIO) for physical layer device (PHY) management.
> > > > +
> > > > +$ref: ethernet-switch.yaml#    
> > > 
> > > This needs to be: ethernet-switch.yaml#/$defs/ethernet-ports
> > > 
> > > With that, you can drop much of the below part. More below...
> > >   
> > > > +
> > > > +properties:  
> > 
> > So it shall be after the "properties:"
> > 
> > $ref: ethernet-switch.yaml#/$defs/ethernet-ports   [*]  
> 
> It can stay where it is, just add "/$defs/ethernet-ports"
> 
> 
> > > > +  compatible:
> > > > +    const: nxp,imx287-mtip-switch
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  phy-supply:
> > > > +    description:
> > > > +      Regulator that powers Ethernet PHYs.
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: Register accessing clock
> > > > +      - description: Bus access clock
> > > > +      - description: Output clock for external device - e.g.
> > > > PHY source clock
> > > > +      - description: IEEE1588 timer clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: ipg
> > > > +      - const: ahb
> > > > +      - const: enet_out
> > > > +      - const: ptp
> > > > +
> > > > +  interrupts:
> > > > +    items:
> > > > +      - description: Switch interrupt
> > > > +      - description: ENET0 interrupt
> > > > +      - description: ENET1 interrupt
> > > > +
> > > > +  pinctrl-names: true
> > > > +    
> > >   
> > > > +  ethernet-ports:  
> > 
> > And then this "node" can be removed as it has been referenced above
> > [*]?  
> 
> Well, you have to keep it to have 'required' in the child nodes.
> 
> > 
> > (I shall only keep the properties, which are not standard to [*] if
> > any).
> >   
> > > > +    type: object
> > > > +    additionalProperties: false
> > > > +
> > > > +    properties:
> > > > +      '#address-cells':
> > > > +        const: 1
> > > > +      '#size-cells':
> > > > +        const: 0
> > > > +
> > > > +    patternProperties:
> > > > +      '^port@[12]+$':    
> > > 
> > > Note that 'ethernet-port' is the preferred node name though
> > > 'port' is allowed.  
> > 
> > Ok. That would be the correct define:
> > 
> > ethernet-ports {
> >      mtip_port1: ethernet-port@1 {
> >                reg = <1>;
> >                label = "lan0";
> >                local-mac-address = [ 00 00 00 00 00 00 ];
> >                phy-mode = "rmii";
> >                phy-handle = <&ethphy0>;
> > 	       };
> > 
> >                mtip_port2: port@2 {  
> 
> And here...
> 
> > 		....
> > 	       };
> >   
> > >   
> > > > +        type: object
> > > > +        description: MTIP L2 switch external ports
> > > > +
> > > > +        $ref: ethernet-controller.yaml#
> > > > +        unevaluatedProperties: false
> > > > +
> > > > +        properties:
> > > > +          reg:
> > > > +            items:
> > > > +              - enum: [1, 2]
> > > > +            description: MTIP L2 switch port number
> > > > +
> > > > +          label:
> > > > +            description: Label associated with this port
> > > > +
> > > > +        required:
> > > > +          - reg
> > > > +          - label
> > > > +          - phy-mode
> > > > +          - phy-handle    
> > > 
> > > All the above under 'ethernet-ports' can be dropped though you
> > > might want to keep 'required' part.  
> > 
> > Ok, I will keep it.  
> 
> So I think you just want this:
> 
> ethernet-ports:
>   type: object
>   additionalProperties: true  # Check if you need this
> 
>   patternProperties:
>     '^ethernet-port@[12]$':
>       required:
>         - label
>         - phy-mode
>         - phy-handle
> 

I had to add:

$ref: ethernet-switch.yaml#/$defs/ethernet-ports
patternProperties:
  "^(ethernet-)?ports$":
    type: object
    additionalProperties: true

after the 
description:

to avoid issues when checking DT schema.

Simple adding 

   patternProperties:
     '^ethernet-port@[12]$':
       required:
         - label
         - phy-mode
         - phy-handle

Causes more errors.

> Rob




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 2/4] ARM: dts: nxp: mxs: Adjust the imx28.dtsi L2 switch description

[Andrew Lunn]

On Mon, Mar 31, 2025 at 12:31:14PM +0200, Lukasz Majewski wrote:
> The current range of 'reg' property is too small to allow full control
> of the L2 switch on imx287.
> 
> As this IP block also uses ENET-MAC blocks for its operation, the address
> range for it must be included as well.
> 
> Moreover, some SoC common properties (like compatible, clocks, interrupts
> numbers) have been moved to this node.
> 
> Signed-off-by: Lukasz Majewski <lukma@denx.de>

Assuming it passed the DT checking:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH v3 4/4] net: mtip: The L2 switch driver for imx287

[Andrew Lunn]

> +static void read_atable(struct switch_enet_private *fep, int index,
> +			unsigned long *read_lo, unsigned long *read_hi)
> +{
> +	unsigned long atable_base = (unsigned long)fep->hwentry;
> +
> +	*read_lo = readl((const void *)atable_base + (index << 3));
> +	*read_hi = readl((const void *)atable_base + (index << 3) + 4);
> +}
> +
> +static void write_atable(struct switch_enet_private *fep, int index,
> +			 unsigned long write_lo, unsigned long write_hi)
> +{
> +	unsigned long atable_base = (unsigned long)fep->hwentry;
> +
> +	writel(write_lo, (void *)atable_base + (index << 3));
> +	writel(write_hi, (void *)atable_base + (index << 3) + 4);
> +}

It would be nice to have the mtip_ prefix on all functions.

> +static int mtip_open(struct net_device *dev)
> +{
> +	struct mtip_ndev_priv *priv = netdev_priv(dev);
> +	struct switch_enet_private *fep = priv->fep;
> +	int ret, port_idx = priv->portnum - 1;
> +
> +	if (fep->usage_count == 0) {
> +		clk_enable(fep->clk_ipg);
> +		netif_napi_add(dev, &fep->napi, mtip_rx_napi);
> +
> +		ret = mtip_alloc_buffers(dev);
> +		if (ret)
> +			return ret;

nitpick: You might want to turn the clock off before returning the
error.

> +	}
> +
> +	fep->link[port_idx] = 0;
> +
> +	/* Probe and connect to PHY when open the interface, if already
> +	 * NOT done in the switch driver probe (or when the device is
> +	 * re-opened).
> +	 */
> +	ret = mtip_mii_probe(dev);
> +	if (ret) {
> +		mtip_free_buffers(dev);

I've not checked. Does this do the opposite of netif_napi_add()?

> +static void mtip_set_multicast_list(struct net_device *dev)
> +{
> +	unsigned int i, bit, data, crc;
> +
> +	if (dev->flags & IFF_PROMISC) {
> +		dev_info(&dev->dev, "%s: IFF_PROMISC\n", __func__);

You can save one level of indentation with a return here.

> +	} else {
> +		if (dev->flags & IFF_ALLMULTI) {
> +			dev_info(&dev->dev, "%s: IFF_ALLMULTI\n", __func__);

and other level here.

> +		} else {
> +			struct netdev_hw_addr *ha;
> +			u_char *addrs;
> +
> +			netdev_for_each_mc_addr(ha, dev) {
> +				addrs = ha->addr;
> +				/* Only support group multicast for now */
> +				if (!(*addrs & 1))
> +					continue;

You could pull there CRC caluclation out into a helper. You might also
want to search the tree and see if it exists somewhere else.

> +
> +				/* calculate crc32 value of mac address */
> +				crc = 0xffffffff;
> +
> +				for (i = 0; i < 6; i++) {

Is 6 the lengh of a MAC address? There is a #define for that.

> +					data = addrs[i];
> +					for (bit = 0; bit < 8;
> +					     bit++, data >>= 1) {
> +						crc = (crc >> 1) ^
> +						(((crc ^ data) & 1) ?
> +						CRC32_POLY : 0);
> +					}
> +				}
> +			}
> +		}
> +	}
> +}
> +

> +struct switch_enet_private *mtip_netdev_get_priv(const struct net_device *ndev)
> +{
> +	if (ndev->netdev_ops == &mtip_netdev_ops)
> +		return netdev_priv(ndev);
> +
> +	return NULL;
> +}
> +

> +static int __init mtip_switch_dma_init(struct switch_enet_private *fep)
> +{
> +	struct cbd_t *bdp, *cbd_base;
> +	int ret, i;
> +
> +	/* Check mask of the streaming and coherent API */
> +	ret = dma_set_mask_and_coherent(&fep->pdev->dev, DMA_BIT_MASK(32));
> +	if (ret < 0) {
> +		dev_warn(&fep->pdev->dev, "No suitable DMA available\n");

Can you recover from this? Or should it be dev_err()?

More later...

	Andrew

Re: [PATCH v3 4/4] net: mtip: The L2 switch driver for imx287

[Andrew Lunn]

> +struct switch_enet_private *mtip_netdev_get_priv(const struct net_device *ndev)
> +{
> +	if (ndev->netdev_ops == &mtip_netdev_ops)
> +		return netdev_priv(ndev);
> +
> +	return NULL;
> +}

> +static bool mtip_port_dev_check(const struct net_device *ndev)
> +{
> +	if (!mtip_netdev_get_priv(ndev))
> +		return false;
> +
> +	return true;
> +}

This appears to be the only use of mtip_netdev_get_priv(). It does not
care what priv actually is. In my previous review i said i think you
can simply this. I still think that is true.

    Andrew

Re: [PATCH v3 4/4] net: mtip: The L2 switch driver for imx287

[Andrew Lunn]

> +struct switch_enet_private *mtip_netdev_get_priv(const struct net_device *ndev)
> +{
> +	if (ndev->netdev_ops == &mtip_netdev_ops)
> +		return netdev_priv(ndev);
> +
> +	return NULL;
> +}

> +static bool mtip_port_dev_check(const struct net_device *ndev)
> +{
> +	if (!mtip_netdev_get_priv(ndev))
> +		return false;
> +
> +	return true;
> +}
> +

Rearranging the code a bit to make my point....

mtip_port_dev_check() tells us if this ndev is one of the ports of
this switch.

> +/* netdev notifier */
> +static int mtip_netdevice_event(struct notifier_block *unused,
> +				unsigned long event, void *ptr)
> +{
> +	struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
> +	struct netdev_notifier_changeupper_info *info;
> +	int ret = NOTIFY_DONE;
> +
> +	if (!mtip_port_dev_check(ndev))
> +		return NOTIFY_DONE;

We have received a notification about some interface. This filters out
all but the switches interfaces.

> +
> +	switch (event) {
> +	case NETDEV_CHANGEUPPER:
> +		info = ptr;

CHANGERUPPER is that a netdev has been added or removed from a bridge,
or some other sort of master device, e.g. a bond.

> +
> +		if (netif_is_bridge_master(info->upper_dev)) {
> +			if (info->linking)
> +				ret = mtip_ndev_port_link(ndev,
> +							  info->upper_dev);

Call mtip_ndev_port_link() has been added to some bridge.

> +static int mtip_ndev_port_link(struct net_device *ndev,
> +			       struct net_device *br_ndev)
> +{
> +	struct mtip_ndev_priv *priv = netdev_priv(ndev);
> +	struct switch_enet_private *fep = priv->fep;
> +
> +	dev_dbg(&ndev->dev, "%s: ndev: %s br: %s fep: 0x%x\n",
> +		__func__, ndev->name,  br_ndev->name, (unsigned int)fep);
> +
> +	/* Check if MTIP switch is already enabled */
> +	if (!fep->br_offload) {
> +		if (!priv->master_dev)
> +			priv->master_dev = br_ndev;
> +
> +		fep->br_offload = 1;
> +		mtip_switch_dis_port_separation(fep);
> +		mtip_clear_atable(fep);
> +	}

So lets consider

ip link add br0 type bridge
ip link add br1 type bridge
ip link set dev lan1 master br0

We create two bridges, and add the first port to one of the bridges.

fep->br_offload should be False
priv->master_dev should be NULL.

So fep->br_offload is set to 1, priv->master_dev is set to br0 and the
separation between the ports is removed.

It seems like the hardware will now be bridging packets between the
two interfaces, despite lan2 not being a member of any bridge....

Now

ip link set dev lan2 master br1

I make the second port a member of some other bridge. fep->br_offload
is True, so nothing happens.

This is why i said this code needs expanding.

If you look at other switch drivers, you will see each port keeps
track of what bridge it has been joined to. There is then logic which
iterates over the ports, finds which ports are members of the same
bridge, and enables packets to flow between those ports.

With only two ports, you can make some simplifications, but you should
only disable the separation once both ports are the member of the same
bridge.

	Andrew

Re: [PATCH v3 4/4] net: mtip: The L2 switch driver for imx287

[Lukasz Majewski]

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

Hi Andrew,

> > +static void read_atable(struct switch_enet_private *fep, int index,
> > +			unsigned long *read_lo, unsigned long
> > *read_hi) +{
> > +	unsigned long atable_base = (unsigned long)fep->hwentry;
> > +
> > +	*read_lo = readl((const void *)atable_base + (index << 3));
> > +	*read_hi = readl((const void *)atable_base + (index << 3)
> > + 4); +}
> > +
> > +static void write_atable(struct switch_enet_private *fep, int
> > index,
> > +			 unsigned long write_lo, unsigned long
> > write_hi) +{
> > +	unsigned long atable_base = (unsigned long)fep->hwentry;
> > +
> > +	writel(write_lo, (void *)atable_base + (index << 3));
> > +	writel(write_hi, (void *)atable_base + (index << 3) + 4);
> > +}  
> 
> It would be nice to have the mtip_ prefix on all functions.

Ok.

> 
> > +static int mtip_open(struct net_device *dev)
> > +{
> > +	struct mtip_ndev_priv *priv = netdev_priv(dev);
> > +	struct switch_enet_private *fep = priv->fep;
> > +	int ret, port_idx = priv->portnum - 1;
> > +
> > +	if (fep->usage_count == 0) {
> > +		clk_enable(fep->clk_ipg);
> > +		netif_napi_add(dev, &fep->napi, mtip_rx_napi);
> > +
> > +		ret = mtip_alloc_buffers(dev);
> > +		if (ret)
> > +			return ret;  
> 
> nitpick: You might want to turn the clock off before returning the
> error.

Ok.

> 
> > +	}
> > +
> > +	fep->link[port_idx] = 0;
> > +
> > +	/* Probe and connect to PHY when open the interface, if
> > already
> > +	 * NOT done in the switch driver probe (or when the device
> > is
> > +	 * re-opened).
> > +	 */
> > +	ret = mtip_mii_probe(dev);
> > +	if (ret) {
> > +		mtip_free_buffers(dev);  
> 
> I've not checked. Does this do the opposite of netif_napi_add()?

No, the netif_napi_add() is required here as well.

> 
> > +static void mtip_set_multicast_list(struct net_device *dev)
> > +{
> > +	unsigned int i, bit, data, crc;
> > +
> > +	if (dev->flags & IFF_PROMISC) {
> > +		dev_info(&dev->dev, "%s: IFF_PROMISC\n",
> > __func__);  
> 
> You can save one level of indentation with a return here.

Ok.

> 
> > +	} else {
> > +		if (dev->flags & IFF_ALLMULTI) {
> > +			dev_info(&dev->dev, "%s: IFF_ALLMULTI\n",
> > __func__);  
> 
> and other level here.

Ok.

> 
> > +		} else {
> > +			struct netdev_hw_addr *ha;
> > +			u_char *addrs;
> > +
> > +			netdev_for_each_mc_addr(ha, dev) {
> > +				addrs = ha->addr;
> > +				/* Only support group multicast
> > for now */
> > +				if (!(*addrs & 1))
> > +					continue;  
> 
> You could pull there CRC caluclation out into a helper. You might also
> want to search the tree and see if it exists somewhere else.
> 

The ether_crc_le(ndev->addr_len,·ha->addr); could be the replacement.

However, when I look on the code and compare it with fec_main.c's
set_multicast_list() - it looks like a dead code.

The calculated hash is not used at all (in fec_main.c it is written to
some registers).

I've refactored the code to do similar things, but taking into account
already set switch setup (promisc must be enabled from the outset).

> > +
> > +				/* calculate crc32 value of mac
> > address */
> > +				crc = 0xffffffff;
> > +
> > +				for (i = 0; i < 6; i++) {  
> 
> Is 6 the lengh of a MAC address? There is a #define for that.

This is not needed and can be replaced with already present function.

> 
> > +					data = addrs[i];
> > +					for (bit = 0; bit < 8;
> > +					     bit++, data >>= 1) {
> > +						crc = (crc >> 1) ^
> > +						(((crc ^ data) &
> > 1) ?
> > +						CRC32_POLY : 0);
> > +					}
> > +				}
> > +			}
> > +		}
> > +	}
> > +}
> > +  
> 
> > +struct switch_enet_private *mtip_netdev_get_priv(const struct
> > net_device *ndev) +{
> > +	if (ndev->netdev_ops == &mtip_netdev_ops)
> > +		return netdev_priv(ndev);
> > +
> > +	return NULL;
> > +}
> > +  
> 
> > +static int __init mtip_switch_dma_init(struct switch_enet_private
> > *fep) +{
> > +	struct cbd_t *bdp, *cbd_base;
> > +	int ret, i;
> > +
> > +	/* Check mask of the streaming and coherent API */
> > +	ret = dma_set_mask_and_coherent(&fep->pdev->dev,
> > DMA_BIT_MASK(32));
> > +	if (ret < 0) {
> > +		dev_warn(&fep->pdev->dev, "No suitable DMA
> > available\n");  
> 
> Can you recover from this? Or should it be dev_err()?
> 

It was my mistake - of course there shall be dev_err().

> More later...
> 
> 	Andrew


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 4/4] net: mtip: The L2 switch driver for imx287

[Lukasz Majewski]

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

Hi Andrew,

> > +struct switch_enet_private *mtip_netdev_get_priv(const struct
> > net_device *ndev) +{
> > +	if (ndev->netdev_ops == &mtip_netdev_ops)
> > +		return netdev_priv(ndev);
> > +
> > +	return NULL;
> > +}  
> 
> > +static bool mtip_port_dev_check(const struct net_device *ndev)
> > +{
> > +	if (!mtip_netdev_get_priv(ndev))
> > +		return false;
> > +
> > +	return true;
> > +}
> > +  
> 
> Rearranging the code a bit to make my point....
> 
> mtip_port_dev_check() tells us if this ndev is one of the ports of
> this switch.
> 
> > +/* netdev notifier */
> > +static int mtip_netdevice_event(struct notifier_block *unused,
> > +				unsigned long event, void *ptr)
> > +{
> > +	struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
> > +	struct netdev_notifier_changeupper_info *info;
> > +	int ret = NOTIFY_DONE;
> > +
> > +	if (!mtip_port_dev_check(ndev))
> > +		return NOTIFY_DONE;  
> 
> We have received a notification about some interface. This filters out
> all but the switches interfaces.
> 
> > +
> > +	switch (event) {
> > +	case NETDEV_CHANGEUPPER:
> > +		info = ptr;  
> 
> CHANGERUPPER is that a netdev has been added or removed from a bridge,
> or some other sort of master device, e.g. a bond.
> 
> > +
> > +		if (netif_is_bridge_master(info->upper_dev)) {
> > +			if (info->linking)
> > +				ret = mtip_ndev_port_link(ndev,
> > +
> > info->upper_dev);  
> 
> Call mtip_ndev_port_link() has been added to some bridge.
> 
> > +static int mtip_ndev_port_link(struct net_device *ndev,
> > +			       struct net_device *br_ndev)
> > +{
> > +	struct mtip_ndev_priv *priv = netdev_priv(ndev);
> > +	struct switch_enet_private *fep = priv->fep;
> > +
> > +	dev_dbg(&ndev->dev, "%s: ndev: %s br: %s fep: 0x%x\n",
> > +		__func__, ndev->name,  br_ndev->name, (unsigned
> > int)fep); +
> > +	/* Check if MTIP switch is already enabled */
> > +	if (!fep->br_offload) {
> > +		if (!priv->master_dev)
> > +			priv->master_dev = br_ndev;
> > +
> > +		fep->br_offload = 1;
> > +		mtip_switch_dis_port_separation(fep);
> > +		mtip_clear_atable(fep);
> > +	}  
> 
> So lets consider
> 
> ip link add br0 type bridge
> ip link add br1 type bridge
> ip link set dev lan1 master br0
> 
> We create two bridges, and add the first port to one of the bridges.
> 
> fep->br_offload should be False
> priv->master_dev should be NULL.
> 
> So fep->br_offload is set to 1, priv->master_dev is set to br0 and the
> separation between the ports is removed.
> 
> It seems like the hardware will now be bridging packets between the
> two interfaces, despite lan2 not being a member of any bridge....
> 
> Now
> 
> ip link set dev lan2 master br1
> 
> I make the second port a member of some other bridge. fep->br_offload
> is True, so nothing happens.
> 
> This is why i said this code needs expanding.
> 
> If you look at other switch drivers, you will see each port keeps
> track of what bridge it has been joined to. There is then logic which
> iterates over the ports, finds which ports are members of the same
> bridge, and enables packets to flow between those ports.
> 
> With only two ports, you can make some simplifications, but you should
> only disable the separation once both ports are the member of the same
> bridge.
> 

I think that I do have your point. Thanks for the info.

> 	Andrew




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]