[PATCH 1/2] i2c/busses: Add PM support

[PATCH 1/2] i2c/busses: Add PM support

[Viresh Kumar]

From: Deepak Sikri <deepak.sikri-qxv4g6HH51o@public.gmane.org>

This patch adds in support for standby/S2R/hybernate for i2c-designware driver.

Signed-off-by: Deepak Sikri <deepak.sikri-qxv4g6HH51o@public.gmane.org>
Signed-off-by: Rajeev Kumar <rajeev-dlh.kumar-qxv4g6HH51o@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-platdrv.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 5244c47..c5ac2dc 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -36,6 +36,7 @@
 #include <linux/interrupt.h>
 #include <linux/of_i2c.h>
 #include <linux/platform_device.h>
+#include <linux/pm.h>
 #include <linux/io.h>
 #include <linux/slab.h>
 #include "i2c-designware-core.h"
@@ -198,6 +199,31 @@ static const struct of_device_id dw_i2c_of_match[] = {
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
 #endif
 
+#ifdef CONFIG_PM
+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);
+
+	return 0;
+}
+
+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);
+	i2c_dw_init(i_dev);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(dw_i2c_dev_pm_ops, dw_i2c_suspend, dw_i2c_resume);
+
 /* work with hotplug and coldplug */
 MODULE_ALIAS("platform:i2c_designware");
 
@@ -207,6 +233,7 @@ static struct platform_driver dw_i2c_driver = {
 		.name	= "i2c_designware",
 		.owner	= THIS_MODULE,
 		.of_match_table = of_match_ptr(dw_i2c_of_match),
+		.pm	= &dw_i2c_dev_pm_ops,
 	},
 };
 
-- 
1.7.8.110.g4cb5d

[PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Viresh Kumar]

From: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>

Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C adapter
driver that performs i2c bus recovery after timeout. The scope of this routine
is to define i2c bus recovery procedure as specified in the i2c protocol Rev. 03
section 3.16 titled "Bus clear".

Since the Designware I2C controller doesn't provide direct control over SDA and
SCL lines hence the intent is to let platform try to recover the bus if they
have the capability to take control of I2C pads and follow the recovery
protocol.

Signed-off-by: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
Signed-off-by: Shiraz Hashim <shiraz.hashim-qxv4g6HH51o@public.gmane.org>
Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware-core.c    |    4 +++
 drivers/i2c/busses/i2c-designware-core.h    |    3 +-
 drivers/i2c/busses/i2c-designware-platdrv.c |    8 ++++++
 include/linux/i2c/i2c-designware.h          |   33 +++++++++++++++++++++++++++
 4 files changed, 47 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 df87992..91d9357 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -525,6 +525,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");
+		if (dev->i2c_recover_bus) {
+			dev_info(dev->dev, "try i2c bus recovery\n");
+			dev->i2c_recover_bus(dev->recovery_data);
+		}
 		i2c_dw_init(dev);
 		ret = -ETIMEDOUT;
 		goto done;
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 02d1a2d..e2f3119 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -34,7 +34,6 @@
 #define DW_IC_CON_RESTART_EN		0x20
 #define DW_IC_CON_SLAVE_DISABLE		0x40
 
-
 /**
  * struct dw_i2c_dev - private i2c-designware data
  * @dev: driver model device node
@@ -88,6 +87,8 @@ struct dw_i2c_dev {
 	u32			master_cfg;
 	unsigned int		tx_fifo_depth;
 	unsigned int		rx_fifo_depth;
+	void			(*i2c_recover_bus)(void *);
+	void			*recovery_data;
 };
 
 extern u32 dw_readl(struct dw_i2c_dev *dev, int offset);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index c5ac2dc..2237398 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>
@@ -55,6 +56,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 */
@@ -98,6 +100,12 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 	}
 	clk_enable(dev->clk);
 
+	pdata = dev_get_platdata(&pdev->dev);
+	if (pdata && pdata->i2c_recover_bus) {
+		dev->i2c_recover_bus = pdata->i2c_recover_bus;
+		dev->recovery_data = &pdev;
+	}
+
 	dev->functionality =
 		I2C_FUNC_I2C |
 		I2C_FUNC_10BIT_ADDR |
diff --git a/include/linux/i2c/i2c-designware.h b/include/linux/i2c/i2c-designware.h
new file mode 100644
index 0000000..e40ad85
--- /dev/null
+++ b/include/linux/i2c/i2c-designware.h
@@ -0,0 +1,33 @@
+/*
+ * Synopsys DesignWare I2C adapter driver's platform data
+ *
+ * Copyright (C) 2012 ST Microelectronics.
+ * Author: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
+ *
+ * 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>
+
+/* I2C Designware Platform Data */
+struct i2c_dw_pdata {
+	/*
+	 * The scope of this routine is to define i2c bus recovery procedure
+	 * as specified in the i2c protocol Rev. 03 section 3.16 titled
+	 * "Bus clear".
+	 * Its implementation is platform dependant.
+	 */
+	void (*i2c_recover_bus)(void *);
+};
+
+#endif /* I2C_DESIGNWARE_H */
-- 
1.7.8.110.g4cb5d

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Shubhrajyoti Datta]

On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org> wrote:
> From: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>
> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C adapter
> driver that performs i2c bus recovery after timeout. The scope of this routine
> is to define i2c bus recovery procedure as specified in the i2c protocol Rev. 03
> section 3.16 titled "Bus clear".
What do you do in the function ?

Could we have it in the driver file itself?

>
> Since the Designware I2C controller doesn't provide direct control over SDA and
> SCL lines hence the intent is to let platform try to recover the bus if they
> have the capability to take control of I2C pads and follow the recovery
> protocol.

So the function is passed in the pdata.
>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Shiraz Hashim <shiraz.hashim-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-designware-core.c    |    4 +++
>  drivers/i2c/busses/i2c-designware-core.h    |    3 +-
>  drivers/i2c/busses/i2c-designware-platdrv.c |    8 ++++++
>  include/linux/i2c/i2c-designware.h          |   33 +++++++++++++++++++++++++++
>  4 files changed, 47 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 df87992..91d9357 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -525,6 +525,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");
> +               if (dev->i2c_recover_bus) {
> +                       dev_info(dev->dev, "try i2c bus recovery\n");
> +                       dev->i2c_recover_bus(dev->recovery_data);
> +               }
>                i2c_dw_init(dev);
>                ret = -ETIMEDOUT;
>                goto done;
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index 02d1a2d..e2f3119 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -34,7 +34,6 @@
>  #define DW_IC_CON_RESTART_EN           0x20
>  #define DW_IC_CON_SLAVE_DISABLE                0x40
>
> -
>  /**
>  * struct dw_i2c_dev - private i2c-designware data
>  * @dev: driver model device node
> @@ -88,6 +87,8 @@ struct dw_i2c_dev {
>        u32                     master_cfg;
>        unsigned int            tx_fifo_depth;
>        unsigned int            rx_fifo_depth;
> +       void                    (*i2c_recover_bus)(void *);
> +       void                    *recovery_data;
>  };
>
>  extern u32 dw_readl(struct dw_i2c_dev *dev, int offset);
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index c5ac2dc..2237398 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>
> @@ -55,6 +56,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 */
> @@ -98,6 +100,12 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
>        }
>        clk_enable(dev->clk);
>
> +       pdata = dev_get_platdata(&pdev->dev);
> +       if (pdata && pdata->i2c_recover_bus) {
> +               dev->i2c_recover_bus = pdata->i2c_recover_bus;
> +               dev->recovery_data = &pdev;
> +       }
> +
>        dev->functionality =
>                I2C_FUNC_I2C |
>                I2C_FUNC_10BIT_ADDR |
> diff --git a/include/linux/i2c/i2c-designware.h b/include/linux/i2c/i2c-designware.h
> new file mode 100644
> index 0000000..e40ad85
> --- /dev/null
> +++ b/include/linux/i2c/i2c-designware.h
> @@ -0,0 +1,33 @@
> +/*
> + * Synopsys DesignWare I2C adapter driver's platform data
> + *
> + * Copyright (C) 2012 ST Microelectronics.
> + * Author: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
> + *
> + * 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>
> +
> +/* I2C Designware Platform Data */
> +struct i2c_dw_pdata {
> +       /*
> +        * The scope of this routine is to define i2c bus recovery procedure
> +        * as specified in the i2c protocol Rev. 03 section 3.16 titled
> +        * "Bus clear".
> +        * Its implementation is platform dependant.
> +        */
> +       void (*i2c_recover_bus)(void *);
> +};
> +
> +#endif /* I2C_DESIGNWARE_H */
> --
> 1.7.8.110.g4cb5d
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Laxman Dewangan]

On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
> On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>  wrote:
>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>
>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C adapter
>> driver that performs i2c bus recovery after timeout. The scope of this routine
>> is to define i2c bus recovery procedure as specified in the i2c protocol Rev. 03
>> section 3.16 titled "Bus clear".
> What do you do in the function ?
>
> Could we have it in the driver file itself?
>
I think bus recovery mechanism is to send  extra clock on SCL line by 
toggling the pin (using gpio apis) and keep watching of sda line whether 
it becomes high or not.
We can put this algorithms in the some common file (i2c/algos/) and so 
if any i2c bus driver want to use, they can use it.
Little background: I am working on tegra i2c controller and we have 
similar logic in tegra-i2c driver which we want to upstream.
If similar function is in common place, we can use this and need not to 
duplicating it on every bus driver.


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Rajeev kumar]

