Re: [PATCH 3/3] OMAP3: PM: Wakeup from TWL4030 keypad with OFF mode

[Kevin Hilman <khilman@deeprootsystems.com>]

"Premi, Sanjeev" <premi@ti.com> writes:

>> -----Original Message-----
>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>> Sent: Friday, February 05, 2010 3:22 AM
>> To: Premi, Sanjeev
>> Cc: linux-omap@vger.kernel.org
>> Subject: Re: [PATCH 3/3] OMAP3: PM: Wakeup from TWL4030 
>> keypad with OFF mode
>> 
>> Sanjeev Premi <premi@ti.com> writes:
>> 
>> > This patch allows wakeup from TWL4030 keypad when
>> > OFF mode is hit during suspend.
>> >
>> > Tested on OMAP3EVM.
>> >
>> > Signed-off-by: Sanjeev Premi <premi@ti.com>
>> 
>> The idea here looks good, but there is nothing realy EVM specific here
>> AFAICT, and this exact code could be used on any other board using
>> the T2 keypad, right?  How about a common location?
>
> [sp] The EVM uses SYS_NIRQ for hooking up with the T2 keypad. Not
>      sure of everyone would be doing so. Also, PADCONFs are more
>      applicable for OMAP3, but T2 can be used with other processors
>      as well...
>
>      I did initially inplement these functions in:
>      drivers/input/keyboard/twl4030_keypad.c
>
>      ...but then moved to board specific file.

I know at least SDP and EVM have these hooked up identically, so some common
way to set them up would be preferred.  Maybe a mach-omap2/t2-keypad.c that
could be shared between various board files.  The board files would still
configure the IRQ line and the padconfs used.

Kevin



>> 
>> Also, after looking at this patch, I think the 'void *pstate' member
>> of struct twl4030_keypad_data should be renamed to 'void *data' to
>> be more clear that it's just a pointer.
>
> [sp] Will make the change. However, will wait for your response on the
>      earlier comment before re-submit.
>
> ~sanjeev
>
>> 
>> Kevin
>> 
>> 
>
> [snip]--[snip]