On Thu, Oct 25, 2012 at 12:32:39PM +0800, 劉嘉駿 wrote:

Hi Dmitry,
	Thanks for review.

quoted

-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
Sent: Thursday, October 25, 2012 2:13 AM
To: Scott Liu
Cc: linux-input@vger.kernel.org; linux-i2c@vger.kernel.org;

linux-kernel@vger.kernel.org;

quoted

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:

quoted

This patch is for Elan eKTF Touchscreen product, I2C adpater module.

Signed-off-by: Scott Liu <redacted>
---

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.

quoted

- why do you need a separate probe work instead of doing what you
  need in elants_probe()

will fix.

quoted

- it is not a good idea to register input device first and then
  allocating memory for MT handling.

Ooop...will fix.

quoted

- 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