| Message ID | 20181230195612.6657-9-okaya@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Specify CONFIG_PCI dependency explicitly | expand |
On Sun, Dec 30, 2018 at 07:56:09PM +0000, Sinan Kaya wrote: > Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on > CONFIG_PCI set but this is not specified anywhere. I don't have the cover letter or anything for this series, what's going on with dependencies?
On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote: > > On Sun, Dec 30, 2018 at 07:56:09PM +0000, Sinan Kaya wrote: > > Code does unconditional select for IOSF_MBI. IOSF_MBI driver depends on > > CONFIG_PCI set but this is not specified anywhere. > > I don't have the cover letter or anything for this series, what's going > on with dependencies? Here is the executive summary: I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI used to select PCI. This is no longer true. You can build an ACPI system without any PCI devices.
On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote: > On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote: > > I don't have the cover letter or anything for this series, what's going > > on with dependencies? > Here is the executive summary: > I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI > used to select PCI. This is no longer true. > You can build an ACPI system without any PCI devices. So there's no dependency and I can just apply this?
On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote: > > On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote: > > On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote: > > > > I don't have the cover letter or anything for this series, what's going > > > on with dependencies? > > > Here is the executive summary: > > > I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI > > used to select PCI. This is no longer true. > > > You can build an ACPI system without any PCI devices. > > So there's no dependency and I can just apply this? The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch.
On 12/31/18 1:35 PM, Sinan Kaya wrote: > On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote: >> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote: >>> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote: >>>> I don't have the cover letter or anything for this series, what's going >>>> on with dependencies? >>> Here is the executive summary: >>> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI >>> used to select PCI. This is no longer true. >>> You can build an ACPI system without any PCI devices. >> So there's no dependency and I can just apply this? > The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch. Anytime we change the Kconfig settings for audio, we get all kinds of problems with randconfig and 0day/kbuild due to depend/select issues. I'd like to give this a spin first, can you share a link to the entire series? Thanks!
On Mon, Dec 31, 2018 at 11:29 PM Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote: > > > On 12/31/18 1:35 PM, Sinan Kaya wrote: > > On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote: > >> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote: > >>> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote: > >>>> I don't have the cover letter or anything for this series, what's going > >>>> on with dependencies? > >>> Here is the executive summary: > >>> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI > >>> used to select PCI. This is no longer true. > >>> You can build an ACPI system without any PCI devices. > >> So there's no dependency and I can just apply this? > > The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch. > Anytime we change the Kconfig settings for audio, we get all kinds of > problems with randconfig and 0day/kbuild due to depend/select issues. > I'd like to give this a spin first, can you share a link to the entire > series? Thanks! Sure, You can find them here https://lore.kernel.org/patchwork/patch/1028330/ Click related.
On 12/31/18 2:35 PM, Sinan Kaya wrote: > On Mon, Dec 31, 2018 at 11:29 PM Pierre-Louis Bossart > <pierre-louis.bossart@linux.intel.com> wrote: >> >> On 12/31/18 1:35 PM, Sinan Kaya wrote: >>> On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote: >>>> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote: >>>>> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote: >>>>>> I don't have the cover letter or anything for this series, what's going >>>>>> on with dependencies? >>>>> Here is the executive summary: >>>>> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI >>>>> used to select PCI. This is no longer true. >>>>> You can build an ACPI system without any PCI devices. >>>> So there's no dependency and I can just apply this? >>> The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch. >> Anytime we change the Kconfig settings for audio, we get all kinds of >> problems with randconfig and 0day/kbuild due to depend/select issues. >> I'd like to give this a spin first, can you share a link to the entire >> series? Thanks! > Sure, > > You can find them here > > https://lore.kernel.org/patchwork/patch/1028330/ > > Click related. Something must be missing, I get compilation errors when PCI is not defined? And I see tons of references to pci stuff in drivers/acpi. drivers/acpi/reboot.c: In function ‘acpi_reboot’: drivers/acpi/reboot.c:37:10: error: implicit declaration of function ‘pci_find_bus’; did you mean ‘pci_find_next_bus’? [-Werror=implicit-function-declaration] bus0 = pci_find_bus(0, 0); ^~~~~~~~~~~~ pci_find_next_bus drivers/acpi/reboot.c:37:8: warning: assignment makes pointer from integer without a cast [-Wint-conversion] bus0 = pci_find_bus(0, 0); ^ drivers/acpi/reboot.c:45:3: error: implicit declaration of function ‘pci_bus_write_config_byte’; did you mean ‘pci_write_config_byte’? [-Werror=implicit-function-declaration] pci_bus_write_config_byte(bus0, devfn, ^~~~~~~~~~~~~~~~~~~~~~~~~ pci_write_config_byte
On Tue, Jan 1, 2019 at 12:42 AM Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote: > > > On 12/31/18 2:35 PM, Sinan Kaya wrote: > > On Mon, Dec 31, 2018 at 11:29 PM Pierre-Louis Bossart > > <pierre-louis.bossart@linux.intel.com> wrote: > >> > >> On 12/31/18 1:35 PM, Sinan Kaya wrote: > >>> On Mon, Dec 31, 2018 at 10:30 PM Mark Brown <broonie@kernel.org> wrote: > >>>> On Mon, Dec 31, 2018 at 08:52:52PM +0300, Sinan Kaya wrote: > >>>>> On Mon, Dec 31, 2018 at 8:47 PM Mark Brown <broonie@kernel.org> wrote: > >>>>>> I don't have the cover letter or anything for this series, what's going > >>>>>> on with dependencies? > >>>>> Here is the executive summary: > >>>>> I have a changeset that separates ACPI from PCI on 4.21. CONFIG_ACPI > >>>>> used to select PCI. This is no longer true. > >>>>> You can build an ACPI system without any PCI devices. > >>>> So there's no dependency and I can just apply this? > >>> The plan is to apply this patchset via ACPI tree. Need an Acked-by per patch. > >> Anytime we change the Kconfig settings for audio, we get all kinds of > >> problems with randconfig and 0day/kbuild due to depend/select issues. > >> I'd like to give this a spin first, can you share a link to the entire > >> series? Thanks! > > Sure, > > > > You can find them here > > > > https://lore.kernel.org/patchwork/patch/1028330/ > > > > Click related. > > Something must be missing, I get compilation errors when PCI is not > defined? And I see tons of references to pci stuff in drivers/acpi. > > drivers/acpi/reboot.c: In function ‘acpi_reboot’: > drivers/acpi/reboot.c:37:10: error: implicit declaration of function > ‘pci_find_bus’; did you mean ‘pci_find_next_bus’? > [-Werror=implicit-function-declaration] > bus0 = pci_find_bus(0, 0); > ^~~~~~~~~~~~ > pci_find_next_bus > drivers/acpi/reboot.c:37:8: warning: assignment makes pointer from > integer without a cast [-Wint-conversion] > bus0 = pci_find_bus(0, 0); > ^ > drivers/acpi/reboot.c:45:3: error: implicit declaration of function > ‘pci_bus_write_config_byte’; did you mean ‘pci_write_config_byte’? > [-Werror=implicit-function-declaration] > pci_bus_write_config_byte(bus0, devfn, > ^~~~~~~~~~~~~~~~~~~~~~~~~ > pci_write_config_byte > > Please check out this tag next-20181224 and apply the patches afterwards.
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.
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.
> 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 --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
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(-)