Re: [PATCH/RFC 10/14] dt-bindings: power: Document Renesas R-Car X5H Module Controller

Hi Marek,

On Thu, 7 May 2026 at 23:36, Marek Vasut <[email protected]> wrote:
> On 5/7/26 9:37 AM, Geert Uytterhoeven wrote:
> > On Thu, 7 May 2026 at 00:58, Marek Vasut <[email protected]> wrote:
> >> On 4/21/26 8:11 PM, Geert Uytterhoeven wrote:
> >>> +  '#power-domain-cells':
> >>> +    description: |
> >>> +      - The first power domain specifier cell must be either the Module
> >>> +        Power Domain Gating (MPDG) register index (0x00-0x3f) from the
> >>> +        datasheet,
> >>
> >> I agree with this part.
> >>
> >>> or a Power Domain number, as defined in
> >>> +        <dt-bindings/power/renesas,r8a78000-mdlc.h>,
> >>
> >> I do not understand this part, please see end of this email ...
> >>
> >>> +      - The second power domain specifier cell must be the module number
> >>> +        (0x00-0xff), composed of the Module System Reset (MSRES) register index
> >>> +        in the high nibble, and the Module Reset Destination bitfield index in
> >>> +        the low nibble.
> >>> +    const: 2
> >>
> >> I am unsure about this part.
> >>
> >> There are multiple MDLC blocks, AON, SCP, HSCN, and so on. Each MDLC
> >> block contains multiple Module Power Domain Gating registers (MPDGn) and
> >> multiple Module System RESet register (MSRES) .
> >>
> >> I do understand and agree that the first power-domains-cells cell must
> >> be the identifier of power domain within the MDLC block.
> >>
> >> However, I do not understand the second cell. The MDLC bindings already
> >> contain reset-cells, which should be used to refer to a reset within the
> >> MDLC block. Resets within the MDLC block are operated using the MSRES
> >> registers. Why are resets conflated into power-domain-cells ?
> >
> > The Module Reset Destination bitfields in the MSRES registers are
> > 2-bit wide, and control both Reset and Module Standby.  Hence the
> > same register bitfields are referred to in the power-domains and
> > resets properties, through the module number.
> >
> > Module Standby controls the clock(s) going into the module,
> > and is modelled as an SCMI clock (SCP_CLOCK_ID_MDLC_*) by the SCP
> > firmware. This is very similar to how MSTP (Module Stop) clocks are
> > handled on earlier R-Car SoCs (except that the SCP_CLOCK_ID_MDLC_*
> > clocks have a zero rate :-(.
> >
> > Summarized, the first cell is the power domain part, and the second
> > cell is the clock domain part.
>
> Thank you for the clarification.
>
> Since there are up to 32 MPDG registers, and 256 resets, can we encode
> both into a single cell ?
>
> (mpdg_register_offset << 16) | (reset_bit_offset << 0)

We could.  I did consider it (with a shift of 8 cfr. 256 modules),
but see below...

> I cannot tell whether this is much better, but it at least ties the PD
> components (power domain and clock domain) into a single value, which
> matches reality a bit better. The current split power domain and clock
> domain description in two cells gives me the illusion that it is
> possible to mix and match power domains and clock domains in DT
> description, but in fact the two cells are strongly tied together.

They are only tied together in the sense that a module (hardware block)
is part of a power domain, and has module standby (clock) control.
Some power domains are backed by MDLC hardware registers,
others are not, hence the need for the additional definitions in
<dt-bindings/power/renesas,r8a78000-mdlc.h>.
I am not aware (yet) of modules that are part of a power domain,
but do not have module standby control. If these exist, we
need an additional definition (R8A78000_MDLC_MODULE_NONE?) in
<dt-bindings/power/renesas,r8a78000-mdlc.h>.

Due to this separation, and due to a possible future need for expansion
(R8A78000_MDLC_MODULE_NONE, MDLCs with more than 256 modules, ...),
I went for two cells.

> If we cannot encode the two into a single cell, maybe we can at least
> have some sort of macro for this, e.g. this (0xff as no MPDG register
> bits for this block):
> #define R8A78000_MDLC_PD_HSCIF0 (0xff << 16) ((0x5 << 4) | (0x3 << 0))
>
> What do you think ?

I (and I believe the DT maintainers) are not so fond of defines for
numbers that can be (more or less) just read from the documentation.
(and 0xff should be R8A78000_MDLC_PD_APL?)

> > So perhaps I will clarify like this:
> >
> >        - The first power domain specifier cell is the power domain part, and
> >          must be either the Module Power Domain Gating (MPDG) register index
>
> ... for power domains which are backed by MDPG bits, and which can be
> controlled in that manner ...

OK.

> >          (0x00-0x3f) from the datasheet, or a Power Domain number, as defined in
> >          <dt-bindings/power/renesas,r8a78000-mdlc.h>,
>
> ... for power domains which are always on, and for which there are no
> MPDG bits which can be used to control them ...

OK,

>
> >        - The second power domain specifier cell is the clock domain part, and

Upon second thought: s/clock domain/module standby/

> >          must be the module number (0x00-0xff), composed of the Module System
> >          Reset (MSRES) register index in the high nibble, and the Module Reset
> >          Destination bitfield index in the low nibble.
>
> I can understand this.
>
> >>> +  '#reset-cells':
> >>> +    description:
> >>> +      The single reset specifier cell must be the module number (0x00-0xff).
> >>> +    const: 1
> >>
> >> [...]
> >>
> >>> +#ifndef __DT_BINDINGS_POWER_RENESAS_R8A78000_MDLC_H__
> >>> +#define __DT_BINDINGS_POWER_RENESAS_R8A78000_MDLC_H__
> >>> +
> >>> +/* R-Car X5H MDLC Power Domains */
> >>> +
> >>> +#define R8A78000_MDLC_PD_AON                 0x40
> >>> +#define R8A78000_MDLC_PD_SCP                 0x41
> >>> +#define R8A78000_MDLC_PD_APL                 0x42
> >>> +#define R8A78000_MDLC_PD_CMN                 0x43
> >>> +#define R8A78000_MDLC_PD_ACL                 0x44
> >> ... what do these numbers represent ? Shouldn't those be register
> >> offsets from MDLC MPDG00 according to power-domain-cells ?
> >
> > These are Power Domains that are not backed by any of the 64 Module
> > Power Domain Gating (MPDG) registers in MDLC blocks.
>
> I suspect that might not be entirely correct for all of them, please
> read on and see CMN below.

Thanks, looks like R8A78000_MDLC_PD_CMN should be dropped.

> Let's take PD_AC00 , AP core 0 , as a domain of interest. My
> understanding is, that the domain structure for PD_AC00 looks as follows:
>
> PD_AON {
>    PD_SCP { };
>    PD_APL {
>      hierarchy is SYSSS
>      always-power-on
>      PD_CMN {
>        hierarchy is CMNN
>        power-gating-bit is MDLC_CMNN 20
>        PD_APU0 {
>          hierarchy is SYSSS
>          power-gating is done by APMU
>          PD_ACL0 {
>            hierarchy is CMNN
>            power-gating-bit is MDLC_CMNN 16
>            PD_AC00 {
>              hierarchy is CMNN
>              power-gating-bit is MDLC_CMNN 0
>            };
>            ...
>          };
>          ...
>        };
>        ...
>      };
>      ...
>      PD_HSCIF0 {
>        hierarchy is PERW
>        power-gating-bit is MDLC_PERW 23
>      };
>    };
>    ...
> };
>
> With this in mind, I think CPU 0 DT node should refer to the PD_AC00
> power domain this way:
>
> cpu@0 {
>    ...
>    power-domains = <&mdlc_cmnn R8A78000_MDLC_PD_AC00>;
>    ...
> };

So we do have a few modules (I found a few more) that are part of
power domains, but do no support module standby.  One more reason to
decouple them in power-domains.

However, CPU cores are controlled through PSCI (the slightly less evil
brother of SCMI? ;-), so
Documentation/devicetree/bindings/arm/psci.yaml applies, too?

>
> The MDLC driver would pass the PD_AC00 domain ID to matching SCMI power
> domain management protocol call, or, for bare-metal MDLC driver, would
> have to internally encode PD hierarchy, walk it, and apply PD operations
> in each step.
>
> I think even for SCIF/HSCIF, the power domain reference should be
> something along the lines of the following description. The MDLC driver
> should internally encode that R8A78000_MLDC_PD_HSCIF0 is a sub-domain of
> R8A78000_MDLC_PD_APL .
>
> serial@c0710000 {
>    ...
>    power-domains = <&mdlc_perw R8A78000_MDLC_PD_HSCIF0>;
>    ...
> };

R8A78000_MLDC_PD_HSCIF0 is a not a full sub-domain, but merely standby
(clock) control inside the PD_APL clock domain?


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds