dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
Subsystems (2)
PATCH v3 1/1 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
---
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
@@ -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
Quoted 8 lines · Fabio Estevam
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)
Quoted 8 lines
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 longSigned-off-by Fabio Estevam <[email protected]>
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.
Quoted 24 lines
---
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@@ -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-matrixSignature · 1 line
2.34.1
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 +30 -3
@@ -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
Quoted 40 lines · Fabio Estevam
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 required: · line 81
Quoted 4 lines
Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +1
-0@@ -69,15 +78,33 @@ required: - fsl,tmu-calibration - '#thermal-sensor-cells' +
^^ extra newline here I think.
Quoted 45 lines
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_checkfails:$ 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#
Quoted 2 lines
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?
Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +33 -3
@@ -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>,
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
DTEXDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
CHKDT Documentation/devicetree/bindings/processed-schema.json
SCHEMA Documentation/devicetree/bindings/processed-schema.json DTC_CHKDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dtb
Any suggestions?
Thanks
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_checkcorrectly 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 00>;
make
dt_binding_checkDT_SCHEMA_FILES=qoriq-thermal.yaml
LINTDocumentation/devicetree/bindings
DTEXDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
CHKDTDocumentation/devicetree/bindings/processed-schema.json
SCHEMADocumentation/devicetree/bindings/processed-schema.jsonDTC_CHKDocumentation/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:
Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml +27 -3
@@ -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
maintainerQuoted 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_checkcorrectly 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 00>;
make
dt_binding_checkDT_SCHEMA_FILES=qoriq-thermal.yaml
LINTDocumentation/devicetree/bindings
DTEXDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
CHKDTDocumentation/devicetree/bindings/processed-schema.json
SCHEMADocumentation/devicetree/bindings/processed-schema.jsonDTC_CHKDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dtbQuoted 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.
Rob
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_checkcorrectly 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 00>;
make
dt_binding_checkDT_SCHEMA_FILES=qoriq-thermal.yaml
LINTDocumentation/devicetree/bindings
DTEXDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
CHKDTDocumentation/devicetree/bindings/processed-schema.json
SCHEMADocumentation/devicetree/bindings/processed-schema.jsonDTC_CHKDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dtbQuoted 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?
Rob Herring
maintainerQuoted 56 lines · Conor Dooley
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_checkcorrectly 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 00>;
make
dt_binding_checkDT_SCHEMA_FILES=qoriq-thermal.yaml
LINTDocumentation/devicetree/bindings
DTEXDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dts
CHKDTDocumentation/devicetree/bindings/processed-schema.json
SCHEMADocumentation/devicetree/bindings/processed-schema.jsonDTC_CHKDocumentation/devicetree/bindings/thermal/qoriq-thermal.example.dtbQuoted 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 7 lines
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.
Rob
Quoted 8 lines · Rob Herring
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.
Quoted 8 lines
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/[email protected]/
It fixes all fsl,tmu-range warnings