On Sat, 26 Feb 2011, David Brown wrote:

On Fri, Feb 25 2011, Will Deacon wrote:

quoted

On Thu, 2011-02-24 at 18:44 +0000, Stephen Boyd wrote:

quoted

We don't want the compiler to remove these asm statements or
reorder them in any way. Mark them as volatile to be sure.

Signed-off-by: Stephen Boyd <redacted>
---
 arch/arm/mach-msm/scm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-msm/scm.c b/arch/arm/mach-msm/scm.c
index f4b9bc9..ba57b5a 100644
--- a/arch/arm/mach-msm/scm.c
+++ b/arch/arm/mach-msm/scm.c

@@ -174,7 +174,7 @@ static u32 smc(u32 cmd_addr)
        register u32 r0 asm("r0") = 1;
        register u32 r1 asm("r1") = (u32)&context_id;
        register u32 r2 asm("r2") = cmd_addr;
-       asm(
+       asm volatile(
                __asmeq("%0", "r0")
                __asmeq("%1", "r0")
                __asmeq("%2", "r1")

@@ -271,7 +271,7 @@ u32 scm_get_version(void)
                return version;

        mutex_lock(&scm_lock);
-       asm(
+       asm volatile(
                __asmeq("%0", "r1")
                __asmeq("%1", "r0")
                __asmeq("%2", "r1")

These asm blocks all have sensible looking output constraints. Why
do they need to be marked volatile?

Without the volatile, the compiler is free to assume the only side
effects of the asm are to modify the output registers.  The volatile is
needed to indicate to the compiler that the asm has other side effects.
There isn't enough optimization, yet, in gcc to change the generated
code in this case, so it happens to generate the correct code without
it.

The second probably doesn't need it, unless we are expecting the version
to change dynamically.  The volatile makes the scm_get_version()
function clearly a call to scm, though, so is probably useful to
document the intent.

If the inline asm does have side effects which are not obvious other 
than producing a result for the output operand then it is a good idea to 
add a comment to that effect.  Otherwise it is always best to omit the 
volatile and let gcc move the inline asm around or even delete it 
entirely when possible.


Nicolas