[RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Zhao Yakui]

Subject:ACPI: Get the device power state in the course of scanning device
From: Zhao Yakui <yakui.zhao@intel.com>

Get the device power state in the course of scanning device if the device 
power flag is power_managable. i.e. The device has the _PSx/_PRx object.

At the same time before the drivers/acpi/power module is loaded, there is no
relation between acpi_power_resource and acpi device. So the first parameter 
of acpi_power_get_state is changed to acpi_handle.

http://bugzilla.kernel.org/show_bug.cgi?id=8049
http://bugzilla.kernel.org/show_bug.cgi?id=11000

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Signed-off-by: Li Shaohua <shaohua.li@intel.com>

---
 drivers/acpi/power.c |   30 ++++++++++++++++--------------
 drivers/acpi/scan.c  |    1 +
 2 files changed, 17 insertions(+), 14 deletions(-)

Index: linux-2.6/drivers/acpi/power.c
===================================================================
--- linux-2.6.orig/drivers/acpi/power.c
+++ linux-2.6/drivers/acpi/power.c
@@ -128,16 +128,16 @@ acpi_power_get_context(acpi_handle handl
 	return 0;
 }
 
-static int acpi_power_get_state(struct acpi_power_resource *resource, int *state)
+static int acpi_power_get_state(acpi_handle handle, int *state)
 {
 	acpi_status status = AE_OK;
 	unsigned long sta = 0;
 
 
-	if (!resource || !state)
+	if (!handle || !state)
 		return -EINVAL;
 
-	status = acpi_evaluate_integer(resource->device->handle, "_STA", NULL, &sta);
+	status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
@@ -145,7 +145,7 @@ static int acpi_power_get_state(struct a
 			      ACPI_POWER_RESOURCE_STATE_OFF;
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Resource [%s] is %s\n",
-			  resource->name, state ? "on" : "off"));
+			  acpi_ut_get_node_name(handle), state ? "on" : "off"));
 
 	return 0;
 }
