devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
@ 2024-08-18 17:30 Krzysztof Kozlowski
  2024-08-18 17:30 ` [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints Krzysztof Kozlowski
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-18 17:30 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel
  Cc: Krzysztof Kozlowski

Each variable-length property like interrupts or resets must have fixed
constraints on number of items for given variant in binding.  The
clauses in "if:then:" block should define both limits: upper and lower.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/display/renesas,du.yaml          | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
index c5b9e6812bce..147842b6465a 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
@@ -173,6 +173,7 @@ allOf:
             - pattern: '^dclkin\.[01]$'
 
         interrupts:
+          minItems: 2
           maxItems: 2
 
         resets:
@@ -228,6 +229,7 @@ allOf:
             - pattern: '^dclkin\.[01]$'
 
         interrupts:
+          minItems: 2
           maxItems: 2
 
         resets:
@@ -281,6 +283,7 @@ allOf:
             - pattern: '^dclkin\.[01]$'
 
         interrupts:
+          minItems: 2
           maxItems: 2
 
         resets:
@@ -335,6 +338,7 @@ allOf:
             - pattern: '^dclkin\.[01]$'
 
         interrupts:
+          minItems: 2
           maxItems: 2
 
         resets:
@@ -396,6 +400,7 @@ allOf:
             - pattern: '^dclkin\.[012]$'
 
         interrupts:
+          minItems: 3
           maxItems: 3
 
         resets:
@@ -460,9 +465,11 @@ allOf:
             - pattern: '^dclkin\.[0123]$'
 
         interrupts:
+          minItems: 4
           maxItems: 4
 
         resets:
+          minItems: 2
           maxItems: 2
 
         reset-names:
@@ -489,9 +496,11 @@ allOf:
 
         renesas,cmms:
           minItems: 4
+          maxItems: 4
 
         renesas,vsps:
           minItems: 4
+          maxItems: 4
 
       required:
         - clock-names
@@ -531,9 +540,11 @@ allOf:
             - pattern: '^dclkin\.[012]$'
 
         interrupts:
+          minItems: 3
           maxItems: 3
 
         resets:
+          minItems: 2
           maxItems: 2
 
         reset-names:
@@ -558,9 +569,11 @@ allOf:
 
         renesas,cmms:
           minItems: 3
+          maxItems: 3
 
         renesas,vsps:
           minItems: 3
+          maxItems: 3
 
       required:
         - clock-names
@@ -600,9 +613,11 @@ allOf:
             - pattern: '^dclkin\.[013]$'
 
         interrupts:
+          minItems: 3
           maxItems: 3
 
         resets:
+          minItems: 2
           maxItems: 2
 
         reset-names:
@@ -627,9 +642,11 @@ allOf:
 
         renesas,cmms:
           minItems: 3
+          maxItems: 3
 
         renesas,vsps:
           minItems: 3
+          maxItems: 3
 
       required:
         - clock-names
@@ -684,6 +701,7 @@ allOf:
 
         renesas,vsps:
           minItems: 1
+          maxItems: 1
 
       required:
         - clock-names
@@ -719,6 +737,7 @@ allOf:
             - pattern: '^dclkin\.[01]$'
 
         interrupts:
+          minItems: 2
           maxItems: 2
 
         resets:
@@ -746,9 +765,11 @@ allOf:
 
         renesas,cmms:
           minItems: 2
+          maxItems: 2
 
         renesas,vsps:
           minItems: 2
+          maxItems: 2
 
       required:
         - clock-names
@@ -799,6 +820,7 @@ allOf:
 
         renesas,vsps:
           minItems: 2
+          maxItems: 2
 
       required:
         - clock-names
-- 
2.43.0


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

* [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints
  2024-08-18 17:30 [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants Krzysztof Kozlowski
@ 2024-08-18 17:30 ` Krzysztof Kozlowski
  2024-08-18 17:46   ` Laurent Pinchart
  2024-08-19 17:11   ` Conor Dooley
  2024-08-18 17:41 ` [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants Laurent Pinchart
  2024-08-19 17:12 ` Conor Dooley
  2 siblings, 2 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-18 17:30 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel
  Cc: Krzysztof Kozlowski

