[PATCH 0/2] OMAP3 PM: sleep code clean up

[PATCH 0/2] OMAP3 PM: sleep code clean up

[Vishwanath BS]

This patch series has some clean up in OMAP3 sleep code.
Patches have been rebased to latest kevin's pm branch.

Vishwanath BS (2):
  OMAP3 PM: move omap3 sleep to ddr
  OMAP3 PM: sleep code clean up

 arch/arm/mach-omap2/pm34xx.c              |    9 +-
 arch/arm/mach-omap2/sleep34xx.S           |  377 ++++++++++++++---------------
 arch/arm/plat-omap/include/plat/control.h |    2 +
 3 files changed, 190 insertions(+), 198 deletions(-)

[PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Vishwanath BS]

There is no need to keep omap3 sleep code in SRAM. This code can be run very
well on DDR. This would help us to instrument CPUIdle latencies.

Tested on ZOOM3.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Kevin Hillman <khilman@deeprootsystems.com>
Cc: <Linaro> linaro-dev@lists.linaro.org
---
 arch/arm/mach-omap2/pm34xx.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 85ef245..ed9d12c 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -79,8 +79,6 @@ struct power_state {
 
 static LIST_HEAD(pwrst_list);
 
-static void (*_omap_sram_idle)(u32 *addr, int save_state);
-
 static int (*_omap_save_secure_sram)(u32 *addr);
 
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
@@ -360,9 +358,6 @@ void omap_sram_idle(void)
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;
 
-	if (!_omap_sram_idle)
-		return;
-
 	pwrdm_clear_all_prev_pwrst(mpu_pwrdm);
 	pwrdm_clear_all_prev_pwrst(neon_pwrdm);
 	pwrdm_clear_all_prev_pwrst(core_pwrdm);
@@ -438,7 +433,7 @@ void omap_sram_idle(void)
 	 * get saved. The restore path then reads from this
 	 * location and restores them back.
 	 */
-	_omap_sram_idle(omap3_arm_context, save_state);
+	omap34xx_cpu_suspend(omap3_arm_context, save_state);
 	cpu_init();
 
 	if (is_suspending())
@@ -995,8 +990,6 @@ static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
 
 void omap_push_sram_idle(void)
 {
-	_omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
-					omap34xx_cpu_suspend_sz);
 	if (omap_type() != OMAP2_DEVICE_TYPE_GP)
 		_omap_save_secure_sram = omap_sram_push(save_secure_ram_context,
 				save_secure_ram_context_sz);
-- 
1.7.0.4

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Tony Lindgren]

* Vishwanath BS <vishwanath.bs@ti.com> [100924 03:50]:
> There is no need to keep omap3 sleep code in SRAM. This code can be run very
> well on DDR. This would help us to instrument CPUIdle latencies.

Uhh, are you sure about this? To me it sounds like you're then
relying on the code running from the cache for off-idle?

What about CONFIG_CPU_ICACHE_DISABLE?

Regards,

Tony

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [100924 11:53]:
> * Vishwanath BS <vishwanath.bs@ti.com> [100924 03:50]:
> > There is no need to keep omap3 sleep code in SRAM. This code can be run very
> > well on DDR. This would help us to instrument CPUIdle latencies.
> 
> Uhh, are you sure about this? To me it sounds like you're then
> relying on the code running from the cache for off-idle?

Actually the caches are off too for off-idle..

Tony

RE: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Shilimkar, Santosh]

> -----Original Message-----
> From: linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-omap-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Tony Lindgren
> Sent: Saturday, September 25, 2010 12:40 AM
> To: Sripathy, Vishwanath
> Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org; Kevin Hillman
> Subject: Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr
> 
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [100924 11:53]:
> > * Vishwanath BS <vishwanath.bs-l0cyMroinI0@public.gmane.org> [100924 03:50]:
> > > There is no need to keep omap3 sleep code in SRAM. This code can be
> run very
> > > well on DDR. This would help us to instrument CPUIdle latencies.
> >
> > Uhh, are you sure about this? To me it sounds like you're then
> > relying on the code running from the cache for off-idle?
> 
> Actually the caches are off too for off-idle..
> 
The low power code can be executed from DDR and need not be from
cache. The only real implementation on OMAP3 is around DVFS code,
where you need to put DDR to self refresh and execute some
piece of code. That still needs to be executed from some other
memory like (SRAM).

Regards,
Santosh

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> There is no need to keep omap3 sleep code in SRAM. 
> This code can be run very well on DDR.

/me remains skeptical

> This would help us to instrument CPUIdle latencies.

Indeed, but...

I'm afraid we will need a much more descriptive changelog here,
describing in detail why this will still work, and what about the
previous motiviations for running SRAM has changed such that this will
work out of SRAM.

> Tested on ZOOM3.

Tested in what way?  This path is used for both idle and suspend, and 
needs to be validated for retention and off mode.

Kevin

> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Kevin Hillman <khilman@deeprootsystems.com>
> Cc: <Linaro> linaro-dev@lists.linaro.org
> ---
>  arch/arm/mach-omap2/pm34xx.c |    9 +--------
>  1 files changed, 1 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 85ef245..ed9d12c 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -79,8 +79,6 @@ struct power_state {
>  
>  static LIST_HEAD(pwrst_list);
>  
> -static void (*_omap_sram_idle)(u32 *addr, int save_state);
> -
>  static int (*_omap_save_secure_sram)(u32 *addr);
>  
>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
> @@ -360,9 +358,6 @@ void omap_sram_idle(void)
>  	int core_prev_state, per_prev_state;
>  	u32 sdrc_pwr = 0;
>  
> -	if (!_omap_sram_idle)
> -		return;
> -
>  	pwrdm_clear_all_prev_pwrst(mpu_pwrdm);
>  	pwrdm_clear_all_prev_pwrst(neon_pwrdm);
>  	pwrdm_clear_all_prev_pwrst(core_pwrdm);
> @@ -438,7 +433,7 @@ void omap_sram_idle(void)
>  	 * get saved. The restore path then reads from this
>  	 * location and restores them back.
>  	 */
> -	_omap_sram_idle(omap3_arm_context, save_state);
> +	omap34xx_cpu_suspend(omap3_arm_context, save_state);
>  	cpu_init();
>  
>  	if (is_suspending())
> @@ -995,8 +990,6 @@ static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
>  
>  void omap_push_sram_idle(void)
>  {
> -	_omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> -					omap34xx_cpu_suspend_sz);
>  	if (omap_type() != OMAP2_DEVICE_TYPE_GP)
>  		_omap_save_secure_sram = omap_sram_push(save_secure_ram_context,
>  				save_secure_ram_context_sz);

RE: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Sripathy, Vishwanath]

Kevin,

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Saturday, September 25, 2010 1:28 AM
> To: Sripathy, Vishwanath
> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
> Subject: Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr
> 
> Vishwanath BS <vishwanath.bs@ti.com> writes:
> 
> > There is no need to keep omap3 sleep code in SRAM.
> > This code can be run very well on DDR.
> 
> /me remains skeptical
> 
> > This would help us to instrument CPUIdle latencies.
> 
> Indeed, but...
> 
> I'm afraid we will need a much more descriptive changelog here,
> describing in detail why this will still work, and what about the
> previous motiviations for running SRAM has changed such that this will
> work out of SRAM.
Running the code from SRAM for CPUIdle was more of legacy from OMAP2 and there was no technical reason why it cannot be run from DDR. Also note that, once MPU enters off mode, wake up code used to run from DDR earlier (w/o these changes) as well. I will put more details in the change log.
> 
> > Tested on ZOOM3.
> 
> Tested in what way?  This path is used for both idle and suspend, and
> needs to be validated for retention and off mode.
I have tested for both retention and off mode in both CPU idle and suspend path. I will update the changelog in next version. 

Vishwa
> 
> Kevin
> 
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > Cc: Kevin Hillman <khilman@deeprootsystems.com>
> > Cc: <Linaro> linaro-dev@lists.linaro.org
> > ---
> >  arch/arm/mach-omap2/pm34xx.c |    9 +--------
> >  1 files changed, 1 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> > index 85ef245..ed9d12c 100644
> > --- a/arch/arm/mach-omap2/pm34xx.c
> > +++ b/arch/arm/mach-omap2/pm34xx.c
> > @@ -79,8 +79,6 @@ struct power_state {
> >
> >  static LIST_HEAD(pwrst_list);
> >
> > -static void (*_omap_sram_idle)(u32 *addr, int save_state);
> > -
> >  static int (*_omap_save_secure_sram)(u32 *addr);
> >
> >  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
> > @@ -360,9 +358,6 @@ void omap_sram_idle(void)
> >  	int core_prev_state, per_prev_state;
> >  	u32 sdrc_pwr = 0;
> >
> > -	if (!_omap_sram_idle)
> > -		return;
> > -
> >  	pwrdm_clear_all_prev_pwrst(mpu_pwrdm);
> >  	pwrdm_clear_all_prev_pwrst(neon_pwrdm);
> >  	pwrdm_clear_all_prev_pwrst(core_pwrdm);
> > @@ -438,7 +433,7 @@ void omap_sram_idle(void)
> >  	 * get saved. The restore path then reads from this
> >  	 * location and restores them back.
> >  	 */
> > -	_omap_sram_idle(omap3_arm_context, save_state);
> > +	omap34xx_cpu_suspend(omap3_arm_context, save_state);
> >  	cpu_init();
> >
> >  	if (is_suspending())
> > @@ -995,8 +990,6 @@ static int __init clkdms_setup(struct clockdomain *clkdm,
> void *unused)
> >
> >  void omap_push_sram_idle(void)
> >  {
> > -	_omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> > -					omap34xx_cpu_suspend_sz);
> >  	if (omap_type() != OMAP2_DEVICE_TYPE_GP)
> >  		_omap_save_secure_sram =
> omap_sram_push(save_secure_ram_context,
> >  				save_secure_ram_context_sz);

[PATCH 2/2] OMAP3 PM: sleep code clean up

[Vishwanath BS]

This patch has done some clean up of omap3 sleep code.
Basically all possible hardcodings are removed and code is Reorganized
into more logical buckets for better readability and instrumentation.

Tested on ZOOM3.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Kevin Hillman <khilman@deeprootsystems.com>
Cc: <Linaro> linaro-dev@lists.linaro.org
---
 arch/arm/mach-omap2/sleep34xx.S           |  377 ++++++++++++++---------------
 arch/arm/plat-omap/include/plat/control.h |    2 +
 2 files changed, 189 insertions(+), 190 deletions(-)

diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index ba53191..734f82a
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -33,17 +33,20 @@
 #include "prm.h"
 #include "sdrc.h"
 
-#define SDRC_SCRATCHPAD_SEM_V	0xfa00291c
+#define SDRC_SCRATCHPAD_SEM_OFFS	0xc
+#define SDRC_SCRATCHPAD_SEM_V	OMAP343X_SCRATCHPAD_REGADDR \
+							(SDRC_SCRATCHPAD_SEM_OFFS)
 
 #define PM_PREPWSTST_CORE_V	OMAP34XX_PRM_REGADDR(CORE_MOD, \
-				OMAP3430_PM_PREPWSTST)
-#define PM_PREPWSTST_CORE_P	0x48306AE8
+							OMAP3430_PM_PREPWSTST)
+#define PM_PREPWSTST_CORE_P	OMAP3430_PRM_BASE + CORE_MOD + \
+								OMAP3430_PM_PREPWSTST
 #define PM_PREPWSTST_MPU_V	OMAP34XX_PRM_REGADDR(MPU_MOD, \
 				OMAP3430_PM_PREPWSTST)
 #define PM_PWSTCTRL_MPU_P	OMAP3430_PRM_BASE + MPU_MOD + OMAP2_PM_PWSTCTRL
 #define CM_IDLEST1_CORE_V	OMAP34XX_CM_REGADDR(CORE_MOD, CM_IDLEST1)
-#define SRAM_BASE_P		0x40200000
-#define CONTROL_STAT		0x480022F0
+#define SRAM_BASE_P			0x40200000
+#define CONTROL_STAT		OMAP343X_CTRL_BASE + OMAP343X_CONTROL_STATUS
 #define SCRATCHPAD_MEM_OFFS	0x310 /* Move this as correct place is
 				       * available */
 #define SCRATCHPAD_BASE_P	(OMAP343X_CTRL_BASE + OMAP343X_CONTROL_MEM_WKUP\
@@ -184,29 +187,16 @@ api_params:
 ENTRY(save_secure_ram_context_sz)
 	.word	. - save_secure_ram_context
 
-/*
- * Forces OMAP into idle state
- *
- * omap34xx_suspend() - This bit of code just executes the WFI
- * for normal idles.
- *
- * Note: This code get's copied to internal SRAM at boot. When the OMAP
- *	 wakes up it continues execution at the point it went to sleep.
- */
-ENTRY(omap34xx_cpu_suspend)
+/* Function to execute WFI. When the MPU wakes up from retention
+ * or inactive mode, it continues execution just after wfi */
+ENTRY(omap34xx_do_wfi)
 	stmfd	sp!, {r0-r12, lr}		@ save registers on stack
-loop:
-	/*b	loop*/	@Enable to debug by stepping through code
-	/* r0 contains restore pointer in sdram */
-	/* r1 contains information about saving context */
+
 	ldr     r4, sdrc_power          @ read the SDRC_POWER register
 	ldr     r5, [r4]                @ read the contents of SDRC_POWER
 	orr     r5, r5, #0x40           @ enable self refresh on idle req
 	str     r5, [r4]                @ write back to SDRC_POWER register
 
-	cmp	r1, #0x0
-	/* If context save is required, do that and execute wfi */
-	bne	save_context_wfi
 	/* Data memory barrier and Data sync barrier */
 	mov	r1, #0
 	mcr	p15, 0, r1, c7, c10, 4
@@ -225,8 +215,182 @@ loop:
 	nop
 	nop
 	bl wait_sdrc_ok
+	ldmfd	sp!, {r0-r12, pc}		@ restore regs and return
+
+/*
+ * Forces OMAP into idle state
+ *
+ * omap34xx_cpu_suspend() - This bit of code just executes the WFI
+ * for normal idles and saves the context before WFI on off modes.
+ *
+ */
+
+ENTRY(omap34xx_cpu_suspend)
+	stmfd	sp!, {r0-r12, lr}		@ save registers on stack
+loop:
+	/*b	loop*/	@Enable to debug by stepping through code
+	/* r0 contains restore pointer in sdram */
+	/* r1 contains information about saving context */
+
+	cmp	r1, #0x0
+	/* If context save is required, do that and execute wfi */
+	bne	save_context_wfi
+	bl omap34xx_do_wfi
 
 	ldmfd	sp!, {r0-r12, pc}		@ restore regs and return
