[PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Viresh Kumar]

Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c
protocol Rev. 03 section 3.1.16 titled "Bus clear".

http://www.nxp.com/documents/user_manual/UM10204.pdf

Sometimes during operation i2c bus hangs and we need to give dummy clocks to
slave device to start the transfer again. Now we may have capability in the bus
controller to generate these clocks or platform may have gpio pins which can be
toggled to generate dummy clocks. This patch supports both.

This patch also adds in generic bus recovery routines gpio or scl line based
which can be used by bus controller. In addition controller driver may provide
its own version of the bus recovery routine.

This doesn't support multi-master recovery for now.

Reviewed-by: Paul Carpenter <paul-YHLC2tV1sDlxR4N9A70vTlRxknfHcPLb9dF7HbQ/qKg@public.gmane.org>
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Most number of versions for any patchset i submitted :)

V8->V9:
- removed skip_sda_polling variable
- simplified/fixed i2c_generic_recovery() routine
- added new routine i2c_recover_bus()
- added gpio_is_valid() checks over scl and sda gpios
- code is rearranged at places (based on Wolfram's comments)

 drivers/i2c/i2c-core.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h    |  39 +++++++++++++
 2 files changed, 194 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..8076e52 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -27,7 +27,9 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/delay.h>
 #include <linux/errno.h>
+#include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
@@ -109,6 +111,127 @@ static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 #define i2c_device_uevent	NULL
 #endif	/* CONFIG_HOTPLUG */
 
+/* i2c bus recovery routines */
+#define RECOVERY_CLK_CNT	9
+#define DEFAULT_CLK_RATE	100	/* KHz */
+/* 10^6/KHz for delay in ns */
+unsigned long clk_delay = DIV_ROUND_UP(1000000, DEFAULT_CLK_RATE * 2);
+
+static int get_scl_gpio_value(struct i2c_adapter *adap)
+{
+	return gpio_get_value(adap->bus_recovery_info->scl_gpio);
+}
+
+static void set_scl_gpio_value(struct i2c_adapter *adap, int val)
+{
+	gpio_set_value(adap->bus_recovery_info->scl_gpio, val);
+}
+
+static int get_sda_gpio_value(struct i2c_adapter *adap)
+{
+	return gpio_get_value(adap->bus_recovery_info->sda_gpio);
+}
+
+static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
+{
+	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+	struct device *dev = &adap->dev;
+	int ret = 0;
+
+	ret = gpio_request_one(bri->scl_gpio, GPIOF_OPEN_DRAIN |
+			GPIOF_OUT_INIT_HIGH, "i2c-scl");
+	if (ret) {
+		dev_warn(dev, "gpio request fail: %d\n", bri->scl_gpio);
+		return ret;
+	}
+
+	if (bri->get_sda) {
+		if (gpio_request_one(bri->sda_gpio, GPIOF_IN, "i2c-sda")) {
+			/* work without sda polling */
+			dev_warn(dev, "can't get sda: %d. Skip sda polling\n",
+					bri->sda_gpio);
+			bri->get_sda = NULL;
+		}
+	}
+
+	return ret;
+}
+
+static void i2c_put_gpios_for_recovery(struct i2c_adapter *adap)
+{
+	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+
+	if (bri->get_sda)
+		gpio_free(bri->sda_gpio);
+
+	gpio_free(bri->scl_gpio);
+}
+
+static int i2c_generic_recovery(struct i2c_adapter *adap)
+{
+	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+	int i = 0, val = 1, ret = 0;
+
+	if (bri->prepare_recovery)
+		bri->prepare_recovery(bri);
+
+	/*
+	 * By this time SCL is high, as we need to give 9 falling-rising edges
+	 */
+	while (i++ < RECOVERY_CLK_CNT * 2) {
+		/* SCL shouldn't be low here */
+		if (val && !bri->get_scl(adap)) {
+			dev_err(&adap->dev, "SCL is stuck Low exit recovery\n");
+			ret = -EBUSY;
+			goto unprepare;
+		}
+
+		val = !val;
+		bri->set_scl(adap, val);
+		ndelay(clk_delay);
+
+		/* break if sda got high, check only when scl line is high */
+		if (bri->get_sda && val)
+			if (bri->get_sda(adap))
+				break;
+	}
+
+unprepare:
+	if (bri->unprepare_recovery)
+		bri->unprepare_recovery(bri);
+
+	return ret;
+}
+
+int i2c_generic_scl_recovery(struct i2c_adapter *adap)
+{
+	adap->bus_recovery_info->set_scl(adap, 1);
+	return i2c_generic_recovery(adap);
+}
+
+int i2c_generic_gpio_recovery(struct i2c_adapter *adap)
+{
+	int ret;
+
+	ret = i2c_get_gpios_for_recovery(adap);
+	if (ret)
+		return ret;
+
+	ret = i2c_generic_recovery(adap);
+	i2c_put_gpios_for_recovery(adap);
+
+	return ret;
+}
+
+int i2c_recover_bus(struct i2c_adapter *adap)
+{
+	if (!adap->bus_recovery_info)
+		return -EOPNOTSUPP;
+
+	dev_dbg(&adap->dev, "try i2c bus recovery\n");
+	return adap->bus_recovery_info->recover_bus(adap);
+}
+
 static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
@@ -902,6 +1025,38 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 			 "Failed to create compatibility class link\n");
 #endif
 
+	/* bus recovery specific initialization */
+	if (adap->bus_recovery_info) {
+		struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+
+		if (!bri->recover_bus) {
+			dev_err(&adap->dev, "Invalid recover_bus(), skip recovery\n");
+			adap->bus_recovery_info = NULL;
+			goto exit_recovery;
+		}
+
+		/* GPIO recovery */
+		if (bri->recover_bus == i2c_generic_gpio_recovery) {
+			if (!gpio_is_valid(bri->scl_gpio)) {
+				dev_err(&adap->dev, "Invalid scl gpio, skip recovery\n");
+				adap->bus_recovery_info = NULL;
+				goto exit_recovery;
+			}
+
+			if (gpio_is_valid(bri->sda_gpio))
+				bri->get_sda = get_sda_gpio_value;
+			else
+				bri->get_sda = NULL;
+
+			bri->get_scl = get_scl_gpio_value;
+			bri->set_scl = set_scl_gpio_value;
+		} else if (!bri->set_scl) {
+			dev_warn(&adap->dev,
+					"set_scl() is must for scl recovery\n");
+		}
+	}
+
+exit_recovery:
 	/* create pre-declared device nodes */
 	if (adap->nr < __i2c_first_dynamic_bus_num)
 		i2c_scan_static_board_info(adap);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index d0c4db7..da9358b 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -370,6 +370,43 @@ struct i2c_algorithm {
 	u32 (*functionality) (struct i2c_adapter *);
 };
 
+/**
+ * struct i2c_bus_recovery_info - I2C bus recovery information
+ * @recover_bus: Recover routine. Either pass driver's recover_bus() routine, or
+ *	i2c_generic_scl_recovery() or i2c_generic_gpio_recovery().
+ * @get_scl: This gets current value of scl line. Mandatory for generic SCL
+ *      recovery. Used internally for generic GPIO recovery.
+ * @set_scl: This sets/clears scl line. Mandatory for generic SCL recovery. Used
+ *      internally for generic GPIO recovery.
+ * @get_sda: This gets current value of sda line. Optional for generic SCL
+ *      recovery. Used internally, if sda_gpio != -1, for generic GPIO recovery.
+ * @prepare_recovery: This will be called before starting recovery. Platform may
+ *	configure padmux here for sda/scl line or something else they want.
+ * @unprepare_recovery: This will be called after completing recovery. Platform
+ *	may configure padmux here for sda/scl line or something else they want.
+ * @scl_gpio: gpio number of the scl line. Only required for GPIO recovery.
+ * @sda_gpio: gpio number of the sda line. Only required for GPIO recovery.
+ */
+struct i2c_bus_recovery_info {
+	int (*recover_bus)(struct i2c_adapter *);
+
+	int (*get_scl)(struct i2c_adapter *);
+	void (*set_scl)(struct i2c_adapter *, int val);
+	int (*get_sda)(struct i2c_adapter *);
+
+	void (*prepare_recovery)(struct i2c_bus_recovery_info *bri);
+	void (*unprepare_recovery)(struct i2c_bus_recovery_info *bri);
+
+	/* gpio recovery */
+	unsigned scl_gpio;
+	unsigned sda_gpio;
+};
+
+/* Generic recovery routines */
+int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
+int i2c_generic_scl_recovery(struct i2c_adapter *adap);
+int i2c_recover_bus(struct i2c_adapter *adap);
+
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.
@@ -393,6 +430,8 @@ struct i2c_adapter {
 
 	struct mutex userspace_clients_lock;
 	struct list_head userspace_clients;
+
+	struct i2c_bus_recovery_info *bus_recovery_info;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
-- 
1.7.12.rc2.18.g61b472e

[PATCH V9 2/2] i2c/designware: Provide i2c bus recovery support

[Viresh Kumar]

Add bus recovery support for designware_i2c controller. It uses generic gpio
based i2c_gpio_recover_bus() routine. Platforms need to pass struct
i2c_bus_recovery_info as platform data to designware I2C controller.

Signed-off-by: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
Signed-off-by: Shiraz Hashim <shiraz.hashim-qxv4g6HH51o@public.gmane.org>
Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
V8->V9:
- Use i2c_recover_bus()
- simplified dw_i2c_probe() for bus recovery code
- removed floating kfree()'s of adap->bus_recovery_info

 drivers/i2c/busses/i2c-designware-core.c    | 5 ++++-
 drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index f5258c2..d0423ef 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -539,7 +539,10 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete, HZ);
 	if (ret == 0) {
 		dev_err(dev->dev, "controller timed out\n");
-		i2c_dw_init(dev);
+
+		if (i2c_recover_bus(adap) < 0)
+			i2c_dw_init(dev);
+
 		ret = -ETIMEDOUT;
 		goto done;
 	} else if (ret < 0)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 343357a..9142f0c 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -141,6 +141,12 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
+	/* Bus recovery support */
+	adap->bus_recovery_info = dev_get_platdata(&pdev->dev);
+	if (adap->bus_recovery_info)
+		adap->bus_recovery_info->recover_bus =
+			i2c_generic_gpio_recovery;
+
 	adap->nr = pdev->id;
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Viresh Kumar]

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

On 24 January 2013 16:06, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c
> protocol Rev. 03 section 3.1.16 titled "Bus clear".
>
> http://www.nxp.com/documents/user_manual/UM10204.pdf
>
> Sometimes during operation i2c bus hangs and we need to give dummy clocks to
> slave device to start the transfer again. Now we may have capability in the bus
> controller to generate these clocks or platform may have gpio pins which can be
> toggled to generate dummy clocks. This patch supports both.
>
> This patch also adds in generic bus recovery routines gpio or scl line based
> which can be used by bus controller. In addition controller driver may provide
> its own version of the bus recovery routine.
>
> This doesn't support multi-master recovery for now.
>
> Reviewed-by: Paul Carpenter <paul-YHLC2tV1sDlxR4N9A70vTlRxknfHcPLb9dF7HbQ/qKg@public.gmane.org>
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

