Re: [PATCH v2 04/11] ASoC: qcom: Add QAIF CIF (CDC DMA) DAI ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] DAI callbacks fetch the DAI ID from the 0-th CPU DAI in the ALSA runtime link instead of using the provided `dai` parameter.
--

commit 32d00a3e9dcf1c7105fbcf435a7ab50e4e85ba7b
Author: Harendra Gautam <harendra.gautam@oss.qualcomm.com>

    ASoC: qcom: Add QAIF CIF (CDC DMA) DAI ops
    
    Add DAI operations for the QAIF Codec Interface (CIF), which connects the
    QAIF DMA engine to the internal Bolero digital codec.

> diff --git a/sound/soc/qcom/qaif-cpu.c b/sound/soc/qcom/qaif-cpu.c
> new file mode 100644
> index 0000000000000..072c1bec078aa
> --- /dev/null
> +++ b/sound/soc/qcom/qaif-cpu.c
> @@ -0,0 +1,321 @@
[ ... ]
> +static struct qaif_cdc_intfctl *qaif_get_cif_intfctl_handle(struct snd_pcm_substream *substream,
> +							    struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *soc_runtime = snd_soc_substream_to_rtd(substream);
> +	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(soc_runtime, 0);
> +	struct qaif_drv_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +	const struct qaif_variant *v = drvdata->variant;
> +	unsigned int dai_id = cpu_dai->driver->id;

[Severity: High]
Does this code fetch the incorrect DAI ID in multi-CPU DAI ALSA topologies?

By using snd_soc_rtd_to_cpu(soc_runtime, 0), the DAI ID is derived from the
hardcoded index 0 CPU DAI instead of the dai parameter passed to the function.

Could this cause mismatched data lookups and leave subsequent target DAIs
uninitialized?

[ ... ]
> +static int qaif_cif_daiops_hw_params(struct snd_pcm_substream *substream,
> +				     struct snd_pcm_hw_params *params,
> +				     struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *soc_runtime = snd_soc_substream_to_rtd(substream);
> +	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(soc_runtime, 0);
> +	struct qaif_drv_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +	const struct qaif_variant *v = drvdata->variant;
> +	struct qaif_cdc_intfctl *intfctl = NULL;
> +	unsigned int dai_id = cpu_dai->driver->id;

[Severity: High]
Will deriving dai_id from the hardcoded index 0 CPU DAI configure the wrong
DMA channel for subsequent DAIs during hw_params() callbacks?

[ ... ]
> +static int qaif_cif_daiops_trigger(struct snd_pcm_substream *substream,
> +				   int cmd,
> +				   struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *soc_runtime = snd_soc_substream_to_rtd(substream);
> +	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(soc_runtime, 0);
> +	struct qaif_drv_data *drvdata = snd_soc_dai_get_drvdata(dai);
> +	const struct qaif_variant *v = drvdata->variant;
> +	unsigned int dai_id = cpu_dai->driver->id;

[Severity: High]
Can fetching dai_id via snd_soc_rtd_to_cpu(soc_runtime, 0) resolve the wrong
index during stream start/stop trigger events?

This could apply stream hardware configurations to the first CPU DAI
repeatedly instead of configuring the current dai, corrupting the DMA state
of the first DAI and leaving others uninitialized.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701105757.2779738-1-harendra.gautam@oss.qualcomm.com?part=4