+
+save_context_wfi:
+	/*b	save_context_wfi*/	@ enable to debug save code
+	mov	r8, r0 /* Store SDRAM address in r8 */
+	mrc	p15, 0, r5, c1, c0, 1	@ Read Auxiliary Control Register
+	mov	r4, #0x1		@ Number of parameters for restore call
+	stmia	r8!, {r4-r5}		@ Push parameters for restore call
+	mrc	p15, 1, r5, c9, c0, 2	@ Read L2 AUX ctrl register
+	stmia	r8!, {r4-r5}		@ Push parameters for restore call
+        /* Check what that target sleep state is:stored in r1*/
+        /* 1 - Only L1 and logic lost */
+        /* 2 - Only L2 lost */
+        /* 3 - Both L1 and L2 lost */
+	cmp	r1, #0x2 /* Only L2 lost */
+	beq	clean_l2
+	cmp	r1, #0x1 /* L2 retained */
+	/* r9 stores whether to clean L2 or not*/
+	moveq	r9, #0x0 /* Dont Clean L2 */
+	movne	r9, #0x1 /* Clean L2 */
+l1_logic_lost:
+	/* Store sp and spsr to SDRAM */
+	mov	r4, sp
+	mrs	r5, spsr
+	mov	r6, lr
+	stmia	r8!, {r4-r6}
+	/* Save all ARM registers */
+	/* Coprocessor access control register */
+	mrc	p15, 0, r6, c1, c0, 2
+	stmia	r8!, {r6}
+	/* TTBR0, TTBR1 and Translation table base control */
+	mrc	p15, 0, r4, c2, c0, 0
+	mrc	p15, 0, r5, c2, c0, 1
+	mrc	p15, 0, r6, c2, c0, 2
+	stmia	r8!, {r4-r6}
+	/* Domain access control register, data fault status register,
+	and instruction fault status register */
+	mrc	p15, 0, r4, c3, c0, 0
+	mrc	p15, 0, r5, c5, c0, 0
+	mrc	p15, 0, r6, c5, c0, 1
+	stmia	r8!, {r4-r6}
+	/* Data aux fault status register, instruction aux fault status,
+	datat fault address register and instruction fault address register*/
+	mrc	p15, 0, r4, c5, c1, 0
+	mrc	p15, 0, r5, c5, c1, 1
+	mrc	p15, 0, r6, c6, c0, 0
+	mrc	p15, 0, r7, c6, c0, 2
+	stmia	r8!, {r4-r7}
+	/* user r/w thread and process ID, user r/o thread and process ID,
+	priv only thread and process ID, cache size selection */
+	mrc	p15, 0, r4, c13, c0, 2
+	mrc	p15, 0, r5, c13, c0, 3
+	mrc	p15, 0, r6, c13, c0, 4
+	mrc	p15, 2, r7, c0, c0, 0
+	stmia	r8!, {r4-r7}
+	/* Data TLB lockdown, instruction TLB lockdown registers */
+	mrc	p15, 0, r5, c10, c0, 0
+	mrc	p15, 0, r6, c10, c0, 1
+	stmia	r8!, {r5-r6}
+	/* Secure or non secure vector base address, FCSE PID, Context PID*/
+	mrc	p15, 0, r4, c12, c0, 0
+	mrc	p15, 0, r5, c13, c0, 0
+	mrc	p15, 0, r6, c13, c0, 1
+	stmia	r8!, {r4-r6}
+	/* Primary remap, normal remap registers */
+	mrc	p15, 0, r4, c10, c2, 0
+	mrc	p15, 0, r5, c10, c2, 1
+	stmia	r8!,{r4-r5}
+
+	/* Store current cpsr*/
+	mrs	r2, cpsr
+	stmia	r8!, {r2}
+
+	mrc	p15, 0, r4, c1, c0, 0
+	/* save control register */
+	stmia	r8!, {r4}
+clean_caches:
+	/* Clean Data or unified cache to POU*/
+	/* How to invalidate only L1 cache???? - #FIX_ME# */
+	/* mcr	p15, 0, r11, c7, c11, 1 */
+	cmp	r9, #1 /* Check whether L2 inval is required or not*/
+	bne	skip_l2_inval
+clean_l2:
+	/* read clidr */
+	mrc     p15, 1, r0, c0, c0, 1
+	/* extract loc from clidr */
+	ands    r3, r0, #0x7000000
+	/* left align loc bit field */
+	mov     r3, r3, lsr #23
+	/* if loc is 0, then no need to clean */
+	beq     finished
+	/* start clean at cache level 0 */
+	mov     r10, #0
+loop1:
+	/* work out 3x current cache level */
+	add     r2, r10, r10, lsr #1
+	/* extract cache type bits from clidr*/
+	mov     r1, r0, lsr r2
+	/* mask of the bits for current cache only */
+	and     r1, r1, #7
+	/* see what cache we have at this level */
+	cmp     r1, #2
+	/* skip if no cache, or just i-cache */
+	blt     skip
+	/* select current cache level in cssr */
+	mcr     p15, 2, r10, c0, c0, 0
+	/* isb to sych the new cssr&csidr */
+	isb
+	/* read the new csidr */
+	mrc     p15, 1, r1, c0, c0, 0
+	/* extract the length of the cache lines */
+	and     r2, r1, #7
+	/* add 4 (line length offset) */
+	add     r2, r2, #4
+	ldr     r4, assoc_mask
+	/* find maximum number on the way size */
+	ands    r4, r4, r1, lsr #3
+	/* find bit position of way size increment */
+	clz     r5, r4
+	ldr     r7, numset_mask
+	/* extract max number of the index size*/
+	ands    r7, r7, r1, lsr #13
+loop2:
+	mov     r9, r4
+	/* create working copy of max way size*/
+loop3:
+	/* factor way and cache number into r11 */
+	orr     r11, r10, r9, lsl r5
+	/* factor index number into r11 */
+	orr     r11, r11, r7, lsl r2
+	/*clean & invalidate by set/way */
+	mcr     p15, 0, r11, c7, c10, 2
+	/* decrement the way*/
+	subs    r9, r9, #1
+	bge     loop3
+	/*decrement the index */
+	subs    r7, r7, #1
+	bge     loop2
+skip:
+	add     r10, r10, #2
+	/* increment cache number */
+	cmp     r3, r10
+	bgt     loop1
+finished:
+	/*swith back to cache level 0 */
+	mov     r10, #0
+	/* select current cache level in cssr */
+	mcr     p15, 2, r10, c0, c0, 0
+	isb
+skip_l2_inval:
+	bl omap34xx_do_wfi
+	ldmfd   sp!, {r0-r12, pc}
+
+/* This is where ROM code jumps when MPU comes out of off mode */
 restore_es3:
 	/*b restore_es3*/		@ Enable to debug restore code
 	ldr	r5, pm_prepwstst_core_p
@@ -437,175 +601,8 @@ usettbr0:
 	ldr	r2, cache_pred_disable_mask
 	and	r4, r2
 	mcr	p15, 0, r4, c1, c0, 0
-
 	ldmfd	sp!, {r0-r12, pc}		@ restore regs and return
-save_context_wfi:
-	/*b	save_context_wfi*/	@ enable to debug save code
-	mov	r8, r0 /* Store SDRAM address in r8 */
-	mrc	p15, 0, r5, c1, c0, 1	@ Read Auxiliary Control Register
-	mov	r4, #0x1		@ Number of parameters for restore call
-	stmia	r8!, {r4-r5}		@ Push parameters for restore call
-	mrc	p15, 1, r5, c9, c0, 2	@ Read L2 AUX ctrl register
-	stmia	r8!, {r4-r5}		@ Push parameters for restore call
-        /* Check what that target sleep state is:stored in r1*/
-        /* 1 - Only L1 and logic lost */
-        /* 2 - Only L2 lost */
-        /* 3 - Both L1 and L2 lost */
-	cmp	r1, #0x2 /* Only L2 lost */
-	beq	clean_l2
-	cmp	r1, #0x1 /* L2 retained */
-	/* r9 stores whether to clean L2 or not*/
-	moveq	r9, #0x0 /* Dont Clean L2 */
-	movne	r9, #0x1 /* Clean L2 */
-l1_logic_lost:
-	/* Store sp and spsr to SDRAM */
-	mov	r4, sp
-	mrs	r5, spsr
-	mov	r6, lr
-	stmia	r8!, {r4-r6}
-	/* Save all ARM registers */
-	/* Coprocessor access control register */
-	mrc	p15, 0, r6, c1, c0, 2
-	stmia	r8!, {r6}
-	/* TTBR0, TTBR1 and Translation table base control */
-	mrc	p15, 0, r4, c2, c0, 0
-	mrc	p15, 0, r5, c2, c0, 1
-	mrc	p15, 0, r6, c2, c0, 2
-	stmia	r8!, {r4-r6}
-	/* Domain access control register, data fault status register,
-	and instruction fault status register */
-	mrc	p15, 0, r4, c3, c0, 0
-	mrc	p15, 0, r5, c5, c0, 0
-	mrc	p15, 0, r6, c5, c0, 1
-	stmia	r8!, {r4-r6}
-	/* Data aux fault status register, instruction aux fault status,
-	datat fault address register and instruction fault address register*/
-	mrc	p15, 0, r4, c5, c1, 0
-	mrc	p15, 0, r5, c5, c1, 1
-	mrc	p15, 0, r6, c6, c0, 0
-	mrc	p15, 0, r7, c6, c0, 2
-	stmia	r8!, {r4-r7}
-	/* user r/w thread and process ID, user r/o thread and process ID,
-	priv only thread and process ID, cache size selection */
-	mrc	p15, 0, r4, c13, c0, 2
-	mrc	p15, 0, r5, c13, c0, 3
-	mrc	p15, 0, r6, c13, c0, 4
-	mrc	p15, 2, r7, c0, c0, 0
-	stmia	r8!, {r4-r7}
-	/* Data TLB lockdown, instruction TLB lockdown registers */
-	mrc	p15, 0, r5, c10, c0, 0
-	mrc	p15, 0, r6, c10, c0, 1
-	stmia	r8!, {r5-r6}
-	/* Secure or non secure vector base address, FCSE PID, Context PID*/
-	mrc	p15, 0, r4, c12, c0, 0
-	mrc	p15, 0, r5, c13, c0, 0
-	mrc	p15, 0, r6, c13, c0, 1
-	stmia	r8!, {r4-r6}
-	/* Primary remap, normal remap registers */
-	mrc	p15, 0, r4, c10, c2, 0
-	mrc	p15, 0, r5, c10, c2, 1
-	stmia	r8!,{r4-r5}
-
-	/* Store current cpsr*/
-	mrs	r2, cpsr
-	stmia	r8!, {r2}
 
-	mrc	p15, 0, r4, c1, c0, 0
-	/* save control register */
-	stmia	r8!, {r4}
-clean_caches:
-	/* Clean Data or unified cache to POU*/
-	/* How to invalidate only L1 cache???? - #FIX_ME# */
-	/* mcr	p15, 0, r11, c7, c11, 1 */
-	cmp	r9, #1 /* Check whether L2 inval is required or not*/
-	bne	skip_l2_inval
-clean_l2:
-	/* read clidr */
-	mrc     p15, 1, r0, c0, c0, 1
-	/* extract loc from clidr */
-	ands    r3, r0, #0x7000000
-	/* left align loc bit field */
-	mov     r3, r3, lsr #23
-	/* if loc is 0, then no need to clean */
-	beq     finished
-	/* start clean at cache level 0 */
-	mov     r10, #0
-loop1:
-	/* work out 3x current cache level */
-	add     r2, r10, r10, lsr #1
-	/* extract cache type bits from clidr*/
-	mov     r1, r0, lsr r2
-	/* mask of the bits for current cache only */
-	and     r1, r1, #7
-	/* see what cache we have at this level */
-	cmp     r1, #2
-	/* skip if no cache, or just i-cache */
-	blt     skip
-	/* select current cache level in cssr */
-	mcr     p15, 2, r10, c0, c0, 0
-	/* isb to sych the new cssr&csidr */
-	isb
-	/* read the new csidr */
-	mrc     p15, 1, r1, c0, c0, 0
-	/* extract the length of the cache lines */
-	and     r2, r1, #7
-	/* add 4 (line length offset) */
-	add     r2, r2, #4
-	ldr     r4, assoc_mask
-	/* find maximum number on the way size */
-	ands    r4, r4, r1, lsr #3
-	/* find bit position of way size increment */
-	clz     r5, r4
-	ldr     r7, numset_mask
-	/* extract max number of the index size*/
-	ands    r7, r7, r1, lsr #13
-loop2:
-	mov     r9, r4
-	/* create working copy of max way size*/
-loop3:
-	/* factor way and cache number into r11 */
-	orr     r11, r10, r9, lsl r5
-	/* factor index number into r11 */
-	orr     r11, r11, r7, lsl r2
-	/*clean & invalidate by set/way */
-	mcr     p15, 0, r11, c7, c10, 2
-	/* decrement the way*/
-	subs    r9, r9, #1
-	bge     loop3
-	/*decrement the index */
-	subs    r7, r7, #1
-	bge     loop2
-skip:
-	add     r10, r10, #2
-	/* increment cache number */
-	cmp     r3, r10
-	bgt     loop1
-finished:
-	/*swith back to cache level 0 */
-	mov     r10, #0
-	/* select current cache level in cssr */
-	mcr     p15, 2, r10, c0, c0, 0
-	isb
-skip_l2_inval:
-	/* Data memory barrier and Data sync barrier */
-	mov     r1, #0
-	mcr     p15, 0, r1, c7, c10, 4
-	mcr     p15, 0, r1, c7, c10, 5
-
-	wfi                             @ wait for interrupt
-	nop
-	nop
-	nop
-	nop
-	nop
-	nop
-	nop
-	nop
-	nop
-	nop
-	bl wait_sdrc_ok
-	/* restore regs and return */
-	ldmfd   sp!, {r0-r12, pc}
 
 /* Make sure SDRC accesses are ok */
 wait_sdrc_ok:
@@ -669,4 +666,4 @@ cache_pred_disable_mask:
 control_stat:
 	.word	CONTROL_STAT
 ENTRY(omap34xx_cpu_suspend_sz)
-	.word	. - omap34xx_cpu_suspend
+	.word   . - omap34xx_cpu_suspend
diff --git a/arch/arm/plat-omap/include/plat/control.h b/arch/arm/plat-omap/include/plat/control.h
index 46e166d..306259e 100644
--- a/arch/arm/plat-omap/include/plat/control.h
+++ b/arch/arm/plat-omap/include/plat/control.h
@@ -314,6 +314,8 @@
 #define OMAP343X_SCRATCHPAD_ROM		(OMAP343X_CTRL_BASE + 0x860)
 #define OMAP343X_SCRATCHPAD		(OMAP343X_CTRL_BASE + 0x910)
 #define OMAP343X_SCRATCHPAD_ROM_OFFSET	0x19C
