[PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

2010-12-18 Thread jean . pihet
From: Jean Pihet j-pi...@ti.com

- Reworked and simplified the execution paths for better
  readability and to avoid duplication of code,
- Added comments on the entry and exit points and the interaction
  with the ROM code for OFF mode restore,
- Reworked the existing comments for better readability.

Tested on N900 and Beagleboard with full RET and OFF modes,
using cpuidle and suspend.

Signed-off-by: Jean Pihet j-pi...@ti.com
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
Tested-by: Nishanth Menon n...@ti.com
---
 arch/arm/mach-omap2/control.c   |9 +-
 arch/arm/mach-omap2/sleep34xx.S |  317 +++
 2 files changed, 195 insertions(+), 131 deletions(-)

diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 728f268..f4b19ed 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -239,7 +239,14 @@ void omap3_save_scratchpad_contents(void)
struct omap3_scratchpad_prcm_block prcm_block_contents;
struct omap3_scratchpad_sdrc_block sdrc_block_contents;
 
-   /* Populate the Scratchpad contents */
+   /*
+* Populate the Scratchpad contents
+*
+* The get_*restore_pointer functions are used to provide a
+* physical restore address where the ROM code jumps while waking
+* up from MPU OFF/OSWR state.
+* The restore pointer is stored into the scratchpad.
+*/
scratchpad_contents.boot_config_ptr = 0x0;
if (cpu_is_omap3630())
scratchpad_contents.public_restore_ptr =
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index beeb682..0e27429 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -71,6 +71,13 @@
  * API functions
  */
 
+/*
+ * The get_*restore_pointer functions are used to provide a
+ * physical restore address where the ROM code jumps while waking
+ * up from MPU OFF/OSWR state.
+ * The restore pointer is stored into the scratchpad.
+ */
+
.text
 /* Function call to get the restore pointer for resume from OFF */
 ENTRY(get_restore_pointer)
@@ -102,7 +109,7 @@ ENTRY(get_es3_restore_pointer_sz)
 /*
  * L2 cache needs to be toggled for stable OFF mode functionality on 3630.
  * This function sets up a flag that will allow for this toggling to take
- * place on 3630. Hopefully some version in the future maynot need this
+ * place on 3630. Hopefully some version in the future may not need this.
  */
 ENTRY(enable_omap3630_toggle_l2_on_restore)
 stmfd   sp!, {lr} @ save registers on stack
@@ -144,34 +151,162 @@ ENTRY(save_secure_ram_context_sz)
.word   . - save_secure_ram_context
 
 /*
+ * ==
+ * == Idle entry point ==
+ * ==
+ */
+
+/*
  * Forces OMAP into idle state
  *
- * omap34xx_suspend() - This bit of code just executes the WFI
- * for normal idles.
+ * omap34xx_cpu_suspend() - This bit of code saves the CPU context if needed
+ * and executes the WFI instruction. Calling WFI effectively changes the
+ * power domains states to the desired target power states.
+ *
  *
- * 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.
+ * Notes:
+ * - this code gets copied to internal SRAM at boot. The execution pointer
+ *   in SRAM is _omap_sram_idle.
+ * - when the OMAP wakes up it continues at different execution points
+ *   depending on the low power mode (non-OFF vs OFF modes),
+ *   cf. 'Resume path for xxx mode' comments.
  */
 ENTRY(omap34xx_cpu_suspend)
stmfd   sp!, {r0-r12, lr}   @ save registers on stack
 
-   /* 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
+   /*
+* r0 contains restore pointer in sdram
+* r1 contains information about saving context:
+*   0 - No context lost
+*   1 - Only L1 and logic lost
+*   2 - Only L2 lost
+*   3 - Both L1 and L2 lost
+*/
 
+   /* Directly jump to WFI is the context save is not required */
cmp r1, #0x0
-   /* If context save is required, do that and execute wfi */
-   bne save_context_wfi
+   beq omap3_do_wfi
+
+   /* Otherwise fall through to the save context code */
+save_context_wfi:
+   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
+   

[PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

2010-12-17 Thread jean . pihet
From: Jean Pihet j-pi...@ti.com

- Reworked and simplified the execution paths for better
  readability and to avoid duplication of code,
- Added comments on the entry and exit points and the interaction
  with the ROM code for OFF mode restore,
- Reworked the existing comments for better readability.

Tested on N900 and Beagleboard with full RET and OFF modes,
using cpuidle and suspend.

Signed-off-by: Jean Pihet j-pi...@ti.com
---
 arch/arm/mach-omap2/control.c   |9 +-
 arch/arm/mach-omap2/sleep34xx.S |  313 +++
 2 files changed, 191 insertions(+), 131 deletions(-)

diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 728f268..f4b19ed 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -239,7 +239,14 @@ void omap3_save_scratchpad_contents(void)
struct omap3_scratchpad_prcm_block prcm_block_contents;
struct omap3_scratchpad_sdrc_block sdrc_block_contents;
 
-   /* Populate the Scratchpad contents */
+   /*
+* Populate the Scratchpad contents
+*
+* The get_*restore_pointer functions are used to provide a
+* physical restore address where the ROM code jumps while waking
+* up from MPU OFF/OSWR state.
+* The restore pointer is stored into the scratchpad.
+*/
scratchpad_contents.boot_config_ptr = 0x0;
if (cpu_is_omap3630())
scratchpad_contents.public_restore_ptr =
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index beeb682..12061fd 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -71,6 +71,13 @@
  * API functions
  */
 
+/*
+ * The get_*restore_pointer functions are used to provide a
+ * physical restore address where the ROM code jumps while waking
+ * up from MPU OFF/OSWR state.
+ * The restore pointer is stored into the scratchpad.
+ */
+
.text
 /* Function call to get the restore pointer for resume from OFF */
 ENTRY(get_restore_pointer)
@@ -102,7 +109,7 @@ ENTRY(get_es3_restore_pointer_sz)
 /*
  * L2 cache needs to be toggled for stable OFF mode functionality on 3630.
  * This function sets up a flag that will allow for this toggling to take
- * place on 3630. Hopefully some version in the future maynot need this
+ * place on 3630. Hopefully some version in the future may not need this.
  */
 ENTRY(enable_omap3630_toggle_l2_on_restore)
 stmfd   sp!, {lr} @ save registers on stack
@@ -144,34 +151,158 @@ ENTRY(save_secure_ram_context_sz)
.word   . - save_secure_ram_context
 
 /*
+ * ==
+ * == Idle entry point ==
+ * ==
+ */
+
+/*
  * Forces OMAP into idle state
  *
- * omap34xx_suspend() - This bit of code just executes the WFI
- * for normal idles.
+ * omap34xx_suspend() - This bit of code saves the CPU context if needed
+ * and executes the WFI instruction
  *
- * 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.
+ * Notes:
+ * - this code gets copied to internal SRAM at boot.
+ * - when the OMAP wakes up it continues at different execution points
+ *   depending on the low power mode (non-OFF vs OFF modes),
+ *   cf. 'Resume path for xxx mode' comments.
  */
 ENTRY(omap34xx_cpu_suspend)
stmfd   sp!, {r0-r12, lr}   @ save registers on stack
 
-   /* 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
+   /*
+* r0 contains restore pointer in sdram
+* r1 contains information about saving context:
+*   0 - No context lost
+*   1 - Only L1 and logic lost
+*   2 - Only L2 lost
+*   3 - Both L1 and L2 lost
+*/
 
+   /* Save context only if required */
cmp r1, #0x0
-   /* If context save is required, do that and execute wfi */
-   bne save_context_wfi
+   beq omap3_do_wfi
+
+save_context_wfi:
+   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 from r1 */
+   cmp r1, #0x2@ Only L2 lost, no need to save context
+   beq clean_caches
+
+l1_logic_lost:
+   /* Store 

Re: [PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

2010-12-17 Thread Jean Pihet
On Fri, Dec 17, 2010 at 6:04 AM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
 ow...@vger.kernel.org] On Behalf Of jean.pi...@newoldbits.com
 Sent: Thursday, December 16, 2010 11:21 PM
 To: linux-omap@vger.kernel.org
 Cc: khil...@deeprootsystems.com; linux-arm-ker...@lists.infradead.org;
 Jean Pihet
 Subject: [PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

 From: Jean Pihet j-pi...@ti.com

 - Reworked and simplified the execution paths for better
   readability and to avoid duplication of code,
 - Added comments on the entry and exit points and the interaction
   with the ROM code for OFF mode restore,
 - Reworked the existing comments for better readability.

 Tested on N900 and Beagleboard with full RET and OFF modes,
 using cpuidle and suspend.

 Signed-off-by: Jean Pihet j-pi...@ti.com
 ---
  arch/arm/mach-omap2/control.c   |   10 +-
  arch/arm/mach-omap2/sleep34xx.S |  309
 ++
 
  2 files changed, 188 insertions(+), 131 deletions(-)

 diff --git a/arch/arm/mach-omap2/control.c
 b/arch/arm/mach-omap2/control.c
 index 728f268..5cb7276 100644
 --- a/arch/arm/mach-omap2/control.c
 +++ b/arch/arm/mach-omap2/control.c
 @@ -239,7 +239,15 @@ void omap3_save_scratchpad_contents(void)
       struct omap3_scratchpad_prcm_block prcm_block_contents;
       struct omap3_scratchpad_sdrc_block sdrc_block_contents;

 -     /* Populate the Scratchpad contents */
 +     /*
 +      * Populate the Scratchpad contents
 +      *
 +      * The get_*restore_pointer functions are used to get the resume
 +      *  function pointer to be called by the ROM code when back from
 WFI
 +      *  in OFF mode.
 Align your text here.
 +      * The restore pointer is stored into the scratchpad for later
 access
 +      *  by the ROM code.
 The text needs to be reworked a bit.

 The get_*restore_pointer functions are used to provide a physical
 restore
 address where ROM code jumps while waking up from MPU OFF/OSWR state.
 The restore pointer is stored into the scratchpad

That sounds better. Fixed.


 +      */
       scratchpad_contents.boot_config_ptr = 0x0;
       if (cpu_is_omap3630())
               scratchpad_contents.public_restore_ptr =
 diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-
 omap2/sleep34xx.S
 index 426af02..55ddd5a 100644
 --- a/arch/arm/mach-omap2/sleep34xx.S
 +++ b/arch/arm/mach-omap2/sleep34xx.S
 @@ -70,6 +70,11 @@
   * API functions
   */

 +/*
 + * The get_*restore_pointer functions are returning the resume
 + *  function pointer to be called by the ROM code when back from WFI
 + *  in OFF mode.
 Ditto
 The get_*restore_pointer functions are used to provide a physical
 restore
 address where ROM code jumps while waking up from MPU OFF/OSWR state.
 The restore pointer is stored into the scratchpad
Idem


 + */
       .text
  /* Function call to get the restore pointer for resume from OFF */
  ENTRY(get_restore_pointer)
 @@ -101,7 +106,7 @@ ENTRY(get_es3_restore_pointer_sz)
  /*
   * L2 cache needs to be toggled for stable OFF mode functionality on
 3630.
   * This function sets up a flag that will allow for this toggling to
 take
 - * place on 3630. Hopefully some version in the future maynot need this
 + * place on 3630. Hopefully some version in the future maynot need
 this.
 'maynot' . may not
Fixed

   */
  ENTRY(enable_omap3630_toggle_l2_on_restore)
          stmfd   sp!, {lr}     @ save registers on stack
 @@ -143,34 +148,156 @@ ENTRY(save_secure_ram_context_sz)
       .word   . - save_secure_ram_context

  /*
 + * ==
 + * == Idle entry point ==
 + * ==
 + */
 +
 +/*
   * Forces OMAP into idle state
   *
 - * omap34xx_suspend() - This bit of code just executes the WFI
 - * for normal idles.
 + * omap34xx_suspend() - This bit of code saves the CPU context if
 needed
 + * and executes the WFI instruction
   *
 - * 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.
 + * Notes:
 + * - this code gets copied to internal SRAM at boot.
 + * - when the OMAP wakes up it continues at different execution points
 + *   depending on the low power mode (non-OFF vs OFF modes),
 + *   cf. 'Resume path for xxx mode' comments.
   */
  ENTRY(omap34xx_cpu_suspend)
       stmfd   sp!, {r0-r12, lr}               @ save registers on stack

 -     /* 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
 +     /*
 +      * r0 contains restore pointer in sdram
 +      * r1 contains information about saving context

RE: [PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

2010-12-17 Thread Santosh Shilimkar
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
 ow...@vger.kernel.org] On Behalf Of jean.pi...@newoldbits.com
 Sent: Friday, December 17, 2010 3:38 PM
 To: linux-omap@vger.kernel.org
 Cc: khil...@deeprootsystems.com; linux-arm-ker...@lists.infradead.org;
 Jean Pihet
 Subject: [PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

 From: Jean Pihet j-pi...@ti.com

 - Reworked and simplified the execution paths for better
   readability and to avoid duplication of code,
 - Added comments on the entry and exit points and the interaction
   with the ROM code for OFF mode restore,
 - Reworked the existing comments for better readability.

 Tested on N900 and Beagleboard with full RET and OFF modes,
 using cpuidle and suspend.

 Signed-off-by: Jean Pihet j-pi...@ti.com

Acked-by: Santosh Shilimkar santosh.shilim...@ti.com

 ---
  arch/arm/mach-omap2/control.c   |9 +-
  arch/arm/mach-omap2/sleep34xx.S |  313
+++---
 -
  2 files changed, 191 insertions(+), 131 deletions(-)

 diff --git a/arch/arm/mach-omap2/control.c
b/arch/arm/mach-omap2/control.c
 index 728f268..f4b19ed 100644
 --- a/arch/arm/mach-omap2/control.c
 +++ b/arch/arm/mach-omap2/control.c
 @@ -239,7 +239,14 @@ void omap3_save_scratchpad_contents(void)
   struct omap3_scratchpad_prcm_block prcm_block_contents;
   struct omap3_scratchpad_sdrc_block sdrc_block_contents;

 - /* Populate the Scratchpad contents */
 + /*
 +  * Populate the Scratchpad contents
 +  *
 +  * The get_*restore_pointer functions are used to provide a
 +  * physical restore address where the ROM code jumps while waking
 +  * up from MPU OFF/OSWR state.
 +  * The restore pointer is stored into the scratchpad.
 +  */
   scratchpad_contents.boot_config_ptr = 0x0;
   if (cpu_is_omap3630())
   scratchpad_contents.public_restore_ptr =
 diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-
 omap2/sleep34xx.S
 index beeb682..12061fd 100644
 --- a/arch/arm/mach-omap2/sleep34xx.S
 +++ b/arch/arm/mach-omap2/sleep34xx.S
 @@ -71,6 +71,13 @@
   * API functions
   */

 +/*
 + * The get_*restore_pointer functions are used to provide a
 + * physical restore address where the ROM code jumps while waking
 + * up from MPU OFF/OSWR state.
 + * The restore pointer is stored into the scratchpad.
 + */
 +
   .text
  /* Function call to get the restore pointer for resume from OFF */
  ENTRY(get_restore_pointer)
 @@ -102,7 +109,7 @@ ENTRY(get_es3_restore_pointer_sz)
  /*
   * L2 cache needs to be toggled for stable OFF mode functionality on
3630.
   * This function sets up a flag that will allow for this toggling to
take
 - * place on 3630. Hopefully some version in the future maynot need this
 + * place on 3630. Hopefully some version in the future may not need
this.
   */
  ENTRY(enable_omap3630_toggle_l2_on_restore)
  stmfd   sp!, {lr} @ save registers on stack
 @@ -144,34 +151,158 @@ ENTRY(save_secure_ram_context_sz)
   .word   . - save_secure_ram_context

  /*
 + * ==
 + * == Idle entry point ==
 + * ==
 + */
 +
 +/*
   * Forces OMAP into idle state
   *
 - * omap34xx_suspend() - This bit of code just executes the WFI
 - * for normal idles.
 + * omap34xx_suspend() - This bit of code saves the CPU context if
needed
 + * and executes the WFI instruction
   *
 - * 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.
 + * Notes:
 + * - this code gets copied to internal SRAM at boot.
 + * - when the OMAP wakes up it continues at different execution points
 + *   depending on the low power mode (non-OFF vs OFF modes),
 + *   cf. 'Resume path for xxx mode' comments.
   */
  ENTRY(omap34xx_cpu_suspend)
   stmfd   sp!, {r0-r12, lr}   @ save registers on stack

 - /* 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
 + /*
 +  * r0 contains restore pointer in sdram
 +  * r1 contains information about saving context:
 +  *   0 - No context lost
 +  *   1 - Only L1 and logic lost
 +  *   2 - Only L2 lost
 +  *   3 - Both L1 and L2 lost
 +  */

 + /* Save context only if required */
   cmp r1, #0x0
 - /* If context save is required, do that and execute wfi */
 - bne save_context_wfi
 + beq omap3_do_wfi
 +
 +save_context_wfi:
 + mov r8, r0  @ Store SDRAM address in r8
 + mrc p15, 0, r5, c1, c0, 1   @ Read Auxiliary Control Register
 + mov r4, #0x1

Re: [PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

2010-12-17 Thread Nishanth Menon
jean.pi...@newoldbits.com had written, on 12/17/2010 04:08 AM, the 
following:

From: Jean Pihet j-pi...@ti.com

- Reworked and simplified the execution paths for better
  readability and to avoid duplication of code,
is it possible to split this good rewrite into logical chunks for better 
and easier reviewability?


my suggestion clean each of the paths independently - like the code upto 
wfi.. and then we can clean up the resume path as well separately.



- Added comments on the entry and exit points and the interaction
  with the ROM code for OFF mode restore,
- Reworked the existing comments for better readability.
thanks for doing this, but, just my suggestion, looking at the amount of 
changes done in this patch - if you can keep one patch for functional 
change which is separate from cosmetic change (such as comment cleanup 
and addition), it makes a reviewer's life easier :)




Tested on N900 and Beagleboard with full RET and OFF modes,
using cpuidle and suspend.

Signed-off-by: Jean Pihet j-pi...@ti.com
---
 arch/arm/mach-omap2/control.c   |9 +-
 arch/arm/mach-omap2/sleep34xx.S |  313 +++
 2 files changed, 191 insertions(+), 131 deletions(-)

diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 728f268..f4b19ed 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -239,7 +239,14 @@ void omap3_save_scratchpad_contents(void)
struct omap3_scratchpad_prcm_block prcm_block_contents;
struct omap3_scratchpad_sdrc_block sdrc_block_contents;
 
-	/* Populate the Scratchpad contents */

+   /*
+* Populate the Scratchpad contents
+*
+* The get_*restore_pointer functions are used to provide a
+* physical restore address where the ROM code jumps while waking
+* up from MPU OFF/OSWR state.
Just curious: we dont have OSWR(open switch retention) in l-o unless I 
am mistaken.. we have cswr (closed switch retention).



+* The restore pointer is stored into the scratchpad.
+*/
scratchpad_contents.boot_config_ptr = 0x0;
if (cpu_is_omap3630())
scratchpad_contents.public_restore_ptr =
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index beeb682..12061fd 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -71,6 +71,13 @@
  * API functions
  */
 
+/*

+ * The get_*restore_pointer functions are used to provide a
+ * physical restore address where the ROM code jumps while waking
+ * up from MPU OFF/OSWR state.
+ * The restore pointer is stored into the scratchpad.
+ */
+

we need to cleanup comments such as those used below:


.text
 /* Function call to get the restore pointer for resume from OFF */


may be make it explicit what the difference for get_restore_pointer is etc..


 ENTRY(get_restore_pointer)
@@ -102,7 +109,7 @@ ENTRY(get_es3_restore_pointer_sz)
 /*
  * L2 cache needs to be toggled for stable OFF mode functionality on 3630.
  * This function sets up a flag that will allow for this toggling to take
- * place on 3630. Hopefully some version in the future maynot need this
+ * place on 3630. Hopefully some version in the future may not need this.
I think we should split the patch up for the comment cleanup - it is a 
little nuisance trying to review the actual functional change mixed with 
comment cleanups together..



  */
 ENTRY(enable_omap3630_toggle_l2_on_restore)
 stmfd   sp!, {lr} @ save registers on stack
@@ -144,34 +151,158 @@ ENTRY(save_secure_ram_context_sz)
.word   . - save_secure_ram_context
 
 /*

+ * ==
+ * == Idle entry point ==
+ * ==
+ */
+
+/*
  * Forces OMAP into idle state
  *
- * omap34xx_suspend() - This bit of code just executes the WFI
- * for normal idles.
+ * omap34xx_suspend() - This bit of code saves the CPU context if needed
+ * and executes the WFI instruction
since we are cleaning up, I might as well suggest - what condition do we 
save the CPU context might be worth mentioning. is this called during 
RET transition or OFF transition is not clear..

  *
- * 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.
+ * Notes:
+ * - this code gets copied to internal SRAM at boot.
+ * - when the OMAP wakes up it continues at different execution points
+ *   depending on the low power mode (non-OFF vs OFF modes),
+ *   cf. 'Resume path for xxx mode' comments.
Further, might as well help people and say that this is the call 
_omap_sram_idle actually takes ;)


Also might be a little indepth pseudo code overview of this complex code 
flow might help for code maintainers later on..


omap34xx_cpu_suspend
   |- if Context save not required - omap3_do_wfi - wfi
   \- if context save required: - save_context_wfi
  |

[PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

2010-12-16 Thread jean . pihet
From: Jean Pihet j-pi...@ti.com

- Reworked and simplified the execution paths for better
  readability and to avoid duplication of code,
- Added comments on the entry and exit points and the interaction
  with the ROM code for OFF mode restore,
- Reworked the existing comments for better readability.

Tested on N900 and Beagleboard with full RET and OFF modes,
using cpuidle and suspend.

Signed-off-by: Jean Pihet j-pi...@ti.com
---
 arch/arm/mach-omap2/control.c   |   10 +-
 arch/arm/mach-omap2/sleep34xx.S |  309 ++
 2 files changed, 188 insertions(+), 131 deletions(-)

diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 728f268..5cb7276 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -239,7 +239,15 @@ void omap3_save_scratchpad_contents(void)
struct omap3_scratchpad_prcm_block prcm_block_contents;
struct omap3_scratchpad_sdrc_block sdrc_block_contents;
 
-   /* Populate the Scratchpad contents */
+   /*
+* Populate the Scratchpad contents
+*
+* The get_*restore_pointer functions are used to get the resume
+*  function pointer to be called by the ROM code when back from WFI
+*  in OFF mode.
+* The restore pointer is stored into the scratchpad for later access
+*  by the ROM code.
+*/
scratchpad_contents.boot_config_ptr = 0x0;
if (cpu_is_omap3630())
scratchpad_contents.public_restore_ptr =
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 426af02..55ddd5a 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -70,6 +70,11 @@
  * API functions
  */
 
+/*
+ * The get_*restore_pointer functions are returning the resume
+ *  function pointer to be called by the ROM code when back from WFI
+ *  in OFF mode.
+ */
.text
 /* Function call to get the restore pointer for resume from OFF */
 ENTRY(get_restore_pointer)
@@ -101,7 +106,7 @@ ENTRY(get_es3_restore_pointer_sz)
 /*
  * L2 cache needs to be toggled for stable OFF mode functionality on 3630.
  * This function sets up a flag that will allow for this toggling to take
- * place on 3630. Hopefully some version in the future maynot need this
+ * place on 3630. Hopefully some version in the future maynot need this.
  */
 ENTRY(enable_omap3630_toggle_l2_on_restore)
 stmfd   sp!, {lr} @ save registers on stack
@@ -143,34 +148,156 @@ ENTRY(save_secure_ram_context_sz)
.word   . - save_secure_ram_context
 
 /*
+ * ==
+ * == Idle entry point ==
+ * ==
+ */
+
+/*
  * Forces OMAP into idle state
  *
- * omap34xx_suspend() - This bit of code just executes the WFI
- * for normal idles.
+ * omap34xx_suspend() - This bit of code saves the CPU context if needed
+ * and executes the WFI instruction
  *
- * 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.
+ * Notes:
+ * - this code gets copied to internal SRAM at boot.
+ * - when the OMAP wakes up it continues at different execution points
+ *   depending on the low power mode (non-OFF vs OFF modes),
+ *   cf. 'Resume path for xxx mode' comments.
  */
 ENTRY(omap34xx_cpu_suspend)
stmfd   sp!, {r0-r12, lr}   @ save registers on stack
 
-   /* 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
+   /*
+* r0 contains restore pointer in sdram
+* r1 contains information about saving context:
+*   0 - No context lost
+*   1 - Only L1 and logic lost
+*   2 - Only L2 lost
+*   3 - Both L1 and L2 lost
+*/
 
+   /* Save context only if required */
cmp r1, #0x0
-   /* If context save is required, do that and execute wfi */
-   bne save_context_wfi
+   beq omap3_do_wfi
+
+save_context_wfi:
+   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 from r1 */
+   cmp r1, #0x2@ Only L2 lost, no need to save context
+   beq clean_caches
+
+l1_logic_lost:
+   /* Store sp and spsr to SDRAM */
+   

RE: [PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

2010-12-16 Thread Santosh Shilimkar
 -Original Message-
 From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
 ow...@vger.kernel.org] On Behalf Of jean.pi...@newoldbits.com
 Sent: Thursday, December 16, 2010 11:21 PM
 To: linux-omap@vger.kernel.org
 Cc: khil...@deeprootsystems.com; linux-arm-ker...@lists.infradead.org;
 Jean Pihet
 Subject: [PATCH 5/7] OMAP3: rework of the ASM sleep code execution paths

 From: Jean Pihet j-pi...@ti.com

 - Reworked and simplified the execution paths for better
   readability and to avoid duplication of code,
 - Added comments on the entry and exit points and the interaction
   with the ROM code for OFF mode restore,
 - Reworked the existing comments for better readability.

 Tested on N900 and Beagleboard with full RET and OFF modes,
 using cpuidle and suspend.

 Signed-off-by: Jean Pihet j-pi...@ti.com
 ---
  arch/arm/mach-omap2/control.c   |   10 +-
  arch/arm/mach-omap2/sleep34xx.S |  309
++
 
  2 files changed, 188 insertions(+), 131 deletions(-)

 diff --git a/arch/arm/mach-omap2/control.c
b/arch/arm/mach-omap2/control.c
 index 728f268..5cb7276 100644
 --- a/arch/arm/mach-omap2/control.c
 +++ b/arch/arm/mach-omap2/control.c
 @@ -239,7 +239,15 @@ void omap3_save_scratchpad_contents(void)
   struct omap3_scratchpad_prcm_block prcm_block_contents;
   struct omap3_scratchpad_sdrc_block sdrc_block_contents;

 - /* Populate the Scratchpad contents */
 + /*
 +  * Populate the Scratchpad contents
 +  *
 +  * The get_*restore_pointer functions are used to get the resume
 +  *  function pointer to be called by the ROM code when back from
WFI
 +  *  in OFF mode.
Align your text here.
 +  * The restore pointer is stored into the scratchpad for later
 access
 +  *  by the ROM code.
The text needs to be reworked a bit.

The get_*restore_pointer functions are used to provide a physical
restore
address where ROM code jumps while waking up from MPU OFF/OSWR state.
The restore pointer is stored into the scratchpad

 +  */
   scratchpad_contents.boot_config_ptr = 0x0;
   if (cpu_is_omap3630())
   scratchpad_contents.public_restore_ptr =
 diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-
 omap2/sleep34xx.S
 index 426af02..55ddd5a 100644
 --- a/arch/arm/mach-omap2/sleep34xx.S
 +++ b/arch/arm/mach-omap2/sleep34xx.S
 @@ -70,6 +70,11 @@
   * API functions
   */

 +/*
 + * The get_*restore_pointer functions are returning the resume
 + *  function pointer to be called by the ROM code when back from WFI
 + *  in OFF mode.
Ditto
The get_*restore_pointer functions are used to provide a physical
restore
address where ROM code jumps while waking up from MPU OFF/OSWR state.
The restore pointer is stored into the scratchpad

 + */
   .text
  /* Function call to get the restore pointer for resume from OFF */
  ENTRY(get_restore_pointer)
 @@ -101,7 +106,7 @@ ENTRY(get_es3_restore_pointer_sz)
  /*
   * L2 cache needs to be toggled for stable OFF mode functionality on
3630.
   * This function sets up a flag that will allow for this toggling to
take
 - * place on 3630. Hopefully some version in the future maynot need this
 + * place on 3630. Hopefully some version in the future maynot need
this.
'maynot' . may not
   */
  ENTRY(enable_omap3630_toggle_l2_on_restore)
  stmfd   sp!, {lr} @ save registers on stack
 @@ -143,34 +148,156 @@ ENTRY(save_secure_ram_context_sz)
   .word   . - save_secure_ram_context

  /*
 + * ==
 + * == Idle entry point ==
 + * ==
 + */
 +
 +/*
   * Forces OMAP into idle state
   *
 - * omap34xx_suspend() - This bit of code just executes the WFI
 - * for normal idles.
 + * omap34xx_suspend() - This bit of code saves the CPU context if
needed
 + * and executes the WFI instruction
   *
 - * 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.
 + * Notes:
 + * - this code gets copied to internal SRAM at boot.
 + * - when the OMAP wakes up it continues at different execution points
 + *   depending on the low power mode (non-OFF vs OFF modes),
 + *   cf. 'Resume path for xxx mode' comments.
   */
  ENTRY(omap34xx_cpu_suspend)
   stmfd   sp!, {r0-r12, lr}   @ save registers on stack

 - /* 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
 + /*
 +  * r0 contains restore pointer in sdram
 +  * r1 contains information about saving context:
 +  *   0 - No context lost
 +  *   1 - Only L1 and logic lost
 +  *   2 - Only L2 lost
 +  *   3 - Both L1 and L2 lost