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

[Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>]

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@oss.qualcomm.com>
>> ---
> 
> [...]
> 
>> +/* 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