diff mbox series

[5/7] dt-bindings: pinctrl: add compatible for Allwinner A523/T527

Message ID 20241111005750.13071-6-andre.przywara@arm.com
State New
Headers show
Series pinctrl: sunxi: Add Allwinner A523 support | expand

Commit Message

Andre Przywara Nov. 11, 2024, 12:57 a.m. UTC
The A523 contains a pin controller similar to previous SoCs, although
using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
This introduces a new style of binding, where the pinmux values for each
pin group is stored in the new "allwinner,pinmux" property in the DT
node, instead of requiring every driver to store a mapping between the
function names and the required pinmux.

Add the new name to the list of compatible strings, and required it to
have 10 interrupts described. Also add the new pinmux property.

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

Comments

Rob Herring (Arm) Nov. 12, 2024, 3:38 p.m. UTC | #1
On Mon, Nov 11, 2024 at 12:57:48AM +0000, Andre Przywara wrote:
> The A523 contains a pin controller similar to previous SoCs, although
> using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> This introduces a new style of binding, where the pinmux values for each
> pin group is stored in the new "allwinner,pinmux" property in the DT
> node, instead of requiring every driver to store a mapping between the
> function names and the required pinmux.
> 
> Add the new name to the list of compatible strings, and required it to
> have 10 interrupts described. Also add the new pinmux property.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
>  1 file changed, 21 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 4502405703145..6fc18e92e1e94 100644
> --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> @@ -56,6 +56,8 @@ properties:
>        - allwinner,sun50i-h6-r-pinctrl
>        - allwinner,sun50i-h616-pinctrl
>        - allwinner,sun50i-h616-r-pinctrl
> +      - allwinner,sun55i-a523-pinctrl
> +      - allwinner,sun55i-a523-r-pinctrl
>        - allwinner,suniv-f1c100s-pinctrl
>        - nextthing,gr8-pinctrl
>  
> @@ -64,7 +66,7 @@ properties:
>  
>    interrupts:
>      minItems: 1
> -    maxItems: 8
> +    maxItems: 10
>      description:
>        One interrupt per external interrupt bank supported on the
>        controller, sorted by bank number ascending order.
> @@ -119,13 +121,17 @@ patternProperties:
>          $ref: /schemas/types.yaml#/definitions/uint32
>          enum: [10, 20, 30, 40]
>  
> +      allwinner,pinmux:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: pinmux selector for each pin

Please add some constraints and/or description about what goes in the 
array.

> +
>      required:
>        - pins
>        - function
>  
>      additionalProperties: false
>  
> -  "^vcc-p[a-ilm]-supply$":
> +  "^vcc-p[a-klm]-supply$":
>      description:
>        Power supplies for pin banks.
>  
> @@ -156,6 +162,17 @@ allOf:
>          - interrupts
>          - interrupt-controller
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun55i-a523-pinctrl
> +
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 10
> +
>    - if:
>        properties:
>          compatible:
> @@ -166,6 +183,7 @@ allOf:
>        properties:
>          interrupts:
>            minItems: 8
> +          maxItems: 8
>  
>    - if:
>        properties:
> @@ -244,6 +262,7 @@ allOf:
>              - allwinner,sun8i-v3s-pinctrl
>              - allwinner,sun9i-a80-r-pinctrl
>              - allwinner,sun50i-h6-r-pinctrl
> +            - allwinner,sun55i-a523-r-pinctrl
>  
>      then:
>        properties:
> -- 
> 2.46.2
>
Chen-Yu Tsai Nov. 13, 2024, 8:50 a.m. UTC | #2
On Mon, Nov 11, 2024 at 8:58 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> The A523 contains a pin controller similar to previous SoCs, although
> using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> This introduces a new style of binding, where the pinmux values for each
> pin group is stored in the new "allwinner,pinmux" property in the DT
> node, instead of requiring every driver to store a mapping between the
> function names and the required pinmux.
>
> Add the new name to the list of compatible strings, and required it to
> have 10 interrupts described. Also add the new pinmux property.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
>  1 file changed, 21 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 4502405703145..6fc18e92e1e94 100644
> --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> @@ -56,6 +56,8 @@ properties:
>        - allwinner,sun50i-h6-r-pinctrl
>        - allwinner,sun50i-h616-pinctrl
>        - allwinner,sun50i-h616-r-pinctrl
> +      - allwinner,sun55i-a523-pinctrl
> +      - allwinner,sun55i-a523-r-pinctrl
>        - allwinner,suniv-f1c100s-pinctrl
>        - nextthing,gr8-pinctrl
>
> @@ -64,7 +66,7 @@ properties:
>
>    interrupts:
>      minItems: 1
> -    maxItems: 8
> +    maxItems: 10
>      description:
>        One interrupt per external interrupt bank supported on the
>        controller, sorted by bank number ascending order.
> @@ -119,13 +121,17 @@ patternProperties:
>          $ref: /schemas/types.yaml#/definitions/uint32
>          enum: [10, 20, 30, 40]
>
> +      allwinner,pinmux:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: pinmux selector for each pin
> +

