Re: [PATCH v5 8/9] ASoC: qcom: lpass-tx-macro: Add support for Bolero v4.0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] cleanup: Mixing goto-based error handling with __free() scope-based cleanup
- [Medium] ASoC: qcom: lpass-tx-macro: SWR_MIC bitwise check misidentifies RX_SWR_MIC
--

commit 64e799e2b76860a53ceff3f36c16c6368d70c60e
Author: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>

ASoC: qcom: lpass-tx-macro: Add support for Bolero v4.0

This commit adds support for the Bolero v4.0 TX macro in the Hawi platform.
It introduces v4.0 specific register defaults, extends regmap accessors, and
registers a separate decimator widget table to handle the new mux enum and
DAPM events.

> diff --git a/sound/soc/codecs/lpass-tx-macro.c b/sound/soc/codecs/lpass-tx-macro.c
> index 6deaaabe43afa..6480a6c9032e3 100644
> --- a/sound/soc/codecs/lpass-tx-macro.c
> +++ b/sound/soc/codecs/lpass-tx-macro.c
[ ... ]
> @@ -929,6 +998,10 @@ static int tx_macro_enable_dec(struct snd_soc_dapm_widget *w,
>  
>  	switch (event) {
>  	case SND_SOC_DAPM_PRE_PMU:
> +		/* Disable the ADPT block before enabling the decimator */
> +		if (tx->data->has_adpt)
> +			snd_soc_component_write_field(component, CDC_ADPT0_ADPT_CTRL,
> +						      CDC_TX_ADPT_CTRL_EN_MASK, 0x0);
>  		adc_mux_reg = CDC_TX_INP_MUX_ADC_MUXn_CFG1(decimator);
>  		if (snd_soc_component_read(component, adc_mux_reg) & SWR_MIC) {

[Severity: Medium]
Will this bitwise check incorrectly match the newly added RX_SWR_MIC?

The adc_mux_text_v9_3 enum added below introduces RX_SWR_MIC at index 3. When
RX_SWR_MIC is selected, snd_soc_component_read() & SWR_MIC evaluates to
3 & 1 == 1 (true).

This erroneously triggers SWR_MIC-specific logic for RX_SWR_MIC in both
tx_macro_enable_dec() and is_amic_enabled(). If a machine driver routes
RX_SWR_MIC and powers it up, the PRE_PMU block will read the irrelevant SMIC
mux register and overwrite CDC_TX_TOP_CSR_SWR_DMICn_CTL if the leftover value
is >= 5.

Even if not routed, the teardown bug in PRE_PMD executes when userspace
switches the mux away from an active path to RX_SWR_MIC.

[ ... ]
> @@ -2027,6 +2103,418 @@ static const struct snd_soc_dapm_route tx_audio_map_v9_2[] = {
>  	{"TX SMIC MUX7", "SWR_MIC11", "TX SWR_INPUT11"},
>  };
>  
> +static const char * const adc_mux_text_v9_3[] = {
> +	"MSM_DMIC", "SWR_MIC", "ANC_FB_TUNE1", "RX_SWR_MIC"
> +};

[ ... ]
> @@ -2250,6 +2751,9 @@ static int tx_macro_probe(struct platform_device *pdev)
>  	struct device_node *np = dev->of_node;
>  	struct tx_macro *tx;
>  	void __iomem *base;
> +	struct reg_default *reg_defaults __free(kfree) = NULL;
> +	struct regmap_config *reg_config __free(kfree) = NULL;
> +	size_t def_count;
>  	int ret, reg;

[ ... ]
> @@ -2304,7 +2808,40 @@ static int tx_macro_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	tx->regmap = devm_regmap_init_mmio(dev, base, &tx_regmap_config);
> +	reg_config = kmemdup(&tx_regmap_config, sizeof(*reg_config), GFP_KERNEL);
> +	if (!reg_config) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}

[Severity: Medium]
Is it safe to mix goto-based error handling with scope-based cleanup in this
function?

The new reg_defaults and reg_config pointers are declared with the
__free(kfree) attribute for automatic scope-based cleanup. However,
tx_macro_probe() handles errors unconditionally by jumping to the err or
err_rpm_put labels via goto statements.

The global Linux kernel cleanup guidelines explicitly forbid mixing goto-based
error paths with __free() cleanup blocks in the same function. Doing so causes
confusing ownership semantics and can lead to bugs where the compiler emits
cleanup instructions at label destinations that interfere with the expected
control flow. The subsystem rules require converting all resources to
scope-based cleanup or converting none of them.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-add_shikra_support_v5-v5-0-fb9f0667e613@oss.qualcomm.com?part=8