All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v3 1/2] PM / Core: suspend_again callback for suspend_ops.
@ 2011-05-11  5:18 ` MyungJoo Ham
  0 siblings, 0 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-11  5:18 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Pavel Machek, Rafael J. Wysocki,
	Greg Kroah-Hartman, kyungmin.park, myungjoo.ham, Len Brown

A system or a device may need to control suspend/wakeup events. It may
want to wakeup the system after a predefined amount of time or at a
predefined event decided while entering suspend for polling or delayed
work. Then, it may want to enter suspend again if its predefined wakeup
condition is the only wakeup reason and there is no outstanding events;
thus, it does not wakeup the userspace unnecessary or unnecessary
devices and keeps suspended as long as possible (saving the power).

Enabling a system to wakeup after a specified time can be easily
achieved by using RTC. However, to enter suspend again immediately
without invoking userland and unrelated devices, we need additional
features in the suspend framework.

Such need comes from:

1. Monitoring a critical device status without interrupts that can
wakeup the system. (in-suspend polling)
 An example is ambient temperature monitoring that needs to shut down
the system or a specific device function if it is too hot or cold. The
temperature of a specific device may be needed to be monitored as well;
e.g., a charger monitors battery temperature in order to stop charging
if overheated.

2. Execute critical "delayed work" at suspend.
 A driver or a system/board may have a delayed work (or any similar
things) that it wants to execute at the requested time.
 For example, some chargers want to check the battery voltage some
time (e.g., 30 seconds) after the battery is fully charged and the
charger has stopped. Then, the charger restarts charging if the voltage
has dropped more than a threshold, which is smaller than "restart-charger"
voltage, which is a threshold to restart charging regardless of the
time passed.

This patch allows to add "suspend_again" callback at struct
platform_suspend_ops and let the "suspend_again" callback return true if
the system is required to enter suspend again after the current instance
of wakeup. Device-wise suspend_again implemented at dev_pm_ops or
syscore is not done because: a) suspend_again feature is usually under
platform-wise decision and controls the behavior of the whole platform
and b) There are very limited devices related to the usage cases of
suspend_again; chargers and temperature sensors are mentioned so far.

With suspend_again callback registered at struct platform_suspend_ops
suspend_ops in kernel/power/suspend.c with suspend_set_ops by the
platform, the suspend framework tries to enter suspend again by
looping suspend_enter() if suspend_again has returned true and there has
been no errors in the suspending sequence or pending wakeups (by
pm_wakeup_pending).

Tested at Exynos4-NURI.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

--
Changed from v2:
	- moved (again) from dev_pm_ops to suspend_ops
	- settled suspend_again point at around suspend_enter().
Changes from v1:
	- moved from syscore to dev_pm_ops
	- added generic ops for subsystems.
	- cleaned up suspend_again code at kernel/power/suspend.
---
 include/linux/suspend.h |    8 ++++++++
 kernel/power/suspend.c  |   13 ++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 5a89e36..caf5e97 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -92,6 +92,13 @@ typedef int __bitwise suspend_state_t;
  *	@enter() and @wake(), even if any of them fails.  It is executed after
  *	a failing @prepare.
  *
+ * @suspend_again: Returns whether the system should suspend again (true) or
+ *	not (false). If the platform wants to poll sensors or execute some
+ *	code during suspended without invoking userspace and most of devices,
+ *	suspend_again callback is the place assuming that periodic-wakeup or
+ *	alarm-wakeup is already setup. This allows to execute some codes while
+ *	being kept suspended in the view of userland and devices.
+ *
  * @end: Called by the PM core right after resuming devices, to indicate to
  *	the platform that the system has returned to the working state or
  *	the transition to the sleep state has been aborted.
@@ -113,6 +120,7 @@ struct platform_suspend_ops {
 	int (*enter)(suspend_state_t state);
 	void (*wake)(void);
 	void (*finish)(void);
+	bool (*suspend_again)(void);
 	void (*end)(void);
 	void (*recover)(void);
 };
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 08515b4..e655f43 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -128,10 +128,12 @@ void __attribute__ ((weak)) arch_suspend_enable_irqs(void)
 /**
  *	suspend_enter - enter the desired system sleep state.
  *	@state:		state to enter
+ *	@pm_wkup_pending:	indicates that the power transition in progress
+ *				should be aborted.
  *
  *	This function should be called after devices have been suspended.
  */
-static int suspend_enter(suspend_state_t state)
+static int suspend_enter(suspend_state_t state, bool *pm_wkup_pending)
 {
 	int error;
 
@@ -167,7 +169,8 @@ static int suspend_enter(suspend_state_t state)
 	if (!error)
 		error = syscore_suspend();
 	if (!error) {
-		if (!(suspend_test(TEST_CORE) || pm_wakeup_pending())) {
+		*pm_wkup_pending = pm_wakeup_pending();
+		if (!(suspend_test(TEST_CORE) || *pm_wkup_pending)) {
 			error = suspend_ops->enter(state);
 			events_check_enabled = false;
 		}
@@ -202,6 +205,7 @@ static int suspend_enter(suspend_state_t state)
 int suspend_devices_and_enter(suspend_state_t state)
 {
 	int error;
+	bool pm_wkup_pending = false;
 
 	if (!suspend_ops)
 		return -ENOSYS;
@@ -224,7 +228,10 @@ int suspend_devices_and_enter(suspend_state_t state)
 	if (suspend_test(TEST_DEVICES))
 		goto Recover_platform;
 
-	error = suspend_enter(state);
+	do {
+		error = suspend_enter(state, &pm_wkup_pending);
+	} while (suspend_ops->suspend_again && suspend_ops->suspend_again() &&
+		 !error && !pm_wkup_pending);
 
  Resume_devices:
 	suspend_test_start();
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v3 1/2] PM / Core: suspend_again callback for suspend_ops.
@ 2011-05-11  5:18 ` MyungJoo Ham
  0 siblings, 0 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-11  5:18 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park

A system or a device may need to control suspend/wakeup events. It may
want to wakeup the system after a predefined amount of time or at a
predefined event decided while entering suspend for polling or delayed
work. Then, it may want to enter suspend again if its predefined wakeup
condition is the only wakeup reason and there is no outstanding events;
thus, it does not wakeup the userspace unnecessary or unnecessary
devices and keeps suspended as long as possible (saving the power).

Enabling a system to wakeup after a specified time can be easily
achieved by using RTC. However, to enter suspend again immediately
without invoking userland and unrelated devices, we need additional
features in the suspend framework.

Such need comes from:

1. Monitoring a critical device status without interrupts that can
wakeup the system. (in-suspend polling)
 An example is ambient temperature monitoring that needs to shut down
the system or a specific device function if it is too hot or cold. The
temperature of a specific device may be needed to be monitored as well;
e.g., a charger monitors battery temperature in order to stop charging
if overheated.

2. Execute critical "delayed work" at suspend.
 A driver or a system/board may have a delayed work (or any similar
things) that it wants to execute at the requested time.
 For example, some chargers want to check the battery voltage some
time (e.g., 30 seconds) after the battery is fully charged and the
charger has stopped. Then, the charger restarts charging if the voltage
has dropped more than a threshold, which is smaller than "restart-charger"
voltage, which is a threshold to restart charging regardless of the
time passed.

This patch allows to add "suspend_again" callback at struct
platform_suspend_ops and let the "suspend_again" callback return true if
the system is required to enter suspend again after the current instance
of wakeup. Device-wise suspend_again implemented at dev_pm_ops or
syscore is not done because: a) suspend_again feature is usually under
platform-wise decision and controls the behavior of the whole platform
and b) There are very limited devices related to the usage cases of
suspend_again; chargers and temperature sensors are mentioned so far.

With suspend_again callback registered at struct platform_suspend_ops
suspend_ops in kernel/power/suspend.c with suspend_set_ops by the
platform, the suspend framework tries to enter suspend again by
looping suspend_enter() if suspend_again has returned true and there has
been no errors in the suspending sequence or pending wakeups (by
pm_wakeup_pending).

Tested at Exynos4-NURI.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

--
Changed from v2:
	- moved (again) from dev_pm_ops to suspend_ops
	- settled suspend_again point at around suspend_enter().
Changes from v1:
	- moved from syscore to dev_pm_ops
	- added generic ops for subsystems.
	- cleaned up suspend_again code at kernel/power/suspend.
---
 include/linux/suspend.h |    8 ++++++++
 kernel/power/suspend.c  |   13 ++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 5a89e36..caf5e97 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -92,6 +92,13 @@ typedef int __bitwise suspend_state_t;
  *	@enter() and @wake(), even if any of them fails.  It is executed after
  *	a failing @prepare.
  *
+ * @suspend_again: Returns whether the system should suspend again (true) or
+ *	not (false). If the platform wants to poll sensors or execute some
+ *	code during suspended without invoking userspace and most of devices,
+ *	suspend_again callback is the place assuming that periodic-wakeup or
+ *	alarm-wakeup is already setup. This allows to execute some codes while
+ *	being kept suspended in the view of userland and devices.
+ *
  * @end: Called by the PM core right after resuming devices, to indicate to
  *	the platform that the system has returned to the working state or
  *	the transition to the sleep state has been aborted.
@@ -113,6 +120,7 @@ struct platform_suspend_ops {
 	int (*enter)(suspend_state_t state);
 	void (*wake)(void);
 	void (*finish)(void);
+	bool (*suspend_again)(void);
 	void (*end)(void);
 	void (*recover)(void);
 };
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 08515b4..e655f43 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -128,10 +128,12 @@ void __attribute__ ((weak)) arch_suspend_enable_irqs(void)
 /**
  *	suspend_enter - enter the desired system sleep state.
  *	@state:		state to enter
+ *	@pm_wkup_pending:	indicates that the power transition in progress
+ *				should be aborted.
  *
  *	This function should be called after devices have been suspended.
  */
-static int suspend_enter(suspend_state_t state)
+static int suspend_enter(suspend_state_t state, bool *pm_wkup_pending)
 {
 	int error;
 
@@ -167,7 +169,8 @@ static int suspend_enter(suspend_state_t state)
 	if (!error)
 		error = syscore_suspend();
 	if (!error) {
-		if (!(suspend_test(TEST_CORE) || pm_wakeup_pending())) {
+		*pm_wkup_pending = pm_wakeup_pending();
+		if (!(suspend_test(TEST_CORE) || *pm_wkup_pending)) {
 			error = suspend_ops->enter(state);
 			events_check_enabled = false;
 		}
@@ -202,6 +205,7 @@ static int suspend_enter(suspend_state_t state)
 int suspend_devices_and_enter(suspend_state_t state)
 {
 	int error;
+	bool pm_wkup_pending = false;
 
 	if (!suspend_ops)
 		return -ENOSYS;
@@ -224,7 +228,10 @@ int suspend_devices_and_enter(suspend_state_t state)
 	if (suspend_test(TEST_DEVICES))
 		goto Recover_platform;
 
-	error = suspend_enter(state);
+	do {
+		error = suspend_enter(state, &pm_wkup_pending);
+	} while (suspend_ops->suspend_again && suspend_ops->suspend_again() &&
+		 !error && !pm_wkup_pending);
 
  Resume_devices:
 	suspend_test_start();
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v3 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-11  5:18 ` MyungJoo Ham
@ 2011-05-11  5:18   ` MyungJoo Ham
  -1 siblings, 0 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-11  5:18 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Pavel Machek, Rafael J. Wysocki,
	Greg Kroah-Hartman, kyungmin.park, myungjoo.ham, Len Brown

The API, suspend_again, is supposed to be used by platforms when the
platforms want to execute some code while suspended (wakeup by an alarm
or periodic ticks, run code, suspend again). Because suspend_again is
not called when every device is resumed, but is called right after
suspend_enter() is called, the suspend_again callback should include
device resume and suspend features.

This patch provides two APIs: dpm_partial_resume and
dpm_partial_suspend. They are supposed to be used in suspend_again to
actiavte required devices.

A usage example is:

/* Devices required to run "monitor_something()". */
static device *devs[] = {
	&console_serial,
	&device_x,
	&device_y,
	...
};

bool example_suspend_again(void)
{
	int error, monitor_result;

	if (!wakeup_reason_is_polling())
		return false;

	dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));
	resume_console();

	monitor_result = monitor_something();

	suspend_console();
	error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));

	if (error || monitor_result == ERROR)
		return false;

	return true;
}

Tested at Exynos4-NURI.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/base/power/main.c |  154 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h        |    4 +
 2 files changed, 158 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 052dc53..71dc693 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1082,3 +1082,157 @@ int device_pm_wait_for_dev(struct device *subordinate, struct device *dev)
 	return async_error;
 }
 EXPORT_SYMBOL_GPL(device_pm_wait_for_dev);
+
+/**
+ * __dpm_partial_resume - Execute "resume" callbacks for the listed devices.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ */
+static void __dpm_partial_resume(pm_message_t state, struct device **devs,
+				 int size)
+{
+	int i;
+	struct device *dev;
+
+	for (i = 0; i < size; i++) {
+		int error;
+
+		dev = devs[i];
+		get_device(dev);
+
+		error = device_resume(dev, state, false);
+		if (error)
+			pm_dev_err(dev, state, "", error);
+
+		put_device(dev);
+	}
+}
+
+/**
+ * __dpm_partial_complete - Complete a PM transition for the listed devices.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ */
+static void __dpm_partial_complete(pm_message_t state, struct device **devs,
+				   int size)
+{
+	int i;
+	struct device *dev;
+
+	for (i = 0; i < size; i++) {
+		dev = devs[i];
+
+		get_device(dev);
+		dev->power.in_suspend = false;
+
+		device_complete(dev, state);
+
+		put_device(dev);
+	}
+}
+
+/**
+ * dpm_partial_resume - Execute "resume" callbacks and complete system
+ *			transaction for the chosen devices only.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ *
+ * Execute "resume" callbacks for the listed devices and complete the PM
+ * transition of them.
+ *
+ * Because the only a part of devices will be resumed, asynchronous resume
+ * is dropped.
+ */
+void dpm_partial_resume(pm_message_t state, struct device **devs, int size)
+{
+	/* Partial dpm_resume */
+	__dpm_partial_resume(state, devs, size);
+
+	/* Partial dpm_complete */
+	__dpm_partial_complete(state, devs, size);
+}
+EXPORT_SYMBOL_GPL(dpm_partial_resume);
+
+/**
+ * dpm_partial_suspend - Prepare the given devices for PM transition and
+ *			suspend them.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being suspended
+ * @size: The size of devs array.
+ *
+ * Prepare the given devices for system PM transition and execute "suspend"
+ * callbacks for them.
+ *
+ * The devs array is iterated in the reversed order to use the same devs
+ * array with dpm_partial_resume().
+ */
+int dpm_partial_suspend(pm_message_t state, struct device **devs, int size)
+{
+	int error = 0, i;
+	struct device *dev;
+
+	/* Partial dpm_prepare */
+	for (i = size - 1; i >= 0; i--) {
+		dev = devs[i];
+
+		get_device(dev);
+
+		pm_runtime_get_noresume(dev);
+		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+			pm_wakeup_event(dev, 0);
+
+		pm_runtime_put_sync(dev);
+		error = pm_wakeup_pending() ?
+				-EBUSY : device_prepare(dev, state);
+
+		if (error) {
+			if (error == -EAGAIN) {
+				put_device(dev);
+				error = 0;
+				i++; /* Try Again */
+				continue;
+			}
+			printk(KERN_INFO "PM: Device %s not prepared "
+				"for power transition: code %d\n",
+				dev_name(dev), error);
+			put_device(dev);
+			break;
+		}
+		dev->power.in_suspend = true;
+		put_device(dev);
+	}
+
+	if (error)
+		goto err_prepare;
+
+	/* Partial dpm_suspend */
+	for (i = size - 1; i >= 0; i--) {
+		dev = devs[i];
+
+		get_device(dev);
+
+		/* Synchronous suspend. The list shouldn't be long */
+		error = __device_suspend(dev, pm_transition, false);
+
+		if (error) {
+			pm_dev_err(dev, state, "", error);
+			put_device(dev);
+			break;
+		}
+		put_device(dev);
+	}
+
+	if (!error)
+		return 0;
+
+	__dpm_partial_resume(PMSG_RESUME, devs + i + 1, size - i - 1);
+	i = -1;
+err_prepare:
+	__dpm_partial_complete(PMSG_RESUME, devs + i + 1, size - i - 1);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(dpm_partial_suspend);
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 512e091..b407762 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -540,10 +540,14 @@ static inline int sysdev_resume(void) { return 0; }
 extern void device_pm_lock(void);
 extern void dpm_resume_noirq(pm_message_t state);
 extern void dpm_resume_end(pm_message_t state);
+extern void dpm_partial_resume(pm_message_t state, struct device **devs,
+			       int size);
 
 extern void device_pm_unlock(void);
 extern int dpm_suspend_noirq(pm_message_t state);
 extern int dpm_suspend_start(pm_message_t state);
+extern int dpm_partial_suspend(pm_message_t state, struct device **devs,
+			       int size);
 
 extern void __suspend_report_result(const char *function, void *fn, int ret);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC PATCH v3 2/2] PM / Core: partial resume/suspend API for suspend_again users.
@ 2011-05-11  5:18   ` MyungJoo Ham
  0 siblings, 0 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-11  5:18 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park

The API, suspend_again, is supposed to be used by platforms when the
platforms want to execute some code while suspended (wakeup by an alarm
or periodic ticks, run code, suspend again). Because suspend_again is
not called when every device is resumed, but is called right after
suspend_enter() is called, the suspend_again callback should include
device resume and suspend features.

This patch provides two APIs: dpm_partial_resume and
dpm_partial_suspend. They are supposed to be used in suspend_again to
actiavte required devices.

A usage example is:

/* Devices required to run "monitor_something()". */
static device *devs[] = {
	&console_serial,
	&device_x,
	&device_y,
	...
};

bool example_suspend_again(void)
{
	int error, monitor_result;

	if (!wakeup_reason_is_polling())
		return false;

	dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));
	resume_console();

	monitor_result = monitor_something();

	suspend_console();
	error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));

	if (error || monitor_result == ERROR)
		return false;

	return true;
}

Tested at Exynos4-NURI.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/base/power/main.c |  154 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h        |    4 +
 2 files changed, 158 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 052dc53..71dc693 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1082,3 +1082,157 @@ int device_pm_wait_for_dev(struct device *subordinate, struct device *dev)
 	return async_error;
 }
 EXPORT_SYMBOL_GPL(device_pm_wait_for_dev);
+
+/**
+ * __dpm_partial_resume - Execute "resume" callbacks for the listed devices.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ */
+static void __dpm_partial_resume(pm_message_t state, struct device **devs,
+				 int size)
+{
+	int i;
+	struct device *dev;
+
+	for (i = 0; i < size; i++) {
+		int error;
+
+		dev = devs[i];
+		get_device(dev);
+
+		error = device_resume(dev, state, false);
+		if (error)
+			pm_dev_err(dev, state, "", error);
+
+		put_device(dev);
+	}
+}
+
+/**
+ * __dpm_partial_complete - Complete a PM transition for the listed devices.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ */
+static void __dpm_partial_complete(pm_message_t state, struct device **devs,
+				   int size)
+{
+	int i;
+	struct device *dev;
+
+	for (i = 0; i < size; i++) {
+		dev = devs[i];
+
+		get_device(dev);
+		dev->power.in_suspend = false;
+
+		device_complete(dev, state);
+
+		put_device(dev);
+	}
+}
+
+/**
+ * dpm_partial_resume - Execute "resume" callbacks and complete system
+ *			transaction for the chosen devices only.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ *
+ * Execute "resume" callbacks for the listed devices and complete the PM
+ * transition of them.
+ *
+ * Because the only a part of devices will be resumed, asynchronous resume
+ * is dropped.
+ */
+void dpm_partial_resume(pm_message_t state, struct device **devs, int size)
+{
+	/* Partial dpm_resume */
+	__dpm_partial_resume(state, devs, size);
+
+	/* Partial dpm_complete */
+	__dpm_partial_complete(state, devs, size);
+}
+EXPORT_SYMBOL_GPL(dpm_partial_resume);
+
+/**
+ * dpm_partial_suspend - Prepare the given devices for PM transition and
+ *			suspend them.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being suspended
+ * @size: The size of devs array.
+ *
+ * Prepare the given devices for system PM transition and execute "suspend"
+ * callbacks for them.
+ *
+ * The devs array is iterated in the reversed order to use the same devs
+ * array with dpm_partial_resume().
+ */
+int dpm_partial_suspend(pm_message_t state, struct device **devs, int size)
+{
+	int error = 0, i;
+	struct device *dev;
+
+	/* Partial dpm_prepare */
+	for (i = size - 1; i >= 0; i--) {
+		dev = devs[i];
+
+		get_device(dev);
+
+		pm_runtime_get_noresume(dev);
+		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+			pm_wakeup_event(dev, 0);
+
+		pm_runtime_put_sync(dev);
+		error = pm_wakeup_pending() ?
+				-EBUSY : device_prepare(dev, state);
+
+		if (error) {
+			if (error == -EAGAIN) {
+				put_device(dev);
+				error = 0;
+				i++; /* Try Again */
+				continue;
+			}
+			printk(KERN_INFO "PM: Device %s not prepared "
+				"for power transition: code %d\n",
+				dev_name(dev), error);
+			put_device(dev);
+			break;
+		}
+		dev->power.in_suspend = true;
+		put_device(dev);
+	}
+
+	if (error)
+		goto err_prepare;
+
+	/* Partial dpm_suspend */
+	for (i = size - 1; i >= 0; i--) {
+		dev = devs[i];
+
+		get_device(dev);
+
+		/* Synchronous suspend. The list shouldn't be long */
+		error = __device_suspend(dev, pm_transition, false);
+
+		if (error) {
+			pm_dev_err(dev, state, "", error);
+			put_device(dev);
+			break;
+		}
+		put_device(dev);
+	}
+
+	if (!error)
+		return 0;
+
+	__dpm_partial_resume(PMSG_RESUME, devs + i + 1, size - i - 1);
+	i = -1;
+err_prepare:
+	__dpm_partial_complete(PMSG_RESUME, devs + i + 1, size - i - 1);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(dpm_partial_suspend);
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 512e091..b407762 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -540,10 +540,14 @@ static inline int sysdev_resume(void) { return 0; }
 extern void device_pm_lock(void);
 extern void dpm_resume_noirq(pm_message_t state);
 extern void dpm_resume_end(pm_message_t state);
+extern void dpm_partial_resume(pm_message_t state, struct device **devs,
+			       int size);
 
 extern void device_pm_unlock(void);
 extern int dpm_suspend_noirq(pm_message_t state);
 extern int dpm_suspend_start(pm_message_t state);
+extern int dpm_partial_suspend(pm_message_t state, struct device **devs,
+			       int size);
 
 extern void __suspend_report_result(const char *function, void *fn, int ret);
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v3 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-11  5:18 ` MyungJoo Ham
  (?)
  (?)
