All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
@ 2022-09-17  4:11 Sergio Paracuellos
  2022-09-18 11:22 ` Thomas Bogendoerfer
  2022-09-19 11:17 ` Krzysztof Kozlowski
  0 siblings, 2 replies; 14+ messages in thread
From: Sergio Paracuellos @ 2022-09-17  4:11 UTC (permalink / raw)
  To: devicetree; +Cc: tsbogend, robh+dt, krzk+dt, arinc.unal

Add the yaml binding for available CPUs in MIPS architecture.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
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@gmail.com>
+
+description: |+
+  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.
+
+properties:
+  reg:
+    maxItems: 1
+
+  compatible:
+    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
+      - mips,mips24KEc
+      - mips,mips4KEc
+      - mips,mips74Kc
+      - mips,mips24Kc
+      - mti,mips24KEc
+      - mti,mips14KEc
+      - mti,mips14Kc
+      - mti,interaptiv
+
+if:
+  properties:
+    compatible:
+      enum:
+        - brcm,bmips3300
+        - brcm,bmips4350
+        - brcm,bmips4380
+        - brcm,bmips5000
+        - brcm,bmips5200
+then:
+  patternProperties:
+    mips-hpt-frequency:
+      $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+
+additionalProperties: true
+
+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>;
+
+      cpu@0 {
+        compatible = "brcm,bmips4350";
+        device_type = "cpu";
+        reg = <0>;
+      };
+
+      cpu@1 {
+        compatible = "brcm,bmips4350";
+        device_type = "cpu";
+        reg = <1>;
+      };
+    };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-17  4:11 [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture Sergio Paracuellos
@ 2022-09-18 11:22 ` Thomas Bogendoerfer
  2022-09-18 15:15   ` Sergio Paracuellos
  2022-09-19 11:17 ` Krzysztof Kozlowski
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Bogendoerfer @ 2022-09-18 11:22 UTC (permalink / raw)
  To: Sergio Paracuellos; +Cc: devicetree, robh+dt, krzk+dt, arinc.unal

On Sat, Sep 17, 2022 at 06:11:36AM +0200, Sergio Paracuellos wrote:
> +  compatible:
> +    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
> +      - mips,mips24KEc
> +      - mips,mips4KEc

could you add mips,mips4Kc ? I have a board, which I'm switching to
DT, which uses that core.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-18 11:22 ` Thomas Bogendoerfer
@ 2022-09-18 15:15   ` Sergio Paracuellos
  0 siblings, 0 replies; 14+ messages in thread
From: Sergio Paracuellos @ 2022-09-18 15:15 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring, Krzysztof Kozlowski, Arınç ÜNAL

Hi Thomas,

On Sun, Sep 18, 2022 at 1:22 PM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
>
> On Sat, Sep 17, 2022 at 06:11:36AM +0200, Sergio Paracuellos wrote:
> > +  compatible:
> > +    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
> > +      - mips,mips24KEc
> > +      - mips,mips4KEc
>
> could you add mips,mips4Kc ? I have a board, which I'm switching to
> DT, which uses that core.

Sure, I will add it and send v3. I am going to wait to send v3 until
Rob's review just in case something is still wrong / missing here.

Thanks,
    Sergio Paracuellos
>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-17  4:11 [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture Sergio Paracuellos
  2022-09-18 11:22 ` Thomas Bogendoerfer
@ 2022-09-19 11:17 ` Krzysztof Kozlowski
  2022-09-19 12:29   ` Sergio Paracuellos
  1 sibling, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-19 11:17 UTC (permalink / raw)
  To: Sergio Paracuellos, devicetree; +Cc: tsbogend, robh+dt, krzk+dt, arinc.unal

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@gmail.com>

Rebase on some recent Linux kernel tree and use scripts/get_maintainers.pl


> ---
> 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@gmail.com>

What about existing maintainers?



> +
> +description: |+

|+ seems not needed


> +  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/

> +
> +properties:
> +  reg:
> +    maxItems: 1
> +
> +  compatible:

compatible goes first.

> +    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?

> +      - mips,mips24KEc
> +      - mips,mips4KEc
> +      - mips,mips74Kc
> +      - mips,mips24Kc
> +      - mti,mips24KEc
> +      - mti,mips14KEc
> +      - mti,mips14Kc
> +      - mti,interaptiv
> +
> +if:

Out it in allOf block

> +  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.

else mips-hpt-frequency: false

> +
> +required:
> +  - compatible
> +
> +additionalProperties: true

and this is why you did not notice errors...

> +
> +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?

> +
> +      cpu@0 {
> +        compatible = "brcm,bmips4350";
> +        device_type = "cpu";
> +        reg = <0>;
> +      };
> +
> +      cpu@1 {
> +        compatible = "brcm,bmips4350";
> +        device_type = "cpu";
> +        reg = <1>;
> +      };
> +    };


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-19 11:17 ` Krzysztof Kozlowski
@ 2022-09-19 12:29   ` Sergio Paracuellos
  2022-09-19 12:48     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Sergio Paracuellos @ 2022-09-19 12:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL

On Mon, Sep 19, 2022 at 1:17 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> 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@gmail.com>
>
> 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@gmail.com>
>
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-19 12:29   ` Sergio Paracuellos
@ 2022-09-19 12:48     ` Krzysztof Kozlowski
  2022-09-19 13:41       ` Sergio Paracuellos
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-19 12:48 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL

On 19/09/2022 14:29, Sergio Paracuellos wrote:
>>
>> 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?

This is okay, but it caused you did not notice errors...

> 
>>
>>> +
>>> +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.

You either define new schema for /cpus node (and match by name, define
children etc) or include it in schema for top-level properties. The
first is tricky, because the cpus node does not have compatible (like
nvidia,tegra194-ccplex.yaml).

The second should work, but then it's a bit cluttered (top-level mixed
with cpus).

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-19 12:48     ` Krzysztof Kozlowski
@ 2022-09-19 13:41       ` Sergio Paracuellos
  2022-09-19 16:08         ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Sergio Paracuellos @ 2022-09-19 13:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL

Hi Krzysztof,

On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 19/09/2022 14:29, Sergio Paracuellos wrote:
> >>
> >> 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?
>
> This is okay, but it caused you did not notice errors...
>
> >
> >>
> >>> +
> >>> +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.
>
> You either define new schema for /cpus node (and match by name, define
> children etc) or include it in schema for top-level properties. The
> first is tricky, because the cpus node does not have compatible (like
> nvidia,tegra194-ccplex.yaml).
>
> The second should work, but then it's a bit cluttered (top-level mixed
> with cpus).

I don't know if I am understanding you but maybe it is because my
explanation about the requirement was not good at all. So let me
explain a bit better.

This is the normal way of definition of cpus for BMIPS:

cpus {
      #address-cells = <1>;
      #size-cells = <0>;

      mips-hpt-frequency = <150000000>;

      cpu@0 {
        compatible = "brcm,bmips4350";
        device_type = "cpu";
        reg = <0>;
      };

      cpu@1 {
        compatible = "brcm,bmips4350";
        device_type = "cpu";
        reg = <1>;
      };
    };

What I need to say in schema is that 'mips-hpt-frequency' must be only
present if cpu@0 and cpu@1 nodes contain a compatible matching
brcm,bmips*. In the same cpu@0 or cpu@1 node
the following below will be sufficient. How can I express the same but
referring that 'mips-hpt-frequency' must be on the parent node?
Because as it is below the validator complains because
'mips-hpt-frequency'
is not present in cpu@0 and cpu@1 nodes:

allOf:
   - if:
        properties:
           compatible:
               enum:
                   - brcm,bmips3300
                   - brcm,bmips4350
                   - brcm,bmips4380
                   - brcm,bmips5000
                   - brcm,bmips5200
     then:
        required:
           - mips-hpt-frequency
     else:
        properties:
           mips-hpt-frequency: false

Thanks,
    Sergio Paracuellos
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-19 13:41       ` Sergio Paracuellos
@ 2022-09-19 16:08         ` Krzysztof Kozlowski
  2022-09-20  5:51           ` Sergio Paracuellos
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-19 16:08 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL

On 19/09/2022 15:41, Sergio Paracuellos wrote:
> Hi Krzysztof,
> 
> On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 19/09/2022 14:29, Sergio Paracuellos wrote:
>>>>
>>>> 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?
>>
>> This is okay, but it caused you did not notice errors...
>>
>>>
>>>>
>>>>> +
>>>>> +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.
>>
>> You either define new schema for /cpus node (and match by name, define
>> children etc) or include it in schema for top-level properties. The
>> first is tricky, because the cpus node does not have compatible (like
>> nvidia,tegra194-ccplex.yaml).
>>
>> The second should work, but then it's a bit cluttered (top-level mixed
>> with cpus).
> 
> I don't know if I am understanding you but maybe it is because my
> explanation about the requirement was not good at all. So let me
> explain a bit better.
> 
> This is the normal way of definition of cpus for BMIPS:

I know, I checked the DTS.

