[U-Boot] [PATCH 1/3] i2c: tegra: use repeated start for reads

[Stephen Warren]

On 06/26/2014 01:11 PM, Joakim Tjernlund wrote:
> Stephen Warren <swarren at wwwdotorg.org> wrote on 2014/06/26 18:47:55:
>>
>> On 06/26/2014 02:11 AM, Joakim Tjernlund wrote:
>>>> From: Stephen Warren <swarren at wwwdotorg.org>
>>>> To: u-boot at lists.denx.de, Heiko Schocher <hs at denx.de>, 
>>>> Cc: Stephen Warren <swarren at nvidia.com>, Tom Warren 
> <twarren at nvidia.com>
>>>> Date: 2014/06/25 19:05
>>>> Subject: [U-Boot] [PATCH 1/3] i2c: tegra: use repeated start for 
> reads
>>>> Sent by: u-boot-bounces at lists.denx.de
>>>>
>>>> From: Stephen Warren <swarren at nvidia.com>
>>>>
>>>> I2C read transactions are typically implemented as follows:
>>>>
>>>> START(write) address REPEATED_START(read) data... STOP
>>>>
>>>> However, Tegra's I2C driver currently implements reads as follows:
>>>>
>>>> START(write) address STOP START(read) data... STOP
>>>>
>>>> This sequence confuses at least the AS3722 PMIC on the Jetson TK1 
> board,
>>>> leading to corrupted read data in some cases. Fix the driver to chain
>>>> the transactions together using repeated starts to solve this.
>>>
>>> While I agree to use Repeated START I just wanted to share this:
>>> A common reason for STOP START(read) sequence not working sometimes is 
> 
>>> that
>>> the driver initializes STOP but does not wait for the STOP to complete
>>> before issuing a START.
>>
>> I don't believe that's the case here, since all this patch does is set a
>> flag to indicate whether the write transaction (to set the intra-chip
>> register address) generates STOP or REPEATED_START at the end. If the
>> code or HW wasn't waiting for the STOP to complete, I see no reason it
>> would wait for the REPEATED_START to complete either, so I think the
>> subsequent register read transaction would be corrupted in either case.
> 
> But there is, you have STOP + START vs. ReSTART only and if the code only 
> flips a flag to change I think there is a chance in this case.
> You could easily test by adding a udelay(5) after STOP is initiated. 

If the code doesn't wait for the STOP/REPEATED_START to complete, then
whatever comes after will trample it, whether it's a START, or the data
transfer for the next transaction.

Anyway, as I note below, the code is waiting:

>> There's certainly code in tegra_i2c.c:wait_for_transfer_complete() to
>> poll until each transaction completes before starting the next, and
>> there's even error handling to detect any problems there:-)