Hi Martin,
On 02/01/2017 22:24, Martin Kaiser wrote:
> From: Martin Kaiser <mar...@kaiser.cx>
>
> We can use the same header length calculations for both imximage v1 and
> v2. This addresses TODO comments about imximage v1 in the current code.
>
I see, absoulutely a good idea.
> With this patch applied, *header_size_ptr in imximage_set_header() will
> have the correct value for both imximage v1 and v2. This is necessary
> for people wanting to add proprietary data behind the created imximage.
>
> Signed-off-by: Martin Kaiser <mar...@kaiser.cx>
> Cc: sba...@denx.de
> ---
>
> I discovered the problem when I tried to use csf_ptr with imximage v1
> (as part of a private modification).
>
> *csf_ptr = params->ep + *header_size_ptr - imximage_init_loadsize;
> = params->ep + sbuf->st_size +
> imximage_ivt_offset - imximage_init_loadsize;
> = params->ep + padded data file size + alloc_len -
> (imximage_init_loadsize - imximage_ivt_offset);
>
> This works only if
> alloc_len == imximage_init_loadsize - imximage_ivt_offset,
> not if alloc_len is always 4096.
>
I remember that this was a "reasonable" value that initially avoided
overlapping. But it was not computed.
> I rebased my tree to make sure that the patch still applies against
> current master. I'd really appreciate any feedback, this has been
> pending since September.
>
> tools/imximage.c | 38 ++++++++++++++++----------------------
> 1 file changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/tools/imximage.c b/tools/imximage.c
> index 2cd8d88..0c43196 100644
> --- a/tools/imximage.c
> +++ b/tools/imximage.c
> @@ -300,8 +300,7 @@ static void set_imx_hdr_v1(struct imx_header *imxhdr,
> uint32_t dcd_len,
> /* Set magic number */
> fhdr_v1->app_code_barker = APP_CODE_BARKER;
>
> - /* TODO: check i.MX image V1 handling, for now use 'old' style */
> - hdr_base = entry_point - 4096;
> + hdr_base = entry_point - imximage_init_loadsize + flash_offset;
> fhdr_v1->app_dest_ptr = hdr_base - flash_offset;
> fhdr_v1->app_code_jump_vector = entry_point;
>
> @@ -833,18 +832,19 @@ 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_version == IMXIMAGE_V1)
> + header_size = sizeof(flash_header_v1_t);
> + else {
> header_size = sizeof(flash_header_v2_t) + sizeof(boot_data_t);
> if (!plugin_image)
> header_size += sizeof(dcd_v2_t);
> else
> header_size += MAX_PLUGIN_CODE_SIZE;
> -
> - if (imximage_init_loadsize < imximage_ivt_offset + header_size)
> - imximage_init_loadsize = imximage_ivt_offset +
> - header_size;
> }
>
> + if (imximage_init_loadsize < imximage_ivt_offset + header_size)
> + imximage_init_loadsize = imximage_ivt_offset +
> header_size;
> +
> /* Set the imx header */
> (*set_imx_hdr)(imxhdr, dcd_len, params->ep, imximage_ivt_offset);
>
> @@ -913,23 +913,21 @@ static int imximage_generate(struct image_tool_params
> *params,
> /* Parse dcd configuration file */
> parse_cfg_file(&imximage_header, params->imagename);
>
> - /* TODO: check i.MX image V1 handling, for now use 'old' style */
> - if (imximage_version == IMXIMAGE_V1) {
> - alloc_len = 4096;
> - header_size = 4096;
> - } else {
> + if (imximage_version == IMXIMAGE_V1)
> + header_size = sizeof(imx_header_v1_t);
> + else {
> header_size = sizeof(flash_header_v2_t) + sizeof(boot_data_t);
> if (!plugin_image)
> header_size += sizeof(dcd_v2_t);
> else
> header_size += MAX_PLUGIN_CODE_SIZE;
> -
> - if (imximage_init_loadsize < imximage_ivt_offset + header_size)
> - imximage_init_loadsize = imximage_ivt_offset +
> - header_size;
> - alloc_len = imximage_init_loadsize - imximage_ivt_offset;
> }
>
> + if (imximage_init_loadsize < imximage_ivt_offset + header_size)
> + imximage_init_loadsize = imximage_ivt_offset +
> header_size;
> +
> + alloc_len = imximage_init_loadsize - imximage_ivt_offset;
> +
> if (alloc_len < header_size) {
> fprintf(stderr, "%s: header error\n",
> params->cmdname);
> @@ -959,11 +957,7 @@ static int imximage_generate(struct image_tool_params
> *params,
>
> pad_len = ROUND(sbuf.st_size, 4096) - sbuf.st_size;
>
> - /* TODO: check i.MX image V1 handling, for now use 'old' style */
> - if (imximage_version == IMXIMAGE_V1)
> - return 0;
> - else
> - return pad_len;
> + return pad_len;
> }
IMHO it is ok and it was pending since a lot of time. I merge it.
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
