[PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs and critical resets for s2idle

[PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs and critical resets for s2idle

[Claudiu Beznea]

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

When the system suspends to s2idle, any of the IP blocks may be in the
wakeup path. For a device in the wakeup path, the generic PM domain (genpd)
does not disable/enable its clocks across suspend/resume. Re-initializing
the MSTOP bits for such a device leads to synchronous aborts when its
registers are accessed for the first time after resume.

In a suspend mode that is not controlled by firmware, power to the SoC is
not lost, so the MSTOP and critical reset state is retained and does not
need re-initialization in the first place. Skip it when
pm_suspend_via_firmware() reports such a mode.

Fixes: 5cd33db56956 ("clk: renesas: rzg2l: Add support for MSTOP in clock enable/disable API")
Cc: stable@vger.kernel.org
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 drivers/clk/renesas/rzg2l-cpg.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c
index f6293c968bcf..3d2281456036 100644
--- a/drivers/clk/renesas/rzg2l-cpg.c
+++ b/drivers/clk/renesas/rzg2l-cpg.c
@@ -31,6 +31,7 @@
 #include <linux/reset-controller.h>
 #include <linux/slab.h>
 #include <linux/string_choices.h>
+#include <linux/suspend.h>
 #include <linux/units.h>
 
 #include <dt-bindings/clock/renesas-cpg-mssr.h>
@@ -2697,6 +2698,9 @@ static int rzg2l_cpg_resume(struct device *dev)
 	struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
 	int ret;
 
+	if (!pm_suspend_via_firmware())
+		return 0;
+
 	ret = rzg2l_cpg_deassert_crit_resets(&priv->rcdev, priv->info);
 	if (ret)
 		return ret;
-- 
2.43.0

RE: [PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs and critical resets for s2idle

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@kernel.org>
> Sent: 19 October 2026 13:25
> Subject: [PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs and critical resets for s2idle
> 
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> When the system suspends to s2idle, any of the IP blocks may be in the wakeup path. For a device in the
> wakeup path, the generic PM domain (genpd) does not disable/enable its clocks across suspend/resume. Re-
> initializing the MSTOP bits for such a device leads to synchronous aborts when its registers are accessed
> for the first time after resume.
> 
> In a suspend mode that is not controlled by firmware, power to the SoC is not lost, so the MSTOP and
> critical reset state is retained and does not need re-initialization in the first place. Skip it when
> pm_suspend_via_firmware() reports such a mode.
> 
> Fixes: 5cd33db56956 ("clk: renesas: rzg2l: Add support for MSTOP in clock enable/disable API")
> Cc: stable@vger.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>  drivers/clk/renesas/rzg2l-cpg.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index
> f6293c968bcf..3d2281456036 100644
> --- a/drivers/clk/renesas/rzg2l-cpg.c
> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> @@ -31,6 +31,7 @@
>  #include <linux/reset-controller.h>
>  #include <linux/slab.h>
>  #include <linux/string_choices.h>
> +#include <linux/suspend.h>
>  #include <linux/units.h>
> 
>  #include <dt-bindings/clock/renesas-cpg-mssr.h>
> @@ -2697,6 +2698,9 @@ static int rzg2l_cpg_resume(struct device *dev)
>  	struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
>  	int ret;
> 
> +	if (!pm_suspend_via_firmware())
> +		return 0;
> +

Please move this after deassert, otherwise it will lead to SCIF not getting console
prompt during s2idle as reset is asserted during suspend?

Cheers,
Biju

>  	ret = rzg2l_cpg_deassert_crit_resets(&priv->rcdev, priv->info);
>  	if (ret)
>  		return ret;

Re: [PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs and critical resets for s2idle

[Claudiu Beznea]

Hi, Biju,

On 10/19/26 15:36, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Claudiu Beznea <claudiu.beznea@kernel.org>
>> Sent: 19 October 2026 13:25
>> Subject: [PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs and critical resets for s2idle
>>
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> When the system suspends to s2idle, any of the IP blocks may be in the wakeup path. For a device in the
>> wakeup path, the generic PM domain (genpd) does not disable/enable its clocks across suspend/resume. Re-
>> initializing the MSTOP bits for such a device leads to synchronous aborts when its registers are accessed
>> for the first time after resume.
>>
>> In a suspend mode that is not controlled by firmware, power to the SoC is not lost, so the MSTOP and
>> critical reset state is retained and does not need re-initialization in the first place. Skip it when
>> pm_suspend_via_firmware() reports such a mode.
>>
>> Fixes: 5cd33db56956 ("clk: renesas: rzg2l: Add support for MSTOP in clock enable/disable API")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>   drivers/clk/renesas/rzg2l-cpg.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index
>> f6293c968bcf..3d2281456036 100644
>> --- a/drivers/clk/renesas/rzg2l-cpg.c
>> +++ b/drivers/clk/renesas/rzg2l-cpg.c
>> @@ -31,6 +31,7 @@
>>   #include <linux/reset-controller.h>
>>   #include <linux/slab.h>
>>   #include <linux/string_choices.h>
>> +#include <linux/suspend.h>
>>   #include <linux/units.h>
>>
>>   #include <dt-bindings/clock/renesas-cpg-mssr.h>
>> @@ -2697,6 +2698,9 @@ static int rzg2l_cpg_resume(struct device *dev)
>>   	struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
>>   	int ret;
>>
>> +	if (!pm_suspend_via_firmware())
>> +		return 0;
>> +
> 
> Please move this after deassert, otherwise it will lead to SCIF not getting console
> prompt during s2idle as reset is asserted during suspend?
How did you reproduce this?

Who asserts the SCIF reset in s2idle?

Is the s2idle done with the help of firmware in your case?

If you have issues on any of your platforms could you check after applying this 
patch: 
https://lore.kernel.org/all/20261005111157.17256-1-ovidiu.panait.rb@renesas.com/ ?

Thank you,
Claudiu

RE: [PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs and critical resets for s2idle

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@kernel.org>
> Sent: 19 October 2026 14:00
> Subject: Re: [PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs and critical resets for s2idle
> 
> Hi, Biju,
> 
> On 10/19/26 15:36, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Claudiu Beznea <claudiu.beznea@kernel.org>
> >> Sent: 19 October 2026 13:25
> >> Subject: [PATCH] clk: renesas: rzg2l-cpg: Skip re-initializing MSTOPs
> >> and critical resets for s2idle
> >>
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> When the system suspends to s2idle, any of the IP blocks may be in
> >> the wakeup path. For a device in the wakeup path, the generic PM
> >> domain (genpd) does not disable/enable its clocks across
> >> suspend/resume. Re- initializing the MSTOP bits for such a device leads to synchronous aborts when its
> registers are accessed for the first time after resume.
> >>
> >> In a suspend mode that is not controlled by firmware, power to the
> >> SoC is not lost, so the MSTOP and critical reset state is retained
> >> and does not need re-initialization in the first place. Skip it when
> >> pm_suspend_via_firmware() reports such a mode.
> >>
> >> Fixes: 5cd33db56956 ("clk: renesas: rzg2l: Add support for MSTOP in
> >> clock enable/disable API")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >> ---
> >>   drivers/clk/renesas/rzg2l-cpg.c | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/clk/renesas/rzg2l-cpg.c
> >> b/drivers/clk/renesas/rzg2l-cpg.c index
> >> f6293c968bcf..3d2281456036 100644
> >> --- a/drivers/clk/renesas/rzg2l-cpg.c
> >> +++ b/drivers/clk/renesas/rzg2l-cpg.c
> >> @@ -31,6 +31,7 @@
> >>   #include <linux/reset-controller.h>
> >>   #include <linux/slab.h>
> >>   #include <linux/string_choices.h>
> >> +#include <linux/suspend.h>
> >>   #include <linux/units.h>
> >>
> >>   #include <dt-bindings/clock/renesas-cpg-mssr.h>
> >> @@ -2697,6 +2698,9 @@ static int rzg2l_cpg_resume(struct device *dev)
> >>   	struct rzg2l_cpg_priv *priv = dev_get_drvdata(dev);
> >>   	int ret;
> >>
> >> +	if (!pm_suspend_via_firmware())
> >> +		return 0;
> >> +
> >
> > Please move this after deassert, otherwise it will lead to SCIF not
> > getting console prompt during s2idle as reset is asserted during suspend?
> How did you reproduce this?
> 
> Who asserts the SCIF reset in s2idle?

For getting SCIF interrupts, DMA needs to be deasseted. That is the reason
DMA is defined as critical reset. During suspend [1] get asserted,
but assert will be skipped by [2]. So your code looks ok.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/dma/sh/rz-dmac.c?h=next-20261016#n1591

[2]
 https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/clk/renesas/rzg2l-cpg.c?h=next-20261016#n2380

Cheers,
Biju