diff mbox

[U-Boot] net: move bootfile/ipaddr init into eth_initialize

Message ID 1326670329-18644-1-git-send-email-vapier@gentoo.org
State Rejected
Headers show

Commit Message

Mike Frysinger Jan. 15, 2012, 11:32 p.m. UTC 
All arches init these variables the same way, so move the logic
into the core net code to avoid duplicating it everywhere else.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/arm/lib/board.c        |   11 -----------
 arch/avr32/lib/board.c      |    5 -----
 arch/blackfin/lib/board.c   |    9 +--------
 arch/m68k/lib/board.c       |   14 --------------
 arch/microblaze/lib/board.c |    7 -------
 arch/mips/lib/board.c       |   11 -----------
 arch/nds32/lib/board.c      |    9 ---------
 arch/nios2/lib/board.c      |    2 --
 arch/powerpc/lib/board.c    |   11 -----------
 arch/sandbox/lib/board.c    |    3 ---
 arch/sh/lib/board.c         |   22 ++--------------------
 arch/sparc/lib/board.c      |    7 -------
 arch/x86/lib/board.c        |   12 ------------
 net/eth.c                   |   12 ++++++++++++
 14 files changed, 15 insertions(+), 120 deletions(-)

Comments

Thomas Chou Jan. 16, 2012, 2:28 a.m. UTC | #1 
Hi Mike,

On 01/16/2012 07:32 AM, Mike Frysinger wrote:
> All arches init these variables the same way, so move the logic
> into the core net code to avoid duplicating it everywhere else.
>
> Signed-off-by: Mike Frysinger<vapier@gentoo.org>
> ---
>   arch/arm/lib/board.c        |   11 -----------
>   arch/avr32/lib/board.c      |    5 -----
>   arch/blackfin/lib/board.c   |    9 +--------
>   arch/m68k/lib/board.c       |   14 --------------
>   arch/microblaze/lib/board.c |    7 -------
>   arch/mips/lib/board.c       |   11 -----------
>   arch/nds32/lib/board.c      |    9 ---------
>   arch/nios2/lib/board.c      |    2 --
>   arch/powerpc/lib/board.c    |   11 -----------
>   arch/sandbox/lib/board.c    |    3 ---
>   arch/sh/lib/board.c         |   22 ++--------------------
>   arch/sparc/lib/board.c      |    7 -------
>   arch/x86/lib/board.c        |   12 ------------
>   net/eth.c                   |   12 ++++++++++++
>   14 files changed, 15 insertions(+), 120 deletions(-)

Tested on nios2 boards.

Tested-by: Thomas Chou <thomas@wytron.com.tw>

Best regards,
Thomas
Wolfgang Denk Jan. 16, 2012, 8:03 a.m. UTC | #2 
Dear Mike Frysinger,

In message <1326670329-18644-1-git-send-email-vapier@gentoo.org> you wrote:
> All arches init these variables the same way, so move the logic
> into the core net code to avoid duplicating it everywhere else.

This is the wrong approach.

There are many more ieces of code in arch/*/lib/board.c which are
duplicated across some or all architectures.  Instread of ripping
these apart and fixing a bit here and a bit thre we should combine
efforts and merge all arch/*/lib/board.c into a common file.


Additionally, your patch potentially breaks a number of boards.

> index 3d78274..2c4276b 100644
> --- a/arch/arm/lib/board.c
> +++ b/arch/arm/lib/board.c
> @@ -536,9 +536,6 @@ void board_init_r(gd_t *id, ulong dest_addr)
>  	arm_pci_init();
>  #endif
>  
> -	/* IP Address */
> -	gd->bd->bi_ip_addr = getenv_IPaddr("ipaddr");

The code which you remove here does NOT depend on CONFIG_CMD_NET;
note that this is intentional.

When moving this into net/eth.c, it will be missing for all oards that
do not have CONFIG_CMD_NET defined.

So NAK.

Best regards,

Wolfgang Denk
Mike Frysinger Jan. 16, 2012, 10:05 a.m. UTC | #3 
On Monday 16 January 2012 03:03:23 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > All arches init these variables the same way, so move the logic
> > into the core net code to avoid duplicating it everywhere else.
> 
> This is the wrong approach.
> 
> There are many more ieces of code in arch/*/lib/board.c which are
> duplicated across some or all architectures.  Instread of ripping
> these apart and fixing a bit here and a bit thre we should combine
> efforts and merge all arch/*/lib/board.c into a common file.

or, if we add bit by bit, we have an easily testable solution rather than one 
giant leap

> > --- a/arch/arm/lib/board.c
> > +++ b/arch/arm/lib/board.c
> > @@ -536,9 +536,6 @@ void board_init_r(gd_t *id, ulong dest_addr)
> > 
> >  	arm_pci_init();
> >  
> >  #endif
> > 
> > -	/* IP Address */
> > -	gd->bd->bi_ip_addr = getenv_IPaddr("ipaddr");
> 
> The code which you remove here does NOT depend on CONFIG_CMD_NET;
> note that this is intentional.

i noticed that, however, i'm not sure it matters.  i can't find any place in u-
boot that actually reads this variable.

arch/*/include/asm/u-boot.h: bi_ip_addr is declared as part of bd_t
arch/*/lib/board.c: bi_ip_addr gets initialized based on env
common/cmd_bdinfo.c: the value of bi_ip_addr gets displayed
common/cmd_nvedit.c: bi_ip_addr gets written when user does "setenv ipaddr"
board/esd/cpci405/cpci405.c: bi_ip_addr is written via the env
net/net.c: bi_ip_addr gets written as part of global env sync

is there any reason i shouldn't just rip out all bi_ip_addr handling ?
-mike
Wolfgang Denk Jan. 16, 2012, 11:18 a.m. UTC | #4 
Dear Mike Frysinger,

In message <201201160505.17834.vapier@gentoo.org> you wrote:
>
> > The code which you remove here does NOT depend on CONFIG_CMD_NET;
> > note that this is intentional.
>
> i noticed that, however, i'm not sure it matters.  i can't find any place in u-
> boot that actually reads this variable.

For example, the "bdinfo" command prints it; see common/cmd_bdinfo.c

Best regards,

Wolfgang Denk
Mike Frysinger Jan. 16, 2012, 7:59 p.m. UTC | #5 
On Monday 16 January 2012 06:18:26 Wolfgang Denk wrote:
> Mike Frysinger wrote:
> > > The code which you remove here does NOT depend on CONFIG_CMD_NET;
> > > note that this is intentional.
> > 
> > i noticed that, however, i'm not sure it matters.  i can't find any place
> > in u- boot that actually reads this variable.
> 
> For example, the "bdinfo" command prints it; see common/cmd_bdinfo.c

which i documented as the *only* place where the member gets read:
	common/cmd_bdinfo.c: the value of bi_ip_addr gets displayed

and that would be absolutely trivial to replace with getenv("ipaddr")
-mike
diff mbox

Patch

diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
index 3d78274..2c4276b 100644
--- a/arch/arm/lib/board.c
+++ b/arch/arm/lib/board.c
@@ -536,9 +536,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 	arm_pci_init();
 #endif
 
-	/* IP Address */
-	gd->bd->bi_ip_addr = getenv_IPaddr("ipaddr");
-
 	stdio_init();	/* get the devices list going. */
 
 	jumptable_init();
@@ -576,14 +573,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 
 	/* Initialize from environment */
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
-#if defined(CONFIG_CMD_NET)
-	{
-		char *s = getenv("bootfile");
-
-		if (s != NULL)
-			copy_filename(BootFile, s, sizeof(BootFile));
-	}
-#endif
 
 #ifdef CONFIG_BOARD_LATE_INIT
 	board_late_init();
diff --git a/arch/avr32/lib/board.c b/arch/avr32/lib/board.c
index 63fe297..4bc3c8a 100644
--- a/arch/avr32/lib/board.c
+++ b/arch/avr32/lib/board.c
@@ -304,8 +304,6 @@  void board_init_r(gd_t *new_gd, ulong dest_addr)
 	/* initialize environment */
 	env_relocate();
 
-	bd->bi_ip_addr = getenv_IPaddr ("ipaddr");
-
 	stdio_init();
 	jumptable_init();
 	console_init_r();
@@ -317,9 +315,6 @@  void board_init_r(gd_t *new_gd, ulong dest_addr)
 	bb_miiphy_init();
 #endif
 #if defined(CONFIG_CMD_NET)
-	s = getenv("bootfile");
-	if (s)
-		copy_filename(BootFile, s, sizeof(BootFile));
 	puts("Net:   ");
 	eth_initialize(gd->bd);
 #endif
diff --git a/arch/blackfin/lib/board.c b/arch/blackfin/lib/board.c
index e3ee4cd..4310fef 100644
--- a/arch/blackfin/lib/board.c
+++ b/arch/blackfin/lib/board.c
@@ -294,15 +294,8 @@  static void board_net_init_r(bd_t *bd)
 	bb_miiphy_init();
 #endif
 #ifdef CONFIG_CMD_NET
-	char *s;
-
-	if ((s = getenv("bootfile")) != NULL)
-		copy_filename(BootFile, s, sizeof(BootFile));
-
-	bd->bi_ip_addr = getenv_IPaddr("ipaddr");
-
 	printf("Net:   ");
-	eth_initialize(gd->bd);
+	eth_initialize(bd);
 #endif
 }
 
diff --git a/arch/m68k/lib/board.c b/arch/m68k/lib/board.c
index 259b71c..b3dca77 100644
--- a/arch/m68k/lib/board.c
+++ b/arch/m68k/lib/board.c
@@ -507,15 +507,6 @@  void board_init_r (gd_t *id, ulong dest_addr)
 	/* relocate environment function pointers etc. */
 	env_relocate ();
 
-	/*
-	 * Fill in missing fields of bd_info.
-	 * We do this here, where we have "normal" access to the
-	 * environment; we used to do this still running from ROM,
-	 * where had to use getenv_f(), which can be pretty slow when
-	 * the environment is in EEPROM.
-	 */
-	bd->bi_ip_addr = getenv_IPaddr ("ipaddr");
-
 	WATCHDOG_RESET ();
 
 #if defined(CONFIG_PCI)
@@ -568,11 +559,6 @@  void board_init_r (gd_t *id, ulong dest_addr)
 
 	/* Initialize from environment */
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
-#if defined(CONFIG_CMD_NET)
-	if ((s = getenv ("bootfile")) != NULL) {
-		copy_filename (BootFile, s, sizeof (BootFile));
-	}
-#endif
 
 	WATCHDOG_RESET ();
 
diff --git a/arch/microblaze/lib/board.c b/arch/microblaze/lib/board.c
index 9828b76..f3679d5 100644
--- a/arch/microblaze/lib/board.c
+++ b/arch/microblaze/lib/board.c
@@ -176,19 +176,12 @@  void board_init (void)
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
 
 #if defined(CONFIG_CMD_NET)
-	/* IP Address */
-	bd->bi_ip_addr = getenv_IPaddr("ipaddr");
-
 	printf("Net:   ");
 	eth_initialize(gd->bd);
 
 	uchar enetaddr[6];
 	eth_getenv_enetaddr("ethaddr", enetaddr);
 	printf("MAC:   %pM\n", enetaddr);
-
-	s = getenv("bootfile");
-	if (s != NULL)
-		copy_filename(BootFile, s, sizeof(BootFile));
 #endif
 
 	/* main_loop */
diff --git a/arch/mips/lib/board.c b/arch/mips/lib/board.c
index d998f0e..6b36a98 100644
--- a/arch/mips/lib/board.c
+++ b/arch/mips/lib/board.c
@@ -316,9 +316,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 	/* relocate environment function pointers etc. */
 	env_relocate();
 
-	/* IP Address */
-	bd->bi_ip_addr = getenv_IPaddr("ipaddr");
-
 #if defined(CONFIG_PCI)
 	/*
 	 * Do pci configuration
@@ -338,14 +335,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 
 	/* Initialize from environment */
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
-#if defined(CONFIG_CMD_NET)
-	{
-		char *s = getenv("bootfile");
-
-		if (s != NULL)
-			copy_filename(BootFile, s, sizeof(BootFile));
-	}
-#endif
 
 #ifdef CONFIG_CMD_SPI
 	puts("SPI:   ");
diff --git a/arch/nds32/lib/board.c b/arch/nds32/lib/board.c
index 66e4537..446e0fd 100644
--- a/arch/nds32/lib/board.c
+++ b/arch/nds32/lib/board.c
@@ -368,9 +368,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 	nds32_pci_init();
 #endif
 
-	/* IP Address */
-	gd->bd->bi_ip_addr = getenv_IPaddr("ipaddr");
-
 	stdio_init();	/* get the devices list going. */
 
 	jumptable_init();
@@ -401,12 +398,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 	/* Initialize from environment */
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
 
-#if defined(CONFIG_CMD_NET)
-	s = getenv("bootfile");
-	if (s != NULL)
-		copy_filename(BootFile, s, sizeof(BootFile));
-#endif
-
 #ifdef BOARD_LATE_INIT
 	board_late_init();
 #endif
diff --git a/arch/nios2/lib/board.c b/arch/nios2/lib/board.c
index 65de26e..ca8a3e5 100644
--- a/arch/nios2/lib/board.c
+++ b/arch/nios2/lib/board.c
@@ -143,8 +143,6 @@  void board_init (void)
 	WATCHDOG_RESET ();
 	env_relocate();
 
-	bd->bi_ip_addr = getenv_IPaddr ("ipaddr");
-
 	WATCHDOG_RESET ();
 	stdio_init();
 	jumptable_init();
diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
index ff5888e..931b098 100644
--- a/arch/powerpc/lib/board.c
+++ b/arch/powerpc/lib/board.c
@@ -877,9 +877,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 #endif
 #endif /* CONFIG_CMD_NET */
 
-	/* IP Address */
-	bd->bi_ip_addr = getenv_IPaddr("ipaddr");
-
 	WATCHDOG_RESET();
 
 #if defined(CONFIG_PCI) && !defined(CONFIG_SYS_EARLY_PCI_INIT)
@@ -935,14 +932,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 
 	/* Initialize from environment */
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
-#if defined(CONFIG_CMD_NET)
-	{
-		char *s = getenv("bootfile");
-
-		if (s != NULL)
-			copy_filename(BootFile, s, sizeof(BootFile));
-	}
-#endif
 
 	WATCHDOG_RESET();
 
diff --git a/arch/sandbox/lib/board.c b/arch/sandbox/lib/board.c
index b7997e9..a71b1ba 100644
--- a/arch/sandbox/lib/board.c
+++ b/arch/sandbox/lib/board.c
@@ -226,9 +226,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 	/* initialize environment */
 	env_relocate();
 
-	/* IP Address */
-	gd->bd->bi_ip_addr = getenv_IPaddr("ipaddr");
-
 	stdio_init();	/* get the devices list going. */
 
 	jumptable_init();
diff --git a/arch/sh/lib/board.c b/arch/sh/lib/board.c
index d9c0c22..e1a5739 100644
--- a/arch/sh/lib/board.c
+++ b/arch/sh/lib/board.c
@@ -99,14 +99,6 @@  static int sh_mem_env_init(void)
 	return 0;
 }
 
