[PATCH v2 1/2] input: st1232: Add reset pin handling

[PATCH v2 1/2] input: st1232: Add reset pin handling

[Bastian Hecht]

We add the possiblity to hand over a GPIO number for the reset pin.
This way we can remove existing board code that takes care of it and
group this information properly in the platform data or in the device
tree confguration.

The implementation is analogous to the cy8ctmg110 driver, thanks.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
v2:

- using a gpio phandle now in the DT code
- rename from reset_pin to reset_gpio
- gpio number 0 is allowed now and the absence of the reset gpio is marked with an error code
- using devm_request_gpio instead of plain request_gpio to simplify things

 .../bindings/input/touchscreen/sitronix-st1232.txt |   24 ++++++++++++
 drivers/input/touchscreen/st1232.c                 |   41 +++++++++++++++++++-
 include/linux/input/st1232_pdata.h                 |   13 +++++++
 3 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
 create mode 100644 include/linux/input/st1232_pdata.h

diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
new file mode 100644
index 0000000..64ad48b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
@@ -0,0 +1,24 @@
+* Sitronix st1232 touchscreen controller
+
+Required properties:
+- compatible: must be "sitronix,st1232"
+- reg: I2C address of the chip
+- interrupts: interrupt to which the chip is connected
+
+Optional properties:
+- gpios: a phandle to the reset GPIO
+
+Example:
+
+	i2c@00000000 {
+		/* ... */
+
+		touchscreen@55 {
+			compatible = "sitronix,st1232";
+			reg = <0x55>;
+			interrupts = <2 0>;
+			gpios = <&gpio1 166 0>;
+		};
+
+		/* ... */
+	};
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index d9d05e2..498d18b 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -19,13 +19,16 @@
  */
 
 #include <linux/delay.h>
+#include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/of_gpio.h>
 #include <linux/pm_qos.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/input/st1232_pdata.h>
 
 #define ST1232_TS_NAME	"st1232-ts"
 
@@ -48,6 +51,7 @@ struct st1232_ts_data {
 	struct input_dev *input_dev;
 	struct st1232_ts_finger finger[MAX_FINGERS];
 	struct dev_pm_qos_request low_latency_req;
+	int reset_gpio;
 };
 
 static int st1232_ts_read_data(struct st1232_ts_data *ts)
@@ -139,10 +143,17 @@ end:
 	return IRQ_HANDLED;
 }
 
+static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
+{
+	if (ts->reset_gpio >= 0)
+		gpio_direction_output(ts->reset_gpio, poweron);
+}
+
 static int st1232_ts_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
 	struct st1232_ts_data *ts;
+	struct st1232_pdata *pdata = client->dev.platform_data;
 	struct input_dev *input_dev;
 	int error;
 
@@ -167,6 +178,25 @@ static int st1232_ts_probe(struct i2c_client *client,
 	ts->client = client;
 	ts->input_dev = input_dev;
 
+	if (client->dev.of_node)
+		ts->reset_gpio = of_get_gpio(client->dev.of_node, 0);
+	else if (pdata)
+		ts->reset_gpio = pdata->reset_gpio;
+	else
+		ts->reset_gpio = -ENODEV;
+
+	if (ts->reset_gpio >= 0) {
+		error = devm_gpio_request(&client->dev, ts->reset_gpio, NULL);
+		if (error) {
+			dev_err(&client->dev,
+				"Unable to request GPIO pin %d.\n",
+				ts->reset_gpio);
+				goto err_free_mem;
+		}
+	}
+
+	st1232_ts_power(ts, true);
+
 	input_dev->name = "st1232-touchscreen";
 	input_dev->id.bustype = BUS_I2C;
 	input_dev->dev.parent = &client->dev;
@@ -210,6 +240,7 @@ static int st1232_ts_remove(struct i2c_client *client)
 {
 	struct st1232_ts_data *ts = i2c_get_clientdata(client);
 
+	st1232_ts_power(ts, false);
 	device_init_wakeup(&client->dev, 0);
 	free_irq(client->irq, ts);
 	input_unregister_device(ts->input_dev);
@@ -222,11 +253,14 @@ static int st1232_ts_remove(struct i2c_client *client)
 static int st1232_ts_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct st1232_ts_data *ts = i2c_get_clientdata(client);
 
 	if (device_may_wakeup(&client->dev))
 		enable_irq_wake(client->irq);
-	else
+	else {
 		disable_irq(client->irq);
+		st1232_ts_power(ts, false);
+	}
 
 	return 0;
 }
@@ -234,11 +268,14 @@ static int st1232_ts_suspend(struct device *dev)
 static int st1232_ts_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct st1232_ts_data *ts = i2c_get_clientdata(client);
 
 	if (device_may_wakeup(&client->dev))
 		disable_irq_wake(client->irq);
-	else
+	else {
+		st1232_ts_power(ts, true);
 		enable_irq(client->irq);
+	}
 
 	return 0;
 }
