"reboot" swsusp mode leaves moon icon blinking

"reboot" swsusp mode leaves moon icon blinking

[Pavel Machek]

Hi!

...and cause is really simple.

During resume, we do not know that "reboot" method was used, so we
assume plaform and make the led blink...

Unfortunately I see no easy solution, and this may/will cause other
problems -- in case of broken bios and user telling us not to call
that bios, we'll call it anyway.

(Ouch and I think this is regression after 2.6.20?)

Signed-off-by: Pavel Machek <pavel@suse.cz>

								Pavel

diff --git a/kernel/power/disk.c b/kernel/power/disk.c
index 873cdf8..dee0ff4 100644
--- a/kernel/power/disk.c
+++ b/kernel/power/disk.c
@@ -241,18 +241,11 @@ static int software_resume(void)
 		goto Done;
 	}
 
-	error = platform_prepare();
-	if (error) {
-		swsusp_free();
-		goto Thaw;
-	}
-
 	pr_debug("PM: Reading swsusp image.\n");
 
 	error = swsusp_read();
 	if (error) {
 		swsusp_free();
-		platform_finish();
 		goto Thaw;
 	}
 
@@ -270,7 +263,6 @@ static int software_resume(void)
 	enable_nonboot_cpus();
  Free:
 	swsusp_free();
-	platform_finish();
 	device_resume();
 	resume_console();
  Thaw:

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

Re: "reboot" swsusp mode leaves moon icon blinking

[Johannes Weiner]

Hi,

On Tue, Mar 20, 2007 at 01:32:45PM +0100, Pavel Machek wrote:
> Date: Tue, 20 Mar 2007 13:32:45 +0100
> From: Pavel Machek <pavel@ucw.cz>
> To: "Rafael J. Wysocki" <rjw@sisk.pl>,
> 	kernel list <linux-kernel@vger.kernel.org>
> Cc: Andrew Morton <akpm@osdl.org>
> Subject: "reboot" swsusp mode leaves moon icon blinking
> 
> Hi!
> 
> ...and cause is really simple.
> 
> During resume, we do not know that "reboot" method was used, so we
> assume plaform and make the led blink...

What does the "reboot" swsusp mean?

I am having this (or a similar problem):

	echo shutdown > /sys/power/disk; echo disk > /sys/power/state

suspends normally.  But after a resume, the moon keeps blinking _and_
I don't get ACPI events anymore.  At least all ibm-acpi features refuse
to work (hotkeys, lcd-brightness, lid, ...).

I saved some /proc files (interrupts, acpi/processor/power, ...) and
compared them before the suspend and after the resume but I could not
find any differences.

> diff --git a/kernel/power/disk.c b/kernel/power/disk.c
> index 873cdf8..dee0ff4 100644
> --- a/kernel/power/disk.c
> +++ b/kernel/power/disk.c
> @@ -241,18 +241,11 @@ static int software_resume(void)
>  		goto Done;
>  	}
>  
> -	error = platform_prepare();
> -	if (error) {
> -		swsusp_free();
> -		goto Thaw;
> -	}
> -

What is platform_prepare() actually good for when _resuming_? I tried to
follow the function calls and it seems to disable a lot of thing instead
of re-enabling them...?

=Hannes

Re: "reboot" swsusp mode leaves moon icon blinking

[Rafael J. Wysocki]

On Tuesday, 20 March 2007 17:50, Johannes Weiner wrote:
> Hi,
> 
> On Tue, Mar 20, 2007 at 01:32:45PM +0100, Pavel Machek wrote:
> > Date: Tue, 20 Mar 2007 13:32:45 +0100
> > From: Pavel Machek <pavel@ucw.cz>
> > To: "Rafael J. Wysocki" <rjw@sisk.pl>,
> > 	kernel list <linux-kernel@vger.kernel.org>
> > Cc: Andrew Morton <akpm@osdl.org>
> > Subject: "reboot" swsusp mode leaves moon icon blinking
> > 
> > Hi!
> > 
> > ...and cause is really simple.
> > 
> > During resume, we do not know that "reboot" method was used, so we
> > assume plaform and make the led blink...
> 
> What does the "reboot" swsusp mean?
> 
> I am having this (or a similar problem):
> 
> 	echo shutdown > /sys/power/disk; echo disk > /sys/power/state
> 
> suspends normally.  But after a resume, the moon keeps blinking _and_
> I don't get ACPI events anymore.  At least all ibm-acpi features refuse
> to work (hotkeys, lcd-brightness, lid, ...).
> 
> I saved some /proc files (interrupts, acpi/processor/power, ...) and
> compared them before the suspend and after the resume but I could not
> find any differences.
> 
> > diff --git a/kernel/power/disk.c b/kernel/power/disk.c
> > index 873cdf8..dee0ff4 100644
> > --- a/kernel/power/disk.c
> > +++ b/kernel/power/disk.c
> > @@ -241,18 +241,11 @@ static int software_resume(void)
> >  		goto Done;
> >  	}
> >  
> > -	error = platform_prepare();
> > -	if (error) {
> > -		swsusp_free();
> > -		goto Thaw;
> > -	}
> > -
> 
> What is platform_prepare() actually good for when _resuming_? I tried to
> follow the function calls and it seems to disable a lot of thing instead
> of re-enabling them...?

