Re: [PATCH 1/3] Input: DaVinci Key Scan Driver

[Miguel Aguilar <miguel.aguilar-9uBrGCPFOa1Wk0Htik3J/w@public.gmane.org>]

Dmitry Torokhov wrote:
> On Wed, Sep 30, 2009 at 07:40:24AM -0600, Miguel Aguilar wrote:
>> Kevin Hilman wrote:
>>> "Paulraj, Sandeep" <s-paulraj-l0cyMroinI0@public.gmane.org> writes:
>>>
>>>>> On Fri, Sep 25, 2009 at 08:45:31AM -0600, Miguel Aguilar wrote:
>>>>>>>>>>> +     /* Enable Keyscan module and enable */
>>>>>>>>>>> +     davinci_ks_write(davinci_ks, DAVINCI_KEYSCAN_AUTODET |
>>>>>>>> DAVINCI_KEYSCAN_KEYEN,
>>>>>>>>>>> +                     DAVINCI_KEYSCAN_KEYCTRL);
>>>>>>>>>> We seem to be hardcoded to the 4x4 matrix type here.
>>>>>>>>>> Some DM365 boards could use 5x3 matrix.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Sekhar
>>>>>>>>>>
>>>>>>>>> [MA] I will add a platform option for matrix type to choose 4x4 or
>>>>> 5x3
>>>>>>>> matrix.
>>>>>>>>> What do think about this?
>>>>>>>>>
>>>>>>>> Sounds good. I am not sure if any other piece of code
>>>>>>>> needs to change though.
>>>>>>> The strobe and interval have to change.  Afcourse that change comes
>>>>> into the picture only when the mode is changed from 4x4 to 5x3.
>>>>>>> I can see that there are options given for this in board specific code.
>>>>>>>> Thanks,
>>>>>>>> Sekhar
>>>>>>> Thanks,
>>>>>>> Sandeep
>>>>>>>
>>>>>> struct davinci_ks_platform_data {
>>>>>> 	unsigned short	*keymap;
>>>>>> 	u32		keymapsize;
>>>>>> 	u32		rep:1;
>>>>>> 	u32		strobe;
>>>>>> 	u32		interval;
>>>>>> +	u32		matrix_type;
>>>>>> };
>>>>>>
>>>>>> I will add something like this, so from the board specific code you can
>>>>>> define strobe, interval and matrix type.
>>>>> Make it enum at least. BTW, is it possible to add u8 rows, cols instead
>>>>> and be able to handle more matrix variations?
>>>> The IP can handle only 4x4 or 5x3.
>>>>
>>> Even so, the rows, cols approach is more readable IMO.
>>>
>> The key sacan mode is set just by one bit 4X4 or 5X3, Is it really 
>> necessary use rows and cols in this case?
> 
> No, if the hardware only has 2 modes (switchable by a single bit or
> something like that) then an enum is more clear IMO.
> 

ok, I will use just a enum with the two possible cases.

Thanks,

Miguel Aguilar