Re: [PATCH 30/35] ASoC: Intel: Reuse sst_pdata::fw_name field

Cezary Rojewski <cezary.rojewski@xxxxxxxxx> · Sat, 24 Aug 2019 12:57:40 +0200

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@xxxxxxxxx>
---
  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) {

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel







