richardretanubun wrote: > Added for convenience for other platforms that uses MPC8360 (has 8 UCC). > 6 eth interface is chosen because the platform I am using combines > UCC1&2 and UCC3&4 as gigEth and the other 4 UCC as 10/100 Eth.
We are about to fall into a never ending explosion of #define CONFIG_ETHnnnADDR. This is bad. Very bad. :-( Kumar solved this problem WRT cpu/mpc83xx/fdt.c fdt_fixup_ethernet(void *fdt) (and other CPUs) by using the device tree to find all the ethernets and configure them. <http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/45554> Is bdinfo relevant? If not, return; /* that was easy */ If bdinfo is still relevant, look at how Kumar solved the #define explosion in cpu/*/fdt.c. You should be able to adapt it in the code you augmented with CONFIG_ETH[45]ADDR. Suggestion: Look at changing CONFIG_ETH*ADDR to CONFIG_ETH_FDT where that notation indicates the code should find the ethernet info in the fdt blob rather than #defines/env variables. One fly in the ointment is if we don't have a fdt blob to pick the ethernet parameters out of. Perhaps this is because we are going to load a fdt blob over the ethernet. We discussed having a default blob (probably the more elegant solution). A simpler and more "the way it is now" solution would be to define a limited number of #define CONFIG_ETH*ADDR values (I think 2 would be adequate, but at least limit ourselves to the current 4 max). Then bare board fdt blob loading would have to be over one/two/four fixed ethernet ports, but once a valid fdt blob was loaded all of the ethernets would be configured properly by u-boot. > --- > README | 3 ++ > common/cmd_bdinfo.c | 17 +++++++++++++++- > common/env_common.c | 6 +++++ > common/env_embedded.c | 6 +++++ > cpu/mpc83xx/fdt.c | 3 +- > drivers/qe/uec.c | 48 > +++++++++++++++++++++++++++++++++++++++++++++- > include/asm-ppc/u-boot.h | 6 +++++ > lib_ppc/board.c | 30 ++++++++++++++++++++++++++++ > net/eth.c | 6 +++++ > tools/env/fw_env.c | 6 +++++ > 10 files changed, 128 insertions(+), 3 deletions(-) > > diff --git a/README b/README > index ccd839c..8802304 100644 > --- a/README > +++ b/README > @@ -1095,8 +1095,11 @@ The following options need to be configured: > > - Ethernet address: > CONFIG_ETHADDR > + CONFIG_ETH1ADDR > CONFIG_ETH2ADDR > CONFIG_ETH3ADDR > + CONFIG_ETH4ADDR > + CONFIG_ETH5ADDR Bleah! Oh, right, I said that already. [snip] > +#if defined(CONFIG_HAS_ETH4) > + puts ("\neth4addr ="); > + for (i=0; i<6; ++i) { > + printf ("%c%02X", i ? ':' : ' ', bd->bi_enet4addr[i]); > + } > +#endif > + > +#if defined(CONFIG_HAS_ETH5) > + puts ("\neth5addr ="); > + for (i=0; i<6; ++i) { > + printf ("%c%02X", i ? ':' : ' ', bd->bi_enet5addr[i]); > + } > +#endif > + No, don't add more cut'n'pastes. Bleah! Use a fdt_*() loop (a'la Kumar's code). This would have to be conditional on CONFIG_LIBFDT since not all boards use LIBFDT. If !defined(CONFIG_LIBFDT), use the existing code. > diff --git a/common/env_common.c b/common/env_common.c > index 77f9944..0fee3af 100644 > --- a/common/env_common.c > +++ b/common/env_common.c > @@ -91,6 +91,12 @@ uchar default_environment[] = { > #ifdef CONFIG_ETH3ADDR > "eth3addr=" MK_STR(CONFIG_ETH3ADDR) "\0" > #endif > +#ifdef CONFIG_ETH4ADDR > + "eth4addr=" MK_STR(CONFIG_ETH4ADDR) "\0" > +#endif > +#ifdef CONFIG_ETH5ADDR > + "eth5addr=" MK_STR(CONFIG_ETH5ADDR) "\0" > +#endif Bleah! I would prefer to cut our losses at CONFIG_ETH1ADDR (or CONFIG_ETH3ADDR), see lead-in discussion. > diff --git a/common/env_embedded.c b/common/env_embedded.c > index 77e5619..e79f843 100644 > --- a/common/env_embedded.c > +++ b/common/env_embedded.c > @@ -135,6 +135,12 @@ env_t environment __PPCENV__ = { > #ifdef CONFIG_ETH3ADDR > "eth3addr=" MK_STR(CONFIG_ETH3ADDR) "\0" > #endif > +#ifdef CONFIG_ETH4ADDR > + "eth4addr=" MK_STR(CONFIG_ETH4ADDR) "\0" > +#endif > +#ifdef CONFIG_ETH5ADDR > + "eth5addr=" MK_STR(CONFIG_ETH5ADDR) "\0" > +#endif > #ifdef CONFIG_ETHPRIME > "ethprime=" CONFIG_ETHPRIME "\0" > #endif Bleah! ...but I repeat myself. Do we need eth[1-5]?addr env variables? I don't think so. If we really do need them, they can be generated from the fdt blob a'la Kumar's loop. > diff --git a/cpu/mpc83xx/fdt.c b/cpu/mpc83xx/fdt.c > index 39bd9dc..3e3e1c8 100644 > --- a/cpu/mpc83xx/fdt.c > +++ b/cpu/mpc83xx/fdt.c > @@ -52,7 +52,8 @@ void ft_cpu_setup(void *blob, bd_t *bd) > fdt_fixup_crypto_node(blob, 0x0204); > > #if defined(CONFIG_HAS_ETH0) || defined(CONFIG_HAS_ETH1) ||\ > - defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) > + defined(CONFIG_HAS_ETH2) || defined(CONFIG_HAS_ETH3) ||\ > + defined(CONFIG_HAS_ETH4) || defined(CONFIG_HAS_ETH5) Bleah! I counter-propose a single CONFIG_ETH_FDT. Alternatively, if defined(CONFIG_HAS_ETH0), it is a good bet that you want to call fdt_fixup_ethernet(blob); and *that* routine handles up all defined ethernet spiggots (thanks, Kumar!). > fdt_fixup_ethernet(blob); > #endif > > diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c > index 344c649..e1dec5e 100644 > --- a/drivers/qe/uec.c > +++ b/drivers/qe/uec.c > @@ -123,8 +123,54 @@ static uec_info_t eth4_uec_info = { > .enet_interface = CFG_UEC4_INTERFACE_MODE, > }; > #endif > +#ifdef CONFIG_UEC_ETH5 > +static uec_info_t eth5_uec_info = { > + .uf_info = { > + .ucc_num = CFG_UEC5_UCC_NUM, > + .rx_clock = CFG_UEC5_RX_CLK, > + .tx_clock = CFG_UEC5_TX_CLK, > + .eth_type = CFG_UEC5_ETH_TYPE, > + }, > +#if (CFG_UEC5_ETH_TYPE == FAST_ETH) > + .num_threads_tx = UEC_NUM_OF_THREADS_1, > + .num_threads_rx = UEC_NUM_OF_THREADS_1, > +#else > + .num_threads_tx = UEC_NUM_OF_THREADS_4, > + .num_threads_rx = UEC_NUM_OF_THREADS_4, > +#endif > + .riscTx = QE_RISC_ALLOCATION_RISC1_AND_RISC2, > + .riscRx = QE_RISC_ALLOCATION_RISC1_AND_RISC2, > + .tx_bd_ring_len = 16, > + .rx_bd_ring_len = 16, > + .phy_address = CFG_UEC5_PHY_ADDR, > + .enet_interface = CFG_UEC5_INTERFACE_MODE, > +}; > +#endif > +#ifdef CONFIG_UEC_ETH6 > +static uec_info_t eth6_uec_info = { > + .uf_info = { > + .ucc_num = CFG_UEC6_UCC_NUM, > + .rx_clock = CFG_UEC6_RX_CLK, > + .tx_clock = CFG_UEC6_TX_CLK, > + .eth_type = CFG_UEC6_ETH_TYPE, > + }, > +#if (CFG_UEC6_ETH_TYPE == FAST_ETH) > + .num_threads_tx = UEC_NUM_OF_THREADS_1, > + .num_threads_rx = UEC_NUM_OF_THREADS_1, > +#else > + .num_threads_tx = UEC_NUM_OF_THREADS_4, > + .num_threads_rx = UEC_NUM_OF_THREADS_4, > +#endif > + .riscTx = QE_RISC_ALLOCATION_RISC1_AND_RISC2, > + .riscRx = QE_RISC_ALLOCATION_RISC1_AND_RISC2, > + .tx_bd_ring_len = 16, > + .rx_bd_ring_len = 16, > + .phy_address = CFG_UEC6_PHY_ADDR, > + .enet_interface = CFG_UEC6_INTERFACE_MODE, > +}; > +#endif Is this in the fdt blob? If not, we should add it (possibly coordinating with the kernel folks. > -#define MAXCONTROLLERS (4) > +#define MAXCONTROLLERS (6) > > static struct eth_device *devlist[MAXCONTROLLERS]; Hmmm, I don't have an clever answer for this one. Looks like we will need a MAXCONTROLLERS to make our devlist[]. > diff --git a/include/asm-ppc/u-boot.h b/include/asm-ppc/u-boot.h > index 54ac01d..7451905 100644 > --- a/include/asm-ppc/u-boot.h > +++ b/include/asm-ppc/u-boot.h > @@ -111,6 +111,12 @@ typedef struct bd_info { > #ifdef CONFIG_HAS_ETH3 > unsigned char bi_enet3addr[6]; > #endif > +#ifdef CONFIG_HAS_ETH4 > + unsigned char bi_enet4addr[6]; > +#endif > +#ifdef CONFIG_HAS_ETH5 > + unsigned char bi_enet5addr[6]; > +#endif More evil. I would like to se the bd_info struct go away. Please check if anybody really needs bi_enet[0-5]addr after looking into my suggestions/objections above. If we cannot make bd_info go away, we should be able to stop making it bigger. [snipped remainder of the same] Thanks, gvb _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
