[PATCH] i2c: omap: implement bus recovery

[PATCH] i2c: omap: implement bus recovery

[Felipe Balbi]

implement bus recovery methods for i2c-omap
so we can recover from situations where SCL/SDA
are stuck low.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---

rebased on 4.1-rc2, switched over to generic i2c recover mechanism,
retested on am437x sk and beaglebone black.

 drivers/i2c/busses/i2c-omap.c | 69 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 0e894193accf..dcf559ac92c5 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -481,10 +481,8 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev)
 
 	timeout = jiffies + OMAP_I2C_TIMEOUT;
 	while (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG) & OMAP_I2C_STAT_BB) {
-		if (time_after(jiffies, timeout)) {
-			dev_warn(dev->dev, "timeout waiting for bus ready\n");
-			return -ETIMEDOUT;
-		}
+		if (time_after(jiffies, timeout))
+			return i2c_recover_bus(&dev->adapter);
 		msleep(1);
 	}
 
@@ -1209,6 +1207,68 @@ MODULE_DEVICE_TABLE(of, omap_i2c_of_match);
 #define OMAP_I2C_SCHEME_0		0
 #define OMAP_I2C_SCHEME_1		1
 
+static int omap_i2c_get_scl(struct i2c_adapter *adap)
+{
+	struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
+	u32 reg;
+
+	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+
+	return reg & OMAP_I2C_SYSTEST_SCL_I_FUNC;
+}
+
+static int omap_i2c_get_sda(struct i2c_adapter *adap)
+{
+	struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
+	u32 reg;
+
+	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+
+	return reg & OMAP_I2C_SYSTEST_SDA_I_FUNC;
+}
+
+static void omap_i2c_set_scl(struct i2c_adapter *adap, int val)
+{
+	struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
+	u32 reg;
+
+	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	if (val)
+		reg |= OMAP_I2C_SYSTEST_SCL_O;
+	else
+		reg &= ~OMAP_I2C_SYSTEST_SCL_O;
+	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
+}
+
+static void omap_i2c_prepare_recovery(struct i2c_adapter *adap)
+{
+	struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
+	u32 reg;
+
+	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	reg |= OMAP_I2C_SYSTEST_ST_EN;
+	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
+}
+
+static void omap_i2c_unprepare_recovery(struct i2c_adapter *adap)
+{
+	struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
+	u32 reg;
+
+	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	reg &= ~OMAP_I2C_SYSTEST_ST_EN;
+	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
+}
+
+static struct i2c_bus_recovery_info omap_i2c_bus_recovery_info = {
+	.get_scl		= omap_i2c_get_scl,
+	.get_sda		= omap_i2c_get_sda,
+	.set_scl		= omap_i2c_set_scl,
+	.prepare_recovery	= omap_i2c_prepare_recovery,
+	.unprepare_recovery	= omap_i2c_unprepare_recovery,
+	.recover_bus		= i2c_generic_scl_recovery,
+};
+
 static int
 omap_i2c_probe(struct platform_device *pdev)
 {
@@ -1358,6 +1418,7 @@ omap_i2c_probe(struct platform_device *pdev)
 	adap->algo = &omap_i2c_algo;
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
+	adap->bus_recovery_info = &omap_i2c_bus_recovery_info;
 
 	/* i2c device drivers may be active on return from add_adapter() */
 	adap->nr = pdev->id;
-- 
2.4.0.rc3

Re: [PATCH] i2c: omap: implement bus recovery

[Wolfram Sang]

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

On Wed, May 06, 2015 at 11:50:27AM -0500, Felipe Balbi wrote:
> implement bus recovery methods for i2c-omap
> so we can recover from situations where SCL/SDA
> are stuck low.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Applied to for-next, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] i2c: omap: implement bus recovery

[Felipe Balbi]

If either SCL or SDA are stuck low, we need to
recover the bus using the procedure described
on section 3.1.16 of the I2C specification.

Note that we're trying to implement the procedure
exactly as described by that section. First we
check which line is stuck low, then implement
one or the other procedure. If SDA recovery procedure
fails, we reset our IP in an attempt to make it work.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---

Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with
1000 iterations of i2cdetect on all available buses.

That said, I couldn't get any device to hold the bus busy so I could
see this working. If anybody has any good way of forcing a condition
so that we need bus recovery, I'd be glad to look at.

cheers

 drivers/i2c/busses/i2c-omap.c | 71 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 0e894193accf..c3e4da751adf 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -472,6 +472,73 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 	return 0;
 }
 
+static void omap_i2c_clock_pulse(struct omap_i2c_dev *dev)
+{
+	u32 reg;
+	int i;
+
+	/* Enable testmode */
+	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	reg |= OMAP_I2C_SYSTEST_ST_EN;
+	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
+
+	for (i = 0; i < 9; i++) {
+		reg |= OMAP_I2C_SYSTEST_SCL_O;
+		omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
+		mdelay(100);
+		reg &= ~OMAP_I2C_SYSTEST_SCL_O;
+		omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
+		mdelay(100);
+	}
+
+	/* Disable testmode */
+	reg &= ~OMAP_I2C_SYSTEST_ST_EN;
+	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
+}
+
+static void omap_i2c_bus_recover(struct omap_i2c_dev *dev)
+{
+	u32 reg1;
+	u32 reg2;
+
+	/*
+	 * First differentiate SCL stuck low from SDA stuck low using our
+	 * SYSTEST register. Depending on which line is stuck low, we will
+	 * either Reset our I2C IP (SCL stuck low) or drive 9 clock pulses on
+	 * SCL (SDA stuck low) to tell the device to release the bus.
+	 *
+	 * If, after 9 clock pulses on SCL device still doesn't release the
+	 * bus, there's nothing more we can do; we will still try to Reset
+	 * our I2C IP anyway.
+	 */
+
+	reg1 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	msleep(1);
+	reg2 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+
+	if (!(reg1 & OMAP_I2C_SYSTEST_SCL_I_FUNC) &&
+			!(reg2 & OMAP_I2C_SYSTEST_SCL_I_FUNC)) {
+		dev_err(dev->dev, "SCL is stuck low, resetting\n");
+		omap_i2c_reset(dev);
+	}
+
+	if (!(reg1 & OMAP_I2C_SYSTEST_SDA_I_FUNC) &&
+			!(reg2 & OMAP_I2C_SYSTEST_SDA_I_FUNC)) {
+		dev_err(dev->dev, "SDA is stuck low, driving 9 pulses on SCL\n");
+		omap_i2c_clock_pulse(dev);
+
+		reg1 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+		msleep(1);
+		reg2 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+
+		if ((reg1 & OMAP_I2C_SYSTEST_SDA_I_FUNC) &&
+				(reg2 & OMAP_I2C_SYSTEST_SDA_I_FUNC)) {
+			dev_err(dev->dev, "SDA still stuck, resetting\n");
+			omap_i2c_reset(dev);
+		}
+	}
+}
+
 /*
  * Waiting on Bus Busy
  */
@@ -482,8 +549,8 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev)
 	timeout = jiffies + OMAP_I2C_TIMEOUT;
 	while (omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG) & OMAP_I2C_STAT_BB) {
 		if (time_after(jiffies, timeout)) {
-			dev_warn(dev->dev, "timeout waiting for bus ready\n");
-			return -ETIMEDOUT;
+			omap_i2c_bus_recover(dev);
+			return 0;
 		}
 		msleep(1);
 	}
-- 
2.3.0

Re: [PATCH] i2c: omap: implement bus recovery

[Felipe Balbi]

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

