On Fri, Nov 8, 2013 at 8:16 PM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote: > On 8 November 2013 23:13, Stratos Karafotis <skarafotis@xxxxxxxxx> wrote: >> Please let me rephrase my previous post. In some circumstances (depending >> on freq_step and freq_table values) CPU frequency will never reach to >> policy->max. >> >> For example suppose that (for simplicity values in MHz): >> policy->max = 1000 >> policy->cur = 800 >> requested_freq = 800 >> freq_target = 300 >> >> In 'first' iteration, if we return early with this code (because >> requested_freq will be >> 1100): >> if (dbs_info->requested_freq >= policy->max) >> return; > > That's not correct. At this point requested_freq would have been > 800 only, and would have increased after this instruction to 1100. > So, in the first transition we will go to max freq, but not from the > second. > > Though this piece of code is more simplified by the new solution > I gave. > Yes, you are right. >> CPU freq will never go over 800MHz. >> >> I think the current code works correctly. >> - The requested freq will go to 1100 in first iteration. >> - __cpufreq_driver_target will change CPU freq to 1000 >> - dbs_cpufreq_notifier will adjust the requested_freq to 1000 > >> So, I think there is no need for an extra check because of >> dbs_cpufreq_notifier code. > > Now with the new code in place we are correcting requested_freq > in cs_check_cpu(), then why do we need dbs_cpufreq_notifier()? > > What do you think? I removed the check you proposed in this commit 934dac1ea072 to avoid the duplicate check in cs_check_cpu and in dbs_cpufreq_notifier. I agree that we don't need dbs_cpufreq_notifier if we transfer checks in cs_check_cpu. But I'm not 100% sure if the notifier also covers other cases and if it can be safely removed. Stratos Karafotis -- To unsubscribe from this list: send the line "unsubscribe cpufreq" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html