| Message ID | 1331127494-6913-1-git-send-email-ldewangan@nvidia.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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.
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@opensource.wolfsonmicro.com> 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
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@opensource.wolfsonmicro.com> > Typo ;) Not at all, it's a direct demonstration of the quick review I did! :P
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@opensource.wolfsonmicro.com> >> Typo ;) > Not at all, it's a direct demonstration of the quick review I did! :P > > * Unknown Key > * 0x6E30FDDD -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Acked-by: Linus Walleij <linus.walleij@linaro.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?
On Thu, Mar 8, 2012 at 8:41 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: >> Acked-by: Linus Walleij <linus.walleij@linaro.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 -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@linaro.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.
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;
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@nvidia.com> --- drivers/i2c/busses/i2c-gpio.c | 78 ++++++++++++++-------------------------- 1 files changed, 27 insertions(+), 51 deletions(-)