[PATCH 0/1] TWL4030 keypad: lock / unlock feature

[PATCH 0/1] TWL4030 keypad: lock / unlock feature

[Samu Onkalo]

Patch adds a sysfs control for disabling and enabling the twl4030 keyboard
at HW level. This is needed to avoid unnecessary system wakeup events because
of accidential key presses.

Power management support added to disable matrix during suspend.

Samu Onkalo (1):
  TWL4030 keypad: keypad lock / unlock

 drivers/input/keyboard/twl4030_keypad.c |  181 +++++++++++++++++++++++++++----
 1 files changed, 158 insertions(+), 23 deletions(-)

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

[Samu Onkalo]

Sysfs interface for disabling and enabling keypad HW and
PM management functions added to twl4030 keypad driver.

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 drivers/input/keyboard/twl4030_keypad.c |  181 +++++++++++++++++++++++++++----
 1 files changed, 158 insertions(+), 23 deletions(-)

diff --git a/drivers/input/keyboard/twl4030_keypad.c b/drivers/input/keyboard/twl4030_keypad.c
index 9a2977c..2b5945a 100644
--- a/drivers/input/keyboard/twl4030_keypad.c
+++ b/drivers/input/keyboard/twl4030_keypad.c
@@ -32,6 +32,7 @@
 #include <linux/input.h>
 #include <linux/platform_device.h>
 #include <linux/i2c/twl4030.h>
+#include <linux/mutex.h>
 
 
 /*
@@ -59,9 +60,12 @@ struct twl4030_keypad {
 	unsigned	n_rows;
 	unsigned	n_cols;
 	unsigned	irq;
+	unsigned	user_disabled:1;
+	unsigned	disable_depth;
 
 	struct device *dbg_dev;
 	struct input_dev *input;
+	struct mutex	mutex;
 };
 
 /*----------------------------------------------------------------------*/
@@ -155,6 +159,24 @@ static int twl4030_kpwrite_u8(struct twl4030_keypad *kp, u8 data, u32 reg)
 	return ret;
 }
 
+static int twl4030_kp_enable_interrupts(struct twl4030_keypad *kp)
+{
+	u8 reg;
+	int ret;
+	/* Enable KP and TO interrupts now. */
+	reg = (u8)~(KEYP_IMR1_KP | KEYP_IMR1_TO);
+	ret = twl4030_kpwrite_u8(kp, reg, KEYP_IMR1);
+	return ret;
+}
+
+static void twl4030_kp_disable_interrupts(struct twl4030_keypad *kp)
+{
+	u8 reg;
+	/* mask all events - we don't care about the result */
+	reg = KEYP_IMR1_MIS | KEYP_IMR1_TO | KEYP_IMR1_LK | KEYP_IMR1_KP;
+	(void)twl4030_kpwrite_u8(kp, reg, KEYP_IMR1);
+}
+
 static inline u16 twl4030_col_xlate(struct twl4030_keypad *kp, u8 col)
 {
 	/* If all bits in a row are active for all coloumns then
@@ -198,25 +220,11 @@ static int twl4030_is_in_ghost_state(struct twl4030_keypad *kp, u16 *key_state)
 	return 0;
 }
 
-static void twl4030_kp_scan(struct twl4030_keypad *kp, bool release_all)
+static void twl4030_kp_report_changes(struct twl4030_keypad *kp, u16 *new_state)
 {
 	struct input_dev *input = kp->input;
-	u16 new_state[TWL4030_MAX_ROWS];
 	int col, row;
 
-	if (release_all)
-		memset(new_state, 0, sizeof(new_state));
-	else {
-		/* check for any changes */
-		int ret = twl4030_read_kp_matrix_state(kp, new_state);
-
-		if (ret < 0)	/* panic ... */
-			return;
-
-		if (twl4030_is_in_ghost_state(kp, new_state))
-			return;
-	}
-
 	/* check for changes and print those */
 	for (row = 0; row < kp->n_rows; row++) {
 		int changed = new_state[row] ^ kp->kp_state[row];
@@ -244,6 +252,79 @@ static void twl4030_kp_scan(struct twl4030_keypad *kp, bool release_all)
 	input_sync(input);
 }
 
+static inline int twl4030_kp_disabled(struct twl4030_keypad *kp)
+{
+	return kp->disable_depth != 0;
+}
+
+static void twl4030_kp_enable(struct twl4030_keypad *kp)
+{
+	BUG_ON(!twl4030_kp_disabled(kp));
+	if (--kp->disable_depth == 0) {
+		enable_irq(kp->irq);
+		twl4030_kp_enable_interrupts(kp);
+	}
+}
+
+static int twl4030_kp_scan(struct twl4030_keypad *kp, u16 *new_state)
+{
+	/* check for any changes */
+	int ret = twl4030_read_kp_matrix_state(kp, new_state);
+	if (ret < 0)    /* panic ... */
+		return ret;
+
+	return twl4030_is_in_ghost_state(kp, new_state);
+}
+
+static void twl4030_kp_disable(struct twl4030_keypad *kp)
+{
+	u16 new_state[TWL4030_MAX_ROWS];
+
+	if (kp->disable_depth++ == 0) {
+		memset(new_state, 0, sizeof(new_state));
+		twl4030_kp_report_changes(kp, new_state);
+		twl4030_kp_disable_interrupts(kp);
+		disable_irq(kp->irq);
+	}
+}
+
+static ssize_t twl4030_kp_disable_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct twl4030_keypad *kp = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%u\n", twl4030_kp_disabled(kp));
+}
+
+static ssize_t twl4030_kp_disable_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct twl4030_keypad *kp = dev_get_drvdata(dev);
+	long i = 0;
+	int ret;
+
+	ret = strict_strtoul(buf, 10, &i);
+	if (ret)
+		return -EINVAL;
+	i = !!i;
+
+	mutex_lock(&kp->mutex);
+	if (i == kp->user_disabled) {
+		mutex_unlock(&kp->mutex);
+		return count;
+	}
+	kp->user_disabled = i;
+
+	if (i)
+		twl4030_kp_disable(kp);
+	else
+		twl4030_kp_enable(kp);
+
+	mutex_unlock(&kp->mutex);
+	return count;
+}
+
 /*
  * Keypad interrupt handler
  */
@@ -252,6 +333,7 @@ static irqreturn_t do_kp_irq(int irq, void *_kp)
 	struct twl4030_keypad *kp = _kp;
 	u8 reg;
 	int ret;
+	u16 new_state[TWL4030_MAX_ROWS];
 
 #ifdef CONFIG_LOCKDEP
 	/* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
@@ -264,12 +346,22 @@ static irqreturn_t do_kp_irq(int irq, void *_kp)
 	/* Read & Clear TWL4030 pending interrupt */
 	ret = twl4030_kpread(kp, &reg, KEYP_ISR1, 1);
 
+	mutex_lock(&kp->mutex);
+	if (twl4030_kp_disabled(kp)) {
+		mutex_unlock(&kp->mutex);
+		return IRQ_HANDLED;
+	}
+
 	/* Release all keys if I2C has gone bad or
 	 * the KEYP has gone to idle state */
 	if (ret >= 0 && (reg & KEYP_IMR1_KP))
-		twl4030_kp_scan(kp, false);
+		twl4030_kp_scan(kp, new_state);
 	else
-		twl4030_kp_scan(kp, true);
+		memset(new_state, 0, sizeof(new_state));
+
+	twl4030_kp_report_changes(kp, new_state);
+
+	mutex_unlock(&kp->mutex);
 
 	return IRQ_HANDLED;
 }
@@ -327,6 +419,9 @@ static int __devinit twl4030_kp_program(struct twl4030_keypad *kp)
 	return 0;
 }
 
+static DEVICE_ATTR(disable_kp, 0664, twl4030_kp_disable_show,
+		  twl4030_kp_disable_store);
+
 /*
  * Registers keypad device with input subsystem
  * and configures TWL4030 keypad registers
@@ -337,7 +432,6 @@ static int __devinit twl4030_kp_probe(struct platform_device *pdev)
 	const struct matrix_keymap_data *keymap_data = pdata->keymap_data;
 	struct twl4030_keypad *kp;
 	struct input_dev *input;
-	u8 reg;
 	int error;
 
 	if (!pdata || !pdata->rows || !pdata->cols ||
@@ -353,6 +447,8 @@ static int __devinit twl4030_kp_probe(struct platform_device *pdev)
 		goto err1;
 	}
 
+	mutex_init(&kp->mutex);
+
 	/* Get the debug Device */
 	kp->dbg_dev = &pdev->dev;
 	kp->input = input;
