[PATCH 1/4] at91 : coding style fixes

[PATCH 1/4] at91 : coding style fixes

[Daniel Lezcano]

This patch is mindless and does only fix the line length.
The purpose is to facilitate the review of the next patches.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 arch/arm/mach-at91/pm.h |   31 +++++++++++++++++++++----------
 1 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index ce9a206..92d2223 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -20,14 +20,16 @@ static inline u32 sdram_selfrefresh_enable(void)
 	return saved_lpr;
 }
 
-#define sdram_selfrefresh_disable(saved_lpr)	at91_sys_write(AT91_SDRAMC_LPR, saved_lpr)
-#define wait_for_interrupt_enable()		asm volatile ("mcr p15, 0, %0, c7, c0, 4" \
-								: : "r" (0))
+#define sdram_selfrefresh_disable(saved_lpr) \
+	at91_sys_write(AT91_SDRAMC_LPR, saved_lpr)
+
+#define wait_for_interrupt_enable() \
+	asm volatile ("mcr p15, 0, %0, c7, c0, 4" \
+		      : : "r" (0))
 
 #elif defined(CONFIG_ARCH_AT91CAP9)
 #include <mach/at91cap9_ddrsdr.h>
 
-
 static inline u32 sdram_selfrefresh_enable(void)
 {
 	u32 saved_lpr, lpr;
@@ -35,12 +37,16 @@ static inline u32 sdram_selfrefresh_enable(void)
 	saved_lpr = at91_ramc_read(0, AT91_DDRSDRC_LPR);
 
 	lpr = saved_lpr & ~AT91_DDRSDRC_LPCB;
-	at91_ramc_write(0, AT91_DDRSDRC_LPR, lpr | AT91_DDRSDRC_LPCB_SELF_REFRESH);
+	at91_ramc_write(0, AT91_DDRSDRC_LPR, lpr |
+			AT91_DDRSDRC_LPCB_SELF_REFRESH);
 	return saved_lpr;
 }
 
-#define sdram_selfrefresh_disable(saved_lpr)	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr)
-#define wait_for_interrupt_enable()		cpu_do_idle()
+#define sdram_selfrefresh_disable(saved_lpr) \
+	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr)
+
+#define wait_for_interrupt_enable() \
+	cpu_do_idle()
 
 #elif defined(CONFIG_ARCH_AT91SAM9G45)
 #include <mach/at91sam9_ddrsdr.h>
@@ -77,6 +83,7 @@ static inline u32 sdram_selfrefresh_enable(void)
 		at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0); \
 		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1); \
 	} while (0)
+
 #define wait_for_interrupt_enable()		cpu_do_idle()
 
 #else
@@ -97,11 +104,15 @@ static inline u32 sdram_selfrefresh_enable(void)
 	saved_lpr = at91_ramc_read(0, AT91_SDRAMC_LPR);
 
 	lpr = saved_lpr & ~AT91_SDRAMC_LPCB;
-	at91_ramc_write(0, AT91_SDRAMC_LPR, lpr | AT91_SDRAMC_LPCB_SELF_REFRESH);
+	at91_ramc_write(0, AT91_SDRAMC_LPR, lpr |
+			AT91_SDRAMC_LPCB_SELF_REFRESH);
 	return saved_lpr;
 }
 
-#define sdram_selfrefresh_disable(saved_lpr)	at91_ramc_write(0, AT91_SDRAMC_LPR, saved_lpr)
-#define wait_for_interrupt_enable()		cpu_do_idle()
+#define sdram_selfrefresh_disable(saved_lpr) \
+	at91_ramc_write(0, AT91_SDRAMC_LPR, saved_lpr)
+
+#define wait_for_interrupt_enable() \
+	cpu_do_idle()
 
 #endif
-- 
1.7.5.4

[PATCH 2/4] at91 : declare header name

[Daniel Lezcano]

Add the header and define the macro to prevent multiple inclusion
like the others headers.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 arch/arm/mach-at91/pm.h |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 92d2223..325ef76 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -1,3 +1,16 @@
+/*
+ * AT91 Power Management
+ *
+ * Copyright (C) 2005 David Brownell
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#ifndef __ARCH_ARM_MACH_AT91_PM
+#define __ARCH_ARM_MACH_AT91_PM
+
 #ifdef CONFIG_ARCH_AT91RM9200
 #include <mach/at91rm9200_mc.h>
 
@@ -116,3 +129,5 @@ static inline u32 sdram_selfrefresh_enable(void)
 	cpu_do_idle()
 
 #endif
+
+#endif
-- 
1.7.5.4

[PATCH 3/4] at91 : remove wait_for_interrupt definition

[Daniel Lezcano]

All the "wait_for_interrupt" definition are aliases to cpu_do_idle.
Only the rm9200 has an asm routine to switch to wfi. But the cpu_do_idle
for this platform has exactly the same asm routine.

arch/arm/mm/proc-arm920.S
...
ENTRY(cpu_arm920_do_idle)
        mcr     p15, 0, r0, c7, c0, 4           @ Wait for interrupt
...

Then it is safe to invoke cpu_do_idle for this platform. As all the
wait_for_interrupts are definition for cpu_do_idle, let's remove it
and replace its invokation by cpu_do_idle.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 arch/arm/mach-at91/pm.c |    2 +-
 arch/arm/mach-at91/pm.h |   12 ------------
 2 files changed, 1 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 62ad955..acf32c7 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -272,7 +272,7 @@ static int at91_pm_enter(suspend_state_t state)
 					: /* no input */
 					: "r0");
 			saved_lpr = sdram_selfrefresh_enable();
