Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751660Ab1FHSg6 (ORCPT ); Wed, 8 Jun 2011 14:36:58 -0400 Received: from mail-pw0-f46.google.com ([209.85.160.46]:55481 "EHLO mail-pw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750943Ab1FHSg5 (ORCPT ); Wed, 8 Jun 2011 14:36:57 -0400 Date: Wed, 8 Jun 2011 12:36:55 -0600 From: Grant Likely To: Margarita Olaya Cc: linux-kernel@vger.kernel.org, "Girdwood, Liam" , Graeme Gregory Subject: Re: [PATCHv4 3/4][RESEND] tps65912: gpio: add gpio driver Message-ID: <20110608183655.GN8499@ponder.secretlab.ca> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7567 Lines: 256 On Wed, Jun 08, 2011 at 12:42:47PM -0500, Margarita Olaya wrote: > TPS65912 has five GPIOs that can be configured for different > purposes. > > Signed-off-by: Margarita Olaya Cabrera Looks like a good driver. A few minor comments below, but you can add my acked by when they are fixed. Acked-by: Grant Likely Since this depends on the earlier patches in the series, it should be merged via the mfd tree. g. > --- > drivers/gpio/Kconfig | 6 ++ > drivers/gpio/Makefile | 1 + > drivers/gpio/tps65912-gpio.c | 159 ++++++++++++++++++++++++++++++++++++++++++ drivers/gpio/gpio-tps65912.c > include/linux/mfd/tps65912.h | 1 + > 4 files changed, 167 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpio/tps65912-gpio.c > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index d3b2953..7d79b7d 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -247,6 +247,12 @@ config GPIO_TWL4030 > Say yes here to access the GPIO signals of various multi-function > power management chips from Texas Instruments. > > +config GPIO_TPS65912 > + tristate "TI TPS65912 GPIO" > + depends on (MFD_TPS65912_I2C || MFD_TPS65912_SPI) > + help > + This driver supports TPS65912 gpio chip > + Nit: keep this file list sorted alphabetically please. > config GPIO_WM831X > tristate "WM831x GPIOs" > depends on MFD_WM831X > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index becef59..f03ccc4 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -27,6 +27,7 @@ obj-$(CONFIG_GPIO_STMPE) += stmpe-gpio.o > obj-$(CONFIG_GPIO_TC3589X) += tc3589x-gpio.o > obj-$(CONFIG_GPIO_TIMBERDALE) += timbgpio.o > obj-$(CONFIG_GPIO_TWL4030) += twl4030-gpio.o > +obj-$(CONFIG_GPIO_TPS65912) += tps65912-gpio.o > obj-$(CONFIG_GPIO_UCB1400) += ucb1400_gpio.o > obj-$(CONFIG_GPIO_XILINX) += xilinx_gpio.o > obj-$(CONFIG_GPIO_CS5535) += cs5535-gpio.o > diff --git a/drivers/gpio/tps65912-gpio.c b/drivers/gpio/tps65912-gpio.c > new file mode 100644 > index 0000000..2be15a2 > --- /dev/null > +++ b/drivers/gpio/tps65912-gpio.c > @@ -0,0 +1,159 @@ > +/* > + * tps65912-gpio.c -- TI TPS6591x Nit: No need to put the file name in the header block. A description of /what/ the driver is for is generally more useful here. > + * > + * Copyright 2011 Texas Instruments Inc. > + * > + * Author: Margarita Olaya > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + * > + * This driver is based on wm8350 implementation. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +struct tps65912_gpio_data { > + struct tps65912 *tps65912; > + struct gpio_chip gpio_chip; > +}; > + > +static int tps65912_gpio_get(struct gpio_chip *gc, unsigned offset) > +{ > + struct tps65912 *tps65912 = container_of(gc, struct tps65912, gpio); > + int val; > + > + val = tps65912_reg_read(tps65912, TPS65912_GPIO1 + offset); > + > + if (val & GPIO_STS_MASK) > + return 1; > + > + return 0; > +} > + > +static void tps65912_gpio_set(struct gpio_chip *gc, unsigned offset, > + int value) > +{ > + struct tps65912 *tps65912 = container_of(gc, struct tps65912, gpio); > + > + if (value) > + tps65912_set_bits(tps65912, TPS65912_GPIO1 + offset, > + GPIO_SET_MASK); > + else > + tps65912_clear_bits(tps65912, TPS65912_GPIO1 + offset, > + GPIO_SET_MASK); > +} > + > +static int tps65912_gpio_output(struct gpio_chip *gc, unsigned offset, > + int value) > +{ > + struct tps65912 *tps65912 = container_of(gc, struct tps65912, gpio); > + > + /* Set the initial value */ > + tps65912_gpio_set(gc, offset, value); > + > + return tps65912_set_bits(tps65912, TPS65912_GPIO1 + offset, > + GPIO_CFG_MASK); > +} > + > +static int tps65912_gpio_input(struct gpio_chip *gc, unsigned offset) > +{ > + struct tps65912 *tps65912 = container_of(gc, struct tps65912, gpio); > + > + return tps65912_clear_bits(tps65912, TPS65912_GPIO1 + offset, > + GPIO_CFG_MASK); > + > +} > + > +static struct gpio_chip template_chip = { const? > + .label = "tps65912", > + .owner = THIS_MODULE, > + .direction_input = tps65912_gpio_input, > + .direction_output = tps65912_gpio_output, > + .get = tps65912_gpio_get, > + .set = tps65912_gpio_set, > + .can_sleep = 1, > +}; > + > +static int __devinit tps65912_gpio_probe(struct platform_device *pdev) > +{ > + struct tps65912 *tps65912 = dev_get_drvdata(pdev->dev.parent); > + struct tps65912_board *pdata = tps65912->dev->platform_data; > + struct tps65912_gpio_data *tps65912_gpio; > + int ret; > + > + tps65912_gpio = kzalloc(sizeof(*tps65912_gpio), GFP_KERNEL); > + if (tps65912_gpio == NULL) > + return -ENOMEM; > + > + tps65912_gpio->tps65912 = tps65912; > + tps65912_gpio->gpio_chip = template_chip; > + tps65912_gpio->gpio_chip.ngpio = 5; Should gpio_chip.ngpio also be in the template chip? > + tps65912_gpio->gpio_chip.dev = &pdev->dev; > + if (pdata && pdata->gpio_base) > + tps65912_gpio->gpio_chip.base = pdata->gpio_base; > + else > + tps65912_gpio->gpio_chip.base = -1; Ditto for the default value of gpio_chip.base. > + > + ret = gpiochip_add(&tps65912_gpio->gpio_chip); > + if (ret < 0) { > + dev_err(&pdev->dev, "Failed to register gpiochip, %d\n", ret); > + goto err; > + } > + > + platform_set_drvdata(pdev, tps65912_gpio); > + > + return ret; > + > +err: > + kfree(tps65912_gpio); > + return ret; > +} > + > +static int __devexit tps65912_gpio_remove(struct platform_device *pdev) > +{ > + struct tps65912_gpio_data *tps65912_gpio = platform_get_drvdata(pdev); > + int ret; > + > + ret = gpiochip_remove(&tps65912_gpio->gpio_chip); > + if (ret == 0) > + kfree(tps65912_gpio); > + > + return ret; > +} > + > +static struct platform_driver tps65912_gpio_driver = { > + .driver = { > + .name = "tps65912-gpio", > + .owner = THIS_MODULE, > + }, > + .probe = tps65912_gpio_probe, > + .remove = __devexit_p(tps65912_gpio_remove), > +}; > + > +static int __init tps65912_gpio_init(void) > +{ > + return platform_driver_register(&tps65912_gpio_driver); > +} > +subsys_initcall(tps65912_gpio_init); > + > +static void __exit tps65912_gpio_exit(void) > +{ > + platform_driver_unregister(&tps65912_gpio_driver); > +} > +module_exit(tps65912_gpio_exit); > + > +MODULE_AUTHOR("Margarita Olaya Cabrera "); > +MODULE_DESCRIPTION("GPIO interface for TPS65912 PMICs"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:tps65912-gpio"); > diff --git a/include/linux/mfd/tps65912.h b/include/linux/mfd/tps65912.h > index be60fb2..aaceab4 100644 > --- a/include/linux/mfd/tps65912.h > +++ b/include/linux/mfd/tps65912.h > @@ -275,6 +275,7 @@ struct tps65912_board { > int is_dcdc4_avs; > int irq; > int irq_base; > + int gpio_base; > struct regulator_init_data *tps65912_pmic_init_data; > }; > > -- > 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/