Linux Power Management development
 help / color / mirror / Atom feed
* [PATCH] PM: fix pm_test support for freeze state
@ 2013-03-08  5:28 Zhang Rui
  2013-03-11 23:58 ` Rafael J. Wysocki
  0 siblings, 1 reply; 4+ messages in thread
From: Zhang Rui @ 2013-03-08  5:28 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Linux PM list, Zhang, Rui, Kristen Carlson Accardi

>From d6e5d4f4e8b9a8af79fa50d8e13b14c89d8a9264 Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Wed, 6 Mar 2013 21:22:21 +0800
Subject: [PATCH] PM: fix pm_test support for freeze state

freeze state is a software suspend state that does not run into
low-level platform callbacks which may interact with BIOS.
And freeze state does not need to disable the processors.
Thus we should disable TEST_CPUS and TEST_CORE mode
when running pm_test for freeze state.

Fix the problem so that users will get a warning message and fail to
enter freeze state, when pm_test mode is set to TEST_CPUS/TEST_CORE.

Plus, change the code so that freeze_enter() is invoked after
suspend_test(TEST_PLATFORM) being invoked, then pm_test can be used to
check if freeze state is working well after all devices are suspended
and before processors are blocked.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 kernel/power/suspend.c |   18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index d4feda0..e29909a 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -76,8 +76,16 @@ EXPORT_SYMBOL_GPL(suspend_set_ops);
 
 bool valid_state(suspend_state_t state)
 {
-	if (state == PM_SUSPEND_FREEZE)
-		return true;
+	if (state == PM_SUSPEND_FREEZE) {
+		if (pm_test_level == TEST_NONE ||
+		    pm_test_level == TEST_FREEZER ||
+		    pm_test_level == TEST_DEVICES ||
+		    pm_test_level == TEST_PLATFORM)
+			return true;
+		printk(KERN_WARNING "Unsupported pm_test mode for freeze state,"
+			" please choose none/freezer/devices/platform.\n");
+		return false;
+	}
 	/*
 	 * PM_SUSPEND_STANDBY and PM_SUSPEND_MEMORY states need lowlevel
 	 * support and need to be valid to the lowlevel
@@ -184,6 +192,9 @@ static int suspend_enter(suspend_state_t state, bool
*wakeup)
 			goto Platform_wake;
 	}
 
+	if (suspend_test(TEST_PLATFORM))
+		goto Platform_wake;
+
 	/*
 	 * PM_SUSPEND_FREEZE equals
 	 * frozen processes + suspended devices + idle processors.
@@ -195,9 +206,6 @@ static int suspend_enter(suspend_state_t state, bool
*wakeup)
 		goto Platform_wake;
 	}
 
-	if (suspend_test(TEST_PLATFORM))
-		goto Platform_wake;
-
 	error = disable_nonboot_cpus();
 	if (error || suspend_test(TEST_CPUS))
 		goto Enable_cpus;
-- 
1.7.9.5




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

* Re: [PATCH] PM: fix pm_test support for freeze state
  2013-03-08  5:28 [PATCH] PM: fix pm_test support for freeze state Zhang Rui
@ 2013-03-11 23:58 ` Rafael J. Wysocki
  2013-03-12  0:03   ` Zhang Rui
  0 siblings, 1 reply; 4+ messages in thread
From: Rafael J. Wysocki @ 2013-03-11 23:58 UTC (permalink / raw)
  To: Zhang Rui; +Cc: Linux PM list, Kristen Carlson Accardi

On Friday, March 08, 2013 01:28:50 PM Zhang Rui wrote:
> From d6e5d4f4e8b9a8af79fa50d8e13b14c89d8a9264 Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Wed, 6 Mar 2013 21:22:21 +0800
> Subject: [PATCH] PM: fix pm_test support for freeze state
> 
> freeze state is a software suspend state that does not run into
> low-level platform callbacks which may interact with BIOS.
> And freeze state does not need to disable the processors.
> Thus we should disable TEST_CPUS and TEST_CORE mode
> when running pm_test for freeze state.
> 
> Fix the problem so that users will get a warning message and fail to
> enter freeze state, when pm_test mode is set to TEST_CPUS/TEST_CORE.
> 
> Plus, change the code so that freeze_enter() is invoked after
> suspend_test(TEST_PLATFORM) being invoked, then pm_test can be used to
> check if freeze state is working well after all devices are suspended
> and before processors are blocked.

Can you please do one change per patch?  The second change is not related
to the first one and I don't think it is correct event.

Thanks,
Rafael


> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  kernel/power/suspend.c |   18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index d4feda0..e29909a 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -76,8 +76,16 @@ EXPORT_SYMBOL_GPL(suspend_set_ops);
>  
>  bool valid_state(suspend_state_t state)
>  {
> -	if (state == PM_SUSPEND_FREEZE)
> -		return true;
> +	if (state == PM_SUSPEND_FREEZE) {
> +		if (pm_test_level == TEST_NONE ||
> +		    pm_test_level == TEST_FREEZER ||
> +		    pm_test_level == TEST_DEVICES ||
> +		    pm_test_level == TEST_PLATFORM)
> +			return true;
> +		printk(KERN_WARNING "Unsupported pm_test mode for freeze state,"
> +			" please choose none/freezer/devices/platform.\n");
> +		return false;
> +	}
>  	/*
>  	 * PM_SUSPEND_STANDBY and PM_SUSPEND_MEMORY states need lowlevel
>  	 * support and need to be valid to the lowlevel
> @@ -184,6 +192,9 @@ static int suspend_enter(suspend_state_t state, bool
> *wakeup)
>  			goto Platform_wake;
>  	}
>  
> +	if (suspend_test(TEST_PLATFORM))
> +		goto Platform_wake;
> +
>  	/*
>  	 * PM_SUSPEND_FREEZE equals
>  	 * frozen processes + suspended devices + idle processors.
> @@ -195,9 +206,6 @@ static int suspend_enter(suspend_state_t state, bool
> *wakeup)
>  		goto Platform_wake;
>  	}
>  
> -	if (suspend_test(TEST_PLATFORM))
> -		goto Platform_wake;
> -
>  	error = disable_nonboot_cpus();
>  	if (error || suspend_test(TEST_CPUS))
>  		goto Enable_cpus;
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH] PM: fix pm_test support for freeze state
  2013-03-11 23:58 ` Rafael J. Wysocki
