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