On 11:44-20130302, Santosh Shilimkar wrote:
> On Saturday 02 March 2013 03:06 AM, Nishanth Menon wrote:
> > On 17:40-20130301, Santosh Shilimkar wrote:
> >> Add power management code to handle the CPU off mode. 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.
> >>
> >> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
> >> ---
> >> arch/arm/mach-omap2/omap-mpuss-lowpower.c | 31 +++++++----
> >> arch/arm/mach-omap2/omap-secure.h | 1 +
> >> arch/arm/mach-omap2/omap4-sar-layout.h | 2 +
> >> arch/arm/mach-omap2/sleep_omap4plus.S | 80 +++++++++++++++++++++++++++++
> >> 4 files changed, 104 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> >> index 9fda96b..275f9a4 100644
> >> --- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> >> +++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
> >> @@ -76,10 +76,12 @@ 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);
> >> + void (*hotplug_restart)(void);
> >> };
> >>
> >> extern int omap4_finish_suspend(unsigned long cpu_state);
> >> extern void omap4_cpu_resume(void);
> >> +extern int omap5_finish_suspend(unsigned long cpu_state);
> >>
> >> static DEFINE_PER_CPU(struct omap4_cpu_pm_info, omap4_pm_info);
> >> static struct powerdomain *mpuss_pd;
> >> @@ -102,6 +104,7 @@ struct cpu_pm_ops omap_pm_ops = {
> >> .finish_suspend = default_finish_suspend,
> >> .resume = dummy_cpu_resume,
> >> .scu_prepare = dummy_scu_prepare,
> >> + .hotplug_restart = dummy_cpu_resume,
> >> };
> >>
> >> /*
> >> @@ -334,7 +337,6 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
> >> int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state)
> >> {
> >> unsigned int cpu_state = 0;
> >> - struct omap4_cpu_pm_info *pm_info = &per_cpu(omap4_pm_info, cpu);
> >>
> >> if (omap_rev() == OMAP4430_REV_ES1_0)
> >> return -ENXIO;
> >> @@ -344,7 +346,7 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state)
> >>
> >> clear_cpu_prev_pwrst(cpu);
> >> set_cpu_next_pwrst(cpu, power_state);
> >> - set_cpu_wakeup_addr(cpu, virt_to_phys(pm_info->secondary_startup));
> >> + set_cpu_wakeup_addr(cpu, virt_to_phys(omap_pm_ops.hotplug_restart));
> >> omap_pm_ops.scu_prepare(cpu, power_state);
> >>
> >> /*
> >> @@ -379,6 +381,7 @@ static void enable_mercury_retention_mode(void)
> >> int __init omap4_mpuss_init(void)
> >> {
> >> struct omap4_cpu_pm_info *pm_info;
> >> + u32 cpu_wakeup_addr = 0;
> >>
> >> if (omap_rev() == OMAP4430_REV_ES1_0) {
> >> WARN(1, "Power Management not supported on OMAP4430 ES1.0\n");
> >> @@ -388,9 +391,13 @@ int __init omap4_mpuss_init(void)
> >> sar_base = omap4_get_sar_ram_base();
> >>
> >> /* Initilaise per CPU PM information */
> >> + if (cpu_is_omap44xx())
> >> + cpu_wakeup_addr = CPU0_WAKEUP_NS_PA_ADDR_OFFSET;
> >> + else if (soc_is_omap54xx())
> >> + cpu_wakeup_addr = OMAP5_CPU0_WAKEUP_NS_PA_ADDR_OFFSET;
> >> pm_info = &per_cpu(omap4_pm_info, 0x0);
> >> pm_info->scu_sar_addr = sar_base + SCU_OFFSET0;
> >> - pm_info->wkup_sar_addr = sar_base + CPU0_WAKEUP_NS_PA_ADDR_OFFSET;
> >> + pm_info->wkup_sar_addr = sar_base + cpu_wakeup_addr;
> >> pm_info->l2x0_sar_addr = sar_base + L2X0_SAVE_OFFSET0;
> >> pm_info->pwrdm = pwrdm_lookup("cpu0_pwrdm");
> >> if (!pm_info->pwrdm) {
> >> @@ -405,14 +412,14 @@ int __init omap4_mpuss_init(void)
> >> /* Initialise CPU0 power domain state to ON */
> >> pwrdm_set_next_pwrst(pm_info->pwrdm, PWRDM_POWER_ON);
> >>
> >> + if (cpu_is_omap44xx())
> >> + cpu_wakeup_addr = CPU1_WAKEUP_NS_PA_ADDR_OFFSET;
> >> + else if (soc_is_omap54xx())
> >> + cpu_wakeup_addr = OMAP5_CPU1_WAKEUP_NS_PA_ADDR_OFFSET;
> >> pm_info = &per_cpu(omap4_pm_info, 0x1);
> >> pm_info->scu_sar_addr = sar_base + SCU_OFFSET1;
> >> - pm_info->wkup_sar_addr = sar_base + CPU1_WAKEUP_NS_PA_ADDR_OFFSET;
> >> + pm_info->wkup_sar_addr = sar_base + cpu_wakeup_addr;
> >> pm_info->l2x0_sar_addr = sar_base + L2X0_SAVE_OFFSET1;
> >> - if (cpu_is_omap446x())
> >> - pm_info->secondary_startup = omap_secondary_startup_4460;
> >> - else
> >> - pm_info->secondary_startup = omap_secondary_startup;
> >>
> >> pm_info->pwrdm = pwrdm_lookup("cpu1_pwrdm");
> >> if (!pm_info->pwrdm) {
> >> @@ -445,15 +452,19 @@ 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;
> > could we handle omap_pm_ops.hotplug_restart = omap_secondary_startup_4460
> > here as well with the interest of keeping all function inits
> > in consecutive source location?
> I don't wanted to have multiple indentation and 4460 bug is just
> too annoying that it is better to keep it seperate.
ok
>
>
> >> diff --git a/arch/arm/mach-omap2/omap4-sar-layout.h b/arch/arm/mach-omap2/omap4-sar-layout.h
> >> index 6822d0a..ee8215b 100644
> >> --- a/arch/arm/mach-omap2/omap4-sar-layout.h
> >> +++ b/arch/arm/mach-omap2/omap4-sar-layout.h
> >> @@ -31,6 +31,8 @@
> >> /* CPUx Wakeup Non-Secure Physical Address offsets in SAR_BANK3 */
> >> #define CPU0_WAKEUP_NS_PA_ADDR_OFFSET 0xa04
> >> #define CPU1_WAKEUP_NS_PA_ADDR_OFFSET 0xa08
> >> +#define OMAP5_CPU0_WAKEUP_NS_PA_ADDR_OFFSET 0xe00
> >> +#define OMAP5_CPU1_WAKEUP_NS_PA_ADDR_OFFSET 0xe04
> >>
> >> #define SAR_BACKUP_STATUS_OFFSET (SAR_BANK3_OFFSET + 0x500)
> >> #define SAR_SECURE_RAM_SIZE_OFFSET (SAR_BANK3_OFFSET + 0x504)
> >> diff --git a/arch/arm/mach-omap2/sleep_omap4plus.S b/arch/arm/mach-omap2/sleep_omap4plus.S
> >> index 88ff83a..3322fc8 100644
> >> --- a/arch/arm/mach-omap2/sleep_omap4plus.S
> >> +++ b/arch/arm/mach-omap2/sleep_omap4plus.S
> >> @@ -326,6 +326,86 @@ skip_l2en:
> >>
> >> b cpu_resume @ Jump to generic resume
> >> ENDPROC(omap4_cpu_resume)
> >> +
> >> +/*
> >> + * ================================
> >> + * == OMAP5 CPU suspend finisher ==
> >> + * ================================
> >> + *
> >> + * OMAP5 MPUSS states for the context save:
> >> + * save_state =
> >> + * 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: DEVICE OFF
> >> + */
> >> +ENTRY(omap5_finish_suspend)
> >> + stmfd sp!, {r4-r12, lr}
> >> + cmp r0, #0x0
> >> + beq do_wfi @ No lowpower state, jump to WFI
> >> +
> >> + /*
> >> + * Flush all data from the L1 data cache before disabling
> >> + * SCTLR.C bit.
> >> + */
> >> + bl omap4_get_sar_ram_base
> >> + ldr r9, [r0, #OMAP_TYPE_OFFSET]
> >> + cmp r9, #0x1 @ Check for HS device
> >> + bne skip_secure_l1_clean_op
> >> + mov r0, #0 @ Clean secure L1
> >> + stmfd r13!, {r4-r12, r14}
> >> + ldr r12, =OMAP5_MON_CACHES_CLEAN_INDEX
> >> + DO_SMC
> >> + ldmfd r13!, {r4-r12, r14}
> >> +skip_secure_l1_clean_op:
> >> + bl v7_flush_dcache_louis
> >> +
> >> + /*
> >> + * Clear the SCTLR.C bit to prevent further data cache
> >> + * allocation. Clearing SCTLR.C would make all the data accesses
> >> + * strongly ordered and would not hit the cache.
> >> + */
> >> + mrc p15, 0, r0, c1, c0, 0
> >> + bic r0, r0, #(1 << 2) @ Disable the C bit
> >> + mcr p15, 0, r0, c1, c0, 0
> >> + isb
> >> +
> >> + /* Clean and Invalidate L1 data cache. */
> >> + bl v7_flush_dcache_louis
> > Curious question - once we have flushed and invalidated L1 on
> > skip_secure_l1_clean_op:, we disable SCTLR.C to make all accesses SO,
> > what is the need to go through clean and invalidate again? is'nt it an
> > un-necessary cycle consuming NOP? What kind of data are we cleaning out
> > here?
> Obviously you haven't read all the old emails internally as well as
> externally on this topic. Its needed.
OK. Might be good to briefly document the same in code comments.
>
> >> +
> >> + /*
> >> + * Take CPU out of Symmetric Multiprocessing (SMP) mode and thus
> >> + * preventing the CPU from receiving cache, TLB, or BTB
> >> + * maintenance operations broadcast by other CPUs in the cluster.
> >> + */
> >> + mrc p15, 0, r0, c1, c1, 2 @ Read NSACR data
> >> + tst r0, #(1 << 18)
> >> + mrcne p15, 0, r0, c1, c0, 1
> >> + bicne r0, r0, #(1 << 6) @ Disable SMP bit
> >> + mcrne p15, 0, r0, c1, c0, 1
> >> + isb
> >> + dsb
> >> +
> > when save_state=3, as per the function comment:
> > 3 - CPUx L1 and logic lost + GIC + L2 lost: DEVICE OFF
> > But we dont flush L2 nonsecure OR secure at this point. results wont be
> > pretty.
> Subject adds CPU off support only. I have added the comments section in
> this patch. I can move that for next patch.
Thanks - it will be good NOT to have DEVICE OFF mentioned here.
>
> > If we dont want to deal with "3", then should'nt we add an adequate
> > handler on entry and remove the code comment claiming we support
> > it?
> > is'nt it better to squash in "[PATCH 11/15] ARM: OMAP5: PM: Add L2
> > memory power down support" here?
> Why. You don't need L2 support for CPU OFF. As I said I will move the
> comment in further patch which seems to confused you. And sure, I will
> remove that device OFF reference but still support mode where L2
> can be destroyed. It can be done in MPU OSWR as well with a setting
> to loose L2.
yep - the DEVICE OFF documentation threw me off I suppose.
>
> >> +do_wfi:
> >> + bl omap_do_wfi
> >> +
> >> + /*
> >> + * CPU is here when it failed to enter OFF/DORMANT or
> >> + * no low power state was attempted.
> > This might need a bit more clarity IMHO. successful WFI (which is
> > arguably an power state if we consider ARM internal clock gating takes
> > place), will reach here as well. So would OMAP CPU INA state. The
> > comment is probably valid for the defined save_states in code comments
> > for CPU OFF which is un-successful.
> >> + */
> >> + mrc p15, 0, r0, c1, c0, 0
> >> + tst r0, #(1 << 2) @ Check C bit enabled?
> >> + orreq r0, r0, #(1 << 2) @ Enable the C bit
> >> + mcreq p15, 0, r0, c1, c0, 0
> >> + isb
> >> + mrc p15, 0, r0, c1, c0, 1
> >> + tst r0, #(1 << 6) @ Check SMP bit enabled?
> >> + orreq r0, r0, #(1 << 6)
> >> + mcreq p15, 0, r0, c1, c0, 1
> >> + isb
> >> + dsb
> >> + ldmfd sp!, {r4-r12, pc}
> >> +ENDPROC(omap5_finish_suspend)
> >> #endif
> >>
> >> #ifndef CONFIG_OMAP4_ERRATA_I688
> >
> > I was curious at this point -> given that we added documentation that
> > we will be able to go to CPU OFF here, but I did not see an resume
> > handler registered. Untill I looked down in the series to see:
> >
> I should have mentioned abou CPU OFF in hotplug path which doesn't
> need resume entry. Will update commit message accordingly. Thanks
> for comments.
Thanks.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
