Messages in this thread | | | Date | Fri, 23 May 2014 09:56:04 -0600 | From | Stephen Warren <> | 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@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.
| |