[U-Boot] [PATCH 1/5] Added documentation for CONFIG_SYS_TEXT_BASE for ARM.

[U-Boot] [PATCH 1/5] Added documentation for CONFIG_SYS_TEXT_BASE for ARM.

[Christopher Harvey]

Signed-off-by: Christopher Harvey <charvey@matrox.com>
---
 README |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/README b/README
index a760cf3..a0090ef 100644
--- a/README
+++ b/README
@@ -2355,6 +2355,15 @@ Configuration Settings:
 - CONFIG_SYS_SDRAM_BASE:
 		Physical start address of SDRAM. _Must_ be 0 here.
 
+- CONFIG_SYS_TEXT_BASE:
+		- ARM:
+		Is the address of the u-boot code that is loaded in memory. 
+		This value can be in ROM space since u-boot can run from
+		within ROM. CONFIG_SYS_TEXT_BASE is simply called _TEXT_BASE
+		in some files, like arch/arm/lib/board.c.
+		This value has nothing to do with the relocation destination
+		in RAM. See doc/README.arm-relocation for more info.
+
 - CONFIG_SYS_MBIO_BASE:
 		Physical start address of Motherboard I/O (if using a
 		Cogent motherboard)
-- 
1.7.3.4

[U-Boot] [PATCH 1/5] Added documentation for CONFIG_SYS_TEXT_BASE for ARM.

[Wolfgang Denk]

Dear Christopher Harvey,

In message <20110704174320.GB3016@harvey-pc.matrox.com> you wrote:
> Signed-off-by: Christopher Harvey <charvey@matrox.com>
> ---
>  README |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)

Please don't make this ARM specific.  CONFIG_SYS_TEXT_BASE has the
same meaning for all architectures.

> diff --git a/README b/README
> index a760cf3..a0090ef 100644
> --- a/README
> +++ b/README
> @@ -2355,6 +2355,15 @@ Configuration Settings:
>  - CONFIG_SYS_SDRAM_BASE:
>  		Physical start address of SDRAM. _Must_ be 0 here.
>  
> +- CONFIG_SYS_TEXT_BASE:
> +		- ARM:
> +		Is the address of the u-boot code that is loaded in memory. 

To be precise: it is the address of the start address of U-Boot in
memory.

Please omit the "that is loaded in memory", because for example on
systems booting from ROM (or NOR flash etc.) nothing gets loaded.

> +		This value can be in ROM space since u-boot can run from
> +		within ROM. ...

Again, not strictly correct.  Only some parts of U-Boot (the part
before relocation) can run from ROM.

> +		        ... CONFIG_SYS_TEXT_BASE is simply called _TEXT_BASE
> +		in some files, like arch/arm/lib/board.c.

I consider this a bad idea. It should not be documented, but rather
fixed.

> +		This value has nothing to do with the relocation destination
> +		in RAM. See doc/README.arm-relocation for more info.

Again, I'm unhappy to see this being ARM specific. It ain't so, on
contrary - we changed ARM to do this so as ARMis in sync with other
architectures.


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
Uncontrolled power will turn even saints into savages. And we can all
be counted on to live down to our lowest impulses.
	-- Parmen, "Plato's Stepchildren", stardate 5784.3

[U-Boot] [PATCH 2/5] Added extra documentation about how the relocation address to RAM is picked for ARM.

[Christopher Harvey]

Signed-off-by: Christopher Harvey <charvey@matrox.com>
---
 doc/README.arm-relocation |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/doc/README.arm-relocation b/doc/README.arm-relocation
index 5a9a2fb..954627d 100644
--- a/doc/README.arm-relocation
+++ b/doc/README.arm-relocation
@@ -22,7 +22,7 @@ At cpu level: modify linker file and add a relocation and fixup loop
 At board level:
 
 	dram_init(): bd pointer is now at this point not accessible, so only
-	detect the real dramsize, and store it in gd->ram_size. Bst detected
+	detect the real dramsize, and store it in gd->ram_size. Best detected
 	with get_ram_size().
 
 TODO:	move also dram initialization there on boards where it is possible.
@@ -38,6 +38,13 @@ At lib level:
 
 Boards which are not fixed to support relocation will be REMOVED!
 
+The code that picks the location in RAM for ARM can be found in the 
+arch/arm/lib/board.c file under the board_init_f function. 
+The postfix _f means executed from flash, and _r means from RAM. 
+The new location is picked with respect to the highest RAM address, and the
+exact final value depends heavily on compile time options. The source is the
+best documentation here. 
+
 -----------------------------------------------------------------------------
 
 For boards which boot from nand_spl, it is possible to save one copy
-- 
1.7.3.4

[U-Boot] [PATCH 2/5] Added extra documentation about how the relocation address to RAM is picked for ARM.

[Wolfgang Denk]

Dear Christopher Harvey,

In message <20110704174348.GC3016@harvey-pc.matrox.com> you wrote:
> Signed-off-by: Christopher Harvey <charvey@matrox.com>
> ---
>  doc/README.arm-relocation |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)

Please let's stop with ARM specific documatation of things that are
considered generic.

> +The code that picks the location in RAM for ARM can be found in the 
> +arch/arm/lib/board.c file under the board_init_f function. 

under the function? Who dropped that code? :-)  s/under/in/

Also delete "for ARM", and s/arm/<arch>/.  This is not ARM specific.

> +The postfix _f means executed from flash, and _r means from RAM. 

Maybe we whould rather write "... from NOR flash" to indicate that we
actually mean ROM.

> +The new location is picked with respect to the highest RAM address, and the
> +exact final value depends heavily on compile time options. The source is the
> +best documentation here. 

This should be changed, i. e. better documentation should be provided
:-)

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
"Obviously, a major malfunction has occurred."
              -- Steve Nesbitt, voice of Mission Control, January 28,
                 1986, as the shuttle Challenger exploded within view
                 of the grandstands.

[U-Boot] [PATCH 2/5] Added extra documentation about how the relocation address to RAM is picked for ARM.

[Christopher Harvey]

On Mon, Jul 04, 2011 at 09:43:24PM +0200, Wolfgang Denk wrote:
> Dear Christopher Harvey,
> 
> In message <20110704174348.GC3016@harvey-pc.matrox.com> you wrote:
> > Signed-off-by: Christopher Harvey <charvey@matrox.com>
> > ---
> >  doc/README.arm-relocation |    9 ++++++++-
> >  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> Please let's stop with ARM specific documatation of things that are
> considered generic.
> 
> > +The code that picks the location in RAM for ARM can be found in the 
> > +arch/arm/lib/board.c file under the board_init_f function. 
> 
> under the function? Who dropped that code? :-)  s/under/in/
> 
> Also delete "for ARM", and s/arm/<arch>/.  This is not ARM specific.
> 

I don't understand, I found the following snippet in 
arch/arm/lib/board.c

       --addr defined and set here--

	gd->relocaddr = addr;
	gd->start_addr_sp = addr_sp;
	gd->reloc_off = addr - _TEXT_BASE;
	debug ("relocation Offset is: %08lx\n", gd->reloc_off);
	memcpy (id, (void *)gd, sizeof (gd_t));

	relocate_code (addr_sp, id, addr);
	/* NOTREACHED - relocate_code() does not return */

Running grep -R gd->relocaddr *,
I found similar assignments for various architectures. 

 ...
 [snip]
 ...
> 
> Best regards,
> 
> Wolfgang Denk
> 

Thanks for the clarification,
-Chris

[U-Boot] [PATCH 2/5] Added extra documentation about how the relocation address to RAM is picked for ARM.

[Wolfgang Denk]

Dear Christopher Harvey,

