From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Thu, 26 Jun 2014 13:18:50 -0600 Subject: [U-Boot] [PATCH 1/3] i2c: tegra: use repeated start for reads In-Reply-To: References: <1403715449-2177-1-git-send-email-swarren@wwwdotorg.org> <53AC4EBB.8030108@wwwdotorg.org> Message-ID: <53AC721A.5030602@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 06/26/2014 01:11 PM, Joakim Tjernlund wrote: > Stephen Warren wrote on 2014/06/26 18:47:55: >> >> On 06/26/2014 02:11 AM, Joakim Tjernlund wrote: >>>> From: Stephen Warren >>>> To: u-boot at lists.denx.de, Heiko Schocher , >>>> Cc: Stephen Warren , 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 at lists.denx.de >>>> >>>> From: Stephen Warren >>>> >>>> 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:-)