[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[richardretanubun]

Added for convenience for other platforms that uses MPC8360 (has 8 UCC).
6 eth interface is chosen because the platform I am using combines
UCC1&2 and UCC3&4 as gigEth and the other 4 UCC as 10/100 Eth.

- Richard

From: Richard Retanubun <RichardRetanubun@RugggedCom.com>
Date: Wed, 24 Sep 2008 17:21:47 -0400
Subject: [PATCH] Add two more ethernet interface for 83XX
Signed-off-by: Richard Retanubun <RichardRetanubun@ruggedcom.com>
---
 README                   |    3 ++
 common/cmd_bdinfo.c      |   17 +++++++++++++++-
 common/env_common.c      |    6 +++++
 common/env_embedded.c    |    6 +++++
 cpu/mpc83xx/fdt.c        |    3 +-
 drivers/qe/uec.c         |   48 +++++++++++++++++++++++++++++++++++++++++++++-
 include/asm-ppc/u-boot.h |    6 +++++
 lib_ppc/board.c          |   30 ++++++++++++++++++++++++++++
 net/eth.c                |    6 +++++
 tools/env/fw_env.c       |    6 +++++
 10 files changed, 128 insertions(+), 3 deletions(-)

diff --git a/README b/README
index ccd839c..8802304 100644
--- a/README
+++ b/README
@@ -1095,8 +1095,11 @@ The following options need to be configured:
 
 - Ethernet address:
 		CONFIG_ETHADDR
+		CONFIG_ETH1ADDR
 		CONFIG_ETH2ADDR
 		CONFIG_ETH3ADDR
+		CONFIG_ETH4ADDR
+		CONFIG_ETH5ADDR
 
 		Define a default value for Ethernet address to use
 		for the respective Ethernet interface, in case this
diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
index f4d9d40..67cc64f 100644
--- a/common/cmd_bdinfo.c
+++ b/common/cmd_bdinfo.c
@@ -91,11 +91,12 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	print_str ("pevfreq",	    strmhz(buf, bd->bi_pevfreq));
 #endif
 
+#if defined(CONFIG_HAS_ETH0)
 	puts ("ethaddr     =");
 	for (i=0; i<6; ++i) {
 		printf ("%c%02X", i ? ':' : ' ', bd->bi_enetaddr[i]);
 	}
-
+#endif
 #if defined(CONFIG_HAS_ETH1)
 	puts ("\neth1addr    =");
 	for (i=0; i<6; ++i) {
@@ -117,6 +118,20 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	}
 #endif
 
+#if defined(CONFIG_HAS_ETH4)
+       puts ("\neth4addr    =");
+       for (i=0; i<6; ++i) {
+		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet4addr[i]);
+	}
+#endif
+
+#if defined(CONFIG_HAS_ETH5)
+       puts ("\neth5addr    =");
+       for (i=0; i<6; ++i) {
+		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet5addr[i]);
+	}
+#endif
+
 #ifdef CONFIG_HERMES
 	print_str ("ethspeed",	    strmhz(buf, bd->bi_ethspeed));
 #endif
diff --git a/common/env_common.c b/common/env_common.c
index 77f9944..0fee3af 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -91,6 +91,12 @@ uchar default_environment[] = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
+#endif
 #ifdef	CONFIG_IPADDR
 	"ipaddr="	MK_STR(CONFIG_IPADDR)		"\0"
 #endif
diff --git a/common/env_embedded.c b/common/env_embedded.c
index 77e5619..e79f843 100644
--- a/common/env_embedded.c
+++ b/common/env_embedded.c
@@ -135,6 +135,12 @@ env_t environment __PPCENV__ = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
+#endif
 #ifdef	CONFIG_ETHPRIME
 	"ethprime="	CONFIG_ETHPRIME			"\0"
 #endif
diff --git a/cpu/mpc83xx/fdt.c b/cpu/mpc83xx/fdt.c
index 39bd9dc..3e3e1c8 100644
--- a/cpu/mpc83xx/fdt.c
+++ b/cpu/mpc83xx/fdt.c
@@ -52,7 +52,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 		fdt_fixup_crypto_node(blob, 0x0204);
 
 #if defined(CONFIG_HAS_ETH0) || defined(CONFIG_HAS_ETH1) ||\
-    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3)
+    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) ||\
+    defined(CONFIG_HAS_ETH4) || defined(CONFIG_HAS_ETH5)
 	fdt_fixup_ethernet(blob);
 #endif
 
diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
index 344c649..e1dec5e 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -123,8 +123,54 @@ static uec_info_t eth4_uec_info = {
 	.enet_interface		= CFG_UEC4_INTERFACE_MODE,
 };
 #endif
+#ifdef CONFIG_UEC_ETH5
+static uec_info_t eth5_uec_info = {
+	.uf_info		= {
+		.ucc_num	= CFG_UEC5_UCC_NUM,
+		.rx_clock	= CFG_UEC5_RX_CLK,
+		.tx_clock	= CFG_UEC5_TX_CLK,
+		.eth_type	= CFG_UEC5_ETH_TYPE,
+	},
+#if (CFG_UEC5_ETH_TYPE == FAST_ETH)
+	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
+#else
+	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
+#endif
+	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.tx_bd_ring_len		= 16,
+	.rx_bd_ring_len		= 16,
+	.phy_address		= CFG_UEC5_PHY_ADDR,
+	.enet_interface		= CFG_UEC5_INTERFACE_MODE,
+};
+#endif
+#ifdef CONFIG_UEC_ETH6
+static uec_info_t eth6_uec_info = {
+	.uf_info		= {
+		.ucc_num	= CFG_UEC6_UCC_NUM,
+		.rx_clock	= CFG_UEC6_RX_CLK,
+		.tx_clock	= CFG_UEC6_TX_CLK,
+		.eth_type	= CFG_UEC6_ETH_TYPE,
+	},
+#if (CFG_UEC6_ETH_TYPE == FAST_ETH)
+	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
+#else
+	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
+#endif
+	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.tx_bd_ring_len		= 16,
+	.rx_bd_ring_len		= 16,
+	.phy_address		= CFG_UEC6_PHY_ADDR,
+	.enet_interface		= CFG_UEC6_INTERFACE_MODE,
+};
+#endif
 
-#define MAXCONTROLLERS	(4)
+#define MAXCONTROLLERS	(6)
 
 static struct eth_device *devlist[MAXCONTROLLERS];
 
diff --git a/include/asm-ppc/u-boot.h b/include/asm-ppc/u-boot.h
index 54ac01d..7451905 100644
--- a/include/asm-ppc/u-boot.h
+++ b/include/asm-ppc/u-boot.h
@@ -111,6 +111,12 @@ typedef struct bd_info {
 #ifdef CONFIG_HAS_ETH3
 	unsigned char   bi_enet3addr[6];
 #endif
+#ifdef CONFIG_HAS_ETH4
+	unsigned char   bi_enet4addr[6];
+#endif
+#ifdef CONFIG_HAS_ETH5
+	unsigned char   bi_enet5addr[6];
+#endif
 
 #if defined(CONFIG_405GP) || defined(CONFIG_405EP) || \
     defined(CONFIG_405EZ) || defined(CONFIG_440GX) || \
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index c02ac62..d440722 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -944,6 +944,36 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	}
 #endif
 
+#ifdef CONFIG_HAS_ETH4
+	/* handle 5th ethernet address */
+	s = getenv("eth4addr");
+#if defined(CONFIG_XPEDITE1K) || defined(CONFIG_METROBOX) || defined(CONFIG_KAREF)
+	if (s == NULL)
+		board_get_enetaddr(bd->bi_enet4addr);
+	else
+#endif
+	for (i = 0; i < 6; ++i) {
+		bd->bi_enet4addr[i] = s ? simple_strtoul (s, &e, 16) : 0;
+		if (s)
+			s = (*e) ? e + 1 : e;
+	}
+#endif
+
+#ifdef CONFIG_HAS_ETH5
+	/* handle 6th ethernet address */
+	s = getenv("eth5addr");
+#if defined(CONFIG_XPEDITE1K) || defined(CONFIG_METROBOX) || defined(CONFIG_KAREF)
+	if (s == NULL)
+		board_get_enetaddr(bd->bi_enet5addr);
+	else
+#endif
+	for (i = 0; i < 6; ++i) {
+		bd->bi_enet5addr[i] = s ? simple_strtoul (s, &e, 16) : 0;
+		if (s)
+			s = (*e) ? e + 1 : e;
+	}
+#endif
+
 #if defined(CONFIG_TQM8xxL) || defined(CONFIG_TQM8260) || \
     defined(CONFIG_TQM8272) || \
     defined(CONFIG_CCM) || defined(CONFIG_KUP4K) || \
diff --git a/net/eth.c b/net/eth.c
index 432dd60..ccd871a 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -180,6 +180,12 @@ int eth_initialize(bd_t *bis)
 #if defined(CONFIG_UEC_ETH4)
 	uec_initialize(3);
 #endif
+#if defined(CONFIG_UEC_ETH5)
+	uec_initialize(4);
+#endif
+#if defined(CONFIG_UEC_ETH6)
+	uec_initialize(5);
+#endif
 
 #if defined(FEC_ENET) || defined(CONFIG_ETHER_ON_FCC)
 	fec_initialize(bis);
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 46747d3..6e9c34f 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -157,6 +157,12 @@ static char default_environment[] = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr=" MK_STR (CONFIG_ETH3ADDR) "\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr=" MK_STR (CONFIG_ETH4ADDR) "\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr=" MK_STR (CONFIG_ETH5ADDR) "\0"
+#endif
 #ifdef	CONFIG_ETHPRIME
 	"ethprime=" CONFIG_ETHPRIME "\0"
 #endif
-- 
1.5.5.GIT

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Kim Phillips]

On Thu, 25 Sep 2008 08:53:24 -0400
richardretanubun <richardretanubun@ruggedcom.com> wrote:

> Added for convenience for other platforms that uses MPC8360 (has 8 UCC).
> 6 eth interface is chosen because the platform I am using combines
> UCC1&2 and UCC3&4 as gigEth and the other 4 UCC as 10/100 Eth.
> 
> - Richard
> 
> From: Richard Retanubun <RichardRetanubun@RugggedCom.com>
> Date: Wed, 24 Sep 2008 17:21:47 -0400
> Subject: [PATCH] Add two more ethernet interface for 83XX
> Signed-off-by: Richard Retanubun <RichardRetanubun@ruggedcom.com>

Unless you're sending a patch on someone else's behalf, you don't need
the From:, Date:, and Subject: lines above, esp. since your From email
does not match your S-o-b email.  Please also omit the "- Richard" from
the git commit part of the patch; if you want say something in addition
to the commit message, put it below the '---' line..

> ---

..i.e, here.

>  README                   |    3 ++
>  common/cmd_bdinfo.c      |   17 +++++++++++++++-
>  common/env_common.c      |    6 +++++
>  common/env_embedded.c    |    6 +++++
>  cpu/mpc83xx/fdt.c        |    3 +-
>  drivers/qe/uec.c         |   48 +++++++++++++++++++++++++++++++++++++++++++++-
>  include/asm-ppc/u-boot.h |    6 +++++
>  lib_ppc/board.c          |   30 ++++++++++++++++++++++++++++
>  net/eth.c                |    6 +++++
>  tools/env/fw_env.c       |    6 +++++
>  10 files changed, 128 insertions(+), 3 deletions(-)

