Re: [PATCH v1 3/3] ASoC: qcom: lpass-va-macro: Add shikra compatible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The max_register limit for the Shikra regmap is set to 0x0980, but the default register array contains registers up to 0x09A4, leading to regmap initialization failure.
- [High] The newly added Shikra ADPT filter configuration registers (except ADPT_CTRL) are missing from the register access whitelist, rendering them unwritable.
--

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

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

This commit adds shikra specific VA macro data and register programming
needed by the LPASS codec v4.0 implementation. It includes a shikra regmap
table, new match data flags, and enables ADPT control programming.

> diff --git a/sound/soc/codecs/lpass-va-macro.c b/sound/soc/codecs/lpass-va-macro.c
> index 528d5b167ecff..4a8daa07ec1c6 100644
> --- a/sound/soc/codecs/lpass-va-macro.c
> +++ b/sound/soc/codecs/lpass-va-macro.c
> @@ -155,7 +155,57 @@

[ ... ]

>  #define VA_MAX_OFFSET				(0x07A8)
> +#define VA_SHIKRA_MAX_OFFSET			(0x0980)

[Severity: High]
Does this offset limit need to be higher?

The max_register limit for the Shikra regmap is set to 0x0980 here, but
the va_shikra_defaults array contains registers up to 0x09A4
(CDC_VA_CDC_ADPT3_CUTOFF_FSM_CTRL_5).

During probe, devm_regmap_init_mmio() validates that all default registers
fall within the [0, max_register] range. Because 0x09A4 is greater than
0x0980, it seems regmap_init() will reject the defaults with -EINVAL,
causing the probe to fail.

>  
>  #define VA_MACRO_NUM_DECIMATORS 4
>  #define VA_MACRO_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |\

[ ... ]

> @@ -424,6 +464,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]
Should the other newly added Shikra ADPT filter configuration registers be
included in this whitelist?

The patch adds numerous adaptive filter configuration registers like
ADPT_GAIN_0, ADPT_GAIN_1, DH_FSM_CTRL, and CUTOFF_FSM_CTRL_*. However, only
the 4 base CDC_VA_CDC_ADPTx_ADPT_CTRL registers are whitelisted here.

Since this function is provided as the regmap's .writeable_reg callback, the
regmap core will likely consider the remaining 36 ADPT registers to be
inaccessible, preventing the hardware adaptive filters from receiving their
intended configuration.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608172023.2965292-1-mohammad.rafi.shaik@oss.qualcomm.com?part=3