Message-ID: <20250612121428.1667-3-rf@opensource.cirrus.com>
Date: Thu, 12 Jun 2025 13:14:27 +0100
From: Richard Fitzgerald <rf@...nsource.cirrus.com>
To: broonie@...nel.org
Cc: linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
        patches@...nsource.cirrus.com
Subject: [PATCH 2/3] ASoC: cs35l56: Use SoundWire address as alternate firmware suffix on L56 B0

Use the SoundWire link number and device unique ID as the firmware file
qualifier suffix on CS35L56 B0 if .bin files are not found with the older
suffix. Some changes in wm_adsp needed to support this have been included
in this patch because they are trivial.

The allows future products with CS35L56 B0 silicon to use the same firmware
file naming as CS35L57 and cs35L63, while retaining backward compatibility
for firmware that has already been published with the old naming scheme.

The old suffix is searched first, partly because there are already many
files using that naming scheme, but also because they are a smaller subset
of all the possible fallback name options offered by wm_adsp so we know
that it will either find the qualified files or fail. All the firmware
files already published have the wmfw qualified with only the ACPI SSID and
the bin files qualified with both SSID and the suffix.

Originally, the firmware file names indicated which amplifier instance they
were for by appending the ALSA prefix string. This is the standard ASoC way
of distinguishing different instances of the same device. However, on
SoundWire systems the SoundWire physical unique address is available as a
unique identifier for each amp, and this address is hardwired by the
address pin on the amp.

The firmware files are specific for each physical amp so they must be
applied to that amp. Using the ALSA prefix for the filename qualifier means
that to name a firmware file it must be determined what prefix string the
machine driver will assign to each device and then use that to name the
firmware file correctly. This is straightforward in traditional ASoC
systems where the machine driver is specific to a particular piece of
hardware. But on SoundWire the machine driver is generic and can handle a
very wide range of hardware. It is more difficult to determine exactly what
the prefix will be on any particular production device, and more prone to
mistakes. Also, when the machine driver switches to generating this
automatically from SDCA properties in ACPI, there is an additional layer of
complexity in determining the mapping. This uncertainty is unnecessary
because the firmware is built for a specific amp. with known address, so we
can use that directly instead of introducing a redundant intermediate
alias. This ensures the firmware is applied to the amp it was intended for.

There are already many published firmware for CS35L56 B0 silicon so this
first looks for the original name suffix, to keep backward compatibility.
If this doesn't find .bin files it will switch to using the new name suffix
so that future products using CS35L56 B0 can start to use the new suffix.

Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
---
 sound/soc/codecs/cs35l56.c | 59 +++++++++++++++++++++++++++-----------
 sound/soc/codecs/cs35l56.h |  1 +
 sound/soc/codecs/wm_adsp.c |  6 ++++
 sound/soc/codecs/wm_adsp.h |  1 +
 4 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
index 6e6120c39965..1b42586794ad 100644
--- a/sound/soc/codecs/cs35l56.c
+++ b/sound/soc/codecs/cs35l56.c
@@ -706,16 +706,40 @@ static int cs35l56_write_cal(struct cs35l56_private *cs35l56)
 	return ret;
 }
 
