On Tue, 19 Sep 2023 10:32:09 +0200, Bard Liao wrote:
+#ifdef CONFIG_SND_HDA_PATCH_LOADER +static char *loadable_patch[SNDRV_CARDS];
Hm, this size looks wrong.
In the code later in this patch, dev_index points to the codec address, and it's basically irrelevant with SNDRV_CARDS. As SNDRV_CARDS might be 8 depending on the config, it can go beyond the array size, too. Use a different array size to match with the codec address ranges.
+module_param_array_named(patch, loadable_patch, charp, NULL, 0444); +MODULE_PARM_DESC(patch, "Patch file for Intel HD audio interface.");
Better to give a bit more description; when it's a codec address array, it can be non-zero, and user may wonder why it's not applied.
+#ifdef CONFIG_SND_HDA_PATCH_LOADER
- 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;
So the hda_pvt->fw is only for a temporary use, already released already here. Then...
--- a/sound/soc/codecs/hdac_hda.h +++ b/sound/soc/codecs/hdac_hda.h @@ -26,6 +26,10 @@ struct hdac_hda_priv { struct hda_codec *codec; struct hdac_hda_pcm pcm[HDAC_DAI_ID_NUM]; bool need_display_power;
- int dev_index;
+#ifdef CONFIG_SND_HDA_PATCH_LOADER
- const struct firmware *fw;
+#endif
... we don't have to add a new extra field, right?
thanks,
Takashi