[U-Boot] [PATCH] drivers/net/e1000.c: Fix GCC 4.6 build warnings
Fix: e1000.c: In function 'e1000_read_mac_addr': e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
Signed-off-by: Anatolij Gustschin agust@denx.de --- drivers/net/e1000.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index 6b71bd9..54f6425 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -45,6 +45,7 @@ tested on both gig copper and gig fiber boards */
#include "e1000.h" +#include <asm/unaligned.h>
#define TOUT_LOOP 100000
@@ -1146,7 +1147,8 @@ e1000_read_mac_addr(struct eth_device *nic) nic->enetaddr[5] ^= 1;
#ifdef CONFIG_E1000_FALLBACK_MAC - if ( *(u32*)(nic->enetaddr) == 0 || *(u32*)(nic->enetaddr) == ~0 ) { + if (get_unaligned_be32(nic->enetaddr) == 0 || + get_unaligned_be32(nic->enetaddr) == ~0) { unsigned char fb_mac[NODE_ADDRESS_SIZE] = CONFIG_E1000_FALLBACK_MAC;
memcpy (nic->enetaddr, fb_mac, NODE_ADDRESS_SIZE);
On Dec 20, 2011, at 10:49, Anatolij Gustschin wrote:
Fix: e1000.c: In function 'e1000_read_mac_addr': e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
[...]
#ifdef CONFIG_E1000_FALLBACK_MAC
- if ( *(u32*)(nic->enetaddr) == 0 || *(u32*)(nic->enetaddr) == ~0 ) {
if (get_unaligned_be32(nic->enetaddr) == 0 ||
get_unaligned_be32(nic->enetaddr) == ~0) {unsigned char fb_mac[NODE_ADDRESS_SIZE] = CONFIG_E1000_FALLBACK_MAC;
memcpy (nic->enetaddr, fb_mac, NODE_ADDRESS_SIZE);
No, if you are going to fix this code then make it use the right function for the job: is_valid_ether_addr()
Furthermore, while the E1000 chipset does not generally work very well without a proper SPI EEPROM image (if at all), I think it would be better for the driver to load successfully and use the "macaddr" from the U-Boot environment instead of some hardcoded compile-time constant.
IE: That whole code block should be ripped out and instead just tweak the "valid MAC address" check further down.
Cheers, Kyle Moffett
-- Curious about my work on the Debian powerpcspe port? I'm keeping a blog here: http://pureperl.blogspot.com/
On Tuesday 20 December 2011 11:07:30 Moffett, Kyle D wrote:
On Dec 20, 2011, at 10:49, Anatolij Gustschin wrote:
#ifdef CONFIG_E1000_FALLBACK_MAC
- if ( *(u32*)(nic->enetaddr) == 0 || *(u32*)(nic->enetaddr) == ~0 ) {
if (get_unaligned_be32(nic->enetaddr) == 0 ||
get_unaligned_be32(nic->enetaddr) == ~0) {unsigned char fb_mac[NODE_ADDRESS_SIZE] = CONFIG_E1000_FALLBACK_MAC; memcpy (nic->enetaddr, fb_mac, NODE_ADDRESS_SIZE);
No, if you are going to fix this code then make it use the right function for the job: is_valid_ether_addr()
+1 -mike
On Tue, 20 Dec 2011 10:07:30 -0600 "Moffett, Kyle D" Kyle.D.Moffett@boeing.com wrote:
On Dec 20, 2011, at 10:49, Anatolij Gustschin wrote:
Fix: e1000.c: In function 'e1000_read_mac_addr': e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
[...]
#ifdef CONFIG_E1000_FALLBACK_MAC
- if ( *(u32*)(nic->enetaddr) == 0 || *(u32*)(nic->enetaddr) == ~0 ) {
if (get_unaligned_be32(nic->enetaddr) == 0 ||
get_unaligned_be32(nic->enetaddr) == ~0) {unsigned char fb_mac[NODE_ADDRESS_SIZE] = CONFIG_E1000_FALLBACK_MAC;
memcpy (nic->enetaddr, fb_mac, NODE_ADDRESS_SIZE);
No, if you are going to fix this code then make it use the right function for the job: is_valid_ether_addr()
You are right, I'll fix it using is_valid_ether_addr().
Furthermore, while the E1000 chipset does not generally work very well without a proper SPI EEPROM image (if at all), I think it would be better for the driver to load successfully and use the "macaddr" from the U-Boot environment instead of some hardcoded compile-time constant.
IE: That whole code block should be ripped out and instead just tweak the "valid MAC address" check further down.
The config option is documented in README: CONFIG_E1000_FALLBACK_MAC default MAC for empty EEPROM after production.
I assume this block is only for a possibility to use the driver until the eeprom is programmed with a valid mac address.
Thanks, Anatolij
Fix: e1000.c: In function 'e1000_read_mac_addr': e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
Signed-off-by: Anatolij Gustschin agust@denx.de Cc: Kyle Moffett Kyle.D.Moffett@boeing.com --- v2: - use is_valid_ether_addr() for the check as suggested by Kyle Moffett
drivers/net/e1000.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index 6b71bd9..e726f39 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -1146,7 +1146,7 @@ e1000_read_mac_addr(struct eth_device *nic) nic->enetaddr[5] ^= 1;
#ifdef CONFIG_E1000_FALLBACK_MAC - if ( *(u32*)(nic->enetaddr) == 0 || *(u32*)(nic->enetaddr) == ~0 ) { + if (!is_valid_ether_addr(nic->enetaddr)) { unsigned char fb_mac[NODE_ADDRESS_SIZE] = CONFIG_E1000_FALLBACK_MAC;
memcpy (nic->enetaddr, fb_mac, NODE_ADDRESS_SIZE);
On Dec 20, 2011, at 12:36, Anatolij Gustschin wrote:
Fix: e1000.c: In function 'e1000_read_mac_addr': e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
Signed-off-by: Anatolij Gustschin agust@denx.de Cc: Kyle Moffett Kyle.D.Moffett@boeing.com
Looks great, thanks!
Acked-by: Kyle Moffett Kyle.D.Moffett@boeing.com
Cheers, Kyle Moffett
-- Curious about my work on the Debian powerpcspe port? I'm keeping a blog here: http://pureperl.blogspot.com/
Dear Anatolij Gustschin,
In message 1324402599-781-1-git-send-email-agust@denx.de you wrote:
Fix: e1000.c: In function 'e1000_read_mac_addr': e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
e1000.c:1149:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
Signed-off-by: Anatolij Gustschin agust@denx.de Cc: Kyle Moffett Kyle.D.Moffett@boeing.com
v2:
- use is_valid_ether_addr() for the check as suggested by Kyle Moffett
drivers/net/e1000.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (4)
-
Anatolij Gustschin -
Mike Frysinger -
Moffett, Kyle D -
Wolfgang Denk