Patchwork [U-Boot] ARM: cfi_flash: Avoid generation of unaligned accesses

login
register
mail settings
Submitter Andrew Gabbasov
Date April 28, 2013, 10:39 a.m.
Message ID <1367145576-12748-1-git-send-email-andrew_gabbasov@mentor.com>
Download mbox | patch
Permalink /patch/240244/
State Changes Requested
Delegated to: Marek Vasut
Headers show

Comments

Andrew Gabbasov - April 28, 2013, 10:39 a.m.
Packed structure cfi_qry contains unaligned 16- and 32-bits members,
accessing which causes problems when cfi_flash driver is compiled with
-munaligned-access option: flash initialization hangs, probably
due to data error.

Since fixing the code to use some other way of accessing those fields
seems quite expensive, just force the compiler to use -mno-unaligned-access.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/mtd/Makefile |    3 +++
 1 file changed, 3 insertions(+)
Marek Vasut - April 28, 2013, 6:58 p.m.
Dear Andrew Gabbasov,

> Packed structure cfi_qry contains unaligned 16- and 32-bits members,
> accessing which causes problems when cfi_flash driver is compiled with
> -munaligned-access option: flash initialization hangs, probably
> due to data error.
> 
> Since fixing the code to use some other way of accessing those fields
> seems quite expensive, just force the compiler to use
> -mno-unaligned-access.
> 
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> ---
>  drivers/mtd/Makefile |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index 543c845..a120f0b 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -46,6 +46,9 @@ all:	$(LIB)
>  $(LIB):	$(obj).depend $(OBJS)
>  	$(call cmd_link_o_target, $(OBJS))
> 
> +# SEE README.arm-unaligned-accesses
> +$(obj)cfi_flash.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
> +
>  #########################################################################
> 
>  # defines $(obj).depend target

I'd say rather fix the structure than use this workaround. See, using

u8[3]
u16

will make unaligned access. The fix would be to split that u16 into two u8 and 
add a proper accessor. It'll be much larger effort, but it'd also be more 
correct.

Best regards,
Marek Vasut
Andrew Gabbasov - April 29, 2013, 9:40 a.m.
> From: Marek Vasut [marek.vasut@gmail.com]
> Sent: Sunday, April 28, 2013 22:58
> To: u-boot@lists.denx.de
> Cc: Gabbasov, Andrew; Albert ARIBAUD; Tom Rini
> Subject: Re: [U-Boot] [PATCH] ARM: cfi_flash: Avoid generation of unaligned accesses
> 
> Dear Andrew Gabbasov,
> 
> > Packed structure cfi_qry contains unaligned 16- and 32-bits members,
> > accessing which causes problems when cfi_flash driver is compiled with
> > -munaligned-access option: flash initialization hangs, probably
> > due to data error.
> >
> > Since fixing the code to use some other way of accessing those fields
> > seems quite expensive, just force the compiler to use
> > -mno-unaligned-access.
> >
> > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> > ---
> >  drivers/mtd/Makefile |    3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> > index 543c845..a120f0b 100644
> > --- a/drivers/mtd/Makefile
> > +++ b/drivers/mtd/Makefile
> > @@ -46,6 +46,9 @@ all:        $(LIB)
> >  $(LIB):      $(obj).depend $(OBJS)
> >       $(call cmd_link_o_target, $(OBJS))
> >
> > +# SEE README.arm-unaligned-accesses
> > +$(obj)cfi_flash.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
> > +
> >  #########################################################################
> >
> >  # defines $(obj).depend target
> 
> I'd say rather fix the structure than use this workaround. See, using
> 
> u8[3]
> u16
> 
> will make unaligned access. The fix would be to split that u16 into two u8 and
> add a proper accessor. It'll be much larger effort, but it'd also be more
> correct.
> 
> Best regards,
> Marek Vasut

Hi Marek,

Thank you for your response.

I was thinking about this way of fixing, but it seemed too require too much effort.
However, after some closer look it turned out to be not quite difficult, since
there are quite a few accesses to such unaligned fields. So, I made another fix and
sent to separately to the list with Subject: "[U-Boot][PATCH] ARM: cfi_flash: Fix unaligned
accesses to cfi_qry structure". Please, look at it, if it is OK from your point of view.

Thanks.

Best regards,
Andrew
Andrew Gabbasov - May 8, 2013, 2:56 p.m.
> From: Gabbasov, Andrew
> Sent: Monday, April 29, 2013 13:40
> To: Marek Vasut; u-boot@lists.denx.de
> Cc: Albert ARIBAUD; Tom Rini
> Subject: RE: [U-Boot] [PATCH] ARM: cfi_flash: Avoid generation of unaligned accesses

[ skipped ]

> Hi Marek,
> 
> Thank you for your response.
> 
> I was thinking about this way of fixing, but it seemed too require too much effort.
> However, after some closer look it turned out to be not quite difficult, since
> there are quite a few accesses to such unaligned fields. So, I made another fix and
> sent to separately to the list with Subject: "[U-Boot][PATCH] ARM: cfi_flash: Fix unaligned
> accesses to cfi_qry structure". Please, look at it, if it is OK from your point of view.
> 
> Thanks.
> 
> Best regards,
> Andrew

Any comments on that new fix?

Thanks.

Best regards,
Andrew

Patch

diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 543c845..a120f0b 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -46,6 +46,9 @@  all:	$(LIB)
 $(LIB):	$(obj).depend $(OBJS)
 	$(call cmd_link_o_target, $(OBJS))
 
+# SEE README.arm-unaligned-accesses
+$(obj)cfi_flash.o: CFLAGS += $(PLATFORM_NO_UNALIGNED)
+
 #########################################################################
 
 # defines $(obj).depend target