Added documentation for CONFIG_SYS_TEXT_BASE for ARM.

PATCH Superseded Christopher Harvey 5 participants 26 messages Jul 4, 2011 – Jul 7, 2011 1 message not in the archive Arm (32-bit)

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Christopher Harvey

author

2011-07-04 17:43 UTC #1 lore raw

PATCH 1/5 Added documentation for CONFIG_SYS_TEXT_BASE for ARM.

Signed-off-by Christopher Harvey <[email protected]>

---
 README |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

README +9 -0

index a760cf3..a0090ef 100644
@@ -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)

Signature · 1 line

1.7.3.4

Wolfgang Denk

2011-07-04 19:39 UTC #2 lore raw

PATCH 1/5 Added documentation for CONFIG_SYS_TEXT_BASE for ARM.

Dear Christopher Harvey,

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

Signed-off-by Christopher Harvey <[email protected]>
---
 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.

README Configuration Settings: · line 2360

Quoted 11 lines

README +3 -0

index a760cf3..a0090ef 100644
@@ -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.

README Configuration Settings: · line 2362

Quoted 2 lines

README +2 -0

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

README Configuration Settings: · line 2364

Quoted 2 lines

README +2 -0

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

README Configuration Settings: · line 2365

Quoted 2 lines

README +2 -0

+		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

Signature · 6 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
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

Christopher Harvey

author

2011-07-04 17:43 UTC #3 lore raw

PATCH 2/5 Added extra documentation about how the relocation address to RAM is picked for ARM.

In reply to a message not in the archive

Signed-off-by Christopher Harvey <[email protected]>

---
 doc/README.arm-relocation |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

doc/README.arm-relocation +8 -1

index 5a9a2fb..954627d 100644
@@ -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

Signature · 1 line

1.7.3.4

Wolfgang Denk

2011-07-04 19:43 UTC #4 lore raw

PATCH 2/5 Added extra documentation about how the relocation address to RAM is picked for ARM.

Dear Christopher Harvey,

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

Signed-off-by Christopher Harvey <[email protected]>
---
 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.

doc/README.arm-relocation At lib level: · line 42

Quoted 2 lines

doc/README.arm-relocation +2 -0

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

Quoted 1 line

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

doc/README.arm-relocation At lib level: · line 46

Quoted 3 lines

doc/README.arm-relocation +3 -0

+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

Signature · 7 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
"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.

Christopher Harvey

author

2011-07-06 20:58 UTC #5 lore raw

PATCH 2/5 Added extra documentation about how the relocation address to RAM is picked for ARM.

Quoted 11 lines · Wolfgang Denk

Dear Christopher Harvey,
Quoted 4 lines · In message <[email protected]> you wrote:
Signed-off-by Christopher Harvey <[email protected]>
---
 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.

Quoted 7 lines

doc/README.arm-relocation At lib level: · line 42
Quoted 2 lines
doc/README.arm-relocation +2 -0
+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]
...

Quoted 5 lines

Best regards,
Wolfgang Denk

Thanks for the clarification,
-Chris

Wolfgang Denk

2011-07-06 21:29 UTC #6 lore raw

PATCH 2/5 Added extra documentation about how the relocation address to RAM is picked for ARM.

Dear Christopher Harvey,

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

Quoted 2 lines
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 */

Quoted 2 lines

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

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

Albert ARIBAUD

2011-07-07 16:10 UTC #7 lore raw

PATCH 2/5 Added extra documentation about how the relocation address to RAM is picked for ARM.

In reply to Christopher Harvey

Hi Christopher,

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

Quoted 1 line

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

s/postfix/suffix/

Amicalement,

Signature · 1 line

Albert.

Christopher Harvey

author

2011-07-04 17:44 UTC #8 lore raw

PATCH 3/5 Removed unused define, CONFIG_ARMV7.

In reply to a message not in the archive

Signed-off-by Christopher Harvey <[email protected]>

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

include/configs/am3517_crane.h +1 -1

index 09cb951..b809053 100644
@@ -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 */

include/configs/am3517_evm.h +1 -1

index 80ad342..db026c4 100644
@@ -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 */

include/configs/ca9x4_ct_vxp.h +1 -1

index 7f83249..fd92137 100644
@@ -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

include/configs/cm_t35.h +1 -1

index 93a1b26..b4cec35 100644
@@ -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 */

include/configs/devkit8000.h +1 -1

index 125c690..f97a4ed 100644
@@ -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 */

include/configs/dig297.h +1 -1

index 7aeb24e..ee0c6be 100644
@@ -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 */

include/configs/igep0020.h +1 -1

index 5af9bec..1c36bc2 100644
@@ -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 */

include/configs/igep0030.h +1 -1

index 92144af..8594b87 100644
@@ -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 */

include/configs/omap3_beagle.h +1 -1

index 9fd80ed..aa602e4 100644
@@ -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 */

include/configs/omap3_evm.h +1 -1

index 13a4fbf..d07c63e 100644
@@ -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 */

include/configs/omap3_overo.h +1 -1

index 242b317..127ff47 100644
@@ -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 */

include/configs/omap3_pandora.h +1 -1

index 39c87a8..e8caa8c 100644
@@ -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 */

include/configs/omap3_sdp3430.h +1 -1

index 55bbcd4..2a1a13d 100644
@@ -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 */

include/configs/omap3_zoom1.h +1 -1

index 9183849..beac7b0 100644
@@ -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 */

include/configs/omap3_zoom2.h +1 -1

index 3573edf..214c13b 100644
@@ -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 */

include/configs/omap4_panda.h +1 -1

index b4e7f41..d26f903 100644
@@ -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 */

include/configs/omap4_sdp4430.h +1 -1

index 584a52b..dee32bc 100644
@@ -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 */

include/configs/s5p_goni.h +1 -1

index d648ce8..5309d57 100644
@@ -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 */

include/configs/s5pc210_universal.h +1 -1

index 5915984..b410f97 100644
@@ -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 */

include/configs/smdkc100.h +1 -1

index 70e23b5..b2f8a00 100644
@@ -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 */

include/configs/smdkv310.h +1 -1

index a7f5850..5d0806f 100644
@@ -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 */

Signature · 1 line

1.7.3.4

Jason

2011-07-04 18:00 UTC #9 lore raw

PATCH 3/5 Removed unused define, CONFIG_ARMV7.

Quoted 30 lines · Christopher Harvey

Signed-off-by Christopher Harvey <[email protected]>

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

include/configs/am3517_crane.h +0 -0

index 09cb951..b809053 100644

include/configs/am3517_crane.h line 31

Quoted 5 lines

include/configs/am3517_crane.h +1 -1

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

Quoted 21 lines

include/configs/am3517_crane.h +0 -0

 #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 */

include/configs/am3517_evm.h +1 -1

index 80ad342..db026c4 100644
@@ -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 */

include/configs/ca9x4_ct_vxp.h +0 -0

index 7f83249..fd92137 100644

include/configs/ca9x4_ct_vxp.h line 37

Quoted 6 lines

include/configs/ca9x4_ct_vxp.h +1 -1

@@ -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?

Quoted 238 lines

include/configs/ca9x4_ct_vxp.h +0 -0

 #define CONFIG_SYS_MEMTEST_START	0x60000000
 #define CONFIG_SYS_MEMTEST_END		0x20000000

include/configs/cm_t35.h +1 -1

index 93a1b26..b4cec35 100644
@@ -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 */

include/configs/devkit8000.h +1 -1

index 125c690..f97a4ed 100644
@@ -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 */

include/configs/dig297.h +1 -1

index 7aeb24e..ee0c6be 100644
@@ -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 */

include/configs/igep0020.h +1 -1

index 5af9bec..1c36bc2 100644
@@ -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 */

include/configs/igep0030.h +1 -1

index 92144af..8594b87 100644
@@ -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 */

include/configs/omap3_beagle.h +1 -1

index 9fd80ed..aa602e4 100644
@@ -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 */

include/configs/omap3_evm.h +1 -1

index 13a4fbf..d07c63e 100644
@@ -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 */

include/configs/omap3_overo.h +1 -1

index 242b317..127ff47 100644
@@ -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 */

include/configs/omap3_pandora.h +1 -1

index 39c87a8..e8caa8c 100644
@@ -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 */

include/configs/omap3_sdp3430.h +1 -1

index 55bbcd4..2a1a13d 100644
@@ -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 */

include/configs/omap3_zoom1.h +1 -1

index 9183849..beac7b0 100644
@@ -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 */

include/configs/omap3_zoom2.h +1 -1

index 3573edf..214c13b 100644
@@ -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 */

include/configs/omap4_panda.h +1 -1

index b4e7f41..d26f903 100644
@@ -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 */

include/configs/omap4_sdp4430.h +1 -1

index 584a52b..dee32bc 100644
@@ -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 */

include/configs/s5p_goni.h +1 -1

index d648ce8..5309d57 100644
@@ -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 */

include/configs/s5pc210_universal.h +1 -1

index 5915984..b410f97 100644
@@ -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 */

include/configs/smdkc100.h +1 -1

index 70e23b5..b2f8a00 100644
@@ -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 */

include/configs/smdkv310.h +1 -1

index a7f5850..5d0806f 100644
@@ -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 */

Signature · 1 line

1.7.3.4

thx,

Jason.

Christopher Harvey

author

2011-07-04 18:46 UTC #10 lore raw

PATCH 3/5 Removed unused define, CONFIG_ARMV7.

Quoted 26 lines · Jason

Quoted 25 lines · Christopher Harvey

Signed-off-by Christopher Harvey <[email protected]>

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

Quoted 13 lines

include/configs/am3517_crane.h line 31
Quoted 10 lines
include/configs/am3517_crane.h +1 -1
index 09cb951..b809053 100644
@@ -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.

Quoted 24 lines

include/configs/am3517_crane.h +0 -0

 #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 */

include/configs/am3517_evm.h +1 -1

index 80ad342..db026c4 100644
@@ -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 */

include/configs/ca9x4_ct_vxp.h +0 -0

index 7f83249..fd92137 100644
@@ -33,7 +33,7 @@
 #define CONFIG_SYS_TEXT_BASE		0x60800000

Quoted 7 lines

include/configs/ca9x4_ct_vxp.h line 37
Quoted 4 lines
include/configs/ca9x4_ct_vxp.h +1 -1
 /* 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.

Quoted 242 lines

Quoted 239 lines

include/configs/ca9x4_ct_vxp.h +0 -0

 #define CONFIG_SYS_MEMTEST_START	0x60000000
 #define CONFIG_SYS_MEMTEST_END		0x20000000

include/configs/cm_t35.h +1 -1

index 93a1b26..b4cec35 100644
@@ -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 */

include/configs/devkit8000.h +1 -1

index 125c690..f97a4ed 100644
@@ -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 */

include/configs/dig297.h +1 -1

index 7aeb24e..ee0c6be 100644
@@ -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 */

include/configs/igep0020.h +1 -1

index 5af9bec..1c36bc2 100644
@@ -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 */

include/configs/igep0030.h +1 -1

index 92144af..8594b87 100644
@@ -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 */

include/configs/omap3_beagle.h +1 -1

index 9fd80ed..aa602e4 100644
@@ -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 */

include/configs/omap3_evm.h +1 -1

index 13a4fbf..d07c63e 100644
@@ -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 */

include/configs/omap3_overo.h +1 -1

index 242b317..127ff47 100644
@@ -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 */

include/configs/omap3_pandora.h +1 -1

index 39c87a8..e8caa8c 100644
@@ -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 */

include/configs/omap3_sdp3430.h +1 -1

index 55bbcd4..2a1a13d 100644
@@ -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 */

include/configs/omap3_zoom1.h +1 -1

index 9183849..beac7b0 100644
@@ -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 */

include/configs/omap3_zoom2.h +1 -1

index 3573edf..214c13b 100644
@@ -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 */

include/configs/omap4_panda.h +1 -1

index b4e7f41..d26f903 100644
@@ -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 */

include/configs/omap4_sdp4430.h +1 -1

index 584a52b..dee32bc 100644
@@ -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 */

