[Skiboot] [PATCH v2] hw/p8-i2c: Fix OCC locking
Stewart Smith
stewart at linux.vnet.ibm.com
Tue Aug 29 18:17:26 AEST 2017
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.
More information about the Skiboot
mailing list