[RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Rafael J. Wysocki]

Hi,

This series of patches implements changes that are possible/necessary/desirable
(IMO) after the introduction of the .set_target() method in 'struct pm_ops'
(the patch that introduces it is in -mm,
http://marc.info/?l=linux-mm-commits&m=118306698814722&w=2).

The patches make the following changes:
* make ACPI use the new .set_target() method in 'struct pm_ops'
* add an ACPI helper function for the devices to determine the power state
  to put the device into
* move the definition of 'struct pm_ops' to <include/suspend.h>
* change the name of 'struct pm_ops' to 'struct platform_suspend_ops' and
  modify the names of some related functions and global variables accordingly
* modify 'struct platform_suspend_ops' so that .prepare() and .finish() don't
  take arguments (.enter() still takes the state argument, because some
  platforms don't need to implement the other callbacks)
* make some functions normally defined in kernel/power/main.c be also defined
  when CONFIG_PM is unset
* make suspend_ops be a static variable
* rework 'struct hibernation_ops' to add the new method analogous to
  .set_target()
* rename 'struct hibernation_ops' to 'struct platform_hibernation_ops' (in
  analogy with 'struct platform_suspend_ops')
The details are in the changelogs.

The series is on top of the current -mm (which is somewhat updated with respect
to 2.6.22-rc6-mm1).  For convenience, there is a series of patches applicable
on top of 2.6.22-rc6-mm1, including the $subject patchset, at:
http://www.sisk.pl/kernel/patches/2.6.22-rc6-mm1/ .

Comments welcome.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

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

[RFC][PATCH -mm 1/9] ACPI: Implement the set_target() callback from pm_ops

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

In the future some drivers may need to use ACPI to determine the low power
states in which to place their devices, but to provide the drivers with this
information the ACPI core needs to know what sleep state the system is going to
enter.  Namely, the device's state should not be too high power for given system
sleep state and, if the device is supposed to be able to wake up the system, its
state should not be too low power for the wake up to be possible).  For this
purpose, the ACPI core needs to implement the set_target() method in 'struct
pm_ops' and store the target system sleep state passed by the PM core in a
variable.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep/main.c |   84 ++++++++++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 32 deletions(-)

Index: linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/acpi/sleep/main.c	2007-06-28 23:45:43.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c	2007-06-30 12:18:47.000000000 +0200
@@ -34,34 +34,54 @@ static u32 acpi_suspend_states[] = {
 
 static int init_8259A_after_S1;
 
+extern int acpi_sleep_prepare(u32 acpi_state);
+extern void acpi_power_off(void);
+
+static u32 acpi_target_sleep_state = ACPI_STATE_S0;
+
+/**
+ *	acpi_pm_set_target - Set the target system sleep state to the state
+ *		associated with given @pm_state, if supported.
+ */
+
+static int acpi_pm_set_target(suspend_state_t pm_state)
+{
+	u32 acpi_state = acpi_suspend_states[pm_state];
+	int error = 0;
+
+	if (sleep_states[acpi_state]) {
+		acpi_target_sleep_state = acpi_state;
+	} else {
+		printk(KERN_ERR "ACPI does not support this state: %d\n",
+			pm_state);
+		error = -ENOSYS;
+	}
+	return error;
+}
+
 /**
  *	acpi_pm_prepare - Do preliminary suspend work.
- *	@pm_state:		suspend state we're entering.
+ *	@pm_state: ignored
  *
- *	Make sure we support the state. If we do, and we need it, set the
- *	firmware waking vector and do arch-specific nastiness to get the 
- *	wakeup code to the waking vector. 
+ *	If necessary, set the firmware waking vector and do arch-specific
+ *	nastiness to get the wakeup code to the waking vector.
  */
 
-extern int acpi_sleep_prepare(u32 acpi_state);
-extern void acpi_power_off(void);
-
 static int acpi_pm_prepare(suspend_state_t pm_state)
 {
-	u32 acpi_state = acpi_suspend_states[pm_state];
+	int error = acpi_sleep_prepare(acpi_target_sleep_state);
 
-	if (!sleep_states[acpi_state]) {
-		printk("acpi_pm_prepare does not support %d \n", pm_state);
-		return -EPERM;
-	}
-	return acpi_sleep_prepare(acpi_state);
+	if (error)
+		acpi_target_sleep_state = ACPI_STATE_S0;
+
+	return error;
 }
 
 /**
  *	acpi_pm_enter - Actually enter a sleep state.
- *	@pm_state:		State we're entering.
+ *	@pm_state: ignored
  *
- *	Flush caches and go to sleep. For STR or STD, we have to call 
+ *	Flush caches and go to sleep. For STR or S2, we have to call
  *	arch-specific assembly, which in turn call acpi_enter_sleep_state().
  *	It's unfortunate, but it works. Please fix if you're feeling frisky.
  */
@@ -70,31 +90,32 @@ static int acpi_pm_enter(suspend_state_t
 {
 	acpi_status status = AE_OK;
 	unsigned long flags = 0;
-	u32 acpi_state = acpi_suspend_states[pm_state];
+	u32 acpi_state = acpi_target_sleep_state;
 
 	ACPI_FLUSH_CPU_CACHE();
 
 	/* Do arch specific saving of state. */
-	if (pm_state > PM_SUSPEND_STANDBY) {
+	if (acpi_state == ACPI_STATE_S2 || acpi_state == ACPI_STATE_S3) {
 		int error = acpi_save_state_mem();
-		if (error)
+
+		if (error) {
+			acpi_target_sleep_state = ACPI_STATE_S0;
 			return error;
+		}
 	}
 
 	local_irq_save(flags);
 	acpi_enable_wakeup_device(acpi_state);
-	switch (pm_state) {
-	case PM_SUSPEND_STANDBY:
+	switch (acpi_state) {
+	case ACPI_STATE_S1:
 		barrier();
 		status = acpi_enter_sleep_state(acpi_state);
 		break;
 
-	case PM_SUSPEND_MEM:
+	case ACPI_STATE_S2:
+	case ACPI_STATE_S3:
 		do_suspend_lowlevel();
 		break;
-
-	default:
-		return -EINVAL;
 	}
 
 	/* ACPI 3.0 specs (P62) says that it's the responsabilty
@@ -107,12 +128,8 @@ static int acpi_pm_enter(suspend_state_t
 	local_irq_restore(flags);
 	printk(KERN_DEBUG "Back to C!\n");
 
-	/* restore processor state
-	 * We should only be here if we're coming back from STR or STD.
-	 * And, in the case of the latter, the memory image should have already
-	 * been loaded from disk.
-	 */
-	if (pm_state > PM_SUSPEND_STANDBY)
+	/* restore processor state */
+	if (acpi_state == ACPI_STATE_S2 || acpi_state == ACPI_STATE_S3)
 		acpi_restore_state_mem();
 
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
@@ -120,7 +137,7 @@ static int acpi_pm_enter(suspend_state_t
 
 /**
  *	acpi_pm_finish - Finish up suspend sequence.
- *	@pm_state:		State we're coming out of.
+ *	@pm_state: ignored
  *
  *	This is called after we wake back up (or if entering the sleep state
  *	failed). 
@@ -128,7 +145,7 @@ static int acpi_pm_enter(suspend_state_t
 
 static int acpi_pm_finish(suspend_state_t pm_state)
 {
-	u32 acpi_state = acpi_suspend_states[pm_state];
+	u32 acpi_state = acpi_target_sleep_state;
 
 	acpi_leave_sleep_state(acpi_state);
 	acpi_disable_wakeup_device(acpi_state);
@@ -136,6 +153,8 @@ static int acpi_pm_finish(suspend_state_
 	/* reset firmware waking vector */
 	acpi_set_firmware_waking_vector((acpi_physical_address) 0);
 
+	acpi_target_sleep_state = ACPI_STATE_S0;
+
 	if (init_8259A_after_S1) {
 		printk("Broken toshiba laptop -> kicking interrupts\n");
 		init_8259A(0);
@@ -176,6 +195,7 @@ static int acpi_pm_state_valid(suspend_s
 
 static struct pm_ops acpi_pm_ops = {
 	.valid = acpi_pm_state_valid,
+	.set_target = acpi_pm_set_target,
 	.prepare = acpi_pm_prepare,
 	.enter = acpi_pm_enter,
 	.finish = acpi_pm_finish,

[RFC][PATCH -mm 2/9] ACPI: Add acpi_pm_device_sleep_state helper routine

[Rafael J. Wysocki]

From: Shaohua Li <shaohua.li@intel.com>, Rafael J. Wysocki <rjw@sisk.pl>

Based on the David Brownell's patch at
http://marc.info/?l=linux-acpi&m=117873972806360&w=2

Add a helper routine returning the lowest power (highest number) ACPI device
power state that given device can be in while the system is in the sleep state
indicated by acpi_target_sleep_state .

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep/main.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h   |    2 +
 2 files changed, 53 insertions(+)

Index: linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/acpi/sleep/main.c	2007-06-30 12:18:47.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c	2007-06-30 12:18:58.000000000 +0200
@@ -255,6 +255,57 @@ static struct hibernation_ops acpi_hiber
 };
 #endif				/* CONFIG_SOFTWARE_SUSPEND */
 
+/**
+ *	acpi_pm_device_sleep_state - return the lowest power (highest number)
+ *				     ACPI device power state given device can be
+ *				     in while the system is in the sleep state
+ *				     indicated by %acpi_target_sleep_state
+ *	@handle: Represents the device the state is evaluated for
+ */
+
+int acpi_pm_device_sleep_state(acpi_handle handle)
+{
+	char acpi_method[] = "_SxD";
+	unsigned long d_min, d_max;
+	struct acpi_device *dev;
+
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
+		printk(KERN_ERR "ACPI handle has no context!\n");
+		return -ENODEV;
+	}
+	acpi_method[2] = '0' + acpi_target_sleep_state;
+	/*
+	 * If the sleep state is S0, we will return D3, but if the device has
+	 * _S0W, we will use the value from _S0W
+	 */
+	d_min = ACPI_STATE_D3;
+	d_max = ACPI_STATE_D3;
+	/*
+	 * If present, _SxD methods give the minimum D-state we may use
+	 * for each S-state ... with lowest latency state switching.
+	 *
+	 * We rely on acpi_evaluate_integer() not clobbering the integer
+	 * provided -- that's our fault recovery, we ignore retval.
+	 */
+	if (acpi_target_sleep_state > ACPI_STATE_S0)
+		acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);
+
+	/*
+	 * If _PRW says we can wake from the upcoming system state, the _SxD
+	 * value can wake ... and we'll assume a wakeup-aware driver.  If _SxW
+	 * methods exist (ACPI 3.x), they give the lowest power D-state that
+	 * can also wake the system.  _S0W can be valid.
+	 */
+	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
+	    (dev->wakeup.state.enabled &&
+	     dev->wakeup.sleep_state <= acpi_target_sleep_state)) {
+		d_max = d_min;
+		acpi_method[3] = 'W';
+		acpi_evaluate_integer(handle, acpi_method, NULL, &d_max);
+	}
+	return d_max;
+}
+
 /*
  * Toshiba fails to preserve interrupts over S1, reinitialization
  * of 8259 is needed after S1 resume.
Index: linux-2.6.22-rc6-mm1/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/acpi/acpi_bus.h	2007-06-28 21:56:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/acpi/acpi_bus.h	2007-06-30 12:18:58.000000000 +0200
@@ -364,6 +364,8 @@ acpi_handle acpi_get_child(acpi_handle, 
 acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
 #define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
 
+int acpi_pm_device_sleep_state(acpi_handle handle);
+
 #endif				/* CONFIG_ACPI */
 
 #endif /*__ACPI_BUS_H__*/

Re: [RFC][PATCH -mm 2/9] ACPI: Add acpi_pm_device_sleep_state helper routine

[David Brownell]

On Saturday 30 June 2007, Rafael J. Wysocki wrote:
> From: Shaohua Li <shaohua.li@intel.com>, Rafael J. Wysocki <rjw@sisk.pl>
> 
> Based on the David Brownell's patch at
> http://marc.info/?l=linux-acpi&m=117873972806360&w=2

I hope someone's going to refresh the PCI glue calling this, then...
making pci_choose_state() work was the goal of that patch!!

Also the updates to teach how the PCI root bridge wakeup capabilities
fit into the equation, for non-mainboard devices (all kinds of add-on
cards that talk PCI) ... that stuff was unclear to me, which is most
of why I left it out of that patch.

Correct me if I'm wrong here, but nothing else in this patch set
depends on this particular patch ... yes?


> Add a helper routine returning the lowest power (highest number) ACPI device
> power state that given device can be in while the system is in the sleep state
> indicated by acpi_target_sleep_state .
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/sleep/main.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_bus.h   |    2 +
>  2 files changed, 53 insertions(+)
> 
> Index: linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c
> ===================================================================
> --- linux-2.6.22-rc6-mm1.orig/drivers/acpi/sleep/main.c	2007-06-30 12:18:47.000000000 +0200
> +++ linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c	2007-06-30 12:18:58.000000000 +0200
> @@ -255,6 +255,57 @@ static struct hibernation_ops acpi_hiber
>  };
>  #endif				/* CONFIG_SOFTWARE_SUSPEND */
>  
> +/**
> + *	acpi_pm_device_sleep_state - return the lowest power (highest number)
> + *				     ACPI device power state given device can be
> + *				     in while the system is in the sleep state
> + *				     indicated by %acpi_target_sleep_state
> + *	@handle: Represents the device the state is evaluated for
> + */
> +
> +int acpi_pm_device_sleep_state(acpi_handle handle)

Yeah, moving this from the PCI glue to the ACPI core is probably
better.  But I'd like to see the comment use standard kerneldoc ...
that is, the descriptive paragraph follows a set of (one line)
summaries of the function and its parameter(s).  The description
needs to cover the fault returns too.

Also, recall that this was originally PCI-specific.  A generalized
approach would return a range of states, not a single state that
embeds a particular policy that may not be universally appropriate...


> +{
> +	char acpi_method[] = "_SxD";
> +	unsigned long d_min, d_max;
> +	struct acpi_device *dev;
> +
> +	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
> +		printk(KERN_ERR "ACPI handle has no context!\n");

The description above should cover this fault return ... the caller
would normally need some default to apply in such cases, too.

(Example:  PCI could just look at the PCI PM resource and see what
states are supported there.  Choosing PCI_D2, where it's available,
could eliminate various driver re-initialization problems.  That
might make some video code work better, from what I'm told ...)


> +		return -ENODEV;
> +	}
> +	acpi_method[2] = '0' + acpi_target_sleep_state;
> +	/*
> +	 * If the sleep state is S0, we will return D3, but if the device has
> +	 * _S0W, we will use the value from _S0W

There was also the assumption that if it can wake, it can do
so from D3 *or* there will be an _SxD method... remembering
that lots of systems don't have ACPI 3.0 _SxW methods.

But returning D3 is not necessarily best, here...


> +	 */
> +	d_min = ACPI_STATE_D3;
> +	d_max = ACPI_STATE_D3;

Other than the lack of empty lines separating code paragraphs ...
I recall choosing "d_min = ACPI_STATE_D3" with specific reference
to PCI.  It's not clear to me that's appropriate for non-PCI
devices.

The logic was that PCI devices can all support PCI_D0 and PCI_D3.
For non-PCI devices we can't actually know that.  So "min" should
probably default to ACPI_STATE_D0.  If this returns a range, then
such issues can stay out of this code.


> +	/*
> +	 * If present, _SxD methods give the minimum D-state we may use
> +	 * for each S-state ... with lowest latency state switching.
> +	 *
> +	 * We rely on acpi_evaluate_integer() not clobbering the integer
> +	 * provided -- that's our fault recovery, we ignore retval.
> +	 */
> +	if (acpi_target_sleep_state > ACPI_STATE_S0)
> +		acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);

... "lowest latency" implies avoiding ACPI D3.  Thing is, in my
testing with PCI, most devices didn't have _SxD methods, so the
only way to return anything other than PCI_D0 (i.e. some state
that saved power) was to force a different default.  D3 was the
only option that always worked.


> +
> +	/*
> +	 * If _PRW says we can wake from the upcoming system state, the _SxD
> +	 * value can wake ... and we'll assume a wakeup-aware driver.  If _SxW
> +	 * methods exist (ACPI 3.x), they give the lowest power D-state that
> +	 * can also wake the system.  _S0W can be valid.
> +	 */
> +	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> +	    (dev->wakeup.state.enabled &&

This used device_may_wakeup() before.  That ACPI flag is not a
direct analogue ... without looking again at the issues, I'd
say the right solution is to phase out the ACPI-only flags in
new code.  Maybe just expect the caller to pass the results
of device_may_wakeup() in ... or update the signature to accept
a "struct device", and fetch the handle from there.  (That'd
be a better match for most callers, I'd think...)


> +	     dev->wakeup.sleep_state <= acpi_target_sleep_state)) {
> +		d_max = d_min;

None of my systems happend to have _SxW methods to execute.
So with few _SxD methods, most of the time d_max never changed.
All the more reason to return both min and max...


> +		acpi_method[3] = 'W';
> +		acpi_evaluate_integer(handle, acpi_method, NULL, &d_max);
> +	}
> +	return d_max;

... so there's a range of from d_min to d_max that would be OK
with ACPI, but this particular routine is only showing one end
of the range.  For a general purpose routine, that doesn't seem
like it's obviously the best answer.

- Dave



> +}
> +
>  /*
>   * Toshiba fails to preserve interrupts over S1, reinitialization
>   * of 8259 is needed after S1 resume.
> Index: linux-2.6.22-rc6-mm1/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-2.6.22-rc6-mm1.orig/include/acpi/acpi_bus.h	2007-06-28 21:56:24.000000000 +0200
> +++ linux-2.6.22-rc6-mm1/include/acpi/acpi_bus.h	2007-06-30 12:18:58.000000000 +0200
> @@ -364,6 +364,8 @@ acpi_handle acpi_get_child(acpi_handle, 
>  acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
>  #define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
>  
> +int acpi_pm_device_sleep_state(acpi_handle handle);
> +
>  #endif				/* CONFIG_ACPI */
>  
>  #endif /*__ACPI_BUS_H__*/
> 

Re: [RFC][PATCH -mm 2/9] ACPI: Add acpi_pm_device_sleep_state helper routine

[Rafael J. Wysocki]

On Monday, 2 July 2007 07:49, David Brownell wrote:
> On Saturday 30 June 2007, Rafael J. Wysocki wrote:
> > From: Shaohua Li <shaohua.li@intel.com>, Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Based on the David Brownell's patch at
> > http://marc.info/?l=linux-acpi&m=117873972806360&w=2
> 
> I hope someone's going to refresh the PCI glue calling this, then...
> making pci_choose_state() work was the goal of that patch!!
> 
> Also the updates to teach how the PCI root bridge wakeup capabilities
> fit into the equation, for non-mainboard devices (all kinds of add-on
> cards that talk PCI) ... that stuff was unclear to me, which is most
> of why I left it out of that patch.
> 
> Correct me if I'm wrong here, but nothing else in this patch set
> depends on this particular patch ... yes?

That's correct.

> > Add a helper routine returning the lowest power (highest number) ACPI device
> > power state that given device can be in while the system is in the sleep state
> > indicated by acpi_target_sleep_state .
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  drivers/acpi/sleep/main.c |   51 ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/acpi/acpi_bus.h   |    2 +
> >  2 files changed, 53 insertions(+)
> > 
> > Index: linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c
> > ===================================================================
> > --- linux-2.6.22-rc6-mm1.orig/drivers/acpi/sleep/main.c	2007-06-30 12:18:47.000000000 +0200
> > +++ linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c	2007-06-30 12:18:58.000000000 +0200
> > @@ -255,6 +255,57 @@ static struct hibernation_ops acpi_hiber
> >  };
> >  #endif				/* CONFIG_SOFTWARE_SUSPEND */
> >  
> > +/**
> > + *	acpi_pm_device_sleep_state - return the lowest power (highest number)
> > + *				     ACPI device power state given device can be
> > + *				     in while the system is in the sleep state
> > + *				     indicated by %acpi_target_sleep_state
> > + *	@handle: Represents the device the state is evaluated for
> > + */
> > +
> > +int acpi_pm_device_sleep_state(acpi_handle handle)
> 
> Yeah, moving this from the PCI glue to the ACPI core is probably
> better.  But I'd like to see the comment use standard kerneldoc ...
> that is, the descriptive paragraph follows a set of (one line)
> summaries of the function and its parameter(s).  The description
> needs to cover the fault returns too.

OK

> Also, recall that this was originally PCI-specific.  A generalized
> approach would return a range of states, not a single state that
> embeds a particular policy that may not be universally appropriate...

Via pointers and the return value equal to 0 on success?

> > +{
> > +	char acpi_method[] = "_SxD";
> > +	unsigned long d_min, d_max;
> > +	struct acpi_device *dev;
> > +
> > +	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &dev))) {
> > +		printk(KERN_ERR "ACPI handle has no context!\n");
> 
> The description above should cover this fault return ... the caller
> would normally need some default to apply in such cases, too.
> 
> (Example:  PCI could just look at the PCI PM resource and see what
> states are supported there.  Choosing PCI_D2, where it's available,
> could eliminate various driver re-initialization problems.  That
> might make some video code work better, from what I'm told ...)

OK
 
> > +		return -ENODEV;
> > +	}
> > +	acpi_method[2] = '0' + acpi_target_sleep_state;
> > +	/*
> > +	 * If the sleep state is S0, we will return D3, but if the device has
> > +	 * _S0W, we will use the value from _S0W
> 
> There was also the assumption that if it can wake, it can do
> so from D3 *or* there will be an _SxD method... remembering
> that lots of systems don't have ACPI 3.0 _SxW methods.
> 
> But returning D3 is not necessarily best, here...
> 
> 
> > +	 */
> > +	d_min = ACPI_STATE_D3;
> > +	d_max = ACPI_STATE_D3;
> 
> Other than the lack of empty lines separating code paragraphs ...
> I recall choosing "d_min = ACPI_STATE_D3" with specific reference
> to PCI.  It's not clear to me that's appropriate for non-PCI
> devices.
>
> The logic was that PCI devices can all support PCI_D0 and PCI_D3.
> For non-PCI devices we can't actually know that.  So "min" should
> probably default to ACPI_STATE_D0.  If this returns a range, then
> such issues can stay out of this code.

The ACPI spec says that all devices must support D0 and D3.
 
> > +	/*
> > +	 * If present, _SxD methods give the minimum D-state we may use
> > +	 * for each S-state ... with lowest latency state switching.
> > +	 *
> > +	 * We rely on acpi_evaluate_integer() not clobbering the integer
> > +	 * provided -- that's our fault recovery, we ignore retval.
> > +	 */
> > +	if (acpi_target_sleep_state > ACPI_STATE_S0)
> > +		acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);
> 
> ... "lowest latency" implies avoiding ACPI D3.  Thing is, in my
> testing with PCI, most devices didn't have _SxD methods, so the
> only way to return anything other than PCI_D0 (i.e. some state
> that saved power) was to force a different default.  D3 was the
> only option that always worked.

I think that usually D3 will be returned here.

> > +
> > +	/*
> > +	 * If _PRW says we can wake from the upcoming system state, the _SxD
> > +	 * value can wake ... and we'll assume a wakeup-aware driver.  If _SxW
> > +	 * methods exist (ACPI 3.x), they give the lowest power D-state that
> > +	 * can also wake the system.  _S0W can be valid.
> > +	 */
> > +	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> > +	    (dev->wakeup.state.enabled &&
> 
> This used device_may_wakeup() before.  That ACPI flag is not a
> direct analogue ... without looking again at the issues, I'd
> say the right solution is to phase out the ACPI-only flags in
> new code.

Hm, I'm not sure.  This is an ACPI routine after all ...

> Maybe just expect the caller to pass the results 
> of device_may_wakeup() in ... or update the signature to accept
> a "struct device", and fetch the handle from there.  (That'd
> be a better match for most callers, I'd think...)

In that case it would be nicer to pass 'struct acpi_device *' rahter than
'struct device *', IMO.

> > +	     dev->wakeup.sleep_state <= acpi_target_sleep_state)) {
> > +		d_max = d_min;
> 
> None of my systems happend to have _SxW methods to execute.
> So with few _SxD methods, most of the time d_max never changed.
> All the more reason to return both min and max...
> 
> 
> > +		acpi_method[3] = 'W';
> > +		acpi_evaluate_integer(handle, acpi_method, NULL, &d_max);
> > +	}
> > +	return d_max;
> 
> ... so there's a range of from d_min to d_max that would be OK
> with ACPI, but this particular routine is only showing one end
> of the range.  For a general purpose routine, that doesn't seem
> like it's obviously the best answer.

We can return a range.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [RFC][PATCH -mm 2/9] ACPI: Add acpi_pm_device_sleep_state helper routine

[David Brownell]

On Monday 02 July 2007, Rafael J. Wysocki wrote:
> On Monday, 2 July 2007 07:49, David Brownell wrote:

> > Also, recall that this was originally PCI-specific.  A generalized
> > approach would return a range of states, not a single state that
> > embeds a particular policy that may not be universally appropriate...
> 
> Via pointers and the return value equal to 0 on success?

It could as easily be one pointer:  "int minmax[2]".
Then min == minmax[0] and max == minmax[1], and the
ACPI calls could write the caller's data directly.

But in general, yes -- pointer, not single return value.


> > The logic was that PCI devices can all support PCI_D0 and PCI_D3.
> > For non-PCI devices we can't actually know that.  So "min" should
> > probably default to ACPI_STATE_D0.  If this returns a range, then
> > such issues can stay out of this code.
> 
> The ACPI spec says that all devices must support D0 and D3.

ISTR that it actually says they must "recognize" D3.  That's
not the same as actually implementing a software controllable
power-off state ... and not the same as imposing a "use the
biggest numbered D-state" policy at this level.


> > > +	/*
> > > +	 * If _PRW says we can wake from the upcoming system state, the _SxD
> > > +	 * value can wake ... and we'll assume a wakeup-aware driver.  If _SxW
> > > +	 * methods exist (ACPI 3.x), they give the lowest power D-state that
> > > +	 * can also wake the system.  _S0W can be valid.
> > > +	 */
> > > +	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> > > +	    (dev->wakeup.state.enabled &&
> > 
> > This used device_may_wakeup() before.  That ACPI flag is not a
> > direct analogue ... without looking again at the issues, I'd
> > say the right solution is to phase out the ACPI-only flags in
> > new code.
> 
> Hm, I'm not sure.  This is an ACPI routine after all ...

It's in the Linux kernel, talking about Linux devices.
Some of them have ACPI support; many don't.  (Like the
USB device nodes, and anything else layered on top of
mainboard devices.)


> > Maybe just expect the caller to pass the results 
> > of device_may_wakeup() in ... or update the signature to accept
> > a "struct device", and fetch the handle from there.  (That'd
> > be a better match for most callers, I'd think...)
> 
> In that case it would be nicer to pass 'struct acpi_device *' rahter than
> 'struct device *', IMO.

I'll disagree.  Outside of the ACPI code, virtually nothing
has any reason to see an "acpi_device" ... but virtually
everything has reason to see a "device".  Even ACPI code
can rely on there being a "device" inside "acpi_device"!


This touches on a different problem.  The ACPI device tree
is parallel to the "real" tree.  In the cases there's a
one-to-one correspondence, there's no confusion; any
acpi_device corresponds to one "real" device, and vice
versa.  BUT ... there are cases where the correspondence
isn't one-to-one.  Those cases need to be addressed, by
moving towards a one-to-one correspondence.

Cases like PCI bridges can be easily dealt with now, given
some resequencing of init logic:  use the PNP node instead
of faking out a toplevel /sys/devices/pci0000:00 node.

But things like PS2 keyboard and mouse nodes are funkier.

- Dave

Re: [RFC][PATCH -mm 2/9] ACPI: Add acpi_pm_device_sleep_state helper routine

[Rafael J. Wysocki]

On Monday, 2 July 2007 19:24, David Brownell wrote:
> On Monday 02 July 2007, Rafael J. Wysocki wrote:
> > On Monday, 2 July 2007 07:49, David Brownell wrote:
> 
> > > Also, recall that this was originally PCI-specific.  A generalized
> > > approach would return a range of states, not a single state that
> > > embeds a particular policy that may not be universally appropriate...
> > 
> > Via pointers and the return value equal to 0 on success?
> 
> It could as easily be one pointer:  "int minmax[2]".
> Then min == minmax[0] and max == minmax[1], and the
> ACPI calls could write the caller's data directly.
> 
> But in general, yes -- pointer, not single return value.
> 
> 
> > > The logic was that PCI devices can all support PCI_D0 and PCI_D3.
> > > For non-PCI devices we can't actually know that.  So "min" should
> > > probably default to ACPI_STATE_D0.  If this returns a range, then
> > > such issues can stay out of this code.
> > 
> > The ACPI spec says that all devices must support D0 and D3.
> 
> ISTR that it actually says they must "recognize" D3.  That's
> not the same as actually implementing a software controllable
> power-off state ... and not the same as imposing a "use the
> biggest numbered D-state" policy at this level.

No, it literally says "All devices must support the D0 and D3 states" (that
actually is stated in Appendix A, A.3.4 in the 3.0b specification).

Still, I think that the question really is what we should return if _SxD or
_SxW are not present.

> > > > +	/*
> > > > +	 * If _PRW says we can wake from the upcoming system state, the _SxD
> > > > +	 * value can wake ... and we'll assume a wakeup-aware driver.  If _SxW
> > > > +	 * methods exist (ACPI 3.x), they give the lowest power D-state that
> > > > +	 * can also wake the system.  _S0W can be valid.
> > > > +	 */
> > > > +	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> > > > +	    (dev->wakeup.state.enabled &&

These things are the result of the evaluation of _PRW for that device and
we need to check them here (or evaluate _PRW ourselves, but that wouldn't make
sense, since it's already been evaulated), unless the caller tells us not to
bother (we should provide the caller with a means to do it, though).

> > > 
> > > This used device_may_wakeup() before.  That ACPI flag is not a
> > > direct analogue ... without looking again at the issues, I'd
> > > say the right solution is to phase out the ACPI-only flags in
> > > new code.
> > 
> > Hm, I'm not sure.  This is an ACPI routine after all ...
> 
> It's in the Linux kernel, talking about Linux devices.
> Some of them have ACPI support; many don't.  (Like the
> USB device nodes, and anything else layered on top of
> mainboard devices.)

I've explained above why these ACPI flags should generally be used here.

> > > Maybe just expect the caller to pass the results 
> > > of device_may_wakeup() in ... or update the signature to accept
> > > a "struct device", and fetch the handle from there.  (That'd
> > > be a better match for most callers, I'd think...)
> > 
> > In that case it would be nicer to pass 'struct acpi_device *' rahter than
> > 'struct device *', IMO.
> 
> I'll disagree.  Outside of the ACPI code, virtually nothing
> has any reason to see an "acpi_device" ... but virtually
> everything has reason to see a "device".  Even ACPI code
> can rely on there being a "device" inside "acpi_device"!

Okay, we can use DEVICE_ACPI_HANDLE() in the same way as in your original
patch, no problem with that.

> This touches on a different problem.  The ACPI device tree
> is parallel to the "real" tree.  In the cases there's a
> one-to-one correspondence, there's no confusion; any
> acpi_device corresponds to one "real" device, and vice
> versa.  BUT ... there are cases where the correspondence
> isn't one-to-one.  Those cases need to be addressed, by
> moving towards a one-to-one correspondence.

Agreed.
 
> Cases like PCI bridges can be easily dealt with now, given
> some resequencing of init logic:  use the PNP node instead
> of faking out a toplevel /sys/devices/pci0000:00 node.
> 
> But things like PS2 keyboard and mouse nodes are funkier.

Yes. :-)

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [RFC][PATCH -mm 2/9] ACPI: Add acpi_pm_device_sleep_state helper routine

[Rafael J. Wysocki]

On Monday, 2 July 2007 22:15, Rafael J. Wysocki wrote:
> On Monday, 2 July 2007 19:24, David Brownell wrote:
> > On Monday 02 July 2007, Rafael J. Wysocki wrote:
> > > On Monday, 2 July 2007 07:49, David Brownell wrote:
> > 
> > > > Also, recall that this was originally PCI-specific.  A generalized
> > > > approach would return a range of states, not a single state that
> > > > embeds a particular policy that may not be universally appropriate...
> > > 
> > > Via pointers and the return value equal to 0 on success?
> > 
> > It could as easily be one pointer:  "int minmax[2]".
> > Then min == minmax[0] and max == minmax[1], and the
> > ACPI calls could write the caller's data directly.
> > 
> > But in general, yes -- pointer, not single return value.
> > 
> > 
> > > > The logic was that PCI devices can all support PCI_D0 and PCI_D3.
> > > > For non-PCI devices we can't actually know that.  So "min" should
> > > > probably default to ACPI_STATE_D0.  If this returns a range, then
> > > > such issues can stay out of this code.
> > > 
> > > The ACPI spec says that all devices must support D0 and D3.
> > 
> > ISTR that it actually says they must "recognize" D3.  That's
> > not the same as actually implementing a software controllable
> > power-off state ... and not the same as imposing a "use the
> > biggest numbered D-state" policy at this level.
> 
> No, it literally says "All devices must support the D0 and D3 states" (that
> actually is stated in Appendix A, A.3.4 in the 3.0b specification).
> 
> Still, I think that the question really is what we should return if _SxD or
> _SxW are not present.
> 
> > > > > +	/*
> > > > > +	 * If _PRW says we can wake from the upcoming system state, the _SxD
> > > > > +	 * value can wake ... and we'll assume a wakeup-aware driver.  If _SxW
> > > > > +	 * methods exist (ACPI 3.x), they give the lowest power D-state that
> > > > > +	 * can also wake the system.  _S0W can be valid.
> > > > > +	 */
> > > > > +	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
> > > > > +	    (dev->wakeup.state.enabled &&
> 
> These things are the result of the evaluation of _PRW for that device and
> we need to check them here (or evaluate _PRW ourselves, but that wouldn't make
> sense, since it's already been evaulated), unless the caller tells us not to
> bother (we should provide the caller with a means to do it, though).

I looked at the ACPI spec again and updated the patch (please see below).

Greetings,
Rafael


---
From: Shaohua Li <shaohua.li@intel.com>, Rafael J. Wysocki <rjw@sisk.pl>

Based on the David Brownell's patch at
http://marc.info/?l=linux-acpi&m=117873972806360&w=2

Add a helper routine returning the lowest power (highest number) ACPI device
power state that given device can be in while the system is in the sleep state
indicated by acpi_target_sleep_state .

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep/main.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h   |    2 +
 2 files changed, 74 insertions(+)

Index: linux-2.6.22-rc7/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.22-rc7.orig/drivers/acpi/sleep/main.c	2007-07-02 22:38:34.000000000 +0200
+++ linux-2.6.22-rc7/drivers/acpi/sleep/main.c	2007-07-03 11:57:27.000000000 +0200
@@ -269,6 +269,78 @@ static struct platform_hibernation_ops a
 };
 #endif				/* CONFIG_SOFTWARE_SUSPEND */
 
+/**
+ *	acpi_pm_device_sleep_state - return preferred power state of ACPI device
+ *		in the system sleep state given by %acpi_target_sleep_state
+ *	@dev: device to examine
+ *	@wake: if set, the device should be able to wake up the system
+ *	@d_min_p: used to store the upper limit of allowed states range
+ *	Return value: preferred power state of the device on success, -ENODEV on
+ *		failure (ie. if there's no 'struct acpi_device' for @dev)
+ *
+ *	Find the lowest power (highest number) ACPI device power state that
+ *	device @dev can be in while the system is in the sleep state represented
+ *	by %acpi_target_sleep_state.  If @wake is nonzero, the device should be
+ *	able to wake up the system from this sleep state.  If @d_min_p is set,
+ *	the highest power (lowest number) device power state of @dev allowed
+ *	in this system sleep state is stored at the location pointed to by it.
+ *
+ *	The caller must ensure that @dev is valid before using this function.
+ */
+
+int acpi_pm_device_sleep_state(struct device *dev, int wake, int *d_min_p)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(dev);
+	struct acpi_device *adev;
+	char acpi_method[] = "_SxD";
+	unsigned long d_min, d_max;
+
+	if (!handle || ACPI_FAILURE(acpi_bus_get_device(handle, &adev))) {
+		printk(KERN_ERR "ACPI handle has no context!\n");
+		return -ENODEV;
+	}
+
+	acpi_method[2] = '0' + acpi_target_sleep_state;
+	/*
+	 * If the sleep state is S0, we will return D3, but if the device has
+	 * _S0W, we will use the value from _S0W
+	 */
+	d_min = ACPI_STATE_D0;
+	d_max = ACPI_STATE_D3;
+
+	/*
+	 * If present, _SxD methods return the minimum D-state (highest power
+	 * state) we can use for the corresponding S-states.  Otherwise, the
+	 * minimum D-state is D0 (ACPI 3.x).
+	 *
+	 * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
+	 * provided -- that's our fault recovery, we ignore retval.
+	 */
+	if (acpi_target_sleep_state > ACPI_STATE_S0)
+		acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);
+
+	/*
+	 * If _PRW says we can wake up the system from the target sleep state,
+	 * the D-state returned by _SxD is sufficient for that (we assume a
+	 * wakeup-aware driver if wake is set).  Still, if _SxW exists
+	 * (ACPI 3.x), it should return the maximum (lowest power) D-state that
+	 * can wake the system.  _S0W may be valid, too.
+	 */
+	if (acpi_target_sleep_state == ACPI_STATE_S0 ||
+	    (wake && adev->wakeup.state.enabled &&
+	     adev->wakeup.sleep_state <= acpi_target_sleep_state)) {
+		acpi_method[3] = 'W';
+		acpi_evaluate_integer(handle, acpi_method, NULL, &d_max);
+		/* Sanity check */
+		if (d_max < d_min)
+			d_min = d_max;
+	}
+
+	if (d_min_p)
+		*d_min_p = d_min;
+	return d_max;
+}
+
 /*
  * Toshiba fails to preserve interrupts over S1, reinitialization
  * of 8259 is needed after S1 resume.
Index: linux-2.6.22-rc7/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.22-rc7.orig/include/acpi/acpi_bus.h	2007-07-02 22:36:30.000000000 +0200
+++ linux-2.6.22-rc7/include/acpi/acpi_bus.h	2007-07-02 23:06:34.000000000 +0200
@@ -364,6 +364,8 @@ acpi_handle acpi_get_child(acpi_handle, 
 acpi_handle acpi_get_pci_rootbridge_handle(unsigned int, unsigned int);
 #define DEVICE_ACPI_HANDLE(dev) ((acpi_handle)((dev)->archdata.acpi_handle))
 
+int acpi_pm_device_sleep_state(struct device *, int, int *);
+
 #endif				/* CONFIG_ACPI */
 
 #endif /*__ACPI_BUS_H__*/

[RFC][PATCH -mm 3/9] PM: Move definition of struct pm_ops to suspend.h

[Rafael J. Wysocki]

[Previous version of this was acked by Pavel, but I've updated it since then.]
---
From: Rafael J. Wysocki <rjw@sisk.pl>

Move the definition of 'struct pm_ops' and related functions from <linux/pm.h>
to <linux/suspend.h> .

There are, at least, the following reasons to do that:
* 'struct pm_ops' is specifically related to suspend and not to the power
  management in general.
* As long as 'struct pm_ops' is defined in <linux/pm.h>, any modification of it
  causes the entire kernel to be recompiled, which is unnecessary and annoying.
* Some suspend-related features are already defined in <linux/suspend.h>, so it
  is logical to move the definition of 'struct pm_ops' into there.
* 'struct hibernation_ops', being the hibernation-related counterpart of
  'struct pm_ops', is defined in <linux/suspend.h> .

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/common/sharpsl_pm.c             |    1 
 arch/arm/mach-at91/pm.c                  |    3 
 arch/arm/mach-omap1/pm.c                 |    3 
 arch/arm/mach-omap2/pm.c                 |    9 --
 arch/arm/mach-pnx4008/pm.c               |    2 
 arch/arm/mach-pxa/pxa25x.c               |    2 
 arch/arm/mach-pxa/pxa27x.c               |    2 
 arch/blackfin/mach-common/pm.c           |    2 
 arch/powerpc/platforms/52xx/mpc52xx_pm.c |    2 
 drivers/char/apm-emulation.c             |    2 
 include/asm-arm/arch-pxa/pm.h            |    2 
 include/linux/pm.h                       |   91 ------------------------
 include/linux/suspend.h                  |  113 +++++++++++++++++++++++++++----
 13 files changed, 114 insertions(+), 120 deletions(-)

Index: linux-2.6.22-rc6/include/linux/suspend.h
===================================================================
--- linux-2.6.22-rc6.orig/include/linux/suspend.h
+++ linux-2.6.22-rc6/include/linux/suspend.h
@@ -1,5 +1,5 @@
-#ifndef _LINUX_SWSUSP_H
-#define _LINUX_SWSUSP_H
+#ifndef _LINUX_SUSPEND_H
+#define _LINUX_SUSPEND_H
 
 #if defined(CONFIG_X86) || defined(CONFIG_FRV) || defined(CONFIG_PPC32) || defined(CONFIG_PPC64)
 #include <asm/suspend.h>
@@ -10,6 +10,105 @@
 #include <linux/pm.h>
 #include <linux/mm.h>
 
+#if defined(CONFIG_PM) && defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
+extern int pm_prepare_console(void);
+extern void pm_restore_console(void);
+#else
+static inline int pm_prepare_console(void) { return 0; }
+static inline void pm_restore_console(void) {}
+#endif
+
+typedef int __bitwise suspend_state_t;
+
+#define PM_SUSPEND_ON		((__force suspend_state_t) 0)
+#define PM_SUSPEND_STANDBY	((__force suspend_state_t) 1)
+#define PM_SUSPEND_MEM		((__force suspend_state_t) 3)
+#define PM_SUSPEND_MAX		((__force suspend_state_t) 4)
+
+/**
+ * struct pm_ops - Callbacks for managing platform dependent system sleep
+ *	states.
+ *
+ * @valid: Callback to determine if given system sleep state is supported by
+ *	the platform.
+ *	Valid (ie. supported) states are advertised in /sys/power/state.  Note
+ *	that it still may be impossible to enter given system sleep state if the
+ *	conditions aren't right.
+ *	There is the %pm_valid_only_mem function available that can be assigned
+ *	to this if the platform only supports mem sleep.
+ *
+ * @set_target: Tell the platform which system sleep state is going to be
+ *	entered.
+ *	@set_target() is executed right prior to suspending devices.  The
+ *	information conveyed to the platform code by @set_target() should be
+ *	disregarded by the platform as soon as @finish() is executed and if
+ *	@prepare() fails.  If @set_target() fails (ie. returns nonzero),
+ *	@prepare(), @enter() and @finish() will not be called by the PM core.
+ *	This callback is optional.  However, if it is implemented, the argument
+ *	passed to @prepare(), @enter() and @finish() is meaningless and should
+ *	be ignored.
+ *
+ * @prepare: Prepare the platform for entering the system sleep state indicated
+ *	by @set_target() or represented by the argument if @set_target() is not
+ *	implemented.
+ *	@prepare() is called right after devices have been suspended (ie. the
+ *	appropriate .suspend() method has been executed for each device) and
+ *	before the nonboot CPUs are disabled (it is executed with IRQs enabled).
+ *	This callback is optional.  It returns 0 on success or a negative
+ *	error code otherwise, in which case the system cannot enter the desired
+ *	sleep state (@enter() and @finish() will not be called in that case).
+ *
+ * @enter: Enter the system sleep state indicated by @set_target() or
+ *	represented by the argument if @set_target() is not implemented.
+ *	This callback is mandatory.  It returns 0 on success or a negative
+ *	error code otherwise, in which case the system cannot enter the desired
+ *	sleep state.
+ *
+ * @finish: Called when the system has just left a sleep state, right after
+ *	the nonboot CPUs have been enabled and before devices are resumed (it is
+ *	executed with IRQs enabled).  If @set_target() is not implemented, the
+ *	argument represents the sleep state being left.
+ *	This callback is optional, but should be implemented by the platforms
+ *	that implement @prepare().  If implemented, it is always called after
+ *	@enter() (even if @enter() fails).
+ */
+struct pm_ops {
+	int (*valid)(suspend_state_t state);
+	int (*set_target)(suspend_state_t state);
+	int (*prepare)(suspend_state_t state);
+	int (*enter)(suspend_state_t state);
+	int (*finish)(suspend_state_t state);
+};
+
+extern struct pm_ops *pm_ops;
+
+/**
+ * pm_set_ops - set platform dependent power management ops
+ * @pm_ops: The new power management operations to set.
+ */
+extern void pm_set_ops(struct pm_ops *pm_ops);
+extern int pm_valid_only_mem(suspend_state_t state);
+
+/**
+ * arch_suspend_disable_irqs - disable IRQs for suspend
+ *
+ * Disables IRQs (in the default case). This is a weak symbol in the common
+ * code and thus allows architectures to override it if more needs to be
+ * done. Not called for suspend to disk.
+ */
+extern void arch_suspend_disable_irqs(void);
+
+/**
+ * arch_suspend_enable_irqs - enable IRQs after suspend
+ *
+ * Enables IRQs (in the default case). This is a weak symbol in the common
+ * code and thus allows architectures to override it if more needs to be
+ * done. Not called for suspend to disk.
+ */
+extern void arch_suspend_enable_irqs(void);
+
+extern int pm_suspend(suspend_state_t state);
+
 /* struct pbe is used for creating lists of pages that should be restored
  * atomically during the resume from disk, because the page frames they have
  * occupied before the suspend are in use.
@@ -24,14 +123,6 @@ struct pbe {
 extern void drain_local_pages(void);
 extern void mark_free_pages(struct zone *zone);
 
-#if defined(CONFIG_PM) && defined(CONFIG_VT) && defined(CONFIG_VT_CONSOLE)
-extern int pm_prepare_console(void);
-extern void pm_restore_console(void);
-#else
-static inline int pm_prepare_console(void) { return 0; }
-static inline void pm_restore_console(void) {}
-#endif
-
 /**
  * struct hibernation_ops - hibernation platform support
  *
@@ -127,4 +218,4 @@ static inline void register_nosave_regio
 }
 #endif
 
-#endif /* _LINUX_SWSUSP_H */
+#endif /* _LINUX_SUSPEND_H */
Index: linux-2.6.22-rc6/arch/arm/common/sharpsl_pm.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/arm/common/sharpsl_pm.c
+++ linux-2.6.22-rc6/arch/arm/common/sharpsl_pm.c
@@ -24,6 +24,7 @@
 #include <linux/platform_device.h>
 #include <linux/leds.h>
 #include <linux/apm-emulation.h>
+#include <linux/suspend.h>
 
 #include <asm/hardware.h>
 #include <asm/mach-types.h>
Index: linux-2.6.22-rc6/arch/arm/mach-pnx4008/pm.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/arm/mach-pnx4008/pm.c
+++ linux-2.6.22-rc6/arch/arm/mach-pnx4008/pm.c
@@ -15,7 +15,7 @@
 #include <linux/rtc.h>
 #include <linux/sched.h>
 #include <linux/proc_fs.h>
-#include <linux/pm.h>
+#include <linux/suspend.h>
 #include <linux/delay.h>
 #include <linux/clk.h>
 
Index: linux-2.6.22-rc6/arch/arm/mach-omap1/pm.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/arm/mach-omap1/pm.c
+++ linux-2.6.22-rc6/arch/arm/mach-omap1/pm.c
@@ -35,10 +35,9 @@
  * 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
-#include <linux/pm.h>
+#include <linux/suspend.h>
 #include <linux/sched.h>
 #include <linux/proc_fs.h>
-#include <linux/pm.h>
 #include <linux/interrupt.h>
 #include <linux/sysfs.h>
 #include <linux/module.h>
Index: linux-2.6.22-rc6/arch/arm/mach-omap2/pm.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/arm/mach-omap2/pm.c
+++ linux-2.6.22-rc6/arch/arm/mach-omap2/pm.c
@@ -16,10 +16,9 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/pm.h>
+#include <linux/suspend.h>
 #include <linux/sched.h>
 #include <linux/proc_fs.h>
-#include <linux/pm.h>
 #include <linux/interrupt.h>
 #include <linux/sysfs.h>
 #include <linux/module.h>
@@ -85,9 +84,6 @@ static int omap2_pm_prepare(suspend_stat
 	case PM_SUSPEND_MEM:
 		break;
 
-	case PM_SUSPEND_DISK:
-		return -ENOTSUPP;
-
 	default:
 		return -EINVAL;
 	}
@@ -353,9 +349,6 @@ static int omap2_pm_enter(suspend_state_
 	case PM_SUSPEND_MEM:
 		ret = omap2_pm_suspend();
 		break;
-	case PM_SUSPEND_DISK:
-		ret = -ENOTSUPP;
-		break;
 	default:
 		ret = -EINVAL;
 	}
Index: linux-2.6.22-rc6/arch/arm/mach-at91/pm.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/arm/mach-at91/pm.c
+++ linux-2.6.22-rc6/arch/arm/mach-at91/pm.c
@@ -10,10 +10,9 @@
  * (at your option) any later version.
  */
 
-#include <linux/pm.h>
+#include <linux/suspend.h>
 #include <linux/sched.h>
 #include <linux/proc_fs.h>
-#include <linux/pm.h>
 #include <linux/interrupt.h>
 #include <linux/sysfs.h>
 #include <linux/module.h>
Index: linux-2.6.22-rc6/arch/blackfin/mach-common/pm.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/blackfin/mach-common/pm.c
+++ linux-2.6.22-rc6/arch/blackfin/mach-common/pm.c
@@ -32,7 +32,7 @@
  * 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
-#include <linux/pm.h>
+#include <linux/suspend.h>
 #include <linux/sched.h>
 #include <linux/proc_fs.h>
 
Index: linux-2.6.22-rc6/arch/powerpc/platforms/52xx/mpc52xx_pm.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/powerpc/platforms/52xx/mpc52xx_pm.c
+++ linux-2.6.22-rc6/arch/powerpc/platforms/52xx/mpc52xx_pm.c
@@ -1,5 +1,5 @@
 #include <linux/init.h>
-#include <linux/pm.h>
+#include <linux/suspend.h>
 #include <linux/io.h>
 #include <asm/time.h>
 #include <asm/cacheflush.h>
Index: linux-2.6.22-rc6/include/linux/pm.h
===================================================================
--- linux-2.6.22-rc6.orig/include/linux/pm.h
+++ linux-2.6.22-rc6/include/linux/pm.h
@@ -102,97 +102,6 @@ struct pm_dev
 extern void (*pm_idle)(void);
 extern void (*pm_power_off)(void);
 
-typedef int __bitwise suspend_state_t;
-
-#define PM_SUSPEND_ON		((__force suspend_state_t) 0)
-#define PM_SUSPEND_STANDBY	((__force suspend_state_t) 1)
-#define PM_SUSPEND_MEM		((__force suspend_state_t) 3)
-#define PM_SUSPEND_MAX		((__force suspend_state_t) 4)
-
-/**
- * struct pm_ops - Callbacks for managing platform dependent system sleep
- *	states.
- *
- * @valid: Callback to determine if given system sleep state is supported by
- *	the platform.
- *	Valid (ie. supported) states are advertised in /sys/power/state.  Note
- *	that it still may be impossible to enter given system sleep state if the
- *	conditions aren't right.
- *	There is the %pm_valid_only_mem function available that can be assigned
- *	to this if the platform only supports mem sleep.
- *
- * @set_target: Tell the platform which system sleep state is going to be
- *	entered.
- *	@set_target() is executed right prior to suspending devices.  The
- *	information conveyed to the platform code by @set_target() should be
- *	disregarded by the platform as soon as @finish() is executed and if
- *	@prepare() fails.  If @set_target() fails (ie. returns nonzero),
- *	@prepare(), @enter() and @finish() will not be called by the PM core.
- *	This callback is optional.  However, if it is implemented, the argument
- *	passed to @prepare(), @enter() and @finish() is meaningless and should
- *	be ignored.
- *
- * @prepare: Prepare the platform for entering the system sleep state indicated
- *	by @set_target() or represented by the argument if @set_target() is not
- *	implemented.
- *	@prepare() is called right after devices have been suspended (ie. the
- *	appropriate .suspend() method has been executed for each device) and
- *	before the nonboot CPUs are disabled (it is executed with IRQs enabled).
- *	This callback is optional.  It returns 0 on success or a negative
- *	error code otherwise, in which case the system cannot enter the desired
- *	sleep state (@enter() and @finish() will not be called in that case).
- *
- * @enter: Enter the system sleep state indicated by @set_target() or
- *	represented by the argument if @set_target() is not implemented.
- *	This callback is mandatory.  It returns 0 on success or a negative
- *	error code otherwise, in which case the system cannot enter the desired
- *	sleep state.
- *
- * @finish: Called when the system has just left a sleep state, right after
- *	the nonboot CPUs have been enabled and before devices are resumed (it is
- *	executed with IRQs enabled).  If @set_target() is not implemented, the
- *	argument represents the sleep state being left.
- *	This callback is optional, but should be implemented by the platforms
- *	that implement @prepare().  If implemented, it is always called after
- *	@enter() (even if @enter() fails).
- */
-struct pm_ops {
-	int (*valid)(suspend_state_t state);
-	int (*set_target)(suspend_state_t state);
-	int (*prepare)(suspend_state_t state);
-	int (*enter)(suspend_state_t state);
-	int (*finish)(suspend_state_t state);
-};
-
-extern struct pm_ops *pm_ops;
-
-/**
- * pm_set_ops - set platform dependent power management ops
- * @pm_ops: The new power management operations to set.
- */
-extern void pm_set_ops(struct pm_ops *pm_ops);
-extern int pm_valid_only_mem(suspend_state_t state);
-
-/**
- * arch_suspend_disable_irqs - disable IRQs for suspend
- *
- * Disables IRQs (in the default case). This is a weak symbol in the common
- * code and thus allows architectures to override it if more needs to be
- * done. Not called for suspend to disk.
- */
-extern void arch_suspend_disable_irqs(void);
-
-/**
- * arch_suspend_enable_irqs - enable IRQs after suspend
- *
- * Enables IRQs (in the default case). This is a weak symbol in the common
- * code and thus allows architectures to override it if more needs to be
- * done. Not called for suspend to disk.
- */
-extern void arch_suspend_enable_irqs(void);
-
-extern int pm_suspend(suspend_state_t state);
-
 /*
  * Device power management
  */
Index: linux-2.6.22-rc6/arch/arm/mach-pxa/pxa25x.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/arm/mach-pxa/pxa25x.c
+++ linux-2.6.22-rc6/arch/arm/mach-pxa/pxa25x.c
@@ -19,7 +19,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
-#include <linux/pm.h>
+#include <linux/suspend.h>
 
 #include <asm/hardware.h>
 #include <asm/arch/pxa-regs.h>
Index: linux-2.6.22-rc6/arch/arm/mach-pxa/pxa27x.c
===================================================================
--- linux-2.6.22-rc6.orig/arch/arm/mach-pxa/pxa27x.c
+++ linux-2.6.22-rc6/arch/arm/mach-pxa/pxa27x.c
@@ -14,7 +14,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
-#include <linux/pm.h>
+#include <linux/suspend.h>
 #include <linux/platform_device.h>
 
 #include <asm/hardware.h>
Index: linux-2.6.22-rc6/include/asm-arm/arch-pxa/pm.h
===================================================================
--- linux-2.6.22-rc6.orig/include/asm-arm/arch-pxa/pm.h
+++ linux-2.6.22-rc6/include/asm-arm/arch-pxa/pm.h
@@ -7,6 +7,8 @@
  *
  */
 
+#include <linux/suspend.h>
+
 extern int pxa_pm_prepare(suspend_state_t state);
 extern int pxa_pm_enter(suspend_state_t state);
 extern int pxa_pm_finish(suspend_state_t state);
Index: linux-2.6.22-rc6/drivers/char/apm-emulation.c
===================================================================
--- linux-2.6.22-rc6.orig/drivers/char/apm-emulation.c
+++ linux-2.6.22-rc6/drivers/char/apm-emulation.c
@@ -18,7 +18,7 @@
 #include <linux/apm_bios.h>
 #include <linux/capability.h>
 #include <linux/sched.h>
-#include <linux/pm.h>
+#include <linux/suspend.h>
 #include <linux/apm-emulation.h>
 #include <linux/freezer.h>
 #include <linux/device.h>

Re: [RFC][PATCH -mm 3/9] PM: Move definition of struct pm_ops to suspend.h

[Pavel Machek]

On Sat 2007-06-30 23:01:55, Rafael J. Wysocki wrote:
> [Previous version of this was acked by Pavel, but I've updated it since then.]
> ---
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Move the definition of 'struct pm_ops' and related functions from <linux/pm.h>
> to <linux/suspend.h> .
> 
> There are, at least, the following reasons to do that:
> * 'struct pm_ops' is specifically related to suspend and not to the power
>   management in general.
> * As long as 'struct pm_ops' is defined in <linux/pm.h>, any modification of it
>   causes the entire kernel to be recompiled, which is unnecessary and annoying.
> * Some suspend-related features are already defined in <linux/suspend.h>, so it
>   is logical to move the definition of 'struct pm_ops' into there.
> * 'struct hibernation_ops', being the hibernation-related counterpart of
>   'struct pm_ops', is defined in <linux/suspend.h> .
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC][PATCH -mm 4/9] PM: Rename struct pm_ops and related things

[Rafael J. Wysocki]

[Previous version of this was acked by Pavel, but I've updated it since then.]
---
From: Rafael J. Wysocki <rjw@sisk.pl>

The name of 'struct pm_ops' suggests that it is related to the power management
in general, but in fact it is only related to suspend.  Moreover, its name
should indicate what this structure is used for, so it seems reasonable to
change it to 'struct platform_suspend_ops'.  In that case, the name of
the global variable of this type used by the PM core and the names of related
functions should be changed accordingly.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 Documentation/power/interface.txt        |    2 -
 arch/arm/common/sharpsl_pm.c             |    8 +++----
 arch/arm/mach-at91/pm.c                  |    4 +--
 arch/arm/mach-omap1/pm.c                 |    6 ++---
 arch/arm/mach-omap2/pm.c                 |    6 ++---
 arch/arm/mach-pnx4008/pm.c               |    4 +--
 arch/arm/mach-pxa/pm.c                   |    6 ++---
 arch/arm/mach-sa1100/pm.c                |    6 ++---
 arch/arm/plat-s3c24xx/pm.c               |    6 ++---
 arch/blackfin/mach-common/pm.c           |    4 +--
 arch/powerpc/platforms/52xx/mpc52xx_pm.c |    4 +--
 arch/sh/boards/hp6xx/pm.c                |    6 ++---
 drivers/acpi/sleep/main.c                |    6 ++---
 include/linux/suspend.h                  |   20 +++++++++---------
 kernel/power/main.c                      |   34 +++++++++++++++----------------
 15 files changed, 61 insertions(+), 61 deletions(-)

Index: linux-2.6.22-rc6-mm1/include/linux/suspend.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/linux/suspend.h	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/linux/suspend.h	2007-06-30 12:37:53.000000000 +0200
@@ -26,16 +26,16 @@ typedef int __bitwise suspend_state_t;
 #define PM_SUSPEND_MAX		((__force suspend_state_t) 4)
 
 /**
- * struct pm_ops - Callbacks for managing platform dependent system sleep
- *	states.
+ * struct platform_suspend_ops - Callbacks for managing platform dependent
+ *	system sleep states.
  *
  * @valid: Callback to determine if given system sleep state is supported by
  *	the platform.
  *	Valid (ie. supported) states are advertised in /sys/power/state.  Note
  *	that it still may be impossible to enter given system sleep state if the
  *	conditions aren't right.
- *	There is the %pm_valid_only_mem function available that can be assigned
- *	to this if the platform only supports mem sleep.
+ *	There is the %suspend_valid_only_mem function available that can be
+ *	assigned to this if the platform only supports mem sleep.
  *
  * @set_target: Tell the platform which system sleep state is going to be
  *	entered.
@@ -72,7 +72,7 @@ typedef int __bitwise suspend_state_t;
  *	that implement @prepare().  If implemented, it is always called after
  *	@enter() (even if @enter() fails).
  */
-struct pm_ops {
+struct platform_suspend_ops {
 	int (*valid)(suspend_state_t state);
 	int (*set_target)(suspend_state_t state);
 	int (*prepare)(suspend_state_t state);
@@ -80,14 +80,14 @@ struct pm_ops {
 	int (*finish)(suspend_state_t state);
 };
 
-extern struct pm_ops *pm_ops;
+extern struct platform_suspend_ops *suspend_ops;
 
 /**
- * pm_set_ops - set platform dependent power management ops
- * @pm_ops: The new power management operations to set.
+ * suspend_set_ops - set platform dependent suspend operations
+ * @ops: The new suspend operations to set.
  */
-extern void pm_set_ops(struct pm_ops *pm_ops);
-extern int pm_valid_only_mem(suspend_state_t state);
+extern void suspend_set_ops(struct platform_suspend_ops *ops);
+extern int suspend_valid_only_mem(suspend_state_t state);
 
 /**
  * arch_suspend_disable_irqs - disable IRQs for suspend
Index: linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/acpi/sleep/main.c	2007-06-30 12:37:23.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c	2007-06-30 12:37:53.000000000 +0200
@@ -21,7 +21,7 @@
 
 u8 sleep_states[ACPI_S_STATE_COUNT];
 
-static struct pm_ops acpi_pm_ops;
+static struct platform_suspend_ops acpi_pm_ops;
 
 extern void do_suspend_lowlevel(void);
 
@@ -193,7 +193,7 @@ static int acpi_pm_state_valid(suspend_s
 	}
 }
 
-static struct pm_ops acpi_pm_ops = {
+static struct platform_suspend_ops acpi_pm_ops = {
 	.valid = acpi_pm_state_valid,
 	.set_target = acpi_pm_set_target,
 	.prepare = acpi_pm_prepare,
@@ -347,7 +347,7 @@ int __init acpi_sleep_init(void)
 	}
 	printk(")\n");
 
-	pm_set_ops(&acpi_pm_ops);
+	suspend_set_ops(&acpi_pm_ops);
 
 #ifdef CONFIG_SOFTWARE_SUSPEND
 	if (sleep_states[ACPI_STATE_S4])
Index: linux-2.6.22-rc6-mm1/kernel/power/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/main.c	2007-06-30 12:36:55.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/main.c	2007-06-30 12:37:53.000000000 +0200
@@ -30,28 +30,28 @@ BLOCKING_NOTIFIER_HEAD(pm_chain_head);
 
 DEFINE_MUTEX(pm_mutex);
 
-struct pm_ops *pm_ops;
+struct platform_suspend_ops *suspend_ops;
 
 /**
- *	pm_set_ops - Set the global power method table. 
+ *	suspend_set_ops - Set the global suspend method table.
  *	@ops:	Pointer to ops structure.
  */
 
-void pm_set_ops(struct pm_ops * ops)
+void suspend_set_ops(struct platform_suspend_ops *ops)
 {
 	mutex_lock(&pm_mutex);
-	pm_ops = ops;
+	suspend_ops = ops;
 	mutex_unlock(&pm_mutex);
 }
 
 /**
- * pm_valid_only_mem - generic memory-only valid callback
+ * suspend_valid_only_mem - generic memory-only valid callback
  *
- * pm_ops drivers that implement mem suspend only and only need
+ * Platform drivers that implement mem suspend only and only need
  * to check for that in their .valid callback can use this instead
  * of rolling their own .valid callback.
  */
-int pm_valid_only_mem(suspend_state_t state)
+int suspend_valid_only_mem(suspend_state_t state)
 {
 	return state == PM_SUSPEND_MEM;
 }
@@ -59,8 +59,8 @@ int pm_valid_only_mem(suspend_state_t st
 
 static inline void pm_finish(suspend_state_t state)
 {
-	if (pm_ops->finish)
-		pm_ops->finish(state);
+	if (suspend_ops->finish)
+		suspend_ops->finish(state);
 }
 
 /**
@@ -74,7 +74,7 @@ static int suspend_prepare(void)
 	int error;
 	unsigned int free_pages;
 
-	if (!pm_ops || !pm_ops->enter)
+	if (!suspend_ops || !suspend_ops->enter)
 		return -EPERM;
 
 	error = pm_notifier_call_chain(PM_SUSPEND_PREPARE);
@@ -137,7 +137,7 @@ int suspend_enter(suspend_state_t state)
 		printk(KERN_ERR "Some devices failed to power down\n");
 		goto Done;
 	}
-	error = pm_ops->enter(state);
+	error = suspend_ops->enter(state);
 	device_power_up();
  Done:
 	arch_suspend_enable_irqs();
@@ -154,11 +154,11 @@ int suspend_devices_and_enter(suspend_st
 {
 	int error;
 
-	if (!pm_ops)
+	if (!suspend_ops)
 		return -ENOSYS;
 
-	if (pm_ops->set_target) {
-		error = pm_ops->set_target(state);
+	if (suspend_ops->set_target) {
+		error = suspend_ops->set_target(state);
 		if (error)
 			return error;
 	}
@@ -168,8 +168,8 @@ int suspend_devices_and_enter(suspend_st
 		printk(KERN_ERR "Some devices failed to suspend\n");
 		goto Resume_console;
 	}
-	if (pm_ops->prepare) {
-		error = pm_ops->prepare(state);
+	if (suspend_ops->prepare) {
+		error = suspend_ops->prepare(state);
 		if (error)
 			goto Resume_devices;
 	}
@@ -212,7 +212,7 @@ static inline int valid_state(suspend_st
 	/* All states need lowlevel support and need to be valid
 	 * to the lowlevel implementation, no valid callback
 	 * implies that none are valid. */
-	if (!pm_ops || !pm_ops->valid || !pm_ops->valid(state))
+	if (!suspend_ops || !suspend_ops->valid || !suspend_ops->valid(state))
 		return 0;
 	return 1;
 }
Index: linux-2.6.22-rc6-mm1/Documentation/power/interface.txt
===================================================================
--- linux-2.6.22-rc6-mm1.orig/Documentation/power/interface.txt	2007-06-28 21:56:02.000000000 +0200
+++ linux-2.6.22-rc6-mm1/Documentation/power/interface.txt	2007-06-30 12:37:53.000000000 +0200
@@ -20,7 +20,7 @@ states.
 /sys/power/disk controls the operating mode of the suspend-to-disk
 mechanism. Suspend-to-disk can be handled in several ways. We have a
 few options for putting the system to sleep - using the platform driver
-(e.g. ACPI or other pm_ops), powering off the system or rebooting the
+(e.g. ACPI or other suspend_ops), powering off the system or rebooting the
 system (for testing).
 
 Additionally, /sys/power/disk can be used to turn on one of the two testing
Index: linux-2.6.22-rc6-mm1/arch/arm/common/sharpsl_pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/common/sharpsl_pm.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/common/sharpsl_pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -766,11 +766,11 @@ static void sharpsl_apm_get_power_status
 	info->battery_life = sharpsl_pm.battstat.mainbat_percent;
 }
 
-static struct pm_ops sharpsl_pm_ops = {
+static struct platform_suspend_ops sharpsl_pm_ops = {
 	.prepare	= pxa_pm_prepare,
 	.enter		= corgi_pxa_pm_enter,
 	.finish		= pxa_pm_finish,
-	.valid		= pm_valid_only_mem,
+	.valid		= suspend_valid_only_mem,
 };
 
 static int __init sharpsl_pm_probe(struct platform_device *pdev)
@@ -802,7 +802,7 @@ static int __init sharpsl_pm_probe(struc
 
 	apm_get_power_status = sharpsl_apm_get_power_status;
 
-	pm_set_ops(&sharpsl_pm_ops);
+	suspend_set_ops(&sharpsl_pm_ops);
 
 	mod_timer(&sharpsl_pm.ac_timer, jiffies + msecs_to_jiffies(250));
 
@@ -811,7 +811,7 @@ static int __init sharpsl_pm_probe(struc
 
 static int sharpsl_pm_remove(struct platform_device *pdev)
 {
-	pm_set_ops(NULL);
+	suspend_set_ops(NULL);
 
 	device_remove_file(&pdev->dev, &dev_attr_battery_percentage);
 	device_remove_file(&pdev->dev, &dev_attr_battery_voltage);
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-at91/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-at91/pm.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-at91/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -198,7 +198,7 @@ error:
 }
 
 
-static struct pm_ops at91_pm_ops ={
+static struct platform_suspend_ops at91_pm_ops ={
 	.valid		= at91_pm_valid_state,
 	.set_target	= at91_pm_set_target,
 	.enter		= at91_pm_enter,
@@ -219,7 +219,7 @@ static int __init at91_pm_init(void)
 	/* Disable SDRAM low-power mode.  Cannot be used with self-refresh. */
 	at91_sys_write(AT91_SDRAMC_LPR, 0);
 
-	pm_set_ops(&at91_pm_ops);
+	suspend_set_ops(&at91_pm_ops);
 
 	return 0;
 }
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-omap1/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-omap1/pm.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-omap1/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -687,11 +687,11 @@ static struct irqaction omap_wakeup_irq 
 
 
 
-static struct pm_ops omap_pm_ops ={
+static struct platform_suspend_ops omap_pm_ops ={
 	.prepare	= omap_pm_prepare,
 	.enter		= omap_pm_enter,
 	.finish		= omap_pm_finish,
-	.valid		= pm_valid_only_mem,
+	.valid		= suspend_valid_only_mem,
 };
 
 static int __init omap_pm_init(void)
@@ -748,7 +748,7 @@ static int __init omap_pm_init(void)
 	else if (cpu_is_omap16xx())
 		omap_writel(OMAP1610_IDLECT3_VAL, OMAP1610_IDLECT3);
 
-	pm_set_ops(&omap_pm_ops);
+	suspend_set_ops(&omap_pm_ops);
 
 #if defined(DEBUG) && defined(CONFIG_PROC_FS)
 	omap_pm_init_proc();
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-omap2/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-omap2/pm.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-omap2/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -362,11 +362,11 @@ static int omap2_pm_finish(suspend_state
 	return 0;
 }
 
-static struct pm_ops omap_pm_ops = {
+static struct platform_suspend_ops omap_pm_ops = {
 	.prepare	= omap2_pm_prepare,
 	.enter		= omap2_pm_enter,
 	.finish		= omap2_pm_finish,
-	.valid		= pm_valid_only_mem,
+	.valid		= suspend_valid_only_mem,
 };
 
 int __init omap2_pm_init(void)
@@ -390,7 +390,7 @@ int __init omap2_pm_init(void)
 	omap2_sram_suspend = omap_sram_push(omap24xx_cpu_suspend,
 					    omap24xx_cpu_suspend_sz);
 
-	pm_set_ops(&omap_pm_ops);
+	suspend_set_ops(&omap_pm_ops);
 	pm_idle = omap2_pm_idle;
 
 	pmdomain_init();
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-pnx4008/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-pnx4008/pm.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-pnx4008/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -117,7 +117,7 @@ static int pnx4008_pm_valid(suspend_stat
 	       (state == PM_SUSPEND_MEM);
 }
 
-static struct pm_ops pnx4008_pm_ops = {
+static struct platform_suspend_ops pnx4008_pm_ops = {
 	.enter = pnx4008_pm_enter,
 	.valid = pnx4008_pm_valid,
 };
@@ -146,7 +146,7 @@ static int __init pnx4008_pm_init(void)
 		return -ENOMEM;
 	}
 
-	pm_set_ops(&pnx4008_pm_ops);
+	suspend_set_ops(&pnx4008_pm_ops);
 	return 0;
 }
 
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-pxa/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-pxa/pm.c	2007-06-28 21:56:03.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-pxa/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -223,16 +223,16 @@ int pxa_pm_finish(suspend_state_t state)
 
 EXPORT_SYMBOL_GPL(pxa_pm_finish);
 
-static struct pm_ops pxa_pm_ops = {
+static struct platform_suspend_ops pxa_pm_ops = {
 	.prepare	= pxa_pm_prepare,
 	.enter		= pxa_pm_enter,
 	.finish		= pxa_pm_finish,
-	.valid		= pm_valid_only_mem,
+	.valid		= suspend_valid_only_mem,
 };
 
 static int __init pxa_pm_init(void)
 {
-	pm_set_ops(&pxa_pm_ops);
+	suspend_set_ops(&pxa_pm_ops);
 	return 0;
 }
 
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-sa1100/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-sa1100/pm.c	2007-06-28 21:56:03.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-sa1100/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -131,14 +131,14 @@ unsigned long sleep_phys_sp(void *sp)
 	return virt_to_phys(sp);
 }
 
-static struct pm_ops sa11x0_pm_ops = {
+static struct platform_suspend_ops sa11x0_pm_ops = {
 	.enter		= sa11x0_pm_enter,
-	.valid		= pm_valid_only_mem,
+	.valid		= suspend_valid_only_mem,
 };
 
 static int __init sa11x0_pm_init(void)
 {
-	pm_set_ops(&sa11x0_pm_ops);
+	suspend_set_ops(&sa11x0_pm_ops);
 	return 0;
 }
 
Index: linux-2.6.22-rc6-mm1/arch/arm/plat-s3c24xx/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/plat-s3c24xx/pm.c	2007-06-28 21:56:03.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/plat-s3c24xx/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -612,9 +612,9 @@ static int s3c2410_pm_enter(suspend_stat
 	return 0;
 }
 
-static struct pm_ops s3c2410_pm_ops = {
+static struct platform_suspend_ops s3c2410_pm_ops = {
 	.enter		= s3c2410_pm_enter,
-	.valid		= pm_valid_only_mem,
+	.valid		= suspend_valid_only_mem,
 };
 
 /* s3c2410_pm_init
@@ -628,6 +628,6 @@ int __init s3c2410_pm_init(void)
 {
 	printk("S3C2410 Power Management, (c) 2004 Simtec Electronics\n");
 
-	pm_set_ops(&s3c2410_pm_ops);
+	suspend_set_ops(&s3c2410_pm_ops);
 	return 0;
 }
Index: linux-2.6.22-rc6-mm1/arch/blackfin/mach-common/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/blackfin/mach-common/pm.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/blackfin/mach-common/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -158,7 +158,7 @@ static int bfin_pm_finish(suspend_state_
 	return 0;
 }
 
-struct pm_ops bfin_pm_ops = {
+struct platform_suspend_ops bfin_pm_ops = {
 	.prepare = bfin_pm_prepare,
 	.enter = bfin_pm_enter,
 	.finish = bfin_pm_finish,
@@ -166,7 +166,7 @@ struct pm_ops bfin_pm_ops = {
 
 static int __init bfin_pm_init(void)
 {
-	pm_set_ops(&bfin_pm_ops);
+	suspend_set_ops(&bfin_pm_ops);
 	return 0;
 }
 
Index: linux-2.6.22-rc6-mm1/arch/powerpc/platforms/52xx/mpc52xx_pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/powerpc/platforms/52xx/mpc52xx_pm.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/powerpc/platforms/52xx/mpc52xx_pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -177,7 +177,7 @@ int mpc52xx_pm_finish(suspend_state_t st
 	return 0;
 }
 
-static struct pm_ops mpc52xx_pm_ops = {
+static struct platform_suspend_ops mpc52xx_pm_ops = {
 	.valid		= mpc52xx_pm_valid,
 	.prepare	= mpc52xx_pm_prepare,
 	.enter		= mpc52xx_pm_enter,
@@ -186,6 +186,6 @@ static struct pm_ops mpc52xx_pm_ops = {
 
 int __init mpc52xx_pm_init(void)
 {
-	pm_set_ops(&mpc52xx_pm_ops);
+	suspend_set_ops(&mpc52xx_pm_ops);
 	return 0;
 }
Index: linux-2.6.22-rc6-mm1/arch/sh/boards/hp6xx/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/sh/boards/hp6xx/pm.c	2007-06-28 21:56:07.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/sh/boards/hp6xx/pm.c	2007-06-30 12:37:53.000000000 +0200
@@ -67,14 +67,14 @@ static int hp6x0_pm_enter(suspend_state_
 	return 0;
 }
 
-static struct pm_ops hp6x0_pm_ops = {
+static struct platform_suspend_ops hp6x0_pm_ops = {
 	.enter		= hp6x0_pm_enter,
-	.valid		= pm_valid_only_mem,
+	.valid		= suspend_valid_only_mem,
 };
 
 static int __init hp6x0_pm_init(void)
 {
-	pm_set_ops(&hp6x0_pm_ops);
+	suspend_set_ops(&hp6x0_pm_ops);
 	return 0;
 }
 

Re: [RFC][PATCH -mm 4/9] PM: Rename struct pm_ops and related things

[Pavel Machek]

On Sat 2007-06-30 23:03:57, Rafael J. Wysocki wrote:
> [Previous version of this was acked by Pavel, but I've updated it since then.]
> ---
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The name of 'struct pm_ops' suggests that it is related to the power management
> in general, but in fact it is only related to suspend.  Moreover, its name
> should indicate what this structure is used for, so it seems reasonable to
> change it to 'struct platform_suspend_ops'.  In that case, the name of
> the global variable of this type used by the PM core and the names of related
> functions should be changed accordingly.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC][PATCH -mm 5/9] PM: Rework struct platform_suspend_ops

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

There is no reason why the .prepare() and .finish() methods in 'struct
platform_suspend_ops' should take any arguments, since architectures
don't use these methods' argument in any practically meaningful way (ie. either
the target system sleep state is conveyed to the platform by .set_target(), or
there is only one suspend state supported and it is indicated to the PM core by
.valid(), or .prepare() and .finish() aren't defined at all).  There also is
no reason why .finish() should return any result.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 arch/arm/common/sharpsl_pm.c             |    2 -
 arch/arm/mach-omap1/pm.c                 |   20 +-----------
 arch/arm/mach-omap2/pm.c                 |   20 +-----------
 arch/arm/mach-pxa/pm.c                   |   24 ---------------
 arch/arm/mach-pxa/pxa25x.c               |   12 -------
 arch/arm/mach-pxa/pxa27x.c               |   11 ------
 arch/blackfin/mach-common/pm.c           |   49 +++----------------------------
 arch/powerpc/platforms/52xx/mpc52xx_pm.c |    9 +----
 drivers/acpi/sleep/main.c                |    7 +---
 include/asm-arm/arch-pxa/pm.h            |    2 -
 include/linux/suspend.h                  |   13 +++-----
 kernel/power/main.c                      |   12 +------
 12 files changed, 24 insertions(+), 157 deletions(-)

Index: linux-2.6.22-rc6-mm1/include/linux/suspend.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/linux/suspend.h	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/linux/suspend.h	2007-06-30 12:38:10.000000000 +0200
@@ -45,12 +45,10 @@ typedef int __bitwise suspend_state_t;
  *	@prepare() fails.  If @set_target() fails (ie. returns nonzero),
  *	@prepare(), @enter() and @finish() will not be called by the PM core.
  *	This callback is optional.  However, if it is implemented, the argument
- *	passed to @prepare(), @enter() and @finish() is meaningless and should
- *	be ignored.
+ *	passed to @enter() is meaningless and should be ignored.
  *
  * @prepare: Prepare the platform for entering the system sleep state indicated
- *	by @set_target() or represented by the argument if @set_target() is not
- *	implemented.
+ *	by @set_target().
  *	@prepare() is called right after devices have been suspended (ie. the
  *	appropriate .suspend() method has been executed for each device) and
  *	before the nonboot CPUs are disabled (it is executed with IRQs enabled).
@@ -66,8 +64,7 @@ typedef int __bitwise suspend_state_t;
  *
  * @finish: Called when the system has just left a sleep state, right after
  *	the nonboot CPUs have been enabled and before devices are resumed (it is
- *	executed with IRQs enabled).  If @set_target() is not implemented, the
- *	argument represents the sleep state being left.
+ *	executed with IRQs enabled).
  *	This callback is optional, but should be implemented by the platforms
  *	that implement @prepare().  If implemented, it is always called after
  *	@enter() (even if @enter() fails).
@@ -75,9 +72,9 @@ typedef int __bitwise suspend_state_t;
 struct platform_suspend_ops {
 	int (*valid)(suspend_state_t state);
 	int (*set_target)(suspend_state_t state);
-	int (*prepare)(suspend_state_t state);
+	int (*prepare)(void);
 	int (*enter)(suspend_state_t state);
-	int (*finish)(suspend_state_t state);
+	void (*finish)(void);
 };
 
 extern struct platform_suspend_ops *suspend_ops;
Index: linux-2.6.22-rc6-mm1/kernel/power/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/main.c	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/main.c	2007-06-30 12:38:10.000000000 +0200
@@ -56,13 +56,6 @@ int suspend_valid_only_mem(suspend_state
 	return state == PM_SUSPEND_MEM;
 }
 
-
-static inline void pm_finish(suspend_state_t state)
-{
-	if (suspend_ops->finish)
-		suspend_ops->finish(state);
-}
-
 /**
  *	suspend_prepare - Do prep work before entering low-power state.
  *
@@ -169,7 +162,7 @@ int suspend_devices_and_enter(suspend_st
 		goto Resume_console;
 	}
 	if (suspend_ops->prepare) {
-		error = suspend_ops->prepare(state);
+		error = suspend_ops->prepare();
 		if (error)
 			goto Resume_devices;
 	}
@@ -178,7 +171,8 @@ int suspend_devices_and_enter(suspend_st
 		suspend_enter(state);
 
 	enable_nonboot_cpus();
-	pm_finish(state);
+	if (suspend_ops->finish)
+		suspend_ops->finish();
  Resume_devices:
 	device_resume();
  Resume_console:
Index: linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/acpi/sleep/main.c	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c	2007-06-30 12:38:10.000000000 +0200
@@ -61,13 +61,12 @@ static int acpi_pm_set_target(suspend_st
 
 /**
  *	acpi_pm_prepare - Do preliminary suspend work.
- *	@pm_state: ignored
  *
  *	If necessary, set the firmware waking vector and do arch-specific
  *	nastiness to get the wakeup code to the waking vector.
  */
 
-static int acpi_pm_prepare(suspend_state_t pm_state)
+static int acpi_pm_prepare(void)
 {
 	int error = acpi_sleep_prepare(acpi_target_sleep_state);
 
@@ -137,13 +136,12 @@ static int acpi_pm_enter(suspend_state_t
 
 /**
  *	acpi_pm_finish - Finish up suspend sequence.
- *	@pm_state: ignored
  *
  *	This is called after we wake back up (or if entering the sleep state
  *	failed). 
  */
 
-static int acpi_pm_finish(suspend_state_t pm_state)
+static void acpi_pm_finish(void)
 {
 	u32 acpi_state = acpi_target_sleep_state;
 
@@ -159,7 +157,6 @@ static int acpi_pm_finish(suspend_state_
 		printk("Broken toshiba laptop -> kicking interrupts\n");
 		init_8259A(0);
 	}
-	return 0;
 }
 
 int acpi_suspend(u32 acpi_state)
Index: linux-2.6.22-rc6-mm1/arch/arm/common/sharpsl_pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/common/sharpsl_pm.c	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/common/sharpsl_pm.c	2007-06-30 12:38:10.000000000 +0200
@@ -767,9 +767,7 @@ static void sharpsl_apm_get_power_status
 }
 
 static struct platform_suspend_ops sharpsl_pm_ops = {
-	.prepare	= pxa_pm_prepare,
 	.enter		= corgi_pxa_pm_enter,
-	.finish		= pxa_pm_finish,
 	.valid		= suspend_valid_only_mem,
 };
 
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-pxa/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-pxa/pm.c	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-pxa/pm.c	2007-06-30 12:38:10.000000000 +0200
@@ -201,32 +201,8 @@ unsigned long sleep_phys_sp(void *sp)
 	return virt_to_phys(sp);
 }
 
-/*
- * Called after processes are frozen, but before we shut down devices.
- */
-int pxa_pm_prepare(suspend_state_t state)
-{
-	extern int pxa_cpu_pm_prepare(suspend_state_t state);
-
-	return pxa_cpu_pm_prepare(state);
-}
-
-EXPORT_SYMBOL_GPL(pxa_pm_prepare);
-
-/*
- * Called after devices are re-setup, but before processes are thawed.
- */
-int pxa_pm_finish(suspend_state_t state)
-{
-	return 0;
-}
-
-EXPORT_SYMBOL_GPL(pxa_pm_finish);
-
 static struct platform_suspend_ops pxa_pm_ops = {
-	.prepare	= pxa_pm_prepare,
 	.enter		= pxa_pm_enter,
-	.finish		= pxa_pm_finish,
 	.valid		= suspend_valid_only_mem,
 };
 
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-pxa/pxa25x.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-pxa/pxa25x.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-pxa/pxa25x.c	2007-06-30 12:38:10.000000000 +0200
@@ -105,18 +105,6 @@ EXPORT_SYMBOL(get_lcdclk_frequency_10khz
 
 #ifdef CONFIG_PM
 
-int pxa_cpu_pm_prepare(suspend_state_t state)
-{
-	switch (state) {
-	case PM_SUSPEND_MEM:
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 void pxa_cpu_pm_enter(suspend_state_t state)
 {
 	extern void pxa_cpu_suspend(unsigned int);
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-pxa/pxa27x.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-pxa/pxa27x.c	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-pxa/pxa27x.c	2007-06-30 12:38:10.000000000 +0200
@@ -122,17 +122,6 @@ EXPORT_SYMBOL(get_lcdclk_frequency_10khz
 
 #ifdef CONFIG_PM
 
-int pxa_cpu_pm_prepare(suspend_state_t state)
-{
-	switch (state) {
-	case PM_SUSPEND_MEM:
-	case PM_SUSPEND_STANDBY:
-		return 0;
-	default:
-		return -EINVAL;
-	}
-}
-
 void pxa_cpu_pm_enter(suspend_state_t state)
 {
 	extern void pxa_cpu_standby(void);
Index: linux-2.6.22-rc6-mm1/include/asm-arm/arch-pxa/pm.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/asm-arm/arch-pxa/pm.h	2007-06-30 12:37:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/asm-arm/arch-pxa/pm.h	2007-06-30 12:38:10.000000000 +0200
@@ -9,6 +9,4 @@
 
 #include <linux/suspend.h>
 
-extern int pxa_pm_prepare(suspend_state_t state);
 extern int pxa_pm_enter(suspend_state_t state);
-extern int pxa_pm_finish(suspend_state_t state);
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-omap1/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-omap1/pm.c	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-omap1/pm.c	2007-06-30 12:38:10.000000000 +0200
@@ -613,27 +613,15 @@ static void (*saved_idle)(void) = NULL;
 
 /*
  *	omap_pm_prepare - Do preliminary suspend work.
- *	@state:		suspend state we're entering.
  *
  */
-static int omap_pm_prepare(suspend_state_t state)
+static int omap_pm_prepare(void)
 {
-	int error = 0;
-
 	/* We cannot sleep in idle until we have resumed */
 	saved_idle = pm_idle;
 	pm_idle = NULL;
 
-	switch (state)
-	{
-	case PM_SUSPEND_STANDBY:
-	case PM_SUSPEND_MEM:
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return error;
+	return 0;
 }
 
 
@@ -661,16 +649,14 @@ static int omap_pm_enter(suspend_state_t
 
 /**
  *	omap_pm_finish - Finish up suspend sequence.
- *	@state:		State we're coming out of.
  *
  *	This is called after we wake back up (or if entering the sleep state
  *	failed).
  */
 
-static int omap_pm_finish(suspend_state_t state)
+static void omap_pm_finish(void)
 {
 	pm_idle = saved_idle;
-	return 0;
 }
 
 
Index: linux-2.6.22-rc6-mm1/arch/arm/mach-omap2/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/arm/mach-omap2/pm.c	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/arm/mach-omap2/pm.c	2007-06-30 12:38:10.000000000 +0200
@@ -70,25 +70,12 @@ void omap2_pm_idle(void)
 	local_irq_enable();
 }
 
-static int omap2_pm_prepare(suspend_state_t state)
+static int omap2_pm_prepare(void)
 {
-	int error = 0;
-
 	/* We cannot sleep in idle until we have resumed */
 	saved_idle = pm_idle;
 	pm_idle = NULL;
-
-	switch (state)
-	{
-	case PM_SUSPEND_STANDBY:
-	case PM_SUSPEND_MEM:
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
-	return error;
+	return 0;
 }
 
 #define INT0_WAKE_MASK	(OMAP_IRQ_BIT(INT_24XX_GPIO_BANK1) |	\
@@ -356,10 +343,9 @@ static int omap2_pm_enter(suspend_state_
 	return ret;
 }
 
-static int omap2_pm_finish(suspend_state_t state)
+static void omap2_pm_finish(void)
 {
 	pm_idle = saved_idle;
-	return 0;
 }
 
 static struct platform_suspend_ops omap_pm_ops = {
Index: linux-2.6.22-rc6-mm1/arch/blackfin/mach-common/pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/blackfin/mach-common/pm.c	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/blackfin/mach-common/pm.c	2007-06-30 12:38:10.000000000 +0200
@@ -89,28 +89,15 @@ void bfin_pm_suspend_standby_enter(void)
 #endif				/* CONFIG_PM_WAKEUP_GPIO_BY_SIC_IWR */
 }
 
-
 /*
- *	bfin_pm_prepare - Do preliminary suspend work.
- *	@state:		suspend state we're entering.
+ *	bfin_pm_valid - Tell the PM core that we only support the standby sleep
+ *			state
+ *	@state:		suspend state we're checking.
  *
  */
-static int bfin_pm_prepare(suspend_state_t state)
+static int bfin_pm_valid(suspend_state_t state)
 {
-	int error = 0;
-
-	switch (state) {
-	case PM_SUSPEND_STANDBY:
-		break;
-
-	case PM_SUSPEND_MEM:
-		return -ENOTSUPP;
-
-	default:
-		return -EINVAL;
-	}
-
-	return error;
+	return (state == PM_SUSPEND_STANDBY);
 }
 
 /*
@@ -135,33 +122,9 @@ static int bfin_pm_enter(suspend_state_t
 	return 0;
 }
 
-/*
- *	bfin_pm_finish - Finish up suspend sequence.
- *	@state:		State we're coming out of.
- *
- *	This is called after we wake back up (or if entering the sleep state
- *	failed).
- */
-static int bfin_pm_finish(suspend_state_t state)
-{
-	switch (state) {
-	case PM_SUSPEND_STANDBY:
-		break;
-
-	case PM_SUSPEND_MEM:
-		return -ENOTSUPP;
-
-	default:
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 struct platform_suspend_ops bfin_pm_ops = {
-	.prepare = bfin_pm_prepare,
+	.valid = bfin_pm_valid,
 	.enter = bfin_pm_enter,
-	.finish = bfin_pm_finish,
 };
 
 static int __init bfin_pm_init(void)
Index: linux-2.6.22-rc6-mm1/arch/powerpc/platforms/52xx/mpc52xx_pm.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/arch/powerpc/platforms/52xx/mpc52xx_pm.c	2007-06-30 12:37:53.000000000 +0200
+++ linux-2.6.22-rc6-mm1/arch/powerpc/platforms/52xx/mpc52xx_pm.c	2007-06-30 12:38:10.000000000 +0200
@@ -57,11 +57,8 @@ int mpc52xx_set_wakeup_gpio(u8 pin, u8 l
 	return 0;
 }
 
-int mpc52xx_pm_prepare(suspend_state_t state)
+static int mpc52xx_pm_prepare(void)
 {
-	if (state != PM_SUSPEND_STANDBY)
-		return -EINVAL;
-
 	/* map the whole register space */
 	mbar = mpc52xx_find_and_map("mpc5200");
 	if (!mbar) {
@@ -166,15 +163,13 @@ int mpc52xx_pm_enter(suspend_state_t sta
 	return 0;
 }
 
-int mpc52xx_pm_finish(suspend_state_t state)
+static void mpc52xx_pm_finish(void)
 {
 	/* call board resume code */
 	if (mpc52xx_suspend.board_resume_finish)
 		mpc52xx_suspend.board_resume_finish(mbar);
 
 	iounmap(mbar);
-
-	return 0;
 }
 
 static struct platform_suspend_ops mpc52xx_pm_ops = {

[RFC][PATCH -mm 6/9] PM: Fix compilation of suspend code if CONFIG_PM is unset

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Some platforms, particualrly in the ARM tree, require that suspend_set_ops() and
pm_suspend() be defined even if CONFIG_PM is not set.  Make this requirement be
satisfied.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 include/linux/suspend.h |    7 +++++++
 1 file changed, 7 insertions(+)

Index: linux-2.6.22-rc6/include/linux/suspend.h
===================================================================
--- linux-2.6.22-rc6.orig/include/linux/suspend.h	2007-06-29 22:42:55.000000000 +0200
+++ linux-2.6.22-rc6/include/linux/suspend.h	2007-06-29 22:43:37.000000000 +0200
@@ -79,6 +79,7 @@ struct platform_suspend_ops {
 
 extern struct platform_suspend_ops *suspend_ops;
 
+#ifdef CONFIG_PM
 /**
  * suspend_set_ops - set platform dependent suspend operations
  * @ops: The new suspend operations to set.
@@ -105,6 +106,12 @@ extern void arch_suspend_disable_irqs(vo
 extern void arch_suspend_enable_irqs(void);
 
 extern int pm_suspend(suspend_state_t state);
+#else /* CONFIG_PM */
+#define suspend_valid_only_mem	NULL
+
+static inline void suspend_set_ops(struct platform_suspend_ops *ops) {}
+static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; }
+#endif /* CONFIG_PM */
 
 /* struct pbe is used for creating lists of pages that should be restored
  * atomically during the resume from disk, because the page frames they have

[RFC][PATCH -mm 7/9] PM: Make suspend_ops static

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The variable suspend_ops representing the set of global platform-specific
suspend-related operations, used by the PM core, need not be exported outside of
kernel/power/main.c .  Make it static.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 include/linux/suspend.h |    2 --
 kernel/power/main.c     |    2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

Index: linux-2.6.22-rc6-mm1/include/linux/suspend.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/linux/suspend.h	2007-06-30 21:21:15.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/linux/suspend.h	2007-06-30 21:21:33.000000000 +0200
@@ -77,8 +77,6 @@ struct platform_suspend_ops {
 	void (*finish)(void);
 };
 
-extern struct platform_suspend_ops *suspend_ops;
-
 #ifdef CONFIG_PM
 /**
  * suspend_set_ops - set platform dependent suspend operations
Index: linux-2.6.22-rc6-mm1/kernel/power/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/main.c	2007-06-30 21:21:01.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/main.c	2007-06-30 21:21:33.000000000 +0200
@@ -30,7 +30,7 @@ BLOCKING_NOTIFIER_HEAD(pm_chain_head);
 
 DEFINE_MUTEX(pm_mutex);
 
-struct platform_suspend_ops *suspend_ops;
+static struct platform_suspend_ops *suspend_ops;
 
 /**
  *	suspend_set_ops - Set the global suspend method table.

Re: [RFC][PATCH -mm 7/9] PM: Make suspend_ops static

[Pavel Machek]

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The variable suspend_ops representing the set of global platform-specific
> suspend-related operations, used by the PM core, need not be exported outside of
> kernel/power/main.c .  Make it static.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[RFC][PATCH -mm 8/9] PM: Rework struct hibernation_ops

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

During hibernation we also need to tell the ACPI core that we're going to put
the system into the S4 sleep state.  For this reason, an additional method
in 'struct hibernation_ops' is needed, playing the role of set_target() in
'struct platform_suspend_operations'.  Moreover, the role of the .prepare()
method is now different, so it's better to introduce another method, that in
general may be different from .prepare(), that will be used to prepare the
platform for creating the hibernation image (.prepare() is used anyway to
notify the platform that we're going to enter the low power state after the
image has been saved).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep/main.c |   12 +++++++++++-
 include/linux/suspend.h   |   38 ++++++++++++++++++++++++++++++++------
 kernel/power/disk.c       |   27 +++++++++++++++++++++------
 3 files changed, 64 insertions(+), 13 deletions(-)

Index: linux-2.6.22-rc6-mm1/include/linux/suspend.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/linux/suspend.h	2007-06-30 21:14:38.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/linux/suspend.h	2007-06-30 21:14:44.000000000 +0200
@@ -133,16 +133,42 @@ extern void mark_free_pages(struct zone 
  *
  * All three methods must be assigned.
  *
- * @prepare: prepare system for hibernation
- * @enter: shut down system after state has been saved to disk
- * @finish: finish/clean up after state has been reloaded
- * @pre_restore: prepare system for the restoration from a hibernation image
- * @restore_cleanup: clean up after a failing image restoration
+ * @start: Tell the platform driver that we're starting hibernation.
+ *	Called right after shrinking memory and before freezing devices.
+ *
+ * @pre_snapshot: Prepare the platform for creating the hibernation image.
+ *	Called right after devices have been frozen and before the nonboot
+ *	CPUs are disabled (runs with IRQs on).
+ *
+ * @finish: Restore the previous state of the platform after the hibernation
+ *	image has been created *or* put the platform into the normal operation
+ *	mode after the hibernation (the same method is executed in both cases).
+ *	Called right after the nonboot CPUs have been enabled and before
+ *	thawing devices (runs with IRQs on).
+ *
+ * @prepare: Prepare the platform for entering the low power state.
+ *	Called right after the hibernation image has been saved and before
+ *	devices are prepared for entering the low power state.
+ *
+ * @enter: Put the system into the low power state after the hibernation image
+ *	has been saved to disk.
+ *	Called after the nonboot CPUs have been disabled and all of the low
+ *	level devices have been shut down (runs with IRQs off).
+ *
+ * @pre_restore: Prepare system for the restoration from a hibernation image.
+ *	Called right after devices have been frozen and before the nonboot
+ *	CPUs are disabled (runs with IRQs on).
+ *
+ * @restore_cleanup: Clean up after a failing image restoration.
+ *	Called right after the nonboot CPUs have been enabled and before
+ *	thawing devices (runs with IRQs on).
  */
 struct hibernation_ops {
+	int (*start)(void);
+	int (*pre_snapshot)(void);
+	void (*finish)(void);
 	int (*prepare)(void);
 	int (*enter)(void);
-	void (*finish)(void);
 	int (*pre_restore)(void);
 	void (*restore_cleanup)(void);
 };
Index: linux-2.6.22-rc6-mm1/kernel/power/disk.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/disk.c	2007-06-30 21:14:23.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/disk.c	2007-06-30 21:14:44.000000000 +0200
@@ -54,8 +54,9 @@ static struct hibernation_ops *hibernati
 
 void hibernation_set_ops(struct hibernation_ops *ops)
 {
-	if (ops && !(ops->prepare && ops->enter && ops->finish
-	    && ops->pre_restore && ops->restore_cleanup)) {
+	if (ops && !(ops->start && ops->pre_snapshot && ops->finish
+	    && ops->prepare && ops->enter && ops->pre_restore
+	    && ops->restore_cleanup)) {
 		WARN_ON(1);
 		return;
 	}
@@ -69,16 +70,26 @@ void hibernation_set_ops(struct hibernat
 	mutex_unlock(&pm_mutex);
 }
 
+/**
+ *	platform_start - tell the platform driver that we're starting
+ *	hibernation
+ */
+
+static int platform_start(int platform_mode)
+{
+	return (platform_mode && hibernation_ops) ?
+		hibernation_ops->start() : 0;
+}
 
 /**
- *	platform_prepare - prepare the machine for hibernation using the
+ *	platform_pre_snapshot - prepare the machine for hibernation using the
  *	platform driver if so configured and return an error code if it fails
  */
 
-static int platform_prepare(int platform_mode)
+static int platform_pre_snapshot(int platform_mode)
 {
 	return (platform_mode && hibernation_ops) ?
-		hibernation_ops->prepare() : 0;
+		hibernation_ops->pre_snapshot() : 0;
 }
 
 /**
@@ -135,12 +146,16 @@ int hibernation_snapshot(int platform_mo
 	if (error)
 		return error;
 
+	error = platform_start(platform_mode);
+	if (error)
+		return error;
+
 	suspend_console();
 	error = device_suspend(PMSG_FREEZE);
 	if (error)
 		goto Resume_console;
 
-	error = platform_prepare(platform_mode);
+	error = platform_pre_snapshot(platform_mode);
 	if (error)
 		goto Resume_devices;
 
Index: linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/acpi/sleep/main.c	2007-06-30 21:14:23.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c	2007-06-30 21:14:44.000000000 +0200
@@ -199,6 +199,12 @@ static struct platform_suspend_ops acpi_
 };
 
 #ifdef CONFIG_SOFTWARE_SUSPEND
+static int acpi_hibernation_start(void)
+{
+	acpi_target_sleep_state = ACPI_STATE_S4;
+	return 0;
+}
+
 static int acpi_hibernation_prepare(void)
 {
 	return acpi_sleep_prepare(ACPI_STATE_S4);
@@ -227,6 +233,8 @@ static void acpi_hibernation_finish(void
 
 	/* reset firmware waking vector */
 	acpi_set_firmware_waking_vector((acpi_physical_address) 0);
+
+	acpi_target_sleep_state = ACPI_STATE_S0;
 }
 
 static int acpi_hibernation_pre_restore(void)
@@ -244,9 +252,11 @@ static void acpi_hibernation_restore_cle
 }
 
 static struct hibernation_ops acpi_hibernation_ops = {
+	.start = acpi_hibernation_start,
+	.pre_snapshot = acpi_hibernation_prepare,
+	.finish = acpi_hibernation_finish,
 	.prepare = acpi_hibernation_prepare,
 	.enter = acpi_hibernation_enter,
-	.finish = acpi_hibernation_finish,
 	.pre_restore = acpi_hibernation_pre_restore,
 	.restore_cleanup = acpi_hibernation_restore_cleanup,
 };

[RFC][PATCH -mm 9/9] PM: Rename hibernation_ops to platform_hibernation_ops

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Rename 'struct hibernation_ops' to 'struct platform_hibernation_ops' in analogy
with 'struct platform_suspend_ops'.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep/main.c |    2 +-
 include/linux/suspend.h   |    8 ++++----
 kernel/power/disk.c       |    4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

Index: linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/acpi/sleep/main.c	2007-06-30 21:14:44.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/acpi/sleep/main.c	2007-06-30 21:14:54.000000000 +0200
@@ -251,7 +251,7 @@ static void acpi_hibernation_restore_cle
 	acpi_hw_enable_all_runtime_gpes();
 }
 
-static struct hibernation_ops acpi_hibernation_ops = {
+static struct platform_hibernation_ops acpi_hibernation_ops = {
 	.start = acpi_hibernation_start,
 	.pre_snapshot = acpi_hibernation_prepare,
 	.finish = acpi_hibernation_finish,
Index: linux-2.6.22-rc6-mm1/include/linux/suspend.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/linux/suspend.h	2007-06-30 21:14:44.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/linux/suspend.h	2007-06-30 21:14:54.000000000 +0200
@@ -126,7 +126,7 @@ extern void drain_local_pages(void);
 extern void mark_free_pages(struct zone *zone);
 
 /**
- * struct hibernation_ops - hibernation platform support
+ * struct platform_hibernation_ops - hibernation platform support
  *
  * The methods in this structure allow a platform to override the default
  * mechanism of shutting down the machine during a hibernation transition.
@@ -163,7 +163,7 @@ extern void mark_free_pages(struct zone 
  *	Called right after the nonboot CPUs have been enabled and before
  *	thawing devices (runs with IRQs on).
  */
-struct hibernation_ops {
+struct platform_hibernation_ops {
 	int (*start)(void);
 	int (*pre_snapshot)(void);
 	void (*finish)(void);
@@ -190,14 +190,14 @@ extern void swsusp_set_page_free(struct 
 extern void swsusp_unset_page_free(struct page *);
 extern unsigned long get_safe_page(gfp_t gfp_mask);
 
-extern void hibernation_set_ops(struct hibernation_ops *ops);
+extern void hibernation_set_ops(struct platform_hibernation_ops *ops);
 extern int hibernate(void);
 #else /* CONFIG_SOFTWARE_SUSPEND */
 static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
 static inline void swsusp_set_page_free(struct page *p) {}
 static inline void swsusp_unset_page_free(struct page *p) {}
 
-static inline void hibernation_set_ops(struct hibernation_ops *ops) {}
+static inline void hibernation_set_ops(struct platform_hibernation_ops *ops) {}
 static inline int hibernate(void) { return -ENOSYS; }
 #endif /* CONFIG_SOFTWARE_SUSPEND */
 
Index: linux-2.6.22-rc6-mm1/kernel/power/disk.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/disk.c	2007-06-30 21:14:44.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/disk.c	2007-06-30 21:14:54.000000000 +0200
@@ -45,14 +45,14 @@ enum {
 
 static int hibernation_mode = HIBERNATION_SHUTDOWN;
 
-static struct hibernation_ops *hibernation_ops;
+static struct platform_hibernation_ops *hibernation_ops;
 
 /**
  * hibernation_set_ops - set the global hibernate operations
  * @ops: the hibernation operations to use in subsequent hibernation transitions
  */
 
-void hibernation_set_ops(struct hibernation_ops *ops)
+void hibernation_set_ops(struct platform_hibernation_ops *ops)
 {
 	if (ops && !(ops->start && ops->pre_snapshot && ops->finish
 	    && ops->prepare && ops->enter && ops->pre_restore

Re: [RFC][PATCH -mm 9/9] PM: Rename hibernation_ops to platform_hibernation_ops

[Pavel Machek]

On Sat 2007-06-30 23:11:58, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Rename 'struct hibernation_ops' to 'struct platform_hibernation_ops' in analogy
> with 'struct platform_suspend_ops'.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Russell King]

On Sat, Jun 30, 2007 at 10:57:16PM +0200, Rafael J. Wysocki wrote:
> Hi,
> 
> This series of patches implements changes that are possible/necessary/desirable
> (IMO) after the introduction of the .set_target() method in 'struct pm_ops'
> (the patch that introduces it is in -mm,
> http://marc.info/?l=linux-mm-commits&m=118306698814722&w=2).

I suggest you have a look in:

 http://ftp.arm.linux.org.uk/pub/linux/arm/kernel/git-cur/arm:pxa2.diff

since there's some duplication and clashes between your patch set and mine.
I'm intending submitting the pxa2 branch when the next merge window opens.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Rafael J. Wysocki]

On Sunday, 1 July 2007 01:22, Russell King wrote:
> On Sat, Jun 30, 2007 at 10:57:16PM +0200, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > This series of patches implements changes that are possible/necessary/desirable
> > (IMO) after the introduction of the .set_target() method in 'struct pm_ops'
> > (the patch that introduces it is in -mm,
> > http://marc.info/?l=linux-mm-commits&m=118306698814722&w=2).
> 
> I suggest you have a look in:
> 
>  http://ftp.arm.linux.org.uk/pub/linux/arm/kernel/git-cur/arm:pxa2.diff
> 
> since there's some duplication and clashes between your patch set and mine.
> I'm intending submitting the pxa2 branch when the next merge window opens.

Okay, I'll rediff the patch series so that it applies on top of your changes.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[David Brownell]

On Saturday 30 June 2007, Rafael J. Wysocki wrote:
> 
> The patches make the following changes:
> * make ACPI use the new .set_target() method in 'struct pm_ops'
> * add an ACPI helper function for the devices to determine the power state
>   to put the device into
> * move the definition of 'struct pm_ops' to <include/suspend.h>
> * change the name of 'struct pm_ops' to 'struct platform_suspend_ops' and
>   modify the names of some related functions and global variables accordingly
> * modify 'struct platform_suspend_ops' so that .prepare() and .finish() don't
>   take arguments (.enter() still takes the state argument, because some
>   platforms don't need to implement the other callbacks)
> * make some functions normally defined in kernel/power/main.c be also defined
>   when CONFIG_PM is unset
> * make suspend_ops be a static variable
> * rework 'struct hibernation_ops' to add the new method analogous to
>   .set_target()
> * rename 'struct hibernation_ops' to 'struct platform_hibernation_ops' (in
>   analogy with 'struct platform_suspend_ops')

These look like good cleanups and, in some cases, enhancements.
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Rafael J. Wysocki]

On Monday, 2 July 2007 06:28, David Brownell wrote:
> On Saturday 30 June 2007, Rafael J. Wysocki wrote:
> > 
> > The patches make the following changes:
> > * make ACPI use the new .set_target() method in 'struct pm_ops'
> > * add an ACPI helper function for the devices to determine the power state
> >   to put the device into
> > * move the definition of 'struct pm_ops' to <include/suspend.h>
> > * change the name of 'struct pm_ops' to 'struct platform_suspend_ops' and
> >   modify the names of some related functions and global variables accordingly
> > * modify 'struct platform_suspend_ops' so that .prepare() and .finish() don't
> >   take arguments (.enter() still takes the state argument, because some
> >   platforms don't need to implement the other callbacks)
> > * make some functions normally defined in kernel/power/main.c be also defined
> >   when CONFIG_PM is unset
> > * make suspend_ops be a static variable
> > * rework 'struct hibernation_ops' to add the new method analogous to
> >   .set_target()
> > * rename 'struct hibernation_ops' to 'struct platform_hibernation_ops' (in
> >   analogy with 'struct platform_suspend_ops')
> 
> These look like good cleanups and, in some cases, enhancements.

Thanks. :-)

Well, the patches 1/9 and 3/9-9/9 seem to be in a good shape (2/9 needs some
more work, perhaps I'll be able to do something about it later today), but I
have to wait with them for the Russell's ARM patchset to be merged.  For now,
I'll just include them in the hibernation and suspend patchset
(http://www.sisk.pl/kernel/hibernation_and_suspend/2.6.22-rc7/patches/).

BTW, Russell, is there any chance that your patchest will be in -mm before
it's merged?  My patches will go through -mm anyway ...

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Russell King]

On Mon, Jul 02, 2007 at 04:28:25PM +0200, Rafael J. Wysocki wrote:
> BTW, Russell, is there any chance that your patchest will be in -mm before
> it's merged?  My patches will go through -mm anyway ...

If akpm picks up the devel branch from the ARM tree (which I believe he
does) he'll get the PXA stuff with that next time around.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Rafael J. Wysocki]

On Monday, 2 July 2007 16:36, Russell King wrote:
> On Mon, Jul 02, 2007 at 04:28:25PM +0200, Rafael J. Wysocki wrote:
> > BTW, Russell, is there any chance that your patchest will be in -mm before
> > it's merged?  My patches will go through -mm anyway ...
> 
> If akpm picks up the devel branch from the ARM tree (which I believe he
> does) he'll get the PXA stuff with that next time around.
> 

OK, thanks.

I'll push my patches to Andrew when the PXA update hits -mm.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Pavel Machek]

Hi!


> This series of patches implements changes that are possible/necessary/desirable
> (IMO) after the introduction of the .set_target() method in 'struct pm_ops'
> (the patch that introduces it is in -mm,
> http://marc.info/?l=linux-mm-commits&m=118306698814722&w=2).

(I think this series is already in -mm, right?)

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Rafael J. Wysocki]

Hi,

On Wednesday, 11 July 2007 12:53, Pavel Machek wrote:
> Hi!
> 
> 
> > This series of patches implements changes that are possible/necessary/desirable
> > (IMO) after the introduction of the .set_target() method in 'struct pm_ops'
> > (the patch that introduces it is in -mm,
> > http://marc.info/?l=linux-mm-commits&m=118306698814722&w=2).
> 
> (I think this series is already in -mm, right?)

No, it is not, because I'm waiting for the inclusion of the ARM update.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: [RFC][PATCH -mm 0/9] PM: Update global suspend and hibernation operations framework

[Russell King]

On Wed, Jul 11, 2007 at 01:16:07PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, 11 July 2007 12:53, Pavel Machek wrote:
> > > This series of patches implements changes that are possible/necessary/desirable
> > > (IMO) after the introduction of the .set_target() method in 'struct pm_ops'
> > > (the patch that introduces it is in -mm,
> > > http://marc.info/?l=linux-mm-commits&m=118306698814722&w=2).
> > 
> > (I think this series is already in -mm, right?)
> 
> No, it is not, because I'm waiting for the inclusion of the ARM update.

Which should be tomorrow evening...

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of: