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

[Biju Das]

Hi Geert,

> -----Original Message-----
> From: Biju Das
> Sent: 23 March 2026 15:00
> Subject: RE: [PATCH v7 05/10] clk: renesas: rzg2l-cpg: Re-enable critical module clocks during resume
> 
> 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@xxxxxx
> > esas.com/
> > ---
> >  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;
> +}


On second thought

I will introduce a helper rzg2l_mod_clock_endisable_helper(struct clk_hw *hw, bool enable, bool set_mstop_state)
and avoid setting mstop state twice during resume()

The helper will allow to enable only the critical clock without setting mstop state.

Cheers,
Biju