2022-03-03 16:32:51

by Simon Trimmer

[permalink] [raw]
Subject: [PATCH] ASoC: wm_adsp: Expand firmware loading search options

The parts supported by this driver can have product-specific
firmware and tunings files. Typically these have been used on
embedded systems where the manufacturer is responsible for
installing the correct product-specific firmware files into
/lib/firmware. However, the linux-firmware repository places all
available firmwares into /lib/firmware and it is up to the driver to
select the correct product-specific firmware from that directory.

For example a product containing four smart amplifiers may provide
firmware specific for that product and each of the amplifiers may
have coefficient files containing tunings for their placement in the
mechanical design.

This change extends firmware (wmfw) and coefficient (bin) filenames
to be of the general form:

<cirrus/>part-dspN-fwtype<-system_name<-asoc_component_prefix>>.type

Where the cirrus subdirectory, system_name and asoc_component_prefix
are optional.

New files will be placed in the cirrus subdirectory to avoid
polluting the main /lib/firmware/ location. The generic name must be
searched in /lib/firmware before /lib/firmware/cirrus so that a
generic file in the new location does not override existing
product-specific files in the legacy location.

The search order for firmware files is:
- cirrus/part-dspN-fwtype-system_name-asoc_component_prefix.wmfw
- cirrus/part-dspN-fwtype-system_name.wmfw
- part-dspN-fwtype.wmfw
- cirrus/part-dspN-fwtype.wmfw

- Qualifications are added to the filename so that rightwards is more
specific.
- The system_name is provided by the codec driver.
- The asoc_component_prefix is used to identify tunings for individual
parts because it would already exist to disambiguate the controls
and it makes it obvious which firmware file applies to which device.

The optional coefficient file must have the same filename
construction as the discovered wmfw except:
- where the wmfw has only system_name then the bin file can
optionally include the asoc_component_prefix. This is to allow a
common wmfw for all amps but separate tunings per amp.

Signed-off-by: Simon Trimmer <[email protected]>
---
sound/soc/codecs/wm_adsp.c | 99 +++++++++++++++++++++++++++++++++-----
sound/soc/codecs/wm_adsp.h | 1 +
2 files changed, 88 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c
index 0582585236a2..5ba88c5272d0 100644
--- a/sound/soc/codecs/wm_adsp.c
+++ b/sound/soc/codecs/wm_adsp.c
@@ -744,21 +744,48 @@ static void wm_adsp_release_firmware_files(struct wm_adsp *dsp,
}

static int wm_adsp_request_firmware_file(struct wm_adsp *dsp,
- const struct firmware **firmware,
- char **filename,
- char *suffix)
+ const struct firmware **firmware, char **filename,
+ const char *dir, const char *system_name,
+ const char *asoc_component_prefix,
+ const char *filetype)
{
struct cs_dsp *cs_dsp = &dsp->cs_dsp;
+ char *s, c;
int ret = 0;

- *filename = kasprintf(GFP_KERNEL, "%s-%s-%s.%s", dsp->part, dsp->fwf_name,
- wm_adsp_fw[dsp->fw].file, suffix);
+ if (system_name && asoc_component_prefix)
+ *filename = kasprintf(GFP_KERNEL, "%s%s-%s-%s-%s-%s.%s", dir, dsp->part,
+ dsp->fwf_name, wm_adsp_fw[dsp->fw].file, system_name,
+ asoc_component_prefix, filetype);
+ else if (system_name)
+ *filename = kasprintf(GFP_KERNEL, "%s%s-%s-%s-%s.%s", dir, dsp->part,
+ dsp->fwf_name, wm_adsp_fw[dsp->fw].file, system_name,
+ filetype);
+ else
+ *filename = kasprintf(GFP_KERNEL, "%s%s-%s-%s.%s", dir, dsp->part, dsp->fwf_name,
+ wm_adsp_fw[dsp->fw].file, filetype);
+
if (*filename == NULL)
return -ENOMEM;

- ret = request_firmware(firmware, *filename, cs_dsp->dev);
+ /*
+ * Make sure that filename is lower-case and any non alpha-numeric
+ * characters except full stop and forward slash are replaced with
+ * hyphens.
+ */
+ s = *filename;
+ while (*s) {
+ c = *s;
+ if (isalnum(c))
+ *s = tolower(c);
+ else if ((c != '.') && (c != '/'))
+ *s = '-';
+ s++;
+ }
+
+ ret = firmware_request_nowarn(firmware, *filename, cs_dsp->dev);
if (ret != 0) {
- adsp_err(dsp, "Failed to request '%s'\n", *filename);
+ adsp_dbg(dsp, "Failed to request '%s'\n", *filename);
kfree(*filename);
*filename = NULL;
}
@@ -766,21 +793,69 @@ static int wm_adsp_request_firmware_file(struct wm_adsp *dsp,
return ret;
}

+static const char *cirrus_dir = "cirrus/";
static int wm_adsp_request_firmware_files(struct wm_adsp *dsp,
const struct firmware **wmfw_firmware,
char **wmfw_filename,
const struct firmware **coeff_firmware,
char **coeff_filename)
{
+ const char *system_name = dsp->system_name;
+ const char *asoc_component_prefix = dsp->component->name_prefix;
int ret = 0;

- ret = wm_adsp_request_firmware_file(dsp, wmfw_firmware, wmfw_filename, "wmfw");
- if (ret != 0)
- return ret;
+ if (system_name && asoc_component_prefix) {
+ if (!wm_adsp_request_firmware_file(dsp, wmfw_firmware, wmfw_filename,
+ cirrus_dir, system_name,
+ asoc_component_prefix, "wmfw")) {
+ adsp_dbg(dsp, "Found '%s'\n", *wmfw_filename);
+ wm_adsp_request_firmware_file(dsp, coeff_firmware, coeff_filename,
+ cirrus_dir, system_name,
+ asoc_component_prefix, "bin");
+ return 0;
+ }
+ }

- wm_adsp_request_firmware_file(dsp, coeff_firmware, coeff_filename, "bin");
+ if (system_name) {
+ if (!wm_adsp_request_firmware_file(dsp, wmfw_firmware, wmfw_filename,
+ cirrus_dir, system_name,
+ NULL, "wmfw")) {
+ adsp_dbg(dsp, "Found '%s'\n", *wmfw_filename);
+ if (asoc_component_prefix)
+ wm_adsp_request_firmware_file(dsp, coeff_firmware, coeff_filename,
+ cirrus_dir, system_name,
+ asoc_component_prefix, "bin");
+
+ if (!*coeff_firmware)
+ wm_adsp_request_firmware_file(dsp, coeff_firmware, coeff_filename,
+ cirrus_dir, system_name,
+ NULL, "bin");
+ return 0;
+ }
+ }

- return 0;
+ if (!wm_adsp_request_firmware_file(dsp, wmfw_firmware, wmfw_filename,
+ "", NULL, NULL, "wmfw")) {
+ adsp_dbg(dsp, "Found '%s'\n", *wmfw_filename);
+ wm_adsp_request_firmware_file(dsp, coeff_firmware, coeff_filename,
+ "", NULL, NULL, "bin");
+ return 0;
+ }
+
+ ret = wm_adsp_request_firmware_file(dsp, wmfw_firmware, wmfw_filename,
+ cirrus_dir, NULL, NULL, "wmfw");
+ if (!ret) {
+ adsp_dbg(dsp, "Found '%s'\n", *wmfw_filename);
+ wm_adsp_request_firmware_file(dsp, coeff_firmware, coeff_filename,
+ cirrus_dir, NULL, NULL, "bin");
+ return 0;
+ }
+
+ adsp_err(dsp, "Failed to request firmware <%s>%s-%s-%s<-%s<%s>>.wmfw\n",
+ cirrus_dir, dsp->part, dsp->fwf_name, wm_adsp_fw[dsp->fw].file,
+ system_name, asoc_component_prefix);
+
+ return -ENOENT;
}

static int wm_adsp_common_init(struct wm_adsp *dsp)
diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index 7f4fabbc6ad3..375009a65828 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -28,6 +28,7 @@ struct wm_adsp {
struct cs_dsp cs_dsp;
const char *part;
const char *fwf_name;
+ const char *system_name;
struct snd_soc_component *component;

unsigned int sys_config_size;
--
2.34.1


2022-03-03 18:16:08

by Charles Keepax

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm_adsp: Expand firmware loading search options

On Thu, Mar 03, 2022 at 03:50:16PM +0000, Simon Trimmer wrote:
> The parts supported by this driver can have product-specific
> firmware and tunings files. Typically these have been used on
> embedded systems where the manufacturer is responsible for
> installing the correct product-specific firmware files into
> /lib/firmware. However, the linux-firmware repository places all
> available firmwares into /lib/firmware and it is up to the driver to
> select the correct product-specific firmware from that directory.
>
> For example a product containing four smart amplifiers may provide
> firmware specific for that product and each of the amplifiers may
> have coefficient files containing tunings for their placement in the
> mechanical design.
>
> This change extends firmware (wmfw) and coefficient (bin) filenames
> to be of the general form:
>
> <cirrus/>part-dspN-fwtype<-system_name<-asoc_component_prefix>>.type
>
> Where the cirrus subdirectory, system_name and asoc_component_prefix
> are optional.
>
> New files will be placed in the cirrus subdirectory to avoid
> polluting the main /lib/firmware/ location. The generic name must be
> searched in /lib/firmware before /lib/firmware/cirrus so that a
> generic file in the new location does not override existing
> product-specific files in the legacy location.
>
> The search order for firmware files is:
> - cirrus/part-dspN-fwtype-system_name-asoc_component_prefix.wmfw
> - cirrus/part-dspN-fwtype-system_name.wmfw
> - part-dspN-fwtype.wmfw
> - cirrus/part-dspN-fwtype.wmfw
>
> - Qualifications are added to the filename so that rightwards is more
> specific.
> - The system_name is provided by the codec driver.
> - The asoc_component_prefix is used to identify tunings for individual
> parts because it would already exist to disambiguate the controls
> and it makes it obvious which firmware file applies to which device.
>
> The optional coefficient file must have the same filename
> construction as the discovered wmfw except:
> - where the wmfw has only system_name then the bin file can
> optionally include the asoc_component_prefix. This is to allow a
> common wmfw for all amps but separate tunings per amp.
>
> Signed-off-by: Simon Trimmer <[email protected]>
> ---

Acked-by: Charles Keepax <[email protected]>

Thanks,
Charles

2022-03-09 01:21:24

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] ASoC: wm_adsp: Expand firmware loading search options

On Thu, 3 Mar 2022 15:50:16 +0000, Simon Trimmer wrote:
> The parts supported by this driver can have product-specific
> firmware and tunings files. Typically these have been used on
> embedded systems where the manufacturer is responsible for
> installing the correct product-specific firmware files into
> /lib/firmware. However, the linux-firmware repository places all
> available firmwares into /lib/firmware and it is up to the driver to
> select the correct product-specific firmware from that directory.
>
> [...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: wm_adsp: Expand firmware loading search options
commit: b6b62d942bbc4d926bcf3799ea3bcaeb105fd04f

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark