Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1448182 > unrolled thread

[PATCH] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index()

Started byViresh Kumar <viresh.kumar@linaro.org>
First post2016-07-21 23:30 +0200
Last post2016-07-21 23:40 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index() Viresh Kumar <viresh.kumar@linaro.org> - 2016-07-21 23:30 +0200
    Re: [PATCH] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index() "Rafael J. Wysocki" <rafael@kernel.org> - 2016-07-21 23:40 +0200

#1448182 — [PATCH] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index()

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-07-21 23:30 +0200
Subject[PATCH] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index()
Message-ID<rXtHA-4Eb-37@gated-at.bofh.it>
The handlers provided by cpufreq core are sufficient for resolving the
frequency for drivers providing ->target_index(), as the core already
has the frequency table and so ->resolve_freq() isn't required for such
platforms.

This patch disallows drivers with ->target_index() callback to use the
->resolve_freq() callback.

Also, it fixes a potential kernel crash for drivers providing ->target()
but no ->resolve_freq().

Fixes: e3c062360870 ("cpufreq: add cpufreq_driver_resolve_freq()")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b696baeb249d..342bbee8d0e1 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -507,12 +507,20 @@ unsigned int cpufreq_driver_resolve_freq(struct cpufreq_policy *policy,
 {
 	target_freq = clamp_val(target_freq, policy->min, policy->max);
 	policy->cached_target_freq = target_freq;
+
+	if (cpufreq_driver->target_index) {
+		int idx;
+
+		idx = cpufreq_frequency_table_target(policy, target_freq,
+						     CPUFREQ_RELATION_L);
+		policy->cached_resolved_idx = idx;
+		return policy->freq_table[idx].frequency;
+	}
+
 	if (cpufreq_driver->resolve_freq)
 		return cpufreq_driver->resolve_freq(policy, target_freq);
-	policy->cached_resolved_idx =
-		cpufreq_frequency_table_target(policy, target_freq,
-					       CPUFREQ_RELATION_L);
-	return policy->freq_table[policy->cached_resolved_idx].frequency;
+
+	return UINT_MAX;
 }
 
 /*********************************************************************
-- 
2.7.4

[toc] | [next] | [standalone]


#1448193

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-07-21 23:40 +0200
Message-ID<rXtRf-4HJ-7@gated-at.bofh.it>
In reply to#1448182
On Thu, Jul 21, 2016 at 11:23 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> The handlers provided by cpufreq core are sufficient for resolving the
> frequency for drivers providing ->target_index(), as the core already
> has the frequency table and so ->resolve_freq() isn't required for such
> platforms.
>
> This patch disallows drivers with ->target_index() callback to use the
> ->resolve_freq() callback.
>
> Also, it fixes a potential kernel crash for drivers providing ->target()
> but no ->resolve_freq().
>
> Fixes: e3c062360870 ("cpufreq: add cpufreq_driver_resolve_freq()")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index b696baeb249d..342bbee8d0e1 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -507,12 +507,20 @@ unsigned int cpufreq_driver_resolve_freq(struct cpufreq_policy *policy,
>  {
>         target_freq = clamp_val(target_freq, policy->min, policy->max);
>         policy->cached_target_freq = target_freq;
> +
> +       if (cpufreq_driver->target_index) {
> +               int idx;
> +
> +               idx = cpufreq_frequency_table_target(policy, target_freq,
> +                                                    CPUFREQ_RELATION_L);
> +               policy->cached_resolved_idx = idx;
> +               return policy->freq_table[idx].frequency;
> +       }
> +
>         if (cpufreq_driver->resolve_freq)
>                 return cpufreq_driver->resolve_freq(policy, target_freq);
> -       policy->cached_resolved_idx =
> -               cpufreq_frequency_table_target(policy, target_freq,
> -                                              CPUFREQ_RELATION_L);
> -       return policy->freq_table[policy->cached_resolved_idx].frequency;
> +
> +       return UINT_MAX;

I think it should just return target_freq in this case or the UINT_MAX
will confuse things in schedutil.

Thanks,
Rafael

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web