[U-Boot] [PATCH v3 7/8] imximage: Add MX53 boot image support

[Jason Liu]

Hi, Stefano,

2010/12/31 Jason Liu <liu.h.jason at gmail.com>:
> Hi, Stefano,
>
> 2010/12/30 Stefano Babic <sbabic at denx.de>:
>> On 12/29/2010 01:49 PM, Jason Liu wrote:
>>> This patch add the MX53 boot image support.
>>> @@ -82,44 +213,91 @@ static int imximage_check_image_types(uint8_t type)
>>>  static int imximage_verify_header(unsigned char *ptr, int image_size,
>>>                       struct mkimage_params *params)
>>>  {
>>> -
>>>       struct imx_header *imx_hdr = (struct imx_header *) ptr;
>>> -     flash_header_t *hdr = &imx_hdr->fhdr;
>>> +     flash_header_v1_t *fhdr = &imx_hdr->header.hdr_v1.fhdr;
>>> +
>>> +     if (imx_hdr->imximage_version != IMXIMAGE_V1)
>>> +             return 0;
>>
>> I think this is not correct. mkimage can be called with "-l" option to
>> check the correctness of the produced image without an imximage.cfg
>> file, and from your code it seems to me that actual images are not
>> recognized anymore. In fact, you recognize the header from a version
>> field that you add in the header, but it is not actually provided.
>>
>> You should find the version of the header checking its structure, for
>> example recognizing APP_CODE_BARKER and DCD_BARKER for V1, and for
>> example DCD_HEADER_TAG for V2.
>>
>> Please note that print_header and verify_header are part of the mkimage
>> API (in imximage_params table), and they are called by mkimage
>> independently without parsing any configuration file.
>>
>>> +static void imximage_print_header(const void *ptr)
>>> +{
>>> +     struct imx_header *imx_hdr = (struct imx_header *) ptr;
>>> +
>>> +     switch (imx_hdr->imximage_version) {
>>
>> As I said, this does not work with actual images. The tool should be
>> able to recognize the version directly from its structure (we have
>> enough magic numbers, or better, barkers, to do it), and not storing the
>> version into the header.
>
> OK,  make sense. I get your mean. I will do the change.

But, I did think again about your comments. If we store the version
into the header which
will make the version check more easier, the side-effect is that we
store some boot
ROM useless information into the header, but which should not bring some issues.

mkimage -l can work with the actual images if the version info stored
into the head.

Here is the log I get:

r64343 at r64343-desktop:~/work_space/u-boot-upstream/u-boot$
./tools/mkimage -l u-boot.imx
Image Type:   Freescale IMX Boot Image
Image Ver:    1 (i.MX25/35/51 compatible)
Data Size:    170936 Bytes = 166.93 kB = 0.16 MB
Load Address: 977ff7fc
Entry Point:  97800000

What's you comments?

>
> Thanks for the review.
> Happy New Year!
>
>>
>> Best regards,
>> Stefano Babic
>>
>> --
>> =====================================================================
>> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
>> =====================================================================
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>