diff mbox series

[net-next,1/2] dt-bindings: net: dsa: b53: Add YAML bindings

Message ID 20201010164627.9309-2-kurt@kmk-computers.de
State Changes Requested
Headers show
Series dt-bindings: net: dsa: b53: Add YAML bindings | expand

Checks

Context Check Description
robh/dt-meta-schema success
robh/checkpatch success

Commit Message

Kurt Kanzenbach Oct. 10, 2020, 4:46 p.m. UTC
Convert the b53 DSA device tree bindings to YAML in order to allow
for automatic checking and such.

Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
---
 .../devicetree/bindings/net/dsa/b53.yaml      | 249 ++++++++++++++++++
 1 file changed, 249 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/b53.yaml

Comments

Florian Fainelli Oct. 11, 2020, 2:28 a.m. UTC | #1
On 10/10/2020 9:46 AM, Kurt Kanzenbach wrote:
> Convert the b53 DSA device tree bindings to YAML in order to allow
> for automatic checking and such.
> 
> Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>

Thanks for making this change, there are quite a few warnings that are 
going to show up because the binding was defined in a way that it would 
define chip compatible strings, which not all DTS files are using. I 
don't know if Rob would be comfortable with taking this until we resolve 
all warnings first.
Kurt Kanzenbach Oct. 11, 2020, 8:32 a.m. UTC | #2
On Sat Oct 10 2020, Florian Fainelli wrote:
> On 10/10/2020 9:46 AM, Kurt Kanzenbach wrote:
>> Convert the b53 DSA device tree bindings to YAML in order to allow
>> for automatic checking and such.
>> 
>> Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
>> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
>
> Thanks for making this change, there are quite a few warnings that are 
> going to show up because the binding was defined in a way that it would 
> define chip compatible strings, which not all DTS files are using.

Oh, I didn't know there is a second make command for doing the actual
check against the dtbs. I've just used `make dt_binding_check'.

So, it seems like a lot of the errors are caused by the include files
such as

[linux]/arch/arm/boot/dts/bcm5301x.dtsi

	srab: srab@18007000 {
		compatible = "brcm,bcm5301x-srab";
		reg = <0x18007000 0x1000>;

		status = "disabled";

		/* ports are defined in board DTS */
	};

The nodename should be "switch" not "srab" as enforced by
dsa.yaml. Furthermore, some DTS files are not adding the chip specific
compatible strings and the ports leading to more errors.

There are also some minor errors regarding the reg-names and such for
specific instances.

How should we proceed? Adding the missing compatible strings and ports
to the DTS files? Or adjusting the include files?

> I don't know if Rob would be comfortable with taking this until we
> resolve all warnings first.

Probably not. We should fix the existing device trees first.

Thanks,
Kurt
Florian Fainelli Oct. 11, 2020, 3:46 p.m. UTC | #3
On 10/11/2020 1:32 AM, Kurt Kanzenbach wrote:
> On Sat Oct 10 2020, Florian Fainelli wrote:
>> On 10/10/2020 9:46 AM, Kurt Kanzenbach wrote:
>>> Convert the b53 DSA device tree bindings to YAML in order to allow
>>> for automatic checking and such.
>>>
>>> Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
>>> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
>>
>> Thanks for making this change, there are quite a few warnings that are
>> going to show up because the binding was defined in a way that it would
>> define chip compatible strings, which not all DTS files are using.
> 
> Oh, I didn't know there is a second make command for doing the actual
> check against the dtbs. I've just used `make dt_binding_check'.
> 
> So, it seems like a lot of the errors are caused by the include files
> such as
> 
> [linux]/arch/arm/boot/dts/bcm5301x.dtsi
> 
> 	srab: srab@18007000 {
> 		compatible = "brcm,bcm5301x-srab";
> 		reg = <0x18007000 0x1000>;
> 
> 		status = "disabled";
> 
> 		/* ports are defined in board DTS */
> 	};
> 
> The nodename should be "switch" not "srab" as enforced by
> dsa.yaml. Furthermore, some DTS files are not adding the chip specific
> compatible strings and the ports leading to more errors.
> 
> There are also some minor errors regarding the reg-names and such for
> specific instances.
> 
> How should we proceed? Adding the missing compatible strings and ports
> to the DTS files? Or adjusting the include files?

The include is correct as it provides the fallback family string which 
is what the driver will be looking for unless we do not provide a chip 
compatible. The various DTS should be updated to contain both the chip 
compatible and the fallback family (brcm,bcm5301x-srab) string, I will 
update the various DTS and submit these for review later next week.

Then we could imagine me taking this YAML change through the Broadcom 
ARM SoC pull requests that way no new regressions are introduced.

Sounds good?
Kurt Kanzenbach Oct. 12, 2020, 5:45 a.m. UTC | #4
On Sun Oct 11 2020, Florian Fainelli wrote:
> On 10/11/2020 1:32 AM, Kurt Kanzenbach wrote:
>> How should we proceed? Adding the missing compatible strings and ports
>> to the DTS files? Or adjusting the include files?
>
> The include is correct as it provides the fallback family string which 
> is what the driver will be looking for unless we do not provide a chip 
> compatible. The various DTS should be updated to contain both the chip 
> compatible and the fallback family (brcm,bcm5301x-srab) string, I will 
> update the various DTS and submit these for review later next week.

OK. It's not just the compatible strings, there are other issues as
well. You can check with `make dtbs_check DT_SCHEMA_FILES=path/to/b53.yaml'.

>
> Then we could imagine me taking this YAML change through the Broadcom 
> ARM SoC pull requests that way no new regressions are introduced.
>
> Sounds good?

Sounds like a plan. But, Rob or other device tree maintainers should
have a look at the YAML file to spot issues in there first.

Thanks,
Kurt
Rob Herring Oct. 12, 2020, 6:48 p.m. UTC | #5
On Mon, Oct 12, 2020 at 07:45:10AM +0200, Kurt Kanzenbach wrote:
> On Sun Oct 11 2020, Florian Fainelli wrote:
> > On 10/11/2020 1:32 AM, Kurt Kanzenbach wrote:
> >> How should we proceed? Adding the missing compatible strings and ports
> >> to the DTS files? Or adjusting the include files?
> >
> > The include is correct as it provides the fallback family string which 
> > is what the driver will be looking for unless we do not provide a chip 
> > compatible. The various DTS should be updated to contain both the chip 
> > compatible and the fallback family (brcm,bcm5301x-srab) string, I will 
> > update the various DTS and submit these for review later next week.
> 
> OK. It's not just the compatible strings, there are other issues as
> well. You can check with `make dtbs_check DT_SCHEMA_FILES=path/to/b53.yaml'.
> 
> >
> > Then we could imagine me taking this YAML change through the Broadcom 
> > ARM SoC pull requests that way no new regressions are introduced.
> >
> > Sounds good?
> 
> Sounds like a plan. But, Rob or other device tree maintainers should
> have a look at the YAML file to spot issues in there first.

Looks pretty good to me.

Rob
diff mbox series

Patch

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@gmail.com>
+
+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
+    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 {
+            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;
+                    };
+                };
+            };
+        };
+    };