tools: imximage: Load a size that is multiple of 512

PATCH New Fabio Estevam 4 participants 6 messages Jan 3, 2013 – Jan 5, 2013 Arm i.MX and Layerscape

lore

Share & tools

lore

Fabio Estevam

author

2013-01-03 18:24 UTC #0 lore raw

PATCH v2 1/1 tools: imximage: Load a size that is multiple of 512

In order to mx53 ROM to properly load the U-boot image, its header size should be multiple of 512 bytes.

This issue was observed with gcc 4.6.2/4.7.3, which caused data aborts:

U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 13:13:28)

Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
In:    serial
Out:   serial
Err:   serial
CPU:   Freescale i.MX53 family rev2.1 at 1000 MHz
Reset cause: WDOG
Net:   FEC
Warning: FEC using MAC address from net device

Hit any key to stop autoboot: 0
data abort

MAYBE you should read doc/README.arm-unaligned-accesses

pc : [<aff72220>]          lr : [<aff721fc>]

sp : af565e20 ip : af566918 fp : 00000000
r10: 00000003 r9 : affabb5b r8 : af565f58
r7 : 00000000 r6 : 36747fff r5 : af5668e8 r4 : 36747fff
r3 : af5668ec r2 : af5668eb r1 : 00000000 r0 : af5668e8
Flags: NzcV IRQs off FIQs off Mode SVC_32
Resetting CPU ...

resetting ...

,and this patch fixes it.

Also, even though the ROUND macro is already defined in common.h, the reason for redefining it in image.h is explained by Stefano Babic:

"I will remark a previous comment - even if including common.h seems a good idea to avoid duplications, it makes tools like mkimage to depend on the selected board, because <board>_config must run. Even if this is not a problem for us u-boot developers, it becomes an issue when these tools are included in distros (like u-boot-tools in Ubuntu) and cannot be packaged."

Signed-off-by Troy Kisky <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
Changes since v1:
- Improvec commit log

 include/image.h  |    3 +++
 tools/imximage.c |    9 ++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

include/image.h +3 -0

index f54d983..e1e83b4 100644
@@ -179,6 +179,9 @@
 #define IH_MAGIC	0x27051956	/* Image Magic Number		*/
 #define IH_NMLEN		32	/* Image Name Length		*/
 
+/* Reused from common.h */
+#define ROUND(a, b)		(((a) + (b) - 1) & ~((b) - 1))
+
 /*
  * Legacy format image header,
  * all data in network byte order (aka natural aka bigendian).

tools/imximage.c +8 -1

index 63f88b6..a93d7eb 100644
@@ -515,7 +515,14 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd,
 
 	/* Set the imx header */
 	(*set_imx_hdr)(imxhdr, dcd_len, params->ep, imxhdr->flash_offset);
-	*header_size_ptr = sbuf->st_size + imxhdr->flash_offset;
+
+	/*
+	 * ROM bug alert
+	 * mx53 only loads 512 byte multiples.
+	 * The remaining fraction of a block bytes would
+	 * not be loaded.
+	 */
+	*header_size_ptr = ROUND(sbuf->st_size + imxhdr->flash_offset, 512);
 }
 
 int imximage_check_params(struct mkimage_params *params)

Signature · 1 line

1.7.9.5

Dirk Behme

2013-01-04 06:20 UTC #1 lore raw

PATCH v2 1/1 tools: imximage: Load a size that is multiple of 512

Quoted 3 lines · Fabio Estevam

In order to mx53 ROM to properly load the U-boot image, its header size should be multiple of 512 bytes.

...

Quoted 28 lines

Signed-off-by Troy Kisky <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
Changes since v1:
- Improvec commit log

 include/image.h  |    3 +++
 tools/imximage.c |    9 ++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

include/image.h +3 -0

index f54d983..e1e83b4 100644
@@ -179,6 +179,9 @@
 #define IH_MAGIC	0x27051956	/* Image Magic Number		*/
 #define IH_NMLEN		32	/* Image Name Length		*/
 
+/* Reused from common.h */
+#define ROUND(a, b)		(((a) + (b) - 1) & ~((b) - 1))
+
 /*
  * Legacy format image header,
  * all data in network byte order (aka natural aka bigendian).

tools/imximage.c +0 -0

index 63f88b6..a93d7eb 100644

tools/imximage.c static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, · line 521

Quoted 8 lines

tools/imximage.c +4 -1

@@ -515,7 +515,14 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd,
 
 	/* Set the imx header */
 	(*set_imx_hdr)(imxhdr, dcd_len, params->ep, imxhdr->flash_offset);
