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

[Stewart Smith]

ppaidipe <ppaidipe at linux.vnet.ibm.com> writes:
> On 2017-08-25 10:59, Stewart Smith wrote:
>> 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(-)
>>> ---
>>> 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
>> 
>> 
>> Pridhiviraj, could you have a look at adding something to our EEPROM
>> tests in op-test-framework to do something like the above? That is,
>> md5sum the result of reading it and read the eeprom in a loop a few
>> times.
>> 
>> (that is, if we don't already have that test there)
>
> Stewart, added the above one here, Please take a look at it.
>
> https://github.com/open-power/op-test-framework/pull/156/commits/c3a1ebd41cb02d5591e929c5a1b9e1c92b9a2ec1

Thanks for that! I merged it with a bit of a change so that it used
difflib to display the difference between reads - so we get a really
nice error message out.

-- 
Stewart Smith
OPAL Architect, IBM.