[linux-next PATCH 6/7] PM / devfreq: allow sysfs governor node to switch governor

[linux-next PATCH 6/7] PM / devfreq: allow sysfs governor node to switch governor

[Nishanth Menon]

This allows us to select governor runtime from the
default configuration without having to rebuild kernel
or the devfreq driver using the sysfs node:
/sys/class/devfreq/.../governor
cat of the governor will return valid governor
and an echo 'governor_name'>governor will switch
governor

Cc: Rajagopal Venkat <rajagopal.venkat@linaro.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Kevin Hilman <khilman@ti.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 Documentation/ABI/testing/sysfs-class-devfreq |    2 +-
 drivers/devfreq/devfreq.c                     |   40 ++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index e672ccb..293f6b2 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -11,7 +11,7 @@ What:		/sys/class/devfreq/.../governor
 Date:		September 2011
 Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
 Description:
-		The /sys/class/devfreq/.../governor shows the name of the
+		The /sys/class/devfreq/.../governor show or set the name of the
 		governor used by the corresponding devfreq object.
 
 What:		/sys/class/devfreq/.../cur_freq
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index bd96e32..8535f8c 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -629,6 +629,44 @@ static ssize_t show_governor(struct device *dev,
 	return sprintf(buf, "%s\n", to_devfreq(dev)->governor->name);
 }
 
+static ssize_t store_governor(struct device *dev, struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct devfreq *df = to_devfreq(dev);
+	int ret = 0;
+	struct devfreq_governor *governor;
+
+	mutex_lock(&devfreq_list_lock);
+	governor = find_devfreq_governor(buf);
+	if (IS_ERR(governor)) {
+		ret = PTR_ERR(governor);
+		goto out;
+	}
+	if (df->governor == governor)
+		goto out;
+
+	if (df->governor) {
+		ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
+		if (ret) {
+			dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
+				 __func__, df->governor->name, ret);
+			goto out;
+		}
+	}
+	df->governor = governor;
+	strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN);
+	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
+	if (ret)
+		dev_warn(dev, "%s: Governor %s not started(%d)\n",
+			 __func__, df->governor->name, ret);
+out:
+	mutex_unlock(&devfreq_list_lock);
+
+	if (!ret)
+		ret = count;
+	return ret;
+}
+
 static ssize_t show_freq(struct device *dev,
 			 struct device_attribute *attr, char *buf)
 {
@@ -773,7 +811,7 @@ static ssize_t show_available_freqs(struct device *d,
 }
 
 static struct device_attribute devfreq_attrs[] = {
-	__ATTR(governor, S_IRUGO, show_governor, NULL),
+	__ATTR(governor, S_IRUGO | S_IWUSR, show_governor, store_governor),
 	__ATTR(cur_freq, S_IRUGO, show_freq, NULL),
 	__ATTR(available_frequencies, S_IRUGO, show_available_freqs, NULL),
 	__ATTR(target_freq, S_IRUGO, show_target_freq, NULL),
-- 
1.7.9.5

Re: [linux-next PATCH 6/7] PM / devfreq: allow sysfs governor node to switch governor

[MyungJoo Ham]

> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -629,6 +629,44 @@ static ssize_t show_governor(struct device *dev,
>  	return sprintf(buf, "%s\n", to_devfreq(dev)->governor->name);
>  }
>  
> +static ssize_t store_governor(struct device *dev, struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct devfreq *df = to_devfreq(dev);
> +	int ret = 0;
> +	struct devfreq_governor *governor;
> +
> +	mutex_lock(&devfreq_list_lock);

Please remove the trailing \n from buf here.
When user enters "userspace", buf gets "userspace\n".

With some printks in find_devfreq_governor (printing the buf and governor names):

# echo userspace > /sys/class/devfreq/exynos4210-busfreq.0/governor
[   65.975000] [userspace
[   65.975000] ].[userspace]
[   65.980000] [userspace
[   65.980000] ].[powersave]
[   65.985000] [userspace
[   65.985000] ].[performance]
[   65.990000] [userspace
[   65.990000] ].[simple_ondemand]
[   65.995000] err no = -19
# 





Cheers,
MyungJoo

> +	governor = find_devfreq_governor(buf);
> +	if (IS_ERR(governor)) {
> +		ret = PTR_ERR(governor);
> +		goto out;
> +	}
> +	if (df->governor == governor)
> +		goto out;
> +
> +	if (df->governor) {
> +		ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
> +		if (ret) {
> +			dev_warn(dev, "%s: Governor %s not stopped(%d)\n",
> +				 __func__, df->governor->name, ret);
> +			goto out;
> +		}
> +	}
> +	df->governor = governor;
> +	strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN);
> +	ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
> +	if (ret)
> +		dev_warn(dev, "%s: Governor %s not started(%d)\n",
> +			 __func__, df->governor->name, ret);
> +out:
> +	mutex_unlock(&devfreq_list_lock);
> +
> +	if (!ret)
> +		ret = count;
> +	return ret;
> +}
> +
[]

Re: [linux-next PATCH 6/7] PM / devfreq: allow sysfs governor node to switch governor

[Nishanth Menon]

On 18:18-20121106, MyungJoo Ham wrote:
> 2012. 11. 6. 오후 6:02에 "MyungJoo Ham" <myungjoo.ham@samsung.com>님이 작성:
> >
> > > --- a/drivers/devfreq/devfreq.c
> > > +++ b/drivers/devfreq/devfreq.c
> > > @@ -629,6 +629,44 @@ static ssize_t show_governor(struct device *dev,
> > >       return sprintf(buf, "%s\n", to_devfreq(dev)->governor->name);
> > >  }
> > >
> > > +static ssize_t store_governor(struct device *dev, struct
> device_attribute *attr,
> > > +                           const char *buf, size_t count)
> > > +{
> > > +     struct devfreq *df = to_devfreq(dev);
> > > +     int ret = 0;
> > > +     struct devfreq_governor *governor;
> > > +
> > > +     mutex_lock(&devfreq_list_lock);
> >
> > Please remove the trailing \n from buf here.
> > When user enters "userspace", buf gets "userspace\n".
> >
> > With some printks in find_devfreq_governor (printing the buf and governor
> names):
> >
> > # echo userspace > /sys/class/devfreq/exynos4210-busfreq.0/governor
> > [   65.975000] [userspace
> > [   65.975000] ].[userspace]
> > [   65.980000] [userspace
> > [   65.980000] ].[powersave]
> > [   65.985000] [userspace
> > [   65.985000] ].[performance]
> > [   65.990000] [userspace
> > [   65.990000] ].[simple_ondemand]
> > [   65.995000] err no = -19
> > #
> >
> 
> Anyway this seems quite wierd. I will look more into this one in my target
> system.
Thanks for catching this.
:( I missed the \n (been using echo -n toooo often now grr..)
What do you think of the following change to the patch?

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 586b6fd..9a4e898 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -614,11 +614,16 @@ static ssize_t store_governor(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
 	struct devfreq *df = to_devfreq(dev);
-	int ret = 0;
+	int ret;
+	char str_governor[DEVFREQ_NAME_LEN + 1];
 	struct devfreq_governor *governor;
 
+	ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor);
+	if (ret != 1)
+	        return -EINVAL;
+
 	mutex_lock(&devfreq_list_lock);
-	governor = find_devfreq_governor(buf);
+	governor = find_devfreq_governor(str_governor);
 	if (IS_ERR(governor)) {
 		ret = PTR_ERR(governor);
 		goto out;
-- 
Regards,
Nishanth Menon

output:
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
simple_ondemand
/sys/devices/platform/iva.0/devfreq/iva.0 # echo -n "userspace">governor 
/sys/devices/platform/iva.0/devfreq/iva.0 # echo -n "simple_ondeman">governor 
sh: write error: No such device
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
userspace
/sys/devices/platform/iva.0/devfreq/iva.0 # echo -n "simple_ondemand">governor 
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
simple_ondemand
/sys/devices/platform/iva.0/devfreq/iva.0 # echo "userspace">governor 
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
userspace
/sys/devices/platform/iva.0/devfreq/iva.0 # echo  "simple_ondemand">governor 
/sys/devices/platform/iva.0/devfreq/iva.0 # echo "userspace123123213123213123212
3123123131231">governor 
sh: write error: No such device
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
simple_ondemand