| Message ID | 1364205910-32392-1-git-send-email-santosh.shilimkar@ti.com |
|---|---|
| State | New |
| Headers | show |
Hi Santosh, On Monday 25 March 2013 03:34 PM, Santosh Shilimkar wrote: > Kevin, > > Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted > earlier (March 1st 2013). Patch-set incorporates comments from Nishant > Menon (Thanks for review NM) and his acked-by tags. I would like to get this > queued for 3.10 merge window if you are ok with the series. > > Series is built on top of my pull requests [1] [2] [3] sent to Tony and your > 'for_3.10/pm/cleanup' branch. For testing, I have created a branch [4] > which put together all the needed dependencies, fixes which should make it > to 3.10 merge window. > > Series adds OMAP5 MPUSS power management support for system wide suspend > and CPUidle. Its heavy re-use from OMAP4 and hence only ~400 odd lines are > needed to add OMAP5 PM support on top of existing OMAP4 PM support. > > OMAP5 adds a mercury retention feature which is an enhancement of > existing retention feature to reduce the leakage. No change in > programming model except one time enabling of mercury retention > during init. > > One more notable change in OMAP5 vs OMAP4 devices, CPUx power domains > support retention state which lets you hit MPUSS and Core retention with > very low latency C-states. > > Tested on OMAP4430 SDP, OMAP4460 Panda, OMAP5430 SDP and OMAP5432 Panda > devices with suspend and CPUIdle. Rootfs is mounted over ramdisk since > the mmc and nfs based fs needs DMA engine patches. For suspend wakeup, > I used Sourav's couple of serial wakeup wip patches from the lists. I did the following build tests on [1]: -> Native omap2plus build -> Omap2 only build -> Omap3 only build -> Omap4 only build -> Omap5 only build -> AM33XX only build. -> omap1_defconfig And also did functional testing on [2] where omap5_pm branch[1] is merged. On OMAP5430 EVM: Suspend to RAM (UART wakeup) CPU_IDLE On OMAP4430 SDP: Suspend to RAM (UART wakeup) CPU_IDLE Note: 1) Disabled SMP for doing build test on Omap2/3 only builds. 2) If we enable CPU_IDLE on OMAP4430, debug message flood from reset_ctrl_regs() will appear. As this is already disussed and a patch is already sent on Mainline Will get more info on this here[3] Tested-by: Lokesh Vutla <lokeshvutla@ti.com> Thanks and Regards, Lokesh [1] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux for_3.10/omap5_pm [2] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git testing/3.10/omap5_int [3] https://lkml.org/lkml/2013/3/13/50 > > The following changes since commit 9981cde24de52e5bb9a7cd918d1e54de241f85c9: > > Merge branch 'for_3.10/omap5_data_files' of git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux into for_3.10/omap5_pm (2013-03-25 12:29:34 +0530) > > are available in the git repository at: > > > git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git for_3.10/omap5_pm > > for you to fetch changes up to 75bd846d3103da858a208fe07127151903d1f608: > > ARM: OMAP5: PM: handle device instance for warm reset (2013-03-25 12:37:44 +0530) > > ---------------------------------------------------------------- > Nishanth Menon (1): > ARM: OMAP5: PM: handle device instance for warm reset > > Santosh Shilimkar (17): > ARM: OMAP4+: PM: Consolidate MPU subsystem PM code for re-use > ARM: OMAP5: PM: Update CPU context register offset > ARM: OMAP4+: PM: Consolidate and use OMAP4 PM code for OMAP5 > ARM: OMAP5: PM: Set MPUSS-EMIF clock-domain static dependency > ARM: OMAP5: PM: Enables ES2 PM mode by default > ARM: OMAP5: PM: Enable Mercury retention mode on CPUx powerdomains > ARM: OMAP5: Add init_late() hook to enable pm initialization > ARM: OMAP5: PM: Add CPU power off in hotplug path > ARM: OMAP4+: PM: Restore CPU power state to ON with clockdomain force > wakeup method > ARM: OMAP5: PM: Add MPU Open Switch Retention support > ARM: OMAP5: PM: Add L2 memory power down support > ARM: OMAP4: CPUidle: Avoid double idle driver registration > ARM: OMAP: CPUidle: Unregister drivere on device registration failure > ARM: OMAP4: CPUidle: Make C-state description field more precise > ARM: OMAP4+: CPUidle: Consolidate idle driver for OMAP5 support > ARM: OMAP4+: CPUidle: Deprecate use of > omap4_mpuss_read_prev_context_state() > ARM: OMAP4+: CPUidle: Add OMAP5 idle driver support > > arch/arm/mach-omap2/Kconfig | 1 + > arch/arm/mach-omap2/Makefile | 12 +- > arch/arm/mach-omap2/board-generic.c | 1 + > arch/arm/mach-omap2/common.h | 8 +- > arch/arm/mach-omap2/cpuidle34xx.c | 6 +- > .../{cpuidle44xx.c => cpuidle_omap4plus.c} | 151 ++++++++++++++++--- > arch/arm/mach-omap2/io.c | 8 + > arch/arm/mach-omap2/omap-mpuss-lowpower.c | 155 +++++++++++++++----- > arch/arm/mach-omap2/omap-secure.h | 9 ++ > arch/arm/mach-omap2/omap-smp.c | 12 +- > arch/arm/mach-omap2/omap-wakeupgen.c | 30 +++- > arch/arm/mach-omap2/omap-wakeupgen.h | 1 + > arch/arm/mach-omap2/omap4-sar-layout.h | 2 + > arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 90 ++++++++++-- > arch/arm/mach-omap2/prminst44xx.c | 10 +- > .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 106 +++++++++++++ > 16 files changed, 512 insertions(+), 90 deletions(-) > rename arch/arm/mach-omap2/{cpuidle44xx.c => cpuidle_omap4plus.c} (58%) > rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (74%) > rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (77%) >
On Monday 25 March 2013 05:16 PM, Lokesh Vutla wrote: > Hi Santosh, > > On Monday 25 March 2013 03:34 PM, Santosh Shilimkar wrote: >> Kevin, >> >> Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted >> earlier (March 1st 2013). Patch-set incorporates comments from Nishant >> Menon (Thanks for review NM) and his acked-by tags. I would like to get this >> queued for 3.10 merge window if you are ok with the series. >> >> Series is built on top of my pull requests [1] [2] [3] sent to Tony and your >> 'for_3.10/pm/cleanup' branch. For testing, I have created a branch [4] >> which put together all the needed dependencies, fixes which should make it >> to 3.10 merge window. >> >> Series adds OMAP5 MPUSS power management support for system wide suspend >> and CPUidle. Its heavy re-use from OMAP4 and hence only ~400 odd lines are >> needed to add OMAP5 PM support on top of existing OMAP4 PM support. >> >> OMAP5 adds a mercury retention feature which is an enhancement of >> existing retention feature to reduce the leakage. No change in >> programming model except one time enabling of mercury retention >> during init. >> >> One more notable change in OMAP5 vs OMAP4 devices, CPUx power domains >> support retention state which lets you hit MPUSS and Core retention with >> very low latency C-states. >> >> Tested on OMAP4430 SDP, OMAP4460 Panda, OMAP5430 SDP and OMAP5432 Panda >> devices with suspend and CPUIdle. Rootfs is mounted over ramdisk since >> the mmc and nfs based fs needs DMA engine patches. For suspend wakeup, >> I used Sourav's couple of serial wakeup wip patches from the lists. > > I did the following build tests on [1]: > -> Native omap2plus build > -> Omap2 only build > -> Omap3 only build > -> Omap4 only build > -> Omap5 only build > -> AM33XX only build. > -> omap1_defconfig > Thanks for the build coverage. > And also did functional testing on [2] where omap5_pm branch[1] is merged. > On OMAP5430 EVM: Suspend to RAM (UART wakeup) > CPU_IDLE > On OMAP4430 SDP: Suspend to RAM (UART wakeup) > CPU_IDLE Excellent. > Note: > 1) Disabled SMP for doing build test on Omap2/3 only builds. I noticed this one as well. > 2) If we enable CPU_IDLE on OMAP4430, debug message flood from reset_ctrl_regs() will appear. > As this is already disussed and a patch is already sent on Mainline > Will get more info on this here[3] > Yep. I applied the patch while testing. The patch is already in RMK's queue as per Will D. > Tested-by: Lokesh Vutla <lokeshvutla@ti.com> > Thanks a bunch for detailed testing and your tested-by tag. Regards, Santosh > > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux for_3.10/omap5_pm > [2] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git testing/3.10/omap5_int > [3] https://lkml.org/lkml/2013/3/13/50 >
Hi Santosh, On Monday 25 March 2013 03:34 PM, Santosh Shilimkar wrote: > Kevin, > > Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted > earlier (March 1st 2013). Patch-set incorporates comments from Nishant > Menon (Thanks for review NM) and his acked-by tags. I would like to get this > queued for 3.10 merge window if you are ok with the series. > > Series is built on top of my pull requests [1] [2] [3] sent to Tony and your > 'for_3.10/pm/cleanup' branch. For testing, I have created a branch [4] > which put together all the needed dependencies, fixes which should make it > to 3.10 merge window. > > Series adds OMAP5 MPUSS power management support for system wide suspend > and CPUidle. Its heavy re-use from OMAP4 and hence only ~400 odd lines are > needed to add OMAP5 PM support on top of existing OMAP4 PM support. > > OMAP5 adds a mercury retention feature which is an enhancement of > existing retention feature to reduce the leakage. No change in > programming model except one time enabling of mercury retention > during init. > > One more notable change in OMAP5 vs OMAP4 devices, CPUx power domains > support retention state which lets you hit MPUSS and Core retention with > very low latency C-states. > > Tested on OMAP4430 SDP, OMAP4460 Panda, OMAP5430 SDP and OMAP5432 Panda > devices with suspend and CPUIdle. Rootfs is mounted over ramdisk since > the mmc and nfs based fs needs DMA engine patches. For suspend wakeup, > I used Sourav's couple of serial wakeup wip patches from the lists. > > The following changes since commit 9981cde24de52e5bb9a7cd918d1e54de241f85c9: > > Merge branch 'for_3.10/omap5_data_files' of git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux into for_3.10/omap5_pm (2013-03-25 12:29:34 +0530) > > are available in the git repository at: > > > git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git for_3.10/omap5_pm > > for you to fetch changes up to 75bd846d3103da858a208fe07127151903d1f608: > > ARM: OMAP5: PM: handle device instance for warm reset (2013-03-25 12:37:44 +0530) > > ---------------------------------------------------------------- > Nishanth Menon (1): > ARM: OMAP5: PM: handle device instance for warm reset > > Santosh Shilimkar (17): > ARM: OMAP4+: PM: Consolidate MPU subsystem PM code for re-use > ARM: OMAP5: PM: Update CPU context register offset > ARM: OMAP4+: PM: Consolidate and use OMAP4 PM code for OMAP5 > ARM: OMAP5: PM: Set MPUSS-EMIF clock-domain static dependency > ARM: OMAP5: PM: Enables ES2 PM mode by default > ARM: OMAP5: PM: Enable Mercury retention mode on CPUx powerdomains > ARM: OMAP5: Add init_late() hook to enable pm initialization > ARM: OMAP5: PM: Add CPU power off in hotplug path > ARM: OMAP4+: PM: Restore CPU power state to ON with clockdomain force > wakeup method > ARM: OMAP5: PM: Add MPU Open Switch Retention support > ARM: OMAP5: PM: Add L2 memory power down support > ARM: OMAP4: CPUidle: Avoid double idle driver registration > ARM: OMAP: CPUidle: Unregister drivere on device registration failure > ARM: OMAP4: CPUidle: Make C-state description field more precise > ARM: OMAP4+: CPUidle: Consolidate idle driver for OMAP5 support > ARM: OMAP4+: CPUidle: Deprecate use of > omap4_mpuss_read_prev_context_state() > ARM: OMAP4+: CPUidle: Add OMAP5 idle driver support > > arch/arm/mach-omap2/Kconfig | 1 + > arch/arm/mach-omap2/Makefile | 12 +- > arch/arm/mach-omap2/board-generic.c | 1 + > arch/arm/mach-omap2/common.h | 8 +- > arch/arm/mach-omap2/cpuidle34xx.c | 6 +- > .../{cpuidle44xx.c => cpuidle_omap4plus.c} | 151 ++++++++++++++++--- > arch/arm/mach-omap2/io.c | 8 + > arch/arm/mach-omap2/omap-mpuss-lowpower.c | 155 +++++++++++++++----- > arch/arm/mach-omap2/omap-secure.h | 9 ++ > arch/arm/mach-omap2/omap-smp.c | 12 +- > arch/arm/mach-omap2/omap-wakeupgen.c | 30 +++- > arch/arm/mach-omap2/omap-wakeupgen.h | 1 + > arch/arm/mach-omap2/omap4-sar-layout.h | 2 + > arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 90 ++++++++++-- > arch/arm/mach-omap2/prminst44xx.c | 10 +- > .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 106 +++++++++++++ > 16 files changed, 512 insertions(+), 90 deletions(-) > rename arch/arm/mach-omap2/{cpuidle44xx.c => cpuidle_omap4plus.c} (58%) > rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (74%) > rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (77%) > Build tested [1] for omap2plus_defconfig. For [2], where omap5_pm_branch[1] is merged, testing details are as follows: * Boot tested on omap4430 sdp, omap5 evm. * Wakeup from UART after suspend using dt was tested on omap4430sdp and omap5430 evm. Note: For the above UART testing, we need to append "no_console_suspend" in our bootargs. Tested-by: Sourav Poddar <sourav.poddar@ti.com> Thanks, Sourav [1] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux for_3.10/omap5_pm [2] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git testing/3.10/omap5_int
On Monday 25 March 2013 05:57 PM, Sourav Poddar wrote: > Hi Santosh, > On Monday 25 March 2013 03:34 PM, Santosh Shilimkar wrote: >> Kevin, >> >> Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted >> earlier (March 1st 2013). Patch-set incorporates comments from Nishant >> Menon (Thanks for review NM) and his acked-by tags. I would like to get this >> queued for 3.10 merge window if you are ok with the series. >> >> Series is built on top of my pull requests [1] [2] [3] sent to Tony and your >> 'for_3.10/pm/cleanup' branch. For testing, I have created a branch [4] >> which put together all the needed dependencies, fixes which should make it >> to 3.10 merge window. >> >> Series adds OMAP5 MPUSS power management support for system wide suspend >> and CPUidle. Its heavy re-use from OMAP4 and hence only ~400 odd lines are >> needed to add OMAP5 PM support on top of existing OMAP4 PM support. >> >> OMAP5 adds a mercury retention feature which is an enhancement of >> existing retention feature to reduce the leakage. No change in >> programming model except one time enabling of mercury retention >> during init. >> >> One more notable change in OMAP5 vs OMAP4 devices, CPUx power domains >> support retention state which lets you hit MPUSS and Core retention with >> very low latency C-states. >> >> Tested on OMAP4430 SDP, OMAP4460 Panda, OMAP5430 SDP and OMAP5432 Panda >> devices with suspend and CPUIdle. Rootfs is mounted over ramdisk since >> the mmc and nfs based fs needs DMA engine patches. For suspend wakeup, >> I used Sourav's couple of serial wakeup wip patches from the lists. >> >> The following changes since commit 9981cde24de52e5bb9a7cd918d1e54de241f85c9: >> >> Merge branch 'for_3.10/omap5_data_files' of git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux into for_3.10/omap5_pm (2013-03-25 12:29:34 +0530) >> >> are available in the git repository at: >> >> >> git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git for_3.10/omap5_pm >> >> for you to fetch changes up to 75bd846d3103da858a208fe07127151903d1f608: >> >> ARM: OMAP5: PM: handle device instance for warm reset (2013-03-25 12:37:44 +0530) >> >> ---------------------------------------------------------------- >> Nishanth Menon (1): >> ARM: OMAP5: PM: handle device instance for warm reset >> >> Santosh Shilimkar (17): >> ARM: OMAP4+: PM: Consolidate MPU subsystem PM code for re-use >> ARM: OMAP5: PM: Update CPU context register offset >> ARM: OMAP4+: PM: Consolidate and use OMAP4 PM code for OMAP5 >> ARM: OMAP5: PM: Set MPUSS-EMIF clock-domain static dependency >> ARM: OMAP5: PM: Enables ES2 PM mode by default >> ARM: OMAP5: PM: Enable Mercury retention mode on CPUx powerdomains >> ARM: OMAP5: Add init_late() hook to enable pm initialization >> ARM: OMAP5: PM: Add CPU power off in hotplug path >> ARM: OMAP4+: PM: Restore CPU power state to ON with clockdomain force >> wakeup method >> ARM: OMAP5: PM: Add MPU Open Switch Retention support >> ARM: OMAP5: PM: Add L2 memory power down support >> ARM: OMAP4: CPUidle: Avoid double idle driver registration >> ARM: OMAP: CPUidle: Unregister drivere on device registration failure >> ARM: OMAP4: CPUidle: Make C-state description field more precise >> ARM: OMAP4+: CPUidle: Consolidate idle driver for OMAP5 support >> ARM: OMAP4+: CPUidle: Deprecate use of >> omap4_mpuss_read_prev_context_state() >> ARM: OMAP4+: CPUidle: Add OMAP5 idle driver support >> >> arch/arm/mach-omap2/Kconfig | 1 + >> arch/arm/mach-omap2/Makefile | 12 +- >> arch/arm/mach-omap2/board-generic.c | 1 + >> arch/arm/mach-omap2/common.h | 8 +- >> arch/arm/mach-omap2/cpuidle34xx.c | 6 +- >> .../{cpuidle44xx.c => cpuidle_omap4plus.c} | 151 ++++++++++++++++--- >> arch/arm/mach-omap2/io.c | 8 + >> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 155 +++++++++++++++----- >> arch/arm/mach-omap2/omap-secure.h | 9 ++ >> arch/arm/mach-omap2/omap-smp.c | 12 +- >> arch/arm/mach-omap2/omap-wakeupgen.c | 30 +++- >> arch/arm/mach-omap2/omap-wakeupgen.h | 1 + >> arch/arm/mach-omap2/omap4-sar-layout.h | 2 + >> arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 90 ++++++++++-- >> arch/arm/mach-omap2/prminst44xx.c | 10 +- >> .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 106 +++++++++++++ >> 16 files changed, 512 insertions(+), 90 deletions(-) >> rename arch/arm/mach-omap2/{cpuidle44xx.c => cpuidle_omap4plus.c} (58%) >> rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (74%) >> rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (77%) >> > Build tested [1] for omap2plus_defconfig. > > For [2], where omap5_pm_branch[1] is merged, testing details are as follows: > * Boot tested on omap4430 sdp, omap5 evm. > > * Wakeup from UART after suspend using dt was tested on omap4430sdp and > omap5430 evm. > Note: For the above UART testing, we need to append "no_console_suspend" in our > bootargs. Is this for both omap4 and omap5 that you need 'no_console_suspend'? > > Tested-by: Sourav Poddar <sourav.poddar@ti.com> > > Thanks, > Sourav > > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux for_3.10/omap5_pm > [2] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git testing/3.10/omap5_int > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Rajendra, On Monday 25 March 2013 06:17 PM, Rajendra Nayak wrote: > On Monday 25 March 2013 05:57 PM, Sourav Poddar wrote: >> Hi Santosh, >> On Monday 25 March 2013 03:34 PM, Santosh Shilimkar wrote: >>> Kevin, >>> >>> Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted >>> earlier (March 1st 2013). Patch-set incorporates comments from Nishant >>> Menon (Thanks for review NM) and his acked-by tags. I would like to get this >>> queued for 3.10 merge window if you are ok with the series. >>> >>> Series is built on top of my pull requests [1] [2] [3] sent to Tony and your >>> 'for_3.10/pm/cleanup' branch. For testing, I have created a branch [4] >>> which put together all the needed dependencies, fixes which should make it >>> to 3.10 merge window. >>> >>> Series adds OMAP5 MPUSS power management support for system wide suspend >>> and CPUidle. Its heavy re-use from OMAP4 and hence only ~400 odd lines are >>> needed to add OMAP5 PM support on top of existing OMAP4 PM support. >>> >>> OMAP5 adds a mercury retention feature which is an enhancement of >>> existing retention feature to reduce the leakage. No change in >>> programming model except one time enabling of mercury retention >>> during init. >>> >>> One more notable change in OMAP5 vs OMAP4 devices, CPUx power domains >>> support retention state which lets you hit MPUSS and Core retention with >>> very low latency C-states. >>> >>> Tested on OMAP4430 SDP, OMAP4460 Panda, OMAP5430 SDP and OMAP5432 Panda >>> devices with suspend and CPUIdle. Rootfs is mounted over ramdisk since >>> the mmc and nfs based fs needs DMA engine patches. For suspend wakeup, >>> I used Sourav's couple of serial wakeup wip patches from the lists. >>> >>> The following changes since commit 9981cde24de52e5bb9a7cd918d1e54de241f85c9: >>> >>> Merge branch 'for_3.10/omap5_data_files' of git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux into for_3.10/omap5_pm (2013-03-25 12:29:34 +0530) >>> >>> are available in the git repository at: >>> >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git for_3.10/omap5_pm >>> >>> for you to fetch changes up to 75bd846d3103da858a208fe07127151903d1f608: >>> >>> ARM: OMAP5: PM: handle device instance for warm reset (2013-03-25 12:37:44 +0530) >>> >>> ---------------------------------------------------------------- >>> Nishanth Menon (1): >>> ARM: OMAP5: PM: handle device instance for warm reset >>> >>> Santosh Shilimkar (17): >>> ARM: OMAP4+: PM: Consolidate MPU subsystem PM code for re-use >>> ARM: OMAP5: PM: Update CPU context register offset >>> ARM: OMAP4+: PM: Consolidate and use OMAP4 PM code for OMAP5 >>> ARM: OMAP5: PM: Set MPUSS-EMIF clock-domain static dependency >>> ARM: OMAP5: PM: Enables ES2 PM mode by default >>> ARM: OMAP5: PM: Enable Mercury retention mode on CPUx powerdomains >>> ARM: OMAP5: Add init_late() hook to enable pm initialization >>> ARM: OMAP5: PM: Add CPU power off in hotplug path >>> ARM: OMAP4+: PM: Restore CPU power state to ON with clockdomain force >>> wakeup method >>> ARM: OMAP5: PM: Add MPU Open Switch Retention support >>> ARM: OMAP5: PM: Add L2 memory power down support >>> ARM: OMAP4: CPUidle: Avoid double idle driver registration >>> ARM: OMAP: CPUidle: Unregister drivere on device registration failure >>> ARM: OMAP4: CPUidle: Make C-state description field more precise >>> ARM: OMAP4+: CPUidle: Consolidate idle driver for OMAP5 support >>> ARM: OMAP4+: CPUidle: Deprecate use of >>> omap4_mpuss_read_prev_context_state() >>> ARM: OMAP4+: CPUidle: Add OMAP5 idle driver support >>> >>> arch/arm/mach-omap2/Kconfig | 1 + >>> arch/arm/mach-omap2/Makefile | 12 +- >>> arch/arm/mach-omap2/board-generic.c | 1 + >>> arch/arm/mach-omap2/common.h | 8 +- >>> arch/arm/mach-omap2/cpuidle34xx.c | 6 +- >>> .../{cpuidle44xx.c => cpuidle_omap4plus.c} | 151 ++++++++++++++++--- >>> arch/arm/mach-omap2/io.c | 8 + >>> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 155 +++++++++++++++----- >>> arch/arm/mach-omap2/omap-secure.h | 9 ++ >>> arch/arm/mach-omap2/omap-smp.c | 12 +- >>> arch/arm/mach-omap2/omap-wakeupgen.c | 30 +++- >>> arch/arm/mach-omap2/omap-wakeupgen.h | 1 + >>> arch/arm/mach-omap2/omap4-sar-layout.h | 2 + >>> arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 90 ++++++++++-- >>> arch/arm/mach-omap2/prminst44xx.c | 10 +- >>> .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 106 +++++++++++++ >>> 16 files changed, 512 insertions(+), 90 deletions(-) >>> rename arch/arm/mach-omap2/{cpuidle44xx.c => cpuidle_omap4plus.c} (58%) >>> rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (74%) >>> rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (77%) >>> >> Build tested [1] for omap2plus_defconfig. >> >> For [2], where omap5_pm_branch[1] is merged, testing details are as follows: >> * Boot tested on omap4430 sdp, omap5 evm. >> >> * Wakeup from UART after suspend using dt was tested on omap4430sdp and >> omap5430 evm. >> Note: For the above UART testing, we need to append "no_console_suspend" in our >> bootargs. > Is this for both omap4 and omap5 that you need 'no_console_suspend'? > For omap5, using "no_console_suspend" is mandatory as of now. For omap4 dt case, We can also do without "no_console_suspend". But the tested branch[2] contains a patch which fixes wakeup from uart on both omap4/5 with dt, while using "no_console_suspend" in the bootargs.. ~Sourav >> Tested-by: Sourav Poddar<sourav.poddar@ti.com> >> >> Thanks, >> Sourav >> >> [1] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux for_3.10/omap5_pm >> [2] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git testing/3.10/omap5_int >> >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Santosh, Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > OMAP5 and future OMAP based SOCs has backward compatible MPUSS > IP block with OMAP4. It's programming model is mostly similar. > Hence consolidate the OMAP MPUSS code so that it can be re-used > on OMAP5 and future SOCs. > > No functional change. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/mach-omap2/omap-mpuss-lowpower.c | 65 ++++++++++++++++++++++++----- > 1 file changed, 54 insertions(+), 11 deletions(-) > > diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c > index d650f91..d9e4843 100644 > --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c > +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c > @@ -71,10 +71,46 @@ struct omap4_cpu_pm_info { > void (*secondary_startup)(void); > }; > > +/** > + * struct cpu_pm_ops - CPU pm operations > + * @finish_suspend: CPU suspend finisher function pointer > + * @resume: CPU resume function pointer > + * @scu_prepare: CPU Snoop Control program function pointer > + * > + * Structure holds functions pointer for CPU low power operations like > + * suspend, resume and scu programming. > + */ > +struct cpu_pm_ops { > + int (*finish_suspend)(unsigned long cpu_state); > + void (*resume)(void); > + void (*scu_prepare)(unsigned int cpu_id, unsigned int cpu_state); > +}; > + > +extern int omap4_finish_suspend(unsigned long cpu_state); > +extern void omap4_cpu_resume(void); checkpatch should've yelled at you for adding externs to .c files. Also, aren't these already defined in common.h anyways? Otherwise, patch looks fine. Kevin > static DEFINE_PER_CPU(struct omap4_cpu_pm_info, omap4_pm_info); > static struct powerdomain *mpuss_pd; > static void __iomem *sar_base; > > +static int default_finish_suspend(unsigned long cpu_state) > +{ > + omap_do_wfi(); > + return 0; > +} > + > +static void dummy_cpu_resume(void) > +{} > + > +static void dummy_scu_prepare(unsigned int cpu_id, unsigned int cpu_state) > +{} > + > +struct cpu_pm_ops omap_pm_ops = { > + .finish_suspend = default_finish_suspend, > + .resume = dummy_cpu_resume, > + .scu_prepare = dummy_scu_prepare, > +}; > + > /* > * Program the wakeup routine address for the CPU0 and CPU1 > * used for OFF or DORMANT wakeup. > @@ -172,11 +208,12 @@ static void save_l2x0_context(void) > { > u32 val; > void __iomem *l2x0_base = omap4_get_l2cache_base(); > - > - val = __raw_readl(l2x0_base + L2X0_AUX_CTRL); > - __raw_writel(val, sar_base + L2X0_AUXCTRL_OFFSET); > - val = __raw_readl(l2x0_base + L2X0_PREFETCH_CTRL); > - __raw_writel(val, sar_base + L2X0_PREFETCH_CTRL_OFFSET); > + if (l2x0_base) { > + val = __raw_readl(l2x0_base + L2X0_AUX_CTRL); > + __raw_writel(val, sar_base + L2X0_AUXCTRL_OFFSET); > + val = __raw_readl(l2x0_base + L2X0_PREFETCH_CTRL); > + __raw_writel(val, sar_base + L2X0_PREFETCH_CTRL_OFFSET); > + } > } > #else > static void save_l2x0_context(void) > @@ -239,17 +276,17 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state) > > cpu_clear_prev_logic_pwrst(cpu); > pwrdm_set_next_pwrst(pm_info->pwrdm, power_state); > - set_cpu_wakeup_addr(cpu, virt_to_phys(omap4_cpu_resume)); > - scu_pwrst_prepare(cpu, power_state); > + set_cpu_wakeup_addr(cpu, virt_to_phys(omap_pm_ops.resume)); > + omap_pm_ops.scu_prepare(cpu, power_state); > l2x0_pwrst_prepare(cpu, save_state); > > /* > * Call low level function with targeted low power state. > */ > if (save_state) > - cpu_suspend(save_state, omap4_finish_suspend); > + cpu_suspend(save_state, omap_pm_ops.finish_suspend); > else > - omap4_finish_suspend(save_state); > + omap_pm_ops.finish_suspend(save_state); > > /* > * Restore the CPUx power state to ON otherwise CPUx > @@ -285,14 +322,14 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state) > pwrdm_clear_all_prev_pwrst(pm_info->pwrdm); > pwrdm_set_next_pwrst(pm_info->pwrdm, power_state); > set_cpu_wakeup_addr(cpu, virt_to_phys(pm_info->secondary_startup)); > - scu_pwrst_prepare(cpu, power_state); > + omap_pm_ops.scu_prepare(cpu, power_state); > > /* > * CPU never retuns back if targeted power state is OFF mode. > * CPU ONLINE follows normal CPU ONLINE ptah via > * omap_secondary_startup(). > */ > - omap4_finish_suspend(cpu_state); > + omap_pm_ops.finish_suspend(cpu_state); > > pwrdm_set_next_pwrst(pm_info->pwrdm, PWRDM_POWER_ON); > return 0; > @@ -369,6 +406,12 @@ int __init omap4_mpuss_init(void) > > save_l2x0_context(); > > + if (cpu_is_omap44xx()) { > + omap_pm_ops.finish_suspend = omap4_finish_suspend; > + omap_pm_ops.resume = omap4_cpu_resume; > + omap_pm_ops.scu_prepare = scu_pwrst_prepare; > + } > + > return 0; > }
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > 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. > > With consolidated code, let basic power management code build > for OMAP5 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} | 54 ++++++++++++++++---- > .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 0 > 3 files changed, 49 insertions(+), 14 deletions(-) > rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (86%) > 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 5d5ff91..d91ae0f 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 86% > rename from arch/arm/mach-omap2/pm44xx.c > rename to arch/arm/mach-omap2/pm_omap4plus.c > index 5ba6d88..e920c34 100644 > --- a/arch/arm/mach-omap2/pm44xx.c > +++ b/arch/arm/mach-omap2/pm_omap4plus.c > @@ -1,7 +1,7 @@ > /* > - * OMAP4 Power Management Routines > + * OMAP4PLUS Power Management Routines nit: OMAP4+ (you only need to spell out "plus" in the filename. > * > - * 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 dropped the __init here, but it's still only called from another __init function, so I suspect it should stay. > { > - 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; > } > > /* > @@ -179,7 +179,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 +188,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 OMAP4PLUS devices.\n"); s/PLUS/+/ Kevin > + > + 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 +239,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; > diff --git a/arch/arm/mach-omap2/sleep44xx.S b/arch/arm/mach-omap2/sleep_omap4plus.S > similarity index 100% > rename from arch/arm/mach-omap2/sleep44xx.S > rename to arch/arm/mach-omap2/sleep_omap4plus.S
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > Enables MPUSS ES2 power management mode using ES2_PM_MODE in > AMBA_IF_MODE register. > > 0x0: ES1 behavior, CPU cores would enter and exit OFF mode together. Broken What is broken? > 0x1: ES2 behavior, CPU cores are allowed to enter/exit OFF mode independently. > > The AMBA_IF_MODE register value is stored on SAR RAM and restored by > ROM code. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/mach-omap2/omap-secure.h | 2 ++ > arch/arm/mach-omap2/omap-wakeupgen.c | 19 +++++++++++++++++++ > arch/arm/mach-omap2/omap-wakeupgen.h | 1 + > 3 files changed, 22 insertions(+) > > diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h > index 0e72917..82b3c4c 100644 > --- a/arch/arm/mach-omap2/omap-secure.h > +++ b/arch/arm/mach-omap2/omap-secure.h > @@ -42,6 +42,8 @@ > #define OMAP4_MON_L2X0_AUXCTRL_INDEX 0x109 > #define OMAP4_MON_L2X0_PREFETCH_INDEX 0x113 > > +#define OMAP5_MON_AMBA_IF_INDEX 0x108 > + > /* Secure PPA(Primary Protected Application) APIs */ > #define OMAP4_PPA_L2_POR_INDEX 0x23 > #define OMAP4_PPA_CPU_ACTRL_SMP_INDEX 0x25 > diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c > index f8bb3b9..8bcaa8c 100644 > --- a/arch/arm/mach-omap2/omap-wakeupgen.c > +++ b/arch/arm/mach-omap2/omap-wakeupgen.c > @@ -42,6 +42,7 @@ > #define CPU1_ID 0x1 > #define OMAP4_NR_BANKS 4 > #define OMAP4_NR_IRQS 128 > +#define OMAP5_AMBA_IF_PM_MODE (1 << 5) nit: use BIT() Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > In addition to the standard power-management technique, the OMAP5 > MPU subsystem also employs an SR3-APG (mercury) power management > technology to reduce leakage. > > It allows for full logic and memories retention on MPU_C0 and MPU_C1 and > is controlled by the PRCM_MPU. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/mach-omap2/omap-mpuss-lowpower.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c > index b1441b1..d390d18 100644 > --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c > +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c > @@ -62,6 +62,10 @@ > #include "prm44xx.h" > #include "prm-regbits-44xx.h" > > +/* Add defines needed for mercury mode. Refer MPU's PRM_PSCON_COUNT */ > +#define PRM_PSCON_HG_EN (1 << 24) > +#define PRM_PSCON_HG_RAMPUP (1 << 25) nit: use BIT() > #ifdef CONFIG_SMP > > struct omap4_cpu_pm_info { > @@ -337,6 +341,28 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state) > return 0; > } > > +/** > + * enable_mercury_retention_mode: Enable OMAP5 mercury retention feature > + * > + * OMAP5 devices supports SR3-APG (mercury) power management technology to > + * reduce leakage. It allows for full logic and memories retention on > + * MPU_C0 and MPU_C1 and is controlled by the PRCM_MPU. The function enable > + * the mercury retention feature. > + */ > +static void enable_mercury_retention_mode(void) __init ? This is very OMAP5 specific (unless you generalize the offsets used) so should this have omap5_ prefix? Kevin > +{ > + u32 reg; > + > + /* > + * To enable mercury mode, both HG_EN and HG_RAMPUP needs to be > + * enabled from PRM_PSCON_COUNT register. > + */ > + reg = omap4_prcm_mpu_read_inst_reg(OMAP54XX_PRCM_MPU_DEVICE_INST, > + OMAP54XX_PRCM_MPU_PRM_PSCON_COUNT_OFFSET); > + reg |= PRM_PSCON_HG_EN | PRM_PSCON_HG_RAMPUP; > + omap4_prcm_mpu_write_inst_reg(reg, OMAP54XX_PRCM_MPU_DEVICE_INST, > + OMAP54XX_PRCM_MPU_PRM_PSCON_COUNT_OFFSET); nit: when I see 'reg', I expect it to be a register/offset. But this is a register value. Maybe use 'val' instead as you've done elsewhere in this series. Kevin > +} > > /* > * Initialise OMAP4 MPUSS > @@ -415,6 +441,7 @@ int __init omap4_mpuss_init(void) > cpu_context_offset = OMAP4_RM_CPU0_CPU0_CONTEXT_OFFSET; > } else if (soc_is_omap54xx()) { > cpu_context_offset = OMAP54XX_RM_CPU0_CPU0_CONTEXT_OFFSET; > + enable_mercury_retention_mode(); > } > > return 0;
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > With consolidated code, now we can add the .init_late hook for > OMAP5 to enable power management and mux initialization. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/mach-omap2/board-generic.c | 1 + > arch/arm/mach-omap2/common.h | 3 ++- > arch/arm/mach-omap2/io.c | 8 ++++++++ > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c > index e54a480..8e261a6 100644 > --- a/arch/arm/mach-omap2/board-generic.c > +++ b/arch/arm/mach-omap2/board-generic.c > @@ -180,6 +180,7 @@ DT_MACHINE_START(OMAP5_DT, "Generic OMAP5 (Flattened Device Tree)") > .init_irq = omap_gic_of_init, > .init_machine = omap_generic_init, > .init_time = omap5_realtime_timer_init, > + .init_late = omap5_init_late, > .dt_compat = omap5_boards_compat, > .restart = omap44xx_restart, > MACHINE_END > diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h > index 40f4a03..f75d972 100644 > --- a/arch/arm/mach-omap2/common.h > +++ b/arch/arm/mach-omap2/common.h > @@ -59,7 +59,7 @@ static inline int omap3_pm_init(void) > } > #endif > > -#if defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP4) > +#if defined(CONFIG_PM) && (defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)) > int omap4_pm_init(void); > #else > static inline int omap4_pm_init(void) > @@ -108,6 +108,7 @@ void omap35xx_init_late(void); > void omap3630_init_late(void); > void am35xx_init_late(void); > void ti81xx_init_late(void); > +void omap5_init_late(void); > int omap2_common_pm_late_init(void); > > #if defined(CONFIG_SOC_OMAP2420) || defined(CONFIG_SOC_OMAP2430) > diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c > index e948a39..cdd1264 100644 > --- a/arch/arm/mach-omap2/io.c > +++ b/arch/arm/mach-omap2/io.c > @@ -636,6 +636,14 @@ void __init omap5_init_early(void) > omap_hwmod_init_postsetup(); > > } > + > +void __init omap5_init_late(void) > +{ > + omap_mux_late_init(); > + omap2_common_pm_late_init(); > + omap4_pm_init(); > + omap2_clk_enable_autoidle_all(); > +} Since you're consolidating, why not rename omap4430_init_late to omap4plus_init_late and use it for both OMAP4 and OMAP5? Kevin > #endif > > void __init omap_sdrc_init(struct omap_sdrc_params *sdrc_cs0,
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > Add power management code to handle the CPU off mode to enable CPUP hotplug > mode for OMAP5 devices. Separate suspend finisher is used for OMAP5(Cortex-A15) > because it doesn't use SCU power status register and external PL310 L2 cache > which makes code flow bit different. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> [...] > @@ -436,14 +445,21 @@ int __init omap4_mpuss_init(void) > > if (cpu_is_omap44xx()) { > omap_pm_ops.finish_suspend = omap4_finish_suspend; > + omap_pm_ops.hotplug_restart = omap_secondary_startup; > omap_pm_ops.resume = omap4_cpu_resume; > omap_pm_ops.scu_prepare = scu_pwrst_prepare; > cpu_context_offset = OMAP4_RM_CPU0_CPU0_CONTEXT_OFFSET; > } else if (soc_is_omap54xx()) { > + omap_pm_ops.finish_suspend = omap5_finish_suspend; > + omap_pm_ops.hotplug_restart = omap5_secondary_startup; > cpu_context_offset = OMAP54XX_RM_CPU0_CPU0_CONTEXT_OFFSET; > enable_mercury_retention_mode(); > } > > + /* Over-write the OMAP4 hook to take care of ROM BUG */ > + if (cpu_is_omap446x()) > + omap_pm_ops.hotplug_restart = omap_secondary_startup_4460; A couple nits... I think this would go better at the end of the 'if omap44xx' block above. Also, while you're hear, maybe it's time to rename the current secondary startup functions to match the new one. IOW omap4_..., omap4460_... and omap5_... Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > While waking up CPU from off state using clock domain force wakeup, restore > the CPU power state to ON state before putting CPU clock domain under > hardware control. Otherwise CPU wakeup might fail. The change is recommended > for all OMAP4+ devices though the PRCM weakness was observed on OMAP5 > devices first. Sounds reasonable, but can you describe the "weakness" a little more? IOW, what exactly happens if this is not done? It sounds like the CPU might immediately go back to retention, but how does that happen unless it does a WFI? Also, this sounds like a fix to me, and should probably be broken out accordingly. Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > When the entire MPUSS cluster is powered down in device off state, L2 cache > memory looses it's content and hence while targetting such a state, > l2 cache needs to be flushed to main memory. > > Add the necessary low power code support for the same. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/mach-omap2/omap-secure.h | 1 + > arch/arm/mach-omap2/sleep_omap4plus.S | 21 +++++++++++++++++++++ > 2 files changed, 22 insertions(+) > > diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h > index 1739468..a171a5a 100644 > --- a/arch/arm/mach-omap2/omap-secure.h > +++ b/arch/arm/mach-omap2/omap-secure.h > @@ -47,6 +47,7 @@ > #define OMAP4_MON_L2X0_PREFETCH_INDEX 0x113 > #define OMAP5_MON_CACHES_CLEAN_INDEX 0x103 > #define OMAP5_MON_AUX_CTRL_INDEX 0x107 > +#define OMAP5_MON_L2AUX_CTRL_INDEX 0x104 this #define is not used in this patch. Kevin > #define OMAP5_MON_AMBA_IF_INDEX 0x108 > > diff --git a/arch/arm/mach-omap2/sleep_omap4plus.S b/arch/arm/mach-omap2/sleep_omap4plus.S > index 4a5e2e4..288f62f 100644 > --- a/arch/arm/mach-omap2/sleep_omap4plus.S > +++ b/arch/arm/mach-omap2/sleep_omap4plus.S > @@ -337,6 +337,7 @@ ENDPROC(omap4_cpu_resume) > * 0 - Nothing lost and no need to save: MPUSS INA/CSWR > * 1 - CPUx L1 and logic lost: CPU OFF, MPUSS INA/CSWR > * 2 - CPUx L1 and logic lost + GIC lost: MPUSS OSWR > + * 3 - CPUx L1 and logic lost + GIC,L2 lost: MPU OSWR & L2 lost(debug only) > */ > ENTRY(omap5_finish_suspend) > stmfd sp!, {r4-r12, lr} > @@ -391,6 +392,26 @@ skip_secure_l1_clean_op: > isb > dsb > > + bl omap4_get_sar_ram_base > + mov r8, r0 > + mrc p15, 0, r5, c0, c0, 5 @ Read MPIDR > + ands r5, r5, #0x0f > + ldreq r0, [r8, #L2X0_SAVE_OFFSET0] @ Retrieve L2 state > + ldrne r0, [r8, #L2X0_SAVE_OFFSET1] > + cmp r0, #3 > + bne do_wfi > + bl omap4_get_sar_ram_base > + ldr r9, [r0, #OMAP_TYPE_OFFSET] > + cmp r9, #0x1 @ Check for HS device > + bne skip_secure_l2_clean_op > + mov r0, #1 @ Clean secure L2 > + stmfd r13!, {r4-r12, r14} > + ldr r12, =OMAP5_MON_CACHES_CLEAN_INDEX > + DO_SMC > + ldmfd r13!, {r4-r12, r14} > +skip_secure_l2_clean_op: > + bl v7_flush_dcache_all > + > do_wfi: > bl omap_do_wfi
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > OMAP4 CPUidle driver registration call is under a loop which leads > to calling cpuidle_register_driver twice which is not intended. > > Fix it by moving the driver registration outside the loop. > > Reported-by: Nishanth Menon <nm@ti.com> > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> Looks good. I've already got a for_3.10/fixes/cpuidle branch going, so I'll apply this one there. Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > If the CPUidle device registration fails for some reason, we should > unregister the driver on error path. > > Fix the code accordingly. Also when at it, check of the driver registration > failure too. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> Adding to my for_3.10/fixes/cpuidle branch. Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > It is useful to know the CPU power state along with MPUSS power state > in a supported C-state. Since the data is available via sysfs, one can > avoid scrolling the source code for precise construction of C-state. > > Reported-by: Nishanth Menon <nm@ti.com> > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> Nice. Adding to for_3.10/fixes/cpuidle branch. Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > The OMAP5 idle driver can re-use most of OMAP4 CPUidle driver > implementation. Also the next derivative SOCs are going to re-use > the MPUSS so, same driver with minor updates can be re-used. > > Prepare the code so that its easier to add CPUidle support for > OMAP5 devices. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/mach-omap2/cpuidle44xx.c | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c > index b8a22f0..ac6d526 100644 > --- a/arch/arm/mach-omap2/cpuidle44xx.c > +++ b/arch/arm/mach-omap2/cpuidle44xx.c > @@ -1,7 +1,7 @@ > /* > - * OMAP4 CPU idle Routines > + * OMAP4PLUS CPU idle Routines nit: s/PLUS/+/ in a few other places in this patch also. > * > - * Copyright (C) 2011 Texas Instruments, Inc. > + * Copyright (C) 2011-2013 Texas Instruments, Inc. > * Santosh Shilimkar <santosh.shilimkar@ti.com> > * Rajendra Nayak <rnayak@ti.com> > * > @@ -24,13 +24,13 @@ > #include "clockdomain.h" > > /* Machine specific information */ > -struct omap4_idle_statedata { > +struct idle_statedata { > u32 cpu_state; > u32 mpu_logic_state; > u32 mpu_state; > }; > > -static struct omap4_idle_statedata omap4_idle_data[] = { > +static struct idle_statedata omap4_idle_data[] = { > { > .cpu_state = PWRDM_POWER_ON, > .mpu_state = PWRDM_POWER_ON, > @@ -53,11 +53,12 @@ static struct clockdomain *cpu_clkdm[NR_CPUS]; > > static atomic_t abort_barrier; > static bool cpu_done[NR_CPUS]; > +static struct idle_statedata *state_ptr = &omap4_idle_data[0]; The assignment at init time (from the next patch) should be done here for 44xx, and the next patch can just add OMAP5 support. Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > The OMAP5 idle driver can re-use OMAP4 CPUidle driver implementation thanks > to compatible MPUSS design. > > Though unlike OMAP4, on OMAP5 devices, MPUSS CSWR (Close Switch > Retention) power states can be achieved with respective power states > on CPU0 and CPU1 power domain. This mode was broken on OMAP4 devices > because of hardware limitation. I'm a bit confused by the description here. I gather from the code that this means that CPU0 and CPU1 can hit CSWR independently, correct? > Also there is no CPU low power entry order requirement like > master CPU etc for CSWR C-state, which is icing on the cake. Even on secure devices? > Code makes use of voting scheme for cluster low power state to support > MPUSS CSWR C-state. The voting scheme and associated locking should be better described here, or commented in the code itself. > Supported OMAP5 CPUidle C-states: > C1 - CPU0 ON(WFI) + CPU1 ON(WFI) + MPUSS ON > C2 - CPU0 CSWR + CPU1 CSWR + MPUSS CSWR > C3 - CPU0 OFF + CPU1 OFF + MPUSS OSWR > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> [...] > +static int omap_enter_idle_smp(struct cpuidle_device *dev, > + struct cpuidle_driver *drv, > + int index) > +{ > + struct idle_statedata *cx = state_ptr + index; > + int cpu_id = smp_processor_id(); > + unsigned long flag; > + > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); I think the CPUidle core handles the broadcast notification now. > + raw_spin_lock_irqsave(&mpu_lock, flag); > + cx->mpu_state_vote++; How about using an atomic_t and atomic_inc()/atomic_dec() instead, which will avoid the need for a spinlock. Even with that, it still seems potentially racy. Do you need a memory barrier here to be sure that any changes from another CPU are visible here, and vice versa? > + if (cx->mpu_state_vote == num_online_cpus()) { > + pwrdm_set_logic_retst(mpu_pd, cx->mpu_logic_state); > + omap_set_pwrdm_state(mpu_pd, cx->mpu_state); > + } > + raw_spin_unlock_irqrestore(&mpu_lock, flag); > + > + omap4_enter_lowpower(dev->cpu, cx->cpu_state); > + > + raw_spin_lock_irqsave(&mpu_lock, flag); > + if (cx->mpu_state_vote == num_online_cpus()) > + omap_set_pwrdm_state(mpu_pd, PWRDM_POWER_ON); > + cx->mpu_state_vote--; > + raw_spin_unlock_irqrestore(&mpu_lock, flag); > + > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id); > + > + return index; > +} Kevin
Hi Santosh, Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted > earlier (March 1st 2013). Patch-set incorporates comments from Nishant > Menon (Thanks for review NM) and his acked-by tags. I would like to get this > queued for 3.10 merge window if you are ok with the series. I did a review of this series with several comments on the individual patches. I've already queued most of the CPUidle fixes/cleanups in my for_3.10/cleanup/cpuidle branch and will get that to Tony soon. For the rest, I suggest probably breaking it up into cleanup/consolidation stuff and then OMAP5 support. The latter will depend on all the OMAP5 data/headers but the cleanup/consolidation stuff could be merged sooner (but probably a bit late for v3.10 now.) Kevin
On Thursday 04 April 2013 01:14 AM, Kevin Hilman wrote: > Hi Santosh, > > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> OMAP5 and future OMAP based SOCs has backward compatible MPUSS >> IP block with OMAP4. It's programming model is mostly similar. >> Hence consolidate the OMAP MPUSS code so that it can be re-used >> on OMAP5 and future SOCs. >> >> No functional change. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- >> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 65 ++++++++++++++++++++++++----- >> 1 file changed, 54 insertions(+), 11 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c >> index d650f91..d9e4843 100644 >> --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c >> +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c >> @@ -71,10 +71,46 @@ struct omap4_cpu_pm_info { >> void (*secondary_startup)(void); >> }; >> >> +/** >> + * struct cpu_pm_ops - CPU pm operations >> + * @finish_suspend: CPU suspend finisher function pointer >> + * @resume: CPU resume function pointer >> + * @scu_prepare: CPU Snoop Control program function pointer >> + * >> + * Structure holds functions pointer for CPU low power operations like >> + * suspend, resume and scu programming. >> + */ >> +struct cpu_pm_ops { >> + int (*finish_suspend)(unsigned long cpu_state); >> + void (*resume)(void); >> + void (*scu_prepare)(unsigned int cpu_id, unsigned int cpu_state); >> +}; >> + >> +extern int omap4_finish_suspend(unsigned long cpu_state); >> +extern void omap4_cpu_resume(void); > > checkpatch should've yelled at you for adding externs to .c files. > It does. I didn't see they were already in header file and considering they were shared between asm and mpuss file, I just kept it that way. i have seen many places in kernel for asm exports, this is being used and hence kept it. > Also, aren't these already defined in common.h anyways? > Yep. I will just drop above hunk. > Otherwise, patch looks fine. > I will take that as an ack then ? Regards, Santosh
On Thursday 04 April 2013 01:50 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> 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. >> >> With consolidated code, let basic power management code build >> for OMAP5 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} | 54 ++++++++++++++++---- >> .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 0 >> 3 files changed, 49 insertions(+), 14 deletions(-) >> rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (86%) >> rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (100%) >> [..] >> diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm_omap4plus.c >> similarity index 86% >> rename from arch/arm/mach-omap2/pm44xx.c >> rename to arch/arm/mach-omap2/pm_omap4plus.c >> index 5ba6d88..e920c34 100644 >> --- a/arch/arm/mach-omap2/pm44xx.c >> +++ b/arch/arm/mach-omap2/pm_omap4plus.c >> @@ -1,7 +1,7 @@ >> /* >> - * OMAP4 Power Management Routines >> + * OMAP4PLUS Power Management Routines > > nit: OMAP4+ (you only need to spell out "plus" in the filename. > OK. I will replace '+' instead of 'PLUS' in rest of the places. >> * >> - * 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 dropped the __init here, but it's still only called from another > __init function, so I suspect it should stay. > Yep. Will keep that in next version. Regards, Santosh
On Thursday 04 April 2013 05:21 PM, Santosh Shilimkar wrote: > On Thursday 04 April 2013 01:50 AM, Kevin Hilman wrote: >> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >> >>> 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. >>> >>> With consolidated code, let basic power management code build >>> for OMAP5 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} | 54 ++++++++++++++++---- >>> .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 0 >>> 3 files changed, 49 insertions(+), 14 deletions(-) >>> rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (86%) >>> rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (100%) >>> > > [..] >>> >>> /** >>> - * 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 dropped the __init here, but it's still only called from another >> __init function, so I suspect it should stay. >> > Yep. Will keep that in next version. > I was too quick to respond. The function is "static inline" so there is no need to add __init here. Regards, Santosh
On Thursday 04 April 2013 01:55 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> Enables MPUSS ES2 power management mode using ES2_PM_MODE in >> AMBA_IF_MODE register. >> >> 0x0: ES1 behavior, CPU cores would enter and exit OFF mode together. Broken > > What is broken? > Should have added clarification here. Sorry. Changelog is updated as below 0x0: ES1 behavior, CPU cores would enter and exit OFF mode together. 0x1: ES2 behavior, CPU cores are allowed to enter/exit OFF mode independently. ES1.0 CPUx OFF mode behavior never worked and after analysis by design team, it was declared as a broken hardware feature. That lead to addition of ES2.0 behavior which works. >> 0x1: ES2 behavior, CPU cores are allowed to enter/exit OFF mode independently. >> >> The AMBA_IF_MODE register value is stored on SAR RAM and restored by >> ROM code. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- [..] >> diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c >> index f8bb3b9..8bcaa8c 100644 >> --- a/arch/arm/mach-omap2/omap-wakeupgen.c >> +++ b/arch/arm/mach-omap2/omap-wakeupgen.c >> @@ -42,6 +42,7 @@ >> #define CPU1_ID 0x1 >> #define OMAP4_NR_BANKS 4 >> #define OMAP4_NR_IRQS 128 >> +#define OMAP5_AMBA_IF_PM_MODE (1 << 5) > > nit: use BIT() > Ok. Regards, santosh
On Thursday 04 April 2013 02:01 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> In addition to the standard power-management technique, the OMAP5 >> MPU subsystem also employs an SR3-APG (mercury) power management >> technology to reduce leakage. >> >> It allows for full logic and memories retention on MPU_C0 and MPU_C1 and >> is controlled by the PRCM_MPU. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- >> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 27 +++++++++++++++++++++++++++ >> 1 file changed, 27 insertions(+) >> >> diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c >> index b1441b1..d390d18 100644 >> --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c >> +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c >> @@ -62,6 +62,10 @@ >> #include "prm44xx.h" >> #include "prm-regbits-44xx.h" >> >> +/* Add defines needed for mercury mode. Refer MPU's PRM_PSCON_COUNT */ >> +#define PRM_PSCON_HG_EN (1 << 24) >> +#define PRM_PSCON_HG_RAMPUP (1 << 25) > > nit: use BIT() > ok >> #ifdef CONFIG_SMP >> >> struct omap4_cpu_pm_info { >> @@ -337,6 +341,28 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state) >> return 0; >> } >> >> +/** >> + * enable_mercury_retention_mode: Enable OMAP5 mercury retention feature >> + * >> + * OMAP5 devices supports SR3-APG (mercury) power management technology to >> + * reduce leakage. It allows for full logic and memories retention on >> + * MPU_C0 and MPU_C1 and is controlled by the PRCM_MPU. The function enable >> + * the mercury retention feature. >> + */ >> +static void enable_mercury_retention_mode(void) > > __init ? > yep. > This is very OMAP5 specific (unless you generalize the offsets used) so > should this have omap5_ prefix? > Agree. > >> +{ >> + u32 reg; >> + >> + /* >> + * To enable mercury mode, both HG_EN and HG_RAMPUP needs to be >> + * enabled from PRM_PSCON_COUNT register. >> + */ >> + reg = omap4_prcm_mpu_read_inst_reg(OMAP54XX_PRCM_MPU_DEVICE_INST, >> + OMAP54XX_PRCM_MPU_PRM_PSCON_COUNT_OFFSET); >> + reg |= PRM_PSCON_HG_EN | PRM_PSCON_HG_RAMPUP; >> + omap4_prcm_mpu_write_inst_reg(reg, OMAP54XX_PRCM_MPU_DEVICE_INST, >> + OMAP54XX_PRCM_MPU_PRM_PSCON_COUNT_OFFSET); > > nit: when I see 'reg', I expect it to be a register/offset. But this is > a register value. Maybe use 'val' instead as you've done elsewhere in > this series. > I generally use 'offset' if it is offset and 'reg' for actual register read. Anyways, I have changed it to 'val' now. Regards, Santosh
On Thursday 04 April 2013 02:03 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> With consolidated code, now we can add the .init_late hook for >> OMAP5 to enable power management and mux initialization. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- [..] >> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c >> index e948a39..cdd1264 100644 >> --- a/arch/arm/mach-omap2/io.c >> +++ b/arch/arm/mach-omap2/io.c >> @@ -636,6 +636,14 @@ void __init omap5_init_early(void) >> omap_hwmod_init_postsetup(); >> >> } >> + >> +void __init omap5_init_late(void) >> +{ >> + omap_mux_late_init(); >> + omap2_common_pm_late_init(); >> + omap4_pm_init(); >> + omap2_clk_enable_autoidle_all(); >> +} > > Since you're consolidating, why not rename omap4430_init_late to > omap4plus_init_late and use it for both OMAP4 and OMAP5? > Now re-looking again, I need to drop omap_mux_late_init() since OMAP5 is DT only. And hence OMAP5 needs to be a separate hook at least for now. Regards, Santosh
On Thursday 04 April 2013 02:19 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> Add power management code to handle the CPU off mode to enable CPUP hotplug >> mode for OMAP5 devices. Separate suspend finisher is used for OMAP5(Cortex-A15) >> because it doesn't use SCU power status register and external PL310 L2 cache >> which makes code flow bit different. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > > [...] > >> @@ -436,14 +445,21 @@ int __init omap4_mpuss_init(void) >> >> if (cpu_is_omap44xx()) { >> omap_pm_ops.finish_suspend = omap4_finish_suspend; >> + omap_pm_ops.hotplug_restart = omap_secondary_startup; >> omap_pm_ops.resume = omap4_cpu_resume; >> omap_pm_ops.scu_prepare = scu_pwrst_prepare; >> cpu_context_offset = OMAP4_RM_CPU0_CPU0_CONTEXT_OFFSET; >> } else if (soc_is_omap54xx()) { >> + omap_pm_ops.finish_suspend = omap5_finish_suspend; >> + omap_pm_ops.hotplug_restart = omap5_secondary_startup; >> cpu_context_offset = OMAP54XX_RM_CPU0_CPU0_CONTEXT_OFFSET; >> enable_mercury_retention_mode(); >> } >> >> + /* Over-write the OMAP4 hook to take care of ROM BUG */ >> + if (cpu_is_omap446x()) >> + omap_pm_ops.hotplug_restart = omap_secondary_startup_4460; > > A couple nits... > > I think this would go better at the end of the 'if omap44xx' block > above. > Nishant commented on this as well. The indentation was looking ugly and I thought its better to have this BUG hunk separate. I prefer it this way though if you really insist, i have to change it. > Also, while you're hear, maybe it's time to rename the current secondary > startup functions to match the new one. IOW omap4_..., omap4460_... and omap5_... > Good idea. Will do it in a separate patch since these have been there in few files. Regards, Santosh
On Thursday 04 April 2013 02:28 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> When the entire MPUSS cluster is powered down in device off state, L2 cache >> memory looses it's content and hence while targetting such a state, >> l2 cache needs to be flushed to main memory. >> >> Add the necessary low power code support for the same. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- >> arch/arm/mach-omap2/omap-secure.h | 1 + >> arch/arm/mach-omap2/sleep_omap4plus.S | 21 +++++++++++++++++++++ >> 2 files changed, 22 insertions(+) >> >> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h >> index 1739468..a171a5a 100644 >> --- a/arch/arm/mach-omap2/omap-secure.h >> +++ b/arch/arm/mach-omap2/omap-secure.h >> @@ -47,6 +47,7 @@ >> #define OMAP4_MON_L2X0_PREFETCH_INDEX 0x113 >> #define OMAP5_MON_CACHES_CLEAN_INDEX 0x103 >> #define OMAP5_MON_AUX_CTRL_INDEX 0x107 >> +#define OMAP5_MON_L2AUX_CTRL_INDEX 0x104 > > this #define is not used in this patch. > Yep. Will drop above hunk. I forgot to drop this one after dropping the L2X0 setting from last version. Its no longer needed on OMAP5 ES2.0. Regards, Santosh
On Thursday 04 April 2013 02:33 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> OMAP4 CPUidle driver registration call is under a loop which leads >> to calling cpuidle_register_driver twice which is not intended. >> >> Fix it by moving the driver registration outside the loop. >> >> Reported-by: Nishanth Menon <nm@ti.com> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > > Looks good. > > I've already got a for_3.10/fixes/cpuidle branch going, so I'll apply > this one there. > ok. Regards, Santosh
On Thursday 04 April 2013 02:33 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> If the CPUidle device registration fails for some reason, we should >> unregister the driver on error path. >> >> Fix the code accordingly. Also when at it, check of the driver registration >> failure too. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > > Adding to my for_3.10/fixes/cpuidle branch. > Thanks. regards, Santosh
On Thursday 04 April 2013 02:35 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> It is useful to know the CPU power state along with MPUSS power state >> in a supported C-state. Since the data is available via sysfs, one can >> avoid scrolling the source code for precise construction of C-state. >> >> Reported-by: Nishanth Menon <nm@ti.com> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > > Nice. > > Adding to for_3.10/fixes/cpuidle branch. > Thanks. regards, Santosh
On Thursday 04 April 2013 03:07 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> Current OMAP4 CPUIdle driver is using omap4_mpuss_read_prev_context_state() >> function to check whether the MPU cluster lost context or not. Thanks to >> couple CPUIdle, cluster low power entry is almost guaranteed and hence >> the programmed cluster check is enough in idle exit path. The API was >> more of an optimization for corner cases, where if the cluster low power >> entry fails for some reason, the cluster context restore gets skipped. >> >> Moving forward, OMAP CPUidle drivers needs to be moved to drivers/idle/* >> once the PRM/CM code gets moved to drivers. This patch also reduces one >> dependency with platform code for idle driver movement. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > > Looks good, but I find the changelog a bit confusing. > > Below is a slightly modified patch (I only modified the changelog). If > those changes are OK with you, I'll add it to my cpuidle branch as well. > Looks good to me Kevin. Thanks for the change-log update. Regards, Santosh
On Thursday 04 April 2013 02:40 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> The OMAP5 idle driver can re-use most of OMAP4 CPUidle driver >> implementation. Also the next derivative SOCs are going to re-use >> the MPUSS so, same driver with minor updates can be re-used. >> >> Prepare the code so that its easier to add CPUidle support for >> OMAP5 devices. >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> --- >> arch/arm/mach-omap2/cpuidle44xx.c | 31 ++++++++++++++++--------------- >> 1 file changed, 16 insertions(+), 15 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c >> index b8a22f0..ac6d526 100644 >> --- a/arch/arm/mach-omap2/cpuidle44xx.c >> +++ b/arch/arm/mach-omap2/cpuidle44xx.c >> @@ -1,7 +1,7 @@ >> /* >> - * OMAP4 CPU idle Routines >> + * OMAP4PLUS CPU idle Routines > > nit: s/PLUS/+/ > > in a few other places in this patch also. > yes. Will update that. >> @@ -24,13 +24,13 @@ >> #include "clockdomain.h" >> >> /* Machine specific information */ >> -struct omap4_idle_statedata { >> +struct idle_statedata { >> u32 cpu_state; >> u32 mpu_logic_state; >> u32 mpu_state; >> }; >> >> -static struct omap4_idle_statedata omap4_idle_data[] = { >> +static struct idle_statedata omap4_idle_data[] = { >> { >> .cpu_state = PWRDM_POWER_ON, >> .mpu_state = PWRDM_POWER_ON, >> @@ -53,11 +53,12 @@ static struct clockdomain *cpu_clkdm[NR_CPUS]; >> >> static atomic_t abort_barrier; >> static bool cpu_done[NR_CPUS]; >> +static struct idle_statedata *state_ptr = &omap4_idle_data[0]; > > The assignment at init time (from the next patch) should be done here > for 44xx, and the next patch can just add OMAP5 support. > You don't need that to be differentiated at init at this patch since only OMAP4 is supported and static assignment takes care of it. Next patch ofcouse we need to differentiate and hence added the checks there. Regards, Santosh
On Thursday 04 April 2013 02:55 AM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> The OMAP5 idle driver can re-use OMAP4 CPUidle driver implementation thanks >> to compatible MPUSS design. >> >> Though unlike OMAP4, on OMAP5 devices, MPUSS CSWR (Close Switch >> Retention) power states can be achieved with respective power states >> on CPU0 and CPU1 power domain. This mode was broken on OMAP4 devices >> because of hardware limitation. > > I'm a bit confused by the description here. > > I gather from the code that this means that CPU0 and CPU1 can hit CSWR > independently, correct? > They can be programmed independently without any ordering(like CPU0 last etc), but the actual transition to the low power CSWR happens together. Till that the first CPU hit WFI remains in WFI state waiting for other CPU to hit WFI and then both transition together. Completely managed inside hardware. >> Also there is no CPU low power entry order requirement like >> master CPU etc for CSWR C-state, which is icing on the cake. > > Even on secure devices? > Yes. On secure devices too. Actually since we don't loose context, secure entry/exit doesn't come into picture. >> Code makes use of voting scheme for cluster low power state to support >> MPUSS CSWR C-state. > > The voting scheme and associated locking should be better described > here, or commented in the code itself. > You are right. It deserves some description. >> Supported OMAP5 CPUidle C-states: >> C1 - CPU0 ON(WFI) + CPU1 ON(WFI) + MPUSS ON >> C2 - CPU0 CSWR + CPU1 CSWR + MPUSS CSWR >> C3 - CPU0 OFF + CPU1 OFF + MPUSS OSWR >> >> Acked-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > > [...] > >> +static int omap_enter_idle_smp(struct cpuidle_device *dev, >> + struct cpuidle_driver *drv, >> + int index) >> +{ >> + struct idle_statedata *cx = state_ptr + index; >> + int cpu_id = smp_processor_id(); >> + unsigned long flag; >> + >> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); > > I think the CPUidle core handles the broadcast notification now. > Not in mainline yet. And those patches came after my patches and I don't wanted un-necessary merge dependency, I avoided it. Its trivial though to drop if from here once the idle next is merged. >> + raw_spin_lock_irqsave(&mpu_lock, flag); >> + cx->mpu_state_vote++; > > How about using an atomic_t and atomic_inc()/atomic_dec() instead, which > will avoid the need for a spinlock. > Spin lock is not just for the vote variable. I had atomics opps in first version I gave it to product team. But they found a race condition in where MPU power state was getting overwritten by other CPU. > Even with that, it still seems potentially racy. Do you need a memory > barrier here to be sure that any changes from another CPU are visible > here, and vice versa? > With locks, you don't need barriers since the updated copy is guaranteed. Can you please elaborate on potential race ? I have given pretty hard thought and didn't see any race which can be exposed with locks in place. Regards, Santosh
On Thursday 04 April 2013 04:22 AM, Kevin Hilman wrote: > Hi Santosh, > > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted >> earlier (March 1st 2013). Patch-set incorporates comments from Nishant >> Menon (Thanks for review NM) and his acked-by tags. I would like to get this >> queued for 3.10 merge window if you are ok with the series. > > I did a review of this series with several comments on the individual > patches. > Thanks a lot Kevin. I have addressed all the comments and have updated patches ready. > I've already queued most of the CPUidle fixes/cleanups in my > for_3.10/cleanup/cpuidle branch and will get that to Tony soon. For the > rest, I suggest probably breaking it up into cleanup/consolidation stuff > and then OMAP5 support. The latter will depend on all the OMAP5 > data/headers but the cleanup/consolidation stuff could be merged sooner > (but probably a bit late for v3.10 now.) > I have already split the patch-set into 'consolidation' and 'OMAP5 support' since it was straight forward. Need to just build the branches on top of your branches. So waiting for you to update your 'for_3.10/*' branches. For the merge, its your call:) As you said OMAP5 support(couple of patches) has a dependency on OMAP5 data files. I will at least make the branches ready on top of yours. Regards, Santosh
On Thursday 04 April 2013 08:04 PM, Santosh Shilimkar wrote: > On Thursday 04 April 2013 04:22 AM, Kevin Hilman wrote: >> Hi Santosh, >> >> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >> >>> Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted >>> earlier (March 1st 2013). Patch-set incorporates comments from Nishant >>> Menon (Thanks for review NM) and his acked-by tags. I would like to get this >>> queued for 3.10 merge window if you are ok with the series. >> >> I did a review of this series with several comments on the individual >> patches. >> > Thanks a lot Kevin. I have addressed all the comments and have > updated patches ready. > >> I've already queued most of the CPUidle fixes/cleanups in my >> for_3.10/cleanup/cpuidle branch and will get that to Tony soon. For the >> rest, I suggest probably breaking it up into cleanup/consolidation stuff >> and then OMAP5 support. The latter will depend on all the OMAP5 >> data/headers but the cleanup/consolidation stuff could be merged sooner >> (but probably a bit late for v3.10 now.) >> > I have already split the patch-set into 'consolidation' and 'OMAP5 > support' since it was straight forward. Need to just build the > branches on top of your branches. So waiting for you to update > your 'for_3.10/*' branches. > > For the merge, its your call:) > As you said OMAP5 support(couple of patches) has a dependency > on OMAP5 data files. I will at least make the branches ready on > top of yours. > After looking at your tree, looks like the branches are already there. Idle branch name is not for_3.10/fixes/cpuidle though :) So I created 'for_3.10/omap_pm-cleanup' and 'for_3.10/omap5_pm_v2' and pushed on my git tree [1] in case you would like to have a look. I briefly tested them on OMAP4 device with CPUidle enabled and they seems to work fine. Will do OMAP5 testing tomorrow and then post the updated patches on list. Regards, Santosh [1] git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > On Thursday 04 April 2013 02:19 AM, Kevin Hilman wrote: >> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >> >>> Add power management code to handle the CPU off mode to enable CPUP hotplug >>> mode for OMAP5 devices. Separate suspend finisher is used for OMAP5(Cortex-A15) >>> because it doesn't use SCU power status register and external PL310 L2 cache >>> which makes code flow bit different. >>> >>> Acked-by: Nishanth Menon <nm@ti.com> >>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> >> [...] >> >>> @@ -436,14 +445,21 @@ int __init omap4_mpuss_init(void) >>> >>> if (cpu_is_omap44xx()) { >>> omap_pm_ops.finish_suspend = omap4_finish_suspend; >>> + omap_pm_ops.hotplug_restart = omap_secondary_startup; >>> omap_pm_ops.resume = omap4_cpu_resume; >>> omap_pm_ops.scu_prepare = scu_pwrst_prepare; >>> cpu_context_offset = OMAP4_RM_CPU0_CPU0_CONTEXT_OFFSET; >>> } else if (soc_is_omap54xx()) { >>> + omap_pm_ops.finish_suspend = omap5_finish_suspend; >>> + omap_pm_ops.hotplug_restart = omap5_secondary_startup; >>> cpu_context_offset = OMAP54XX_RM_CPU0_CPU0_CONTEXT_OFFSET; >>> enable_mercury_retention_mode(); >>> } >>> >>> + /* Over-write the OMAP4 hook to take care of ROM BUG */ >>> + if (cpu_is_omap446x()) >>> + omap_pm_ops.hotplug_restart = omap_secondary_startup_4460; >> >> A couple nits... >> >> I think this would go better at the end of the 'if omap44xx' block >> above. >> > Nishant commented on this as well. The indentation was looking ugly > and I thought its better to have this BUG hunk separate. I prefer > it this way though if you really insist, i have to change it. I insist. >> Also, while you're hear, maybe it's time to rename the current secondary >> startup functions to match the new one. IOW omap4_..., omap4460_... and omap5_... >> > Good idea. Will do it in a separate patch since these have been there in few > files. OK. Thanks, Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > On Thursday 04 April 2013 02:24 AM, Kevin Hilman wrote: >> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >> >>> While waking up CPU from off state using clock domain force wakeup, restore >>> the CPU power state to ON state before putting CPU clock domain under >>> hardware control. Otherwise CPU wakeup might fail. The change is recommended >>> for all OMAP4+ devices though the PRCM weakness was observed on OMAP5 >>> devices first. >> >> Sounds reasonable, but can you describe the "weakness" a little more? >> >> IOW, what exactly happens if this is not done? It sounds like the CPU >> might immediately go back to retention, but how does that happen unless >> it does a WFI? >> > Its more of lock-up inside the hardware state machine and results > are UN-predictable. We have seen hard-locks most of the time where system > is just frozen. The hardware gets into wrong state machine if the power > domain state isn't restored. I will add this information to changelog. > >> Also, this sounds like a fix to me, and should probably be broken out >> accordingly. >> > Yeah. You mean a separate patch from the series, right ? This patch > actually can be independently added. > > In case you decide to apply it for the fixes branch, updated patch > at end of the email. Curious which branch you applied it to? It didn't apply cleanly to v3.9-rc5 (but did with fuzz). So I've now added it to my for_3.10/fixes/pm branch. Kevin > From a0cef44760d859b63a34603010f8c0621da4b8ab Mon Sep 17 00:00:00 2001 > From: Santosh Shilimkar <santosh.shilimkar@ti.com> > Date: Fri, 8 Feb 2013 22:50:58 +0530 > Subject: [PATCH] ARM: OMAP4+: PM: Restore CPU power state to ON with > clockdomain force wakeup method > > While waking up CPU from off state using clock domain force wakeup, restore > the CPU power state to ON state before putting CPU clock domain under > hardware control. Otherwise CPU wakeup might fail. The change is recommended > for all OMAP4+ devices though the PRCM weakness was observed on OMAP5 > devices first. > > As a result of weakness, lock-up is observed inside the hardware state > machine of local CPU PRCM and results are UN-predictable as per designers. > In software testing, we have seen hard-locks most of the time where system > gets frozen. With power domain state restored, system behaves correctly. > > So update the code accordingly. > > Acked-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > --- > arch/arm/mach-omap2/cpuidle44xx.c | 1 + > arch/arm/mach-omap2/omap-smp.c | 12 ++++++++++-- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c > index 944e64a..9de47a7 100644 > --- a/arch/arm/mach-omap2/cpuidle44xx.c > +++ b/arch/arm/mach-omap2/cpuidle44xx.c > @@ -131,6 +131,7 @@ static int omap4_enter_idle_coupled(struct cpuidle_device *dev, > /* Wakeup CPU1 only if it is not offlined */ > if (dev->cpu == 0 && cpumask_test_cpu(1, cpu_online_mask)) { > clkdm_wakeup(cpu_clkdm[1]); > + omap_set_pwrdm_state(cpu_pd[1], PWRDM_POWER_ON); > clkdm_allow_idle(cpu_clkdm[1]); > } > > diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c > index 5b20165..8106e8d 100644 > --- a/arch/arm/mach-omap2/omap-smp.c > +++ b/arch/arm/mach-omap2/omap-smp.c > @@ -83,6 +83,7 @@ static int __cpuinit omap4_boot_secondary(unsigned int cpu, struct task_struct * > { > static struct clockdomain *cpu1_clkdm; > static bool booted; > + static struct powerdomain *cpu1_pwrdm; > void __iomem *base = omap_get_wakeupgen_base(); > > /* > @@ -102,8 +103,10 @@ static int __cpuinit omap4_boot_secondary(unsigned int cpu, struct task_struct * > else > __raw_writel(0x20, base + OMAP_AUX_CORE_BOOT_0); > > - if (!cpu1_clkdm) > + if (!cpu1_clkdm && !cpu1_pwrdm) { > cpu1_clkdm = clkdm_lookup("mpu1_clkdm"); > + cpu1_pwrdm = pwrdm_lookup("cpu1_pwrdm"); > + } > > /* > * The SGI(Software Generated Interrupts) are not wakeup capable > @@ -116,7 +119,7 @@ static int __cpuinit omap4_boot_secondary(unsigned int cpu, struct task_struct * > * Section : > * 4.3.4.2 Power States of CPU0 and CPU1 > */ > - if (booted) { > + if (booted && cpu1_pwrdm && cpu1_clkdm) { > /* > * GIC distributor control register has changed between > * CortexA9 r1pX and r2pX. The Control Register secure > @@ -137,7 +140,12 @@ static int __cpuinit omap4_boot_secondary(unsigned int cpu, struct task_struct * > gic_dist_disable(); > } > > + /* > + * Ensure that CPU power state is set to ON to avoid CPU > + * powerdomain transition on wfi > + */ > clkdm_wakeup(cpu1_clkdm); > + omap_set_pwrdm_state(cpu1_pwrdm, PWRDM_POWER_ON); > clkdm_allow_idle(cpu1_clkdm); > > if (IS_PM44XX_ERRATUM(PM_OMAP4_ROM_SMP_BOOT_ERRATUM_GICD)) {
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > On Thursday 04 April 2013 02:55 AM, Kevin Hilman wrote: >> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >> >>> The OMAP5 idle driver can re-use OMAP4 CPUidle driver implementation thanks >>> to compatible MPUSS design. >>> >>> Though unlike OMAP4, on OMAP5 devices, MPUSS CSWR (Close Switch >>> Retention) power states can be achieved with respective power states >>> on CPU0 and CPU1 power domain. This mode was broken on OMAP4 devices >>> because of hardware limitation. >> >> I'm a bit confused by the description here. >> >> I gather from the code that this means that CPU0 and CPU1 can hit CSWR >> independently, correct? >> > They can be programmed independently without any ordering(like > CPU0 last etc), but the actual transition to the low power CSWR > happens together. Till that the first CPU hit WFI remains in WFI > state waiting for other CPU to hit WFI and then both transition > together. > Completely managed inside hardware. OK, elaborating this in the changelog would be helpful. Use the "will I understand this changelog in a year" rule to see if the changelog is detailed enough. Or better, "will Kevin understand this changelog in a year." (hint: the answer is usually no.) ;) >>> Also there is no CPU low power entry order requirement like >>> master CPU etc for CSWR C-state, which is icing on the cake. >> >> Even on secure devices? >> > Yes. On secure devices too. Actually since we don't loose context, > secure entry/exit doesn't come into picture. > >>> Code makes use of voting scheme for cluster low power state to support >>> MPUSS CSWR C-state. >> >> The voting scheme and associated locking should be better described >> here, or commented in the code itself. >> > You are right. It deserves some description. > >>> Supported OMAP5 CPUidle C-states: >>> C1 - CPU0 ON(WFI) + CPU1 ON(WFI) + MPUSS ON >>> C2 - CPU0 CSWR + CPU1 CSWR + MPUSS CSWR >>> C3 - CPU0 OFF + CPU1 OFF + MPUSS OSWR >>> >>> Acked-by: Nishanth Menon <nm@ti.com> >>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >> >> [...] >> >>> +static int omap_enter_idle_smp(struct cpuidle_device *dev, >>> + struct cpuidle_driver *drv, >>> + int index) >>> +{ >>> + struct idle_statedata *cx = state_ptr + index; >>> + int cpu_id = smp_processor_id(); >>> + unsigned long flag; >>> + >>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); >> >> I think the CPUidle core handles the broadcast notification now. >> > Not in mainline yet. And those patches came after my patches and > I don't wanted un-necessary merge dependency, I avoided it. Its trivial > though to drop if from here once the idle next is merged. OK. I believe that stuff is already queued, no? Maybe ahave this as an add-on separate patch that can be used for your loal testing, but does not go upstream. I would only include this if you're sure the other series is not going upstream. >>> + raw_spin_lock_irqsave(&mpu_lock, flag); >>> + cx->mpu_state_vote++; >> >> How about using an atomic_t and atomic_inc()/atomic_dec() instead, which >> will avoid the need for a spinlock. >> > Spin lock is not just for the vote variable. I had atomics opps in first > version I gave it to product team. But they found a race condition in > where MPU power state was getting overwritten by other CPU. > >> Even with that, it still seems potentially racy. Do you need a memory >> barrier here to be sure that any changes from another CPU are visible >> here, and vice versa? >> > With locks, you don't need barriers since the updated copy is guaranteed. It's guaranteed because the spinlock implementation uses barriers. > Can you please elaborate on potential race ? I have given pretty hard > thought and didn't see any race which can be exposed with locks in place. I was referring to using atomic ops. With atomic ops, you'd need an explicit barrier (which you're getting inside the spinlock implementation) That being said, I have not thought through all the corner cases as you have, so I'll defer to your choice (as long as it's well documented.) If you decide to stick with spinlocks, be sure to describe all of what the spinlock is protecting, and why. Thanks, Kevin
Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > On Thursday 04 April 2013 08:04 PM, Santosh Shilimkar wrote: >> On Thursday 04 April 2013 04:22 AM, Kevin Hilman wrote: >>> Hi Santosh, >>> >>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >>> >>>> Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted >>>> earlier (March 1st 2013). Patch-set incorporates comments from Nishant >>>> Menon (Thanks for review NM) and his acked-by tags. I would like to get this >>>> queued for 3.10 merge window if you are ok with the series. >>> >>> I did a review of this series with several comments on the individual >>> patches. >>> >> Thanks a lot Kevin. I have addressed all the comments and have >> updated patches ready. >> >>> I've already queued most of the CPUidle fixes/cleanups in my >>> for_3.10/cleanup/cpuidle branch and will get that to Tony soon. For the >>> rest, I suggest probably breaking it up into cleanup/consolidation stuff >>> and then OMAP5 support. The latter will depend on all the OMAP5 >>> data/headers but the cleanup/consolidation stuff could be merged sooner >>> (but probably a bit late for v3.10 now.) >>> >> I have already split the patch-set into 'consolidation' and 'OMAP5 >> support' since it was straight forward. Need to just build the >> branches on top of your branches. So waiting for you to update >> your 'for_3.10/*' branches. >> >> For the merge, its your call:) >> As you said OMAP5 support(couple of patches) has a dependency >> on OMAP5 data files. I will at least make the branches ready on >> top of yours. >> > After looking at your tree, looks like the branches are already > there. Idle branch name is not for_3.10/fixes/cpuidle though :) oh, sorry. I changed it to 'cleanup'. > So I created 'for_3.10/omap_pm-cleanup' and 'for_3.10/omap5_pm_v2' > and pushed on my git tree [1] in case you would like to have > a look. I briefly tested them on OMAP4 device with > CPUidle enabled and they seems to work fine. Will do OMAP5 > testing tomorrow and then post the updated patches on list. Perfect, thanks. Kevin
On Thursday 04 April 2013 11:01 PM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> On Thursday 04 April 2013 02:19 AM, Kevin Hilman wrote: >>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >>> >>>> Add power management code to handle the CPU off mode to enable CPUP hotplug >>>> mode for OMAP5 devices. Separate suspend finisher is used for OMAP5(Cortex-A15) >>>> because it doesn't use SCU power status register and external PL310 L2 cache >>>> which makes code flow bit different. >>>> >>>> Acked-by: Nishanth Menon <nm@ti.com> >>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >>> >>> [...] >>> >>>> @@ -436,14 +445,21 @@ int __init omap4_mpuss_init(void) >>>> >>>> if (cpu_is_omap44xx()) { >>>> omap_pm_ops.finish_suspend = omap4_finish_suspend; >>>> + omap_pm_ops.hotplug_restart = omap_secondary_startup; >>>> omap_pm_ops.resume = omap4_cpu_resume; >>>> omap_pm_ops.scu_prepare = scu_pwrst_prepare; >>>> cpu_context_offset = OMAP4_RM_CPU0_CPU0_CONTEXT_OFFSET; >>>> } else if (soc_is_omap54xx()) { >>>> + omap_pm_ops.finish_suspend = omap5_finish_suspend; >>>> + omap_pm_ops.hotplug_restart = omap5_secondary_startup; >>>> cpu_context_offset = OMAP54XX_RM_CPU0_CPU0_CONTEXT_OFFSET; >>>> enable_mercury_retention_mode(); >>>> } >>>> >>>> + /* Over-write the OMAP4 hook to take care of ROM BUG */ >>>> + if (cpu_is_omap446x()) >>>> + omap_pm_ops.hotplug_restart = omap_secondary_startup_4460; >>> >>> A couple nits... >>> >>> I think this would go better at the end of the 'if omap44xx' block >>> above. >>> >> Nishant commented on this as well. The indentation was looking ugly >> and I thought its better to have this BUG hunk separate. I prefer >> it this way though if you really insist, i have to change it. > > I insist. > Will update the patch accordingly then. Regards, Santosh
On Thursday 04 April 2013 11:12 PM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> On Thursday 04 April 2013 02:24 AM, Kevin Hilman wrote: >>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >>> >>>> While waking up CPU from off state using clock domain force wakeup, restore >>>> the CPU power state to ON state before putting CPU clock domain under >>>> hardware control. Otherwise CPU wakeup might fail. The change is recommended >>>> for all OMAP4+ devices though the PRCM weakness was observed on OMAP5 >>>> devices first. >>> >>> Sounds reasonable, but can you describe the "weakness" a little more? >>> >>> IOW, what exactly happens if this is not done? It sounds like the CPU >>> might immediately go back to retention, but how does that happen unless >>> it does a WFI? >>> >> Its more of lock-up inside the hardware state machine and results >> are UN-predictable. We have seen hard-locks most of the time where system >> is just frozen. The hardware gets into wrong state machine if the power >> domain state isn't restored. I will add this information to changelog. >> >>> Also, this sounds like a fix to me, and should probably be broken out >>> accordingly. >>> >> Yeah. You mean a separate patch from the series, right ? This patch >> actually can be independently added. >> >> In case you decide to apply it for the fixes branch, updated patch >> at end of the email. > > Curious which branch you applied it to? It didn't apply cleanly to > v3.9-rc5 (but did with fuzz). > Mostly applied on top of the Tony's pull request branches. > So I've now added it to my for_3.10/fixes/pm branch. > Thanks. I will pull that in to re-base other patches. Regards, Santosh
On Thursday 04 April 2013 11:25 PM, Kevin Hilman wrote: > Santosh Shilimkar <santosh.shilimkar@ti.com> writes: > >> On Thursday 04 April 2013 02:55 AM, Kevin Hilman wrote: >>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >>> >>>> The OMAP5 idle driver can re-use OMAP4 CPUidle driver implementation thanks >>>> to compatible MPUSS design. >>>> >>>> Though unlike OMAP4, on OMAP5 devices, MPUSS CSWR (Close Switch >>>> Retention) power states can be achieved with respective power states >>>> on CPU0 and CPU1 power domain. This mode was broken on OMAP4 devices >>>> because of hardware limitation. >>> >>> I'm a bit confused by the description here. >>> >>> I gather from the code that this means that CPU0 and CPU1 can hit CSWR >>> independently, correct? >>> >> They can be programmed independently without any ordering(like >> CPU0 last etc), but the actual transition to the low power CSWR >> happens together. Till that the first CPU hit WFI remains in WFI >> state waiting for other CPU to hit WFI and then both transition >> together. >> Completely managed inside hardware. > > OK, elaborating this in the changelog would be helpful. Use the "will I > understand this changelog in a year" rule to see if the changelog is > detailed enough. Or better, "will Kevin understand this changelog in a > year." (hint: the answer is usually no.) ;) > :-) I added above description in change-log. >>>> Also there is no CPU low power entry order requirement like >>>> master CPU etc for CSWR C-state, which is icing on the cake. >>> >>> Even on secure devices? >>> >> Yes. On secure devices too. Actually since we don't loose context, >> secure entry/exit doesn't come into picture. >> >>>> Code makes use of voting scheme for cluster low power state to support >>>> MPUSS CSWR C-state. >>> >>> The voting scheme and associated locking should be better described >>> here, or commented in the code itself. >>> >> You are right. It deserves some description. >> >>>> Supported OMAP5 CPUidle C-states: >>>> C1 - CPU0 ON(WFI) + CPU1 ON(WFI) + MPUSS ON >>>> C2 - CPU0 CSWR + CPU1 CSWR + MPUSS CSWR >>>> C3 - CPU0 OFF + CPU1 OFF + MPUSS OSWR >>>> >>>> Acked-by: Nishanth Menon <nm@ti.com> >>>> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com> >>> >>> [...] >>> >>>> +static int omap_enter_idle_smp(struct cpuidle_device *dev, >>>> + struct cpuidle_driver *drv, >>>> + int index) >>>> +{ >>>> + struct idle_statedata *cx = state_ptr + index; >>>> + int cpu_id = smp_processor_id(); >>>> + unsigned long flag; >>>> + >>>> + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id); >>> >>> I think the CPUidle core handles the broadcast notification now. >>> >> Not in mainline yet. And those patches came after my patches and >> I don't wanted un-necessary merge dependency, I avoided it. Its trivial >> though to drop if from here once the idle next is merged. > > OK. > > I believe that stuff is already queued, no? Maybe ahave this as an > add-on separate patch that can be used for your loal testing, but does > not go upstream. > Will do. > I would only include this if you're sure the other series is not going > upstream. > It might go upstream so I will manually apply the patch or pull a branch if available. >>>> + raw_spin_lock_irqsave(&mpu_lock, flag); >>>> + cx->mpu_state_vote++; >>> >>> How about using an atomic_t and atomic_inc()/atomic_dec() instead, which >>> will avoid the need for a spinlock. >>> >> Spin lock is not just for the vote variable. I had atomics opps in first >> version I gave it to product team. But they found a race condition in >> where MPU power state was getting overwritten by other CPU. >> >>> Even with that, it still seems potentially racy. Do you need a memory >>> barrier here to be sure that any changes from another CPU are visible >>> here, and vice versa? >>> >> With locks, you don't need barriers since the updated copy is guaranteed. > > It's guaranteed because the spinlock implementation uses barriers. > Yeah. >> Can you please elaborate on potential race ? I have given pretty hard >> thought and didn't see any race which can be exposed with locks in place. > > I was referring to using atomic ops. With atomic ops, you'd need an > explicit barrier (which you're getting inside the spinlock > implementation) > > That being said, I have not thought through all the corner cases as you > have, so I'll defer to your choice (as long as it's well documented.) > If you decide to stick with spinlocks, be sure to describe all of what > the spinlock is protecting, and why. > Have updated the change-log as well as code to elaborate the lock use. Thanks for review. Regards, Santosh
On Friday 05 April 2013 02:37 PM, Santosh Shilimkar wrote: > On Thursday 04 April 2013 11:12 PM, Kevin Hilman wrote: >> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >> >>> On Thursday 04 April 2013 02:24 AM, Kevin Hilman wrote: >>>> Santosh Shilimkar <santosh.shilimkar@ti.com> writes: >>>> >>>>> While waking up CPU from off state using clock domain force wakeup, restore >>>>> the CPU power state to ON state before putting CPU clock domain under >>>>> hardware control. Otherwise CPU wakeup might fail. The change is recommended >>>>> for all OMAP4+ devices though the PRCM weakness was observed on OMAP5 >>>>> devices first. >>>> >>>> Sounds reasonable, but can you describe the "weakness" a little more? >>>> >>>> IOW, what exactly happens if this is not done? It sounds like the CPU >>>> might immediately go back to retention, but how does that happen unless >>>> it does a WFI? >>>> >>> Its more of lock-up inside the hardware state machine and results >>> are UN-predictable. We have seen hard-locks most of the time where system >>> is just frozen. The hardware gets into wrong state machine if the power >>> domain state isn't restored. I will add this information to changelog. >>> >>>> Also, this sounds like a fix to me, and should probably be broken out >>>> accordingly. >>>> >>> Yeah. You mean a separate patch from the series, right ? This patch >>> actually can be independently added. >>> >>> In case you decide to apply it for the fixes branch, updated patch >>> at end of the email. >> >> Curious which branch you applied it to? It didn't apply cleanly to >> v3.9-rc5 (but did with fuzz). >> > Mostly applied on top of the Tony's pull request branches. > >> So I've now added it to my for_3.10/fixes/pm branch. >> > Thanks. I will pull that in to re-base other patches. > While pulling your 'for_3.10/fixes/pm' branch on top of Tony's pull request[1] sent to arm-soc already. In my tree, I had pulled Tony's couple of pull requests. Regards, Santosh [1] http://www.spinics.net/lists/arm-kernel/msg235788.html
Kevin, Here is the refreshed version(v2) of the OMAP5 PM suspport which was posted earlier (March 1st 2013). Patch-set incorporates comments from Nishant Menon (Thanks for review NM) and his acked-by tags. I would like to get this queued for 3.10 merge window if you are ok with the series. Series is built on top of my pull requests [1] [2] [3] sent to Tony and your 'for_3.10/pm/cleanup' branch. For testing, I have created a branch [4] which put together all the needed dependencies, fixes which should make it to 3.10 merge window. Series adds OMAP5 MPUSS power management support for system wide suspend and CPUidle. Its heavy re-use from OMAP4 and hence only ~400 odd lines are needed to add OMAP5 PM support on top of existing OMAP4 PM support. OMAP5 adds a mercury retention feature which is an enhancement of existing retention feature to reduce the leakage. No change in programming model except one time enabling of mercury retention during init. One more notable change in OMAP5 vs OMAP4 devices, CPUx power domains support retention state which lets you hit MPUSS and Core retention with very low latency C-states. Tested on OMAP4430 SDP, OMAP4460 Panda, OMAP5430 SDP and OMAP5432 Panda devices with suspend and CPUIdle. Rootfs is mounted over ramdisk since the mmc and nfs based fs needs DMA engine patches. For suspend wakeup, I used Sourav's couple of serial wakeup wip patches from the lists. The following changes since commit 9981cde24de52e5bb9a7cd918d1e54de241f85c9: Merge branch 'for_3.10/omap5_data_files' of git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux into for_3.10/omap5_pm (2013-03-25 12:29:34 +0530) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux.git for_3.10/omap5_pm for you to fetch changes up to 75bd846d3103da858a208fe07127151903d1f608: ARM: OMAP5: PM: handle device instance for warm reset (2013-03-25 12:37:44 +0530) ---------------------------------------------------------------- Nishanth Menon (1): ARM: OMAP5: PM: handle device instance for warm reset Santosh Shilimkar (17): ARM: OMAP4+: PM: Consolidate MPU subsystem PM code for re-use ARM: OMAP5: PM: Update CPU context register offset ARM: OMAP4+: PM: Consolidate and use OMAP4 PM code for OMAP5 ARM: OMAP5: PM: Set MPUSS-EMIF clock-domain static dependency ARM: OMAP5: PM: Enables ES2 PM mode by default ARM: OMAP5: PM: Enable Mercury retention mode on CPUx powerdomains ARM: OMAP5: Add init_late() hook to enable pm initialization ARM: OMAP5: PM: Add CPU power off in hotplug path ARM: OMAP4+: PM: Restore CPU power state to ON with clockdomain force wakeup method ARM: OMAP5: PM: Add MPU Open Switch Retention support ARM: OMAP5: PM: Add L2 memory power down support ARM: OMAP4: CPUidle: Avoid double idle driver registration ARM: OMAP: CPUidle: Unregister drivere on device registration failure ARM: OMAP4: CPUidle: Make C-state description field more precise ARM: OMAP4+: CPUidle: Consolidate idle driver for OMAP5 support ARM: OMAP4+: CPUidle: Deprecate use of omap4_mpuss_read_prev_context_state() ARM: OMAP4+: CPUidle: Add OMAP5 idle driver support arch/arm/mach-omap2/Kconfig | 1 + arch/arm/mach-omap2/Makefile | 12 +- arch/arm/mach-omap2/board-generic.c | 1 + arch/arm/mach-omap2/common.h | 8 +- arch/arm/mach-omap2/cpuidle34xx.c | 6 +- .../{cpuidle44xx.c => cpuidle_omap4plus.c} | 151 ++++++++++++++++--- arch/arm/mach-omap2/io.c | 8 + arch/arm/mach-omap2/omap-mpuss-lowpower.c | 155 +++++++++++++++----- arch/arm/mach-omap2/omap-secure.h | 9 ++ arch/arm/mach-omap2/omap-smp.c | 12 +- arch/arm/mach-omap2/omap-wakeupgen.c | 30 +++- arch/arm/mach-omap2/omap-wakeupgen.h | 1 + arch/arm/mach-omap2/omap4-sar-layout.h | 2 + arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} | 90 ++++++++++-- arch/arm/mach-omap2/prminst44xx.c | 10 +- .../mach-omap2/{sleep44xx.S => sleep_omap4plus.S} | 106 +++++++++++++ 16 files changed, 512 insertions(+), 90 deletions(-) rename arch/arm/mach-omap2/{cpuidle44xx.c => cpuidle_omap4plus.c} (58%) rename arch/arm/mach-omap2/{pm44xx.c => pm_omap4plus.c} (74%) rename arch/arm/mach-omap2/{sleep44xx.S => sleep_omap4plus.S} (77%)