Re: [PATCH 2/2] ASoC: SOF: SND_INTEL_DSP_CONFIG dependency

From: Takashi Iwai <tiwai@suse.de>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Arnd Bergmann <arnd@kernel.org>,
	alsa-devel@alsa-project.org, broonie@kernel.org,
	vkoul@kernel.org, Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH 2/2] ASoC: SOF: SND_INTEL_DSP_CONFIG dependency
Date: Fri, 22 Jan 2021 09:58:01 +0100	[thread overview]
Message-ID: <s5hwnw58i1y.wl-tiwai@suse.de> (raw)
In-Reply-To: <20210122005725.94163-3-pierre-louis.bossart@linux.intel.com>

On Fri, 22 Jan 2021 01:57:25 +0100,
Pierre-Louis Bossart wrote:
> 
> The sof-pci-dev driver fails to link when built into the kernel
> and CONFIG_SND_INTEL_DSP_CONFIG is set to =m:
> 
> arm-linux-gnueabi-ld: sound/soc/sof/sof-pci-dev.o: in function `sof_pci_probe':
> sof-pci-dev.c:(.text+0x1c): undefined reference to `snd_intel_dsp_driver_probe'
> 
> As a temporary fix, use IS_REACHABLE to prevent the problem from
> happening. A more complete solution is to move this code to
> Intel-specific parts, restructure the drivers and Kconfig as discussed
> with Arnd Bergmann and Takashi Iwai.
> 
> Fixes: 82d9d54a6c0e ("ALSA: hda: add Intel DSP configuration / probe code")
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

This might not be good enough.  The code may still refer to the
snd_intel_dsp_probe() symbol even if IS_REACHABLE() is false
(although, practically seen, gcc should optimize it out).

You need #if IS_REACHABLE() instead of the plain if.
Then you don't need to change the indentation, and the patch will be
eventually smaller, too :)

thanks,

Takashi

> ---
>  sound/soc/sof/sof-acpi-dev.c | 11 ++++++-----
>  sound/soc/sof/sof-pci-dev.c  | 10 ++++++----
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/soc/sof/sof-acpi-dev.c b/sound/soc/sof/sof-acpi-dev.c
> index 2a369c2c6551..cc2e257087e4 100644
> --- a/sound/soc/sof/sof-acpi-dev.c
> +++ b/sound/soc/sof/sof-acpi-dev.c
> @@ -131,12 +131,13 @@ static int sof_acpi_probe(struct platform_device *pdev)
>  	if (!id)
>  		return -ENODEV;
>  
> -	ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
> -	if (ret != SND_INTEL_DSP_DRIVER_ANY && ret != SND_INTEL_DSP_DRIVER_SOF) {
> -		dev_dbg(dev, "SOF ACPI driver not selected, aborting probe\n");
> -		return -ENODEV;
> +	if (IS_REACHABLE(CONFIG_SND_INTEL_DSP_CONFIG)) {
> +		ret = snd_intel_acpi_dsp_driver_probe(dev, id->id);
> +		if (ret != SND_INTEL_DSP_DRIVER_ANY && ret != SND_INTEL_DSP_DRIVER_SOF) {
> +			dev_dbg(dev, "SOF ACPI driver not selected, aborting probe\n");
> +			return -ENODEV;
> +		}
>  	}
> -
>  	dev_dbg(dev, "ACPI DSP detected");
>  
>  	sof_pdata = devm_kzalloc(dev, sizeof(*sof_pdata), GFP_KERNEL);
> diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
> index 7757463bd81a..388772f9c4d2 100644
> --- a/sound/soc/sof/sof-pci-dev.c
> +++ b/sound/soc/sof/sof-pci-dev.c
> @@ -346,10 +346,12 @@ static int sof_pci_probe(struct pci_dev *pci,
>  	const struct snd_sof_dsp_ops *ops;
>  	int ret;
>  
> -	ret = snd_intel_dsp_driver_probe(pci);
> -	if (ret != SND_INTEL_DSP_DRIVER_ANY && ret != SND_INTEL_DSP_DRIVER_SOF) {
> -		dev_dbg(&pci->dev, "SOF PCI driver not selected, aborting probe\n");
> -		return -ENODEV;
> +	if (IS_REACHABLE(CONFIG_SND_INTEL_DSP_CONFIG)) {
> +		ret = snd_intel_dsp_driver_probe(pci);
> +		if (ret != SND_INTEL_DSP_DRIVER_ANY && ret != SND_INTEL_DSP_DRIVER_SOF) {
> +			dev_dbg(&pci->dev, "SOF PCI driver not selected, aborting probe\n");
> +			return -ENODEV;
> +		}
>  	}
>  	dev_dbg(&pci->dev, "PCI DSP detected");
>  
> -- 
> 2.25.1
> 