-			wait_for_interrupt_enable();
+			cpu_do_idle();
 			sdram_selfrefresh_disable(saved_lpr);
 			break;
 
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 325ef76..624c99c 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -36,10 +36,6 @@ static inline u32 sdram_selfrefresh_enable(void)
 #define sdram_selfrefresh_disable(saved_lpr) \
 	at91_sys_write(AT91_SDRAMC_LPR, saved_lpr)
 
-#define wait_for_interrupt_enable() \
-	asm volatile ("mcr p15, 0, %0, c7, c0, 4" \
-		      : : "r" (0))
-
 #elif defined(CONFIG_ARCH_AT91CAP9)
 #include <mach/at91cap9_ddrsdr.h>
 
@@ -58,9 +54,6 @@ static inline u32 sdram_selfrefresh_enable(void)
 #define sdram_selfrefresh_disable(saved_lpr) \
 	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr)
 
-#define wait_for_interrupt_enable() \
-	cpu_do_idle()
-
 #elif defined(CONFIG_ARCH_AT91SAM9G45)
 #include <mach/at91sam9_ddrsdr.h>
 
@@ -97,8 +90,6 @@ static inline u32 sdram_selfrefresh_enable(void)
 		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1); \
 	} while (0)
 
-#define wait_for_interrupt_enable()		cpu_do_idle()
-
 #else
 #include <mach/at91sam9_sdramc.h>
 
@@ -125,9 +116,6 @@ static inline u32 sdram_selfrefresh_enable(void)
 #define sdram_selfrefresh_disable(saved_lpr) \
 	at91_ramc_write(0, AT91_SDRAMC_LPR, saved_lpr)
 
-#define wait_for_interrupt_enable() \
-	cpu_do_idle()
-
 #endif
 
 #endif
-- 
1.7.5.4

[PATCH 3/4] at91 : remove wait_for_interrupt definition

[Russell King - ARM Linux]

On Wed, Jan 25, 2012 at 12:56:07AM +0100, Daniel Lezcano wrote:
> All the "wait_for_interrupt" definition are aliases to cpu_do_idle.
> Only the rm9200 has an asm routine to switch to wfi. But the cpu_do_idle
> for this platform has exactly the same asm routine.
> 
> arch/arm/mm/proc-arm920.S
> ...
> ENTRY(cpu_arm920_do_idle)
>         mcr     p15, 0, r0, c7, c0, 4           @ Wait for interrupt
> ...
> 
> Then it is safe to invoke cpu_do_idle for this platform.

No it is not.

Please read Nicolas' post:

http://lists.arm.linux.org.uk/lurker/message/20120112.144129.827ae490.en.html

and think about what "DWB is needed before putting SDRAM into self-refresh
because any subsequent access to SDRAM will force it to resume from
self-refresh state" means.

Consider that if you _branch_ somewhere else, you _could_ cause a cache
line fetch, which will have to come from SDRAM.

>From Nicolas' post, it's pretty clear to me that the AT91RM9200 requires
carefully crafted assembly which can't be separated in this way to work,
which I mostly supplied in this mail:

http://lists.arm.linux.org.uk/lurker/message/20120109.144443.3626e5a6.en.html

[PATCH 3/4] at91 : remove wait_for_interrupt definition

[Daniel Lezcano]

On 01/25/2012 01:18 AM, Russell King - ARM Linux wrote:
> On Wed, Jan 25, 2012 at 12:56:07AM +0100, Daniel Lezcano wrote:
>> All the "wait_for_interrupt" definition are aliases to cpu_do_idle.
>> Only the rm9200 has an asm routine to switch to wfi. But the cpu_do_idle
>> for this platform has exactly the same asm routine.
>>
>> arch/arm/mm/proc-arm920.S
>> ...
>> ENTRY(cpu_arm920_do_idle)
>>          mcr     p15, 0, r0, c7, c0, 4           @ Wait for interrupt
>> ...
>>
>> Then it is safe to invoke cpu_do_idle for this platform.
>
> No it is not.
>
> Please read Nicolas' post:
>
> http://lists.arm.linux.org.uk/lurker/message/20120112.144129.827ae490.en.html
>
> and think about what "DWB is needed before putting SDRAM into self-refresh
> because any subsequent access to SDRAM will force it to resume from
> self-refresh state" means.
>
> Consider that if you _branch_ somewhere else, you _could_ cause a cache
> line fetch, which will have to come from SDRAM.