include/configs/s5p_goni.h +1 -1

index d648ce8..5309d57 100644
@@ -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 */

include/configs/s5pc210_universal.h +1 -1

index 5915984..b410f97 100644
@@ -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 */

include/configs/smdkc100.h +1 -1

index 70e23b5..b2f8a00 100644
@@ -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 */

include/configs/smdkv310.h +1 -1

index a7f5850..5d0806f 100644
@@ -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 */

Signature · 1 line

1.7.3.4

thx,

Quoted 1 line

Jason.

thanks for the input,
-Chris

Wolfgang Denk

2011-07-04 19:47 UTC #11 lore raw

PATCH 3/5 Removed unused define, CONFIG_ARMV7.

Dear Christopher Harvey,

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

Quoted 3 lines
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.

Quoted 11 lines

Quoted 9 lines
include/configs/ca9x4_ct_vxp.h line 36
Quoted 6 lines
#define CONFIG_SYS_TEXT_BASE 0x60800000
/* High Level Configuration Options */
include/configs/ca9x4_ct_vxp.h +1 -1
-#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

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
Wenn Du ein' weise Antwort verlangst, Mu?t Du vern?nftig fragen.
                                                -- Goethe, Invektiven

Christopher Harvey

author

2011-07-04 17:45 UTC #12 lore raw

PATCH 4/5 Don't compile in large memory test function by default.

In reply to a message not in the archive

Signed-off-by Christopher Harvey <[email protected]>

---
 common/cmd_mem.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

common/cmd_mem.c +5 -0

index a5576aa..833af66 100644
@@ -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(

Signature · 1 line

1.7.3.4

Albert ARIBAUD

2011-07-07 16:13 UTC #13 lore raw

PATCH 4/5 Don't compile in large memory test function by default.

Hi Christopher,

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

Quoted 19 lines

Signed-off-by Christopher Harvey<[email protected]>

---
  common/cmd_mem.c |    5 +++++
  1 files changed, 5 insertions(+), 0 deletions(-)

common/cmd_mem.c +2 -0

index a5576aa..833af66 100644
@@ -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

common/cmd_mem.c int do_mem_mtest (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) · line 970

Quoted 4 lines

common/cmd_mem.c +1 -0

@@ -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 */").

Quoted 6 lines

common/cmd_mem.c +0 -0

  /* Modify memory.
   *

@@ -1245,11 +1248,13 @@ U_BOOT_CMD(
  );
  #endif /* CONFIG_LOOPW */

common/cmd_mem.c U_BOOT_CMD( · line 1260

Quoted 10 lines

common/cmd_mem.c +2 -0

+#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,

Signature · 1 line

Albert.

Christopher Harvey

author

2011-07-04 17:45 UTC #14 lore raw

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.

In reply to a message not in the archive

Signed-off-by Christopher Harvey <[email protected]>

---
 arch/arm/lib/board.c |    4 ++++
 arch/arm/lib/bootm.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

arch/arm/lib/board.c +4 -0

index 169dfeb..dbb835a 100644
@@ -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 ... */

arch/arm/lib/bootm.c +6 -0

index 802e833..70b3b76 100644
@@ -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

Signature · 1 line

1.7.3.4

Jason

2011-07-04 18:08 UTC #15 lore raw

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.

Quoted 11 lines · Christopher Harvey

Signed-off-by Christopher Harvey <[email protected]>
---
 arch/arm/lib/board.c |    4 ++++
 arch/arm/lib/bootm.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/lib/board.c +0 -0
index 169dfeb..dbb835a 100644

arch/arm/lib/board.c void board_init_f (ulong bootflag) · line 407

Quoted 4 lines

arch/arm/lib/board.c +1 -0

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

Quoted 11 lines

arch/arm/lib/board.c +3 -0

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

arch/arm/lib/bootm.c +0 -0

index 802e833..70b3b76 100644

arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 118

Quoted 6 lines

arch/arm/lib/bootm.c +3 -0

@@ -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?

Quoted 8 lines

arch/arm/lib/bootm.c +3 -0

+	}
+#endif
+
 	show_boot_progress (15);
 
 #ifdef CONFIG_OF_LIBFDT

Signature · 1 line

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.

Christopher Harvey

author

2011-07-04 18:55 UTC #16 lore raw

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.

Quoted 7 lines · Jason

Quoted 6 lines · Christopher Harvey
Signed-off-by Christopher Harvey <[email protected]>
---
 arch/arm/lib/board.c |    4 ++++
 arch/arm/lib/bootm.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

Quoted 11 lines

arch/arm/lib/board.c void board_init_f (ulong bootflag) · line 407
Quoted 9 lines
arch/arm/lib/board.c +1 -0
index 169dfeb..dbb835a 100644
@@ -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".

Quoted 15 lines

arch/arm/lib/board.c +3 -0

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

arch/arm/lib/bootm.c +0 -0

index 802e833..70b3b76 100644
@@ -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);
 	}

Quoted 5 lines

arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 118
Quoted 3 lines
arch/arm/lib/bootm.c +3 -0
+#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.

Quoted 2 lines

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.

Quoted 6 lines

Quoted 6 lines
arch/arm/lib/bootm.c +3 -0
+	}
+#endif
+
 	show_boot_progress (15);
 

Quoted 6 lines

Quoted 3 lines
arch/arm/lib/bootm.c +0 -0
 #ifdef CONFIG_OF_LIBFDT
Signature · 1 line
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.

Quoted 4 lines

thx,
Jason.

thanks,
-Chris

Wolfgang Denk

2011-07-04 19:56 UTC #17 lore raw

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.

Dear Christopher Harvey,

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

Quoted 7 lines · Jason
Quoted 6 lines · Christopher Harvey
Signed-off-by Christopher Harvey <[email protected]>
---
 arch/arm/lib/board.c |    4 ++++
 arch/arm/lib/bootm.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)
Quoted 11 lines
arch/arm/lib/board.c void board_init_f (ulong bootflag) · line 407
Quoted 9 lines
arch/arm/lib/board.c +1 -0
index 169dfeb..dbb835a 100644
@@ -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/

Quoted 2 lines

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.

Quoted 3 lines

Quoted 1 line
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

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

Jason

2011-07-04 20:13 UTC #18 lore raw

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.

In reply to Christopher Harvey

Quoted 16 lines · Christopher Harvey

Quoted 15 lines · Jason

Quoted 14 lines · Christopher Harvey

arch/arm/lib/board.c +3 -0

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

arch/arm/lib/bootm.c +0 -0

index 802e833..70b3b76 100644
@@ -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);
 	}

Quoted 8 lines

Quoted 6 lines
arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 118
Quoted 3 lines
arch/arm/lib/bootm.c +3 -0
+#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.

Quoted 4 lines

Quoted 2 lines
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).

Quoted 4 lines

Quoted 3 lines
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.

Christopher Harvey

author

2011-07-04 20:32 UTC #19 lore raw

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.

Quoted 29 lines · Jason

Quoted 16 lines · Christopher Harvey
Quoted 15 lines · Jason
Quoted 14 lines · Christopher Harvey
arch/arm/lib/board.c +3 -0
+	   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 ... */
arch/arm/lib/bootm.c +0 -0
index 802e833..70b3b76 100644
@@ -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);
 	}
 
Quoted 8 lines
Quoted 6 lines
arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 118
Quoted 3 lines
arch/arm/lib/bootm.c +3 -0
+#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.

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
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?

Quoted 8 lines

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

Quoted 5 lines

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.

Quoted 10 lines

Quoted 5 lines
Quoted 4 lines
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

Jason

2011-07-04 21:24 UTC #20 lore raw

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.

Quoted 35 lines · Christopher Harvey

Quoted 34 lines · Jason
Quoted 16 lines · Christopher Harvey
Quoted 15 lines · Jason
Quoted 14 lines · Christopher Harvey
arch/arm/lib/board.c +3 -0
+	   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 ... */
arch/arm/lib/bootm.c +0 -0
index 802e833..70b3b76 100644
@@ -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);
 	}
 