Why not just the standard "pinmux" property, as given in
Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml

>      required:
>        - pins
>        - function

This section should be made to apply only to the existing
compatibles? Maybe we could just split the files and have
a clean slate for sun55i?

ChenYu

>      additionalProperties: false
>
> -  "^vcc-p[a-ilm]-supply$":
> +  "^vcc-p[a-klm]-supply$":
>      description:
>        Power supplies for pin banks.
>
> @@ -156,6 +162,17 @@ allOf:
>          - interrupts
>          - interrupt-controller
>
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun55i-a523-pinctrl
> +
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 10
> +
>    - if:
>        properties:
>          compatible:
> @@ -166,6 +183,7 @@ allOf:
>        properties:
>          interrupts:
>            minItems: 8
> +          maxItems: 8
>
>    - if:
>        properties:
> @@ -244,6 +262,7 @@ allOf:
>              - allwinner,sun8i-v3s-pinctrl
>              - allwinner,sun9i-a80-r-pinctrl
>              - allwinner,sun50i-h6-r-pinctrl
> +            - allwinner,sun55i-a523-r-pinctrl
>
>      then:
>        properties:
> --
> 2.46.2
>
Andre Przywara Nov. 20, 2024, 10:12 a.m. UTC | #3
On Wed, 13 Nov 2024 16:50:19 +0800
Chen-Yu Tsai <wens@csie.org> wrote:

Hi Chen-Yu,

sorry for the late reply, I was away for a week.

> On Mon, Nov 11, 2024 at 8:58 AM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > The A523 contains a pin controller similar to previous SoCs, although
> > using 10 GPIO banks (PortB-PortK), all of them being IRQ capable.
> > This introduces a new style of binding, where the pinmux values for each
> > pin group is stored in the new "allwinner,pinmux" property in the DT
> > node, instead of requiring every driver to store a mapping between the
> > function names and the required pinmux.
> >
> > Add the new name to the list of compatible strings, and required it to
> > have 10 interrupts described. Also add the new pinmux property.
> >
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml  | 23 +++++++++++++++++--
> >  1 file changed, 21 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 4502405703145..6fc18e92e1e94 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
> > @@ -56,6 +56,8 @@ properties:
> >        - allwinner,sun50i-h6-r-pinctrl
> >        - allwinner,sun50i-h616-pinctrl
> >        - allwinner,sun50i-h616-r-pinctrl
> > +      - allwinner,sun55i-a523-pinctrl
> > +      - allwinner,sun55i-a523-r-pinctrl
> >        - allwinner,suniv-f1c100s-pinctrl
> >        - nextthing,gr8-pinctrl
> >
> > @@ -64,7 +66,7 @@ properties:
> >
> >    interrupts:
> >      minItems: 1
> > -    maxItems: 8
> > +    maxItems: 10
> >      description:
> >        One interrupt per external interrupt bank supported on the
> >        controller, sorted by bank number ascending order.
> > @@ -119,13 +121,17 @@ patternProperties:
> >          $ref: /schemas/types.yaml#/definitions/uint32
> >          enum: [10, 20, 30, 40]
> >
> > +      allwinner,pinmux:
> > +        $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        description: pinmux selector for each pin
> > +  
> 
> Why not just the standard "pinmux" property, as given in
> Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml

I had it like this in my last post two years ago, but learned from
LinusW [1] that the generic pinmux property has a slightly different
meaning, and abusing it for just the pinmux index values would not match
the generic definition.
We *could* use the generic definition, but then this would include what's
in the "pins" property, like I sketched out in the cover letter, as an
alternative to this approach:
	pinmux = <SUNXI_PIN(PB, 9, 2)>, <SUNXI_PIN(PB, 10, 2)>;
