Create sysfs files that can be used to perform factory calibration.
During manufacture, the production line must perform a factory calibration
of the amps. This patch adds this functionality via sysfs files.
Sysfs is used here to restrict access to the factory calibration.
It is only intended to be used during manufacture. It is not something
that a normal user should ever touch. Calibration affects the matching of
the amp hardware to the external speakers. If not done correctly it can
cause the speakers to be under-protected.
As this is only needed during manufacture, there is no need for this to be
available in a normal system so a Kconfig item has been added to enable
this. The new Kconfig option is inside a sub-menu because items do not
group and indent if the parent is invisible or there are multiple parent
dependencies. Anyway the sub-menu reduces the clutter.
cs35l56_hda_apply_calibration() has been changed to return an error code
that can be reported back through the sysfs write. The original call to
this function doesn't check the return code because in normal use it
doesn't matter whether this fails - the firmware will default to a safe
calibration for the platform. But tooling using sysfs might want to know
if there was an error.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
sound/hda/codecs/side-codecs/Kconfig | 15 +++
sound/hda/codecs/side-codecs/cs35l56_hda.c | 139 ++++++++++++++++++++-
2 files changed, 149 insertions(+), 5 deletions(-)
diff --git a/sound/hda/codecs/side-codecs/Kconfig b/sound/hda/codecs/side-codecs/Kconfig
index cbf1847896bc..0218aa41bba2 100644
--- a/sound/hda/codecs/side-codecs/Kconfig
+++ b/sound/hda/codecs/side-codecs/Kconfig
@@ -88,6 +88,21 @@ config SND_HDA_SCODEC_CS35L56_SPI
Say Y or M here to include CS35L56 amplifier support with
SPI control.
+menu "CS35L56 driver options"
+ depends on SND_HDA_SCODEC_CS35L56
+
+config SND_HDA_SCODEC_CS35L56_CAL_SYSFS
+ bool "CS35L56 create sysfs for factory calibration"
+ default N
+ select SND_SOC_CS35L56_CAL_SYSFS_COMMON
+ help
+ Create sysfs entries used during factory-line manufacture
+ for factory calibration.
+ This is not needed for normal use.
+
+ If unsure select "N".
+endmenu
+
config SND_HDA_SCODEC_TAS2781
tristate
select SND_HDA_GENERIC
diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
index 5bb1c4ebeaf3..4a1bd934887a 100644
--- a/sound/hda/codecs/side-codecs/cs35l56_hda.c
+++ b/sound/hda/codecs/side-codecs/cs35l56_hda.c
@@ -548,20 +548,24 @@ static void cs35l56_hda_release_firmware_files(const struct firmware *wmfw_firmw
kfree(coeff_filename);
}
-static void cs35l56_hda_apply_calibration(struct cs35l56_hda *cs35l56)
+static int cs35l56_hda_apply_calibration(struct cs35l56_hda *cs35l56)
{
int ret;
if (!cs35l56->base.cal_data_valid || cs35l56->base.secured)
- return;
+ return -EACCES;
ret = cs_amp_write_cal_coeffs(&cs35l56->cs_dsp,
&cs35l56_calibration_controls,
&cs35l56->base.cal_data);
- if (ret < 0)
+ if (ret < 0) {
dev_warn(cs35l56->base.dev, "Failed to write calibration: %d\n", ret);
- else
- dev_info(cs35l56->base.dev, "Calibration applied\n");
+ return ret;
+ }
+
+ dev_info(cs35l56->base.dev, "Calibration applied\n");
+
+ return 0;
}
static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
@@ -669,7 +673,9 @@ static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
if (ret)
dev_dbg(cs35l56->base.dev, "%s: cs_dsp_run ret %d\n", __func__, ret);
+ /* Don't need to check return code, it's not fatal if this fails */
cs35l56_hda_apply_calibration(cs35l56);
+
ret = cs35l56_mbox_send(&cs35l56->base, CS35L56_MBOX_CMD_AUDIO_REINIT);
if (ret)
cs_dsp_stop(&cs35l56->cs_dsp);
@@ -695,6 +701,126 @@ static void cs35l56_hda_dsp_work(struct work_struct *work)
cs35l56_hda_fw_load(cs35l56);
}
+static ssize_t calibrate_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
+ ssize_t ret;
+
+ ret = pm_runtime_resume_and_get(cs35l56->base.dev);
+ if (ret)
+ return ret;
+
+ ret = cs35l56_calibrate_sysfs_store(&cs35l56->base, buf, count);
+ pm_runtime_autosuspend(cs35l56->base.dev);
+
+ return ret;
+}
+
+static ssize_t cal_temperature_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
+ ssize_t ret;
+
+ ret = pm_runtime_resume_and_get(cs35l56->base.dev);
+ if (ret)
+ return ret;
+
+ ret = cs35l56_cal_ambient_sysfs_store(&cs35l56->base, buf, count);
+ pm_runtime_autosuspend(cs35l56->base.dev);
+
+ return ret;
+}
+
+static ssize_t cal_data_read(struct file *filp, struct kobject *kobj,
+ const struct bin_attribute *battr, char *buf, loff_t pos,
+ size_t count)
+{
+ struct cs35l56_hda *cs35l56 = dev_get_drvdata(kobj_to_dev(kobj));
+ ssize_t ret;
+
+ ret = pm_runtime_resume_and_get(cs35l56->base.dev);
+ if (ret)
+ return ret;
+
+ ret = cs35l56_cal_data_sysfs_read(&cs35l56->base, buf, pos, count);
+ pm_runtime_autosuspend(cs35l56->base.dev);
+
+ return ret;
+}
+
+static ssize_t cal_data_write(struct file *filp, struct kobject *kobj,
+ const struct bin_attribute *battr, char *buf, loff_t pos,
+ size_t count)
+{
+ struct cs35l56_hda *cs35l56 = dev_get_drvdata(kobj_to_dev(kobj));
+ ssize_t ret;
+
+ ret = cs35l56_cal_data_sysfs_write(&cs35l56->base, buf, pos, count);
+ if (ret == -ENODATA)
+ return count; /* Ignore writes of empty cal blobs */
+
+ if (ret < 0)
+ return ret;
+
+ ret = pm_runtime_resume_and_get(cs35l56->base.dev);
+ if (ret)
+ return ret;
+
+ ret = cs35l56_hda_apply_calibration(cs35l56);
+ if (ret == 0)
+ cs35l56_mbox_send(&cs35l56->base, CS35L56_MBOX_CMD_AUDIO_REINIT);
+ else
+ count = -EIO;
+
+ pm_runtime_autosuspend(cs35l56->base.dev);
+
+ return count;
+}
+
+static const DEVICE_ATTR_WO(calibrate);
+static const DEVICE_ATTR_WO(cal_temperature);
+static const BIN_ATTR_RW(cal_data, sizeof_field(struct cs35l56_base, cal_data));
+
+static const struct attribute *cs35l56_hda_cal_attributes[] = {
+ &dev_attr_calibrate.attr,
+ &dev_attr_cal_temperature.attr,
+ NULL
+};
+
+static void cs35l56_hda_create_calibration_sysfs(struct cs35l56_hda *cs35l56)
+{
+ struct device *dev = cs35l56->base.dev;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_SND_HDA_SCODEC_CS35L56_CAL_SYSFS))
+ return;
+
+ ret = sysfs_create_files(&dev->kobj, cs35l56_hda_cal_attributes);
+ if (ret)
+ goto err;
+
+ ret = sysfs_create_bin_file(&dev->kobj, &bin_attr_cal_data);
+ if (ret)
+ goto err;
+
+ return;
+err:
+ dev_err_probe(dev, ret, "Failed creating calibration sysfs\n");
+}
+
+static void cs35l56_hda_remove_calibration_sysfs(struct cs35l56_hda *cs35l56)
+{
+ struct device *dev = cs35l56->base.dev;
+
+ if (!IS_ENABLED(CONFIG_SND_HDA_SCODEC_CS35L56_CAL_SYSFS))
+ return;
+
+ sysfs_remove_files(&dev->kobj, cs35l56_hda_cal_attributes);
+ sysfs_remove_bin_file(&dev->kobj, &bin_attr_cal_data);
+}
+
static int cs35l56_hda_bind(struct device *dev, struct device *master, void *master_data)
{
struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
@@ -722,6 +848,8 @@ static int cs35l56_hda_bind(struct device *dev, struct device *master, void *mas
cs_dsp_init_debugfs(&cs35l56->cs_dsp, cs35l56->debugfs_root);
#endif
+ cs35l56_hda_create_calibration_sysfs(cs35l56);
+
dev_dbg(cs35l56->base.dev, "Bound\n");
return 0;
@@ -736,6 +864,7 @@ static void cs35l56_hda_unbind(struct device *dev, struct device *master, void *
cancel_work_sync(&cs35l56->dsp_work);
cs35l56_hda_remove_controls(cs35l56);
+ cs35l56_hda_remove_calibration_sysfs(cs35l56);
#if IS_ENABLED(CONFIG_SND_DEBUG)
cs_dsp_cleanup_debugfs(&cs35l56->cs_dsp);
--
2.47.3
On Thu, 16 Oct 2025 12:42:36 +0200,
Richard Fitzgerald wrote:
>
> Create sysfs files that can be used to perform factory calibration.
>
> During manufacture, the production line must perform a factory calibration
> of the amps. This patch adds this functionality via sysfs files.
>
> Sysfs is used here to restrict access to the factory calibration.
> It is only intended to be used during manufacture. It is not something
> that a normal user should ever touch. Calibration affects the matching of
> the amp hardware to the external speakers. If not done correctly it can
> cause the speakers to be under-protected.
>
> As this is only needed during manufacture, there is no need for this to be
> available in a normal system so a Kconfig item has been added to enable
> this. The new Kconfig option is inside a sub-menu because items do not
> group and indent if the parent is invisible or there are multiple parent
> dependencies. Anyway the sub-menu reduces the clutter.
>
> cs35l56_hda_apply_calibration() has been changed to return an error code
> that can be reported back through the sysfs write. The original call to
> this function doesn't check the return code because in normal use it
> doesn't matter whether this fails - the firmware will default to a safe
> calibration for the platform. But tooling using sysfs might want to know
> if there was an error.
>
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
What kind of data format should be written to calibrate sysfs entry?
Since those are no trivial files, we may need to document the
formats.
Also, the scenario isn't clear -- who should write this sysfs at which
moment? e.g. is this supposed to be set up at each boot?
thanks,
Takashi
> ---
> sound/hda/codecs/side-codecs/Kconfig | 15 +++
> sound/hda/codecs/side-codecs/cs35l56_hda.c | 139 ++++++++++++++++++++-
> 2 files changed, 149 insertions(+), 5 deletions(-)
>
> diff --git a/sound/hda/codecs/side-codecs/Kconfig b/sound/hda/codecs/side-codecs/Kconfig
> index cbf1847896bc..0218aa41bba2 100644
> --- a/sound/hda/codecs/side-codecs/Kconfig
> +++ b/sound/hda/codecs/side-codecs/Kconfig
> @@ -88,6 +88,21 @@ config SND_HDA_SCODEC_CS35L56_SPI
> Say Y or M here to include CS35L56 amplifier support with
> SPI control.
>
> +menu "CS35L56 driver options"
> + depends on SND_HDA_SCODEC_CS35L56
> +
> +config SND_HDA_SCODEC_CS35L56_CAL_SYSFS
> + bool "CS35L56 create sysfs for factory calibration"
> + default N
> + select SND_SOC_CS35L56_CAL_SYSFS_COMMON
> + help
> + Create sysfs entries used during factory-line manufacture
> + for factory calibration.
> + This is not needed for normal use.
> +
> + If unsure select "N".
> +endmenu
> +
> config SND_HDA_SCODEC_TAS2781
> tristate
> select SND_HDA_GENERIC
> diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> index 5bb1c4ebeaf3..4a1bd934887a 100644
> --- a/sound/hda/codecs/side-codecs/cs35l56_hda.c
> +++ b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> @@ -548,20 +548,24 @@ static void cs35l56_hda_release_firmware_files(const struct firmware *wmfw_firmw
> kfree(coeff_filename);
> }
>
> -static void cs35l56_hda_apply_calibration(struct cs35l56_hda *cs35l56)
> +static int cs35l56_hda_apply_calibration(struct cs35l56_hda *cs35l56)
> {
> int ret;
>
> if (!cs35l56->base.cal_data_valid || cs35l56->base.secured)
> - return;
> + return -EACCES;
>
> ret = cs_amp_write_cal_coeffs(&cs35l56->cs_dsp,
> &cs35l56_calibration_controls,
> &cs35l56->base.cal_data);
> - if (ret < 0)
> + if (ret < 0) {
> dev_warn(cs35l56->base.dev, "Failed to write calibration: %d\n", ret);
> - else
> - dev_info(cs35l56->base.dev, "Calibration applied\n");
> + return ret;
> + }
> +
> + dev_info(cs35l56->base.dev, "Calibration applied\n");
> +
> + return 0;
> }
>
> static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
> @@ -669,7 +673,9 @@ static void cs35l56_hda_fw_load(struct cs35l56_hda *cs35l56)
> if (ret)
> dev_dbg(cs35l56->base.dev, "%s: cs_dsp_run ret %d\n", __func__, ret);
>
> + /* Don't need to check return code, it's not fatal if this fails */
> cs35l56_hda_apply_calibration(cs35l56);
> +
> ret = cs35l56_mbox_send(&cs35l56->base, CS35L56_MBOX_CMD_AUDIO_REINIT);
> if (ret)
> cs_dsp_stop(&cs35l56->cs_dsp);
> @@ -695,6 +701,126 @@ static void cs35l56_hda_dsp_work(struct work_struct *work)
> cs35l56_hda_fw_load(cs35l56);
> }
>
> +static ssize_t calibrate_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
> + ssize_t ret;
> +
> + ret = pm_runtime_resume_and_get(cs35l56->base.dev);
> + if (ret)
> + return ret;
> +
> + ret = cs35l56_calibrate_sysfs_store(&cs35l56->base, buf, count);
> + pm_runtime_autosuspend(cs35l56->base.dev);
> +
> + return ret;
> +}
> +
> +static ssize_t cal_temperature_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
> + ssize_t ret;
> +
> + ret = pm_runtime_resume_and_get(cs35l56->base.dev);
> + if (ret)
> + return ret;
> +
> + ret = cs35l56_cal_ambient_sysfs_store(&cs35l56->base, buf, count);
> + pm_runtime_autosuspend(cs35l56->base.dev);
> +
> + return ret;
> +}
> +
> +static ssize_t cal_data_read(struct file *filp, struct kobject *kobj,
> + const struct bin_attribute *battr, char *buf, loff_t pos,
> + size_t count)
> +{
> + struct cs35l56_hda *cs35l56 = dev_get_drvdata(kobj_to_dev(kobj));
> + ssize_t ret;
> +
> + ret = pm_runtime_resume_and_get(cs35l56->base.dev);
> + if (ret)
> + return ret;
> +
> + ret = cs35l56_cal_data_sysfs_read(&cs35l56->base, buf, pos, count);
> + pm_runtime_autosuspend(cs35l56->base.dev);
> +
> + return ret;
> +}
> +
> +static ssize_t cal_data_write(struct file *filp, struct kobject *kobj,
> + const struct bin_attribute *battr, char *buf, loff_t pos,
> + size_t count)
> +{
> + struct cs35l56_hda *cs35l56 = dev_get_drvdata(kobj_to_dev(kobj));
> + ssize_t ret;
> +
> + ret = cs35l56_cal_data_sysfs_write(&cs35l56->base, buf, pos, count);
> + if (ret == -ENODATA)
> + return count; /* Ignore writes of empty cal blobs */
> +
> + if (ret < 0)
> + return ret;
> +
> + ret = pm_runtime_resume_and_get(cs35l56->base.dev);
> + if (ret)
> + return ret;
> +
> + ret = cs35l56_hda_apply_calibration(cs35l56);
> + if (ret == 0)
> + cs35l56_mbox_send(&cs35l56->base, CS35L56_MBOX_CMD_AUDIO_REINIT);
> + else
> + count = -EIO;
> +
> + pm_runtime_autosuspend(cs35l56->base.dev);
> +
> + return count;
> +}
> +
> +static const DEVICE_ATTR_WO(calibrate);
> +static const DEVICE_ATTR_WO(cal_temperature);
> +static const BIN_ATTR_RW(cal_data, sizeof_field(struct cs35l56_base, cal_data));
> +
> +static const struct attribute *cs35l56_hda_cal_attributes[] = {
> + &dev_attr_calibrate.attr,
> + &dev_attr_cal_temperature.attr,
> + NULL
> +};
> +
> +static void cs35l56_hda_create_calibration_sysfs(struct cs35l56_hda *cs35l56)
> +{
> + struct device *dev = cs35l56->base.dev;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_SND_HDA_SCODEC_CS35L56_CAL_SYSFS))
> + return;
> +
> + ret = sysfs_create_files(&dev->kobj, cs35l56_hda_cal_attributes);
> + if (ret)
> + goto err;
> +
> + ret = sysfs_create_bin_file(&dev->kobj, &bin_attr_cal_data);
> + if (ret)
> + goto err;
> +
> + return;
> +err:
> + dev_err_probe(dev, ret, "Failed creating calibration sysfs\n");
> +}
> +
> +static void cs35l56_hda_remove_calibration_sysfs(struct cs35l56_hda *cs35l56)
> +{
> + struct device *dev = cs35l56->base.dev;
> +
> + if (!IS_ENABLED(CONFIG_SND_HDA_SCODEC_CS35L56_CAL_SYSFS))
> + return;
> +
> + sysfs_remove_files(&dev->kobj, cs35l56_hda_cal_attributes);
> + sysfs_remove_bin_file(&dev->kobj, &bin_attr_cal_data);
> +}
> +
> static int cs35l56_hda_bind(struct device *dev, struct device *master, void *master_data)
> {
> struct cs35l56_hda *cs35l56 = dev_get_drvdata(dev);
> @@ -722,6 +848,8 @@ static int cs35l56_hda_bind(struct device *dev, struct device *master, void *mas
> cs_dsp_init_debugfs(&cs35l56->cs_dsp, cs35l56->debugfs_root);
> #endif
>
> + cs35l56_hda_create_calibration_sysfs(cs35l56);
> +
> dev_dbg(cs35l56->base.dev, "Bound\n");
>
> return 0;
> @@ -736,6 +864,7 @@ static void cs35l56_hda_unbind(struct device *dev, struct device *master, void *
> cancel_work_sync(&cs35l56->dsp_work);
>
> cs35l56_hda_remove_controls(cs35l56);
> + cs35l56_hda_remove_calibration_sysfs(cs35l56);
>
> #if IS_ENABLED(CONFIG_SND_DEBUG)
> cs_dsp_cleanup_debugfs(&cs35l56->cs_dsp);
> --
> 2.47.3
>
On Thu, Oct 16, 2025 at 01:01:41PM +0200, Takashi Iwai wrote: > Richard Fitzgerald wrote: > > Create sysfs files that can be used to perform factory calibration. > What kind of data format should be written to calibrate sysfs entry? > Since those are no trivial files, we may need to document the > formats. This feels like it might be a better fit for debugfs or possibly configfs? It's not really within the sysfs rules, and especially debugfs is a lot more relaxed about everything. > Also, the scenario isn't clear -- who should write this sysfs at which > moment? e.g. is this supposed to be set up at each boot? AIUI it's some tooling that gets run in the factory only.
On 16/10/2025 12:27 pm, Mark Brown wrote: > On Thu, Oct 16, 2025 at 01:01:41PM +0200, Takashi Iwai wrote: >> Richard Fitzgerald wrote: > >>> Create sysfs files that can be used to perform factory calibration. > >> What kind of data format should be written to calibrate sysfs entry? >> Since those are no trivial files, we may need to document the >> formats. > > This feels like it might be a better fit for debugfs or possibly > configfs? It's not really within the sysfs rules, and especially > debugfs is a lot more relaxed about everything. > debugfs is an easy change in the driver but more complex for the tooling. Unlike sysfs there's no standard layout or naming convention for the ASoC debugfs tree, so it's more troublesome to locate the amp entries. ASoC creates a debugfs root by default if DEBUGFS is enabled. But HDA doesn't have its own debugfs. There is an ALSA debugfs enabled by CONFIG_SND_DEBUG, which is normally disabled. But enabling CONFIG_SND_DEBUG has other side-effects, it enables more than debugfs. Or we could create a new debugfs node specifically for Cirrus Logic directly in /sys/kernel/debug. I'll have a look at configfs. Although it works backwards (userside must create the directory in which attributes will appear) it might have some benefits.
On Thu, Oct 16, 2025 at 12:58:15PM +0100, Richard Fitzgerald wrote: > On 16/10/2025 12:27 pm, Mark Brown wrote: > > This feels like it might be a better fit for debugfs or possibly > > configfs? It's not really within the sysfs rules, and especially > > debugfs is a lot more relaxed about everything. > debugfs is an easy change in the driver but more complex for the > tooling. Unlike sysfs there's no standard layout or naming convention > for the ASoC debugfs tree, so it's more troublesome to locate the amp > entries. > ASoC creates a debugfs root by default if DEBUGFS is enabled. But HDA > doesn't have its own debugfs. There is an ALSA debugfs enabled by > CONFIG_SND_DEBUG, which is normally disabled. But enabling > CONFIG_SND_DEBUG has other side-effects, it enables more than debugfs. > Or we could create a new debugfs node specifically for Cirrus Logic > directly in /sys/kernel/debug. Dunno about Takashi but for this application I'd be happy for you to just put something in the root of debugfs.
On Thu, 16 Oct 2025 14:13:16 +0200, Mark Brown wrote: > > On Thu, Oct 16, 2025 at 12:58:15PM +0100, Richard Fitzgerald wrote: > > On 16/10/2025 12:27 pm, Mark Brown wrote: > > > > This feels like it might be a better fit for debugfs or possibly > > > configfs? It's not really within the sysfs rules, and especially > > > debugfs is a lot more relaxed about everything. > > > debugfs is an easy change in the driver but more complex for the > > tooling. Unlike sysfs there's no standard layout or naming convention > > for the ASoC debugfs tree, so it's more troublesome to locate the amp > > entries. > > > ASoC creates a debugfs root by default if DEBUGFS is enabled. But HDA > > doesn't have its own debugfs. There is an ALSA debugfs enabled by > > CONFIG_SND_DEBUG, which is normally disabled. But enabling > > CONFIG_SND_DEBUG has other side-effects, it enables more than debugfs. > > > Or we could create a new debugfs node specifically for Cirrus Logic > > directly in /sys/kernel/debug. > > Dunno about Takashi but for this application I'd be happy for you to > just put something in the root of debugfs. I'm fine with debugfs root. The current approach with sysfs looks too generic for the need of this access, so I find debugfs would be a better fit, too. thanks, Takashi
© 2016 - 2026 Red Hat, Inc.