[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Jean-Christophe PLAGNIOL-VILLARD]

On 07:15 Sun 19 Jul     , Prafulla Wadaskar wrote:
> For more details refer docs/README.kwbimage
> 
> Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
> ---
>  Makefile            |    5 ++
>  common/image.c      |    1 +
>  doc/README.kwbimage |   77 ++++++++++++++++++++++
>  include/image.h     |    1 +
>  tools/Makefile      |    1 +
>  tools/kwbimage.c    |  181 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/kwbimage.h    |  105 +++++++++++++++++++++++++++++
>  tools/mkimage.c     |   23 ++++++-
>  8 files changed, 393 insertions(+), 1 deletions(-)
>  create mode 100644 doc/README.kwbimage
>  create mode 100644 tools/kwbimage.c
>  create mode 100644 tools/kwbimage.h
> 
> diff --git a/Makefile b/Makefile
> index 55fc2a3..b9e45ed 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -339,6 +339,10 @@ $(obj)u-boot.img:	$(obj)u-boot.bin
>  			sed -e 's/"[	 ]*$$/ for $(BOARD) board"/') \
>  		-d $< $@
>  
> +$(obj)u-boot.kwb:	$(obj)u-boot.bin
> +		./tools/mkimage -n ./board/$(BOARDDIR)/kwbimage.cfg \
> +		-T kwbimage -a $(TEXT_BASE) -e $(TEXT_BASE) -d $< $@
It will be better to make it a few more generic (and btw fix the out of tree
support) as this
MKIMAGE = $(OBJTREE)/tools/mkimage

$(obj)u-boot.kwb:	$(obj)u-boot.bin
		$(MKIMAGE) -n $(KWD_CONFIG) -T kwbimage -a $(TEXT_BASE) \
		-e $(TEXT_BASE) -d $< $@

and in the board config.mk or soc anyother config.mk
KWD_CONFIG = $(SRCTREE)/board/$(BOARDDIR)/kwbimage.cfg

Best Regards,
J.

[U-Boot] [PATCH 3/3] Kirkwood: Sheevaplug: kwimage configuration

[Jean-Christophe PLAGNIOL-VILLARD]

> +
> +# Boot Media configurations
> +BOOT_FROM	nand
> +NAND_ECC_MODE	default
> +NAND_PAGE_SIZE	0x0800
> +
> +# SOC registers configuration for initial values using bootrom header extension
> +# Maximum KWBIMAGE_MAX_CONFIG configurations allowed
> +
> +# Configure RGMII-0 interface pad voltage to 1.8V
> +0xFFD100e0 0x1b1b1b9b
> +# DRAM Configuration
> +0xFFD01400 0x43000c30
could you give more details about the configs

Best Regards,
J.

[U-Boot] [PATCH 1/3] tools: mkimage: hdr_size used to facilitate customized support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1247967958-4446-1-git-send-email-prafulla@marvell.com> you wrote:
> hdr_size variable is initialized
> at the start of image creation algorithm instead of reading it each time.
> This facilitate to use the common code for other image type implementations
> for ex. kwbimage

Hm... I have no idea what you are trying to do, or why. You are aware
that image_get_header_size()  is  a  static  inline,  returning  just
"sizeof (image_header_t)"?

As far as the context of mkimage goes, you could as well consider it a
constant.

> Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
> ---
>  tools/mkimage.c |   18 ++++++++----------
>  1 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index 967fe9a..c5b593c 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -250,9 +250,10 @@ NXTARG:		;
>  	 *
>  	 * write dummy header, to be fixed later
>  	 */
> -	memset (hdr, 0, image_get_header_size ());
> -
> -	if (write(ifd, hdr, image_get_header_size ()) != image_get_header_size ()) {
> +	int hdr_size;

We don't allow variable declarations in the middle of the code.


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 at denx.de
Objects in mirror are closer than they appear.

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Wolfgang Denk]

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

-- 
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 at denx.de
"Plan to throw one away. You will anyway."
                              - Fred Brooks, "The Mythical Man Month"

[U-Boot] [PATCH 1/3] tools: mkimage: hdr_size used to facilitate customized support

[Prafulla Wadaskar]

hdr_size variable is initialized
at the start of image creation algorithm instead of reading it each time.
This facilitate to use the common code for other image type implementations
for ex. kwbimage

Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
 tools/mkimage.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 967fe9a..c5b593c 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -250,9 +250,10 @@ NXTARG:		;
 	 *
 	 * write dummy header, to be fixed later
 	 */
-	memset (hdr, 0, image_get_header_size ());
-
-	if (write(ifd, hdr, image_get_header_size ()) != image_get_header_size ()) {
+	int hdr_size;
+	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",
 			cmdname, imagefile, strerror(errno));
 		exit (EXIT_FAILURE);
@@ -339,14 +340,12 @@ NXTARG:		;
 	hdr = (image_header_t *)ptr;
 
 	checksum = crc32 (0,
-			  (const char *)(ptr + image_get_header_size ()),
-			  sbuf.st_size - image_get_header_size ()
-			 );
-
+			(const char *)(ptr + hdr_size),
+			sbuf.st_size - hdr_size);
 	/* Build new header */
 	image_set_magic (hdr, IH_MAGIC);
 	image_set_time (hdr, sbuf.st_mtime);
-	image_set_size (hdr, sbuf.st_size - image_get_header_size ());
+	image_set_size (hdr, sbuf.st_size - hdr_size);
 	image_set_load (hdr, addr);
 	image_set_ep (hdr, ep);
 	image_set_dcrc (hdr, checksum);
@@ -354,10 +353,9 @@ NXTARG:		;
 	image_set_arch (hdr, opt_arch);
 	image_set_type (hdr, opt_type);
 	image_set_comp (hdr, opt_comp);
-
 	image_set_name (hdr, name);
 
-	checksum = crc32 (0, (const char *)hdr, image_get_header_size ());
+	checksum = crc32 (0, (const char *)hdr, hdr_size);
 
 	image_set_hcrc (hdr, checksum);
 
-- 
1.5.3.4

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Prafulla Wadaskar]

For more details refer docs/README.kwbimage

Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
 Makefile            |    5 ++
 common/image.c      |    1 +
 doc/README.kwbimage |   77 ++++++++++++++++++++++
 include/image.h     |    1 +
 tools/Makefile      |    1 +
 tools/kwbimage.c    |  181 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/kwbimage.h    |  105 +++++++++++++++++++++++++++++
 tools/mkimage.c     |   23 ++++++-
 8 files changed, 393 insertions(+), 1 deletions(-)
 create mode 100644 doc/README.kwbimage
 create mode 100644 tools/kwbimage.c
 create mode 100644 tools/kwbimage.h

diff --git a/Makefile b/Makefile
index 55fc2a3..b9e45ed 100644
--- a/Makefile
+++ b/Makefile
@@ -339,6 +339,10 @@ $(obj)u-boot.img:	$(obj)u-boot.bin
 			sed -e 's/"[	 ]*$$/ for $(BOARD) board"/') \
 		-d $< $@
 
+$(obj)u-boot.kwb:	$(obj)u-boot.bin
+		./tools/mkimage -n ./board/$(BOARDDIR)/kwbimage.cfg \
+		-T kwbimage -a $(TEXT_BASE) -e $(TEXT_BASE) -d $< $@
+
 $(obj)u-boot.sha1:	$(obj)u-boot.bin
 		$(obj)tools/ubsha1 $(obj)u-boot.bin
 
@@ -3638,6 +3642,7 @@ clobber:	clean
 	@rm -f $(OBJS) $(obj)*.bak $(obj)ctags $(obj)etags $(obj)TAGS \
 		$(obj)cscope.* $(obj)*.*~
 	@rm -f $(obj)u-boot $(obj)u-boot.map $(obj)u-boot.hex $(ALL)
+	@rm -f $(obj)u-boot.kwb
 	@rm -f $(obj)tools/{env/crc32.c,inca-swap-bytes}
 	@rm -f $(obj)cpu/mpc824x/bedbug_603e.c
 	@rm -f $(obj)include/asm/proc $(obj)include/asm/arch $(obj)include/asm