@@ -411,18 +507,20 @@ static int __devinit twl4030_kp_probe(struct platform_device *pdev)
 	}
 
 	/* Enable KP and TO interrupts now. */
-	reg = (u8) ~(KEYP_IMR1_KP | KEYP_IMR1_TO);
-	if (twl4030_kpwrite_u8(kp, reg, KEYP_IMR1)) {
-		error = -EIO;
+	error = twl4030_kp_enable_interrupts(kp);
+	if (error < 0)
+		goto err4;
+
+	error = device_create_file(&pdev->dev, &dev_attr_disable_kp);
+	if (error < 0)
 		goto err4;
-	}
 
 	platform_set_drvdata(pdev, kp);
 	return 0;
 
 err4:
 	/* mask all events - we don't care about the result */
-	(void) twl4030_kpwrite_u8(kp, 0xff, KEYP_IMR1);
+	twl4030_kp_disable_interrupts(kp);
 err3:
 	free_irq(kp->irq, NULL);
 err2:
@@ -441,11 +539,45 @@ static int __devexit twl4030_kp_remove(struct platform_device *pdev)
 	free_irq(kp->irq, kp);
 	input_unregister_device(kp->input);
 	platform_set_drvdata(pdev, NULL);
+	device_remove_file(&pdev->dev, &dev_attr_disable_kp);
 	kfree(kp);
 
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int twl4030_kp_suspend(struct platform_device *pdev, pm_message_t mesg)
+{
+	struct twl4030_keypad *kp = dev_get_drvdata(&pdev->dev);
+	mutex_lock(&kp->mutex);
+	twl4030_kp_disable(kp);
+	mutex_unlock(&kp->mutex);
+	return 0;
+}
+
+static int twl4030_kp_resume(struct platform_device *pdev)
+{
+	struct twl4030_keypad *kp = dev_get_drvdata(&pdev->dev);
+	mutex_lock(&kp->mutex);
+	twl4030_kp_enable(kp);
+	mutex_unlock(&kp->mutex);
+	return 0;
+}
+
+static void twl4030_kp_shutdown(struct platform_device *pdev)
+{
+	struct twl4030_keypad *kp = dev_get_drvdata(&pdev->dev);
+	/* Disable controller */
+	twl4030_kpwrite_u8(kp, 0, KEYP_CTRL);
+}
+#else
+
+#define twl4030_kp_suspend	NULL
+#define twl4030_kp_resume	NULL
+#define twl4030_kp_shutdown	NULL
+
+#endif /* CONFIG_PM */
+
 /*
  * NOTE: twl4030 are multi-function devices connected via I2C.
  * So this device is a child of an I2C parent, thus it needs to
@@ -455,6 +587,9 @@ static int __devexit twl4030_kp_remove(struct platform_device *pdev)
 static struct platform_driver twl4030_kp_driver = {
 	.probe		= twl4030_kp_probe,
 	.remove		= __devexit_p(twl4030_kp_remove),
+	.suspend	= twl4030_kp_suspend,
+	.resume		= twl4030_kp_resume,
+	.shutdown	= twl4030_kp_shutdown,
 	.driver		= {
 		.name	= "twl4030_keypad",
 		.owner	= THIS_MODULE,
-- 
1.5.6.3

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

[Mark Brown]

On Tue, Nov 10, 2009 at 10:24:08AM +0200, Samu Onkalo wrote:
> Sysfs interface for disabling and enabling keypad HW and
> PM management functions added to twl4030 keypad driver.

Might be nice to have the longer explanation in your cover letter in the
patch description...

> Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>

Shouldn't this be done via the existing device wakeup API?  That also
presents a sysfs control (power/wakeup IIRC).  The driver calls
device_init_wakeup() to flag support for this at startup then checks
device_may_wakeup() when suspending and configures the hardware
appropriately.

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

[Dmitry Torokhov]

On Tue, Nov 10, 2009 at 01:43:30PM +0000, Mark Brown wrote:
> On Tue, Nov 10, 2009 at 10:24:08AM +0200, Samu Onkalo wrote:
> > Sysfs interface for disabling and enabling keypad HW and
> > PM management functions added to twl4030 keypad driver.
> 
> Might be nice to have the longer explanation in your cover letter in the
> patch description...
> 
> > Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> 
> Shouldn't this be done via the existing device wakeup API?  That also
> presents a sysfs control (power/wakeup IIRC).  The driver calls
> device_init_wakeup() to flag support for this at startup then checks
> device_may_wakeup() when suspending and configures the hardware
> appropriately.

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.

Overall it seems that every input device used in embedded has it's own
way of disabling itself, we need a generic solution... Maybe
userspace-controlled PM is the answer).

-- 
Dmitry

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

[Onkalo Samu]

On Tue, 2009-11-10 at 18:26 +0100, ext Dmitry Torokhov wrote:
> On Tue, Nov 10, 2009 at 01:43:30PM +0000, Mark Brown wrote:
> > On Tue, Nov 10, 2009 at 10:24:08AM +0200, Samu Onkalo wrote:
> > > Sysfs interface for disabling and enabling keypad HW and
> > > PM management functions added to twl4030 keypad driver.
> > 
> > Might be nice to have the longer explanation in your cover letter in the
> > patch description...

Definitely true. Thanks for advice.

> > 
> > > Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> > 
> > Shouldn't this be done via the existing device wakeup API?  That also
> > presents a sysfs control (power/wakeup IIRC).  The driver calls
> > device_init_wakeup() to flag support for this at startup then checks
> > device_may_wakeup() when suspending and configures the hardware
> > appropriately.
> 
> 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.

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.

A master switch in sysfs would be quite simple way to control the
keylock. Decision can be done in one place and all the users of the
device are locked or unlocked. 

I'm not familiar with bind / unbind. I tried to play with this.
It seems that I managed to unbind the driver since the /dev/input/eventx
disappeared. Similarly bind restored the device node. Userspace
program which tried to access device node was not happy at all after
unbind.

> Overall it seems that every input device used in embedded has it's own
> way of disabling itself, we need a generic solution... Maybe
> userspace-controlled PM is the answer).

That is something which should be somehow handled. Depending on what is
happening at the userspace, there can be different needs for input
devices. Current way will be a mess since every driver has its own
tricks to save power. But power saving is really needed for embedded
devices.

Regards,
Samu

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

[Mark Brown]

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.

> > Overall it seems that every input device used in embedded has it's own
> > way of disabling itself, we need a generic solution... Maybe
> > userspace-controlled PM is the answer).

> That is something which should be somehow handled. Depending on what is
> happening at the userspace, there can be different needs for input
> devices. Current way will be a mess since every driver has its own
> tricks to save power. But power saving is really needed for embedded
> devices.

It doesn't look that bad to me, to be honest - mostly it seems to be a
case of shutting down the device as much as possible when it's closed
plus other things that the particular device can get away with without
impacting actual operation.

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

[Onkalo Samu]

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

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

[Mark Brown]

On Mon, Nov 23, 2009 at 02:05:55PM +0200, Onkalo Samu wrote:

> twl4030 keypad is more or less connected to omap based systems and this
> kind of disabling and enabling feature is clearly targetted to embedded

There's rather a lot of embedded systems and off the shelf application
stacks for them so having an interface that isn't driver-specific is
also useful for them.

> 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?

Something along these lines which makes the user space interface part of
the common code, enabled if driver provides functions to implement the
behaviour does seem like a good approach.

sysfs might be easier to use than an ioctl(), though.

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

[Onkalo Samu]

On Mon, 2009-11-23 at 14:54 +0100, ext Mark Brown wrote:
> On Mon, Nov 23, 2009 at 02:05:55PM +0200, Onkalo Samu wrote:
> 
> > twl4030 keypad is more or less connected to omap based systems and this
> > kind of disabling and enabling feature is clearly targetted to embedded
> 
> There's rather a lot of embedded systems and off the shelf application
> stacks for them so having an interface that isn't driver-specific is
> also useful for them.
> 
> > 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?
> 
> Something along these lines which makes the user space interface part of
> the common code, enabled if driver provides functions to implement the
> behaviour does seem like a good approach.
> 
> sysfs might be easier to use than an ioctl(), though.

Something like that:
diff --git a/drivers/input/input.c b/drivers/input/input.c
index cc763c9..94824a4 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1305,6 +1305,49 @@ static int input_dev_uevent(struct device
*device, struct kobj_uevent_env *env)
 		}						\
 	} while (0)
 
+/* SYSFS */
+static ssize_t input_dev_get_disable(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct input_dev *input_dev = to_input_dev(dev);
+	return sprintf(buf, "%u\n", input_dev->disabled);
+}
+
+static ssize_t input_dev_set_disable(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct input_dev *input_dev = to_input_dev(dev);
+	long i = 0;
+	int ret;
+
+	if (!(input_dev->disable && input_dev->enable))
+		return 0;
+
+	ret = strict_strtoul(buf, 0, &i);
+	if (ret)
+		return -EINVAL;
+	i = !!i;
+
+	mutex_lock(&input_dev->mutex);
+	if (input_dev->disabled == i) {
+		mutex_unlock(&input_dev->mutex);
+		return count;
+	}
+	input_dev->disabled = i;

should this be a counter or boolean? Or should this be in driver itself
and not in input system.

+
+	if (i)
+		input_dev->disable(input_dev);
+	else
+		input_dev->enable(input_dev);
+
+	mutex_unlock(&input_dev->mutex);
+	return count;
+}
+
+static DEVICE_ATTR(disable, S_IRUGO | S_IWUSR, input_dev_get_disable,
+		input_dev_set_disable);
+
 #ifdef CONFIG_PM
 static void input_dev_reset(struct input_dev *dev, bool activate)
 {
@@ -1539,6 +1582,13 @@ int input_register_device(struct input_dev *dev)
 		return error;
 	}
 
+	error = device_create_file(&dev->dev, &dev_attr_disable);
+	if (error < 0) {
+		device_del(&dev->dev);
+		mutex_unlock(&input_mutex);
+		return error;
+	}
+
 	list_add_tail(&dev->node, &input_dev_list);
 
 	list_for_each_entry(handler, &input_handler_list, node)
@@ -1578,6 +1628,8 @@ void input_unregister_device(struct input_dev
*dev)
 
 	mutex_unlock(&input_mutex);
 
+	device_remove_file(&dev->dev, &dev_attr_disable);
+
 	device_unregister(&dev->dev);
 }
 EXPORT_SYMBOL(input_unregister_device);
diff --git a/include/linux/input.h b/include/linux/input.h
index 0ccfc30..e6e1098 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -1048,6 +1048,11 @@ struct ff_effect {
  *	or EV_SND. The device is expected to carry out the requested
  *	action (turn on a LED, play sound, etc.) The call is protected
  *	by @event_lock and must not sleep
+ * @enable: method is called when user wants to enable driver which was
+ *	disabled using disable-method (optional).
+ * @disable: method is called when user wants to temporarily disable
the
+ *	driver (example: tell keyboard driver to disable scanning at
+ *	HW level) (optional).
  * @grab: input handle that currently has the device grabbed (via
  *	EVIOCGRAB ioctl). When a handle grabs a device it becomes sole
  *	recipient for all input events coming from the device
@@ -1116,6 +1121,8 @@ struct input_dev {
 	void (*close)(struct input_dev *dev);
 	int (*flush)(struct input_dev *dev, struct file *file);
 	int (*event)(struct input_dev *dev, unsigned int type, unsigned int
code, int value);
+	int (*enable)(struct input_dev *dev);
+	int (*disable)(struct input_dev *dev);
 
 	struct input_handle *grab;
 
@@ -1124,6 +1131,7 @@ struct input_dev {
 
 	unsigned int users;
 	bool going_away;
+	bool disabled;
 
 	struct device dev;
 

TWL4030 driver just setup the pointers to HW control functions.

-Samu
 

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

[Mark Brown]

On Wed, Nov 25, 2009 at 04:22:17PM +0200, Onkalo Samu wrote:

> Something like that:

The userspace interface looks reasonable to me but...

> +		return count;
> +	}
> +	input_dev->disabled = i;

> should this be a counter or boolean? Or should this be in driver itself
> and not in input system.

I'd expect a boolean - userspace can refcount itself if it needs to, but
it'd be strange if configuration written into sysfs didn't always take
effect.

> +	if (i)
> +		input_dev->disable(input_dev);
> +	else
> +		input_dev->enable(input_dev);

...it'd seem more natural to have the sense of the sysfs file be the
other way around, but that is a matter of taste.  It just feels better
to me for "true" to mean "on".

I don't know what the best way to sync with the device being open and
in use is, though.