-	*header_size_ptr = sbuf->st_size + imxhdr->flash_offset;
+
+	/*
+	 * ROM bug alert
+	 * mx53 only loads 512 byte multiples.

Is this i.MX53 specific or is this valid for i.MX6, too?

Best regards

Dirk

Stefano Babic

2013-01-04 09:03 UTC #2 lore raw

PATCH v2 1/1 tools: imximage: Load a size that is multiple of 512

Quoted 4 lines · Dirk Behme

Quoted 3 lines · Fabio Estevam
In order to mx53 ROM to properly load the U-boot image, its header size should be multiple of 512 bytes.

Hi Dirk,

Quoted 31 lines

...

Quoted 30 lines

Signed-off-by Troy Kisky <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
Changes since v1:
- Improvec commit log

 include/image.h  |    3 +++
 tools/imximage.c |    9 ++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

include/image.h +3 -0

index f54d983..e1e83b4 100644
@@ -179,6 +179,9 @@
 #define IH_MAGIC    0x27051956    /* Image Magic Number        */
 #define IH_NMLEN        32    /* Image Name Length        */
 
+/* Reused from common.h */
+#define ROUND(a, b)        (((a) + (b) - 1) & ~((b) - 1))
+
 /*
  * Legacy format image header,
  * all data in network byte order (aka natural aka bigendian).

tools/imximage.c +0 -0

index 63f88b6..a93d7eb 100644
@@ -515,7 +515,14 @@ static void imximage_set_header(void *ptr, struct
 stat *sbuf, int ifd,

Quoted 9 lines

tools/imximage.c static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, · line 521
Quoted 7 lines
tools/imximage.c +4 -1
     /* Set the imx header */
     (*set_imx_hdr)(imxhdr, dcd_len, params->ep, imxhdr->flash_offset);
-    *header_size_ptr = sbuf->st_size + imxhdr->flash_offset;
+
+    /*
+     * ROM bug alert
+     * mx53 only loads 512 byte multiples.
Is this i.MX53 specific or is this valid for i.MX6, too?

It seems that i.MX6 is not afflicted by this issue. For i.MX6 it adds only some padding to the resulting image.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Fabio Estevam

reviewer

2013-01-05 00:08 UTC #3 lore raw

PATCH v2 1/1 tools: imximage: Load a size that is multiple of 512

Quoted 4 lines · Stefano Babic

Quoted 1 line
Is this i.MX53 specific or is this valid for i.MX6, too?
It seems that i.MX6 is not afflicted by this issue. For i.MX6 it adds only some padding to the resulting image.

Correct, I also tested this change on mx25/mx51/mx6 and they all boot fine.

It is safe to apply this patch into 2012.01.

Regards,

Fabio Estevam

reviewer

2013-01-05 00:16 UTC #4 lore raw

PATCH v2 1/1 tools: imximage: Load a size that is multiple of 512

Quoted 10 lines · Fabio Estevam

Quoted 4 lines · Stefano Babic
Quoted 1 line
Is this i.MX53 specific or is this valid for i.MX6, too?
It seems that i.MX6 is not afflicted by this issue. For i.MX6 it adds only some padding to the resulting image.
Correct, I also tested this change on mx25/mx51/mx6 and they all boot fine.
It is safe to apply this patch into 2012.01.

Sorry, I meant 2013.01

Stefano Babic

2013-01-05 17:10 UTC #5 lore raw

PATCH v2 1/1 tools: imximage: Load a size that is multiple of 512

In reply to Fabio Estevam

Quoted 46 lines · Fabio Estevam

In order to mx53 ROM to properly load the U-boot image, its header size should be multiple of 512 bytes.
This issue was observed with gcc 4.6.2/4.7.3, which caused data aborts:
U-Boot 2013.01-rc2-00172-gf8cfcf1-dirty (Dec 26 2012 - 13:13:28)
Board: MX53 LOCO
I2C:   ready
DRAM:  1 GiB
MMC:   FSL_SDHC: 0, FSL_SDHC: 1
In:    serial
Out:   serial
Err:   serial
CPU:   Freescale i.MX53 family rev2.1 at 1000 MHz
Reset cause: WDOG
Net:   FEC
Warning: FEC using MAC address from net device
Hit any key to stop autoboot: 0
data abort
MAYBE you should read doc/README.arm-unaligned-accesses
pc : [<aff72220>]          lr : [<aff721fc>]
sp : af565e20 ip : af566918 fp : 00000000
r10: 00000003 r9 : affabb5b r8 : af565f58
r7 : 00000000 r6 : 36747fff r5 : af5668e8 r4 : 36747fff
r3 : af5668ec r2 : af5668eb r1 : 00000000 r0 : af5668e8
Flags: NzcV IRQs off FIQs off Mode SVC_32
Resetting CPU ...
resetting ...
,and this patch fixes it.
Also, even though the ROUND macro is already defined in common.h, the reason for redefining it in image.h is explained by Stefano Babic:
"I will remark a previous comment - even if including common.h seems a good idea to avoid duplications, it makes tools like mkimage to depend on the selected board, because <board>_config must run. Even if this is not a problem for us u-boot developers, it becomes an issue when these tools are included in distros (like u-boot-tools in Ubuntu) and cannot be packaged."

Quoted 3 lines

Signed-off-by Troy Kisky <[email protected]>
Signed-off-by Fabio Estevam <[email protected]>
---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================