[U-Boot] A question about unconfigured pads check in omap24xx_i2c

[Lubomir Popov]

Hi Nikita,

On 11-Nov-13 13:15, Nikita Kiryanov wrote:
> On 11/08/2013 11:26 PM, Lubomir Popov wrote:
>> Hi Nikita,
>>
>>> On 11/06/2013 03:19 PM, Lubomir Popov wrote:
>>>> On 06-Nov-13 14:12, Nikita Kiryanov wrote:
>>>>> In drivers/i2c/omap24xx_i2c.c there are a few checks that attempt to
>>>>> detect unconfigured pads for the i2c bus in use. These checks are
>>>>> all in the form of
>>>>>
>>>>> if (status == I2C_STAT_XRDY) {
>>>>>      printf("unconfigured pads\n");
>>>>>      return -1;
>>>>> }
>>>>>
>>>>> This check seems peculiar to me since the meaning of I2C_STAT_XRDY is
>>>>> that new data is requested for transmission. Why is that 
>>>>> indication that
>>>>> the bus is not padconf'd for I2C?
>>>> Hi Nikita,
>>>>
>>>> This has been empirically confirmed on OMAP4 and OMAP5. When the pads
>>>> are not
>>>> configured, the I2C controller is actually disconnected from the bus.
>>>> The clock
>>>> input for its state machine has to come from the bus however due to
>>>> stretching
>>>> etc., although it is internally generated. So actually nothing changes
>>>> within
>>>> the controller after a transaction attempt is made, and it keeps its
>>>> initial
>>>> state with XRDY set only (ready to accept transmit data). I use 
>>>> this as an
>>>> indicator. Not perfect, but works in most cases.
>>>>
>>>> Regards,
>>>> Lubo
>>>>
>>>>
>>>
>>> Thanks for the quick reply.
>>> The reason I stumbled across this is that this check hasn't been 
>>> working
>>> well on our OMAP3 based devices. Some I2C transactions work fine, but
>>> some of them fail this check in the address phase, especially if the 
>>> I2C
>>> transactions happen in quick successions.
>> You mean that you occasionally get this error on an otherwise properly
>> configured and working bus, right?
>
> Yes.
>
>> Does this happen with particular
>> slave devices only, or randomly? And is it happening in the SPL, or in
>> regular U-Boot?
>
> It happens in U-Boot when communicating with the PMIC (TPS65930),
> but like I said, it worked in the driver's previous version, on the
> same module.
>
>>
>>
>>> We did not have any I2C issues
>>> with the previous driver, and while this behavior is symptomatic of
>>> timing issues playing around with the delays didn't help.
>> Which delays did you modify? Did you try to increase 
>> I2C_WAIT/I2C_TIMEOUT?
>
> I tried to increase both I2C_WAIT and I2C_TIMEOUT, and place my own
> delays as well.
>
>>
>>> I was wondering if you might have some insights as to what may cause 
>>> the
>>> controller to behave this way other than unconfigured pads?
>> Unfortunately I don't have any hands-on experience with OMAP3 (apart 
>> from
>> some very quick testing on a AM3359 derivative), and cannot guarantee 
>> that
>> the I2C controller IP on OMAP3 is absolutely the same as on OMAP4/5 
>> (most
>> probably it isn't; shall check this on Monday). Anyway, if everything 
>> else
>> is OK (muxmode/pad config, pull-ups, power, etc.), the only reasonable
>> explanation would be that there is not enough time for the controller to
>> update its status register under certain conditions, and these would be
>> either a slower clock rate (that makes byte transmission time come close
>> to the timeout), or clock stretching by some slaves; btw, the latter
>> seems probable, judging from your words that this happens in the address
>> phase, when some devices could take longer to prepare for action, and 
>> they
>> do this by stretching the clock. That is why I'm asking if you tried to
>> increase I2C_TIMEOUT. Did you do any measurements on the bus to see 
>> if all
>> is OK and the clock rate is right, and if it gets stretched by some 
>> slaves?
>
> I believe I found the cause of the problem. In the new version of the
> driver the following line was added to the exit sequence of i2c_write:
>
> writew(0, &i2c_base->cnt);
>
> Removing this line solved the problem (module has been doing I2C
> transactions for the last 16 hours or so without failing), and I
> believe the reason has to do with Advisory 1.2 in the DM3730 errata:
>
> Advisory 1.2        I2C Module Does Not Allow 0-Byte Data Requests
> Revision(s) Affected    1.2, 1.1 and 1.0
> Details            When configured as the master, the I2C module
>             does not allow 0-byte data transfers.
>             Programming I2Ci.I2C_CNT[15:0]: DCOUNT = 0 will
>             cause undefined behavior.
> Workaround(s)        There is no workaround for this issue. Do not
>             use 0-byte data requests.
>
> While the mentioned write is done at the end of i2c_write, perhaps the
> driver's MO still triggers this issue. It certainly is plausible
> that this line was missing from the old i2c_write exit sequence on
> purpose, and not by accident (i2c_read, i2c_probe, and i2c_init all
> had it in the old driver).
Many thanks for catching this. Feel free to post a patch.

Lubo