Re: [PATCH v6 03/16] ASoC: rsnd: Add reset controller support to rsnd_mod

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect hardware teardown sequence in `rsnd_mod_quit()`.
- [Low] Unused `<linux/delay.h>` header inclusion.
--

commit 15ebc4bc5eee087115d0971e0b8a5f59090c9c82
Author: John Madieu <john.madieu.xa@bp.renesas.com>

ASoC: rsnd: Add reset controller support to rsnd_mod

This commit adds reset controller support to the Renesas sound (rsnd) module.
It updates rsnd_mod_init to accept a reset control parameter, deasserts it
during initialization, and mirrors this with an assert in rsnd_mod_quit.

> diff --git a/sound/soc/renesas/rcar/core.c b/sound/soc/renesas/rcar/core.c
> index 2dc078358612d..c3c139a6fc60a 100644
> --- a/sound/soc/renesas/rcar/core.c
> +++ b/sound/soc/renesas/rcar/core.c
> @@ -90,6 +90,7 @@
>   *
>   */
>  
> +#include <linux/delay.h>

This isn't a bug, but is the inclusion of linux/delay.h necessary?

I don't see any delay functions used in the updated code. Was a hardware
stabilization delay intended after the reset_control_deassert() call, or
can this inclusion be safely removed?

[ ... ]
> @@ -217,6 +229,8 @@ void rsnd_mod_quit(struct rsnd_mod *mod)
>  {
>  	clk_unprepare(mod->clk);
>  	mod->clk = NULL;
> +	reset_control_assert(mod->rstc);

Does the hardware require the clock to be running to latch the reset state
change?

In rsnd_mod_init(), the initialization sequence enables the clock before
deasserting the reset. This suggests the IP block needs an active clock
to propagate the reset.

However, in rsnd_mod_quit(), clk_unprepare() is called before
reset_control_assert(). Since the clock is unprepared and not enabled
when the reset is asserted, can the hardware fail to latch the reset state,
or potentially hang the bus when the reset controller accesses a
powered-down domain?

Should the teardown sequence mirror the initialization by temporarily
enabling the clock, asserting the reset, and then disabling and
unpreparing the clock?

Also, a later patch in the series (ASoC: rsnd: Add system suspend/resume
support) appears to introduce rsnd_suspend_clk_reset() which repeats this
exact sequence of an unprepared clock followed by reset assertion.

> +	mod->rstc = NULL;
>  }
>  
>  void rsnd_mod_interrupt(struct rsnd_mod *mod,

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