Hi, Stefano, 2010/12/31 Jason Liu <[email protected]>: > Hi, Stefano, > > 2010/12/30 Stefano Babic <[email protected]>: >> 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: r64...@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: [email protected] >> ===================================================================== >> _______________________________________________ >> U-Boot mailing list >> [email protected] >> http://lists.denx.de/mailman/listinfo/u-boot >> > _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