> 
> cpus {
>       #address-cells = <1>;
>       #size-cells = <0>;
> 
>       mips-hpt-frequency = <150000000>;
> 
>       cpu@0 {
>         compatible = "brcm,bmips4350";
>         device_type = "cpu";
>         reg = <0>;
>       };
> 
>       cpu@1 {
>         compatible = "brcm,bmips4350";
>         device_type = "cpu";
>         reg = <1>;
>       };
>     };
> 
> What I need to say in schema is that 'mips-hpt-frequency' must be only
> present if cpu@0 and cpu@1 nodes contain a compatible matching
> brcm,bmips*. In the same cpu@0 or cpu@1 node
> the following below will be sufficient. How can I express the same but
> referring that 'mips-hpt-frequency' must be on the parent node?

As I said you had two ways. In your current patch, I think you cannot.

> Because as it is below the validator complains because
> 'mips-hpt-frequency'
> is not present in cpu@0 and cpu@1 nodes:
> 
> allOf:
>    - if:
>         properties:
>            compatible:
>                enum:
>                    - brcm,bmips3300
>                    - brcm,bmips4350
>                    - brcm,bmips4380
>                    - brcm,bmips5000
>                    - brcm,bmips5200
>      then:
>         required:
>            - mips-hpt-frequency
>      else:
>         properties:
>            mips-hpt-frequency: false
> 

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-19 16:08         ` Krzysztof Kozlowski
@ 2022-09-20  5:51           ` Sergio Paracuellos
  2022-09-21  6:42             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Sergio Paracuellos @ 2022-09-20  5:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL

Hi Krzysztof,

On Mon, Sep 19, 2022 at 6:08 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 19/09/2022 15:41, Sergio Paracuellos wrote:
> > Hi Krzysztof,
> >
> > On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 19/09/2022 14:29, Sergio Paracuellos wrote:
> >>>>
> >>>> 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?
> >>
> >> This is okay, but it caused you did not notice errors...
> >>
> >>>
> >>>>
> >>>>> +
> >>>>> +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.
> >>
> >> You either define new schema for /cpus node (and match by name, define
> >> children etc) or include it in schema for top-level properties. The
> >> first is tricky, because the cpus node does not have compatible (like
> >> nvidia,tegra194-ccplex.yaml).

Ok so if I am understanding correctly having two schemas is a way to go:

One for brcm,bmips-cpus.yaml (since there is no compatible, should
this be a valid name for this?) containing something like:

properties:
  $nodename:
     const: cpus

     mips-hpt-frequency:
        $ref: /schemas/types.yaml#/definitions/uint32
        description: |
           This is common to all CPUs in the system so it lives
            under the "cpus" node.

additionalProperties: true

examples:
  - |
     cpus {
       #address-cells = <1>;
       #size-cells = <0>;

        mips-hpt-frequency = <150000000>;

        cpu@0 {
          compatible = "brcm,bmips4350";
          device_type = "cpu";
           reg = <0>;
        };

         cpu@1 {
           compatible = "brcm,bmips4350";
           device_type = "cpu";
           reg = <1>;
        };
  };

And the other as 'cpus.yaml' having:

properties:
 compatible:
    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,m14Kc
      - mips,mips4Kc
      - mips,mips4KEc
      - mips,mips24Kc
      - mips,mips24KEc
      - mips,mips74Kc
      - mips,mips1004Kc
      - mti,interaptiv
      - mti,mips24KEc
      - mti,mips14KEc
      - mti,mips14Kc

  reg:
    maxItems: 1

required:
  - compatible

additionalProperties: true

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>;
      };
    };

Should this be a valid approach?

Thanks,
    Sergio Paracuellos