just because 83xx is the first user doesn't mean it has to go
through the 83xx tree.  This patch is really not 83xx specific at all
and should probably go through net (Ben Warren) if not WD himself.

Having said that, this patch does transcend 4 subsystem areas, so if
Ben/gvb/WD want to ack/sign off on it, I can handle pushing this
upstream.

> diff --git a/README b/README
> index ccd839c..8802304 100644
> --- a/README
> +++ b/README
> @@ -1095,8 +1095,11 @@ The following options need to be configured:
>  
>  - Ethernet address:
>  		CONFIG_ETHADDR
> +		CONFIG_ETH1ADDR
>  		CONFIG_ETH2ADDR

hmm..historically ETHADDR has been the implicit ETH1ADDR.  Did you mean
to s/ETHADDR/ETH1ADDR/ ?  if so, you'd need a better justification and
a much larger patch.  Otherwise, please don't do this; add a
CONFIG_ETH6ADDR below instead.

>  		CONFIG_ETH3ADDR
> +		CONFIG_ETH4ADDR
> +		CONFIG_ETH5ADDR
>  
>  		Define a default value for Ethernet address to use
>  		for the respective Ethernet interface, in case this
> diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
> index f4d9d40..67cc64f 100644
> --- a/common/cmd_bdinfo.c
> +++ b/common/cmd_bdinfo.c
> @@ -91,11 +91,12 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  	print_str ("pevfreq",	    strmhz(buf, bd->bi_pevfreq));
>  #endif
>  
> +#if defined(CONFIG_HAS_ETH0)
>  	puts ("ethaddr     =");
>  	for (i=0; i<6; ++i) {
>  		printf ("%c%02X", i ? ':' : ' ', bd->bi_enetaddr[i]);
>  	}
> -
> +#endif

how is the above change relevant to the patch subject?

Thanks,

Kim

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[richardretanubun]

Kim Phillips wrote:

> On Thu, 25 Sep 2008 08:53:24 -0400
> richardretanubun <richardretanubun@ruggedcom.com> wrote:
>
>> Added for convenience for other platforms that uses MPC8360 (has 8 UCC).
>> 6 eth interface is chosen because the platform I am using combines
>> UCC1&2 and UCC3&4 as gigEth and the other 4 UCC as 10/100 Eth.
>>
>> - Richard
>>
>> From: Richard Retanubun <RichardRetanubun@RugggedCom.com>
>> Date: Wed, 24 Sep 2008 17:21:47 -0400
>> Subject: [PATCH] Add two more ethernet interface for 83XX
>> Signed-off-by: Richard Retanubun <RichardRetanubun@ruggedcom.com>
>
> Unless you're sending a patch on someone else's behalf, you don't need
> the From:, Date:, and Subject: lines above, esp. since your From email
> does not match your S-o-b email.  Please also omit the "- Richard" from
> the git commit part of the patch; if you want say something in addition
> to the commit message, put it below the '---' line..
>
>> ---
>
> ..i.e, here.
>
Understood, thanks for the clarification, will heed for future patches.

>>  README                   |    3 ++
>>  common/cmd_bdinfo.c      |   17 +++++++++++++++-
>>  common/env_common.c      |    6 +++++
>>  common/env_embedded.c    |    6 +++++
>>  cpu/mpc83xx/fdt.c        |    3 +-
>>  drivers/qe/uec.c         |   48 +++++++++++++++++++++++++++++++++++++++++++++-
>>  include/asm-ppc/u-boot.h |    6 +++++
>>  lib_ppc/board.c          |   30 ++++++++++++++++++++++++++++
>>  net/eth.c                |    6 +++++
>>  tools/env/fw_env.c       |    6 +++++
>>  10 files changed, 128 insertions(+), 3 deletions(-)
>
> just because 83xx is the first user doesn't mean it has to go
> through the 83xx tree.  This patch is really not 83xx specific at all
> and should probably go through net (Ben Warren) if not WD himself.
>
> Having said that, this patch does transcend 4 subsystem areas, so if
> Ben/gvb/WD want to ack/sign off on it, I can handle pushing this
> upstream.
>
Thanks for the help, I realize this touches many subsystems, but I figured I start at the 83xx community
since (I think) it is the most probable community to find platforms with these many eth interfaces.

>> diff --git a/README b/README
>> index ccd839c..8802304 100644
>> --- a/README
>> +++ b/README
>> @@ -1095,8 +1095,11 @@ The following options need to be configured:
>>  
>>  - Ethernet address:
>>  		CONFIG_ETHADDR
>> +		CONFIG_ETH1ADDR
>>  		CONFIG_ETH2ADDR
>
> hmm..historically ETHADDR has been the implicit ETH1ADDR.  Did you mean
> to s/ETHADDR/ETH1ADDR/ ?  if so, you'd need a better justification and
> a much larger patch.  Otherwise, please don't do this; add a
> CONFIG_ETH6ADDR below instead.
>
I will add CONFIG_ETH6ADDR below.

>>  		CONFIG_ETH3ADDR
>> +		CONFIG_ETH4ADDR
>> +		CONFIG_ETH5ADDR
>>  
>>  		Define a default value for Ethernet address to use
>>  		for the respective Ethernet interface, in case this
>> diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
>> index f4d9d40..67cc64f 100644
>> --- a/common/cmd_bdinfo.c
>> +++ b/common/cmd_bdinfo.c
>> @@ -91,11 +91,12 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>>  	print_str ("pevfreq",	    strmhz(buf, bd->bi_pevfreq));
>>  #endif
>>  
>> +#if defined(CONFIG_HAS_ETH0)
>>  	puts ("ethaddr     =");
>>  	for (i=0; i<6; ++i) {
>>  		printf ("%c%02X", i ? ':' : ' ', bd->bi_enetaddr[i]);
>>  	}
>> -
>> +#endif
>
> how is the above change relevant to the patch subject?
Good catch, I lumped it together because I was in the code neighborhood 
got carried away in making the code uniform. I will pull it out of this patch.

Is the idea of adding an #ifdef here valid though? 
If it is, I can submit a separate patch for it.

>
> Thanks,
>
> Kim
Thanks for all the feedback

Richard

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Kim Phillips]

On Thu, 25 Sep 2008 16:25:20 -0400
richardretanubun <richardretanubun@ruggedcom.com> wrote:

> Kim Phillips wrote:
> 
> > On Thu, 25 Sep 2008 08:53:24 -0400
> > richardretanubun <richardretanubun@ruggedcom.com> wrote:
> >> ---
> >
> > ..i.e, here.
> >
> Understood, thanks for the clarification, will heed for future patches.

excellent, thanks.

> > Having said that, this patch does transcend 4 subsystem areas, so if
> > Ben/gvb/WD want to ack/sign off on it, I can handle pushing this
> > upstream.
> >
> Thanks for the help, I realize this touches many subsystems, but I figured I start at the 83xx community
> since (I think) it is the most probable community to find platforms with these many eth interfaces.

fyi, 85xx does too (and they even have more interface connections on
their boards).

> >> diff --git a/README b/README
> >> index ccd839c..8802304 100644
> >> --- a/README
> >> +++ b/README
> >> @@ -1095,8 +1095,11 @@ The following options need to be configured:
> >>  
> >>  - Ethernet address:
> >>  		CONFIG_ETHADDR
> >> +		CONFIG_ETH1ADDR
> >>  		CONFIG_ETH2ADDR
> >
> > hmm..historically ETHADDR has been the implicit ETH1ADDR.  Did you mean
> > to s/ETHADDR/ETH1ADDR/ ?  if so, you'd need a better justification and
> > a much larger patch.  Otherwise, please don't do this; add a
> > CONFIG_ETH6ADDR below instead.
> >
> I will add CONFIG_ETH6ADDR below.

wait, no, CONFIG_ETH1ADDR is indeed valid and being used.  It's good
you're adding this to the documentation to not confuse people like
myself - ack!  and no need to add ETH6ADDR below either.

> >> +++ b/common/cmd_bdinfo.c
> >> @@ -91,11 +91,12 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> >>  	print_str ("pevfreq",	    strmhz(buf, bd->bi_pevfreq));
> >>  #endif
> >>  
> >> +#if defined(CONFIG_HAS_ETH0)
> >>  	puts ("ethaddr     =");
> >>  	for (i=0; i<6; ++i) {
> >>  		printf ("%c%02X", i ? ':' : ' ', bd->bi_enetaddr[i]);
> >>  	}
> >> -
> >> +#endif
> >
> > how is the above change relevant to the patch subject?
> Good catch, I lumped it together because I was in the code neighborhood 
> got carried away in making the code uniform. I will pull it out of this patch.
> 
> Is the idea of adding an #ifdef here valid though? 
> If it is, I can submit a separate patch for it.

not sure; I seem to remember some code depending on its existence even
if the board didn't configure any interfaces, but, sure, send a patch
and I'm sure it'll get tested.  I'm assuming none of this is for 2008.10
release, btw.

Kim

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Wolfgang Denk]

Dear Kim Phillips,

In message <20080925154546.4ff2cae2.kim.phillips@freescale.com> you wrote:
>
> > >>  - Ethernet address:
> > >>  		CONFIG_ETHADDR
> > >> +		CONFIG_ETH1ADDR
> > >>  		CONFIG_ETH2ADDR
> > >
> > > hmm..historically ETHADDR has been the implicit ETH1ADDR.  Did you mean
> > > to s/ETHADDR/ETH1ADDR/ ?  if so, you'd need a better justification and
> > > a much larger patch.  Otherwise, please don't do this; add a
> > > CONFIG_ETH6ADDR below instead.

No, numbering starts at 0 and matches Linux interface names.

CONFIG_ETHADDR  => "ethaddr"  => "eth0"
CONFIG_ETH1ADDR => "eth1addr" => "eth1"
...
CONFIG_ETHNADDR => "ethNaddr" => "ethN"

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 v2] [83xx] Adds two more ethernet interface to 83xx

[richardretanubun]

Added for convenience for other platforms that uses MPC8360 (has eight UCC).
Six eth interface is chosen because the platform I am using combines
UCC1&2 and UCC3&4 as two gigEth and the other four UCC as 10/100 Eth.

Signed-off-by: Richard Retanubun <RichardRetanubun@RugggedCom.com>
---
V2 of this patch adds CONFIG_ETH6ADDR to README and removes 
#if defined(CONFIG_HAS_ETH0) from common/cmd_bdinfo.c

 README                   |    3 ++
 common/cmd_bdinfo.c      |   14 +++++++++++++
 common/env_common.c      |    9 ++++++++
 common/env_embedded.c    |    9 ++++++++
 cpu/mpc83xx/fdt.c        |    3 +-
 drivers/qe/uec.c         |   48 +++++++++++++++++++++++++++++++++++++++++++++-
 include/asm-ppc/u-boot.h |    6 +++++
 lib_ppc/board.c          |   30 ++++++++++++++++++++++++++++
 net/eth.c                |    6 +++++
 tools/env/fw_env.c       |    9 ++++++++
 10 files changed, 135 insertions(+), 2 deletions(-)

diff --git a/README b/README
index ccd839c..d623037 100644
--- a/README
+++ b/README
@@ -1097,6 +1097,9 @@ The following options need to be configured:
 		CONFIG_ETHADDR
 		CONFIG_ETH2ADDR
 		CONFIG_ETH3ADDR
+		CONFIG_ETH4ADDR
+		CONFIG_ETH5ADDR
+		CONFIG_ETH6ADDR
 
 		Define a default value for Ethernet address to use
 		for the respective Ethernet interface, in case this
diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
index f4d9d40..5018930 100644
--- a/common/cmd_bdinfo.c
+++ b/common/cmd_bdinfo.c
@@ -117,6 +117,20 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	}
 #endif
 
+#if defined(CONFIG_HAS_ETH4)
+       puts ("\neth4addr    =");
+       for (i=0; i<6; ++i) {
+		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet4addr[i]);
+	}
+#endif
+
+#if defined(CONFIG_HAS_ETH5)
+       puts ("\neth5addr    =");
+       for (i=0; i<6; ++i) {
+		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet5addr[i]);
+	}
+#endif
+
 #ifdef CONFIG_HERMES
 	print_str ("ethspeed",	    strmhz(buf, bd->bi_ethspeed));
 #endif
diff --git a/common/env_common.c b/common/env_common.c
index 77f9944..d536137 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -91,6 +91,15 @@ uchar default_environment[] = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
+#endif
+#ifdef	CONFIG_ETH6ADDR
+	"eth6addr="	MK_STR(CONFIG_ETH6ADDR)		"\0"
+#endif
 #ifdef	CONFIG_IPADDR
 	"ipaddr="	MK_STR(CONFIG_IPADDR)		"\0"
 #endif
diff --git a/common/env_embedded.c b/common/env_embedded.c
index 77e5619..4848bf1 100644
--- a/common/env_embedded.c
+++ b/common/env_embedded.c
@@ -135,6 +135,15 @@ env_t environment __PPCENV__ = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
+#endif
+#ifdef	CONFIG_ETH6ADDR
+	"eth6addr="	MK_STR(CONFIG_ETH6ADDR)		"\0"
+#endif
 #ifdef	CONFIG_ETHPRIME
 	"ethprime="	CONFIG_ETHPRIME			"\0"
 #endif
diff --git a/cpu/mpc83xx/fdt.c b/cpu/mpc83xx/fdt.c
index 39bd9dc..3e3e1c8 100644
--- a/cpu/mpc83xx/fdt.c
+++ b/cpu/mpc83xx/fdt.c
@@ -52,7 +52,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 		fdt_fixup_crypto_node(blob, 0x0204);
 
 #if defined(CONFIG_HAS_ETH0) || defined(CONFIG_HAS_ETH1) ||\
-    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3)
+    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) ||\
+    defined(CONFIG_HAS_ETH4) || defined(CONFIG_HAS_ETH5)
 	fdt_fixup_ethernet(blob);
 #endif
 
diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
index 344c649..e1dec5e 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -123,8 +123,54 @@ static uec_info_t eth4_uec_info = {
 	.enet_interface		= CFG_UEC4_INTERFACE_MODE,
 };
 #endif
+#ifdef CONFIG_UEC_ETH5
+static uec_info_t eth5_uec_info = {
+	.uf_info		= {
+		.ucc_num	= CFG_UEC5_UCC_NUM,
+		.rx_clock	= CFG_UEC5_RX_CLK,
+		.tx_clock	= CFG_UEC5_TX_CLK,
+		.eth_type	= CFG_UEC5_ETH_TYPE,
+	},
+#if (CFG_UEC5_ETH_TYPE == FAST_ETH)
+	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
+#else
+	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
+#endif
+	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.tx_bd_ring_len		= 16,
+	.rx_bd_ring_len		= 16,
+	.phy_address		= CFG_UEC5_PHY_ADDR,
+	.enet_interface		= CFG_UEC5_INTERFACE_MODE,
+};
+#endif
+#ifdef CONFIG_UEC_ETH6
+static uec_info_t eth6_uec_info = {
+	.uf_info		= {
+		.ucc_num	= CFG_UEC6_UCC_NUM,
+		.rx_clock	= CFG_UEC6_RX_CLK,
+		.tx_clock	= CFG_UEC6_TX_CLK,
+		.eth_type	= CFG_UEC6_ETH_TYPE,
+	},
+#if (CFG_UEC6_ETH_TYPE == FAST_ETH)
+	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
+#else
+	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
+#endif
+	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.tx_bd_ring_len		= 16,
+	.rx_bd_ring_len		= 16,
+	.phy_address		= CFG_UEC6_PHY_ADDR,
+	.enet_interface		= CFG_UEC6_INTERFACE_MODE,
+};
+#endif
 
-#define MAXCONTROLLERS	(4)
+#define MAXCONTROLLERS	(6)
 
 static struct eth_device *devlist[MAXCONTROLLERS];
 
diff --git a/include/asm-ppc/u-boot.h b/include/asm-ppc/u-boot.h
index 54ac01d..7451905 100644
--- a/include/asm-ppc/u-boot.h
+++ b/include/asm-ppc/u-boot.h
@@ -111,6 +111,12 @@ typedef struct bd_info {
 #ifdef CONFIG_HAS_ETH3
 	unsigned char   bi_enet3addr[6];
 #endif
+#ifdef CONFIG_HAS_ETH4
+	unsigned char   bi_enet4addr[6];
+#endif
+#ifdef CONFIG_HAS_ETH5
+	unsigned char   bi_enet5addr[6];
+#endif
 
 #if defined(CONFIG_405GP) || defined(CONFIG_405EP) || \
     defined(CONFIG_405EZ) || defined(CONFIG_440GX) || \
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index c02ac62..d440722 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -944,6 +944,36 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	}
 #endif
 
+#ifdef CONFIG_HAS_ETH4
+	/* handle 5th ethernet address */
+	s = getenv("eth4addr");
+#if defined(CONFIG_XPEDITE1K) || defined(CONFIG_METROBOX) || defined(CONFIG_KAREF)
+	if (s == NULL)
+		board_get_enetaddr(bd->bi_enet4addr);
+	else
+#endif
+	for (i = 0; i < 6; ++i) {
+		bd->bi_enet4addr[i] = s ? simple_strtoul (s, &e, 16) : 0;
+		if (s)
+			s = (*e) ? e + 1 : e;
+	}
+#endif
+
+#ifdef CONFIG_HAS_ETH5
+	/* handle 6th ethernet address */
+	s = getenv("eth5addr");
+#if defined(CONFIG_XPEDITE1K) || defined(CONFIG_METROBOX) || defined(CONFIG_KAREF)
+	if (s == NULL)
+		board_get_enetaddr(bd->bi_enet5addr);
+	else
+#endif
+	for (i = 0; i < 6; ++i) {
+		bd->bi_enet5addr[i] = s ? simple_strtoul (s, &e, 16) : 0;
+		if (s)
+			s = (*e) ? e + 1 : e;
+	}
+#endif
+
 #if defined(CONFIG_TQM8xxL) || defined(CONFIG_TQM8260) || \
     defined(CONFIG_TQM8272) || \
     defined(CONFIG_CCM) || defined(CONFIG_KUP4K) || \
diff --git a/net/eth.c b/net/eth.c
index 432dd60..ccd871a 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -180,6 +180,12 @@ int eth_initialize(bd_t *bis)
 #if defined(CONFIG_UEC_ETH4)
 	uec_initialize(3);
 #endif
+#if defined(CONFIG_UEC_ETH5)
+	uec_initialize(4);
+#endif
+#if defined(CONFIG_UEC_ETH6)
+	uec_initialize(5);
+#endif
 
 #if defined(FEC_ENET) || defined(CONFIG_ETHER_ON_FCC)
 	fec_initialize(bis);
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 46747d3..d41175f 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -157,6 +157,15 @@ static char default_environment[] = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr=" MK_STR (CONFIG_ETH3ADDR) "\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr=" MK_STR (CONFIG_ETH4ADDR) "\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr=" MK_STR (CONFIG_ETH5ADDR) "\0"
+#endif
+#ifdef	CONFIG_ETH6ADDR
+	"eth6addr=" MK_STR (CONFIG_ETH6ADDR) "\0"
+#endif
 #ifdef	CONFIG_ETHPRIME
 	"ethprime=" CONFIG_ETHPRIME "\0"
 #endif
-- 
1.5.5.GIT

[U-Boot] [PATCH v2] [83xx] Adds two more ethernet interface to 83xx

[Wolfgang Denk]

Dear richardretanubun,

In message <48DC0652.5030409@ruggedcom.com> you wrote:
>
> +++ b/README
> @@ -1097,6 +1097,9 @@ The following options need to be configured:
>  		CONFIG_ETHADDR
>  		CONFIG_ETH2ADDR
>  		CONFIG_ETH3ADDR
> +		CONFIG_ETH4ADDR
> +		CONFIG_ETH5ADDR
> +		CONFIG_ETH6ADDR

Sorry, but I still don't get why CONFIG_ETH1ADDR isn't there?

This will make the loop in fdt_fixup_ethernet() terminate unexpectedly
early.

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
Men don't talk peace unless they're ready to back it up with war.
	-- Col. Green, "The Savage Curtain", stardate 5906.4

[U-Boot] [PATCH v2] [83xx] Adds two more ethernet interface to 83xx

[richardretanubun]

Wolfgang Denk wrote:

> Dear richardretanubun,
>
> In message <48DC0652.5030409@ruggedcom.com> you wrote:
>> +++ b/README
>> @@ -1097,6 +1097,9 @@ The following options need to be configured:
>>  		CONFIG_ETHADDR
>>  		CONFIG_ETH2ADDR
>>  		CONFIG_ETH3ADDR
>> +		CONFIG_ETH4ADDR
>> +		CONFIG_ETH5ADDR
>> +		CONFIG_ETH6ADDR
>
> Sorry, but I still don't get why CONFIG_ETH1ADDR isn't there?
>
> This will make the loop in fdt_fixup_ethernet() terminate unexpectedly
> early.
>
Sorry Wolfgang, the V2 is sent before I red Kim's !ack 
about the comments on CONFIG_ETH1ADDR being the implicit CONFIG_ETHADDR (which is not true)

Please disregard V2. V1 is correct, except for this change:

diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
>> index f4d9d40..67cc64f 100644
>> --- a/common/cmd_bdinfo.c
>> +++ b/common/cmd_bdinfo.c
>> @@ -91,11 +91,12 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>>  	print_str ("pevfreq",	    strmhz(buf, bd->bi_pevfreq));
>>  #endif
>>  
>> +#if defined(CONFIG_HAS_ETH0)
>>  	puts ("ethaddr     =");
>>  	for (i=0; i<6; ++i) {
>>  		printf ("%c%02X", i ? ':' : ' ', bd->bi_enetaddr[i]);
>>  	}
>> -
>> +#endif
>
>[KP] how is the above change relevant to the patch subject?
[RR] Good catch, I lumped it together because I was in the code neighborhood 
     and got carried away in making the code uniform. I will pull it out of this patch.


- Richard

[U-Boot] [PATCH v2] [83xx] Adds two more ethernet interface to 83xx

[Wolfgang Denk]

Dear richardretanubun,

In message <48DC1637.5000707@ruggedcom.com> you wrote:
> 
> Please disregard V2. V1 is correct, except for this change:

So you will submit a V3?

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
Roses are red
Violets are blue
Some poems rhyme

[U-Boot] [PATCH v2] [83xx] Adds two more ethernet interface to 83xx

[richardretanubun]

Wolfgang Denk wrote:

> Dear richardretanubun,
>
> In message <48DC1637.5000707@ruggedcom.com> you wrote:
>> Please disregard V2. V1 is correct, except for this change:
>
> So you will submit a V3?
I am not so sure about it.

This patch came to be when I wanted to make it convenient for using u-boot to test a platform 
with six eth interface without the need of an fdt.
 
However, I seem to have sparked a discussion of a larger scope than I expected. :)

Everyone's comment seems to indicate that streamlining is needed for the network code.
 
I'd hate to muck the existing code some more with more copy-paste just for convenience's sake.

However, if you are ok with V3 being "V1 minus the common/cmd_bdinfo.c #if defined(CONFIG_HAS_ETH0) hunk" 
then I can post this version.

>
> Best regards,
>
> Wolfgang Denk
>
>   
Cheers,

- Richard

[U-Boot] [PATCH v2] [83xx] Adds two more ethernet interface to 83xx

[Jerry Van Baren]

Hi Richard & Wolfgang,

richardretanubun wrote:
> Wolfgang Denk wrote:
> 
>> Dear richardretanubun,
>>
>> In message <48DC1637.5000707@ruggedcom.com> you wrote:
>>> Please disregard V2. V1 is correct, except for this change:
>> So you will submit a V3?
> I am not so sure about it.
> 
> This patch came to be when I wanted to make it convenient for using
> u-boot to test a platform with six eth interface without the need of
> an fdt.
>  
> However, I seem to have sparked a discussion of a larger scope than I
> expected. :)
> 
> Everyone's comment seems to indicate that streamlining is needed for
> the network code.
>  
> I'd hate to muck the existing code some more with more copy-paste
> just for convenience's sake.

I was the most vocal "everybody", but my counterproposal thoughts were 
poorly developed.  Until I or someone else has (takes) the time to 
develop a viable alternative, I don't have any problem with your current 
solution.

When the next sucker tries to extend from six to eight ethernets, maybe 
we will develop a scalable alternative that is viable.  :-/

[snip]

> Cheers,
> - Richard

Thanks,
gvb

[U-Boot] [PATCH v3] [83xx] Adds two more ethernet interface to 83xx

[richardretanubun]

Added as a convenience for other platforms that uses MPC8360 (has 8 UCC).
Six eth interface is chosen because the platform I am using combines
UCC1&2 and UCC3&4 as 1000 Eth and the other four UCCs as 10/100 Eth.

Signed-off-by: Richard Retanubun <RichardRetanubun@RugggedCom.com>
---
Thank you for all the feedback guys,
version 3 removes the extra #ifdef CONFIG_HAS_ETH0 from common/cmd_bdinfo.c

 README                   |    3 ++
 common/cmd_bdinfo.c      |   14 +++++++++++++
 common/env_common.c      |    6 +++++
 common/env_embedded.c    |    6 +++++
 cpu/mpc83xx/fdt.c        |    3 +-
 drivers/qe/uec.c         |   48 +++++++++++++++++++++++++++++++++++++++++++++-
 include/asm-ppc/u-boot.h |    6 +++++
 lib_ppc/board.c          |   30 ++++++++++++++++++++++++++++
 net/eth.c                |    6 +++++
 tools/env/fw_env.c       |    6 +++++
 10 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/README b/README
index ccd839c..8802304 100644
--- a/README
+++ b/README
@@ -1095,8 +1095,11 @@ The following options need to be configured:
 
 - Ethernet address:
 		CONFIG_ETHADDR
+		CONFIG_ETH1ADDR
 		CONFIG_ETH2ADDR
 		CONFIG_ETH3ADDR
+		CONFIG_ETH4ADDR
+		CONFIG_ETH5ADDR
 
 		Define a default value for Ethernet address to use
 		for the respective Ethernet interface, in case this
diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
index f4d9d40..5018930 100644
--- a/common/cmd_bdinfo.c
+++ b/common/cmd_bdinfo.c
@@ -117,6 +117,20 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	}
 #endif
 
+#if defined(CONFIG_HAS_ETH4)
+       puts ("\neth4addr    =");
+       for (i=0; i<6; ++i) {
+		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet4addr[i]);
+	}
+#endif
+
+#if defined(CONFIG_HAS_ETH5)
+       puts ("\neth5addr    =");
+       for (i=0; i<6; ++i) {
+		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet5addr[i]);
+	}
+#endif
+
 #ifdef CONFIG_HERMES
 	print_str ("ethspeed",	    strmhz(buf, bd->bi_ethspeed));
 #endif
diff --git a/common/env_common.c b/common/env_common.c
index 77f9944..0fee3af 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -91,6 +91,12 @@ uchar default_environment[] = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
+#endif
 #ifdef	CONFIG_IPADDR
 	"ipaddr="	MK_STR(CONFIG_IPADDR)		"\0"
 #endif
diff --git a/common/env_embedded.c b/common/env_embedded.c
index 77e5619..e79f843 100644
--- a/common/env_embedded.c
+++ b/common/env_embedded.c
@@ -135,6 +135,12 @@ env_t environment __PPCENV__ = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
+#endif
 #ifdef	CONFIG_ETHPRIME
 	"ethprime="	CONFIG_ETHPRIME			"\0"
 #endif
diff --git a/cpu/mpc83xx/fdt.c b/cpu/mpc83xx/fdt.c
index 39bd9dc..3e3e1c8 100644
--- a/cpu/mpc83xx/fdt.c
+++ b/cpu/mpc83xx/fdt.c
@@ -52,7 +52,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 		fdt_fixup_crypto_node(blob, 0x0204);
 
 #if defined(CONFIG_HAS_ETH0) || defined(CONFIG_HAS_ETH1) ||\
-    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3)
+    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) ||\
+    defined(CONFIG_HAS_ETH4) || defined(CONFIG_HAS_ETH5)
 	fdt_fixup_ethernet(blob);
 #endif
 
diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
index 344c649..e1dec5e 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -123,8 +123,54 @@ static uec_info_t eth4_uec_info = {
 	.enet_interface		= CFG_UEC4_INTERFACE_MODE,
 };
 #endif
+#ifdef CONFIG_UEC_ETH5
+static uec_info_t eth5_uec_info = {
+	.uf_info		= {
+		.ucc_num	= CFG_UEC5_UCC_NUM,
+		.rx_clock	= CFG_UEC5_RX_CLK,
+		.tx_clock	= CFG_UEC5_TX_CLK,
+		.eth_type	= CFG_UEC5_ETH_TYPE,
+	},
+#if (CFG_UEC5_ETH_TYPE == FAST_ETH)
+	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
+#else
+	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
+#endif
+	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.tx_bd_ring_len		= 16,
+	.rx_bd_ring_len		= 16,
+	.phy_address		= CFG_UEC5_PHY_ADDR,
+	.enet_interface		= CFG_UEC5_INTERFACE_MODE,
+};
+#endif
+#ifdef CONFIG_UEC_ETH6
+static uec_info_t eth6_uec_info = {
+	.uf_info		= {
+		.ucc_num	= CFG_UEC6_UCC_NUM,
+		.rx_clock	= CFG_UEC6_RX_CLK,
+		.tx_clock	= CFG_UEC6_TX_CLK,
+		.eth_type	= CFG_UEC6_ETH_TYPE,
+	},
+#if (CFG_UEC6_ETH_TYPE == FAST_ETH)
+	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
+#else
+	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
+	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
+#endif
+	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
+	.tx_bd_ring_len		= 16,
+	.rx_bd_ring_len		= 16,
+	.phy_address		= CFG_UEC6_PHY_ADDR,
+	.enet_interface		= CFG_UEC6_INTERFACE_MODE,
+};
+#endif
 
-#define MAXCONTROLLERS	(4)
+#define MAXCONTROLLERS	(6)
 
 static struct eth_device *devlist[MAXCONTROLLERS];
 
diff --git a/include/asm-ppc/u-boot.h b/include/asm-ppc/u-boot.h
index 54ac01d..7451905 100644
--- a/include/asm-ppc/u-boot.h
+++ b/include/asm-ppc/u-boot.h
@@ -111,6 +111,12 @@ typedef struct bd_info {
 #ifdef CONFIG_HAS_ETH3
 	unsigned char   bi_enet3addr[6];
 #endif
+#ifdef CONFIG_HAS_ETH4
+	unsigned char   bi_enet4addr[6];
+#endif
+#ifdef CONFIG_HAS_ETH5
+	unsigned char   bi_enet5addr[6];
+#endif
 
 #if defined(CONFIG_405GP) || defined(CONFIG_405EP) || \
     defined(CONFIG_405EZ) || defined(CONFIG_440GX) || \
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index c02ac62..d440722 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -944,6 +944,36 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	}
 #endif
 
+#ifdef CONFIG_HAS_ETH4
+	/* handle 5th ethernet address */
+	s = getenv("eth4addr");
+#if defined(CONFIG_XPEDITE1K) || defined(CONFIG_METROBOX) || defined(CONFIG_KAREF)
+	if (s == NULL)
+		board_get_enetaddr(bd->bi_enet4addr);
+	else
+#endif
+	for (i = 0; i < 6; ++i) {
+		bd->bi_enet4addr[i] = s ? simple_strtoul (s, &e, 16) : 0;
+		if (s)
+			s = (*e) ? e + 1 : e;
+	}
+#endif
+
+#ifdef CONFIG_HAS_ETH5
+	/* handle 6th ethernet address */
+	s = getenv("eth5addr");
+#if defined(CONFIG_XPEDITE1K) || defined(CONFIG_METROBOX) || defined(CONFIG_KAREF)
+	if (s == NULL)
+		board_get_enetaddr(bd->bi_enet5addr);
+	else
+#endif
+	for (i = 0; i < 6; ++i) {
+		bd->bi_enet5addr[i] = s ? simple_strtoul (s, &e, 16) : 0;
+		if (s)
+			s = (*e) ? e + 1 : e;
+	}
+#endif
+
 #if defined(CONFIG_TQM8xxL) || defined(CONFIG_TQM8260) || \
     defined(CONFIG_TQM8272) || \
     defined(CONFIG_CCM) || defined(CONFIG_KUP4K) || \
diff --git a/net/eth.c b/net/eth.c
index 432dd60..ccd871a 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -180,6 +180,12 @@ int eth_initialize(bd_t *bis)
 #if defined(CONFIG_UEC_ETH4)
 	uec_initialize(3);
 #endif
+#if defined(CONFIG_UEC_ETH5)
+	uec_initialize(4);
+#endif
+#if defined(CONFIG_UEC_ETH6)
+	uec_initialize(5);
+#endif
 
 #if defined(FEC_ENET) || defined(CONFIG_ETHER_ON_FCC)
 	fec_initialize(bis);
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 46747d3..6e9c34f 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -157,6 +157,12 @@ static char default_environment[] = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr=" MK_STR (CONFIG_ETH3ADDR) "\0"
 #endif
+#ifdef	CONFIG_ETH4ADDR
+	"eth4addr=" MK_STR (CONFIG_ETH4ADDR) "\0"
+#endif
+#ifdef	CONFIG_ETH5ADDR
+	"eth5addr=" MK_STR (CONFIG_ETH5ADDR) "\0"
+#endif
 #ifdef	CONFIG_ETHPRIME
 	"ethprime=" CONFIG_ETHPRIME "\0"
 #endif
-- 
1.5.5.GIT

[U-Boot] [PATCH v3] [83xx] Adds two more ethernet interface to 83xx

[Kim Phillips]

On Mon, 29 Sep 2008 18:28:23 -0400
richardretanubun <richardretanubun@ruggedcom.com> wrote:

> Added as a convenience for other platforms that uses MPC8360 (has 8 UCC).
> Six eth interface is chosen because the platform I am using combines
> UCC1&2 and UCC3&4 as 1000 Eth and the other four UCCs as 10/100 Eth.
> 
> Signed-off-by: Richard Retanubun <RichardRetanubun@RugggedCom.com>

Acked-by: Kim Phillips <kim.phillips@freescale.com>

does someone (Ben, gvb, wd) want to pick this up, do a s/ggg/gg/ on
Richard's s-o-b, and put it in their next branch, or shall I?

Kim

[U-Boot] [PATCH v3] [83xx] Adds two more ethernet interface to 83xx

[Jerry Van Baren]

Kim Phillips wrote:
> On Mon, 29 Sep 2008 18:28:23 -0400
> richardretanubun <richardretanubun@ruggedcom.com> wrote:
> 
>> Added as a convenience for other platforms that uses MPC8360 (has 8 UCC).
>> Six eth interface is chosen because the platform I am using combines
>> UCC1&2 and UCC3&4 as 1000 Eth and the other four UCCs as 10/100 Eth.
>>
>> Signed-off-by: Richard Retanubun <RichardRetanubun@RugggedCom.com>
> 
> Acked-by: Kim Phillips <kim.phillips@freescale.com>
> 
> does someone (Ben, gvb, wd) want to pick this up, do a s/ggg/gg/ on
> Richard's s-o-b, and put it in their next branch, or shall I?
> 
> Kim

Hi Kim,

I would defer to you as 83xx custodian (first choice) or Ben as net 
custodian.

Thanks,
gvb

[U-Boot] [PATCH v3] [83xx] Adds two more ethernet interface to 83xx

[Ben Warren]

Kim Phillips wrote:
> On Mon, 29 Sep 2008 18:28:23 -0400
> richardretanubun <richardretanubun@ruggedcom.com> wrote:
>
>   
>> Added as a convenience for other platforms that uses MPC8360 (has 8 UCC).
>> Six eth interface is chosen because the platform I am using combines
>> UCC1&2 and UCC3&4 as 1000 Eth and the other four UCCs as 10/100 Eth.
>>
>> Signed-off-by: Richard Retanubun <RichardRetanubun@RugggedCom.com>
>>     
>
> Acked-by: Kim Phillips <kim.phillips@freescale.com>
>
> does someone (Ben, gvb, wd) want to pick this up, do a s/ggg/gg/ on
> Richard's s-o-b, and put it in their next branch, or shall I?
>   
I'll take it.

Ben

[U-Boot] [PATCH v3] [83xx] Adds two more ethernet interface to 83xx

[Ben Warren]

richardretanubun wrote:
> Added as a convenience for other platforms that uses MPC8360 (has 8 UCC).
> Six eth interface is chosen because the platform I am using combines
> UCC1&2 and UCC3&4 as 1000 Eth and the other four UCCs as 10/100 Eth.
>
> Signed-off-by: Richard Retanubun <RichardRetanubun@RugggedCom.com>
> ---
Applied to net/next

regards,
Ben

[U-Boot] [PATCH v3] [83xx] Adds two more ethernet interface to 83xx

[richardretanubun]

Fixed compiler warning "declared but unused" eth5_uec_info and eth6_uec_info.
Signed-off-by: Richard Retanubun <RichardRetanubun@RugggedCom.com>
---
Hi Ben,

Thanks for applying the patch. 
I got a compiler warning when using it, here is a patch to fix that.

Thanks for all the help

Richard

 drivers/qe/uec.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
index e1dec5e..582a1c0 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -1414,6 +1414,14 @@ int uec_initialize(int index)
 #ifdef CONFIG_UEC_ETH4
 		uec_info = &eth4_uec_info;
 #endif
+	} else if (index == 4) {
+#ifdef CONFIG_UEC_ETH5
+		uec_info = &eth5_uec_info;
+#endif
+	} else if (index == 5) {
+#ifdef CONFIG_UEC_ETH6
+		uec_info = &eth6_uec_info;
+#endif
 	} else {
 		printf("%s: index is illegal.\n", __FUNCTION__);
 		return -EINVAL;
-- 
1.5.5.GIT

[U-Boot] [PATCH v3] [83xx] Adds two more ethernet interface to 83xx

[Ben Warren]

richardretanubun wrote:
> Fixed compiler warning "declared but unused" eth5_uec_info and 
> eth6_uec_info.
> Signed-off-by: Richard Retanubun <RichardRetanubun@RugggedCom.com>
> ---
> Hi Ben,
>
> Thanks for applying the patch. I got a compiler warning when using it, 
> here is a patch to fix that.
>
> Thanks for all the help
>
> Richard
>
> drivers/qe/uec.c |    8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
> index e1dec5e..582a1c0 100644
> --- a/drivers/qe/uec.c
> +++ b/drivers/qe/uec.c
> @@ -1414,6 +1414,14 @@ int uec_initialize(int index)
> #ifdef CONFIG_UEC_ETH4
>         uec_info = &eth4_uec_info;
> #endif
> +    } else if (index == 4) {
> +#ifdef CONFIG_UEC_ETH5
> +        uec_info = &eth5_uec_info;
> +#endif
> +    } else if (index == 5) {
> +#ifdef CONFIG_UEC_ETH6
> +        uec_info = &eth6_uec_info;
> +#endif
>     } else {
>         printf("%s: index is illegal.\n", __FUNCTION__);
>         return -EINVAL;
Applied to net/next

regards,
Ben

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Wolfgang Denk]

Dear Kim Phillips,

In message <20080925145035.cebab0df.kim.phillips@freescale.com> you wrote:
>
> >  - Ethernet address:
> >  		CONFIG_ETHADDR
> > +		CONFIG_ETH1ADDR
> >  		CONFIG_ETH2ADDR
> 
> hmm..historically ETHADDR has been the implicit ETH1ADDR.  Did you mean

No. ETHADDR is ETH0ADDR

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
Suffocating together ... would create heroic camaraderie.
	-- Khan Noonian Singh, "Space Seed", stardate 3142.8

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Jerry Van Baren]

richardretanubun wrote:
> Added for convenience for other platforms that uses MPC8360 (has 8 UCC).
> 6 eth interface is chosen because the platform I am using combines
> UCC1&2 and UCC3&4 as gigEth and the other 4 UCC as 10/100 Eth.

