Add ability to take MAC address from the environment to DM9000 driver
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
drivers/dm9000x.c | 14 ++++++++++++++ include/net.h | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 0 deletions(-)
drivers/dm9000x.c +14 -0
@@ -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 ]);
include/net.h +39 -0
@@ -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);
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 1 line · In message <[email protected]> you wrote:
Signed-off-by Mike Rapoport <[email protected]>
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
Signature · 4 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 Computers are not intelligent. They only think they are.
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 6 lines · Wolfgang Denk
Quoted 1 line · In message <[email protected]> you wrote:
Signed-off-by Mike Rapoport <[email protected]>Patch does not apply to current u-boot (neither -testing nor mainline). Please fix and resubmit:
Quoted 13 lines
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.
Quoted 4 lines
Best regards,
Wolfgang Denk
Signature · 10 lines
Sincerely yours, Mike. -------------- next part -------------- A non-text attachment was scrubbed... Name: dm9000.patch Type: text/x-patch Size: 2425 bytes Desc: not available Url : http://lists.denx.de/pipermail/u-boot/attachments/20070807/ab6da7e9/attachment.bin
Wolfgang Denk
AppliedPATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 3 lines · In message <[email protected]> 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
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 ...though his invention worked superbly -- his theory was a crock of sewage from beginning to end. - Vernor Vinge, "The Peace War"
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines · In message <[email protected]> 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
Signature · 6 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 Lispers are among the best grads of the Sweep-It-Under-Someone- Else's-Carpet School of Simulated Simplicity. [Was that sufficiently incendiary? :-)] - Larry Wall in <[email protected]
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 8 lines · Wolfgang Denk
Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 2 lines · In message <[email protected]> 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.
Quoted 6 lines
Undone. Sorry for the confusion.
Best regards,
Wolfgang Denk
--- drivers/dm9000x.c | 15 +++++++++++++++ include/net.h | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 0 deletions(-)
drivers/dm9000x.c +15 -0
@@ -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 ]);
include/net.h +23 -0
@@ -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);
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Wolfgang/Mike,
Quoted 5 lines · Mike Rapoport
Quoted 3 lines · Wolfgang Denk
Quoted 1 line · In message <[email protected]> you wrote:
In message <[email protected]> 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
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
In reply to Mike RapoportQuoted 1 line · In message <[email protected]> you wrote:
Signed-off-by Mike Rapoport <[email protected]>
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
Signature · 6 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 It became apparent that one reason why the Ice Giants were known as the Ice Giants was because they were, well, giants. The other was that they were made of ice. -Terry Pratchett, _Sourcery_
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
In reply to Mike RapoportHi Mike,
Quoted 11 lines · Mike Rapoport
Signed-off-by Mike Rapoport <[email protected]>drivers/dm9000x.c | 14 ++++++++++++++ include/net.h | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 0 deletions(-)drivers/dm9000x.c +0
-0
drivers/dm9000x.c eth_init(bd_t * bd) · line 311
Quoted 10 lines
drivers/dm9000x.c +6
-0@@ -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.
drivers/dm9000x.c eth_init(bd_t * bd)
Quoted 8 lines
drivers/dm9000x.c +7
-0+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...
Quoted 37 lines
drivers/dm9000x.c +1
-0+ 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]);include/net.h +25
-0@@ -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
include/net.h static inline void NetCopyLong(ulong *to, ulong *from) · line 472
Quoted 10 lines
include/net.h +9
-0+* @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
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 8 lines · Ben Warren
Hi Mike,
drivers/dm9000x.c eth_init(bd_t * bd) · line 310
Quoted 2 lines · Mike Rapoport
drivers/dm9000x.c +1
-0+ 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.
Quoted 10 lines
Review
Quoted 3 lines
(quoted diff) +2
-0+ 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...
Quoted 4 lines
+
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]);
Quoted 14 lines
include/net.h static inline void NetCopyLong(ulong *to, ulong *from) · line 471
Quoted 12 lines
include/net.h +11
-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.
What about 'is_valid_ether_src_addr'?
Quoted 13 lines
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 at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users
Signature · 2 lines
Sincerely yours, Mike.
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 3 lines
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 ?
Quoted 4 lines
Quoted 2 lines
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
Signature · 6 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 The more complex the mind, the greater the need for the simplicity of play. -- Kirk, "Shore Leave", stardate 3025.8
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 8 lines · Wolfgang Denk
Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 2 lines
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.
Quoted 7 lines
Quoted 3 lines
Quoted 2 lines
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...
Quoted 7 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.
Quoted 4 lines
Best regards,
Wolfgang Denk
Signature · 2 lines
Sincerely yours, Mike.
PATCH 1/1 Add ability to take MAC address from the environment to DM9000 driver
Quoted 10 lines · Mike Rapoport
Quoted 8 lines · Wolfgang Denk
Quoted 4 lines · In message <[email protected]> you wrote:
Quoted 2 lines
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 ?
Quoted 8 lines
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