Messages in this thread | | | Date | Wed, 7 May 2014 20:55:41 -0500 | Subject | Re: [PATCH 1/3] PM / OPP: Add support for descending order for cpufreq table | From | Nishanth Menon <> |
| |
On Wed, May 7, 2014 at 8:22 PM, Jonghwan Choi <jhbird.choi@samsung.com> wrote: >> @Jonghwan: Please consider doing this: >> - Don't play with the order of frequencies in table. >> - Instead initialize .driver_data filed with values that you need to write >> in the registers for all frequencies. i.e. 0 for highest frequency and >> FREQ_COUNT-1 for lowest one. > > -> For that, I changed like this. > For initializing .driver_data, I changed dev_pm_opp_init_cpufreq_table function(). > > > --- a/drivers/base/power/opp.c > +++ b/drivers/base/power/opp.c > @@ -622,12 +622,12 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_disable); > * or in contexts where mutex locking cannot be used. > */ > int dev_pm_opp_init_cpufreq_table(struct device *dev, > - struct cpufreq_frequency_table **table) > + struct cpufreq_frequency_table **table, int order) > { > struct device_opp *dev_opp; > struct dev_pm_opp *opp; > struct cpufreq_frequency_table *freq_table; > - int i = 0; > + int i = 0, index = 0; > > /* Pretend as if I am an updater */ > mutex_lock(&dev_opp_list_lock); > @@ -649,16 +649,22 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev, > return -ENOMEM; > } > > + if (OPP_TABLE_ORDER_DESCENDING == order) > + index = dev_pm_opp_get_opp_count(dev) - 1; > + > list_for_each_entry(opp, &dev_opp->opp_list, node) { > if (opp->available) { > - freq_table[i].driver_data = i; > + if (OPP_TABLE_ORDER_DESCENDING == order) > + freq_table[i].driver_data = index--; > + else > + freq_table[i].driver_data = index++; > freq_table[i].frequency = opp->rate / 1000; > i++; > } > } > mutex_unlock(&dev_opp_list_lock); > > - freq_table[i].driver_data = i; > + freq_table[i].driver_data = index; > freq_table[i].frequency = CPUFREQ_TABLE_END; > > *table = &freq_table[0]; > > > Is it acceptiable?
Personally, I feel that filling up driver_data should be left to the driver(caller of dev_pm_opp_init_cpufreq_table). for example providing a function pointer which decides what that value should be (be it index or some magical register value).. Viresh might have better opinions.
Regards, Nishanth Menon
| |