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

From: Fabio Estevam <[email protected]>

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 <[email protected]>

---
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(-)

Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +2 -1

index ee4780f186f9..60b9d79e7543 100644
@@ -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

Signature · 1 line

2.34.1

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

Hi Conor,

Quoted 3 lines · Conor Dooley

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:

Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +10 -1

@@ -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

Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +19 -2

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

Hi Conor,

Quoted 2 lines · Conor Dooley

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:

Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +1 -1

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

Quoted 7 lines · Fabio Estevam

Hi Conor,

Quoted 2 lines · Conor Dooley

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

Quoted 8 lines

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.

Quoted 26 lines

does not.

On top of your patch:

Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +1 -1

@@ -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

Quoted 1 line

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:

Quoted 53 lines · Conor Dooley

Quoted 7 lines · Fabio Estevam

Hi Conor,

Quoted 2 lines · Conor Dooley

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

Quoted 8 lines

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.

Quoted 26 lines

does not.

On top of your patch:

Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +1 -1

@@ -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

Quoted 1 line

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.

Quoted 2 lines

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.

Quoted 54 lines · Rob Herring

Quoted 53 lines · Conor Dooley

Quoted 7 lines · Fabio Estevam

Hi Conor,

Quoted 2 lines · Conor Dooley

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

Quoted 8 lines

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.

Quoted 26 lines

does not.

On top of your patch:

Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +1 -1

@@ -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

Quoted 1 line

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.

Quoted 5 lines

Quoted 2 lines

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.