| Message ID | 20250328133544.4149716-2-lukma@denx.de (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: mtip: Add support for MTIP imx287 L2 switch driver | expand |
On 28/03/2025 14:35, 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) Please implement all the changes, not only the rename. I gave several comments, although quick glance suggests you did implement them, so then changelog is just incomplete. > --- > .../bindings/net/nxp,imx287-mtip-switch.yaml | 165 ++++++++++++++++++ > 1 file changed, 165 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..a3e0fe7783ec > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml > @@ -0,0 +1,165 @@ > +# 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. > + If this is ethernet switch, why it does not reference ethernet-switch schema? or dsa.yaml or dsa/ethernet-ports? I am not sure which one should go here, but surprising to see none. > +properties: > + compatible: > + const: nxp,imx287-mtip--switch Just one -. > + > + reg: > + maxItems: 1 > + description: > + The physical base address and size of the MTIP L2 SW module IO range Wasn't here, drop. > + > + 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 Drop > + > + ethernet-ports: > + type: object > + additionalProperties: false > + > + properties: > + '#address-cells': > + const: 1 > + '#size-cells': > + const: 0 > + > + patternProperties: > + "^port@[0-9]+$": Keep consistent quotes, either " or '. Also [01] > + 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 > + - '#address-cells' > + - '#size-cells' > + > +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 = <®_fec_3v3>; > + interrupts = <100>, <101>, <102>; > + clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>; > + clock-names = "ipg", "ahb", "enet_out", "ptp"; > + status = "okay"; Drop > + > + ethernet-ports { > + #address-cells = <1>; > + #size-cells = <0>; Messed indentation. See example-schema or writing-schema. Best regards, Krzysztof
> + 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 (AND'ed) */ > + interrupt-parent = <&gpio4>; > + interrupts = <13 IRQ_TYPE_EDGE_FALLING>; Shared interrupts cannot be edge. They are level, so that either can hold the interrupt active until it is cleared. Also, PHY interrupts in general are level, because there are multiple interrupt sources within the PHY, and you need to clear them all before the interrupt is released. Andrew
> + /* Both PHYs (i.e. 0,1) have the same, single GPIO, */ > + /* line to handle both, their interrupts (AND'ed) */ ORed, not ANDed. Often, the interrupt line has a weak pullup resistor, so by default it is high. Either PHY can then pull it low, using an open collector, which is HI-Z when not driving. Andrew
On Fri, 28 Mar 2025 14:35:41 +0100, 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) > --- > .../bindings/net/nxp,imx287-mtip-switch.yaml | 165 ++++++++++++++++++ > 1 file changed, 165 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.example.dtb: /example-0/switch@800f0000: failed to match any schema with compatible: ['nxp,imx287-mtip-switch'] /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.example.dtb: ethernet-phy@0: interrupts: [[13], [2]] is too long from schema $id: http://devicetree.org/schemas/net/ethernet-phy.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.example.dtb: ethernet-phy@1: interrupts: [[13], [2]] is too long from schema $id: http://devicetree.org/schemas/net/ethernet-phy.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250328133544.4149716-2-lukma@denx.de The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Fri, Mar 28, 2025 at 02:35:41PM +0100, 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) > --- > .../bindings/net/nxp,imx287-mtip-switch.yaml | 165 ++++++++++++++++++ > 1 file changed, 165 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..a3e0fe7783ec > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml > @@ -0,0 +1,165 @@ > +# 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. > + > +properties: > + compatible: > + const: nxp,imx287-mtip--switch > + > + reg: > + maxItems: 1 > + description: > + The physical base address and size of the MTIP L2 SW module IO range > + > + 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@[0-9]+$": > + 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 > + - '#address-cells' > + - '#size-cells' > + > +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 = <®_fec_3v3>; > + interrupts = <100>, <101>, <102>; > + clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>; > + clock-names = "ipg", "ahb", "enet_out", "ptp"; > + 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 = <ðphy0>; > + }; > + > + mtip_port2: port@2 { > + reg = <2>; > + label = "lan1"; > + local-mac-address = [ 00 00 00 00 00 00 ]; > + phy-mode = "rmii"; > + phy-handle = <ðphy1>; > + }; > + }; > + > + 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>; > + smsc,disable-energy-detect; With a custom property, you should have a specific compatible. > + /* Both PHYs (i.e. 0,1) have the same, single GPIO, */ > + /* line to handle both, their interrupts (AND'ed) */ > + interrupt-parent = <&gpio4>; > + interrupts = <13 IRQ_TYPE_EDGE_FALLING>; The error report is because the examples have to guess the number of provider interrupt cells and only 1 guess is supported. It guessed 1 from above. In any case, unless the phys are built-in and fixed, they are out of scope of this binding. So perhaps drop the interrupts and smsc property. Rob
Hi Rob, > On Fri, Mar 28, 2025 at 02:35:41PM +0100, 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) > > --- > > .../bindings/net/nxp,imx287-mtip-switch.yaml | 165 > > ++++++++++++++++++ 1 file changed, 165 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..a3e0fe7783ec --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml > > @@ -0,0 +1,165 @@ +# 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. > > + > > +properties: > > + compatible: > > + const: nxp,imx287-mtip--switch > > + > > + reg: > > + maxItems: 1 > > + description: > > + The physical base address and size of the MTIP L2 SW module > > IO range + > > + 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@[0-9]+$": > > + 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 > > + - '#address-cells' > > + - '#size-cells' > > + > > +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 = <®_fec_3v3>; > > + interrupts = <100>, <101>, <102>; > > + clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>; > > + clock-names = "ipg", "ahb", "enet_out", "ptp"; > > + 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 = <ðphy0>; > > + }; > > + > > + mtip_port2: port@2 { > > + reg = <2>; > > + label = "lan1"; > > + local-mac-address = [ 00 00 00 00 00 00 ]; > > + phy-mode = "rmii"; > > + phy-handle = <ðphy1>; > > + }; > > + }; > > + > > + 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>; > > + smsc,disable-energy-detect; > > With a custom property, you should have a specific compatible. I could add: compatible = "ethernet-phy-id0007.c0f0","ethernet-phy-ieee802.3-c22"; but it would not make things either clearer nor simpler. I will just remove this property. > > > + /* Both PHYs (i.e. 0,1) have the same, > > single GPIO, */ > > + /* line to handle both, their interrupts > > (AND'ed) */ > > + interrupt-parent = <&gpio4>; > > + interrupts = <13 IRQ_TYPE_EDGE_FALLING>; > > The error report is because the examples have to guess the number of > provider interrupt cells and only 1 guess is supported. It guessed 1 > from above. > > In any case, unless the phys are built-in and fixed, they are out of > scope of this binding. So perhaps drop the interrupts and smsc > property. Ok, I will remove them. > > 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
Hi Krzysztof, > On 28/03/2025 14:35, 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) > > Please implement all the changes, not only the rename. I gave several > comments, although quick glance suggests you did implement them, so > then changelog is just incomplete. Those comments were IMHO addressed automatically, as this time I took (I suppose :-) ) better file as a starting point. To be more specific it was: ./Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml as I've been advised to use for the MTIP driver the same DTS description as the above one has (i.e. they are conceptually similar, so DTS description ABI can be used for both). I've also checked the: make CHECK_DTBS=y DT_SCHEMA_FILES=nxp,imx287-mtip-switch.yaml nxp/mxs/imx28-xea.dtb on Linux next and it gave no errors. > > > --- > > .../bindings/net/nxp,imx287-mtip-switch.yaml | 165 > > ++++++++++++++++++ 1 file changed, 165 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..a3e0fe7783ec --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml > > @@ -0,0 +1,165 @@ +# 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. > > + > > If this is ethernet switch, why it does not reference ethernet-switch > schema? or dsa.yaml or dsa/ethernet-ports? I am not sure which one > should go here, but surprising to see none. It uses: $ref:·ethernet-controller.yaml# for "ports". Other crucial node is "mdio", which references $ref: mdio.yaml# > > > +properties: > > + compatible: > > + const: nxp,imx287-mtip--switch > > Just one -. > Ok. > > + > > + reg: > > + maxItems: 1 > > + description: > > + The physical base address and size of the MTIP L2 SW module > > IO range > > Wasn't here, drop. > The 'reg' property (reg = <0x800f0000 0x20000>;) is defined in imx28.dtsi, where the SoC generic properties (as suggested by Andrew - like clocks, interrupts, clock-names) are moved. > > + > > + 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 > > Drop The 'pinctrl-names = "default";' are specified. Shouldn't it be kept? > > > + > > + ethernet-ports: > > + type: object > > + additionalProperties: false > > + > > + properties: > > + '#address-cells': > > + const: 1 > > + '#size-cells': > > + const: 0 > > + > > + patternProperties: > > + "^port@[0-9]+$": > > Keep consistent quotes, either " or '. Also [01] > [12] - ports are numbered starting from 1. > > > + 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 > > + - '#address-cells' > > + - '#size-cells' > > + > > +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 = <®_fec_3v3>; > > + interrupts = <100>, <101>, <102>; > > + clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>; > > + clock-names = "ipg", "ahb", "enet_out", "ptp"; > > + status = "okay"; > > Drop Ok. > > > + > > + ethernet-ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > Messed indentation. See example-schema or writing-schema. > Ok. > > > Best regards, > Krzysztof 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
Hi Andrew, > > + 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 > > (AND'ed) */ > > + interrupt-parent = <&gpio4>; > > + interrupts = <13 IRQ_TYPE_EDGE_FALLING>; > > Shared interrupts cannot be edge. They are level, so that either can > hold the interrupt active until it is cleared. > > Also, PHY interrupts in general are level, because there are multiple > interrupt sources within the PHY, and you need to clear them all > before the interrupt is released. Good point, I will update it accordingly. Thanks. > > 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
Hi Andrew, > > + /* Both PHYs (i.e. 0,1) have the same, > > single GPIO, */ > > + /* line to handle both, their interrupts > > (AND'ed) */ > > ORed, not ANDed. > Yes, correct. > Often, the interrupt line has a weak pullup resistor, so by default it > is high. Either PHY can then pull it low, using an open collector, > which is HI-Z when not driving. Yes, correct - this is how it works. > > 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
On 29/03/2025 23:10, Lukasz Majewski wrote: >>> + >> >> If this is ethernet switch, why it does not reference ethernet-switch >> schema? or dsa.yaml or dsa/ethernet-ports? I am not sure which one >> should go here, but surprising to see none. > > It uses: > $ref:·ethernet-controller.yaml# > > for "ports". > > Other crucial node is "mdio", which references $ref: mdio.yaml# These are children, I am speaking about this device node. > >> >>> +properties: >>> + compatible: >>> + const: nxp,imx287-mtip--switch >> >> Just one -. >> > > Ok. > >>> + >>> + reg: >>> + maxItems: 1 >>> + description: >>> + The physical base address and size of the MTIP L2 SW module >>> IO range >> >> Wasn't here, drop. >> > > The 'reg' property (reg = <0x800f0000 0x20000>;) is defined in > imx28.dtsi, where the SoC generic properties (as suggested by Andrew - > like clocks, interrupts, clock-names) are moved. Drop description, not the reg. Reg was in the previous version. You added random changes here, not coming from the previous review. Best regards, Krzysztof
Hi Krzysztof, > On 29/03/2025 23:10, Lukasz Majewski wrote: > >>> + > >> > >> If this is ethernet switch, why it does not reference > >> ethernet-switch schema? or dsa.yaml or dsa/ethernet-ports? I am > >> not sure which one should go here, but surprising to see none. > > > > It uses: > > $ref:·ethernet-controller.yaml# > > > > for "ports". > > > > Other crucial node is "mdio", which references $ref: mdio.yaml# > > These are children, I am speaking about this device node. It looks like there is no such reference. I've checked the aforementioned ti,cpsw-switch.yaml, microchip,lan966x-switch.yaml and renesas,r8a779f0-ether-switch.yaml. Those only have $ref: for ethernet-port children node. The "outer" one doesn't have it. Or am I missing something? > > > > >> > >>> +properties: > >>> + compatible: > >>> + const: nxp,imx287-mtip--switch > >> > >> Just one -. > >> > > > > Ok. > > > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + description: > >>> + The physical base address and size of the MTIP L2 SW module > >>> IO range > >> > >> Wasn't here, drop. > >> > > > > The 'reg' property (reg = <0x800f0000 0x20000>;) is defined in > > imx28.dtsi, where the SoC generic properties (as suggested by > > Andrew - like clocks, interrupts, clock-names) are moved. > > Drop description, not the reg. Reg was in the previous version. You > added random changes here, not coming from the previous review. > Ach... You mean the "description" in the: reg: maxItems: 1 description: XX YY Ok, I will remove it. > Best regards, > Krzysztof 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
On 30/03/2025 23:04, Lukasz Majewski wrote: > Hi Krzysztof, > >> On 29/03/2025 23:10, Lukasz Majewski wrote: >>>>> + >>>> >>>> If this is ethernet switch, why it does not reference >>>> ethernet-switch schema? or dsa.yaml or dsa/ethernet-ports? I am >>>> not sure which one should go here, but surprising to see none. >>> >>> It uses: >>> $ref:·ethernet-controller.yaml# >>> >>> for "ports". >>> >>> Other crucial node is "mdio", which references $ref: mdio.yaml# >> >> These are children, I am speaking about this device node. > > It looks like there is no such reference. > > I've checked the aforementioned ti,cpsw-switch.yaml, > microchip,lan966x-switch.yaml and renesas,r8a779f0-ether-switch.yaml. > > Those only have $ref: for ethernet-port children node. > > The "outer" one doesn't have it. > > > Or am I missing something? There is ethernet-switch.yaml for non-DSA switches and there is DSA (using ethernet switch, btw). I don't know why these devices do not use it, I guess no one converted them after we split ethernet-switch out of DSA. Best regards, Krzysztof
Hi Krzysztof, > On 30/03/2025 23:04, Lukasz Majewski wrote: > > Hi Krzysztof, > > > >> On 29/03/2025 23:10, Lukasz Majewski wrote: > >>>>> + > >>>> > >>>> If this is ethernet switch, why it does not reference > >>>> ethernet-switch schema? or dsa.yaml or dsa/ethernet-ports? I am > >>>> not sure which one should go here, but surprising to see none. > >>>> > >>> > >>> It uses: > >>> $ref:·ethernet-controller.yaml# > >>> > >>> for "ports". > >>> > >>> Other crucial node is "mdio", which references $ref: mdio.yaml# > >>> > >> > >> These are children, I am speaking about this device node. > > > > It looks like there is no such reference. > > > > I've checked the aforementioned ti,cpsw-switch.yaml, > > microchip,lan966x-switch.yaml and > > renesas,r8a779f0-ether-switch.yaml. > > > > Those only have $ref: for ethernet-port children node. > > > > The "outer" one doesn't have it. > > > > > > Or am I missing something? > > There is ethernet-switch.yaml for non-DSA switches and there is DSA > (using ethernet switch, btw). I don't know why these devices do not > use it, I guess no one converted them after we split ethernet-switch > out of DSA. In net next there is: Documentation/devicetree/bindings/net/dsa/dsa.yaml Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml which uses $ref: ethernet-switch.yaml# I will add it in a similar way as it is in dsa.yaml. > > Best regards, > Krzysztof 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
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..a3e0fe7783ec --- /dev/null +++ b/Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml @@ -0,0 +1,165 @@ +# 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. + +properties: + compatible: + const: nxp,imx287-mtip--switch + + reg: + maxItems: 1 + description: + The physical base address and size of the MTIP L2 SW module IO range + + 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@[0-9]+$": + 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 + - '#address-cells' + - '#size-cells' + +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 = <®_fec_3v3>; + interrupts = <100>, <101>, <102>; + clocks = <&clks 57>, <&clks 57>, <&clks 64>, <&clks 35>; + clock-names = "ipg", "ahb", "enet_out", "ptp"; + 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 = <ðphy0>; + }; + + mtip_port2: port@2 { + reg = <2>; + label = "lan1"; + local-mac-address = [ 00 00 00 00 00 00 ]; + phy-mode = "rmii"; + phy-handle = <ðphy1>; + }; + }; + + 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>; + smsc,disable-energy-detect; + /* Both PHYs (i.e. 0,1) have the same, single GPIO, */ + /* line to handle both, their interrupts (AND'ed) */ + interrupt-parent = <&gpio4>; + interrupts = <13 IRQ_TYPE_EDGE_FALLING>; + }; + + ethphy1: ethernet-phy@1 { + reg = <1>; + smsc,disable-energy-detect; + interrupt-parent = <&gpio4>; + interrupts = <13 IRQ_TYPE_EDGE_FALLING>; + }; + }; + };
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) --- .../bindings/net/nxp,imx287-mtip-switch.yaml | 165 ++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/nxp,imx287-mtip-switch.yaml