On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote:
> If either SCL or SDA are stuck low, we need to
> recover the bus using the procedure described
> on section 3.1.16 of the I2C specification.
> 
> Note that we're trying to implement the procedure
> exactly as described by that section. First we
> check which line is stuck low, then implement
> one or the other procedure. If SDA recovery procedure
> fails, we reset our IP in an attempt to make it work.
> 
> Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> ---
> 
> Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with
> 1000 iterations of i2cdetect on all available buses.
> 
> That said, I couldn't get any device to hold the bus busy so I could
> see this working. If anybody has any good way of forcing a condition
> so that we need bus recovery, I'd be glad to look at.
> 
> cheers
> 
>  drivers/i2c/busses/i2c-omap.c | 71 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 69 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 0e894193accf..c3e4da751adf 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -472,6 +472,73 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>  	return 0;
>  }
>  
> +static void omap_i2c_clock_pulse(struct omap_i2c_dev *dev)
> +{
> +	u32 reg;
> +	int i;
> +
> +	/* Enable testmode */
> +	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
> +	reg |= OMAP_I2C_SYSTEST_ST_EN;
> +	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
> +
> +	for (i = 0; i < 9; i++) {
> +		reg |= OMAP_I2C_SYSTEST_SCL_O;
> +		omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
> +		mdelay(100);
> +		reg &= ~OMAP_I2C_SYSTEST_SCL_O;
> +		omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
> +		mdelay(100);
> +	}
> +
> +	/* Disable testmode */
> +	reg &= ~OMAP_I2C_SYSTEST_ST_EN;
> +	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
> +}
> +
> +static void omap_i2c_bus_recover(struct omap_i2c_dev *dev)
> +{
> +	u32 reg1;
> +	u32 reg2;
> +
> +	/*
> +	 * First differentiate SCL stuck low from SDA stuck low using our
> +	 * SYSTEST register. Depending on which line is stuck low, we will
> +	 * either Reset our I2C IP (SCL stuck low) or drive 9 clock pulses on
> +	 * SCL (SDA stuck low) to tell the device to release the bus.
> +	 *
> +	 * If, after 9 clock pulses on SCL device still doesn't release the
> +	 * bus, there's nothing more we can do; we will still try to Reset
> +	 * our I2C IP anyway.
> +	 */
> +
> +	reg1 = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
> +	msleep(1);

Hmm... I wonder if this msleep() should be scaled based on i2c bus
frequency.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: omap: implement bus recovery

[Felipe Balbi]

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

On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote:
> If either SCL or SDA are stuck low, we need to
> recover the bus using the procedure described
> on section 3.1.16 of the I2C specification.
> 
> Note that we're trying to implement the procedure
> exactly as described by that section. First we
> check which line is stuck low, then implement
> one or the other procedure. If SDA recovery procedure
> fails, we reset our IP in an attempt to make it work.
> 
> Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> ---
> 
> Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with
> 1000 iterations of i2cdetect on all available buses.
> 
> That said, I couldn't get any device to hold the bus busy so I could
> see this working. If anybody has any good way of forcing a condition
> so that we need bus recovery, I'd be glad to look at.

ping

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: omap: implement bus recovery

[Felipe Balbi]

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

On Mon, Mar 09, 2015 at 11:39:17AM -0500, Felipe Balbi wrote:
> On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote:
> > If either SCL or SDA are stuck low, we need to
> > recover the bus using the procedure described
> > on section 3.1.16 of the I2C specification.
> > 
> > Note that we're trying to implement the procedure
> > exactly as described by that section. First we
> > check which line is stuck low, then implement
> > one or the other procedure. If SDA recovery procedure
> > fails, we reset our IP in an attempt to make it work.
> > 
> > Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> > ---
> > 
> > Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with
> > 1000 iterations of i2cdetect on all available buses.
> > 
> > That said, I couldn't get any device to hold the bus busy so I could
> > see this working. If anybody has any good way of forcing a condition
> > so that we need bus recovery, I'd be glad to look at.
> 
> ping

any comments here ?? Anybody at all ????

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: omap: implement bus recovery

[Grygorii Strashko]

Hi Felipe,

On 03/11/2015 03:50 AM, Felipe Balbi wrote:
> On Mon, Mar 09, 2015 at 11:39:17AM -0500, Felipe Balbi wrote:
>> On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote:
>>> If either SCL or SDA are stuck low, we need to
>>> recover the bus using the procedure described
>>> on section 3.1.16 of the I2C specification.
>>>
>>> Note that we're trying to implement the procedure
>>> exactly as described by that section. First we
>>> check which line is stuck low, then implement
>>> one or the other procedure. If SDA recovery procedure
>>> fails, we reset our IP in an attempt to make it work.
>>>
>>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>>> ---
>>>
>>> Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with
>>> 1000 iterations of i2cdetect on all available buses.
>>>
>>> That said, I couldn't get any device to hold the bus busy so I could
>>> see this working. If anybody has any good way of forcing a condition
>>> so that we need bus recovery, I'd be glad to look at.
>>
>> ping
>
> any comments here ?? Anybody at all ????
>

I think the I2C bus recovery infrastructure should be used here ;)
As I did there https://lkml.org/lkml/2014/12/1/397, but
there are no comments too :(

regards,
-grygorii

Re: [PATCH] i2c: omap: implement bus recovery

[Wolfram Sang]

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

On Wed, Mar 11, 2015 at 03:47:53PM +0200, Grygorii Strashko wrote:
> Hi Felipe,
> 
> On 03/11/2015 03:50 AM, Felipe Balbi wrote:
> >On Mon, Mar 09, 2015 at 11:39:17AM -0500, Felipe Balbi wrote:
> >>On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote:
> >>>If either SCL or SDA are stuck low, we need to
> >>>recover the bus using the procedure described
> >>>on section 3.1.16 of the I2C specification.
> >>>
> >>>Note that we're trying to implement the procedure
> >>>exactly as described by that section. First we
> >>>check which line is stuck low, then implement
> >>>one or the other procedure. If SDA recovery procedure
> >>>fails, we reset our IP in an attempt to make it work.
> >>>
> >>>Signed-off-by: Felipe Balbi <balbi@ti.com>
> >>>---
> >>>
> >>>Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with
> >>>1000 iterations of i2cdetect on all available buses.
> >>>
> >>>That said, I couldn't get any device to hold the bus busy so I could
> >>>see this working. If anybody has any good way of forcing a condition
> >>>so that we need bus recovery, I'd be glad to look at.
> >>
> >>ping
> >
> >any comments here ?? Anybody at all ????
> >
> 
> I think the I2C bus recovery infrastructure should be used here ;)
> As I did there https://lkml.org/lkml/2014/12/1/397, but
> there are no comments too :(

Sorry, guys, a lot of stuff going on in I2C. Bus recovery needs a more
generic look. I'll try, but can't promise for 4.1. If it fails 4.1., it
will get priority for 4.2.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: omap: implement bus recovery

[Wolfram Sang]

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

On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote:
> If either SCL or SDA are stuck low, we need to
> recover the bus using the procedure described
> on section 3.1.16 of the I2C specification.
> 
> Note that we're trying to implement the procedure
> exactly as described by that section. First we
> check which line is stuck low, then implement
> one or the other procedure. If SDA recovery procedure
> fails, we reset our IP in an attempt to make it work.
> 
> Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>

As Grygorii already mentioned: can you convert it to the standard i2c
bus recovery mechanism?

And is the timeout you replace with the recovery caused by SDA stuck
high (check the thread starting with
http://thread.gmane.org/gmane.linux.kernel/1841371/focus=22435)

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] i2c: omap: implement bus recovery

[Felipe Balbi]

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

On Fri, Apr 10, 2015 at 11:41:22PM +0200, Wolfram Sang wrote:
> On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote:
> > If either SCL or SDA are stuck low, we need to
> > recover the bus using the procedure described
> > on section 3.1.16 of the I2C specification.
> > 
> > Note that we're trying to implement the procedure
> > exactly as described by that section. First we
> > check which line is stuck low, then implement
> > one or the other procedure. If SDA recovery procedure
> > fails, we reset our IP in an attempt to make it work.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> As Grygorii already mentioned: can you convert it to the standard i2c
> bus recovery mechanism?
> 
> And is the timeout you replace with the recovery caused by SDA stuck
> high (check the thread starting with
> http://thread.gmane.org/gmane.linux.kernel/1841371/focus=22435)

SDA stuck low is one reason, yes. There could be other reasons like the
far end (i2c client) just crapping out. I know of one touchscreen
controller which will just die if you continuously try to read from
unexistent registers. It will NAK for a while and later just die,
completely. The only way to recover from that today is resetting the
board. I have a feeling that resetting the touch controller might be
doable if we have a reset pin tied to a GPIO, haven't tried that yet. In
any case this is off-topic.

I'll rebase the patch and fix what you asked, then retest, and resend.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]