Hi, On 4/28/22 08:24, Paul Gortmaker wrote: > This caught my eye when I saw it was def_bool and hence largely > pointless to have a Kconfig for it at all. > > Yet there is no reason why you shouldn't be able to opt out of Atom > platform support if you only care about desktop and server class CPUs. > > It was introduced as def_bool, but there is no obvious reason as to why > it was forcibly built-in for everyone, other than LPSS implicitly > relying on it (which is now fixed). So here we fix up the Kconfig and > open the door for people to opt out. > > Since putting "default y" on anything that isn't absolutely essential is > generally frowned upon, I made the default be CONFIG_MATOM. People who > use "make oldconfig" or similar won't notice any difference. > > The two "unchanged" lines for PCI and COMMON_CLK appear in the diff from > fixing a whitespace issue that somehow managed to live on despite being > moved between two different Kconfig files since its introduction. > > Cc: Aubrey Li <aubrey.li@xxxxxxxxxxxxxxx> > Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Cc: Hans de Goede <hdegoede@xxxxxxxxxx> > Cc: Mark Gross <markgross@xxxxxxxxxx> > Cc: platform-driver-x86@xxxxxxxxxxxxxxx > Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx> > --- > drivers/platform/x86/Kconfig | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index f08ad85683cb..86459e99d831 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1163,6 +1163,11 @@ config WINMATE_FM07_KEYS > endif # X86_PLATFORM_DEVICES > > config PMC_ATOM > - def_bool y > - depends on PCI > - select COMMON_CLK > + bool "Intel Atom SOC Power Management Controller driver" > + default MATOM Besides the remarks from Andy, this does seem like a weird default, MATOM means that gcc is passed -march=atom nothing more and nothing less. For a distro kernel which may e.g. set CONFIG_GENERIC_CPU we still want this enabled. But as said it is brought in by CONFIG_X86_INTEL_LPSS when that is set. Thinking more about this I think it might be best to just do this instead, replacing patch 2 + 4 of this set: diff --git a/drivers/clk/x86/Makefile b/drivers/clk/x86/Makefile index 1244c4e568ff..c2088b3c4081 100644 --- a/drivers/clk/x86/Makefile +++ b/drivers/clk/x86/Makefile @@ -1,6 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-$(CONFIG_PMC_ATOM) += clk-pmc-atom.o obj-$(CONFIG_X86_AMD_PLATFORM_DEVICE) += clk-fch.o -clk-x86-lpss-y := clk-lpss-atom.o -obj-$(CONFIG_X86_INTEL_LPSS) += clk-x86-lpss.o +obj-$(CONFIG_X86_INTEL_LPSS) += clk-lpss-atom.o clk-pmc-atom.o obj-$(CONFIG_CLK_LGM_CGU) += clk-cgu.o clk-cgu-pll.o clk-lgm.o diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index f08ad85683cb..85c396a43048 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -1161,8 +1161,3 @@ config WINMATE_FM07_KEYS that delivers key events when these buttons are pressed. endif # X86_PLATFORM_DEVICES - -config PMC_ATOM - def_bool y - depends on PCI - select COMMON_CLK diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 4a59f47a46e2..cc2a74713313 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -126,7 +126,7 @@ obj-$(CONFIG_INTEL_SCU_PCI) += intel_scu_pcidrv.o obj-$(CONFIG_INTEL_SCU_PLATFORM) += intel_scu_pltdrv.o obj-$(CONFIG_INTEL_SCU_WDT) += intel_scu_wdt.o obj-$(CONFIG_INTEL_SCU_IPC_UTIL) += intel_scu_ipcutil.o -obj-$(CONFIG_PMC_ATOM) += pmc_atom.o +obj-$(CONFIG_X86_INTEL_LPSS) += pmc_atom.o # Siemens Simatic Industrial PCs obj-$(CONFIG_SIEMENS_SIMATIC_IPC) += simatic-ipc.o This just folds the enabling of the pmc_atom code into the same Kconfig option as the one used the the LPSS code, so this actually simplify things; while at the same time making it possible to not build the pmc_atom code by unselecting the CONFIG_X86_INTEL_LPSS option. Regards, Hans
