Re: [PATCH v2 5/5] ASoC: qcom: sc8280xp: Add shikra-iqs-evk compatible support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Clock provider collision on the I2S bus: both CPU and Codec DAIs are configured as clock providers, resulting in a bus collision on the bitclock and frame clock lines.
--

commit 38d45c7da72e3c695c8ea9cd9e58fbc7be8c7070
Author: Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>

ASoC: qcom: sc8280xp: Add shikra-iqs-evk compatible support
    
Add machine driver support for the Qualcomm Shikra IQS EVK board.
The Shikra IQS platform uses a third-party MAX98091 codec with
modem-DSP support.

> diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
> index 5bcdaa1927048..0551110a685f6 100644
> --- a/sound/soc/qcom/sc8280xp.c
> +++ b/sound/soc/qcom/sc8280xp.c

[ ... ]

> @@ -543,6 +557,19 @@ static const struct qcom_snd_soc_common shikra_cqs_priv_data = {
>  	.codec_sysclk_set = true,
>  };
>  
> +static const struct qcom_snd_soc_common shikra_iqs_priv_data = {
> +	.driver_name = "shikra",
> +	.dapm_widgets = shikra_iqs_dapm_widgets,
> +	.num_dapm_widgets = ARRAY_SIZE(shikra_iqs_dapm_widgets),
> +	.controls = shikra_iqs_controls,
> +	.num_controls = ARRAY_SIZE(shikra_iqs_controls),
> +	.codec_dai_fmt = SND_SOC_DAIFMT_CBP_CFP |
> +			 SND_SOC_DAIFMT_NB_NF |
> +			 SND_SOC_DAIFMT_I2S,

[Severity: High]
Does this configuration cause a clock provider collision on the I2S bus?

Here in shikra_iqs_priv_data, codec_dai_fmt configures the codec as the
clock provider (SND_SOC_DAIFMT_CBP_CFP). However, cpu_dai_fmt is left
uninitialized.

During audio playback, when sc8280xp_snd_hw_params() is called, an
uninitialized cpu_dai_fmt triggers a fallback path:

sc8280xp_snd_hw_params() {
        ...
        if (data->priv->cpu_dai_fmt) {
                ...
        } else {
                ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_BP_FP);
                ...
        }
        ...
}

This fallback configures the CPU DAI as a clock provider as well. With
both the codec and the CPU configured to actively drive the bitclock and
frame clock lines, will this lead to a regression with an electrical
collision on the bus?

If the CPU is intended to be the master (as implied by mi2s_bclk_enable
being true), the codec should be configured with SND_SOC_DAIFMT_CBC_CFC.
If the codec is the intended master, cpu_dai_fmt should be explicitly set
to SND_SOC_DAIFMT_BC_FC.

> +	.codec_sysclk_set = true,
> +	.mi2s_bclk_enable = true,
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-shikra-machine-v2-v2-0-9bf6bad589d8@oss.qualcomm.com?part=5