Re: [PATCH v6 11/15] ASoC: qcom: sm8250: add support for LPI_MI2S_RX_0 and LPI_MI2S_TX_3

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The machine driver enables sysclk on startup for the new LPI_MI2S DAIs but fails to disable it on shutdown, resulting in a clock and power leak.
- [Medium] Ignored return values from hardware configuration functions `snd_soc_dai_set_sysclk` and `snd_soc_dai_set_fmt`.
--

--- Patch [11]: [PATCH v6 11/15] ASoC: qcom: sm8250: add support for LPI_MI2S_RX_0 and LPI_MI2S_TX_3 ---
commit ed124256581921c49604c5e5d63d1989a91f17c4
Author: Richard Acayan <mailingradian@gmail.com>

ASoC: qcom: sm8250: add support for LPI_MI2S_RX_0 and LPI_MI2S_TX_3

This commit adds support for the LPI_MI2S_RX_0 and LPI_MI2S_TX_3 ports
on SDM660. The new ports are connected to digital and analog WCD codecs
and are initialized using the same logic as the existing ports.

> diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c
> --- a/sound/soc/qcom/sm8250.c
> +++ b/sound/soc/qcom/sm8250.c
[ ... ]
> @@ -112,6 +112,22 @@ static int sm8250_snd_startup(struct snd_pcm_substream *substream)
>  		snd_soc_dai_set_fmt(cpu_dai, fmt);
>  		snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt);
>  		break;
> +	case LPI_MI2S_RX_0:
> +		codec_dai_fmt |= SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_I2S;
> +		snd_soc_dai_set_sysclk(cpu_dai,
> +			Q6AFE_LPASS_CLK_ID_INT0_MI2S_IBIT,
> +			MI2S_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);

[Severity: High]
Does this code leak the DSP clock reference? The sysclk is enabled for the
new LPI_MI2S DAIs on stream startup by passing a positive frequency, but
there does not appear to be a corresponding shutdown handler to set the
frequency back to 0. 

This might leave the clock running and prevent the SoC from entering deeper
sleep states.

> +		snd_soc_dai_set_fmt(cpu_dai, fmt);
> +		snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt);

[Severity: Medium]
Should the return values of snd_soc_dai_set_sysclk() and
snd_soc_dai_set_fmt() be evaluated? 

Hardware initialization errors in sm8250_snd_startup() will be silently
ignored, which could cause cryptic audio playback or capture failures rather
than a clean initialization error.

> +		break;
> +	case LPI_MI2S_TX_3:
> +		codec_dai_fmt |= SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_I2S;
> +		snd_soc_dai_set_sysclk(cpu_dai,
> +			Q6AFE_LPASS_CLK_ID_INT3_MI2S_IBIT,
> +			MI2S_BCLK_RATE, SNDRV_PCM_STREAM_CAPTURE);
> +		snd_soc_dai_set_fmt(cpu_dai, fmt);
> +		snd_soc_dai_set_fmt(codec_dai, codec_dai_fmt);
> +		break;
>  	default:
>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730174353.108023-1-mailingradian@gmail.com?part=11