Date:   Thu, 18 Nov 2021 18:35:49 +0000
From:   John Keeping <john@metanate.com>
To:     Tobias Schramm <t.schramm@manjaro.org>
Cc:     linux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
        Heiko Stuebner <heiko@sntech.de>
Subject: Re: [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to
 fix stuck low SDA
Message-ID: <YZadBW3TGhVSEtQP@donbot>
References: <20210826174632.91887-1-t.schramm@manjaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210826174632.91887-1-t.schramm@manjaro.org>
Precedence: bulk

On Thu, Aug 26, 2021 at 07:46:32PM +0200, Tobias Schramm wrote:
> Previously the SDA line sometimes remained stuck low after timeouts
> rendering the I2C bus unusable. Testing has shown that disabling and
> reenabling the I2C peripheral after sending the stop condition seems to
> unstick SDA reliably.
> Disable and reenable the I2C controller on timeout after sending stop
> condition to unstick SDA.
> 
> Signed-off-by: Tobias Schramm <t.schramm@manjaro.org>

I haven't seen the issue described here, so I can't test whether this
fix works, but the explanation makes sense, so:

Reviewed-by: John Keeping <john@metanate.com>

> ---
>  drivers/i2c/busses/i2c-rk3x.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index 819ab4ee517e..fc330cc3686a 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -1108,6 +1108,16 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap,
>  			val |= REG_CON_EN | REG_CON_STOP;
>  			i2c_writel(i2c, val, REG_CON);
>  
> +			/*
> +			 * Sometimes SDA remains stuck low after timeouts.
> +			 * Disable and reenable the I2C peripheral to unstick
> +			 * SDA.
> +			 */
> +			val &= ~REG_CON_EN;
> +			i2c_writel(i2c, val, REG_CON);
> +			val |= REG_CON_EN;
> +			i2c_writel(i2c, val, REG_CON);
> +
>  			i2c->state = STATE_IDLE;
>  
>  			ret = -ETIMEDOUT;