Thread (4 messages) 4 messages, 3 authors, 2013-02-26

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

From: Heiko Stübner <heiko@sntech.de>
Date: 2013-02-25 13:47:17
Also in: linux-devicetree

Possibly related (same subject, not in this thread)

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:
quoted
Hi Dmitry,

Am Sonntag, 24. Februar 2013, 09:00:15 schrieb Dmitry Torokhov:
quoted
Hi Heiko,

On Sat, Feb 23, 2013 at 12:58:16PM +0100, Heiko Stübner wrote:
quoted
+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
quoted hunk ↗ jump to hunk
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,
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help