mxs-i2c: Fix internal address byte order

PATCH New Torsten Fleischer 3 participants 6 messages Apr 17, 2012 – May 9, 2012

Subsystems (2)

Arm i.MX and Layerscape I2C

lore

Share & tools

lore

Torsten Fleischer

author

2012-04-17 15:37 UTC #0 lore raw

PATCH 1/1 mxs-i2c: Fix internal address byte order

Large EEPROMs, e.g. 24lc32, need 2 byte to address the internal memory. These devices require that the high byte of the internal address has to be written first.
The mxs_i2c driver currently writes the address' low byte first.

The following patch fixes the byte order of the internal address that should be written to the I2C device.

Signed-off-by Torsten Fleischer <[email protected]>

CC Marek Vasut <[email protected]>

CC Stefano Babic <[email protected]>

CC Fabio Estevam <[email protected]>

---
 drivers/i2c/mxs_i2c.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/i2c/mxs_i2c.c +1 -1

index c8fea32..48aaaa6 100644
@@ -97,7 +97,7 @@ void mxs_i2c_write(uchar chip, uint addr, int alen,
 
 	for (i = 0; i < alen; i++) {
 		data >>= 8;
-		data |= ((char *)&addr)[i] << 24;
+		data |= ((char *)&addr)[alen - i - 1] << 24;
 		if ((i & 3) == 2)
 			writel(data, &i2c_regs->hw_i2c_data);
 	}

Signature · 1 line

1.7.7

Marek Vasut

2012-04-17 20:30 UTC #1 lore raw

PATCH 1/1 mxs-i2c: Fix internal address byte order

Dear Torsten Fleischer,

Quoted 9 lines

Large EEPROMs, e.g. 24lc32, need 2 byte to address the internal memory. These devices require that the high byte of the internal address has to be written first.
The mxs_i2c driver currently writes the address' low byte first.
The following patch fixes the byte order of the internal address that should be written to the I2C device.
Signed-off-by Torsten Fleischer <[email protected]>

Acked-by Marek Vasut <[email protected]>

Quoted 13 lines

CC Marek Vasut <[email protected]>
CC Stefano Babic <[email protected]>
CC Fabio Estevam <[email protected]>
---
 drivers/i2c/mxs_i2c.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/mxs_i2c.c +0 -0
index c8fea32..48aaaa6 100644

drivers/i2c/mxs_i2c.c void mxs_i2c_write(uchar chip, uint addr, int alen, · line 103

Quoted 8 lines

drivers/i2c/mxs_i2c.c +1 -1

@@ -97,7 +97,7 @@ void mxs_i2c_write(uchar chip, uint addr, int alen,
 
 	for (i = 0; i < alen; i++) {
 		data >>= 8;
-		data |= ((char *)&addr)[i] << 24;
+		data |= ((char *)&addr)[alen - i - 1] << 24;
 		if ((i & 3) == 2)
 			writel(data, &i2c_regs->hw_i2c_data);
 	}

Best regards,
Marek Vasut

Marek Vasut

2012-05-01 02:45 UTC #2 lore raw

PATCH 1/1 mxs-i2c: Fix internal address byte order

In reply to Torsten Fleischer

Dear Torsten Fleischer,

Quoted 4 lines

Large EEPROMs, e.g. 24lc32, need 2 byte to address the internal memory. These devices require that the high byte of the internal address has to be written first.
The mxs_i2c driver currently writes the address' low byte first.

Are you sure about it? Are you sure what you're seeing isn't FIFO overrun? Basically, how does your problem manifest? How did you test this? We've been having similar issues in Linux recently.

Quoted 18 lines

The following patch fixes the byte order of the internal address that should be written to the I2C device.
Signed-off-by Torsten Fleischer <[email protected]>
CC Marek Vasut <[email protected]>
CC Stefano Babic <[email protected]>
CC Fabio Estevam <[email protected]>
---
 drivers/i2c/mxs_i2c.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/mxs_i2c.c +0 -0
index c8fea32..48aaaa6 100644

drivers/i2c/mxs_i2c.c void mxs_i2c_write(uchar chip, uint addr, int alen, · line 103

Quoted 8 lines

drivers/i2c/mxs_i2c.c +1 -1

@@ -97,7 +97,7 @@ void mxs_i2c_write(uchar chip, uint addr, int alen,
 
 	for (i = 0; i < alen; i++) {
 		data >>= 8;
-		data |= ((char *)&addr)[i] << 24;
+		data |= ((char *)&addr)[alen - i - 1] << 24;
 		if ((i & 3) == 2)
 			writel(data, &i2c_regs->hw_i2c_data);
 	}

Best regards,
Marek Vasut

Torsten Fleischer

author

2012-05-07 14:59 UTC #3 lore raw

PATCH 1/1 mxs-i2c: Fix internal address byte order

Dear Marek Vasut,

Quoted 6 lines

Quoted 4 lines
Large EEPROMs, e.g. 24lc32, need 2 byte to address the internal memory. These devices require that the high byte of the internal address has to be written first.
The mxs_i2c driver currently writes the address' low byte first.
Are you sure about it? Are you sure what you're seeing isn't FIFO overrun?

Yes, because I used 'i2c md' and 'i2c mw' for the test and these commands do only a small write transfer. They write 3 and 4 bytes respectively. This is in both cases only one FIFO word.

Quoted 1 line

Basically, how does your problem manifest? How did you test this?

First I wrote different bytes into the addresses 0x00 and 0x01 of the EEPROM using 'i2c mw', for example:

=> i2c mw 54 0.2 12
=> i2c mw 54 1.2 34

Then I read the address range 0x00 - 0x01 and got:

=> i2c md 54 0.2 2
0000: 12 ff ..

That's not what I had expected. On address 0x01 there should be '34' instead of 'ff'.
But when I read directly from the address 0x01 the command returned the correct value:

=> i2c md 54 1.2 1
0001: 34 4

Thereupon I checked the I2C frame of the read and write commands to the address 0x01 with an oscilloscope. The scope showed that after the device's address the bytes 0x01 followed by 0x00 were sent to the EEPROM. But the two bytes must be in reverse order.

After inspecting the source code I swapped the address bytes, i.e. I used '100.2' instead of '1.2'. With it the read/write command accessed the address 0x01 of the EEPROM. I verified this with the scope.

Reading the range 0x00 - 0x01 returned then the expected content:

=> i2c md 54 0.2 2
0000: 12 34 .4

I also tested the address byte swapping - before and after modifying the driver - with other addresses (e.g. 0x0180 and 0x0fff) by checking the I2C frames with the scope.
After fixing the driver, I additionally read the whole EEPROM using 'i2c md' and verified the written addresses.

Best Regards
Torsten Fleischer

Marek Vasut

2012-05-07 16:13 UTC #4 lore raw

PATCH 1/1 mxs-i2c: Fix internal address byte order

Dear Torsten Fleischer,

Quoted 46 lines

Dear Marek Vasut,
Quoted 7 lines
Quoted 4 lines
Large EEPROMs, e.g. 24lc32, need 2 byte to address the internal memory. These devices require that the high byte of the internal address has to be written first.
The mxs_i2c driver currently writes the address' low byte first.
Are you sure about it? Are you sure what you're seeing isn't FIFO overrun?
Yes, because I used 'i2c md' and 'i2c mw' for the test and these commands do only a small write transfer. They write 3 and 4 bytes respectively. This is in both cases only one FIFO word.
Quoted 1 line
Basically, how does your problem manifest? How did you test this?
First I wrote different bytes into the addresses 0x00 and 0x01 of the EEPROM using 'i2c mw', for example:
=> i2c mw 54 0.2 12
=> i2c mw 54 1.2 34
Then I read the address range 0x00 - 0x01 and got:
=> i2c md 54 0.2 2
0000: 12 ff ..
That's not what I had expected. On address 0x01 there should be '34' instead of 'ff'.
But when I read directly from the address 0x01 the command returned the correct value:
=> i2c md 54 1.2 1
0001: 34 4
Thereupon I checked the I2C frame of the read and write commands to the address 0x01 with an oscilloscope. The scope showed that after the device's address the bytes 0x01 followed by 0x00 were sent to the EEPROM. But the two bytes must be in reverse order.
After inspecting the source code I swapped the address bytes, i.e. I used '100.2' instead of '1.2'. With it the read/write command accessed the address 0x01 of the EEPROM. I verified this with the scope.
Reading the range 0x00 - 0x01 returned then the expected content:

Quoted 8 lines

=> i2c md 54 0.2 2
0000: 12 34 .4
I also tested the address byte swapping - before and after modifying the driver - with other addresses (e.g. 0x0180 and 0x0fff) by checking the I2C frames with the scope.
After fixing the driver, I additionally read the whole EEPROM using 'i2c md' and verified the written addresses.

Oh this is very nice! Thanks a lot!

Obviously:

Acked-by Marek Vasut <[email protected]>

Quoted 2 lines

Best Regards
Torsten Fleischer

Best regards,
Marek Vasut

Stefano Babic

2012-05-09 09:41 UTC #5 lore raw

PATCH 1/1 mxs-i2c: Fix internal address byte order

In reply to Torsten Fleischer

Quoted 22 lines · Torsten Fleischer

Large EEPROMs, e.g. 24lc32, need 2 byte to address the internal memory. These devices require that the high byte of the internal address has to be written first.
The mxs_i2c driver currently writes the address' low byte first.
The following patch fixes the byte order of the internal address that should be written to the I2C device.
Signed-off-by Torsten Fleischer <[email protected]>
CC Marek Vasut <[email protected]>
CC Stefano Babic <[email protected]>
CC Fabio Estevam <[email protected]>
---
 drivers/i2c/mxs_i2c.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/i2c/mxs_i2c.c +0 -0
index c8fea32..48aaaa6 100644

drivers/i2c/mxs_i2c.c void mxs_i2c_write(uchar chip, uint addr, int alen, · line 103

Quoted 8 lines

drivers/i2c/mxs_i2c.c +1 -1

@@ -97,7 +97,7 @@ void mxs_i2c_write(uchar chip, uint addr, int alen,
 
 	for (i = 0; i < alen; i++) {
 		data >>= 8;
-		data |= ((char *)&addr)[i] << 24;
+		data |= ((char *)&addr)[alen - i - 1] << 24;
 		if ((i & 3) == 2)
 			writel(data, &i2c_regs->hw_i2c_data);
 	}

I forwarded your patch to Heiko (I2C custodian), but here my:

Acked-by Stefano Babic <[email protected]>

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================