Re: [PATCH 2/2] tsc2007: reduced number of I2C transfers

Dmitry Torokhov
2009-08-05T05:04:51Z

On Tue, Aug 04, 2009 at 02:12:35PM +0200, Richard Röjfors wrote:
> Dmitry Torokhov wrote:
>>
>> I modified the patch a bit before applying. In fact there were a few
>> changes to all the patces so it would be nice if you could take a look
>> at the 'next' branch of my tree:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
>>
>> Also, I di dnot quite like the logic of the patch making
>> get_pendown_state callback optional; could you please try the patch
>> below and let me know of it works for you?
>
> I like your changes, there is just a small miss while checking the
> platform data. You forgot to remove the check for get_pen_down_state in
> the probe function, see the patch below (to be applied on top of yours):
>
> I also would like a check if the power down failed or not during probe.
>
> --Richard
>
> Input: tsc2007 - Check if I2C communication works during probe
>
> Check the result when sending the power down command to the controller.
>
> Signed-off-by: Richard Röjfors <richard.rojfors.ext@mocean-labs.com>
> ---
> Index: linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c
> ===================================================================
> --- linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1071)
> +++ linux-2.6.31-rc2/drivers/input/touchscreen/tsc2007.c (revision 1073)
> @@ -269,7 +269,7 @@
> struct input_dev *input_dev;
> int err;
>
> - if (!pdata || !pdata->get_pendown_state) {
> + if (!pdata) {
> dev_err(&client->dev, "platform data is required!\n");
> return -EINVAL;
> }

Ah, yes, thanks.

> @@ -326,10 +326,14 @@
> i2c_set_clientdata(client, ts);
>
> /* Prepare for touch readings - power down ADC and enable PENIRQ */
> - tsc2007_xfer(ts, PWRDOWN);
> + err = tsc2007_xfer(ts, PWRDOWN);
> + if (err < 0)
> + goto err_unreg_dev;
>
> return 0;
>
> + err_unreg_dev:
> + input_unregister_device(ts->input);

This will case double free.. I will adjust locally.

> err_free_irq:
> tsc2007_free_irq(ts);
> if (pdata->exit_platform_hw)

--
Dmitry