+#define OMAP343X_SCRATCHPAD_REGADDR(reg)	\
+			OMAP2_L4_IO_ADDRESS(OMAP343X_SCRATCHPAD + reg)
 
 /* AM35XX_CONTROL_IPSS_CLK_CTRL bits */
 #define AM35XX_USBOTG_VBUSP_CLK_SHIFT   0
-- 
1.7.0.4

Re: [PATCH 2/2] OMAP3 PM: sleep code clean up

[Kevin Hilman]

Vishwanath BS <vishwanath.bs-l0cyMroinI0@public.gmane.org> writes:

> This patch has done some clean up of omap3 sleep code.
> Basically all possible hardcodings are removed and code is Reorganized
> into more logical buckets for better readability and instrumentation.
>
> Tested on ZOOM3.

Again, please describe more about how it was tested.  idle?  suspend?
retention? off?

Also please fix long-line checkpatch warnings.

While breaking this up in to subroutines, why not just call them all
from the C-code instead of assembly?

But this also makes me wonder, if we're going to clean this up, the bulk
of it could be re-written in C, with some inline asm helpers as needed.

Kevin

> Signed-off-by: Vishwanath BS <vishwanath.bs-l0cyMroinI0@public.gmane.org>
> Cc: Kevin Hillman <khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
> Cc: <Linaro> linaro-dev-cunTk1MwBs8s++Sfvej+rw@public.gmane.org
> ---
>  arch/arm/mach-omap2/sleep34xx.S           |  377 ++++++++++++++---------------
>  arch/arm/plat-omap/include/plat/control.h |    2 +
>  2 files changed, 189 insertions(+), 190 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index ba53191..734f82a
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -33,17 +33,20 @@
>  #include "prm.h"
>  #include "sdrc.h"
>  
> -#define SDRC_SCRATCHPAD_SEM_V	0xfa00291c
> +#define SDRC_SCRATCHPAD_SEM_OFFS	0xc
> +#define SDRC_SCRATCHPAD_SEM_V	OMAP343X_SCRATCHPAD_REGADDR \
> +							(SDRC_SCRATCHPAD_SEM_OFFS)
>  
>  #define PM_PREPWSTST_CORE_V	OMAP34XX_PRM_REGADDR(CORE_MOD, \
> -				OMAP3430_PM_PREPWSTST)
> -#define PM_PREPWSTST_CORE_P	0x48306AE8
> +							OMAP3430_PM_PREPWSTST)
> +#define PM_PREPWSTST_CORE_P	OMAP3430_PRM_BASE + CORE_MOD + \
> +								OMAP3430_PM_PREPWSTST
>  #define PM_PREPWSTST_MPU_V	OMAP34XX_PRM_REGADDR(MPU_MOD, \
>  				OMAP3430_PM_PREPWSTST)
>  #define PM_PWSTCTRL_MPU_P	OMAP3430_PRM_BASE + MPU_MOD + OMAP2_PM_PWSTCTRL
>  #define CM_IDLEST1_CORE_V	OMAP34XX_CM_REGADDR(CORE_MOD, CM_IDLEST1)
> -#define SRAM_BASE_P		0x40200000
> -#define CONTROL_STAT		0x480022F0
> +#define SRAM_BASE_P			0x40200000
> +#define CONTROL_STAT		OMAP343X_CTRL_BASE + OMAP343X_CONTROL_STATUS
>  #define SCRATCHPAD_MEM_OFFS	0x310 /* Move this as correct place is
>  				       * available */
>  #define SCRATCHPAD_BASE_P	(OMAP343X_CTRL_BASE + OMAP343X_CONTROL_MEM_WKUP\
> @@ -184,29 +187,16 @@ api_params:
>  ENTRY(save_secure_ram_context_sz)
>  	.word	. - save_secure_ram_context
>  
> -/*
> - * Forces OMAP into idle state
> - *
> - * omap34xx_suspend() - This bit of code just executes the WFI
> - * for normal idles.
> - *
> - * Note: This code get's copied to internal SRAM at boot. When the OMAP
> - *	 wakes up it continues execution at the point it went to sleep.
> - */
> -ENTRY(omap34xx_cpu_suspend)
> +/* Function to execute WFI. When the MPU wakes up from retention
> + * or inactive mode, it continues execution just after wfi */

fix multi-line comment style

> +ENTRY(omap34xx_do_wfi)
>  	stmfd	sp!, {r0-r12, lr}		@ save registers on stack
> -loop:
> -	/*b	loop*/	@Enable to debug by stepping through code
> -	/* r0 contains restore pointer in sdram */
> -	/* r1 contains information about saving context */
> +
>  	ldr     r4, sdrc_power          @ read the SDRC_POWER register
>  	ldr     r5, [r4]                @ read the contents of SDRC_POWER
>  	orr     r5, r5, #0x40           @ enable self refresh on idle req
>  	str     r5, [r4]                @ write back to SDRC_POWER register
>  
> -	cmp	r1, #0x0
> -	/* If context save is required, do that and execute wfi */
> -	bne	save_context_wfi
>  	/* Data memory barrier and Data sync barrier */
>  	mov	r1, #0
>  	mcr	p15, 0, r1, c7, c10, 4
> @@ -225,8 +215,182 @@ loop:
>  	nop
>  	nop
>  	bl wait_sdrc_ok
> +	ldmfd	sp!, {r0-r12, pc}		@ restore regs and return
> +
> +/*
> + * Forces OMAP into idle state
> + *
> + * omap34xx_cpu_suspend() - This bit of code just executes the WFI
> + * for normal idles and saves the context before WFI on off modes.
> + *
> + */
> +
> +ENTRY(omap34xx_cpu_suspend)
> +	stmfd	sp!, {r0-r12, lr}		@ save registers on stack
> +loop:
> +	/*b	loop*/	@Enable to debug by stepping through code
> +	/* r0 contains restore pointer in sdram */
> +	/* r1 contains information about saving context */
> +
> +	cmp	r1, #0x0
> +	/* If context save is required, do that and execute wfi */
> +	bne	save_context_wfi
> +	bl omap34xx_do_wfi
>  
>  	ldmfd	sp!, {r0-r12, pc}		@ restore regs and return
> +
> +save_context_wfi:
> +	/*b	save_context_wfi*/	@ enable to debug save code
> +	mov	r8, r0 /* Store SDRAM address in r8 */
> +	mrc	p15, 0, r5, c1, c0, 1	@ Read Auxiliary Control Register
> +	mov	r4, #0x1		@ Number of parameters for restore call
> +	stmia	r8!, {r4-r5}		@ Push parameters for restore call
> +	mrc	p15, 1, r5, c9, c0, 2	@ Read L2 AUX ctrl register
> +	stmia	r8!, {r4-r5}		@ Push parameters for restore call
> +        /* Check what that target sleep state is:stored in r1*/
> +        /* 1 - Only L1 and logic lost */
> +        /* 2 - Only L2 lost */
> +        /* 3 - Both L1 and L2 lost */
> +	cmp	r1, #0x2 /* Only L2 lost */
> +	beq	clean_l2
> +	cmp	r1, #0x1 /* L2 retained */
> +	/* r9 stores whether to clean L2 or not*/
> +	moveq	r9, #0x0 /* Dont Clean L2 */
> +	movne	r9, #0x1 /* Clean L2 */
> +l1_logic_lost:
> +	/* Store sp and spsr to SDRAM */
> +	mov	r4, sp
> +	mrs	r5, spsr
> +	mov	r6, lr
> +	stmia	r8!, {r4-r6}
> +	/* Save all ARM registers */
> +	/* Coprocessor access control register */
> +	mrc	p15, 0, r6, c1, c0, 2
> +	stmia	r8!, {r6}
> +	/* TTBR0, TTBR1 and Translation table base control */
> +	mrc	p15, 0, r4, c2, c0, 0
> +	mrc	p15, 0, r5, c2, c0, 1
> +	mrc	p15, 0, r6, c2, c0, 2
> +	stmia	r8!, {r4-r6}
> +	/* Domain access control register, data fault status register,
> +	and instruction fault status register */
> +	mrc	p15, 0, r4, c3, c0, 0
> +	mrc	p15, 0, r5, c5, c0, 0
> +	mrc	p15, 0, r6, c5, c0, 1
> +	stmia	r8!, {r4-r6}
> +	/* Data aux fault status register, instruction aux fault status,
> +	datat fault address register and instruction fault address register*/
> +	mrc	p15, 0, r4, c5, c1, 0
> +	mrc	p15, 0, r5, c5, c1, 1
> +	mrc	p15, 0, r6, c6, c0, 0
> +	mrc	p15, 0, r7, c6, c0, 2
> +	stmia	r8!, {r4-r7}
> +	/* user r/w thread and process ID, user r/o thread and process ID,
> +	priv only thread and process ID, cache size selection */
> +	mrc	p15, 0, r4, c13, c0, 2
> +	mrc	p15, 0, r5, c13, c0, 3
> +	mrc	p15, 0, r6, c13, c0, 4
> +	mrc	p15, 2, r7, c0, c0, 0
> +	stmia	r8!, {r4-r7}
> +	/* Data TLB lockdown, instruction TLB lockdown registers */
> +	mrc	p15, 0, r5, c10, c0, 0
> +	mrc	p15, 0, r6, c10, c0, 1
> +	stmia	r8!, {r5-r6}
> +	/* Secure or non secure vector base address, FCSE PID, Context PID*/
> +	mrc	p15, 0, r4, c12, c0, 0
> +	mrc	p15, 0, r5, c13, c0, 0
> +	mrc	p15, 0, r6, c13, c0, 1
> +	stmia	r8!, {r4-r6}
> +	/* Primary remap, normal remap registers */
> +	mrc	p15, 0, r4, c10, c2, 0
> +	mrc	p15, 0, r5, c10, c2, 1
> +	stmia	r8!,{r4-r5}
> +
> +	/* Store current cpsr*/
> +	mrs	r2, cpsr
> +	stmia	r8!, {r2}
> +
> +	mrc	p15, 0, r4, c1, c0, 0
> +	/* save control register */
> +	stmia	r8!, {r4}
> +clean_caches:
> +	/* Clean Data or unified cache to POU*/
> +	/* How to invalidate only L1 cache???? - #FIX_ME# */
> +	/* mcr	p15, 0, r11, c7, c11, 1 */
> +	cmp	r9, #1 /* Check whether L2 inval is required or not*/
> +	bne	skip_l2_inval
> +clean_l2:
> +	/* read clidr */
> +	mrc     p15, 1, r0, c0, c0, 1
> +	/* extract loc from clidr */
> +	ands    r3, r0, #0x7000000
> +	/* left align loc bit field */
> +	mov     r3, r3, lsr #23
> +	/* if loc is 0, then no need to clean */
> +	beq     finished
> +	/* start clean at cache level 0 */
> +	mov     r10, #0
> +loop1:
> +	/* work out 3x current cache level */
> +	add     r2, r10, r10, lsr #1
> +	/* extract cache type bits from clidr*/
> +	mov     r1, r0, lsr r2
> +	/* mask of the bits for current cache only */
> +	and     r1, r1, #7
> +	/* see what cache we have at this level */
> +	cmp     r1, #2
> +	/* skip if no cache, or just i-cache */
> +	blt     skip
> +	/* select current cache level in cssr */
> +	mcr     p15, 2, r10, c0, c0, 0
> +	/* isb to sych the new cssr&csidr */
> +	isb
> +	/* read the new csidr */
> +	mrc     p15, 1, r1, c0, c0, 0
> +	/* extract the length of the cache lines */
> +	and     r2, r1, #7
> +	/* add 4 (line length offset) */
> +	add     r2, r2, #4
> +	ldr     r4, assoc_mask
> +	/* find maximum number on the way size */
> +	ands    r4, r4, r1, lsr #3
> +	/* find bit position of way size increment */
> +	clz     r5, r4
> +	ldr     r7, numset_mask
> +	/* extract max number of the index size*/
> +	ands    r7, r7, r1, lsr #13
> +loop2:
> +	mov     r9, r4
> +	/* create working copy of max way size*/
> +loop3:
> +	/* factor way and cache number into r11 */
> +	orr     r11, r10, r9, lsl r5
> +	/* factor index number into r11 */
> +	orr     r11, r11, r7, lsl r2
> +	/*clean & invalidate by set/way */
> +	mcr     p15, 0, r11, c7, c10, 2
> +	/* decrement the way*/
> +	subs    r9, r9, #1
> +	bge     loop3
> +	/*decrement the index */
> +	subs    r7, r7, #1
> +	bge     loop2
> +skip:
> +	add     r10, r10, #2
> +	/* increment cache number */
> +	cmp     r3, r10
> +	bgt     loop1
> +finished:
> +	/*swith back to cache level 0 */
> +	mov     r10, #0
> +	/* select current cache level in cssr */
> +	mcr     p15, 2, r10, c0, c0, 0
> +	isb
> +skip_l2_inval:
> +	bl omap34xx_do_wfi
> +	ldmfd   sp!, {r0-r12, pc}
> +
> +/* This is where ROM code jumps when MPU comes out of off mode */
>  restore_es3:
>  	/*b restore_es3*/		@ Enable to debug restore code
>  	ldr	r5, pm_prepwstst_core_p
> @@ -437,175 +601,8 @@ usettbr0:
>  	ldr	r2, cache_pred_disable_mask
>  	and	r4, r2
>  	mcr	p15, 0, r4, c1, c0, 0
> -
>  	ldmfd	sp!, {r0-r12, pc}		@ restore regs and return
> -save_context_wfi:
> -	/*b	save_context_wfi*/	@ enable to debug save code
> -	mov	r8, r0 /* Store SDRAM address in r8 */
> -	mrc	p15, 0, r5, c1, c0, 1	@ Read Auxiliary Control Register
> -	mov	r4, #0x1		@ Number of parameters for restore call
> -	stmia	r8!, {r4-r5}		@ Push parameters for restore call
> -	mrc	p15, 1, r5, c9, c0, 2	@ Read L2 AUX ctrl register
> -	stmia	r8!, {r4-r5}		@ Push parameters for restore call
> -        /* Check what that target sleep state is:stored in r1*/
> -        /* 1 - Only L1 and logic lost */
> -        /* 2 - Only L2 lost */
> -        /* 3 - Both L1 and L2 lost */
> -	cmp	r1, #0x2 /* Only L2 lost */
> -	beq	clean_l2
> -	cmp	r1, #0x1 /* L2 retained */
> -	/* r9 stores whether to clean L2 or not*/
> -	moveq	r9, #0x0 /* Dont Clean L2 */
> -	movne	r9, #0x1 /* Clean L2 */
> -l1_logic_lost:
> -	/* Store sp and spsr to SDRAM */
> -	mov	r4, sp
> -	mrs	r5, spsr
> -	mov	r6, lr
> -	stmia	r8!, {r4-r6}
> -	/* Save all ARM registers */
> -	/* Coprocessor access control register */
> -	mrc	p15, 0, r6, c1, c0, 2
> -	stmia	r8!, {r6}
> -	/* TTBR0, TTBR1 and Translation table base control */
> -	mrc	p15, 0, r4, c2, c0, 0
> -	mrc	p15, 0, r5, c2, c0, 1
> -	mrc	p15, 0, r6, c2, c0, 2
> -	stmia	r8!, {r4-r6}
> -	/* Domain access control register, data fault status register,
> -	and instruction fault status register */
> -	mrc	p15, 0, r4, c3, c0, 0
> -	mrc	p15, 0, r5, c5, c0, 0
> -	mrc	p15, 0, r6, c5, c0, 1
> -	stmia	r8!, {r4-r6}
> -	/* Data aux fault status register, instruction aux fault status,
> -	datat fault address register and instruction fault address register*/
> -	mrc	p15, 0, r4, c5, c1, 0
> -	mrc	p15, 0, r5, c5, c1, 1
> -	mrc	p15, 0, r6, c6, c0, 0
> -	mrc	p15, 0, r7, c6, c0, 2
> -	stmia	r8!, {r4-r7}
> -	/* user r/w thread and process ID, user r/o thread and process ID,
> -	priv only thread and process ID, cache size selection */
> -	mrc	p15, 0, r4, c13, c0, 2
> -	mrc	p15, 0, r5, c13, c0, 3
> -	mrc	p15, 0, r6, c13, c0, 4
> -	mrc	p15, 2, r7, c0, c0, 0
> -	stmia	r8!, {r4-r7}
> -	/* Data TLB lockdown, instruction TLB lockdown registers */
> -	mrc	p15, 0, r5, c10, c0, 0
> -	mrc	p15, 0, r6, c10, c0, 1
> -	stmia	r8!, {r5-r6}
> -	/* Secure or non secure vector base address, FCSE PID, Context PID*/
> -	mrc	p15, 0, r4, c12, c0, 0
> -	mrc	p15, 0, r5, c13, c0, 0
> -	mrc	p15, 0, r6, c13, c0, 1
> -	stmia	r8!, {r4-r6}
> -	/* Primary remap, normal remap registers */
> -	mrc	p15, 0, r4, c10, c2, 0
> -	mrc	p15, 0, r5, c10, c2, 1
> -	stmia	r8!,{r4-r5}
> -
> -	/* Store current cpsr*/
> -	mrs	r2, cpsr
> -	stmia	r8!, {r2}
>  
> -	mrc	p15, 0, r4, c1, c0, 0
> -	/* save control register */
> -	stmia	r8!, {r4}
> -clean_caches:
> -	/* Clean Data or unified cache to POU*/
> -	/* How to invalidate only L1 cache???? - #FIX_ME# */
> -	/* mcr	p15, 0, r11, c7, c11, 1 */
> -	cmp	r9, #1 /* Check whether L2 inval is required or not*/
> -	bne	skip_l2_inval
> -clean_l2:
> -	/* read clidr */
> -	mrc     p15, 1, r0, c0, c0, 1
> -	/* extract loc from clidr */
> -	ands    r3, r0, #0x7000000
> -	/* left align loc bit field */
> -	mov     r3, r3, lsr #23
> -	/* if loc is 0, then no need to clean */
> -	beq     finished
> -	/* start clean at cache level 0 */
> -	mov     r10, #0
> -loop1:
> -	/* work out 3x current cache level */
> -	add     r2, r10, r10, lsr #1
> -	/* extract cache type bits from clidr*/
> -	mov     r1, r0, lsr r2
> -	/* mask of the bits for current cache only */
> -	and     r1, r1, #7
> -	/* see what cache we have at this level */
> -	cmp     r1, #2
> -	/* skip if no cache, or just i-cache */
> -	blt     skip
> -	/* select current cache level in cssr */
> -	mcr     p15, 2, r10, c0, c0, 0
> -	/* isb to sych the new cssr&csidr */
> -	isb
> -	/* read the new csidr */
> -	mrc     p15, 1, r1, c0, c0, 0
> -	/* extract the length of the cache lines */
> -	and     r2, r1, #7
> -	/* add 4 (line length offset) */
> -	add     r2, r2, #4
> -	ldr     r4, assoc_mask
> -	/* find maximum number on the way size */
> -	ands    r4, r4, r1, lsr #3
> -	/* find bit position of way size increment */
> -	clz     r5, r4
> -	ldr     r7, numset_mask
> -	/* extract max number of the index size*/
> -	ands    r7, r7, r1, lsr #13
> -loop2:
> -	mov     r9, r4
> -	/* create working copy of max way size*/
> -loop3:
> -	/* factor way and cache number into r11 */
> -	orr     r11, r10, r9, lsl r5
> -	/* factor index number into r11 */
> -	orr     r11, r11, r7, lsl r2
> -	/*clean & invalidate by set/way */
> -	mcr     p15, 0, r11, c7, c10, 2
> -	/* decrement the way*/
> -	subs    r9, r9, #1
> -	bge     loop3
> -	/*decrement the index */
> -	subs    r7, r7, #1
> -	bge     loop2
> -skip:
> -	add     r10, r10, #2
> -	/* increment cache number */
> -	cmp     r3, r10
> -	bgt     loop1
> -finished:
> -	/*swith back to cache level 0 */
> -	mov     r10, #0
> -	/* select current cache level in cssr */
> -	mcr     p15, 2, r10, c0, c0, 0
> -	isb
> -skip_l2_inval:
> -	/* Data memory barrier and Data sync barrier */
> -	mov     r1, #0
> -	mcr     p15, 0, r1, c7, c10, 4
> -	mcr     p15, 0, r1, c7, c10, 5
> -
> -	wfi                             @ wait for interrupt
> -	nop
> -	nop
> -	nop
> -	nop
> -	nop
> -	nop
> -	nop
> -	nop
> -	nop
> -	nop
> -	bl wait_sdrc_ok
> -	/* restore regs and return */
> -	ldmfd   sp!, {r0-r12, pc}
>  
>  /* Make sure SDRC accesses are ok */
>  wait_sdrc_ok:
> @@ -669,4 +666,4 @@ cache_pred_disable_mask:
>  control_stat:
>  	.word	CONTROL_STAT
>  ENTRY(omap34xx_cpu_suspend_sz)
> -	.word	. - omap34xx_cpu_suspend
> +	.word   . - omap34xx_cpu_suspend
> diff --git a/arch/arm/plat-omap/include/plat/control.h b/arch/arm/plat-omap/include/plat/control.h
> index 46e166d..306259e 100644
> --- a/arch/arm/plat-omap/include/plat/control.h
> +++ b/arch/arm/plat-omap/include/plat/control.h
> @@ -314,6 +314,8 @@
>  #define OMAP343X_SCRATCHPAD_ROM		(OMAP343X_CTRL_BASE + 0x860)
>  #define OMAP343X_SCRATCHPAD		(OMAP343X_CTRL_BASE + 0x910)
>  #define OMAP343X_SCRATCHPAD_ROM_OFFSET	0x19C
> +#define OMAP343X_SCRATCHPAD_REGADDR(reg)	\
> +			OMAP2_L4_IO_ADDRESS(OMAP343X_SCRATCHPAD + reg)
>  
>  /* AM35XX_CONTROL_IPSS_CLK_CTRL bits */
>  #define AM35XX_USBOTG_VBUSP_CLK_SHIFT   0

