On 2012/11/2 19:17, Rafael J. Wysocki wrote:
On Friday, November 02, 2012 04:03:50 PM Lan Tianyu wrote:
Since dev_pm_qos_add_request(), dev_pm_qos_update_request() and
dev_pm_qos_remove_request() for pm qos flags should not be invoked
when device in RPM_SUSPENDED. Add pm_runtime_get_sync() and pm_runtime_put()
around these functions in the pm core to ensure device not in RPM_SUSPENDED.
Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
---
drivers/base/power/qos.c | 10 ++++++++--
drivers/base/power/sysfs.c | 4 ++++
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 31d3f48..b50ba72 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -624,15 +624,19 @@ int dev_pm_qos_expose_flags(struct device *dev, s32 val)
if (!req)
return -ENOMEM;
+ pm_runtime_get_sync(dev);
ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_FLAGS, val);
+ pm_runtime_put(dev);
I would drop this one ...
if (ret < 0)
return ret;
dev->power.qos->flags_req = req;
ret = pm_qos_sysfs_add_flags(dev);
- if (ret)
+ if (ret) {
+ pm_runtime_get_sync(dev);
__dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
-
+ pm_runtime_put(dev);
... and move this one before the return statement (plus a label for
goto from the ret < 0 check after adding the request).
What you mean likes following?
+ pm_runtime_get_sync(dev);
ret = dev_pm_qos_add_request(dev, req, DEV_PM_QOS_FLAGS, val);
if (ret < 0)
- return ret;
+ goto fail;
dev->power.qos->flags_req = req;
ret = pm_qos_sysfs_add_flags(dev);
if (ret)
__dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
+fail:
+ pm_runtime_put(dev);
return ret;
}
+ }
return ret;
}
EXPORT_SYMBOL_GPL(dev_pm_qos_expose_flags);
@@ -645,7 +649,9 @@ void dev_pm_qos_hide_flags(struct device *dev)
{
if (dev->power.qos && dev->power.qos->flags_req) {
pm_qos_sysfs_remove_flags(dev);
+ pm_runtime_get_sync(dev);
__dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_FLAGS);
+ pm_runtime_put(dev);
I'm not sure if these two are necessary. If we remove a request,
then what happens worst case is that some flags will be cleared
effectively which means fewer restrictions on the next sleep state.
Then, it shouldn't hurt that the current sleep state is more
restricted.
But this mean the device can be put into lower power state(power off).
So why not do that? that can save more power, right?
Hmm. Perhaps a comment would suffice?
}
}
EXPORT_SYMBOL_GPL(dev_pm_qos_hide_flags);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 50d16e3..240bfaa 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -264,7 +264,9 @@ static ssize_t pm_qos_no_power_off_store(struct device *dev,
if (ret != 0 && ret != 1)
return -EINVAL;
+ pm_runtime_get_sync(dev);
ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_NO_POWER_OFF, ret);
+ pm_runtime_put(dev);
return ret < 0 ? ret : n;
}
Well, you haven't noticed that dev_pm_qos_update_flags() already does
pm_runtime_get_sync()/pm_runtime_put(). :-)
Oh. Yeah. I ignore it. :)
@@ -291,7 +293,9 @@ static ssize_t pm_qos_remote_wakeup_store(struct device *dev,
if (ret != 0 && ret != 1)
return -EINVAL;
+ pm_runtime_get_sync(dev);
ret = dev_pm_qos_update_flags(dev, PM_QOS_FLAG_REMOTE_WAKEUP, ret);
+ pm_runtime_put(dev);
return ret < 0 ? ret : n;
}
Thanks,
Rafael
--
Best Regards
Tianyu Lan
linux kernel enabling team
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
