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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: architecture and commit message mismatch
--

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

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

Remote processors may announce multiple GPIO controllers over an RPMSG
channel. These GPIO controllers may require corresponding device tree
nodes, especially when acting as providers, to supply phandles for their
consumers.

>     Define an RPMSG node to work as a container for a group of RPMSG channels
>     under the imx_rproc node. Each subnode within "rpmsg" represents an
>     individual RPMSG channel. The name of each subnode corresponds to the
>     channel name as defined by the remote processor.
>     
>     All remote devices associated with a given channel are defined as child
>     nodes under the corresponding channel node.

[Severity: Medium]
Does this description match the implemented schema?

The commit message explicitly claims a 3-level device tree hierarchy where the
channel node name strictly matches the remote processor's channel name, and
remote devices are defined as child nodes under the channel.

However, the schema implements a 2-level hierarchy where the remote device is a
direct child of the rpmsg node, uses standard DT unit addresses instead of
RPMSG channel names, and explicitly forbids child nodes.

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml
> new file mode 100644
> index 0000000000000..41eb2e1499429
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-rpmsg.yaml

[ ... ]

> +allOf:
> +  - $ref: /schemas/gpio/gpio.yaml#
> +
> +unevaluatedProperties: false

[Severity: Medium]
Does this explicitly prevent the addition of remote devices as child nodes?

The lack of child node definitions combined with unevaluatedProperties: false
breaks the design described in the commit message where remote devices are
defined as child nodes.

> diff --git a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> index c18f71b648890..5c82255b92a50 100644
> --- a/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml

[ ... ]

> @@ -88,6 +88,29 @@ 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 devices running on the bus.
> +    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: Medium]
Does this enforce GPIO controllers as direct children of the rpmsg node?

By using standard DT unit addresses like gpio@0 directly under the rpmsg
node, this contradicts the claimed 3-level hierarchy and literal
channel-naming convention described in the commit message.

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