* [PATCH 1/4] ACPI / PM: Make acpi_bus_init_power() more robust
2013-01-22 2:05 [PATCH 0/4] ACPI / PM: Initial power state checking and power state setting changes Rafael J. Wysocki
@ 2013-01-22 2:09 ` Rafael J. Wysocki
2013-01-22 7:10 ` Mika Westerberg
2013-01-22 2:09 ` [PATCH 2/4] ACPI / PM: Introduce helper for executing _PSn methods Rafael J. Wysocki
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2013-01-22 2:09 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML, Linux PM list, Lv Zheng, Mika Westerberg
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The ACPI specification requires the _PSC method to be present under
a device object if its power state cannot be inferred from the states
of power resources used by it (ACPI 5, Section 7.6.2). However, it
also requires that (for power states D0-D2 and D3hot) if the _PSn
(n = 0, 1, 2, 3) method is present under the device object, it also
must be executed after the power resources have been set
appropriately for the device to go into power state Dn (D3 means
D3hot in this case). Thus it is not clear from the specification
whether or not the _PSn method should be executed if the initial
configuraion of power resources used by the device indicates power
state Dn and the _PSC method is not present.
The current implementation of acpi_bus_init_power() is based on the
assumption that it should not be necessary to execute _PSn in the
above situation, but experience shows that in fact that assumption
need not be satisfied. For this reason, make acpi_bus_init_power()
always execute _PSn if the initial configuration of device power
resources indicates power state Dn.
Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/device_pm.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -330,13 +330,23 @@ int acpi_bus_init_power(struct acpi_devi
if (result)
return result;
- if (device->power.flags.power_resources)
+ if (state < ACPI_STATE_D3_COLD && device->power.flags.power_resources) {
result = acpi_power_on_resources(device, state);
+ if (result)
+ return result;
- if (!result)
- device->power.state = state;
+ if (device->power.states[state].flags.explicit_set) {
+ char method[5] = { '_', 'P', 'S', '0' + state, '\0' };
+ acpi_status status;
- return result;
+ status = acpi_evaluate_object(device->handle, method,
+ NULL, NULL);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+ }
+ }
+ device->power.state = state;
+ return 0;
}
int acpi_bus_update_power(acpi_handle handle, int *state_p)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/4] ACPI / PM: Make acpi_bus_init_power() more robust
2013-01-22 2:09 ` [PATCH 1/4] ACPI / PM: Make acpi_bus_init_power() more robust Rafael J. Wysocki
@ 2013-01-22 7:10 ` Mika Westerberg
0 siblings, 0 replies; 6+ messages in thread
From: Mika Westerberg @ 2013-01-22 7:10 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: ACPI Devel Maling List, LKML, Linux PM list, Lv Zheng
On Tue, Jan 22, 2013 at 03:09:01AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The ACPI specification requires the _PSC method to be present under
> a device object if its power state cannot be inferred from the states
> of power resources used by it (ACPI 5, Section 7.6.2). However, it
> also requires that (for power states D0-D2 and D3hot) if the _PSn
> (n = 0, 1, 2, 3) method is present under the device object, it also
> must be executed after the power resources have been set
> appropriately for the device to go into power state Dn (D3 means
> D3hot in this case). Thus it is not clear from the specification
> whether or not the _PSn method should be executed if the initial
> configuraion of power resources used by the device indicates power
> state Dn and the _PSC method is not present.
>
> The current implementation of acpi_bus_init_power() is based on the
> assumption that it should not be necessary to execute _PSn in the
> above situation, but experience shows that in fact that assumption
> need not be satisfied. For this reason, make acpi_bus_init_power()
> always execute _PSn if the initial configuration of device power
> resources indicates power state Dn.
>
> Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com>
You can add also,
Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
if you like. Thanks for fixing this.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/4] ACPI / PM: Introduce helper for executing _PSn methods
2013-01-22 2:05 [PATCH 0/4] ACPI / PM: Initial power state checking and power state setting changes Rafael J. Wysocki
2013-01-22 2:09 ` [PATCH 1/4] ACPI / PM: Make acpi_bus_init_power() more robust Rafael J. Wysocki
@ 2013-01-22 2:09 ` Rafael J. Wysocki
2013-01-22 2:10 ` [PATCH 3/4] ACPI / PM: Always evaluate _PSn after setting power resources Rafael J. Wysocki
2013-01-22 2:11 ` [PATCH 4/4] ACPI / PM: Sanitize checks in acpi_power_on_resources() Rafael J. Wysocki
3 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2013-01-22 2:09 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML, Linux PM list, Lv Zheng, Mika Westerberg
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
To reduce code duplication between acpi_device_set_power() and
acpi_bus_init_power(), introduce a new helper function for executing
ACPI devices' _PSn (n = 0..3) methods, acpi_dev_pm_explicit_set().
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/device_pm.c | 51 +++++++++++++++++++++--------------------------
1 file changed, 23 insertions(+), 28 deletions(-)
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -186,6 +186,19 @@ int acpi_device_get_power(struct acpi_de
return 0;
}
+static int acpi_dev_pm_explicit_set(struct acpi_device *adev, int state)
+{
+ if (adev->power.states[state].flags.explicit_set) {
+ char method[5] = { '_', 'P', 'S', '0' + state, '\0' };
+ acpi_status status;
+
+ status = acpi_evaluate_object(adev->handle, method, NULL, NULL);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+ }
+ return 0;
+}
+
/**
* acpi_device_set_power - Set power state of an ACPI device.
* @device: Device to set the power state of.
@@ -197,8 +210,6 @@ int acpi_device_get_power(struct acpi_de
int acpi_device_set_power(struct acpi_device *device, int state)
{
int result = 0;
- acpi_status status = AE_OK;
- char object_name[5] = { '_', 'P', 'S', '0' + state, '\0' };
bool cut_power = false;
if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3_COLD))
@@ -228,7 +239,6 @@ int acpi_device_set_power(struct acpi_de
if (state == ACPI_STATE_D3_COLD
&& device->power.states[ACPI_STATE_D3_COLD].flags.os_accessible) {
state = ACPI_STATE_D3_HOT;
- object_name[3] = '3';
cut_power = true;
}
@@ -251,23 +261,14 @@ int acpi_device_set_power(struct acpi_de
if (result)
goto end;
}
- if (device->power.states[state].flags.explicit_set) {
- status = acpi_evaluate_object(device->handle,
- object_name, NULL, NULL);
- if (ACPI_FAILURE(status)) {
- result = -ENODEV;
- goto end;
- }
- }
+ result = acpi_dev_pm_explicit_set(device, state);
+ if (result)
+ goto end;
} else {
- if (device->power.states[state].flags.explicit_set) {
- status = acpi_evaluate_object(device->handle,
- object_name, NULL, NULL);
- if (ACPI_FAILURE(status)) {
- result = -ENODEV;
- goto end;
- }
- }
+ result = acpi_dev_pm_explicit_set(device, state);
+ if (result)
+ goto end;
+
if (device->power.flags.power_resources) {
result = acpi_power_transition(device, state);
if (result)
@@ -335,15 +336,9 @@ int acpi_bus_init_power(struct acpi_devi
if (result)
return result;
- if (device->power.states[state].flags.explicit_set) {
- char method[5] = { '_', 'P', 'S', '0' + state, '\0' };
- acpi_status status;
-
- status = acpi_evaluate_object(device->handle, method,
- NULL, NULL);
- if (ACPI_FAILURE(status))
- return -ENODEV;
- }
+ result = acpi_dev_pm_explicit_set(device, state);
+ if (result)
+ return result;
}
device->power.state = state;
return 0;
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/4] ACPI / PM: Always evaluate _PSn after setting power resources
2013-01-22 2:05 [PATCH 0/4] ACPI / PM: Initial power state checking and power state setting changes Rafael J. Wysocki
2013-01-22 2:09 ` [PATCH 1/4] ACPI / PM: Make acpi_bus_init_power() more robust Rafael J. Wysocki
2013-01-22 2:09 ` [PATCH 2/4] ACPI / PM: Introduce helper for executing _PSn methods Rafael J. Wysocki
@ 2013-01-22 2:10 ` Rafael J. Wysocki
2013-01-22 2:11 ` [PATCH 4/4] ACPI / PM: Sanitize checks in acpi_power_on_resources() Rafael J. Wysocki
3 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2013-01-22 2:10 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML, Linux PM list, Lv Zheng, Mika Westerberg
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The ACPI specitication (ACPI 5, Sections 7.2.8 - 7.2.11) requires
that the _PSn (n = 0..3) method, if present, be executed after the
power resources for the given device power state have been set
appropriately. However, acpi_device_set_power() does that only
if the new power state is going to be higher-power (lower-number)
than the power state the device is in already. Otherwise, the
ordering is reverse to protect against situations in which _PSn
might access device registers unavailable after configuring the
power resources for power state Dn (D3 meaning D3hot).
Such situations are very unlikely to happen, though, and _PSn may
actually be implemented with the assumption that power resources
have been configured for power state Dn in advance, so change the
code to follow the specification literally.
This change was previously porposed in a different form by Lv Zheng.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/device_pm.c | 46 +++++++++++++++++-----------------------------
1 file changed, 17 insertions(+), 29 deletions(-)
Index: linux-pm/drivers/acpi/device_pm.c
===================================================================
--- linux-pm.orig/drivers/acpi/device_pm.c
+++ linux-pm/drivers/acpi/device_pm.c
@@ -242,50 +242,38 @@ int acpi_device_set_power(struct acpi_de
cut_power = true;
}
+ if (state < device->power.state && state != ACPI_STATE_D0
+ && device->power.state >= ACPI_STATE_D3_HOT) {
+ printk(KERN_WARNING PREFIX
+ "Cannot transition to non-D0 state from D3\n");
+ return -ENODEV;
+ }
+
/*
* Transition Power
* ----------------
- * On transitions to a high-powered state we first apply power (via
- * power resources) then evalute _PSx. Conversly for transitions to
- * a lower-powered state.
+ * In accordance with the ACPI specification first apply power (via
+ * power resources) and then evalute _PSx.
*/
- if (state < device->power.state) {
- if (device->power.state >= ACPI_STATE_D3_HOT &&
- state != ACPI_STATE_D0) {
- printk(KERN_WARNING PREFIX
- "Cannot transition to non-D0 state from D3\n");
- return -ENODEV;
- }
- if (device->power.flags.power_resources) {
- result = acpi_power_transition(device, state);
- if (result)
- goto end;
- }
- result = acpi_dev_pm_explicit_set(device, state);
- if (result)
- goto end;
- } else {
- result = acpi_dev_pm_explicit_set(device, state);
+ if (device->power.flags.power_resources) {
+ result = acpi_power_transition(device, state);
if (result)
goto end;
-
- if (device->power.flags.power_resources) {
- result = acpi_power_transition(device, state);
- if (result)
- goto end;
- }
}
+ result = acpi_dev_pm_explicit_set(device, state);
+ if (result)
+ goto end;
if (cut_power)
result = acpi_power_transition(device, ACPI_STATE_D3_COLD);
- end:
- if (result)
+ end:
+ if (result) {
printk(KERN_WARNING PREFIX
"Device [%s] failed to transition to %s\n",
device->pnp.bus_id,
acpi_power_state_string(state));
- else {
+ } else {
device->power.state = state;
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"Device [%s] transitioned to %s\n",
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 4/4] ACPI / PM: Sanitize checks in acpi_power_on_resources()
2013-01-22 2:05 [PATCH 0/4] ACPI / PM: Initial power state checking and power state setting changes Rafael J. Wysocki
` (2 preceding siblings ...)
2013-01-22 2:10 ` [PATCH 3/4] ACPI / PM: Always evaluate _PSn after setting power resources Rafael J. Wysocki
@ 2013-01-22 2:11 ` Rafael J. Wysocki
3 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2013-01-22 2:11 UTC (permalink / raw)
To: ACPI Devel Maling List; +Cc: LKML, Linux PM list, Lv Zheng, Mika Westerberg
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
After the only user of acpi_power_on_resources(),
acpi_bus_init_power(), has been changed to avoid calling it
for state equal to ACPI_STATE_D3_COLD, it doesn't have to special
case that state any more.
For this reason, modify the checks in acpi_power_on_resources()
so that it returns -EINVAL for ACPI_STATE_D3_COLD as it should.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/power.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
Index: linux-pm/drivers/acpi/power.c
===================================================================
--- linux-pm.orig/drivers/acpi/power.c
+++ linux-pm/drivers/acpi/power.c
@@ -622,12 +622,9 @@ int acpi_power_get_inferred_state(struct
int acpi_power_on_resources(struct acpi_device *device, int state)
{
- if (!device || state < ACPI_STATE_D0 || state > ACPI_STATE_D3_COLD)
+ if (!device || state < ACPI_STATE_D0 || state > ACPI_STATE_D3_HOT)
return -EINVAL;
- if (state == ACPI_STATE_D3_COLD)
- return 0;
-
return acpi_power_on_list(&device->power.states[state].resources);
}
^ permalink raw reply [flat|nested] 6+ messages in thread