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

[Rob Herring]

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@xxxxxxxxxx>
> ---
>  .../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@xxxxxxx>
> +  - Arınç ÜNAL <arinc.unal@xxxxxxxxxx>
> +
> +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