From 2c7d132a589077b31493b3ea82ac83b1f72c93e1 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 30 Jul 2013 14:34:00 +0200 Subject: [PATCH 1/5] ACPI / PM: Only set power states of devices that are power manageable Make acpi_device_set_power() check if the given device is power manageable before checking if the given power state is valid for that device. Otherwise it will print that "Device does not support" that power state into the kernel log, which may not make sense for some power states (D0 and D3cold are supported by all devices by definition). Tested-by: Yinghai Lu Signed-off-by: Rafael J. Wysocki --- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index 4ab807dc8518..63324b873636 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_device *device, int state) int result = 0; bool cut_power = false; - if (!device || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3_COLD)) + if (!device || !device->flags.power_manageable + || (state < ACPI_STATE_D0) || (state > ACPI_STATE_D3_COLD)) return -EINVAL; /* Make sure this is a valid target state */ From b69137a74b7a9451b3da504d1ef9ead7cb393922 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 30 Jul 2013 14:34:55 +0200 Subject: [PATCH 2/5] ACPI / PM: Make messages in acpi_device_set_power() print device names Modify acpi_device_set_power() so that diagnostic messages printed by it to the kernel log always contain the name of the device concerned to make it possible to identify the device that triggered the message if need be. Also replace printk(KERN_WARNING ) with dev_warn() everywhere in that function. Signed-off-by: Rafael J. Wysocki Reviewed-by: Aaron Lu --- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index 63324b873636..8234e1f8c79d 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_device *device, int state) /* Make sure this is a valid target state */ if (state == device->power.state) { - ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at %s\n", + ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] already in %s\n", + device->pnp.bus_id, acpi_power_state_string(state))); return 0; } if (!device->power.states[state].flags.valid) { - printk(KERN_WARNING PREFIX "Device does not support %s\n", - acpi_power_state_string(state)); + dev_warn(&device->dev, "Power state %s not supported\n", + acpi_power_state_string(state)); return -ENODEV; } if (device->parent && (state < device->parent->power.state)) { - printk(KERN_WARNING PREFIX - "Cannot set device to a higher-powered" - " state than parent\n"); + dev_warn(&device->dev, + "Cannot transition to a higher-powered state than parent\n"); return -ENODEV; } @@ -192,8 +192,8 @@ int acpi_device_set_power(struct acpi_device *device, int state) 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"); + dev_warn(&device->dev, + "Cannot transition to non-D0 state from D3\n"); return -ENODEV; } @@ -220,10 +220,8 @@ int acpi_device_set_power(struct acpi_device *device, int state) end: if (result) { - printk(KERN_WARNING PREFIX - "Device [%s] failed to transition to %s\n", - device->pnp.bus_id, - acpi_power_state_string(state)); + dev_warn(&device->dev, "Failed to change power state to %s\n", + acpi_power_state_string(state)); } else { device->power.state = state; ACPI_DEBUG_PRINT((ACPI_DB_INFO, From 8ad928d52e63a9b7d69f0873d7318c4561e2f8cd Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Tue, 30 Jul 2013 14:36:20 +0200 Subject: [PATCH 3/5] ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere There are several places in the tree where ACPI_STATE_D3 is used instead of ACPI_STATE_D3_COLD which should be used instead for clarity. Modify them all to use ACPI_STATE_D3_COLD as appropriate. [The definition of ACPI_STATE_D3 itself cannot go away at this point as it is part of ACPICA.] Signed-off-by: Rafael J. Wysocki Reviewed-by: Aaron Lu --- drivers/acpi/fan.c | 4 ++-- drivers/acpi/power.c | 2 +- drivers/acpi/scan.c | 4 ++-- drivers/ata/libata-acpi.c | 4 ++-- drivers/ide/ide-acpi.c | 5 +++-- drivers/pnp/pnpacpi/core.c | 6 +++--- include/acpi/acpi_bus.h | 3 ++- 7 files changed, 15 insertions(+), 13 deletions(-) diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c index 5b02a0aa540c..41ade6570bc0 100644 --- a/drivers/acpi/fan.c +++ b/drivers/acpi/fan.c @@ -93,7 +93,7 @@ static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned long if (result) return result; - *state = (acpi_state == ACPI_STATE_D3 ? 0 : + *state = (acpi_state == ACPI_STATE_D3_COLD ? 0 : (acpi_state == ACPI_STATE_D0 ? 1 : -1)); return 0; } @@ -108,7 +108,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state) return -EINVAL; result = acpi_bus_set_power(device->handle, - state ? ACPI_STATE_D0 : ACPI_STATE_D3); + state ? ACPI_STATE_D0 : ACPI_STATE_D3_COLD); return result; } diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c index 5c28c894c0fc..4cdcc0cf0c56 100644 --- a/drivers/acpi/power.c +++ b/drivers/acpi/power.c @@ -786,7 +786,7 @@ int acpi_power_get_inferred_state(struct acpi_device *device, int *state) } } - *state = ACPI_STATE_D3; + *state = ACPI_STATE_D3_COLD; return 0; } diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 8a46c924effd..c30df3ad2d71 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1450,8 +1450,8 @@ static void acpi_bus_get_power_flags(struct acpi_device *device) /* Set defaults for D0 and D3 states (always valid) */ device->power.states[ACPI_STATE_D0].flags.valid = 1; device->power.states[ACPI_STATE_D0].power = 100; - device->power.states[ACPI_STATE_D3].flags.valid = 1; - device->power.states[ACPI_STATE_D3].power = 0; + device->power.states[ACPI_STATE_D3_COLD].flags.valid = 1; + device->power.states[ACPI_STATE_D3_COLD].power = 0; /* Set D3cold's explicit_set flag if _PS3 exists. */ if (device->power.states[ACPI_STATE_D3_HOT].flags.explicit_set) diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c index cf4e7020adac..da8170dfc90f 100644 --- a/drivers/ata/libata-acpi.c +++ b/drivers/ata/libata-acpi.c @@ -947,11 +947,11 @@ static void pata_acpi_set_state(struct ata_port *ap, pm_message_t state) continue; acpi_bus_set_power(dev_handle, state.event & PM_EVENT_RESUME ? - ACPI_STATE_D0 : ACPI_STATE_D3); + ACPI_STATE_D0 : ACPI_STATE_D3_COLD); } if (!(state.event & PM_EVENT_RESUME)) - acpi_bus_set_power(port_handle, ACPI_STATE_D3); + acpi_bus_set_power(port_handle, ACPI_STATE_D3_COLD); } /** diff --git a/drivers/ide/ide-acpi.c b/drivers/ide/ide-acpi.c index f1a6796b165c..140c8ef50529 100644 --- a/drivers/ide/ide-acpi.c +++ b/drivers/ide/ide-acpi.c @@ -520,11 +520,12 @@ void ide_acpi_set_state(ide_hwif_t *hwif, int on) ide_port_for_each_present_dev(i, drive, hwif) { if (drive->acpidata->obj_handle) acpi_bus_set_power(drive->acpidata->obj_handle, - on ? ACPI_STATE_D0 : ACPI_STATE_D3); + on ? ACPI_STATE_D0 : ACPI_STATE_D3_COLD); } if (!on) - acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D3); + acpi_bus_set_power(hwif->acpidata->obj_handle, + ACPI_STATE_D3_COLD); } /** diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c index 55cd459a3908..34049b0b4c73 100644 --- a/drivers/pnp/pnpacpi/core.c +++ b/drivers/pnp/pnpacpi/core.c @@ -131,7 +131,7 @@ static int pnpacpi_disable_resources(struct pnp_dev *dev) /* acpi_unregister_gsi(pnp_irq(dev, 0)); */ ret = 0; if (acpi_bus_power_manageable(handle)) - acpi_bus_set_power(handle, ACPI_STATE_D3); + acpi_bus_set_power(handle, ACPI_STATE_D3_COLD); /* continue even if acpi_bus_set_power() fails */ if (ACPI_FAILURE(acpi_evaluate_object(handle, "_DIS", NULL, NULL))) ret = -ENODEV; @@ -174,10 +174,10 @@ static int pnpacpi_suspend(struct pnp_dev *dev, pm_message_t state) if (acpi_bus_power_manageable(handle)) { int power_state = acpi_pm_device_sleep_state(&dev->dev, NULL, - ACPI_STATE_D3); + ACPI_STATE_D3_COLD); if (power_state < 0) power_state = (state.event == PM_EVENT_ON) ? - ACPI_STATE_D0 : ACPI_STATE_D3; + ACPI_STATE_D0 : ACPI_STATE_D3_COLD; /* * acpi_bus_set_power() often fails (keyboard port can't be diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 56e6b68c8d2f..916b18933879 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -478,7 +478,8 @@ static inline int acpi_pm_device_sleep_state(struct device *d, int *p, int m) if (p) *p = ACPI_STATE_D0; - return (m >= ACPI_STATE_D0 && m <= ACPI_STATE_D3) ? m : ACPI_STATE_D0; + return (m >= ACPI_STATE_D0 && m <= ACPI_STATE_D3_COLD) ? + m : ACPI_STATE_D0; } static inline void acpi_dev_pm_add_dependent(acpi_handle handle, struct device *depdev) {} From 7b4e0c4ac1809eab6fcfe6818ec8b70be79b41bc Mon Sep 17 00:00:00 2001 From: Aaron Lu Date: Wed, 31 Jul 2013 14:07:15 +0200 Subject: [PATCH 4/5] ACPI / PM: Remove redundant power manageable check from acpi_bus_set_power() Now that acpi_device_set_power() checks whether or not the given device is power manageable, it is not necessary to do this check in acpi_bus_set_power() any more, so remove it. Signed-off-by: Aaron Lu Signed-off-by: Rafael J. Wysocki --- drivers/acpi/device_pm.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index 8234e1f8c79d..beb9625e8458 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -243,13 +243,6 @@ int acpi_bus_set_power(acpi_handle handle, int state) 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; - } - return acpi_device_set_power(device, state); } EXPORT_SYMBOL(acpi_bus_set_power); From 593298e68a3a53bd2fe942244250dfef4d68d477 Mon Sep 17 00:00:00 2001 From: Aaron Lu Date: Sat, 3 Aug 2013 21:13:22 +0200 Subject: [PATCH 5/5] ACPI / PM: Add state information to error message in acpi_device_set_power() The state information can be useful to know what the problem is when an error message about a device can not being set to a higher power state than its parent appeared, so this patch adds such state information for both the target state of the device and the current state of its parent. Signed-off-by: Aaron Lu Signed-off-by: Rafael J. Wysocki --- drivers/acpi/device_pm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index beb9625e8458..59d3202f6b36 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -179,7 +179,9 @@ int acpi_device_set_power(struct acpi_device *device, int state) } if (device->parent && (state < device->parent->power.state)) { dev_warn(&device->dev, - "Cannot transition to a higher-powered state than parent\n"); + "Cannot transition to power state %s for parent in %s\n", + acpi_power_state_string(state), + acpi_power_state_string(device->parent->power.state)); return -ENODEV; }