On Wed, Jan 2, 2019 at 9:33 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
I have three opens with this ACPI/PCI change
- the baseline change fails on my cross-compilation checks, see below
the result of the attached script (simplification of the one I use to avoid 0day reports).
What baseline change?
That failure is not related to PCI if I'm not missing anything.
- there are different patterns to express the dependency on PCI e.g.
config MMC_SDHCI_ACPI tristate "SDHCI support for ACPI enumerated SDHCI controllers" depends on MMC_SDHCI && ACPI
- select IOSF_MBI if X86
- select IOSF_MBI if (X86 && PCI)
but
config SND_SST_ATOM_HIFI2_PLATFORM_ACPI tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms" default ACPI
- depends on X86 && ACPI
- depends on X86 && ACPI && PCI select SND_SST_IPC_ACPI select SND_SST_ATOM_HIFI2_PLATFORM select SND_SOC_ACPI_INTEL_MATCH
IOSF is only needed for Baytrail-CR detection, and the code will compile fine without it, so maybe it'd be a better model if you used the following diff?
diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index 2fd1b61e8331..68af0ea5c96c 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -95,7 +95,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_ACPI select SND_SST_IPC_ACPI select SND_SST_ATOM_HIFI2_PLATFORM select SND_SOC_ACPI_INTEL_MATCH
select IOSF_MBI
select IOSF_MBI if PCI
Well, does it actually make sense to ever set SND_SST_ATOM_HIFI2_PLATFORM_ACPI without PCI?
- All the Intel machine drivers depend on X86_INTEL_LPSS which depends
on PCI. But for Baytrail/Haswell/Broadwell we have only a dependency on ACPI, so we expose drivers that can be selected but fail on probe since there are no machine drivers. I am not sure if we want to be strict and only expose meaningful configurations, or allow for more compilations tests and corner cases?
I would only expose meaningful configurations to start with and then *maybe* relax that going forward as long as the benefit is worth it.
Cheers, Rafael