| Message ID | 1365166743-5940-3-git-send-email-santosh.shilimkar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 05 April 2013 06:51 PM, Felipe Balbi wrote: > On Fri, Apr 05, 2013 at 06:29:01PM +0530, Santosh Shilimkar wrote: >> OMAP5 has backward compatible PRCM block and it's programming >> model is mostly similar to OMAP4. Same is going to be maintained >> for future OMAP4 based SOCs. Hence consolidate the OMAP4 power >> management code so that it can be re-used on OMAP5 and later devices. >> >> While at it, update the kernel-doc for omap4_pm_init(). >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- >> arch/arm/mach-omap2/Makefile | 9 +-- >> arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 58 ++++++++++++++++---- >> .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 0 >> 3 files changed, 53 insertions(+), 14 deletions(-) >> rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (83%) >> rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (100%) >> >> diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile >> index b068b7f..3e59895 100644 >> --- a/arch/arm/mach-omap2/Makefile >> +++ b/arch/arm/mach-omap2/Makefile >> @@ -35,14 +35,14 @@ obj-$(CONFIG_SOC_HAS_OMAP2_SDRC) += sdrc.o >> obj-$(CONFIG_SMP) += omap-smp.o omap-headsmp.o >> obj-$(CONFIG_HOTPLUG_CPU) += omap-hotplug.o >> omap-4-5-common = omap4-common.o omap-wakeupgen.o \ >> - sleep44xx.o >> + sleep_omap4plus.o >> obj-$(CONFIG_ARCH_OMAP4) += $(omap-4-5-common) >> obj-$(CONFIG_SOC_OMAP5) += $(omap-4-5-common) >> >> plus_sec := $(call as-instr,.arch_extension sec,+sec) >> AFLAGS_omap-headsmp.o :=-Wa,-march=armv7-a$(plus_sec) >> AFLAGS_omap-smc.o :=-Wa,-march=armv7-a$(plus_sec) >> -AFLAGS_sleep44xx.o :=-Wa,-march=armv7-a$(plus_sec) >> +AFLAGS_sleep_omap4plus.o :=-Wa,-march=armv7-a$(plus_sec) >> >> # Functions loaded to SRAM >> obj-$(CONFIG_SOC_OMAP2420) += sram242x.o >> @@ -80,11 +80,12 @@ endif >> obj-$(CONFIG_OMAP_PM_NOOP) += omap-pm-noop.o >> >> ifeq ($(CONFIG_PM),y) >> +omap4plus-common-pm = omap-mpuss-lowpower.o pm_omap4plus.o >> obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o >> obj-$(CONFIG_ARCH_OMAP2) += sleep24xx.o >> obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o >> -obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o omap-mpuss-lowpower.o >> -obj-$(CONFIG_SOC_OMAP5) += omap-mpuss-lowpower.o >> +obj-$(CONFIG_ARCH_OMAP4) += $(omap4plus-common-pm) >> +obj-$(CONFIG_SOC_OMAP5) += $(omap4plus-common-pm) >> obj-$(CONFIG_PM_DEBUG) += pm-debug.o >> >> obj-$(CONFIG_POWER_AVS_OMAP) += sr_device.o >> diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm_omap4plus.c >> similarity index 83% >> rename from arch/arm/mach-omap2/pm44xx.c >> rename to arch/arm/mach-omap2/pm_omap4plus.c >> index 5ba6d88..228deca 100644 >> --- a/arch/arm/mach-omap2/pm44xx.c >> +++ b/arch/arm/mach-omap2/pm_omap4plus.c >> @@ -1,7 +1,7 @@ >> /* >> - * OMAP4 Power Management Routines >> + * OMAP4+ Power Management Routines >> * >> - * Copyright (C) 2010-2011 Texas Instruments, Inc. >> + * Copyright (C) 2010-2013 Texas Instruments, Inc. >> * Rajendra Nayak <rnayak@ti.com> >> * Santosh Shilimkar <santosh.shilimkar@ti.com> >> * >> @@ -135,16 +135,16 @@ static void omap_default_idle(void) >> } >> >> /** >> - * omap4_pm_init - Init routine for OMAP4 PM >> + * omap4_init_static_deps - Add OMAP4 static dependencies >> * >> - * Initializes all powerdomain and clockdomain target states >> - * and all PRCM settings. >> + * Add needed static clockdomain dependencies on OMAP4 devices. >> + * Return: 0 on success or 'err' on failures >> */ >> -int __init omap4_pm_init(void) >> +static inline int omap4_init_static_deps(void) > > you can let compiler inline this if it chooses to, right ?? Otherwise, > if you really want to make sure this will be inlined, you could(should?) > be using __always_inline. > Compilers are smart now so that fine.
* Santosh Shilimkar <santosh.shilimkar@ti.com> [130405 06:01]: > OMAP5 has backward compatible PRCM block and it's programming > model is mostly similar to OMAP4. Same is going to be maintained > for future OMAP4 based SOCs. Hence consolidate the OMAP4 power > management code so that it can be re-used on OMAP5 and later devices. > > While at it, update the kernel-doc for omap4_pm_init(). > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/mach-omap2/Makefile | 9 +-- > arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 58 ++++++++++++++++---- > .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 0 > 3 files changed, 53 insertions(+), 14 deletions(-) > rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (83%) > rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (100%) I suggest you leave out the rename. That just adds churn and has flame potential. Regards, Tony
On Saturday 06 April 2013 03:04 AM, Tony Lindgren wrote: > * Santosh Shilimkar <santosh.shilimkar@ti.com> [130405 06:01]: >> OMAP5 has backward compatible PRCM block and it's programming >> model is mostly similar to OMAP4. Same is going to be maintained >> for future OMAP4 based SOCs. Hence consolidate the OMAP4 power >> management code so that it can be re-used on OMAP5 and later devices. >> >> While at it, update the kernel-doc for omap4_pm_init(). >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- >> arch/arm/mach-omap2/Makefile | 9 +-- >> arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 58 ++++++++++++++++---- >> .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 0 >> 3 files changed, 53 insertions(+), 14 deletions(-) >> rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (83%) >> rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (100%) > > I suggest you leave out the rename. That just adds churn and has > flame potential. > OK. I can leave that mow but have to do renames anyways when I add OMAP5 support. OMAP54XX support inside pm44xx.c and sleep44x.S will be really odd. Let me know if you have concern on renaming it while OMAP5 support gets added ? Regards, Santosh
* Santosh Shilimkar <santosh.shilimkar@ti.com> [130408 03:51]: > On Saturday 06 April 2013 03:04 AM, Tony Lindgren wrote: > > * Santosh Shilimkar <santosh.shilimkar@ti.com> [130405 06:01]: > >> OMAP5 has backward compatible PRCM block and it's programming > >> model is mostly similar to OMAP4. Same is going to be maintained > >> for future OMAP4 based SOCs. Hence consolidate the OMAP4 power > >> management code so that it can be re-used on OMAP5 and later devices. > >> > >> While at it, update the kernel-doc for omap4_pm_init(). > >> > >> Acked-by: Nishanth Menon <nm@ti.com> > >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > >> --- > >> arch/arm/mach-omap2/Makefile | 9 +-- > >> arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 58 ++++++++++++++++---- > >> .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 0 > >> 3 files changed, 53 insertions(+), 14 deletions(-) > >> rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (83%) > >> rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (100%) > > > > I suggest you leave out the rename. That just adds churn and has > > flame potential. > > > OK. I can leave that mow but have to do renames anyways when > I add OMAP5 support. OMAP54XX support inside pm44xx.c and sleep44x.S > will be really odd. Well that should be just fine if the hardware is the same. > Let me know if you have concern on renaming it while OMAP5 support > gets added ? If the rename is done, it should have a clear reason to do it in a separate patch. IMHO it's just fine to have omap4 in some name and then assume that at least some follow up SoCs also use that code. In the worst case we may end up renaming things many times: omap-xyz.c -> omap2-xyz.c -> omap2plus-xyz.c -> omap2-to-4-xyz.c -> omap2-to-4-and-am35xx-xyz.c etc :) If we rename something, the description should have a clear reason for doing it like "To avoid confusing between PM code that does not have support for bootrom assisted off-idle on SMP omaps with PM code that's not bootrom assisted, let's rename foo to.." For the naming, the only safe naming convention is to use something actually describing the piece of hardware. Maybe some combination of bootrom-assisted-off-idle + SMP in this case if there's no actual name for this hwblock? Regards, Tony
On Monday 08 April 2013 10:12 PM, Tony Lindgren wrote: > * Santosh Shilimkar <santosh.shilimkar@ti.com> [130408 03:51]: >> On Saturday 06 April 2013 03:04 AM, Tony Lindgren wrote: >>> * Santosh Shilimkar <santosh.shilimkar@ti.com> [130405 06:01]: >>>> OMAP5 has backward compatible PRCM block and it's programming >>>> model is mostly similar to OMAP4. Same is going to be maintained >>>> for future OMAP4 based SOCs. Hence consolidate the OMAP4 power >>>> management code so that it can be re-used on OMAP5 and later devices. >>>> >>>> While at it, update the kernel-doc for omap4_pm_init(). >>>> >>>> Acked-by: Nishanth Menon <nm@ti.com> >>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >>>> --- >>>> arch/arm/mach-omap2/Makefile | 9 +-- >>>> arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 58 ++++++++++++++++---- >>>> .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 0 >>>> 3 files changed, 53 insertions(+), 14 deletions(-) >>>> rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (83%) >>>> rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (100%) >>> >>> I suggest you leave out the rename. That just adds churn and has >>> flame potential. >>> >> OK. I can leave that mow but have to do renames anyways when >> I add OMAP5 support. OMAP54XX support inside pm44xx.c and sleep44x.S >> will be really odd. > > Well that should be just fine if the hardware is the same. > >> Let me know if you have concern on renaming it while OMAP5 support >> gets added ? > > If the rename is done, it should have a clear reason to do it in > a separate patch. IMHO it's just fine to have omap4 in some name and > then assume that at least some follow up SoCs also use that code. > In the worst case we may end up renaming things many times: > Agree. omap4_* is just fine and thats why many omap4_* are not renamed. Here the files were calling specific family of OMAP4 i.e OMAP44XX and hence the rename was appropriate. > omap-xyz.c -> omap2-xyz.c -> omap2plus-xyz.c -> omap2-to-4-xyz.c -> > omap2-to-4-and-am35xx-xyz.c etc :) > pm_omap4plus.c and sleep_omap4plus.S was chosen specifically considering the OMAP4+ devices share the PRCM IP. But PRCM_IP itself isn't new so calling by IP wasn't an option. > If we rename something, the description should have a clear reason > for doing it like "To avoid confusing between PM code that does not > have support for bootrom assisted off-idle on SMP omaps with PM code > that's not bootrom assisted, let's rename foo to.." > > For the naming, the only safe naming convention is to use something > actually describing the piece of hardware. Maybe some combination > of bootrom-assisted-off-idle + SMP in this case if there's no actual > name for this hwblock? > As I said, the IP has been there from OMAP2XX days. Here the case that IP version is very similar between OMAP4, OMAP5. DRA(next SOC) and its derivatives. Hence can share most of the code. I thought this was good enough reason considering at least 4 family of SOC's can make use of the code. It has nothing to do with SMP etc specifically and rather the similarity between the PM infrastructure on the mentioned SOCs. Let me know if you can suggested better name than what I chose ? Regards, Santosh
* Santosh Shilimkar <santosh.shilimkar@ti.com> [130408 23:16]: > > > As I said, the IP has been there from OMAP2XX days. Here the case that > IP version is very similar between OMAP4, OMAP5. DRA(next SOC) and its > derivatives. Hence can share most of the code. I thought this was good > enough reason considering at least 4 family of SOC's can make use > of the code. Well at least that might be enough of a reasoning to rename it as it is somewhat futureproof. > It has nothing to do with SMP etc specifically and rather the similarity > between the PM infrastructure on the mentioned SOCs. > > Let me know if you can suggested better name than what I chose ? Not really except something like pm-iprevXXX.[chS] where the rev is the first revision that it works with? But then again if it's touching registers all over the place directly, that naming does not make much sense either :) Regards, Tony
On Tuesday 09 April 2013 10:25 PM, Tony Lindgren wrote: > * Santosh Shilimkar <santosh.shilimkar@ti.com> [130408 23:16]: >>> >> As I said, the IP has been there from OMAP2XX days. Here the case that >> IP version is very similar between OMAP4, OMAP5. DRA(next SOC) and its >> derivatives. Hence can share most of the code. I thought this was good >> enough reason considering at least 4 family of SOC's can make use >> of the code. > > Well at least that might be enough of a reasoning to rename it as > it is somewhat futureproof. > Yep. I can't see to much further in future but there is a *strong* mandate to not break compatibility so hopefully we won't see too much churn for similar IP blocks in future. >> It has nothing to do with SMP etc specifically and rather the similarity >> between the PM infrastructure on the mentioned SOCs. >> >> Let me know if you can suggested better name than what I chose ? > > Not really except something like pm-iprevXXX.[chS] where the rev is > the first revision that it works with? > > But then again if it's touching registers all over the place directly, > that naming does not make much sense either :) > Exactly. Thanks for the discussion. I will go ahead with rename when we add OMAP5 stuff like I did in earlier patch. i.e pm44xx.c --> pm_omap4plus.c sleep44xx.S --> sleep_omap4plus.S Regrads, Santosh
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index b068b7f..3e59895 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -35,14 +35,14 @@ obj-$(CONFIG_SOC_HAS_OMAP2_SDRC) += sdrc.o obj-$(CONFIG_SMP) += omap-smp.o omap-headsmp.o obj-$(CONFIG_HOTPLUG_CPU) += omap-hotplug.o omap-4-5-common = omap4-common.o omap-wakeupgen.o \ - sleep44xx.o + sleep_omap4plus.o obj-$(CONFIG_ARCH_OMAP4) += $(omap-4-5-common) obj-$(CONFIG_SOC_OMAP5) += $(omap-4-5-common) plus_sec := $(call as-instr,.arch_extension sec,+sec) AFLAGS_omap-headsmp.o :=-Wa,-march=armv7-a$(plus_sec) AFLAGS_omap-smc.o :=-Wa,-march=armv7-a$(plus_sec) -AFLAGS_sleep44xx.o :=-Wa,-march=armv7-a$(plus_sec) +AFLAGS_sleep_omap4plus.o :=-Wa,-march=armv7-a$(plus_sec) # Functions loaded to SRAM obj-$(CONFIG_SOC_OMAP2420) += sram242x.o @@ -80,11 +80,12 @@ endif obj-$(CONFIG_OMAP_PM_NOOP) += omap-pm-noop.o ifeq ($(CONFIG_PM),y) +omap4plus-common-pm = omap-mpuss-lowpower.o pm_omap4plus.o obj-$(CONFIG_ARCH_OMAP2) += pm24xx.o obj-$(CONFIG_ARCH_OMAP2) += sleep24xx.o obj-$(CONFIG_ARCH_OMAP3) += pm34xx.o sleep34xx.o -obj-$(CONFIG_ARCH_OMAP4) += pm44xx.o omap-mpuss-lowpower.o -obj-$(CONFIG_SOC_OMAP5) += omap-mpuss-lowpower.o +obj-$(CONFIG_ARCH_OMAP4) += $(omap4plus-common-pm) +obj-$(CONFIG_SOC_OMAP5) += $(omap4plus-common-pm) obj-$(CONFIG_PM_DEBUG) += pm-debug.o obj-$(CONFIG_POWER_AVS_OMAP) += sr_device.o diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm_omap4plus.c similarity index 83% rename from arch/arm/mach-omap2/pm44xx.c rename to arch/arm/mach-omap2/pm_omap4plus.c index 5ba6d88..228deca 100644 --- a/arch/arm/mach-omap2/pm44xx.c +++ b/arch/arm/mach-omap2/pm_omap4plus.c @@ -1,7 +1,7 @@ /* - * OMAP4 Power Management Routines + * OMAP4+ Power Management Routines * - * Copyright (C) 2010-2011 Texas Instruments, Inc. + * Copyright (C) 2010-2013 Texas Instruments, Inc. * Rajendra Nayak <rnayak@ti.com> * Santosh Shilimkar <santosh.shilimkar@ti.com> * @@ -135,16 +135,16 @@ static void omap_default_idle(void) } /** - * omap4_pm_init - Init routine for OMAP4 PM + * omap4_init_static_deps - Add OMAP4 static dependencies * - * Initializes all powerdomain and clockdomain target states - * and all PRCM settings. + * Add needed static clockdomain dependencies on OMAP4 devices. + * Return: 0 on success or 'err' on failures */ -int __init omap4_pm_init(void) +static inline int omap4_init_static_deps(void) { - int ret; struct clockdomain *emif_clkdm, *mpuss_clkdm, *l3_1_clkdm; struct clockdomain *ducati_clkdm, *l3_2_clkdm; + int ret = 0; if (omap_rev() == OMAP4430_REV_ES1_0) { WARN(1, "Power Management not supported on OMAP4430 ES1.0\n"); @@ -163,7 +163,7 @@ int __init omap4_pm_init(void) ret = pwrdm_for_each(pwrdms_setup, NULL); if (ret) { pr_err("Failed to setup powerdomains\n"); - goto err2; + return ret; } /* @@ -171,6 +171,10 @@ int __init omap4_pm_init(void) * MPUSS -> L4_PER/L3_* and DUCATI -> L3_* doesn't work as * expected. The hardware recommendation is to enable static * dependencies for these to avoid system lock ups or random crashes. + * The L4 wakeup depedency is added to workaround the OCP sync hardware + * BUG with 32K synctimer which lead to incorrect timer value read + * from the 32K counter. The BUG applies for GPTIMER1 and WDT2 which + * are part of L4 wakeup clockdomain. */ mpuss_clkdm = clkdm_lookup("mpuss_clkdm"); emif_clkdm = clkdm_lookup("l3_emif_clkdm"); @@ -179,7 +183,7 @@ int __init omap4_pm_init(void) ducati_clkdm = clkdm_lookup("ducati_clkdm"); if ((!mpuss_clkdm) || (!emif_clkdm) || (!l3_1_clkdm) || (!l3_2_clkdm) || (!ducati_clkdm)) - goto err2; + return -EINVAL; ret = clkdm_add_wkdep(mpuss_clkdm, emif_clkdm); ret |= clkdm_add_wkdep(mpuss_clkdm, l3_1_clkdm); @@ -188,9 +192,42 @@ int __init omap4_pm_init(void) ret |= clkdm_add_wkdep(ducati_clkdm, l3_2_clkdm); if (ret) { pr_err("Failed to add MPUSS -> L3/EMIF/L4PER, DUCATI -> L3 wakeup dependency\n"); + return -EINVAL; + } + + return ret; +} + +/** + * omap4_pm_init - Init routine for OMAP4+ devices + * + * Initializes all powerdomain and clockdomain target states + * and all PRCM settings. + * Return: Returns the error code returned by called functions. + */ +int __init omap4_pm_init(void) +{ + int ret = 0; + + if (omap_rev() == OMAP4430_REV_ES1_0) { + WARN(1, "Power Management not supported on OMAP4430 ES1.0\n"); + return -ENODEV; + } + + pr_info("Power Management for TI OMAP4+ devices.\n"); + + ret = pwrdm_for_each(pwrdms_setup, NULL); + if (ret) { + pr_err("Failed to setup powerdomains.\n"); goto err2; } + if (cpu_is_omap44xx()) { + ret = omap4_init_static_deps(); + if (ret) + goto err2; + } + ret = omap4_mpuss_init(); if (ret) { pr_err("Failed to initialise OMAP4 MPUSS\n"); @@ -206,7 +243,8 @@ int __init omap4_pm_init(void) /* Overwrite the default cpu_do_idle() */ arm_pm_idle = omap_default_idle; - omap4_idle_init(); + if (cpu_is_omap44xx()) + omap4_idle_init(); err2: return ret;