Properties with variable number of items per each device are expected to
have widest constraints in top-level "properties:" block and further
customized (narrowed) in "if:then:".  Add missing top-level constraints
for clocks, clock-names, interrupts, resets, reset-names, renesas,cmms
and renesas,vsps.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/display/renesas,du.yaml          | 26 ++++++++++++++++---
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
index 147842b6465a..9a2d1c08cb1f 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.yaml
+++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
@@ -46,12 +46,26 @@ properties:
     maxItems: 1
 
   # See compatible-specific constraints below.
-  clocks: true
-  clock-names: true
+  clocks:
+    minItems: 1
+    maxItems: 8
+
+  clock-names:
+    minItems: 1
+    maxItems: 8
+
   interrupts:
+    minItems: 1
+    maxItems: 4
     description: Interrupt specifiers, one per DU channel
-  resets: true
-  reset-names: true
+
+  resets:
+    minItems: 1
+    maxItems: 2
+
+  reset-names:
+    minItems: 1
+    maxItems: 2
 
   power-domains:
     maxItems: 1
@@ -77,6 +91,8 @@ properties:
 
   renesas,cmms:
     $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 2
+    maxItems: 4
     items:
       maxItems: 1
     description:
@@ -85,6 +101,8 @@ properties:
 
   renesas,vsps:
     $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 4
     items:
       items:
         - description: phandle to VSP instance that serves the DU channel
-- 
2.43.0


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

* Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
  2024-08-18 17:30 [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants Krzysztof Kozlowski
  2024-08-18 17:30 ` [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints Krzysztof Kozlowski
@ 2024-08-18 17:41 ` Laurent Pinchart
  2024-08-18 17:44   ` Krzysztof Kozlowski
  2024-08-19 17:12 ` Conor Dooley
  2 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2024-08-18 17:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

Hi Krzysztof,

Thank you for the patch.

On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote:
> Each variable-length property like interrupts or resets must have fixed
> constraints on number of items for given variant in binding.  The
> clauses in "if:then:" block should define both limits: upper and lower.

I thought that, when only one of minItems or maxItems was specified, the
other automatically defaulted to the same value. I'm pretty sure I
recall Rob asking me to drop one of the two in some bindings. Has the
rule changes ? Is it documented somewhere ?

> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/display/renesas,du.yaml          | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> index c5b9e6812bce..147842b6465a 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> @@ -173,6 +173,7 @@ allOf:
>              - pattern: '^dclkin\.[01]$'
>  
>          interrupts:
> +          minItems: 2
>            maxItems: 2
>  
>          resets:
> @@ -228,6 +229,7 @@ allOf:
>              - pattern: '^dclkin\.[01]$'
>  
>          interrupts:
> +          minItems: 2
>            maxItems: 2
>  
>          resets:
> @@ -281,6 +283,7 @@ allOf:
>              - pattern: '^dclkin\.[01]$'
>  
>          interrupts:
> +          minItems: 2
>            maxItems: 2
>  
>          resets:
> @@ -335,6 +338,7 @@ allOf:
>              - pattern: '^dclkin\.[01]$'
>  
>          interrupts:
> +          minItems: 2
>            maxItems: 2
>  
>          resets:
> @@ -396,6 +400,7 @@ allOf:
>              - pattern: '^dclkin\.[012]$'
>  
>          interrupts:
> +          minItems: 3
>            maxItems: 3
>  
>          resets:
> @@ -460,9 +465,11 @@ allOf:
>              - pattern: '^dclkin\.[0123]$'
>  
>          interrupts:
> +          minItems: 4
>            maxItems: 4
>  
>          resets:
> +          minItems: 2
>            maxItems: 2
>  
>          reset-names:
> @@ -489,9 +496,11 @@ allOf:
>  
>          renesas,cmms:
>            minItems: 4
> +          maxItems: 4
>  
>          renesas,vsps:
>            minItems: 4
> +          maxItems: 4
>  
>        required:
>          - clock-names
> @@ -531,9 +540,11 @@ allOf:
>              - pattern: '^dclkin\.[012]$'
>  
>          interrupts:
> +          minItems: 3
>            maxItems: 3
>  
>          resets:
> +          minItems: 2
>            maxItems: 2
>  
>          reset-names:
> @@ -558,9 +569,11 @@ allOf:
>  
>          renesas,cmms:
>            minItems: 3
> +          maxItems: 3
>  
>          renesas,vsps:
>            minItems: 3
> +          maxItems: 3
>  
>        required:
>          - clock-names
> @@ -600,9 +613,11 @@ allOf:
>              - pattern: '^dclkin\.[013]$'
>  
>          interrupts:
> +          minItems: 3
>            maxItems: 3
>  
>          resets:
> +          minItems: 2
>            maxItems: 2
>  
>          reset-names:
> @@ -627,9 +642,11 @@ allOf:
>  
>          renesas,cmms:
>            minItems: 3
> +          maxItems: 3
>  
>          renesas,vsps:
>            minItems: 3
> +          maxItems: 3
>  
>        required:
>          - clock-names
> @@ -684,6 +701,7 @@ allOf:
>  
>          renesas,vsps:
>            minItems: 1
> +          maxItems: 1
>  
>        required:
>          - clock-names
> @@ -719,6 +737,7 @@ allOf:
>              - pattern: '^dclkin\.[01]$'
>  
>          interrupts:
> +          minItems: 2
>            maxItems: 2
>  
>          resets:
> @@ -746,9 +765,11 @@ allOf:
>  
>          renesas,cmms:
>            minItems: 2
> +          maxItems: 2
>  
>          renesas,vsps:
>            minItems: 2
> +          maxItems: 2
>  
>        required:
>          - clock-names
> @@ -799,6 +820,7 @@ allOf:
>  
>          renesas,vsps:
>            minItems: 2
> +          maxItems: 2
>  
>        required:
>          - clock-names

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
  2024-08-18 17:41 ` [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants Laurent Pinchart
@ 2024-08-18 17:44   ` Krzysztof Kozlowski
  2024-08-18 17:51     ` Laurent Pinchart
  0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-18 17:44 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

On 18/08/2024 19:41, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> Thank you for the patch.
> 
> On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote:
>> Each variable-length property like interrupts or resets must have fixed
>> constraints on number of items for given variant in binding.  The
>> clauses in "if:then:" block should define both limits: upper and lower.
> 
> I thought that, when only one of minItems or maxItems was specified, the
> other automatically defaulted to the same value. I'm pretty sure I
> recall Rob asking me to drop one of the two in some bindings. Has the
> rule changes ? Is it documented somewhere ?

New dtschema changed it and, even if previous behavior is restored, the
size in if:then: always had to be constrained. You could have skipped
one side of limit if it was equal to outer/top-level limit, e.g:

properties:
  clocks:
    minItems: 1
    maxItems: 2


if:then:properties:
  clocks:
    minItems: 2


Best regards,
Krzysztof


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

* Re: [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints
  2024-08-18 17:30 ` [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints Krzysztof Kozlowski
@ 2024-08-18 17:46   ` Laurent Pinchart
  2024-08-19 17:11   ` Conor Dooley
  1 sibling, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2024-08-18 17:46 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

Hi Krzysztof,

Thank you for the patch.

On Sun, Aug 18, 2024 at 07:30:03PM +0200, Krzysztof Kozlowski wrote:
> Properties with variable number of items per each device are expected to
> have widest constraints in top-level "properties:" block and further
> customized (narrowed) in "if:then:".  Add missing top-level constraints
> for clocks, clock-names, interrupts, resets, reset-names, renesas,cmms
> and renesas,vsps.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
>  .../bindings/display/renesas,du.yaml          | 26 ++++++++++++++++---
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> index 147842b6465a..9a2d1c08cb1f 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> @@ -46,12 +46,26 @@ properties:
>      maxItems: 1
>  
>    # See compatible-specific constraints below.
> -  clocks: true
> -  clock-names: true
> +  clocks:
> +    minItems: 1
> +    maxItems: 8
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 8
> +
>    interrupts:
> +    minItems: 1
> +    maxItems: 4
>      description: Interrupt specifiers, one per DU channel
> -  resets: true
> -  reset-names: true
> +
> +  resets:
> +    minItems: 1
> +    maxItems: 2
> +
> +  reset-names:
> +    minItems: 1
> +    maxItems: 2
>  
>    power-domains:
>      maxItems: 1
> @@ -77,6 +91,8 @@ properties:
>  
>    renesas,cmms:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 2
> +    maxItems: 4
>      items:
>        maxItems: 1
>      description:
> @@ -85,6 +101,8 @@ properties:
>  
>    renesas,vsps:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 1
> +    maxItems: 4
>      items:
>        items:
>          - description: phandle to VSP instance that serves the DU channel

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
  2024-08-18 17:44   ` Krzysztof Kozlowski
@ 2024-08-18 17:51     ` Laurent Pinchart
  2024-08-18 18:48       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2024-08-18 17:51 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

On Sun, Aug 18, 2024 at 07:44:22PM +0200, Krzysztof Kozlowski wrote:
> On 18/08/2024 19:41, Laurent Pinchart wrote:
> > Hi Krzysztof,
> > 
> > Thank you for the patch.
> > 
> > On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote:
> >> Each variable-length property like interrupts or resets must have fixed
> >> constraints on number of items for given variant in binding.  The
> >> clauses in "if:then:" block should define both limits: upper and lower.
> > 
> > I thought that, when only one of minItems or maxItems was specified, the
> > other automatically defaulted to the same value. I'm pretty sure I
> > recall Rob asking me to drop one of the two in some bindings. Has the
> > rule changes ? Is it documented somewhere ?
> 
> New dtschema changed it and, even if previous behavior is restored, the
> size in if:then: always had to be constrained. You could have skipped
> one side of limit if it was equal to outer/top-level limit, e.g:
> 
> properties:
>   clocks:
>     minItems: 1
>     maxItems: 2
> 
> 
> if:then:properties:
>   clocks:
>     minItems: 2

Where can I find a description of the behaviour of the new dtschema
(hopefully with some documentation) ?

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
  2024-08-18 17:51     ` Laurent Pinchart
@ 2024-08-18 18:48       ` Krzysztof Kozlowski
  2024-08-28 12:45         ` Laurent Pinchart
  0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-18 18:48 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

On 18/08/2024 19:51, Laurent Pinchart wrote:
> On Sun, Aug 18, 2024 at 07:44:22PM +0200, Krzysztof Kozlowski wrote:
>> On 18/08/2024 19:41, Laurent Pinchart wrote:
>>> Hi Krzysztof,
>>>
>>> Thank you for the patch.
>>>
>>> On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote:
>>>> Each variable-length property like interrupts or resets must have fixed
>>>> constraints on number of items for given variant in binding.  The
>>>> clauses in "if:then:" block should define both limits: upper and lower.
>>>
>>> I thought that, when only one of minItems or maxItems was specified, the
>>> other automatically defaulted to the same value. I'm pretty sure I
>>> recall Rob asking me to drop one of the two in some bindings. Has the
>>> rule changes ? Is it documented somewhere ?
>>
>> New dtschema changed it and, even if previous behavior is restored, the
>> size in if:then: always had to be constrained. You could have skipped
>> one side of limit if it was equal to outer/top-level limit, e.g:
>>
>> properties:
>>   clocks:
>>     minItems: 1
>>     maxItems: 2
>>
>>
>> if:then:properties:
>>   clocks:
>>     minItems: 2
> 
> Where can I find a description of the behaviour of the new dtschema
> (hopefully with some documentation) ?

No clue, but I feel there is some core concept missing. Your earlier
statement:
"I thought that, when only one of minItems or maxItems was specified, the"

was never logically correct for the "if:then", except for the case I
mentioned above. That's why all schema used as examples had it explicit:

My talk from 2022, page 30:
https://static.sched.com/hosted_files/osseu2022/bd/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro.pdf?_gl=1*kmzqmt*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1
all constraints defined,.

My talk from 2023, page 34:
https://static.sched.com/hosted_files/eoss2023/a8/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro%20-%20ELCE%202023.pdf?_gl=1*1jgx6d3*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1

Recently, I started using other example as "useful reference":
https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132

That's nothing. All three above reference examples I keep giving are
already there and repeated in emails all the time.

So aren't you confusing the entire "skip one limit" for top-level
properties? This patch is not about it all and dtschema did not change.

Best regards,
Krzysztof


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

* Re: [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints
  2024-08-18 17:30 ` [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints Krzysztof Kozlowski
  2024-08-18 17:46   ` Laurent Pinchart
@ 2024-08-19 17:11   ` Conor Dooley
  1 sibling, 0 replies; 12+ messages in thread
From: Conor Dooley @ 2024-08-19 17:11 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Laurent Pinchart, Kieran Bingham, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

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

On Sun, Aug 18, 2024 at 07:30:03PM +0200, Krzysztof Kozlowski wrote:
> Properties with variable number of items per each device are expected to
> have widest constraints in top-level "properties:" block and further
> customized (narrowed) in "if:then:".  Add missing top-level constraints
> for clocks, clock-names, interrupts, resets, reset-names, renesas,cmms
> and renesas,vsps.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

> ---
>  .../bindings/display/renesas,du.yaml          | 26 ++++++++++++++++---
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml b/Documentation/devicetree/bindings/display/renesas,du.yaml
> index 147842b6465a..9a2d1c08cb1f 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.yaml
> +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml
> @@ -46,12 +46,26 @@ properties:
>      maxItems: 1
>  
>    # See compatible-specific constraints below.
> -  clocks: true
> -  clock-names: true
> +  clocks:
> +    minItems: 1
> +    maxItems: 8
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 8
> +
>    interrupts:
> +    minItems: 1
> +    maxItems: 4
>      description: Interrupt specifiers, one per DU channel
> -  resets: true
> -  reset-names: true
> +
> +  resets:
> +    minItems: 1
> +    maxItems: 2
> +
> +  reset-names:
> +    minItems: 1
> +    maxItems: 2
>  
>    power-domains:
>      maxItems: 1
> @@ -77,6 +91,8 @@ properties:
>  
>    renesas,cmms:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 2
> +    maxItems: 4
>      items:
>        maxItems: 1
>      description:
> @@ -85,6 +101,8 @@ properties:
>  
>    renesas,vsps:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 1
> +    maxItems: 4
>      items:
>        items:
>          - description: phandle to VSP instance that serves the DU channel
> -- 
> 2.43.0
> 

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

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

* Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
  2024-08-18 17:30 [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants Krzysztof Kozlowski
  2024-08-18 17:30 ` [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints Krzysztof Kozlowski
  2024-08-18 17:41 ` [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants Laurent Pinchart
@ 2024-08-19 17:12 ` Conor Dooley
  2 siblings, 0 replies; 12+ messages in thread
From: Conor Dooley @ 2024-08-19 17:12 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Laurent Pinchart, Kieran Bingham, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

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

On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote:
> Each variable-length property like interrupts or resets must have fixed
> constraints on number of items for given variant in binding.  The
> clauses in "if:then:" block should define both limits: upper and lower.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

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

* Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
  2024-08-18 18:48       ` Krzysztof Kozlowski
@ 2024-08-28 12:45         ` Laurent Pinchart
  2024-08-28 12:52           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2024-08-28 12:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

Hi Krzysztof,

On Sun, Aug 18, 2024 at 08:48:54PM +0200, Krzysztof Kozlowski wrote:
> On 18/08/2024 19:51, Laurent Pinchart wrote:
> > On Sun, Aug 18, 2024 at 07:44:22PM +0200, Krzysztof Kozlowski wrote:
> >> On 18/08/2024 19:41, Laurent Pinchart wrote:
> >>> On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote:
> >>>> Each variable-length property like interrupts or resets must have fixed
> >>>> constraints on number of items for given variant in binding.  The
> >>>> clauses in "if:then:" block should define both limits: upper and lower.
> >>>
> >>> I thought that, when only one of minItems or maxItems was specified, the
> >>> other automatically defaulted to the same value. I'm pretty sure I
> >>> recall Rob asking me to drop one of the two in some bindings. Has the
> >>> rule changes ? Is it documented somewhere ?
> >>
> >> New dtschema changed it and, even if previous behavior is restored, the
> >> size in if:then: always had to be constrained. You could have skipped
> >> one side of limit if it was equal to outer/top-level limit, e.g:
> >>
> >> properties:
> >>   clocks:
> >>     minItems: 1
> >>     maxItems: 2
> >>
> >>
> >> if:then:properties:
> >>   clocks:
> >>     minItems: 2
> > 
> > Where can I find a description of the behaviour of the new dtschema
> > (hopefully with some documentation) ?
> 
> No clue, but I feel there is some core concept missing. Your earlier
> statement:
> "I thought that, when only one of minItems or maxItems was specified, the"
> 
> was never logically correct for the "if:then", except for the case I
> mentioned above. That's why all schema used as examples had it explicit:
> 
> My talk from 2022, page 30:
> https://static.sched.com/hosted_files/osseu2022/bd/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro.pdf?_gl=1*kmzqmt*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1
> all constraints defined,.
> 
> My talk from 2023, page 34:
> https://static.sched.com/hosted_files/eoss2023/a8/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro%20-%20ELCE%202023.pdf?_gl=1*1jgx6d3*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1
> 
> Recently, I started using other example as "useful reference":
> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132
> 
> That's nothing. All three above reference examples I keep giving are
> already there and repeated in emails all the time.
> 
> So aren't you confusing the entire "skip one limit" for top-level
> properties? This patch is not about it all and dtschema did not change.

There must have been a misunderstanding indeed, I interpreted "New
dtschema changed it" as meaning there were now new rules. Is that
incorrect ?

If you don't mind clarifying, what is the current recommendation to
indicate that a property has a fixed number of items ? Which of the
following three options is preferred ?

properties:
  clocks:
    minItems: 2

properties:
  clocks:
    maxItems: 2

properties:
  clocks:
    minItems: 2
    maxItems: 2

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
  2024-08-28 12:45         ` Laurent Pinchart
@ 2024-08-28 12:52           ` Krzysztof Kozlowski
  2024-08-28 13:29             ` Laurent Pinchart
  0 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-28 12:52 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

On 28/08/2024 14:45, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> On Sun, Aug 18, 2024 at 08:48:54PM +0200, Krzysztof Kozlowski wrote:
>> On 18/08/2024 19:51, Laurent Pinchart wrote:
>>> On Sun, Aug 18, 2024 at 07:44:22PM +0200, Krzysztof Kozlowski wrote:
>>>> On 18/08/2024 19:41, Laurent Pinchart wrote:
>>>>> On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote:
>>>>>> Each variable-length property like interrupts or resets must have fixed
>>>>>> constraints on number of items for given variant in binding.  The
>>>>>> clauses in "if:then:" block should define both limits: upper and lower.
>>>>>
>>>>> I thought that, when only one of minItems or maxItems was specified, the
>>>>> other automatically defaulted to the same value. I'm pretty sure I
>>>>> recall Rob asking me to drop one of the two in some bindings. Has the
>>>>> rule changes ? Is it documented somewhere ?
>>>>
>>>> New dtschema changed it and, even if previous behavior is restored, the
>>>> size in if:then: always had to be constrained. You could have skipped
>>>> one side of limit if it was equal to outer/top-level limit, e.g:
>>>>
>>>> properties:
>>>>   clocks:
>>>>     minItems: 1
>>>>     maxItems: 2
>>>>
>>>>
>>>> if:then:properties:
>>>>   clocks:
>>>>     minItems: 2
>>>
>>> Where can I find a description of the behaviour of the new dtschema
>>> (hopefully with some documentation) ?
>>
>> No clue, but I feel there is some core concept missing. Your earlier
>> statement:
>> "I thought that, when only one of minItems or maxItems was specified, the"
>>
>> was never logically correct for the "if:then", except for the case I
>> mentioned above. That's why all schema used as examples had it explicit:
>>
>> My talk from 2022, page 30:
>> https://static.sched.com/hosted_files/osseu2022/bd/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro.pdf?_gl=1*kmzqmt*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1
>> all constraints defined,.
>>
>> My talk from 2023, page 34:
>> https://static.sched.com/hosted_files/eoss2023/a8/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro%20-%20ELCE%202023.pdf?_gl=1*1jgx6d3*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1
>>
>> Recently, I started using other example as "useful reference":
>> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132
>>
>> That's nothing. All three above reference examples I keep giving are
>> already there and repeated in emails all the time.
>>
>> So aren't you confusing the entire "skip one limit" for top-level
>> properties? This patch is not about it all and dtschema did not change.
> 
> There must have been a misunderstanding indeed, I interpreted "New
> dtschema changed it" as meaning there were now new rules. Is that
> incorrect ?

For the binding with a property defined only in top-level properties: no
changes, no new rules.

For the binding with top-level and if:then:else: dtschema since few
months changed interpretation.

> 
> If you don't mind clarifying, what is the current recommendation to
> indicate that a property has a fixed number of items ? Which of the
> following three options is preferred ?
> 

Answer below assumes we have clocks defined in top-level properties and
there is no if:then:else customizing it.

> properties:
>   clocks:
>     minItems: 2

That's wrong, because items are unconstrained.

> 
> properties:
>   clocks:
>     maxItems: 2

This one is preferred.

> 
> properties:
>   clocks:
>     minItems: 2
>     maxItems: 2

This one is correct, but less preferred.


Best regards,
Krzysztof


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

* Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
  2024-08-28 12:52           ` Krzysztof Kozlowski
@ 2024-08-28 13:29             ` Laurent Pinchart
  0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2024-08-28 13:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, dri-devel, linux-renesas-soc, devicetree,
	linux-kernel

On Wed, Aug 28, 2024 at 02:52:25PM +0200, Krzysztof Kozlowski wrote:
> On 28/08/2024 14:45, Laurent Pinchart wrote:
> > On Sun, Aug 18, 2024 at 08:48:54PM +0200, Krzysztof Kozlowski wrote:
> >> On 18/08/2024 19:51, Laurent Pinchart wrote:
> >>> On Sun, Aug 18, 2024 at 07:44:22PM +0200, Krzysztof Kozlowski wrote:
> >>>> On 18/08/2024 19:41, Laurent Pinchart wrote:
> >>>>> On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote:
> >>>>>> Each variable-length property like interrupts or resets must have fixed
> >>>>>> constraints on number of items for given variant in binding.  The
> >>>>>> clauses in "if:then:" block should define both limits: upper and lower.
> >>>>>
> >>>>> I thought that, when only one of minItems or maxItems was specified, the
> >>>>> other automatically defaulted to the same value. I'm pretty sure I
> >>>>> recall Rob asking me to drop one of the two in some bindings. Has the
> >>>>> rule changes ? Is it documented somewhere ?
> >>>>
> >>>> New dtschema changed it and, even if previous behavior is restored, the
> >>>> size in if:then: always had to be constrained. You could have skipped
> >>>> one side of limit if it was equal to outer/top-level limit, e.g:
> >>>>
> >>>> properties:
> >>>>   clocks:
> >>>>     minItems: 1
> >>>>     maxItems: 2
> >>>>
> >>>>
> >>>> if:then:properties:
> >>>>   clocks:
> >>>>     minItems: 2
> >>>
> >>> Where can I find a description of the behaviour of the new dtschema
> >>> (hopefully with some documentation) ?
> >>
> >> No clue, but I feel there is some core concept missing. Your earlier
> >> statement:
> >> "I thought that, when only one of minItems or maxItems was specified, the"
> >>
> >> was never logically correct for the "if:then", except for the case I
> >> mentioned above. That's why all schema used as examples had it explicit:
> >>
> >> My talk from 2022, page 30:
> >> https://static.sched.com/hosted_files/osseu2022/bd/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro.pdf?_gl=1*kmzqmt*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1
> >> all constraints defined,.
> >>
> >> My talk from 2023, page 34:
> >> https://static.sched.com/hosted_files/eoss2023/a8/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro%20-%20ELCE%202023.pdf?_gl=1*1jgx6d3*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1
> >>
> >> Recently, I started using other example as "useful reference":
> >> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132
> >>
> >> That's nothing. All three above reference examples I keep giving are
> >> already there and repeated in emails all the time.
> >>
> >> So aren't you confusing the entire "skip one limit" for top-level
> >> properties? This patch is not about it all and dtschema did not change.
> > 
> > There must have been a misunderstanding indeed, I interpreted "New
> > dtschema changed it" as meaning there were now new rules. Is that
> > incorrect ?
> 
> For the binding with a property defined only in top-level properties: no
> changes, no new rules.
> 
> For the binding with top-level and if:then:else: dtschema since few
> months changed interpretation.

OK, that's what I didn't understand correctly.

> > If you don't mind clarifying, what is the current recommendation to
> > indicate that a property has a fixed number of items ? Which of the
> > following three options is preferred ?
> 
> Answer below assumes we have clocks defined in top-level properties and
> there is no if:then:else customizing it.
> 
> > properties:
> >   clocks:
> >     minItems: 2
> 
> That's wrong, because items are unconstrained.
> 
> > properties:
> >   clocks:
> >     maxItems: 2
> 
> This one is preferred.
> 
> > properties:
> >   clocks:
> >     minItems: 2
> >     maxItems: 2
> 
> This one is correct, but less preferred.

Thank you, that is clear now.

-- 
Regards,

Laurent Pinchart

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

end of thread, other threads:[~2024-08-28 13:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-18 17:30 [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants Krzysztof Kozlowski
2024-08-18 17:30 ` [PATCH 2/2] dt-bindings: display: renesas,du: add top-level constraints Krzysztof Kozlowski
2024-08-18 17:46   ` Laurent Pinchart
2024-08-19 17:11   ` Conor Dooley
2024-08-18 17:41 ` [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants Laurent Pinchart
2024-08-18 17:44   ` Krzysztof Kozlowski
2024-08-18 17:51     ` Laurent Pinchart
2024-08-18 18:48       ` Krzysztof Kozlowski
2024-08-28 12:45         ` Laurent Pinchart
2024-08-28 12:52           ` Krzysztof Kozlowski
2024-08-28 13:29             ` Laurent Pinchart
2024-08-19 17:12 ` Conor Dooley

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