Re: SYSFS "errors"

[Felipe Balbi]

On Mon, Feb 18, 2013 at 04:48:06PM -0500, Alan Stern wrote:
> On Mon, 18 Feb 2013, Felipe Balbi wrote:
> 
> > Hi, On Mon, Feb 18, 2013 at 09:49:16AM -0800, Greg KH wrote:
> > > > Input/output error - /sys/devices/cpu/power/autosuspend_delay_ms
> > > 
> > > The issue with this file is, if the power.use_autosuspend flag is not
> > > set for the device, then it can't be read or written to.  This flag
> > > changes dynamically with the system state
> > > (__pm_runtime_use_autosuspend() can change it), so we can't just not
> > > show the file if the flag is not set properly, sorry.
> > > 
> > > So the "error" is correct here, as is the 0644 file value.
> > 
> > hmm... we could create the file at pm_runtime_enable() time and remove
> > it on pm_runtime_disable() time, no ? Addin Rafael to Cc
> 
> In theory this could be done, although the times would be when runtime 
> autosuspend is turned on or off, not when pm_runtime_enable is called.

Right, I got that after replying. Here's a patch I plan to test
tomorrow:

8<------------------------ cut here ----------------------------

From 2cdeeb483383490946cc98727a167843c40d7d27 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <balbi@xxxxxx>
Date: Mon, 18 Feb 2013 21:20:28 +0200
Subject: [PATCH] base: power: create autosuspend_delay_ms from
 pm_runtime_use_autosuspend()

this patch moves the creation of autosuspend_delay_ms
sysfs file to pm_runtime_use_autosuspend() and, conversely,
its deletion to pm_runtime_dont_use_autosuspend.

The idea behind this patch is such that the file in
question is only readable after pm_runtime_use_autosuspend()
has been called for a particular device, so we might
as well move the creation of the file to that place.

Signed-off-by: Felipe Balbi <balbi@xxxxxx>
---
 drivers/base/power/power.h   |  4 ++++
 drivers/base/power/runtime.c |  6 ++++++
 drivers/base/power/sysfs.c   | 25 ++++++++++++++++++++++++-
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index b16686a..3822b94 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -90,6 +90,8 @@ static inline void device_pm_init(struct device *dev)
 
 extern int dpm_sysfs_add(struct device *dev);
 extern void dpm_sysfs_remove(struct device *dev);
+extern int dpm_autosuspend_sysfs_add(struct device *dev);
+extern void dpm_autosuspend_sysfs_remove(struct device *dev);
 extern void rpm_sysfs_remove(struct device *dev);
 extern int wakeup_sysfs_add(struct device *dev);
 extern void wakeup_sysfs_remove(struct device *dev);
@@ -102,6 +104,8 @@ extern void pm_qos_sysfs_remove_flags(struct device *dev);
 
 static inline int dpm_sysfs_add(struct device *dev) { return 0; }
 static inline void dpm_sysfs_remove(struct device *dev) {}
+static inline int dpm_autosuspend_sysfs_add(struct device *dev) { return 0; }
+static inline void dpm_autosuspend_sysfs_remove(struct device *dev) {}
 static inline void rpm_sysfs_remove(struct device *dev) {}
 static inline int wakeup_sysfs_add(struct device *dev) { return 0; }
 static inline void wakeup_sysfs_remove(struct device *dev) {}
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 3148b10..a162ba5 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1266,6 +1266,12 @@ void __pm_runtime_use_autosuspend(struct device *dev, bool use)
 	old_use = dev->power.use_autosuspend;
 	dev->power.use_autosuspend = use;
 	update_autosuspend(dev, old_delay, old_use);
+
+	if (use)
+		dpm_autosuspend_sysfs_add(dev);
+	else
+		dpm_autosuspend_sysfs_remove(dev);
+
 	spin_unlock_irq(&dev->power.lock);
 }
 EXPORT_SYMBOL_GPL(__pm_runtime_use_autosuspend);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 50d16e3..ef2a41c 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -609,15 +609,26 @@ static struct attribute *runtime_attrs[] = {
 	&dev_attr_control.attr,
 	&dev_attr_runtime_suspended_time.attr,
 	&dev_attr_runtime_active_time.attr,
-	&dev_attr_autosuspend_delay_ms.attr,
 #endif /* CONFIG_PM_RUNTIME */
 	NULL,
 };
+
 static struct attribute_group pm_runtime_attr_group = {
 	.name	= power_group_name,
 	.attrs	= runtime_attrs,
 };
 
+static struct attribute *runtime_autosuspend_attrs[] = {
+#ifdef CONFIG_PM_RUNTIME
+	&dev_attr_autosuspend_delay_ms.attr,
+#endif /* CONFIG_PM_RUNTIME */
+};
+
+static struct attribute_group pm_runtime_autosuspend_attr_group = {
+	.name	= power_group_name,
+	.attrs	= runtime_autosuspend_attrs,
+};
+
 static struct attribute *pm_qos_latency_attrs[] = {
 #ifdef CONFIG_PM_RUNTIME
 	&dev_attr_pm_qos_resume_latency_us.attr,
@@ -671,6 +682,12 @@ int dpm_sysfs_add(struct device *dev)
 	return rc;
 }
 
+int dpm_autosuspend_sysfs_add(struct device *dev)
+{
+	return sysfs_merge_group(&dev->kobj,
+			&pm_runtime_autosuspend_attr_group);
+}
+
 int wakeup_sysfs_add(struct device *dev)
 {
 	return sysfs_merge_group(&dev->kobj, &pm_wakeup_attr_group);
@@ -712,3 +729,9 @@ void dpm_sysfs_remove(struct device *dev)
 	sysfs_unmerge_group(&dev->kobj, &pm_wakeup_attr_group);
 	sysfs_remove_group(&dev->kobj, &pm_attr_group);
 }
+
+void dpm_autosuspend_sysfs_remove(struct device *dev)
+{
+	sysfs_unmerge_group(&dev->kobj,
+			&pm_runtime_autosuspend_attr_group);
+}
-- 
1.8.1.rc1.5.g7e0651a

-- 
balbi

Attachment: signature.asc
Description: Digital signature