Re: [RFC PATCH 1/9] dt-bindings: clock: renesas,cpg-mssr: Support RZ/T2H system controllers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `ranges` property is not restricted in the `else` block for non-target SoCs, violating the commit's intent.
- [Medium] Properties explicitly documented as required for the RZ/T2H SoC are not enforced in the schema's `required` list.
- [Medium] The commit message scope contradicts the implementation by silently including the RZ/N2H SoC.
--

commit 05c781ffa6d79749c3fc36af69e7b0775046bc24
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

dt-bindings: clock: renesas,cpg-mssr: Support RZ/T2H system controllers

This commit adds support for system controller child nodes within the
CPG/MSSR address space for the RZ/T2H SoC. It adds #address-cells,
#size-cells, and ranges properties to the CPG/MSSR binding and restricts
them to the target hardware.

> dt-bindings: clock: renesas,cpg-mssr: Support RZ/T2H system controllers
>
> The RZ/T2H SoC features system controller blocks for low-power management
> and access control that reside within the CPG/MSSR address space. To
> properly represent these hardware modules, allow them to be defined as
> child nodes of the CPG/MSSR controller.
>
> Update the CPG/MSSR binding to support child nodes by adding
> system-controller child nodes using a patternProperty, requiring a reg
> property and a renesas,sys-block property to identify the controller
> instance.
>
> Restrict these new properties to the RZ/T2H SoC to ensure existing
> bindings for other Renesas SoCs remain unaffected.

This isn't a bug, but the commit message explicitly claims to restrict
these new properties to the RZ/T2H SoC. However, the schema conditionally
allows these properties for the RZ/N2H SoC as well. Could the commit
message be updated to mention RZ/N2H if it is also intended to use these
properties?

> diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.yaml b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.yaml
> index 655154534c0f1..ec13fcaf763e7 100644
> --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.yaml
> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.yaml
[ ... ]
> +  '#address-cells':
> +    description:
> +      Required for the RZ/T2H SoC to define the address space for child
> +      system-controller nodes.
> +    const: 2
> +
> +  '#size-cells':
> +    description:
> +      Required for the RZ/T2H SoC to define the size of the register
> +      blocks for child system-controller nodes.
> +    const: 2
> +
> +  ranges:
> +    description:
> +      Required for the RZ/T2H SoC to provide 1:1 address translation
> +      from the child system-controller nodes to the CPU address space.

Since these properties are explicitly documented as "Required for the RZ/T2H
SoC", should they be enforced in the schema's required list for the target
SoC? Without this enforcement, incomplete device tree nodes missing these
properties might silently pass validation.

[ ... ]
> @@ -126,6 +161,10 @@ allOf:
>        properties:
>          reg:
>            maxItems: 1
> +        '#address-cells': false
> +        '#size-cells': false

Does this else block also need to include ranges: false?

While #address-cells and #size-cells are correctly disabled for non-target
SoCs, ranges appears to be omitted. This could globally allow ranges for
all other Renesas SoCs, potentially letting invalid device tree
configurations silently pass schema validation on older hardware.

> +      patternProperties:
> +        "^system-controller@[0-9a-f]+$": false
>    - if:
>        not:
>          properties:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511185058.1926869-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=1