@@ -153,7 +153,6 @@ static int acpi_power_get_state(struct a
 static int acpi_power_get_list_state(struct acpi_handle_list *list, int *state)
 {
 	int result = 0, state1;
-	struct acpi_power_resource *resource = NULL;
 	u32 i = 0;
 
 
@@ -161,12 +160,15 @@ static int acpi_power_get_list_state(str
 		return -EINVAL;
 
 	/* The state of the list is 'on' IFF all resources are 'on'. */
+	/* */
 
 	for (i = 0; i < list->count; i++) {
-		result = acpi_power_get_context(list->handles[i], &resource);
-		if (result)
-			return result;
-		result = acpi_power_get_state(resource, &state1);
+		/*
+		 * The state of the power resource can be obtained by
+		 * using the ACPI handle. In such case it is unnecessary to
+		 * get the Power resource first and then get its state again.
+		 */
+		result = acpi_power_get_state(list->handles[i], &state1);
 		if (result)
 			return result;
 
@@ -226,7 +228,7 @@ static int acpi_power_on(acpi_handle han
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	result = acpi_power_get_state(resource, &state);
+	result = acpi_power_get_state(resource->device->handle, &state);
 	if (result)
 		return result;
 	if (state != ACPI_POWER_RESOURCE_STATE_ON)
@@ -277,7 +279,7 @@ static int acpi_power_off_device(acpi_ha
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	result = acpi_power_get_state(resource, &state);
+	result = acpi_power_get_state(handle, &state);
 	if (result)
 		return result;
 	if (state != ACPI_POWER_RESOURCE_STATE_OFF)
@@ -555,7 +557,7 @@ static int acpi_power_seq_show(struct se
 	if (!resource)
 		goto end;
 
-	result = acpi_power_get_state(resource, &state);
+	result = acpi_power_get_state(resource->device->handle, &state);
 	if (result)
 		goto end;
 
@@ -668,7 +670,7 @@ static int acpi_power_add(struct acpi_de
 	resource->system_level = acpi_object.power_resource.system_level;
 	resource->order = acpi_object.power_resource.resource_order;
 
-	result = acpi_power_get_state(resource, &state);
+	result = acpi_power_get_state(device->handle, &state);
 	if (result)
 		goto end;
 
@@ -735,7 +737,7 @@ static int acpi_power_resume(struct acpi
 
 	resource = (struct acpi_power_resource *)acpi_driver_data(device);
 
-	result = acpi_power_get_state(resource, &state);
+	result = acpi_power_get_state(device->handle, &state);
 	if (result)
 		return result;
 
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -814,6 +814,7 @@ static int acpi_bus_get_power_flags(stru
 	/* TBD: System wake support and resource requirements. */
 
 	device->power.state = ACPI_STATE_UNKNOWN;
+	acpi_bus_get_power(device->handle, &(device->power.state));
 
 	return 0;
 }

Re: [RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Andi Kleen]

On Mon, Aug 11, 2008 at 02:55:05PM +0800, Zhao Yakui wrote:
> Subject:ACPI: Get the device power state in the course of scanning device
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
> Get the device power state in the course of scanning device if the device 
> power flag is power_managable. i.e. The device has the _PSx/_PRx object.
> 
> At the same time before the drivers/acpi/power module is loaded, there is no
> relation between acpi_power_resource and acpi device. So the first parameter 
> of acpi_power_get_state is changed to acpi_handle.
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=8049
> http://bugzilla.kernel.org/show_bug.cgi?id=11000

Seems to be an pretty old bug. I assume it's not critical
enough to require pushing into .27? 

-Andi

Re: [RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Zhao Yakui]

On Mon, 2008-08-11 at 08:45 +0200, Andi Kleen wrote:
> On Mon, Aug 11, 2008 at 02:55:05PM +0800, Zhao Yakui wrote:
> > Subject:ACPI: Get the device power state in the course of scanning device
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> > Get the device power state in the course of scanning device if the device 
> > power flag is power_managable. i.e. The device has the _PSx/_PRx object.
> > 
> > At the same time before the drivers/acpi/power module is loaded, there is no
> > relation between acpi_power_resource and acpi device. So the first parameter 
> > of acpi_power_get_state is changed to acpi_handle.
> > 
> > http://bugzilla.kernel.org/show_bug.cgi?id=8049
> > http://bugzilla.kernel.org/show_bug.cgi?id=11000
> 
> Seems to be an pretty old bug. I assume it's not critical
> enough to require pushing into .27? 
Yes. It is not critical. 
But in fact windows can work well on such broken BIOS. In order to make
Linux be compatible with this broken BIOS, it will be OK to merge them.

Thanks.
> 
> -Andi

Re: [RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Andi Kleen]

On Mon, Aug 11, 2008 at 03:02:20PM +0800, Zhao Yakui wrote:
> On Mon, 2008-08-11 at 08:45 +0200, Andi Kleen wrote:
> > On Mon, Aug 11, 2008 at 02:55:05PM +0800, Zhao Yakui wrote:
> > > Subject:ACPI: Get the device power state in the course of scanning device
> > > From: Zhao Yakui <yakui.zhao@intel.com>
> > > 
> > > Get the device power state in the course of scanning device if the device 
> > > power flag is power_managable. i.e. The device has the _PSx/_PRx object.
> > > 
> > > At the same time before the drivers/acpi/power module is loaded, there is no
> > > relation between acpi_power_resource and acpi device. So the first parameter 
> > > of acpi_power_get_state is changed to acpi_handle.
> > > 
> > > http://bugzilla.kernel.org/show_bug.cgi?id=8049
> > > http://bugzilla.kernel.org/show_bug.cgi?id=11000
> > 
> > Seems to be an pretty old bug. I assume it's not critical
> > enough to require pushing into .27? 
> Yes. It is not critical. 
> But in fact windows can work well on such broken BIOS. In order to make

Any guess how do they do that? Do they have a blacklist for this case?

> Linux be compatible with this broken BIOS, it will be OK to merge them.

The option is fine since it is off by default, but I'm a little scaried 
about the init sequence ordering for .27. That might break something.

-Andi

Re: [RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Zhao Yakui]

On Mon, 2008-08-11 at 08:52 +0200, Andi Kleen wrote:
> On Mon, Aug 11, 2008 at 03:02:20PM +0800, Zhao Yakui wrote:
> > On Mon, 2008-08-11 at 08:45 +0200, Andi Kleen wrote:
> > > On Mon, Aug 11, 2008 at 02:55:05PM +0800, Zhao Yakui wrote:
> > > > Subject:ACPI: Get the device power state in the course of scanning device
> > > > From: Zhao Yakui <yakui.zhao@intel.com>
> > > > 
> > > > Get the device power state in the course of scanning device if the device 
> > > > power flag is power_managable. i.e. The device has the _PSx/_PRx object.
> > > > 
> > > > At the same time before the drivers/acpi/power module is loaded, there is no
> > > > relation between acpi_power_resource and acpi device. So the first parameter 
> > > > of acpi_power_get_state is changed to acpi_handle.
> > > > 
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=8049
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=11000
> > > 
> > > Seems to be an pretty old bug. I assume it's not critical
> > > enough to require pushing into .27? 
> > Yes. It is not critical. 
> > But in fact windows can work well on such broken BIOS. In order to make
> 
> Any guess how do they do that? Do they have a blacklist for this case?
I do such a test by using the KVM and find that there is no power state
check during power transition. So it will be reasonable that Linux also
disables power state check during power transition.

> > Linux be compatible with this broken BIOS, it will be OK to merge them.
> 
> The option is fine since it is off by default, but I'm a little scaried 
> about the init sequence ordering for .27. That might break something.
After the patch is applied, Linux will still do the power state check
unless the boot option of "acpi.power_nocheck=1" is added.  In such case
it won't affect anything. 
In fact we expect that the boot option of "acpi.power_nocheck=1" becomes
the default. But we will have to wait for more response. Only after more
tests are done, it will be OK.

what the init sequence ordering for .27 means? 

thanks.


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

Re: [RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Andi Kleen]

> After the patch is applied, Linux will still do the power state check
> unless the boot option of "acpi.power_nocheck=1" is added.  In such case
> it won't affect anything. 

Understood that.

> In fact we expect that the boot option of "acpi.power_nocheck=1" becomes
> the default. But we will have to wait for more response. Only after more
> tests are done, it will be OK.

Ok. 

> what the init sequence ordering for .27 means? 

The first patch moved acpi_device_set_context to be earlier, so 
the init sequence is different. All the other changes are optional,
as in conditional on the flag, but that one is not.

I have not been completely able to satisfy myself that it is low 
risk enough.

-Andi

Re: [RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Zhao Yakui]

On Mon, 2008-08-11 at 09:56 +0200, Andi Kleen wrote:
> > After the patch is applied, Linux will still do the power state check
> > unless the boot option of "acpi.power_nocheck=1" is added.  In such case
> > it won't affect anything. 
> 
> Understood that.
> 
> > In fact we expect that the boot option of "acpi.power_nocheck=1" becomes
> > the default. But we will have to wait for more response. Only after more
> > tests are done, it will be OK.
> 
> Ok. 
> 
> > what the init sequence ordering for .27 means? 
> 
> The first patch moved acpi_device_set_context to be earlier, so 
> the init sequence is different. All the other changes are optional,
> as in conditional on the flag, but that one is not.
Although the acpi_device_set_context is moved to the earlier in the
first patch, it won't break anything.  In the patch the acpi device is
bound with the acpi handle before getting the flag of Power
management/Wakeup/Performance.
Maybe it can be put to the acpi test tree to test whether it will break
anything.
Thanks.

> I have not been completely able to satisfy myself that it is low 
> risk enough.
> 
> -Andi

Re: [RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Andi Kleen]

On Mon, Aug 11, 2008 at 06:21:15PM +0800, Zhao Yakui wrote:
> On Mon, 2008-08-11 at 09:56 +0200, Andi Kleen wrote:
> > > After the patch is applied, Linux will still do the power state check
> > > unless the boot option of "acpi.power_nocheck=1" is added.  In such case
> > > it won't affect anything. 
> > 
> > Understood that.
> > 
> > > In fact we expect that the boot option of "acpi.power_nocheck=1" becomes
> > > the default. But we will have to wait for more response. Only after more
> > > tests are done, it will be OK.
> > 
> > Ok. 
> > 
> > > what the init sequence ordering for .27 means? 
> > 
> > The first patch moved acpi_device_set_context to be earlier, so 
> > the init sequence is different. All the other changes are optional,
> > as in conditional on the flag, but that one is not.
> Although the acpi_device_set_context is moved to the earlier in the
> first patch, it won't break anything.

Well they always say that :) But I'll double check.

-Andi

Re: [RESEND] [PATCH 2/4] ACPI: Get the device power state in the course of scanning device

[Rafael J. Wysocki]

On Monday, 11 of August 2008, Andi Kleen wrote:
> On Mon, Aug 11, 2008 at 06:21:15PM +0800, Zhao Yakui wrote:
> > On Mon, 2008-08-11 at 09:56 +0200, Andi Kleen wrote:
> > > > After the patch is applied, Linux will still do the power state check
> > > > unless the boot option of "acpi.power_nocheck=1" is added.  In such case
> > > > it won't affect anything. 
> > > 
> > > Understood that.
> > > 
> > > > In fact we expect that the boot option of "acpi.power_nocheck=1" becomes
> > > > the default. But we will have to wait for more response. Only after more
> > > > tests are done, it will be OK.
> > > 
> > > Ok. 
> > > 
> > > > what the init sequence ordering for .27 means? 
> > > 
> > > The first patch moved acpi_device_set_context to be earlier, so 
> > > the init sequence is different. All the other changes are optional,
> > > as in conditional on the flag, but that one is not.
> > Although the acpi_device_set_context is moved to the earlier in the
> > first patch, it won't break anything.
> 
> Well they always say that :) But I'll double check.

FWIW, I'd feel much more comfortable if this goes into 2.6.28 rather than into
2.6.27, the main reason being that there are many suspend-related changes
in 2.6.27 that can potentially break things.  In particular, there are the
PCI-ACPI patches related to wake-up that may interfere whith this change
(low probability, but still).

Thanks,
Rafael