[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Ben Warren]

Add new board_eth_init() function, moving all TSEC initializations to board
code.

Signed-off-by: Ben Warren <biggerbadderben@gmail.com>
---
board/atum8548/atum8548.c                 |   18 +++++++++++++++
board/freescale/mpc8313erdb/mpc8313erdb.c |   12 ++++++++++
board/freescale/mpc8315erdb/mpc8315erdb.c |   12 ++++++++++
board/freescale/mpc8349emds/mpc8349emds.c |   12 ++++++++++
board/freescale/mpc8349itx/mpc8349itx.c   |   12 ++++++++++
board/freescale/mpc837xemds/mpc837xemds.c |   12 ++++++++++
board/freescale/mpc837xerdb/mpc837xerdb.c |   12 ++++++++++
board/freescale/mpc8540ads/mpc8540ads.c   |   22 ++++++++++++++++++
board/freescale/mpc8541cds/mpc8541cds.c   |   12 ++++++++++
board/freescale/mpc8544ds/mpc8544ds.c     |   18 +++++++++++++++
board/freescale/mpc8548cds/mpc8548cds.c   |   18 +++++++++++++++
board/freescale/mpc8555cds/mpc8555cds.c   |   12 ++++++++++
board/freescale/mpc8560ads/mpc8560ads.c   |   12 ++++++++++
board/freescale/mpc8568mds/mpc8568mds.c   |   12 ++++++++++
board/freescale/mpc8641hpcn/mpc8641hpcn.c |   18 +++++++++++++++
board/mpc8540eval/mpc8540eval.c           |   23 +++++++++++++++++++
board/pm854/pm854.c                       |   23 +++++++++++++++++++
board/pm856/pm856.c                       |   12 ++++++++++
board/sbc8349/sbc8349.c                   |   12 ++++++++++
board/sbc8548/sbc8548.c                   |   18 +++++++++++++++
board/sbc8560/sbc8560.c                   |    9 +++++++
board/sbc8641d/sbc8641d.c                 |   18 +++++++++++++++
board/stxgp3/stxgp3.c                     |   12 ++++++++++
board/stxssa/stxssa.c                     |   12 ++++++++++
board/tqm834x/tqm834x.c                   |   12 ++++++++++
board/tqm85xx/tqm85xx.c                   |   22 ++++++++++++++++++
include/netdev.h                          |   35 
+++++++++++++++++++++++++++++
net/eth.c                                 |   25 ++++++--------------
28 files changed, 430 insertions(+), 17 deletions(-)
create mode 100644 include/netdev.h

diff --git a/board/atum8548/atum8548.c b/board/atum8548/atum8548.c
index 2f6ae29..d6bd8ae 100644
--- a/board/atum8548/atum8548.c
+++ b/board/atum8548/atum8548.c
@@ -34,6 +34,7 @@
#include <miiphy.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)
extern void ddr_enable_ecc(unsigned int dram_size);
@@ -417,3 +418,20 @@ ft_board_setup(void *blob, bd_t *bd)
	}
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8313erdb/mpc8313erdb.c 
b/board/freescale/mpc8313erdb/mpc8313erdb.c
index 42019fb..fdbc9bf 100644
--- a/board/freescale/mpc8313erdb/mpc8313erdb.c
+++ b/board/freescale/mpc8313erdb/mpc8313erdb.c
@@ -28,6 +28,7 @@
#endif
#include <pci.h>
#include <mpc83xx.h>
+#include <netdev.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -107,3 +108,14 @@ void ft_board_setup(void *blob, bd_t *bd)
#endif
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8315erdb/mpc8315erdb.c 
b/board/freescale/mpc8315erdb/mpc8315erdb.c
index 7af36dd..136b0aa 100644
--- a/board/freescale/mpc8315erdb/mpc8315erdb.c
+++ b/board/freescale/mpc8315erdb/mpc8315erdb.c
@@ -30,6 +30,7 @@
#endif
#include <pci.h>
#include <mpc83xx.h>
+#include <netdev.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -130,3 +131,14 @@ void ft_board_setup(void *blob, bd_t *bd)
#endif
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8349emds/mpc8349emds.c 
b/board/freescale/mpc8349emds/mpc8349emds.c
index 6c82596..59ace6c 100644
--- a/board/freescale/mpc8349emds/mpc8349emds.c
+++ b/board/freescale/mpc8349emds/mpc8349emds.c
@@ -30,6 +30,7 @@
#include <spi.h>
#include <miiphy.h>
#include <spd_sdram.h>
+#include <netdev.h>

#if defined(CONFIG_OF_LIBFDT)
#include <libfdt.h>
@@ -287,3 +288,14 @@ void ft_board_setup(void *blob, bd_t *bd)
#endif
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8349itx/mpc8349itx.c 
b/board/freescale/mpc8349itx/mpc8349itx.c
index 972361f..8f7ae72 100644
--- a/board/freescale/mpc8349itx/mpc8349itx.c
+++ b/board/freescale/mpc8349itx/mpc8349itx.c
@@ -25,6 +25,7 @@
#include <mpc83xx.h>
#include <i2c.h>
#include <miiphy.h>
+#include <netdev.h>
#ifdef CONFIG_PCI
#include <asm/mpc8349_pci.h>
#include <pci.h>
@@ -387,3 +388,14 @@ void ft_board_setup(void *blob, bd_t *bd)
#endif
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc837xemds/mpc837xemds.c 
b/board/freescale/mpc837xemds/mpc837xemds.c
index e57a53f..1ebd23d 100644
--- a/board/freescale/mpc837xemds/mpc837xemds.c
+++ b/board/freescale/mpc837xemds/mpc837xemds.c
@@ -13,6 +13,7 @@
#include <common.h>
#include <i2c.h>
#include <spd_sdram.h>
+#include <netdev.h>
#if defined(CONFIG_OF_LIBFDT)
#include <libfdt.h>
#endif
@@ -127,3 +128,14 @@ void ft_board_setup(void *blob, bd_t *bd)
#endif
}
#endif /* CONFIG_OF_BOARD_SETUP */
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc837xerdb/mpc837xerdb.c 
b/board/freescale/mpc837xerdb/mpc837xerdb.c
index bed0fc3..ba9ba22 100644
--- a/board/freescale/mpc837xerdb/mpc837xerdb.c
+++ b/board/freescale/mpc837xerdb/mpc837xerdb.c
@@ -16,6 +16,7 @@
#include <i2c.h>
#include <asm/io.h>
#include <spd_sdram.h>
+#include <netdev.h>

#if defined(CFG_DRAM_TEST)
int
@@ -145,3 +146,14 @@ void ft_board_setup(void *blob, bd_t *bd)
	ft_cpu_setup(blob, bd);
}
#endif /* CONFIG_OF_BOARD_SETUP */
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8540ads/mpc8540ads.c 
b/board/freescale/mpc8540ads/mpc8540ads.c
index a951b9e..d227a2f 100644
--- a/board/freescale/mpc8540ads/mpc8540ads.c
+++ b/board/freescale/mpc8540ads/mpc8540ads.c
@@ -32,6 +32,7 @@
#include <spd_sdram.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)
extern void ddr_enable_ecc(unsigned int dram_size);
@@ -343,3 +344,24 @@ ft_board_setup(void *blob, bd_t *bd)
	}
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_MPC85XX_FEC)
+	tsec_initialize(bis, 2, CONFIG_MPC85XX_FEC_NAME);
+#else
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8541cds/mpc8541cds.c 
b/board/freescale/mpc8541cds/mpc8541cds.c
index 62c8d63..4317a16 100644
--- a/board/freescale/mpc8541cds/mpc8541cds.c
+++ b/board/freescale/mpc8541cds/mpc8541cds.c
@@ -30,6 +30,7 @@
#include <spd_sdram.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#include "../common/cadmus.h"
#include "../common/eeprom.h"
@@ -530,3 +531,14 @@ ft_pci_setup(void *blob, bd_t *bd)
	}
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8544ds/mpc8544ds.c 
b/board/freescale/mpc8544ds/mpc8544ds.c
index 8107016..54b5659 100644
--- a/board/freescale/mpc8544ds/mpc8544ds.c
+++ b/board/freescale/mpc8544ds/mpc8544ds.c
@@ -31,6 +31,7 @@
#include <miiphy.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#include "../common/pixis.h"

