Skip to contentSkip to conversation

ACPI: don't check power state after _ON/_OFF

PATCH Applied Bjorn Helgaas 3 participants 3 messages May 21, 2009 – May 28, 2009 ACPI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ACPI: don't check power state after _ON/_OFF

We used to evaluate _STA to check the power state of a device after running _ON or _OFF. But as far as I can tell, there's no benefit to evaluating _STA, and sometimes we trip over bugs when BIOSes don't implement _STA correctly.

Yakui says Windows XP doesn't evaluate _STA during power transition. So let's skip it in Linux, too. It's conceivable that we'll need to check _STA in the future for some reason, but until we do, I don't see a reason to clutter this code path.

References:

    http://bugzilla.kernel.org/show_bug.cgi?id=13243
    http://marc.info/?l=linux-acpi&m=124166053803753&w=2
    http://marc.info/?l=linux-acpi&m=124175761408256&w=2
    http://marc.info/?l=linux-acpi&m=124210593114061&w=2
CC Yakui Zhao <[email protected]>
CC Matthew Garrett <[email protected]>
CC "Rafael J. Wysocki" <[email protected]>
CC Witold Szczeponik <[email protected]>
CC Alexey Starikovskiy <[email protected]>
CC Len Brown <[email protected]>
---
 drivers/acpi/power.c |   28 ++--------------------------
 1 files changed, 2 insertions(+), 26 deletions(-)
drivers/acpi/power.c +2 -26
index 56665a6..d74365d 100644
@@ -194,7 +194,7 @@ static int acpi_power_get_list_state(struct acpi_handle_list *list, int *state)
 
 static int acpi_power_on(acpi_handle handle, struct acpi_device *dev)
 {
-	int result = 0, state;
+	int result = 0;
 	int found = 0;
 	acpi_status status = AE_OK;
 	struct acpi_power_resource *resource = NULL;
@@ -236,18 +236,6 @@ static int acpi_power_on(acpi_handle handle, struct acpi_device *dev)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	if (!acpi_power_nocheck) {
-		/*
-		 * If acpi_power_nocheck is set, it is unnecessary to check
-		 * the power state after power transition.
-		 */
-		result = acpi_power_get_state(resource->device->handle,
-				&state);
-		if (result)
-			return result;
-		if (state != ACPI_POWER_RESOURCE_STATE_ON)
-			return -ENOEXEC;
-	}
 	/* Update the power resource's _device_ power state */
 	resource->device->power.state = ACPI_STATE_D0;
 
@@ -258,7 +246,7 @@ static int acpi_power_on(acpi_handle handle, struct acpi_device *dev)
 
 static int acpi_power_off_device(acpi_handle handle, struct acpi_device *dev)
 {
-	int result = 0, state;
+	int result = 0;
 	acpi_status status = AE_OK;
 	struct acpi_power_resource *resource = NULL;
 	struct list_head *node, *next;
@@ -293,18 +281,6 @@ static int acpi_power_off_device(acpi_handle handle, struct acpi_device *dev)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	if (!acpi_power_nocheck) {
-		/*
-		 * If acpi_power_nocheck is set, it is unnecessary to check
-		 * the power state after power transition.
-		 */
-		result = acpi_power_get_state(handle, &state);
-		if (result)
-			return result;
-		if (state != ACPI_POWER_RESOURCE_STATE_OFF)
-			return -ENOEXEC;
-	}
-
 	/* Update the power resource's _device_ power state */
 	resource->device->power.state = ACPI_STATE_D3;
 
Acked-by on v1 1/1
#1 lore raw
Quoted 16 lines · Bjorn Helgaas

We used to evaluate _STA to check the power state of a device after running _ON or _OFF. But as far as I can tell, there's no benefit to evaluating _STA, and sometimes we trip over bugs when BIOSes don't implement _STA correctly.

Yakui says Windows XP doesn't evaluate _STA during power transition. So let's skip it in Linux, too. It's conceivable that we'll need to check _STA in the future for some reason, but until we do, I don't see a reason to clutter this code path.

References:

    http://bugzilla.kernel.org/show_bug.cgi?id=13243
    http://marc.info/?l=linux-acpi&m=124166053803753&w=2
    http://marc.info/?l=linux-acpi&m=124175761408256&w=2
    http://marc.info/?l=linux-acpi&m=124210593114061&w=2
Quoted 7 lines
CC Yakui Zhao <[email protected]>
CC Matthew Garrett <[email protected]>
CC "Rafael J. Wysocki" <[email protected]>
CC Witold Szczeponik <[email protected]>
CC Alexey Starikovskiy <[email protected]>
CC Len Brown <[email protected]>
Quoted 65 lines
 drivers/acpi/power.c |   28 ++--------------------------
 1 files changed, 2 insertions(+), 26 deletions(-)
drivers/acpi/power.c +2 -26
index 56665a6..d74365d 100644
@@ -194,7 +194,7 @@ static int acpi_power_get_list_state(struct acpi_handle_list *list, int *state)
 
 static int acpi_power_on(acpi_handle handle, struct acpi_device *dev)
 {
-	int result = 0, state;
+	int result = 0;
 	int found = 0;
 	acpi_status status = AE_OK;
 	struct acpi_power_resource *resource = NULL;
@@ -236,18 +236,6 @@ static int acpi_power_on(acpi_handle handle, struct acpi_device *dev)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	if (!acpi_power_nocheck) {
-		/*
-		 * If acpi_power_nocheck is set, it is unnecessary to check
-		 * the power state after power transition.
-		 */
-		result = acpi_power_get_state(resource->device->handle,
-				&state);
-		if (result)
-			return result;
-		if (state != ACPI_POWER_RESOURCE_STATE_ON)
-			return -ENOEXEC;
-	}
 	/* Update the power resource's _device_ power state */
 	resource->device->power.state = ACPI_STATE_D0;
 
@@ -258,7 +246,7 @@ static int acpi_power_on(acpi_handle handle, struct acpi_device *dev)
 
 static int acpi_power_off_device(acpi_handle handle, struct acpi_device *dev)
 {
-	int result = 0, state;
+	int result = 0;
 	acpi_status status = AE_OK;
 	struct acpi_power_resource *resource = NULL;
 	struct list_head *node, *next;
@@ -293,18 +281,6 @@ static int acpi_power_off_device(acpi_handle handle, struct acpi_device *dev)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	if (!acpi_power_nocheck) {
-		/*
-		 * If acpi_power_nocheck is set, it is unnecessary to check
-		 * the power state after power transition.
-		 */
-		result = acpi_power_get_state(handle, &state);
-		if (result)
-			return result;
-		if (state != ACPI_POWER_RESOURCE_STATE_OFF)
-			return -ENOEXEC;
-	}
-
 	/* Update the power resource's _device_ power state */
 	resource->device->power.state = ACPI_STATE_D3;
 
Signature · 1 line
Matthew Garrett | [email protected]