All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
@ 2021-08-26 17:46 ` Tobias Schramm
  0 siblings, 0 replies; 9+ messages in thread
From: Tobias Schramm @ 2021-08-26 17:46 UTC (permalink / raw)
  To: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip
  Cc: Heiko Stuebner, Tobias Schramm

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(+)

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


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
@ 2021-08-26 17:46 ` Tobias Schramm
  0 siblings, 0 replies; 9+ messages in thread
From: Tobias Schramm @ 2021-08-26 17:46 UTC (permalink / raw)
  To: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip
  Cc: Heiko Stuebner, Tobias Schramm

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(+)

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
@ 2021-08-26 17:46 ` Tobias Schramm
  0 siblings, 0 replies; 9+ messages in thread
From: Tobias Schramm @ 2021-08-26 17:46 UTC (permalink / raw)
  To: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip
  Cc: Heiko Stuebner, Tobias Schramm

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(+)

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
  2021-08-26 17:46 ` Tobias Schramm
  (?)
@ 2021-11-18 18:35   ` John Keeping
  -1 siblings, 0 replies; 9+ messages in thread
From: John Keeping @ 2021-11-18 18:35 UTC (permalink / raw)
  To: Tobias Schramm
  Cc: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip,
	Heiko Stuebner

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;

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
@ 2021-11-18 18:35   ` John Keeping
  0 siblings, 0 replies; 9+ messages in thread
From: John Keeping @ 2021-11-18 18:35 UTC (permalink / raw)
  To: Tobias Schramm
  Cc: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip,
	Heiko Stuebner

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;

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
@ 2021-11-18 18:35   ` John Keeping
  0 siblings, 0 replies; 9+ messages in thread
From: John Keeping @ 2021-11-18 18:35 UTC (permalink / raw)
  To: Tobias Schramm
  Cc: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip,
	Heiko Stuebner

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;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
  2021-08-26 17:46 ` Tobias Schramm
  (?)
@ 2021-11-29 10:44   ` Wolfram Sang
  -1 siblings, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2021-11-29 10:44 UTC (permalink / raw)
  To: Tobias Schramm
  Cc: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip,
	Heiko Stuebner


[-- Attachment #1.1: Type: text/plain, Size: 1528 bytes --]

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
> 

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

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
@ 2021-11-29 10:44   ` Wolfram Sang
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2021-11-29 10:44 UTC (permalink / raw)
  To: Tobias Schramm
  Cc: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip,
	Heiko Stuebner


[-- Attachment #1.1: Type: text/plain, Size: 1528 bytes --]

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
> 

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA
@ 2021-11-29 10:44   ` Wolfram Sang
  0 siblings, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2021-11-29 10:44 UTC (permalink / raw)
  To: Tobias Schramm
  Cc: linux-arm-kernel, linux-i2c, linux-kernel, linux-rockchip,
	Heiko Stuebner

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

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
> 

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

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-11-29 11:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-26 17:46 [PATCH] i2c: rk3x: disable and reenable I2C after timeouts to fix stuck low SDA Tobias Schramm
2021-08-26 17:46 ` Tobias Schramm
2021-08-26 17:46 ` Tobias Schramm
2021-11-18 18:35 ` John Keeping
2021-11-18 18:35   ` John Keeping
2021-11-18 18:35   ` John Keeping
2021-11-29 10:44 ` Wolfram Sang
2021-11-29 10:44   ` Wolfram Sang
2021-11-29 10:44   ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.