ARM mail servers are broken now a days, so these patches may not apply
from mail.
Find them attached.

[-- Attachment #2: 0001-i2c-adapter-Add-bus-recovery-infrastructure.patch --]
[-- Type: application/octet-stream, Size: 8651 bytes --]

From fe4fba74b7bac4fe56e26b2a947280390182c66c Mon Sep 17 00:00:00 2001
Message-Id: <fe4fba74b7bac4fe56e26b2a947280390182c66c.1359023434.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Tue, 28 Feb 2012 18:26:31 +0530
Subject: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c
protocol Rev. 03 section 3.1.16 titled "Bus clear".

http://www.nxp.com/documents/user_manual/UM10204.pdf

Sometimes during operation i2c bus hangs and we need to give dummy clocks to
slave device to start the transfer again. Now we may have capability in the bus
controller to generate these clocks or platform may have gpio pins which can be
toggled to generate dummy clocks. This patch supports both.

This patch also adds in generic bus recovery routines gpio or scl line based
which can be used by bus controller. In addition controller driver may provide
its own version of the bus recovery routine.

This doesn't support multi-master recovery for now.

Reviewed-by: Paul Carpenter <paul@pcserviceselectronics.co.uk>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Most number of versions for any patchset i submitted :)

V8->V9:
- removed skip_sda_polling variable
- simplified/fixed i2c_generic_recovery() routine
- added new routine i2c_recover_bus()
- added gpio_is_valid() checks over scl and sda gpios
- code is rearranged at places (based on Wolfram's comments)

 drivers/i2c/i2c-core.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h    |  39 +++++++++++++
 2 files changed, 194 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..8076e52 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -27,7 +27,9 @@
 
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/delay.h>
 #include <linux/errno.h>
+#include <linux/gpio.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
@@ -109,6 +111,127 @@ static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 #define i2c_device_uevent	NULL
 #endif	/* CONFIG_HOTPLUG */
 
+/* i2c bus recovery routines */
+#define RECOVERY_CLK_CNT	9
+#define DEFAULT_CLK_RATE	100	/* KHz */
+/* 10^6/KHz for delay in ns */
+unsigned long clk_delay = DIV_ROUND_UP(1000000, DEFAULT_CLK_RATE * 2);
+
+static int get_scl_gpio_value(struct i2c_adapter *adap)
+{
+	return gpio_get_value(adap->bus_recovery_info->scl_gpio);
+}
+
+static void set_scl_gpio_value(struct i2c_adapter *adap, int val)
+{
+	gpio_set_value(adap->bus_recovery_info->scl_gpio, val);
+}
+
+static int get_sda_gpio_value(struct i2c_adapter *adap)
+{
+	return gpio_get_value(adap->bus_recovery_info->sda_gpio);
+}
+
+static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
+{
+	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+	struct device *dev = &adap->dev;
+	int ret = 0;
+
+	ret = gpio_request_one(bri->scl_gpio, GPIOF_OPEN_DRAIN |
+			GPIOF_OUT_INIT_HIGH, "i2c-scl");
+	if (ret) {
+		dev_warn(dev, "gpio request fail: %d\n", bri->scl_gpio);
+		return ret;
+	}
+
+	if (bri->get_sda) {
+		if (gpio_request_one(bri->sda_gpio, GPIOF_IN, "i2c-sda")) {
+			/* work without sda polling */
+			dev_warn(dev, "can't get sda: %d. Skip sda polling\n",
+					bri->sda_gpio);
+			bri->get_sda = NULL;
+		}
+	}
+
+	return ret;
+}
+
+static void i2c_put_gpios_for_recovery(struct i2c_adapter *adap)
+{
+	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+
+	if (bri->get_sda)
+		gpio_free(bri->sda_gpio);
+
+	gpio_free(bri->scl_gpio);
+}
+
+static int i2c_generic_recovery(struct i2c_adapter *adap)
+{
+	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+	int i = 0, val = 1, ret = 0;
+
+	if (bri->prepare_recovery)
+		bri->prepare_recovery(bri);
+
+	/*
+	 * By this time SCL is high, as we need to give 9 falling-rising edges
+	 */
+	while (i++ < RECOVERY_CLK_CNT * 2) {
+		/* SCL shouldn't be low here */
+		if (val && !bri->get_scl(adap)) {
+			dev_err(&adap->dev, "SCL is stuck Low exit recovery\n");
+			ret = -EBUSY;
+			goto unprepare;
+		}
+
+		val = !val;
+		bri->set_scl(adap, val);
+		ndelay(clk_delay);
+
+		/* break if sda got high, check only when scl line is high */
+		if (bri->get_sda && val)
+			if (bri->get_sda(adap))
+				break;
+	}
+
+unprepare:
+	if (bri->unprepare_recovery)
+		bri->unprepare_recovery(bri);
+
+	return ret;
+}
+
+int i2c_generic_scl_recovery(struct i2c_adapter *adap)
+{
+	adap->bus_recovery_info->set_scl(adap, 1);
+	return i2c_generic_recovery(adap);
+}
+
+int i2c_generic_gpio_recovery(struct i2c_adapter *adap)
+{
+	int ret;
+
+	ret = i2c_get_gpios_for_recovery(adap);
+	if (ret)
+		return ret;
+
+	ret = i2c_generic_recovery(adap);
+	i2c_put_gpios_for_recovery(adap);
+
+	return ret;
+}
+
+int i2c_recover_bus(struct i2c_adapter *adap)
+{
+	if (!adap->bus_recovery_info)
+		return -EOPNOTSUPP;
+
+	dev_dbg(&adap->dev, "try i2c bus recovery\n");
+	return adap->bus_recovery_info->recover_bus(adap);
+}
+
 static int i2c_device_probe(struct device *dev)
 {
 	struct i2c_client	*client = i2c_verify_client(dev);
@@ -902,6 +1025,38 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 			 "Failed to create compatibility class link\n");
 #endif
 
+	/* bus recovery specific initialization */
+	if (adap->bus_recovery_info) {
+		struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
+
+		if (!bri->recover_bus) {
+			dev_err(&adap->dev, "Invalid recover_bus(), skip recovery\n");
+			adap->bus_recovery_info = NULL;
+			goto exit_recovery;
+		}
+
+		/* GPIO recovery */
+		if (bri->recover_bus == i2c_generic_gpio_recovery) {
+			if (!gpio_is_valid(bri->scl_gpio)) {
+				dev_err(&adap->dev, "Invalid scl gpio, skip recovery\n");
+				adap->bus_recovery_info = NULL;
+				goto exit_recovery;
+			}
+
+			if (gpio_is_valid(bri->sda_gpio))
+				bri->get_sda = get_sda_gpio_value;
+			else
+				bri->get_sda = NULL;
+
+			bri->get_scl = get_scl_gpio_value;
+			bri->set_scl = set_scl_gpio_value;
+		} else if (!bri->set_scl) {
+			dev_warn(&adap->dev,
+					"set_scl() is must for scl recovery\n");
+		}
+	}
+
+exit_recovery:
 	/* create pre-declared device nodes */
 	if (adap->nr < __i2c_first_dynamic_bus_num)
 		i2c_scan_static_board_info(adap);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index d0c4db7..da9358b 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -370,6 +370,43 @@ struct i2c_algorithm {
 	u32 (*functionality) (struct i2c_adapter *);
 };
 
+/**
+ * struct i2c_bus_recovery_info - I2C bus recovery information
+ * @recover_bus: Recover routine. Either pass driver's recover_bus() routine, or
+ *	i2c_generic_scl_recovery() or i2c_generic_gpio_recovery().
+ * @get_scl: This gets current value of scl line. Mandatory for generic SCL
+ *      recovery. Used internally for generic GPIO recovery.
+ * @set_scl: This sets/clears scl line. Mandatory for generic SCL recovery. Used
+ *      internally for generic GPIO recovery.
+ * @get_sda: This gets current value of sda line. Optional for generic SCL
+ *      recovery. Used internally, if sda_gpio != -1, for generic GPIO recovery.
+ * @prepare_recovery: This will be called before starting recovery. Platform may
+ *	configure padmux here for sda/scl line or something else they want.
+ * @unprepare_recovery: This will be called after completing recovery. Platform
+ *	may configure padmux here for sda/scl line or something else they want.
+ * @scl_gpio: gpio number of the scl line. Only required for GPIO recovery.
+ * @sda_gpio: gpio number of the sda line. Only required for GPIO recovery.
+ */
+struct i2c_bus_recovery_info {
+	int (*recover_bus)(struct i2c_adapter *);
+
+	int (*get_scl)(struct i2c_adapter *);
+	void (*set_scl)(struct i2c_adapter *, int val);
+	int (*get_sda)(struct i2c_adapter *);
+
+	void (*prepare_recovery)(struct i2c_bus_recovery_info *bri);
+	void (*unprepare_recovery)(struct i2c_bus_recovery_info *bri);
+
+	/* gpio recovery */
+	unsigned scl_gpio;
+	unsigned sda_gpio;
+};
+
+/* Generic recovery routines */
+int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
+int i2c_generic_scl_recovery(struct i2c_adapter *adap);
+int i2c_recover_bus(struct i2c_adapter *adap);
+
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.
@@ -393,6 +430,8 @@ struct i2c_adapter {
 
 	struct mutex userspace_clients_lock;
 	struct list_head userspace_clients;
+
+	struct i2c_bus_recovery_info *bus_recovery_info;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
-- 
1.7.12.rc2.18.g61b472e


[-- Attachment #3: 0002-i2c-designware-Provide-i2c-bus-recovery-support.patch --]
[-- Type: application/octet-stream, Size: 2465 bytes --]

From 59d21326020cdd28a624ed283b294ec65a175c92 Mon Sep 17 00:00:00 2001
Message-Id: <59d21326020cdd28a624ed283b294ec65a175c92.1359023434.git.viresh.kumar@linaro.org>
In-Reply-To: <fe4fba74b7bac4fe56e26b2a947280390182c66c.1359023434.git.viresh.kumar@linaro.org>
References: <fe4fba74b7bac4fe56e26b2a947280390182c66c.1359023434.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Wed, 29 Feb 2012 23:16:15 +0530
Subject: [PATCH V9 2/2] i2c/designware: Provide i2c bus recovery support

Add bus recovery support for designware_i2c controller. It uses generic gpio
based i2c_gpio_recover_bus() routine. Platforms need to pass struct
i2c_bus_recovery_info as platform data to designware I2C controller.

Signed-off-by: Vincenzo Frascino <vincenzo.frascino@st.com>
Signed-off-by: Shiraz Hashim <shiraz.hashim@st.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V8->V9:
- Use i2c_recover_bus()
- simplified dw_i2c_probe() for bus recovery code
- removed floating kfree()'s of adap->bus_recovery_info

 drivers/i2c/busses/i2c-designware-core.c    | 5 ++++-
 drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index f5258c2..d0423ef 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -539,7 +539,10 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete, HZ);
 	if (ret == 0) {
 		dev_err(dev->dev, "controller timed out\n");
-		i2c_dw_init(dev);
+
+		if (i2c_recover_bus(adap) < 0)
+			i2c_dw_init(dev);
+
 		ret = -ETIMEDOUT;
 		goto done;
 	} else if (ret < 0)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 343357a..9142f0c 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -141,6 +141,12 @@ static int dw_i2c_probe(struct platform_device *pdev)
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
+	/* Bus recovery support */
+	adap->bus_recovery_info = dev_get_platdata(&pdev->dev);
+	if (adap->bus_recovery_info)
+		adap->bus_recovery_info->recover_bus =
+			i2c_generic_gpio_recovery;
+
 	adap->nr = pdev->id;
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
-- 
1.7.12.rc2.18.g61b472e

Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Uwe Kleine-König]

Hello,

On Thu, Jan 24, 2013 at 04:06:37PM +0530, Viresh Kumar wrote:
> Most number of versions for any patchset i submitted :)
So let's see if you do a v10 ... :-)
 
> +static int i2c_generic_recovery(struct i2c_adapter *adap)
> +{
> +	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +	int i = 0, val = 1, ret = 0;
> +
> +	if (bri->prepare_recovery)
> +		bri->prepare_recovery(bri);
> +
Do we want to break out here if sda is high?

> +	/*
> +	 * By this time SCL is high, as we need to give 9 falling-rising edges
> +	 */
> +	while (i++ < RECOVERY_CLK_CNT * 2) {
> +		/* SCL shouldn't be low here */
> +		if (val && !bri->get_scl(adap)) {
> +			dev_err(&adap->dev, "SCL is stuck Low exit recovery\n");
> +			ret = -EBUSY;
> +			goto unprepare;
> +		}
> +
> +		val = !val;
> +		bri->set_scl(adap, val);
> +		ndelay(clk_delay);
> +
> +		/* break if sda got high, check only when scl line is high */
Above you wrote "SCL", here "scl". I suggest to use one of them
consistently and use the same capitalisation for sda.

> +		if (bri->get_sda && val)
> +			if (bri->get_sda(adap))
> +				break;
Maybe better:
		/* break if sda got high */
		if (bri->get_sda && bri->get_sda(adap)) {
			/* don't leave with scl low */
			if (!val)
				bri->set_scl(adap, 1);
			break;
		}

> +	}
> +
> +unprepare:
> +	if (bri->unprepare_recovery)
> +		bri->unprepare_recovery(bri);
> +
> +	return ret;
> +}
> +
> +int i2c_generic_scl_recovery(struct i2c_adapter *adap)
> +{
> +	adap->bus_recovery_info->set_scl(adap, 1);
Why this?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Viresh Kumar]

On 24 January 2013 16:36, Uwe Kleine-König
<u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> On Thu, Jan 24, 2013 at 04:06:37PM +0530, Viresh Kumar wrote:
>> Most number of versions for any patchset i submitted :)
> So let's see if you do a v10 ... :-)

Haha..

>> +static int i2c_generic_recovery(struct i2c_adapter *adap)
>> +{
>> +     struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
>> +     int i = 0, val = 1, ret = 0;
>> +
>> +     if (bri->prepare_recovery)
>> +             bri->prepare_recovery(bri);
>> +
> Do we want to break out here if sda is high?

Good point actually. It may happen we are asked to recover already working
system :)

>> +     /*
>> +      * By this time SCL is high, as we need to give 9 falling-rising edges
>> +      */
>> +     while (i++ < RECOVERY_CLK_CNT * 2) {
>> +             /* SCL shouldn't be low here */
>> +             if (val && !bri->get_scl(adap)) {
>> +                     dev_err(&adap->dev, "SCL is stuck Low exit recovery\n");
>> +                     ret = -EBUSY;
>> +                     goto unprepare;
>> +             }
>> +
>> +             val = !val;
>> +             bri->set_scl(adap, val);
>> +             ndelay(clk_delay);
>> +
>> +             /* break if sda got high, check only when scl line is high */
> Above you wrote "SCL", here "scl". I suggest to use one of them
> consistently and use the same capitalisation for sda.

Sure.

>> +             if (bri->get_sda && val)
>> +                     if (bri->get_sda(adap))
>> +                             break;
> Maybe better:
>                 /* break if sda got high */
>                 if (bri->get_sda && bri->get_sda(adap)) {
>                         /* don't leave with scl low */
>                         if (!val)
>                                 bri->set_scl(adap, 1);
>                         break;
>                 }

So, the whole loop is for 9 pulses at max and it runs 18 times. Twice per
clock. With my code, i only check for sda after supplying full clock pulse,
and you are asking me to check that in middle of clock. Isn't it wrong?

>> +int i2c_generic_scl_recovery(struct i2c_adapter *adap)
>> +{
>> +     adap->bus_recovery_info->set_scl(adap, 1);
> Why this?

This came out of some earlier discussion we had. We should start
with high and then do low-high 9 times.

Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Uwe Kleine-König]

Hello,

On Thu, Jan 24, 2013 at 04:47:53PM +0530, Viresh Kumar wrote:
> On 24 January 2013 16:36, Uwe Kleine-König
> <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> > On Thu, Jan 24, 2013 at 04:06:37PM +0530, Viresh Kumar wrote:
> >> Most number of versions for any patchset i submitted :)
> > So let's see if you do a v10 ... :-)
> 
> Haha..
> 
> >> +static int i2c_generic_recovery(struct i2c_adapter *adap)
> >> +{
> >> +     struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> >> +     int i = 0, val = 1, ret = 0;
> >> +
> >> +     if (bri->prepare_recovery)
> >> +             bri->prepare_recovery(bri);
> >> +
> > Do we want to break out here if sda is high?
> 
> Good point actually. It may happen we are asked to recover already working
> system :)
> 
> >> +     /*
> >> +      * By this time SCL is high, as we need to give 9 falling-rising edges
> >> +      */
> >> +     while (i++ < RECOVERY_CLK_CNT * 2) {
> >> +             /* SCL shouldn't be low here */
> >> +             if (val && !bri->get_scl(adap)) {
> >> +                     dev_err(&adap->dev, "SCL is stuck Low exit recovery\n");
> >> +                     ret = -EBUSY;
> >> +                     goto unprepare;
> >> +             }
> >> +
> >> +             val = !val;
> >> +             bri->set_scl(adap, val);
> >> +             ndelay(clk_delay);
> >> +
> >> +             /* break if sda got high, check only when scl line is high */
> > Above you wrote "SCL", here "scl". I suggest to use one of them
> > consistently and use the same capitalisation for sda.
> 
> Sure.
> 
> >> +             if (bri->get_sda && val)
> >> +                     if (bri->get_sda(adap))
> >> +                             break;
> > Maybe better:
> >                 /* break if sda got high */
> >                 if (bri->get_sda && bri->get_sda(adap)) {
> >                         /* don't leave with scl low */
> >                         if (!val)
> >                                 bri->set_scl(adap, 1);
> >                         break;
> >                 }
> 
> So, the whole loop is for 9 pulses at max and it runs 18 times. Twice per
> clock. With my code, i only check for sda after supplying full clock pulse,
> and you are asking me to check that in middle of clock. Isn't it wrong?
Actually in the data phase of a transfer sda only changes when scl is
low. (Otherwise it's a stop condition.) So it should make sense to check
after pulling scl low, doesn't it? Hmm, it trades one ndelay for
(possibly several) get_sda. hmm, *shrug*.

> >> +int i2c_generic_scl_recovery(struct i2c_adapter *adap)
> >> +{
> >> +     adap->bus_recovery_info->set_scl(adap, 1);
> > Why this?
> 
> This came out of some earlier discussion we had. We should start
> with high and then do low-high 9 times.
Ah, I missed the first val = !val and so thought you start with setting
to 1 anyhow. So yes, I agree. Maybe document this assumption in a
comment?

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Viresh Kumar]

On 24 January 2013 17:13, Uwe Kleine-König
<u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> On Thu, Jan 24, 2013 at 04:47:53PM +0530, Viresh Kumar wrote:
>> So, the whole loop is for 9 pulses at max and it runs 18 times. Twice per
>> clock. With my code, i only check for sda after supplying full clock pulse,
>> and you are asking me to check that in middle of clock. Isn't it wrong?
> Actually in the data phase of a transfer sda only changes when scl is
> low. (Otherwise it's a stop condition.) So it should make sense to check
> after pulling scl low, doesn't it? Hmm, it trades one ndelay for
> (possibly several) get_sda. hmm, *shrug*.

In any case, we have to make scl high again. So, why not check it at
that stage only?

>> >> +int i2c_generic_scl_recovery(struct i2c_adapter *adap)
>> >> +{
>> >> +     adap->bus_recovery_info->set_scl(adap, 1);
>> > Why this?
>>
>> This came out of some earlier discussion we had. We should start
>> with high and then do low-high 9 times.
> Ah, I missed the first val = !val and so thought you start with setting
> to 1 anyhow. So yes, I agree. Maybe document this assumption in a
> comment?

Some sort of comment is present in the routine which toggles the line,
which says "by this time scl must be high and we should apply 9 pulses
now"

Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Wolfram Sang]

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

On Thu, Jan 24, 2013 at 04:06:37PM +0530, Viresh Kumar wrote:
> Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c
> protocol Rev. 03 section 3.1.16 titled "Bus clear".
> 
> http://www.nxp.com/documents/user_manual/UM10204.pdf
> 
> Sometimes during operation i2c bus hangs and we need to give dummy clocks to
> slave device to start the transfer again. Now we may have capability in the bus
> controller to generate these clocks or platform may have gpio pins which can be
> toggled to generate dummy clocks. This patch supports both.
> 
> This patch also adds in generic bus recovery routines gpio or scl line based
> which can be used by bus controller. In addition controller driver may provide
> its own version of the bus recovery routine.
> 
> This doesn't support multi-master recovery for now.
> 
> Reviewed-by: Paul Carpenter <paul-YHLC2tV1sDlxR4N9A70vTlRxknfHcPLb9dF7HbQ/qKg@public.gmane.org>
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Most number of versions for any patchset i submitted :)
> 
> V8->V9:
> - removed skip_sda_polling variable
> - simplified/fixed i2c_generic_recovery() routine
> - added new routine i2c_recover_bus()
> - added gpio_is_valid() checks over scl and sda gpios
> - code is rearranged at places (based on Wolfram's comments)
> 
>  drivers/i2c/i2c-core.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h    |  39 +++++++++++++
>  2 files changed, 194 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index e388590..8076e52 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -27,7 +27,9 @@
>  
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> +#include <linux/delay.h>
>  #include <linux/errno.h>
> +#include <linux/gpio.h>
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
>  #include <linux/init.h>
> @@ -109,6 +111,127 @@ static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env)
>  #define i2c_device_uevent	NULL
>  #endif	/* CONFIG_HOTPLUG */
>  
> +/* i2c bus recovery routines */
> +#define RECOVERY_CLK_CNT	9
> +#define DEFAULT_CLK_RATE	100	/* KHz */

RECOVERY_CLK_RATE

> +/* 10^6/KHz for delay in ns */
> +unsigned long clk_delay = DIV_ROUND_UP(1000000, DEFAULT_CLK_RATE * 2);

Since all inputs are constant values, the result will be a constant as
well. We might even get rid of this variable altogether.

> +
> +static int get_scl_gpio_value(struct i2c_adapter *adap)
> +{
> +	return gpio_get_value(adap->bus_recovery_info->scl_gpio);
> +}
> +
> +static void set_scl_gpio_value(struct i2c_adapter *adap, int val)
> +{
> +	gpio_set_value(adap->bus_recovery_info->scl_gpio, val);
> +}
> +
> +static int get_sda_gpio_value(struct i2c_adapter *adap)
> +{
> +	return gpio_get_value(adap->bus_recovery_info->sda_gpio);
> +}
> +
> +static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
> +{
> +	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +	struct device *dev = &adap->dev;
> +	int ret = 0;
> +
> +	ret = gpio_request_one(bri->scl_gpio, GPIOF_OPEN_DRAIN |
> +			GPIOF_OUT_INIT_HIGH, "i2c-scl");
> +	if (ret) {
> +		dev_warn(dev, "gpio request fail: %d\n", bri->scl_gpio);
> +		return ret;
> +	}
> +
> +	if (bri->get_sda) {
> +		if (gpio_request_one(bri->sda_gpio, GPIOF_IN, "i2c-sda")) {
> +			/* work without sda polling */
> +			dev_warn(dev, "can't get sda: %d. Skip sda polling\n",
> +					bri->sda_gpio);
> +			bri->get_sda = NULL;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void i2c_put_gpios_for_recovery(struct i2c_adapter *adap)
> +{
> +	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +
> +	if (bri->get_sda)
> +		gpio_free(bri->sda_gpio);
> +
> +	gpio_free(bri->scl_gpio);
> +}
> +
> +static int i2c_generic_recovery(struct i2c_adapter *adap)
> +{
> +	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +	int i = 0, val = 1, ret = 0;
> +
> +	if (bri->prepare_recovery)
> +		bri->prepare_recovery(bri);
> +
> +	/*
> +	 * By this time SCL is high, as we need to give 9 falling-rising edges
> +	 */
> +	while (i++ < RECOVERY_CLK_CNT * 2) {
> +		/* SCL shouldn't be low here */
> +		if (val && !bri->get_scl(adap)) {
> +			dev_err(&adap->dev, "SCL is stuck Low exit recovery\n");

s/Low/low/

> +			ret = -EBUSY;
> +			goto unprepare;
> +		}
> +
> +		val = !val;
> +		bri->set_scl(adap, val);
> +		ndelay(clk_delay);
> +
> +		/* break if sda got high, check only when scl line is high */
> +		if (bri->get_sda && val)
> +			if (bri->get_sda(adap))
> +				break;

if (val && bri->get_sda && bri->get_sda(adap)
	break;

Didn't we agree to move the SDA check before the SCL setting?

> +	}
> +
> +unprepare:
> +	if (bri->unprepare_recovery)
> +		bri->unprepare_recovery(bri);
> +
> +	return ret;
> +}
> +
> +int i2c_generic_scl_recovery(struct i2c_adapter *adap)
> +{
> +	adap->bus_recovery_info->set_scl(adap, 1);
> +	return i2c_generic_recovery(adap);
> +}
> +
> +int i2c_generic_gpio_recovery(struct i2c_adapter *adap)
> +{
> +	int ret;
> +
> +	ret = i2c_get_gpios_for_recovery(adap);
> +	if (ret)
> +		return ret;
> +
> +	ret = i2c_generic_recovery(adap);
> +	i2c_put_gpios_for_recovery(adap);
> +
> +	return ret;
> +}
> +
> +int i2c_recover_bus(struct i2c_adapter *adap)
> +{
> +	if (!adap->bus_recovery_info)
> +		return -EOPNOTSUPP;
> +
> +	dev_dbg(&adap->dev, "try i2c bus recovery\n");

s/try/trying/

> +	return adap->bus_recovery_info->recover_bus(adap);
> +}
> +
>  static int i2c_device_probe(struct device *dev)
>  {
>  	struct i2c_client	*client = i2c_verify_client(dev);
> @@ -902,6 +1025,38 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>  			 "Failed to create compatibility class link\n");
>  #endif
>  
> +	/* bus recovery specific initialization */
> +	if (adap->bus_recovery_info) {
> +		struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> +
> +		if (!bri->recover_bus) {
> +			dev_err(&adap->dev, "Invalid recover_bus(), skip recovery\n");

s/Invalid/No/
s/skip/not using/
The latter for all later occurences, too.

> +			adap->bus_recovery_info = NULL;
> +			goto exit_recovery;
> +		}
> +
> +		/* GPIO recovery */
> +		if (bri->recover_bus == i2c_generic_gpio_recovery) {
> +			if (!gpio_is_valid(bri->scl_gpio)) {
> +				dev_err(&adap->dev, "Invalid scl gpio, skip recovery\n");
> +				adap->bus_recovery_info = NULL;
> +				goto exit_recovery;
> +			}
> +
> +			if (gpio_is_valid(bri->sda_gpio))
> +				bri->get_sda = get_sda_gpio_value;
> +			else
> +				bri->get_sda = NULL;
> +
> +			bri->get_scl = get_scl_gpio_value;
> +			bri->set_scl = set_scl_gpio_value;
> +		} else if (!bri->set_scl) {
> +			dev_warn(&adap->dev,
> +					"set_scl() is must for scl recovery\n");

What about get_scl. You don't check here, but you don't check when you
use it above.

> +		}
> +	}
> +
> +exit_recovery:
>  	/* create pre-declared device nodes */
>  	if (adap->nr < __i2c_first_dynamic_bus_num)
>  		i2c_scan_static_board_info(adap);
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index d0c4db7..da9358b 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -370,6 +370,43 @@ struct i2c_algorithm {
>  	u32 (*functionality) (struct i2c_adapter *);
>  };
>  
> +/**
> + * struct i2c_bus_recovery_info - I2C bus recovery information
> + * @recover_bus: Recover routine. Either pass driver's recover_bus() routine, or
> + *	i2c_generic_scl_recovery() or i2c_generic_gpio_recovery().
> + * @get_scl: This gets current value of scl line. Mandatory for generic SCL
> + *      recovery. Used internally for generic GPIO recovery.
> + * @set_scl: This sets/clears scl line. Mandatory for generic SCL recovery. Used
> + *      internally for generic GPIO recovery.
> + * @get_sda: This gets current value of sda line. Optional for generic SCL
> + *      recovery. Used internally, if sda_gpio != -1, for generic GPIO recovery.

s/!= -1/is a valid GPIO/

> + * @prepare_recovery: This will be called before starting recovery. Platform may
> + *	configure padmux here for sda/scl line or something else they want.
> + * @unprepare_recovery: This will be called after completing recovery. Platform
> + *	may configure padmux here for sda/scl line or something else they want.
> + * @scl_gpio: gpio number of the scl line. Only required for GPIO recovery.
> + * @sda_gpio: gpio number of the sda line. Only required for GPIO recovery.
> + */
> +struct i2c_bus_recovery_info {
> +	int (*recover_bus)(struct i2c_adapter *);
> +
> +	int (*get_scl)(struct i2c_adapter *);
> +	void (*set_scl)(struct i2c_adapter *, int val);
> +	int (*get_sda)(struct i2c_adapter *);
> +
> +	void (*prepare_recovery)(struct i2c_bus_recovery_info *bri);
> +	void (*unprepare_recovery)(struct i2c_bus_recovery_info *bri);
> +
> +	/* gpio recovery */
> +	unsigned scl_gpio;
> +	unsigned sda_gpio;
> +};
> +
> +/* Generic recovery routines */
> +int i2c_generic_gpio_recovery(struct i2c_adapter *adap);
> +int i2c_generic_scl_recovery(struct i2c_adapter *adap);
> +int i2c_recover_bus(struct i2c_adapter *adap);
> +
>  /*
>   * i2c_adapter is the structure used to identify a physical i2c bus along
>   * with the access algorithms necessary to access it.
> @@ -393,6 +430,8 @@ struct i2c_adapter {
>  
>  	struct mutex userspace_clients_lock;
>  	struct list_head userspace_clients;
> +
> +	struct i2c_bus_recovery_info *bus_recovery_info;
>  };
>  #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure

[Viresh Kumar]

On 25 January 2013 14:45, Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> On Thu, Jan 24, 2013 at 04:06:37PM +0530, Viresh Kumar wrote:
>> +static int i2c_generic_recovery(struct i2c_adapter *adap)
>> +{
>> +     struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
>> +     int i = 0, val = 1, ret = 0;
>> +
>> +     if (bri->prepare_recovery)
>> +             bri->prepare_recovery(bri);
>> +
>> +     /*
>> +      * By this time SCL is high, as we need to give 9 falling-rising edges
>> +      */
>> +     while (i++ < RECOVERY_CLK_CNT * 2) {
>> +             /* SCL shouldn't be low here */
>> +             if (val && !bri->get_scl(adap)) {
>> +                     dev_err(&adap->dev, "SCL is stuck Low exit recovery\n");
>> +                     ret = -EBUSY;
>> +                     goto unprepare;
>> +             }
>> +
>> +             val = !val;
>> +             bri->set_scl(adap, val);
>> +             ndelay(clk_delay);
>> +
>> +             /* break if sda got high, check only when scl line is high */
>> +             if (bri->get_sda && val)
>> +                     if (bri->get_sda(adap))
>> +                             break;
>
> Didn't we agree to move the SDA check before the SCL setting?

Yes. There was a check for SCL (separately) before this loop. I got that into
this loop to save on redundant code. And now, after sending a complete clock,
we first check sda, then go to next iteration of loop and check scl.

Over that, this routine is a modified a bit again, due to Uwe's comment.

Wait for v10. You would get that in 2 mins :)

BTW, all other comments are accepted.