-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Wednesday, July 22, 2009 2:06 PM To: Prafulla Wadaskar Cc: u-boot@lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik Subject: Re: [U-Boot] [PATCH v2 2/4] tools: mkimage (type=kwbimage) kirkwood boot image support
<snip>
if (datacmd_cnt > KWBIMAGE_MAX_CONFIG ) {printf("Err.. found more than max ""allowed(%d) configurations\n",KWBIMAGE_MAX_CONFIG);Where is this limit coming from? Can the be arbitrarily changed, or is it some inherent linif of your image format?
This limit is coming from the BootROM header size on kirkwood.
<snip>
@@ -164,6 +165,10 @@ NXTARG: ; (lflag && (dflag || fflag))) usage();
- if (((argc != 1) && (opt_type == IH_TYPE_KWBIMAGE)) &&
(xflag || lflag))usage();Do you promise to add support for listing the image (lflag) later?
Yes, I have already done it :-)
if (!eflag) { ep = addr; /* If XIP, entry point must be after the U-Boot
header */
@@ -251,7 +256,12 @@ NXTARG: ; * * write dummy header, to be fixed later */
- hdr_size = image_get_header_size ();
- if (opt_type == IH_TYPE_KWBIMAGE) {
hdr = kwbimage_get_header_ptr();hdr_size = kwbimage_get_header_size ();- } else
hdr_size = image_get_header_size ();Maybe we should use a function pointer for image_get_header_size() which can be set just once, so we don't have to change the code?
<snip>
sbuf.st_size += sizeof(uint32_t);kwbimage_set_header (hdr, &sbuf, addr, ep, name);- } else { checksum = crc32 (0,
Here indentation becomes wrong.
And I don't like this if()...else... thingy here. I think we should move the respective code into functions, and just set a pointer to the function the builds the header.
This prevents an ever growing list of if()...else...else...else... in case more image formats need to get added.
That's good idea. My initial intention was not to disturb mkimage code much. But doing this will make more easier for growing support. I will have to restructure my code and mkimage code too. I will do it.
Regards.. Prafulla . .