| Message ID | 20231209205351.880797-8-cristian.ciocaltea@collabora.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve SOF support for Steam Deck OLED | expand |
On 09/12/2023 22:53, Cristian Ciocaltea wrote: > Some SOF drivers like AMD ACP do not always rely on a single static > firmware file, but may require multiple files having their names > dynamically computed on probe time, e.g. based on chip name. I see, AMD vangogh needs two binary files to be loaded sometimes, it is not using the default name as it hints via the sof_dev_desc ("sof-vangogh.ri"). The constructed names for the two files are just using different pattern: sof-%PLAT%.ri vs sof-%PLAT%-code.bin sof-%PLAT%-data.bin iow, instead of the combined .ri file which includes the code and data segment it has 'raw' bin files and cannot use the core for loading the firmware. What is the reason for this? an .ri file can have two 'modules' one to be written to IRAM the other to DRAM. sof_ipc3_load_fw_to_dsp() > In those cases, providing an invalid default_fw_filename in their > sof_dev_desc struct will prevent probing due to 'SOF firmware > and/or topology file not found' error. > Fix the issue by allowing drivers to omit initialization for this member > (or alternatively provide a dynamic override via ipc_file_profile_base) > and update sof_test_firmware_file() to verify the given profile data and > skip firmware testing if either fw_path or fw_name is not defined. > > Fixes: 6c393ebbd74a ("ASoC: SOF: core: Implement IPC version fallback if firmware files are missing") > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > sound/soc/sof/fw-file-profile.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c > index 138a1ca2c4a8..e63700234df0 100644 > --- a/sound/soc/sof/fw-file-profile.c > +++ b/sound/soc/sof/fw-file-profile.c > @@ -21,6 +21,9 @@ static int sof_test_firmware_file(struct device *dev, > const u32 *magic; > int ret; > > + if (!profile->fw_path || !profile->fw_name || !*profile->fw_name) > + return 0; I would rather make the firmware file skipping based on custom loader use and print a dev_dbg() alongside: diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c index 138a1ca2c4a8..7b91c9551ada 100644 --- a/sound/soc/sof/fw-file-profile.c +++ b/sound/soc/sof/fw-file-profile.c @@ -89,6 +89,15 @@ static int sof_test_topology_file(struct device *dev, return ret; } +static bool sof_platform_uses_generic_loader(struct snd_sof_dev *sdev) +{ + if (sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_raw || + sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_memcpy) + return true; + + return false; +} + static int sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, enum sof_ipc_type ipc_type, @@ -130,7 +139,8 @@ sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, * For default path and firmware name do a verification before * continuing further. */ - if (base_profile->fw_path || base_profile->fw_name) { + if ((base_profile->fw_path || base_profile->fw_name) && + sof_platform_uses_generic_loader(sdev)) { ret = sof_test_firmware_file(dev, out_profile, &ipc_type); if (ret) return ret; @@ -181,7 +191,8 @@ sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, out_profile->ipc_type = ipc_type; /* Test only default firmware file */ - if (!base_profile->fw_path && !base_profile->fw_name) + if ((!base_profile->fw_path && !base_profile->fw_name) && + sof_platform_uses_generic_loader(sdev)) ret = sof_test_firmware_file(dev, out_profile, NULL); if (!ret) @@ -265,7 +276,9 @@ static void sof_print_profile_info(struct snd_sof_dev *sdev, "Using fallback IPC type %d (requested type was %d)\n", profile->ipc_type, ipc_type); - dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); + /* The firmware path only valid when generic loader is used */ + if (sof_platform_uses_generic_loader(sdev)) + dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); dev_info(dev, " Firmware file: %s/%s\n", profile->fw_path, profile->fw_name); if (profile->fw_lib_path) > + > fw_filename = kasprintf(GFP_KERNEL, "%s/%s", profile->fw_path, > profile->fw_name); > if (!fw_filename) checking for custom loader allows you to drop the next patch. I would also skip the fw path printing in case of a custom loader.
On 12/14/23 16:18, Péter Ujfalusi wrote: Thanks for your time Peter! > > On 09/12/2023 22:53, Cristian Ciocaltea wrote: >> Some SOF drivers like AMD ACP do not always rely on a single static >> firmware file, but may require multiple files having their names >> dynamically computed on probe time, e.g. based on chip name. > I see, AMD vangogh needs two binary files to be loaded sometimes, it is not using the default name as it hints via the sof_dev_desc ("sof-vangogh.ri"). > > The constructed names for the two files are just using different pattern: > sof-%PLAT%.ri > vs > sof-%PLAT%-code.bin > sof-%PLAT%-data.bin > > iow, instead of the combined .ri file which includes the code and data segment it has 'raw' bin files and cannot use the core for loading the firmware. > > What is the reason for this? an .ri file can have two 'modules' one to be written to IRAM the other to DRAM. > sof_ipc3_load_fw_to_dsp() For AMD Vangogh platform devices signed firmware image is required, so split .ri image into code and data images. Only Code.bin will be signed and loaded into corresponding IRAM location. > >> In those cases, providing an invalid default_fw_filename in their >> sof_dev_desc struct will prevent probing due to 'SOF firmware >> and/or topology file not found' error. > >> Fix the issue by allowing drivers to omit initialization for this member >> (or alternatively provide a dynamic override via ipc_file_profile_base) >> and update sof_test_firmware_file() to verify the given profile data and >> skip firmware testing if either fw_path or fw_name is not defined. >> >> Fixes: 6c393ebbd74a ("ASoC: SOF: core: Implement IPC version fallback if firmware files are missing") >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> sound/soc/sof/fw-file-profile.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c >> index 138a1ca2c4a8..e63700234df0 100644 >> --- a/sound/soc/sof/fw-file-profile.c >> +++ b/sound/soc/sof/fw-file-profile.c >> @@ -21,6 +21,9 @@ static int sof_test_firmware_file(struct device *dev, >> const u32 *magic; >> int ret; >> >> + if (!profile->fw_path || !profile->fw_name || !*profile->fw_name) >> + return 0; > I would rather make the firmware file skipping based on custom loader use and print a dev_dbg() alongside: > > diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c > index 138a1ca2c4a8..7b91c9551ada 100644 > --- a/sound/soc/sof/fw-file-profile.c > +++ b/sound/soc/sof/fw-file-profile.c > @@ -89,6 +89,15 @@ static int sof_test_topology_file(struct device *dev, > return ret; > } > > +static bool sof_platform_uses_generic_loader(struct snd_sof_dev *sdev) > +{ > + if (sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_raw || > + sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_memcpy) > + return true; > + > + return false; > +} > + > static int > sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, > enum sof_ipc_type ipc_type, > @@ -130,7 +139,8 @@ sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, > * For default path and firmware name do a verification before > * continuing further. > */ > - if (base_profile->fw_path || base_profile->fw_name) { > + if ((base_profile->fw_path || base_profile->fw_name) && > + sof_platform_uses_generic_loader(sdev)) { > ret = sof_test_firmware_file(dev, out_profile, &ipc_type); > if (ret) > return ret; > @@ -181,7 +191,8 @@ sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, > out_profile->ipc_type = ipc_type; > > /* Test only default firmware file */ > - if (!base_profile->fw_path && !base_profile->fw_name) > + if ((!base_profile->fw_path && !base_profile->fw_name) && > + sof_platform_uses_generic_loader(sdev)) > ret = sof_test_firmware_file(dev, out_profile, NULL); > > if (!ret) > @@ -265,7 +276,9 @@ static void sof_print_profile_info(struct snd_sof_dev *sdev, > "Using fallback IPC type %d (requested type was %d)\n", > profile->ipc_type, ipc_type); > > - dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); > + /* The firmware path only valid when generic loader is used */ > + if (sof_platform_uses_generic_loader(sdev)) > + dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); > > dev_info(dev, " Firmware file: %s/%s\n", profile->fw_path, profile->fw_name); > if (profile->fw_lib_path) > >> + >> fw_filename = kasprintf(GFP_KERNEL, "%s/%s", profile->fw_path, >> profile->fw_name); >> if (!fw_filename) > checking for custom loader allows you to drop the next patch. > I would also skip the fw path printing in case of a custom loader. >
On 12/14/23 12:48, Péter Ujfalusi wrote: > > > On 09/12/2023 22:53, Cristian Ciocaltea wrote: >> Some SOF drivers like AMD ACP do not always rely on a single static >> firmware file, but may require multiple files having their names >> dynamically computed on probe time, e.g. based on chip name. > > I see, AMD vangogh needs two binary files to be loaded sometimes, it is not using the default name as it hints via the sof_dev_desc ("sof-vangogh.ri"). > > The constructed names for the two files are just using different pattern: > sof-%PLAT%.ri > vs > sof-%PLAT%-code.bin > sof-%PLAT%-data.bin > > iow, instead of the combined .ri file which includes the code and data segment it has 'raw' bin files and cannot use the core for loading the firmware. > > What is the reason for this? an .ri file can have two 'modules' one to be written to IRAM the other to DRAM. > sof_ipc3_load_fw_to_dsp() > >> In those cases, providing an invalid default_fw_filename in their >> sof_dev_desc struct will prevent probing due to 'SOF firmware >> and/or topology file not found' error. > >> Fix the issue by allowing drivers to omit initialization for this member >> (or alternatively provide a dynamic override via ipc_file_profile_base) >> and update sof_test_firmware_file() to verify the given profile data and >> skip firmware testing if either fw_path or fw_name is not defined. >> >> Fixes: 6c393ebbd74a ("ASoC: SOF: core: Implement IPC version fallback if firmware files are missing") >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> sound/soc/sof/fw-file-profile.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c >> index 138a1ca2c4a8..e63700234df0 100644 >> --- a/sound/soc/sof/fw-file-profile.c >> +++ b/sound/soc/sof/fw-file-profile.c >> @@ -21,6 +21,9 @@ static int sof_test_firmware_file(struct device *dev, >> const u32 *magic; >> int ret; >> >> + if (!profile->fw_path || !profile->fw_name || !*profile->fw_name) >> + return 0; > > I would rather make the firmware file skipping based on custom loader use and print a dev_dbg() alongside: Agree, that's a better approach. Thanks for the hint! > diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c > index 138a1ca2c4a8..7b91c9551ada 100644 > --- a/sound/soc/sof/fw-file-profile.c > +++ b/sound/soc/sof/fw-file-profile.c > @@ -89,6 +89,15 @@ static int sof_test_topology_file(struct device *dev, > return ret; > } > > +static bool sof_platform_uses_generic_loader(struct snd_sof_dev *sdev) > +{ > + if (sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_raw || > + sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_memcpy) > + return true; > + > + return false; > +} I would drop the conditional and simply return. > static int > sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, > enum sof_ipc_type ipc_type, > @@ -130,7 +139,8 @@ sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, > * For default path and firmware name do a verification before > * continuing further. > */ > - if (base_profile->fw_path || base_profile->fw_name) { > + if ((base_profile->fw_path || base_profile->fw_name) && > + sof_platform_uses_generic_loader(sdev)) { > ret = sof_test_firmware_file(dev, out_profile, &ipc_type); > if (ret) > return ret; > @@ -181,7 +191,8 @@ sof_file_profile_for_ipc_type(struct snd_sof_dev *sdev, > out_profile->ipc_type = ipc_type; > > /* Test only default firmware file */ > - if (!base_profile->fw_path && !base_profile->fw_name) > + if ((!base_profile->fw_path && !base_profile->fw_name) && > + sof_platform_uses_generic_loader(sdev)) > ret = sof_test_firmware_file(dev, out_profile, NULL); > > if (!ret) > @@ -265,7 +276,9 @@ static void sof_print_profile_info(struct snd_sof_dev *sdev, > "Using fallback IPC type %d (requested type was %d)\n", > profile->ipc_type, ipc_type); > > - dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); > + /* The firmware path only valid when generic loader is used */ > + if (sof_platform_uses_generic_loader(sdev)) > + dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); > > dev_info(dev, " Firmware file: %s/%s\n", profile->fw_path, profile->fw_name); > if (profile->fw_lib_path) > >> + >> fw_filename = kasprintf(GFP_KERNEL, "%s/%s", profile->fw_path, >> profile->fw_name); >> if (!fw_filename) > > checking for custom loader allows you to drop the next patch. Yes, I will take only the context information and use it to update the current patch description. > I would also skip the fw path printing in case of a custom loader. Sure, makes sense. Thanks for the review, Cristian
On 14/12/2023 13:29, Cristian Ciocaltea wrote: >> diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c >> index 138a1ca2c4a8..7b91c9551ada 100644 >> --- a/sound/soc/sof/fw-file-profile.c >> +++ b/sound/soc/sof/fw-file-profile.c >> @@ -89,6 +89,15 @@ static int sof_test_topology_file(struct device *dev, >> return ret; >> } >> >> +static bool sof_platform_uses_generic_loader(struct snd_sof_dev *sdev) >> +{ >> + if (sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_raw || >> + sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_memcpy) >> + return true; >> + >> + return false; >> +} > > I would drop the conditional and simply return. What do you mean? We need to check if the platform is using either type of the generic load_firmware helper (the _memcpy is calling the _raw to load the file).
On 12/14/23 13:35, Péter Ujfalusi wrote: > > > On 14/12/2023 13:29, Cristian Ciocaltea wrote: >>> diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c >>> index 138a1ca2c4a8..7b91c9551ada 100644 >>> --- a/sound/soc/sof/fw-file-profile.c >>> +++ b/sound/soc/sof/fw-file-profile.c >>> @@ -89,6 +89,15 @@ static int sof_test_topology_file(struct device *dev, >>> return ret; >>> } >>> >>> +static bool sof_platform_uses_generic_loader(struct snd_sof_dev *sdev) >>> +{ >>> + if (sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_raw || >>> + sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_memcpy) >>> + return true; >>> + >>> + return false; >>> +} >> >> I would drop the conditional and simply return. > > What do you mean? We need to check if the platform is using either type > of the generic load_firmware helper (the _memcpy is calling the _raw to > load the file). I mean to simply replace the if statement with: static bool sof_platform_uses_generic_loader(struct snd_sof_dev *sdev) { return (sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_raw || sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_memcpy); }
On 14/12/2023 12:48, Péter Ujfalusi wrote: > @@ -265,7 +276,9 @@ static void sof_print_profile_info(struct snd_sof_dev *sdev, > "Using fallback IPC type %d (requested type was %d)\n", > profile->ipc_type, ipc_type); > > - dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); > + /* The firmware path only valid when generic loader is used */ > + if (sof_platform_uses_generic_loader(sdev)) > + dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); > This is the correct section in here, sorry: - dev_info(dev, " Firmware file: %s/%s\n", profile->fw_path, profile->fw_name); + /* The firmware path only valid when generic loader is used */ + if (sof_platform_uses_generic_loader(sdev)) + dev_info(dev, " Firmware file: %s/%s\n", profile->fw_path, profile->fw_name); + if (profile->fw_lib_path)
On 14/12/2023 13:40, Cristian Ciocaltea wrote: > On 12/14/23 13:35, Péter Ujfalusi wrote: >> >> >> On 14/12/2023 13:29, Cristian Ciocaltea wrote: >>>> diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c >>>> index 138a1ca2c4a8..7b91c9551ada 100644 >>>> --- a/sound/soc/sof/fw-file-profile.c >>>> +++ b/sound/soc/sof/fw-file-profile.c >>>> @@ -89,6 +89,15 @@ static int sof_test_topology_file(struct device *dev, >>>> return ret; >>>> } >>>> >>>> +static bool sof_platform_uses_generic_loader(struct snd_sof_dev *sdev) >>>> +{ >>>> + if (sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_raw || >>>> + sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_memcpy) >>>> + return true; >>>> + >>>> + return false; >>>> +} >>> >>> I would drop the conditional and simply return. >> >> What do you mean? We need to check if the platform is using either type >> of the generic load_firmware helper (the _memcpy is calling the _raw to >> load the file). > > I mean to simply replace the if statement with: > > static bool sof_platform_uses_generic_loader(struct snd_sof_dev *sdev) > { > return (sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_raw || > sdev->pdata->desc->ops->load_firmware == snd_sof_load_firmware_memcpy); > } ah, OK.
On 14/12/2023 12:58, Venkata Prasad Potturu wrote: > > On 12/14/23 16:18, Péter Ujfalusi wrote: > Thanks for your time Peter! >> >> On 09/12/2023 22:53, Cristian Ciocaltea wrote: >>> Some SOF drivers like AMD ACP do not always rely on a single static >>> firmware file, but may require multiple files having their names >>> dynamically computed on probe time, e.g. based on chip name. >> I see, AMD vangogh needs two binary files to be loaded sometimes, it >> is not using the default name as it hints via the sof_dev_desc >> ("sof-vangogh.ri"). >> >> The constructed names for the two files are just using different pattern: >> sof-%PLAT%.ri >> vs >> sof-%PLAT%-code.bin >> sof-%PLAT%-data.bin >> >> iow, instead of the combined .ri file which includes the code and data >> segment it has 'raw' bin files and cannot use the core for loading the >> firmware. >> >> What is the reason for this? an .ri file can have two 'modules' one to >> be written to IRAM the other to DRAM. >> sof_ipc3_load_fw_to_dsp() > > For AMD Vangogh platform devices signed firmware image is required, so > split .ri image into code and data images. > > Only Code.bin will be signed and loaded into corresponding IRAM location. This is not different than what the Intel .ri files are made of. The module which is to be loaded to IRAM is signed code the module which goes to DRAM is not signed. The loader itself is not looking into the sections of the .ri image, it just parses the header and copies them where they belong. if the issue is name collision then you could try to put the signed firmware file under 'signed' folder (fw_path_postfix) of the platform like Intel does with the community signed ones? It would be great if somehow we can handle all of these in core, have shared code and familiar prints among vendors, platforms.. Fwiw, I'm planning the path, filename creation to be moved to core for the current platforms, but it implies that they do use single firmware file. struct sof_dev_desc would only have two strings: vendor - AMD / iMX / Intel / Mediatek platform - tgl, vaggogh, etc I need to adjust it based on what I have learned today about vangogh.
On 12/14/23 13:51, Péter Ujfalusi wrote: > > > On 14/12/2023 12:48, Péter Ujfalusi wrote: >> @@ -265,7 +276,9 @@ static void sof_print_profile_info(struct snd_sof_dev *sdev, >> "Using fallback IPC type %d (requested type was %d)\n", >> profile->ipc_type, ipc_type); >> >> - dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); >> + /* The firmware path only valid when generic loader is used */ >> + if (sof_platform_uses_generic_loader(sdev)) >> + dev_info(dev, "Firmware paths/files for ipc type %d:\n", profile->ipc_type); >> > > This is the correct section in here, sorry: > > - dev_info(dev, " Firmware file: %s/%s\n", profile->fw_path, profile->fw_name); > + /* The firmware path only valid when generic loader is used */ > + if (sof_platform_uses_generic_loader(sdev)) > + dev_info(dev, " Firmware file: %s/%s\n", profile->fw_path, profile->fw_name); > + > if (profile->fw_lib_path) No problem, thanks for the update!
On 12/14/23 17:27, Péter Ujfalusi wrote: > > On 14/12/2023 12:58, Venkata Prasad Potturu wrote: >> On 12/14/23 16:18, Péter Ujfalusi wrote: >> Thanks for your time Peter! >>> On 09/12/2023 22:53, Cristian Ciocaltea wrote: >>>> Some SOF drivers like AMD ACP do not always rely on a single static >>>> firmware file, but may require multiple files having their names >>>> dynamically computed on probe time, e.g. based on chip name. >>> I see, AMD vangogh needs two binary files to be loaded sometimes, it >>> is not using the default name as it hints via the sof_dev_desc >>> ("sof-vangogh.ri"). >>> >>> The constructed names for the two files are just using different pattern: >>> sof-%PLAT%.ri >>> vs >>> sof-%PLAT%-code.bin >>> sof-%PLAT%-data.bin >>> >>> iow, instead of the combined .ri file which includes the code and data >>> segment it has 'raw' bin files and cannot use the core for loading the >>> firmware. >>> >>> What is the reason for this? an .ri file can have two 'modules' one to >>> be written to IRAM the other to DRAM. >>> sof_ipc3_load_fw_to_dsp() >> For AMD Vangogh platform devices signed firmware image is required, so >> split .ri image into code and data images. >> >> Only Code.bin will be signed and loaded into corresponding IRAM location. > This is not different than what the Intel .ri files are made of. The > module which is to be loaded to IRAM is signed code the module which > goes to DRAM is not signed. > The loader itself is not looking into the sections of the .ri image, it > just parses the header and copies them where they belong. > > if the issue is name collision then you could try to put the signed > firmware file under 'signed' folder (fw_path_postfix) of the platform > like Intel does with the community signed ones? We have a limitation that code image can't be signed during compilation. So splitting the .ri image into code and data bin and sign the code bin and load into IRAM. > > It would be great if somehow we can handle all of these in core, have > shared code and familiar prints among vendors, platforms.. > > Fwiw, I'm planning the path, filename creation to be moved to core for > the current platforms, but it implies that they do use single firmware file. > struct sof_dev_desc would only have two strings: > vendor - AMD / iMX / Intel / Mediatek > platform - tgl, vaggogh, etc > > I need to adjust it based on what I have learned today about vangogh. >
diff --git a/sound/soc/sof/fw-file-profile.c b/sound/soc/sof/fw-file-profile.c index 138a1ca2c4a8..e63700234df0 100644 --- a/sound/soc/sof/fw-file-profile.c +++ b/sound/soc/sof/fw-file-profile.c @@ -21,6 +21,9 @@ static int sof_test_firmware_file(struct device *dev, const u32 *magic; int ret; + if (!profile->fw_path || !profile->fw_name || !*profile->fw_name) + return 0; + fw_filename = kasprintf(GFP_KERNEL, "%s/%s", profile->fw_path, profile->fw_name); if (!fw_filename)
Some SOF drivers like AMD ACP do not always rely on a single static firmware file, but may require multiple files having their names dynamically computed on probe time, e.g. based on chip name. In those cases, providing an invalid default_fw_filename in their sof_dev_desc struct will prevent probing due to 'SOF firmware and/or topology file not found' error. Fix the issue by allowing drivers to omit initialization for this member (or alternatively provide a dynamic override via ipc_file_profile_base) and update sof_test_firmware_file() to verify the given profile data and skip firmware testing if either fw_path or fw_name is not defined. Fixes: 6c393ebbd74a ("ASoC: SOF: core: Implement IPC version fallback if firmware files are missing") Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> --- sound/soc/sof/fw-file-profile.c | 3 +++ 1 file changed, 3 insertions(+)