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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The legacy fallback branch in the `oneOf` block fails to explicitly forbid newly added capability properties (`gpio-controller`, `interrupt-controller`), creating a schema validation loophole.
- [Low] The device tree example will fail `dt_binding_check` compilation due to an unresolved phandle reference to the undeclared label `&pinctrl`.
--

commit 055c2d417b67cea0bcdcafe1444e526610028d07
Author: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>

dt-bindings: pinctrl: s32g2-siul2: describe GPIO and EIRQ resources

This commit extends the S32G2 SIUL2 pinctrl binding to describe GPIO data
and external interrupt resources. It adds GPIO and interrupt controller
properties and expands the reg property ranges. It also provides a oneOf
construct to support legacy and new node layouts.

> diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> index a24286e4def62..e4cc1a3a795c8 100644
> --- a/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32g2-siul2-pinctrl.yaml
[ ... ]
> @@ -86,10 +118,32 @@ required:
>    - compatible
>    - reg
>  
> +oneOf:
> +  - description: Legacy pinctrl-only node
> +    properties:
> +      reg:
> +        minItems: 6
> +        maxItems: 6

[Severity: Medium]
Since gpio-controller and interrupt-controller are allowed at the top level,
does this legacy branch need to explicitly forbid them?

Without setting gpio-controller: false and interrupt-controller: false here,
could a node matching this legacy branch declare these capabilities without
providing the 11 reg items and gpio-ranges required to actually use them,
thereby bypassing schema validation?

> +
> +  - 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
> +
>  additionalProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
>      pinctrl@4009c240 {
>          compatible = "nxp,s32g2-siul2-pinctrl";

[Severity: Low]
Does this node need a pinctrl: label?

The gpio-ranges property added below attempts to reference &pinctrl, which
will cause dt_binding_check to fail with a 'reference to non-existent node
or label' error.

[ ... ]
>                /* 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>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602080132.3256239-1-khristineandreea.barbulescu@oss.nxp.com?part=4