@ 2011-05-12  6:19 ` Pavel Machek
  2011-05-17  4:59   ` [PATCH v4 " MyungJoo Ham
  2011-05-17  4:59   ` MyungJoo Ham
  -1 siblings, 2 replies; 22+ messages in thread
From: Pavel Machek @ 2011-05-12  6:19 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: linux-pm, linux-kernel, Rafael J. Wysocki, Greg Kroah-Hartman,
	kyungmin.park, myungjoo.ham, Len Brown

Hi!

> A system or a device may need to control suspend/wakeup events. It may
> want to wakeup the system after a predefined amount of time or at a
> predefined event decided while entering suspend for polling or delayed
> work. Then, it may want to enter suspend again if its predefined wakeup
> condition is the only wakeup reason and there is no outstanding events;
> thus, it does not wakeup the userspace unnecessary or unnecessary
> devices and keeps suspended as long as possible (saving the power).
> 
> Enabling a system to wakeup after a specified time can be easily
> achieved by using RTC. However, to enter suspend again immediately
> without invoking userland and unrelated devices, we need additional
> features in the suspend framework.

Looks ok to me.

>  /**
>   *	suspend_enter - enter the desired system sleep state.
>   *	@state:		state to enter
> + *	@pm_wkup_pending:	indicates that the power transition in progress
> + *				should be aborted.
>   *

Wowels are cheap, I'd add them here.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC PATCH v3 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-11  5:18 ` MyungJoo Ham
                   ` (2 preceding siblings ...)
  (?)
@ 2011-05-12  6:19 ` Pavel Machek
  -1 siblings, 0 replies; 22+ messages in thread
From: Pavel Machek @ 2011-05-12  6:19 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park, linux-pm

Hi!

> A system or a device may need to control suspend/wakeup events. It may
> want to wakeup the system after a predefined amount of time or at a
> predefined event decided while entering suspend for polling or delayed
> work. Then, it may want to enter suspend again if its predefined wakeup
> condition is the only wakeup reason and there is no outstanding events;
> thus, it does not wakeup the userspace unnecessary or unnecessary
> devices and keeps suspended as long as possible (saving the power).
> 
> Enabling a system to wakeup after a specified time can be easily
> achieved by using RTC. However, to enter suspend again immediately
> without invoking userland and unrelated devices, we need additional
> features in the suspend framework.

Looks ok to me.

>  /**
>   *	suspend_enter - enter the desired system sleep state.
>   *	@state:		state to enter
> + *	@pm_wkup_pending:	indicates that the power transition in progress
> + *				should be aborted.
>   *

Wowels are cheap, I'd add them here.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-12  6:19 ` [RFC PATCH v3 1/2] PM / Core: suspend_again callback for suspend_ops Pavel Machek
@ 2011-05-17  4:59   ` MyungJoo Ham
  2011-05-17  4:59     ` [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users MyungJoo Ham
                       ` (3 more replies)
  2011-05-17  4:59   ` MyungJoo Ham
  1 sibling, 4 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-17  4:59 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Len Brown, Pavel Machek, Rafael J. Wysocki,
	Greg Kroah-Hartman, kyungmin.park, myungjoo.ham

A system or a device may need to control suspend/wakeup events. It may
want to wakeup the system after a predefined amount of time or at a
predefined event decided while entering suspend for polling or delayed
work. Then, it may want to enter suspend again if its predefined wakeup
condition is the only wakeup reason and there is no outstanding events;
thus, it does not wakeup the userspace unnecessary or unnecessary
devices and keeps suspended as long as possible (saving the power).

Enabling a system to wakeup after a specified time can be easily
achieved by using RTC. However, to enter suspend again immediately
without invoking userland and unrelated devices, we need additional
features in the suspend framework.

Such need comes from:

1. Monitoring a critical device status without interrupts that can
wakeup the system. (in-suspend polling)
 An example is ambient temperature monitoring that needs to shut down
the system or a specific device function if it is too hot or cold. The
temperature of a specific device may be needed to be monitored as well;
e.g., a charger monitors battery temperature in order to stop charging
if overheated.

2. Execute critical "delayed work" at suspend.
 A driver or a system/board may have a delayed work (or any similar
things) that it wants to execute at the requested time.
 For example, some chargers want to check the battery voltage some
time (e.g., 30 seconds) after the battery is fully charged and the
charger has stopped. Then, the charger restarts charging if the voltage
has dropped more than a threshold, which is smaller than "restart-charger"
voltage, which is a threshold to restart charging regardless of the
time passed.

This patch allows to add "suspend_again" callback at struct
platform_suspend_ops and let the "suspend_again" callback return true if
the system is required to enter suspend again after the current instance
of wakeup. Device-wise suspend_again implemented at dev_pm_ops or
syscore is not done because: a) suspend_again feature is usually under
platform-wise decision and controls the behavior of the whole platform
and b) There are very limited devices related to the usage cases of
suspend_again; chargers and temperature sensors are mentioned so far.

With suspend_again callback registered at struct platform_suspend_ops
suspend_ops in kernel/power/suspend.c with suspend_set_ops by the
platform, the suspend framework tries to enter suspend again by
looping suspend_enter() if suspend_again has returned true and there has
been no errors in the suspending sequence or pending wakeups (by
pm_wakeup_pending).

Tested at Exynos4-NURI.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

--
Thank you for your valuable comments, Pavel, Rafael, Greg, and others.

Changed from v3:
	- renamed local variable "pm_wkup_pending" to "wakeup_pending".
	pm_wakeup_pending is not used because there is a function with the
	same name.
Changed from v2:
	- moved (again) from dev_pm_ops to suspend_ops
	- settled suspend_again point at around suspend_enter().
Changes from v1:
	- moved from syscore to dev_pm_ops
	- added generic ops for subsystems.
	- cleaned up suspend_again code at kernel/power/suspend.
---
 include/linux/suspend.h |    8 ++++++++
 kernel/power/suspend.c  |   13 ++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 5a89e36..caf5e97 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -92,6 +92,13 @@ typedef int __bitwise suspend_state_t;
  *	@enter() and @wake(), even if any of them fails.  It is executed after
  *	a failing @prepare.
  *
+ * @suspend_again: Returns whether the system should suspend again (true) or
+ *	not (false). If the platform wants to poll sensors or execute some
+ *	code during suspended without invoking userspace and most of devices,
+ *	suspend_again callback is the place assuming that periodic-wakeup or
+ *	alarm-wakeup is already setup. This allows to execute some codes while
+ *	being kept suspended in the view of userland and devices.
+ *
  * @end: Called by the PM core right after resuming devices, to indicate to
  *	the platform that the system has returned to the working state or
  *	the transition to the sleep state has been aborted.
@@ -113,6 +120,7 @@ struct platform_suspend_ops {
 	int (*enter)(suspend_state_t state);
 	void (*wake)(void);
 	void (*finish)(void);
+	bool (*suspend_again)(void);
 	void (*end)(void);
 	void (*recover)(void);
 };
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 08515b4..63a6b0b 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -128,10 +128,12 @@ void __attribute__ ((weak)) arch_suspend_enable_irqs(void)
 /**
  *	suspend_enter - enter the desired system sleep state.
  *	@state:		state to enter
+ *	@wakeup_pending:	indicates that the power transition in progress
+ *				should be aborted.
  *
  *	This function should be called after devices have been suspended.
  */
-static int suspend_enter(suspend_state_t state)
+static int suspend_enter(suspend_state_t state, bool *wakeup_pending)
 {
 	int error;
 
@@ -167,7 +169,8 @@ static int suspend_enter(suspend_state_t state)
 	if (!error)
 		error = syscore_suspend();
 	if (!error) {
-		if (!(suspend_test(TEST_CORE) || pm_wakeup_pending())) {
+		*wakeup_pending = pm_wakeup_pending();
+		if (!(suspend_test(TEST_CORE) || *wakeup_pending)) {
 			error = suspend_ops->enter(state);
 			events_check_enabled = false;
 		}
@@ -202,6 +205,7 @@ static int suspend_enter(suspend_state_t state)
 int suspend_devices_and_enter(suspend_state_t state)
 {
 	int error;
+	bool wakeup_pending = false;
 
 	if (!suspend_ops)
 		return -ENOSYS;
@@ -224,7 +228,10 @@ int suspend_devices_and_enter(suspend_state_t state)
 	if (suspend_test(TEST_DEVICES))
 		goto Recover_platform;
 
-	error = suspend_enter(state);
+	do {
+		error = suspend_enter(state, &wakeup_pending);
+	} while (suspend_ops->suspend_again && suspend_ops->suspend_again() &&
+		 !error && !wakeup_pending);
 
  Resume_devices:
 	suspend_test_start();
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-12  6:19 ` [RFC PATCH v3 1/2] PM / Core: suspend_again callback for suspend_ops Pavel Machek
  2011-05-17  4:59   ` [PATCH v4 " MyungJoo Ham
@ 2011-05-17  4:59   ` MyungJoo Ham
  1 sibling, 0 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-17  4:59 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park

A system or a device may need to control suspend/wakeup events. It may
want to wakeup the system after a predefined amount of time or at a
predefined event decided while entering suspend for polling or delayed
work. Then, it may want to enter suspend again if its predefined wakeup
condition is the only wakeup reason and there is no outstanding events;
thus, it does not wakeup the userspace unnecessary or unnecessary
devices and keeps suspended as long as possible (saving the power).

Enabling a system to wakeup after a specified time can be easily
achieved by using RTC. However, to enter suspend again immediately
without invoking userland and unrelated devices, we need additional
features in the suspend framework.

Such need comes from:

1. Monitoring a critical device status without interrupts that can
wakeup the system. (in-suspend polling)
 An example is ambient temperature monitoring that needs to shut down
the system or a specific device function if it is too hot or cold. The
temperature of a specific device may be needed to be monitored as well;
e.g., a charger monitors battery temperature in order to stop charging
if overheated.

2. Execute critical "delayed work" at suspend.
 A driver or a system/board may have a delayed work (or any similar
things) that it wants to execute at the requested time.
 For example, some chargers want to check the battery voltage some
time (e.g., 30 seconds) after the battery is fully charged and the
charger has stopped. Then, the charger restarts charging if the voltage
has dropped more than a threshold, which is smaller than "restart-charger"
voltage, which is a threshold to restart charging regardless of the
time passed.

This patch allows to add "suspend_again" callback at struct
platform_suspend_ops and let the "suspend_again" callback return true if
the system is required to enter suspend again after the current instance
of wakeup. Device-wise suspend_again implemented at dev_pm_ops or
syscore is not done because: a) suspend_again feature is usually under
platform-wise decision and controls the behavior of the whole platform
and b) There are very limited devices related to the usage cases of
suspend_again; chargers and temperature sensors are mentioned so far.

With suspend_again callback registered at struct platform_suspend_ops
suspend_ops in kernel/power/suspend.c with suspend_set_ops by the
platform, the suspend framework tries to enter suspend again by
looping suspend_enter() if suspend_again has returned true and there has
been no errors in the suspending sequence or pending wakeups (by
pm_wakeup_pending).

Tested at Exynos4-NURI.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

--
Thank you for your valuable comments, Pavel, Rafael, Greg, and others.

Changed from v3:
	- renamed local variable "pm_wkup_pending" to "wakeup_pending".
	pm_wakeup_pending is not used because there is a function with the
	same name.
Changed from v2:
	- moved (again) from dev_pm_ops to suspend_ops
	- settled suspend_again point at around suspend_enter().
Changes from v1:
	- moved from syscore to dev_pm_ops
	- added generic ops for subsystems.
	- cleaned up suspend_again code at kernel/power/suspend.
---
 include/linux/suspend.h |    8 ++++++++
 kernel/power/suspend.c  |   13 ++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 5a89e36..caf5e97 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -92,6 +92,13 @@ typedef int __bitwise suspend_state_t;
  *	@enter() and @wake(), even if any of them fails.  It is executed after
  *	a failing @prepare.
  *
+ * @suspend_again: Returns whether the system should suspend again (true) or
+ *	not (false). If the platform wants to poll sensors or execute some
+ *	code during suspended without invoking userspace and most of devices,
+ *	suspend_again callback is the place assuming that periodic-wakeup or
+ *	alarm-wakeup is already setup. This allows to execute some codes while
+ *	being kept suspended in the view of userland and devices.
+ *
  * @end: Called by the PM core right after resuming devices, to indicate to
  *	the platform that the system has returned to the working state or
  *	the transition to the sleep state has been aborted.
@@ -113,6 +120,7 @@ struct platform_suspend_ops {
 	int (*enter)(suspend_state_t state);
 	void (*wake)(void);
 	void (*finish)(void);
+	bool (*suspend_again)(void);
 	void (*end)(void);
 	void (*recover)(void);
 };
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 08515b4..63a6b0b 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -128,10 +128,12 @@ void __attribute__ ((weak)) arch_suspend_enable_irqs(void)
 /**
  *	suspend_enter - enter the desired system sleep state.
  *	@state:		state to enter
+ *	@wakeup_pending:	indicates that the power transition in progress
+ *				should be aborted.
  *
  *	This function should be called after devices have been suspended.
  */
-static int suspend_enter(suspend_state_t state)
+static int suspend_enter(suspend_state_t state, bool *wakeup_pending)
 {
 	int error;
 
@@ -167,7 +169,8 @@ static int suspend_enter(suspend_state_t state)
 	if (!error)
 		error = syscore_suspend();
 	if (!error) {
-		if (!(suspend_test(TEST_CORE) || pm_wakeup_pending())) {
+		*wakeup_pending = pm_wakeup_pending();
+		if (!(suspend_test(TEST_CORE) || *wakeup_pending)) {
 			error = suspend_ops->enter(state);
 			events_check_enabled = false;
 		}
@@ -202,6 +205,7 @@ static int suspend_enter(suspend_state_t state)
 int suspend_devices_and_enter(suspend_state_t state)
 {
 	int error;
+	bool wakeup_pending = false;
 
 	if (!suspend_ops)
 		return -ENOSYS;
@@ -224,7 +228,10 @@ int suspend_devices_and_enter(suspend_state_t state)
 	if (suspend_test(TEST_DEVICES))
 		goto Recover_platform;
 
-	error = suspend_enter(state);
+	do {
+		error = suspend_enter(state, &wakeup_pending);
+	} while (suspend_ops->suspend_again && suspend_ops->suspend_again() &&
+		 !error && !wakeup_pending);
 
  Resume_devices:
 	suspend_test_start();
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-17  4:59   ` [PATCH v4 " MyungJoo Ham
@ 2011-05-17  4:59     ` MyungJoo Ham
  2011-05-17 20:52       ` Rafael J. Wysocki
  2011-05-17 20:52       ` Rafael J. Wysocki
  2011-05-17  4:59     ` MyungJoo Ham
                       ` (2 subsequent siblings)
  3 siblings, 2 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-17  4:59 UTC (permalink / raw)
  To: linux-pm
  Cc: linux-kernel, Len Brown, Pavel Machek, Rafael J. Wysocki,
	Greg Kroah-Hartman, kyungmin.park, myungjoo.ham

The API, suspend_again, is supposed to be used by platforms when the
platforms want to execute some code while suspended (wakeup by an alarm
or periodic ticks, run code, suspend again). Because suspend_again is
not called when every device is resumed, but is called right after
suspend_enter() is called, the suspend_again callback should include
device resume and suspend features.

This patch provides two APIs: dpm_partial_resume and
dpm_partial_suspend. They are supposed to be used in suspend_again to
actiavte required devices.

A usage example is:

/* Devices required to run "monitor_something()". */
static device *devs[] = {
	&console_serial,
	&device_x,
	&device_y,
	...
};

bool example_suspend_again(void)
{
	int error, monitor_result;

	if (!wakeup_reason_is_polling())
		return false;

	dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));
	resume_console();

	monitor_result = monitor_something();

	suspend_console();
	error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));

	if (error || monitor_result == ERROR)
		return false;

	return true;
}

Tested at Exynos4-NURI.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
No changes from v3.
---
 drivers/base/power/main.c |  154 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h        |    4 +
 2 files changed, 158 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 052dc53..71dc693 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1082,3 +1082,157 @@ int device_pm_wait_for_dev(struct device *subordinate, struct device *dev)
 	return async_error;
 }
 EXPORT_SYMBOL_GPL(device_pm_wait_for_dev);
+
+/**
+ * __dpm_partial_resume - Execute "resume" callbacks for the listed devices.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ */
+static void __dpm_partial_resume(pm_message_t state, struct device **devs,
+				 int size)
+{
+	int i;
+	struct device *dev;
+
+	for (i = 0; i < size; i++) {
+		int error;
+
+		dev = devs[i];
+		get_device(dev);
+
+		error = device_resume(dev, state, false);
+		if (error)
+			pm_dev_err(dev, state, "", error);
+
+		put_device(dev);
+	}
+}
+
+/**
+ * __dpm_partial_complete - Complete a PM transition for the listed devices.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ */
+static void __dpm_partial_complete(pm_message_t state, struct device **devs,
+				   int size)
+{
+	int i;
+	struct device *dev;
+
+	for (i = 0; i < size; i++) {
+		dev = devs[i];
+
+		get_device(dev);
+		dev->power.in_suspend = false;
+
+		device_complete(dev, state);
+
+		put_device(dev);
+	}
+}
+
+/**
+ * dpm_partial_resume - Execute "resume" callbacks and complete system
+ *			transaction for the chosen devices only.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ *
+ * Execute "resume" callbacks for the listed devices and complete the PM
+ * transition of them.
+ *
+ * Because the only a part of devices will be resumed, asynchronous resume
+ * is dropped.
+ */
+void dpm_partial_resume(pm_message_t state, struct device **devs, int size)
+{
+	/* Partial dpm_resume */
+	__dpm_partial_resume(state, devs, size);
+
+	/* Partial dpm_complete */
+	__dpm_partial_complete(state, devs, size);
+}
+EXPORT_SYMBOL_GPL(dpm_partial_resume);
+
+/**
+ * dpm_partial_suspend - Prepare the given devices for PM transition and
+ *			suspend them.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being suspended
+ * @size: The size of devs array.
+ *
+ * Prepare the given devices for system PM transition and execute "suspend"
+ * callbacks for them.
+ *
+ * The devs array is iterated in the reversed order to use the same devs
+ * array with dpm_partial_resume().
+ */
+int dpm_partial_suspend(pm_message_t state, struct device **devs, int size)
+{
+	int error = 0, i;
+	struct device *dev;
+
+	/* Partial dpm_prepare */
+	for (i = size - 1; i >= 0; i--) {
+		dev = devs[i];
+
+		get_device(dev);
+
+		pm_runtime_get_noresume(dev);
+		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+			pm_wakeup_event(dev, 0);
+
+		pm_runtime_put_sync(dev);
+		error = pm_wakeup_pending() ?
+				-EBUSY : device_prepare(dev, state);
+
+		if (error) {
+			if (error == -EAGAIN) {
+				put_device(dev);
+				error = 0;
+				i++; /* Try Again */
+				continue;
+			}
+			printk(KERN_INFO "PM: Device %s not prepared "
+				"for power transition: code %d\n",
+				dev_name(dev), error);
+			put_device(dev);
+			break;
+		}
+		dev->power.in_suspend = true;
+		put_device(dev);
+	}
+
+	if (error)
+		goto err_prepare;
+
+	/* Partial dpm_suspend */
+	for (i = size - 1; i >= 0; i--) {
+		dev = devs[i];
+
+		get_device(dev);
+
+		/* Synchronous suspend. The list shouldn't be long */
+		error = __device_suspend(dev, pm_transition, false);
+
+		if (error) {
+			pm_dev_err(dev, state, "", error);
+			put_device(dev);
+			break;
+		}
+		put_device(dev);
+	}
+
+	if (!error)
+		return 0;
+
+	__dpm_partial_resume(PMSG_RESUME, devs + i + 1, size - i - 1);
+	i = -1;
+err_prepare:
+	__dpm_partial_complete(PMSG_RESUME, devs + i + 1, size - i - 1);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(dpm_partial_suspend);
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 512e091..b407762 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -540,10 +540,14 @@ static inline int sysdev_resume(void) { return 0; }
 extern void device_pm_lock(void);
 extern void dpm_resume_noirq(pm_message_t state);
 extern void dpm_resume_end(pm_message_t state);
+extern void dpm_partial_resume(pm_message_t state, struct device **devs,
+			       int size);
 
 extern void device_pm_unlock(void);
 extern int dpm_suspend_noirq(pm_message_t state);
 extern int dpm_suspend_start(pm_message_t state);
+extern int dpm_partial_suspend(pm_message_t state, struct device **devs,
+			       int size);
 
 extern void __suspend_report_result(const char *function, void *fn, int ret);
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-17  4:59   ` [PATCH v4 " MyungJoo Ham
  2011-05-17  4:59     ` [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users MyungJoo Ham
@ 2011-05-17  4:59     ` MyungJoo Ham
  2011-05-17 20:40     ` [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops Rafael J. Wysocki
  2011-05-17 20:40     ` Rafael J. Wysocki
  3 siblings, 0 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-17  4:59 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park

The API, suspend_again, is supposed to be used by platforms when the
platforms want to execute some code while suspended (wakeup by an alarm
or periodic ticks, run code, suspend again). Because suspend_again is
not called when every device is resumed, but is called right after
suspend_enter() is called, the suspend_again callback should include
device resume and suspend features.

This patch provides two APIs: dpm_partial_resume and
dpm_partial_suspend. They are supposed to be used in suspend_again to
actiavte required devices.

A usage example is:

/* Devices required to run "monitor_something()". */
static device *devs[] = {
	&console_serial,
	&device_x,
	&device_y,
	...
};

bool example_suspend_again(void)
{
	int error, monitor_result;

	if (!wakeup_reason_is_polling())
		return false;

	dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));
	resume_console();

	monitor_result = monitor_something();

	suspend_console();
	error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));

	if (error || monitor_result == ERROR)
		return false;

	return true;
}

Tested at Exynos4-NURI.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
No changes from v3.
---
 drivers/base/power/main.c |  154 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h        |    4 +
 2 files changed, 158 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 052dc53..71dc693 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1082,3 +1082,157 @@ int device_pm_wait_for_dev(struct device *subordinate, struct device *dev)
 	return async_error;
 }
 EXPORT_SYMBOL_GPL(device_pm_wait_for_dev);
+
+/**
+ * __dpm_partial_resume - Execute "resume" callbacks for the listed devices.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ */
+static void __dpm_partial_resume(pm_message_t state, struct device **devs,
+				 int size)
+{
+	int i;
+	struct device *dev;
+
+	for (i = 0; i < size; i++) {
+		int error;
+
+		dev = devs[i];
+		get_device(dev);
+
+		error = device_resume(dev, state, false);
+		if (error)
+			pm_dev_err(dev, state, "", error);
+
+		put_device(dev);
+	}
+}
+
+/**
+ * __dpm_partial_complete - Complete a PM transition for the listed devices.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ */
+static void __dpm_partial_complete(pm_message_t state, struct device **devs,
+				   int size)
+{
+	int i;
+	struct device *dev;
+
+	for (i = 0; i < size; i++) {
+		dev = devs[i];
+
+		get_device(dev);
+		dev->power.in_suspend = false;
+
+		device_complete(dev, state);
+
+		put_device(dev);
+	}
+}
+
+/**
+ * dpm_partial_resume - Execute "resume" callbacks and complete system
+ *			transaction for the chosen devices only.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being resumed
+ * @size: The size of devs array.
+ *
+ * Execute "resume" callbacks for the listed devices and complete the PM
+ * transition of them.
+ *
+ * Because the only a part of devices will be resumed, asynchronous resume
+ * is dropped.
+ */
+void dpm_partial_resume(pm_message_t state, struct device **devs, int size)
+{
+	/* Partial dpm_resume */
+	__dpm_partial_resume(state, devs, size);
+
+	/* Partial dpm_complete */
+	__dpm_partial_complete(state, devs, size);
+}
+EXPORT_SYMBOL_GPL(dpm_partial_resume);
+
+/**
+ * dpm_partial_suspend - Prepare the given devices for PM transition and
+ *			suspend them.
+ * @state: PM transition being carried out.
+ * @devs: Array of pointers for the devices being suspended
+ * @size: The size of devs array.
+ *
+ * Prepare the given devices for system PM transition and execute "suspend"
+ * callbacks for them.
+ *
+ * The devs array is iterated in the reversed order to use the same devs
+ * array with dpm_partial_resume().
+ */
+int dpm_partial_suspend(pm_message_t state, struct device **devs, int size)
+{
+	int error = 0, i;
+	struct device *dev;
+
+	/* Partial dpm_prepare */
+	for (i = size - 1; i >= 0; i--) {
+		dev = devs[i];
+
+		get_device(dev);
+
+		pm_runtime_get_noresume(dev);
+		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
+			pm_wakeup_event(dev, 0);
+
+		pm_runtime_put_sync(dev);
+		error = pm_wakeup_pending() ?
+				-EBUSY : device_prepare(dev, state);
+
+		if (error) {
+			if (error == -EAGAIN) {
+				put_device(dev);
+				error = 0;
+				i++; /* Try Again */
+				continue;
+			}
+			printk(KERN_INFO "PM: Device %s not prepared "
+				"for power transition: code %d\n",
+				dev_name(dev), error);
+			put_device(dev);
+			break;
+		}
+		dev->power.in_suspend = true;
+		put_device(dev);
+	}
+
+	if (error)
+		goto err_prepare;
+
+	/* Partial dpm_suspend */
+	for (i = size - 1; i >= 0; i--) {
+		dev = devs[i];
+
+		get_device(dev);
+
+		/* Synchronous suspend. The list shouldn't be long */
+		error = __device_suspend(dev, pm_transition, false);
+
+		if (error) {
+			pm_dev_err(dev, state, "", error);
+			put_device(dev);
+			break;
+		}
+		put_device(dev);
+	}
+
+	if (!error)
+		return 0;
+
+	__dpm_partial_resume(PMSG_RESUME, devs + i + 1, size - i - 1);
+	i = -1;
+err_prepare:
+	__dpm_partial_complete(PMSG_RESUME, devs + i + 1, size - i - 1);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(dpm_partial_suspend);
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 512e091..b407762 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -540,10 +540,14 @@ static inline int sysdev_resume(void) { return 0; }
 extern void device_pm_lock(void);
 extern void dpm_resume_noirq(pm_message_t state);
 extern void dpm_resume_end(pm_message_t state);
+extern void dpm_partial_resume(pm_message_t state, struct device **devs,
+			       int size);
 
 extern void device_pm_unlock(void);
 extern int dpm_suspend_noirq(pm_message_t state);
 extern int dpm_suspend_start(pm_message_t state);
+extern int dpm_partial_suspend(pm_message_t state, struct device **devs,
+			       int size);
 
 extern void __suspend_report_result(const char *function, void *fn, int ret);
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-17  4:59   ` [PATCH v4 " MyungJoo Ham
                       ` (2 preceding siblings ...)
  2011-05-17 20:40     ` [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops Rafael J. Wysocki
@ 2011-05-17 20:40     ` Rafael J. Wysocki
  2011-05-18  9:07       ` MyungJoo Ham
  2011-05-18  9:07       ` MyungJoo Ham
  3 siblings, 2 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2011-05-17 20:40 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: linux-pm, linux-kernel, Len Brown, Pavel Machek,
	Greg Kroah-Hartman, kyungmin.park, myungjoo.ham

On Tuesday, May 17, 2011, MyungJoo Ham wrote:
> A system or a device may need to control suspend/wakeup events. It may
> want to wakeup the system after a predefined amount of time or at a
> predefined event decided while entering suspend for polling or delayed
> work. Then, it may want to enter suspend again if its predefined wakeup
> condition is the only wakeup reason and there is no outstanding events;
> thus, it does not wakeup the userspace unnecessary or unnecessary
> devices and keeps suspended as long as possible (saving the power).
> 
> Enabling a system to wakeup after a specified time can be easily
> achieved by using RTC. However, to enter suspend again immediately
> without invoking userland and unrelated devices, we need additional
> features in the suspend framework.
> 
> Such need comes from:
> 
> 1. Monitoring a critical device status without interrupts that can
> wakeup the system. (in-suspend polling)
>  An example is ambient temperature monitoring that needs to shut down
> the system or a specific device function if it is too hot or cold. The
> temperature of a specific device may be needed to be monitored as well;
> e.g., a charger monitors battery temperature in order to stop charging
> if overheated.
> 
> 2. Execute critical "delayed work" at suspend.
>  A driver or a system/board may have a delayed work (or any similar
> things) that it wants to execute at the requested time.
>  For example, some chargers want to check the battery voltage some
> time (e.g., 30 seconds) after the battery is fully charged and the
> charger has stopped. Then, the charger restarts charging if the voltage
> has dropped more than a threshold, which is smaller than "restart-charger"
> voltage, which is a threshold to restart charging regardless of the
> time passed.
> 
> This patch allows to add "suspend_again" callback at struct
> platform_suspend_ops and let the "suspend_again" callback return true if
> the system is required to enter suspend again after the current instance
> of wakeup. Device-wise suspend_again implemented at dev_pm_ops or
> syscore is not done because: a) suspend_again feature is usually under
> platform-wise decision and controls the behavior of the whole platform
> and b) There are very limited devices related to the usage cases of
> suspend_again; chargers and temperature sensors are mentioned so far.
> 
> With suspend_again callback registered at struct platform_suspend_ops
> suspend_ops in kernel/power/suspend.c with suspend_set_ops by the
> platform, the suspend framework tries to enter suspend again by
> looping suspend_enter() if suspend_again has returned true and there has
> been no errors in the suspending sequence or pending wakeups (by
> pm_wakeup_pending).
> 
> Tested at Exynos4-NURI.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> 
> --
> Thank you for your valuable comments, Pavel, Rafael, Greg, and others.
> 
> Changed from v3:
> 	- renamed local variable "pm_wkup_pending" to "wakeup_pending".
> 	pm_wakeup_pending is not used because there is a function with the
> 	same name.
> Changed from v2:
> 	- moved (again) from dev_pm_ops to suspend_ops
> 	- settled suspend_again point at around suspend_enter().
> Changes from v1:
> 	- moved from syscore to dev_pm_ops
> 	- added generic ops for subsystems.
> 	- cleaned up suspend_again code at kernel/power/suspend.
> ---
>  include/linux/suspend.h |    8 ++++++++
>  kernel/power/suspend.c  |   13 ++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 5a89e36..caf5e97 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -92,6 +92,13 @@ typedef int __bitwise suspend_state_t;
>   *	@enter() and @wake(), even if any of them fails.  It is executed after
>   *	a failing @prepare.
>   *
> + * @suspend_again: Returns whether the system should suspend again (true) or
> + *	not (false). If the platform wants to poll sensors or execute some
> + *	code during suspended without invoking userspace and most of devices,
> + *	suspend_again callback is the place assuming that periodic-wakeup or
> + *	alarm-wakeup is already setup. This allows to execute some codes while
> + *	being kept suspended in the view of userland and devices.
> + *
>   * @end: Called by the PM core right after resuming devices, to indicate to
>   *	the platform that the system has returned to the working state or
>   *	the transition to the sleep state has been aborted.
> @@ -113,6 +120,7 @@ struct platform_suspend_ops {
>  	int (*enter)(suspend_state_t state);
>  	void (*wake)(void);
>  	void (*finish)(void);
> +	bool (*suspend_again)(void);
>  	void (*end)(void);
>  	void (*recover)(void);
>  };
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 08515b4..63a6b0b 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -128,10 +128,12 @@ void __attribute__ ((weak)) arch_suspend_enable_irqs(void)
>  /**
>   *	suspend_enter - enter the desired system sleep state.
>   *	@state:		state to enter
> + *	@wakeup_pending:	indicates that the power transition in progress
> + *				should be aborted.
>   *
>   *	This function should be called after devices have been suspended.
>   */
> -static int suspend_enter(suspend_state_t state)
> +static int suspend_enter(suspend_state_t state, bool *wakeup_pending)

You don't need to use the wakeup_pending argument at all.  I think you
shouldn't use it even, because in theory there may be a wakeup event after
you've called pm_wakeup_pending() in suspend_enter() and in that case
you should break the loop too.

>  {
>  	int error;
>  
> @@ -167,7 +169,8 @@ static int suspend_enter(suspend_state_t state)
>  	if (!error)
>  		error = syscore_suspend();
>  	if (!error) {
> -		if (!(suspend_test(TEST_CORE) || pm_wakeup_pending())) {
> +		*wakeup_pending = pm_wakeup_pending();
> +		if (!(suspend_test(TEST_CORE) || *wakeup_pending)) {
>  			error = suspend_ops->enter(state);
>  			events_check_enabled = false;
>  		}
> @@ -202,6 +205,7 @@ static int suspend_enter(suspend_state_t state)
>  int suspend_devices_and_enter(suspend_state_t state)
>  {
>  	int error;
> +	bool wakeup_pending = false;
>  
>  	if (!suspend_ops)
>  		return -ENOSYS;
> @@ -224,7 +228,10 @@ int suspend_devices_and_enter(suspend_state_t state)
>  	if (suspend_test(TEST_DEVICES))
>  		goto Recover_platform;
>  
> -	error = suspend_enter(state);
> +	do {
> +		error = suspend_enter(state, &wakeup_pending);
> +	} while (suspend_ops->suspend_again && suspend_ops->suspend_again() &&
> +		 !error && !wakeup_pending);

So I would simply call pm_wakeup_pending() here again.

>  
>   Resume_devices:
>  	suspend_test_start();
> 

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-17  4:59   ` [PATCH v4 " MyungJoo Ham
  2011-05-17  4:59     ` [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users MyungJoo Ham
  2011-05-17  4:59     ` MyungJoo Ham
@ 2011-05-17 20:40     ` Rafael J. Wysocki
  2011-05-17 20:40     ` Rafael J. Wysocki
  3 siblings, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2011-05-17 20:40 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park, linux-pm

On Tuesday, May 17, 2011, MyungJoo Ham wrote:
> A system or a device may need to control suspend/wakeup events. It may
> want to wakeup the system after a predefined amount of time or at a
> predefined event decided while entering suspend for polling or delayed
> work. Then, it may want to enter suspend again if its predefined wakeup
> condition is the only wakeup reason and there is no outstanding events;
> thus, it does not wakeup the userspace unnecessary or unnecessary
> devices and keeps suspended as long as possible (saving the power).
> 
> Enabling a system to wakeup after a specified time can be easily
> achieved by using RTC. However, to enter suspend again immediately
> without invoking userland and unrelated devices, we need additional
> features in the suspend framework.
> 
> Such need comes from:
> 
> 1. Monitoring a critical device status without interrupts that can
> wakeup the system. (in-suspend polling)
>  An example is ambient temperature monitoring that needs to shut down
> the system or a specific device function if it is too hot or cold. The
> temperature of a specific device may be needed to be monitored as well;
> e.g., a charger monitors battery temperature in order to stop charging
> if overheated.
> 
> 2. Execute critical "delayed work" at suspend.
>  A driver or a system/board may have a delayed work (or any similar
> things) that it wants to execute at the requested time.
>  For example, some chargers want to check the battery voltage some
> time (e.g., 30 seconds) after the battery is fully charged and the
> charger has stopped. Then, the charger restarts charging if the voltage
> has dropped more than a threshold, which is smaller than "restart-charger"
> voltage, which is a threshold to restart charging regardless of the
> time passed.
> 
> This patch allows to add "suspend_again" callback at struct
> platform_suspend_ops and let the "suspend_again" callback return true if
> the system is required to enter suspend again after the current instance
> of wakeup. Device-wise suspend_again implemented at dev_pm_ops or
> syscore is not done because: a) suspend_again feature is usually under
> platform-wise decision and controls the behavior of the whole platform
> and b) There are very limited devices related to the usage cases of
> suspend_again; chargers and temperature sensors are mentioned so far.
> 
> With suspend_again callback registered at struct platform_suspend_ops
> suspend_ops in kernel/power/suspend.c with suspend_set_ops by the
> platform, the suspend framework tries to enter suspend again by
> looping suspend_enter() if suspend_again has returned true and there has
> been no errors in the suspending sequence or pending wakeups (by
> pm_wakeup_pending).
> 
> Tested at Exynos4-NURI.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> 
> --
> Thank you for your valuable comments, Pavel, Rafael, Greg, and others.
> 
> Changed from v3:
> 	- renamed local variable "pm_wkup_pending" to "wakeup_pending".
> 	pm_wakeup_pending is not used because there is a function with the
> 	same name.
> Changed from v2:
> 	- moved (again) from dev_pm_ops to suspend_ops
> 	- settled suspend_again point at around suspend_enter().
> Changes from v1:
> 	- moved from syscore to dev_pm_ops
> 	- added generic ops for subsystems.
> 	- cleaned up suspend_again code at kernel/power/suspend.
> ---
>  include/linux/suspend.h |    8 ++++++++
>  kernel/power/suspend.c  |   13 ++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 5a89e36..caf5e97 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -92,6 +92,13 @@ typedef int __bitwise suspend_state_t;
>   *	@enter() and @wake(), even if any of them fails.  It is executed after
>   *	a failing @prepare.
>   *
> + * @suspend_again: Returns whether the system should suspend again (true) or
> + *	not (false). If the platform wants to poll sensors or execute some
> + *	code during suspended without invoking userspace and most of devices,
> + *	suspend_again callback is the place assuming that periodic-wakeup or
> + *	alarm-wakeup is already setup. This allows to execute some codes while
> + *	being kept suspended in the view of userland and devices.
> + *
>   * @end: Called by the PM core right after resuming devices, to indicate to
>   *	the platform that the system has returned to the working state or
>   *	the transition to the sleep state has been aborted.
> @@ -113,6 +120,7 @@ struct platform_suspend_ops {
>  	int (*enter)(suspend_state_t state);
>  	void (*wake)(void);
>  	void (*finish)(void);
> +	bool (*suspend_again)(void);
>  	void (*end)(void);
>  	void (*recover)(void);
>  };
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 08515b4..63a6b0b 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -128,10 +128,12 @@ void __attribute__ ((weak)) arch_suspend_enable_irqs(void)
>  /**
>   *	suspend_enter - enter the desired system sleep state.
>   *	@state:		state to enter
> + *	@wakeup_pending:	indicates that the power transition in progress
> + *				should be aborted.
>   *
>   *	This function should be called after devices have been suspended.
>   */
> -static int suspend_enter(suspend_state_t state)
> +static int suspend_enter(suspend_state_t state, bool *wakeup_pending)

You don't need to use the wakeup_pending argument at all.  I think you
shouldn't use it even, because in theory there may be a wakeup event after
you've called pm_wakeup_pending() in suspend_enter() and in that case
you should break the loop too.

>  {
>  	int error;
>  
> @@ -167,7 +169,8 @@ static int suspend_enter(suspend_state_t state)
>  	if (!error)
>  		error = syscore_suspend();
>  	if (!error) {
> -		if (!(suspend_test(TEST_CORE) || pm_wakeup_pending())) {
> +		*wakeup_pending = pm_wakeup_pending();
> +		if (!(suspend_test(TEST_CORE) || *wakeup_pending)) {
>  			error = suspend_ops->enter(state);
>  			events_check_enabled = false;
>  		}
> @@ -202,6 +205,7 @@ static int suspend_enter(suspend_state_t state)
>  int suspend_devices_and_enter(suspend_state_t state)
>  {
>  	int error;
> +	bool wakeup_pending = false;
>  
>  	if (!suspend_ops)
>  		return -ENOSYS;
> @@ -224,7 +228,10 @@ int suspend_devices_and_enter(suspend_state_t state)
>  	if (suspend_test(TEST_DEVICES))
>  		goto Recover_platform;
>  
> -	error = suspend_enter(state);
> +	do {
> +		error = suspend_enter(state, &wakeup_pending);
> +	} while (suspend_ops->suspend_again && suspend_ops->suspend_again() &&
> +		 !error && !wakeup_pending);

So I would simply call pm_wakeup_pending() here again.

>  
>   Resume_devices:
>  	suspend_test_start();
> 

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-17  4:59     ` [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users MyungJoo Ham
@ 2011-05-17 20:52       ` Rafael J. Wysocki
  2011-05-18  5:58         ` MyungJoo Ham
  2011-05-18  5:58         ` MyungJoo Ham
  2011-05-17 20:52       ` Rafael J. Wysocki
  1 sibling, 2 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2011-05-17 20:52 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: linux-pm, linux-kernel, Len Brown, Pavel Machek,
	Greg Kroah-Hartman, kyungmin.park, myungjoo.ham

On Tuesday, May 17, 2011, MyungJoo Ham wrote:
> The API, suspend_again, is supposed to be used by platforms when the
> platforms want to execute some code while suspended (wakeup by an alarm
> or periodic ticks, run code, suspend again). Because suspend_again is
> not called when every device is resumed, but is called right after
> suspend_enter() is called, the suspend_again callback should include
> device resume and suspend features.
> 
> This patch provides two APIs: dpm_partial_resume and
> dpm_partial_suspend. They are supposed to be used in suspend_again to
> actiavte required devices.
> 
> A usage example is:
> 
> /* Devices required to run "monitor_something()". */
> static device *devs[] = {
> 	&console_serial,
> 	&device_x,
> 	&device_y,
> 	...
> };
> 
> bool example_suspend_again(void)
> {
> 	int error, monitor_result;
> 
> 	if (!wakeup_reason_is_polling())
> 		return false;
> 
> 	dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));

I'm not sure you need the first argument here.  Also, if the array is
NULL terminated, you won't need the third one.

> 	resume_console();
> 
> 	monitor_result = monitor_something();
> 
> 	suspend_console();
> 	error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));

Same here.

> 	if (error || monitor_result == ERROR)
> 		return false;
> 
> 	return true;
> }

That said, I don't like this patch.  The reason is that you call
suspend_enter() is a loop and it calls the _noirq() callbacks for
_all_ devices and now you're going to only call .resume() and
.suspend() for several selected devices.  This is not too consistent.

I wonder if those devices needed for .suspend_again() to work on
your platform could be resumed and suspended by their drivers'
_noirq() callbacks?

Rafael


> Tested at Exynos4-NURI.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> --
> No changes from v3.
> ---
>  drivers/base/power/main.c |  154 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pm.h        |    4 +
>  2 files changed, 158 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 052dc53..71dc693 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1082,3 +1082,157 @@ int device_pm_wait_for_dev(struct device *subordinate, struct device *dev)
>  	return async_error;
>  }
>  EXPORT_SYMBOL_GPL(device_pm_wait_for_dev);
> +
> +/**
> + * __dpm_partial_resume - Execute "resume" callbacks for the listed devices.
> + * @state: PM transition being carried out.
> + * @devs: Array of pointers for the devices being resumed
> + * @size: The size of devs array.
> + */
> +static void __dpm_partial_resume(pm_message_t state, struct device **devs,
> +				 int size)
> +{
> +	int i;
> +	struct device *dev;
> +
> +	for (i = 0; i < size; i++) {
> +		int error;
> +
> +		dev = devs[i];
> +		get_device(dev);
> +
> +		error = device_resume(dev, state, false);
> +		if (error)
> +			pm_dev_err(dev, state, "", error);
> +
> +		put_device(dev);
> +	}
> +}
> +
> +/**
> + * __dpm_partial_complete - Complete a PM transition for the listed devices.
> + * @state: PM transition being carried out.
> + * @devs: Array of pointers for the devices being resumed
> + * @size: The size of devs array.
> + */
> +static void __dpm_partial_complete(pm_message_t state, struct device **devs,
> +				   int size)
> +{
> +	int i;
> +	struct device *dev;
> +
> +	for (i = 0; i < size; i++) {
> +		dev = devs[i];
> +
> +		get_device(dev);
> +		dev->power.in_suspend = false;
> +
> +		device_complete(dev, state);
> +
> +		put_device(dev);
> +	}
> +}
> +
> +/**
> + * dpm_partial_resume - Execute "resume" callbacks and complete system
> + *			transaction for the chosen devices only.
> + * @state: PM transition being carried out.
> + * @devs: Array of pointers for the devices being resumed
> + * @size: The size of devs array.
> + *
> + * Execute "resume" callbacks for the listed devices and complete the PM
> + * transition of them.
> + *
> + * Because the only a part of devices will be resumed, asynchronous resume
> + * is dropped.
> + */
> +void dpm_partial_resume(pm_message_t state, struct device **devs, int size)
> +{
> +	/* Partial dpm_resume */
> +	__dpm_partial_resume(state, devs, size);
> +
> +	/* Partial dpm_complete */
> +	__dpm_partial_complete(state, devs, size);
> +}
> +EXPORT_SYMBOL_GPL(dpm_partial_resume);
> +
> +/**
> + * dpm_partial_suspend - Prepare the given devices for PM transition and
> + *			suspend them.
> + * @state: PM transition being carried out.
> + * @devs: Array of pointers for the devices being suspended
> + * @size: The size of devs array.
> + *
> + * Prepare the given devices for system PM transition and execute "suspend"
> + * callbacks for them.
> + *
> + * The devs array is iterated in the reversed order to use the same devs
> + * array with dpm_partial_resume().
> + */
> +int dpm_partial_suspend(pm_message_t state, struct device **devs, int size)
> +{
> +	int error = 0, i;
> +	struct device *dev;
> +
> +	/* Partial dpm_prepare */
> +	for (i = size - 1; i >= 0; i--) {
> +		dev = devs[i];
> +
> +		get_device(dev);
> +
> +		pm_runtime_get_noresume(dev);
> +		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
> +			pm_wakeup_event(dev, 0);
> +
> +		pm_runtime_put_sync(dev);
> +		error = pm_wakeup_pending() ?
> +				-EBUSY : device_prepare(dev, state);
> +
> +		if (error) {
> +			if (error == -EAGAIN) {
> +				put_device(dev);
> +				error = 0;
> +				i++; /* Try Again */
> +				continue;
> +			}
> +			printk(KERN_INFO "PM: Device %s not prepared "
> +				"for power transition: code %d\n",
> +				dev_name(dev), error);
> +			put_device(dev);
> +			break;
> +		}
> +		dev->power.in_suspend = true;
> +		put_device(dev);
> +	}
> +
> +	if (error)
> +		goto err_prepare;
> +
> +	/* Partial dpm_suspend */
> +	for (i = size - 1; i >= 0; i--) {
> +		dev = devs[i];
> +
> +		get_device(dev);
> +
> +		/* Synchronous suspend. The list shouldn't be long */
> +		error = __device_suspend(dev, pm_transition, false);
> +
> +		if (error) {
> +			pm_dev_err(dev, state, "", error);
> +			put_device(dev);
> +			break;
> +		}
> +		put_device(dev);
> +	}
> +
> +	if (!error)
> +		return 0;
> +
> +	__dpm_partial_resume(PMSG_RESUME, devs + i + 1, size - i - 1);
> +	i = -1;
> +err_prepare:
> +	__dpm_partial_complete(PMSG_RESUME, devs + i + 1, size - i - 1);
> +
> +	return error;
> +}
> +EXPORT_SYMBOL_GPL(dpm_partial_suspend);
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 512e091..b407762 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -540,10 +540,14 @@ static inline int sysdev_resume(void) { return 0; }
>  extern void device_pm_lock(void);
>  extern void dpm_resume_noirq(pm_message_t state);
>  extern void dpm_resume_end(pm_message_t state);
> +extern void dpm_partial_resume(pm_message_t state, struct device **devs,
> +			       int size);
>  
>  extern void device_pm_unlock(void);
>  extern int dpm_suspend_noirq(pm_message_t state);
>  extern int dpm_suspend_start(pm_message_t state);
> +extern int dpm_partial_suspend(pm_message_t state, struct device **devs,
> +			       int size);
>  
>  extern void __suspend_report_result(const char *function, void *fn, int ret);
>  
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-17  4:59     ` [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users MyungJoo Ham
  2011-05-17 20:52       ` Rafael J. Wysocki
@ 2011-05-17 20:52       ` Rafael J. Wysocki
  1 sibling, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2011-05-17 20:52 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park, linux-pm

On Tuesday, May 17, 2011, MyungJoo Ham wrote:
> The API, suspend_again, is supposed to be used by platforms when the
> platforms want to execute some code while suspended (wakeup by an alarm
> or periodic ticks, run code, suspend again). Because suspend_again is
> not called when every device is resumed, but is called right after
> suspend_enter() is called, the suspend_again callback should include
> device resume and suspend features.
> 
> This patch provides two APIs: dpm_partial_resume and
> dpm_partial_suspend. They are supposed to be used in suspend_again to
> actiavte required devices.
> 
> A usage example is:
> 
> /* Devices required to run "monitor_something()". */
> static device *devs[] = {
> 	&console_serial,
> 	&device_x,
> 	&device_y,
> 	...
> };
> 
> bool example_suspend_again(void)
> {
> 	int error, monitor_result;
> 
> 	if (!wakeup_reason_is_polling())
> 		return false;
> 
> 	dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));

I'm not sure you need the first argument here.  Also, if the array is
NULL terminated, you won't need the third one.

> 	resume_console();
> 
> 	monitor_result = monitor_something();
> 
> 	suspend_console();
> 	error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));

Same here.

> 	if (error || monitor_result == ERROR)
> 		return false;
> 
> 	return true;
> }

That said, I don't like this patch.  The reason is that you call
suspend_enter() is a loop and it calls the _noirq() callbacks for
_all_ devices and now you're going to only call .resume() and
.suspend() for several selected devices.  This is not too consistent.

I wonder if those devices needed for .suspend_again() to work on
your platform could be resumed and suspended by their drivers'
_noirq() callbacks?

Rafael


> Tested at Exynos4-NURI.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> --
> No changes from v3.
> ---
>  drivers/base/power/main.c |  154 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pm.h        |    4 +
>  2 files changed, 158 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 052dc53..71dc693 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1082,3 +1082,157 @@ int device_pm_wait_for_dev(struct device *subordinate, struct device *dev)
>  	return async_error;
>  }
>  EXPORT_SYMBOL_GPL(device_pm_wait_for_dev);
> +
> +/**
> + * __dpm_partial_resume - Execute "resume" callbacks for the listed devices.
> + * @state: PM transition being carried out.
> + * @devs: Array of pointers for the devices being resumed
> + * @size: The size of devs array.
> + */
> +static void __dpm_partial_resume(pm_message_t state, struct device **devs,
> +				 int size)
> +{
> +	int i;
> +	struct device *dev;
> +
> +	for (i = 0; i < size; i++) {
> +		int error;
> +
> +		dev = devs[i];
> +		get_device(dev);
> +
> +		error = device_resume(dev, state, false);
> +		if (error)
> +			pm_dev_err(dev, state, "", error);
> +
> +		put_device(dev);
> +	}
> +}
> +
> +/**
> + * __dpm_partial_complete - Complete a PM transition for the listed devices.
> + * @state: PM transition being carried out.
> + * @devs: Array of pointers for the devices being resumed
> + * @size: The size of devs array.
> + */
> +static void __dpm_partial_complete(pm_message_t state, struct device **devs,
> +				   int size)
> +{
> +	int i;
> +	struct device *dev;
> +
> +	for (i = 0; i < size; i++) {
> +		dev = devs[i];
> +
> +		get_device(dev);
> +		dev->power.in_suspend = false;
> +
> +		device_complete(dev, state);
> +
> +		put_device(dev);
> +	}
> +}
> +
> +/**
> + * dpm_partial_resume - Execute "resume" callbacks and complete system
> + *			transaction for the chosen devices only.
> + * @state: PM transition being carried out.
> + * @devs: Array of pointers for the devices being resumed
> + * @size: The size of devs array.
> + *
> + * Execute "resume" callbacks for the listed devices and complete the PM
> + * transition of them.
> + *
> + * Because the only a part of devices will be resumed, asynchronous resume
> + * is dropped.
> + */
> +void dpm_partial_resume(pm_message_t state, struct device **devs, int size)
> +{
> +	/* Partial dpm_resume */
> +	__dpm_partial_resume(state, devs, size);
> +
> +	/* Partial dpm_complete */
> +	__dpm_partial_complete(state, devs, size);
> +}
> +EXPORT_SYMBOL_GPL(dpm_partial_resume);
> +
> +/**
> + * dpm_partial_suspend - Prepare the given devices for PM transition and
> + *			suspend them.
> + * @state: PM transition being carried out.
> + * @devs: Array of pointers for the devices being suspended
> + * @size: The size of devs array.
> + *
> + * Prepare the given devices for system PM transition and execute "suspend"
> + * callbacks for them.
> + *
> + * The devs array is iterated in the reversed order to use the same devs
> + * array with dpm_partial_resume().
> + */
> +int dpm_partial_suspend(pm_message_t state, struct device **devs, int size)
> +{
> +	int error = 0, i;
> +	struct device *dev;
> +
> +	/* Partial dpm_prepare */
> +	for (i = size - 1; i >= 0; i--) {
> +		dev = devs[i];
> +
> +		get_device(dev);
> +
> +		pm_runtime_get_noresume(dev);
> +		if (pm_runtime_barrier(dev) && device_may_wakeup(dev))
> +			pm_wakeup_event(dev, 0);
> +
> +		pm_runtime_put_sync(dev);
> +		error = pm_wakeup_pending() ?
> +				-EBUSY : device_prepare(dev, state);
> +
> +		if (error) {
> +			if (error == -EAGAIN) {
> +				put_device(dev);
> +				error = 0;
> +				i++; /* Try Again */
> +				continue;
> +			}
> +			printk(KERN_INFO "PM: Device %s not prepared "
> +				"for power transition: code %d\n",
> +				dev_name(dev), error);
> +			put_device(dev);
> +			break;
> +		}
> +		dev->power.in_suspend = true;
> +		put_device(dev);
> +	}
> +
> +	if (error)
> +		goto err_prepare;
> +
> +	/* Partial dpm_suspend */
> +	for (i = size - 1; i >= 0; i--) {
> +		dev = devs[i];
> +
> +		get_device(dev);
> +
> +		/* Synchronous suspend. The list shouldn't be long */
> +		error = __device_suspend(dev, pm_transition, false);
> +
> +		if (error) {
> +			pm_dev_err(dev, state, "", error);
> +			put_device(dev);
> +			break;
> +		}
> +		put_device(dev);
> +	}
> +
> +	if (!error)
> +		return 0;
> +
> +	__dpm_partial_resume(PMSG_RESUME, devs + i + 1, size - i - 1);
> +	i = -1;
> +err_prepare:
> +	__dpm_partial_complete(PMSG_RESUME, devs + i + 1, size - i - 1);
> +
> +	return error;
> +}
> +EXPORT_SYMBOL_GPL(dpm_partial_suspend);
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 512e091..b407762 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -540,10 +540,14 @@ static inline int sysdev_resume(void) { return 0; }
>  extern void device_pm_lock(void);
>  extern void dpm_resume_noirq(pm_message_t state);
>  extern void dpm_resume_end(pm_message_t state);
> +extern void dpm_partial_resume(pm_message_t state, struct device **devs,
> +			       int size);
>  
>  extern void device_pm_unlock(void);
>  extern int dpm_suspend_noirq(pm_message_t state);
>  extern int dpm_suspend_start(pm_message_t state);
> +extern int dpm_partial_suspend(pm_message_t state, struct device **devs,
> +			       int size);
>  
>  extern void __suspend_report_result(const char *function, void *fn, int ret);
>  
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-17 20:52       ` Rafael J. Wysocki
  2011-05-18  5:58         ` MyungJoo Ham
@ 2011-05-18  5:58         ` MyungJoo Ham
  2011-05-18 20:18           ` Rafael J. Wysocki
  2011-05-18 20:18           ` Rafael J. Wysocki
  1 sibling, 2 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-18  5:58 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-pm, linux-kernel, Len Brown, Pavel Machek,
	Greg Kroah-Hartman, kyungmin.park

2011/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> On Tuesday, May 17, 2011, MyungJoo Ham wrote:
>> A usage example is:
>>
>> bool example_suspend_again(void)
>> {
>>       int error, monitor_result;
>>
>>       if (!wakeup_reason_is_polling())
>>               return false;
>>
>>       dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));
>
> I'm not sure you need the first argument here.  Also, if the array is
> NULL terminated, you won't need the third one.
>

It was to pass the parameter to device_resume(), pm_dev_err(),
device_complete(), and device_prepare(), which have been using the
state value. However, as it is supposed to be used in the context of
suspend_devices_and_enter(), I'll remove it and assume PMSG_SUSPEND
and PMSG_RESUME are in effect.

For the array style, do we generally prefer to use NULL at the end
rather than supplying the size? If so, I'll change the array style.

>>       resume_console();
>>
>>       monitor_result = monitor_something();
>>
>>       suspend_console();
>>       error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));
>
> Same here.
>
>>       if (error || monitor_result == ERROR)
>>               return false;
>>
>>       return true;
>> }
>
> That said, I don't like this patch.  The reason is that you call
> suspend_enter() is a loop and it calls the _noirq() callbacks for
> _all_ devices and now you're going to only call .resume() and
> .suspend() for several selected devices.  This is not too consistent.
>
> I wonder if those devices needed for .suspend_again() to work on
> your platform could be resumed and suspended by their drivers'
> _noirq() callbacks?

