[PATCH 0/3] MFD/GPIO: Add twl6040 GPO driver

[PATCH 0/3] MFD/GPIO: Add twl6040 GPO driver

[Peter Ujfalusi]

Hello,

The following series adds support for the GPO (General Purpose Output) on the
twl6040/41 audio chip.
The series has been tested on SDP4430, compile tested for x86_64 and x86_32 bit
to be sure it does not introduce build breakage.

The series depends on pending changes going through the MFD tree (twl6040
revision fixes, twl6041 support).

Linus, Grant, Samuel: Would it be possible to queue this series via the MFD tree
to avoid merge issues? It applies cleanly on top of gpio-next I believe it will
not cause problems via MFD.

Regards,
Peter
---
Peter Ujfalusi (3):
  mfd: twl6040: Fix GPO mask
  mfd: twl6040: Add twl6040-gpio child
  gpio: Add basic support for TWL6040 GPOs

 Documentation/devicetree/bindings/mfd/twl6040.txt |    4 +
 drivers/gpio/Kconfig                              |    7 +
 drivers/gpio/Makefile                             |    1 +
 drivers/gpio/gpio-twl6040.c                       |  137 +++++++++++++++++++++
 drivers/mfd/twl6040-core.c                        |   27 ++++
 include/linux/mfd/twl6040.h                       |   11 ++-
 6 files changed, 185 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpio/gpio-twl6040.c

-- 
1.7.8.6

[PATCH 1/3] mfd: twl6040: Fix GPO mask

[Peter Ujfalusi]

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 include/linux/mfd/twl6040.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h
index eaad49f..269b706 100644
--- a/include/linux/mfd/twl6040.h
+++ b/include/linux/mfd/twl6040.h
@@ -143,7 +143,7 @@
 
 #define TWL6040_GPO1			0x01
 #define TWL6040_GPO2			0x02
-#define TWL6040_GPO3			0x03
+#define TWL6040_GPO3			0x04
 
 /* ACCCTL (0x2D) fields */
 
-- 
1.7.8.6

Re: [PATCH 1/3] mfd: twl6040: Fix GPO mask

[Linus Walleij]

On Wed, Jul 18, 2012 at 11:20 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

>  #define TWL6040_GPO1                   0x01
>  #define TWL6040_GPO2                   0x02
> -#define TWL6040_GPO3                   0x03
> +#define TWL6040_GPO3                   0x04

Maybe a stupid question but does these register defines really belong in that
header file? Can't you do a separate patch moving them into the driver per
se?

Yours,
Linus Walleij

Re: [PATCH 1/3] mfd: twl6040: Fix GPO mask

[Peter Ujfalusi]

Hello Linus,

On 07/28/2012 01:45 AM, Linus Walleij wrote:
> On Wed, Jul 18, 2012 at 11:20 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> 
>>  #define TWL6040_GPO1                   0x01
>>  #define TWL6040_GPO2                   0x02
>> -#define TWL6040_GPO3                   0x03
>> +#define TWL6040_GPO3                   0x04
> 
> Maybe a stupid question but does these register defines really belong in that
> header file? Can't you do a separate patch moving them into the driver per
> se?

I thought about that but in this way the header file can be used as
documentation for the register bits of twl6040.
The GPIO driver do not use these defines, it is using shift to address the
correct bits in the GPO register.

Regards,
Péter

[PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Peter Ujfalusi]

Add needed platform data structure and code to be able to load
the GPO child of twl6040.
Update the devicetree binding documentation at the same time.

Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 Documentation/devicetree/bindings/mfd/twl6040.txt |    4 +++
 drivers/mfd/twl6040-core.c                        |   27 +++++++++++++++++++++
 include/linux/mfd/twl6040.h                       |    9 ++++++-
 3 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
index c855240..2a3d55c 100644
--- a/Documentation/devicetree/bindings/mfd/twl6040.txt
+++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
@@ -10,6 +10,8 @@ Required properties:
 - reg: must be 0x4b for i2c address
 - interrupts: twl6040 has one interrupt line connecteded to the main SoC
 - interrupt-parent: The parent interrupt controller
