Re: [PATCH v7 18/18] ASoC: rsnd: Add system suspend/resume support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Incorrect clock and reset ordering in `rsnd_suspend_clk_reset` and `rsnd_resume_clk_reset`.
- [Medium] The `scu_supply` clock is omitted from suspend/resume management in the SRC module.
- [Medium] Return values from critical resource management functions are silently ignored during system resume.
--

commit 0690a34131ef80f23f3d83d8daa691b816da9afc
Author: John Madieu <john.madieu.xa@bp.renesas.com>
Date:   Mon May 25 11:02:30 2026 +0000

ASoC: rsnd: Add system suspend/resume support
    
This commit adds system suspend and resume support for the ASoC rsnd driver,
distributing the module-specific suspend/resume operations across individual
files instead of centralizing them in core.c.

> diff --git a/sound/soc/renesas/rcar/core.c b/sound/soc/renesas/rcar/core.c
> index fbf7f72364600..9ce56cd84f46d 100644
> --- a/sound/soc/renesas/rcar/core.c
> +++ b/sound/soc/renesas/rcar/core.c

[ ... ]

> @@ -2159,11 +2160,35 @@ static void rsnd_remove(struct platform_device *pdev)
>  		remove_func[i](priv);
>  }
>  
> +void rsnd_suspend_clk_reset(struct clk *clk, struct reset_control *rstc)
> +{
> +	clk_unprepare(clk);
> +	reset_control_assert(rstc);

[Severity: High]
Will this sequence send the reset command to an unclocked IP block?

Looking at the driver's normal module quit sequence in rsnd_mod_quit(), the
reset is asserted before unpreparing the clock. Could asserting the reset
after unpreparing the clock here cause the IP blocks to hang?

> +}
> +
> +void rsnd_resume_clk_reset(struct clk *clk, struct reset_control *rstc)
> +{
> +	reset_control_deassert(rstc);
> +	clk_prepare(clk);

[Severity: High]
Is it safe to deassert the reset before the clock is prepared?

The hardware typically requires the clock to be stable before deasserting
the reset, which is explicitly noted in rsnd_dma_resume() and done correctly
in rsnd_mod_init(). Will deasserting early here cause issues for the
modules using this helper (SSI, SRC, CTU, MIX, DVC, and ADG)?

[Severity: Medium]
Should the return values of clk_prepare() and reset_control_deassert() be
checked here?

If a clock fails to prepare or a reset fails to deassert, these failures
are silently ignored. Could subsequent register accesses to these IP blocks
trigger a synchronous external abort if they didn't actually resume?

> +}

[ ... ]

> diff --git a/sound/soc/renesas/rcar/dma.c b/sound/soc/renesas/rcar/dma.c
> index 537b71841f8e2..793dd4adbe5c6 100644
> --- a/sound/soc/renesas/rcar/dma.c
> +++ b/sound/soc/renesas/rcar/dma.c

[ ... ]

> @@ -1035,3 +1035,25 @@ int rsnd_dma_probe(struct rsnd_priv *priv)
> +void rsnd_dma_resume(struct rsnd_priv *priv)
> +{
> +	struct rsnd_dma_ctrl *dmac = rsnd_priv_to_dmac(priv);
> +
> +	if (dmac) {
> +		/* Clock must be stable before reset is deasserted */
> +		clk_prepare_enable(dmac->audmapp_clk);

[Severity: Medium]
Is it necessary to check the return value of clk_prepare_enable() here?

If enabling the DMA clock fails, the hardware won't be accessible, and the
error will be silently dropped, potentially leading to panics when the block
is accessed.

> +		rsnd_resume_clk_reset(NULL, dmac->audmapp_rstc);
> +	}
> +}

[ ... ]

> diff --git a/sound/soc/renesas/rcar/src.c b/sound/soc/renesas/rcar/src.c
> index cad15fcc37f22..ac806bdc96d9e 100644
> --- a/sound/soc/renesas/rcar/src.c
> +++ b/sound/soc/renesas/rcar/src.c

[ ... ]

> @@ -834,3 +834,37 @@ void rsnd_src_remove(struct rsnd_priv *priv)
> +void rsnd_src_suspend(struct rsnd_priv *priv)
> +{
> +	struct rsnd_src_ctrl *src_ctrl = rsnd_priv_to_src_ctrl(priv);
> +	struct rsnd_src *src;
> +	int i;
> +
> +	if (!src_ctrl)
> +		return;
> +
> +	for_each_rsnd_src(src, priv, i)
> +		rsnd_suspend_clk_reset(rsnd_mod_get(src)->clk,
> +				       rsnd_mod_get(src)->rstc);
> +
> +	clk_disable_unprepare(src_ctrl->scu_x2);
> +	clk_disable_unprepare(src_ctrl->scu);

[Severity: Medium]
Does this skip disabling the scu_supply clock during suspend?

During rsnd_src_probe(), scu, scu_x2, and scu_supply are all acquired and
enabled. If scu_supply is omitted here, will it remain enabled throughout
system suspend and prevent the clock domain from entering a low-power state?

> +}
> +
> +void rsnd_src_resume(struct rsnd_priv *priv)
> +{
> +	struct rsnd_src_ctrl *src_ctrl = rsnd_priv_to_src_ctrl(priv);
> +	struct rsnd_src *src;
> +	int i;
> +
> +	if (!src_ctrl)
> +		return;
> +
> +	clk_prepare_enable(src_ctrl->scu);
> +	clk_prepare_enable(src_ctrl->scu_x2);

[Severity: Medium]
Should the return values of these clk_prepare_enable() calls be checked?

If the clocks fail to prepare, the error is discarded.

Also, similar to the suspend path, should scu_supply be re-enabled here?

> +
> +	for_each_rsnd_src(src, priv, i)
> +		rsnd_resume_clk_reset(rsnd_mod_get(src)->clk,
> +				      rsnd_mod_get(src)->rstc);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525110230.4014435-1-john.madieu.xa@bp.renesas.com?part=18