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

Jason Liu
2010-12-31T06:06:24Z

Hi, Stefano,

2010/12/31 Jason Liu <liu.h.jason@gmail.com>:
> Hi, Stefano,
>
> 2010/12/30 Stefano Babic <sbabic@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
>>
>