imximage: Fix imximage IVT bug for EIM-NOR boot
PATCH 1/1 imximage: Fix imximage IVT bug for EIM-NOR boot
From: "Ye.Li" <[email protected]>
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.
--- tools/imximage.c | 7 +++++++ 1 file changed, 7 insertions(+)
tools/imximage.c +7 -0
@@ -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);
Signature · 1 line
1.7.9.5
PATCH 1/1 imximage: Fix imximage IVT bug for EIM-NOR boot
Hi Ye,
Quoted 19 lines · Ye.Li
From: "Ye.Li" <[email protected]>
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 theboot_dataand DCD overlay the application area.Signed-off-by Ye.Li <[email protected]>--- tools/imximage.c | 7 +++++++ 1 file changed, 7 insertions(+)tools/imximage.c +0
-0
tools/imximage.c static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, · line 577
Quoted 10 lines
tools/imximage.c +7
-0@@ -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
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: [email protected] =====================================================================
PATCH 1/1 imximage: Fix imximage IVT bug for EIM-NOR boot
Hi Stefano Babic,
Quoted 51 lines · Stefano Babic
Hi Ye,
Quoted 19 lines · Ye.Li
From: "Ye.Li" <[email protected]>
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 theboot_dataand DCD overlay the application area.Signed-off-by Ye.Li <[email protected]>--- tools/imximage.c | 7 +++++++ 1 file changed, 7 insertions(+)tools/imximage.c +0
-0tools/imximage.c static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd, · line 577
Quoted 10 lines
tools/imximage.c +7
-0@@ -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)
388imximage_init_loadsize=imximage_ivt_offset;It seems more logical that the check should be here.
The other point is:
#define
FLASH_LOADSIZE_NOR0x0/* 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:
Quoted 12 lines
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
PATCH 1/1 imximage: Fix imximage IVT bug for EIM-NOR boot
Hi Ye,
Quoted 7 lines · 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":1. The "
imximage_version" must be got before parsing aCMD_BOOT_FROMcommand. This compels theCMD_IMAGE_VERSIONpreceding theCMD_BOOT_FROMin script.imximage_init_loadsizeis 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.
Quoted 9 lines
2. Since the "
imximage_generate" function already implements post fixing forimximage_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
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 =====================================================================