Re: [PATCH v2] Support Elan Touchscreen eKTF product.

From: Dmitry Torokhov
Date: Thu Oct 25 2012 - 03:10:22 EST


On Thu, Oct 25, 2012 at 12:32:39PM +0800, ååé wrote:
> Hi Dmitry,
> Thanks for review.
>
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx]
> > Sent: Thursday, October 25, 2012 2:13 AM
> > To: Scott Liu
> > Cc: linux-input@xxxxxxxxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx;
> > Benjamin Tissoires; Jesse; Vincent Wang; Paul
> > Subject: Re: [PATCH v2] Support Elan Touchscreen eKTF product.
> >
> > Hi Scott,
> >
> > On Wed, Oct 24, 2012 at 09:41:43AM +0800, Scott Liu wrote:
> > > This patch is for Elan eKTF Touchscreen product, I2C adpater module.
> > >
> > > Signed-off-by: Scott Liu <scott.liu@xxxxxxxxxx>
> > > ---
> > >
> > > Hi,
> > > v2 revision I have fixed some bug as your advise.
> > > 1. To target the mainline
> > > 2. No Android dependency
> > > 3. reuse those duplication code from Henrik's patchset.
> > > (input_mt_sync_frame() / input_mt_get_slot_by_key())
> >
> > Just a quick run through the code, so:
> >
> > - please remove polling support, it is not useful in production;
>
> OK.
>
> > - why do you need a separate probe work instead of doing what you
> > need in elants_probe()
>
> will fix.
>
> > - it is not a good idea to register input device first and then
> > allocating memory for MT handling.
>
> Ooop...will fix.
>
> > - I do not understand why kfifo is needed
>
> The firmware and the host would conflict by read command and finger report
> simultaneously. So I'm simply using kfifo in IRQ thread function.
>
> * read command: writing 4 bytes commands and the device asserts GPIO
> interrupt and then response 4 bytes data.
>
> There was an error if we do not use kfifo:
> With heavy loading by finger report / read command, the driver may
> get finger report as response data.
>
> So, do you understand my meaning?

No I don't. Most of my confusion stems from the fact that you only put
data into kfifo but not actually use it anywhere. You do fetch the data
in your "drop old" function, but that data is just dropped. So I really
do not see the point.

Thanks.

--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/