| Message ID | 1369248498-1799-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
On Wed, May 22, 2013 at 11:48 AM, Stephen Warren <swarren@wwwdotorg.org>wrote: > From: Stephen Warren <swarren@nvidia.com> > > Initialized character arrays on the stack can cause gcc to emit code that > performs unaligned accessess. Make the data static to avoid this. > > Note that the unaligned accesses are made when copying data to prefix[] on > the stack from .rodata. By making the data static, the copy is completely > avoided. All explicitly written code treats the data as u8[], so will never > cause any unaligned accesses. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > Acked-by: Simon Glass <sjg@chromium.org> Thanks for fixing. I hit this with gcc 4.7. I wonder if previous revisions would not make this assumption? Another problem I have is that the 'linux' in 'linux,keymap' in the device compile turns into '1' since gcc predefines 'linux' to 1: I think I'm going to add a -Ulinux to dts/Makefile. Regards. Simon
Dear Simon Glass, In message <CAPnjgZ1_M-7EgGade-p-TLrYnPNNXGyh8zh32w8a2+2qGVYuow@mail.gmail.com> you wrote: > > Another problem I have is that the 'linux' in 'linux,keymap' in the device > compile turns into '1' since gcc predefines 'linux' to 1: Should this not be considered a GCC bug? After all, "linux" is not a reserved identifier. [Defining __linux, or __linux__ would be probab- ly OK, but "linux" or "arm" are not - IMO.] Best regards, Wolfgang Denk
Hi Wolfgang, On Sun, May 26, 2013 at 1:55 PM, Wolfgang Denk <wd@denx.de> wrote: > Dear Simon Glass, > > In message < > CAPnjgZ1_M-7EgGade-p-TLrYnPNNXGyh8zh32w8a2+2qGVYuow@mail.gmail.com> you > wrote: > > > > Another problem I have is that the 'linux' in 'linux,keymap' in the > device > > compile turns into '1' since gcc predefines 'linux' to 1: > > Should this not be considered a GCC bug? After all, "linux" is not a > reserved identifier. [Defining __linux, or __linux__ would be probab- > ly OK, but "linux" or "arm" are not - IMO.] > It certainly surprised me, but if it is a bug, then it might be too late to fix it, since that release is widespread. > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > Der Horizont vieler Menschen ist ein Kreis mit Radius Null -- > und das nennen sie ihren Standpunkt. > That was worth translating :-) Regards, Simon
On 05/26/2013 01:28 PM, Simon Glass wrote: > > On Wed, May 22, 2013 at 11:48 AM, Stephen Warren <swarren@wwwdotorg.org > <mailto:swarren@wwwdotorg.org>> wrote: > > From: Stephen Warren <swarren@nvidia.com <mailto:swarren@nvidia.com>> > > Initialized character arrays on the stack can cause gcc to emit code > that > performs unaligned accessess. Make the data static to avoid this. > > Note that the unaligned accesses are made when copying data to > prefix[] on > the stack from .rodata. By making the data static, the copy is > completely > avoided. All explicitly written code treats the data as u8[], so > will never > cause any unaligned accesses. > > Signed-off-by: Stephen Warren <swarren@nvidia.com > <mailto:swarren@nvidia.com>> > > > Acked-by: Simon Glass <sjg@chromium.org <mailto:sjg@chromium.org>> > > Thanks for fixing. > > I hit this with gcc 4.7. I wonder if previous revisions would not make > this assumption? IIRC, gcc-4.7 introduces the emission of native unaligned accesses, and it's been back-ported to Linaro gcc-4.6. > Another problem I have is that the 'linux' in 'linux,keymap' in the > device compile turns into '1' since gcc predefines 'linux' to 1: > > I think I'm going to add a -Ulinux to dts/Makefile. I forget the exact details, but if you check the Linux makefiles for dtc+cpp, they don't suffer from this issue any more; it may have been due to use of -x assembler-with-cpp. I do also have a bug filed internally to NVIDIA to fix that, which is assigned to Tom. But, I'm sure he'd be glad if you fixed it:-)
On 05/22/2013 12:48 PM, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > Initialized character arrays on the stack can cause gcc to emit code that > performs unaligned accessess. Make the data static to avoid this. > > Note that the unaligned accesses are made when copying data to prefix[] on > the stack from .rodata. By making the data static, the copy is completely > avoided. All explicitly written code treats the data as u8[], so will never > cause any unaligned accesses. Tom, does this patch look good? The discussion following it was unrelated to this patch, but rather related to pre-processing of device-trees, so I don't think should prevent this patch being merged.
On Wed, May 22, 2013 at 08:48:18AM -0000, Stephen Warren wrote: > From: Stephen Warren <swarren@nvidia.com> > > Initialized character arrays on the stack can cause gcc to emit code that > performs unaligned accessess. Make the data static to avoid this. > > Note that the unaligned accesses are made when copying data to prefix[] on > the stack from .rodata. By making the data static, the copy is completely > avoided. All explicitly written code treats the data as u8[], so will never > cause any unaligned accesses. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > Acked-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
On Wed, Jun 05, 2013 at 08:34:20AM -0400, Tom Rini wrote: > On Wed, May 22, 2013 at 08:48:18AM -0000, Stephen Warren wrote: > > > From: Stephen Warren <swarren@nvidia.com> > > > > Initialized character arrays on the stack can cause gcc to emit code that > > performs unaligned accessess. Make the data static to avoid this. > > > > Note that the unaligned accesses are made when copying data to prefix[] on > > the stack from .rodata. By making the data static, the copy is completely > > avoided. All explicitly written code treats the data as u8[], so will never > > cause any unaligned accesses. > > > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > > Acked-by: Simon Glass <sjg@chromium.org> > > Applied to u-boot/master, thanks! Bah! I see I applied v1 and not v2, I shall post and apply the delta between them momentarily.
diff --git a/drivers/input/key_matrix.c b/drivers/input/key_matrix.c index 946a186..c8b048e 100644 --- a/drivers/input/key_matrix.c +++ b/drivers/input/key_matrix.c @@ -158,7 +158,7 @@ int key_matrix_decode_fdt(struct key_matrix *config, const void *blob, int node) { const struct fdt_property *prop; - const char prefix[] = "linux,"; + static const char prefix[] = "linux,"; int plen = sizeof(prefix) - 1; int offset;