[PATCH] tg3: fix big endian MAC address collection failure

[PATCH] tg3: fix big endian MAC address collection failure

[James Bottomley]

[sorry for repost; wrong netdev address first time around]

We noticed on parisc that our broadcoms all swapped MAC addresses going
from 2.6.29 to 2.6.30-rc1:

Apr 11 07:48:24 ion kernel: eth0: Tigon3 [partno(BCM95700A6) rev 0105] (PCI:66MHz:64-bit) MAC address 00:30:6e:4b:15:59
Apr 13 07:34:34 ion kernel: eth0: Tigon3 [partno(BCM95700A6) rev 0105] (PCI:66MHz:64-bit) MAC address 00:00:59:15:4b:6e

The problem patch is:

commit 6d348f2c1e0bb1cf7a494b51fc921095ead3f6ae
Author: Matt Carlson <mcarlson@broadcom.com>
Date:   Wed Feb 25 14:25:52 2009 +0000

    tg3: Eliminate tg3_nvram_read_swab()

With the root cause being the use of memcpy to set the mac address:

   memcpy(&dev->dev_addr[0], ((char *)&hi) + 2, 2);
   memcpy(&dev->dev_addr[2], (char *)&lo, sizeof(lo));

This might work on little endian machines, but it can't on big endian
ones.  You have to use the original setting mechanism to be correct on
all architectures.

The attached patch fixes parisc.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

