Re: [PATCH v7 04/10] ASoC: qcom: Add QAIF CPU DAI ops, regmap, DT parsing and platform init

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] soc: qcom: qaif: stream clocks left enabled during system suspend
--

commit 9b2ed711801920b93b0556d83c037043032dc672
Author: Harendra Gautam <harendra.gautam@oss.qualcomm.com>
Subject: ASoC: qcom: Add QAIF CPU DAI ops, regmap, DT parsing and platform init

This commit adds the QAIF CPU DAI driver to support Qualcomm audio interface
paths. It implements CIF and AIF DAI operations, MMIO regmap setup, DT
parsing, clock management, and platform registration.

> diff --git a/sound/soc/qcom/qaif-cpu.c b/sound/soc/qcom/qaif-cpu.c
> new file mode 100644
> index 0000000000000..e1868dc95cf4f
> --- /dev/null
> +++ b/sound/soc/qcom/qaif-cpu.c
[ ... ]
> +static int qaif_aif_cpu_daiops_startup(struct snd_pcm_substream *substream,
> +				       struct snd_soc_dai *dai)
> +{
> +	struct qaif_drv_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +	const struct qaif_variant *v = drvdata->variant;
> +	int idx, ret;
> +
> +	idx = v->get_dma_idx(dai->driver->id);
> +	if (idx < 0) {
> +		dev_err(dai->dev, "Invalid DMA index: %d\n", idx);
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_prepare_enable(drvdata->aud_dma_clk);
> +	if (ret) {
> +		dev_err(dai->dev, "error enabling aud_dma clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(drvdata->aud_dma_mem_clk);
> +	if (ret) {
> +		dev_err(dai->dev, "error enabling aud_dma_mem clk: %d\n", ret);
> +		clk_disable_unprepare(drvdata->aud_dma_clk);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(drvdata->mi2s_bit_clk[idx]);
> +	if (ret) {
> +		dev_err(dai->dev, "error enabling mi2s bit clk: %d\n", ret);
> +		clk_disable_unprepare(drvdata->aud_dma_mem_clk);
> +		clk_disable_unprepare(drvdata->aud_dma_clk);
> +	}
> +	return ret;
> +}
[ ... ]
> +static int qaif_aif_cpu_daiops_trigger(struct snd_pcm_substream *substream,
> +				       int cmd, struct snd_soc_dai *dai)
> +{
[ ... ]
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		ret = regmap_update_bits(drvdata->audio_qaif_map,
> +					 QAIF_AUD_INTF_CTL_REG(idx), enable_bit, enable_bit);
> +		if (ret)
> +			dev_err(dai->dev, "error setting AIF enable bit: %d\n", ret);
> +		return ret;
> +	case SNDRV_PCM_TRIGGER_STOP:
> +	case SNDRV_PCM_TRIGGER_SUSPEND:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +		ret = regmap_update_bits(drvdata->audio_qaif_map,
> +					 QAIF_AUD_INTF_CTL_REG(idx), enable_bit, 0);
> +		if (ret)
> +			dev_err(dai->dev, "error clearing AIF enable bit: %d\n", ret);
> +		return ret;
[ ... ]
> +static int asoc_qcom_qaif_runtime_suspend(struct device *dev)
> +{
> +	struct qaif_drv_data *drvdata = dev_get_drvdata(dev);
> +
> +	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);

[Severity: Medium]
Will this leave stream-specific clocks enabled during system suspend if an
audio stream is active?

When an audio stream is active, qaif_aif_cpu_daiops_startup() explicitly
prepares and enables aud_dma_clk, aud_dma_mem_clk, and mi2s_bit_clk.

If a user initiates system suspend while the stream is active, ASoC issues
SNDRV_PCM_TRIGGER_SUSPEND to halt the stream but does not call
qaif_aif_cpu_daiops_shutdown(). The qaif_aif_cpu_daiops_trigger() callback
handles the suspend trigger but does not disable these stream-specific clocks.

Furthermore, pm_runtime_force_suspend() calls asoc_qcom_qaif_runtime_suspend(),
which only disables the bulk clocks (drvdata->clks).

Could this result in the stream-specific AP clock votes remaining asserted,
potentially blocking the system power manager (RPM/RPMh) from entering deep
sleep (S3)?

> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-b4-qaif-shikra-audio-v7-0-d5b6f056568e@oss.qualcomm.com?part=4