Re: [PATCH v6 07/10] ASoC: qcom: Add QAIF PCM operations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unchecked DMA channel counts passed to GENMASK() can evaluate to GENMASK(-1, 0), leading to undefined behavior and hardware resource conflicts.
- [High] Hardware initialization state is permanently lost if the QAIF domain is power-collapsed during system suspend.
--

commit 6bb3c2e1288c0727263b66bf59c762a0401fb99b
Author: Harendra Gautam <harendra.gautam@oss.qualcomm.com>
ASoC: qcom: Add QAIF PCM operations

This commit adds PCM platform callbacks for QAIF DAIs to handle playback and
capture. It allocates DMA buffers, performs hardware initialization, and
manages DMA resources across the stream lifecycle.

> diff --git a/sound/soc/qcom/qaif-platform.c b/sound/soc/qcom/qaif-platform.c
> new file mode 100644
> index 0000000000000..84ce3ad9a1f2d
> --- /dev/null
> +++ b/sound/soc/qcom/qaif-platform.c
[ ... ]
> +static int qaif_map_ee_resource(struct qaif_drv_data *drvdata)
> +{
> +	const struct qaif_variant *v = drvdata->variant;
> +	struct regmap *map = drvdata->audio_qaif_map;
> +	int ret;
> +
> +	ret = regmap_write(map, QAIF_EE_RDDMA_MAP_REG(v), GENMASK(v->num_rddma - 1, 0));

[Severity: High]
If a hardware variant has 0 channels for a specific DMA type (for example,
playback-only), will v->num_rddma evaluate to 0? Passing 0 to
GENMASK(v->num_rddma - 1, 0) results in an unsigned underflow and creates an
all-ones bitmask. Could this write corrupt the execution environment mapping
by unintentionally claiming all channels of that type?

This same underflow pattern appears below for num_wrdma, num_codec_rddma,
and num_codec_wrdma.

> +	if (ret)
> +		return ret;
> +	ret = regmap_write(map, QAIF_EE_WRDMA_MAP_REG(v), GENMASK(v->num_wrdma - 1, 0));
> +	if (ret)
> +		return ret;
> +	if (v->num_intf > 0) {
> +		ret = regmap_write(map, QAIF_EE_INTF_MAP_REG(v), GENMASK(v->num_intf - 1, 0));
> +		if (ret)
> +			return ret;
> +	}
> +	ret = regmap_write(map, QAIF_EE_CODEC_RDDMA_MAP_REG(v),
> +			   GENMASK(v->num_codec_rddma - 1, 0));
> +	if (ret)
> +		return ret;
> +	return regmap_write(map, QAIF_EE_CODEC_WRDMA_MAP_REG(v),
> +			    GENMASK(v->num_codec_wrdma - 1, 0));
> +}
[ ... ]
> +static int qaif_init(struct snd_soc_component *component)
> +{
> +	struct qaif_drv_data *drvdata = snd_soc_component_get_drvdata(component);
> +	int ret;
> +
> +	if (drvdata->qaif_hw_configured)
> +		return 0;

[Severity: High]
Is the hardware initialization state permanently lost if the QAIF domain is
power-collapsed during system suspend?

If the QAIF block registers (SHRAM boundaries, EE mappings, and DMA routing)
are reset to zero when the audio power domain collapses during system sleep,
and drvdata->qaif_hw_configured is never cleared by suspend/resume callbacks,
this early return prevents the hardware from being reprogrammed when opening
a new stream after resume.

> +
> +	ret = qaif_config_shram(drvdata);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-b4-qaif-shikra-audio-v6-0-e9addb358201@oss.qualcomm.com?part=7