@ 2013-03-12  0:03   ` Zhang Rui
  2013-03-12  0:13     ` Rafael J. Wysocki
  0 siblings, 1 reply; 4+ messages in thread
From: Zhang Rui @ 2013-03-12  0:03 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Linux PM list, Kristen Carlson Accardi

On Tue, 2013-03-12 at 00:58 +0100, Rafael J. Wysocki wrote:
> On Friday, March 08, 2013 01:28:50 PM Zhang Rui wrote:
> > From d6e5d4f4e8b9a8af79fa50d8e13b14c89d8a9264 Mon Sep 17 00:00:00 2001
> > From: Zhang Rui <rui.zhang@intel.com>
> > Date: Wed, 6 Mar 2013 21:22:21 +0800
> > Subject: [PATCH] PM: fix pm_test support for freeze state
> > 
> > freeze state is a software suspend state that does not run into
> > low-level platform callbacks which may interact with BIOS.
> > And freeze state does not need to disable the processors.
> > Thus we should disable TEST_CPUS and TEST_CORE mode
> > when running pm_test for freeze state.
> > 
> > Fix the problem so that users will get a warning message and fail to
> > enter freeze state, when pm_test mode is set to TEST_CPUS/TEST_CORE.
> > 
> > Plus, change the code so that freeze_enter() is invoked after
> > suspend_test(TEST_PLATFORM) being invoked, then pm_test can be used to
> > check if freeze state is working well after all devices are suspended
> > and before processors are blocked.
> 
> Can you please do one change per patch? 

okay. will do that.

>  The second change is not related
> to the first one

right.

>  and I don't think it is correct event.
> 
hmmm,
do you mean two changes in one patch is wrong or do you mean the code
for the second change is wrong?

