Re: [PATCH 10/10] dt-bindings: net: dsa: b53: Add YAML bindings

From: Vladimir Oltean
Date: Tue Nov 10 2020 - 17:43:17 EST

On Mon, Nov 09, 2020 at 07:31:13PM -0800, Florian Fainelli wrote:
> diff --git a/Documentation/devicetree/bindings/net/dsa/b53.yaml b/Documentation/devicetree/bindings/net/dsa/b53.yaml
> new file mode 100644
> index 000000000000..4fcbac1de95b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/b53.yaml
> @@ -0,0 +1,249 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/b53.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM53xx Ethernet switches
> +
> +allOf:
> + - $ref: dsa.yaml#
> +
> +maintainers:
> + - Florian Fainelli <f.fainelli@xxxxxxxxx>
> +
> +description:
> + Broadcom BCM53xx Ethernet switches
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: brcm,bcm5325
> + - const: brcm,bcm53115
> + - const: brcm,bcm53125
> + - const: brcm,bcm53128
> + - const: brcm,bcm5365
> + - const: brcm,bcm5395
> + - const: brcm,bcm5389
> + - const: brcm,bcm5397
> + - const: brcm,bcm5398
> + - items:
> + - const: brcm,bcm11360-srab
> + - const: brcm,cygnus-srab
> + - items:
> + - enum:
> + - brcm,bcm53010-srab
> + - brcm,bcm53011-srab
> + - brcm,bcm53012-srab
> + - brcm,bcm53018-srab
> + - brcm,bcm53019-srab
> + - const: brcm,bcm5301x-srab
> + - items:
> + - enum:
> + - brcm,bcm11404-srab
> + - brcm,bcm11407-srab
> + - brcm,bcm11409-srab
> + - brcm,bcm58310-srab
> + - brcm,bcm58311-srab
> + - brcm,bcm58313-srab
> + - const: brcm,omega-srab
> + - items:
> + - enum:
> + - brcm,bcm58522-srab
> + - brcm,bcm58523-srab
> + - brcm,bcm58525-srab
> + - brcm,bcm58622-srab
> + - brcm,bcm58623-srab
> + - brcm,bcm58625-srab
> + - brcm,bcm88312-srab
> + - const: brcm,nsp-srab
> + - items:
> + - enum:
> + - brcm,bcm3384-switch
> + - brcm,bcm6328-switch
> + - brcm,bcm6368-switch
> + - const: brcm,bcm63xx-switch
> +
> +required:
> + - compatible
> + - reg
> +
> +# BCM585xx/586xx/88312 SoCs
> +if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - brcm,bcm58522-srab
> + - brcm,bcm58523-srab
> + - brcm,bcm58525-srab
> + - brcm,bcm58622-srab
> + - brcm,bcm58623-srab
> + - brcm,bcm58625-srab
> + - brcm,bcm88312-srab
> +then:
> + properties:
> + reg:
> + minItems: 3
> + maxItems: 3
> + reg-names:
> + items:
> + - const: srab
> + - const: mux_config
> + - const: sgmii_config

I am only reading these with a human eye, I don't parse YAML syntax.
Does the syntax enforce that these reg-names are declared in this
precise order, which is necessary for the proper operation of the
driver?

> + interrupts:
> + minItems: 13
> + maxItems: 13
> + interrupt-names:
> + items:
> + - const: link_state_p0
> + - const: link_state_p1
> + - const: link_state_p2
> + - const: link_state_p3
> + - const: link_state_p4
> + - const: link_state_p5
> + - const: link_state_p7
> + - const: link_state_p8
> + - const: phy
> + - const: ts
> + - const: imp_sleep_timer_p5
> + - const: imp_sleep_timer_p7
> + - const: imp_sleep_timer_p8
> + required:
> + - interrupts
> +else:
> + properties:
> + reg:
> + maxItems: 1
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + switch@1e {

You have renamed a node called 'ethernet-switch' into one called
'switch'. Was it deliberate?

> + compatible = "brcm,bcm53125";
> + reg = <30>;
> +
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + label = "lan1";
> + };
> +
> + port@1 {
> + reg = <1>;
> + label = "lan2";
> + };
> +
> + port@5 {
> + reg = <5>;
> + label = "cable-modem";
> + phy-mode = "rgmii-txid";
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + port@8 {
> + reg = <8>;
> + label = "cpu";
> + phy-mode = "rgmii-txid";
> + ethernet = <&eth0>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> + };
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + axi {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + switch@36000 {
> + compatible = "brcm,bcm58623-srab", "brcm,nsp-srab";
> + reg = <0x36000 0x1000>,
> + <0x3f308 0x8>,
> + <0x3f410 0xc>;
> + reg-names = "srab", "mux_config", "sgmii_config";
> + interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "link_state_p0",
> + "link_state_p1",
> + "link_state_p2",
> + "link_state_p3",
> + "link_state_p4",
> + "link_state_p5",
> + "link_state_p7",
> + "link_state_p8",
> + "phy",
> + "ts",
> + "imp_sleep_timer_p5",
> + "imp_sleep_timer_p7",
> + "imp_sleep_timer_p8";
> +
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + label = "port0";
> + reg = <0>;
> + };
> +
> + port@1 {
> + label = "port1";
> + reg = <1>;
> + };
> +
> + port@2 {
> + label = "port2";
> + reg = <2>;
> + };
> +
> + port@3 {
> + label = "port3";
> + reg = <3>;
> + };
> +
> + port@4 {
> + label = "port4";
> + reg = <4>;
> + };
> +
> + port@8 {
> + ethernet = <&amac2>;
> + label = "cpu";
> + reg = <8>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> + };