Re: [PATCH v3 4/4] i2c: designware: add i2c gpio recovery option

[Jarkko Nikula <jarkko.nikula@linux.intel.com>]

On 08/30/2017 09:17 AM, Phil Reid wrote:
> From: Tim Sander <tim@krieglstein.org>
> 
> This patch contains much input from Phil Reid and has been tested
> on Intel/Altera Cyclone V SOC Hardware with Altera GPIO's for the
> SCL and SDA GPIO's. I am still a little unsure about the recover
> in the timeout case (i2c-designware-core.c:770) as i could not
> test this codepath.
> 
> Signed-off-by: Tim Sander <tim@krieglstein.org>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>   drivers/i2c/busses/i2c-designware-common.c | 11 ++++--
>   drivers/i2c/busses/i2c-designware-core.h   |  1 +
>   drivers/i2c/busses/i2c-designware-master.c | 57 ++++++++++++++++++++++++++++++
>   3 files changed, 66 insertions(+), 3 deletions(-)
> 
While taking into account Andy's comments please modify the last 
sentence in the above commit log - i2c-designware-core.c doesn't exist 
anymore. Maybe better is to have the uncertainty documented as a 
"REVISIT:" comment in the code etc.

> @@ -254,9 +258,10 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
>   	for_each_set_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
>   		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
>   
> -	if (abort_source & DW_IC_TX_ARB_LOST)
> +	if (abort_source & DW_IC_TX_ARB_LOST) {
> +		i2c_recover_bus(&dev->adapter);

Are you sure about doing recovery for arbitration lost case? To me it 
seems wrong to do it if another master is accessing the bus.

-- 
Jarkko