From mboxrd@z Thu Jan  1 00:00:00 1970
From: Li Ye-B37916 <b37916@freescale.com>
Date: Thu, 21 Aug 2014 15:04:11 +0800
Subject: [U-Boot] [PATCH] imximage: Fix imximage IVT bug for EIM-NOR boot
In-Reply-To: <53F46DE7.2050504@denx.de>
References: <1408524932-26712-1-git-send-email-Ye.Li@freescale.com>
	<53F46DE7.2050504@denx.de>
Message-ID: <53F599EB.5030607@freescale.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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