| Message ID | 20210826174632.91887-1-t.schramm@manjaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA | expand |
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;
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. Does it also help to not send STOP? This looks a bit like a hack which went into the driver somehow. The proper solution might be to remove the STOP and add proper i2c_bus_recovery at the beginning of a transfer. > > Signed-off-by: Tobias Schramm <t.schramm@manjaro.org> > --- > 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; > -- > 2.31.1 >
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;
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> --- drivers/i2c/busses/i2c-rk3x.c | 10 ++++++++++ 1 file changed, 10 insertions(+)