Re: [PATCH v2 2/2] ASoC: cs35l56: Allow factory calibration through ALSA controls

[Mark Brown]

On Wed, Mar 25, 2026 at 05:08:41PM +0000, Richard Fitzgerald wrote:
> Add support for using ALSA controls to trigger a factory calibration.
> This is protected by a new Kconfig option so that it is only available
> if explicitly enabled in the kernel. By default it is not enabled.

> Factory calibration is normally done through debugfs files.
> Google have requested that factory calibration can be performed by
> repair shops. These repair shops only have access to the standard
> "user" kernel, which does not include debugfs.

AFAICT this doesn't have an EFI write command like the debugfs code does
(when you write "store_uefi").  I don't mind, just wanted to double
check that this is desirable?

> 
> Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> ---
> Changes in V2:
> - Rename "Calibrate" control to "Calibrate Switch" to conform to naming
>   convention for boolean controls.
> - Mark "Calibrate Switch" control volatile.
> - Cache the value written to the "CAL_AMBIENT" control so that a read will
>   return the last set value.
> - Return 1 from writing "CAL_AMBIENT" if the value was changed.
> - Return 1 from writing the "Calibrate Switch" control to true because
>   that always causes some activity in the amp.
> - Replace use of the confusing in_range() with normal comparisons against
>   limits and make the limits the same as the control definition.
> - Simplify the code in cs35l56_calibrate_ctl_set(). It's a boolean so the
>   value written to it is either false or true.
> 
>  include/sound/cs35l56.h           |  1 +
>  sound/soc/codecs/Kconfig          | 13 +++++
>  sound/soc/codecs/cs35l56-shared.c |  9 +++
>  sound/soc/codecs/cs35l56.c        | 96 +++++++++++++++++++++++++++++++
>  sound/soc/codecs/cs35l56.h        |  2 +
>  5 files changed, 121 insertions(+)
> 
> diff --git a/include/sound/cs35l56.h b/include/sound/cs35l56.h
> index 7ca25487030a..c3b10587cb4c 100644
> --- a/include/sound/cs35l56.h
> +++ b/include/sound/cs35l56.h
> @@ -435,6 +435,7 @@ ssize_t cs35l56_cal_data_debugfs_read(struct cs35l56_base *cs35l56_base,
>  ssize_t cs35l56_cal_data_debugfs_write(struct cs35l56_base *cs35l56_base,
>  				       const char __user *from, size_t count,
>  				       loff_t *ppos);
> +int cs35l56_factory_calibrate(struct cs35l56_base *cs35l56_base);
>  void cs35l56_create_cal_debugfs(struct cs35l56_base *cs35l56_base,
>  				const struct cs35l56_cal_debugfs_fops *fops);
>  void cs35l56_remove_cal_debugfs(struct cs35l56_base *cs35l56_base);
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index d6104796db4f..ca3e47db126e 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -921,6 +921,19 @@ config SND_SOC_CS35L56_CAL_SET_CTRL
>  
>  	  If unsure select "N".
>  
> +config SND_SOC_CS35L56_CAL_PERFORM_CTRL
> +	bool "CS35L56 ALSA control to perform factory calibration"
> +	default N
> +	select SND_SOC_CS35L56_CAL_DEBUGFS_COMMON
> +	help
> +	  Allow performing factory calibration data through an ALSA
> +	  control. It is recommended to use the debugfs method instead
> +	  because debugfs has restricted access permissions.
> +
> +	  On most platforms this is not needed.
> +
> +	  If unsure select "N".
> +
>  config SND_SOC_CS35L56_TEST
>  	tristate "KUnit test for Cirrus Logic cs35l56 driver" if !KUNIT_ALL_TESTS
>  	depends on SND_SOC_CS35L56 && KUNIT
> diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c
> index af87ebae98cb..e05d975ba794 100644
> --- a/sound/soc/codecs/cs35l56-shared.c
> +++ b/sound/soc/codecs/cs35l56-shared.c
> @@ -1185,6 +1185,15 @@ ssize_t cs35l56_calibrate_debugfs_write(struct cs35l56_base *cs35l56_base,
>  }
>  EXPORT_SYMBOL_NS_GPL(cs35l56_calibrate_debugfs_write, "SND_SOC_CS35L56_SHARED");
>  
> +int cs35l56_factory_calibrate(struct cs35l56_base *cs35l56_base)
> +{
> +	if (!IS_ENABLED(CONFIG_SND_SOC_CS35L56_CAL_PERFORM_CTRL))
> +		return -ENXIO;
> +
> +	return cs35l56_perform_calibration(cs35l56_base);
> +}
> +EXPORT_SYMBOL_NS_GPL(cs35l56_factory_calibrate, "SND_SOC_CS35L56_SHARED");
> +
>  ssize_t cs35l56_cal_ambient_debugfs_write(struct cs35l56_base *cs35l56_base,
>  					  const char __user *from, size_t count,
>  					  loff_t *ppos)
> diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
> index 9d35797e000a..378017fcea10 100644
> --- a/sound/soc/codecs/cs35l56.c
> +++ b/sound/soc/codecs/cs35l56.c
> @@ -1109,6 +1109,88 @@ static int cs35l56_cal_data_ctl_set(struct snd_kcontrol *kcontrol,
>  	return 1;
>  }
>  
> +static int cs35l56_cal_ambient_ctl_get(struct snd_kcontrol *kcontrol,
> +				       struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(component);
> +
> +	ucontrol->value.integer.value[0] = cs35l56->ambient_ctl_value;
> +
> +	return 0;
> +}
> +
> +static int cs35l56_cal_ambient_ctl_set(struct snd_kcontrol *kcontrol,
> +				       struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(component);
> +	struct snd_soc_dapm_context *dapm;
> +	int temperature = ucontrol->value.integer.value[0];
> +	int ret;
> +
> +	if (temperature == cs35l56->ambient_ctl_value)
> +		return 0;
> +
> +	if ((temperature < 0) || (temperature > 40))
> +		return -EINVAL;
> +
> +	dapm = cs35l56_power_up_for_cal(cs35l56);
> +	if (IS_ERR(dapm))
> +		return PTR_ERR(dapm);
> +
> +	ret = cs_amp_write_ambient_temp(&cs35l56->dsp.cs_dsp,
> +					cs35l56->base.calibration_controls,
> +					temperature);
> +	cs35l56_power_down_after_cal(cs35l56);
> +
> +	if (ret)
> +		return ret;
> +
> +	cs35l56->ambient_ctl_value = temperature;
> +
> +	return 1;
> +}
> +
> +static int cs35l56_calibrate_ctl_get(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	/*
> +	 * Allow reading because of user-side libraries that assume all
> +	 * controls are readable. But always return false to prevent dumb
> +	 * save-restore tools like alsactl accidentically triggering a
> +	 * factory calibration when they restore.
> +	 */
> +	ucontrol->value.integer.value[0] = 0;
> +
> +	return 0;
> +}
> +
> +static int cs35l56_calibrate_ctl_set(struct snd_kcontrol *kcontrol,
> +				     struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(component);
> +	struct snd_soc_dapm_context *dapm;
> +	int ret;
> +
> +	if (ucontrol->value.integer.value[0] == 0)
> +		return 0;
> +
> +	dapm = cs35l56_power_up_for_cal(cs35l56);
> +	if (IS_ERR(dapm))
> +		return PTR_ERR(dapm);
> +
> +	snd_soc_dapm_mutex_lock(dapm);
> +	ret = cs35l56_factory_calibrate(&cs35l56->base);
> +	snd_soc_dapm_mutex_unlock(dapm);
> +	cs35l56_power_down_after_cal(cs35l56);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 1;
> +}
> +
>  static const struct snd_kcontrol_new cs35l56_cal_data_restore_controls[] = {
>  	SND_SOC_BYTES_E("CAL_DATA", 0, sizeof(struct cirrus_amp_cal_data) / sizeof(u32),
>  			cs35l56_cal_data_ctl_get, cs35l56_cal_data_ctl_set),
> @@ -1117,6 +1199,14 @@ static const struct snd_kcontrol_new cs35l56_cal_data_restore_controls[] = {
>  			SNDRV_CTL_ELEM_ACCESS_READ | SNDRV_CTL_ELEM_ACCESS_VOLATILE),
>  };
>  
> +static const struct snd_kcontrol_new cs35l56_cal_perform_controls[] = {
> +	SOC_SINGLE_EXT("CAL_AMBIENT", SND_SOC_NOPM, 0, 40, 0,
> +		       cs35l56_cal_ambient_ctl_get, cs35l56_cal_ambient_ctl_set),
> +	SOC_SINGLE_BOOL_EXT_ACC("Calibrate Switch", 0,
> +				cs35l56_calibrate_ctl_get, cs35l56_calibrate_ctl_set,
> +				SNDRV_CTL_ELEM_ACCESS_READWRITE | SNDRV_CTL_ELEM_ACCESS_VOLATILE),
> +};
> +
>  VISIBLE_IF_KUNIT int cs35l56_set_fw_suffix(struct cs35l56_private *cs35l56)
>  {
>  	unsigned short vendor, device;
> @@ -1290,6 +1380,12 @@ static int cs35l56_component_probe(struct snd_soc_component *component)
>  						     ARRAY_SIZE(cs35l56_cal_data_restore_controls));
>  	}
>  
> +	if (!ret && IS_ENABLED(CONFIG_SND_SOC_CS35L56_CAL_PERFORM_CTRL)) {
> +		ret = snd_soc_add_component_controls(component,
> +						     cs35l56_cal_perform_controls,
> +						     ARRAY_SIZE(cs35l56_cal_perform_controls));
> +	}
> +
>  	if (ret)
>  		return dev_err_probe(cs35l56->base.dev, ret, "unable to add controls\n");
>  
> diff --git a/sound/soc/codecs/cs35l56.h b/sound/soc/codecs/cs35l56.h
> index 36d239d571cd..cd71b23b2a3a 100644
> --- a/sound/soc/codecs/cs35l56.h
> +++ b/sound/soc/codecs/cs35l56.h
> @@ -54,6 +54,8 @@ struct cs35l56_private {
>  	bool sysclk_set;
>  	u8 sdw_link_num;
>  	u8 sdw_unique_id;
> +
> +	u8 ambient_ctl_value;
>  };
>  
>  static inline struct cs35l56_private *cs35l56_private_from_base(struct cs35l56_base *cs35l56_base)
> -- 
> 2.47.3
>

Attachment: signature.asc
Description: PGP signature