Re: [PATCH 2/2] i2c: designware: Add support for AMD I2C controller
On Tue, Sep 23, 2014 at 3:05 AM, Mika Westerberg mika.westerb...@linux.intel.com wrote: arch/arm/boot/dts/spear1310.dtsi arch/arm/boot/dts/spear320.dtsi arch/arm/boot/dts/spear3xx.dtsi arch/arm/boot/dts/spear600.dtsi I think we are good to go with the first three but not sure about the last five. Adding Viresh Kumar and Daniel Tang to the thread. Viresh, Daniel, The thread is here http://patchwork.ozlabs.org/patch/390695/. In summary, we are adding COMMON_CLK dependency to the i2c-designware-platdrv.c in order to get clocks to one AMD DW based host controller and wanted to check that we don't break the existing users (nspire and SPEAR). A dependency on COMMON_CLK is fine for SPEAr . -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V10 Resend] i2c/designware: Provide i2c bus recovery support
On 17 June 2013 18:46, Viresh Kumar viresh.ku...@linaro.org wrote: 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.frasc...@st.com Signed-off-by: Shiraz Hashim shiraz.has...@st.com Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- This patch isn't drawing attention since sometime, so sending it again. drivers/i2c/busses/i2c-designware-core.c| 5 - drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++ 2 files changed, 10 insertions(+), 1 deletion(-) Ping!! -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V10 Resend] 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.frasc...@st.com
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
This patch isn't drawing attention since sometime, so sending it again.
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 c41ca63..5ecb2f0 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -583,7 +583,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 35b70a1..c3ada3e 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -162,6 +162,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;
+
r = i2c_add_numbered_adapter(adap);
if (r) {
dev_err(pdev-dev, failure adding adapter\n);
--
1.7.12.rc2.18.g61b472e
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V10 2/2] i2c/designware: Provide i2c bus recovery support
On 13 May 2013 14:48, Viresh Kumar viresh.ku...@linaro.org wrote: On 5 April 2013 12:12, Viresh Kumar viresh.ku...@linaro.org wrote: Hi Wolfram, On 25 January 2013 15:17, Viresh Kumar viresh.ku...@linaro.org wrote: 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.frasc...@st.com Signed-off-by: Shiraz Hashim shiraz.has...@st.com Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- V9-V10: None drivers/i2c/busses/i2c-designware-core.c| 5 - drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++ 2 files changed, 10 insertions(+), 1 deletion(-) Can you apply this one too? Ping!! Ping!! -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V10 2/2] i2c/designware: Provide i2c bus recovery support
On 5 April 2013 12:12, Viresh Kumar viresh.ku...@linaro.org wrote: Hi Wolfram, On 25 January 2013 15:17, Viresh Kumar viresh.ku...@linaro.org wrote: 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.frasc...@st.com Signed-off-by: Shiraz Hashim shiraz.has...@st.com Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- V9-V10: None drivers/i2c/busses/i2c-designware-core.c| 5 - drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++ 2 files changed, 10 insertions(+), 1 deletion(-) Can you apply this one too? Ping!! -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V10 2/2] i2c/designware: Provide i2c bus recovery support
Hi Wolfram, On 25 January 2013 15:17, Viresh Kumar viresh.ku...@linaro.org wrote: 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.frasc...@st.com Signed-off-by: Shiraz Hashim shiraz.has...@st.com Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- V9-V10: None drivers/i2c/busses/i2c-designware-core.c| 5 - drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++ 2 files changed, 10 insertions(+), 1 deletion(-) Can you apply this one too? -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V10 1/2] i2c/adapter: Add bus recovery infrastructure
On 24 March 2013 19:37, Viresh Kumar viresh.ku...@linaro.org wrote: On 21 March 2013 15:06, Wolfram Sang w...@the-dreams.de wrote: I applied V11 of the core changes with minor modifications. I do wonder I can't find it linux-next or here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git I can see it now in your for-next branch, thanks. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V10 1/2] i2c/adapter: Add bus recovery infrastructure
On 26 January 2013 10:43, Viresh Kumar viresh.ku...@linaro.org wrote: I have looked them again and attached are the final patches. Following are the improvements i have: Author: Viresh Kumar viresh.ku...@linaro.org Date: Sat Jan 26 10:31:42 2013 +0530 fixup! i2c/adapter: Add bus recovery infrastructure Hi Wolfram, Any inputs ? -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 25 January 2013 14:20, Wolfram Sang w.s...@pengutronix.de wrote: I kept it to make sure we don't do this calculation every time... I would like to keep it for better performance.. Please move it. It is a constant anyhow, and globals are best avoided. I will move it. But i couldn't understand what you mean by it is a constant anyhow, because that was one of the reason for keeping it global. Now onto V9... You will find the same there too :) Anyway i need to do some minor cleanups requested by uwe, so v10 is required. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure
On 25 January 2013 14:45, Wolfram Sang w.s...@pengutronix.de 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.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V10 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.frasc...@st.com
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
V9-V10: None
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
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V10 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 p...@pcserviceselectronics.co.uk
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
V9-V10:
- Check sda value at the begining of loop to guarantee that we are not trying to
clock a working bus.
- Remove clk_delay variable, do the calculation in advance and create
RECOVERY_NDELAY macro for it
- Fix grammer and casing of few strings
- check get_scl() too at the begining as it was mandatory for SCL type recovery
drivers/i2c/i2c-core.c | 156 +
include/linux/i2c.h| 40 +
2 files changed, 196 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..24c8a1e 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,128 @@ 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 */
+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. not using 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);
+}
+
+/*
+ * We need to provide delay after every half clock pulse,
+ * delay in ns = (10^6/ 100 KHz)/2
+ */
+#define RECOVERY_CLK_CNT 9
+#define RECOVERY_NDELAY5000
+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) {
+ if (val) {
+ /* break if SDA got high */
+ if (bri-get_sda bri-get_sda(adap))
+ break;
+ /* SCL shouldn't be low here */
+ if (!bri-get_scl(adap)) {
+ dev_err(adap-dev,
+ SCL is stuck low exit recovery\n);
+ ret = -EBUSY;
+ break;
+ }
+ }
+
+ val = !val;
+ bri-set_scl(adap, val);
+ ndelay(RECOVERY_NDELAY);
+ }
+
+ 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
Re: [PATCH V10 1/2] i2c/adapter: Add bus recovery infrastructure
On 25 January 2013 15:17, Viresh Kumar viresh.ku...@linaro.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. ARM mail servers are still broken. Please apply attached patches. 0001-i2c-adapter-Add-bus-recovery-infrastructure.patch Description: Binary data 0002-i2c-designware-Provide-i2c-bus-recovery-support.patch Description: Binary data
Re: [PATCH V10 1/2] i2c/adapter: Add bus recovery infrastructure
On 25 January 2013 16:43, Wolfram Sang w.s...@pengutronix.de wrote:
Code-wise I am mostly satisfied, except that I need to think about
clock-stretching a little more. We could add that later, too.
Great!!
However, while I understand you are keen to get rid of this series ASAP,
it seems that this eagerness results in some sloppyness which is a bit
cumbersome (most lines changed since last time have something to comment
on).
:(
So, I'll try a different review style this time and let you do the
detective work :)
1) Please be strict and consistent with spaces around operators.
Sorry, but i couldn't find any such issues in code (but were there
in a comment)
2) Make sure the warnings and printouts match the code
3) Take the printouts serious and invest a thought if they can be
further improved.
I have looked them again and attached are the final patches.
Following are the improvements i have:
Author: Viresh Kumar viresh.ku...@linaro.org
Date: Sat Jan 26 10:31:42 2013 +0530
fixup! i2c/adapter: Add bus recovery infrastructure
---
drivers/i2c/i2c-core.c | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 24c8a1e..3ec040e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -136,14 +136,14 @@ static int i2c_get_gpios_for_recovery(struct
i2c_adapter *adap)
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);
+ dev_warn(dev, Can't get SCL gpio: %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. not using
SDA polling\n,
+ dev_warn(dev, Can't get SDA gpio: %d. Not
using SDA polling\n,
bri-sda_gpio);
bri-get_sda = NULL;
}
@@ -163,8 +163,11 @@ static void i2c_put_gpios_for_recovery(struct
i2c_adapter *adap)
}
/*
- * We need to provide delay after every half clock pulse,
- * delay in ns = (10^6/ 100 KHz)/2
+ * We are generating clock pulse. ndelay() would decide durating of clk pulse.
+ * We will generate clock with rate 100 KHz and so duration of both
clock levels
+ * would be:
+ *
+ * delay in ns = (10^6 / 100) / 2
*/
#define RECOVERY_CLK_CNT 9
#define RECOVERY_NDELAY5000
@@ -181,13 +184,13 @@ static int i2c_generic_recovery(struct i2c_adapter *adap)
*/
while (i++ RECOVERY_CLK_CNT * 2) {
if (val) {
- /* break if SDA got high */
+ /* Break if SDA is high */
if (bri-get_sda bri-get_sda(adap))
break;
/* SCL shouldn't be low here */
if (!bri-get_scl(adap)) {
dev_err(adap-dev,
- SCL is stuck low exit recovery\n);
+ SCL is stuck low, exit recovery\n);
ret = -EBUSY;
break;
}
@@ -229,7 +232,7 @@ int i2c_recover_bus(struct i2c_adapter *adap)
if (!adap-bus_recovery_info)
return -EOPNOTSUPP;
- dev_dbg(adap-dev, trying i2c bus recovery\n);
+ dev_dbg(adap-dev, Trying i2c bus recovery\n);
return adap-bus_recovery_info-recover_bus(adap);
}
@@ -1031,12 +1034,12 @@ static int i2c_register_adapter(struct
i2c_adapter *adap)
struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
if (!bri-recover_bus) {
- dev_err(adap-dev, No recover_bus(), not
using recovery\n);
+ dev_err(adap-dev, No recover_bus() found,
not using recovery\n);
adap-bus_recovery_info = NULL;
goto exit_recovery;
}
- /* GPIO recovery */
+ /* Generic GPIO recovery */
if (bri-recover_bus == i2c_generic_gpio_recovery) {
if (!gpio_is_valid(bri-scl_gpio)) {
dev_err(adap-dev, Invalid SCL gpio,
not using recovery\n);
@@ -1052,8 +1055,9 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
bri-get_scl = get_scl_gpio_value;
bri-set_scl = set_scl_gpio_value;
} else if (!bri-set_scl || !bri-get_scl) {
- dev_warn(adap-dev,
- set_scl() is must for SCL recovery\n);
+ /* Generic SCL recovery
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 24 January 2013 12:54, Wolfram Sang w.s...@pengutronix.de wrote:
Hi Viresh,
Hi Wolfram
I think we are getting close.
Wow!!
On Mon, Dec 03, 2012 at 08:24:04AM +0530, Viresh Kumar wrote:
This doesn't support multi-master recovery for now.
Assuming that recover_bus is not called on BUS_BUSY but on TIMEOUTs,,
this should work?
@Uwe/Paul: ??
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
+/* 10^6/KHz for delay in ns */
+unsigned long clk_delay = DIV_ROUND_UP(100, DEFAULT_CLK_RATE * 2);
no global variable. should go into the function needing it.
I kept it to make sure we don't do this calculation every time... I would like
to keep it for better performance..
+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-skip_sda_polling) {
* if (is_valid_gpio(bri-sda_gpio)) ...
Hmm.. looks pretty good and correct. Will buy this one :)
+ 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-skip_sda_polling = true;
* Instead of above line:
bri-get_sda = get_sda_gpio_value;
Hmm.. couldn't get this one...
So, what i understood is, because we don't have skip_sda_polling now, we
have to choose some other way to say, we don't support sda.. so we can
mark get_sda as NULL. right?
+static int i2c_generic_recovery(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ int i, val = 0;
+
+ if (bri-prepare_recovery)
+ bri-prepare_recovery(bri);
prepare_recovery and unprepare should be in i2c_generic_scl_recovery and
i2c_generic_gpio_recovery since they are probably needed to turn SDA/SCL
into GPIOs and back. We want that before the gpio_get/put routines.
i2c_generic_recovery() is an local routine called from both scl and
gpio recovery
routines. So, keeping a single copy of prepare/unprepare calls makes more sense.
The other point about getting gpios first and then calling prepare..
I don't think that would be right thing to do. Suppose we call prepare
first, which would
update padmux on the board. At this time gpio may be in output mode and my burn
our boards. So, its better to get gpios in desired modes and then change muxing.
+ /* SCL shouldn't be low here */
+ if (!bri-get_scl(adap)) {
+ dev_err(adap-dev, SCL is stuck Low, exiting recovery.\n);
returning -EBUSY?
Yes. Will recheck on errors returned from the routine.
+ /* Check SCL again to see fault condition */
+ if (!bri-get_scl(adap)) {
+ dev_err(adap-dev, SCL is stuck Low during
recovery, exiting recovery.\n);
returning -EBUSY?
This scl check should not depend on skip_sda_polling, or?
yes.
+ goto unprepare;
+ }
+
+ if (bri-get_sda(adap))
+ break;
Checking SDA should be done before setting SCL? That would
a) let us escape early if SDA got HIGH magically somehow until we enter
the for loop for the first time
b) breaking out of the for loop after the last pulse is moot
Good one.
+ dev_warn(adap-dev,
+ !get_sda. skip sda
polling\n);
That message needs improvement (!get_sda).
+1
All other comments are accepted too.. Hopefully it may get in 3.9 :)
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[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.frasc...@st.com
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@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
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[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 p...@pcserviceselectronics.co.uk
Signed-off-by: Viresh Kumar viresh.ku...@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(100, 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
Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure
On 24 January 2013 16:06, Viresh Kumar viresh.ku...@linaro.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 p...@pcserviceselectronics.co.uk Signed-off-by: Viresh Kumar viresh.ku...@linaro.org ARM mail servers are broken now a days, so these patches may not apply from mail. Find them attached. 0001-i2c-adapter-Add-bus-recovery-infrastructure.patch Description: Binary data 0002-i2c-designware-Provide-i2c-bus-recovery-support.patch Description: Binary data
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 24 January 2013 16:24, Uwe Kleine-König
u.kleine-koe...@pengutronix.de wrote:
On Thu, Jan 24, 2013 at 08:24:45AM +0100, Wolfram Sang wrote:
Some more comments below.
Always welcomed :)
* if (val bri-get_sda)
+ /* Check SCL again to see fault condition */
+ if (!bri-get_scl(adap)) {
+ dev_err(adap-dev, SCL is stuck Low during
recovery, exiting recovery.\n);
+ goto unprepare;
+ }
+
+ if (bri-get_sda(adap))
+ break;
Indention suggests that you want this in the body of the if (val
bri-get_sda). Unfortunately the C-Compiler won't notice without braces.
Wow!! It was a blunder. Don't know how the braces got dropped.
My bad, but the new patchset (already posted), must have fixed it by
mistake :)
Checking SDA should be done before setting SCL? That would
a) let us escape early if SDA got HIGH magically somehow until we enter
the for loop for the first time
b) breaking out of the for loop after the last pulse is moot
You mean:
val = 0;
for (i = 0; i RECOVERY_CLK_CNT * 2; i++, val = !val) {
if (!val !bri-get_scl(adap))
scl stuck low (or wait a bit to sort out
clock streching)
if (bri-get_sda bri-get_sda(adap))
break;
bri-set_scl(adap, val);
ndelay(clk_delay);
}
Looks ok I think. This would also get rid of the seperate scl check
before the loop.
I have done something similar in V9..
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure
On 24 January 2013 16:36, Uwe Kleine-König
u.kleine-koe...@pengutronix.de 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.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V9 1/2] i2c/adapter: Add bus recovery infrastructure
On 24 January 2013 17:13, Uwe Kleine-König
u.kleine-koe...@pengutronix.de 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
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 7 January 2013 16:02, Viresh Kumar viresh.ku...@linaro.org wrote: On 20 December 2012 14:47, Viresh Kumar viresh.ku...@linaro.org wrote: On 19 December 2012 05:30, Wolfram Sang w.s...@pengutronix.de wrote: Since I missed the things Paul luckily spotted, I think it might make sense to actually use the framework myself to get me a better feeling that I have not missed anything else. So, I am going to add support for another I2C master controller to your framework and see what will happen when I actually use it. It will be probably the mxs controller, I collected the hardware for that already... Nothing could be better than that :) Hi Wolfram, HNY '13. You got some chance to test this stuff ? Ping!! -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 2/2] i2c/designware: Provide i2c bus recovery support
On 24 January 2013 12:54, Wolfram Sang w.s...@pengutronix.de wrote:
On Mon, Dec 03, 2012 at 08:24:05AM +0530, Viresh Kumar wrote:
@@ -538,7 +538,11 @@ 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 (adap-bus_recovery_info) {
+ dev_dbg(dev-dev, try i2c bus recovery\n);
+ adap-bus_recovery_info-recover_bus(adap);
+ }
+
I think we need something like i2c_recover_bus in the core which does
the above and also returns the return code from recover_bus. If there is
no recover_bus it should return EOPNOTSUPP.
Then the driver can do
ret = i2c_recover_bus(adap);
if (ret 0)
i2c_dw_init();
If not calling i2c_dw_init, you will probably cause a regression.
Fair enough.
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
+err_free_recovery_info:
+ kfree(recovery_info);
Leftover of earlier versions, my mistake :(
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 20 December 2012 14:47, Viresh Kumar viresh.ku...@linaro.org wrote: On 19 December 2012 05:30, Wolfram Sang w.s...@pengutronix.de wrote: Since I missed the things Paul luckily spotted, I think it might make sense to actually use the framework myself to get me a better feeling that I have not missed anything else. So, I am going to add support for another I2C master controller to your framework and see what will happen when I actually use it. It will be probably the mxs controller, I collected the hardware for that already... Nothing could be better than that :) Hi Wolfram, HNY '13. You got some chance to test this stuff ? -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 19 December 2012 05:30, Wolfram Sang w.s...@pengutronix.de wrote: Since I missed the things Paul luckily spotted, I think it might make sense to actually use the framework myself to get me a better feeling that I have not missed anything else. So, I am going to add support for another I2C master controller to your framework and see what will happen when I actually use it. It will be probably the mxs controller, I collected the hardware for that already... Nothing could be better than that :) -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 7 December 2012 02:00, Paul Carpenter p...@pcserviceselectronics.co.uk wrote: OK by me Reviewed-by: Paul Carpenter p...@pcserviceselectronics.co.uk Are you taking it for 3.8 now? -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 6 December 2012 21:28, Paul Carpenter p...@pcserviceselectronics.co.uk wrote: Nothing more from me. I will keep quiet now :-) As you have invested some effort in reviewing it, you want to give your Reviewed-by? -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 1/2] i2c/adapter: Add bus recovery infrastructure
On 3 December 2012 08:24, Viresh Kumar viresh.ku...@linaro.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. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org --- V7-V8: - Clk rate fixed to 100KHz - Check SCL line to see if it is LOW due to some faults - removed last use of unlikely() in earlier patch - Enhanced comment over skip_sda_polling As merge window is shifted for few more days, i am trying another time to get this in 3.8 :) @Paul/Wolfram: Any more comments ? -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6 1/2] i2c/adapter: Add bus recovery infrastructure
Hi Wolfram/Paul,
I have tried to fix my patches as per review comments from you.
Following is diff of the new changes i have made, I am sending a
new complete version separately:
commit 62cf40359c63ac028b3e8a6395f9440f6a3b0162
Author: Viresh Kumar viresh.ku...@linaro.org
Date: Mon Dec 3 08:12:55 2012 +0530
fixup! i2c/adapter: Add bus recovery infrastructure
---
drivers/i2c/i2c-core.c | 42 +++---
include/linux/i2c.h| 19 +++
2 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index d100276..9b6a1a6 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -109,15 +109,22 @@ static int i2c_device_uevent(struct device *dev,
struct kobj_uevent_env *env)
/* 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(100, DEFAULT_CLK_RATE * 2);
-static void set_scl_gpio_value(struct i2c_adapter *adap, int val)
+static int get_sda_gpio_value(struct i2c_adapter *adap)
{
- gpio_set_value(adap-bus_recovery_info-scl_gpio, val);
+ return gpio_get_value(adap-bus_recovery_info-sda_gpio);
}
-static int get_sda_gpio_value(struct i2c_adapter *adap)
+static int get_scl_gpio_value(struct i2c_adapter *adap)
{
- return gpio_get_value(adap-bus_recovery_info-sda_gpio);
+ 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 i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
@@ -158,28 +165,37 @@ static void i2c_put_gpios_for_recovery(struct
i2c_adapter *adap)
static int i2c_generic_recovery(struct i2c_adapter *adap)
{
struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
- unsigned long delay = 100; /* 10^6/KHz for delay in ns */
int i, val = 0;
if (bri-prepare_recovery)
bri-prepare_recovery(bri);
+ /* SCL shouldn't be low here */
+ if (!bri-get_scl(adap)) {
+ dev_err(adap-dev, SCL is stuck Low, exiting recovery.\n);
+ goto unprepare;
+ }
+
/*
* By this time SCL is high, as we need to give 9 falling-rising edges
*/
-
- delay = DIV_ROUND_UP(delay, bri-clock_rate_khz * 2);
-
for (i = 0; i RECOVERY_CLK_CNT * 2; i++, val = !val) {
bri-set_scl(adap, val);
- ndelay(delay);
+ ndelay(clk_delay);
/* break if sda got high, check only when scl line is high */
if (!bri-skip_sda_polling val)
- if (unlikely(bri-get_sda(adap)))
+ /* Check SCL again to see fault condition */
+ if (!bri-get_scl(adap)) {
+ dev_err(adap-dev, SCL is stuck Low
during recovery, exiting recovery.\n);
+ goto unprepare;
+ }
+
+ if (bri-get_sda(adap))
break;
}
+unprepare:
if (bri-unprepare_recovery)
bri-unprepare_recovery(bri);
@@ -1008,13 +1024,9 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
goto exit_recovery;
}
- if (!bri-clock_rate_khz) {
- dev_warn(adap-dev, default clk rate used: 100KHz\n);
- bri-clock_rate_khz = DEFAULT_CLK_RATE;
- }
-
/* GPIO recovery */
if (bri-recover_bus == i2c_generic_gpio_recovery) {
+ bri-get_scl = get_scl_gpio_value;
bri-set_scl = set_scl_gpio_value;
if (!bri-skip_sda_polling)
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 63acd9d..165282f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -373,14 +373,17 @@ struct i2c_algorithm {
* @recover_bus: Recover routine. Either pass driver's recover_bus()
routine, or
* i2c_generic_scl_recovery() or i2c_generic_gpio_recovery().
* @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.
- * @clock_rate_khz: clock rate of dummy clock in khz. Required for
both gpio and
- * scl type recovery.
- * @set_scl: This sets/clears scl line. For GPIO recovery set_scl_gpio_value()
- * is used here otherwise controller specific routine must be passed.
+ * it became high or not. Platforms/controllers which don't have
+ * configuration registers to control sda line must set this. Only required
+ * if recover_bus == NULL.
* @get_sda: This gets current value of sda line. For GPIO recovery
* get_sda_gpio_value() is used here otherwise controller
[PATCH V8 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.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
V7-V8:
- Clk rate fixed to 100KHz
- Check SCL line to see if it is LOW due to some faults
- removed last use of unlikely() in earlier patch
- Enhanced comment over skip_sda_polling
V6-V7:
- Lots of review comments from Wolfram incorporated
- get[put]_gpio updated to more generic [un]prepare_recovery with return type as
void.
- prototypes of generic recovery routines are exposed to controller driver and
they are now required to pass recover_bus()
- gpio flags removed from recovery info
- default clock rate is 100 KHz if not passed by controller driver
- clk count is 9, fixed.
V5-V6:
- Removed sda_gpio_flags
- Make scl_gpio_flags as GPIOF_OPEN_DRAIN | GPIOF_OUT_INIT_HIGH by default
- update bri-set_scl and bri-get_sda for gpio recovery case in i2c core
- Guaranteed to generate 9 falling-rising edges for bus recovery
V4-V5:
- section name corrected to 3.1.16
- merged gpio and non-gpio recovery routines to remove code redundancy
- Changed types of gpio and gpio-flags to unsigned and unsigned long
- Checking return value of get_gpio() now
- using DIV_ROUND_UP for calculating delay, to get more correct value
drivers/i2c/i2c-core.c | 148 +
include/linux/i2c.h| 50 +
2 files changed, 198 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a7edf98..9b6a1a6 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
@@ -104,6 +106,122 @@ 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(100, DEFAULT_CLK_RATE * 2);
+
+static int get_sda_gpio_value(struct i2c_adapter *adap)
+{
+ return gpio_get_value(adap-bus_recovery_info-sda_gpio);
+}
+
+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 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-skip_sda_polling) {
+ 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-skip_sda_polling = true;
+ }
+ }
+
+ 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-skip_sda_polling)
+ 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, val = 0;
+
+ if (bri-prepare_recovery)
+ bri-prepare_recovery(bri);
+
+ /* SCL shouldn't be low here */
+ if (!bri-get_scl(adap)) {
+ dev_err(adap-dev, SCL is stuck Low, exiting recovery.\n);
+ goto unprepare;
+ }
+
+ /*
+* By this time SCL is high, as we need to give 9 falling-rising edges
+*/
+ for (i = 0; i RECOVERY_CLK_CNT * 2; i++, val = !val) {
+ bri-set_scl(adap, val);
+ ndelay
Re: [PATCH V7 1/2] i2c/adapter: Add bus recovery infrastructure
On 26 November 2012 17:15, Paul Carpenter p...@pcserviceselectronics.co.uk wrote: Unless you know why the bus is stuck, how can you reset reminder this method ONLY works if SCL is high and SDA probably was low and the slave device is what is stuck in wrong state. NOTE SCL and SDA high can be considered as BUS IDLE or having passed SDA = 1 waiting next clock edge or state transition without other state information. From i2c protocol Rev. 03 section 3.1.16 titled Bus clear first bit - In the unlikely event where the clock (SCL) is stuck LOW, the preferential procedure is to reset the bus using the HW reset signal if your I2C devices have HW reset inputs. If the I2C devices do not have HW reset inputs, cycle power to the devices to activate the mandatory internal Power-On Reset (POR) circuit. So if you do not check that SCL is NOT stuck Low the rest will just do nothing, it wont clear the bus and the fault will not be reported. The open drain nature of the bus means that you may attempt to toggle it but if SCL is stuck low nothing will happen on the bus. Attempting to configure an SCL drive as any form of high driving output will create a situation that when the SCL is driven high a high driver on the GPIO will be driving into a stuck low situation potentially creating a short between a power source and a power sink, that in most cases will damage or shorten life of one or both ends. In over 10 years of using I2C in various forms the only times I have seen bus stuck or device in wrong state 1/ Some code changed a bus switch when Bus was NOT idle i.e mid transaction 2/ Bit-banged I2C software was wrong incomplete transaction 3/ Short on bus 4/ Faulty device 5/ Faulty design so 1 and 0 thresholds were compromised If you ever support multi-master you need to monitor SCL to ensure your clock toggling is doing what you expect. Hi Paul, I completely agree with what you said, and i also feel the need of it now. We don't have a solution for SCL stuck low, just by playing with pad values. And need some sort of reset of device, etc as mentioned by you earlier. What we can do here, is check SCL's value and give an error message for SCL stuck low + return error. The short circuit mentioned earlier will not happen in our case as we are setting gpio flags as OPEN_DRAIN and so we would never be writing 1 on SCL pad. Though i would still need confirmation from Wolfram before implementing this change. Checking that would be tricky. For GPIO recovery, we have to make it GPIO first in input mode and read its value. Right? Depends on the GPIO, some even when set as output can still be read as that is the way the GPIO registers are constructed in some cases. Yes. Probably can read it in output mode too. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 1/2] i2c/adapter: Add bus recovery infrastructure
On 26 November 2012 18:58, Paul Carpenter p...@pcserviceselectronics.co.uk wrote: +ret = gpio_request_one(bri-scl_gpio, GPIOF_OPEN_DRAIN | +GPIOF_OUT_INIT_HIGH, i2c-scl); I always get wary of people driving I2C with non-open-drain, especially with stuck busses I would want to know what GPIOF_OUT_INIT_HIGH did as that is suspiciously driving output high at startup. I didn't get the question correctly. You mean you want me to explain what GPIOF_OPEN_DRAIN and GPIOF_OUT_INIT_HIGH will do? If no, exit(); GPIOF_OPEN_DRAIN: With this flag, all will work normally, leaving gpio_set_val(1); This will put the GPIO in input mode instead of writing one to it in output mode. GPIOF_OUT_INIT_HIGH: This will set the direction to output first, and then will do gpio_set_val(1), which will do what is mentioned in GPIOF_OPEN_DRAIN and so this action should be safe enough. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 1/2] i2c/adapter: Add bus recovery infrastructure
On 27 November 2012 01:50, Uwe Kleine-König u.kleine-koe...@pengutronix.de wrote: In reply to a more detailed problem report that should be addressed by this series on spear-devel I asked a few questions to rule out 6/ and 7/. I cannot find it in an public accessible archive though. It's about a sta529 codec. When switching sample rates the codec then holds down SDA. After an additional clock pulse the device and bus are OK again. Thanks for adding these here :) -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 1/2] i2c/adapter: Add bus recovery infrastructure
On 25 November 2012 17:22, Paul Carpenter
p...@pcserviceselectronics.co.uk wrote:
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
You should also take note of section 3.1.14 and its notes on Software
Reset -
Precautions must be taken to ensure that a device is not
pulling down the SDA or SCL line after applying the supply
voltage, since these low levels would block the bus
Hmm.. This patch has taken a very long time for being accepted, and the
reason for that was, it was generalizing much more than what is required.
And I am not sure, if checking SCL low would be considered like that only. :)
@Wolfram: I would need your point of view on that, before trying it out.
+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;
Where is the check that SCL is NOT low (bus fault or device doing clock
stretching)
Pending for Wolfram's point of view :)
+static int i2c_generic_recovery(struct i2c_adapter *adap)
+{
+/*
+ * By this time SCL is high, as we need to give 9 falling-rising
edges
+ */
In my view that needs to be done by an actual check of the real SCL not
assumption.
Checking that would be tricky. For GPIO recovery, we have to make it GPIO
first in input mode and read its value. Right?
+delay = DIV_ROUND_UP(delay, bri-clock_rate_khz * 2);
+
+for (i = 0; i RECOVERY_CLK_CNT * 2; i++, val = !val) {
+bri-set_scl(adap, val);
+ndelay(delay);
+
+/* break if sda got high, check only when scl line is high */
+if (!bri-skip_sda_polling val)
Dont use 'val' read the actual SCL line which ensures you you are not
wasting your time because of hardware fault. Possibly saving your GPIO
from being stuck permanently.
Again, wolfram can decide if it is really required :)
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6 1/2] i2c/adapter: Add bus recovery infrastructure
Hi Wolfram,
Thanks for sharing your opinion.
On 25 November 2012 02:29, Wolfram Sang w.s...@pengutronix.de wrote:
On Thu, Oct 04, 2012 at 04:34:53PM +0530, Viresh Kumar wrote:
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
+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;
+
+ if (bri-get_gpio) {
+ ret = bri-get_gpio(bri-scl_gpio);
+ if (ret) {
+ dev_warn(dev, scl get_gpio: %d\n, bri-scl_gpio);
This warning is probably not very helpful to a user.
This is what i thought about it: This is a platform specific routine and
most probably it will fail the first time this code is ever hit and so a
warning would be very helpful for them.
But, now i think platforms should have these prints in their get_gpio
implementations. And we can make it have return type void.
+ return ret;
+ }
+ }
+
+ ret = gpio_request_one(bri-scl_gpio, bri-scl_gpio_flags, i2c-scl);
+ if (ret) {
+ dev_warn(dev, gpio request fail: %d\n, bri-scl_gpio);
+ goto scl_put_gpio;
+ }
+
+ if (!bri-skip_sda_polling) {
+ if (bri-get_gpio)
+ ret = bri-get_gpio(bri-sda_gpio);
+
+ if (unlikely(ret ||
Since the unlikely() are not in hot-paths, you probably better skip
them.
It will be removed as get_gpio() would have return type void.
+ 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-skip_sda_polling = true;
+ if (!ret bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+
+ ret = 0;
+ }
+ }
+static int i2c_recover_bus(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ unsigned long delay = 100;
What is this magic value?
10 ^ 6 For time conversion to ns. clock duration is given by:
1/ (1000 * KHz) sec = 10^6 /(10^6 * 1000 * KHz) sec = 10^6/KHz nsec
+/**
+ * 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.
What about having those options?
NULL or custom_pointer or i2c_generic_scl_recovery or
i2c_generic_gpio_recovery
Looks good. But NULL should be an error then ?
where i2c_generic_gpio_recovery is probably:
get_gpios_for_recovery
i2c_generic_scl_recovery
put_gpios_for_recovery
Ok.
and i2c_generic_scl_recovery is basically your current
i2c_generic_recovery. That makes it easier to add other generic routines
if that should ever become necessary.
yes, that a good point.
+ * @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.
Does a user really need to set this?
This was done for platforms and i2c controllers which don't have configuration
to control scl line. So i still feel we need it.
+ * @is_gpio_recovery: true, select gpio type else scl type. Only required if
+ * recover_bus == NULL.
This could be dropped in favor of i2c_generic_*_recovery in recover_bus
Yes.
+ * @clock_rate_khz: clock rate of dummy clock in khz. Required for both
gpio and
+ * scl type recovery.
Does a user really need this? We could probably use something close to
100kHz always?
Not sure. Doesn't it depend on the current clk rate of controller ?
If not then yes it can be 100 KHz
+ * @clock_cnt: count of max clocks to be generated. Required for both gpio
and
+ * scl type recovery.
Don't think this should be something else than 9. If so, it should be
increased generally in the core and not inside some platform data.
I don't remember well, but somebody pointed it out in earlier version as
they require more of these. But will drop it for now and make it 9.
+ * @set_scl: controller specific routine, if is_gpio_recovery == false.
+ * set_scl_gpio_value otherwise
+ * @get_sda: controller specific routine, if is_gpio_recovery == false.
+ * get_sda_gpio_value otherwise
Basically OK, documentation should be more user-centric not
implementation centric :)
Yes, i realize it now.
+ * @get_gpio: called before recover_bus() to get padmux configured for scl
line.
+ * as gpio. Only required if is_gpio_recovery == true. Return 0 on
success.
+ * @put_gpio: called after recover_bus() to get padmux configured for scl
line
+ * as scl. Only required if is_gpio_recovery == true.
I wonder if it makes sense to have
Re: [PATCH V6 2/2] i2c/designware: Provide i2c bus recovery support
On 25 November 2012 02:33, Wolfram Sang w.s...@pengutronix.de wrote:
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
@@ -538,7 +538,12 @@ 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 (adap-bus_recovery_info
+ adap-bus_recovery_info-recover_bus) {
+ dev_dbg(dev-dev, try i2c bus recovery\n);
+ adap-bus_recovery_info-recover_bus(adap);
+ }
+
This should be in the core?
Because wait_for_completion() would fail in controllers, so i kept this
code here. How will we come to know about xfer failure in core?
Though i realize that check for adap-bus_recovery_info-recover_bus()
is just not required.
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
+ /* Bus recovery support */
+ pdata = dev_get_platdata(pdev-dev);
+ if (pdata) {
+ recovery_info = kzalloc(sizeof(*recovery_info), GFP_KERNEL);
+ if (!recovery_info) {
+ adap-bus_recovery_info = NULL;
+ dev_err(pdev-dev,
+ failure to allocate memory for bus
recovery\n);
+ goto skip_recovery;
+ }
+
+ recovery_info-is_gpio_recovery = true;
+ recovery_info-get_gpio = pdata-get_gpio;
+ recovery_info-put_gpio = pdata-put_gpio;
+ recovery_info-scl_gpio = pdata-scl_gpio;
+ recovery_info-scl_gpio_flags = pdata-scl_gpio_flags;
+ recovery_info-clock_rate_khz = clk_get_rate(dev-clk) / 1000;
It is probably easier to define the whole bri structure in the platform
and simply pass it on here? Hmm, now devicetree also comes to my mind.
Will think about that a little, too, but main implementation should be
left for someone needing that.
Ok. I didn't went for DT as recovery_info also has some routines, for which
we need special implementation for platforms.
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V7 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.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
Hi Wolfram,
I am sending V7 before closing our comments completely on V6 (Very few are left
now :) ), so that you can get an idea of what i am implementing now. It
incorporates all your comments leaving:
- removing skip_sda_polling
- removing clock_rate_khz
- calling recover_bus from core instead of controller driver
V6-V7:
- Lots of review comments from Wolfram incorporated
- get[put]_gpio updated to more generic [un]prepare_recovery with return type as
void.
- prototypes of generic recovery routines are exposed to controller driver and
they are now required to pass recover_bus()
- gpio flags removed from recovery info
- default clock rate is 100 KHz if not passed by controller driver
- clk count is 9, fixed.
V5-V6:
- Removed sda_gpio_flags
- Make scl_gpio_flags as GPIOF_OPEN_DRAIN | GPIOF_OUT_INIT_HIGH by default
- update bri-set_scl and bri-get_sda for gpio recovery case in i2c core
- Guaranteed to generate 9 falling-rising edges for bus recovery
V4-V5:
- section name corrected to 3.1.16
- merged gpio and non-gpio recovery routines to remove code redundancy
- Changed types of gpio and gpio-flags to unsigned and unsigned long
- Checking return value of get_gpio() now
- using DIV_ROUND_UP for calculating delay, to get more correct value
drivers/i2c/i2c-core.c | 136 +
include/linux/i2c.h| 47 +
2 files changed, 183 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a7edf98..d100276 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
@@ -104,6 +106,106 @@ 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 */
+
+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-skip_sda_polling) {
+ 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-skip_sda_polling = true;
+ }
+ }
+
+ 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-skip_sda_polling)
+ 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;
+ unsigned long delay = 100; /* 10^6/KHz for delay in ns */
+ int i, val = 0;
+
+ if (bri-prepare_recovery)
+ bri-prepare_recovery(bri);
+
+ /*
+* By this time SCL is high, as we need to give 9 falling-rising edges
+*/
+
+ delay = DIV_ROUND_UP(delay, bri-clock_rate_khz * 2);
+
+ for (i = 0; i RECOVERY_CLK_CNT * 2; i++, val = !val) {
+ bri-set_scl(adap, val);
+ ndelay(delay);
+
+ /* break if sda got high, check only when scl line is high */
+ if (!bri-skip_sda_polling val
[PATCH V7 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 designware I2C controller.
Signed-off-by: Vincenzo Frascino vincenzo.frasc...@st.com
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
V6-V7:
- removed include/linux/i2c/i2c-designware.h and use struct
i2c_bus_recovery_info as platform data.
- initialize recover_bus with i2c_generic_gpio_recovery()
drivers/i2c/busses/i2c-designware-core.c| 6 +-
drivers/i2c/busses/i2c-designware-platdrv.c | 18 --
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index cbba7db..24feaaf 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -538,7 +538,11 @@ 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 (adap-bus_recovery_info) {
+ dev_dbg(dev-dev, try i2c bus recovery\n);
+ adap-bus_recovery_info-recover_bus(adap);
+ }
+
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 0506fef..c7725b2 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -55,6 +55,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_bus_recovery_info *recovery_info;
int irq, r;
/* NOTE: driver uses the static register mapping */
@@ -141,17 +142,28 @@ 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 */
+ recovery_info = dev_get_platdata(pdev-dev);
+ if (recovery_info) {
+ recovery_info-recover_bus = i2c_generic_gpio_recovery;
+ recovery_info-clock_rate_khz = clk_get_rate(dev-clk) / 1000;
+ adap-bus_recovery_info = recovery_info;
+ } else {
+ adap-bus_recovery_info = NULL;
+ }
+
adap-nr = pdev-id;
r = i2c_add_numbered_adapter(adap);
if (r) {
dev_err(pdev-dev, failure adding adapter\n);
- goto err_free_irq;
+ goto err_free_recovery_info;
}
of_i2c_register_devices(adap);
return 0;
-err_free_irq:
+err_free_recovery_info:
+ kfree(recovery_info);
free_irq(dev-irq, dev);
err_iounmap:
iounmap(dev-base);
@@ -174,6 +186,8 @@ static int __devexit dw_i2c_remove(struct platform_device
*pdev)
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
struct resource *mem;
+ kfree(dev-adapter.bus_recovery_info);
+
platform_set_drvdata(pdev, NULL);
i2c_del_adapter(dev-adapter);
put_device(pdev-dev);
--
1.7.12.rc2.18.g61b472e
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V6 1/2] i2c/adapter: Add bus recovery infrastructure
On 4 October 2012 16:34, Viresh Kumar viresh.ku...@linaro.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. Hi Wolfram, I haven't pinged you earlier as i knew merge window is around. Can you please share your opinion for this now? -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
On 4 October 2012 13:30, Uwe Kleine-König
u.kleine-koe...@pengutronix.de wrote:
On Fri, Sep 28, 2012 at 04:58:43PM +0530, Viresh Kumar wrote:
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
+static inline void set_scl_value(struct i2c_adapter *adap, int val)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+
+ if (bri-is_gpio_recovery)
+ gpio_set_value(bri-scl_gpio, val);
+ else
+ bri-set_scl(adap, val);
I would have done this in a different way (with function pointers
instead of an if for each bang). Well, I don't care much.
That would be better. Will be fixed :)
+static int i2c_recover_bus(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ unsigned long delay = 100;
+ int i, ret, val = 1;
+
+ if (bri-is_gpio_recovery) {
+ ret = i2c_get_gpios_for_recovery(adap);
+ if (ret)
+ return ret;
+ }
+
+ delay = DIV_ROUND_UP(delay, bri-clock_rate_khz * 2);
+
+ for (i = 0; i bri-clock_cnt * 2; i++, val = !val) {
+ set_scl_value(adap, val);
+ ndelay(delay);
+
+ /* break if sda got high, check only when scl line is high */
+ if (!bri-skip_sda_polling val)
+ if (unlikely(get_sda_value(adap)))
+ break;
+ }
With clock_cnt usually being 9 (and skipping sda polling) assume a
device pulls down SDA because it was just addressed but the last clock
pulse to release SDA didn't occur:
SDAout ¯\_/¯¯¯\___/¯¯¯\___/¯¯
SCL¯¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯\_/¯
S 1 2 3 4 5 6 7 8 9
This adresses an eeprom with address 0x50. When SCL is released for the
9th clock the eeprom pulls down SDA to ack being addressed. After that
the eeprom expects the master to write a byte.
Now you do write 0xff:
i 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17
SDAin ___/¯¯\__
SCL¯¯\_/¯\_/¯\_/¯\_/¯\_/¯¯\__/¯¯\__/¯¯\__/¯¯\__/
9 1 2 3 45 6 7 8
(assuming the SCL line goes up some time later to its idle state).
That is you leave the bus in exactly the same state as before: The 9th
clock isn't complete and the eeprom asserts SDA low to ack the byte just
written. So the bus is still stalled. The problem is BTW the exact same
one I introduced first in my bus clear routine (for barebox though).
Even though you count to 2*9 you only do 8 real clock pulses because you
have a half cycle at both the start and the end.
Fantastic explanation!!
So, we actually need to do Low-High 9 times instead of High-Low.
So, initializing val to 0 should fix it?
+ * @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.
Didn't you want to change this to GPIOF_OUT_INIT_HIGH |
GPIOF_OPEN_DRAIN? Hmm, I just wonder how to distinguish
GPIOF_OUT_INIT_LOW from unset. Hmm, maybe assume unset because _LOW is
wrong?
Hmmm H... Hmmm... :)
Two things:
- Why should we default it to GPIOF_OPEN_DRAIN? Open drain would mean,
gpio_set_value() will not write one for it, but would put it in input mode.
I don't think that would be correct, as an generic case.
- Obviously _LOW seems to be incorrect. So we can actually do something as
simple as: pass (sda_gpio_flags | GPIOF_OUT_INIT_HIGH). That will work
in both cases, user passed a value or not.
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
On 4 October 2012 14:50, Uwe Kleine-König u.kleine-koe...@pengutronix.de wrote: So, we actually need to do Low-High 9 times instead of High-Low. So, initializing val to 0 should fix it? I don't think this is enough. If you cut off the last half clock of the first sequence above doing 9 times low-high isn't enough. So you have to do high + 9x low-high to assert 9 full cycles. I am not cutting the last half clock. val is the variable which keeps track of value to be set on the line. I am asking to start from zero. You mean, we should do a high first and then 9 low-high? But this line was high initially.. what difference will it make by making it high again? Sorry, i am not a I2C expert, so need some help :) + * @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. Didn't you want to change this to GPIOF_OUT_INIT_HIGH | GPIOF_OPEN_DRAIN? Hmm, I just wonder how to distinguish GPIOF_OUT_INIT_LOW from unset. Hmm, maybe assume unset because _LOW is wrong? Hmmm H... Hmmm... :) Two things: - Why should we default it to GPIOF_OPEN_DRAIN? Open drain would mean, gpio_set_value() will not write one for it, but would put it in input mode. I don't think that would be correct, as an generic case. As the i2c-bus is open drain and multi-master capable it's wrong to pull it to 1 because this can result in a short circuit. Even in a single-master scenario (where you can pull SCL in both directions) you must not drive SDA to 1. Hmm... Hopefully i understood it well this time. - We actually don't need these flags then. Always pass: - scl_flags: GPIOF_OPEN_DRAIN | GPIOF_OUT_INIT_HIGH - sda_flags: GPIOF_IN Why would anybody else require something different for any SoC? -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
On 4 October 2012 15:17, Uwe Kleine-König u.kleine-koe...@pengutronix.de wrote: On Thu, Oct 04, 2012 at 03:02:26PM +0530, Viresh Kumar wrote: On 4 October 2012 14:50, Uwe Kleine-König u.kleine-koe...@pengutronix.de wrote: So, we actually need to do Low-High 9 times instead of High-Low. So, initializing val to 0 should fix it? I don't think this is enough. If you cut off the last half clock of the first sequence above doing 9 times low-high isn't enough. So you have to do high + 9x low-high to assert 9 full cycles. I am not cutting the last half clock. val is the variable which keeps track of value to be set on the line. I am asking to start from zero. I meant the sequence that created the stall, not the one intending to clear it. If you remove the last rising edge from that the SCL line is initially low. But then, wouldn't only 8.5 cycles are enough? As with 8.5 cycles we will achieve 9 low-high cycles? I can't find you on IRC (#linaro on freenode). Want to finish this up quickly, so that i can send a fixup patch ASAP and get your reviewed-by :) -- viresh -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
On 4 October 2012 13:30, Uwe Kleine-König
u.kleine-koe...@pengutronix.de wrote:
Hi Uwe,
Please see if following fixup looks fine to you:
fixup! i2c/adapter: Add bus recovery infrastructure
---
drivers/i2c/i2c-core.c | 33 ++---
include/linux/i2c.h| 22 --
2 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index bdc249a..393d5f7 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -107,24 +107,14 @@ static int i2c_device_uevent(struct device *dev,
struct kobj_uevent_env *env)
#endif /* CONFIG_HOTPLUG */
/* i2c bus recovery routines */
-static inline void set_scl_value(struct i2c_adapter *adap, int val)
+static void set_scl_gpio_value(struct i2c_adapter *adap, int val)
{
- struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
-
- if (bri-is_gpio_recovery)
- gpio_set_value(bri-scl_gpio, val);
- else
- bri-set_scl(adap, val);
+ gpio_set_value(adap-bus_recovery_info-scl_gpio, val);
}
-static inline int get_sda_value(struct i2c_adapter *adap)
+static int get_sda_gpio_value(struct i2c_adapter *adap)
{
- struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
-
- if (bri-is_gpio_recovery)
- return gpio_get_value(bri-sda_gpio);
- else
- return bri-get_sda(adap);
+ return gpio_get_value(adap-bus_recovery_info-sda_gpio);
}
static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
@@ -152,8 +142,7 @@ static int i2c_get_gpios_for_recovery(struct
i2c_adapter *adap)
ret = bri-get_gpio(bri-sda_gpio);
if (unlikely(ret ||
- gpio_request_one(bri-sda_gpio, bri-sda_gpio_flags,
- i2c-sda))) {
+ 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);
@@ -190,7 +179,7 @@ static int i2c_recover_bus(struct i2c_adapter *adap)
{
struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
unsigned long delay = 100;
- int i, ret, val = 1;
+ int i, ret, val = 0;
if (bri-is_gpio_recovery) {
ret = i2c_get_gpios_for_recovery(adap);
@@ -201,12 +190,12 @@ static int i2c_recover_bus(struct i2c_adapter *adap)
delay = DIV_ROUND_UP(delay, bri-clock_rate_khz * 2);
for (i = 0; i bri-clock_cnt * 2; i++, val = !val) {
- set_scl_value(adap, val);
+ bri-set_scl(adap, val);
ndelay(delay);
/* break if sda got high, check only when scl line is high */
if (!bri-skip_sda_polling val)
- if (unlikely(get_sda_value(adap)))
+ if (unlikely(bri-get_sda(adap)))
break;
}
@@ -1030,6 +1019,12 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
bri-recover_bus = i2c_recover_bus;
if (bri-is_gpio_recovery) {
+ if (!bri-scl_gpio_flags)
+ bri-scl_gpio_flags = GPIOF_OPEN_DRAIN |
+ GPIOF_OUT_INIT_HIGH;
+
+ bri-set_scl = set_scl_gpio_value;
+ bri-get_sda = get_sda_gpio_value;
dev_info(adap-dev,
registered for gpio bus recovery\n);
} else if (bri-set_scl) {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index c43e5c4..dd470a1 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -382,10 +382,10 @@ struct i2c_algorithm {
* 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.
+ * @set_scl: controller specific routine, if is_gpio_recovery == false.
+ * set_scl_gpio_value otherwise
+ * @get_sda: controller specific routine, if is_gpio_recovery == false.
+ * get_sda_gpio_value otherwise
* @get_gpio: called before recover_bus() to get padmux configured
for scl line.
* as gpio. Only required if is_gpio_recovery == true. Return 0 on success.
* @put_gpio: called after recover_bus() to get padmux configured for scl line
@@ -394,10 +394,9 @@ struct i2c_algorithm {
* true.
* @sda_gpio: gpio number of the sda line. Only required if is_gpio_recovery ==
* true and skip_sda_polling == false.
- *
[PATCH V6 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.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
V5-V6:
- Removed sda_gpio_flags
- Make scl_gpio_flags as GPIOF_OPEN_DRAIN | GPIOF_OUT_INIT_HIGH by default
- update bri-set_scl and bri-get_sda for gpio recovery case in i2c core
- Guaranteed to generate 9 falling-rising edges for bus recovery
V4-V5:
- section name corrected to 3.1.16
- merged gpio and non-gpio recovery routines to remove code redundancy
- Changed types of gpio and gpio-flags to unsigned and unsigned long
- Checking return value of get_gpio() now
- using DIV_ROUND_UP for calculating delay, to get more correct value
drivers/i2c/i2c-core.c | 156 +
include/linux/i2c.h| 55 +
2 files changed, 211 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a7edf98..e78033b 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
@@ -104,6 +106,111 @@ 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 */
+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;
+
+ if (bri-get_gpio) {
+ ret = bri-get_gpio(bri-scl_gpio);
+ if (ret) {
+ dev_warn(dev, scl get_gpio: %d\n, bri-scl_gpio);
+ return ret;
+ }
+ }
+
+ ret = gpio_request_one(bri-scl_gpio, bri-scl_gpio_flags, i2c-scl);
+ if (ret) {
+ dev_warn(dev, gpio request fail: %d\n, bri-scl_gpio);
+ goto scl_put_gpio;
+ }
+
+ if (!bri-skip_sda_polling) {
+ if (bri-get_gpio)
+ ret = bri-get_gpio(bri-sda_gpio);
+
+ if (unlikely(ret ||
+ 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-skip_sda_polling = true;
+ if (!ret bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+
+ ret = 0;
+ }
+ }
+
+scl_put_gpio:
+ if (bri-put_gpio)
+ bri-put_gpio(bri-scl_gpio);
+
+ return ret;
+}
+
+static void i2c_put_gpios_for_recovery(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+
+ gpio_free(bri-scl_gpio);
+
+ if (!bri-skip_sda_polling) {
+ gpio_free(bri-sda_gpio);
+
+ if (bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+ }
+}
+
+static int i2c_recover_bus(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ unsigned long delay = 100;
+ int i, ret, val = 0;
+
+ if (bri-is_gpio_recovery) {
+ ret = i2c_get_gpios_for_recovery(adap);
+ if (ret)
+ return ret;
+ } else {
+ bri-set_scl(adap, 1);
+ }
+
+ /*
+* By this time SCL is high, as we need to give 9 falling-rising edges
+*/
+
+ delay = DIV_ROUND_UP(delay, bri-clock_rate_khz * 2);
+
+ for (i = 0; i bri-clock_cnt * 2; i++, val = !val) {
+ bri-set_scl(adap, val);
+ ndelay(delay);
+
+ /* break if sda got high, check only when scl line is high */
+ if (!bri-skip_sda_polling val)
+ if (unlikely(bri-get_sda(adap
[PATCH V6 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.
Signed-off-by: Vincenzo Frascino vincenzo.frasc...@st.com
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
V5-V6:
- removed sda_gpio_flags
drivers/i2c/busses/i2c-designware-core.c| 7 -
drivers/i2c/busses/i2c-designware-platdrv.c | 38 ++--
include/linux/i2c/i2c-designware.h | 46 +
3 files changed, 88 insertions(+), 3 deletions(-)
create mode 100644 include/linux/i2c/i2c-designware.h
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index cbba7db..9b7c9bf 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -538,7 +538,12 @@ 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 (adap-bus_recovery_info
+ adap-bus_recovery_info-recover_bus) {
+ dev_dbg(dev-dev, try i2c bus recovery\n);
+ adap-bus_recovery_info-recover_bus(adap);
+ }
+
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 0506fef..afa0df9 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -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,7 @@ static struct i2c_algorithm i2c_dw_algo = {
.master_xfer= i2c_dw_xfer,
.functionality = i2c_dw_func,
};
+
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
{
return clk_get_rate(dev-clk)/1000;
@@ -55,6 +57,8 @@ 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;
+ struct i2c_bus_recovery_info *recovery_info = NULL;
int irq, r;
/* NOTE: driver uses the static register mapping */
@@ -141,17 +145,45 @@ 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) {
+ recovery_info = kzalloc(sizeof(*recovery_info), GFP_KERNEL);
+ if (!recovery_info) {
+ adap-bus_recovery_info = NULL;
+ dev_err(pdev-dev,
+ failure to allocate memory for bus
recovery\n);
+ goto skip_recovery;
+ }
+
+ recovery_info-is_gpio_recovery = true;
+ recovery_info-get_gpio = pdata-get_gpio;
+ recovery_info-put_gpio = pdata-put_gpio;
+ recovery_info-scl_gpio = pdata-scl_gpio;
+ recovery_info-scl_gpio_flags = pdata-scl_gpio_flags;
+ recovery_info-clock_rate_khz = clk_get_rate(dev-clk) / 1000;
+
+ if (!pdata-skip_sda_polling)
+ recovery_info-sda_gpio = pdata-sda_gpio;
+
+ adap-bus_recovery_info = recovery_info;
+ } else {
+ adap-bus_recovery_info = NULL;
+ }
+
+skip_recovery:
adap-nr = pdev-id;
r = i2c_add_numbered_adapter(adap);
if (r) {
dev_err(pdev-dev, failure adding adapter\n);
- goto err_free_irq;
+ goto err_free_recovery_info;
}
of_i2c_register_devices(adap);
return 0;
-err_free_irq:
+err_free_recovery_info:
+ kfree(recovery_info);
free_irq(dev-irq, dev);
err_iounmap:
iounmap(dev-base);
@@ -174,6 +206,8 @@ static int __devexit dw_i2c_remove(struct platform_device
*pdev)
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
struct resource *mem;
+ kfree(dev-adapter.bus_recovery_info);
+
platform_set_drvdata(pdev, NULL);
i2c_del_adapter(dev-adapter);
put_device(pdev-dev);
diff --git a/include/linux/i2c/i2c-designware.h
b/include/linux/i2c/i2c-designware.h
new file mode 100644
index 000..522dec0
--- /dev/null
+++ b/include/linux/i2c/i2c-designware.h
@@ -0,0 +1,46 @@
+/*
+ * Synopsys DesignWare I2C adapter driver's platform data
+ *
+ * Copyright (C) 2012 ST Microelectronics.
+ * Author: Vincenzo Frascino vincenzo.frasc...@st.com
+ *
+ * This software is licensed under
Re: [PATCH V4 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
On 28 September 2012 12:30, Wolfram Sang w.s...@pengutronix.de wrote: I am trying to have another I2C weekend this weekend. And your patches have been scheduled for that. The generic feeling is: Very useful but the interface could probably be simplified. This is why it takes me so long, working on interfaces needs more thought than the average patch review :/ Hi, Did this poor fellow got some attention this weekend? :) [PATCH V5 Resend 1/2] i2c/adapter: Add bus recovery infrastructure -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
On 28 September 2012 16:58, Viresh Kumar viresh.ku...@linaro.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. Hi Uwe, If you have gone through V5 and have no noticeable issues with it, can it have a reviewed-by: from you? -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
On 28 September 2012 12:30, Wolfram Sang w.s...@pengutronix.de wrote: I am trying to have another I2C weekend this weekend. And your patches have been scheduled for that. The generic feeling is: Very useful but the interface could probably be simplified. This is why it takes me so long, working on interfaces needs more thought than the average patch review :/ Glad to hear that. Even i would like to make it better :) Waiting for your comments. -- Viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
Hi Uwe, On 28 September 2012 12:57, Uwe Kleine-König u.kleine-koe...@pengutronix.de wrote: On Fri, Sep 28, 2012 at 09:11:34AM +0530, viresh kumar wrote: This was done, because few platforms may not have configuration bits to read status of sda line.. For them skip_sda_polling was required. Whereas, others would need this to see if we can return early. What is the upside of returning early? I'd say, just don't do it. Save time. Why give additional clocks when you don't actually need them? Can use likely/unlikely to make it more efficient for 9 clock pulse scenario though. + * @put_gpio: called after recover_bus() to get padmux configured for scl line + * as scl. Only required if is_gpio_recovery == true. + * @scl_gpio: gpio number of the scl line. Only required if is_gpio_recovery == + * true. + * @sda_gpio: gpio number of the sda line. Only required if is_gpio_recovery == + * true and skip_sda_polling == false. + * @scl_gpio_flags: flag for gpio_request_one of scl_gpio. 0 implies + * GPIOF_OUT_INIT_LOW. IMHO you should not make this configurable but use GPIOF_OUT_INIT_HIGH | GPIOF_OPEN_DRAIN Discussed here: http://www.spinics.net/lists/linux-i2c/msg07325.html Why do you default to GPIOF_OUT_INIT_LOW? The idle state of scl is high. Using low here introduces an additional clk pulse. We aren't giving any clock pulses to scl line. Are you talking about sda? -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
On 28 September 2012 12:57, Uwe Kleine-König
u.kleine-koe...@pengutronix.de wrote:
Why do you default to GPIOF_OUT_INIT_LOW? The idle state of scl is high.
Using low here introduces an additional clk pulse.
Went deep into the code i wrote ages ago to check why i did so :)
My initial idea was, because the idle state of scl is high, giving another high
initially with gpio would be a waste. As the slave will not notice a change.
So, i will do it low and then following code will run with delay before setting
gpio again. This will ensure, the initial gpio-set is used as clock.
for (i = 0; i bri-clock_cnt * 2; i++, val = !val) {
ndelay(delay);
gpio_set_value(bri-scl_gpio, val);
...
}
Yes, you are correct in saying that i have generated 9 *2 + 1 = 19
half-clocks...
or 9.5 clocks. Will fix it by making initial value of i as 1.
Also, will take care of this when user sends his own flags :)
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V5 1/2] i2c/adapter: Add bus recovery infrastructure
From: Viresh Kumar viresh.ku...@st.com
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.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
V4-V5:
- section name corrected to 3.1.16
- merged gpio and non-gpio recovery routines to remove code redundancy
- Changed types of gpio and gpio-flags to unsigned and unsigned long
- Checking return value of get_gpio() now
- using DIV_ROUND_UP for calculating delay, to get more correct value
drivers/i2c/i2c-core.c | 153 +
include/linux/i2c.h| 52 +
2 files changed, 205 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a7edf98..bdc249a 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
@@ -104,6 +106,116 @@ 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 */
+static inline void set_scl_value(struct i2c_adapter *adap, int val)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+
+ if (bri-is_gpio_recovery)
+ gpio_set_value(bri-scl_gpio, val);
+ else
+ bri-set_scl(adap, val);
+}
+
+static inline int get_sda_value(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+
+ if (bri-is_gpio_recovery)
+ return gpio_get_value(bri-sda_gpio);
+ else
+ return bri-get_sda(adap);
+}
+
+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;
+
+ if (bri-get_gpio) {
+ ret = bri-get_gpio(bri-scl_gpio);
+ if (ret) {
+ dev_warn(dev, scl get_gpio: %d\n, bri-scl_gpio);
+ return ret;
+ }
+ }
+
+ ret = gpio_request_one(bri-scl_gpio, bri-scl_gpio_flags, i2c-scl);
+ if (ret) {
+ dev_warn(dev, gpio request fail: %d\n, bri-scl_gpio);
+ goto scl_put_gpio;
+ }
+
+ if (!bri-skip_sda_polling) {
+ if (bri-get_gpio)
+ ret = bri-get_gpio(bri-sda_gpio);
+
+ if (unlikely(ret ||
+ gpio_request_one(bri-sda_gpio, bri-sda_gpio_flags,
+ i2c-sda))) {
+ /* work without sda polling */
+ dev_warn(dev, can't get sda: %d. Skip sda polling\n,
+ bri-sda_gpio);
+ bri-skip_sda_polling = true;
+ if (!ret bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+
+ ret = 0;
+ }
+ }
+
+scl_put_gpio:
+ if (bri-put_gpio)
+ bri-put_gpio(bri-scl_gpio);
+
+ return ret;
+}
+
+static void i2c_put_gpios_for_recovery(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+
+ gpio_free(bri-scl_gpio);
+
+ if (!bri-skip_sda_polling) {
+ gpio_free(bri-sda_gpio);
+
+ if (bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+ }
+}
+
+static int i2c_recover_bus(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ unsigned long delay = 100;
+ int i, ret, val = 1;
+
+ if (bri-is_gpio_recovery) {
+ ret = i2c_get_gpios_for_recovery(adap);
+ if (ret)
+ return ret;
+ }
+
+ delay = DIV_ROUND_UP(delay, bri-clock_rate_khz * 2);
+
+ for (i = 0; i bri-clock_cnt * 2; i++, val = !val) {
+ set_scl_value(adap, val);
+ ndelay(delay);
+
+ /* break if sda got high, check only when scl line is high */
+ if (!bri-skip_sda_polling val)
+ if (unlikely(get_sda_value(adap
[PATCH V5 2/2] i2c/designware: Provide i2c bus recovery support
From: Viresh Kumar viresh.ku...@st.com
Add bus recovery support for designware_i2c controller. It uses generic gpio
based i2c_gpio_recover_bus() routine.
Signed-off-by: Vincenzo Frascino vincenzo.frasc...@st.com
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
drivers/i2c/busses/i2c-designware-core.c| 7 -
drivers/i2c/busses/i2c-designware-platdrv.c | 40 +--
include/linux/i2c/i2c-designware.h | 49 +
3 files changed, 93 insertions(+), 3 deletions(-)
create mode 100644 include/linux/i2c/i2c-designware.h
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 7b8ebbe..3073178 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -538,7 +538,12 @@ 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 (adap-bus_recovery_info
+ adap-bus_recovery_info-recover_bus) {
+ dev_dbg(dev-dev, try i2c bus recovery\n);
+ adap-bus_recovery_info-recover_bus(adap);
+ }
+
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 0506fef..9e8b7e3 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -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,7 @@ static struct i2c_algorithm i2c_dw_algo = {
.master_xfer= i2c_dw_xfer,
.functionality = i2c_dw_func,
};
+
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
{
return clk_get_rate(dev-clk)/1000;
@@ -55,6 +57,8 @@ 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;
+ struct i2c_bus_recovery_info *recovery_info = NULL;
int irq, r;
/* NOTE: driver uses the static register mapping */
@@ -141,17 +145,47 @@ 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) {
+ recovery_info = kzalloc(sizeof(*recovery_info), GFP_KERNEL);
+ if (!recovery_info) {
+ adap-bus_recovery_info = NULL;
+ dev_err(pdev-dev,
+ failure to allocate memory for bus
recovery\n);
+ goto skip_recovery;
+ }
+
+ recovery_info-is_gpio_recovery = true;
+ recovery_info-get_gpio = pdata-get_gpio;
+ recovery_info-put_gpio = pdata-put_gpio;
+ recovery_info-scl_gpio = pdata-scl_gpio;
+ recovery_info-scl_gpio_flags = pdata-scl_gpio_flags;
+ recovery_info-clock_rate_khz = clk_get_rate(dev-clk) / 1000;
+
+ if (!pdata-skip_sda_polling) {
+ recovery_info-sda_gpio = pdata-sda_gpio;
+ recovery_info-sda_gpio_flags = pdata-sda_gpio_flags;
+ }
+
+ adap-bus_recovery_info = recovery_info;
+ } else {
+ adap-bus_recovery_info = NULL;
+ }
+
+skip_recovery:
adap-nr = pdev-id;
r = i2c_add_numbered_adapter(adap);
if (r) {
dev_err(pdev-dev, failure adding adapter\n);
- goto err_free_irq;
+ goto err_free_recovery_info;
}
of_i2c_register_devices(adap);
return 0;
-err_free_irq:
+err_free_recovery_info:
+ kfree(recovery_info);
free_irq(dev-irq, dev);
err_iounmap:
iounmap(dev-base);
@@ -174,6 +208,8 @@ static int __devexit dw_i2c_remove(struct platform_device
*pdev)
struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
struct resource *mem;
+ kfree(dev-adapter.bus_recovery_info);
+
platform_set_drvdata(pdev, NULL);
i2c_del_adapter(dev-adapter);
put_device(pdev-dev);
diff --git a/include/linux/i2c/i2c-designware.h
b/include/linux/i2c/i2c-designware.h
new file mode 100644
index 000..d60cb61c
--- /dev/null
+++ b/include/linux/i2c/i2c-designware.h
@@ -0,0 +1,49 @@
+/*
+ * Synopsys DesignWare I2C adapter driver's platform data
+ *
+ * Copyright (C) 2012 ST Microelectronics
[PATCH V4 Resend 1/2] i2c/adapter: Add bus recovery infrastructure
From: Viresh Kumar viresh.ku...@st.com
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.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
drivers/i2c/i2c-core.c | 125 +
include/linux/i2c.h| 52
2 files changed, 177 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 96ef3d8..7ede75d 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
@@ -104,6 +106,88 @@ 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 */
+static int i2c_gpio_recover_bus(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ unsigned long delay = 100;
+ int i, ret, val = 1;
+
+ if (bri-get_gpio)
+ bri-get_gpio(bri-scl_gpio);
+
+ ret = gpio_request_one(bri-scl_gpio, bri-scl_gpio_flags, i2c-scl);
+ if (ret 0) {
+ dev_warn(adap-dev, gpio request fail: %d\n, bri-scl_gpio);
+ return ret;
+ }
+
+ if (!bri-skip_sda_polling) {
+ if (bri-get_gpio)
+ bri-get_gpio(bri-sda_gpio);
+
+ ret = gpio_request_one(bri-sda_gpio, bri-sda_gpio_flags,
+ i2c-sda);
+ if (ret 0) {
+ /* work without sda polling */
+ dev_warn(adap-dev,
+ sda_gpio request fail: %d. Skip sda polling\n,
+ bri-scl_gpio);
+ bri-skip_sda_polling = true;
+ if (bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+ }
+ }
+
+ delay /= bri-clock_rate_khz * 2;
+
+ for (i = 0; i bri-clock_cnt * 2; i++, val = !val) {
+ ndelay(delay);
+ gpio_set_value(bri-scl_gpio, val);
+
+ /* break if sda got high, check only when scl line is high */
+ if (!bri-skip_sda_polling val)
+ if (gpio_get_value(bri-sda_gpio))
+ break;
+ }
+
+ gpio_free(bri-scl_gpio);
+
+ if (bri-put_gpio)
+ bri-put_gpio(bri-scl_gpio);
+
+ if (!bri-skip_sda_polling) {
+ gpio_free(bri-sda_gpio);
+
+ if (bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+ }
+
+ return 0;
+}
+
+static int i2c_scl_recover_bus(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ int i, val = 0;
+ unsigned long delay = 100;
+
+ delay /= bri-clock_rate_khz * 2;
+
+ for (i = 0; i bri-clock_cnt * 2; i++,
+ val = !val) {
+ bri-set_scl(adap, val);
+ ndelay(delay);
+
+ /* break if sda got high, check only when scl line is high */
+ if (!bri-skip_sda_polling val)
+ if (bri-get_sda(adap))
+ break;
+ }
+
+ return 0;
+}
+
static int i2c_device_probe(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
@@ -879,6 +963,47 @@ 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_info(adap-dev,
+ registered for non-generic bus recovery\n);
+ } else {
+ /* Use generic recovery routines */
+ if (!bri-clock_rate_khz) {
+ dev_warn(adap-dev,
+ doesn't have valid recovery clock
rate\n);
+ goto exit_recovery
[PATCH V4 Resend 2/2] i2c/designware: Provide i2c bus recovery support
From: Viresh Kumar viresh.ku...@st.com
Add bus recovery support for designware_i2c controller. It uses generic gpio
based i2c_gpio_recover_bus() routine.
Signed-off-by: Vincenzo Frascino vincenzo.frasc...@st.com
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
drivers/i2c/busses/i2c-designware-core.c| 7 -
drivers/i2c/busses/i2c-designware-platdrv.c | 24 ++
include/linux/i2c/i2c-designware.h | 49 +
3 files changed, 79 insertions(+), 1 deletion(-)
create mode 100644 include/linux/i2c/i2c-designware.h
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 1e48bec..f29f283 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -536,7 +536,12 @@ 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 (adap-bus_recovery_info
+ adap-bus_recovery_info-recover_bus) {
+ dev_dbg(dev-dev, try i2c bus recovery\n);
+ adap-bus_recovery_info-recover_bus(adap);
+ }
+
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 0506fef..3a7a4e8 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -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,11 @@ static struct i2c_algorithm i2c_dw_algo = {
.master_xfer= i2c_dw_xfer,
.functionality = i2c_dw_func,
};
+
+static struct i2c_bus_recovery_info dw_recovery_info = {
+ .is_gpio_recovery = true,
+};
+
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
{
return clk_get_rate(dev-clk)/1000;
@@ -55,6 +61,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 +148,23 @@ 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.get_gpio = pdata-get_gpio;
+ dw_recovery_info.put_gpio = pdata-put_gpio;
+ dw_recovery_info.scl_gpio = pdata-scl_gpio;
+ dw_recovery_info.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.sda_gpio = pdata-sda_gpio;
+ dw_recovery_info.sda_gpio_flags = pdata-sda_gpio_flags;
+ }
+
+ adap-bus_recovery_info = dw_recovery_info;
+ }
+
adap-nr = pdev-id;
r = i2c_add_numbered_adapter(adap);
if (r) {
diff --git a/include/linux/i2c/i2c-designware.h
b/include/linux/i2c/i2c-designware.h
new file mode 100644
index 000..341bd4c
--- /dev/null
+++ b/include/linux/i2c/i2c-designware.h
@@ -0,0 +1,49 @@
+/*
+ * Synopsys DesignWare I2C adapter driver's platform data
+ *
+ * Copyright (C) 2012 ST Microelectronics.
+ * Author: Vincenzo Frascino vincenzo.frasc...@st.com
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef I2C_DESIGNWARE_H
+#define I2C_DESIGNWARE_H
+
+#include linux/platform_device.h
+
+/*
+ * struct i2c_dw_pdata - Designware I2c platform data
+ * @scl_gpio: gpio number of the scl line.
+ * @scl_gpio_flags: flag for gpio_request_one of scl_gpio. 0 implies
+ * GPIOF_OUT_INIT_LOW.
+ * @get_gpio: called before recover_bus() to get padmux configured for scl line
+ * as gpio. Only required if is_gpio_recovery == true
+ * @put_gpio: called after recover_bus() to get padmux configured for scl line
+ * as scl. Only required if is_gpio_recovery == true
+ * @skip_sda_polling: if true, bus
Re: [PATCH V4 1/2] i2c/adapter: Add bus recovery infrastructure
On Tue, May 15, 2012 at 7:58 AM, Viresh Kumar viresh.ku...@st.com wrote: On 5/4/2012 3:10 PM, Viresh KUMAR wrote: 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. Signed-off-by: Viresh Kumar viresh.ku...@st.com --- 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. Ping. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 1/2] i2c/adapter: Add bus recovery infrastructure
On 5/4/2012 3:10 PM, Viresh KUMAR wrote: 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. Signed-off-by: Viresh Kumar viresh.ku...@st.com --- 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 -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[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.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
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
diff --git a/Documentation/i2c/bus-recovery b/Documentation/i2c/bus-recovery
new file mode 100644
index 000..518dff6
--- /dev/null
+++ b/Documentation/i2c/bus-recovery
@@ -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 viresh.ku...@st.com
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a6ad32b..dc0e93d8 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
[PATCH V4 0/2] 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 -- 1.7.9 -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 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.
Signed-off-by: Vincenzo Frascino vincenzo.frasc...@st.com
Signed-off-by: Shiraz Hashim shiraz.has...@st.com
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
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
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 1e48bec..0ef7ddc 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -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)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0506fef..e30651e 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -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) {
diff --git a/include/linux/i2c/i2c-designware.h
b/include/linux/i2c/i2c-designware.h
new file mode 100644
index 000..341bd4c
--- /dev/null
+++ b/include/linux/i2c/i2c-designware.h
@@ -0,0 +1,49 @@
+/*
+ * Synopsys DesignWare I2C adapter driver's platform data
+ *
+ * Copyright (C) 2012 ST Microelectronics.
+ * Author: Vincenzo Frascino vincenzo.frasc...@st.com
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef I2C_DESIGNWARE_H
+#define I2C_DESIGNWARE_H
+
+#include linux/platform_device.h
+
+/*
+ * struct i2c_dw_pdata - Designware I2c platform data
+ * @scl_gpio: gpio number of the scl line.
+ * @scl_gpio_flags: flag for gpio_request_one of scl_gpio. 0 implies
+ * GPIOF_OUT_INIT_LOW.
+ * @get_gpio: called before recover_bus() to get padmux configured for scl line
+ * as gpio. Only required if is_gpio_recovery == true
+ * @put_gpio: called after recover_bus() to get padmux configured for scl line
+ * as scl. Only required if is_gpio_recovery == true
Re: [PATCH V2] i2c: designware: Add clk_{un}prepare() support
On 4/22/12, Wolfram Sang w.s...@pengutronix.de wrote:
On Tue, Apr 17, 2012 at 05:04:31PM +0530, Viresh Kumar wrote:
clk_{un}prepare is mandatory for platforms using common clock framework.
Since
this driver is used by SPEAr platform, which supports common clock
framework,
add clk_{un}prepare() support for designware i2c.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
? Can't apply, which version is this against? Looks good otherwise.
Because following is waiting in you todo list and must be applied
before this one: :)
Author: Deepak Sikri deepak.si...@st.com
Date: Mon Sep 26 18:42:10 2011 +0530
i2c/busses: Add PM support
This patch adds in support for standby/S2R/hybernate for
i2c-designware driver.
Signed-off-by: Deepak Sikri deepak.si...@st.com
Signed-off-by: Rajeev Kumar rajeev-dlh.ku...@st.com
---
drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++
1 files changed, 27 insertions(+), 0 deletions(-)
Actually suspend/resume of above are also fixed by this patch.
What are your plans for following patchset:
i2c/adapter: Add bus recovery infrastructure
Would be good if you can apply that or ask for another version if you see some
shortcomings. Otherwise this kind of issues will happen again.
Please allocate some time for this, it is really important for other bus
drivers. All controllers/slaves have that bug.
They will be fixed only once this is applied. :)
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 0/2] I2C: Add bus recovery infrastructure
On 3/2/2012 11:53 AM, Viresh KUMAR wrote: 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 Hi Wolfram, I know that you have been very busy with other stuff, but Can you please apply this patchset if it looks fine to you? It had been in queue for over 1.5 months now :( -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i2c: designware: Add clk_{un}prepare() support
clk_{un}prepare is mandatory for platforms using common clock framework. Since
this driver is used by SPEAr platform, which supports common clock framework,
add clk_{un}prepare() support for designware i2c.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
drivers/i2c/busses/i2c-designware-platdrv.c | 19 ---
1 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 76bf108..7b50eec 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -97,13 +97,20 @@ static int __devinit dw_i2c_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, dev);
dev-clk = clk_get(pdev-dev, NULL);
- dev-get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
-
if (IS_ERR(dev-clk)) {
r = -ENODEV;
goto err_free_mem;
}
- clk_enable(dev-clk);
+
+ r = clk_prepare(dev-clk);
+ if (r)
+ goto err_put_clk;
+
+ r = clk_enable(dev-clk);
+ if (r)
+ goto err_unprepare_clk;
+
+ dev-get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
dev-functionality =
I2C_FUNC_I2C |
@@ -181,6 +188,9 @@ err_iounmap:
iounmap(dev-base);
err_unuse_clocks:
clk_disable(dev-clk);
+err_unprepare_clk:
+ clk_unprepare(dev-clk);
+err_put_clk:
clk_put(dev-clk);
dev-clk = NULL;
err_free_mem:
@@ -203,6 +213,7 @@ static int __devexit dw_i2c_remove(struct platform_device
*pdev)
put_device(pdev-dev);
clk_disable(dev-clk);
+ clk_unprepare(dev-clk);
clk_put(dev-clk);
dev-clk = NULL;
@@ -230,6 +241,7 @@ static int dw_i2c_suspend(struct device *dev)
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
clk_disable(i_dev-clk);
+ clk_unprepare(i_dev-clk);
return 0;
}
@@ -239,6 +251,7 @@ static int dw_i2c_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
+ clk_prepare(i_dev-clk);
clk_enable(i_dev-clk);
i2c_dw_init(i_dev);
--
1.7.9
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2] i2c: designware: Add clk_{un}prepare() support
clk_{un}prepare is mandatory for platforms using common clock framework. Since
this driver is used by SPEAr platform, which supports common clock framework,
add clk_{un}prepare() support for designware i2c.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
V1-V2:
- Use clk_prepare_enable and clk_disable_unprepare
drivers/i2c/busses/i2c-designware-platdrv.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 76bf108..3a7a4e8 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -103,7 +103,7 @@ static int __devinit dw_i2c_probe(struct platform_device
*pdev)
r = -ENODEV;
goto err_free_mem;
}
- clk_enable(dev-clk);
+ clk_prepare_enable(dev-clk);
dev-functionality =
I2C_FUNC_I2C |
@@ -180,7 +180,7 @@ err_free_irq:
err_iounmap:
iounmap(dev-base);
err_unuse_clocks:
- clk_disable(dev-clk);
+ clk_disable_unprepare(dev-clk);
clk_put(dev-clk);
dev-clk = NULL;
err_free_mem:
@@ -202,7 +202,7 @@ static int __devexit dw_i2c_remove(struct platform_device
*pdev)
i2c_del_adapter(dev-adapter);
put_device(pdev-dev);
- clk_disable(dev-clk);
+ clk_disable_unprepare(dev-clk);
clk_put(dev-clk);
dev-clk = NULL;
@@ -229,7 +229,7 @@ static int dw_i2c_suspend(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
- clk_disable(i_dev-clk);
+ clk_disable_unprepare(i_dev-clk);
return 0;
}
@@ -239,7 +239,7 @@ static int dw_i2c_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
- clk_enable(i_dev-clk);
+ clk_prepare_enable(i_dev-clk);
i2c_dw_init(i_dev);
return 0;
--
1.7.9
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c/busses: Add PM support
On 2/24/2012 5:01 PM, Viresh KUMAR wrote: From: Deepak Sikri deepak.si...@st.com This patch adds in support for standby/S2R/hybernate for i2c-designware driver. Signed-off-by: Deepak Sikri deepak.si...@st.com Signed-off-by: Rajeev Kumar rajeev-dlh.ku...@st.com --- drivers/i2c/busses/i2c-designware-platdrv.c | 27 +++ 1 files changed, 27 insertions(+), 0 deletions(-) Hi Wolfram, Any inputs of this patch? You can discard 2/2 of this patchset, as that patch is included in a separate patchset I2C: Add bus recovery infrastructure. -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 0/2] I2C: Add bus recovery infrastructure
On 3/2/2012 11:53 AM, Viresh KUMAR wrote: Hello, 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 Hi, Did somebody applied these patches? Or any more comments? -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V3 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.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
drivers/i2c/i2c-core.c | 125
include/linux/i2c.h| 52
2 files changed, 177 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e9c1893..5cd5f83 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -26,7 +26,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
@@ -103,6 +105,88 @@ 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 */
+static int i2c_gpio_recover_bus(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ unsigned long delay = 100;
+ int i, ret, val = 1;
+
+ if (bri-get_gpio)
+ bri-get_gpio(bri-scl_gpio);
+
+ ret = gpio_request_one(bri-scl_gpio, bri-scl_gpio_flags, i2c-scl);
+ if (ret 0) {
+ dev_warn(adap-dev, gpio request fail: %d\n, bri-scl_gpio);
+ return ret;
+ }
+
+ if (!bri-skip_sda_polling) {
+ if (bri-get_gpio)
+ bri-get_gpio(bri-sda_gpio);
+
+ ret = gpio_request_one(bri-sda_gpio, bri-sda_gpio_flags,
+ i2c-sda);
+ if (ret 0) {
+ /* work without sda polling */
+ dev_warn(adap-dev,
+ sda_gpio request fail: %d. Skip sda polling\n,
+ bri-scl_gpio);
+ bri-skip_sda_polling = true;
+ if (bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+ }
+ }
+
+ delay /= bri-clock_rate_khz * 2;
+
+ for (i = 0; i bri-clock_cnt * 2; i++, val = !val) {
+ ndelay(delay);
+ gpio_set_value(bri-scl_gpio, val);
+
+ /* break if sda got high, check only when scl line is high */
+ if (!bri-skip_sda_polling val)
+ if (gpio_get_value(bri-sda_gpio))
+ break;
+ }
+
+ gpio_free(bri-scl_gpio);
+
+ if (bri-put_gpio)
+ bri-put_gpio(bri-scl_gpio);
+
+ if (!bri-skip_sda_polling) {
+ gpio_free(bri-sda_gpio);
+
+ if (bri-put_gpio)
+ bri-put_gpio(bri-sda_gpio);
+ }
+
+ return 0;
+}
+
+static int i2c_scl_recover_bus(struct i2c_adapter *adap)
+{
+ struct i2c_bus_recovery_info *bri = adap-bus_recovery_info;
+ int i, val = 0;
+ unsigned long delay = 100;
+
+ delay /= bri-clock_rate_khz * 2;
+
+ for (i = 0; i bri-clock_cnt * 2; i++,
+ val = !val) {
+ bri-set_scl(adap, val);
+ ndelay(delay);
+
+ /* break if sda got high, check only when scl line is high */
+ if (!bri-skip_sda_polling val)
+ if (bri-get_sda(adap))
+ break;
+ }
+
+ return 0;
+}
+
static int i2c_device_probe(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
@@ -861,6 +945,47 @@ 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_info(adap-dev,
+ registered for non-generic bus recovery\n);
+ } else {
+ /* Use generic recovery routines */
+ if (!bri-clock_rate_khz) {
+ dev_warn(adap-dev,
+ doesn't have valid recovery clock
rate\n);
+ goto exit_recovery;
+ }
+
+ /* Most
Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function
On 2/29/2012 5:22 PM, Laxman Dewangan wrote:
On Tuesday 28 February 2012 06:53 PM, viresh kumar wrote:
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
+static int i2c_gpio_recover_bus(struct i2c_adapter *adap)
+{
+int tmp, val = 1;
+unsigned long delay = 100;
+
+tmp = gpio_request_one(adap-scl_gpio, GPIOF_DIR_OUT |
+GPIOF_INIT_LOW, i2c-bus-recover);
Should rename tmp to ret or status. tmp does not looks appropriate.
Ok.
+if (tmp 0) {
+dev_warn(adap-dev, gpio request one fail: %d\n,
+adap-scl_gpio);
+return tmp;
+}
+
+delay /= adap-clock_rate * 2;
Here delay is turning as micor sec and function used as the nano sec.
clock_rate is in KHz, mentioned in comment of clock_rate.
Makes sense now or am i missing something?
+
+for (tmp = 0; tmp adap-clock_cnt * 2; tmp++, val = !val) {
+ndelay(delay);
should be udelay()?
Please add blank lines before and after your comment. That makes
it more readable.
+gpio_set_value(adap-scl_gpio, val);
I think it should check for the sda line for coming out of the loop.
There may be possibility that we may not need 9 clock pulses.
I asked this in another mail, how to be sure that it will work.
+u8 scl_gpio;
gpio can be more than 256. better to use int.
Take scl_gpio_flag and use in the gpio_request_one.
Ok.
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function
On 2/29/12, Laxman Dewangan ldewan...@nvidia.com wrote:
Here is V2:
From: Viresh Kumar viresh.ku...@st.com
Date: Tue, 28 Feb 2012 18:26:31 +0530
Subject: [PATCH] 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.
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 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.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
drivers/i2c/i2c-core.c | 85
include/linux/i2c.h| 32 ++
2 files changed, 117 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e9c1893..1d2c8a0 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -26,7 +26,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
@@ -103,6 +105,54 @@ 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 */
+static int i2c_gpio_recover_bus(struct i2c_adapter *adap)
+{
+ int i, ret, val = 1, gpio = adap-bus_recovery_info-gpio;
+ unsigned long delay = 100;
+
+ if (adap-bus_recovery_info-get_gpio)
+ adap-bus_recovery_info-get_gpio(gpio);
+
+ ret = gpio_request_one(gpio, GPIOF_DIR_OUT |
+ GPIOF_INIT_LOW, i2c-bus-recover);
+ if (ret 0) {
+ dev_warn(adap-dev, gpio request one fail: %d\n, gpio);
+ return ret;
+ }
+
+ delay /= adap-bus_recovery_info-clock_rate_khz * 2;
+
+ for (i = 0; i adap-bus_recovery_info-clock_cnt * 2;
+ i++, val = !val) {
+ ndelay(delay);
+ gpio_set_value(gpio, val);
+ }
+
+ gpio_free(adap-bus_recovery_info-clock_cnt);
+
+ if (adap-bus_recovery_info-put_gpio)
+ adap-bus_recovery_info-put_gpio(gpio);
+
+ return 0;
+}
+
+static int i2c_scl_recover_bus(struct i2c_adapter *adap)
+{
+ int i, val = 0;
+ unsigned long delay = 100;
+
+ delay /= adap-bus_recovery_info-clock_rate_khz * 2;
+
+ for (i = 0; i adap-bus_recovery_info-clock_cnt * 2; i++,
+ val = !val) {
+ adap-bus_recovery_info-set_scl(adap, val);
+ ndelay(delay);
+ }
+
+ return 0;
+}
+
static int i2c_device_probe(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
@@ -861,6 +911,41 @@ 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) {
+ if (adap-bus_recovery_info-recover_bus) {
+ dev_info(adap-dev,
+ registered for non-generic bus recovery\n);
+ } else {
+ /* Use generic recovery routines */
+ if (!adap-bus_recovery_info-clock_rate_khz) {
+ dev_warn(adap-dev,
+ doesn't have valid recovery clock
rate\n);
+ goto exit_recovery;
+ }
+
+ /* Most controller need 9 clocks at max */
+ if (!adap-bus_recovery_info-clock_cnt)
+ adap-bus_recovery_info-clock_cnt = 9;
+
+ if (adap-bus_recovery_info-is_gpio_recovery) {
+ adap-bus_recovery_info-recover_bus =
+ i2c_gpio_recover_bus;
+ dev_info(adap-dev,
+ registered for gpio bus recovery\n);
+ } else if (adap-bus_recovery_info-set_scl) {
+ adap-bus_recovery_info-recover_bus =
+ i2c_scl_recover_bus;
+ dev_info(adap-dev,
+ registered for scl bus recovery\n);
+ } else {
+ dev_warn(adap-dev,
+ doesn't have valid recovery
routine\n);
+ }
+ }
+ }
+
+exit_recovery:
/* create pre
Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function
On 3/1/2012 11:40 AM, Laxman Dewangan wrote: Most of the socs have i2c pin as open drain type. By having this flag we can tell that in is whether open drain type or not. There is patch to support open drain in gpiolib which is under review. Ok. Got it now. I will add gpio_flags in struct i2c_bus_recovery_info. If it is non-zero, will use it, otherwise use default flags (GPIOF_DIR_OUT | GPIOF_INIT_LOW). -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function
On 2/27/12, Viresh Kumar viresh.ku...@st.com wrote:
we can give generalized function inside i2c framework for this, so that
multiple
drivers can use it.
Secondly there are drivers/devices where control of pins is available. For
them
we can provide driver hooks.
I would try to provide a patch for this ASAP. Other suggestions are welcome.
Here we go, please provide your feedbacks.:
From: Viresh Kumar viresh.ku...@st.com
Date: Tue, 28 Feb 2012 18:26:31 +0530
Subject: [PATCH] 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.
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 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.
Signed-off-by: Viresh Kumar viresh.ku...@st.com
---
drivers/i2c/i2c-core.c | 56
include/linux/i2c.h| 22 ++
2 files changed, 78 insertions(+), 0 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e9c1893..c9f0daf 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -26,7 +26,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
@@ -103,6 +105,47 @@ 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 */
+static int i2c_gpio_recover_bus(struct i2c_adapter *adap)
+{
+ int tmp, val = 1;
+ unsigned long delay = 100;
+
+ tmp = gpio_request_one(adap-scl_gpio, GPIOF_DIR_OUT |
+ GPIOF_INIT_LOW, i2c-bus-recover);
+ if (tmp 0) {
+ dev_warn(adap-dev, gpio request one fail: %d\n,
+ adap-scl_gpio);
+ return tmp;
+ }
+
+ delay /= adap-clock_rate * 2;
+
+ for (tmp = 0; tmp adap-clock_cnt * 2; tmp++, val = !val) {
+ ndelay(delay);
+ gpio_set_value(adap-scl_gpio, val);
+ }
+
+ gpio_free(adap-clock_cnt);
+
+ return 0;
+}
+
+static int i2c_scl_recover_bus(struct i2c_adapter *adap)
+{
+ int i, val = 0;
+ unsigned long delay = 100;
+
+ delay /= adap-clock_rate * 2;
+
+ for (i = 0; i adap-clock_cnt * 2; i++, val = !val) {
+ adap-set_scl(adap, val);
+ ndelay(delay);
+ }
+
+ return 0;
+}
+
static int i2c_device_probe(struct device *dev)
{
struct i2c_client *client = i2c_verify_client(dev);
@@ -861,6 +904,19 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
Failed to create compatibility class link\n);
#endif
+ /* bus recovery specific initialization */
+ if (!adap-recover_bus) {
+ if (!adap-clock_cnt || !adap-clock_rate)
+ goto warn_no_recovery;
+ else if (adap-is_gpio_recovery)
+ adap-recover_bus = i2c_gpio_recover_bus;
+ else if (adap-set_scl)
+ adap-recover_bus = i2c_scl_recover_bus;
+
+warn_no_recovery:
+ dev_warn(adap-dev, doesn't have recovery method\n);
+ }
+
/* 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 8e25a91..b2a6d97 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -388,6 +388,28 @@ struct i2c_adapter {
struct mutex userspace_clients_lock;
struct list_head userspace_clients;
+
+ /*
+* bus recovery specific fields: Either pass driver's recover_bus()
+* routine, or pass it NULL to use generic ones. There are two type of
+* generic one's available:
+* - controlling scl line as gpio, pass is_gpio_recovery as true
+* and valid scl_gpio number
+* - controlling scl line directly via controller, pass
+* is_gpio_recovery as false and valid set_scl routine's pointer
+*
+* Both schemes require valid values of
+* - clock_cnt: total number of dummy clocks to be generated
+* - clock_rate: rate of dummy clock
+*
+* scl_gpio.
+*/
+ int (*recover_bus)(struct i2c_adapter *);
+ void (*set_scl)(struct i2c_adapter *, int val);
+ bool is_gpio_recovery;
+ u8 scl_gpio
Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function
On 2/28/2012 7:25 PM, Salvatore DE DOMINICIS wrote:
What happens if the bus is still stuck?
Do we need to check also for a change in SDA line?
I mean, if some device is not behaving correctly and does not change the SDA
(as mandated by standard) then we don't solve the issue.
I also wanted to ask this question over list, so that experienced people
can suggest what should we do here.
Following is mentioned in: UM10204: I2C-bus specification and user manual
http://www.nxp.com/documents/user_manual/UM10204.pdf
3.1.16 Bus clear
In the unlikely event where the clock (SCL) is stuck LOW, the preferential
procedure is to
reset the bus using the HW reset signal if your I2C devices have HW reset
inputs. If the
I2C devices do not have HW reset inputs, cycle power to the devices to activate
the
mandatory internal Power-On Reset (POR) circuit.
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.
It says that the hang situation is SDA is stuck LOW and 9 clock pulses should
be enough to get it out of hang (Can somebody tell me how this figure of 9
derived?)
SDA will become High, but what guarantees that this will not be low immediately
after that, while we are reading SDA line? Or Is reading SDA line after 9 pulses
sufficient?
static int i2c_device_probe(struct device *dev)
{
+ /* bus recovery specific initialization */
+ if (!adap-recover_bus) {
+ if (!adap-clock_cnt || !adap-clock_rate)
+ goto warn_no_recovery;
I will also change this code to something like:
if (!adap-recover_bus) {
if (!adap-clock_cnt)
adap-clock_cnt = 9;
if (!adap-clock_rate)
goto warn_no_recovery;
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function
On 2/27/2012 3:40 PM, Rajeev KUMAR wrote:
In some i2c controller (like synopsys) you dont have control over i2c
data and clock lines. So clock toggling, you need to use gpio lines
which in turns maps on sda and scl line.
For the controller in which you have control over sda and scl line there
is not need for gpio lines. You can directly write on registers.
So to make the function more generic its better to control i2c lines
with gpio.
There would be many drivers where we need to convert i2c pins to gpio pins
and generate clock pulses:
gpio_request(gpio);
gpio_set_direction(gpio, out, 0);
for (i = 1; i count; i++) {
gpio set val(gpio, 1);
gpio set val(gpio, 0);
}
gpio_free(gpio);
we can give generalized function inside i2c framework for this, so that multiple
drivers can use it.
Secondly there are drivers/devices where control of pins is available. For them
we can provide driver hooks.
I would try to provide a patch for this ASAP. Other suggestions are welcome.
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c/designware: dw_i2c_init_driver as subsys initcall
On 11/16/2011 10:28 AM, Pratyush ANAND wrote:
There are few drivers(for example stmpe-gpio) which are available on i2c
bus but has been initialized as subsys initcall. Therefore i2c driver
must also be initialized as subsys initcall.
Signed-off-by: Pratyush Anand pratyush.an...@st.com
---
drivers/i2c/busses/i2c-designware-pcidrv.c |2 +-
drivers/i2c/busses/i2c-designware-platdrv.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c
b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 9e89e73..7854565 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -379,7 +379,7 @@ static int __init dw_i2c_init_driver(void)
{
return pci_register_driver(dw_i2c_driver);
}
-module_init(dw_i2c_init_driver);
+subsys_initcall(dw_i2c_init_driver);
static void __exit dw_i2c_exit_driver(void)
{
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
b/drivers/i2c/busses/i2c-designware-platdrv.c
index 2d3657a..4fbdcd5 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -202,7 +202,7 @@ static int __init dw_i2c_init_driver(void)
{
return platform_driver_probe(dw_i2c_driver, dw_i2c_probe);
}
-module_init(dw_i2c_init_driver);
+subsys_initcall(dw_i2c_init_driver);
static void __exit dw_i2c_exit_driver(void)
{
Hi Jean/Ben/Wolfram,
Can anyone of you apply this patch, if it looks fine.
--
viresh
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c/designware: dw_i2c_init_driver as subsys initcall
On 11/16/2011 1:52 PM, Baruch Siach wrote: Dependencies should be stated explicitly. Since this subsys_initcall thing is quite common among the i2c masters I'm willing to ack this one for now. But the real solution is to make the dependencies between devices clear and explicit. Baruch, Did you apply this patch? -- viresh -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
