diff mbox series

[v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems

Message ID 20230928222130.580487-1-festevam@gmail.com
State Changes Requested
Headers show
Series [v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 9 lines checked
robh/patch-applied fail build log

Commit Message

Fabio Estevam Sept. 28, 2023, 10:21 p.m. UTC
From: Fabio Estevam <festevam@denx.de>

The number of fsl,tmu-range entries vary among the several NXP SoCs.

- lx2160a has two fsl,tmu-range entries  (fsl,qoriq-tmu compatible)
- imx8mq has four fsl,tmu-range entries. (fsl,imx8mq-tmu compatible)
- imx93 has seven fsl,tmu-range entries. (fsl,qoriq-tmu compatible)

Change minItems and maxItems accordingly.

This fixes the following schema warning:

imx93-11x11-evk.dtb: tmu@44482000: fsl,tmu-range: 'oneOf' conditional failed, one must be fixed:
        [2147483866, 2147483881, 2147483906, 2147483946, 2147484006, 2147484071, 2147484086] is too long

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v1:
- Adjust min/maxItems to cover all NXP SoCs.

 Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Conor Dooley Oct. 2, 2023, 12:27 p.m. UTC | #1
On Thu, Sep 28, 2023 at 07:21:30PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> The number of fsl,tmu-range entries vary among the several NXP SoCs.
> 
> - lx2160a has two fsl,tmu-range entries  (fsl,qoriq-tmu compatible)
> - imx8mq has four fsl,tmu-range entries. (fsl,imx8mq-tmu compatible)
> - imx93 has seven fsl,tmu-range entries. (fsl,qoriq-tmu compatible)
> 
> Change minItems and maxItems accordingly.
> 
> This fixes the following schema warning:
> 
> imx93-11x11-evk.dtb: tmu@44482000: fsl,tmu-range: 'oneOf' conditional failed, one must be fixed:
>         [2147483866, 2147483881, 2147483906, 2147483946, 2147484006, 2147484071, 2147484086] is too long
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>

tbh, this seems like a situation where per compatible constraints should
be added, since each of the devices listed above has different
requirements.

Thanks,
Conor.

> ---
> Changes since v1:
> - Adjust min/maxItems to cover all NXP SoCs.
> 
>  Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> index ee4780f186f9..60b9d79e7543 100644
> --- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> @@ -36,7 +36,8 @@ properties:
>      description: |
>        The values to be programmed into TTRnCR, as specified by the SoC
>        reference manual. The first cell is TTR0CR, the second is TTR1CR, etc.
> -    minItems: 4
> +    minItems: 2
> +    maxItems: 7
>  
>    fsl,tmu-calibration:
>      $ref: /schemas/types.yaml#/definitions/uint32-matrix
> -- 
> 2.34.1
>
Fabio Estevam Dec. 9, 2023, 7:22 p.m. UTC | #2
Hi Conor,

On Mon, Oct 2, 2023 at 9:27 AM Conor Dooley <conor@kernel.org> wrote:

> tbh, this seems like a situation where per compatible constraints should
> be added, since each of the devices listed above has different
> requirements.

Ok, I am trying to add the device constraints as suggested.

For example: I am trying to describe that imx93 has 7 items for fsl,tmu-range:

--- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
@@ -21,6 +21,7 @@ properties:
     enum:
       - fsl,qoriq-tmu
       - fsl,imx8mq-tmu
+      - fsl,imx93-tmu

   reg:
     maxItems: 1
@@ -33,7 +34,15 @@ properties:
     description: |
       The values to be programmed into TTRnCR, as specified by the SoC
       reference manual. The first cell is TTR0CR, the second is TTR1CR, etc.
-    maxItems: 4
+    items:
+      - description: TTR0CR
+      - description: TTR1CR
+      - description: TTR2CR
+      - description: TTR3CR
+      - description: TTR4CR
+      - description: TTR5CR
+      - description: TTR6CR
+    minItems: 4

   fsl,tmu-calibration:
     $ref: /schemas/types.yaml#/definitions/uint32-matrix
@@ -69,15 +78,33 @@ required:
   - fsl,tmu-calibration
   - '#thermal-sensor-cells'

+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx93-tmu
+    then:
+      properties:
+        fsl,tmu-range:
+          minItems: 7
+          maxItems: 7
+    else:
+      properties:
+        fsl,tmu-range:
+          maxItems: 4
+
 additionalProperties: false

 examples:
   - |
     tmu@f0000 {
-        compatible = "fsl,qoriq-tmu";
+        compatible = "fsl,imx93-tmu";
         reg = <0xf0000 0x1000>;
         interrupts = <18 2 0 0>;
-        fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a>;
+        fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0 0 0>;
         fsl,tmu-calibration = <0x00000000 0x00000025>,
                               <0x00000001 0x00000028>,
                               <0x00000002 0x0000002d>,

but dt_binding_check fails:

$ make dt_binding_check DT_SCHEMA_FILES=qoriq-thermal.yaml -j12
  LINT    Documentation/devicetree/bindings
  DTEX    Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
  CHKDT   Documentation/devicetree/bindings/processed-schema.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  DTC_CHK Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb
/home/fabio/linux-next/Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb:
tmu@f0000: fsl,tmu-range: [[655360, 589862, 524362, 65642, 0, 0, 0]]
is too short
from schema $id: http://devicetree.org/schemas/thermal/qoriq-thermal.yaml#

What is wrong with the yaml changes to tell that imx93 has 7 items for
fsl,tmu-range?

Thanks
Conor Dooley Dec. 9, 2023, 8:23 p.m. UTC | #3
On Sat, Dec 09, 2023 at 04:22:31PM -0300, Fabio Estevam wrote:
> Hi Conor,
> 
> On Mon, Oct 2, 2023 at 9:27 AM Conor Dooley <conor@kernel.org> wrote:
> 
> > tbh, this seems like a situation where per compatible constraints should
> > be added, since each of the devices listed above has different
> > requirements.
> 
> Ok, I am trying to add the device constraints as suggested.
> 
> For example: I am trying to describe that imx93 has 7 items for fsl,tmu-range:
> 
> --- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> @@ -21,6 +21,7 @@ properties:
>      enum:
>        - fsl,qoriq-tmu
>        - fsl,imx8mq-tmu
> +      - fsl,imx93-tmu
> 
>    reg:
>      maxItems: 1
> @@ -33,7 +34,15 @@ properties:
>      description: |
>        The values to be programmed into TTRnCR, as specified by the SoC
>        reference manual. The first cell is TTR0CR, the second is TTR1CR, etc.
> -    maxItems: 4
> +    items:
> +      - description: TTR0CR
> +      - description: TTR1CR
> +      - description: TTR2CR
> +      - description: TTR3CR
> +      - description: TTR4CR
> +      - description: TTR5CR
> +      - description: TTR6CR
> +    minItems: 4
> 
>    fsl,tmu-calibration:
>      $ref: /schemas/types.yaml#/definitions/uint32-matrix
> @@ -69,15 +78,33 @@ required:
>    - fsl,tmu-calibration
>    - '#thermal-sensor-cells'
> 
> +

^^ extra newline here I think.

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,imx93-tmu
> +    then:
> +      properties:
> +        fsl,tmu-range:
> +          minItems: 7
> +          maxItems: 7
> +    else:
> +      properties:
> +        fsl,tmu-range:
> +          maxItems: 4
> +
>  additionalProperties: false
> 
>  examples:
>    - |
>      tmu@f0000 {
> -        compatible = "fsl,qoriq-tmu";
> +        compatible = "fsl,imx93-tmu";
>          reg = <0xf0000 0x1000>;
>          interrupts = <18 2 0 0>;
> -        fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a>;
> +        fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0 0 0>;
>          fsl,tmu-calibration = <0x00000000 0x00000025>,
>                                <0x00000001 0x00000028>,
>                                <0x00000002 0x0000002d>,
> 
> but dt_binding_check fails:
> 
> $ make dt_binding_check DT_SCHEMA_FILES=qoriq-thermal.yaml -j12
>   LINT    Documentation/devicetree/bindings
>   DTEX    Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>   DTC_CHK Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb
> /home/fabio/linux-next/Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb:
> tmu@f0000: fsl,tmu-range: [[655360, 589862, 524362, 65642, 0, 0, 0]]
> is too short
> from schema $id: http://devicetree.org/schemas/thermal/qoriq-thermal.yaml#
> 
> What is wrong with the yaml changes to tell that imx93 has 7 items for
> fsl,tmu-range?

You're adding the constraints and items at the wrong level AFAICT.
I think something like the below better matches your constraints?

diff --git a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
index 145744027234..540169806cc8 100644
--- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
@@ -21,6 +21,7 @@ properties:
     enum:
       - fsl,qoriq-tmu
       - fsl,imx8mq-tmu
+      - fsl,imx93-tmu
 
   reg:
     maxItems: 1
@@ -33,7 +34,17 @@ properties:
     description: |
       The values to be programmed into TTRnCR, as specified by the SoC
       reference manual. The first cell is TTR0CR, the second is TTR1CR, etc.
-    maxItems: 4
+    items:
+      items:
+        - description: TTR0CR
+        - description: TTR1CR
+        - description: TTR2CR
+        - description: TTR3CR
+        - description: TTR4CR
+        - description: TTR5CR
+        - description: TTR6CR
+      minItems: 4
+    maxItems: 1
 
   fsl,tmu-calibration:
     $ref: /schemas/types.yaml#/definitions/uint32-matrix
@@ -69,15 +80,34 @@ required:
   - fsl,tmu-calibration
   - '#thermal-sensor-cells'
 
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx93-tmu
+    then:
+      properties:
+        fsl,tmu-range:
+          items:
+            minItems: 7
+    else:
+      properties:
+        fsl,tmu-range:
+          items:
+            maxItems: 4
+
 additionalProperties: false
 
 examples:
   - |
     tmu@f0000 {
-        compatible = "fsl,qoriq-tmu";
+        compatible = "fsl,imx93-tmu";
         reg = <0xf0000 0x1000>;
         interrupts = <18 2 0 0>;
-        fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a>;
+        fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0 0 0>;
         fsl,tmu-calibration = <0x00000000 0x00000025>,
                               <0x00000001 0x00000028>,
                               <0x00000002 0x0000002d>,
Fabio Estevam Dec. 9, 2023, 8:59 p.m. UTC | #4
Hi Conor,

On 09/12/2023 17:23, Conor Dooley wrote:

> You're adding the constraints and items at the wrong level AFAICT.
> I think something like the below better matches your constraints?

Thanks for your example.

With your change the fsl,imx93-tmu case works correctly:
if I pass the number of fsl,tmu-range entries different than 7,
dt_binding_check correctly complains.

However, if I pass 7 entries to fsl,qoriq-tmu it should complain as it 
expects 4, but it
does not.

On top of your patch:

--- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
@@ -104,7 +104,7 @@ additionalProperties: false
  examples:
    - |
      tmu@f0000 {
-        compatible = "fsl,imx93-tmu";
+        compatible = "fsl,qoriq-tmu";
          reg = <0xf0000 0x1000>;
          interrupts = <18 2 0 0>;
          fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0 
0 0>;

make dt_binding_check DT_SCHEMA_FILES=qoriq-thermal.yaml
   LINT    Documentation/devicetree/bindings
   DTEX    
Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
   CHKDT   Documentation/devicetree/bindings/processed-schema.json
   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
   DTC_CHK 
Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb

Any suggestions?

Thanks
Conor Dooley Dec. 10, 2023, 2:52 p.m. UTC | #5
On Sat, Dec 09, 2023 at 05:59:17PM -0300, Fabio Estevam wrote:
> Hi Conor,
> 
> On 09/12/2023 17:23, Conor Dooley wrote:
> 
> > You're adding the constraints and items at the wrong level AFAICT.
> > I think something like the below better matches your constraints?
> 
> Thanks for your example.
> 
> With your change the fsl,imx93-tmu case works correctly:
> if I pass the number of fsl,tmu-range entries different than 7,
> dt_binding_check correctly complains.
> 
> However, if I pass 7 entries to fsl,qoriq-tmu it should complain as it
> expects 4, but it

btw, unrelated - minItems seems (from a grep) like it needs to be 2 not
4.

> does not.
> 
> On top of your patch:
> 
> --- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> @@ -104,7 +104,7 @@ additionalProperties: false
>  examples:
>    - |
>      tmu@f0000 {
> -        compatible = "fsl,imx93-tmu";
> +        compatible = "fsl,qoriq-tmu";
>          reg = <0xf0000 0x1000>;
>          interrupts = <18 2 0 0>;
>          fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0 0
> 0>;
> 
> make dt_binding_check DT_SCHEMA_FILES=qoriq-thermal.yaml
>   LINT    Documentation/devicetree/bindings
>   DTEX
> Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>   DTC_CHK
> Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb
> 
> Any suggestions?

I dunno. I spent far far longer than I would like to admit trying to fix
this. Firstly my suggestion here is crap I think and only applies to
___matrices___. I think it needs to be the way you had it in your diff,
but I cannot figure out why it doesn't apply the maxItems constraint.

Perhaps Rob or Krzysztof can figure out what we were overlooking.
The diff in question was something like:

diff --git a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
index 145744027234..50787938d605 100644
--- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
@@ -21,6 +21,7 @@ properties:
     enum:
       - fsl,qoriq-tmu
       - fsl,imx8mq-tmu
+      - fsl,imx93-tmu
 
   reg:
     maxItems: 1
@@ -33,7 +34,15 @@ properties:
     description: |
       The values to be programmed into TTRnCR, as specified by the SoC
       reference manual. The first cell is TTR0CR, the second is TTR1CR, etc.
-    maxItems: 4
+    minItems: 2
+    items:
+      - description: TTR0CR
+      - description: TTR1CR
+      - description: TTR2CR
+      - description: TTR3CR
+      - description: TTR4CR
+      - description: TTR5CR
+      - description: TTR6CR
 
   fsl,tmu-calibration:
     $ref: /schemas/types.yaml#/definitions/uint32-matrix
@@ -69,15 +78,30 @@ required:
   - fsl,tmu-calibration
   - '#thermal-sensor-cells'
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,imx93-tmu
+    then:
+      properties:
+        fsl,tmu-range:
+          minItems: 7
+    else:
+      properties:
+        fsl,tmu-range:
+          maxItems: 4
+
 additionalProperties: false
 
 examples:
   - |
     tmu@f0000 {
-        compatible = "fsl,qoriq-tmu";
+        compatible = "fsl,imx8mq-tmu";
         reg = <0xf0000 0x1000>;
         interrupts = <18 2 0 0>;
-        fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a>;
+        fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0x0 0x0>;
         fsl,tmu-calibration = <0x00000000 0x00000025>,
                               <0x00000001 0x00000028>,
                               <0x00000002 0x0000002d>,
Rob Herring (Arm) Dec. 10, 2023, 3:37 p.m. UTC | #6
On Sun, Dec 10, 2023 at 02:52:45PM +0000, Conor Dooley wrote:
> On Sat, Dec 09, 2023 at 05:59:17PM -0300, Fabio Estevam wrote:
> > Hi Conor,
> > 
> > On 09/12/2023 17:23, Conor Dooley wrote:
> > 
> > > You're adding the constraints and items at the wrong level AFAICT.
> > > I think something like the below better matches your constraints?
> > 
> > Thanks for your example.
> > 
> > With your change the fsl,imx93-tmu case works correctly:
> > if I pass the number of fsl,tmu-range entries different than 7,
> > dt_binding_check correctly complains.
> > 
> > However, if I pass 7 entries to fsl,qoriq-tmu it should complain as it
> > expects 4, but it
> 
> btw, unrelated - minItems seems (from a grep) like it needs to be 2 not
> 4.
> 
> > does not.
> > 
> > On top of your patch:
> > 
> > --- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> > +++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> > @@ -104,7 +104,7 @@ additionalProperties: false
> >  examples:
> >    - |
> >      tmu@f0000 {
> > -        compatible = "fsl,imx93-tmu";
> > +        compatible = "fsl,qoriq-tmu";
> >          reg = <0xf0000 0x1000>;
> >          interrupts = <18 2 0 0>;
> >          fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0 0
> > 0>;
> > 
> > make dt_binding_check DT_SCHEMA_FILES=qoriq-thermal.yaml
> >   LINT    Documentation/devicetree/bindings
> >   DTEX
> > Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
> >   CHKDT   Documentation/devicetree/bindings/processed-schema.json
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> >   DTC_CHK
> > Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb
> > 
> > Any suggestions?
> 
> I dunno. I spent far far longer than I would like to admit trying to fix
> this. Firstly my suggestion here is crap I think and only applies to
> ___matrices___. I think it needs to be the way you had it in your diff,
> but I cannot figure out why it doesn't apply the maxItems constraint.
> 
> Perhaps Rob or Krzysztof can figure out what we were overlooking.
> The diff in question was something like:

I suspect something is going afoul in the fixups. Will look at it 
tomorrow.

Rob
Conor Dooley Dec. 14, 2023, 5:13 p.m. UTC | #7
On Sun, Dec 10, 2023 at 09:37:49AM -0600, Rob Herring wrote:
> On Sun, Dec 10, 2023 at 02:52:45PM +0000, Conor Dooley wrote:
> > On Sat, Dec 09, 2023 at 05:59:17PM -0300, Fabio Estevam wrote:
> > > Hi Conor,
> > > 
> > > On 09/12/2023 17:23, Conor Dooley wrote:
> > > 
> > > > You're adding the constraints and items at the wrong level AFAICT.
> > > > I think something like the below better matches your constraints?
> > > 
> > > Thanks for your example.
> > > 
> > > With your change the fsl,imx93-tmu case works correctly:
> > > if I pass the number of fsl,tmu-range entries different than 7,
> > > dt_binding_check correctly complains.
> > > 
> > > However, if I pass 7 entries to fsl,qoriq-tmu it should complain as it
> > > expects 4, but it
> > 
> > btw, unrelated - minItems seems (from a grep) like it needs to be 2 not
> > 4.
> > 
> > > does not.
> > > 
> > > On top of your patch:
> > > 
> > > --- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> > > +++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> > > @@ -104,7 +104,7 @@ additionalProperties: false
> > >  examples:
> > >    - |
> > >      tmu@f0000 {
> > > -        compatible = "fsl,imx93-tmu";
> > > +        compatible = "fsl,qoriq-tmu";
> > >          reg = <0xf0000 0x1000>;
> > >          interrupts = <18 2 0 0>;
> > >          fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0 0
> > > 0>;
> > > 
> > > make dt_binding_check DT_SCHEMA_FILES=qoriq-thermal.yaml
> > >   LINT    Documentation/devicetree/bindings
> > >   DTEX
> > > Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
> > >   CHKDT   Documentation/devicetree/bindings/processed-schema.json
> > >   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> > >   DTC_CHK
> > > Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb
> > > 
> > > Any suggestions?
> > 
> > I dunno. I spent far far longer than I would like to admit trying to fix
> > this. Firstly my suggestion here is crap I think and only applies to
> > ___matrices___. I think it needs to be the way you had it in your diff,
> > but I cannot figure out why it doesn't apply the maxItems constraint.
> > 
> > Perhaps Rob or Krzysztof can figure out what we were overlooking.
> > The diff in question was something like:
> 
> I suspect something is going afoul in the fixups. Will look at it 
> tomorrow.

Did you manage to figure out what was causing this btw?
Rob Herring (Arm) Jan. 3, 2024, 9:39 p.m. UTC | #8
On Thu, Dec 14, 2023 at 10:13 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Sun, Dec 10, 2023 at 09:37:49AM -0600, Rob Herring wrote:
> > On Sun, Dec 10, 2023 at 02:52:45PM +0000, Conor Dooley wrote:
> > > On Sat, Dec 09, 2023 at 05:59:17PM -0300, Fabio Estevam wrote:
> > > > Hi Conor,
> > > >
> > > > On 09/12/2023 17:23, Conor Dooley wrote:
> > > >
> > > > > You're adding the constraints and items at the wrong level AFAICT.
> > > > > I think something like the below better matches your constraints?
> > > >
> > > > Thanks for your example.
> > > >
> > > > With your change the fsl,imx93-tmu case works correctly:
> > > > if I pass the number of fsl,tmu-range entries different than 7,
> > > > dt_binding_check correctly complains.
> > > >
> > > > However, if I pass 7 entries to fsl,qoriq-tmu it should complain as it
> > > > expects 4, but it
> > >
> > > btw, unrelated - minItems seems (from a grep) like it needs to be 2 not
> > > 4.
> > >
> > > > does not.
> > > >
> > > > On top of your patch:
> > > >
> > > > --- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> > > > +++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
> > > > @@ -104,7 +104,7 @@ additionalProperties: false
> > > >  examples:
> > > >    - |
> > > >      tmu@f0000 {
> > > > -        compatible = "fsl,imx93-tmu";
> > > > +        compatible = "fsl,qoriq-tmu";
> > > >          reg = <0xf0000 0x1000>;
> > > >          interrupts = <18 2 0 0>;
> > > >          fsl,tmu-range = <0x000a0000 0x00090026 0x0008004a 0x0001006a 0 0
> > > > 0>;
> > > >
> > > > make dt_binding_check DT_SCHEMA_FILES=qoriq-thermal.yaml
> > > >   LINT    Documentation/devicetree/bindings
> > > >   DTEX
> > > > Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
> > > >   CHKDT   Documentation/devicetree/bindings/processed-schema.json
> > > >   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> > > >   DTC_CHK
> > > > Documentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb
> > > >
> > > > Any suggestions?
> > >
> > > I dunno. I spent far far longer than I would like to admit trying to fix
> > > this. Firstly my suggestion here is crap I think and only applies to
> > > ___matrices___. I think it needs to be the way you had it in your diff,
> > > but I cannot figure out why it doesn't apply the maxItems constraint.
> > >
> > > Perhaps Rob or Krzysztof can figure out what we were overlooking.
> > > The diff in question was something like:
> >
> > I suspect something is going afoul in the fixups. Will look at it
> > tomorrow.
>
> Did you manage to figure out what was causing this btw?

Yes. There's not really an immediate fix I see. The issue is in the
if/then schemas we don't have enough information to know the type of
fsl,tmu-range. To work correctly, it needs to be transformed to:

fsl,tmu-range:
  items:
    minItems: 7
    maxItems: 7

This goes back to everything gets encoded into a 2 dim matrix, but the
schemas try to hide this encoding. My plan here is to eventually drop
doing that and decode properties to their correct type. That will drop
a lot of the fixups. I have patches to do that, but then it has other
corner cases.

So short term, I'd just leave things such that they don't warn or just
drop the conditional.

Rob
Fabio Estevam Jan. 3, 2024, 9:51 p.m. UTC | #9
On Wed, Jan 3, 2024 at 6:39 PM Rob Herring <robh@kernel.org> wrote:

> Yes. There's not really an immediate fix I see. The issue is in the
> if/then schemas we don't have enough information to know the type of
> fsl,tmu-range. To work correctly, it needs to be transformed to:
>
> fsl,tmu-range:
>   items:
>     minItems: 7
>     maxItems: 7

Is this a typo?

minItems cannot be 7.

- lx2160a has two fsl,tmu-range entries
- imx8mq has four fsl,tmu-range entries.

> This goes back to everything gets encoded into a 2 dim matrix, but the
> schemas try to hide this encoding. My plan here is to eventually drop
> doing that and decode properties to their correct type. That will drop
> a lot of the fixups. I have patches to do that, but then it has other
> corner cases.
>
> So short term, I'd just leave things such that they don't warn or just
> drop the conditional.

Is my v3 proposal good then?
https://patchwork.kernel.org/project/linux-pm/patch/20230928222130.580487-1-festevam@gmail.com/

It fixes all fsl,tmu-range warnings
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
index ee4780f186f9..60b9d79e7543 100644
--- a/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml
@@ -36,7 +36,8 @@  properties:
     description: |
       The values to be programmed into TTRnCR, as specified by the SoC
       reference manual. The first cell is TTR0CR, the second is TTR1CR, etc.
-    minItems: 4
+    minItems: 2
+    maxItems: 7
 
   fsl,tmu-calibration:
     $ref: /schemas/types.yaml#/definitions/uint32-matrix