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

[Mohammad Rafi Shaik]

On 6/9/2026 2:45 PM, Konrad Dybcio wrote:
> On 6/8/26 7:20 PM, Mohammad Rafi Shaik wrote:
> > Add shikra specific VA macro data and register programming needed by
> > the LPASS codec v4.0 implementation.
> >
> > This adds a shikra regmap/default table covering the ADPT register
> > range, wires new match data flags (bypass FS control and ADPT block
> > presence), and enables the ADPT control programming path during TX
> > decimator bring-up.
> >
> > Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@xxxxxxxxxxxxxxxx>
> > ---
>
> [...]
>
> > +/* ADPT control registers - Shikra adaptive filter blocks */
> > +#define CDC_VA_CDC_ADPT0_ADPT_CTRL		(0x0800)
> > +#define CDC_VA_CDC_ADPT0_ADPT_GAIN_0		(0x0804)
> > +#define CDC_VA_CDC_ADPT0_ADPT_GAIN_1		(0x0808)
> > +#define CDC_VA_CDC_ADPT0_DH_FSM_CTRL		(0x080C)
> > +#define CDC_VA_CDC_ADPT0_CUTOFF_FSM_CTRL_0	(0x0810)
> > +#define CDC_VA_CDC_ADPT0_CUTOFF_FSM_CTRL_1	(0x0814)
> > +#define CDC_VA_CDC_ADPT0_CUTOFF_FSM_CTRL_2	(0x0818)
> > +#define CDC_VA_CDC_ADPT0_CUTOFF_FSM_CTRL_3	(0x081C)
> > +#define CDC_VA_CDC_ADPT0_CUTOFF_FSM_CTRL_4	(0x0820)
> > +#define CDC_VA_CDC_ADPT0_CUTOFF_FSM_CTRL_5	(0x0824)
>
> Please use lowercase hex and drop the parentheses

will fix it in the next revision.

> [...]
>
> > +static const struct reg_default va_shikra_defaults[] = {
> > +	/* VA macro */
> > +	{ CDC_VA_CLK_RST_CTRL_MCLK_CONTROL, 0x00},
> > +	{ CDC_VA_CLK_RST_CTRL_FS_CNT_CONTROL, 0x00},
> > +	{ CDC_VA_CLK_RST_CTRL_SWR_CONTROL, 0x00},
>
> lowercase hex likewise, please keep a consistent space before the closing
> curly bracket

Ack,

> [...]
>
> > +		if (va->bypass_fs_control)
> > +			regmap_update_bits(regmap, CDC_VA_CLK_RST_CTRL_FS_CNT_CONTROL,
> > +					   0x80, 0x80);
>
> regmap_set_bits()
>
> [...]
>
> >   	case SND_SOC_DAPM_POST_PMU:
> > +		if (va->has_adpt_block)
> > +			snd_soc_component_update_bits(comp, adapt_ctrl, 0xFF, 0x00);
>
> Can you define these magic numbers?

sure, will define and use the same.

> [...]
>
> > +	va->regmap = devm_regmap_init_mmio(dev, base,
> > +					   data->regmap_config ? data->regmap_config
> > +						: &va_regmap_config);
>
> data->regmap_config ?: &va_regmap_config

Will fix in the next revision,

Thanks & regards,
Rafi.

> Konrad