Re: [PATCH v2 3/6] ASoC: soc-acpi: move common function topology helpers

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

On 07/11/2026 21:26, Syed Saba Kareem wrote:
> From: Syed Saba Kareem <Syed.SabaKareem@amd.com>
> 
> From: Siva Subramanian Ravi Saravanan <sravisar@amd.com>
> 
> Move the Intel function topology helpers and the topology device enum
> to soc-acpi so that they can be shared with other platforms too. The
> moved helpers are:
>  - get_platform_name() as snd_soc_acpi_get_platform_name()

This function is SOF specific (expetcts the string to be
sof-<platform>), the generic name after move did not reflect this fack.

>  - tplg_files_exist() as snd_soc_acpi_tplg_files_exist()

This only checks for existance of a topology file, probably better off
in soc-topology.c ?
and a comment inline for the moved function later.

>  - get_tplg_filename() as snd_soc_acpi_get_tplg_filename()
> 
> No functional change for Intel.

It looks like there is a build regression according to lkp@intel.com

> Signed-off-by: Siva Subramanian Ravi Saravanan <sravisar@amd.com>
> Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
> ---
>  include/sound/soc-acpi.h                      | 25 +++++
>  .../intel/common/sof-function-topology-lib.c  | 93 ++-----------------
>  sound/soc/soc-acpi.c                          | 73 +++++++++++++++
>  3 files changed, 106 insertions(+), 85 deletions(-)
> 
> diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
> index 2ee303b791dc..aea47a42f43d 100644
> --- a/include/sound/soc-acpi.h
> +++ b/include/sound/soc-acpi.h
> @@ -257,4 +257,29 @@ bool snd_soc_acpi_sdw_link_slaves_found(struct device *dev,
>  					const struct snd_soc_acpi_link_adr *link,
>  					struct sdw_peripherals *peripherals);
>  
> +enum tplg_device_id {
> +	TPLG_DEVICE_SDCA_JACK,
> +	TPLG_DEVICE_SDCA_AMP,
> +	TPLG_DEVICE_SDCA_MIC,
> +	TPLG_DEVICE_INTEL_PCH_DMIC,
> +	TPLG_DEVICE_HDMI,
> +	TPLG_DEVICE_SSP_JACK,
> +	TPLG_DEVICE_SSP_AMP,
> +	TPLG_DEVICE_SSP_BT,
> +	TPLG_DEVICE_SSP_HDMI_IN,
> +	TPLG_DEVICE_LOOPBACK_VIRTUAL,
> +	TPLG_DEVICE_ACP_SDW_JACK,
> +	TPLG_DEVICE_ACP_SDW_AMP,
> +	TPLG_DEVICE_ACP_SDW_MIC,
> +	TPLG_DEVICE_ACP_DMIC,

this patch does a but  more than just move function which is not
mentioned in commit title, nor in the message..

> +	TPLG_DEVICE_MAX
> +};
> +
> +int snd_soc_acpi_get_platform_name(struct snd_soc_card *card,
> +				   const struct snd_soc_acpi_mach *mach, char *platform);
> +bool snd_soc_acpi_tplg_files_exist(struct device *dev, const char *tplg_files);
> +char *snd_soc_acpi_get_tplg_filename(struct device *dev, const char *prefix,
> +				     const char *platform, const char *tplg_dev_name,
> +				     int dai_link_id, int tplg_dev);
> +
>  #endif
> diff --git a/sound/soc/intel/common/sof-function-topology-lib.c b/sound/soc/intel/common/sof-function-topology-lib.c
> index 57e089f8e721..413d6f11a00e 100644
> --- a/sound/soc/intel/common/sof-function-topology-lib.c
> +++ b/sound/soc/intel/common/sof-function-topology-lib.c
> @@ -13,88 +13,11 @@
>  #include <sound/soc-acpi.h>
>  #include "sof-function-topology-lib.h"
>  
> -enum tplg_device_id {
> -	TPLG_DEVICE_SDCA_JACK,
> -	TPLG_DEVICE_SDCA_AMP,
> -	TPLG_DEVICE_SDCA_MIC,
> -	TPLG_DEVICE_INTEL_PCH_DMIC,
> -	TPLG_DEVICE_HDMI,
> -	TPLG_DEVICE_SSP_JACK,
> -	TPLG_DEVICE_SSP_AMP,
> -	TPLG_DEVICE_SSP_BT,
> -	TPLG_DEVICE_SSP_HDMI_IN,
> -	TPLG_DEVICE_LOOPBACK_VIRTUAL,
> -	TPLG_DEVICE_MAX
> -};
> -
>  #define SDCA_DEVICE_MASK (BIT(TPLG_DEVICE_SDCA_JACK) | BIT(TPLG_DEVICE_SDCA_AMP) | \
>  			  BIT(TPLG_DEVICE_SDCA_MIC))
>  
>  #define SOF_INTEL_PLATFORM_NAME_MAX 4
>  
> -static int get_platform_name(struct snd_soc_card *card,
> -			     const struct snd_soc_acpi_mach *mach, char *platform)
> -{
> -	int ret;
> -
> -	ret = sscanf(mach->sof_tplg_filename, "sof-%3s-*.tplg", platform);
> -	if (ret != 1) {
> -		dev_err(card->dev, "Invalid platform name of tplg %s\n",
> -			mach->sof_tplg_filename);
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
> -static bool tplg_files_exist(struct device *dev, const char *tplg_files)
> -{
> -	const struct firmware *fw;
> -	int ret;
> -
> -	ret = firmware_request_nowarn(&fw, tplg_files, dev);
> -	if (!ret) {
> -		release_firmware(fw);
> -		return true;
> -	}
> -
> -	dev_warn(dev,
> -		 "Failed to open topology file: %s, you might need to\n",
> -		 tplg_files);
> -	dev_warn(dev,
> -		 "download it from https://github.com/thesofproject/sof-bin/\n");
> -	return false;
> -}
> -
> -static char *get_tplg_filename(struct device *dev, const char *prefix,
> -			       const char *platform, const char *tplg_dev_name,
> -			       int dai_link_id, int tplg_dev)
> -{
> -	char *filename = NULL;
> -
> -	/*
> -	 * The tplg file naming rule is sof-<platform>-<function>-id<BE id number>.tplg
> -	 * where <platform> is required for functions that depend on NHLT blobs (e.g. DMIC/SSP)
> -	 * as the nhlt blob is platform dependent.
> -	 */
> -	switch (tplg_dev) {
> -	case TPLG_DEVICE_INTEL_PCH_DMIC:
> -	case TPLG_DEVICE_SSP_JACK:
> -	case TPLG_DEVICE_SSP_AMP:
> -	case TPLG_DEVICE_SSP_BT:
> -	case TPLG_DEVICE_SSP_HDMI_IN:
> -		filename = devm_kasprintf(dev, GFP_KERNEL, "%s/sof-%s-%s-id%d.tplg",
> -					  prefix, platform, tplg_dev_name, dai_link_id);
> -		break;
> -	default:
> -		filename = devm_kasprintf(dev, GFP_KERNEL, "%s/sof-%s-id%d.tplg",
> -					  prefix, tplg_dev_name, dai_link_id);
> -		break;
> -	}
> -
> -	return filename;
> -}
> -
>  static int get_dmic_tplg_dev(struct device *dev, int dmic_num,
>  			     int *tplg_dev, char **tplg_dev_name)
>  {
> @@ -182,7 +105,7 @@ int sof_sdw_get_tplg_files(struct snd_soc_card *card, const struct snd_soc_acpi_
>  	int ret;
>  	int i;
>  
> -	ret = get_platform_name(card, mach, platform);
> +	ret = snd_soc_acpi_get_platform_name(card, mach, platform);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -235,13 +158,13 @@ int sof_sdw_get_tplg_files(struct snd_soc_card *card, const struct snd_soc_acpi_
>  		if (tplg_mask & BIT(tplg_dev))
>  			continue;
>  
> -		tplg_file = get_tplg_filename(card->dev, prefix, platform, tplg_dev_name,
> -					      dai_link->id, tplg_dev);
> +		tplg_file = snd_soc_acpi_get_tplg_filename(card->dev, prefix, platform,
> +							   tplg_dev_name, dai_link->id, tplg_dev);
>  		if (!tplg_file)
>  			return -ENOMEM;
>  
>  		/* Check presence of sub-topologies */
> -		if (!tplg_files_exist(card->dev, tplg_file)) {
> +		if (!snd_soc_acpi_tplg_files_exist(card->dev, tplg_file)) {
>  			devm_kfree(card->dev, tplg_file);
>  			if (best_effort)
>  				continue;
> @@ -275,7 +198,7 @@ int sof_i2s_get_tplg_files(struct snd_soc_card *card, const struct snd_soc_acpi_
>  	int ret;
>  	int i;
>  
> -	ret = get_platform_name(card, mach, platform);
> +	ret = snd_soc_acpi_get_platform_name(card, mach, platform);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -307,13 +230,13 @@ int sof_i2s_get_tplg_files(struct snd_soc_card *card, const struct snd_soc_acpi_
>  		if (tplg_mask & BIT(tplg_dev))
>  			continue;
>  
> -		tplg_file = get_tplg_filename(card->dev, prefix, platform, tplg_dev_name,
> -					      dai_link->id, tplg_dev);
> +		tplg_file = snd_soc_acpi_get_tplg_filename(card->dev, prefix, platform,
> +							   tplg_dev_name, dai_link->id, tplg_dev);
>  		if (!tplg_file)
>  			return -ENOMEM;
>  
>  		/* Check presence of sub-topologies */
> -		if (!tplg_files_exist(card->dev, tplg_file)) {
> +		if (!snd_soc_acpi_tplg_files_exist(card->dev, tplg_file)) {
>  			devm_kfree(card->dev, tplg_file);
>  			if (best_effort)
>  				continue;
> diff --git a/sound/soc/soc-acpi.c b/sound/soc/soc-acpi.c
> index 270f9777942f..7e9d7b3c6b32 100644
> --- a/sound/soc/soc-acpi.c
> +++ b/sound/soc/soc-acpi.c
> @@ -4,7 +4,9 @@
>  //
>  // Copyright (c) 2013-15, Intel Corporation.
>  
> +#include <linux/device.h>
>  #include <linux/export.h>
> +#include <linux/firmware.h>
>  #include <linux/module.h>
>  #include <sound/soc-acpi.h>
>  
> @@ -200,5 +202,76 @@ bool snd_soc_acpi_sdw_link_slaves_found(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_acpi_sdw_link_slaves_found);
>  
> +int snd_soc_acpi_get_platform_name(struct snd_soc_card *card,
> +				   const struct snd_soc_acpi_mach *mach, char *platform)
> +{
> +	int ret;
> +
> +	/* platform must hold at least 8 bytes */
> +	ret = sscanf(mach->sof_tplg_filename, "sof-%7[^-]-*.tplg", platform);
> +	if (ret != 1) {
> +		dev_err(card->dev, "Invalid platform name of tplg %s\n",
> +			mach->sof_tplg_filename);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_acpi_get_platform_name);
> +
> +bool snd_soc_acpi_tplg_files_exist(struct device *dev, const char *tplg_files)
> +{
> +	const struct firmware *fw;
> +	int ret;
> +
> +	ret = firmware_request_nowarn(&fw, tplg_files, dev);
> +	if (!ret) {
> +		release_firmware(fw);
> +		return true;
> +	}
> +
> +	dev_warn(dev,
> +		 "Failed to open topology file: %s, you might need to\n",
> +		 tplg_files);
> +	dev_warn(dev,
> +		 "download it from https://github.com/thesofproject/sof-bin/\n");

This is certainly true for Intel platforms but the print makes this also
SOF specific which is not reflected in the function name.

> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_acpi_tplg_files_exist);
> +
> +char *snd_soc_acpi_get_tplg_filename(struct device *dev, const char *prefix,
> +				     const char *platform, const char *tplg_dev_name,
> +				     int dai_link_id, int tplg_dev)
> +{
> +	char *filename = NULL;
> +
> +	/*
> +	 * The tplg file naming rule is sof-<platform>-<function>-id<BE id number>.tplg
> +	 * where <platform> is required for functions that depend on NHLT blobs (e.g. DMIC/SSP)
> +	 * as the nhlt blob is platform dependent.
> +	 */
> +	switch (tplg_dev) {
> +	case TPLG_DEVICE_INTEL_PCH_DMIC:
> +	case TPLG_DEVICE_SSP_JACK:
> +	case TPLG_DEVICE_SSP_AMP:
> +	case TPLG_DEVICE_SSP_BT:
> +	case TPLG_DEVICE_SSP_HDMI_IN:
> +	case TPLG_DEVICE_ACP_SDW_JACK:
> +	case TPLG_DEVICE_ACP_SDW_AMP:
> +	case TPLG_DEVICE_ACP_SDW_MIC:
> +	case TPLG_DEVICE_ACP_DMIC:
> +		filename = devm_kasprintf(dev, GFP_KERNEL, "%s/sof-%s-%s-id%d.tplg",
> +					  prefix, platform, tplg_dev_name, dai_link_id);
> +		break;
> +	default:
> +		filename = devm_kasprintf(dev, GFP_KERNEL, "%s/sof-%s-id%d.tplg",
> +					  prefix, tplg_dev_name, dai_link_id);
> +		break;
> +	}
> +
> +	return filename;

This will also sonstruct a SOF specific file name, yet the name reads
generic.

> +}
> +EXPORT_SYMBOL_GPL(snd_soc_acpi_get_tplg_filename);
> +
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("ALSA SoC ACPI module");

-- 
Péter