| Message ID | 1381361098-8283-1-git-send-email-nm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/09/2013 06:24 PM, Nishanth Menon wrote: > Call OMAP2+ generic lateinit hook from AM specific late init hook. > This allows the generic late initializations such as cpufreq hooks > to be active. > > Cc: Benoit Cousson <bcousson@baylibre.com> > Cc: Kevin Hilman <khilman@deeprootsystems.com> > Cc: Paul Walmsley <paul@pwsan.com> > Cc: Tony Lindgren <tony@atomide.com> > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > arch/arm/mach-omap2/board-generic.c | 1 + > arch/arm/mach-omap2/common.h | 1 + > arch/arm/mach-omap2/io.c | 6 ++++++ > 3 files changed, 8 insertions(+) > > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > index 87162e1..b474498 100644 > --- a/arch/arm/mach-omap2/board-generic.c > +++ b/arch/arm/mach-omap2/board-generic.c > @@ -180,6 +180,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)") > .init_irq = omap_intc_of_init, > .handle_irq = omap3_intc_handle_irq, > .init_machine = omap_generic_init, > + .init_late = am33xx_init_late, Instead of adding a new a *_init_late function for every platform, perhaps better to just do: .init_late = omap2_common_pm_late_init; since that's the only function you're calling. Later if more functions are added, then it can be wrapped around. regards, -Joel
On 00:32-20131010, Joel Fernandes wrote: > On 10/09/2013 06:24 PM, Nishanth Menon wrote: > > Call OMAP2+ generic lateinit hook from AM specific late init hook. > > This allows the generic late initializations such as cpufreq hooks > > to be active. > > > > Cc: Benoit Cousson <bcousson@baylibre.com> > > Cc: Kevin Hilman <khilman@deeprootsystems.com> > > Cc: Paul Walmsley <paul@pwsan.com> > > Cc: Tony Lindgren <tony@atomide.com> > > Signed-off-by: Nishanth Menon <nm@ti.com> > > --- > > arch/arm/mach-omap2/board-generic.c | 1 + > > arch/arm/mach-omap2/common.h | 1 + > > arch/arm/mach-omap2/io.c | 6 ++++++ > > 3 files changed, 8 insertions(+) > > > > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > > index 87162e1..b474498 100644 > > --- a/arch/arm/mach-omap2/board-generic.c > > +++ b/arch/arm/mach-omap2/board-generic.c > > @@ -180,6 +180,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)") > > .init_irq = omap_intc_of_init, > > .handle_irq = omap3_intc_handle_irq, > > .init_machine = omap_generic_init, > > + .init_late = am33xx_init_late, > > Instead of adding a new a *_init_late function for every platform, perhaps > better to just do: > .init_late = omap2_common_pm_late_init; > > since that's the only function you're calling. > > Later if more functions are added, then it can be wrapped around. And what benefit would that give us? we break consistency of functions available in io.c, considering the work we have already done in out-of-tree patches on ti-forks, we *do know* that there is more incoming and has to be done anyways, I prefer having symmetric functions and a placeholder that folks can add on to.
Hi Joel. On Thu, Oct 10, 2013 at 1:32 AM, Joel Fernandes <joelf@ti.com> wrote: > On 10/09/2013 06:24 PM, Nishanth Menon wrote: >> Call OMAP2+ generic lateinit hook from AM specific late init hook. >> This allows the generic late initializations such as cpufreq hooks >> to be active. >> >> Cc: Benoit Cousson <bcousson@baylibre.com> >> Cc: Kevin Hilman <khilman@deeprootsystems.com> >> Cc: Paul Walmsley <paul@pwsan.com> >> Cc: Tony Lindgren <tony@atomide.com> >> Signed-off-by: Nishanth Menon <nm@ti.com> >> --- >> arch/arm/mach-omap2/board-generic.c | 1 + >> arch/arm/mach-omap2/common.h | 1 + >> arch/arm/mach-omap2/io.c | 6 ++++++ >> 3 files changed, 8 insertions(+) >> >> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c >> index 87162e1..b474498 100644 >> --- a/arch/arm/mach-omap2/board-generic.c >> +++ b/arch/arm/mach-omap2/board-generic.c >> @@ -180,6 +180,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)") >> .init_irq = omap_intc_of_init, >> .handle_irq = omap3_intc_handle_irq, >> .init_machine = omap_generic_init, >> + .init_late = am33xx_init_late, > > Instead of adding a new a *_init_late function for every platform, perhaps > better to just do: > .init_late = omap2_common_pm_late_init; > > since that's the only function you're calling. > For now... As the PM support gets added there will be other function calls and that can and is at times soc specific. Regards, Vaibhav
On 10/10/2013 08:32 AM, Nishanth Menon wrote: > On 00:32-20131010, Joel Fernandes wrote: >> On 10/09/2013 06:24 PM, Nishanth Menon wrote: >>> Call OMAP2+ generic lateinit hook from AM specific late init hook. >>> This allows the generic late initializations such as cpufreq hooks >>> to be active. >>> >>> Cc: Benoit Cousson <bcousson@baylibre.com> >>> Cc: Kevin Hilman <khilman@deeprootsystems.com> >>> Cc: Paul Walmsley <paul@pwsan.com> >>> Cc: Tony Lindgren <tony@atomide.com> >>> Signed-off-by: Nishanth Menon <nm@ti.com> >>> --- >>> arch/arm/mach-omap2/board-generic.c | 1 + >>> arch/arm/mach-omap2/common.h | 1 + >>> arch/arm/mach-omap2/io.c | 6 ++++++ >>> 3 files changed, 8 insertions(+) >>> >>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c >>> index 87162e1..b474498 100644 >>> --- a/arch/arm/mach-omap2/board-generic.c >>> +++ b/arch/arm/mach-omap2/board-generic.c >>> @@ -180,6 +180,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)") >>> .init_irq = omap_intc_of_init, >>> .handle_irq = omap3_intc_handle_irq, >>> .init_machine = omap_generic_init, >>> + .init_late = am33xx_init_late, >> >> Instead of adding a new a *_init_late function for every platform, perhaps >> better to just do: >> .init_late = omap2_common_pm_late_init; >> >> since that's the only function you're calling. >> >> Later if more functions are added, then it can be wrapped around. > > And what benefit would that give us? we break consistency of functions > available in io.c, considering the work we have already done in > out-of-tree patches on ti-forks, we *do know* that there is more > incoming and has to be done anyways, I prefer having symmetric > functions and a placeholder that folks can add on to. > Ok, sure if you think such placeholders are right way and there's more code later to be added to init_late callbacks. Adding 6 lines of identical code for every platform seems redundant, I'd just define an omap_common_late_init for all platforms for now, and then fork off for the future differences. That way we save on LOC and readability in the header as well. Your call. thanks, -Joel
On 10/10/2013 10:20 AM, Joel Fernandes wrote: > On 10/10/2013 08:32 AM, Nishanth Menon wrote: >> On 00:32-20131010, Joel Fernandes wrote: >>> On 10/09/2013 06:24 PM, Nishanth Menon wrote: >>>> Call OMAP2+ generic lateinit hook from AM specific late init hook. >>>> This allows the generic late initializations such as cpufreq hooks >>>> to be active. >>>> >>>> Cc: Benoit Cousson <bcousson@baylibre.com> >>>> Cc: Kevin Hilman <khilman@deeprootsystems.com> >>>> Cc: Paul Walmsley <paul@pwsan.com> >>>> Cc: Tony Lindgren <tony@atomide.com> >>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>> --- >>>> arch/arm/mach-omap2/board-generic.c | 1 + >>>> arch/arm/mach-omap2/common.h | 1 + >>>> arch/arm/mach-omap2/io.c | 6 ++++++ >>>> 3 files changed, 8 insertions(+) >>>> >>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c >>>> index 87162e1..b474498 100644 >>>> --- a/arch/arm/mach-omap2/board-generic.c >>>> +++ b/arch/arm/mach-omap2/board-generic.c >>>> @@ -180,6 +180,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)") >>>> .init_irq = omap_intc_of_init, >>>> .handle_irq = omap3_intc_handle_irq, >>>> .init_machine = omap_generic_init, >>>> + .init_late = am33xx_init_late, >>> >>> Instead of adding a new a *_init_late function for every platform, perhaps >>> better to just do: >>> .init_late = omap2_common_pm_late_init; >>> >>> since that's the only function you're calling. >>> >>> Later if more functions are added, then it can be wrapped around. >> >> And what benefit would that give us? we break consistency of functions >> available in io.c, considering the work we have already done in >> out-of-tree patches on ti-forks, we *do know* that there is more >> incoming and has to be done anyways, I prefer having symmetric >> functions and a placeholder that folks can add on to. >> > > Ok, sure if you think such placeholders are right way and there's more code > later to be added to init_late callbacks. > > Adding 6 lines of identical code for every platform seems redundant, I'd just > define an omap_common_late_init for all platforms for now, and then fork off for > the future differences. That way we save on LOC and readability in the header as > well. Your call. I see a function of that name already exists. I guess you can leave your patch as is then and not have to do this. regards, -Joel
On Thu, Oct 10, 2013 at 10:23 AM, Joel Fernandes <joelf@ti.com> wrote: > I see a function of that name already exists. I guess you can leave your patch > as is then and not have to do this. Can I consider that as an Acked-by :) ? Regards, Nishanth Menon
On 10/10/2013 10:45 AM, Nishanth Menon wrote: > On Thu, Oct 10, 2013 at 10:23 AM, Joel Fernandes <joelf@ti.com> wrote: >> I see a function of that name already exists. I guess you can leave your patch >> as is then and not have to do this. > > > Can I consider that as an Acked-by :) ? Yes, sure, for this particular patch: Acked-by: Joel Fernandes <joelf@ti.com> thanks, -Joel
* Joel Fernandes <joelf@ti.com> [131010 08:31]: > On 10/10/2013 10:20 AM, Joel Fernandes wrote: > > > > Adding 6 lines of identical code for every platform seems redundant, I'd just > > define an omap_common_late_init for all platforms for now, and then fork off for > > the future differences. That way we save on LOC and readability in the header as > > well. Your call. > > I see a function of that name already exists. I guess you can leave your patch > as is then and not have to do this. Yeah but why not just call the common late_init function directly for the cases where we know it's never needed. If thoses will be populated in the later patches, then it's OK to add them. In any case, Nishant, can you please squash the late_init patches into a single patch and make sure it also applies on top of the omap-for-v3.13/quirk branch as there's been some changes to board-generic.c there. Regards, Tony
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c index 87162e1..b474498 100644 --- a/arch/arm/mach-omap2/board-generic.c +++ b/arch/arm/mach-omap2/board-generic.c @@ -180,6 +180,7 @@ DT_MACHINE_START(AM33XX_DT, "Generic AM33XX (Flattened Device Tree)") .init_irq = omap_intc_of_init, .handle_irq = omap3_intc_handle_irq, .init_machine = omap_generic_init, + .init_late = am33xx_init_late, .init_time = omap3_gptimer_timer_init, .dt_compat = am33xx_boards_compat, .restart = am33xx_restart, diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h index 4a5684b..049c58d 100644 --- a/arch/arm/mach-omap2/common.h +++ b/arch/arm/mach-omap2/common.h @@ -109,6 +109,7 @@ void omap35xx_init_late(void); void omap3630_init_late(void); void am35xx_init_late(void); void ti81xx_init_late(void); +void am33xx_init_late(void); int omap2_common_pm_late_init(void); void dra7xx_init_early(void); diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c index a9896ea..38678bf 100644 --- a/arch/arm/mach-omap2/io.c +++ b/arch/arm/mach-omap2/io.c @@ -594,6 +594,12 @@ void __init am33xx_init_early(void) omap_hwmod_init_postsetup(); omap_clk_init = am33xx_dt_clk_init; } + +void __init am33xx_init_late(void) +{ + omap2_common_pm_late_init(); +} + #endif #ifdef CONFIG_SOC_AM43XX
Call OMAP2+ generic lateinit hook from AM specific late init hook. This allows the generic late initializations such as cpufreq hooks to be active. Cc: Benoit Cousson <bcousson@baylibre.com> Cc: Kevin Hilman <khilman@deeprootsystems.com> Cc: Paul Walmsley <paul@pwsan.com> Cc: Tony Lindgren <tony@atomide.com> Signed-off-by: Nishanth Menon <nm@ti.com> --- arch/arm/mach-omap2/board-generic.c | 1 + arch/arm/mach-omap2/common.h | 1 + arch/arm/mach-omap2/io.c | 6 ++++++ 3 files changed, 8 insertions(+)