Skip to contentSkip to conversation

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

PATCH New Richard Zhao 4 participants 7 messages Dec 21, 2012 – Jan 2, 2013

Subsystems (3)
Arm (32-bit) Arm sub-architectures Tegra architecture support
lore
Share & tools
lore

Richard Zhao

author
#0 lore raw

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

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 <[email protected]>
---
 arch/arm/mach-tegra/cpu-tegra.c |   40 +++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)
arch/arm/mach-tegra/cpu-tegra.c +20 -20
index 627bf0f..4faa3c6 100644
@@ -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);
 }
Signature · 1 line
1.7.9.5
#1 lore raw

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

Quoted 3 lines · 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.

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?

#2 lore raw

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

Quoted 10 lines · Stephen Warren
Quoted 3 lines · 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.

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.

author
#3 lore raw

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

Quoted 8 lines · Prashant Gaikwad
Quoted 7 lines · Stephen Warren
Quoted 4 lines · 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.

Quoted 7 lines
Quoted 7 lines

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.

Quoted 4 lines

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

#4 lore raw

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

In reply to Prashant Gaikwad
Quoted 7 lines · Prashant Gaikwad
Quoted 6 lines · Stephen Warren
Quoted 3 lines · 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.

I think this is reasonable.

Quoted 8 lines
Quoted 4 lines

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.

#5 lore raw

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

In reply to Stephen Warren
Quoted 11 lines · Stephen Warren
Quoted 3 lines · 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.

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.

Applied
#6 lore raw

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

In reply to Richard Zhao
Quoted 3 lines · 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.

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