[Skiboot] [PATCH v2] hw/p8-i2c: Fix OCC locking

[Stewart Smith]

Oliver O'Halloran <oohall at gmail.com> writes:
> There's a few issues with the Host<->OCC I2C bus handshaking. First up,
> skiboot is currently examining the wrong bit when checking if the OCC
> is currently using the bus. Secondly, when we need to wait for the OCC
> to release the bus we are scheduling a recovery timer to run zero
> timebase ticks after the current moment so the recovery timeout handler
> will run immediately after the bus was requested, which will in turn
> re-schedule itself, etc, etc. There's also a race between the OCC
> interrupt and the recovery handler which can result in an assertion
> failure in the recovery thread. All of this is bad.
>
> This patch addresses all these issues and sets the recovery timeout to
> 10ms.
>
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
> ---
>  hw/p8-i2c.c | 58 +++++++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 37 insertions(+), 21 deletions(-)

Thanks! Merged to master as of 201fd50f208d680cfb19c0e508ca46b4f9cc75dc

> ---
> I've been using this script to test the changes. With this patch it's fairly
> stable, but there's still intermittent data corruption issues when
> when reading from the bus which I'm still investigating.
> ---
> #!/bin/bash -e
>
> while true;
> do
> 	for f in $(find /sys/devices/platform/ -name '*-????'|grep a3000 )
> 	do
> 		bus="$(basename $f | awk -F - -- '{ print $1 }' )"
> 		dev="$(basename $f | awk -F - -- '{ print $2 }' )"
>
> 		i2cdump -y $bus 0x$dev i  > $bus-$dev
> 		md5sum $bus-$dev
> 		sleep 0.1
> 	done
> done

how does this compare to
https://github.com/open-power/op-test-framework/blob/master/testcases/AT24driver.py#L97
?

It looks like we're about functionally equivalent, except our eeprom
gathering code is:
https://github.com/open-power/op-test-framework/blob/master/testcases/I2C.py#L139

Patches welcome there if you don't think it looks quite right.

with "current" code (op-build of a day or two ago, skiboot that I just
pushed) we do get the kind of fail you mention:

======================================================================
FAIL [7.456s]: runTest (testcases.AT24driver.AT24driver)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/stewart/op-test-framework/testcases/AT24driver.py", line 118, in runTest
    err="repeated i2cdump read of EEPROM doesn't match")
  File "/home/stewart/op-test-framework/testcases/AT24driver.py", line 149, in diff_commands
    self.assertMultiLineEqual(''.join(r),''.join(last_r), "%s:\n%s" % (err,diff))
AssertionError: repeated i2cdump read of EEPROM doesn't match:
--- i2cdump -f -y 4 0x50 w
+++ i2cdump -f -y 4 0x50 w
@@ -1,5 +1,5 @@
      0,8  1,9  2,a  3,b  4,c  5,d  6,e  7,f
-00: 1623 1430 0000 0000 0000 3358 0000 0000 
+00: 0000 1430 0000 0000 0000 3358 0000 0000 
 08: 0000 0000 0000 0000 0000 0000 ffff ffff 
 10: ffff ffff ffff ffff ffff ffff ffff ffff 
 18: ffff ffff ffff ffff ffff ffff ffff ffff 


----------------------------------------------------------------------


-- 
Stewart Smith
OPAL Architect, IBM.