For now, we need to access PMIC, Fuel-Gauge, Charger (implemented as
regulators), RTC, ADC (w/ hwmon), and UART(drivers/tty/serial).
For PMIC, Fuel-Gauge, Charger, and RTC, they will work with no_irq
callbacks without any modification.
ADC will work if we just let it suspend and resume with no_irq callbacks.

However, for UART/serial, although it is only used for debugging, it
wouldn't be easy and clean to enable only with no_irq callbacks. If we
can keep observing the console with suspend_console_enabled=0, it
would be much helpful.
In order to let some UART/serial drivers work with no-irq callbacks,
we'd need reconstruction in drivers/tty/serial/serial_core.c, too.

Anyway, as the effect of not having partial suspend/resume is limited
for now (support for UART/serial can wait), I may defer this part and
wait for better and clean methods. And yes, because of the possibility
of having dependencies between devices, the inconsistency you've
mentioned could be an issue for some systems. However, because such
dependencies are not explicitly expressed to kernel while the platform
should know about them, partial_suspend/resume was supposed to be
called by the platform's suspend_again ops.

>
> Rafael
>
>


Cheers!

- MyungJoo
-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-17 20:52       ` Rafael J. Wysocki
@ 2011-05-18  5:58         ` MyungJoo Ham
  2011-05-18  5:58         ` MyungJoo Ham
  1 sibling, 0 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-18  5:58 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park, linux-pm

2011/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> On Tuesday, May 17, 2011, MyungJoo Ham wrote:
>> A usage example is:
>>
>> bool example_suspend_again(void)
>> {
>>       int error, monitor_result;
>>
>>       if (!wakeup_reason_is_polling())
>>               return false;
>>
>>       dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));
>
> I'm not sure you need the first argument here.  Also, if the array is
> NULL terminated, you won't need the third one.
>

It was to pass the parameter to device_resume(), pm_dev_err(),
device_complete(), and device_prepare(), which have been using the
state value. However, as it is supposed to be used in the context of
suspend_devices_and_enter(), I'll remove it and assume PMSG_SUSPEND
and PMSG_RESUME are in effect.

For the array style, do we generally prefer to use NULL at the end
rather than supplying the size? If so, I'll change the array style.

>>       resume_console();
>>
>>       monitor_result = monitor_something();
>>
>>       suspend_console();
>>       error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));
>
> Same here.
>
>>       if (error || monitor_result == ERROR)
>>               return false;
>>
>>       return true;
>> }
>
> That said, I don't like this patch.  The reason is that you call
> suspend_enter() is a loop and it calls the _noirq() callbacks for
> _all_ devices and now you're going to only call .resume() and
> .suspend() for several selected devices.  This is not too consistent.
>
> I wonder if those devices needed for .suspend_again() to work on
> your platform could be resumed and suspended by their drivers'
> _noirq() callbacks?

For now, we need to access PMIC, Fuel-Gauge, Charger (implemented as
regulators), RTC, ADC (w/ hwmon), and UART(drivers/tty/serial).
For PMIC, Fuel-Gauge, Charger, and RTC, they will work with no_irq
callbacks without any modification.
ADC will work if we just let it suspend and resume with no_irq callbacks.

However, for UART/serial, although it is only used for debugging, it
wouldn't be easy and clean to enable only with no_irq callbacks. If we
can keep observing the console with suspend_console_enabled=0, it
would be much helpful.
In order to let some UART/serial drivers work with no-irq callbacks,
we'd need reconstruction in drivers/tty/serial/serial_core.c, too.

Anyway, as the effect of not having partial suspend/resume is limited
for now (support for UART/serial can wait), I may defer this part and
wait for better and clean methods. And yes, because of the possibility
of having dependencies between devices, the inconsistency you've
mentioned could be an issue for some systems. However, because such
dependencies are not explicitly expressed to kernel while the platform
should know about them, partial_suspend/resume was supposed to be
called by the platform's suspend_again ops.

>
> Rafael
>
>


Cheers!

- MyungJoo
-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-17 20:40     ` Rafael J. Wysocki
@ 2011-05-18  9:07       ` MyungJoo Ham
  2011-05-18 20:20         ` Rafael J. Wysocki
  2011-05-18 20:20         ` Rafael J. Wysocki
  2011-05-18  9:07       ` MyungJoo Ham
  1 sibling, 2 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-18  9:07 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-pm, linux-kernel, Len Brown, Pavel Machek,
	Greg Kroah-Hartman, kyungmin.park

