Re: [PATCH v7 6/7] ASoC: qcom: common: add DAI-node TDM slot helpers

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent masking of device tree configuration errors due to conflated error codes.
--

--- Patch [6]: [PATCH v7 6/7] ASoC: qcom: common: add DAI-node TDM slot helpers ---
commit 59f92682a3e73f6ecf0f09ddbbaa5f3be6fc0e76
Author: Prasad Kumpatla <[email protected]>

ASoC: qcom: common: add DAI-node TDM slot helpers

Add common helpers to parse standard dai-tdm-slot-* properties from the
CPU and codec child nodes of a backend DAI link and apply the result to
the active DAIs.

> diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
> index f8782e5cfaaea..d231024206dbf 100644
> --- a/sound/soc/qcom/common.c
> +++ b/sound/soc/qcom/common.c

[ ... ]

> +static int qcom_snd_normalize_tdm_slots(struct qcom_snd_tdm_slot_cfg *cpu_cfg,
> +					struct qcom_snd_tdm_slot_cfg *codec_cfg)
> +{
> +	unsigned int slots;
> +	unsigned int slot_width;
> +
> +	if (cpu_cfg->slots && codec_cfg->slots && cpu_cfg->slots != codec_cfg->slots)
> +		return -EINVAL;
> +
> +	if (cpu_cfg->slot_width && codec_cfg->slot_width &&
> +	    cpu_cfg->slot_width != codec_cfg->slot_width)
> +		return -EINVAL;

[ ... ]

> +int qcom_snd_apply_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct qcom_snd_tdm_slot_cfg cpu_cfg;
> +	struct qcom_snd_tdm_slot_cfg codec_cfg;
> +	int ret;
> +
> +	ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
> +	if (ret)
> +		return ret == -EINVAL ? 0 : ret;

[Severity: Medium]
Will this silently ignore invalid TDM configurations in the device tree?

When qcom_snd_normalize_tdm_slots() detects an invalid configuration, like
mismatched slot counts or missing slot widths, it returns -EINVAL.

However, it looks like qcom_snd_apply_dai_tdm_slots() conflates this -EINVAL
return with the "not a TDM link" condition (which qcom_snd_parse_dai_tdm_slots()
also reports as -EINVAL when a link node is missing) and unconditionally
converts it to 0 (success).

Does this need to differentiate between a missing link node and an invalid
configuration to avoid masking configuration errors?

> +
> +	return qcom_snd_apply_dai_tdm_slots_cfg(rtd, &cpu_cfg, &codec_cfg);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6