: ACPI: Skip the power state check in power transition
diff mbox

Message ID 1242106060.3773.216.camel@localhost.localdomain
State Superseded, archived
Headers show

Commit Message

Zhao, Yakui May 12, 2009, 5:27 a.m. UTC
From: Zhao Yakui <yakui.zhao@intel.com>

Skip the power state check in course of power transition by changing the
default value of acpi_power_nocheck to 1. If so, it is unnecessary to add the
boot option of "acpi.power_nocheck=1" or add it into DMI power check table to
skip the power state check.

Of course the power state check still can be enabled by adding the boot option
of "acpi.power_nocheck=0".

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Acked-by: Matthew Garrett <mjg59@srcf.ucam.org>
---
 drivers/acpi/power.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)



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

Comments

Bjorn Helgaas May 12, 2009, 2:57 p.m. UTC | #1
On Monday 11 May 2009 11:27:40 pm yakui_zhao wrote:
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
> Skip the power state check in course of power transition by changing the
> default value of acpi_power_nocheck to 1. If so, it is unnecessary to add the
> boot option of "acpi.power_nocheck=1" or add it into DMI power check table to
> skip the power state check.
> 
> Of course the power state check still can be enabled by adding the boot option
> of "acpi.power_nocheck=0".

What is the value of keeping the "acpi.power_nocheck" option at all?

If we can get along without it, it'd be nice to just remove the
whole thing.

Bjorn

> Index: linux-2.6/drivers/acpi/power.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/power.c	2009-04-16 16:10:24.000000000 +0800
> +++ linux-2.6/drivers/acpi/power.c	2009-05-12 13:12:27.000000000 +0800
> @@ -54,7 +54,7 @@
>  #define ACPI_POWER_RESOURCE_STATE_ON	0x01
>  #define ACPI_POWER_RESOURCE_STATE_UNKNOWN 0xFF
>  
> -int acpi_power_nocheck;
> +int acpi_power_nocheck = 1;
>  module_param_named(power_nocheck, acpi_power_nocheck, bool, 000);
>  
>  static int acpi_power_add(struct acpi_device *device);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhao, Yakui May 13, 2009, 3:13 a.m. UTC | #2
On Tue, 2009-05-12 at 22:57 +0800, Bjorn Helgaas wrote:
> On Monday 11 May 2009 11:27:40 pm yakui_zhao wrote:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> > Skip the power state check in course of power transition by changing the
> > default value of acpi_power_nocheck to 1. If so, it is unnecessary to add the
> > boot option of "acpi.power_nocheck=1" or add it into DMI power check table to
> > skip the power state check.
> > 
> > Of course the power state check still can be enabled by adding the boot option
> > of "acpi.power_nocheck=0".
> 
> What is the value of keeping the "acpi.power_nocheck" option at all?
> 
> If we can get along without it, it'd be nice to just remove the
> whole thing.
Yes. If we delete the course of the power state check in power
transition, it will be unnecessary to add the boot option.

In fact this object is defined in ACPI spec. And we had better follow
that. IMO Linux ACPI does the right thing.
The boot option of "acpi.power_nocheck" is only to make Linux be
compatible with windows.

At the same time if we want to make the power state check more strict,
we will have to re-add the source-code. 

So IMO it will be better that the power state check in power transition
is controlled by one module parameter.

Best regards.
    Yakui.

> 
> Bjorn
> 
> > Index: linux-2.6/drivers/acpi/power.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/power.c	2009-04-16 16:10:24.000000000 +0800
> > +++ linux-2.6/drivers/acpi/power.c	2009-05-12 13:12:27.000000000 +0800
> > @@ -54,7 +54,7 @@
> >  #define ACPI_POWER_RESOURCE_STATE_ON	0x01
> >  #define ACPI_POWER_RESOURCE_STATE_UNKNOWN 0xFF
> >  
> > -int acpi_power_nocheck;
> > +int acpi_power_nocheck = 1;
> >  module_param_named(power_nocheck, acpi_power_nocheck, bool, 000);
> >  
> >  static int acpi_power_add(struct acpi_device *device);
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Garrett May 13, 2009, 1:08 p.m. UTC | #3
On Wed, May 13, 2009 at 11:13:04AM +0800, yakui_zhao wrote:

