Re: [PATCH v9 5/7] dt-bindings: pinctrl: s32g2-siul2: describe GPIO and EIRQ resources

[Krzysztof Kozlowski <krzk@kernel.org>]

On Mon, May 04, 2026 at 03:11:46PM +0200, Khristine Andreea Barbulescu wrote:
> Extend the S32G2 SIUL2 pinctrl binding to describe the additional
> resources used by the updated SIUL2 pinctrl driver.

Please describe hardware, not drivers. Statement is not even true -
drivers do not use these resources, unless you organized your patchset
wrong (see submitting bindings documents, both).

Nothing above explains why you need new compatible.

> 
> Besides the MSCR and IMCR register ranges used for pinmux and
> pin configuration, the SIUL2 block also contains PGPDO and PGPDI
> registers for GPIO output and input operations, as well as EIRQ
> registers used for external interrupt configuration.
> 
> Add GPIO controller properties:
>   - gpio-controller
>   - #gpio-cells
>   - gpio-ranges
> 
> Add interrupt controller properties:
>   - interrupt-controller
>   - #interrupt-cells
>   - interrupts

Do not explain what you did, but say why.

> 
> Also update the binding example 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      | 107 ++++++++++++++++--
>  1 file changed, 98 insertions(+), 9 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..0bd341f1e84d 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,26 +17,39 @@ 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.
>  
>  properties:
>    compatible:
> -    enum:
> -      - nxp,s32g2-siul2-pinctrl
> +    oneOf:
> +      - const: nxp,s32g2-siul2-pinctrl
> +      - items:
> +          - const: nxp,s32g2-siul2-pinctrl-gpio
> +          - const: nxp,s32g2-siul2-pinctrl

I don't get how this binding develops. You were asked to grow existing
binding instead of deprecating it and now you did not grow it: you added
completely new compatible.

Please go back to previous version comments.

Best regards,
Krzysztof