[PATCH V1] i2c: gpio: Use open drain support from gpio driver

[PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Laxman Dewangan]

The gpio core driver (gpio library) supports the open
drain pin handling. Therefore, it is not require it
to handle in the i2c-gpio driver, just require
to pass the OPEN_DRAIN type flag when requesting the gpio.

Remove the handling of open drain pin in the i2c-gpio driver.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/i2c/busses/i2c-gpio.c |   78 ++++++++++++++--------------------------
 1 files changed, 27 insertions(+), 51 deletions(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index 69fbfae..662a747 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -16,22 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
 
-/* Toggle SDA by changing the direction of the pin */
-static void i2c_gpio_setsda_dir(void *data, int state)
-{
-	struct i2c_gpio_platform_data *pdata = data;
-
-	if (state)
-		gpio_direction_input(pdata->sda_pin);
-	else
-		gpio_direction_output(pdata->sda_pin, 0);
-}
-
-/*
- * Toggle SDA by changing the output value of the pin. This is only
- * valid for pins configured as open drain (i.e. setting the value
- * high effectively turns off the output driver.)
- */
+/* Toggle SDA by setting value, gpio library take care of open drain pins.*/
 static void i2c_gpio_setsda_val(void *data, int state)
 {
 	struct i2c_gpio_platform_data *pdata = data;
@@ -39,23 +24,7 @@ static void i2c_gpio_setsda_val(void *data, int state)
 	gpio_set_value(pdata->sda_pin, state);
 }
 
-/* Toggle SCL by changing the direction of the pin. */
-static void i2c_gpio_setscl_dir(void *data, int state)
-{
-	struct i2c_gpio_platform_data *pdata = data;
-
-	if (state)
-		gpio_direction_input(pdata->scl_pin);
-	else
-		gpio_direction_output(pdata->scl_pin, 0);
-}
-
-/*
- * Toggle SCL by changing the output value of the pin. This is used
- * for pins that are configured as open drain and for output-only
- * pins. The latter case will break the i2c protocol, but it will
- * often work in practice.
- */
+ /* Toggle SCL by setting value, gpio library take care of open drain pins.*/
 static void i2c_gpio_setscl_val(void *data, int state)
 {
 	struct i2c_gpio_platform_data *pdata = data;
@@ -83,6 +52,8 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	struct i2c_algo_bit_data *bit_data;
 	struct i2c_adapter *adap;
 	int ret;
+	unsigned long sda_gpio_flags;
+	unsigned long scl_gpio_flags;
 
 	pdata = pdev->dev.platform_data;
 	if (!pdata)
@@ -96,28 +67,33 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	if (!bit_data)
 		goto err_alloc_bit_data;
 
-	ret = gpio_request(pdata->sda_pin, "sda");
-	if (ret)
+	/* Initially, SCL and SDA pin should be HIGH */
+	sda_gpio_flags = GPIOF_OUT_INIT_HIGH;
+	scl_gpio_flags = GPIOF_OUT_INIT_HIGH;
+
+	if (pdata->sda_is_open_drain)
+		sda_gpio_flags |= GPIOF_OPEN_DRAIN;
+
+	if (pdata->scl_is_open_drain && !pdata->scl_is_output_only)
+		scl_gpio_flags |= GPIOF_OPEN_DRAIN;
+
+
+
+	ret = gpio_request_one(pdata->sda_pin, sda_gpio_flags, "sda");
+	if (ret) {
+		pr_err("%s(): Error in requesting sda gpio%d, ret %d\n",
+				__func__, pdata->sda_pin, ret);
 		goto err_request_sda;
-	ret = gpio_request(pdata->scl_pin, "scl");
-	if (ret)
+	}
+	ret = gpio_request_one(pdata->scl_pin, scl_gpio_flags, "scl");
+	if (ret) {
+		pr_err("%s(): Error in requesting scl gpio%d, ret %d\n",
+				__func__, pdata->scl_pin, ret);
 		goto err_request_scl;
-
-	if (pdata->sda_is_open_drain) {
-		gpio_direction_output(pdata->sda_pin, 1);
-		bit_data->setsda = i2c_gpio_setsda_val;
-	} else {
-		gpio_direction_input(pdata->sda_pin);
-		bit_data->setsda = i2c_gpio_setsda_dir;
 	}
 
-	if (pdata->scl_is_open_drain || pdata->scl_is_output_only) {
-		gpio_direction_output(pdata->scl_pin, 1);
-		bit_data->setscl = i2c_gpio_setscl_val;
-	} else {
-		gpio_direction_input(pdata->scl_pin);
-		bit_data->setscl = i2c_gpio_setscl_dir;
-	}
+	bit_data->setsda = i2c_gpio_setsda_val;
+	bit_data->setscl = i2c_gpio_setscl_val;
 
 	if (!pdata->scl_is_output_only)
 		bit_data->getscl = i2c_gpio_getscl;
-- 
1.7.1.1

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Mark Brown]

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

On Wed, Mar 07, 2012 at 07:08:14PM +0530, Laxman Dewangan wrote:
> The gpio core driver (gpio library) supports the open
> drain pin handling. Therefore, it is not require it
> to handle in the i2c-gpio driver, just require
> to pass the OPEN_DRAIN type flag when requesting the gpio.

Reviwed-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

but note that this is a new gpiolib feature in -next and will need to
either be applied via Grant's tree or wait until after the merge window.

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

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Wolfram Sang]

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

On Wed, Mar 07, 2012 at 05:31:57PM +0000, Mark Brown wrote:
> On Wed, Mar 07, 2012 at 07:08:14PM +0530, Laxman Dewangan wrote:
> > The gpio core driver (gpio library) supports the open
> > drain pin handling. Therefore, it is not require it
> > to handle in the i2c-gpio driver, just require
> > to pass the OPEN_DRAIN type flag when requesting the gpio.
> 
> Reviwed-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

Typo ;)

> 
> but note that this is a new gpiolib feature in -next and will need to
> either be applied via Grant's tree or wait until after the merge window.

I'll pick it for v3.5 later. Thanks for the review!

   Wolfram

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

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

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Mark Brown]

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

On Wed, Mar 07, 2012 at 06:39:22PM +0100, Wolfram Sang wrote:
> On Wed, Mar 07, 2012 at 05:31:57PM +0000, Mark Brown wrote:

> > Reviwed-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

> Typo ;)

Not at all, it's a direct demonstration of the quick review I did!  :P

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

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Laxman Dewangan]

The open drain support is already available from next-20120306.
I was waiting for the change to be in tree before sending this patch.
Hope I am not missing anything here.


On Wednesday 07 March 2012 11:14 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Wed, Mar 07, 2012 at 06:39:22PM +0100, Wolfram Sang wrote:
>> On Wed, Mar 07, 2012 at 05:31:57PM +0000, Mark Brown wrote:
>>> Reviwed-by: Mark Brown<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
>> Typo ;)
> Not at all, it's a direct demonstration of the quick review I did!  :P
>
> * Unknown Key
> * 0x6E30FDDD

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Mark Brown]

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

On Wed, Mar 07, 2012 at 11:38:25PM +0530, Laxman Dewangan wrote:
> The open drain support is already available from next-20120306.
> I was waiting for the change to be in tree before sending this patch.
> Hope I am not missing anything here.

Nobody should be merging -next into their trees, it's rebuilt every day
- you can only rely on things in Linus tree or things which have been
explicitly cross merged into other trees.  Otherwise the individual tree
won't build as it doesn't contain the patch you depend on which breaks
bisection.

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

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Linus Walleij]

On Wed, Mar 7, 2012 at 6:31 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Mar 07, 2012 at 07:08:14PM +0530, Laxman Dewangan wrote:
>> The gpio core driver (gpio library) supports the open
>> drain pin handling. Therefore, it is not require it
>> to handle in the i2c-gpio driver, just require
>> to pass the OPEN_DRAIN type flag when requesting the gpio.
>
> Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> but note that this is a new gpiolib feature in -next and will need to
> either be applied via Grant's tree or wait until after the merge window.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
I'd ask Grant to take this into the GPIO tree if Broonie can ACK it.

Yours,
Linus Walleij

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Wolfram Sang]

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

> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> I'd ask Grant to take this into the GPIO tree if Broonie can ACK it.

Uhm, why? What's wrong with going in via I2C?

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

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

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Linus Walleij]

On Thu, Mar 8, 2012 at 8:41 AM, Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:

>> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> I'd ask Grant to take this into the GPIO tree if Broonie can ACK it.
>
> Uhm, why? What's wrong with going in via I2C?

So it can be merged without bisect regressions for this merge
window, Laxman seems to prefer that it get in now.

If it doesn't get merged until v3.5, no issue.

Yours,
Linus Walleij

Re: [PATCH V1] i2c: gpio: Use open drain support from gpio driver

[Mark Brown]

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

On Thu, Mar 08, 2012 at 07:24:39AM +0100, Linus Walleij wrote:

> > but note that this is a new gpiolib feature in -next and will need to
> > either be applied via Grant's tree or wait until after the merge window.

> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> I'd ask Grant to take this into the GPIO tree if Broonie can ACK it.

You're looking for Wolfram or Ben there, I don't really do much I2C
stuff.

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