2010-11-24 23:16:38

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 0/13] ACPI / PM: Rework power resources management

Hi,

Unfortunately there are a few problems with ACPI power resources management
that should be fixed by the following series of patches.

Patch [1/13] is necessary to fix a recent regression in the mainline kernel and
patch [2/13] also should go into 2.6.37 IMO. The remaining patches are not so
urgent, but they all are necessary.

[1/13] - Fix regression if fan resume resulting from the rework of power
resources reference counting.

[2/13] - Do not reference count power resources that can't be turned on.

[3/13] - Rework acpi_power_get_inferred_state() so that it doesn't update
device->power.state behind the caller's back.

[4/13] - Introduce functions for handling lists of power resources.

[5/13] - Introduce helper function for reference counting power resources
for a given device and power state.

[6/13] - Introduce function for reading device power state without modifying
the device object.

[7/13] - Add function for device power state initialization.

[8/13] - Add function for updating device power state in a consistent way.

[9/13] - Register acpi_power_driver before scanning the root of the namespace.

[10/13] - Add power resource ACPI device objects as soon as they are needed.

[11/13] - Rework the management of ACPI power resources in the ACPI fan driver.

[12/13] - Drop acpi_bus_get_power() that has no users now.

[13/13] - Drop acpi_power_nocheck which isn't necessary any more.

The patches have been tested on my HP nx6325 (which is the only machine using
ACPI power resources I have) and a combo patch functionally equivalen to this
series has been tested by Maciej Rutecki.

Please apply.

Thanks,
Rafael


2010-11-24 23:16:41

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 4/13] ACPI / PM: Add functions for manipulating lists of power resources

From: Rafael J. Wysocki <[email protected]>

ACPI device power resources should be reference counted during
device initialization, so that their reference counters are always
up to date. It is convenient to do that with the help of a function
that will reference count and possibly turn on power resources in
a given list, so introduce that function, acpi_power_on_list().
For symmetry, introduce acpi_power_off_list() for performing the
reverse operation and use the both of them to simplify
acpi_power_transition().

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/power.c | 67 ++++++++++++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 29 deletions(-)

Index: linux-2.6/drivers/acpi/power.c
===================================================================
--- linux-2.6.orig/drivers/acpi/power.c
+++ linux-2.6/drivers/acpi/power.c
@@ -266,6 +266,35 @@ static int acpi_power_off_device(acpi_ha
return result;
}

+static void __acpi_power_off_list(struct acpi_handle_list *list, int num_res)
+{
+ int i;
+
+ for (i = num_res - 1; i >= 0 ; i--)
+ acpi_power_off_device(list->handles[i]);
+}
+
+static void acpi_power_off_list(struct acpi_handle_list *list)
+{
+ __acpi_power_off_list(list, list->count);
+}
+
+static int acpi_power_on_list(struct acpi_handle_list *list)
+{
+ int result = 0;
+ int i;
+
+ for (i = 0; i < list->count; i++) {
+ result = acpi_power_on(list->handles[i]);
+ if (result) {
+ __acpi_power_off_list(list, i);
+ break;
+ }
+ }
+
+ return result;
+}
+
/**
* acpi_device_sleep_wake - execute _DSW (Device Sleep Wake) or (deprecated in
* ACPI 3.0) _PSW (Power State Wake)
@@ -458,10 +487,7 @@ int acpi_power_get_inferred_state(struct

int acpi_power_transition(struct acpi_device *device, int state)
{
- int result = 0;
- struct acpi_handle_list *cl = NULL; /* Current Resources */
- struct acpi_handle_list *tl = NULL; /* Target Resources */
- int i = 0;
+ int result;

if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3))
return -EINVAL;
@@ -473,37 +499,20 @@ int acpi_power_transition(struct acpi_de
|| (device->power.state > ACPI_STATE_D3))
return -ENODEV;

- cl = &device->power.states[device->power.state].resources;
- tl = &device->power.states[state].resources;
-
/* TBD: Resources must be ordered. */

/*
* First we reference all power resources required in the target list
- * (e.g. so the device doesn't lose power while transitioning).
- */
- for (i = 0; i < tl->count; i++) {
- result = acpi_power_on(tl->handles[i]);
- if (result)
- goto end;
- }
-
- /*
- * Then we dereference all power resources used in the current list.
+ * (e.g. so the device doesn't lose power while transitioning). Then,
+ * we dereference all power resources used in the current list.
*/
- for (i = 0; i < cl->count; i++) {
- result = acpi_power_off_device(cl->handles[i]);
- if (result)
- goto end;
- }
+ result = acpi_power_on_list(&device->power.states[state].resources);
+ if (!result)
+ acpi_power_off_list(
+ &device->power.states[device->power.state].resources);

- end:
- if (result)
- device->power.state = ACPI_STATE_UNKNOWN;
- else {
- /* We shouldn't change the state till all above operations succeed */
- device->power.state = state;
- }
+ /* We shouldn't change the state unless the above operations succeed. */
+ device->power.state = result ? ACPI_STATE_UNKNOWN : state;

return result;
}

2010-11-24 23:16:55

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 3/13] ACPI / PM: Prevent acpi_power_get_inferred_state() from making changes

From: Rafael J. Wysocki <[email protected]>

acpi_power_get_inferred_state() should not update
device->power.state behind the back of its caller, so make it return
the state via a pointer instead.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 3 ++-
drivers/acpi/internal.h | 2 +-
drivers/acpi/power.c | 12 ++++--------
3 files changed, 7 insertions(+), 10 deletions(-)

Index: linux-2.6/drivers/acpi/power.c
===================================================================
--- linux-2.6.orig/drivers/acpi/power.c
+++ linux-2.6/drivers/acpi/power.c
@@ -423,19 +423,16 @@ int acpi_disable_wakeup_device_power(str
Device Power Management
-------------------------------------------------------------------------- */

-int acpi_power_get_inferred_state(struct acpi_device *device)
+int acpi_power_get_inferred_state(struct acpi_device *device, int *state)
{
int result = 0;
struct acpi_handle_list *list = NULL;
int list_state = 0;
int i = 0;

-
- if (!device)
+ if (!device || !state)
return -EINVAL;

- device->power.state = ACPI_STATE_UNKNOWN;
-
/*
* We know a device's inferred power state when all the resources
* required for a given D-state are 'on'.
@@ -450,13 +447,12 @@ int acpi_power_get_inferred_state(struct
return result;

if (list_state == ACPI_POWER_RESOURCE_STATE_ON) {
- device->power.state = i;
+ *state = i;
return 0;
}
}

- device->power.state = ACPI_STATE_D3;
-
+ *state = ACPI_STATE_D3;
return 0;
}

Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -222,7 +222,8 @@ int acpi_bus_get_power(acpi_handle handl
* indirectly (via power resources).
*/
if (device->power.flags.power_resources) {
- result = acpi_power_get_inferred_state(device);
+ result = acpi_power_get_inferred_state(device,
+ &device->power.state);
if (result)
return result;
} else if (device->power.flags.explicit_get) {
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
--- linux-2.6.orig/drivers/acpi/internal.h
+++ linux-2.6/drivers/acpi/internal.h
@@ -41,7 +41,7 @@ static inline int acpi_debugfs_init(void
int acpi_power_init(void);
int acpi_device_sleep_wake(struct acpi_device *dev,
int enable, int sleep_state, int dev_state);
-int acpi_power_get_inferred_state(struct acpi_device *device);
+int acpi_power_get_inferred_state(struct acpi_device *device, int *state);
int acpi_power_transition(struct acpi_device *device, int state);
extern int acpi_power_nocheck;

2010-11-24 23:16:51

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 1/13] ACPI / PM: Check device state before refcounting power resources

From: Rafael J. Wysocki <[email protected]>

Commit 3e384ee6c687cb397581ee8f9440fc8220cfac80 (ACPI / PM: Fix
reference counting of power resources) introduced a regression by
causing fan power resources to be turned on and reference counted
unnecessarily during resume, so on some boxes fans are always on
after resume.

Fix the problem by checking if the current device state is different
from the new state before reference counting and turning on power
resources in acpi_power_transition().

Addresses https://bugzilla.kernel.org/show_bug.cgi?id=22932 .

Signed-off-by: Rafael J. Wysocki <[email protected]>
Reported-and-tested-by: Maciej Rutecki <[email protected]>
---
drivers/acpi/power.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

Index: linux-2.6/drivers/acpi/power.c
===================================================================
--- linux-2.6.orig/drivers/acpi/power.c
+++ linux-2.6/drivers/acpi/power.c
@@ -465,10 +465,12 @@ int acpi_power_transition(struct acpi_de
struct acpi_handle_list *tl = NULL; /* Target Resources */
int i = 0;

-
if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3))
return -EINVAL;

+ if (device->power.state == state)
+ return 0;
+
if ((device->power.state < ACPI_STATE_D0)
|| (device->power.state > ACPI_STATE_D3))
return -ENODEV;
@@ -488,10 +490,6 @@ int acpi_power_transition(struct acpi_de
goto end;
}

- if (device->power.state == state) {
- goto end;
- }
-
/*
* Then we dereference all power resources used in the current list.
*/

2010-11-24 23:17:20

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 9/13] ACPI / PM: Register acpi_power_driver early

From: Rafael J. Wysocki <[email protected]>

The ACPI device driver used for handling power resources,
acpi_power_driver, creates a struct acpi_power_resource object for
each ACPI device representing a power resource. These objects are
then used when setting and reading the power states of devices using
the corresponding power resources. Unfortunately, acpi_power_driver
is registered after acpi_scan_init() that may add devices using the
power resources before acpi_power_driver has a chance to create
struct acpi_power_resource objects for them (specifically, the power
resources may be referred to during the scanning process through
acpi_bus_get_power() before they have been initialized).

As the first step towards fixing this issue, move the registration
of acpi_power_driver into acpi_scan_init() so that power resource
devices can be initialized by it as soon as they have been found in
the namespace.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 1 -
drivers/acpi/scan.c | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -1099,7 +1099,6 @@ static int __init acpi_init(void)

acpi_scan_init();
acpi_ec_init();
- acpi_power_init();
acpi_debugfs_init();
acpi_sleep_proc_init();
acpi_wakeup_device_init();
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -1547,6 +1547,8 @@ int __init acpi_scan_init(void)
printk(KERN_ERR PREFIX "Could not register bus type\n");
}