Oh, right. I am not familiar with this part, thanks for the clarification.

>  From Nicolas' post, it's pretty clear to me that the AT91RM9200 requires
> carefully crafted assembly which can't be separated in this way to work,
> which I mostly supplied in this mail:
>
> http://lists.arm.linux.org.uk/lurker/message/20120109.144443.3626e5a6.en.html

Ok, this is what does the patch 4/4, it changes the self-refresh and wfi 
into an asm routine where cpu_do_idle call is removed. Can I consider by 
folding 3/4 and 4/4 ? So the buggy cpu_do_idle change won't appear...

Thanks
   -- Daniel

-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 3/4] at91 : remove wait_for_interrupt definition

[Russell King - ARM Linux]

On Wed, Jan 25, 2012 at 03:39:49PM +0100, Daniel Lezcano wrote:
> On 01/25/2012 01:18 AM, Russell King - ARM Linux wrote:
>> On Wed, Jan 25, 2012 at 12:56:07AM +0100, Daniel Lezcano wrote:
>>> All the "wait_for_interrupt" definition are aliases to cpu_do_idle.
>>> Only the rm9200 has an asm routine to switch to wfi. But the cpu_do_idle
>>> for this platform has exactly the same asm routine.
>>>
>>> arch/arm/mm/proc-arm920.S
>>> ...
>>> ENTRY(cpu_arm920_do_idle)
>>>          mcr     p15, 0, r0, c7, c0, 4           @ Wait for interrupt
>>> ...
>>>
>>> Then it is safe to invoke cpu_do_idle for this platform.
>>
>> No it is not.
>>
>> Please read Nicolas' post:
>>
>> http://lists.arm.linux.org.uk/lurker/message/20120112.144129.827ae490.en.html
>>
>> and think about what "DWB is needed before putting SDRAM into self-refresh
>> because any subsequent access to SDRAM will force it to resume from
>> self-refresh state" means.
>>
>> Consider that if you _branch_ somewhere else, you _could_ cause a cache
>> line fetch, which will have to come from SDRAM.
>
> Oh, right. I am not familiar with this part, thanks for the clarification.
>
>>  From Nicolas' post, it's pretty clear to me that the AT91RM9200 requires
>> carefully crafted assembly which can't be separated in this way to work,
>> which I mostly supplied in this mail:
>>
>> http://lists.arm.linux.org.uk/lurker/message/20120109.144443.3626e5a6.en.html
>
> Ok, this is what does the patch 4/4, it changes the self-refresh and wfi  
> into an asm routine where cpu_do_idle call is removed. Can I consider by  
> folding 3/4 and 4/4 ? So the buggy cpu_do_idle change won't appear...

So I see your patch is in arm-soc now, inspite of my note that it's
probably broken.

If it's been tested, then all the asm() stuff in arch/arm/mach-at91/pm.c
along with the sdram crap can be removed.

[PATCH 3/4] at91 : remove wait_for_interrupt definition

[Daniel Lezcano]

On 02/27/2012 01:50 PM, Russell King - ARM Linux wrote:
> On Wed, Jan 25, 2012 at 03:39:49PM +0100, Daniel Lezcano wrote:
>> On 01/25/2012 01:18 AM, Russell King - ARM Linux wrote:
>>> On Wed, Jan 25, 2012 at 12:56:07AM +0100, Daniel Lezcano wrote:
>>>> All the "wait_for_interrupt" definition are aliases to cpu_do_idle.
>>>> Only the rm9200 has an asm routine to switch to wfi. But the cpu_do_idle
>>>> for this platform has exactly the same asm routine.
>>>>
>>>> arch/arm/mm/proc-arm920.S
>>>> ...
>>>> ENTRY(cpu_arm920_do_idle)
>>>>           mcr     p15, 0, r0, c7, c0, 4           @ Wait for interrupt
>>>> ...
>>>>
>>>> Then it is safe to invoke cpu_do_idle for this platform.
>>>
>>> No it is not.
>>>
>>> Please read Nicolas' post:
>>>
>>> http://lists.arm.linux.org.uk/lurker/message/20120112.144129.827ae490.en.html
>>>
>>> and think about what "DWB is needed before putting SDRAM into self-refresh
>>> because any subsequent access to SDRAM will force it to resume from
>>> self-refresh state" means.
>>>
>>> Consider that if you _branch_ somewhere else, you _could_ cause a cache
>>> line fetch, which will have to come from SDRAM.
>>
>> Oh, right. I am not familiar with this part, thanks for the clarification.
>>
>>>    From Nicolas' post, it's pretty clear to me that the AT91RM9200 requires
>>> carefully crafted assembly which can't be separated in this way to work,
>>> which I mostly supplied in this mail:
>>>
>>> http://lists.arm.linux.org.uk/lurker/message/20120109.144443.3626e5a6.en.html
>>
>> Ok, this is what does the patch 4/4, it changes the self-refresh and wfi
>> into an asm routine where cpu_do_idle call is removed. Can I consider by
>> folding 3/4 and 4/4 ? So the buggy cpu_do_idle change won't appear...
>
> So I see your patch is in arm-soc now, inspite of my note that it's
> probably broken.
>
> If it's been tested, then all the asm() stuff in arch/arm/mach-at91/pm.c
> along with the sdram crap can be removed.

