On Thu, 14 Jan 2021 14:45:10 +0100,
Kai Vehmanen wrote:
>
> Hi,
>
> On Thu, 14 Jan 2021, Takashi Iwai wrote:
>
> > On Thu, 14 Jan 2021 12:55:58 +0100, Kai Vehmanen wrote:
> > > --- a/sound/soc/sof/sof-pci-dev.c
> > > +++ b/sound/soc/sof/sof-pci-dev.c
> [...]
> > > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_TIGERLAKE)
> > > +#if IS_ENABLED(CONFIG_SND_SOC_SOF_TIGERLAKE) || IS_ENABLED(CONFIG_SND_SOC_SOF_ALDERLAKE)
> > > static const struct sof_dev_desc tgl_desc = {
> > > .machines = snd_soc_acpi_intel_tgl_machines,
> > > .alt_machines = snd_soc_acpi_intel_tgl_sdw_machines,
> >
> > I guess snd_soc_acpi_intel_tgl_* would be missing even by this patch
> > alone if TIGERLAKE=n and ALDERLAKE=y.
>
> The ACPI matching tables soc-acpi-intel-match.h are not behind Kconfig
> ifdefs, so this is ok. I did actually test all the build combinations this
> time :) and the builds go through.
>
> > IMO, the easiest fix would be to select CONFIG_SND_SOC_SOF_TIGERLAKE
> > from CONFIG_SND_SOC_SOF_ALDERLAKE forcibly in Kconfig instead. Then
> > no tweak of ifdef is needed.
>
> I'd rather keep this here. We may later update sof_dev_desc for ADL and no
> longer reuse tgl_desc, and then it's easier to cleanup if the #ifdef is
> only here, close to where the dependency is.
>
> I too think the ifdefs are too finegrained, so maybe the next step is to
> just drop these, and/or reduce their number. We can review and discuss
> this when we refactor sof-pci-dev to address the concern Arnd raised.
Fair enough. Thanks.
Takashi
