| Message ID | 20230928222130.580487-1-festevam@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems | expand |
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 >
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
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>,
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
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
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?
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
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