[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130718172007.GE32381@polaris.bitmath.org>
Date: Thu, 18 Jul 2013 19:20:07 +0200
From: rydberg@...omail.se
To: Nick Dyer <nick.dyer@...ev.co.uk>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Daniel Kurtz <djkurtz@...omium.org>,
Joonyoung Shim <jy0922.shim@...sung.com>,
Alan Bowens <Alan.Bowens@...el.com>,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
Peter Meerwald <pmeerw@...erw.net>,
Benson Leung <bleung@...omium.org>,
Olof Johansson <olofj@...omium.org>
Subject: Re: [PATCH 26/51] Input: atmel_mxt_ts - Move input device init into
separate function
On Thu, Jun 27, 2013 at 01:49:01PM +0100, Nick Dyer wrote:
> Signed-off-by: Nick Dyer <nick.dyer@...ev.co.uk>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 129 +++++++++++++++++-------------
> 1 file changed, 75 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 8632133..030ebc5 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1728,73 +1728,39 @@ static int mxt_handle_pdata(struct mxt_data *data)
> return 0;
> }
>
> -static int mxt_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> +static int mxt_initialize_t9_input_device(struct mxt_data *data)
> {
> - struct mxt_data *data;
> + struct device *dev = &data->client->dev;
> + const struct mxt_platform_data *pdata = data->pdata;
Similar code was removed from mxt_initialize(), some consistency would be nice.
> struct input_dev *input_dev;
> int error;
> unsigned int num_mt_slots;
> unsigned int mt_flags = 0;
> int i;
>
> - data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
> input_dev = input_allocate_device();
> - if (!data || !input_dev) {
> - dev_err(&client->dev, "Failed to allocate memory\n");
> - error = -ENOMEM;
> - goto err_free_mem;
> + if (!input_dev) {
> + dev_err(dev, "Failed to allocate memory\n");
> + return -ENOMEM;
> }
>
> input_dev->name = "Atmel maXTouch Touchscreen";
> - snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0",
> - client->adapter->nr, client->addr);
> -
> input_dev->phys = data->phys;
> -
> input_dev->id.bustype = BUS_I2C;
> - input_dev->dev.parent = &client->dev;
> + input_dev->dev.parent = dev;
> input_dev->open = mxt_input_open;
> input_dev->close = mxt_input_close;
>
> - data->client = client;
> - data->input_dev = input_dev;
> - data->irq = client->irq;
> - i2c_set_clientdata(client, data);
> -
> - error = mxt_handle_pdata(data);
> - if (error)
> - goto err_free_mem;
> -
> - init_completion(&data->bl_completion);
> - init_completion(&data->reset_completion);
> - init_completion(&data->crc_completion);
> -
> - error = request_threaded_irq(data->irq, NULL, mxt_interrupt,
> - data->pdata->irqflags | IRQF_ONESHOT,
> - client->name, data);
> - if (error) {
> - dev_err(&client->dev, "Failed to register interrupt\n");
> - goto err_free_pdata;
> - }
> -
> - disable_irq(client->irq);
> -
> - error = mxt_initialize(data);
> - if (error)
> - goto err_free_irq;
> -
> __set_bit(EV_ABS, input_dev->evbit);
> - __set_bit(EV_KEY, input_dev->evbit);
> - __set_bit(BTN_TOUCH, input_dev->keybit);
> + input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
>
> - if (data->pdata->t19_num_keys) {
> + if (pdata->t19_num_keys) {
> __set_bit(INPUT_PROP_BUTTONPAD, input_dev->propbit);
>
> - for (i = 0; i < data->pdata->t19_num_keys; i++)
> - if (data->pdata->t19_keymap[i] != KEY_RESERVED)
> + for (i = 0; i < pdata->t19_num_keys; i++)
> + if (pdata->t19_keymap[i] != KEY_RESERVED)
> input_set_capability(input_dev, EV_KEY,
> - data->pdata->t19_keymap[i]);
> + pdata->t19_keymap[i]);
>
> mt_flags |= INPUT_MT_POINTER;
>
> @@ -1819,8 +1785,11 @@ static int mxt_probe(struct i2c_client *client,
> /* For multi touch */
> num_mt_slots = data->T9_reportid_max - data->T9_reportid_min + 1;
> error = input_mt_init_slots(input_dev, num_mt_slots, mt_flags);
> - if (error)
> - goto err_free_object;
> + if (error) {
> + dev_err(dev, "Error %d initialising slots\n", error);
> + goto err_free_mem;
> + }
> +
> input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
> 0, MXT_MAX_AREA, 0, 0);
> input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> @@ -1834,11 +1803,64 @@ static int mxt_probe(struct i2c_client *client,
>
> error = input_register_device(input_dev);
> if (error) {
> - dev_err(&client->dev, "Error %d registering input device\n",
> - error);
> - goto err_free_object;
> + dev_err(dev, "Error %d registering input device\n", error);
> + goto err_free_mem;
> }
>
> + data->input_dev = input_dev;
> +
> + return 0;
> +
> +err_free_mem:
> + input_free_device(input_dev);
> + return error;
> +}
> +
> +static int mxt_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct mxt_data *data;
> + int error;
> +
> + data = kzalloc(sizeof(struct mxt_data), GFP_KERNEL);
> + if (!data) {
> + dev_err(&client->dev, "Failed to allocate memory\n");
> + return -ENOMEM;
> + }
> +
> + snprintf(data->phys, sizeof(data->phys), "i2c-%u-%04x/input0",
> + client->adapter->nr, client->addr);
> +
> + data->client = client;
> + data->irq = client->irq;
> + i2c_set_clientdata(client, data);
> +
> + error = mxt_handle_pdata(data);
> + if (error)
> + goto err_free_mem;
> +
> + init_completion(&data->bl_completion);
> + init_completion(&data->reset_completion);
> + init_completion(&data->crc_completion);
> +
> + error = request_threaded_irq(data->irq, NULL, mxt_interrupt,
> + data->pdata->irqflags | IRQF_ONESHOT,
> + client->name, data);
> + if (error) {
> + dev_err(&client->dev, "Failed to register interrupt\n");
> + goto err_free_pdata;
> + }
> +
> + disable_irq(data->irq);
> +
> + error = mxt_initialize(data);
> + if (error)
> + goto err_free_irq;
> +
> + error = mxt_initialize_t9_input_device(data);
> + if (error)
> + goto err_free_object;
> +
> error = sysfs_create_group(&client->dev.kobj, &mxt_attr_group);
> if (error) {
> dev_err(&client->dev, "Failure %d creating sysfs group\n",
> @@ -1849,8 +1871,8 @@ static int mxt_probe(struct i2c_client *client,
> return 0;
>
> err_unregister_device:
> - input_unregister_device(input_dev);
> - input_dev = NULL;
> + input_unregister_device(data->input_dev);
> + data->input_dev = NULL;
> err_free_object:
> kfree(data->object_table);
> err_free_irq:
> @@ -1859,7 +1881,6 @@ err_free_pdata:
> if (!dev_get_platdata(&data->client->dev))
> kfree(data->pdata);
> err_free_mem:
> - input_free_device(input_dev);
> kfree(data);
> return error;
> }
> --
> 1.7.10.4
>
In what way does this patch simplify things?
Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists