[U-Boot-Users] [PATCH] Add ability to take MAC address from the environment to DM9000 driver
Signed-off-by: Mike Rapoport mike@compulab.co.il
drivers/dm9000x.c | 14 ++++++++++++++ include/net.h | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/drivers/dm9000x.c b/drivers/dm9000x.c index 6877076..6486699 100644 --- a/drivers/dm9000x.c +++ b/drivers/dm9000x.c @@ -302,6 +302,20 @@ eth_init(bd_t * bd) /* Set Node address */ for (i = 0; i < 6; i++) ((u16 *) bd->bi_enetaddr)[i] = read_srom_word(i); + + if (!is_valid_ether_addr(bd->bi_enetaddr)) { + /* try reading from environment */ + u8 i; + char *s, *e; + s = getenv ("ethaddr"); + for (i = 0; i < 6; ++i) { + bd->bi_enetaddr[i] = s ? + simple_strtoul (s, &e, 16) : 0; + if (s) + s = (*e) ? e + 1 : e; + } + } + printf("MAC: %02x:%02x:%02x:%02x:%02x:%02x\n", bd->bi_enetaddr[0], bd->bi_enetaddr[1], bd->bi_enetaddr[2], bd->bi_enetaddr[3], bd->bi_enetaddr[4], bd->bi_enetaddr[5]); diff --git a/include/net.h b/include/net.h index 461e038..34967e4 100644 --- a/include/net.h +++ b/include/net.h @@ -435,6 +435,45 @@ static inline void NetCopyLong(ulong *to, ulong *from) memcpy((void*)to, (void*)from, sizeof(ulong)); }
+/** + * is_zero_ether_addr - Determine if give Ethernet address is all zeros. + * @addr: Pointer to a six-byte array containing the Ethernet address + * + * Return true if the address is all zeroes. + */ +static inline int is_zero_ether_addr(const u8 *addr) +{ + return !(addr[0] | addr[1] | addr[2] | addr[3] | addr[4] | addr[5]); +} + +/** + * is_multicast_ether_addr - Determine if the Ethernet address is a multicast. + * @addr: Pointer to a six-byte array containing the Ethernet address + * + * Return true if the address is a multicast address. + * By definition the broadcast address is also a multicast address. + */ +static inline int is_multicast_ether_addr(const u8 *addr) +{ + return (0x01 & addr[0]); +} + +/** + * is_valid_ether_addr - Determine if the given Ethernet address is valid + * @addr: Pointer to a six-byte array containing the Ethernet address + * + * Check that the Ethernet address (MAC) is not 00:00:00:00:00:00, is not + * a multicast address, and is not FF:FF:FF:FF:FF:FF. + * + * Return true if the address is valid. + */ +static inline int is_valid_ether_addr(const u8 *addr) +{ + /* FF:FF:FF:FF:FF:FF is a multicast address so we don't need to + * explicitly check for it here. */ + return !is_multicast_ether_addr(addr) && !is_zero_ether_addr(addr); +} + /* Convert an IP address to a string */ extern void ip_to_string (IPaddr_t x, char *s);
In message 46B84A47.3030908@compulab.co.il you wrote:
Signed-off-by: Mike Rapoport mike@compulab.co.il
Patch does not apply to current u-boot (neither -testing nor mainline). Please fix and resubmit:
error: patch failed: drivers/dm9000x.c:302 error: drivers/dm9000x.c: patch does not apply error: patch failed: include/net.h:435 error: include/net.h: patch does not apply Using index info to reconstruct a base tree... error: patch failed: drivers/dm9000x.c:302 error: drivers/dm9000x.c: patch does not apply error: patch failed: include/net.h:435 error: include/net.h: patch does not apply Did you hand edit your patch? It does not apply to blobs recorded in its index. Cannot fall back to three-way merge. Patch failed at 0001.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 46B84A47.3030908@compulab.co.il you wrote:
Signed-off-by: Mike Rapoport mike@compulab.co.il
Patch does not apply to current u-boot (neither -testing nor mainline). Please fix and resubmit:
error: patch failed: drivers/dm9000x.c:302 error: drivers/dm9000x.c: patch does not apply error: patch failed: include/net.h:435 error: include/net.h: patch does not apply Using index info to reconstruct a base tree... error: patch failed: drivers/dm9000x.c:302 error: drivers/dm9000x.c: patch does not apply error: patch failed: include/net.h:435 error: include/net.h: patch does not apply Did you hand edit your patch? It does not apply to blobs recorded in its index. Cannot fall back to three-way merge. Patch failed at 0001.
I've just cloned the mainline HEAD, and created the patch from there. This time the patch attached to avoid Thunderbird intervention.
Best regards,
Wolfgang Denk
In message 46B88FDD.4090708@compulab.co.il you wrote:
I've just cloned the mainline HEAD, and created the patch from there. This time the patch attached to avoid Thunderbird intervention.
Applied, thanks.
Best regards,
Wolfgang Denk
In message 20070809210451.E4756353428@atlas.denx.de you wrote:
In message 46B88FDD.4090708@compulab.co.il you wrote:
I've just cloned the mainline HEAD, and created the patch from there. This time the patch attached to avoid Thunderbird intervention.
Applied, thanks.
Oops. There was later discussion on this, and probably a better patch coming?
Undone. Sorry for the confusion.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 20070809210451.E4756353428@atlas.denx.de you wrote:
In message 46B88FDD.4090708@compulab.co.il you wrote:
I've just cloned the mainline HEAD, and created the patch from there. This time the patch attached to avoid Thunderbird intervention.
Applied, thanks.
Oops. There was later discussion on this, and probably a better patch coming?
Following the comments of the discussion, below is the updated patch.
Undone. Sorry for the confusion.
Best regards,
Wolfgang Denk
Signed-off-by: Mike Rapoport mike@compulab.co.il --- drivers/dm9000x.c | 15 +++++++++++++++ include/net.h | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/drivers/dm9000x.c b/drivers/dm9000x.c index 6877076..78acb09 100644 --- a/drivers/dm9000x.c +++ b/drivers/dm9000x.c @@ -302,6 +302,21 @@ eth_init(bd_t * bd) /* Set Node address */ for (i = 0; i < 6; i++) ((u16 *) bd->bi_enetaddr)[i] = read_srom_word(i); + + if (!is_zero_ether_addr(bd->bi_enetaddr) && + !is_mutlicast_ether_addr(bd->bi_enetaddr)) { + /* try reading from environment */ + u8 i; + char *s, *e; + s = getenv ("ethaddr"); + for (i = 0; i < 6; ++i) { + bd->bi_enetaddr[i] = s ? + simple_strtoul (s, &e, 16) : 0; + if (s) + s = (*e) ? e + 1 : e; + } + } + printf("MAC: %02x:%02x:%02x:%02x:%02x:%02x\n", bd->bi_enetaddr[0], bd->bi_enetaddr[1], bd->bi_enetaddr[2], bd->bi_enetaddr[3], bd->bi_enetaddr[4], bd->bi_enetaddr[5]); diff --git a/include/net.h b/include/net.h index 9671948..aa58e33 100644 --- a/include/net.h +++ b/include/net.h @@ -435,6 +435,29 @@ static inline void NetCopyLong(ulong *to, ulong *from) memcpy((void*)to, (void*)from, sizeof(ulong)); }
+/** + * is_zero_ether_addr - Determine if give Ethernet address is all zeros. + * @addr: Pointer to a six-byte array containing the Ethernet address + * + * Return true if the address is all zeroes. + */ +static inline int is_zero_ether_addr(const u8 *addr) +{ + return !(addr[0] | addr[1] | addr[2] | addr[3] | addr[4] | addr[5]); +} + +/** + * is_multicast_ether_addr - Determine if the Ethernet address is a multicast. + * @addr: Pointer to a six-byte array containing the Ethernet address + * + * Return true if the address is a multicast address. + * By definition the broadcast address is also a multicast address. + */ +static inline int is_multicast_ether_addr(const u8 *addr) +{ + return (0x01 & addr[0]); +} + /* Convert an IP address to a string */ extern void ip_to_string (IPaddr_t x, char *s);
Wolfgang/Mike,
Mike Rapoport wrote:
Wolfgang Denk wrote:
In message 20070809210451.E4756353428@atlas.denx.de you wrote:
In message 46B88FDD.4090708@compulab.co.il you wrote:
I'm currently pulling in a few net-related patches and will include this one. Expect a pull request some time today or early tomorrow.
regards, Ben
In message 46B84A47.3030908@compulab.co.il you wrote:
Signed-off-by: Mike Rapoport mike@compulab.co.il
Ditto:
error: patch failed: include/asm-arm/arch-pxa/pxa-regs.h:150 error: include/asm-arm/arch-pxa/pxa-regs.h: patch does not apply Using index info to reconstruct a base tree... error: patch failed: include/asm-arm/arch-pxa/pxa-regs.h:150 error: include/asm-arm/arch-pxa/pxa-regs.h: patch does not apply Did you hand edit your patch? It does not apply to blobs recorded in its index. Cannot fall back to three-way merge. Patch failed at 0001.
Best regards,
Wolfgang Denk
Hi Mike,
Mike Rapoport wrote:
Signed-off-by: Mike Rapoport mike@compulab.co.il
drivers/dm9000x.c | 14 ++++++++++++++ include/net.h | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/drivers/dm9000x.c b/drivers/dm9000x.c index 6877076..6486699 100644 --- a/drivers/dm9000x.c +++ b/drivers/dm9000x.c @@ -302,6 +302,20 @@ eth_init(bd_t * bd) /* Set Node address */ for (i = 0; i < 6; i++) ((u16 *) bd->bi_enetaddr)[i] = read_srom_word(i);
- if (!is_valid_ether_addr(bd->bi_enetaddr)) {
/* try reading from environment */u8 i;char *s, *e;s = getenv ("ethaddr");
Could this ever be other than the first Ethernet controller on a board? If so, 'ethaddr' won't cut it.
for (i = 0; i < 6; ++i) {bd->bi_enetaddr[i] = s ?simple_strtoul (s, &e, 16) : 0;if (s)s = (*e) ? e + 1 : e;}- }
While this is probably fine, it looks scary to me. My little brain is going to need a few more passes...
- printf("MAC: %02x:%02x:%02x:%02x:%02x:%02x\n", bd->bi_enetaddr[0], bd->bi_enetaddr[1], bd->bi_enetaddr[2], bd->bi_enetaddr[3], bd->bi_enetaddr[4], bd->bi_enetaddr[5]);
diff --git a/include/net.h b/include/net.h index 461e038..34967e4 100644 --- a/include/net.h +++ b/include/net.h @@ -435,6 +435,45 @@ static inline void NetCopyLong(ulong *to, ulong *from) memcpy((void*)to, (void*)from, sizeof(ulong)); }
+/**
- is_zero_ether_addr - Determine if give Ethernet address is all zeros.
- @addr: Pointer to a six-byte array containing the Ethernet address
- Return true if the address is all zeroes.
- */
+static inline int is_zero_ether_addr(const u8 *addr) +{
- return !(addr[0] | addr[1] | addr[2] | addr[3] | addr[4] | addr[5]);
+}
+/**
- is_multicast_ether_addr - Determine if the Ethernet address is a multicast.
- @addr: Pointer to a six-byte array containing the Ethernet address
- Return true if the address is a multicast address.
- By definition the broadcast address is also a multicast address.
- */
+static inline int is_multicast_ether_addr(const u8 *addr) +{
- return (0x01 & addr[0]);
+}
+/**
- is_valid_ether_addr - Determine if the given Ethernet address is valid
- @addr: Pointer to a six-byte array containing the Ethernet address
- Check that the Ethernet address (MAC) is not 00:00:00:00:00:00, is not
- a multicast address, and is not FF:FF:FF:FF:FF:FF.
- Return true if the address is valid.
- */
+static inline int is_valid_ether_addr(const u8 *addr) +{
Please choose a better name for this function. While multicast addresses are poor choices for source, they're perfectly fine for destination.
thanks, Ben
Ben Warren wrote:
Hi Mike,
Mike Rapoport wrote:
s = getenv ("ethaddr");Could this ever be other than the first Ethernet controller on a board? If so, 'ethaddr' won't cut it.
If a board has more than one DM9000, the driver in its current state does not support it anyway.
for (i = 0; i < 6; ++i) {- }
While this is probably fine, it looks scary to me. My little brain is going to need a few more passes...
- printf("MAC: %02x:%02x:%02x:%02x:%02x:%02x\n", bd->bi_enetaddr[0], bd->bi_enetaddr[1], bd->bi_enetaddr[2], bd->bi_enetaddr[3], bd->bi_enetaddr[4], bd->bi_enetaddr[5]);
+/**
- is_valid_ether_addr - Determine if the given Ethernet address is valid
- @addr: Pointer to a six-byte array containing the Ethernet address
- Check that the Ethernet address (MAC) is not 00:00:00:00:00:00, is not
- a multicast address, and is not FF:FF:FF:FF:FF:FF.
- Return true if the address is valid.
- */
+static inline int is_valid_ether_addr(const u8 *addr) +{
Please choose a better name for this function. While multicast addresses are poor choices for source, they're perfectly fine for destination.
What about 'is_valid_ether_src_addr'?
thanks, Ben
This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
In message 46B9563B.1010807@compulab.co.il you wrote:
Could this ever be other than the first Ethernet controller on a board? If so, 'ethaddr' won't cut it.
If a board has more than one DM9000, the driver in its current state does not support it anyway.
Assume the board has an XXX Ethernet controller as eth0, and a DM9000 as eth1 ?
Please choose a better name for this function. While multicast addresses are poor choices for source, they're perfectly fine for destination.
What about 'is_valid_ether_src_addr'?
That's a very long name which is difficult to read, difficult to type, and makes for very long source code lines...
And I will probably have to look up the definition of this function each time I read the code. You use this function exactly once, so I suggest to NOT define it at all and write down what it does:
if (!is_multicast_ether_addr(addr) && !is_zero_ether_addr(addr)) ...
This is IMHO much better to read and to understand.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
In message 46B9563B.1010807@compulab.co.il you wrote:
Could this ever be other than the first Ethernet controller on a board? If so, 'ethaddr' won't cut it.
If a board has more than one DM9000, the driver in its current state does not support it anyway.
Assume the board has an XXX Ethernet controller as eth0, and a DM9000 as eth1 ?
The only solution I can think of in this case is adding some #define CONFIG_DM9000_ETHER_INDEX in the board config and approptiate '#if' clauses in the driver. If it's Ok with you I'll do it and resend the patch. Still, the configuration you mention seems very theoretical for me and I can hardly imagine someone designing a board with such configuration.
Please choose a better name for this function. While multicast addresses are poor choices for source, they're perfectly fine for destination.
What about 'is_valid_ether_src_addr'?
That's a very long name which is difficult to read, difficult to type, and makes for very long source code lines...
And I will probably have to look up the definition of this function each time I read the code. You use this function exactly once, so I suggest to NOT define it at all and write down what it does:
if (!is_multicast_ether_addr(addr) && !is_zero_ether_addr(addr)) ...
This is IMHO much better to read and to understand.
I agree and I'll fix it.
Best regards,
Wolfgang Denk
Mike Rapoport wrote:
Wolfgang Denk wrote:
In message 46B9563B.1010807@compulab.co.il you wrote:
Could this ever be other than the first Ethernet controller on a board? If so, 'ethaddr' won't cut it.
If a board has more than one DM9000, the driver in its current state does not support it anyway.
Assume the board has an XXX Ethernet controller as eth0, and a DM9000 as eth1 ?
The only solution I can think of in this case is adding some #define CONFIG_DM9000_ETHER_INDEX in the board config and approptiate '#if' clauses in the driver. If it's Ok with you I'll do it and resend the patch. Still, the configuration you mention seems very theoretical for me and I can hardly imagine someone designing a board with such configuration.
This may not be necessary, since this controller isn't CONFIG_NET_MULTI compatible, although it should be. Yeah, I know you pointed this out already. Sorry...
This type of configuration is not at all theoretical. While it's hardly a shining example of hardware design, we have a board that uses both TSECs on an MPC8349 for high speed traffic, but we needed another 10/100 interface for control purposes. We put a cheap MAC/PHY chip on the local bus to meet this need. It wasn't a DM9000, but it could have been.
regards, Ben
participants (3)
-
Ben Warren -
Mike Rapoport -
Wolfgang Denk