Adds two more ethernet interface to 83xx

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 <[email protected]>
Date: Wed, 24 Sep 2008 17:21:47 -0400
Subject: [PATCH] Add two more ethernet interface for 83XX

Signed-off-by Richard Retanubun <[email protected]>

---

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

README +2 -0

 		CONFIG_ETH3ADDR
+		CONFIG_ETH4ADDR
+		CONFIG_ETH5ADDR
 
 		Define a default value for Ethernet address to use
 		for the respective Ethernet interface, in case this

common/cmd_bdinfo.c +0 -0

index f4d9d40..67cc64f 100644

Quoted 10 lines · On Thu, 25 Sep 2008 08:53:24 -0400 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 <[email protected]>
Date: Wed, 24 Sep 2008 17:21:47 -0400
Subject: [PATCH] Add two more ethernet interface for 83XX

Signed-off-by Richard Retanubun <[email protected]>

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

Quoted 1 line

---

..i.e, here.

Quoted 11 lines

 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.

Quoted 6 lines

README +0 -0

index ccd839c..8802304 100644
@@ -1095,8 +1095,11 @@ The following options need to be configured:
 

README The following options need to be configured: · line 1099

Quoted 4 lines

README +1 -0

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

Quoted 14 lines

README +2 -0

 		CONFIG_ETH3ADDR
+		CONFIG_ETH4ADDR
+		CONFIG_ETH5ADDR
 
 		Define a default value for Ethernet address to use
 		for the respective Ethernet interface, in case this

common/cmd_bdinfo.c +0 -0

index f4d9d40..67cc64f 100644
@@ -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
 

common/cmd_bdinfo.c int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) · line 99

Quoted 7 lines

common/cmd_bdinfo.c +2 -1

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

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.

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

README +0 -0

index ccd839c..8802304 100644

README +4 -0

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

common/env_common.c +0 -0

index 77f9944..0fee3af 100644

common/env_embedded.c +0 -0

index 77e5619..e79f843 100644
@@ -135,6 +135,12 @@ env_t environment __PPCENV__ = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"

cpu/mpc83xx/fdt.c +0 -0

 	fdt_fixup_ethernet(blob);
 #endif
 

drivers/qe/uec.c +36 -0

index 344c649..e1dec5e 100644
@@ -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,

include/asm-ppc/u-boot.h +0 -0

index 54ac01d..7451905 100644

Quoted 6 lines · Kim Phillips

Quoted 1 line · On Thu, 25 Sep 2008 08:53:24 -0400 richardretanubun…

---

..i.e, here.

Understood, thanks for the clarification, will heed for future patches.

Quoted 4 lines

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.

Quoted 6 lines

Quoted 6 lines

README +0 -0

index ccd839c..8802304 100644
@@ -1095,8 +1095,11 @@ The following options need to be configured:
 

Quoted 10 lines

README The following options need to be configured: · line 1099

Quoted 4 lines

README +1 -0

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

Quoted 5 lines

Quoted 5 lines

README +1 -0

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

Quoted 9 lines

README int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) · line 99

Quoted 7 lines

README +2 -1

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

Quoted 10 lines

Quoted 10 lines

README The following options need to be configured: · line 1099

Quoted 5 lines

- Ethernet address:

README +1 -0

 		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.

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>

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.

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.

README The following options need to be configured: · line 1099

Quoted 5 lines

- Ethernet address:

README +1 -0

 		CONFIG_ETHADDR
+		CONFIG_ETH1ADDR
 		CONFIG_ETH2ADDR

hmm..historically ETHADDR has been the implicit ETH1ADDR. Did you mean

Dear richardretanubun,

Review The following options need to be configured: · line 1102

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

+++ b/README

(quoted diff) +3 -0

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

Quoted 7 lines

index f4d9d40..67cc64f 100644

common/cmd_bdinfo.c +0 -0

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

common/cmd_bdinfo.c int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) · line 99

Quoted 7 lines

common/cmd_bdinfo.c +2 -1

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

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

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.

README +0 -0

index ccd839c..8802304 100644
@@ -1095,8 +1095,11 @@ The following options need to be configured:
 

common/cmd_bdinfo.c +6 -1

index f4d9d40..67cc64f 100644
@@ -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]);

common/env_common.c +6 -0

index 77f9944..0fee3af 100644
@@ -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

common/env_embedded.c +0 -0

index 77e5619..e79f843 100644
@@ -135,6 +135,12 @@ env_t environment __PPCENV__ = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr="	MK_STR(CONFIG_ETH3ADDR)		"\0"

drivers/qe/uec.c +36 -0

index 344c649..e1dec5e 100644
@@ -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,

drivers/qe/uec.c +0 -0

 static struct eth_device *devlist[MAXCONTROLLERS];
 

include/asm-ppc/u-boot.h +0 -0

index 54ac01d..7451905 100644

lib_ppc/board.c +4 -0

index c02ac62..d440722 100644
@@ -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)

net/eth.c +0 -0

index 432dd60..ccd871a 100644

net/eth.c +0 -0

 #if defined(FEC_ENET) || defined(CONFIG_ETHER_ON_FCC)
 	fec_initialize(bis);

tools/env/fw_env.c +0 -0

index 46747d3..6e9c34f 100644
@@ -157,6 +157,12 @@ static char default_environment[] = {
 #ifdef	CONFIG_ETH3ADDR
 	"eth3addr=" MK_STR (CONFIG_ETH3ADDR) "\0"

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.

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

Quoted 4 lines · 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.

[snip]

Quoted 7 lines

README +0 -0

index ccd839c..8802304 100644
@@ -1095,8 +1095,11 @@ The following options need to be configured:
 

README The following options need to be configured: · line 1105

Quoted 10 lines

README +3 -0

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

Quoted 27 lines

common/cmd_bdinfo.c +7 -1

index f4d9d40..67cc64f 100644
@@ -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]);
+	}

common/cmd_bdinfo.c int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) · line 135

Quoted 10 lines

common/cmd_bdinfo.c +9 -0

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

Quoted 30 lines

common/env_common.c +6 -0

index 77f9944..0fee3af 100644
@@ -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

common/env_embedded.c +5 -0

index 77e5619..e79f843 100644
@@ -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"

common/env_embedded.c env_t environment __PPCENV__ = { · line 146

Quoted 5 lines

common/env_embedded.c +1 -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.)

Quoted 7 lines

cpu/mpc83xx/fdt.c +0 -0

index 39bd9dc..3e3e1c8 100644
@@ -52,7 +52,8 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 		fdt_fixup_crypto_node(blob, 0x0204);
 

cpu/mpc83xx/fdt.c void ft_cpu_setup(void *blob, bd_t *bd) · line 59

Quoted 7 lines

cpu/mpc83xx/fdt.c +2 -1

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

Quoted 45 lines

drivers/qe/uec.c +37 -0

index 344c649..e1dec5e 100644
@@ -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,

drivers/qe/uec.c static uec_info_t eth4_uec_info = { · line 172

Quoted 10 lines

drivers/qe/uec.c +9 -0

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

drivers/qe/uec.c static uec_info_t eth4_uec_info = { · line 174

Quoted 3 lines

drivers/qe/uec.c +1 -1

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

CONFIG_NUM_ETH? (see above for discussion)

Quoted 14 lines

drivers/qe/uec.c +0 -0

 static struct eth_device *devlist[MAXCONTROLLERS];
 

include/asm-ppc/u-boot.h +4 -0

index 54ac01d..7451905 100644
@@ -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

include/asm-ppc/u-boot.h typedef struct bd_info { · line 120

Quoted 3 lines

include/asm-ppc/u-boot.h +2 -0

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

Quoted 13 lines

lib_ppc/board.c +5 -0

index c02ac62..d440722 100644
@@ -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)

lib_ppc/board.c void board_init_r (gd_t *id, ulong dest_addr) · line 961

Quoted 10 lines

lib_ppc/board.c +9 -0

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

Quoted 9 lines

net/eth.c +1 -0

index 432dd60..ccd871a 100644
@@ -180,6 +180,12 @@ int eth_initialize(bd_t *bis)
 #if defined(CONFIG_UEC_ETH4)
 	uec_initialize(3);
 #endif
+#if defined(CONFIG_UEC_ETH5)

net/eth.c int eth_initialize(bd_t *bis) · line 189

Quoted 6 lines

net/eth.c +5 -0

+	uec_initialize(4);
+#endif
+#if defined(CONFIG_UEC_ETH6)
+	uec_initialize(5);
+#endif
 

My kingdom for a loop!

Quoted 15 lines

net/eth.c +0 -0

 #if defined(FEC_ENET) || defined(CONFIG_ETHER_ON_FCC)
 	fec_initialize(bis);

tools/env/fw_env.c +5 -0

index 46747d3..6e9c34f 100644
@@ -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"

tools/env/fw_env.c static char default_environment[] = { · line 168

Quoted 5 lines

tools/env/fw_env.c +1 -0

+#endif
 #ifdef	CONFIG_ETHPRIME
 	"ethprime=" CONFIG_ETHPRIME "\0"
 #endif

Curmudgeonly yours,
gvb

Dear richardretanubun,

Quoted 1 line · In message <[email protected]> you wrote:

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

So you will submit a V3?

Best regards,

Wolfgang Denk

Quoted 5 lines · Wolfgang Denk

Dear richardretanubun,

Quoted 1 line · In message <[email protected]> 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.

Cheers,
- Richard

Quoted 4 lines · On Mon, 29 Sep 2008 18:28:23 -0400 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.

Quoted 1 line

Signed-off-by Richard Retanubun <[email protected]>

Acked-by Kim Phillips <[email protected]>

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

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 <[email protected]>

Quoted 4 lines · On Mon, 29 Sep 2008 18:28:23 -0400 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.

Quoted 2 lines

Signed-off-by Richard Retanubun <[email protected]>

Acked-by Kim Phillips <[email protected]>

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?

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 <[email protected]>

---

Fixed compiler warning "declared but unused" eth5_uec_info and eth6_uec_info.

Signed-off-by Richard Retanubun <[email protected]>

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

drivers/qe/uec.c +1 -0

index e1dec5e..582a1c0 100644
@@ -1414,6 +1414,14 @@ int uec_initialize(int index)
 #ifdef CONFIG_UEC_ETH4
        uec_info = &eth4_uec_info;
 #endif
+    } else if (index == 4) {