RE: [PATCH v7 05/10] clk: renesas: rzg2l-cpg: Re-enable critical module clocks during resume

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Biju <biju.das.au@xxxxxxxxx>
> Sent: 20 March 2026 10:50
> Subject: [PATCH v7 05/10] clk: renesas: rzg2l-cpg: Re-enable critical module clocks during resume
> 
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> After a suspend/resume cycle, critical module clocks (CLK_IS_CRITICAL) may be left disabled as there
> is no owning driver to restore them, unlike regular clocks.
> Add rzg2l_mod_enable_crit_clock_init_mstop() which walks all module clocks on resume, re-enables any
> critical clock found disabled, and then restores the MSTOP state for clocks that have one via the
> existing helper. This replaces the direct call to rzg2l_mod_clock_init_mstop() in rzg2l_cpg_resume(),
> preserving the correct clock-before-MSTOP restore ordering.
> 
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v6->v7:
>  * Updated commit description
>  * RZ/V2M has critical clocks but no mstop, so move the mstop check after
>    enabling critical clocks. After this, we need to restore only mstop for
>    module clocks, so remove the inverted logic and continue statement and
>    directly call rzg2l_mod_clock_init_mstop_helper() if the clock has
>    mstop.
> v5->v6:
>  * Updated commit description
>  * Dropped the list implementation.
>  * Replaced  rzg2l_mod_clock_init_mstop->rzg2l_mod_enable_crit_clock_init_mstop()
>    for enabling critical clks and restoring mstop state during resume.
> v4->v5:
>  * No change
> v4:
>  * Moved this patch from [1] as it is boot-dependent  [1]
> https://lore.kernel.org/all/20260306134228.871815-1-biju.das.jz@xxxxxxxxxxxxxx/
> ---
>  drivers/clk/renesas/rzg2l-cpg.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index
> b68b0312f0e3..038b3f8e85a1 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -1600,6 +1600,21 @@ static void rzg2l_mod_clock_init_mstop_helper(struct rzg2l_cpg_priv *priv,
>  	}
>  }
> 
> +static void rzg2l_mod_enable_crit_clock_init_mstop(struct
> +rzg2l_cpg_priv *priv) {
> +	struct mod_clock *clk;
> +	struct clk_hw *hw;
> +
> +	for_each_mod_clock(clk, hw, priv) {
> +		if ((clk_hw_get_flags(&clk->hw) & CLK_IS_CRITICAL) &&
> +		    (!rzg2l_mod_clock_is_enabled(&clk->hw)))
> +			rzg2l_mod_clock_endisable(&clk->hw, true);

This call will keep increment mstop->usecnt on every str cycle.
So, planning to add same check for normal state like standby state
in rzg2l_mod_clock_module_set_state()[1] at early in the else path. 
Also planning to add helper [2] for code reuse. Are you ok with this
or you have different opinion? Please let me know.

[1]
	criticals = rzg2l_mod_get_critical_clock_count(clock);
      ...
	...
       } else {
+               if (criticals && criticals == atomic_read(&mstop->usecnt))
+                       return;
+
                if (!atomic_read(&mstop->usecnt))
                        update = true;

[2]
+static unsigned int rzg2l_mod_get_critical_clock_count(struct mod_clock *clock)
+{
+       unsigned int criticals = 0;
+
+       for (unsigned int i = 0; i < clock->num_shared_mstop_clks; i++) {
+               struct mod_clock *clk = clock->shared_mstop_clks[i];
+
+               if (clk_hw_get_flags(&clk->hw) & CLK_IS_CRITICAL)
+                       criticals++;
+       }
+
+       if (!clock->num_shared_mstop_clks &&
+           clk_hw_get_flags(&clock->hw) & CLK_IS_CRITICAL)
+               criticals++;
+
+       return criticals;
+}

Cheers,
Biju