diff --git a/include/linux/input/st1232_pdata.h b/include/linux/input/st1232_pdata.h
new file mode 100644
index 0000000..cac3e7b
--- /dev/null
+++ b/include/linux/input/st1232_pdata.h
@@ -0,0 +1,13 @@
+#ifndef _LINUX_ST1232_PDATA_H
+#define _LINUX_ST1232_PDATA_H
+
+/*
+ * Optional platform data
+ *
+ * Use this if you want the driver to drive the reset pin.
+ */
+struct st1232_pdata {
+	int reset_gpio;
+};
+
+#endif
-- 
1.7.9.5

[PATCH v2 2/2] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin handling

[Bastian Hecht]

We no longer need to set up the reset pin for the st1232 in the board
code, but can pass the GPIO number via the platform data to the driver.
This results in a cleaner grouping of the device setup.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
---
v2:
reset_pin changed to reset_gpio

 arch/arm/mach-shmobile/board-armadillo800eva.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index bf4c3d2..d1c1ff4 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -24,6 +24,7 @@
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/input.h>
+#include <linux/input/st1232_pdata.h>
 #include <linux/irq.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
@@ -997,10 +998,15 @@ static struct platform_device i2c_gpio_device = {
 };
 
 /* I2C */
+static struct st1232_pdata st1232_i2c0_pdata = {
+	.reset_gpio = 166,
+};
+
 static struct i2c_board_info i2c0_devices[] = {
 	{
 		I2C_BOARD_INFO("st1232-ts", 0x55),
 		.irq = irq_pin(10),
+		.platform_data = &st1232_i2c0_pdata,
 	},
 	{
 		I2C_BOARD_INFO("wm8978", 0x1a),
@@ -1120,7 +1126,6 @@ static void __init eva_init(void)
 
 	/* Touchscreen */
 	gpio_request(GPIO_FN_IRQ10,	NULL); /* TP_INT */
-	gpio_request_one(166, GPIOF_OUT_INIT_HIGH, NULL); /* TP_RST_B */
 
 	/* GETHER */
 	gpio_request(GPIO_FN_ET_CRS,		NULL);
-- 
1.7.9.5

Re: [PATCH v2 2/2] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin handling

[Laurent Pinchart]

Hi Bastian,

Thanks for the patch.

On Monday 08 April 2013 14:52:27 Bastian Hecht wrote:
> We no longer need to set up the reset pin for the st1232 in the board
> code, but can pass the GPIO number via the platform data to the driver.
> This results in a cleaner grouping of the device setup.
> 
> Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> v2:
> reset_pin changed to reset_gpio
> 
>  arch/arm/mach-shmobile/board-armadillo800eva.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c
> b/arch/arm/mach-shmobile/board-armadillo800eva.c index bf4c3d2..d1c1ff4
> 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -24,6 +24,7 @@
>  #include <linux/err.h>
>  #include <linux/kernel.h>
>  #include <linux/input.h>
> +#include <linux/input/st1232_pdata.h>
>  #include <linux/irq.h>
>  #include <linux/platform_device.h>
>  #include <linux/gpio.h>
> @@ -997,10 +998,15 @@ static struct platform_device i2c_gpio_device = {
>  };
> 
>  /* I2C */
> +static struct st1232_pdata st1232_i2c0_pdata = {
> +	.reset_gpio = 166,
> +};
> +
>  static struct i2c_board_info i2c0_devices[] = {
>  	{
>  		I2C_BOARD_INFO("st1232-ts", 0x55),
>  		.irq = irq_pin(10),
> +		.platform_data = &st1232_i2c0_pdata,
>  	},
>  	{
>  		I2C_BOARD_INFO("wm8978", 0x1a),
> @@ -1120,7 +1126,6 @@ static void __init eva_init(void)
> 
>  	/* Touchscreen */
>  	gpio_request(GPIO_FN_IRQ10,	NULL); /* TP_INT */
> -	gpio_request_one(166, GPIOF_OUT_INIT_HIGH, NULL); /* TP_RST_B */
> 
>  	/* GETHER */
>  	gpio_request(GPIO_FN_ET_CRS,		NULL);
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/2] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin handling

[Simon Horman]

On Mon, Apr 08, 2013 at 04:17:22PM +0200, Laurent Pinchart wrote:
> Hi Bastian,
> 
> Thanks for the patch.
> 
> On Monday 08 April 2013 14:52:27 Bastian Hecht wrote:
> > We no longer need to set up the reset pin for the st1232 in the board
> > code, but can pass the GPIO number via the platform data to the driver.
> > This results in a cleaner grouping of the device setup.
> > 
> > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks.

I have queued this up in the boards-armadillo800eva branch.

There were some conflicts when I applied the code which
I fixed manually. Bastian, can you please check that the
following is correct? If it is not please send an incremental patch.


>From 14e0f336a45cdb9150aab5731c2b5b74f4dc3588 Mon Sep 17 00:00:00 2001
From: Bastian Hecht <hechtb@gmail.com>
Date: Mon, 8 Apr 2013 12:52:27 +0000
Subject: [PATCH] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin
 handling

We no longer need to set up the reset pin for the st1232 in the board
code, but can pass the GPIO number via the platform data to the driver.
This results in a cleaner grouping of the device setup.

Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 arch/arm/mach-shmobile/board-armadillo800eva.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index f322a18..60fca5d 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -24,6 +24,7 @@
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/input.h>
+#include <linux/input/st1232_pdata.h>
 #include <linux/irq.h>
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
@@ -968,10 +969,14 @@ static struct platform_device i2c_gpio_device = {
 };
 
 /* I2C */
+static struct st1232_pdata st1232_i2c0_pdata = {
+	.reset_gpio = 166,
+};
 static struct i2c_board_info i2c0_devices[] = {
 	{
 		I2C_BOARD_INFO("st1232-ts", 0x55),
 		.irq = evt2irq(0x0340),
+		.platform_data = &st1232_i2c0_pdata,
 	},
 	{
 		I2C_BOARD_INFO("wm8978", 0x1a),
@@ -1092,7 +1097,6 @@ static void __init eva_init(void)
 
 	/* Touchscreen */
 	gpio_request(GPIO_FN_IRQ10,	NULL); /* TP_INT */
-	gpio_request_one(166, GPIOF_OUT_INIT_HIGH, NULL); /* TP_RST_B */
 
 	/* GETHER */
 	gpio_request(GPIO_FN_ET_CRS,		NULL);
-- 
1.7.10.4

Re: [PATCH v2 2/2] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin handling

[Simon Horman]

On Tue, Apr 09, 2013 at 05:26:44PM +0900, Simon Horman wrote:
> On Mon, Apr 08, 2013 at 04:17:22PM +0200, Laurent Pinchart wrote:
> > Hi Bastian,
> > 
> > Thanks for the patch.
> > 
> > On Monday 08 April 2013 14:52:27 Bastian Hecht wrote:
> > > We no longer need to set up the reset pin for the st1232 in the board
> > > code, but can pass the GPIO number via the platform data to the driver.
> > > This results in a cleaner grouping of the device setup.
> > > 
> > > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> > 
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks.
> 
> I have queued this up in the boards-armadillo800eva branch.
> 
> There were some conflicts when I applied the code which
> I fixed manually. Bastian, can you please check that the
> following is correct? If it is not please send an incremental patch.

I was trying to apply your patch to an old branch.
It applies cleanly to the correct new branch :)

Re: [PATCH v2 2/2] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin handling

[Bastian Hecht]

2013/4/9 Simon Horman <horms@verge.net.au>:
> On Tue, Apr 09, 2013 at 05:26:44PM +0900, Simon Horman wrote:
>> On Mon, Apr 08, 2013 at 04:17:22PM +0200, Laurent Pinchart wrote:
>> > Hi Bastian,
>> >
>> > Thanks for the patch.
>> >
>> > On Monday 08 April 2013 14:52:27 Bastian Hecht wrote:
>> > > We no longer need to set up the reset pin for the st1232 in the board
>> > > code, but can pass the GPIO number via the platform data to the driver.
>> > > This results in a cleaner grouping of the device setup.
>> > >
>> > > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
>> >
>> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> Thanks.
>>
>> I have queued this up in the boards-armadillo800eva branch.
>>
>> There were some conflicts when I applied the code which
>> I fixed manually. Bastian, can you please check that the
>> following is correct? If it is not please send an incremental patch.
>
> I was trying to apply your patch to an old branch.
> It applies cleanly to the correct new branch :)

Ah sweet, I was indeed a bit confused about going back to the INTC as
primary controller.

Re: [PATCH v2 2/2] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin handling

[Simon Horman]

On Tue, Apr 09, 2013 at 11:16:18AM +0200, Bastian Hecht wrote:
> 2013/4/9 Simon Horman <horms@verge.net.au>:
> > On Tue, Apr 09, 2013 at 05:26:44PM +0900, Simon Horman wrote:
> >> On Mon, Apr 08, 2013 at 04:17:22PM +0200, Laurent Pinchart wrote:
> >> > Hi Bastian,
> >> >
> >> > Thanks for the patch.
> >> >
> >> > On Monday 08 April 2013 14:52:27 Bastian Hecht wrote:
> >> > > We no longer need to set up the reset pin for the st1232 in the board
> >> > > code, but can pass the GPIO number via the platform data to the driver.
> >> > > This results in a cleaner grouping of the device setup.
> >> > >
> >> > > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> >> >
> >> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> Thanks.
> >>
> >> I have queued this up in the boards-armadillo800eva branch.
> >>
> >> There were some conflicts when I applied the code which
> >> I fixed manually. Bastian, can you please check that the
> >> following is correct? If it is not please send an incremental patch.
> >
> > I was trying to apply your patch to an old branch.
> > It applies cleanly to the correct new branch :)
> 
> Ah sweet, I was indeed a bit confused about going back to the INTC as
> primary controller.

Sorry for the confusion.

Re: [PATCH v2 2/2] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin handling

[Simon Horman]

On Tue, Apr 09, 2013 at 11:16:18AM +0200, Bastian Hecht wrote:
> 2013/4/9 Simon Horman <horms@verge.net.au>:
> > On Tue, Apr 09, 2013 at 05:26:44PM +0900, Simon Horman wrote:
> >> On Mon, Apr 08, 2013 at 04:17:22PM +0200, Laurent Pinchart wrote:
> >> > Hi Bastian,
> >> >
> >> > Thanks for the patch.
> >> >
> >> > On Monday 08 April 2013 14:52:27 Bastian Hecht wrote:
> >> > > We no longer need to set up the reset pin for the st1232 in the board
> >> > > code, but can pass the GPIO number via the platform data to the driver.
> >> > > This results in a cleaner grouping of the device setup.
> >> > >
> >> > > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> >> >
> >> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> Thanks.
> >>
> >> I have queued this up in the boards-armadillo800eva branch.
> >>
> >> There were some conflicts when I applied the code which
> >> I fixed manually. Bastian, can you please check that the
> >> following is correct? If it is not please send an incremental patch.
> >
> > I was trying to apply your patch to an old branch.
> > It applies cleanly to the correct new branch :)
> 
> Ah sweet, I was indeed a bit confused about going back to the INTC as
> primary controller.

After all that I had to drop the patch as it depends on patch 1/2.
Lets sort out which tree that patch is going through.
I have sent an email to try and sort that out.

Re: [PATCH v2 2/2] ARM: shmobile: Armadillo800EVA: Move st1232 reset pin handling

[Bastian Hecht]

2013/4/9 Simon Horman <horms@verge.net.au>:
> On Tue, Apr 09, 2013 at 11:16:18AM +0200, Bastian Hecht wrote:
>> 2013/4/9 Simon Horman <horms@verge.net.au>:
>> > On Tue, Apr 09, 2013 at 05:26:44PM +0900, Simon Horman wrote:
>> >> On Mon, Apr 08, 2013 at 04:17:22PM +0200, Laurent Pinchart wrote:
>> >> > Hi Bastian,
>> >> >
>> >> > Thanks for the patch.
>> >> >
>> >> > On Monday 08 April 2013 14:52:27 Bastian Hecht wrote:
>> >> > > We no longer need to set up the reset pin for the st1232 in the board
>> >> > > code, but can pass the GPIO number via the platform data to the driver.
>> >> > > This results in a cleaner grouping of the device setup.
>> >> > >
>> >> > > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
>> >> >
>> >> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> >>
>> >> Thanks.
>> >>
>> >> I have queued this up in the boards-armadillo800eva branch.
>> >>
>> >> There were some conflicts when I applied the code which
>> >> I fixed manually. Bastian, can you please check that the
>> >> following is correct? If it is not please send an incremental patch.
>> >
>> > I was trying to apply your patch to an old branch.
>> > It applies cleanly to the correct new branch :)
>>
>> Ah sweet, I was indeed a bit confused about going back to the INTC as
>> primary controller.
>
> After all that I had to drop the patch as it depends on patch 1/2.
> Lets sort out which tree that patch is going through.
> I have sent an email to try and sort that out.

