[U-Boot] [PATCH] arm: omap3: i2c: don't zero cnt in i2c_write

[U-Boot] [PATCH] arm: omap3: i2c: don't zero cnt in i2c_write

[Nikita Kiryanov]

The zeroing of I2Ci.I2C_CNT register at the end of i2c_write causes
random I2C failures in OMAP3 based devices. This is probably related
to the following advisory in OMAP3 errata:

I2C Module Does Not Allow 0-Byte Data Requests
Details:
When configured as the master, the I2C module does not allow 0-byte data
transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
undefined behavior.
Workaround(s):
No workaround. Do not use 0-byte data requests.

Make the write conditional so that only non-OMAP3 devices will perform it.

Cc: Heiko Schocher <hs@denx.de>
Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
---
 drivers/i2c/omap24xx_i2c.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
index 3d38c03..2862a8f 100644
--- a/drivers/i2c/omap24xx_i2c.c
+++ b/drivers/i2c/omap24xx_i2c.c
@@ -473,7 +473,10 @@ static int omap24_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 wr_exit:
 	flush_fifo(adap);
 	writew(0xFFFF, &i2c_base->stat);
+	/* This write is known to cause undefined behavior on OMAP3 */
+#ifndef CONFIG_OMAP34XX
 	writew(0, &i2c_base->cnt);
+#endif
 	return i2c_error;
 }
 
-- 
1.8.1.2

[U-Boot] [PATCH] arm: omap3: i2c: don't zero cnt in i2c_write

[Tom Rini]

On Wed, Nov 27, 2013 at 04:44:09PM +0200, Nikita Kiryanov wrote:
> The zeroing of I2Ci.I2C_CNT register at the end of i2c_write causes
> random I2C failures in OMAP3 based devices. This is probably related
> to the following advisory in OMAP3 errata:
> 
> I2C Module Does Not Allow 0-Byte Data Requests
> Details:
> When configured as the master, the I2C module does not allow 0-byte data
> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
> undefined behavior.
> Workaround(s):
> No workaround. Do not use 0-byte data requests.
> 
> Make the write conditional so that only non-OMAP3 devices will perform it.
> 
> Cc: Heiko Schocher <hs@denx.de>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> ---
>  drivers/i2c/omap24xx_i2c.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
> index 3d38c03..2862a8f 100644
> --- a/drivers/i2c/omap24xx_i2c.c
> +++ b/drivers/i2c/omap24xx_i2c.c
> @@ -473,7 +473,10 @@ static int omap24_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
>  wr_exit:
>  	flush_fifo(adap);
>  	writew(0xFFFF, &i2c_base->stat);
> +	/* This write is known to cause undefined behavior on OMAP3 */
> +#ifndef CONFIG_OMAP34XX
>  	writew(0, &i2c_base->cnt);
> +#endif
>  	return i2c_error;
>  }

Yay for tracking this down.  Can you please expand the comment to
mention the specific advisory?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20131127/8a074c06/attachment.pgp>

[U-Boot] [PATCH] arm: omap3: i2c: don't zero cnt in i2c_write

[Nikita Kiryanov]

On 11/27/2013 05:09 PM, Tom Rini wrote:
> On Wed, Nov 27, 2013 at 04:44:09PM +0200, Nikita Kiryanov wrote:
>> The zeroing of I2Ci.I2C_CNT register at the end of i2c_write causes
>> random I2C failures in OMAP3 based devices. This is probably related
>> to the following advisory in OMAP3 errata:
>>
>> I2C Module Does Not Allow 0-Byte Data Requests
>> Details:
>> When configured as the master, the I2C module does not allow 0-byte data
>> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
>> undefined behavior.
>> Workaround(s):
>> No workaround. Do not use 0-byte data requests.
>>
>> Make the write conditional so that only non-OMAP3 devices will perform it.
>>
>> Cc: Heiko Schocher <hs@denx.de>
>> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
>> ---
>>   drivers/i2c/omap24xx_i2c.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
>> index 3d38c03..2862a8f 100644
>> --- a/drivers/i2c/omap24xx_i2c.c
>> +++ b/drivers/i2c/omap24xx_i2c.c
>> @@ -473,7 +473,10 @@ static int omap24_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
>>   wr_exit:
>>   	flush_fifo(adap);
>>   	writew(0xFFFF, &i2c_base->stat);
>> +	/* This write is known to cause undefined behavior on OMAP3 */
>> +#ifndef CONFIG_OMAP34XX
>>   	writew(0, &i2c_base->cnt);
>> +#endif
>>   	return i2c_error;
>>   }
>
> Yay for tracking this down.  Can you please expand the comment to
> mention the specific advisory?  Thanks!
>

