input_polling_devices design question + adxl34x polling mode patch

[<paul.chavent@fnac.net>]

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

Hi.

I have  a board with an adxl34x (an accelerometer) on a two wire bus. Its interrupt line aren't routed.

So i would like to use the driver in polling mode.

I submit this patch as a beta version of my work.

I tried to reuse the input_polling structure but I'm facing some problems.

The driver has a "rate" attribute that i would like to control when i setup the "interval" attribute of the input_pollling.

And vice versa, when i setup the "interval" attribute i would like to setup the "rate".

So my questions :
 - Is it possible to reimplement a workqueue for this driver only ? As it seems to have been done yet in other drivers, i wonder if it's acceptable, or if we should avoid this practice.
 - I think it would be complicated to have hooks in input and input_polling for calling each other. I wonder if i haven't any design problem.

I would need an advice in order to cleaning this patch please.

Thank for your help.

Paul.



[-- Attachment #2: 0001-Polling-option-for-adxl34x-input-driver.patch --]
[-- Type: application/octet-stream, Size: 6773 bytes --]

From a03ffc414bff628317d191fde4a9e8fe8b6e669c Mon Sep 17 00:00:00 2001
From: Paul Chavent <paul.chavent@fnac.net>
Date: Mon, 11 Apr 2011 18:01:33 +0200
Subject: [PATCH] Polling option for adxl34x input driver.

---
 drivers/input/misc/adxl34x.c |  138 +++++++++++++++++++++++++++++++++++++-----
 1 files changed, 122 insertions(+), 16 deletions(-)

diff --git a/drivers/input/misc/adxl34x.c b/drivers/input/misc/adxl34x.c
index 144ddbd..88b99f4 100644
--- a/drivers/input/misc/adxl34x.c
+++ b/drivers/input/misc/adxl34x.c
@@ -11,6 +11,9 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/input.h>
+#if defined(CONFIG_INPUT_POLLDEV)
+#include <linux/input-polldev.h>
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/slab.h>
@@ -190,6 +193,9 @@ struct axis_triple {
 struct adxl34x {
 	struct device *dev;
 	struct input_dev *input;
+#if defined(CONFIG_INPUT_POLLDEV)
+	struct input_polled_dev *input_polled;
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
 	struct mutex mutex;	/* reentrant protection for struct */
 	struct adxl34x_platform_data pdata;
 	struct axis_triple swcal;
@@ -688,27 +694,75 @@ static void adxl34x_input_close(struct input_dev *input)
 	mutex_unlock(&ac->mutex);
 }
 
+#if defined(CONFIG_INPUT_POLLDEV)
+
+static void adxl34x_input_polled_open(struct input_polled_dev *dev)
+{
+	adxl34x_input_open(dev->input);
+}
+
+static void adxl34x_input_polled_close(struct input_polled_dev *dev)
+{
+	adxl34x_input_close(dev->input);
+}
+
+static void adxl34x_input_polled_poll(struct input_polled_dev *dev)
+{
+	struct adxl34x *ac = input_get_drvdata(dev->input);
+
+	adxl34x_irq(ac->irq, ac);
+}
+
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
+
 struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 			      bool fifo_delay_default,
 			      const struct adxl34x_bus_ops *bops)
 {
 	struct adxl34x *ac;
 	struct input_dev *input_dev;
+#if defined(CONFIG_INPUT_POLLDEV)
+	struct input_polled_dev *input_polled_dev;
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
 	const struct adxl34x_platform_data *pdata;
 	int err, range, i;
 	unsigned char revid;
 
-	if (!irq) {
-		dev_err(dev, "no IRQ?\n");
-		err = -ENODEV;
+
+	ac = kzalloc(sizeof(*ac), GFP_KERNEL);
+	if (!ac) {
+		err = -ENOMEM;
 		goto err_out;
 	}
 
-	ac = kzalloc(sizeof(*ac), GFP_KERNEL);
+	ac->irq = irq;
+
+	if (ac->irq) {
+#if defined(CONFIG_INPUT_POLLDEV)
+		input_polled_dev = 0;
+		ac->input_polled = input_polled_dev;
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
+
 	input_dev = input_allocate_device();
-	if (!ac || !input_dev) {
+		if (!input_dev) {
 		err = -ENOMEM;
-		goto err_free_mem;
+			goto err_free_ac;
+		}
+	} else {
+#if defined(CONFIG_INPUT_POLLDEV)
+		input_polled_dev = input_allocate_polled_device();
+		if (!input_polled_dev) {
+			err = -ENOMEM;
+			goto err_free_ac;
+		}
+		ac->input_polled = input_polled_dev;
+
+		input_dev = input_polled_dev->input;
+#else
+		dev_err(dev, "no IRQ? (perhaps you need to set CONFIG_INPUT_POLLDEV=y)\n");
+		err = -ENODEV;
+		goto err_free_ac;
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
 	}
 
 	ac->fifo_delay = fifo_delay_default;
@@ -725,7 +779,6 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 
 	ac->input = input_dev;
 	ac->dev = dev;
-	ac->irq = irq;
 	ac->bops = bops;
 
 	mutex_init(&ac->mutex);
@@ -743,7 +796,7 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 	default:
 		dev_err(dev, "Failed to probe %s\n", input_dev->name);
 		err = -ENODEV;
-		goto err_free_mem;
+		goto err_free_dev;
 	}
 
 	snprintf(ac->phys, sizeof(ac->phys), "%s/input0", dev_name(dev));
@@ -752,8 +805,15 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 	input_dev->dev.parent = dev;
 	input_dev->id.product = ac->model;
 	input_dev->id.bustype = bops->bustype;
+	if (ac->irq) {
 	input_dev->open = adxl34x_input_open;
 	input_dev->close = adxl34x_input_close;
+	} else {
+#if defined(CONFIG_INPUT_POLLDEV)
+		input_polled_dev->open = adxl34x_input_polled_open;
+		input_polled_dev->close = adxl34x_input_polled_close;
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
+	}
 
 	input_set_drvdata(input_dev, ac);
 
@@ -810,21 +870,44 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 
 	ac->bops->write(dev, POWER_CTL, 0);
 
+	if (ac->irq) {
 	err = request_threaded_irq(ac->irq, NULL, adxl34x_irq,
 				   IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
 				   dev_name(dev), ac);
+
 	if (err) {
 		dev_err(dev, "irq %d busy?\n", ac->irq);
-		goto err_free_mem;
+			goto err_free_dev;
 	}
 
-	err = sysfs_create_group(&dev->kobj, &adxl34x_attr_group);
+		err = input_register_device(input_dev);
 	if (err)
 		goto err_free_irq;
 
-	err = input_register_device(input_dev);
+		err = sysfs_create_group(&dev->kobj, &adxl34x_attr_group);
+		if (err)
+			goto err_unregister_dev;
+	} else {
+#if defined(CONFIG_INPUT_POLLDEV)
+		input_polled_dev->poll = adxl34x_input_polled_poll;
+
+		err = input_register_polled_device(input_polled_dev);
+		if (err)
+			goto err_free_dev;
+
+#if 0
+		err = sysfs_merge_group(&dev->kobj, &adxl34x_attr_group);
 	if (err)
-		goto err_remove_attr;
+			goto err_unregister_dev;
+#else
+#warning "FIXME : test with > 2.6.36"
+		err = sysfs_create_group(&dev->kobj, &adxl34x_attr_group);
+		if (err)
+			goto err_unregister_dev;
+#endif
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
+	}
+
 
 	AC_WRITE(ac, THRESH_TAP, pdata->tap_threshold);
 	AC_WRITE(ac, OFSX, pdata->x_axis_offset);
@@ -885,12 +968,27 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
 
 	return ac;
 
- err_remove_attr:
-	sysfs_remove_group(&dev->kobj, &adxl34x_attr_group);
+ err_unregister_dev:
+	if (ac->irq) {
+		input_unregister_device(input_dev);
+	} else {
+#if defined(CONFIG_INPUT_POLLDEV)
+		input_unregister_polled_device(input_polled_dev);
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
+	}
  err_free_irq:
+	if (ac->irq) {
 	free_irq(ac->irq, ac);
- err_free_mem:
+	}
+ err_free_dev:
+	if (ac->irq) {
 	input_free_device(input_dev);
+	} else {
+#if defined(CONFIG_INPUT_POLLDEV)
+		input_free_polled_device(input_polled_dev);
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
+	}
+ err_free_ac:
 	kfree(ac);
  err_out:
 	return ERR_PTR(err);
@@ -899,9 +997,17 @@ EXPORT_SYMBOL_GPL(adxl34x_probe);
 
 int adxl34x_remove(struct adxl34x *ac)
 {
+	if (ac->irq) {
 	sysfs_remove_group(&ac->dev->kobj, &adxl34x_attr_group);
-	free_irq(ac->irq, ac);
 	input_unregister_device(ac->input);
+		free_irq(ac->irq, ac);
+		input_free_device(ac->input);
+	} else {
+#if defined(CONFIG_INPUT_POLLDEV)
+		input_unregister_polled_device(ac->input_polled);
+		input_free_polled_device(ac->input_polled);
+#endif /* defined(CONFIG_INPUT_POLLDEV) */
+	}
 	dev_dbg(ac->dev, "unregistered accelerometer\n");
 	kfree(ac);
 
-- 
1.6.4