add keyboard driver in st1.5

add keyboard driver in st1.5

BUG=NONE
TEST=Built and ran on st1.5
Signed-off-by: Horace Fu <horace.fu@quantatw.com>;

[nleeder, 2010-03-09 21:28:48]

http://codereview.chromium.org/723001/diff/1/4
File drivers/input/keyboard/qci_kbd.c (right):

http://codereview.chromium.org/723001/diff/1/4#newcode62
drivers/input/keyboard/qci_kbd.c:62: #define KEYBOARD_INT_GPIO			37
GPIO number should be passed in from platform data in the board file, not
hard-coded in driver

http://codereview.chromium.org/723001/diff/1/4#newcode261
drivers/input/keyboard/qci_kbd.c:261: struct i2ckbd_drv_data *iContext = 0;
Please rename CamelCase variables.

http://codereview.chromium.org/723001/diff/1/4#newcode331
drivers/input/keyboard/qci_kbd.c:331: gpio_request_fail:
Should set_i2c_clientdata(client, NULL) as iContext is destroyed

[Horace, 2010-03-11 12:47:02]

On 2010/03/09 21:28:48, nleeder wrote:

Please help to review once again, thanks!

[nleeder, 2010-03-11 23:13:38]

Similar comments to the trackpad driver re signed-off-by line etc.  Thanks!

http://codereview.chromium.org/723001/diff/4001/5001
File arch/arm/mach-msm/board-qsd8x50.c (right):

http://codereview.chromium.org/723001/diff/4001/5001#newcode1971
arch/arm/mach-msm/board-qsd8x50.c:1971: .irq = KEYBOARD_INT_GPIO
These changes in the board-qsd8x50.c file are correct, but they shouldn't be
included in this patch. Changes in the driver tree and mach-msm tree should be
in separate patches. Please remove board-qsd8x50.c from this patch. Qualcomm can
add the mach-msm patch.

http://codereview.chromium.org/723001/diff/4001/5002
File drivers/input/keyboard/Kconfig (right):

http://codereview.chromium.org/723001/diff/4001/5002#newcode366
drivers/input/keyboard/Kconfig:366: default m
Should have a depends line such as: 
depends on I2C && ARCH_MSM_SCORPION 

Also it should default to 'n'. In the defconfig file for the appropriate target
system it can be selected as 'm'. Qualcomm can do that patch.

[Horace, 2010-03-12 05:58:18]

On 2010/03/11 23:13:38, nleeder wrote:

Got it, and will inform qualcomm to help to do that. thanks!

[nleeder, 2010-03-12 21:11:06]

lgtm

[nleeder, 2010-03-17 18:50:30]

Please also run the patch through scripts/checkpatch.pl and fix any errors.
Also add user msb to the review list. Mandeep will need to approve the patch in
order to mainline it.

http://codereview.chromium.org/723001/diff/12001/13001
File drivers/input/keyboard/Kconfig (right):

http://codereview.chromium.org/723001/diff/12001/13001#newcode369
drivers/input/keyboard/Kconfig:369: Say Y here if you want to use the Quanta
keyboard driver for ST 1.5 platform.
This needs to be split into two lines to keep each line < 80 characters long

[nleeder, 2010-03-17 19:06:58]

+msb
Mandeep, could you review this too? Thanks.

[Mandeep Singh Baines, 2010-03-18 22:05:55]

Please run scripts/checkpatch.pl on this code. I see a lot of errors.

Please sign-off on this patch.

http://codereview.chromium.org/723001/diff/12001/13003
File drivers/input/keyboard/qci_kbd.c (right):

http://codereview.chromium.org/723001/diff/12001/13003#newcode60
drivers/input/keyboard/qci_kbd.c:60: /* 00h */ NOKEY, NOKEY, NOKEY, KEY_5,
KEY_7,
Please use 1 line per key. See amikbd.c for an example.

http://codereview.chromium.org/723001/diff/12001/13003#newcode162
drivers/input/keyboard/qci_kbd.c:162: /* Pinter to the Input device structure */
s/Pinter/Pointer/

Is this a stale comment?

http://codereview.chromium.org/723001/diff/12001/13003#newcode175
drivers/input/keyboard/qci_kbd.c:175: .name  = KEYBOARD_ID_NAME,
.pm = &qcikbd_pm_ops,

http://codereview.chromium.org/723001/diff/12001/13003#newcode180
drivers/input/keyboard/qci_kbd.c:180: .resume   = qcikbd_resume,
Use .pm instead.

http://codereview.chromium.org/723001/diff/12001/13003#newcode227
drivers/input/keyboard/qci_kbd.c:227: }
These brackets don't look like they are aligned correctly.

http://codereview.chromium.org/723001/diff/12001/13003#newcode245
drivers/input/keyboard/qci_kbd.c:245: struct i2ckbd_drv_data *context = 0;
Add a CR here.

Initializing to 0 seems unnecessary.

http://codereview.chromium.org/723001/diff/12001/13003#newcode248
drivers/input/keyboard/qci_kbd.c:248: return err;
Might be clearer to not initialize err and return -ENOMEM here. The code would
be easier to follow.

http://codereview.chromium.org/723001/diff/12001/13003#newcode290
drivers/input/keyboard/qci_kbd.c:290: for (i = 1; i < 512 ; i++)
on2_keycode only has 256 elements!

Maybe use a sizeof here instead?

[nleeder, 2010-03-23 22:09:55]

Patch updated by Horace.

[Mandeep Singh Baines, 2010-03-23 22:48:04]

Code looks great! Just once last nit before committing.

LGTM after fixing keycode array and adding Signed-off-by to codereview
description.

You can change the description by clicking the "Edit Issue" button on the
codereview site:

http://codereview.chromium.org/723001

LGTM = Looks Good To Me (i.e. approved for submission)

http://codereview.chromium.org/723001/diff/26001/27003
File drivers/input/keyboard/qci_kbd.c (right):

http://codereview.chromium.org/723001/diff/26001/27003#newcode60
drivers/input/keyboard/qci_kbd.c:60: NOKEY,
I was hoping for something like this:

[0]   = NOKEY,
[1]   = NOKEY,

But the way you had it before was also fine. (Sorry for nitpicking on this one.)

So please change it back to how you had it before or add the  indexes. Your
preference.

[Horace, 2010-03-25 09:01:42]

Update on2_keycode array! Please help to review again, thanks!

[nleeder, 2010-03-29 17:32:35]

Is this done now - can it be committed?  Thanks.

[Mandeep Singh Baines, 2010-03-29 22:26:01]

Thanks! Committed to qualcomm-2.6.31.12

http://chromiumos-git/?p=kernel.git;a=commit;h=c8470ae08909f314e7c91e9db5612a...