linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag
@ 2012-03-13  0:23 Rafael J. Wysocki
  2012-03-13  0:24 ` [PATCH 1/6] PM / Domains: Fix handling of wakeup devices during system resume Rafael J. Wysocki
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13  0:23 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

Hi all,

The following patches fix system suspend/resume and hibernation device
callback routines in the generic PM domains code and add an "always on" flag
for devices allowing their drivers to protect them from surprise power
removals.

[1/6] - Fix system resume of devices involved in system wakeup.
[2/6] - Fix hibernation restore of devices.
[3/6] - Introduce "always on" flag for "special" devices in PM domains.
[4/6] - Make TMU clocksource driver use the "always on" flag.
[5/6] - Make CMT clocksource driver use the "always on" flag.
[6/6] - Make MTU2 clocksource driver use the "always on" flag.

All of them are v3.4 material if there are no objections.

Thanks,
Rafael

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

* [PATCH 1/6] PM / Domains: Fix handling of wakeup devices during system resume
  2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
@ 2012-03-13  0:24 ` Rafael J. Wysocki
  2012-03-13  0:24 ` [PATCH 2/6] PM / Domains: Fix hibernation restore of devices Rafael J. Wysocki
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13  0:24 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

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

During system suspend pm_genpd_suspend_noirq() checks if the given
device is in a wakeup path (i.e. it appears to be needed for one or
more wakeup devices to work or is a wakeup device itself) and if it
needs to be "active" for wakeup to work.  If that is the case, the
function returns 0 without incrementing the device domain's counter
of suspended devices and without executing genpd_stop_dev() for the
device.  In consequence, the device is not stopped (e.g. its clock
isn't disabled) and power is always supplied to its domain in the
resulting system sleep state.

However, pm_genpd_resume_noirq() doesn't repeat that check and it
runs genpd_start_dev() and decrements the domain's counter of
suspended devices even for the wakeup device that weren't stopped by
pm_genpd_suspend_noirq().  As a result, the start callback may be run
unnecessarily for them and their domains' counters of suspended
devices may become negative.  Both outcomes aren't desirable, so fix
pm_genpd_resume_noirq() to look for wakeup devices that might not be
stopped by during system suspend.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -896,7 +896,8 @@ static int pm_genpd_resume_noirq(struct
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	if (genpd->suspend_power_off)
+	if (genpd->suspend_power_off
+	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
 		return 0;
 
 	/*


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

* [PATCH 2/6] PM / Domains: Fix hibernation restore of devices
  2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
  2012-03-13  0:24 ` [PATCH 1/6] PM / Domains: Fix handling of wakeup devices during system resume Rafael J. Wysocki
@ 2012-03-13  0:24 ` Rafael J. Wysocki
  2012-03-13 21:32   ` [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2 Rafael J. Wysocki
  2012-03-13  0:25 ` [PATCH 3/6] PM / Domains: Introduce "always on" device flag Rafael J. Wysocki
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13  0:24 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

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

During resume from hibernation pm_genpd_restore_noirq() has to
deal with software state left by pm_genpd_suspend_noirq() and
unknown hardware state (the boot kernel may leave all PM domains and
devices in arbitrary states).  For this reason, make it attempt to
power cycle each domain when before resuming its first device to
possibly get rid of any unwanted hardware state that may interfere
with genpd_start_dev() later on.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -770,8 +770,10 @@ static int pm_genpd_prepare(struct devic
 
 	genpd_acquire_lock(genpd);
 
-	if (genpd->prepared_count++ == 0)
+	if (genpd->prepared_count++ == 0) {
+		genpd->suspended_count = 0;
 		genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
+	}
 
 	genpd_release_lock(genpd);
 
@@ -1103,20 +1105,24 @@ static int pm_genpd_restore_noirq(struct
 	 * Since all of the "noirq" callbacks are executed sequentially, it is
 	 * guaranteed that this function will never run twice in parallel for
 	 * the same PM domain, so it is not necessary to use locking here.
+	 *
+	 * At this point suspended_count == 0 means this routing is being run
+	 * for the first time for the given domain in the present cycle.
 	 */
-	genpd->status = GPD_STATE_POWER_OFF;
-	if (genpd->suspend_power_off) {
+	if (genpd->suspended_count++ == 0) {
 		/*
-		 * The boot kernel might put the domain into the power on state,
-		 * so make sure it really is powered off.
+		 * The boot kernel might put the domain into arbitrary state,
+		 * so make sure it is powered off.
 		 */
+		genpd->status = GPD_STATE_POWER_OFF;
 		if (genpd->power_off)
 			genpd->power_off(genpd);
-		return 0;
 	}
 
+	if (genpd->suspend_power_off)
+		return 0;
+
 	pm_genpd_poweron(genpd);
-	genpd->suspended_count--;
 
 	return genpd_start_dev(genpd, dev);
 }
@@ -1623,7 +1629,6 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->poweroff_task = NULL;
 	genpd->resume_count = 0;
 	genpd->device_count = 0;
-	genpd->suspended_count = 0;
 	genpd->max_off_time_ns = -1;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
 	genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;


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

* [PATCH 3/6] PM / Domains: Introduce "always on" device flag
  2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
  2012-03-13  0:24 ` [PATCH 1/6] PM / Domains: Fix handling of wakeup devices during system resume Rafael J. Wysocki
  2012-03-13  0:24 ` [PATCH 2/6] PM / Domains: Fix hibernation restore of devices Rafael J. Wysocki
@ 2012-03-13  0:25 ` Rafael J. Wysocki
  2012-03-13 21:34   ` [Update][PATCH " Rafael J. Wysocki
  2012-03-13  0:26 ` [PATCH 4/6] PM / shmobile: Make TMU driver use pm_genpd_dev_always_on() Rafael J. Wysocki
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13  0:25 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

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

The TMU device on the Mackerel board belongs to the A4R power domain
and loses power when the domain is turned off.  Unfortunately, the
TMU driver is not prepared to cope with such situations and crashes
the system when that happens.  To work around this problem introduce
a new helper function, pm_genpd_dev_always_on(), allowing a device
driver to mark its device as "always on" in case it belongs to a PM
domain, which will make the generic PM domains core code avoid
powering off the domain containing the device, both at run time and
during system suspend.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   35 ++++++++++++++++++++++++++++++-----
 include/linux/pm_domain.h   |    3 +++
 2 files changed, 33 insertions(+), 5 deletions(-)

Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -372,7 +372,7 @@ static int pm_genpd_poweroff(struct gene
 	not_suspended = 0;
 	list_for_each_entry(pdd, &genpd->dev_list, list_node)
 		if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
-		    || pdd->dev->power.irq_safe))
+		    || pdd->dev->power.irq_safe || to_gpd_data(pdd)->always_on))
 			not_suspended++;
 
 	if (not_suspended > genpd->in_progress)
@@ -509,6 +509,9 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
+	if (dev_gpd_data(dev)->always_on)
+		return -EBUSY;
+
 	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
 	if (stop_ok && !stop_ok(dev))
 		return -EBUSY;
