linux-kernel - Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable) frequencies

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 23 May 2014 09:56:04 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Viresh Kumar <viresh.kumar@...aro.org>
CC:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Lists linaro-kernel <linaro-kernel@...ts.linaro.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Arvind Chauhan <arvind.chauhan@....com>,
	Stephen Warren <swarren@...dia.com>,
	Doug Anderson <dianders@...omium.org>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Nicolas Pitre <nicolas.pitre@...aro.org>,
	Thomas Abraham <thomas.abraham@...aro.org>,
	Peter De Schrijver <pdeschrijver@...dia.com>
Subject: Re: [PATCH V4 2/3] cpufreq: add support for intermediate (stable)
 frequencies

On 05/22/2014 10:24 PM, Viresh Kumar wrote:
> On 22 May 2014 22:07, Stephen Warren <swarren@...dotorg.org> wrote:
>> It seems rather odd to set both freqs->old and freqs->new to the
>> intermediate frequency upon success. It feels like it would make more
>> sense to remove that final else clause above, and do the following where
>> this function is called:

>>>  static int __target_index(struct cpufreq_policy *policy,
>>>                         struct cpufreq_frequency_table *freq_table, int index)
>>>  {
>>> -     struct cpufreq_freqs freqs;
>>> +     struct cpufreq_freqs freqs = {.old = policy->cur, .flags = 0};
>>> +     unsigned int intermediate_freq = 0;
>>>       int retval = -EINVAL;
>>>       bool notify;
>>>
>>>       notify = !(cpufreq_driver->flags & CPUFREQ_ASYNC_NOTIFICATION);
>>> -
>>>       if (notify) {
>>> -             freqs.old = policy->cur;
>>> -             freqs.new = freq_table[index].frequency;
>>> -             freqs.flags = 0;
>>> +             /* Handle switching to intermediate frequency */
>>> +             if (cpufreq_driver->get_intermediate) {
>>> +                     retval = __target_intermediate(policy, &freqs, index);
>>> +                     if (retval)
>>> +                             return retval;
>>
>> Shouldn't this all be outside the if (notify) block, so that
>> __target_intermediate is always called, and it's just the notify
>> callbacks that gets skipped if (!notify)?
> 
> So, this is logic I had:
> 
> Should we support 'intermediate freq' infrastructure when driver wants
> to handle notifications themselves?
> 
> Probably not.
> 
> The whole point of implementing this 'intermediate freq' infrastructure is to
> get rid of code redundancy while sending notifications. If drivers want to
> handle notifications then they better handle intermediate freqs as well in
> their target_index() callback. They don't need to implement another routine
> for intermediate stuff..

Oh OK, I guess the "notify" value is static then, and driver defined, so
this is fine.

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