diff --git a/common/image.c b/common/image.c
index e22c974..845006f 100644
--- a/common/image.c
+++ b/common/image.c
@@ -145,6 +145,7 @@ static table_entry_t uimage_type[] = {
 	{	IH_TYPE_SCRIPT,     "script",	  "Script",		},
 	{	IH_TYPE_STANDALONE, "standalone", "Standalone Program", },
 	{	IH_TYPE_FLATDT,     "flat_dt",    "Flat Device Tree",	},
+	{	IH_TYPE_KWBIMAGE,   "kwbimage",   "Kirkwood Boot Image",},
 	{	-1,		    "",		  "",			},
 };
 
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
+
+Board specific configuration file specifications:-
+1. This file must present in the $(BOARDDIR) and the name should be kwbimage.cfg (since this is used in Makefile)
+2. This file can have empty lines and lines starting with "#" as first character to put comments
+3. This file can have configuration command lines as mentioned below, any other information in this file is treated as invalid.
+
+Configuration command line syntax:
+1. Each command line is must have two strings, first one command or address and second one data string
+2. Following are the valid command strings and data strings associated combinations
+	Command string		data string
+	BOOT_FROM		nand/spi/sata
+	NAND_ECC_MODE		default/rs/hamming/disabled
+	NAND_PAGE_SIZE		any u16 hex value
+	SATA_PIO_MODE		any u32 hex value
+	DDR_INIT_DELAY		any u32 hex value
+	<u32 register address>	<u32 vlaue for it>, you can have maximum 55 such commands
+3. all commands are optional to program
+
+Typical example of kwimage.cfg file:-
+
+# Boot Media configurations
+BOOT_FROM	nand
+NAND_ECC_MODE	default
+NAND_PAGE_SIZE	0x0800
+
+# Configure RGMII-0 interface pad voltage to 1.8V
+0xFFD100e0 0x1b1b1b9b
+# DRAM Configuration
+0xFFD01400 0x43000c30
+0xFFD01404 0x37543000
+0xFFD01408 0x22125451
+0xFFD0140C 0x00000a33
+0xFFD01410 0x000000cc
+0xFFD01414 0x00000000
+0xFFD01418 0x00000000
+0xFFD0141C 0x00000C52
+0xFFD01420 0x00000040
+0xFFD01424 0x0000F17F
+0xFFD01428 0x00085520
+0xFFD0147C 0x00008552
+0xFFD01504 0x0FFFFFF1
+0xFFD01508 0x10000000
+0xFFD0150C 0x0FFFFFF5
+0xFFD01514 0x00000000
+0xFFD0151C 0x00000000
+0xFFD01494 0x00030000
+0xFFD01498 0x00000000
+0xFFD0149C 0x0000E803
+0xFFD01480 0x00000001
+# End of Header extension
+0x0 0x0
+
+Author: Prafulla Wadaskar <prafulla@marvell.com>
+------------------------------------------------------
+
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		*/
 
 /*
  * Compression Types
diff --git a/tools/Makefile b/tools/Makefile
index 43c284c..5ead74c 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -167,6 +167,7 @@ $(obj)img2srec$(SFX):	$(obj)img2srec.o
 	$(STRIP) $@
 
 $(obj)mkimage$(SFX):	$(obj)mkimage.o $(obj)crc32.o $(obj)image.o $(obj)md5.o \
+			$(obj)kwbimage.o \
 			$(obj)sha1.o $(LIBFDT_OBJS) $(obj)os_support.o
 	$(CC) $(CFLAGS) $(HOST_LDFLAGS) -o $@ $^
 	$(STRIP) $@
diff --git a/tools/kwbimage.c b/tools/kwbimage.c
new file mode 100644
index 0000000..06f256e
--- /dev/null
+++ b/tools/kwbimage.c
@@ -0,0 +1,181 @@
+/*
+ * (C) Copyright 2008
+ * Marvell Semiconductor <www.marvell.com>
+ * Written-by: Prafulla Wadaskar <prafulla@marvell.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include "kwbimage.h"
+
+struct kwb_header kwbimage_header;
+/*
+ * Generates 8 bit checksum
+ */
+u8 kwbimage_checksum8(void *start, u32 len, u8 csum)
+{
+	register u8 sum = csum;
+	volatile u8 *startp = (volatile u8 *)start;
+
+	do {
+		sum += *startp;
+		startp++;
+	} while (--len);
+	return (sum);
+}
+
+/*
+ * 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);
+}
+
+void *kwbimage_get_header_ptr (void)
+{
+	return ((void *)&kwbimage_header);
+}
+
+int kwbimage_get_header_size (void)
+{
+	return (sizeof (struct kwb_header));
+}
+
+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 # */
+		lineno++;
+		if (!(line[0] != '#' && strlen(line) != 1))
+			continue;
+
+		if (strlen(line) > (MAX_KWBCMD_LEN + MAX_KWBDATA_LEN)) {
+			printf("Err.. %s(line no %d) too long\n",
+				fname, lineno);
+			exit (EXIT_FAILURE);
+		}
+
+		sscanf(line, "%s %s\n", cmdstr, datastr);
+		if (strcmp(cmdstr, "BOOT_FROM") == 0) {
+			if (strcmp(datastr, "spi") == 0) {
+				mhdr->blockid = IBR_HDR_SPI_ID;
+			} else if (strcmp(datastr, "nand") == 0) {
+				mhdr->blockid = IBR_HDR_NAND_ID;
+			} else if (strcmp(datastr, "sata") == 0) {
+				mhdr->blockid = IBR_HDR_SATA_ID;
+			} else {
+				printf("Err.. Invalid kwimage data\n");
+				exit (EXIT_FAILURE);
+			}
+			printf("Preparing kirkwood boot image to boot from %s\n", datastr);
+		} else if (strcmp(cmdstr, "NAND_ECC_MODE") == 0) {
+			if (strcmp(datastr, "default") == 0) {
+				mhdr->nandeccmode = IBR_HDR_ECC_DEFAULT;
+			} else if (strcmp(datastr, "hamming") == 0) {
+				mhdr->nandeccmode = IBR_HDR_ECC_FORCED_HAMMING;
+			} else if (strcmp(datastr, "rs") == 0) {
+				mhdr->nandeccmode = IBR_HDR_ECC_FORCED_RS;
+			} else if (strcmp(datastr, "disabled") == 0) {
+				mhdr->nandeccmode = IBR_HDR_ECC_DISABLED;
+			} else {
+				printf("Err.. Invalid kwimage data\n");
+				exit (EXIT_FAILURE);
+			}
+			printf("Nand ECC mode = %s\n", datastr);
+		} else if (strcmp(cmdstr, "NAND_PAGE_SIZE") == 0) {
+			mhdr->nandpagesize = strtoul (datastr, line, 16);
+			printf("Nand page size = %x\n", mhdr->nandpagesize);
+		} else if (strcmp(cmdstr, "SATA_PIO_MODE") == 0) {
+			mhdr->satapiomode = strtoul (datastr, line, 16);
+			printf("Sata PIO mode = %x\n", mhdr->satapiomode);
+		} else if (strcmp(cmdstr, "DDR_INIT_DELAY") == 0) {
+			mhdr->ddrinitdelay = strtoul (datastr, line, 16);
+			printf("DDR init delay = %d msec\n",
+						mhdr->ddrinitdelay);
+		} else {
+			exthdr->rcfg[i].raddr = strtoul (cmdstr, line, 16);
+			exthdr->rcfg[i].rdata = strtoul (datastr, line, 16);
+			if (i > KWBIMAGE_MAX_CONFIG ) {
+				printf("Err.. found more than max "
+					"allowed(%d) configurations\n",
+					KWBIMAGE_MAX_CONFIG);
+				exit (EXIT_FAILURE);
+			} else
+				i++;
+		}
+	}
+	if (line)
+		free(line);
+
+	fclose(fd);
+}
+
+void kwbimage_set_header (struct kwb_header *hdr, struct stat *sbuf,
+					u32 dadr, u32 eadr, char *fname)
+{
+	bhr_t *mhdr = &hdr->kwb_hdr;
+	extbhr_t *exthdr = &hdr->kwb_exthdr;
+
+	mhdr->blocksize = sbuf->st_size - kwbimage_get_header_size();
+	mhdr->srcaddr = kwbimage_get_header_size();
+	mhdr->destaddr= dadr;
+	mhdr->execaddr =eadr;
+	mhdr->ext = 0x1; /* header extension appended */
+
+	kwdimage_set_ext_header(hdr, fname);
+	/* calculate checksums */
+	mhdr->checkSum = kwbimage_checksum8((void *)mhdr, sizeof(bhr_t), 0);
+	exthdr->checkSum = kwbimage_checksum8((void *)exthdr,
+						sizeof(extbhr_t),0);
+}
+
diff --git a/tools/kwbimage.h b/tools/kwbimage.h
new file mode 100644
index 0000000..c54b701
--- /dev/null
+++ b/tools/kwbimage.h
@@ -0,0 +1,105 @@
+/*
+ * (C) Copyright 2008
+ * Marvell Semiconductor <www.marvell.com>
+ * iWritten-by: Prafulla Wadaskar <prafulla@marvell.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _KWBIMAGE_H_
+#define _KWBIMAGE_H_
+
+#define KWBIMAGE_MAX_CONFIG	((0x1dc - 0x20)/sizeof(struct reg_config))
+#define MAX_KWBCMD_LEN		32
+#define MAX_KWBDATA_LEN		32
+
+/* NAND ECC Mode */
+#define IBR_HDR_ECC_DEFAULT		0x00
+#define IBR_HDR_ECC_FORCED_HAMMING	0x01
+#define IBR_HDR_ECC_FORCED_RS  		0x02
+#define IBR_HDR_ECC_DISABLED  		0x03
+
+/* Boot Type - block ID */
+#define IBR_HDR_I2C_ID			0x4D
+#define IBR_HDR_SPI_ID			0x5A
+#define IBR_HDR_NAND_ID			0x8B
+#define IBR_HDR_SATA_ID			0x78
+#define IBR_HDR_PEX_ID			0x9C
+#define IBR_HDR_UART_ID			0x69
+#define IBR_DEF_ATTRIB	 		0x00
+
+/* 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;
+
+typedef struct bhr_t {
+	u8 blockid;		/*0     */
+	u8 nandeccmode;		/*1     */
+	u16 nandpagesize;	/*2-3   */
+	u32 blocksize;		/*4-7   */
+	u32 rsvd1;		/*8-11  */
+	u32 srcaddr;		/*12-15 */
+	u32 destaddr;		/*16-19 */
+	u32 execaddr;		/*20-23 */
+	u8 satapiomode;		/*24    */
+	u8 rsvd3;		/*25    */
+	u16 ddrinitdelay;	/*26-27 */
+	u16 rsvd2;		/*28-29 */
+	u8 ext;			/*30    */
+	u8 checkSum;		/*31    */
+} bhr_t, *pbhr_t;
+
+struct reg_config {
+	u32 raddr;
+	u32 rdata;
+};
+
+typedef struct extbhr_t {
+	u32 dramregsoffs;
+	u8 rsrvd1[0x20 - sizeof(u32)];
+	struct reg_config rcfg[KWBIMAGE_MAX_CONFIG];
+	u8 rsrvd2[7];
+	u8 checkSum;
+} extbhr_t, *pextbhr_t;
+
+struct kwb_header {
+	bhr_t kwb_hdr;
+	extbhr_t kwb_exthdr;
+};
+
+/*
+ * functions
+ */
+u8 kwbimage_checksum8(void *start, u32 len, u8 csum);
+u32 kwbimage_checksum32(void *start, u32 len, u32 csum);
+void *kwbimage_get_header_ptr(void);
+int kwbimage_get_header_size (void);
+void kwbimage_set_header (struct kwb_header *hdr, struct stat *sbuf,
+					u32 dadr, u32 eadr, char *fname);
+
+#endif /* _KWBIMAGE_H_ */
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 */
-- 
1.5.3.4

[U-Boot] [PATCH 3/3] Kirkwood: Sheevaplug: kwimage configuration

[Prafulla Wadaskar]

Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
 board/Marvell/sheevaplug/kwbimage.cfg |   61 +++++++++++++++++++++++++++++++++
 1 files changed, 61 insertions(+), 0 deletions(-)
 create mode 100644 board/Marvell/sheevaplug/kwbimage.cfg

diff --git a/board/Marvell/sheevaplug/kwbimage.cfg b/board/Marvell/sheevaplug/kwbimage.cfg
new file mode 100644
index 0000000..874dcd2
--- /dev/null
+++ b/board/Marvell/sheevaplug/kwbimage.cfg
@@ -0,0 +1,61 @@
+#
+# (C) Copyright 2009
+# Marvell Semiconductor <www.marvell.com>
+# Written-by: Prafulla Wadaskar <prafulla@marvell.com>
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA 02110-1301 USA
+#
+# Refer docs/README.kwimage for more details about how-to configure
+# and create kirkwood boot image
+#
+
+# Boot Media configurations
+BOOT_FROM	nand
+NAND_ECC_MODE	default
+NAND_PAGE_SIZE	0x0800
+
+# SOC registers configuration for initial values using bootrom header extension
+# Maximum KWBIMAGE_MAX_CONFIG configurations allowed
+
+# Configure RGMII-0 interface pad voltage to 1.8V
+0xFFD100e0 0x1b1b1b9b
+# DRAM Configuration
+0xFFD01400 0x43000c30
+0xFFD01404 0x37543000
+0xFFD01408 0x22125451
+0xFFD0140C 0x00000a33
+0xFFD01410 0x000000cc
+0xFFD01414 0x00000000
+0xFFD01418 0x00000000
+0xFFD0141C 0x00000C52
+0xFFD01420 0x00000040
+0xFFD01424 0x0000F17F
+0xFFD01428 0x00085520
+0xFFD0147C 0x00008552
+0xFFD01504 0x0FFFFFF1
+0xFFD01508 0x10000000
+0xFFD0150C 0x0FFFFFF5
+0xFFD01514 0x00000000
+0xFFD0151C 0x00000000
+0xFFD01494 0x00030000
+0xFFD01498 0x00000000
+0xFFD0149C 0x0000E803
+0xFFD01480 0x00000001
+# End of Header extension
+0x0 0x0
-- 
1.5.3.4

[U-Boot] [PATCH 3/3] Kirkwood: Sheevaplug: kwimage configuration

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Jean-Christophe PLAGNIOL-VILLARD [mailto:plagnioj at jcrosoft.com] 
> Sent: Sunday, July 19, 2009 2:56 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Manas Saksena; Ronen Shitrit; 
> Nicolas Pitre; Ashish Karkare; Prabhanjan Sarnaik; Lennert Buijtenhek
> Subject: Re: [U-Boot] [PATCH 3/3] Kirkwood: Sheevaplug: 
> kwimage configuration
> 
> > +
> > +# Boot Media configurations
> > +BOOT_FROM	nand
> > +NAND_ECC_MODE	default
> > +NAND_PAGE_SIZE	0x0800
> > +
> > +# SOC registers configuration for initial values using 
> bootrom header 
> > +extension # Maximum KWBIMAGE_MAX_CONFIG configurations allowed
> > +
> > +# Configure RGMII-0 interface pad voltage to 1.8V 0xFFD100e0 
> > +0x1b1b1b9b # DRAM Configuration 0xFFD01400 0x43000c30
> could you give more details about the configs
Dear Jean
I will add details for each configuration

Regards..
Prafulla . .

> 
> Best Regards,
> J.
> 

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Prafulla Wadaskar]

Dear Wolfgang
Thanks for your quick feedback 

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Sunday, July 19, 2009 3:33 AM
> To: Prafulla Wadaskar
> Cc: u-boot at 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 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.
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 at denx.de "Plan to throw one away. You will anyway."
>                               - Fred Brooks, "The Mythical Man Month"
> 

[U-Boot] [PATCH 1/3] tools: mkimage: hdr_size used to facilitate customized support

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Sunday, July 19, 2009 3:04 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Manas Saksena; Ronen Shitrit; 
> Nicolas Pitre; Ashish Karkare; Prabhanjan Sarnaik; Lennert Buijtenhek
> Subject: Re: [U-Boot] [PATCH 1/3] tools: mkimage: hdr_size 
> used to facilitate customized support
> 
> Dear Prafulla Wadaskar,
> 
> In message 
> <1247967958-4446-1-git-send-email-prafulla@marvell.com> you wrote:
> > hdr_size variable is initialized
> > at the start of image creation algorithm instead of reading 
> it each time.
> > This facilitate to use the common code for other image type 
> > implementations for ex. kwbimage
> 
> Hm... I have no idea what you are trying to do, or why. You 
> are aware that image_get_header_size()  is  a  static  
> inline,  returning  just "sizeof (image_header_t)"?
Dear Wolfgang

This is what done in image_get_header_size().
I do not want to disturb current mkimage implementation.
whereas for mkbimage creation the image_header size will come form kwbimag.h and not generic one which is used further in image creation part.

In brief, header size will be decided by which kind of image you are creating.

This is a generic change required for kwbimage support.
I have kept this change in separate patch to get some better readability of generic changes for kwbimage support.

> 
> As far as the context of mkimage goes, you could as well 
> consider it a constant.
This will be decided runtime hence can not be considered as constant.
Well I can update the relevant line kwbimage patch to read sizeof() instead of api kwbimage_get_header_size()
 
> 
> > Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
> > ---
> >  tools/mkimage.c |   18 ++++++++----------
> >  1 files changed, 8 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tools/mkimage.c b/tools/mkimage.c index 
> 967fe9a..c5b593c 
> > 100644
> > --- a/tools/mkimage.c
> > +++ b/tools/mkimage.c
> > @@ -250,9 +250,10 @@ NXTARG:		;
> >  	 *
> >  	 * write dummy header, to be fixed later
> >  	 */
> > -	memset (hdr, 0, image_get_header_size ());
> > -
> > -	if (write(ifd, hdr, image_get_header_size ()) != 
> image_get_header_size ()) {
> > +	int hdr_size;
> 
> We don't allow variable declarations in the middle of the code.
Okay I will more this up

Thanks and regards..
Prafulla . .

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Prafulla Wadaskar]

________________________________________
From: u-boot-bounces@lists.denx.de [u-boot-bounces at lists.denx.de] On Behalf Of Prafulla Wadaskar [prafulla at marvell.com]
Sent: Sunday, July 19, 2009 10:57 AM
To: Wolfgang Denk
Cc: Manas Saksena; Ronen Shitrit; Nicolas Pitre; u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik; Lennert Buijtenhek
Subject: Re: [U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

Dear Wolfgang
Thanks for your quick feedback

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Sunday, July 19, 2009 3:33 AM
> To: Prafulla Wadaskar
> Cc: u-boot at 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

<snip..>
> > 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.

Hi Wolfgang
Any suggestions for this?
I could used <linux/types.h> or <asm/types.h> but these typedefs are ifdefed
under __KERNEL_STRICT_NAMES and __KERNEL__.
I don't wish to redefine or disturb them for kwbimage support

Regards..
Prafulla . .

>
> > 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.
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 at denx.de "Plan to throw one away. You will anyway."
>                               - Fred Brooks, "The Mythical Man Month"
>
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Prafulla Wadaskar]

________________________________________
From: Jean-Christophe PLAGNIOL-VILLARD [plagnioj at jcrosoft.com]
Sent: Sunday, July 19, 2009 2:55 AM
To: Prafulla Wadaskar
Cc: u-boot at 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

On 07:15 Sun 19 Jul     , Prafulla Wadaskar wrote:
> For more details refer docs/README.kwbimage
>
> Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
> ---
>  Makefile            |    5 ++
>  common/image.c      |    1 +
>  doc/README.kwbimage |   77 ++++++++++++++++++++++
>  include/image.h     |    1 +
>  tools/Makefile      |    1 +
>  tools/kwbimage.c    |  181 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tools/kwbimage.h    |  105 +++++++++++++++++++++++++++++
>  tools/mkimage.c     |   23 ++++++-
>  8 files changed, 393 insertions(+), 1 deletions(-)
>  create mode 100644 doc/README.kwbimage
>  create mode 100644 tools/kwbimage.c
>  create mode 100644 tools/kwbimage.h
>
> diff --git a/Makefile b/Makefile
> index 55fc2a3..b9e45ed 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -339,6 +339,10 @@ $(obj)u-boot.img:        $(obj)u-boot.bin
>                       sed -e 's/"[     ]*$$/ for $(BOARD) board"/') \
>               -d $< $@
>
> +$(obj)u-boot.kwb:    $(obj)u-boot.bin
> +             ./tools/mkimage -n ./board/$(BOARDDIR)/kwbimage.cfg \
> +             -T kwbimage -a $(TEXT_BASE) -e $(TEXT_BASE) -d $< $@
It will be better to make it a few more generic (and btw fix the out of tree
support) as this
MKIMAGE = $(OBJTREE)/tools/mkimage
[prafulla] this is generic change and should be done for other mkimage references too.
for kwbimage patch I am skipping this, I will issue separate patch for this.

$(obj)u-boot.kwb:       $(obj)u-boot.bin
                $(MKIMAGE) -n $(KWD_CONFIG) -T kwbimage -a $(TEXT_BASE) \
                -e $(TEXT_BASE) -d $< $@
[prafulla] done..