2011/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> On Tuesday, May 17, 2011, MyungJoo Ham wrote:
[]
>> -static int suspend_enter(suspend_state_t state)
>> +static int suspend_enter(suspend_state_t state, bool *wakeup_pending)
>
> You don't need to use the wakeup_pending argument at all.  I think you
> shouldn't use it even, because in theory there may be a wakeup event after
> you've called pm_wakeup_pending() in suspend_enter() and in that case
> you should break the loop too.

In the case where:
- at the first instance of suspend_enter, pm_wakeup_pending() returns false.
- after suspend_ops->enter(state), the pm_wakeup_pending() "wants" to
return true.
- however, suspend_again forces to loop again.
- then, at the second instance of suspend_enter, pm_wakeup_pending()
returns true.
- the suspend_again's loop breaks.

Although it did not break the loop at the first while, it breaks
without calling suspend_ops->enter again anyway.

[]
>
> So I would simply call pm_wakeup_pending() here again.
>

Besides, if we simply call pm_wakeup_pending() again at there, the
loop will NOT break with pm_wakeup_pending() is true at the first call
inside of suspend_enter(). The function pm_wakeup_pending() clears out
the pending wakeup at each call; thus, in the following example, the
loop will not break:

- At the first instance of suspend_enter in the suspend-again loop,
pm_wakeup_pending() returns true in suspend_enter().
- suspend_enter() returns without error.
- pm_wakeup_pending() is called again at the while statement along
with suspend_ops->suspend_again().
- pm_wakeup_pending() now returns false because it has already
returned true before and cleared "events_check_enabled"
- the loop continues.

Because pm_save_wakeup_count will return true only once for each
wakeup-preventing-event, the result of pm_wakeup_pending in
suspend_enter() should be relayed outside to the loop anyway.

>>
>>   Resume_devices:
>>       suspend_test_start();
>>
>
> Thanks,
> Rafael
>


Cheers!

- MyungJoo
-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-17 20:40     ` Rafael J. Wysocki
  2011-05-18  9:07       ` MyungJoo Ham
@ 2011-05-18  9:07       ` MyungJoo Ham
  1 sibling, 0 replies; 22+ messages in thread
From: MyungJoo Ham @ 2011-05-18  9:07 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park, linux-pm

2011/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> On Tuesday, May 17, 2011, MyungJoo Ham wrote:
[]
>> -static int suspend_enter(suspend_state_t state)
>> +static int suspend_enter(suspend_state_t state, bool *wakeup_pending)
>
> You don't need to use the wakeup_pending argument at all.  I think you
> shouldn't use it even, because in theory there may be a wakeup event after
> you've called pm_wakeup_pending() in suspend_enter() and in that case
> you should break the loop too.

In the case where:
- at the first instance of suspend_enter, pm_wakeup_pending() returns false.
- after suspend_ops->enter(state), the pm_wakeup_pending() "wants" to
return true.
- however, suspend_again forces to loop again.
- then, at the second instance of suspend_enter, pm_wakeup_pending()
returns true.
- the suspend_again's loop breaks.

Although it did not break the loop at the first while, it breaks
without calling suspend_ops->enter again anyway.

[]
>
> So I would simply call pm_wakeup_pending() here again.
>

Besides, if we simply call pm_wakeup_pending() again at there, the
loop will NOT break with pm_wakeup_pending() is true at the first call
inside of suspend_enter(). The function pm_wakeup_pending() clears out
the pending wakeup at each call; thus, in the following example, the
loop will not break:

- At the first instance of suspend_enter in the suspend-again loop,
pm_wakeup_pending() returns true in suspend_enter().
- suspend_enter() returns without error.
- pm_wakeup_pending() is called again at the while statement along
with suspend_ops->suspend_again().
- pm_wakeup_pending() now returns false because it has already
returned true before and cleared "events_check_enabled"
- the loop continues.

Because pm_save_wakeup_count will return true only once for each
wakeup-preventing-event, the result of pm_wakeup_pending in
suspend_enter() should be relayed outside to the loop anyway.

>>
>>   Resume_devices:
>>       suspend_test_start();
>>
>
> Thanks,
> Rafael
>


Cheers!

- MyungJoo
-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-18  5:58         ` MyungJoo Ham
  2011-05-18 20:18           ` Rafael J. Wysocki
@ 2011-05-18 20:18           ` Rafael J. Wysocki
  1 sibling, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2011-05-18 20:18 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: linux-pm, linux-kernel, Len Brown, Pavel Machek,
	Greg Kroah-Hartman, kyungmin.park

On Wednesday, May 18, 2011, MyungJoo Ham wrote:
> 2011/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Tuesday, May 17, 2011, MyungJoo Ham wrote:
> >> A usage example is:
> >>
> >> bool example_suspend_again(void)
> >> {
> >>       int error, monitor_result;
> >>
> >>       if (!wakeup_reason_is_polling())
> >>               return false;
> >>
> >>       dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));
> >
> > I'm not sure you need the first argument here.  Also, if the array is
> > NULL terminated, you won't need the third one.
> >
> 
> It was to pass the parameter to device_resume(), pm_dev_err(),
> device_complete(), and device_prepare(), which have been using the
> state value. However, as it is supposed to be used in the context of
> suspend_devices_and_enter(), I'll remove it and assume PMSG_SUSPEND
> and PMSG_RESUME are in effect.
> 
> For the array style, do we generally prefer to use NULL at the end
> rather than supplying the size? If so, I'll change the array style.
> 
> >>       resume_console();
> >>
> >>       monitor_result = monitor_something();
> >>
> >>       suspend_console();
> >>       error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));
> >
> > Same here.
> >
> >>       if (error || monitor_result == ERROR)
> >>               return false;
> >>
> >>       return true;
> >> }
> >
> > That said, I don't like this patch.  The reason is that you call
> > suspend_enter() is a loop and it calls the _noirq() callbacks for
> > _all_ devices and now you're going to only call .resume() and
> > .suspend() for several selected devices.  This is not too consistent.
> >
> > I wonder if those devices needed for .suspend_again() to work on
> > your platform could be resumed and suspended by their drivers'
> > _noirq() callbacks?
> 
> For now, we need to access PMIC, Fuel-Gauge, Charger (implemented as
> regulators), RTC, ADC (w/ hwmon), and UART(drivers/tty/serial).
> For PMIC, Fuel-Gauge, Charger, and RTC, they will work with no_irq
> callbacks without any modification.
> ADC will work if we just let it suspend and resume with no_irq callbacks.
> 
> However, for UART/serial, although it is only used for debugging, it
> wouldn't be easy and clean to enable only with no_irq callbacks. If we
> can keep observing the console with suspend_console_enabled=0, it
> would be much helpful.
> In order to let some UART/serial drivers work with no-irq callbacks,
> we'd need reconstruction in drivers/tty/serial/serial_core.c, too.
> 
> Anyway, as the effect of not having partial suspend/resume is limited
> for now (support for UART/serial can wait), I may defer this part and
> wait for better and clean methods. And yes, because of the possibility
> of having dependencies between devices, the inconsistency you've
> mentioned could be an issue for some systems. However, because such
> dependencies are not explicitly expressed to kernel while the platform
> should know about them, partial_suspend/resume was supposed to be
> called by the platform's suspend_again ops.