+static int cs35l56_dsp_download_and_power_up(struct cs35l56_private *cs35l56,
+					     bool load_firmware)
+{
+	int ret;
+
+	/*
+	 * Abort the first load if it didn't find the suffixed bins and
+	 * we have an alternate fallback suffix.
+	 */
+	cs35l56->dsp.bin_mandatory = (load_firmware && cs35l56->fallback_fw_suffix);
+
+	ret = wm_adsp_power_up(&cs35l56->dsp, load_firmware);
+	if ((ret == -ENOENT) && cs35l56->dsp.bin_mandatory) {
+		cs35l56->dsp.fwf_suffix = cs35l56->fallback_fw_suffix;
+		cs35l56->fallback_fw_suffix = NULL;
+		cs35l56->dsp.bin_mandatory = false;
+		ret = wm_adsp_power_up(&cs35l56->dsp, load_firmware);
+	}
+
+	if (ret) {
+		dev_dbg(cs35l56->base.dev, "wm_adsp_power_up ret %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static void cs35l56_reinit_patch(struct cs35l56_private *cs35l56)
 {
 	int ret;
 
-	/* Use wm_adsp to load and apply the firmware patch and coefficient files */
-	ret = wm_adsp_power_up(&cs35l56->dsp, true);
-	if (ret) {
-		dev_dbg(cs35l56->base.dev, "%s: wm_adsp_power_up ret %d\n", __func__, ret);
+	ret = cs35l56_dsp_download_and_power_up(cs35l56, true);
+	if (ret)
 		return;
-	}
 
 	cs35l56_write_cal(cs35l56);
 
@@ -750,11 +774,9 @@ static void cs35l56_patch(struct cs35l56_private *cs35l56, bool firmware_missing
 	 * but only if firmware is missing. If firmware is already patched just
 	 * power-up wm_adsp without downloading firmware.
 	 */
-	ret = wm_adsp_power_up(&cs35l56->dsp, !!firmware_missing);
-	if (ret) {
-		dev_dbg(cs35l56->base.dev, "%s: wm_adsp_power_up ret %d\n", __func__, ret);
+	ret = cs35l56_dsp_download_and_power_up(cs35l56, firmware_missing);
+	if (ret)
 		goto err;
-	}
 
 	mutex_lock(&cs35l56->base.irq_lock);
 
@@ -861,13 +883,6 @@ static int cs35l56_set_fw_suffix(struct cs35l56_private *cs35l56)
 	if (!cs35l56->sdw_peripheral)
 		return 0;
 
-	/*
-	 * There are published firmware files for L56 B0 silicon using
-	 * the default wm_adsp name suffixing so don't change those.
-	 */
-	if ((cs35l56->base.type == 0x56) && (cs35l56->base.rev == 0xb0))
-		return 0;
-
 	cs35l56->dsp.fwf_suffix = devm_kasprintf(cs35l56->base.dev, GFP_KERNEL,
 						 "l%uu%u",
 						 cs35l56->sdw_link_num,
@@ -875,6 +890,16 @@ static int cs35l56_set_fw_suffix(struct cs35l56_private *cs35l56)
 	if (!cs35l56->dsp.fwf_suffix)
 		return -ENOMEM;
 
+	/*
+	 * There are published firmware files for L56 B0 silicon using
+	 * the ALSA prefix as the filename suffix. Default to trying these
+	 * first, with the new name as an alternate.
+	 */
+	if ((cs35l56->base.type == 0x56) && (cs35l56->base.rev == 0xb0)) {
+		cs35l56->fallback_fw_suffix = cs35l56->dsp.fwf_suffix;
+		cs35l56->dsp.fwf_suffix = cs35l56->component->name_prefix;
+	}
+
 	return 0;
 }
 
@@ -916,11 +941,11 @@ static int cs35l56_component_probe(struct snd_soc_component *component)
 	if (!cs35l56->dsp.part)
 		return -ENOMEM;
 
+	cs35l56->component = component;
 	ret = cs35l56_set_fw_suffix(cs35l56);
 	if (ret)
 		return ret;
 
-	cs35l56->component = component;
 	wm_adsp2_component_probe(&cs35l56->dsp, component);
 
 	debugfs_create_bool("init_done", 0444, debugfs_root, &cs35l56->base.init_done);
diff --git a/sound/soc/codecs/cs35l56.h b/sound/soc/codecs/cs35l56.h
index a84c83eb2d7c..bd77a57249d7 100644
--- a/sound/soc/codecs/cs35l56.h
+++ b/sound/soc/codecs/cs35l56.h
@@ -38,6 +38,7 @@ struct cs35l56_private {
 	struct snd_soc_component *component;
 	struct regulator_bulk_data supplies[CS35L56_NUM_BULK_SUPPLIES];
 	struct sdw_slave *sdw_peripheral;
+	const char *fallback_fw_suffix;
 	struct work_struct sdw_irq_work;
 	bool sdw_irq_no_unmask;
 	bool soft_resetting;
diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 13db9c6650d5..8a1d5cc75d6c 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -1000,11 +1000,17 @@ int wm_adsp_power_up(struct wm_adsp *dsp, bool load_firmware)
 			return ret;
 	}
 
+	if (dsp->bin_mandatory && !coeff_firmware) {
+		ret = -ENOENT;
+		goto err;
+	}
+
 	ret = cs_dsp_power_up(&dsp->cs_dsp,
 			      wmfw_firmware, wmfw_filename,
 			      coeff_firmware, coeff_filename,
 			      wm_adsp_fw_text[dsp->fw]);
 
+err:
 	wm_adsp_release_firmware_files(dsp,
 				       wmfw_firmware, wmfw_filename,
 				       coeff_firmware, coeff_filename);
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 075ea0bd06eb..25210d404bf1 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -36,6 +36,7 @@ struct wm_adsp {
 
 	int fw;
 	bool wmfw_optional;
+	bool bin_mandatory;
 
 	struct work_struct boot_work;
 	int (*control_add)(struct wm_adsp *dsp, struct cs_dsp_coeff_ctl *cs_ctl);
-- 
2.43.0

Powered by blists - more mailing lists