> >>
> >> The second should work, but then it's a bit cluttered (top-level mixed
> >> with cpus).
> >
> > I don't know if I am understanding you but maybe it is because my
> > explanation about the requirement was not good at all. So let me
> > explain a bit better.
> >
> > This is the normal way of definition of cpus for BMIPS:
>
> I know, I checked the DTS.
>
> >
> > cpus {
> >       #address-cells = <1>;
> >       #size-cells = <0>;
> >
> >       mips-hpt-frequency = <150000000>;
> >
> >       cpu@0 {
> >         compatible = "brcm,bmips4350";
> >         device_type = "cpu";
> >         reg = <0>;
> >       };
> >
> >       cpu@1 {
> >         compatible = "brcm,bmips4350";
> >         device_type = "cpu";
> >         reg = <1>;
> >       };
> >     };
> >
> > What I need to say in schema is that 'mips-hpt-frequency' must be only
> > present if cpu@0 and cpu@1 nodes contain a compatible matching
> > brcm,bmips*. In the same cpu@0 or cpu@1 node
> > the following below will be sufficient. How can I express the same but
> > referring that 'mips-hpt-frequency' must be on the parent node?
>
> As I said you had two ways. In your current patch, I think you cannot.
>
> > Because as it is below the validator complains because
> > 'mips-hpt-frequency'
> > is not present in cpu@0 and cpu@1 nodes:
> >
> > allOf:
> >    - if:
> >         properties:
> >            compatible:
> >                enum:
> >                    - brcm,bmips3300
> >                    - brcm,bmips4350
> >                    - brcm,bmips4380
> >                    - brcm,bmips5000
> >                    - brcm,bmips5200
> >      then:
> >         required:
> >            - mips-hpt-frequency
> >      else:
> >         properties:
> >            mips-hpt-frequency: false
> >
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-20  5:51           ` Sergio Paracuellos
@ 2022-09-21  6:42             ` Krzysztof Kozlowski
  2022-09-21  7:18               ` Sergio Paracuellos
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-21  6:42 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL

On 20/09/2022 07:51, Sergio Paracuellos wrote:
> Hi Krzysztof,
> 
> On Mon, Sep 19, 2022 at 6:08 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 19/09/2022 15:41, Sergio Paracuellos wrote:
>>> Hi Krzysztof,
>>>
>>> On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 19/09/2022 14:29, Sergio Paracuellos wrote:
>>>>>>
>>>>>> 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?
>>>>
>>>> This is okay, but it caused you did not notice errors...
>>>>
>>>>>
>>>>>>
>>>>>>> +
>>>>>>> +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.
>>>>
>>>> You either define new schema for /cpus node (and match by name, define
>>>> children etc) or include it in schema for top-level properties. The
>>>> first is tricky, because the cpus node does not have compatible (like
>>>> nvidia,tegra194-ccplex.yaml).
> 
> Ok so if I am understanding correctly having two schemas is a way to go:
> 
> One for brcm,bmips-cpus.yaml (since there is no compatible, should
> this be a valid name for this?) containing something like:
> 
> properties:
>   $nodename:
>      const: cpus
> 
>      mips-hpt-frequency:
>         $ref: /schemas/types.yaml#/definitions/uint32
>         description: |
>            This is common to all CPUs in the system so it lives
>             under the "cpus" node.
> 
> additionalProperties: true

Almost. Such schema will allow mips-hpt-frequency in each cpus node,
everywhere. On every board and architecture.

You need to limit it per top-level compatibles.

You can also wait a week and maybe Rob will have some ideas.

> 
> examples:
>   - |
>      cpus {
>        #address-cells = <1>;
>        #size-cells = <0>;
> 
>         mips-hpt-frequency = <150000000>;
> 
>         cpu@0 {
>           compatible = "brcm,bmips4350";
>           device_type = "cpu";
>            reg = <0>;
>         };
> 
>          cpu@1 {
>            compatible = "brcm,bmips4350";
>            device_type = "cpu";
>            reg = <1>;
>         };
>   };
> 
> And the other as 'cpus.yaml' having:

Yes.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-21  6:42             ` Krzysztof Kozlowski
@ 2022-09-21  7:18               ` Sergio Paracuellos
  2022-09-21  7:51                 ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Sergio Paracuellos @ 2022-09-21  7:18 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL

Hi Krzysztof,

On Wed, Sep 21, 2022 at 8:42 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 20/09/2022 07:51, Sergio Paracuellos wrote:
> > Hi Krzysztof,
> >
> > On Mon, Sep 19, 2022 at 6:08 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 19/09/2022 15:41, Sergio Paracuellos wrote:
> >>> Hi Krzysztof,
> >>>
> >>> On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> On 19/09/2022 14:29, Sergio Paracuellos wrote:
> >>>>>>
> >>>>>> 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?
> >>>>
> >>>> This is okay, but it caused you did not notice errors...
> >>>>
> >>>>>
> >>>>>>
> >>>>>>> +
> >>>>>>> +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.
> >>>>
> >>>> You either define new schema for /cpus node (and match by name, define
> >>>> children etc) or include it in schema for top-level properties. The
> >>>> first is tricky, because the cpus node does not have compatible (like
> >>>> nvidia,tegra194-ccplex.yaml).
> >
> > Ok so if I am understanding correctly having two schemas is a way to go:
> >
> > One for brcm,bmips-cpus.yaml (since there is no compatible, should
> > this be a valid name for this?) containing something like:
> >
> > properties:
> >   $nodename:
> >      const: cpus
> >
> >      mips-hpt-frequency:
> >         $ref: /schemas/types.yaml#/definitions/uint32
> >         description: |
> >            This is common to all CPUs in the system so it lives
> >             under the "cpus" node.
> >
> > additionalProperties: true
>
> Almost. Such schema will allow mips-hpt-frequency in each cpus node,
> everywhere. On every board and architecture.

Yes, that is what I thought since no compatible to match this is
included in current node.

>
> You need to limit it per top-level compatibles.

Any sample of how to do this? So this bmips SoCs use compatible
strings that are described in:
https://elixir.bootlin.com/linux/v6.0-rc5/source/Documentation/devicetree/bindings/mips/brcm/soc.txt

Can the top level compatible string be used in some way to filter this
easily from this new 'brcm,bmips-cpus.yaml'

>
> You can also wait a week and maybe Rob will have some ideas.

Ideas are always welcome :). Ok, we can wait to Rob and see what
should be the correct approach to handle this.

Thanks,
    Sergio Paracuellos
>
> >
> > examples:
> >   - |
> >      cpus {
> >        #address-cells = <1>;
> >        #size-cells = <0>;
> >
> >         mips-hpt-frequency = <150000000>;
> >
> >         cpu@0 {
> >           compatible = "brcm,bmips4350";
> >           device_type = "cpu";
> >            reg = <0>;
> >         };
> >
> >          cpu@1 {
> >            compatible = "brcm,bmips4350";
> >            device_type = "cpu";
> >            reg = <1>;
> >         };
> >   };
> >
> > And the other as 'cpus.yaml' having:
>
> Yes.
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-21  7:18               ` Sergio Paracuellos
@ 2022-09-21  7:51                 ` Krzysztof Kozlowski
  2022-09-21  8:11                   ` Sergio Paracuellos
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-21  7:51 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL

On 21/09/2022 09:18, Sergio Paracuellos wrote:
> Hi Krzysztof,
> 
> On Wed, Sep 21, 2022 at 8:42 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 20/09/2022 07:51, Sergio Paracuellos wrote:
>>> Hi Krzysztof,
>>>
>>> On Mon, Sep 19, 2022 at 6:08 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 19/09/2022 15:41, Sergio Paracuellos wrote:
>>>>> Hi Krzysztof,
>>>>>
>>>>> On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
>>>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>>>
>>>>>> On 19/09/2022 14:29, Sergio Paracuellos wrote:
>>>>>>>>
>>>>>>>> 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?
>>>>>>
>>>>>> This is okay, but it caused you did not notice errors...
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>> +
>>>>>>>>> +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.
>>>>>>
>>>>>> You either define new schema for /cpus node (and match by name, define
>>>>>> children etc) or include it in schema for top-level properties. The
>>>>>> first is tricky, because the cpus node does not have compatible (like
>>>>>> nvidia,tegra194-ccplex.yaml).
>>>
>>> Ok so if I am understanding correctly having two schemas is a way to go:
>>>
>>> One for brcm,bmips-cpus.yaml (since there is no compatible, should
>>> this be a valid name for this?) containing something like:
>>>
>>> properties:
>>>   $nodename:
>>>      const: cpus
>>>
>>>      mips-hpt-frequency:
>>>         $ref: /schemas/types.yaml#/definitions/uint32
>>>         description: |
>>>            This is common to all CPUs in the system so it lives
>>>             under the "cpus" node.
>>>
>>> additionalProperties: true
>>
>> Almost. Such schema will allow mips-hpt-frequency in each cpus node,
>> everywhere. On every board and architecture.
> 
> Yes, that is what I thought since no compatible to match this is
> included in current node.
> 
>>
>> You need to limit it per top-level compatibles.
> 
> Any sample of how to do this? So this bmips SoCs use compatible
> strings that are described in:
> https://elixir.bootlin.com/linux/v6.0-rc5/source/Documentation/devicetree/bindings/mips/brcm/soc.txt

Could be something like this:
https://lore.kernel.org/all/20220830065744.161163-2-krzysztof.kozlowski@linaro.org/
which is a part of top-level schema or add a new one. The new one will
duplicate the compatibles and parts from that one there, so maybe better
to keep it in top-level?

I am not sure, any suggestions are welcome. Also platform/architecture
maintainers might have their preference to organize it.

Anyway, you did not Cc the actual platform maintainers (Rafał and Hauke).

> 
> Can the top level compatible string be used in some way to filter this
> easily from this new 'brcm,bmips-cpus.yaml'

Yes. If schema matches the top level compatible, then in allOf:if:then
you can add restriction to disallow it for other variants:

For example:
https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212

