[PATCH 0/4] Input: auo_pixcir_ts - add devicetree support

[PATCH 0/4] Input: auo_pixcir_ts - add devicetree support

[Heiko Stübner]

Some small cleanups needed and the parsing of the dt data.

Heiko Stuebner (4):
  Input: auo-pixcir-ts - set input direction for interrupt gpio
  Input: auo-pixcir-ts - handle reset gpio directly
  Input: auo_pixcir_ts - keep own pointer to platform_data
  Input: auo_pixcir_ts - add devicetree support

 .../bindings/input/touchscreen/auo_pixcir_ts.txt   |   30 ++++++
 drivers/input/touchscreen/auo-pixcir-ts.c          |  104 +++++++++++++++++--
 include/linux/input/auo-pixcir-ts.h                |    4 +-
 3 files changed, 124 insertions(+), 14 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt

-- 
1.7.2.3

[PATCH 1/4] Input: auo-pixcir-ts - set input direction for interrupt gpio

[Heiko Stübner]

Previously the gpio was not configured at all.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 drivers/input/touchscreen/auo-pixcir-ts.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/auo-pixcir-ts.c b/drivers/input/touchscreen/auo-pixcir-ts.c
index c6e19a9..813413e 100644
--- a/drivers/input/touchscreen/auo-pixcir-ts.c
+++ b/drivers/input/touchscreen/auo-pixcir-ts.c
@@ -504,6 +504,13 @@ static int auo_pixcir_probe(struct i2c_client *client,
 		goto err_gpio_int;
 	}
 
+	ret = gpio_direction_input(pdata->gpio_int);
+	if (ret) {
+		dev_err(&client->dev, "setting direction of gpio %d failed %d\n",
+			pdata->gpio_int, ret);
+		goto err_gpio_dir;
+	}
+
 	if (pdata->init_hw)
 		pdata->init_hw(client);
 
@@ -592,6 +599,7 @@ err_fw_vers:
 err_input_alloc:
 	if (pdata->exit_hw)
 		pdata->exit_hw(client);
+err_gpio_dir:
 	gpio_free(pdata->gpio_int);
 err_gpio_int:
 	kfree(ts);
-- 
1.7.2.3

[PATCH 2/4] Input: auo-pixcir-ts - handle reset gpio directly

[Heiko Stübner]

Devicetree based platforms don't handle device callbacks very well
and until now no board has come along that needs more extended hwinit
than pulling the rst gpio high.

Therefore pull the reset handling directly into the driver and remove
the callbacks from the driver.

If extended device setup is needed at some later point, power-sequences
would probably be the solution of choice.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 drivers/input/touchscreen/auo-pixcir-ts.c |   26 ++++++++++++++++++++------
 include/linux/input/auo-pixcir-ts.h       |    4 +---
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/auo-pixcir-ts.c b/drivers/input/touchscreen/auo-pixcir-ts.c
index 813413e..6317a9c 100644
--- a/drivers/input/touchscreen/auo-pixcir-ts.c
+++ b/drivers/input/touchscreen/auo-pixcir-ts.c
@@ -511,8 +511,21 @@ static int auo_pixcir_probe(struct i2c_client *client,
 		goto err_gpio_dir;
 	}
 
-	if (pdata->init_hw)
-		pdata->init_hw(client);
+	ret = gpio_request(pdata->gpio_rst, "auo_pixcir_ts_rst");
+	if (ret) {
+		dev_err(&client->dev, "request of gpio %d failed, %d\n",
+			pdata->gpio_rst, ret);
+		goto err_gpio_dir;
+	}
+
+	ret = gpio_direction_output(pdata->gpio_rst, 1);
+	if (ret) {
+		dev_err(&client->dev, "setting direction of gpio %d failed %d\n",
+			pdata->gpio_rst, ret);
+		goto err_gpio_rst;
+	}
+
+	msleep(200);
 
 	ts->client = client;
 	ts->touch_ind_mode = 0;
@@ -597,8 +610,9 @@ err_input_register:
 err_fw_vers:
 	input_free_device(input_dev);
 err_input_alloc:
-	if (pdata->exit_hw)
-		pdata->exit_hw(client);
+	gpio_set_value(pdata->gpio_rst, 0);
+err_gpio_rst:
+	gpio_free(pdata->gpio_rst);
 err_gpio_dir:
 	gpio_free(pdata->gpio_int);
 err_gpio_int:
@@ -616,8 +630,8 @@ static int auo_pixcir_remove(struct i2c_client *client)
 
 	input_unregister_device(ts->input);
 
-	if (pdata->exit_hw)
-		pdata->exit_hw(client);
+	gpio_set_value(pdata->gpio_rst, 0);
+	gpio_free(pdata->gpio_rst);
 
 	gpio_free(pdata->gpio_int);
 
diff --git a/include/linux/input/auo-pixcir-ts.h b/include/linux/input/auo-pixcir-ts.h
index 75d4be7..5049f21 100644
--- a/include/linux/input/auo-pixcir-ts.h
+++ b/include/linux/input/auo-pixcir-ts.h
@@ -43,12 +43,10 @@
  */
 struct auo_pixcir_ts_platdata {
 	int gpio_int;
+	int gpio_rst;
 
 	int int_setting;
 
-	void (*init_hw)(struct i2c_client *);
-	void (*exit_hw)(struct i2c_client *);
-
 	unsigned int x_max;
 	unsigned int y_max;
 };
-- 
1.7.2.3

[PATCH 3/4] Input: auo_pixcir_ts - keep own pointer to platform_data

[Heiko Stübner]

When supporting devicetree the platformdata may not necessarily come
from the dev but may be generated in the driver instead.

Therefore keep the pointer in the driver struct instead of using
dev.platform_data.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 drivers/input/touchscreen/auo-pixcir-ts.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/auo-pixcir-ts.c b/drivers/input/touchscreen/auo-pixcir-ts.c
index 6317a9c..c0a2483 100644
--- a/drivers/input/touchscreen/auo-pixcir-ts.c
+++ b/drivers/input/touchscreen/auo-pixcir-ts.c
@@ -111,6 +111,7 @@
 struct auo_pixcir_ts {
 	struct i2c_client	*client;
 	struct input_dev	*input;
+	const struct auo_pixcir_ts_platdata *pdata;
 	char			phys[32];
 
 	/* special handling for touch_indicate interupt mode */
@@ -132,7 +133,7 @@ static int auo_pixcir_collect_data(struct auo_pixcir_ts *ts,
 				   struct auo_point_t *point)
 {
 	struct i2c_client *client = ts->client;
-	const struct auo_pixcir_ts_platdata *pdata = client->dev.platform_data;
+	const struct auo_pixcir_ts_platdata *pdata = ts->pdata;
 	uint8_t raw_coord[8];
 	uint8_t raw_area[4];
 	int i, ret;
@@ -178,8 +179,7 @@ static int auo_pixcir_collect_data(struct auo_pixcir_ts *ts,
 static irqreturn_t auo_pixcir_interrupt(int irq, void *dev_id)
 {
 	struct auo_pixcir_ts *ts = dev_id;
-	struct i2c_client *client = ts->client;
-	const struct auo_pixcir_ts_platdata *pdata = client->dev.platform_data;
+	const struct auo_pixcir_ts_platdata *pdata = ts->pdata;
 	struct auo_point_t point[AUO_PIXCIR_REPORT_POINTS];
 	int i;
 	int ret;
@@ -290,7 +290,7 @@ static int auo_pixcir_int_config(struct auo_pixcir_ts *ts,
 					   int int_setting)
 {
 	struct i2c_client *client = ts->client;
-	struct auo_pixcir_ts_platdata *pdata = client->dev.platform_data;
+	const struct auo_pixcir_ts_platdata *pdata = ts->pdata;
 	int ret;
 
 	ret = i2c_smbus_read_byte_data(client, AUO_PIXCIR_REG_INT_SETTING);
@@ -527,6 +527,7 @@ static int auo_pixcir_probe(struct i2c_client *client,
 
 	msleep(200);
 
+	ts->pdata = pdata;
 	ts->client = client;
 	ts->touch_ind_mode = 0;
 	init_waitqueue_head(&ts->wait);
@@ -624,7 +625,7 @@ err_gpio_int:
 static int auo_pixcir_remove(struct i2c_client *client)
 {
 	struct auo_pixcir_ts *ts = i2c_get_clientdata(client);
-	const struct auo_pixcir_ts_platdata *pdata = client->dev.platform_data;
+	const struct auo_pixcir_ts_platdata *pdata = ts->pdata;
 
 	free_irq(client->irq, ts);
 
-- 
1.7.2.3

[PATCH 4/4] Input: auo_pixcir_ts - add devicetree support

[Heiko Stübner]

Add the necessary code to create the needed platformdata from devicetree
informations.

The interrupt mode of the chip is not set via devicetree, as it is not
a property of the hardware but instead only a preferred type of operation.
This should probably be made settable via configfs in the future.
The option set as default is the mode the datasheet mentions as default.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 .../bindings/input/touchscreen/auo_pixcir_ts.txt   |   30 ++++++++++
 drivers/input/touchscreen/auo-pixcir-ts.c          |   59 ++++++++++++++++++++
 2 files changed, 89 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt
new file mode 100644
index 0000000..f40f21c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt
@@ -0,0 +1,30 @@
+* AUO in-cell touchscreen controller using Pixcir sensors
+
+Required properties:
+- compatible: must be "auo,auo_pixcir_ts"
+- reg: I2C address of the chip
+- interrupts: interrupt to which the chip is connected
+- gpios: gpios the chip is connected to
+  first one is the interrupt gpio and second one the reset gpio
+- x-size: horizontal resolution of touchscreen
+- y-size: vertical resolution of touchscreen
+
+Example:
+
+	i2c@00000000 {
+		/* ... */
+
+		auo_pixcir_ts@5c {
+			compatible = "auo,auo_pixcir_ts";
+			reg = <0x5c>;
+			interrupts = <2 0>;
+
+			gpios = <&gpf 2 0 2>, /* INT */
+				<&gpf 5 1 0>; /* RST */
+
+			x-size = <800>;
+			y-size = <600>;
+		};
+
+		/* ... */
+	};
diff --git a/drivers/input/touchscreen/auo-pixcir-ts.c b/drivers/input/touchscreen/auo-pixcir-ts.c
index c0a2483..6d1f8c1 100644
--- a/drivers/input/touchscreen/auo-pixcir-ts.c
+++ b/drivers/input/touchscreen/auo-pixcir-ts.c
@@ -31,6 +31,8 @@
 #include <linux/delay.h>
 #include <linux/gpio.h>
 #include <linux/input/auo-pixcir-ts.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 /*
  * Coordinate calculation:
@@ -479,6 +481,51 @@ unlock:
 }
 #endif
 
+static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct device *dev)
+{
+	struct auo_pixcir_ts_platdata *pdata = NULL;
+
+#ifdef CONFIG_OF
+	struct device_node *np = dev->of_node;
+
+	if (!np)
+		return NULL;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "failed to allocate platform data\n");
+		return NULL;
+	}
+
+	pdata->gpio_int = of_get_gpio(np, 0);
+	if (!gpio_is_valid(pdata->gpio_int)) {
+		dev_err(dev, "failed to get interrupt gpio\n");
+		return NULL;
+	}
+
+	pdata->gpio_rst = of_get_gpio(np, 1);
+	if (!gpio_is_valid(pdata->gpio_rst)) {
+		dev_err(dev, "failed to get reset gpio\n");
+		return NULL;
+	}
+
+	if (of_property_read_u32(np, "x-size", &pdata->x_max)) {
+		dev_err(dev, "failed to get x-size property\n");
+		return NULL;
+	};
+
+	if (of_property_read_u32(np, "y-size", &pdata->y_max)) {
+		dev_err(dev, "failed to get y-size property\n");
+		return NULL;
+	};
+
+	/* default to asserting the interrupt when the screen is touched */
+	pdata->int_setting = AUO_PIXCIR_INT_TOUCH_IND;
+#endif
+
+	return pdata;
+}
+
 static SIMPLE_DEV_PM_OPS(auo_pixcir_pm_ops, auo_pixcir_suspend,
 			 auo_pixcir_resume);
 
@@ -491,6 +538,9 @@ static int auo_pixcir_probe(struct i2c_client *client,
 	int ret;
 
 	if (!pdata)
+		pdata = auo_pixcir_parse_dt(&client->dev);
+
+	if (!pdata)
 		return -EINVAL;
 
 	ts = kzalloc(sizeof(struct auo_pixcir_ts), GFP_KERNEL);
@@ -647,11 +697,20 @@ static const struct i2c_device_id auo_pixcir_idtable[] = {
 };
 MODULE_DEVICE_TABLE(i2c, auo_pixcir_idtable);
 
+#ifdef CONFIG_OF
+static struct of_device_id auo_pixcir_ts_dt_idtable[] = {
+	{ .compatible = "auo,auo_pixcir_ts" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, auo_pixcir_ts_dt_idtable);
+#endif
+
 static struct i2c_driver auo_pixcir_driver = {
 	.driver = {
 		.owner	= THIS_MODULE,
 		.name	= "auo_pixcir_ts",
 		.pm	= &auo_pixcir_pm_ops,
+		.of_match_table	= of_match_ptr(auo_pixcir_ts_dt_idtable),
 	},
 	.probe		= auo_pixcir_probe,
 	.remove		= auo_pixcir_remove,
-- 
1.7.2.3

Re: [PATCH 4/4] Input: auo_pixcir_ts - add devicetree support

[Dmitry Torokhov]

Hi Heiko,

On Sat, Feb 23, 2013 at 12:58:16PM +0100, Heiko Stübner wrote:
> +static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct device *dev)
> +{
> +	struct auo_pixcir_ts_platdata *pdata = NULL;
> +
> +#ifdef CONFIG_OF
> +	struct device_node *np = dev->of_node;
> +
> +	if (!np)
> +		return NULL;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata) {
> +		dev_err(dev, "failed to allocate platform data\n");
> +		return NULL;
> +	}

I disike #ifdefs in the middle of the code, also it would be nice if we
signal the proper error instead of always using -EINVAL when there are
issues with platform/DT data.

How about the version of the patch below?

Thanks.
-- 
Dmitry

Input: auo_pixcir_ts - add devicetree support

From: Heiko Stübner <heiko@sntech.de>

Add the necessary code to create the needed platformdata from devicetree
informations.

The interrupt mode of the chip is not set via devicetree, as it is not
a property of the hardware but instead only a preferred type of operation.
This should probably be made settable via configfs in the future.
The option set as default is the mode the datasheet mentions as default.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 .../bindings/input/touchscreen/auo_pixcir_ts.txt   |   30 ++++++++
 drivers/input/touchscreen/auo-pixcir-ts.c          |   74 +++++++++++++++++++-
 2 files changed, 99 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt
new file mode 100644
index 0000000..f40f21c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/auo_pixcir_ts.txt
@@ -0,0 +1,30 @@
+* AUO in-cell touchscreen controller using Pixcir sensors
+
+Required properties:
+- compatible: must be "auo,auo_pixcir_ts"
+- reg: I2C address of the chip
+- interrupts: interrupt to which the chip is connected
+- gpios: gpios the chip is connected to
+  first one is the interrupt gpio and second one the reset gpio
+- x-size: horizontal resolution of touchscreen
+- y-size: vertical resolution of touchscreen
+
+Example:
+
+	i2c@00000000 {
+		/* ... */
+
+		auo_pixcir_ts@5c {
+			compatible = "auo,auo_pixcir_ts";
+			reg = <0x5c>;
+			interrupts = <2 0>;
+
+			gpios = <&gpf 2 0 2>, /* INT */
+				<&gpf 5 1 0>; /* RST */
+
+			x-size = <800>;
+			y-size = <600>;
+		};
+
+		/* ... */
+	};
diff --git a/drivers/input/touchscreen/auo-pixcir-ts.c b/drivers/input/touchscreen/auo-pixcir-ts.c
index c0a2483..dfa6d54 100644
--- a/drivers/input/touchscreen/auo-pixcir-ts.c
+++ b/drivers/input/touchscreen/auo-pixcir-ts.c
@@ -31,6 +31,8 @@
 #include <linux/delay.h>
 #include <linux/gpio.h>
 #include <linux/input/auo-pixcir-ts.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 /*
  * Coordinate calculation:
@@ -479,19 +481,72 @@ unlock:
 }
 #endif
 
-static SIMPLE_DEV_PM_OPS(auo_pixcir_pm_ops, auo_pixcir_suspend,
-			 auo_pixcir_resume);
+static SIMPLE_DEV_PM_OPS(auo_pixcir_pm_ops,
+			 auo_pixcir_suspend, auo_pixcir_resume);
+
+#ifdef CONFIG_OF
+static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct device *dev)
+{
+	struct auo_pixcir_ts_platdata *pdata;
+	struct device_node *np = dev->of_node;
+
+	if (!np)
+		return ERR_PTR(-ENOENT);
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "failed to allocate platform data\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	pdata->gpio_int = of_get_gpio(np, 0);
+	if (!gpio_is_valid(pdata->gpio_int)) {
+		dev_err(dev, "failed to get interrupt gpio\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata->gpio_rst = of_get_gpio(np, 1);
+	if (!gpio_is_valid(pdata->gpio_rst)) {
+		dev_err(dev, "failed to get reset gpio\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32(np, "x-size", &pdata->x_max)) {
+		dev_err(dev, "failed to get x-size property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32(np, "y-size", &pdata->y_max)) {
+		dev_err(dev, "failed to get y-size property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* default to asserting the interrupt when the screen is touched */
+	pdata->int_setting = AUO_PIXCIR_INT_TOUCH_IND;
+
+	return pdata;
+}
+#else
+static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct device *dev)
+{
+	return ERR_PTR(-EINVAL);
+}
+#endif
 
 static int auo_pixcir_probe(struct i2c_client *client,
 				      const struct i2c_device_id *id)
 {
-	const struct auo_pixcir_ts_platdata *pdata = client->dev.platform_data;
+	const struct auo_pixcir_ts_platdata *pdata;
 	struct auo_pixcir_ts *ts;
 	struct input_dev *input_dev;
 	int ret;
 
-	if (!pdata)
-		return -EINVAL;
+	pdata = dev_get_platdata(&client->dev);
+	if (!pdata) {
+		pdata = auo_pixcir_parse_dt(&client->dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
 
 	ts = kzalloc(sizeof(struct auo_pixcir_ts), GFP_KERNEL);
 	if (!ts)
@@ -647,11 +702,20 @@ static const struct i2c_device_id auo_pixcir_idtable[] = {
 };
 MODULE_DEVICE_TABLE(i2c, auo_pixcir_idtable);
 
+#ifdef CONFIG_OF
+static struct of_device_id auo_pixcir_ts_dt_idtable[] = {
+	{ .compatible = "auo,auo_pixcir_ts" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, auo_pixcir_ts_dt_idtable);
+#endif
+
 static struct i2c_driver auo_pixcir_driver = {
 	.driver = {
 		.owner	= THIS_MODULE,
 		.name	= "auo_pixcir_ts",
 		.pm	= &auo_pixcir_pm_ops,
+		.of_match_table	= of_match_ptr(auo_pixcir_ts_dt_idtable),
 	},
 	.probe		= auo_pixcir_probe,
 	.remove		= auo_pixcir_remove,
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] Input: auo_pixcir_ts - add devicetree support

[Heiko Stübner]

Hi Dmitry,

Am Sonntag, 24. Februar 2013, 09:00:15 schrieb Dmitry Torokhov:
> Hi Heiko,
> 
> On Sat, Feb 23, 2013 at 12:58:16PM +0100, Heiko Stübner wrote:
> > +static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct device
> > *dev) +{
> > +	struct auo_pixcir_ts_platdata *pdata = NULL;
> > +
> > +#ifdef CONFIG_OF
> > +	struct device_node *np = dev->of_node;
> > +
> > +	if (!np)
> > +		return NULL;
> > +
> > +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> > +	if (!pdata) {
> > +		dev_err(dev, "failed to allocate platform data\n");
> > +		return NULL;
> > +	}
> 
> I disike #ifdefs in the middle of the code, also it would be nice if we
> signal the proper error instead of always using -EINVAL when there are
> issues with platform/DT data.
> 
> How about the version of the patch below?

I tested it and everything of course still works :-) .

I have no real preference for one way or the other. I had seen both types in 
different drivers and could not really decide which I like better, so I'm fine 
with using your variant instead.


Thanks
Heiko

Re: [PATCH 4/4] Input: auo_pixcir_ts - add devicetree support

[Dmitry Torokhov]

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

On Sun, Feb 24, 2013 at 10:58:00AM +0100, Heiko Stübner wrote:
> Hi Dmitry,
> 
> Am Sonntag, 24. Februar 2013, 09:00:15 schrieb Dmitry Torokhov:
> > Hi Heiko,
> > 
> > On Sat, Feb 23, 2013 at 12:58:16PM +0100, Heiko Stübner wrote:
> > > +static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct device
> > > *dev) +{
> > > +	struct auo_pixcir_ts_platdata *pdata = NULL;
> > > +
> > > +#ifdef CONFIG_OF
> > > +	struct device_node *np = dev->of_node;
> > > +
> > > +	if (!np)
> > > +		return NULL;
> > > +
> > > +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> > > +	if (!pdata) {
> > > +		dev_err(dev, "failed to allocate platform data\n");
> > > +		return NULL;
> > > +	}
> > 
> > I disike #ifdefs in the middle of the code, also it would be nice if we
> > signal the proper error instead of always using -EINVAL when there are
> > issues with platform/DT data.
> > 
> > How about the version of the patch below?
> 
> I tested it and everything of course still works :-) .

OK, great, then I will queue these for the next merge window.

Could you also try this patch (it however needs attached patch enhancing
devres to support custom actions).

Thanks.

-- 
Dmitry

Input: auo-pixer-ts - switch to using managed resources

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

This simplifier error unwinding and device teardown.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/auo-pixcir-ts.c |  162 ++++++++++++-----------------
 1 file changed, 68 insertions(+), 94 deletions(-)

diff --git a/drivers/input/touchscreen/auo-pixcir-ts.c b/drivers/input/touchscreen/auo-pixcir-ts.c
index dfa6d54..f4ba6ffa 100644
--- a/drivers/input/touchscreen/auo-pixcir-ts.c
+++ b/drivers/input/touchscreen/auo-pixcir-ts.c
@@ -533,13 +533,21 @@ static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct device *dev)
 }
 #endif
 
+static void auo_pixcir_reset(void *data)
+{
+	struct auo_pixcir_ts *ts = data;
+
+	gpio_set_value(ts->pdata->gpio_rst, 0);
+}
+
 static int auo_pixcir_probe(struct i2c_client *client,
-				      const struct i2c_device_id *id)
+			    const struct i2c_device_id *id)
 {
 	const struct auo_pixcir_ts_platdata *pdata;
 	struct auo_pixcir_ts *ts;
 	struct input_dev *input_dev;
-	int ret;
+	int version;
+	int error;
 
 	pdata = dev_get_platdata(&client->dev);
 	if (!pdata) {
@@ -548,60 +556,30 @@ static int auo_pixcir_probe(struct i2c_client *client,
 			return PTR_ERR(pdata);
 	}
 
-	ts = kzalloc(sizeof(struct auo_pixcir_ts), GFP_KERNEL);
+	ts = devm_kzalloc(&client->dev,
+			  sizeof(struct auo_pixcir_ts), GFP_KERNEL);
 	if (!ts)
 		return -ENOMEM;
 
-	ret = gpio_request(pdata->gpio_int, "auo_pixcir_ts_int");
-	if (ret) {
-		dev_err(&client->dev, "request of gpio %d failed, %d\n",
-			pdata->gpio_int, ret);
-		goto err_gpio_int;
-	}
-
-	ret = gpio_direction_input(pdata->gpio_int);
-	if (ret) {
-		dev_err(&client->dev, "setting direction of gpio %d failed %d\n",
-			pdata->gpio_int, ret);
-		goto err_gpio_dir;
-	}
-
-	ret = gpio_request(pdata->gpio_rst, "auo_pixcir_ts_rst");
-	if (ret) {
-		dev_err(&client->dev, "request of gpio %d failed, %d\n",
-			pdata->gpio_rst, ret);
-		goto err_gpio_dir;
-	}
-
-	ret = gpio_direction_output(pdata->gpio_rst, 1);
-	if (ret) {
-		dev_err(&client->dev, "setting direction of gpio %d failed %d\n",
-			pdata->gpio_rst, ret);
-		goto err_gpio_rst;
+	input_dev = devm_input_allocate_device(&client->dev);
+	if (!input_dev) {
+		dev_err(&client->dev, "could not allocate input device\n");
+		return -ENOMEM;
 	}
 
-	msleep(200);
-
 	ts->pdata = pdata;
 	ts->client = client;
+	ts->input = input_dev;
 	ts->touch_ind_mode = 0;
+	ts->stopped = true;
 	init_waitqueue_head(&ts->wait);
 
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));
 
-	input_dev = input_allocate_device();
-	if (!input_dev) {
-		dev_err(&client->dev, "could not allocate input device\n");
-		goto err_input_alloc;
-	}
-
-	ts->input = input_dev;
-
 	input_dev->name = "AUO-Pixcir touchscreen";
 	input_dev->phys = ts->phys;
 	input_dev->id.bustype = BUS_I2C;
-	input_dev->dev.parent = &client->dev;
 
 	input_dev->open = auo_pixcir_input_open;
 	input_dev->close = auo_pixcir_input_close;
@@ -626,72 +604,69 @@ static int auo_pixcir_probe(struct i2c_client *client,
 			     AUO_PIXCIR_MAX_AREA, 0, 0);
 	input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);
 
-	ret = i2c_smbus_read_byte_data(client, AUO_PIXCIR_REG_VERSION);
-	if (ret < 0)
-		goto err_fw_vers;
-	dev_info(&client->dev, "firmware version 0x%X\n", ret);
-
-	ret = auo_pixcir_int_config(ts, pdata->int_setting);
-	if (ret)
-		goto err_fw_vers;
-
 	input_set_drvdata(ts->input, ts);
-	ts->stopped = true;
 
-	ret = request_threaded_irq(client->irq, NULL, auo_pixcir_interrupt,
-				   IRQF_TRIGGER_RISING | IRQF_ONESHOT,
-				   input_dev->name, ts);
-	if (ret) {
-		dev_err(&client->dev, "irq %d requested failed\n", client->irq);
-		goto err_fw_vers;
+	error = devm_gpio_request_one(&client->dev, pdata->gpio_int,
+				      GPIOF_DIR_IN, "auo_pixcir_ts_int");
+	if (error) {
+		dev_err(&client->dev, "request of gpio %d failed, %d\n",
+			pdata->gpio_int, error);
+		return error;
 	}
 
-	/* stop device and put it into deep sleep until it is opened */
-	ret = auo_pixcir_stop(ts);
-	if (ret < 0)
-		goto err_input_register;
-
-	ret = input_register_device(input_dev);
-	if (ret) {
-		dev_err(&client->dev, "could not register input device\n");
-		goto err_input_register;
+	error = devm_gpio_request_one(&client->dev, pdata->gpio_rst,
+				      GPIOF_DIR_OUT | GPIOF_INIT_HIGH,
+				      "auo_pixcir_ts_int");
+	if (error) {
+		dev_err(&client->dev, "request of gpio %d failed, %d\n",
+			pdata->gpio_rst, error);
+		return error;
 	}
 
-	i2c_set_clientdata(client, ts);
-
-	return 0;
-
-err_input_register:
-	free_irq(client->irq, ts);
-err_fw_vers:
-	input_free_device(input_dev);
-err_input_alloc:
-	gpio_set_value(pdata->gpio_rst, 0);
-err_gpio_rst:
-	gpio_free(pdata->gpio_rst);
-err_gpio_dir:
-	gpio_free(pdata->gpio_int);
-err_gpio_int:
-	kfree(ts);
+	error = devm_add_action(&client->dev, auo_pixcir_reset, ts);
+	if (error) {
+		auo_pixcir_reset(ts);
+		dev_err(&client->dev, "failed to register xxx action, %d\n",
+			error);
+		return error;
+	}
 
-	return ret;
-}
+	msleep(200);
 
-static int auo_pixcir_remove(struct i2c_client *client)
-{
-	struct auo_pixcir_ts *ts = i2c_get_clientdata(client);
-	const struct auo_pixcir_ts_platdata *pdata = ts->pdata;
+	version = i2c_smbus_read_byte_data(client, AUO_PIXCIR_REG_VERSION);
+	if (version < 0) {
+		error = version;
+		return error;
+	}
 
-	free_irq(client->irq, ts);
+	dev_info(&client->dev, "firmware version 0x%X\n", version);
 
-	input_unregister_device(ts->input);
+	error = auo_pixcir_int_config(ts, pdata->int_setting);
+	if (error)
+		return error;
 
-	gpio_set_value(pdata->gpio_rst, 0);
-	gpio_free(pdata->gpio_rst);
+	error = request_threaded_irq(client->irq, NULL, auo_pixcir_interrupt,
+				     IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+				     input_dev->name, ts);
+	if (error) {
+		dev_err(&client->dev, "irq %d requested failed, %d\n",
+			client->irq, error);
+		return error;
+	}
 
-	gpio_free(pdata->gpio_int);
+	/* stop device and put it into deep sleep until it is opened */
+	error = auo_pixcir_stop(ts);
+	if (error)
+		return error;
+
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(&client->dev, "could not register input device, %d\n",
+			error);
+		return error;
+	}
 
-	kfree(ts);
+	i2c_set_clientdata(client, ts);
 
 	return 0;
 }
@@ -718,7 +693,6 @@ static struct i2c_driver auo_pixcir_driver = {
 		.of_match_table	= of_match_ptr(auo_pixcir_ts_dt_idtable),
 	},
 	.probe		= auo_pixcir_probe,
-	.remove		= auo_pixcir_remove,
 	.id_table	= auo_pixcir_idtable,
 };
 

[-- Attachment #2: devm-add-custom-action.patch --]
[-- Type: text/plain, Size: 3599 bytes --]

devres: allow adding custom actions to the stack

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Sometimes drivers need to execute one-off actions in their error handling
or device teardown paths. An example would be toggling a GPIO line to
reset the controlled device into predefined state.

To allow performing such actions when using managed resources let's allow
adding them to stack/group of devres resources.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/devres.c  |   74 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h |    4 +++
 2 files changed, 78 insertions(+)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 6683906..507379e 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -671,6 +671,80 @@ int devres_release_group(struct device *dev, void *id)
 EXPORT_SYMBOL_GPL(devres_release_group);
 
 /*
+ * Custom devres actions allow inserting a simple function call
+ * into the teadown sequence.
+ */
+
+struct action_devres {
+	void *data;
+	void (*action)(void *);
+};
+
+static int devm_action_match(struct device *dev, void *res, void *p)
+{
+	struct action_devres *devres = res;
+	struct action_devres *target = p;
+
+	return devres->action == target->action &&
+	       devres->data == target->data;
+}
+
+static void devm_action_release(struct device *dev, void *res)
+{
+	struct action_devres *devres = res;
+
+	devres->action(devres->data);
+}
+
+/**
+ * devm_add_action() - add a custom action to list of managed resources
+ * @dev: Device that owns the action
+ * @action: Function that should be called
+ * @data: Pointer to data passed to @action implementation
+ *
+ * This adds a custom action to the list of managed resources so that
+ * it gets executed as part of standard resource unwinding.
+ */
+int devm_add_action(struct device *dev, void (*action)(void *), void *data)
+{
+	struct action_devres *devres;
+
+	devres = devres_alloc(devm_action_release,
+			      sizeof(struct action_devres), GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	devres->data = data;
+	devres->action = action;
+
+	devres_add(dev, devres);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devm_add_action);
+
+/**
+ * devm_remove_action() - removes previously added custom action
+ * @dev: Device that owns the action
+ * @action: Function implementing the action
+ * @data: Pointer to data passed to @action implementation
+ *
+ * Removes instance of @action previously added by devm_add_action().
+ * Both action and data should match one of the existing entries.
+ */
+void devm_remove_action(struct device *dev, void (*action)(void *), void *data)
+{
+	struct action_devres devres = {
+		.data = data,
+		.action = action,
+	};
+
+	WARN_ON(devres_destroy(dev, devm_action_release, devm_action_match,
+			       &devres));
+
+}
+EXPORT_SYMBOL_GPL(devm_remove_action);
+
+/*
  * Managed kzalloc/kfree
  */
 static void devm_kzalloc_release(struct device *dev, void *res)
diff --git a/include/linux/device.h b/include/linux/device.h
index 2fbf088..ba7c802 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -578,6 +578,10 @@ extern void devm_kfree(struct device *dev, void *p);
 void __iomem *devm_request_and_ioremap(struct device *dev,
 			struct resource *res);
 
+/* allows to add/remove a custom action to devres stack */
+int devm_add_action(struct device *dev, void (*action)(void *), void *data);
+void devm_remove_action(struct device *dev, void (*action)(void *), void *data);
+
 struct device_dma_parameters {
 	/*
 	 * a low level driver may set these to teach IOMMU code about

Re: [PATCH 4/4] Input: auo_pixcir_ts - add devicetree support

[Heiko Stübner]

Am Montag, 25. Februar 2013, 04:06:41 schrieb Dmitry Torokhov:
> On Sun, Feb 24, 2013 at 10:58:00AM +0100, Heiko Stübner wrote:
> > Hi Dmitry,
> > 
> > Am Sonntag, 24. Februar 2013, 09:00:15 schrieb Dmitry Torokhov:
> > > Hi Heiko,
> > > 
> > > On Sat, Feb 23, 2013 at 12:58:16PM +0100, Heiko Stübner wrote:
> > > > +static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct
> > > > device *dev) +{
> > > > +	struct auo_pixcir_ts_platdata *pdata = NULL;
> > > > +
> > > > +#ifdef CONFIG_OF
> > > > +	struct device_node *np = dev->of_node;
> > > > +
> > > > +	if (!np)
> > > > +		return NULL;
> > > > +
> > > > +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> > > > +	if (!pdata) {
> > > > +		dev_err(dev, "failed to allocate platform data\n");
> > > > +		return NULL;
> > > > +	}
> > > 
> > > I disike #ifdefs in the middle of the code, also it would be nice if we
> > > signal the proper error instead of always using -EINVAL when there are
> > > issues with platform/DT data.
> > > 
> > > How about the version of the patch below?
> > 
> > I tested it and everything of course still works :-) .
> 
> OK, great, then I will queue these for the next merge window.
> 
> Could you also try this patch (it however needs attached patch enhancing
> devres to support custom actions).
> 
> Thanks.

In general looks really nice and also works. I have some small nitpicks (patch 
desc, reset gpio naming) and it will also need to use 
devm_request_threaded_irq to get the irq correctly freed on removal. [all also 
marked at the corresponding places below]

Otherwise:

Tested-by: Heiko Stuebner <heiko@sntech.de>


Heiko

> Input: auo-pixer-ts - switch to using managed resources
> 
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> This simplifier error unwinding and device teardown.
		^^ simplifies

> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
>  drivers/input/touchscreen/auo-pixcir-ts.c |  162
>  ++++++++++++----------------- 1 file changed, 68 insertions(+), 94
>  deletions(-)
> 
> diff --git a/drivers/input/touchscreen/auo-pixcir-ts.c
> b/drivers/input/touchscreen/auo-pixcir-ts.c index dfa6d54..f4ba6ffa 100644
> --- a/drivers/input/touchscreen/auo-pixcir-ts.c
> +++ b/drivers/input/touchscreen/auo-pixcir-ts.c
> @@ -533,13 +533,21 @@ static struct auo_pixcir_ts_platdata
> *auo_pixcir_parse_dt(struct device *dev)
> 
>  }
>  #endif
> 
> +static void auo_pixcir_reset(void *data)
> +{
> +       struct auo_pixcir_ts *ts = data;
> +
> +       gpio_set_value(ts->pdata->gpio_rst, 0);
> +}
> +
> 
>  static int auo_pixcir_probe(struct i2c_client *client,
> 
> -                                     const struct i2c_device_id *id)
> +                           const struct i2c_device_id *id)
> 
>  {
>  
>         const struct auo_pixcir_ts_platdata *pdata;
>         struct auo_pixcir_ts *ts;
>         struct input_dev *input_dev;
> 
> -       int ret;
> +       int version;
> +       int error;
> 
>         pdata = dev_get_platdata(&client->dev);
>         if (!pdata) {
> 
> @@ -548,60 +556,30 @@ static int auo_pixcir_probe(struct i2c_client
> *client,
> 
>                         return PTR_ERR(pdata);
>         
>         }
> 
> -       ts = kzalloc(sizeof(struct auo_pixcir_ts), GFP_KERNEL);
> +       ts = devm_kzalloc(&client->dev,
> +                         sizeof(struct auo_pixcir_ts), GFP_KERNEL);
> 
>         if (!ts)
>         
>                 return -ENOMEM;
> 
> -       ret = gpio_request(pdata->gpio_int, "auo_pixcir_ts_int");
> -       if (ret) {
> -               dev_err(&client->dev, "request of gpio %d failed, %d\n",
> -                       pdata->gpio_int, ret);
> -               goto err_gpio_int;
> -       }
> -
> -       ret = gpio_direction_input(pdata->gpio_int);
> -       if (ret) {
> -               dev_err(&client->dev, "setting direction of gpio %d failed
> %d\n", -                       pdata->gpio_int, ret);
> -               goto err_gpio_dir;
> -       }
> -
> -       ret = gpio_request(pdata->gpio_rst, "auo_pixcir_ts_rst");
> -       if (ret) {
> -               dev_err(&client->dev, "request of gpio %d failed, %d\n",
> -                       pdata->gpio_rst, ret);
> -               goto err_gpio_dir;
> -       }
> -
> -       ret = gpio_direction_output(pdata->gpio_rst, 1);
> -       if (ret) {
> -               dev_err(&client->dev, "setting direction of gpio %d failed
> %d\n", -                       pdata->gpio_rst, ret);
> -               goto err_gpio_rst;
> +       input_dev = devm_input_allocate_device(&client->dev);
> +       if (!input_dev) {
> +               dev_err(&client->dev, "could not allocate input device\n");
> +               return -ENOMEM;
> 
>         }
> 
> -       msleep(200);
> -
> 
>         ts->pdata = pdata;
>         ts->client = client;
> 
> +       ts->input = input_dev;
> 
>         ts->touch_ind_mode = 0;
> 
> +       ts->stopped = true;
> 
>         init_waitqueue_head(&ts->wait);
>         
>         snprintf(ts->phys, sizeof(ts->phys),
>         
>                  "%s/input0", dev_name(&client->dev));
> 
> -       input_dev = input_allocate_device();
> -       if (!input_dev) {
> -               dev_err(&client->dev, "could not allocate input device\n");
> -               goto err_input_alloc;
> -       }
> -
> -       ts->input = input_dev;
> -
> 
>         input_dev->name = "AUO-Pixcir touchscreen";
>         input_dev->phys = ts->phys;
>         input_dev->id.bustype = BUS_I2C;
> 
> -       input_dev->dev.parent = &client->dev;
> 
>         input_dev->open = auo_pixcir_input_open;
>         input_dev->close = auo_pixcir_input_close;
> 
> @@ -626,72 +604,69 @@ static int auo_pixcir_probe(struct i2c_client
> *client,
> 
>                              AUO_PIXCIR_MAX_AREA, 0, 0);
>         
>         input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);
> 
> -       ret = i2c_smbus_read_byte_data(client, AUO_PIXCIR_REG_VERSION);
> -       if (ret < 0)
> -               goto err_fw_vers;
> -       dev_info(&client->dev, "firmware version 0x%X\n", ret);
> -
> -       ret = auo_pixcir_int_config(ts, pdata->int_setting);
> -       if (ret)
> -               goto err_fw_vers;
> -
> 
>         input_set_drvdata(ts->input, ts);
> 
> -       ts->stopped = true;
> 
> -       ret = request_threaded_irq(client->irq, NULL, auo_pixcir_interrupt,
> -                                  IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> -                                  input_dev->name, ts);
> -       if (ret) {
> -               dev_err(&client->dev, "irq %d requested failed\n",
> client->irq); -               goto err_fw_vers;
> +       error = devm_gpio_request_one(&client->dev, pdata->gpio_int,
> +                                     GPIOF_DIR_IN, "auo_pixcir_ts_int");
> +       if (error) {
> +               dev_err(&client->dev, "request of gpio %d failed, %d\n",
> +                       pdata->gpio_int, error);
> +               return error;
> 
>         }
> 
> -       /* stop device and put it into deep sleep until it is opened */
> -       ret = auo_pixcir_stop(ts);
> -       if (ret < 0)
> -               goto err_input_register;
> -
> -       ret = input_register_device(input_dev);
> -       if (ret) {
> -               dev_err(&client->dev, "could not register input device\n");
> -               goto err_input_register;
> +       error = devm_gpio_request_one(&client->dev, pdata->gpio_rst,
> +                                     GPIOF_DIR_OUT | GPIOF_INIT_HIGH,
> +                                     "auo_pixcir_ts_int");

									  ^^ auo_pixcir_ts_rst


> +       if (error) {
> +               dev_err(&client->dev, "request of gpio %d failed, %d\n",
> +                       pdata->gpio_rst, error);
> +               return error;
> 
>         }
> 
> -       i2c_set_clientdata(client, ts);
> -
> -       return 0;
> -
> -err_input_register:
> -       free_irq(client->irq, ts);
> -err_fw_vers:
> -       input_free_device(input_dev);
> -err_input_alloc:
> -       gpio_set_value(pdata->gpio_rst, 0);
> -err_gpio_rst:
> -       gpio_free(pdata->gpio_rst);
> -err_gpio_dir:
> -       gpio_free(pdata->gpio_int);
> -err_gpio_int:
> -       kfree(ts);
> +       error = devm_add_action(&client->dev, auo_pixcir_reset, ts);
> +       if (error) {
> +               auo_pixcir_reset(ts);
> +               dev_err(&client->dev, "failed to register xxx action,
> %d\n", +                       error);
> +               return error;
> +       }
> 
> -       return ret;
> -}
> +       msleep(200);
> 
> -static int auo_pixcir_remove(struct i2c_client *client)
> -{
> -       struct auo_pixcir_ts *ts = i2c_get_clientdata(client);
> -       const struct auo_pixcir_ts_platdata *pdata = ts->pdata;
> +       version = i2c_smbus_read_byte_data(client, AUO_PIXCIR_REG_VERSION);
> +       if (version < 0) {
> +               error = version;
> +               return error;
> +       }
> 
> -       free_irq(client->irq, ts);
> +       dev_info(&client->dev, "firmware version 0x%X\n", version);
> 
> -       input_unregister_device(ts->input);
> +       error = auo_pixcir_int_config(ts, pdata->int_setting);
> +       if (error)
> +               return error;
> 
> -       gpio_set_value(pdata->gpio_rst, 0);
> -       gpio_free(pdata->gpio_rst);
> +       error = request_threaded_irq(client->irq, NULL,

+       error = devm_request_threaded_irq(&client->dev, client->irq, NULL,


> auo_pixcir_interrupt, +                                   
> IRQF_TRIGGER_RISING | IRQF_ONESHOT, +                                   
> input_dev->name, ts);
> +       if (error) {
> +               dev_err(&client->dev, "irq %d requested failed, %d\n",
> +                       client->irq, error);
> +               return error;
> +       }
> 
> -       gpio_free(pdata->gpio_int);
> +       /* stop device and put it into deep sleep until it is opened */
> +       error = auo_pixcir_stop(ts);
> +       if (error)
> +               return error;
> +
> +       error = input_register_device(input_dev);
> +       if (error) {
> +               dev_err(&client->dev, "could not register input device,
> %d\n", +                       error);
> +               return error;
> +       }
> 
> -       kfree(ts);
> +       i2c_set_clientdata(client, ts);
> 
>         return 0;
>  
>  }
> 
> @@ -718,7 +693,6 @@ static struct i2c_driver auo_pixcir_driver = {
> 
>                 .of_match_table = of_match_ptr(auo_pixcir_ts_dt_idtable),
>         
>         },
>         .probe          = auo_pixcir_probe,
> 
> -       .remove         = auo_pixcir_remove,
> 
>         .id_table       = auo_pixcir_idtable,
>  
>  };
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] Input: auo_pixcir_ts - add devicetree support

[Dmitry Torokhov]

On Mon, Feb 25, 2013 at 02:47:17PM +0100, Heiko Stübner wrote:
> Am Montag, 25. Februar 2013, 04:06:41 schrieb Dmitry Torokhov:
> > On Sun, Feb 24, 2013 at 10:58:00AM +0100, Heiko Stübner wrote:
> > > Hi Dmitry,
> > > 
> > > Am Sonntag, 24. Februar 2013, 09:00:15 schrieb Dmitry Torokhov:
> > > > Hi Heiko,
> > > > 
> > > > On Sat, Feb 23, 2013 at 12:58:16PM +0100, Heiko Stübner wrote:
> > > > > +static struct auo_pixcir_ts_platdata *auo_pixcir_parse_dt(struct
> > > > > device *dev) +{
> > > > > +	struct auo_pixcir_ts_platdata *pdata = NULL;
> > > > > +
> > > > > +#ifdef CONFIG_OF
> > > > > +	struct device_node *np = dev->of_node;
> > > > > +
> > > > > +	if (!np)
> > > > > +		return NULL;
> > > > > +
> > > > > +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> > > > > +	if (!pdata) {
> > > > > +		dev_err(dev, "failed to allocate platform data\n");
> > > > > +		return NULL;
> > > > > +	}
> > > > 
> > > > I disike #ifdefs in the middle of the code, also it would be nice if we
> > > > signal the proper error instead of always using -EINVAL when there are
> > > > issues with platform/DT data.
> > > > 
> > > > How about the version of the patch below?
> > > 
> > > I tested it and everything of course still works :-) .
> > 
> > OK, great, then I will queue these for the next merge window.
> > 
> > Could you also try this patch (it however needs attached patch enhancing
> > devres to support custom actions).
> > 
> > Thanks.
> 
> In general looks really nice and also works. I have some small nitpicks (patch 
> desc, reset gpio naming) and it will also need to use 
> devm_request_threaded_irq to get the irq correctly freed on removal. [all also 
> marked at the corresponding places below]
> 
> Otherwise:
> 
> Tested-by: Heiko Stuebner <heiko@sntech.de>

Thank you Heiko, I made the changes you mentioned and queued with the
rest of the patches for 3.10.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html