Yes, this is what the patch 4/4 does right after, also removing the 
cpu_do_idle. There no more asm routine in pm.c now.

Thanks
   -- Daniel

-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 3/4] at91 : remove wait_for_interrupt definition

[Rob Lee]

Hey Daniel and at91 people,

Just FYI, in my latest cpuidle consolidation patchset I also included
a patch to make the consolidation changes to the at91 cpuidle.c.  I
realize other work is going on with at91 cpuidle but I decided to go
ahead and include it, for reference if nothing else.  For another
cpuidle consolidation patch coming soon, please let me know if you'd
rather me not submit the changes to the at91 cpuidle.c.

Best Regards,
Rob

[PATCH 4/4] at91 : implement the standby function for pm/cpuidle

[Daniel Lezcano]

This patch groups the self-refresh on/cpu_do_idle/self-refresh off into
a single 'standby' function.

The standby routine for rm9200 has been turned into an asm routine to have
a better control of the self refresh and to prevent a memory access when
running this code.

Draining the write buffer is done automatically when switching for the self
refresh on sam9, so the instruction is added to the rm9200 only.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 arch/arm/mach-at91/cpuidle.c |   11 ++-----
 arch/arm/mach-at91/pm.c      |   12 +-------
 arch/arm/mach-at91/pm.h      |   62 ++++++++++++++++++++++++-----------------
 3 files changed, 40 insertions(+), 45 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index a851e6c..555d956 100644
