RE: [PATCH 17/22] ASoC: rsnd: Add system suspend/resume support

[John Madieu]

Hi Kuninori,

Thanks for the review.

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Sent: Monday, March 23, 2026 2:57 AM
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH 17/22] ASoC: rsnd: Add system suspend/resume support
> 
> 
> Hi John
> 
> > On RZ/G3E and similar SoCs, the audio subsystem loses its state during
> > deep sleep, due to lacking of proper clock and reset management in the
> > PM path.
> >
> > Implement suspend/resume callbacks that save and restore the hardware
> > state by managing clocks and reset controls in the correct order:
> > - Suspend follows reverse probe order
> > - Resume follows probe order
> >
> > Note that module clocks (mod->clk) are left in "prepared but disabled"
> > state after rsnd_mod_init(), so suspend only needs to unprepare them
> > and resume only needs to prepare them.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > ---
> 
> If my memory was correct, besically, all mods (SSI, SCU, etc) will be
> called with SNDRV_PCM_TRIGGER_SUSPEND/RESUME when suspend/resume.
> So they are basically automatically stopped when suspend, and
> automatically started when resume. see rsnd_soc_dai_trigger()
> 
> We need to care about ADG when suspend/resume because it is always ON
> device. At least on R-Car.
> If you need special handling for RZ, you need to care whether it is R-Car
> or RZ, etc.

You're right. The ALSA framework already handles per-module stop/start
through SNDRV_PCM_TRIGGER_SUSPEND/RESUME, which calls each module's
.quit/.init (and thus rsnd_mod_power_off/rsnd_mod_power_on).

So the per-module clk_prepare/clk_unprepare cycle in rsnd_suspend_mod/
rsnd_resume_mod is unnecessary.

What RZ/G3E actually needs beyond the existing ADG handling is:

1- Reset handling for modules that have reset control
2- audmac-pp clock/reset toggle (infrastructure, like ADG)

However, there is no need to make these changes conditionally
based on SoC family as the optional clock/reset APIs are used.
Do you find any issues with my approach ?

Regards,
John

> 
> >  sound/soc/renesas/rcar/core.c | 108
> > +++++++++++++++++++++++++++++++++-
> >  1 file changed, 106 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/renesas/rcar/core.c
> > b/sound/soc/renesas/rcar/core.c index 6a25580b9c6a..eb504551e410
> > 100644
> > --- a/sound/soc/renesas/rcar/core.c
> > +++ b/sound/soc/renesas/rcar/core.c
> > @@ -962,7 +962,8 @@ static int rsnd_soc_hw_rule_channels(struct
> > snd_pcm_hw_params *params,  static const struct snd_pcm_hardware
> rsnd_pcm_hardware = {
> >  	.info =		SNDRV_PCM_INFO_INTERLEAVED	|
> >  			SNDRV_PCM_INFO_MMAP		|
> > -			SNDRV_PCM_INFO_MMAP_VALID,
> > +			SNDRV_PCM_INFO_MMAP_VALID	|
> > +			SNDRV_PCM_INFO_RESUME,
> >  	.buffer_bytes_max	= 64 * 1024,
> >  	.period_bytes_min	= 32,
> >  	.period_bytes_max	= 8192,
> > @@ -2059,11 +2060,70 @@ static void rsnd_remove(struct platform_device
> *pdev)
> >  		remove_func[i](priv);
> >  }
> >
> > +static void rsnd_suspend_mod(struct rsnd_mod *mod) {
> > +	if (!mod)
> > +		return;
> > +
> > +	clk_unprepare(mod->clk);
> > +	reset_control_assert(mod->rstc);
> > +}
> > +
> > +static void rsnd_resume_mod(struct rsnd_mod *mod) {
> > +	if (!mod)
> > +		return;
> > +
> > +	reset_control_deassert(mod->rstc);
> > +	clk_prepare(mod->clk);
> > +}
> > +
> >  static int rsnd_suspend(struct device *dev)  {
> >  	struct rsnd_priv *priv = dev_get_drvdata(dev);
> > +	int i;
> > +
> > +	/*
> > +	 * Reverse order of probe:
> > +	 * ADG -> DVC -> MIX -> CTU -> SRC -> SSIU -> SSI -> DMA
> > +	 */
> >
> > +	/* ADG */
> > +	/* ADG clock disabled via rsnd_adg_clk_disable() -> adg->adg */
> >  	rsnd_adg_clk_disable(priv);
> > +	rsnd_suspend_mod(rsnd_adg_mod_get(priv));
> > +
> > +	/* DVC */
> > +	for (i = priv->dvc_nr - 1; i >= 0; i--)
> > +		rsnd_suspend_mod(rsnd_dvc_mod_get(priv, i));
> > +
> > +	/* MIX */
> > +	for (i = priv->mix_nr - 1; i >= 0; i--)
> > +		rsnd_suspend_mod(rsnd_mix_mod_get(priv, i));
> > +
> > +	/* CTU */
> > +	for (i = priv->ctu_nr - 1; i >= 0; i--)
> > +		rsnd_suspend_mod(rsnd_ctu_mod_get(priv, i));
> > +
> > +	/* SRC */
> > +	for (i = priv->src_nr - 1; i >= 0; i--)
> > +		rsnd_suspend_mod(rsnd_src_mod_get(priv, i));
> > +
> > +	clk_disable_unprepare(priv->clk_scu_x2);
> > +	clk_disable_unprepare(priv->clk_scu);
> > +
> > +	/* SSIU */
> > +	for (i = priv->ssiu_nr - 1; i >= 0; i--)
> > +		rsnd_suspend_mod(rsnd_ssiu_mod_get(priv, i));
> > +
> > +	/* SSI */
> > +	for (i = priv->ssi_nr - 1; i >= 0; i--)
> > +		rsnd_suspend_mod(rsnd_ssi_mod_get(priv, i));
> > +
> > +	/* DMA */
> > +	clk_disable_unprepare(priv->clk_audmac_pp);
> > +	if (priv->rstc_audmac_pp)
> > +		reset_control_assert(priv->rstc_audmac_pp);
> >
> >  	return 0;
> >  }
> > @@ -2071,8 +2131,52 @@ static int rsnd_suspend(struct device *dev)
> > static int rsnd_resume(struct device *dev)  {
> >  	struct rsnd_priv *priv = dev_get_drvdata(dev);
> > +	int i;
> > +
> > +	/*
> > +	 * Same order as probe:
> > +	 * DMA -> SSI -> SSIU -> SRC -> CTU -> MIX -> DVC -> ADG
> > +	 */
> > +
> > +	/* DMA */
> > +	if (priv->rstc_audmac_pp)
> > +		reset_control_deassert(priv->rstc_audmac_pp);
> >
> > -	return rsnd_adg_clk_enable(priv);
> > +	clk_prepare_enable(priv->clk_audmac_pp);
> > +
> > +	/* SSI */
> > +	for (i = 0; i < priv->ssi_nr; i++)
> > +		rsnd_resume_mod(rsnd_ssi_mod_get(priv, i));
> > +
> > +	/* SSIU */
> > +	for (i = 0; i < priv->ssiu_nr; i++)
> > +		rsnd_resume_mod(rsnd_ssiu_mod_get(priv, i));
> > +
> > +	/* SRC */
> > +	clk_prepare_enable(priv->clk_scu);
> > +	clk_prepare_enable(priv->clk_scu_x2);
> > +
> > +	for (i = 0; i < priv->src_nr; i++)
> > +		rsnd_resume_mod(rsnd_src_mod_get(priv, i));
> > +
> > +	/* CTU */
> > +	for (i = 0; i < priv->ctu_nr; i++)
> > +		rsnd_resume_mod(rsnd_ctu_mod_get(priv, i));
> > +
> > +	/* MIX */
> > +	for (i = 0; i < priv->mix_nr; i++)
> > +		rsnd_resume_mod(rsnd_mix_mod_get(priv, i));
> > +
> > +	/* DVC */
> > +	for (i = 0; i < priv->dvc_nr; i++)
> > +		rsnd_resume_mod(rsnd_dvc_mod_get(priv, i));
> > +
> > +	/* ADG */
> > +	rsnd_resume_mod(rsnd_adg_mod_get(priv));
> > +	/* ADG clock enabled via rsnd_adg_clk_enable() -> adg->adg */
> > +	rsnd_adg_clk_enable(priv);
> > +
> > +	return 0;
> >  }
> >
> >  static const struct dev_pm_ops rsnd_pm_ops = {
> > --
> > 2.25.1
> >