[PATCH 2/6] PM / Domains: Fix hibernation restore of devices

Re: [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2

Simon Horman — Fri, 16 Mar 2012 01:10:02 +0100

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

Re: [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2

Rafael J. Wysocki — Thu, 15 Mar 2012 22:00:02 +0100

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

Re: [Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2

Simon Horman — Thu, 15 Mar 2012 10:50:02 +0100

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

[Replacement][PATCH 2/6] PM / Domains: Fix hibernation restore of devices, v2

Rafael J. Wysocki — Tue, 13 Mar 2012 22:30:02 +0100

On Tuesday, March 13, 2012, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki
>
> 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

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

[PATCH 2/6] PM / Domains: Fix hibernation restore of devices

Rafael J. Wysocki — Tue, 13 Mar 2012 01:50:02 +0100

From: Rafael J. Wysocki

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

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