Sure, but I'm going to let the conversation we have about this issue in
another thread play out a little first.

-- 
Regards,
Nikita.

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Nikita Kiryanov]

Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
based devices. This seems to be related to the following advisory which
apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
OMAP4430 TRM:

Advisory:
I2C Module Does Not Allow 0-Byte Data Requests
Details:
When configured as the master, the I2C module does not allow 0-byte data
transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
undefined behavior.
Workaround(s):
No workaround. Do not use 0-byte data requests.

The writes in question are unnecessary from a functional point of view.
Most of them are done after I/O has finished, and the only one that preceds
I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
actual data transmission takes place.

Therefore, remove all writes that zero the cnt register.

Cc: Heiko Schocher <hs@denx.de>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Tom Rini <trini@ti.com>
Cc: Lubomir Popov <lpopov@mm-sol.com>
Cc: Enric Balletbo Serra <eballetbo@gmail.com>
Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
---
Changes in V2:	
	Removed all instances of writew(0, &i2c_base->cnt) instead of just the
	one in i2c_write (following a test of V1 by Thomas Petazzoni).

 drivers/i2c/omap24xx_i2c.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
index 3d38c03..c784004 100644
--- a/drivers/i2c/omap24xx_i2c.c
+++ b/drivers/i2c/omap24xx_i2c.c
@@ -158,7 +158,6 @@ static void omap24_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
 	udelay(1000);
 	flush_fifo(adap);
 	writew(0xFFFF, &i2c_base->stat);
-	writew(0, &i2c_base->cnt);
 }
 
 static void flush_fifo(struct i2c_adapter *adap)
@@ -198,8 +197,6 @@ static int omap24_i2c_probe(struct i2c_adapter *adap, uchar chip)
 		return res;
 
 	/* No data transfer, slave addr only */
-	writew(0, &i2c_base->cnt);
-	/* Set slave address */
 	writew(chip, &i2c_base->sa);
 	/* Stop bit needed here */
 	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |
@@ -234,7 +231,6 @@ static int omap24_i2c_probe(struct i2c_adapter *adap, uchar chip)
 pr_exit:
 	flush_fifo(adap);
 	writew(0xFFFF, &i2c_base->stat);
-	writew(0, &i2c_base->cnt);
 	return res;
 }
 
@@ -372,7 +368,6 @@ static int omap24_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
 rd_exit:
 	flush_fifo(adap);
 	writew(0xFFFF, &i2c_base->stat);
-	writew(0, &i2c_base->cnt);
 	return i2c_error;
 }
 
@@ -473,7 +468,6 @@ static int omap24_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
 wr_exit:
 	flush_fifo(adap);
 	writew(0xFFFF, &i2c_base->stat);
-	writew(0, &i2c_base->cnt);
 	return i2c_error;
 }
 
-- 
1.8.1.2

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Enric Balletbo Serra]

Hi Nikita,

2013/11/28 Nikita Kiryanov <nikita@compulab.co.il>:
> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
> based devices. This seems to be related to the following advisory which
> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
> OMAP4430 TRM:
>
> Advisory:
> I2C Module Does Not Allow 0-Byte Data Requests
> Details:
> When configured as the master, the I2C module does not allow 0-byte data
> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
> undefined behavior.
> Workaround(s):
> No workaround. Do not use 0-byte data requests.
>
> The writes in question are unnecessary from a functional point of view.
> Most of them are done after I/O has finished, and the only one that preceds
> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
> actual data transmission takes place.
>
> Therefore, remove all writes that zero the cnt register.
>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Lubomir Popov <lpopov@mm-sol.com>
> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> ---
> Changes in V2:
>         Removed all instances of writew(0, &i2c_base->cnt) instead of just the
>         one in i2c_write (following a test of V1 by Thomas Petazzoni).
>
>  drivers/i2c/omap24xx_i2c.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
> index 3d38c03..c784004 100644
> --- a/drivers/i2c/omap24xx_i2c.c
> +++ b/drivers/i2c/omap24xx_i2c.c
> @@ -158,7 +158,6 @@ static void omap24_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd)
>         udelay(1000);
>         flush_fifo(adap);
>         writew(0xFFFF, &i2c_base->stat);
> -       writew(0, &i2c_base->cnt);
>  }
>
>  static void flush_fifo(struct i2c_adapter *adap)
> @@ -198,8 +197,6 @@ static int omap24_i2c_probe(struct i2c_adapter *adap, uchar chip)
>                 return res;
>
>         /* No data transfer, slave addr only */
> -       writew(0, &i2c_base->cnt);
> -       /* Set slave address */
>         writew(chip, &i2c_base->sa);
>         /* Stop bit needed here */
>         writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |
> @@ -234,7 +231,6 @@ static int omap24_i2c_probe(struct i2c_adapter *adap, uchar chip)
>  pr_exit:
>         flush_fifo(adap);
>         writew(0xFFFF, &i2c_base->stat);
> -       writew(0, &i2c_base->cnt);
>         return res;
>  }
>
> @@ -372,7 +368,6 @@ static int omap24_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr,
>  rd_exit:
>         flush_fifo(adap);
>         writew(0xFFFF, &i2c_base->stat);
> -       writew(0, &i2c_base->cnt);
>         return i2c_error;
>  }
>
> @@ -473,7 +468,6 @@ static int omap24_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr,
>  wr_exit:
>         flush_fifo(adap);
>         writew(0xFFFF, &i2c_base->stat);
> -       writew(0, &i2c_base->cnt);
>         return i2c_error;
>  }
>
> --
> 1.8.1.2
>
Tested with various OMAP3 IGEP boards (with OMAP353x and DM373x) and works.

Thanks,
  Enric

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Lubomir Popov]

Hi Nikita,

On 28/11/13 18:04, Nikita Kiryanov wrote:
> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
> based devices. This seems to be related to the following advisory which
> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
> OMAP4430 TRM:
>
> Advisory:
> I2C Module Does Not Allow 0-Byte Data Requests
> Details:
> When configured as the master, the I2C module does not allow 0-byte data
> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
> undefined behavior.
> Workaround(s):
> No workaround. Do not use 0-byte data requests.
>
> The writes in question are unnecessary from a functional point of view.
> Most of them are done after I/O has finished, and the only one that preceds
> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
> actual data transmission takes place.
>
> Therefore, remove all writes that zero the cnt register.
>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Lubomir Popov <lpopov@mm-sol.com>
> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> ---
> Changes in V2:	
> 	Removed all instances of writew(0, &i2c_base->cnt) instead of just the
> 	one in i2c_write (following a test of V1 by Thomas Petazzoni).
Tested today on a OMAP5432uEVM and on a custom OMAP5430 board. All is OK.
Shall test on OMAP4 next week.

-- 
Regards,
Lubo

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Andreas Naumann]

Hi,

Am 28.11.2013 17:04, schrieb Nikita Kiryanov:
> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
> based devices. This seems to be related to the following advisory which
> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
> OMAP4430 TRM:
>
> Advisory:
> I2C Module Does Not Allow 0-Byte Data Requests
> Details:
> When configured as the master, the I2C module does not allow 0-byte data
> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
> undefined behavior.
> Workaround(s):
> No workaround. Do not use 0-byte data requests.
>
> The writes in question are unnecessary from a functional point of view.
> Most of them are done after I/O has finished, and the only one that preceds
> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
> actual data transmission takes place.
>
> Therefore, remove all writes that zero the cnt register.
>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Lubomir Popov <lpopov@mm-sol.com>
> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> ---
> Changes in V2:	
> 	Removed all instances of writew(0, &i2c_base->cnt) instead of just the
> 	one in i2c_write (following a test of V1 by Thomas Petazzoni).

I can also confirm this patch eliminates the frequent occurence of the 
following message:
	i2c_write: pads on bus 0 probably not configured (status=0x10)
	Could not write grp_sel to reg 82 (2)

Tested on a custom AM37xx board with 2013.07. Before the patch I got 
this message on about 5% of all reboots (I have test data of several 
thousand reboots). However, the board never failed to boot, with or 
without patch.

cheers,
Andi

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Thomas Petazzoni]

Dear Nikita Kiryanov,