@@ -865,7 +868,7 @@ static int pm_genpd_suspend_noirq(struct
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	if (genpd->suspend_power_off
+	if (genpd->suspend_power_off || dev_gpd_data(dev)->always_on
 	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
 		return 0;
 
@@ -898,7 +901,7 @@ static int pm_genpd_resume_noirq(struct
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	if (genpd->suspend_power_off
+	if (genpd->suspend_power_off || dev_gpd_data(dev)->always_on
 	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
 		return 0;
 
@@ -1018,7 +1021,8 @@ static int pm_genpd_freeze_noirq(struct
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : genpd_stop_dev(genpd, dev);
+	return genpd->suspend_power_off || dev_gpd_data(dev)->always_on ?
+		0 : genpd_stop_dev(genpd, dev);
 }
 
 /**
@@ -1038,7 +1042,8 @@ static int pm_genpd_thaw_noirq(struct de
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : genpd_start_dev(genpd, dev);
+	return genpd->suspend_power_off || dev_gpd_data(dev)->always_on ?
+		0 : genpd_start_dev(genpd, dev);
 }
 
 /**
@@ -1288,6 +1293,26 @@ int pm_genpd_remove_device(struct generi
 }
 
 /**
+ * pm_genpd_dev_always_on - Set/unset the "always on" flag for a given device.
+ * @dev: Device to set/unset the flag for.
+ * @val: The new value of the device's "always on" flag.
+ */
+void pm_genpd_dev_always_on(struct device *dev, bool val)
+{
+	struct pm_subsys_data *psd;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	psd = dev_to_psd(dev);
+	if (psd && psd->domain_data)
+		to_gpd_data(psd->domain_data)->always_on = val;
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_dev_always_on);
+
+/**
  * pm_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
  * @genpd: Master PM domain to add the subdomain to.
  * @subdomain: Subdomain to be added.
Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -97,6 +97,7 @@ struct generic_pm_domain_data {
 	struct gpd_dev_ops ops;
 	struct gpd_timing_data td;
 	bool need_restore;
+	bool always_on;
 };
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
@@ -125,6 +126,7 @@ static inline int pm_genpd_add_device(st
 
 extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 				  struct device *dev);
+extern void pm_genpd_dev_always_on(struct device *dev, bool val);
 extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
@@ -167,6 +169,7 @@ static inline int pm_genpd_remove_device
 {
 	return -ENOSYS;
 }
+static inline void pm_genpd_dev_always_on(struct device *dev, bool val) {}
 static inline int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 					 struct generic_pm_domain *new_sd)
 {


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

* [PATCH 4/6] PM / shmobile: Make TMU driver use pm_genpd_dev_always_on()
  2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2012-03-13  0:25 ` [PATCH 3/6] PM / Domains: Introduce "always on" device flag Rafael J. Wysocki
@ 2012-03-13  0:26 ` Rafael J. Wysocki
  2012-03-13  0:27 ` [PATCH 5/6] PM / shmobile: Make CMT " Rafael J. Wysocki
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13  0:26 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

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

Make the TMU clocksource driver mark its device as "always on"
using pm_genpd_dev_always_on() to protect it from surprise power
removals and make sh7372_add_standard_devices() add TMU devices on
sh7372 to the A4R power domain so that their "always on" flags
are taken into account as appropriate.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/setup-sh7372.c |    2 ++
 drivers/clocksource/sh_tmu.c          |    4 ++++
 2 files changed, 6 insertions(+)

Index: linux/drivers/clocksource/sh_tmu.c
===================================================================
--- linux.orig/drivers/clocksource/sh_tmu.c
+++ linux/drivers/clocksource/sh_tmu.c
@@ -32,6 +32,7 @@
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/pm_domain.h>
 
 struct sh_tmu_priv {
 	void __iomem *mapbase;
@@ -410,6 +411,9 @@ static int __devinit sh_tmu_probe(struct
 	struct sh_tmu_priv *p = platform_get_drvdata(pdev);
 	int ret;
 
+	if (!is_early_platform_device(pdev))
+		pm_genpd_dev_always_on(&pdev->dev, true);
+
 	if (p) {
 		dev_info(&pdev->dev, "kept as earlytimer\n");
 		return 0;
Index: linux/arch/arm/mach-shmobile/setup-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/setup-sh7372.c
+++ linux/arch/arm/mach-shmobile/setup-sh7372.c
@@ -1043,6 +1043,8 @@ void __init sh7372_add_standard_devices(
 	sh7372_add_device_to_domain(&sh7372_a4r, &veu2_device);
 	sh7372_add_device_to_domain(&sh7372_a4r, &veu3_device);
 	sh7372_add_device_to_domain(&sh7372_a4r, &jpu_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &tmu00_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &tmu01_device);
 }
 
 void __init sh7372_add_early_devices(void)


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

* [PATCH 5/6] PM / shmobile: Make CMT driver use pm_genpd_dev_always_on()
  2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2012-03-13  0:26 ` [PATCH 4/6] PM / shmobile: Make TMU driver use pm_genpd_dev_always_on() Rafael J. Wysocki
@ 2012-03-13  0:27 ` Rafael J. Wysocki
  2012-03-13  0:28 ` [PATCH 6/6] PM / shmobile: Make MTU2 " Rafael J. Wysocki
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13  0:27 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

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

Make the CMT clocksource driver mark its device as "always on"
using pm_genpd_dev_always_on() to protect it from surprise power
removals.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/clocksource/sh_cmt.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux/drivers/clocksource/sh_cmt.c
===================================================================
--- linux.orig/drivers/clocksource/sh_cmt.c
+++ linux/drivers/clocksource/sh_cmt.c
@@ -32,6 +32,7 @@
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/pm_domain.h>
 
 struct sh_cmt_priv {
 	void __iomem *mapbase;
@@ -689,6 +690,9 @@ static int __devinit sh_cmt_probe(struct
 	struct sh_cmt_priv *p = platform_get_drvdata(pdev);
 	int ret;
 
+	if (!is_early_platform_device(pdev))
+		pm_genpd_dev_always_on(&pdev->dev, true);
+
 	if (p) {
 		dev_info(&pdev->dev, "kept as earlytimer\n");
 		return 0;


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

* [PATCH 6/6] PM / shmobile: Make MTU2 driver use pm_genpd_dev_always_on()
  2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2012-03-13  0:27 ` [PATCH 5/6] PM / shmobile: Make CMT " Rafael J. Wysocki
@ 2012-03-13  0:28 ` Rafael J. Wysocki
  2012-03-13  1:27 ` [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Simon Horman
  2012-03-13  1:44 ` Paul Mundt
  7 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13  0:28 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

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

Make the MTU2 clocksource driver mark its device as "always on"
using pm_genpd_dev_always_on() to protect it from surprise power
removals.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/clocksource/sh_mtu2.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux/drivers/clocksource/sh_mtu2.c
===================================================================
--- linux.orig/drivers/clocksource/sh_mtu2.c
+++ linux/drivers/clocksource/sh_mtu2.c
@@ -31,6 +31,7 @@
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/pm_domain.h>
 
 struct sh_mtu2_priv {
 	void __iomem *mapbase;
@@ -306,6 +307,9 @@ static int __devinit sh_mtu2_probe(struc
 	struct sh_mtu2_priv *p = platform_get_drvdata(pdev);
 	int ret;
 
+	if (!is_early_platform_device(pdev))
+		pm_genpd_dev_always_on(&pdev->dev, true);
+
 	if (p) {
 		dev_info(&pdev->dev, "kept as earlytimer\n");
 		return 0;


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

* Re: [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag
  2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2012-03-13  0:28 ` [PATCH 6/6] PM / shmobile: Make MTU2 " Rafael J. Wysocki
@ 2012-03-13  1:27 ` Simon Horman
  2012-03-13  1:44 ` Paul Mundt
  7 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2012-03-13  1:27 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux-sh list, Linux PM list, Paul Mundt, Magnus Damm, LKML,
	Cao Minh Hiep

On Tue, Mar 13, 2012 at 01:23:15AM +0100, Rafael J. Wysocki wrote:
> Hi all,
> 
> The following patches fix system suspend/resume and hibernation device
> callback routines in the generic PM domains code and add an "always on" flag
> for devices allowing their drivers to protect them from surprise power
> removals.
> 
> [1/6] - Fix system resume of devices involved in system wakeup.
> [2/6] - Fix hibernation restore of devices.
> [3/6] - Introduce "always on" flag for "special" devices in PM domains.
> [4/6] - Make TMU clocksource driver use the "always on" flag.
> [5/6] - Make CMT clocksource driver use the "always on" flag.
> [6/6] - Make MTU2 clocksource driver use the "always on" flag.
> 
> All of them are v3.4 material if there are no objections.

Hi Rafael,

I have tested that my Mackerel boots with the default config and each of
these patches applied in turn. The tests were made on head of the
linux-next branch of your linux-pm tree ("sh_mmcif / PM: Use PM QoS latency
constraint") + the "sh-sci / PM: Avoid deadlocking runtime PM" patch at the
link below.

http://marc.info/?l=linux-sh&m=133073065924863&w=4

Please let me know if there are other tests you would like run.

All patches:

Tested-by: Simon Horman <horms@verge.net.au>

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

* Re: [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag
  2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2012-03-13  1:27 ` [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Simon Horman
@ 2012-03-13  1:44 ` Paul Mundt
  7 siblings, 0 replies; 14+ messages in thread
From: Paul Mundt @ 2012-03-13  1:44 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux-sh list, Linux PM list, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

On Tue, Mar 13, 2012 at 01:23:15AM +0100, Rafael J. Wysocki wrote:
> The following patches fix system suspend/resume and hibernation device
> callback routines in the generic PM domains code and add an "always on" flag
> for devices allowing their drivers to protect them from surprise power
> removals.
> 
> [1/6] - Fix system resume of devices involved in system wakeup.
> [2/6] - Fix hibernation restore of devices.
> [3/6] - Introduce "always on" flag for "special" devices in PM domains.
> [4/6] - Make TMU clocksource driver use the "always on" flag.
> [5/6] - Make CMT clocksource driver use the "always on" flag.
> [6/6] - Make MTU2 clocksource driver use the "always on" flag.
> 
> All of them are v3.4 material if there are no objections.
> 
Looks good to me.

Acked-by: Paul Mundt <lethal@linux-sh.org>

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

* [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2
  2012-03-13  0:24 ` [PATCH 2/6] PM / Domains: Fix hibernation restore of devices Rafael J. Wysocki
@ 2012-03-13 21:32   ` Rafael J. Wysocki
  2012-03-15  9:39     ` Simon Horman
  0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13 21:32 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

On Tuesday, March 13, 2012, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> During resume from hibernation pm_genpd_restore_noirq() has to
> deal with software state left by pm_genpd_suspend_noirq() and
> unknown hardware state (the boot kernel may leave all PM domains and
> devices in arbitrary states).  For this reason, make it attempt to
> power cycle each domain when before resuming its first device to
> possibly get rid of any unwanted hardware state that may interfere
> with genpd_start_dev() later on.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

I realized that this wasn't a good idea because of patch [3/6] (we
can't power cycle domains containing "always on" devices), so I
decided to only fix the really broken things in pm_genpd_restore_noirq().

Of course, patch [3/6] also needs to be updated on top of the below to
avoid starting "always on" devices in pm_genpd_restore_noirq() (it has
to assume that they will be "always on" in the boot kernel too, but that
seems to be a reasonable expectation).

Please note that those changes only affect resume from hibernation, so
they don't invalidate the testing that has already been carried out.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / Domains: Fix hibernation restore of devices, v2

During resume from hibernation pm_genpd_restore_noirq() should only
power off domains whose suspend_power_off flags are set once and
not every time it is called for a device in the given domain.
Moreover, it shouldn't decrement genpd->suspended_count, because
that field is not touched during device freezing and therefore it is
always equal to 0 when pm_genpd_restore_noirq() runs for the first
device in the given domain.

This means pm_genpd_restore_noirq() may use genpd->suspended_count
to determine whether or not it it has been called for the domain in
question already in this cycle (it only needs to increment that
field every time it runs for this purpose) and whether or not it
should check if the domain needs to be powered off.  For that to
work, though, pm_genpd_prepare() has to clear genpd->suspended_count
when it runs for the first device in the given domain (in which case
that flag need not be cleared during domain initialization).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -764,8 +764,10 @@ static int pm_genpd_prepare(struct devic
 
 	genpd_acquire_lock(genpd);
 
-	if (genpd->prepared_count++ == 0)
+	if (genpd->prepared_count++ == 0) {
+		genpd->suspended_count = 0;
 		genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
+	}
 
 	genpd_release_lock(genpd);
 
@@ -1097,20 +1099,30 @@ static int pm_genpd_restore_noirq(struct
 	 * Since all of the "noirq" callbacks are executed sequentially, it is
 	 * guaranteed that this function will never run twice in parallel for
 	 * the same PM domain, so it is not necessary to use locking here.
+	 *
+	 * At this point suspended_count == 0 means we are being run for the
+	 * first time for the given domain in the present cycle.
 	 */
-	genpd->status = GPD_STATE_POWER_OFF;
-	if (genpd->suspend_power_off) {
+	if (genpd->suspended_count++ == 0) {
 		/*
-		 * The boot kernel might put the domain into the power on state,
-		 * so make sure it really is powered off.
+		 * The boot kernel might put the domain into arbitrary state,
+		 * so make it appear as powered off to pm_genpd_poweron(), so
+		 * that it tries to power it on in case it was really off.
 		 */
-		if (genpd->power_off)
-			genpd->power_off(genpd);
-		return 0;
+		genpd->status = GPD_STATE_POWER_OFF;
+		if (genpd->suspend_power_off) {
+			/*
+			 * If the domain was off before the hibernation, make
+			 * sure it will be off going forward.
+			 */
+			if (genpd->power_off)
+				genpd->power_off(genpd);
+
+			return 0;
+		}
 	}
 
 	pm_genpd_poweron(genpd);
-	genpd->suspended_count--;
 
 	return genpd_start_dev(genpd, dev);
 }
@@ -1617,7 +1629,6 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->poweroff_task = NULL;
 	genpd->resume_count = 0;
 	genpd->device_count = 0;
-	genpd->suspended_count = 0;
 	genpd->max_off_time_ns = -1;
 	genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
 	genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;

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

* [Update][PATCH 3/6] PM / Domains: Introduce "always on" device flag
  2012-03-13  0:25 ` [PATCH 3/6] PM / Domains: Introduce "always on" device flag Rafael J. Wysocki
@ 2012-03-13 21:34   ` Rafael J. Wysocki
  0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-13 21:34 UTC (permalink / raw)
  To: Linux-sh list
  Cc: Linux PM list, Paul Mundt, Simon Horman, Magnus Damm, LKML,
	Cao Minh Hiep

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

The TMU device on the Mackerel board belongs to the A4R power domain
and loses power when the domain is turned off.  Unfortunately, the
TMU driver is not prepared to cope with such situations and crashes
the system when that happens.  To work around this problem introduce
a new helper function, pm_genpd_dev_always_on(), allowing a device
driver to mark its device as "always on" in case it belongs to a PM
domain, which will make the generic PM domains core code avoid
powering off the domain containing the device, both at run time and
during system suspend.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Tested-by: Simon Horman <horms@verge.net.au>
Acked-by: Paul Mundt <lethal@linux-sh.org>
---

The only change with respect to the previous version is the hunk in
pm_genpd_restore_noirq() which only affects hibernation.

Thanks,
Rafael

---
 drivers/base/power/domain.c |   37 +++++++++++++++++++++++++++++++------
 include/linux/pm_domain.h   |    3 +++
 2 files changed, 34 insertions(+), 6 deletions(-)

Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -366,7 +366,7 @@ static int pm_genpd_poweroff(struct gene
 	not_suspended = 0;
 	list_for_each_entry(pdd, &genpd->dev_list, list_node)
 		if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
-		    || pdd->dev->power.irq_safe))
+		    || pdd->dev->power.irq_safe || to_gpd_data(pdd)->always_on))
 			not_suspended++;
 
 	if (not_suspended > genpd->in_progress)
@@ -503,6 +503,9 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
+	if (dev_gpd_data(dev)->always_on)
+		return -EBUSY;
+
 	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
 	if (stop_ok && !stop_ok(dev))
 		return -EBUSY;
@@ -859,7 +862,7 @@ static int pm_genpd_suspend_noirq(struct
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	if (genpd->suspend_power_off
+	if (genpd->suspend_power_off || dev_gpd_data(dev)->always_on
 	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
 		return 0;
 
@@ -892,7 +895,7 @@ static int pm_genpd_resume_noirq(struct
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	if (genpd->suspend_power_off
+	if (genpd->suspend_power_off || dev_gpd_data(dev)->always_on
 	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
 		return 0;
 
@@ -1012,7 +1015,8 @@ static int pm_genpd_freeze_noirq(struct
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : genpd_stop_dev(genpd, dev);
+	return genpd->suspend_power_off || dev_gpd_data(dev)->always_on ?
+		0 : genpd_stop_dev(genpd, dev);
 }
 
 /**
@@ -1032,7 +1036,8 @@ static int pm_genpd_thaw_noirq(struct de
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : genpd_start_dev(genpd, dev);
+	return genpd->suspend_power_off || dev_gpd_data(dev)->always_on ?
+		0 : genpd_start_dev(genpd, dev);
 }
 
 /**
@@ -1125,7 +1130,7 @@ static int pm_genpd_restore_noirq(struct
 
 	pm_genpd_poweron(genpd);
 
-	return genpd_start_dev(genpd, dev);
+	return dev_gpd_data(dev)->always_on ? 0 : genpd_start_dev(genpd, dev);
 }
 
 /**
@@ -1289,6 +1294,26 @@ int pm_genpd_remove_device(struct generi
 }
 
 /**
+ * pm_genpd_dev_always_on - Set/unset the "always on" flag for a given device.
+ * @dev: Device to set/unset the flag for.
+ * @val: The new value of the device's "always on" flag.
+ */
+void pm_genpd_dev_always_on(struct device *dev, bool val)
+{
+	struct pm_subsys_data *psd;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	psd = dev_to_psd(dev);
+	if (psd && psd->domain_data)
+		to_gpd_data(psd->domain_data)->always_on = val;
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_dev_always_on);
+
+/**
  * pm_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
  * @genpd: Master PM domain to add the subdomain to.
  * @subdomain: Subdomain to be added.
Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -97,6 +97,7 @@ struct generic_pm_domain_data {
 	struct gpd_dev_ops ops;
 	struct gpd_timing_data td;
 	bool need_restore;
+	bool always_on;
 };
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
@@ -125,6 +126,7 @@ static inline int pm_genpd_add_device(st
 
 extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 				  struct device *dev);
+extern void pm_genpd_dev_always_on(struct device *dev, bool val);
 extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
@@ -167,6 +169,7 @@ static inline int pm_genpd_remove_device
 {
 	return -ENOSYS;
 }
+static inline void pm_genpd_dev_always_on(struct device *dev, bool val) {}
 static inline int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 					 struct generic_pm_domain *new_sd)
 {

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

* Re: [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2
  2012-03-13 21:32   ` [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2 Rafael J. Wysocki
@ 2012-03-15  9:39     ` Simon Horman
  2012-03-15 21:00       ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Simon Horman @ 2012-03-15  9:39 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux-sh list, Linux PM list, Paul Mundt, Magnus Damm, LKML,
	Cao Minh Hiep

On Tue, Mar 13, 2012 at 10:32:42PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, March 13, 2012, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > During resume from hibernation pm_genpd_restore_noirq() has to
> > deal with software state left by pm_genpd_suspend_noirq() and
> > unknown hardware state (the boot kernel may leave all PM domains and
> > devices in arbitrary states).  For this reason, make it attempt to
> > power cycle each domain when before resuming its first device to
> > possibly get rid of any unwanted hardware state that may interfere
> > with genpd_start_dev() later on.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> I realized that this wasn't a good idea because of patch [3/6] (we
> can't power cycle domains containing "always on" devices), so I
> decided to only fix the really broken things in pm_genpd_restore_noirq().
> 
> Of course, patch [3/6] also needs to be updated on top of the below to
> avoid starting "always on" devices in pm_genpd_restore_noirq() (it has
> to assume that they will be "always on" in the boot kernel too, but that
> seems to be a reasonable expectation).
> 
> Please note that those changes only affect resume from hibernation, so
> they don't invalidate the testing that has already been carried out.

Hi Rafael,

sorry for such a naeive question, but if I was to test
hibernate on the Mackerel how would I achieve resume?
Is there a button I should press?

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

* Re: [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2
  2012-03-15  9:39     ` Simon Horman
@ 2012-03-15 21:00       ` Rafael J. Wysocki
  2012-03-16  0:03         ` Simon Horman
  0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-03-15 21:00 UTC (permalink / raw)
  To: Simon Horman
  Cc: Linux-sh list, Linux PM list, Paul Mundt, Magnus Damm, LKML,
	Cao Minh Hiep

On Thursday, March 15, 2012, Simon Horman wrote:
> On Tue, Mar 13, 2012 at 10:32:42PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, March 13, 2012, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > During resume from hibernation pm_genpd_restore_noirq() has to
> > > deal with software state left by pm_genpd_suspend_noirq() and
> > > unknown hardware state (the boot kernel may leave all PM domains and
> > > devices in arbitrary states).  For this reason, make it attempt to
> > > power cycle each domain when before resuming its first device to
> > > possibly get rid of any unwanted hardware state that may interfere
> > > with genpd_start_dev() later on.
> > > 
> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > I realized that this wasn't a good idea because of patch [3/6] (we
> > can't power cycle domains containing "always on" devices), so I
> > decided to only fix the really broken things in pm_genpd_restore_noirq().
> > 
> > Of course, patch [3/6] also needs to be updated on top of the below to
> > avoid starting "always on" devices in pm_genpd_restore_noirq() (it has
> > to assume that they will be "always on" in the boot kernel too, but that
> > seems to be a reasonable expectation).
> > 
> > Please note that those changes only affect resume from hibernation, so
> > they don't invalidate the testing that has already been carried out.
> 
> Hi Rafael,
> 
> sorry for such a naeive question, but if I was to test
> hibernate on the Mackerel how would I achieve resume?
> Is there a button I should press?

Hibernation is not supported on Mackerel at the moment, so you'd need to add
some platform hooks first.  Then, you'd need some persistent storage with
a swap patrition (or swap file at least) to save the image.  You'd need
to point your kernel to the image storage using the resume= and resume_offset=
command line switches.  Then, you should be able to create an image and the
kernel would look for it automatically on the next boot.

Still, I'm not sure if doing all that at this point is worth the effort.

Thanks,
Rafael

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

* Re: [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2
  2012-03-15 21:00       ` Rafael J. Wysocki
@ 2012-03-16  0:03         ` Simon Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2012-03-16  0:03 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux-sh list, Linux PM list, Paul Mundt, Magnus Damm, LKML,
	Cao Minh Hiep

On Thu, Mar 15, 2012 at 10:00:09PM +0100, Rafael J. Wysocki wrote:
> On Thursday, March 15, 2012, Simon Horman wrote:
> > On Tue, Mar 13, 2012 at 10:32:42PM +0100, Rafael J. Wysocki wrote:
> > > On Tuesday, March 13, 2012, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > > 
> > > > During resume from hibernation pm_genpd_restore_noirq() has to
> > > > deal with software state left by pm_genpd_suspend_noirq() and
> > > > unknown hardware state (the boot kernel may leave all PM domains and
> > > > devices in arbitrary states).  For this reason, make it attempt to
> > > > power cycle each domain when before resuming its first device to
> > > > possibly get rid of any unwanted hardware state that may interfere
> > > > with genpd_start_dev() later on.
> > > > 
> > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > I realized that this wasn't a good idea because of patch [3/6] (we
> > > can't power cycle domains containing "always on" devices), so I
> > > decided to only fix the really broken things in pm_genpd_restore_noirq().
> > > 
> > > Of course, patch [3/6] also needs to be updated on top of the below to
> > > avoid starting "always on" devices in pm_genpd_restore_noirq() (it has
> > > to assume that they will be "always on" in the boot kernel too, but that
> > > seems to be a reasonable expectation).
> > > 
> > > Please note that those changes only affect resume from hibernation, so
> > > they don't invalidate the testing that has already been carried out.
> > 
> > Hi Rafael,
> > 
> > sorry for such a naeive question, but if I was to test
> > hibernate on the Mackerel how would I achieve resume?
> > Is there a button I should press?
> 
> Hibernation is not supported on Mackerel at the moment, so you'd need to add
> some platform hooks first.  Then, you'd need some persistent storage with
> a swap patrition (or swap file at least) to save the image.  You'd need
> to point your kernel to the image storage using the resume= and resume_offset=
> command line switches.  Then, you should be able to create an image and the
> kernel would look for it automatically on the next boot.
> 
> Still, I'm not sure if doing all that at this point is worth the effort.

Point taken.

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

end of thread, other threads:[~2012-03-16  0:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-13  0:23 [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Rafael J. Wysocki
2012-03-13  0:24 ` [PATCH 1/6] PM / Domains: Fix handling of wakeup devices during system resume Rafael J. Wysocki
2012-03-13  0:24 ` [PATCH 2/6] PM / Domains: Fix hibernation restore of devices Rafael J. Wysocki
2012-03-13 21:32   ` [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2 Rafael J. Wysocki
2012-03-15  9:39     ` Simon Horman
2012-03-15 21:00       ` Rafael J. Wysocki
2012-03-16  0:03         ` Simon Horman
2012-03-13  0:25 ` [PATCH 3/6] PM / Domains: Introduce "always on" device flag Rafael J. Wysocki
2012-03-13 21:34   ` [Update][PATCH " Rafael J. Wysocki
2012-03-13  0:26 ` [PATCH 4/6] PM / shmobile: Make TMU driver use pm_genpd_dev_always_on() Rafael J. Wysocki
2012-03-13  0:27 ` [PATCH 5/6] PM / shmobile: Make CMT " Rafael J. Wysocki
2012-03-13  0:28 ` [PATCH 6/6] PM / shmobile: Make MTU2 " Rafael J. Wysocki
2012-03-13  1:27 ` [PATCH 0/6] PM / Domains: System suspend fixes and "always on" flag Simon Horman
2012-03-13  1:44 ` Paul Mundt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).