Thanks Simon!

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Laurent Pinchart]

Hi Bastian,

Thanks for the patch.

On Monday 08 April 2013 14:52:26 Bastian Hecht wrote:
> We add the possiblity to hand over a GPIO number for the reset pin.
> This way we can remove existing board code that takes care of it and
> group this information properly in the platform data or in the device
> tree confguration.
> 
> The implementation is analogous to the cy8ctmg110 driver, thanks.
> 
> Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> v2:
> 
> - using a gpio phandle now in the DT code
> - rename from reset_pin to reset_gpio
> - gpio number 0 is allowed now and the absence of the reset gpio is marked
> with an error code
> - using devm_request_gpio instead of plain request_gpio to simplify things
> 
>  .../bindings/input/touchscreen/sitronix-st1232.txt |   24 ++++++++++++
>  drivers/input/touchscreen/st1232.c                 |   41 ++++++++++++++++-
>  include/linux/input/st1232_pdata.h                 |   13 +++++++
>  3 files changed, 76 insertions(+), 2 deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
> create mode 100644 include/linux/input/st1232_pdata.h
> 
> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
> b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
> new file mode 100644
> index 0000000..64ad48b
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
> @@ -0,0 +1,24 @@
> +* Sitronix st1232 touchscreen controller
> +
> +Required properties:
> +- compatible: must be "sitronix,st1232"
> +- reg: I2C address of the chip
> +- interrupts: interrupt to which the chip is connected
> +
> +Optional properties:
> +- gpios: a phandle to the reset GPIO
> +
> +Example:
> +
> +	i2c@00000000 {
> +		/* ... */
> +
> +		touchscreen@55 {
> +			compatible = "sitronix,st1232";
> +			reg = <0x55>;
> +			interrupts = <2 0>;
> +			gpios = <&gpio1 166 0>;
> +		};
> +
> +		/* ... */
> +	};
> diff --git a/drivers/input/touchscreen/st1232.c
> b/drivers/input/touchscreen/st1232.c index d9d05e2..498d18b 100644
> --- a/drivers/input/touchscreen/st1232.c
> +++ b/drivers/input/touchscreen/st1232.c
> @@ -19,13 +19,16 @@
>   */
> 
>  #include <linux/delay.h>
> +#include <linux/gpio.h>
>  #include <linux/i2c.h>
>  #include <linux/input.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
> +#include <linux/of_gpio.h>
>  #include <linux/pm_qos.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> +#include <linux/input/st1232_pdata.h>
> 
>  #define ST1232_TS_NAME	"st1232-ts"
> 
> @@ -48,6 +51,7 @@ struct st1232_ts_data {
>  	struct input_dev *input_dev;
>  	struct st1232_ts_finger finger[MAX_FINGERS];
>  	struct dev_pm_qos_request low_latency_req;
> +	int reset_gpio;
>  };
> 
>  static int st1232_ts_read_data(struct st1232_ts_data *ts)
> @@ -139,10 +143,17 @@ end:
>  	return IRQ_HANDLED;
>  }
> 
> +static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
> +{
> +	if (ts->reset_gpio >= 0)
> +		gpio_direction_output(ts->reset_gpio, poweron);
> +}
> +
>  static int st1232_ts_probe(struct i2c_client *client,
>  					const struct i2c_device_id *id)
>  {
>  	struct st1232_ts_data *ts;
> +	struct st1232_pdata *pdata = client->dev.platform_data;
>  	struct input_dev *input_dev;
>  	int error;
> 
> @@ -167,6 +178,25 @@ static int st1232_ts_probe(struct i2c_client *client,
>  	ts->client = client;
>  	ts->input_dev = input_dev;
> 
> +	if (client->dev.of_node)
> +		ts->reset_gpio = of_get_gpio(client->dev.of_node, 0);
> +	else if (pdata)
> +		ts->reset_gpio = pdata->reset_gpio;
> +	else
> +		ts->reset_gpio = -ENODEV;
> +
> +	if (ts->reset_gpio >= 0) {
> +		error = devm_gpio_request(&client->dev, ts->reset_gpio, NULL);
> +		if (error) {
> +			dev_err(&client->dev,
> +				"Unable to request GPIO pin %d.\n",
> +				ts->reset_gpio);
> +				goto err_free_mem;
> +		}
> +	}
> +
> +	st1232_ts_power(ts, true);
> +
>  	input_dev->name = "st1232-touchscreen";
>  	input_dev->id.bustype = BUS_I2C;
>  	input_dev->dev.parent = &client->dev;
> @@ -210,6 +240,7 @@ static int st1232_ts_remove(struct i2c_client *client)
>  {
>  	struct st1232_ts_data *ts = i2c_get_clientdata(client);
> 
> +	st1232_ts_power(ts, false);
>  	device_init_wakeup(&client->dev, 0);
>  	free_irq(client->irq, ts);
>  	input_unregister_device(ts->input_dev);
> @@ -222,11 +253,14 @@ static int st1232_ts_remove(struct i2c_client *client)
> static int st1232_ts_suspend(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> +	struct st1232_ts_data *ts = i2c_get_clientdata(client);
> 
>  	if (device_may_wakeup(&client->dev))
>  		enable_irq_wake(client->irq);
> -	else
> +	else {
>  		disable_irq(client->irq);
> +		st1232_ts_power(ts, false);
> +	}
> 
>  	return 0;
>  }
> @@ -234,11 +268,14 @@ static int st1232_ts_suspend(struct device *dev)
>  static int st1232_ts_resume(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> +	struct st1232_ts_data *ts = i2c_get_clientdata(client);
> 
>  	if (device_may_wakeup(&client->dev))
>  		disable_irq_wake(client->irq);
> -	else
> +	else {
> +		st1232_ts_power(ts, true);
>  		enable_irq(client->irq);
> +	}
> 
>  	return 0;
>  }
> diff --git a/include/linux/input/st1232_pdata.h
> b/include/linux/input/st1232_pdata.h new file mode 100644
> index 0000000..cac3e7b
> --- /dev/null
> +++ b/include/linux/input/st1232_pdata.h
> @@ -0,0 +1,13 @@
> +#ifndef _LINUX_ST1232_PDATA_H
> +#define _LINUX_ST1232_PDATA_H
> +
> +/*
> + * Optional platform data
> + *
> + * Use this if you want the driver to drive the reset pin.
> + */
> +struct st1232_pdata {
> +	int reset_gpio;
> +};
> +
> +#endif
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Simon Horman]

On Mon, Apr 08, 2013 at 04:17:20PM +0200, Laurent Pinchart wrote:
> Hi Bastian,
> 
> Thanks for the patch.
> 
> On Monday 08 April 2013 14:52:26 Bastian Hecht wrote:
> > We add the possiblity to hand over a GPIO number for the reset pin.
> > This way we can remove existing board code that takes care of it and
> > group this information properly in the platform data or in the device
> > tree confguration.
> > 
> > The implementation is analogous to the cy8ctmg110 driver, thanks.
> > 
> > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Bastian, I'm unclear on which tree this will go through.
Please let me know if it is mine.

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Bastian Hecht]

Hi Simon,

2013/4/9 Simon Horman <horms@verge.net.au>:
> On Mon, Apr 08, 2013 at 04:17:20PM +0200, Laurent Pinchart wrote:
>> Hi Bastian,
>>
>> Thanks for the patch.
>>
>> On Monday 08 April 2013 14:52:26 Bastian Hecht wrote:
>> > We add the possiblity to hand over a GPIO number for the reset pin.
>> > This way we can remove existing board code that takes care of it and
>> > group this information properly in the platform data or in the device
>> > tree confguration.
>> >
>> > The implementation is analogous to the cy8ctmg110 driver, thanks.
>> >
>> > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
>>
>> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Bastian, I'm unclear on which tree this will go through.
> Please let me know if it is mine.

To be honest, no idea. If it doesn't have to be a maintainer of
linux-input, I would be glad if you could take up that patch in your
tree, yes. And for your information, I don't rely on it until the pfc
DT version is merged. Then it would be nice to have it for the
touchscreen in the arma reference implementation.

Thanks,
 Bastian

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Simon Horman]

