Re: [PATCH] Remove CONFIG_PM altogether, enable power management all the time

2011-02-08 Thread Ingo Molnar

* Tim Bird  wrote:

> On Tue, Feb 8, 2011 at 4:21 AM, Ingo Molnar  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  vmlinuxwithCONFIG_PM
| 6512652  3553116   9994240  20060008  1321768  vmlinuxwithout CONFIG_PM
|
|   41258 1904 0 43162  delta
|

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] ACPI / PM: Move references to pm_flags into sleep.c

2011-02-08 Thread Linus Torvalds
On Tue, Feb 8, 2011 at 4:37 PM, Rafael J. Wysocki  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
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] ACPI / PM: Move references to pm_flags into sleep.c

2011-02-08 Thread Rafael J. Wysocki
On Wednesday, February 09, 2011, Linus Torvalds wrote:
> On Tue, Feb 8, 2011 at 1:20 PM, Rafael J. Wysocki  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 
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 
---
 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 
 #include 
 #include 
+#include 
 
 #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);
+

Re: [PATCH 5/5] PM: Clean up Kconfig dependencies

2011-02-08 Thread Linus Torvalds
Ack on patches 2-5 in this series. It's just patch 1/5 that I think is
too ugly/odd to live.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] ACPI / PM: Move references to pm_flags into sleep.c

2011-02-08 Thread Linus Torvalds
On Tue, Feb 8, 2011 at 1:20 PM, Rafael J. Wysocki  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
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Remove CONFIG_PM altogether, enable power management all the time

2011-02-08 Thread Frank Rowand
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  vmlinuxwithCONFIG_PM
6512652  3553116   9994240  20060008  1321768  vmlinuxwithout 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

--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Remove CONFIG_PM altogether, enable power management all the time

2011-02-08 Thread Tim Bird
On Tue, Feb 8, 2011 at 4:21 AM, Ingo Molnar  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
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/5] PM: Reorder power management Kconfig options

2011-02-08 Thread Rafael J. Wysocki
From: Rafael J. Wysocki 

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 
---
 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  as well as the
-  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 100 | 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
+ p

[PATCH 4/5] PM: Replace CONFIG_PM_OPS with CONFIG_PM

2011-02-08 Thread Rafael J. Wysocki
From: Rafael J. Wysocki 

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 
---
 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_shut

[PATCH 2/5] PM: Make CONFIG_PM depend on (CONFIG_PM_SLEEP || CONFIG_PM_RUNTIME)

2011-02-08 Thread Rafael J. Wysocki
From: Rafael J. Wysocki 

>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 
---
 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  or
- Tuxmobil - Linux on Mobile Computers at 
- and the Battery Powered Linux mini-HOWTO, available from
- .
-
- 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

--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 5/5] PM: Clean up Kconfig dependencies

2011-02-08 Thread Rafael J. Wysocki
From: Rafael J. Wysocki 

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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/5] ACPI / PM: Move references to pm_flags into sleep.c

2011-02-08 Thread Rafael J. Wysocki
From: Rafael J. Wysocki 

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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/5] Re: Remove CONFIG_PM altogether, enable power management all the time

2011-02-08 Thread Rafael J. Wysocki
On Tuesday, February 08, 2011, Ingo Molnar wrote:
> 
> * Rafael J. Wysocki  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 

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
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] PM: Hide CONFIG_PM from users

2011-02-08 Thread Paul Mundt
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] PM: Hide CONFIG_PM from users

2011-02-08 Thread Pavel Machek
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
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] PM: Hide CONFIG_PM from users

2011-02-08 Thread Mark Brown
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Remove CONFIG_PM altogether, enable power management all the time

2011-02-08 Thread Ingo Molnar

* Rafael J. Wysocki  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 

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
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] PM: Hide CONFIG_PM from users

2011-02-08 Thread Mark Brown
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 
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] PM: Hide CONFIG_PM from users

2011-02-08 Thread Mark Brown
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] PM: Hide CONFIG_PM from users

2011-02-08 Thread Rafael J. Wysocki
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. :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html