On Wed, Nov 19, 2014 at 05:44:48PM +0100, Daniel Mack wrote: > On 11/19/2014 04:30 PM, jeffrey.lin wrote: > > From: "jeffrey.lin" <jeffrey....@gmail.com> > > > > this patch is porting Raydium I2C touch driver. Developer can enable > > raydium touch driver by modifying define "CONFIG_TOUCHSCREEN_RM_TS". > > > > BUG: None > > TEST: built and test with peppy > > > > Signed-off-by: jeffrey....@rad-ic.com > > Change-Id: I05d54e5ef29249d2a6eae97222c90bed11e839f9 > > --- > > Just some general remarks that IMO need to be addressed before a more > thorough review can happen: > > > * Please remove all dead code, such as code in comments and in #if 0 > blocks. > > * Use the regmap framework to abstract the hardware access on the I2C > layer (see drivers/input/keyboard/cap1106.c and many other drivers > as an example, and check include/linux/regmap.h). That makes the code > a lot shorter and more comprehensible to read. > > * By using request_threaded_irq(), you don't have to manually control > your worker and can simplify your code quite a bit. > > * See if you can claim all the resources the driver needs by using > devm_* variants. > > * Don't use uppercase names for filenames, structs, functions and IDs > > * Why do you need a miscdevice for this? Isn't the input event layer > enough? > > * Also, run scripts/checkpatch.pl on the new patch, which will help > you find some more typical pitfalls.
Also please convert driver to MT-B (slotted) protocol (you are currently using MT-Ai version). And please remove join_bytes(). It is called get_unaligned_le16(). Most of the things that need fixing have already been listed in response to your original submission, please do address them before posting the driver again. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