[ Cc Dimitry Torokhov ]

On Tue, Apr 09, 2013 at 11:23:51AM +0200, Bastian Hecht wrote:
> Hi Simon,
> 
> 2013/4/9 Simon Horman <horms@verge.net.au>:
> > On Mon, Apr 08, 2013 at 04:17:20PM +0200, Laurent Pinchart wrote:
> >> Hi Bastian,
> >>
> >> Thanks for the patch.
> >>
> >> On Monday 08 April 2013 14:52:26 Bastian Hecht wrote:
> >> > We add the possiblity to hand over a GPIO number for the reset pin.
> >> > This way we can remove existing board code that takes care of it and
> >> > group this information properly in the platform data or in the device
> >> > tree confguration.
> >> >
> >> > The implementation is analogous to the cy8ctmg110 driver, thanks.
> >> >
> >> > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> >>
> >> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Bastian, I'm unclear on which tree this will go through.
> > Please let me know if it is mine.
> 
> To be honest, no idea. If it doesn't have to be a maintainer of
> linux-input, I would be glad if you could take up that patch in your
> tree, yes. And for your information, I don't rely on it until the pfc
> DT version is merged. Then it would be nice to have it for the
> touchscreen in the arma reference implementation.

Dimitry,

you seem to have signed-off on most of the st1232 I see in the git history.
Could you take this patch?