On Thu, 28 Nov 2013 18:04:42 +0200, Nikita Kiryanov wrote:
> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
> based devices. This seems to be related to the following advisory which
> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
> OMAP4430 TRM:
> 
> Advisory:
> I2C Module Does Not Allow 0-Byte Data Requests
> Details:
> When configured as the master, the I2C module does not allow 0-byte data
> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
> undefined behavior.
> Workaround(s):
> No workaround. Do not use 0-byte data requests.
> 
> The writes in question are unnecessary from a functional point of view.
> Most of them are done after I/O has finished, and the only one that preceds
> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
> actual data transmission takes place.
> 
> Therefore, remove all writes that zero the cnt register.
> 
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Lubomir Popov <lpopov@mm-sol.com>
> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> ---
> Changes in V2:	
> 	Removed all instances of writew(0, &i2c_base->cnt) instead of just the
> 	one in i2c_write (following a test of V1 by Thomas Petazzoni).

Tested-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

On IGEPv2, OMAP36XX/37XX-GP ES1.2.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Lubomir Popov]

Hi Nikita,

On 28/11/13 18:04, Nikita Kiryanov wrote:
> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
> based devices. This seems to be related to the following advisory which
> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
> OMAP4430 TRM:
>
> Advisory:
> I2C Module Does Not Allow 0-Byte Data Requests
> Details:
> When configured as the master, the I2C module does not allow 0-byte data
> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
> undefined behavior.
> Workaround(s):
> No workaround. Do not use 0-byte data requests.
>
> The writes in question are unnecessary from a functional point of view.
> Most of them are done after I/O has finished, and the only one that preceds
> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
> actual data transmission takes place.
>
> Therefore, remove all writes that zero the cnt register.
>
> Cc: Heiko Schocher <hs@denx.de>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Lubomir Popov <lpopov@mm-sol.com>
> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
> ---
> Changes in V2:	
> 	Removed all instances of writew(0, &i2c_base->cnt) instead of just the
> 	one in i2c_write (following a test of V1 by Thomas Petazzoni).
>
>
Tested-by: Lubomir Popov <lpopov@mm-sol.com>

In addition to the OMAP5430/32 tests performed last week, tested today
on OMAP4 (4430/60/70) and on AM3359. Thus tests have covered OMAP4/5-
compatible I2C IPs with revnb_lo=[0x000a to 0x000c] (revnb_hi is 0x5040
for all those IPs).

-- 
Regards,
Lubo

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Lokesh Vutla]

Hi Lubomir,
On Monday 02 December 2013 09:17 PM, Lubomir Popov wrote:
> Hi Nikita,
> 
> On 28/11/13 18:04, Nikita Kiryanov wrote:
>> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
>> based devices. This seems to be related to the following advisory which
>> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
>> OMAP4430 TRM:
>>
>> Advisory:
>> I2C Module Does Not Allow 0-Byte Data Requests
>> Details:
>> When configured as the master, the I2C module does not allow 0-byte data
>> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
>> undefined behavior.
>> Workaround(s):
>> No workaround. Do not use 0-byte data requests.
>>
>> The writes in question are unnecessary from a functional point of view.
>> Most of them are done after I/O has finished, and the only one that preceds
>> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
>> actual data transmission takes place.
>>
>> Therefore, remove all writes that zero the cnt register.
>>
>> Cc: Heiko Schocher <hs@denx.de>
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Cc: Tom Rini <trini@ti.com>
>> Cc: Lubomir Popov <lpopov@mm-sol.com>
>> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
>> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
>> ---
>> Changes in V2:   
>>     Removed all instances of writew(0, &i2c_base->cnt) instead of just the
>>     one in i2c_write (following a test of V1 by Thomas Petazzoni).
>>
>>
> Tested-by: Lubomir Popov <lpopov@mm-sol.com>
> 
> In addition to the OMAP5430/32 tests performed last week, tested today
> on OMAP4 (4430/60/70) and on AM3359. Thus tests have covered OMAP4/5-
> compatible I2C IPs with revnb_lo=[0x000a to 0x000c] (revnb_hi is 0x5040
> for all those IPs).
May I know on top of which tree,tag you are trying this patch ?
I tried OMAP4 on top of v2014.01-rc1, but I am not able to boot. I applied this patch and still
not able to boot. There is a mail thread going on, on this topic.
So I just wanted to know that I am not missing very obvious. 