> In fact this object is defined in ACPI spec. And we had better follow
> that. IMO Linux ACPI does the right thing.
> The boot option of "acpi.power_nocheck" is only to make Linux be
> compatible with windows.

The default behaviour should be to be compatible with Windows, 
regardless of what the spec says. There's an argument for providing a 
strict interpretation of the spec for testing purposes, but I don't see 
any reason for it to be split up into dozens of individual kernel 
parameters.
Zhao, Yakui May 14, 2009, 1:47 a.m. UTC | #4
On Wed, 2009-05-13 at 21:08 +0800, Matthew Garrett wrote:
> The default behaviour should be to be compatible with Windows, 
> regardless of what the spec says. There's an argument for providing a 
> strict interpretation of the spec for testing purposes, but I don't
> see 
> any reason for it to be split up into dozens of individual kernel 
> parameters
The ACPI 1.0 spec is followed by windows XP. And the power state is not
checked in power transition under windows XP.
But we don't know whether it is still skipped on the new version
windows.(For example: Windows 7).

If the module param is removed, we must delete the source code related
with power state check. And if the power state is checked in power
transition on windows 7, what we should do? It is not reasonable to add
them again.

Maybe it is better to determine whether the power state check is skipped
in power transition.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Garrett May 14, 2009, 10:56 a.m. UTC | #5
On Thu, May 14, 2009 at 09:47:39AM +0800, yakui_zhao wrote:
> On Wed, 2009-05-13 at 21:08 +0800, Matthew Garrett wrote:
> > The default behaviour should be to be compatible with Windows, 
> > regardless of what the spec says. There's an argument for providing a 
> > strict interpretation of the spec for testing purposes, but I don't
> > see 
> > any reason for it to be split up into dozens of individual kernel 
> > parameters
> The ACPI 1.0 spec is followed by windows XP. And the power state is not
> checked in power transition under windows XP.
> But we don't know whether it is still skipped on the new version
> windows.(For example: Windows 7).
>
> If the module param is removed, we must delete the source code related
> with power state check. And if the power state is checked in power
> transition on windows 7, what we should do? It is not reasonable to add
> them again.

If Windows 7 changes the behaviour then the correct approach is to key 
this behaviour on whether the system firmware requests the Windows 7 OSI 
string. The code can be #if 0ed out until then, or placed under an 
acpi.strict kernel option that turns on all standards-compliant but 
windows-incompatible code.

> Maybe it is better to determine whether the power state check is skipped
> in power transition.

We have a stated policy that Linux will default to being Windows 
compatible. You've demonstrated that in this case Linux isn't Windows 
compatible, which means that it's a bug. The correct behaviour for Linux 
here is to ignore the _STA value (or, indeed, not to call _STA at all in 
this path).
Len Brown May 28, 2009, 1:43 a.m. UTC | #6
I agree with Matthew and Bjorn.
This is the right thing to do, but we should take it a step further and
simplify the code.

thanks,
Len Brown, Intel Open Source Technology Center

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

Patch
diff mbox

Index: linux-2.6/drivers/acpi/power.c
===================================================================
--- linux-2.6.orig/drivers/acpi/power.c	2009-04-16 16:10:24.000000000 +0800
+++ linux-2.6/drivers/acpi/power.c	2009-05-12 13:12:27.000000000 +0800
@@ -54,7 +54,7 @@ 
 #define ACPI_POWER_RESOURCE_STATE_ON	0x01
 #define ACPI_POWER_RESOURCE_STATE_UNKNOWN 0xFF
 
-int acpi_power_nocheck;
+int acpi_power_nocheck = 1;
 module_param_named(power_nocheck, acpi_power_nocheck, bool, 000);
 
 static int acpi_power_add(struct acpi_device *device);