On 25-05-16, 19:52, Steve Muckle wrote: > Cpufreq governors may need to know what a particular target frequency > maps to in the driver without necessarily wanting to set the frequency. > Support this operation via a new cpufreq API, > cpufreq_driver_resolve_freq(). > > The above API will call a new cpufreq driver callback, resolve_freq(), > if it has been registered by the driver. If that callback has not been > registered and a frequency table is available then the frequency table > is walked using cpufreq_frequency_table_target(). > > UINT_MAX is returned if no driver callback or frequency table is > available.
Why should we return UINT_MAX here? We should return target_freq, no ? > Suggested-by: Rafael J. Wysocki <[email protected]> > Signed-off-by: Steve Muckle <[email protected]> > --- > drivers/cpufreq/cpufreq.c | 25 +++++++++++++++++++++++++ > include/linux/cpufreq.h | 11 +++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 77d77a4e3b74..3b44f4bdc071 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -1849,6 +1849,31 @@ unsigned int cpufreq_driver_fast_switch(struct > cpufreq_policy *policy, > } > EXPORT_SYMBOL_GPL(cpufreq_driver_fast_switch); > > +unsigned int cpufreq_driver_resolve_freq(struct cpufreq_policy *policy, > + unsigned int target_freq) > +{ > + struct cpufreq_frequency_table *freq_table; > + int index, retval; > + > + clamp_val(target_freq, policy->min, policy->max); > + > + if (cpufreq_driver->resolve_freq) > + return cpufreq_driver->resolve_freq(policy, target_freq); > + > + freq_table = cpufreq_frequency_get_table(policy->cpu); I have sent a separate patch to provide a light weight alternative to this. If that gets accepted, we can switch over to using it. > + if (!freq_table) > + return UINT_MAX; > + > + retval = cpufreq_frequency_table_target(policy, freq_table, > + target_freq, CPUFREQ_RELATION_L, > + &index); > + if (retval) > + return UINT_MAX; > + > + return freq_table[index].frequency; > +} > +EXPORT_SYMBOL_GPL(cpufreq_driver_resolve_freq); > + > /* Must set freqs->new to intermediate frequency */ > static int __target_intermediate(struct cpufreq_policy *policy, > struct cpufreq_freqs *freqs, int index) > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h > index 4e81e08db752..675f17f98e75 100644 > --- a/include/linux/cpufreq.h > +++ b/include/linux/cpufreq.h > @@ -271,6 +271,13 @@ struct cpufreq_driver { > int (*target_intermediate)(struct cpufreq_policy *policy, > unsigned int index); > > + /* > + * Return the driver-supported frequency that a particular target > + * frequency maps to (does not set the new frequency). > + */ > + unsigned int (*resolve_freq)(struct cpufreq_policy *policy, > + unsigned int target_freq); We have 3 categories of cpufreq-drivers today: 1. setpolicy drivers: They don't use the cpufreq governors we are working on. 2. non-setpolicy drivers: A. with ->target_index() callback, these will always provide a freq-table. B. with ->target() callback, ONLY these should be allowed to provide the ->resolve_freq() callback and no one else. And so I would suggest adding an additional check in cpufreq_register_driver() to catch incorrect usage of this callback. -- viresh
