| Message ID | 1ba7771e910084cd0820c19ca5994fe1b3d6451d.1404231535.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
On 07/01/14 09:32, Viresh Kumar wrote: > OPPs can be populated statically, via DT, or added at run time with > dev_pm_opp_add(). > > While this driver handles the first case correctly, it would fail to populate > OPPs added at runtime. Because call to of_init_opp_table() would fail as there > are no OPPs in DT and probe will return early. > > To fix this, remove error checking and call dev_pm_opp_init_cpufreq_table() > unconditionally. > > Suggested-by: Stephen Boyd <sboyd@codeaurora.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/cpufreq/cpufreq-cpu0.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) Please update the binding as well to indicate that this property is now optional.
On Tuesday 01 July 2014 12:32 PM, Viresh Kumar wrote: > OPPs can be populated statically, via DT, or added at run time with > dev_pm_opp_add(). > > While this driver handles the first case correctly, it would fail to populate > OPPs added at runtime. Because call to of_init_opp_table() would fail as there > are no OPPs in DT and probe will return early. > > To fix this, remove error checking and call dev_pm_opp_init_cpufreq_table() > unconditionally. > > Suggested-by: Stephen Boyd <sboyd@codeaurora.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- Assuming you are updating bidnings as suggested by Stephen, patch looks good to me. Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com> -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9 July 2014 20:14, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > Assuming you are updating bidnings as suggested by Stephen, > patch looks good to me. I have already sent a patch in reply to this one earlier with updated bindings. > Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com> Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 9 July 2014 20:14, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > Assuming you are updating bidnings as suggested by Stephen, > patch looks good to me. > Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com> Why do you still have a separate cpufreq driver for omap? Would this patch help getting that out? I see this for omap: static inline void omap_init_cpufreq(void) { struct platform_device_info devinfo = { }; if (!of_have_populated_dt()) devinfo.name = "omap-cpufreq"; else devinfo.name = "cpufreq-generic"; platform_device_register_full(&devinfo); } and it makes me believe that you were just waiting for this patch? -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 10, 2014 at 6:19 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 9 July 2014 20:14, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: >> Assuming you are updating bidnings as suggested by Stephen, >> patch looks good to me. >> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > > Why do you still have a separate cpufreq driver for omap? > Would this patch help getting that out? > > I see this for omap: > > static inline void omap_init_cpufreq(void) > { > struct platform_device_info devinfo = { }; > > if (!of_have_populated_dt()) > devinfo.name = "omap-cpufreq"; > else > devinfo.name = "cpufreq-generic"; > platform_device_register_full(&devinfo); > } > > and it makes me believe that you were just waiting for this patch? Sorry, am away on vacation and slow on emails. The plan was to kill omap cpufreq once all platforms convert to device tree only boot. Only platform left is OMAP3 based platforms - though the date for removing non-dt support has changed a couple of kernel revisions - but we should be able to remove that entire file with this change. We will need this support to go with the solution recommended for opp modifier series[1] - where platform code will populate or add OPPs based on "speed grade" sample detection. [1]http://comments.gmane.org/gmane.linux.ports.arm.kernel/309466 --- Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 10 July 2014 08:39 AM, Nishanth Menon wrote: > On Thu, Jul 10, 2014 at 6:19 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> On 9 July 2014 20:14, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: >>> Assuming you are updating bidnings as suggested by Stephen, >>> patch looks good to me. >>> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> >> Why do you still have a separate cpufreq driver for omap? >> Would this patch help getting that out? >> >> I see this for omap: >> >> static inline void omap_init_cpufreq(void) >> { >> struct platform_device_info devinfo = { }; >> >> if (!of_have_populated_dt()) >> devinfo.name = "omap-cpufreq"; >> else >> devinfo.name = "cpufreq-generic"; >> platform_device_register_full(&devinfo); >> } >> >> and it makes me believe that you were just waiting for this patch? > > Sorry, am away on vacation and slow on emails. The plan was to kill > omap cpufreq once all platforms convert to device tree only boot. Only > platform left is OMAP3 based platforms - though the date for removing > non-dt support has changed a couple of kernel revisions - but we > should be able to remove that entire file with this change. > > We will need this support to go with the solution recommended for opp > modifier series[1] - where platform code will populate or add OPPs > based on "speed grade" sample detection. > Yep. Last time I blocked the series because all the DT conversions were not done. Considering now the cpufreq-generic can work on non DT platforms, I am ok to remove the omap-cpufreq. Regards, Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10 July 2014 19:01, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote: > Yep. Last time I blocked the series because all the DT conversions > were not done. Considering now the cpufreq-generic can work on non > DT platforms, I am ok to remove the omap-cpufreq. Great. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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/drivers/cpufreq/cpufreq-cpu0.c b/drivers/cpufreq/cpufreq-cpu0.c index b5b8e1c..f47f703 100644 --- a/drivers/cpufreq/cpufreq-cpu0.c +++ b/drivers/cpufreq/cpufreq-cpu0.c @@ -164,11 +164,8 @@ static int cpu0_cpufreq_probe(struct platform_device *pdev) goto out_put_reg; } - ret = of_init_opp_table(cpu_dev); - if (ret) { - pr_err("failed to init OPP table: %d\n", ret); - goto out_put_clk; - } + /* OPPs might be populated at runtime, don't check for error here */ + of_init_opp_table(cpu_dev); ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table); if (ret) {
OPPs can be populated statically, via DT, or added at run time with dev_pm_opp_add(). While this driver handles the first case correctly, it would fail to populate OPPs added at runtime. Because call to of_init_opp_table() would fail as there are no OPPs in DT and probe will return early. To fix this, remove error checking and call dev_pm_opp_init_cpufreq_table() unconditionally. Suggested-by: Stephen Boyd <sboyd@codeaurora.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/cpufreq-cpu0.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)