Re: [PATCH v4 5/8] ASoC: qcom: lpass-va-macro: Add SoundWire microphone support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: qcom: lpass-va-macro: inactive mux write hijacks active stream
- [Medium] ASoC: qcom: lpass-va-macro: dynamic mux switch leaves stale hardware state
--

commit 930ab6f652b9c31708f76061b90cd77f78ea08ce
Author: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>

ASoC: qcom: lpass-va-macro: Add SoundWire microphone support
    
The VA decimators can currently capture audio only from DMIC inputs.
Add support for routing audio from SoundWire microphone (SMIC) inputs
as well.

> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
> index 02dc3346924f1..79abbdd158b70 100644
> --- a/sound/soc/codecs/lpass-va-macro.c
> +++ b/sound/soc/codecs/lpass-va-macro.c
[ ... ]
> @@ -794,10 +794,22 @@ static int va_macro_put_dec_enum(struct snd_kcontrol *kcontrol,
>  		return -EINVAL;
>  	}
>  
> -	if (val != 0)
> -		snd_soc_component_update_bits(component, mic_sel_reg,
> -					      CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK,
> -					      CDC_VA_TX_PATH_ADC_DMIC_SEL_DMIC);
> +	if (val != 0) {
> +		/*
> +		 * SMIC enums use shift 0 (lower nibble of CFG0),
> +		 * DMIC enums use shift 4 (upper nibble of CFG0).
> +		 * Use e->shift_l to distinguish them without relying
> +		 * on widget name comparisons.
> +		 */
> +		if (e->shift_l == 0)
> +			snd_soc_component_update_bits(component, mic_sel_reg,
> +						      CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK,
> +						      CDC_VA_TX_PATH_ADC_DMIC_SEL_ADC);
> +		else
> +			snd_soc_component_update_bits(component, mic_sel_reg,
> +						      CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK,
> +						      CDC_VA_TX_PATH_ADC_DMIC_SEL_DMIC);
> +	}

[Severity: High]
Does this unconditionally overwrite the shared decimator input domain bit
regardless of which path DAPM has actively routed?

If a user-space application writes a non-zero value to the inactive sub-mux
(e.g., VA SMIC MUX0) while the decimator is actively capturing from the other
source (VA DMIC MUX0), could this instantly corrupt the active audio capture
stream mid-stream?

>  
>  	return snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
>  }
[ ... ]
> @@ -1004,6 +1025,17 @@ static int va_macro_enable_dec(struct snd_soc_dapm_widget *w,
>  
>  	switch (event) {
>  	case SND_SOC_DAPM_PRE_PMU:
> +		/*
> +		 * ADC_DMIC_SEL is shared by the DMIC and the SoundWire mic
> +		 * mux, so a mixer write to the mux that is not part of the
> +		 * active path can leave it stale. Program it from the mux
> +		 * state DAPM resolved for this decimator.
> +		 */
> +		snd_soc_component_update_bits(comp, dec_cfg_reg,
> +					      CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK,
> +					      is_amic_enabled(comp, decimator) ?
> +					      CDC_VA_TX_PATH_ADC_DMIC_SEL_ADC :
> +					      CDC_VA_TX_PATH_ADC_DMIC_SEL_DMIC);

[Severity: Medium]
Does this apply the hardware state only on PRE_PMU?

If a user dynamically switches the decimator source while it is active
(e.g., changing VA DECX MUX via mixer controls), DAPM might not execute the
PRE_PMU event since the widget remains powered.

Would this leave the newly added ADC_DMIC_SEL configuration unexecuted,
causing the hardware register to remain in a stale state and breaking the
capture stream?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922-add_shikra_support-v1-0-6c6cb6444caf@oss.qualcomm.com?part=5