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

Hi Claudiu,

-----Original Message-----
From: claudiu beznea <claudiu.beznea@tuxon.dev>
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:

quoted

Hi Claudiu,

quoted

-----Original Message-----
From: claudiu beznea <claudiu.beznea@tuxon.dev>
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:

quoted

quoted

-----Original Message-----
From: claudiu beznea <claudiu.beznea@tuxon.dev>
Sent: Thursday, February 8, 2024 4:53 PM
To: Biju Das <biju.das.jz@bp.renesas.com>; geert+renesas@glider.be;
mturquette@baylibre.com; sboyd@kernel.org; robh@kernel.org;
krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
magnus.damm@gmail.com; paul.walmsley@sifive.com;
palmer@dabbelt.com; aou@eecs.berkeley.edu
Cc: linux-renesas-soc@vger.kernel.org; linux-clk@vger.kernel.org;
devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
riscv@lists.infradead.org; Claudiu Beznea
[off-list ref]
Subject: Re: [PATCH 01/17] dt-bindings: clock: r9a07g043-cpg: Add
power domain IDs



On 08.02.2024 18:28, Biju Das wrote:

quoted

quoted

-----Original Message-----
From: claudiu beznea <claudiu.beznea@tuxon.dev>
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:

quoted

Hi Claudiu,

Thanks for the patch.

quoted

-----Original Message-----
From: Claudiu <claudiu.beznea@tuxon.dev>
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@bp.renesas.com>

Add power domain IDs for RZ/G2UL (R9A07G043) SoC.

Signed-off-by: Claudiu Beznea
[off-list ref]
---
 include/dt-bindings/clock/r9a07g043-cpg.h | 48
+++++++++++++++++++++++
 1 file changed, 48 insertions(+)

[ ... ]

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+#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.

quoted

quoted

- some HW blocks (e.g. OTFDE_DDR) have no MSTOP bits associated (as
of

my

quoted

quoted

  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.

quoted

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