On Wed, Feb 3, 2016 at 2:21 PM, Viresh Kumar <viresh.ku...@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