* [PATCH 0/3] ACPI / PM: Device PM cleanups @ 2013-07-27 13:09 Rafael J. Wysocki 2013-07-27 13:10 ` [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable Rafael J. Wysocki ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-27 13:09 UTC (permalink / raw) To: ACPI Devel Maling List Cc: LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide Hi All, The following 3 patches clean up ACPI device PM a bit: [1/3] Fix acpi_device_set_power() to avoid printing useless messages for devices that aren't power manageable. [2/3] Fix messages in acpi_device_set_power() to always contain a device name. [3/3] Replace ACPI_STATE_D3 witn ACPI_STATE_D3_COLD everywhere in the tree (leave definition as part of ACPICA). Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable 2013-07-27 13:09 [PATCH 0/3] ACPI / PM: Device PM cleanups Rafael J. Wysocki @ 2013-07-27 13:10 ` Rafael J. Wysocki 2013-07-29 14:09 ` Aaron Lu 2013-07-27 13:11 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Rafael J. Wysocki 2013-07-27 13:14 ` [PATCH 3/3] ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere Rafael J. Wysocki 2 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-27 13:10 UTC (permalink / raw) To: ACPI Devel Maling List Cc: LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> 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 <yinghai@kernel.org> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: linux-pm/drivers/acpi/device_pm.c =================================================================== --- linux-pm.orig/drivers/acpi/device_pm.c +++ linux-pm/drivers/acpi/device_pm.c @@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_de 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 */ ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable 2013-07-27 13:10 ` [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable Rafael J. Wysocki @ 2013-07-29 14:09 ` Aaron Lu 2013-07-29 22:21 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Aaron Lu @ 2013-07-29 14:09 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On 07/27/2013 09:10 PM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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). It will not print "Device does not support" that power state if that power state is D0 or D3cold since we have unconditionally set those two power state's valid flag. OTOH, what value should we return for a device node that is not power manageable in acpi_device_set_power when the target state is D0 or D3 cold? The old behavior is to return 0, meanning success without taking any actual action. In acpi_bus_set_power, if the device is not power manageable, we will return -ENODEV; in acpi_dev_pm_full/low_power, we will return 0 as in the original acpi_device_set_power. So return -EINVAL here is correct? -Aaron > > Tested-by: Yinghai Lu <yinghai@kernel.org> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/device_pm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > Index: linux-pm/drivers/acpi/device_pm.c > =================================================================== > --- linux-pm.orig/drivers/acpi/device_pm.c > +++ linux-pm/drivers/acpi/device_pm.c > @@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_de > 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 */ > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable 2013-07-29 14:09 ` Aaron Lu @ 2013-07-29 22:21 ` Rafael J. Wysocki 2013-07-29 23:43 ` Aaron Lu 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-29 22:21 UTC (permalink / raw) To: Aaron Lu Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On Monday, July 29, 2013 10:09:53 PM Aaron Lu wrote: > On 07/27/2013 09:10 PM, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > 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). > > It will not print "Device does not support" that power state if that > power state is D0 or D3cold since we have unconditionally set those two > power state's valid flag. So you didn't actually looked at acpi_bus_get_power_flags() that set the power.states[].flags.valid flag, because If you had looked at it, you would have seen that that's not the case. No, we don't set the valid flag for devices that aren't power manageable (i.e. have flags.power_manageable unset), which is the *whole* *point* of this change. > OTOH, what value should we return for a device node that is not power > manageable in acpi_device_set_power when the target state is D0 or D3 > cold? The old behavior is to return 0, meanning success without taking > any actual action. > > In acpi_bus_set_power, if the device is not power manageable, we will > return -ENODEV; in acpi_dev_pm_full/low_power, we will return 0 as in > the original acpi_device_set_power. So return -EINVAL here is correct? No, the original acpi_device_set_power() will return -ENODEV then, but in my opinion returning -EINVAL is more accurate, because "power manageable" means "you can change power state of it". Thanks, Rafael > > Tested-by: Yinghai Lu <yinghai@kernel.org> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > drivers/acpi/device_pm.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > Index: linux-pm/drivers/acpi/device_pm.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/device_pm.c > > +++ linux-pm/drivers/acpi/device_pm.c > > @@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_de > > 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 */ > > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable 2013-07-29 22:21 ` Rafael J. Wysocki @ 2013-07-29 23:43 ` Aaron Lu 2013-07-30 14:04 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Aaron Lu @ 2013-07-29 23:43 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On 07/30/2013 06:21 AM, Rafael J. Wysocki wrote: > On Monday, July 29, 2013 10:09:53 PM Aaron Lu wrote: >> On 07/27/2013 09:10 PM, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> 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). >> >> It will not print "Device does not support" that power state if that >> power state is D0 or D3cold since we have unconditionally set those two >> power state's valid flag. > > So you didn't actually looked at acpi_bus_get_power_flags() that set the > power.states[].flags.valid flag, because If you had looked at it, you would > have seen that that's not the case. > > No, we don't set the valid flag for devices that aren't power manageable > (i.e. have flags.power_manageable unset), which is the *whole* *point* of > this change. Right, I missed this. Sorry for the noise. > >> OTOH, what value should we return for a device node that is not power >> manageable in acpi_device_set_power when the target state is D0 or D3 >> cold? The old behavior is to return 0, meanning success without taking >> any actual action. >> >> In acpi_bus_set_power, if the device is not power manageable, we will >> return -ENODEV; in acpi_dev_pm_full/low_power, we will return 0 as in >> the original acpi_device_set_power. So return -EINVAL here is correct? > > No, the original acpi_device_set_power() will return -ENODEV then, but > in my opinion returning -EINVAL is more accurate, because "power > manageable" means "you can change power state of it". Shall I prepare a patch to update the errno in acpi_bus_set_power? Thanks, Aaron > > Thanks, > Rafael > > >>> Tested-by: Yinghai Lu <yinghai@kernel.org> >>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> --- >>> drivers/acpi/device_pm.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> Index: linux-pm/drivers/acpi/device_pm.c >>> =================================================================== >>> --- linux-pm.orig/drivers/acpi/device_pm.c >>> +++ linux-pm/drivers/acpi/device_pm.c >>> @@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_de >>> 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 */ >>> >> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable 2013-07-29 23:43 ` Aaron Lu @ 2013-07-30 14:04 ` Rafael J. Wysocki 2013-07-31 6:48 ` Aaron Lu 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-30 14:04 UTC (permalink / raw) To: Aaron Lu Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On Tuesday, July 30, 2013 07:43:48 AM Aaron Lu wrote: > On 07/30/2013 06:21 AM, Rafael J. Wysocki wrote: > > On Monday, July 29, 2013 10:09:53 PM Aaron Lu wrote: > >> On 07/27/2013 09:10 PM, Rafael J. Wysocki wrote: > >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >>> > >>> 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). > >> > >> It will not print "Device does not support" that power state if that > >> power state is D0 or D3cold since we have unconditionally set those two > >> power state's valid flag. > > > > So you didn't actually looked at acpi_bus_get_power_flags() that set the > > power.states[].flags.valid flag, because If you had looked at it, you would > > have seen that that's not the case. > > > > No, we don't set the valid flag for devices that aren't power manageable > > (i.e. have flags.power_manageable unset), which is the *whole* *point* of > > this change. > > Right, I missed this. Sorry for the noise. > > > > >> OTOH, what value should we return for a device node that is not power > >> manageable in acpi_device_set_power when the target state is D0 or D3 > >> cold? The old behavior is to return 0, meanning success without taking > >> any actual action. > >> > >> In acpi_bus_set_power, if the device is not power manageable, we will > >> return -ENODEV; in acpi_dev_pm_full/low_power, we will return 0 as in > >> the original acpi_device_set_power. So return -EINVAL here is correct? > > > > No, the original acpi_device_set_power() will return -ENODEV then, but > > in my opinion returning -EINVAL is more accurate, because "power > > manageable" means "you can change power state of it". > > Shall I prepare a patch to update the errno in acpi_bus_set_power? In fact, it doesn't need to check flags.power_manageable after this patch and the debug message won't be missed I think, so please just remove the whole if () from there, if that's not a problem. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable 2013-07-30 14:04 ` Rafael J. Wysocki @ 2013-07-31 6:48 ` Aaron Lu 2013-07-31 10:29 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Aaron Lu @ 2013-07-31 6:48 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On 07/30/2013 10:04 PM, Rafael J. Wysocki wrote: > On Tuesday, July 30, 2013 07:43:48 AM Aaron Lu wrote: >> On 07/30/2013 06:21 AM, Rafael J. Wysocki wrote: >>> On Monday, July 29, 2013 10:09:53 PM Aaron Lu wrote: >>>> On 07/27/2013 09:10 PM, Rafael J. Wysocki wrote: >>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>>>> >>>>> 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). >>>> >>>> It will not print "Device does not support" that power state if that >>>> power state is D0 or D3cold since we have unconditionally set those two >>>> power state's valid flag. >>> >>> So you didn't actually looked at acpi_bus_get_power_flags() that set the >>> power.states[].flags.valid flag, because If you had looked at it, you would >>> have seen that that's not the case. >>> >>> No, we don't set the valid flag for devices that aren't power manageable >>> (i.e. have flags.power_manageable unset), which is the *whole* *point* of >>> this change. >> >> Right, I missed this. Sorry for the noise. >> >>> >>>> OTOH, what value should we return for a device node that is not power >>>> manageable in acpi_device_set_power when the target state is D0 or D3 >>>> cold? The old behavior is to return 0, meanning success without taking >>>> any actual action. >>>> >>>> In acpi_bus_set_power, if the device is not power manageable, we will >>>> return -ENODEV; in acpi_dev_pm_full/low_power, we will return 0 as in >>>> the original acpi_device_set_power. So return -EINVAL here is correct? >>> >>> No, the original acpi_device_set_power() will return -ENODEV then, but >>> in my opinion returning -EINVAL is more accurate, because "power >>> manageable" means "you can change power state of it". >> >> Shall I prepare a patch to update the errno in acpi_bus_set_power? > > In fact, it doesn't need to check flags.power_manageable after this patch > and the debug message won't be missed I think, so please just remove > the whole if () from there, if that's not a problem. Patch to remove the redundant check, apply on top of this one. From: Aaron Lu <aaron.lu@intel.com> Subject: [PATCH 1/3] ACPI / PM: Remove redundant check for power manageable in acpi_bus_set_power Now that we will check if a device is power manageable in acpi_device_set_power, it is no longer necessary to do this check in acpi_bus_set_power, so remove it. Signed-off-by: Aaron Lu <aaron.lu@intel.com> --- 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 63324b8..8270711 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -245,13 +245,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); -- 1.8.3.2.10.g43d11f4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable 2013-07-31 6:48 ` Aaron Lu @ 2013-07-31 10:29 ` Rafael J. Wysocki 0 siblings, 0 replies; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-31 10:29 UTC (permalink / raw) To: Aaron Lu Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On Wednesday, July 31, 2013 02:48:58 PM Aaron Lu wrote: > On 07/30/2013 10:04 PM, Rafael J. Wysocki wrote: > > On Tuesday, July 30, 2013 07:43:48 AM Aaron Lu wrote: > >> On 07/30/2013 06:21 AM, Rafael J. Wysocki wrote: > >>> On Monday, July 29, 2013 10:09:53 PM Aaron Lu wrote: > >>>> On 07/27/2013 09:10 PM, Rafael J. Wysocki wrote: > >>>>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >>>>> > >>>>> 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). > >>>> > >>>> It will not print "Device does not support" that power state if that > >>>> power state is D0 or D3cold since we have unconditionally set those two > >>>> power state's valid flag. > >>> > >>> So you didn't actually looked at acpi_bus_get_power_flags() that set the > >>> power.states[].flags.valid flag, because If you had looked at it, you would > >>> have seen that that's not the case. > >>> > >>> No, we don't set the valid flag for devices that aren't power manageable > >>> (i.e. have flags.power_manageable unset), which is the *whole* *point* of > >>> this change. > >> > >> Right, I missed this. Sorry for the noise. > >> > >>> > >>>> OTOH, what value should we return for a device node that is not power > >>>> manageable in acpi_device_set_power when the target state is D0 or D3 > >>>> cold? The old behavior is to return 0, meanning success without taking > >>>> any actual action. > >>>> > >>>> In acpi_bus_set_power, if the device is not power manageable, we will > >>>> return -ENODEV; in acpi_dev_pm_full/low_power, we will return 0 as in > >>>> the original acpi_device_set_power. So return -EINVAL here is correct? > >>> > >>> No, the original acpi_device_set_power() will return -ENODEV then, but > >>> in my opinion returning -EINVAL is more accurate, because "power > >>> manageable" means "you can change power state of it". > >> > >> Shall I prepare a patch to update the errno in acpi_bus_set_power? > > > > In fact, it doesn't need to check flags.power_manageable after this patch > > and the debug message won't be missed I think, so please just remove > > the whole if () from there, if that's not a problem. > > Patch to remove the redundant check, apply on top of this one. > > From: Aaron Lu <aaron.lu@intel.com> > Subject: [PATCH 1/3] ACPI / PM: Remove redundant check for power manageable in > acpi_bus_set_power > > Now that we will check if a device is power manageable in > acpi_device_set_power, it is no longer necessary to do this check in > acpi_bus_set_power, so remove it. > > Signed-off-by: Aaron Lu <aaron.lu@intel.com> Looks good, I'll queue it up for 3.12. Thanks, Rafael > --- > 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 63324b8..8270711 100644 > --- a/drivers/acpi/device_pm.c > +++ b/drivers/acpi/device_pm.c > @@ -245,13 +245,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); > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-27 13:09 [PATCH 0/3] ACPI / PM: Device PM cleanups Rafael J. Wysocki 2013-07-27 13:10 ` [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable Rafael J. Wysocki @ 2013-07-27 13:11 ` Rafael J. Wysocki 2013-07-29 2:29 ` Aaron Lu 2013-07-29 3:06 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Lan Tianyu 2013-07-27 13:14 ` [PATCH 3/3] ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere Rafael J. Wysocki 2 siblings, 2 replies; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-27 13:11 UTC (permalink / raw) To: ACPI Devel Maling List Cc: LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> 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 <rafael.j.wysocki@intel.com> --- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) Index: linux-pm/drivers/acpi/device_pm.c =================================================================== --- linux-pm.orig/drivers/acpi/device_pm.c +++ linux-pm/drivers/acpi/device_pm.c @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de /* 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_de 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_de 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, ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-27 13:11 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Rafael J. Wysocki @ 2013-07-29 2:29 ` Aaron Lu 2013-07-29 12:20 ` Rafael J. Wysocki 2013-07-29 3:06 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Lan Tianyu 1 sibling, 1 reply; 23+ messages in thread From: Aaron Lu @ 2013-07-29 2:29 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide On 07/27/2013 09:11 PM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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 <rafael.j.wysocki@intel.com> > --- > drivers/acpi/device_pm.c | 22 ++++++++++------------ > 1 file changed, 10 insertions(+), 12 deletions(-) > > Index: linux-pm/drivers/acpi/device_pm.c > =================================================================== > --- linux-pm.orig/drivers/acpi/device_pm.c > +++ linux-pm/drivers/acpi/device_pm.c > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de > /* 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"); I think the state information would also be useful here: dev_warn(&device->dev, "Cannot transition to a higher-powereed " "state %d than paeren's state %d\n", state, device->parent->power.state); Thanks, Aaron > return -ENODEV; > } > > @@ -192,8 +192,8 @@ int acpi_device_set_power(struct acpi_de > > 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_de > > 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, > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ide" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-29 2:29 ` Aaron Lu @ 2013-07-29 12:20 ` Rafael J. Wysocki 2013-07-31 6:52 ` Aaron Lu 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-29 12:20 UTC (permalink / raw) To: Aaron Lu Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On Monday, July 29, 2013 10:29:36 AM Aaron Lu wrote: > On 07/27/2013 09:11 PM, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > 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 <rafael.j.wysocki@intel.com> > > --- > > drivers/acpi/device_pm.c | 22 ++++++++++------------ > > 1 file changed, 10 insertions(+), 12 deletions(-) > > > > Index: linux-pm/drivers/acpi/device_pm.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/device_pm.c > > +++ linux-pm/drivers/acpi/device_pm.c > > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de > > /* 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"); > > I think the state information would also be useful here: > > dev_warn(&device->dev, "Cannot transition to a higher-powereed " > "state %d than paeren's state %d\n", state, > device->parent->power.state); This is not the scope of this patch, please send another one on top of it. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-29 12:20 ` Rafael J. Wysocki @ 2013-07-31 6:52 ` Aaron Lu 2013-07-31 10:27 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Aaron Lu @ 2013-07-31 6:52 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On 07/29/2013 08:20 PM, Rafael J. Wysocki wrote: > On Monday, July 29, 2013 10:29:36 AM Aaron Lu wrote: >> On 07/27/2013 09:11 PM, Rafael J. Wysocki wrote: >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >>> >>> 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 <rafael.j.wysocki@intel.com> >>> --- >>> drivers/acpi/device_pm.c | 22 ++++++++++------------ >>> 1 file changed, 10 insertions(+), 12 deletions(-) >>> >>> Index: linux-pm/drivers/acpi/device_pm.c >>> =================================================================== >>> --- linux-pm.orig/drivers/acpi/device_pm.c >>> +++ linux-pm/drivers/acpi/device_pm.c >>> @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de >>> /* 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"); >> >> I think the state information would also be useful here: >> >> dev_warn(&device->dev, "Cannot transition to a higher-powereed " >> "state %d than paeren's state %d\n", state, >> device->parent->power.state); > > This is not the scope of this patch, please send another one on top of it. > Patch to add state information in error message, apply on top of this one. From: Aaron Lu <aaron.lu@intel.com> Subject: [PATCH] ACPI / PM: Add state information in error message for acpi_device_set_power The state information can be useful to know what the problem is when it appeared in an error message about a device can not being set to a higher power state than its parent, so this patch adds such state information for both the target state of the device failed to be set and the current parent's state. Signed-off-by: Aaron Lu <aaron.lu@intel.com> --- 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 beb9625..707258b 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -179,7 +179,8 @@ 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 a higher-powered state %d than parent's state %d\n", + state, device->parent->power.state); return -ENODEV; } -- 1.8.3.2.10.g43d11f4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-31 6:52 ` Aaron Lu @ 2013-07-31 10:27 ` Rafael J. Wysocki 2013-08-01 0:49 ` [PATCH updated] ACPI / PM: Add state information in error message for acpi_device_set_power Aaron Lu 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-31 10:27 UTC (permalink / raw) To: Aaron Lu Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On Wednesday, July 31, 2013 02:52:53 PM Aaron Lu wrote: > On 07/29/2013 08:20 PM, Rafael J. Wysocki wrote: > > On Monday, July 29, 2013 10:29:36 AM Aaron Lu wrote: > >> On 07/27/2013 09:11 PM, Rafael J. Wysocki wrote: > >>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > >>> > >>> 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 <rafael.j.wysocki@intel.com> > >>> --- > >>> drivers/acpi/device_pm.c | 22 ++++++++++------------ > >>> 1 file changed, 10 insertions(+), 12 deletions(-) > >>> > >>> Index: linux-pm/drivers/acpi/device_pm.c > >>> =================================================================== > >>> --- linux-pm.orig/drivers/acpi/device_pm.c > >>> +++ linux-pm/drivers/acpi/device_pm.c > >>> @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de > >>> /* 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"); > >> > >> I think the state information would also be useful here: > >> > >> dev_warn(&device->dev, "Cannot transition to a higher-powereed " > >> "state %d than paeren's state %d\n", state, > >> device->parent->power.state); > > > > This is not the scope of this patch, please send another one on top of it. > > > > Patch to add state information in error message, apply on top of this > one. > > From: Aaron Lu <aaron.lu@intel.com> > Subject: [PATCH] ACPI / PM: Add state information in error message for > acpi_device_set_power > > The state information can be useful to know what the problem is when it > appeared in an error message about a device can not being set to a higher > power state than its parent, so this patch adds such state information > for both the target state of the device failed to be set and the current > parent's state. > > Signed-off-by: Aaron Lu <aaron.lu@intel.com> > --- > 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 beb9625..707258b 100644 > --- a/drivers/acpi/device_pm.c > +++ b/drivers/acpi/device_pm.c > @@ -179,7 +179,8 @@ 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 a higher-powered state %d than parent's state %d\n", This message will look a little odd I think -> > + state, device->parent->power.state); -> and please don't use raw numbers in such messages. What about "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; > } Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH updated] ACPI / PM: Add state information in error message for acpi_device_set_power 2013-07-31 10:27 ` Rafael J. Wysocki @ 2013-08-01 0:49 ` Aaron Lu 0 siblings, 0 replies; 23+ messages in thread From: Aaron Lu @ 2013-08-01 0:49 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On 07/31/2013 06:27 PM, Rafael J. Wysocki wrote: >> Patch to add state information in error message, apply on top of this >> > one. >> > >> > From: Aaron Lu <aaron.lu@intel.com> >> > Subject: [PATCH] ACPI / PM: Add state information in error message for >> > acpi_device_set_power >> > >> > The state information can be useful to know what the problem is when it >> > appeared in an error message about a device can not being set to a higher >> > power state than its parent, so this patch adds such state information >> > for both the target state of the device failed to be set and the current >> > parent's state. >> > >> > Signed-off-by: Aaron Lu <aaron.lu@intel.com> >> > --- >> > 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 beb9625..707258b 100644 >> > --- a/drivers/acpi/device_pm.c >> > +++ b/drivers/acpi/device_pm.c >> > @@ -179,7 +179,8 @@ 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 a higher-powered state %d than parent's state %d\n", > This message will look a little odd I think -> > >> > + state, device->parent->power.state); > -> and please don't use raw numbers in such messages. > > What about > > "Cannot transition to power state %s for parent in %s\n", > acpi_power_state_string(state), > acpi_power_state_string(device->parent->power.state) Thanks for the suggestion, updated patch here: From: Aaron Lu <aaron.lu@intel.com> Subject: [PATCH] ACPI / PM: Add state information in error message for 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 <aaron.lu@intel.com> --- 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 beb9625..59d3202 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; } -- 1.8.3.2.10.g43d11f4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-27 13:11 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Rafael J. Wysocki 2013-07-29 2:29 ` Aaron Lu @ 2013-07-29 3:06 ` Lan Tianyu 2013-07-29 3:11 ` Joe Perches 2013-07-29 12:11 ` Rafael J. Wysocki 1 sibling, 2 replies; 23+ messages in thread From: Lan Tianyu @ 2013-07-29 3:06 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide 2013/7/27 Rafael J. Wysocki <rjw@sisk.pl>: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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 <rafael.j.wysocki@intel.com> > --- > drivers/acpi/device_pm.c | 22 ++++++++++------------ > 1 file changed, 10 insertions(+), 12 deletions(-) > > Index: linux-pm/drivers/acpi/device_pm.c > =================================================================== > --- linux-pm.orig/drivers/acpi/device_pm.c > +++ linux-pm/drivers/acpi/device_pm.c > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de > /* 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", ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ Missing "is" or it should be omitted? > + 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_de > > 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_de > > 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, > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best regards Tianyu Lan ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-29 3:06 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Lan Tianyu @ 2013-07-29 3:11 ` Joe Perches 2013-07-29 12:17 ` Rafael J. Wysocki 2013-07-29 12:11 ` Rafael J. Wysocki 1 sibling, 1 reply; 23+ messages in thread From: Joe Perches @ 2013-07-29 3:11 UTC (permalink / raw) To: Lan Tianyu Cc: Rafael J. Wysocki, ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide On Mon, 2013-07-29 at 11:06 +0800, Lan Tianyu wrote: > 2013/7/27 Rafael J. Wysocki <rjw@sisk.pl>: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> [] > > drivers/acpi/device_pm.c | 22 ++++++++++------------ [] > > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de [] > > 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"); coalesce format please. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-29 3:11 ` Joe Perches @ 2013-07-29 12:17 ` Rafael J. Wysocki 2013-07-29 12:16 ` Sergei Shtylyov 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-29 12:17 UTC (permalink / raw) To: Joe Perches Cc: Lan Tianyu, ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide On Sunday, July 28, 2013 08:11:53 PM Joe Perches wrote: > On Mon, 2013-07-29 at 11:06 +0800, Lan Tianyu wrote: > > 2013/7/27 Rafael J. Wysocki <rjw@sisk.pl>: > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > [] > > > drivers/acpi/device_pm.c | 22 ++++++++++------------ > [] > > > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de > [] > > > 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"); > > coalesce format please. I can, but then it'll cross the 80 columns boundary. Thanks, Rafael ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-29 12:17 ` Rafael J. Wysocki @ 2013-07-29 12:16 ` Sergei Shtylyov 2013-07-29 13:36 ` Rafael J. Wysocki 0 siblings, 1 reply; 23+ messages in thread From: Sergei Shtylyov @ 2013-07-29 12:16 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Joe Perches, Lan Tianyu, ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide Hello. On 29-07-2013 16:17, Rafael J. Wysocki wrote: [...] >>>> @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de >> [] >>>> 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"); >> coalesce format please. > I can, but then it'll cross the 80 columns boundary. It's not a problem with checkpatch.pl anymore. Contrariwise, it whines about the broken up string literals, AFAIR. > Thanks, > Rafael WBR, Sergei ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-29 12:16 ` Sergei Shtylyov @ 2013-07-29 13:36 ` Rafael J. Wysocki 2013-07-29 14:15 ` Aaron Lu 0 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-29 13:36 UTC (permalink / raw) To: Sergei Shtylyov Cc: Joe Perches, Lan Tianyu, ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide On Monday, July 29, 2013 04:16:31 PM Sergei Shtylyov wrote: > Hello. > > On 29-07-2013 16:17, Rafael J. Wysocki wrote: > > [...] > > >>>> @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de > >> [] > >>>> 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"); > > >> coalesce format please. > > > I can, but then it'll cross the 80 columns boundary. > > It's not a problem with checkpatch.pl anymore. Contrariwise, it whines > about the broken up string literals, AFAIR. Oh, at last. Updated patch follows. Thanks, Rafael --- From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Subject: 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 <rafael.j.wysocki@intel.com> --- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) Index: linux-pm/drivers/acpi/device_pm.c =================================================================== --- linux-pm.orig/drivers/acpi/device_pm.c +++ linux-pm/drivers/acpi/device_pm.c @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de /* 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_de 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_de 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, ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-29 13:36 ` Rafael J. Wysocki @ 2013-07-29 14:15 ` Aaron Lu 0 siblings, 0 replies; 23+ messages in thread From: Aaron Lu @ 2013-07-29 14:15 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Sergei Shtylyov, Joe Perches, Lan Tianyu, ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On 07/29/2013 09:36 PM, Rafael J. Wysocki wrote: > On Monday, July 29, 2013 04:16:31 PM Sergei Shtylyov wrote: >> Hello. >> >> On 29-07-2013 16:17, Rafael J. Wysocki wrote: >> >> [...] >> >>>>>> @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de >>>> [] >>>>>> 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"); >> >>>> coalesce format please. >> >>> I can, but then it'll cross the 80 columns boundary. >> >> It's not a problem with checkpatch.pl anymore. Contrariwise, it whines >> about the broken up string literals, AFAIR. > > Oh, at last. > > Updated patch follows. > > Thanks, > Rafael > > > --- > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Subject: 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. This makes the log much more informative, thanks! > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Reviewed-by: Aaron Lu <aaron.lu@intel.com> And I'll add the state information in another patch on top of this one later. -Aaron > --- > drivers/acpi/device_pm.c | 22 ++++++++++------------ > 1 file changed, 10 insertions(+), 12 deletions(-) > > Index: linux-pm/drivers/acpi/device_pm.c > =================================================================== > --- linux-pm.orig/drivers/acpi/device_pm.c > +++ linux-pm/drivers/acpi/device_pm.c > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de > /* 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_de > > 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_de > > 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, > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names 2013-07-29 3:06 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Lan Tianyu 2013-07-29 3:11 ` Joe Perches @ 2013-07-29 12:11 ` Rafael J. Wysocki 1 sibling, 0 replies; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-29 12:11 UTC (permalink / raw) To: Lan Tianyu Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide On Monday, July 29, 2013 11:06:36 AM Lan Tianyu wrote: > 2013/7/27 Rafael J. Wysocki <rjw@sisk.pl>: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > 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 <rafael.j.wysocki@intel.com> > > --- > > drivers/acpi/device_pm.c | 22 ++++++++++------------ > > 1 file changed, 10 insertions(+), 12 deletions(-) > > > > Index: linux-pm/drivers/acpi/device_pm.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/device_pm.c > > +++ linux-pm/drivers/acpi/device_pm.c > > @@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de > > /* 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", > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ > Missing "is" or it should be omitted? The "is" is not necessary here (in my opinion). Kind of in analogy with "This has been done already" and "Already done". Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/3] ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere 2013-07-27 13:09 [PATCH 0/3] ACPI / PM: Device PM cleanups Rafael J. Wysocki 2013-07-27 13:10 ` [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable Rafael J. Wysocki 2013-07-27 13:11 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Rafael J. Wysocki @ 2013-07-27 13:14 ` Rafael J. Wysocki 2013-07-29 14:28 ` Aaron Lu 2 siblings, 1 reply; 23+ messages in thread From: Rafael J. Wysocki @ 2013-07-27 13:14 UTC (permalink / raw) To: ACPI Devel Maling List Cc: LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Aaron Lu, Tejun Heo, linux-ide From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> 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 <rafael.j.wysocki@intel.com> --- 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(-) Index: linux-pm/drivers/acpi/fan.c =================================================================== --- linux-pm.orig/drivers/acpi/fan.c +++ linux-pm/drivers/acpi/fan.c @@ -93,7 +93,7 @@ static int fan_get_cur_state(struct ther 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 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; } Index: linux-pm/drivers/acpi/power.c =================================================================== --- linux-pm.orig/drivers/acpi/power.c +++ linux-pm/drivers/acpi/power.c @@ -784,7 +784,7 @@ int acpi_power_get_inferred_state(struct } } - *state = ACPI_STATE_D3; + *state = ACPI_STATE_D3_COLD; return 0; } Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -1409,8 +1409,8 @@ static void acpi_bus_get_power_flags(str /* 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) Index: linux-pm/drivers/ata/libata-acpi.c =================================================================== --- linux-pm.orig/drivers/ata/libata-acpi.c +++ linux-pm/drivers/ata/libata-acpi.c @@ -947,11 +947,11 @@ static void pata_acpi_set_state(struct a 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); } /** Index: linux-pm/drivers/ide/ide-acpi.c =================================================================== --- linux-pm.orig/drivers/ide/ide-acpi.c +++ linux-pm/drivers/ide/ide-acpi.c @@ -520,11 +520,12 @@ void ide_acpi_set_state(ide_hwif_t *hwif 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); } /** Index: linux-pm/drivers/pnp/pnpacpi/core.c =================================================================== --- linux-pm.orig/drivers/pnp/pnpacpi/core.c +++ linux-pm/drivers/pnp/pnpacpi/core.c @@ -131,7 +131,7 @@ static int pnpacpi_disable_resources(str /* 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_de 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 Index: linux-pm/include/acpi/acpi_bus.h =================================================================== --- linux-pm.orig/include/acpi/acpi_bus.h +++ linux-pm/include/acpi/acpi_bus.h @@ -477,7 +477,8 @@ static inline int acpi_pm_device_sleep_s 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) {} ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 3/3] ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere 2013-07-27 13:14 ` [PATCH 3/3] ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere Rafael J. Wysocki @ 2013-07-29 14:28 ` Aaron Lu 0 siblings, 0 replies; 23+ messages in thread From: Aaron Lu @ 2013-07-29 14:28 UTC (permalink / raw) To: Rafael J. Wysocki Cc: ACPI Devel Maling List, LKML, Linux PM list, Yinghai Lu, Bjorn Helgaas, Tejun Heo, linux-ide On 07/27/2013 09:14 PM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > 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 <rafael.j.wysocki@intel.com> Reviewed-by: Aaron Lu <aaron.lu@intel.com> Thanks, Aaron > --- > 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(-) > > Index: linux-pm/drivers/acpi/fan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/fan.c > +++ linux-pm/drivers/acpi/fan.c > @@ -93,7 +93,7 @@ static int fan_get_cur_state(struct ther > 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 > 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; > } > Index: linux-pm/drivers/acpi/power.c > =================================================================== > --- linux-pm.orig/drivers/acpi/power.c > +++ linux-pm/drivers/acpi/power.c > @@ -784,7 +784,7 @@ int acpi_power_get_inferred_state(struct > } > } > > - *state = ACPI_STATE_D3; > + *state = ACPI_STATE_D3_COLD; > return 0; > } > > Index: linux-pm/drivers/acpi/scan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -1409,8 +1409,8 @@ static void acpi_bus_get_power_flags(str > /* 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) > Index: linux-pm/drivers/ata/libata-acpi.c > =================================================================== > --- linux-pm.orig/drivers/ata/libata-acpi.c > +++ linux-pm/drivers/ata/libata-acpi.c > @@ -947,11 +947,11 @@ static void pata_acpi_set_state(struct a > 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); > } > > /** > Index: linux-pm/drivers/ide/ide-acpi.c > =================================================================== > --- linux-pm.orig/drivers/ide/ide-acpi.c > +++ linux-pm/drivers/ide/ide-acpi.c > @@ -520,11 +520,12 @@ void ide_acpi_set_state(ide_hwif_t *hwif > 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); > } > > /** > Index: linux-pm/drivers/pnp/pnpacpi/core.c > =================================================================== > --- linux-pm.orig/drivers/pnp/pnpacpi/core.c > +++ linux-pm/drivers/pnp/pnpacpi/core.c > @@ -131,7 +131,7 @@ static int pnpacpi_disable_resources(str > /* 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_de > > 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 > Index: linux-pm/include/acpi/acpi_bus.h > =================================================================== > --- linux-pm.orig/include/acpi/acpi_bus.h > +++ linux-pm/include/acpi/acpi_bus.h > @@ -477,7 +477,8 @@ static inline int acpi_pm_device_sleep_s > 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) {} > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2013-08-01 0:49 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-27 13:09 [PATCH 0/3] ACPI / PM: Device PM cleanups Rafael J. Wysocki 2013-07-27 13:10 ` [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable Rafael J. Wysocki 2013-07-29 14:09 ` Aaron Lu 2013-07-29 22:21 ` Rafael J. Wysocki 2013-07-29 23:43 ` Aaron Lu 2013-07-30 14:04 ` Rafael J. Wysocki 2013-07-31 6:48 ` Aaron Lu 2013-07-31 10:29 ` Rafael J. Wysocki 2013-07-27 13:11 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Rafael J. Wysocki 2013-07-29 2:29 ` Aaron Lu 2013-07-29 12:20 ` Rafael J. Wysocki 2013-07-31 6:52 ` Aaron Lu 2013-07-31 10:27 ` Rafael J. Wysocki 2013-08-01 0:49 ` [PATCH updated] ACPI / PM: Add state information in error message for acpi_device_set_power Aaron Lu 2013-07-29 3:06 ` [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names Lan Tianyu 2013-07-29 3:11 ` Joe Perches 2013-07-29 12:17 ` Rafael J. Wysocki 2013-07-29 12:16 ` Sergei Shtylyov 2013-07-29 13:36 ` Rafael J. Wysocki 2013-07-29 14:15 ` Aaron Lu 2013-07-29 12:11 ` Rafael J. Wysocki 2013-07-27 13:14 ` [PATCH 3/3] ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere Rafael J. Wysocki 2013-07-29 14:28 ` Aaron Lu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.