diff mbox series

[net-next,v2,08/10] dt-bindings: net: dsa: marvell: convert to json-schema

Message ID 20230916110902.234273-9-arinc.unal@arinc9.com (mailing list archive)
State New, archived
Headers show
Series define and enforce phylink bindings | expand

Commit Message

Arınç ÜNAL Sept. 16, 2023, 11:09 a.m. UTC
Convert the document for Marvell ethernet switches to json-schema.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../devicetree/bindings/net/dsa/marvell.txt   | 109 ----------
 .../devicetree/bindings/net/dsa/marvell.yaml  | 204 ++++++++++++++++++
 2 files changed, 204 insertions(+), 109 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.txt
 create mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.yaml

Comments

Andrew Lunn Sept. 16, 2023, 2:05 p.m. UTC | #1
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: |
> +          Switch has base address 0x10. Use with models:
> +          6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165, 6171, 6172, 6175,
> +          6176, 6185, 6240, 6320, 6321, 6341, 6350, 6351, 6352
> +        const: marvell,mv88e6085

The .txt file contained:

    The compatibility string is used only to find an identification
    register, which is at a different MDIO base address in different
    switch families.

Please keep this text. There has been a lot of confusion in the past,
lots of circular arguments etc, and it is way too late to do anything
different. So i want to make it clear what the compatible is used for,
juts to find the ID registers, nothing more.

> +
> +      - description: |
> +          Switch has base address 0x00. Use with models:
> +          6190, 6190X, 6191, 6290, 6361, 6390, 6390X

The .txt file is missing the 6191X and 6393X, which belong here. If
you need to respin, please could you add them here.

	Andrew
Arınç ÜNAL Sept. 16, 2023, 3:44 p.m. UTC | #2
On 16.09.2023 17:05, Andrew Lunn wrote:
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - description: |
>> +          Switch has base address 0x10. Use with models:
>> +          6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165, 6171, 6172, 6175,
>> +          6176, 6185, 6240, 6320, 6321, 6341, 6350, 6351, 6352
>> +        const: marvell,mv88e6085
> 
> The .txt file contained:
> 
>      The compatibility string is used only to find an identification
>      register, which is at a different MDIO base address in different
>      switch families.
> 
> Please keep this text. There has been a lot of confusion in the past,
> lots of circular arguments etc, and it is way too late to do anything
> different. So i want to make it clear what the compatible is used for,
> juts to find the ID registers, nothing more.

Will do.

> 
>> +
>> +      - description: |
>> +          Switch has base address 0x00. Use with models:
>> +          6190, 6190X, 6191, 6290, 6361, 6390, 6390X
> 
> The .txt file is missing the 6191X and 6393X, which belong here. If
> you need to respin, please could you add them here.

Will do.

