| Message ID | 1408524932-26712-1-git-send-email-Ye.Li@freescale.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Stefano Babic |
| Headers | show |
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": > > 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. 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. > > 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. 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
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);