[PATCH] input: Enable / disable methods

[PATCH] input: Enable / disable methods

[Samu Onkalo]

First patch adds a generic way for userspace to enable and
disable an input device. Userspace interface is a sysfs entry.
Device is enabled by writing '1' to enable entry and disabled by
writing '0' to enabled entry. Current state can be read from the entry.
Actual disabling happens via callback functions. If the device driver doesn't
provide callback functions, write to sysfs entry returns an error code ENOSYS.

Purpose is to provide a generic way to force some input device to disabled
or enabled state regardless of the number of the users of that input device.
When disabling of the device happens in the driver level, it is possible
to turn off the corresponfing HW block to prevent unwanted activity.
For example system wide keypad locking feature can utilize this feature.
When several user space programs keep the input device open, one
user space program can set input device to disabled state. 

Second patch utilizes new feature and adds HW level disable / enable feature
to twl4030_keypad driver. Regardless of the number of users one control
program can temporarily disable all keypad interrupts from the twl4030 keypad
block. Keypad HW is also disabled during suspend mode.

Samu Onkalo (2):
  input: enable / disable methods with sysfs entry
  input: twl4030_keypad enable / disable feature

 drivers/input/input.c                   |   45 +++++++++
 drivers/input/keyboard/twl4030_keypad.c |  153 ++++++++++++++++++++++++++-----
 include/linux/input.h                   |    8 ++
 3 files changed, 183 insertions(+), 23 deletions(-)

[PATCH] input: enable / disable methods with sysfs entry

[Samu Onkalo]

Add optional enable and disable methods to input system.
Add sysfs entry for enabling and disabling input device.

When registering a device to input system, device can provide
optional enable and disable methods. State is set to enabled by default.
Opening / closing the input device doesn't change the state.
If the callback functions are not provided, sysfs entry returns
ENOSYS error code.

Purpose:
Several user space applications can keep device open at the same time.
One user space application can enable and disable the device
via sysfs entry. Depending on the device driver, disable / enable
functions can totally turn off the HW or prevent interrupts to
come trough.

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 drivers/input/input.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/input.h |    8 ++++++++
 2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 5d6421b..3cd47b4 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1077,11 +1077,54 @@ static ssize_t input_dev_show_modalias(struct device *dev,
 }
 static DEVICE_ATTR(modalias, S_IRUGO, input_dev_show_modalias, NULL);
 
+static ssize_t input_dev_get_enable(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->enabled);
+}
+
+static ssize_t input_dev_set_enable(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct input_dev *input_dev = to_input_dev(dev);
+	long i;
+	int ret;
+
+	if (!(input_dev->disable && input_dev->enable))
+		return -ENOSYS;
+
+	ret = strict_strtoul(buf, 0, &i);
+	if (ret)
+		return -EINVAL;
+	i = !!i;
+
+	mutex_lock(&input_dev->mutex);
+	if (input_dev->enabled == i) {
+		mutex_unlock(&input_dev->mutex);
+		return count;
+	}
+	input_dev->enabled = i;
+
+	if (i)
+		input_dev->enable(input_dev);
+	else
+		input_dev->disable(input_dev);
+
+	mutex_unlock(&input_dev->mutex);
+	return count;
+}
+
+static DEVICE_ATTR(enable, S_IRUGO | S_IWUSR, input_dev_get_enable,
+		input_dev_set_enable);
+
 static struct attribute *input_dev_attrs[] = {
 	&dev_attr_name.attr,
 	&dev_attr_phys.attr,
 	&dev_attr_uniq.attr,
 	&dev_attr_modalias.attr,
+	&dev_attr_enable.attr,
 	NULL
 };
 
@@ -1539,6 +1582,8 @@ int input_register_device(struct input_dev *dev)
 		return error;
 	}
 
+	dev->enabled = true;
+
 	list_add_tail(&dev->node, &input_dev_list);
 
 	list_for_each_entry(handler, &input_handler_list, node)
diff --git a/include/linux/input.h b/include/linux/input.h
index db563bb..31b462b 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -1057,6 +1057,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
@@ -1125,6 +1130,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;
 
