ACPI / PM: Device PM cleanups
Subsystems (4)
cover ACPI / PM: Device PM cleanups
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
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
PATCH 1/3 ACPI / PM: Only set power states of devices that are power manageable
From: Rafael J. Wysocki <[email protected]>
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).
--- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/acpi/device_pm.c +2 -1
@@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_deint 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 */
Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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
Quoted 22 lines
Tested-by Yinghai Lu <[email protected]>Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +2
-1@@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_deint 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 */
Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable
Quoted 10 lines · Aaron Lu
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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).
Quoted 3 lines
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.
Quoted 8 lines
OTOH, what value should we return for a device node that is not power manageable in
acpi_device_set_powerwhen 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 originalacpi_device_set_power. So return-EINVALhere 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
Quoted 22 lines
Quoted 21 lines
Tested-by Yinghai Lu <[email protected]>Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +2
-1@@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_deint 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 */
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable
Quoted 15 lines · Rafael J. Wysocki
Quoted 10 lines · Aaron Lu
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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).Quoted 3 lines
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.
Quoted 7 lines
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.
Quoted 6 lines
Quoted 6 lines
OTOH, what value should we return for a device node that is not power manageable in
acpi_device_set_powerwhen the target state is D0 or D3 cold? The old behavior is to return 0, meanning success without taking any actual action.
Quoted 7 lines
Quoted 3 lines
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 originalacpi_device_set_power. So return-EINVALhere is correct?No, the original
acpi_device_set_power()will return-ENODEVthen, but in my opinion returning-EINVALis 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
Quoted 27 lines
Thanks,
RafaelQuoted 22 lines
Quoted 21 lines
Tested-by Yinghai Lu <[email protected]>Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +2
-1@@ -159,7 +159,8 @@ int acpi_device_set_power(struct acpi_deint 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 */
Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable
Quoted 36 lines · Aaron Lu
Quoted 15 lines · Rafael J. Wysocki
Quoted 10 lines · Aaron Lu
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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).Quoted 3 lines
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.
Quoted 7 lines
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.
Quoted 10 lines
Quoted 9 lines
OTOH, what value should we return for a device node that is not power manageable in
acpi_device_set_powerwhen 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 originalacpi_device_set_power. So return-EINVALhere is correct?
Quoted 5 lines
Quoted 3 lines
No, the original
acpi_device_set_power()will return-ENODEVthen, but in my opinion returning-EINVALis 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
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable
Quoted 41 lines · Rafael J. Wysocki
Quoted 40 lines · Aaron Lu
Quoted 15 lines · Rafael J. Wysocki
Quoted 10 lines · Aaron Lu
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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).Quoted 3 lines
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.
Quoted 7 lines
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.
Quoted 13 lines
Quoted 9 lines
OTOH, what value should we return for a device node that is not power manageable in
acpi_device_set_powerwhen 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 originalacpi_device_set_power. So return-EINVALhere is correct?No, the original
acpi_device_set_power()will return-ENODEVthen, but in my opinion returning-EINVALis more accurate, because "power manageable" means "you can change power state of it".
Quoted 5 lines
Quoted 1 line
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 <[email protected]>
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.
--- drivers/acpi/device_pm.c | 7 ------- 1 file changed, 7 deletions(-)
drivers/acpi/device_pm.c +0 -7
@@ -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);
Signature · 1 line
1.8.3.2.10.g43d11f4
Re: [PATCH 1/3] ACPI / PM: Only set power states of devices that are power manageable
Quoted 54 lines · Aaron Lu
Quoted 41 lines · Rafael J. Wysocki
Quoted 40 lines · Aaron Lu
Quoted 15 lines · Rafael J. Wysocki
Quoted 10 lines · Aaron Lu
Quoted 8 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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).Quoted 3 lines
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.
Quoted 7 lines
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.
Quoted 13 lines
Quoted 9 lines
OTOH, what value should we return for a device node that is not power manageable in
acpi_device_set_powerwhen 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 originalacpi_device_set_power. So return-EINVALhere is correct?No, the original
acpi_device_set_power()will return-ENODEVthen, but in my opinion returning-EINVALis more accurate, because "power manageable" means "you can change power state of it".Quoted 5 lines
Quoted 1 line
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 <[email protected]>
Subject: [PATCH 1/3] ACPI / PM: Remove redundant check for power manageable in
acpi_bus_set_power
Quoted 5 lines
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 inacpi_bus_set_power, so remove it.Signed-off-by Aaron Lu <[email protected]>
Looks good, I'll queue it up for 3.12.
Thanks,
Rafael
Quoted 23 lines
--- drivers/acpi/device_pm.c | 7 ------- 1 file changed, 7 deletions(-)drivers/acpi/device_pm.c +0
-7@@ -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);
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
PATCH 2/3 ACPI / PM: Make messages in acpi_device_set_power() print device names
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
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.
--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)
drivers/acpi/device_pm.c +10 -12
@@ -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_deif (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,
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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 )withdev_warn()everywhere in that function.Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +2
-3@@ -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));
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_de · line 182
Quoted 10 lines
drivers/acpi/device_pm.c +4
-3+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
Quoted 33 lines
drivers/acpi/device_pm.c +4
-7return -ENODEV ; }@@ -192,8 +192,8 @@ int acpi_device_set_power(struct acpi_deif (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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Quoted 47 lines · Aaron Lu
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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 )withdev_warn()everywhere in that function.Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +2
-3@@ -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));drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_de · line 182
Quoted 10 lines
drivers/acpi/device_pm.c +4
-3+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:
Quoted 3 lines
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
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Quoted 48 lines · Rafael J. Wysocki
Quoted 47 lines · Aaron Lu
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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 )withdev_warn()everywhere in that function.Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +2
-3@@ -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));drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_de · line 182
Quoted 10 lines
drivers/acpi/device_pm.c +4
-3+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:
Quoted 6 lines
Quoted 3 lines
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 <[email protected]>
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.
--- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/acpi/device_pm.c +2 -1
@@ -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 ; }
Signature · 1 line
1.8.3.2.10.g43d11f4
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Quoted 79 lines · Aaron Lu
Quoted 48 lines · Rafael J. Wysocki
Quoted 47 lines · Aaron Lu
Quoted 33 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
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 )withdev_warn()everywhere in that function.Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +2
-3@@ -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));drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_de · line 182
Quoted 10 lines
drivers/acpi/device_pm.c +4
-3+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:
Quoted 6 lines
Quoted 3 lines
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 <[email protected]>
Subject: [PATCH] ACPI / PM: Add state information in error message for
acpi_device_set_powerThe 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 <[email protected]>--- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/acpi/device_pm.c +0
-0
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_device *device, int state) · line 182
Quoted 5 lines
drivers/acpi/device_pm.c +1
-1@@ -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 ->
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_device *device, int state) · line 183
Quoted 1 line
drivers/acpi/device_pm.c +1
-0+ 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)
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_device *device, int state) · line 185
Quoted 2 lines
drivers/acpi/device_pm.c +0
-0return -ENODEV ; }
Thanks,
Rafael
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
PATCH 1/1 ACPI / PM: Add state information in error message for acpi_device_set_power
Quoted 30 lines · Rafael J. Wysocki
Quoted 18 lines
Patch to add state information in error message, apply on top of this
Quoted 17 lines
one.
From: Aaron Lu <[email protected]>
Subject: [PATCH] ACPI / PM: Add state information in error message for
acpi_device_set_powerThe 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 <[email protected]>--- drivers/acpi/device_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Quoted 10 lines
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_device *device, int state) · line 182
Quoted 10 lines
drivers/acpi/device_pm.c +1
-1@@ -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 ->
Quoted 8 lines
Quoted 1 line
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_device *device, int state) · line 183
Quoted 1 line
drivers/acpi/device_pm.c +1
-0+ 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 <[email protected]>
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.
--- drivers/acpi/device_pm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/acpi/device_pm.c +3 -1
@@ -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 ; }
Signature · 1 line
1.8.3.2.10.g43d11f4
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
In reply to Rafael J. Wysocki2013/7/27 Rafael J. Wysocki <[email protected]>:
Quoted 20 lines
From: Rafael J. Wysocki <[email protected]>
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 )withdev_warn()everywhere in that function.Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_de · line 169
Quoted 5 lines
drivers/acpi/device_pm.c +1
-1@@ -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?
Quoted 50 lines
drivers/acpi/device_pm.c +9
-12+ 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_deif (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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Best regards Tianyu Lan
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Quoted 2 lines · Lan Tianyu
2013/7/27 Rafael J. Wysocki <[email protected]>:
Quoted 1 line
From: Rafael J. Wysocki <[email protected]>
[]
Quoted 1 line
Quoted 1 line
drivers/acpi/device_pm.c | 22 ++++++++++------------
[]
Quoted 1 line
Review int acpi_device_set_power(struct acpi_de
Quoted 1 line
(quoted diff) +0
-0@@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de
[]
Quoted 6 lines
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_de · line 182
Quoted 6 lines
drivers/acpi/device_pm.c +2
-3if (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.
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Quoted 8 lines · Joe Perches
Quoted 2 lines · Lan Tianyu
2013/7/27 Rafael J. Wysocki <[email protected]>:
Quoted 1 line
From: Rafael J. Wysocki <[email protected]>
[]
Quoted 1 line
Quoted 1 line
drivers/acpi/device_pm.c | 22 ++++++++++------------[]
Quoted 1 line
Review int acpi_device_set_power(struct acpi_de
Quoted 1 line
(quoted diff) +0
-0@@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de[]
Quoted 8 lines
Quoted 6 lines
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_de · line 182
Quoted 6 lines
drivers/acpi/device_pm.c +2
-3if (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
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Hello.
On 29-07-2013 16:17, Rafael J. Wysocki wrote:
[...]
Quoted 9 lines
Quoted 8 lines
Quoted 1 line
Review int acpi_device_set_power(struct acpi_de
Quoted 1 line
(quoted diff) +0
-0@@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de[]
Quoted 6 lines
Quoted 6 lines
drivers/acpi/device_pm.c +2
-3if (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" );
Quoted 3 lines
Quoted 1 line
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.
Quoted 2 lines
Thanks,
Rafael
WBR, Sergei
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Quoted 15 lines · Sergei Shtylyov
Hello.
On 29-07-2013 16:17, Rafael J. Wysocki wrote:
[...]
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Review int acpi_device_set_power(struct acpi_de
Quoted 1 line
(quoted diff) +0
-0@@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de[]
Quoted 6 lines
Quoted 6 lines
(quoted diff) +2
-3if (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");
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
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 <[email protected]>
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.
--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)
drivers/acpi/device_pm.c +10 -12
@@ -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_deif (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,
Aaron Lu
Reviewed-byRe: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
Quoted 35 lines · Rafael J. Wysocki
Quoted 15 lines · Sergei Shtylyov
Hello.
On 29-07-2013 16:17, Rafael J. Wysocki wrote:
[...]
Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Review int acpi_device_set_power(struct acpi_de
Quoted 1 line
(quoted diff) +0
-0@@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de[]
Quoted 6 lines
Quoted 6 lines
drivers/acpi/device_pm.c +2
-3if (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" );Quoted 6 lines
Quoted 3 lines
Quoted 1 line
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 <[email protected]>
Subject: ACPI / PM: Make messages inacpi_device_set_power()print device names
Quoted 7 lines
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 )withdev_warn()everywhere in that function.
This makes the log much more informative, thanks!
Quoted 2 lines
Signed-off-by Rafael J. Wysocki <[email protected]>
And I'll add the state information in another patch on top of this one later.
-Aaron
Quoted 60 lines
--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +10
-12@@ -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_deif (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,
Re: [PATCH 2/3] ACPI / PM: Make messages in acpi_device_set_power() print device names
In reply to Lan TianyuQuoted 23 lines · Lan Tianyu
2013/7/27 Rafael J. Wysocki <[email protected]>:
Quoted 22 lines
From: Rafael J. Wysocki <[email protected]>
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 )withdev_warn()everywhere in that function.Signed-off-by Rafael J. Wysocki <[email protected]>--- drivers/acpi/device_pm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)Index:
linux-pm/drivers/acpi/device_pm.c
===================================================================drivers/acpi/device_pm.c +0
-0@@ -166,20 +166,20 @@ int acpi_device_set_power(struct acpi_de/* Make sure this is a valid target state */
Quoted 5 lines
drivers/acpi/device_pm.c int acpi_device_set_power(struct acpi_de · line 169
Quoted 3 lines
drivers/acpi/device_pm.c +1
-1if (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
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
PATCH 3/3 ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
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.]
--- 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(-)
drivers/acpi/fan.c +2 -2
@@ -93,7 +93,7 @@ static int fan_get_cur_state(struct therif (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_coolingreturn -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; }
drivers/acpi/power.c +1 -1
@@ -784,7 +784,7 @@ int acpi_power_get_inferred_state(struct } } - *state =ACPI_STATE_D3 ; + *state =ACPI_STATE_D3_COLD ;return 0 ; }
drivers/acpi/scan.c +2 -2
@@ -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)
drivers/ata/libata-acpi.c +2 -2
@@ -947,11 +947,11 @@ static void pata_acpi_set_state(struct acontinue ;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 ); }/**
drivers/ide/ide-acpi.c +3 -2
@@ -520,11 +520,12 @@ void ide_acpi_set_state(ide_hwif_t *hwifide_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 ); }/**
drivers/pnp/pnpacpi/core.c +3 -3
@@ -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_deif (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
include/acpi/acpi_bus.h +2 -1
@@ -477,7 +477,8 @@ static inline int acpi_pm_device_sleep_sif (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) {}
Re: [PATCH 3/3] ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere
Quoted 10 lines · Rafael J. Wysocki
From: Rafael J. Wysocki <[email protected]>
There are several places in the tree where
ACPI_STATE_D3is used instead ofACPI_STATE_D3_COLDwhich should be used instead for clarity. Modify them all to useACPI_STATE_D3_COLDas appropriate.[The definition of
ACPI_STATE_D3itself cannot go away at this point
as it is part of ACPICA.]Signed-off-by Rafael J. Wysocki <[email protected]>
Thanks,
Aaron
Quoted 138 lines
--- 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
===================================================================drivers/acpi/fan.c +2
-2@@ -93,7 +93,7 @@ static int fan_get_cur_state(struct therif (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_coolingreturn -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
===================================================================drivers/acpi/power.c +1
-1@@ -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
===================================================================drivers/acpi/scan.c +2
-2@@ -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
===================================================================drivers/ata/libata-acpi.c +2
-2@@ -947,11 +947,11 @@ static void pata_acpi_set_state(struct acontinue ;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
===================================================================drivers/ide/ide-acpi.c +3
-2@@ -520,11 +520,12 @@ void ide_acpi_set_state(ide_hwif_t *hwifide_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
===================================================================drivers/pnp/pnpacpi/core.c +3
-3@@ -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_deif (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
===================================================================include/acpi/acpi_bus.h +2
-1@@ -477,7 +477,8 @@ static inline int acpi_pm_device_sleep_sif (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) {}