+ acpi_power_init();
+
/*
* Enumerate devices in the ACPI namespace.
*/

2010-11-24 23:17:24

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 12/13] ACPI / PM: Drop acpi_bus_get_power()

From: Rafael J. Wysocki <[email protected]>

There are no more users of acpi_bus_get_power(), so it can be
dropped. Moreover, it should be dropped, because it modifies
the device->power.state field of an ACPI device without updating
the reference counters of the device's power resources, which is
wrong.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 19 -------------------
include/acpi/acpi_bus.h | 1 -
2 files changed, 20 deletions(-)

Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -236,25 +236,6 @@ static int __acpi_bus_get_power(struct a
}


-int acpi_bus_get_power(acpi_handle handle, int *state)
-{
- struct acpi_device *device;
- int result;
-
- result = acpi_bus_get_device(handle, &device);
- if (result)
- return result;
-
- result = __acpi_bus_get_power(device, state);
- if (result)
- return result;
-
- device->power.state = *state;
- return 0;
-}
-EXPORT_SYMBOL(acpi_bus_get_power);
-
-
static int __acpi_bus_set_power(struct acpi_device *device, int state)
{
int result = 0;
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -327,7 +327,6 @@ void acpi_bus_data_handler(acpi_handle h
acpi_status acpi_bus_get_status_handle(acpi_handle handle,
unsigned long long *sta);
int acpi_bus_get_status(struct acpi_device *device);
-int acpi_bus_get_power(acpi_handle handle, int *state);
int acpi_bus_set_power(acpi_handle handle, int state);
int acpi_bus_update_power(acpi_handle handle, int *state_p);
bool acpi_bus_power_manageable(acpi_handle handle);

2010-11-24 23:17:56

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 10/13] ACPI / PM: Register power resource devices as soon as they are needed

From: Rafael J. Wysocki <[email protected]>

Depending on the organization of the ACPI namespace, power resource
device objects may generally be scanned after the "regular" device
objects that they are referred from through _PRn. This, in turn, may
cause acpi_bus_get_power_flags() to attempt to access them through
acpi_bus_init_power() before they are registered (and initialized by
acpi_power_driver). [This is not a theoretical issue, it actually
happens for one PnP device on my testbed HP nx6325.]

To fix this problem, make acpi_bus_get_power_flags() attempt to
register power resource devices as soon as they have been found in
the _PRn output for any other devices.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/scan.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -827,6 +827,8 @@ end:
return 0;
}

+static void acpi_bus_add_power_resource(acpi_handle handle);
+
static int acpi_bus_get_power_flags(struct acpi_device *device)
{
acpi_status status = 0;
@@ -855,8 +857,12 @@ static int acpi_bus_get_power_flags(stru
acpi_evaluate_reference(device->handle, object_name, NULL,
&ps->resources);
if (ps->resources.count) {
+ int j;
+
device->power.flags.power_resources = 1;
ps->flags.valid = 1;
+ for (j = 0; j < ps->resources.count; j++)
+ acpi_bus_add_power_resource(ps->resources.handles[j]);
}

/* Evaluate "_PSx" to see if we can do explicit sets */
@@ -1303,6 +1309,20 @@ end:
#define ACPI_STA_DEFAULT (ACPI_STA_DEVICE_PRESENT | ACPI_STA_DEVICE_ENABLED | \
ACPI_STA_DEVICE_UI | ACPI_STA_DEVICE_FUNCTIONING)

+static void acpi_bus_add_power_resource(acpi_handle handle)
+{
+ struct acpi_bus_ops ops = {
+ .acpi_op_add = 1,
+ .acpi_op_start = 1,
+ };
+ struct acpi_device *device = NULL;
+
+ acpi_bus_get_device(handle, &device);
+ if (!device)
+ acpi_add_single_object(&device, handle, ACPI_BUS_TYPE_POWER,
+ ACPI_STA_DEFAULT, &ops);
+}
+
static int acpi_bus_type_and_status(acpi_handle handle, int *type,
unsigned long long *sta)
{

2010-11-24 23:17:41

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 8/13] ACPI / PM: Add function for updating device power state consistently

From: Rafael J. Wysocki <[email protected]>

Add function acpi_bus_update_power() for reading the actual power
state of an ACPI device and updating its device->power.state field
in such a way that its power resources' reference counters will
remain consistent with that field.

For this purpose introduce __acpi_bus_set_power() setting the
power state of an ACPI device without updating its
device->power.state field and make acpi_bus_set_power() and
acpi_bus_update_power() use it (acpi_bus_set_power() retains the
current behavior for now).

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 90 ++++++++++++++++++++++++++++++++----------------
include/acpi/acpi_bus.h | 1
2 files changed, 62 insertions(+), 29 deletions(-)

Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -255,44 +255,17 @@ int acpi_bus_get_power(acpi_handle handl
EXPORT_SYMBOL(acpi_bus_get_power);


-int acpi_bus_set_power(acpi_handle handle, int state)
+static int __acpi_bus_set_power(struct acpi_device *device, int state)
{
int result = 0;
acpi_status status = AE_OK;
- struct acpi_device *device = NULL;
char object_name[5] = { '_', 'P', 'S', '0' + state, '\0' };

-
- result = acpi_bus_get_device(handle, &device);
- if (result)
- return result;
-
- if ((state < ACPI_STATE_D0) || (state > ACPI_STATE_D3))
+ if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3))
return -EINVAL;

/* Make sure this is a valid target state */

- if (!device->flags.power_manageable) {
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device `[%s]' is not power manageable\n",
- kobject_name(&device->dev.kobj)));
- return -ENODEV;
- }
- /*
- * Get device's current power state
- */
- if (!acpi_power_nocheck) {
- /*
- * Maybe the incorrect power state is returned on the bogus
- * bios, which is different with the real power state.
- * For example: the bios returns D0 state and the real power
- * state is D3. OS expects to set the device to D0 state. In
- * such case if OS uses the power state returned by the BIOS,
- * the device can't be transisted to the correct power state.
- * So if the acpi_power_nocheck is set, it is unnecessary to
- * get the power state by calling acpi_bus_get_power.
- */
- acpi_bus_get_power(device->handle, &device->power.state);
- }
if ((state == device->power.state) && !device->flags.force_power_state) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at D%d\n",
state));
@@ -362,6 +335,42 @@ int acpi_bus_set_power(acpi_handle handl
return result;
}

+
+int acpi_bus_set_power(acpi_handle handle, int state)
+{
+ struct acpi_device *device;
+ int result;
+
+ result = acpi_bus_get_device(handle, &device);
+ if (result)
+ return result;
+
+ if (!device->flags.power_manageable) {
+ ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+ "Device [%s] is not power manageable\n",
+ dev_name(&device->dev)));
+ return -ENODEV;
+ }
+
+ /*
+ * Get device's current power state
+ */
+ if (!acpi_power_nocheck) {
+ /*
+ * Maybe the incorrect power state is returned on the bogus
+ * bios, which is different with the real power state.
+ * For example: the bios returns D0 state and the real power
+ * state is D3. OS expects to set the device to D0 state. In
+ * such case if OS uses the power state returned by the BIOS,
+ * the device can't be transisted to the correct power state.
+ * So if the acpi_power_nocheck is set, it is unnecessary to
+ * get the power state by calling acpi_bus_get_power.
+ */
+ __acpi_bus_get_power(device, &device->power.state);
+ }
+
+ return __acpi_bus_set_power(device, state);
+}
EXPORT_SYMBOL(acpi_bus_set_power);


@@ -389,6 +398,29 @@ int acpi_bus_init_power(struct acpi_devi
}


+int acpi_bus_update_power(acpi_handle handle, int *state_p)
+{
+ struct acpi_device *device;
+ int state;
+ int result;
+
+ result = acpi_bus_get_device(handle, &device);
+ if (result)
+ return result;
+
+ result = __acpi_bus_get_power(device, &state);
+ if (result)
+ return result;
+
+ result = __acpi_bus_set_power(device, state);
+ if (!result && state_p)
+ *state_p = state;
+
+ return result;
+}
+EXPORT_SYMBOL_GPL(acpi_bus_update_power);
+
+
bool acpi_bus_power_manageable(acpi_handle handle)
{
struct acpi_device *device;
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -330,6 +330,7 @@ acpi_status acpi_bus_get_status_handle(a
int acpi_bus_get_status(struct acpi_device *device);
int acpi_bus_get_power(acpi_handle handle, int *state);
int acpi_bus_set_power(acpi_handle handle, int state);
+int acpi_bus_update_power(acpi_handle handle, int *state_p);
bool acpi_bus_power_manageable(acpi_handle handle);
bool acpi_bus_can_wakeup(acpi_handle handle);
#ifdef CONFIG_ACPI_PROC_EVENT

2010-11-24 23:17:14

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 6/13] ACPI / PM: Introduce __acpi_bus_get_power()

From: Rafael J. Wysocki <[email protected]>

It sometimes is necessary to get the power state of an ACPI device
without updating its device->power.state field, for example to
avoid inconsistencies between device->power.state and the reference
counters of the device's power resources. For this purpose introduce
__acpi_bus_get_power() that will return the given device's power
state via a pointer (instead of modifying device->power.state)
and make acpi_bus_get_power() use it.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 48 +++++++++++++++++++++++++++++-------------------
1 file changed, 29 insertions(+), 19 deletions(-)

Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -196,34 +196,24 @@ EXPORT_SYMBOL(acpi_bus_get_private_data)
Power Management
-------------------------------------------------------------------------- */

-int acpi_bus_get_power(acpi_handle handle, int *state)
+static int __acpi_bus_get_power(struct acpi_device *device, int *state)
{
int result = 0;
acpi_status status = 0;
- struct acpi_device *device = NULL;
unsigned long long psc = 0;

-
- result = acpi_bus_get_device(handle, &device);
- if (result)
- return result;
+ if (!device || !state)
+ return -EINVAL;

*state = ACPI_STATE_UNKNOWN;

- if (!device->flags.power_manageable) {
- /* TBD: Non-recursive algorithm for walking up hierarchy */
- if (device->parent)
- *state = device->parent->power.state;
- else
- *state = ACPI_STATE_D0;
- } else {
+ if (device->flags.power_manageable) {
/*
* Get the device's power state either directly (via _PSC) or
* indirectly (via power resources).
*/
if (device->power.flags.power_resources) {
- result = acpi_power_get_inferred_state(device,
- &device->power.state);
+ result = acpi_power_get_inferred_state(device, state);
if (result)
return result;
} else if (device->power.flags.explicit_get) {
@@ -231,20 +221,40 @@ int acpi_bus_get_power(acpi_handle handl
NULL, &psc);
if (ACPI_FAILURE(status))
return -ENODEV;
- device->power.state = (int)psc;
+ *state = (int)psc;
}
-
- *state = device->power.state;
+ } else {
+ /* TBD: Non-recursive algorithm for walking up hierarchy. */
+ *state = device->parent ?
+ device->parent->power.state : ACPI_STATE_D0;
}

ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] power state is D%d\n",
- device->pnp.bus_id, device->power.state));
+ device->pnp.bus_id, *state));

return 0;
}

+
+int acpi_bus_get_power(acpi_handle handle, int *state)
+{
+ struct acpi_device *device;
+ int result;
+
+ result = acpi_bus_get_device(handle, &device);
+ if (result)
+ return result;
+
+ result = __acpi_bus_get_power(device, state);
+ if (result)
+ return result;
+
+ device->power.state = *state;
+ return 0;
+}
EXPORT_SYMBOL(acpi_bus_get_power);

+
int acpi_bus_set_power(acpi_handle handle, int state)
{
int result = 0;

2010-11-24 23:17:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 5/13] ACPI / PM: Introduce function for refcounting device power resources

From: Rafael J. Wysocki <[email protected]>

Introduce function acpi_power_on_resources() that reference counts
and possibly turns on ACPI power resources for a given device and
a given power state of it.

This function will be used for reference counting device power
resources during initialization.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/internal.h | 1 +
drivers/acpi/power.c | 8 ++++++++
2 files changed, 9 insertions(+)

Index: linux-2.6/drivers/acpi/power.c
===================================================================
--- linux-2.6.orig/drivers/acpi/power.c
+++ linux-2.6/drivers/acpi/power.c
@@ -485,6 +485,14 @@ int acpi_power_get_inferred_state(struct
return 0;
}

+int acpi_power_on_resources(struct acpi_device *device, int state)
+{
+ if (!device || state < ACPI_STATE_D0 || state > ACPI_STATE_D3)
+ return -EINVAL;
+
+ return acpi_power_on_list(&device->power.states[state].resources);
+}
+
int acpi_power_transition(struct acpi_device *device, int state)
{
int result;
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
--- linux-2.6.orig/drivers/acpi/internal.h
+++ linux-2.6/drivers/acpi/internal.h
@@ -42,6 +42,7 @@ int acpi_power_init(void);
int acpi_device_sleep_wake(struct acpi_device *dev,
int enable, int sleep_state, int dev_state);
int acpi_power_get_inferred_state(struct acpi_device *device, int *state);
+int acpi_power_on_resources(struct acpi_device *device, int state);
int acpi_power_transition(struct acpi_device *device, int state);
extern int acpi_power_nocheck;

2010-11-24 23:18:11

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 13/13] ACPI / PM: Drop acpi_power_nocheck

From: Rafael J. Wysocki <[email protected]>

Since acpi_bus_set_power() should not use __acpi_bus_get_power() to
update the device's device->power.state field before changing its
power state (this may cause device->power.state to be inconsistent
with the device power resources' reference counters), remove this
call from it. In consequence, the acpi_power_nocheck variable is not
necessary any more, so it can be dropped along with the DMI table
used for setting that variable for HP Pavilion 05.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 39 ---------------------------------------
drivers/acpi/internal.h | 1 -
drivers/acpi/power.c | 3 ---
3 files changed, 43 deletions(-)

Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -52,22 +52,6 @@ EXPORT_SYMBOL(acpi_root_dir);

#define STRUCT_TO_INT(s) (*((int*)&s))

-static int set_power_nocheck(const struct dmi_system_id *id)
-{
- printk(KERN_NOTICE PREFIX "%s detected - "
- "disable power check in power transition\n", id->ident);
- acpi_power_nocheck = 1;
- return 0;
-}
-static struct dmi_system_id __cpuinitdata power_nocheck_dmi_table[] = {
- {
- set_power_nocheck, "HP Pavilion 05", {
- DMI_MATCH(DMI_BIOS_VENDOR, "Phoenix Technologies LTD"),
- DMI_MATCH(DMI_SYS_VENDOR, "HP Pavilion 05"),
- DMI_MATCH(DMI_PRODUCT_VERSION, "2001211RE101GLEND") }, NULL},
- {},
-};
-

#ifdef CONFIG_X86
static int set_copy_dsdt(const struct dmi_system_id *id)
@@ -333,23 +317,6 @@ int acpi_bus_set_power(acpi_handle handl
return -ENODEV;
}

- /*
- * Get device's current power state
- */
- if (!acpi_power_nocheck) {
- /*
- * Maybe the incorrect power state is returned on the bogus
- * bios, which is different with the real power state.
- * For example: the bios returns D0 state and the real power
- * state is D3. OS expects to set the device to D0 state. In
- * such case if OS uses the power state returned by the BIOS,
- * the device can't be transisted to the correct power state.
- * So if the acpi_power_nocheck is set, it is unnecessary to
- * get the power state by calling acpi_bus_get_power.
- */
- __acpi_bus_get_power(device, &device->power.state);
- }
-
return __acpi_bus_set_power(device, state);
}
EXPORT_SYMBOL(acpi_bus_set_power);
@@ -1072,12 +1039,6 @@ static int __init acpi_init(void)
if (acpi_disabled)
return result;

- /*
- * If the laptop falls into the DMI check table, the power state check
- * will be disabled in the course of device power transition.
- */
- dmi_check_system(power_nocheck_dmi_table);
-
acpi_scan_init();
acpi_ec_init();
acpi_debugfs_init();
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
--- linux-2.6.orig/drivers/acpi/internal.h
+++ linux-2.6/drivers/acpi/internal.h
@@ -45,7 +45,6 @@ int acpi_power_get_inferred_state(struct
int acpi_power_on_resources(struct acpi_device *device, int state);
int acpi_power_transition(struct acpi_device *device, int state);
int acpi_bus_init_power(struct acpi_device *device);
-extern int acpi_power_nocheck;

int acpi_wakeup_device_init(void);
void acpi_early_processor_set_pdc(void);
Index: linux-2.6/drivers/acpi/power.c
===================================================================
--- linux-2.6.orig/drivers/acpi/power.c
+++ linux-2.6/drivers/acpi/power.c
@@ -56,9 +56,6 @@ ACPI_MODULE_NAME("power");
#define ACPI_POWER_RESOURCE_STATE_ON 0x01
#define ACPI_POWER_RESOURCE_STATE_UNKNOWN 0xFF

-int acpi_power_nocheck;
-module_param_named(power_nocheck, acpi_power_nocheck, bool, 000);
-
static int acpi_power_add(struct acpi_device *device);
static int acpi_power_remove(struct acpi_device *device, int type);
static int acpi_power_resume(struct acpi_device *device);

2010-11-24 23:18:14

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 11/13] ACPI / Fan: Rework the handling of power resources

From: Rafael J. Wysocki <[email protected]>

Use the new function acpi_bus_update_power() for manipulating power
resources used by ACPI fan devices, which allows them to be put into
the right state during initialization and resume. Consequently,
remove the flags.force_power_state field from struct acpi_device,
which is not necessary any more.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 2 +-
drivers/acpi/fan.c | 27 +++++++--------------------
drivers/acpi/thermal.c | 5 +++--
include/acpi/acpi_bus.h | 3 +--
4 files changed, 12 insertions(+), 25 deletions(-)

Index: linux-2.6/drivers/acpi/fan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/fan.c
+++ linux-2.6/drivers/acpi/fan.c
@@ -86,7 +86,7 @@ static int fan_get_cur_state(struct ther
if (!device)
return -EINVAL;

- result = acpi_bus_get_power(device->handle, &acpi_state);
+ result = acpi_bus_update_power(device->handle, &acpi_state);
if (result)
return result;

@@ -123,7 +123,6 @@ static struct thermal_cooling_device_ops
static int acpi_fan_add(struct acpi_device *device)
{
int result = 0;
- int state = 0;
struct thermal_cooling_device *cdev;

if (!device)
@@ -132,16 +131,12 @@ static int acpi_fan_add(struct acpi_devi
strcpy(acpi_device_name(device), "Fan");
strcpy(acpi_device_class(device), ACPI_FAN_CLASS);

- result = acpi_bus_get_power(device->handle, &state);
+ result = acpi_bus_update_power(device->handle, NULL);
if (result) {
- printk(KERN_ERR PREFIX "Reading power state\n");
+ printk(KERN_ERR PREFIX "Setting initial power state\n");
goto end;
}

- device->flags.force_power_state = 1;
- acpi_bus_set_power(device->handle, state);
- device->flags.force_power_state = 0;
-
cdev = thermal_cooling_device_register("Fan", device,
&fan_cooling_ops);
if (IS_ERR(cdev)) {
@@ -200,22 +195,14 @@ static int acpi_fan_suspend(struct acpi_

static int acpi_fan_resume(struct acpi_device *device)
{
- int result = 0;
- int power_state = 0;
+ int result;

if (!device)
return -EINVAL;

- result = acpi_bus_get_power(device->handle, &power_state);
- if (result) {
- printk(KERN_ERR PREFIX
- "Error reading fan power state\n");
- return result;
- }
-
- device->flags.force_power_state = 1;
- acpi_bus_set_power(device->handle, power_state);
- device->flags.force_power_state = 0;
+ result = acpi_bus_update_power(device->handle, NULL);
+ if (result)
+ printk(KERN_ERR PREFIX "Error updating fan power state\n");

return result;
}
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -149,8 +149,7 @@ struct acpi_device_flags {
u32 power_manageable:1;
u32 performance_manageable:1;
u32 wake_capable:1; /* Wakeup(_PRW) supported? */
- u32 force_power_state:1;
- u32 reserved:22;
+ u32 reserved:23;
};

/* File System */
Index: linux-2.6/drivers/acpi/thermal.c
===================================================================
--- linux-2.6.orig/drivers/acpi/thermal.c
+++ linux-2.6/drivers/acpi/thermal.c
@@ -1059,8 +1059,9 @@ static int acpi_thermal_resume(struct ac
break;
tz->trips.active[i].flags.enabled = 1;
for (j = 0; j < tz->trips.active[i].devices.count; j++) {
- result = acpi_bus_get_power(tz->trips.active[i].devices.
- handles[j], &power_state);
+ result = acpi_bus_update_power(
+ tz->trips.active[i].devices.handles[j],
+ &power_state);
if (result || (power_state != ACPI_STATE_D0)) {
tz->trips.active[i].flags.enabled = 0;
break;
Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -266,7 +266,7 @@ static int __acpi_bus_set_power(struct a

/* Make sure this is a valid target state */

- if ((state == device->power.state) && !device->flags.force_power_state) {
+ if (state == device->power.state) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at D%d\n",
state));
return 0;

2010-11-24 23:18:42

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 7/13] ACPI / PM: Add function for device power state initialization

From: Rafael J. Wysocki <[email protected]>

Add function acpi_bus_init_power() for getting the initial power
state of an ACPI device and reference counting its power resources
as appropriate.

Make acpi_bus_get_power_flags() use the new function instead of
acpi_bus_get_power() that updates device->power.state without
reference counting the device's power resources.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/bus.c | 25 +++++++++++++++++++++++++
drivers/acpi/internal.h | 1 +
drivers/acpi/scan.c | 5 +----
3 files changed, 27 insertions(+), 4 deletions(-)

Index: linux-2.6/drivers/acpi/bus.c
===================================================================
--- linux-2.6.orig/drivers/acpi/bus.c
+++ linux-2.6/drivers/acpi/bus.c
@@ -364,6 +364,31 @@ int acpi_bus_set_power(acpi_handle handl

EXPORT_SYMBOL(acpi_bus_set_power);

+
+int acpi_bus_init_power(struct acpi_device *device)
+{
+ int state;
+ int result;
+
+ if (!device)
+ return -EINVAL;
+
+ device->power.state = ACPI_STATE_UNKNOWN;
+
+ result = __acpi_bus_get_power(device, &state);
+ if (result)
+ return result;
+
+ if (device->power.flags.power_resources)
+ result = acpi_power_on_resources(device, state);
+
+ if (!result)
+ device->power.state = state;
+
+ return result;
+}
+
+
bool acpi_bus_power_manageable(acpi_handle handle)
{
struct acpi_device *device;
Index: linux-2.6/drivers/acpi/scan.c
===================================================================
--- linux-2.6.orig/drivers/acpi/scan.c
+++ linux-2.6/drivers/acpi/scan.c
@@ -881,10 +881,7 @@ static int acpi_bus_get_power_flags(stru
device->power.states[ACPI_STATE_D3].flags.valid = 1;
device->power.states[ACPI_STATE_D3].power = 0;

- /* TBD: System wake support and resource requirements. */
-
- device->power.state = ACPI_STATE_UNKNOWN;
- acpi_bus_get_power(device->handle, &(device->power.state));
+ acpi_bus_init_power(device);

return 0;
}
Index: linux-2.6/drivers/acpi/internal.h
===================================================================
--- linux-2.6.orig/drivers/acpi/internal.h
+++ linux-2.6/drivers/acpi/internal.h
@@ -44,6 +44,7 @@ int acpi_device_sleep_wake(struct acpi_d
int acpi_power_get_inferred_state(struct acpi_device *device, int *state);
int acpi_power_on_resources(struct acpi_device *device, int state);
int acpi_power_transition(struct acpi_device *device, int state);
+int acpi_bus_init_power(struct acpi_device *device);
extern int acpi_power_nocheck;

int acpi_wakeup_device_init(void);

2010-11-24 23:19:12

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 2/13] ACPI / PM: Do not refcount power resources that can't be turned on

From: Rafael J. Wysocki <[email protected]>

If turning on a power resource fails, do not reference count it,
since it cannot be in use in that case.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/power.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/power.c
===================================================================
--- linux-2.6.orig/drivers/acpi/power.c
+++ linux-2.6/drivers/acpi/power.c
@@ -213,11 +213,13 @@ static int acpi_power_on(acpi_handle han
resource->name));
} else {
result = __acpi_power_on(resource);
+ if (result)
+ resource->ref_count--;
}

mutex_unlock(&resource->resource_lock);

- return 0;
+ return result;
}

static int acpi_power_off_device(acpi_handle handle)

2010-11-25 09:11:08

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

Hi Rafael,

I applied your patch-set against linux-next (next-20101124) with
backlight-type patches from Matthew.

# grep "error:" build_linux-next_next20101124.dileks.2.log
/home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
error: implicit declaration of function ‘acpi_bus_get_power’

Looking into the source:

# grep acpi_bus_get_power -r
/home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/
/home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:
result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
/home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:
result = acpi_bus_get_power(fujitsu_hotkey->acpi_handle, &state);

Checking the acpi tree:

# grep "acpi_bus_get_power(" -r
/home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/acpi/
/home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/acpi/bus.c:static
int __acpi_bus_get_power(struct acpi_device *device, int *state)
/home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/acpi/bus.c:
result = __acpi_bus_get_power(device, &state);
/home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/acpi/bus.c:
result = __acpi_bus_get_power(device, &state);

Just wanna let you know.

Kind Regards,
- Sedat -

2010-11-25 09:41:54

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Thu, Nov 25, 2010 at 10:11 AM, Sedat Dilek
<[email protected]> wrote:
> Hi Rafael,
>
> I applied your patch-set against linux-next (next-20101124) with
> backlight-type patches from Matthew.
>
> # grep "error:" build_linux-next_next20101124.dileks.2.log
> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
> error: implicit declaration of function ‘acpi_bus_get_power’
>
> Looking into the source:
>
> # grep acpi_bus_get_power -r
> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/
> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:
>   result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:
>   result = acpi_bus_get_power(fujitsu_hotkey->acpi_handle, &state);
>
> Checking the acpi tree:
>
> # grep "acpi_bus_get_power(" -r
> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/acpi/
> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/acpi/bus.c:static
> int __acpi_bus_get_power(struct acpi_device *device, int *state)
> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/acpi/bus.c:
>      result = __acpi_bus_get_power(device, &state);
> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/acpi/bus.c:
>      result = __acpi_bus_get_power(device, &state);
>
> Just wanna let you know.
>
> Kind Regards,
> - Sedat -
>

Guess this is conflicting with your patch
"12-13-ACPI-PM-Drop-acpi_bus_get_power.patch".

- Sedat -

P.S.: Note to myself, add the following snippet for testing purposes:

[ debian/config/i386/none/config.686 ]
...
##
## file: drivers/platform/x86/Kconfig
##
# CONFIG_FUJITSU_LAPTOP is not set
- EOT -

2010-11-25 16:14:00

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Thu, Nov 25, 2010 at 10:41 AM, Sedat Dilek
<[email protected]> wrote:
> On Thu, Nov 25, 2010 at 10:11 AM, Sedat Dilek
> <[email protected]> wrote:
>> Hi Rafael,
>>
[ ... ]
>> # grep "error:" build_linux-next_next20101124.dileks.2.log
>> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
>> error: implicit declaration of function ‘acpi_bus_get_power’
[ ... ]
>
> Guess this is conflicting with your patch
> "12-13-ACPI-PM-Drop-acpi_bus_get_power.patch".
>
> - Sedat -
>
> P.S.: Note to myself, add the following snippet for testing purposes:
>
> [ debian/config/i386/none/config.686 ]
> ...
> ##
> ## file: drivers/platform/x86/Kconfig
> ##
> # CONFIG_FUJITSU_LAPTOP is not set
> - EOT -
>

Applied on top of linux-next (next-20101125) and running.

Tested-by: Sedat Dilek <[email protected]>

- Sedat -

2010-11-25 20:15:51

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Thursday, November 25, 2010, Sedat Dilek wrote:
> On Thu, Nov 25, 2010 at 10:41 AM, Sedat Dilek
> <[email protected]> wrote:
> > On Thu, Nov 25, 2010 at 10:11 AM, Sedat Dilek
> > <[email protected]> wrote:
> >> Hi Rafael,
> >>
> [ ... ]
> >> # grep "error:" build_linux-next_next20101124.dileks.2.log
> >> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
> >> error: implicit declaration of function ‘acpi_bus_get_power’
> [ ... ]
> >
> > Guess this is conflicting with your patch
> > "12-13-ACPI-PM-Drop-acpi_bus_get_power.patch".
> >
> > - Sedat -
> >
> > P.S.: Note to myself, add the following snippet for testing purposes:
> >
> > [ debian/config/i386/none/config.686 ]
> > ...
> > ##
> > ## file: drivers/platform/x86/Kconfig
> > ##
> > # CONFIG_FUJITSU_LAPTOP is not set
> > - EOT -
> >
>
> Applied on top of linux-next (next-20101125) and running.
>
> Tested-by: Sedat Dilek <[email protected]>

Great, thanks a lot!

I should have said the patchset is on top of the current Linus' tree.

Rafael

2010-11-25 20:29:51

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Thursday, November 25, 2010, Rafael J. Wysocki wrote:
> On Thursday, November 25, 2010, Sedat Dilek wrote:
> > On Thu, Nov 25, 2010 at 10:41 AM, Sedat Dilek
> > <[email protected]> wrote:
> > > On Thu, Nov 25, 2010 at 10:11 AM, Sedat Dilek
> > > <[email protected]> wrote:
> > >> Hi Rafael,
> > >>
> > [ ... ]
> > >> # grep "error:" build_linux-next_next20101124.dileks.2.log
> > >> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
> > >> error: implicit declaration of function ‘acpi_bus_get_power’
> > [ ... ]
> > >
> > > Guess this is conflicting with your patch
> > > "12-13-ACPI-PM-Drop-acpi_bus_get_power.patch".
> > >
> > > - Sedat -
> > >
> > > P.S.: Note to myself, add the following snippet for testing purposes:
> > >
> > > [ debian/config/i386/none/config.686 ]
> > > ...
> > > ##
> > > ## file: drivers/platform/x86/Kconfig
> > > ##
> > > # CONFIG_FUJITSU_LAPTOP is not set
> > > - EOT -
> > >
> >
> > Applied on top of linux-next (next-20101125) and running.
> >
> > Tested-by: Sedat Dilek <[email protected]>
>
> Great, thanks a lot!
>
> I should have said the patchset is on top of the current Linus' tree.

Actually it also breaks with the current Linus' tree, but the appended patch
should fix it.

Thanks,
Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()

Use the new function acpi_bus_update_power(), which is safer than
acpi_bus_get_power(), for getting device power state in
acpi_fujitsu_add().

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
===================================================================
--- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
+++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
@@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
if (error)
goto err_free_input_dev;

- result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
+ result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
if (result) {
printk(KERN_ERR "Error reading power state\n");
goto err_unregister_input_dev;

2010-11-25 20:37:48

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Thu, Nov 25, 2010 at 9:29 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Thursday, November 25, 2010, Rafael J. Wysocki wrote:
>> On Thursday, November 25, 2010, Sedat Dilek wrote:
>> > On Thu, Nov 25, 2010 at 10:41 AM, Sedat Dilek
>> > <[email protected]> wrote:
>> > > On Thu, Nov 25, 2010 at 10:11 AM, Sedat Dilek
>> > > <[email protected]> wrote:
>> > >> Hi Rafael,
>> > >>
>> > [ ... ]
>> > >> # grep "error:" build_linux-next_next20101124.dileks.2.log
>> > >> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
>> > >> error: implicit declaration of function ‘acpi_bus_get_power’
>> > [ ... ]
>> > >
>> > > Guess this is conflicting with your patch
>> > > "12-13-ACPI-PM-Drop-acpi_bus_get_power.patch".
>> > >
>> > > - Sedat -
>> > >
>> > > P.S.: Note to myself, add the following snippet for testing purposes:
>> > >
>> > > [ debian/config/i386/none/config.686 ]
>> > > ...
>> > > ##
>> > > ## file: drivers/platform/x86/Kconfig
>> > > ##
>> > > # CONFIG_FUJITSU_LAPTOP is not set
>> > > - EOT -
>> > >
>> >
>> > Applied on top of linux-next (next-20101125) and running.
>> >
>> >      Tested-by: Sedat Dilek <[email protected]>
>>
>> Great, thanks a lot!
>>
>> I should have said the patchset is on top of the current Linus' tree.
>
> Actually it also breaks with the current Linus' tree, but the appended patch
> should fix it.
>
> Thanks,
> Rafael
>
> ---
> From: Rafael J. Wysocki <[email protected]>
> Subject: Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()
>
> Use the new function acpi_bus_update_power(), which is safer than
> acpi_bus_get_power(), for getting device power state in
> acpi_fujitsu_add().
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>  drivers/platform/x86/fujitsu-laptop.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> ===================================================================
> --- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
> +++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> @@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
>        if (error)
>                goto err_free_input_dev;
>
> -       result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
> +       result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
>        if (result) {
>                printk(KERN_ERR "Error reading power state\n");
>                goto err_unregister_input_dev;
>

Sorry, I crapped my own quotes.
Missed to catch line #860?

- Sedat -

$ cd ~/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none

$ grep --line-number -r acpi_bus_get_power -r drivers/platform/x86/
drivers/platform/x86/fujitsu-laptop.c:692: result =
acpi_bus_get_power(fujitsu->acpi_handle, &state);
drivers/platform/x86/fujitsu-laptop.c:860: result =
acpi_bus_get_power(fujitsu_hotkey->acpi_handle, &state);

2010-11-25 20:46:09

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Thursday, November 25, 2010, Sedat Dilek wrote:
> On Thu, Nov 25, 2010 at 9:29 PM, Rafael J. Wysocki <[email protected]> wrote:
> > On Thursday, November 25, 2010, Rafael J. Wysocki wrote:
> >> On Thursday, November 25, 2010, Sedat Dilek wrote:
> >> > On Thu, Nov 25, 2010 at 10:41 AM, Sedat Dilek
> >> > <[email protected]> wrote:
> >> > > On Thu, Nov 25, 2010 at 10:11 AM, Sedat Dilek
> >> > > <[email protected]> wrote:
> >> > >> Hi Rafael,
> >> > >>
> >> > [ ... ]
> >> > >> # grep "error:" build_linux-next_next20101124.dileks.2.log
> >> > >> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
> >> > >> error: implicit declaration of function ‘acpi_bus_get_power’
> >> > [ ... ]
> >> > >
> >> > > Guess this is conflicting with your patch
> >> > > "12-13-ACPI-PM-Drop-acpi_bus_get_power.patch".
> >> > >
> >> > > - Sedat -
> >> > >
> >> > > P.S.: Note to myself, add the following snippet for testing purposes:
> >> > >
> >> > > [ debian/config/i386/none/config.686 ]
> >> > > ...
> >> > > ##
> >> > > ## file: drivers/platform/x86/Kconfig
> >> > > ##
> >> > > # CONFIG_FUJITSU_LAPTOP is not set
> >> > > - EOT -
> >> > >
> >> >
> >> > Applied on top of linux-next (next-20101125) and running.
> >> >
> >> > Tested-by: Sedat Dilek <[email protected]>
> >>
> >> Great, thanks a lot!
> >>
> >> I should have said the patchset is on top of the current Linus' tree.
> >
> > Actually it also breaks with the current Linus' tree, but the appended patch
> > should fix it.
> >
> > Thanks,
> > Rafael
> >
> > ---
> > From: Rafael J. Wysocki <[email protected]>
> > Subject: Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()
> >
> > Use the new function acpi_bus_update_power(), which is safer than
> > acpi_bus_get_power(), for getting device power state in
> > acpi_fujitsu_add().
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/platform/x86/fujitsu-laptop.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
> > +++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> > @@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
> > if (error)
> > goto err_free_input_dev;
> >
> > - result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
> > + result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
> > if (result) {
> > printk(KERN_ERR "Error reading power state\n");
> > goto err_unregister_input_dev;
> >
>
> Sorry, I crapped my own quotes.
> Missed to catch line #860?

Indeed, sorry. Updated patch is appended.

Rafael


---
From: Rafael J. Wysocki <[email protected]>
Subject: Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()

Use the new function acpi_bus_update_power(), which is safer than
acpi_bus_get_power(), for getting device power state in
acpi_fujitsu_add().

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
===================================================================
--- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
+++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
@@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
if (error)
goto err_free_input_dev;

- result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
+ result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
if (result) {
printk(KERN_ERR "Error reading power state\n");
goto err_unregister_input_dev;
@@ -857,7 +857,7 @@ static int acpi_fujitsu_hotkey_add(struc
if (error)
goto err_free_input_dev;

- result = acpi_bus_get_power(fujitsu_hotkey->acpi_handle, &state);
+ result = acpi_bus_update_power(fujitsu_hotkey->acpi_handle, &state);
if (result) {
printk(KERN_ERR "Error reading power state\n");
goto err_unregister_input_dev;

2010-11-26 08:12:16

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Thu, Nov 25, 2010 at 9:45 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Thursday, November 25, 2010, Sedat Dilek wrote:
>> On Thu, Nov 25, 2010 at 9:29 PM, Rafael J. Wysocki <[email protected]> wrote:
>> > On Thursday, November 25, 2010, Rafael J. Wysocki wrote:
>> >> On Thursday, November 25, 2010, Sedat Dilek wrote:
>> >> > On Thu, Nov 25, 2010 at 10:41 AM, Sedat Dilek
>> >> > <[email protected]> wrote:
>> >> > > On Thu, Nov 25, 2010 at 10:11 AM, Sedat Dilek
>> >> > > <[email protected]> wrote:
>> >> > >> Hi Rafael,
>> >> > >>
>> >> > [ ... ]
>> >> > >> # grep "error:" build_linux-next_next20101124.dileks.2.log
>> >> > >> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
>> >> > >> error: implicit declaration of function ‘acpi_bus_get_power’
>> >> > [ ... ]
>> >> > >
>> >> > > Guess this is conflicting with your patch
>> >> > > "12-13-ACPI-PM-Drop-acpi_bus_get_power.patch".
>> >> > >
>> >> > > - Sedat -
>> >> > >
>> >> > > P.S.: Note to myself, add the following snippet for testing purposes:
>> >> > >
>> >> > > [ debian/config/i386/none/config.686 ]
>> >> > > ...
>> >> > > ##
>> >> > > ## file: drivers/platform/x86/Kconfig
>> >> > > ##
>> >> > > # CONFIG_FUJITSU_LAPTOP is not set
>> >> > > - EOT -
>> >> > >
>> >> >
>> >> > Applied on top of linux-next (next-20101125) and running.
>> >> >
>> >> >      Tested-by: Sedat Dilek <[email protected]>
>> >>
>> >> Great, thanks a lot!
>> >>
>> >> I should have said the patchset is on top of the current Linus' tree.
>> >
>> > Actually it also breaks with the current Linus' tree, but the appended patch
>> > should fix it.
>> >
>> > Thanks,
>> > Rafael
>> >
>> > ---
>> > From: Rafael J. Wysocki <[email protected]>
>> > Subject: Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()
>> >
>> > Use the new function acpi_bus_update_power(), which is safer than
>> > acpi_bus_get_power(), for getting device power state in
>> > acpi_fujitsu_add().
>> >
>> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>> > ---
>> >  drivers/platform/x86/fujitsu-laptop.c |    2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
>> > ===================================================================
>> > --- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
>> > +++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
>> > @@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
>> >        if (error)
>> >                goto err_free_input_dev;
>> >
>> > -       result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
>> > +       result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
>> >        if (result) {
>> >                printk(KERN_ERR "Error reading power state\n");
>> >                goto err_unregister_input_dev;
>> >
>>
>> Sorry, I crapped my own quotes.
>> Missed to catch line #860?
>
> Indeed, sorry.  Updated patch is appended.
>
> Rafael
>
>
> ---
> From: Rafael J. Wysocki <[email protected]>
> Subject: Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()
>
> Use the new function acpi_bus_update_power(), which is safer than
> acpi_bus_get_power(), for getting device power state in
> acpi_fujitsu_add().
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>  drivers/platform/x86/fujitsu-laptop.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> ===================================================================
> --- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
> +++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> @@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
>        if (error)
>                goto err_free_input_dev;
>
> -       result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
> +       result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
>        if (result) {
>                printk(KERN_ERR "Error reading power state\n");
>                goto err_unregister_input_dev;
> @@ -857,7 +857,7 @@ static int acpi_fujitsu_hotkey_add(struc
>        if (error)
>                goto err_free_input_dev;
>
> -       result = acpi_bus_get_power(fujitsu_hotkey->acpi_handle, &state);
> +       result = acpi_bus_update_power(fujitsu_hotkey->acpi_handle, &state);
>        if (result) {
>                printk(KERN_ERR "Error reading power state\n");
>                goto err_unregister_input_dev;
>

Feel free to add for
"Platform-x86-Make-fujitsu_laptop-use-acpi_bus_update_power.patch":

Reported-and-Tested-by: Sedat Dilek <[email protected]>

- Sedat -

# grep OK setup_linux-next_next20101126.dileks.1.log | grep ACPI-PM
(+) OK ACPI-PM/1-13-ACPI-PM-Check-device-state-before-refcounting-power-resources.patch
(+) OK ACPI-PM/2-13-ACPI-PM-Do-not-refcount-power-resources-that-can-t-be-turned-on.patch
(+) OK ACPI-PM/3-13-ACPI-PM-Prevent-acpi_power_get_inferred_state-from-making-changes.patch
(+) OK ACPI-PM/4-13-ACPI-PM-Add-functions-for-manipulating-lists-of-power-resources.patch
(+) OK ACPI-PM/5-13-ACPI-PM-Introduce-function-for-refcounting-device-power-resources.patch
(+) OK ACPI-PM/6-13-ACPI-PM-Introduce-__acpi_bus_get_power.patch
(+) OK ACPI-PM/7-13-ACPI-PM-Add-function-for-device-power-state-initialization.patch
(+) OK ACPI-PM/8-13-ACPI-PM-Add-function-for-updating-device-power-state-consistently.patch
(+) OK ACPI-PM/9-13-ACPI-PM-Register-acpi_power_driver-early.patch
(+) OK ACPI-PM/10-13-ACPI-PM-Register-power-resource-devices-as-soon-as-they-are-needed.patch
(+) OK ACPI-PM/11-13-ACPI-Fan-Rework-the-handling-of-power-resources.patch
(+) OK ACPI-PM/12-13-ACPI-PM-Drop-acpi_bus_get_power.patch
(+) OK ACPI-PM/13-13-ACPI-PM-Drop-acpi_power_nocheck.patch
(+) OK ACPI-PM-fix/Platform-x86-Make-fujitsu_laptop-use-acpi_bus_update_power.patch

# modinfo fujitsu-laptop
filename:
/lib/modules/2.6.37-rc3-686/kernel/drivers/platform/x86/fujitsu-laptop.ko
alias: dmi:*:svnFUJITSU:*:pvr:rvnFUJITSU:rnFJNB19C:*:cvrS7020:*
alias: dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1E6:*:cvrS6420:*
alias: dmi:*:svnFUJITSUSIEMENS:*:pvr:rvnFUJITSU:rnFJNB1D3:*:cvrS6410:*
license: GPL
version: 0.6.0
description: Fujitsu laptop extras support
author: Jonathan Woithe, Peter Gruber, Tony Vroon
srcversion: FFA7AB6B6C940453C0946B4
alias: acpi*:FUJ02E3:*
alias: pnp:dFUJ02E3*
alias: acpi*:FUJ02B1:*
alias: pnp:dFUJ02B1*
alias: acpi*:FUJ02BF:*
alias: pnp:dFUJ02bf*
depends:
vermagic: 2.6.37-rc3-686 SMP mod_unload modversions 686
parm: use_alt_lcd_levels:Use alternative interface for
lcd_levels (needed for Lifebook s6410). (uint)
parm: disable_brightness_adjust:Disable brightness adjustment . (uint)

2010-11-26 19:27:56

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Friday, November 26, 2010, Sedat Dilek wrote:
> On Thu, Nov 25, 2010 at 9:45 PM, Rafael J. Wysocki <[email protected]> wrote:
> > On Thursday, November 25, 2010, Sedat Dilek wrote:
> >> On Thu, Nov 25, 2010 at 9:29 PM, Rafael J. Wysocki <[email protected]> wrote:
> >> > On Thursday, November 25, 2010, Rafael J. Wysocki wrote:
> >> >> On Thursday, November 25, 2010, Sedat Dilek wrote:
> >> >> > On Thu, Nov 25, 2010 at 10:41 AM, Sedat Dilek
> >> >> > <[email protected]> wrote:
> >> >> > > On Thu, Nov 25, 2010 at 10:11 AM, Sedat Dilek
> >> >> > > <[email protected]> wrote:
> >> >> > >> Hi Rafael,
> >> >> > >>
> >> >> > [ ... ]
> >> >> > >> # grep "error:" build_linux-next_next20101124.dileks.2.log
> >> >> > >> /home/sd/src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/drivers/platform/x86/fujitsu-laptop.c:692:
> >> >> > >> error: implicit declaration of function ‘acpi_bus_get_power’
> >> >> > [ ... ]
> >> >> > >
> >> >> > > Guess this is conflicting with your patch
> >> >> > > "12-13-ACPI-PM-Drop-acpi_bus_get_power.patch".
> >> >> > >
> >> >> > > - Sedat -
> >> >> > >
> >> >> > > P.S.: Note to myself, add the following snippet for testing purposes:
> >> >> > >
> >> >> > > [ debian/config/i386/none/config.686 ]
> >> >> > > ...
> >> >> > > ##
> >> >> > > ## file: drivers/platform/x86/Kconfig
> >> >> > > ##
> >> >> > > # CONFIG_FUJITSU_LAPTOP is not set
> >> >> > > - EOT -
> >> >> > >
> >> >> >
> >> >> > Applied on top of linux-next (next-20101125) and running.
> >> >> >
> >> >> > Tested-by: Sedat Dilek <[email protected]>
> >> >>
> >> >> Great, thanks a lot!
> >> >>
> >> >> I should have said the patchset is on top of the current Linus' tree.
> >> >
> >> > Actually it also breaks with the current Linus' tree, but the appended patch
> >> > should fix it.
> >> >
> >> > Thanks,
> >> > Rafael
> >> >
> >> > ---
> >> > From: Rafael J. Wysocki <[email protected]>
> >> > Subject: Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()
> >> >
> >> > Use the new function acpi_bus_update_power(), which is safer than
> >> > acpi_bus_get_power(), for getting device power state in
> >> > acpi_fujitsu_add().
> >> >
> >> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> >> > ---
> >> > drivers/platform/x86/fujitsu-laptop.c | 2 +-
> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> >> > ===================================================================
> >> > --- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
> >> > +++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> >> > @@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
> >> > if (error)
> >> > goto err_free_input_dev;
> >> >
> >> > - result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
> >> > + result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
> >> > if (result) {
> >> > printk(KERN_ERR "Error reading power state\n");
> >> > goto err_unregister_input_dev;
> >> >
> >>
> >> Sorry, I crapped my own quotes.
> >> Missed to catch line #860?
> >
> > Indeed, sorry. Updated patch is appended.
> >
> > Rafael
> >
> >
> > ---
> > From: Rafael J. Wysocki <[email protected]>
> > Subject: Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()
> >
> > Use the new function acpi_bus_update_power(), which is safer than
> > acpi_bus_get_power(), for getting device power state in
> > acpi_fujitsu_add().
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/platform/x86/fujitsu-laptop.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
> > +++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
> > @@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
> > if (error)
> > goto err_free_input_dev;
> >
> > - result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
> > + result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
> > if (result) {
> > printk(KERN_ERR "Error reading power state\n");
> > goto err_unregister_input_dev;
> > @@ -857,7 +857,7 @@ static int acpi_fujitsu_hotkey_add(struc
> > if (error)
> > goto err_free_input_dev;
> >
> > - result = acpi_bus_get_power(fujitsu_hotkey->acpi_handle, &state);
> > + result = acpi_bus_update_power(fujitsu_hotkey->acpi_handle, &state);
> > if (result) {
> > printk(KERN_ERR "Error reading power state\n");
> > goto err_unregister_input_dev;
> >
>
> Feel free to add for
> "Platform-x86-Make-fujitsu_laptop-use-acpi_bus_update_power.patch":
>
> Reported-and-Tested-by: Sedat Dilek <[email protected]>

Thanks for testing!

Rafael

2010-11-26 21:56:12

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH] Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()

From: Rafael J. Wysocki <[email protected]>

Use the new function acpi_bus_update_power(), which is safer than
acpi_bus_get_power(), for getting device power state in
acpi_fujitsu_add().

Signed-off-by: Rafael J. Wysocki <[email protected]>
Reported-and-Tested-by: Sedat Dilek <[email protected]>
---

Hi Len,

This patch should go after [11/13] in the "ACPI / PM: Rework power resources
management" series. I hope Matthew won't mind if it goes in through your tree.

Thanks,
Rafael

---
drivers/platform/x86/fujitsu-laptop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/platform/x86/fujitsu-laptop.c
===================================================================
--- linux-2.6.orig/drivers/platform/x86/fujitsu-laptop.c
+++ linux-2.6/drivers/platform/x86/fujitsu-laptop.c
@@ -689,7 +689,7 @@ static int acpi_fujitsu_add(struct acpi_
if (error)
goto err_free_input_dev;

- result = acpi_bus_get_power(fujitsu->acpi_handle, &state);
+ result = acpi_bus_update_power(fujitsu->acpi_handle, &state);
if (result) {
printk(KERN_ERR "Error reading power state\n");
goto err_unregister_input_dev;
@@ -857,7 +857,7 @@ static int acpi_fujitsu_hotkey_add(struc
if (error)
goto err_free_input_dev;

- result = acpi_bus_get_power(fujitsu_hotkey->acpi_handle, &state);
+ result = acpi_bus_update_power(fujitsu_hotkey->acpi_handle, &state);
if (result) {
printk(KERN_ERR "Error reading power state\n");
goto err_unregister_input_dev;

2010-12-01 22:03:03

by Len Brown

[permalink] [raw]
Subject: Re: [PATCH 12/13] ACPI / PM: Drop acpi_bus_get_power()

CC [M] drivers/platform/x86/fujitsu-laptop.o
drivers/platform/x86/fujitsu-laptop.c: In function ?acpi_fujitsu_add?:
drivers/platform/x86/fujitsu-laptop.c:692: error: implicit declaration of
function ?acpi_bus_get_power?

2010-12-01 22:07:52

by Len Brown

[permalink] [raw]
Subject: Re: [PATCH] Platform / x86: Make fujitsu_laptop use acpi_bus_update_power()

okay, applie between patch 11/13 and patch 12/13.

thanks,
Len Brown, Intel Open Source Technology Center

2010-12-01 22:08:17

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 12/13] ACPI / PM: Drop acpi_bus_get_power()

On Wednesday, December 01, 2010, Len Brown wrote:
> CC [M] drivers/platform/x86/fujitsu-laptop.o
> drivers/platform/x86/fujitsu-laptop.c: In function ‘acpi_fujitsu_add’:
> drivers/platform/x86/fujitsu-laptop.c:692: error: implicit declaration of
> function ‘acpi_bus_get_power’

Please see:

https://patchwork.kernel.org/patch/360022/

Thanks,
Rafael

2010-12-13 15:43:21

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

Hi Rafael,

nice to see your patchset is in linux-next (next-20101213) now.
More precisely in linux-acpi-2.6: 1/13 + 2/13 in release and rest/13
in test GIT branch.

What about the fix
"Platform-x86-Make-fujitsu_laptop-use-acpi_bus_update_power.patch"
[1]?
Will this go over platform-x86/linux-next into linux-next GIT tree?
Are you co-ordinating this with mjg?

Regards,
- Sedat -

[1] https://patchwork.kernel.org/patch/357422/
[2] http://git.kernel.org/?p=linux/kernel/git/mjg59/platform-drivers-x86.git;a=shortlog;h=refs/heads/linux-next

2010-12-14 09:20:51

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH 0/13] ACPI / PM: Rework power resources management

On Mon, Dec 13, 2010 at 4:43 PM, Sedat Dilek <[email protected]> wrote:
> Hi Rafael,
>
> nice to see your patchset is in linux-next (next-20101213) now.
> More precisely in linux-acpi-2.6: 1/13 + 2/13 in release and rest/13
> in test GIT branch.
>
> What about the fix
> "Platform-x86-Make-fujitsu_laptop-use-acpi_bus_update_power.patch"
> [1]?
> Will this go over platform-x86/linux-next into linux-next GIT tree?
> Are you co-ordinating this with mjg?
>
> Regards,
> - Sedat -
>
> [1] https://patchwork.kernel.org/patch/357422/
> [2] http://git.kernel.org/?p=linux/kernel/git/mjg59/platform-drivers-x86.git;a=shortlog;h=refs/heads/linux-next
>

Thanks, the patch is now included in linux-next (next-20101214) (see [1]).

- Sedat -

[1] http://git.kernel.org/?p=linux/kernel/git/lenb/linux-acpi-2.6.git;a=commit;h=5fd12c41e206d6bfda6eef1ed50e995a293e0777