@@ -545,3 +546,20 @@ ft_board_setup(void *blob, bd_t *bd)
	}
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8548cds/mpc8548cds.c 
b/board/freescale/mpc8548cds/mpc8548cds.c
index dc39fbe..cebd92c 100644
--- a/board/freescale/mpc8548cds/mpc8548cds.c
+++ b/board/freescale/mpc8548cds/mpc8548cds.c
@@ -31,6 +31,7 @@
#include <miiphy.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#include "../common/cadmus.h"
#include "../common/eeprom.h"
@@ -543,3 +544,20 @@ ft_pci_setup(void *blob, bd_t *bd)
	}
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8555cds/mpc8555cds.c 
b/board/freescale/mpc8555cds/mpc8555cds.c
index 8acbba4..96a9b00 100644
--- a/board/freescale/mpc8555cds/mpc8555cds.c
+++ b/board/freescale/mpc8555cds/mpc8555cds.c
@@ -28,6 +28,7 @@
#include <spd_sdram.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#include "../common/cadmus.h"
#include "../common/eeprom.h"
@@ -530,3 +531,14 @@ ft_pci_setup(void *blob, bd_t *bd)
	}
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8560ads/mpc8560ads.c 
b/board/freescale/mpc8560ads/mpc8560ads.c
index 8d4b8a8..2d00a0a 100644
--- a/board/freescale/mpc8560ads/mpc8560ads.c
+++ b/board/freescale/mpc8560ads/mpc8560ads.c
@@ -34,6 +34,7 @@
#include <miiphy.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)
extern void ddr_enable_ecc(unsigned int dram_size);
@@ -562,3 +563,14 @@ ft_board_setup(void *blob, bd_t *bd)
	}
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8568mds/mpc8568mds.c 
b/board/freescale/mpc8568mds/mpc8568mds.c
index 4568aa1..b295d01 100644
--- a/board/freescale/mpc8568mds/mpc8568mds.c
+++ b/board/freescale/mpc8568mds/mpc8568mds.c
@@ -32,6 +32,7 @@
#include <ioports.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#include "bcsr.h"

@@ -561,3 +562,14 @@ ft_board_setup(void *blob, bd_t *bd)
	}
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/freescale/mpc8641hpcn/mpc8641hpcn.c 
b/board/freescale/mpc8641hpcn/mpc8641hpcn.c
index 31e7d67..4f240b9 100644
--- a/board/freescale/mpc8641hpcn/mpc8641hpcn.c
+++ b/board/freescale/mpc8641hpcn/mpc8641hpcn.c
@@ -29,6 +29,7 @@
#include <asm/io.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#include "../common/pixis.h"

@@ -415,3 +416,20 @@ get_board_sys_clk(ulong dummy)

	return val;
}
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+	return 0;
+}
diff --git a/board/mpc8540eval/mpc8540eval.c 
b/board/mpc8540eval/mpc8540eval.c
index 8328b3a..b436ab1 100644
--- a/board/mpc8540eval/mpc8540eval.c
+++ b/board/mpc8540eval/mpc8540eval.c
@@ -27,6 +27,7 @@
#include <asm/processor.h>
#include <asm/immap_85xx.h>
#include <spd_sdram.h>
+#include <netdev.h>

long int fixed_sdram (void);

@@ -243,3 +244,25 @@ long int fixed_sdram (void)
	return (CFG_SDRAM_SIZE * 1024 * 1024);
}
#endif	/* !defined(CONFIG_SPD_EEPROM) */
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_MPC85XX_FEC)
+	tsec_initialize(bis, 2, CONFIG_MPC85XX_FEC_NAME);
+#else
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+#endif
+
+	return 0;
+}
diff --git a/board/pm854/pm854.c b/board/pm854/pm854.c
index 5e7bf34..89e91a6 100644
--- a/board/pm854/pm854.c
+++ b/board/pm854/pm854.c
@@ -30,6 +30,7 @@
#include <asm/processor.h>
#include <asm/immap_85xx.h>
#include <spd_sdram.h>
+#include <netdev.h>

#if defined(CONFIG_DDR_ECC)
extern void ddr_enable_ecc(unsigned int dram_size);
@@ -285,3 +286,25 @@ pci_init_board(void)
	pci_mpc85xx_init(&hose);
#endif /* CONFIG_PCI */
}
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_MPC85XX_FEC)
+	tsec_initialize(bis, 2, CONFIG_MPC85XX_FEC_NAME);
+#else
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+#endif
+
+	return 0;
+}
diff --git a/board/pm856/pm856.c b/board/pm856/pm856.c
index 792d1e5..7c8d579 100644
--- a/board/pm856/pm856.c
+++ b/board/pm856/pm856.c
@@ -32,6 +32,7 @@
#include <ioports.h>
#include <spd_sdram.h>
#include <miiphy.h>
+#include <netdev.h>

#if defined(CONFIG_DDR_ECC)
extern void ddr_enable_ecc(unsigned int dram_size);
@@ -440,3 +441,14 @@ pci_init_board(void)
	pci_mpc85xx_init(&hose);
#endif /* CONFIG_PCI */
}
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/sbc8349/sbc8349.c b/board/sbc8349/sbc8349.c
index e89b6e8..5668203 100644
--- a/board/sbc8349/sbc8349.c
+++ b/board/sbc8349/sbc8349.c
@@ -32,6 +32,7 @@
#include <i2c.h>
#include <spd_sdram.h>
#include <miiphy.h>
+#include <netdev.h>
#if defined(CONFIG_OF_LIBFDT)
#include <libfdt.h>
#endif
@@ -240,3 +241,14 @@ void ft_board_setup(void *blob, bd_t *bd)
#endif
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/sbc8548/sbc8548.c b/board/sbc8548/sbc8548.c
index 8a6ced3..d97f04b 100644
--- a/board/sbc8548/sbc8548.c
+++ b/board/sbc8548/sbc8548.c
@@ -34,6 +34,7 @@
#include <miiphy.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)
extern void ddr_enable_ecc(unsigned int dram_size);
@@ -566,3 +567,20 @@ ft_board_setup(void *blob, bd_t *bd)
#endif
}
#endif
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+	return 0;
+}
diff --git a/board/sbc8560/sbc8560.c b/board/sbc8560/sbc8560.c
index 8df4f3a..4e33982 100644
--- a/board/sbc8560/sbc8560.c
+++ b/board/sbc8560/sbc8560.c
@@ -33,6 +33,7 @@
#include <ioports.h>
#include <spd_sdram.h>
#include <miiphy.h>
+#include <netdev.h>

long int fixed_sdram (void);

@@ -452,3 +453,11 @@ long int fixed_sdram (void)
	return CFG_SDRAM_SIZE * 1024 * 1024;
}
#endif	/* !defined(CONFIG_SPD_EEPROM) */
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+	return 0;
+}
diff --git a/board/sbc8641d/sbc8641d.c b/board/sbc8641d/sbc8641d.c
index b3dd9c8..45ffb4a 100644
--- a/board/sbc8641d/sbc8641d.c
+++ b/board/sbc8641d/sbc8641d.c
@@ -37,6 +37,7 @@
#include <spd_sdram.h>
#include <libfdt.h>
#include <fdt_support.h>
+#include <netdev.h>

#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER)
extern void ddr_enable_ecc (unsigned int dram_size);
@@ -415,3 +416,20 @@ unsigned long get_board_sys_clk (ulong dummy)

	return val;
}
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+	return 0;
+}
diff --git a/board/stxgp3/stxgp3.c b/board/stxgp3/stxgp3.c
index f04ffa8..7f3f882 100644
--- a/board/stxgp3/stxgp3.c
+++ b/board/stxgp3/stxgp3.c
@@ -37,6 +37,7 @@
#include <asm/io.h>
#include <spd_sdram.h>
#include <miiphy.h>
+#include <netdev.h>

long int fixed_sdram (void);

@@ -373,3 +374,14 @@ pci_init_board(void)
	pci_mpc85xx_init(&hose);
#endif /* CONFIG_PCI */
}
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/stxssa/stxssa.c b/board/stxssa/stxssa.c
index 08177e1..92febca 100644
--- a/board/stxssa/stxssa.c
+++ b/board/stxssa/stxssa.c
@@ -37,6 +37,7 @@
#include <asm/io.h>
#include <spd_sdram.h>
#include <miiphy.h>
+#include <netdev.h>

long int fixed_sdram (void);

@@ -396,3 +397,14 @@ pci_init_board(void)
	pci_mpc85xx_init(hose);
#endif /* CONFIG_PCI */
}
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/tqm834x/tqm834x.c b/board/tqm834x/tqm834x.c
index aea985c..cffc7b3 100644
--- a/board/tqm834x/tqm834x.c
+++ b/board/tqm834x/tqm834x.c
@@ -30,6 +30,7 @@
#include <miiphy.h>
#include <asm-ppc/mmu.h>
#include <pci.h>
+#include <netdev.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -431,3 +432,14 @@ static void set_ddr_config(void) {
#endif
	}
}
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+	return 0;
+}
diff --git a/board/tqm85xx/tqm85xx.c b/board/tqm85xx/tqm85xx.c
index 8fa0162..f8237a0 100644
--- a/board/tqm85xx/tqm85xx.c
+++ b/board/tqm85xx/tqm85xx.c
@@ -33,6 +33,7 @@
#include <asm/immap_85xx.h>
#include <ioports.h>
#include <flash.h>
+#include <netdev.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -417,3 +418,24 @@ int board_early_init_r (void)
	return (0);
}
#endif /* CONFIG_BOARD_EARLY_INIT_R */
+
+int board_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_TSEC1)
+	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
+#endif
+#if defined(CONFIG_TSEC2)
+	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
+#endif
+#if defined(CONFIG_MPC85XX_FEC)
+	tsec_initialize(bis, 2, CONFIG_MPC85XX_FEC_NAME);
+#else
+#if defined(CONFIG_TSEC3)
+	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
+#endif
+#if defined(CONFIG_TSEC4)
+	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
+#endif
+#endif
+	return 0;
+}
diff --git a/include/netdev.h b/include/netdev.h
new file mode 100644
index 0000000..19195fa
--- /dev/null
+++ b/include/netdev.h
@@ -0,0 +1,35 @@
+/*
+ * (C) Copyright 2008
+ * Benjamin Warren, biggerbadderben at gmail.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+/*
+ * netdev.h - definitions an prototypes for network devices
+ */
+
+#ifndef _NETDEV_H_
+#define _NETDEV_H_
+
+int tsec_initialize(bd_t * bis, int index, char *devname);
+
+#endif /* _NETDEV_H_ */
+
+
diff --git a/net/eth.c b/net/eth.c
index 16a6dcb..3d02272 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -28,6 +28,13 @@

#if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)

+int __board_eth_init(bd_t *bis)
+{
+	return 0;
+}
+
+int board_eth_init(bd_t *bis) __attribute((weak, 
alias("__board_eth_init")));
+
#ifdef CFG_GT_6426x
extern int gt6426x_eth_initialize(bd_t *bis);
#endif
@@ -55,7 +62,6 @@ extern int scc_initialize(bd_t*);
extern int skge_initialize(bd_t*);
extern int tsi108_eth_initialize(bd_t*);
extern int uli526x_initialize(bd_t *);
-extern int tsec_initialize(bd_t*, int, char *);
extern int npe_initialize(bd_t *);
extern int uec_initialize(int);
extern int bfin_EMAC_initialize(bd_t *);
@@ -164,6 +170,7 @@ int eth_initialize(bd_t *bis)
#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII)
	miiphy_init();
#endif
+	board_eth_init(bis);

#if defined(CONFIG_DB64360) || defined(CONFIG_CPCI750)
	mv6436x_eth_initialize(bis);
@@ -195,22 +202,6 @@ int eth_initialize(bd_t *bis)
#if defined(CONFIG_SK98)
	skge_initialize(bis);
#endif
-#if defined(CONFIG_TSEC1)
-	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
-#endif
-#if defined(CONFIG_TSEC2)
-	tsec_initialize(bis, 1, CONFIG_TSEC2_NAME);
-#endif
-#if defined(CONFIG_MPC85XX_FEC)
-	tsec_initialize(bis, 2, CONFIG_MPC85XX_FEC_NAME);
-#else
-#    if defined(CONFIG_TSEC3)
-	tsec_initialize(bis, 2, CONFIG_TSEC3_NAME);
-#    endif
-#    if defined(CONFIG_TSEC4)
-	tsec_initialize(bis, 3, CONFIG_TSEC4_NAME);
-#    endif
-#endif
#if defined(CONFIG_UEC_ETH1)
	uec_initialize(0);
#endif
-- 
1.5.2.5

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Shinya Kuribayashi]

Hi Ben,

with a quick glance, find the comments below.

Ben Warren wrote:
> diff --git a/include/netdev.h b/include/netdev.h
> new file mode 100644
> index 0000000..19195fa
> --- /dev/null
> +++ b/include/netdev.h
> @@ -0,0 +1,35 @@
> +/*
> + * (C) Copyright 2008
> + * Benjamin Warren, biggerbadderben at gmail.com
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +/*
> + * netdev.h - definitions an prototypes for network devices
> + */
> +
> +#ifndef _NETDEV_H_
> +#define _NETDEV_H_
> +
> +int tsec_initialize(bd_t * bis, int index, char *devname);
> +
> +#endif /* _NETDEV_H_ */
> +
> +

Do we need such controller dependent declarations in netdev.h?
FWIW board_eth_init() is reasonable for me.

And I know we need to go on a step-by-step basis, do you have any
plan to have register/unregister interfaces?

Ah, remove the last 2 lines :-)

> diff --git a/net/eth.c b/net/eth.c
> index 16a6dcb..3d02272 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -28,6 +28,13 @@
> 
> #if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
> 
> +int __board_eth_init(bd_t *bis)
> +{
> +	return 0;
> +}
> +
> +int board_eth_init(bd_t *bis) __attribute((weak, 
> alias("__board_eth_init")));
> +
> #ifdef CFG_GT_6426x
> extern int gt6426x_eth_initialize(bd_t *bis);
> #endif

[This comment is not for Ben, but for everyone.]

Do we need such alias to empty function? Only __attribute((weak)) is
enough for me. If we have some default behavior which isn't empty, I
can see the weak & alias. Thought?

  Shinya

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Ben Warren]

Gaaah!  Sorry, forgot to CC the list.

On Sat, Mar 22, 2008 at 8:05 AM, Ben Warren <biggerbadderben@gmail.com> wrote:
>
> On Sat, Mar 22, 2008 at 1:03 AM, Shinya Kuribayashi
>  <skuribay@ruby.dti.ne.jp> wrote:
>  > Hi Ben,
>  >
>  >  with a quick glance, find the comments below.
>  >
>  >
>  >
>  >  Ben Warren wrote:
>  >  > diff --git a/include/netdev.h b/include/netdev.h
>  >  > new file mode 100644
>  >  > index 0000000..19195fa
>  >  > --- /dev/null
>  >  > +++ b/include/netdev.h
>  >  > @@ -0,0 +1,35 @@
>  >  > +/*
>  >  > + * (C) Copyright 2008
>  >  > + * Benjamin Warren, biggerbadderben at gmail.com
>  >  > + *
>  >  > + * See file CREDITS for list of people who contributed to this
>  >  > + * project.
>  >  > + *
>  >  > + * This program is free software; you can redistribute it and/or
>  >  > + * modify it under the terms of the GNU General Public License as
>  >  > + * published by the Free Software Foundation; either version 2 of
>  >  > + * the License, or (at your option) any later version.
>  >  > + *
>  >  > + * This program is distributed in the hope that it will be useful,
>  >  > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>  >  > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.      See the
>  >  > + * GNU General Public License for more details.
>  >  > + *
>  >  > + * You should have received a copy of the GNU General Public License
>  >  > + * along with this program; if not, write to the Free Software
>  >  > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>  >  > + * MA 02111-1307 USA
>  >  > + */
>  >  > +
>  >  > +/*
>  >  > + * netdev.h - definitions an prototypes for network devices
>  >  > + */
>  >  > +
>  >  > +#ifndef _NETDEV_H_
>  >  > +#define _NETDEV_H_
>  >  > +
>  >  > +int tsec_initialize(bd_t * bis, int index, char *devname);
>  >  > +
>  >  > +#endif /* _NETDEV_H_ */
>  >  > +
>  >  > +
>  >
>  >  Do we need such controller dependent declarations in netdev.h?
>  >  FWIW board_eth_init() is reasonable for me.
>  >
>
>  This was done to prevent the board code from complaining at compile
>  time.  I have much bigger plans than initialize() prototypes for this
>  file, but this is all it needs for now.  Another option I considered
>  here was to #include netdev from within common.h, but I've never been
>  a fan of nesting header files.
>
>
>  >  And I know we need to go on a step-by-step basis, do you have any
>  >  plan to have register/unregister interfaces?
>  >
>  >  Ah, remove the last 2 lines :-)
>  >
>  >
>  >  > diff --git a/net/eth.c b/net/eth.c
>  >  > index 16a6dcb..3d02272 100644
>  >  > --- a/net/eth.c
>  >  > +++ b/net/eth.c
>  >  > @@ -28,6 +28,13 @@
>  >  >
>  >  > #if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
>  >  >
>  >  > +int __board_eth_init(bd_t *bis)
>  >  > +{
>  >  > +     return 0;
>  >  > +}
>  >  > +
>  >  > +int board_eth_init(bd_t *bis) __attribute((weak,
>  >  > alias("__board_eth_init")));
>  >  > +
>  >  > #ifdef CFG_GT_6426x
>  >  > extern int gt6426x_eth_initialize(bd_t *bis);
>  >  > #endif
>  >
>  >  [This comment is not for Ben, but for everyone.]
>  >
>  >  Do we need such alias to empty function? Only __attribute((weak)) is
>  >  enough for me. If we have some default behavior which isn't empty, I
>  >  can see the weak & alias. Thought?
>  >
>
>  I plead ignorance on this one.  I've never used weak functions before,
>  and every example I found within U-boot, GCC documentation and Google
>  had the alias.  I'll try to get rid of it.
>
>  >   Shinya
>  >
>  >
>  Thanks for the feedback!
>
>  regards,
>  Ben
>

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Vlad Lungu]

