[PATCH] b44: fix eeprom endianess issue

[PATCH] b44: fix eeprom endianess issue

[Michael Buesch]

Hi Andrew,

Please apply this patch to -mm for testing.
I think in the long term we want to convert b44 to use the
new ssb backend driver, which would also fix the issue, but
for now I think this small fix is best.

Please note that this test is only compile tested, as
I don't have a b44 device.

--

This fixes eeprom read on big-endian architectures.

Signed-off-by: Michael Buesch <mb@bu3sch.de>

Index: linux-2.6/drivers/net/b44.c
===================================================================
--- linux-2.6.orig/drivers/net/b44.c	2006-08-22 11:27:56.000000000 +0200
+++ linux-2.6/drivers/net/b44.c	2006-08-23 12:26:31.000000000 +0200
@@ -2055,7 +2055,7 @@
 	u16 *ptr = (u16 *) data;
 
 	for (i = 0; i < 128; i += 2)
-		ptr[i / 2] = readw(bp->regs + 4096 + i);
+		ptr[i / 2] = cpu_to_le16(readw(bp->regs + 4096 + i));
 
 	return 0;
 }


-- 
Greetings Michael.

Re: [PATCH] b44: fix eeprom endianess issue

[Michael Buesch]

Oh, I think I should have CCed Jeff ;)
Sorry.

On Wednesday 23 August 2006 12:32, Michael Buesch wrote:
> Hi Andrew,
> 
> Please apply this patch to -mm for testing.
> I think in the long term we want to convert b44 to use the
> new ssb backend driver, which would also fix the issue, but
> for now I think this small fix is best.
> 
> Please note that this test is only compile tested, as
> I don't have a b44 device.
> 
> --
> 
> This fixes eeprom read on big-endian architectures.
> 
> Signed-off-by: Michael Buesch <mb@bu3sch.de>
> 
> Index: linux-2.6/drivers/net/b44.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/b44.c	2006-08-22 11:27:56.000000000 +0200
> +++ linux-2.6/drivers/net/b44.c	2006-08-23 12:26:31.000000000 +0200
> @@ -2055,7 +2055,7 @@
>  	u16 *ptr = (u16 *) data;
>  
>  	for (i = 0; i < 128; i += 2)
> -		ptr[i / 2] = readw(bp->regs + 4096 + i);
> +		ptr[i / 2] = cpu_to_le16(readw(bp->regs + 4096 + i));
>  
>  	return 0;
>  }
> 
> 

-- 
Greetings Michael.

Re: [PATCH] b44: fix eeprom endianess issue

[Jeff Garzik]

Michael Buesch wrote:
>> Please note that this test is only compile tested, as
>> I don't have a b44 device.

>> @@ -2055,7 +2055,7 @@
>>  	u16 *ptr = (u16 *) data;
>>  
>>  	for (i = 0; i < 128; i += 2)
>> -		ptr[i / 2] = readw(bp->regs + 4096 + i);
>> +		ptr[i / 2] = cpu_to_le16(readw(bp->regs + 4096 + i));
>>  


This looks a bit weird.  readw() swaps on big-endian already.

This patch swaps each word -again- on big-endian, even though the only 
user of the eeprom data is the get-invariants code that reads the MAC 
address and phy id.

	Jeff

Re: [PATCH] b44: fix eeprom endianess issue

[Michael Buesch]

On Wednesday 23 August 2006 18:55, Jeff Garzik wrote:
> Michael Buesch wrote:
> >> Please note that this test is only compile tested, as
> >> I don't have a b44 device.
> 
> >> @@ -2055,7 +2055,7 @@
> >>  	u16 *ptr = (u16 *) data;
> >>  
> >>  	for (i = 0; i < 128; i += 2)
> >> -		ptr[i / 2] = readw(bp->regs + 4096 + i);
> >> +		ptr[i / 2] = cpu_to_le16(readw(bp->regs + 4096 + i));
> >>  
> 
> 
> This looks a bit weird.  readw() swaps on big-endian already.
> 
> This patch swaps each word -again- on big-endian, even though the only 
> user of the eeprom data is the get-invariants code that reads the MAC 
> address and phy id.

Yeah. But look at where the data is stored.
The data ends up in a _byte_ array. A byte array is litte endian
(little end first). array[0] is low and array[1] is high.
Look at the pointer cast above:

u16 *ptr = (u16 *) data;

We store data in _cpu_ order in this byte array.
That's wrong.
If we are on a little endian machine, that's ok. But if
we are on a big endian machine, this will write bytes swapped.

LE will result in an array:
ABABABABABABAB
BE will result in an array:
BABABABABABABA

But only the first result (LE) is valid, because that's
expected later when interpreting the data.

-- 
Greetings Michael.