---

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 6a736dd..7a837c4 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -12443,8 +12443,13 @@ static int __devinit tg3_get_device_address(struct tg3 *tp)
 		/* Next, try NVRAM. */
 		if (!tg3_nvram_read_be32(tp, mac_offset + 0, &hi) &&
 		    !tg3_nvram_read_be32(tp, mac_offset + 4, &lo)) {
-			memcpy(&dev->dev_addr[0], ((char *)&hi) + 2, 2);
-			memcpy(&dev->dev_addr[2], (char *)&lo, sizeof(lo));
+			dev->dev_addr[0] = ((hi >> 16) & 0xff);
+			dev->dev_addr[1] = ((hi >> 24) & 0xff);
+			dev->dev_addr[2] = ((lo >>  0) & 0xff);
+			dev->dev_addr[3] = ((lo >>  8) & 0xff);
+			dev->dev_addr[4] = ((lo >> 16) & 0xff);
+			dev->dev_addr[5] = ((lo >> 24) & 0xff);
+
 		}
 		/* Finally just fetch it out of the MAC control regs. */
 		else {

Re: [PATCH] tg3: fix big endian MAC address collection failure

[David Miller]

From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: Mon, 13 Apr 2009 15:29:54 +0000

> We noticed on parisc that our broadcoms all swapped MAC addresses going
> from 2.6.29 to 2.6.30-rc1:
> 
> Apr 11 07:48:24 ion kernel: eth0: Tigon3 [partno(BCM95700A6) rev 0105] (PCI:66MHz:64-bit) MAC address 00:30:6e:4b:15:59
> Apr 13 07:34:34 ion kernel: eth0: Tigon3 [partno(BCM95700A6) rev 0105] (PCI:66MHz:64-bit) MAC address 00:00:59:15:4b:6e
> 
> The problem patch is:
> 
> commit 6d348f2c1e0bb1cf7a494b51fc921095ead3f6ae
> Author: Matt Carlson <mcarlson@broadcom.com>
> Date:   Wed Feb 25 14:25:52 2009 +0000
> 
>     tg3: Eliminate tg3_nvram_read_swab()
> 
> With the root cause being the use of memcpy to set the mac address:
> 
>    memcpy(&dev->dev_addr[0], ((char *)&hi) + 2, 2);
>    memcpy(&dev->dev_addr[2], (char *)&lo, sizeof(lo));
> 
> This might work on little endian machines, but it can't on big endian
> ones.  You have to use the original setting mechanism to be correct on
> all architectures.
> 
> The attached patch fixes parisc.
> 
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

I'm applying this, thanks a lot James!

Re: [PATCH] tg3: fix big endian MAC address collection failure

[James Bottomley]

On Mon, 2009-04-13 at 14:32 -0700, David Miller wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> Date: Mon, 13 Apr 2009 15:29:54 +0000
> 
> > We noticed on parisc that our broadcoms all swapped MAC addresses going
> > from 2.6.29 to 2.6.30-rc1:
> > 
> > Apr 11 07:48:24 ion kernel: eth0: Tigon3 [partno(BCM95700A6) rev 0105] (PCI:66MHz:64-bit) MAC address 00:30:6e:4b:15:59
> > Apr 13 07:34:34 ion kernel: eth0: Tigon3 [partno(BCM95700A6) rev 0105] (PCI:66MHz:64-bit) MAC address 00:00:59:15:4b:6e
> > 
> > The problem patch is:
> > 
> > commit 6d348f2c1e0bb1cf7a494b51fc921095ead3f6ae
> > Author: Matt Carlson <mcarlson@broadcom.com>
> > Date:   Wed Feb 25 14:25:52 2009 +0000
> > 
> >     tg3: Eliminate tg3_nvram_read_swab()
> > 
> > With the root cause being the use of memcpy to set the mac address:
> > 
> >    memcpy(&dev->dev_addr[0], ((char *)&hi) + 2, 2);
> >    memcpy(&dev->dev_addr[2], (char *)&lo, sizeof(lo));
> > 
> > This might work on little endian machines, but it can't on big endian
> > ones.  You have to use the original setting mechanism to be correct on
> > all architectures.
> > 
> > The attached patch fixes parisc.
> > 
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> I'm applying this, thanks a lot James!

Actually, hang on ... I fat fingered the list address first time around
and we've been replying on the wrong list.

Michael apparently has the code working for sparc and we're all a bit
stumped why it doesn't work on parisc since they should follow identical
code paths.

This is the latest state of play:

---
On Mon, 2009-04-13 at 11:37 -0700, Matt Carlson wrote:
> But that is exactly what the code is doing.  tg3_nvram_read_be32() will
> return the data in bytestream format.  A memcpy() should be all that is
> needed to transport the data to a different memory location.

But not the one you've done.  cpu_to_be32 is a nop pass through on our
architecture, so tg3_nvram_read_be32 is equivalent to tg3_nvram_read on
our architecture (i.e. identical to the code that was doing the read in
2.6.29).  However, the memcpy is the wrong way around for us.  If you
look at an example, the original code said

dev_addr[0] = hi >> 16;
dev_addr[1] = hi >> 24

So MSB-1 and MSB.  However, on a BE machine these are at offset one and
zero from the start of the word.  The replacement memcopy is:

memcpy(&dev->dev_addr[0], ((char *)&hi) + 2, 2)

i.e. offset 3 and 4, which actually copies LSB-1 and LSB into there.
You can follow similar logic to show that the lo copy is wrong too.

Perhaps the fix is just to put the tg3_nvram_read() back as well as the
original by loads?
---

James

Re: [PATCH] tg3: fix big endian MAC address collection failure

[David Miller]

From: James Bottomley <James.Bottomley@HansenPartnership.com>
Date: Mon, 13 Apr 2009 21:42:31 +0000

> Actually, hang on ... I fat fingered the list address first time around
> and we've been replying on the wrong list.
> 
> Michael apparently has the code working for sparc and we're all a bit
> stumped why it doesn't work on parisc since they should follow identical
> code paths.
> 
> This is the latest state of play:

Too late, I already pushed out to my net-2.6 tree on kernel.org

You'll need to send me any fixups relative to this patch.

Thanks.

Re: [PATCH] tg3: fix big endian MAC address collection failure

[Michael Chan]

On Mon, 2009-04-13 at 14:42 -0700, James Bottomley wrote:

> ---
> On Mon, 2009-04-13 at 11:37 -0700, Matt Carlson wrote:
> > But that is exactly what the code is doing.  tg3_nvram_read_be32() will
> > return the data in bytestream format.  A memcpy() should be all that is
> > needed to transport the data to a different memory location.
> 
> But not the one you've done.  cpu_to_be32 is a nop pass through on our
> architecture, so tg3_nvram_read_be32 is equivalent to tg3_nvram_read on
> our architecture (i.e. identical to the code that was doing the read in
> 2.6.29).  However, the memcpy is the wrong way around for us.  If you
> look at an example, the original code said

The old tg3_nvram_read() had a swab32() after the readl().  The new
tg3_nvram_read() no longer has the swab32().  There were too many layers
of swapping in the old code and that's why Matt wanted to clean it up.

James, can do dump out the nvram content on the parisc?

ethtool -e eth0 length 0x90

Thanks.

> 
> dev_addr[0] = hi >> 16;
> dev_addr[1] = hi >> 24
> 
> So MSB-1 and MSB.  However, on a BE machine these are at offset one and
> zero from the start of the word.  The replacement memcopy is:
> 
> memcpy(&dev->dev_addr[0], ((char *)&hi) + 2, 2)
> 
> i.e. offset 3 and 4, which actually copies LSB-1 and LSB into there.
> You can follow similar logic to show that the lo copy is wrong too.
> 
> Perhaps the fix is just to put the tg3_nvram_read() back as well as the
> original by loads?
> ---
> 
> James
> 
> 
> 

Re: [PATCH] tg3: fix big endian MAC address collection failure

[James Bottomley]

On Mon, 2009-04-13 at 15:17 -0700, Michael Chan wrote:
> On Mon, 2009-04-13 at 14:42 -0700, James Bottomley wrote:
> 
> > ---
> > On Mon, 2009-04-13 at 11:37 -0700, Matt Carlson wrote:
> > > But that is exactly what the code is doing.  tg3_nvram_read_be32() will
> > > return the data in bytestream format.  A memcpy() should be all that is
> > > needed to transport the data to a different memory location.
> > 
> > But not the one you've done.  cpu_to_be32 is a nop pass through on our
> > architecture, so tg3_nvram_read_be32 is equivalent to tg3_nvram_read on
> > our architecture (i.e. identical to the code that was doing the read in
> > 2.6.29).  However, the memcpy is the wrong way around for us.  If you
> > look at an example, the original code said
> 
> The old tg3_nvram_read() had a swab32() after the readl().  The new
> tg3_nvram_read() no longer has the swab32().  There were too many layers
> of swapping in the old code and that's why Matt wanted to clean it up.
> 
> James, can do dump out the nvram content on the parisc?
> 
> ethtool -e eth0 length 0x90
> 
> Thanks.

Sure, ion's is

ion:~# ethtool -e eth0 length 0x90
Address         Data
----------      ----
0x00000000      0xaa
0x00000001      0x55
0x00000002      0x99
0x00000003      0x66
0x00000004      0x00
0x00000005      0x00
0x00000006      0x00
0x00000007      0x08
0x00000008      0x29
0x00000009      0x02
0x0000000a      0x00
0x0000000b      0x00
0x0000000c      0x00
0x0000000d      0x02
0x0000000e      0x00
0x0000000f      0x00
0x00000010      0xc5
0x00000011      0x3c
0x00000012      0x82
0x00000013      0x9b
0x00000014      0x00
0x00000015      0x00
0x00000016      0x00
0x00000017      0x00
0x00000018      0x00
0x00000019      0x00
0x0000001a      0x00
0x0000001b      0x00
0x0000001c      0x00
0x0000001d      0x00
0x0000001e      0x00
0x0000001f      0x00
0x00000020      0x00
0x00000021      0x00
0x00000022      0x00
0x00000023      0x00
0x00000024      0x00
0x00000025      0x00
0x00000026      0x00
0x00000027      0x00
0x00000028      0x00
0x00000029      0x00
0x0000002a      0x00
0x0000002b      0x00
0x0000002c      0x00
0x0000002d      0x00
0x0000002e      0x00
0x0000002f      0x00
0x00000030      0x00
0x00000031      0x00
0x00000032      0x00
0x00000033      0x00
0x00000034      0x00
0x00000035      0x00
0x00000036      0x00
0x00000037      0x00
0x00000038      0x00
0x00000039      0x00
0x0000003a      0x00
0x0000003b      0x00
0x0000003c      0x00
0x0000003d      0x00
0x0000003e      0x00
0x0000003f      0x00
0x00000040      0x00
0x00000041      0x00
0x00000042      0x00
0x00000043      0x00
0x00000044      0x00
0x00000045      0x00
0x00000046      0x00
0x00000047      0x00
0x00000048      0x00
0x00000049      0x00
0x0000004a      0x00
0x0000004b      0x00
0x0000004c      0x00
0x0000004d      0x00
0x0000004e      0x00
0x0000004f      0x00
0x00000050      0x00
0x00000051      0x00
0x00000052      0x00
0x00000053      0x00
0x00000054      0x00
0x00000055      0x00
0x00000056      0x00
0x00000057      0x00
0x00000058      0x00
0x00000059      0x00
0x0000005a      0x00
0x0000005b      0x00
0x0000005c      0x00
0x0000005d      0x00
0x0000005e      0x00
0x0000005f      0x00
0x00000060      0x00
0x00000061      0x00
0x00000062      0x00
0x00000063      0x00
0x00000064      0x00
0x00000065      0x00
0x00000066      0x00
0x00000067      0x00
0x00000068      0x00
0x00000069      0x00
0x0000006a      0x00
0x0000006b      0x00
0x0000006c      0x00
0x0000006d      0x00
0x0000006e      0x00
0x0000006f      0x00
0x00000070      0x00
0x00000071      0x00
0x00000072      0x00
0x00000073      0x00
0x00000074      0x8c
0x00000075      0x00
0x00000076      0x00
0x00000077      0x43
0x00000078      0x10
0x00000079      0x61
0x0000007a      0x20
0x0000007b      0x00
0x0000007c      0x30
0x0000007d      0x00
0x0000007e      0x00
0x0000007f      0x00
0x00000080      0x59
0x00000081      0x15
0x00000082      0x4b
0x00000083      0x6e
0x00000084      0x39
0x00000085      0x4d
0x00000086      0x43
0x00000087      0x42
0x00000088      0x30
0x00000089      0x30
0x0000008a      0x37
0x0000008b      0x35
0x0000008c      0x00
0x0000008d      0x00
0x0000008e      0x36
0x0000008f      0x41

James

Re: [PATCH] tg3: fix big endian MAC address collection failure

[Matt Carlson]

On Mon, Apr 13, 2009 at 03:32:14PM -0700, James Bottomley wrote:
> On Mon, 2009-04-13 at 15:17 -0700, Michael Chan wrote:
> > On Mon, 2009-04-13 at 14:42 -0700, James Bottomley wrote:
> > 
> > > ---
> > > On Mon, 2009-04-13 at 11:37 -0700, Matt Carlson wrote:
> > > > But that is exactly what the code is doing.  tg3_nvram_read_be32() will
> > > > return the data in bytestream format.  A memcpy() should be all that is
> > > > needed to transport the data to a different memory location.
> > > 
> > > But not the one you've done.  cpu_to_be32 is a nop pass through on our
> > > architecture, so tg3_nvram_read_be32 is equivalent to tg3_nvram_read on
> > > our architecture (i.e. identical to the code that was doing the read in
> > > 2.6.29).  However, the memcpy is the wrong way around for us.  If you
> > > look at an example, the original code said
> > 
> > The old tg3_nvram_read() had a swab32() after the readl().  The new
> > tg3_nvram_read() no longer has the swab32().  There were too many layers
> > of swapping in the old code and that's why Matt wanted to clean it up.
> > 
> > James, can do dump out the nvram content on the parisc?
> > 
> > ethtool -e eth0 length 0x90
> > 
> > Thanks.
> 
> Sure, ion's is
> 
> ion:~# ethtool -e eth0 length 0x90
> Address         Data
> ----------      ----
> 0x00000000      0xaa
> 0x00000001      0x55
> 0x00000002      0x99
> 0x00000003      0x66

Michael noticed that your NVRAM signature is byteswapped in NVRAM. (!)
That also explains why the driver is trying to obtain the MAC address
through NVRAM, rather than getting it from shared memory.  The device's
bootcode is not working correctly.

> 0x00000004      0x00
> 0x00000005      0x00
> 0x00000006      0x00
> 0x00000007      0x08
> 0x00000008      0x29
> 0x00000009      0x02
> 0x0000000a      0x00
> 0x0000000b      0x00
> 0x0000000c      0x00
> 0x0000000d      0x02
> 0x0000000e      0x00
> 0x0000000f      0x00
> 0x00000010      0xc5
> 0x00000011      0x3c
> 0x00000012      0x82
> 0x00000013      0x9b
> 0x00000014      0x00
> 0x00000015      0x00
> 0x00000016      0x00
> 0x00000017      0x00
> 0x00000018      0x00
> 0x00000019      0x00
> 0x0000001a      0x00
> 0x0000001b      0x00
> 0x0000001c      0x00
> 0x0000001d      0x00
> 0x0000001e      0x00
> 0x0000001f      0x00
> 0x00000020      0x00
> 0x00000021      0x00
> 0x00000022      0x00
> 0x00000023      0x00
> 0x00000024      0x00
> 0x00000025      0x00
> 0x00000026      0x00
> 0x00000027      0x00
> 0x00000028      0x00
> 0x00000029      0x00
> 0x0000002a      0x00
> 0x0000002b      0x00
> 0x0000002c      0x00
> 0x0000002d      0x00
> 0x0000002e      0x00
> 0x0000002f      0x00
> 0x00000030      0x00
> 0x00000031      0x00
> 0x00000032      0x00
> 0x00000033      0x00
> 0x00000034      0x00
> 0x00000035      0x00
> 0x00000036      0x00
> 0x00000037      0x00
> 0x00000038      0x00
> 0x00000039      0x00
> 0x0000003a      0x00
> 0x0000003b      0x00
> 0x0000003c      0x00
> 0x0000003d      0x00
> 0x0000003e      0x00
> 0x0000003f      0x00
> 0x00000040      0x00
> 0x00000041      0x00
> 0x00000042      0x00
> 0x00000043      0x00
> 0x00000044      0x00
> 0x00000045      0x00
> 0x00000046      0x00
> 0x00000047      0x00
> 0x00000048      0x00
> 0x00000049      0x00
> 0x0000004a      0x00
> 0x0000004b      0x00
> 0x0000004c      0x00
> 0x0000004d      0x00
> 0x0000004e      0x00
> 0x0000004f      0x00
> 0x00000050      0x00
> 0x00000051      0x00
> 0x00000052      0x00
> 0x00000053      0x00
> 0x00000054      0x00
> 0x00000055      0x00
> 0x00000056      0x00
> 0x00000057      0x00
> 0x00000058      0x00
> 0x00000059      0x00
> 0x0000005a      0x00
> 0x0000005b      0x00
> 0x0000005c      0x00
> 0x0000005d      0x00
> 0x0000005e      0x00
> 0x0000005f      0x00
> 0x00000060      0x00
> 0x00000061      0x00
> 0x00000062      0x00
> 0x00000063      0x00
> 0x00000064      0x00
> 0x00000065      0x00
> 0x00000066      0x00
> 0x00000067      0x00
> 0x00000068      0x00
> 0x00000069      0x00
> 0x0000006a      0x00
> 0x0000006b      0x00
> 0x0000006c      0x00
> 0x0000006d      0x00
> 0x0000006e      0x00
> 0x0000006f      0x00
> 0x00000070      0x00
> 0x00000071      0x00
> 0x00000072      0x00
> 0x00000073      0x00
> 0x00000074      0x8c
> 0x00000075      0x00
> 0x00000076      0x00
> 0x00000077      0x43
> 0x00000078      0x10
> 0x00000079      0x61
> 0x0000007a      0x20
> 0x0000007b      0x00
> 0x0000007c      0x30
> 0x0000007d      0x00
> 0x0000007e      0x00
> 0x0000007f      0x00
> 0x00000080      0x59
> 0x00000081      0x15
> 0x00000082      0x4b
> 0x00000083      0x6e

Yes.  And your MAC address in NVRAM is 00:00:59:15:4b:6e.  And again, HP
MAC addresses start with 00:30:6e, so the data is byteswapped in NVRAM.
(00:00:59 MAC addresses belong to "hellige gmbh").

> 0x00000084      0x39
> 0x00000085      0x4d
> 0x00000086      0x43
> 0x00000087      0x42
> 0x00000088      0x30
> 0x00000089      0x30
> 0x0000008a      0x37
> 0x0000008b      0x35
> 0x0000008c      0x00
> 0x0000008d      0x00
> 0x0000008e      0x36
> 0x0000008f      0x41
> 
> James
> 
> 
> 

Re: [PATCH] tg3: fix big endian MAC address collection failure

[David Miller]

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Mon, 13 Apr 2009 18:25:48 -0700

> Michael noticed that your NVRAM signature is byteswapped in NVRAM. (!)
> That also explains why the driver is trying to obtain the MAC address
> through NVRAM, rather than getting it from shared memory.  The device's
> bootcode is not working correctly.

If this problem is pervasive, which it seems it is since we
have at least two people hitting this problem, we'll need
to find a way to handle it without saying "update your
BIOS or system firmware"

Re: [PATCH] tg3: fix big endian MAC address collection failure

[Matt Carlson]

On Mon, Apr 13, 2009 at 06:40:35PM -0700, David Miller wrote:
> From: "Matt Carlson" <mcarlson@broadcom.com>
> Date: Mon, 13 Apr 2009 18:25:48 -0700
> 
> > Michael noticed that your NVRAM signature is byteswapped in NVRAM. (!)
> > That also explains why the driver is trying to obtain the MAC address
> > through NVRAM, rather than getting it from shared memory.  The device's
> > bootcode is not working correctly.
> 
> If this problem is pervasive, which it seems it is since we
> have at least two people hitting this problem, we'll need
> to find a way to handle it without saying "update your
> BIOS or system firmware"

Well, Robin's problem looks distinctly different than James'.  I believe
Robin's problem has to do with the legacy EEPROM access routines.  The
patchset may have overlooked something in that area.  I just need to
gather evidence to prove it.

I did run some tests this afternoon on a similar IA64 HP machine and
they ran fine.  Consequently, I'm optimistic that James' problem is local
to his machine.  If this turns out to be a wider problem, then I agree
we'll need to find an appropriate solution.

Re: [PATCH] tg3: fix big endian MAC address collection failure

[Kyle McMartin]

On Mon, Apr 13, 2009 at 07:00:15PM -0700, Matt Carlson wrote:
> I did run some tests this afternoon on a similar IA64 HP machine and
> they ran fine.  Consequently, I'm optimistic that James' problem is local
> to his machine.  If this turns out to be a wider problem, then I agree
> we'll need to find an appropriate solution.
> 

ia64 Linux is little endian...

regards, Kyle

Re: [PATCH] tg3: fix big endian MAC address collection failure

[David Miller]

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Mon, 13 Apr 2009 19:00:15 -0700

> I did run some tests this afternoon on a similar IA64 HP machine and
> they ran fine.

It can't be similar, James Bottomly is on HP PARISC (big endian) not
IA64 (which is little endian).

I'm still convinced this is a big-endian driver problem and it has
nothing to do with broken NVRAM or similar.

Re: [PATCH] tg3: fix big endian MAC address collection failure

[James Bottomley]

On Mon, 2009-04-13 at 18:25 -0700, Matt Carlson wrote:
> On Mon, Apr 13, 2009 at 03:32:14PM -0700, James Bottomley wrote:
> > On Mon, 2009-04-13 at 15:17 -0700, Michael Chan wrote:
> > > On Mon, 2009-04-13 at 14:42 -0700, James Bottomley wrote:
> > > 
> > > > ---
> > > > On Mon, 2009-04-13 at 11:37 -0700, Matt Carlson wrote:
> > > > > But that is exactly what the code is doing.  tg3_nvram_read_be32() will
> > > > > return the data in bytestream format.  A memcpy() should be all that is
> > > > > needed to transport the data to a different memory location.
> > > > 
> > > > But not the one you've done.  cpu_to_be32 is a nop pass through on our
> > > > architecture, so tg3_nvram_read_be32 is equivalent to tg3_nvram_read on
> > > > our architecture (i.e. identical to the code that was doing the read in
> > > > 2.6.29).  However, the memcpy is the wrong way around for us.  If you
> > > > look at an example, the original code said
> > > 
> > > The old tg3_nvram_read() had a swab32() after the readl().  The new
> > > tg3_nvram_read() no longer has the swab32().  There were too many layers
> > > of swapping in the old code and that's why Matt wanted to clean it up.
> > > 
> > > James, can do dump out the nvram content on the parisc?
> > > 
> > > ethtool -e eth0 length 0x90
> > > 
> > > Thanks.
> > 
> > Sure, ion's is
> > 
> > ion:~# ethtool -e eth0 length 0x90
> > Address         Data
> > ----------      ----
> > 0x00000000      0xaa
> > 0x00000001      0x55
> > 0x00000002      0x99
> > 0x00000003      0x66
> 
> Michael noticed that your NVRAM signature is byteswapped in NVRAM. (!)
> That also explains why the driver is trying to obtain the MAC address
> through NVRAM, rather than getting it from shared memory.  The device's
> bootcode is not working correctly.

Um, well, this is a parisc:  the device's boot code won't be working at
all (parisc doesn't have open firmware boot).  The values might be laid
down by the platform IODC, but usually for add in cards, they're the
default initialise values the card comes up with.

James

Re: [PATCH] tg3: fix big endian MAC address collection failure

[Michael Chan]

David Miller wrote:

> From: "Matt Carlson" <mcarlson@broadcom.com>
> Date: Mon, 13 Apr 2009 19:00:15 -0700
>
> > I did run some tests this afternoon on a similar IA64 HP machine and
> > they ran fine.
>
> It can't be similar, James Bottomly is on HP PARISC (big endian) not
> IA64 (which is little endian).

Matt brought up IA64 because Robin Holt reported what seemed like an
identical issue on SGI IA64 machine.

>
> I'm still convinced this is a big-endian driver problem and it has
> nothing to do with broken NVRAM or similar.
>
>

Agreed.  James' problem appears to be a big endian issue.  Everything
is endian-swapped in his NVRAM when he did the ethtool dump.

Re: [PATCH] tg3: fix big endian MAC address collection failure

[Michael Chan]

James Bottomley wrote:

> On Mon, 2009-04-13 at 18:25 -0700, Matt Carlson wrote:
> > On Mon, Apr 13, 2009 at 03:32:14PM -0700, James Bottomley wrote:
> > >
> > > ion:~# ethtool -e eth0 length 0x90
> > > Address         Data
> > > ----------      ----
> > > 0x00000000      0xaa
> > > 0x00000001      0x55
> > > 0x00000002      0x99
> > > 0x00000003      0x66
> >
> > Michael noticed that your NVRAM signature is byteswapped in
> NVRAM. (!)
> > That also explains why the driver is trying to obtain the
> MAC address
> > through NVRAM, rather than getting it from shared memory.
> The device's
> > bootcode is not working correctly.
>
> Um, well, this is a parisc:  the device's boot code won't be
> working at
> all (parisc doesn't have open firmware boot).  The values
> might be laid
> down by the platform IODC, but usually for add in cards, they're the
> default initialise values the card comes up with.
>

Matt was referring to MIPS code that runs inside the MIPS core inside
the TG3 chip.  The code is loaded from NVRAM and will start running
after chip reset.  But I don't agree with Matt that the swapped NVRAM
values were caused by bad MIPS code programmed in the NVRAM.

I agree with DaveM that this appears to be a driver big-endian problem
when reading the NVRAM.  Can you run the same ethtool -e on 2.6.29 to
confirm?  Thanks.

Re: [PATCH] tg3: fix big endian MAC address collection failure

[James Bottomley]

On Mon, 2009-04-13 at 21:11 -0700, Michael Chan wrote:
> James Bottomley wrote:
> 
> > On Mon, 2009-04-13 at 18:25 -0700, Matt Carlson wrote:
> > > On Mon, Apr 13, 2009 at 03:32:14PM -0700, James Bottomley wrote:
> > > >
> > > > ion:~# ethtool -e eth0 length 0x90
> > > > Address         Data
> > > > ----------      ----
> > > > 0x00000000      0xaa
> > > > 0x00000001      0x55
> > > > 0x00000002      0x99
> > > > 0x00000003      0x66
> > >
> > > Michael noticed that your NVRAM signature is byteswapped in
> > NVRAM. (!)
> > > That also explains why the driver is trying to obtain the
> > MAC address
> > > through NVRAM, rather than getting it from shared memory.
> > The device's
> > > bootcode is not working correctly.
> >
> > Um, well, this is a parisc:  the device's boot code won't be
> > working at
> > all (parisc doesn't have open firmware boot).  The values
> > might be laid
> > down by the platform IODC, but usually for add in cards, they're the
> > default initialise values the card comes up with.
> >
> 
> Matt was referring to MIPS code that runs inside the MIPS core inside
> the TG3 chip.  The code is loaded from NVRAM and will start running
> after chip reset.  But I don't agree with Matt that the swapped NVRAM
> values were caused by bad MIPS code programmed in the NVRAM.

Um, yes, me too ... if the boot process doesn't involve the platform,
there's no reason the values would get swapped (unless we're accessing
them wrongly).

> I agree with DaveM that this appears to be a driver big-endian problem
> when reading the NVRAM.  Can you run the same ethtool -e on 2.6.29 to
> confirm?  Thanks.

Sure (just had to rebuild a 2.6.29 kernel):

ion:~# ethtool -e eth0 length 0x90
Address         Data
----------      ----
0x00000000      0x66
0x00000001      0x99
0x00000002      0x55
0x00000003      0xaa
0x00000004      0x08
0x00000005      0x00
0x00000006      0x00
0x00000007      0x00
0x00000008      0x00
0x00000009      0x00
0x0000000a      0x02
0x0000000b      0x29
0x0000000c      0x00
0x0000000d      0x00
0x0000000e      0x02
0x0000000f      0x00
0x00000010      0x9b
0x00000011      0x82
0x00000012      0x3c
0x00000013      0xc5
0x00000014      0x00
0x00000015      0x00
0x00000016      0x00
0x00000017      0x00
0x00000018      0x00
0x00000019      0x00
0x0000001a      0x00
0x0000001b      0x00
0x0000001c      0x00
0x0000001d      0x00
0x0000001e      0x00
0x0000001f      0x00
0x00000020      0x00
0x00000021      0x00
0x00000022      0x00
0x00000023      0x00
0x00000024      0x00
0x00000025      0x00
0x00000026      0x00
0x00000027      0x00
0x00000028      0x00
0x00000029      0x00
0x0000002a      0x00
0x0000002b      0x00
0x0000002c      0x00
0x0000002d      0x00
0x0000002e      0x00
0x0000002f      0x00
0x00000030      0x00
0x00000031      0x00
0x00000032      0x00
0x00000033      0x00
0x00000034      0x00
0x00000035      0x00
0x00000036      0x00
0x00000037      0x00
0x00000038      0x00
0x00000039      0x00
0x0000003a      0x00
0x0000003b      0x00
0x0000003c      0x00
0x0000003d      0x00
0x0000003e      0x00
0x0000003f      0x00
0x00000040      0x00
0x00000041      0x00
0x00000042      0x00
0x00000043      0x00
0x00000044      0x00
0x00000045      0x00
0x00000046      0x00
0x00000047      0x00
0x00000048      0x00
0x00000049      0x00
0x0000004a      0x00
0x0000004b      0x00
0x0000004c      0x00
0x0000004d      0x00
0x0000004e      0x00
0x0000004f      0x00
0x00000050      0x00
0x00000051      0x00
0x00000052      0x00
0x00000053      0x00
0x00000054      0x00
0x00000055      0x00
0x00000056      0x00
0x00000057      0x00
0x00000058      0x00
0x00000059      0x00
0x0000005a      0x00
0x0000005b      0x00
0x0000005c      0x00
0x0000005d      0x00
0x0000005e      0x00
0x0000005f      0x00
0x00000060      0x00
0x00000061      0x00
0x00000062      0x00
0x00000063      0x00
0x00000064      0x00
0x00000065      0x00
0x00000066      0x00
0x00000067      0x00
0x00000068      0x00
0x00000069      0x00
0x0000006a      0x00
0x0000006b      0x00
0x0000006c      0x00
0x0000006d      0x00
0x0000006e      0x00
0x0000006f      0x00
0x00000070      0x00
0x00000071      0x00
0x00000072      0x00
0x00000073      0x00
0x00000074      0x43
0x00000075      0x00
0x00000076      0x00
0x00000077      0x8c
0x00000078      0x00
0x00000079      0x20
0x0000007a      0x61
0x0000007b      0x10
0x0000007c      0x00
0x0000007d      0x00
0x0000007e      0x00
0x0000007f      0x30
0x00000080      0x6e
0x00000081      0x4b
0x00000082      0x15
0x00000083      0x59
0x00000084      0x42
0x00000085      0x43
0x00000086      0x4d
0x00000087      0x39
0x00000088      0x35
0x00000089      0x37
0x0000008a      0x30
0x0000008b      0x30
0x0000008c      0x41
0x0000008d      0x36
0x0000008e      0x00
0x0000008f      0x00

So that's byteswapped from the 2.6.30 output ... confirming your theory,
I think.

James

Re: [PATCH] tg3: fix big endian MAC address collection failure

[Robin Holt]

Please don't let me distract from this conversation, but here are the
ethtool dumps on ia64 for the same BCM5701 adapter for both 2.6.30-rc1
and 2.6.27.y

When I dump BCM5704 adapters, the first four bytes match.

2.6.27.19
Address   	Data
----------	----
0x00000000	0x66
0x00000001	0x99
0x00000002	0x55
0x00000003	0xaa
0x00000004	0x08
0x00000005	0x00
0x00000006	0x00
0x00000007	0x00
0x00000008	0x00
0x00000009	0x00
0x0000000a	0x02
0x0000000b	0xa5
0x0000000c	0x00
0x0000000d	0x00
0x0000000e	0x02
0x0000000f	0x00
0x00000010	0x08
0x00000011	0xdf
0x00000012	0x2a
0x00000013	0xb1
0x00000014	0x00
0x00000015	0x01
0x00000016	0x00
0x00000017	0x00
0x00000018	0x00
0x00000019	0x00
0x0000001a	0x3c
0x0000001b	0x00
0x0000001c	0x00
0x0000001d	0x00
0x0000001e	0x10
0x0000001f	0x00
0x00000020	0x00
0x00000021	0x00
0x00000022	0x00
0x00000023	0x00
0x00000024	0x00
0x00000025	0x00
0x00000026	0x00
0x00000027	0x00
0x00000028	0x00
0x00000029	0x00
0x0000002a	0x00
0x0000002b	0x00
0x0000002c	0x00
0x0000002d	0x00
0x0000002e	0x00
0x0000002f	0x00
0x00000030	0x00
0x00000031	0x00
0x00000032	0x00
0x00000033	0x00
0x00000034	0x00
0x00000035	0x00
0x00000036	0x00
0x00000037	0x00
0x00000038	0x00
0x00000039	0x00
0x0000003a	0x00
0x0000003b	0x00
0x0000003c	0x00
0x0000003d	0x00
0x0000003e	0x00
0x0000003f	0x00
0x00000040	0x00
0x00000041	0x00
0x00000042	0x00
0x00000043	0x00
0x00000044	0x00
0x00000045	0x00
0x00000046	0x00
0x00000047	0x00
0x00000048	0x00
0x00000049	0x00
0x0000004a	0x00
0x0000004b	0x00
0x0000004c	0x00
0x0000004d	0x00
0x0000004e	0x00
0x0000004f	0x00
0x00000050	0x00
0x00000051	0x00
0x00000052	0x00
0x00000053	0x00
0x00000054	0x00
0x00000055	0x00
0x00000056	0x00
0x00000057	0x00
0x00000058	0x00
0x00000059	0x00
0x0000005a	0x00
0x0000005b	0x00
0x0000005c	0x00
0x0000005d	0x00
0x0000005e	0x00
0x0000005f	0x00
0x00000060	0x00
0x00000061	0x00
0x00000062	0x00
0x00000063	0x00
0x00000064	0x00
0x00000065	0x00
0x00000066	0x00
0x00000067	0x00
0x00000068	0x00
0x00000069	0x00
0x0000006a	0x00
0x0000006b	0x00
0x0000006c	0x00
0x0000006d	0x00
0x0000006e	0x00
0x0000006f	0x00
0x00000070	0x00
0x00000071	0x00
0x00000072	0x00
0x00000073	0x00
0x00000074	0x43
0x00000075	0x00
0x00000076	0x00
0x00000077	0x8c
0x00000078	0x00
0x00000079	0x00
0x0000007a	0x00
0x0000007b	0x01
0x0000007c	0x00
0x0000007d	0x00
0x0000007e	0x08
0x0000007f	0x00
0x00000080	0x69
0x00000081	0x13
0x00000082	0xe6
0x00000083	0x3c
0x00000084	0x33
0x00000085	0x43
0x00000086	0x39
0x00000087	0x39
0x00000088	0x36
0x00000089	0x42
0x0000008a	0x2d
0x0000008b	0x54
0x0000008c	0x00
0x0000008d	0x00
0x0000008e	0x00
0x0000008f	0x00


2.6.30-rc1
Address   	Data
----------	----
0x00000000	0xaa
0x00000001	0x55
0x00000002	0x99
0x00000003	0x66
0x00000004	0x00
0x00000005	0x00
0x00000006	0x00
0x00000007	0x08
0x00000008	0xa5
0x00000009	0x02
0x0000000a	0x00
0x0000000b	0x00
0x0000000c	0x00
0x0000000d	0x02
0x0000000e	0x00
0x0000000f	0x00
0x00000010	0xb1
0x00000011	0x2a
0x00000012	0xdf
0x00000013	0x08
0x00000014	0x00
0x00000015	0x00
0x00000016	0x01
0x00000017	0x00
0x00000018	0x00
0x00000019	0x3c
0x0000001a	0x00
0x0000001b	0x00
0x0000001c	0x00
0x0000001d	0x10
0x0000001e	0x00
0x0000001f	0x00
0x00000020	0x00
0x00000021	0x00
0x00000022	0x00
0x00000023	0x00
0x00000024	0x00
0x00000025	0x00
0x00000026	0x00
0x00000027	0x00
0x00000028	0x00
0x00000029	0x00
0x0000002a	0x00
0x0000002b	0x00
0x0000002c	0x00
0x0000002d	0x00
0x0000002e	0x00
0x0000002f	0x00
0x00000030	0x00
0x00000031	0x00
0x00000032	0x00
0x00000033	0x00
0x00000034	0x00
0x00000035	0x00
0x00000036	0x00
0x00000037	0x00
0x00000038	0x00
0x00000039	0x00
0x0000003a	0x00
0x0000003b	0x00
0x0000003c	0x00
0x0000003d	0x00
0x0000003e	0x00
0x0000003f	0x00
0x00000040	0x00
0x00000041	0x00
0x00000042	0x00
0x00000043	0x00
0x00000044	0x00
0x00000045	0x00
0x00000046	0x00
0x00000047	0x00
0x00000048	0x00
0x00000049	0x00
0x0000004a	0x00
0x0000004b	0x00
0x0000004c	0x00
0x0000004d	0x00
0x0000004e	0x00
0x0000004f	0x00
0x00000050	0x00
0x00000051	0x00
0x00000052	0x00
0x00000053	0x00
0x00000054	0x00
0x00000055	0x00
0x00000056	0x00
0x00000057	0x00
0x00000058	0x00
0x00000059	0x00
0x0000005a	0x00
0x0000005b	0x00
0x0000005c	0x00
0x0000005d	0x00
0x0000005e	0x00
0x0000005f	0x00
0x00000060	0x00
0x00000061	0x00
0x00000062	0x00
0x00000063	0x00
0x00000064	0x00
0x00000065	0x00
0x00000066	0x00
0x00000067	0x00
0x00000068	0x00
0x00000069	0x00
0x0000006a	0x00
0x0000006b	0x00
0x0000006c	0x00
0x0000006d	0x00
0x0000006e	0x00
0x0000006f	0x00
0x00000070	0x00
0x00000071	0x00
0x00000072	0x00
0x00000073	0x00
0x00000074	0x8c
0x00000075	0x00
0x00000076	0x00
0x00000077	0x43
0x00000078	0x01
0x00000079	0x00
0x0000007a	0x00
0x0000007b	0x00
0x0000007c	0x00
0x0000007d	0x08
0x0000007e	0x00
0x0000007f	0x00
0x00000080	0x3c
0x00000081	0xe6
0x00000082	0x13
0x00000083	0x69
0x00000084	0x39
0x00000085	0x39
0x00000086	0x43
0x00000087	0x33
0x00000088	0x54
0x00000089	0x2d
0x0000008a	0x42
0x0000008b	0x36
0x0000008c	0x00
0x0000008d	0x00
0x0000008e	0x00
0x0000008f	0x00