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.

...

+Author: Prafulla Wadaskar prafulla@marvell.com +------------------------------------------------------

Please do not add trailing empty lines.

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.

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.

+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/

• 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).

• 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" ?

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.

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 err on %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.

Best regards,

Wolfgang Denk