* [U-Boot] [PATCH] mxs-i2c: Fix internal address byte order
2012-04-17 15:37 [U-Boot] [PATCH] mxs-i2c: Fix internal address byte order Torsten Fleischer
@ 2012-04-17 20:30 ` Marek Vasut
2012-05-01 2:45 ` Marek Vasut
2012-05-09 9:41 ` Stefano Babic
2 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2012-04-17 20:30 UTC (permalink / raw)
To: u-boot
Dear 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 <to-fleischer@t-online.de>
Acked-by: Marek Vasut <marex@denx.de>
>
> CC: Marek Vasut <marex@denx.de>
> CC: Stefano Babic <sbabic@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/i2c/mxs_i2c.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c
> index c8fea32..48aaaa6 100644
> --- a/drivers/i2c/mxs_i2c.c
> +++ b/drivers/i2c/mxs_i2c.c
> @@ -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
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH] mxs-i2c: Fix internal address byte order
2012-04-17 15:37 [U-Boot] [PATCH] mxs-i2c: Fix internal address byte order Torsten Fleischer
2012-04-17 20:30 ` Marek Vasut
@ 2012-05-01 2:45 ` Marek Vasut
2012-05-07 14:59 ` Torsten Fleischer
2012-05-09 9:41 ` Stefano Babic
2 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2012-05-01 2:45 UTC (permalink / raw)
To: u-boot
Dear 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.
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.
>
> The following patch fixes the byte order of the internal address that
> should be written to the I2C device.
>
> Signed-off-by: Torsten Fleischer <to-fleischer@t-online.de>
>
> CC: Marek Vasut <marex@denx.de>
> CC: Stefano Babic <sbabic@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/i2c/mxs_i2c.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c
> index c8fea32..48aaaa6 100644
> --- a/drivers/i2c/mxs_i2c.c
> +++ b/drivers/i2c/mxs_i2c.c
> @@ -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
^ permalink raw reply [flat|nested] 6+ messages in thread* [U-Boot] [PATCH] mxs-i2c: Fix internal address byte order
2012-05-01 2:45 ` Marek Vasut
@ 2012-05-07 14:59 ` Torsten Fleischer
2012-05-07 16:13 ` Marek Vasut
0 siblings, 1 reply; 6+ messages in thread
From: Torsten Fleischer @ 2012-05-07 14:59 UTC (permalink / raw)
To: u-boot
Dear Marek Vasut,
> > 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.
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mxs-i2c: Fix internal address byte order
2012-05-07 14:59 ` Torsten Fleischer
@ 2012-05-07 16:13 ` Marek Vasut
0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2012-05-07 16:13 UTC (permalink / raw)
To: u-boot
Dear Torsten Fleischer,
> Dear Marek Vasut,
>
> > > 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.
>
> > 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.
Oh this is very nice! Thanks a lot!
Obviously:
Acked-by: Marek Vasut <marex@denx.de>
> Best Regards
> Torsten Fleischer
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] mxs-i2c: Fix internal address byte order
2012-04-17 15:37 [U-Boot] [PATCH] mxs-i2c: Fix internal address byte order Torsten Fleischer
2012-04-17 20:30 ` Marek Vasut
2012-05-01 2:45 ` Marek Vasut
@ 2012-05-09 9:41 ` Stefano Babic
2 siblings, 0 replies; 6+ messages in thread
From: Stefano Babic @ 2012-05-09 9:41 UTC (permalink / raw)
To: u-boot
On 17/04/2012 17:37, Torsten Fleischer wrote:
> 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 <to-fleischer@t-online.de>
>
> CC: Marek Vasut <marex@denx.de>
> CC: Stefano Babic <sbabic@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/i2c/mxs_i2c.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/mxs_i2c.c b/drivers/i2c/mxs_i2c.c
> index c8fea32..48aaaa6 100644
> --- a/drivers/i2c/mxs_i2c.c
> +++ b/drivers/i2c/mxs_i2c.c
> @@ -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 <sbabic@denx.de>
Best regards,
Stefano Babic
--
=====================================================================
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
=====================================================================
^ permalink raw reply [flat|nested] 6+ messages in thread