| Message ID | cc399206d57ef946d27a98203c6b0a247cd1b245.1400670427.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 21/05/14 12:10, Viresh Kumar wrote: > OPP tables are already initialized for CPU0 by cpufreq core and so we don't need > to reinitialize them from imx6q specific code. > > Acked-by: Shawn Guo <shawn.guo@linaro.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > arch/arm/mach-imx/mach-imx6q.c | 36 ++++++++---------------------------- > drivers/cpufreq/imx6q-cpufreq.c | 20 +------------------- > 2 files changed, 9 insertions(+), 47 deletions(-) > > diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c > index e60456d..03819e7 100644 > --- a/arch/arm/mach-imx/mach-imx6q.c > +++ b/arch/arm/mach-imx/mach-imx6q.c > @@ -290,12 +290,18 @@ static void __init imx6q_init_machine(void) > #define OCOTP_CFG3_SPEED_996MHZ 0x2 > #define OCOTP_CFG3_SPEED_852MHZ 0x1 > > -static void __init imx6q_opp_check_speed_grading(struct device *cpu_dev) > +static void __init imx6q_opp_check_speed_grading(void) > { > + struct device *cpu_dev = get_cpu_device(0); > struct device_node *np; > void __iomem *base; > u32 val; > > + if (!cpu_dev) { > + pr_warn("failed to get cpu0 device\n"); > + return; > + } > + > np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ocotp"); > if (!np) { > pr_warn("failed to find ocotp node\n"); > @@ -336,32 +342,6 @@ put_node: > of_node_put(np); > } > > -static void __init imx6q_opp_init(void) > -{ > - struct device_node *np; > - struct device *cpu_dev = get_cpu_device(0); > - > - if (!cpu_dev) { > - pr_warn("failed to get cpu0 device\n"); > - return; > - } > - np = of_node_get(cpu_dev->of_node); > - if (!np) { > - pr_warn("failed to find cpu0 node\n"); > - return; > - } > - > - if (of_init_opp_table(cpu_dev)) { > - pr_warn("failed to init OPP table\n"); > - goto put_node; > - } > - > - imx6q_opp_check_speed_grading(cpu_dev); > - > -put_node: > - of_node_put(np); > -} > - > static struct platform_device imx6q_cpufreq_pdev = { > .name = "imx6q-cpufreq", > }; > @@ -376,7 +356,7 @@ static void __init imx6q_init_late(void) > imx6q_cpuidle_init(); > > if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) { > - imx6q_opp_init(); > + imx6q_opp_check_speed_grading(); [Query] Not exactly related to this patch, but asking it here to get clarified. This OPP limiting is done as part of late initcall and if the cpufreq driver is built in the kernel, there will be a small window where the OPPs not supported are still enabled ? Will that not be an issue if say performance governor is selected by default. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 21, 2014 at 03:07:36PM +0100, Sudeep Holla wrote: > >@@ -376,7 +356,7 @@ static void __init imx6q_init_late(void) > > imx6q_cpuidle_init(); > > > > if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) { > >- imx6q_opp_init(); > >+ imx6q_opp_check_speed_grading(); > > [Query] Not exactly related to this patch, but asking it here to get clarified. > > This OPP limiting is done as part of late initcall and if the cpufreq driver is > built in the kernel, there will be a small window where the OPPs not supported > are still enabled ? Will that not be an issue if say performance governor is > selected by default. Even if cpufreq driver is built in the kernel, it won't be probed until platform_device_register(&imx6q_cpufreq_pdev) is called. And we make this call only after the OPP limiting. Shawn -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21/05/14 15:16, Shawn Guo wrote: > On Wed, May 21, 2014 at 03:07:36PM +0100, Sudeep Holla wrote: >>> @@ -376,7 +356,7 @@ static void __init imx6q_init_late(void) >>> imx6q_cpuidle_init(); >>> >>> if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) { >>> - imx6q_opp_init(); >>> + imx6q_opp_check_speed_grading(); >> >> [Query] Not exactly related to this patch, but asking it here to get clarified. >> >> This OPP limiting is done as part of late initcall and if the cpufreq driver is >> built in the kernel, there will be a small window where the OPPs not supported >> are still enabled ? Will that not be an issue if say performance governor is >> selected by default. > > Even if cpufreq driver is built in the kernel, it won't be probed until > platform_device_register(&imx6q_cpufreq_pdev) is called. And we make > this call only after the OPP limiting. > Ah right, I missed that. Thanks for clarifying. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c index e60456d..03819e7 100644 --- a/arch/arm/mach-imx/mach-imx6q.c +++ b/arch/arm/mach-imx/mach-imx6q.c @@ -290,12 +290,18 @@ static void __init imx6q_init_machine(void) #define OCOTP_CFG3_SPEED_996MHZ 0x2 #define OCOTP_CFG3_SPEED_852MHZ 0x1 -static void __init imx6q_opp_check_speed_grading(struct device *cpu_dev) +static void __init imx6q_opp_check_speed_grading(void) { + struct device *cpu_dev = get_cpu_device(0); struct device_node *np; void __iomem *base; u32 val; + if (!cpu_dev) { + pr_warn("failed to get cpu0 device\n"); + return; + } + np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-ocotp"); if (!np) { pr_warn("failed to find ocotp node\n"); @@ -336,32 +342,6 @@ put_node: of_node_put(np); } -static void __init imx6q_opp_init(void) -{ - struct device_node *np; - struct device *cpu_dev = get_cpu_device(0); - - if (!cpu_dev) { - pr_warn("failed to get cpu0 device\n"); - return; - } - np = of_node_get(cpu_dev->of_node); - if (!np) { - pr_warn("failed to find cpu0 node\n"); - return; - } - - if (of_init_opp_table(cpu_dev)) { - pr_warn("failed to init OPP table\n"); - goto put_node; - } - - imx6q_opp_check_speed_grading(cpu_dev); - -put_node: - of_node_put(np); -} - static struct platform_device imx6q_cpufreq_pdev = { .name = "imx6q-cpufreq", }; @@ -376,7 +356,7 @@ static void __init imx6q_init_late(void) imx6q_cpuidle_init(); if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) { - imx6q_opp_init(); + imx6q_opp_check_speed_grading(); platform_device_register(&imx6q_cpufreq_pdev); } } diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c index af366c2..b72e94f 100644 --- a/drivers/cpufreq/imx6q-cpufreq.c +++ b/drivers/cpufreq/imx6q-cpufreq.c @@ -190,26 +190,8 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) goto put_reg; } - /* - * We expect an OPP table supplied by platform. - * Just, incase the platform did not supply the OPP - * table, it will try to get it. - */ num = dev_pm_opp_get_opp_count(cpu_dev); - if (num < 0) { - ret = of_init_opp_table(cpu_dev); - if (ret < 0) { - dev_err(cpu_dev, "failed to init OPP table: %d\n", ret); - goto put_reg; - } - - num = dev_pm_opp_get_opp_count(cpu_dev); - if (num < 0) { - ret = num; - dev_err(cpu_dev, "no OPP table is found: %d\n", ret); - goto put_reg; - } - } + WARN_ON(num < 0); ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); if (ret) {