[U-Boot] [PATCH] imximage: Fix imximage IVT bug for EIM-NOR boot

Li Ye-B37916
2014-08-21T07:04:11Z

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