x86: Make Atom PMC driver configurable.
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
arch/x86/Kconfig +1 -1
@@ -2485,7 +2485,7 @@ config X86_DMA_REMAP depends on STA2X11 config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver" depends on PCI source "net/Kconfig"
Hi,
Quoted 10 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>arch/x86/Kconfig +0
-0
arch/x86/Kconfig config X86_DMA_REMAP · line 2488
Quoted 5 lines
arch/x86/Kconfig +1
-1@@ -2485,7 +2485,7 @@ config X86_DMA_REMAP depends on STA2X11 config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver"
looks like you should still have this as default y just to make sure you a simple defconfig still enables this as it did before.
Signature · 1 line
balbi
Quoted 15 lines · Felipe Balbi
Hi,
Quoted 12 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>arch/x86/Kconfig +0
-0@@ -2485,7 +2485,7 @@ config X86_DMA_REMAP depends on STA2X11
Quoted 6 lines
arch/x86/Kconfig config X86_DMA_REMAP · line 2488
Quoted 3 lines
arch/x86/Kconfig +1
-1config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver"looks like you should still have this as default y just to make sure you a simple defconfig still enables this as it did before.
I could, but why should this be default y ? There's no real justification to inflict this on everyone, given atom is at best a niche area of x86.
Dave
Quoted 23 lines · Dave Jones
Quoted 15 lines · Felipe Balbi
Hi,
Quoted 12 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>arch/x86/Kconfig +0
-0@@ -2485,7 +2485,7 @@ config X86_DMA_REMAP depends on STA2X11Quoted 6 lines
arch/x86/Kconfig config X86_DMA_REMAP · line 2488
Quoted 3 lines
arch/x86/Kconfig +1
-1config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver"looks like you should still have this as default y just to make sure you a simple defconfig still enables this as it did before.
Quoted 3 lines
I could, but why should this be default y ? There's no real justification to inflict this on everyone, given atom is at best a niche area of x86.
well, because it already was a bool ? There might be distros out there who would mysteriously loose PMC support after upgrade the kernel without realizing that PMC_ATOM isn't a default y anymore.
Frankly though, no strong feelings. I won't be the one having to tell users to change their .config ;-)
Signature · 1 line
balbi
Quoted 23 lines · On Wed, 15 Oct 2014 10:59:24 -0400 Dave Jones <[email protected]> wrote:
Quoted 15 lines · Felipe Balbi
Hi,
Quoted 12 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>arch/x86/Kconfig +0
-0@@ -2485,7 +2485,7 @@ config X86_DMA_REMAP depends on STA2X11Quoted 6 lines
arch/x86/Kconfig config X86_DMA_REMAP · line 2488
Quoted 3 lines
arch/x86/Kconfig +1
-1config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver"looks like you should still have this as default y just to make sure you a simple defconfig still enables this as it did before.
Quoted 3 lines
I could, but why should this be default y ? There's no real justification to inflict this on everyone, given atom is at best a niche area of x86.
Possibly because you work for an enterprise vendor. Atom is not remotely niche to everyone else.
This really does want to be a default Y for X86 at least, although I can see that for the enterprise market you probably don't care quite so much right now. Without that you are going to break a lot of users configurations in a deeply surprising way.
Alan
PATCH v2 1/1 x86: Make Atom PMC driver configurable.
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
arch/x86/Kconfig +2 -1
@@ -2485,7 +2485,8 @@ config X86_DMA_REMAP depends on STA2X11 config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver" + default y depends on PCI source "net/Kconfig"
Quoted 12 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>Cc One Thousand Gnomes <[email protected]>arch/x86/Kconfig +0
-0
arch/x86/Kconfig config X86_DMA_REMAP · line 2488
Quoted 5 lines
arch/x86/Kconfig +1
-1@@ -2485,7 +2485,8 @@ config X86_DMA_REMAP depends on STA2X11 config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver"
PMC driver provides core function like reboot, better to change to bool, or did you see a scenario it can be as a module?
Thanks,
-Aubrey
Quoted 6 lines
arch/x86/Kconfig +1
-0+ default y depends on PCI source "net/Kconfig"
Quoted 15 lines · Aubrey
Quoted 14 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>Cc One Thousand Gnomes <[email protected]>arch/x86/Kconfig +0
-0@@ -2485,7 +2485,8 @@ config X86_DMA_REMAP depends on STA2X11
Quoted 6 lines
arch/x86/Kconfig config X86_DMA_REMAP · line 2488
Quoted 3 lines
arch/x86/Kconfig +1
-1config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver"PMC driver provides core function like reboot, better to change to bool, or did you see a scenario it can be as a module?
All the MODULE_* stuff in arch/x86/kernel/pmc_atom.c threw me off. It could also use a help text. I suspect you might be in a better position than me to write one though.
Dave
Quoted 12 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>Cc One Thousand Gnomes <[email protected]>arch/x86/Kconfig +0
-0
arch/x86/Kconfig config X86_DMA_REMAP · line 2491
Quoted 8 lines
arch/x86/Kconfig +2
-1@@ -2485,7 +2485,8 @@ config X86_DMA_REMAP depends on STA2X11 config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver" + default y depends on PCI
So what I think should happen is to decouple of the 'must work' features from the optional debug features in this 'driver': the Atom SoC power-off quirk should be made unconditional, as long as the .config is Atom-supported (CPU_SUP_INTEL I guess).
All the other bits, such as the debugfs interface, should be in a separately and appropriately named config option, CONFIG_X86_INTEL_ATOM_PMC_DEBUG=y or so, with 'default n'.
The file should probably be split up, the quirk moved into one of the generic quirk files, while pmc_atom.c should have the debugfs interface.
That way we don't break anyone and remove the unnecessary code as well. It's also a nice clean up.
Thanks,
Ingo
Quoted 33 lines · Ingo Molnar
Quoted 12 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>Cc One Thousand Gnomes <[email protected]>arch/x86/Kconfig +0
-0arch/x86/Kconfig config X86_DMA_REMAP · line 2491
Quoted 8 lines
arch/x86/Kconfig +2
-1@@ -2485,7 +2485,8 @@ config X86_DMA_REMAP depends on STA2X11 config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver" + default y depends on PCISo what I think should happen is to decouple of the 'must work' features from the optional debug features in this 'driver': the Atom SoC power-off quirk should be made unconditional, as long as the .config is Atom-supported (
CPU_SUP_INTELI guess).All the other bits, such as the debugfs interface, should be in a separately and appropriately named config option,
CONFIG_X86_INTEL_ATOM_PMC_DEBUG=yor so, with 'default n'.
Quoted 6 lines
The file should probably be split up, the quirk moved into one of the generic quirk files, while
pmc_atom.cshould have the debugfs interface.That way we don't break anyone and remove the unnecessary code as well. It's also a nice clean up.
Thanks for the suggestion, I'll take a look if I can refine it after I clean up my plate. Please expect a delay here.
-Aubrey
Quoted 6 lines
Thanks,
Ingo
Quoted 29 lines · Ingo Molnar
Quoted 12 lines · Dave Jones
The Atom PMC driver is always built-in, regardless of whether the kernel being built is going to be run on an Atom (or even Intel) CPU.
Signed-off-by Dave Jones <[email protected]>Cc One Thousand Gnomes <[email protected]>arch/x86/Kconfig +0
-0arch/x86/Kconfig config X86_DMA_REMAP · line 2491
Quoted 8 lines
arch/x86/Kconfig +2
-1@@ -2485,7 +2485,8 @@ config X86_DMA_REMAP depends on STA2X11 config PMC_ATOM - def_bool y + tristate "Intel Atom SOC power management controller driver" + default y depends on PCISo what I think should happen is to decouple of the 'must work' features from the optional debug features in this 'driver': the Atom SoC power-off quirk should be made unconditional, as long as the .config is Atom-supported (
CPU_SUP_INTELI guess).
Quoted 8 lines
All the other bits, such as the debugfs interface, should be in a separately and appropriately named config option,
CONFIG_X86_INTEL_ATOM_PMC_DEBUG=yor so, with 'default n'.The file should probably be split up, the quirk moved into one of the generic quirk files, while
pmc_atom.cshould have the debugfs interface.
The quirk isn't really a quirk, though. Maybe a separate poweroff driver would make sense, similar to the other poweroff drivers in drivers/power/reset/.
It might also make sense to rework it as mfd client driver and tie it to the lpc_ich driver, to be loaded when the lpc_ich driver is loaded.
I don't personally see a problem with making it tristate, as long as a remove function is defined (which is not the case today). This way it would not have to be loaded for non-Atom systems.
Guenter