RE: [PATCH 2/2] OMAP3 PM: sleep code clean up

[Sripathy, Vishwanath]

Kevin,

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Saturday, September 25, 2010 1:35 AM
> To: Sripathy, Vishwanath
> Cc: linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
> Subject: Re: [PATCH 2/2] OMAP3 PM: sleep code clean up
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > This patch has done some clean up of omap3 sleep code.
> > Basically all possible hardcodings are removed and code is Reorganized
> > into more logical buckets for better readability and instrumentation.
> >
> > Tested on ZOOM3.
>
> Again, please describe more about how it was tested.  idle?  suspend?
> retention? off?
This has been tested for both RET and OFF mode in Idle and suspend path. Will update the change log for the same.
>
> Also please fix long-line checkpatch warnings.
OK.
>
> While breaking this up in to subroutines, why not just call them all
> from the C-code instead of assembly?
I thought about that. But unfortunately, while saving CPU registers (in save_context_wfi), LR also gets saved. So if I call wfi routine (omap34xx_do_wfi ) from C code itself, then LR would have pointed to omap34xx_do_wfi while saving
the registers which is not correct.
>
> But this also makes me wonder, if we're going to clean this up, the bulk
> of it could be re-written in C, with some inline asm helpers as needed.
Probably yes. But as this code is specific to OMAP3, do you think it's worth spending time on rewriting the entire code in C? It might be a significant effort and it will not be reusable for other ARM SOCs.

Vishwa
>
> Kevin
>
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > Cc: Kevin Hillman <khilman@deeprootsystems.com>
> > Cc: <Linaro> linaro-dev@lists.linaro.org
> > ---
> >  arch/arm/mach-omap2/sleep34xx.S           |  377 ++++++++++++++-----------
> ----
> >  arch/arm/plat-omap/include/plat/control.h |    2 +
> >  2 files changed, 189 insertions(+), 190 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-
> omap2/sleep34xx.S
> > index ba53191..734f82a
> > --- a/arch/arm/mach-omap2/sleep34xx.S
> > +++ b/arch/arm/mach-omap2/sleep34xx.S
> > @@ -33,17 +33,20 @@
> >  #include "prm.h"
> >  #include "sdrc.h"
> >
> > -#define SDRC_SCRATCHPAD_SEM_V      0xfa00291c
> > +#define SDRC_SCRATCHPAD_SEM_OFFS   0xc
> > +#define SDRC_SCRATCHPAD_SEM_V      OMAP343X_SCRATCHPAD_REGADDR \
> > +                                                   (SDRC_SCRATCHPAD_SEM_OFFS)
> >
> >  #define PM_PREPWSTST_CORE_V        OMAP34XX_PRM_REGADDR(CORE_MOD, \
> > -                           OMAP3430_PM_PREPWSTST)
> > -#define PM_PREPWSTST_CORE_P        0x48306AE8
> > +                                                   OMAP3430_PM_PREPWSTST)
> > +#define PM_PREPWSTST_CORE_P        OMAP3430_PRM_BASE + CORE_MOD + \
> > +                                                           OMAP3430_PM_PREPWSTST
> >  #define PM_PREPWSTST_MPU_V OMAP34XX_PRM_REGADDR(MPU_MOD, \
> >                             OMAP3430_PM_PREPWSTST)
> >  #define PM_PWSTCTRL_MPU_P  OMAP3430_PRM_BASE + MPU_MOD +
> OMAP2_PM_PWSTCTRL
> >  #define CM_IDLEST1_CORE_V  OMAP34XX_CM_REGADDR(CORE_MOD,
> CM_IDLEST1)
> > -#define SRAM_BASE_P                0x40200000
> > -#define CONTROL_STAT               0x480022F0
> > +#define SRAM_BASE_P                        0x40200000
> > +#define CONTROL_STAT               OMAP343X_CTRL_BASE +
> OMAP343X_CONTROL_STATUS
> >  #define SCRATCHPAD_MEM_OFFS        0x310 /* Move this as correct place is
> >                                    * available */
> >  #define SCRATCHPAD_BASE_P  (OMAP343X_CTRL_BASE +
> OMAP343X_CONTROL_MEM_WKUP\
> > @@ -184,29 +187,16 @@ api_params:
> >  ENTRY(save_secure_ram_context_sz)
> >     .word   . - save_secure_ram_context
> >
> > -/*
> > - * Forces OMAP into idle state
> > - *
> > - * omap34xx_suspend() - This bit of code just executes the WFI
> > - * for normal idles.
> > - *
> > - * Note: This code get's copied to internal SRAM at boot. When the OMAP
> > - *  wakes up it continues execution at the point it went to sleep.
> > - */
> > -ENTRY(omap34xx_cpu_suspend)
> > +/* Function to execute WFI. When the MPU wakes up from retention
> > + * or inactive mode, it continues execution just after wfi */
>
> fix multi-line comment style
>
> > +ENTRY(omap34xx_do_wfi)
> >     stmfd   sp!, {r0-r12, lr}               @ save registers on stack
> > -loop:
> > -   /*b     loop*/  @Enable to debug by stepping through code
> > -   /* r0 contains restore pointer in sdram */
> > -   /* r1 contains information about saving context */
> > +
> >     ldr     r4, sdrc_power          @ read the SDRC_POWER register
> >     ldr     r5, [r4]                @ read the contents of SDRC_POWER
> >     orr     r5, r5, #0x40           @ enable self refresh on idle req
> >     str     r5, [r4]                @ write back to SDRC_POWER register
> >
> > -   cmp     r1, #0x0
> > -   /* If context save is required, do that and execute wfi */
> > -   bne     save_context_wfi
> >     /* Data memory barrier and Data sync barrier */
> >     mov     r1, #0
> >     mcr     p15, 0, r1, c7, c10, 4
> > @@ -225,8 +215,182 @@ loop:
> >     nop
> >     nop
> >     bl wait_sdrc_ok
> > +   ldmfd   sp!, {r0-r12, pc}               @ restore regs and return
> > +
> > +/*
> > + * Forces OMAP into idle state
> > + *
> > + * omap34xx_cpu_suspend() - This bit of code just executes the WFI
> > + * for normal idles and saves the context before WFI on off modes.
> > + *
> > + */
> > +
> > +ENTRY(omap34xx_cpu_suspend)
> > +   stmfd   sp!, {r0-r12, lr}               @ save registers on stack
> > +loop:
> > +   /*b     loop*/  @Enable to debug by stepping through code
> > +   /* r0 contains restore pointer in sdram */
> > +   /* r1 contains information about saving context */
> > +
> > +   cmp     r1, #0x0
> > +   /* If context save is required, do that and execute wfi */
> > +   bne     save_context_wfi
> > +   bl omap34xx_do_wfi
> >
> >     ldmfd   sp!, {r0-r12, pc}               @ restore regs and return
> > +
> > +save_context_wfi:
> > +   /*b     save_context_wfi*/      @ enable to debug save code
> > +   mov     r8, r0 /* Store SDRAM address in r8 */
> > +   mrc     p15, 0, r5, c1, c0, 1   @ Read Auxiliary Control Register
> > +   mov     r4, #0x1                @ Number of parameters for restore call
> > +   stmia   r8!, {r4-r5}            @ Push parameters for restore call
> > +   mrc     p15, 1, r5, c9, c0, 2   @ Read L2 AUX ctrl register
> > +   stmia   r8!, {r4-r5}            @ Push parameters for restore call
> > +        /* Check what that target sleep state is:stored in r1*/
> > +        /* 1 - Only L1 and logic lost */
> > +        /* 2 - Only L2 lost */
> > +        /* 3 - Both L1 and L2 lost */
> > +   cmp     r1, #0x2 /* Only L2 lost */
> > +   beq     clean_l2
> > +   cmp     r1, #0x1 /* L2 retained */
> > +   /* r9 stores whether to clean L2 or not*/
> > +   moveq   r9, #0x0 /* Dont Clean L2 */
> > +   movne   r9, #0x1 /* Clean L2 */
> > +l1_logic_lost:
> > +   /* Store sp and spsr to SDRAM */
> > +   mov     r4, sp
> > +   mrs     r5, spsr
> > +   mov     r6, lr
> > +   stmia   r8!, {r4-r6}
> > +   /* Save all ARM registers */
> > +   /* Coprocessor access control register */
> > +   mrc     p15, 0, r6, c1, c0, 2
> > +   stmia   r8!, {r6}
> > +   /* TTBR0, TTBR1 and Translation table base control */
> > +   mrc     p15, 0, r4, c2, c0, 0
> > +   mrc     p15, 0, r5, c2, c0, 1
> > +   mrc     p15, 0, r6, c2, c0, 2
> > +   stmia   r8!, {r4-r6}
> > +   /* Domain access control register, data fault status register,
> > +   and instruction fault status register */
> > +   mrc     p15, 0, r4, c3, c0, 0
> > +   mrc     p15, 0, r5, c5, c0, 0
> > +   mrc     p15, 0, r6, c5, c0, 1
> > +   stmia   r8!, {r4-r6}
> > +   /* Data aux fault status register, instruction aux fault status,
> > +   datat fault address register and instruction fault address register*/
> > +   mrc     p15, 0, r4, c5, c1, 0
> > +   mrc     p15, 0, r5, c5, c1, 1
> > +   mrc     p15, 0, r6, c6, c0, 0
> > +   mrc     p15, 0, r7, c6, c0, 2
> > +   stmia   r8!, {r4-r7}
> > +   /* user r/w thread and process ID, user r/o thread and process ID,
> > +   priv only thread and process ID, cache size selection */
> > +   mrc     p15, 0, r4, c13, c0, 2
> > +   mrc     p15, 0, r5, c13, c0, 3
> > +   mrc     p15, 0, r6, c13, c0, 4
> > +   mrc     p15, 2, r7, c0, c0, 0
> > +   stmia   r8!, {r4-r7}
> > +   /* Data TLB lockdown, instruction TLB lockdown registers */
> > +   mrc     p15, 0, r5, c10, c0, 0
> > +   mrc     p15, 0, r6, c10, c0, 1
> > +   stmia   r8!, {r5-r6}
> > +   /* Secure or non secure vector base address, FCSE PID, Context PID*/
> > +   mrc     p15, 0, r4, c12, c0, 0
> > +   mrc     p15, 0, r5, c13, c0, 0
> > +   mrc     p15, 0, r6, c13, c0, 1
> > +   stmia   r8!, {r4-r6}
> > +   /* Primary remap, normal remap registers */
> > +   mrc     p15, 0, r4, c10, c2, 0
> > +   mrc     p15, 0, r5, c10, c2, 1
> > +   stmia   r8!,{r4-r5}
> > +
> > +   /* Store current cpsr*/
> > +   mrs     r2, cpsr
> > +   stmia   r8!, {r2}
> > +
> > +   mrc     p15, 0, r4, c1, c0, 0
> > +   /* save control register */
> > +   stmia   r8!, {r4}
> > +clean_caches:
> > +   /* Clean Data or unified cache to POU*/
> > +   /* How to invalidate only L1 cache???? - #FIX_ME# */
> > +   /* mcr  p15, 0, r11, c7, c11, 1 */
> > +   cmp     r9, #1 /* Check whether L2 inval is required or not*/
> > +   bne     skip_l2_inval
> > +clean_l2:
> > +   /* read clidr */
> > +   mrc     p15, 1, r0, c0, c0, 1
> > +   /* extract loc from clidr */
> > +   ands    r3, r0, #0x7000000
> > +   /* left align loc bit field */
> > +   mov     r3, r3, lsr #23
> > +   /* if loc is 0, then no need to clean */
> > +   beq     finished
> > +   /* start clean at cache level 0 */
> > +   mov     r10, #0
> > +loop1:
> > +   /* work out 3x current cache level */
> > +   add     r2, r10, r10, lsr #1
> > +   /* extract cache type bits from clidr*/
> > +   mov     r1, r0, lsr r2
> > +   /* mask of the bits for current cache only */
> > +   and     r1, r1, #7
> > +   /* see what cache we have at this level */
> > +   cmp     r1, #2
> > +   /* skip if no cache, or just i-cache */
> > +   blt     skip
> > +   /* select current cache level in cssr */
> > +   mcr     p15, 2, r10, c0, c0, 0
> > +   /* isb to sych the new cssr&csidr */
> > +   isb
> > +   /* read the new csidr */
> > +   mrc     p15, 1, r1, c0, c0, 0
> > +   /* extract the length of the cache lines */
> > +   and     r2, r1, #7
> > +   /* add 4 (line length offset) */
> > +   add     r2, r2, #4
> > +   ldr     r4, assoc_mask
> > +   /* find maximum number on the way size */
> > +   ands    r4, r4, r1, lsr #3
> > +   /* find bit position of way size increment */
> > +   clz     r5, r4
> > +   ldr     r7, numset_mask
> > +   /* extract max number of the index size*/
> > +   ands    r7, r7, r1, lsr #13
> > +loop2:
> > +   mov     r9, r4
> > +   /* create working copy of max way size*/
> > +loop3:
> > +   /* factor way and cache number into r11 */
> > +   orr     r11, r10, r9, lsl r5
> > +   /* factor index number into r11 */
> > +   orr     r11, r11, r7, lsl r2
> > +   /*clean & invalidate by set/way */
> > +   mcr     p15, 0, r11, c7, c10, 2
> > +   /* decrement the way*/
> > +   subs    r9, r9, #1
> > +   bge     loop3
> > +   /*decrement the index */
> > +   subs    r7, r7, #1
> > +   bge     loop2
> > +skip:
> > +   add     r10, r10, #2
> > +   /* increment cache number */
> > +   cmp     r3, r10
> > +   bgt     loop1
> > +finished:
> > +   /*swith back to cache level 0 */
> > +   mov     r10, #0
> > +   /* select current cache level in cssr */
> > +   mcr     p15, 2, r10, c0, c0, 0
> > +   isb
> > +skip_l2_inval:
> > +   bl omap34xx_do_wfi
> > +   ldmfd   sp!, {r0-r12, pc}
> > +
> > +/* This is where ROM code jumps when MPU comes out of off mode */
> >  restore_es3:
> >     /*b restore_es3*/               @ Enable to debug restore code
> >     ldr     r5, pm_prepwstst_core_p
> > @@ -437,175 +601,8 @@ usettbr0:
> >     ldr     r2, cache_pred_disable_mask
> >     and     r4, r2
> >     mcr     p15, 0, r4, c1, c0, 0
> > -
> >     ldmfd   sp!, {r0-r12, pc}               @ restore regs and return
> > -save_context_wfi:
> > -   /*b     save_context_wfi*/      @ enable to debug save code
> > -   mov     r8, r0 /* Store SDRAM address in r8 */
> > -   mrc     p15, 0, r5, c1, c0, 1   @ Read Auxiliary Control Register
> > -   mov     r4, #0x1                @ Number of parameters for restore call
> > -   stmia   r8!, {r4-r5}            @ Push parameters for restore call
> > -   mrc     p15, 1, r5, c9, c0, 2   @ Read L2 AUX ctrl register
> > -   stmia   r8!, {r4-r5}            @ Push parameters for restore call
> > -        /* Check what that target sleep state is:stored in r1*/
> > -        /* 1 - Only L1 and logic lost */
> > -        /* 2 - Only L2 lost */
> > -        /* 3 - Both L1 and L2 lost */
> > -   cmp     r1, #0x2 /* Only L2 lost */
> > -   beq     clean_l2
> > -   cmp     r1, #0x1 /* L2 retained */
> > -   /* r9 stores whether to clean L2 or not*/
> > -   moveq   r9, #0x0 /* Dont Clean L2 */
> > -   movne   r9, #0x1 /* Clean L2 */
> > -l1_logic_lost:
> > -   /* Store sp and spsr to SDRAM */
> > -   mov     r4, sp
> > -   mrs     r5, spsr
> > -   mov     r6, lr
> > -   stmia   r8!, {r4-r6}
> > -   /* Save all ARM registers */
> > -   /* Coprocessor access control register */
> > -   mrc     p15, 0, r6, c1, c0, 2
> > -   stmia   r8!, {r6}
> > -   /* TTBR0, TTBR1 and Translation table base control */
> > -   mrc     p15, 0, r4, c2, c0, 0
> > -   mrc     p15, 0, r5, c2, c0, 1
> > -   mrc     p15, 0, r6, c2, c0, 2
> > -   stmia   r8!, {r4-r6}
> > -   /* Domain access control register, data fault status register,
> > -   and instruction fault status register */
> > -   mrc     p15, 0, r4, c3, c0, 0
> > -   mrc     p15, 0, r5, c5, c0, 0
> > -   mrc     p15, 0, r6, c5, c0, 1
> > -   stmia   r8!, {r4-r6}
> > -   /* Data aux fault status register, instruction aux fault status,
> > -   datat fault address register and instruction fault address register*/
> > -   mrc     p15, 0, r4, c5, c1, 0
> > -   mrc     p15, 0, r5, c5, c1, 1
> > -   mrc     p15, 0, r6, c6, c0, 0
> > -   mrc     p15, 0, r7, c6, c0, 2
> > -   stmia   r8!, {r4-r7}
> > -   /* user r/w thread and process ID, user r/o thread and process ID,
> > -   priv only thread and process ID, cache size selection */
> > -   mrc     p15, 0, r4, c13, c0, 2
> > -   mrc     p15, 0, r5, c13, c0, 3
> > -   mrc     p15, 0, r6, c13, c0, 4
> > -   mrc     p15, 2, r7, c0, c0, 0
> > -   stmia   r8!, {r4-r7}
> > -   /* Data TLB lockdown, instruction TLB lockdown registers */
> > -   mrc     p15, 0, r5, c10, c0, 0
> > -   mrc     p15, 0, r6, c10, c0, 1
> > -   stmia   r8!, {r5-r6}
> > -   /* Secure or non secure vector base address, FCSE PID, Context PID*/
> > -   mrc     p15, 0, r4, c12, c0, 0
> > -   mrc     p15, 0, r5, c13, c0, 0
> > -   mrc     p15, 0, r6, c13, c0, 1
> > -   stmia   r8!, {r4-r6}
> > -   /* Primary remap, normal remap registers */
> > -   mrc     p15, 0, r4, c10, c2, 0
> > -   mrc     p15, 0, r5, c10, c2, 1
> > -   stmia   r8!,{r4-r5}
> > -
> > -   /* Store current cpsr*/
> > -   mrs     r2, cpsr
> > -   stmia   r8!, {r2}
> >
> > -   mrc     p15, 0, r4, c1, c0, 0
> > -   /* save control register */
> > -   stmia   r8!, {r4}
> > -clean_caches:
> > -   /* Clean Data or unified cache to POU*/
> > -   /* How to invalidate only L1 cache???? - #FIX_ME# */
> > -   /* mcr  p15, 0, r11, c7, c11, 1 */
> > -   cmp     r9, #1 /* Check whether L2 inval is required or not*/
> > -   bne     skip_l2_inval
> > -clean_l2:
> > -   /* read clidr */
> > -   mrc     p15, 1, r0, c0, c0, 1
> > -   /* extract loc from clidr */
> > -   ands    r3, r0, #0x7000000
> > -   /* left align loc bit field */
> > -   mov     r3, r3, lsr #23
> > -   /* if loc is 0, then no need to clean */
> > -   beq     finished
> > -   /* start clean at cache level 0 */
> > -   mov     r10, #0
> > -loop1:
> > -   /* work out 3x current cache level */
> > -   add     r2, r10, r10, lsr #1
> > -   /* extract cache type bits from clidr*/
> > -   mov     r1, r0, lsr r2
> > -   /* mask of the bits for current cache only */
> > -   and     r1, r1, #7
> > -   /* see what cache we have at this level */
> > -   cmp     r1, #2
> > -   /* skip if no cache, or just i-cache */
> > -   blt     skip
> > -   /* select current cache level in cssr */
> > -   mcr     p15, 2, r10, c0, c0, 0
> > -   /* isb to sych the new cssr&csidr */
> > -   isb
> > -   /* read the new csidr */
> > -   mrc     p15, 1, r1, c0, c0, 0
> > -   /* extract the length of the cache lines */
> > -   and     r2, r1, #7
> > -   /* add 4 (line length offset) */
> > -   add     r2, r2, #4
> > -   ldr     r4, assoc_mask
> > -   /* find maximum number on the way size */
> > -   ands    r4, r4, r1, lsr #3
> > -   /* find bit position of way size increment */
> > -   clz     r5, r4
> > -   ldr     r7, numset_mask
> > -   /* extract max number of the index size*/
> > -   ands    r7, r7, r1, lsr #13
> > -loop2:
> > -   mov     r9, r4
> > -   /* create working copy of max way size*/
> > -loop3:
> > -   /* factor way and cache number into r11 */
> > -   orr     r11, r10, r9, lsl r5
> > -   /* factor index number into r11 */
> > -   orr     r11, r11, r7, lsl r2
> > -   /*clean & invalidate by set/way */
> > -   mcr     p15, 0, r11, c7, c10, 2
> > -   /* decrement the way*/
> > -   subs    r9, r9, #1
> > -   bge     loop3
> > -   /*decrement the index */
> > -   subs    r7, r7, #1
> > -   bge     loop2
> > -skip:
> > -   add     r10, r10, #2
> > -   /* increment cache number */
> > -   cmp     r3, r10
> > -   bgt     loop1
> > -finished:
> > -   /*swith back to cache level 0 */
> > -   mov     r10, #0
> > -   /* select current cache level in cssr */
> > -   mcr     p15, 2, r10, c0, c0, 0
> > -   isb
> > -skip_l2_inval:
> > -   /* Data memory barrier and Data sync barrier */
> > -   mov     r1, #0
> > -   mcr     p15, 0, r1, c7, c10, 4
> > -   mcr     p15, 0, r1, c7, c10, 5
> > -
> > -   wfi                             @ wait for interrupt
> > -   nop
> > -   nop
> > -   nop
> > -   nop
> > -   nop
> > -   nop
> > -   nop
> > -   nop
> > -   nop
> > -   nop
> > -   bl wait_sdrc_ok
> > -   /* restore regs and return */
> > -   ldmfd   sp!, {r0-r12, pc}
> >
> >  /* Make sure SDRC accesses are ok */
> >  wait_sdrc_ok:
> > @@ -669,4 +666,4 @@ cache_pred_disable_mask:
> >  control_stat:
> >     .word   CONTROL_STAT
> >  ENTRY(omap34xx_cpu_suspend_sz)
> > -   .word   . - omap34xx_cpu_suspend
> > +   .word   . - omap34xx_cpu_suspend
> > diff --git a/arch/arm/plat-omap/include/plat/control.h b/arch/arm/plat-
> omap/include/plat/control.h
> > index 46e166d..306259e 100644
> > --- a/arch/arm/plat-omap/include/plat/control.h
> > +++ b/arch/arm/plat-omap/include/plat/control.h
> > @@ -314,6 +314,8 @@
> >  #define OMAP343X_SCRATCHPAD_ROM            (OMAP343X_CTRL_BASE +
> 0x860)
> >  #define OMAP343X_SCRATCHPAD                (OMAP343X_CTRL_BASE + 0x910)
> >  #define OMAP343X_SCRATCHPAD_ROM_OFFSET     0x19C
> > +#define OMAP343X_SCRATCHPAD_REGADDR(reg)   \
> > +                   OMAP2_L4_IO_ADDRESS(OMAP343X_SCRATCHPAD + reg)
> >
> >  /* AM35XX_CONTROL_IPSS_CLK_CTRL bits */
> >  #define AM35XX_USBOTG_VBUSP_CLK_SHIFT   0

Re: [PATCH 2/2] OMAP3 PM: sleep code clean up

[Amit Kucheria]

On 10 Oct 04, Sripathy, Vishwanath wrote:
> Kevin,
 
[snip]

> >
> > But this also makes me wonder, if we're going to clean this up, the bulk
> > of it could be re-written in C, with some inline asm helpers as needed.
> Probably yes. But as this code is specific to OMAP3, do you think it's
> worth spending time on rewriting the entire code in C? It might be a
> significant effort and it will not be reusable for other ARM SOCs.

The C code will be more maintainable and offer more opportunities to
refactor across OMAP flavours - perhaps OMAP4 as well.

I'm also interested in code reuse across other SoCs. We've got to be able to
shut down the ARM core using common code instead of each SoC copying buggy
code from each other. Could you list reasons why you don't think the code is
re-usable?

Regards,
Amit

RE: [PATCH 2/2] OMAP3 PM: sleep code clean up

[Shilimkar, Santosh]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Amit Kucheria
> Sent: Monday, October 04, 2010 2:27 PM
> To: Sripathy, Vishwanath
> Cc: Kevin Hilman; linux-omap@vger.kernel.org; linaro-dev@lists.linaro.org
> Subject: Re: [PATCH 2/2] OMAP3 PM: sleep code clean up
> 
> On 10 Oct 04, Sripathy, Vishwanath wrote:
> > Kevin,
> 
> [snip]
> 
> > >
> > > But this also makes me wonder, if we're going to clean this up, the
> bulk
> > > of it could be re-written in C, with some inline asm helpers as needed.
> > Probably yes. But as this code is specific to OMAP3, do you think it's
> > worth spending time on rewriting the entire code in C? It might be a
> > significant effort and it will not be reusable for other ARM SOCs.
> 
> The C code will be more maintainable and offer more opportunities to
> refactor across OMAP flavours - perhaps OMAP4 as well.
> 
> I'm also interested in code reuse across other SoCs. We've got to be able
> to
> shut down the ARM core using common code instead of each SoC copying buggy
> code from each other. Could you list reasons why you don't think the code
> is
> re-usable?
Trust zone implementation and it's varients

OMAP3 sleep code clean-up

[Jean Pihet]

Vishwanath BS <vishwanath.bs@ti.com>, Jean Pihet <j-pihet@ti.com>:

 [PATCH 1/2]: OMAP3 PM: move omap3 sleep to ddr

   For historical reasons the OMAP3 sleep code is run from SRAM.
   This code can run from DDR which provides better performance and
   leaves the SRAM available for other uses.

   Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
   with full RET and OFF modes.

 [PATCH 2/2] OMAP3: clean up ASM idle code

   Clean up of the ASM code:
   - reworked and simplified the execution paths, for better
      readability and to avoid duplication of code,
   - reworked the comments for better readability,
   - reworked the code formating and alignment,
   - added comments for the i443 errata workarounds,
   - replaced the cache flush code by a call to the kernel
      common code for ARMv7 (v7_flush_kern_cache_all), which is
      better maintained and so more mature,
   - clean up of non used symbols.

   Tested on Zoom3, OMAP3EVM, Beagleboard, n900
   with full RET and OFF mode.

[PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Jean Pihet]

From: Vishwanath BS <vishwanath.bs@ti.com>

For historical reasons the OMAP3 sleep code is run from SRAM.
This code can run from DDR which provides better performance and
leaves the SRAM available for other uses.

Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
with full RET and OFF modes.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Kevin Hillman <khilman@deeprootsystems.com>

Changed the commit comments.

Cc: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 75c0cd1..035ca47 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -65,8 +65,6 @@ struct power_state {
 
 static LIST_HEAD(pwrst_list);
 
-static void (*_omap_sram_idle)(u32 *addr, int save_state);
-
 static int (*_omap_save_secure_sram)(u32 *addr);
 
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
@@ -346,9 +344,6 @@ void omap_sram_idle(void)
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;
 
-	if (!_omap_sram_idle)
-		return;
-
 	pwrdm_clear_all_prev_pwrst(mpu_pwrdm);
 	pwrdm_clear_all_prev_pwrst(neon_pwrdm);
 	pwrdm_clear_all_prev_pwrst(core_pwrdm);
@@ -422,7 +417,7 @@ void omap_sram_idle(void)
 	 * get saved. The restore path then reads from this
 	 * location and restores them back.
 	 */
-	_omap_sram_idle(omap3_arm_context, save_state);
+	omap34xx_cpu_suspend(omap3_arm_context, save_state);
 	cpu_init();
 
 	/* Restore normal SDRC POWER settings */
@@ -972,8 +967,6 @@ static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
 
 void omap_push_sram_idle(void)
 {
-	_omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
-					omap34xx_cpu_suspend_sz);
 	if (omap_type() != OMAP2_DEVICE_TYPE_GP)
 		_omap_save_secure_sram = omap_sram_push(save_secure_ram_context,
 				save_secure_ram_context_sz);
-- 
1.7.2.3

RE: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Nishanth Menon]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Jean Pihet
> Sent: Thursday, November 18, 2010 8:52 AM
> To: linux-omap@vger.kernel.org
> Cc: Vishwanath BS; Kevin Hillman; Jean Pihet
> Subject: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr
>
> From: Vishwanath BS <vishwanath.bs@ti.com>
>
> For historical reasons the OMAP3 sleep code is run from SRAM.
> This code can run from DDR which provides better performance and
> leaves the SRAM available for other uses.
>
> Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
> with full RET and OFF modes.

Sorry, But I disagree with this patch.

There is a silicon errata which cannot be handled with this - RTA disable
- errata i608

You need to disable RTA while coming out of OFF - we cannot handle this on

GP devices if this is not done.

>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Cc: Kevin Hillman <khilman@deeprootsystems.com>
>
> Changed the commit comments.
>
> Cc: Jean Pihet <j-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |    9 +--------
>  1 files changed, 1 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 75c0cd1..035ca47 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -65,8 +65,6 @@ struct power_state {
>
>  static LIST_HEAD(pwrst_list);
>
> -static void (*_omap_sram_idle)(u32 *addr, int save_state);
> -
>  static int (*_omap_save_secure_sram)(u32 *addr);
>
>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
> @@ -346,9 +344,6 @@ void omap_sram_idle(void)
>  	int core_prev_state, per_prev_state;
>  	u32 sdrc_pwr = 0;
>
> -	if (!_omap_sram_idle)
> -		return;
> -
>  	pwrdm_clear_all_prev_pwrst(mpu_pwrdm);
>  	pwrdm_clear_all_prev_pwrst(neon_pwrdm);
>  	pwrdm_clear_all_prev_pwrst(core_pwrdm);
> @@ -422,7 +417,7 @@ void omap_sram_idle(void)
>  	 * get saved. The restore path then reads from this
>  	 * location and restores them back.
>  	 */
> -	_omap_sram_idle(omap3_arm_context, save_state);
> +	omap34xx_cpu_suspend(omap3_arm_context, save_state);
>  	cpu_init();
>
>  	/* Restore normal SDRC POWER settings */
> @@ -972,8 +967,6 @@ static int __init clkdms_setup(struct clockdomain
> *clkdm, void *unused)
>
>  void omap_push_sram_idle(void)
>  {
> -	_omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> -					omap34xx_cpu_suspend_sz);
>  	if (omap_type() != OMAP2_DEVICE_TYPE_GP)
>  		_omap_save_secure_sram =
> omap_sram_push(save_secure_ram_context,
>  				save_secure_ram_context_sz);
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Sripathy, Vishwanath]

NIshant,

On Thu, Nov 18, 2010 at 8:27 PM, Nishanth Menon <nm@ti.com> wrote:
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Jean Pihet
>> Sent: Thursday, November 18, 2010 8:52 AM
>> To: linux-omap@vger.kernel.org
>> Cc: Vishwanath BS; Kevin Hillman; Jean Pihet
>> Subject: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr
>>
>> From: Vishwanath BS <vishwanath.bs@ti.com>
>>
>> For historical reasons the OMAP3 sleep code is run from SRAM.
>> This code can run from DDR which provides better performance and
>> leaves the SRAM available for other uses.
>>
>> Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
>> with full RET and OFF modes.
>
> Sorry, But I disagree with this patch.
>
> There is a silicon errata which cannot be handled with this - RTA disable
> - errata i608
>
> You need to disable RTA while coming out of OFF - we cannot handle this on
>
> GP devices if this is not done.
When you come out of Core off, SRAM contents are anyway lost. So you
have to run from DDR after ROM Code completes. This behavior has not
changed with this patch. I fail to understand your concern here.

Vishwa
>
>>
>> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
>> Cc: Kevin Hillman <khilman@deeprootsystems.com>
>>
>> Changed the commit comments.
>>
>> Cc: Jean Pihet <j-pihet@ti.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |    9 +--------
>>  1 files changed, 1 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
>> index 75c0cd1..035ca47 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -65,8 +65,6 @@ struct power_state {
>>
>>  static LIST_HEAD(pwrst_list);
>>
>> -static void (*_omap_sram_idle)(u32 *addr, int save_state);
>> -
>>  static int (*_omap_save_secure_sram)(u32 *addr);
>>
>>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>> @@ -346,9 +344,6 @@ void omap_sram_idle(void)
>>       int core_prev_state, per_prev_state;
>>       u32 sdrc_pwr = 0;
>>
>> -     if (!_omap_sram_idle)
>> -             return;
>> -
>>       pwrdm_clear_all_prev_pwrst(mpu_pwrdm);
>>       pwrdm_clear_all_prev_pwrst(neon_pwrdm);
>>       pwrdm_clear_all_prev_pwrst(core_pwrdm);
>> @@ -422,7 +417,7 @@ void omap_sram_idle(void)
>>        * get saved. The restore path then reads from this
>>        * location and restores them back.
>>        */
>> -     _omap_sram_idle(omap3_arm_context, save_state);
>> +     omap34xx_cpu_suspend(omap3_arm_context, save_state);
>>       cpu_init();
>>
>>       /* Restore normal SDRC POWER settings */
>> @@ -972,8 +967,6 @@ static int __init clkdms_setup(struct clockdomain
>> *clkdm, void *unused)
>>
>>  void omap_push_sram_idle(void)
>>  {
>> -     _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
>> -                                     omap34xx_cpu_suspend_sz);
>>       if (omap_type() != OMAP2_DEVICE_TYPE_GP)
>>               _omap_save_secure_sram =
>> omap_sram_push(save_secure_ram_context,
>>                               save_secure_ram_context_sz);
>> --
>> 1.7.2.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Nishanth Menon]

> -----Original Message-----
> From: Sripathy, Vishwanath [mailto:vishwanath.bs@ti.com]
> Sent: Thursday, November 18, 2010 9:09 AM
> To: Nishanth Menon
> Cc: Jean Pihet; linux-omap@vger.kernel.org; Kevin Hillman; Jean
Pihet-XID
> Subject: Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr
>
> NIshant,
>
> On Thu, Nov 18, 2010 at 8:27 PM, Nishanth Menon <nm@ti.com> wrote:
> >> -----Original Message-----
> >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >> owner@vger.kernel.org] On Behalf Of Jean Pihet
> >> Sent: Thursday, November 18, 2010 8:52 AM
> >> To: linux-omap@vger.kernel.org
> >> Cc: Vishwanath BS; Kevin Hillman; Jean Pihet
> >> Subject: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr
> >>
> >> From: Vishwanath BS <vishwanath.bs@ti.com>
> >>
> >> For historical reasons the OMAP3 sleep code is run from SRAM.
> >> This code can run from DDR which provides better performance and
> >> leaves the SRAM available for other uses.
> >>
> >> Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
> >> with full RET and OFF modes.
> >
> > Sorry, But I disagree with this patch.
> >
> > There is a silicon errata which cannot be handled with this - RTA
> disable
> > - errata i608
> >
> > You need to disable RTA while coming out of OFF - we cannot handle
this
> on
> >
> > GP devices if this is not done.
> When you come out of Core off, SRAM contents are anyway lost. So you
> have to run from DDR after ROM Code completes. This behavior has not
> changed with this patch. I fail to understand your concern here.
I could potentially be mistaken. Let me do one thing - I will post out the

patches that I have to the list and we can see how this all works
together.

Regards,
Nishanth Menon

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Jean Pihet]

Hi Nishant,

On Thu, Nov 18, 2010 at 4:11 PM, Nishanth Menon <nm@ti.com> wrote:
>> -----Original Message-----
>> From: Sripathy, Vishwanath [mailto:vishwanath.bs@ti.com]
>> Sent: Thursday, November 18, 2010 9:09 AM
>> To: Nishanth Menon
>> Cc: Jean Pihet; linux-omap@vger.kernel.org; Kevin Hillman; Jean
> Pihet-XID
>> Subject: Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr
>>
>> NIshant,
>>
>> On Thu, Nov 18, 2010 at 8:27 PM, Nishanth Menon <nm@ti.com> wrote:
>> >> -----Original Message-----
>> >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> >> owner@vger.kernel.org] On Behalf Of Jean Pihet
>> >> Sent: Thursday, November 18, 2010 8:52 AM
>> >> To: linux-omap@vger.kernel.org
>> >> Cc: Vishwanath BS; Kevin Hillman; Jean Pihet
>> >> Subject: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr
>> >>
>> >> From: Vishwanath BS <vishwanath.bs@ti.com>
>> >>
>> >> For historical reasons the OMAP3 sleep code is run from SRAM.
>> >> This code can run from DDR which provides better performance and
>> >> leaves the SRAM available for other uses.
>> >>
>> >> Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
>> >> with full RET and OFF modes.
>> >
>> > Sorry, But I disagree with this patch.
>> >
>> > There is a silicon errata which cannot be handled with this - RTA
>> disable
>> > - errata i608
>> >
>> > You need to disable RTA while coming out of OFF - we cannot handle
> this
>> on
>> >
>> > GP devices if this is not done.
>> When you come out of Core off, SRAM contents are anyway lost. So you
>> have to run from DDR after ROM Code completes. This behavior has not
>> changed with this patch. I fail to understand your concern here.
> I could potentially be mistaken. Let me do one thing - I will post out the
>
> patches that I have to the list and we can see how this all works
> together
Ok, fine.
I did not find any 36xx specific workaround for the errata i608 issue.
Is there one already available?

Thanks,
Jean

>
> Regards,
> Nishanth Menon
>

RE: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Nishanth Menon]

> -----Original Message-----
> From: Jean Pihet [mailto:jean.pihet@newoldbits.com]
> Sent: Thursday, November 18, 2010 9:15 AM

[...]
> >> >>
> >> >> Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
> >> >> with full RET and OFF modes.
> >> >
> >> > Sorry, But I disagree with this patch.
> >> >
> >> > There is a silicon errata which cannot be handled with this - RTA
> >> disable
> >> > - errata i608
> >> >
> >> > You need to disable RTA while coming out of OFF - we cannot handle
> > this
> >> on
> >> >
> >> > GP devices if this is not done.
> >> When you come out of Core off, SRAM contents are anyway lost. So you
> >> have to run from DDR after ROM Code completes. This behavior has not
> >> changed with this patch. I fail to understand your concern here.
> > I could potentially be mistaken. Let me do one thing - I will post out
> the
> >
> > patches that I have to the list and we can see how this all works
> > together
> Ok, fine.
> I did not find any 36xx specific workaround for the errata i608 issue.
> Is there one already available?

Yes, I have a series of patches addressing that as well.. hoping to test
On EMU/HS devices prior to posting - GP devices (3430/3630) tested.
Regards,
Nishanth Menon

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

>> From: Vishwanath BS <vishwanath.bs@ti.com>
>>
>> For historical reasons the OMAP3 sleep code is run from SRAM.
>> This code can run from DDR which provides better performance and
>> leaves the SRAM available for other uses.
>>
>> Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
>> with full RET and OFF modes.
>
> Sorry, But I disagree with this patch.
>
> There is a silicon errata which cannot be handled with this - RTA disable
> - errata i608
>
> You need to disable RTA while coming out of OFF - we cannot handle
> this on GP devices if this is not done.

You need to  provide some more details here as to exactly why this patch
prevents the ability to do this workaround.

As Vishwa pointed out, when returning from OFF mode, current code
already starts in DDR since SRAM is lost.  The current code also already
can jump back into SRAM for certain errata/fixup (c.f. es3_sdrc_fix in
current code.)

Kevin

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Nishanth Menon]

Kevin Hilman had written, on 11/18/2010 09:52 AM, the following:
> Nishanth Menon <nm@ti.com> writes:
> 
>>> From: Vishwanath BS <vishwanath.bs@ti.com>
>>>
>>> For historical reasons the OMAP3 sleep code is run from SRAM.
>>> This code can run from DDR which provides better performance and
>>> leaves the SRAM available for other uses.
>>>
>>> Tested on ZOOM3, OMAP3EVM, Beagleboard, n900
>>> with full RET and OFF modes.
>> Sorry, But I disagree with this patch.
>>
>> There is a silicon errata which cannot be handled with this - RTA disable
>> - errata i608
>>
>> You need to disable RTA while coming out of OFF - we cannot handle
>> this on GP devices if this is not done.
> 
> You need to  provide some more details here as to exactly why this patch
> prevents the ability to do this workaround.
> 
> As Vishwa pointed out, when returning from OFF mode, current code
> already starts in DDR since SRAM is lost.  The current code also already
> can jump back into SRAM for certain errata/fixup (c.f. es3_sdrc_fix in
> current code.)
scratchpad_contents.public_restore_ptr -> this is the restore pointer 
that is invoked when we get out of OFF mode.
	-> on 3430 and 3630, I agree it in SDRAM, for es3_sdrc_fix, it 
relocates the required code to sram as it cannot be run in ddr. - so I 
believe no issues there.

But after wfi in wait_sdrc_ok as part of the code executing in SRAM 
today omap34xx_cpu_suspend -> we are waiting for DPLL3 lock prior to 
accessing DDR -> how do we execute that logic in SDRAM?


-- 
Regards,
Nishanth Menon

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Tony Lindgren]

* Nishanth Menon <nm@ti.com> [101118 08:46]:
> 
> But after wfi in wait_sdrc_ok as part of the code executing in SRAM
> today omap34xx_cpu_suspend -> we are waiting for DPLL3 lock prior to
> accessing DDR -> how do we execute that logic in SDRAM?

I too am a bit concerned how this will all keep working. For light
testing it may be running OK if it happens to run from cache..

Also, moving the code out of SRAM will limit the options for what we
may need to do with DDR or L3.

Retention is something to consider, are there issues with that?

Regards,

Tony

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [101118 09:42]:
> * Nishanth Menon <nm@ti.com> [101118 08:46]:
> > 
> > But after wfi in wait_sdrc_ok as part of the code executing in SRAM
> > today omap34xx_cpu_suspend -> we are waiting for DPLL3 lock prior to
> > accessing DDR -> how do we execute that logic in SDRAM?
> 
> I too am a bit concerned how this will all keep working. For light
> testing it may be running OK if it happens to run from cache..

This in the retention case, in off-idle those should be powered down..

Tony

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Jean Pihet]

On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Nishanth Menon <nm@ti.com> [101118 08:46]:
>>
>> But after wfi in wait_sdrc_ok as part of the code executing in SRAM
>> today omap34xx_cpu_suspend -> we are waiting for DPLL3 lock prior to
>> accessing DDR -> how do we execute that logic in SDRAM?
> I too am a bit concerned how this will all keep working. For light
> testing it may be running OK if it happens to run from cache..

As Vishwa pointed out, when going out of OFF mode the SRAM and the
caches are lost.
That means the low power mode _has to_ rely on SDRAM at wake-up.

About the DPLL lock:
1) wait_sdrc_ok is only called when back from the non-OFF modes,
2) I checked that when running wait_sdrc_ok the CORE is already out of
idle and the DPLL is already locked. Note: l-o code has no support for
the voltages OFF and the external clocks OFF.

What to conclude from 1) and 2)? In my test setup ot looks like
wait_sdrc_ok is of no use, but I agree this a premature conclusion.

> Also, moving the code out of SRAM will limit the options for what we
> may need to do with DDR or L3.
What are the options? All the executable code runs from DDR anyway.

> Retention is something to consider, are there issues with that?
No issue so far. See the points 1) and 2) here above.

>
> Regards,
>
> Tony
>

Jean

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Tony Lindgren]

* Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> About the DPLL lock:
> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
> 2) I checked that when running wait_sdrc_ok the CORE is already out of
> idle and the DPLL is already locked. Note: l-o code has no support for
> the voltages OFF and the external clocks OFF.
> 
> What to conclude from 1) and 2)? In my test setup ot looks like
> wait_sdrc_ok is of no use, but I agree this a premature conclusion.

Yeah we should figure out in which cases wait_sdrc_ok is needed.

BTW, are you sure you're hitting core idle in your tests?

Regards,

Tony

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Jean Pihet]

On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
>>
>> About the DPLL lock:
>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
>> idle and the DPLL is already locked. Note: l-o code has no support for
>> the voltages OFF and the external clocks OFF.
>>
>> What to conclude from 1) and 2)? In my test setup ot looks like
>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
>
> Yeah we should figure out in which cases wait_sdrc_ok is needed.
>
> BTW, are you sure you're hitting core idle in your tests?
Yes it is OK from the console messages and the counters values in
/debug/pm_debug/count.

Let me confirm asap with the PRCM registers dump.

>
> Regards,
>
> Tony
>

Thanks,
Jean

>

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Jean Pihet]

HI Tony,

On Thu, Nov 18, 2010 at 7:34 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
>> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
>>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>
>>> About the DPLL lock:
>>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
>>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
>>> idle and the DPLL is already locked. Note: l-o code has no support for
>>> the voltages OFF and the external clocks OFF.
>>>
>>> What to conclude from 1) and 2)? In my test setup ot looks like
>>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
>>
>> Yeah we should figure out in which cases wait_sdrc_ok is needed.
>>
>> BTW, are you sure you're hitting core idle in your tests?
> Yes it is OK from the console messages and the counters values in
> /debug/pm_debug/count.
>
> Let me confirm asap with the PRCM registers dump.

Here is what I experimented:
1) added a cache flush (v7_flush_kern_cache_all) just before WFI, in all cases,
2) checked the real state entered in low power mode from the console
messages, the output of /debug/pm_debug/count and PRCM registers dump

2) is OK, which means that the RET and OFF modes are correctly hit.

Can I conclude from 1) that the wake-up code is not running from the
cache in RETention?

Thanks,
Jean

>
>>
>> Regards,
>>
>> Tony
>>
>
> Thanks,
> Jean
>
>>
>

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Tony Lindgren]

* Jean Pihet <jean.pihet@newoldbits.com> [101119 07:27]:
> HI Tony,
> 
> On Thu, Nov 18, 2010 at 7:34 PM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> > On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
> >> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
> >>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
> >>>
> >>> About the DPLL lock:
> >>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
> >>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
> >>> idle and the DPLL is already locked. Note: l-o code has no support for
> >>> the voltages OFF and the external clocks OFF.
> >>>
> >>> What to conclude from 1) and 2)? In my test setup ot looks like
> >>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
> >>
> >> Yeah we should figure out in which cases wait_sdrc_ok is needed.
> >>
> >> BTW, are you sure you're hitting core idle in your tests?
> > Yes it is OK from the console messages and the counters values in
> > /debug/pm_debug/count.
> >
> > Let me confirm asap with the PRCM registers dump.
> 
> Here is what I experimented:
> 1) added a cache flush (v7_flush_kern_cache_all) just before WFI, in all cases,
> 2) checked the real state entered in low power mode from the console
> messages, the output of /debug/pm_debug/count and PRCM registers dump
> 
> 2) is OK, which means that the RET and OFF modes are correctly hit.
> 
> Can I conclude from 1) that the wake-up code is not running from the
> cache in RETention?

Sounds pretty safe to me then, thanks for testing. Now we just have
to find out in which cases wait_sdrc_ok is needed..

Tony

RE: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Derrick, David]

>-----Original Message-----
>From: Jean Pihet [mailto:jean.pihet@newoldbits.com] 
>Sent: Friday, November 19, 2010 9:37 AM

>On Thu, Nov 18, 2010 at 7:34 PM, Jean Pihet <jean.pihet@newoldbits.com> >wrote:
>> On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
>>> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
>>>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>>
>>>> About the DPLL lock:
>>>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
>>>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
>>>> idle and the DPLL is already locked. Note: l-o code has no support for
>>>> the voltages OFF and the external clocks OFF.
>>>>
>>>> What to conclude from 1) and 2)? In my test setup ot looks like
>>>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
>>>
>>> Yeah we should figure out in which cases wait_sdrc_ok is needed.
>>>
>>> BTW, are you sure you're hitting core idle in your tests?
>> Yes it is OK from the console messages and the counters values in
>> /debug/pm_debug/count.
>>
>> Let me confirm asap with the PRCM registers dump.

>Here is what I experimented:
>1) added a cache flush (v7_flush_kern_cache_all) just before WFI, in all >cases,
>2) checked the real state entered in low power mode from the console
>messages, the output of /debug/pm_debug/count and PRCM registers dump

>2) is OK, which means that the RET and OFF modes are correctly hit.

>Can I conclude from 1) that the wake-up code is not running from the
>cache in RETention?

[Derrick, David] 

To add some context to the wait_sdrc_ok function and why it was added:

wait_sdrc_ok was added because the DLL takes 500 L3 clock cycles 
to lock. So you do not want to go back to DDR before DLL is locked. Also, we 
found some times DLL never locked so we introduced the DLL kick procedure to 
force it to lock.

Regards,
David

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Jean Pihet]

On Fri, Nov 19, 2010 at 5:14 PM, Derrick, David <dderrick@ti.com> wrote:
>>-----Original Message-----
>>From: Jean Pihet [mailto:jean.pihet@newoldbits.com]
>>Sent: Friday, November 19, 2010 9:37 AM
>
>>On Thu, Nov 18, 2010 at 7:34 PM, Jean Pihet <jean.pihet@newoldbits.com> >wrote:
>>> On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
>>>>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
>>>>>
>>>>> About the DPLL lock:
>>>>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
>>>>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
>>>>> idle and the DPLL is already locked. Note: l-o code has no support for
>>>>> the voltages OFF and the external clocks OFF.
>>>>>
>>>>> What to conclude from 1) and 2)? In my test setup ot looks like
>>>>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
>>>>
>>>> Yeah we should figure out in which cases wait_sdrc_ok is needed.
>>>>
>>>> BTW, are you sure you're hitting core idle in your tests?
>>> Yes it is OK from the console messages and the counters values in
>>> /debug/pm_debug/count.
>>>
>>> Let me confirm asap with the PRCM registers dump.
>
>>Here is what I experimented:
>>1) added a cache flush (v7_flush_kern_cache_all) just before WFI, in all >cases,
>>2) checked the real state entered in low power mode from the console
>>messages, the output of /debug/pm_debug/count and PRCM registers dump
>
>>2) is OK, which means that the RET and OFF modes are correctly hit.
>
>>Can I conclude from 1) that the wake-up code is not running from the
>>cache in RETention?
>
> [Derrick, David]
>
> To add some context to the wait_sdrc_ok function and why it was added:
>
> wait_sdrc_ok was added because the DLL takes 500 L3 clock cycles
> to lock. So you do not want to go back to DDR before DLL is locked. Also, we
> found some times DLL never locked so we introduced the DLL kick procedure to
> force it to lock.

Ok thanks for the details.
Does that mean that it is unsafe to run the sleep code (in
sleep34xx.S) from DDR?

>
> Regards,
> David
>
>

Regards,
Jean

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Peter 'p2' De Schrijver]