This is a difficult problem in general.  One solution of it I had been
considering some time ago might be to use two or more "suspend lists"
of devices instead of the single dpm_list we have right now.

Namely, suppose we have an array of lists that are idexed by priority
such that devices in the list of priority 0 are suspended first,
devices in the list of priority 1 are suspended next and so on (the
oredering of resume will have to be reversed).  Then, your new code may
request to only resume the highest pririty list and then call .suspend_again()
and possiblty continue with normal resume if it returns "false".
[Please note that you need to call .resume_noirq() for all devices anyway,
because device interrupts cannot be enabled before calling any drivers'
.resume_noirq() in case .suspend_again() returns "false".]

Devices will be added to the priority 0 list during registration and
there will be a special function to move a device to another list
(increase its suspend pririty) that will ensure the requisite dependencies
are met (i.e. the suspend priority of a device cannot be greater than
the suspend priority of its parent, so that the parent would be suspended
first).

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users.
  2011-05-18  5:58         ` MyungJoo Ham
@ 2011-05-18 20:18           ` Rafael J. Wysocki
  2011-05-18 20:18           ` Rafael J. Wysocki
  1 sibling, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2011-05-18 20:18 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park, linux-pm

On Wednesday, May 18, 2011, MyungJoo Ham wrote:
> 2011/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Tuesday, May 17, 2011, MyungJoo Ham wrote:
> >> A usage example is:
> >>
> >> bool example_suspend_again(void)
> >> {
> >>       int error, monitor_result;
> >>
> >>       if (!wakeup_reason_is_polling())
> >>               return false;
> >>
> >>       dpm_partial_resume(PMSG_RESUME, devs, ARRAY_SIZE(devs));
> >
> > I'm not sure you need the first argument here.  Also, if the array is
> > NULL terminated, you won't need the third one.
> >
> 
> It was to pass the parameter to device_resume(), pm_dev_err(),
> device_complete(), and device_prepare(), which have been using the
> state value. However, as it is supposed to be used in the context of
> suspend_devices_and_enter(), I'll remove it and assume PMSG_SUSPEND
> and PMSG_RESUME are in effect.
> 
> For the array style, do we generally prefer to use NULL at the end
> rather than supplying the size? If so, I'll change the array style.
> 
> >>       resume_console();
> >>
> >>       monitor_result = monitor_something();
> >>
> >>       suspend_console();
> >>       error = dpm_partial_suspend(PMSG_SUSPEND, devs, ARRAY_SIZE(devs));
> >
> > Same here.
> >
> >>       if (error || monitor_result == ERROR)
> >>               return false;
> >>
> >>       return true;
> >> }
> >
> > That said, I don't like this patch.  The reason is that you call
> > suspend_enter() is a loop and it calls the _noirq() callbacks for
> > _all_ devices and now you're going to only call .resume() and
> > .suspend() for several selected devices.  This is not too consistent.
> >
> > I wonder if those devices needed for .suspend_again() to work on
> > your platform could be resumed and suspended by their drivers'
> > _noirq() callbacks?
> 
> For now, we need to access PMIC, Fuel-Gauge, Charger (implemented as
> regulators), RTC, ADC (w/ hwmon), and UART(drivers/tty/serial).
> For PMIC, Fuel-Gauge, Charger, and RTC, they will work with no_irq
> callbacks without any modification.
> ADC will work if we just let it suspend and resume with no_irq callbacks.
> 
> However, for UART/serial, although it is only used for debugging, it
> wouldn't be easy and clean to enable only with no_irq callbacks. If we
> can keep observing the console with suspend_console_enabled=0, it
> would be much helpful.
> In order to let some UART/serial drivers work with no-irq callbacks,
> we'd need reconstruction in drivers/tty/serial/serial_core.c, too.
> 
> Anyway, as the effect of not having partial suspend/resume is limited
> for now (support for UART/serial can wait), I may defer this part and
> wait for better and clean methods. And yes, because of the possibility
> of having dependencies between devices, the inconsistency you've
> mentioned could be an issue for some systems. However, because such
> dependencies are not explicitly expressed to kernel while the platform
> should know about them, partial_suspend/resume was supposed to be
> called by the platform's suspend_again ops.

This is a difficult problem in general.  One solution of it I had been
considering some time ago might be to use two or more "suspend lists"
of devices instead of the single dpm_list we have right now.

Namely, suppose we have an array of lists that are idexed by priority
such that devices in the list of priority 0 are suspended first,
devices in the list of priority 1 are suspended next and so on (the
oredering of resume will have to be reversed).  Then, your new code may
request to only resume the highest pririty list and then call .suspend_again()
and possiblty continue with normal resume if it returns "false".
[Please note that you need to call .resume_noirq() for all devices anyway,
because device interrupts cannot be enabled before calling any drivers'
.resume_noirq() in case .suspend_again() returns "false".]

Devices will be added to the priority 0 list during registration and
there will be a special function to move a device to another list
(increase its suspend pririty) that will ensure the requisite dependencies
are met (i.e. the suspend priority of a device cannot be greater than
the suspend priority of its parent, so that the parent would be suspended
first).

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-18  9:07       ` MyungJoo Ham
  2011-05-18 20:20         ` Rafael J. Wysocki
@ 2011-05-18 20:20         ` Rafael J. Wysocki
  1 sibling, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2011-05-18 20:20 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: linux-pm, linux-kernel, Len Brown, Pavel Machek,
	Greg Kroah-Hartman, kyungmin.park

On Wednesday, May 18, 2011, MyungJoo Ham wrote:
> 2011/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Tuesday, May 17, 2011, MyungJoo Ham wrote:
> []
> >> -static int suspend_enter(suspend_state_t state)
> >> +static int suspend_enter(suspend_state_t state, bool *wakeup_pending)
> >
> > You don't need to use the wakeup_pending argument at all.  I think you
> > shouldn't use it even, because in theory there may be a wakeup event after
> > you've called pm_wakeup_pending() in suspend_enter() and in that case
> > you should break the loop too.
> 
> In the case where:
> - at the first instance of suspend_enter, pm_wakeup_pending() returns false.
> - after suspend_ops->enter(state), the pm_wakeup_pending() "wants" to
> return true.
> - however, suspend_again forces to loop again.
> - then, at the second instance of suspend_enter, pm_wakeup_pending()
> returns true.
> - the suspend_again's loop breaks.
> 
> Although it did not break the loop at the first while, it breaks
> without calling suspend_ops->enter again anyway.
> 
> []
> >
> > So I would simply call pm_wakeup_pending() here again.
> >
> 
> Besides, if we simply call pm_wakeup_pending() again at there, the
> loop will NOT break with pm_wakeup_pending() is true at the first call
> inside of suspend_enter(). The function pm_wakeup_pending() clears out
> the pending wakeup at each call;

Ah, that's correct, sorry.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops.
  2011-05-18  9:07       ` MyungJoo Ham
@ 2011-05-18 20:20         ` Rafael J. Wysocki
  2011-05-18 20:20         ` Rafael J. Wysocki
  1 sibling, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2011-05-18 20:20 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: Len Brown, Greg Kroah-Hartman, linux-kernel, kyungmin.park, linux-pm

On Wednesday, May 18, 2011, MyungJoo Ham wrote:
> 2011/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Tuesday, May 17, 2011, MyungJoo Ham wrote:
> []
> >> -static int suspend_enter(suspend_state_t state)
> >> +static int suspend_enter(suspend_state_t state, bool *wakeup_pending)
> >
> > You don't need to use the wakeup_pending argument at all.  I think you
> > shouldn't use it even, because in theory there may be a wakeup event after
> > you've called pm_wakeup_pending() in suspend_enter() and in that case
> > you should break the loop too.
> 
> In the case where:
> - at the first instance of suspend_enter, pm_wakeup_pending() returns false.
> - after suspend_ops->enter(state), the pm_wakeup_pending() "wants" to
> return true.
> - however, suspend_again forces to loop again.
> - then, at the second instance of suspend_enter, pm_wakeup_pending()
> returns true.
> - the suspend_again's loop breaks.
> 
> Although it did not break the loop at the first while, it breaks
> without calling suspend_ops->enter again anyway.
> 
> []
> >
> > So I would simply call pm_wakeup_pending() here again.
> >
> 
> Besides, if we simply call pm_wakeup_pending() again at there, the
> loop will NOT break with pm_wakeup_pending() is true at the first call
> inside of suspend_enter(). The function pm_wakeup_pending() clears out
> the pending wakeup at each call;

Ah, that's correct, sorry.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2011-05-18 20:20 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-11  5:18 [RFC PATCH v3 1/2] PM / Core: suspend_again callback for suspend_ops MyungJoo Ham
2011-05-11  5:18 ` MyungJoo Ham
2011-05-11  5:18 ` [RFC PATCH v3 2/2] PM / Core: partial resume/suspend API for suspend_again users MyungJoo Ham
2011-05-11  5:18   ` MyungJoo Ham
2011-05-12  6:19 ` [RFC PATCH v3 1/2] PM / Core: suspend_again callback for suspend_ops Pavel Machek
2011-05-17  4:59   ` [PATCH v4 " MyungJoo Ham
2011-05-17  4:59     ` [PATCH v4 2/2] PM / Core: partial resume/suspend API for suspend_again users MyungJoo Ham
2011-05-17 20:52       ` Rafael J. Wysocki
2011-05-18  5:58         ` MyungJoo Ham
2011-05-18  5:58         ` MyungJoo Ham
2011-05-18 20:18           ` Rafael J. Wysocki
2011-05-18 20:18           ` Rafael J. Wysocki
2011-05-17 20:52       ` Rafael J. Wysocki
2011-05-17  4:59     ` MyungJoo Ham
2011-05-17 20:40     ` [PATCH v4 1/2] PM / Core: suspend_again callback for suspend_ops Rafael J. Wysocki
2011-05-17 20:40     ` Rafael J. Wysocki
2011-05-18  9:07       ` MyungJoo Ham
2011-05-18 20:20         ` Rafael J. Wysocki
2011-05-18 20:20         ` Rafael J. Wysocki
2011-05-18  9:07       ` MyungJoo Ham
2011-05-17  4:59   ` MyungJoo Ham
2011-05-12  6:19 ` [RFC PATCH v3 " Pavel Machek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.