devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
@ 2023-09-28 22:21 Fabio Estevam
  2023-10-02 12:27 ` Conor Dooley
  0 siblings, 1 reply; 10+ messages in thread
From: Fabio Estevam @ 2023-09-28 22:21 UTC (permalink / raw)
  To: daniel.lezcano
  Cc: rafael, krzysztof.kozlowski+dt, robh+dt, conor+dt, linux-pm,
	devicetree, Fabio Estevam

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

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


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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2023-09-28 22:21 [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems Fabio Estevam
@ 2023-10-02 12:27 ` Conor Dooley
  2023-12-09 19:22   ` Fabio Estevam
  0 siblings, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2023-10-02 12:27 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: daniel.lezcano, rafael, krzysztof.kozlowski+dt, robh+dt, conor+dt,
	linux-pm, devicetree, Fabio Estevam

[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2023-10-02 12:27 ` Conor Dooley
@ 2023-12-09 19:22   ` Fabio Estevam
  2023-12-09 20:23     ` Conor Dooley
  0 siblings, 1 reply; 10+ messages in thread
From: Fabio Estevam @ 2023-12-09 19:22 UTC (permalink / raw)
  To: Conor Dooley
  Cc: daniel.lezcano, rafael, krzysztof.kozlowski+dt, robh+dt, conor+dt,
	linux-pm, devicetree, Fabio Estevam

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

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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2023-12-09 19:22   ` Fabio Estevam
@ 2023-12-09 20:23     ` Conor Dooley
  2023-12-09 20:59       ` Fabio Estevam
  0 siblings, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2023-12-09 20:23 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: daniel.lezcano, rafael, krzysztof.kozlowski+dt, robh+dt, conor+dt,
	linux-pm, devicetree, Fabio Estevam

[-- Attachment #1: Type: text/plain, Size: 5423 bytes --]

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2023-12-09 20:23     ` Conor Dooley
@ 2023-12-09 20:59       ` Fabio Estevam
  2023-12-10 14:52         ` Conor Dooley
  0 siblings, 1 reply; 10+ messages in thread
From: Fabio Estevam @ 2023-12-09 20:59 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Fabio Estevam, daniel.lezcano, rafael, krzysztof.kozlowski+dt,
	robh+dt, conor+dt, linux-pm, devicetree

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

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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2023-12-09 20:59       ` Fabio Estevam
@ 2023-12-10 14:52         ` Conor Dooley
  2023-12-10 15:37           ` Rob Herring
  0 siblings, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2023-12-10 14:52 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Fabio Estevam, daniel.lezcano, rafael, krzysztof.kozlowski+dt,
	robh+dt, conor+dt, linux-pm, devicetree

[-- Attachment #1: Type: text/plain, Size: 3952 bytes --]

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2023-12-10 14:52         ` Conor Dooley
@ 2023-12-10 15:37           ` Rob Herring
  2023-12-14 17:13             ` Conor Dooley
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2023-12-10 15:37 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Fabio Estevam, Fabio Estevam, daniel.lezcano, rafael,
	krzysztof.kozlowski+dt, conor+dt, linux-pm, devicetree

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

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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2023-12-10 15:37           ` Rob Herring
@ 2023-12-14 17:13             ` Conor Dooley
  2024-01-03 21:39               ` Rob Herring
  0 siblings, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2023-12-14 17:13 UTC (permalink / raw)
  To: Rob Herring
  Cc: Fabio Estevam, Fabio Estevam, daniel.lezcano, rafael,
	krzysztof.kozlowski+dt, conor+dt, linux-pm, devicetree

[-- Attachment #1: Type: text/plain, Size: 2510 bytes --]

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2023-12-14 17:13             ` Conor Dooley
@ 2024-01-03 21:39               ` Rob Herring
  2024-01-03 21:51                 ` Fabio Estevam
  0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2024-01-03 21:39 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Fabio Estevam, Fabio Estevam, daniel.lezcano, rafael,
	krzysztof.kozlowski+dt, conor+dt, linux-pm, devicetree

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

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

* Re: [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems
  2024-01-03 21:39               ` Rob Herring
@ 2024-01-03 21:51                 ` Fabio Estevam
  0 siblings, 0 replies; 10+ messages in thread
From: Fabio Estevam @ 2024-01-03 21:51 UTC (permalink / raw)
  To: Rob Herring
  Cc: Conor Dooley, Fabio Estevam, daniel.lezcano, rafael,
	krzysztof.kozlowski+dt, conor+dt, linux-pm, devicetree

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

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

end of thread, other threads:[~2024-01-03 21:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-28 22:21 [PATCH v3] dt-bindings: thermal: qoriq-thermal: Adjust fsl,tmu-range min/maxItems Fabio Estevam
2023-10-02 12:27 ` Conor Dooley
2023-12-09 19:22   ` Fabio Estevam
2023-12-09 20:23     ` Conor Dooley
2023-12-09 20:59       ` Fabio Estevam
2023-12-10 14:52         ` Conor Dooley
2023-12-10 15:37           ` Rob Herring
2023-12-14 17:13             ` Conor Dooley
2024-01-03 21:39               ` Rob Herring
2024-01-03 21:51                 ` Fabio Estevam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).