[PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target()

[PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target()

[Viresh Kumar]

Cpufreq core checks the range of target_freq before calling driver->target() and
so we don't need to do it again.

Remove it.

Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/mach-davinci/cpufreq.c  | 9 ---------
 drivers/cpufreq/dbx500-cpufreq.c | 6 ------
 2 files changed, 15 deletions(-)

diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
index 55eb870..8fb0c2a 100644
--- a/arch/arm/mach-davinci/cpufreq.c
+++ b/arch/arm/mach-davinci/cpufreq.c
@@ -79,15 +79,6 @@ static int davinci_target(struct cpufreq_policy *policy,
 	struct davinci_cpufreq_config *pdata = cpufreq.dev->platform_data;
 	struct clk *armclk = cpufreq.armclk;
 
-	/*
-	 * Ensure desired rate is within allowed range.  Some govenors
-	 * (ondemand) will just pass target_freq=0 to get the minimum.
-	 */
-	if (target_freq < policy->cpuinfo.min_freq)
-		target_freq = policy->cpuinfo.min_freq;
-	if (target_freq > policy->cpuinfo.max_freq)
-		target_freq = policy->cpuinfo.max_freq;
-
 	freqs.old = davinci_getspeed(0);
 	freqs.new = clk_round_rate(armclk, target_freq * 1000) / 1000;
 
diff --git a/drivers/cpufreq/dbx500-cpufreq.c b/drivers/cpufreq/dbx500-cpufreq.c
index 7192a6d..15ed367 100644
--- a/drivers/cpufreq/dbx500-cpufreq.c
+++ b/drivers/cpufreq/dbx500-cpufreq.c
@@ -37,12 +37,6 @@ static int dbx500_cpufreq_target(struct cpufreq_policy *policy,
 	unsigned int idx;
 	int ret;
 
-	/* scale the target frequency to one of the extremes supported */
-	if (target_freq < policy->cpuinfo.min_freq)
-		target_freq = policy->cpuinfo.min_freq;
-	if (target_freq > policy->cpuinfo.max_freq)
-		target_freq = policy->cpuinfo.max_freq;
-
 	/* Lookup the next frequency */
 	if (cpufreq_frequency_table_target(policy, freq_table, target_freq,
 					relation, &idx))
-- 
1.7.12.rc2.18.g61b472e

[PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Viresh Kumar]

Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
aren't required as part of it is done by cpufreq driver or cpufreq core.

Remove them.

At some places we merge multiple lines together too.

Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: linux-sh@vger.kernel.org
Cc: linux-omap@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/cpu-freq/cpu-drivers.txt | 5 +++--
 arch/arm/mach-davinci/cpufreq.c        | 4 ----
 arch/arm/mach-imx/cpufreq.c            | 3 ---
 arch/sh/kernel/cpufreq.c               | 9 +++------
 drivers/cpufreq/cpufreq-nforce2.c      | 6 ++----
 drivers/cpufreq/omap-cpufreq.c         | 4 +---
 6 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
index 72f70b1..c94383f 100644
--- a/Documentation/cpu-freq/cpu-drivers.txt
+++ b/Documentation/cpu-freq/cpu-drivers.txt
@@ -108,8 +108,9 @@ policy->governor		must contain the "default policy" for
 				cpufreq_driver.target is called with
 				these values.
 
-For setting some of these values, the frequency table helpers might be
-helpful. See the section 2 for more information on them.
+For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
+frequency table helpers might be helpful. See the section 2 for more information
+on them.
 
 SMP systems normally have same clock source for a group of cpus. For these the
 .init() would be called only once for the first online cpu. Here the .init()
diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
index 8fb0c2a..ff46862 100644
--- a/arch/arm/mach-davinci/cpufreq.c
+++ b/arch/arm/mach-davinci/cpufreq.c
@@ -149,10 +149,6 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
 		policy->cpuinfo.max_freq = policy->max;
 	}
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-	policy->cur = davinci_getspeed(0);
-
 	/*
 	 * Time measurement across the target() function yields ~1500-1800us
 	 * time taken with no drivers on notification list.
diff --git a/arch/arm/mach-imx/cpufreq.c b/arch/arm/mach-imx/cpufreq.c
index cfce5e3..387dc4c 100644
--- a/arch/arm/mach-imx/cpufreq.c
+++ b/arch/arm/mach-imx/cpufreq.c
@@ -144,14 +144,11 @@ static int mxc_cpufreq_init(struct cpufreq_policy *policy)
 	imx_freq_table[i].frequency = CPUFREQ_TABLE_END;
 
 	policy->cur = clk_get_rate(cpu_clk) / 1000;
-	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
-	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
 
 	/* Manual states, that PLL stabilizes in two CLK32 periods */
 	policy->cpuinfo.transition_latency = 2 * NANOSECOND / CLK32_FREQ;
 
 	ret = cpufreq_frequency_table_cpuinfo(policy, imx_freq_table);
-
 	if (ret < 0) {
 		printk(KERN_ERR "%s: failed to register i.MXC CPUfreq with error code %d\n",
 		       __func__, ret);
diff --git a/arch/sh/kernel/cpufreq.c b/arch/sh/kernel/cpufreq.c
index 0fdf64b..88c8fee 100644
--- a/arch/sh/kernel/cpufreq.c
+++ b/arch/sh/kernel/cpufreq.c
@@ -116,7 +116,7 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		return PTR_ERR(cpuclk);
 	}
 
-	policy->cur = policy->min = policy->max = sh_cpufreq_get(cpu);
+	policy->cur = sh_cpufreq_get(cpu);
 
 	freq_table = cpuclk->nr_freqs ? cpuclk->freq_table : NULL;
 	if (freq_table) {
@@ -129,15 +129,12 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		dev_notice(dev, "no frequency table found, falling back "
 			   "to rate rounding.\n");
 
-		policy->cpuinfo.min_freq =
+		policy->min = policy->cpuinfo.min_freq =
 			(clk_round_rate(cpuclk, 1) + 500) / 1000;
-		policy->cpuinfo.max_freq =
+		policy->max = policy->cpuinfo.max_freq =
 			(clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
 	}
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 
 	dev_info(dev, "CPU Frequencies - Minimum %u.%03u MHz, "
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index 224a478..af1542d 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -359,12 +359,10 @@ static int nforce2_cpu_init(struct cpufreq_policy *policy)
 		min_fsb = NFORCE2_MIN_FSB;
 
 	/* cpuinfo and default policy values */
-	policy->cpuinfo.min_freq = min_fsb * fid * 100;
-	policy->cpuinfo.max_freq = max_fsb * fid * 100;
+	policy->min = policy->cpuinfo.min_freq = min_fsb * fid * 100;
+	policy->max = policy->cpuinfo.max_freq = max_fsb * fid * 100;
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 	policy->cur = nforce2_get(policy->cpu);
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
 
 	return 0;
 }
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index b610edd..ad7549c 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -177,7 +177,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 		goto fail_ck;
 	}
 
-	policy->cur = policy->min = policy->max = omap_getspeed(policy->cpu);
+	policy->cur = omap_getspeed(policy->cpu);
 
 	if (!freq_table)
 		result = opp_init_cpufreq_table(mpu_dev, &freq_table);
@@ -196,8 +196,6 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 
 	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
 	policy->cur = omap_getspeed(policy->cpu);
 
 	/*
-- 
1.7.12.rc2.18.g61b472e

[PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Viresh Kumar]

Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
aren't required as part of it is done by cpufreq driver or cpufreq core.

Remove them.

At some places we merge multiple lines together too.

Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: linux-sh@vger.kernel.org
Cc: linux-omap@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/cpu-freq/cpu-drivers.txt | 5 +++--
 arch/arm/mach-davinci/cpufreq.c        | 4 ----
 arch/arm/mach-imx/cpufreq.c            | 3 ---
 arch/sh/kernel/cpufreq.c               | 9 +++------
 drivers/cpufreq/cpufreq-nforce2.c      | 6 ++----
 drivers/cpufreq/omap-cpufreq.c         | 4 +---
 6 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
index 72f70b1..c94383f 100644
--- a/Documentation/cpu-freq/cpu-drivers.txt
+++ b/Documentation/cpu-freq/cpu-drivers.txt
@@ -108,8 +108,9 @@ policy->governor		must contain the "default policy" for
 				cpufreq_driver.target is called with
 				these values.
 
-For setting some of these values, the frequency table helpers might be
-helpful. See the section 2 for more information on them.
+For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
+frequency table helpers might be helpful. See the section 2 for more information
+on them.
 
 SMP systems normally have same clock source for a group of cpus. For these the
 .init() would be called only once for the first online cpu. Here the .init()
diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
index 8fb0c2a..ff46862 100644
--- a/arch/arm/mach-davinci/cpufreq.c
+++ b/arch/arm/mach-davinci/cpufreq.c
@@ -149,10 +149,6 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
 		policy->cpuinfo.max_freq = policy->max;
 	}
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-	policy->cur = davinci_getspeed(0);
-
 	/*
 	 * Time measurement across the target() function yields ~1500-1800us
 	 * time taken with no drivers on notification list.
diff --git a/arch/arm/mach-imx/cpufreq.c b/arch/arm/mach-imx/cpufreq.c
index cfce5e3..387dc4c 100644
--- a/arch/arm/mach-imx/cpufreq.c
+++ b/arch/arm/mach-imx/cpufreq.c
@@ -144,14 +144,11 @@ static int mxc_cpufreq_init(struct cpufreq_policy *policy)
 	imx_freq_table[i].frequency = CPUFREQ_TABLE_END;
 
 	policy->cur = clk_get_rate(cpu_clk) / 1000;
-	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
-	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
 
 	/* Manual states, that PLL stabilizes in two CLK32 periods */
 	policy->cpuinfo.transition_latency = 2 * NANOSECOND / CLK32_FREQ;
 
 	ret = cpufreq_frequency_table_cpuinfo(policy, imx_freq_table);
-
 	if (ret < 0) {
 		printk(KERN_ERR "%s: failed to register i.MXC CPUfreq with error code %d\n",
 		       __func__, ret);
diff --git a/arch/sh/kernel/cpufreq.c b/arch/sh/kernel/cpufreq.c
index 0fdf64b..88c8fee 100644
--- a/arch/sh/kernel/cpufreq.c
+++ b/arch/sh/kernel/cpufreq.c
@@ -116,7 +116,7 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		return PTR_ERR(cpuclk);
 	}
 
-	policy->cur = policy->min = policy->max = sh_cpufreq_get(cpu);
+	policy->cur = sh_cpufreq_get(cpu);
 
 	freq_table = cpuclk->nr_freqs ? cpuclk->freq_table : NULL;
 	if (freq_table) {
@@ -129,15 +129,12 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		dev_notice(dev, "no frequency table found, falling back "
 			   "to rate rounding.\n");
 
-		policy->cpuinfo.min_freq +		policy->min = policy->cpuinfo.min_freq  			(clk_round_rate(cpuclk, 1) + 500) / 1000;
-		policy->cpuinfo.max_freq +		policy->max = policy->cpuinfo.max_freq  			(clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
 	}
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 
 	dev_info(dev, "CPU Frequencies - Minimum %u.%03u MHz, "
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index 224a478..af1542d 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -359,12 +359,10 @@ static int nforce2_cpu_init(struct cpufreq_policy *policy)
 		min_fsb = NFORCE2_MIN_FSB;
 
 	/* cpuinfo and default policy values */
-	policy->cpuinfo.min_freq = min_fsb * fid * 100;
-	policy->cpuinfo.max_freq = max_fsb * fid * 100;
+	policy->min = policy->cpuinfo.min_freq = min_fsb * fid * 100;
+	policy->max = policy->cpuinfo.max_freq = max_fsb * fid * 100;
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 	policy->cur = nforce2_get(policy->cpu);
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
 
 	return 0;
 }
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index b610edd..ad7549c 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -177,7 +177,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 		goto fail_ck;
 	}
 
-	policy->cur = policy->min = policy->max = omap_getspeed(policy->cpu);
+	policy->cur = omap_getspeed(policy->cpu);
 
 	if (!freq_table)
 		result = opp_init_cpufreq_table(mpu_dev, &freq_table);
@@ -196,8 +196,6 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 
 	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
 	policy->cur = omap_getspeed(policy->cpu);
 
 	/*
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/24/2013 8:59 PM, Viresh Kumar wrote:
> Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
> aren't required as part of it is done by cpufreq driver or cpufreq core.
> 
> Remove them.
> 
> At some places we merge multiple lines together too.
> 
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: linux-sh@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/cpu-freq/cpu-drivers.txt | 5 +++--
>  arch/arm/mach-davinci/cpufreq.c        | 4 ----
>  arch/arm/mach-imx/cpufreq.c            | 3 ---
>  arch/sh/kernel/cpufreq.c               | 9 +++------
>  drivers/cpufreq/cpufreq-nforce2.c      | 6 ++----
>  drivers/cpufreq/omap-cpufreq.c         | 4 +---
>  6 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
> index 72f70b1..c94383f 100644
> --- a/Documentation/cpu-freq/cpu-drivers.txt
> +++ b/Documentation/cpu-freq/cpu-drivers.txt
> @@ -108,8 +108,9 @@ policy->governor		must contain the "default policy" for
>  				cpufreq_driver.target is called with
>  				these values.
>  
> -For setting some of these values, the frequency table helpers might be
> -helpful. See the section 2 for more information on them.
> +For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
> +frequency table helpers might be helpful. See the section 2 for more information
> +on them.
>  
>  SMP systems normally have same clock source for a group of cpus. For these the
>  .init() would be called only once for the first online cpu. Here the .init()
> diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
> index 8fb0c2a..ff46862 100644
> --- a/arch/arm/mach-davinci/cpufreq.c
> +++ b/arch/arm/mach-davinci/cpufreq.c
> @@ -149,10 +149,6 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
>  		policy->cpuinfo.max_freq = policy->max;
>  	}
>  
> -	policy->min = policy->cpuinfo.min_freq;
> -	policy->max = policy->cpuinfo.max_freq;
> -	policy->cur = davinci_getspeed(0);

There is a line in the code a little above the ones you deleted that
also sets these same variables. I guess you were relying on that line to
set policy->cur, but that also sets policy->{min, max} which can be
cleaned up.

Thanks,
Sekhar

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/24/2013 8:59 PM, Viresh Kumar wrote:
> Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
> aren't required as part of it is done by cpufreq driver or cpufreq core.
> 
> Remove them.
> 
> At some places we merge multiple lines together too.
> 
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: linux-sh@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/cpu-freq/cpu-drivers.txt | 5 +++--
>  arch/arm/mach-davinci/cpufreq.c        | 4 ----
>  arch/arm/mach-imx/cpufreq.c            | 3 ---
>  arch/sh/kernel/cpufreq.c               | 9 +++------
>  drivers/cpufreq/cpufreq-nforce2.c      | 6 ++----
>  drivers/cpufreq/omap-cpufreq.c         | 4 +---
>  6 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
> index 72f70b1..c94383f 100644
> --- a/Documentation/cpu-freq/cpu-drivers.txt
> +++ b/Documentation/cpu-freq/cpu-drivers.txt
> @@ -108,8 +108,9 @@ policy->governor		must contain the "default policy" for
>  				cpufreq_driver.target is called with
>  				these values.
>  
> -For setting some of these values, the frequency table helpers might be
> -helpful. See the section 2 for more information on them.
> +For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
> +frequency table helpers might be helpful. See the section 2 for more information
> +on them.
>  
>  SMP systems normally have same clock source for a group of cpus. For these the
>  .init() would be called only once for the first online cpu. Here the .init()
> diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
> index 8fb0c2a..ff46862 100644
> --- a/arch/arm/mach-davinci/cpufreq.c
> +++ b/arch/arm/mach-davinci/cpufreq.c
> @@ -149,10 +149,6 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
>  		policy->cpuinfo.max_freq = policy->max;
>  	}
>  
> -	policy->min = policy->cpuinfo.min_freq;
> -	policy->max = policy->cpuinfo.max_freq;
> -	policy->cur = davinci_getspeed(0);

There is a line in the code a little above the ones you deleted that
also sets these same variables. I guess you were relying on that line to
set policy->cur, but that also sets policy->{min, max} which can be
cleaned up.

Thanks,
Sekhar

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Viresh Kumar]

On 25 March 2013 14:06, Sekhar Nori <nsekhar@ti.com> wrote:
> There is a line in the code a little above the ones you deleted that
> also sets these same variables. I guess you were relying on that line to
> set policy->cur, but that also sets policy->{min, max} which can be
> cleaned up.

This code is rather confusing or wrong, this was the state of code before
this patch:

	policy->cur = policy->min = policy->max = davinci_getspeed(0);

	if (freq_table) {
		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
		if (!result)
			cpufreq_frequency_table_get_attr(freq_table,
							policy->cpu);
	} else {
		policy->cpuinfo.min_freq = policy->min;
		policy->cpuinfo.max_freq = policy->max;
	}

        policy->min = policy->cpuinfo.min_freq;
        policy->max = policy->cpuinfo.max_freq;
        policy->cur = davinci_getspeed(0);


The tricky part is if/else, where if don't return error if
cpufreq_frequency_table_cpuinfo() fails. We want to set ->min[max]
and cpuinfo.min[max] always. And i can see this code not doing that for some
case even with my patch.

Possible scenarios:
1. Valid freq_table: My patch + what you suggested is required.
2. Invalid freq_table: We never set cpuinfo.min[max] with or without my patch
3. No freq_table: Only my patch is required.

If i do what you suggested then 2 and 3 would fail... If you want to
return error
in case cpufreq_frequency_table_cpuinfo(), then i can fix it properly.

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/24/2013 8:59 PM, Viresh Kumar wrote:
> Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
> aren't required as part of it is done by cpufreq driver or cpufreq core.
> 
> Remove them.
> 
> At some places we merge multiple lines together too.
> 
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: linux-sh@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/cpu-freq/cpu-drivers.txt | 5 +++--
>  arch/arm/mach-davinci/cpufreq.c        | 4 ----
>  arch/arm/mach-imx/cpufreq.c            | 3 ---
>  arch/sh/kernel/cpufreq.c               | 9 +++------
>  drivers/cpufreq/cpufreq-nforce2.c      | 6 ++----
>  drivers/cpufreq/omap-cpufreq.c         | 4 +---
>  6 files changed, 9 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
> index 72f70b1..c94383f 100644
> --- a/Documentation/cpu-freq/cpu-drivers.txt
> +++ b/Documentation/cpu-freq/cpu-drivers.txt
> @@ -108,8 +108,9 @@ policy->governor		must contain the "default policy" for
>  				cpufreq_driver.target is called with
>  				these values.
>  
> -For setting some of these values, the frequency table helpers might be
> -helpful. See the section 2 for more information on them.
> +For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
> +frequency table helpers might be helpful. See the section 2 for more information
> +on them.
>  
>  SMP systems normally have same clock source for a group of cpus. For these the
>  .init() would be called only once for the first online cpu. Here the .init()
> diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
> index 8fb0c2a..ff46862 100644
> --- a/arch/arm/mach-davinci/cpufreq.c
> +++ b/arch/arm/mach-davinci/cpufreq.c
> @@ -149,10 +149,6 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
>  		policy->cpuinfo.max_freq = policy->max;
>  	}
>  
> -	policy->min = policy->cpuinfo.min_freq;
> -	policy->max = policy->cpuinfo.max_freq;
> -	policy->cur = davinci_getspeed(0);

There is a line in the code a little above the ones you deleted that
also sets these same variables. I guess you were relying on that line to
set policy->cur, but that also sets policy->{min, max} which can be
cleaned up.

Thanks,
Sekhar

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Viresh Kumar]

On 25 March 2013 14:06, Sekhar Nori <nsekhar@ti.com> wrote:
> There is a line in the code a little above the ones you deleted that
> also sets these same variables. I guess you were relying on that line to
> set policy->cur, but that also sets policy->{min, max} which can be
> cleaned up.

This code is rather confusing or wrong, this was the state of code before
this patch:

	policy->cur = policy->min = policy->max = davinci_getspeed(0);

	if (freq_table) {
		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
		if (!result)
			cpufreq_frequency_table_get_attr(freq_table,
							policy->cpu);
	} else {
		policy->cpuinfo.min_freq = policy->min;
		policy->cpuinfo.max_freq = policy->max;
	}

        policy->min = policy->cpuinfo.min_freq;
        policy->max = policy->cpuinfo.max_freq;
        policy->cur = davinci_getspeed(0);


The tricky part is if/else, where if don't return error if
cpufreq_frequency_table_cpuinfo() fails. We want to set ->min[max]
and cpuinfo.min[max] always. And i can see this code not doing that for some
case even with my patch.

Possible scenarios:
1. Valid freq_table: My patch + what you suggested is required.
2. Invalid freq_table: We never set cpuinfo.min[max] with or without my patch
3. No freq_table: Only my patch is required.

If i do what you suggested then 2 and 3 would fail... If you want to
return error
in case cpufreq_frequency_table_cpuinfo(), then i can fix it properly.

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/25/2013 2:15 PM, Viresh Kumar wrote:
> On 25 March 2013 14:06, Sekhar Nori <nsekhar@ti.com> wrote:
>> There is a line in the code a little above the ones you deleted that
>> also sets these same variables. I guess you were relying on that line to
>> set policy->cur, but that also sets policy->{min, max} which can be
>> cleaned up.
> 
> This code is rather confusing or wrong, this was the state of code before
> this patch:
> 
> 	policy->cur = policy->min = policy->max = davinci_getspeed(0);
> 
> 	if (freq_table) {
> 		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
> 		if (!result)
> 			cpufreq_frequency_table_get_attr(freq_table,
> 							policy->cpu);
> 	} else {
> 		policy->cpuinfo.min_freq = policy->min;
> 		policy->cpuinfo.max_freq = policy->max;
> 	}
> 
>         policy->min = policy->cpuinfo.min_freq;
>         policy->max = policy->cpuinfo.max_freq;
>         policy->cur = davinci_getspeed(0);
> 
> 
> The tricky part is if/else, where if don't return error if
> cpufreq_frequency_table_cpuinfo() fails. We want to set ->min[max]
> and cpuinfo.min[max] always. And i can see this code not doing that for some
> case even with my patch.
> 
> Possible scenarios:
> 1. Valid freq_table: My patch + what you suggested is required.
> 2. Invalid freq_table: We never set cpuinfo.min[max] with or without my patch
> 3. No freq_table: Only my patch is required.
> 
> If i do what you suggested then 2 and 3 would fail... If you want to
> return error
> in case cpufreq_frequency_table_cpuinfo(), then i can fix it properly.

So down in the cpufreq driver probe below, we bail out if freq_table is
not provided. So all this checking for freq_table in the code you pasted
above is superfluous. If you can clean that part up and add checking for
cpufreq_frequency_table_cpuinfo() as you proposed, I will be glad to
test it out ;)

Thanks,
Sekhar

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/25/2013 2:15 PM, Viresh Kumar wrote:
> On 25 March 2013 14:06, Sekhar Nori <nsekhar@ti.com> wrote:
>> There is a line in the code a little above the ones you deleted that
>> also sets these same variables. I guess you were relying on that line to
>> set policy->cur, but that also sets policy->{min, max} which can be
>> cleaned up.
> 
> This code is rather confusing or wrong, this was the state of code before
> this patch:
> 
> 	policy->cur = policy->min = policy->max = davinci_getspeed(0);
> 
> 	if (freq_table) {
> 		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
> 		if (!result)
> 			cpufreq_frequency_table_get_attr(freq_table,
> 							policy->cpu);
> 	} else {
> 		policy->cpuinfo.min_freq = policy->min;
> 		policy->cpuinfo.max_freq = policy->max;
> 	}
> 
>         policy->min = policy->cpuinfo.min_freq;
>         policy->max = policy->cpuinfo.max_freq;
>         policy->cur = davinci_getspeed(0);
> 
> 
> The tricky part is if/else, where if don't return error if
> cpufreq_frequency_table_cpuinfo() fails. We want to set ->min[max]
> and cpuinfo.min[max] always. And i can see this code not doing that for some
> case even with my patch.
> 
> Possible scenarios:
> 1. Valid freq_table: My patch + what you suggested is required.
> 2. Invalid freq_table: We never set cpuinfo.min[max] with or without my patch
> 3. No freq_table: Only my patch is required.
> 
> If i do what you suggested then 2 and 3 would fail... If you want to
> return error
> in case cpufreq_frequency_table_cpuinfo(), then i can fix it properly.

So down in the cpufreq driver probe below, we bail out if freq_table is
not provided. So all this checking for freq_table in the code you pasted
above is superfluous. If you can clean that part up and add checking for
cpufreq_frequency_table_cpuinfo() as you proposed, I will be glad to
test it out ;)

Thanks,
Sekhar

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/25/2013 2:15 PM, Viresh Kumar wrote:
> On 25 March 2013 14:06, Sekhar Nori <nsekhar@ti.com> wrote:
>> There is a line in the code a little above the ones you deleted that
>> also sets these same variables. I guess you were relying on that line to
>> set policy->cur, but that also sets policy->{min, max} which can be
>> cleaned up.
> 
> This code is rather confusing or wrong, this was the state of code before
> this patch:
> 
> 	policy->cur = policy->min = policy->max = davinci_getspeed(0);
> 
> 	if (freq_table) {
> 		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
> 		if (!result)
> 			cpufreq_frequency_table_get_attr(freq_table,
> 							policy->cpu);
> 	} else {
> 		policy->cpuinfo.min_freq = policy->min;
> 		policy->cpuinfo.max_freq = policy->max;
> 	}
> 
>         policy->min = policy->cpuinfo.min_freq;
>         policy->max = policy->cpuinfo.max_freq;
>         policy->cur = davinci_getspeed(0);
> 
> 
> The tricky part is if/else, where if don't return error if
> cpufreq_frequency_table_cpuinfo() fails. We want to set ->min[max]
> and cpuinfo.min[max] always. And i can see this code not doing that for some
> case even with my patch.
> 
> Possible scenarios:
> 1. Valid freq_table: My patch + what you suggested is required.
> 2. Invalid freq_table: We never set cpuinfo.min[max] with or without my patch
> 3. No freq_table: Only my patch is required.
> 
> If i do what you suggested then 2 and 3 would fail... If you want to
> return error
> in case cpufreq_frequency_table_cpuinfo(), then i can fix it properly.

So down in the cpufreq driver probe below, we bail out if freq_table is
not provided. So all this checking for freq_table in the code you pasted
above is superfluous. If you can clean that part up and add checking for
cpufreq_frequency_table_cpuinfo() as you proposed, I will be glad to
test it out ;)

Thanks,
Sekhar

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Viresh Kumar]

[-- Attachment #1: Type: text/plain, Size: 1763 bytes --]

On 25 March 2013 15:11, Sekhar Nori <nsekhar@ti.com> wrote:
> So down in the cpufreq driver probe below, we bail out if freq_table is
> not provided. So all this checking for freq_table in the code you pasted
> above is superfluous. If you can clean that part up and add checking for
> cpufreq_frequency_table_cpuinfo() as you proposed, I will be glad to
> test it out ;)

Attached is the complete patch and following is your fixup for davinci
(just to review):

diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
index ff46862..7c2e943 100644
--- a/arch/arm/mach-davinci/cpufreq.c
+++ b/arch/arm/mach-davinci/cpufreq.c
@@ -137,18 +137,17 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
                        return result;
        }

-       policy->cur = policy->min = policy->max = davinci_getspeed(0);
-
-       if (freq_table) {
-               result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
-               if (!result)
-                       cpufreq_frequency_table_get_attr(freq_table,
-                                                       policy->cpu);
-       } else {
-               policy->cpuinfo.min_freq = policy->min;
-               policy->cpuinfo.max_freq = policy->max;
+       policy->cur = davinci_getspeed(0);
+
+       result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
+       if (result) {
+               pr_err("%s: cpufreq_frequency_table_cpuinfo() failed",
+                               __func__);
+               return result;
        }

+       cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
+
        /*
         * Time measurement across the target() function yields ~1500-1800us
         * time taken with no drivers on notification list.

[-- Attachment #2: 0001-cpufreq-drivers-Remove-unnecessary-assignments-of-po.patch --]
[-- Type: application/octet-stream, Size: 6364 bytes --]

From 1f741c6e1143339361c1201ccba3238bcd99b57d Mon Sep 17 00:00:00 2001
Message-Id: <1f741c6e1143339361c1201ccba3238bcd99b57d.1364207009.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Sun, 24 Mar 2013 20:51:31 +0530
Subject: [PATCH] cpufreq: drivers: Remove unnecessary assignments of policy->
 members

Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
aren't required as part of it is done by cpufreq driver or cpufreq core.

Remove them.

At some places we merge multiple lines together too.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/cpu-freq/cpu-drivers.txt |  5 +++--
 arch/arm/mach-davinci/cpufreq.c        | 21 ++++++++-------------
 arch/arm/mach-imx/cpufreq.c            |  3 ---
 arch/sh/kernel/cpufreq.c               |  9 +++------
 drivers/cpufreq/cpufreq-nforce2.c      |  6 ++----
 drivers/cpufreq/omap-cpufreq.c         |  4 +---
 6 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
index 72f70b1..c94383f 100644
--- a/Documentation/cpu-freq/cpu-drivers.txt
+++ b/Documentation/cpu-freq/cpu-drivers.txt
@@ -108,8 +108,9 @@ policy->governor		must contain the "default policy" for
 				cpufreq_driver.target is called with
 				these values.
 
-For setting some of these values, the frequency table helpers might be
-helpful. See the section 2 for more information on them.
+For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
+frequency table helpers might be helpful. See the section 2 for more information
+on them.
 
 SMP systems normally have same clock source for a group of cpus. For these the
 .init() would be called only once for the first online cpu. Here the .init()
diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
index 8fb0c2a..7c2e943 100644
--- a/arch/arm/mach-davinci/cpufreq.c
+++ b/arch/arm/mach-davinci/cpufreq.c
@@ -137,21 +137,16 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
 			return result;
 	}
 
-	policy->cur = policy->min = policy->max = davinci_getspeed(0);
-
-	if (freq_table) {
-		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
-		if (!result)
-			cpufreq_frequency_table_get_attr(freq_table,
-							policy->cpu);
-	} else {
-		policy->cpuinfo.min_freq = policy->min;
-		policy->cpuinfo.max_freq = policy->max;
+	policy->cur = davinci_getspeed(0);
+
+	result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
+	if (result) {
+		pr_err("%s: cpufreq_frequency_table_cpuinfo() failed",
+				__func__);
+		return result;
 	}
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-	policy->cur = davinci_getspeed(0);
+	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
 
 	/*
 	 * Time measurement across the target() function yields ~1500-1800us
diff --git a/arch/arm/mach-imx/cpufreq.c b/arch/arm/mach-imx/cpufreq.c
index cfce5e3..387dc4c 100644
--- a/arch/arm/mach-imx/cpufreq.c
+++ b/arch/arm/mach-imx/cpufreq.c
@@ -144,14 +144,11 @@ static int mxc_cpufreq_init(struct cpufreq_policy *policy)
 	imx_freq_table[i].frequency = CPUFREQ_TABLE_END;
 
 	policy->cur = clk_get_rate(cpu_clk) / 1000;
-	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
-	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
 
 	/* Manual states, that PLL stabilizes in two CLK32 periods */
 	policy->cpuinfo.transition_latency = 2 * NANOSECOND / CLK32_FREQ;
 
 	ret = cpufreq_frequency_table_cpuinfo(policy, imx_freq_table);
-
 	if (ret < 0) {
 		printk(KERN_ERR "%s: failed to register i.MXC CPUfreq with error code %d\n",
 		       __func__, ret);
diff --git a/arch/sh/kernel/cpufreq.c b/arch/sh/kernel/cpufreq.c
index 0fdf64b..88c8fee 100644
--- a/arch/sh/kernel/cpufreq.c
+++ b/arch/sh/kernel/cpufreq.c
@@ -116,7 +116,7 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		return PTR_ERR(cpuclk);
 	}
 
-	policy->cur = policy->min = policy->max = sh_cpufreq_get(cpu);
+	policy->cur = sh_cpufreq_get(cpu);
 
 	freq_table = cpuclk->nr_freqs ? cpuclk->freq_table : NULL;
 	if (freq_table) {
@@ -129,15 +129,12 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		dev_notice(dev, "no frequency table found, falling back "
 			   "to rate rounding.\n");
 
-		policy->cpuinfo.min_freq =
+		policy->min = policy->cpuinfo.min_freq =
 			(clk_round_rate(cpuclk, 1) + 500) / 1000;
-		policy->cpuinfo.max_freq =
+		policy->max = policy->cpuinfo.max_freq =
 			(clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
 	}
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 
 	dev_info(dev, "CPU Frequencies - Minimum %u.%03u MHz, "
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index 224a478..af1542d 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -359,12 +359,10 @@ static int nforce2_cpu_init(struct cpufreq_policy *policy)
 		min_fsb = NFORCE2_MIN_FSB;
 
 	/* cpuinfo and default policy values */
-	policy->cpuinfo.min_freq = min_fsb * fid * 100;
-	policy->cpuinfo.max_freq = max_fsb * fid * 100;
+	policy->min = policy->cpuinfo.min_freq = min_fsb * fid * 100;
+	policy->max = policy->cpuinfo.max_freq = max_fsb * fid * 100;
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 	policy->cur = nforce2_get(policy->cpu);
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
 
 	return 0;
 }
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index b610edd..ad7549c 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -177,7 +177,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 		goto fail_ck;
 	}
 
-	policy->cur = policy->min = policy->max = omap_getspeed(policy->cpu);
+	policy->cur = omap_getspeed(policy->cpu);
 
 	if (!freq_table)
 		result = opp_init_cpufreq_table(mpu_dev, &freq_table);
@@ -196,8 +196,6 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 
 	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
 	policy->cur = omap_getspeed(policy->cpu);
 
 	/*
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Viresh Kumar]

[-- Attachment #1: Type: text/plain, Size: 1763 bytes --]

On 25 March 2013 15:11, Sekhar Nori <nsekhar@ti.com> wrote:
> So down in the cpufreq driver probe below, we bail out if freq_table is
> not provided. So all this checking for freq_table in the code you pasted
> above is superfluous. If you can clean that part up and add checking for
> cpufreq_frequency_table_cpuinfo() as you proposed, I will be glad to
> test it out ;)

Attached is the complete patch and following is your fixup for davinci
(just to review):

diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
index ff46862..7c2e943 100644
--- a/arch/arm/mach-davinci/cpufreq.c
+++ b/arch/arm/mach-davinci/cpufreq.c
@@ -137,18 +137,17 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
                        return result;
        }

-       policy->cur = policy->min = policy->max = davinci_getspeed(0);
-
-       if (freq_table) {
-               result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
-               if (!result)
-                       cpufreq_frequency_table_get_attr(freq_table,
-                                                       policy->cpu);
-       } else {
-               policy->cpuinfo.min_freq = policy->min;
-               policy->cpuinfo.max_freq = policy->max;
+       policy->cur = davinci_getspeed(0);
+
+       result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
+       if (result) {
+               pr_err("%s: cpufreq_frequency_table_cpuinfo() failed",
+                               __func__);
+               return result;
        }

+       cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
+
        /*
         * Time measurement across the target() function yields ~1500-1800us
         * time taken with no drivers on notification list.

[-- Attachment #2: 0001-cpufreq-drivers-Remove-unnecessary-assignments-of-po.patch --]
[-- Type: application/octet-stream, Size: 6364 bytes --]

From 1f741c6e1143339361c1201ccba3238bcd99b57d Mon Sep 17 00:00:00 2001
Message-Id: <1f741c6e1143339361c1201ccba3238bcd99b57d.1364207009.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Sun, 24 Mar 2013 20:51:31 +0530
Subject: [PATCH] cpufreq: drivers: Remove unnecessary assignments of policy->
 members

Some assignments of policy-> min/max/cur/cpuinfo.min_freq/cpuinfo.max_freq
aren't required as part of it is done by cpufreq driver or cpufreq core.

Remove them.

At some places we merge multiple lines together too.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/cpu-freq/cpu-drivers.txt |  5 +++--
 arch/arm/mach-davinci/cpufreq.c        | 21 ++++++++-------------
 arch/arm/mach-imx/cpufreq.c            |  3 ---
 arch/sh/kernel/cpufreq.c               |  9 +++------
 drivers/cpufreq/cpufreq-nforce2.c      |  6 ++----
 drivers/cpufreq/omap-cpufreq.c         |  4 +---
 6 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/Documentation/cpu-freq/cpu-drivers.txt b/Documentation/cpu-freq/cpu-drivers.txt
index 72f70b1..c94383f 100644
--- a/Documentation/cpu-freq/cpu-drivers.txt
+++ b/Documentation/cpu-freq/cpu-drivers.txt
@@ -108,8 +108,9 @@ policy->governor		must contain the "default policy" for
 				cpufreq_driver.target is called with
 				these values.
 
-For setting some of these values, the frequency table helpers might be
-helpful. See the section 2 for more information on them.
+For setting some of these values (cpuinfo.min[max]_freq, policy->min[max]), the
+frequency table helpers might be helpful. See the section 2 for more information
+on them.
 
 SMP systems normally have same clock source for a group of cpus. For these the
 .init() would be called only once for the first online cpu. Here the .init()
diff --git a/arch/arm/mach-davinci/cpufreq.c b/arch/arm/mach-davinci/cpufreq.c
index 8fb0c2a..7c2e943 100644
--- a/arch/arm/mach-davinci/cpufreq.c
+++ b/arch/arm/mach-davinci/cpufreq.c
@@ -137,21 +137,16 @@ static int davinci_cpu_init(struct cpufreq_policy *policy)
 			return result;
 	}
 
-	policy->cur = policy->min = policy->max = davinci_getspeed(0);
-
-	if (freq_table) {
-		result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
-		if (!result)
-			cpufreq_frequency_table_get_attr(freq_table,
-							policy->cpu);
-	} else {
-		policy->cpuinfo.min_freq = policy->min;
-		policy->cpuinfo.max_freq = policy->max;
+	policy->cur = davinci_getspeed(0);
+
+	result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
+	if (result) {
+		pr_err("%s: cpufreq_frequency_table_cpuinfo() failed",
+				__func__);
+		return result;
 	}
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-	policy->cur = davinci_getspeed(0);
+	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
 
 	/*
 	 * Time measurement across the target() function yields ~1500-1800us
diff --git a/arch/arm/mach-imx/cpufreq.c b/arch/arm/mach-imx/cpufreq.c
index cfce5e3..387dc4c 100644
--- a/arch/arm/mach-imx/cpufreq.c
+++ b/arch/arm/mach-imx/cpufreq.c
@@ -144,14 +144,11 @@ static int mxc_cpufreq_init(struct cpufreq_policy *policy)
 	imx_freq_table[i].frequency = CPUFREQ_TABLE_END;
 
 	policy->cur = clk_get_rate(cpu_clk) / 1000;
-	policy->min = policy->cpuinfo.min_freq = cpu_freq_khz_min;
-	policy->max = policy->cpuinfo.max_freq = cpu_freq_khz_max;
 
 	/* Manual states, that PLL stabilizes in two CLK32 periods */
 	policy->cpuinfo.transition_latency = 2 * NANOSECOND / CLK32_FREQ;
 
 	ret = cpufreq_frequency_table_cpuinfo(policy, imx_freq_table);
-
 	if (ret < 0) {
 		printk(KERN_ERR "%s: failed to register i.MXC CPUfreq with error code %d\n",
 		       __func__, ret);
diff --git a/arch/sh/kernel/cpufreq.c b/arch/sh/kernel/cpufreq.c
index 0fdf64b..88c8fee 100644
--- a/arch/sh/kernel/cpufreq.c
+++ b/arch/sh/kernel/cpufreq.c
@@ -116,7 +116,7 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		return PTR_ERR(cpuclk);
 	}
 
-	policy->cur = policy->min = policy->max = sh_cpufreq_get(cpu);
+	policy->cur = sh_cpufreq_get(cpu);
 
 	freq_table = cpuclk->nr_freqs ? cpuclk->freq_table : NULL;
 	if (freq_table) {
@@ -129,15 +129,12 @@ static int sh_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		dev_notice(dev, "no frequency table found, falling back "
 			   "to rate rounding.\n");
 
-		policy->cpuinfo.min_freq =
+		policy->min = policy->cpuinfo.min_freq =
 			(clk_round_rate(cpuclk, 1) + 500) / 1000;
-		policy->cpuinfo.max_freq =
+		policy->max = policy->cpuinfo.max_freq =
 			(clk_round_rate(cpuclk, ~0UL) + 500) / 1000;
 	}
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
-
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 
 	dev_info(dev, "CPU Frequencies - Minimum %u.%03u MHz, "
diff --git a/drivers/cpufreq/cpufreq-nforce2.c b/drivers/cpufreq/cpufreq-nforce2.c
index 224a478..af1542d 100644
--- a/drivers/cpufreq/cpufreq-nforce2.c
+++ b/drivers/cpufreq/cpufreq-nforce2.c
@@ -359,12 +359,10 @@ static int nforce2_cpu_init(struct cpufreq_policy *policy)
 		min_fsb = NFORCE2_MIN_FSB;
 
 	/* cpuinfo and default policy values */
-	policy->cpuinfo.min_freq = min_fsb * fid * 100;
-	policy->cpuinfo.max_freq = max_fsb * fid * 100;
+	policy->min = policy->cpuinfo.min_freq = min_fsb * fid * 100;
+	policy->max = policy->cpuinfo.max_freq = max_fsb * fid * 100;
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 	policy->cur = nforce2_get(policy->cpu);
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
 
 	return 0;
 }
diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index b610edd..ad7549c 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -177,7 +177,7 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 		goto fail_ck;
 	}
 
-	policy->cur = policy->min = policy->max = omap_getspeed(policy->cpu);
+	policy->cur = omap_getspeed(policy->cpu);
 
 	if (!freq_table)
 		result = opp_init_cpufreq_table(mpu_dev, &freq_table);
@@ -196,8 +196,6 @@ static int __cpuinit omap_cpu_init(struct cpufreq_policy *policy)
 
 	cpufreq_frequency_table_get_attr(freq_table, policy->cpu);
 
-	policy->min = policy->cpuinfo.min_freq;
-	policy->max = policy->cpuinfo.max_freq;
 	policy->cur = omap_getspeed(policy->cpu);
 
 	/*
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/25/2013 3:54 PM, Viresh Kumar wrote:
> On 25 March 2013 15:11, Sekhar Nori <nsekhar@ti.com> wrote:
>> So down in the cpufreq driver probe below, we bail out if freq_table is
>> not provided. So all this checking for freq_table in the code you pasted
>> above is superfluous. If you can clean that part up and add checking for
>> cpufreq_frequency_table_cpuinfo() as you proposed, I will be glad to
>> test it out ;)
> 
> Attached is the complete patch and following is your fixup for davinci
> (just to review):

For the attached patch,

[nsekhar@ti.com: tested on da850 evm]
Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
sekhar

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/25/2013 3:54 PM, Viresh Kumar wrote:
> On 25 March 2013 15:11, Sekhar Nori <nsekhar@ti.com> wrote:
>> So down in the cpufreq driver probe below, we bail out if freq_table is
>> not provided. So all this checking for freq_table in the code you pasted
>> above is superfluous. If you can clean that part up and add checking for
>> cpufreq_frequency_table_cpuinfo() as you proposed, I will be glad to
>> test it out ;)
> 
> Attached is the complete patch and following is your fixup for davinci
> (just to review):

For the attached patch,

[nsekhar@ti.com: tested on da850 evm]
Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
sekhar

Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members

[Sekhar Nori]

On 3/25/2013 3:54 PM, Viresh Kumar wrote:
> On 25 March 2013 15:11, Sekhar Nori <nsekhar@ti.com> wrote:
>> So down in the cpufreq driver probe below, we bail out if freq_table is
>> not provided. So all this checking for freq_table in the code you pasted
>> above is superfluous. If you can clean that part up and add checking for
>> cpufreq_frequency_table_cpuinfo() as you proposed, I will be glad to
>> test it out ;)
> 
> Attached is the complete patch and following is your fixup for davinci
> (just to review):

For the attached patch,

[nsekhar@ti.com: tested on da850 evm]
Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
sekhar

Re: [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target()

[Linus Walleij]

On Sun, Mar 24, 2013 at 4:29 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> Cpufreq core checks the range of target_freq before calling driver->target() and
> so we don't need to do it again.
>
> Remove it.
>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij