Re: [PATCH RFC 1/2] ASoC: codecs: lpass-tx-macro: Use correct config for sc7280

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Changing the match data for qcom,sc7280-lpass-tx-macro breaks userspace ALSA ABI and in-kernel DTS routing.

Pre-existing issues:
- [High] The v9.2 SMIC selection logic contains dead code and fails to initialize DMIC clocks.
--

commit 75eb73d3fec8fc578947e66030a2b59ca6333c91
Author: Luca Weiss <luca.weiss@fairphone.com>

ASoC: codecs: lpass-tx-macro: Use correct config for sc7280

This commit updates the match data for the qcom,sc7280-lpass-tx-macro
compatible string to use lpass_ver_9_2 instead of lpass_ver_9. The commit
message notes this will change control names and warns it is a breaking
change.

> Note, that this will change control names so it's a breaking change.

[Severity: High]
Does this violate the kernel's policy against breaking userspace ABI?

Changing the match data to lpass_ver_9_2 alters the names of DAPM widgets
and ALSA controls. This will break existing ALSA UCM configurations that
rely on the old names. 

Additionally, will this break in-kernel DTS routing for other SC7280 boards?

Multiple in-tree SC7280 device tree files (like sc7280-herobrine and
sc7280-idp) hardcode the old routing names (TX SWR_ADCn and TX SWR_DMICn).

Since these DTS files aren't updated, could the sound card fail to
instantiate on those boards due to missing audio routes?

> diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
> index f7d168f557dd8..7f93ab0c38723 100644
> --- a/sound/soc/codecs/lpass-tx-macro.c
> +++ b/sound/soc/codecs/lpass-tx-macro.c
> @@ -2493,14 +2493,9 @@ static const struct tx_macro_data lpass_ver_11 = {
>  
>  static const struct of_device_id tx_macro_dt_match[] = {
>  	{
> -		/*
> -		 * The block is actually LPASS v9.4, but keep LPASS v9 match
> -		 * data and audio widgets, due to compatibility reasons.
> -		 * Microphones are working on SC7280 fine, so apparently the fix
> -		 * is not necessary.
> -		 */
>  		.compatible = "qcom,sc7280-lpass-tx-macro",
> -		.data = &lpass_ver_9,
> +		/* LPASS v9.4 */
> +		.data = &lpass_ver_9_2,

[Severity: High]
This is a pre-existing issue, but by forcing SC7280 to use the v9.2 code paths,
does this expose the device to a broken DMIC clock initialization logic?

In tx_macro_put_dec_enum(), tx_macro_update_smic_sel_v9_2() is exclusively
called in the else branch when widget->shift is 0:

sound/soc/codecs/lpass-tx-macro.c:tx_macro_put_dec_enum() {
	...
	if (val != 0) {
		if (widget->shift) /* MSM DMIC */
			snd_soc_component_write_field(...);
		else if (tx->data->ver <= LPASS_VER_9_0_0)
			tx_macro_update_smic_sel_v9(...);
		else
			tx_macro_update_smic_sel_v9_2(component, widget, tx,
						      mic_sel_reg, val);
	}
	...
}

However, inside tx_macro_update_smic_sel_v9_2(), there is an if block intended
to initialize the DMIC clocks:

sound/soc/codecs/lpass-tx-macro.c:tx_macro_update_smic_sel_v9_2() {
	...
	if (widget->shift) {
		/* MSM DMIC */
		snd_soc_component_write_field(component, mic_sel_reg,
					      CDC_TXn_ADC_DMIC_SEL_MASK, 1);
		...
		snd_soc_component_write_field(component, dmic_clk_reg,
					      CDC_TX_SWR_DMIC_CLK_SEL_MASK,
					      CDC_TX_SWR_MIC_CLK_DEFAULT);
	}
}

Because widget->shift is always 0 when this function is called, this block
appears to be dead code and structurally impossible to reach.

Consequently, are DMIC clocks failing to initialize for v9.2 hardware?

>  	}, {
>  		.compatible = "qcom,sm6115-lpass-tx-macro",
>  		.data = &lpass_ver_10_sm6115,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-sc7280-tx-macro-v1-0-1aad6900fec0@fairphone.com?part=1