Re: [PATCH v3 2/2] ASoC: spacemit: add K3 SoC support with additional clocks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unprotected rate changes on shared clocks `c_sysclk` and `c_bclk` can corrupt active audio streams on other I2S controllers.
--

commit f3c6244a03afa7c9321de2747d7f0c3a9bc65bd2
Author: Troy Mitchell <troy.mitchell@linux.spacemit.com>

ASoC: spacemit: add K3 SoC support with additional clocks

This commit adds support for the SpacemiT K3 SoC I2S controller. It acquires
new clocks (sysclk_div, c_sysclk, and c_bclk) using
devm_clk_get_optional_enabled() and configures their rates dynamically based
on the sample rate during stream setup.

> diff --git a/sound/soc/spacemit/k1_i2s.c b/sound/soc/spacemit/k1_i2s.c
> index 5420ca2aefbd8..8871fc15b29cc 100644
> --- a/sound/soc/spacemit/k1_i2s.c
> +++ b/sound/soc/spacemit/k1_i2s.c

[ ... ]

> @@ -206,6 +209,14 @@ static int spacemit_i2s_hw_params(struct snd_pcm_substream *substream,
>  		    params_rate(params) *
>  		    data_bits;
>  
> +	ret = clk_set_rate(i2s->c_sysclk, bclk_rate * 2);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_set_rate(i2s->c_bclk, bclk_rate);
> +	if (ret)
> +		return ret;
> +

[Severity: Medium]
The commit message notes that c_sysclk and c_bclk are shared across multiple
I2S controllers on the K3 SoC. If multiple controllers are concurrently
active and request different sample rates, will calling clk_set_rate() here
silently overwrite the shared clock rates?

Could this instantaneous frequency change disrupt the active audio stream on
the other controllers, causing pitch shifts or FIFO underflows?

Would it be safer to use clk_set_rate_exclusive() to lock the rate while the
stream is active and reject conflicting requests?

>  	ret = clk_set_rate(i2s->bclk, bclk_rate);
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-k3-i2s-v3-0-08e3eb811d93@linux.spacemit.com?part=2