diff mbox series

[v4,08/11] ASoC: Intel: atom: Make PCI dependency explicit

Message ID 20181230195612.6657-9-okaya@kernel.org (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Sinan Kaya Dec. 30, 2018, 7:56 p.m. UTC
Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on
CONFIG_PCI set but this is not specified anywhere.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 sound/soc/intel/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rafael J. Wysocki Jan. 2, 2019, 9:34 a.m. UTC | #1
On Sun, Dec 30, 2018 at 8:56 PM Sinan Kaya <okaya@kernel.org> wrote:
>
> Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on
> CONFIG_PCI set but this is not specified anywhere.

IMO it would be better to say

"After commit 5d32a66541c46 (PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set) dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly.  For this reason, add a direct dependency on
CONFIG_PCI to the IOSF_MBI driver."

If you did that, the reviewers would know upfront what this was about
and that might save at least one back-and-forth e-mail exchange in
each case.
Rafael J. Wysocki Jan. 2, 2019, 9:36 a.m. UTC | #2
On Wed, Jan 2, 2019 at 10:34 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sun, Dec 30, 2018 at 8:56 PM Sinan Kaya <okaya@kernel.org> wrote:
> >
> > Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on
> > CONFIG_PCI set but this is not specified anywhere.
>
> IMO it would be better to say
>
> "After commit 5d32a66541c46 (PCI/ACPI: Allow ACPI to be built without
> CONFIG_PCI set) dependencies on CONFIG_PCI that previously were
> satisfied implicitly through dependencies on CONFIG_ACPI have to be
> specified directly.  For this reason, add a direct dependency on
> CONFIG_PCI to the IOSF_MBI driver."
>
> If you did that, the reviewers would know upfront what this was about
> and that might save at least one back-and-forth e-mail exchange in
> each case.

But, of course, the changelog doesn't even match the patch contents in
this particular case.  Please fix that.
Pierre-Louis Bossart Jan. 2, 2019, 4:34 p.m. UTC | #3
> Please check out this tag next-20181224 and apply the patches afterwards.
Thanks, will do. I think this patchset will uncover additional 
inconsistencies, e.g. for legacy Haswell/Broadwell/Baytrail the machine 
drivers depend on X86_INTEL_LPSS, which depends in turn on PCI, but the 
platform drivers only depend on ACPI, so there is a risk of creating a 
config that makes no sense (or should only be used for COMPILE_TEST)
diff mbox series

Patch

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 2fd1b61e8331..b0764b2fe001 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -91,7 +91,7 @@  config SND_SST_ATOM_HIFI2_PLATFORM_PCI
 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