Re: [PATCH v10 4/6] dt-bindings: pinctrl: s32g2-siul2: describe GPIO and EIRQ resources

[Conor Dooley <conor@kernel.org>]

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

On Tue, Jun 02, 2026 at 10:01:30AM +0200, Khristine Andreea Barbulescu wrote:
> Extend the S32G2 SIUL2 pinctrl binding to describe the GPIO data and
> external interrupt resources present in the same SIUL2 hardware block.
> 
> Besides the MSCR and IMCR registers used for pin multiplexing and pad
> configuration, SIUL2 also contains PGPDO and PGPDI registers
> for GPIO data and EIRQ registers for external interrupt control.
> 
> Add GPIO controller properties because the SIUL2 block also provides
> GPIO functionality, and gpio-ranges are needed to describe the
> mapping between GPIO lines and pin controller pins.
> 
> Document the interrupt controller properties. The SIUL2 block
> contains EIRQ hardware as part of the same register space. IRQ support
> itself will be added in a follow-up patch series.
> 
> Update the example accordingly to show the complete SIUL2 register
> layout, including the GPIO data and EIRQ register windows.
> 
> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
> ---
>  .../pinctrl/nxp,s32g2-siul2-pinctrl.yaml      | 83 +++++++++++++++++--
>  1 file changed, 78 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> index a24286e4def6..e4cc1a3a795c 100644
> --- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> -# Copyright 2022 NXP
> +# Copyright 2022, 2026 NXP
>  %YAML 1.2
>  ---
>  $id: http://devicetree.org/schemas/pinctrl/nxp,s32g2-siul2-pinctrl.yaml#
> @@ -17,8 +17,10 @@ description: |
>      SIUL2_0 @ 0x4009c000
>      SIUL2_1 @ 0x44010000
>  
> -  Every SIUL2 region has multiple register types, and here only MSCR and
> -  IMCR registers need to be revealed for kernel to configure pinmux.
> +  Every SIUL2 region has multiple register types. MSCR and IMCR registers
> +  need to be revealed for kernel to configure pinmux. PGPDO and PGPDI
> +  registers are used for GPIO output/input operations. EIRQ registers
> +  are used for external interrupt configuration.
>  
>    Please note that some register indexes are reserved in S32G2, such as
>    MSCR102-MSCR111, MSCR123-MSCR143, IMCR84-IMCR118 and IMCR398-IMCR429.
> @@ -29,14 +31,22 @@ properties:
>        - nxp,s32g2-siul2-pinctrl
>  
>    reg:
> +    minItems: 6
>      description: |
> -      A list of MSCR/IMCR register regions to be reserved.
> +      A list of MSCR/IMCR/PGPDO/PGPDI/EIRQ register regions to be reserved.
>        - MSCR (Multiplexed Signal Configuration Register)
>          An MSCR register can configure the associated pin as either a GPIO pin
>          or a function output pin depends on the selected signal source.
>        - IMCR (Input Multiplexed Signal Configuration Register)
>          An IMCR register can configure the associated pin as function input
>          pin depends on the selected signal source.
> +      - PGPDO (Parallel GPIO Pad Data Out Register)
> +        A PGPDO register is used to set the output value of a GPIO pin.
> +      - PGPDI (Parallel GPIO Pad Data In Register)
> +        A PGPDI register is used to read the input value of a GPIO pin.
> +      - EIRQ (External Interrupt Request)
> +        EIRQ registers are used to configure and manage external interrupts.
> +
>      items:
>        - description: MSCR registers group 0 in SIUL2_0
>        - description: MSCR registers group 1 in SIUL2_1
> @@ -44,6 +54,28 @@ properties:
>        - description: IMCR registers group 0 in SIUL2_0
>        - description: IMCR registers group 1 in SIUL2_1
>        - description: IMCR registers group 2 in SIUL2_1
> +      - description: PGPDO registers in SIUL2_0
> +      - description: PGPDI registers in SIUL2_0
> +      - description: PGPDO registers in SIUL2_1
> +      - description: PGPDI registers in SIUL2_1
> +      - description: EIRQ registers in SIUL2_1
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-ranges:
> +    minItems: 1
> +    maxItems: 4
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupts:
> +    maxItems: 1
>  
>  patternProperties:
>    '-pins$':
> @@ -86,10 +118,32 @@ required:
>    - compatible
>    - reg
>  
> +oneOf:
> +  - description: Legacy pinctrl-only node
> +    properties:
> +      reg:
> +        minItems: 6

drop minItems here, since it matches the outer minItems.

> +        maxItems: 6

You're missing having
properties:
   gpio-controller: false
etc here, the condition you have doesn't prevent having them without the
reg properties to support them. E.g. with a diff like this applied:
diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
index e4cc1a3a795c..f0fc6a771f32 100644
--- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
@@ -158,17 +158,7 @@ examples:
               /* IMCR119-IMCR397 registers on siul2_1 */
               <0x44010c1c 0x45c>,
               /* IMCR430-IMCR495 registers on siul2_1 */
-              <0x440110f8 0x108>,
-              /* PGPDO registers on siul2_0 */
-              <0x4009d700 0x10>,
-              /* PGPDI registers on siul2_0 */
-              <0x4009d740 0x10>,
-              /* PGPDO registers on siul2_1 */
-              <0x44011700 0x18>,
-              /* PGPDI registers on siul2_1 */
-              <0x44011740 0x18>,
-              /* EIRQ registers on siul2_1 */
-              <0x44010010 0x34>;
+              <0x440110f8 0x108>;

> +
> +  - description: Pinctrl node with GPIO and external interrupt support
> +    required:
> +      - gpio-controller
> +      - "#gpio-cells"
> +      - gpio-ranges
> +      - interrupt-controller
> +      - "#interrupt-cells"
> +      - interrupts
> +    properties:
> +      reg:
> +        minItems: 11
> +        maxItems: 11

And the same here for maxItems.

Cheers,
Conor.

> +
>  additionalProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
>      pinctrl@4009c240 {
>          compatible = "nxp,s32g2-siul2-pinctrl";
>  
> @@ -104,7 +158,26 @@ examples:
>                /* IMCR119-IMCR397 registers on siul2_1 */
>                <0x44010c1c 0x45c>,
>                /* IMCR430-IMCR495 registers on siul2_1 */
> -              <0x440110f8 0x108>;
> +              <0x440110f8 0x108>,
> +              /* PGPDO registers on siul2_0 */
> +              <0x4009d700 0x10>,
> +              /* PGPDI registers on siul2_0 */
> +              <0x4009d740 0x10>,
> +              /* PGPDO registers on siul2_1 */
> +              <0x44011700 0x18>,
> +              /* PGPDI registers on siul2_1 */
> +              <0x44011740 0x18>,
> +              /* EIRQ registers on siul2_1 */
> +              <0x44010010 0x34>;
> +
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +        gpio-ranges = <&pinctrl 0 0 102>,
> +                      <&pinctrl 112 112 79>;
> +
> +        interrupt-controller;
> +        #interrupt-cells = <2>;
> +        interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
>  
>          llce-can0-pins {
>              llce-can0-grp0 {
> -- 
> 2.34.1
> 
--
pw-bot: changes-requested

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