| Message ID | 20181222090720.19234-1-okaya@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Specify CONFIG_PCI dependency explicitly | expand |
On Sat, Dec 22, 2018 at 6:47 PM Sinan Kaya <okaya@kernel.org> wrote: > > Collect reviewed-by for 3/11 and 6/11 from Lukas > Add a few words about 8/11 why PCI dependency is being added > > Sinan Kaya (11): > ACPI / LPSS: Add guards against CONFIG_PCI > ata: make PCI dependency explicit for PATA_ACPI > vga-switcheroo: make PCI dependency explicit > platform/x86: make PCI dependency explicit > platform/x86: intel_pmc: Hide PCI specific pieces behind CONFIG_PCI > apple-gmux: Make PCI dependency explicit > drivers: thermal: Hide PCI driver when CONFIG_PCI is unset > ASoC: Intel: Make PCI dependency explicit > mmc: add PCI dependency into IOSF_MBI > x86: select IOSF_MBI only when CONFIG_PCI is set > drivers: thermal: Make PCI dependency explicit > > arch/x86/Kconfig | 2 +- > drivers/acpi/acpi_lpss.c | 5 ++++- > drivers/ata/Kconfig | 2 +- > drivers/gpu/vga/Kconfig | 1 + > drivers/mmc/host/Kconfig | 2 +- > drivers/platform/x86/Kconfig | 3 ++- > drivers/platform/x86/intel_pmc_ipc.c | 6 ++++++ > drivers/thermal/intel/int340x_thermal/Kconfig | 2 +- > .../intel/int340x_thermal/processor_thermal_device.c | 8 ++++++-- > sound/soc/intel/Kconfig | 2 +- > 10 files changed, 24 insertions(+), 9 deletions(-) Why exactly do you think that adding #ifdefs around stuff in random places just because then don't build without CONFIG_PCI makes any sense at all? Please don't do that. If something requires CONFIG_PCI to build, make it depend on PCI, unless you *know* it for a fact that it *will* work with your new #ifdefs and without PCI. In which case the changelog must say that and specify the platform you have tested in on.
On 12/23/2018 10:52 PM, Rafael J. Wysocki wrote: > On Sat, Dec 22, 2018 at 6:47 PM Sinan Kaya <okaya@kernel.org> wrote: >> >> Collect reviewed-by for 3/11 and 6/11 from Lukas >> Add a few words about 8/11 why PCI dependency is being added >> >> Sinan Kaya (11): >> ACPI / LPSS: Add guards against CONFIG_PCI >> ata: make PCI dependency explicit for PATA_ACPI >> vga-switcheroo: make PCI dependency explicit >> platform/x86: make PCI dependency explicit >> platform/x86: intel_pmc: Hide PCI specific pieces behind CONFIG_PCI >> apple-gmux: Make PCI dependency explicit >> drivers: thermal: Hide PCI driver when CONFIG_PCI is unset >> ASoC: Intel: Make PCI dependency explicit >> mmc: add PCI dependency into IOSF_MBI >> x86: select IOSF_MBI only when CONFIG_PCI is set >> drivers: thermal: Make PCI dependency explicit >> >> arch/x86/Kconfig | 2 +- >> drivers/acpi/acpi_lpss.c | 5 ++++- >> drivers/ata/Kconfig | 2 +- >> drivers/gpu/vga/Kconfig | 1 + >> drivers/mmc/host/Kconfig | 2 +- >> drivers/platform/x86/Kconfig | 3 ++- >> drivers/platform/x86/intel_pmc_ipc.c | 6 ++++++ >> drivers/thermal/intel/int340x_thermal/Kconfig | 2 +- >> .../intel/int340x_thermal/processor_thermal_device.c | 8 ++++++-- >> sound/soc/intel/Kconfig | 2 +- >> 10 files changed, 24 insertions(+), 9 deletions(-) > > Why exactly do you think that adding #ifdefs around stuff in random > places just because then don't build without CONFIG_PCI makes any > sense at all? I don't believe it is fair to say that I threw in random #ifdef into all places. Even if I did, we rely on code review to get these issues resolved. I have already shown (v15) that I take feedback and move the code into a shape where it makes more sense. I don't claim that I'm familiar with the entire list of code. I was hoping the maintainer of each file to chime in and let me know what they prefer or what makes more sense. On the other hand, I can certainly say that I did compile test only. No functional testing. > > Please don't do that. If something requires CONFIG_PCI to build, make > it depend on PCI, unless you *know* it for a fact that it *will* work > with your new #ifdefs and without PCI. In which case the changelog > must say that and specify the platform you have tested in on. > OK. I can also do that.
On Sun, Dec 23, 2018 at 11:29 PM Sinan Kaya <okaya@kernel.org> wrote: > > On 12/23/2018 10:52 PM, Rafael J. Wysocki wrote: > > On Sat, Dec 22, 2018 at 6:47 PM Sinan Kaya <okaya@kernel.org> wrote: > >> > >> Collect reviewed-by for 3/11 and 6/11 from Lukas > >> Add a few words about 8/11 why PCI dependency is being added > >> > >> Sinan Kaya (11): > >> ACPI / LPSS: Add guards against CONFIG_PCI > >> ata: make PCI dependency explicit for PATA_ACPI > >> vga-switcheroo: make PCI dependency explicit > >> platform/x86: make PCI dependency explicit > >> platform/x86: intel_pmc: Hide PCI specific pieces behind CONFIG_PCI > >> apple-gmux: Make PCI dependency explicit > >> drivers: thermal: Hide PCI driver when CONFIG_PCI is unset > >> ASoC: Intel: Make PCI dependency explicit > >> mmc: add PCI dependency into IOSF_MBI > >> x86: select IOSF_MBI only when CONFIG_PCI is set > >> drivers: thermal: Make PCI dependency explicit > >> > >> arch/x86/Kconfig | 2 +- > >> drivers/acpi/acpi_lpss.c | 5 ++++- > >> drivers/ata/Kconfig | 2 +- > >> drivers/gpu/vga/Kconfig | 1 + > >> drivers/mmc/host/Kconfig | 2 +- > >> drivers/platform/x86/Kconfig | 3 ++- > >> drivers/platform/x86/intel_pmc_ipc.c | 6 ++++++ > >> drivers/thermal/intel/int340x_thermal/Kconfig | 2 +- > >> .../intel/int340x_thermal/processor_thermal_device.c | 8 ++++++-- > >> sound/soc/intel/Kconfig | 2 +- > >> 10 files changed, 24 insertions(+), 9 deletions(-) > > > > Why exactly do you think that adding #ifdefs around stuff in random > > places just because then don't build without CONFIG_PCI makes any > > sense at all? > > I don't believe it is fair to say that I threw in random #ifdef into > all places. Well, it isn't. Sorry about that. > Even if I did, we rely on code review to get these issues resolved. > I have already shown (v15) that I take feedback and move the code into a > shape where it makes more sense. > > I don't claim that I'm familiar with the entire list of code. > > I was hoping the maintainer of each file to chime in and let me > know what they prefer or what makes more sense. Even so, without checking if the code works in all cases with the changes made, that is questionable overall. By adding a new #ifdef anywhere in the code, you basically increase the test matrix for that code by a factor of 2, so there should be a very good reason to do that.