and in the board config.mk or soc anyother config.mk
KWD_CONFIG = $(SRCTREE)/board/$(BOARDDIR)/kwbimage.cfg
[prafulla] done in board config.mk, 
thanks for this suggestion, this will add more flexibility to share kwbimage config file for multiple boards

Regards..
Prafulla . .

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <73173D32E9439E4ABB5151606C3E19E202DDF272E2@SC-VEXCH1.marvell.com> you wrote:
>
> > > +		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

Hm...

Normally the use of lex (and eventually yacc) is recommended if you
need a parser - writing parsers in C is a typical "programming" task
(see definition in signature below :-)

If you have an extremely simple syntax, you might for example proceed
like this:

1) drop all lines starting with '#' as comment lines.
2) use strtok_r() to split your lines of input into white-space
   separated tokens.
3) drop all lines with zero tokens (= empty lines)
4) process the rest

[See example in the strtok()/strtok_r() man page).

> As explained in doc.README.kwimage,
> any other line apart from above will be considered as valid configuration l> ine.
> This is bare minimal parsing provided here which is sufficient
...

> > 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 ;-)

This makes little sense to me. When we have one command to  build  an
image,  I  definitely want to have a command to check and display the
contents of an image, too, and/or to verify it's integrity.

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

I understand your position. I guess you  agree  that  _full_  support
(i.e.  including the ability to check and list and image, both on the
host and on the target) would be even better, and more in  line  with
existing U-Boot code?

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 at denx.de
PROGRAM - n.  A magic spell cast over a computer  allowing it to turn
one's input into error messages.
v. tr. - To engage in a pastime similar to banging one's head against
a wall, but with fewer opportunities for reward.

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <73173D32E9439E4ABB5151606C3E19E202DE09CE02@SC-VEXCH1.marvell.com> you wrote:
> 
> > > 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.
> 
> Hi Wolfgang
> Any suggestions for this?
> I could used <linux/types.h> or <asm/types.h> but these typedefs are ifdefe=
> d
> under __KERNEL_STRICT_NAMES and __KERNEL__.
> I don't wish to redefine or disturb them for kwbimage support

This is code running on the host, so please use ISO C99 standard types:
#include <stdint.h> and use int8_t, int16_t, int32_t, int64_t resp.
uint8_t, uint16_t, uint32_t, uint64_t.



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 at denx.de
        Ordnung ist die Lust der Vernunft,
        aber Unordnung die Wonne der Phantasie         - Paul Claudel

[U-Boot] [PATCH 2/3] tools: mkimage (type=kwbimage) kirkwood boot image support

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Monday, July 20, 2009 8:27 PM
> To: Prafulla Wadaskar
> Cc: u-boot at 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 
> <73173D32E9439E4ABB5151606C3E19E202DDF272E2@SC-VEXCH1.marvell.
> com> you wrote:
> >
> > > > +		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
> 
> Hm...
> 
> Normally the use of lex (and eventually yacc) is recommended 
> if you need a parser - writing parsers in C is a typical 
> "programming" task (see definition in signature below :-)
> 
> If you have an extremely simple syntax, you might for example 
> proceed like this:
> 
> 1) drop all lines starting with '#' as comment lines.
> 2) use strtok_r() to split your lines of input into white-space
>    separated tokens.
> 3) drop all lines with zero tokens (= empty lines)
> 4) process the rest
Dear Wolfgang
I used strtok_r() since it is extremely simple parser

> 
> [See example in the strtok()/strtok_r() man page).
> 
> > As explained in doc.README.kwimage,
> > any other line apart from above will be considered as valid 
> configuration l> ine.
> > This is bare minimal parsing provided here which is sufficient
> ...
> 
> > > 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 ;-)
> 
> This makes little sense to me. When we have one command to  
> build  an image,  I  definitely want to have a command to 
> check and display the contents of an image, too, and/or to 
> verify it's integrity.
> 
> > 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.
> 
> I understand your position. I guess you  agree  that  _full_  
> support (i.e.  including the ability to check and list and 
> image, both on the host and on the target) would be even 
> better, and more in  line  with existing U-Boot code?
I agreed...
I have posted separate patch to support check and list support.
Thanks for wishing this to make it complete kwbimage support :-)

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 at denx.de PROGRAM - n.  A magic spell cast over a computer  
> allowing it to turn one's input into error messages.
> v. tr. - To engage in a pastime similar to banging one's head 
> against a wall, but with fewer opportunities for reward.
>