It is very rare to find a current system which is both sufficiently
resource constrained to want to compile out power management support
and sufficiently power insensitive to be able to tolerate doing so.
Since having the configuration option requires non-zero effort to
maintain, with ifdefery in most drivers, but it is used with vanishing
rarity it is simpler to just remove the option.
Begin doing so by hiding it from users - this should attract complaints
from any active users. The option is left disabled for the IA64 Ski
simulator which is a partial simulator for IA64 systems mostly missing
device support. This is a very limited use case which is unlikely to
ever want to enable most drivers.
Signed-off-by: Mark Brown <[email protected]>
---
kernel/power/Kconfig | 21 ++-------------------
1 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 2657299..99e3c52 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -1,23 +1,6 @@
config PM
- bool "Power Management support"
- depends on !IA64_HP_SIM
- ---help---
- "Power Management" means that parts of your computer are shut
- off or put into a power conserving "sleep" mode if they are not
- being used. There are two competing standards for doing this: APM
- and ACPI. If you want to use either one, say Y here and then also
- to the requisite support below.
-
- Power Management is most important for battery powered laptop
- computers; if you have a laptop, check out the Linux Laptop home
- page on the WWW at <http://www.linux-on-laptops.com/> or
- Tuxmobil - Linux on Mobile Computers at <http://www.tuxmobil.org/>
- and the Battery Powered Linux mini-HOWTO, available from
- <http://www.tldp.org/docs.html#howto>.
-
- Note that, even if you say N here, Linux on the x86 architecture
- will issue the hlt instruction if nothing is to be done, thereby
- sending the processor to sleep and saving power.
+ bool
+ default y if !IA64_HP_SIM
config PM_DEBUG
bool "Power Management Debug Support"
--
1.7.2.3
On Mon, Feb 7, 2011 at 13:22, Mark Brown
<[email protected]> wrote:
> It is very rare to find a current system which is both sufficiently
> resource constrained to want to compile out power management support
> and sufficiently power insensitive to be able to tolerate doing so.
Hmmm...
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -1,23 +1,6 @@
> config PM
> - bool "Power Management support"
> - depends on !IA64_HP_SIM
> - ---help---
> - "Power Management" means that parts of your computer are shut
> - off or put into a power conserving "sleep" mode if they are not
> - being used. There are two competing standards for doing this: APM
> - and ACPI. If you want to use either one, say Y here and then also
> - to the requisite support below.
Good to see the PC-centric comments are going away :-)
> -
> - Power Management is most important for battery powered laptop
> - computers; if you have a laptop, check out the Linux Laptop home
> - page on the WWW at <http://www.linux-on-laptops.com/> or
> - Tuxmobil - Linux on Mobile Computers at <http://www.tuxmobil.org/>
> - and the Battery Powered Linux mini-HOWTO, available from
> - <http://www.tldp.org/docs.html#howto>.
> -
> - Note that, even if you say N here, Linux on the x86 architecture
> - will issue the hlt instruction if nothing is to be done, thereby
> - sending the processor to sleep and saving power.
> + bool
> + default y if !IA64_HP_SIM
The following architectures do not source "kernel/power/Kconfig":
alpha
cris
h8300
Kconfig
m32r
m68k
m68knommu
microblaze
parisc
score
sparc
tile
um
xtensa
Which means that (for now) I don't have to care that CONFIG_PM becomes
unclearable. Other people may care, though...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
* Mark Brown <[email protected]> wrote:
> It is very rare to find a current system which is both sufficiently
> resource constrained to want to compile out power management support
> and sufficiently power insensitive to be able to tolerate doing so.
> Since having the configuration option requires non-zero effort to
> maintain, with ifdefery in most drivers, but it is used with vanishing
> rarity it is simpler to just remove the option.
Well, either make it dependent on CONFIG_EXPERT or remove the option altogether.
Thanks,
Ingo
On Mon, Feb 07, 2011 at 01:48:46PM +0100, Ingo Molnar wrote:
> * Mark Brown <[email protected]> wrote:
> > Since having the configuration option requires non-zero effort to
> > maintain, with ifdefery in most drivers, but it is used with vanishing
> > rarity it is simpler to just remove the option.
> Well, either make it dependent on CONFIG_EXPERT or remove the option altogether.
The goal is the latter but when I saw the IA64 emulator there I didn't
want to make it instabuggy. The current patch means we get the UI
effect of the change (especially in terms of avoiding it turning up in
randconfigs or whatever).
On Mon, Feb 07, 2011 at 01:40:28PM +0100, Geert Uytterhoeven wrote:
> Which means that (for now) I don't have to care that CONFIG_PM becomes
> unclearable. Other people may care, though...
The whole point of the patch is that (other than the IA64 emulator which
explicitly requires that CONFIG_PM be disabled) I strongly suspect that
nobody is actually using this configuration except as a result of
randconfig type testing.
Hi Mark,
On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <[email protected]> wrote:
>
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 2657299..99e3c52 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -1,23 +1,6 @@
> config PM
> - bool "Power Management support"
> - depends on !IA64_HP_SIM
> - ---help---
> - "Power Management" means that parts of your computer are shut
> - off or put into a power conserving "sleep" mode if they are not
> - being used. There are two competing standards for doing this: APM
> - and ACPI. If you want to use either one, say Y here and then also
> - to the requisite support below.
> -
> - Power Management is most important for battery powered laptop
> - computers; if you have a laptop, check out the Linux Laptop home
> - page on the WWW at <http://www.linux-on-laptops.com/> or
> - Tuxmobil - Linux on Mobile Computers at <http://www.tuxmobil.org/>
> - and the Battery Powered Linux mini-HOWTO, available from
> - <http://www.tldp.org/docs.html#howto>.
> -
> - Note that, even if you say N here, Linux on the x86 architecture
> - will issue the hlt instruction if nothing is to be done, thereby
> - sending the processor to sleep and saving power.
> + bool
> + default y if !IA64_HP_SIM
Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
server configs), so this will unexpectedly turn it on for them.
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/
On Tue, Feb 08, 2011 at 01:13:24AM +1100, Stephen Rothwell wrote:
> On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <[email protected]> wrote:
> > + bool
> > + default y if !IA64_HP_SIM
> Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
> server configs), so this will unexpectedly turn it on for them.
Do you mean that these systems require CONFIG_PM be turned off, or just
that people tend not to turn it on? If the latter would you expect any
ill effects from doing so?
Hi Mark,
On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <[email protected]> wrote:
>
> On Tue, Feb 08, 2011 at 01:13:24AM +1100, Stephen Rothwell wrote:
> > On Mon, 7 Feb 2011 12:22:15 +0000 Mark Brown <[email protected]> wrote:
>
> > > + bool
> > > + default y if !IA64_HP_SIM
>
> > Several powerpc configs have CONFIG_PM (implicitly) disabled (e.g. the
> > server configs), so this will unexpectedly turn it on for them.
>
> Do you mean that these systems require CONFIG_PM be turned off, or just
> that people tend not to turn it on? If the latter would you expect any
> ill effects from doing so?
I don't know the answer to either question without testing. All I am
saying is that currently the default for CONFIG_PM is "off" and you are
changing it to be "on" and there may not have been any testing done of
that in some situations. We don't know where it was explicitly
turned off any more since we shrank our defconfig files (which was done
automatically) ... since it is off by default, it doesn't need to be
mentioned in a defconfig unless it needs to be turned on.
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/
On Tue, Feb 08, 2011 at 01:44:32AM +1100, Stephen Rothwell wrote:
> On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <[email protected]> wrote:
> > Do you mean that these systems require CONFIG_PM be turned off, or just
> > that people tend not to turn it on? If the latter would you expect any
> > ill effects from doing so?
> I don't know the answer to either question without testing. All I am
> saying is that currently the default for CONFIG_PM is "off" and you are
> changing it to be "on" and there may not have been any testing done of
> that in some situations. We don't know where it was explicitly
> turned off any more since we shrank our defconfig files (which was done
> automatically) ... since it is off by default, it doesn't need to be
> mentioned in a defconfig unless it needs to be turned on.
My suspicion would be that it'll have been turned off by someone hitting
return through a config upgrade rather than through deliberate effort.
On the other hand if it is essential for some machines to have it
disabled they probably want to have somethnig in Kconfig.
On Mon, Feb 7, 2011 at 15:50, Mark Brown
<[email protected]> wrote:
> On Tue, Feb 08, 2011 at 01:44:32AM +1100, Stephen Rothwell wrote:
>> On Mon, 7 Feb 2011 14:18:29 +0000 Mark Brown <[email protected]> wrote:
>
>> > Do you mean that these systems require CONFIG_PM be turned off, or just
>> > that people tend not to turn it on? If the latter would you expect any
>> > ill effects from doing so?
>
>> I don't know the answer to either question without testing. All I am
>> saying is that currently the default for CONFIG_PM is "off" and you are
>> changing it to be "on" and there may not have been any testing done of
>> that in some situations. We don't know where it was explicitly
>> turned off any more since we shrank our defconfig files (which was done
>> automatically) ... since it is off by default, it doesn't need to be
>> mentioned in a defconfig unless it needs to be turned on.
>
> My suspicion would be that it'll have been turned off by someone hitting
> return through a config upgrade rather than through deliberate effort.
> On the other hand if it is essential for some machines to have it
> disabled they probably want to have somethnig in Kconfig.
$ git grep "CONFIG_PM is not set"
7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
256
$
7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
savedefconfig, so that's a safe revision with untrimmed defconfigs.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
On Mon, 7 Feb 2011 16:00:55 +0100 Geert Uytterhoeven <[email protected]> wrote:
>
> $ git grep "CONFIG_PM is not set"
> 7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
> 256
> $
>
> 7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
> savedefconfig, so that's a safe revision with untrimmed defconfigs.
Yeah, but we can't tell if CONFIG_PM is turned off on purpose in those
defconfigs, or just off because noone explicitly turned it on.
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/
On Tue, 8 Feb 2011 02:10:45 +1100 Stephen Rothwell <[email protected]> wrote:
>
> On Mon, 7 Feb 2011 16:00:55 +0100 Geert Uytterhoeven <[email protected]> wrote:
> >
> > $ git grep "CONFIG_PM is not set"
> > 7cf3d73b4360e91b14326632ab1aeda4cb26308d^ -- arch/ | wc -l
> > 256
> > $
> >
> > 7cf3d73b4360e91b14326632ab1aeda4cb26308d is the commit that introduced
> > savedefconfig, so that's a safe revision with untrimmed defconfigs.
>
> Yeah, but we can't tell if CONFIG_PM is turned off on purpose in those
> defconfigs, or just off because noone explicitly turned it on.
At least some of the powerpc defconfigs were added with CONFIG_PM
disabled. I assume that was on purpose (though it may not have been).
--
Cheers,
Stephen Rothwell [email protected]
http://www.canb.auug.org.au/~sfr/
On Tue, Feb 08, 2011 at 02:19:16AM +1100, Stephen Rothwell wrote:
> At least some of the powerpc defconfigs were added with CONFIG_PM
> disabled. I assume that was on purpose (though it may not have been).
I'd not be so sure - since it's a bool without an explicit default set
Kconfig will default to disabling it and if anything enabling it is the
option that requires special effort.
On Mon, 7 Feb 2011, Mark Brown wrote:
> On Tue, Feb 08, 2011 at 02:19:16AM +1100, Stephen Rothwell wrote:
>
> > At least some of the powerpc defconfigs were added with CONFIG_PM
> > disabled. I assume that was on purpose (though it may not have been).
>
> I'd not be so sure - since it's a bool without an explicit default set
> Kconfig will default to disabling it and if anything enabling it is the
> option that requires special effort.
This may be a naive suggestion, but have you considered simply _asking_
the people who added those defconfigs?
Alan Stern
On Mon, Feb 07, 2011 at 10:36:31AM -0500, Alan Stern wrote:
> On Mon, 7 Feb 2011, Mark Brown wrote:
> > I'd not be so sure - since it's a bool without an explicit default set
> > Kconfig will default to disabling it and if anything enabling it is the
> > option that requires special effort.
> This may be a naive suggestion, but have you considered simply _asking_
> the people who added those defconfigs?
I'm rather hoping that they'll notice the mailing list thread or that
someone else who knows what's going on with them does - as Geert pointed
out there's a considerable number of defconfigs that have this turned
off. It seems more sensible to get some idea if this seems sane to
people in the general case before going trying to identify and contact
so many individuals.
If there are systems that really require disabling CONFIG_PM we probably
need to add stuff to Kconfig to make sure it can't be enabled anyway;
this shouldn't enable any new configurations.
On Monday, February 07, 2011, Mark Brown wrote:
> It is very rare to find a current system which is both sufficiently
> resource constrained to want to compile out power management support
> and sufficiently power insensitive to be able to tolerate doing so.
> Since having the configuration option requires non-zero effort to
> maintain, with ifdefery in most drivers, but it is used with vanishing
> rarity it is simpler to just remove the option.
>
> Begin doing so by hiding it from users - this should attract complaints
> from any active users. The option is left disabled for the IA64 Ski
> simulator which is a partial simulator for IA64 systems mostly missing
> device support. This is a very limited use case which is unlikely to
> ever want to enable most drivers.
>
> Signed-off-by: Mark Brown <[email protected]>
> ---
> kernel/power/Kconfig | 21 ++-------------------
> 1 files changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 2657299..99e3c52 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -1,23 +1,6 @@
> config PM
> - bool "Power Management support"
> - depends on !IA64_HP_SIM
> - ---help---
> - "Power Management" means that parts of your computer are shut
> - off or put into a power conserving "sleep" mode if they are not
> - being used. There are two competing standards for doing this: APM
> - and ACPI. If you want to use either one, say Y here and then also
> - to the requisite support below.
> -
> - Power Management is most important for battery powered laptop
> - computers; if you have a laptop, check out the Linux Laptop home
> - page on the WWW at <http://www.linux-on-laptops.com/> or
> - Tuxmobil - Linux on Mobile Computers at <http://www.tuxmobil.org/>
> - and the Battery Powered Linux mini-HOWTO, available from
> - <http://www.tldp.org/docs.html#howto>.
> -
> - Note that, even if you say N here, Linux on the x86 architecture
> - will issue the hlt instruction if nothing is to be done, thereby
> - sending the processor to sleep and saving power.
> + bool
> + default y if !IA64_HP_SIM
>
> config PM_DEBUG
> bool "Power Management Debug Support"
I think it would be better to simply rename CONFIG_PM_OPS into CONFIG_PM.
However, there's a number of things that I'm afraid wouldn't build correctly
if none of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME were set in that case.
Anyway, I'll try to prepare a patch doing that and see what happens.
Stay tuned.
Thanks,
Rafael
On Monday, February 07, 2011, Mark Brown wrote:
> On Mon, Feb 07, 2011 at 10:36:31AM -0500, Alan Stern wrote:
> > On Mon, 7 Feb 2011, Mark Brown wrote:
>
> > > I'd not be so sure - since it's a bool without an explicit default set
> > > Kconfig will default to disabling it and if anything enabling it is the
> > > option that requires special effort.
>
> > This may be a naive suggestion, but have you considered simply _asking_
> > the people who added those defconfigs?
>
> I'm rather hoping that they'll notice the mailing list thread or that
> someone else who knows what's going on with them does - as Geert pointed
> out there's a considerable number of defconfigs that have this turned
> off. It seems more sensible to get some idea if this seems sane to
> people in the general case before going trying to identify and contact
> so many individuals.
>
> If there are systems that really require disabling CONFIG_PM we probably
> need to add stuff to Kconfig to make sure it can't be enabled anyway;
> this shouldn't enable any new configurations.
Well, as I've just said, I don't like this change. I'd very much prefer it if
CONFIG_PM_OPS were renamed to CONFIG_PM.
Thanks,
Rafael
On Mon, Feb 07, 2011 at 08:14:03PM +0100, Rafael J. Wysocki wrote:
> On Monday, February 07, 2011, Mark Brown wrote:
> > config PM_DEBUG
> > bool "Power Management Debug Support"
> I think it would be better to simply rename CONFIG_PM_OPS into CONFIG_PM.
That still leaves the IA64 emulator to worry about but I'm not
fundamentally opposed to that, it achieves a similar effect. The main
thing I'm looking for here is to cut down on the configuration options
we have to maintain.
> However, there's a number of things that I'm afraid wouldn't build correctly
> if none of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME were set in that case.
Actually CONFIG_PM_OPS probably also wants to be on independantly of
those two sometimes for .poweroff() which I'd expect to run even if we
can't suspend.
On Monday, February 07, 2011, Mark Brown wrote:
> On Mon, Feb 07, 2011 at 08:14:03PM +0100, Rafael J. Wysocki wrote:
> > On Monday, February 07, 2011, Mark Brown wrote:
>
> > > config PM_DEBUG
> > > bool "Power Management Debug Support"
>
> > I think it would be better to simply rename CONFIG_PM_OPS into CONFIG_PM.
>
> That still leaves the IA64 emulator to worry about
Why exactly?
> but I'm not fundamentally opposed to that, it achieves a similar effect. The
> main thing I'm looking for here is to cut down on the configuration options
> we have to maintain.
But I must say you chose a particularly bad time for that from my point of view.
> > However, there's a number of things that I'm afraid wouldn't build correctly
> > if none of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME were set in that case.
>
> Actually CONFIG_PM_OPS probably also wants to be on independantly of
> those two sometimes for .poweroff() which I'd expect to run even if we
> can't suspend.
If you worry about that, then add CONFIG_PM_POWEROFF and make CONFIG_PM(_OPS)
depend on it, but I don't think it really is worth it, because people
generally don't make the poweroff code depend on CONFIG_PM.
Thanks,
Rafael
On Mon, Feb 07, 2011 at 08:46:48PM +0100, Rafael J. Wysocki wrote:
> On Monday, February 07, 2011, Mark Brown wrote:
> > On Mon, Feb 07, 2011 at 08:14:03PM +0100, Rafael J. Wysocki wrote:
> > > I think it would be better to simply rename CONFIG_PM_OPS into CONFIG_PM.
> > That still leaves the IA64 emulator to worry about
> Why exactly?
Actually not so much the IA64 emulator (which does have the !PM
dependency declared already - I expect that'd just be moved) as any
other platforms with an undeclared dependency on !PM.
> > but I'm not fundamentally opposed to that, it achieves a similar effect. The
> > main thing I'm looking for here is to cut down on the configuration options
> > we have to maintain.
> But I must say you chose a particularly bad time for that from my point of view.
This doesn't seem like it's a worse time than any other?
> > > However, there's a number of things that I'm afraid wouldn't build correctly
> > > if none of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME were set in that case.
> > Actually CONFIG_PM_OPS probably also wants to be on independantly of
> > those two sometimes for .poweroff() which I'd expect to run even if we
> > can't suspend.
> If you worry about that, then add CONFIG_PM_POWEROFF and make CONFIG_PM(_OPS)
> depend on it, but I don't think it really is worth it, because people
> generally don't make the poweroff code depend on CONFIG_PM.
Yeah, but some people seem very keen on removing the pointers to the PM
ops entirely when CONFIG_PM is disabled which means that you end up with
varying idioms for what you do with the PM ops as stuff gets ifdefed
out. Then again I'm not sure anything would make those people any
happier.
On Monday, February 07, 2011, Mark Brown wrote:
> On Mon, Feb 07, 2011 at 08:46:48PM +0100, Rafael J. Wysocki wrote:
> > On Monday, February 07, 2011, Mark Brown wrote:
> > > On Mon, Feb 07, 2011 at 08:14:03PM +0100, Rafael J. Wysocki wrote:
>
> > > > I think it would be better to simply rename CONFIG_PM_OPS into CONFIG_PM.
>
> > > That still leaves the IA64 emulator to worry about
>
> > Why exactly?
>
> Actually not so much the IA64 emulator (which does have the !PM
> dependency declared already - I expect that'd just be moved) as any
> other platforms with an undeclared dependency on !PM.
If they depend on !PM, they cannot set PM_SLEEP or PM_RUNTIME, right?
So, if PM is defined as PM_SLEEP || PM_RUNTIME, everything should be fine
in that respect I suppose.
> > > but I'm not fundamentally opposed to that, it achieves a similar effect. The
> > > main thing I'm looking for here is to cut down on the configuration options
> > > we have to maintain.
>
> > But I must say you chose a particularly bad time for that from my point of view.
>
> This doesn't seem like it's a worse time than any other?
It is, because I'm working on some other changes right now that this interferes
with quite a bit. Whatever.
> > > > However, there's a number of things that I'm afraid wouldn't build correctly
> > > > if none of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME were set in that case.
>
> > > Actually CONFIG_PM_OPS probably also wants to be on independantly of
> > > those two sometimes for .poweroff() which I'd expect to run even if we
> > > can't suspend.
>
> > If you worry about that, then add CONFIG_PM_POWEROFF and make CONFIG_PM(_OPS)
> > depend on it, but I don't think it really is worth it, because people
> > generally don't make the poweroff code depend on CONFIG_PM.
>
> Yeah, but some people seem very keen on removing the pointers to the PM
> ops entirely when CONFIG_PM is disabled which means that you end up with
> varying idioms for what you do with the PM ops as stuff gets ifdefed
> out. Then again I'm not sure anything would make those people any
> happier.
I really think we should do things that makes sense rather that worry about
who's going to like or dislike it (except for Linus maybe, but he tends to like
things that make sense anyway). At this point I think the change I suggested
makes sense, because it (a) simplifies things and (b) follows the quite common
practice which is to make PM callbacks depend on CONFIG_PM.
Anyway, below is a proof-of-concept patch. It builds for me with a few
different combinations of the relevant options, but YMMV. It probably
should be split into two or three patches for merging, but I guess a single
patch is better for testing. Please let me know if there are any problems
with it.
I'd appreciate it if people could review/test it and drop their comments.
Thanks,
Rafael
---
arch/x86/xen/Kconfig | 2 +-
drivers/acpi/Kconfig | 1 -
drivers/acpi/bus.c | 4 +---
drivers/acpi/internal.h | 6 ++++++
drivers/acpi/sleep.c | 13 +++++++++++--
drivers/base/power/Makefile | 3 +--
drivers/net/e1000e/netdev.c | 8 ++++----
drivers/net/pch_gbe/pch_gbe_main.c | 2 +-
drivers/pci/pci-driver.c | 4 ++--
drivers/scsi/Makefile | 2 +-
drivers/scsi/scsi_priv.h | 2 +-
drivers/scsi/scsi_sysfs.c | 2 +-
drivers/usb/core/hcd-pci.c | 4 ++--
include/acpi/acpi_bus.h | 2 +-
include/linux/pm.h | 2 +-
kernel/power/Kconfig | 29 +++--------------------------
16 files changed, 37 insertions(+), 49 deletions(-)
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -1,24 +1,3 @@
-config PM
- bool "Power Management support"
- depends on !IA64_HP_SIM
- ---help---
- "Power Management" means that parts of your computer are shut
- off or put into a power conserving "sleep" mode if they are not
- being used. There are two competing standards for doing this: APM
- and ACPI. If you want to use either one, say Y here and then also
- to the requisite support below.
-
- Power Management is most important for battery powered laptop
- computers; if you have a laptop, check out the Linux Laptop home
- page on the WWW at <http://www.linux-on-laptops.com/> or
- Tuxmobil - Linux on Mobile Computers at <http://www.tuxmobil.org/>
- and the Battery Powered Linux mini-HOWTO, available from
- <http://www.tldp.org/docs.html#howto>.
-
- Note that, even if you say N here, Linux on the x86 architecture
- will issue the hlt instruction if nothing is to be done, thereby
- sending the processor to sleep and saving power.
-
config PM_DEBUG
bool "Power Management Debug Support"
depends on PM
@@ -102,7 +81,7 @@ config PM_SLEEP_ADVANCED_DEBUG
config SUSPEND
bool "Suspend to RAM and standby"
- depends on PM && ARCH_SUSPEND_POSSIBLE
+ depends on ARCH_SUSPEND_POSSIBLE
default y
---help---
Allow the system to enter sleep states in which main memory is
@@ -133,7 +112,7 @@ config SUSPEND_FREEZER
config HIBERNATION
bool "Hibernation (aka 'suspend to disk')"
- depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
+ depends on SWAP && ARCH_HIBERNATION_POSSIBLE
select LZO_COMPRESS
select LZO_DECOMPRESS
---help---
@@ -224,7 +203,6 @@ config APM_EMULATION
config PM_RUNTIME
bool "Run-time PM core functionality"
- depends on PM
---help---
Enable functionality allowing I/O devices to be put into energy-saving
(low power) states at run time (or autosuspended) after a specified
@@ -236,7 +214,7 @@ config PM_RUNTIME
responsible for the actual handling of the autosuspend requests and
wake-up events.
-config PM_OPS
+config PM
bool
depends on PM_SLEEP || PM_RUNTIME
default y
@@ -246,7 +224,6 @@ config ARCH_HAS_OPP
config PM_OPP
bool "Operating Performance Point (OPP) Layer library"
- depends on PM
depends on ARCH_HAS_OPP
---help---
SOCs have a standard set of tuples consisting of frequency and
Index: linux-2.6/drivers/acpi/Kconfig
===================================================================
--- linux-2.6.orig/drivers/acpi/Kconfig
+++ linux-2.6/drivers/acpi/Kconfig
@@ -7,7 +7,6 @@ menuconfig ACPI
depends on !IA64_HP_SIM
depends on IA64 || X86
depends on PCI
- depends on PM
select PNP
default y
help
Index: linux-2.6/arch/x86/xen/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/xen/Kconfig
+++ linux-2.6/arch/x86/xen/Kconfig
@@ -38,7 +38,7 @@ config XEN_MAX_DOMAIN_MEMORY
config XEN_SAVE_RESTORE
bool
- depends on XEN && PM
+ depends on XEN
default y
config XEN_DEBUG_FS
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000e/netdev.c
+++ linux-2.6/drivers/net/e1000e/netdev.c
@@ -5307,7 +5307,7 @@ void e1000e_disable_aspm(struct pci_dev
__e1000e_disable_aspm(pdev, state);
}
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
static bool e1000e_pm_ready(struct e1000_adapter *adapter)
{
return !!adapter->tx_ring->buffer_info;
@@ -5458,7 +5458,7 @@ static int e1000_runtime_resume(struct d
return __e1000_resume(pdev);
}
#endif /* CONFIG_PM_RUNTIME */
-#endif /* CONFIG_PM_OPS */
+#endif /* CONFIG_PM */
static void e1000_shutdown(struct pci_dev *pdev)
{
@@ -6164,7 +6164,7 @@ static DEFINE_PCI_DEVICE_TABLE(e1000_pci
};
MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
static const struct dev_pm_ops e1000_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(e1000_suspend, e1000_resume)
SET_RUNTIME_PM_OPS(e1000_runtime_suspend,
@@ -6178,7 +6178,7 @@ static struct pci_driver e1000_driver =
.id_table = e1000_pci_tbl,
.probe = e1000_probe,
.remove = __devexit_p(e1000_remove),
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
.driver.pm = &e1000_pm_ops,
#endif
.shutdown = e1000_shutdown,
Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -567,7 +567,7 @@ int acpi_suspend(u32 acpi_state)
return -EINVAL;
}
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
/**
* acpi_pm_device_sleep_state - return preferred power state of ACPI device
* in the system sleep state given by %acpi_target_sleep_state
@@ -653,9 +653,18 @@ int acpi_pm_device_sleep_state(struct de
*d_min_p = d_min;
return d_max;
}
-#endif /* CONFIG_PM_OPS */
+#endif /* CONFIG_PM */
#ifdef CONFIG_PM_SLEEP
+bool acpi_set_pm_flag(void)
+{
+ if (!(pm_flags & PM_APM)) {
+ pm_flags |= PM_ACPI;
+ return true;
+ }
+ return false;
+}
+
/**
* acpi_pm_device_sleep_wake - enable or disable the system wake-up
* capability of given device
Index: linux-2.6/drivers/base/power/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/power/Makefile
+++ linux-2.6/drivers/base/power/Makefile
@@ -1,7 +1,6 @@
-obj-$(CONFIG_PM) += sysfs.o
+obj-$(CONFIG_PM) += sysfs.o generic_ops.o
obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o
obj-$(CONFIG_PM_RUNTIME) += runtime.o
-obj-$(CONFIG_PM_OPS) += generic_ops.o
obj-$(CONFIG_PM_TRACE_RTC) += trace.o
obj-$(CONFIG_PM_OPP) += opp.o
Index: linux-2.6/drivers/scsi/scsi_priv.h
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_priv.h
+++ linux-2.6/drivers/scsi/scsi_priv.h
@@ -146,7 +146,7 @@ static inline void scsi_netlink_exit(voi
#endif
/* scsi_pm.c */
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
extern const struct dev_pm_ops scsi_bus_pm_ops;
#endif
#ifdef CONFIG_PM_RUNTIME
Index: linux-2.6/drivers/scsi/Makefile
===================================================================
--- linux-2.6.orig/drivers/scsi/Makefile
+++ linux-2.6/drivers/scsi/Makefile
@@ -165,7 +165,7 @@ scsi_mod-$(CONFIG_SCSI_NETLINK) += scsi_
scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o
scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o
scsi_mod-y += scsi_trace.o
-scsi_mod-$(CONFIG_PM_OPS) += scsi_pm.o
+scsi_mod-$(CONFIG_PM) += scsi_pm.o
scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o
Index: linux-2.6/drivers/scsi/scsi_sysfs.c
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_sysfs.c
+++ linux-2.6/drivers/scsi/scsi_sysfs.c
@@ -383,7 +383,7 @@ struct bus_type scsi_bus_type = {
.name = "scsi",
.match = scsi_bus_match,
.uevent = scsi_bus_uevent,
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
.pm = &scsi_bus_pm_ops,
#endif
};
Index: linux-2.6/drivers/usb/core/hcd-pci.c
===================================================================
--- linux-2.6.orig/drivers/usb/core/hcd-pci.c
+++ linux-2.6/drivers/usb/core/hcd-pci.c
@@ -335,7 +335,7 @@ void usb_hcd_pci_shutdown(struct pci_dev
}
EXPORT_SYMBOL_GPL(usb_hcd_pci_shutdown);
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
#ifdef CONFIG_PPC_PMAC
static void powermac_set_asic(struct pci_dev *pci_dev, int enable)
@@ -580,4 +580,4 @@ const struct dev_pm_ops usb_hcd_pci_pm_o
};
EXPORT_SYMBOL_GPL(usb_hcd_pci_pm_ops);
-#endif /* CONFIG_PM_OPS */
+#endif /* CONFIG_PM */
Index: linux-2.6/drivers/pci/pci-driver.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-driver.c
+++ linux-2.6/drivers/pci/pci-driver.c
@@ -431,7 +431,7 @@ static void pci_device_shutdown(struct d
pci_msix_shutdown(pci_dev);
}
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
/* Auxiliary functions used for system resume and run-time resume. */
@@ -1059,7 +1059,7 @@ static int pci_pm_runtime_idle(struct de
#endif /* !CONFIG_PM_RUNTIME */
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
const struct dev_pm_ops pci_dev_pm_ops = {
.prepare = pci_pm_prepare,
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -380,7 +380,7 @@ struct acpi_pci_root *acpi_pci_find_root
int acpi_enable_wakeup_device_power(struct acpi_device *dev, int state);
int acpi_disable_wakeup_device_power(struct acpi_device *dev);
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
int acpi_pm_device_sleep_state(struct device *, int *);
#else
static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -267,7 +267,7 @@ const struct dev_pm_ops name = { \
* callbacks provided by device drivers supporting both the system sleep PM and
* runtime PM, make the pm member point to generic_subsys_pm_ops.
*/
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
extern struct dev_pm_ops generic_subsys_pm_ops;
#define GENERIC_SUBSYS_PM_OPS (&generic_subsys_pm_ops)
#else
Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -1025,9 +1025,7 @@ static int __init acpi_init(void)
if (!result) {
pci_mmcfg_late_init();
- if (!(pm_flags & PM_APM))
- pm_flags |= PM_ACPI;
- else {
+ if (!acpi_set_pm_flag()) {
printk(KERN_INFO PREFIX
"APM is already active, exiting\n");
disable_acpi();
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
--- linux-2.6.orig/drivers/acpi/internal.h
+++ linux-2.6/drivers/acpi/internal.h
@@ -82,12 +82,18 @@ void acpi_ec_unblock_transactions_early(
extern int acpi_sleep_init(void);
#ifdef CONFIG_ACPI_SLEEP
+/* drivers/acpi/sleep.c */
+bool acpi_set_pm_flag(void);
+
+/* drivers/acpi/nvs.c */
int acpi_sleep_proc_init(void);
int suspend_nvs_alloc(void);
void suspend_nvs_free(void);
int suspend_nvs_save(void);
void suspend_nvs_restore(void);
#else
+static inline bool acpi_set_pm_flag(void) { return true; }
+
static inline int acpi_sleep_proc_init(void) { return 0; }
static inline int suspend_nvs_alloc(void) { return 0; }
static inline void suspend_nvs_free(void) {}
Index: linux-2.6/drivers/net/pch_gbe/pch_gbe_main.c
===================================================================
--- linux-2.6.orig/drivers/net/pch_gbe/pch_gbe_main.c
+++ linux-2.6/drivers/net/pch_gbe/pch_gbe_main.c
@@ -2430,7 +2430,7 @@ static struct pci_driver pch_gbe_pcidev
.id_table = pch_gbe_pcidev_id,
.probe = pch_gbe_probe,
.remove = pch_gbe_remove,
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
.driver.pm = &pch_gbe_pm_ops,
#endif
.shutdown = pch_gbe_shutdown,
On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote:
> On Monday, February 07, 2011, Mark Brown wrote:
> >
> > Yeah, but some people seem very keen on removing the pointers to the PM
> > ops entirely when CONFIG_PM is disabled which means that you end up with
> > varying idioms for what you do with the PM ops as stuff gets ifdefed
> > out. Then again I'm not sure anything would make those people any
> > happier.
>
> I really think we should do things that makes sense rather that worry about
> who's going to like or dislike it (except for Linus maybe, but he tends to like
> things that make sense anyway). At this point I think the change I suggested
> makes sense, because it (a) simplifies things and (b) follows the quite common
> practice which is to make PM callbacks depend on CONFIG_PM.
Many people make these callback dependent on PM not because it makes
much sense but because it is possible to do so. However, aside of
randconfig compile testing, nobody really tests drivers that implement
PM in the !CONFIG_PM setting.
--
Dmitry
On Monday, February 07, 2011, Dmitry Torokhov wrote:
> On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote:
> > On Monday, February 07, 2011, Mark Brown wrote:
> > >
> > > Yeah, but some people seem very keen on removing the pointers to the PM
> > > ops entirely when CONFIG_PM is disabled which means that you end up with
> > > varying idioms for what you do with the PM ops as stuff gets ifdefed
> > > out. Then again I'm not sure anything would make those people any
> > > happier.
> >
> > I really think we should do things that makes sense rather that worry about
> > who's going to like or dislike it (except for Linus maybe, but he tends to like
> > things that make sense anyway). At this point I think the change I suggested
> > makes sense, because it (a) simplifies things and (b) follows the quite common
> > practice which is to make PM callbacks depend on CONFIG_PM.
>
> Many people make these callback dependent on PM not because it makes
> much sense but because it is possible to do so. However, aside of
> randconfig compile testing, nobody really tests drivers that implement
> PM in the !CONFIG_PM setting.
That I can agree with, but I'm not sure whether it is an argument against
the patch I've just posted or for it?
Rafael
On Mon, Feb 07, 2011 at 11:00:03PM +0100, Rafael J. Wysocki wrote:
> On Monday, February 07, 2011, Dmitry Torokhov wrote:
> > On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote:
> > > On Monday, February 07, 2011, Mark Brown wrote:
> > > >
> > > > Yeah, but some people seem very keen on removing the pointers to the PM
> > > > ops entirely when CONFIG_PM is disabled which means that you end up with
> > > > varying idioms for what you do with the PM ops as stuff gets ifdefed
> > > > out. Then again I'm not sure anything would make those people any
> > > > happier.
> > >
> > > I really think we should do things that makes sense rather that worry about
> > > who's going to like or dislike it (except for Linus maybe, but he tends to like
> > > things that make sense anyway). At this point I think the change I suggested
> > > makes sense, because it (a) simplifies things and (b) follows the quite common
> > > practice which is to make PM callbacks depend on CONFIG_PM.
> >
> > Many people make these callback dependent on PM not because it makes
> > much sense but because it is possible to do so. However, aside of
> > randconfig compile testing, nobody really tests drivers that implement
> > PM in the !CONFIG_PM setting.
>
> That I can agree with, but I'm not sure whether it is an argument against
> the patch I've just posted or for it?
More of an observation for your (b) justification. I'd probably force
CONFIG_PM to always 'y'w while we weeding references to it from
drivers...
--
Dmitry
On Monday, February 07, 2011, Dmitry Torokhov wrote:
> On Mon, Feb 07, 2011 at 11:00:03PM +0100, Rafael J. Wysocki wrote:
> > On Monday, February 07, 2011, Dmitry Torokhov wrote:
> > > On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote:
> > > > On Monday, February 07, 2011, Mark Brown wrote:
> > > > >
> > > > > Yeah, but some people seem very keen on removing the pointers to the PM
> > > > > ops entirely when CONFIG_PM is disabled which means that you end up with
> > > > > varying idioms for what you do with the PM ops as stuff gets ifdefed
> > > > > out. Then again I'm not sure anything would make those people any
> > > > > happier.
> > > >
> > > > I really think we should do things that makes sense rather that worry about
> > > > who's going to like or dislike it (except for Linus maybe, but he tends to like
> > > > things that make sense anyway). At this point I think the change I suggested
> > > > makes sense, because it (a) simplifies things and (b) follows the quite common
> > > > practice which is to make PM callbacks depend on CONFIG_PM.
> > >
> > > Many people make these callback dependent on PM not because it makes
> > > much sense but because it is possible to do so. However, aside of
> > > randconfig compile testing, nobody really tests drivers that implement
> > > PM in the !CONFIG_PM setting.
> >
> > That I can agree with, but I'm not sure whether it is an argument against
> > the patch I've just posted or for it?
>
> More of an observation for your (b) justification. I'd probably force
> CONFIG_PM to always 'y'w while we weeding references to it from
> drivers...
We simply can't force CONFIG_PM to 'y', because some platforms want it to be 'n'.
OTOH, if CONFIG_PM = CONFIG_PM_SLEEP||CONFIG_PM_RUNTIME, we can just leave the
#ifdefs as they are and simply avoid adding new ones, or use CONFIG_PM for all
PM callbacks.
Thanks,
Rafael
On Tue, Feb 08, 2011 at 12:05:40AM +0100, Rafael J. Wysocki wrote:
> On Monday, February 07, 2011, Dmitry Torokhov wrote:
> > On Mon, Feb 07, 2011 at 11:00:03PM +0100, Rafael J. Wysocki wrote:
> > > On Monday, February 07, 2011, Dmitry Torokhov wrote:
> > > > On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote:
> > > > > On Monday, February 07, 2011, Mark Brown wrote:
> > > > > >
> > > > > > Yeah, but some people seem very keen on removing the pointers to the PM
> > > > > > ops entirely when CONFIG_PM is disabled which means that you end up with
> > > > > > varying idioms for what you do with the PM ops as stuff gets ifdefed
> > > > > > out. Then again I'm not sure anything would make those people any
> > > > > > happier.
> > > > >
> > > > > I really think we should do things that makes sense rather that worry about
> > > > > who's going to like or dislike it (except for Linus maybe, but he tends to like
> > > > > things that make sense anyway). At this point I think the change I suggested
> > > > > makes sense, because it (a) simplifies things and (b) follows the quite common
> > > > > practice which is to make PM callbacks depend on CONFIG_PM.
> > > >
> > > > Many people make these callback dependent on PM not because it makes
> > > > much sense but because it is possible to do so. However, aside of
> > > > randconfig compile testing, nobody really tests drivers that implement
> > > > PM in the !CONFIG_PM setting.
> > >
> > > That I can agree with, but I'm not sure whether it is an argument against
> > > the patch I've just posted or for it?
> >
> > More of an observation for your (b) justification. I'd probably force
> > CONFIG_PM to always 'y'w while we weeding references to it from
> > drivers...
>
> We simply can't force CONFIG_PM to 'y', because some platforms want it to be 'n'.
>
Again, want or need? It would be nice to know answer to this question.
Thanks.
--
Dmitry
On Mon, Feb 7, 2011 at 7:49 AM, Mark Brown
<[email protected]> wrote:
> I'm rather hoping that they'll notice the mailing list thread or that
> someone else who knows what's going on with them does
Surely you're joking. I mean, do _you_ scan every message that comes
through lkml and its various sister lists?
Do a git blame and add them to the CC:. It's the polite thing to do.
On 02/07/11 04:22, Mark Brown wrote:
> It is very rare to find a current system which is both sufficiently
> resource constrained to want to compile out power management support
> and sufficiently power insensitive to be able to tolerate doing so.
> Since having the configuration option requires non-zero effort to
> maintain, with ifdefery in most drivers, but it is used with vanishing
> rarity it is simpler to just remove the option.
Proof by assertion that it is used with vanishing rarity.
> Begin doing so by hiding it from users - this should attract complaints
> from any active users. The option is left disabled for the IA64 Ski
> simulator which is a partial simulator for IA64 systems mostly missing
> device support. This is a very limited use case which is unlikely to
> ever want to enable most drivers.
That is not a good method of getting feedback from users.
1) It immediately removes the ability to have CONFIG_PM undefined,
without first giving active users a chance to provide feedback.
2) The removal of that ability is not obvious ("make oldconfig" does
not say anything about CONFIG_PM). It is easy to overlook a
config change that happens silently.
3) The active users may not move to a newer version of the kernel
that contains this change until after it has been decided that
there are no users of the config option since no one complained
in a timely manner.
Would it be appropriate to use Documentation/feature-removal-schedule.txt
if this truly will be removed?
-Frank
On Tuesday, February 08, 2011, Dmitry Torokhov wrote:
> On Tue, Feb 08, 2011 at 12:05:40AM +0100, Rafael J. Wysocki wrote:
> > On Monday, February 07, 2011, Dmitry Torokhov wrote:
> > > On Mon, Feb 07, 2011 at 11:00:03PM +0100, Rafael J. Wysocki wrote:
> > > > On Monday, February 07, 2011, Dmitry Torokhov wrote:
> > > > > On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote:
> > > > > > On Monday, February 07, 2011, Mark Brown wrote:
> > > > > > >
> > > > > > > Yeah, but some people seem very keen on removing the pointers to the PM
> > > > > > > ops entirely when CONFIG_PM is disabled which means that you end up with
> > > > > > > varying idioms for what you do with the PM ops as stuff gets ifdefed
> > > > > > > out. Then again I'm not sure anything would make those people any
> > > > > > > happier.
> > > > > >
> > > > > > I really think we should do things that makes sense rather that worry about
> > > > > > who's going to like or dislike it (except for Linus maybe, but he tends to like
> > > > > > things that make sense anyway). At this point I think the change I suggested
> > > > > > makes sense, because it (a) simplifies things and (b) follows the quite common
> > > > > > practice which is to make PM callbacks depend on CONFIG_PM.
> > > > >
> > > > > Many people make these callback dependent on PM not because it makes
> > > > > much sense but because it is possible to do so. However, aside of
> > > > > randconfig compile testing, nobody really tests drivers that implement
> > > > > PM in the !CONFIG_PM setting.
> > > >
> > > > That I can agree with, but I'm not sure whether it is an argument against
> > > > the patch I've just posted or for it?
> > >
> > > More of an observation for your (b) justification. I'd probably force
> > > CONFIG_PM to always 'y'w while we weeding references to it from
> > > drivers...
> >
> > We simply can't force CONFIG_PM to 'y', because some platforms want it to be 'n'.
> >
>
> Again, want or need? It would be nice to know answer to this question.
I _think_ the answer is "want", which doesn't change a lot, because I'm not
going to convince people to stop wanting things. :-)
On Mon, Feb 07, 2011 at 05:17:59PM -0800, Ray Lee wrote:
> On Mon, Feb 7, 2011 at 7:49 AM, Mark Brown
> > I'm rather hoping that they'll notice the mailing list thread or that
> > someone else who knows what's going on with them does
> Surely you're joking. I mean, do _you_ scan every message that comes
> through lkml and its various sister lists?
Actually I do at least scan most of the lists.
> Do a git blame and add them to the CC:. It's the polite thing to do.
It's also going to result in the mail not going to the mailing lists as
there's a limit on the number of people you can CC enforced by vger
which probably isn't constructive. It's moot now but as I said in the
text you've helpfully cut I'd have suggested contacting them after the
thread had come to a conclusion.
On Mon, Feb 07, 2011 at 10:15:59PM +0100, Rafael J. Wysocki wrote:
> I really think we should do things that makes sense rather that worry about
> who's going to like or dislike it (except for Linus maybe, but he tends to like
> things that make sense anyway). At this point I think the change I suggested
> makes sense, because it (a) simplifies things and (b) follows the quite common
> practice which is to make PM callbacks depend on CONFIG_PM.
So, part of the issue here is that it's not clear if having the ifdefs
in the drivers is really something that makes sense. It's not at all
clear that anyone is actually making active use of them on real systems
rather than just doing build coverage testing, it really feels like the
ifdefs that people are currently using are just being cargo culted
around.
The reason it's come up for me is that dev_pm_ops changes things a bit
as you've got to decide how to handle the struct itself and there's no
clear decision on what the best way forward for that is (is it OK to
leave the struct if it's empty?) so you end up with the approach you
need to take varying between maintainers which is annoying.
> I'd appreciate it if people could review/test it and drop their comments.
Looks good to me:
Reviewed/Tested-by: Mark Brown <[email protected]>
* Rafael J. Wysocki <[email protected]> wrote:
> I'd appreciate it if people could review/test it and drop their comments.
>
> Thanks,
> Rafael
>
> ---
> arch/x86/xen/Kconfig | 2 +-
> drivers/acpi/Kconfig | 1 -
> drivers/acpi/bus.c | 4 +---
> drivers/acpi/internal.h | 6 ++++++
> drivers/acpi/sleep.c | 13 +++++++++++--
> drivers/base/power/Makefile | 3 +--
> drivers/net/e1000e/netdev.c | 8 ++++----
> drivers/net/pch_gbe/pch_gbe_main.c | 2 +-
> drivers/pci/pci-driver.c | 4 ++--
> drivers/scsi/Makefile | 2 +-
> drivers/scsi/scsi_priv.h | 2 +-
> drivers/scsi/scsi_sysfs.c | 2 +-
> drivers/usb/core/hcd-pci.c | 4 ++--
> include/acpi/acpi_bus.h | 2 +-
> include/linux/pm.h | 2 +-
> kernel/power/Kconfig | 29 +++--------------------------
> 16 files changed, 37 insertions(+), 49 deletions(-)
Ok, there's some real bang for bucks in this patch, nice! It's a beginning.
Reviewed-by: Ingo Molnar <[email protected]>
Also, i've Cc:-ed Linus, to check whether the idea to make power management a
permanent, core portion of Linux has any obvious downsides we missed.
Rafael, could you do a defconfig-ish x86 build with and without CONFIG_PM, and post
the 'size vmlinux' comparison - so that we can see the size difference? We make some
things CONFIG_EXPERT configurable just to enable folks who *really* want to cut down
on kernel size to configure it out.
Note that those usecases, even if they want a super-small kernel, might not care
about PM at all while they care about size: small boot kernels in ROMs, or simple
devices where CPU-idling implies deep low power mode, etc.
So the vmlinux size comparisons would be needed really. If it's 5k nobody will care.
If it's 50k-100k that's borderline. In the other side of the scale we have the 1500+
#ifdef CONFIG_PM lines strewn around the kernel source, and the frequent !PM build
breakages.
Ingo
On Mon, Feb 07, 2011 at 06:52:00PM -0800, Frank Rowand wrote:
> On 02/07/11 04:22, Mark Brown wrote:
> > Since having the configuration option requires non-zero effort to
> > maintain, with ifdefery in most drivers, but it is used with vanishing
> > rarity it is simpler to just remove the option.
> Proof by assertion that it is used with vanishing rarity.
Sure, hopefully if it's incorrect people will come out of the woodwork
to correct me :)
> That is not a good method of getting feedback from users.
> 1) It immediately removes the ability to have CONFIG_PM undefined,
> without first giving active users a chance to provide feedback.
Note that it's not a terribly difficult change to reverse; if someone
urgently does need to do so then I'd be surprised if they were able to
build a kernel but not cope with that change.
> 2) The removal of that ability is not obvious ("make oldconfig" does
> not say anything about CONFIG_PM). It is easy to overlook a
> config change that happens silently.
It will expose the sub-options which actually do stuff, though - it's
only the top level option for PM.
> Would it be appropriate to use Documentation/feature-removal-schedule.txt
> if this truly will be removed?
I guess, though I'm a bit pessimistic about anyone actually noticing.
With Raphael's version it's not such a big deal as CONFIG_PM is selected
by other options that previously depended on it instead of being enabled
all the time.
Hi!
> It is very rare to find a current system which is both sufficiently
> resource constrained to want to compile out power management support
> and sufficiently power insensitive to be able to tolerate doing so.
Ok, how much memory do we talk about here?
Lots of embedded systems are AC powered or powered from some powerful
source -- such as alternator in car. PM can be unwanted complexity
there.
Ot maybe they are running under realtime hypervisor...
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Tue, Feb 08, 2011 at 12:05:40AM +0100, Rafael J. Wysocki wrote:
> On Monday, February 07, 2011, Dmitry Torokhov wrote:
> > More of an observation for your (b) justification. I'd probably force
> > CONFIG_PM to always 'y'w while we weeding references to it from
> > drivers...
>
> We simply can't force CONFIG_PM to 'y', because some platforms want it to be 'n'.
>
> OTOH, if CONFIG_PM = CONFIG_PM_SLEEP||CONFIG_PM_RUNTIME, we can just leave the
> #ifdefs as they are and simply avoid adding new ones, or use CONFIG_PM for all
> PM callbacks.
>
For sh at least turning on CONFIG_PM without PM_SLEEP or PM_RUNTIME is
largely pointless, so the bulk of the defconfigs have it turned off. The
few platforms that do use CONFIG_PM for something also have more
comprehensive support implemented. The few times we do have it enabled
without one of the others supported is simply for build coverage, mostly
due to sharing drivers (whatever isn't already triggered through
rand/allyes/modconfigs).
I would expect that this is a common scenario for the bulk of the
defconfigs that reflect PM being a platform-specific vs architecture-wide
property regardless of architecture, however.
From: Rafael J. Wysocki <[email protected]>
>From the users' point of view CONFIG_PM is really only used for
making it possible to set CONFIG_SUSPEND, CONFIG_HIBERNATION,
CONFIG_PM_RUNTIME and (surprisingly enough) CONFIG_XEN_SAVE_RESTORE
(CONFIG_PM_OPP also depends on CONFIG_PM, but quite artificially).
However, both CONFIG_SUSPEND and CONFIG_HIBERNATION require platform
support (independent of CONFIG_PM) and it is not quite obvious that
CONFIG_PM has to be set for CONFIG_XEN_SAVE_RESTORE to be available.
Thus, from the users' point of view, it would be more logical to
automatically select CONFIG_PM if any of the above options depending
on it are set.
Make CONFIG_PM depend on (CONFIG_PM_SLEEP || CONFIG_PM_RUNTIME),
which will cause it to be selected when any of CONFIG_SUSPEND,
CONFIG_HIBERNATION, CONFIG_PM_RUNTIME, CONFIG_XEN_SAVE_RESTORE is
set and will clarify its meaning.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
arch/x86/xen/Kconfig | 2 +-
kernel/power/Kconfig | 29 ++++++-----------------------
2 files changed, 7 insertions(+), 24 deletions(-)
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -1,23 +1,7 @@
config PM
- bool "Power Management support"
- depends on !IA64_HP_SIM
- ---help---
- "Power Management" means that parts of your computer are shut
- off or put into a power conserving "sleep" mode if they are not
- being used. There are two competing standards for doing this: APM
- and ACPI. If you want to use either one, say Y here and then also
- to the requisite support below.
-
- Power Management is most important for battery powered laptop
- computers; if you have a laptop, check out the Linux Laptop home
- page on the WWW at <http://www.linux-on-laptops.com/> or
- Tuxmobil - Linux on Mobile Computers at <http://www.tuxmobil.org/>
- and the Battery Powered Linux mini-HOWTO, available from
- <http://www.tldp.org/docs.html#howto>.
-
- Note that, even if you say N here, Linux on the x86 architecture
- will issue the hlt instruction if nothing is to be done, thereby
- sending the processor to sleep and saving power.
+ bool
+ depends on PM_SLEEP || PM_RUNTIME
+ default y
config PM_DEBUG
bool "Power Management Debug Support"
@@ -102,7 +86,7 @@ config PM_SLEEP_ADVANCED_DEBUG
config SUSPEND
bool "Suspend to RAM and standby"
- depends on PM && ARCH_SUSPEND_POSSIBLE
+ depends on ARCH_SUSPEND_POSSIBLE
default y
---help---
Allow the system to enter sleep states in which main memory is
@@ -133,7 +117,7 @@ config SUSPEND_FREEZER
config HIBERNATION
bool "Hibernation (aka 'suspend to disk')"
- depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
+ depends on SWAP && ARCH_HIBERNATION_POSSIBLE
select LZO_COMPRESS
select LZO_DECOMPRESS
---help---
@@ -224,7 +208,7 @@ config APM_EMULATION
config PM_RUNTIME
bool "Run-time PM core functionality"
- depends on PM
+ depends on !IA64_HP_SIM
---help---
Enable functionality allowing I/O devices to be put into energy-saving
(low power) states at run time (or autosuspended) after a specified
@@ -246,7 +230,6 @@ config ARCH_HAS_OPP
config PM_OPP
bool "Operating Performance Point (OPP) Layer library"
- depends on PM
depends on ARCH_HAS_OPP
---help---
SOCs have a standard set of tuples consisting of frequency and
Index: linux-2.6/arch/x86/xen/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/xen/Kconfig
+++ linux-2.6/arch/x86/xen/Kconfig
@@ -38,7 +38,7 @@ config XEN_MAX_DOMAIN_MEMORY
config XEN_SAVE_RESTORE
bool
- depends on XEN && PM
+ depends on XEN
default y
config XEN_DEBUG_FS
On Tuesday, February 08, 2011, Ingo Molnar wrote:
>
> * Rafael J. Wysocki <[email protected]> wrote:
>
> > I'd appreciate it if people could review/test it and drop their comments.
> >
> > Thanks,
> > Rafael
> >
> > ---
> > arch/x86/xen/Kconfig | 2 +-
> > drivers/acpi/Kconfig | 1 -
> > drivers/acpi/bus.c | 4 +---
> > drivers/acpi/internal.h | 6 ++++++
> > drivers/acpi/sleep.c | 13 +++++++++++--
> > drivers/base/power/Makefile | 3 +--
> > drivers/net/e1000e/netdev.c | 8 ++++----
> > drivers/net/pch_gbe/pch_gbe_main.c | 2 +-
> > drivers/pci/pci-driver.c | 4 ++--
> > drivers/scsi/Makefile | 2 +-
> > drivers/scsi/scsi_priv.h | 2 +-
> > drivers/scsi/scsi_sysfs.c | 2 +-
> > drivers/usb/core/hcd-pci.c | 4 ++--
> > include/acpi/acpi_bus.h | 2 +-
> > include/linux/pm.h | 2 +-
> > kernel/power/Kconfig | 29 +++--------------------------
> > 16 files changed, 37 insertions(+), 49 deletions(-)
>
> Ok, there's some real bang for bucks in this patch, nice! It's a beginning.
>
> Reviewed-by: Ingo Molnar <[email protected]>
In the meantime I've split it into a series of patches that should make it a
bit easier to diagnose problems, if there are any. I'll post those patches
in replies to this message:
[1/5] - Deal with dependencies on CONFIG_PM in ACPI
[2/5] - Redefine CONFIG_PM as (CONFIG_PM_SLEEP || CONFIG_PM_RUNTIME)
[3/5] - Reorder options in kernel/power/Kconfig
[4/5] - Replace CONFIG_PM_OPS with CONFIG_PM
[5/5] - Clean up dependencies in kernel/power/Kconfig
> Also, i've Cc:-ed Linus, to check whether the idea to make power management a
> permanent, core portion of Linux has any obvious downsides we missed.
>
> Rafael, could you do a defconfig-ish x86 build with and without CONFIG_PM, and post
> the 'size vmlinux' comparison - so that we can see the size difference? We make some
> things CONFIG_EXPERT configurable just to enable folks who *really* want to cut down
> on kernel size to configure it out.
Sure, I will.
Thanks,
Rafael
From: Rafael J. Wysocki <[email protected]>
If direct references to pm_flags are moved from bus.c to sleep.c,
CONFIG_ACPI will not need to depend on CONFIG_PM any more.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/Kconfig | 1 -
drivers/acpi/bus.c | 4 +---
drivers/acpi/internal.h | 6 ++++++
drivers/acpi/sleep.c | 12 ++++++++++++
4 files changed, 19 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -1025,9 +1025,7 @@ static int __init acpi_init(void)
if (!result) {
pci_mmcfg_late_init();
- if (!(pm_flags & PM_APM))
- pm_flags |= PM_ACPI;
- else {
+ if (!acpi_pm_enabled()) {
printk(KERN_INFO PREFIX
"APM is already active, exiting\n");
disable_acpi();
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
--- linux-2.6.orig/drivers/acpi/internal.h
+++ linux-2.6/drivers/acpi/internal.h
@@ -82,12 +82,18 @@ void acpi_ec_unblock_transactions_early(
extern int acpi_sleep_init(void);
#ifdef CONFIG_ACPI_SLEEP
+/* drivers/acpi/sleep.c */
+bool acpi_pm_enabled(void);
+
+/* drivers/acpi/nvs.c */
int acpi_sleep_proc_init(void);
int suspend_nvs_alloc(void);
void suspend_nvs_free(void);
int suspend_nvs_save(void);
void suspend_nvs_restore(void);
#else
+static inline bool acpi_pm_enabled(void) { return true; }
+
static inline int acpi_sleep_proc_init(void) { return 0; }
static inline int suspend_nvs_alloc(void) { return 0; }
static inline void suspend_nvs_free(void) {}
Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -657,6 +657,18 @@ int acpi_pm_device_sleep_state(struct de
#ifdef CONFIG_PM_SLEEP
/**
+ *
+ */
+bool acpi_pm_enabled(void)
+{
+ if (!(pm_flags & PM_APM)) {
+ pm_flags |= PM_ACPI;
+ return true;
+ }
+ return false;
+}
+
+/**
* acpi_pm_device_sleep_wake - enable or disable the system wake-up
* capability of given device
* @dev: device to handle
Index: linux-2.6/drivers/acpi/Kconfig
===================================================================
--- linux-2.6.orig/drivers/acpi/Kconfig
+++ linux-2.6/drivers/acpi/Kconfig
@@ -7,7 +7,6 @@ menuconfig ACPI
depends on !IA64_HP_SIM
depends on IA64 || X86
depends on PCI
- depends on PM
select PNP
default y
help
From: Rafael J. Wysocki <[email protected]>
After redefining CONFIG_PM to depend on (CONFIG_PM_SLEEP ||
CONFIG_PM_RUNTIME) the CONFIG_PM_OPS option is redundant and can be
replaced with CONFIG_PM.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/sleep.c | 4 ++--
drivers/base/power/Makefile | 3 +--
drivers/net/e1000e/netdev.c | 8 ++++----
drivers/net/pch_gbe/pch_gbe_main.c | 2 +-
drivers/pci/pci-driver.c | 4 ++--
drivers/scsi/Makefile | 2 +-
drivers/scsi/scsi_priv.h | 2 +-
drivers/scsi/scsi_sysfs.c | 2 +-
drivers/usb/core/hcd-pci.c | 4 ++--
include/acpi/acpi_bus.h | 2 +-
include/linux/pm.h | 2 +-
kernel/power/Kconfig | 5 -----
12 files changed, 17 insertions(+), 23 deletions(-)
Index: linux-2.6/drivers/net/e1000e/netdev.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000e/netdev.c
+++ linux-2.6/drivers/net/e1000e/netdev.c
@@ -5307,7 +5307,7 @@ void e1000e_disable_aspm(struct pci_dev
__e1000e_disable_aspm(pdev, state);
}
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
static bool e1000e_pm_ready(struct e1000_adapter *adapter)
{
return !!adapter->tx_ring->buffer_info;
@@ -5458,7 +5458,7 @@ static int e1000_runtime_resume(struct d
return __e1000_resume(pdev);
}
#endif /* CONFIG_PM_RUNTIME */
-#endif /* CONFIG_PM_OPS */
+#endif /* CONFIG_PM */
static void e1000_shutdown(struct pci_dev *pdev)
{
@@ -6164,7 +6164,7 @@ static DEFINE_PCI_DEVICE_TABLE(e1000_pci
};
MODULE_DEVICE_TABLE(pci, e1000_pci_tbl);
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
static const struct dev_pm_ops e1000_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(e1000_suspend, e1000_resume)
SET_RUNTIME_PM_OPS(e1000_runtime_suspend,
@@ -6178,7 +6178,7 @@ static struct pci_driver e1000_driver =
.id_table = e1000_pci_tbl,
.probe = e1000_probe,
.remove = __devexit_p(e1000_remove),
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
.driver.pm = &e1000_pm_ops,
#endif
.shutdown = e1000_shutdown,
Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -567,7 +567,7 @@ int acpi_suspend(u32 acpi_state)
return -EINVAL;
}
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
/**
* acpi_pm_device_sleep_state - return preferred power state of ACPI device
* in the system sleep state given by %acpi_target_sleep_state
@@ -653,7 +653,7 @@ int acpi_pm_device_sleep_state(struct de
*d_min_p = d_min;
return d_max;
}
-#endif /* CONFIG_PM_OPS */
+#endif /* CONFIG_PM */
#ifdef CONFIG_PM_SLEEP
/**
Index: linux-2.6/drivers/base/power/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/power/Makefile
+++ linux-2.6/drivers/base/power/Makefile
@@ -1,7 +1,6 @@
-obj-$(CONFIG_PM) += sysfs.o
+obj-$(CONFIG_PM) += sysfs.o generic_ops.o
obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o
obj-$(CONFIG_PM_RUNTIME) += runtime.o
-obj-$(CONFIG_PM_OPS) += generic_ops.o
obj-$(CONFIG_PM_TRACE_RTC) += trace.o
obj-$(CONFIG_PM_OPP) += opp.o
Index: linux-2.6/drivers/scsi/scsi_priv.h
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_priv.h
+++ linux-2.6/drivers/scsi/scsi_priv.h
@@ -146,7 +146,7 @@ static inline void scsi_netlink_exit(voi
#endif
/* scsi_pm.c */
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
extern const struct dev_pm_ops scsi_bus_pm_ops;
#endif
#ifdef CONFIG_PM_RUNTIME
Index: linux-2.6/drivers/scsi/Makefile
===================================================================
--- linux-2.6.orig/drivers/scsi/Makefile
+++ linux-2.6/drivers/scsi/Makefile
@@ -165,7 +165,7 @@ scsi_mod-$(CONFIG_SCSI_NETLINK) += scsi_
scsi_mod-$(CONFIG_SYSCTL) += scsi_sysctl.o
scsi_mod-$(CONFIG_SCSI_PROC_FS) += scsi_proc.o
scsi_mod-y += scsi_trace.o
-scsi_mod-$(CONFIG_PM_OPS) += scsi_pm.o
+scsi_mod-$(CONFIG_PM) += scsi_pm.o
scsi_tgt-y += scsi_tgt_lib.o scsi_tgt_if.o
Index: linux-2.6/drivers/scsi/scsi_sysfs.c
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_sysfs.c
+++ linux-2.6/drivers/scsi/scsi_sysfs.c
@@ -383,7 +383,7 @@ struct bus_type scsi_bus_type = {
.name = "scsi",
.match = scsi_bus_match,
.uevent = scsi_bus_uevent,
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
.pm = &scsi_bus_pm_ops,
#endif
};
Index: linux-2.6/drivers/usb/core/hcd-pci.c
===================================================================
--- linux-2.6.orig/drivers/usb/core/hcd-pci.c
+++ linux-2.6/drivers/usb/core/hcd-pci.c
@@ -335,7 +335,7 @@ void usb_hcd_pci_shutdown(struct pci_dev
}
EXPORT_SYMBOL_GPL(usb_hcd_pci_shutdown);
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
#ifdef CONFIG_PPC_PMAC
static void powermac_set_asic(struct pci_dev *pci_dev, int enable)
@@ -580,4 +580,4 @@ const struct dev_pm_ops usb_hcd_pci_pm_o
};
EXPORT_SYMBOL_GPL(usb_hcd_pci_pm_ops);
-#endif /* CONFIG_PM_OPS */
+#endif /* CONFIG_PM */
Index: linux-2.6/drivers/pci/pci-driver.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-driver.c
+++ linux-2.6/drivers/pci/pci-driver.c
@@ -431,7 +431,7 @@ static void pci_device_shutdown(struct d
pci_msix_shutdown(pci_dev);
}
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
/* Auxiliary functions used for system resume and run-time resume. */
@@ -1059,7 +1059,7 @@ static int pci_pm_runtime_idle(struct de
#endif /* !CONFIG_PM_RUNTIME */
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
const struct dev_pm_ops pci_dev_pm_ops = {
.prepare = pci_pm_prepare,
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -380,7 +380,7 @@ struct acpi_pci_root *acpi_pci_find_root
int acpi_enable_wakeup_device_power(struct acpi_device *dev, int state);
int acpi_disable_wakeup_device_power(struct acpi_device *dev);
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
int acpi_pm_device_sleep_state(struct device *, int *);
#else
static inline int acpi_pm_device_sleep_state(struct device *d, int *p)
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -267,7 +267,7 @@ const struct dev_pm_ops name = { \
* callbacks provided by device drivers supporting both the system sleep PM and
* runtime PM, make the pm member point to generic_subsys_pm_ops.
*/
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
extern struct dev_pm_ops generic_subsys_pm_ops;
#define GENERIC_SUBSYS_PM_OPS (&generic_subsys_pm_ops)
#else
Index: linux-2.6/drivers/net/pch_gbe/pch_gbe_main.c
===================================================================
--- linux-2.6.orig/drivers/net/pch_gbe/pch_gbe_main.c
+++ linux-2.6/drivers/net/pch_gbe/pch_gbe_main.c
@@ -2430,7 +2430,7 @@ static struct pci_driver pch_gbe_pcidev
.id_table = pch_gbe_pcidev_id,
.probe = pch_gbe_probe,
.remove = pch_gbe_remove,
-#ifdef CONFIG_PM_OPS
+#ifdef CONFIG_PM
.driver.pm = &pch_gbe_pm_ops,
#endif
.shutdown = pch_gbe_shutdown,
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -220,11 +220,6 @@ config APM_EMULATION
anything, try disabling/enabling this option (or disabling/enabling
APM in your BIOS).
-config PM_OPS
- bool
- depends on PM_SLEEP || PM_RUNTIME
- default y
-
config ARCH_HAS_OPP
bool
From: Rafael J. Wysocki <[email protected]>
CONFIG_PM_SLEEP_ADVANCED_DEBUG should depend on CONFIG_PM_SLEEP
and CONFIG_CAN_PM_TRACE need not depend on EXPERIMENTAL. Modify
kernel/power/Kconfig along those lines.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -142,7 +142,7 @@ config PM_ADVANCED_DEBUG
config PM_SLEEP_ADVANCED_DEBUG
bool
- depends on PM_ADVANCED_DEBUG
+ depends on PM_ADVANCED_DEBUG && PM_SLEEP
default n
config PM_TEST_SUSPEND
@@ -158,7 +158,7 @@ config PM_TEST_SUSPEND
config CAN_PM_TRACE
def_bool y
- depends on PM_DEBUG && PM_SLEEP && EXPERIMENTAL
+ depends on PM_DEBUG && PM_SLEEP
config PM_TRACE
bool
From: Rafael J. Wysocki <[email protected]>
Reorder configuration options in kernel/power/Kconfig so that
the options depended on are at the top of the list.
This patch doesn't introduce any functional changes.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/Kconfig | 222 +++++++++++++++++++++++++--------------------------
1 file changed, 111 insertions(+), 111 deletions(-)
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -1,89 +1,3 @@
-config PM
- bool
- depends on PM_SLEEP || PM_RUNTIME
- default y
-
-config PM_DEBUG
- bool "Power Management Debug Support"
- depends on PM
- ---help---
- This option enables various debugging support in the Power Management
- code. This is helpful when debugging and reporting PM bugs, like
- suspend support.
-
-config PM_ADVANCED_DEBUG
- bool "Extra PM attributes in sysfs for low-level debugging/testing"
- depends on PM_DEBUG
- default n
- ---help---
- Add extra sysfs attributes allowing one to access some Power Management
- fields of device objects from user space. If you are not a kernel
- developer interested in debugging/testing Power Management, say "no".
-
-config PM_VERBOSE
- bool "Verbose Power Management debugging"
- depends on PM_DEBUG
- default n
- ---help---
- This option enables verbose messages from the Power Management code.
-
-config CAN_PM_TRACE
- def_bool y
- depends on PM_DEBUG && PM_SLEEP && EXPERIMENTAL
-
-config PM_TRACE
- bool
- help
- This enables code to save the last PM event point across
- reboot. The architecture needs to support this, x86 for
- example does by saving things in the RTC, see below.
-
- The architecture specific code must provide the extern
- functions from <linux/resume-trace.h> as well as the
- <asm/resume-trace.h> header with a TRACE_RESUME() macro.
-
- The way the information is presented is architecture-
- dependent, x86 will print the information during a
- late_initcall.
-
-config PM_TRACE_RTC
- bool "Suspend/resume event tracing"
- depends on CAN_PM_TRACE
- depends on X86
- select PM_TRACE
- default n
- ---help---
- This enables some cheesy code to save the last PM event point in the
- RTC across reboots, so that you can debug a machine that just hangs
- during suspend (or more commonly, during resume).
-
- To use this debugging feature you should attempt to suspend the
- machine, reboot it and then run
-
- dmesg -s 1000000 | grep 'hash matches'
-
- CAUTION: this option will cause your machine's real-time clock to be
- set to an invalid time after a resume.
-
-config PM_SLEEP_SMP
- bool
- depends on SMP
- depends on ARCH_SUSPEND_POSSIBLE || ARCH_HIBERNATION_POSSIBLE
- depends on PM_SLEEP
- select HOTPLUG
- select HOTPLUG_CPU
- default y
-
-config PM_SLEEP
- bool
- depends on SUSPEND || HIBERNATION || XEN_SAVE_RESTORE
- default y
-
-config PM_SLEEP_ADVANCED_DEBUG
- bool
- depends on PM_ADVANCED_DEBUG
- default n
-
config SUSPEND
bool "Suspend to RAM and standby"
depends on ARCH_SUSPEND_POSSIBLE
@@ -93,17 +7,6 @@ config SUSPEND
powered and thus its contents are preserved, such as the
suspend-to-RAM state (e.g. the ACPI S3 state).
-config PM_TEST_SUSPEND
- bool "Test suspend/resume and wakealarm during bootup"
- depends on SUSPEND && PM_DEBUG && RTC_CLASS=y
- ---help---
- This option will let you suspend your machine during bootup, and
- make it wake up a few seconds later using an RTC wakeup alarm.
- Enable this with a kernel parameter like "test_suspend=mem".
-
- You probably want to have your system's RTC driver statically
- linked, ensuring that it's available when this test runs.
-
config SUSPEND_FREEZER
bool "Enable freezer for suspend to RAM/standby" \
if ARCH_WANTS_FREEZER_CONTROL || BROKEN
@@ -180,6 +83,117 @@ config PM_STD_PARTITION
suspended image to. It will simply pick the first available swap
device.
+config PM_SLEEP
+ bool
+ depends on SUSPEND || HIBERNATION || XEN_SAVE_RESTORE
+ default y
+
+config PM_SLEEP_SMP
+ bool
+ depends on SMP
+ depends on ARCH_SUSPEND_POSSIBLE || ARCH_HIBERNATION_POSSIBLE
+ depends on PM_SLEEP
+ select HOTPLUG
+ select HOTPLUG_CPU
+ default y
+
+config PM_RUNTIME
+ bool "Run-time PM core functionality"
+ depends on !IA64_HP_SIM
+ ---help---
+ Enable functionality allowing I/O devices to be put into energy-saving
+ (low power) states at run time (or autosuspended) after a specified
+ period of inactivity and woken up in response to a hardware-generated
+ wake-up event or a driver's request.
+
+ Hardware support is generally required for this functionality to work
+ and the bus type drivers of the buses the devices are on are
+ responsible for the actual handling of the autosuspend requests and
+ wake-up events.
+
+config PM
+ bool
+ depends on PM_SLEEP || PM_RUNTIME
+ default y
+
+config PM_DEBUG
+ bool "Power Management Debug Support"
+ depends on PM
+ ---help---
+ This option enables various debugging support in the Power Management
+ code. This is helpful when debugging and reporting PM bugs, like
+ suspend support.
+
+config PM_VERBOSE
+ bool "Verbose Power Management debugging"
+ depends on PM_DEBUG
+ default n
+ ---help---
+ This option enables verbose messages from the Power Management code.
+
+config PM_ADVANCED_DEBUG
+ bool "Extra PM attributes in sysfs for low-level debugging/testing"
+ depends on PM_DEBUG
+ default n
+ ---help---
+ Add extra sysfs attributes allowing one to access some Power Management
+ fields of device objects from user space. If you are not a kernel
+ developer interested in debugging/testing Power Management, say "no".
+
+config PM_SLEEP_ADVANCED_DEBUG
+ bool
+ depends on PM_ADVANCED_DEBUG
+ default n
+
+config PM_TEST_SUSPEND
+ bool "Test suspend/resume and wakealarm during bootup"
+ depends on SUSPEND && PM_DEBUG && RTC_CLASS=y
+ ---help---
+ This option will let you suspend your machine during bootup, and
+ make it wake up a few seconds later using an RTC wakeup alarm.
+ Enable this with a kernel parameter like "test_suspend=mem".
+
+ You probably want to have your system's RTC driver statically
+ linked, ensuring that it's available when this test runs.
+
+config CAN_PM_TRACE
+ def_bool y
+ depends on PM_DEBUG && PM_SLEEP && EXPERIMENTAL
+
+config PM_TRACE
+ bool
+ help
+ This enables code to save the last PM event point across
+ reboot. The architecture needs to support this, x86 for
+ example does by saving things in the RTC, see below.
+
+ The architecture specific code must provide the extern
+ functions from <linux/resume-trace.h> as well as the
+ <asm/resume-trace.h> header with a TRACE_RESUME() macro.
+
+ The way the information is presented is architecture-
+ dependent, x86 will print the information during a
+ late_initcall.
+
+config PM_TRACE_RTC
+ bool "Suspend/resume event tracing"
+ depends on CAN_PM_TRACE
+ depends on X86
+ select PM_TRACE
+ default n
+ ---help---
+ This enables some cheesy code to save the last PM event point in the
+ RTC across reboots, so that you can debug a machine that just hangs
+ during suspend (or more commonly, during resume).
+
+ To use this debugging feature you should attempt to suspend the
+ machine, reboot it and then run
+
+ dmesg -s 1000000 | grep 'hash matches'
+
+ CAUTION: this option will cause your machine's real-time clock to be
+ set to an invalid time after a resume.
+
config APM_EMULATION
tristate "Advanced Power Management Emulation"
depends on PM && SYS_SUPPORTS_APM_EMULATION
@@ -206,20 +220,6 @@ config APM_EMULATION
anything, try disabling/enabling this option (or disabling/enabling
APM in your BIOS).
-config PM_RUNTIME
- bool "Run-time PM core functionality"
- depends on !IA64_HP_SIM
- ---help---
- Enable functionality allowing I/O devices to be put into energy-saving
- (low power) states at run time (or autosuspended) after a specified
- period of inactivity and woken up in response to a hardware-generated
- wake-up event or a driver's request.
-
- Hardware support is generally required for this functionality to work
- and the bus type drivers of the buses the devices are on are
- responsible for the actual handling of the autosuspend requests and
- wake-up events.
-
config PM_OPS
bool
depends on PM_SLEEP || PM_RUNTIME
On Tue, Feb 8, 2011 at 4:21 AM, Ingo Molnar <[email protected]> wrote:
> Rafael, could you do a defconfig-ish x86 build with and without CONFIG_PM, and post
> the 'size vmlinux' comparison - so that we can see the size difference? We make some
> things CONFIG_EXPERT configurable just to enable folks who *really* want to cut down
> on kernel size to configure it out.
I'm one of those people who *really* wants to cut down the kernel size.
I've recently worked on a product where the kernel RAM budget is ~1M.
>
> Note that those usecases, even if they want a super-small kernel, might not care
> about PM at all while they care about size: small boot kernels in ROMs, or simple
> devices where CPU-idling implies deep low power mode, etc.
>
> So the vmlinux size comparisons would be needed really. If it's 5k nobody will care.
I care about 5K. (But honestly, I don't actively hunt stuff less than
10K in size, because
there's too many of them to chase, currently).
-- Tim
On 02/08/11 04:21, Ingo Molnar wrote:
< snip >
> Also, i've Cc:-ed Linus, to check whether the idea to make power management a
> permanent, core portion of Linux has any obvious downsides we missed.
>
> Rafael, could you do a defconfig-ish x86 build with and without CONFIG_PM, and post
> the 'size vmlinux' comparison - so that we can see the size difference? We make some
> things CONFIG_EXPERT configurable just to enable folks who *really* want to cut down
> on kernel size to configure it out.
For 2.6.38-rc4, x86_64, CONFIG_NR_CPUS=4:
size vmlinux
text data bss dec hex filename
6553910 3555020 9994240 20103170 132c002 vmlinux with CONFIG_PM
6512652 3553116 9994240 20060008 1321768 vmlinux without CONFIG_PM
41258 1904 0 43162 delta
That is big enough for me to care.
Turning on CONFIG_PM also forces a few other options on:
295a296
> CONFIG_XEN_SAVE_RESTORE=y
422c423,431
< # CONFIG_PM is not set
---
> CONFIG_PM=y
> # CONFIG_PM_DEBUG is not set
> CONFIG_PM_SLEEP_SMP=y
> CONFIG_PM_SLEEP=y
> # CONFIG_SUSPEND is not set
> # CONFIG_HIBERNATION is not set
> # CONFIG_PM_RUNTIME is not set
> CONFIG_PM_OPS=y
> # CONFIG_ACPI is not set
451,454c460
< CONFIG_CPU_IDLE=y
< CONFIG_CPU_IDLE_GOV_LADDER=y
< CONFIG_CPU_IDLE_GOV_MENU=y
< # CONFIG_INTEL_IDLE is not set
---
> # CONFIG_CPU_IDLE is not set
>
> Note that those usecases, even if they want a super-small kernel, might not care
> about PM at all while they care about size: small boot kernels in ROMs, or simple
> devices where CPU-idling implies deep low power mode, etc.
>
> So the vmlinux size comparisons would be needed really. If it's 5k nobody will care.
> If it's 50k-100k that's borderline. In the other side of the scale we have the 1500+
> #ifdef CONFIG_PM lines strewn around the kernel source, and the frequent !PM build
> breakages.
>
> Ingo
-Frank
On Tue, Feb 8, 2011 at 1:20 PM, Rafael J. Wysocki <[email protected]> wrote:
>
> If direct references to pm_flags are moved from bus.c to sleep.c,
> CONFIG_ACPI will not need to depend on CONFIG_PM any more.
The patch may _work_, but I really hate it. That function naming is insane:
> ?#ifdef CONFIG_ACPI_SLEEP
> ?#else
> +static inline bool acpi_pm_enabled(void) { return true; }
acpi_pm_enabled() returns true if ACPI_SLEEP is _not_ enabled? That's
just crazy.
... followed by more crazy:
> +bool acpi_pm_enabled(void)
> +{
> + ? ? ? if (!(pm_flags & PM_APM)) {
> + ? ? ? ? ? ? ? pm_flags |= PM_ACPI;
> + ? ? ? ? ? ? ? return true;
> + ? ? ? }
> + ? ? ? return false;
> +}
IOW, that function doesn't do anything _remotely_ like what the naming
indicates.
Any sane person would expect that a function called
'acpi_pm_enabled()' would return true if ACPI PM was enabled, and
false otherwise. But it's not what it does at all. Instead, what it
does is to say "if APM isn't enabled, let's enable ACPI and return
true". Except then for the non-ACPI sleep case, we just return true
regardless, which still looks damn odd, wouldn't you say?
That isn't good. Maybe it all does what you want it to do, but from a
code readability standpoint, it's just one honking big "WTF?". Please
don't do that. Call it something else. Make the naming actually follow
what the semantics are. Appropriate naming should also make it
sensible to return "true" when ACPI_SLEEP is disabled, and should make
the caller look sane.
Now, I don't know what that particular naming might be, but maybe it
would be about APM being enabled. Which is what the caller actually
seems to care about and talks about for the failure case. Maybe you
need separate functions for the "is APM enabled" case for the naming
to make sense. Hmm?
Linus
Ack on patches 2-5 in this series. It's just patch 1/5 that I think is
too ugly/odd to live.
Linus
On Wednesday, February 09, 2011, Linus Torvalds wrote:
> On Tue, Feb 8, 2011 at 1:20 PM, Rafael J. Wysocki <[email protected]> wrote:
> >
> > If direct references to pm_flags are moved from bus.c to sleep.c,
> > CONFIG_ACPI will not need to depend on CONFIG_PM any more.
>
> The patch may _work_, but I really hate it. That function naming is insane:
>
> > #ifdef CONFIG_ACPI_SLEEP
> > #else
> > +static inline bool acpi_pm_enabled(void) { return true; }
>
> acpi_pm_enabled() returns true if ACPI_SLEEP is _not_ enabled? That's
> just crazy.
>
> ... followed by more crazy:
>
> > +bool acpi_pm_enabled(void)
> > +{
> > + if (!(pm_flags & PM_APM)) {
> > + pm_flags |= PM_ACPI;
> > + return true;
> > + }
> > + return false;
> > +}
>
> IOW, that function doesn't do anything _remotely_ like what the naming
> indicates.
>
> Any sane person would expect that a function called
> 'acpi_pm_enabled()' would return true if ACPI PM was enabled, and
> false otherwise. But it's not what it does at all. Instead, what it
> does is to say "if APM isn't enabled, let's enable ACPI and return
> true". Except then for the non-ACPI sleep case, we just return true
> regardless, which still looks damn odd, wouldn't you say?
>
> That isn't good. Maybe it all does what you want it to do, but from a
> code readability standpoint, it's just one honking big "WTF?". Please
> don't do that. Call it something else. Make the naming actually follow
> what the semantics are. Appropriate naming should also make it
> sensible to return "true" when ACPI_SLEEP is disabled, and should make
> the caller look sane.
>
> Now, I don't know what that particular naming might be,
I had the same problem and I must admit I'm not good at inventing function
names.
> but maybe it would be about APM being enabled. Which is what the caller
> actually seems to care about and talks about for the failure case. Maybe
> you need separate functions for the "is APM enabled" case for the naming
> to make sense. Hmm?
That sounds like a good idea. What about the following patch?
---
From: Rafael J. Wysocki <[email protected]>
Subject: PM / ACPI: Remove references to pm_flags from bus.c
If direct references to pm_flags are removed from drivers/acpi/bus.c,
CONFIG_ACPI will not need to depend on CONFIG_PM any more. Make that
happen.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/Kconfig | 1 -
drivers/acpi/bus.c | 7 ++++---
include/linux/suspend.h | 6 ++++++
kernel/power/main.c | 12 +++++++++++-
4 files changed, 21 insertions(+), 5 deletions(-)
Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -40,6 +40,7 @@
#include <acpi/acpi_bus.h>
#include <acpi/acpi_drivers.h>
#include <linux/dmi.h>
+#include <linux/suspend.h>
#include "internal.h"
@@ -1025,13 +1026,13 @@ static int __init acpi_init(void)
if (!result) {
pci_mmcfg_late_init();
- if (!(pm_flags & PM_APM))
- pm_flags |= PM_ACPI;
- else {
+ if (pm_apm_enabled()) {
printk(KERN_INFO PREFIX
"APM is already active, exiting\n");
disable_acpi();
result = -ENODEV;
+ } else {
+ pm_set_acpi_flag();
}
} else
disable_acpi();
Index: linux-2.6/drivers/acpi/Kconfig
===================================================================
--- linux-2.6.orig/drivers/acpi/Kconfig
+++ linux-2.6/drivers/acpi/Kconfig
@@ -7,7 +7,6 @@ menuconfig ACPI
depends on !IA64_HP_SIM
depends on IA64 || X86
depends on PCI
- depends on PM
select PNP
default y
help
Index: linux-2.6/include/linux/suspend.h
===================================================================
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -272,6 +272,9 @@ extern int unregister_pm_notifier(struct
register_pm_notifier(&fn##_nb); \
}
+extern bool pm_apm_enabled(void);
+extern void pm_set_acpi_flag(void);
+
/* drivers/base/power/wakeup.c */
extern bool events_check_enabled;
@@ -292,6 +295,9 @@ static inline int unregister_pm_notifier
#define pm_notifier(fn, pri) do { (void)(fn); } while (0)
+static inline bool pm_apm_enabled(void) { return false; }
+static inline void pm_set_acpi_flag(void) {}
+
static inline bool pm_wakeup_pending(void) { return false; }
#endif /* !CONFIG_PM_SLEEP */
Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -17,10 +17,20 @@
DEFINE_MUTEX(pm_mutex);
+#ifdef CONFIG_PM_SLEEP
+
unsigned int pm_flags;
EXPORT_SYMBOL(pm_flags);
-#ifdef CONFIG_PM_SLEEP
+bool pm_apm_enabled(void)
+{
+ return !!(pm_flags & PM_APM);
+}
+
+void pm_set_acpi_flag(void)
+{
+ pm_flags |= PM_ACPI;
+}
/* Routines for PM-transition notifications */
On Tue, Feb 8, 2011 at 4:37 PM, Rafael J. Wysocki <[email protected]> wrote:
>
>> but maybe it would be about APM being enabled. Which is what the caller
>> actually seems to care about and talks about for the failure case. Maybe
>> you need separate functions for the "is APM enabled" case for the naming
>> to make sense. Hmm?
>
> That sounds like a good idea. ?What about the following patch?
This patch I have no problems with.
Linus
* Tim Bird <[email protected]> wrote:
> On Tue, Feb 8, 2011 at 4:21 AM, Ingo Molnar <[email protected]> wrote:
>
> > Rafael, could you do a defconfig-ish x86 build with and without CONFIG_PM, and
> > post the 'size vmlinux' comparison - so that we can see the size difference? We
> > make some things CONFIG_EXPERT configurable just to enable folks who *really*
> > want to cut down on kernel size to configure it out.
>
> I'm one of those people who *really* wants to cut down the kernel size. I've
> recently worked on a product where the kernel RAM budget is ~1M.
Did that kernel have CONFIG_PM disabled?
> > Note that those usecases, even if they want a super-small kernel, might not care
> > about PM at all while they care about size: small boot kernels in ROMs, or
> > simple devices where CPU-idling implies deep low power mode, etc.
>
> So the vmlinux size comparisons would be needed really. If it's 5k nobody will
> care. I care about 5K. (But honestly, I don't actively hunt stuff less than 10K
> in size, because there's too many of them to chase, currently).
The numbers that Frank Rowand sent show 40K+:
|
| For 2.6.38-rc4, x86_64, CONFIG_NR_CPUS=4:
|
| size vmlinux
| text data bss dec hex filename
|
| 6553910 3555020 9994240 20103170 132c002 vmlinux with CONFIG_PM
| 6512652 3553116 9994240 20060008 1321768 vmlinux without CONFIG_PM
|
| 41258 1904 0 43162 delta
|
Thanks,
Ingo
On Tue, Feb 08, 2011 at 03:35:29PM -0800, Frank Rowand wrote:
> For 2.6.38-rc4, x86_64, CONFIG_NR_CPUS=4:
> size vmlinux
> text data bss dec hex filename
>
> 6553910 3555020 9994240 20103170 132c002 vmlinux with CONFIG_PM
> 6512652 3553116 9994240 20060008 1321768 vmlinux without CONFIG_PM
>
> 41258 1904 0 43162 delta
> That is big enough for me to care.
Hrm, that's pretty surprising. It'd be interesting to know how much of
that is due to the PM core itself and how much of that is from drivers.
For the drivers CONFIG_PM isn't really the option they should be using
in the first place - they mostly want some combination of PM_SLEEP and
PM_RUNTIME for the specific functionality. I'm running some checks now.
> > CONFIG_PM_SLEEP=y
Raphael's patch will make this a user visible option in place of raw
CONFIG_PM by default so you'd be able to turn that off.
On Wed, Feb 09, 2011 at 11:41:37AM +0000, Mark Brown wrote:
> Hrm, that's pretty surprising. It'd be interesting to know how much of
> that is due to the PM core itself and how much of that is from drivers.
> For the drivers CONFIG_PM isn't really the option they should be using
> in the first place - they mostly want some combination of PM_SLEEP and
> PM_RUNTIME for the specific functionality. I'm running some checks now.
OK, on ARM with slightly more than an allnoconfig (allnoconfig itself
wouldn't build) I see:
text data bss dec hex filename
1361476 71360 167320 1600156 186a9c vmlinux.nopm
1364228 71520 167480 1603228 18769c vmlinux
2752 160 160 3072
so 3K from CONFIG_PM there, mostly from the ARM VIC management code
which I suspect really wants to be under one of the more specific PM
options, though obviously this comes back to the issue with bitrotted
ifdefs for PM whcih pervades the kernel.
On Wednesday, February 09, 2011, Mark Brown wrote:
> On Tue, Feb 08, 2011 at 03:35:29PM -0800, Frank Rowand wrote:
>
> > For 2.6.38-rc4, x86_64, CONFIG_NR_CPUS=4:
>
> > size vmlinux
> > text data bss dec hex filename
> >
> > 6553910 3555020 9994240 20103170 132c002 vmlinux with CONFIG_PM
> > 6512652 3553116 9994240 20060008 1321768 vmlinux without CONFIG_PM
> >
> > 41258 1904 0 43162 delta
>
> > That is big enough for me to care.
>
> Hrm, that's pretty surprising. It'd be interesting to know how much of
> that is due to the PM core itself and how much of that is from drivers.
> For the drivers CONFIG_PM isn't really the option they should be using
> in the first place - they mostly want some combination of PM_SLEEP and
> PM_RUNTIME for the specific functionality. I'm running some checks now.
>
> > > CONFIG_PM_SLEEP=y
>
> Raphael's patch will make this a user visible option in place of raw
> CONFIG_PM by default so you'd be able to turn that off.
No, it won't (just to clarify).
Thanks,
Rafael
On 02/09/11 09:07, Rafael J. Wysocki wrote:
> On Wednesday, February 09, 2011, Mark Brown wrote:
>> On Tue, Feb 08, 2011 at 03:35:29PM -0800, Frank Rowand wrote:
>>
>>> For 2.6.38-rc4, x86_64, CONFIG_NR_CPUS=4:
>>
>>> size vmlinux
>>> text data bss dec hex filename
>>>
>>> 6553910 3555020 9994240 20103170 132c002 vmlinux with CONFIG_PM
>>> 6512652 3553116 9994240 20060008 1321768 vmlinux without CONFIG_PM
>>>
>>> 41258 1904 0 43162 delta
>>
>>> That is big enough for me to care.
>>
>> Hrm, that's pretty surprising. It'd be interesting to know how much of
>> that is due to the PM core itself and how much of that is from drivers.
>> For the drivers CONFIG_PM isn't really the option they should be using
>> in the first place - they mostly want some combination of PM_SLEEP and
>> PM_RUNTIME for the specific functionality. I'm running some checks now.
>>
>>> > CONFIG_PM_SLEEP=y
>>
>> Raphael's patch will make this a user visible option in place of raw
>> CONFIG_PM by default so you'd be able to turn that off.
>
> No, it won't (just to clarify).
Raphael's patch will turn on CONFIG_PM in the correct circumstances, and
leave it off when not needed by other config options. That means that
the size overhead will _not_ be an issue for me because CONFIG_PM
will not be enabled when not needed.
-Frank
On Wed, Feb 09, 2011 at 10:31:29AM -0800, Frank Rowand wrote:
> Raphael's patch will turn on CONFIG_PM in the correct circumstances, and
> leave it off when not needed by other config options. That means that
> the size overhead will _not_ be an issue for me because CONFIG_PM
> will not be enabled when not needed.
That's not the issue you seemed to be raising, though. While PM is now
turned on by PM_SLEEP that'll end up getting turned on by default due to
the dependency on SUSPEND - you appeared to be raising the concern that
this could happen and surprise users.
On 02/09/11 10:40, Mark Brown wrote:
> On Wed, Feb 09, 2011 at 10:31:29AM -0800, Frank Rowand wrote:
>
>> Raphael's patch will turn on CONFIG_PM in the correct circumstances, and
>> leave it off when not needed by other config options. That means that
>> the size overhead will _not_ be an issue for me because CONFIG_PM
>> will not be enabled when not needed.
>
> That's not the issue you seemed to be raising, though. While PM is now
> turned on by PM_SLEEP that'll end up getting turned on by default due to
> the dependency on SUSPEND - you appeared to be raising the concern that
> this could happen and surprise users.
No, that is not my concern. I was saying that Raphael's patches do
not trigger any concern from me.
My concern was that in your very first email that started this thread,
you wrote:
On 02/07/11 04:22, Mark Brown wrote:
> It is very rare to find a current system which is both sufficiently
> resource constrained to want to compile out power management support
> and sufficiently power insensitive to be able to tolerate doing so.
> Since having the configuration option requires non-zero effort to
> maintain, with ifdefery in most drivers, but it is used with vanishing
> rarity it is simpler to just remove the option.
and my understanding of this proposal was a goal to remove the ability
to have CONFIG_PM disabled, which results in increased memory usage
for some configurations.
-Frank
On Wed, Feb 09, 2011 at 11:00:17AM -0800, Frank Rowand wrote:
> and my understanding of this proposal was a goal to remove the ability
> to have CONFIG_PM disabled, which results in increased memory usage
> for some configurations.
Not really, the goal was to simplify the PM config options to ones that
are actually useful and cut down on the number of silly combinations
that the randconfigs turn up. CONFIG_PM is there mostly for historical
reasons, it doesn't really mean much by itself except as a gate to other
options.
This comes from the fact that the options exposed and the use of them
doesn't really reflect the kernel power management infrastructure any
more. The PM options at Kconfig level are really SUSPEND, HIBERNATION
and PM_RUNTIME but for historical reasons the idoms are all based on the
root PM option. This gets fiddly and confusing as you move over build
coverage.
On 02/09/2011 11:25 AM, Mark Brown wrote:
> On Wed, Feb 09, 2011 at 11:00:17AM -0800, Frank Rowand wrote:
>
>> and my understanding of this proposal was a goal to remove the ability
>> to have CONFIG_PM disabled, which results in increased memory usage
>> for some configurations.
>
> Not really, the goal was to simplify the PM config options to ones that
> are actually useful and cut down on the number of silly combinations
> that the randconfigs turn up. CONFIG_PM is there mostly for historical
> reasons, it doesn't really mean much by itself except as a gate to other
> options.
I'm confused. Do you plan to retain the option to
turn off PM features completely, or not? I thought that's
what CONFIG_PM did today.
-- Tim
=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Network Entertainment
=============================
On Wed, Feb 09, 2011 at 11:53:52AM -0800, Tim Bird wrote:
> On 02/09/2011 11:25 AM, Mark Brown wrote:
> > Not really, the goal was to simplify the PM config options to ones that
> > are actually useful and cut down on the number of silly combinations
> > that the randconfigs turn up. CONFIG_PM is there mostly for historical
> > reasons, it doesn't really mean much by itself except as a gate to other
> > options.
> I'm confused. Do you plan to retain the option to
> turn off PM features completely, or not? I thought that's
> what CONFIG_PM did today.
Raphael's patches do that in a much better way than my original patch,
my original patch would have force CONFIG_PM on but still allowed all
the PM features that it controls to be turned on and off individually.
On Wed, 9 Feb 2011, Mark Brown wrote:
> On Wed, Feb 09, 2011 at 11:53:52AM -0800, Tim Bird wrote:
> > I'm confused. Do you plan to retain the option to
> > turn off PM features completely, or not? I thought that's
> > what CONFIG_PM did today.
>
> Raphael's patches do that in a much better way than my original patch,
> my original patch would have force CONFIG_PM on but still allowed all
> the PM features that it controls to be turned on and off individually.
Or to put it another way, if you enable any of CONFIG_PM_SUSPEND,
CONFIG_PM_HIBERNATION, or CONFIG_PM_RUNTIME, then CONFIG_PM will
automatically be turned on. Otherwise it will automatically be turned
off.
Alan Stern
On Wed, Feb 09, 2011 at 03:09:42PM -0500, Alan Stern wrote:
> On Wed, 9 Feb 2011, Mark Brown wrote:
> > Raphael's patches do that in a much better way than my original patch,
> > my original patch would have force CONFIG_PM on but still allowed all
> > the PM features that it controls to be turned on and off individually.
> Or to put it another way, if you enable any of CONFIG_PM_SUSPEND,
> CONFIG_PM_HIBERNATION, or CONFIG_PM_RUNTIME, then CONFIG_PM will
> automatically be turned on. Otherwise it will automatically be turned
> off.
Plus everything controlled by the old CONFIG_PM_OPS is now controlled
directly by CONFIG_PM.
On Tuesday, February 08, 2011, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> CONFIG_PM_SLEEP_ADVANCED_DEBUG should depend on CONFIG_PM_SLEEP
> and CONFIG_CAN_PM_TRACE need not depend on EXPERIMENTAL. Modify
> kernel/power/Kconfig along those lines.
It turns out that CONFIG_PM_SLEEP_ADVANCED_DEBUG is not used any more, so we
can simply get rid of it.
Thanks,
Rafael
---
From: Rafael J. Wysocki <[email protected]>
Subject: PM: Clean up PM_TRACE dependencies and drop unnecessary Kconfig option
CONFIG_PM_SLEEP_ADVANCED_DEBUG is not used any more, so drop it
and CONFIG_CAN_PM_TRACE need not depend on EXPERIMENTAL, so remove
that dependency.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/Kconfig | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -140,11 +140,6 @@ config PM_ADVANCED_DEBUG
fields of device objects from user space. If you are not a kernel
developer interested in debugging/testing Power Management, say "no".
-config PM_SLEEP_ADVANCED_DEBUG
- bool
- depends on PM_ADVANCED_DEBUG
- default n
-
config PM_TEST_SUSPEND
bool "Test suspend/resume and wakealarm during bootup"
depends on SUSPEND && PM_DEBUG && RTC_CLASS=y
@@ -158,7 +153,7 @@ config PM_TEST_SUSPEND
config CAN_PM_TRACE
def_bool y
- depends on PM_DEBUG && PM_SLEEP && EXPERIMENTAL
+ depends on PM_DEBUG && PM_SLEEP
config PM_TRACE
bool