In message <20110706205857.GB2168@harvey-pc.matrox.com> you wrote:
>
> > Also delete "for ARM", and s/arm/<arch>/.  This is not ARM specific.
> 
> I don't understand, I found the following snippet in 
> arch/arm/lib/board.c
> 
>        --addr defined and set here--
> 
> 	gd->relocaddr = addr;
> 	gd->start_addr_sp = addr_sp;
> 	gd->reloc_off = addr - _TEXT_BASE;
> 	debug ("relocation Offset is: %08lx\n", gd->reloc_off);
> 	memcpy (id, (void *)gd, sizeof (gd_t));
> 
> 	relocate_code (addr_sp, id, addr);
> 	/* NOTREACHED - relocate_code() does not return */
> 
> Running grep -R gd->relocaddr *,
> I found similar assignments for various architectures. 

Yes, that's what I said: this is not ARM specific, it is supposed to
be common code used by all architectures (except for the sad fact that
there are several non-conforming implementations).  But if we document
it, we should document the nominal state.  [If in doubt, use the
powerpc implementation as reference.]

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
"It is better to have tried and failed than to have  failed  to  try,
but the result's the same."                           - Mike Dennison

[U-Boot] [PATCH 2/5] Added extra documentation about how the relocation address to RAM is picked for ARM.

[Albert ARIBAUD]

Hi Christopher,

Le 04/07/2011 19:43, Christopher Harvey a ?crit :

> +The postfix _f means executed from flash, and _r means from RAM.

s/postfix/suffix/

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/5] Removed unused define, CONFIG_ARMV7.

[Christopher Harvey]

Signed-off-by: Christopher Harvey <charvey@matrox.com>
---
 include/configs/am3517_crane.h      |    2 +-
 include/configs/am3517_evm.h        |    2 +-
 include/configs/ca9x4_ct_vxp.h      |    2 +-
 include/configs/cm_t35.h            |    2 +-
 include/configs/devkit8000.h        |    2 +-
 include/configs/dig297.h            |    2 +-
 include/configs/igep0020.h          |    2 +-
 include/configs/igep0030.h          |    2 +-
 include/configs/omap3_beagle.h      |    2 +-
 include/configs/omap3_evm.h         |    2 +-
 include/configs/omap3_overo.h       |    2 +-
 include/configs/omap3_pandora.h     |    2 +-
 include/configs/omap3_sdp3430.h     |    2 +-
 include/configs/omap3_zoom1.h       |    2 +-
 include/configs/omap3_zoom2.h       |    2 +-
 include/configs/omap4_panda.h       |    2 +-
 include/configs/omap4_sdp4430.h     |    2 +-
 include/configs/s5p_goni.h          |    2 +-
 include/configs/s5pc210_universal.h |    2 +-
 include/configs/smdkc100.h          |    2 +-
 include/configs/smdkv310.h          |    2 +-
 21 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 09cb951..b809053 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -28,7 +28,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3_AM3517CRANE	1	/* working with CRANEBOARD */
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index 80ad342..db026c4 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -28,7 +28,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3_AM3517EVM	1	/* working with AM3517EVM */
diff --git a/include/configs/ca9x4_ct_vxp.h b/include/configs/ca9x4_ct_vxp.h
index 7f83249..fd92137 100644
--- a/include/configs/ca9x4_ct_vxp.h
+++ b/include/configs/ca9x4_ct_vxp.h
@@ -33,7 +33,7 @@
 #define CONFIG_SYS_TEXT_BASE		0x60800000
 
 /* High Level Configuration Options */
-#define CONFIG_ARMV7			1
+
 
 #define CONFIG_SYS_MEMTEST_START	0x60000000
 #define CONFIG_SYS_MEMTEST_END		0x20000000
diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
index 93a1b26..b4cec35 100644
--- a/include/configs/cm_t35.h
+++ b/include/configs/cm_t35.h
@@ -36,7 +36,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index 125c690..f97a4ed 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -32,7 +32,7 @@
 #define __CONFIG_H
 
 /* High Level Configuration Options */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/dig297.h b/include/configs/dig297.h
index 7aeb24e..ee0c6be 100644
--- a/include/configs/dig297.h
+++ b/include/configs/dig297.h
@@ -35,7 +35,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		/* which is a 34XX */
 #define CONFIG_OMAP3430		/* which is in a 3430 */
diff --git a/include/configs/igep0020.h b/include/configs/igep0020.h
index 5af9bec..1c36bc2 100644
--- a/include/configs/igep0020.h
+++ b/include/configs/igep0020.h
@@ -25,7 +25,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/igep0030.h b/include/configs/igep0030.h
index 92144af..8594b87 100644
--- a/include/configs/igep0030.h
+++ b/include/configs/igep0030.h
@@ -25,7 +25,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index 9fd80ed..aa602e4 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -31,7 +31,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
index 13a4fbf..d07c63e 100644
--- a/include/configs/omap3_evm.h
+++ b/include/configs/omap3_evm.h
@@ -36,7 +36,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/omap3_overo.h b/include/configs/omap3_overo.h
index 242b317..127ff47 100644
--- a/include/configs/omap3_overo.h
+++ b/include/configs/omap3_overo.h
@@ -23,7 +23,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/omap3_pandora.h b/include/configs/omap3_pandora.h
index 39c87a8..e8caa8c 100644
--- a/include/configs/omap3_pandora.h
+++ b/include/configs/omap3_pandora.h
@@ -26,7 +26,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/omap3_sdp3430.h b/include/configs/omap3_sdp3430.h
index 55bbcd4..2a1a13d 100644
--- a/include/configs/omap3_sdp3430.h
+++ b/include/configs/omap3_sdp3430.h
@@ -36,7 +36,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/omap3_zoom1.h b/include/configs/omap3_zoom1.h
index 9183849..beac7b0 100644
--- a/include/configs/omap3_zoom1.h
+++ b/include/configs/omap3_zoom1.h
@@ -32,7 +32,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/omap3_zoom2.h b/include/configs/omap3_zoom2.h
index 3573edf..214c13b 100644
--- a/include/configs/omap3_zoom2.h
+++ b/include/configs/omap3_zoom2.h
@@ -33,7 +33,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP34XX		1	/* which is a 34XX */
 #define CONFIG_OMAP3430		1	/* which is in a 3430 */
diff --git a/include/configs/omap4_panda.h b/include/configs/omap4_panda.h
index b4e7f41..d26f903 100644
--- a/include/configs/omap4_panda.h
+++ b/include/configs/omap4_panda.h
@@ -30,7 +30,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP44XX		1	/* which is a 44XX */
 #define CONFIG_OMAP4430		1	/* which is in a 4430 */
diff --git a/include/configs/omap4_sdp4430.h b/include/configs/omap4_sdp4430.h
index 584a52b..dee32bc 100644
--- a/include/configs/omap4_sdp4430.h
+++ b/include/configs/omap4_sdp4430.h
@@ -31,7 +31,7 @@
 /*
  * High Level Configuration Options
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_OMAP		1	/* in a TI OMAP core */
 #define CONFIG_OMAP44XX		1	/* which is a 44XX */
 #define CONFIG_OMAP4430		1	/* which is in a 4430 */
diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h
index d648ce8..5309d57 100644
--- a/include/configs/s5p_goni.h
+++ b/include/configs/s5p_goni.h
@@ -28,7 +28,7 @@
 #define __CONFIG_H
 
 /* High Level Configuration Options */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
 #define CONFIG_S5P		1	/* which is in a S5P Family */
 #define CONFIG_S5PC110		1	/* which is in a S5PC110 */
diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h
index 5915984..b410f97 100644
--- a/include/configs/s5pc210_universal.h
+++ b/include/configs/s5pc210_universal.h
@@ -30,7 +30,7 @@
  * High Level Configuration Options
  * (easy to change)
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
 #define CONFIG_S5P		1	/* which is in a S5P Family */
 #define CONFIG_S5PC210		1	/* which is in a S5PC210 */
diff --git a/include/configs/smdkc100.h b/include/configs/smdkc100.h
index 70e23b5..b2f8a00 100644
--- a/include/configs/smdkc100.h
+++ b/include/configs/smdkc100.h
@@ -32,7 +32,7 @@
  * High Level Configuration Options
  * (easy to change)
  */
-#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
+
 #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
 #define CONFIG_S5P		1	/* which is in a S5P Family */
 #define CONFIG_S5PC100		1	/* which is in a S5PC100 */
diff --git a/include/configs/smdkv310.h b/include/configs/smdkv310.h
index a7f5850..5d0806f 100644
--- a/include/configs/smdkv310.h
+++ b/include/configs/smdkv310.h
@@ -26,7 +26,7 @@
 #define __CONFIG_H
 
 /* High Level Configuration Options */
-#define CONFIG_ARMV7			1	/*This is an ARM V7 CPU core */
+
 #define CONFIG_SAMSUNG			1	/* in a SAMSUNG core */
 #define CONFIG_S5P			1	/* S5P Family */
 #define CONFIG_S5PC210			1	/* which is in a S5PC210 SoC */
-- 
1.7.3.4

[U-Boot] [PATCH 3/5] Removed unused define, CONFIG_ARMV7.

[Jason]

On Mon, Jul 04, 2011 at 01:44:26PM -0400, Christopher Harvey wrote:
> Signed-off-by: Christopher Harvey <charvey@matrox.com>
> ---
>  include/configs/am3517_crane.h      |    2 +-
>  include/configs/am3517_evm.h        |    2 +-
>  include/configs/ca9x4_ct_vxp.h      |    2 +-
>  include/configs/cm_t35.h            |    2 +-
>  include/configs/devkit8000.h        |    2 +-
>  include/configs/dig297.h            |    2 +-
>  include/configs/igep0020.h          |    2 +-
>  include/configs/igep0030.h          |    2 +-
>  include/configs/omap3_beagle.h      |    2 +-
>  include/configs/omap3_evm.h         |    2 +-
>  include/configs/omap3_overo.h       |    2 +-
>  include/configs/omap3_pandora.h     |    2 +-
>  include/configs/omap3_sdp3430.h     |    2 +-
>  include/configs/omap3_zoom1.h       |    2 +-
>  include/configs/omap3_zoom2.h       |    2 +-
>  include/configs/omap4_panda.h       |    2 +-
>  include/configs/omap4_sdp4430.h     |    2 +-
>  include/configs/s5p_goni.h          |    2 +-
>  include/configs/s5pc210_universal.h |    2 +-
>  include/configs/smdkc100.h          |    2 +-
>  include/configs/smdkv310.h          |    2 +-
>  21 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
> index 09cb951..b809053 100644
> --- a/include/configs/am3517_crane.h
> +++ b/include/configs/am3517_crane.h
> @@ -28,7 +28,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +

Need the empty line that wasn't there before?  Same Q for every file in
this patch.

>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3_AM3517CRANE	1	/* working with CRANEBOARD */
> diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
> index 80ad342..db026c4 100644
> --- a/include/configs/am3517_evm.h
> +++ b/include/configs/am3517_evm.h
> @@ -28,7 +28,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3_AM3517EVM	1	/* working with AM3517EVM */
> diff --git a/include/configs/ca9x4_ct_vxp.h b/include/configs/ca9x4_ct_vxp.h
> index 7f83249..fd92137 100644
> --- a/include/configs/ca9x4_ct_vxp.h
> +++ b/include/configs/ca9x4_ct_vxp.h
> @@ -33,7 +33,7 @@
>  #define CONFIG_SYS_TEXT_BASE		0x60800000
>  
>  /* High Level Configuration Options */
> -#define CONFIG_ARMV7			1
> +
>  

If that was the only remaining high level config option, should the
comment be removed?

>  #define CONFIG_SYS_MEMTEST_START	0x60000000
>  #define CONFIG_SYS_MEMTEST_END		0x20000000
> diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
> index 93a1b26..b4cec35 100644
> --- a/include/configs/cm_t35.h
> +++ b/include/configs/cm_t35.h
> @@ -36,7 +36,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
> index 125c690..f97a4ed 100644
> --- a/include/configs/devkit8000.h
> +++ b/include/configs/devkit8000.h
> @@ -32,7 +32,7 @@
>  #define __CONFIG_H
>  
>  /* High Level Configuration Options */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/dig297.h b/include/configs/dig297.h
> index 7aeb24e..ee0c6be 100644
> --- a/include/configs/dig297.h
> +++ b/include/configs/dig297.h
> @@ -35,7 +35,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		/* which is a 34XX */
>  #define CONFIG_OMAP3430		/* which is in a 3430 */
> diff --git a/include/configs/igep0020.h b/include/configs/igep0020.h
> index 5af9bec..1c36bc2 100644
> --- a/include/configs/igep0020.h
> +++ b/include/configs/igep0020.h
> @@ -25,7 +25,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/igep0030.h b/include/configs/igep0030.h
> index 92144af..8594b87 100644
> --- a/include/configs/igep0030.h
> +++ b/include/configs/igep0030.h
> @@ -25,7 +25,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> index 9fd80ed..aa602e4 100644
> --- a/include/configs/omap3_beagle.h
> +++ b/include/configs/omap3_beagle.h
> @@ -31,7 +31,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
> index 13a4fbf..d07c63e 100644
> --- a/include/configs/omap3_evm.h
> +++ b/include/configs/omap3_evm.h
> @@ -36,7 +36,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/omap3_overo.h b/include/configs/omap3_overo.h
> index 242b317..127ff47 100644
> --- a/include/configs/omap3_overo.h
> +++ b/include/configs/omap3_overo.h
> @@ -23,7 +23,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/omap3_pandora.h b/include/configs/omap3_pandora.h
> index 39c87a8..e8caa8c 100644
> --- a/include/configs/omap3_pandora.h
> +++ b/include/configs/omap3_pandora.h
> @@ -26,7 +26,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/omap3_sdp3430.h b/include/configs/omap3_sdp3430.h
> index 55bbcd4..2a1a13d 100644
> --- a/include/configs/omap3_sdp3430.h
> +++ b/include/configs/omap3_sdp3430.h
> @@ -36,7 +36,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/omap3_zoom1.h b/include/configs/omap3_zoom1.h
> index 9183849..beac7b0 100644
> --- a/include/configs/omap3_zoom1.h
> +++ b/include/configs/omap3_zoom1.h
> @@ -32,7 +32,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/omap3_zoom2.h b/include/configs/omap3_zoom2.h
> index 3573edf..214c13b 100644
> --- a/include/configs/omap3_zoom2.h
> +++ b/include/configs/omap3_zoom2.h
> @@ -33,7 +33,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
>  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> diff --git a/include/configs/omap4_panda.h b/include/configs/omap4_panda.h
> index b4e7f41..d26f903 100644
> --- a/include/configs/omap4_panda.h
> +++ b/include/configs/omap4_panda.h
> @@ -30,7 +30,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP44XX		1	/* which is a 44XX */
>  #define CONFIG_OMAP4430		1	/* which is in a 4430 */
> diff --git a/include/configs/omap4_sdp4430.h b/include/configs/omap4_sdp4430.h
> index 584a52b..dee32bc 100644
> --- a/include/configs/omap4_sdp4430.h
> +++ b/include/configs/omap4_sdp4430.h
> @@ -31,7 +31,7 @@
>  /*
>   * High Level Configuration Options
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_OMAP		1	/* in a TI OMAP core */
>  #define CONFIG_OMAP44XX		1	/* which is a 44XX */
>  #define CONFIG_OMAP4430		1	/* which is in a 4430 */
> diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h
> index d648ce8..5309d57 100644
> --- a/include/configs/s5p_goni.h
> +++ b/include/configs/s5p_goni.h
> @@ -28,7 +28,7 @@
>  #define __CONFIG_H
>  
>  /* High Level Configuration Options */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
>  #define CONFIG_S5P		1	/* which is in a S5P Family */
>  #define CONFIG_S5PC110		1	/* which is in a S5PC110 */
> diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h
> index 5915984..b410f97 100644
> --- a/include/configs/s5pc210_universal.h
> +++ b/include/configs/s5pc210_universal.h
> @@ -30,7 +30,7 @@
>   * High Level Configuration Options
>   * (easy to change)
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
>  #define CONFIG_S5P		1	/* which is in a S5P Family */
>  #define CONFIG_S5PC210		1	/* which is in a S5PC210 */
> diff --git a/include/configs/smdkc100.h b/include/configs/smdkc100.h
> index 70e23b5..b2f8a00 100644
> --- a/include/configs/smdkc100.h
> +++ b/include/configs/smdkc100.h
> @@ -32,7 +32,7 @@
>   * High Level Configuration Options
>   * (easy to change)
>   */
> -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> +
>  #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
>  #define CONFIG_S5P		1	/* which is in a S5P Family */
>  #define CONFIG_S5PC100		1	/* which is in a S5PC100 */
> diff --git a/include/configs/smdkv310.h b/include/configs/smdkv310.h
> index a7f5850..5d0806f 100644
> --- a/include/configs/smdkv310.h
> +++ b/include/configs/smdkv310.h
> @@ -26,7 +26,7 @@
>  #define __CONFIG_H
>  
>  /* High Level Configuration Options */
> -#define CONFIG_ARMV7			1	/*This is an ARM V7 CPU core */
> +
>  #define CONFIG_SAMSUNG			1	/* in a SAMSUNG core */
>  #define CONFIG_S5P			1	/* S5P Family */
>  #define CONFIG_S5PC210			1	/* which is in a S5PC210 SoC */
> -- 
> 1.7.3.4

thx,

Jason.

[U-Boot] [PATCH 3/5] Removed unused define, CONFIG_ARMV7.

[Christopher Harvey]

On Mon, Jul 04, 2011 at 02:00:12PM -0400, Jason wrote:
> On Mon, Jul 04, 2011 at 01:44:26PM -0400, Christopher Harvey wrote:
> > Signed-off-by: Christopher Harvey <charvey@matrox.com>
> > ---
> >  include/configs/am3517_crane.h      |    2 +-
> >  include/configs/am3517_evm.h        |    2 +-
> >  include/configs/ca9x4_ct_vxp.h      |    2 +-
> >  include/configs/cm_t35.h            |    2 +-
> >  include/configs/devkit8000.h        |    2 +-
> >  include/configs/dig297.h            |    2 +-
> >  include/configs/igep0020.h          |    2 +-
> >  include/configs/igep0030.h          |    2 +-
> >  include/configs/omap3_beagle.h      |    2 +-
> >  include/configs/omap3_evm.h         |    2 +-
> >  include/configs/omap3_overo.h       |    2 +-
> >  include/configs/omap3_pandora.h     |    2 +-
> >  include/configs/omap3_sdp3430.h     |    2 +-
> >  include/configs/omap3_zoom1.h       |    2 +-
> >  include/configs/omap3_zoom2.h       |    2 +-
> >  include/configs/omap4_panda.h       |    2 +-
> >  include/configs/omap4_sdp4430.h     |    2 +-
> >  include/configs/s5p_goni.h          |    2 +-
> >  include/configs/s5pc210_universal.h |    2 +-
> >  include/configs/smdkc100.h          |    2 +-
> >  include/configs/smdkv310.h          |    2 +-
> >  21 files changed, 21 insertions(+), 21 deletions(-)
> > 
> > diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
> > index 09cb951..b809053 100644
> > --- a/include/configs/am3517_crane.h
> > +++ b/include/configs/am3517_crane.h
> > @@ -28,7 +28,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> 
> Need the empty line that wasn't there before?  Same Q for every file in
> this patch.

I generated this patch with sed actually. I could easily re-run it and
tell it to remove the newlines. Lets see if it gets
considered/accepted first.

> 
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3_AM3517CRANE	1	/* working with CRANEBOARD */
> > diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
> > index 80ad342..db026c4 100644
> > --- a/include/configs/am3517_evm.h
> > +++ b/include/configs/am3517_evm.h
> > @@ -28,7 +28,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3_AM3517EVM	1	/* working with AM3517EVM */
> > diff --git a/include/configs/ca9x4_ct_vxp.h b/include/configs/ca9x4_ct_vxp.h
> > index 7f83249..fd92137 100644
> > --- a/include/configs/ca9x4_ct_vxp.h
> > +++ b/include/configs/ca9x4_ct_vxp.h
> > @@ -33,7 +33,7 @@
> >  #define CONFIG_SYS_TEXT_BASE		0x60800000
> >  
> >  /* High Level Configuration Options */
> > -#define CONFIG_ARMV7			1
> > +
> >  
> 
> If that was the only remaining high level config option, should the
> comment be removed?

I figured maintainers would get to it eventually. 

> 
> >  #define CONFIG_SYS_MEMTEST_START	0x60000000
> >  #define CONFIG_SYS_MEMTEST_END		0x20000000
> > diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
> > index 93a1b26..b4cec35 100644
> > --- a/include/configs/cm_t35.h
> > +++ b/include/configs/cm_t35.h
> > @@ -36,7 +36,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
> > index 125c690..f97a4ed 100644
> > --- a/include/configs/devkit8000.h
> > +++ b/include/configs/devkit8000.h
> > @@ -32,7 +32,7 @@
> >  #define __CONFIG_H
> >  
> >  /* High Level Configuration Options */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/dig297.h b/include/configs/dig297.h
> > index 7aeb24e..ee0c6be 100644
> > --- a/include/configs/dig297.h
> > +++ b/include/configs/dig297.h
> > @@ -35,7 +35,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		/* which is a 34XX */
> >  #define CONFIG_OMAP3430		/* which is in a 3430 */
> > diff --git a/include/configs/igep0020.h b/include/configs/igep0020.h
> > index 5af9bec..1c36bc2 100644
> > --- a/include/configs/igep0020.h
> > +++ b/include/configs/igep0020.h
> > @@ -25,7 +25,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/igep0030.h b/include/configs/igep0030.h
> > index 92144af..8594b87 100644
> > --- a/include/configs/igep0030.h
> > +++ b/include/configs/igep0030.h
> > @@ -25,7 +25,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> > index 9fd80ed..aa602e4 100644
> > --- a/include/configs/omap3_beagle.h
> > +++ b/include/configs/omap3_beagle.h
> > @@ -31,7 +31,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
> > index 13a4fbf..d07c63e 100644
> > --- a/include/configs/omap3_evm.h
> > +++ b/include/configs/omap3_evm.h
> > @@ -36,7 +36,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/omap3_overo.h b/include/configs/omap3_overo.h
> > index 242b317..127ff47 100644
> > --- a/include/configs/omap3_overo.h
> > +++ b/include/configs/omap3_overo.h
> > @@ -23,7 +23,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/omap3_pandora.h b/include/configs/omap3_pandora.h
> > index 39c87a8..e8caa8c 100644
> > --- a/include/configs/omap3_pandora.h
> > +++ b/include/configs/omap3_pandora.h
> > @@ -26,7 +26,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/omap3_sdp3430.h b/include/configs/omap3_sdp3430.h
> > index 55bbcd4..2a1a13d 100644
> > --- a/include/configs/omap3_sdp3430.h
> > +++ b/include/configs/omap3_sdp3430.h
> > @@ -36,7 +36,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/omap3_zoom1.h b/include/configs/omap3_zoom1.h
> > index 9183849..beac7b0 100644
> > --- a/include/configs/omap3_zoom1.h
> > +++ b/include/configs/omap3_zoom1.h
> > @@ -32,7 +32,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/omap3_zoom2.h b/include/configs/omap3_zoom2.h
> > index 3573edf..214c13b 100644
> > --- a/include/configs/omap3_zoom2.h
> > +++ b/include/configs/omap3_zoom2.h
> > @@ -33,7 +33,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP34XX		1	/* which is a 34XX */
> >  #define CONFIG_OMAP3430		1	/* which is in a 3430 */
> > diff --git a/include/configs/omap4_panda.h b/include/configs/omap4_panda.h
> > index b4e7f41..d26f903 100644
> > --- a/include/configs/omap4_panda.h
> > +++ b/include/configs/omap4_panda.h
> > @@ -30,7 +30,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP44XX		1	/* which is a 44XX */
> >  #define CONFIG_OMAP4430		1	/* which is in a 4430 */
> > diff --git a/include/configs/omap4_sdp4430.h b/include/configs/omap4_sdp4430.h
> > index 584a52b..dee32bc 100644
> > --- a/include/configs/omap4_sdp4430.h
> > +++ b/include/configs/omap4_sdp4430.h
> > @@ -31,7 +31,7 @@
> >  /*
> >   * High Level Configuration Options
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_OMAP		1	/* in a TI OMAP core */
> >  #define CONFIG_OMAP44XX		1	/* which is a 44XX */
> >  #define CONFIG_OMAP4430		1	/* which is in a 4430 */
> > diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h
> > index d648ce8..5309d57 100644
> > --- a/include/configs/s5p_goni.h
> > +++ b/include/configs/s5p_goni.h
> > @@ -28,7 +28,7 @@
> >  #define __CONFIG_H
> >  
> >  /* High Level Configuration Options */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
> >  #define CONFIG_S5P		1	/* which is in a S5P Family */
> >  #define CONFIG_S5PC110		1	/* which is in a S5PC110 */
> > diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h
> > index 5915984..b410f97 100644
> > --- a/include/configs/s5pc210_universal.h
> > +++ b/include/configs/s5pc210_universal.h
> > @@ -30,7 +30,7 @@
> >   * High Level Configuration Options
> >   * (easy to change)
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
> >  #define CONFIG_S5P		1	/* which is in a S5P Family */
> >  #define CONFIG_S5PC210		1	/* which is in a S5PC210 */
> > diff --git a/include/configs/smdkc100.h b/include/configs/smdkc100.h
> > index 70e23b5..b2f8a00 100644
> > --- a/include/configs/smdkc100.h
> > +++ b/include/configs/smdkc100.h
> > @@ -32,7 +32,7 @@
> >   * High Level Configuration Options
> >   * (easy to change)
> >   */
> > -#define CONFIG_ARMV7		1	/* This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_SAMSUNG		1	/* in a SAMSUNG core */
> >  #define CONFIG_S5P		1	/* which is in a S5P Family */
> >  #define CONFIG_S5PC100		1	/* which is in a S5PC100 */
> > diff --git a/include/configs/smdkv310.h b/include/configs/smdkv310.h
> > index a7f5850..5d0806f 100644
> > --- a/include/configs/smdkv310.h
> > +++ b/include/configs/smdkv310.h
> > @@ -26,7 +26,7 @@
> >  #define __CONFIG_H
> >  
> >  /* High Level Configuration Options */
> > -#define CONFIG_ARMV7			1	/*This is an ARM V7 CPU core */
> > +
> >  #define CONFIG_SAMSUNG			1	/* in a SAMSUNG core */
> >  #define CONFIG_S5P			1	/* S5P Family */
> >  #define CONFIG_S5PC210			1	/* which is in a S5PC210 SoC */
> > -- 
> > 1.7.3.4
> 
> thx,
> 
> Jason.

thanks for the input,
-Chris

[U-Boot] [PATCH 3/5] Removed unused define, CONFIG_ARMV7.

[Wolfgang Denk]

Dear Christopher Harvey,

In message <20110704184659.GG3016@harvey-pc.matrox.com> you wrote:
>
> > Need the empty line that wasn't there before?  Same Q for every file in
> > this patch.
> 
> I generated this patch with sed actually. I could easily re-run it and
> tell it to remove the newlines. Lets see if it gets
> considered/accepted first.

Please fix this (and the other comments), and it goes in.

> > >  #define CONFIG_SYS_TEXT_BASE		0x60800000
> > >  
> > >  /* High Level Configuration Options */
> > > -#define CONFIG_ARMV7			1
> > > +
> > >  
> > 
> > If that was the only remaining high level config option, should the
> > comment be removed?
> 
> I figured maintainers would get to it eventually. 

Such "eventually" never works.  Please go through this manually and
clean up.  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
Wenn Du ein' weise Antwort verlangst, Mu?t Du vern?nftig fragen.
                                                -- Goethe, Invektiven

[U-Boot] [PATCH 4/5] Don't compile in large memory test function by default.

[Christopher Harvey]

Signed-off-by: Christopher Harvey <charvey@matrox.com>
---
 common/cmd_mem.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/common/cmd_mem.c b/common/cmd_mem.c
index a5576aa..833af66 100644
--- a/common/cmd_mem.c
+++ b/common/cmd_mem.c
@@ -610,6 +610,8 @@ int do_mem_loopw (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 }
 #endif /* CONFIG_LOOPW */
 
+#ifdef CONFIG_CMD_MTEST
+
 /*
  * Perform a memory test. A more complete alternative test can be
  * configured using CONFIG_SYS_ALT_MEMTEST. The complete test loops until
@@ -965,6 +967,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	return 0;	/* not reached */
 }
 
+#endif /* CONFIG_CMD_MTEST */
 
 /* Modify memory.
  *
@@ -1245,11 +1248,13 @@ U_BOOT_CMD(
 );
 #endif /* CONFIG_LOOPW */
 
+#ifdef CONFIG_CMD_MTEST
 U_BOOT_CMD(
 	mtest,	5,	1,	do_mem_mtest,
 	"simple RAM read/write test",
 	"[start [end [pattern [iterations]]]]"
 );
+#endif /* CONFIG_CMD_MTEST */
 
 #ifdef CONFIG_MX_CYCLIC
 U_BOOT_CMD(
-- 
1.7.3.4

[U-Boot] [PATCH 4/5] Don't compile in large memory test function by default.

[Albert ARIBAUD]

Hi Christopher,

Le 04/07/2011 19:45, Christopher Harvey a ?crit :
> Signed-off-by: Christopher Harvey<charvey@matrox.com>
> ---
>   common/cmd_mem.c |    5 +++++
>   1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/common/cmd_mem.c b/common/cmd_mem.c
> index a5576aa..833af66 100644
> --- a/common/cmd_mem.c
> +++ b/common/cmd_mem.c
> @@ -610,6 +610,8 @@ int do_mem_loopw (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   }
>   #endif /* CONFIG_LOOPW */
>
> +#ifdef CONFIG_CMD_MTEST
> +
>   /*
>    * Perform a memory test. A more complete alternative test can be
>    * configured using CONFIG_SYS_ALT_MEMTEST. The complete test loops until
> @@ -965,6 +967,7 @@ int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   	return 0;	/* not reached */
>   }
>
> +#endif /* CONFIG_CMD_MTEST */

Line spacing around ifdef/endif seems inconsistent with others in the 
file (see "#endif /* CONFIG_LOOPW */").

