On Sunday 23 November 2008 02:09:03 pm Witold Szczeponik wrote:Subject: Enable PNPACI Power Management
Hi Witold,
Thanks for your patch. I'm glad somebody is paying attention to
PNP and power. I CC'd Adam and Rafael because they care about this
area, too, but might not read everything on linux-acpi.
Do you know of anything that specifies the order of the _CRS/_PS0
and the _PS3/_DIS evaluation? I don't know much about power
management, and I couldn't find anything obvious in the spec.
It seems plausible that we should run _CRS before turning on
the power, but I really don't know.
Is pnpacpi_set_resources() the only place that needs this change?
For active devices, we normally don't call pnpacpi_set_resources()
at all. So I suppose on these ThinkPads, we exercise this path
because the serial ports are initially disabled?
No regressions were observed on hardware that does not require
this patch.
The patch is applied against 2.6.27.7 (vanilla).
Signed-off-by: Witold Szczeponik <Witold.Szczeponik@xxxxxxx>
Index: linux/drivers/pnp/pnpacpi/core.c
===================================================================
--- linux.orig/drivers/pnp/pnpacpi/core.c
+++ linux/drivers/pnp/pnpacpi/core.c
@@ -98,18 +98,24 @@ static int pnpacpi_set_resources(struct
status = acpi_set_current_resources(handle, &buffer);
if (ACPI_FAILURE(status))
ret = -EINVAL;
+ else if (acpi_bus_power_manageable(handle))
+ ret = acpi_bus_set_power(handle, ACPI_STATE_D0);
I don't really like testing acpi_bus_power_manageable() first.
I think we should just call acpi_bus_set_power() and let *it*
bail out if the device doesn't support it.
kfree(buffer.pointer);
return ret;
}
static int pnpacpi_disable_resources(struct pnp_dev *dev)
{
+ acpi_handle handle = dev->data;
+ int ret = 0;
acpi_status status;
- /* acpi_unregister_gsi(pnp_irq(dev, 0)); */
Can you leave the "unregister_gsi" comment there, since it's not
related to your patch? It's a reminder that we need to think about
how to handle interrupts when enabling/disabling devices.
- status = acpi_evaluate_object((acpi_handle) dev->data,
- "_DIS", NULL, NULL);
- return ACPI_FAILURE(status) ? -ENODEV : 0;
+ if (acpi_bus_power_manageable(handle))
+ ret = acpi_bus_set_power(handle, ACPI_STATE_D3);
+ status = acpi_evaluate_object(handle, "_DIS", NULL, NULL);
+ if (ACPI_FAILURE(status))
+ ret = -ENODEV;
+ return ret;
}
#ifdef CONFIG_ACPI_SLEEP