Re: [2/3] i2c: exynos5: implement bus recovery functionality

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 1401 bytes --]

Hi,

On Thu, Nov 30, 2017 at 03:30:06PM +0100, Andrzej Hajda wrote:
> In some circumstances I2C clients can be in abnormal state, to allow
> recover them from it I2C master can pulse SCL line until SDA line
> goes up and then generate STOP condition. Exynos driver does not have
> possibility to manipulate I2C lines directly, but it can provide similar
> functionality using manual commands. Replacing I2C adapter reset with

Really? READ_DATA looks like 8 clock pulses, but you need 9.

> bus recovery significantly incereases I2C bus robustness in case of timeouts.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>



> @@ -642,7 +666,7 @@ static int exynos5_i2c_xfer_msg(struct exynos5_i2c *i2c,
>  		ret = exynos5_i2c_wait_bus_idle(i2c);
>  
>  	if (ret < 0) {
> -		exynos5_i2c_reset(i2c);
> +		exynos5_i2c_recovery(i2c);

Bus recovery is only defined for stalled busses, i.e. when SDA is stuck
low. It is not a suitable method to deal with general timeouts.


> +int exynos5_i2c_recover_bus(struct i2c_adapter *adap)
> +{
> +	struct exynos5_i2c *i2c = adap->algo_data;
> +
> +	i2c_lock_adapter(adap);
> +	clk_enable(i2c->clk);
> +	exynos5_i2c_recovery(i2c);
> +	clk_disable(i2c->clk);
> +	i2c_unlock_adapter(adap);

Why do you need to lock? Aren't you protected by the call to master_xfer
which then detected a problem?

Regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]