+- gpio-controller:
+- #gpio-cells = <1>: twl6040 provides GPO lines.
 - twl6040,audpwron-gpio: Power on GPIO line for the twl6040
 
 - vio-supply: Regulator for the twl6040 VIO supply
@@ -29,6 +31,8 @@ Required properties:
 - ti,viblmotor-res: Resistance parameter for left motor
 - ti,viblmotor-res: Resistance parameter for right motor
 
+- ti,use-gpo: Set it to <1> if the GPO functionality is in use
+
 Optional properties within vibra { } section:
 - vddvibl_uV: If the vddvibl default voltage need to be changed
 - vddvibr_uV: If the vddvibr default voltage need to be changed
diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c
index b0fad0f..8ea5b9e 100644
--- a/drivers/mfd/twl6040-core.c
+++ b/drivers/mfd/twl6040-core.c
@@ -59,6 +59,22 @@ static bool twl6040_has_vibra(struct twl6040_platform_data *pdata,
 	return false;
 }
 
+static bool twl6040_has_gpo(struct twl6040_platform_data *pdata,
+			      struct device_node *node)
+{
+	int use_gpo;
+
+	if (pdata && pdata->gpo)
+		return true;
+
+#ifdef CONFIG_OF
+	if (!of_property_read_u32(node, "ti,use-gpo", &use_gpo) && use_gpo)
+		return true;
+#endif
+
+	return false;
+}
+
 int twl6040_reg_read(struct twl6040 *twl6040, unsigned int reg)
 {
 	int ret;
@@ -631,6 +647,17 @@ static int __devinit twl6040_probe(struct i2c_client *client,
 		children++;
 	}
 
+	if (twl6040_has_gpo(pdata, node)) {
+		cell = &twl6040->cells[children];
+		cell->name = "twl6040-gpo";
+
+		if (pdata && pdata->gpo) {
+			cell->platform_data = pdata->gpo;
+			cell->pdata_size = sizeof(*pdata->gpo);
+		}
+		children++;
+	}
+
 	ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
 			      NULL, 0);
 	if (ret)
diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h
index 269b706..8991532 100644
--- a/include/linux/mfd/twl6040.h
+++ b/include/linux/mfd/twl6040.h
@@ -158,7 +158,7 @@
 #define TWL6040_VIBROCDET		0x20
 #define TWL6040_TSHUTDET                0x40
 
-#define TWL6040_CELLS			2
+#define TWL6040_CELLS			3
 
 #define TWL6040_REV_ES1_0		0x00
 #define TWL6040_REV_ES1_1		0x01 /* Rev ES1.1 and ES1.2 */
@@ -176,6 +176,8 @@
 #define TWL6040_SYSCLK_SEL_LPPLL	0
 #define TWL6040_SYSCLK_SEL_HPPLL	1
 
+#define TWL6040_GPO_MAX	3
+
 struct twl6040_codec_data {
 	u16 hs_left_step;
 	u16 hs_right_step;
@@ -192,12 +194,17 @@ struct twl6040_vibra_data {
 	int vddvibr_uV;			/* VDDVIBR volt, set 0 for fixed reg */
 };
 
+struct twl6040_gpo_data {
+	int gpio_base;
+};
+
 struct twl6040_platform_data {
 	int audpwron_gpio;	/* audio power-on gpio */
 	unsigned int irq_base;
 
 	struct twl6040_codec_data *codec;
 	struct twl6040_vibra_data *vibra;
+	struct twl6040_gpo_data *gpo;
 };
 
 struct regmap;
-- 
1.7.8.6

Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Linus Walleij]

On Wed, Jul 18, 2012 at 11:20 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

I think you need to CC DT bindings to devicetree-discuss.

> diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
> index c855240..2a3d55c 100644
> --- a/Documentation/devicetree/bindings/mfd/twl6040.txt
> +++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
> @@ -10,6 +10,8 @@ Required properties:
>  - reg: must be 0x4b for i2c address
>  - interrupts: twl6040 has one interrupt line connecteded to the main SoC
>  - interrupt-parent: The parent interrupt controller
> +- gpio-controller:
> +- #gpio-cells = <1>: twl6040 provides GPO lines.
>  - twl6040,audpwron-gpio: Power on GPIO line for the twl6040
>
>  - vio-supply: Regulator for the twl6040 VIO supply
> @@ -29,6 +31,8 @@ Required properties:
>  - ti,viblmotor-res: Resistance parameter for left motor
>  - ti,viblmotor-res: Resistance parameter for right motor
>
> +- ti,use-gpo: Set it to <1> if the GPO functionality is in use
> +

It's not like I'm sort of expert on bindings, but I've seen the
pattern status = "okay" used for this kind of boolean stuff,
and if that node is not there it's implicitly not OK.

Can someone shed some light on this convention?

Yours,
Linus Walleij

Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Peter Ujfalusi]

Hello Linus,

On 07/28/2012 01:50 AM, Linus Walleij wrote:
> On Wed, Jul 18, 2012 at 11:20 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:
> 
> I think you need to CC DT bindings to devicetree-discuss.
> 
>> diff --git a/Documentation/devicetree/bindings/mfd/twl6040.txt b/Documentation/devicetree/bindings/mfd/twl6040.txt
>> index c855240..2a3d55c 100644
>> --- a/Documentation/devicetree/bindings/mfd/twl6040.txt
>> +++ b/Documentation/devicetree/bindings/mfd/twl6040.txt
>> @@ -10,6 +10,8 @@ Required properties:
>>  - reg: must be 0x4b for i2c address
>>  - interrupts: twl6040 has one interrupt line connecteded to the main SoC
>>  - interrupt-parent: The parent interrupt controller
>> +- gpio-controller:
>> +- #gpio-cells = <1>: twl6040 provides GPO lines.
>>  - twl6040,audpwron-gpio: Power on GPIO line for the twl6040
>>
>>  - vio-supply: Regulator for the twl6040 VIO supply
>> @@ -29,6 +31,8 @@ Required properties:
>>  - ti,viblmotor-res: Resistance parameter for left motor
>>  - ti,viblmotor-res: Resistance parameter for right motor
>>
>> +- ti,use-gpo: Set it to <1> if the GPO functionality is in use
>> +
> 
> It's not like I'm sort of expert on bindings, but I've seen the
> pattern status = "okay" used for this kind of boolean stuff,
> and if that node is not there it's implicitly not OK.

Yes I have also seen status = "okay" in dts files, but usually they were
associated with sections with compatible flag.
Here I'm not sure how to use it to indicate that the core driver should create
the mfd child for the GPIO driver.
I do not have parameters for the GPIO driver.

I could, if this is working do this:

ti,twl6040-gpo {
	status = "okay";
};

If the board needs the gpo driver, but in the driver(s) I need to check for
the existence of the "ti,twl6040-gpo" node and check if the status is "okay".
I think it is easier to just get the value of "ti,use-gpo", if it exist and it
is 1 I enable the GPIO driver otherwise I don't.


Regards,
Péter

Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Mark Brown]

On Mon, Jul 30, 2012 at 05:13:17PM +0300, Peter Ujfalusi wrote:

> If the board needs the gpo driver, but in the driver(s) I need to check for
> the existence of the "ti,twl6040-gpo" node and check if the status is "okay".
> I think it is easier to just get the value of "ti,use-gpo", if it exist and it
> is 1 I enable the GPIO driver otherwise I don't.

Will having the GPIO driver actually cause any problems if it's not in
use?  It's not like things like RTC which are directly visible to
userspace and so can create problems if they're non-functional, unless
it does something like grab resources that might be needed by another
subfunction driver there's not really a substantial cost in just
instantiating it (people who are really space pressured probably won't
build the driver in in the first place).

