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

[Andrew Donnellan]

On 30/11/18 5:17 am, Frederic Barrat wrote:
> 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.
> 
> Signed-off-by: Frederic Barrat <fbarrat at linux.ibm.com>

This seems like a reasonable solution.

Do we want this to go to stable?

Reviewed-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>

> ---
>   core/i2c.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/core/i2c.c b/core/i2c.c
> index 26926fc2..3c600024 100644
> --- a/core/i2c.c
> +++ b/core/i2c.c
> @@ -21,6 +21,7 @@
>   #include <opal-msg.h>
>   #include <timebase.h>
>   #include <processor.h>
> +#include <timer.h>
>   
>   static LIST_HEAD(i2c_bus_list);
>   
> @@ -177,6 +178,7 @@ int i2c_request_send(int bus_id, int dev_addr, int read_write,
>   	struct i2c_bus *bus;
>   	uint64_t time_to_wait = 0;
>   	struct i2c_sync_userdata ud;
> +	uint64_t timer_period = msecs_to_tb(5), timer_count;
>   
>   	bus = i2c_find_bus_by_id(bus_id);
>   	if (!bus) {
> @@ -215,6 +217,7 @@ int i2c_request_send(int bus_id, int dev_addr, int read_write,
>   
>   	for (retries = 0; retries <= MAX_NACK_RETRIES; retries++) {
>   		waited = 0;
> +		timer_count = 0;
>   
>   		i2c_queue_req(req);
>   
> @@ -224,6 +227,19 @@ int i2c_request_send(int bus_id, int dev_addr, int read_write,
>   				time_to_wait = REQ_COMPLETE_POLLING;
>   			time_wait(time_to_wait);
>   			waited += time_to_wait;
> +			timer_count += time_to_wait;
> +			if (timer_count > timer_period) {
> +				/*
> +				 * The above request may be relying on
> +				 * timers to complete, yet there may
> +				 * not be called, especially during
> +				 * opal init. We could be looping here
> +				 * forever. So explicitly check the
> +				 * timers once in a while
> +				 */
> +				check_timers(false);
> +				timer_count = 0;
> +			}
>   		} while (!ud.done);
>   
>   		lwsync();
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited