Hi Uli Luckas,
On Tue, Jun 2, 2009 at 8:25 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?
> 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.
Following is on radar after this driver gets picked up for the
mainline merge window (should open soon as 2.6.30-rc8 already out) by
Dmitry.
1. passing settling time from platform data as mentioned by you.
Android gpio matrix driver I have pointed out earlier is already doing
this.
2. adding phantom key removal support - probably picking it up
straight from android driver.
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
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
