On 05/22/2014 10:24 PM, Viresh Kumar wrote:
> On 22 May 2014 22:07, Stephen Warren <swar...@wwwdotorg.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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
