Re: [PATCH] b44: fix eeprom endianess issue
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. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] b44: fix eeprom endianess issue
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 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] b44: fix eeprom endianess issue
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 <[EMAIL PROTECTED]> > > Index: linux-2.6/drivers/net/b44.c > === > --- linux-2.6.orig/drivers/net/b44.c 2006-08-22 11:27:56.0 +0200 > +++ linux-2.6/drivers/net/b44.c 2006-08-23 12:26:31.0 +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. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] b44: fix eeprom endianess issue
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 <[EMAIL PROTECTED]> Index: linux-2.6/drivers/net/b44.c === --- linux-2.6.orig/drivers/net/b44.c2006-08-22 11:27:56.0 +0200 +++ linux-2.6/drivers/net/b44.c 2006-08-23 12:26:31.0 +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. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