There is an accompanying patch for the Armaillo800EVA which depends on this
change, so it would be good if you could take that too.

Alternatively I can take both patches through my renesas tree if
that is better for you.

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Dmitry Torokhov]

On Wed, Apr 10, 2013 at 12:01:06AM +0900, Simon Horman wrote:
> [ Cc Dimitry Torokhov ]
> 
> On Tue, Apr 09, 2013 at 11:23:51AM +0200, Bastian Hecht wrote:
> > Hi Simon,
> > 
> > 2013/4/9 Simon Horman <horms@verge.net.au>:
> > > On Mon, Apr 08, 2013 at 04:17:20PM +0200, Laurent Pinchart wrote:
> > >> Hi Bastian,
> > >>
> > >> Thanks for the patch.
> > >>
> > >> On Monday 08 April 2013 14:52:26 Bastian Hecht wrote:
> > >> > We add the possiblity to hand over a GPIO number for the reset pin.
> > >> > This way we can remove existing board code that takes care of it and
> > >> > group this information properly in the platform data or in the device
> > >> > tree confguration.
> > >> >
> > >> > The implementation is analogous to the cy8ctmg110 driver, thanks.
> > >> >
> > >> > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> > >>
> > >> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > Bastian, I'm unclear on which tree this will go through.
> > > Please let me know if it is mine.
> > 
> > To be honest, no idea. If it doesn't have to be a maintainer of
> > linux-input, I would be glad if you could take up that patch in your
> > tree, yes. And for your information, I don't rely on it until the pfc
> > DT version is merged. Then it would be nice to have it for the
> > touchscreen in the arma reference implementation.
> 
> Dimitry,
> 
> you seem to have signed-off on most of the st1232 I see in the git history.
> Could you take this patch?

Yes, I will, once feedback is addressed.

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Simon Horman]

On Tue, Apr 09, 2013 at 11:51:57PM -0700, Dmitry Torokhov wrote:
> On Wed, Apr 10, 2013 at 12:01:06AM +0900, Simon Horman wrote:
> > [ Cc Dimitry Torokhov ]
> > 
> > On Tue, Apr 09, 2013 at 11:23:51AM +0200, Bastian Hecht wrote:
> > > Hi Simon,
> > > 
> > > 2013/4/9 Simon Horman <horms@verge.net.au>:
> > > > On Mon, Apr 08, 2013 at 04:17:20PM +0200, Laurent Pinchart wrote:
> > > >> Hi Bastian,
> > > >>
> > > >> Thanks for the patch.
> > > >>
> > > >> On Monday 08 April 2013 14:52:26 Bastian Hecht wrote:
> > > >> > We add the possiblity to hand over a GPIO number for the reset pin.
> > > >> > This way we can remove existing board code that takes care of it and
> > > >> > group this information properly in the platform data or in the device
> > > >> > tree confguration.
> > > >> >
> > > >> > The implementation is analogous to the cy8ctmg110 driver, thanks.
> > > >> >
> > > >> > Signed-off-by: Bastian Hecht <hechtb+renesas@gmail.com>
> > > >>
> > > >> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > Bastian, I'm unclear on which tree this will go through.
> > > > Please let me know if it is mine.
> > > 
> > > To be honest, no idea. If it doesn't have to be a maintainer of
> > > linux-input, I would be glad if you could take up that patch in your
> > > tree, yes. And for your information, I don't rely on it until the pfc
> > > DT version is merged. Then it would be nice to have it for the
> > > touchscreen in the arma reference implementation.
> > 
> > Dimitry,
> > 
> > you seem to have signed-off on most of the st1232 I see in the git history.
> > Could you take this patch?
> 
> Yes, I will, once feedback is addressed.

Great, thanks.
Please let me know if you need anything from me.

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Dmitry Torokhov]

Hi Bastian,

On Mon, Apr 08, 2013 at 02:52:26PM +0200, Bastian Hecht wrote:
> +static void st1232_ts_power(struct st1232_ts_data *ts, bool poweron)
> +{
> +	if (ts->reset_gpio >= 0)

This should be gpio_is_valid() I think.

> +		gpio_direction_output(ts->reset_gpio, poweron);
> +}
> +
>  static int st1232_ts_probe(struct i2c_client *client,
>  					const struct i2c_device_id *id)
>  {
>  	struct st1232_ts_data *ts;
> +	struct st1232_pdata *pdata = client->dev.platform_data;
>  	struct input_dev *input_dev;
>  	int error;
>  
> @@ -167,6 +178,25 @@ static int st1232_ts_probe(struct i2c_client *client,
>  	ts->client = client;
>  	ts->input_dev = input_dev;
>  
> +	if (client->dev.of_node)
> +		ts->reset_gpio = of_get_gpio(client->dev.of_node, 0);
> +	else if (pdata)
> +		ts->reset_gpio = pdata->reset_gpio;

I believe we should favor platform data, then firmware, so that you can
override if necessary.

> +	else
> +		ts->reset_gpio = -ENODEV;
> +
> +	if (ts->reset_gpio >= 0) {

gpio_is_valid() again?

> +		error = devm_gpio_request(&client->dev, ts->reset_gpio, NULL);

Frankly I do not like mixing managed and noon-managed resources. Maybe
the entire driver needs to be converted to managed resources?

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Laurent Pinchart]

Hi Dmitry,

On Tuesday 09 April 2013 23:51:14 Dmitry Torokhov wrote:
> On Mon, Apr 08, 2013 at 02:52:26PM +0200, Bastian Hecht wrote:

[snip]

> > +		error = devm_gpio_request(&client->dev, ts->reset_gpio, NULL);
> 
> Frankly I do not like mixing managed and noon-managed resources. Maybe
> the entire driver needs to be converted to managed resources?

I've just sent a patch to do so.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/2] input: st1232: Add reset pin handling

[Bastian Hecht]

Hi all,

2013/4/10 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> Hi Dmitry,
>
> On Tuesday 09 April 2013 23:51:14 Dmitry Torokhov wrote:
>> On Mon, Apr 08, 2013 at 02:52:26PM +0200, Bastian Hecht wrote:
>
> [snip]
>
>> > +           error = devm_gpio_request(&client->dev, ts->reset_gpio, NULL);
>>
>> Frankly I do not like mixing managed and noon-managed resources. Maybe
>> the entire driver needs to be converted to managed resources?
>
> I've just sent a patch to do so.

Thanks Laurent! I've tested it - works fine.

Dimitri, I've addressed your suggestions and use gpio_valid and have
reordered the probing for platform and OF data. Thanks for your
feedback. I've based my changes on top of it and will post it shortly.

Cheers,

 Bastian