[PATCH 2/2] AT91: pm: make sure that r0 is 0 when dealing with cache operations

[PATCH 2/2] AT91: pm: make sure that r0 is 0 when dealing with cache operations

[Russell King - ARM Linux]

On Fri, Oct 22, 2010 at 07:29:00PM +0200, Nicolas Ferre wrote:
> When using CP15 cache operations (c7), we make sure that Rd (r0)
> is actually 0 as ARM 926 TRM is saying.

Err, no.  From the GCC manual:

|    Note that even a volatile `asm' instruction can be moved in ways
| that appear insignificant to the compiler, such as across jump
| instructions.  You can't expect a sequence of volatile `asm'
| instructions to remain perfectly consecutive.  If you want consecutive
| output, use a single `asm'.  Also, GCC will perform some optimizations
| across a volatile `asm' instruction; GCC does not "forget everything"
| when it encounters a volatile `asm' instruction the way some other
| compilers do.

So:

	asm volatile("foo");
	asm volatile("bar");

is not guaranteed to produce the following uninterrupted code sequence:

	foo
	bar

The compiler is free to insert other instructions inbetween these two
asm statements.

It's also not permitted to modify registers without telling gcc that
they've been modified.

> +			asm("mov r0, #0");			/* clear r0 for CP15 accesses */
>  			asm("b 1f; .align 5; 1:");
>  			asm("mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */

That means the above is completely unacceptable.  It should be:

	asm("mov r0, #0;"
	"b 1f;"
	".align 5;"
	"1: mcr p15, 0, r0, c7, c10, 4" : : : "r0");



>  			saved_lpr = sdram_selfrefresh_enable();
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index 2c4424b..be081c9 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -21,7 +21,7 @@ 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("mcr p15, 0, r0, c7, c0, 4")
> +#define wait_for_interrupt_enable()		asm("mcr p15, 0, r0, c7, c0, 4") /* r0 is 0 here */

No, r0 is not guaranteed to be zero here.  Use dsb() here instead which
gets it right.


>  #elif defined(CONFIG_ARCH_AT91CAP9)
>  #include <mach/at91cap9_ddrsdr.h>
> diff --git a/arch/arm/mach-at91/pm_slowclock.S b/arch/arm/mach-at91/pm_slowclock.S
> index b6b00a1..f7922a4 100644
> --- a/arch/arm/mach-at91/pm_slowclock.S
> +++ b/arch/arm/mach-at91/pm_slowclock.S
> @@ -124,6 +124,7 @@ ENTRY(at91_slow_clock)
>  	ldr	r5, .at91_va_base_ramc1
>  
>  	/* Drain write buffer */
> +	mov	r0, #0
>  	mcr	p15, 0, r0, c7, c10, 4

This one being in asm code is fine.

[PATCH 1/2] AT91: pm: use plain cpu_do_idle() for "wait for interrupt"

[Nicolas Ferre]

For power management at91_pm_enter() routine, use the cpu_do_idle() for a
rock solid "wait for interrupt" implementation.
For AT91SAM9 ARM 926 based chips, we can exceed the cache line length as
we can access RAM even while in self-refresh mode.
We keep plain access to CP15 for at91rm9200 as this feature is not
available: instructions have to be in a single cache line.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 arch/arm/mach-at91/pm.c |    6 ++++--
 arch/arm/mach-at91/pm.h |    4 ++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 6156689..87a31ba 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -258,16 +258,18 @@ static int at91_pm_enter(suspend_state_t state)
 			 * NOTE: the Wait-for-Interrupt instruction needs to be
 			 * in icache so no SDRAM accesses are needed until the
 			 * wakeup IRQ occurs and self-refresh is terminated.
+			 * For ARM 926 based chips, this requirement is weaker
+			 * as at91sam9 can access a RAM in self-refresh mode.
 			 */
 			asm("b 1f; .align 5; 1:");
 			asm("mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */
 			saved_lpr = sdram_selfrefresh_enable();
-			asm("mcr p15, 0, r0, c7, c0, 4");	/* wait for interrupt */
+			wait_for_interrupt_enable();
 			sdram_selfrefresh_disable(saved_lpr);
 			break;
 
 		case PM_SUSPEND_ON:
-			asm("mcr p15, 0, r0, c7, c0, 4");	/* wait for interrupt */
+			cpu_do_idle();
 			break;
 
 		default:
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 8c87d0c..2c4424b 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -21,6 +21,7 @@ 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("mcr p15, 0, r0, c7, c0, 4")
 
 #elif defined(CONFIG_ARCH_AT91CAP9)
 #include <mach/at91cap9_ddrsdr.h>
@@ -38,6 +39,7 @@ 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>
@@ -74,6 +76,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
 #include <mach/at91sam9_sdramc.h>
@@ -98,5 +101,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
-- 
1.7.3

[PATCH v3] at91/atmel-mci: inclusion of sd/mmc driver in at91sam9g45 chip and board

[Nicolas Ferre]

This adds the support of atmel-mci sd/mmc driver in at91sam9g45 devices and
board files. This also configures the DMA controller slave interface for
at_hdmac dmaengine driver.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
v3: remove the gpio setting to "-1": gpiolib is corectly dealing with "0" value
using gpio_request() function.

 arch/arm/mach-at91/at91sam9g45_devices.c |  165 ++++++++++++++++++++++++++++++
 arch/arm/mach-at91/board-sam9m10g45ek.c  |   24 +++++
 drivers/mmc/host/Kconfig                 |    2 +-
 3 files changed, 190 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-at91/at91sam9g45_devices.c b/arch/arm/mach-at91/at91sam9g45_devices.c
index 1276bab..1e8f275 100644
--- a/arch/arm/mach-at91/at91sam9g45_devices.c
+++ b/arch/arm/mach-at91/at91sam9g45_devices.c
@@ -15,6 +15,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
 #include <linux/i2c-gpio.h>
+#include <linux/atmel-mci.h>
 
 #include <linux/fb.h>
 #include <video/atmel_lcdc.h>
@@ -25,6 +26,7 @@
 #include <mach/at91sam9g45_matrix.h>
 #include <mach/at91sam9_smc.h>
 #include <mach/at_hdmac.h>
+#include <mach/atmel-mci.h>
 
 #include "generic.h"
 
@@ -350,6 +352,169 @@ void __init at91_add_device_eth(struct at91_eth_data *data) {}
 
 
 /* --------------------------------------------------------------------
+ *  MMC / SD
+ * -------------------------------------------------------------------- */
+
+#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
+static u64 mmc_dmamask = DMA_BIT_MASK(32);
+static struct mci_platform_data mmc0_data, mmc1_data;
+
+static struct resource mmc0_resources[] = {
+	[0] = {
+		.start	= AT91SAM9G45_BASE_MCI0,
+		.end	= AT91SAM9G45_BASE_MCI0 + SZ_16K - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= AT91SAM9G45_ID_MCI0,
+		.end	= AT91SAM9G45_ID_MCI0,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device at91sam9g45_mmc0_device = {
+	.name		= "atmel_mci",
+	.id		= 0,
+	.dev		= {
+				.dma_mask		= &mmc_dmamask,
+				.coherent_dma_mask	= DMA_BIT_MASK(32),
+				.platform_data		= &mmc0_data,
+	},
+	.resource	= mmc0_resources,
+	.num_resources	= ARRAY_SIZE(mmc0_resources),
+};
+
+static struct resource mmc1_resources[] = {
+	[0] = {
+		.start	= AT91SAM9G45_BASE_MCI1,
+		.end	= AT91SAM9G45_BASE_MCI1 + SZ_16K - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= AT91SAM9G45_ID_MCI1,
+		.end	= AT91SAM9G45_ID_MCI1,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device at91sam9g45_mmc1_device = {
+	.name		= "atmel_mci",
+	.id		= 1,
+	.dev		= {
+				.dma_mask		= &mmc_dmamask,
+				.coherent_dma_mask	= DMA_BIT_MASK(32),
+				.platform_data		= &mmc1_data,
+	},
+	.resource	= mmc1_resources,
+	.num_resources	= ARRAY_SIZE(mmc1_resources),
+};
+
+/* Consider only one slot : slot 0 */
+void __init at91_add_device_mci(short mmc_id, struct mci_platform_data *data)
+{
+
+	if (!data)
+		return;
+
+	/* Must have at least one usable slot */
+	if (!data->slot[0].bus_width)
+		return;
+
+#if defined(CONFIG_AT_HDMAC) || defined(CONFIG_AT_HDMAC_MODULE)
+	{
+	struct at_dma_slave	*atslave;
+	struct mci_dma_data	*alt_atslave;
+
+	alt_atslave = kzalloc(sizeof(struct mci_dma_data), GFP_KERNEL);
+	atslave = &alt_atslave->sdata;
+
+	/* DMA slave channel configuration */
+	atslave->dma_dev = &at_hdmac_device.dev;
+	atslave->reg_width = AT_DMA_SLAVE_WIDTH_32BIT;
+	atslave->cfg = ATC_FIFOCFG_HALFFIFO
+			| ATC_SRC_H2SEL_HW | ATC_DST_H2SEL_HW;
+	atslave->ctrla = ATC_SCSIZE_16 | ATC_DCSIZE_16;
+	if (mmc_id == 0)	/* MCI0 */
+		atslave->cfg |= ATC_SRC_PER(AT_DMA_ID_MCI0)
+			      | ATC_DST_PER(AT_DMA_ID_MCI0);
+
+	else			/* MCI1 */
+		atslave->cfg |= ATC_SRC_PER(AT_DMA_ID_MCI1)
+			      | ATC_DST_PER(AT_DMA_ID_MCI1);
+
+	data->dma_slave = alt_atslave;
+	}
+#endif
+
+
+	/* input/irq */
+	if (data->slot[0].detect_pin) {
+		at91_set_gpio_input(data->slot[0].detect_pin, 1);
+		at91_set_deglitch(data->slot[0].detect_pin, 1);
+	}
+	if (data->slot[0].wp_pin)
+		at91_set_gpio_input(data->slot[0].wp_pin, 1);
+
+	if (mmc_id == 0) {		/* MCI0 */
+
+		/* CLK */
+		at91_set_A_periph(AT91_PIN_PA0, 0);
+
+		/* CMD */
+		at91_set_A_periph(AT91_PIN_PA1, 1);
+
+		/* DAT0, maybe DAT1..DAT3 and maybe DAT4..DAT7 */
+		at91_set_A_periph(AT91_PIN_PA2, 1);
+		if (data->slot[0].bus_width == 4) {
+			at91_set_A_periph(AT91_PIN_PA3, 1);
+			at91_set_A_periph(AT91_PIN_PA4, 1);
+			at91_set_A_periph(AT91_PIN_PA5, 1);
+			if (data->slot[0].bus_width == 8) {
+				at91_set_A_periph(AT91_PIN_PA6, 1);
+				at91_set_A_periph(AT91_PIN_PA7, 1);
+				at91_set_A_periph(AT91_PIN_PA8, 1);
+				at91_set_A_periph(AT91_PIN_PA9, 1);
+			}
+		}
+
+		mmc0_data = *data;
+		at91_clock_associate("mci0_clk", &at91sam9g45_mmc0_device.dev, "mci_clk");
+		platform_device_register(&at91sam9g45_mmc0_device);
+
+	} else {			/* MCI1 */
+
+		/* CLK */
+		at91_set_A_periph(AT91_PIN_PA31, 0);
+
+		/* CMD */
+		at91_set_A_periph(AT91_PIN_PA22, 1);
+
+		/* DAT0, maybe DAT1..DAT3 and maybe DAT4..DAT7 */
+		at91_set_A_periph(AT91_PIN_PA23, 1);
+		if (data->slot[0].bus_width == 4) {
+			at91_set_A_periph(AT91_PIN_PA24, 1);
+			at91_set_A_periph(AT91_PIN_PA25, 1);
+			at91_set_A_periph(AT91_PIN_PA26, 1);
+			if (data->slot[0].bus_width == 8) {
+				at91_set_A_periph(AT91_PIN_PA27, 1);
+				at91_set_A_periph(AT91_PIN_PA28, 1);
+				at91_set_A_periph(AT91_PIN_PA29, 1);
+				at91_set_A_periph(AT91_PIN_PA30, 1);
+			}
+		}
+
+		mmc1_data = *data;
+		at91_clock_associate("mci1_clk", &at91sam9g45_mmc1_device.dev, "mci_clk");
+		platform_device_register(&at91sam9g45_mmc1_device);
+
+	}
+}
+#else
+void __init at91_add_device_mci(short mmc_id, struct mci_platform_data *data) {}
+#endif
+
+
+/* --------------------------------------------------------------------
  *  NAND / SmartMedia
  * -------------------------------------------------------------------- */
 
diff --git a/arch/arm/mach-at91/board-sam9m10g45ek.c b/arch/arm/mach-at91/board-sam9m10g45ek.c
index 7913984..86ff4b5 100644
--- a/arch/arm/mach-at91/board-sam9m10g45ek.c
+++ b/arch/arm/mach-at91/board-sam9m10g45ek.c
@@ -24,7 +24,9 @@
 #include <linux/input.h>
 #include <linux/leds.h>
 #include <linux/clk.h>
+#include <linux/atmel-mci.h>
 
+#include <mach/hardware.h>
 #include <video/atmel_lcdc.h>
 
 #include <asm/setup.h>
@@ -98,6 +100,25 @@ static struct spi_board_info ek_spi_devices[] = {
 
 
 /*
+ * MCI (SD/MMC)
+ */
+static struct mci_platform_data __initdata mci0_data = {
+	.slot[0] = {
+		.bus_width	= 4,
+		.detect_pin	= AT91_PIN_PD10,
+	},
+};
+
+static struct mci_platform_data __initdata mci1_data = {
+	.slot[0] = {
+		.bus_width	= 4,
+		.detect_pin	= AT91_PIN_PD11,
+		.wp_pin		= AT91_PIN_PD29,
+	},
+};
+
+
+/*
  * MACB Ethernet device
  */
 static struct at91_eth_data __initdata ek_macb_data = {
@@ -380,6 +401,9 @@ static void __init ek_board_init(void)
 	at91_add_device_usba(&ek_usba_udc_data);
 	/* SPI */
 	at91_add_device_spi(ek_spi_devices, ARRAY_SIZE(ek_spi_devices));
+	/* MMC */
+	at91_add_device_mci(0, &mci0_data);
+	at91_add_device_mci(1, &mci1_data);
 	/* Ethernet */
 	at91_add_device_eth(&ek_macb_data);
 	/* NAND */
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 68d1279..1a02611 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -237,7 +237,7 @@ endchoice
 
 config MMC_ATMELMCI_DMA
 	bool "Atmel MCI DMA support (EXPERIMENTAL)"
-	depends on MMC_ATMELMCI && AVR32 && DMA_ENGINE && EXPERIMENTAL
+	depends on MMC_ATMELMCI && (AVR32 || ARCH_AT91SAM9G45) && DMA_ENGINE && EXPERIMENTAL
 	help
 	  Say Y here to have the Atmel MCI driver use a DMA engine to
 	  do data transfers and thus increase the throughput and
-- 
1.7.3

[PATCH 2/2] AT91: pm: make sure that r0 is 0 when dealing with cache operations

[Nicolas Ferre]

When using CP15 cache operations (c7), we make sure that Rd (r0)
is actually 0 as ARM 926 TRM is saying.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 arch/arm/mach-at91/pm.c           |    1 +
 arch/arm/mach-at91/pm.h           |    2 +-
 arch/arm/mach-at91/pm_slowclock.S |    1 +
 3 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 87a31ba..1d9f204 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -261,6 +261,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("mov r0, #0");			/* clear r0 for CP15 accesses */
 			asm("b 1f; .align 5; 1:");
 			asm("mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */
 			saved_lpr = sdram_selfrefresh_enable();
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 2c4424b..be081c9 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -21,7 +21,7 @@ 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("mcr p15, 0, r0, c7, c0, 4")
+#define wait_for_interrupt_enable()		asm("mcr p15, 0, r0, c7, c0, 4") /* r0 is 0 here */
 
 #elif defined(CONFIG_ARCH_AT91CAP9)
 #include <mach/at91cap9_ddrsdr.h>
diff --git a/arch/arm/mach-at91/pm_slowclock.S b/arch/arm/mach-at91/pm_slowclock.S
index b6b00a1..f7922a4 100644
--- a/arch/arm/mach-at91/pm_slowclock.S
+++ b/arch/arm/mach-at91/pm_slowclock.S
@@ -124,6 +124,7 @@ ENTRY(at91_slow_clock)
 	ldr	r5, .at91_va_base_ramc1
 
 	/* Drain write buffer */
+	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4
 
 #ifdef CONFIG_ARCH_AT91RM9200
-- 
1.7.3

[PATCH] AT91: rtc: enable built-in RTC in Kconfig for at91sam9g45 family

[Nicolas Ferre]

Enable built-in RTC IP in Kconfig and modify comments and help messages.
RTT as RTC is still available but should not be selected in common case.

Reported-by: Yegor Yefremov <yegor_sub1@visionsystems.de>
Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 drivers/rtc/Kconfig |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 48ca713..2785a0f 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -765,15 +765,15 @@ config RTC_DRV_AT32AP700X
 	  AT32AP700x family processors.
 
 config RTC_DRV_AT91RM9200
-	tristate "AT91RM9200 or AT91SAM9RL"
-	depends on ARCH_AT91RM9200 || ARCH_AT91SAM9RL
+	tristate "AT91RM9200 or some AT91SAM9 RTC"
+	depends on ARCH_AT91RM9200 || ARCH_AT91SAM9RL || ARCH_AT91SAM9G45
 	help
 	  Driver for the internal RTC (Realtime Clock) module found on
-	  Atmel AT91RM9200's and AT91SAM9RL chips. On SAM9RL chips
+	  Atmel AT91RM9200's and some  AT91SAM9 chips. On AT91SAM9 chips
 	  this is powered by the backup power supply.
 
 config RTC_DRV_AT91SAM9
-	tristate "AT91SAM9x/AT91CAP9"
+	tristate "AT91SAM9x/AT91CAP9 RTT as RTC"
 	depends on ARCH_AT91 && !(ARCH_AT91RM9200 || ARCH_AT91X40)
 	help
 	  RTC driver for the Atmel AT91SAM9x and AT91CAP9 internal RTT
@@ -781,8 +781,8 @@ config RTC_DRV_AT91SAM9
 	  supply (such as a small coin cell battery), but do not need to
 	  be used as RTCs.
 
-	  (On AT91SAM9rl chips you probably want to use the dedicated RTC
-	  module and leave the RTT available for other uses.)
+	  (On AT91SAM9rl and AT91SAM9G45 chips you probably want to use the
+	  dedicated RTC module and leave the RTT available for other uses.)
 
 config RTC_DRV_AT91SAM9_RTT
 	int
-- 
1.7.3

[PATCH 2/2] AT91: pm: make sure that r0 is 0 when dealing with cache operations

[Nicolas Ferre]

Hi Russell,

Le 22/10/2010 18:42, Russell King - ARM Linux :
> On Fri, Oct 22, 2010 at 07:29:00PM +0200, Nicolas Ferre wrote:
>> When using CP15 cache operations (c7), we make sure that Rd (r0)
>> is actually 0 as ARM 926 TRM is saying.
> 
> Err, no.  From the GCC manual:
> 
> |    Note that even a volatile `asm' instruction can be moved in ways
> | that appear insignificant to the compiler, such as across jump
> | instructions.  You can't expect a sequence of volatile `asm'
> | instructions to remain perfectly consecutive.  If you want consecutive
> | output, use a single `asm'.  Also, GCC will perform some optimizations
> | across a volatile `asm' instruction; GCC does not "forget everything"
> | when it encounters a volatile `asm' instruction the way some other
> | compilers do.
> 
> So:
> 
> 	asm volatile("foo");
> 	asm volatile("bar");
> 
> is not guaranteed to produce the following uninterrupted code sequence:
> 
> 	foo
> 	bar
> 
> The compiler is free to insert other instructions inbetween these two
> asm statements.
> 
> It's also not permitted to modify registers without telling gcc that
> they've been modified.
> 
>> +			asm("mov r0, #0");			/* clear r0 for CP15 accesses */
>>  			asm("b 1f; .align 5; 1:");
>>  			asm("mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */
> 
> That means the above is completely unacceptable.  It should be:
> 
> 	asm("mov r0, #0;"
> 	"b 1f;"
> 	".align 5;"
> 	"1: mcr p15, 0, r0, c7, c10, 4" : : : "r0");


Thanks a lot for your detailed explanation.

I modify my patch according to your comments:

--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -261,8 +261,13 @@ 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("b 1f; .align 5; 1:");
-                       asm("mcr p15, 0, r0, c7, c10, 4");      /* drain write buffer */
+                       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();
                        wait_for_interrupt_enable();
                        sdram_selfrefresh_disable(saved_lpr);


>> --- a/arch/arm/mach-at91/pm.h
>> +++ b/arch/arm/mach-at91/pm.h
>> @@ -21,7 +21,7 @@ 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("mcr p15, 0, r0, c7, c0, 4")
>> +#define wait_for_interrupt_enable()		asm("mcr p15, 0, r0, c7, c0, 4") /* r0 is 0 here */
> 
> No, r0 is not guaranteed to be zero here.  Use dsb() here instead which
> gets it right.

In fact it is Wait For Interrupt and not dsb... but anyway you are right: I modify it like this:

--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -21,7 +21,8 @@ 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("mcr p15, 0, r0, c7, c0, 4")
+#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>

I repost this modified patch now...

Bye,
-- 
Nicolas Ferre

[PATCH 2/2 v2] AT91: pm: make sure that r0 is 0 when dealing with cache operations

[Nicolas Ferre]

When using CP15 cache operations (c7), we make sure that Rd (r0)
is actually 0 as ARM 926 TRM is saying.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
v2: modify asm according to Russell's comments: be sure to insert asm lines in
the proper order and inform gcc about register used.

 arch/arm/mach-at91/pm.c           |    9 +++++++--
 arch/arm/mach-at91/pm.h           |    3 ++-
 arch/arm/mach-at91/pm_slowclock.S |    1 +
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 87a31ba..dafbacc 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -261,8 +261,13 @@ 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("b 1f; .align 5; 1:");
-			asm("mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */
+			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();
 			wait_for_interrupt_enable();
 			sdram_selfrefresh_disable(saved_lpr);
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 2c4424b..ce9a206 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -21,7 +21,8 @@ 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("mcr p15, 0, r0, c7, c0, 4")
+#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>
diff --git a/arch/arm/mach-at91/pm_slowclock.S b/arch/arm/mach-at91/pm_slowclock.S
index b6b00a1..f7922a4 100644
--- a/arch/arm/mach-at91/pm_slowclock.S
+++ b/arch/arm/mach-at91/pm_slowclock.S
@@ -124,6 +124,7 @@ ENTRY(at91_slow_clock)
 	ldr	r5, .at91_va_base_ramc1
 
 	/* Drain write buffer */
+	mov	r0, #0
 	mcr	p15, 0, r0, c7, c10, 4
 
 #ifdef CONFIG_ARCH_AT91RM9200
-- 
1.7.3