--- a/arch/arm/mach-at91/cpuidle.c
+++ b/arch/arm/mach-at91/cpuidle.c
@@ -39,20 +39,15 @@ static int at91_enter_idle(struct cpuidle_device *dev,
 {
 	struct timeval before, after;
 	int idle_time;
-	u32 saved_lpr;
 
 	local_irq_disable();
 	do_gettimeofday(&before);
 	if (index == 0)
 		/* Wait for interrupt state */
 		cpu_do_idle();
-	else if (index == 1) {
-		asm("b 1f; .align 5; 1:");
-		asm("mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */
-		saved_lpr = sdram_selfrefresh_enable();
-		cpu_do_idle();
-		sdram_selfrefresh_disable(saved_lpr);
-	}
+	else if (index == 1)
+		at91_standby();
+
 	do_gettimeofday(&after);
 	local_irq_enable();
 	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index acf32c7..94acc12 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -208,7 +208,6 @@ extern u32 at91_slow_clock_sz;
 
 static int at91_pm_enter(suspend_state_t state)
 {
-	u32 saved_lpr;
 	at91_gpio_suspend();
 	at91_irq_suspend();
 
@@ -264,16 +263,7 @@ static int at91_pm_enter(suspend_state_t state)
 			 * For ARM 926 based chips, this requirement is weaker
 			 * as at91sam9 can access a RAM in self-refresh mode.
 			 */
-			asm volatile (	"mov r0, #0\n\t"
-					"b 1f\n\t"
-					".align 5\n\t"
-					"1: mcr p15, 0, r0, c7, c10, 4\n\t"
-					: /* no output */
-					: /* no input */
-					: "r0");
-			saved_lpr = sdram_selfrefresh_enable();
-			cpu_do_idle();
-			sdram_selfrefresh_disable(saved_lpr);
+			at91_standby();
 			break;
 
 		case PM_SUSPEND_ON:
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 624c99c..3892879 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -24,22 +24,31 @@
  * still in self-refresh is "not recommended", but seems to work.
  */
 
-static inline u32 sdram_selfrefresh_enable(void)
+static inline void at91rm9200_standby(void)
 {
-	u32 saved_lpr = at91_sys_read(AT91_SDRAMC_LPR);
 
-	at91_sys_write(AT91_SDRAMC_LPR, 0);
-	at91_sys_write(AT91_SDRAMC_SRR, 1);
-	return saved_lpr;
+	u32 lpr = at91_sys_read(AT91_SDRAMC_LPR);
+
+	asm volatile(
+		"b    1f\n\t"
+		".align    5\n\t"
+		"1:  mcr    p15, 0, %0, c7, c10, 4\n\t"
+		"    str    %0, [%1, %2]\n\t"
+		"    str    %3, [%1, %4]\n\t"
+		"    mcr    p15, 0, %0, c7, c0, 4\n\t"
+		"    str    %5, [%1, %2]"
+		:
+		: "r" (0), "r" (AT91_BASE_SYS), "r" (AT91_SDRAMC_LPR),
+		  "r" (1), "r" (AT91_SDRAMC_SRR),
+		  "r" (lpr));
 }
 
-#define sdram_selfrefresh_disable(saved_lpr) \
-	at91_sys_write(AT91_SDRAMC_LPR, saved_lpr)
+#define at91_standby at91rm9200_standby
 
 #elif defined(CONFIG_ARCH_AT91CAP9)
 #include <mach/at91cap9_ddrsdr.h>
 
-static inline u32 sdram_selfrefresh_enable(void)
+static inline void at91cap9_standby(void)
 {
 	u32 saved_lpr, lpr;
 
@@ -48,11 +57,13 @@ static inline u32 sdram_selfrefresh_enable(void)
 	lpr = saved_lpr & ~AT91_DDRSDRC_LPCB;
 	at91_ramc_write(0, AT91_DDRSDRC_LPR, lpr |
 			AT91_DDRSDRC_LPCB_SELF_REFRESH);
-	return saved_lpr;
+
+	cpu_do_idle();
+
+	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr);
 }
 
-#define sdram_selfrefresh_disable(saved_lpr) \
-	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr)
+#define at91_standby at91cap9_standby
 
 #elif defined(CONFIG_ARCH_AT91SAM9G45)
 #include <mach/at91sam9_ddrsdr.h>
@@ -60,14 +71,12 @@ static inline u32 sdram_selfrefresh_enable(void)
 /* We manage both DDRAM/SDRAM controllers, we need more than one value to
  * remember.
  */
-static u32 saved_lpr1;
-
-static inline u32 sdram_selfrefresh_enable(void)
+static inline void at91sam9g45_standby(void)
 {
-	/* Those tow values allow us to delay self-refresh activation
+	/* Those two values allow us to delay self-refresh activation
 	 * to the maximum. */
 	u32 lpr0, lpr1;
-	u32 saved_lpr0;
+	u32 saved_lpr0, saved_lpr1;
 
 	saved_lpr1 = at91_ramc_read(1, AT91_DDRSDRC_LPR);
 	lpr1 = saved_lpr1 & ~AT91_DDRSDRC_LPCB;
@@ -81,14 +90,13 @@ static inline u32 sdram_selfrefresh_enable(void)
 	at91_ramc_write(0, AT91_DDRSDRC_LPR, lpr0);
 	at91_ramc_write(1, AT91_DDRSDRC_LPR, lpr1);
 
-	return saved_lpr0;
+	cpu_do_idle();
+
+	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0);
+	at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1);
 }
 
-#define sdram_selfrefresh_disable(saved_lpr0)	\
-	do { \
-		at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr0); \
-		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1); \
-	} while (0)
+#define at91_standby at91sam9g45_standby
 
 #else
 #include <mach/at91sam9_sdramc.h>
@@ -101,7 +109,7 @@ static inline u32 sdram_selfrefresh_enable(void)
 #warning Assuming EB1 SDRAM controller is *NOT* used
 #endif
 
-static inline u32 sdram_selfrefresh_enable(void)
+static inline void at91sam9_standby(void)
 {
 	u32 saved_lpr, lpr;
 
@@ -110,11 +118,13 @@ static inline u32 sdram_selfrefresh_enable(void)
 	lpr = saved_lpr & ~AT91_SDRAMC_LPCB;
 	at91_ramc_write(0, AT91_SDRAMC_LPR, lpr |
 			AT91_SDRAMC_LPCB_SELF_REFRESH);
-	return saved_lpr;
+
+	cpu_do_idle();
+
+	at91_ramc_write(0, AT91_SDRAMC_LPR, saved_lpr);
 }
 
-#define sdram_selfrefresh_disable(saved_lpr) \
-	at91_ramc_write(0, AT91_SDRAMC_LPR, saved_lpr)
+#define at91_standby at91sam9_standby
 
 #endif
 
-- 
1.7.5.4

at91: pm.h cleanup (was: [PATCH 1/4] at91 : coding style fixes)

[Nicolas Ferre]

Daniel,

I have rebased your patch series on top of:

- at91 late device/board patches that are planned for 3.3
- at91-fixes branch (should go also in 3.3)
* rmk/for-next branch (with a merge conflict resolved)
* the removal of CAP9 SoC family

You can find the resulting code in our git tree:

git://github.com/at91linux/linux-at91.git at91-pm_cleanup

I hope that I will be able to send this branch to arm-soc soon with a
minimum of future rebase (when the two first series cited above will be
in Linus' tree actually).

FYI, the four patch series that will be the base of all our 3.4 work is
also stored in a branch: at91-3.4-base

Thanks, best regards,
-- 
Nicolas Ferre

at91: pm.h cleanup (was: [PATCH 1/4] at91 : coding style fixes)

[Russell King - ARM Linux]

On Thu, Jan 26, 2012 at 05:18:21PM +0100, Nicolas Ferre wrote:
> Daniel,
> 
> I have rebased your patch series on top of:
> 
> - at91 late device/board patches that are planned for 3.3
> - at91-fixes branch (should go also in 3.3)
> * rmk/for-next branch (with a merge conflict resolved)
> * the removal of CAP9 SoC family
> 
> You can find the resulting code in our git tree:
> 
> git://github.com/at91linux/linux-at91.git at91-pm_cleanup
> 
> I hope that I will be able to send this branch to arm-soc soon with a
> minimum of future rebase (when the two first series cited above will be
> in Linus' tree actually).

No you won't, not if you're including rmk/for-next in it.

Take a moment to think about that: rmk/for-next is NOT a topic branch.
It is purely a branch published for the sake of SFR.  It gets torn down
and regenerated regularly.  You can't base work off it.  It's all explained
here:

	http://www.arm.linux.org.uk/developer/git-arm.php

at91: pm.h cleanup (was: [PATCH 1/4] at91 : coding style fixes)

[Russell King - ARM Linux]

On Thu, Jan 26, 2012 at 08:33:50PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 26, 2012 at 05:18:21PM +0100, Nicolas Ferre wrote:
> > Daniel,
> > 
> > I have rebased your patch series on top of:
> > 
> > - at91 late device/board patches that are planned for 3.3
> > - at91-fixes branch (should go also in 3.3)
> > * rmk/for-next branch (with a merge conflict resolved)
> > * the removal of CAP9 SoC family
> > 
> > You can find the resulting code in our git tree:
> > 
> > git://github.com/at91linux/linux-at91.git at91-pm_cleanup
> > 
> > I hope that I will be able to send this branch to arm-soc soon with a
> > minimum of future rebase (when the two first series cited above will be
> > in Linus' tree actually).
> 
> No you won't, not if you're including rmk/for-next in it.
> 
> Take a moment to think about that: rmk/for-next is NOT a topic branch.
> It is purely a branch published for the sake of SFR.  It gets torn down
> and regenerated regularly.  You can't base work off it.  It's all explained
> here:
> 
> 	http://www.arm.linux.org.uk/developer/git-arm.php

As a follow-up to this, your action has put in jeopardy two things:

1. The ability for me to publish patches in my git tree for people to be
   able to test large patch sets.

2. The ability to publish patches which are ready for mainline but have
   not yet had all the acks etc received.

The official position on the publication of git commits is that once
they're published, they can't be changed - with the exception of the
linux-next tree.

What that means is that if I followed that rule, I would not publish very
many changes until the last minute for the simple reason that people in
the ARM community absolutely suck to the back teeth giving acks and so
forth to large patch sets.  Example: had I followed that guidance, the
restart changes would not have gone in during the last merge window.

So, either *EVERYONE* understands how I run my tree and they DO NOT
EVER include any branch into their own tree without FIRST TALKING TO
ME, or I withdraw my tree from public access.  It's that simple.  It's
not something ANYONE can make a mistake over.  You make a mistake and
it causes BIG PROBLEMS.

SFR _has_ noticed and _is_ complaining about this.  It's only time
before it gets noticed elsewhere.

So, I've withdrawn the highly unstable devel-3.3 branch from public
view as of NOW.  I'm going to remove anything in for-next which isn't
100% ready.  That includes the mach-types update, because I intend
to redo it at some later time.

Congratulations.  And thanks for causing this situation through your
lack of due dilligence.

at91: pm.h cleanup

[Nicolas Ferre]

On 01/27/2012 12:34 AM, Russell King - ARM Linux :
> On Thu, Jan 26, 2012 at 08:33:50PM +0000, Russell King - ARM Linux wrote:
>> On Thu, Jan 26, 2012 at 05:18:21PM +0100, Nicolas Ferre wrote:
>>> Daniel,
>>>
>>> I have rebased your patch series on top of:
>>>
>>> - at91 late device/board patches that are planned for 3.3
>>> - at91-fixes branch (should go also in 3.3)
>>> * rmk/for-next branch (with a merge conflict resolved)
>>> * the removal of CAP9 SoC family
>>>
>>> You can find the resulting code in our git tree:
>>>
>>> git://github.com/at91linux/linux-at91.git at91-pm_cleanup
>>>
>>> I hope that I will be able to send this branch to arm-soc soon with a
>>> minimum of future rebase (when the two first series cited above will be
>>> in Linus' tree actually).
>>
>> No you won't, not if you're including rmk/for-next in it.
>>
>> Take a moment to think about that: rmk/for-next is NOT a topic branch.
>> It is purely a branch published for the sake of SFR.  It gets torn down
>> and regenerated regularly.  You can't base work off it.  It's all explained
>> here:
>>
>> 	http://www.arm.linux.org.uk/developer/git-arm.php
> 
> As a follow-up to this, your action has put in jeopardy two things:
> 
> 1. The ability for me to publish patches in my git tree for people to be
>    able to test large patch sets.
> 
> 2. The ability to publish patches which are ready for mainline but have
>    not yet had all the acks etc received.
> 
> The official position on the publication of git commits is that once
> they're published, they can't be changed - with the exception of the
> linux-next tree.
> 
> What that means is that if I followed that rule, I would not publish very
> many changes until the last minute for the simple reason that people in
> the ARM community absolutely suck to the back teeth giving acks and so
> forth to large patch sets.  Example: had I followed that guidance, the
> restart changes would not have gone in during the last merge window.
> 
> So, either *EVERYONE* understands how I run my tree and they DO NOT
> EVER include any branch into their own tree without FIRST TALKING TO
> ME, or I withdraw my tree from public access.  It's that simple.  It's
> not something ANYONE can make a mistake over.  You make a mistake and
> it causes BIG PROBLEMS.
> 
> SFR _has_ noticed and _is_ complaining about this.  It's only time
> before it gets noticed elsewhere.
> 
> So, I've withdrawn the highly unstable devel-3.3 branch from public
> view as of NOW.  I'm going to remove anything in for-next which isn't
> 100% ready.  That includes the mach-types update, because I intend
> to redo it at some later time.
> 
> Congratulations.  And thanks for causing this situation through your
> lack of due dilligence.

Russell,

So many problems on my shoulders!

I was just trying to anticipate the git branch that you are putting in
place for 3.4 development (your have now published one with "for-armsoc"
if I am following correctly the plans).

I was mentioning in my email that this at91-pm_cleanup branch may be
rebased and I *never* thought of making a pull request out of it.

My mistake was to push it to our at91-next which ends up being included
in linux-next. Yes it was dumb and I will ask Stephen to remove the
at91-next branch from his pull list.
AT91 will rely on arm-soc to get exposure in linux-next which is simpler
and less prone to errors.

Best regards,
-- 
Nicolas Ferre

[PATCH 0/4] at91 : cleanup pm.h

[Daniel Lezcano]

This patchset makes some cleanup and fix the rm9200 code by replacing
the standby routine by a asm routine where we have a better control.

This patchset has been tested on a sam9263, compiled on the other
platform but not tested because I don't have the hardware. 

This is my first attempt for an asm arm routine ...

Daniel Lezcano (4):
  at91 : coding style fixes
  at91 : declare header name
  at91 : remove wait_for_interrupt definition
  at91 : implement the standby function for pm/cpuidle

 arch/arm/mach-at91/cpuidle.c |   11 +----
 arch/arm/mach-at91/pm.c      |   12 +-----
 arch/arm/mach-at91/pm.h      |   86 +++++++++++++++++++++++++++---------------
 3 files changed, 59 insertions(+), 50 deletions(-)

-- 
1.7.5.4

[PATCH 3/4] at91 : remove wait_for_interrupt definition

[Daniel Lezcano]

All the "wait_for_interrupt" definition are aliases to cpu_do_idle.
Only the rm9200 has an asm routine to switch to wfi. But the cpu_do_idle
for this platform has exactly the same asm routine.

arch/arm/mm/proc-arm920.S
...
ENTRY(cpu_arm920_do_idle)
        mcr     p15, 0, r0, c7, c0, 4           @ Wait for interrupt
...

Then it is safe to invoke cpu_do_idle for this platform. As all the
wait_for_interrupts are definition for cpu_do_idle, let's remove it
and replace its invokation by cpu_do_idle.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 arch/arm/mach-at91/pm.c |    2 +-
 arch/arm/mach-at91/pm.h |   12 ------------
 2 files changed, 1 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 62ad955..acf32c7 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -272,7 +272,7 @@ static int at91_pm_enter(suspend_state_t state)
 					: /* no input */
 					: "r0");
 			saved_lpr = sdram_selfrefresh_enable();
-			wait_for_interrupt_enable();
+			cpu_do_idle();
 			sdram_selfrefresh_disable(saved_lpr);
 			break;
 
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 325ef76..624c99c 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -36,10 +36,6 @@ static inline u32 sdram_selfrefresh_enable(void)
 #define sdram_selfrefresh_disable(saved_lpr) \
 	at91_sys_write(AT91_SDRAMC_LPR, saved_lpr)
 
-#define wait_for_interrupt_enable() \
-	asm volatile ("mcr p15, 0, %0, c7, c0, 4" \
-		      : : "r" (0))
-
 #elif defined(CONFIG_ARCH_AT91CAP9)
 #include <mach/at91cap9_ddrsdr.h>
 
@@ -58,9 +54,6 @@ static inline u32 sdram_selfrefresh_enable(void)
 #define sdram_selfrefresh_disable(saved_lpr) \
 	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr)
 
-#define wait_for_interrupt_enable() \
-	cpu_do_idle()
-
 #elif defined(CONFIG_ARCH_AT91SAM9G45)
 #include <mach/at91sam9_ddrsdr.h>
 
@@ -97,8 +90,6 @@ static inline u32 sdram_selfrefresh_enable(void)
 		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1); \
 	} while (0)
 
-#define wait_for_interrupt_enable()		cpu_do_idle()
-
 #else
 #include <mach/at91sam9_sdramc.h>
 
@@ -125,9 +116,6 @@ static inline u32 sdram_selfrefresh_enable(void)
 #define sdram_selfrefresh_disable(saved_lpr) \
 	at91_ramc_write(0, AT91_SDRAMC_LPR, saved_lpr)
 
-#define wait_for_interrupt_enable() \
-	cpu_do_idle()
-
 #endif
 
 #endif
-- 
1.7.5.4

[PATCH 3/4] at91 : remove wait_for_interrupt definition

[Ryan Mallon]

On 18/01/12 10:40, Daniel Lezcano wrote:

> All the "wait_for_interrupt" definition are aliases to cpu_do_idle.
> Only the rm9200 has an asm routine to switch to wfi. But the cpu_do_idle
> for this platform has exactly the same asm routine.
> 
> arch/arm/mm/proc-arm920.S
> ...
> ENTRY(cpu_arm920_do_idle)
>         mcr     p15, 0, r0, c7, c0, 4           @ Wait for interrupt
> ...
> 
> Then it is safe to invoke cpu_do_idle for this platform. As all the
> wait_for_interrupts are definition for cpu_do_idle, let's remove it
> and replace its invokation by cpu_do_idle.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>


This patch looks like an improvement. Though, because you are removing
all of the definitions of wait_for_interrupt_enable in this patch, it
doesn't seem worth fixing the coding style of them in patch 1 of this
series.

Reviewed-by: Ryan Mallon <rmallon@gmail.com>

> ---
>  arch/arm/mach-at91/pm.c |    2 +-
>  arch/arm/mach-at91/pm.h |   12 ------------
>  2 files changed, 1 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 62ad955..acf32c7 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -272,7 +272,7 @@ static int at91_pm_enter(suspend_state_t state)
>  					: /* no input */
>  					: "r0");
>  			saved_lpr = sdram_selfrefresh_enable();
> -			wait_for_interrupt_enable();
> +			cpu_do_idle();
>  			sdram_selfrefresh_disable(saved_lpr);
>  			break;
>  
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index 325ef76..624c99c 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -36,10 +36,6 @@ static inline u32 sdram_selfrefresh_enable(void)
>  #define sdram_selfrefresh_disable(saved_lpr) \
>  	at91_sys_write(AT91_SDRAMC_LPR, saved_lpr)
>  
> -#define wait_for_interrupt_enable() \
> -	asm volatile ("mcr p15, 0, %0, c7, c0, 4" \
> -		      : : "r" (0))
> -
>  #elif defined(CONFIG_ARCH_AT91CAP9)
>  #include <mach/at91cap9_ddrsdr.h>
>  
> @@ -58,9 +54,6 @@ static inline u32 sdram_selfrefresh_enable(void)
>  #define sdram_selfrefresh_disable(saved_lpr) \
>  	at91_ramc_write(0, AT91_DDRSDRC_LPR, saved_lpr)
>  
> -#define wait_for_interrupt_enable() \
> -	cpu_do_idle()
> -
>  #elif defined(CONFIG_ARCH_AT91SAM9G45)
>  #include <mach/at91sam9_ddrsdr.h>
>  
> @@ -97,8 +90,6 @@ static inline u32 sdram_selfrefresh_enable(void)
>  		at91_ramc_write(1, AT91_DDRSDRC_LPR, saved_lpr1); \
>  	} while (0)
>  
> -#define wait_for_interrupt_enable()		cpu_do_idle()
> -
>  #else
>  #include <mach/at91sam9_sdramc.h>
>  
> @@ -125,9 +116,6 @@ static inline u32 sdram_selfrefresh_enable(void)
>  #define sdram_selfrefresh_disable(saved_lpr) \
>  	at91_ramc_write(0, AT91_SDRAMC_LPR, saved_lpr)
>  
> -#define wait_for_interrupt_enable() \
> -	cpu_do_idle()
> -
>  #endif
>  
>  #endif