@@ -1133,6 +1140,7 @@ struct input_dev {
 
 	unsigned int users;
 	bool going_away;
+	bool enabled;
 
 	struct device dev;
 
-- 
1.6.0.4

[PATCH] input: twl4030_keypad enable / disable feature

[Samu Onkalo]

Implements HW level enable / disable functionality to twl4030_keypad
driver. Disable functionality turns interrupt generation off from
keypad driver which effectively blocks all keypad activity.
Enable functions allows interrupt handling again.
If the key was pressed while keypad is re-enabled, event is not registered.
A new key must be pressed or currently pressed key must be released.

This functionality is controlled via input subsystem. twl4030_keypad
driver sets up callback functions for input subsystem.

Keypad is also blocked during suspend mode.

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

diff --git a/drivers/input/keyboard/twl4030_keypad.c b/drivers/input/keyboard/twl4030_keypad.c
index 9a2977c..18468d9 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,11 @@ struct twl4030_keypad {
 	unsigned	n_rows;
 	unsigned	n_cols;
 	unsigned	irq;
+	unsigned	disable_depth;
 
 	struct device *dbg_dev;
 	struct input_dev *input;
+	struct mutex	mutex;
 };
 
 /*----------------------------------------------------------------------*/
@@ -155,6 +158,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 +219,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 +251,60 @@ 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)
+{
+	mutex_lock(&kp->mutex);
+	WARN_ON(!twl4030_kp_disabled(kp));
+	if (--kp->disable_depth == 0) {
+		enable_irq(kp->irq);
+		twl4030_kp_enable_interrupts(kp);
+	}
+	mutex_unlock(&kp->mutex);
+}
+
+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];
+
+	mutex_lock(&kp->mutex);
+	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);
+	}
+	mutex_unlock(&kp->mutex);
+}
+
+static int twl4030_disable(struct input_dev *dev)
+{
+	struct twl4030_keypad *kp = dev_get_drvdata(dev->dev.parent);
+	twl4030_kp_disable(kp);
+	return 0;
+}
+
+static int twl4030_enable(struct input_dev *dev)
+{
+	struct twl4030_keypad *kp = dev_get_drvdata(dev->dev.parent);
+	twl4030_kp_enable(kp);
+	return 0;
+}
+
 /*
  * Keypad interrupt handler
  */
@@ -252,6 +313,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 +326,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;
 }
@@ -337,7 +409,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 +424,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;
@@ -379,6 +452,9 @@ static int __devinit twl4030_kp_probe(struct platform_device *pdev)
 	input->id.product	= 0x0001;
 	input->id.version	= 0x0003;
 
+	input->enable		= twl4030_enable;
+	input->disable		= twl4030_disable;
+
 	input->keycode		= kp->keymap;
 	input->keycodesize	= sizeof(kp->keymap[0]);
 	input->keycodemax	= ARRAY_SIZE(kp->keymap);
@@ -411,18 +487,17 @@ 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;
-	}
 
 	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:
@@ -446,6 +521,35 @@ static int __devexit twl4030_kp_remove(struct platform_device *pdev)
 	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);
+	twl4030_kp_disable(kp);
+	return 0;
+}
+
+static int twl4030_kp_resume(struct platform_device *pdev)
+{
+	struct twl4030_keypad *kp = dev_get_drvdata(&pdev->dev);
+	twl4030_kp_enable(kp);
+	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 +559,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.6.0.4

Re: [PATCH] input: enable / disable methods with sysfs entry

[Mark Brown]

On Thu, Dec 03, 2009 at 12:58:19PM +0200, Samu Onkalo wrote:
> Add optional enable and disable methods to input system.
> Add sysfs entry for enabling and disabling input device.

> When registering a device to input system, device can provide
> optional enable and disable methods. State is set to enabled by default.
> Opening / closing the input device doesn't change the state.
> If the callback functions are not provided, sysfs entry returns
> ENOSYS error code.

I'm wondering if it might be nice to provide a default implementation
which causes the input core to ignore the events generated by the device
if the driver doesn't support being enabled or disabled?  Obviously this
wouldn't be of any benefit in preventing resume or providing power
saving but it would give userspace the same effect which might be useful
to it for UI purposes.

Re: [PATCH] input: enable / disable methods with sysfs entry

[Joonyoung Shim]

On 12/3/2009 8:08 PM, Mark Brown wrote:
> On Thu, Dec 03, 2009 at 12:58:19PM +0200, Samu Onkalo wrote:
>> Add optional enable and disable methods to input system.
>> Add sysfs entry for enabling and disabling input device.
> 
>> When registering a device to input system, device can provide
>> optional enable and disable methods. State is set to enabled by default.
>> Opening / closing the input device doesn't change the state.
>> If the callback functions are not provided, sysfs entry returns
>> ENOSYS error code.
> 
> I'm wondering if it might be nice to provide a default implementation
> which causes the input core to ignore the events generated by the device
> if the driver doesn't support being enabled or disabled?  Obviously this
> wouldn't be of any benefit in preventing resume or providing power
> saving but it would give userspace the same effect which might be useful
> to it for UI purposes.

I think a default implementation of the input core for this is useful 
ex. at gpio-keys driver.

Re: [PATCH] input: enable / disable methods with sysfs entry

[Ferenc Wagner]

Joonyoung Shim <jy0922.shim@samsung.com> writes:

> On 12/3/2009 8:08 PM, Mark Brown wrote:
>
>> On Thu, Dec 03, 2009 at 12:58:19PM +0200, Samu Onkalo wrote:
>>
>>> Add optional enable and disable methods to input system.
>>> Add sysfs entry for enabling and disabling input device.
>>>
>>> When registering a device to input system, device can provide
>>> optional enable and disable methods. State is set to enabled by default.
>>> Opening / closing the input device doesn't change the state.
>>> If the callback functions are not provided, sysfs entry returns
>>> ENOSYS error code.
>> 
>> I'm wondering if it might be nice to provide a default implementation
>> which causes the input core to ignore the events generated by the device
>> if the driver doesn't support being enabled or disabled?  Obviously this
>> wouldn't be of any benefit in preventing resume or providing power
>> saving but it would give userspace the same effect which might be useful
>> to it for UI purposes.
>
> I think a default implementation of the input core for this is useful 
> ex. at gpio-keys driver.

Mika Westerberg is proposing an actual device specific implementation
for gpio-keys right now.  The implementation details are not decided
yet, but unifying the interface would be preferable, so Cc-ing him.
Do you work at different departments? :)
-- 
Regards,
Feri.

Re: [PATCH] input: enable / disable methods with sysfs entry

[Mika Westerberg]

On Thu, Dec 03, 2009 at 04:20:23PM +0100, Ferenc Wagner wrote:
> Joonyoung Shim <jy0922.shim@samsung.com> writes:
> 
> > On 12/3/2009 8:08 PM, Mark Brown wrote:
> >
> >> On Thu, Dec 03, 2009 at 12:58:19PM +0200, Samu Onkalo wrote:
> >>
> >>> Add optional enable and disable methods to input system.
> >>> Add sysfs entry for enabling and disabling input device.
> >>>
> >>> When registering a device to input system, device can provide
> >>> optional enable and disable methods. State is set to enabled by default.
> >>> Opening / closing the input device doesn't change the state.
> >>> If the callback functions are not provided, sysfs entry returns
> >>> ENOSYS error code.
> >> 
> >> I'm wondering if it might be nice to provide a default implementation
> >> which causes the input core to ignore the events generated by the device
> >> if the driver doesn't support being enabled or disabled?  Obviously this
> >> wouldn't be of any benefit in preventing resume or providing power
> >> saving but it would give userspace the same effect which might be useful
> >> to it for UI purposes.
> >
> > I think a default implementation of the input core for this is useful 
> > ex. at gpio-keys driver.
> 
> Mika Westerberg is proposing an actual device specific implementation
> for gpio-keys right now.  The implementation details are not decided
> yet, but unifying the interface would be preferable, so Cc-ing him.

Unifying these would be nice. However, in the gpio-keys patch the
idea is that we could disable separate GPIO lines from the device,
not the whole device. Maybe something like generic sysfs entry for
the input device to disable the whole device /and/ also possibility
to disable single events or something like that?

> Do you work at different departments? :)

Well we work at different locations :)

Thanks,
MW