* [U-Boot] [PATCH 1/8] tools: mkimage : bugfix returns correct value for list command
@ 2009-07-28 18:04 Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific Prafulla Wadaskar
2009-08-07 22:11 ` [U-Boot] [PATCH 1/8] tools: mkimage : bugfix returns correct value for list command Wolfgang Denk
0 siblings, 2 replies; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-07-28 18:04 UTC (permalink / raw)
To: u-boot
List command always return "EXIT_SUCCESS" even in case of
failure by any means.
This patch return 0 if list command is sucessful,
returns negative value reported by check_header functions
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
tools/mkimage.c | 27 +++++++++++++--------------
1 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index 02cdb95..b7b383a 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -28,7 +28,7 @@
extern unsigned long crc32 (unsigned long crc, const char *buf, unsigned int len);
static void copy_file (int, const char *, int);
static void usage (void);
-static void image_verify_header (char *, int);
+static int image_verify_header (char *, int);
static void fit_handle_file (void);
char *datafile;
@@ -60,6 +60,7 @@ main (int argc, char **argv)
struct stat sbuf;
unsigned char *ptr;
char *name = "";
+ int retval;
cmdname = *argv;
@@ -219,24 +220,21 @@ NXTARG: ;
exit (EXIT_FAILURE);
}
- if (fdt_check_header (ptr)) {
- /* old-style image */
- image_verify_header ((char *)ptr, sbuf.st_size);
- image_print_contents ((image_header_t *)ptr);
- } else {
- /* FIT image */
+ if (!(retval = fdt_check_header (ptr))) /* FIT image */
fit_print_contents (ptr);
- }
+ else if (!(retval = image_verify_header (
+ (char *)ptr, sbuf.st_size))) /* old-style image */
+ image_print_contents ((image_header_t *)ptr);
(void) munmap((void *)ptr, sbuf.st_size);
(void) close (ifd);
- exit (EXIT_SUCCESS);
+ exit (retval);
} else if (fflag) {
/* Flattened Image Tree (FIT) format handling */
debug ("FIT format handling\n");
fit_handle_file ();
- exit (EXIT_SUCCESS);
+ exit (retval);
}
/*
@@ -480,7 +478,7 @@ usage ()
exit (EXIT_FAILURE);
}
-static void
+static int
image_verify_header (char *ptr, int image_size)
{
int len;
@@ -500,7 +498,7 @@ image_verify_header (char *ptr, int image_size)
fprintf (stderr,
"%s: Bad Magic Number: \"%s\" is no valid image\n",
cmdname, imagefile);
- exit (EXIT_FAILURE);
+ return -FDT_ERR_BADMAGIC;
}
data = (char *)hdr;
@@ -513,7 +511,7 @@ image_verify_header (char *ptr, int image_size)
fprintf (stderr,
"%s: ERROR: \"%s\" has bad header checksum!\n",
cmdname, imagefile);
- exit (EXIT_FAILURE);
+ return -FDT_ERR_BADSTATE;
}
data = ptr + sizeof(image_header_t);
@@ -523,8 +521,9 @@ image_verify_header (char *ptr, int image_size)
fprintf (stderr,
"%s: ERROR: \"%s\" has corrupted data!\n",
cmdname, imagefile);
- exit (EXIT_FAILURE);
+ return -FDT_ERR_BADSTRUCTURE;
}
+ return 0;
}
/**
--
1.5.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific
2009-07-28 18:04 [U-Boot] [PATCH 1/8] tools: mkimage : bugfix returns correct value for list command Prafulla Wadaskar
@ 2009-07-28 18:04 ` Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files Prafulla Wadaskar
2009-08-07 22:30 ` [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific Wolfgang Denk
2009-08-07 22:11 ` [U-Boot] [PATCH 1/8] tools: mkimage : bugfix returns correct value for list command Wolfgang Denk
1 sibling, 2 replies; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-07-28 18:04 UTC (permalink / raw)
To: u-boot
This is first step towards cleaning mkimage code for kwbimage
support in clean way. Current mkimage code is very specific to
uimge generation whereas the same framework can be used to
generate other image types like kwbimage.
For this, the architecture of mkimage code need to modified.
Here is the brief plan for the same:-
a) Abstract image specific code to saperate file (this patch)
b) Move image header code from mkimage.c to respective
image specific code
c) Implement callback function for image specific functions
d) Add kwbimage type support to this framework
In this patch-
1. Image specific code abstracted from mkimage.c to
default_image.c/h to make mkimage code more generic
2. struct mkimage_params introduced to pass basic mkimage
specific flags and variables to image specific functions
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
tools/Makefile | 4 +
tools/default_image.c | 227 ++++++++++++++++++++++++++++++
tools/default_image.h | 36 +++++
tools/mkimage.c | 368 ++++++++++++++-----------------------------------
tools/mkimage.h | 25 ++++
5 files changed, 394 insertions(+), 266 deletions(-)
create mode 100644 tools/default_image.c
create mode 100644 tools/default_image.h
diff --git a/tools/Makefile b/tools/Makefile
index b5a1e39..6ef15d9 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -171,6 +171,7 @@ $(obj)img2srec$(SFX): $(obj)img2srec.o
$(STRIP) $@
$(obj)mkimage$(SFX): $(obj)mkimage.o $(obj)crc32.o $(obj)image.o $(obj)md5.o \
+ $(obj)default_image.o \
$(obj)sha1.o $(LIBFDT_OBJS) $(obj)os_support.o
$(CC) $(CFLAGS) $(HOST_LDFLAGS) -o $@ $^
$(STRIP) $@
@@ -203,6 +204,9 @@ $(obj)bin2header$(SFX): $(obj)bin2header.o
$(obj)image.o: $(SRCTREE)/common/image.c
$(CC) -g $(FIT_CFLAGS) -c -o $@ $<
+$(obj)default_image.o: $(SRCTREE)/tools/default_image.c
+ $(CC) -g $(FIT_CFLAGS) -c -o $@ $<
+
$(obj)mkimage.o: $(SRCTREE)/tools/mkimage.c
$(CC) -g $(FIT_CFLAGS) -c -o $@ $<
diff --git a/tools/default_image.c b/tools/default_image.c
new file mode 100644
index 0000000..45afde1
--- /dev/null
+++ b/tools/default_image.c
@@ -0,0 +1,227 @@
+/*
+ * (C) Copyright 2008 Semihalf
+ *
+ * (C) Copyright 2000-2004
+ * DENX Software Engineering
+ * Wolfgang Denk, wd at denx.de
+ * Updated-by: Prafulla Wadaskar <prafulla@marvell.com>
+ * image specific code abstracted from mkimage.c
+ * some functions added to address abstraction
+ * All rights reserved.
+ *
+ * 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 "mkimage.h"
+#include <image.h>
+#include "default_image.h"
+
+int image_check_params (struct mkimage_params *params)
+{
+ return ((params->dflag && (params->fflag || params->lflag)) ||
+ (params->fflag && (params->dflag || params->lflag)) ||
+ (params->lflag && (params->dflag || params->fflag)));
+}
+
+int
+image_verify_header (char *ptr, int image_size,
+ struct mkimage_params *params)
+{
+ int len;
+ char *data;
+ uint32_t checksum;
+ image_header_t header;
+ image_header_t *hdr = &header;
+
+ /*
+ * create copy of header so that we can blank out the
+ * checksum field for checking - this can't be done
+ * on the PROT_READ mapped data.
+ */
+ memcpy (hdr, ptr, sizeof(image_header_t));
+
+ if (be32_to_cpu(hdr->ih_magic) != IH_MAGIC) {
+ fprintf (stderr,
+ "%s: Bad Magic Number: \"%s\" is no valid image\n",
+ params->cmdname, params->imagefile);
+ return -FDT_ERR_BADMAGIC;
+ }
+
+ data = (char *)hdr;
+ len = sizeof(image_header_t);
+
+ checksum = be32_to_cpu(hdr->ih_hcrc);
+ hdr->ih_hcrc = cpu_to_be32(0); /* clear for re-calculation */
+
+ if (crc32 (0, data, len) != checksum) {
+ fprintf (stderr,
+ "%s: ERROR: \"%s\" has bad header checksum!\n",
+ params->cmdname, params->imagefile);
+ return -FDT_ERR_BADSTATE;
+ }
+
+ data = ptr + sizeof(image_header_t);
+ len = image_size - sizeof(image_header_t) ;
+
+ if (crc32 (0, data, len) != be32_to_cpu(hdr->ih_dcrc)) {
+ fprintf (stderr,
+ "%s: ERROR: \"%s\" has corrupted data!\n",
+ params->cmdname, params->imagefile);
+ return -FDT_ERR_BADSTRUCTURE;
+ }
+ return 0;
+}
+
+void image_print_header (char *ptr)
+{
+ image_header_t * hdr = (image_header_t *)ptr;
+
+ image_print_contents (hdr);
+}
+
+void image_set_header (char *ptr, struct stat *sbuf,
+ struct mkimage_params *params)
+{
+ uint32_t checksum;
+
+ image_header_t * hdr = (image_header_t *)ptr;
+
+ checksum = crc32 (0,
+ (const char *)(ptr + image_get_header_size ()),
+ sbuf->st_size - image_get_header_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_load (hdr, params->addr);
+ image_set_ep (hdr, params->ep);
+ image_set_dcrc (hdr, checksum);
+ image_set_os (hdr, params->opt_os);
+ image_set_arch (hdr, params->opt_arch);
+ image_set_type (hdr, params->opt_type);
+ image_set_comp (hdr, params->opt_comp);
+
+ image_set_name (hdr, params->imagename);
+
+ checksum = crc32 (0, (const char *)hdr, image_get_header_size ());
+
+ image_set_hcrc (hdr, checksum);
+}
+
+/**
+ * fit_handle_file - main FIT file processing function
+ *
+ * fit_handle_file() runs dtc to convert .its to .itb, includes
+ * binary data, updates timestamp property and calculates hashes.
+ *
+ * datafile - .its file
+ * imagefile - .itb file
+ *
+ * returns:
+ * only on success, otherwise calls exit (EXIT_FAILURE);
+ */
+void fit_handle_file (struct mkimage_params *params)
+{
+ char tmpfile[MKIMAGE_MAX_TMPFILE_LEN];
+ char cmd[MKIMAGE_MAX_DTC_CMDLINE_LEN];
+ int tfd;
+ struct stat sbuf;
+ unsigned char *ptr;
+
+ /* call dtc to include binary properties into the tmp file */
+ if (strlen (params->imagefile) +
+ strlen (MKIMAGE_TMPFILE_SUFFIX) + 1 > sizeof (tmpfile)) {
+ fprintf (stderr, "%s: Image file name (%s) too long, "
+ "can't create tmpfile",
+ params->imagefile, params->cmdname);
+ exit (EXIT_FAILURE);
+ }
+ sprintf (tmpfile, "%s%s", params->imagefile, MKIMAGE_TMPFILE_SUFFIX);
+
+ /* dtc -I dts -O -p 200 datafile > tmpfile */
+ sprintf (cmd, "%s %s %s > %s",
+ MKIMAGE_DTC, params->opt_dtc, params->datafile, tmpfile);
+ debug ("Trying to execute \"%s\"\n", cmd);
+ if (system (cmd) == -1) {
+ fprintf (stderr, "%s: system(%s) failed: %s\n",
+ params->cmdname, cmd, strerror(errno));
+ unlink (tmpfile);
+ exit (EXIT_FAILURE);
+ }
+
+ /* load FIT blob into memory */
+ tfd = open (tmpfile, O_RDWR|O_BINARY);
+
+ if (tfd < 0) {
+ fprintf (stderr, "%s: Can't open %s: %s\n",
+ params->cmdname, tmpfile, strerror(errno));
+ unlink (tmpfile);
+ exit (EXIT_FAILURE);
+ }
+
+ if (fstat (tfd, &sbuf) < 0) {
+ fprintf (stderr, "%s: Can't stat %s: %s\n",
+ params->cmdname, tmpfile, strerror(errno));
+ unlink (tmpfile);
+ exit (EXIT_FAILURE);
+ }
+
+ ptr = mmap (0, sbuf.st_size, PROT_READ|PROT_WRITE, MAP_SHARED,
+ tfd, 0);
+ if (ptr == MAP_FAILED) {
+ fprintf (stderr, "%s: Can't read %s: %s\n",
+ params->cmdname, tmpfile, strerror(errno));
+ unlink (tmpfile);
+ exit (EXIT_FAILURE);
+ }
+
+ /* check if ptr has a valid blob */
+ if (fdt_check_header (ptr)) {
+ fprintf (stderr, "%s: Invalid FIT blob\n", params->cmdname);
+ unlink (tmpfile);
+ exit (EXIT_FAILURE);
+ }
+
+ /* set hashes for images in the blob */
+ if (fit_set_hashes (ptr)) {
+ fprintf (stderr, "%s Can't add hashes to FIT blob",
+ params->cmdname);
+ unlink (tmpfile);
+ exit (EXIT_FAILURE);
+ }
+
+ /* add a timestamp at offset 0 i.e., root */
+ if (fit_set_timestamp (ptr, 0, sbuf.st_mtime)) {
+ fprintf (stderr, "%s: Can't add image timestamp\n",
+ params->cmdname);
+ unlink (tmpfile);
+ exit (EXIT_FAILURE);
+ }
+ debug ("Added timestamp successfully\n");
+
+ munmap ((void *)ptr, sbuf.st_size);
+ close (tfd);
+
+ if (rename (tmpfile, params->imagefile) == -1) {
+ fprintf (stderr, "%s: Can't rename %s to %s: %s\n",
+ params->cmdname, tmpfile, params->imagefile,
+ strerror (errno));
+ unlink (tmpfile);
+ unlink (params->imagefile);
+ exit (EXIT_FAILURE);
+ }
+}
diff --git a/tools/default_image.h b/tools/default_image.h
new file mode 100644
index 0000000..6ebc689
--- /dev/null
+++ b/tools/default_image.h
@@ -0,0 +1,36 @@
+/*
+ * (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
+ */
+
+#ifndef _DEFAULT_IMAGE_H_
+#define _DEFAULT_IMAGE_H_
+
+int image_check_params (struct mkimage_params *params);
+int image_verify_header (char *ptr, int image_size,
+ struct mkimage_params *params);
+void image_print_header (char *ptr);
+void image_set_header (char *ptr, struct stat *sbuf,
+ struct mkimage_params *params);
+void fit_handle_file (struct mkimage_params *params);
+
+#endif /* _DEFAULT_IMAGE_H_ */
diff --git a/tools/mkimage.c b/tools/mkimage.c
index b7b383a..66d6c8e 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -24,28 +24,30 @@
#include "mkimage.h"
#include <image.h>
-
-extern unsigned long crc32 (unsigned long crc, const char *buf, unsigned int len);
-static void copy_file (int, const char *, int);
-static void usage (void);
-static int image_verify_header (char *, int);
-static void fit_handle_file (void);
-
-char *datafile;
-char *imagefile;
-char *cmdname;
-
-int dflag = 0;
-int eflag = 0;
-int fflag = 0;
-int lflag = 0;
-int vflag = 0;
-int xflag = 0;
-int opt_os = IH_OS_LINUX;
-int opt_arch = IH_ARCH_PPC;
-int opt_type = IH_TYPE_KERNEL;
-int opt_comp = IH_COMP_GZIP;
-char *opt_dtc = MKIMAGE_DEFAULT_DTC_OPTIONS;
+#include "default_image.h"
+
+static void copy_file(int, const char *, int);
+static void usage(void);
+
+struct mkimage_params params = {
+ .dflag = 0,
+ .eflag = 0,
+ .fflag = 0,
+ .lflag = 0,
+ .vflag = 0,
+ .xflag = 0,
+ .opt_os = IH_OS_LINUX,
+ .opt_arch = IH_ARCH_PPC,
+ .opt_type = IH_TYPE_KERNEL,
+ .opt_comp = IH_COMP_GZIP,
+ .opt_dtc = MKIMAGE_DEFAULT_DTC_OPTIONS,
+ .addr = 0,
+ .ep = 0,
+ .imagename = "",
+ .datafile = "",
+ .imagefile = "",
+ .cmdname = "",
+};
image_header_t header;
image_header_t *hdr = &header;
@@ -54,96 +56,98 @@ int
main (int argc, char **argv)
{
int ifd = -1;
- uint32_t checksum;
- uint32_t addr;
- uint32_t ep;
struct stat sbuf;
unsigned char *ptr;
- char *name = "";
int retval;
- cmdname = *argv;
+ params.cmdname = *argv;
- addr = ep = 0;
+ params.addr = params.ep = 0;
while (--argc > 0 && **++argv == '-') {
while (*++*argv) {
switch (**argv) {
case 'l':
- lflag = 1;
+ params.lflag = 1;
break;
case 'A':
if ((--argc <= 0) ||
- (opt_arch = genimg_get_arch_id (*++argv)) < 0)
+ (params.opt_arch =
+ genimg_get_arch_id (*++argv)) < 0)
usage ();
goto NXTARG;
case 'C':
if ((--argc <= 0) ||
- (opt_comp = genimg_get_comp_id (*++argv)) < 0)
+ (params.opt_comp =
+ genimg_get_comp_id (*++argv)) < 0)
usage ();
goto NXTARG;
case 'D':
if (--argc <= 0)
usage ();
- opt_dtc = *++argv;
+ params.opt_dtc = *++argv;
goto NXTARG;
case 'O':
if ((--argc <= 0) ||
- (opt_os = genimg_get_os_id (*++argv)) < 0)
+ (params.opt_os =
+ genimg_get_os_id (*++argv)) < 0)
usage ();
goto NXTARG;
case 'T':
if ((--argc <= 0) ||
- (opt_type = genimg_get_type_id (*++argv)) < 0)
+ (params.opt_type =
+ genimg_get_type_id (*++argv)) < 0)
usage ();
goto NXTARG;
case 'a':
if (--argc <= 0)
usage ();
- addr = strtoul (*++argv, (char **)&ptr, 16);
+ params.addr = strtoul (*++argv,
+ (char **)&ptr, 16);
if (*ptr) {
fprintf (stderr,
"%s: invalid load address %s\n",
- cmdname, *argv);
+ params.cmdname, *argv);
exit (EXIT_FAILURE);
}
goto NXTARG;
case 'd':
if (--argc <= 0)
usage ();
- datafile = *++argv;
- dflag = 1;
+ params.datafile = *++argv;
+ params.dflag = 1;
goto NXTARG;
case 'e':
if (--argc <= 0)
usage ();
- ep = strtoul (*++argv, (char **)&ptr, 16);
+ params.ep = strtoul (*++argv,
+ (char **)&ptr, 16);
if (*ptr) {
fprintf (stderr,
"%s: invalid entry point %s\n",
- cmdname, *argv);
+ params.cmdname, *argv);
exit (EXIT_FAILURE);
}
- eflag = 1;
+ params.eflag = 1;
goto NXTARG;
case 'f':
if (--argc <= 0)
usage ();
- datafile = *++argv;
- fflag = 1;
+ params.datafile = *++argv;
+ params.fflag = 1;
goto NXTARG;
case 'n':
if (--argc <= 0)
usage ();
- name = *++argv;
+ params.imagename = *++argv;
goto NXTARG;
case 'v':
- vflag++;
+ params.vflag++;
break;
case 'x':
- xflag++;
+ params.xflag++;
break;
default:
usage ();
@@ -152,88 +156,89 @@ main (int argc, char **argv)
NXTARG: ;
}
- if ((argc != 1) ||
- (dflag && (fflag || lflag)) ||
- (fflag && (dflag || lflag)) ||
- (lflag && (dflag || fflag)))
- usage();
+ if ((argc != 1) || image_check_params (¶ms))
+ usage ();
- if (!eflag) {
- ep = addr;
+ if (!params.eflag) {
+ params.ep = params.addr;
/* If XIP, entry point must be after the U-Boot header */
- if (xflag)
- ep += image_get_header_size ();
+ if (params.xflag)
+ params.ep += image_get_header_size ();
}
/*
* If XIP, ensure the entry point is equal to the load address plus
* the size of the U-Boot header.
*/
- if (xflag) {
- if (ep != addr + image_get_header_size ()) {
+ if (params.xflag) {
+ if (params.ep != params.addr + image_get_header_size ()) {
fprintf (stderr,
"%s: For XIP, the entry point must be the load addr + %lu\n",
- cmdname,
+ params.cmdname,
(unsigned long)image_get_header_size ());
exit (EXIT_FAILURE);
}
}
- imagefile = *argv;
+ params.imagefile = *argv;
- if (!fflag){
- if (lflag) {
- ifd = open (imagefile, O_RDONLY|O_BINARY);
+ if (!params.fflag){
+ if (params.lflag) {
+ ifd = open (params.imagefile, O_RDONLY|O_BINARY);
} else {
- ifd = open (imagefile,
+ ifd = open (params.imagefile,
O_RDWR|O_CREAT|O_TRUNC|O_BINARY, 0666);
}
if (ifd < 0) {
fprintf (stderr, "%s: Can't open %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile,
+ strerror(errno));
exit (EXIT_FAILURE);
}
}
- if (lflag) {
+ if (params.lflag) {
/*
* list header information of existing image
*/
if (fstat(ifd, &sbuf) < 0) {
fprintf (stderr, "%s: Can't stat %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile,
+ strerror(errno));
exit (EXIT_FAILURE);
}
if ((unsigned)sbuf.st_size < image_get_header_size ()) {
fprintf (stderr,
"%s: Bad size: \"%s\" is no valid image\n",
- cmdname, imagefile);
+ params.cmdname, params.imagefile);
exit (EXIT_FAILURE);
}
ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, ifd, 0);
if (ptr == MAP_FAILED) {
fprintf (stderr, "%s: Can't read %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile,
+ strerror(errno));
exit (EXIT_FAILURE);
}
if (!(retval = fdt_check_header (ptr))) /* FIT image */
fit_print_contents (ptr);
else if (!(retval = image_verify_header (
- (char *)ptr, sbuf.st_size))) /* old-style image */
+ (char *)ptr, sbuf.st_size,
+ ¶ms))) /* old-style image */
image_print_contents ((image_header_t *)ptr);
(void) munmap((void *)ptr, sbuf.st_size);
(void) close (ifd);
exit (retval);
- } else if (fflag) {
+ } else if (params.fflag) {
/* Flattened Image Tree (FIT) format handling */
debug ("FIT format handling\n");
- fit_handle_file ();
+ fit_handle_file (¶ms);
exit (retval);
}
@@ -246,12 +251,12 @@ NXTARG: ;
if (write(ifd, hdr, image_get_header_size ()) != image_get_header_size ()) {
fprintf (stderr, "%s: Write error on %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile, strerror(errno));
exit (EXIT_FAILURE);
}
- if (opt_type == IH_TYPE_MULTI || opt_type == IH_TYPE_SCRIPT) {
- char *file = datafile;
+ if (params.opt_type == IH_TYPE_MULTI || params.opt_type == IH_TYPE_SCRIPT) {
+ char *file = params.datafile;
uint32_t size;
for (;;) {
@@ -264,7 +269,7 @@ NXTARG: ;
if (stat (file, &sbuf) < 0) {
fprintf (stderr, "%s: Can't stat %s: %s\n",
- cmdname, file, strerror(errno));
+ params.cmdname, file, strerror(errno));
exit (EXIT_FAILURE);
}
size = cpu_to_uimage (sbuf.st_size);
@@ -274,7 +279,8 @@ NXTARG: ;
if (write(ifd, (char *)&size, sizeof(size)) != sizeof(size)) {
fprintf (stderr, "%s: Write error on %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile,
+ strerror(errno));
exit (EXIT_FAILURE);
}
@@ -290,7 +296,7 @@ NXTARG: ;
}
}
- file = datafile;
+ file = params.datafile;
for (;;) {
char *sep = strchr(file, ':');
@@ -305,7 +311,7 @@ NXTARG: ;
}
}
} else {
- copy_file (ifd, datafile, 0);
+ copy_file (ifd, params.datafile, 0);
}
/* We're a bit of paranoid */
@@ -317,43 +323,20 @@ NXTARG: ;
if (fstat(ifd, &sbuf) < 0) {
fprintf (stderr, "%s: Can't stat %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile, strerror(errno));
exit (EXIT_FAILURE);
}
ptr = mmap(0, sbuf.st_size, PROT_READ|PROT_WRITE, MAP_SHARED, ifd, 0);
if (ptr == MAP_FAILED) {
fprintf (stderr, "%s: Can't map %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile, strerror(errno));
exit (EXIT_FAILURE);
}
- hdr = (image_header_t *)ptr;
-
- checksum = crc32 (0,
- (const char *)(ptr + image_get_header_size ()),
- sbuf.st_size - image_get_header_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_load (hdr, addr);
- image_set_ep (hdr, ep);
- image_set_dcrc (hdr, checksum);
- image_set_os (hdr, opt_os);
- image_set_arch (hdr, opt_arch);
- image_set_type (hdr, opt_type);
- image_set_comp (hdr, opt_comp);
-
- image_set_name (hdr, name);
+ image_set_header (ptr, &sbuf, ¶ms);
- checksum = crc32 (0, (const char *)hdr, image_get_header_size ());
-
- image_set_hcrc (hdr, checksum);
-
- image_print_contents (hdr);
+ image_print_header (ptr);
(void) munmap((void *)ptr, sbuf.st_size);
@@ -366,7 +349,7 @@ NXTARG: ;
if (close(ifd)) {
fprintf (stderr, "%s: Write error on %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile, strerror(errno));
exit (EXIT_FAILURE);
}
@@ -384,30 +367,30 @@ copy_file (int ifd, const char *datafile, int pad)
int offset = 0;
int size;
- if (vflag) {
+ if (params.vflag) {
fprintf (stderr, "Adding Image %s\n", datafile);
}
if ((dfd = open(datafile, O_RDONLY|O_BINARY)) < 0) {
fprintf (stderr, "%s: Can't open %s: %s\n",
- cmdname, datafile, strerror(errno));
+ params.cmdname, datafile, strerror(errno));
exit (EXIT_FAILURE);
}
if (fstat(dfd, &sbuf) < 0) {
fprintf (stderr, "%s: Can't stat %s: %s\n",
- cmdname, datafile, strerror(errno));
+ params.cmdname, datafile, strerror(errno));
exit (EXIT_FAILURE);
}
ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, dfd, 0);
if (ptr == MAP_FAILED) {
fprintf (stderr, "%s: Can't read %s: %s\n",
- cmdname, datafile, strerror(errno));
+ params.cmdname, datafile, strerror(errno));
exit (EXIT_FAILURE);
}
- if (xflag) {
+ if (params.xflag) {
unsigned char *p = NULL;
/*
* XIP: do not append the image_header_t@the
@@ -418,7 +401,7 @@ copy_file (int ifd, const char *datafile, int pad)
if ((unsigned)sbuf.st_size < image_get_header_size ()) {
fprintf (stderr,
"%s: Bad size: \"%s\" is too small for XIP\n",
- cmdname, datafile);
+ params.cmdname, datafile);
exit (EXIT_FAILURE);
}
@@ -426,7 +409,7 @@ copy_file (int ifd, const char *datafile, int pad)
if ( *p != 0xff ) {
fprintf (stderr,
"%s: Bad file: \"%s\" has invalid buffer for XIP\n",
- cmdname, datafile);
+ params.cmdname, datafile);
exit (EXIT_FAILURE);
}
}
@@ -437,7 +420,7 @@ copy_file (int ifd, const char *datafile, int pad)
size = sbuf.st_size - offset;
if (write(ifd, ptr + offset, size) != size) {
fprintf (stderr, "%s: Write error on %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile, strerror(errno));
exit (EXIT_FAILURE);
}
@@ -445,7 +428,8 @@ copy_file (int ifd, const char *datafile, int pad)
if (write(ifd, (char *)&zero, 4-tail) != 4-tail) {
fprintf (stderr, "%s: Write error on %s: %s\n",
- cmdname, imagefile, strerror(errno));
+ params.cmdname, params.imagefile,
+ strerror(errno));
exit (EXIT_FAILURE);
}
}
@@ -459,7 +443,7 @@ usage ()
{
fprintf (stderr, "Usage: %s -l image\n"
" -l ==> list image header information\n",
- cmdname);
+ params.cmdname);
fprintf (stderr, " %s [-x] -A arch -O os -T type -C comp "
"-a addr -e ep -n name -d data_file[:data_file...] image\n"
" -A ==> set architecture to 'arch'\n"
@@ -471,157 +455,9 @@ usage ()
" -n ==> set image name to 'name'\n"
" -d ==> use image data from 'datafile'\n"
" -x ==> set XIP (execute in place)\n",
- cmdname);
+ params.cmdname);
fprintf (stderr, " %s [-D dtc_options] -f fit-image.its fit-image\n",
- cmdname);
+ params.cmdname);
exit (EXIT_FAILURE);
}
-
-static int
-image_verify_header (char *ptr, int image_size)
-{
- int len;
- char *data;
- uint32_t checksum;
- image_header_t header;
- image_header_t *hdr = &header;
-
- /*
- * create copy of header so that we can blank out the
- * checksum field for checking - this can't be done
- * on the PROT_READ mapped data.
- */
- memcpy (hdr, ptr, sizeof(image_header_t));
-
- if (be32_to_cpu(hdr->ih_magic) != IH_MAGIC) {
- fprintf (stderr,
- "%s: Bad Magic Number: \"%s\" is no valid image\n",
- cmdname, imagefile);
- return -FDT_ERR_BADMAGIC;
- }
-
- data = (char *)hdr;
- len = sizeof(image_header_t);
-
- checksum = be32_to_cpu(hdr->ih_hcrc);
- hdr->ih_hcrc = cpu_to_be32(0); /* clear for re-calculation */
-
- if (crc32 (0, data, len) != checksum) {
- fprintf (stderr,
- "%s: ERROR: \"%s\" has bad header checksum!\n",
- cmdname, imagefile);
- return -FDT_ERR_BADSTATE;
- }
-
- data = ptr + sizeof(image_header_t);
- len = image_size - sizeof(image_header_t) ;
-
- if (crc32 (0, data, len) != be32_to_cpu(hdr->ih_dcrc)) {
- fprintf (stderr,
- "%s: ERROR: \"%s\" has corrupted data!\n",
- cmdname, imagefile);
- return -FDT_ERR_BADSTRUCTURE;
- }
- return 0;
-}
-
-/**
- * fit_handle_file - main FIT file processing function
- *
- * fit_handle_file() runs dtc to convert .its to .itb, includes
- * binary data, updates timestamp property and calculates hashes.
- *
- * datafile - .its file
- * imagefile - .itb file
- *
- * returns:
- * only on success, otherwise calls exit (EXIT_FAILURE);
- */
-static void fit_handle_file (void)
-{
- char tmpfile[MKIMAGE_MAX_TMPFILE_LEN];
- char cmd[MKIMAGE_MAX_DTC_CMDLINE_LEN];
- int tfd;
- struct stat sbuf;
- unsigned char *ptr;
-
- /* call dtc to include binary properties into the tmp file */
- if (strlen (imagefile) + strlen (MKIMAGE_TMPFILE_SUFFIX) + 1 >
- sizeof (tmpfile)) {
- fprintf (stderr, "%s: Image file name (%s) too long, "
- "can't create tmpfile",
- imagefile, cmdname);
- exit (EXIT_FAILURE);
- }
- sprintf (tmpfile, "%s%s", imagefile, MKIMAGE_TMPFILE_SUFFIX);
-
- /* dtc -I dts -O -p 200 datafile > tmpfile */
- sprintf (cmd, "%s %s %s > %s",
- MKIMAGE_DTC, opt_dtc, datafile, tmpfile);
- debug ("Trying to execute \"%s\"\n", cmd);
- if (system (cmd) == -1) {
- fprintf (stderr, "%s: system(%s) failed: %s\n",
- cmdname, cmd, strerror(errno));
- unlink (tmpfile);
- exit (EXIT_FAILURE);
- }
-
- /* load FIT blob into memory */
- tfd = open (tmpfile, O_RDWR|O_BINARY);
-
- if (tfd < 0) {
- fprintf (stderr, "%s: Can't open %s: %s\n",
- cmdname, tmpfile, strerror(errno));
- unlink (tmpfile);
- exit (EXIT_FAILURE);
- }
-
- if (fstat (tfd, &sbuf) < 0) {
- fprintf (stderr, "%s: Can't stat %s: %s\n",
- cmdname, tmpfile, strerror(errno));
- unlink (tmpfile);
- exit (EXIT_FAILURE);
- }
-
- ptr = mmap (0, sbuf.st_size, PROT_READ|PROT_WRITE, MAP_SHARED, tfd, 0);
- if (ptr == MAP_FAILED) {
- fprintf (stderr, "%s: Can't read %s: %s\n",
- cmdname, tmpfile, strerror(errno));
- unlink (tmpfile);
- exit (EXIT_FAILURE);
- }
-
- /* check if ptr has a valid blob */
- if (fdt_check_header (ptr)) {
- fprintf (stderr, "%s: Invalid FIT blob\n", cmdname);
- unlink (tmpfile);
- exit (EXIT_FAILURE);
- }
-
- /* set hashes for images in the blob */
- if (fit_set_hashes (ptr)) {
- fprintf (stderr, "%s Can't add hashes to FIT blob", cmdname);
- unlink (tmpfile);
- exit (EXIT_FAILURE);
- }
-
- /* add a timestamp@offset 0 i.e., root */
- if (fit_set_timestamp (ptr, 0, sbuf.st_mtime)) {
- fprintf (stderr, "%s: Can't add image timestamp\n", cmdname);
- unlink (tmpfile);
- exit (EXIT_FAILURE);
- }
- debug ("Added timestamp successfully\n");
-
- munmap ((void *)ptr, sbuf.st_size);
- close (tfd);
-
- if (rename (tmpfile, imagefile) == -1) {
- fprintf (stderr, "%s: Can't rename %s to %s: %s\n",
- cmdname, tmpfile, imagefile, strerror (errno));
- unlink (tmpfile);
- unlink (imagefile);
- exit (EXIT_FAILURE);
- }
-}
diff --git a/tools/mkimage.h b/tools/mkimage.h
index 70c53ad..0418644 100644
--- a/tools/mkimage.h
+++ b/tools/mkimage.h
@@ -20,6 +20,9 @@
* MA 02111-1307 USA
*/
+#ifndef _MKIIMAGE_H_
+#define _MKIIMAGE_H_
+
#include "os_support.h"
#include <errno.h>
#include <fcntl.h>
@@ -45,3 +48,25 @@
#define MKIMAGE_DEFAULT_DTC_OPTIONS "-I dts -O dtb -p 500"
#define MKIMAGE_MAX_DTC_CMDLINE_LEN 512
#define MKIMAGE_DTC "dtc" /* assume dtc is in $PATH */
+
+struct mkimage_params {
+ int dflag;
+ int eflag;
+ int fflag;
+ int lflag;
+ int vflag;
+ int xflag;
+ int opt_os;
+ int opt_arch;
+ int opt_type;
+ int opt_comp;
+ char *opt_dtc;
+ uint32_t addr;
+ uint32_t ep;
+ char *imagename;
+ char *datafile;
+ char *imagefile;
+ char *cmdname;
+};
+
+#endif /* _MKIIMAGE_H_ */
--
1.5.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files
2009-07-28 18:04 ` [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific Prafulla Wadaskar
@ 2009-07-28 18:04 ` Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support Prafulla Wadaskar
2009-08-07 22:43 ` [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files Wolfgang Denk
2009-08-07 22:30 ` [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific Wolfgang Denk
1 sibling, 2 replies; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-07-28 18:04 UTC (permalink / raw)
To: u-boot
This is second step towards cleaning mkimage code for kwbimage
support in clean way. In this patch-
1. The image_get_header_size function call is replaced by sizeof(image_header_t)
in default_image.c
2. image header code moved form mkimage.c to default_image .c
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
tools/default_image.c | 22 ++++++++++++++++++----
tools/default_image.h | 1 +
tools/mkimage.c | 24 ++++++++++++------------
tools/mkimage.h | 8 ++++++++
4 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/tools/default_image.c b/tools/default_image.c
index 45afde1..e1a9bc7 100644
--- a/tools/default_image.c
+++ b/tools/default_image.c
@@ -29,6 +29,20 @@
#include <image.h>
#include "default_image.h"
+image_header_t header;
+
+/*
+ * Default image parameters
+ */
+struct image_type_params defimage_params = {
+ .header_size = sizeof(image_header_t),
+ .hdr = (void*)&header,
+};
+
+void *image_get_tparams (void){
+ return (void *)&defimage_params;
+}
+
int image_check_params (struct mkimage_params *params)
{
return ((params->dflag && (params->fflag || params->lflag)) ||
@@ -100,13 +114,13 @@ void image_set_header (char *ptr, struct stat *sbuf,
image_header_t * 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 + sizeof(image_header_t)),
+ sbuf->st_size - sizeof(image_header_t));
/* 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 - sizeof(image_header_t));
image_set_load (hdr, params->addr);
image_set_ep (hdr, params->ep);
image_set_dcrc (hdr, checksum);
@@ -117,7 +131,7 @@ void image_set_header (char *ptr, struct stat *sbuf,
image_set_name (hdr, params->imagename);
- checksum = crc32 (0, (const char *)hdr, image_get_header_size ());
+ checksum = crc32 (0, (const char *)hdr, sizeof(image_header_t));
image_set_hcrc (hdr, checksum);
}
diff --git a/tools/default_image.h b/tools/default_image.h
index 6ebc689..2311ede 100644
--- a/tools/default_image.h
+++ b/tools/default_image.h
@@ -25,6 +25,7 @@
#ifndef _DEFAULT_IMAGE_H_
#define _DEFAULT_IMAGE_H_
+void *image_get_tparams (void);
int image_check_params (struct mkimage_params *params);
int image_verify_header (char *ptr, int image_size,
struct mkimage_params *params);
diff --git a/tools/mkimage.c b/tools/mkimage.c
index 66d6c8e..d1636d8 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -49,9 +49,6 @@ struct mkimage_params params = {
.cmdname = "",
};
-image_header_t header;
-image_header_t *hdr = &header;
-
int
main (int argc, char **argv)
{
@@ -59,6 +56,7 @@ main (int argc, char **argv)
struct stat sbuf;
unsigned char *ptr;
int retval;
+ struct image_type_params *tparams = image_get_tparams ();
params.cmdname = *argv;
@@ -163,7 +161,7 @@ NXTARG: ;
params.ep = params.addr;
/* If XIP, entry point must be after the U-Boot header */
if (params.xflag)
- params.ep += image_get_header_size ();
+ params.ep += tparams->header_size;
}
/*
@@ -171,11 +169,11 @@ NXTARG: ;
* the size of the U-Boot header.
*/
if (params.xflag) {
- if (params.ep != params.addr + image_get_header_size ()) {
+ if (params.ep != params.addr + tparams->header_size) {
fprintf (stderr,
"%s: For XIP, the entry point must be the load addr + %lu\n",
params.cmdname,
- (unsigned long)image_get_header_size ());
+ (unsigned long)tparams->header_size);
exit (EXIT_FAILURE);
}
}
@@ -209,7 +207,7 @@ NXTARG: ;
exit (EXIT_FAILURE);
}
- if ((unsigned)sbuf.st_size < image_get_header_size ()) {
+ if ((unsigned)sbuf.st_size < tparams->header_size) {
fprintf (stderr,
"%s: Bad size: \"%s\" is no valid image\n",
params.cmdname, params.imagefile);
@@ -247,9 +245,10 @@ NXTARG: ;
*
* write dummy header, to be fixed later
*/
- memset (hdr, 0, image_get_header_size ());
+ memset (tparams->hdr, 0, tparams->header_size);
- if (write(ifd, hdr, image_get_header_size ()) != image_get_header_size ()) {
+ if (write(ifd, tparams->hdr, tparams->header_size)
+ != tparams->header_size) {
fprintf (stderr, "%s: Write error on %s: %s\n",
params.cmdname, params.imagefile, strerror(errno));
exit (EXIT_FAILURE);
@@ -366,6 +365,7 @@ copy_file (int ifd, const char *datafile, int pad)
int zero = 0;
int offset = 0;
int size;
+ struct image_type_params *tparams = image_get_tparams ();
if (params.vflag) {
fprintf (stderr, "Adding Image %s\n", datafile);
@@ -398,14 +398,14 @@ copy_file (int ifd, const char *datafile, int pad)
* reserved for it.
*/
- if ((unsigned)sbuf.st_size < image_get_header_size ()) {
+ if ((unsigned)sbuf.st_size < tparams->header_size) {
fprintf (stderr,
"%s: Bad size: \"%s\" is too small for XIP\n",
params.cmdname, datafile);
exit (EXIT_FAILURE);
}
- for (p = ptr; p < ptr + image_get_header_size (); p++) {
+ for (p = ptr; p < ptr + tparams->header_size; p++) {
if ( *p != 0xff ) {
fprintf (stderr,
"%s: Bad file: \"%s\" has invalid buffer for XIP\n",
@@ -414,7 +414,7 @@ copy_file (int ifd, const char *datafile, int pad)
}
}
- offset = image_get_header_size ();
+ offset = tparams->header_size;
}
size = sbuf.st_size - offset;
diff --git a/tools/mkimage.h b/tools/mkimage.h
index 0418644..3e94911 100644
--- a/tools/mkimage.h
+++ b/tools/mkimage.h
@@ -69,4 +69,12 @@ struct mkimage_params {
char *cmdname;
};
+/*
+ * These are image type specific variables
+ */
+struct image_type_params {
+ uint32_t header_size;
+ void *hdr;
+};
+
#endif /* _MKIIMAGE_H_ */
--
1.5.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support
2009-07-28 18:04 ` [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files Prafulla Wadaskar
@ 2009-07-28 18:04 ` Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 5/8] tools: mkimage: Making genimg_print_size API global Prafulla Wadaskar
2009-08-07 22:55 ` [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support Wolfgang Denk
2009-08-07 22:43 ` [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files Wolfgang Denk
1 sibling, 2 replies; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-07-28 18:04 UTC (permalink / raw)
To: u-boot
This is Third step towards cleaning mkimage code for kwbimage
support in clean way. In this patch-
1. callback functions are used in mkimage.c those are defined
in defaut_image.c for currently supported images.
2. scan loop implemented for image header verify and print
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
include/image.h | 1 +
tools/default_image.c | 38 +++++++++++++++++++++++++++++++---
tools/default_image.h | 3 +-
tools/mkimage.c | 53 ++++++++++++++++++++++++++++++++++++++++--------
tools/mkimage.h | 15 +++++++++++++
5 files changed, 96 insertions(+), 14 deletions(-)
diff --git a/include/image.h b/include/image.h
index beb3a16..b519e5e 100644
--- a/include/image.h
+++ b/include/image.h
@@ -155,6 +155,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_MAX IH_TYPE_FLATDT /* Max supported types */
/*
* Compression Types
diff --git a/tools/default_image.c b/tools/default_image.c
index e1a9bc7..78e29f2 100644
--- a/tools/default_image.c
+++ b/tools/default_image.c
@@ -37,6 +37,10 @@ image_header_t header;
struct image_type_params defimage_params = {
.header_size = sizeof(image_header_t),
.hdr = (void*)&header,
+ .check_params = image_check_params,
+ .verify_header = image_verify_header,
+ .print_header = image_print_header,
+ .set_header = image_set_header,
};
void *image_get_tparams (void){
@@ -101,12 +105,10 @@ image_verify_header (char *ptr, int image_size,
void image_print_header (char *ptr)
{
- image_header_t * hdr = (image_header_t *)ptr;
-
- image_print_contents (hdr);
+ image_print_contents ((image_header_t *)ptr);
}
-void image_set_header (char *ptr, struct stat *sbuf,
+void image_set_header (char *ptr, struct stat *sbuf, int ifd,
struct mkimage_params *params)
{
uint32_t checksum;
@@ -136,6 +138,34 @@ void image_set_header (char *ptr, struct stat *sbuf,
image_set_hcrc (hdr, checksum);
}
+/*
+ * FIT image support
+ */
+int
+fitimage_verify_header (char *ptr, int image_size,
+ struct mkimage_params *params)
+{
+ fdt_check_header ((void *)ptr);
+}
+
+void fitimage_print_header (char *ptr)
+{
+ fit_print_contents ((void *)ptr);
+}
+
+struct image_type_params fitimage_params = {
+ .header_size = sizeof(image_header_t),
+ .hdr = (void*)&header,
+ .check_params = image_check_params,
+ .verify_header = fitimage_verify_header,
+ .print_header = fitimage_print_header,
+ .set_header = image_set_header,
+};
+
+void *fitimage_get_tparams (void){
+ return (void *)&fitimage_params;
+}
+
/**
* fit_handle_file - main FIT file processing function
*
diff --git a/tools/default_image.h b/tools/default_image.h
index 2311ede..ba95521 100644
--- a/tools/default_image.h
+++ b/tools/default_image.h
@@ -25,12 +25,13 @@
#ifndef _DEFAULT_IMAGE_H_
#define _DEFAULT_IMAGE_H_
+void *fitimage_get_tparams (void);
void *image_get_tparams (void);
int image_check_params (struct mkimage_params *params);
int image_verify_header (char *ptr, int image_size,
struct mkimage_params *params);
void image_print_header (char *ptr);
-void image_set_header (char *ptr, struct stat *sbuf,
+void image_set_header (char *ptr, struct stat *sbuf, int ifd,
struct mkimage_params *params);
void fit_handle_file (struct mkimage_params *params);
diff --git a/tools/mkimage.c b/tools/mkimage.c
index d1636d8..3a3cb19 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -49,14 +49,28 @@ struct mkimage_params params = {
.cmdname = "",
};
+static struct image_functions image_ftable[] = {
+ {image_get_tparams,}, /* for IH_TYPE_INVALID */
+ {image_get_tparams,}, /* for IH_TYPE_FILESYSTEM */
+ {image_get_tparams,}, /* for IH_TYPE_FIRMWARE */
+ {image_get_tparams,}, /* for IH_TYPE_KERNEL */
+ {image_get_tparams,}, /* for IH_TYPE_MULTI */
+ {image_get_tparams,}, /* for IH_TYPE_RAMDISK */
+ {image_get_tparams,}, /* for IH_TYPE_SCRIPT */
+ {image_get_tparams,}, /* for IH_TYPE_STANDALONE */
+ {fitimage_get_tparams,}, /* for IH_TYPE_FLATDT */
+};
+
int
main (int argc, char **argv)
{
int ifd = -1;
struct stat sbuf;
unsigned char *ptr;
- int retval;
+ int i, retval;
struct image_type_params *tparams = image_get_tparams ();
+ struct image_type_params *scantparams;
+ int image_ftable_size = ARRAY_SIZE(image_ftable);
params.cmdname = *argv;
@@ -97,6 +111,15 @@ main (int argc, char **argv)
(params.opt_type =
genimg_get_type_id (*++argv)) < 0)
usage ();
+ if (image_ftable_size <= params.opt_type) {
+ fprintf (stderr,
+ "%s: Unsupported image type %s\n",
+ params.cmdname, *argv);
+ exit (EXIT_FAILURE);
+ }
+ tparams = image_ftable[params.opt_type]
+ .get_tparams ();
+
goto NXTARG;
case 'a':
@@ -154,7 +177,7 @@ main (int argc, char **argv)
NXTARG: ;
}
- if ((argc != 1) || image_check_params (¶ms))
+ if ((argc != 1) || tparams->check_params (¶ms))
usage ();
if (!params.eflag) {
@@ -222,12 +245,24 @@ NXTARG: ;
exit (EXIT_FAILURE);
}
- if (!(retval = fdt_check_header (ptr))) /* FIT image */
- fit_print_contents (ptr);
- else if (!(retval = image_verify_header (
+ /*
+ * Scan image headers for all supported types
+ * and print if veryfy_header sucessful
+ */
+ for (i = image_ftable_size - 1; i != IH_TYPE_INVALID; i--) {
+ scantparams = image_ftable[i].get_tparams ();
+ fprintf (stderr, "%s: Verify header for type=%s\n",
+ params.cmdname, genimg_get_type_name (i));
+
+ retval = scantparams->verify_header (
(char *)ptr, sbuf.st_size,
- ¶ms))) /* old-style image */
- image_print_contents ((image_header_t *)ptr);
+ ¶ms);
+
+ if (retval == 0) {
+ scantparams->print_header ((char *)ptr);
+ break;
+ }
+ }
(void) munmap((void *)ptr, sbuf.st_size);
(void) close (ifd);
@@ -333,9 +368,9 @@ NXTARG: ;
exit (EXIT_FAILURE);
}
- image_set_header (ptr, &sbuf, ¶ms);
+ tparams->set_header (ptr, &sbuf, ifd, ¶ms);
- image_print_header (ptr);
+ tparams->print_header (ptr);
(void) munmap((void *)ptr, sbuf.st_size);
diff --git a/tools/mkimage.h b/tools/mkimage.h
index 3e94911..a9f20a7 100644
--- a/tools/mkimage.h
+++ b/tools/mkimage.h
@@ -71,10 +71,25 @@ struct mkimage_params {
/*
* These are image type specific variables
+ * and callback functions
*/
struct image_type_params {
uint32_t header_size;
void *hdr;
+ int (*check_params) (struct mkimage_params *);
+ int (*verify_header) (char *, int, struct mkimage_params *);
+ void (*print_header) (char *);
+ void (*set_header) (char *, struct stat *, int,
+ struct mkimage_params *);
};
+/*
+ * Image type specific function pointers list
+ */
+struct image_functions {
+ void * (*get_tparams) (void);
+};
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+
#endif /* _MKIIMAGE_H_ */
--
1.5.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 5/8] tools: mkimage: Making genimg_print_size API global
2009-07-28 18:04 ` [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support Prafulla Wadaskar
@ 2009-07-28 18:04 ` Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global Prafulla Wadaskar
2009-08-07 22:55 ` [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support Wolfgang Denk
1 sibling, 1 reply; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-07-28 18:04 UTC (permalink / raw)
To: u-boot
Currently it is used by image.c only
This API can be used by additional mkimage types supports
for ex. kwbimage, to use it the API is made global
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
common/image.c | 3 +--
include/image.h | 1 +
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/image.c b/common/image.c
index e22c974..232348c 100644
--- a/common/image.c
+++ b/common/image.c
@@ -158,7 +158,6 @@ static table_entry_t uimage_comp[] = {
uint32_t crc32 (uint32_t, const unsigned char *, uint);
uint32_t crc32_wd (uint32_t, const unsigned char *, uint, uint);
-static void genimg_print_size (uint32_t size);
#if defined(CONFIG_TIMESTAMP) || defined(CONFIG_CMD_DATE) || defined(USE_HOSTCC)
static void genimg_print_time (time_t timestamp);
#endif
@@ -472,7 +471,7 @@ void memmove_wd (void *to, void *from, size_t len, ulong chunksz)
}
#endif /* !USE_HOSTCC */
-static void genimg_print_size (uint32_t size)
+void genimg_print_size (uint32_t size)
{
#ifndef USE_HOSTCC
printf ("%d Bytes = ", size);
diff --git a/include/image.h b/include/image.h
index b519e5e..88a13ab 100644
--- a/include/image.h
+++ b/include/image.h
@@ -300,6 +300,7 @@ int genimg_get_comp_id (const char *name);
#define IMAGE_FORMAT_LEGACY 0x01 /* legacy image_header based format */
#define IMAGE_FORMAT_FIT 0x02 /* new, libfdt based format */
+void genimg_print_size (uint32_t size);
int genimg_get_format (void *img_addr);
int genimg_has_config (bootm_headers_t *images);
ulong genimg_get_image (ulong img_addr);
--
1.5.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global
2009-07-28 18:04 ` [U-Boot] [PATCH 5/8] tools: mkimage: Making genimg_print_size API global Prafulla Wadaskar
@ 2009-07-28 18:04 ` Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage) Prafulla Wadaskar
2009-08-07 23:02 ` [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global Wolfgang Denk
0 siblings, 2 replies; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-07-28 18:04 UTC (permalink / raw)
To: u-boot
1. get_table_entry_id API made global
2. get_table_entry_name API made global
3. typedef table_entry_t moved to image.h
Currently it is used by image.c only
These APIs can be used by additional mkimage types supports
for ex. kwbimage, to use it the API is made global
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
common/image.c | 10 ++--------
include/image.h | 10 ++++++++++
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/common/image.c b/common/image.c
index 232348c..8cf675d 100644
--- a/common/image.c
+++ b/common/image.c
@@ -74,12 +74,6 @@ static image_header_t* image_get_ramdisk (ulong rd_addr, uint8_t arch,
#include <image.h>
#endif /* !USE_HOSTCC*/
-typedef struct table_entry {
- int id; /* as defined in image.h */
- char *sname; /* short (input) name */
- char *lname; /* long (output) name */
-} table_entry_t;
-
static table_entry_t uimage_arch[] = {
{ IH_ARCH_INVALID, NULL, "Invalid ARCH", },
{ IH_ARCH_ALPHA, "alpha", "Alpha", },
@@ -513,7 +507,7 @@ static void genimg_print_time (time_t timestamp)
* long entry name if translation succeeds
* msg otherwise
*/
-static char *get_table_entry_name (table_entry_t *table, char *msg, int id)
+char *get_table_entry_name (table_entry_t *table, char *msg, int id)
{
for (; table->id >= 0; ++table) {
if (table->id == id)
@@ -560,7 +554,7 @@ const char *genimg_get_comp_name (uint8_t comp)
* entry id if translation succeeds
* -1 otherwise
*/
-static int get_table_entry_id (table_entry_t *table,
+int get_table_entry_id (table_entry_t *table,
const char *table_name, const char *name)
{
table_entry_t *t;
diff --git a/include/image.h b/include/image.h
index 88a13ab..f119cee 100644
--- a/include/image.h
+++ b/include/image.h
@@ -168,6 +168,15 @@
#define IH_MAGIC 0x27051956 /* Image Magic Number */
#define IH_NMLEN 32 /* Image Name Length */
+typedef struct table_entry {
+ int id; /* as defined in image.h */
+ char *sname; /* short (input) name */
+ char *lname; /* long (output) name */
+} table_entry_t;
+
+int get_table_entry_id (table_entry_t *table,
+ const char *table_name, const char *name);
+
/*
* Legacy format image header,
* all data in network byte order (aka natural aka bigendian).
@@ -300,6 +309,7 @@ int genimg_get_comp_id (const char *name);
#define IMAGE_FORMAT_LEGACY 0x01 /* legacy image_header based format */
#define IMAGE_FORMAT_FIT 0x02 /* new, libfdt based format */
+char *get_table_entry_name (table_entry_t *table, char *msg, int id);
void genimg_print_size (uint32_t size);
int genimg_get_format (void *img_addr);
int genimg_has_config (bootm_headers_t *images);
--
1.5.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage)
2009-07-28 18:04 ` [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global Prafulla Wadaskar
@ 2009-07-28 18:04 ` Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH v4 8/8] Kirkwood: Sheevaplug: kwimage configuration Prafulla Wadaskar
2009-08-07 23:24 ` [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage) Wolfgang Denk
2009-08-07 23:02 ` [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global Wolfgang Denk
1 sibling, 2 replies; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-07-28 18:04 UTC (permalink / raw)
To: u-boot
This is Third step towards cleaning mkimage code for kwbimage
support in clean way.
This patch adds type kwbimabe support for mkimage
For more details refer docs/README.kwbimage
This patch is tested with Wheevaplub board
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
Makefile | 5 +
common/image.c | 1 +
doc/README.kwbimage | 93 +++++++++++++
include/image.h | 3 +-
tools/Makefile | 4 +
tools/kwbimage.c | 364 +++++++++++++++++++++++++++++++++++++++++++++++++++
tools/kwbimage.h | 118 +++++++++++++++++
tools/mkimage.c | 4 +-
8 files changed, 590 insertions(+), 2 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 ef535ed..336dcd4 100644
--- a/Makefile
+++ b/Makefile
@@ -313,6 +313,10 @@ $(obj)u-boot.img: $(obj)u-boot.bin
sed -e 's/"[ ]*$$/ for $(BOARD) board"/') \
-d $< $@
+$(obj)u-boot.kwb: $(obj)u-boot.bin
+ $(obj)tools/mkimage -n $(KWD_CONFIG) -T kwbimage \
+ -a $(TEXT_BASE) -e $(TEXT_BASE) -d $< $@
+
$(obj)u-boot.sha1: $(obj)u-boot.bin
$(obj)tools/ubsha1 $(obj)u-boot.bin
@@ -3660,6 +3664,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 8cf675d..10b5deb 100644
--- a/common/image.c
+++ b/common/image.c
@@ -139,6 +139,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..ca5b228
--- /dev/null
+++ b/doc/README.kwbimage
@@ -0,0 +1,93 @@
+---------------------------------------------
+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 -l <kwboot_file>
+ to list the kwb image file details
+
+./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 associated data strings:-
+ Command string data string
+ -------------- -----------
+ BOOT_FROM nand/spi/sata
+ NAND_ECC_MODE default/rs/hamming/disabled
+ NAND_PAGE_SIZE any uint16_t hex value
+ SATA_PIO_MODE any uint32_t hex value
+ DDR_INIT_DELAY any uint32_t hex value
+ DATA regaddr and regdara hex value
+ you can have maximum 55 such register programming 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
+DATA 0xFFD100e0 0x1b1b1b9b
+# DRAM Configuration
+DATA 0xFFD01400 0x43000c30
+DATA 0xFFD01404 0x37543000
+DATA 0xFFD01408 0x22125451
+DATA 0xFFD0140C 0x00000a33
+DATA 0xFFD01410 0x000000cc
+DATA 0xFFD01414 0x00000000
+DATA 0xFFD01418 0x00000000
+DATA 0xFFD0141C 0x00000C52
+DATA 0xFFD01420 0x00000040
+DATA 0xFFD01424 0x0000F17F
+DATA 0xFFD01428 0x00085520
+DATA 0xFFD0147C 0x00008552
+DATA 0xFFD01504 0x0FFFFFF1
+DATA 0xFFD01508 0x10000000
+DATA 0xFFD0150C 0x0FFFFFF5
+DATA 0xFFD01514 0x00000000
+DATA 0xFFD0151C 0x00000000
+DATA 0xFFD01494 0x00030000
+DATA 0xFFD01498 0x00000000
+DATA 0xFFD0149C 0x0000E803
+DATA 0xFFD01480 0x00000001
+# End of Header extension
+DATA 0x0 0x0
+
+------------------------------------------------
+Author: Prafulla Wadaskar <prafulla@marvell.com>
diff --git a/include/image.h b/include/image.h
index f119cee..513f285 100644
--- a/include/image.h
+++ b/include/image.h
@@ -155,7 +155,8 @@
#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_MAX IH_TYPE_FLATDT /* Max supported types */
+#define IH_TYPE_KWBIMAGE 9 /* Kirkwood Boot Image */
+#define IH_TYPE_MAX IH_TYPE_KWBIMAGE
/*
* Compression Types
diff --git a/tools/Makefile b/tools/Makefile
index 6ef15d9..07a1c27 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -172,6 +172,7 @@ $(obj)img2srec$(SFX): $(obj)img2srec.o
$(obj)mkimage$(SFX): $(obj)mkimage.o $(obj)crc32.o $(obj)image.o $(obj)md5.o \
$(obj)default_image.o \
+ $(obj)kwbimage.o \
$(obj)sha1.o $(LIBFDT_OBJS) $(obj)os_support.o
$(CC) $(CFLAGS) $(HOST_LDFLAGS) -o $@ $^
$(STRIP) $@
@@ -207,6 +208,9 @@ $(obj)image.o: $(SRCTREE)/common/image.c
$(obj)default_image.o: $(SRCTREE)/tools/default_image.c
$(CC) -g $(FIT_CFLAGS) -c -o $@ $<
+$(obj)kwbimage.o: $(SRCTREE)/tools/kwbimage.c
+ $(CC) -g $(FIT_CFLAGS) -c -o $@ $<
+
$(obj)mkimage.o: $(SRCTREE)/tools/mkimage.c
$(CC) -g $(FIT_CFLAGS) -c -o $@ $<
diff --git a/tools/kwbimage.c b/tools/kwbimage.c
new file mode 100644
index 0000000..bebe142
--- /dev/null
+++ b/tools/kwbimage.c
@@ -0,0 +1,364 @@
+/*
+ * (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 "mkimage.h"
+#include <image.h>
+#include "kwbimage.h"
+
+/*
+ * Suported commands for configuration file
+ */
+static table_entry_t kwbimage_cmds[] = {
+ { CMD_BOOT_FROM, "BOOT_FROM", "", },
+ { CMD_NAND_ECC_MODE, "NAND_ECC_MODE", "", },
+ { CMD_NAND_PAGE_SIZE, "NAND_PAGE_SIZE", "", },
+ { CMD_SATA_PIO_MODE, "SATA_PIO_MODE", "", },
+ { CMD_DDR_INIT_DELAY, "DDR_INIT_DELAY", "", },
+ { CMD_DATA, "DATA", "", },
+ { CMD_INVALID, "", "", },
+};
+
+/*
+ * Suported Boot options for configuration file
+ */
+static table_entry_t kwbimage_bootops[] = {
+ { IBR_HDR_SPI_ID, "spi", "SPI Flash", },
+ { IBR_HDR_NAND_ID, "nand", "NAND Flash", },
+ { IBR_HDR_SATA_ID, "sata", "Sata port", },
+ { IBR_HDR_PEX_ID, "pex", "PCIe port", },
+ { IBR_HDR_UART_ID, "uart", "Serial port", },
+ { -1, "", "Invalid", },
+};
+
+/*
+ * Suported NAND ecc options configuration file
+ */
+static table_entry_t kwbimage_eccmodes[] = {
+ { IBR_HDR_ECC_DEFAULT, "default", "", },
+ { IBR_HDR_ECC_FORCED_HAMMING,"hamming", "", },
+ { IBR_HDR_ECC_FORCED_RS, "rs", "", },
+ { IBR_HDR_ECC_DISABLED, "disabled", "", },
+ { -1, "", "", },
+};
+
+static struct kwb_header kwbimage_header;
+static int datacmd_cnt = 0;
+static char * fname = "Unknown";
+static int lineno = -1;
+
+struct image_type_params kwbimage_params = {
+ .header_size = sizeof(struct kwb_header),
+ .hdr = (void*)&kwbimage_header,
+ .check_params = kwbimage_check_params,
+ .verify_header = kwbimage_verify_header,
+ .print_header = kwbimage_print_header,
+ .set_header = kwbimage_set_header,
+};
+
+void *kwbimage_get_tparams (void){
+ return (void *)&kwbimage_params;
+}
+
+/*
+ * Report Error if xflag is set in addition to default
+ */
+int kwbimage_check_params (struct mkimage_params *params)
+{
+ if (!strlen (params->imagename)) {
+ printf ("Error:%s - Configuration file not specified, "
+ "it is needed for kwbimage generation\n",
+ params->cmdname);
+ return CFG_INVALID;
+ }
+ return ((params->dflag && (params->fflag || params->lflag)) ||
+ (params->fflag && (params->dflag || params->lflag)) ||
+ (params->lflag && (params->dflag || params->fflag)) ||
+ (params->xflag) || !(strlen (params->imagename)));
+}
+
+uint32_t check_get_hexval (char *token)
+{
+ uint32_t hexval;
+
+ if (!sscanf (token, "%x", &hexval)) {
+ printf ("Error:%s[%d] - Invalid hex data\n", fname, lineno);
+ exit (EXIT_FAILURE);
+ }
+ return hexval;
+}
+
+/*
+ * Generates 8 bit checksum
+ */
+uint8_t kwbimage_checksum8 (void *start, uint32_t len, uint8_t csum)
+{
+ register uint8_t sum = csum;
+ volatile uint8_t *startp = (volatile uint8_t *)start;
+
+ do {
+ sum += *startp;
+ startp++;
+ } while (--len);
+ return (sum);
+}
+
+/*
+ * Generates 32 bit checksum
+ */
+uint32_t kwbimage_checksum32 (uint32_t *start, uint32_t len, uint32_t csum)
+{
+ register uint32_t sum = csum;
+ volatile uint32_t *startp = start;
+
+ do {
+ sum += *startp;
+ startp++;
+ len -= sizeof(uint32_t);
+ } while (len > 0);
+ return (sum);
+}
+
+void kwbimage_check_cfgdata (char *token, enum kwbimage_cmd cmdsw,
+ struct kwb_header *kwbhdr)
+{
+ bhr_t *mhdr = &kwbhdr->kwb_hdr;
+ extbhr_t *exthdr = &kwbhdr->kwb_exthdr;
+ int i;
+
+ switch (cmdsw) {
+ case CMD_BOOT_FROM:
+ i = get_table_entry_id (kwbimage_bootops,
+ "Kwbimage boot option", token);
+
+ if (i < 0)
+ goto INVL_DATA;
+
+ mhdr->blockid = i;
+ printf ("Preparing kirkwood boot image to boot "
+ "from %s\n", token);
+ break;
+ case CMD_NAND_ECC_MODE:
+ i = get_table_entry_id (kwbimage_eccmodes,
+ "NAND ecc mode", token);
+
+ if (i < 0)
+ goto INVL_DATA;
+
+ mhdr->nandeccmode = i;
+ printf ("Nand ECC mode = %s\n", token);
+ break;
+ case CMD_NAND_PAGE_SIZE:
+ mhdr->nandpagesize =
+ (uint16_t) check_get_hexval (token);
+ printf ("Nand page size = 0x%x\n",
+ mhdr->nandpagesize);
+ break;
+ case CMD_SATA_PIO_MODE:
+ mhdr->satapiomode =
+ (uint8_t) check_get_hexval (token);
+ printf ("Sata PIO mode = 0x%x\n",
+ mhdr->satapiomode);
+ break;
+ case CMD_DDR_INIT_DELAY:
+ mhdr->ddrinitdelay =
+ (uint16_t) check_get_hexval (token);
+ printf ("DDR init delay = %d msec\n",
+ mhdr->ddrinitdelay);
+ break;
+ case CMD_DATA:
+ exthdr->rcfg[datacmd_cnt].raddr =
+ (uint32_t) check_get_hexval (token);
+
+ break;
+ case CMD_INVALID:
+ default:
+INVL_DATA:
+ printf ("Error:%s[%d] - Invalid data\n", fname, lineno);
+ exit (EXIT_FAILURE);
+ }
+}
+
+void kwdimage_set_ext_header (struct kwb_header *kwbhdr, char* name) {
+ bhr_t *mhdr = &kwbhdr->kwb_hdr;
+ extbhr_t *exthdr = &kwbhdr->kwb_exthdr;
+ FILE *fd = NULL;
+ int j;
+ char *line = NULL;
+ char * token, *saveptr1, *saveptr2;
+ size_t len = 0;
+ enum kwbimage_cmd cmd;
+
+ fname = name;
+ /* set dram register offset */
+ exthdr->dramregsoffs = (uint32_t)&exthdr->rcfg - (uint32_t)mhdr;
+
+ if ((fd = fopen (name, "r")) == 0) {
+ printf ("Error:%s - Can't open\n", fname);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Simple kwimage.cfg file parser */
+ lineno=0;
+ while ((getline (&line, &len, fd)) != -1) {
+ lineno++;
+ token = strtok_r (line, "\r\n", &saveptr1);
+ /* drop all lines with zero tokens (= empty lines) */
+ if (token == NULL)
+ continue;
+
+ for (j = 0, cmd = CMD_INVALID, line = token; ; line = NULL) {
+ token = strtok_r (line, " \t", &saveptr2);
+ if (token == NULL)
+ break;
+ /* Drop all text starting with '#' as comments */
+ if (token[0] == '#')
+ break;
+
+ /* Process rest as valid config command line */
+ switch (j) {
+ case CFG_COMMAND:
+ cmd = get_table_entry_id (kwbimage_cmds,
+ "Kwbimage command", token);
+
+ if (cmd == CMD_INVALID)
+ goto INVL_CMD;
+ break;
+
+ case CFG_DATA0:
+ kwbimage_check_cfgdata (token, cmd, kwbhdr);
+ break;
+
+ case CFG_DATA1:
+ if (cmd != CMD_DATA)
+ goto INVL_CMD;
+
+ exthdr->rcfg[datacmd_cnt].rdata =
+ (uint32_t) check_get_hexval (token);
+
+ if (datacmd_cnt > KWBIMAGE_MAX_CONFIG ) {
+ printf ("Error:%s[%d] - Found more "
+ "than max(%d) allowed "
+ "data configurations\n",
+ fname, lineno,
+ KWBIMAGE_MAX_CONFIG);
+ exit (EXIT_FAILURE);
+ } else
+ datacmd_cnt++;
+ break;
+
+ default:
+ /*
+ * Command format permits maximum three
+ * strings on a line, i.e. command and data
+ * if more than two are observed, then error
+ * will be reported
+ */
+INVL_CMD:
+ printf ("Error:%s[%d] - Invalid command\n",
+ fname, lineno);
+ exit (EXIT_FAILURE);
+ }
+ j++;
+ }
+ }
+ if (line)
+ free (line);
+
+ fclose (fd);
+}
+
+void kwbimage_set_header (char *ptr, struct stat *sbuf, int ifd,
+ struct mkimage_params *params)
+{
+ struct kwb_header *hdr = (struct kwb_header *)ptr;
+ bhr_t *mhdr = &hdr->kwb_hdr;
+ extbhr_t *exthdr = &hdr->kwb_exthdr;
+ uint32_t checksum;
+ int size;
+
+ /* Build and add image checksum header */
+ checksum = kwbimage_checksum32 ((uint32_t *)ptr, sbuf->st_size, 0);
+
+ size = write (ifd, &checksum, sizeof(uint32_t));
+ if (size != sizeof(uint32_t)) {
+ printf ("Error:%s - Checksum write %d bytes %s\n",
+ params->cmdname, params->imagefile, size);
+ exit (EXIT_FAILURE);
+ }
+
+ sbuf->st_size += sizeof(uint32_t);
+
+ mhdr->blocksize = sbuf->st_size - sizeof(struct kwb_header);
+ mhdr->srcaddr = sizeof(struct kwb_header);
+ mhdr->destaddr= params->addr;
+ mhdr->execaddr =params->ep;
+ mhdr->ext = 0x1; /* header extension appended */
+
+ kwdimage_set_ext_header (hdr, params->imagename);
+ /* calculate checksums */
+ mhdr->checkSum = kwbimage_checksum8 ((void *)mhdr, sizeof(bhr_t), 0);
+ exthdr->checkSum = kwbimage_checksum8 ((void *)exthdr,
+ sizeof(extbhr_t), 0);
+}
+
+
+/* -l support functions */
+int kwbimage_verify_header (char *ptr, int image_size,
+ struct mkimage_params *params)
+{
+ struct kwb_header *hdr = (struct kwb_header *)ptr;
+ bhr_t *mhdr = &hdr->kwb_hdr;
+ extbhr_t *exthdr = &hdr->kwb_exthdr;
+ uint8_t calc_hdrcsum;
+ uint8_t calc_exthdrcsum;
+
+ calc_hdrcsum = kwbimage_checksum8 ((void *)mhdr,
+ sizeof(bhr_t) - sizeof(uint8_t), 0);
+ if (calc_hdrcsum != mhdr->checkSum) {
+ return -FDT_ERR_BADSTRUCTURE; /* mhdr csum not matched */
+ }
+ calc_exthdrcsum = kwbimage_checksum8 ((void *)exthdr,
+ sizeof(extbhr_t) - sizeof(uint8_t), 0);
+ if (calc_hdrcsum != mhdr->checkSum) {
+ return -FDT_ERR_BADSTRUCTURE; /* exthdr csum not matched */
+ }
+ return 0;
+}
+
+void kwbimage_print_header (char *ptr)
+{
+ struct kwb_header *hdr = (struct kwb_header *) ptr;
+ bhr_t *mhdr = &hdr->kwb_hdr;
+
+ printf ("Image Type: Kirkwood Boot from %s Image\n",
+ get_table_entry_name (kwbimage_bootops,
+ "Kwbimage boot option",
+ (int) mhdr->blockid));
+
+ printf ("Data Size: ");
+ genimg_print_size (mhdr->blocksize - sizeof(uint32_t));
+ printf ("Load Address: %08x\n", mhdr->destaddr);
+ printf ("Entry Point: %08x\n", mhdr->execaddr);
+}
+
diff --git a/tools/kwbimage.h b/tools/kwbimage.h
new file mode 100644
index 0000000..8a24cb4
--- /dev/null
+++ b/tools/kwbimage.h
@@ -0,0 +1,118 @@
+/*
+ * (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
+ */
+
+#ifndef _KWBIMAGE_H_
+#define _KWBIMAGE_H_
+
+#include <stdint.h>
+
+#define KWBIMAGE_MAX_CONFIG ((0x1dc - 0x20)/sizeof(struct reg_config))
+#define MAX_TEMPBUF_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
+
+enum kwbimage_cmd {
+ CMD_INVALID,
+ CMD_BOOT_FROM,
+ CMD_NAND_ECC_MODE,
+ CMD_NAND_PAGE_SIZE,
+ CMD_SATA_PIO_MODE,
+ CMD_DDR_INIT_DELAY,
+ CMD_DATA
+};
+
+enum kwbimage_cmd_types {
+ CFG_INVALID = -1,
+ CFG_COMMAND,
+ CFG_DATA0,
+ CFG_DATA1
+};
+
+/* typedefs */
+typedef struct bhr_t {
+ uint8_t blockid; /*0 */
+ uint8_t nandeccmode; /*1 */
+ uint16_t nandpagesize; /*2-3 */
+ uint32_t blocksize; /*4-7 */
+ uint32_t rsvd1; /*8-11 */
+ uint32_t srcaddr; /*12-15 */
+ uint32_t destaddr; /*16-19 */
+ uint32_t execaddr; /*20-23 */
+ uint8_t satapiomode; /*24 */
+ uint8_t rsvd3; /*25 */
+ uint16_t ddrinitdelay; /*26-27 */
+ uint16_t rsvd2; /*28-29 */
+ uint8_t ext; /*30 */
+ uint8_t checkSum; /*31 */
+} bhr_t, *pbhr_t;
+
+struct reg_config {
+ uint32_t raddr;
+ uint32_t rdata;
+};
+
+typedef struct extbhr_t {
+ uint32_t dramregsoffs;
+ uint8_t rsrvd1[0x20 - sizeof(uint32_t)];
+ struct reg_config rcfg[KWBIMAGE_MAX_CONFIG];
+ uint8_t rsrvd2[7];
+ uint8_t checkSum;
+} extbhr_t, *pextbhr_t;
+
+struct kwb_header {
+ bhr_t kwb_hdr;
+ extbhr_t kwb_exthdr;
+};
+
+/*
+ * functions
+ */
+uint8_t kwbimage_checksum8 (void *start, uint32_t len, uint8_t csum);
+uint32_t kwbimage_checksum32 (uint32_t *start, uint32_t len, uint32_t csum);
+
+/*
+ * callback functions for mkimage
+ */
+void *kwbimage_get_tparams (void);
+int kwbimage_check_params (struct mkimage_params *params);
+int kwbimage_verify_header (char *ptr, int image_size,
+ struct mkimage_params *params);
+void kwbimage_print_header (char *ptr);
+void kwbimage_set_header (char *ptr, struct stat *sbuf, int ifd,
+ struct mkimage_params *params);
+
+#endif /* _KWBIMAGE_H_ */
diff --git a/tools/mkimage.c b/tools/mkimage.c
index 3a3cb19..7e29610 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -25,6 +25,7 @@
#include "mkimage.h"
#include <image.h>
#include "default_image.h"
+#include "kwbimage.h"
static void copy_file(int, const char *, int);
static void usage(void);
@@ -59,6 +60,7 @@ static struct image_functions image_ftable[] = {
{image_get_tparams,}, /* for IH_TYPE_SCRIPT */
{image_get_tparams,}, /* for IH_TYPE_STANDALONE */
{fitimage_get_tparams,}, /* for IH_TYPE_FLATDT */
+ {kwbimage_get_tparams,}, /* for IH_TYPE_KWBIMAGE */
};
int
@@ -111,7 +113,7 @@ main (int argc, char **argv)
(params.opt_type =
genimg_get_type_id (*++argv)) < 0)
usage ();
- if (image_ftable_size <= params.opt_type) {
+ if (image_ftable_size < params.opt_type) {
fprintf (stderr,
"%s: Unsupported image type %s\n",
params.cmdname, *argv);
--
1.5.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v4 8/8] Kirkwood: Sheevaplug: kwimage configuration
2009-07-28 18:04 ` [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage) Prafulla Wadaskar
@ 2009-07-28 18:04 ` Prafulla Wadaskar
2009-08-07 23:24 ` [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage) Wolfgang Denk
1 sibling, 0 replies; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-07-28 18:04 UTC (permalink / raw)
To: u-boot
Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
Change log:
v2: updated as per review feedback for v1
v3: some white spaces removed
v4: tuned as per new kwbimage parsing (DATA command added for register setup)
board/Marvell/sheevaplug/config.mk | 3 +
board/Marvell/sheevaplug/kwbimage.cfg | 162 +++++++++++++++++++++++++++++++++
2 files changed, 165 insertions(+), 0 deletions(-)
create mode 100644 board/Marvell/sheevaplug/kwbimage.cfg
diff --git a/board/Marvell/sheevaplug/config.mk b/board/Marvell/sheevaplug/config.mk
index a4ea769..2bd9f79 100644
--- a/board/Marvell/sheevaplug/config.mk
+++ b/board/Marvell/sheevaplug/config.mk
@@ -23,3 +23,6 @@
#
TEXT_BASE = 0x00600000
+
+# Kirkwood Boot Image configuration file
+KWD_CONFIG = $(SRCTREE)/board/$(BOARDDIR)/kwbimage.cfg
diff --git a/board/Marvell/sheevaplug/kwbimage.cfg b/board/Marvell/sheevaplug/kwbimage.cfg
new file mode 100644
index 0000000..6c47d62
--- /dev/null
+++ b/board/Marvell/sheevaplug/kwbimage.cfg
@@ -0,0 +1,162 @@
+#
+# (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 using bootrom header extension
+# Maximum KWBIMAGE_MAX_CONFIG configurations allowed
+
+# Configure RGMII-0 interface pad voltage to 1.8V
+DATA 0xFFD100e0 0x1b1b1b9b
+
+#Dram initalization for SINGLE x16 CL=5 @ 400MHz
+DATA 0xFFD01400 0x43000c30 # DDR Configuration register
+# bit13-0: 0xc30 (3120 DDR2 clks refresh rate)
+# bit23-14: zero
+# bit24: 1= enable exit self refresh mode on DDR access
+# bit25: 1 required
+# bit29-26: zero
+# bit31-30: 01
+
+DATA 0xFFD01404 0x37543000 # DDR Controller Control Low
+# bit 4: 0=addr/cmd in smame cycle
+# bit 5: 0=clk is driven during self refresh, we don't care for APX
+# bit 6: 0=use recommended falling edge of clk for addr/cmd
+# bit14: 0=input buffer always powered up
+# bit18: 1=cpu lock transaction enabled
+# bit23-20: 5=recommended value for CL=5 and STARTBURST_DEL disabled bit31=0
+# bit27-24: 7= CL+2, STARTBURST sample stages, for freqs 400MHz, unbuffered DIMM
+# bit30-28: 3 required
+# bit31: 0=no additional STARTBURST delay
+
+DATA 0xFFD01408 0x22125451 # DDR Timing (Low) (active cycles value +1)
+# bit3-0: TRAS lsbs
+# bit7-4: TRCD
+# bit11- 8: TRP
+# bit15-12: TWR
+# bit19-16: TWTR
+# bit20: TRAS msb
+# bit23-21: 0x0
+# bit27-24: TRRD
+# bit31-28: TRTP
+
+DATA 0xFFD0140C 0x00000a33 # DDR Timing (High)
+# bit6-0: TRFC
+# bit8-7: TR2R
+# bit10-9: TR2W
+# bit12-11: TW2W
+# bit31-13: zero required
+
+DATA 0xFFD01410 0x00000099 # DDR Address Control
+# bit1-0: 01, Cs0width=x16
+# bit3-2: 10, Cs0size=512Mb
+# bit5-4: 01, Cs1width=x16
+# bit7-6: 10, Cs1size=512Mb
+# bit9-8: 00, Cs2width=nonexistent
+# bit11-10: 00, Cs2size =nonexistent
+# bit13-12: 00, Cs3width=nonexistent
+# bit15-14: 00, Cs3size =nonexistent
+# bit16: 0, Cs0AddrSel
+# bit17: 0, Cs1AddrSel
+# bit18: 0, Cs2AddrSel
+# bit19: 0, Cs3AddrSel
+# bit31-20: 0 required
+
+DATA 0xFFD01414 0x00000000 # DDR Open Pages Control
+# bit0: 0, OpenPage enabled
+# bit31-1: 0 required
+
+DATA 0xFFD01418 0x00000000 # DDR Operation
+# bit3-0: 0x0, DDR cmd
+# bit31-4: 0 required
+
+DATA 0xFFD0141C 0x00000C52 # DDR Mode
+# bit2-0: 2, BurstLen=2 required
+# bit3: 0, BurstType=0 required
+# bit6-4: 4, CL=5
+# bit7: 0, TestMode=0 normal
+# bit8: 0, DLL reset=0 normal
+# bit11-9: 6, auto-precharge write recovery ????????????
+# bit12: 0, PD must be zero
+# bit31-13: 0 required
+
+DATA 0xFFD01420 0x00000040 # DDR Extended Mode
+# bit0: 0, DDR DLL enabled
+# bit1: 0, DDR drive strenght normal
+# bit2: 0, DDR ODT control lsd (disabled)
+# bit5-3: 000, required
+# bit6: 1, DDR ODT control msb, (disabled)
+# bit9-7: 000, required
+# bit10: 0, differential DQS enabled
+# bit11: 0, required
+# bit12: 0, DDR output buffer enabled
+# bit31-13: 0 required
+
+DATA 0xFFD01424 0x0000F17F # DDR Controller Control High
+# bit2-0: 111, required
+# bit3 : 1 , MBUS Burst Chop disabled
+# bit6-4: 111, required
+# bit7 : 0
+# bit8 : 1 , add writepath sample stage, must be 1 for DDR freq >= 300MHz
+# bit9 : 0 , no half clock cycle addition to dataout
+# bit10 : 0 , 1/4 clock cycle skew enabled for addr/ctl signals
+# bit11 : 0 , 1/4 clock cycle skew disabled for write mesh
+# bit15-12: 1111 required
+# bit31-16: 0 required
+
+DATA 0xFFD01428 0x00085520 # DDR2 ODT Read Timing (default values)
+DATA 0xFFD0147C 0x00008552 # DDR2 ODT Write Timing (default values)
+
+DATA 0xFFD01500 0x00000000 # CS[0]n Base address to 0x0
+DATA 0xFFD01504 0x0FFFFFF1 # CS[0]n Size
+# bit0: 1, Window enabled
+# bit1: 0, Write Protect disabled
+# bit3-2: 00, CS0 hit selected
+# bit23-4: ones, required
+# bit31-24: 0x0F, Size (i.e. 256MB)
+
+DATA 0xFFD01508 0x10000000 # CS[1]n Base address to 256Mb
+DATA 0xFFD0150C 0x0FFFFFF5 # CS[1]n Size 256Mb Window enabled for CS1
+
+DATA 0xFFD01514 0x00000000 # CS[2]n Size, window disabled
+DATA 0xFFD0151C 0x00000000 # CS[3]n Size, window disabled
+
+DATA 0xFFD01494 0x00030000 # DDR ODT Control (Low)
+DATA 0xFFD01498 0x00000000 # DDR ODT Control (High)
+# bit1-0: 00, ODT0 controlled by ODT Control (low) register above
+# bit3-2: 01, ODT1 active NEVER!
+# bit31-4: zero, required
+
+DATA 0xFFD0149C 0x0000E803 # CPU ODT Control
+DATA 0xFFD01480 0x00000001 # DDR Initialization Control
+#bit0=1, enable DDR init upon this register write
+
+# End of Header extension
+DATA 0x0 0x0
--
1.5.3.3
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 1/8] tools: mkimage : bugfix returns correct value for list command
2009-07-28 18:04 [U-Boot] [PATCH 1/8] tools: mkimage : bugfix returns correct value for list command Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific Prafulla Wadaskar
@ 2009-08-07 22:11 ` Wolfgang Denk
1 sibling, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2009-08-07 22:11 UTC (permalink / raw)
To: u-boot
Dear Prafulla Wadaskar,
In message <1248804270-13715-1-git-send-email-prafulla@marvell.com> you wrote:
> List command always return "EXIT_SUCCESS" even in case of
> failure by any means.
>
> This patch return 0 if list command is sucessful,
> returns negative value reported by check_header functions
>
> Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
> ---
> tools/mkimage.c | 27 +++++++++++++--------------
> 1 files changed, 13 insertions(+), 14 deletions(-)
Just a minor nitpick:
...
> - if (fdt_check_header (ptr)) {
> - /* old-style image */
> - image_verify_header ((char *)ptr, sbuf.st_size);
> - image_print_contents ((image_header_t *)ptr);
> - } else {
> - /* FIT image */
> + if (!(retval = fdt_check_header (ptr))) /* FIT image */
> fit_print_contents (ptr);
> - }
> + else if (!(retval = image_verify_header (
> + (char *)ptr, sbuf.st_size))) /* old-style image */
> + image_print_contents ((image_header_t *)ptr);
The resulting code looks ugly to me. Please write as:
if (!(retval = fdt_check_header(ptr))) {
/* FIT image */
fit_print_contents (ptr);
} else if (!(retval = image_verify_header((char *)ptr,
sbuf.st_size))) {
/* old-style image */
image_print_contents ((image_header_t *)ptr);
}
Thanks.
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
There is a multi-legged creature crawling on your shoulder.
-- Spock, "A Taste of Armageddon", stardate 3193.9
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific
2009-07-28 18:04 ` [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files Prafulla Wadaskar
@ 2009-08-07 22:30 ` Wolfgang Denk
1 sibling, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2009-08-07 22:30 UTC (permalink / raw)
To: u-boot
Dear Prafulla Wadaskar,
In message <1248804270-13715-2-git-send-email-prafulla@marvell.com> you wrote:
> This is first step towards cleaning mkimage code for kwbimage
> support in clean way. Current mkimage code is very specific to
> uimge generation whereas the same framework can be used to
> generate other image types like kwbimage.
> For this, the architecture of mkimage code need to modified.
>
> Here is the brief plan for the same:-
> a) Abstract image specific code to saperate file (this patch)
> b) Move image header code from mkimage.c to respective
> image specific code
> c) Implement callback function for image specific functions
> d) Add kwbimage type support to this framework
>
> In this patch-
> 1. Image specific code abstracted from mkimage.c to
> default_image.c/h to make mkimage code more generic
> 2. struct mkimage_params introduced to pass basic mkimage
> specific flags and variables to image specific functions
>
> Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
> ---
> tools/Makefile | 4 +
> tools/default_image.c | 227 ++++++++++++++++++++++++++++++
> tools/default_image.h | 36 +++++
> tools/mkimage.c | 368 ++++++++++++++-----------------------------------
> tools/mkimage.h | 25 ++++
> 5 files changed, 394 insertions(+), 266 deletions(-)
> create mode 100644 tools/default_image.c
> create mode 100644 tools/default_image.h
Just nitpicking...
> diff --git a/tools/Makefile b/tools/Makefile
> index b5a1e39..6ef15d9 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -171,6 +171,7 @@ $(obj)img2srec$(SFX): $(obj)img2srec.o
> $(STRIP) $@
>
> $(obj)mkimage$(SFX): $(obj)mkimage.o $(obj)crc32.o $(obj)image.o $(obj)md5.o \
> + $(obj)default_image.o \
> $(obj)sha1.o $(LIBFDT_OBJS) $(obj)os_support.o
Please let's sort the files...
> $(CC) $(CFLAGS) $(HOST_LDFLAGS) -o $@ $^
> $(STRIP) $@
> @@ -203,6 +204,9 @@ $(obj)bin2header$(SFX): $(obj)bin2header.o
> $(obj)image.o: $(SRCTREE)/common/image.c
> $(CC) -g $(FIT_CFLAGS) -c -o $@ $<
>
> +$(obj)default_image.o: $(SRCTREE)/tools/default_image.c
> + $(CC) -g $(FIT_CFLAGS) -c -o $@ $<
> +
Please sort here, too.
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index b7b383a..66d6c8e 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -24,28 +24,30 @@
...
> +struct mkimage_params params = {
...
> + .opt_os = IH_OS_LINUX,
> + .opt_arch = IH_ARCH_PPC,
> + .opt_type = IH_TYPE_KERNEL,
> + .opt_comp = IH_COMP_GZIP,
> + .opt_dtc = MKIMAGE_DEFAULT_DTC_OPTIONS,
...
> + .imagename = "",
> + .datafile = "",
> + .imagefile = "",
> + .cmdname = "",
I see no reason to initialize these strings. Please omit.
> +};
...
> case 'A':
> if ((--argc <= 0) ||
> - (opt_arch = genimg_get_arch_id (*++argv)) < 0)
> + (params.opt_arch =
> + genimg_get_arch_id (*++argv)) < 0)
> usage ();
> goto NXTARG;
> case 'C':
> if ((--argc <= 0) ||
> - (opt_comp = genimg_get_comp_id (*++argv)) < 0)
> + (params.opt_comp =
> + genimg_get_comp_id (*++argv)) < 0)
The new code looks really ugly here and in all the following case:s;
This is mostly caused by the fact that the new code is much longer
"params.opt_XXX" versus "opt_XXX"). With the new code all the options
are in the params structure anyway, so ther eis no danger to confuse
these withother variables, so we should omit the "opt_" part, I think.
This gives:
struct mkimage_params params = {
...
.os = IH_OS_LINUX,
.arch = IH_ARCH_PPC,
.type = IH_TYPE_KERNEL,
.comp = IH_COMP_GZIP,
.dtc = MKIMAGE_DEFAULT_DTC_OPTIONS,
...
};
...
case 'A':
if ((--argc <= 0) ||
(params.arch = genimg_get_arch_id(*++argv)) < 0) {
usage ();
}
goto NXTARG;
case 'C':
if ((--argc <= 0) ||
(params.comp = genimg_get_comp_id(*++argv)) < 0) {
usage ();
}
goto NXTARG;
...
etc.
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
Extended Epstein-Heisenberg Principle: In an R & D orbit, only 2 of
the existing 3 parameters can be defined simultaneously. The parame-
ters are: task, time and resources ($).
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files
2009-07-28 18:04 ` [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support Prafulla Wadaskar
@ 2009-08-07 22:43 ` Wolfgang Denk
1 sibling, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2009-08-07 22:43 UTC (permalink / raw)
To: u-boot
Dear Prafulla Wadaskar,
In message <1248804270-13715-3-git-send-email-prafulla@marvell.com> you wrote:
> This is second step towards cleaning mkimage code for kwbimage
> support in clean way. In this patch-
> 1. The image_get_header_size function call is replaced by sizeof(image_header_t)
> in default_image.c
I still fail to understand why this would be needed, or even wanted.
> 2. image header code moved form mkimage.c to default_image .c
This looks incorrect to me (not to mention the "form/from" typo):
I see no code being moved, only variable declarations.
> +/*
> + * Default image parameters
> + */
> +struct image_type_params defimage_params = {
> + .header_size = sizeof(image_header_t),
> + .hdr = (void*)&header,
> +};
> +
> +void *image_get_tparams (void){
> + return (void *)&defimage_params;
> +}
We need better documentation. Why would that function be needed?
> int image_check_params (struct mkimage_params *params)
> {
> return ((params->dflag && (params->fflag || params->lflag)) ||
> @@ -100,13 +114,13 @@ void image_set_header (char *ptr, struct stat *sbuf,
> image_header_t * 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 + sizeof(image_header_t)),
> + sbuf->st_size - sizeof(image_header_t));
I cannot help it, but this change looks like a step backward to me.
Why don't we use the header_size entry from the image_type_params
here, too?
...
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -49,9 +49,6 @@ struct mkimage_params params = {
> .cmdname = "",
> };
>
> -image_header_t header;
> -image_header_t *hdr = &header;
> -
> int
> main (int argc, char **argv)
> {
> @@ -59,6 +56,7 @@ main (int argc, char **argv)
> struct stat sbuf;
> unsigned char *ptr;
> int retval;
> + struct image_type_params *tparams = image_get_tparams ();
>
> params.cmdname = *argv;
>
> @@ -163,7 +161,7 @@ NXTARG: ;
> params.ep = params.addr;
> /* If XIP, entry point must be after the U-Boot header */
> if (params.xflag)
> - params.ep += image_get_header_size ();
> + params.ep += tparams->header_size;
Would it not be better to stick with the image_get_header_size()
function, and instead make it aware which image type we are asking
for?
The old code was homnogenuous: it consistently used
image_get_header_size(); now we have a sizeof() here and a
ptr->header_size there.
I really dislike that.
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
"Here's a fish hangs in the net like a poor man's right in the law.
'Twill hardly come out." - Shakespeare, Pericles, Act II, Scene 1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support
2009-07-28 18:04 ` [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 5/8] tools: mkimage: Making genimg_print_size API global Prafulla Wadaskar
@ 2009-08-07 22:55 ` Wolfgang Denk
1 sibling, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2009-08-07 22:55 UTC (permalink / raw)
To: u-boot
Dear Prafulla Wadaskar,
In message <1248804270-13715-4-git-send-email-prafulla@marvell.com> you wrote:
> This is Third step towards cleaning mkimage code for kwbimage
BTW - I think "cleaning mkimage code" is not correct. This is not a
clean up, but a major rework.
> 1. callback functions are used in mkimage.c those are defined
> in defaut_image.c for currently supported images.
Typo...
> 2. scan loop implemented for image header verify and print
What does that mean? Please provide better documentation / comments so
we understand what you are doing.
> void image_print_header (char *ptr)
> {
> - image_header_t * hdr = (image_header_t *)ptr;
> -
> - image_print_contents (hdr);
> + image_print_contents ((image_header_t *)ptr);
> }
The new code looks uglier than the old one. Please don;t change.
> +/*
> + * FIT image support
> + */
> +int
> +fitimage_verify_header (char *ptr, int image_size,
> + struct mkimage_params *params)
> +{
> + fdt_check_header ((void *)ptr);
> +}
Do other image types need the provided parameters?
> +void fitimage_print_header (char *ptr)
> +{
> + fit_print_contents ((void *)ptr);
> +}
Maybe we can omit this function, then?
> +struct image_type_params fitimage_params = {
> + .header_size = sizeof(image_header_t),
> + .hdr = (void*)&header,
> + .check_params = image_check_params,
> + .verify_header = fitimage_verify_header,
> + .print_header = fitimage_print_header,
... and just use ".print_header = fit_print_contents," here?
...
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index d1636d8..3a3cb19 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -49,14 +49,28 @@ struct mkimage_params params = {
> .cmdname = "",
> };
>
> +static struct image_functions image_ftable[] = {
> + {image_get_tparams,}, /* for IH_TYPE_INVALID */
> + {image_get_tparams,}, /* for IH_TYPE_FILESYSTEM */
> + {image_get_tparams,}, /* for IH_TYPE_FIRMWARE */
> + {image_get_tparams,}, /* for IH_TYPE_KERNEL */
> + {image_get_tparams,}, /* for IH_TYPE_MULTI */
> + {image_get_tparams,}, /* for IH_TYPE_RAMDISK */
> + {image_get_tparams,}, /* for IH_TYPE_SCRIPT */
> + {image_get_tparams,}, /* for IH_TYPE_STANDALONE */
> + {fitimage_get_tparams,}, /* for IH_TYPE_FLATDT */
> +};
What is this? Please explain. The table looks pretty bogus to me. If
all but one use the same function, why do we need such a table then?
> int
> main (int argc, char **argv)
> {
> int ifd = -1;
> struct stat sbuf;
> unsigned char *ptr;
> - int retval;
> + int i, retval;
> struct image_type_params *tparams = image_get_tparams ();
Such initialization here looks bad to me, as it is in no way
correlated to the image_ftable[] settinga above.
> + struct image_type_params *scantparams;
> + int image_ftable_size = ARRAY_SIZE(image_ftable);
>
> params.cmdname = *argv;
>
> @@ -97,6 +111,15 @@ main (int argc, char **argv)
> (params.opt_type =
> genimg_get_type_id (*++argv)) < 0)
> usage ();
> + if (image_ftable_size <= params.opt_type) {
Logic here looks inverted to me. And is the <= correct?
> + /*
> + * Scan image headers for all supported types
> + * and print if veryfy_header sucessful
Typo.
I have to admit that I do not understand what you are actually doing
here.
> + */
> + for (i = image_ftable_size - 1; i != IH_TYPE_INVALID; i--) {
> + scantparams = image_ftable[i].get_tparams ();
> + fprintf (stderr, "%s: Verify header for type=%s\n",
> + params.cmdname, genimg_get_type_name (i));
> +
> + retval = scantparams->verify_header (
> (char *)ptr, sbuf.st_size,
> - ¶ms))) /* old-style image */
> - image_print_contents ((image_header_t *)ptr);
> + ¶ms);
> +
> + if (retval == 0) {
> + scantparams->print_header ((char *)ptr);
> + break;
> + }
> + }
Maybe you can explain what you're doing?
> (void) munmap((void *)ptr, sbuf.st_size);
> (void) close (ifd);
> @@ -333,9 +368,9 @@ NXTARG: ;
> exit (EXIT_FAILURE);
> }
>
> - image_set_header (ptr, &sbuf, ¶ms);
> + tparams->set_header (ptr, &sbuf, ifd, ¶ms);
>
> - image_print_header (ptr);
> + tparams->print_header (ptr);
The longer I read this, the more I dislike the tparams-> code. I tend
to think the type switching should be transparent, and not visible in
the code.
> +/*
> + * Image type specific function pointers list
> + */
> +struct image_functions {
> + void * (*get_tparams) (void);
> +};
What's that?
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
Why do you need to declare this here? Please use standard features.
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
Technology is dominated by those who manage what they do not under-
stand.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global
2009-07-28 18:04 ` [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage) Prafulla Wadaskar
@ 2009-08-07 23:02 ` Wolfgang Denk
2009-08-09 13:54 ` Prafulla Wadaskar
1 sibling, 1 reply; 17+ messages in thread
From: Wolfgang Denk @ 2009-08-07 23:02 UTC (permalink / raw)
To: u-boot
Dear Prafulla Wadaskar,
In message <1248804270-13715-6-git-send-email-prafulla@marvell.com> you wrote:
>
> diff --git a/include/image.h b/include/image.h
> index 88a13ab..f119cee 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -168,6 +168,15 @@
> #define IH_MAGIC 0x27051956 /* Image Magic Number */
> #define IH_NMLEN 32 /* Image Name Length */
>
> +typedef struct table_entry {
> + int id; /* as defined in image.h */
> + char *sname; /* short (input) name */
> + char *lname; /* long (output) name */
> +} table_entry_t;
Now read this code again, with the distance of a couple of days, and
tell me what you think.
"as defined in image.h" - hey, this _is_ image.h !
So, "struct table_entry" - what sort of table is this?
"short (input) name", "long (output) name" - what the heck is this
about?
In the old version, the declaration of the struct was followed by the
declarations of the actual tables, so we could _see_ what was meant.
You rip the code out of context, which makes it unreadable and
ununderstandable.
This is a really bad idea, it seems.
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
"He only drinks when he gets depressed." "Why does he get depressed?"
"Sometimes it's because he hasn't had a drink."
- Terry Pratchett, _Men at Arms_
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage)
2009-07-28 18:04 ` [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage) Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH v4 8/8] Kirkwood: Sheevaplug: kwimage configuration Prafulla Wadaskar
@ 2009-08-07 23:24 ` Wolfgang Denk
1 sibling, 0 replies; 17+ messages in thread
From: Wolfgang Denk @ 2009-08-07 23:24 UTC (permalink / raw)
To: u-boot
Dear Prafulla Wadaskar,
In message <1248804270-13715-7-git-send-email-prafulla@marvell.com> you wrote:
> This is Third step towards cleaning mkimage code for kwbimage
> support in clean way.
Umm... The "Third step" was already in patch 4/8. I guess you have to
check your commit messages better...
> diff --git a/tools/Makefile b/tools/Makefile
> index 6ef15d9..07a1c27 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -172,6 +172,7 @@ $(obj)img2srec$(SFX): $(obj)img2srec.o
>
> $(obj)mkimage$(SFX): $(obj)mkimage.o $(obj)crc32.o $(obj)image.o $(obj)md5.o \
> $(obj)default_image.o \
> + $(obj)kwbimage.o \
> $(obj)sha1.o $(LIBFDT_OBJS) $(obj)os_support.o
Please keep sorted.
> --- /dev/null
> +++ b/tools/kwbimage.c
> @@ -0,0 +1,364 @@
> +/*
> + * (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 "mkimage.h"
> +#include <image.h>
> +#include "kwbimage.h"
> +
> +/*
> + * Suported commands for configuration file
> + */
> +static table_entry_t kwbimage_cmds[] = {
> + { CMD_BOOT_FROM, "BOOT_FROM", "", },
> + { CMD_NAND_ECC_MODE, "NAND_ECC_MODE", "", },
> + { CMD_NAND_PAGE_SIZE, "NAND_PAGE_SIZE", "", },
> + { CMD_SATA_PIO_MODE, "SATA_PIO_MODE", "", },
> + { CMD_DDR_INIT_DELAY, "DDR_INIT_DELAY", "", },
> + { CMD_DATA, "DATA", "", },
> + { CMD_INVALID, "", "", },
> +};
Why don't these entries have any printable names associated?
> +/*
> + * Suported Boot options for configuration file
Typo.
...
> +/*
> + * Suported NAND ecc options configuration file
> + */
> +static table_entry_t kwbimage_eccmodes[] = {
> + { IBR_HDR_ECC_DEFAULT, "default", "", },
> + { IBR_HDR_ECC_FORCED_HAMMING,"hamming", "", },
> + { IBR_HDR_ECC_FORCED_RS, "rs", "", },
> + { IBR_HDR_ECC_DISABLED, "disabled", "", },
> + { -1, "", "", },
> +};
Why don't these entries have any printable names associated?
> +uint32_t check_get_hexval (char *token)
> +{
> + uint32_t hexval;
> +
> + if (!sscanf (token, "%x", &hexval)) {
> + printf ("Error:%s[%d] - Invalid hex data\n", fname, lineno);
> + exit (EXIT_FAILURE);
> + }
> + return hexval;
> +}
scanf() is not a good or reliable conversion tool. Better avoid it.
Use strto[u]l() instead.
In the error case, please also print the string that you cannot
convert.
> +/*
> + * Generates 8 bit checksum
> + */
> +uint8_t kwbimage_checksum8 (void *start, uint32_t len, uint8_t csum)
> +{
> + register uint8_t sum = csum;
> + volatile uint8_t *startp = (volatile uint8_t *)start;
> +
> + do {
> + sum += *startp;
> + startp++;
> + } while (--len);
"startp" is incorrectly named. It is only _start_ at the beginning.
Just call it "p" :-)
Maybe you should add handling for the "len=0" case.
> +/*
> + * Generates 32 bit checksum
> + */
> +uint32_t kwbimage_checksum32 (uint32_t *start, uint32_t len, uint32_t csum)
> +{
> + register uint32_t sum = csum;
> + volatile uint32_t *startp = start;
> +
> + do {
> + sum += *startp;
> + startp++;
> + len -= sizeof(uint32_t);
> + } while (len > 0);
> + return (sum);
You should error handling for the cas ethet len is not an even
multiple of sizeof(uint32_t).
Maybe you should add handling for the "len=0" case.
...
> + case CMD_NAND_PAGE_SIZE:
> + mhdr->nandpagesize =
> + (uint16_t) check_get_hexval (token);
> + printf ("Nand page size = 0x%x\n",
> + mhdr->nandpagesize);
Indentation incorrect.
> + break;
> + case CMD_SATA_PIO_MODE:
> + mhdr->satapiomode =
> + (uint8_t) check_get_hexval (token);
> + printf ("Sata PIO mode = 0x%x\n",
> + mhdr->satapiomode);
Indentation incorrect.
> + break;
> + case CMD_DDR_INIT_DELAY:
> + mhdr->ddrinitdelay =
> + (uint16_t) check_get_hexval (token);
> + printf ("DDR init delay = %d msec\n",
> + mhdr->ddrinitdelay);
Indentation incorrect.
> + break;
> + case CMD_DATA:
> + exthdr->rcfg[datacmd_cnt].raddr =
> + (uint32_t) check_get_hexval (token);
Umm... why are all these casts needed?
Please get rid of these.
> + break;
> + case CMD_INVALID:
> + default:
> +INVL_DATA:
> + printf ("Error:%s[%d] - Invalid data\n", fname, lineno);
> + exit (EXIT_FAILURE);
> + }
I really dislike this jumping into another case. Please don;t. Move
this code out of the switch().
> +}
> +
> +void kwdimage_set_ext_header (struct kwb_header *kwbhdr, char* name) {
How about some comments what all these functions are supposed to do?
> + case CFG_DATA1:
> + if (cmd != CMD_DATA)
> + goto INVL_CMD;
> +
> + exthdr->rcfg[datacmd_cnt].rdata =
> + (uint32_t) check_get_hexval (token);
> +
> + if (datacmd_cnt > KWBIMAGE_MAX_CONFIG ) {
> + printf ("Error:%s[%d] - Found more "
> + "than max(%d) allowed "
> + "data configurations\n",
> + fname, lineno,
> + KWBIMAGE_MAX_CONFIG);
> + exit (EXIT_FAILURE);
Indentation incorrect.
> + } else
> + datacmd_cnt++;
> + break;
> +
> + default:
> + /*
> + * Command format permits maximum three
> + * strings on a line, i.e. command and data
> + * if more than two are observed, then error
> + * will be reported
> + */
That does not make sense. If three three stings are permitted, then
why throwing an error when there are three?
And "three strings on a line, i.e. command and data" sounds incorrect,
too.
> +INVL_CMD:
> + printf ("Error:%s[%d] - Invalid command\n",
> + fname, lineno);
> + exit (EXIT_FAILURE);
Please do not jump right in the middle of a switch. Move this error
code out of the switch.
> +/* -l support functions */
> +int kwbimage_verify_header (char *ptr, int image_size,
> + struct mkimage_params *params)
> +{
> + struct kwb_header *hdr = (struct kwb_header *)ptr;
> + bhr_t *mhdr = &hdr->kwb_hdr;
> + extbhr_t *exthdr = &hdr->kwb_exthdr;
> + uint8_t calc_hdrcsum;
> + uint8_t calc_exthdrcsum;
> +
> + calc_hdrcsum = kwbimage_checksum8 ((void *)mhdr,
> + sizeof(bhr_t) - sizeof(uint8_t), 0);
> + if (calc_hdrcsum != mhdr->checkSum) {
> + return -FDT_ERR_BADSTRUCTURE; /* mhdr csum not matched */
> + }
No braces needed for single line statements.
> + calc_exthdrcsum = kwbimage_checksum8 ((void *)exthdr,
> + sizeof(extbhr_t) - sizeof(uint8_t), 0);
> + if (calc_hdrcsum != mhdr->checkSum) {
> + return -FDT_ERR_BADSTRUCTURE; /* exthdr csum not matched */
> + }
No braces needed for single line statements.
> +void kwbimage_print_header (char *ptr)
> +{
> + struct kwb_header *hdr = (struct kwb_header *) ptr;
> + bhr_t *mhdr = &hdr->kwb_hdr;
> +
> + printf ("Image Type: Kirkwood Boot from %s Image\n",
> + get_table_entry_name (kwbimage_bootops,
> + "Kwbimage boot option",
> + (int) mhdr->blockid));
Indentation looks ugly.
...
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index 3a3cb19..7e29610 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -25,6 +25,7 @@
> #include "mkimage.h"
> #include <image.h>
> #include "default_image.h"
> +#include "kwbimage.h"
>
> static void copy_file(int, const char *, int);
> static void usage(void);
> @@ -59,6 +60,7 @@ static struct image_functions image_ftable[] = {
> {image_get_tparams,}, /* for IH_TYPE_SCRIPT */
> {image_get_tparams,}, /* for IH_TYPE_STANDALONE */
> {fitimage_get_tparams,}, /* for IH_TYPE_FLATDT */
> + {kwbimage_get_tparams,}, /* for IH_TYPE_KWBIMAGE */
> };
>
> int
> @@ -111,7 +113,7 @@ main (int argc, char **argv)
> (params.opt_type =
> genimg_get_type_id (*++argv)) < 0)
> usage ();
> - if (image_ftable_size <= params.opt_type) {
> + if (image_ftable_size < params.opt_type) {
Ah! What's going on here????
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
When choosing between two evils, I always like to take the one I've
never tried before. -- Mae West, "Klondike Annie"
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global
2009-08-07 23:02 ` [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global Wolfgang Denk
@ 2009-08-09 13:54 ` Prafulla Wadaskar
2009-08-09 20:29 ` Wolfgang Denk
0 siblings, 1 reply; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-08-09 13:54 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Saturday, August 08, 2009 4:32 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
> Subject: Re: [U-Boot] [PATCH 6/8] tools: mkimage: Making
> table_entry code global
>
> Dear Prafulla Wadaskar,
>
> In message
> <1248804270-13715-6-git-send-email-prafulla@marvell.com> you wrote:
> >
> > diff --git a/include/image.h b/include/image.h index
> 88a13ab..f119cee
> > 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -168,6 +168,15 @@
> > #define IH_MAGIC 0x27051956 /* Image Magic Number
> */
> > #define IH_NMLEN 32 /* Image Name Length
> */
> >
> > +typedef struct table_entry {
> > + int id; /* as defined in image.h */
> > + char *sname; /* short (input) name */
> > + char *lname; /* long (output) name */
> > +} table_entry_t;
>
> Now read this code again, with the distance of a couple of
> days, and tell me what you think.
>
> "as defined in image.h" - hey, this _is_ image.h !
>
> So, "struct table_entry" - what sort of table is this?
>
> "short (input) name", "long (output) name" - what the heck is
> this about?
>
> In the old version, the declaration of the struct was
> followed by the declarations of the actual tables, so we
> could _see_ what was meant.
>
> You rip the code out of context, which makes it unreadable
> and ununderstandable.
Dear Wolfgang,
I have gone through all the feedback that you have provided for this entire patch series.
Thanks a lot...
Now I need to take care of issues raised by you on the top of presenting them,
with comments wherever necessary to make it better readable and understandable.
I tried to maintain the flow in the series for easy understanding, but I failed :-(
With these patches, new code looks differently compared to original code.
So I feel instead of providing incremental patches- I should provide
1. basic mkimge framework patch,
2. mkimage: add: uimage support patch,
3. mkimage: add: fit image support patch,
4. mkimage: add: Kirkwood boot image support patch.
I will do this in my next post.
I hope you will like it.
If you have any inputs in this regard or you think this is bad plan, pls kindly let me know.
Regards..
Prafulla . .
>
> This is a really bad idea, it seems.
>
> 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 "He only drinks when he gets depressed." "Why does
> he get depressed?"
> "Sometimes it's because he hasn't had a drink."
> - Terry Pratchett, _Men at Arms_
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global
2009-08-09 13:54 ` Prafulla Wadaskar
@ 2009-08-09 20:29 ` Wolfgang Denk
2009-08-10 6:12 ` Prafulla Wadaskar
0 siblings, 1 reply; 17+ messages in thread
From: Wolfgang Denk @ 2009-08-09 20:29 UTC (permalink / raw)
To: u-boot
Dear Prafulla Wadaskar,
In message <73173D32E9439E4ABB5151606C3E19E202E121613D@SC-VEXCH1.marvell.com> you wrote:
>
> I have gone through all the feedback that you have provided for this entire patch series.
> Thanks a lot...
Thank you for your pat6ience. Sorry it's such a long story for such a
apparently small addition.
> I tried to maintain the flow in the series for easy understanding, but I failed :-(
Actually you did not. I just did not agree with some implementation
details. And the changes need beter documentation.
> With these patches, new code looks differently compared to original code.
> So I feel instead of providing incremental patches- I should provide
> 1. basic mkimge framework patch,
> 2. mkimage: add: uimage support patch,
> 3. mkimage: add: fit image support patch,
Um... these three must never be split. I mean, we should not add any
patch that breaks the (existing) mkimage support.
> 4. mkimage: add: Kirkwood boot image support patch.
> I will do this in my next post.
> I hope you will like it.
> If you have any inputs in this regard or you think this is bad plan, pls kindly let me know.
I'm not sure if this will be an improvement. I guess it will make it
even harder to understand the changes.
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
"Infidels in all ages have battled for the rights of man, and have at
all times been the fearless advocates of liberty and justice."
- Robert Green Ingersoll
^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global
2009-08-09 20:29 ` Wolfgang Denk
@ 2009-08-10 6:12 ` Prafulla Wadaskar
0 siblings, 0 replies; 17+ messages in thread
From: Prafulla Wadaskar @ 2009-08-10 6:12 UTC (permalink / raw)
To: u-boot
> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Monday, August 10, 2009 2:00 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
> Subject: Re: [U-Boot] [PATCH 6/8] tools: mkimage: Making
> table_entry code global
>
> Dear Prafulla Wadaskar,
>
> In message
> <73173D32E9439E4ABB5151606C3E19E202E121613D@SC-VEXCH1.marvell.
> com> you wrote:
> >
> > I have gone through all the feedback that you have provided
> for this entire patch series.
> > Thanks a lot...
>
> Thank you for your pat6ience. Sorry it's such a long story for such a
> apparently small addition.
That's what my earlier intention was:-) Just to add small support with minimal modifications.
But while doing it, the idea has generated why not improve it further? Like header_size, function callbacks.
That need more work/rework and I can understand there will be more spins to get it through.
So here I am to support you :-D
>
> > I tried to maintain the flow in the series for easy
> understanding, but I failed :-(
>
> Actually you did not. I just did not agree with some implementation
> details. And the changes need beter documentation.
Yes, I will take care of documentation, thanks
>
> > With these patches, new code looks differently compared to
> original code.
> > So I feel instead of providing incremental patches- I should provide
> > 1. basic mkimge framework patch,
> > 2. mkimage: add: uimage support patch,
> > 3. mkimage: add: fit image support patch,
>
> Um... these three must never be split. I mean, we should not add any
> patch that breaks the (existing) mkimage support.
Okay I will take care of this
>
> > 4. mkimage: add: Kirkwood boot image support patch.
>
> > I will do this in my next post.
> > I hope you will like it.
> > If you have any inputs in this regard or you think this is
> bad plan, pls kindly let me know.
>
> I'm not sure if this will be an improvement. I guess it will make it
> even harder to understand the changes.
Then I will try to post patches better combination of incremental ones :-)
Thanks and 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
> "Infidels in all ages have battled for the rights of man, and have at
> all times been the fearless advocates of liberty and justice."
> - Robert Green Ingersoll
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2009-08-10 6:12 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-28 18:04 [U-Boot] [PATCH 1/8] tools: mkimage : bugfix returns correct value for list command Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 5/8] tools: mkimage: Making genimg_print_size API global Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage) Prafulla Wadaskar
2009-07-28 18:04 ` [U-Boot] [PATCH v4 8/8] Kirkwood: Sheevaplug: kwimage configuration Prafulla Wadaskar
2009-08-07 23:24 ` [U-Boot] [PATCH 7/8] tools: mkimage: Add: Kirkwood Boot Image support (kwbimage) Wolfgang Denk
2009-08-07 23:02 ` [U-Boot] [PATCH 6/8] tools: mkimage: Making table_entry code global Wolfgang Denk
2009-08-09 13:54 ` Prafulla Wadaskar
2009-08-09 20:29 ` Wolfgang Denk
2009-08-10 6:12 ` Prafulla Wadaskar
2009-08-07 22:55 ` [U-Boot] [PATCH 4/8] tools: mkimage: Implemented callback function for default image support Wolfgang Denk
2009-08-07 22:43 ` [U-Boot] [PATCH 3/8] tools: mkimage: Move image header code to image specific files Wolfgang Denk
2009-08-07 22:30 ` [U-Boot] [PATCH 2/8] tools: mkimage: code abstraction generic and image specific Wolfgang Denk
2009-08-07 22:11 ` [U-Boot] [PATCH 1/8] tools: mkimage : bugfix returns correct value for list command Wolfgang Denk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.