Re: [PATCH 07/17] clk: renesas: rzg2l: Extend power domain support

On 16.02.2024 16:08, Geert Uytterhoeven wrote:

Hi Claudiu,

On Thu, Feb 8, 2024 at 1:44 PM Claudiu [off-list ref] wrote:

quoted

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

RZ/{G2L, V2L, G3S}-based CPG versions have support for saving extra
power when clocks are disabled by activating module standby. This is done
through MSTOP-specific registers that are part of CPG. Each individual
module has one or more bits associated with one MSTOP register (see table
"Registers for Module Standby Mode" from HW manuals). Hardware manual
associates modules' clocks with one or more MSTOP bits. There are 3 mappings
available (identified by researching RZ/G2L, RZ/G3S, RZ/V2L HW manuals):

case 1: N clocks mapped to N MSTOP bits (with N={0, ..., X})
case 2: N clocks mapped to 1 MSTOP bit  (with N={0, ..., X})
case 3: N clocks mapped to M MSTOP bits (with N={0, ..., X}, M={0, ..., Y})

Case 3 has been currently identified on RZ/V2L for the VCPL4 module.

To cover all three cases, the individual platform drivers will provide to
clock driver MSTOP register offset and associated bits in this register
as a bitmask and the clock driver will apply this bitmask to proper
MSTOP register.

Apart from MSTOP support, RZ/G3S can save more power by powering down the
individual IPs (after MSTOP has been set) if proper bits in
CPG_PWRDN_IP{1,2} registers are set.

The MSTOP and IP power down support were implemented through power
domains. Platform-specific clock drivers will register an array of
type struct rzg2l_cpg_pm_domain_init_data, which will be used to
instantiate properly the power domains.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

quoted

--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c

@@ -1559,9 +1556,34 @@ static bool rzg2l_cpg_is_pm_clk(struct rzg2l_cpg_priv *priv,
        return true;
 }

[ ... ]

quoted

@@ -234,6 +246,54 @@ struct rzg2l_reset {
 #define DEF_RST(_id, _off, _bit)       \
        DEF_RST_MON(_id, _off, _bit, -1)

+/**
+ * struct rzg2l_cpg_pm_domain_conf - PM domain configuration data structure
+ * @mstop: MSTOP configuration (MSB = register offset, LSB = bitmask)
+ * @pwrdn: PWRDN configuration (MSB = register offset, LSB = register bit)
+ */
+struct rzg2l_cpg_pm_domain_conf {
+       u32 mstop;
+       u32 pwrdn;

Why not

    u16 mstop_off;
    u16 mstop_mask;
    u16 pwrdn_off;
    u16 pwrdn_mask;

so you can drop the MSTOP*() and PWRDN*() macros below?

I did it like this to align with the already existing approach for this
kind of things available in this driver. I can do it as you proposed.

For the rest of your comments on this patch: I agree and will adjust the
patch in the next version.

Thank you,
Claudiu Beznea