diff mbox series

dt-bindings: Fix array property constraints

Message ID 20240925232409.2208515-1-robh@kernel.org (mailing list archive)
State New
Delegated to: Rob Herring
Headers show
Series dt-bindings: Fix array property constraints | expand

Commit Message

Rob Herring (Arm) Sept. 25, 2024, 11:24 p.m. UTC
Schemas for array properties should only have 1 level of array
constraints (e.g. items, maxItems, minItems). Sometimes the old
encoding of all properties into a matrix leaked into the schema, and
didn't matter for validation. Now the inner constraints are just
silently ignored as json-schema array keywords are ignored on scalar
values.

Generally, keep the inner constraints and drop the outer "items". With
gicv3 "mbi-alias" property, it is more appropriately a uint32 or uint64
as it is an address and size depends on "#address-cells".

Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 Documentation/devicetree/bindings/cache/l2c2x0.yaml  |  5 ++---
 .../devicetree/bindings/dma/dma-common.yaml          |  7 +++----
 .../bindings/interrupt-controller/arm,gic-v3.yaml    | 12 +++++-------
 .../devicetree/bindings/media/i2c/thine,thp7312.yaml |  3 +--
 .../bindings/memory-controllers/exynos-srom.yaml     |  5 ++---
 .../devicetree/bindings/pci/brcm,stb-pcie.yaml       |  5 ++---
 .../devicetree/bindings/soc/qcom/qcom,smp2p.yaml     |  3 +--
 7 files changed, 16 insertions(+), 24 deletions(-)

Comments

Krzysztof Kozlowski Sept. 26, 2024, 6:16 a.m. UTC | #1
On 26/09/2024 01:24, Rob Herring (Arm) wrote:
> Schemas for array properties should only have 1 level of array
> constraints (e.g. items, maxItems, minItems). Sometimes the old
> encoding of all properties into a matrix leaked into the schema, and
> didn't matter for validation. Now the inner constraints are just
> silently ignored as json-schema array keywords are ignored on scalar
> values.
> 
> Generally, keep the inner constraints and drop the outer "items". With
> gicv3 "mbi-alias" property, it is more appropriately a uint32 or uint64
> as it is an address and size depends on "#address-cells".
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Laurent Pinchart Sept. 26, 2024, 8:36 a.m. UTC | #2
Hi Rob,

Thank you for the patch.

On Wed, Sep 25, 2024 at 06:24:06PM -0500, Rob Herring (Arm) wrote:
> Schemas for array properties should only have 1 level of array
> constraints (e.g. items, maxItems, minItems). Sometimes the old
> encoding of all properties into a matrix leaked into the schema, and
> didn't matter for validation. Now the inner constraints are just
> silently ignored as json-schema array keywords are ignored on scalar
> values.
> 
> Generally, keep the inner constraints and drop the outer "items". With
> gicv3 "mbi-alias" property, it is more appropriately a uint32 or uint64
> as it is an address and size depends on "#address-cells".

I haven't followed what changed in the validation tools, but this is
definitely less confusing. Thanks for improving the experience.

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

> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/cache/l2c2x0.yaml  |  5 ++---
>  .../devicetree/bindings/dma/dma-common.yaml          |  7 +++----
>  .../bindings/interrupt-controller/arm,gic-v3.yaml    | 12 +++++-------
>  .../devicetree/bindings/media/i2c/thine,thp7312.yaml |  3 +--
>  .../bindings/memory-controllers/exynos-srom.yaml     |  5 ++---
>  .../devicetree/bindings/pci/brcm,stb-pcie.yaml       |  5 ++---
>  .../devicetree/bindings/soc/qcom/qcom,smp2p.yaml     |  3 +--
>  7 files changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/cache/l2c2x0.yaml b/Documentation/devicetree/bindings/cache/l2c2x0.yaml
> index d7840a5c4037..10c1a900202f 100644
> --- a/Documentation/devicetree/bindings/cache/l2c2x0.yaml
> +++ b/Documentation/devicetree/bindings/cache/l2c2x0.yaml
> @@ -100,9 +100,8 @@ properties:
>        filter. Addresses in the filter window are directed to the M1 port. Other
>        addresses will go to the M0 port.
>      $ref: /schemas/types.yaml#/definitions/uint32-array
> -    items:
> -      minItems: 2
> -      maxItems: 2
> +    minItems: 2
> +    maxItems: 2
>  
>    arm,io-coherent:
>      description: indicates that the system is operating in an hardware
> diff --git a/Documentation/devicetree/bindings/dma/dma-common.yaml b/Documentation/devicetree/bindings/dma/dma-common.yaml
> index ea700f8ee6c6..fde5160b5d29 100644
> --- a/Documentation/devicetree/bindings/dma/dma-common.yaml
> +++ b/Documentation/devicetree/bindings/dma/dma-common.yaml
> @@ -32,10 +32,9 @@ properties:
>        The first item in the array is for channels 0-31, the second is for
>        channels 32-63, etc.
>      $ref: /schemas/types.yaml#/definitions/uint32-array
> -    items:
> -      minItems: 1
> -      # Should be enough
> -      maxItems: 255
> +    minItems: 1
> +    # Should be enough
> +    maxItems: 255
>  
>    dma-channels:
>      $ref: /schemas/types.yaml#/definitions/uint32
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
> index 5f051c666cbe..f3247a47f9ee 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
> @@ -132,10 +132,9 @@ properties:
>        Address property. Base address of an alias of the GICD region containing
>        only the {SET,CLR}SPI registers to be used if isolation is required,
>        and if supported by the HW.
> -    $ref: /schemas/types.yaml#/definitions/uint32-array
> -    items:
> -      minItems: 1
> -      maxItems: 2
> +    oneOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +      - $ref: /schemas/types.yaml#/definitions/uint64
>  
>    ppi-partitions:
>      type: object
> @@ -223,9 +222,8 @@ patternProperties:
>            (u32, u32) tuple describing the untranslated
>            address and size of the pre-ITS window.
>          $ref: /schemas/types.yaml#/definitions/uint32-array
> -        items:
> -          minItems: 2
> -          maxItems: 2
> +        minItems: 2
> +        maxItems: 2
>  
>      required:
>        - compatible
> diff --git a/Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml b/Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml
> index 535acf2b88a9..bc339a7374b2 100644
> --- a/Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml
> @@ -135,8 +135,7 @@ properties:
>  
>            data-lanes:
>              $ref: /schemas/media/video-interfaces.yaml#/properties/data-lanes
> -            items:
> -              maxItems: 4
> +            maxItems: 4
>              description:
>                This property is for lane reordering between the THP7312 and the imaging
>                sensor that it is connected to.
> diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.yaml b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.yaml
> index 10a2d97e5f8b..a5598ade399f 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.yaml
> @@ -66,9 +66,8 @@ patternProperties:
>  
>        samsung,srom-timing:
>          $ref: /schemas/types.yaml#/definitions/uint32-array
> -        items:
> -          minItems: 6
> -          maxItems: 6
> +        minItems: 6
> +        maxItems: 6
>          description: |
>            Array of 6 integers, specifying bank timings in the following order:
>            Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs.
> diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> index 0925c520195a..2ad1652c2584 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
> @@ -92,9 +92,8 @@ properties:
>        may have two component regions -- base and extended -- so
>        this information cannot be deduced from the dma-ranges.
>      $ref: /schemas/types.yaml#/definitions/uint64-array
> -    items:
> -      minItems: 1
> -      maxItems: 3
> +    minItems: 1
> +    maxItems: 3
>  
>    resets:
>      minItems: 1
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml
> index 141d666dc3f7..1ba1d419e83b 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml
> @@ -55,8 +55,7 @@ properties:
>  
>    qcom,smem:
>      $ref: /schemas/types.yaml#/definitions/uint32-array
> -    items:
> -      maxItems: 2
> +    maxItems: 2
>      description:
>        Two identifiers of the inbound and outbound smem items used for this edge.
>
Conor Dooley Sept. 26, 2024, 4:36 p.m. UTC | #3
On Wed, Sep 25, 2024 at 06:24:06PM -0500, Rob Herring (Arm) wrote:
> Schemas for array properties should only have 1 level of array
> constraints (e.g. items, maxItems, minItems). Sometimes the old
> encoding of all properties into a matrix leaked into the schema, and
> didn't matter for validation. Now the inner constraints are just
> silently ignored as json-schema array keywords are ignored on scalar
> values.
> 
> Generally, keep the inner constraints and drop the outer "items". With
> gicv3 "mbi-alias" property, it is more appropriately a uint32 or uint64
> as it is an address and size depends on "#address-cells".
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Vinod Koul Oct. 22, 2024, 5:33 a.m. UTC | #4
On Wed, 25 Sep 2024 18:24:06 -0500, Rob Herring (Arm) wrote:
> Schemas for array properties should only have 1 level of array
> constraints (e.g. items, maxItems, minItems). Sometimes the old
> encoding of all properties into a matrix leaked into the schema, and
> didn't matter for validation. Now the inner constraints are just
> silently ignored as json-schema array keywords are ignored on scalar
> values.
> 
> [...]

Applied, thanks!

[1/1] dt-bindings: Fix array property constraints
      commit: 72c65390c61fc96cebfb91c300ca774925565383

Best regards,
Rob Herring (Arm) Oct. 22, 2024, 8:46 p.m. UTC | #5
On Tue, Oct 22, 2024 at 12:33 AM Vinod Koul <vkoul@kernel.org> wrote:
>
>
> On Wed, 25 Sep 2024 18:24:06 -0500, Rob Herring (Arm) wrote:
> > Schemas for array properties should only have 1 level of array
> > constraints (e.g. items, maxItems, minItems). Sometimes the old
> > encoding of all properties into a matrix leaked into the schema, and
> > didn't matter for validation. Now the inner constraints are just
> > silently ignored as json-schema array keywords are ignored on scalar
> > values.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] dt-bindings: Fix array property constraints
>       commit: 72c65390c61fc96cebfb91c300ca774925565383

