| Message ID | 20130405213250.GA8534@kahuna (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
* Nishanth Menon <nm@ti.com> [130405 14:37]: > On 14:10-20130405, Tony Lindgren wrote: > > * Nishanth Menon <nm@ti.com> [130405 13:06]: > > > On 12:28-20130405, Tony Lindgren wrote: > [...] > > > > How about just set it up in omap2_common_pm_init instead > > > > of the board-generic? > > > umm.. We want to eventually want to get rid of mach-omap2/pm.c (all > > > those create processor devices etc should go away with proper > > > representation of devices as nodes in DT if possible. > > > But, I think you mean something like in the "else" condition of > > > https://patchwork.kernel.org/patch/2359711/ ? - I'd again have the same > > > request of not having anything to do with pm.c and keeping as little as > > > possible for all TI processors in mach-omap2. > > > > > > Could you enlighten me about why you'd not like it in board-generic.c? > > > will creating an function ti_generic_cpufreq_init() in board-generic.c > > > and calling it from omap_generic_init help? > > > > I'd like to keep board-generic.c down to minimum. Can't you > > set it up in omap_init_cpufreq() in your second patch of this > > series? > Thanks. That seems to be a better compromise. Will do. I can sequence patch > [2] above the current patch[1] if there is a need to fix multi-arch builds > for 3.9: in that case I will probably leave the current [2] patch as > is, and once our clock representation discussion is done, the rev 4 of > the patch [1], I can do the following: OK makes sense to me. > diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c > index 8d15f9a..b250689 100644 > --- a/arch/arm/mach-omap2/pm.c > +++ b/arch/arm/mach-omap2/pm.c > @@ -267,8 +267,14 @@ static void __init omap4_init_voltages(void) > > static inline void omap_init_cpufreq(void) > { > - struct platform_device_info devinfo = { .name = "omap-cpufreq", }; > - platform_device_register_full(&devinfo); > + struct platform_device_info devinfo = { .name = "omap-cpufreq", } > + > + if (!of_have_populated_dt()) { > + platform_device_register_full(&devinfo); > + } else if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) { > + devinfo.name = "cpufreq-cpu0" > + platform_device_register_full(&devinfo); > + } > } Maybe move platform_device_register_full(&devinfo) out of the if else as the different naming needed is the only difference? Regards, Tony -- 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 14:40-20130405, Tony Lindgren wrote: > * Nishanth Menon <nm@ti.com> [130405 14:37]: > > On 14:10-20130405, Tony Lindgren wrote: > > > * Nishanth Menon <nm@ti.com> [130405 13:06]: > > > > On 12:28-20130405, Tony Lindgren wrote: > > [...] > > > > > How about just set it up in omap2_common_pm_init instead > > > > > of the board-generic? > > > > umm.. We want to eventually want to get rid of mach-omap2/pm.c (all > > > > those create processor devices etc should go away with proper > > > > representation of devices as nodes in DT if possible. > > > > But, I think you mean something like in the "else" condition of > > > > https://patchwork.kernel.org/patch/2359711/ ? - I'd again have the same > > > > request of not having anything to do with pm.c and keeping as little as > > > > possible for all TI processors in mach-omap2. > > > > > > > > Could you enlighten me about why you'd not like it in board-generic.c? > > > > will creating an function ti_generic_cpufreq_init() in board-generic.c > > > > and calling it from omap_generic_init help? > > > > > > I'd like to keep board-generic.c down to minimum. Can't you > > > set it up in omap_init_cpufreq() in your second patch of this > > > series? > > Thanks. That seems to be a better compromise. Will do. I can sequence patch > > [2] above the current patch[1] if there is a need to fix multi-arch builds > > for 3.9: in that case I will probably leave the current [2] patch as > > is, and once our clock representation discussion is done, the rev 4 of > > the patch [1], I can do the following: > > OK makes sense to me. Kevin has picked up [2]. So, my V4 will just contain this patch :). Will hold sending it out till we conclude on Roger's thread. > > > diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c > > index 8d15f9a..b250689 100644 > > --- a/arch/arm/mach-omap2/pm.c > > +++ b/arch/arm/mach-omap2/pm.c > > @@ -267,8 +267,14 @@ static void __init omap4_init_voltages(void) > > > > static inline void omap_init_cpufreq(void) > > { > > - struct platform_device_info devinfo = { .name = "omap-cpufreq", }; > > - platform_device_register_full(&devinfo); > > + struct platform_device_info devinfo = { .name = "omap-cpufreq", } > > + > > + if (!of_have_populated_dt()) { > > + platform_device_register_full(&devinfo); > > + } else if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) { > > + devinfo.name = "cpufreq-cpu0" > > + platform_device_register_full(&devinfo); > > + } > > } > > Maybe move platform_device_register_full(&devinfo) out of > the if else as the different naming needed is the only > difference? How does the following look? Option a) not have a dummy node if CPUFREQ_CPU0 is not configured: static inline void omap_init_cpufreq(void) { struct platform_device_info devinfo = { }; if (!of_have_populated_dt()) devinfo.name = "omap-cpufreq"; else if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) devinfo.name = "cpufreq-cpu0" if (devinfo.name) platform_device_register_full(&devinfo); } Option b) leave a dummy node registered 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-cpu0" platform_device_register_full(&devinfo); } If there are no objections to (b), I dont mind it either.
* Nishanth Menon <nm@ti.com> [130405 15:15]: > How does the following look? > Option a) not have a dummy node if CPUFREQ_CPU0 is not configured: > static inline void omap_init_cpufreq(void) > { > struct platform_device_info devinfo = { }; > > if (!of_have_populated_dt()) > devinfo.name = "omap-cpufreq"; > else if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) > devinfo.name = "cpufreq-cpu0" > > if (devinfo.name) > platform_device_register_full(&devinfo); > } > > Option b) leave a dummy node registered > 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-cpu0" > > platform_device_register_full(&devinfo); > } > > If there are no objections to (b), I dont mind it either. To me option (b) looks better as it's if else :) Tony -- 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 15:17-20130405, Tony Lindgren wrote: > * Nishanth Menon <nm@ti.com> [130405 15:15]: > > Option b) leave a dummy node registered > > 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-cpu0" > > > > platform_device_register_full(&devinfo); > > } > > > > If there are no objections to (b), I dont mind it either. > > To me option (b) looks better as it's if else :) Alrity.. Going once...twice...sold! All Nay-sayers will now be branded with if-else braces ;) I will now wait for conclusion of https://lkml.org/lkml/2013/4/2/84
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c index 8d15f9a..b250689 100644 --- a/arch/arm/mach-omap2/pm.c +++ b/arch/arm/mach-omap2/pm.c @@ -267,8 +267,14 @@ static void __init omap4_init_voltages(void) static inline void omap_init_cpufreq(void) { - struct platform_device_info devinfo = { .name = "omap-cpufreq", }; - platform_device_register_full(&devinfo); + struct platform_device_info devinfo = { .name = "omap-cpufreq", } + + if (!of_have_populated_dt()) { + platform_device_register_full(&devinfo); + } else if (IS_ENABLED(CONFIG_GENERIC_CPUFREQ_CPU0)) { + devinfo.name = "cpufreq-cpu0" + platform_device_register_full(&devinfo); + } } static int __init omap2_common_pm_init(void) @@ -301,9 +307,9 @@ int __init omap2_common_pm_late_init(void) /* Smartreflex device init */ omap_devinit_smartreflex(); - /* cpufreq dummy device instantiation */ - omap_init_cpufreq(); } + /* cpufreq dummy device instantiation */ + omap_init_cpufreq(); #ifdef CONFIG_SUSPEND suspend_set_ops(&omap_pm_ops);