Quoted 8 lines
Quoted 6 lines
arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 118
Quoted 3 lines
arch/arm/lib/bootm.c +3 -0
+#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.
Quoted 4 lines
Quoted 2 lines
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.

Quoted 6 lines

Quoted 3 lines
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.

Quoted 2 lines

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.

Igor Grinberg

2011-07-05 07:21 UTC #21 lore raw

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.

Quoted 42 lines · Jason

Quoted 30 lines · Christopher Harvey
Quoted 29 lines · Jason
Quoted 16 lines · Christopher Harvey
Quoted 15 lines · Jason
Quoted 14 lines · Christopher Harvey
arch/arm/lib/board.c +3 -0
+	   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 ... */
arch/arm/lib/bootm.c +0 -0
index 802e833..70b3b76 100644
@@ -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);
 	}
 
Quoted 7 lines
Quoted 5 lines
arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 118
Quoted 3 lines
arch/arm/lib/bootm.c +3 -0
+#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.
Quoted 2 lines
Quoted 2 lines
Also, shouldn't the compile fail in this case (#error)? Or, at least #warn?
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
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()

Quoted 7 lines

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.

Signature · 2 lines

Regards,
Igor.

Igor Grinberg

2011-07-05 07:31 UTC #22 lore raw

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.

In reply to Christopher Harvey

Quoted 23 lines · Christopher Harvey

Quoted 22 lines · Jason
Quoted 21 lines · Christopher Harvey
Quoted 20 lines · Jason
Quoted 11 lines · Christopher Harvey
arch/arm/lib/board.c +3 -0
+	   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 ... */
arch/arm/lib/bootm.c +0 -0
index 802e833..70b3b76 100644
arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 118
Quoted 6 lines
arch/arm/lib/bootm.c +3 -0
@@ -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/ ;-)

Quoted 4 lines

Quoted 4 lines
Quoted 2 lines
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!

Quoted 23 lines

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
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?
Quoted 11 lines
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.

Quoted 7 lines

Quoted 2 lines
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.

Signature · 2 lines

Regards,
Igor.

Wolfgang Denk

2011-07-04 19:53 UTC #23 lore raw

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.

In reply to Christopher Harvey

Dear Christopher Harvey,

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

Signed-off-by Christopher Harvey <[email protected]>
---
 arch/arm/lib/board.c |    4 ++++
 arch/arm/lib/bootm.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)
arch/arm/lib/board.c +0 -0
index 169dfeb..dbb835a 100644

arch/arm/lib/board.c void board_init_f (ulong bootflag) · line 410

Quoted 7 lines

arch/arm/lib/board.c +4 -0

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

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

Igor Grinberg

2011-07-05 07:38 UTC #24 lore raw

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.

In reply to Christopher Harvey

Quoted 26 lines · Christopher Harvey

Signed-off-by Christopher Harvey <[email protected]>

---
 arch/arm/lib/board.c |    4 ++++
 arch/arm/lib/bootm.c |    6 ++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

arch/arm/lib/board.c +4 -0

index 169dfeb..dbb835a 100644
@@ -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 ... */

arch/arm/lib/bootm.c +0 -0

index 802e833..70b3b76 100644

arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 117

Quoted 5 lines

arch/arm/lib/bootm.c +2 -0

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

arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 120

Quoted 3 lines

arch/arm/lib/bootm.c +3 -0

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

Signature · 2 lines

Regards,
Igor.

Wolfgang Denk

2011-07-05 10:04 UTC #25 lore raw

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.

Dear Igor Grinberg,

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

arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 120
Quoted 4 lines
arch/arm/lib/bootm.c +3 -0
+	        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

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

Igor Grinberg

2011-07-05 10:46 UTC #26 lore raw

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.

Quoted 12 lines · Wolfgang Denk

Dear Igor Grinberg,
Quoted 9 lines · In message <[email protected]> you wrote:
arch/arm/lib/bootm.c int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images) · line 120
Quoted 3 lines
arch/arm/lib/bootm.c +3 -0
+	        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).

Quoted 1 line

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

Agreed!

Quoted 7 lines

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

Signature · 2 lines

Regards,
Igor.