Re: [PATCH v2 2/2] ASoC: simple-card-utils: support system-clock-id DT property

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The commit introduces asymmetric sysclk configuration by failing to update `clk_id` in the shutdown path, leading to incorrect hardware states and resource leaks.
--

commit fb04cb3ea63c6e1c03c9835d2530dab5d343394b
Author: Sen Wang <sen@ti.com>

ASoC: simple-card-utils: support system-clock-id DT property

This commit adds an optional system-clock-id DT property to the cpu/codec
sub-nodes. It updates simple_util_parse_clk(), simple_util_hw_params(),
and simple_init_dai() to read and use this clock ID instead of hardcoding 0.

> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index e5cb602fd2487..441f68df85220 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c

[ ... ]

> @@ -506,14 +509,16 @@ int simple_util_hw_params(struct snd_pcm_substream *substream,
>  			/* CPU first */
>  			for_each_rtd_cpu_dais(rtd, i, sdai) {
>  				pdai = simple_props_to_dai_cpu(props, i);
> -				ret = snd_soc_dai_set_sysclk(sdai, 0, mclk, pdai->clk_direction);
> +				ret = snd_soc_dai_set_sysclk(sdai, pdai->clk_id,
> +						mclk, pdai->clk_direction);

Does the teardown path in simple_util_shutdown() also need to be updated to
use clk_id?

It currently hardcodes 0 when disabling the clock:

sound/soc/generic/simple-card-utils.c:simple_util_shutdown() {
	...
	for_each_prop_dai_cpu(props, i, dai) {
		struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, i);

		if (props->mclk_fs && !dai->clk_fixed && !snd_soc_dai_active(cpu_dai))
			snd_soc_dai_set_sysclk(cpu_dai, 0, 0, dai->clk_direction);
	...
}

If a device is configured with a non-zero system-clock-id, will the shutdown
path disable the wrong clock, leaving the intended clock running?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515161358.1462453-1-sen@ti.com?part=2