Skip to contentSkip to conversation

new uImage patchset1 - cleanup

PATCH New Marian Balakowicz 8 participants 40 messages Jan 11, 2008 – Jan 15, 2008 Cramfs filesystem

lore
Share & tools
lore
#0 lore raw

cover new uImage patchset1 - cleanup

This is a first set of patches that provides a initial cleanup and code rearrangements before the proper new uImage format implementation.

API is defined for the current (old) image related processing, and it'is being introduced in place of the direct header access.

[new uImage] Return error on image move/uncompress overwrites [new uImage] Cleanup image header pointer use in bootm code [new uImage] Coding style cleanup - part 1
[new uImage] Add memmove_wd() common routine
[new uImage] Fix FDT header verification in PPC do_boot_linux() routine [new uImage] Fix uImage header pointer use in i386 do_bootm_linux() [new uImage] Remove I386 uImage fake_header() routine
[new uImage] Move CHUNKSZ definition to image.h
[new uImage] Move gunzip() common code to common/gunzip.c [new uImage] Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux() [new uImage] Move PPC do_bootm_linux() to lib_ppc/ppc_linux.c [new uImage] Define a API for image handling operations
Add missing cmd_ximg.o to common/Makefile

Comments and suggestions are welcome.

Due to a 40K message limit, not all of the patches can be sent to ml, oversized patches will provide a http location instead.

Cheers,
Marian

#1 lore raw

PATCH 1/13 Add missing cmd_ximg.o to common/Makefile

---

 common/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
common/Makefile +1 -0
index ace8cc7..7554eae 100644
@@ -88,6 +88,7 @@ COBJS-$(CONFIG_CMD_SCSI) += cmd_scsi.o
 COBJS-$(CONFIG_CMD_SPI) += cmd_spi.o
 COBJS-$(CONFIG_CMD_UNIVERSE) += cmd_universe.o
 COBJS-$(CONFIG_CMD_USB) += cmd_usb.o
+COBJS-$(CONFIG_CMD_XIMG) += cmd_ximg.o
 COBJS-y += cmd_vfd.o
 COBJS-y += command.o
 COBJS-y += console.o
#2 lore raw

PATCH 2/13 Define a API for image handling operations

In reply to Marian Balakowicz

- Add inline helper macros for basic header processing
- Move common non inline code common/image.c
- Replace direct header access with the API routines
- Rename IH_CPU_* to IH_ARCH_*

---

 Makefile                          |    3 
 board/cray/L1/L1.c                |    4 
 board/esd/common/auto_update.c    |   73 ++++----
 board/mcc200/auto_update.c        |   71 ++++----
 board/mpl/common/common_util.c    |   37 ++--
 board/siemens/common/fpga.c       |   32 +---
 board/trab/auto_update.c          |   82 ++++-----
 common/Makefile                   |    1 
 common/cmd_autoscript.c           |   30 +--
 common/cmd_bootm.c                |  331 +++++++++++++------------------------
 common/cmd_doc.c                  |    4 
 common/cmd_fdc.c                  |    6 -
 common/cmd_fpga.c                 |    8 -
 common/cmd_ide.c                  |   12 -
 common/cmd_nand.c                 |   12 +
 common/cmd_scsi.c                 |   12 -
 common/cmd_usb.c                  |   12 -
 common/cmd_ximg.c                 |   36 +---
 common/image.c                    |   91 ++++++++++
 common/lynxkdi.c                  |    8 -
 include/image.h                   |  174 +++++++++++++++++--
 lib_arm/armlinux.c                |   45 ++---
 lib_avr32/avr32_linux.c           |   47 ++---
 lib_blackfin/bf533_linux.c        |    3 
 lib_i386/i386_linux.c             |   44 ++---
 lib_i386/zimage.c                 |   33 ++--
 lib_m68k/m68k_linux.c             |   62 ++-----
 lib_microblaze/microblaze_linux.c |   39 ++--
 lib_mips/mips_linux.c             |   41 ++---
 lib_nios2/nios_linux.c            |    2 
 tools/Makefile                    |   11 +
 tools/mkimage.c                   |  180 ++++++++------------
 tools/mkimage.h                   |   59 +++++++
 33 files changed, 809 insertions(+), 796 deletions(-)
 create mode 100644 common/image.c
 create mode 100644 tools/mkimage.h

This patch size is over 40K list limit, please refer to:
http://semihalf.com/~m8/new-image/patchset1/02-old-api.patch

#3 lore raw

PATCH 2/13 Define a API for image handling operations

Dear Marian,

Quoted 6 lines · in message <[email protected]> you wrote:

- Add inline helper macros for basic header processing
- Move common non inline code common/image.c
- Replace direct header access with the API routines
- Rename IH_CPU_* to IH_ARCH_*

I tried to apply your patches to the u-boot-testing repository; unfortunately, this does now work:

-> git-am -3 -i -u --whitespace=strip ~/Mail/U-Boot/13330 Commit Body is:
--------------------------
Define a API for image handling operations

- Add inline helper macros for basic header processing
- Move common non inline code common/image.c
- Replace direct header access with the API routines
- Rename IH_CPU_* to IH_ARCH_*

--------------------------
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y

Applying Define a API for image handling operations

error: No changes
Using index info to reconstruct a base tree...
error: No changes
Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Cannot fall back to three-way merge.
Patch failed at 0001.
When you have resolved this problem run "git-am -i -3 --resolved". If you would prefer to skip this patch, instead run "git-am -i -3 --skip".

Can you please check and - if necessary - rebase the patch set?

Thanks.

Best regards,

Wolfgang Denk

Signature · 5 lines
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
People are always a lot more complicated than you  think.  It's  very
important to remember that.             - Terry Pratchett, _Truckers_
#4 lore raw

PATCH 2/13 Define a API for image handling operations

Quoted 38 lines · Wolfgang Denk
Quoted 6 lines · in message <[email protected]> you wrote:

- Add inline helper macros for basic header processing
- Move common non inline code common/image.c
- Replace direct header access with the API routines
- Rename IH_CPU_* to IH_ARCH_*

I tried to apply your patches to the u-boot-testing repository; unfortunately, this does now work:

-> git-am -3 -i -u --whitespace=strip ~/Mail/U-Boot/13330 Commit Body is:
--------------------------
Define a API for image handling operations

- Add inline helper macros for basic header processing
- Move common non inline code common/image.c
- Replace direct header access with the API routines
- Rename IH_CPU_* to IH_ARCH_*

--------------------------
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y

Applying Define a API for image handling operations

error: No changes
Using index info to reconstruct a base tree...
error: No changes
Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Cannot fall back to three-way merge.
Patch failed at 0001.
When you have resolved this problem run "git-am -i -3 --resolved". If you would prefer to skip this patch, instead run "git-am -i -3 --skip".

Quoted 1 line

Can you please check and - if necessary - rebase the patch set?

The baseline was "Release v1.3.1" commit, I'll rebase to current HEAD as there are some conflicts with the latest commits.

But from the above it seems that you were attempting to apply patch 02/13 for which patch body was not posted to the ml?

Cheers,
m.

#5 lore raw

PATCH 2/13 Define a API for image handling operations

Quoted 3 lines · In message <[email protected]> you wrote:

The baseline was "Release v1.3.1" commit, I'll rebase to current HEAD as there are some conflicts with the latest commits.

thanks.

Quoted 2 lines

But from the above it seems that you were attempting to apply patch 02/13 for which patch body was not posted to the ml?

Yes, but you gave us an URL where I downloaded the missing pieces from...

[A repository on pollux would be easier for me, of course.]

Best regards,

Wolfgang Denk

Signature · 4 lines
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
What's the sound a name makes when it's dropped?
#6 lore raw

PATCH 2/13 Define a API for image handling operations

In reply to Marian Balakowicz
Quoted 3 lines · In message <[email protected]> you wrote:

This patch size is over 40K list limit, please refer to:
http://semihalf.com/~m8/new-image/patchset1/02-old-api.patch

Please *post* patches; I will let reasonable patches through; the hard limit is 100 KiB.

Best regards,

Wolfgang Denk

Signature · 4 lines
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
When all is said and done, more is said than done.
#8 lore raw

PATCH 2/13 Define a API for image handling operations

In reply to Marian Balakowicz
Quoted 4 lines · In message <[email protected]> you wrote:

- Add inline helper macros for basic header processing
- Move common non inline code common/image.c
- Replace direct header access with the API routines
- Rename IH_CPU_* to IH_ARCH_*

Please review your patches and make sure to keep the original for- matting of the U-Boot output; for example:

--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
...
- if (crc32 (0, (uchar *)data, len) != ntohl(hdr->ih_dcrc)) {
- puts (" Bad Data CRC\n");
- }
- puts ("OK\n");
+ if (!image_check_dcrc(hdr))
+ puts ("Bad Data CRC\n");

This is IMO a change to the worse. Please fix.

--- a/include/image.h
+++ b/include/image.h
...
+char *strncpy(char *dest, const char *src, size_t count);

Please don't do this! Include the appropriate header file instead.

+static inline int image_check_target_arch(image_header_t *hdr)

Theoretical thought: could it ever happen that we might have a multi-architecture image?

Best regards,

Wolfgang Denk

Signature · 5 lines
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
Q:  What is "SMOORPLAY"?
A:  It's what SMURFS do before they SMUCK, of course!
#9 lore raw

PATCH 2/13 Define a API for image handling operations

Quoted 8 lines · Wolfgang Denk
Quoted 4 lines · In message <[email protected]> you wrote:

- Add inline helper macros for basic header processing
- Move common non inline code common/image.c
- Replace direct header access with the API routines
- Rename IH_CPU_* to IH_ARCH_*

Please review your patches and make sure to keep the original for- matting of the U-Boot output; for example:

[snip]

Quoted 6 lines
include/image.h +1 -0
...
+char *strncpy(char *dest, const char *src, size_t count);

Please don't do this! Include the appropriate header file instead.

Will fix.

Quoted 4 lines

+static inline int image_check_target_arch(image_header_t *hdr)

Theoretical thought: could it ever happen that we might have a multi-architecture image?

With the new uImage that would be possible, each component image will have it's own arch/os properties.

The above routine (handling current image format which does not allow multi arch images) checks whether image arch matches target arch.

It is a reasonable check, running other arch images usually make little sens. That would also apply for new uImage format, we may have multiple component images (each for different arch) but before running selected image we check for arch compatibility.

Cheers,
m.

#10 lore raw

PATCH 3/13 Move PPC do_bootm_linux() to lib_ppc/ppc_linux.c

In reply to Marian Balakowicz

