Re: [PATCH v14 2/5] dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `gpio-rpmsg.yaml` binding adds GPIO and interrupt controller provider capabilities but fails to list `gpio-controller` and `interrupt-controller` in its `required` properties array.
- [Medium] The `rpmsg` node schema hardcodes `rpmsg-io` as the only allowed subnode, contradicting the commit message's description of flexible channel names.
- [Low] The `patternProperties` regex used for matching GPIO subnodes in `fsl,imx-rproc.yaml` is missing a start-of-string anchor (`^`).
--

commit e02180a974397446bfafe8bc43641010b7365078
Author: Shenwei Wang <shenwei.wang@nxp.com>

dt-bindings: remoteproc: imx_rproc: Add "rpmsg" subnode support

This commit adds device tree binding support for RPMSG GPIO controllers
exposed by a remote processor. It introduces a generic RPMSG GPIO controller
schema and updates the fsl,imx-rproc binding to support a new rpmsg subnode
container for these channels.

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> new file mode 100644
> index 0000000000000..6c78b6850321c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
[ ... ]
> +  "#interrupt-cells":
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#gpio-cells"
> +  - "#interrupt-cells"

[Severity: Medium]
Should gpio-controller and interrupt-controller be included in this required
list? The binding adds GPIO and interrupt controller provider capabilities,
so omitting them might allow incomplete device tree nodes to pass validation.

> +
> +allOf:
> +  - $ref: /schemas/gpio/gpio.yaml#
> +
> +unevaluatedProperties: false
> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index ce8ec0119469c..aea33205a881a 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
[ ... ]
> @@ -85,6 +85,34 @@ properties:
>        This property is to specify the resource id of the remote processor in SoC
>        which supports SCFW
>  
> +  rpmsg:
> +    type: object
> +    additionalProperties: false
> +    description:
> +      Represents the RPMSG bus between Linux and the remote system. Contains
> +      a group of RPMSG channel devices running on the bus.
> +
> +    properties:
> +      rpmsg-io:
> +        type: object
> +        additionalProperties: false

[Severity: Medium]
The commit message states "The name of each subnode corresponds to the
channel name as defined by the remote processor".

Does hardcoding rpmsg-io as a property and setting additionalProperties to
false unintentionally restrict the schema from allowing flexible channel
names?

> +        properties:
> +          '#address-cells':
> +            const: 1
> +
> +          '#size-cells':
> +            const: 0
> +
> +        patternProperties:
> +          "gpio@[0-9a-f]+$":
> +            type: object
> +            $ref: /schemas/gpio/gpio-rpmsg.yaml#
> +            unevaluatedProperties: false

[Severity: Low]
Does this regex need a start-of-string anchor (^)? Without it, could this
unintentionally match any node name that ends in gpio@[0-9a-f]+?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625155432.815185-1-shenwei.wang@oss.nxp.com?part=2