On Sunday, 31. May 2009, Eric Miao wrote:
> +#ifdef CONFIG_PM
> +static int matrix_keypad_suspend(struct platform_device *pdev,
> pm_message_t state)
> +{
> + struct matrix_keypad *keypad = platform_get_drvdata(pdev);
> + struct matrix_keypad_platform_data *pdata = keypad->pdata;
> + int i;
> +
> + disable_row_irqs(keypad);
> + flush_work(&keypad->work.work);
>
Hi Eric,
thanks for fixing the problems. One comment on the approach:
disable_row_irqs uses disable_irq_nosync. I guess this still works as
the interrupt handler is requested with IRQF_DISABLED.
Some more non-show stoppers:
1) On some SoCs wakeup gpios are a precoius resource. Therfore a hardware design which ORs all row gpios and connects the result to a single row_interrupt might be smart. Could
the driver be made generic enought to support this?
2) Did anyone ever wonder why this driver is able to detect a second key press on the same row? Actually the driver has polling behavior while any key is pressed. This is
because scanning the key matix triggers a new interrupt on every row. Every one of these interrupts will fire as soon as enable_irq is called, scheduling a new scan after the
debounce period. I think that is worth being documented in the source code.
regards,
Uli
--
------- ROAD ...the handyPC Company - - - ) ) )
Uli Luckas
Head of Software Development
ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 62 | fax: +49 (30) 230069 - 69
url: www.road.de
Amtsgericht Charlottenburg: HRB 96688 B
Managing director: Hans-Peter Constien
--
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