>   /* Modify memory.
>    *
> @@ -1245,11 +1248,13 @@ U_BOOT_CMD(
>   );
>   #endif /* CONFIG_LOOPW */
>
> +#ifdef CONFIG_CMD_MTEST
>   U_BOOT_CMD(
>   	mtest,	5,	1,	do_mem_mtest,
>   	"simple RAM read/write test",
>   	"[start [end [pattern [iterations]]]]"
>   );
> +#endif /* CONFIG_CMD_MTEST */
>
>   #ifdef CONFIG_MX_CYCLIC
>   U_BOOT_CMD(


Amicalement,
-- 
Albert.

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Christopher Harvey]

Signed-off-by: Christopher Harvey <charvey@matrox.com>
---
 arch/arm/lib/board.c |    4 ++++
 arch/arm/lib/bootm.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 169dfeb..dbb835a 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
 	post_bootmode_init();
 	post_run (NULL, POST_ROM | post_bootmode_get(0));
 #endif
+	/* 0xffffffff is used to mark is value as "unset".
+	   Hopefully there will never be this many machines. 
+	   Can't use 0 since 0 is already used as a mach-type. */
+	gd->bd->bi_arch_number = 0xffffffff; 
 
 	gd->bd->bi_baudrate = gd->baudrate;
 	/* Ram ist board specific, so move it to board code ... */
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 802e833..70b3b76 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
 		printf ("Using machid 0x%x from environment\n", machid);
 	}
 
+#ifdef DEBUG
+	if(machid==0xffffffff) {
+	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
+	}
+#endif
+
 	show_boot_progress (15);
 
 #ifdef CONFIG_OF_LIBFDT
-- 
1.7.3.4

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Jason]

On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> Signed-off-by: Christopher Harvey <charvey@matrox.com>
> ---
>  arch/arm/lib/board.c |    4 ++++
>  arch/arm/lib/bootm.c |    6 ++++++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 169dfeb..dbb835a 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
>  	post_bootmode_init();
>  	post_run (NULL, POST_ROM | post_bootmode_get(0));
>  #endif
> +	/* 0xffffffff is used to mark is value as "unset".

s/mark is/mark a/

> +	   Hopefully there will never be this many machines. 
> +	   Can't use 0 since 0 is already used as a mach-type. */
> +	gd->bd->bi_arch_number = 0xffffffff; 
>  
>  	gd->bd->bi_baudrate = gd->baudrate;
>  	/* Ram ist board specific, so move it to board code ... */
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 802e833..70b3b76 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>  		printf ("Using machid 0x%x from environment\n", machid);
>  	}
>  
> +#ifdef DEBUG
> +	if(machid==0xffffffff) {
> +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");

s/finish/start/ ;-)

Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?

> +	}
> +#endif
> +
>  	show_boot_progress (15);
>  
>  #ifdef CONFIG_OF_LIBFDT
> -- 
> 1.7.3.4

Please take comments with a grain of salt, I'm asking, not telling.  I'm
fairly new to this as well.

thx,

Jason.

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Christopher Harvey]

On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > Signed-off-by: Christopher Harvey <charvey@matrox.com>
> > ---
> >  arch/arm/lib/board.c |    4 ++++
> >  arch/arm/lib/bootm.c |    6 ++++++
> >  2 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> > index 169dfeb..dbb835a 100644
> > --- a/arch/arm/lib/board.c
> > +++ b/arch/arm/lib/board.c
> > @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
> >  	post_bootmode_init();
> >  	post_run (NULL, POST_ROM | post_bootmode_get(0));
> >  #endif
> > +	/* 0xffffffff is used to mark is value as "unset".
> 
> s/mark is/mark a/

Yes, what I meant was:
0xffffffff is used to mark a value as "unset".

> 
> > +	   Hopefully there will never be this many machines. 
> > +	   Can't use 0 since 0 is already used as a mach-type. */
> > +	gd->bd->bi_arch_number = 0xffffffff; 
> >  
> >  	gd->bd->bi_baudrate = gd->baudrate;
> >  	/* Ram ist board specific, so move it to board code ... */
> > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > index 802e833..70b3b76 100644
> > --- a/arch/arm/lib/bootm.c
> > +++ b/arch/arm/lib/bootm.c
> > @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
> >  		printf ("Using machid 0x%x from environment\n", machid);
> >  	}
> >  
> > +#ifdef DEBUG
> > +	if(machid==0xffffffff) {
> > +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
> 
> s/finish/start/ ;-)
I'll have to disagree here.  Linux will decompress and some functions
will run but it will eventually stop, hence will not finish.
> 
> Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
The compiler can't know what machid will be at runtime. Maybe a "would
you like to continue?" prompt could work.
> 
> > +	}
> > +#endif
> > +
> >  	show_boot_progress (15);
> >  
> >  #ifdef CONFIG_OF_LIBFDT
> > -- 
> > 1.7.3.4
> 
> Please take comments with a grain of salt, I'm asking, not telling.  I'm
> fairly new to this as well.
I'm happy to clarify. 
> 
> thx,
> 
> Jason.

thanks,
-Chris

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Wolfgang Denk]

Dear Christopher Harvey,

In message <20110704185554.GH3016@harvey-pc.matrox.com> you wrote:
> On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> > On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > > Signed-off-by: Christopher Harvey <charvey@matrox.com>
> > > ---
> > >  arch/arm/lib/board.c |    4 ++++
> > >  arch/arm/lib/bootm.c |    6 ++++++
> > >  2 files changed, 10 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> > > index 169dfeb..dbb835a 100644
> > > --- a/arch/arm/lib/board.c
> > > +++ b/arch/arm/lib/board.c
> > > @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
> > >  	post_bootmode_init();
> > >  	post_run (NULL, POST_ROM | post_bootmode_get(0));
> > >  #endif
> > > +	/* 0xffffffff is used to mark is value as "unset".
> > 
> > s/mark is/mark a/
> 
> Yes, what I meant was:
> 0xffffffff is used to mark a value as "unset".

But this is wrong.  It is not unset (= undefined), it is set.
More specifically, it is set to an illegal value.


> > Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
> The compiler can't know what machid will be at runtime. Maybe a "would
> you like to continue?" prompt could work.

No. Just print a warning message, and continue.

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
To know how another being, another creature feels -  that  is  impos-
sible.                  - Terry Pratchett, _The Dark Side of the Sun_

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Jason]

On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
> On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> > On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > > +	   Hopefully there will never be this many machines. 
> > > +	   Can't use 0 since 0 is already used as a mach-type. */
> > > +	gd->bd->bi_arch_number = 0xffffffff; 
> > >  
> > >  	gd->bd->bi_baudrate = gd->baudrate;
> > >  	/* Ram ist board specific, so move it to board code ... */
> > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > > index 802e833..70b3b76 100644
> > > --- a/arch/arm/lib/bootm.c
> > > +++ b/arch/arm/lib/bootm.c
> > > @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
> > >  		printf ("Using machid 0x%x from environment\n", machid);
> > >  	}
> > >  
> > > +#ifdef DEBUG
> > > +	if(machid==0xffffffff) {
> > > +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
> > 
> > s/finish/start/ ;-)
> >
> I'll have to disagree here.  Linux will decompress and some functions
> will run but it will eventually stop, hence will not finish.

On further investigation, you're right, it doesn't finish
starting/booting.  Sorry for the noise.

> > Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
> >
> The compiler can't know what machid will be at runtime. Maybe a "would
> you like to continue?" prompt could work.

Since the kernel throws a nice fat error message when the MACH_TYPE
doesn't match what it was compiled for, I don't see the point to adding
another message at the same point in the development process.

Perhaps use the constant CONFIG_MACH_TYPE, set to 0xffffffff.  Each
board config file sets it to MACH_TYPE_WHATEVER and then you could
do:

#if CONFIG_MACH_TYPE == 0xffffffff
#warning "Machine type not set!  Linux will not finish booting!"
#endif

You could use -Werror to fail on such things.  DBGFLAGS in ./config.mk
might be a good place.

If the maintainers choose to move to a menuconfig style configuration
system, this logic could be handled in there (invalid config file).
 
> > Please take comments with a grain of salt, I'm asking, not telling.  I'm
> > fairly new to this as well.
> >
> I'm happy to clarify. 

Thanks for exercising my brain before I seek out the beer and
explosives. ;-)

Jason.

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Christopher Harvey]

On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
> On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
> > On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> > > On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > > > +	   Hopefully there will never be this many machines. 
> > > > +	   Can't use 0 since 0 is already used as a mach-type. */
> > > > +	gd->bd->bi_arch_number = 0xffffffff; 
> > > >  
> > > >  	gd->bd->bi_baudrate = gd->baudrate;
> > > >  	/* Ram ist board specific, so move it to board code ... */
> > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > > > index 802e833..70b3b76 100644
> > > > --- a/arch/arm/lib/bootm.c
> > > > +++ b/arch/arm/lib/bootm.c
> > > > @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
> > > >  		printf ("Using machid 0x%x from environment\n", machid);
> > > >  	}
> > > >  
> > > > +#ifdef DEBUG
> > > > +	if(machid==0xffffffff) {
> > > > +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
> > > 
> > > s/finish/start/ ;-)
> > >
> > I'll have to disagree here.  Linux will decompress and some functions
> > will run but it will eventually stop, hence will not finish.
> 
> On further investigation, you're right, it doesn't finish
> starting/booting.  Sorry for the noise.
> 
> > > Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
> > >
> > The compiler can't know what machid will be at runtime. Maybe a "would
> > you like to continue?" prompt could work.
> 
> Since the kernel throws a nice fat error message when the MACH_TYPE
> doesn't match what it was compiled for, I don't see the point to adding
> another message at the same point in the development process.

I didn't see that message. Do you know what lines of code in the
kernel print it? Or maybe just the message itself? 
If the kernel can check the value why would it need to be passed 
in the first place?

> Perhaps use the constant CONFIG_MACH_TYPE, set to 0xffffffff.  Each
> board config file sets it to MACH_TYPE_WHATEVER and then you could
> do:
> 
> #if CONFIG_MACH_TYPE == 0xffffffff
> #warning "Machine type not set!  Linux will not finish booting!"
> #endif
> 
> You could use -Werror to fail on such things.  DBGFLAGS in ./config.mk
> might be a good place.
> 
> If the maintainers choose to move to a menuconfig style configuration
> system, this logic could be handled in there (invalid config file).

Right now CONFIG_MACH_TYPE is only used in a few boards and isn't used
in core u-boot code, so I ignored it. I would agree that perhaps
adding a CONFIG_MACH_TYPE to u-boot would be a more elegant solution
than checking to make sure that it is a valid value before boot, but
that would be another patch.

>  
> > > Please take comments with a grain of salt, I'm asking, not telling.  I'm
> > > fairly new to this as well.
> > >
> > I'm happy to clarify. 
> 
> Thanks for exercising my brain before I seek out the beer and
> explosives. ;-)
> 
> Jason.

-Chris

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Jason]

On Mon, Jul 04, 2011 at 04:32:35PM -0400, Christopher Harvey wrote:
> On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
> > On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
> > > On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
> > > > On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
> > > > > +	   Hopefully there will never be this many machines. 
> > > > > +	   Can't use 0 since 0 is already used as a mach-type. */
> > > > > +	gd->bd->bi_arch_number = 0xffffffff; 
> > > > >  
> > > > >  	gd->bd->bi_baudrate = gd->baudrate;
> > > > >  	/* Ram ist board specific, so move it to board code ... */
> > > > > diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> > > > > index 802e833..70b3b76 100644
> > > > > --- a/arch/arm/lib/bootm.c
> > > > > +++ b/arch/arm/lib/bootm.c
> > > > > @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
> > > > >  		printf ("Using machid 0x%x from environment\n", machid);
> > > > >  	}
> > > > >  
> > > > > +#ifdef DEBUG
> > > > > +	if(machid==0xffffffff) {
> > > > > +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
> > > > 
> > > > s/finish/start/ ;-)
> > > >
> > > I'll have to disagree here.  Linux will decompress and some functions
> > > will run but it will eventually stop, hence will not finish.
> > 
> > On further investigation, you're right, it doesn't finish
> > starting/booting.  Sorry for the noise.
> > 
> > > > Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
> > > >
> > > The compiler can't know what machid will be at runtime. Maybe a "would
> > > you like to continue?" prompt could work.
> > 
> > Since the kernel throws a nice fat error message when the MACH_TYPE
> > doesn't match what it was compiled for, I don't see the point to adding
> > another message at the same point in the development process.
> 
> I didn't see that message. Do you know what lines of code in the
> kernel print it? Or maybe just the message itself? 

In init/main.c
	start_kernel() calls
		setup_arch()

In arch/arm/kernel/setup.c
	setup_arch() calls
		setup_machine_tags() which calls
			dump_machine_table()

when the value in r1 doesn't match any of the mach-types the kernel was
compiled for.

> If the kernel can check the value why would it need to be passed 
> in the first place?

Because the kernel has no way of easily determining which arm board it's
running on without this feature.

hth,

Jason.

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Igor Grinberg]

On 07/05/11 00:24, Jason wrote:

> On Mon, Jul 04, 2011 at 04:32:35PM -0400, Christopher Harvey wrote:
>> On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
>>> On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
>>>> On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
>>>>> On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
>>>>>> +	   Hopefully there will never be this many machines. 
>>>>>> +	   Can't use 0 since 0 is already used as a mach-type. */
>>>>>> +	gd->bd->bi_arch_number = 0xffffffff; 
>>>>>>  
>>>>>>  	gd->bd->bi_baudrate = gd->baudrate;
>>>>>>  	/* Ram ist board specific, so move it to board code ... */
>>>>>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>>>>>> index 802e833..70b3b76 100644
>>>>>> --- a/arch/arm/lib/bootm.c
>>>>>> +++ b/arch/arm/lib/bootm.c
>>>>>> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>>>>>>  		printf ("Using machid 0x%x from environment\n", machid);
>>>>>>  	}
>>>>>>  
>>>>>> +#ifdef DEBUG
>>>>>> +	if(machid==0xffffffff) {
>>>>>> +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
>>>>> s/finish/start/ ;-)
>>>>>
>>>> I'll have to disagree here.  Linux will decompress and some functions
>>>> will run but it will eventually stop, hence will not finish.
>>> On further investigation, you're right, it doesn't finish
>>> starting/booting.  Sorry for the noise.
>>>
>>>>> Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
>>>>>
>>>> The compiler can't know what machid will be at runtime. Maybe a "would
>>>> you like to continue?" prompt could work.
>>> Since the kernel throws a nice fat error message when the MACH_TYPE
>>> doesn't match what it was compiled for, I don't see the point to adding
>>> another message at the same point in the development process.
>> I didn't see that message. Do you know what lines of code in the
>> kernel print it? Or maybe just the message itself? 
> In init/main.c
> 	start_kernel() calls
> 		setup_arch()
>
> In arch/arm/kernel/setup.c
> 	setup_arch() calls
> 		setup_machine_tags() which calls
> 			dump_machine_table()
>
> when the value in r1 doesn't match any of the mach-types the kernel was
> compiled for.

If you don't have the earlyprintk enabled, will this still be seen?
I don't think so...

So, I think there is a point to add a warning message.

-- 
Regards,
Igor.

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Igor Grinberg]

