Hi Lucas,

On 24 October 2014 at 18:35, Lucas Stach [off-list ref] wrote:

        if (!IS_ERR(cpu_reg)) {
-               struct dev_pm_opp *opp;
-               unsigned long min_uV, max_uV;
-               int i;
+               unsigned long opp_freq = 0;

                /*
-                * OPP is maintained in order of increasing frequency, and
-                * freq_table initialised from OPP is therefore sorted in the
-                * same order.
+                * Disable any OPPs where the connected regulator isn't able to
+                * provide the specified voltage and record minimum and maximum
+                * voltage levels.
                 */
-               for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++)
-                       ;
-               rcu_read_lock();
-               opp = dev_pm_opp_find_freq_exact(cpu_dev,
-                               freq_table[0].frequency * 1000, true);
-               min_uV = dev_pm_opp_get_voltage(opp);
-               opp = dev_pm_opp_find_freq_exact(cpu_dev,
-                               freq_table[i-1].frequency * 1000, true);
-               max_uV = dev_pm_opp_get_voltage(opp);
-               rcu_read_unlock();
+               while (1) {
+                       struct dev_pm_opp *opp;
+                       unsigned long opp_uV, tol_uV;
+
+                       rcu_read_lock();
+                       opp = dev_pm_opp_find_freq_ceil(cpu_dev, &opp_freq);
+                       if (IS_ERR(opp)) {
+                               rcu_read_unlock();
+                               break;
+                       }
+                       opp_uV = dev_pm_opp_get_voltage(opp);
+                       rcu_read_unlock();
+
+                       tol_uV = opp_uV * priv->voltage_tolerance / 100;
+                       if (regulator_is_supported_voltage(cpu_reg, opp_uV,
+                                                          opp_uV + tol_uV)) {

Sorry for opening an relatively old thread. Shouldn't we instead do this here:

                       if (regulator_is_supported_voltage(cpu_reg,
opp_uV - tol_uV,
                                                          opp_uV + tol_uV)) {

??

Because the minimum voltage we will try to set is: min_uV - tol_uV (as we
do it with help of: regulator_set_voltage_tol())..

+                               if (opp_uV < min_uV)
+                                       min_uV = opp_uV;
+                               if (opp_uV > max_uV)
+                                       max_uV = opp_uV;

Also the min/max _uV we are using here to calculate latency *may* also
include the tolerance voltage ? i.e. min_uV = min_uV - tol_uV and
max_uV = max_uV + tol_volt ??

+                       } else {
+                               dev_pm_opp_disable(cpu_dev, opp_freq);
+                       }
+
+                       opp_freq++;
+               }
+
                ret = regulator_set_voltage_time(cpu_reg, min_uV, max_uV);
                if (ret > 0)
                        transition_latency += ret * 1000;
        }