[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 at denx.de>
>>>> Cc: Thomas Petazzoni <thomas.petazzoni at free-electrons.com>
>>>> Cc: Tom Rini <trini at ti.com>
>>>> Cc: Lubomir Popov <lpopov at mm-sol.com>
>>>> Cc: Enric Balletbo Serra <eballetbo at gmail.com>
>>>> Signed-off-by: Nikita Kiryanov <nikita at 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 at 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
>