[U-Boot] [PATCH] imximage: Fix imximage IVT bug for EIM-NOR boot
From: "Ye.Li" Ye.Li@freescale.com
The load region size of EIM-NOR are defined to 0. For this case, the parameter "imximage_init_loadsize" must be calculated. The imximage tool implements the calculation in the "imximage_generate" function, but the following function "imximage_set_header" resets the value and not calculate. This bug cause some fields of IVT head are not correct, for example the boot_data and DCD overlay the application area.
Signed-off-by: Ye.Li B37916@freescale.com --- tools/imximage.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/imximage.c b/tools/imximage.c index 18dc051..faba238 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -568,6 +568,13 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, /* Parse dcd configuration file */ dcd_len = parse_cfg_file(imxhdr, params->imagename);
+ if (imximage_version == IMXIMAGE_V2) { + if (imximage_init_loadsize < imximage_ivt_offset + + sizeof(imx_header_v2_t)) + imximage_init_loadsize = imximage_ivt_offset + + sizeof(imx_header_v2_t); + } + /* Set the imx header */ (*set_imx_hdr)(imxhdr, dcd_len, params->ep, imximage_ivt_offset);
Hi Ye,
On 20/08/2014 10:55, Ye.Li wrote:
From: "Ye.Li" Ye.Li@freescale.com
The load region size of EIM-NOR are defined to 0. For this case, the parameter "imximage_init_loadsize" must be calculated. The imximage tool implements the calculation in the "imximage_generate" function, but the following function "imximage_set_header" resets the value and not calculate. This bug cause some fields of IVT head are not correct, for example the boot_data and DCD overlay the application area.
Signed-off-by: Ye.Li B37916@freescale.com
tools/imximage.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/imximage.c b/tools/imximage.c index 18dc051..faba238 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -568,6 +568,13 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, /* Parse dcd configuration file */ dcd_len = parse_cfg_file(imxhdr, params->imagename);
- if (imximage_version == IMXIMAGE_V2) {
if (imximage_init_loadsize < imximage_ivt_offset +sizeof(imx_header_v2_t))imximage_init_loadsize = imximage_ivt_offset +sizeof(imx_header_v2_t);- }
Agree that this must be fixed for NOR flash. I am checking where the fix should be done.
As far as I see, there are multiple entry involved in the computation:
387 if (imximage_init_loadsize < imximage_ivt_offset) 388 imximage_init_loadsize = imximage_ivt_offset;
It seems more logical that the check should be here.
The other point is:
#define FLASH_LOADSIZE_NOR 0x0 /* entire image */
Well, if the comment is correct (with NOR the SOC can load the entire image and there is no limit as with other storage), setting it to "0" has nothing to do with it. Then it seems to me clearer something at line 372:
if (!imximage_init_loadsize && imximage_version == IMXIMAGE_V2) imximage_init_loadsize = imximage_ivt_offset + sizeof(imx_header_v2_t);
What do you think ?
Best regards, Stefano Babic
Hi Stefano Babic,
On 8/20/2014 5:44 PM, Stefano Babic wrote:
Hi Ye,
On 20/08/2014 10:55, Ye.Li wrote:
From: "Ye.Li" Ye.Li@freescale.com
The load region size of EIM-NOR are defined to 0. For this case, the parameter "imximage_init_loadsize" must be calculated. The imximage tool implements the calculation in the "imximage_generate" function, but the following function "imximage_set_header" resets the value and not calculate. This bug cause some fields of IVT head are not correct, for example the boot_data and DCD overlay the application area.
Signed-off-by: Ye.Li B37916@freescale.com
tools/imximage.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/imximage.c b/tools/imximage.c index 18dc051..faba238 100644 --- a/tools/imximage.c +++ b/tools/imximage.c @@ -568,6 +568,13 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, /* Parse dcd configuration file */ dcd_len = parse_cfg_file(imxhdr, params->imagename);
- if (imximage_version == IMXIMAGE_V2) {
if (imximage_init_loadsize < imximage_ivt_offset +sizeof(imx_header_v2_t))imximage_init_loadsize = imximage_ivt_offset +sizeof(imx_header_v2_t);- }
Agree that this must be fixed for NOR flash. I am checking where the fix should be done.
As far as I see, there are multiple entry involved in the computation:
387 if (imximage_init_loadsize < imximage_ivt_offset) 388 imximage_init_loadsize = imximage_ivt_offset;
It seems more logical that the check should be here.
The other point is:
#define FLASH_LOADSIZE_NOR 0x0 /* entire image */
Well, if the comment is correct (with NOR the SOC can load the entire image and there is no limit as with other storage), setting it to "0" has nothing to do with it. Then it seems to me clearer something at line 372:
if (!imximage_init_loadsize && imximage_version == IMXIMAGE_V2) imximage_init_loadsize = imximage_ivt_offset + sizeof(imx_header_v2_t);
What do you think ?
Best regards, Stefano Babic
There are two minor impacts if putting the check in the function "parse_cfg_cmd":
1. The "imximage_version" must be got before parsing a CMD_BOOT_FROM command. This compels the CMD_IMAGE_VERSION preceding the CMD_BOOT_FROM in script. imximage_init_loadsize is only needed by V2 version.
2. Since the "imximage_generate" function already implements post fixing for imximage_init_loadsize, this post fixing needs be removed.
Actually, putting the check in the parsing or post the parsing are ok for me. Both can resolve the issue. The comment for "FLASH_LOADSIZE_NOR" sources from iMX reference manual, it is correct.
Best regards, Ye Li
Hi Ye,
On 21/08/2014 09:04, Li Ye-B37916 wrote:
There are two minor impacts if putting the check in the function "parse_cfg_cmd":
- The "imximage_version" must be got before parsing a CMD_BOOT_FROM
command. This compels the CMD_IMAGE_VERSION preceding the CMD_BOOT_FROM in script. imximage_init_loadsize is only needed by V2 version.
I know abot this, and this is a minor impact. It is common that a version number of a document must be set first in the document.
- Since the "imximage_generate" function already implements post
fixing for imximage_init_loadsize, this post fixing needs be removed.
Actually, putting the check in the parsing or post the parsing are ok for me. Both can resolve the issue. The comment for "FLASH_LOADSIZE_NOR" sources from iMX reference manual, it is correct.
Well, I think it is clear for both of us because we worked with i-IM images. But setting a size to 0x0 with the comment that this is the size of whole image can be confusing (I think I was the author of this comment, so I am guilty for that). Anyway, it is a very minor issue.
I hope that people will read carefully the manual together with code.
Anyway, comments are very minor issues - I will apply the patch.
Best regards, Stefano Babic
participants (3)
-
Li Ye-B37916 -
Stefano Babic -
Ye.Li