I believe that 3 item is required for DVFS. Those are frequency, voltage, divider value. Currently OPP only supports voltage and frequency. So some cpufreq and devfreq driver get a divider value from struct divider table.
How about adding that divider value into struct dev_pm_opp like this; struct dev_pm_opp { struct list_head node; bool available; unsigned long rate; unsigned long u_volt; unsigned int ctl[2]; // Added struct device_opp *dev_opp; struct rcu_head head; }; In my test, it works very wel.. I got a this idea from _PCT in acpi spec. Then we can remove a lot of code related to divide table. And we also can solve this problem. Thanks Best Regarfs. > -----Original Message----- > From: menon.nisha...@gmail.com [mailto:menon.nisha...@gmail.com] On Behalf > Of Nishanth Menon > Sent: Thursday, May 08, 2014 10:56 AM > To: Jonghwan Choi > Cc: Viresh Kumar; Linux PM list; open list; Rafael J. Wysocki; Len Brown; > Amit Daniel Kachhap > Subject: Re: [PATCH 1/3] PM / OPP: Add support for descending order for > cpufreq table > > On Wed, May 7, 2014 at 8:22 PM, Jonghwan Choi <jhbird.c...@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 -- 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/