[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

Accessing global data before relocation needs
special handling if link address != load address.
Use LINK_OFF to calculate the difference.
---
 common/cmd_nvedit.c           |    2 ++
 common/console.c              |   12 +++++++++---
 common/env_common.c           |    2 +-
 cpu/mpc83xx/cpu.c             |   10 +++++-----
 cpu/mpc83xx/cpu_init.c        |   38 ++++++++++++++++++++------------------
 cpu/mpc83xx/speed.c           |   28 +++++++++++-----------------
 drivers/serial/serial.c       |   21 +++++++++++----------
 include/linux/ctype.h         |    6 +++---
 lib_generic/crc32.c           |    7 ++++++-
 lib_generic/ctype.c           |    2 +-
 lib_generic/display_options.c |    5 +++--
 lib_generic/vsprintf.c        |    9 ++++++---
 lib_ppc/board.c               |    5 +++--
 tools/updater/ctype.c         |    2 +-
 14 files changed, 82 insertions(+), 67 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9f8d531..182c6fe 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -512,6 +512,7 @@ char *getenv (char *name)
 {
 	int i, nxt;
 
+	name = LINK_OFF(name);
 	WATCHDOG_RESET();
 
 	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
@@ -534,6 +535,7 @@ int getenv_r (char *name, char *buf, unsigned len)
 {
 	int i, nxt;
 
+	name = LINK_OFF(name);
 	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
 		int val, n;
 
diff --git a/common/console.c b/common/console.c
index dc0d13b..afda83a 100644
--- a/common/console.c
+++ b/common/console.c
@@ -346,7 +346,7 @@ void putc(const char c)
 	}
 }
 
-void puts(const char *s)
+static void printf_puts(const char *s)
 {
 #ifdef CONFIG_SILENT_CONSOLE
 	if (gd->flags & GD_FLG_SILENT)
@@ -367,12 +367,18 @@ void puts(const char *s)
 	}
 }
 
+void puts(const char *s)
+{
+	printf_puts(LINK_OFF(s));
+}
+
 void printf(const char *fmt, ...)
 {
 	va_list args;
 	uint i;
 	char printbuffer[CONFIG_SYS_PBSIZE];
 
+	fmt = LINK_OFF(fmt);
 	va_start(args, fmt);
 
 	/* For this to work, printbuffer must be larger than
@@ -382,7 +388,7 @@ void printf(const char *fmt, ...)
 	va_end(args);
 
 	/* Print the string */
-	puts(printbuffer);
+	printf_puts(printbuffer);
 }
 
 void vprintf(const char *fmt, va_list args)
@@ -396,7 +402,7 @@ void vprintf(const char *fmt, va_list args)
 	i = vsprintf(printbuffer, fmt, args);
 
 	/* Print the string */
-	puts(printbuffer);
+	printf_puts(printbuffer);
 }
 
 /* test if ctrl-c was pressed */
diff --git a/common/env_common.c b/common/env_common.c
index 439a4a9..107e711 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -153,7 +153,7 @@ static uchar env_get_char_init (int index)
 	{
 		c = env_get_char_spec(index);
 	} else {
-		c = default_environment[index];
+		c = LINK_OFF(default_environment)[index];
 	}
 
 	return (c);
diff --git a/cpu/mpc83xx/cpu.c b/cpu/mpc83xx/cpu.c
index e38a372..12a2a84 100644
--- a/cpu/mpc83xx/cpu.c
+++ b/cpu/mpc83xx/cpu.c
@@ -51,8 +51,8 @@ int checkcpu(void)
 	char buf[32];
 	int i;
 
-	const struct cpu_type {
-		char name[15];
+	static const struct cpu_type {
+		char *name;
 		u32 partid;
 	} cpu_type_list [] = {
 		CPU_TYPE_ENTRY(8311),
@@ -72,6 +72,7 @@ int checkcpu(void)
 		CPU_TYPE_ENTRY(8378),
 		CPU_TYPE_ENTRY(8379),
 	};
+	const struct cpu_type *cpu_ptr = LINK_OFF(cpu_type_list);
 
 	immr = (immap_t *)CONFIG_SYS_IMMR;
 
@@ -99,11 +100,10 @@ int checkcpu(void)
 	}
 
 	spridr = immr->sysconf.spridr;
-
 	for (i = 0; i < ARRAY_SIZE(cpu_type_list); i++)
