Re: [PATCH v3 v3 4/5] ASoC: qcom: lpass-va-macro: Add shikra compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ASoC: qcom: lpass-va-macro: missing ADPT registers in rw_register whitelist
- [Medium] ASoC: qcom: lpass-va-macro: missing hardware version cross-check in probe
- [Medium] ASoC: qcom: lpass-va-macro: out-of-bounds regmap access on legacy compatibles
--

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

ASoC: qcom: lpass-va-macro: Add shikra compatible

Adds shikra specific VA macro data and register programming for the LPASS
codec v4.1 implementation. It extends the VA macro with four adaptive filter
(ADPT) blocks and concatenates the new registers with the common defaults at
probe time.

> The register layout has to be chosen before the regmap exists, i.e.
> before CDC_VA_TOP_CSR_CORE_ID_* can be read, so it comes from match data;
> probe then cross checks it against the version reported by the hardware.

[Severity: Medium]
The commit message states probe cross-checks the version reported by the
hardware, but does the implementation bypass this check entirely?

Looking at va_macro_probe(), if data->version is provided (which it is
for shikra_va_data), va_macro_set_lpass_codec_version() is skipped,
meaning the driver blindly trusts the device tree compatible string
rather than verifying the hardware CORE_ID_* registers.

> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
> --- a/sound/soc/codecs/lpass-va-macro.c
> +++ b/sound/soc/codecs/lpass-va-macro.c
> @@ -430,6 +601,10 @@ static bool va_is_rw_register(struct device *dev, unsigned int reg)
>  	case CDC_VA_TX3_TX_PATH_SEC4:
>  	case CDC_VA_TX3_TX_PATH_SEC5:
>  	case CDC_VA_TX3_TX_PATH_SEC6:
> +	case CDC_VA_CDC_ADPT0_ADPT_CTRL:
> +	case CDC_VA_CDC_ADPT1_ADPT_CTRL:
> +	case CDC_VA_CDC_ADPT2_ADPT_CTRL:
> +	case CDC_VA_CDC_ADPT3_ADPT_CTRL:
>  		return true;
>  	}

[Severity: High]
Are the other newly added ADPT registers missing from this whitelist?

The va_4_1_defaults array defines 40 new registers (including ADPT_GAIN_0,
ADPT_GAIN_1, DH_FSM_CTRL, and CUTOFF_FSM_CTRL_0..5 for all 4 decimators),
but only the 4 ADPT_CTRL registers are returned as true here.

Since va_is_readable_register() falls back to this writable list, the
remaining 36 registers will be strictly rejected by the regmap subsystem
and regcache_sync() will silently drop them, leaving the adaptive filters
unconfigured.

[ ... ]

> @@ -772,6 +974,10 @@ static int va_macro_enable_dec(struct snd_soc_dapm_widget *w,
>  		/* Enable TX PGA Mute */
>  		break;
>  	case SND_SOC_DAPM_POST_PMU:
> +		if (va->has_adpt)
> +			snd_soc_component_update_bits(comp, adpt_ctrl_reg,
> +						      CDC_VA_ADPT_CTRL_EN_MASK, 0x00);
> +
>  		/* Enable TX CLK */
>  		snd_soc_component_update_bits(comp, tx_vol_ctl_reg,

[Severity: Medium]
Will this cause an out-of-bounds regmap access on legacy compatibles?

If v4.1 hardware is booted with an older fallback compatible string (e.g.,
qcom,sm8550-lpass-va-macro), data->regmap_config is NULL. This means regmap
is initialized using the fallback va_regmap_config which restricts
max_register to 0x07A8.

However, because data->version is 0 for those older compatibles, dynamic
hardware detection correctly identifies v4.1 and sets va->has_adpt = true.
During audio playback, this block writes to adpt_ctrl_reg (>0x0800), which
regmap will reject for exceeding the 0x07A8 maximum limit.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-add_shikra_support-v3-0-fc0e79c341aa@oss.qualcomm.com?part=4