Skip to contentSkip to conversation

ne2000: take MAC address from config if available

PATCH New Daniel Mack 3 participants 8 messages Nov 28, 2008 – Jan 27, 2009 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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

drivers/net/ne2000_base.c +15 -0
index f93f932..f8480a3 100644
@@ -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;
#1 lore raw

PATCH 1/1 ne2000: take MAC address from config if available

Hi Daniel,

Quoted 10 lines · Daniel Mack

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

#2 lore raw

PATCH 1/1 ne2000: take MAC address from config if available

Hi Ben,

Quoted 11 lines · Ben Warren
Quoted 8 lines

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.

Quoted 4 lines

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.

drivers/net/ne2000_base.c +28 -10
index f93f932..86c1380 100644
@@ -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;
#3 lore raw

PATCH 1/1 ne2000: take MAC address from config if available

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.

---
 ne2000_base.c |   38 ++++++++++++++++++++++++++++----------
  1 file changed, 28 insertions(+), 10 deletions(-)
drivers/net/ne2000_base.c +28 -10
index f93f932..86c1380 100644
@@ -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;
#4 lore raw

PATCH 1/1 ne2000: take MAC address from config if available

Dear Daniel Mack,

Quoted 10 lines · In message <[email protected]> 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.

---
 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

Signature · 5 lines
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
"There was no difference between  the  behavior  of  a  god  and  the
operations of pure chance..."   - Thomas Pynchon, _Gravity's Rainbow_
#5 lore raw

PATCH 1/1 ne2000: take MAC address from config if available

Hi Wolfgang,

Quoted 5 lines · Wolfgang Denk
Quoted 5 lines

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.

Quoted 8 lines
Quoted 5 lines
---
 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.

Quoted 5 lines

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

#6 lore raw

PATCH 1/1 ne2000: take MAC address from config if available

Picking up an acient thread,

Quoted 26 lines · Daniel Mack
Quoted 5 lines · Wolfgang Denk
Quoted 5 lines

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.

Quoted 8 lines
Quoted 5 lines
---
 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.

Quoted 5 lines

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.

Quoted 4 lines

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

#7 lore raw

PATCH 1/1 ne2000: take MAC address from config if available

Dear Daniel,

Quoted 8 lines · In message <[email protected]> you wrote:
Quoted 5 lines

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.

Quoted 4 lines

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

Signature · 5 lines
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
Business is like a wheelbarrow. Nothing ever happens until you  start
pushing.