https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml#L152


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-21  7:51                 ` Krzysztof Kozlowski
@ 2022-09-21  8:11                   ` Sergio Paracuellos
  2022-09-21 11:40                     ` Sergio Paracuellos
  0 siblings, 1 reply; 14+ messages in thread
From: Sergio Paracuellos @ 2022-09-21  8:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL, Hauke Mehrtens, zajec5

Hi Krzysztof,

[cc: Hauke Mehrtens and Rafał Miłecki as maintainers for
'Documentation/devicetree/bindings/mips/brcm/']

On Wed, Sep 21, 2022 at 9:51 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/09/2022 09:18, Sergio Paracuellos wrote:
> > Hi Krzysztof,
> >
> > On Wed, Sep 21, 2022 at 8:42 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 20/09/2022 07:51, Sergio Paracuellos wrote:
> >>> Hi Krzysztof,
> >>>
> >>> On Mon, Sep 19, 2022 at 6:08 PM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> On 19/09/2022 15:41, Sergio Paracuellos wrote:
> >>>>> Hi Krzysztof,
> >>>>>
> >>>>> On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
> >>>>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>>>
> >>>>>> On 19/09/2022 14:29, Sergio Paracuellos wrote:
> >>>>>>>>
> >>>>>>>> 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?
> >>>>>>
> >>>>>> This is okay, but it caused you did not notice errors...
> >>>>>>
> >>>>>>>
> >>>>>>>>
> >>>>>>>>> +
> >>>>>>>>> +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.
> >>>>>>
> >>>>>> You either define new schema for /cpus node (and match by name, define
> >>>>>> children etc) or include it in schema for top-level properties. The
> >>>>>> first is tricky, because the cpus node does not have compatible (like
> >>>>>> nvidia,tegra194-ccplex.yaml).
> >>>
> >>> Ok so if I am understanding correctly having two schemas is a way to go:
> >>>
> >>> One for brcm,bmips-cpus.yaml (since there is no compatible, should
> >>> this be a valid name for this?) containing something like:
> >>>
> >>> properties:
> >>>   $nodename:
> >>>      const: cpus
> >>>
> >>>      mips-hpt-frequency:
> >>>         $ref: /schemas/types.yaml#/definitions/uint32
> >>>         description: |
> >>>            This is common to all CPUs in the system so it lives
> >>>             under the "cpus" node.
> >>>
> >>> additionalProperties: true
> >>
> >> Almost. Such schema will allow mips-hpt-frequency in each cpus node,
> >> everywhere. On every board and architecture.
> >
> > Yes, that is what I thought since no compatible to match this is
> > included in current node.
> >
> >>
> >> You need to limit it per top-level compatibles.
> >
> > Any sample of how to do this? So this bmips SoCs use compatible
> > strings that are described in:
> > https://elixir.bootlin.com/linux/v6.0-rc5/source/Documentation/devicetree/bindings/mips/brcm/soc.txt
>
> Could be something like this:
> https://lore.kernel.org/all/20220830065744.161163-2-krzysztof.kozlowski@linaro.org/
> which is a part of top-level schema or add a new one. The new one will
> duplicate the compatibles and parts from that one there, so maybe better
> to keep it in top-level?
>
> I am not sure, any suggestions are welcome. Also platform/architecture
> maintainers might have their preference to organize it.

I am also not sure.

>
> Anyway, you did not Cc the actual platform maintainers (Rafał and Hauke).

True, sorry for the inconvenience. Added now to CC list.

>
> >
> > Can the top level compatible string be used in some way to filter this
> > easily from this new 'brcm,bmips-cpus.yaml'
>
> Yes. If schema matches the top level compatible, then in allOf:if:then
> you can add restriction to disallow it for other variants:
>
> For example:
> https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212
>
> https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml#L152

Thanks for the clue.

I'll try to do some tests and come back here later :)

Thanks,
    Sergio Paracuellos
>
>
> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture
  2022-09-21  8:11                   ` Sergio Paracuellos
