On Mon, Sep 19, 2022 at 1:17 PM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > On 17/09/2022 06:11, Sergio Paracuellos wrote: > > Add the yaml binding for available CPUs in MIPS architecture. > > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx> > > Rebase on some recent Linux kernel tree and use scripts/get_maintainers.pl Understood. > > > > --- > > Changes in v2: > > - Remove 'bindings/mips/brcm/brcm,bmips.txt' > > - Include 'mips-hpt-frequency' in cpus YAML schema for bmips CPUS's > > - Add a BMIPS CPU node sample > > > > .../bindings/mips/brcm/brcm,bmips.txt | 8 -- > > .../devicetree/bindings/mips/cpus.yaml | 100 ++++++++++++++++++ > > 2 files changed, 100 insertions(+), 8 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/mips/brcm/brcm,bmips.txt > > create mode 100644 Documentation/devicetree/bindings/mips/cpus.yaml > > > > diff --git a/Documentation/devicetree/bindings/mips/brcm/brcm,bmips.txt b/Documentation/devicetree/bindings/mips/brcm/brcm,bmips.txt > > deleted file mode 100644 > > index 8ef71b4085ca..000000000000 > > --- a/Documentation/devicetree/bindings/mips/brcm/brcm,bmips.txt > > +++ /dev/null > > @@ -1,8 +0,0 @@ > > -* Broadcom MIPS (BMIPS) CPUs > > - > > -Required properties: > > -- compatible: "brcm,bmips3300", "brcm,bmips4350", "brcm,bmips4380", > > - "brcm,bmips5000" > > - > > -- mips-hpt-frequency: This is common to all CPUs in the system so it lives > > - under the "cpus" node. > > diff --git a/Documentation/devicetree/bindings/mips/cpus.yaml b/Documentation/devicetree/bindings/mips/cpus.yaml > > new file mode 100644 > > index 000000000000..361afde8ce0a > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mips/cpus.yaml > > @@ -0,0 +1,100 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mips/cpus.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: MIPS CPUs bindings > > + > > +maintainers: > > + - Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx> > > What about existing maintainers? Will add Thomas as maintainer, thanks. > > > > > + > > +description: |+ > > |+ seems not needed Will drop. > > > > + The device tree allows to describe the layout of CPUs in a system through > > + the "cpus" node, which in turn contains a number of subnodes (ie "cpu") > > + defining properties for every cpu. > > s/cpu/CPU/ Will change. > > > + > > +properties: > > + reg: > > + maxItems: 1 > > + > > + compatible: > > compatible goes first. Understood. > > > + enum: > > + - brcm,bmips3300 > > + - brcm,bmips4350 > > + - brcm,bmips4380 > > + - brcm,bmips5000 > > + - brcm,bmips5200 > > + - ingenic,xburst-mxu1.0 > > + - ingenic,xburst-fpu1.0-mxu1.1 > > + - ingenic,xburst-fpu2.0-mxu2.0 > > + - loongson,gs264 > > + - mips,mips1004Kc > > + - mips,m14Kc > > Maybe keep alphabetical order? Ok. > > > + - mips,mips24KEc > > + - mips,mips4KEc > > + - mips,mips74Kc > > + - mips,mips24Kc > > + - mti,mips24KEc > > + - mti,mips14KEc > > + - mti,mips14Kc > > + - mti,interaptiv > > + > > +if: > > Out it in allOf block Understood. > > > + properties: > > + compatible: > > + enum: > > + - brcm,bmips3300 > > + - brcm,bmips4350 > > + - brcm,bmips4380 > > + - brcm,bmips5000 > > + - brcm,bmips5200 > > +then: > > + patternProperties: > > + mips-hpt-frequency: > > It's not a pattern. Did you test the bindings? > > > + $ref: /schemas/types.yaml#/definitions/uint32 > > Missing description. Will add it. > > else mips-hpt-frequency: false > > > + > > +required: > > + - compatible > > + > > +additionalProperties: true > > and this is why you did not notice errors... Current arch/mips/boot/dts folder dts files are a mess for cpu nodes, so I set additionalProperties to true and only make required for 'compatible'. What should be the correct approach? > > > + > > +examples: > > + - | > > + cpus { > > + #size-cells = <0>; > > + #address-cells = <1>; > > + > > + cpu@0 { > > + device_type = "cpu"; > > + compatible = "mips,mips1004Kc"; > > + reg = <0>; > > + }; > > + > > + cpu@1 { > > + device_type = "cpu"; > > + compatible = "mips,mips1004Kc"; > > + reg = <1>; > > + }; > > + }; > > + > > + - | > > + // Example 2 (BMIPS CPU) > > + cpus { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + mips-hpt-frequency = <150000000>; > > Does not match your bindings. Are you sure you tested the patches? Yes I did: $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/mips/cpus.yaml LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/mips/cpus.example.dts DTC Documentation/devicetree/bindings/mips/cpus.example.dtb ' CHECK Documentation/devicetree/bindings/mips/cpus.example.dtb Can you please point me to a sample of how to make required in a parent node of cpu@X property 'mips-hpt-frequency' only for some compatible strings inside the node? What can this be properly expressed using schema?? I was looking and testing different things for a while without success at all. Thanks in advance for your time. Best regards, Sergio Paracuellos > > > + > > + cpu@0 { > > + compatible = "brcm,bmips4350"; > > + device_type = "cpu"; > > + reg = <0>; > > + }; > > + > > + cpu@1 { > > + compatible = "brcm,bmips4350"; > > + device_type = "cpu"; > > + reg = <1>; > > + }; > > + }; > > > Best regards, > Krzysztof
