diff mbox series

[v2,02/21] dt-bindings: pinctrl: Add Allwinner H616 compatible strings

Message ID 20201211011934.6171-3-andre.przywara@arm.com (mailing list archive)
State New, archived
Headers show
Series arm64: sunxi: Initial Allwinner H616 SoC support | expand

Commit Message

Andre Przywara Dec. 11, 2020, 1:19 a.m. UTC
A new SoC, a new compatible string.
Also we were too miserly with just allowing seven interrupt banks.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml   | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Maxime Ripard Dec. 14, 2020, 9:37 a.m. UTC | #1
On Fri, Dec 11, 2020 at 01:19:15AM +0000, Andre Przywara wrote:
> A new SoC, a new compatible string.
> Also we were too miserly with just allowing seven interrupt banks.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml   | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> index 5240487dfe50..292b05d9ed08 100644
> --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> @@ -53,6 +53,8 @@ properties:
>        - allwinner,sun50i-h5-pinctrl
>        - allwinner,sun50i-h6-pinctrl
>        - allwinner,sun50i-h6-r-pinctrl
> +      - allwinner,sun50i-h616-pinctrl
> +      - allwinner,sun50i-h616-r-pinctrl
>        - allwinner,suniv-f1c100s-pinctrl
>        - nextthing,gr8-pinctrl
>  
> @@ -61,7 +63,7 @@ properties:
>  
>    interrupts:
>      minItems: 1
> -    maxItems: 7
> +    maxItems: 8
>      description:
>        One interrupt per external interrupt bank supported on the
>        controller, sorted by bank number ascending order.
> @@ -91,7 +93,7 @@ properties:
>        bank found in the controller
>      $ref: /schemas/types.yaml#/definitions/uint32-array
>      minItems: 1
> -    maxItems: 5
> +    maxItems: 8
>  
>  patternProperties:
>    # It's pretty scary, but the basic idea is that:
> @@ -145,6 +147,18 @@ allOf:
>    # boards are defining it at the moment so it would generate a lot of
>    # warnings.
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun50i-h616-pinctrl
> +
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 8
> +          maxItems: 8
> +

You don't need to have both if they are equals, and in this particular
case we already check that the maximum is 8 so there's no need to repeat
that check here.

Maxime
Andre Przywara Jan. 14, 2021, 12:45 a.m. UTC | #2
On Mon, 14 Dec 2020 10:37:28 +0100
Maxime Ripard <maxime@cerno.tech> wrote:

> On Fri, Dec 11, 2020 at 01:19:15AM +0000, Andre Przywara wrote:
> > A new SoC, a new compatible string.
> > Also we were too miserly with just allowing seven interrupt banks.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml   | 18
> > ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > index 5240487dfe50..292b05d9ed08 100644 ---
> > a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > +++
> > b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > @@ -53,6 +53,8 @@ properties:
> >        - allwinner,sun50i-h5-pinctrl
> >        - allwinner,sun50i-h6-pinctrl
> >        - allwinner,sun50i-h6-r-pinctrl
> > +      - allwinner,sun50i-h616-pinctrl
> > +      - allwinner,sun50i-h616-r-pinctrl
> >        - allwinner,suniv-f1c100s-pinctrl
> >        - nextthing,gr8-pinctrl
> >  
> > @@ -61,7 +63,7 @@ properties:
> >  
> >    interrupts:
> >      minItems: 1
> > -    maxItems: 7
> > +    maxItems: 8
> >      description:
> >        One interrupt per external interrupt bank supported on the
> >        controller, sorted by bank number ascending order.
> > @@ -91,7 +93,7 @@ properties:
> >        bank found in the controller
> >      $ref: /schemas/types.yaml#/definitions/uint32-array
> >      minItems: 1
> > -    maxItems: 5
> > +    maxItems: 8
> >  
> >  patternProperties:
> >    # It's pretty scary, but the basic idea is that:
> > @@ -145,6 +147,18 @@ allOf:
> >    # boards are defining it at the moment so it would generate a
> > lot of # warnings.
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - allwinner,sun50i-h616-pinctrl
> > +
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          minItems: 8
> > +          maxItems: 8
> > +  
> 
> You don't need to have both if they are equals, and in this particular

Mmh, but all the other compatibles have both equal, so what would be
the recommended way to describe this? Just minItems? I don't find a
good explanation at the moment how to handle an explicit number, other
than by enumerating the items explicitly.

> case we already check that the maximum is 8 so there's no need to
> repeat that check here.

Are you referring to the overall "maxItems: 8" above, in the 2nd hunk?
While this will become redundant, this is apparently prone to changes
(as only "7" would be redundant at the moment), so I would rather not
rely on a global limit.

Cheers,
Andre.
Maxime Ripard Jan. 14, 2021, 11:57 a.m. UTC | #3
On Thu, Jan 14, 2021 at 12:45:12AM +0000, Andre Przywara wrote:
> On Mon, 14 Dec 2020 10:37:28 +0100
> Maxime Ripard <maxime@cerno.tech> wrote:
> 
> > On Fri, Dec 11, 2020 at 01:19:15AM +0000, Andre Przywara wrote:
> > > A new SoC, a new compatible string.
> > > Also we were too miserly with just allowing seven interrupt banks.
> > > 
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > ---
> > >  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml   | 18
> > > ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > index 5240487dfe50..292b05d9ed08 100644 ---
> > > a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > +++
> > > b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > > @@ -53,6 +53,8 @@ properties:
> > >        - allwinner,sun50i-h5-pinctrl
> > >        - allwinner,sun50i-h6-pinctrl
> > >        - allwinner,sun50i-h6-r-pinctrl
> > > +      - allwinner,sun50i-h616-pinctrl
> > > +      - allwinner,sun50i-h616-r-pinctrl
> > >        - allwinner,suniv-f1c100s-pinctrl
> > >        - nextthing,gr8-pinctrl
> > >  
> > > @@ -61,7 +63,7 @@ properties:
> > >  
> > >    interrupts:
> > >      minItems: 1
> > > -    maxItems: 7
> > > +    maxItems: 8
> > >      description:
> > >        One interrupt per external interrupt bank supported on the
> > >        controller, sorted by bank number ascending order.
> > > @@ -91,7 +93,7 @@ properties:
> > >        bank found in the controller
> > >      $ref: /schemas/types.yaml#/definitions/uint32-array
> > >      minItems: 1
> > > -    maxItems: 5
> > > +    maxItems: 8
> > >  
> > >  patternProperties:
> > >    # It's pretty scary, but the basic idea is that:
> > > @@ -145,6 +147,18 @@ allOf:
> > >    # boards are defining it at the moment so it would generate a
> > > lot of # warnings.
> > >  
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          enum:
> > > +            - allwinner,sun50i-h616-pinctrl
> > > +
> > > +    then:
> > > +      properties:
> > > +        interrupts:
> > > +          minItems: 8
> > > +          maxItems: 8
> > > +  
> > 
> > You don't need to have both if they are equals, and in this particular
> 
> Mmh, but all the other compatibles have both equal, so what would be
> the recommended way to describe this? Just minItems? I don't find a
> good explanation at the moment how to handle an explicit number, other
> than by enumerating the items explicitly.

This is where the magic happens:
https://github.com/devicetree-org/dt-schema/blob/master/dtschema/lib.py#L258

So, if there's an items property, it will expand minItems and maxItems
according to the length of the list. Else, it will see if there's either
minItems and maxItems and set the other one if it's missing.

In this case, minItems and maxItems are equals, so you could just fill
one of them

> > case we already check that the maximum is 8 so there's no need to
> > repeat that check here.
> 
> Are you referring to the overall "maxItems: 8" above, in the 2nd hunk?
> While this will become redundant, this is apparently prone to changes
> (as only "7" would be redundant at the moment), so I would rather not
> rely on a global limit.

Yeah, my point was that since the upper schema checks for the interrupts
array length to be between 1 and 8, there's no need to specify a max of
8, the upper schema has it covered.

You're right that the max is increased regularly, however we can still
rely on the above logic to fill maxItems to 8 anyway

Maxime
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
index 5240487dfe50..292b05d9ed08 100644
--- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
@@ -53,6 +53,8 @@  properties:
       - allwinner,sun50i-h5-pinctrl
       - allwinner,sun50i-h6-pinctrl
       - allwinner,sun50i-h6-r-pinctrl
+      - allwinner,sun50i-h616-pinctrl
+      - allwinner,sun50i-h616-r-pinctrl
       - allwinner,suniv-f1c100s-pinctrl
       - nextthing,gr8-pinctrl
 
@@ -61,7 +63,7 @@  properties:
 
   interrupts:
     minItems: 1
-    maxItems: 7
+    maxItems: 8
     description:
       One interrupt per external interrupt bank supported on the
       controller, sorted by bank number ascending order.
@@ -91,7 +93,7 @@  properties:
       bank found in the controller
     $ref: /schemas/types.yaml#/definitions/uint32-array
     minItems: 1
-    maxItems: 5
+    maxItems: 8
 
 patternProperties:
   # It's pretty scary, but the basic idea is that:
@@ -145,6 +147,18 @@  allOf:
   # boards are defining it at the moment so it would generate a lot of
   # warnings.
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - allwinner,sun50i-h616-pinctrl
+
+    then:
+      properties:
+        interrupts:
+          minItems: 8
+          maxItems: 8
+
   - if:
       properties:
         compatible: