[U-Boot] [PATCH] [83xx] 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 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
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
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_ETH4ADDRCONFIG_ETH5ADDRDefine 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
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_ETH1ADDRhmm..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_ETH4ADDRCONFIG_ETH5ADDRDefine 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
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_ETH1ADDRhmm..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
Dear Kim Phillips,
In message 20080925154546.4ff2cae2.kim.phillips@freescale.com you wrote:
- Ethernet address: CONFIG_ETHADDR
CONFIG_ETH1ADDRhmm..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
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
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_ETH4ADDRCONFIG_ETH5ADDRCONFIG_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
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_ETH4ADDRCONFIG_ETH5ADDRCONFIG_ETH6ADDRSorry, 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
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
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
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
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
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
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
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
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
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 = ð4_uec_info; #endif + } else if (index == 4) { +#ifdef CONFIG_UEC_ETH5 + uec_info = ð5_uec_info; +#endif + } else if (index == 5) { +#ifdef CONFIG_UEC_ETH6 + uec_info = ð6_uec_info; +#endif } else { printf("%s: index is illegal.\n", __FUNCTION__); return -EINVAL;
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 = ð4_uec_info; #endif
- } else if (index == 4) {
+#ifdef CONFIG_UEC_ETH5
uec_info = ð5_uec_info;+#endif
- } else if (index == 5) {
+#ifdef CONFIG_UEC_ETH6
uec_info = ð6_uec_info;+#endif } else { printf("%s: index is illegal.\n", __FUNCTION__); return -EINVAL;
Applied to net/next
regards, Ben
Dear Kim Phillips,
In message 20080925145035.cebab0df.kim.phillips@freescale.com you wrote:
- Ethernet address: CONFIG_ETHADDR
CONFIG_ETH1ADDRhmm..historically ETHADDR has been the implicit ETH1ADDR. Did you mean
No. ETHADDR is ETH0ADDR
Best regards,
Wolfgang Denk
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_ETH1ADDRCONFIG_ETH4ADDRCONFIG_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
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
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_ETH1ADDRCONFIG_ETH2ADDR CONFIG_ETH3ADDR
CONFIG_ETH4ADDRCONFIG_ETH5ADDRDefine 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
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
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_ETH1ADDRCONFIG_ETH2ADDR CONFIG_ETH3ADDR
CONFIG_ETH4ADDRCONFIG_ETH5ADDRDefine 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
participants (6)
-
Ben Warren -
Jerry Van Baren -
Jerry Van Baren -
Kim Phillips -
richardretanubun -
Wolfgang Denk