Hello Shubhra, On Tue, Aug 24, 2010 at 08:37:16 +0200, Datta, Shubhrajyoti wrote: > > + * ske_keypad_chip_init : init keypad controller configuration > > + * > > + * Enable Multi key press detection, auto scan mode > > + */ > May consider making it an init function as called only at probe? Okay. will do so. > > + > Nitpick : Bonus space Sure. > > +out_unregisterinput: > > + input_unregister_device(input); > > + input = NULL; > > + clk_disable(keypad->clk); > > +out_freeinput: > Unregister and then free you may want to have a relook. > > + input_free_device(input); Hmm. Is there something wrong here? input_free_device() is being called with a NULL after input_unregister_device(), which is okay. > > +out_freekeypad: > > + kfree(keypad); > > +out_freeclk: > Kfree keypad and then access keypad->clk ? > > > + clk_put(keypad->clk); Oops. It ought to be only clk. > > + clk_disable(keypad->clk); > > + clk_put(keypad->clk); > What is the cpu you tested on? Just curious not a comment. I have tested it on the U8500 platform. Thanx for the review; I will re-post with the changes. Regards, Sundar -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
