Patchwork [U-Boot] lcd: remove unaligned access in lcd_dt_simplefb_configure_node()

login
register
mail settings
Submitter Stephen Warren
Date June 13, 2013, 11:13 p.m.
Message ID <1371165191-11174-1-git-send-email-swarren@wwwdotorg.org>
Download mbox | patch
Permalink /patch/251194/
State Accepted
Delegated to: Anatolij Gustschin
Headers show

Comments

Stephen Warren - June 13, 2013, 11:13 p.m.
From: Stephen Warren <swarren@nvidia.com>

Some ARM compilers may emit code that makes unaligned accesses when
faced with constructs such as:

const char format[] = "r5g6b5";

Make this data static since it doesn't chagne; the compiler will simply
place it into the .rodata section directly, and avoid any unaligned run-
time initialization.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 common/lcd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Stephen Warren - July 1, 2013, 4:30 p.m.
On 06/13/2013 05:13 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Some ARM compilers may emit code that makes unaligned accesses when
> faced with constructs such as:
> 
> const char format[] = "r5g6b5";
> 
> Make this data static since it doesn't chagne; the compiler will simply
> place it into the .rodata section directly, and avoid any unaligned run-
> time initialization.

I don't think this has been applied yet. Will it be applied for this
release? Should I resend the patch?
Tom Rini - July 1, 2013, 4:35 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 07/01/2013 12:30 PM, Stephen Warren wrote:
> On 06/13/2013 05:13 PM, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>> 
>> Some ARM compilers may emit code that makes unaligned accesses
>> when faced with constructs such as:
>> 
>> const char format[] = "r5g6b5";
>> 
>> Make this data static since it doesn't chagne; the compiler will
>> simply place it into the .rodata section directly, and avoid any
>> unaligned run- time initialization.
> 
> I don't think this has been applied yet. Will it be applied for
> this release? Should I resend the patch?

It's a bug fix and as such I want it.  Anatolij, there's a few "video"
patches out there, are you going to grab things / has this spurred you
to grab things or should I take a look around and pull up a patch set?
 Thanks!

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJR0a/hAAoJENk4IS6UOR1WiV4P/RtMBhLZRq8fW9NNCaVupJC9
Y2F7kOnDOGR9jdvyWbTXA5Xvw8hMa+zx0ue3tGPf2y2rl4/EL2IPLS/PudwqTMHt
n5PnTAdadOUC09OZiRz8GBENis6RBFp6xHVKDxkvdTlVx5/wItOKKfLDW60n7hM+
4++VALba8/Ii6mWBRFdTyUEPcPCeVQxsKkfzeYrXwtBuKeEZ8f9upohfc19gscBU
pGm0wsb48mJGwk8Kq0JtDmVzak4paTptJAnlA6wcqOd2MRTw7crRuVhvyLymCNki
VcFtgwLI6sK7y0ohi1y3vJepL0soXGxswSRFYg2xyr8a+80uC3ZGUfkObddo+074
gGMDK7B58KVD/ZBlhsj+qpzAHUDsjRVBV01wvuTZ0FHE5DBJMISX1sYJihhLrxjc
DNX+a62guUW3Zc0xJIoHckwTh7FbXgiGK+0VtJHC3ueIw0ABRs3Q2PGDa3Ud3FvA
Y392ht9ftakbfIDQa/QTcw3h7VXIKfKWON621if/prsPg3GBbFV/P6AVar1NqzPR
imZQdyPJimkVZs/bFjFKqH/SvYIwFuoA8jwca84XZ2kbN8aZdcgXq+GKUq83Wkyn
rTr6K6c9lTixSpcpqOZMnkmT0hqglpnlVLzwZ6ONYUwtlbWK3zc6ZafxbQ2rWSLq
nuQ/vigESYDEfCYIWOuQ
=EA3R
-----END PGP SIGNATURE-----
Anatolij Gustschin - July 1, 2013, 6:08 p.m.
On Mon, 1 Jul 2013 12:35:46 -0400
Tom Rini <trini@ti.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 07/01/2013 12:30 PM, Stephen Warren wrote:
> > On 06/13/2013 05:13 PM, Stephen Warren wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >> 
> >> Some ARM compilers may emit code that makes unaligned accesses
> >> when faced with constructs such as:
> >> 
> >> const char format[] = "r5g6b5";
> >> 
> >> Make this data static since it doesn't chagne; the compiler will
> >> simply place it into the .rodata section directly, and avoid any
> >> unaligned run- time initialization.
> > 
> > I don't think this has been applied yet. Will it be applied for
> > this release? Should I resend the patch?
> 
> It's a bug fix and as such I want it.  Anatolij, there's a few "video"
> patches out there, are you going to grab things / has this spurred you
> to grab things or should I take a look around and pull up a patch set?
>  Thanks!

I'll look at video patches and grab them for release. Thanks for
reminding me!

Anatolij
Anatolij Gustschin - July 1, 2013, 6:13 p.m.
On Thu, 13 Jun 2013 17:13:11 -0600
Stephen Warren <swarren@wwwdotorg.org> wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> Some ARM compilers may emit code that makes unaligned accesses when
> faced with constructs such as:
> 
> const char format[] = "r5g6b5";
> 
> Make this data static since it doesn't chagne; the compiler will simply
> place it into the .rodata section directly, and avoid any unaligned run-
> time initialization.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  common/lcd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Applied to u-boot-video/master. Thanks!

Anatolij

Patch

diff --git a/common/lcd.c b/common/lcd.c
index 3a60484..c9a589e 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -1193,7 +1193,7 @@  static int lcd_dt_simplefb_configure_node(void *blob, int off)
 	u32 stride;
 	fdt32_t cells[2];
 	int ret;
-	const char format[] =
+	static const char format[] =
 #if LCD_BPP == LCD_COLOR16
 		"r5g6b5";
 #else
@@ -1239,8 +1239,8 @@  static int lcd_dt_simplefb_configure_node(void *blob, int off)
 
 int lcd_dt_simplefb_add_node(void *blob)
 {
-	const char compat[] = "simple-framebuffer";
-	const char disabled[] = "disabled";
+	static const char compat[] = "simple-framebuffer";
+	static const char disabled[] = "disabled";
 	int off, ret;
 
 	off = fdt_add_subnode(blob, 0, "framebuffer");