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

2012-12-21 10:09 UTC #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

Stephen Warren

2012-12-21 21:27 UTC #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?

Prashant Gaikwad

2012-12-23 10:35 UTC #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.

Richard Zhao

author

2012-12-24 02:26 UTC #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

Peter De Schrijver

2013-01-02 12:20 UTC #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.

Peter De Schrijver

2013-01-02 12:30 UTC #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.

Stephen Warren

Applied

2013-01-02 19:36 UTC #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.