On Fri, Jul 27, 2007 at 03:57:39PM -0700, Linus Torvalds wrote:
> 
> 
> On Sat, 28 Jul 2007, Adrian Bunk wrote:
> > 
> > The dependency of SUSPEND_SMP on HOTPLUG_CPU is quite unintuitive, so 
> > what about something like the patch below?
> 
> Yeah, this looks reasonable.
> 
> May I suggest another level of indirection, though:
> 
> > +config SUSPEND_SMP_POSSIBLE
> > +   bool
> > +   depends on (X86 && !X86_VOYAGER) || (PPC64 && (PPC_PSERIES || PPC_PMAC))
> > +   depends on SMP
> > +   default y
> 
> How about making this a bit more split up, and do it as
> 
>       # SMP suspend is possible on ..
>       config SUSPEND_SMP_POSSIBLE
>               bool
>               depends on (X86 && !X86_VOYAGER) || (PPC64 && (PPC_PSERIES || 
> PPC_PMAC))
>               default y
> 
>       # UP suspend is possible on ..
>       config SUSPEND_UP_POSSIBLE
>               bool
>               depends on X86 || PPC64_SWSUSP || FRV || PPC32
>               default y 

Sounds good.

>       # Can we suspend?
>       config SUSPEND_POSSIBLE
>               bool
>               depends on (SMP && SUSPEND_SMP_POSSIBLE) || 
> (SUSPEND_UP_POSSIBLE && !SMP)
>               default y

IMHO not required:

config SOFTWARE_SUSPEND
        bool "Software Suspend (Hibernation)"
        depends on PM && SWAP
        depends on SUSPEND_UP_POSSIBLE || SUSPEND_SMP_POSSIBLE