On 07/04/11 23:32, Christopher Harvey wrote:
> On Mon, Jul 04, 2011 at 04:13:49PM -0400, Jason wrote:
>> On Mon, Jul 04, 2011 at 02:55:54PM -0400, Christopher Harvey wrote:
>>> On Mon, Jul 04, 2011 at 02:08:44PM -0400, Jason wrote:
>>>> On Mon, Jul 04, 2011 at 01:45:41PM -0400, Christopher Harvey wrote:
>>>>> +	   Hopefully there will never be this many machines. 
>>>>> +	   Can't use 0 since 0 is already used as a mach-type. */
>>>>> +	gd->bd->bi_arch_number = 0xffffffff; 
>>>>>  
>>>>>  	gd->bd->bi_baudrate = gd->baudrate;
>>>>>  	/* Ram ist board specific, so move it to board code ... */
>>>>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>>>>> index 802e833..70b3b76 100644
>>>>> --- a/arch/arm/lib/bootm.c
>>>>> +++ b/arch/arm/lib/bootm.c
>>>>> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>>>>>  		printf ("Using machid 0x%x from environment\n", machid);
>>>>>  	}
>>>>>  
>>>>> +#ifdef DEBUG
>>>>> +	if(machid==0xffffffff) {
>>>>> +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
>>>> s/finish/start/ ;-)
>>>>
>>> I'll have to disagree here.  Linux will decompress and some functions
>>> will run but it will eventually stop, hence will not finish.
>> On further investigation, you're right, it doesn't finish
>> starting/booting.  Sorry for the noise.

To remove all doubts, why not make it:
Warning: machid not set! Linux will not be able to boot properly!

>>>> Also, shouldn't the compile fail in this case (#error)?  Or, at least #warn?
>>>>
>>> The compiler can't know what machid will be at runtime. Maybe a "would
>>> you like to continue?" prompt could work.
>> Since the kernel throws a nice fat error message when the MACH_TYPE
>> doesn't match what it was compiled for, I don't see the point to adding
>> another message at the same point in the development process.
> I didn't see that message. Do you know what lines of code in the
> kernel print it? Or maybe just the message itself? 
> If the kernel can check the value why would it need to be passed 
> in the first place?
>
>> Perhaps use the constant CONFIG_MACH_TYPE, set to 0xffffffff.  Each
>> board config file sets it to MACH_TYPE_WHATEVER and then you could
>> do:
>>
>> #if CONFIG_MACH_TYPE == 0xffffffff
>> #warning "Machine type not set!  Linux will not finish booting!"
>> #endif
>>
>> You could use -Werror to fail on such things.  DBGFLAGS in ./config.mk
>> might be a good place.
>>
>> If the maintainers choose to move to a menuconfig style configuration
>> system, this logic could be handled in there (invalid config file).
> Right now CONFIG_MACH_TYPE is only used in a few boards and isn't used
> in core u-boot code, so I ignored it. I would agree that perhaps
> adding a CONFIG_MACH_TYPE to u-boot would be a more elegant solution
> than checking to make sure that it is a valid value before boot, but
> that would be another patch.

There is a patch ("arm: add CONFIG_MACH_TYPE option and documentation") pending
that adds CONFIG_MACH_TYPE (in case Jason missed it) as the formal config option.
But, again, it can be _runtime_ configurable, so you _can't_ fail the compilation.


-- 
Regards,
Igor.

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Wolfgang Denk]

Dear Christopher Harvey,

In message <20110704174541.GF3016@harvey-pc.matrox.com> you wrote:
> Signed-off-by: Christopher Harvey <charvey@matrox.com>
> ---
>  arch/arm/lib/board.c |    4 ++++
>  arch/arm/lib/bootm.c |    6 ++++++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 169dfeb..dbb835a 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
>  	post_bootmode_init();
>  	post_run (NULL, POST_ROM | post_bootmode_get(0));
>  #endif
> +	/* 0xffffffff is used to mark is value as "unset".
> +	   Hopefully there will never be this many machines. 
> +	   Can't use 0 since 0 is already used as a mach-type. */
> +	gd->bd->bi_arch_number = 0xffffffff; 

Incorrect multiline comment style.

Instead of using hardwired magic numbers, please use a #define'd
constant (add to arch/arm/include/asm/u-boot.h close to where
bi_arch_number gets defined).

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
[Braddock:] Mr. Churchill, you are drunk.
[Churchill:] And you madam, are ugly.  But I shall be sober tomorrow.

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Igor Grinberg]

On 07/04/11 20:45, Christopher Harvey wrote:

> Signed-off-by: Christopher Harvey <charvey@matrox.com>
> ---
>  arch/arm/lib/board.c |    4 ++++
>  arch/arm/lib/bootm.c |    6 ++++++
>  2 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> index 169dfeb..dbb835a 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -404,6 +404,10 @@ void board_init_f (ulong bootflag)
>  	post_bootmode_init();
>  	post_run (NULL, POST_ROM | post_bootmode_get(0));
>  #endif
> +	/* 0xffffffff is used to mark is value as "unset".
> +	   Hopefully there will never be this many machines. 
> +	   Can't use 0 since 0 is already used as a mach-type. */
> +	gd->bd->bi_arch_number = 0xffffffff; 
>  
>  	gd->bd->bi_baudrate = gd->baudrate;
>  	/* Ram ist board specific, so move it to board code ... */
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index 802e833..70b3b76 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -113,6 +113,12 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>  		printf ("Using machid 0x%x from environment\n", machid);
>  	}
>  
> +#ifdef DEBUG
> +	if(machid==0xffffffff) {

this one lacks some white spaces:
if (machid == 0xffffffff) {
and I agree with Wolfgang, the illegal value should be a self describing define instead.

> +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
> +	}
> +#endif

Is there a reason to close this in ifdef DEBUG? and also use debug()?
I would print this in any case, because machid must be set for Linux to boot
properly.
This message will not hurt anyone (just add ~50 bytes, this is not an spl code)
and if someone hacks Linux to boot in any case (without checking the machid),
then he can also hack U-Boot and remove the message (if it bothers him).


-- 
Regards,
Igor.

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Wolfgang Denk]

Dear Igor Grinberg,

In message <4E12BF5D.6080307@compulab.co.il> you wrote:
>
> > +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
> > +	}
> > +#endif
> 
> Is there a reason to close this in ifdef DEBUG? and also use debug()?
> I would print this in any case, because machid must be set for Linux to boot
> properly.
> This message will not hurt anyone (just add ~50 bytes, this is not an spl code)
> and if someone hacks Linux to boot in any case (without checking the machid),
> then he can also hack U-Boot and remove the message (if it bothers him).

I think "Warning: machid not set" should be sufficient.

Note: we should't print excessive newlines. No \n at the begin of the
string, only one at the end.

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
"We don't have to protect the environment -- the Second Coming is  at
hand."                                                   - James Watt

[U-Boot] [PATCH 5/5] Warn when the machine ID isn't passed to an ARM kernel and u-boot is compiled in debug mode. The kernel cannot boot without it.

[Igor Grinberg]

On 07/05/11 13:04, Wolfgang Denk wrote:

> Dear Igor Grinberg,
>
> In message <4E12BF5D.6080307@compulab.co.il> you wrote:
>>> +	        debug("\nWarning: machid not set! Linux will not finish booting.\n\n");
>>> +	}
>>> +#endif
>> Is there a reason to close this in ifdef DEBUG? and also use debug()?
>> I would print this in any case, because machid must be set for Linux to boot
>> properly.
>> This message will not hurt anyone (just add ~50 bytes, this is not an spl code)
>> and if someone hacks Linux to boot in any case (without checking the machid),
>> then he can also hack U-Boot and remove the message (if it bothers him).
> I think "Warning: machid not set" should be sufficient.

Agreed!

> Note: we should't print excessive newlines. No \n at the begin of the
> string, only one at the end.
>
> Best regards,
>
> Wolfgang Denk
>

-- 
Regards,
Igor.