-		if (cpu_type_list[i].partid == PARTID_NO_E(spridr)) {
+		if (cpu_ptr[i].partid == PARTID_NO_E(spridr)) {
 			puts("MPC");
-			puts(cpu_type_list[i].name);
+			puts(cpu_ptr[i].name);
 			if (IS_E_PROCESSOR(spridr))
 				puts("E");
 			if (REVID_MAJOR(spridr) >= 2)
diff --git a/cpu/mpc83xx/cpu_init.c b/cpu/mpc83xx/cpu_init.c
index 031e8d5..e1f9018 100644
--- a/cpu/mpc83xx/cpu_init.c
+++ b/cpu/mpc83xx/cpu_init.c
@@ -42,13 +42,14 @@ static void config_qe_ioports(void)
 	u8	port, pin;
 	int	dir, open_drain, assign;
 	int	i;
-
-	for (i = 0; qe_iop_conf_tab[i].assign != QE_IOP_TAB_END; i++) {
-		port		= qe_iop_conf_tab[i].port;
-		pin		= qe_iop_conf_tab[i].pin;
-		dir		= qe_iop_conf_tab[i].dir;
-		open_drain	= qe_iop_conf_tab[i].open_drain;
-		assign		= qe_iop_conf_tab[i].assign;
+	qe_iop_conf_t *qe_ptr = LINK_OFF(qe_iop_conf_tab);
+
+	for (i = 0; qe_ptr[i].assign != QE_IOP_TAB_END; i++) {
+		port		= qe_ptr[i].port;
+		pin		= qe_ptr[i].pin;
+		dir		= qe_ptr[i].dir;
+		open_drain	= qe_ptr[i].open_drain;
+		assign		= qe_ptr[i].assign;
 		qe_config_iopin(port, pin, dir, open_drain, assign);
 	}
 }
@@ -378,7 +379,7 @@ int cpu_init_r (void)
 #if defined(CONFIG_DISPLAY_AER_FULL)
 static int print_83xx_arb_event(int force)
 {
-	static char* event[] = {
+	static const char* event[] = {
 		"Address Time Out",
 		"Data Time Out",
 		"Address Only Transfer Type",
@@ -388,7 +389,7 @@ static int print_83xx_arb_event(int force)
 		"reserved",
 		"reserved"
 	};
-	static char* master[] = {
+	static const char* master[] = {
 		"e300 Core Data Transaction",
 		"reserved",
 		"e300 Core Instruction Fetch",
@@ -422,7 +423,7 @@ static int print_83xx_arb_event(int force)
 		"PCI Express 2",
 		"TDM-DMAC"
 	};
-	static char *transfer[] = {
+	static const char *transfer[] = {
 		"Address-only, Clean Block",
 		"Address-only, lwarx reservation set",
 		"Single-beat or Burst write",
@@ -473,11 +474,11 @@ static int print_83xx_arb_event(int force)
 
 	puts("Arbiter Event Status:\n");
 	printf("       Event Address: 0x%08lX\n", gd->arbiter_event_address);
-	printf("       Event Type:    0x%1x  = %s\n", etype, event[etype]);
-	printf("       Master ID:     0x%02x = %s\n", mstr_id, master[mstr_id]);
+	printf("       Event Type:    0x%1x  = %s\n", etype, LINK_OFF(event)[etype]);
+	printf("       Master ID:     0x%02x = %s\n", mstr_id, LINK_OFF(master)[mstr_id]);
 	printf("       Transfer Size: 0x%1x  = %d bytes\n", (tbst<<3) | tsize,
 				tbst ? (tsize ? tsize : 8) : 16 + 8 * tsize);
-	printf("       Transfer Type: 0x%02x = %s\n", ttype, transfer[ttype]);
+	printf("       Transfer Type: 0x%02x = %s\n", ttype, LINK_OFF(transfer)[ttype]);
 
 	return gd->arbiter_event_address;
 }
@@ -501,7 +502,7 @@ static int print_83xx_arb_event(int force)
  */
 int prt_83xx_rsr(void)
 {
-	static struct {
+	static const struct reset_type {
 		ulong mask;
 		char *desc;
 	} bits[] = {
@@ -515,7 +516,7 @@ int prt_83xx_rsr(void)
 		RSR_SRS,  "External/Internal Soft"}, {
 		RSR_HRS,  "External/Internal Hard"}
 	};
-	static int n = sizeof bits / sizeof bits[0];
+	const struct reset_type *bp = LINK_OFF(bits);
 	ulong rsr = gd->reset_status;
 	int i;
 	char *sep;
@@ -523,9 +524,10 @@ int prt_83xx_rsr(void)
 	puts("Reset Status:");
 
 	sep = " ";
-	for (i = 0; i < n; i++)
-		if (rsr & bits[i].mask) {
-			printf("%s%s", sep, bits[i].desc);
+	for (i = 0; i < ARRAY_SIZE(bits); i++)
+		if (rsr & bp[i].mask) {
+			puts(sep);
+			puts(bp[i].desc);
 			sep = ", ";
 		}
 	puts("\n");
diff --git a/cpu/mpc83xx/speed.c b/cpu/mpc83xx/speed.c
index bde7e92..001387a 100644
--- a/cpu/mpc83xx/speed.c
+++ b/cpu/mpc83xx/speed.c
@@ -98,7 +98,8 @@ int get_clocks(void)
 	u32 corecnf_tab_index;
 	u8 corepll;
 	u32 lcrr;
-
+	corecnf_t *cnf_tab;
+	int csb_r;
 	u32 csb_clk;
 #if defined(CONFIG_MPC834x) || defined(CONFIG_MPC831x) || defined(CONFIG_MPC837x)
 	u32 tsec1_clk;
@@ -413,28 +414,21 @@ int get_clocks(void)
 		/* corecnf_tab_index is too high, possibly worng value */
 		return -11;
 	}
-	switch (corecnf_tab[corecnf_tab_index].core_csb_ratio) {
-	case _byp:
-	case _x1:
-	case _1x:
+	cnf_tab = LINK_OFF(corecnf_tab);
+	csb_r = cnf_tab[corecnf_tab_index].core_csb_ratio;
+	/* Cannot use a switch stmt here, it uses linked address */
+	if (csb_r == _byp || csb_r == _x1 || csb_r == _1x)
 		core_clk = csb_clk;
-		break;
-	case _1_5x:
+	else if (csb_r == _1_5x)
 		core_clk = (3 * csb_clk) / 2;
-		break;
-	case _2x:
+	else if (csb_r == _2x)
 		core_clk = 2 * csb_clk;
-		break;
-	case _2_5x:
+	else if (csb_r == _2_5x)
 		core_clk = (5 * csb_clk) / 2;
-		break;
-	case _3x:
+	else if (csb_r == _3x)
 		core_clk = 3 * csb_clk;
-		break;
-	default:
-		/* unkown core to csb ratio */
+	else  /* unkown core to csb ratio */
 		return -13;
-	}
 
 #if defined(CONFIG_MPC8360) || defined(CONFIG_MPC832x)
 	qepmf = (im->reset.rcwl & HRCWL_CEPMF) >> HRCWL_CEPMF_SHIFT;
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
index dd5f332..4ccfb56 100644
--- a/drivers/serial/serial.c
+++ b/drivers/serial/serial.c
@@ -85,9 +85,9 @@ static NS16550_t serial_ports[4] = {
 #endif
 };
 
-#define PORT	serial_ports[port-1]
+#define PORT	(LINK_OFF(serial_ports)[port-1])
 #if defined(CONFIG_CONS_INDEX)
-#define CONSOLE	(serial_ports[CONFIG_CONS_INDEX-1])
+#define CONSOLE	(LINK_OFF(serial_ports)[CONFIG_CONS_INDEX-1])
 #endif
 
 #if defined(CONFIG_SERIAL_MULTI)
@@ -159,26 +159,27 @@ static int calc_divisor (NS16550_t port)
 int serial_init (void)
 {
 	int clock_divisor;
+	NS16550_t * sp = LINK_OFF(serial_ports);
 
 #ifdef CONFIG_NS87308
 	initialise_ns87308();
 #endif
 
 #ifdef CONFIG_SYS_NS16550_COM1
-	clock_divisor = calc_divisor(serial_ports[0]);
-	NS16550_init(serial_ports[0], clock_divisor);
+	clock_divisor = calc_divisor(sp[0]);
+	NS16550_init(sp[0], clock_divisor);
 #endif
 #ifdef CONFIG_SYS_NS16550_COM2
-	clock_divisor = calc_divisor(serial_ports[1]);
-	NS16550_init(serial_ports[1], clock_divisor);
+	clock_divisor = calc_divisor(sp[1]);
+	NS16550_init(sp[1], clock_divisor);
 #endif
 #ifdef CONFIG_SYS_NS16550_COM3
-	clock_divisor = calc_divisor(serial_ports[2]);
-	NS16550_init(serial_ports[2], clock_divisor);
+	clock_divisor = calc_divisor(sp[2]);
+	NS16550_init(sp[2], clock_divisor);
 #endif
 #ifdef CONFIG_SYS_NS16550_COM4
-	clock_divisor = calc_divisor(serial_ports[3]);
-	NS16550_init(serial_ports[3], clock_divisor);
+	clock_divisor = calc_divisor(sp[3]);
+	NS16550_init(sp[3], clock_divisor);
 #endif
 
 	return (0);
diff --git a/include/linux/ctype.h b/include/linux/ctype.h
index afa3639..5873c55 100644
--- a/include/linux/ctype.h
+++ b/include/linux/ctype.h
@@ -1,6 +1,6 @@
 #ifndef _LINUX_CTYPE_H
 #define _LINUX_CTYPE_H
-
+#include <common.h>
 /*
  * NOTE! This ctype does not handle EOF like the standard C
  * library is required to.
@@ -15,9 +15,9 @@
 #define _X	0x40	/* hex digit */
 #define _SP	0x80	/* hard space (0x20) */
 
-extern unsigned char _ctype[];
+extern const unsigned char _ctype[];
 
-#define __ismask(x) (_ctype[(int)(unsigned char)(x)])
+#define __ismask(x) (LINK_OFF(_ctype)[(int)(unsigned char)(x)])
 
 #define isalnum(c)	((__ismask(c)&(_U|_L|_D)) != 0)
 #define isalpha(c)	((__ismask(c)&(_U|_L)) != 0)
diff --git a/lib_generic/crc32.c b/lib_generic/crc32.c
index b27048c..2e11548 100644
--- a/lib_generic/crc32.c
+++ b/lib_generic/crc32.c
@@ -148,7 +148,7 @@ const uint32_t * ZEXPORT get_crc_table()
 #endif
 
 /* ========================================================================= */
-#define DO1(buf) crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8);
+#define DO1(buf) crc = crc_tab[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8);
 #define DO2(buf)  DO1(buf); DO1(buf);
 #define DO4(buf)  DO2(buf); DO2(buf);
 #define DO8(buf)  DO4(buf); DO4(buf);
@@ -156,6 +156,11 @@ const uint32_t * ZEXPORT get_crc_table()
 /* ========================================================================= */
 uint32_t ZEXPORT crc32 (uint32_t crc, const Bytef *buf, uInt len)
 {
+#ifdef LINK_OFF
+    const uint32_t *crc_tab = LINK_OFF(crc_table);
+#else
+    const uint32_t *crc_tab = crc_table;
+#endif
 #ifdef DYNAMIC_CRC_TABLE
     if (crc_table_empty)
       make_crc_table();
diff --git a/lib_generic/ctype.c b/lib_generic/ctype.c
index 6ed0468..dffe563 100644
--- a/lib_generic/ctype.c
+++ b/lib_generic/ctype.c
@@ -29,7 +29,7 @@
 
 #include <linux/ctype.h>
 
-unsigned char _ctype[] = {
+const unsigned char _ctype[] = {
 _C,_C,_C,_C,_C,_C,_C,_C,			/* 0-7 */
 _C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C,		/* 8-15 */
 _C,_C,_C,_C,_C,_C,_C,_C,			/* 16-23 */
diff --git a/lib_generic/display_options.c b/lib_generic/display_options.c
index 2dc2567..6b9fba2 100644
--- a/lib_generic/display_options.c
+++ b/lib_generic/display_options.c
@@ -31,9 +31,9 @@ int display_options (void)
 	extern char version_string[];
 
 #if defined(BUILD_TAG)
-	printf ("\n\n%s, Build: %s\n\n", version_string, BUILD_TAG);
+	printf ("\n\n%s, Build: %s\n\n", LINK_OFF(version_string), LINK_OFF(BUILD_TAG));
 #else
-	printf ("\n\n%s\n\n", version_string);
+	printf ("\n\n%s\n\n", LINK_OFF(version_string));
 #endif
 	return 0;
 }
@@ -49,6 +49,7 @@ void print_size (phys_size_t size, const char *s)
 	phys_size_t d = 1 << 30;		/* 1 GB */
 	char  c = 'G';
 
+	s = LINK_OFF(s);
 	if (size < d) {			/* try MB */
 		c = 'M';
 		d = 1 << 20;
diff --git a/lib_generic/vsprintf.c b/lib_generic/vsprintf.c
index 3d95728..b779f56 100644
--- a/lib_generic/vsprintf.c
+++ b/lib_generic/vsprintf.c
@@ -37,8 +37,8 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
 
 
 const char hex_asc[] = "0123456789abcdef";
-#define hex_asc_lo(x)   hex_asc[((x) & 0x0f)]
-#define hex_asc_hi(x)   hex_asc[((x) & 0xf0) >> 4]
+#define hex_asc_lo(x)   LINK_OFF(hex_asc)[((x) & 0x0f)]
+#define hex_asc_hi(x)   LINK_OFF(hex_asc)[((x) & 0xf0) >> 4]
 
 static inline char *pack_hex_byte(char *buf, u8 byte)
 {
@@ -303,7 +303,7 @@ static char *number(char *buf, unsigned NUM_TYPE num, int base, int size, int pr
 		int shift = 3;
 		if (base == 16) shift = 4;
 		do {
-			tmp[i++] = (digits[((unsigned char)num) & mask] | locase);
+			tmp[i++] = (LINK_OFF(digits)[((unsigned char)num) & mask] | locase);
 			num >>= shift;
 		} while (num);
 	} else { /* base 10 */
@@ -675,6 +675,7 @@ int sprintf(char * buf, const char *fmt, ...)
 	va_list args;
 	int i;
 
+	fmt = LINK_OFF(fmt);
 	va_start(args, fmt);
 	i=vsprintf(buf,fmt,args);
 	va_end(args);
@@ -684,6 +685,8 @@ int sprintf(char * buf, const char *fmt, ...)
 void panic(const char *fmt, ...)
 {
 	va_list	args;
+
+	fmt = LINK_OFF(fmt);
 	va_start(args, fmt);
 	vprintf(fmt, args);
 	putc('\n');
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index 765f97a..f9a7d30 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -384,8 +384,9 @@ void board_init_f (ulong bootflag)
 	memset ((void *) gd, 0, sizeof (gd_t));
 #endif
 
-	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
-		if ((*init_fnc_ptr) () != 0) {
+	for (init_fnc_ptr = LINK_OFF(init_sequence); *init_fnc_ptr;
+	     ++init_fnc_ptr) {
+		if (((init_fnc_t *)link_off(*init_fnc_ptr)) () != 0) {
 			hang ();
 		}
 	}
diff --git a/tools/updater/ctype.c b/tools/updater/ctype.c
index 6ed0468..dffe563 100644
--- a/tools/updater/ctype.c
+++ b/tools/updater/ctype.c
@@ -29,7 +29,7 @@
 
 #include <linux/ctype.h>
 
-unsigned char _ctype[] = {
+const unsigned char _ctype[] = {
 _C,_C,_C,_C,_C,_C,_C,_C,			/* 0-7 */
 _C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C,		/* 8-15 */
 _C,_C,_C,_C,_C,_C,_C,_C,			/* 16-23 */
-- 
1.6.4.4

[U-Boot] [PATCH 0/4] Make u-boot true PIC for ppc

[Joakim Tjernlund]

This series adds link_off(), a function to calculate
the difference between load address and link address.

Using this function it is possible to make u-boot 100%
PIC by wrapping global data accesses LINK_OFF() calls.
Plenty of examples in the code to show how to use it.

All start.S needs to be updated too and I have done
so with mpc83xx.

-DCONFIG_LINK_OFF in your platform config.mk to
make use of this function.

Needs the -ffixed-r14 patches I just sent or
you can fix the conflicts manually.

Would really appreciate some testing by 83xx owners.
Scott?

Note: this is a resend.

        Jocke

Joakim Tjernlund (4):
  ppc: Add const void *link_off(const void *addr)
  Use LINK_OFF to access global data
  Use LINK_OFF in enviroment too
  ppc: Make mpc83xx start.S relative.

 common/cmd_nvedit.c           |    2 +
 common/console.c              |   12 ++++++--
 common/env_common.c           |    2 +-
 common/env_flash.c            |   65 ++++++++++++++++++++++++----------------
 cpu/mpc83xx/cpu.c             |   10 +++---
 cpu/mpc83xx/cpu_init.c        |   38 ++++++++++++-----------
 cpu/mpc83xx/speed.c           |   28 +++++++-----------
 cpu/mpc83xx/start.S           |   35 +++++++++++++++++----
 drivers/serial/serial.c       |   21 +++++++------
 include/common.h              |    7 ++++
 include/linux/ctype.h         |    6 ++--
 lib_generic/crc32.c           |    7 ++++-
 lib_generic/ctype.c           |    2 +-
 lib_generic/display_options.c |    5 ++-
 lib_generic/vsprintf.c        |    9 ++++--
 lib_ppc/board.c               |    5 ++-
 lib_ppc/reloc.S               |   21 +++++++++++++
 tools/updater/ctype.c         |    2 +-
 18 files changed, 177 insertions(+), 100 deletions(-)

[U-Boot] [PATCH 1/4] ppc: Add const void *link_off(const void *addr)

[Joakim Tjernlund]

Calculates the offset between global data link address
and where the data is actually loaded. Add this offset
to 'addr' arg and return the result.
Useful for true PIC and when relocating code to RAM.
---
 include/common.h |    7 +++++++
 lib_ppc/reloc.S  |   21 +++++++++++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/include/common.h b/include/common.h
index 7df9afa..b37cb1d 100644
--- a/include/common.h
+++ b/include/common.h
@@ -108,6 +108,13 @@ typedef volatile unsigned char	vu_char;
 #include <asm/blackfin.h>
 #endif
 
+#ifdef CONFIG_LINK_OFF
+const void * link_off(const void * addr);
+#else
+#define link_off(addr) ((const void *) (addr))
+#endif
+#define LINK_OFF(x) ((__typeof__(&(x)[0]))link_off(x))
+
 #include <part.h>
 #include <flash.h>
 #include <image.h>
diff --git a/lib_ppc/reloc.S b/lib_ppc/reloc.S
index 50f9a83..3ec26cc 100644
--- a/lib_ppc/reloc.S
+++ b/lib_ppc/reloc.S
@@ -47,3 +47,24 @@ trap_reloc:
 	blr
 	.size	trap_reloc, .-trap_reloc
 #endif
+
+#ifdef CONFIG_LINK_OFF
+	/* Calculate the offset between global data link address
+	 * and where the data is actually loaded. Add this offset
+	 * to 'addr' arg and return the result.
+	 * Useful for true PIC and when relocating code to RAM */
+	.globl	link_off
+	.type	link_off, @function
+link_off: /* const void * link_off(const void * addr) */
+	/* In asm as we cannot use the stack */
+	mflr	r5
+	bl	_GLOBAL_OFFSET_TABLE_ at local-4
+	mflr	r4
+	mtlr	r5
+	addi	r4,r4,12 /* find first .got2 entry */
+	lwz	r5,0(r4) /* get link address */
+	subf	r4,r5,r4 /* r4 = r4 - r5 */
+	add	r3,r3,r4 /* r3 = r4 + r3 */
+	blr
+	.size	link_off, .-link_off
+#endif
-- 
1.6.4.4

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

Accessing global data before relocation needs
special handling if link address != load address.
Use LINK_OFF to calculate the difference.
---
 common/cmd_nvedit.c           |    2 ++
 common/console.c              |   12 +++++++++---
 common/env_common.c           |    2 +-
 cpu/mpc83xx/cpu.c             |   10 +++++-----
 cpu/mpc83xx/cpu_init.c        |   38 ++++++++++++++++++++------------------
 cpu/mpc83xx/speed.c           |   28 +++++++++++-----------------
 drivers/serial/serial.c       |   21 +++++++++++----------
 include/linux/ctype.h         |    6 +++---
 lib_generic/crc32.c           |    7 ++++++-
 lib_generic/ctype.c           |    2 +-
 lib_generic/display_options.c |    5 +++--
 lib_generic/vsprintf.c        |    9 ++++++---
 lib_ppc/board.c               |    5 +++--
 tools/updater/ctype.c         |    2 +-
 14 files changed, 82 insertions(+), 67 deletions(-)

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 9f8d531..182c6fe 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -512,6 +512,7 @@ char *getenv (char *name)
 {
 	int i, nxt;
 
+	name = LINK_OFF(name);
 	WATCHDOG_RESET();
 
 	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
@@ -534,6 +535,7 @@ int getenv_r (char *name, char *buf, unsigned len)
 {
 	int i, nxt;
 
+	name = LINK_OFF(name);
 	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
 		int val, n;
 
diff --git a/common/console.c b/common/console.c
index dc0d13b..afda83a 100644
--- a/common/console.c
+++ b/common/console.c
@@ -346,7 +346,7 @@ void putc(const char c)
 	}
 }
 
-void puts(const char *s)
+static void printf_puts(const char *s)
 {
 #ifdef CONFIG_SILENT_CONSOLE
 	if (gd->flags & GD_FLG_SILENT)
@@ -367,12 +367,18 @@ void puts(const char *s)
 	}
 }
 
+void puts(const char *s)
+{
+	printf_puts(LINK_OFF(s));
+}
+
 void printf(const char *fmt, ...)
 {
 	va_list args;
 	uint i;
 	char printbuffer[CONFIG_SYS_PBSIZE];
 
+	fmt = LINK_OFF(fmt);
 	va_start(args, fmt);
 
 	/* For this to work, printbuffer must be larger than
@@ -382,7 +388,7 @@ void printf(const char *fmt, ...)
 	va_end(args);
 
 	/* Print the string */
-	puts(printbuffer);
+	printf_puts(printbuffer);
 }
 
 void vprintf(const char *fmt, va_list args)
@@ -396,7 +402,7 @@ void vprintf(const char *fmt, va_list args)
 	i = vsprintf(printbuffer, fmt, args);
 
 	/* Print the string */
-	puts(printbuffer);
+	printf_puts(printbuffer);
 }
 
 /* test if ctrl-c was pressed */
diff --git a/common/env_common.c b/common/env_common.c
index 439a4a9..107e711 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -153,7 +153,7 @@ static uchar env_get_char_init (int index)
 	{
 		c = env_get_char_spec(index);
 	} else {
-		c = default_environment[index];
+		c = LINK_OFF(default_environment)[index];
 	}
 
 	return (c);
diff --git a/cpu/mpc83xx/cpu.c b/cpu/mpc83xx/cpu.c
index e38a372..12a2a84 100644
--- a/cpu/mpc83xx/cpu.c
+++ b/cpu/mpc83xx/cpu.c
@@ -51,8 +51,8 @@ int checkcpu(void)
 	char buf[32];
 	int i;
 
-	const struct cpu_type {
-		char name[15];
+	static const struct cpu_type {
+		char *name;
 		u32 partid;
 	} cpu_type_list [] = {
 		CPU_TYPE_ENTRY(8311),
@@ -72,6 +72,7 @@ int checkcpu(void)
 		CPU_TYPE_ENTRY(8378),
 		CPU_TYPE_ENTRY(8379),
 	};
+	const struct cpu_type *cpu_ptr = LINK_OFF(cpu_type_list);
 
 	immr = (immap_t *)CONFIG_SYS_IMMR;
 
@@ -99,11 +100,10 @@ int checkcpu(void)
 	}
 
 	spridr = immr->sysconf.spridr;
-
 	for (i = 0; i < ARRAY_SIZE(cpu_type_list); i++)
-		if (cpu_type_list[i].partid == PARTID_NO_E(spridr)) {
+		if (cpu_ptr[i].partid == PARTID_NO_E(spridr)) {
 			puts("MPC");
-			puts(cpu_type_list[i].name);
+			puts(cpu_ptr[i].name);
 			if (IS_E_PROCESSOR(spridr))
 				puts("E");
 			if (REVID_MAJOR(spridr) >= 2)
diff --git a/cpu/mpc83xx/cpu_init.c b/cpu/mpc83xx/cpu_init.c
index 031e8d5..e1f9018 100644
--- a/cpu/mpc83xx/cpu_init.c
+++ b/cpu/mpc83xx/cpu_init.c
@@ -42,13 +42,14 @@ static void config_qe_ioports(void)
 	u8	port, pin;
 	int	dir, open_drain, assign;
 	int	i;
-
-	for (i = 0; qe_iop_conf_tab[i].assign != QE_IOP_TAB_END; i++) {
-		port		= qe_iop_conf_tab[i].port;
-		pin		= qe_iop_conf_tab[i].pin;
-		dir		= qe_iop_conf_tab[i].dir;
-		open_drain	= qe_iop_conf_tab[i].open_drain;
-		assign		= qe_iop_conf_tab[i].assign;
+	qe_iop_conf_t *qe_ptr = LINK_OFF(qe_iop_conf_tab);
+
+	for (i = 0; qe_ptr[i].assign != QE_IOP_TAB_END; i++) {
+		port		= qe_ptr[i].port;
+		pin		= qe_ptr[i].pin;
+		dir		= qe_ptr[i].dir;
+		open_drain	= qe_ptr[i].open_drain;
+		assign		= qe_ptr[i].assign;
 		qe_config_iopin(port, pin, dir, open_drain, assign);
 	}
 }
@@ -378,7 +379,7 @@ int cpu_init_r (void)
 #if defined(CONFIG_DISPLAY_AER_FULL)
 static int print_83xx_arb_event(int force)
 {
-	static char* event[] = {
+	static const char* event[] = {
 		"Address Time Out",
 		"Data Time Out",
 		"Address Only Transfer Type",
@@ -388,7 +389,7 @@ static int print_83xx_arb_event(int force)
 		"reserved",
 		"reserved"
 	};
-	static char* master[] = {
+	static const char* master[] = {
 		"e300 Core Data Transaction",
 		"reserved",
 		"e300 Core Instruction Fetch",
@@ -422,7 +423,7 @@ static int print_83xx_arb_event(int force)
 		"PCI Express 2",
 		"TDM-DMAC"
 	};
-	static char *transfer[] = {
+	static const char *transfer[] = {
 		"Address-only, Clean Block",
 		"Address-only, lwarx reservation set",
 		"Single-beat or Burst write",
@@ -473,11 +474,11 @@ static int print_83xx_arb_event(int force)
 
 	puts("Arbiter Event Status:\n");
 	printf("       Event Address: 0x%08lX\n", gd->arbiter_event_address);
-	printf("       Event Type:    0x%1x  = %s\n", etype, event[etype]);
-	printf("       Master ID:     0x%02x = %s\n", mstr_id, master[mstr_id]);
+	printf("       Event Type:    0x%1x  = %s\n", etype, LINK_OFF(event)[etype]);
+	printf("       Master ID:     0x%02x = %s\n", mstr_id, LINK_OFF(master)[mstr_id]);
 	printf("       Transfer Size: 0x%1x  = %d bytes\n", (tbst<<3) | tsize,
 				tbst ? (tsize ? tsize : 8) : 16 + 8 * tsize);
-	printf("       Transfer Type: 0x%02x = %s\n", ttype, transfer[ttype]);
+	printf("       Transfer Type: 0x%02x = %s\n", ttype, LINK_OFF(transfer)[ttype]);
 
 	return gd->arbiter_event_address;
 }
@@ -501,7 +502,7 @@ static int print_83xx_arb_event(int force)
  */
 int prt_83xx_rsr(void)
 {
-	static struct {
+	static const struct reset_type {
 		ulong mask;
 		char *desc;
 	} bits[] = {
@@ -515,7 +516,7 @@ int prt_83xx_rsr(void)
 		RSR_SRS,  "External/Internal Soft"}, {
 		RSR_HRS,  "External/Internal Hard"}
 	};
-	static int n = sizeof bits / sizeof bits[0];
+	const struct reset_type *bp = LINK_OFF(bits);
 	ulong rsr = gd->reset_status;
 	int i;
 	char *sep;
@@ -523,9 +524,10 @@ int prt_83xx_rsr(void)
 	puts("Reset Status:");
 
 	sep = " ";
-	for (i = 0; i < n; i++)
-		if (rsr & bits[i].mask) {
-			printf("%s%s", sep, bits[i].desc);
+	for (i = 0; i < ARRAY_SIZE(bits); i++)
+		if (rsr & bp[i].mask) {
+			puts(sep);
+			puts(bp[i].desc);
 			sep = ", ";
 		}
 	puts("\n");
diff --git a/cpu/mpc83xx/speed.c b/cpu/mpc83xx/speed.c
index bde7e92..001387a 100644
--- a/cpu/mpc83xx/speed.c
+++ b/cpu/mpc83xx/speed.c
@@ -98,7 +98,8 @@ int get_clocks(void)
 	u32 corecnf_tab_index;
 	u8 corepll;
 	u32 lcrr;
-
+	corecnf_t *cnf_tab;
+	int csb_r;
 	u32 csb_clk;
 #if defined(CONFIG_MPC834x) || defined(CONFIG_MPC831x) || defined(CONFIG_MPC837x)
 	u32 tsec1_clk;
@@ -413,28 +414,21 @@ int get_clocks(void)
 		/* corecnf_tab_index is too high, possibly worng value */
 		return -11;
 	}
-	switch (corecnf_tab[corecnf_tab_index].core_csb_ratio) {
-	case _byp:
-	case _x1:
-	case _1x:
+	cnf_tab = LINK_OFF(corecnf_tab);
+	csb_r = cnf_tab[corecnf_tab_index].core_csb_ratio;
+	/* Cannot use a switch stmt here, it uses linked address */
+	if (csb_r == _byp || csb_r == _x1 || csb_r == _1x)
 		core_clk = csb_clk;
-		break;
-	case _1_5x:
+	else if (csb_r == _1_5x)
 		core_clk = (3 * csb_clk) / 2;
-		break;
-	case _2x:
+	else if (csb_r == _2x)
 		core_clk = 2 * csb_clk;
-		break;
-	case _2_5x:
+	else if (csb_r == _2_5x)
 		core_clk = (5 * csb_clk) / 2;
-		break;
-	case _3x:
+	else if (csb_r == _3x)
 		core_clk = 3 * csb_clk;
-		break;
-	default:
-		/* unkown core to csb ratio */
+	else  /* unkown core to csb ratio */
 		return -13;
-	}
 
 #if defined(CONFIG_MPC8360) || defined(CONFIG_MPC832x)
 	qepmf = (im->reset.rcwl & HRCWL_CEPMF) >> HRCWL_CEPMF_SHIFT;
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
index dd5f332..4ccfb56 100644
--- a/drivers/serial/serial.c
+++ b/drivers/serial/serial.c
@@ -85,9 +85,9 @@ static NS16550_t serial_ports[4] = {
 #endif
 };
 
-#define PORT	serial_ports[port-1]
+#define PORT	(LINK_OFF(serial_ports)[port-1])
 #if defined(CONFIG_CONS_INDEX)
-#define CONSOLE	(serial_ports[CONFIG_CONS_INDEX-1])
+#define CONSOLE	(LINK_OFF(serial_ports)[CONFIG_CONS_INDEX-1])
 #endif
 
 #if defined(CONFIG_SERIAL_MULTI)
@@ -159,26 +159,27 @@ static int calc_divisor (NS16550_t port)
 int serial_init (void)
 {
 	int clock_divisor;
+	NS16550_t * sp = LINK_OFF(serial_ports);
 
 #ifdef CONFIG_NS87308
 	initialise_ns87308();
 #endif
 
 #ifdef CONFIG_SYS_NS16550_COM1
-	clock_divisor = calc_divisor(serial_ports[0]);
-	NS16550_init(serial_ports[0], clock_divisor);
+	clock_divisor = calc_divisor(sp[0]);
+	NS16550_init(sp[0], clock_divisor);
 #endif
 #ifdef CONFIG_SYS_NS16550_COM2
-	clock_divisor = calc_divisor(serial_ports[1]);
-	NS16550_init(serial_ports[1], clock_divisor);
+	clock_divisor = calc_divisor(sp[1]);
+	NS16550_init(sp[1], clock_divisor);
 #endif
 #ifdef CONFIG_SYS_NS16550_COM3
-	clock_divisor = calc_divisor(serial_ports[2]);
-	NS16550_init(serial_ports[2], clock_divisor);
+	clock_divisor = calc_divisor(sp[2]);
+	NS16550_init(sp[2], clock_divisor);
 #endif
 #ifdef CONFIG_SYS_NS16550_COM4
-	clock_divisor = calc_divisor(serial_ports[3]);
-	NS16550_init(serial_ports[3], clock_divisor);
+	clock_divisor = calc_divisor(sp[3]);
+	NS16550_init(sp[3], clock_divisor);
 #endif
 
 	return (0);
diff --git a/include/linux/ctype.h b/include/linux/ctype.h
index afa3639..5873c55 100644
--- a/include/linux/ctype.h
+++ b/include/linux/ctype.h
@@ -1,6 +1,6 @@
 #ifndef _LINUX_CTYPE_H
 #define _LINUX_CTYPE_H
-
+#include <common.h>
 /*
  * NOTE! This ctype does not handle EOF like the standard C
  * library is required to.
@@ -15,9 +15,9 @@
 #define _X	0x40	/* hex digit */
 #define _SP	0x80	/* hard space (0x20) */
 
-extern unsigned char _ctype[];
+extern const unsigned char _ctype[];
 
-#define __ismask(x) (_ctype[(int)(unsigned char)(x)])
+#define __ismask(x) (LINK_OFF(_ctype)[(int)(unsigned char)(x)])
 
 #define isalnum(c)	((__ismask(c)&(_U|_L|_D)) != 0)
 #define isalpha(c)	((__ismask(c)&(_U|_L)) != 0)
diff --git a/lib_generic/crc32.c b/lib_generic/crc32.c
index b27048c..2e11548 100644
--- a/lib_generic/crc32.c
+++ b/lib_generic/crc32.c
@@ -148,7 +148,7 @@ const uint32_t * ZEXPORT get_crc_table()
 #endif
 
 /* ========================================================================= */
-#define DO1(buf) crc = crc_table[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8);
+#define DO1(buf) crc = crc_tab[((int)crc ^ (*buf++)) & 0xff] ^ (crc >> 8);
 #define DO2(buf)  DO1(buf); DO1(buf);
 #define DO4(buf)  DO2(buf); DO2(buf);
 #define DO8(buf)  DO4(buf); DO4(buf);
@@ -156,6 +156,11 @@ const uint32_t * ZEXPORT get_crc_table()
 /* ========================================================================= */
 uint32_t ZEXPORT crc32 (uint32_t crc, const Bytef *buf, uInt len)
 {
+#ifdef LINK_OFF
+    const uint32_t *crc_tab = LINK_OFF(crc_table);
+#else
+    const uint32_t *crc_tab = crc_table;
+#endif
 #ifdef DYNAMIC_CRC_TABLE
     if (crc_table_empty)
       make_crc_table();
diff --git a/lib_generic/ctype.c b/lib_generic/ctype.c
index 6ed0468..dffe563 100644
--- a/lib_generic/ctype.c
+++ b/lib_generic/ctype.c
@@ -29,7 +29,7 @@
 
 #include <linux/ctype.h>
 
-unsigned char _ctype[] = {
+const unsigned char _ctype[] = {
 _C,_C,_C,_C,_C,_C,_C,_C,			/* 0-7 */
 _C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C,		/* 8-15 */
 _C,_C,_C,_C,_C,_C,_C,_C,			/* 16-23 */
diff --git a/lib_generic/display_options.c b/lib_generic/display_options.c
index 2dc2567..6b9fba2 100644
--- a/lib_generic/display_options.c
+++ b/lib_generic/display_options.c
@@ -31,9 +31,9 @@ int display_options (void)
 	extern char version_string[];
 
 #if defined(BUILD_TAG)
-	printf ("\n\n%s, Build: %s\n\n", version_string, BUILD_TAG);
+	printf ("\n\n%s, Build: %s\n\n", LINK_OFF(version_string), LINK_OFF(BUILD_TAG));
 #else
-	printf ("\n\n%s\n\n", version_string);
+	printf ("\n\n%s\n\n", LINK_OFF(version_string));
 #endif
 	return 0;
 }
@@ -49,6 +49,7 @@ void print_size (phys_size_t size, const char *s)
 	phys_size_t d = 1 << 30;		/* 1 GB */
 	char  c = 'G';
 
+	s = LINK_OFF(s);
 	if (size < d) {			/* try MB */
 		c = 'M';
 		d = 1 << 20;
diff --git a/lib_generic/vsprintf.c b/lib_generic/vsprintf.c
index 3d95728..b779f56 100644
--- a/lib_generic/vsprintf.c
+++ b/lib_generic/vsprintf.c
@@ -37,8 +37,8 @@ extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
 
 
 const char hex_asc[] = "0123456789abcdef";
-#define hex_asc_lo(x)   hex_asc[((x) & 0x0f)]
-#define hex_asc_hi(x)   hex_asc[((x) & 0xf0) >> 4]
+#define hex_asc_lo(x)   LINK_OFF(hex_asc)[((x) & 0x0f)]
+#define hex_asc_hi(x)   LINK_OFF(hex_asc)[((x) & 0xf0) >> 4]
 
 static inline char *pack_hex_byte(char *buf, u8 byte)
 {
@@ -303,7 +303,7 @@ static char *number(char *buf, unsigned NUM_TYPE num, int base, int size, int pr
 		int shift = 3;
 		if (base == 16) shift = 4;
 		do {
-			tmp[i++] = (digits[((unsigned char)num) & mask] | locase);
+			tmp[i++] = (LINK_OFF(digits)[((unsigned char)num) & mask] | locase);
 			num >>= shift;
 		} while (num);
 	} else { /* base 10 */
@@ -675,6 +675,7 @@ int sprintf(char * buf, const char *fmt, ...)
 	va_list args;
 	int i;
 
+	fmt = LINK_OFF(fmt);
 	va_start(args, fmt);
 	i=vsprintf(buf,fmt,args);
 	va_end(args);
@@ -684,6 +685,8 @@ int sprintf(char * buf, const char *fmt, ...)
 void panic(const char *fmt, ...)
 {
 	va_list	args;
+
+	fmt = LINK_OFF(fmt);
 	va_start(args, fmt);
 	vprintf(fmt, args);
 	putc('\n');
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index 765f97a..f9a7d30 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -384,8 +384,9 @@ void board_init_f (ulong bootflag)
 	memset ((void *) gd, 0, sizeof (gd_t));
 #endif
 
-	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
-		if ((*init_fnc_ptr) () != 0) {
+	for (init_fnc_ptr = LINK_OFF(init_sequence); *init_fnc_ptr;
+	     ++init_fnc_ptr) {
+		if (((init_fnc_t *)link_off(*init_fnc_ptr)) () != 0) {
 			hang ();
 		}
 	}
diff --git a/tools/updater/ctype.c b/tools/updater/ctype.c
index 6ed0468..dffe563 100644
--- a/tools/updater/ctype.c
+++ b/tools/updater/ctype.c
@@ -29,7 +29,7 @@
 
 #include <linux/ctype.h>
 
-unsigned char _ctype[] = {
+const unsigned char _ctype[] = {
 _C,_C,_C,_C,_C,_C,_C,_C,			/* 0-7 */
 _C,_C|_S,_C|_S,_C|_S,_C|_S,_C|_S,_C,_C,		/* 8-15 */
 _C,_C,_C,_C,_C,_C,_C,_C,			/* 16-23 */
-- 
1.6.4.4

[U-Boot] [PATCH 3/4] Use LINK_OFF in enviroment too

[Joakim Tjernlund]

This is the most complex change. Keep this
one as a separate commit for now.
---
 common/env_flash.c |   65 +++++++++++++++++++++++++++++++--------------------
 1 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/common/env_flash.c b/common/env_flash.c
index b860c48..64882d2 100644
--- a/common/env_flash.c
+++ b/common/env_flash.c
@@ -52,27 +52,28 @@ DECLARE_GLOBAL_DATA_PTR;
 
 char * env_name_spec = "Flash";
 
+static int flash_env_swapped;
+
 #ifdef ENV_IS_EMBEDDED
 
 extern uchar environment[];
 env_t *env_ptr = (env_t *)(&environment[0]);
 
 #ifdef CMD_SAVEENV
-/* static env_t *flash_addr = (env_t *)(&environment[0]);-broken on ARM-wd-*/
-static env_t *flash_addr = (env_t *)CONFIG_ENV_ADDR;
+#define flash_addr  f_flash_addr()
 #endif
 
 #else /* ! ENV_IS_EMBEDDED */
 
 env_t *env_ptr = (env_t *)CONFIG_ENV_ADDR;
 #ifdef CMD_SAVEENV
-static env_t *flash_addr = (env_t *)CONFIG_ENV_ADDR;
+#define flash_addr f_flash_addr()
 #endif
 
 #endif /* ENV_IS_EMBEDDED */
 
 #ifdef CONFIG_ENV_ADDR_REDUND
-static env_t *flash_addr_new = (env_t *)CONFIG_ENV_ADDR_REDUND;
+#define flash_addr_new  f_flash_addr_new()
 
 /* CONFIG_ENV_ADDR is supposed to be on sector boundary */
 static ulong end_addr = CONFIG_ENV_ADDR + CONFIG_ENV_SECT_SIZE - 1;
@@ -80,10 +81,35 @@ static ulong end_addr_new = CONFIG_ENV_ADDR_REDUND + CONFIG_ENV_SECT_SIZE - 1;
 
 #define ACTIVE_FLAG   1
 #define OBSOLETE_FLAG 0
+
+static void flash_env_swap(void)
+{
+	ulong ltmp = end_addr;
+
+	flash_env_swapped = !flash_env_swapped;
+
+	end_addr = end_addr_new;
+	end_addr_new = ltmp;
+}
+
+static env_t * f_flash_addr_new(void)
+{
+	if (!*(LINK_OFF(&flash_env_swapped)))
+		return (env_t *)CONFIG_ENV_ADDR_REDUND;
+	else
+		return (env_t *)CONFIG_ENV_ADDR;
+}
 #endif /* CONFIG_ENV_ADDR_REDUND */
 
 extern uchar default_environment[];
 
+static env_t * f_flash_addr(void)
+{
+	if (!(*LINK_OFF(&flash_env_swapped)))
+		return (env_t *)CONFIG_ENV_ADDR;
+	else
+		return (env_t *)CONFIG_ENV_ADDR_REDUND;
+}
 
 uchar env_get_char_spec (int index)
 {
@@ -99,7 +125,7 @@ int  env_init(void)
 	uchar flag1 = flash_addr->flags;
 	uchar flag2 = flash_addr_new->flags;
 
-	ulong addr_default = (ulong)&default_environment[0];
+	ulong addr_default = (ulong)LINK_OFF(default_environment);
 	ulong addr1 = (ulong)&(flash_addr->data);
 	ulong addr2 = (ulong)&(flash_addr_new->data);
 
@@ -218,14 +244,7 @@ int saveenv(void)
 	}
 #endif
 	{
-		env_t * etmp = flash_addr;
-		ulong ltmp = end_addr;
-
-		flash_addr = flash_addr_new;
-		flash_addr_new = etmp;
-
-		end_addr = end_addr_new;
-		end_addr_new = ltmp;
+		flash_env_swap();
 	}
 
 	rc = 0;
@@ -245,13 +264,15 @@ Done:
 
 int  env_init(void)
 {
-	if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
-		gd->env_addr  = (ulong)&(env_ptr->data);
+	env_t *ep = *LINK_OFF(&env_ptr);
+
+	if (crc32(0, ep->data, ENV_SIZE) == ep->crc) {
+		gd->env_addr  = (ulong)&(ep->data);
 		gd->env_valid = 1;
 		return(0);
 	}
 
-	gd->env_addr  = (ulong)&default_environment[0];
+	gd->env_addr  = (ulong)LINK_OFF(default_environment);
 	gd->env_valid = 0;
 	return (0);
 }
@@ -334,16 +355,8 @@ void env_relocate_spec (void)
 {
 #if !defined(ENV_IS_EMBEDDED) || defined(CONFIG_ENV_ADDR_REDUND)
 #ifdef CONFIG_ENV_ADDR_REDUND
-	if (gd->env_addr != (ulong)&(flash_addr->data)) {
-		env_t * etmp = flash_addr;
-		ulong ltmp = end_addr;
-
-		flash_addr = flash_addr_new;
-		flash_addr_new = etmp;
-
-		end_addr = end_addr_new;
-		end_addr_new = ltmp;
-	}
+	if (gd->env_addr != (ulong)&(flash_addr->data))
+		flash_env_swap();
 
 	if (flash_addr_new->flags != OBSOLETE_FLAG &&
 	    crc32(0, flash_addr_new->data, ENV_SIZE) ==
-- 
1.6.4.4

[U-Boot] [PATCH 4/4] ppc: Make mpc83xx start.S relative.

[Joakim Tjernlund]

To use a different link address than load address,
start.S must not make any absolute accesses. This
makes it so. Use link_off(), if defined, to calculate
the difference in load and link address.
---
 cpu/mpc83xx/start.S |   35 ++++++++++++++++++++++++++++-------
 1 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/cpu/mpc83xx/start.S b/cpu/mpc83xx/start.S
index 68bb620..bc17a60 100644
--- a/cpu/mpc83xx/start.S
+++ b/cpu/mpc83xx/start.S
@@ -240,9 +240,23 @@ boot_warm: /* time t 5 */
 	/* there and deflate the flash size back to minimal size      */
 	/*------------------------------------------------------------*/
 	bl map_flash_by_law1
-	lis r4, (CONFIG_SYS_MONITOR_BASE)@h
-	ori r4, r4, (CONFIG_SYS_MONITOR_BASE)@l
-	addi r5, r4, in_flash - _start + EXC_OFF_SYS_RESET
+
+	/* Calculate address in flash and jump there */
+	bl	1f
+1:	mflr	r5   /* get current address */
+	addi	r5, r5, in_flash - 1b
+	/* Check if already inside flash address space. */
+	/* if so, do not add CONFIG_SYS_FLASH_BASE */
+	lis	r4, (CONFIG_SYS_FLASH_BASE)@h
+	ori	r4, r4, (CONFIG_SYS_FLASH_BASE)@l
+	cmplw	cr0, r5, r4
+	ble	cr0, 2f /* r5 < r4 ? */
+	lis	r6, (CONFIG_SYS_FLASH_BASE+CONFIG_SYS_FLASH_SIZE)@h
+	ori	r6, r6, (CONFIG_SYS_FLASH_BASE+CONFIG_SYS_FLASH_SIZE)@l
+	cmplw	cr0, r5, r6
+	bgt	cr0, 3f	 /* r5 > r6 ? */
+2:	add	r5,r5,r4
+3:
 	mtlr r5
 	blr
 in_flash:
@@ -831,11 +845,18 @@ relocate_code:
 	mr	r10, r5		/* Save copy of Destination Address */
 
 	GET_GOT
-	mr	r3,  r5				/* Destination Address */
-	lis	r4, CONFIG_SYS_MONITOR_BASE at h		/* Source      Address */
-	ori	r4, r4, CONFIG_SYS_MONITOR_BASE at l
+	li	r3, 0
+#ifdef CONFIG_LINK_OFF
+	bl	link_off /* const void * link_off(const void *) */
+#endif
+	lwz	r4, GOT(_start)	/* Source      Address */
+	add	r4, r4, r3
+	addi	r4, r4, -EXC_OFF_SYS_RESET
 	lwz	r5, GOT(__bss_start)
-	sub	r5, r5, r4
+	add	r5, r5, r3
+	mr	r3, r10	/* Destination Address */
+
+	sub	r5, r5, r4	/* r4 - r5 */
 	li	r6, CONFIG_SYS_CACHELINE_SIZE		/* Cache Line Size */
 
 	/*
-- 
1.6.4.4

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Mike Frysinger]

On Wednesday 30 December 2009 10:08:30 Joakim Tjernlund wrote:
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
> @@ -512,6 +512,7 @@ char *getenv (char *name)
>  {
>  	int i, nxt;
> 
> +	name = LINK_OFF(name);
>  	WATCHDOG_RESET();
> 
>  	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
> @@ -534,6 +535,7 @@ int getenv_r (char *name, char *buf, unsigned len)
>  {
>  	int i, nxt;
> 
> +	name = LINK_OFF(name);
>  	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
>  		int val, n;

you have no guarantee that getenv() is called with a const string which is in 
the .rodata section.  there's code that generates the env name in a buffer on 
the stack and gives that to getenv().  does LINK_OFF() still work then ?

> --- a/common/console.c
> +++ b/common/console.c
> @@ -346,7 +346,7 @@ void putc(const char c)
>  	}
>  }
> 
> -void puts(const char *s)
> +static void printf_puts(const char *s)
>  {
>  #ifdef CONFIG_SILENT_CONSOLE
>  	if (gd->flags & GD_FLG_SILENT)
> @@ -367,12 +367,18 @@ void puts(const char *s)
>  	}
>  }
> 
> +void puts(const char *s)
> +{
> +	printf_puts(LINK_OFF(s));
> +}

and if CONFIG_LINK_OFF isnt defined, does gcc correctly inline this ?  if not, 
i think there needs to be #ifdef CONFIG_LINK_OFF handling here.

> --- a/include/linux/ctype.h
> +++ b/include/linux/ctype.h
> 
> -extern unsigned char _ctype[];
> +extern const unsigned char _ctype[];
>
> --- a/lib_generic/ctype.c
> +++ b/lib_generic/ctype.c
> 
> -unsigned char _ctype[] = {
> +const unsigned char _ctype[] = {
>
> --- a/tools/updater/ctype.c
> +++ b/tools/updater/ctype.c
> 
> -unsigned char _ctype[] = {
> +const unsigned char _ctype[] = {

seems like these const changes should be split and pushed separately

> --- a/lib_generic/crc32.c
> +++ b/lib_generic/crc32.c
> @@ -156,6 +156,11 @@
>  */ uint32_t ZEXPORT crc32 (uint32_t crc, const Bytef *buf, uInt len) {
> +#ifdef LINK_OFF
> +    const uint32_t *crc_tab = LINK_OFF(crc_table);
> +#else
> +    const uint32_t *crc_tab = crc_table;
> +#endif

the patch 1/4 you posted always defines LINK_OFF.  it's CONFIG_LINK_OFF which 
is dynamic.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091231/cc24036e/attachment.pgp 

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

Mike Frysinger <vapier@gentoo.org> wrote on 31/12/2009 19:44:40:

Happy new year :)

>
> On Wednesday 30 December 2009 10:08:30 Joakim Tjernlund wrote:
> > --- a/common/cmd_nvedit.c
> > +++ b/common/cmd_nvedit.c
> > @@ -512,6 +512,7 @@ char *getenv (char *name)
> >  {
> >     int i, nxt;
> >
> > +   name = LINK_OFF(name);
> >     WATCHDOG_RESET();
> >
> >     for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
> > @@ -534,6 +535,7 @@ int getenv_r (char *name, char *buf, unsigned len)
> >  {
> >     int i, nxt;
> >
> > +   name = LINK_OFF(name);
> >     for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
> >        int val, n;
>
> you have no guarantee that getenv() is called with a const string which is in
> the .rodata section.  there's code that generates the env name in a buffer on
> the stack and gives that to getenv().  does LINK_OFF() still work then ?

True. LINK_OFF will not work iff link addr != load addr and name isn't a const string.
Basically if you want to use the LINK_OFF feature you have to use a const string.

>
> > --- a/common/console.c
> > +++ b/common/console.c
> > @@ -346,7 +346,7 @@ void putc(const char c)
> >     }
> >  }
> >
> > -void puts(const char *s)
> > +static void printf_puts(const char *s)
> >  {
> >  #ifdef CONFIG_SILENT_CONSOLE
> >     if (gd->flags & GD_FLG_SILENT)
> > @@ -367,12 +367,18 @@ void puts(const char *s)
> >     }
> >  }
> >
> > +void puts(const char *s)
> > +{
> > +   printf_puts(LINK_OFF(s));
> > +}
>
> and if CONFIG_LINK_OFF isnt defined, does gcc correctly inline this ?  if not,
> i think there needs to be #ifdef CONFIG_LINK_OFF handling here.

Possibly, however LINK_OFF is is a NOP if CONFIG_LINK_OFF isn't defined.

>
> > --- a/include/linux/ctype.h
> > +++ b/include/linux/ctype.h
> >
> > -extern unsigned char _ctype[];
> > +extern const unsigned char _ctype[];
> >
> > --- a/lib_generic/ctype.c
> > +++ b/lib_generic/ctype.c
> >
> > -unsigned char _ctype[] = {
> > +const unsigned char _ctype[] = {
> >
> > --- a/tools/updater/ctype.c
> > +++ b/tools/updater/ctype.c
> >
> > -unsigned char _ctype[] = {
> > +const unsigned char _ctype[] = {
>
> seems like these const changes should be split and pushed separately

Yeah, can't remember if this was required or if this was just something
I noticed while adding LINK_OFF

>
> > --- a/lib_generic/crc32.c
> > +++ b/lib_generic/crc32.c
> > @@ -156,6 +156,11 @@
> >  */ uint32_t ZEXPORT crc32 (uint32_t crc, const Bytef *buf, uInt len) {
> > +#ifdef LINK_OFF
> > +    const uint32_t *crc_tab = LINK_OFF(crc_table);
> > +#else
> > +    const uint32_t *crc_tab = crc_table;
> > +#endif
>
> the patch 1/4 you posted always defines LINK_OFF.  it's CONFIG_LINK_OFF which
> is dynamic.

Yes, but LINK_OFF will work too. I can change this though, it looks better.

The bigger question is if the LINN_OFF changes in general are acceptable to u-boot.
Any board/arch that doesn't want this functionality should not notice I think.

      Jocke

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Mike Frysinger]

On Thursday 31 December 2009 20:39:10 Joakim Tjernlund wrote:
> Mike Frysinger <vapier@gentoo.org> wrote on 31/12/2009 19:44:40:
> > On Wednesday 30 December 2009 10:08:30 Joakim Tjernlund wrote:
> > > --- a/common/cmd_nvedit.c
> > > +++ b/common/cmd_nvedit.c
> > > @@ -512,6 +512,7 @@ char *getenv (char *name)
> > >  {
> > >     int i, nxt;
> > >
> > > +   name = LINK_OFF(name);
> > >     WATCHDOG_RESET();
> > >
> > >     for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
> > > @@ -534,6 +535,7 @@ int getenv_r (char *name, char *buf, unsigned len)
> > >  {
> > >     int i, nxt;
> > >
> > > +   name = LINK_OFF(name);
> > >     for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
> > >        int val, n;
> >
> > you have no guarantee that getenv() is called with a const string which
> > is in the .rodata section.  there's code that generates the env name in a
> > buffer on the stack and gives that to getenv().  does LINK_OFF() still
> > work then ?
> 
> True. LINK_OFF will not work iff link addr != load addr and name isn't a
>  const string. Basically if you want to use the LINK_OFF feature you have
>  to use a const string.

some of the other functions in these patch sets fall into the same issue ... 
like the output functions

> > > --- a/common/console.c
> > > +++ b/common/console.c
> > > @@ -346,7 +346,7 @@ void putc(const char c)
> > >     }
> > >  }
> > >
> > > -void puts(const char *s)
> > > +static void printf_puts(const char *s)
> > >  {
> > >  #ifdef CONFIG_SILENT_CONSOLE
> > >     if (gd->flags & GD_FLG_SILENT)
> > > @@ -367,12 +367,18 @@ void puts(const char *s)
> > >     }
> > >  }
> > >
> > > +void puts(const char *s)
> > > +{
> > > +   printf_puts(LINK_OFF(s));
> > > +}
> >
> > and if CONFIG_LINK_OFF isnt defined, does gcc correctly inline this ?  if
> > not, i think there needs to be #ifdef CONFIG_LINK_OFF handling here.
> 
> Possibly, however LINK_OFF is is a NOP if CONFIG_LINK_OFF isn't defined.

yes, but that doesnt mean gcc takes care of inlining all of printf_puts() into 
the puts() and all the new call sites go to puts()

> > > --- a/lib_generic/crc32.c
> > > +++ b/lib_generic/crc32.c
> > > @@ -156,6 +156,11 @@
> > >  */ uint32_t ZEXPORT crc32 (uint32_t crc, const Bytef *buf, uInt len) {
> > > +#ifdef LINK_OFF
> > > +    const uint32_t *crc_tab = LINK_OFF(crc_table);
> > > +#else
> > > +    const uint32_t *crc_tab = crc_table;
> > > +#endif
> >
> > the patch 1/4 you posted always defines LINK_OFF.  it's CONFIG_LINK_OFF
> > which is dynamic.
> 
> Yes, but LINK_OFF will work too. I can change this though, it looks better.

my point is that it should either be checking CONFIG_LINK_OFF or always using 
LINK_OFF (since it nops when the config is off as you point out)

> The bigger question is if the LINN_OFF changes in general are acceptable to
>  u-boot. Any board/arch that doesn't want this functionality should not
>  notice I think.

i dont have any plans on wanting this, and it seems pretty invasive ... and 
easy to introduce new code that breaks PIC people but no one else really 
notices ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100101/53c5c142/attachment.pgp 

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

Mike Frysinger <vapier@gentoo.org> wrote on 01/01/2010 07:18:44:

> From: Mike Frysinger <vapier@gentoo.org>
> To: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> Cc: u-boot at lists.denx.de
> Date: 01/01/2010 07:19
> Subject: Re: [U-Boot] [PATCH 2/4] Use LINK_OFF to access global data
>
> On Thursday 31 December 2009 20:39:10 Joakim Tjernlund wrote:
> > Mike Frysinger <vapier@gentoo.org> wrote on 31/12/2009 19:44:40:
> > > On Wednesday 30 December 2009 10:08:30 Joakim Tjernlund wrote:
> > > > --- a/common/cmd_nvedit.c
> > > > +++ b/common/cmd_nvedit.c
> > > > @@ -512,6 +512,7 @@ char *getenv (char *name)
> > > >  {
> > > >     int i, nxt;
> > > >
> > > > +   name = LINK_OFF(name);
> > > >     WATCHDOG_RESET();
> > > >
> > > >     for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
> > > > @@ -534,6 +535,7 @@ int getenv_r (char *name, char *buf, unsigned len)
> > > >  {
> > > >     int i, nxt;
> > > >
> > > > +   name = LINK_OFF(name);
> > > >     for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
> > > >        int val, n;
> > >
> > > you have no guarantee that getenv() is called with a const string which
> > > is in the .rodata section.  there's code that generates the env name in a
> > > buffer on the stack and gives that to getenv().  does LINK_OFF() still
> > > work then ?
> >
> > True. LINK_OFF will not work iff link addr != load addr and name isn't a
> >  const string. Basically if you want to use the LINK_OFF feature you have
> >  to use a const string.
>
> some of the other functions in these patch sets fall into the same issue ...
> like the output functions

Yes, I wish gcc would have an option to access string literals relative the
text segment so one would not access these via the GOT.

>
> > > > --- a/common/console.c
> > > > +++ b/common/console.c
> > > > @@ -346,7 +346,7 @@ void putc(const char c)
> > > >     }
> > > >  }
> > > >
> > > > -void puts(const char *s)
> > > > +static void printf_puts(const char *s)
> > > >  {
> > > >  #ifdef CONFIG_SILENT_CONSOLE
> > > >     if (gd->flags & GD_FLG_SILENT)
> > > > @@ -367,12 +367,18 @@ void puts(const char *s)
> > > >     }
> > > >  }
> > > >
> > > > +void puts(const char *s)
> > > > +{
> > > > +   printf_puts(LINK_OFF(s));
> > > > +}
> > >
> > > and if CONFIG_LINK_OFF isnt defined, does gcc correctly inline this ?  if
> > > not, i think there needs to be #ifdef CONFIG_LINK_OFF handling here.
> >
> > Possibly, however LINK_OFF is is a NOP if CONFIG_LINK_OFF isn't defined.
>
> yes, but that doesnt mean gcc takes care of inlining all of printf_puts() into
> the puts() and all the new call sites go to puts()

Sure, gcc might not inline the current code in this case. I guess
you don't want the extra size this would incur or is there some else you
are concerned about?

>
> > > > --- a/lib_generic/crc32.c
> > > > +++ b/lib_generic/crc32.c
> > > > @@ -156,6 +156,11 @@
> > > >  */ uint32_t ZEXPORT crc32 (uint32_t crc, const Bytef *buf, uInt len) {
> > > > +#ifdef LINK_OFF
> > > > +    const uint32_t *crc_tab = LINK_OFF(crc_table);
> > > > +#else
> > > > +    const uint32_t *crc_tab = crc_table;
> > > > +#endif
> > >
> > > the patch 1/4 you posted always defines LINK_OFF.  it's CONFIG_LINK_OFF
> > > which is dynamic.
> >
> > Yes, but LINK_OFF will work too. I can change this though, it looks better.
>
> my point is that it should either be checking CONFIG_LINK_OFF or always using
> LINK_OFF (since it nops when the config is off as you point out)

Yes, I will change this in the next spin.

>
> > The bigger question is if the LINN_OFF changes in general are acceptable to
> >  u-boot. Any board/arch that doesn't want this functionality should not
> >  notice I think.
>
> i dont have any plans on wanting this, and it seems pretty invasive ... and
> easy to introduce new code that breaks PIC people but no one else really
> notices ...

Yes, it is a bit invasive hence the question if this is acceptable to u-boot.
I have been looking for other ways to do this but there isn't one sans modifying
gcc. You are sort of an gcc guy, what do you think the odds are
that gcc would add a few new options mainly useful for smaller
embedded progs like u-boot (and possible the kernel too)?

If this is accepted a few boards could be changed to use this new function by
default and then one would detect breakage earlier.

    Jocke

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Mike Frysinger]

On Friday 01 January 2010 11:29:41 Joakim Tjernlund wrote:
> Mike Frysinger <vapier@gentoo.org> wrote on 01/01/2010 07:18:44:
> > yes, but that doesnt mean gcc takes care of inlining all of printf_puts()
> > into the puts() and all the new call sites go to puts()
> 
> Sure, gcc might not inline the current code in this case. I guess
> you don't want the extra size this would incur or is there some else you
> are concerned about?

the extra size

> > i dont have any plans on wanting this, and it seems pretty invasive ...
> > and easy to introduce new code that breaks PIC people but no one else
> > really notices ...
> 
> Yes, it is a bit invasive hence the question if this is acceptable to
>  u-boot. I have been looking for other ways to do this but there isn't one
>  sans modifying gcc. You are sort of an gcc guy, what do you think the odds
>  are
> that gcc would add a few new options mainly useful for smaller
> embedded progs like u-boot (and possible the kernel too)?

i'm not really a gcc guy ... i just sometimes fake it.  i honestly dont have 
an idea here how they would respond.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100101/9c19df64/attachment.pgp 

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1262185712-11890-3-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> Accessing global data before relocation needs
> special handling if link address != load address.
> Use LINK_OFF to calculate the difference.
> ---
>  common/cmd_nvedit.c           |    2 ++
>  common/console.c              |   12 +++++++++---
>  common/env_common.c           |    2 +-
>  cpu/mpc83xx/cpu.c             |   10 +++++-----
>  cpu/mpc83xx/cpu_init.c        |   38 ++++++++++++++++++++------------------
>  cpu/mpc83xx/speed.c           |   28 +++++++++++-----------------
>  drivers/serial/serial.c       |   21 +++++++++++----------
>  include/linux/ctype.h         |    6 +++---
>  lib_generic/crc32.c           |    7 ++++++-
>  lib_generic/ctype.c           |    2 +-
>  lib_generic/display_options.c |    5 +++--
>  lib_generic/vsprintf.c        |    9 ++++++---
>  lib_ppc/board.c               |    5 +++--
>  tools/updater/ctype.c         |    2 +-
>  14 files changed, 82 insertions(+), 67 deletions(-)
...
> -void puts(const char *s)
> +static void printf_puts(const char *s)

The name "printf_puts" makes my (remaining) hair stand on end. Either
we have printf(), or we have puts(), but please let's never use
"printf_puts" - I fear there might be a "printf_getchar" coming, too.

> +void puts(const char *s)
> +{
> +	printf_puts(LINK_OFF(s));
> +}

Will such changes be needed to all functions that work on (constant?)
strings?  Or why here?

> --- a/cpu/mpc83xx/cpu.c
> +++ b/cpu/mpc83xx/cpu.c
> @@ -51,8 +51,8 @@ int checkcpu(void)
>  	char buf[32];
>  	int i;
>  
> -	const struct cpu_type {
> -		char name[15];
> +	static const struct cpu_type {
> +		char *name;

I guess we have similar constructs in many other places as well. Do
all of these need to be changed?  Really?

> -	switch (corecnf_tab[corecnf_tab_index].core_csb_ratio) {
> -	case _byp:
> -	case _x1:
> -	case _1x:
> +	cnf_tab = LINK_OFF(corecnf_tab);
> +	csb_r = cnf_tab[corecnf_tab_index].core_csb_ratio;
> +	/* Cannot use a switch stmt here, it uses linked address */

Yet another of these really, really invasive changes.  Is this
really unavoidable?

Why can we use "if" but not "switch/case" ?

> --- a/drivers/serial/serial.c
> +++ b/drivers/serial/serial.c
> @@ -85,9 +85,9 @@ static NS16550_t serial_ports[4] = {
>  #endif
>  };
>  
> -#define PORT	serial_ports[port-1]
> +#define PORT	(LINK_OFF(serial_ports)[port-1])
>  #if defined(CONFIG_CONS_INDEX)
> -#define CONSOLE	(serial_ports[CONFIG_CONS_INDEX-1])
> +#define CONSOLE	(LINK_OFF(serial_ports)[CONFIG_CONS_INDEX-1])
>  #endif

I wonder how you selected the places where such changes were needed,
and where not. There is certainly lots of similar looking code that
you don't touch.  Why not?  Because it's not needed (then why do we
need these changes here?), or because it just did not result in errors
in your tests so far (then what's the estimate for the full amount of
changes?) ???

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"355/113 -- Not the famous irrational number PI,  but  an  incredible
simulation!"

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Wolfgang Denk]

Dear Joakim Tjernlund,

in message <OF7EEC72D7.43E7C3B7-ONC125769E.00078331-C125769E.0009144C@transmode.se> you wrote:
> 
> Happy new year :)

Thanks, and same to you and everybody else.

> The bigger question is if the LINN_OFF changes in general are acceptable to u-boot.
> Any board/arch that doesn't want this functionality should not notice I think.


What's the impact?  So far it seems to me that there are pretty heavy
changes to lots of global code anyway, so we can then enable it for
all boards as well. Otherwise we just add a lot of #ifdef's and make
the code even harder to read and to understand.  Which does not mean
that I understood what you are doing :-(

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
For those who like this sort of thing, this is the sort of thing they
like.                                               - Abraham Lincoln

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 02/01/2010 19:13:29:
>
> Dear Joakim Tjernlund,
>
> In message <1262185712-11890-3-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > Accessing global data before relocation needs
> > special handling if link address != load address.
> > Use LINK_OFF to calculate the difference.
> > ---
> >  common/cmd_nvedit.c           |    2 ++
> >  common/console.c              |   12 +++++++++---
> >  common/env_common.c           |    2 +-
> >  cpu/mpc83xx/cpu.c             |   10 +++++-----
> >  cpu/mpc83xx/cpu_init.c        |   38 ++++++++++++++++++++------------------
> >  cpu/mpc83xx/speed.c           |   28 +++++++++++-----------------
> >  drivers/serial/serial.c       |   21 +++++++++++----------
> >  include/linux/ctype.h         |    6 +++---
> >  lib_generic/crc32.c           |    7 ++++++-
> >  lib_generic/ctype.c           |    2 +-
> >  lib_generic/display_options.c |    5 +++--
> >  lib_generic/vsprintf.c        |    9 ++++++---
> >  lib_ppc/board.c               |    5 +++--
> >  tools/updater/ctype.c         |    2 +-
> >  14 files changed, 82 insertions(+), 67 deletions(-)
> ...
> > -void puts(const char *s)
> > +static void printf_puts(const char *s)
>
> The name "printf_puts" makes my (remaining) hair stand on end. Either
> we have printf(), or we have puts(), but please let's never use
> "printf_puts" - I fear there might be a "printf_getchar" coming, too.

OK, I can fix the name easily.

>
> > +void puts(const char *s)
> > +{
> > +   printf_puts(LINK_OFF(s));
> > +}
>
> Will such changes be needed to all functions that work on (constant?)
> strings?  Or why here?

Only those that work on constant strings before relocation to RAM. One alternative
is to change all call sites to puts(LINK_OFF(s)).
I this particular case I need an intermediate function as puts() are
used by printf() too and printf() don't want the LINK_OFF transformation.

>
> > --- a/cpu/mpc83xx/cpu.c
> > +++ b/cpu/mpc83xx/cpu.c
> > @@ -51,8 +51,8 @@ int checkcpu(void)
> >     char buf[32];
> >     int i;
> >
> > -   const struct cpu_type {
> > -      char name[15];
> > +   static const struct cpu_type {
> > +      char *name;
>
> I guess we have similar constructs in many other places as well. Do
> all of these need to be changed?  Really?

I guess I could have changed the code below:
-			puts(cpu_type_list[i].name);
+			puts(cpu_ptr[i].name);
to something else instead but the change I did do felt best as
we hardly need name to be 15 chars wide and we don't need to initialise
it at runtime.
but yes, all such constructs in cpu_init like code needs to be looked upon
iff one wants to use this feature.

>
> > -   switch (corecnf_tab[corecnf_tab_index].core_csb_ratio) {
> > -   case _byp:
> > -   case _x1:
> > -   case _1x:
> > +   cnf_tab = LINK_OFF(corecnf_tab);
> > +   csb_r = cnf_tab[corecnf_tab_index].core_csb_ratio;
> > +   /* Cannot use a switch stmt here, it uses linked address */
>
> Yet another of these really, really invasive changes.  Is this
> really unavoidable?

Yes, global data and strings accessed before relocation to RAM needs
to be wrapped with LINK_OFF to calculate the real address.

>
> Why can we use "if" but not "switch/case" ?

For some reason, the code generated by gcc wasn't true PIC. The jump table
that the switch stmt generated accessed the GOT. This may very well be a gcc bug.

>
> > --- a/drivers/serial/serial.c
> > +++ b/drivers/serial/serial.c
> > @@ -85,9 +85,9 @@ static NS16550_t serial_ports[4] = {
> >  #endif
> >  };
> >
> > -#define PORT   serial_ports[port-1]
> > +#define PORT   (LINK_OFF(serial_ports)[port-1])
> >  #if defined(CONFIG_CONS_INDEX)
> > -#define CONSOLE   (serial_ports[CONFIG_CONS_INDEX-1])
> > +#define CONSOLE   (LINK_OFF(serial_ports)[CONFIG_CONS_INDEX-1])
> >  #endif
>
> I wonder how you selected the places where such changes were needed,
> and where not. There is certainly lots of similar looking code that
> you don't touch.  Why not?  Because it's not needed (then why do we
> need these changes here?), or because it just did not result in errors
> in your tests so far (then what's the estimate for the full amount of
> changes?) ???

I selected the changes I needed to for my 83xx board and then some. I don't
think serial.c needs any more changes.
Remember you only have to consider code that uses global data and
is executed before relocation to RAM and that limits the scope quite a lot.
The places to look at you find by following board.c's init_sequence.
I think my changes are fairly complete for PowerPC,83xx. There are missing
bits to do, mainly in other archs I think, but boards that doesn't need/want
this functionality don't have to change anything.

 Jocke

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 02/01/2010 19:17:52:
>
> Dear Joakim Tjernlund,
>
> in message <OF7EEC72D7.43E7C3B7-ONC125769E.00078331-C125769E.
> 0009144C at transmode.se> you wrote:
> >
> > Happy new year :)
>
> Thanks, and same to you and everybody else.
>
> > The bigger question is if the LINN_OFF changes in general are acceptable to u-boot.
> > Any board/arch that doesn't want this functionality should not notice I think.
>
>
> What's the impact?  So far it seems to me that there are pretty heavy
> changes to lots of global code anyway, so we can then enable it for
> all boards as well. Otherwise we just add a lot of #ifdef's and make
> the code even harder to read and to understand.  Which does not mean
> that I understood what you are doing :-(

We don't have to add #ifdefs. I #defined a version a LINK_OFF for non users
too. That way we don't have to litter the code with #ifdefs and we get
some type checking from gcc too even for boards that don't use it.

I don't think we can enable this for all boards at once, there will be too much that break.
My goal was to get 83xx fully functional and then other boards could
follow.

 Jocke

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF3EBC8B6E.2C26AE15-ONC12576A0.003379DE-C12576A0.0039F518@transmode.se> you wrote:
>
> > Will such changes be needed to all functions that work on (constant?)
> > strings?  Or why here?
> 
> Only those that work on constant strings before relocation to RAM. One alternative
> is to change all call sites to puts(LINK_OFF(s)).

Well, I understand this is not only puts(), but also all places where
a constant strings are used. And this is a lot of places and a lot of
functions.

> > Yet another of these really, really invasive changes.  Is this
> > really unavoidable?
> 
> Yes, global data and strings accessed before relocation to RAM needs
> to be wrapped with LINK_OFF to calculate the real address.

Ouch.

> I selected the changes I needed to for my 83xx board and then some. I don't
> think serial.c needs any more changes.

Understood. But that means we don't see the real scope of this change
yet, as adapting a new board to use this featue will become a
trial-and-error procedure, adding incrementally more and more of these
changes (unless someone performs a thorough review and catches most of
these places in an initial commit).

> Remember you only have to consider code that uses global data and
> is executed before relocation to RAM and that limits the scope quite a lot.
> The places to look at you find by following board.c's init_sequence.
> I think my changes are fairly complete for PowerPC,83xx. There are missing
> bits to do, mainly in other archs I think, but boards that doesn't need/want
> this functionality don't have to change anything.

I have to admit that I dislike the impact of this change, and I'm not
sure if we really want to take this route.

Comments from others are welcome and needed here!

I think as follows:

In the past, the majority of systems supported by U-Boot where
booting from NOR flash or other memory devices. This made it easy to
use common code (like library functions) both before and after
relocation to the final location in RAM. For your current changes
this means that we have a large number of places where we have to add
this LINK_OFF stuff. This makes the code harder to read, much harder
to understand (especially if it's not working during the initial
bringup on new hardware), and harder to debug in general.

If I try to see trends in the development of U-Boot I notice a
growing number of systems that boot from NAND flash, DataFlash or
that come with on-chip ROM code to load images from SDCard and other
storage media. Such systems cannot make real benefit from the
original design of U-Boot, as here U-Boot is inherently a
second-stage boot loader which gets loaded by some other means. Even
for NAND booting systems where we have the NAND boot code included
within the U-Boot source tree we often cannot share much of the code
between the primary and the secondary loader stages as there are
usually tight restrictions on the maximum size for the primary loader
image. Here a sharper separation of "primary" and "secondary" boot
code within U-Boot would be benefical.

I feel (but this is really just a feeling, and I definitely would like
to hear what others think about this!) your PIC changes would be (or
have been) useful for the former usage mode, but they come at a pretty
heavy cost as they are really invasive to the code.  For the second
usage mode they are not usable, or at least not useful.  This makes me
wonder if we really should continue to work in this direction.

Comments welcome...

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
Research is what I'm doing when I don't know what I'm doing.
                                                 -- Wernher von Braun

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Albert ARIBAUD]

Wolfgang Denk a ?crit :

> I think as follows:
> 
> In the past, the majority of systems supported by U-Boot where
> booting from NOR flash or other memory devices. This made it easy to
> use common code (like library functions) both before and after
> relocation to the final location in RAM. For your current changes
> this means that we have a large number of places where we have to add
> this LINK_OFF stuff. This makes the code harder to read, much harder
> to understand (especially if it's not working during the initial
> bringup on new hardware), and harder to debug in general.
> 
> If I try to see trends in the development of U-Boot I notice a
> growing number of systems that boot from NAND flash, DataFlash or
> that come with on-chip ROM code to load images from SDCard and other
> storage media. Such systems cannot make real benefit from the
> original design of U-Boot, as here U-Boot is inherently a
> second-stage boot loader which gets loaded by some other means. Even
> for NAND booting systems where we have the NAND boot code included
> within the U-Boot source tree we often cannot share much of the code
> between the primary and the secondary loader stages as there are
> usually tight restrictions on the maximum size for the primary loader
> image. Here a sharper separation of "primary" and "secondary" boot
> code within U-Boot would be benefical.
> 
> I feel (but this is really just a feeling, and I definitely would like
> to hear what others think about this!) your PIC changes would be (or
> have been) useful for the former usage mode, but they come at a pretty
> heavy cost as they are really invasive to the code.  For the second
> usage mode they are not usable, or at least not useful.  This makes me
> wonder if we really should continue to work in this direction.
> 
> Comments welcome...
> 
> Best regards,
> 
> Wolfgang Denk

Hmm... PIC is interesting only if you want the same binary to run from 
two places, like NOR then RAM, which is the case when U-boot is the code 
which gets run in NOR at power-up and ends up running in RAM later.

For NAND-based boards, the NAND bootloader will load U-boot to RAM, and 
U-boot will never run from anywhere else but its intended RAM location.

Why not make the same two-stage separation systematic, even on NOR-based 
devices and others where U-boot is currently the one executed at 
power-up? Split the current U-boot into a small primary bootloader (U1?) 
and a fuller secondary bootloader (U2?). U1 would initialize RAM (and a 
console?) and U2 would initialize everything else. Each stage would only 
run from a fixed location and type of memory, removing the need for PIC.

Comment given off the top of my head, so feel free to open fire. :)

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4B40F8DB.1090509@free.fr> you wrote:
>
> Hmm... PIC is interesting only if you want the same binary to run from 
> two places, like NOR then RAM, which is the case when U-boot is the code 
> which gets run in NOR at power-up and ends up running in RAM later.

This scenario is interesting for a lot of other use cases, for
example to load the new version to some free location in RAM, verify
that it works, then copy it (eventually by itself) to persistent
storage; this is especially useful for systems when your JTAG
debugger does not support programming images to NAND or DataFlash or
SPI flash or whatever storage device is used to store the image.

> For NAND-based boards, the NAND bootloader will load U-boot to RAM, and 
> U-boot will never run from anywhere else but its intended RAM location.

Assume you have systems with different RAM size configurations. Being
able to load the same image to any address is beneficial then, too.
[And the NAND bootstrap code often does not allow for additional code
as needed for example for relocation.]

> Why not make the same two-stage separation systematic, even on NOR-based 
> devices and others where U-boot is currently the one executed at 
> power-up? Split the current U-boot into a small primary bootloader (U1?) 

There are many reasons: ease of porting and debugging, minimization of
boot time, etc.

> and a fuller secondary bootloader (U2?). U1 would initialize RAM (and a 
> console?) and U2 would initialize everything else. Each stage would only 
> run from a fixed location and type of memory, removing the need for PIC.

If you use a console in U1, you will need to share a LOT of code with
U2 - things like printf() and all it's dependencies, most of the
str*() and mem*() functions, etc.  And especially for such complicted
and error prone actions like initializing the RAM you _do_ want to be
able to use a console port to print error messages and debug
information.

Thsi is _exactly_ where the current design is coming from.

> Comment given off the top of my head, so feel free to open fire. :)

No, of course not. This is a complicated issue, where it's easy to
overlook one thing or another, so every comment is appreciated.

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
Randal said it would be tough to do in sed. He didn't say  he  didn't
understand  sed.  Randal  understands sed quite well. Which is why he
uses Perl. :-)         - Larry Wall in <7874@jpl-devvax.JPL.NASA.GOV>

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Albert ARIBAUD]

Wolfgang Denk a ?crit :
> Dear Albert ARIBAUD,
> 
> In message <4B40F8DB.1090509@free.fr> you wrote:
>> Hmm... PIC is interesting only if you want the same binary to run from 
>> two places, like NOR then RAM, which is the case when U-boot is the code 
>> which gets run in NOR at power-up and ends up running in RAM later.
> 
> This scenario is interesting for a lot of other use cases, for
> example to load the new version to some free location in RAM, verify
> that it works, then copy it (eventually by itself) to persistent
> storage; this is especially useful for systems when your JTAG
> debugger does not support programming images to NAND or DataFlash or
> SPI flash or whatever storage device is used to store the image.

There is a way out of this one -- I used it myself -- where you build 
both versions of U-boot, the RAM-located one and the FLASH-located one. 
You load the RAM one, run it, and it loads and flashes the FLASH one.

>> For NAND-based boards, the NAND bootloader will load U-boot to RAM, and 
>> U-boot will never run from anywhere else but its intended RAM location.
> 
> Assume you have systems with different RAM size configurations. Being
> able to load the same image to any address is beneficial then, too.
> [And the NAND bootstrap code often does not allow for additional code
> as needed for example for relocation.]

The U1 bootloader might be given the ability to relocate the U2 code. 
that's probably far-fetched, but when linking U2, a map file could be 
generated and a script could produce a relocation table for U1 to use. 
The table could be put in NAND along with the U2 code, so U1 might not 
need to be regenerated for every new U2 build.

>> Why not make the same two-stage separation systematic, even on NOR-based 
>> devices and others where U-boot is currently the one executed at 
>> power-up? Split the current U-boot into a small primary bootloader (U1?) 
> 
> There are many reasons: ease of porting and debugging, minimization of
> boot time, etc.

Granted you'd have some added effort there, but possibly not so much in 
the porting and boot time departments, as executing U1 then U2 would 
roughly be the same as running today's full U-Boot, and equally, porting 
effort would be split rather than duplicated.

>> and a fuller secondary bootloader (U2?). U1 would initialize RAM (and a 
>> console?) and U2 would initialize everything else. Each stage would only 
>> run from a fixed location and type of memory, removing the need for PIC.
> 
> If you use a console in U1, you will need to share a LOT of code with
> U2 - things like printf() and all it's dependencies, most of the
> str*() and mem*() functions, etc.  And especially for such complicted
> and error prone actions like initializing the RAM you _do_ want to be
> able to use a console port to print error messages and debug
> information.

Nothing prevents linking in the same source code in U1 and U2, I think. 
Of course that would make U1 bigger, but you'd need the code in there so 
that's the price to pay -- and it would be a temporary use of RAM, as 
the RAM for U1 could be reused freed when U2 comes into play.

> Thsi is _exactly_ where the current design is coming from.

But obviously your call for comments also calls for a revision of the 
current design, doesn't it?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4B4100F3.8080507@free.fr> you wrote:
>
> > This scenario is interesting for a lot of other use cases, for
> > example to load the new version to some free location in RAM, verify
> > that it works, then copy it (eventually by itself) to persistent
> > storage; this is especially useful for systems when your JTAG
> > debugger does not support programming images to NAND or DataFlash or
> > SPI flash or whatever storage device is used to store the image.
> 
> There is a way out of this one -- I used it myself -- where you build 
> both versions of U-boot, the RAM-located one and the FLASH-located one. 
> You load the RAM one, run it, and it loads and flashes the FLASH one.

Of course this works, but this is then _another_ image, which was not
tested yet. This is bad both from the administration point of view
(need of two differently built images), and bad for the nerves (as you
install an untested image).

> >> For NAND-based boards, the NAND bootloader will load U-boot to RAM, and 
> >> U-boot will never run from anywhere else but its intended RAM location.
> > 
> > Assume you have systems with different RAM size configurations. Being
> > able to load the same image to any address is beneficial then, too.
> > [And the NAND bootstrap code often does not allow for additional code
> > as needed for example for relocation.]
> 
> The U1 bootloader might be given the ability to relocate the U2 code. 
> that's probably far-fetched, but when linking U2, a map file could be 
> generated and a script could produce a relocation table for U1 to use. 
> The table could be put in NAND along with the U2 code, so U1 might not 
> need to be regenerated for every new U2 build.

Keep in mind that U1 often has to fit in as little memory as 4 KB or
so...

> > If you use a console in U1, you will need to share a LOT of code with
> > U2 - things like printf() and all it's dependencies, most of the
> > str*() and mem*() functions, etc.  And especially for such complicted
> > and error prone actions like initializing the RAM you _do_ want to be
> > able to use a console port to print error messages and debug
> > information.
> 
> Nothing prevents linking in the same source code in U1 and U2, I think. 
> Of course that would make U1 bigger, but you'd need the code in there so 
> that's the price to pay -- and it would be a temporary use of RAM, as 
> the RAM for U1 could be reused freed when U2 comes into play.

Creating twobig parts (U1 and U2) is bad for the overal flash memory
footprint, and for the boot time. And usually iut doesn;t work at all
on NAND etc.

> But obviously your call for comments also calls for a revision of the 
> current design, doesn't it?

Sure.

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
Things are not as simple as they seem at first.        - Edward Thorp

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Graeme Russ]

On Mon, Jan 4, 2010 at 7:41 AM, Albert ARIBAUD <albert.aribaud@free.fr> wrote:
> Wolfgang Denk a ?crit :
>> Dear Albert ARIBAUD,
>>
>> In message <4B40F8DB.1090509@free.fr> you wrote:
>>> Hmm... PIC is interesting only if you want the same binary to run from
>>> two places, like NOR then RAM, which is the case when U-boot is the code
>>> which gets run in NOR at power-up and ends up running in RAM later.
>>
>> This scenario is interesting for a lot of other use cases, for
>> example to load the new version to some free location in RAM, verify
>> that it works, then copy it (eventually by itself) to persistent
>> storage; this is especially useful for systems when your JTAG
>> debugger does not support programming images to NAND or DataFlash or
>> SPI flash or whatever storage device is used to store the image.

This is a very interesting use-case - I hadn't though about using U-Boot
version A running at location x to bootstrap into U-Boot version B running
at location y before U-Boot version B copies itself to Flash

>
> There is a way out of this one -- I used it myself -- where you build
> both versions of U-boot, the RAM-located one and the FLASH-located one.
> You load the RAM one, run it, and it loads and flashes the FLASH one.
>
>>> For NAND-based boards, the NAND bootloader will load U-boot to RAM, and
>>> U-boot will never run from anywhere else but its intended RAM location.
>>
>> Assume you have systems with different RAM size configurations. Being
>> able to load the same image to any address is beneficial then, too.
>> [And the NAND bootstrap code often does not allow for additional code
>> as needed for example for relocation.]
>
> The U1 bootloader might be given the ability to relocate the U2 code.
> that's probably far-fetched, but when linking U2, a map file could be
> generated and a script could produce a relocation table for U1 to use.
> The table could be put in NAND along with the U2 code, so U1 might not
> need to be regenerated for every new U2 build.
>

Have a look at my x86 relocation patch series - There is no need to do
this as there is enough information left in the binary if you use the
right gcc/ld options. I have a feeling that what I have done for x86
is portable to other arches.

While the above mentioned patch series allows relocation to an arbitrary
RAM location, it does not allow loading into an abitrarty ROM location.
Considering that there is a very limited range of functions that need to
run from an arbitrary ROM location (primarily RAM init and basic console)
I think we could get away with using this LINK_OFF scheme for a minimal
set of pre-RAM functions.

>>> Why not make the same two-stage separation systematic, even on NOR-based
>>> devices and others where U-boot is currently the one executed at
>>> power-up? Split the current U-boot into a small primary bootloader (U1?)

I support the greater seperation of U-Boot into a two stage boot loader. I
find it somewhat clumsy that ROM and RAM targeted code is mixed up together
in the same source files (board.c etc). I think we may now have a unified
path for relocation (without the -mrelocatable PPC only stuff). If this is
the case, we can finish the task of making all arches relocatable and split
U-Boot cleanly down the ROM/RAM divide.

For those of us who don't care about PIC in ROM, life is easy. For those
that want full PIC, they just need to put in a little more effort into
their ROM based code in U1.

>>
>> There are many reasons: ease of porting and debugging, minimization of
>> boot time, etc.

This opens up a new option for the previously mentioned use-case. We could
totally split U1 and U2 into seperate make targets. Once you have a stable
U1/U2 you flash them onto your board and then tweak versions of U2 and load
them into a seperate location in Flash (into seperate banks to avoid erasing
your known good U1/U2). At boot, you could have a console input to load
'last known good config' if your new U2 is a complete dud.

>
> Granted you'd have some added effort there, but possibly not so much in
> the porting and boot time departments, as executing U1 then U2 would
> roughly be the same as running today's full U-Boot, and equally, porting
> effort would be split rather than duplicated.
>
>>> and a fuller secondary bootloader (U2?). U1 would initialize RAM (and a
>>> console?) and U2 would initialize everything else. Each stage would only
>>> run from a fixed location and type of memory, removing the need for PIC.
>>
>> If you use a console in U1, you will need to share a LOT of code with
>> U2 - things like printf() and all it's dependencies, most of the
>> str*() and mem*() functions, etc.  And especially for such complicted
>> and error prone actions like initializing the RAM you _do_ want to be
>> able to use a console port to print error messages and debug
>> information.
>
> Nothing prevents linking in the same source code in U1 and U2, I think.
> Of course that would make U1 bigger, but you'd need the code in there so
> that's the price to pay -- and it would be a temporary use of RAM, as
> the RAM for U1 could be reused freed when U2 comes into play.
>
>> Thsi is _exactly_ where the current design is coming from.
>
> But obviously your call for comments also calls for a revision of the
> current design, doesn't it?
>

Regards,

G

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 03/01/2010 20:51:53:
>
> Dear Joakim Tjernlund,
>
> In message <OF3EBC8B6E.2C26AE15-ONC12576A0.003379DE-C12576A0.
> 0039F518 at transmode.se> you wrote:
> >
> > > Will such changes be needed to all functions that work on (constant?)
> > > strings?  Or why here?
> >
> > Only those that work on constant strings before relocation to RAM. One alternative
> > is to change all call sites to puts(LINK_OFF(s)).
>
> Well, I understand this is not only puts(), but also all places where
> a constant strings are used. And this is a lot of places and a lot of
> functions.

It is not too bad when you limit the scope to pre RAM functions.
Consider I got my fairly complex 83xx board working with these
patches and that wasn't too bad was it?

>
> > > Yet another of these really, really invasive changes.  Is this
> > > really unavoidable?
> >
> > Yes, global data and strings accessed before relocation to RAM needs
> > to be wrapped with LINK_OFF to calculate the real address.
>
> Ouch.

Yeah, I whish ppc at least could access strings relative to text :(

>
> > I selected the changes I needed to for my 83xx board and then some. I don't
> > think serial.c needs any more changes.
>
> Understood. But that means we don't see the real scope of this change
> yet, as adapting a new board to use this featue will become a
True.

> trial-and-error procedure, adding incrementally more and more of these
> changes (unless someone performs a thorough review and catches most of
> these places in an initial commit).

So much else is trial-and-error so I don't think this a killer.
After all, this is a fairly advanced feature so it is no surprise it
will cost some effort to add support for a board.

>
> > Remember you only have to consider code that uses global data and
> > is executed before relocation to RAM and that limits the scope quite a lot.
> > The places to look at you find by following board.c's init_sequence.
> > I think my changes are fairly complete for PowerPC,83xx. There are missing
> > bits to do, mainly in other archs I think, but boards that doesn't need/want
> > this functionality don't have to change anything.
>
> I have to admit that I dislike the impact of this change, and I'm not
> sure if we really want to take this route.

This was exactly my thinking too. In the end I had to impl. it
to see how bad it would be. After seeing the result I decided
it was worth it.

>
> Comments from others are welcome and needed here!
>
> I think as follows:
>
> In the past, the majority of systems supported by U-Boot where
> booting from NOR flash or other memory devices. This made it easy to
> use common code (like library functions) both before and after
> relocation to the final location in RAM. For your current changes
> this means that we have a large number of places where we have to add
> this LINK_OFF stuff. This makes the code harder to read, much harder

You don't HAVE to add LINK_OFF all over. Only boards that whishes to use
this feature needs to add some more LINK_OFF calls.

> to understand (especially if it's not working during the initial
> bringup on new hardware), and harder to debug in general.
>
> If I try to see trends in the development of U-Boot I notice a
> growing number of systems that boot from NAND flash, DataFlash or
> that come with on-chip ROM code to load images from SDCard and other
> storage media. Such systems cannot make real benefit from the
> original design of U-Boot, as here U-Boot is inherently a
> second-stage boot loader which gets loaded by some other means. Even
> for NAND booting systems where we have the NAND boot code included
> within the U-Boot source tree we often cannot share much of the code
> between the primary and the secondary loader stages as there are
> usually tight restrictions on the maximum size for the primary loader
> image. Here a sharper separation of "primary" and "secondary" boot
> code within U-Boot would be benefical.
>
> I feel (but this is really just a feeling, and I definitely would like
> to hear what others think about this!) your PIC changes would be (or
> have been) useful for the former usage mode, but they come at a pretty
> heavy cost as they are really invasive to the code.  For the second
> usage mode they are not usable, or at least not useful.  This makes me
> wonder if we really should continue to work in this direction.

I have not worked with NAND based boards yet so I can't really say
if this is useful to such systems. I do think though that
NOR based board will be common for quite some time to come so
this new feature will be useful for a long time. Who knows, perhaps
some new designs will be NOR based just because u-boot has this feature.

FWIW, I do agree with you to not split u-boot into a 2 stage
loader, for pretty much the same reasons you listed.

   Jocke

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Albert ARIBAUD]

Wolfgang Denk a ?crit :
> Dear Albert ARIBAUD,
> 
> In message <4B4100F3.8080507@free.fr> you wrote:
>>> This scenario is interesting for a lot of other use cases, for
>>> example to load the new version to some free location in RAM, verify
>>> that it works, then copy it (eventually by itself) to persistent
>>> storage; this is especially useful for systems when your JTAG
>>> debugger does not support programming images to NAND or DataFlash or
>>> SPI flash or whatever storage device is used to store the image.
>> There is a way out of this one -- I used it myself -- where you build 
>> both versions of U-boot, the RAM-located one and the FLASH-located one. 
>> You load the RAM one, run it, and it loads and flashes the FLASH one.
> 
> Of course this works, but this is then _another_ image, which was not
> tested yet. This is bad both from the administration point of view
> (need of two differently built images), and bad for the nerves (as you
> install an untested image).

Well, testing the FLASH version of U-boot will require that you flash 
it, right? Based on the hypothesis that JTAG cannot flash, then you have 
no choice but to flash it some other way.

What I did with U-boot in this case (worse yet: in a case where I did 
*not* have JTAG equipment at all) was to start from a u-boot with a 
proven low level init code, build it for RAM by disabling the low level 
code, test its flashing capability on unused sectors of FLASH, and once 
validated, build it also for FLASH. That was risky, but if you cannot 
flash with anything but U-boot, there was little else I could do.

>>>> For NAND-based boards, the NAND bootloader will load U-boot to RAM, and 
>>>> U-boot will never run from anywhere else but its intended RAM location.
>>> Assume you have systems with different RAM size configurations. Being
>>> able to load the same image to any address is beneficial then, too.
>>> [And the NAND bootstrap code often does not allow for additional code
>>> as needed for example for relocation.]
>> The U1 bootloader might be given the ability to relocate the U2 code. 
>> that's probably far-fetched, but when linking U2, a map file could be 
>> generated and a script could produce a relocation table for U1 to use. 
>> The table could be put in NAND along with the U2 code, so U1 might not 
>> need to be regenerated for every new U2 build.
> 
> Keep in mind that U1 often has to fit in as little memory as 4 KB or
> so...

If it is tightly limited then it cannot take the console code with 
itself, so it'll have to fetch it from NAND before being able to log 
anything. And for that, it needs RAM access. so basically it won't be 
able to log RAM activation when it happens (unless you think of loading 
extra code in IRAM, but if there is IRAM, it probably contains U1 at 
this stage). Better have a thoroughly JTAG-tested minimal U1 which does 
not log (or if logging is required, U1 logs to a buffer, and U2 will 
print the log once started, a bit like Linux early logging).

>>> If you use a console in U1, you will need to share a LOT of code with
>>> U2 - things like printf() and all it's dependencies, most of the
>>> str*() and mem*() functions, etc.  And especially for such complicted
>>> and error prone actions like initializing the RAM you _do_ want to be
>>> able to use a console port to print error messages and debug
>>> information.
>> Nothing prevents linking in the same source code in U1 and U2, I think. 
>> Of course that would make U1 bigger, but you'd need the code in there so 
>> that's the price to pay -- and it would be a temporary use of RAM, as 
>> the RAM for U1 could be reused freed when U2 comes into play.
> 
> Creating twobig parts (U1 and U2) is bad for the overal flash memory
> footprint, and for the boot time. And usually iut doesn;t work at all
> on NAND etc.

Then there could be a three-part split: U1, U2 and Ulib. Link them all 
as relocatable code, and let U1's core load and relocate Ulib, and U2's 
core relocate its own references to U1's loaded Ulib location. Easily 
said, I know, but we're still in brainstorming stage here so let's give 
it a look. Agreed, this three-way split is going to cost some extra 
execution time but it could keep overall footprint minimal.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Mike Frysinger]

On Sunday 03 January 2010 14:51:53 Wolfgang Denk wrote:
> If I try to see trends in the development of U-Boot I notice a
> growing number of systems that boot from NAND flash, DataFlash or
> that come with on-chip ROM code to load images from SDCard and other
> storage media. Such systems cannot make real benefit from the
> original design of U-Boot, as here U-Boot is inherently a
> second-stage boot loader which gets loaded by some other means. Even
> for NAND booting systems where we have the NAND boot code included
> within the U-Boot source tree we often cannot share much of the code
> between the primary and the secondary loader stages as there are
> usually tight restrictions on the maximum size for the primary loader
> image. Here a sharper separation of "primary" and "secondary" boot
> code within U-Boot would be benefical.
> 
> I feel (but this is really just a feeling, and I definitely would like
> to hear what others think about this!) your PIC changes would be (or
> have been) useful for the former usage mode, but they come at a pretty
> heavy cost as they are really invasive to the code.  For the second
> usage mode they are not usable, or at least not useful.  This makes me
> wonder if we really should continue to work in this direction.

the Blackfin processors are going the direction of an on-chip ROM handles 
loading from some storage (nor/nand/spi/usb/uart/sd/whatever) into RAM.  there 
really isnt any usage model of executing in NOR for some time before running 
out of RAM.  so generally, all this additional support is seen largely as 
overhead on Blackfin parts since there isnt any need/desire for them.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100104/f8e4df11/attachment.pgp 

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Scott Wood]

On Sun, Jan 03, 2010 at 09:06:51PM +0100, Albert ARIBAUD wrote:
> Hmm... PIC is interesting only if you want the same binary to run from 
> two places, like NOR then RAM, which is the case when U-boot is the code 
> which gets run in NOR at power-up and ends up running in RAM later.
> 
> For NAND-based boards, the NAND bootloader will load U-boot to RAM, and 
> U-boot will never run from anywhere else but its intended RAM location.

Note that the first-stage NAND loader still needs to be able to relocate
itself to RAM in order to free up the NAND buffer for loading more data.

-Scott

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Joakim Tjernlund]

u-boot-bounces at lists.denx.de wrote on 05/01/2010 21:20:32:

> From: Scott Wood <scottwood@freescale.com>
> To: Albert ARIBAUD <albert.aribaud@free.fr>
> Cc: u-boot at lists.denx.de
> Date: 05/01/2010 21:22
> Subject: Re: [U-Boot] [PATCH 2/4] Use LINK_OFF to access global data
> Sent by: u-boot-bounces at lists.denx.de
>
> On Sun, Jan 03, 2010 at 09:06:51PM +0100, Albert ARIBAUD wrote:
> > Hmm... PIC is interesting only if you want the same binary to run from
> > two places, like NOR then RAM, which is the case when U-boot is the code
> > which gets run in NOR at power-up and ends up running in RAM later.
> >
> > For NAND-based boards, the NAND bootloader will load U-boot to RAM, and
> > U-boot will never run from anywhere else but its intended RAM location.
>
> Note that the first-stage NAND loader still needs to be able to relocate
> itself to RAM in order to free up the NAND buffer for loading more data.

Hmm, does that mean that the LINK_OFF patches are useful to you or not?

[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Scott Wood]

Joakim Tjernlund wrote:
> 
> u-boot-bounces at lists.denx.de wrote on 05/01/2010 21:20:32:
> 
>> From: Scott Wood <scottwood@freescale.com>
>> To: Albert ARIBAUD <albert.aribaud@free.fr>
>> Cc: u-boot at lists.denx.de
>> Date: 05/01/2010 21:22
>> Subject: Re: [U-Boot] [PATCH 2/4] Use LINK_OFF to access global data
>> Sent by: u-boot-bounces at lists.denx.de
>>
>> On Sun, Jan 03, 2010 at 09:06:51PM +0100, Albert ARIBAUD wrote:
>>> Hmm... PIC is interesting only if you want the same binary to run from
>>> two places, like NOR then RAM, which is the case when U-boot is the code
>>> which gets run in NOR at power-up and ends up running in RAM later.
>>>
>>> For NAND-based boards, the NAND bootloader will load U-boot to RAM, and
>>> U-boot will never run from anywhere else but its intended RAM location.
>> Note that the first-stage NAND loader still needs to be able to relocate
>> itself to RAM in order to free up the NAND buffer for loading more data.
> 
> Hmm, does that mean that the LINK_OFF patches are useful to you or not?

I was just responding to a suggestion that a split similar to the NAND 
loader might eliminate the need for relocation/PIC support.

I am a bit nervous about this stuff, though -- why is it needed?  We 
just got rid of the need for manual relocations, and now we're adding 
them back (pre-reloc instead of post-reloc). :-(

-Scott