Re: [PATCH v5 3/3] ASoC: qcom: sc8280xp: enhance machine driver for board-specific config

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect error code check for unsupported DAI format operations in `sc8280xp_snd_hw_params`.
--

--- Patch [3]: [PATCH v5 3/3] ASoC: qcom: sc8280xp: enhance machine driver for board-specific config ---
commit c24a266f369f96e073e2a8a89d27c04935c6d52d
Author: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

ASoC: qcom: sc8280xp: enhance machine driver for board-specific config

This commit enhances the sc8280xp machine driver to support board-specific
configuration. It allows each board variant to provide its own DAPM widgets
and routes, and enables MI2S MCLK programming.

> diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
> index 98b15a527e37e..109e39b93b2dd 100644
> --- a/sound/soc/qcom/sc8280xp.c
> +++ b/sound/soc/qcom/sc8280xp.c

[ ... ]

> @@ -96,6 +156,63 @@ static int sc8280xp_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
>  	return 0;
>  }
>  
> +static int sc8280xp_snd_hw_params(struct snd_pcm_substream *substream,
> +				  struct snd_pcm_hw_params *params)
> +{
> +	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
> +	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
> +	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
> +	struct sc8280xp_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> +	int mclk_freq = sc8280xp_get_mclk_freq(params);
> +	int bclk_freq = sc8280xp_get_bclk_freq(params);
> +	int ret;
> +
> +	switch (cpu_dai->id) {
> +	case PRIMARY_MI2S_RX ... QUATERNARY_MI2S_TX:
> +	case QUINARY_MI2S_RX ... QUINARY_MI2S_TX:
> +	case SENARY_MI2S_RX ... SENARY_MI2S_TX:
> +		ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_BP_FP);
> +		if (ret && ret != -EOPNOTSUPP)
> +			return ret;

[Severity: Medium]
Should this error check evaluate against -ENOTSUPP instead of -EOPNOTSUPP?

Looking at snd_soc_dai_set_fmt() in sound/soc/soc-dai.c, it initializes ret
to -ENOTSUPP (524) and returns that if the DAI driver doesn't implement the
set_fmt operation. Since -EOPNOTSUPP is a different value (95), will this
inadvertently propagate the error and fail ALSA PCM stream setup on hardware
where the CPU DAI does not implement .set_fmt?

> +
> +		if (data->snd_soc_common_priv->codec_dai_fmt) {
> +			ret = snd_soc_dai_set_fmt(codec_dai,
> +						  data->snd_soc_common_priv->codec_dai_fmt);
> +			if (ret && ret != -EOPNOTSUPP)
> +				return ret;

[Severity: Medium]
Similarly, could this cause stream setup failures for codecs that don't
support set_fmt? 

If the target codec DAI is missing .set_fmt, snd_soc_dai_set_fmt() will
return -ENOTSUPP rather than -EOPNOTSUPP.

> +		}
> +
> +		if (data->snd_soc_common_priv->mi2s_mclk_enable) {
> +			ret = snd_soc_dai_set_sysclk(cpu_dai,
> +						     LPAIF_MI2S_MCLK, mclk_freq,
> +						     SND_SOC_CLOCK_OUT);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714145250.2473461-1-mohammad.rafi.shaik@oss.qualcomm.com?part=3