[U-Boot] [PATCH] drivers/net/e1000.c: Fix GCC 4.6 build warnings

[U-Boot] [PATCH] drivers/net/e1000.c: Fix GCC 4.6 build warnings

[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 <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);
-- 
1.7.5.4

[U-Boot] [PATCH] drivers/net/e1000.c: Fix GCC 4.6 build warnings

[Moffett, Kyle D]

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/

[U-Boot] [PATCH] drivers/net/e1000.c: Fix GCC 4.6 build warnings

[Mike Frysinger]

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

[U-Boot] [PATCH] drivers/net/e1000.c: Fix GCC 4.6 build warnings

[Anatolij Gustschin]

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

[U-Boot] [PATCH v2] drivers/net/e1000.c: Fix GCC 4.6 build warnings

[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 <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);
-- 
1.7.5.4

[U-Boot] [PATCH v2] drivers/net/e1000.c: Fix GCC 4.6 build warnings

[Moffett, Kyle D]

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/

[U-Boot] [PATCH v2] drivers/net/e1000.c: Fix GCC 4.6 build warnings

[Wolfgang Denk]

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

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