Re: [PATCH v2] ASoC: SOF: Intel: initial support to AlderLake-P

[Takashi Iwai <tiwai@suse.de>]

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