On Fri, Nov 19, 2010 at 05:14:15PM +0100, ext Derrick, David wrote:
> >-----Original Message-----
> >From: Jean Pihet [mailto:jean.pihet@newoldbits.com] 
> >Sent: Friday, November 19, 2010 9:37 AM
> 
> >On Thu, Nov 18, 2010 at 7:34 PM, Jean Pihet <jean.pihet@newoldbits.com> >wrote:
> >> On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
> >>> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
> >>>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
> >>>>
> >>>> About the DPLL lock:
> >>>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
> >>>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
> >>>> idle and the DPLL is already locked. Note: l-o code has no support for
> >>>> the voltages OFF and the external clocks OFF.
> >>>>
> >>>> What to conclude from 1) and 2)? In my test setup ot looks like
> >>>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
> >>>
> >>> Yeah we should figure out in which cases wait_sdrc_ok is needed.
> >>>
> >>> BTW, are you sure you're hitting core idle in your tests?
> >> Yes it is OK from the console messages and the counters values in
> >> /debug/pm_debug/count.
> >>
> >> Let me confirm asap with the PRCM registers dump.
> 
> >Here is what I experimented:
> >1) added a cache flush (v7_flush_kern_cache_all) just before WFI, in all >cases,
> >2) checked the real state entered in low power mode from the console
> >messages, the output of /debug/pm_debug/count and PRCM registers dump
> 
> >2) is OK, which means that the RET and OFF modes are correctly hit.
> 
> >Can I conclude from 1) that the wake-up code is not running from the
> >cache in RETention?
> 
> [Derrick, David] 
> 
> To add some context to the wait_sdrc_ok function and why it was added:
> 
> wait_sdrc_ok was added because the DLL takes 500 L3 clock cycles 
> to lock. So you do not want to go back to DDR before DLL is locked. Also, we 
> found some times DLL never locked so we introduced the DLL kick procedure to 
> force it to lock.
> 

The root cause for the DLL not locking has been found though and a
workaround implemented. So it should work now :) That still leaves the
500 L3 cycle delay though.

Cheers,

Peter.

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Kevin Hilman]

"Peter 'p2' De Schrijver" <peter.de-schrijver@nokia.com> writes:

> On Fri, Nov 19, 2010 at 05:14:15PM +0100, ext Derrick, David wrote:
>> >-----Original Message-----
>> >From: Jean Pihet [mailto:jean.pihet@newoldbits.com] 
>> >Sent: Friday, November 19, 2010 9:37 AM
>> 
>> >On Thu, Nov 18, 2010 at 7:34 PM, Jean Pihet <jean.pihet@newoldbits.com> >wrote:
>> >> On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
>> >>> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
>> >>>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
>> >>>>
>> >>>> About the DPLL lock:
>> >>>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
>> >>>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
>> >>>> idle and the DPLL is already locked. Note: l-o code has no support for
>> >>>> the voltages OFF and the external clocks OFF.
>> >>>>
>> >>>> What to conclude from 1) and 2)? In my test setup ot looks like
>> >>>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
>> >>>
>> >>> Yeah we should figure out in which cases wait_sdrc_ok is needed.
>> >>>
>> >>> BTW, are you sure you're hitting core idle in your tests?
>> >> Yes it is OK from the console messages and the counters values in
>> >> /debug/pm_debug/count.
>> >>
>> >> Let me confirm asap with the PRCM registers dump.
>> 
>> >Here is what I experimented:
>> >1) added a cache flush (v7_flush_kern_cache_all) just before WFI, in all >cases,
>> >2) checked the real state entered in low power mode from the console
>> >messages, the output of /debug/pm_debug/count and PRCM registers dump
>> 
>> >2) is OK, which means that the RET and OFF modes are correctly hit.
>> 
>> >Can I conclude from 1) that the wake-up code is not running from the
>> >cache in RETention?
>> 
>> [Derrick, David] 
>> 
>> To add some context to the wait_sdrc_ok function and why it was added:
>> 
>> wait_sdrc_ok was added because the DLL takes 500 L3 clock cycles 
>> to lock. So you do not want to go back to DDR before DLL is locked. Also, we 
>> found some times DLL never locked so we introduced the DLL kick procedure to 
>> force it to lock.
>> 
>
> The root cause for the DLL not locking has been found though and a
> workaround implemented. So it should work now :) 

Is the workaround for this reflected in Nishanth's series?

Kevin

> That still leaves the
> 500 L3 cycle delay though.
>
> Cheers,
>
> Peter.

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Jean Pihet]

On Mon, Nov 22, 2010 at 5:03 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> "Peter 'p2' De Schrijver" <peter.de-schrijver@nokia.com> writes:
>
>> On Fri, Nov 19, 2010 at 05:14:15PM +0100, ext Derrick, David wrote:
>>> >-----Original Message-----
>>> >From: Jean Pihet [mailto:jean.pihet@newoldbits.com]
>>> >Sent: Friday, November 19, 2010 9:37 AM
>>>
>>> >On Thu, Nov 18, 2010 at 7:34 PM, Jean Pihet <jean.pihet@newoldbits.com> >wrote:
>>> >> On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
>>> >>> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
>>> >>>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
>>> >>>>
>>> >>>> About the DPLL lock:
>>> >>>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
>>> >>>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
>>> >>>> idle and the DPLL is already locked. Note: l-o code has no support for
>>> >>>> the voltages OFF and the external clocks OFF.
>>> >>>>
>>> >>>> What to conclude from 1) and 2)? In my test setup ot looks like
>>> >>>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
>>> >>>
>>> >>> Yeah we should figure out in which cases wait_sdrc_ok is needed.
>>> >>>
>>> >>> BTW, are you sure you're hitting core idle in your tests?
>>> >> Yes it is OK from the console messages and the counters values in
>>> >> /debug/pm_debug/count.
>>> >>
>>> >> Let me confirm asap with the PRCM registers dump.
>>>
>>> >Here is what I experimented:
>>> >1) added a cache flush (v7_flush_kern_cache_all) just before WFI, in all >cases,
>>> >2) checked the real state entered in low power mode from the console
>>> >messages, the output of /debug/pm_debug/count and PRCM registers dump
>>>
>>> >2) is OK, which means that the RET and OFF modes are correctly hit.
>>>
>>> >Can I conclude from 1) that the wake-up code is not running from the
>>> >cache in RETention?
>>>
>>> [Derrick, David]
>>>
>>> To add some context to the wait_sdrc_ok function and why it was added:
>>>
>>> wait_sdrc_ok was added because the DLL takes 500 L3 clock cycles
>>> to lock. So you do not want to go back to DDR before DLL is locked. Also, we
>>> found some times DLL never locked so we introduced the DLL kick procedure to
>>> force it to lock.
>>>
>>
>> The root cause for the DLL not locking has been found though and a
>> workaround implemented. So it should work now :)
>
> Is the workaround for this reflected in Nishanth's series?
Yes the patch is '[PATCH 02/13] OMAP3: PM: Errata i581 suppport: dll
kick strategy'

>
> Kevin
>
>> That still leaves the
>> 500 L3 cycle delay though.
>>
>> Cheers,
>>
>> Peter.
>

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Peter 'p2' De Schrijver]

On Mon, Nov 22, 2010 at 05:03:59PM +0100, ext Kevin Hilman wrote:
> "Peter 'p2' De Schrijver" <peter.de-schrijver@nokia.com> writes:
> 
> > On Fri, Nov 19, 2010 at 05:14:15PM +0100, ext Derrick, David wrote:
> >> >-----Original Message-----
> >> >From: Jean Pihet [mailto:jean.pihet@newoldbits.com] 
> >> >Sent: Friday, November 19, 2010 9:37 AM
> >> 
> >> >On Thu, Nov 18, 2010 at 7:34 PM, Jean Pihet <jean.pihet@newoldbits.com> >wrote:
> >> >> On Thu, Nov 18, 2010 at 7:27 PM, Tony Lindgren <tony@atomide.com> wrote:
> >> >>> * Jean Pihet <jean.pihet@newoldbits.com> [101118 10:06]:
> >> >>>> On Thu, Nov 18, 2010 at 6:52 PM, Tony Lindgren <tony@atomide.com> wrote:
> >> >>>>
> >> >>>> About the DPLL lock:
> >> >>>> 1) wait_sdrc_ok is only called when back from the non-OFF modes,
> >> >>>> 2) I checked that when running wait_sdrc_ok the CORE is already out of
> >> >>>> idle and the DPLL is already locked. Note: l-o code has no support for
> >> >>>> the voltages OFF and the external clocks OFF.
> >> >>>>
> >> >>>> What to conclude from 1) and 2)? In my test setup ot looks like
> >> >>>> wait_sdrc_ok is of no use, but I agree this a premature conclusion.
> >> >>>
> >> >>> Yeah we should figure out in which cases wait_sdrc_ok is needed.
> >> >>>
> >> >>> BTW, are you sure you're hitting core idle in your tests?
> >> >> Yes it is OK from the console messages and the counters values in
> >> >> /debug/pm_debug/count.
> >> >>
> >> >> Let me confirm asap with the PRCM registers dump.
> >> 
> >> >Here is what I experimented:
> >> >1) added a cache flush (v7_flush_kern_cache_all) just before WFI, in all >cases,
> >> >2) checked the real state entered in low power mode from the console
> >> >messages, the output of /debug/pm_debug/count and PRCM registers dump
> >> 
> >> >2) is OK, which means that the RET and OFF modes are correctly hit.
> >> 
> >> >Can I conclude from 1) that the wake-up code is not running from the
> >> >cache in RETention?
> >> 
> >> [Derrick, David] 
> >> 
> >> To add some context to the wait_sdrc_ok function and why it was added:
> >> 
> >> wait_sdrc_ok was added because the DLL takes 500 L3 clock cycles 
> >> to lock. So you do not want to go back to DDR before DLL is locked. Also, we 
> >> found some times DLL never locked so we introduced the DLL kick procedure to 
> >> force it to lock.
> >> 
> >
> > The root cause for the DLL not locking has been found though and a
> > workaround implemented. So it should work now :) 
> 
> Is the workaround for this reflected in Nishanth's series?

No. It seems not. The workaround needs VDD2 voltage scaling which seems
to be missing now from l-o ?

Cheers,

Peter.

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Nishanth Menon]

Peter 'p2' De Schrijver had written, on 11/22/2010 10:22 AM, the following:
[...]
>>> The root cause for the DLL not locking has been found though and a
>>> workaround implemented. So it should work now :) 
>> Is the workaround for this reflected in Nishanth's series?
> 
> No. It seems not. The workaround needs VDD2 voltage scaling which seems
> to be missing now from l-o ?

yep, that is correct - we dont have dvfs/OPP/voltage scaling in 
kernel.org as of today - until we have those, it might not be possible 
to push the "real workaround". the mentioned patch is, as documented, 
support for the voltage workaround.

-- 
Regards,
Nishanth Menon

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Kevin Hilman]

Nishanth Menon <nm@ti.com> writes:

> Peter 'p2' De Schrijver had written, on 11/22/2010 10:22 AM, the following:
> [...]
>>>> The root cause for the DLL not locking has been found though and a
>>>> workaround implemented. So it should work now :) 
>>> Is the workaround for this reflected in Nishanth's series?
>>
>> No. It seems not. The workaround needs VDD2 voltage scaling which seems
>> to be missing now from l-o ?
>
> yep, that is correct - we dont have dvfs/OPP/voltage scaling in
> kernel.org as of today - until we have those, it might not be possible
> to push the "real workaround". the mentioned patch is, as documented,
> support for the voltage workaround.

It would be worth noting other known issues that are not worked around
in this series due to external dependencies.

Also, is the current SR/voltage series as proposed capable of handling
the workaround for this fix?

Kevin

Re: [PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Nishanth Menon]

Kevin Hilman had written, on 11/22/2010 12:23 PM, the following:
> Nishanth Menon <nm@ti.com> writes:
> 
>> Peter 'p2' De Schrijver had written, on 11/22/2010 10:22 AM, the following:
>> [...]
>>>>> The root cause for the DLL not locking has been found though and a
>>>>> workaround implemented. So it should work now :) 
>>>> Is the workaround for this reflected in Nishanth's series?
>>> No. It seems not. The workaround needs VDD2 voltage scaling which seems
>>> to be missing now from l-o ?
>> yep, that is correct - we dont have dvfs/OPP/voltage scaling in
>> kernel.org as of today - until we have those, it might not be possible
>> to push the "real workaround". the mentioned patch is, as documented,
>> support for the voltage workaround.
> 
> It would be worth noting other known issues that are not worked around
> in this series due to external dependencies.
I believe I have done that
http://marc.info/?l=linux-omap&m=129013172525234&w=2
"IMPORTANT: this is not a complete workaround implementation as recommended
by the silicon errata. this is a support logic for detecting lockups and
attempting to recover where possible and is known to provide stability
in multiple platforms.
"

> 
> Also, is the current SR/voltage series as proposed capable of handling
> the workaround for this fix?
I believe a new series is due sometime- we should see if the required 
changes are part of that series.

Regards,
Nishanth Menon

[PATCH 0/2] OMAP3 PM: sleep code clean up

[Vishwanath BS]

This patch series has some clean up in OMAP3 sleep code.
Patches have been rebased to latest kevin's pm branch.

Vishwanath BS (2):
  OMAP3 PM: move omap3 sleep to ddr
  OMAP3 PM: sleep code clean up

 arch/arm/mach-omap2/pm34xx.c              |    9 +-
 arch/arm/mach-omap2/sleep34xx.S           |  377 ++++++++++++++---------------
 arch/arm/plat-omap/include/plat/control.h |    2 +
 3 files changed, 190 insertions(+), 198 deletions(-)

[PATCH 1/2] OMAP3 PM: move omap3 sleep to ddr

[Vishwanath BS]

There is no need to keep omap3 sleep code in SRAM. This code can be run very
well on DDR. This would help us to instrument CPUIdle latencies.

Tested on ZOOM3.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Cc: Kevin Hillman <khilman@deeprootsystems.com>
---
 arch/arm/mach-omap2/pm34xx.c |    9 +--------
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 85ef245..ed9d12c 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -79,8 +79,6 @@ struct power_state {
 
 static LIST_HEAD(pwrst_list);
 
-static void (*_omap_sram_idle)(u32 *addr, int save_state);
-
 static int (*_omap_save_secure_sram)(u32 *addr);
 
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
@@ -360,9 +358,6 @@ void omap_sram_idle(void)
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;
 
-	if (!_omap_sram_idle)
-		return;
-
 	pwrdm_clear_all_prev_pwrst(mpu_pwrdm);
 	pwrdm_clear_all_prev_pwrst(neon_pwrdm);
 	pwrdm_clear_all_prev_pwrst(core_pwrdm);
@@ -438,7 +433,7 @@ void omap_sram_idle(void)
 	 * get saved. The restore path then reads from this
 	 * location and restores them back.
 	 */
-	_omap_sram_idle(omap3_arm_context, save_state);
+	omap34xx_cpu_suspend(omap3_arm_context, save_state);
 	cpu_init();
 
 	if (is_suspending())
@@ -995,8 +990,6 @@ static int __init clkdms_setup(struct clockdomain *clkdm, void *unused)
 
 void omap_push_sram_idle(void)
 {
-	_omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
-					omap34xx_cpu_suspend_sz);
 	if (omap_type() != OMAP2_DEVICE_TYPE_GP)
 		_omap_save_secure_sram = omap_sram_push(save_secure_ram_context,
 				save_secure_ram_context_sz);
-- 
1.7.0.4