drivers/net/e1000.c: Fix GCC 4.6 build warnings

PATCH New Anatolij Gustschin 4 participants 7 messages Dec 20, 2011 Network drivers

Share & tools

Anatolij Gustschin

author

2011-12-20 15:49 UTC #0 lore raw

PATCH 1/1 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 <[email protected]>

---
 drivers/net/e1000.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

drivers/net/e1000.c +3 -1

index 6b71bd9..54f6425 100644
@@ -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);

Signature · 1 line

1.7.5.4

Moffett

2011-12-20 16:07 UTC #1 lore raw

PATCH 1/1 drivers/net/e1000.c: Fix GCC 4.6 build warnings

Quoted 4 lines · Anatolij Gustschin

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]

[...]

Quoted 7 lines

#ifdef CONFIG_E1000_FALLBACK_MAC

drivers/net/e1000.c +2 -1

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

Mike Frysinger

2011-12-20 17:20 UTC #2 lore raw

PATCH 1/1 drivers/net/e1000.c: Fix GCC 4.6 build warnings

Quoted 6 lines · Kyle D

Quoted 5 lines · Anatolij Gustschin
#ifdef CONFIG_E1000_FALLBACK_MAC
drivers/net/e1000.c +2 -1
-	if ( *(u32*)(nic->enetaddr) == 0 || *(u32*)(nic->enetaddr) == ~0 ) {
+	if (get_unaligned_be32(nic->enetaddr) == 0 ||
+	    get_unaligned_be32(nic->enetaddr) == ~0) {

Quoted 6 lines

Quoted 3 lines
		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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111220/ff4a1c87/attachment.pgp>

Anatolij Gustschin

author

2011-12-20 17:26 UTC #3 lore raw

PATCH 1/1 drivers/net/e1000.c: Fix GCC 4.6 build warnings

In reply to Moffett

Quoted 12 lines · Kyle D

Quoted 4 lines · Anatolij Gustschin
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]
[...]
Quoted 6 lines
#ifdef CONFIG_E1000_FALLBACK_MAC
drivers/net/e1000.c +2 -1
-	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;

Quoted 4 lines

Quoted 1 line
		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().

Quoted 8 lines

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

Anatolij Gustschin

author

2011-12-20 17:36 UTC #4 lore raw

PATCH v2 1/1 drivers/net/e1000.c: Fix GCC 4.6 build warnings

In reply to Anatolij Gustschin

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 <[email protected]>

Cc Kyle Moffett <[email protected]>

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

drivers/net/e1000.c +1 -1

index 6b71bd9..e726f39 100644
@@ -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);

Signature · 1 line

1.7.5.4

Moffett

2011-12-20 17:44 UTC #5 lore raw

PATCH v2 1/1 drivers/net/e1000.c: Fix GCC 4.6 build warnings

Quoted 10 lines · Anatolij Gustschin

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 <[email protected]>
Cc Kyle Moffett <[email protected]>

Looks great, thanks!

Acked-by Kyle Moffett <[email protected]>

Cheers,
Kyle Moffett

--
Curious about my work on the Debian powerpcspe port? I'm keeping a blog here: http://pureperl.blogspot.com/

Wolfgang Denk

Applied

2011-12-20 22:21 UTC #6 lore raw

PATCH v2 1/1 drivers/net/e1000.c: Fix GCC 4.6 build warnings

In reply to Anatolij Gustschin

Dear Anatolij Gustschin,

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

Quoted 7 lines

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

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
If some day we are defeated, well, war has  its  fortunes,  good  and
bad.
	-- Commander Kor, "Errand of Mercy", stardate 3201.7