Re: [PATCH v7 12/18] ASoC: rsnd: Add ADG reset support for RZ/G3E

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The ADG module reset is deasserted before its module clock is requested or enabled, violating hardware sequencing rules.
- [Medium] The newly introduced ADG reset control is never asserted on driver unbind or probe error paths, leaking the hardware state.
--

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

ASoC: rsnd: Add ADG reset support for RZ/G3E

This commit adds support for an optional ADG reset line for the RZ/G3E SoC.
It fetches the reset control in rsnd_adg_probe() and passes it to
rsnd_mod_init() during initialization.

> diff --git a/sound/soc/renesas/rcar/adg.c b/sound/soc/renesas/rcar/adg.c
> index 0105c60a144e0..813ad5eabba61 100644
> --- a/sound/soc/renesas/rcar/adg.c
> +++ b/sound/soc/renesas/rcar/adg.c
[ ... ]
> @@ -779,8 +780,11 @@ int rsnd_adg_probe(struct rsnd_priv *priv)
>  	if (!adg)
>  		return -ENOMEM;
>  
> -	ret = rsnd_mod_init(priv, &adg->mod, &adg_ops,
> -		      NULL, NULL, 0, 0);
> +	rstc = devm_reset_control_get_optional_exclusive(dev, "adg");
> +	if (IS_ERR(rstc))
> +		return dev_err_probe(dev, PTR_ERR(rstc), "failed to get adg reset\n");
> +
> +	ret = rsnd_mod_init(priv, &adg->mod, &adg_ops, NULL, rstc, 0, 0);

[Severity: High]
Does this sequence deassert the ADG module reset before its module clock is
enabled?

When rsnd_mod_init() is called here, the clk argument is passed as NULL.
Inside rsnd_mod_init(), clk_prepare_enable() does nothing for a NULL clock,
and then reset_control_deassert() is executed immediately.

Since the ADG clock is not fetched until the subsequent call to
rsnd_adg_get_clkin() and is only enabled later in rsnd_adg_clk_enable(),
this appears to deassert a synchronous hardware reset without an active
module clock. Can this invalid sequence fail to propagate the reset state
properly or lead to bus lockups?

>  	if (ret)
>  		return ret;
>  

[Severity: Medium]
Are there missing error handling and teardown paths for this new reset?

If rsnd_adg_probe() fails after rsnd_mod_init() completes successfully
(for example, if rsnd_adg_get_clkin() returns an error), the function
returns directly. It appears there is no call to rsnd_mod_quit() to unwind
the state and assert the reset line on this error path.

While the devm framework will free the reset control reference on probe
failure, it does not automatically assert the line, leaving the hardware
permanently out of reset. Is an update to rsnd_adg_remove() also needed to
ensure the reset is properly asserted during driver unbind?

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