thanks,
rui
> Thanks,
> Rafael
> 
> 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  kernel/power/suspend.c |   18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> > index d4feda0..e29909a 100644
> > --- a/kernel/power/suspend.c
> > +++ b/kernel/power/suspend.c
> > @@ -76,8 +76,16 @@ EXPORT_SYMBOL_GPL(suspend_set_ops);
> >  
> >  bool valid_state(suspend_state_t state)
> >  {
> > -	if (state == PM_SUSPEND_FREEZE)
> > -		return true;
> > +	if (state == PM_SUSPEND_FREEZE) {
> > +		if (pm_test_level == TEST_NONE ||
> > +		    pm_test_level == TEST_FREEZER ||
> > +		    pm_test_level == TEST_DEVICES ||
> > +		    pm_test_level == TEST_PLATFORM)
> > +			return true;
> > +		printk(KERN_WARNING "Unsupported pm_test mode for freeze state,"
> > +			" please choose none/freezer/devices/platform.\n");
> > +		return false;
> > +	}
> >  	/*
> >  	 * PM_SUSPEND_STANDBY and PM_SUSPEND_MEMORY states need lowlevel
> >  	 * support and need to be valid to the lowlevel
> > @@ -184,6 +192,9 @@ static int suspend_enter(suspend_state_t state, bool
> > *wakeup)
> >  			goto Platform_wake;
> >  	}
> >  
> > +	if (suspend_test(TEST_PLATFORM))
> > +		goto Platform_wake;
> > +
> >  	/*
> >  	 * PM_SUSPEND_FREEZE equals
> >  	 * frozen processes + suspended devices + idle processors.
> > @@ -195,9 +206,6 @@ static int suspend_enter(suspend_state_t state, bool
> > *wakeup)
> >  		goto Platform_wake;
> >  	}
> >  
> > -	if (suspend_test(TEST_PLATFORM))
> > -		goto Platform_wake;
> > -
> >  	error = disable_nonboot_cpus();
> >  	if (error || suspend_test(TEST_CPUS))
> >  		goto Enable_cpus;
> > 



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

* Re: [PATCH] PM: fix pm_test support for freeze state
  2013-03-12  0:03   ` Zhang Rui
@ 2013-03-12  0:13     ` Rafael J. Wysocki
  0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2013-03-12  0:13 UTC (permalink / raw)
  To: Zhang Rui; +Cc: Linux PM list, Kristen Carlson Accardi

On Tuesday, March 12, 2013 08:03:15 AM Zhang Rui wrote:
> On Tue, 2013-03-12 at 00:58 +0100, Rafael J. Wysocki wrote:
> > On Friday, March 08, 2013 01:28:50 PM Zhang Rui wrote:
> > > From d6e5d4f4e8b9a8af79fa50d8e13b14c89d8a9264 Mon Sep 17 00:00:00 2001
> > > From: Zhang Rui <rui.zhang@intel.com>
> > > Date: Wed, 6 Mar 2013 21:22:21 +0800
> > > Subject: [PATCH] PM: fix pm_test support for freeze state
> > > 
> > > freeze state is a software suspend state that does not run into
> > > low-level platform callbacks which may interact with BIOS.
> > > And freeze state does not need to disable the processors.
> > > Thus we should disable TEST_CPUS and TEST_CORE mode
> > > when running pm_test for freeze state.
> > > 
> > > Fix the problem so that users will get a warning message and fail to
> > > enter freeze state, when pm_test mode is set to TEST_CPUS/TEST_CORE.
> > > 
> > > Plus, change the code so that freeze_enter() is invoked after
> > > suspend_test(TEST_PLATFORM) being invoked, then pm_test can be used to
> > > check if freeze state is working well after all devices are suspended
> > > and before processors are blocked.
> > 
> > Can you please do one change per patch? 
> 
> okay. will do that.
> 
> >  The second change is not related
> > to the first one
> 
> right.
> 
> >  and I don't think it is correct event.
> > 
> hmmm,
> do you mean two changes in one patch is wrong or do you mean the code
> for the second change is wrong?

I thought that the second change code wasn't correct, but I seem to have
confused things.

Please split it off, though.

Thanks,
Rafael


> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > ---
> > >  kernel/power/suspend.c |   18 +++++++++++++-----
> > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> > > index d4feda0..e29909a 100644
> > > --- a/kernel/power/suspend.c
> > > +++ b/kernel/power/suspend.c
> > > @@ -76,8 +76,16 @@ EXPORT_SYMBOL_GPL(suspend_set_ops);
> > >  
> > >  bool valid_state(suspend_state_t state)
> > >  {
> > > -	if (state == PM_SUSPEND_FREEZE)
> > > -		return true;
> > > +	if (state == PM_SUSPEND_FREEZE) {
> > > +		if (pm_test_level == TEST_NONE ||
> > > +		    pm_test_level == TEST_FREEZER ||
> > > +		    pm_test_level == TEST_DEVICES ||
> > > +		    pm_test_level == TEST_PLATFORM)
> > > +			return true;
> > > +		printk(KERN_WARNING "Unsupported pm_test mode for freeze state,"
> > > +			" please choose none/freezer/devices/platform.\n");
> > > +		return false;
> > > +	}
> > >  	/*
> > >  	 * PM_SUSPEND_STANDBY and PM_SUSPEND_MEMORY states need lowlevel
> > >  	 * support and need to be valid to the lowlevel
> > > @@ -184,6 +192,9 @@ static int suspend_enter(suspend_state_t state, bool
> > > *wakeup)
> > >  			goto Platform_wake;
> > >  	}
> > >  
> > > +	if (suspend_test(TEST_PLATFORM))
> > > +		goto Platform_wake;
> > > +
> > >  	/*
> > >  	 * PM_SUSPEND_FREEZE equals
> > >  	 * frozen processes + suspended devices + idle processors.
> > > @@ -195,9 +206,6 @@ static int suspend_enter(suspend_state_t state, bool
> > > *wakeup)
> > >  		goto Platform_wake;
> > >  	}
> > >  
> > > -	if (suspend_test(TEST_PLATFORM))
> > > -		goto Platform_wake;
> > > -
> > >  	error = disable_nonboot_cpus();
> > >  	if (error || suspend_test(TEST_CPUS))
> > >  		goto Enable_cpus;
> > > 
> 
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

end of thread, other threads:[~2013-03-12  0:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-08  5:28 [PATCH] PM: fix pm_test support for freeze state Zhang Rui
2013-03-11 23:58 ` Rafael J. Wysocki
2013-03-12  0:03   ` Zhang Rui
2013-03-12  0:13     ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox