Skip to contentSkip to conversation

I2C: Add bus recovery infrastructure

PATCH In review Viresh Kumar 5 participants 9 messages May 4, 2012 – Jul 27, 2012 5 messages not in the archive

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

v4 cover I2C: Add bus recovery infrastructure

Hi Wolfram,

This patchset adds i2c bus recovery infrastructure to i2c adapters as specified in the i2c protocol Rev. 03 section 3.16 titled "Bus clear".

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

This patch was earlier part of a separate thread:
http://www.spinics.net/lists/linux-i2c/msg07267.html

V3->V4:
- created single i2c_recover_bus() routine instead of two.
- do bus recovery from i2c core files, instead of individual controller drivers.
- created separate struct for gpio configurations
- Documentation updated for bus recovery
- Renamed few variables to give clear names to them
- few prints changed to dev_dbg

V2->V3:
- gpio flags are now passed from controller drivers
- added support for sda line polling
- Aligned i2c-designware driver with generic recovery support

Viresh Kumar (2):
i2c/adapter: Add bus recovery infrastructure i2c/designware: Provide i2c bus recovery support

 Documentation/i2c/bus-recovery              |   87 +++++++++++++++
 drivers/i2c/busses/i2c-designware-core.c    |    1 -
 drivers/i2c/busses/i2c-designware-platdrv.c |   31 +++++
 drivers/i2c/i2c-core.c                      |  160 +++++++++++++++++++++++++++
 drivers/i2c/i2c-mux.c                       |    9 ++-
 include/linux/i2c.h                         |   58 ++++++++++
 include/linux/i2c/i2c-designware.h          |   49 ++++++++
 7 files changed, 393 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/i2c/bus-recovery
 create mode 100644 include/linux/i2c/i2c-designware.h
Signature · 1 line
1.7.9

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

PATCH v4 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.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.

---
 Documentation/i2c/bus-recovery |   87 ++++++++++++++++++++++
 drivers/i2c/i2c-core.c         |  160 ++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-mux.c          |    9 ++-
 include/linux/i2c.h            |   58 ++++++++++++++
 4 files changed, 313 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/i2c/bus-recovery
Documentation/i2c/bus-recovery new file +87 -0
new file mode 100644
index 0000000..518dff6
@@ -0,0 +1,87 @@
+			I2C Bus Recovery
+			================
+
+Problem:
+========
+There is a generic issue with I2C transfers, where the SDA line is pulled down
+by the slave device and the controller isn't able to control SDA line anymore.
+
+Solution:
+=========
+Following is specified in the i2c protocol Rev. 03 section 3.16 titled "Bus
+clear".
+
+http://www.nxp.com/documents/user_manual/UM10204.pdf
+
+If the data line (SDA) is stuck LOW, the master should send nine clock pulses.
+The device that held the bus LOW should release it sometime within those nine
+clocks. If not, then use the HW reset or cycle power to clear the bus.
+
+Implementation:
+==============
+In order to fix this issue in Linux, bus recovery mechanism is added in
+i2c-core. The controller driver needs to do following to get recovery support
+from i2c-core:
+- Return -ETIMEDOUT from their master_xfer() callback if they are stuck, so that
+  core can try recovery. (Which can only be done if below is passed from
+  controller driver.)
+- Must fill Adapters bus_recovery_info field with valid recovery structure.
+
+/**
+ * struct i2c_bus_recovery_info - I2c bus recovery information
+ * @recover_bus: Recover routine. Either pass driver's recover_bus() routine, or
+ *	pass it NULL to use generic ones, i.e. gpio or scl based.
+ * @skip_sda_polling: if true, bus recovery will not poll sda line to check if
+ *	it became high or not. Only required if recover_bus == NULL.
+ * @is_gpio_recovery: true, select gpio type else scl type. Only required if
+ *	recover_bus == NULL.
+ * @clock_rate_khz: clock rate of dummy clock in khz. Required for both gpio and
+ *	scl type recovery.
+ * @clock_cnt: count of max clocks to be generated. Required for both gpio and
+ *	scl type recovery.
+ * @set_scl: controller specific scl configuration routine. Only required if
+ *	is_gpio_recovery == false
+ * @get_sda: controller specific sda read routine. Only required if
+ *	is_gpio_recovery == false and skip_sda_polling == false.
+ * @gpio_recov: gpio recovery info, only if (is_gpio_recovery == true)
+ */
+struct i2c_bus_recovery_info {
+	int (*recover_bus)(struct i2c_adapter *);
+	bool skip_sda_polling;
+	bool is_gpio_recovery;
+	u32 clock_rate_khz;
+	u8 clock_cnt;
+
+	/* gpio recovery */
+	struct i2c_bus_gpio_recovery_info *gpio_recov;
+	/* scl/sda recovery */
+	void (*set_scl)(struct i2c_adapter *, int val);
+	int (*get_sda)(struct i2c_adapter *);
+};
+
+/**
+ * struct i2c_bus_gpio_recovery_info - I2c bus recovery information if recovery
+ *	type is gpio.
+ * @get_gpio: called before recover_bus() to get padmux configured for scl line.
+ *	as gpio.
+ * @put_gpio: called after recover_bus() to get padmux configured for scl line
+ *	as scl.
+ * @scl_gpio: gpio number of the scl line.
+ * @sda_gpio: gpio number of the sda line.
+ * @scl_gpio_flags: flag for gpio_request_one of scl_gpio. 0 implies
+ *	GPIOF_OUT_INIT_LOW.
+ * @sda_gpio_flags: flag for gpio_request_one of sda_gpio. 0 implies
+ *	GPIOF_OUT_INIT_LOW.
+ */
+struct i2c_bus_gpio_recovery_info {
+	int (*get_gpio)(unsigned gpio);
+	int (*put_gpio)(unsigned gpio);
+	u32 scl_gpio;
+	u32 sda_gpio;
+	u32 scl_gpio_flags;
+	u32 sda_gpio_flags;
+};
+
+Author:
+=======
+Viresh Kumar <[email protected]>
drivers/i2c/i2c-core.c +160 -0

185 lines not on this page · raw message

drivers/i2c/i2c-mux.c +8 -1
index 1038c38..c710319 100644
@@ -50,8 +50,15 @@ static int i2c_mux_master_xfer(struct i2c_adapter *adap,
 	/* Switch to the right mux port and perform the transfer. */
 
 	ret = priv->select(parent, priv->mux_priv, priv->chan_id);
-	if (ret >= 0)
+	if (ret >= 0) {
 		ret = parent->algo->master_xfer(parent, msgs, num);
+		if ((ret == -ETIMEDOUT) && parent->bus_recovery_info) {
+			/* Attempt bus recovery */
+			dev_dbg(&parent->dev, "try bus recovery\n");
+			parent->bus_recovery_info->recover_bus(parent);
+			ret = -EAGAIN;
+		}
+	}
 	if (priv->deselect)
 		priv->deselect(parent, priv->mux_priv, priv->chan_id);
 
include/linux/i2c.h +58 -0

70 lines not on this page · raw message

Signature · 1 line
1.7.9

A message that is not in the archive (sent off-list or not yet received): 3484bf49a423bd55daa6a45e68c2c31dce46eb32.1336124143.git.viresh.kumar-qxv4g6HH51o@public.gmane.org

#4 lore raw

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

Quoted 14 lines · Viresh KUMAR

Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c protocol Rev. 03 section 3.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.

Quoted 8 lines
---
 Documentation/i2c/bus-recovery |   87 ++++++++++++++++++++++
 drivers/i2c/i2c-core.c         |  160 ++++++++++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-mux.c          |    9 ++-
 include/linux/i2c.h            |   58 ++++++++++++++
 4 files changed, 313 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/i2c/bus-recovery

Hi Wolfram,

Any inputs on this patch.

--
viresh

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw

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

Quoted 24 lines · Viresh Kumar
Quoted 14 lines · Viresh KUMAR

Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c protocol Rev. 03 section 3.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.

Quoted 8 lines

---
 Documentation/i2c/bus-recovery |   87 ++++++++++++++++++++++ drivers/i2c/i2c-core.c         |  160 ++++++++++++++++++++++++++++++++++++++++ drivers/i2c/i2c-mux.c          |    9 ++-
 include/linux/i2c.h            |   58 ++++++++++++++

 4 files changed, 313 insertions(+), 1 deletions(-)

 create mode 100644 Documentation/i2c/bus-recovery

Quoted 3 lines

Hi Wolfram,

Any inputs on this patch.

Ping.

A message that is not in the archive (sent off-list or not yet received): CAOh2x=m1XFLpunpTPUW7ozT3cExXNHyLwF7sDy4MxyjuK2uNwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#8 lore raw

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

Hi Wolfram,

Quoted 25 lines · viresh kumar
Quoted 24 lines · Viresh Kumar
Quoted 22 lines · Viresh KUMAR

Add i2c bus recovery infrastructure to i2c adapters as specified in the i2c protocol Rev. 03 section 3.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.

---
 Documentation/i2c/bus-recovery |   87 ++++++++++++++++++++++ drivers/i2c/i2c-core.c         |  160 ++++++++++++++++++++++++++++++++++++++++ drivers/i2c/i2c-mux.c          |    9 ++-
 include/linux/i2c.h            |   58 ++++++++++++++

 4 files changed, 313 insertions(+), 1 deletions(-)

 create mode 100644 Documentation/i2c/bus-recovery

Quoted 5 lines
Quoted 3 lines

Hi Wolfram,

Any inputs on this patch.

Ping.

Any chance of this passing through your tree, as we are dependent on this.

--
regards
Shiraz

A message that is not in the archive (sent off-list or not yet received): 20120702110818.GP1978-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

#10 lore raw

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

Quoted 2 lines · Shiraz Hashim

Any chance of this passing through your tree, as we are dependent on this.

Ping !!

#11 lore raw

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

In reply to a message not in the archive

Hi Viresh,

Quoted 5 lines · Viresh Kumar

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

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

Quoted 9 lines

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.

It might me worth how was this tested?
Was both the approaches given a try?
Also it might help if one of the drivers currently doing it is ported to the framework.

Just a suggestion not an objection to the patch.

Quoted 3 lines
---
 Documentation/i2c/bus-recovery |   87 ++++++++++++++++++++++
#12 lore raw

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

In reply to a message not in the archive

Hi Viresh ,

Quoted 5 lines · Viresh Kumar

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

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

Quoted 9 lines

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.

A few suggestions.

1. Bus recovery should ideally handle both SCL and SDA stuck a 0 conditions. This only appears to handle SDA. For SCL stuck at 0 we need to reset the slave if possible. So maybe there needs to be the ability to register a slave reset handler (if this does not exist).
2. Following on from #1, there should be the ability to query the state of SCL and SDA if the controller supports this. For example, on OMAP we can query whether SCL or SDA are stuck low when the timeout occurs. Then based upon whether SCL or SDA are stuck, we either try the appropriate bus recovery. If there is no mechanism to query SCL or SDA, then we need to decide on the appropriate behaviour. For example, should we try the workaround for SDA stuck first as it is less severe and if that fails try the SCL workaround if a reset handler is registered to reset the device.

. What do we do if the recovery does not work after N attempts? Is
this handled? If not after a certain number of failures it may be necessary to unregister the slave.Or if the slave supports reset
can we contemplate or if it is powered by gpio can we reset it.

What do you think

#13 lore raw

PATCH v4 2/2 i2c/designware: Provide i2c bus recovery support

In reply to a message not in the archive

Add bus recovery support for designware_i2c controller. It uses generic gpio based i2c_gpio_recover_bus() routine.

---
 drivers/i2c/busses/i2c-designware-core.c    |    1 -
 drivers/i2c/busses/i2c-designware-platdrv.c |   31 +++++++++++++++++
 include/linux/i2c/i2c-designware.h          |   49 +++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/i2c/i2c-designware.h
drivers/i2c/busses/i2c-designware-core.c +0 -1
index 1e48bec..0ef7ddc 100644
@@ -536,7 +536,6 @@ 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);
 		ret = -ETIMEDOUT;
 		goto done;
 	} else if (ret < 0)
drivers/i2c/busses/i2c-designware-platdrv.c +31 -0
index 0506fef..e30651e 100644
@@ -29,6 +29,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
+#include <linux/i2c/i2c-designware.h>
 #include <linux/clk.h>
 #include <linux/errno.h>
 #include <linux/sched.h>
@@ -45,6 +46,15 @@ static struct i2c_algorithm i2c_dw_algo = {
 	.master_xfer	= i2c_dw_xfer,
 	.functionality	= i2c_dw_func,
 };
+
+/* gpio recovery info */
+struct i2c_bus_gpio_recovery_info gpio_recovery_info = {
+};
+static struct i2c_bus_recovery_info dw_recovery_info = {
+	.is_gpio_recovery = true,
+	.gpio_recov = &gpio_recovery_info,
+};
+
 static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 {
 	return clk_get_rate(dev->clk)/1000;
@@ -55,6 +65,7 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
 	struct resource *mem, *ioarea;
+	struct i2c_dw_pdata *pdata;
 	int irq, r;
 
 	/* NOTE: driver uses the static register mapping */
@@ -141,6 +152,26 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 	adap->dev.parent = &pdev->dev;
 	adap->dev.of_node = pdev->dev.of_node;
 
+	/* Bus recovery support */
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata) {
+		dw_recovery_info.gpio_recov->get_gpio = pdata->get_gpio;
+		dw_recovery_info.gpio_recov->put_gpio = pdata->put_gpio;
+		dw_recovery_info.gpio_recov->scl_gpio = pdata->scl_gpio;
+		dw_recovery_info.gpio_recov->scl_gpio_flags =
+			pdata->scl_gpio_flags;
+		dw_recovery_info.clock_rate_khz = clk_get_rate(dev->clk) / 1000;
+
+		if (!pdata->skip_sda_polling) {
+			dw_recovery_info.gpio_recov->sda_gpio = pdata->sda_gpio;
+			dw_recovery_info.gpio_recov->sda_gpio_flags =
+				pdata->sda_gpio_flags;
+		}
+
+		dw_recovery_info.skip_sda_polling = pdata->skip_sda_polling;
+		adap->bus_recovery_info = &dw_recovery_info;
+	}
+
 	adap->nr = pdev->id;
 	r = i2c_add_numbered_adapter(adap);
 	if (r) {
include/linux/i2c/i2c-designware.h new file +49 -0

49 lines not on this page · raw message

Signature · 1 line
1.7.9