Why? It is already in my tree and not just dma bindings.

Rob
Vinod Koul Oct. 23, 2024, 6:07 a.m. UTC | #6
On 22-10-24, 15:46, Rob Herring wrote:
> On Tue, Oct 22, 2024 at 12:33 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> >
> > On Wed, 25 Sep 2024 18:24:06 -0500, Rob Herring (Arm) wrote:
> > > Schemas for array properties should only have 1 level of array
> > > constraints (e.g. items, maxItems, minItems). Sometimes the old
> > > encoding of all properties into a matrix leaked into the schema, and
> > > didn't matter for validation. Now the inner constraints are just
> > > silently ignored as json-schema array keywords are ignored on scalar
> > > values.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/1] dt-bindings: Fix array property constraints
> >       commit: 72c65390c61fc96cebfb91c300ca774925565383
> 
> Why? It is already in my tree and not just dma bindings.

Dropped now
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/cache/l2c2x0.yaml b/Documentation/devicetree/bindings/cache/l2c2x0.yaml
index d7840a5c4037..10c1a900202f 100644
--- a/Documentation/devicetree/bindings/cache/l2c2x0.yaml
+++ b/Documentation/devicetree/bindings/cache/l2c2x0.yaml
@@ -100,9 +100,8 @@  properties:
       filter. Addresses in the filter window are directed to the M1 port. Other
       addresses will go to the M0 port.
     $ref: /schemas/types.yaml#/definitions/uint32-array
-    items:
-      minItems: 2
-      maxItems: 2
+    minItems: 2
+    maxItems: 2
 
   arm,io-coherent:
     description: indicates that the system is operating in an hardware
diff --git a/Documentation/devicetree/bindings/dma/dma-common.yaml b/Documentation/devicetree/bindings/dma/dma-common.yaml
index ea700f8ee6c6..fde5160b5d29 100644
--- a/Documentation/devicetree/bindings/dma/dma-common.yaml
+++ b/Documentation/devicetree/bindings/dma/dma-common.yaml
@@ -32,10 +32,9 @@  properties:
       The first item in the array is for channels 0-31, the second is for
       channels 32-63, etc.
     $ref: /schemas/types.yaml#/definitions/uint32-array
-    items:
-      minItems: 1
-      # Should be enough
-      maxItems: 255
+    minItems: 1
+    # Should be enough
+    maxItems: 255
 
   dma-channels:
     $ref: /schemas/types.yaml#/definitions/uint32
diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
index 5f051c666cbe..f3247a47f9ee 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
@@ -132,10 +132,9 @@  properties:
       Address property. Base address of an alias of the GICD region containing
       only the {SET,CLR}SPI registers to be used if isolation is required,
       and if supported by the HW.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-    items:
-      minItems: 1
-      maxItems: 2
+    oneOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - $ref: /schemas/types.yaml#/definitions/uint64
 
   ppi-partitions:
     type: object
@@ -223,9 +222,8 @@  patternProperties:
           (u32, u32) tuple describing the untranslated
           address and size of the pre-ITS window.
         $ref: /schemas/types.yaml#/definitions/uint32-array
-        items:
-          minItems: 2
-          maxItems: 2
+        minItems: 2
+        maxItems: 2
 
     required:
       - compatible
diff --git a/Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml b/Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml
index 535acf2b88a9..bc339a7374b2 100644
--- a/Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml
@@ -135,8 +135,7 @@  properties:
 
           data-lanes:
             $ref: /schemas/media/video-interfaces.yaml#/properties/data-lanes
-            items:
-              maxItems: 4
+            maxItems: 4
             description:
               This property is for lane reordering between the THP7312 and the imaging
               sensor that it is connected to.
diff --git a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.yaml b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.yaml
index 10a2d97e5f8b..a5598ade399f 100644
--- a/Documentation/devicetree/bindings/memory-controllers/exynos-srom.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/exynos-srom.yaml
@@ -66,9 +66,8 @@  patternProperties:
 
       samsung,srom-timing:
         $ref: /schemas/types.yaml#/definitions/uint32-array
-        items:
-          minItems: 6
-          maxItems: 6
+        minItems: 6
+        maxItems: 6
         description: |
           Array of 6 integers, specifying bank timings in the following order:
           Tacp, Tcah, Tcoh, Tacc, Tcos, Tacs.
diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
index 0925c520195a..2ad1652c2584 100644
--- a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -92,9 +92,8 @@  properties:
       may have two component regions -- base and extended -- so
       this information cannot be deduced from the dma-ranges.
     $ref: /schemas/types.yaml#/definitions/uint64-array
-    items:
-      minItems: 1
-      maxItems: 3
+    minItems: 1
+    maxItems: 3
 
   resets:
     minItems: 1
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml
index 141d666dc3f7..1ba1d419e83b 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,smp2p.yaml
@@ -55,8 +55,7 @@  properties:
 
   qcom,smem:
     $ref: /schemas/types.yaml#/definitions/uint32-array
-    items:
-      maxItems: 2
+    maxItems: 2
     description:
       Two identifiers of the inbound and outbound smem items used for this edge.