Shinya Kuribayashi wrote:
>> +int board_eth_init(bd_t *bis) __attribute((weak, 
>> alias("__board_eth_init")));
>> +
>> #ifdef CFG_GT_6426x
>> extern int gt6426x_eth_initialize(bd_t *bis);
>> #endif
>>     
>
> [This comment is not for Ben, but for everyone.]
>
> Do we need such alias to empty function? Only __attribute((weak)) is
> enough for me. If we have some default behavior which isn't empty, I
> can see the weak & alias. Thought?
>
>   
Weak functions with no alias are NULL. You can test the value of 
board_eth_init before calling
or you can provide an alias so there are no surprises. If you don't do 
one or the other... exceptions happen.
Personally, I prefer to have an alias.
Vlad

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Shinya Kuribayashi]

Vlad Lungu wrote:
> Shinya Kuribayashi wrote:
>>> +int board_eth_init(bd_t *bis) __attribute((weak, 
>>> alias("__board_eth_init")));
>>> +
>>> #ifdef CFG_GT_6426x
>>> extern int gt6426x_eth_initialize(bd_t *bis);
>>> #endif
>>>     
>>
>> [This comment is not for Ben, but for everyone.]
>>
>> Do we need such alias to empty function? Only __attribute((weak)) is
>> enough for me. If we have some default behavior which isn't empty, I
>> can see the weak & alias. Thought?
>>
>>   
> Weak functions with no alias are NULL. You can test the value of 
> board_eth_init before calling
> or you can provide an alias so there are no surprises. If you don't do 
> one or the other... exceptions happen.
> Personally, I prefer to have an alias.

