| Message ID | 20201010164627.9309-2-kurt@kmk-computers.de |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dt-bindings: net: dsa: b53: Add YAML bindings | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success |
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.
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
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?
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
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 --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 = <ð0>; + 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; + }; + }; + }; + }; + };
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