[U-Boot] [PATCH] ne2000: take MAC address from config if available
Hi,
the board I'm currently working on has an ASIX AX88796 NE2000 clone but no EEPROM attached to it. Hence, the get_prom() routine returns zeros only so the system won't work.
This patch takes the MAC address given by CONFIG_ETHADDR and translates it to numeric values. This could probably go to some other, more generic place, but I didn't find any.
Best regards, Daniel
diff --git a/drivers/net/ne2000_base.c b/drivers/net/ne2000_base.c index f93f932..f8480a3 100644 --- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c @@ -693,6 +693,7 @@ int eth_init(bd_t *bd) {
nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE;
+#ifndef CONFIG_ETHADDR r = get_prom(dev_addr, nic.base); if (!r) return -1; @@ -703,6 +704,20 @@ int eth_init(bd_t *bd) { dev_addr[4], dev_addr[5]) ; PRINTK("Set environment from HW MAC addr = "%s"\n", ethaddr); setenv ("ethaddr", ethaddr); +#else /* CONFIG_ETHADDR */ +#define STR(X) #X +#define XSTR(X) STR(X) + strncpy(ethaddr, XSTR(CONFIG_ETHADDR), sizeof(ethaddr)); + + /* replace all colons by NULL characters */ + for (r = 0; r < strlen(ethaddr); r++) + if (ethaddr[r] == ':') + ethaddr[r] = '\0'; + + /* convert the string notation */ + for (r = 0; r < 6; r++) + dev_addr[r] = simple_strtol(ethaddr + (r * 3), NULL, 16); +#endif
nic.data = nic.base + DP_DATA; nic.tx_buf1 = START_PG;
Hi Daniel,
On Fri, Nov 28, 2008 at 8:16 AM, Daniel Mack daniel@caiaq.de wrote:
Hi,
the board I'm currently working on has an ASIX AX88796 NE2000 clone but no EEPROM attached to it. Hence, the get_prom() routine returns zeros only so the system won't work.
This patch takes the MAC address given by CONFIG_ETHADDR and translates it to numeric values. This could probably go to some other, more generic place, but I didn't find any.
CONFIG_ETHADDR is bad and shouldn't be used unless you only ever plan on building one board. No new board ports will be accepted that define it. What would be better here, although it's still not very good, would be to have something like CONFIG_NE2000_NOPROM, and if defined, make a 'getenv("ethaddr")' call and program the hardware with the return value (with proper error checking, of course).
regards, Ben
Hi Ben,
On Fri, Nov 28, 2008 at 09:27:37PM -0800, Ben Warren wrote:
the board I'm currently working on has an ASIX AX88796 NE2000 clone but no EEPROM attached to it. Hence, the get_prom() routine returns zeros only so the system won't work.
This patch takes the MAC address given by CONFIG_ETHADDR and translates it to numeric values. This could probably go to some other, more generic place, but I didn't find any.
CONFIG_ETHADDR is bad and shouldn't be used unless you only ever plan on building one board. No new board ports will be accepted that define it.
I agree it's a hack, but in order to boot the board from ethernet, there is need for something, even if it's not perfect.
What would be better here, although it's still not very good, would be to have something like CONFIG_NE2000_NOPROM, and if defined, make a 'getenv("ethaddr")' call and program the hardware with the return value (with proper error checking, of course).
I agree. See the patch below.
Thanks, Daniel
This patch adds CONFIG_NE2000_NOPROM as configuration variable and reads the 'ethaddr' setting from environment to set up the adapter.
Signed-off-by: Daniel Mack daniel@caiaq.de
diff --git a/drivers/net/ne2000_base.c b/drivers/net/ne2000_base.c index f93f932..86c1380 100644 --- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c @@ -669,7 +669,6 @@ void uboot_push_tx_done(int key, int val) { int eth_init(bd_t *bd) { int r; u8 dev_addr[6]; - char ethaddr[20];
PRINTK("### eth_init\n");
@@ -693,16 +692,35 @@ int eth_init(bd_t *bd) {
nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE;
- r = get_prom(dev_addr, nic.base); - if (!r) - return -1; +#ifndef CONFIG_NE2000_NOPROM + { + char ethaddr[20]; + 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); + } +#else /* CONFIG_NE2000_NOPROM */ + { + char *s = getenv("ethaddr");
- 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); + if (!s) { + printf ("CONFIG_NE2000_NOPROM set but no " + "ethaddr given in environment.\n"); + return -1; + } + + /* convert the string notation */ + for (r = 0; r < 6; r++) + dev_addr[r] = simple_strtol(s + (r * 3), NULL, 16); + } +#endif
nic.data = nic.base + DP_DATA; nic.tx_buf1 = START_PG;
This adds CONFIG_NE2000_NOPROM. If set, the ethernet MAC address is taken from the environment variable 'ethaddr' and the NIC is configured accordingly. Needed for boards that don't have an EEPROM to store this setting permanently.
Signed-off-by: Daniel Mack daniel@caiaq.de
--- ne2000_base.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ne2000_base.c b/drivers/net/ne2000_base.c index f93f932..86c1380 100644 --- a/drivers/net/ne2000_base.c +++ b/drivers/net/ne2000_base.c @@ -669,7 +669,6 @@ void uboot_push_tx_done(int key, int val) { int eth_init(bd_t *bd) { int r; u8 dev_addr[6]; - char ethaddr[20];
PRINTK("### eth_init\n");
@@ -693,16 +692,35 @@ int eth_init(bd_t *bd) {
nic.base = (u8 *) CONFIG_DRIVER_NE2000_BASE;
- r = get_prom(dev_addr, nic.base); - if (!r) - return -1; +#ifndef CONFIG_NE2000_NOPROM + { + char ethaddr[20]; + 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); + } +#else /* CONFIG_NE2000_NOPROM */ + { + char *s = getenv("ethaddr");
- 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); + if (!s) { + printf ("CONFIG_NE2000_NOPROM set but no " + "ethaddr given in environment.\n"); + return -1; + } + + /* convert the string notation */ + for (r = 0; r < 6; r++) + dev_addr[r] = simple_strtol(s + (r * 3), NULL, 16); + } +#endif
nic.data = nic.base + DP_DATA; nic.tx_buf1 = START_PG;
Dear Daniel Mack,
In message 20081203010154.GE2012@buzzloop.caiaq.de you wrote:
This adds CONFIG_NE2000_NOPROM. If set, the ethernet MAC address is taken from the environment variable 'ethaddr' and the NIC is configured accordingly. Needed for boards that don't have an EEPROM to store this setting permanently.
Signed-off-by: Daniel Mack daniel@caiaq.de
ne2000_base.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-)
Why do we need a special CONFIG_ option? is it not possible to detect (by software) that there is no EEPROM available, and auto-adjust?
Note that there is existing policy how to handle the situation that we have both a MAC address stored in some other storeage (like EEPROM on the NIC) and in the "ethaddr" environment variable (see for example drivers/net/cs8900.c). In the end, the code shall always only rely on the U-Boot environment settings.
The system will test if MAC addresses are set in EEPROM and in the envrionment; then it should behave as follows:
* If there is no MAC address in the EEPROM, then do nothing, i. e. rely on any previously existing envrionment settings.
* If there is a MAC address in the EEPROM, and ther eis no corresponding "ethaddr" environment variable defined, then initialize (setenv()) this variable with the value read from the EEPROM.
* If there is a MAC address in the EEPROM, and there is a value in the "ethaddr" environment variable, and both are identical, we use that value and everything is fine.
* If there is a MAC address in the EEPROM, and there is a value in the "ethaddr" environment variable, and both are different, we issue a warning "Warning: MAC addresses don't match: ... HW MAC address: ... "ethaddr" value: ..." and then use the value from the ENVIRONMENT.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Sun, Dec 14, 2008 at 12:12:24PM +0100, Wolfgang Denk wrote:
This adds CONFIG_NE2000_NOPROM. If set, the ethernet MAC address is taken from the environment variable 'ethaddr' and the NIC is configured accordingly. Needed for boards that don't have an EEPROM to store this setting permanently.
Signed-off-by: Daniel Mack daniel@caiaq.de
ne2000_base.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-)
Why do we need a special CONFIG_ option? is it not possible to detect (by software) that there is no EEPROM available, and auto-adjust?
Hmm, what's bad about a special config variable? I agree that in general, auto-probing is a good thing, but in U-Boot with its per-board configuration file concept, I don't see much advantage. It is very unlikely that a certain board is produced with and without an EEPROM.
Note that there is existing policy how to handle the situation that we have both a MAC address stored in some other storeage (like EEPROM on the NIC) and in the "ethaddr" environment variable (see for example drivers/net/cs8900.c). In the end, the code shall always only rely on the U-Boot environment settings.
I can send a new version of this patch following this, but I wonder why such a logic has to be implemented in the ethernet drivers and did not find a well-defined place in some kind of layer all drivers make use of? Wouldn't it make more sense to put some effort in this direction?
Best regards, Daniel
Picking up an acient thread,
On Sun, Dec 14, 2008 at 12:39:10PM +0100, Daniel Mack wrote:
On Sun, Dec 14, 2008 at 12:12:24PM +0100, Wolfgang Denk wrote:
This adds CONFIG_NE2000_NOPROM. If set, the ethernet MAC address is taken from the environment variable 'ethaddr' and the NIC is configured accordingly. Needed for boards that don't have an EEPROM to store this setting permanently.
Signed-off-by: Daniel Mack daniel@caiaq.de
ne2000_base.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-)
Why do we need a special CONFIG_ option? is it not possible to detect (by software) that there is no EEPROM available, and auto-adjust?
Hmm, what's bad about a special config variable? I agree that in general, auto-probing is a good thing, but in U-Boot with its per-board configuration file concept, I don't see much advantage. It is very unlikely that a certain board is produced with and without an EEPROM.
Note that there is existing policy how to handle the situation that we have both a MAC address stored in some other storeage (like EEPROM on the NIC) and in the "ethaddr" environment variable (see for example drivers/net/cs8900.c). In the end, the code shall always only rely on the U-Boot environment settings.
I can send a new version of this patch following this, but I wonder why such a logic has to be implemented in the ethernet drivers and did not find a well-defined place in some kind of layer all drivers make use of? Wouldn't it make more sense to put some effort in this direction?
Is there any reason why there was no reply to this anymore suddenly? Did it sound like too much work for anyone to do it the right way? I'll get back to this project soon and will need to address the same problems agains like many weeks ago, and unfortunately, nothing happened to all the issues I pointed out back then, not a single patch I sent in has been taken.
Daniel
Dear Daniel,
In message 20090127130912.GA25722@buzzloop.caiaq.de you wrote:
I can send a new version of this patch following this, but I wonder why such a logic has to be implemented in the ethernet drivers and did not find a well-defined place in some kind of layer all drivers make use of? Wouldn't it make more sense to put some effort in this direction?
Is there any reason why there was no reply to this anymore suddenly? Did it sound like too much work for anyone to do it the right way? I'll get
Well, let me answer with a quote from my signature list:
There are three ways to get something done: do it yourself, hire someone, or forbid your kids to do it.
back to this project soon and will need to address the same problems agains like many weeks ago, and unfortunately, nothing happened to all the issues I pointed out back then, not a single patch I sent in has been taken.
You know the reasons why they were rejected. As for the "nothing happened" part - see signature below :-)
Best regards,
Wolfgang Denk
participants (3)
-
Ben Warren -
Daniel Mack -
Wolfgang Denk