Hi Dmitry,
Thanks for review.
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Thursday, October 25, 2012 2:13 AM
> To: Scott Liu
> Cc: linux-in...@vger.kernel.org; linux-i2c@vger.kernel.org;
linux-ker...@vger.kernel.org;
> 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....@emc.com.tw>
> > ---
> >
> > 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?
> - please remove the rest of the custom threads
OK
> - you do not need to call input_mt_destroy_slots() explicitly
OK
> - use request_firmware() instead of special character device to upload
> firmware.
OK, but I'll remove firmware update function at this patch first.
> - please use standard kernel-doc markup.
> - consider what attributes are there only for debugging and move them to
> debugfs.
OK.
> - I find the use of enums in this driver quite unconventional, just
> standard #defines would probably be more straightforward.
OK.
Thanks,
Scott
>
> Thanks.
>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
