ASoC: skl: always select SND_SOC_HDAC_HDA

Message ID 20181102112456.780127-1-arnd@arndb.de
State New
Headers show
Series
  • ASoC: skl: always select SND_SOC_HDAC_HDA
Related show

Commit Message

Arnd Bergmann Nov. 2, 2018, 11:24 a.m.
The skylake sound support is written to work both with or without
CONFIG_SND_SOC_HDAC_HDA, and uses an #ifdef to decide whether it should
link against that. However, this fails with SND_SOC_ALL_CODECS=m or
SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH=m when the Skylake support itself
is built-in, with this link error:

sound/soc/intel/skylake/skl.o: In function `skl_probe':
skl.c:(.text+0x56c): undefined reference to `snd_soc_hdac_hda_get_ops'

Using an explicit 'select' here simplifies the logic and prevents
it from happening, at the cost of always including the compile
time dependency.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 sound/soc/intel/Kconfig       | 1 +
 sound/soc/intel/skylake/skl.c | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

-- 
2.18.0

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Comments

Takashi Iwai Nov. 5, 2018, 3:25 p.m. | #1
On Mon, 05 Nov 2018 16:07:50 +0100,
Arnd Bergmann wrote:
> 

> On 11/5/18, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> > On Sun, Nov 04, 2018 at 10:45:17AM -0600, Pierre-Louis Bossart wrote:

> >

> >> So yes indeed we have to add a select HDAC_HDA statement under the

> >> SKYLAKE

> >> config - i just don't know of any other means to say "don't build

> >> HDAC_HDA

> >> as a module when SKYLAKE is buit-in"

> >

> > We have this ("strange") lines over the drivers:

> >

> > config BAR

> > depends on FOO || FOO=n

> >

> > which guarantees that FOO will be not module when BAR is built-in.

> 

> That's what I normally use, but I could not figure this one out.

> One problem is that SND_SOC_ALL_CODECS selects

> SND_SOC_HDAC_HDA, and SND_SOC_ALL_CODECS itself

> may be =m, causing the failure for

> SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH=y.

> 

> It might work with a separate dummy symbol:

> 

> config SND_SOC_HDAC_HDA_FORCE

>          tristate

>          depends on SND_SOC_ALL_CODECS != n

>          default SND_SOC_INTEL_SKYLAKE

>          select SND_SOC_HDAC_HDA

> 

> This would make SND_SOC_HDAC_HDA built-in even

> with SND_SOC_ALL_CODECS=m and

> SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH=n

> 

> It seems a bit ugly, and would need some testing.


The mixture of depends and select is often more confusing, so IMO, we
should align with only select as Pierre's suggestion, in this
particular case.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Patch

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 0caa1f4eb94d..c21ce7624af1 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -109,6 +109,7 @@  config SND_SOC_INTEL_SKYLAKE
 	depends on PCI && ACPI
 	select SND_HDA_EXT_CORE
 	select SND_HDA_DSP_LOADER
+	select SND_SOC_HDAC_HDA
 	select SND_SOC_TOPOLOGY
 	select SND_SOC_INTEL_SST
 	select SND_SOC_ACPI_INTEL_MATCH
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 29225623b4b4..1069ee265ce5 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -870,9 +870,7 @@  static int skl_create(struct pci_dev *pci,
 	hbus = skl_to_hbus(skl);
 	bus = skl_to_bus(skl);
 
-#if IS_ENABLED(CONFIG_SND_SOC_HDAC_HDA)
 	ext_ops = snd_soc_hdac_hda_get_ops();
-#endif
 	snd_hdac_ext_bus_init(bus, &pci->dev, &bus_core_ops, io_ops, ext_ops);
 	bus->use_posbuf = 1;
 	skl->pci = pci;