All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@codeaurora.org>
To: Greg KH <greg@kroah.com>
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Arnd Bergmann <arnd@arndb.de>,
	Daniel Walker <dwalker@codeaurora.org>
Subject: [PATCHv2 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile
Date: Thu,  3 Feb 2011 15:48:34 -0800	[thread overview]
Message-ID: <1296776916-21018-2-git-send-email-sboyd@codeaurora.org> (raw)
In-Reply-To: <1296776916-21018-1-git-send-email-sboyd@codeaurora.org>

Without marking the asm __dcc_getstatus() volatile my compiler
decides it can cache the value of __ret in a register and then
check the value of it continually in hvc_dcc_put_chars() (I had
to replace get_wait/put_wait with 1 and fixup the branch
otherwise my disassembler barfed on __dcc_(get|put)char).

00000000 <hvc_dcc_put_chars>:
   0:   ee103e11        mrc     14, 0, r3, cr0, cr1, {0}
   4:   e3a0c000        mov     ip, #0  ; 0x0
   8:   e2033202        and     r3, r3, #536870912      ; 0x20000000
   c:   ea000006        b       2c <hvc_dcc_put_chars+0x2c>
  10:   e3530000        cmp     r3, #0  ; 0x0
  14:   1afffffd        bne     10 <hvc_dcc_put_chars+0x10>
  18:   e7d1000c        ldrb    r0, [r1, ip]
  1c:   ee10fe11        mrc     14, 0, pc, cr0, cr1, {0}
  20:   2afffffd        bcs     1c <hvc_dcc_put_chars+0x1c>
  24:   ee000e15        mcr     14, 0, r0, cr0, cr5, {0}
  28:   e28cc001        add     ip, ip, #1      ; 0x1
  2c:   e15c0002        cmp     ip, r2
  30:   bafffff6        blt     10 <hvc_dcc_put_chars+0x10>
  34:   e1a00002        mov     r0, r2
  38:   e12fff1e        bx      lr

As you can see, the value of the mrc is checked against
DCC_STATUS_TX (bit 29) and then stored in r3 for later use.
Marking the asm volatile produces the following:

00000000 <hvc_dcc_put_chars>:
   0:   e3a03000        mov     r3, #0  ; 0x0
   4:   ea000007        b       28 <hvc_dcc_put_chars+0x28>
   8:   ee100e11        mrc     14, 0, r0, cr0, cr1, {0}
   c:   e3100202        tst     r0, #536870912  ; 0x20000000
  10:   1afffffc        bne     8 <hvc_dcc_put_chars+0x8>
  14:   e7d10003        ldrb    r0, [r1, r3]
  18:   ee10fe11        mrc     14, 0, pc, cr0, cr1, {0}
  1c:   2afffffd        bcs     18 <hvc_dcc_put_chars+0x18>
  20:   ee000e15        mcr     14, 0, r0, cr0, cr5, {0}
  24:   e2833001        add     r3, r3, #1      ; 0x1
  28:   e1530002        cmp     r3, r2
  2c:   bafffff5        blt     8 <hvc_dcc_put_chars+0x8>
  30:   e1a00002        mov     r0, r2
  34:   e12fff1e        bx      lr

which looks better and actually works. Mark all the inline
assembly in this file as volatile since we don't want the
compiler to optimize away these statements or move them around
in any way.

Acked-by: Tony Lindgren <tony@atomide.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: Daniel Walker <dwalker@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/tty/hvc/hvc_dcc.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
index 6470f63..155ec10 100644
--- a/drivers/tty/hvc/hvc_dcc.c
+++ b/drivers/tty/hvc/hvc_dcc.c
@@ -33,8 +33,7 @@
 static inline u32 __dcc_getstatus(void)
 {
 	u32 __ret;
-
-	asm("mrc p14, 0, %0, c0, c1, 0	@ read comms ctrl reg"
+	asm volatile("mrc p14, 0, %0, c0, c1, 0	@ read comms ctrl reg"
 		: "=r" (__ret) : : "cc");
 
 	return __ret;
@@ -46,7 +45,7 @@ static inline char __dcc_getchar(void)
 {
 	char __c;
 
-	asm("get_wait:	mrc p14, 0, pc, c0, c1, 0                          \n\
+	asm volatile("get_wait:	mrc p14, 0, pc, c0, c1, 0                  \n\
 			bne get_wait                                       \n\
 			mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
 		: "=r" (__c) : : "cc");
@@ -58,7 +57,7 @@ static inline char __dcc_getchar(void)
 {
 	char __c;
 
-	asm("mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
+	asm volatile("mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
 		: "=r" (__c));
 
 	return __c;
@@ -68,7 +67,7 @@ static inline char __dcc_getchar(void)
 #if defined(CONFIG_CPU_V7)
 static inline void __dcc_putchar(char c)
 {
-	asm("put_wait:	mrc p14, 0, pc, c0, c1, 0                 \n\
+	asm volatile("put_wait:	mrc p14, 0, pc, c0, c1, 0         \n\
 			bcs put_wait                              \n\
 			mcr p14, 0, %0, c0, c5, 0                   "
 	: : "r" (c) : "cc");
@@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c)
 #else
 static inline void __dcc_putchar(char c)
 {
-	asm("mcr p14, 0, %0, c0, c5, 0	@ write a char"
+	asm volatile("mcr p14, 0, %0, c0, c5, 0	@ write a char"
 		: /* no output register */
 		: "r" (c));
 }
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


WARNING: multiple messages have this Message-ID (diff)
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile
Date: Thu,  3 Feb 2011 15:48:34 -0800	[thread overview]
Message-ID: <1296776916-21018-2-git-send-email-sboyd@codeaurora.org> (raw)
In-Reply-To: <1296776916-21018-1-git-send-email-sboyd@codeaurora.org>

Without marking the asm __dcc_getstatus() volatile my compiler
decides it can cache the value of __ret in a register and then
check the value of it continually in hvc_dcc_put_chars() (I had
to replace get_wait/put_wait with 1 and fixup the branch
otherwise my disassembler barfed on __dcc_(get|put)char).

00000000 <hvc_dcc_put_chars>:
   0:   ee103e11        mrc     14, 0, r3, cr0, cr1, {0}
   4:   e3a0c000        mov     ip, #0  ; 0x0
   8:   e2033202        and     r3, r3, #536870912      ; 0x20000000
   c:   ea000006        b       2c <hvc_dcc_put_chars+0x2c>
  10:   e3530000        cmp     r3, #0  ; 0x0
  14:   1afffffd        bne     10 <hvc_dcc_put_chars+0x10>
  18:   e7d1000c        ldrb    r0, [r1, ip]
  1c:   ee10fe11        mrc     14, 0, pc, cr0, cr1, {0}
  20:   2afffffd        bcs     1c <hvc_dcc_put_chars+0x1c>
  24:   ee000e15        mcr     14, 0, r0, cr0, cr5, {0}
  28:   e28cc001        add     ip, ip, #1      ; 0x1
  2c:   e15c0002        cmp     ip, r2
  30:   bafffff6        blt     10 <hvc_dcc_put_chars+0x10>
  34:   e1a00002        mov     r0, r2
  38:   e12fff1e        bx      lr

As you can see, the value of the mrc is checked against
DCC_STATUS_TX (bit 29) and then stored in r3 for later use.
Marking the asm volatile produces the following:

00000000 <hvc_dcc_put_chars>:
   0:   e3a03000        mov     r3, #0  ; 0x0
   4:   ea000007        b       28 <hvc_dcc_put_chars+0x28>
   8:   ee100e11        mrc     14, 0, r0, cr0, cr1, {0}
   c:   e3100202        tst     r0, #536870912  ; 0x20000000
  10:   1afffffc        bne     8 <hvc_dcc_put_chars+0x8>
  14:   e7d10003        ldrb    r0, [r1, r3]
  18:   ee10fe11        mrc     14, 0, pc, cr0, cr1, {0}
  1c:   2afffffd        bcs     18 <hvc_dcc_put_chars+0x18>
  20:   ee000e15        mcr     14, 0, r0, cr0, cr5, {0}
  24:   e2833001        add     r3, r3, #1      ; 0x1
  28:   e1530002        cmp     r3, r2
  2c:   bafffff5        blt     8 <hvc_dcc_put_chars+0x8>
  30:   e1a00002        mov     r0, r2
  34:   e12fff1e        bx      lr

which looks better and actually works. Mark all the inline
assembly in this file as volatile since we don't want the
compiler to optimize away these statements or move them around
in any way.

Acked-by: Tony Lindgren <tony@atomide.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: Daniel Walker <dwalker@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/tty/hvc/hvc_dcc.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
index 6470f63..155ec10 100644
--- a/drivers/tty/hvc/hvc_dcc.c
+++ b/drivers/tty/hvc/hvc_dcc.c
@@ -33,8 +33,7 @@
 static inline u32 __dcc_getstatus(void)
 {
 	u32 __ret;
-
-	asm("mrc p14, 0, %0, c0, c1, 0	@ read comms ctrl reg"
+	asm volatile("mrc p14, 0, %0, c0, c1, 0	@ read comms ctrl reg"
 		: "=r" (__ret) : : "cc");
 
 	return __ret;
@@ -46,7 +45,7 @@ static inline char __dcc_getchar(void)
 {
 	char __c;
 
-	asm("get_wait:	mrc p14, 0, pc, c0, c1, 0                          \n\
+	asm volatile("get_wait:	mrc p14, 0, pc, c0, c1, 0                  \n\
 			bne get_wait                                       \n\
 			mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
 		: "=r" (__c) : : "cc");
@@ -58,7 +57,7 @@ static inline char __dcc_getchar(void)
 {
 	char __c;
 
-	asm("mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
+	asm volatile("mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
 		: "=r" (__c));
 
 	return __c;
@@ -68,7 +67,7 @@ static inline char __dcc_getchar(void)
 #if defined(CONFIG_CPU_V7)
 static inline void __dcc_putchar(char c)
 {
-	asm("put_wait:	mrc p14, 0, pc, c0, c1, 0                 \n\
+	asm volatile("put_wait:	mrc p14, 0, pc, c0, c1, 0         \n\
 			bcs put_wait                              \n\
 			mcr p14, 0, %0, c0, c5, 0                   "
 	: : "r" (c) : "cc");
@@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c)
 #else
 static inline void __dcc_putchar(char c)
 {
-	asm("mcr p14, 0, %0, c0, c5, 0	@ write a char"
+	asm volatile("mcr p14, 0, %0, c0, c5, 0	@ write a char"
 		: /* no output register */
 		: "r" (c));
 }
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

  reply	other threads:[~2011-02-03 23:48 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-30 19:25 [PATCH] drivers: char: hvc: add arm JTAG DCC console support Daniel Walker
2010-11-30 19:25 ` Daniel Walker
2010-11-30 19:57 ` Nicolas Pitre
2010-11-30 19:57   ` Nicolas Pitre
2010-11-30 21:17   ` Arnd Bergmann
2010-11-30 21:17     ` Arnd Bergmann
2010-12-01  5:30 ` Stephen Boyd
2010-12-01  5:30   ` Stephen Boyd
2010-12-01 18:54   ` Daniel Walker
2010-12-01 18:54     ` Daniel Walker
2010-12-01 19:28     ` Greg KH
2010-12-01 19:28       ` Greg KH
2010-12-18  5:16       ` [PATCH] hvc_dcc: Simplify assembly for v6 and v7 ARM Stephen Boyd
2010-12-18  5:16         ` Stephen Boyd
2010-12-20 17:51         ` Daniel Walker
2010-12-20 17:51           ` Daniel Walker
2010-12-20 18:39           ` Stephen Boyd
2010-12-20 18:39             ` Stephen Boyd
2010-12-20 18:46             ` Nicolas Pitre
2010-12-20 18:46               ` Nicolas Pitre
2010-12-20 20:08         ` [PATCH 0/3] hvc_dcc cleanups and fixes Stephen Boyd
2010-12-20 20:08           ` Stephen Boyd
2010-12-20 20:08           ` [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile Stephen Boyd
2010-12-20 20:08             ` Stephen Boyd
2010-12-20 21:39             ` Nicolas Pitre
2010-12-20 21:39               ` Nicolas Pitre
2011-01-02  9:00               ` Pavel Machek
2011-01-02  9:00                 ` Pavel Machek
2011-01-02 18:49                 ` David Brown
2011-01-02 18:49                   ` David Brown
2011-01-03  5:50                   ` Pavel Machek
2011-01-03  5:50                     ` Pavel Machek
2011-01-04 18:49               ` Tony Lindgren
2011-01-04 18:49                 ` Tony Lindgren
2010-12-20 21:49             ` Arnaud Lacombe
2010-12-20 21:49               ` Arnaud Lacombe
2010-12-20 21:52               ` Stephen Boyd
2010-12-20 21:52                 ` Stephen Boyd
2010-12-20 22:10                 ` Nicolas Pitre
2010-12-20 22:10                   ` Nicolas Pitre
2010-12-20 20:08           ` [PATCH 2/3] hvc_dcc: Simplify put_chars()/get_chars() loops Stephen Boyd
2010-12-20 20:08             ` Stephen Boyd
2010-12-20 20:08           ` [PATCH 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM Stephen Boyd
2010-12-20 20:08             ` Stephen Boyd
2010-12-20 21:44             ` Nicolas Pitre
2010-12-20 21:44               ` Nicolas Pitre
2011-01-04 18:52               ` Tony Lindgren
2011-01-04 18:52                 ` Tony Lindgren
2011-01-06  1:49           ` [PATCH 0/3] hvc_dcc cleanups and fixes Stephen Boyd
2011-01-06  1:49             ` Stephen Boyd
2011-01-06  3:20             ` Greg KH
2011-01-06  3:20               ` Greg KH
2011-02-03 22:17           ` Greg KH
2011-02-03 22:17             ` Greg KH
2011-02-03 22:17             ` Greg KH
2011-02-03 23:19             ` Stephen Boyd
2011-02-03 23:19               ` Stephen Boyd
2011-02-03 23:30               ` Greg KH
2011-02-03 23:30                 ` Greg KH
2011-02-03 23:48             ` [PATCHv2 " Stephen Boyd
2011-02-03 23:48               ` Stephen Boyd
2011-02-03 23:48               ` Stephen Boyd [this message]
2011-02-03 23:48                 ` [PATCHv2 1/3] hvc_dcc: Fix bad code generation by marking assembly volatile Stephen Boyd
2011-02-03 23:48               ` [PATCHv2 2/3] hvc_dcc: Simplify put_chars()/get_chars() loops Stephen Boyd
2011-02-03 23:48                 ` Stephen Boyd
2011-02-03 23:48               ` [PATCHv2 3/3] hvc_dcc: Simplify assembly for v6 and v7 ARM Stephen Boyd
2011-02-03 23:48                 ` Stephen Boyd
2010-12-01 20:20     ` [PATCH] drivers: char: hvc: add arm JTAG DCC console support Stephen Boyd
2010-12-01 20:20       ` Stephen Boyd
2010-12-07 19:10       ` Stephen Boyd
2010-12-07 19:10         ` Stephen Boyd
2011-01-14 19:19         ` Tony Lindgren
2011-01-14 19:19           ` Tony Lindgren
2011-01-14 23:49           ` Stephen Boyd
2011-01-14 23:49             ` Stephen Boyd
2011-04-07 18:39 ` RONETIX - Asen Dimov
2011-04-07 18:39   ` RONETIX - Asen Dimov
2011-04-07 18:57   ` Mike Frysinger
2011-04-07 18:57     ` Mike Frysinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1296776916-21018-2-git-send-email-sboyd@codeaurora.org \
    --to=sboyd@codeaurora.org \
    --cc=arnd@arndb.de \
    --cc=dwalker@codeaurora.org \
    --cc=greg@kroah.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.