We are about to fall into a never ending explosion of #define 
CONFIG_ETHnnnADDR.  This is bad.  Very bad.  :-(

Kumar solved this problem WRT cpu/mpc83xx/fdt.c fdt_fixup_ethernet(void 
*fdt) (and other CPUs) by using the device tree to find all the 
ethernets and configure them.
   <http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/45554>

Is bdinfo relevant? If not, return;  /* that was easy */

If bdinfo is still relevant, look at how Kumar solved the #define 
explosion in cpu/*/fdt.c.  You should be able to adapt it in the code 
you augmented with CONFIG_ETH[45]ADDR.

Suggestion: Look at changing CONFIG_ETH*ADDR to CONFIG_ETH_FDT where 
that notation indicates the code should find the ethernet info in the 
fdt blob rather than #defines/env variables.

One fly in the ointment is if we don't have a fdt blob to pick the 
ethernet parameters out of.  Perhaps this is because we are going to 
load a fdt blob over the ethernet.  We discussed having a default blob 
(probably the more elegant solution).  A simpler and more "the way it is 
now" solution would be to define a limited number of #define 
CONFIG_ETH*ADDR values (I think 2 would be adequate, but at least limit 
ourselves to the current 4 max).  Then bare board fdt blob loading would 
have to be over one/two/four fixed ethernet ports, but once a valid fdt 
blob was loaded all of the ethernets would be configured properly by u-boot.

> ---
>  README                   |    3 ++
>  common/cmd_bdinfo.c      |   17 +++++++++++++++-
>  common/env_common.c      |    6 +++++
>  common/env_embedded.c    |    6 +++++
>  cpu/mpc83xx/fdt.c        |    3 +-
>  drivers/qe/uec.c         |   48 +++++++++++++++++++++++++++++++++++++++++++++-
>  include/asm-ppc/u-boot.h |    6 +++++
>  lib_ppc/board.c          |   30 ++++++++++++++++++++++++++++
>  net/eth.c                |    6 +++++
>  tools/env/fw_env.c       |    6 +++++
>  10 files changed, 128 insertions(+), 3 deletions(-)
> 
> diff --git a/README b/README
> index ccd839c..8802304 100644
> --- a/README
> +++ b/README
> @@ -1095,8 +1095,11 @@ The following options need to be configured:
>  
>  - Ethernet address:
>  		CONFIG_ETHADDR
> +		CONFIG_ETH1ADDR
>  		CONFIG_ETH2ADDR
>  		CONFIG_ETH3ADDR
> +		CONFIG_ETH4ADDR
> +		CONFIG_ETH5ADDR

Bleah!  Oh, right, I said that already.

[snip]

> +#if defined(CONFIG_HAS_ETH4)
> +       puts ("\neth4addr    =");
> +       for (i=0; i<6; ++i) {
> +		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet4addr[i]);
> +	}
> +#endif
> +
> +#if defined(CONFIG_HAS_ETH5)
> +       puts ("\neth5addr    =");
> +       for (i=0; i<6; ++i) {
> +		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet5addr[i]);
> +	}
> +#endif
> +

No, don't add more cut'n'pastes.  Bleah!  Use a fdt_*() loop (a'la 
Kumar's code).  This would have to be conditional on CONFIG_LIBFDT since 
not all boards use LIBFDT.  If !defined(CONFIG_LIBFDT), use the existing 
code.

> diff --git a/common/env_common.c b/common/env_common.c
> index 77f9944..0fee3af 100644
> --- a/common/env_common.c
> +++ b/common/env_common.c
> @@ -91,6 +91,12 @@ uchar default_environment[] = {
>  #ifdef	CONFIG_ETH3ADDR
>  	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
>  #endif
> +#ifdef	CONFIG_ETH4ADDR
> +	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
> +#endif
> +#ifdef	CONFIG_ETH5ADDR
> +	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
> +#endif

Bleah!  I would prefer to cut our losses at CONFIG_ETH1ADDR (or 
CONFIG_ETH3ADDR), see lead-in discussion.

> diff --git a/common/env_embedded.c b/common/env_embedded.c
> index 77e5619..e79f843 100644
> --- a/common/env_embedded.c
> +++ b/common/env_embedded.c
> @@ -135,6 +135,12 @@ env_t environment __PPCENV__ = {
>  #ifdef	CONFIG_ETH3ADDR
>  	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
>  #endif
> +#ifdef	CONFIG_ETH4ADDR
> +	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
> +#endif
> +#ifdef	CONFIG_ETH5ADDR
> +	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
> +#endif
>  #ifdef	CONFIG_ETHPRIME
>  	"ethprime="	CONFIG_ETHPRIME			"\0"
>  #endif

Bleah! ...but I repeat myself.

Do we need eth[1-5]?addr env variables?  I don't think so.

If we really do need them, they can be generated from the fdt blob a'la 
Kumar's loop.

> diff --git a/cpu/mpc83xx/fdt.c b/cpu/mpc83xx/fdt.c
> index 39bd9dc..3e3e1c8 100644
> --- a/cpu/mpc83xx/fdt.c
> +++ b/cpu/mpc83xx/fdt.c
> @@ -52,7 +52,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
>  		fdt_fixup_crypto_node(blob, 0x0204);
>  
>  #if defined(CONFIG_HAS_ETH0) || defined(CONFIG_HAS_ETH1) ||\
> -    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3)
> +    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) ||\
> +    defined(CONFIG_HAS_ETH4) || defined(CONFIG_HAS_ETH5)

Bleah!  I counter-propose a single CONFIG_ETH_FDT.  Alternatively, if 
defined(CONFIG_HAS_ETH0), it is a good bet that you want to call 
fdt_fixup_ethernet(blob); and *that* routine handles up all defined 
ethernet spiggots (thanks, Kumar!).

>  	fdt_fixup_ethernet(blob);
>  #endif
>  
> diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
> index 344c649..e1dec5e 100644
> --- a/drivers/qe/uec.c
> +++ b/drivers/qe/uec.c
> @@ -123,8 +123,54 @@ static uec_info_t eth4_uec_info = {
>  	.enet_interface		= CFG_UEC4_INTERFACE_MODE,
>  };
>  #endif
> +#ifdef CONFIG_UEC_ETH5
> +static uec_info_t eth5_uec_info = {
> +	.uf_info		= {
> +		.ucc_num	= CFG_UEC5_UCC_NUM,
> +		.rx_clock	= CFG_UEC5_RX_CLK,
> +		.tx_clock	= CFG_UEC5_TX_CLK,
> +		.eth_type	= CFG_UEC5_ETH_TYPE,
> +	},
> +#if (CFG_UEC5_ETH_TYPE == FAST_ETH)
> +	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
> +	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
> +#else
> +	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
> +	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
> +#endif
> +	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
> +	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
> +	.tx_bd_ring_len		= 16,
> +	.rx_bd_ring_len		= 16,
> +	.phy_address		= CFG_UEC5_PHY_ADDR,
> +	.enet_interface		= CFG_UEC5_INTERFACE_MODE,
> +};
> +#endif
> +#ifdef CONFIG_UEC_ETH6
> +static uec_info_t eth6_uec_info = {
> +	.uf_info		= {
> +		.ucc_num	= CFG_UEC6_UCC_NUM,
> +		.rx_clock	= CFG_UEC6_RX_CLK,
> +		.tx_clock	= CFG_UEC6_TX_CLK,
> +		.eth_type	= CFG_UEC6_ETH_TYPE,
> +	},
> +#if (CFG_UEC6_ETH_TYPE == FAST_ETH)
> +	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
> +	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
> +#else
> +	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
> +	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
> +#endif
> +	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
> +	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
> +	.tx_bd_ring_len		= 16,
> +	.rx_bd_ring_len		= 16,
> +	.phy_address		= CFG_UEC6_PHY_ADDR,
> +	.enet_interface		= CFG_UEC6_INTERFACE_MODE,
> +};
> +#endif

Is this in the fdt blob?  If not, we should add it (possibly 
coordinating with the kernel folks.

> -#define MAXCONTROLLERS	(4)
> +#define MAXCONTROLLERS	(6)
>  
>  static struct eth_device *devlist[MAXCONTROLLERS];

Hmmm, I don't have an clever answer for this one.  Looks like we will 
need a MAXCONTROLLERS to make our devlist[].

> diff --git a/include/asm-ppc/u-boot.h b/include/asm-ppc/u-boot.h
> index 54ac01d..7451905 100644
> --- a/include/asm-ppc/u-boot.h
> +++ b/include/asm-ppc/u-boot.h
> @@ -111,6 +111,12 @@ typedef struct bd_info {
>  #ifdef CONFIG_HAS_ETH3
>  	unsigned char   bi_enet3addr[6];
>  #endif
> +#ifdef CONFIG_HAS_ETH4
> +	unsigned char   bi_enet4addr[6];
> +#endif
> +#ifdef CONFIG_HAS_ETH5
> +	unsigned char   bi_enet5addr[6];
> +#endif

More evil.  I would like to se the bd_info struct go away.  Please check 
if anybody really needs bi_enet[0-5]addr after looking into my 
suggestions/objections above.  If we cannot make bd_info go away, we 
should be able to stop making it bigger.

[snipped remainder of the same]

Thanks,
gvb

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Wolfgang Denk]

Dear Jerry Van Baren,

In message <48DBF7A9.3010905@ge.com> you wrote:
>
> Kumar solved this problem WRT cpu/mpc83xx/fdt.c fdt_fixup_ethernet(void 
> *fdt) (and other CPUs) by using the device tree to find all the 
> ethernets and configure them.
>    <http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/45554>

But Kumar's  code  still  loops  over  the  "eth[N]addr"  environment
variables.  And  even though discouraged for normal use, there should
be a way to define these for the  (compiled-in)  default  environment
where needed (devices using a real ROM).

> Suggestion: Look at changing CONFIG_ETH*ADDR to CONFIG_ETH_FDT where 
> that notation indicates the code should find the ethernet info in the 
> fdt blob rather than #defines/env variables.

Please have a look at fdt_fixup_ethernet() in "common/fdt_support.c";
my reading of the code does not match your statements here.



> Do we need eth[1-5]?addr env variables?  I don't think so.

Yes, we do. That's what fdt_fixup_ethernet() uses to loop.

> If we really do need them, they can be generated from the fdt blob a'la 
> Kumar's loop.

Either you or I must be missing something.


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
Nobody trips over mountains. It is the small pebble that  causes  you
to  stumble.  Pass all the pebbles in your path and you will find you
have crossed the mountain.

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Jerry Van Baren]

OK, critique v2 (thanks to Wolfgang calling BS on my previous critique :-).

richardretanubun wrote:
> Added for convenience for other platforms that uses MPC8360 (has 8 UCC).
> 6 eth interface is chosen because the platform I am using combines
> UCC1&2 and UCC3&4 as gigEth and the other 4 UCC as 10/100 Eth.

[snip]

> diff --git a/README b/README
> index ccd839c..8802304 100644
> --- a/README
> +++ b/README
> @@ -1095,8 +1095,11 @@ The following options need to be configured:
>  
>  - Ethernet address:
>  		CONFIG_ETHADDR
> +		CONFIG_ETH1ADDR
>  		CONFIG_ETH2ADDR
>  		CONFIG_ETH3ADDR
> +		CONFIG_ETH4ADDR
> +		CONFIG_ETH5ADDR
>  
>  		Define a default value for Ethernet address to use
>  		for the respective Ethernet interface, in case this

OK, the above is unavoidable unless...

A more major change but conceptually slick alternative would be a (weak) 
function generating appropriate MAC addresses since on most/all boards 
the MAC addresses of the etherspiggots are mathematically related.  The 
generation function would use MAXCONTROLLERS (CONFIG_NUM_ETH?) to 
generate the appropriate number of MAC address env variables.

> diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
> index f4d9d40..67cc64f 100644
> --- a/common/cmd_bdinfo.c
> +++ b/common/cmd_bdinfo.c
> @@ -91,11 +91,12 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  	print_str ("pevfreq",	    strmhz(buf, bd->bi_pevfreq));
>  #endif
>  
> +#if defined(CONFIG_HAS_ETH0)
>  	puts ("ethaddr     =");
>  	for (i=0; i<6; ++i) {
>  		printf ("%c%02X", i ? ':' : ' ', bd->bi_enetaddr[i]);
>  	}
> -
> +#endif
>  #if defined(CONFIG_HAS_ETH1)
>  	puts ("\neth1addr    =");
>  	for (i=0; i<6; ++i) {
> @@ -117,6 +118,20 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>  	}
>  #endif
>  
> +#if defined(CONFIG_HAS_ETH4)
> +       puts ("\neth4addr    =");
> +       for (i=0; i<6; ++i) {
> +		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet4addr[i]);
> +	}
> +#endif
> +
> +#if defined(CONFIG_HAS_ETH5)
> +       puts ("\neth5addr    =");
> +       for (i=0; i<6; ++i) {
> +		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet5addr[i]);
> +	}
> +#endif
> +

Here is where a modification Kumar's loop (I would loop over 
/aliases/ethernet[N] instead of the env variables) would cut out 5 
copies of the same code and would scale infinitely.  Hmmm, we have 
MAXCONTROLLERS (CONFIG_NUM_ETH?) to tell us how many etherspiggots we 
have, can we loop on that?

I would also *strongly* prefer eliminating bd->bi_enet[N]addr variables, 
are they needed at all in a fdt-enabled system?  Are there any other 
places that bd->bi_enet[N]addr is used?  If the only use is here, to 
print out the MAC addresses, we can do Kumar's loop and print the env 
variables directly instead of converting the env variable to 
bd->enet[N]addr and then back into ASCII.  Bleah.

> diff --git a/common/env_common.c b/common/env_common.c
> index 77f9944..0fee3af 100644
> --- a/common/env_common.c
> +++ b/common/env_common.c
> @@ -91,6 +91,12 @@ uchar default_environment[] = {
>  #ifdef	CONFIG_ETH3ADDR
>  	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
>  #endif
> +#ifdef	CONFIG_ETH4ADDR
> +	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
> +#endif
> +#ifdef	CONFIG_ETH5ADDR
> +	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
> +#endif
>  #ifdef	CONFIG_IPADDR
>  	"ipaddr="	MK_STR(CONFIG_IPADDR)		"\0"
>  #endif
> diff --git a/common/env_embedded.c b/common/env_embedded.c
> index 77e5619..e79f843 100644
> --- a/common/env_embedded.c
> +++ b/common/env_embedded.c
> @@ -135,6 +135,12 @@ env_t environment __PPCENV__ = {
>  #ifdef	CONFIG_ETH3ADDR
>  	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
>  #endif
> +#ifdef	CONFIG_ETH4ADDR
> +	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
> +#endif
> +#ifdef	CONFIG_ETH5ADDR
> +	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
> +#endif
>  #ifdef	CONFIG_ETHPRIME
>  	"ethprime="	CONFIG_ETHPRIME			"\0"
>  #endif

OK, the above is unavoidable unless we use a function to generate the 
sequence of MAC addresses.  (Caution: undeveloped concept.)

> diff --git a/cpu/mpc83xx/fdt.c b/cpu/mpc83xx/fdt.c
> index 39bd9dc..3e3e1c8 100644
> --- a/cpu/mpc83xx/fdt.c
> +++ b/cpu/mpc83xx/fdt.c
> @@ -52,7 +52,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
>  		fdt_fixup_crypto_node(blob, 0x0204);
>  
>  #if defined(CONFIG_HAS_ETH0) || defined(CONFIG_HAS_ETH1) ||\
> -    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3)
> +    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) ||\
> +    defined(CONFIG_HAS_ETH4) || defined(CONFIG_HAS_ETH5)
>  	fdt_fixup_ethernet(blob);
>  #endif

This is horrible.  We should use CONFIG_HAS_MANY_ETH or something like 
that.  Maybe we can create CONFIG_NUM_ETH and #define it to be the 
number of etherspiggots we have (replacing MAXCONTROLLERS - not a good 
name if it ends up being more widely visible).

> diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
> index 344c649..e1dec5e 100644
> --- a/drivers/qe/uec.c
> +++ b/drivers/qe/uec.c
> @@ -123,8 +123,54 @@ static uec_info_t eth4_uec_info = {
>  	.enet_interface		= CFG_UEC4_INTERFACE_MODE,
>  };
>  #endif
> +#ifdef CONFIG_UEC_ETH5
> +static uec_info_t eth5_uec_info = {
> +	.uf_info		= {
> +		.ucc_num	= CFG_UEC5_UCC_NUM,
> +		.rx_clock	= CFG_UEC5_RX_CLK,
> +		.tx_clock	= CFG_UEC5_TX_CLK,
> +		.eth_type	= CFG_UEC5_ETH_TYPE,
> +	},
> +#if (CFG_UEC5_ETH_TYPE == FAST_ETH)
> +	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
> +	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
> +#else
> +	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
> +	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
> +#endif
> +	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
> +	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
> +	.tx_bd_ring_len		= 16,
> +	.rx_bd_ring_len		= 16,
> +	.phy_address		= CFG_UEC5_PHY_ADDR,
> +	.enet_interface		= CFG_UEC5_INTERFACE_MODE,
> +};
> +#endif
> +#ifdef CONFIG_UEC_ETH6
> +static uec_info_t eth6_uec_info = {
> +	.uf_info		= {
> +		.ucc_num	= CFG_UEC6_UCC_NUM,
> +		.rx_clock	= CFG_UEC6_RX_CLK,
> +		.tx_clock	= CFG_UEC6_TX_CLK,
> +		.eth_type	= CFG_UEC6_ETH_TYPE,
> +	},
> +#if (CFG_UEC6_ETH_TYPE == FAST_ETH)
> +	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
> +	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
> +#else
> +	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
> +	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
> +#endif
> +	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
> +	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
> +	.tx_bd_ring_len		= 16,
> +	.rx_bd_ring_len		= 16,
> +	.phy_address		= CFG_UEC6_PHY_ADDR,
> +	.enet_interface		= CFG_UEC6_INTERFACE_MODE,
> +};
> +#endif

Here I think my previous observation is still applicable: Is this 
information in the fdt blob?  If not, we should add it (possibly 
coordinating with the kernel folks.

> -#define MAXCONTROLLERS	(4)
> +#define MAXCONTROLLERS	(6)

CONFIG_NUM_ETH? (see above for discussion)

>  static struct eth_device *devlist[MAXCONTROLLERS];
>  
> diff --git a/include/asm-ppc/u-boot.h b/include/asm-ppc/u-boot.h
> index 54ac01d..7451905 100644
> --- a/include/asm-ppc/u-boot.h
> +++ b/include/asm-ppc/u-boot.h
> @@ -111,6 +111,12 @@ typedef struct bd_info {
>  #ifdef CONFIG_HAS_ETH3
>  	unsigned char   bi_enet3addr[6];
>  #endif
> +#ifdef CONFIG_HAS_ETH4
> +	unsigned char   bi_enet4addr[6];
> +#endif
> +#ifdef CONFIG_HAS_ETH5
> +	unsigned char   bi_enet5addr[6];
> +#endif

Bleah.  I would like to see the bd_info struct go away.  Please check
if anybody really needs bi_enet[0-5]addr after looking into my
suggestions/objections above.  If we cannot make bd_info go away, we
should at least be able to stop making it bigger.

> diff --git a/lib_ppc/board.c b/lib_ppc/board.c
> index c02ac62..d440722 100644
> --- a/lib_ppc/board.c
> +++ b/lib_ppc/board.c
> @@ -944,6 +944,36 @@ void board_init_r (gd_t *id, ulong dest_addr)
>  	}
>  #endif
>  
> +#ifdef CONFIG_HAS_ETH4
> +	/* handle 5th ethernet address */
> +	s = getenv("eth4addr");
> +#if defined(CONFIG_XPEDITE1K) || defined(CONFIG_METROBOX) || defined(CONFIG_KAREF)
> +	if (s == NULL)
> +		board_get_enetaddr(bd->bi_enet4addr);
> +	else
> +#endif
> +	for (i = 0; i < 6; ++i) {
> +		bd->bi_enet4addr[i] = s ? simple_strtoul (s, &e, 16) : 0;
> +		if (s)
> +			s = (*e) ? e + 1 : e;
> +	}
> +#endif

This is a different version of the repetitive code I complain about 
above (common/cmd_bdinfo.c).  I would like to see a variant of Kumar's 
loop instead of the #if defined() explosion.

[snip duplicate but with different [N] code]

> diff --git a/net/eth.c b/net/eth.c
> index 432dd60..ccd871a 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -180,6 +180,12 @@ int eth_initialize(bd_t *bis)
>  #if defined(CONFIG_UEC_ETH4)
>  	uec_initialize(3);
>  #endif
> +#if defined(CONFIG_UEC_ETH5)
> +	uec_initialize(4);
> +#endif
> +#if defined(CONFIG_UEC_ETH6)
> +	uec_initialize(5);
> +#endif

My kingdom for a loop!

>  #if defined(FEC_ENET) || defined(CONFIG_ETHER_ON_FCC)
>  	fec_initialize(bis);
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 46747d3..6e9c34f 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -157,6 +157,12 @@ static char default_environment[] = {
>  #ifdef	CONFIG_ETH3ADDR
>  	"eth3addr=" MK_STR (CONFIG_ETH3ADDR) "\0"
>  #endif
> +#ifdef	CONFIG_ETH4ADDR
> +	"eth4addr=" MK_STR (CONFIG_ETH4ADDR) "\0"
> +#endif
> +#ifdef	CONFIG_ETH5ADDR
> +	"eth5addr=" MK_STR (CONFIG_ETH5ADDR) "\0"
> +#endif
>  #ifdef	CONFIG_ETHPRIME
>  	"ethprime=" CONFIG_ETHPRIME "\0"
>  #endif

Curmudgeonly yours,
gvb

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Wolfgang Denk]

Dear Jerry Van Baren,

In message <48DC5A97.20305@gmail.com> you wrote:
> 
> Here is where a modification Kumar's loop (I would loop over 
> /aliases/ethernet[N] instead of the env variables) would cut out 5 
> copies of the same code and would scale infinitely.  Hmmm, we have 
> MAXCONTROLLERS (CONFIG_NUM_ETH?) to tell us how many etherspiggots we 
> have, can we loop on that?
> 
> I would also *strongly* prefer eliminating bd->bi_enet[N]addr variables, 
> are they needed at all in a fdt-enabled system?  Are there any other 
> places that bd->bi_enet[N]addr is used?  If the only use is here, to 
> print out the MAC addresses, we can do Kumar's loop and print the env 
> variables directly instead of converting the env variable to 
> bd->enet[N]addr and then back into ASCII.  Bleah.

There are fdt-less systems around, and will be, at least for a looong
time.

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
How can you tell when sour cream goes bad?

[U-Boot] [PATCH] [83xx] Adds two more ethernet interface to 83xx

[Ben Warren]

Jerry Van Baren wrote:
> OK, critique v2 (thanks to Wolfgang calling BS on my previous critique :-).
>
> richardretanubun wrote:
>   
>> Added for convenience for other platforms that uses MPC8360 (has 8 UCC).
>> 6 eth interface is chosen because the platform I am using combines
>> UCC1&2 and UCC3&4 as gigEth and the other 4 UCC as 10/100 Eth.
>>     
>
> [snip]
>
>   
>> diff --git a/README b/README
>> index ccd839c..8802304 100644
>> --- a/README
>> +++ b/README
>> @@ -1095,8 +1095,11 @@ The following options need to be configured:
>>  
>>  - Ethernet address:
>>  		CONFIG_ETHADDR
>> +		CONFIG_ETH1ADDR
>>  		CONFIG_ETH2ADDR
>>  		CONFIG_ETH3ADDR
>> +		CONFIG_ETH4ADDR
>> +		CONFIG_ETH5ADDR
>>  
>>  		Define a default value for Ethernet address to use
>>  		for the respective Ethernet interface, in case this
>>     
>
> OK, the above is unavoidable unless...
>
> A more major change but conceptually slick alternative would be a (weak) 
> function generating appropriate MAC addresses since on most/all boards 
> the MAC addresses of the etherspiggots are mathematically related.  The 
> generation function would use MAXCONTROLLERS (CONFIG_NUM_ETH?) to 
> generate the appropriate number of MAC address env variables.
>
>   
Maybe most/all of the boards worked on by gvb, but I sure wouldn't want 
to generalize much beyond that. In my case, boards usually have one or 
more public ports, requiring registered addresses. These would probably 
be sequential. The other ports usually have private MAC addresses based 
on uniquish things like board serial numbers. And I'm sure there are 
lots of other schemes out there.
>> diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c
>> index f4d9d40..67cc64f 100644
>> --- a/common/cmd_bdinfo.c
>> +++ b/common/cmd_bdinfo.c
>> @@ -91,11 +91,12 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>>  	print_str ("pevfreq",	    strmhz(buf, bd->bi_pevfreq));
>>  #endif
>>  
>> +#if defined(CONFIG_HAS_ETH0)
>>  	puts ("ethaddr     =");
>>  	for (i=0; i<6; ++i) {
>>  		printf ("%c%02X", i ? ':' : ' ', bd->bi_enetaddr[i]);
>>  	}
>> -
>> +#endif
>>  #if defined(CONFIG_HAS_ETH1)
>>  	puts ("\neth1addr    =");
>>  	for (i=0; i<6; ++i) {
>> @@ -117,6 +118,20 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>>  	}
>>  #endif
>>  
>> +#if defined(CONFIG_HAS_ETH4)
>> +       puts ("\neth4addr    =");
>> +       for (i=0; i<6; ++i) {
>> +		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet4addr[i]);
>> +	}
>> +#endif
>> +
>> +#if defined(CONFIG_HAS_ETH5)
>> +       puts ("\neth5addr    =");
>> +       for (i=0; i<6; ++i) {
>> +		printf ("%c%02X", i ? ':' : ' ', bd->bi_enet5addr[i]);
>> +	}
>> +#endif
>> +
>>     
>
> Here is where a modification Kumar's loop (I would loop over 
> /aliases/ethernet[N] instead of the env variables) would cut out 5 
> copies of the same code and would scale infinitely.  Hmmm, we have 
> MAXCONTROLLERS (CONFIG_NUM_ETH?) to tell us how many etherspiggots we 
> have, can we loop on that?
>
> I would also *strongly* prefer eliminating bd->bi_enet[N]addr variables, 
> are they needed at all in a fdt-enabled system?  Are there any other 
> places that bd->bi_enet[N]addr is used?  If the only use is here, to 
> print out the MAC addresses, we can do Kumar's loop and print the env 
> variables directly instead of converting the env variable to 
> bd->enet[N]addr and then back into ASCII.  Bleah.
>
>   
Let's try to back away from fdt for a minute. I know this particular 
controller is PowerPC and so fdt is relevant, but the concept of having 
oodles of network ports is hardly PowerPC-specific and so we need to 
think in more general terms.
>> diff --git a/common/env_common.c b/common/env_common.c
>> index 77f9944..0fee3af 100644
>> --- a/common/env_common.c
>> +++ b/common/env_common.c
>> @@ -91,6 +91,12 @@ uchar default_environment[] = {
>>  #ifdef	CONFIG_ETH3ADDR
>>  	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
>>  #endif
>> +#ifdef	CONFIG_ETH4ADDR
>> +	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
>> +#endif
>> +#ifdef	CONFIG_ETH5ADDR
>> +	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
>> +#endif
>>  #ifdef	CONFIG_IPADDR
>>  	"ipaddr="	MK_STR(CONFIG_IPADDR)		"\0"
>>  #endif
>> diff --git a/common/env_embedded.c b/common/env_embedded.c
>> index 77e5619..e79f843 100644
>> --- a/common/env_embedded.c
>> +++ b/common/env_embedded.c
>> @@ -135,6 +135,12 @@ env_t environment __PPCENV__ = {
>>  #ifdef	CONFIG_ETH3ADDR
>>  	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"
>>  #endif
>> +#ifdef	CONFIG_ETH4ADDR
>> +	"eth4addr="	MK_STR(CONFIG_ETH4ADDR)		"\0"
>> +#endif
>> +#ifdef	CONFIG_ETH5ADDR
>> +	"eth5addr="	MK_STR(CONFIG_ETH5ADDR)		"\0"
>> +#endif
>>  #ifdef	CONFIG_ETHPRIME
>>  	"ethprime="	CONFIG_ETHPRIME			"\0"
>>  #endif
>>     
>
> OK, the above is unavoidable unless we use a function to generate the 
> sequence of MAC addresses.  (Caution: undeveloped concept.)
>
>   
Since we don't allow anyone to define CONFIG_ETHxADDR in board configs, 
do they really need to exist at all? Sorry, this comment isn't related 
to this particular code snippet, but my fingers won't stop moving.
>> diff --git a/cpu/mpc83xx/fdt.c b/cpu/mpc83xx/fdt.c
>> index 39bd9dc..3e3e1c8 100644
>> --- a/cpu/mpc83xx/fdt.c
>> +++ b/cpu/mpc83xx/fdt.c
>> @@ -52,7 +52,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
>>  		fdt_fixup_crypto_node(blob, 0x0204);
>>  
>>  #if defined(CONFIG_HAS_ETH0) || defined(CONFIG_HAS_ETH1) ||\
>> -    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3)
>> +    defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) ||\
>> +    defined(CONFIG_HAS_ETH4) || defined(CONFIG_HAS_ETH5)
>>  	fdt_fixup_ethernet(blob);
>>  #endif
>>     
>
> This is horrible.  We should use CONFIG_HAS_MANY_ETH or something like 
> that.  Maybe we can create CONFIG_NUM_ETH and #define it to be the 
> number of etherspiggots we have (replacing MAXCONTROLLERS - not a good 
> name if it ends up being more widely visible).
>
>   
How about creating a bitmap of present controllers. e.g.