PPC implementation of do_bootm_linux() routine is moved to a dedicated file lib_ppc/ppc_linux.c

---

 common/cmd_bootm.c  |  543 -----------------------------------------------
 lib_ppc/Makefile    |    3 
 lib_ppc/ppc_linux.c |  589 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 592 insertions(+), 543 deletions(-)
 create mode 100644 lib_ppc/ppc_linux.c
common/cmd_bootm.c +1 -542
index ba07e4e..a4952fd 100644
@@ -58,14 +58,6 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
 #include <hush.h>
 #endif
 
-#ifdef CFG_INIT_RAM_LOCK
-#include <asm/cache.h>
-#endif
-
-#ifdef CONFIG_LOGBUFFER
-#include <logbuff.h>
-#endif
-
 #ifdef CONFIG_HAS_DATAFLASH
 #include <dataflash.h>
 #endif
@@ -112,15 +104,8 @@ typedef void boot_os_Fcn (cmd_tbl_t *cmdtp, int flag,
 			  ulong	*len_ptr,	/* multi-file image length table */
 			  int	verify);	/* getenv("verify")[0] != 'n' */
 
-#ifdef	DEBUG
-extern int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
-#endif
-
-#ifdef CONFIG_PPC
-static boot_os_Fcn do_bootm_linux;
-#else
 extern boot_os_Fcn do_bootm_linux;
-#endif
+
 #ifdef CONFIG_SILENT_CONSOLE
 static void fixup_silent_linux (void);
 #endif
@@ -475,532 +460,6 @@ fixup_silent_linux ()
 }
 #endif /* CONFIG_SILENT_CONSOLE */
 
-#ifdef CONFIG_PPC
-static void  __attribute__((noinline))
-do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
-		int	argc, char *argv[],
-		ulong	addr,
-		ulong	*len_ptr,
-		int	verify)
-{
-	ulong	sp;
-	ulong	len;
-	ulong	initrd_start, initrd_end;
-	ulong	cmd_start, cmd_end;
-	ulong	initrd_high;
-	ulong	data;
-	int	initrd_copy_to_ram = 1;
-	char    *cmdline;
-	char	*s;
-	bd_t	*kbd;
-	void	(*kernel)(bd_t *, ulong, ulong, ulong, ulong);
-	image_header_t *hdr = &header;
-#if defined(CONFIG_OF_FLAT_TREE) || defined(CONFIG_OF_LIBFDT)
-	char	*of_flat_tree = NULL;
-	ulong	of_data = 0;
-#endif
-
-	if ((s = getenv ("initrd_high")) != NULL) {
-		/* a value of "no" or a similar string will act like 0,
-		 * turning the "load high" feature off. This is intentional.
-		 */
-		initrd_high = simple_strtoul(s, NULL, 16);
-		if (initrd_high == ~0)
-			initrd_copy_to_ram = 0;
-	} else {	/* not set, no restrictions to load high */
-		initrd_high = ~0;
-	}
-
-#ifdef CONFIG_LOGBUFFER
-	kbd=gd->bd;
-	/* Prevent initrd from overwriting logbuffer */
-	if (initrd_high < (kbd->bi_memsize-LOGBUFF_LEN-LOGBUFF_OVERHEAD))
-		initrd_high = kbd->bi_memsize-LOGBUFF_LEN-LOGBUFF_OVERHEAD;
-	debug ("## Logbuffer at 0x%08lX ", kbd->bi_memsize-LOGBUFF_LEN);
-#endif
-
-	/*
-	 * Booting a (Linux) kernel image
-	 *
-	 * Allocate space for command line and board info - the
-	 * address should be as high as possible within the reach of
-	 * the kernel (see CFG_BOOTMAPSZ settings), but in unused
-	 * memory, which means far enough below the current stack
-	 * pointer.
-	 */
-
-	asm( "mr %0,1": "=r"(sp) : );
-
-	debug ("## Current stack ends at 0x%08lX ", sp);
-
-	sp -= 2048;		/* just to be sure */
-	if (sp > CFG_BOOTMAPSZ)
-		sp = CFG_BOOTMAPSZ;
-	sp &= ~0xF;
-
-	debug ("=> set upper limit to 0x%08lX\n", sp);
-
-	cmdline = (char *)((sp - CFG_BARGSIZE) & ~0xF);
-	kbd = (bd_t *)(((ulong)cmdline - sizeof(bd_t)) & ~0xF);
-
-	if ((s = getenv("bootargs")) == NULL)
-		s = "";
-
-	strcpy (cmdline, s);
-
-	cmd_start    = (ulong)&cmdline[0];
-	cmd_end      = cmd_start + strlen(cmdline);
-
-	*kbd = *(gd->bd);
-
-#ifdef	DEBUG
-	printf ("## cmdline at 0x%08lX ... 0x%08lX\n", cmd_start, cmd_end);
-
-	do_bdinfo (NULL, 0, 0, NULL);
-#endif
-
-	if ((s = getenv ("clocks_in_mhz")) != NULL) {
-		/* convert all clock information to MHz */
-		kbd->bi_intfreq /= 1000000L;
-		kbd->bi_busfreq /= 1000000L;
-#if defined(CONFIG_MPC8220)
-	kbd->bi_inpfreq /= 1000000L;
-	kbd->bi_pcifreq /= 1000000L;
-	kbd->bi_pevfreq /= 1000000L;
-	kbd->bi_flbfreq /= 1000000L;
-	kbd->bi_vcofreq /= 1000000L;
-#endif
-#if defined(CONFIG_CPM2)
-		kbd->bi_cpmfreq /= 1000000L;
-		kbd->bi_brgfreq /= 1000000L;
-		kbd->bi_sccfreq /= 1000000L;
-		kbd->bi_vco     /= 1000000L;
-#endif
-#if defined(CONFIG_MPC5xxx)
-		kbd->bi_ipbfreq /= 1000000L;
-		kbd->bi_pcifreq /= 1000000L;
-#endif /* CONFIG_MPC5xxx */
-	}
-
-	kernel = (void (*)(bd_t *, ulong, ulong, ulong, ulong))image_get_ep(hdr);
-
-	/*
-	 * Check if there is an initrd image
-	 */
-
-#if defined(CONFIG_OF_FLAT_TREE) || defined(CONFIG_OF_LIBFDT)
-	/* Look for a '-' which indicates to ignore the ramdisk argument */
-	if (argc >= 3 && strcmp(argv[2], "-") ==  0) {
-			debug ("Skipping initrd\n");
-			len = data = 0;
-		}
-	else
-#endif
-	if (argc >= 3) {
-		debug ("Not skipping initrd\n");
-		show_boot_progress (9);
-
-		addr = simple_strtoul(argv[2], NULL, 16);
-
-		printf ("## Loading RAMDisk Image at %08lx ...\n", addr);
-		hdr = (image_header_t *)addr;
-
-		if (!image_check_magic(hdr)) {
-			puts ("Bad Magic Number\n");
-			show_boot_progress (-10);
-			do_reset (cmdtp, flag, argc, argv);
-		}
-
-		if (!image_check_hcrc(hdr)) {
-			puts ("Bad Header Checksum\n");
-			show_boot_progress (-11);
-			do_reset (cmdtp, flag, argc, argv);
-		}
-		show_boot_progress (10);
-
-		print_image_hdr (hdr);
-
-		if (verify) {
-			puts ("   Verifying Checksum ... ");
-
-			if (!image_check_dcrc_wd(hdr, CHUNKSZ)) {
-				puts ("Bad Data CRC\n");
-				show_boot_progress (-12);
-				do_reset (cmdtp, flag, argc, argv);
-			}
-			puts ("OK\n");
-		}
-
-		show_boot_progress (11);
-
-		if (!image_check_os(hdr, IH_OS_LINUX) ||
-		    !image_check_arch(hdr, IH_ARCH_PPC) ||
-		    !image_check_type(hdr, IH_TYPE_RAMDISK)) {
-			puts ("No Linux PPC Ramdisk Image\n");
-			show_boot_progress (-13);
-			do_reset (cmdtp, flag, argc, argv);
-		}
-
-		data = image_get_data(hdr);
-		len = image_get_data_size(hdr);
-
-		/*
-		 * Now check if we have a multifile image
-		 */
-	} else if (image_check_type(hdr, IH_TYPE_MULTI) && (len_ptr[1])) {
-		u_long tail    = image_to_cpu(len_ptr[0]) % 4;
-		int i;
-
-		show_boot_progress (13);
-
-		/* skip kernel length and terminator */
-		data = (ulong)(&len_ptr[2]);
-		/* skip any additional image length fields */
-		for (i=1; len_ptr[i]; ++i)
-			data += 4;
-		/* add kernel length, and align */
-		data += image_to_cpu(len_ptr[0]);
-		if (tail) {
-			data += 4 - tail;
-		}
-
-		len   = image_to_cpu(len_ptr[1]);
-
-	} else {
-		/*
-		 * no initrd image
-		 */
-		show_boot_progress (14);
-
-		len = data = 0;
-	}
-
-#if defined(CONFIG_OF_FLAT_TREE) || defined(CONFIG_OF_LIBFDT)
-	if(argc > 3) {
-		of_flat_tree = (char *) simple_strtoul(argv[3], NULL, 16);
-		hdr = (image_header_t *)of_flat_tree;
-#if defined(CONFIG_OF_FLAT_TREE)
-		if (*((ulong *)(of_flat_tree + image_get_header_size())) != OF_DT_HEADER) {
-#else
-		if (fdt_check_header(of_flat_tree + image_get_header_size()) != 0) {
-#endif
-#ifndef CFG_NO_FLASH
-			if (addr2info((ulong)of_flat_tree) != NULL)
-				of_data = (ulong)of_flat_tree;
-#endif
-		} else if (image_check_magic(hdr)) {
-			printf("## Flat Device Tree at %08lX\n", hdr);
-			print_image_hdr(hdr);
-
-			if ((image_get_load(hdr) <  ((unsigned long)hdr + image_get_image_size(hdr))) &&
-			   ((image_get_load(hdr) + image_get_data_size(hdr)) > (unsigned long)hdr)) {
-				puts ("ERROR: fdt overwritten - "
-					"must RESET the board to recover.\n");
-				do_reset (cmdtp, flag, argc, argv);
-			}
-
-			puts ("   Verifying Checksum ... ");
-			if (!image_check_hcrc(hdr)) {
-				puts ("ERROR: fdt header checksum invalid - "
-					"must RESET the board to recover.\n");
-				do_reset (cmdtp, flag, argc, argv);
-			}
-
-			if (!image_check_dcrc(hdr)) {
-				puts ("ERROR: fdt checksum invalid - "
-					"must RESET the board to recover.\n");
-				do_reset (cmdtp, flag, argc, argv);
-			}
-			puts ("OK\n");
-
-			if (!image_check_type(hdr, IH_TYPE_FLATDT)) {
-				puts ("ERROR: uImage is not a fdt - "
-					"must RESET the board to recover.\n");
-				do_reset (cmdtp, flag, argc, argv);
-			}
-			if (image_get_comp(hdr) != IH_COMP_NONE) {
-				puts ("ERROR: uImage is compressed - "
-					"must RESET the board to recover.\n");
-				do_reset (cmdtp, flag, argc, argv);
-			}
-#if defined(CONFIG_OF_FLAT_TREE)
-			if (*((ulong *)(of_flat_tree + image_get_header_size())) != OF_DT_HEADER) {
-#else
-			if (fdt_check_header(of_flat_tree + image_get_header_size()) != 0) {
-#endif
-				puts ("ERROR: uImage data is not a fdt - "
-					"must RESET the board to recover.\n");
-				do_reset (cmdtp, flag, argc, argv);
-			}
-
-			memmove((void *)image_get_load(hdr),
-				(void *)(of_flat_tree + image_get_header_size()),
-				image_get_data_size(hdr));
-
-			of_flat_tree = (char *)image_get_load(hdr);
-		} else {
-			puts ("Did not find a flat Flat Device Tree.\n"
-				"Must RESET the board to recover.\n");
-			do_reset (cmdtp, flag, argc, argv);
-		}
-		printf ("   Booting using the fdt at 0x%x\n",
-				of_flat_tree);
-	} else if (image_check_type(hdr, IH_TYPE_MULTI) && (len_ptr[1]) && (len_ptr[2])) {
-		u_long tail    = image_to_cpu(len_ptr[0]) % 4;
-		int i;
-
-		/* skip kernel length, initrd length, and terminator */
-		of_flat_tree = (char *)(&len_ptr[3]);
-		/* skip any additional image length fields */
-		for (i=2; len_ptr[i]; ++i)
-			of_flat_tree += 4;
-		/* add kernel length, and align */
-		of_flat_tree += image_to_cpu(len_ptr[0]);
-		if (tail) {
-			of_flat_tree += 4 - tail;
-		}
-
-		/* add initrd length, and align */
-		tail = image_to_cpu(len_ptr[1]) % 4;
-		of_flat_tree += image_to_cpu(len_ptr[1]);
-		if (tail) {
-			of_flat_tree += 4 - tail;
-		}
-
-#ifndef CFG_NO_FLASH
-		/* move the blob if it is in flash (set of_data to !null) */
-		if (addr2info ((ulong)of_flat_tree) != NULL)
-			of_data = (ulong)of_flat_tree;
-#endif
-
-
-#if defined(CONFIG_OF_FLAT_TREE)
-		if (*((ulong *)(of_flat_tree)) != OF_DT_HEADER) {
-#else
-		if (fdt_check_header (of_flat_tree) != 0) {
-#endif
-			puts ("ERROR: image is not a fdt - "
-				"must RESET the board to recover.\n");
-			do_reset (cmdtp, flag, argc, argv);
-		}
-
-#if defined(CONFIG_OF_FLAT_TREE)
-		if (((struct boot_param_header *)of_flat_tree)->totalsize !=
-			image_to_cpu (len_ptr[2])) {
-#else
-		if (be32_to_cpu (fdt_totalsize (of_flat_tree)) !=
-			image_to_cpu(len_ptr[2])) {
-#endif
-			puts ("ERROR: fdt size != image size - "
-				"must RESET the board to recover.\n");
-			do_reset (cmdtp, flag, argc, argv);
-		}
-	}
-#endif
-	if (!data) {
-		debug ("No initrd\n");
-	}
-
-	if (data) {
-	    if (!initrd_copy_to_ram) {	/* zero-copy ramdisk support */
-		initrd_start = data;
-		initrd_end = initrd_start + len;
-	    } else {
-		initrd_start  = (ulong)kbd - len;
-		initrd_start &= ~(4096 - 1);	/* align on page */
-
-		if (initrd_high) {
-			ulong nsp;
-
-			/*
-			 * the inital ramdisk does not need to be within
-			 * CFG_BOOTMAPSZ as it is not accessed until after
-			 * the mm system is initialised.
-			 *
-			 * do the stack bottom calculation again and see if
-			 * the initrd will fit just below the monitor stack
-			 * bottom without overwriting the area allocated
-			 * above for command line args and board info.
-			 */
-			asm( "mr %0,1": "=r"(nsp) : );
-			nsp -= 2048;		/* just to be sure */
-			nsp &= ~0xF;
-			if (nsp > initrd_high)	/* limit as specified */
-				nsp = initrd_high;
-			nsp -= len;
-			nsp &= ~(4096 - 1);	/* align on page */
-			if (nsp >= sp)
-				initrd_start = nsp;
-		}
-
-		show_boot_progress (12);
-
-		debug ("## initrd at 0x%08lX ... 0x%08lX (len=%ld=0x%lX)\n",
-			data, data + len - 1, len, len);
-
-		initrd_end    = initrd_start + len;
-		printf ("   Loading Ramdisk to %08lx, end %08lx ... ",
-			initrd_start, initrd_end);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
-		{
-			size_t l = len;
-			void *to = (void *)initrd_start;
-			void *from = (void *)data;
-
-			while (l > 0) {
-				size_t tail = (l > CHUNKSZ) ? CHUNKSZ : l;
-				WATCHDOG_RESET();
-				memmove (to, from, tail);
-				to += tail;
-				from += tail;
-				l -= tail;
-			}
-		}
-#else	/* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
-		memmove ((void *)initrd_start, (void *)data, len);
-#endif	/* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
-		puts ("OK\n");
-	    }
-	} else {
-		initrd_start = 0;
-		initrd_end = 0;
-	}
-
-#if defined(CONFIG_OF_LIBFDT)
-
-#ifdef CFG_BOOTMAPSZ
-	/*
-	 * The blob must be within CFG_BOOTMAPSZ,
-	 * so we flag it to be copied if it is not.
-	 */
-	if (of_flat_tree >= (char *)CFG_BOOTMAPSZ)
-		of_data = (ulong)of_flat_tree;
-#endif
-
-	/* move of_flat_tree if needed */
-	if (of_data) {
-		int err;
-		ulong of_start, of_len;
-
-		of_len = be32_to_cpu(fdt_totalsize(of_data));
-
-		/* position on a 4K boundary before the kbd */
-		of_start  = (ulong)kbd - of_len;
-		of_start &= ~(4096 - 1);	/* align on page */
-		debug ("## device tree at 0x%08lX ... 0x%08lX (len=%ld=0x%lX)\n",
-			of_data, of_data + of_len - 1, of_len, of_len);
-
-		of_flat_tree = (char *)of_start;
-		printf ("   Loading Device Tree to %08lx, end %08lx ... ",
-			of_start, of_start + of_len - 1);
-		err = fdt_open_into((void *)of_data, (void *)of_start, of_len);
-		if (err != 0) {
-			puts ("ERROR: fdt move failed - "
-				"must RESET the board to recover.\n");
-			do_reset (cmdtp, flag, argc, argv);
-		}
-		puts ("OK\n");
-	}
-	/*
-	 * Add the chosen node if it doesn't exist, add the env and bd_t
-	 * if the user wants it (the logic is in the subroutines).
-	 */
-	if (of_flat_tree) {
-		if (fdt_chosen(of_flat_tree, initrd_start, initrd_end, 0) < 0) {
-			puts ("ERROR: /chosen node create failed - "
-				"must RESET the board to recover.\n");
-			do_reset (cmdtp, flag, argc, argv);
-		}
-#ifdef CONFIG_OF_HAS_UBOOT_ENV
-		if (fdt_env(of_flat_tree) < 0) {
-			puts ("ERROR: /u-boot-env node create failed - "
-				"must RESET the board to recover.\n");
-			do_reset (cmdtp, flag, argc, argv);
-		}
-#endif
-#ifdef CONFIG_OF_HAS_BD_T
-		if (fdt_bd_t(of_flat_tree) < 0) {
-			puts ("ERROR: /bd_t node create failed - "
-				"must RESET the board to recover.\n");
-			do_reset (cmdtp, flag, argc, argv);
-		}
-#endif
-#ifdef CONFIG_OF_BOARD_SETUP
-		/* Call the board-specific fixup routine */
-		ft_board_setup(of_flat_tree, gd->bd);
-#endif
-	}
-#endif /* CONFIG_OF_LIBFDT */
-#if defined(CONFIG_OF_FLAT_TREE)
-#ifdef CFG_BOOTMAPSZ
-	/*
-	 * The blob must be within CFG_BOOTMAPSZ,
-	 * so we flag it to be copied if it is not.
-	 */
-	if (of_flat_tree >= (char *)CFG_BOOTMAPSZ)
-		of_data = (ulong)of_flat_tree;
-#endif
-
-	/* move of_flat_tree if needed */
-	if (of_data) {
-		ulong of_start, of_len;
-		of_len = ((struct boot_param_header *)of_data)->totalsize;
-
-		/* provide extra 8k pad */
-		of_start  = (ulong)kbd - of_len - 8192;
-		of_start &= ~(4096 - 1);	/* align on page */
-		debug ("## device tree at 0x%08lX ... 0x%08lX (len=%ld=0x%lX)\n",
-			of_data, of_data + of_len - 1, of_len, of_len);
-
-		of_flat_tree = (char *)of_start;
-		printf ("   Loading Device Tree to %08lx, end %08lx ... ",
-			of_start, of_start + of_len - 1);
-		memmove ((void *)of_start, (void *)of_data, of_len);
-		puts ("OK\n");
-	}
-	/*
-	 * Create the /chosen node and modify the blob with board specific
-	 * values as needed.
-	 */
-	ft_setup(of_flat_tree, kbd, initrd_start, initrd_end);
-	/* ft_dump_blob(of_flat_tree); */
-#endif
-	debug ("## Transferring control to Linux (at address %08lx) ...\n",
-		(ulong)kernel);
-
-	show_boot_progress (15);
-
-#if defined(CFG_INIT_RAM_LOCK) && !defined(CONFIG_E500)
-	unlock_ram_in_cache();
-#endif
-
-#if defined(CONFIG_OF_FLAT_TREE) || defined(CONFIG_OF_LIBFDT)
-	if (of_flat_tree) {	/* device tree; boot new style */
-		/*
-		 * Linux Kernel Parameters (passing device tree):
-		 *   r3: pointer to the fdt, followed by the board info data
-		 *   r4: physical pointer to the kernel itself
-		 *   r5: NULL
-		 *   r6: NULL
-		 *   r7: NULL
-		 */
-		(*kernel) ((bd_t *)of_flat_tree, (ulong)kernel, 0, 0, 0);
-		/* does not return */
-	}
-#endif
-	/*
-	 * Linux Kernel Parameters (passing board info data):
-	 *   r3: ptr to board info data
-	 *   r4: initrd_start or 0 if no initrd
-	 *   r5: initrd_end - unused if r4 is 0
-	 *   r6: Start of command line string
-	 *   r7: End   of command line string
-	 */
-	(*kernel) (kbd, initrd_start, initrd_end, cmd_start, cmd_end);
-	/* does not return */
-}
-#endif /* CONFIG_PPC */
-
 static void
 do_bootm_netbsd (cmd_tbl_t *cmdtp, int flag,
 		int	argc, char *argv[],
lib_ppc/Makefile +2 -1
index 2ba034f..2aa0154 100644
@@ -28,7 +28,8 @@ LIB	= $(obj)lib$(ARCH).a
 SOBJS	= ppcstring.o ticks.o
 
 COBJS	= board.o \
-	  bat_rw.o cache.o extable.o kgdb.o time.o interrupts.o
+	  bat_rw.o cache.o extable.o kgdb.o time.o interrupts.o \
+	  ppc_linux.o
 
 SRCS 	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(SOBJS) $(COBJS))
lib_ppc/ppc_linux.c new file +589 -0

589 lines not on this page · raw message

#11 lore raw

PATCH 3/13 Move PPC do_bootm_linux() to lib_ppc/ppc_linux.c

Quoted 2 lines · In message <[email protected]> you wrote:

PPC implementation of do_bootm_linux() routine is moved to a dedicated file lib_ppc/ppc_linux.c

This is a bad name. "bootm" is more than just booting Linux images.

The ARM port did this wrong, and many others copied the error. Now let's take the opportunity and clean this up instead of making it worse.

Please neame this lib_ppc/bootm.c or similar (ditto for the other architectures).

Best regards,

Wolfgang Denk

Signature · 5 lines
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
Documentation is like sex: when it is good, it is  very,  very  good;
and when it is bad, it is better than nothing.         - Dick Brandon
#12 lore raw

PATCH 3/13 Move PPC do_bootm_linux() to lib_ppc/ppc_linux.c

Quoted 4 lines · Wolfgang Denk
Quoted 2 lines · In message <[email protected]> you wrote:

PPC implementation of do_bootm_linux() routine is moved to a dedicated file lib_ppc/ppc_linux.c

Quoted 8 lines

This is a bad name. "bootm" is more than just booting Linux images.

The ARM port did this wrong, and many others copied the error. Now let's take the opportunity and clean this up instead of making it worse.

Please neame this lib_ppc/bootm.c or similar (ditto for the other architectures).

I see, naming seemed arbitrary, but I did not know the full story.

I'll rename all related files.

Cheers,
m.

#13 lore raw

PATCH 4/13 Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux()

In reply to Marian Balakowicz

Make CONFIG_OF_LIBFDT and CONFIG_OF_FLAT_TREE use more readable in PPC variant of do_bootm_linux() routine.

---

 common/cmd_bootm.c  |    3 +--
 lib_ppc/ppc_linux.c |   20 +++++++++++---------
 2 files changed, 12 insertions(+), 11 deletions(-)
common/cmd_bootm.c +1 -2
index a4952fd..310173a 100644
@@ -40,8 +40,7 @@
 #include <fdt.h>
 #include <libfdt.h>
 #include <fdt_support.h>
-#endif
-#if defined(CONFIG_OF_FLAT_TREE)
+#elif defined(CONFIG_OF_FLAT_TREE)
 #include <ft_build.h>
 #endif
 
lib_ppc/ppc_linux.c +11 -9
index 823758c..3a4b74f 100644
@@ -37,8 +37,7 @@
 #include <fdt.h>
 #include <libfdt.h>
 #include <fdt_support.h>
-#endif
-#if defined(CONFIG_OF_FLAT_TREE)
+#elif defined(CONFIG_OF_FLAT_TREE)
 #include <ft_build.h>
 #endif
 
@@ -268,7 +267,7 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 		hdr = (image_header_t *)of_flat_tree;
 #if defined(CONFIG_OF_FLAT_TREE)
 		if (*((ulong *)(of_flat_tree + image_get_header_size())) != OF_DT_HEADER) {
-#else
+#elif defined(CONFIG_OF_LIBFDT)
 		if (fdt_check_header(of_flat_tree + image_get_header_size()) != 0) {
 #endif
 #ifndef CFG_NO_FLASH
@@ -312,7 +311,7 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 			}
 #if defined(CONFIG_OF_FLAT_TREE)
 			if (*((ulong *)(of_flat_tree + image_get_header_size())) != OF_DT_HEADER) {
-#else
+#elif defined(CONFIG_OF_LIBFDT)
 			if (fdt_check_header(of_flat_tree + image_get_header_size()) != 0) {
 #endif
 				puts ("ERROR: uImage data is not a fdt - "
@@ -363,7 +362,7 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 
 #if defined(CONFIG_OF_FLAT_TREE)
 		if (*((ulong *)(of_flat_tree)) != OF_DT_HEADER) {
-#else
+#elif defined(CONFIG_OF_LIBFDT)
 		if (fdt_check_header (of_flat_tree) != 0) {
 #endif
 			puts ("ERROR: image is not a fdt - "
@@ -374,7 +373,7 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 #if defined(CONFIG_OF_FLAT_TREE)
 		if (((struct boot_param_header *)of_flat_tree)->totalsize !=
 			image_to_cpu (len_ptr[2])) {
-#else
+#elif defined(CONFIG_OF_LIBFDT)
 		if (be32_to_cpu (fdt_totalsize (of_flat_tree)) !=
 			image_to_cpu(len_ptr[2])) {
 #endif
@@ -517,8 +516,9 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 		ft_board_setup(of_flat_tree, gd->bd);
 #endif
 	}
-#endif /* CONFIG_OF_LIBFDT */
-#if defined(CONFIG_OF_FLAT_TREE)
+
+#elif defined(CONFIG_OF_FLAT_TREE)
+
 #ifdef CFG_BOOTMAPSZ
 	/*
 	 * The blob must be within CFG_BOOTMAPSZ,
@@ -551,7 +551,9 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 	 */
 	ft_setup(of_flat_tree, kbd, initrd_start, initrd_end);
 	/* ft_dump_blob(of_flat_tree); */
-#endif
+
+#endif	/* #if defined(CONFIG_OF_LIBFDT) #elif defined(CONFIG_OF_FLAT_TREE) */
+
 	debug ("## Transferring control to Linux (at address %08lx) ...\n",
 		(ulong)kernel);
 
#14 lore raw

PATCH 4/13 Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux()

Quoted 4 lines · Marian Balakowicz

Make CONFIG_OF_LIBFDT and CONFIG_OF_FLAT_TREE use more readable in PPC variant of do_bootm_linux() routine.

What is the wisdom of the crowd, is it possible to port over the remaining 5 boards and removing the "CONFIG_OF_FLAT_TREE" code in this merge window, or is it too risky? My gut says too risky, do it in the next window.

$ find . -name "*.[h]" | xargs grep CONFIG_OF_FLAT_TREE | grep 1
./include/configs/mpc7448hpc2.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/sbc8349.h:#define CONFIG_OF_FLAT_TREE 1
./include/configs/MPC8610HPCD.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/stxxtc.h:#define CONFIG_OF_FLAT_TREE  1
./include/configs/sbc8641d.h:#define CONFIG_OF_FLAT_TREE        1
$ find . -name "*.[h]" | xargs grep CONFIG_OF_LIBFDT | grep 1 | wc -l
25

Thanks,
gvb

#15 lore raw

PATCH 4/13 Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux()

Quoted 11 lines · Jerry Van Baren
Quoted 4 lines · Marian Balakowicz

Make CONFIG_OF_LIBFDT and CONFIG_OF_FLAT_TREE use more readable in PPC variant of do_bootm_linux() routine.

What is the wisdom of the crowd, is it possible to port over the remaining 5 boards and removing the "CONFIG_OF_FLAT_TREE" code in this merge window, or is it too risky? My gut says too risky, do it in the next window.

Quoted 7 lines
$ find . -name "*.[h]" | xargs grep CONFIG_OF_FLAT_TREE | grep 1
./include/configs/mpc7448hpc2.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/sbc8349.h:#define CONFIG_OF_FLAT_TREE 1
./include/configs/MPC8610HPCD.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/stxxtc.h:#define CONFIG_OF_FLAT_TREE  1
./include/configs/sbc8641d.h:#define CONFIG_OF_FLAT_TREE        1

When the 86xx repo is pulled by Wolfgang, the 8610 board will be done!

The sbc8641 board is likely really easy given that I have already done it for the Freescale MPC8641HPCN board. I am not sure who is pushing that code around, though.

jdl

#16 lore raw

PATCH 4/13 Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux()

Quoted 30 lines

-----Original Message-----
From: u-boot-users-bounces at lists.sourceforge.net [mailto:u-boot-users- bounces at lists.sourceforge.net] On Behalf Of Jon Loeliger
Sent: Friday, January 11, 2008 10:49 AM
To: Jerry Van Baren
Cc: u-boot-users at lists.sourceforge.net; Marian Balakowicz
Subject: Re: [U-Boot-Users] [PATCH 04/13] [new uImage] Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux()

Quoted 11 lines · Jerry Van Baren
Quoted 4 lines · Marian Balakowicz

Make CONFIG_OF_LIBFDT and CONFIG_OF_FLAT_TREE use more readable in PPC variant of do_bootm_linux() routine.

What is the wisdom of the crowd, is it possible to port over the remaining 5 boards and removing the "CONFIG_OF_FLAT_TREE" code in this merge window, or is it too risky? My gut says too risky, do it in the next window.

Quoted 7 lines
$ find . -name "*.[h]" | xargs grep CONFIG_OF_FLAT_TREE | grep 1
./include/configs/mpc7448hpc2.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/sbc8349.h:#define CONFIG_OF_FLAT_TREE 1
./include/configs/MPC8610HPCD.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/stxxtc.h:#define CONFIG_OF_FLAT_TREE  1
./include/configs/sbc8641d.h:#define CONFIG_OF_FLAT_TREE        1
Quoted 6 lines

When the 86xx repo is pulled by Wolfgang, the 8610 board will be done!

The sbc8641 board is likely really easy given that I have already done it for the Freescale MPC8641HPCN board. I am not sure who is pushing that code around, though.

I'm going to attempt to get it done this weekend - unless she has plans I don't know about :-P

Quoted 13 lines

jdl

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketpla
ce
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

#17 lore raw

PATCH 4/13 Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux()

In reply to Jerry Van Baren
Quoted 6 lines · On Fri, 11 Jan 2008 11:26:03 -0500 Jerry Van Baren <[email protected]>…
Quoted 4 lines · Marian Balakowicz

Make CONFIG_OF_LIBFDT and CONFIG_OF_FLAT_TREE use more readable in PPC variant of do_bootm_linux() routine.

Quoted 8 lines

What is the wisdom of the crowd, is it possible to port over the remaining 5 boards and removing the "CONFIG_OF_FLAT_TREE" code in this merge window, or is it too risky? My gut says too risky, do it in the next window.

$ find . -name "*.[h]" | xargs grep CONFIG_OF_FLAT_TREE | grep 1
./include/configs/mpc7448hpc2.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/sbc8349.h:#define CONFIG_OF_FLAT_TREE 1

apparently you're not up to date; the sbc8349 has already been converted over.

Kim

#18 lore raw

PATCH 4/13 Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux()

Quoted 13 lines · Kim Phillips
Quoted 10 lines · On Fri, 11 Jan 2008 11:26:03 -0500 Jerry Van Baren <[email protected]>…
Quoted 4 lines · Marian Balakowicz

Make CONFIG_OF_LIBFDT and CONFIG_OF_FLAT_TREE use more readable in PPC variant of do_bootm_linux() routine.

What is the wisdom of the crowd, is it possible to port over the remaining 5 boards and removing the "CONFIG_OF_FLAT_TREE" code in this merge window, or is it too risky? My gut says too risky, do it in the next window.

Quoted 8 lines
Quoted 3 lines
$ find . -name "*.[h]" | xargs grep CONFIG_OF_FLAT_TREE | grep 1
./include/configs/mpc7448hpc2.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/sbc8349.h:#define CONFIG_OF_FLAT_TREE 1

apparently you're not up to date; the sbc8349 has already been converted over.

Kim

Ahh, you're right (blushes).

$ find . -name "*.[h]" | xargs grep CONFIG_OF_FLAT_TREE | grep 1
./include/configs/mpc7448hpc2.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/MPC8610HPCD.h:#define CONFIG_OF_FLAT_TREE     1
./include/configs/stxxtc.h:#define CONFIG_OF_FLAT_TREE  1
./include/configs/sbc8641d.h:#define CONFIG_OF_FLAT_TREE        1
$ find . -name "*.[h]" | xargs grep CONFIG_OF_LIBFDT | grep 1 | wc -l
29

Even better! Add in Jon's pending update and we are down to 3 boards, one of which will be "really easy".

mpc7448hpc2 No maintainer listed, "Created 08/11/2006 Roy Zang"
stxxtc Dan Malek
sbc8641d Joe Hamman

Thanks, everybody,
gvb

#19 lore raw

PATCH 4/13 Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux()

In reply to Jerry Van Baren
Quoted 10 lines · Jerry Van Baren
Quoted 4 lines · Marian Balakowicz

Make CONFIG_OF_LIBFDT and CONFIG_OF_FLAT_TREE use more readable in PPC variant of do_bootm_linux() routine.

What is the wisdom of the crowd, is it possible to port over the remaining 5 boards and removing the "CONFIG_OF_FLAT_TREE" code in this merge window, or is it too risky? My gut says too risky, do it in the next window.

[snip]

I'd be more then happy to removed CONFIG_OF_FLAT_TREE from image/bootm related code. It'll simplify the current code and will allow to avoid unnecessary hassle when adding new functionality.

How about I do such cleanup in the next patchset? It will not hit mainline before the next window and it seems we should be done with converting remaining boards by then. Does that sound reasonable?

Cheers,
m.

#20 lore raw

PATCH 5/13 Move gunzip() common code to common/gunzip.c

In reply to Marian Balakowicz

Move gunzip(), zalloc() and zfree() to a separate file. Share zalloc() and zfree() with cramfs uncompress routine.

---

 common/Makefile        |    1 
 common/cmd_bootm.c     |   94 +---------------------------------------
 common/gunzip.c        |  113 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/cramfs/uncompress.c |   20 +-------
 lib_generic/bzlib.c    |    4 ++
 5 files changed, 123 insertions(+), 109 deletions(-)
 create mode 100644 common/gunzip.c
common/Makefile +1 -0
index 00267e1..c86b3a4 100644
@@ -37,6 +37,7 @@ COBJS-$(CONFIG_CMD_BDI) += cmd_bdinfo.o
 COBJS-$(CONFIG_CMD_BEDBUG) += cmd_bedbug.o
 COBJS-$(CONFIG_CMD_BMP) += cmd_bmp.o
 COBJS-y += image.o
+COBJS-y += gunzip.o
 COBJS-y += cmd_boot.o
 COBJS-y += cmd_bootm.o
 COBJS-$(CONFIG_CMD_CACHE) += cmd_cache.o
common/cmd_bootm.c +3 -91

106 lines not on this page · raw message

common/gunzip.c new file +113 -0

113 lines not on this page · raw message

fs/cramfs/uncompress.c +2 -18
index 2e906eb..e4189e5 100644
@@ -29,24 +29,8 @@
 
 static z_stream stream;
 
-#define ZALLOC_ALIGNMENT	16
-
-static void *zalloc (void *x, unsigned items, unsigned size)
-{
-	void *p;
-
-	size *= items;
-	size = (size + ZALLOC_ALIGNMENT - 1) & ~(ZALLOC_ALIGNMENT - 1);
-
-	p = malloc (size);
-
-	return (p);
-}
-
-static void zfree (void *x, void *addr, unsigned nb)
-{
-	free (addr);
-}
+void *zalloc(void *, unsigned, unsigned);
+void zfree(void *, void *, unsigned);
 
 /* Returns length of decompressed data. */
 int cramfs_uncompress_block (void *dst, void *src, int srclen)
lib_generic/bzlib.c +4 -0
index 87e6a6e..0d3f9c2 100644
@@ -1592,6 +1592,10 @@ const char * BZ_API(BZ2_bzerror) (BZFILE *b, int *errnum)
 }
 #endif
 
+void bz_internal_error(int errcode)
+{
+	printf ("BZIP2 internal error %d\n", errcode);
+}
 
 /*-------------------------------------------------------------*/
 /*--- end                                           bzlib.c ---*/
#21 lore raw

PATCH 6/13 Move CHUNKSZ definition to image.h

In reply to Marian Balakowicz

CHUNKSZ defined for PPC and M68K is set to the same value of 64K, move this definition to a common header.

---

 common/cmd_bootm.c    |    7 -------
 include/image.h       |    7 +++++++
 lib_m68k/m68k_linux.c |    2 --
 lib_ppc/ppc_linux.c   |    2 --
 4 files changed, 7 insertions(+), 11 deletions(-)
common/cmd_bootm.c +0 -7
index 76ea49f..4b2ea3f 100644
@@ -61,13 +61,6 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
 #include <dataflash.h>
 #endif
 
-/*
- * Some systems (for example LWMON) have very short watchdog periods;
- * we must make sure to split long operations like memmove() or
- * crc32() into reasonable chunks.
- */
-#define CHUNKSZ (64 * 1024)
-
 int  gunzip (void *, int, unsigned char *, unsigned long *);
 
 #ifdef CONFIG_BZIP2
include/image.h +7 -0
index b4ba2de..00a4549 100644
@@ -163,6 +163,13 @@ typedef struct image_header {
 	uint8_t		ih_name[IH_NMLEN];	/* Image Name		*/
 } image_header_t;
 
+/*
+ * Some systems (for example LWMON) have very short watchdog periods;
+ * we must make sure to split long operations like memmove() or
+ * crc32() into reasonable chunks.
+ */
+#define CHUNKSZ (64 * 1024)
+
 #define image_to_cpu(x)		ntohl(x)
 #define cpu_to_image(x)		htonl(x)
 
lib_m68k/m68k_linux.c +0 -2
index 6c2a6b2..237cd82 100644
@@ -37,8 +37,6 @@ DECLARE_GLOBAL_DATA_PTR;
 #define LINUX_MAX_ENVS		256
 #define LINUX_MAX_ARGS		256
 
-#define CHUNKSZ			(64 * 1024)
-
 #ifdef CONFIG_SHOW_BOOT_PROGRESS
 # include <status_led.h>
 # define SHOW_BOOT_PROGRESS(arg)	show_boot_progress(arg)
lib_ppc/ppc_linux.c +0 -2
index 3a4b74f..1013f9e 100644
@@ -49,8 +49,6 @@
 #include <asm/cache.h>
 #endif
 
-#define CHUNKSZ (64 * 1024)
-
 DECLARE_GLOBAL_DATA_PTR;
 extern image_header_t header;
 
#22 lore raw

PATCH 7/13 Remove I386 uImage fake_header() routine

In reply to Marian Balakowicz

I386 targets are not using a uImage format, instead fake header is added to ram image before it is further processed by bootm.

Remove this fixup and force proper uImage use for I386.

---

 common/cmd_bootm.c        |   16 ---------------
 include/asm-i386/zimage.h |    1 -
 lib_i386/i386_linux.c     |   12 -----------
 lib_i386/zimage.c         |   48 ---------------------------------------------
 4 files changed, 0 insertions(+), 77 deletions(-)
common/cmd_bootm.c +0 -16
index 4b2ea3f..40df9b0 100644
@@ -79,10 +79,6 @@ static int do_imls (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
 
 static void print_type (image_header_t *hdr);
 
-#ifdef __I386__
-image_header_t *fake_header(image_header_t *hdr, void *ptr, int size);
-#endif
-
 /*
  *  Continue booting an OS image; caller already has:
  *  - copied image header to global variable `header'
@@ -158,21 +154,9 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	memmove (&header, (char *)addr, image_get_header_size());
 
 	if (!image_check_magic(hdr)) {
-#ifdef __I386__	/* correct image format not implemented yet - fake it */
-		if (fake_header(hdr, (void*)addr, -1) != NULL) {
-			/* to compensate for the addition below */
-			addr -= image_get_header_size();
-			/* turnof verify,
-			 * fake_header() does not fake the data crc
-			 */
-			verify = 0;
-		} else
-#endif	/* __I386__ */
-	    {
 		puts ("Bad Magic Number\n");
 		show_boot_progress (-1);
 		return 1;
-	    }
 	}
 	show_boot_progress (2);
 
include/asm-i386/zimage.h +0 -1
index c7103b1..b6266e4 100644
@@ -70,6 +70,5 @@ void *load_zimage(char *image, unsigned long kernel_size,
 		  int auto_boot);
 
 void boot_zimage(void *setup_base);
-image_header_t *fake_zimage_header(image_header_t *hdr, void *ptr, int size);
 
 #endif
lib_i386/i386_linux.c +0 -12
index d8ae4fc..8792d84 100644
@@ -33,18 +33,6 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
 
 extern image_header_t header;           /* from cmd_bootm.c */
 
-
-image_header_t *fake_header(image_header_t *hdr, void *ptr, int size)
-{
-	/* try each supported image type in order */
-	if (NULL != fake_zimage_header(hdr, ptr, size)) {
-		return hdr;
-	}
-
-	return NULL;
-}
-
-
 void do_bootm_linux(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[],
 		ulong addr, ulong *len_ptr, int   verify)
 {
lib_i386/zimage.c +0 -48

51 lines not on this page · raw message

#23 lore raw

PATCH 8/13 Fix uImage header pointer use in i386 do_bootm_linux()

In reply to Marian Balakowicz

Use image header copy instead of a (possibly corrupted) pointer to a initial image location.

---

 lib_i386/i386_linux.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
lib_i386/i386_linux.c +2 -2
index 8792d84..3a92570 100644
@@ -129,13 +129,13 @@ void do_bootm_linux(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[],
 	}
 
 	/* if multi-part image, we need to advance base ptr */
-	if (image_check_type(hdr, IH_TYPE_MULTI) && (len_ptr[1])) {
+	if (image_check_type(&header, IH_TYPE_MULTI) && (len_ptr[1])) {
 		int i;
 		for (i=0, addr+=sizeof(int); len_ptr[i++]; addr+=sizeof(int));
 	}
 
 	base_ptr = load_zimage((void*)addr + image_get_header_size(),
-			       image_get_data_size(hdr),
+			       image_get_data_size(&header),
 			       initrd_start, initrd_end-initrd_start, 0);
 
 	if (NULL == base_ptr) {
#24 lore raw

PATCH 9/13 Fix FDT header verification in PPC do_boot_linux() routine

In reply to Marian Balakowicz

---

 lib_ppc/ppc_linux.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
lib_ppc/ppc_linux.c +2 -2
index 1013f9e..732f1f3 100644
@@ -264,9 +264,9 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 		of_flat_tree = (char *) simple_strtoul(argv[3], NULL, 16);
 		hdr = (image_header_t *)of_flat_tree;
 #if defined(CONFIG_OF_FLAT_TREE)
-		if (*((ulong *)(of_flat_tree + image_get_header_size())) != OF_DT_HEADER) {
+		if (*((ulong *)(of_flat_tree)) == OF_DT_HEADER) {
 #elif defined(CONFIG_OF_LIBFDT)
-		if (fdt_check_header(of_flat_tree + image_get_header_size()) != 0) {
+		if (fdt_check_header(of_flat_tree) == 0) {
 #endif
 #ifndef CFG_NO_FLASH
 			if (addr2info((ulong)of_flat_tree) != NULL)
#25 lore raw

PATCH 10/13 Add memmove_wd() common routine

In reply to Marian Balakowicz

Move common, watchdog sensible memmove code to a helper memmmove_wd() routine.

---

 common/cmd_bootm.c    |   20 ++++----------------
 common/image.c        |   18 ++++++++++++++++++
 include/image.h       |    1 +
 lib_m68k/m68k_linux.c |   23 ++++-------------------
 lib_ppc/ppc_linux.c   |   22 ++++------------------
 5 files changed, 31 insertions(+), 53 deletions(-)
common/cmd_bootm.c +4 -16
index 40df9b0..7c3bb2f 100644
@@ -250,24 +250,12 @@ int do_bootm (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 		if (image_get_load(hdr) == addr) {
 			printf ("   XIP %s ... ", name);
 		} else {
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
-			size_t l = len;
-			void *to = (void *)image_get_load(hdr);
-			void *from = (void *)data;
-
 			printf ("   Loading %s ... ", name);
 
-			while (l > 0) {
-				size_t tail = (l > CHUNKSZ) ? CHUNKSZ : l;
-				WATCHDOG_RESET();
-				memmove (to, from, tail);
-				to += tail;
-				from += tail;
-				l -= tail;
-			}
-#else	/* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
-			memmove ((void *)image_get_load(hdr), (uchar *)data, len);
-#endif	/* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
+			memmove_wd((void *)image_get_load(hdr),
+				   (void *)data, len, CHUNKSZ);
+
+			puts("OK\n");
 		}
 		break;
 	case IH_COMP_GZIP:
common/image.c +18 -0
index 11470bb..7e862be 100644
@@ -24,6 +24,7 @@
  */
 #ifndef USE_HOSTCC
 # include <common.h>
+# include <watchdog.h>
 #else
 # include "mkimage.h"
 #endif
@@ -56,6 +57,7 @@ int image_check_dcrc(image_header_t *hdr)
 	return (dcrc == image_get_dcrc(hdr));
 }
 
+#ifndef USE_HOSTCC
 int image_check_dcrc_wd(image_header_t *hdr, ulong chunksz)
 {
 	ulong dcrc = 0;
@@ -89,3 +91,19 @@ int getenv_verify(void)
 	return (s && (*s == 'n')) ? 0 : 1;
 }
 
+void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
+{
+#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
+	while (len > 0) {
+		size_t tail = (len > chunksz) ? chunksz : len;
+		WATCHDOG_RESET();
+		memmove(to, from, tail);
+		to += tail;
+		from += tail;
+		len -= tail;
+	}
+#else	/* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
+	memmove(to, from, len);
+#endif	/* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
+}
+#endif /* USE_HOSTCC */
include/image.h +1 -0
index 00a4549..1c26a9c 100644
@@ -251,6 +251,7 @@ int image_check_hcrc(image_header_t *);
 int image_check_dcrc(image_header_t *);
 int image_check_dcrc_wd(image_header_t *, ulong);
 int getenv_verify(void);
+void memmove_wd(void *, void *, size_t, ulong);
 
 static inline int image_check_magic(image_header_t *hdr)
 {
lib_m68k/m68k_linux.c +4 -19
index 237cd82..4ea74d7 100644
@@ -266,25 +266,10 @@ void do_bootm_linux(cmd_tbl_t * cmdtp, int flag,
 			initrd_end = initrd_start + len;
 			printf("   Loading Ramdisk to %08lx, end %08lx ... ",
 			       initrd_start, initrd_end);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
-			{
-				size_t l = len;
-				void *to = (void *)initrd_start;
-				void *from = (void *)data;
-
-				while (l > 0) {
-					size_t tail =
-					    (l > CHUNKSZ) ? CHUNKSZ : l;
-					WATCHDOG_RESET();
-					memmove(to, from, tail);
-					to += tail;
-					from += tail;
-					l -= tail;
-				}
-			}
-#else				/* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
-			memmove((void *)initrd_start, (void *)data, len);
-#endif				/* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
+
+			memmove_wd((void *)initrd_start,
+				   (void *)data, len, CHUNKSZ);
+
 			puts("OK\n");
 		}
 	} else {
lib_ppc/ppc_linux.c +4 -18
index 732f1f3..15349b3 100644
@@ -425,24 +425,10 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag,
 		initrd_end    = initrd_start + len;
 		printf ("   Loading Ramdisk to %08lx, end %08lx ... ",
 			initrd_start, initrd_end);
-#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
-		{
-			size_t l = len;
-			void *to = (void *)initrd_start;
-			void *from = (void *)data;
-
-			while (l > 0) {
-				size_t tail = (l > CHUNKSZ) ? CHUNKSZ : l;
-				WATCHDOG_RESET();
-				memmove (to, from, tail);
-				to += tail;
-				from += tail;
-				l -= tail;
-			}
-		}
-#else	/* !(CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG) */
-		memmove ((void *)initrd_start, (void *)data, len);
-#endif	/* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
+
+		memmove_wd((void *)initrd_start,
+			   (void *)data, len, CHUNKSZ);
+
 		puts ("OK\n");
 	    }
 	} else {
#26 lore raw

PATCH 11/13 Coding style cleanup - part 1

In reply to Marian Balakowicz

common/cmd_autoscript.c
common/cmd_bootm.c
common/lynxkdi.c

- sort and cleanup headers, declarations, etc.
- group related routines
- cleanup indentation, white spaces

---

 common/cmd_autoscript.c |   32 +-
 common/cmd_bootm.c      |  755 ++++++++++++++++++++++++-----------------------
 common/lynxkdi.c        |   36 +-
 3 files changed, 415 insertions(+), 408 deletions(-)
common/cmd_autoscript.c +14 -18
index c1ff32a..5948cd5 100644
@@ -49,8 +49,7 @@
 
 #if defined(CONFIG_AUTOSCRIPT) || defined(CONFIG_CMD_AUTOSCRIPT)
 
-int
-autoscript (ulong addr)
+int autoscript(ulong addr)
 {
 	ulong len;
 	image_header_t *hdr = (image_header_t *)addr;
@@ -93,18 +92,17 @@ autoscript (ulong addr)
 
 	debug ("** Script length: %ld\n", len);
 
-	if ((cmd = malloc (len + 1)) == NULL) {
+	if ((cmd = malloc(len + 1)) == NULL)
 		return 1;
-	}
 
 	while (*len_ptr++);
 
 	/* make sure cmd is null terminated */
-	memmove (cmd, (char *)len_ptr, len);
+	memmove(cmd, (char *)len_ptr, len);
 	*(cmd + len) = 0;
 
 #ifdef CFG_HUSH_PARSER /*?? */
-	rcode = parse_string_outer (cmd, FLAG_PARSE_SEMICOLON);
+	rcode = parse_string_outer(cmd, FLAG_PARSE_SEMICOLON);
 #else
 	{
 		char *line = cmd;
@@ -120,9 +118,9 @@ autoscript (ulong addr)
 				*next = '\0';
 				/* run only non-empty commands */
 				if ((next - line) > 1) {
-					debug ("** exec: \"%s\"\n",
-						line);
-					if (run_command (line, 0) < 0) {
+					debug("** exec: \"%s\"\n",
+							line);
+					if (run_command(line, 0) < 0) {
 						rcode = 1;
 						break;
 					}
@@ -133,7 +131,7 @@ autoscript (ulong addr)
 		}
 	}
 #endif
-	free (cmd);
+	free(cmd);
 	return rcode;
 }
 
@@ -141,20 +139,18 @@ autoscript (ulong addr)
 
 /**************************************************/
 #if defined(CONFIG_CMD_AUTOSCRIPT)
-int
-do_autoscript (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
+int do_autoscript(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	ulong addr;
 	int rcode;
 
-	if (argc < 2) {
+	if (argc < 2)
 		addr = CFG_LOAD_ADDR;
-	} else {
-		addr = simple_strtoul (argv[1],0,16);
-	}
+	else
+		addr = simple_strtoul(argv[1], 0, 16);
 
-	printf ("## Executing script at %08lx\n",addr);
-	rcode = autoscript (addr);
+	printf("## Executing script@%08lx\n",addr);
+	rcode = autoscript(addr);
 	return rcode;
 }
 
common/cmd_bootm.c +383 -372

1,077 lines not on this page · raw message

common/lynxkdi.c +18 -18
index 62b2a04..5516f85 100644
@@ -23,14 +23,14 @@
 DECLARE_GLOBAL_DATA_PTR;
 
 #if defined(CONFIG_MPC8260) || defined(CONFIG_440EP) || defined(CONFIG_440GR)
-void lynxkdi_boot ( image_header_t *hdr )
+void lynxkdi_boot(image_header_t *hdr)
 {
 	void (*lynxkdi)(void) = (void(*)(void))image_get_ep(hdr);
 	lynxos_bootparms_t *parms = (lynxos_bootparms_t *)0x0020;
 	bd_t *kbd;
 	u32 *psz = (u32 *)(image_get_load(hdr) + 0x0204);
 
-	memset( parms, 0, sizeof(*parms));
+	memset(parms, 0, sizeof(*parms));
 	kbd = gd->bd;
 	parms->clock_ref = kbd->bi_busfreq;
 	parms->dramsz = kbd->bi_memsize;
@@ -40,25 +40,25 @@ void lynxkdi_boot ( image_header_t *hdr )
 	/* Do a simple check for Bluecat so we can pass the
 	 * kernel command line parameters.
 	 */
-	if( le32_to_cpu(*psz) == image_get_data_size(hdr) ){	/* FIXME: NOT SURE HERE ! */
-	    char *args;
-	    char *cmdline = (char *)(image_get_load(hdr) + 0x020c);
-	    int len;
+	if (le32_to_cpu(*psz) == image_get_data_size(hdr)) {	/* FIXME: NOT SURE HERE ! */
+		char *args;
+		char *cmdline = (char *)(image_get_load(hdr) + 0x020c);
+		int len;
 
-	    printf("Booting Bluecat KDI ...\n");
-	    udelay(200*1000); /* Allow serial port to flush */
-	    if ((args = getenv("bootargs")) == NULL)
-		    args = "";
-	    /* Prepend the cmdline */
-	    len = strlen(args);
-	    if( len && (len + strlen(cmdline) + 2 < (0x0400 - 0x020c))) {
-		memmove( cmdline + strlen(args) + 1, cmdline, strlen(cmdline) );
-		strcpy( cmdline, args );
-		cmdline[len] = ' ';
-	    }
+		printf("Booting Bluecat KDI ...\n");
+		udelay(200*1000); /* Allow serial port to flush */
+		if ((args = getenv("bootargs")) == NULL)
+			args = "";
+		/* Prepend the cmdline */
+		len = strlen(args);
+		if( len && (len + strlen(cmdline) + 2 < (0x0400 - 0x020c))) {
+			memmove( cmdline + strlen(args) + 1, cmdline, strlen(cmdline) );
+			strcpy( cmdline, args );
+			cmdline[len] = ' ';
+		}
 	}
 	else {
-	    printf("Booting LynxOS KDI ...\n");
+		printf("Booting LynxOS KDI ...\n");
 	}
 
 	lynxkdi();
#27 lore raw

PATCH 11/13 Coding style cleanup - part 1

Quoted 7 lines · In message <[email protected]> you wrote:

common/cmd_autoscript.c
common/cmd_bootm.c
common/lynxkdi.c

- sort and cleanup headers, declarations, etc.
- group related routines
- cleanup indentation, white spaces

Be careful!

Quoted 2 lines
common/cmd_autoscript.c +1 -1
-	memmove (cmd, (char *)len_ptr, len);
+	memmove(cmd, (char *)len_ptr, len);

...

Quoted 2 lines
common/cmd_autoscript.c +1 -1
-	rcode = parse_string_outer (cmd, FLAG_PARSE_SEMICOLON);
+	rcode = parse_string_outer(cmd, FLAG_PARSE_SEMICOLON);

...

Quoted 2 lines
common/cmd_autoscript.c +1 -1
-					debug ("** exec: \"%s\"\n",
+					debug("** exec: \"%s\"\n",

...

common/cmd_autoscript.c autoscript (ulong addr) · line 134

Quoted 2 lines
common/cmd_autoscript.c +1 -1
-	free (cmd);
+	free(cmd);

This is not a cleanup, this makes it (IMO) worse. Please undo.

common/cmd_bootm.c U_BOOT_CMD( · line 441

Quoted 6 lines
common/cmd_bootm.c +3 -3
-	int	arg;
-	ulong	addr;
-	int     rcode=0;
+	int arg;
+	ulong addr;
+	int rcode = 0;

Do you really think the new code is better or cleaner? It's just uglier.

Please undo.

Best regards,

Wolfgang Denk

Signature · 4 lines
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
"You'll pay to know what you really think."        - J.R. "Bob" Dobbs
#28 lore raw

PATCH 11/13 Coding style cleanup - part 1

Quoted 1 line · Wolfgang Denk

In message <[email protected]> you wrote:

[snip]

Quoted 10 lines

common/cmd_bootm.c U_BOOT_CMD(

Quoted 6 lines
common/cmd_bootm.c +3 -3
-	int	arg;
-	ulong	addr;
-	int     rcode=
+	int arg;
+	ulong addr;
+	int rcode =;

Do you really think the new code is better or cleaner? It's just uglier.

Quoted 5 lines

Please undo.

Best regards,

Wolfgang Denk

Hi Wolfgang,

I agree the change is uglier. Unfortunately, running a file through Lindent does this. (My personal coding style uses much more whitespace than the Linux style, so sometimes I need to run Lindent to remove spaces my fingers put in automatically.)

Maybe the coding style should be "run 'Lindent -pcs', then undo the following...". I'll admit that I'm already removing all the tabs that Lindent inserts before comment in the case of

#endif /* defined(FOO) */

Best regards,
Larry

#29 lore raw

PATCH 11/13 Coding style cleanup - part 1

In reply to Wolfgang Denk
Quoted 11 lines · Wolfgang Denk
Quoted 7 lines · In message <[email protected]> you wrote:

common/cmd_autoscript.c
common/cmd_bootm.c
common/lynxkdi.c

- sort and cleanup headers, declarations, etc.
- group related routines
- cleanup indentation, white spaces

Be careful!

[snip]

Quoted 2 lines

This is not a cleanup, this makes it (IMO) worse. Please undo.

[snip]

Quoted 3 lines

Do you really think the new code is better or cleaner? It's just uglier.

Thanks for pointing this out, will fix this.

Cheers,
m.

#30 lore raw

PATCH 12/13 Cleanup image header pointer use in bootm code

In reply to Marian Balakowicz

- use single image header pointer instead of a set of auxilliary variables.
- add multi component image helper routines: get component size/data address

---

 common/cmd_autoscript.c           |   10 +-
 common/cmd_bootm.c                |  162 ++++++++++++++--------------------
 common/image.c                    |   85 ++++++++++++++++++
 include/image.h                   |   25 +++++
 lib_arm/armlinux.c                |   92 ++++++++-----------
 lib_avr32/avr32_linux.c           |   70 +++++----------
 lib_blackfin/bf533_linux.c        |    4 -
 lib_i386/i386_linux.c             |   81 +++++++----------
 lib_m68k/m68k_linux.c             |   82 +++++++----------
 lib_microblaze/microblaze_linux.c |   71 ++++++---------
 lib_mips/mips_linux.c             |   68 +++++---------
 lib_nios/nios_linux.c             |    2 
 lib_nios2/nios_linux.c            |    7 -
 lib_ppc/ppc_linux.c               |  176 +++++++++++++++----------------------
 14 files changed, 440 insertions(+), 495 deletions(-)

This patch size is over 40K list limit, please refer to:
http://semihalf.com/~m8/new-image/patchset1/12-multi_helpers-single_hdr.patch

#31 lore raw

PATCH 13/13 Return error on image move/uncompress overwrites

In reply to Marian Balakowicz

Check for overwrites during image move/uncompress, return with error when the original image gets corrupted. Report clear message to the user and prevent further troubles when pointer to the corrupted images is passed to do_bootm_linux routine.

---

 common/cmd_bootm.c  |   23 ++++++++++++++++++++++-
 lib_ppc/ppc_linux.c |   14 ++++++++++++--
 2 files changed, 34 insertions(+), 3 deletions(-)
common/cmd_bootm.c +22 -1
index 6a9d5ad..b5f7526 100644
@@ -133,6 +133,10 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	ulong		img_addr;
 	ulong		os_data, os_len;
 
+	ulong		image_start, image_end;
+	ulong		load_start, load_end;
+
+
 	if (argc < 2)
 		img_addr = load_addr;
 	else
@@ -232,16 +236,21 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	dcache_disable();
 #endif
 
+	image_start = (ulong)hdr;
+	image_end = image_get_image_end(hdr);
+	load_start = image_get_load(hdr);
+	load_end = 0;
+
 	switch (image_get_comp(hdr)) {
 	case IH_COMP_NONE:
 		if (image_get_load(hdr) == img_addr) {
 			printf("   XIP %s ... ", name);
 		} else {
 			printf("   Loading %s ... ", name);
-
 			memmove_wd((void *)image_get_load(hdr),
 				   (void *)os_data, os_len, CHUNKSZ);
 
+			load_end = load_start + os_len;
 			puts("OK\n");
 		}
 		break;
@@ -253,6 +262,8 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 			show_boot_progress(-6);
 			do_reset(cmdtp, flag, argc, argv);
 		}
+
+		load_end = load_start + os_len;
 		break;
 #ifdef CONFIG_BZIP2
 	case IH_COMP_BZIP2:
@@ -270,6 +281,8 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 			show_boot_progress(-6);
 			do_reset(cmdtp, flag, argc, argv);
 		}
+
+		load_end = load_start + unc_len;
 		break;
 #endif /* CONFIG_BZIP2 */
 	default:
@@ -282,6 +295,14 @@ int do_bootm(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	puts("OK\n");
 	show_boot_progress(7);
 
+	if ((load_start < image_end) && (load_end > image_start)) {
+		debug("image_start = 0x%lX, image_end = 0x%lx\n", image_start, image_end);
+		debug("load_start = 0x%lx, load_end = 0x%lx\n", load_start, load_end);
+
+		puts("ERROR: image overwritten - must RESET the board to recover.\n");
+		do_reset (cmdtp, flag, argc, argv);
+	}
+
 	switch (image_get_type(hdr)) {
 	case IH_TYPE_STANDALONE:
 		if (iflag)
lib_ppc/ppc_linux.c +12 -2
index 56cdcc8..e0b55db 100644
@@ -23,6 +23,8 @@
  * MA 02111-1307 USA
  */
 
+#define DEBUG
+
 #include <common.h>
 #include <watchdog.h>
 #include <command.h>
@@ -259,11 +261,19 @@ do_bootm_linux(cmd_tbl_t *cmdtp, int flag,
 				of_data = (ulong)of_flat_tree;
 #endif
 		} else if (image_check_magic(fdt_hdr)) {
+			ulong image_start, image_end;
+			ulong load_start, load_end;
+
 			printf("## Flat Device Tree at %08lX\n", fdt_hdr);
 			print_image_hdr(fdt_hdr);
 
-			if ((image_get_load(fdt_hdr) < image_get_image_end(fdt_hdr)) &&
-			   ((image_get_load(fdt_hdr) + image_get_data_size(fdt_hdr)) > (unsigned long)fdt_hdr)) {
+			image_start = (ulong)fdt_hdr;
+			image_end = image_get_image_end(fdt_hdr);
+
+			load_start = image_get_load(fdt_hdr);
+			load_end = load_start + image_get_data_size(fdt_hdr);
+
+			if ((load_start < image_end) && (load_end > image_start)) {
 				puts ("ERROR: fdt overwritten - "
 					"must RESET the board to recover.\n");
 				do_reset (cmdtp, flag, argc, argv);
#32 lore raw

PATCH 13/13 Return error on image move/uncompress overwrites

Quoted 4 lines · In message <[email protected]> you wrote:

Check for overwrites during image move/uncompress, return with error when the original image gets corrupted. Report clear message to the user and prevent further troubles when pointer to the corrupted images is passed to do_bootm_linux routine.

Does this really work? With compressed images?

lib_ppc/ppc_linux.c do_bootm_linux(cmd_tbl_t *cmdtp, int flag, · line 279

Quoted 10 lines
lib_ppc/ppc_linux.c +7 -0
+			image_start = (ulong)fdt_hdr;
+			image_end = image_get_image_end(fdt_hdr);
+
+			load_start = image_get_load(fdt_hdr);
+			load_end = load_start + image_get_data_size(fdt_hdr);
+
+			if ((load_start < image_end) && (load_end > image_start)) {
 				puts ("ERROR: fdt overwritten - "
 					"must RESET the board to recover.\n");
 				do_reset (cmdtp, flag, argc, argv);

Me thinks that load_end gives a value which does not take into account that the loaded, uncompressed image will be much bigger than image_get_data_size() tells - or am I missing something?

Best regards,

Wolfgang Denk

Signature · 4 lines
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
The Gates in my computer are AND, OR and NOT; they are not Bill.
#33 lore raw

PATCH 13/13 Return error on image move/uncompress overwrites

Quoted 7 lines · Wolfgang Denk
Quoted 4 lines · In message <[email protected]> you wrote:

Check for overwrites during image move/uncompress, return with error when the original image gets corrupted. Report clear message to the user and prevent further troubles when pointer to the corrupted images is passed to do_bootm_linux routine.

Does this really work? With compressed images?

Yes it does, I tested it with compressed linux kernel images.

Quoted 6 lines
Quoted 6 lines
lib_ppc/ppc_linux.c +6 -0
+			image_start = (ulong)fdt_hdr;
+			image_end = image_get_image_end(fdt_hdr);
+
+			load_start = image_get_load(fdt_hdr);
+			load_end = load_start + image_get_data_size(fdt_hdr);
+
Quoted 8 lines

lib_ppc/ppc_linux.c do_bootm_linux(cmd_tbl_t *cmdtp, int flag, · line 279

Quoted 4 lines
lib_ppc/ppc_linux.c +1 -0
+			if ((load_start < image_end) && (load_end > image_start)) {
 				puts ("ERROR: fdt overwritten - "
 					"must RESET the board to recover.\n");
 				do_reset (cmdtp, flag, argc, argv);

Me thinks that load_end gives a value which does not take into account that the loaded, uncompressed image will be much bigger than image_get_data_size() tells - or am I missing something?

That part of the patch deals with the FDT blob image. Compressed blobs are not supported in the current code, so no decompression happens and we can use image_get_data_size(). BTW, this particular overwrite check is already present in the current U-boot.

Cheers,
m.

#34 lore raw

cover new uImage patchset1 - cleanup

In reply to Marian Balakowicz
Quoted 20 lines · Marian Balakowicz

This is a first set of patches that provides a initial cleanup and code rearrangements before the proper new uImage format implementation.

API is defined for the current (old) image related processing, and it'is being introduced in place of the direct header access.

[new uImage] Return error on image move/uncompress overwrites [new uImage] Cleanup image header pointer use in bootm code [new uImage] Coding style cleanup - part 1
[new uImage] Add memmove_wd() common routine
[new uImage] Fix FDT header verification in PPC do_boot_linux() routine [new uImage] Fix uImage header pointer use in i386 do_bootm_linux() [new uImage] Remove I386 uImage fake_header() routine
[new uImage] Move CHUNKSZ definition to image.h
[new uImage] Move gunzip() common code to common/gunzip.c [new uImage] Cleanup OF/FDT #if/#elif/#endif use in do_bootm_linux() [new uImage] Move PPC do_bootm_linux() to lib_ppc/ppc_linux.c [new uImage] Define a API for image handling operations
Add missing cmd_ximg.o to common/Makefile

Quoted 4 lines

Comments and suggestions are welcome.

Due to a 40K message limit, not all of the patches can be sent to ml, oversized patches will provide a http location instead.

Argh! :-(

Best regard,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

#35 lore raw

cover new uImage patchset1 - cleanup

Quoted 1 line · Stefan Roese

On Friday 11 January 2008, Marian Balakowicz wrote:

[snip]

Quoted 7 lines
Quoted 2 lines

Due to a 40K message limit, not all of the patches can be sent to ml, oversized patches will provide a http location instead.

Argh! :-(

Best regard,
Stefan

Fortunately, we browbeat the list admin into exercising some flexibility on oversize messages. :-D

Now we have to start working him over to make the hard limit 100K with a soft limit of 400K to keep up with linux-ppc (and yes, that's a joke!).

Best regards,
gvb

#36 lore raw

cover new uImage patchset1 - cleanup

Quoted 3 lines · In message <[email protected]> you wrote:

Fortunately, we browbeat the list admin into exercising some flexibility on oversize messages. :-D

I still don't know how to sit or lie with all the bruises on my back.

Quoted 2 lines

Now we have to start working him over to make the hard limit 100K with a soft limit of 400K to keep up with linux-ppc (and yes, that's a joke!).

Do you remember my words that 100 kB would not be enough? It already happened. Next week you will ask for a 32 MB limit.

Best regards,

Wolfgang Denk

Signature · 5 lines
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
It usually takes more than three weeks to prepare  a  good  impromptu
speech.                                                  - Mark Twain
#37 lore raw

cover new uImage patchset1 - cleanup

In reply to Stefan Roese
Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 3 lines

Due to a 40K message limit, not all of the patches can be sent to ml, oversized patches will provide a http location instead.

Argh! :-(

Just to tell the complete story: we discussed this internally before, and even a 100 kB klimit would have been too small.

Best regards,

Wolfgang Denk

Signature · 4 lines
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
Nobody goes to that restaurant anymore. It's too crowded.
#38 lore raw

cover new uImage patchset1 - cleanup

In reply to Marian Balakowicz
Quoted 5 lines · In message <[email protected]> you wrote:

This is a first set of patches that provides a initial cleanup and code rearrangements before the proper new uImage format implementation.

API is defined for the current (old) image related processing, and it'is being introduced in place of the direct header access.

Can you please comment a bit about the current state of these patches?

Is this just a RFC, or tested code?

Does the code compile? For which boards / architectures?

How much does it change the memory footprint?

Viele Gr??e,

Wolfgang Denk

Signature · 5 lines
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
Extreme feminine beauty is always disturbing.
	-- Spock, "The Cloud Minders", stardate 5818.4
#39 lore raw

cover new uImage patchset1 - cleanup

Quoted 10 lines · Wolfgang Denk
Quoted 5 lines · In message <[email protected]> you wrote:

This is a first set of patches that provides a initial cleanup and code rearrangements before the proper new uImage format implementation.

API is defined for the current (old) image related processing, and it'is being introduced in place of the direct header access.

Can you please comment a bit about the current state of these patches?

Is this just a RFC, or tested code?

It has been tested on lite5200b board. Tests covering bootm (single na multi component images), autoscr, iminfo, imls.

Quoted 1 line

Does the code compile? For which boards / architectures?

Yes, it does. MAKEALL builds were run for ppc, mips, arm and coldfire (although not all targets, as some of them are broken in the baseline code). I have also attempted to build on i386, but the i386 target builds are broken as well, I have just verified that modified files do compile. Other architectures were skipped due to lack of the toolchains.

It would be good if other architecture maintainers could help testing new uImage patches, as the changes are cross platform. Another good verification would be booting OS other than linux. But, as there are more significant changes coming, it may have more sens to wait with the detailed tests until crucial modifications are finished.

Quoted 1 line

How much does it change the memory footprint?

Here is the build output for lite5200b_LOWBOOT configuration and with the patches applied:

text data bss dec hex filename
222739 13120 305152 541011 84153 ./u-boot

Note, that patches enable DEBUG in do_bootm() and PPC variant of do_bootm_linux(), with those disabled we get:

text data bss dec hex filename
222011 13080 305152 540243 83e53 ./u-boot

and the same configuration build for the 1.3.1 baseline:

text data bss dec hex filename
221463 13040 305408 539911 83d07 ./u-boot

Best Regards,
Marian