Thanks and regards,
Lokesh
> 

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Lubomir Popov]

Hi Lokesh,

On 03/12/13 06:02, Lokesh Vutla wrote:
> Hi Lubomir,
> On Monday 02 December 2013 09:17 PM, Lubomir Popov wrote:
>> Hi Nikita,
>>
>> On 28/11/13 18:04, Nikita Kiryanov wrote:
>>> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
>>> based devices. This seems to be related to the following advisory which
>>> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
>>> OMAP4430 TRM:
>>>
>>> Advisory:
>>> I2C Module Does Not Allow 0-Byte Data Requests
>>> Details:
>>> When configured as the master, the I2C module does not allow 0-byte data
>>> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
>>> undefined behavior.
>>> Workaround(s):
>>> No workaround. Do not use 0-byte data requests.
>>>
>>> The writes in question are unnecessary from a functional point of view.
>>> Most of them are done after I/O has finished, and the only one that preceds
>>> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
>>> actual data transmission takes place.
>>>
>>> Therefore, remove all writes that zero the cnt register.
>>>
>>> Cc: Heiko Schocher <hs@denx.de>
>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Cc: Tom Rini <trini@ti.com>
>>> Cc: Lubomir Popov <lpopov@mm-sol.com>
>>> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
>>> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
>>> ---
>>> Changes in V2:
>>>      Removed all instances of writew(0, &i2c_base->cnt) instead of just the
>>>      one in i2c_write (following a test of V1 by Thomas Petazzoni).
>>>
>>>
>> Tested-by: Lubomir Popov <lpopov@mm-sol.com>
>>
>> In addition to the OMAP5430/32 tests performed last week, tested today
>> on OMAP4 (4430/60/70) and on AM3359. Thus tests have covered OMAP4/5-
>> compatible I2C IPs with revnb_lo=[0x000a to 0x000c] (revnb_hi is 0x5040
>> for all those IPs).
> May I know on top of which tree,tag you are trying this patch ?
> I tried OMAP4 on top of v2014.01-rc1, but I am not able to boot. I applied this patch and still
> not able to boot. There is a mail thread going on, on this topic.
> So I just wanted to know that I am not missing very obvious.
For most boards (the OMAP5432 uEVM, our OMAP5430 board, the 4460 Pandaboard ES, the
AM335x General Purpose EVM and the AM335x Starter Kit) this was a version of the 
master
branch that I cloned on Nov. 12. See dump below (with some debug prints added to 
display
the I2C core revision numbers):

U-Boot SPL 2013.10-00339-gd7adce9-dirty (Dec 03 2013 - 09:31:33)
OMAP5432 ES2.0
SPL: Please implement spl_start_uboot() for your board
SPL: Direct Linux boot not active!
reading u-boot.img
reading u-boot.img


U-Boot 2013.10-00339-gd7adce9-dirty (Dec 03 2013 - 09:31:33)

CPU  : OMAP5432 ES2.0
Board: OMAP5432 uEVM
I2C:   I2C_REVNB_LO: 0x0000000c
I2C_REVNB_HI: 0x00005040
ready
DRAM:  2 GiB
I2C_REVNB_LO: 0x0000000c
I2C_REVNB_HI: 0x00005040
I2C_REVNB_LO: 0x0000000c
I2C_REVNB_HI: 0x00005040
MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
Using default environment

I2C_REVNB_LO: 0x0000000c
I2C_REVNB_HI: 0x00005040
Net:   No ethernet found.
Hit any key to stop autoboot:  0
U-Boot#

For our OMAP4 board (on which I tested the OMAP4430 ES2.1, OMAP4460 ES1.0 and ES1.1,
and OMAP4470 ES1.0, by means of TI Blaze/Tablet processor cards) I used a 2013.04
version of U-Boot, on which I developed the new I2C driver back then and on which
the submitted driver patches were based (the driver was mainlined in 2013.07, 
AFAIR);
in this version I also have working support for OMAP4470/TWL6032 (not mainlined). I
did so because the 2013.10 version used for the other boards won't boot on our 
OMAP4,
and I didn't have time to investigate why (it did boot on the Panda ES though).

The reason to not use the current master branch is ridiculous - my workplace is for
some months now within the TI intranet, and since about mid November, when some 
network
infrastructure reorg happened, the TI proxy blocks my access to any git repo. 
Because
my current work is not related to software, I haven't actively opted for 
granting access
until yesterday, and am now waiting.

In summary, what I have done is essentially to confirm that removing all writes of a
0-byte length to the i2c_cnt register (which is _needed_ to fix the OMAP3 
problem) does
not affect operation on OMAP4/5-compatible I2C IPs. I have not applied Nikita's 
fix as
a patch, but have manually commented out those lines in both U-Boot versions 
used for
the test. This is probably against the formal rules, but I stand behind the 
statement
that functionally all is OK.

On which board did you fail to boot OMAP4? Isn't this a strange coincidence, aren't
we having a regression...? :-(

-- 
Regards,
Lubo

[U-Boot] [PATCH V2] arm: omap: i2c: don't zero cnt in i2c_write

[Lokesh Vutla]

Hi Lubomir,
On Tuesday 03 December 2013 02:20 PM, Lubomir Popov wrote:
> Hi Lokesh,
> 
> On 03/12/13 06:02, Lokesh Vutla wrote:
>> Hi Lubomir,
>> On Monday 02 December 2013 09:17 PM, Lubomir Popov wrote:
>>> Hi Nikita,
>>>
>>> On 28/11/13 18:04, Nikita Kiryanov wrote:
>>>> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
>>>> based devices. This seems to be related to the following advisory which
>>>> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
>>>> OMAP4430 TRM:
>>>>
>>>> Advisory:
>>>> I2C Module Does Not Allow 0-Byte Data Requests
>>>> Details:
>>>> When configured as the master, the I2C module does not allow 0-byte data
>>>> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
>>>> undefined behavior.
>>>> Workaround(s):
>>>> No workaround. Do not use 0-byte data requests.
>>>>
>>>> The writes in question are unnecessary from a functional point of view.
>>>> Most of them are done after I/O has finished, and the only one that preceds
>>>> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
>>>> actual data transmission takes place.
>>>>
>>>> Therefore, remove all writes that zero the cnt register.
>>>>
>>>> Cc: Heiko Schocher <hs@denx.de>
>>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>>> Cc: Tom Rini <trini@ti.com>
>>>> Cc: Lubomir Popov <lpopov@mm-sol.com>
>>>> Cc: Enric Balletbo Serra <eballetbo@gmail.com>
>>>> Signed-off-by: Nikita Kiryanov <nikita@compulab.co.il>
>>>> ---
>>>> Changes in V2:
>>>>      Removed all instances of writew(0, &i2c_base->cnt) instead of just the
>>>>      one in i2c_write (following a test of V1 by Thomas Petazzoni).
>>>>
>>>>
>>> Tested-by: Lubomir Popov <lpopov@mm-sol.com>
>>>
>>> In addition to the OMAP5430/32 tests performed last week, tested today
>>> on OMAP4 (4430/60/70) and on AM3359. Thus tests have covered OMAP4/5-
>>> compatible I2C IPs with revnb_lo=[0x000a to 0x000c] (revnb_hi is 0x5040
>>> for all those IPs).
>> May I know on top of which tree,tag you are trying this patch ?
>> I tried OMAP4 on top of v2014.01-rc1, but I am not able to boot. I applied this patch and still
>> not able to boot. There is a mail thread going on, on this topic.
>> So I just wanted to know that I am not missing very obvious.
> For most boards (the OMAP5432 uEVM, our OMAP5430 board, the 4460 Pandaboard ES, the
> AM335x General Purpose EVM and the AM335x Starter Kit) this was a version of the master
> branch that I cloned on Nov. 12. See dump below (with some debug prints added to display
> the I2C core revision numbers):
> 
> U-Boot SPL 2013.10-00339-gd7adce9-dirty (Dec 03 2013 - 09:31:33)
> OMAP5432 ES2.0
> SPL: Please implement spl_start_uboot() for your board
> SPL: Direct Linux boot not active!
> reading u-boot.img
> reading u-boot.img
> 
> 
> U-Boot 2013.10-00339-gd7adce9-dirty (Dec 03 2013 - 09:31:33)
> 
> CPU  : OMAP5432 ES2.0
> Board: OMAP5432 uEVM
> I2C:   I2C_REVNB_LO: 0x0000000c
> I2C_REVNB_HI: 0x00005040
> ready
> DRAM:  2 GiB
> I2C_REVNB_LO: 0x0000000c
> I2C_REVNB_HI: 0x00005040
> I2C_REVNB_LO: 0x0000000c
> I2C_REVNB_HI: 0x00005040
> MMC:   OMAP SD/MMC: 0, OMAP SD/MMC: 1
> Using default environment
> 
> I2C_REVNB_LO: 0x0000000c
> I2C_REVNB_HI: 0x00005040
> Net:   No ethernet found.
> Hit any key to stop autoboot:  0
> U-Boot#
> 
> For our OMAP4 board (on which I tested the OMAP4430 ES2.1, OMAP4460 ES1.0 and ES1.1,
> and OMAP4470 ES1.0, by means of TI Blaze/Tablet processor cards) I used a 2013.04
> version of U-Boot, on which I developed the new I2C driver back then and on which
> the submitted driver patches were based (the driver was mainlined in 2013.07, AFAIR);
> in this version I also have working support for OMAP4470/TWL6032 (not mainlined). I
> did so because the 2013.10 version used for the other boards won't boot on our OMAP4,
> and I didn't have time to investigate why (it did boot on the Panda ES though).
> 
> The reason to not use the current master branch is ridiculous - my workplace is for
> some months now within the TI intranet, and since about mid November, when some network
> infrastructure reorg happened, the TI proxy blocks my access to any git repo. Because
> my current work is not related to software, I haven't actively opted for granting access
> until yesterday, and am now waiting.
> 
> In summary, what I have done is essentially to confirm that removing all writes of a
> 0-byte length to the i2c_cnt register (which is _needed_ to fix the OMAP3 problem) does
> not affect operation on OMAP4/5-compatible I2C IPs. I have not applied Nikita's fix as
> a patch, but have manually commented out those lines in both U-Boot versions used for
> the test. This is probably against the formal rules, but I stand behind the statement
> that functionally all is OK.
Thanks for the info.
I just wanted to know if you are using v2014.01-rc1. 
> 
> On which board did you fail to boot OMAP4? Isn't this a strange coincidence, aren't
> we having a regression...? :-(
I faced this issue on my PANDA ES board. There is already a discussion going on in U-Boot ML with
subject as "No single character output after update to latest u-boot on pandaboard"

Thanks and regards,
Lokesh
> 

[U-Boot] [U-Boot, V2] arm: omap: i2c: don't zero cnt in i2c_write

[Heiko Schocher]

Hello Nikita,

Am 28.11.2013 17:04, schrieb Nikita Kiryanov:
> Writing zero into I2Ci.I2C_CNT register causes random I2C failures in OMAP3
> based devices. This seems to be related to the following advisory which
> apears in multiple erratas for OMAP3 SoCs (OMAP35xx, DM37xx), as well as
> OMAP4430 TRM:
>
> Advisory:
> I2C Module Does Not Allow 0-Byte Data Requests
> Details:
> When configured as the master, the I2C module does not allow 0-byte data
> transfers. Note: Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will cause
> undefined behavior.
> Workaround(s):
> No workaround. Do not use 0-byte data requests.
>
> The writes in question are unnecessary from a functional point of view.
> Most of them are done after I/O has finished, and the only one that preceds
> I/O (in i2c_probe()) is also unnecessary because a stop bit is sent before
> actual data transmission takes place.
>
> Therefore, remove all writes that zero the cnt register.
>
> Cc: Heiko Schocher<hs@denx.de>
> Cc: Thomas Petazzoni<thomas.petazzoni@free-electrons.com>
> Cc: Tom Rini<trini@ti.com>
> Cc: Lubomir Popov<lpopov@mm-sol.com>
> Cc: Enric Balletbo Serra<eballetbo@gmail.com>
> Signed-off-by: Nikita Kiryanov<nikita@compulab.co.il>
> Tested-by: Thomas Petazzoni<thomas.petazzoni@free-electrons.com>
> Tested-by: Lubomir Popov<lpopov@mm-sol.com>
>
> ---
> Changes in V2:	
> 	Removed all instances of writew(0,&i2c_base->cnt) instead of just the
> 	one in i2c_write (following a test of V1 by Thomas Petazzoni).
>
>   drivers/i2c/omap24xx_i2c.c | 6 ------
>   1 file changed, 6 deletions(-)

Thanks! Applied to u-boot-i2c.git.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany