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

[Nishanth Menon]

On 18:18-20121106, MyungJoo Ham wrote:
> 2012. 11. 6. ìí 6:02ì "MyungJoo Ham" <myungjoo.ham@xxxxxxxxxxx>ëì ìì:
> >
> > > --- 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/