> and then we have just a
> 
>       config SOFTWARE_SUSPEND
>               bool "Software Suspend (Hibernation)"
>               depends on PM && SWAP
>               depends on SUSPEND_POSSIBLE
> 
>       config SUSPEND_SMP
>               bool
>               depends on SOFTWARE_SUSPEND && SMP
>               select HOTPLUG_CPU
>               default y
> 
> and now each of the config options looks pretty simple and describe one 
> thing.
> 
> [ For extra bonus points: the SUSPEND_POSSIBLE thing is still pretty 
>   complicated, and it might actually be a better idea to make it a 
>   per-arch config option, and just make the x86/arch say
> 
>       config SUSPEND_POSSIBLE
>               bool
>               depends on !(X86_VOYAGER && SMP)
>               default y

This would give you "trying to assign nonexistent symbol SUSPEND_POSSIBLE"
kconfig warnings on architectures without SUSPEND_POSSIBLE.

(And you missed the UP case in your example.)

>   instead: since SUSPEND_POSSIBLE is always true on x86 regardless of SMP 
>   or not, just not on X86_VOYAGER. Then, each architecture can have its 
>   own private rules for whether that architecture has SUSPEND_POSSIBLE or 
>   not, so on ppc, it might look like
> 
>       config SUSPEND_POSSIBLE
>               bool
>               depends on (PPC64 && (PPC_PSERIES || PPC_PMAC)) || PPC_SWSUSP
>               bool y
> 
>   or something, but the point is, now the complexity is a per-architecture 
>   thing, so other architectures simply don't have to care any more! ]
> 
> And the user only ever sees one single question: the one for 
> "SOFTWARE_SUSPEND". All the others would directly flow either from the 
> architecture choice, or from that.
> 
> Anybody willing to rewrite it that way?

Patch below.

>                       Linus

cu
Adrian


<--  snip  -->


An implementation detail of the suspend code that is not intuitive for 
the user is the HOTPLUG_CPU dependency of SOFTWARE_SUSPEND if SMP.

This patch handles the dependency of SOFTWARE_SUSPEND on HOTPLUG_CPU 
automatically without the user requiring to know about it.

Thanks to Stefan Richter and Linus Torvalds for valuable feedback.

Signed-off-by: Adrian Bunk <[EMAIL PROTECTED]>

---

 arch/i386/Kconfig    |   16 +++++++++++-----
 arch/powerpc/Kconfig |   11 +++++++++--
 arch/x86_64/Kconfig  |   19 ++++++++++++-------
 kernel/power/Kconfig |   23 +++++++++++++++++------
 4 files changed, 49 insertions(+), 20 deletions(-)

commit bb14e6721dc4e1a97efbfa5398d6021b321af52d
Author: Adrian Bunk <[EMAIL PROTECTED]>
Date:   Sat Jul 28 06:47:03 2007 +0200

    asdf

diff --git a/arch/i386/Kconfig b/arch/i386/Kconfig
index abb582b..eb00a12 100644
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -903,13 +903,19 @@ config PHYSICAL_ALIGN
 
          Don't change this unless you know what you are doing.
 
-config HOTPLUG_CPU
-       bool "Support for suspend on SMP and hot-pluggable CPUs (EXPERIMENTAL)"
+config HOTPLUG_CPU_POSSIBLE
+       bool
        depends on SMP && HOTPLUG && EXPERIMENTAL && !X86_VOYAGER
+       select SUSPEND_SMP_POSSIBLE
+       default y
+
+config HOTPLUG_CPU
+       bool "Support for hot-pluggable CPUs (EXPERIMENTAL)" if !SUSPEND_SMP
+       depends on HOTPLUG_CPU_POSSIBLE
+       default y if SUSPEND_SMP
        ---help---
-         Say Y here to experiment with turning CPUs off and on, and to
-         enable suspend on SMP systems. CPUs can be controlled through
-         /sys/devices/system/cpu.
+         Say Y here to experiment with turning CPUs off and on.
+         CPUs can be controlled through /sys/devices/system/cpu.
 
 config COMPAT_VDSO
        bool "Compat VDSO support"
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 00099ef..950be0b 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -192,9 +192,16 @@ config IOMMU_VMERGE
          from *_map_sg(). Say Y if you know the drivers you are using are
          properly handling this case.
 
-config HOTPLUG_CPU
-       bool "Support for enabling/disabling CPUs"
+config HOTPLUG_CPU_POSSIBLE
+       bool
        depends on SMP && HOTPLUG && EXPERIMENTAL && (PPC_PSERIES || PPC_PMAC)
+       select SUSPEND_SMP_POSSIBLE if PPC64
+       default y
+
+config HOTPLUG_CPU
+       bool "Support for enabling/disabling CPUs (EXPERIMENTAL)" if 
!SUSPEND_SMP
+       depends on HOTPLUG_CPU_POSSIBLE
+       default y if SUSPEND_SMP
        ---help---
          Say Y here to be able to disable and re-enable individual
          CPUs at runtime on SMP machines.
diff --git a/arch/x86_64/Kconfig b/arch/x86_64/Kconfig
index 45f82ae..810ed4b 100644
--- a/arch/x86_64/Kconfig
+++ b/arch/x86_64/Kconfig
@@ -443,16 +443,21 @@ config PHYSICAL_ALIGN
        hex
        default "0x200000"
 
-config HOTPLUG_CPU
-       bool "Support for suspend on SMP and hot-pluggable CPUs (EXPERIMENTAL)"
+config HOTPLUG_CPU_POSSIBLE
+       bool
        depends on SMP && HOTPLUG && EXPERIMENTAL
+       select SUSPEND_SMP_POSSIBLE
+       default y
+
+config HOTPLUG_CPU
+       bool "Support for hot-pluggable CPUs (EXPERIMENTAL)" if !SUSPEND_SMP
+       depends on HOTPLUG_CPU_POSSIBLE
+       default y if SUSPEND_SMP
        help
-               Say Y here to experiment with turning CPUs off and on.  CPUs
-               can be controlled through /sys/devices/system/cpu/cpu#.
-               This is also required for suspend/hibernation on SMP systems.
+         Say Y here to experiment with turning CPUs off and on.
+         CPUs can be controlled through /sys/devices/system/cpu/cpu#.
 
-               Say N if you want to disable CPU hotplug and don't need to
-               suspend.
+         If unsure, say N.
 
 config ARCH_ENABLE_MEMORY_HOTPLUG
        def_bool y
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index c1a106d..c7f74ba 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -72,9 +72,25 @@ config PM_TRACE
        CAUTION: this option will cause your machine's real-time clock to be
        set to an invalid time after a resume.
 
+config SUSPEND_UP_POSSIBLE
+       bool
+       depends on FRV || PPC32 || PPC64_SWSUSP || X86
+       depends on !SMP
+       default y
+
+# select'ed from arch/<arch>/Kconfig if supported
+config SUSPEND_SMP_POSSIBLE
+       bool
+
+config SUSPEND_SMP
+       bool
+       depends on SUSPEND_SMP_POSSIBLE && SOFTWARE_SUSPEND
+       default y
+
 config SOFTWARE_SUSPEND
        bool "Software Suspend (Hibernation)"
-       depends on PM && SWAP && (((X86 || PPC64_SWSUSP) && (!SMP || 
SUSPEND_SMP)) || ((FRV || PPC32) && !SMP))
+       depends on PM && SWAP
+       depends on SUSPEND_UP_POSSIBLE || SUSPEND_SMP_POSSIBLE
        ---help---
          Enable the suspend to disk (STD) functionality, which is usually
          called "hibernation" in user interfaces.  STD checkpoints the
@@ -132,11 +148,6 @@ config PM_STD_PARTITION
          suspended image to. It will simply pick the first available swap 
          device.
 
-config SUSPEND_SMP
-       bool
-       depends on HOTPLUG_CPU && (X86 || PPC64) && PM
-       default y
-
 config APM_EMULATION
        tristate "Advanced Power Management Emulation"
        depends on PM && SYS_SUPPORTS_APM_EMULATION

-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to