Re: [PATCH v4 2/7] cpufreq: Add boost frequency support in core

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Wednesday, July 17, 2013 01:28:29 PM Viresh Kumar wrote:
> On 20 June 2013 03:55, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Wednesday, June 19, 2013 10:31:02 PM Lukasz Majewski wrote:
> >> On Wed, 19 Jun 2013 10:48:53 -0700
> >> Dirk Brandewie <dirk.brandewie@gmail.com> wrote:
> >>
> >> > On 06/19/2013 10:12 AM, Lukasz Majewski wrote:
> 
> >> > > @@ -1936,6 +2019,16 @@ int cpufreq_register_driver(struct
> >> > > + if (!cpufreq_driver->boost_supported)
> >> > > +         boost.attr.mode = 0444;
> >> > > +
> >> > > + ret = cpufreq_sysfs_create_file(&(boost.attr));
> >> > > + if (ret) {
> >> > > +         pr_err("%s: cannot register global boost sysfs
> >> > > file\n",
> >> > > +                __func__);
> >> > > +         goto err_null_driver;
> >> > > + }
> >> > > +
> >> >
> >> > I do not think the boost sysfs should be created at all if boost is
> >> > not supported.
> >>
> >> This was my first thought. But unfortunately this "boost" attribute is
> >> always exported at acpi-cpufreq.c and in my opinion is part of a
> >> legacy API.
> >>
> >> I totally agree with the idea of exporting boost only when supported,
> >> but I would like to know the community opinion about this (especially
> >> Viresh and Rafael shall speak up).
> >
> > Simple: Export it only when supported.
> 
> Guys,
> 
> I got confused here. We originally decided to keep this feature as is
> for acpi-cpufreq.
> 
> So, For acpi-cpufreq:
> - when boost isn't supported: create sysfs boost with ro permissions
> - when boost is supported: create sysfs boost with rw permissions
> 
> So, For others:
> - create sysfs boost with rw permissions only when boost is supported
> 
> .
> 
> Do you want something else? or do you want to change behavior of
> acpi-cpufreq driver? I don't why it was designed this way and what
> applications use it.

First off, I'm not sure how many applications actually use it and I think,
if any, they should be able cope with the attribute not being present.

Of course, if it turns out that yes, there are applications using it and no,
they cannot cope with the missing attribute, we'll need to address this.
That said such applications wouldn't work with earlier kernels in which that
attribute wasn't present at all, so I suppose this is really unlikely.

So, do whichever makes more sense to you: Design things to preserve the old
behavior (which is sightly confusing) or design them to expose the attribute
if the feature is actually supported and be prepared to address the (unlikely)
case when some hypothetical applications break because of that.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.