| Message ID | 1356084595-17800-1-git-send-email-linuxzsc@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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?
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.
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
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.
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.
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.
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); }
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(-)