On Fri, Jun 12, 2009 at 9:01 PM, Trilok Soni<soni.trilok@xxxxxxxxx> wrote: > Hi Dmitry, > >> >> Input: add support for generic GPIO-based matrix keypad >> >> From: Eric Miao <eric.y.miao@xxxxxxxxx> >> >> Original patch by Marek Vasut, modified by Eric in: >> >> 1. use delayed work to simplify the debouncing >> 2. combine col_polarity/row_polarity into a single active_low field >> 3. use a generic bit array based XOR algorithm to detect key >> press/release, which should make the column assertion time >> shorter and code a bit cleaner >> 4. remove the ALT_FN handling, which is no way generic, the ALT_FN >> key should be treated as no different from other keys, and >> translation will be done by user space by commands like 'loadkeys'. >> >> [dtor@xxxxxxx: fix error unwinding path, support changing keymap >> from userspace] >> Signed-off-by: Marek Vasut <marek.vasut@xxxxxxxxx> >> Signed-off-by: Eric Miao <eric.miao@xxxxxxxxxxx> >> Signed-off-by: Dmitry Torokhov <dtor@xxxxxxx> >> --- > > Did you took latest patch submitted from Eric? Because it had more > signed-off and acked-by lines, like this. > > Signed-off-by: Marek Vasut <marek.vasut@xxxxxxxxx> > Reviewed-by: Trilok Soni <soni.trilok@xxxxxxxxx> > Reviewed-by: Uli Luckas <u.luckas@xxxxxxx> > Reviewed-by: Russell King <linux@xxxxxxxxxxxxxxxx> > Reviewed-by: Robert Jarzmik <robert.jarzmik@xxxxxxx> > Signed-off-by: Eric Miao <eric.miao@xxxxxxxxxxx> > > http://markmail.org/message/2wrr2b6mr6qsd4xs#query:+page:1+mid:fkkfxlumfm4mjhk4+state:results > > Eric can confirm otherwise. > >> >> config KEYBOARD_HIL_OLD >> tristate "HP HIL keyboard support (simple driver)" >> @@ -254,7 +263,7 @@ config KEYBOARD_PXA27x >> tristate "PXA27x/PXA3xx keypad support" >> depends on PXA27x || PXA3xx >> help >> - Enable support for PXA27x/PXA3xx keypad controller >> + Enable support for PXA27x/PXA3xx keypad controller. > > Why this change in this patch? > >> + >> + code = (row << 4) + col; > > << 4 logic will break once MAX_ROWS increased, right? > Hi Dmitry, I've tested the driver code, and it's basically OK except for two minor fixes: 1) GPIO and IRQs have to be initialized before input_register_device(), otherwise input->open() will be invoked before that, which will schedule an immediate scan work and fail. 2) disable_irq_rows() called in init_matrix_gpio() so that by default it's initialized to disabled - and will be enabled by input->open() Diff follows: --- drivers/input/keyboard/matrix_keypad.c.orig 2009-06-12 21:18:20.000000000 +0800 +++ drivers/input/keyboard/matrix_keypad.c 2009-06-12 21:07:26.000000000 +0800 @@ -290,6 +290,9 @@ static int __devinit init_matrix_gpio(st goto err_free_irqs; } } + + /* initialized as disabled - enabled by input->open */ + disable_row_irqs(keypad); return 0; err_free_irqs: @@ -376,22 +379,19 @@ static int __devinit matrix_keypad_probe input_set_capability(input_dev, EV_MSC, MSC_SCAN); input_set_drvdata(input_dev, keypad); - err = input_register_device(keypad->input_dev); + err = init_matrix_gpio(pdev, keypad); if (err) goto err_free_mem; - err = init_matrix_gpio(pdev, keypad); + err = input_register_device(keypad->input_dev); if (err) - goto err_unregister; + goto err_free_mem; device_init_wakeup(&pdev->dev, pdata->wakeup); platform_set_drvdata(pdev, keypad); return 0; -err_unregister: - input_unregister_device(input_dev); - input_dev = NULL; err_free_mem: input_free_device(input_dev); kfree(keycodes); -- 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
