* [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() @ 2013-03-24 15:29 Viresh Kumar 2013-03-24 15:41 ` Viresh Kumar 2013-03-27 13:55 ` [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() Linus Walleij 0 siblings, 2 replies; 17+ messages in thread From: Viresh Kumar @ 2013-03-24 15:29 UTC (permalink / raw) To: rjw Cc: arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Viresh Kumar, Sekhar Nori, Linus Walleij 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members 2013-03-24 15:29 [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() Viresh Kumar @ 2013-03-24 15:41 ` Viresh Kumar 2013-03-27 13:55 ` [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() Linus Walleij 1 sibling, 0 replies; 17+ messages in thread From: Viresh Kumar @ 2013-03-24 15:29 UTC (permalink / raw) To: rjw Cc: arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Viresh Kumar, Sekhar Nori, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-24 15:41 ` Viresh Kumar 0 siblings, 0 replies; 17+ messages in thread From: Viresh Kumar @ 2013-03-24 15:41 UTC (permalink / raw) To: rjw Cc: arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Viresh Kumar, Sekhar Nori, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members 2013-03-24 15:41 ` Viresh Kumar (?) @ 2013-03-25 8:36 ` Sekhar Nori -1 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-25 8:36 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-25 8:36 ` Sekhar Nori 0 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-25 8:48 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-25 8:36 ` Sekhar Nori 0 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-25 8:36 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members 2013-03-25 8:36 ` Sekhar Nori @ 2013-03-25 8:57 ` Viresh Kumar -1 siblings, 0 replies; 17+ messages in thread From: Viresh Kumar @ 2013-03-25 8:45 UTC (permalink / raw) To: Sekhar Nori Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-25 8:57 ` Viresh Kumar 0 siblings, 0 replies; 17+ messages in thread From: Viresh Kumar @ 2013-03-25 8:57 UTC (permalink / raw) To: Sekhar Nori Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members 2013-03-25 8:57 ` Viresh Kumar (?) @ 2013-03-25 9:41 ` Sekhar Nori -1 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-25 9:41 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-25 9:41 ` Sekhar Nori 0 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-25 9:53 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-25 9:41 ` Sekhar Nori 0 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-25 9:41 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members 2013-03-25 9:41 ` Sekhar Nori @ 2013-03-25 10:36 ` Viresh Kumar -1 siblings, 0 replies; 17+ messages in thread From: Viresh Kumar @ 2013-03-25 10:24 UTC (permalink / raw) To: Sekhar Nori Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap [-- 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-25 10:36 ` Viresh Kumar 0 siblings, 0 replies; 17+ messages in thread From: Viresh Kumar @ 2013-03-25 10:36 UTC (permalink / raw) To: Sekhar Nori Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap [-- 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members 2013-03-25 10:36 ` Viresh Kumar (?) @ 2013-03-26 6:06 ` Sekhar Nori -1 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-26 6:06 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-26 6:06 ` Sekhar Nori 0 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-26 6:18 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members @ 2013-03-26 6:06 ` Sekhar Nori 0 siblings, 0 replies; 17+ messages in thread From: Sekhar Nori @ 2013-03-26 6:06 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sascha Hauer, Paul Mundt, linux-sh, linux-omap 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() 2013-03-24 15:29 [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() Viresh Kumar 2013-03-24 15:41 ` Viresh Kumar @ 2013-03-27 13:55 ` Linus Walleij 1 sibling, 0 replies; 17+ messages in thread From: Linus Walleij @ 2013-03-27 13:55 UTC (permalink / raw) To: Viresh Kumar Cc: rjw, arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau, charles.garcia-tobin, cpufreq, linux-pm, linux-kernel, linaro-kernel, Sekhar Nori, Rickard ANDERSSON 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-03-27 13:55 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-24 15:29 [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() Viresh Kumar 2013-03-24 15:29 ` [PATCH 2/2] cpufreq: drivers: Remove unnecessary assignments of policy-> members Viresh Kumar 2013-03-24 15:41 ` Viresh Kumar 2013-03-25 8:36 ` Sekhar Nori 2013-03-25 8:48 ` Sekhar Nori 2013-03-25 8:36 ` Sekhar Nori 2013-03-25 8:45 ` Viresh Kumar 2013-03-25 8:57 ` Viresh Kumar 2013-03-25 9:41 ` Sekhar Nori 2013-03-25 9:53 ` Sekhar Nori 2013-03-25 9:41 ` Sekhar Nori 2013-03-25 10:24 ` Viresh Kumar 2013-03-25 10:36 ` Viresh Kumar 2013-03-26 6:06 ` Sekhar Nori 2013-03-26 6:18 ` Sekhar Nori 2013-03-26 6:06 ` Sekhar Nori 2013-03-27 13:55 ` [PATCH 1/2] cpufreq: drivers: don't check range of target freq in .target() Linus Walleij
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.