Where the SUNXI_PIN macro would combine the pin number and the pinmux into
one 32-bit cell. See the Apple GPIO DT nodes for an example.
This looks indeed nicer, but requires quite some rewrite of the existing
pinctrl driver, AFAICS.

[1] Previous reply from LinusW:
https://lore.kernel.org/linux-sunxi/CACRpkdbMc-Q6wjgsiddu6-tWC1dt2uFk+4LyerMdgFk2KRGK4w@mail.gmail.com/

> 
> >      required:
> >        - pins
> >        - function  
> 
> This section should be made to apply only to the existing
> compatibles? Maybe we could just split the files and have
> a clean slate for sun55i?

Yeah, I couldn't find a good example how to make it *required* for one
compatible and *not allowed* for all the others. But creating a whole new
file is actually a good idea, as this also avoids adding another case to
the already quite indented if-else cascade.

Cheers,
Andre

> ChenYu
> 
> >      additionalProperties: false
> >
> > -  "^vcc-p[a-ilm]-supply$":
> > +  "^vcc-p[a-klm]-supply$":
> >      description:
> >        Power supplies for pin banks.
> >
> > @@ -156,6 +162,17 @@ allOf:
> >          - interrupts
> >          - interrupt-controller
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - allwinner,sun55i-a523-pinctrl
> > +
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          minItems: 10
> > +
> >    - if:
> >        properties:
> >          compatible:
> > @@ -166,6 +183,7 @@ allOf:
> >        properties:
> >          interrupts:
> >            minItems: 8
> > +          maxItems: 8
> >
> >    - if:
> >        properties:
> > @@ -244,6 +262,7 @@ allOf:
> >              - allwinner,sun8i-v3s-pinctrl
> >              - allwinner,sun9i-a80-r-pinctrl
> >              - allwinner,sun50i-h6-r-pinctrl
> > +            - allwinner,sun55i-a523-r-pinctrl
> >
> >      then:
> >        properties:
> > --
> > 2.46.2
> >
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 4502405703145..6fc18e92e1e94 100644
--- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml
@@ -56,6 +56,8 @@  properties:
       - allwinner,sun50i-h6-r-pinctrl
       - allwinner,sun50i-h616-pinctrl
       - allwinner,sun50i-h616-r-pinctrl
+      - allwinner,sun55i-a523-pinctrl
+      - allwinner,sun55i-a523-r-pinctrl
       - allwinner,suniv-f1c100s-pinctrl
       - nextthing,gr8-pinctrl
 
@@ -64,7 +66,7 @@  properties:
 
   interrupts:
     minItems: 1
-    maxItems: 8
+    maxItems: 10
     description:
       One interrupt per external interrupt bank supported on the
       controller, sorted by bank number ascending order.
@@ -119,13 +121,17 @@  patternProperties:
         $ref: /schemas/types.yaml#/definitions/uint32
         enum: [10, 20, 30, 40]
 
+      allwinner,pinmux:
+        $ref: /schemas/types.yaml#/definitions/uint32-array
+        description: pinmux selector for each pin
+
     required:
       - pins
       - function
 
     additionalProperties: false
 
-  "^vcc-p[a-ilm]-supply$":
+  "^vcc-p[a-klm]-supply$":
     description:
       Power supplies for pin banks.
 
@@ -156,6 +162,17 @@  allOf:
         - interrupts
         - interrupt-controller
 
+  - if:
+      properties:
+        compatible:
+          enum:
+            - allwinner,sun55i-a523-pinctrl
+
+    then:
+      properties:
+        interrupts:
+          minItems: 10
+
   - if:
       properties:
         compatible:
@@ -166,6 +183,7 @@  allOf:
       properties:
         interrupts:
           minItems: 8
+          maxItems: 8
 
   - if:
       properties:
@@ -244,6 +262,7 @@  allOf:
             - allwinner,sun8i-v3s-pinctrl
             - allwinner,sun9i-a80-r-pinctrl
             - allwinner,sun50i-h6-r-pinctrl
+            - allwinner,sun55i-a523-r-pinctrl
 
     then:
       properties: