Re: [PATCH 1/1] TWL4030 keypad: keypad lock / unlock

[Onkalo Samu <samu.p.onkalo@nokia.com>]

On Wed, 2009-11-11 at 11:08 +0100, ext Mark Brown wrote:
> On Wed, Nov 11, 2009 at 11:57:36AM +0200, Onkalo Samu wrote:
> > On Tue, 2009-11-10 at 18:26 +0100, ext Dmitry Torokhov wrote:
> 
> > > It seems that Samu's patch is a bit different - it completely disables
> > > the keypad. But I wonder if it needs the special attribute or the same
> > > can be simply achieved by simply closing event device when it is not
> > > needed. Or maybe unbinding driver through sysfs.
> 
> > Yes, purpose is to provide keylock feature which totally disables the
> > keymatrix. This way system is not even aware that some key was pressed.
> > It also helps to save power since unnecessary interrupts are not
> > received.
> 
> OK, that's not what your patch description sounded like - it was talking
> specifically about preventing resume so I thought it was focused on
> suspend and resume.
> 
> > It seems that several userspace programs are reading the device. I have
> > no idea how to get all the users to close the device when keylock is
> > needed. Could be quite big number of changes to here and there in
> > userspace side.
> 
> This sounds like something that should have a generic API for - keylock
> functionality is a very common feature.

Hi

I agree that patch description was much too short and inaccurate.
Description must be updated to cover changes better and to tell clearly
that purpose is to provide master switch for disabling keypad HW without
closing all users of the device.

I'm wondering if the implementation is otherwise ok?
twl4030 keypad is more or less connected to omap based systems and this
kind of disabling and enabling feature is clearly targetted to embedded
systems. One more generic way could be to add ioctl to evdev driver
which calls chip-specific driver for disabling / enabling the keypad.
It is then up to chip specific driver to setup necessary callback
functions which makes actual job. Does this sound bad?

-Samu