Patchwork ARM: tegra: cpufreq: move clk_get/put out of function tegra_cpu_init/exit

login
register
mail settings
Submitter Richard Zhao
Date Dec. 21, 2012, 10:09 a.m.
Message ID <1356084595-17800-1-git-send-email-linuxzsc@gmail.com>
Download mbox | patch
Permalink /patch/207776/
State Accepted, archived
Headers show

Comments

Richard Zhao - Dec. 21, 2012, 10:09 a.m.
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@gmail.com>
---
 arch/arm/mach-tegra/cpu-tegra.c |   40 +++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)
Stephen Warren - Dec. 21, 2012, 9:27 p.m.
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?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Prashant Gaikwad - Dec. 23, 2012, 10:35 a.m.
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Richard Zhao - Dec. 24, 2012, 2:26 a.m.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter De Schrijver - Jan. 2, 2013, 12:20 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter De Schrijver - Jan. 2, 2013, 12:30 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - Jan. 2, 2013, 7:36 p.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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);
 }