[U-Boot] [PATCH] ne2000: Fix broken build of three boards after CONFIG_NET_MULTI drop
ne2000 wasn't converted to CONFIG_NET_MULTI when the non-multi support was dropped, so boards using it (qemu-mips, shmin, r7780mp) failed to compile for multiple definition of eth_rx() and friends due to old ne2000_base.c.
Changes:
In drivers/net/ne2000_base.c:
- Convert the driver to use a new probe function which calls eth_register(): - move probing / MAC addr code from eth_init to new ne2000_initialize() - cleanup the moved code to use eth_setenv_enetaddr() - Rename eth_init, eth_rx, eth_send and eth_halt to ne2k_* and use new calls
qemu-mips, shmin, r7780mp: - Call the new ne2000_initialize() from board_eth_init() of the boards.
Cleanups in drivers/net/ne2000_base.c:
- Use NE2000_RX_BUFFER_SIZE, defined to 2000 instead of a 2000 size constant. - dp83902a_init(): - Remove check for dp->base (nic.base): It is always assinged in probe func - Extend #ifdef NE2000_BASIC_INIT to whole function: The function does not have any useful code if NE2000_BASIC_INIT is not defined anymore. - Change printf of ESA MAC to debug PRINTK: Was always overridden(= not used) - In case somebody wants to use the ESA mac, add ifdef'ed memcpy to use it. - Fix gcc warnings in two debug printf calls
- Tested using qemu-mips board, - Tested the two renesas / sh boards r7780mp and shmin to compile again, and should work.
Signed-off-by: Bernhard Kaindl bernhard.kaindl@gmx.net --- board/qemu-mips/qemu-mips.c | 6 ++ board/renesas/r7780mp/r7780mp.c | 3 +- board/shmin/shmin.c | 5 ++ drivers/net/ne2000_base.c | 135 +++++++++++++++++++-------------------- include/netdev.h | 1 + 5 files changed, 81 insertions(+), 69 deletions(-)
diff --git a/board/qemu-mips/qemu-mips.c b/board/qemu-mips/qemu-mips.c index 7a69a00..e0372a6 100644 --- a/board/qemu-mips/qemu-mips.c +++ b/board/qemu-mips/qemu-mips.c @@ -87,3 +87,8 @@ int misc_init_r(void) set_io_port_base(0); return 0; } + +int board_eth_init(bd_t *bis) +{ + return ne2000_initialize(); +} diff --git a/board/renesas/r7780mp/r7780mp.c b/board/renesas/r7780mp/r7780mp.c index 0b80099..004c67e 100644 --- a/board/renesas/r7780mp/r7780mp.c +++ b/board/renesas/r7780mp/r7780mp.c @@ -81,5 +81,6 @@ void pci_init_board(void)
int board_eth_init(bd_t *bis) { - return pci_eth_init(bis); + /* return >= 0 if a chip is found, the board's AX88796L is n2k-based */ + return ne2000_initialize() + pci_eth_init(bis); } diff --git a/board/shmin/shmin.c b/board/shmin/shmin.c index 8742f10..9292af4 100644 --- a/board/shmin/shmin.c +++ b/board/shmin/shmin.c @@ -55,6 +55,11 @@ int dram_init(void) return 0; }
+int board_eth_init(bd_t *bis) +{ + return ne2000_initialize(); +} + void led_set_state(unsigned short value) {
diff --git a/drivers/net/ne2000_base.c b/drivers/net/ne2000_base.c index f93f932..eeda1c4 100644 --- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c @@ -85,6 +85,8 @@ void uboot_push_tx_done(int key, int val); /* NE2000 base header file */ #include "ne2000_base.h"
+#define NE2000_RX_BUFFER_SIZE 2000 + #if defined(CONFIG_DRIVER_AX88796L) /* AX88796L support */ #include "ax88796.h" @@ -96,23 +98,15 @@ void uboot_push_tx_done(int key, int val); static dp83902a_priv_data_t nic; /* just one instance of the card supported */
static bool -dp83902a_init(void) +dp83902a_init(struct eth_device *dev) { +#if defined(NE2000_BASIC_INIT) dp83902a_priv_data_t *dp = &nic; u8* base; -#if defined(NE2000_BASIC_INIT) int i; -#endif - - DEBUG_FUNCTION();
base = dp->base; - if (!base) - return false; /* No device found */ - - DEBUG_LINE();
-#if defined(NE2000_BASIC_INIT) /* AX88796L doesn't need */ /* Prepare ESA */ DP_OUT(base, DP_CR, DP_CR_NODMA | DP_CR_PAGE1); /* Select page 1 */ @@ -121,7 +115,7 @@ dp83902a_init(void) DP_IN(base, DP_P1_PAR0+i, dp->esa[i]); DP_OUT(base, DP_CR, DP_CR_NODMA | DP_CR_PAGE0); /* Select page 0 */
- printf("NE2000 - %s ESA: %02x:%02x:%02x:%02x:%02x:%02x\n", + PRINTK("NE2000 - %s ESA: %02x:%02x:%02x:%02x:%02x:%02x\n", "eeprom", dp->esa[0], dp->esa[1], @@ -129,6 +123,9 @@ dp83902a_init(void) dp->esa[3], dp->esa[4], dp->esa[5] ); +#ifdef NE2000_USE_MAC_FROM_SERIAL_EEPROM /* Define this to use it actually */ + memcpy(dev->enetaddr, dp->esa, 6); +#endif
#endif /* NE2000_BASIC_INIT */ return true; @@ -301,7 +298,7 @@ dp83902a_send(u8 *data, int total_len, u32 key)
/* Put data into buffer */ #if DEBUG & 4 - printf(" sg buf %08lx len %08x\n ", (u32)data, len); + printf(" sg buf %08x len %08x\n ", (u32)data, len); dx = 0; #endif while (len > 0) { @@ -469,7 +466,7 @@ dp83902a_recv(u8 *data, int len) if (data) { mlen = len; #if DEBUG & 4 - printf(" sg buf %08lx len %08x \n", (u32) data, mlen); + printf(" sg buf %08x len %08x\n", (u32) data, mlen); dx = 0; #endif while (0 < mlen) { @@ -651,7 +648,7 @@ static int initialized = 0;
void uboot_push_packet_len(int len) { PRINTK("pushed len = %d\n", len); - if (len >= 2000) { + if (len >= NE2000_RX_BUFFER_SIZE) { printf("NE2000: packet too big\n"); return; } @@ -666,76 +663,33 @@ void uboot_push_tx_done(int key, int val) { pkey = key; }
-int eth_init(bd_t *bd) { - int r; - u8 dev_addr[6]; - char ethaddr[20]; - - PRINTK("### eth_init\n"); - - if (!pbuf) { - pbuf = malloc(2000); - if (!pbuf) { - printf("Cannot allocate rx buffer\n"); - return -1; - } - } - -#ifdef CONFIG_DRIVER_NE2000_CCR - { - vu_char *p = (vu_char *) CONFIG_DRIVER_NE2000_CCR; - - PRINTK("CCR before is %x\n", *p); - *p = CONFIG_DRIVER_NE2000_VAL; - PRINTK("CCR after is %x\n", *p); - } -#endif - - nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE; - - r = get_prom(dev_addr, nic.base); - if (!r) - return -1; - - sprintf (ethaddr, "%02X:%02X:%02X:%02X:%02X:%02X", - dev_addr[0], dev_addr[1], - dev_addr[2], dev_addr[3], - dev_addr[4], dev_addr[5]) ; - PRINTK("Set environment from HW MAC addr = "%s"\n", ethaddr); - setenv ("ethaddr", ethaddr); - - nic.data = nic.base + DP_DATA; - nic.tx_buf1 = START_PG; - nic.tx_buf2 = START_PG2; - nic.rx_buf_start = RX_START; - nic.rx_buf_end = RX_END; - - if (dp83902a_init() == false) - return -1; - - dp83902a_start(dev_addr); +static int ne2k_init(struct eth_device *dev, bd_t *bd) +{ + dp83902a_start(dev->enetaddr); initialized = 1;
return 0; }
-void eth_halt() { - - PRINTK("### eth_halt\n"); +static void ne2k_halt(struct eth_device *dev) +{ + debug("### ne2k_halt\n"); if(initialized) dp83902a_stop(); initialized = 0; }
-int eth_rx() { +static int ne2k_recv(struct eth_device *dev) +{ dp83902a_poll(); return 1; }
-int eth_send(volatile void *packet, int length) { +static int ne2k_send(struct eth_device *dev, volatile void *packet, int length) +{ int tmo;
- PRINTK("### eth_send\n"); + debug("### ne2k_send\n");
pkey = -1;
@@ -755,3 +709,48 @@ int eth_send(volatile void *packet, int length) { } return 0; } + +int ne2000_initialize(void) +{ + struct eth_device *dev; + + nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE; + nic.data = nic.base + DP_DATA; + nic.tx_buf1 = START_PG; + nic.tx_buf2 = START_PG2; + nic.rx_buf_start = RX_START; + nic.rx_buf_end = RX_END; + + dev = calloc(sizeof(*dev), 1); + pbuf = malloc(NE2000_RX_BUFFER_SIZE); + + if (dev == NULL || pbuf == NULL) + return -1; + + if (!get_prom(dev->enetaddr, nic.base)) + return -1; + + dp83902a_init(dev); + + eth_setenv_enetaddr("ethaddr", dev->enetaddr); + + /* For PCMCIA support: See doc/README.ne2000 on how to enable */ +#ifdef CONFIG_DRIVER_NE2000_CCR + { + vu_char *p = (vu_char *) CONFIG_DRIVER_NE2000_CCR; + + PRINTK("CCR before is %x\n", *p); + *p = CONFIG_DRIVER_NE2000_VAL; + PRINTK("CCR after is %x\n", *p); + } +#endif + + dev->init = ne2k_init; + dev->halt = ne2k_halt; + dev->send = ne2k_send; + dev->recv = ne2k_recv; + + sprintf(dev->name, "NE2000"); + + return eth_register(dev); +} diff --git a/include/netdev.h b/include/netdev.h index 480453e..d13a5e2 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -79,6 +79,7 @@ int mpc8220_fec_initialize(bd_t *bis); int mpc82xx_scc_enet_initialize(bd_t *bis); int mvgbe_initialize(bd_t *bis); int natsemi_initialize(bd_t *bis); +int ne2000_initialize(); int npe_initialize(bd_t *bis); int ns8382x_initialize(bd_t *bis); int pcnet_initialize(bd_t *bis);
On Sunday 16 October 2011 14:12:57 Bernhard Kaindl wrote:
ne2000 wasn't converted to CONFIG_NET_MULTI when the non-multi support was dropped, so boards using it (qemu-mips, shmin, r7780mp) failed to compile for multiple definition of eth_rx() and friends due to old ne2000_base.c.
ah i wrote a patch for this but forgot to post it :/
- Tested using qemu-mips board,
- Tested the two renesas / sh boards r7780mp and shmin to compile again, and should work.
but i couldn't test it, so this is even better
--- a/board/qemu-mips/qemu-mips.c +++ b/board/qemu-mips/qemu-mips.c
+int board_eth_init(bd_t *bis) +{
- return ne2000_initialize();
+} --- a/board/shmin/shmin.c +++ b/board/shmin/shmin.c
+int board_eth_init(bd_t *bis) +{
- return ne2000_initialize();
+}
did you need to include netdev.h in this files for the new ne2000_initialize() prototype ?
--- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c
+int ne2000_initialize(void) +{
- struct eth_device *dev;
- nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE;
- nic.data = nic.base + DP_DATA;
- nic.tx_buf1 = START_PG;
- nic.tx_buf2 = START_PG2;
- nic.rx_buf_start = RX_START;
- nic.rx_buf_end = RX_END;
this should be using dev->priv rather than a global "nic" data structure
- dev = calloc(sizeof(*dev), 1);
- pbuf = malloc(NE2000_RX_BUFFER_SIZE);
- if (dev == NULL || pbuf == NULL)
return -1;
if dev worked but pbuf failed, this leaks memory
also, you should return 0 here not -1
- if (!get_prom(dev->enetaddr, nic.base))
return -1;- dp83902a_init(dev);
these should probably be in the eth->init step and not here
- eth_setenv_enetaddr("ethaddr", dev->enetaddr);
NAK: implement eth->write_hwaddr, and the driver should only use dev->enetaddr rather than touching the env
- /* For PCMCIA support: See doc/README.ne2000 on how to enable */
+#ifdef CONFIG_DRIVER_NE2000_CCR
- {
vu_char *p = (vu_char *) CONFIG_DRIVER_NE2000_CCR;PRINTK("CCR before is %x\n", *p);*p = CONFIG_DRIVER_NE2000_VAL;PRINTK("CCR after is %x\n", *p);- }
+#endif
i think this should be in ne2k_init
--- a/include/netdev.h +++ b/include/netdev.h
+int ne2000_initialize();
needs to be "(void)" -mike
Am 16.10.2011 21:39, schrieb Mike Frysinger:
On Sunday 16 October 2011 14:12:57 Bernhard Kaindl wrote:
ne2000 wasn't converted to CONFIG_NET_MULTI when the non-multi support was dropped, so boards using it (qemu-mips, shmin, r7780mp) failed to compile for multiple definition of eth_rx() and friends due to old ne2000_base.c.
ah i wrote a patch for this but forgot to post it :/
Argh, I feared you or somebody else might have done it too.
- Tested using qemu-mips board,
- Tested the two renesas / sh boards r7780mp and shmin to compile again, and should work.
but i couldn't test it, so this is even better
Be my guest (see doc/README.qemu_mips for everything you want to know): ln -s u-boot.bin mips_bios.bin; qemu-system-mips -M mips -L . -nographic qemu-mips # dhcp
--- a/board/qemu-mips/qemu-mips.c +++ b/board/qemu-mips/qemu-mips.c
+int board_eth_init(bd_t *bis) +{
- return ne2000_initialize();
+} --- a/board/shmin/shmin.c +++ b/board/shmin/shmin.c
+int board_eth_init(bd_t *bis) +{
- return ne2000_initialize();
+}
did you need to include netdev.h in this files for the new ne2000_initialize() prototype ?
Thanks, need to add an #include in shmin.c.
Lets really turn on -Werror to once for all put an end to such things. Lazy maintainers can aways put a CFLAGS += Wno-error into their makefiles if they can't fix warnings in a given directory. At the very least, -Werror-implicit-function-declaration should be used. Both flags are supported by gcc-2.95.3 or older gcc.gnu.org says.
--- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c
+int ne2000_initialize(void) +{
- struct eth_device *dev;
- nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE;
- nic.data = nic.base + DP_DATA;
- nic.tx_buf1 = START_PG;
- nic.tx_buf2 = START_PG2;
- nic.rx_buf_start = RX_START;
- nic.rx_buf_end = RX_END;
this should be using dev->priv rather than a global "nic" data structure
Understand, but the 2/3 boards using it only have a single ne2k-based chip, and as no one actually uses the driver multi-card, I skipped these larger changes to make it multi-card and use dev->priv everywhere.
- dev = calloc(sizeof(*dev), 1);
- pbuf = malloc(NE2000_RX_BUFFER_SIZE);
- if (dev == NULL || pbuf == NULL)
return -1;if dev worked but pbuf failed, this leaks memory
If malloc fails here, either CONFIG_SYS_MALLOC_LEN is to small or malloc is broken. Three Ethernet drivers call hang() in this case, which is probably best in explaining to the developer who broke it. I can replace that with "goto error" and give an error message on it like the other recently merged drivers do in this init situation.
also, you should return 0 here not -1
The recently merged drivers (armada100 - heavvy reviewed and you and many others, the xilinx_axi_emac(acked by you) and many more return -1 on malloc error.
- if (!get_prom(dev->enetaddr, nic.base))
return -1;- dp83902a_init(dev);
these should probably be in the eth->init step and not here
At first sight, I thought the same and did it the same but then you get
Net: NE2000 Warning: failed to set MAC address
if you don't call eth_setenv_enetaddr("ethaddr", dev->enetaddr) with the dev->enetaddr retrieved by get_prom(), so this has to be put here.
The error "Warning: failed to set MAC address" is in this case caused by ethaddr not being set in env. See the comment to you NAK below.
There are limits to what we can put into eth->init, and if the board or card has the original MAC address stored in HW (not U-Boot env) like the NE2000, the driver has to read the MAC from the HW and set ethaddr in env before registering the card, see below:
- eth_setenv_enetaddr("ethaddr", dev->enetaddr);
NAK: implement eth->write_hwaddr, and the driver should only use dev->enetaddr rather than touching the env
Wrong NAK: If ethaddr is not set in env, this "return -1" below triggers and no call to eth->write_hwaddr can be reached (below the return -1):
net/eth.c:
int eth_write_hwaddr(struct eth_device *dev, const char *base_name, int eth_number) { unsigned char env_enetaddr[6]; int ret = 0;
if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr)) return -1;
The NE2000 has the MAC in its PROM/EEPROM, which has to be used for it, and the code above requires it to be set in the env as well or the error shown in the previous comment occurs.
Read the call to eth_write_hwaddr() in net/eth.c and what it does.
- /* For PCMCIA support: See doc/README.ne2000 on how to enable */
+#ifdef CONFIG_DRIVER_NE2000_CCR
- {
vu_char *p = (vu_char *) CONFIG_DRIVER_NE2000_CCR;PRINTK("CCR before is %x\n", *p);*p = CONFIG_DRIVER_NE2000_VAL;PRINTK("CCR after is %x\n", *p);- }
+#endif
i think this should be in ne2k_init
It should not be ne2k_init but moved before calling get_prom() because it sets the PCMCIA card configuration register. No board still in mainline U-Boot uses an NE2000 in a PCMCIA slot, so this NE2000 in PCMCIA code could also be dropped in principle.
--- a/include/netdev.h +++ b/include/netdev.h
+int ne2000_initialize();
needs to be "(void)"
Ack.
-mike
Summary:
From this review, the needed changes to the patch to be done are:
- #include <netdev.h> in board/shmin/shmin.c to fix warning - add "void" as argument to function prototype in include/netdev.h - Move PCMCIA CCR init before get_prom() call or remove it (not used)
Bernhard
On Monday 17 October 2011 18:05:28 Bernhard Kaindl wrote:
Lets really turn on -Werror to once for all put an end to such things. Lazy maintainers can aways put a CFLAGS += Wno-error into their makefiles if they can't fix warnings in a given directory. At the very least, -Werror-implicit-function-declaration should be used. Both flags are supported by gcc-2.95.3 or older gcc.gnu.org says.
i don't think -Werror is feasible. there are way too many versions of gcc out there that we try to support from too many different vendors and too many architectures. warnings tend to come and go in between versions, and -Werror is just hell.
personally, i don't mind it to track local development of the latest tree, but it isn't something i'd inflict in general.
-Werror-implicit-function-declaration however sounds wonderful. and we don't have to worry about gcc versions as we have a cc-option helper: CFLAGS += $(call cc-option,-Werror-implicit-function-declaration)
care to send a patch ?
--- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c
+int ne2000_initialize(void) +{
- struct eth_device *dev;
- nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE;
- nic.data = nic.base + DP_DATA;
- nic.tx_buf1 = START_PG;
- nic.tx_buf2 = START_PG2;
- nic.rx_buf_start = RX_START;
- nic.rx_buf_end = RX_END;
this should be using dev->priv rather than a global "nic" data structure
Understand, but the 2/3 boards using it only have a single ne2k-based chip, and as no one actually uses the driver multi-card, I skipped these larger changes to make it multi-card and use dev->priv everywhere.
i'll let that slide since i broke the build ;). but i'd really like to see this fixed long term.
- dev = calloc(sizeof(*dev), 1);
- pbuf = malloc(NE2000_RX_BUFFER_SIZE);
- if (dev == NULL || pbuf == NULL)
return -1;if dev worked but pbuf failed, this leaks memory
If malloc fails here, either CONFIG_SYS_MALLOC_LEN is to small or malloc is broken. Three Ethernet drivers call hang() in this case, which is probably best in explaining to the developer who broke it. I can replace that with "goto error" and give an error message on it like the other recently merged drivers do in this init situation.
i know there's inconsistency here with some drivers, but the current "best practices" is to not leak and return 0.
also, you should return 0 here not -1
The recently merged drivers (armada100 - heavvy reviewed and you and many others, the xilinx_axi_emac(acked by you) and many more return -1 on malloc error.
i'm not perfect and sometimes i miss things :)
- if (!get_prom(dev->enetaddr, nic.base))
return -1;- dp83902a_init(dev);
these should probably be in the eth->init step and not here
At first sight, I thought the same and did it the same but then you get
Net: NE2000 Warning: failed to set MAC address
OK, i looked closer at the code, and it does seem to only look up the MAC address, so this is correct to do in the ne2000_initialize() function. when i saw the diffstat, i thought the init was refactored to do more, not less.
- eth_setenv_enetaddr("ethaddr", dev->enetaddr);
NAK: implement eth->write_hwaddr, and the driver should only use dev->enetaddr rather than touching the env
Wrong NAK: If ethaddr is not set in env, this "return -1" below triggers and no call to eth->write_hwaddr can be reached (below the return -1):
a bug in a diff layer doesn't mean you can add hacks to the driver. if the hardware supports changing the MAC at runtime by programming the registers, then implement write_hwaddr. if it doesn't, and you think the current eth behavior undesirable, then start a new thread. -mike
On Monday 17 October 2011 18:26:38 Mike Frysinger wrote:
On Monday 17 October 2011 18:05:28 Bernhard Kaindl wrote:
- dev = calloc(sizeof(*dev), 1);
- pbuf = malloc(NE2000_RX_BUFFER_SIZE);
- if (dev == NULL || pbuf == NULL)
return -1;if dev worked but pbuf failed, this leaks memory
If malloc fails here, either CONFIG_SYS_MALLOC_LEN is to small or malloc is broken. Three Ethernet drivers call hang() in this case, which is probably best in explaining to the developer who broke it. I can replace that with "goto error" and give an error message on it like the other recently merged drivers do in this init situation.
i know there's inconsistency here with some drivers, but the current "best practices" is to not leak and return 0.
meh, for now, keep the -1. but don't leak. i'll review the drivers and update the documentation. -mike
This fixes the build of the two sh boards shmin and r7780mp and qemu-mips which currently fail to build due to dropped pre-CONFIG_NET_MULTI code.
This v2 patch minimizes the number of lines in the diff for easy review and to eliminate any possible accidential changes resulting from moving lines of code in the file. This also makes the register function very easy.
Any cleanups and improvements are intentionally deferred to follow-up patches to keep this patch as simple and as easy to review as possible.
A new driver register function, ne2k_register() calls the existing one-time setup part of the old init function and calls eth_register().
Changes to shmin, r7780mp and qemu-mips: - Call the new ne2k_register() from board_eth_init() of the boards.
- Tested using qemu-mips board, - Tested the two renesas / sh boards r7780mp and shmin to compile again, and should work.
checkpatch-clean when "--ignore VOLATILE" is added to .checkpatch.conf, and no warnings introduced in none of the three boards using this driver.
Signed-off-by: Bernhard Kaindl bernhard.kaindl@gmx.net --- board/qemu-mips/qemu-mips.c | 6 ++ board/renesas/r7780mp/r7780mp.c | 3 +- board/shmin/shmin.c | 6 ++ drivers/net/ne2000_base.c | 99 ++++++++++++++++++++++++++++----------- include/netdev.h | 1 + 5 files changed, 86 insertions(+), 29 deletions(-)
diff --git a/board/qemu-mips/qemu-mips.c b/board/qemu-mips/qemu-mips.c index 7a69a00..2b3a1d6 100644 --- a/board/qemu-mips/qemu-mips.c +++ b/board/qemu-mips/qemu-mips.c @@ -25,6 +25,7 @@ #include <command.h> #include <asm/mipsregs.h> #include <asm/io.h> +#include <netdev.h>
phys_size_t initdram(int board_type) { @@ -87,3 +88,8 @@ int misc_init_r(void) set_io_port_base(0); return 0; } + +int board_eth_init(bd_t *bis) +{ + return ne2k_register(); +} diff --git a/board/renesas/r7780mp/r7780mp.c b/board/renesas/r7780mp/r7780mp.c index 0b80099..82cef02 100644 --- a/board/renesas/r7780mp/r7780mp.c +++ b/board/renesas/r7780mp/r7780mp.c @@ -81,5 +81,6 @@ void pci_init_board(void)
int board_eth_init(bd_t *bis) { - return pci_eth_init(bis); + /* return >= 0 if a chip is found, the board's AX88796L is n2k-based */ + return ne2k_register() + pci_eth_init(bis); } diff --git a/board/shmin/shmin.c b/board/shmin/shmin.c index 8742f10..7348f52 100644 --- a/board/shmin/shmin.c +++ b/board/shmin/shmin.c @@ -28,6 +28,7 @@ #include <common.h> #include <asm/io.h> #include <asm/processor.h> +#include <netdev.h>
int checkboard(void) { @@ -55,6 +56,11 @@ int dram_init(void) return 0; }
+int board_eth_init(bd_t *bis) +{ + return ne2k_register(); +} + void led_set_state(unsigned short value) {
diff --git a/drivers/net/ne2000_base.c b/drivers/net/ne2000_base.c index f93f932..ad8437d 100644 --- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c @@ -95,8 +95,12 @@ void uboot_push_tx_done(int key, int val);
static dp83902a_priv_data_t nic; /* just one instance of the card supported */
+/** + * This function reads the MAC address from the serial EEPROM, + * used if PROM read fails. Does nothing for ax88796 chips (sh boards) + */ static bool -dp83902a_init(void) +dp83902a_init(unsigned char *enetaddr) { dp83902a_priv_data_t *dp = &nic; u8* base; @@ -130,6 +134,7 @@ dp83902a_init(void) dp->esa[4], dp->esa[5] );
+ memcpy(enetaddr, dp->esa, 6); /* Use MAC from serial EEPROM */ #endif /* NE2000_BASIC_INIT */ return true; } @@ -162,6 +167,8 @@ dp83902a_start(u8 * enaddr) u8 *base = dp->base; int i;
+ debug("The MAC is %pM\n", enaddr); + DEBUG_FUNCTION();
DP_OUT(base, DP_CR, DP_CR_PAGE0 | DP_CR_NODMA | DP_CR_STOP); /* Brutal */ @@ -666,12 +673,16 @@ void uboot_push_tx_done(int key, int val) { pkey = key; }
-int eth_init(bd_t *bd) { - int r; - u8 dev_addr[6]; - char ethaddr[20]; - - PRINTK("### eth_init\n"); +/** + * Setup the driver and init MAC address according to doc/README.enetaddr + * Called by ne2k_register() before registering the driver @eth layer + * + * @param struct ethdevice of this instance of the driver for dev->enetaddr + * @return 0 on success, -1 on error (causing caller to print error msg) + */ +static int ne2k_setup_driver(struct eth_device *dev) +{ + PRINTK("### ne2k_setup_driver\n");
if (!pbuf) { pbuf = malloc(2000); @@ -693,49 +704,56 @@ int eth_init(bd_t *bd) {
nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE;
- r = get_prom(dev_addr, nic.base); - if (!r) - return -1; - - sprintf (ethaddr, "%02X:%02X:%02X:%02X:%02X:%02X", - dev_addr[0], dev_addr[1], - dev_addr[2], dev_addr[3], - dev_addr[4], dev_addr[5]) ; - PRINTK("Set environment from HW MAC addr = "%s"\n", ethaddr); - setenv ("ethaddr", ethaddr); - nic.data = nic.base + DP_DATA; nic.tx_buf1 = START_PG; nic.tx_buf2 = START_PG2; nic.rx_buf_start = RX_START; nic.rx_buf_end = RX_END;
- if (dp83902a_init() == false) - return -1; + /* + * According to doc/README.enetaddr, drivers shall give priority + * to the MAC address value in the environment, so we do not read + * it from the prom or eeprom if it is specified in the environment. + */ + if (!eth_getenv_enetaddr("ethaddr", dev->enetaddr)) { + /* If the MAC address is not in the environment, get it: */ + if (!get_prom(dev->enetaddr, nic.base)) /* get MAC from prom */ + dp83902a_init(dev->enetaddr); /* fallback: seeprom */ + /* And write it into the environment otherwise eth_write_hwaddr + * returns -1 due to eth_getenv_enetaddr_by_index() failing, + * and this causes "Warning: failed to set MAC address", and + * cmd_bdinfo has no ethaddr value which it can show: */ + eth_setenv_enetaddr("ethaddr", dev->enetaddr); + } + return 0; +}
- dp83902a_start(dev_addr); +static int ne2k_init(struct eth_device *dev, bd_t *bd) +{ + dp83902a_start(dev->enetaddr); initialized = 1; - return 0; }
-void eth_halt() { - - PRINTK("### eth_halt\n"); +static void ne2k_halt(struct eth_device *dev) +{ + debug("### ne2k_halt\n"); if(initialized) dp83902a_stop(); initialized = 0; }
-int eth_rx() { +static int ne2k_recv(struct eth_device *dev) +{ dp83902a_poll(); return 1; }
-int eth_send(volatile void *packet, int length) { +static int ne2k_send(struct eth_device *dev, volatile void *packet, int length) +{ int tmo;
- PRINTK("### eth_send\n"); + debug("### ne2k_send\n");
pkey = -1;
@@ -755,3 +773,28 @@ int eth_send(volatile void *packet, int length) { } return 0; } + +/** + * Setup the driver for use and register it with the eth layer + * @return 0 on success, -1 on error (causing caller to print error msg) + */ +int ne2k_register(void) +{ + struct eth_device *dev; + + dev = calloc(sizeof(*dev), 1); + if (dev == NULL) + return -1; + + if (ne2k_setup_driver(dev)) + return -1; + + dev->init = ne2k_init; + dev->halt = ne2k_halt; + dev->send = ne2k_send; + dev->recv = ne2k_recv; + + sprintf(dev->name, "NE2000"); + + return eth_register(dev); +} diff --git a/include/netdev.h b/include/netdev.h index 669f60b..38308e4 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -79,6 +79,7 @@ int mpc8220_fec_initialize(bd_t *bis); int mpc82xx_scc_enet_initialize(bd_t *bis); int mvgbe_initialize(bd_t *bis); int natsemi_initialize(bd_t *bis); +int ne2k_register(void); int npe_initialize(bd_t *bis); int ns8382x_initialize(bd_t *bis); int pcnet_initialize(bd_t *bis);
This fixes the build of the two sh boards shmin and r7780mp and qemu-mips which currently fail to build due to dropped pre-CONFIG_NET_MULTI code.
This v2 patch minimizes the number of lines in the diff for easy review and to eliminate any possible accidential changes resulting from moving lines of code in the file. This also makes the register function very easy.
Any cleanups and improvements are intentionally deferred to follow-up patches to keep this patch as simple and as easy to review as possible.
A new driver register function, ne2k_register() calls the existing one-time setup part of the old init function and calls eth_register().
Changes to shmin, r7780mp and qemu-mips:
Call the new ne2k_register() from board_eth_init() of the boards.
Tested using qemu-mips board,
Tested the two renesas / sh boards r7780mp and shmin to compile again, and should work.
checkpatch-clean when "--ignore VOLATILE" is added to .checkpatch.conf, and no warnings introduced in none of the three boards using this driver.
Signed-off-by: Bernhard Kaindl bernhard.kaindl@gmx.net
board/qemu-mips/qemu-mips.c | 6 ++ board/renesas/r7780mp/r7780mp.c | 3 +- board/shmin/shmin.c | 6 ++ drivers/net/ne2000_base.c | 99 ++++++++++++++++++++++++++++----------- include/netdev.h | 1 + 5 files changed, 86 insertions(+), 29 deletions(-)
I think this patch is absolutelly essential to have in .12 release. I'll test it now and ev. rebase and resubmit. Bernhard, you're ok with me doing minor fixes?
M
Dear Bernhard Kaindl,
In message 1319144219-15731-1-git-send-email-bernhard.kaindl@gmx.net you wrote:
This fixes the build of the two sh boards shmin and r7780mp and qemu-mips which currently fail to build due to dropped pre-CONFIG_NET_MULTI code.
This v2 patch minimizes the number of lines in the diff for easy review and to eliminate any possible accidential changes resulting from moving lines of code in the file. This also makes the register function very easy.
Any cleanups and improvements are intentionally deferred to follow-up patches to keep this patch as simple and as easy to review as possible.
A new driver register function, ne2k_register() calls the existing one-time setup part of the old init function and calls eth_register().
Changes to shmin, r7780mp and qemu-mips:
Call the new ne2k_register() from board_eth_init() of the boards.
Tested using qemu-mips board,
Tested the two renesas / sh boards r7780mp and shmin to compile again, and should work.
checkpatch-clean when "--ignore VOLATILE" is added to .checkpatch.conf, and no warnings introduced in none of the three boards using this driver.
Signed-off-by: Bernhard Kaindl bernhard.kaindl@gmx.net
board/qemu-mips/qemu-mips.c | 6 ++ board/renesas/r7780mp/r7780mp.c | 3 +- board/shmin/shmin.c | 6 ++ drivers/net/ne2000_base.c | 99 ++++++++++++++++++++++++++++----------- include/netdev.h | 1 + 5 files changed, 86 insertions(+), 29 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
On Thursday 20 October 2011 16:56:59 Bernhard Kaindl wrote:
--- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c
- /*
* According to doc/README.enetaddr, drivers shall give priority* to the MAC address value in the environment, so we do not read* it from the prom or eeprom if it is specified in the environment.*/- if (!eth_getenv_enetaddr("ethaddr", dev->enetaddr)) {
/* If the MAC address is not in the environment, get it: */if (!get_prom(dev->enetaddr, nic.base)) /* get MAC from prom */dp83902a_init(dev->enetaddr); /* fallback: seeprom *//* And write it into the environment otherwise eth_write_hwaddr* returns -1 due to eth_getenv_enetaddr_by_index() failing,* and this causes "Warning: failed to set MAC address", and* cmd_bdinfo has no ethaddr value which it can show: */eth_setenv_enetaddr("ethaddr", dev->enetaddr);- }
this env parsing doesn't belong here. the net drivers only read dev-
enetaddr. can you post a patch to drop this ?
-mike
participants (4)
-
Bernhard Kaindl -
Marek Vasut -
Mike Frysinger -
Wolfgang Denk