Re: [PATCH 2/3] mfd: twl6040: Add twl6040-gpio child

[Peter Ujfalusi]

On 08/09/2012 02:55 PM, Mark Brown wrote:
> On Mon, Jul 30, 2012 at 05:13:17PM +0300, Peter Ujfalusi wrote:
> 
>> If the board needs the gpo driver, but in the driver(s) I need to check for
>> the existence of the "ti,twl6040-gpo" node and check if the status is "okay".
>> I think it is easier to just get the value of "ti,use-gpo", if it exist and it
>> is 1 I enable the GPIO driver otherwise I don't.
> 
> Will having the GPIO driver actually cause any problems if it's not in
> use?  It's not like things like RTC which are directly visible to
> userspace and so can create problems if they're non-functional, unless
> it does something like grab resources that might be needed by another
> subfunction driver there's not really a substantial cost in just
> instantiating it (people who are really space pressured probably won't
> build the driver in in the first place).

Fair enough, I'll make the change for the v2.

-- 
Péter

[PATCH 3/3] gpio: Add basic support for TWL6040 GPOs

[Peter Ujfalusi]

TWL6040 provides GPO lines to be used for controlling external devices.The number
of lines different between versions: twl6040 have 3 GPO while TWL6041 have 1.

Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/gpio/Kconfig        |    7 ++
 drivers/gpio/Makefile       |    1 +
 drivers/gpio/gpio-twl6040.c |  137 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-twl6040.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 542f0c0..4ea54a2 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -396,6 +396,13 @@ config GPIO_TWL4030
 	  Say yes here to access the GPIO signals of various multi-function
 	  power management chips from Texas Instruments.
 
+config GPIO_TWL6040
+	tristate "TWL6040 GPO"
+	depends on TWL6040_CORE
+	help
+	  Say yes here to access the GPO signals of twl6040
+	  audio chip from Texas Instruments.
+
 config GPIO_WM831X
 	tristate "WM831x GPIOs"
 	depends on MFD_WM831X
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 0f55662..d20f903 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_ARCH_DAVINCI_TNETV107X) += gpio-tnetv107x.o
 obj-$(CONFIG_GPIO_TPS65910)	+= gpio-tps65910.o
 obj-$(CONFIG_GPIO_TPS65912)	+= gpio-tps65912.o
 obj-$(CONFIG_GPIO_TWL4030)	+= gpio-twl4030.o
+obj-$(CONFIG_GPIO_TWL6040)	+= gpio-twl6040.o
 obj-$(CONFIG_GPIO_UCB1400)	+= gpio-ucb1400.o
 obj-$(CONFIG_GPIO_VR41XX)	+= gpio-vr41xx.o
 obj-$(CONFIG_GPIO_VX855)	+= gpio-vx855.o
diff --git a/drivers/gpio/gpio-twl6040.c b/drivers/gpio/gpio-twl6040.c
new file mode 100644
index 0000000..f0b2703
--- /dev/null
+++ b/drivers/gpio/gpio-twl6040.c
@@ -0,0 +1,137 @@
+/*
+ * Access to GPOs on TWL6040 chip
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * Authors:
+ *	Sergio Aguirre <saaguirre@ti.com>
+ *	Peter Ujfalusi <peter.ujfalusi@ti.com>
+ *
+ * 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 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kthread.h>
+#include <linux/irq.h>
+#include <linux/gpio.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+
+#include <linux/mfd/twl6040.h>
+
+static struct gpio_chip twl6040gpo_chip;
+
+static int twl6040gpo_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct twl6040 *twl6040 = dev_get_drvdata(chip->dev->parent);
+	int ret = 0;
+
+	ret = twl6040_reg_read(twl6040, TWL6040_REG_GPOCTL);
+	if (ret < 0)
+		return ret;
+
+	return (ret >> offset) & 1;
+}
+
+static int twl6040gpo_direction_out(struct gpio_chip *chip, unsigned offset,
+				    int value)
+{
+	/* This only drives GPOs, and can't change direction */
+	return 0;
+}
+
+static void twl6040gpo_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct twl6040 *twl6040 = dev_get_drvdata(chip->dev->parent);
+	int ret;
+	u8 gpoctl;
+
+	ret = twl6040_reg_read(twl6040, TWL6040_REG_GPOCTL);
+	if (ret < 0)
+		return;
+
+	if (value)
+		gpoctl = ret | (1 << offset);
+	else
+		gpoctl = ret & ~(1 << offset);
+
+	twl6040_reg_write(twl6040, TWL6040_REG_GPOCTL, gpoctl);
+}
+
+static struct gpio_chip twl6040gpo_chip = {
+	.label			= "twl6040",
+	.owner			= THIS_MODULE,
+	.get			= twl6040gpo_get,
+	.direction_output	= twl6040gpo_direction_out,
+	.set			= twl6040gpo_set,
+	.can_sleep		= 1,
+};
+
+/*----------------------------------------------------------------------*/
+
+static int __devinit gpo_twl6040_probe(struct platform_device *pdev)
+{
+	struct twl6040_gpo_data *pdata = pdev->dev.platform_data;
+	struct device *twl6040_core_dev = pdev->dev.parent;
+	struct twl6040 *twl6040 = dev_get_drvdata(twl6040_core_dev);
+	int ret;
+
+	if (pdata)
+		twl6040gpo_chip.base = pdata->gpio_base;
+	else
+		twl6040gpo_chip.base = -1;
+
+	if (twl6040_get_revid(twl6040) < TWL6041_REV_ES2_0)
+		twl6040gpo_chip.ngpio = 3; /* twl6040 have 3 GPO */
+	else
+		twl6040gpo_chip.ngpio = 1; /* twl6041 have 3 GPO */
+
+	twl6040gpo_chip.dev = &pdev->dev;
+#ifdef CONFIG_OF_GPIO
+	twl6040gpo_chip.of_node = twl6040_core_dev->of_node;
+#endif
+
+	ret = gpiochip_add(&twl6040gpo_chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "could not register gpiochip, %d\n", ret);
+		twl6040gpo_chip.ngpio = 0;
+	}
+
+	return ret;
+}
+
+static int __devexit gpo_twl6040_remove(struct platform_device *pdev)
+{
+	return gpiochip_remove(&twl6040gpo_chip);
+}
+
+/* Note:  this hardware lives inside an I2C-based multi-function device. */
+MODULE_ALIAS("platform:twl6040-gpo");
+
+static struct platform_driver gpo_twl6040_driver = {
+	.driver = {
+		.name	= "twl6040-gpo",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= gpo_twl6040_probe,
+	.remove		= gpo_twl6040_remove,
+};
+
+module_platform_driver(gpo_twl6040_driver);
+
+MODULE_AUTHOR("Texas Instruments, Inc.");
+MODULE_DESCRIPTION("GPO interface for TWL6040");
+MODULE_LICENSE("GPL");
-- 
1.7.8.6

Re: [PATCH 3/3] gpio: Add basic support for TWL6040 GPOs

[Linus Walleij]

On Wed, Jul 18, 2012 at 11:20 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

> TWL6040 provides GPO lines to be used for controlling external devices.The number
> of lines different between versions: twl6040 have 3 GPO while TWL6041 have 1.
>
> Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

This looks pretty straight-forward and nice!
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 0/3] MFD/GPIO: Add twl6040 GPO driver

[Linus Walleij]

On Wed, Jul 18, 2012 at 11:20 AM, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote:

> Linus, Grant, Samuel: Would it be possible to queue this series via the MFD tree
> to avoid merge issues? It applies cleanly on top of gpio-next I believe it will
> not cause problems via MFD.

Sure, as soon as you have mine or Grants ACK on the GPIO stuff you
can ask Sam to queue it. But first, some review :-)

Linus Walleij