[PATCH] ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

[PATCH] ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

[Richard Zhao]

tegra_cpu_init/exit will be called every time one cpu core is online or
offline. And all cpu cores share same clocks, redundant clk_get/put
wast time,  so I move them out.

Signed-off-by: Richard Zhao <linuxzsc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/mach-tegra/cpu-tegra.c |   40 +++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-tegra/cpu-tegra.c b/arch/arm/mach-tegra/cpu-tegra.c
index 627bf0f..4faa3c6 100644
--- a/arch/arm/mach-tegra/cpu-tegra.c
+++ b/arch/arm/mach-tegra/cpu-tegra.c
@@ -217,24 +217,6 @@ static int tegra_cpu_init(struct cpufreq_policy *policy)
 	if (policy->cpu >= NUM_CPUS)
 		return -EINVAL;
 
-	cpu_clk = clk_get_sys(NULL, "cpu");
-	if (IS_ERR(cpu_clk))
-		return PTR_ERR(cpu_clk);
-
-	pll_x_clk = clk_get_sys(NULL, "pll_x");
-	if (IS_ERR(pll_x_clk))
-		return PTR_ERR(pll_x_clk);
-
-	pll_p_clk = clk_get_sys(NULL, "pll_p");
-	if (IS_ERR(pll_p_clk))
-		return PTR_ERR(pll_p_clk);
-
-	emc_clk = clk_get_sys("cpu", "emc");
-	if (IS_ERR(emc_clk)) {
-		clk_put(cpu_clk);
-		return PTR_ERR(emc_clk);
-	}
-
 	clk_prepare_enable(emc_clk);
 	clk_prepare_enable(cpu_clk);
 
@@ -259,8 +241,6 @@ static int tegra_cpu_exit(struct cpufreq_policy *policy)
 {
 	cpufreq_frequency_table_cpuinfo(policy, freq_table);
 	clk_disable_unprepare(emc_clk);
-	clk_put(emc_clk);
-	clk_put(cpu_clk);
 	return 0;
 }
 
@@ -281,12 +261,32 @@ static struct cpufreq_driver tegra_cpufreq_driver = {
 
 static int __init tegra_cpufreq_init(void)
 {
+	cpu_clk = clk_get_sys(NULL, "cpu");
+	if (IS_ERR(cpu_clk))
+		return PTR_ERR(cpu_clk);
+
+	pll_x_clk = clk_get_sys(NULL, "pll_x");
+	if (IS_ERR(pll_x_clk))
+		return PTR_ERR(pll_x_clk);
+
+	pll_p_clk = clk_get_sys(NULL, "pll_p");
+	if (IS_ERR(pll_p_clk))
+		return PTR_ERR(pll_p_clk);
+
+	emc_clk = clk_get_sys("cpu", "emc");
+	if (IS_ERR(emc_clk)) {
+		clk_put(cpu_clk);
+		return PTR_ERR(emc_clk);
+	}
+
 	return cpufreq_register_driver(&tegra_cpufreq_driver);
 }
 
 static void __exit tegra_cpufreq_exit(void)
 {
         cpufreq_unregister_driver(&tegra_cpufreq_driver);
+	clk_put(emc_clk);
+	clk_put(cpu_clk);
 }
 
 
-- 
1.7.9.5

Re: [PATCH] ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

[Stephen Warren]

On 12/21/2012 03:09 AM, Richard Zhao wrote:
> tegra_cpu_init/exit will be called every time one cpu core is online or
> offline. And all cpu cores share same clocks, redundant clk_get/put
> wast time,  so I move them out.

I think this is reasonable.

Prashant, Peter, any comments?

Is any other work in tegra_cpu_init() redundant? Is tegra_cpu_init()
supposed to be called due to hotplug, or is that a bug in the cpufreq core?

Re: [PATCH] ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

[Prashant Gaikwad]

On Saturday 22 December 2012 02:57 AM, Stephen Warren wrote:
> On 12/21/2012 03:09 AM, Richard Zhao wrote:
>> tegra_cpu_init/exit will be called every time one cpu core is online or
>> offline. And all cpu cores share same clocks, redundant clk_get/put
>> wast time,  so I move them out.
> I think this is reasonable.
>
> Prashant, Peter, any comments?
>
> Is any other work in tegra_cpu_init() redundant? Is tegra_cpu_init()
> supposed to be called due to hotplug, or is that a bug in the cpufreq core?

Looks good. Also, instead of array we can use single variable for 
target_cpu_speed.

Re: [PATCH] ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

[Richard Zhao]

On Sun, Dec 23, 2012 at 6:35 PM, Prashant Gaikwad <pgaikwad@nvidia.com> wrote:
> On Saturday 22 December 2012 02:57 AM, Stephen Warren wrote:
>>
>> On 12/21/2012 03:09 AM, Richard Zhao wrote:
>>>
>>> tegra_cpu_init/exit will be called every time one cpu core is online or
>>> offline. And all cpu cores share same clocks, redundant clk_get/put
>>> wast time,  so I move them out.
>>
>> I think this is reasonable.
>>
>> Prashant, Peter, any comments?
>>
>> Is any other work in tegra_cpu_init() redundant? Is tegra_cpu_init()
>> supposed to be called due to hotplug, or is that a bug in the cpufreq
>> core?
When cpu goes offline, kernel don't know whether it'll come back.
Sounds reasonable.
>
>
> Looks good. Also, instead of array we can use single variable for
> target_cpu_speed.
I think yes. If one day the cores support rate changing independently,
we can create another driver. It supposed to be another patch to do it.

Thanks
Richard

Re: [PATCH] ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

[Peter De Schrijver]

On Sun, Dec 23, 2012 at 11:35:26AM +0100, Prashant Gaikwad wrote:
> On Saturday 22 December 2012 02:57 AM, Stephen Warren wrote:
> > On 12/21/2012 03:09 AM, Richard Zhao wrote:
> >> tegra_cpu_init/exit will be called every time one cpu core is online or
> >> offline. And all cpu cores share same clocks, redundant clk_get/put
> >> wast time,  so I move them out.
> > I think this is reasonable.
> >
> > Prashant, Peter, any comments?
> >
> > Is any other work in tegra_cpu_init() redundant? Is tegra_cpu_init()
> > supposed to be called due to hotplug, or is that a bug in the cpufreq core?
> 
> Looks good. Also, instead of array we can use single variable for 
> target_cpu_speed.
> 

No. The target_cpu_speed becomes useful once we have clusterswitching. We can
use it to determine if we want to switch to the other cluster.

Cheers,

Peter.

Re: [PATCH] ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

[Peter De Schrijver]

On Fri, Dec 21, 2012 at 10:27:43PM +0100, Stephen Warren wrote:
> On 12/21/2012 03:09 AM, Richard Zhao wrote:
> > tegra_cpu_init/exit will be called every time one cpu core is online or
> > offline. And all cpu cores share same clocks, redundant clk_get/put
> > wast time,  so I move them out.
> 
> I think this is reasonable.
> 
> Prashant, Peter, any comments?
> 
> Is any other work in tegra_cpu_init() redundant? Is tegra_cpu_init()
> supposed to be called due to hotplug, or is that a bug in the cpufreq core?

I think we want to keep target_cpu_speed[] as a possible input for the
(to be upstreamed) cluster switching logic. Yes, tegra_cpu_init() should be
called due to hotplug. Some systems might need specific initialization for
cpufreq when a new CPU comes online. Also the available frequencies might
be different per CPU. So this information needs to be provided for each
CPU.

Cheers,

Peter.

Re: [PATCH] ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

[Stephen Warren]

On 12/21/2012 03:09 AM, Richard Zhao wrote:
> tegra_cpu_init/exit will be called every time one cpu core is online or
> offline. And all cpu cores share same clocks, redundant clk_get/put
> wast time,  so I move them out.

I've applied this to Tegra's for-3.9/soc branch.