-#if defined(CONFIG_CMD_NET)
-static int sh_net_init(void)
-{
-	gd->bd->bi_ip_addr = getenv_IPaddr("ipaddr");
-	return 0;
-}
-#endif
-
 #if defined(CONFIG_CMD_MMC)
 static int sh_mmc_init(void)
 {
@@ -144,9 +136,6 @@  init_fnc_t *init_sequence[] =
 #ifdef CONFIG_BOARD_LATE_INIT
 	board_late_init,
 #endif
-#if defined(CONFIG_CMD_NET)
-	sh_net_init,		/* SH specific eth init */
-#endif
 #if defined(CONFIG_CMD_MMC)
 	sh_mmc_init,
 #endif
@@ -200,15 +189,8 @@  void sh_generic_init(void)
 	bb_miiphy_init();
 #endif
 #if defined(CONFIG_CMD_NET)
-	{
-		char *s;
-		puts("Net:   ");
-		eth_initialize(gd->bd);
-
-		s = getenv("bootfile");
-		if (s != NULL)
-			copy_filename(BootFile, s, sizeof(BootFile));
-	}
+	puts("Net:   ");
+	eth_initialize(gd->bd);
 #endif /* CONFIG_CMD_NET */
 
 	while (1) {
diff --git a/arch/sparc/lib/board.c b/arch/sparc/lib/board.c
index 519a4fb..a36f165 100644
--- a/arch/sparc/lib/board.c
+++ b/arch/sparc/lib/board.c
@@ -333,8 +333,6 @@  void board_init_f(ulong bootflag)
 	mac_read_from_eeprom();
 #endif
 
-	/* IP Address */
-	bd->bi_ip_addr = getenv_IPaddr("ipaddr");
 #if defined(CONFIG_PCI)
 	/*
 	 * Do pci configuration
@@ -359,11 +357,6 @@  void board_init_f(ulong bootflag)
 
 	/* Initialize from environment */
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
-#if defined(CONFIG_CMD_NET)
-	if ((s = getenv("bootfile")) != NULL) {
-		copy_filename(BootFile, s, sizeof(BootFile));
-	}
-#endif /* CONFIG_CMD_NET */
 
 	WATCHDOG_RESET();
 
diff --git a/arch/x86/lib/board.c b/arch/x86/lib/board.c
index d742fec..595b3c9 100644
--- a/arch/x86/lib/board.c
+++ b/arch/x86/lib/board.c
@@ -321,12 +321,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 	env_relocate();
 	show_boot_progress(0x26);
 
-
-#ifdef CONFIG_CMD_NET
-	/* IP Address */
-	bd_data.bi_ip_addr = getenv_IPaddr("ipaddr");
-#endif
-
 #if defined(CONFIG_PCI)
 	/*
 	 * Do pci configuration
@@ -373,12 +367,6 @@  void board_init_r(gd_t *id, ulong dest_addr)
 
 	/* Initialize from environment */
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
-#if defined(CONFIG_CMD_NET)
-	s = getenv("bootfile");
-
-	if (s != NULL)
-		copy_filename(BootFile, s, sizeof(BootFile));
-#endif
 
 	WATCHDOG_RESET();
 
diff --git a/net/eth.c b/net/eth.c
index f3a55ba..c5daee1 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -224,6 +224,16 @@  int eth_register(struct eth_device *dev)
 	return 0;
 }
 
+static void eth_env_init(bd_t *bis)
+{
+	const char *s;
+
+	if ((s = getenv("bootfile")) != NULL)
+		copy_filename(BootFile, s, sizeof(BootFile));
+
+	bis->bi_ip_addr = getenv_IPaddr("ipaddr");
+}
+
 int eth_initialize(bd_t *bis)
 {
 	int num_devices = 0;
@@ -239,6 +249,8 @@  int eth_initialize(bd_t *bis)
 	phy_init();
 #endif
 
+	eth_env_init(bis);
+
 	/*
 	 * If board-specific initialization exists, call it.
 	 * If not, call a CPU-specific one