Dear Wolfgang Thanks for your quick feedback
-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Sunday, July 19, 2009 3:33 AM To: Prafulla Wadaskar Cc: u-boot@lists.denx.de; Manas Saksena; Ronen Shitrit; Nicolas Pitre; Ashish Karkare; Prabhanjan Sarnaik; Lennert Buijtenhek Subject: Re: [U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support
Dear Prafulla Wadaskar,
In message 1247967958-4446-2-git-send-email-prafulla@marvell.com you wrote:
For more details refer docs/README.kwbimage
...
diff --git a/doc/README.kwbimage b/doc/README.kwbimage new
file mode
100644 index 0000000..b00a053 --- /dev/null +++ b/doc/README.kwbimage @@ -0,0 +1,77 @@ +--------------------------------------------- +Kirkwood Boot Image generation using mkimage +---------------------------------------------
+This document describes the U-Boot feature as it is
implemented for
+the Kirkwood family of SoCs.
+The Kirkwood SoC's can boot directly from NAND FLASH, SPI
FLASH, SATA
+etc. using its internal bootRom support.
+for more details refer section 24.2 of Kirkwood functional
specifications.
+ref: www.marvell.com/products/embedded.../kirkwood/index.jsp
+Commad syntax: +./tools/mkimage -n <board specific configuration file> \
-T kwbimage -a <start address> -e<execution address> > +-d <input_raw_binary> <output_kwboot_file> > + > +for ex. > +./tools/mkimage -n ./board/Marvell/openrd_base/kwbimage.cfg \ > + -T kwbimage -a 0x00600000 -e 0x00600000 -d u-boot.bin > +u-boot.kwb > + > +kwimage support available with mkimage utility will generate kirkwood > +boot image that can be flashed on the board nand/spi flash
Maximum line length exceeded. Please fix globally.
Okay I will correct them
...
+Author: Prafulla Wadaskar prafulla@marvell.com +------------------------------------------------------
Please do not add trailing empty lines.
Okay
diff --git a/include/image.h b/include/image.h index
f183757..ed94dda
100644 --- a/include/image.h +++ b/include/image.h @@ -158,6 +158,7 @@ #define IH_TYPE_SCRIPT 6 /* Script file
*/#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image
(any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat
Device Tree Blob */
+#define IH_TYPE_KWBIMAGE 9 /* Boot Rom
Image */
Please s/Boot Rom Image/Kirkwood Boot Rom Image/ or similar.
Okay
diff --git a/tools/kwbimage.c b/tools/kwbimage.c new file
mode 100644
index 0000000..06f256e --- /dev/null +++ b/tools/kwbimage.c ... +/*
- Generates 32 bit checksum
- */
+u32 kwbimage_checksum32(void *start, u32 len, u32 csum) {
- register u32 sum = csum;
- volatile u32 *startp = (volatile u32 *)start;
- do {
sum += *startp;startp++;len -= sizeof(u32);- } while (len > 0);
- return (sum);
+}
This will fail if start is not 32bit aligned; if you can guarantee that it is always aligned, then please pass a u32 pointer.
Yes the pointer is char *, I will add check for 32bit alligned ptr and use of temp buffer to correct this
+void kwdimage_set_ext_header(struct kwb_header *kwbhdr,
char* fname) {
- bhr_t *mhdr = &kwbhdr->kwb_hdr;
- extbhr_t *exthdr = &kwbhdr->kwb_exthdr;
- int fd = -1;
- int lineno, i;
- char *line = NULL;
- char cmdstr[MAX_KWBCMD_LEN], datastr[MAX_KWBDATA_LEN];
- size_t len = 0;
- exthdr->dramregsoffs = (u32)&exthdr->rcfg - (u32)mhdr;
- if ((fd = fopen(fname, "r")) == 0) {
fprintf (stderr, "Err: Can't open %s: %s\n",fname, strerror(errno));exit (EXIT_FAILURE);- }
- i = lineno = 0;
- while ((getline(&line, &len, fd)) != -1) {
/* skip empty lines and lines staring with # */s/staring/starting/
Okay
lineno++;if (!(line[0] != '#' && strlen(line) != 1))continue;This is a bit simple-minded. This will for example fail on DOS-formatted files, and for lines that contain only white space (which still look "empty" to most users and are thus hard to spot).
To take care of Dos formatted file I should use "strlen(line) <= 1" right As explained in doc.README.kwimage, any other line apart from above will be considered as valid configuration line. This is bare minimal parsing provided here which is sufficient
if (strlen(line) > (MAX_KWBCMD_LEN + MAX_KWBDATA_LEN)) {printf("Err.. %s(line no %d) too long\n","Error: %s[%d] Line too long\n" ?
Good one.. I will update this
diff --git a/tools/kwbimage.h b/tools/kwbimage.h new file
mode 100644
index 0000000..c54b701 --- /dev/null +++ b/tools/kwbimage.h ... +/* typedefs */ +typedef char s8; +typedef unsigned char u8;
+typedef int s32; +typedef unsigned int u32;
+typedef short s16; +typedef unsigned short u16;
+typedef long s64; +typedef unsigned long u64;
Please get rid of these.
Okay
diff --git a/tools/mkimage.c b/tools/mkimage.c index
c5b593c..9a11071
100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -24,6 +24,7 @@
#include "mkimage.h" #include <image.h> +#include "kwbimage.h"
extern int errno;
@@ -251,7 +252,13 @@ NXTARG: ; * write dummy header, to be fixed later */ int hdr_size;
- 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 ();- memset (hdr, 0, hdr_size); if (write(ifd, hdr, hdr_size) != hdr_size) { fprintf (stderr, "%s: Write error on %s: %s\n",
@@ -339,6 +346,19 @@ NXTARG: ;
hdr = (image_header_t *)ptr;
- /* Build new header */
- if (opt_type == IH_TYPE_KWBIMAGE) {
checksum = kwbimage_checksum32((void *)ptr,sbuf.st_size, 0);
if (write(ifd, &checksum, sizeof(uint32_t))!= sizeof(uint32_t)) {fprintf (stderr, "%s: Checksum wr erron %s: %s\n",
cmdname, imagefile, strerror(errno));exit (EXIT_FAILURE);}sbuf.st_size += sizeof(uint32_t);kwbimage_set_header (hdr, &sbuf, addr, ep, name);- } else { checksum = crc32 (0, (const char *)(ptr + hdr_size), sbuf.st_size - hdr_size);
@@ -361,6 +381,7 @@ NXTARG: ;
image_print_contents (hdr);
} (void) munmap((void *)ptr, sbuf.st_size);
/* We're a bit of paranoid */
Hmm... it seems you add only image creation code. But "mkimage -l" should work on such an image, too. And "imls" in U-Boot should be working, too.
Well I will disable other generic mkimage options including -l for kwbimage ;-) Can we add this in second part which is not required too? For me great thing is that we can support kwimage generation through mkimage.
Regards.. Prafulla . .
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de "Plan to throw one away. You will anyway." - Fred Brooks, "The Mythical Man Month"