Stephen Warren swarren@wwwdotorg.org wrote on 2014/06/26 21:18:50:
On 06/26/2014 01:11 PM, Joakim Tjernlund wrote:
Stephen Warren swarren@wwwdotorg.org wrote on 2014/06/26 18:47:55:
On 06/26/2014 02:11 AM, Joakim Tjernlund wrote:
From: Stephen Warren swarren@wwwdotorg.org To: u-boot@lists.denx.de, Heiko Schocher hs@denx.de, Cc: Stephen Warren swarren@nvidia.com, Tom Warren
Date: 2014/06/25 19:05 Subject: [U-Boot] [PATCH 1/3] i2c: tegra: use repeated start for
reads
Sent by: u-boot-bounces@lists.denx.de
From: Stephen Warren swarren@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.
You misunderstand, there has to be an extra STOP complete time between the STOP and START. When replacing this with only one ReSTART that STOP complete time is gone. So by changing to ReSTART you fixed this error and got much better behaviour too. I am just saying that there might be other places which lacks STOP completion as well For reference you can look at
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=21f4cbb77299788e2b06c9b0f48c... and
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drive... That took a while for me to figure out :)
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:-)