Thanks for clarifying. Now what I feel annoying is turned out to having
an empty function with *another* name, such as `__foo() against foo()'.

Find the patch below:
 - change weak usage
 - change netdev.h usage

FWIW I think this is easier to read wrt the relationship between `weak'
and `overridden'. I'm not going to argue with someone, though.

---

 board/atum8548/atum8548.c |    2 ++
 include/netdev.h          |    2 +-
 net/eth.c                 |    4 +---
 3 files changed, 4 insertions(+), 4 deletions(-)


diff --git a/board/atum8548/atum8548.c b/board/atum8548/atum8548.c
index d6bd8ae..01a5134 100644
--- a/board/atum8548/atum8548.c
+++ b/board/atum8548/atum8548.c
@@ -421,6 +421,8 @@ ft_board_setup(void *blob, bd_t *bd)
 
 int board_eth_init(bd_t *bis)
 {
+	extern int tsec_initialize(bd_t * bis, int index, char *devname);
+
 #if defined(CONFIG_TSEC1)
 	tsec_initialize(bis, 0, CONFIG_TSEC1_NAME);
 #endif
diff --git a/include/netdev.h b/include/netdev.h
index 19195fa..7cba0a7 100644
--- a/include/netdev.h
+++ b/include/netdev.h
@@ -28,7 +28,7 @@
 #ifndef _NETDEV_H_
 #define _NETDEV_H_
 
-int tsec_initialize(bd_t * bis, int index, char *devname);
+extern int board_eth_init(bd_t *bis);
 
 #endif /* _NETDEV_H_ */
 
diff --git a/net/eth.c b/net/eth.c
index 3d02272..80432a1 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -28,13 +28,11 @@
 
 #if defined(CONFIG_CMD_NET) && defined(CONFIG_NET_MULTI)
 
-int __board_eth_init(bd_t *bis)
+int __attribute((weak)) board_eth_init(bd_t *bis)
 {
 	return 0;
 }
 
-int board_eth_init(bd_t *bis) __attribute((weak, alias("__board_eth_init")));
-
 #ifdef CFG_GT_6426x
 extern int gt6426x_eth_initialize(bd_t *bis);
 #endif

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Haavard Skinnemoen]

On Sun, 23 Mar 2008 15:19:55 +0900
Shinya Kuribayashi <skuribay@ruby.dti.ne.jp> wrote:

>  int board_eth_init(bd_t *bis)
>  {
> +	extern int tsec_initialize(bd_t * bis, int index, char *devname);

Eww...no, please don't do that. Function prototypes belong in header
files. By adding extern declarations all over the place, the compiler
won't notice if the caller and callee get out of sync.

Having a header file full of driver-specific declarations does increase
the chances of conflicts, but I think it's worth it. It's definitely an
improvement over the old code. And the conflicts will be absolutely
trivial.

Haavard

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Shinya Kuribayashi]

Haavard Skinnemoen wrote:
> Having a header file full of driver-specific declarations does increase
> the chances of conflicts, but I think it's worth it. It's definitely an
> improvement over the old code. And the conflicts will be absolutely
> trivial.

I just thought that, a header file full of driver-specific declarations,
is somewhat ugly. But I don't have a definite opinion now, so will leave
this up to everyone.

Thanks,

  Shinya

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Haavard Skinnemoen]

On Wed, 26 Mar 2008 20:39:26 +0900
Shinya Kuribayashi <skuribay@ruby.dti.ne.jp> wrote:

> I just thought that, a header file full of driver-specific declarations,
> is somewhat ugly.

The functions must be declared somewhere, but declaring them at the
callsite is not only ugly, but error-prone as well.

One alternative might be to add one header file per driver and put the
declarations there. Maybe we should add a include/netdev directory for
network driver-specific header files?

Haavard

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Ben Warren]

Haavard Skinnemoen wrote:
> On Wed, 26 Mar 2008 20:39:26 +0900
> Shinya Kuribayashi <skuribay@ruby.dti.ne.jp> wrote:
>
>   
>> I just thought that, a header file full of driver-specific declarations,
>> is somewhat ugly.
>>     
>
> The functions must be declared somewhere, but declaring them at the
> callsite is not only ugly, but error-prone as well.
>
> One alternative might be to add one header file per driver and put the
> declarations there. Maybe we should add a include/netdev directory for
> network driver-specific header files?
>
> Haavard
>
>   
No, I like the way I did it with a single header file for all 
interfaces.  It's not ugly - externs in every board file or a directory 
of tiny header files or the existing #ifdef mess is ugly.  One header 
file means one-stop-shopping for finding a controller's interface.  The 
only thing that might be better is to #include netdev.h in common.h, 
getting rid of the requirement to include it in each board file.  OTOH, 
cascading header files is itself an ugly practice.  In an entirely 
biased way, I think the way I've implemented this is a big improvement 
over the status quo and should make things easier going forward.

regards,
Ben

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Haavard Skinnemoen]

On Wed, 26 Mar 2008 10:15:20 -0400
Ben Warren <biggerbadderben@gmail.com> wrote:

> The 
> only thing that might be better is to #include netdev.h in common.h, 
> getting rid of the requirement to include it in each board file.  OTOH, 
> cascading header files is itself an ugly practice.

I think we should try to make common.h smaller, not bigger. Requiring
the board code that does netdev setup to include netdev.h is very
appropriate IMO.

> In an entirely 
> biased way, I think the way I've implemented this is a big improvement 
> over the status quo and should make things easier going forward.

I completely agree.

Btw, if we require the list of prototypes in netdev.h to be kept in
alphabetical order (or some other sort of "random" order instead of
adding stuff at the end), we might reduce the probability of conflicts
slightly.

Haavard

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

Hi Ben,

On Saturday 22 March 2008, Ben Warren wrote:
> Add new board_eth_init() function, moving all TSEC initializations to board
> code.

I would prefer to move this init code not into the board code but into the 
platform/cpu code. Speaking for PPC4xx, here all configuration I know of have 
the same init code. And copying this into all board ports instead on one 
place in the cpu directory seems "imperfect".

Using a weak definition for this board_eth_init() would make this possible. 
The cpu code would define the function with the default 4xx init code, and 
the board code could overwrite it. But unfortunately you already introduced 
such a weak reference to this function in eth.c.

How can this be handled better? Any ideas?

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Wolfgang Denk]

In message <200803220731.23229.sr@denx.de> you wrote:
> 
> I would prefer to move this init code not into the board code but into the 
> platform/cpu code. Speaking for PPC4xx, here all configuration I know of have 
> the same init code. And copying this into all board ports instead on one 
> place in the cpu directory seems "imperfect".

But the ethernet configuration *is* actually bord dependent. A  board
may  or  may  not  use  all  the  ethernet  interfaces  present  on a
processor, or it may have additional network controllers attached for
example to the PCI bus, etc.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If it went on at this rate, in several billion  years  he'd  be  rich
beyond his wildest dreams!            - Terry Pratchett, _Soul Music_

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

On Saturday 22 March 2008, Wolfgang Denk wrote:
> In message <200803220731.23229.sr@denx.de> you wrote:
> > I would prefer to move this init code not into the board code but into
> > the platform/cpu code. Speaking for PPC4xx, here all configuration I know
> > of have the same init code. And copying this into all board ports instead
> > on one place in the cpu directory seems "imperfect".
>
> But the ethernet configuration *is* actually bord dependent. A  board
> may  or  may  not  use  all  the  ethernet  interfaces  present  on a
> processor, or it may have additional network controllers attached for
> example to the PCI bus, etc.

So you vote for adding this code to more than 80 4xx boards that all would use 
the same code here? I don't like this. I would prefer one instance in the cpu 
directory which could be overridden if needed by a board specific version.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Markus Klotzbücher]

Stefan Roese <sr@denx.de> writes:

> On Saturday 22 March 2008, Wolfgang Denk wrote:
>> In message <200803220731.23229.sr@denx.de> you wrote:
>> > I would prefer to move this init code not into the board code but into
>> > the platform/cpu code. Speaking for PPC4xx, here all configuration I know
>> > of have the same init code. And copying this into all board ports instead
>> > on one place in the cpu directory seems "imperfect".
>>
>> But the ethernet configuration *is* actually bord dependent. A  board
>> may  or  may  not  use  all  the  ethernet  interfaces  present  on a
>> processor, or it may have additional network controllers attached for
>> example to the PCI bus, etc.
>
> So you vote for adding this code to more than 80 4xx boards that all would use 
> the same code here? I don't like this. I would prefer one instance in the cpu 
> directory which could be overridden if needed by a board specific version.

Why not have both, a board and a CPU dependant init function like USB?

Best regards

Markus Klotzbuecher

--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Wolfgang Denk]

In message <200803221114.40956.sr@denx.de> you wrote:
>
> > In message <200803220731.23229.sr@denx.de> you wrote:
> > > I would prefer to move this init code not into the board code but into
> > > the platform/cpu code. Speaking for PPC4xx, here all configuration I know
> > > of have the same init code. And copying this into all board ports instead
> > > on one place in the cpu directory seems "imperfect".
> >
> > But the ethernet configuration *is* actually bord dependent. A  board
> > may  or  may  not  use  all  the  ethernet  interfaces  present  on a
> > processor, or it may have additional network controllers attached for
> > example to the PCI bus, etc.
> 
> So you vote for adding this code to more than 80 4xx boards that all would use 
> the same code here? I don't like this. I would prefer one instance in the cpu 
> directory which could be overridden if needed by a board specific version.

We agree that this should be implemented at the highest possible
level, i. e. with touching as little files as possible.

I just wanted to point out that this is  actually  a  board  specific
thing, while you said "all configuration I know of have the same init
code."

All I'm asking for is to make sure that this can be configured  in  a
board  specific  way.  If  there  is a zensible default setting which
covers most cases without need for board-specific stuff  that's  just
all the better.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Old programmers never die, they just become managers.

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Ben Warren]

On Sat, Mar 22, 2008 at 10:01 AM, Wolfgang Denk <wd@denx.de> wrote:
> In message <200803221114.40956.sr@denx.de> you wrote:
>  >
>  > > In message <200803220731.23229.sr@denx.de> you wrote:
>  > > > I would prefer to move this init code not into the board code but into
>  > > > the platform/cpu code. Speaking for PPC4xx, here all configuration I know
>  > > > of have the same init code. And copying this into all board ports instead
>  > > > on one place in the cpu directory seems "imperfect".
>  > >
>  > > But the ethernet configuration *is* actually bord dependent. A  board
>  > > may  or  may  not  use  all  the  ethernet  interfaces  present  on a
>  > > processor, or it may have additional network controllers attached for
>  > > example to the PCI bus, etc.
>  >
>  > So you vote for adding this code to more than 80 4xx boards that all would use
>  > the same code here? I don't like this. I would prefer one instance in the cpu
>  > directory which could be overridden if needed by a board specific version.
>
>  We agree that this should be implemented at the highest possible
>  level, i. e. with touching as little files as possible.
>
>  I just wanted to point out that this is  actually  a  board  specific
>  thing, while you said "all configuration I know of have the same init
>  code."
>
>  All I'm asking for is to make sure that this can be configured  in  a
>  board  specific  way.  If  there  is a zensible default setting which
>  covers most cases without need for board-specific stuff  that's  just
>  all the better.
>
How about something like this:

#ifdef CONFIG_ETH_INIT_DEFAULT
        CFG_ETH_INIT_DEFAULT(bis);
#else
        board_eth_init(bis);
#endif

and then in your 4xx header files put

#define CONFIG_ETH_INIT_DEFAULT
#define CFG_ETH_INIT_DEFAULT(x) ppc_4xx_eth_initialize(x)

OTOH, what I really want to do is gut the concept of drivers getting
configuration info from board header files.  For example, I'd much
rather have board code pass PHY information to the driver than have
the driver get it via #CONFIG ETH1_PHY_ADDR, if you know what I mean.
Obviously, getting to that point is a huge undertaking probably
touching hundreds of files.  I'm willing to do it as long as resources
exist to test on hardware that I don't have.

regards,
Ben

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

On Saturday 22 March 2008, Ben Warren wrote:

<snip>

> >  All I'm asking for is to make sure that this can be configured  in  a
> >  board  specific  way.  If  there  is a zensible default setting which
> >  covers most cases without need for board-specific stuff  that's  just
> >  all the better.
>
> How about something like this:
>
> #ifdef CONFIG_ETH_INIT_DEFAULT
>         CFG_ETH_INIT_DEFAULT(bis);
> #else
>         board_eth_init(bis);
> #endif

Using Markus's idea, why not use a cpu (platform) specific *and* a board 
specific init function, both with an empty weak alias in the common eth.c 
code:

	cpu_eth_init(bis);
	board_eth_init(bis);

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Ben Warren]

On Tue, Mar 25, 2008 at 3:04 AM, Stefan Roese <sr@denx.de> wrote:
> On Saturday 22 March 2008, Ben Warren wrote:
>
>  <snip>
>
>
>  > >  All I'm asking for is to make sure that this can be configured  in  a
>  > >  board  specific  way.  If  there  is a zensible default setting which
>  > >  covers most cases without need for board-specific stuff  that's  just
>  > >  all the better.
>  >
>  > How about something like this:
>  >
>  > #ifdef CONFIG_ETH_INIT_DEFAULT
>  >         CFG_ETH_INIT_DEFAULT(bis);
>  > #else
>  >         board_eth_init(bis);
>  > #endif
>
>  Using Markus's idea, why not use a cpu (platform) specific *and* a board
>  specific init function, both with an empty weak alias in the common eth.c
>  code:
>
>         cpu_eth_init(bis);
>         board_eth_init(bis);
>

Sorry I missed that.  Excellent idea.

regards,
Ben

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Ben Warren]

Stefan Roese wrote:
> On Saturday 22 March 2008, Ben Warren wrote:
>
> <snip>
>
>   
>>>  All I'm asking for is to make sure that this can be configured  in  a
>>>  board  specific  way.  If  there  is a zensible default setting which
>>>  covers most cases without need for board-specific stuff  that's  just
>>>  all the better.
>>>       
>> How about something like this:
>>
>> #ifdef CONFIG_ETH_INIT_DEFAULT
>>         CFG_ETH_INIT_DEFAULT(bis);
>> #else
>>         board_eth_init(bis);
>> #endif
>>     
>
> Using Markus's idea, why not use a cpu (platform) specific *and* a board 
> specific init function, both with an empty weak alias in the common eth.c 
> code:
>
> 	cpu_eth_init(bis);
> 	board_eth_init(bis);
>   

I thought about this some more, and the problem is that cpu_eth_init() and board_eth_init()
are mutually exclusive, with board_eth_init() having a higher priority.
I think the following will work, but would appreciate some feedback.

-----

int board_eth_init(bd_t *bis) __attribute(weak);
int cpu_eth_init(bd_t *bis) __attribute(weak);

.
.
.

if (board_eth_init)
	board_eth_init(bis);
else if (cpu_eth_init)
	cpu_eth_init(bis);

-----

This gets rid of the pointless aliases and gives precedence to the board-specific initialization.


regards,
Ben

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Joakim Tjernlund]

On Tue, 2008-03-25 at 10:22 -0400, Ben Warren wrote:
> Stefan Roese wrote:
> > On Saturday 22 March 2008, Ben Warren wrote:
> >
> > <snip>
> >
> >   
> >>>  All I'm asking for is to make sure that this can be configured  in  a
> >>>  board  specific  way.  If  there  is a zensible default setting which
> >>>  covers most cases without need for board-specific stuff  that's  just
> >>>  all the better.
> >>>       
> >> How about something like this:
> >>
> >> #ifdef CONFIG_ETH_INIT_DEFAULT
> >>         CFG_ETH_INIT_DEFAULT(bis);
> >> #else
> >>         board_eth_init(bis);
> >> #endif
> >>     
> >
> > Using Markus's idea, why not use a cpu (platform) specific *and* a board 
> > specific init function, both with an empty weak alias in the common eth.c 
> > code:
> >
> > 	cpu_eth_init(bis);
> > 	board_eth_init(bis);
> >   
> 
> I thought about this some more, and the problem is that cpu_eth_init() and board_eth_init()
> are mutually exclusive, with board_eth_init() having a higher priority.
> I think the following will work, but would appreciate some feedback.
> 
> -----
> 
> int board_eth_init(bd_t *bis) __attribute(weak);
> int cpu_eth_init(bd_t *bis) __attribute(weak);
> 
> .
> .
> .
> 
> if (board_eth_init)
> 	board_eth_init(bis);
> else if (cpu_eth_init)
> 	cpu_eth_init(bis);
> 
> -----
> 
> This gets rid of the pointless aliases and gives precedence to the board-specific initialization.
> 
> 
> regards,
> Ben

I think you must enable full relocation, CONFIG_RELOC_FIXUP_WORKS, to
make the "if (board_eth_init)" work. This is just a guess though.

  Jocke

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Ben Warren]

Joakim Tjernlund wrote:
> On Tue, 2008-03-25 at 10:22 -0400, Ben Warren wrote:
>   
>> Stefan Roese wrote:
>>     
>>> On Saturday 22 March 2008, Ben Warren wrote:
>>>
>>> <snip>
>>>
>>>   
>>>       
>>>>>  All I'm asking for is to make sure that this can be configured  in  a
>>>>>  board  specific  way.  If  there  is a zensible default setting which
>>>>>  covers most cases without need for board-specific stuff  that's  just
>>>>>  all the better.
>>>>>       
>>>>>           
>>>> How about something like this:
>>>>
>>>> #ifdef CONFIG_ETH_INIT_DEFAULT
>>>>         CFG_ETH_INIT_DEFAULT(bis);
>>>> #else
>>>>         board_eth_init(bis);
>>>> #endif
>>>>     
>>>>         
>>> Using Markus's idea, why not use a cpu (platform) specific *and* a board 
>>> specific init function, both with an empty weak alias in the common eth.c 
>>> code:
>>>
>>> 	cpu_eth_init(bis);
>>> 	board_eth_init(bis);
>>>   
>>>       
>> I thought about this some more, and the problem is that cpu_eth_init() and board_eth_init()
>> are mutually exclusive, with board_eth_init() having a higher priority.
>> I think the following will work, but would appreciate some feedback.
>>
>> -----
>>
>> int board_eth_init(bd_t *bis) __attribute(weak);
>> int cpu_eth_init(bd_t *bis) __attribute(weak);
>>
>> .
>> .
>> .
>>
>> if (board_eth_init)
>> 	board_eth_init(bis);
>> else if (cpu_eth_init)
>> 	cpu_eth_init(bis);
>>
>> -----
>>
>> This gets rid of the pointless aliases and gives precedence to the board-specific initialization.
>>
>>
>> regards,
>> Ben
>>     
>
> I think you must enable full relocation, CONFIG_RELOC_FIXUP_WORKS, to
> make the "if (board_eth_init)" work. This is just a guess though.
>
>   Jocke
>
>   
Nothing a little testing can't figure out.

thanks,
Ben

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Joakim Tjernlund]

On Tue, 2008-03-25 at 10:57 -0400, Ben Warren wrote:
> Joakim Tjernlund wrote:
> > On Tue, 2008-03-25 at 10:22 -0400, Ben Warren wrote:
> >   
> >> Stefan Roese wrote:
> >>     
> >>> On Saturday 22 March 2008, Ben Warren wrote:
> >>>
> >>> <snip>
> >>>
> >>>   
> >>>       
> >>>>>  All I'm asking for is to make sure that this can be configured  in  a
> >>>>>  board  specific  way.  If  there  is a zensible default setting which
> >>>>>  covers most cases without need for board-specific stuff  that's  just
> >>>>>  all the better.
> >>>>>       
> >>>>>           
> >>>> How about something like this:
> >>>>
> >>>> #ifdef CONFIG_ETH_INIT_DEFAULT
> >>>>         CFG_ETH_INIT_DEFAULT(bis);
> >>>> #else
> >>>>         board_eth_init(bis);
> >>>> #endif
> >>>>     
> >>>>         
> >>> Using Markus's idea, why not use a cpu (platform) specific *and* a board 
> >>> specific init function, both with an empty weak alias in the common eth.c 
> >>> code:
> >>>
> >>> 	cpu_eth_init(bis);
> >>> 	board_eth_init(bis);
> >>>   
> >>>       
> >> I thought about this some more, and the problem is that cpu_eth_init() and board_eth_init()
> >> are mutually exclusive, with board_eth_init() having a higher priority.
> >> I think the following will work, but would appreciate some feedback.
> >>
> >> -----
> >>
> >> int board_eth_init(bd_t *bis) __attribute(weak);
> >> int cpu_eth_init(bd_t *bis) __attribute(weak);
> >>
> >> .
> >> .
> >> .
> >>
> >> if (board_eth_init)
> >> 	board_eth_init(bis);
> >> else if (cpu_eth_init)
> >> 	cpu_eth_init(bis);
> >>
> >> -----
> >>
> >> This gets rid of the pointless aliases and gives precedence to the board-specific initialization.
> >>
> >>
> >> regards,
> >> Ben
> >>     
> >
> > I think you must enable full relocation, CONFIG_RELOC_FIXUP_WORKS, to
> > make the "if (board_eth_init)" work. This is just a guess though.
> >
> >   Jocke
> >
> >   
> Nothing a little testing can't figure out.
> 
> thanks,
> Ben

You could do too:
if (!board_eth_init(bis))
    cpu_eth_init(bis);

 Jocke

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Jerry Van Baren]

Joakim Tjernlund wrote:
> On Tue, 2008-03-25 at 10:57 -0400, Ben Warren wrote:
>> Joakim Tjernlund wrote:
>>> On Tue, 2008-03-25 at 10:22 -0400, Ben Warren wrote:
>>>   
>>>> Stefan Roese wrote:
>>>>     
>>>>> On Saturday 22 March 2008, Ben Warren wrote:

[snip]

>>>>> Using Markus's idea, why not use a cpu (platform) specific *and* a board 
>>>>> specific init function, both with an empty weak alias in the common eth.c 
>>>>> code:
>>>>>
>>>>> 	cpu_eth_init(bis);
>>>>> 	board_eth_init(bis);
>>>>>       
>>>> I thought about this some more, and the problem is that cpu_eth_init() and board_eth_init()
>>>> are mutually exclusive, with board_eth_init() having a higher priority.
>>>> I think the following will work, but would appreciate some feedback.
>>>>
>>>> -----
>>>>
>>>> int board_eth_init(bd_t *bis) __attribute(weak);
>>>> int cpu_eth_init(bd_t *bis) __attribute(weak);
>>>>
>>>> .
>>>> .
>>>> .
>>>>
>>>> if (board_eth_init)
>>>> 	board_eth_init(bis);
>>>> else if (cpu_eth_init)
>>>> 	cpu_eth_init(bis);
>>>>
>>>> -----
>>>>
>>>> This gets rid of the pointless aliases and gives precedence to the board-specific initialization.
>>>>
>>>>
>>>> regards,
>>>> Ben
>>>>     
>>> I think you must enable full relocation, CONFIG_RELOC_FIXUP_WORKS, to
>>> make the "if (board_eth_init)" work. This is just a guess though.
>>>
>>>   Jocke
>>>
>>>   
>> Nothing a little testing can't figure out.
>>
>> thanks,
>> Ben
> 
> You could do too:
> if (!board_eth_init(bis))
>     cpu_eth_init(bis);
> 
>  Jocke

Per an earlier discussion on how weak functions are implemented, these 
are not equivalent.  Functions marked "weak" *without* a weak 
implementation become NULL pointers.  The code
	if (board_eth_init)
		board_eth_init(bis);
	else if (cpu_eth_init)
		cpu_eth_init(bis);
uses that knowledge to see if the weak function board_eth_init() exists 
and then calls it if it does.  If it doesn't exist, it sees if 
cpu_eth_init() exists and calls it if it does.

Your counter proposal assumes that a weak function board_eth_init() 
*does* exist and uses the returned result as the condition of executing 
cpu_eth_init() (assuming it also exists).

If you define a weak function that simply returns failure, your 
alternative is close, but still not the same because an overridden 
(*real*) board_eth_init() could return failure too, in which case it 
will (probably erroneously) execute cpu_eth_init().  Beyond that, if 
cpu_eth_init() doesn't exist (doesn't have a default weak function 
defined), the call to it will go *SPLAT*.

HTH,
gvb

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Joakim Tjernlund]

On Tue, 2008-03-25 at 11:56 -0400, Jerry Van Baren wrote:
> Joakim Tjernlund wrote:
> > On Tue, 2008-03-25 at 10:57 -0400, Ben Warren wrote:
> >> Joakim Tjernlund wrote:
> >>> On Tue, 2008-03-25 at 10:22 -0400, Ben Warren wrote:
> >>>   
> >>>> Stefan Roese wrote:
> >>>>     
> >>>>> On Saturday 22 March 2008, Ben Warren wrote:
> 
> [snip]
> 
> >>>>> Using Markus's idea, why not use a cpu (platform) specific *and* a board 
> >>>>> specific init function, both with an empty weak alias in the common eth.c 
> >>>>> code:
> >>>>>
> >>>>> 	cpu_eth_init(bis);
> >>>>> 	board_eth_init(bis);
> >>>>>       
> >>>> I thought about this some more, and the problem is that cpu_eth_init() and board_eth_init()
> >>>> are mutually exclusive, with board_eth_init() having a higher priority.
> >>>> I think the following will work, but would appreciate some feedback.
> >>>>
> >>>> -----
> >>>>
> >>>> int board_eth_init(bd_t *bis) __attribute(weak);
> >>>> int cpu_eth_init(bd_t *bis) __attribute(weak);
> >>>>
> >>>> .
> >>>> .
> >>>> .
> >>>>
> >>>> if (board_eth_init)
> >>>> 	board_eth_init(bis);
> >>>> else if (cpu_eth_init)
> >>>> 	cpu_eth_init(bis);
> >>>>
> >>>> -----
> >>>>
> >>>> This gets rid of the pointless aliases and gives precedence to the board-specific initialization.
> >>>>
> >>>>
> >>>> regards,
> >>>> Ben
> >>>>     
> >>> I think you must enable full relocation, CONFIG_RELOC_FIXUP_WORKS, to
> >>> make the "if (board_eth_init)" work. This is just a guess though.
> >>>
> >>>   Jocke
> >>>
> >>>   
> >> Nothing a little testing can't figure out.
> >>
> >> thanks,
> >> Ben
> > 
> > You could do too:
> > if (!board_eth_init(bis))
> >     cpu_eth_init(bis);
> > 
> >  Jocke
> 
> Per an earlier discussion on how weak functions are implemented, these 
> are not equivalent.  Functions marked "weak" *without* a weak 
> implementation become NULL pointers.  The code
> 	if (board_eth_init)
> 		board_eth_init(bis);
> 	else if (cpu_eth_init)
> 		cpu_eth_init(bis);
> uses that knowledge to see if the weak function board_eth_init() exists 
> and then calls it if it does.  If it doesn't exist, it sees if 
> cpu_eth_init() exists and calls it if it does.
> 
> Your counter proposal assumes that a weak function board_eth_init() 
> *does* exist and uses the returned result as the condition of executing 
> cpu_eth_init() (assuming it also exists).
> 
> If you define a weak function that simply returns failure, your 
> alternative is close, but still not the same because an overridden 
> (*real*) board_eth_init() could return failure too, in which case it

Yes, see below for fix

>  
> will (probably erroneously) execute cpu_eth_init().  Beyond that, if 
> cpu_eth_init() doesn't exist (doesn't have a default weak function 
> defined), the call to it will go *SPLAT*.
> 
> HTH,
> gvb
> 

All true, I was thinking that there would be default weak dummy versions
of both board_eth_init() and cpu_eth_init() that would do the
right thing. To address the weakness you spotted one would need three
return values, something like:
 -1 fail
  0 not impl.
  1 success

 Jocke

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

On Tuesday 25 March 2008, Ben Warren wrote:
> I thought about this some more, and the problem is that cpu_eth_init() and
> board_eth_init() are mutually exclusive,

Right.

> with board_eth_init() having a 
> higher priority. I think the following will work, but would appreciate some
> feedback.
>
> -----
>
> int board_eth_init(bd_t *bis) __attribute(weak);
> int cpu_eth_init(bd_t *bis) __attribute(weak);
>
> .
> .
> .
>
> if (board_eth_init)
> 	board_eth_init(bis);
> else if (cpu_eth_init)
> 	cpu_eth_init(bis);
>
> -----
>
> This gets rid of the pointless aliases and gives precedence to the
> board-specific initialization.

I like it.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Andy Fleming]

On Tue, Mar 25, 2008 at 9:22 AM, Ben Warren <biggerbadderben@gmail.com> wrote:
> Stefan Roese wrote:
>  > On Saturday 22 March 2008, Ben Warren wrote:

>  > Using Markus's idea, why not use a cpu (platform) specific *and* a board
>  > specific init function, both with an empty weak alias in the common eth.c
>  > code:
>  >
>  >       cpu_eth_init(bis);
>  >       board_eth_init(bis);
>  >
>
>  I thought about this some more, and the problem is that cpu_eth_init() and board_eth_init()
>  are mutually exclusive, with board_eth_init() having a higher priority.
>  I think the following will work, but would appreciate some feedback.

I'm not sure that's necessarily the case.  Imagine, for instance, an
85xx board that (for some reason) has on-board ethernet.  I believe
some of the DS systems do this.  So the 85xx has 4 nics which the SOC
knows how to initialize.  But the board has an additional driver to
init.  Why not just allow them both?

Andy

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

On Tuesday 25 March 2008, Andy Fleming wrote:
> >  I thought about this some more, and the problem is that cpu_eth_init()
> > and board_eth_init() are mutually exclusive, with board_eth_init() having
> > a higher priority. I think the following will work, but would appreciate
> > some feedback.
>
> I'm not sure that's necessarily the case.  Imagine, for instance, an
> 85xx board that (for some reason) has on-board ethernet.  I believe
> some of the DS systems do this.  So the 85xx has 4 nics which the SOC
> knows how to initialize.  But the board has an additional driver to
> init.  Why not just allow them both?

Image a board that doesn't want all CPU (SoC) interfaces to get initialized. 
If for such a board a cpu-specific init routine exists, there is no chance to 
not enable (init) all those cpu interfaces as done in cpu_eth_init().

With this approach of mutually exclusive routines, it could define it's 
board_eth_init() and init only the Soc interfaces really needed. Plus 
additional ones of course.

Does this make sense?

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Andy Fleming]

On Tue, Mar 25, 2008 at 11:33 AM, Stefan Roese <sr@denx.de> wrote:
> On Tuesday 25 March 2008, Andy Fleming wrote:
>  > >  I thought about this some more, and the problem is that cpu_eth_init()
>  > > and board_eth_init() are mutually exclusive, with board_eth_init() having
>  > > a higher priority. I think the following will work, but would appreciate
>  > > some feedback.
>  >
>  > I'm not sure that's necessarily the case.  Imagine, for instance, an
>  > 85xx board that (for some reason) has on-board ethernet.  I believe
>  > some of the DS systems do this.  So the 85xx has 4 nics which the SOC
>  > knows how to initialize.  But the board has an additional driver to
>  > init.  Why not just allow them both?
>
>  Image a board that doesn't want all CPU (SoC) interfaces to get initialized.
>  If for such a board a cpu-specific init routine exists, there is no chance to
>  not enable (init) all those cpu interfaces as done in cpu_eth_init().
>
>  With this approach of mutually exclusive routines, it could define it's
>  board_eth_init() and init only the Soc interfaces really needed. Plus
>  additional ones of course.
>
>  Does this make sense?

Well, it makes sense to a degree.  However, we already have a
mechanism for enabling or disabling individual interfaces.  The config
file for each board can be used to determine which interfaces are
setup by the cpu_eth_init() function.  I don't really object to having
them mutually exclusive.  But I suspect the most common use case would
become:

board_eth_init()
{
     my_special_eth_init(bis);

     cpu_eth_init(bis);
}

And if everyone's going to do that, why bother making the functions
mutually exclusive?

Andy

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Ben Warren]

On 3/25/08, Andy Fleming <afleming@gmail.com> wrote:
> On Tue, Mar 25, 2008 at 11:33 AM, Stefan Roese <sr@denx.de> wrote:
> > On Tuesday 25 March 2008, Andy Fleming wrote:
> > > > I thought about this some more, and the problem is that
> cpu_eth_init()
> > > > and board_eth_init() are mutually exclusive, with board_eth_init()
> having
> > > > a higher priority. I think the following will work, but would
> appreciate
> > > > some feedback.
> > >
> > > I'm not sure that's necessarily the case. Imagine, for instance, an
> > > 85xx board that (for some reason) has on-board ethernet. I believe
> > > some of the DS systems do this. So the 85xx has 4 nics which the SOC
> > > knows how to initialize. But the board has an additional driver to
> > > init. Why not just allow them both?
> >
> > Image a board that doesn't want all CPU (SoC) interfaces to get
> initialized.
> > If for such a board a cpu-specific init routine exists, there is no
> chance to
> > not enable (init) all those cpu interfaces as done in cpu_eth_init().
> >
> > With this approach of mutually exclusive routines, it could define it's
> > board_eth_init() and init only the Soc interfaces really needed. Plus
> > additional ones of course.
> >
> > Does this make sense?
>
> Well, it makes sense to a degree. However, we already have a
> mechanism for enabling or disabling individual interfaces. The config
> file for each board can be used to determine which interfaces are
> setup by the cpu_eth_init() function. I don't really object to having
> them mutually exclusive. But I suspect the most common use case would
> become:
>
> board_eth_init()
> {
> my_special_eth_init(bis);
>
> cpu_eth_init(bis);
> }
>
> And if everyone's going to do that, why bother making the functions
> mutually exclusive?
>
> Andy
>

With the current incarnation of network code, that would be a common
use case.  However, I really don't like having interface-specific
#defines in the config file, since it doesn't jibe well with kconfig
and is just ugly.  You know what I'm talking about - #define
CONFIG_TSEC1_PHY_FLAGS etc. (I made this up, but I'm sure there are
definitions like it)  The main reason I went through this exercise is
to move towards a point where all but the most trivial Ethernet
controllers are initialized in board code, allowing us to pass PHY
info, feature flags etc. on a per-interface basis.

regards,
Ben

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Haavard Skinnemoen]

On Tue, 25 Mar 2008 08:04:13 +0100
Stefan Roese <sr@denx.de> wrote:

> Using Markus's idea, why not use a cpu (platform) specific *and* a board 
> specific init function, both with an empty weak alias in the common eth.c 
> code:
> 
> 	cpu_eth_init(bis);
> 	board_eth_init(bis);

Why?

The whole point about this exercise is to get rid of board-specific
knowledge in the common code. What is the purpose of cpu_eth_init()?

Haavard

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

On Wednesday 26 March 2008, Haavard Skinnemoen wrote:
> > Using Markus's idea, why not use a cpu (platform) specific *and* a board
> > specific init function, both with an empty weak alias in the common eth.c
> > code:
> >
> > 	cpu_eth_init(bis);
> > 	board_eth_init(bis);
>
> Why?
>
> The whole point about this exercise is to get rid of board-specific
> knowledge in the common code. What is the purpose of cpu_eth_init()?

Because on PPC4xx for example, all boards are using exactly the same eth_init 
code (for the SoC interfaces). And I don't like adding this code to more than 
80 boards.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Haavard Skinnemoen]

On Wed, 26 Mar 2008 11:14:34 +0100
Stefan Roese <sr@denx.de> wrote:

> > The whole point about this exercise is to get rid of board-specific
> > knowledge in the common code. What is the purpose of cpu_eth_init()?  
> 
> Because on PPC4xx for example, all boards are using exactly the same eth_init 
> code (for the SoC interfaces). And I don't like adding this code to more than 
> 80 boards.

All 80 boards have exactly the same ethernet interface (i.e. same kind
of PHY, same MII address, etc.)?

Can't you just add a weak definition of board_eth_init() in the CPU
code then?

Haavard

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

On Wednesday 26 March 2008, Haavard Skinnemoen wrote:
> > Because on PPC4xx for example, all boards are using exactly the same
> > eth_init code (for the SoC interfaces). And I don't like adding this code
> > to more than 80 boards.
>
> All 80 boards have exactly the same ethernet interface (i.e. same kind
> of PHY, same MII address, etc.)?

Yes, same SoC ethernet interfaces. Currently PHY address etc is configured via 
config options.

> Can't you just add a weak definition of board_eth_init() in the CPU
> code then?

No. The weak definition is already in net/eth.c.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Haavard Skinnemoen]

On Wed, 26 Mar 2008 11:34:08 +0100
Stefan Roese <sr@denx.de> wrote:

> On Wednesday 26 March 2008, Haavard Skinnemoen wrote:
> > > Because on PPC4xx for example, all boards are using exactly the same
> > > eth_init code (for the SoC interfaces). And I don't like adding this code
> > > to more than 80 boards.
> >
> > All 80 boards have exactly the same ethernet interface (i.e. same kind
> > of PHY, same MII address, etc.)?
> 
> Yes, same SoC ethernet interfaces. Currently PHY address etc is configured via 
> config options.

Judging by the number of #ifdefs in ppc_4xx_eth_initialize, that
function contains quite a bit of board-specific code.

> > Can't you just add a weak definition of board_eth_init() in the CPU
> > code then?
> 
> No. The weak definition is already in net/eth.c.

Make it non-weak then. If a platform wants to implement board-specific
ethernet init code as #ifdef hell, it should be free to do so. I don't
think we should add workarounds in the generic code for such platforms
though.

Haavard

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

On Wednesday 26 March 2008, Haavard Skinnemoen wrote:
> > Yes, same SoC ethernet interfaces. Currently PHY address etc is
> > configured via config options.
>
> Judging by the number of #ifdefs in ppc_4xx_eth_initialize, that
> function contains quite a bit of board-specific code.

Most of it is not board-specific, but platform-specific. This driver handles 
all 4xx EMAC variants and there are quite a lot. But basically you are 
correct.

> > > Can't you just add a weak definition of board_eth_init() in the CPU
> > > code then?
> >
> > No. The weak definition is already in net/eth.c.
>
> Make it non-weak then.

In net/eth.c? It can't get overwritten then.

> If a platform wants to implement board-specific 
> ethernet init code as #ifdef hell, it should be free to do so. I don't
> think we should add workarounds in the generic code for such platforms
> though.

I could use board_eth_init() as a common PPC4xx platform code. But I would 
loose the chance being able to implement a "real" board specific 
board_eth_init() for some boards in the future then.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Haavard Skinnemoen]

On Wed, 26 Mar 2008 12:43:50 +0100
Stefan Roese <sr@denx.de> wrote:

> On Wednesday 26 March 2008, Haavard Skinnemoen wrote:
> > > Yes, same SoC ethernet interfaces. Currently PHY address etc is
> > > configured via config options.
> >
> > Judging by the number of #ifdefs in ppc_4xx_eth_initialize, that
> > function contains quite a bit of board-specific code.
> 
> Most of it is not board-specific, but platform-specific. This driver handles 
> all 4xx EMAC variants and there are quite a lot. But basically you are 
> correct.

Yeah, but the platform-specific bits are normal for a platform-specific
driver. I just think that ideally, the board-specific bits should be
split out and placed in board_eth_init() for each board.

> > > > Can't you just add a weak definition of board_eth_init() in the CPU
> > > > code then?
> > >
> > > No. The weak definition is already in net/eth.c.
> >
> > Make it non-weak then.
> 
> In net/eth.c? It can't get overwritten then.

I mean the cpu-specific one. The generic "fallback" function should
stay weak.

> > If a platform wants to implement board-specific 
> > ethernet init code as #ifdef hell, it should be free to do so. I don't
> > think we should add workarounds in the generic code for such platforms
> > though.
> 
> I could use board_eth_init() as a common PPC4xx platform code. But I would 
> loose the chance being able to implement a "real" board specific 
> board_eth_init() for some boards in the future then.

Yeah...it would be bad to prevent future boards from doing the right
thing...

How about you do an internal split in cpu/ppc4xx/4xx_enet.c first,
placing the truly platform-specfic bits in a separate function that can
be called by "new-style" boards, and the board-specific bits in
board_eth_init(), serving as a wrapper around the platform-specific
init function, with as many #ifdefs as needed in order to handle the
existing differences between the boards. This function could be
conditional on CONFIG_PPC4XX_LEGACY_BOARD_ETH_INIT or something so that
boards can be migrated to the new API one by one.

U-Boot could really use a Janitor team ;-)

Haavard

[U-Boot-Users] [RFC][PATCH 1/1] Add board_eth_init() function

[Stefan Roese]

On Wednesday 26 March 2008, Haavard Skinnemoen wrote:
> > I could use board_eth_init() as a common PPC4xx platform code. But I
> > would loose the chance being able to implement a "real" board specific
> > board_eth_init() for some boards in the future then.
>
> Yeah...it would be bad to prevent future boards from doing the right
> thing...

:)

> How about you do an internal split in cpu/ppc4xx/4xx_enet.c first,
> placing the truly platform-specfic bits in a separate function that can
> be called by "new-style" boards, and the board-specific bits in
> board_eth_init(), serving as a wrapper around the platform-specific
> init function, with as many #ifdefs as needed in order to handle the
> existing differences between the boards. This function could be
> conditional on CONFIG_PPC4XX_LEGACY_BOARD_ETH_INIT or something so that
> boards can be migrated to the new API one by one.

Good idea, but unfortunately I have no time to do this in the next few 
days/weeks. So no basic rework of the 4xx emac init code in this merge 
window. At least not from me. Patches as always welcome. :)

> U-Boot could really use a Janitor team ;-)

;)

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [OT] Using MTD to manipulate CFI flash on PCI boards?

[David Hawkins]

Hi,

Sorry, for the slightly off-topic post, but I figured
someone else might have faced a similar issue.

I'm in the process of bringing up an MPC8349E PowerPC
based board. The board is configured as a compact PCI
peripheral board. The board will boot U-Boot off
on-board Spansion S29GL-N Flash, and U-Boot will then
boot Linux. The cPCI interface is used to communicate
with an x86 host CPU board.

The final system will have 15 boards per cPCI crate, and
there will be 8 crates. I can use a BDI2000 to program
the Flash on a board, but this doesn't scale so well
when wanting to perform Flash updates on crates full
of boards using the x86 host CPU.

Basically I'll have a setup where an x86 host can see
the Flash on the PowerPC boards via PCI memory regions.
In the case of a new board with a blank flash, the
PowerPC core will be held in reset, so without using
JTAG/COP, only the x86 host CPU can program the flash.

As far as flash manipulation from a PowerPC on a single
board goes, I figured that once I have U-Boot and Linux
working, I would just use the MTD subsystem.

However, its less clear to me whether I can use MTD
on the x86 Linux host CPU, to access the flash devices
on multiple boards via the PCI bus.

The documentation at:

http://www.linux-mtd.infradead.org/

wasn't much help, so I'll go and look at the code.

So while I go read some more, I figured I'd ask if
anyone else has used the MTD system in a similar way
for programming their flash.

Recommendations anyone?

Thanks!

Regards,
Dave.