RE: [PATCH 01/17] dt-bindings: clock: r9a07g043-cpg: Add power domain IDs

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@xxxxxxxxx>
> Sent: Monday, February 12, 2024 10:17 AM
> Subject: Re: [PATCH 01/17] dt-bindings: clock: r9a07g043-cpg: Add power
> domain IDs
> 
> 
> 
> On 12.02.2024 10:59, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: claudiu beznea <claudiu.beznea@xxxxxxxxx>
> >> Sent: Monday, February 12, 2024 8:02 AM
> >> Subject: Re: [PATCH 01/17] dt-bindings: clock: r9a07g043-cpg: Add
> >> power domain IDs
> >>
> >> Hi, Biju,
> >>
> >> On 08.02.2024 21:20, Biju Das wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: claudiu beznea <claudiu.beznea@xxxxxxxxx>
> >>>> Sent: Thursday, February 8, 2024 4:53 PM
> >>>> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>; geert+renesas@xxxxxxxxx;
> >>>> mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx; robh@xxxxxxxxxx;
> >>>> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> >>>> magnus.damm@xxxxxxxxx; paul.walmsley@xxxxxxxxxx;
> >>>> palmer@xxxxxxxxxxx; aou@xxxxxxxxxxxxxxxxx
> >>>> Cc: linux-renesas-soc@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx;
> >>>> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> >>>> riscv@xxxxxxxxxxxxxxxxxxx; Claudiu Beznea
> >>>> <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>>> Subject: Re: [PATCH 01/17] dt-bindings: clock: r9a07g043-cpg: Add
> >>>> power domain IDs
> >>>>
> >>>>
> >>>>
> >>>> On 08.02.2024 18:28, Biju Das wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: claudiu beznea <claudiu.beznea@xxxxxxxxx>
> >>>>>> Sent: Thursday, February 8, 2024 3:46 PM
> >>>>>> Subject: Re: [PATCH 01/17] dt-bindings: clock: r9a07g043-cpg: Add
> >>>>>> power domain IDs
> >>>>>>
> >>>>>> Hi, Biju,
> >>>>>>
> >>>>>> On 08.02.2024 16:30, Biju Das wrote:
> >>>>>>> Hi Claudiu,
> >>>>>>>
> >>>>>>> Thanks for the patch.
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Claudiu <claudiu.beznea@xxxxxxxxx>
> >>>>>>>> Sent: Thursday, February 8, 2024 12:43 PM
> >>>>>>>> Subject: [PATCH 01/17] dt-bindings: clock: r9a07g043-cpg: Add
> >>>>>>>> power domain IDs
> >>>>>>>>
> >>>>>>>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>>>>>>>
> >>>>>>>> Add power domain IDs for RZ/G2UL (R9A07G043) SoC.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Claudiu Beznea
> >>>>>>>> <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>>>>>>> ---
> >>>>>>>>  include/dt-bindings/clock/r9a07g043-cpg.h | 48
> >>>>>>>> +++++++++++++++++++++++
> >>>>>>>>  1 file changed, 48 insertions(+)
> [ ... ]
> 
> >>>>>>>> +#define R9A07G043_PD_TSU		46
> >>>>>>>
> >>>>>>> Not sure from "Table 42.3 Registers for Module Standby Mode"
> >>>>>>>
> >>>>>>> Power domain ID has to be based on CPG_BUS_***_MSTOP or
> >>>>>>> CPG_CLKON_*** As former reduces number of IDs??
> >>>>>>
> >>>>>> If I understand correctly your point here, you want me to
> >>>>>> describe PM domain in DT with something like:
> >>>>>>
> >>>>>> power-domains = <&cpg CPG_BUS_X_MSTOP>;
> >>>>>
> >>>>> MSTOP bits are distinct for each IP.
> >>>>>
> >>>>> <&cpg CPG_BUS_MCPU1_MSTOP x>; x =1..9
> >>>>>
> >>>>> 2=MTU IP
> >>>>>
> >>>>> 4= GPT
> >>>>>
> >>>>> etc...
> >>>>>
> >>>>> Is it something work??
> >>>>
> >>>> It might work. But:
> >>>>
> >>>> - you have to consider that some IPs have more than one MSTOP bit,
> >>>> thus, do
> >>>>   we want to uniquely identify these with all MSTOP bits (thus the
> >>>> 2nd cell
> >>>>   being a bitmask) or only one is enough?
> >>>
> >>> We can have an encoding in that case 8:16 24 bit entries
> >>
> >> I consider this complicates the bindings. I don't consider this is
> >> the way going forward. But I may be wrong. I'll let Geert to give his
> >> opinion on it and change it afterwards, if any.
> >>
> >>>
> >>>> - some HW blocks (e.g. OTFDE_DDR) have no MSTOP bits associated (as
> >>>> of
> >> my
> >>>>   current research), so, only PWRDN
> >>>
> >>> Why do we want to add power domain support for DDR?
> >>
> >> To power it up (in case bootloader does any settings in this area)
> >> such that the system will not block while booting.
> >
> > DDR is enabled by TF_A and is not touched by linux, so why are we
> > adding Power domain at all in first place. TZC DDR is not accessible in
> normal world.
> >
> > So if you don't add DDR power domains, linux doesn't know about any
> > thing about and it should work like current case.
> 
> It is related to the way MSTOP and PWRDN hardware features works together.
> PWRDN allows you to save more power by setting IP specific bits in this
> registers after you set the MSTOP.
> 
> OFTDE_DDR and TZCDDR have PWRDN bits dedicated as well as other IPs (e.g.
> serial, ethernet, etc) in CPG_PWRDN_IP2. Setting CPG_PWRDN_MSTOP_ENABLE to
> CPG_PWRDN_MSTOP applies the power down for the IPs specified in
> CPG_PWRDN_{IP1, IP2}.
> 
> It may happen (as in my case) to have a bootloader that sets all the bits
> in CPG_PWRD_{IP1,IP2}.
> 
> If you want to save power for the other IPs listed in CPG_PWRD_{IP1,IP2}
> you have to instantiate power domains for the blocks that you don't want
> to be powered down due to setting CPG_PWRDN_MSTOP_ENABLE to
> CPG_PWRDN_MSTOP, to power them up. Otherwise the system will block when
> setting CPG_PWRDN_MSTOP_ENABLE to CPG_PWRDN_MSTOP (if bootloaders
> previously did some settings in the above specified registers).
> 
> Hope it was clear.

OK got it, Basically you are saying linux PM messes up the CPG_PWRDN_IP2 bits({0,1}
set by TF_A for DDR/TZC DDR and leading to boot failure.

Cheers,
Biju