Re: [PATCH] input: add support for generic GPIO-based matrix keypad

[Paulius Zaleckas <paulius.zaleckas@teltonika.lt>]

Trilok Soni wrote:
> Hi Paulius,
> 
> On Fri, Jul 17, 2009 at 1:21 PM, Paulius
> Zaleckas<paulius.zaleckas@teltonika.lt> wrote:
>> Eric Miao wrote:
>>> Dmitry Torokhov wrote:
>>>> Hi Eric,
>>>>
>>>> On Fri, Jun 12, 2009 at 09:26:43PM +0800, Eric Miao wrote:
>>>>> 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()
>>>>>
>>>> Could you please take a look at the driver as it is commited to
>>>> 'for-linus' branch of my tree and let me know if you see anything wrong
>>>> there? Otherwise I inted to submit it in my next update to Linus.
>>>>
>>> Sorry Dmitry,
>>>
>>> Reply so late and til now I have some time to test this. The patch there
>>> still has at least one issue I guess - the keycodes[] allocation size
>>> should be MATRIX_MAX_ROWS * MATRIX_MAX_COLS if I understand correctly.
>>>
>>> I'm experiencing a problem of keypad->pdata being overwritten, and found
>>> keycodes are actually accessed by [(row << 4) + col], so that should be
>>> a full range.
>> As this driver is already in the mainline and I was trying to use it...
>> I get various kernel crashes due to this keycodes[] allocation size bug.
>> Care to send a patch for this bug?
>>
>>> And one more minor issue that may not be significant is the hardcoding of
>>> the '4' in this (row << 4).
>>>
>>> Some of the key hits on my Littleton just worked smoothly with the above
>>> correction, but not all - some key hits just gave intermittent events, I
>>> have no idea how's that - maybe related to my scan delay and debouncing
>>> settings
> 
> Did you tried assigning max_keypmap_size in platform data to
> MATRIX_MAX_COLS * MATRIX_MAX_ROWS ?
> 

Yes, this fixes crashes. But this is just workaround for bug in driver.