Well, if you suspend using the platform mode, platform_finish() will be called
after the image has been restored (ie. by the "old" kernel).

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King

Re: "reboot" swsusp mode leaves moon icon blinking

[Rafael J. Wysocki]

On Tuesday, 20 March 2007 13:32, Pavel Machek wrote:
> Hi!
> 
> ...and cause is really simple.
> 
> During resume, we do not know that "reboot" method was used, so we
> assume plaform and make the led blink...
> 
> Unfortunately I see no easy solution, and this may/will cause other
> problems -- in case of broken bios and user telling us not to call
> that bios, we'll call it anyway.
> 
> (Ouch and I think this is regression after 2.6.20?)

I'm not sure, will check that later.

The solution would be to pass the information about the suspend mode used
in the image header, I think (that's what uswsusp does after my recent patch).

Greetings,
Rafael

Re: "reboot" swsusp mode leaves moon icon blinking

[Pavel Machek]

Hi!

> > ...and cause is really simple.
> > 
> > During resume, we do not know that "reboot" method was used, so we
> > assume plaform and make the led blink...
> > 
> > Unfortunately I see no easy solution, and this may/will cause other
> > problems -- in case of broken bios and user telling us not to call
> > that bios, we'll call it anyway.
> > 
> > (Ouch and I think this is regression after 2.6.20?)
> 
> I'm not sure, will check that later.
> 
> The solution would be to pass the information about the suspend mode used
> in the image header, I think (that's what uswsusp does after my recent patch).

Yes, we could do that.

OTOH, we could simply avoid calling the platform code in resume
path. It worked ok for a long while, and it seems to have no
downsides...

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

Re: "reboot" swsusp mode leaves moon icon blinking

[Pavel Machek]

Hi!

> > ...and cause is really simple.
> > 
> > During resume, we do not know that "reboot" method was used, so we
> > assume plaform and make the led blink...
> 
> What does the "reboot" swsusp mean?

echo reboot > ...disk.

> I am having this (or a similar problem):
> 
> 	echo shutdown > /sys/power/disk; echo disk > /sys/power/state

Yep, that's duplicate. Try patch below.
								Pavel

> I saved some /proc files (interrupts, acpi/processor/power, ...) and
> compared them before the suspend and after the resume but I could not
> find any differences.
> 
> > diff --git a/kernel/power/disk.c b/kernel/power/disk.c
> > index 873cdf8..dee0ff4 100644
> > --- a/kernel/power/disk.c
> > +++ b/kernel/power/disk.c
> > @@ -241,18 +241,11 @@ static int software_resume(void)
> >  		goto Done;
> >  	}
> >  
> > -	error = platform_prepare();
> > -	if (error) {
> > -		swsusp_free();
> > -		goto Thaw;
> > -	}
> > -
> 
> What is platform_prepare() actually good for when _resuming_? I tried to
> follow the function calls and it seems to disable a lot of thing instead
> of re-enabling them...?
> 
> =Hannes
> -
> 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/

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

Re: "reboot" swsusp mode leaves moon icon blinking

[Rafael J. Wysocki]

On Tuesday, 20 March 2007 18:20, Pavel Machek wrote:
> Hi!
> 
> > > ...and cause is really simple.
> > > 
> > > During resume, we do not know that "reboot" method was used, so we
> > > assume plaform and make the led blink...
> > > 
> > > Unfortunately I see no easy solution, and this may/will cause other
> > > problems -- in case of broken bios and user telling us not to call
> > > that bios, we'll call it anyway.
> > > 
> > > (Ouch and I think this is regression after 2.6.20?)
> > 
> > I'm not sure, will check that later.
> > 
> > The solution would be to pass the information about the suspend mode used
> > in the image header, I think (that's what uswsusp does after my recent patch).
> 
> Yes, we could do that.
> 
> OTOH, we could simply avoid calling the platform code in resume
> path. It worked ok for a long while, and it seems to have no
> downsides...

No, we have always done it, actually, but not in this particular place. ;-)

In theory, avoiding it could be problematic, because of the pm_ops->finish()
that gets called after the image has been restored (if the platform mode was
used for suspending).  Still, I think we can try.

Greetings,
Rafael

Re: "reboot" swsusp mode leaves moon icon blinking

[Pavel Machek]

Hi!

> > Yes, we could do that.
> > 
> > OTOH, we could simply avoid calling the platform code in resume
> > path. It worked ok for a long while, and it seems to have no
> > downsides...
> 
> No, we have always done it, actually, but not in this particular place. ;-)
> 
> In theory, avoiding it could be problematic, because of the pm_ops->finish()
> that gets called after the image has been restored (if the platform mode was
> used for suspending).  Still, I think we can try.

If we can return to the method "how we did it in 2.6.20", that would
indeed be preffered ;-).
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: "reboot" swsusp mode leaves moon icon blinking

[Johannes Weiner]

Hi,

On Tue, Mar 20, 2007 at 07:01:21PM +0100, Pavel Machek wrote:
> > What does the "reboot" swsusp mean?
> 
> echo reboot > ...disk.

Thank you.

> > I am having this (or a similar problem):
> > 
> > 	echo shutdown > /sys/power/disk; echo disk > /sys/power/state
> 
> Yep, that's duplicate. Try patch below.

I applied it and the box is suspending/resuming properly again.

=Hannes