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

Hi Geert,

Thank you for the review.

On Mon, Jul 13, 2026 at 4:50 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Thu, 2 Jul 2026 at 14:31, Prabhakar <[email protected]> wrote:
> > From: Lad Prabhakar <[email protected]>
> >
> > The RZ/T2H and RZ/N2H SoCs 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 and RZ/N2H SoCs to ensure
> > existing bindings for other Renesas SoCs remain unaffected.
> >
> > Signed-off-by: Lad Prabhakar <[email protected]>
> > ---
> > v1->v2:
> > - Updated commit message.
>
> Thanks for the update!
>
> > --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.yaml
> > +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.yaml
> > @@ -99,6 +99,41 @@ properties:
> >        the datasheet.
> >      const: 1
> >
> > +  '#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.
> > +
> > +patternProperties:
> > +  "^system-controller@[0-9a-f]+$":
> > +    type: object
> > +    description:
> > +      System controller child nodes representing sub-blocks for
> > +      Low Power Management (LPM) and Access Control.
>
> According to the documentation for the PRCi bit in Section 11.3.1
> ("PRCRN : Non_Safety Area Protect Register"), the low power consumption
> bits are in the xx28_xxxx blocks?
>
Yes the low power consumption (MSTPX) bits are in the xx28_xxxx blocks.

> > +    properties:
> > +      reg:
> > +        maxItems: 1
> > +      renesas,sys-block:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: Index of the system controller instance.
> > +        enum: [0, 1]
>
> I am not so fond of such "index" properties.
> And of subnodes ;-)
>
The intention was to allow consumers to easily specify which sysblock to use.

> > +    required:
> > +      - reg
> > +      - renesas,sys-block
> > +    additionalProperties: false
> > +
> >  required:
> >    - compatible
> >    - reg
> > @@ -126,6 +161,10 @@ allOf:
> >        properties:
> >          reg:
> >            maxItems: 1
> > +        '#address-cells': false
> > +        '#size-cells': false
> > +      patternProperties:
> > +        "^system-controller@[0-9a-f]+$": false
> >    - if:
> >        not:
> >          properties:
>
> As RZ/T2H and RZ/N2H are rather different than the other (mostly
> R-Car) variants, perhaps it makes sense to spin them off to a separate
> bindings document?
>
Ok, I will split this up in the next version.

Cheers,
Prabhakar