[alsa-devel] [PATCH 30/35] ASoC: Intel: Reuse sst_pdata::fw_name field
Cezary Rojewski
cezary.rojewski at intel.com
Sat Aug 24 12:57:40 CEST 2019
On 2019-08-23 22:20, Pierre-Louis Bossart wrote:
>
>
> On 8/22/19 2:04 PM, Cezary Rojewski wrote:
>> struct sst_pdata is equipped with fw_name field - a platform specific
>> filename for basefw module. Usage of such allows for suther
>> simplification of declaration of handlers directly involved with Skylake
>> initialization procedure.
>>
>> This change invalidates mach::fw_filename field and skl::fw_name.
>
> Again bad move. While in theory it's true that a single firmware is all
> you need, you do want to keep the ability to quirk firmware names for
> specific cases. We've been there before, don't remove this capability
> please.
>
Explained this on PATCH 27/35. This is the task for topology manifest.
Please note basefw binary alone does not cut it. There are external
libraries and vendor cases too. In actual real-life specific example,
you need to replace everything, e.g.: basefw, ARSC, WoV and so on.
Don't see how fw_filename can represent several files, really.
>>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski at intel.com>
>> ---
>> sound/soc/intel/common/sst-acpi.c | 5 ++---
>> sound/soc/intel/common/sst-firmware.c | 1 +
>> sound/soc/intel/skylake/skl-messages.c | 2 +-
>> sound/soc/intel/skylake/skl-sst-dsp.h | 3 +--
>> sound/soc/intel/skylake/skl-sst-utils.c | 4 +---
>> sound/soc/intel/skylake/skl.c | 4 ----
>> 6 files changed, 6 insertions(+), 13 deletions(-)
>>
>> diff --git a/sound/soc/intel/common/sst-acpi.c
>> b/sound/soc/intel/common/sst-acpi.c
>> index 53ac23f05966..15f2b27e643f 100644
>> --- a/sound/soc/intel/common/sst-acpi.c
>> +++ b/sound/soc/intel/common/sst-acpi.c
>> @@ -28,11 +28,10 @@ static void sst_acpi_fw_cb(const struct firmware
>> *fw, void *context)
>> struct sst_acpi_priv *sst_acpi = platform_get_drvdata(pdev);
>> struct sst_acpi_desc *desc = sst_acpi->desc;
>> struct sst_pdata *sst_pdata = desc->pdata;
>> - struct snd_soc_acpi_mach *mach = sst_acpi->mach;
>> sst_pdata->fw = fw;
>> if (!fw) {
>> - dev_err(dev, "Cannot load firmware %s\n", mach->fw_filename);
>> + dev_err(dev, "Cannot load firmware %s\n", sst_pdata->fw_name);
>> return;
>> }
>> @@ -119,7 +118,7 @@ int sst_acpi_probe(struct platform_device *pdev)
>> return PTR_ERR(sst_acpi->pdev_mach);
>> /* continue SST probing after firmware is loaded */
>> - ret = request_firmware_nowait(THIS_MODULE, true, mach->fw_filename,
>> + ret = request_firmware_nowait(THIS_MODULE, true, sst_pdata->fw_name,
>> dev, GFP_KERNEL, pdev, sst_acpi_fw_cb);
>> if (ret)
>> platform_device_unregister(sst_acpi->pdev_mach);
>> diff --git a/sound/soc/intel/common/sst-firmware.c
>> b/sound/soc/intel/common/sst-firmware.c
>> index 61d3e6e46b98..cc88849eb10f 100644
>> --- a/sound/soc/intel/common/sst-firmware.c
>> +++ b/sound/soc/intel/common/sst-firmware.c
>> @@ -1218,6 +1218,7 @@ struct sst_dsp *sst_dsp_new(struct device *dev,
>> struct sst_pdata *pdata)
>> sst->thread_context = pdata->dsp;
>> sst->id = pdata->id;
>> sst->irq = pdata->irq;
>> + sst->fw_name = pdata->fw_name;
>> sst->ops = pdata->ops;
>> sst->pdata = pdata;
>> INIT_LIST_HEAD(&sst->used_block_list);
>> diff --git a/sound/soc/intel/skylake/skl-messages.c
>> b/sound/soc/intel/skylake/skl-messages.c
>> index 372c5fb83ddb..e401edd8d44b 100644
>> --- a/sound/soc/intel/skylake/skl-messages.c
>> +++ b/sound/soc/intel/skylake/skl-messages.c
>> @@ -201,7 +201,7 @@ int skl_init_dsp(struct skl_dev *skl, struct
>> sst_pdata *pdata)
>> if (!ops)
>> return -EIO;
>> - ret = skl_sst_ctx_init(skl, skl->fw_name, pdata);
>> + ret = skl_sst_ctx_init(skl, pdata);
>> if (ret < 0)
>> return ret;
>> diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h
>> b/sound/soc/intel/skylake/skl-sst-dsp.h
>> index 8483c60f29ba..a3714b706b8e 100644
>> --- a/sound/soc/intel/skylake/skl-sst-dsp.h
>> +++ b/sound/soc/intel/skylake/skl-sst-dsp.h
>> @@ -223,8 +223,7 @@ int skl_dsp_strip_extended_manifest(struct
>> firmware *fw);
>> void skl_dsp_set_astate_cfg(struct skl_dev *skl, u32 cnt, void *data);
>> -int skl_sst_ctx_init(struct skl_dev *skl, const char *fw_name,
>> - struct sst_pdata *pdata);
>> +int skl_sst_ctx_init(struct skl_dev *skl, struct sst_pdata *pdata);
>> int skl_prepare_lib_load(struct skl_dev *skl, struct skl_lib_info
>> *linfo,
>> struct firmware *stripped_fw,
>> unsigned int hdr_offset, int index);
>> diff --git a/sound/soc/intel/skylake/skl-sst-utils.c
>> b/sound/soc/intel/skylake/skl-sst-utils.c
>> index a4ad213d34e0..ea5419012312 100644
>> --- a/sound/soc/intel/skylake/skl-sst-utils.c
>> +++ b/sound/soc/intel/skylake/skl-sst-utils.c
>> @@ -354,8 +354,7 @@ int skl_dsp_strip_extended_manifest(struct
>> firmware *fw)
>> return 0;
>> }
>> -int skl_sst_ctx_init(struct skl_dev *skl, const char *fw_name,
>> - struct sst_pdata *pdata)
>> +int skl_sst_ctx_init(struct skl_dev *skl, struct sst_pdata *pdata)
>> {
>> struct sst_dsp *sst;
>> struct device *dev = skl->dev;
>> @@ -372,7 +371,6 @@ int skl_sst_ctx_init(struct skl_dev *skl, const
>> char *fw_name,
>> }
>> skl->dsp = sst;
>> - sst->fw_name = fw_name;
>> init_waitqueue_head(&skl->mod_load_wait);
>> skl->is_first_boot = true;
>> diff --git a/sound/soc/intel/skylake/skl.c
>> b/sound/soc/intel/skylake/skl.c
>> index 39442c80a179..3225f4f8793e 100644
>> --- a/sound/soc/intel/skylake/skl.c
>> +++ b/sound/soc/intel/skylake/skl.c
>> @@ -491,9 +491,6 @@ static struct snd_soc_acpi_mach
>> *skl_find_hda_machine(struct skl_dev *skl,
>> /* point to common table */
>> mach = snd_soc_acpi_intel_hda_machines;
>> - /* all entries in the machine table use the same firmware */
>> - mach->fw_filename = machines->fw_filename;
>> -
>> return mach;
>> }
>> @@ -514,7 +511,6 @@ static int skl_find_machine(struct skl_dev *skl,
>> void *driver_data)
>> }
>> skl->mach = mach;
>> - skl->fw_name = mach->fw_filename;
>> pdata = mach->pdata;
>> if (pdata) {
>>
More information about the Alsa-devel
mailing list