| Message ID | 0660b023a0d80c63ec7a1f7fcb692de9a9f4d604.1560944014.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | cpufreq: cleanups | expand |
On Wed, Jun 19, 2019 at 1:36 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > CPUFREQ_CONST_LOOPS was introduced in a very old commit from pre-2.6 > kernel release commit 6a4a93f9c0d5 ("[CPUFREQ] Fix 'out of sync' > issue"). > > Probably the initial idea was to just avoid these checks for set_policy > type drivers and then things got changed over the years. And it is very > unclear why these checks are there at all. > > Replace the CPUFREQ_CONST_LOOPS check with has_target(), which makes > more sense now. > > Also remove () around freq comparison statement as they aren't required > and checkpatch also warns for them. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/cpufreq/cpufreq.c | 13 +++++-------- > 1 file changed, 5 insertions(+), 8 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 54befd775bd6..e59194c2c613 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -359,12 +359,10 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy, > * which is not equal to what the cpufreq core thinks is > * "old frequency". > */ > - if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) { > - if (policy->cur && (policy->cur != freqs->old)) { > - pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n", > - freqs->old, policy->cur); > - freqs->old = policy->cur; > - } > + if (has_target() && policy->cur && policy->cur != freqs->old) { > + pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n", > + freqs->old, policy->cur); > + freqs->old = policy->cur; Is cpufreq_notify_transition() ever called if ->setpolicy drivers are in use? > } > > srcu_notifier_call_chain(&cpufreq_transition_notifier_list, > @@ -1618,8 +1616,7 @@ static unsigned int __cpufreq_get(struct cpufreq_policy *policy) > if (policy->fast_switch_enabled) > return ret_freq; > > - if (ret_freq && policy->cur && > - !(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) { > + if (has_target() && ret_freq && policy->cur) { > /* verify no discrepancy between actual and > saved value exists */ > if (unlikely(ret_freq != policy->cur)) { > -- > 2.21.0.rc0.269.g1a574e7a288b >
On 19-06-19, 14:20, Rafael J. Wysocki wrote: > On Wed, Jun 19, 2019 at 1:36 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > > index 54befd775bd6..e59194c2c613 100644 > > --- a/drivers/cpufreq/cpufreq.c > > +++ b/drivers/cpufreq/cpufreq.c > > @@ -359,12 +359,10 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy, > > * which is not equal to what the cpufreq core thinks is > > * "old frequency". > > */ > > - if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) { > > - if (policy->cur && (policy->cur != freqs->old)) { > > - pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n", > > - freqs->old, policy->cur); > > - freqs->old = policy->cur; > > - } > > + if (has_target() && policy->cur && policy->cur != freqs->old) { > > + pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n", > > + freqs->old, policy->cur); > > + freqs->old = policy->cur; > > Is cpufreq_notify_transition() ever called if ->setpolicy drivers are in use? I tried to find it, but I couldn't find any driver from where we can get this called for setpolicy drivers.
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 54befd775bd6..e59194c2c613 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -359,12 +359,10 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy, * which is not equal to what the cpufreq core thinks is * "old frequency". */ - if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) { - if (policy->cur && (policy->cur != freqs->old)) { - pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n", - freqs->old, policy->cur); - freqs->old = policy->cur; - } + if (has_target() && policy->cur && policy->cur != freqs->old) { + pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n", + freqs->old, policy->cur); + freqs->old = policy->cur; } srcu_notifier_call_chain(&cpufreq_transition_notifier_list, @@ -1618,8 +1616,7 @@ static unsigned int __cpufreq_get(struct cpufreq_policy *policy) if (policy->fast_switch_enabled) return ret_freq; - if (ret_freq && policy->cur && - !(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) { + if (has_target() && ret_freq && policy->cur) { /* verify no discrepancy between actual and saved value exists */ if (unlikely(ret_freq != policy->cur)) {
CPUFREQ_CONST_LOOPS was introduced in a very old commit from pre-2.6 kernel release commit 6a4a93f9c0d5 ("[CPUFREQ] Fix 'out of sync' issue"). Probably the initial idea was to just avoid these checks for set_policy type drivers and then things got changed over the years. And it is very unclear why these checks are there at all. Replace the CPUFREQ_CONST_LOOPS check with has_target(), which makes more sense now. Also remove () around freq comparison statement as they aren't required and checkpatch also warns for them. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/cpufreq.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)