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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] ASoC: qcom: lpass-va-macro: missing mux defaults in
  va_4_1_defaults
- [Medium] ASoC: qcom: lpass-va-macro: stale ADC_DMIC_SEL on live VA
  DECn MUX switch
--

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

ASoC: qcom: lpass-va-macro: Add SoundWire microphone support

This commit adds DAPM mux widgets, routes, and decimator source selection
logic to support SoundWire microphone (SMIC) inputs alongside DMIC inputs.

> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
> index db395674312c8..a113474040587 100644
> --- a/sound/soc/codecs/lpass-va-macro.c
> +++ b/sound/soc/codecs/lpass-va-macro.c
> @@ -754,6 +754,16 @@ static int va_macro_mclk_event(struct snd_soc_dapm_widget *w,
>  	return 0;
>  }
>  
> +static bool is_amic_enabled(struct snd_soc_component *comp, int decimator)
> +{
> +	u16 adc_mux_reg;
> +
> +	adc_mux_reg = CDC_VA_INP_MUX_ADC_MUX0_CFG1 +
> +		      VA_MACRO_ADC_MUX_CFG_OFFSET * decimator;
> +
> +	return snd_soc_component_read(comp, adc_mux_reg) & BIT(0);
> +}

[Severity: High]
Does this fail when the codec is runtime-suspended on LPASS v4.1 (Shikra)?

va_4_1_regmap_config uses REGCACHE_MAPLE backed by va_4_1_defaults[], which
unlike va_defaults[] omits CDC_VA_INP_MUX_ADC_MUX0_CFG0 through
CDC_VA_INP_MUX_ADC_MUX3_CFG1.  Once va_macro_runtime_suspend() enables
cache-only mode, reading or updating those uncached mux registers returns
-EBUSY.

When userspace configures va_smic0..3, va_dmic0..3, or va_dec0..3 while the
codec is idle, snd_soc_component_read() in is_amic_enabled() returns
(unsigned int)-EBUSY (0xfffffff0, where BIT(0) is 0),
snd_soc_dapm_put_enum_double() fails with -EBUSY without updating the mux,
and snd_soc_dapm_get_enum_double() reads 0xfffffff0 and extracts an
out-of-bounds enum index (15).

Adding CDC_VA_INP_MUX_ADC_MUX0_CFG0..CDC_VA_INP_MUX_ADC_MUX3_CFG1 to
va_4_1_defaults[] to match va_defaults[] would populate them in the maple
tree cache.

> +
>  static int va_macro_put_dec_enum(struct snd_kcontrol *kcontrol,
>  				 struct snd_ctl_elem_value *ucontrol)
>  {
> @@ -763,20 +773,26 @@ static int va_macro_put_dec_enum(struct snd_kcontrol *kcontrol,
>  	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
>  	unsigned int val;
>  	u16 mic_sel_reg;
> +	int decimator;
> +	bool smic_mux;
>  
>  	val = ucontrol->value.enumerated.item[0];
>  
>  	switch (e->reg) {
>  	case CDC_VA_INP_MUX_ADC_MUX0_CFG0:
> +		decimator = 0;
>  		mic_sel_reg = CDC_VA_TX0_TX_PATH_CFG0;
>  		break;
>  	case CDC_VA_INP_MUX_ADC_MUX1_CFG0:
> +		decimator = 1;
>  		mic_sel_reg = CDC_VA_TX1_TX_PATH_CFG0;
>  		break;
>  	case CDC_VA_INP_MUX_ADC_MUX2_CFG0:
> +		decimator = 2;
>  		mic_sel_reg = CDC_VA_TX2_TX_PATH_CFG0;
>  		break;
>  	case CDC_VA_INP_MUX_ADC_MUX3_CFG0:
> +		decimator = 3;
>  		mic_sel_reg = CDC_VA_TX3_TX_PATH_CFG0;
>  		break;
>  	default:
> @@ -785,10 +801,24 @@ static int va_macro_put_dec_enum(struct snd_kcontrol *kcontrol,
>  		return -EINVAL;
>  	}
>  
> -	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.
> +	 */
> +	smic_mux = (e->shift_l == 0);
> +
> +	/*
> +	 * Both muxes feed the same decimator and share ADC_DMIC_SEL, so only
> +	 * the one the decimator is routed through may program it. A write to
> +	 * the mux that is not part of the active path would otherwise switch
> +	 * the input domain of a running capture.
> +	 */
> +	if (val != 0 && smic_mux == is_amic_enabled(component, decimator))
>  		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);
> +					      smic_mux ? CDC_VA_TX_PATH_ADC_DMIC_SEL_ADC :
> +							 CDC_VA_TX_PATH_ADC_DMIC_SEL_DMIC);
>  
>  	return snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
>  }

[ ... ]

> @@ -995,6 +1024,18 @@ 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 its value depends on the order in which userspace
> +		 * restores the two controls. Program it from the resolved
> +		 * mux state of this decimator, which is what selects the
> +		 * input domain.
> +		 */
> +		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]
Can switching VA DEC0 MUX..VA DEC3 MUX during an active capture leave
CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK stale?

If capture is running on VA_DMIC (e.g. DMIC0) and userspace pre-selects a
non-ZERO input on VA SMIC MUXn (e.g. SWR_MIC0), va_macro_put_dec_enum()
skips updating CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK because smic_mux does not
match is_amic_enabled().  When userspace then switches VA DECn MUX from
VA_DMIC to SWR_MIC, both input branches have connected endpoints, so
VA DECn MUX stays powered and SND_SOC_DAPM_PRE_PMU in va_macro_enable_dec()
does not fire.

Because va_dec0_mux..va_dec3_mux use plain SOC_DAPM_ENUM() without a custom
put callback or SND_SOC_DAPM_POST_REG event, CDC_VA_TX_PATH_ADC_DMIC_SEL_MASK
remains stuck on the previous input domain for the rest of the stream.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-add_shikra_support-v6-0-2cda302237cc@oss.qualcomm.com?part=5