[Skiboot] [PATCH] i2c: Fix i2c request hang during opal init if timers are not checked

[Stewart Smith]

Frederic Barrat <fbarrat at linux.ibm.com> writes:
> Le 11/12/2018 à 04:41, Stewart Smith a écrit :
>> Frederic Barrat <fbarrat at linux.ibm.com> writes:
>>> If an i2c request cannot go through the first time, because the bus is
>>> found in error and need a reset or it's locked by the OCC for example,
>>> the underlying i2c implementation is using timers to manage the
>>> request. However during opal init, opal pollers may not be called, it
>>> depends in the context in which the i2c request is made. If the
>>> pollers are not called, the timers are not checked and we can end up
>>> with an i2c request which will not move foward and skiboot hangs.
>>>
>>> Fix it by explicitly checking the timers if we are waiting for an i2c
>>> request to complete and it seems to be taking a while.
>> 
>> Okay, I see what happens here: we're doing the i2c request in the main
>> thread and doing time_wait() with a value that'll never run pollers and
>> thus never check_timers() either.
>> 
>> Looking at the code here, is there any reason we shouldn't just call
>> check_timers(false) straight after the time_wait() ?
>
>
> The only reason I was reluctant to do so is that the default 
> time_to_wait we get from i2c_run_req() is pretty short, so I didn't want 
> to degrade a common path for the benefit of a supposedly rarer error 
> path (bus in use by OCC, bus found in error state, ...).
>
> Also I didn't call opal_run_pollers() for a similar reason, as the call 
> is more expensive and I got scared by how we treat it in 
> time_wait_poll(), seemingly avoiding calling it too frequently.
> I basically did the minimum required to get the i2c request unstuck, so 
> that the main thread can get back to do real work during opal init.

That's reasonable, I was lazy and didn't attempt to do the math/measure
it.

We should probably have some kind of spin_on_condition() routine that
will do a bit better rather than open coding this every single
time.

-- 
Stewart Smith
OPAL Architect, IBM.