Messages in this thread | | | Date | Wed, 3 Feb 2016 14:38:47 +0100 | Subject | Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops | From | "Rafael J. Wysocki" <> |
| |
On Wed, Feb 3, 2016 at 2:21 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 03-02-16, 13:42, Rafael J. Wysocki wrote: >> > +static ssize_t governor_show(struct kobject *kobj, struct attribute *attr, >> > + char *buf) >> > +{ >> > + struct dbs_data *dbs_data = to_dbs_data(kobj); >> > + struct governor_attr *gattr = to_gov_attr(attr); >> > + int ret = -EIO; >> > + >> > + down_read(&dbs_data->rwsem); >> > + >> > + if (gattr->show) >> > + ret = gattr->show(dbs_data, buf); >> > + >> > + up_read(&dbs_data->rwsem); >> >> Do we need the lock here too? >> >> show() is only going to read the value, isn't it? And everything u32 >> or smaller is read atomically anyway. > > Okay, will drop it for now. > >> Apart from this it looks good to me. >> >> When you're ready, please resend the whole series without patch [5/5] >> which is premature IMO. > > I have changed that patch a bit, and am dropping just the lock now and > not governor_enable thing. That should be sane enough I believe.
In any case this is not suitable for 4.5 IMO.
> Anyway I will be posting 7 patches now, pick only first 4 if you > aren't confident about the rest.
OK
Thanks, Rafael
| |