Re: HTC: touchscreen driver

["Arve Hjønnevåg" <arve@android.com>]

On Tue, Jul 14, 2009 at 2:46 AM, Pavel Machek<pavel@ucw.cz> wrote:
>
>> >        if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> >                printk(KERN_ERR "synaptics_ts_probe: need I2C_FUNC_I2C\n");
>> > @@ -269,6 +354,9 @@ static int synaptics_ts_probe(
>> >        pdata = client->dev.platform_data;
>> >        if (pdata)
>> >                ts->power = pdata->power;
>> > +       else
>> > +               pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
>> > +
>>
>> Where do you free this?
>
> Well, nowhere; but it should not actually matter. ... btw can this
> path (!pdata) actually trigger? The driver will not be working,
> anyway, in that case..

The driver should work fine without the pdata. The pdata is used to
align the touchscreen data with the screen behind it.

>> > +       u32 flags;
>> > +       u32 inactive_left; /* 0x10000 = screen width */
>> > +       u32 inactive_right; /* 0x10000 = screen width */
>> > +       u32 inactive_top; /* 0x10000 = screen height */
>> > +       u32 inactive_bottom; /* 0x10000 = screen height */
>> > +       u32 snap_left_on; /* 0x10000 = screen width */
>> > +       u32 snap_left_off; /* 0x10000 = screen width */
>> > +       u32 snap_right_on; /* 0x10000 = screen width */
>> > +       u32 snap_right_off; /* 0x10000 = screen width */
>> > +       u32 snap_top_on; /* 0x10000 = screen height */
>> > +       u32 snap_top_off; /* 0x10000 = screen height */
>> > +       u32 snap_bottom_on; /* 0x10000 = screen height */
>> > +       u32 snap_bottom_off; /* 0x10000 = screen height */
>> > +       u32 fuzz_x; /* 0x10000 = screen width */
>> > +       u32 fuzz_y; /* 0x10000 = screen height */
>>
>> I prefer the existing C99 types.
>
> Yes, well, the rest of kernel disagrees with you.

The coding style document claims C99 types are allowed. If that is no
longer the case it should be updated.

>
>> This driver only supports a subset of Synaptics' devices so a more
>> generic driver will eventually be needed. The patch below adds support
>> for a more recent but similar panel.
>
> Ok, that should be simple enough to apply, but lets do improvements
> when we cleaned the code enough for the mainline...?

We need this change now. Your cleanup will cause conflicts for anyone
using our driver, so it would be better if it includes all our fixes.

-- 
Arve Hjønnevåg