On Tue, Jan 12, 2021 at 10:03 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
On 1/12/21 2:32 PM, Arnd Bergmann wrote:
From: Arnd Bergmann arnd@arndb.de
The Kconfig logic around SND_SOC_SOF_INTEL_SOUNDWIRE tries to ensure that all sound modules can be built with the minimal dependencies, but this fails in some configurations:
x86_64-linux-ld: sound/hda/intel-dsp-config.o: in function `snd_intel_dsp_driver_probe': intel-dsp-config.c:(.text+0x134): undefined reference to `sdw_intel_acpi_scan'
Specifically, this happens if the dsp-config driver is built-in but does not need to use soundwire, while CONFIG_SOUNDWIRE_INTEL is enabled as a loadable module.
An easier and more correct way to do this is to remove CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE_LINK and instead have the two drivers that can link against SOUNDWIRE_INTEL, i.e. DSP_CONFIG and SND_SOC_SOF_HDA, select that driver whenever SND_SOC_SOF_INTEL_SOUNDWIRE_LINK is set.
This however means that SND_SOC_SOF_INTEL_SOUNDWIRE cannot be selected by users when SOUNDWIRE is only usable by loadable modules and one or more drivers using SND_SOC_SOF_INTEL_SOUNDWIRE_LINK_BASELINE is built-in.
The problem is real, but the proposal isn't completely right, there is absolutely no logical link or functional dependency between INTEL_DSP_CONFIG and SOUNDWIRE.
If that is true, would it be possible to move the call to sdw_intel_acpi_scan() out of these drivers and one layer higher where the dependency actually is?
I was indeed wondering whether the intel-dsp-config.c is just another layering violation: this is another generic piece of code that seems to contain too much knowledge about specific hardware implementations.
We have a similar case with HDaudio, the two buses can be selected as tristates, but the SOF configuration needs to match.
In both cases, either we add a 'depends' and users need to make sure the configurations match on the two sides. Or we use select but one of the selections will be overridden and becomes meaningless.
Maybe something like this:
config SND_SOC_SOF_INTEL_SOUNDWIRE - bool "SOF support for SoundWire" + tristate "SOF support for SoundWire" - depends on SOUNDWIRE && ACPI + depends on SND_SOC_SOF_INTEL_SOUNDWIRE_LINK_BASELINE + depends on SOUNDWIRE + depends on ACPI + depends on !(SOUNDWIRE=m && SND_SOC_SOF_INTEL_SOUNDWIRE_LINK_BASELINE=y) + select SOUNDWIRE_INTEL
I have not tried it, but that should keep it all in one place.
Arnd, do you mind if I give it a try on my side?
I have no specific attachment to my patch, this was just what I came up with to fix the build regression locally.
Arnd