On Tue, Jun 2, 2009 at 10:55 PM, Uli Luckas <u.luckas@xxxxxxx> wrote:
> 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?
I'd prefer this being left for further improvement when someone comes
with such a hardware and the patch.
> 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.
>
You see, I'm not a good writer, so ... And I think the reader
will be able to figure out. Well, just feel free to send me a
copy if you have a good description of this :)
--
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