Arınç
Rob Herring Sept. 18, 2023, 6:40 p.m. UTC | #3
On Sat, Sep 16, 2023 at 02:09:00PM +0300, Arınç ÜNAL wrote:
> Convert the document for Marvell ethernet switches to json-schema.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../devicetree/bindings/net/dsa/marvell.txt   | 109 ----------
>  .../devicetree/bindings/net/dsa/marvell.yaml  | 204 ++++++++++++++++++
>  2 files changed, 204 insertions(+), 109 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.txt
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> deleted file mode 100644
> index 6ec0c181b6db..000000000000
> --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
> +++ /dev/null
> @@ -1,109 +0,0 @@
> -Marvell DSA Switch Device Tree Bindings
> ----------------------------------------
> -
> -WARNING: This binding is currently unstable. Do not program it into a
> -FLASH never to be changed again. Once this binding is stable, this
> -warning will be removed.
> -
> -If you need a stable binding, use the old dsa.txt binding.
> -
> -Marvell Switches are MDIO devices. The following properties should be
> -placed as a child node of an mdio device.
> -
> -The properties described here are those specific to Marvell devices.
> -Additional required and optional properties can be found in dsa.txt.
> -
> -The compatibility string is used only to find an identification register,
> -which is at a different MDIO base address in different switch families.
> -- "marvell,mv88e6085"	: Switch has base address 0x10. Use with models:
> -			  6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165,
> -			  6171, 6172, 6175, 6176, 6185, 6240, 6320, 6321,
> -			  6341, 6350, 6351, 6352
> -- "marvell,mv88e6190"	: Switch has base address 0x00. Use with models:
> -			  6190, 6190X, 6191, 6290, 6361, 6390, 6390X
> -- "marvell,mv88e6250"	: Switch has base address 0x08 or 0x18. Use with model:
> -			  6220, 6250
> -
> -Required properties:
> -- compatible		: Should be one of "marvell,mv88e6085",
> -			  "marvell,mv88e6190" or "marvell,mv88e6250" as
> -			  indicated above
> -- reg			: Address on the MII bus for the switch.
> -
> -Optional properties:
> -
> -- reset-gpios		: Should be a gpio specifier for a reset line
> -- interrupts		: Interrupt from the switch
> -- interrupt-controller	: Indicates the switch is itself an interrupt
> -			  controller. This is used for the PHY interrupts.
> -#interrupt-cells = <2>	: Controller uses two cells, number and flag
> -- eeprom-length		: Set to the length of an EEPROM connected to the
> -			  switch. Must be set if the switch can not detect
> -			  the presence and/or size of a connected EEPROM,
> -			  otherwise optional.
> -- mdio			: Container of PHY and devices on the switches MDIO
> -			  bus.
> -- mdio?		: Container of PHYs and devices on the external MDIO
> -			  bus. The node must contains a compatible string of
> -			  "marvell,mv88e6xxx-mdio-external"
> -
> -Example:
> -
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		interrupt-parent = <&gpio0>;
> -		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> -		interrupt-controller;
> -		#interrupt-cells = <2>;
> -
> -		switch0: switch@0 {
> -			compatible = "marvell,mv88e6085";
> -			reg = <0>;
> -			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
> -
> -			mdio {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				switch1phy0: switch1phy0@0 {
> -					reg = <0>;
> -					interrupt-parent = <&switch0>;
> -					interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> -				};
> -			};
> -		};
> -	};
> -
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		interrupt-parent = <&gpio0>;
> -		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> -		interrupt-controller;
> -		#interrupt-cells = <2>;
> -
> -		switch0: switch@0 {
> -			compatible = "marvell,mv88e6190";
> -			reg = <0>;
> -			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
> -
> -			mdio {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				switch1phy0: switch1phy0@0 {
> -					reg = <0>;
> -					interrupt-parent = <&switch0>;
> -					interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> -				};
> -			};
> -
> -			mdio1 {
> -				compatible = "marvell,mv88e6xxx-mdio-external";
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				switch1phy9: switch1phy0@9 {
> -					reg = <9>;
> -				};
> -			};
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.yaml b/Documentation/devicetree/bindings/net/dsa/marvell.yaml
> new file mode 100644
> index 000000000000..45756b8d74d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/marvell.yaml
> @@ -0,0 +1,204 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/marvell.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Ethernet Switches
> +
> +$ref: dsa.yaml#/$defs/ethernet-ports
> +
> +maintainers:
> +  - Andrew Lunn <andrew@lunn.ch>
> +  - Arınç ÜNAL <arinc.unal@arinc9.com>
> +
> +description:
> +  Marvell ethernet switches are MDIO devices.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: |
> +          Switch has base address 0x10. Use with models:
> +          6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165, 6171, 6172, 6175,
> +          6176, 6185, 6240, 6320, 6321, 6341, 6350, 6351, 6352
> +        const: marvell,mv88e6085
> +
> +      - description: |
> +          Switch has base address 0x00. Use with models:
> +          6190, 6190X, 6191, 6290, 6361, 6390, 6390X
> +        const: marvell,mv88e6190
> +
> +      - description: |
> +          Switch has base address 0x08 or 0x18. Use with models:
> +          6220, 6250
> +        const: marvell,mv88e6250
> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: GPIO to be used to reset the whole device
> +    maxItems: 1
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  eeprom-length:
> +    description:
> +      Set to the length of an EEPROM connected to the switch. Must be set if the
> +      switch can not detect the presence and/or size of a connected EEPROM,
> +      otherwise optional.
> +
> +  mdio:
> +    description:
> +      The optional node for the MDIO bus of the switch. The bus will be
> +      registered non-OF-based if this is not defined.

That's a detail of the OS behavior.

> +    $ref: /schemas/net/mdio.yaml#

       unevaluatedProperties: false

> +
> +  mdio-external:
> +    description: The externally reachable MDIO bus of the 6390 family switches
> +    $ref: /schemas/net/mdio.yaml#

       unevaluatedProperties: false

> +
> +    properties:
> +      compatible:
> +        const: marvell,mv88e6xxx-mdio-external
> +
> +    required:
> +      - compatible
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - marvell,mv88e6085
> +            - marvell,mv88e6250
> +    then:
> +      properties:
> +        mdio-external: false
> +
> +  - if:
> +      required: [ mdio ]
> +    then:
> +      patternProperties:
> +        "^(ethernet-)?ports$":
> +          patternProperties:
> +            "^(ethernet-)?port@[0-9]+$":
> +              $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink

This probably doesn't work right. The problem is every node ultimately 
needs a single schema with all possible properties in order to not allow 
undefined properties. This is the reason for the complexity with a $ref 
at each level in schemas for these nodes.

I'm not sure it is worth the complexity to enforce what you want here. 
It may be easier to just always allow phylink properties in 
ethernet-port nodes.  

As this is a switch, it should be referencing ethernet-switch.yaml. 

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
deleted file mode 100644
index 6ec0c181b6db..000000000000
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ /dev/null
@@ -1,109 +0,0 @@ 
-Marvell DSA Switch Device Tree Bindings
----------------------------------------
-
-WARNING: This binding is currently unstable. Do not program it into a
-FLASH never to be changed again. Once this binding is stable, this
-warning will be removed.
-
-If you need a stable binding, use the old dsa.txt binding.
-
-Marvell Switches are MDIO devices. The following properties should be
-placed as a child node of an mdio device.
-
-The properties described here are those specific to Marvell devices.
-Additional required and optional properties can be found in dsa.txt.
-
-The compatibility string is used only to find an identification register,
-which is at a different MDIO base address in different switch families.
-- "marvell,mv88e6085"	: Switch has base address 0x10. Use with models:
-			  6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165,
-			  6171, 6172, 6175, 6176, 6185, 6240, 6320, 6321,
-			  6341, 6350, 6351, 6352
-- "marvell,mv88e6190"	: Switch has base address 0x00. Use with models:
-			  6190, 6190X, 6191, 6290, 6361, 6390, 6390X
-- "marvell,mv88e6250"	: Switch has base address 0x08 or 0x18. Use with model:
-			  6220, 6250
-
-Required properties:
-- compatible		: Should be one of "marvell,mv88e6085",
-			  "marvell,mv88e6190" or "marvell,mv88e6250" as
-			  indicated above
-- reg			: Address on the MII bus for the switch.
-
-Optional properties:
-
-- reset-gpios		: Should be a gpio specifier for a reset line
-- interrupts		: Interrupt from the switch
-- interrupt-controller	: Indicates the switch is itself an interrupt
-			  controller. This is used for the PHY interrupts.
-#interrupt-cells = <2>	: Controller uses two cells, number and flag
-- eeprom-length		: Set to the length of an EEPROM connected to the
-			  switch. Must be set if the switch can not detect
-			  the presence and/or size of a connected EEPROM,
-			  otherwise optional.
-- mdio			: Container of PHY and devices on the switches MDIO
-			  bus.
-- mdio?		: Container of PHYs and devices on the external MDIO
-			  bus. The node must contains a compatible string of
-			  "marvell,mv88e6xxx-mdio-external"
-
-Example:
-
-	mdio {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		interrupt-parent = <&gpio0>;
-		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
-		interrupt-controller;
-		#interrupt-cells = <2>;
-
-		switch0: switch@0 {
-			compatible = "marvell,mv88e6085";
-			reg = <0>;
-			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
-
-			mdio {
-				#address-cells = <1>;
-				#size-cells = <0>;
-				switch1phy0: switch1phy0@0 {
-					reg = <0>;
-					interrupt-parent = <&switch0>;
-					interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
-				};
-			};
-		};
-	};
-
-	mdio {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		interrupt-parent = <&gpio0>;
-		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
-		interrupt-controller;
-		#interrupt-cells = <2>;
-
-		switch0: switch@0 {
-			compatible = "marvell,mv88e6190";
-			reg = <0>;
-			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
-
-			mdio {
-				#address-cells = <1>;
-				#size-cells = <0>;
-				switch1phy0: switch1phy0@0 {
-					reg = <0>;
-					interrupt-parent = <&switch0>;
-					interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
-				};
-			};
-
-			mdio1 {
-				compatible = "marvell,mv88e6xxx-mdio-external";
-				#address-cells = <1>;
-				#size-cells = <0>;
-				switch1phy9: switch1phy0@9 {
-					reg = <9>;
-				};
-			};
-		};
-	};
diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.yaml b/Documentation/devicetree/bindings/net/dsa/marvell.yaml
new file mode 100644
index 000000000000..45756b8d74d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.yaml
@@ -0,0 +1,204 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/marvell.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Ethernet Switches
+
+$ref: dsa.yaml#/$defs/ethernet-ports
+
+maintainers:
+  - Andrew Lunn <andrew@lunn.ch>
+  - Arınç ÜNAL <arinc.unal@arinc9.com>
+
+description:
+  Marvell ethernet switches are MDIO devices.
+
+properties:
+  compatible:
+    oneOf:
+      - description: |
+          Switch has base address 0x10. Use with models:
+          6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165, 6171, 6172, 6175,
+          6176, 6185, 6240, 6320, 6321, 6341, 6350, 6351, 6352
+        const: marvell,mv88e6085
+
+      - description: |
+          Switch has base address 0x00. Use with models:
+          6190, 6190X, 6191, 6290, 6361, 6390, 6390X
+        const: marvell,mv88e6190
+
+      - description: |
+          Switch has base address 0x08 or 0x18. Use with models:
+          6220, 6250
+        const: marvell,mv88e6250
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    description: GPIO to be used to reset the whole device
+    maxItems: 1
+
+  "#interrupt-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  interrupts:
+    maxItems: 1
+
+  eeprom-length:
+    description:
+      Set to the length of an EEPROM connected to the switch. Must be set if the
+      switch can not detect the presence and/or size of a connected EEPROM,
+      otherwise optional.
+
+  mdio:
+    description:
+      The optional node for the MDIO bus of the switch. The bus will be
+      registered non-OF-based if this is not defined.
+    $ref: /schemas/net/mdio.yaml#
+
+  mdio-external:
+    description: The externally reachable MDIO bus of the 6390 family switches
+    $ref: /schemas/net/mdio.yaml#
+
+    properties:
+      compatible:
+        const: marvell,mv88e6xxx-mdio-external
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - marvell,mv88e6085
+            - marvell,mv88e6250
+    then:
+      properties:
+        mdio-external: false
+
+  - if:
+      required: [ mdio ]
+    then:
+      patternProperties:
+        "^(ethernet-)?ports$":
+          patternProperties:
+            "^(ethernet-)?port@[0-9]+$":
+              $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        switch@0 {
+            compatible = "marvell,mv88e6085";
+            reg = <0>;
+            reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+
+            ethernet-ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    phy-mode = "rmii";
+                    ethernet = <&fec1>;
+
+                    fixed-link {
+                        speed = <100>;
+                        full-duplex;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    label = "swp1";
+                };
+            };
+        };
+    };
+
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        switch: switch@0 {
+            compatible = "marvell,mv88e6190";
+            reg = <0>;
+            reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+            interrupt-parent = <&gpio3>;
+            interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+
+            ethernet-ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    phy-mode = "rmii";
+                    ethernet = <&fec1>;
+
+                    fixed-link {
+                        speed = <100>;
+                        full-duplex;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    label = "swp1";
+                    phy-handle = <&ethphy1>;
+                };
+
+                port@2 {
+                    reg = <2>;
+                    label = "swp2";
+                    phy-handle = <&ext_ethphy0>;
+                };
+            };
+
+            mdio {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                ethphy1: ethernet-phy@1 {
+                    reg = <1>;
+                    interrupt-parent = <&switch>;
+                    interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
+                };
+            };
+
+            mdio-external {
+                compatible = "marvell,mv88e6xxx-mdio-external";
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                ext_ethphy0: ethernet-phy@0 {
+                    reg = <0>;
+                    compatible = "ethernet-phy-ieee802.3-c45";
+                };
+            };
+        };
+    };