@ 2022-09-21 11:40                     ` Sergio Paracuellos
  0 siblings, 0 replies; 14+ messages in thread
From: Sergio Paracuellos @ 2022-09-21 11:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Thomas Bogendoerfer, Rob Herring, Krzysztof Kozlowski,
	Arınç ÜNAL, Hauke Mehrtens, zajec5

Hi Krzysztof,

On Wed, Sep 21, 2022 at 10:11 AM Sergio Paracuellos
<sergio.paracuellos@gmail.com> wrote:
>
> Hi Krzysztof,
>
> [cc: Hauke Mehrtens and Rafał Miłecki as maintainers for
> 'Documentation/devicetree/bindings/mips/brcm/']
>
> On Wed, Sep 21, 2022 at 9:51 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 21/09/2022 09:18, Sergio Paracuellos wrote:
> > > Hi Krzysztof,
> > >
> > > On Wed, Sep 21, 2022 at 8:42 AM Krzysztof Kozlowski
> > > <krzysztof.kozlowski@linaro.org> wrote:
> > >>
> > >> On 20/09/2022 07:51, Sergio Paracuellos wrote:
> > >>> Hi Krzysztof,
> > >>>
> > >>> On Mon, Sep 19, 2022 at 6:08 PM Krzysztof Kozlowski
> > >>> <krzysztof.kozlowski@linaro.org> wrote:
> > >>>>
> > >>>> On 19/09/2022 15:41, Sergio Paracuellos wrote:
> > >>>>> Hi Krzysztof,
> > >>>>>
> > >>>>> On Mon, Sep 19, 2022 at 2:48 PM Krzysztof Kozlowski
> > >>>>> <krzysztof.kozlowski@linaro.org> wrote:
> > >>>>>>
> > >>>>>> On 19/09/2022 14:29, Sergio Paracuellos wrote:
> > >>>>>>>>
> > >>>>>>>> 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?
> > >>>>>>
> > >>>>>> This is okay, but it caused you did not notice errors...
> > >>>>>>
> > >>>>>>>
> > >>>>>>>>
> > >>>>>>>>> +
> > >>>>>>>>> +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.
> > >>>>>>
> > >>>>>> You either define new schema for /cpus node (and match by name, define
> > >>>>>> children etc) or include it in schema for top-level properties. The
> > >>>>>> first is tricky, because the cpus node does not have compatible (like
> > >>>>>> nvidia,tegra194-ccplex.yaml).
> > >>>
> > >>> Ok so if I am understanding correctly having two schemas is a way to go:
> > >>>
> > >>> One for brcm,bmips-cpus.yaml (since there is no compatible, should
> > >>> this be a valid name for this?) containing something like:
> > >>>
> > >>> properties:
> > >>>   $nodename:
> > >>>      const: cpus
> > >>>
> > >>>      mips-hpt-frequency:
> > >>>         $ref: /schemas/types.yaml#/definitions/uint32
> > >>>         description: |
> > >>>            This is common to all CPUs in the system so it lives
> > >>>             under the "cpus" node.
> > >>>
> > >>> additionalProperties: true
> > >>
> > >> Almost. Such schema will allow mips-hpt-frequency in each cpus node,
> > >> everywhere. On every board and architecture.
> > >
> > > Yes, that is what I thought since no compatible to match this is
> > > included in current node.
> > >
> > >>
> > >> You need to limit it per top-level compatibles.
> > >
> > > Any sample of how to do this? So this bmips SoCs use compatible
> > > strings that are described in:
> > > https://elixir.bootlin.com/linux/v6.0-rc5/source/Documentation/devicetree/bindings/mips/brcm/soc.txt
> >
> > Could be something like this:
> > https://lore.kernel.org/all/20220830065744.161163-2-krzysztof.kozlowski@linaro.org/
> > which is a part of top-level schema or add a new one. The new one will
> > duplicate the compatibles and parts from that one there, so maybe better
> > to keep it in top-level?
> >
> > I am not sure, any suggestions are welcome. Also platform/architecture
> > maintainers might have their preference to organize it.
>
> I am also not sure.
>
> >
> > Anyway, you did not Cc the actual platform maintainers (Rafał and Hauke).
>
> True, sorry for the inconvenience. Added now to CC list.
>
> >
> > >
> > > Can the top level compatible string be used in some way to filter this
> > > easily from this new 'brcm,bmips-cpus.yaml'
> >
> > Yes. If schema matches the top level compatible, then in allOf:if:then
> > you can add restriction to disallow it for other variants:
> >
> > For example:
> > https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212
> >
> > https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml#L152
>
> Thanks for the clue.
>
> I'll try to do some tests and come back here later :)

How about this?

# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
%YAML 1.2
---
$id: http://devicetree.org/schemas/mips/brcm/brcm,bmips-cpus.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

title: BMIPS CPUs bindings

maintainers:
  - Hauke Mehrtens <hauke@hauke-m.de>
  - Rafał Miłecki <zajec5@gmail.com>

description: |
  The device tree allows to describe the layout of BMIPS CPUs.

properties:
  $nodename:
    const: "/"

  compatible:
    enum:
      - "brcm,bcm3368"
      - "brcm,bcm3384"
      - "brcm,bcm33843"
      - "brcm,bcm3384-viper"
      - "brcm,bcm33843-viper"
      - "brcm,bcm6328"
      - "brcm,bcm6358"
      - "brcm,bcm6362"
      - "brcm,bcm6368"
      - "brcm,bcm63168"
      - "brcm,bcm63268"
      - "brcm,bcm7125"
      - "brcm,bcm7346"
      - "brcm,bcm7358"
      - "brcm,bcm7360"
      - "brcm,bcm7362"
      - "brcm,bcm7420"
      - "brcm,bcm7425"

  cpus:
    type: object
    additionalProperties: true
    properties:
      '#address-cells':
        const: 1

      '#size-cells':
        const: 0

      mips-hpt-frequency:
        description: This is common to all CPUs in the system so it lives
          under the "cpus" node.
        $ref: /schemas/types.yaml#/definitions/uint32

    required:
      - '#address-cells'
      - '#size-cells'

    allOf:
      - if:
          properties:
            compatible:
              contains:
                enum:
                  - "brcm,bcm3368"
                  - "brcm,bcm3384"
                  - "brcm,bcm33843"
                  - "brcm,bcm3384-viper"
                  - "brcm,bcm33843-viper"
                  - "brcm,bcm6328"
                  - "brcm,bcm6358"
                  - "brcm,bcm6362"
                  - "brcm,bcm6368"
                  - "brcm,bcm63168"
                  - "brcm,bcm63268"
                  - "brcm,bcm7125"
                  - "brcm,bcm7346"
                  - "brcm,bcm7358"
                  - "brcm,bcm7360"
                  - "brcm,bcm7362"
                  - "brcm,bcm7420"
                  - "brcm,bcm7425"
        then:
          required:
            - mips-hpt-frequency

additionalProperties: true

examples:
  - |
     / {
         #address-cells = <1>;
         #size-cells = <1>;
         compatible = "brcm,bcm3368";

         cpus {
           #address-cells = <1>;
           #size-cells = <0>;

           mips-hpt-frequency = <150000000>;

           cpu@0 {
             compatible = "brcm,bmips4350";
             device_type = "cpu";
             reg = <0>;
           };

           cpu@1 {
             compatible = "brcm,bmips4350";
             device_type = "cpu";
             reg = <1>;
           };
         };
       };

This seems to work as expected with this node:

 make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mips/brcm/brcm,bmips-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/brcm/brcm,bmips-cpus.example.dts
  DTC     Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb
  CHECK   Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb
/home/sergio/GNUBEE-SERGIO-TEST/linux/Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb:
/: 'model' is a required property
From schema: /home/sergio/.local/lib/python3.8/site-packages/dtschema/schemas/root-node.yaml

If I remove the property from the CPU nodes I get:

make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mips/brcm/brcm,bmips-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/brcm/brcm,bmips-cpus.example.dts
  DTC     Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb
  CHECK   Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb
/home/sergio/GNUBEE-SERGIO-TEST/linux/Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb:
/: cpus: 'mips-hpt-frequency' is a required property
From schema: /home/sergio/GNUBEE-SERGIO-TEST/linux/Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.yaml
/home/sergio/GNUBEE-SERGIO-TEST/linux/Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb:
/: 'model' is a required property
From schema: /home/sergio/.local/lib/python3.8/site-packages/dtschema/schemas/root-node.yaml

And if I change the top level compatible it does not complain also:

make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mips/brcm/brcm,bmips-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/brcm/brcm,bmips-cpus.example.dts
  DTC     Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb
  CHECK   Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb
/home/sergio/GNUBEE-SERGIO-TEST/linux/Documentation/devicetree/bindings/mips/brcm/brcm,bmips-cpus.example.dtb:
/: 'model' is a required property

However, the root-node schema requires 'model' as property and there
is no model at all in any real DTS file. I don't know if can be added
only in this sample to avoid the check fail.

Thanks,
    Sergio Paracuellos


>
> Thanks,
>     Sergio Paracuellos
> >
> >
> > Best regards,
> > Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-09-21 11:40 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-17  4:11 [PATCH v2] dt-bindings: mips: add CPU bindings for MIPS architecture Sergio Paracuellos
2022-09-18 11:22 ` Thomas Bogendoerfer
2022-09-18 15:15   ` Sergio Paracuellos
2022-09-19 11:17 ` Krzysztof Kozlowski
2022-09-19 12:29   ` Sergio Paracuellos
2022-09-19 12:48     ` Krzysztof Kozlowski
2022-09-19 13:41       ` Sergio Paracuellos
2022-09-19 16:08         ` Krzysztof Kozlowski
2022-09-20  5:51           ` Sergio Paracuellos
2022-09-21  6:42             ` Krzysztof Kozlowski
2022-09-21  7:18               ` Sergio Paracuellos
2022-09-21  7:51                 ` Krzysztof Kozlowski
2022-09-21  8:11                   ` Sergio Paracuellos
2022-09-21 11:40                     ` Sergio Paracuellos

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.