Re: [PATCH] input: add support for Nomadik SKE keypad controller

Hello Shubhra,

On Tue, Aug 24, 2010 at 08:37:16 +0200, Datta, Shubhrajyoti wrote:

quoted

+ * 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.

quoted

+

Nitpick : Bonus space

Sure.

quoted

+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.

quoted

+     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.

quoted

+out_freekeypad:
+     kfree(keypad);
+out_freeclk:

Kfree keypad and then access keypad->clk ?

quoted

+     clk_put(keypad->clk);

Oops. It ought to be only clk.

quoted

+     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