fixup! ASoC: hdac_hda: add HDA patch loader support

There will be a timing issue if we get drvdata in hdac_hda_dev_probe. hdac_hda_dev_probe will be triggered by hda_codec_device_init() -> snd_hdac_device_register(), and dev_set_drvdata(&codec->core.dev, hda_priv) is called after that. As a result, dev_get_drvdata() may get NULL in hdac_hda_dev_probe(). This patch moves firmware request to hdac_hda_codec_probe() which can guarantee drvdata can be get properly. Signed-off-by: Bard Liao <[email protected]>
thesofproject · Aug 28, 2023 · 5a1cc2c · 5a1cc2c
1 parent 1083fe3
commit 5a1cc2c
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 53 deletions.
diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
@@ -37,9 +37,9 @@
 				 SNDRV_PCM_RATE_192000)
 
 #ifdef CONFIG_SND_HDA_PATCH_LOADER
-static char *patch[SNDRV_CARDS];
+static char *loadable_patch[SNDRV_CARDS];
 
-module_param_array(patch, charp, NULL, 0444);
+module_param_array_named(patch, loadable_patch, charp, NULL, 0444);
 MODULE_PARM_DESC(patch, "Patch file for Intel HD audio interface.");
 #endif
 
@@ -433,11 +433,20 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component)
 	}
 
 #ifdef CONFIG_SND_HDA_PATCH_LOADER
-	if (hda_pvt->fw) {
-		ret = snd_hda_load_patch(hcodec->bus, hda_pvt->fw->size, hda_pvt->fw->data);
-		if (ret < 0) {
-			dev_err(&hdev->dev, "failed to load hda patch %d\n", ret);
+	if (loadable_patch[hda_pvt->dev_index] && *loadable_patch[hda_pvt->dev_index]) {
+		dev_info(&hdev->dev, "Applying patch firmware '%s'\n",
+			 loadable_patch[hda_pvt->dev_index]);
+		ret = request_firmware(&hda_pvt->fw, loadable_patch[hda_pvt->dev_index], &hdev->dev);
+		if (ret < 0)
 			goto error_no_pm;
+		if (hda_pvt->fw) {
+			ret = snd_hda_load_patch(hcodec->bus, hda_pvt->fw->size, hda_pvt->fw->data);
+			if (ret < 0) {
+				dev_err(&hdev->dev, "failed to load hda patch %d\n", ret);
+				goto error_no_pm;
+			}
+			release_firmware(hda_pvt->fw);
+			hda_pvt->fw = NULL;
 		}
 	}
 #endif
@@ -596,45 +605,11 @@ static const struct snd_soc_component_driver hdac_hda_codec = {
 	.endianness		= 1,
 };
 
-#ifdef CONFIG_SND_HDA_PATCH_LOADER
-/* callback from request_firmware_nowait() */
-static void hda_codec_firmware_cb(const struct firmware *fw, void *context)
-{
-	struct hdac_device *hdev = context;
-	struct hdac_hda_priv *hda_pvt = dev_get_drvdata(&hdev->dev);
-
-	if (fw)
-		hda_pvt->fw = fw;
-	else
-		dev_err(&hdev->dev, "%s: Cannot load firmware, continue without patching\n",
-			__func__);
-}
-#endif
-
-static DECLARE_BITMAP(probed_devs, SNDRV_CARDS);
-
 static int hdac_hda_dev_probe(struct hdac_device *hdev)
 {
-	struct hdac_hda_priv *hda_pvt = dev_get_drvdata(&hdev->dev);
 	struct hdac_ext_link *hlink;
 	int ret;
-	int dev;
-
-	dev = find_first_zero_bit(probed_devs, SNDRV_CARDS);
-	if (dev >= SNDRV_CARDS)
-		return -ENODEV;
 
-#ifdef CONFIG_SND_HDA_PATCH_LOADER
-	if (patch[dev] && *patch[dev]) {
-		dev_info(&hdev->dev, "Applying patch firmware '%s'\n",
-			 patch[dev]);
-		ret = request_firmware_nowait(THIS_MODULE, true, patch[dev],
-					      &hdev->dev, GFP_KERNEL, hdev,
-					      hda_codec_firmware_cb);
-		if (ret < 0)
-			return ret;
-	}
-#endif /* CONFIG_SND_HDA_PATCH_LOADER */
 	/* hold the ref while we probe */
 	hlink = snd_hdac_ext_bus_get_hlink_by_name(hdev->bus, dev_name(&hdev->dev));
 	if (!hlink) {
@@ -654,24 +629,15 @@ static int hdac_hda_dev_probe(struct hdac_device *hdev)
 
 	snd_hdac_ext_bus_link_put(hdev->bus, hlink);
 
-	set_bit(dev, probed_devs);
-	hda_pvt->dev_index = dev;
-
 	return ret;
 }
 
 static int hdac_hda_dev_remove(struct hdac_device *hdev)
 {
-	struct hdac_hda_priv *hda_pvt = dev_get_drvdata(&hdev->dev);
-
-	/* Resources are freed in hdac_hda_codec_remove(). */
-
-	clear_bit(hda_pvt->dev_index, probed_devs);
-
-#ifdef CONFIG_SND_HDA_PATCH_LOADER
-	release_firmware(hda_pvt->fw);
-#endif
-
+	/*
+	 * Resources are freed in hdac_hda_codec_remove(). This
+	 * function is kept to keep hda_codec_driver_remove() happy.
+	 */
 	return 0;
 }
 

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
@@ -736,6 +736,7 @@ static int probe_codec(struct hdac_bus *bus, int addr)
 		return PTR_ERR(codec);
 
 	hda_codec->codec = codec;
+	hda_codec->dev_index = addr;
 	dev_set_drvdata(&codec->core.dev, hda_codec);
 
 	/* use legacy bus only for HDA codecs, idisp uses ext bus */

diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
@@ -169,6 +169,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address)
 		return ret;
 
 	hda_priv->codec = codec;
+	hda_priv->dev_index = address;
 	dev_set_drvdata(&codec->core.dev, hda_priv);
 
 	if ((resp & 0xFFFF0000) == IDISP_VID_INTEL) {