public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot-Users] [PATCH] Move init_sequence table into code.
@ 2008-03-28 15:45 Joakim Tjernlund
  2008-03-28 15:45 ` [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function Joakim Tjernlund
  2008-04-14  1:09 ` [U-Boot-Users] [PATCH] Move init_sequence table into code Wolfgang Denk
  0 siblings, 2 replies; 16+ messages in thread
From: Joakim Tjernlund @ 2008-03-28 15:45 UTC (permalink / raw)
  To: u-boot

Global variables are not ideal before relocation to RAM.
---

I hope this still applies.

 lib_ppc/board.c |  144 +++++++++++++++++++++++++-----------------------------
 1 files changed, 67 insertions(+), 77 deletions(-)

diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index 8a18350..c64bbd5 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -288,124 +288,114 @@ static int init_func_watchdog_reset (void)
  ************************************************************************
  */
 
-init_fnc_t *init_sequence[] = {
+/************************************************************************
+ *
+ * This is the first part of the initialization sequence that is
+ * implemented in C, but still running from ROM.
+ *
+ * The main purpose is to provide a (serial) console interface as
+ * soon as possible (so we can see any error messages), and to
+ * initialize the RAM so that we can relocate the monitor code to
+ * RAM.
+ *
+ * Be aware of the restrictions: global data is read-only, BSS is not
+ * initialized, and stack space is limited to a few kB.
+ *
+ ************************************************************************
+ */
+
+void board_init_f (ulong bootflag)
+{
+	bd_t *bd;
+	ulong len, addr, addr_sp;
+	ulong *s;
+	gd_t *id;
+	init_fnc_t **init_fnc_ptr;
+#ifdef CONFIG_PRAM
+	int i;
+	ulong reg;
+	uchar tmp[64];		/* long enough for environment variables */
+#endif
+
+	/* Pointer is writable since we allocated a register for it */
+	gd = (gd_t *) (CFG_INIT_RAM_ADDR + CFG_GBL_DATA_OFFSET);
+	/* compiler optimization barrier needed for GCC >= 3.4 */
+	__asm__ __volatile__("": : :"memory");
+
+#if !defined(CONFIG_CPM2) && !defined(CONFIG_MPC83XX)
+	/* Clear initial global data */
+	memset ((void *) gd, 0, sizeof (gd_t));
+#endif
 
 #if defined(CONFIG_BOARD_EARLY_INIT_F)
-	board_early_init_f,
+	board_early_init_f();
 #endif
 
 #if !defined(CONFIG_8xx_CPUCLK_DEFAULT)
-	get_clocks,		/* get CPU and bus clocks (etc.) */
+	get_clocks();		/* get CPU and bus clocks (etc.) */
 #if defined(CONFIG_TQM8xxL) && !defined(CONFIG_TQM866M) \
     && !defined(CONFIG_TQM885D)
-	adjust_sdram_tbs_8xx,
+	adjust_sdram_tbs_8xx();
 #endif
-	init_timebase,
+	init_timebase();
 #endif
 #ifdef CFG_ALLOC_DPRAM
 #if !defined(CONFIG_CPM2)
-	dpram_init,
+	dpram_init();
 #endif
 #endif
 #if defined(CONFIG_BOARD_POSTCLK_INIT)
-	board_postclk_init,
+	board_postclk_init();
 #endif
-	env_init,
+	env_init();
 #if defined(CONFIG_8xx_CPUCLK_DEFAULT)
-	get_clocks_866,		/* get CPU and bus clocks according to the environment variable */
-	sdram_adjust_866,	/* adjust sdram refresh rate according to the new clock */
-	init_timebase,
-#endif
-	init_baudrate,
-	serial_init,
-	console_init_f,
-	display_options,
+	get_clocks_866();		/* get CPU and bus clocks according to the environment variable */
+	sdram_adjust_866();	/* adjust sdram refresh rate according to the new clock */
+	init_timebase();
+#endif
+	init_baudrate();
+	serial_init();
+	console_init_f();
+	display_options();
 #if defined(CONFIG_8260)
-	prt_8260_rsr,
-	prt_8260_clks,
+	prt_8260_rsr();
+	prt_8260_clks();
 #endif /* CONFIG_8260 */
 #if defined(CONFIG_MPC83XX)
-	prt_83xx_rsr,
+	prt_83xx_rsr();
 #endif
-	checkcpu,
+	checkcpu();
 #if defined(CONFIG_MPC5xxx)
-	prt_mpc5xxx_clks,
+	prt_mpc5xxx_clks();
 #endif /* CONFIG_MPC5xxx */
 #if defined(CONFIG_MPC8220)
-	prt_mpc8220_clks,
+	prt_mpc8220_clks();
 #endif
-	checkboard,
+	checkboard();
 	INIT_FUNC_WATCHDOG_INIT
 #if defined(CONFIG_MISC_INIT_F)
-	misc_init_f,
+	misc_init_f();
 #endif
 	INIT_FUNC_WATCHDOG_RESET
 #if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
-	init_func_i2c,
+	init_func_i2c();
 #endif
 #if defined(CONFIG_HARD_SPI)
-	init_func_spi,
+	init_func_spi();
 #endif
 #if defined(CONFIG_DTT)		/* Digital Thermometers and Thermostats */
-	dtt_init,
+	dtt_init();
 #endif
 #ifdef CONFIG_POST
-	post_init_f,
+	post_init_f();
 #endif
 	INIT_FUNC_WATCHDOG_RESET
-	init_func_ram,
+	init_func_ram();
 #if defined(CFG_DRAM_TEST)
-	testdram,
+	testdram();
 #endif /* CFG_DRAM_TEST */
 	INIT_FUNC_WATCHDOG_RESET
 
-	NULL,			/* Terminate this list */
-};
-
-/************************************************************************
- *
- * This is the first part of the initialization sequence that is
- * implemented in C, but still running from ROM.
- *
- * The main purpose is to provide a (serial) console interface as
- * soon as possible (so we can see any error messages), and to
- * initialize the RAM so that we can relocate the monitor code to
- * RAM.
- *
- * Be aware of the restrictions: global data is read-only, BSS is not
- * initialized, and stack space is limited to a few kB.
- *
- ************************************************************************
- */
-
-void board_init_f (ulong bootflag)
-{
-	bd_t *bd;
-	ulong len, addr, addr_sp;
-	ulong *s;
-	gd_t *id;
-	init_fnc_t **init_fnc_ptr;
-#ifdef CONFIG_PRAM
-	int i;
-	ulong reg;
-	uchar tmp[64];		/* long enough for environment variables */
-#endif
-
-	/* Pointer is writable since we allocated a register for it */
-	gd = (gd_t *) (CFG_INIT_RAM_ADDR + CFG_GBL_DATA_OFFSET);
-	/* compiler optimization barrier needed for GCC >= 3.4 */
-	__asm__ __volatile__("": : :"memory");
-
-#if !defined(CONFIG_CPM2) && !defined(CONFIG_MPC83XX)
-	/* Clear initial global data */
-	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) {
-			hang ();
-		}
-	}
 
 	/*
 	 * Now that we have DRAM mapped and working, we can
-- 
1.5.4.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-03-28 15:45 [U-Boot-Users] [PATCH] Move init_sequence table into code Joakim Tjernlund
@ 2008-03-28 15:45 ` Joakim Tjernlund
  2008-03-30 18:08   ` Jean-Christophe PLAGNIOL-VILLARD
  2008-03-30 20:59   ` Wolfgang Denk
  2008-04-14  1:09 ` [U-Boot-Users] [PATCH] Move init_sequence table into code Wolfgang Denk
  1 sibling, 2 replies; 16+ messages in thread
From: Joakim Tjernlund @ 2008-03-28 15:45 UTC (permalink / raw)
  To: u-boot

This avoids an early global data reference.

---
I hope this still applies.

 api/api.c            |    1 -
 common/cmd_bootm.c   |    3 ---
 common/cmd_nvedit.c  |    3 ---
 common/env_common.c  |   21 +++++++++++++++------
 common/env_eeprom.c  |    1 -
 common/env_nvram.c   |    1 -
 common/fdt_support.c |    4 ----
 common/ft_build.c    |    3 ---
 include/common.h     |    1 +
 9 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/api/api.c b/api/api.c
index 0598d90..c1b2b60 100644
--- a/api/api.c
+++ b/api/api.c
@@ -40,7 +40,6 @@
 
 /* U-Boot routines needed */
 extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