On 2/27/2012 12:57 PM, Laxman Dewangan wrote:
> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>   wrote:
>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>
>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C adapter
>>> driver that performs i2c bus recovery after timeout. The scope of this routine
>>> is to define i2c bus recovery procedure as specified in the i2c protocol Rev. 03
>>> section 3.16 titled "Bus clear".
>> What do you do in the function ?
>>
>> Could we have it in the driver file itself?
>>
> I think bus recovery mechanism is to send  extra clock on SCL line by
> toggling the pin (using gpio apis) and keep watching of sda line whether
> it becomes high or not.
> We can put this algorithms in the some common file (i2c/algos/) and so
> if any i2c bus driver want to use, they can use it.
> Little background: I am working on tegra i2c controller and we have
> similar logic in tegra-i2c driver which we want to upstream.
> If similar function is in common place, we can use this and need not to
> duplicating it on every bus driver.
>

I agreed with Laxman, as this is the protocol issue and so it should not 
be in each individual driver. It should be the part of framework 
itself. We need to implement something like 
drivers/i2c/busses/i2c-gpio.c.  We need to investigate more on it.


Best Regards
Rajeev

>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> .
>

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Laxman Dewangan]

On Monday 27 February 2012 01:40 PM, Rajeev kumar wrote:
> On 2/27/2012 12:57 PM, Laxman Dewangan wrote:
>> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>    wrote:
>>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>>
>>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C adapter
>>>> driver that performs i2c bus recovery after timeout. The scope of this routine
>>>> is to define i2c bus recovery procedure as specified in the i2c protocol Rev. 03
>>>> section 3.16 titled "Bus clear".
>>> What do you do in the function ?
>>>
>>> Could we have it in the driver file itself?
>>>
>> I think bus recovery mechanism is to send  extra clock on SCL line by
>> toggling the pin (using gpio apis) and keep watching of sda line whether
>> it becomes high or not.
>> We can put this algorithms in the some common file (i2c/algos/) and so
>> if any i2c bus driver want to use, they can use it.
>> Little background: I am working on tegra i2c controller and we have
>> similar logic in tegra-i2c driver which we want to upstream.
>> If similar function is in common place, we can use this and need not to
>> duplicating it on every bus driver.
>>
> I agreed with Laxman, as this is the protocol issue and so it should not
> be in each individual driver. It should be the part of framework
> itself. We need to implement something like
> drivers/i2c/busses/i2c-gpio.c.  We need to investigate more on it.
>
I am thinking to put this in drivers/i2c/busses/bus_recovery_gpio.c. 
This will provide only one function and config variable for this code 
can be selected from different driver by select option. it will not be 
enabled as independent driver. Similar to regmap_i2c or regmap_spi which 
is selected through select in kconfig

The function should take the gpio number and gpio flags for scl and sda, 
bus freq and number of clock to be sent.
All gpio related apis should be called on .c file which is implemented.

How bus driver gets the corresponding gpio number is specific to bus 
driver but they need to pass all the information through arguments.



-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Rajeev kumar]

On 2/27/2012 1:52 PM, Laxman Dewangan wrote:
> On Monday 27 February 2012 01:40 PM, Rajeev kumar wrote:
>> On 2/27/2012 12:57 PM, Laxman Dewangan wrote:
>>> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>>>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>     wrote:
>>>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>>>
>>>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C adapter
>>>>> driver that performs i2c bus recovery after timeout. The scope of this routine
>>>>> is to define i2c bus recovery procedure as specified in the i2c protocol Rev. 03
>>>>> section 3.16 titled "Bus clear".
>>>> What do you do in the function ?
>>>>
>>>> Could we have it in the driver file itself?
>>>>
>>> I think bus recovery mechanism is to send  extra clock on SCL line by
>>> toggling the pin (using gpio apis) and keep watching of sda line whether
>>> it becomes high or not.
>>> We can put this algorithms in the some common file (i2c/algos/) and so
>>> if any i2c bus driver want to use, they can use it.
>>> Little background: I am working on tegra i2c controller and we have
>>> similar logic in tegra-i2c driver which we want to upstream.
>>> If similar function is in common place, we can use this and need not to
>>> duplicating it on every bus driver.
>>>
>> I agreed with Laxman, as this is the protocol issue and so it should not
>> be in each individual driver. It should be the part of framework
>> itself. We need to implement something like
>> drivers/i2c/busses/i2c-gpio.c.  We need to investigate more on it.
>>
> I am thinking to put this in drivers/i2c/busses/bus_recovery_gpio.c.
> This will provide only one function and config variable for this code
> can be selected from different driver by select option. it will not be
> enabled as independent driver. Similar to regmap_i2c or regmap_spi which

What is this regmap_i2c or regmap_spi, I am not able to locate it.

> is selected through select in kconfig
>
> The function should take the gpio number and gpio flags for scl and sda,
> bus freq and number of clock to be sent.

Need to pass "number of clock to be sent" or not . I am not very much 
sure. I think number of clock to be sent is fixed.

~Rajeev

> All gpio related apis should be called on .c file which is implemented.
>
> How bus driver gets the corresponding gpio number is specific to bus
> driver but they need to pass all the information through arguments.
>
>
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> .
>

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Laxman Dewangan]

On Monday 27 February 2012 02:11 PM, Rajeev kumar wrote:
> On 2/27/2012 1:52 PM, Laxman Dewangan wrote:
>
>> is selected through select in kconfig
>>
>> The function should take the gpio number and gpio flags for scl and sda,
>> bus freq and number of clock to be sent.
> Need to pass "number of clock to be sent" or not . I am not very much
> sure. I think number of clock to be sent is fixed.
>
Yes, it is said 9 clock pulse. But we have observed that sometimes it 
requires more clock and that's why I wanted to give option to make more 
generic.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Shubhrajyoti Datta]

On Mon, Feb 27, 2012 at 12:57 PM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>>
>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>  wrote:
>>>
>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>
>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C
>>> adapter
>>> driver that performs i2c bus recovery after timeout. The scope of this
>>> routine
>>> is to define i2c bus recovery procedure as specified in the i2c protocol
>>> Rev. 03
>>> section 3.16 titled "Bus clear".
>>
>> What do you do in the function ?
>>
>> Could we have it in the driver file itself?
>>
> I think bus recovery mechanism is to send  extra clock on SCL line by
> toggling the pin (using gpio apis)
You mean the SCL I didnt understand the gpio part?

Why is gpio needed?

 and keep watching of sda line whether it
> becomes high or not.
> We can put this algorithms in the some common file (i2c/algos/) and so if
> any i2c bus driver want to use, they can use it.
> Little background: I am working on tegra i2c controller and we have similar
> logic in tegra-i2c driver which we want to upstream.
> If similar function is in common place, we can use this and need not to
> duplicating it on every bus driver.
>
>

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Laxman Dewangan]

On Monday 27 February 2012 02:42 PM, Shubhrajyoti Datta wrote:
> On Mon, Feb 27, 2012 at 12:57 PM, Laxman Dewangan<ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>  wrote:
>> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>    wrote:
>>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>>
>>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C
>>>> adapter
>>>> driver that performs i2c bus recovery after timeout. The scope of this
>>>> routine
>>>> is to define i2c bus recovery procedure as specified in the i2c protocol
>>>> Rev. 03
>>>> section 3.16 titled "Bus clear".
>>> What do you do in the function ?
>>>
>>> Could we have it in the driver file itself?
>>>
>> I think bus recovery mechanism is to send  extra clock on SCL line by
>> toggling the pin (using gpio apis)
> You mean the SCL I didnt understand the gpio part?
>
> Why is gpio needed?

How do you toggle the SCL pin?

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Rajeev kumar]

Hello Shubhrajyoti

On 2/27/2012 2:42 PM, Shubhrajyoti Datta wrote:
> On Mon, Feb 27, 2012 at 12:57 PM, Laxman Dewangan<ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>  wrote:
>> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>>>
>>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>    wrote:
>>>>
>>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>>
>>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C
>>>> adapter
>>>> driver that performs i2c bus recovery after timeout. The scope of this
>>>> routine
>>>> is to define i2c bus recovery procedure as specified in the i2c protocol
>>>> Rev. 03
>>>> section 3.16 titled "Bus clear".
>>>
>>> What do you do in the function ?
>>>
>>> Could we have it in the driver file itself?
>>>
>> I think bus recovery mechanism is to send  extra clock on SCL line by
>> toggling the pin (using gpio apis)
> You mean the SCL I didnt understand the gpio part?
>
> Why is gpio needed?
>

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.

~Rajeev

>   and keep watching of sda line whether it
>> becomes high or not.
>> We can put this algorithms in the some common file (i2c/algos/) and so if
>> any i2c bus driver want to use, they can use it.
>> Little background: I am working on tegra i2c controller and we have similar
>> logic in tegra-i2c driver which we want to upstream.
>> If similar function is in common place, we can use this and need not to
>> duplicating it on every bus driver.
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
>

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Viresh Kumar]

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

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[viresh kumar]

On 2/27/12, Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org> 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.kumar-qxv4g6HH51o@public.gmane.org>
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.kumar-qxv4g6HH51o@public.gmane.org>
---
 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 = 1000000;
+
+	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 = 1000000;
+
+	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;
+	u8 clock_cnt;
+	u32 clock_rate;	/* In KHz */
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)


--
viresh

RE: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Salvatore DE DOMINICIS]

Hi Viresh,

>From: viresh kumar [viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>Sent: Tuesday, February 28, 2012 14:23
>To: Viresh KUMAR
>Cc: Rajeev KUMAR; Shubhrajyoti Datta; Laxman Dewangan; khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org; ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Armando VISCONTI; Shiraz HASHIM; Vipin KUMAR; Deepak SIKRI; Vipul Kumar SAMAR; Amit VIRDI; Pratyush ANAND; Bhupesh SHARMA; Bhavna YADAV; Vincenzo FRASCINO; Mirko GARDI; Salvatore DE DOMINICIS; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>Subject: Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function
>
>On 2/27/12, Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org> 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.kumar-qxv4g6HH51o@public.gmane.org>
>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.kumar-qxv4g6HH51o@public.gmane.org>
>---
> 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 = 1000000;
>+

Why delay is fixed to this value? This seems to me that this value is dependant on i2c clock speed,
e.g. 100kHz, 400kHz, have different periods and thus need different delay times,
am I correct?

>+       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;
>+}
>+

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.

BR,
Salvatore

>+static int i2c_scl_recover_bus(struct i2c_adapter *adap)
>+{
>+       int i, val = 0;
>+       unsigned long delay = 1000000;
>+
>+       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;
>+       u8 clock_cnt;
>+       u32 clock_rate; /* In KHz */
> };
> #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>
>
>--
>viresh--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Vincenzo Frascino]

Hi Salvatore,

Il 28/02/2012 14:55, Salvatore DE DOMINICIS ha scritto:
> Hi Viresh,
>
>> From: viresh kumar [viresh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org]
>> Sent: Tuesday, February 28, 2012 14:23
>> To: Viresh KUMAR
>> Cc: Rajeev KUMAR; Shubhrajyoti Datta; Laxman Dewangan; khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org; ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Armando VISCONTI; Shiraz HASHIM; Vipin KUMAR; Deepak SIKRI; Vipul Kumar SAMAR; Amit VIRDI; Pratyush ANAND; Bhupesh SHARMA; Bhavna YADAV; Vincenzo FRASCINO; Mirko GARDI; Salvatore DE DOMINICIS; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Subject: Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function
>>
>> On 2/27/12, Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org> 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.kumar-qxv4g6HH51o@public.gmane.org>
>> 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.kumar-qxv4g6HH51o@public.gmane.org>
>> ---
>> 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 = 1000000;
>> +
> Why delay is fixed to this value? This seems to me that this value is dependant on i2c clock speed,
> e.g. 100kHz, 400kHz, have different periods and thus need different delay times,
> am I correct?

This is not a fixed value, delay is only initialized here. (look below)
>
>> +       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;

It is dynamically calculated here through clock_rate.
>> +
>> +       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;
>> +}
>> +
>
BR,

Vincenzo

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Viresh Kumar]

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

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Vincenzo Frascino]

Hi Viresh,

Il 29/02/2012 05:58, Viresh KUMAR ha scritto:
> 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;
>
I think this is ok. It covers the standard situation and the experimental foundings.

Regards,

Vincenzo

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Michael Lawnick]

Am 29.02.2012 05:58, schrieb Viresh Kumar:
...
> 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?)

After 8 data bits there is direction change and ACK/NACK sent.
AFAICS the number of clocks needed:
The situation only occurs if slave was in read mode and drives a data-0.
It will (only) see a NACK when it has driven the rest of current bits,
fall back to idle and remaining cycles will be ignored.
If master stops on first data-1 of slave, slave might ignore changes on
SDA (iow start/stop flags) as it is still in read mode, i.e. driving
actively data and not listening to SDA.

> 
> 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?
...

SDA may not go low while clock is high. This would be a violation of
protocol. Except start/stop flag of master SDA transitions are always
done while clock is low.


HTH
-- 
Michael

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Laxman Dewangan]

On Tuesday 28 February 2012 06:53 PM, viresh kumar wrote:
>
> 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.kumar-qxv4g6HH51o@public.gmane.org>
> ---
>   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 = 1000000;
> +
> +	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.

> +	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.
> +
> +	for (tmp = 0; tmp<  adap->clock_cnt * 2; tmp++, val = !val) {
> +		ndelay(delay);
should be udelay()?
> +		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.

> +	}
> +
> +	gpio_free(adap->clock_cnt);
> +
> +	return 0;
> +}
> +
> +static int i2c_scl_recover_bus(struct i2c_adapter *adap)
> +{
> +	int i, val = 0;
> +	unsigned long delay = 1000000;
> +
> +	delay /= adap->clock_rate * 2;
> +
> +	for (i = 0; i<  adap->clock_cnt * 2; i++, val = !val) {
> +		adap->set_scl(adap, val);
> +		ndelay(delay);
udelay()??
> +	}
> +
> +	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:
Always generated warning..
> +		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;
gpio can be more than 256. better to use int.
Take scl_gpio_flag and use in the gpio_request_one.
> +	u8 clock_cnt;
> +	u32 clock_rate;	/* In KHz */
>   };
>   #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
>
>
> --
> viresh


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Viresh Kumar]

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 = 1000000;
>> +
>> +	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

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Laxman Dewangan]

On Wednesday 29 February 2012 05:28 PM, Viresh Kumar wrote:
>
>>> +	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?
>

Oops, my bad.. better to name the variable as clock_rate_khz kind of to 
avoid the error when we use in our driver.


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

We observed that sometimes it does not require 9 clocks. So you can poll 
for given amount of clock time. Once the device who was holding the SDA 
line to low, release the bus, it can comeout from the loop. Not sure 
this is as per specs or not but this was our observations.




-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[viresh kumar]

On 2/29/12, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:

Here is V2:

From: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
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.kumar-qxv4g6HH51o@public.gmane.org>
---
 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 = 1000000;
+
+	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 = 1000000;
+
+	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-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..ee2d954 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -365,6 +365,35 @@ struct i2c_algorithm {
 	u32 (*functionality) (struct i2c_adapter *);
 };

+/**
+ * struct i2c_bus_recovery_info - I2c bus recovery information
+ * @recover_bus: Recover routine. Either pass driver's recover_bus()
routine, or
+ *	pass it NULL to use generic ones, i.e. gpio or scl based.
+ * @is_gpio_recovery: true, select gpio type else scl type. Only required if
+ *	recover_bus == NULL
+ * @set_scl: controller specific scl configuration routine. Only required if
+ *	is_gpio_recovery == false
+ * @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
+ * @gpio: gpio number of the scl line. Only required if
is_gpio_recovery == true
+ * @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.
+ */
+struct i2c_bus_recovery_info {
+	int (*recover_bus)(struct i2c_adapter *);
+	bool is_gpio_recovery;
+	void (*set_scl)(struct i2c_adapter *, int val);
+	int (*get_gpio)(unsigned gpio);
+	int (*put_gpio)(unsigned gpio);
+	u32 gpio;
+	u32 clock_rate_khz;
+	u8 clock_cnt;
+};
+
 /*
  * i2c_adapter is the structure used to identify a physical i2c bus along
  * with the access algorithms necessary to access it.
@@ -388,6 +417,9 @@ struct i2c_adapter {

 	struct mutex userspace_clients_lock;
 	struct list_head userspace_clients;
+
+	/* Pass valid pointer if recovery infrastructure is required */
+	struct i2c_bus_recovery_info *bus_recovery_info;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)


--
viresh

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Viresh Kumar]

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

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Rajeev kumar]

Hi Shubhrajyoti,

On 2/27/2012 3:40 PM, Rajeev kumar wrote:
> On 2/27/2012 2:42 PM, Shubhrajyoti Datta wrote:
>> On Mon, Feb 27, 2012 at 12:57 PM, Laxman
>> Dewangan<ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>  wrote:
>>> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>>>>
>>>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh
>>>> Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>    wrote:
>>>>>
>>>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>>>
>>>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C
>>>>> adapter
>>>>> driver that performs i2c bus recovery after timeout. The scope of this
>>>>> routine
>>>>> is to define i2c bus recovery procedure as specified in the i2c
>>>>> protocol
>>>>> Rev. 03
>>>>> section 3.16 titled "Bus clear".
>>>>
>>>> What do you do in the function ?
>>>>
>>>> Could we have it in the driver file itself?
>>>>
>>> I think bus recovery mechanism is to send  extra clock on SCL line by
>>> toggling the pin (using gpio apis)
>> You mean the SCL I didnt understand the gpio part?

Sorry, if I am out of sync.
Is any necessary action taken on this patch ?

Best Regards
Rajeev

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Shubhrajyoti Datta]

On Mon, Jul 2, 2012 at 11:28 AM, Rajeev kumar <rajeev-dlh.kumar-qxv4g6HH51o@public.gmane.org> wrote:
> Hi Shubhrajyoti,
>
>
> On 2/27/2012 3:40 PM, Rajeev kumar wrote:
>>
>> On 2/27/2012 2:42 PM, Shubhrajyoti Datta wrote:
>>>
>>> On Mon, Feb 27, 2012 at 12:57 PM, Laxman
>>> Dewangan<ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>  wrote:
>>>>
>>>> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>>>>>
>>>>>
>>>>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh
>>>>> Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>    wrote:
>>>>>>
>>>>>>
>>>>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>>>>
>>>>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C
>>>>>> adapter
>>>>>> driver that performs i2c bus recovery after timeout. The scope of this
>>>>>> routine
>>>>>> is to define i2c bus recovery procedure as specified in the i2c
>>>>>> protocol
>>>>>> Rev. 03
>>>>>> section 3.16 titled "Bus clear".
>>>>>
>>>>>
>>>>> What do you do in the function ?
>>>>>
>>>>> Could we have it in the driver file itself?
>>>>>
>>>> I think bus recovery mechanism is to send  extra clock on SCL line by
>>>> toggling the pin (using gpio apis)
>>>
>>> You mean the SCL I didnt understand the gpio part?
>
>
> Sorry, if I am out of sync.
> Is any necessary action taken on this patch ?
>

IIUC ,

On timeout we need to send some clock cycles.
For this you say you are using gpio apis.
I didnt understand this.

Thanks,

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Rajeev kumar]

Hello Shubhrajyoti,

On 7/2/2012 12:02 PM, Shubhrajyoti Datta wrote:
> On Mon, Jul 2, 2012 at 11:28 AM, Rajeev kumar<rajeev-dlh.kumar-qxv4g6HH51o@public.gmane.org>  wrote:
>> Hi Shubhrajyoti,
>>
>>
>> On 2/27/2012 3:40 PM, Rajeev kumar wrote:
>>>
>>> On 2/27/2012 2:42 PM, Shubhrajyoti Datta wrote:
>>>>
>>>> On Mon, Feb 27, 2012 at 12:57 PM, Laxman
>>>> Dewangan<ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>   wrote:
>>>>>
>>>>> On Monday 27 February 2012 12:51 PM, Shubhrajyoti Datta wrote:
>>>>>>
>>>>>>
>>>>>> On Fri, Feb 24, 2012 at 5:01 PM, Viresh
>>>>>> Kumar<viresh.kumar-qxv4g6HH51o@public.gmane.org>     wrote:
>>>>>>>
>>>>>>>
>>>>>>> From: Vincenzo Frascino<vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>>>>>>
>>>>>>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C
>>>>>>> adapter
>>>>>>> driver that performs i2c bus recovery after timeout. The scope of this
>>>>>>> routine
>>>>>>> is to define i2c bus recovery procedure as specified in the i2c
>>>>>>> protocol
>>>>>>> Rev. 03
>>>>>>> section 3.16 titled "Bus clear".
>>>>>>
>>>>>>
>>>>>> What do you do in the function ?
>>>>>>
>>>>>> Could we have it in the driver file itself?
>>>>>>
>>>>> I think bus recovery mechanism is to send  extra clock on SCL line by
>>>>> toggling the pin (using gpio apis)
>>>>
>>>> You mean the SCL I didnt understand the gpio part?
>>
>>
>> Sorry, if I am out of sync.
>> Is any necessary action taken on this patch ?
>>
>
> IIUC ,
>
> On timeout we need to send some clock cycles.
> For this you say you are using gpio apis.
> I didnt understand this.
>
> Thanks,

The gpio apis are not required in case if you have control over data and 
clock lines i.e. you have some register bit available for data and clock 
line.
In some i2c controller (like synopsys) you don't have control over i2c 
data and clock lines. In that case you have to use gpio lines for clock 
toggling. GPIO lines are mapped on sda and scl line of controller.

Best Regards
Rajeev


> .
>

Re: [PATCH 2/2] i2c/designware: Provide optional i2c bus recovery function

[Vincenzo Frascino]

Hi Shubhrajyoti,

Il 27/02/2012 08:21, Shubhrajyoti Datta ha scritto:
> On Fri, Feb 24, 2012 at 5:01 PM, Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org> wrote:
>> From: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>>
>> Add optional i2c_recover_bus() function to the Synopsys DesignWare I2C adapter
>> driver that performs i2c bus recovery after timeout. The scope of this routine
>> is to define i2c bus recovery procedure as specified in the i2c protocol Rev. 03
>> section 3.16 titled "Bus clear".
> What do you do in the function ?
With this function we provide the bus clear procedure for i2c recovery as
described in the the i2c protocol Rev. 03 section 3.16 titled "Bus clear".
>
> Could we have it in the driver file itself?
As it is thought now, it's implementation depends a little bit from the platform
we are using, but, sure, it can be generalized and put into the driver.
>
>> Since the Designware I2C controller doesn't provide direct control over SDA and
>> SCL lines hence the intent is to let platform try to recover the bus if they
>> have the capability to take control of I2C pads and follow the recovery
>> protocol.
> So the function is passed in the pdata.
Yes, it is.
>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>> Signed-off-by: Shiraz Hashim <shiraz.hashim-qxv4g6HH51o@public.gmane.org>
>> Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
>> ---
>>  drivers/i2c/busses/i2c-designware-core.c    |    4 +++
>>  drivers/i2c/busses/i2c-designware-core.h    |    3 +-
>>  drivers/i2c/busses/i2c-designware-platdrv.c |    8 ++++++
>>  include/linux/i2c/i2c-designware.h          |   33 +++++++++++++++++++++++++++
>>  4 files changed, 47 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 df87992..91d9357 100644
>> --- a/drivers/i2c/busses/i2c-designware-core.c
>> +++ b/drivers/i2c/busses/i2c-designware-core.c
>> @@ -525,6 +525,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");
>> +               if (dev->i2c_recover_bus) {
>> +                       dev_info(dev->dev, "try i2c bus recovery\n");
>> +                       dev->i2c_recover_bus(dev->recovery_data);
>> +               }
>>                i2c_dw_init(dev);
>>                ret = -ETIMEDOUT;
>>                goto done;
>> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
>> index 02d1a2d..e2f3119 100644
>> --- a/drivers/i2c/busses/i2c-designware-core.h
>> +++ b/drivers/i2c/busses/i2c-designware-core.h
>> @@ -34,7 +34,6 @@
>>  #define DW_IC_CON_RESTART_EN           0x20
>>  #define DW_IC_CON_SLAVE_DISABLE                0x40
>>
>> -
>>  /**
>>  * struct dw_i2c_dev - private i2c-designware data
>>  * @dev: driver model device node
>> @@ -88,6 +87,8 @@ struct dw_i2c_dev {
>>        u32                     master_cfg;
>>        unsigned int            tx_fifo_depth;
>>        unsigned int            rx_fifo_depth;
>> +       void                    (*i2c_recover_bus)(void *);
>> +       void                    *recovery_data;
>>  };
>>
>>  extern u32 dw_readl(struct dw_i2c_dev *dev, int offset);
>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
>> index c5ac2dc..2237398 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>
>> @@ -55,6 +56,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 */
>> @@ -98,6 +100,12 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
>>        }
>>        clk_enable(dev->clk);
>>
>> +       pdata = dev_get_platdata(&pdev->dev);
>> +       if (pdata && pdata->i2c_recover_bus) {
>> +               dev->i2c_recover_bus = pdata->i2c_recover_bus;
>> +               dev->recovery_data = &pdev;
>> +       }
>> +
>>        dev->functionality =
>>                I2C_FUNC_I2C |
>>                I2C_FUNC_10BIT_ADDR |
>> diff --git a/include/linux/i2c/i2c-designware.h b/include/linux/i2c/i2c-designware.h
>> new file mode 100644
>> index 0000000..e40ad85
>> --- /dev/null
>> +++ b/include/linux/i2c/i2c-designware.h
>> @@ -0,0 +1,33 @@
>> +/*
>> + * Synopsys DesignWare I2C adapter driver's platform data
>> + *
>> + * Copyright (C) 2012 ST Microelectronics.
>> + * Author: Vincenzo Frascino <vincenzo.frascino-qxv4g6HH51o@public.gmane.org>
>> + *
>> + * 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>
>> +
>> +/* I2C Designware Platform Data */
>> +struct i2c_dw_pdata {
>> +       /*
>> +        * The scope of this routine is to define i2c bus recovery procedure
>> +        * as specified in the i2c protocol Rev. 03 section 3.16 titled
>> +        * "Bus clear".
>> +        * Its implementation is platform dependant.
>> +        */
>> +       void (*i2c_recover_bus)(void *);
>> +};
>> +
>> +#endif /* I2C_DESIGNWARE_H */
>> --
>> 1.7.8.110.g4cb5d
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thanks for the comments,

Vincenzo Frascino

Re: [PATCH 1/2] i2c/busses: Add PM support

[Viresh Kumar]

On 2/24/2012 5:01 PM, Viresh KUMAR wrote:
> From: Deepak Sikri <deepak.sikri-qxv4g6HH51o@public.gmane.org>
> 
> This patch adds in support for standby/S2R/hybernate for i2c-designware driver.
> 
> Signed-off-by: Deepak Sikri <deepak.sikri-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Rajeev Kumar <rajeev-dlh.kumar-qxv4g6HH51o@public.gmane.org>
> ---
>  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

Re: [PATCH 1/2] i2c/busses: Add PM support

[Wolfram Sang]

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

On Fri, Feb 24, 2012 at 05:01:15PM +0530, Viresh Kumar wrote:
> From: Deepak Sikri <deepak.sikri-qxv4g6HH51o@public.gmane.org>
> 
> This patch adds in support for standby/S2R/hybernate for i2c-designware driver.
> 
> Signed-off-by: Deepak Sikri <deepak.sikri-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: Rajeev Kumar <rajeev-dlh.kumar-qxv4g6HH51o@public.gmane.org>

Applied to next, thanks!

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

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