Re: [PATCH v1 3/5] ASoC: cs35l56: Add Mute, Volume and Posture registers to firmware register list

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 7/4/25 16:16, Stefan Binding wrote:
> Registers to set Mute, Volume and Posture are inside firmware,
> which means they should be added to the list of registers set inside
> firmware, in case they vary across Device or Revision.
> 
> These three registers are also used for controls, so additional
> handling is required to be able to obtain and set the register inside
> ALSA controls.
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> ---
>   include/sound/cs35l56.h           |  3 +++
>   sound/pci/hda/cs35l56_hda.c       | 17 +++++++----------
>   sound/soc/codecs/cs35l56-shared.c | 25 ++++++++++++++++++++++++-
>   sound/soc/codecs/cs35l56.c        | 19 +++++++++++++++++--
>   4 files changed, 51 insertions(+), 13 deletions(-)
> 
> diff --git a/include/sound/cs35l56.h b/include/sound/cs35l56.h
> index d712cb79652b..3abe4fbd2085 100644
> --- a/include/sound/cs35l56.h
> +++ b/include/sound/cs35l56.h
> @@ -273,6 +273,9 @@ struct cs35l56_fw_reg {
>   	unsigned int pm_cur_stat;
>   	unsigned int prot_sts;
>   	unsigned int transducer_actual_ps;
> +	unsigned int user_mute;
> +	unsigned int user_volume;
> +	unsigned int posture_number;
>   };
>   
>   struct cs35l56_base {
> diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c
> index f6257fe45b9f..61aba1956338 100644
> --- a/sound/pci/hda/cs35l56_hda.c
> +++ b/sound/pci/hda/cs35l56_hda.c
> @@ -237,7 +237,8 @@ static int cs35l56_hda_posture_get(struct snd_kcontrol *kcontrol,
>   
>   	cs35l56_hda_wait_dsp_ready(cs35l56);
>   
> -	ret = regmap_read(cs35l56->base.regmap, CS35L56_MAIN_POSTURE_NUMBER, &pos);
> +	ret = regmap_read(cs35l56->base.regmap,
> +			  cs35l56->base.fw_reg->posture_number, &pos);
>   	if (ret)
>   		return ret;
>   
> @@ -260,10 +261,8 @@ static int cs35l56_hda_posture_put(struct snd_kcontrol *kcontrol,
>   
>   	cs35l56_hda_wait_dsp_ready(cs35l56);
>   
> -	ret = regmap_update_bits_check(cs35l56->base.regmap,
> -				       CS35L56_MAIN_POSTURE_NUMBER,
> -				       CS35L56_MAIN_POSTURE_MASK,
> -				       pos, &changed);
> +	ret = regmap_update_bits_check(cs35l56->base.regmap, cs35l56->base.fw_reg->posture_number,
> +				       CS35L56_MAIN_POSTURE_MASK, pos, &changed);
>   	if (ret)
>   		return ret;
>   
> @@ -305,7 +304,7 @@ static int cs35l56_hda_vol_get(struct snd_kcontrol *kcontrol,
>   
>   	cs35l56_hda_wait_dsp_ready(cs35l56);
>   
> -	ret = regmap_read(cs35l56->base.regmap, CS35L56_MAIN_RENDER_USER_VOLUME, &raw_vol);
> +	ret = regmap_read(cs35l56->base.regmap, cs35l56->base.fw_reg->user_volume, &raw_vol);
>   
>   	if (ret)
>   		return ret;
> @@ -339,10 +338,8 @@ static int cs35l56_hda_vol_put(struct snd_kcontrol *kcontrol,
>   
>   	cs35l56_hda_wait_dsp_ready(cs35l56);
>   
> -	ret = regmap_update_bits_check(cs35l56->base.regmap,
> -				       CS35L56_MAIN_RENDER_USER_VOLUME,
> -				       CS35L56_MAIN_RENDER_USER_VOLUME_MASK,
> -				       raw_vol, &changed);
> +	ret = regmap_update_bits_check(cs35l56->base.regmap, cs35l56->base.fw_reg->user_volume,
> +				       CS35L56_MAIN_RENDER_USER_VOLUME_MASK, raw_vol, &changed);
>   	if (ret)
>   		return ret;
>   
> diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c
> index bc8f9379bc74..da982774bb4d 100644
> --- a/sound/soc/codecs/cs35l56-shared.c
> +++ b/sound/soc/codecs/cs35l56-shared.c
> @@ -38,7 +38,9 @@ static const struct reg_sequence cs35l56_patch[] = {
>   	{ CS35L56_SWIRE_DP3_CH3_INPUT,		0x00000029 },
>   	{ CS35L56_SWIRE_DP3_CH4_INPUT,		0x00000028 },
>   	{ CS35L56_IRQ1_MASK_18,			0x1f7df0ff },
> +};
>   
> +static const struct reg_sequence cs35l56_patch_fw[] = {
>   	/* These are not reset by a soft-reset, so patch to defaults. */
>   	{ CS35L56_MAIN_RENDER_USER_MUTE,	0x00000000 },
>   	{ CS35L56_MAIN_RENDER_USER_VOLUME,	0x00000000 },
> @@ -47,8 +49,26 @@ static const struct reg_sequence cs35l56_patch[] = {
>   
>   int cs35l56_set_patch(struct cs35l56_base *cs35l56_base)
>   {
> -	return regmap_register_patch(cs35l56_base->regmap, cs35l56_patch,
> +	int ret;
> +
> +	ret = regmap_register_patch(cs35l56_base->regmap, cs35l56_patch,
>   				     ARRAY_SIZE(cs35l56_patch));
> +	if (ret)
> +		return ret;
> +
> +
> +	switch (cs35l56_base->type) {
> +	case 0x54:
> +	case 0x56:
> +	case 0x57:
> +		ret = regmap_register_patch(cs35l56_base->regmap, cs35l56_patch_fw,
> +					    ARRAY_SIZE(cs35l56_patch_fw));
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_NS_GPL(cs35l56_set_patch, "SND_SOC_CS35L56_SHARED");
>   
> @@ -1066,6 +1086,9 @@ const struct cs35l56_fw_reg cs35l56_fw_reg = {
>   	.pm_cur_stat = CS35L56_DSP1_PM_CUR_STATE,
>   	.prot_sts = CS35L56_PROTECTION_STATUS,
>   	.transducer_actual_ps = CS35L56_TRANSDUCER_ACTUAL_PS,
> +	.user_mute = CS35L56_MAIN_RENDER_USER_MUTE,
> +	.user_volume = CS35L56_MAIN_RENDER_USER_VOLUME,
> +	.posture_number = CS35L56_MAIN_POSTURE_NUMBER,
>   };
>   EXPORT_SYMBOL_NS_GPL(cs35l56_fw_reg, "SND_SOC_CS35L56_SHARED");
>   
> diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
> index c1d8bfb803b9..a4a1d09097fc 100644
> --- a/sound/soc/codecs/cs35l56.c
> +++ b/sound/soc/codecs/cs35l56.c
> @@ -838,6 +838,7 @@ static int cs35l56_component_probe(struct snd_soc_component *component)
>   	struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(component);
>   	struct dentry *debugfs_root = component->debugfs_root;
>   	unsigned short vendor, device;
> +	int ret;
>   
>   	BUILD_BUG_ON(ARRAY_SIZE(cs35l56_tx_input_texts) != ARRAY_SIZE(cs35l56_tx_input_values));
>   
> @@ -877,6 +878,22 @@ static int cs35l56_component_probe(struct snd_soc_component *component)
>   	debugfs_create_bool("can_hibernate", 0444, debugfs_root, &cs35l56->base.can_hibernate);
>   	debugfs_create_bool("fw_patched", 0444, debugfs_root, &cs35l56->base.fw_patched);
>   
> +
> +	switch (cs35l56->base.type) {
> +	case 0x54:
> +	case 0x56:
> +	case 0x57:
> +		ret = snd_soc_add_component_controls(component, cs35l56_controls,
> +						     ARRAY_SIZE(cs35l56_controls));
> +		break;
> +	default:
> +		ret = -ENODEV;
> +		break;
> +	}
> +
> +	if (ret)
> +		return dev_err_probe(cs35l56->base.dev, ret, "unable to add controls\n");
> +
>   	queue_work(cs35l56->dsp_wq, &cs35l56->dsp_work);
>   
>   	return 0;
> @@ -932,8 +949,6 @@ static const struct snd_soc_component_driver soc_component_dev_cs35l56 = {
>   	.num_dapm_widgets = ARRAY_SIZE(cs35l56_dapm_widgets),
>   	.dapm_routes = cs35l56_audio_map,
>   	.num_dapm_routes = ARRAY_SIZE(cs35l56_audio_map),
> -	.controls = cs35l56_controls,
> -	.num_controls = ARRAY_SIZE(cs35l56_controls),
>   
>   	.set_bias_level = cs35l56_set_bias_level,
>   
Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>