-extern uchar (*env_get_char)(int);
 extern uchar *env_get_addr(int);
 
 /*****************************************************************************
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 9546729..5062817 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -1150,9 +1150,6 @@ do_bootm_netbsd (cmd_tbl_t *cmdtp, int flag,
 
 #if defined(CONFIG_ARTOS) && defined(CONFIG_PPC)
 
-/* Function that returns a character from the environment */
-extern uchar (*env_get_char)(int);
-
 static void
 do_bootm_artos (cmd_tbl_t *cmdtp, int flag,
 		int	argc, char *argv[],
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index dd263b6..15dca5b 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -68,9 +68,6 @@ DECLARE_GLOBAL_DATA_PTR;
 /************************************************************************
 ************************************************************************/
 
-/* Function that returns a character from the environment */
-extern uchar (*env_get_char)(int);
-
 /* Function that returns a pointer to a value from the environment */
 /* (Only memory version supported / needed). */
 extern uchar *env_get_addr(int);
diff --git a/common/env_common.c b/common/env_common.c
index a494812..e87818b 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -50,7 +50,6 @@ extern void env_relocate_spec (void);
 extern uchar env_get_char_spec(int);
 
 static uchar env_get_char_init (int index);
-uchar (*env_get_char)(int) = env_get_char_init;
 
 /************************************************************************
  * Default settings to be used when no valid environment is found
@@ -182,6 +181,21 @@ uchar env_get_char_memory (int index)
 }
 #endif
 
+uchar env_get_char (int index)
+{
+	uchar c;
+
+	/* if relocated to RAM */
+	if (gd->flags & GD_FLG_RELOC)
+	{
+		c = env_get_char_memory(index);
+	} else {
+		c = env_get_char_init(index);
+	}
+
+	return (c);
+}
+
 uchar *env_get_addr (int index)
 {
 	if (gd->env_valid) {
@@ -215,11 +229,6 @@ void env_relocate (void)
 	DEBUGF ("%s[%d] malloced ENV at %p\n", __FUNCTION__,__LINE__,env_ptr);
 #endif
 
-	/*
-	 * After relocation to RAM, we can always use the "memory" functions
-	 */
-	env_get_char = env_get_char_memory;
-
 	if (gd->env_valid == 0) {
 #if defined(CONFIG_GTH)	|| defined(CFG_ENV_IS_NOWHERE)	/* Environment not changable */
 		puts ("Using default environment\n\n");
diff --git a/common/env_eeprom.c b/common/env_eeprom.c
index 2adc129..fae87ca 100644
--- a/common/env_eeprom.c
+++ b/common/env_eeprom.c
@@ -38,7 +38,6 @@ env_t *env_ptr = NULL;
 
 char * env_name_spec = "EEPROM";
 
-extern uchar (*env_get_char)(int);
 extern uchar env_get_char_memory (int index);
 
 
diff --git a/common/env_nvram.c b/common/env_nvram.c
index 7c18896..bfc8d02 100644
--- a/common/env_nvram.c
+++ b/common/env_nvram.c
@@ -63,7 +63,6 @@ char * env_name_spec = "NVRAM";
 extern uchar default_environment[];
 extern int default_environment_size;
 
-extern uchar (*env_get_char)(int);
 extern uchar env_get_char_memory (int index);
 
 #ifdef CONFIG_AMIGAONEG3SE
diff --git a/common/fdt_support.c b/common/fdt_support.c
index a13c140..fc43b43 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -225,10 +225,6 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force)
 
 #ifdef CONFIG_OF_HAS_UBOOT_ENV
 
-/* Function that returns a character from the environment */
-extern uchar(*env_get_char) (int);
-
-
 int fdt_env(void *fdt)
 {
 	int   nodeoffset;
diff --git a/common/ft_build.c b/common/ft_build.c
index 5a0575e..bd0c915 100644
--- a/common/ft_build.c
+++ b/common/ft_build.c
@@ -396,9 +396,6 @@ void *ft_get_prop(void *bphp, const char *propname, int *szp)
 
 /********************************************************************/
 
-/* Function that returns a character from the environment */
-extern uchar(*env_get_char) (int);
-
 #define BDM(x)	{	.name = #x, .offset = offsetof(bd_t, bi_ ##x ) }
 
 #ifdef CONFIG_OF_HAS_BD_T
diff --git a/include/common.h b/include/common.h
index 54083f1..4fed250 100644
--- a/include/common.h
+++ b/include/common.h
@@ -229,6 +229,7 @@ extern ulong load_addr;		/* Default Load Address */
 /* common/cmd_nvedit.c */
 int	env_init     (void);
 void	env_relocate (void);
+uchar	env_get_char (int);
 int	envmatch     (uchar *, int);
 char	*getenv	     (char *);
 int	getenv_r     (char *name, char *buf, unsigned len);
-- 
1.5.4.3

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-03-28 15:45 ` [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function Joakim Tjernlund
@ 2008-03-30 18:08   ` Jean-Christophe PLAGNIOL-VILLARD
  2008-03-30 20:59   ` Wolfgang Denk
  1 sibling, 0 replies; 16+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2008-03-30 18:08 UTC (permalink / raw)
  To: u-boot

On 16:45 Fri 28 Mar     , Joakim Tjernlund wrote:
> This avoids an early global data reference.
> 
> ---
> I hope this still applies.
> 
>  api/api.c            |    1 -
>  common/cmd_bootm.c   |    3 ---
>  common/cmd_nvedit.c  |    3 ---
>  common/env_common.c  |   21 +++++++++++++++------
>  common/env_eeprom.c  |    1 -
>  common/env_nvram.c   |    1 -
>  common/fdt_support.c |    4 ----
>  common/ft_build.c    |    3 ---
>  include/common.h     |    1 +
>  9 files changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/api/api.c b/api/api.c
> index 0598d90..c1b2b60 100644
> --- a/api/api.c
> +++ b/api/api.c
> @@ -40,7 +40,6 @@
>  
>  /* U-Boot routines needed */
>  extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> -extern uchar (*env_get_char)(int);
>  extern uchar *env_get_addr(int);
>  
>  /*****************************************************************************
> diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> index 9546729..5062817 100644
> --- a/common/cmd_bootm.c
> +++ b/common/cmd_bootm.c
> @@ -1150,9 +1150,6 @@ do_bootm_netbsd (cmd_tbl_t *cmdtp, int flag,
>  
>  #if defined(CONFIG_ARTOS) && defined(CONFIG_PPC)
>  
> -/* Function that returns a character from the environment */
> -extern uchar (*env_get_char)(int);
> -
>  static void
>  do_bootm_artos (cmd_tbl_t *cmdtp, int flag,
>  		int	argc, char *argv[],
> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> index dd263b6..15dca5b 100644
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
> @@ -68,9 +68,6 @@ DECLARE_GLOBAL_DATA_PTR;
>  /************************************************************************
>  ************************************************************************/
>  
> -/* Function that returns a character from the environment */
> -extern uchar (*env_get_char)(int);
> -
>  /* Function that returns a pointer to a value from the environment */
>  /* (Only memory version supported / needed). */
>  extern uchar *env_get_addr(int);
> diff --git a/common/env_common.c b/common/env_common.c
> index a494812..e87818b 100644
> --- a/common/env_common.c
> +++ b/common/env_common.c
> @@ -50,7 +50,6 @@ extern void env_relocate_spec (void);
>  extern uchar env_get_char_spec(int);
>  
>  static uchar env_get_char_init (int index);
> -uchar (*env_get_char)(int) = env_get_char_init;
>  
>  /************************************************************************
>   * Default settings to be used when no valid environment is found
> @@ -182,6 +181,21 @@ uchar env_get_char_memory (int index)
>  }
>  #endif
>  
> +uchar env_get_char (int index)
> +{
> +	uchar c;
> +
> +	/* if relocated to RAM */
> +	if (gd->flags & GD_FLG_RELOC)
> +	{
> +		c = env_get_char_memory(index);
> +	} else {
> +		c = env_get_char_init(index);
> +	}
Please remove occolade
Best Regards,
J.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-03-28 15:45 ` [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function Joakim Tjernlund
  2008-03-30 18:08   ` Jean-Christophe PLAGNIOL-VILLARD
@ 2008-03-30 20:59   ` Wolfgang Denk
  2008-03-30 21:05     ` Joakim Tjernlund
  1 sibling, 1 reply; 16+ messages in thread
From: Wolfgang Denk @ 2008-03-30 20:59 UTC (permalink / raw)
  To: u-boot

In message <1206719159-11200-2-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> This avoids an early global data reference.

What exactly is the problem you're trying to fix? It seems the
resulting code is not exactly smaller than the old one?

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
Contrary to popular belief, thinking does not cause brain damage.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-03-30 20:59   ` Wolfgang Denk
@ 2008-03-30 21:05     ` Joakim Tjernlund
  2008-03-30 22:33       ` Wolfgang Denk
  0 siblings, 1 reply; 16+ messages in thread
From: Joakim Tjernlund @ 2008-03-30 21:05 UTC (permalink / raw)
  To: u-boot


On Sun, 2008-03-30 at 22:59 +0200, Wolfgang Denk wrote:
> In message <1206719159-11200-2-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > This avoids an early global data reference.
> 
> What exactly is the problem you're trying to fix? It seems the
> resulting code is not exactly smaller than the old one?

Just one step closer to full relocation of u-boot. Global variables
before relocation to RAM is hard to deal with. Not sure if the code
got smaller or not.

 Jocke

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-03-30 21:05     ` Joakim Tjernlund
@ 2008-03-30 22:33       ` Wolfgang Denk
  2008-03-31  7:15         ` Joakim Tjernlund
  0 siblings, 1 reply; 16+ messages in thread
From: Wolfgang Denk @ 2008-03-30 22:33 UTC (permalink / raw)
  To: u-boot

In message <1206911129.7589.411.camel@gentoo-jocke.transmode.se> you wrote:
> 
> Just one step closer to full relocation of u-boot. Global variables
> before relocation to RAM is hard to deal with. Not sure if the code
> got smaller or not.

Hm...there are some such variables.

Did you compare sizes?

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
If ignorance is bliss, why aren't there more happy people?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-03-30 22:33       ` Wolfgang Denk
@ 2008-03-31  7:15         ` Joakim Tjernlund
  2008-04-05 17:31           ` Joakim Tjernlund
  0 siblings, 1 reply; 16+ messages in thread
From: Joakim Tjernlund @ 2008-03-31  7:15 UTC (permalink / raw)
  To: u-boot


On Mon, 2008-03-31 at 00:33 +0200, Wolfgang Denk wrote:
> In message <1206911129.7589.411.camel@gentoo-jocke.transmode.se> you wrote:
> > 
> > Just one step closer to full relocation of u-boot. Global variables
> > before relocation to RAM is hard to deal with. Not sure if the code
> > got smaller or not.
> 
> Hm...there are some such variables.
> 
> Did you compare sizes?

I got the hint, thanks :)

size before:
   text	   data	    bss	    dec	    hex	filename
 204117	  13520	  28716	 246353	  3c251	u-boot

size after:
   text	   data	    bss	    dec	    hex	filename
 203893	  13500	  28716	 246109	  3c15d	u-boot

So it also got smaller by 244 bytes.
Updated patch, with signoff and Jean-Christophe
comment addressed.

 Jocke

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-03-31  7:15         ` Joakim Tjernlund
@ 2008-04-05 17:31           ` Joakim Tjernlund
  2008-04-14  1:09             ` Wolfgang Denk
  0 siblings, 1 reply; 16+ messages in thread
From: Joakim Tjernlund @ 2008-04-05 17:31 UTC (permalink / raw)
  To: u-boot

Did you try this one or did du skip it this release?

  Jocke

On Mon, 2008-03-31 at 00:33 +0200, Wolfgang Denk wrote:
> In message <1206911129.7589.411.camel@gentoo-jocke.transmode.se> you wrote:
> > 
> > Just one step closer to full relocation of u-boot. Global variables
> > before relocation to RAM is hard to deal with. Not sure if the code
> > got smaller or not.
> 
> Hm...there are some such variables.
> 
> Did you compare sizes?

I got the hint, thanks :)

size before:
   text	   data	    bss	    dec	    hex	filename
 204117	  13520	  28716	 246353	  3c251	u-boot

size after:
   text	   data	    bss	    dec	    hex	filename
 203893	  13500	  28716	 246109	  3c15d	u-boot

So it also got smaller by 244 bytes.
Updated patch, with signoff and Jean-Christophe
comment addressed.

 Jocke

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Move init_sequence table into code.
  2008-03-28 15:45 [U-Boot-Users] [PATCH] Move init_sequence table into code Joakim Tjernlund
  2008-03-28 15:45 ` [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function Joakim Tjernlund
@ 2008-04-14  1:09 ` Wolfgang Denk
  2008-04-14 21:06   ` Joakim Tjernlund
  1 sibling, 1 reply; 16+ messages in thread
From: Wolfgang Denk @ 2008-04-14  1:09 UTC (permalink / raw)
  To: u-boot

In message <1206719159-11200-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> Global variables are not ideal before relocation to RAM.

Byt they don't cause any real problem either, or am I missing
something?

I tend to reject the patch.

> -init_fnc_t *init_sequence[] = {

The original idea of having such a list  of  funtion  pointers  which
just  get executed one after another was to be able to wrap this into
some "#ifndef CONFIG_INIT_SEQUENCE" and use this to allow for  board-
specific init sequences by just adding a #define with the needed list
of functions to the board config files.

Even though this feature never was used so far, I still  hesitate  to
throw  it away - at least as long as I don't see benefits for the new
solution.

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
Never worry about theory as long as  the  machinery  does  what  it's
supposed to do.                                      - R. A. Heinlein

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-04-05 17:31           ` Joakim Tjernlund
@ 2008-04-14  1:09             ` Wolfgang Denk
  2008-04-14 21:01               ` Joakim Tjernlund
  0 siblings, 1 reply; 16+ messages in thread
From: Wolfgang Denk @ 2008-04-14  1:09 UTC (permalink / raw)
  To: u-boot

In message <024e01c89742$e48d4b80$ada7e280$@Tjernlund@transmode.se> you wrote:
> Did you try this one or did du skip it this release?

Unfortunately this doesn't apply any more:

Applying Change env_get_char from a global function ptr to a function.
error: patch failed: common/cmd_bootm.c:1150
error: common/cmd_bootm.c: patch does not apply
error: patch failed: common/fdt_support.c:225
error: common/fdt_support.c: patch does not apply
error: patch failed: common/ft_build.c:396
error: common/ft_build.c: patch does not apply
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merged common/cmd_bootm.c
CONFLICT (content): Merge conflict in common/cmd_bootm.c
Auto-merged common/cmd_nvedit.c
Auto-merged common/fdt_support.c
CONFLICT (content): Merge conflict in common/fdt_support.c
Auto-merged common/ft_build.c
CONFLICT (content): Merge conflict in common/ft_build.c
Auto-merged include/common.h
Failed to merge in the changes.
Patch failed at 0001.


Could you please update and resubmit? Thanks in advance.

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
You don't have to worry about me. I might have been born yesterday...
but I stayed up all night.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-04-14  1:09             ` Wolfgang Denk
@ 2008-04-14 21:01               ` Joakim Tjernlund
  2008-04-18  4:14                 ` Wolfgang Denk
  0 siblings, 1 reply; 16+ messages in thread
From: Joakim Tjernlund @ 2008-04-14 21:01 UTC (permalink / raw)
  To: u-boot


On Mon, 2008-04-14 at 03:09 +0200, Wolfgang Denk wrote:
> In message <024e01c89742$e48d4b80$ada7e280$@Tjernlund@transmode.se> you wrote:
> > Did you try this one or did du skip it this release?
> 
> Unfortunately this doesn't apply any more:
> 
> Applying Change env_get_char from a global function ptr to a function.
> error: patch failed: common/cmd_bootm.c:1150
> error: common/cmd_bootm.c: patch does not apply
> error: patch failed: common/fdt_support.c:225
> error: common/fdt_support.c: patch does not apply
> error: patch failed: common/ft_build.c:396
> error: common/ft_build.c: patch does not apply
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> Auto-merged common/cmd_bootm.c
> CONFLICT (content): Merge conflict in common/cmd_bootm.c
> Auto-merged common/cmd_nvedit.c
> Auto-merged common/fdt_support.c
> CONFLICT (content): Merge conflict in common/fdt_support.c
> Auto-merged common/ft_build.c
> CONFLICT (content): Merge conflict in common/ft_build.c
> Auto-merged include/common.h
> Failed to merge in the changes.
> Patch failed at 0001.
> 
> 
> Could you please update and resubmit? Thanks in advance.
> 
> Best regards,
> 
> Wolfgang Denk

OK, here it is.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Move init_sequence table into code.
  2008-04-14  1:09 ` [U-Boot-Users] [PATCH] Move init_sequence table into code Wolfgang Denk
@ 2008-04-14 21:06   ` Joakim Tjernlund
  2008-04-15  4:35     ` Wolfgang Denk
  0 siblings, 1 reply; 16+ messages in thread
From: Joakim Tjernlund @ 2008-04-14 21:06 UTC (permalink / raw)
  To: u-boot


On Mon, 2008-04-14 at 03:09 +0200, Wolfgang Denk wrote:
> In message <1206719159-11200-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > Global variables are not ideal before relocation to RAM.
> 
> Byt they don't cause any real problem either, or am I missing
> something?

Makes u-boot smaller too.
It is a step closer towards full relocation of u-boot, I want to get rid
of using global data while in FLASH.

> 
> I tend to reject the patch.
> 
> > -init_fnc_t *init_sequence[] = {
> 
> The original idea of having such a list  of  funtion  pointers  which
> just  get executed one after another was to be able to wrap this into
> some "#ifndef CONFIG_INIT_SEQUENCE" and use this to allow for  board-
> specific init sequences by just adding a #define with the needed list
> of functions to the board config files.

You can do that with weak functions too. Just make all the functions
weak, then a board can overide with its own function.

> 
> Even though this feature never was used so far, I still  hesitate  to
> throw  it away - at least as long as I don't see benefits for the new
> solution.
> 
> Best regards,
> 
> Wolfgang Denk
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Move init_sequence table into code.
  2008-04-14 21:06   ` Joakim Tjernlund
@ 2008-04-15  4:35     ` Wolfgang Denk
  2008-04-15  6:40       ` Joakim Tjernlund
  0 siblings, 1 reply; 16+ messages in thread
From: Wolfgang Denk @ 2008-04-15  4:35 UTC (permalink / raw)
  To: u-boot

In message <1208207184.5911.26.camel@gentoo-jocke.transmode.se> you wrote:
> 
> It is a step closer towards full relocation of u-boot, I want to get rid
> of using global data while in FLASH.

I doubt that this will work, but I'd love to be surprised :-)

> > The original idea of having such a list  of  funtion  pointers  which
> > just  get executed one after another was to be able to wrap this into
> > some "#ifndef CONFIG_INIT_SEQUENCE" and use this to allow for  board-
> > specific init sequences by just adding a #define with the needed list
> > of functions to the board config files.
> 
> You can do that with weak functions too. Just make all the functions
> weak, then a board can overide with its own function.

That would not, for example, to allow to change the sequence - say
one board needs to initialize PCI very early, but another one very
late.

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
If the hours are long enough and the pay  is  short  enough,  someone
will say it's women's work.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Move init_sequence table into code.
  2008-04-15  4:35     ` Wolfgang Denk
@ 2008-04-15  6:40       ` Joakim Tjernlund
  2008-04-18  4:14         ` Wolfgang Denk
  0 siblings, 1 reply; 16+ messages in thread
From: Joakim Tjernlund @ 2008-04-15  6:40 UTC (permalink / raw)
  To: u-boot


On Tue, 2008-04-15 at 06:35 +0200, Wolfgang Denk wrote:
> In message <1208207184.5911.26.camel@gentoo-jocke.transmode.se> you wrote:
> > 
> > It is a step closer towards full relocation of u-boot, I want to get rid
> > of using global data while in FLASH.
> 
> I doubt that this will work, but I'd love to be surprised :-)

It is not going to be a walk in the park :) The big remaining part
is string literals while still in flash, not sure how to solve these
yet.

> 
> > > The original idea of having such a list  of  funtion  pointers  which
> > > just  get executed one after another was to be able to wrap this into
> > > some "#ifndef CONFIG_INIT_SEQUENCE" and use this to allow for  board-
> > > specific init sequences by just adding a #define with the needed list
> > > of functions to the board config files.
> > 
> > You can do that with weak functions too. Just make all the functions
> > weak, then a board can overide with its own function.
> 
> That would not, for example, to allow to change the sequence - say
> one board needs to initialize PCI very early, but another one very
> late.

True, but as no one even uses this code ATM, it can't be a big deal.

 Jocke

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function.
  2008-04-14 21:01               ` Joakim Tjernlund
@ 2008-04-18  4:14                 ` Wolfgang Denk
  0 siblings, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2008-04-18  4:14 UTC (permalink / raw)
  To: u-boot

In message <1208206910.5911.20.camel@gentoo-jocke.transmode.se> you wrote:
...
> > Could you please update and resubmit? Thanks in advance.
> 
> OK, here it is.
> 
> >From 96fdcd78686c22aa6be56575c9f37d1943c91222 Mon Sep 17 00:00:00 2001
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Mon, 14 Apr 2008 22:59:00 +0200
> Subject: [PATCH] Change env_get_char from a global function ptr to a function.
> 
> This avoids an early global data reference.
> ---
>  api/api.c           |    1 -
>  common/cmd_nvedit.c |    3 ---
>  common/env_common.c |   19 +++++++++++++------
>  common/env_eeprom.c |    1 -
>  common/env_nvram.c  |    1 -
>  common/ft_build.c   |    2 --
>  include/common.h    |    1 +
>  7 files changed, 14 insertions(+), 14 deletions(-)

Thanks, applied.

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
A stone was placed at a ford in a river with the inscription:
"When this stone is covered it is dangerous to ford here."

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot-Users] [PATCH] Move init_sequence table into code.
  2008-04-15  6:40       ` Joakim Tjernlund
@ 2008-04-18  4:14         ` Wolfgang Denk
  0 siblings, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2008-04-18  4:14 UTC (permalink / raw)
  To: u-boot

In message <1208241621.5911.32.camel@gentoo-jocke.transmode.se> you wrote:
> 
> It is not going to be a walk in the park :) The big remaining part
> is string literals while still in flash, not sure how to solve these
> yet.
> 
> > 
> > > > The original idea of having such a list  of  funtion  pointers  which
> > > > just  get executed one after another was to be able to wrap this into
> > > > some "#ifndef CONFIG_INIT_SEQUENCE" and use this to allow for  board-
> > > > specific init sequences by just adding a #define with the needed list
> > > > of functions to the board config files.
> > > 
> > > You can do that with weak functions too. Just make all the functions
> > > weak, then a board can overide with its own function.
> > 
> > That would not, for example, to allow to change the sequence - say
> > one board needs to initialize PCI very early, but another one very
> > late.
> 
> True, but as no one even uses this code ATM, it can't be a big deal.

OK. Well, this is not a bug fix, but a (more or less invasive) change
to basic infrastructure. I will not add this so shortly before a
release. Please rebase and resubmit when the next merge window is
open.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"You ain't experienced..." "Well, nor are you." "That's true. But the
point is ... the point is ... the point is we've been not experienced
for a lot longer than you. We've got  a  lot  of  experience  of  not
having any experience."           - Terry Pratchett, _Witches Abroad_

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2008-04-18  4:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-28 15:45 [U-Boot-Users] [PATCH] Move init_sequence table into code Joakim Tjernlund
2008-03-28 15:45 ` [U-Boot-Users] [PATCH] Change env_get_char from a global function ptr to a function Joakim Tjernlund
2008-03-30 18:08   ` Jean-Christophe PLAGNIOL-VILLARD
2008-03-30 20:59   ` Wolfgang Denk
2008-03-30 21:05     ` Joakim Tjernlund
2008-03-30 22:33       ` Wolfgang Denk
2008-03-31  7:15         ` Joakim Tjernlund
2008-04-05 17:31           ` Joakim Tjernlund
2008-04-14  1:09             ` Wolfgang Denk
2008-04-14 21:01               ` Joakim Tjernlund
2008-04-18  4:14                 ` Wolfgang Denk
2008-04-14  1:09 ` [U-Boot-Users] [PATCH] Move init_sequence table into code Wolfgang Denk
2008-04-14 21:06   ` Joakim Tjernlund
2008-04-15  4:35     ` Wolfgang Denk
2008-04-15  6:40       ` Joakim Tjernlund
2008-04-18  4:14         ` Wolfgang Denk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox