On Fri, Dec 07, 2018 at 10:15:11AM +0100, Michal Simek wrote: > On 06. 12. 18 18:33, Kyle Roeschley wrote: > > This reverts commit 8064c616984eaa015f018dba595d78cd24a0cc8c. > > > > According to the Zynq TRM §20.2.2 under heading Read Transfer, HOLD should > > be set or cleared before writing the I2C Address register. This fixes > > sporadic i2c bus lockups on National Instruments Zynq-based devices. > > > > Signed-off-by: Kyle Roeschley <kyle.roeschley@xxxxxx> > > --- > > Tested by flooding the bus with traffic on a National Instruments cRIO-9068. > > Previously the bus would lock up after ~30s, now I can run overnight with no > > problems. > > Can you please share that app you use to be able to replicate it on our > side? I've seen the problem in two cases, neither of which are very easy to replicate: 1. Continuously rebooting a cRIO-9068 running an NI Linux kernel. This was how we caught the error originally, but it unfortunately takes 10+ hours to fail. In this case, I saw a timeout error caused by a two-message transfer immediately on load of the nizynqcpld driver [1]. Linked is our 4.9 tree because its our latest released branch, but we're currently testing on top of 4.14. 2. Writing on the status LED on a cRIO-9068 in a loop. This only takes a 30s to a couple minutes to fail, in which case I see the same timeout error from i2c-cadence and the system hangs. The script I used is below [2]. I'm working on a better MWE (that doesn't use our out of tree driver), but I don't see the same timeout error when initiating the same transfers in a user mode C application via i2c-dev. Do you know why the patch I'm reverting seems to conflict with the Zynq TRM? My only guess is that the "IP datasheet" mentioned in the commit message referes to the Cadence I2C IP manual, but I don't have access to that document. [1] https://github.com/ni/linux/blob/nilrt/18.0/4.9/drivers/misc/nizynqcpld.c#L1569 [2] LED test script: #!/bin/bash set -e LED="/sys/class/leds/nilrt:status:yellow/brightness" i=0 while (( 1 )) do echo $i echo 99999 > $LED echo 0 > $LED i=$((i+1)) done -- Kyle Roeschley Software Engineer National Instruments
