[PATCH 2/2] AT91: pm: make sure that r0 is 0 when dealing with cache operations
Russell King - ARM Linux
linux at arm.linux.org.uk
Fri Oct 22 12:42:47 EDT 2010
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.
is not guaranteed to produce the following uninterrupted code sequence:
The compiler is free to insert other instructions inbetween these two
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;"
"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.
More information about the linux-arm-kernel