| Message ID | 1324396187-25157-1-git-send-email-agust@denx.de |
|---|---|
| State | Superseded |
| Headers | show |
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
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);
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(-)