<some header file>
#define ETH0 1<<0
#define ETH1 1<<1
#define ETH2 1<<2
...

<board config file>
#define ETH_PRESENT (ETH0 | ETH2)

and then you check for non-zero or count the bits. It doesn't scale 
well, as we found with CMDs, but 32 for now seems pretty big.
>> diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
>> index 344c649..e1dec5e 100644
>> --- a/drivers/qe/uec.c
>> +++ b/drivers/qe/uec.c
>> @@ -123,8 +123,54 @@ static uec_info_t eth4_uec_info = {
>>  	.enet_interface		= CFG_UEC4_INTERFACE_MODE,
>>  };
>>  #endif
>> +#ifdef CONFIG_UEC_ETH5
>> +static uec_info_t eth5_uec_info = {
>> +	.uf_info		= {
>> +		.ucc_num	= CFG_UEC5_UCC_NUM,
>> +		.rx_clock	= CFG_UEC5_RX_CLK,
>> +		.tx_clock	= CFG_UEC5_TX_CLK,
>> +		.eth_type	= CFG_UEC5_ETH_TYPE,
>> +	},
>> +#if (CFG_UEC5_ETH_TYPE == FAST_ETH)
>> +	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
>> +	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
>> +#else
>> +	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
>> +	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
>> +#endif
>> +	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
>> +	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
>> +	.tx_bd_ring_len		= 16,
>> +	.rx_bd_ring_len		= 16,
>> +	.phy_address		= CFG_UEC5_PHY_ADDR,
>> +	.enet_interface		= CFG_UEC5_INTERFACE_MODE,
>> +};
>> +#endif
>> +#ifdef CONFIG_UEC_ETH6
>> +static uec_info_t eth6_uec_info = {
>> +	.uf_info		= {
>> +		.ucc_num	= CFG_UEC6_UCC_NUM,
>> +		.rx_clock	= CFG_UEC6_RX_CLK,
>> +		.tx_clock	= CFG_UEC6_TX_CLK,
>> +		.eth_type	= CFG_UEC6_ETH_TYPE,
>> +	},
>> +#if (CFG_UEC6_ETH_TYPE == FAST_ETH)
>> +	.num_threads_tx		= UEC_NUM_OF_THREADS_1,
>> +	.num_threads_rx		= UEC_NUM_OF_THREADS_1,
>> +#else
>> +	.num_threads_tx		= UEC_NUM_OF_THREADS_4,
>> +	.num_threads_rx		= UEC_NUM_OF_THREADS_4,
>> +#endif
>> +	.riscTx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
>> +	.riscRx			= QE_RISC_ALLOCATION_RISC1_AND_RISC2,
>> +	.tx_bd_ring_len		= 16,
>> +	.rx_bd_ring_len		= 16,
>> +	.phy_address		= CFG_UEC6_PHY_ADDR,
>> +	.enet_interface		= CFG_UEC6_INTERFACE_MODE,
>> +};
>> +#endif
>>     
>
> Here I think my previous observation is still applicable: Is this 
> information in the fdt blob?  If not, we should add it (possibly 
> coordinating with the kernel folks.
>
>   
>> -#define MAXCONTROLLERS	(4)
>> +#define MAXCONTROLLERS	(6)
>>     
>
> CONFIG_NUM_ETH? (see above for discussion)
>
>   
>>  static struct eth_device *devlist[MAXCONTROLLERS];
>>  
>> diff --git a/include/asm-ppc/u-boot.h b/include/asm-ppc/u-boot.h
>> index 54ac01d..7451905 100644
>> --- a/include/asm-ppc/u-boot.h
>> +++ b/include/asm-ppc/u-boot.h
>> @@ -111,6 +111,12 @@ typedef struct bd_info {
>>  #ifdef CONFIG_HAS_ETH3
>>  	unsigned char   bi_enet3addr[6];
>>  #endif
>> +#ifdef CONFIG_HAS_ETH4
>> +	unsigned char   bi_enet4addr[6];
>> +#endif
>> +#ifdef CONFIG_HAS_ETH5
>> +	unsigned char   bi_enet5addr[6];
>> +#endif
>>     
>
> Bleah.  I would like to see the bd_info struct go away.  Please check
> if anybody really needs bi_enet[0-5]addr after looking into my
> suggestions/objections above.  If we cannot make bd_info go away, we
> should at least be able to stop making it bigger.
>
>   
It is becoming less and less relevant, but I expect strong resistance...
>> diff --git a/lib_ppc/board.c b/lib_ppc/board.c
>> index c02ac62..d440722 100644
>> --- a/lib_ppc/board.c
>> +++ b/lib_ppc/board.c
>> @@ -944,6 +944,36 @@ void board_init_r (gd_t *id, ulong dest_addr)
>>  	}
>>  #endif
>>  
>> +#ifdef CONFIG_HAS_ETH4
>> +	/* handle 5th ethernet address */
>> +	s = getenv("eth4addr");
>> +#if defined(CONFIG_XPEDITE1K) || defined(CONFIG_METROBOX) || defined(CONFIG_KAREF)
>> +	if (s == NULL)
>> +		board_get_enetaddr(bd->bi_enet4addr);
>> +	else
>> +#endif
>> +	for (i = 0; i < 6; ++i) {
>> +		bd->bi_enet4addr[i] = s ? simple_strtoul (s, &e, 16) : 0;
>> +		if (s)
>> +			s = (*e) ? e + 1 : e;
>> +	}
>> +#endif
>>     
>
> This is a different version of the repetitive code I complain about 
> above (common/cmd_bdinfo.c).  I would like to see a variant of Kumar's 
> loop instead of the #if defined() explosion.
>
> [snip duplicate but with different [N] code]
>
>   
>> diff --git a/net/eth.c b/net/eth.c
>> index 432dd60..ccd871a 100644
>> --- a/net/eth.c
>> +++ b/net/eth.c
>> @@ -180,6 +180,12 @@ int eth_initialize(bd_t *bis)
>>  #if defined(CONFIG_UEC_ETH4)
>>  	uec_initialize(3);
>>  #endif
>> +#if defined(CONFIG_UEC_ETH5)
>> +	uec_initialize(4);
>> +#endif
>> +#if defined(CONFIG_UEC_ETH6)
>> +	uec_initialize(5);
>> +#endif
>>     
>
> My kingdom for a loop!
>
>   
Don't worry, this will be gone soon.
>>  #if defined(FEC_ENET) || defined(CONFIG_ETHER_ON_FCC)
>>  	fec_initialize(bis);
>> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
>> index 46747d3..6e9c34f 100644
>> --- a/tools/env/fw_env.c
>> +++ b/tools/env/fw_env.c
>> @@ -157,6 +157,12 @@ static char default_environment[] = {
>>  #ifdef	CONFIG_ETH3ADDR
>>  	"eth3addr=" MK_STR (CONFIG_ETH3ADDR) "\0"
>>  #endif
>> +#ifdef	CONFIG_ETH4ADDR
>> +	"eth4addr=" MK_STR (CONFIG_ETH4ADDR) "\0"
>> +#endif
>> +#ifdef	CONFIG_ETH5ADDR
>> +	"eth5addr=" MK_STR (CONFIG_ETH5ADDR) "\0"
>> +#endif
>>  #ifdef	CONFIG_ETHPRIME
>>  	"ethprime=" CONFIG_ETHPRIME "\0"
>>  #endif
>>     
>
> Curmudgeonly yours,
> gvb
cheers,
Ben