linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] input: Add support for Kionix KXTJ9 accelerometer
@ 2011-05-26 22:40 Chris Hudson
  2011-05-31  8:03 ` Dmitry Torokhov
  0 siblings, 1 reply; 4+ messages in thread
From: Chris Hudson @ 2011-05-26 22:40 UTC (permalink / raw)
  To: linux-input; +Cc: dmitry.torokhov, jonathan.cameron, linux-kernel, Chris Hudson

Quite a few changes this time around (thanks Jonathan and Dmitry!):
- The sysfs node for poll_interval was changed to emulate input_polldev.  I
  could not actually use input_polldev because some commands need to be sent
  to the hardware when the poll interval is changed.
- Locking cleaned up throughout
- Cleared up distinction between IRQ and polling modes: if client->irq, then
  irq mode is enabled.  Removed register bits required for irq from the header;
  these will be set in the driver if client->irq is populated.
- Put .suspend and .resume into struct dev_pm_ops.
- Many other smaller changes, unnecessary variables removed, etc.

Signed-off-by: Chris Hudson <chudson@kionix.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/input/misc/Kconfig  |   10 +
 drivers/input/misc/Makefile |    1 +
 drivers/input/misc/kxtj9.c  |  606 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/input/kxtj9.h |   76 ++++++
 4 files changed, 693 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/misc/kxtj9.c
 create mode 100644 include/linux/input/kxtj9.h

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index f9cf088..567f3d2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -467,4 +467,14 @@ config INPUT_XEN_KBDDEV_FRONTEND
 	  To compile this driver as a module, choose M here: the
 	  module will be called xen-kbdfront.
 
+config INPUT_KXTJ9
+	tristate "Kionix KXTJ9 tri-axis digital accelerometer"
+	depends on I2C && SYSFS
+	help
+	  If you say yes here you get support for the Kionix KXTJ9 digital
+	  tri-axis accelerometer.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called kxtj9.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index e3f7984..f2477ef 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_INPUT_DM355EVM)		+= dm355evm_keys.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
 obj-$(CONFIG_INPUT_KEYSPAN_REMOTE)	+= keyspan_remote.o
+obj-$(CONFIG_INPUT_KXTJ9)		+= kxtj9.o
 obj-$(CONFIG_INPUT_M68K_BEEP)		+= m68kspkr.o
 obj-$(CONFIG_INPUT_MAX8925_ONKEY)	+= max8925_onkey.o
 obj-$(CONFIG_INPUT_PCAP)		+= pcap_keys.o
diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
new file mode 100644
index 0000000..ef83182
--- /dev/null
+++ b/drivers/input/misc/kxtj9.c
@@ -0,0 +1,606 @@
+/*
+ * Copyright (C) 2011 Kionix, Inc.
+ * Written by Chris Hudson <chudson@kionix.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ * 02111-1307, USA
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <linux/input/kxtj9.h>
+
+#define NAME			"kxtj9"
+#define G_MAX			8000
+/* OUTPUT REGISTERS */
+#define XOUT_L			0x06
+#define WHO_AM_I		0x0F
+/* CONTROL REGISTERS */
+#define INT_REL			0x1A
+#define CTRL_REG1		0x1B
+#define INT_CTRL1		0x1E
+#define DATA_CTRL		0x21
+/* CONTROL REGISTER 1 BITS */
+#define PC1_OFF			0x7F
+#define PC1_ON			(1 << 7)
+/* Data ready funtion enable bit: set during probe if using irq mode */
+#define DRDYE			(1 << 5)
+/* INTERRUPT CONTROL REGISTER 1 BITS */
+/* Set these during probe if using irq mode */
+#define KXTJ9_IEL		(1 << 3)
+#define KXTJ9_IEA		(1 << 4)
+#define KXTJ9_IEN		(1 << 5)
+/* INPUT_ABS CONSTANTS */
+#define FUZZ			3
+#define FLAT			3
+/* RESUME STATE INDICES */
+#define RES_DATA_CTRL		0
+#define RES_CTRL_REG1		1
+#define RES_INT_CTRL1		2
+#define RESUME_ENTRIES		3
+
+/*
+ * The following table lists the maximum appropriate poll interval for each
+ * available output data rate.
+ */
+static const struct {
+	unsigned int cutoff;
+	u8 mask;
+} kxtj9_odr_table[] = {
+	{
+	3,	ODR800F}, {
+	5,	ODR400F}, {
+	10,	ODR200F}, {
+	20,	ODR100F}, {
+	40,	ODR50F}, {
+	80,	ODR25F}, {
+	0,	ODR12_5F},
+};
+
+struct kxtj9_data {
+	struct i2c_client *client;
+	struct kxtj9_platform_data pdata;
+	struct mutex lock;
+	struct delayed_work input_work;
+	struct input_dev *input_dev;
+
+	bool hw_initialized;
+	bool enabled;
+	u8 shift;
+	u8 resume[RESUME_ENTRIES];
+};
+
+static int kxtj9_i2c_read(struct kxtj9_data *tj9, u8 addr, u8 *data, int len)
+{
+	int err;
+
+	struct i2c_msg msgs[] = {
+		{
+		 .addr = tj9->client->addr,
+		 .flags = tj9->client->flags,
+		 .len = 1,
+		 .buf = &addr,
+		 },
+		{
+		 .addr = tj9->client->addr,
+		 .flags = tj9->client->flags | I2C_M_RD,
+		 .len = len,
+		 .buf = data,
+		 },
+	};
+	err = i2c_transfer(tj9->client->adapter, msgs, 2);
+
+	return err;
+}
+
+static int kxtj9_verify(struct kxtj9_data *tj9)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(tj9->client, WHO_AM_I);
+	if (ret < 0)
+		dev_err(&tj9->client->dev, "read err int source\n");
+	if (ret != 0x06)
+		ret = -EIO;
+
+	return ret;
+}
+
+static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
+{
+	switch (new_g_range) {
+	case KXTJ9_G_2G:
+		tj9->shift = SHIFT_ADJ_2G;
+		break;
+	case KXTJ9_G_4G:
+		tj9->shift = SHIFT_ADJ_4G;
+		break;
+	case KXTJ9_G_8G:
+		tj9->shift = SHIFT_ADJ_8G;
+		break;
+	default:
+		return -EINVAL;
+	}
+	tj9->resume[RES_CTRL_REG1] = (tj9->resume[RES_CTRL_REG1] & 0xE7)
+								| new_g_range;
+
+	return 0;
+}
+
+static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
+{
+	int err;
+	int i;
+	u8 config;
+	u8 temp = 0;
+
+	/* Use the lowest ODR that can support the requested poll interval */
+	for (i = 0; i < ARRAY_SIZE(kxtj9_odr_table); i++) {
+		config = kxtj9_odr_table[i].mask;
+		if (poll_interval < kxtj9_odr_table[i].cutoff)
+			break;
+	}
+
+	if (tj9->enabled == true) {
+		err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, temp);
+		if (err < 0)
+			return err;
+		err = i2c_smbus_write_byte_data(tj9->client, DATA_CTRL, config);
+		if (err < 0)
+			return err;
+		temp = tj9->resume[RES_CTRL_REG1];
+		err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, temp);
+		if (err < 0)
+			return err;
+		/* Cancel and restart polling if not in irq mode */
+		if (!tj9->client->irq) {
+			cancel_delayed_work_sync(&tj9->input_work);
+			schedule_delayed_work(&tj9->input_work,
+					msecs_to_jiffies(poll_interval));
+		}
+	}
+	tj9->resume[RES_DATA_CTRL] = config;
+
+	return 0;
+}
+
+static int kxtj9_hw_init(struct kxtj9_data *tj9)
+{
+	int err;
+	u8 buf = 0;
+
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
+	if (err < 0)
+		return err;
+	err = i2c_smbus_write_byte_data(tj9->client, DATA_CTRL,
+						tj9->resume[RES_DATA_CTRL]);
+	if (err < 0)
+		return err;
+	if (tj9->client->irq) /* only write INT_CTRL_REG1 if in irq mode */
+		err = i2c_smbus_write_byte_data(tj9->client, INT_CTRL1,
+						tj9->resume[RES_INT_CTRL1]);
+	if (err < 0)
+		return err;
+
+	err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
+	if (err < 0)
+		return err;
+
+	buf = (tj9->resume[RES_CTRL_REG1] | PC1_ON);
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
+	if (err < 0)
+		return err;
+	tj9->resume[RES_CTRL_REG1] = buf;
+
+	err = kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
+	if (err < 0)
+		return err;
+
+	tj9->hw_initialized = true;
+
+	return 0;
+}
+
+static void kxtj9_device_power_off(struct kxtj9_data *tj9)
+{
+	int err;
+	u8 buf;
+
+	buf = tj9->resume[RES_CTRL_REG1] & PC1_OFF;
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
+	if (err < 0)
+		dev_err(&tj9->client->dev, "soft power off failed\n");
+	if (tj9->pdata.power_off)
+		tj9->pdata.power_off();
+	tj9->resume[RES_CTRL_REG1] = buf;
+	tj9->hw_initialized = false;
+}
+
+static int kxtj9_device_power_on(struct kxtj9_data *tj9)
+{
+	int err;
+
+	if (tj9->pdata.power_on) {
+		err = tj9->pdata.power_on();
+		if (err < 0)
+			return err;
+	}
+	if (!tj9->hw_initialized) {
+		msleep(40);
+		err = kxtj9_hw_init(tj9);
+		if (err < 0) {
+			kxtj9_device_power_off(tj9);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static void kxtj9_get_acceleration_data(struct kxtj9_data *tj9, s16 *xyz)
+{
+	int err;
+	/* Data bytes from hardware xL, xH, yL, yH, zL, zH */
+	s16 acc_data[3];
+
+	err = kxtj9_i2c_read(tj9, XOUT_L, (u8 *)acc_data, 6);
+	if (err < 0)
+		dev_err(&tj9->client->dev, "accelerometer data read failed\n");
+
+	acc_data[0] = le16_to_cpu(acc_data[0]);
+	acc_data[1] = le16_to_cpu(acc_data[1]);
+	acc_data[2] = le16_to_cpu(acc_data[2]);
+
+	acc_data[0] >>= tj9->shift;
+	acc_data[1] >>= tj9->shift;
+	acc_data[2] >>= tj9->shift;
+
+	xyz[0] = (tj9->pdata.negate_x) ? (-acc_data[tj9->pdata.axis_map_x])
+		  : (acc_data[tj9->pdata.axis_map_x]);
+	xyz[1] = (tj9->pdata.negate_y) ? (-acc_data[tj9->pdata.axis_map_y])
+		  : (acc_data[tj9->pdata.axis_map_y]);
+	xyz[2] = (tj9->pdata.negate_z) ? (-acc_data[tj9->pdata.axis_map_z])
+		  : (acc_data[tj9->pdata.axis_map_z]);
+
+	input_report_abs(tj9->input_dev, ABS_X, (int) xyz[0]);
+	input_report_abs(tj9->input_dev, ABS_Y, (int) xyz[1]);
+	input_report_abs(tj9->input_dev, ABS_Z, (int) xyz[2]);
+	input_sync(tj9->input_dev);
+}
+
+static irqreturn_t kxtj9_isr(int irq, void *dev)
+{
+	struct kxtj9_data *tj9 = dev;
+	int ret;
+	s16 xyz[3];
+
+	/* data ready is the only possible interrupt type */
+	kxtj9_get_acceleration_data(tj9, xyz);
+
+	ret = i2c_smbus_read_byte_data(tj9->client, INT_REL);
+	if (ret < 0)
+		dev_err(&tj9->client->dev,
+				"error clearing interrupt status: %d\n", ret);
+
+	return IRQ_HANDLED;
+}
+
+static int kxtj9_enable(struct kxtj9_data *tj9)
+{
+	int err = 0;
+
+	mutex_lock(&tj9->lock);
+	if (tj9->enabled == false) {
+		err = kxtj9_device_power_on(tj9);
+		if (err < 0) {
+			dev_err(&tj9->client->dev,
+					"error during power-on: %d\n", err);
+			goto err0;
+		}
+		err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
+		if (err < 0) {
+			dev_err(&tj9->client->dev,
+					"error clearing interrupt: %d\n", err);
+			goto err0;
+		}
+		if (!tj9->client->irq) /* queue polling if not in irq mode */
+			schedule_delayed_work(&tj9->input_work,
+				msecs_to_jiffies(tj9->pdata.poll_interval));
+		tj9->enabled = true;
+	}
+err0:
+	mutex_unlock(&tj9->lock);
+
+	return err;
+}
+
+static int kxtj9_disable(struct kxtj9_data *tj9)
+{
+	mutex_lock(&tj9->lock);
+	if (tj9->enabled == true) {
+		if (!tj9->client->irq) /* cancel polling if not in irq mode */
+			cancel_delayed_work_sync(&tj9->input_work);
+		kxtj9_device_power_off(tj9);
+		tj9->enabled = false;
+	}
+	mutex_unlock(&tj9->lock);
+
+	return 0;
+}
+
+static void kxtj9_input_work_func(struct work_struct *work)
+{
+	struct kxtj9_data *tj9 = container_of(work, struct kxtj9_data,
+							input_work.work);
+	s16 xyz[3];
+	mutex_lock(&tj9->lock);
+
+	kxtj9_get_acceleration_data(tj9, xyz);
+
+	schedule_delayed_work(&tj9->input_work,
+			msecs_to_jiffies(tj9->pdata.poll_interval));
+	mutex_unlock(&tj9->lock);
+}
+
+static int kxtj9_input_open(struct input_dev *input)
+{
+	return kxtj9_enable(input_get_drvdata(input));
+}
+
+static void kxtj9_input_close(struct input_dev *dev)
+{
+	kxtj9_disable(input_get_drvdata(dev));
+}
+
+static int kxtj9_input_init(struct kxtj9_data *tj9)
+{
+	int err;
+
+	if (!tj9->client->irq) /* only use input_work if not in irq mode */
+		INIT_DELAYED_WORK(&tj9->input_work, kxtj9_input_work_func);
+	tj9->input_dev = input_allocate_device();
+	if (!tj9->input_dev) {
+		err = -ENOMEM;
+		dev_err(&tj9->client->dev, "input device allocate failed\n");
+		goto err0;
+	}
+	tj9->input_dev->open = kxtj9_input_open;
+	tj9->input_dev->close = kxtj9_input_close;
+
+	input_set_drvdata(tj9->input_dev, tj9);
+
+	set_bit(EV_ABS, tj9->input_dev->evbit);
+
+	input_set_abs_params(tj9->input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
+	input_set_abs_params(tj9->input_dev, ABS_Y, -G_MAX, G_MAX, FUZZ, FLAT);
+	input_set_abs_params(tj9->input_dev, ABS_Z, -G_MAX, G_MAX, FUZZ, FLAT);
+
+	tj9->input_dev->name = "kxtj9_accel";
+	tj9->input_dev->id.bustype = BUS_I2C;
+	tj9->input_dev->dev.parent = &tj9->client->dev;
+
+	err = input_register_device(tj9->input_dev);
+	if (err) {
+		dev_err(&tj9->client->dev,
+			"unable to register input polled device %s: %d\n",
+			tj9->input_dev->name, err);
+		goto err1;
+	}
+
+	return 0;
+err1:
+	input_free_device(tj9->input_dev);
+err0:
+	return err;
+}
+
+/* kxtj9_delay_show: returns the currently selected poll interval (in ms) */
+static ssize_t kxtj9_get_poll(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct kxtj9_data *tj9 = i2c_get_clientdata(to_i2c_client(dev));
+	return sprintf(buf, "%d\n", tj9->pdata.poll_interval);
+}
+
+/* kxtj9_delay_store: allows the user to select a new poll interval (in ms) */
+static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
+						const char *buf, size_t count)
+{
+	struct kxtj9_data *tj9 = i2c_get_clientdata(to_i2c_client(dev));
+	unsigned long interval;
+	int ret = kstrtoul(buf, 10, &interval);
+	if (ret < 0)
+		return ret;
+
+	mutex_lock(&tj9->lock);
+	/* set current interval to the greater of the minimum interval or
+	the requested interval */
+	tj9->pdata.poll_interval = max((int)interval, tj9->pdata.min_interval);
+	mutex_unlock(&tj9->lock);
+
+	kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
+
+	return count;
+}
+static DEVICE_ATTR(poll, S_IRUGO|S_IWUSR, kxtj9_get_poll, kxtj9_set_poll);
+
+static struct attribute *kxtj9_attributes[] = {
+	&dev_attr_poll.attr,
+	NULL
+};
+
+static struct attribute_group kxtj9_attribute_group = {
+	.attrs = kxtj9_attributes
+};
+
+static int __devinit kxtj9_probe(struct i2c_client *client,
+						const struct i2c_device_id *id)
+{
+	int err = -1;
+	struct kxtj9_data *tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
+	if (tj9 == NULL) {
+		dev_err(&client->dev,
+			"failed to allocate memory for module data\n");
+		err = -ENOMEM;
+		goto err0;
+	}
+	if (client->dev.platform_data == NULL) {
+		dev_err(&client->dev, "platform data is NULL; exiting\n");
+		err = -ENODEV;
+		goto err0;
+	}
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
+						I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(&client->dev, "client not i2c capable\n");
+		err = -ENODEV;
+		goto err0;
+	}
+	mutex_init(&tj9->lock);
+	mutex_lock(&tj9->lock);
+	tj9->client = client;
+	i2c_set_clientdata(client, tj9);
+
+	memcpy(&tj9->pdata, client->dev.platform_data, sizeof(tj9->pdata));
+	if (tj9->pdata.init) {
+		err = tj9->pdata.init();
+		if (err < 0)
+			goto err1;
+	}
+
+	err = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
+	if (err)
+		goto err2;
+
+	tj9->resume[RES_CTRL_REG1] = tj9->pdata.res_12bit | tj9->pdata.g_range;
+	tj9->resume[RES_DATA_CTRL] = tj9->pdata.data_odr_init;
+
+	if (client->irq) {
+		err = request_threaded_irq(client->irq, NULL, kxtj9_isr,
+			IRQF_TRIGGER_RISING | IRQF_ONESHOT, "kxtj9-irq", tj9);
+		if (err < 0) {
+			pr_err("%s: request irq failed: %d\n", __func__, err);
+			goto err5;
+		}
+		/* if in irq mode, populate INT_CTRL_REG1 and enable DRDY */
+		tj9->resume[RES_INT_CTRL1] = KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
+		tj9->resume[RES_CTRL_REG1] |= DRDYE;
+	}
+
+	err = kxtj9_input_init(tj9);
+	if (err < 0)
+		goto err3;
+
+	err = kxtj9_device_power_on(tj9);
+	if (err < 0)
+		goto err4;
+	tj9->enabled = true;
+
+	err = kxtj9_verify(tj9);
+	if (err < 0) {
+		dev_err(&client->dev, "device not recognized\n");
+		goto err5;
+	}
+
+	mutex_unlock(&tj9->lock);
+
+	return 0;
+
+err5:
+	kxtj9_device_power_off(tj9);
+err4:
+	input_unregister_device(tj9->input_dev);
+err3:
+	sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
+err2:
+	if (tj9->pdata.exit)
+		tj9->pdata.exit();
+err1:
+	mutex_unlock(&tj9->lock);
+err0:
+	kfree(tj9);
+	return err;
+}
+
+static int __devexit kxtj9_remove(struct i2c_client *client)
+{
+	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+
+	sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
+	if (client->irq)
+		free_irq(client->irq, tj9);
+	input_unregister_device(tj9->input_dev);
+	kxtj9_device_power_off(tj9);
+	if (tj9->pdata.exit)
+		tj9->pdata.exit();
+	kfree(tj9);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int kxtj9_resume(struct device *dev)
+{
+	return kxtj9_enable(i2c_get_clientdata(to_i2c_client(dev)));
+}
+
+static int kxtj9_suspend(struct device *dev)
+{
+	return kxtj9_disable(i2c_get_clientdata(to_i2c_client(dev)));
+}
+
+static const struct dev_pm_ops kxtj9_pm_ops = {
+	.suspend = kxtj9_suspend,
+	.resume = kxtj9_resume,
+};
+#endif
+
+static const struct i2c_device_id kxtj9_id[] = {
+	{NAME, 0},
+	{},
+};
+
+MODULE_DEVICE_TABLE(i2c, kxtj9_id);
+
+static struct i2c_driver kxtj9_driver = {
+	.driver = {
+		   .name = NAME,
+		   .owner = THIS_MODULE,
+#ifdef CONFIG_PM
+		   .pm = &kxtj9_pm_ops,
+#endif
+		   },
+	.probe = kxtj9_probe,
+	.remove = __devexit_p(kxtj9_remove),
+	.id_table = kxtj9_id,
+};
+
+static int __init kxtj9_init(void)
+{
+	return i2c_add_driver(&kxtj9_driver);
+}
+
+static void __exit kxtj9_exit(void)
+{
+	i2c_del_driver(&kxtj9_driver);
+}
+
+module_init(kxtj9_init);
+module_exit(kxtj9_exit);
+
+MODULE_DESCRIPTION("KXTJ9 accelerometer driver");
+MODULE_AUTHOR("Chris Hudson <chudson@kionix.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/input/kxtj9.h b/include/linux/input/kxtj9.h
new file mode 100644
index 0000000..cc5928b
--- /dev/null
+++ b/include/linux/input/kxtj9.h
@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2011 Kionix, Inc.
+ * Written by Chris Hudson <chudson@kionix.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ * 02111-1307, USA
+ */
+
+#ifndef __KXTJ9_H__
+#define __KXTJ9_H__
+
+#define KXTJ9_I2C_ADDR		0x0F
+
+/* These shift values are used to provide consistent output data */
+#define SHIFT_ADJ_2G		4
+#define SHIFT_ADJ_4G		3
+#define SHIFT_ADJ_8G		2
+
+#ifdef __KERNEL__
+struct kxtj9_platform_data {
+	int poll_interval;	/* current poll interval (in milli-seconds) */
+	int min_interval;	/* minimum poll interval (in milli-seconds) */
+
+	/* by default, x is axis 0, y is axis 1, z is axis 2; these can be
+	changed to account for sensor orientation within the host device */
+	u8 axis_map_x;
+	u8 axis_map_y;
+	u8 axis_map_z;
+
+	/* each axis can be negated to account for sensor orientation within
+	the host device; 1 = negate this axis; 0 = do not negate this axis */
+	bool negate_x;
+	bool negate_y;
+	bool negate_z;
+
+	/* CTRL_REG1: set resolution, g-range, data ready enable */
+	/* Output resolution: 8-bit valid or 12-bit valid */
+	#define RES_8BIT		0
+	#define RES_12BIT		(1 << 6)
+	u8 res_12bit;
+	/* Output g-range: +/-2g, 4g, or 8g */
+	#define KXTJ9_G_2G		0
+	#define KXTJ9_G_4G		(1 << 3)
+	#define KXTJ9_G_8G		(1 << 4)
+	u8 g_range;
+
+	/* DATA_CTRL_REG: controls the output data rate of the part */
+	#define ODR12_5F		0
+	#define ODR25F			1
+	#define ODR50F			2
+	#define ODR100F			3
+	#define ODR200F			4
+	#define ODR400F			5
+	#define ODR800F			6
+	u8 data_odr_init;
+
+	int (*init)(void);
+	void (*exit)(void);
+	int (*power_on)(void);
+	int (*power_off)(void);
+};
+#endif /* __KERNEL__ */
+
+#endif  /* __KXTJ9_H__ */
+
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] input: Add support for Kionix KXTJ9 accelerometer
  2011-05-26 22:40 [PATCH v4] input: Add support for Kionix KXTJ9 accelerometer Chris Hudson
@ 2011-05-31  8:03 ` Dmitry Torokhov
  2011-06-07 18:04   ` Christopher Hudson
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2011-05-31  8:03 UTC (permalink / raw)
  To: Chris Hudson; +Cc: linux-input, jonathan.cameron, linux-kernel

Hi Chris,
On Thu, May 26, 2011 at 06:40:20PM -0400, Chris Hudson wrote:
> Quite a few changes this time around (thanks Jonathan and Dmitry!):
> - The sysfs node for poll_interval was changed to emulate input_polldev.  I
>   could not actually use input_polldev because some commands need to be sent
>   to the hardware when the poll interval is changed.
> - Locking cleaned up throughout
> - Cleared up distinction between IRQ and polling modes: if client->irq, then
>   irq mode is enabled.  Removed register bits required for irq from the header;
>   these will be set in the driver if client->irq is populated.
> - Put .suspend and .resume into struct dev_pm_ops.
> - Many other smaller changes, unnecessary variables removed, etc.
> 
> Signed-off-by: Chris Hudson <chudson@kionix.com>
> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>

Could you please tell me how badly the patch below breaks the device?

Thanks!

-- 
Dmitry


Input: KXTJ9 assorted changes

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/misc/Kconfig |   27 +-
 drivers/input/misc/kxtj9.c |  654 ++++++++++++++++++++++----------------------
 2 files changed, 344 insertions(+), 337 deletions(-)


diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 567f3d2..7010623 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -209,6 +209,23 @@ config INPUT_KEYSPAN_REMOTE
 	  To compile this driver as a module, choose M here: the module will
 	  be called keyspan_remote.
 
+config INPUT_KXTJ9
+	tristate "Kionix KXTJ9 tri-axis digital accelerometer"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Kionix KXTJ9 digital
+	  tri-axis accelerometer.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called kxtj9.
+
+config KXTJ9_POLLED_MODE
+	bool "Enable polling mode support"
+	depends on INPUT_KXTJ9
+	select INPUT_POLLDEV
+	help
+	  Say Y here if you need accelerometer to work in polling mode.
+
 config INPUT_POWERMATE
 	tristate "Griffin PowerMate and Contour Jog support"
 	depends on USB_ARCH_HAS_HCD
@@ -467,14 +484,4 @@ config INPUT_XEN_KBDDEV_FRONTEND
 	  To compile this driver as a module, choose M here: the
 	  module will be called xen-kbdfront.
 
-config INPUT_KXTJ9
-	tristate "Kionix KXTJ9 tri-axis digital accelerometer"
-	depends on I2C && SYSFS
-	help
-	  If you say yes here you get support for the Kionix KXTJ9 digital
-	  tri-axis accelerometer.
-
-	  This driver can also be built as a module.  If so, the module
-	  will be called kxtj9.
-
 endif
diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
index ef83182..1536bf4 100644
--- a/drivers/input/misc/kxtj9.c
+++ b/drivers/input/misc/kxtj9.c
@@ -61,63 +61,86 @@ static const struct {
 	unsigned int cutoff;
 	u8 mask;
 } kxtj9_odr_table[] = {
-	{
-	3,	ODR800F}, {
-	5,	ODR400F}, {
-	10,	ODR200F}, {
-	20,	ODR100F}, {
-	40,	ODR50F}, {
-	80,	ODR25F}, {
-	0,	ODR12_5F},
+	{ 3,	ODR800F },
+	{ 5,	ODR400F },
+	{ 10,	ODR200F },
+	{ 20,	ODR100F },
+	{ 40,	ODR50F  },
+	{ 80,	ODR25F  },
+	{ 0,	ODR12_5F},
 };
 
 struct kxtj9_data {
 	struct i2c_client *client;
 	struct kxtj9_platform_data pdata;
-	struct mutex lock;
-	struct delayed_work input_work;
 	struct input_dev *input_dev;
-
-	bool hw_initialized;
-	bool enabled;
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+	struct input_dev *input_polled_dev;
+#endif
+	unsigned int last_poll_interval;
 	u8 shift;
-	u8 resume[RESUME_ENTRIES];
+	u8 ctrl_reg1;
+	u8 data_ctrl;
+	u8 int_ctrl;
 };
 
 static int kxtj9_i2c_read(struct kxtj9_data *tj9, u8 addr, u8 *data, int len)
 {
-	int err;
-
 	struct i2c_msg msgs[] = {
 		{
-		 .addr = tj9->client->addr,
-		 .flags = tj9->client->flags,
-		 .len = 1,
-		 .buf = &addr,
-		 },
+			.addr = tj9->client->addr,
+			.flags = tj9->client->flags,
+			.len = 1,
+			.buf = &addr,
+		},
 		{
-		 .addr = tj9->client->addr,
-		 .flags = tj9->client->flags | I2C_M_RD,
-		 .len = len,
-		 .buf = data,
-		 },
+			.addr = tj9->client->addr,
+			.flags = tj9->client->flags | I2C_M_RD,
+			.len = len,
+			.buf = data,
+		},
 	};
-	err = i2c_transfer(tj9->client->adapter, msgs, 2);
 
-	return err;
+	return i2c_transfer(tj9->client->adapter, msgs, 2);
 }
 
-static int kxtj9_verify(struct kxtj9_data *tj9)
+static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
 {
-	int ret;
+	s16 acc_data[3]; /* Data bytes from hardware xL, xH, yL, yH, zL, zH */
+	s16 x, y, z;
+	int err;
 
-	ret = i2c_smbus_read_byte_data(tj9->client, WHO_AM_I);
-	if (ret < 0)
-		dev_err(&tj9->client->dev, "read err int source\n");
-	if (ret != 0x06)
-		ret = -EIO;
+	err = kxtj9_i2c_read(tj9, XOUT_L, (u8 *)acc_data, 6);
+	if (err < 0)
+		dev_err(&tj9->client->dev, "accelerometer data read failed\n");
+
+	x = le16_to_cpu(acc_data[0]) >> tj9->shift;
+	y = le16_to_cpu(acc_data[1]) >> tj9->shift;
+	z = le16_to_cpu(acc_data[2]) >> tj9->shift;
+
+	input_report_abs(tj9->input_dev, ABS_X,
+			 tj9->pdata.negate_x ? -x : x);
+	input_report_abs(tj9->input_dev, ABS_Y,
+			 tj9->pdata.negate_y ? -y : y);
+	input_report_abs(tj9->input_dev, ABS_Z,
+			 tj9->pdata.negate_x ? -y : y);
+	input_sync(tj9->input_dev);
+}
 
-	return ret;
+static irqreturn_t kxtj9_isr(int irq, void *dev)
+{
+	struct kxtj9_data *tj9 = dev;
+	int err;
+
+	/* data ready is the only possible interrupt type */
+	kxtj9_report_acceleration_data(tj9);
+
+	err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
+	if (err < 0)
+		dev_err(&tj9->client->dev,
+			"error clearing interrupt status: %d\n", err);
+
+	return IRQ_HANDLED;
 }
 
 static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
@@ -126,17 +149,21 @@ static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
 	case KXTJ9_G_2G:
 		tj9->shift = SHIFT_ADJ_2G;
 		break;
+
 	case KXTJ9_G_4G:
 		tj9->shift = SHIFT_ADJ_4G;
 		break;
+
 	case KXTJ9_G_8G:
 		tj9->shift = SHIFT_ADJ_8G;
 		break;
+
 	default:
 		return -EINVAL;
 	}
-	tj9->resume[RES_CTRL_REG1] = (tj9->resume[RES_CTRL_REG1] & 0xE7)
-								| new_g_range;
+
+	tj9->ctrl_reg1 &= 0xe7;
+	tj9->ctrl_reg1 |= new_g_range;
 
 	return 0;
 }
@@ -145,72 +172,40 @@ static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
 {
 	int err;
 	int i;
-	u8 config;
-	u8 temp = 0;
 
 	/* Use the lowest ODR that can support the requested poll interval */
 	for (i = 0; i < ARRAY_SIZE(kxtj9_odr_table); i++) {
-		config = kxtj9_odr_table[i].mask;
+		tj9->data_ctrl = kxtj9_odr_table[i].mask;
 		if (poll_interval < kxtj9_odr_table[i].cutoff)
 			break;
 	}
 
-	if (tj9->enabled == true) {
-		err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, temp);
-		if (err < 0)
-			return err;
-		err = i2c_smbus_write_byte_data(tj9->client, DATA_CTRL, config);
-		if (err < 0)
-			return err;
-		temp = tj9->resume[RES_CTRL_REG1];
-		err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, temp);
-		if (err < 0)
-			return err;
-		/* Cancel and restart polling if not in irq mode */
-		if (!tj9->client->irq) {
-			cancel_delayed_work_sync(&tj9->input_work);
-			schedule_delayed_work(&tj9->input_work,
-					msecs_to_jiffies(poll_interval));
-		}
-	}
-	tj9->resume[RES_DATA_CTRL] = config;
-
-	return 0;
-}
-
-static int kxtj9_hw_init(struct kxtj9_data *tj9)
-{
-	int err;
-	u8 buf = 0;
-
-	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
-	if (err < 0)
-		return err;
-	err = i2c_smbus_write_byte_data(tj9->client, DATA_CTRL,
-						tj9->resume[RES_DATA_CTRL]);
-	if (err < 0)
-		return err;
-	if (tj9->client->irq) /* only write INT_CTRL_REG1 if in irq mode */
-		err = i2c_smbus_write_byte_data(tj9->client, INT_CTRL1,
-						tj9->resume[RES_INT_CTRL1]);
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, 0);
 	if (err < 0)
 		return err;
 
-	err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
+	err = i2c_smbus_write_byte_data(tj9->client,
+					DATA_CTRL, tj9->data_ctrl);
 	if (err < 0)
 		return err;
 
-	buf = (tj9->resume[RES_CTRL_REG1] | PC1_ON);
-	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
+	err = i2c_smbus_write_byte_data(tj9->client,
+					CTRL_REG1, tj9->ctrl_reg1);
 	if (err < 0)
 		return err;
-	tj9->resume[RES_CTRL_REG1] = buf;
 
-	err = kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
-	if (err < 0)
-		return err;
+	return 0;
+}
 
-	tj9->hw_initialized = true;
+static int kxtj9_device_power_on(struct kxtj9_data *tj9)
+{
+	int err;
+
+	if (tj9->pdata.power_on) {
+		err = tj9->pdata.power_on();
+		if (err < 0)
+			return err;
+	}
 
 	return 0;
 }
@@ -218,319 +213,306 @@ static int kxtj9_hw_init(struct kxtj9_data *tj9)
 static void kxtj9_device_power_off(struct kxtj9_data *tj9)
 {
 	int err;
-	u8 buf;
 
-	buf = tj9->resume[RES_CTRL_REG1] & PC1_OFF;
-	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
+	tj9->ctrl_reg1 &= PC1_OFF;
+	err = i2c_smbus_write_byte_data(tj9->client,
+					CTRL_REG1, tj9->ctrl_reg1);
 	if (err < 0)
 		dev_err(&tj9->client->dev, "soft power off failed\n");
+
 	if (tj9->pdata.power_off)
 		tj9->pdata.power_off();
-	tj9->resume[RES_CTRL_REG1] = buf;
-	tj9->hw_initialized = false;
 }
 
-static int kxtj9_device_power_on(struct kxtj9_data *tj9)
+static int kxtj9_enable(struct kxtj9_data *tj9)
 {
 	int err;
 
-	if (tj9->pdata.power_on) {
-		err = tj9->pdata.power_on();
+	err = kxtj9_device_power_on(tj9);
+	if (err < 0)
+		return err;
+
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, 0);
+	if (err < 0)
+		return err;
+
+	err = i2c_smbus_write_byte_data(tj9->client,
+					DATA_CTRL, tj9->data_ctrl);
+	if (err < 0)
+		return err;
+
+	/* only write INT_CTRL_REG1 if in irq mode */
+	if (tj9->client->irq) {
+		err = i2c_smbus_write_byte_data(tj9->client,
+						INT_CTRL1, tj9->int_ctrl);
 		if (err < 0)
 			return err;
 	}
-	if (!tj9->hw_initialized) {
-		msleep(40);
-		err = kxtj9_hw_init(tj9);
-		if (err < 0) {
-			kxtj9_device_power_off(tj9);
-			return err;
-		}
-	}
 
-	return 0;
-}
+	err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
+	if (err < 0)
+		return err;
 
-static void kxtj9_get_acceleration_data(struct kxtj9_data *tj9, s16 *xyz)
-{
-	int err;
-	/* Data bytes from hardware xL, xH, yL, yH, zL, zH */
-	s16 acc_data[3];
+	tj9->ctrl_reg1 |= PC1_ON;
+	err = i2c_smbus_write_byte_data(tj9->client,
+					CTRL_REG1, tj9->ctrl_reg1);
+	if (err < 0)
+		return err;
 
-	err = kxtj9_i2c_read(tj9, XOUT_L, (u8 *)acc_data, 6);
+	err = kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
 	if (err < 0)
-		dev_err(&tj9->client->dev, "accelerometer data read failed\n");
+		return err;
 
-	acc_data[0] = le16_to_cpu(acc_data[0]);
-	acc_data[1] = le16_to_cpu(acc_data[1]);
-	acc_data[2] = le16_to_cpu(acc_data[2]);
+	err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
+	if (err < 0) {
+		dev_err(&tj9->client->dev,
+			"error clearing interrupt: %d\n", err);
+		goto fail;
+	}
 
-	acc_data[0] >>= tj9->shift;
-	acc_data[1] >>= tj9->shift;
-	acc_data[2] >>= tj9->shift;
+	return 0;
 
-	xyz[0] = (tj9->pdata.negate_x) ? (-acc_data[tj9->pdata.axis_map_x])
-		  : (acc_data[tj9->pdata.axis_map_x]);
-	xyz[1] = (tj9->pdata.negate_y) ? (-acc_data[tj9->pdata.axis_map_y])
-		  : (acc_data[tj9->pdata.axis_map_y]);
-	xyz[2] = (tj9->pdata.negate_z) ? (-acc_data[tj9->pdata.axis_map_z])
-		  : (acc_data[tj9->pdata.axis_map_z]);
+fail:
+	kxtj9_device_power_off(tj9);
+	return err;
+}
 
-	input_report_abs(tj9->input_dev, ABS_X, (int) xyz[0]);
-	input_report_abs(tj9->input_dev, ABS_Y, (int) xyz[1]);
-	input_report_abs(tj9->input_dev, ABS_Z, (int) xyz[2]);
-	input_sync(tj9->input_dev);
+static void kxtj9_disable(struct kxtj9_data *tj9)
+{
+	kxtj9_device_power_off(tj9);
 }
 
-static irqreturn_t kxtj9_isr(int irq, void *dev)
+static int kxtj9_input_open(struct input_dev *input)
 {
-	struct kxtj9_data *tj9 = dev;
-	int ret;
-	s16 xyz[3];
+	struct kxtj9_data *tj9 = input_get_drvdata(input);
 
-	/* data ready is the only possible interrupt type */
-	kxtj9_get_acceleration_data(tj9, xyz);
+	return kxtj9_enable(tj9);
+}
 
-	ret = i2c_smbus_read_byte_data(tj9->client, INT_REL);
-	if (ret < 0)
-		dev_err(&tj9->client->dev,
-				"error clearing interrupt status: %d\n", ret);
+static void kxtj9_input_close(struct input_dev *dev)
+{
+	struct kxtj9_data *tj9 = input_get_drvdata(dev);
 
-	return IRQ_HANDLED;
+	kxtj9_disable(tj9);
 }
 
-static int kxtj9_enable(struct kxtj9_data *tj9)
+static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
+					      struct input_dev *input_dev)
 {
-	int err = 0;
-
-	mutex_lock(&tj9->lock);
-	if (tj9->enabled == false) {
-		err = kxtj9_device_power_on(tj9);
-		if (err < 0) {
-			dev_err(&tj9->client->dev,
-					"error during power-on: %d\n", err);
-			goto err0;
-		}
-		err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
-		if (err < 0) {
-			dev_err(&tj9->client->dev,
-					"error clearing interrupt: %d\n", err);
-			goto err0;
-		}
-		if (!tj9->client->irq) /* queue polling if not in irq mode */
-			schedule_delayed_work(&tj9->input_work,
-				msecs_to_jiffies(tj9->pdata.poll_interval));
-		tj9->enabled = true;
-	}
-err0:
-	mutex_unlock(&tj9->lock);
-
-	return err;
+	__set_bit(EV_ABS, input_dev->evbit);
+	input_set_abs_params(input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
+	input_set_abs_params(input_dev, ABS_Y, -G_MAX, G_MAX, FUZZ, FLAT);
+	input_set_abs_params(input_dev, ABS_Z, -G_MAX, G_MAX, FUZZ, FLAT);
+
+	input_dev->name = "kxtj9_accel";
+	input_dev->id.bustype = BUS_I2C;
+	input_dev->dev.parent = &tj9->client->dev;
 }
 
-static int kxtj9_disable(struct kxtj9_data *tj9)
+static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
 {
-	mutex_lock(&tj9->lock);
-	if (tj9->enabled == true) {
-		if (!tj9->client->irq) /* cancel polling if not in irq mode */
-			cancel_delayed_work_sync(&tj9->input_work);
-		kxtj9_device_power_off(tj9);
-		tj9->enabled = false;
+	struct input_dev *input_dev;
+	int err;
+
+	input_dev = input_allocate_device();
+	if (!tj9->input_dev) {
+		dev_err(&tj9->client->dev, "input device allocate failed\n");
+		return -ENOMEM;
+	}
+
+	tj9->input_dev = input_dev;
+
+	input_dev->open = kxtj9_input_open;
+	input_dev->close = kxtj9_input_close;
+	input_set_drvdata(input_dev, tj9);
+
+	kxtj9_init_input_device(tj9, input_dev);
+
+	err = input_register_device(tj9->input_dev);
+	if (err) {
+		dev_err(&tj9->client->dev,
+			"unable to register input polled device %s: %d\n",
+			tj9->input_dev->name, err);
+		input_free_device(tj9->input_dev);
 	}
-	mutex_unlock(&tj9->lock);
 
 	return 0;
 }
 
-static void kxtj9_input_work_func(struct work_struct *work)
+#ifdef CONIFG_KXTJ9_POLLED_MODE
+static void kxtj9_poll(struct input_polled_dev *dev)
 {
-	struct kxtj9_data *tj9 = container_of(work, struct kxtj9_data,
-							input_work.work);
-	s16 xyz[3];
-	mutex_lock(&tj9->lock);
+	struct kxtj9_data *tj9 = dev->private;
+	unsigned int poll_interval = dev->poll_interval;
 
-	kxtj9_get_acceleration_data(tj9, xyz);
+	kxtj9_report_acceleration_data(tj9);
 
-	schedule_delayed_work(&tj9->input_work,
-			msecs_to_jiffies(tj9->pdata.poll_interval));
-	mutex_unlock(&tj9->lock);
+	if (poll_interval != tj9->last_poll_interval) {
+		kxtj9_update_odr(tj9, poll_interval);
+		tj9->last_poll_interval = poll_interval;
+	}
 }
 
-static int kxtj9_input_open(struct input_dev *input)
+static int kxtj9_polled_input_open(struct input_polled_dev *dev)
 {
-	return kxtj9_enable(input_get_drvdata(input));
+	struct kxtj9_data *tj9 = dev->private;
+
+	return kxtj9_enable(tj9);
 }
 
-static void kxtj9_input_close(struct input_dev *dev)
+static void kxtj9_polled_input_close(struct input_polled_dev *dev)
 {
-	kxtj9_disable(input_get_drvdata(dev));
+	struct kxtj9_data *tj9 = dev->private;
+
+	kxtj9_disable(tj9);
 }
 
-static int kxtj9_input_init(struct kxtj9_data *tj9)
+static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
 {
-	int err;
+	struct input_polled_dev *poll_dev;
 
-	if (!tj9->client->irq) /* only use input_work if not in irq mode */
-		INIT_DELAYED_WORK(&tj9->input_work, kxtj9_input_work_func);
-	tj9->input_dev = input_allocate_device();
-	if (!tj9->input_dev) {
-		err = -ENOMEM;
-		dev_err(&tj9->client->dev, "input device allocate failed\n");
-		goto err0;
+	poll_dev = input_allocate_polled_device();
+	if (!poll_dev) {
+		dev_err(&tj9->client->dev,
+			"Failed to allocate polled device\n");
+		return -ENOMEM;
 	}
-	tj9->input_dev->open = kxtj9_input_open;
-	tj9->input_dev->close = kxtj9_input_close;
 
-	input_set_drvdata(tj9->input_dev, tj9);
+	tj9->polled_dev = poll_dev;
+	tj9->input_dev = poll_dev->input;
 
-	set_bit(EV_ABS, tj9->input_dev->evbit);
+	poll_dev->private = tj9;
+	poll_dev->poll = kxtj9_poll;
+	poll_dev->open = kxtj9_polled_input_open;
+	poll_dev->close = kxtj9_polled_input_close;
 
-	input_set_abs_params(tj9->input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
-	input_set_abs_params(tj9->input_dev, ABS_Y, -G_MAX, G_MAX, FUZZ, FLAT);
-	input_set_abs_params(tj9->input_dev, ABS_Z, -G_MAX, G_MAX, FUZZ, FLAT);
+	kxtj9_init_input_device(tj9, poll_dev->input);
 
-	tj9->input_dev->name = "kxtj9_accel";
-	tj9->input_dev->id.bustype = BUS_I2C;
-	tj9->input_dev->dev.parent = &tj9->client->dev;
-
-	err = input_register_device(tj9->input_dev);
+	err = input_register_polled_device(poll_dev);
 	if (err) {
 		dev_err(&tj9->client->dev,
-			"unable to register input polled device %s: %d\n",
-			tj9->input_dev->name, err);
-		goto err1;
+			"Unable to register polled device, err=%d\n", err);
+		input_free_polled_device(poll_dev);
+		return 0;
 	}
 
 	return 0;
-err1:
-	input_free_device(tj9->input_dev);
-err0:
-	return err;
 }
 
-/* kxtj9_delay_show: returns the currently selected poll interval (in ms) */
-static ssize_t kxtj9_get_poll(struct device *dev,
-				struct device_attribute *attr, char *buf)
+static kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
 {
-	struct kxtj9_data *tj9 = i2c_get_clientdata(to_i2c_client(dev));
-	return sprintf(buf, "%d\n", tj9->pdata.poll_interval);
+	input_unregister_polled_device(tj9->poll_dev);
+	input_free_polled_device(tj9->poll_dev);
 }
 
-/* kxtj9_delay_store: allows the user to select a new poll interval (in ms) */
-static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
-						const char *buf, size_t count)
+#else
+
+static inline int kxtj9_setup_polled_device(struct kxtj9_data *tj9)
 {
-	struct kxtj9_data *tj9 = i2c_get_clientdata(to_i2c_client(dev));
-	unsigned long interval;
-	int ret = kstrtoul(buf, 10, &interval);
-	if (ret < 0)
-		return ret;
+	return -ENOSYS;
+}
 
-	mutex_lock(&tj9->lock);
-	/* set current interval to the greater of the minimum interval or
-	the requested interval */
-	tj9->pdata.poll_interval = max((int)interval, tj9->pdata.min_interval);
-	mutex_unlock(&tj9->lock);
+static inline void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
+{
+}
+#endif
 
-	kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
+static int __devinit kxtj9_verify(struct kxtj9_data *tj9)
+{
+	int retval;
 
-	return count;
-}
-static DEVICE_ATTR(poll, S_IRUGO|S_IWUSR, kxtj9_get_poll, kxtj9_set_poll);
+	retval = kxtj9_device_power_on(tj9);
+	if (retval < 0)
+		return retval;
 
-static struct attribute *kxtj9_attributes[] = {
-	&dev_attr_poll.attr,
-	NULL
-};
+	retval = i2c_smbus_read_byte_data(tj9->client, WHO_AM_I);
+	if (retval < 0) {
+		dev_err(&tj9->client->dev, "read err int source\n");
+		goto out;
+	}
 
-static struct attribute_group kxtj9_attribute_group = {
-	.attrs = kxtj9_attributes
-};
+	retval = retval != 0x06 ? - EIO : 0;
+
+out:
+	kxtj9_device_power_off(tj9);
+	return retval;
+}
 
 static int __devinit kxtj9_probe(struct i2c_client *client,
-						const struct i2c_device_id *id)
+				 const struct i2c_device_id *id)
 {
-	int err = -1;
-	struct kxtj9_data *tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
-	if (tj9 == NULL) {
-		dev_err(&client->dev,
-			"failed to allocate memory for module data\n");
-		err = -ENOMEM;
-		goto err0;
+	const struct kxtj9_platform_data *pdata = client->dev.platform_data;
+	struct kxtj9_data *tj9;
+	int err;
+
+	if (!i2c_check_functionality(client->adapter,
+				I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE_DATA)) {
+		dev_err(&client->dev, "client is not i2c capable\n");
+		return -ENXIO;
 	}
-	if (client->dev.platform_data == NULL) {
+
+	if (!pdata) {
 		dev_err(&client->dev, "platform data is NULL; exiting\n");
-		err = -ENODEV;
-		goto err0;
+		return -EINVAL;
 	}
-	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
-						I2C_FUNC_SMBUS_BYTE_DATA)) {
-		dev_err(&client->dev, "client not i2c capable\n");
-		err = -ENODEV;
-		goto err0;
+
+	tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
+	if (!tj9) {
+		dev_err(&client->dev,
+			"failed to allocate memory for module data\n");
+		return -ENOMEM;
 	}
-	mutex_init(&tj9->lock);
-	mutex_lock(&tj9->lock);
+
 	tj9->client = client;
-	i2c_set_clientdata(client, tj9);
+	tj9->pdata = *pdata;
 
-	memcpy(&tj9->pdata, client->dev.platform_data, sizeof(tj9->pdata));
-	if (tj9->pdata.init) {
-		err = tj9->pdata.init();
+	if (pdata->init) {
+		err = pdata->init();
 		if (err < 0)
-			goto err1;
+			goto err_free_mem;
 	}
 
-	err = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
-	if (err)
-		goto err2;
+	err = kxtj9_verify(tj9);
+	if (err < 0) {
+		dev_err(&client->dev, "device not recognized\n");
+		goto err_pdata_exit;
+	}
 
-	tj9->resume[RES_CTRL_REG1] = tj9->pdata.res_12bit | tj9->pdata.g_range;
-	tj9->resume[RES_DATA_CTRL] = tj9->pdata.data_odr_init;
+	tj9->ctrl_reg1 = tj9->pdata.res_12bit | tj9->pdata.g_range;
+	tj9->data_ctrl = tj9->pdata.data_odr_init;
 
 	if (client->irq) {
-		err = request_threaded_irq(client->irq, NULL, kxtj9_isr,
-			IRQF_TRIGGER_RISING | IRQF_ONESHOT, "kxtj9-irq", tj9);
-		if (err < 0) {
-			pr_err("%s: request irq failed: %d\n", __func__, err);
-			goto err5;
-		}
-		/* if in irq mode, populate INT_CTRL_REG1 and enable DRDY */
-		tj9->resume[RES_INT_CTRL1] = KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
-		tj9->resume[RES_CTRL_REG1] |= DRDYE;
-	}
+		/* If in irq mode, populate INT_CTRL_REG1 and enable DRDY. */
+		tj9->int_ctrl |= KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
+		tj9->ctrl_reg1 |= DRDYE;
 
-	err = kxtj9_input_init(tj9);
-	if (err < 0)
-		goto err3;
+		err = kxtj9_setup_input_device(tj9);
+		if (err)
+			goto err_pdata_exit;
 
-	err = kxtj9_device_power_on(tj9);
-	if (err < 0)
-		goto err4;
-	tj9->enabled = true;
+		err = request_threaded_irq(client->irq, NULL, kxtj9_isr,
+					   IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+					   "kxtj9-irq", tj9);
+		if (err) {
+			dev_err(&client->dev, "request irq failed: %d\n", err);
+			goto err_destroy_input;
+		}
 
-	err = kxtj9_verify(tj9);
-	if (err < 0) {
-		dev_err(&client->dev, "device not recognized\n");
-		goto err5;
+	} else {
+		err = kxtj9_setup_polled_device(tj9);
+		if (err)
+			goto err_pdata_exit;
 	}
 
-	mutex_unlock(&tj9->lock);
-
+	i2c_set_clientdata(client, tj9);
 	return 0;
 
-err5:
-	kxtj9_device_power_off(tj9);
-err4:
+err_destroy_input:
 	input_unregister_device(tj9->input_dev);
-err3:
-	sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
-err2:
+err_pdata_exit:
 	if (tj9->pdata.exit)
 		tj9->pdata.exit();
-err1:
-	mutex_unlock(&tj9->lock);
-err0:
+err_free_mem:
 	kfree(tj9);
 	return err;
 }
@@ -539,50 +521,69 @@ static int __devexit kxtj9_remove(struct i2c_client *client)
 {
 	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
 
-	sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
-	if (client->irq)
+	if (client->irq) {
 		free_irq(client->irq, tj9);
-	input_unregister_device(tj9->input_dev);
-	kxtj9_device_power_off(tj9);
+		input_unregister_device(tj9->input_dev);
+	} else {
+		kxtj9_teardown_polled_device(tj9);
+	}
+
 	if (tj9->pdata.exit)
 		tj9->pdata.exit();
+
 	kfree(tj9);
 
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int kxtj9_resume(struct device *dev)
+#ifdef CONFIG_PM_SLEEP
+static int kxtj9_suspend(struct device *dev)
 {
-	return kxtj9_enable(i2c_get_clientdata(to_i2c_client(dev)));
+	struct i2c_client *client = to_i2c_client(dev);
+	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+	struct input_dev *input_dev = tj9->input_dev;
+
+	mutex_lock(&input_dev->mutex);
+
+	if (input_dev->users)
+		kxtj9_disable(tj9);
+
+	mutex_unlock(&input_dev->mutex);
+	return 0;
 }
 
-static int kxtj9_suspend(struct device *dev)
+static int kxtj9_resume(struct device *dev)
 {
-	return kxtj9_disable(i2c_get_clientdata(to_i2c_client(dev)));
-}
+	struct i2c_client *client = to_i2c_client(dev);
+	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+	struct input_dev *input_dev = tj9->input_dev;
+	int retval = 0;
 
-static const struct dev_pm_ops kxtj9_pm_ops = {
-	.suspend = kxtj9_suspend,
-	.resume = kxtj9_resume,
-};
+	mutex_lock(&input_dev->mutex);
+
+	if (input_dev->users)
+		kxtj9_enable(tj9);
+
+	mutex_unlock(&input_dev->mutex);
+	return retval;
+}
 #endif
 
+static SIMPLE_DEV_PM_OPS(kxtj9_pm_ops, kxtj9_suspend, kxtj9_resume);
+
 static const struct i2c_device_id kxtj9_id[] = {
-	{NAME, 0},
-	{},
+	{ NAME, 0 },
+	{ },
 };
 
 MODULE_DEVICE_TABLE(i2c, kxtj9_id);
 
 static struct i2c_driver kxtj9_driver = {
 	.driver = {
-		   .name = NAME,
-		   .owner = THIS_MODULE,
-#ifdef CONFIG_PM
-		   .pm = &kxtj9_pm_ops,
-#endif
-		   },
+		.name = NAME,
+		.owner = THIS_MODULE,
+		.pm = &kxtj9_pm_ops,
+	},
 	.probe = kxtj9_probe,
 	.remove = __devexit_p(kxtj9_remove),
 	.id_table = kxtj9_id,
@@ -592,13 +593,12 @@ static int __init kxtj9_init(void)
 {
 	return i2c_add_driver(&kxtj9_driver);
 }
+module_init(kxtj9_init);
 
 static void __exit kxtj9_exit(void)
 {
 	i2c_del_driver(&kxtj9_driver);
 }
-
-module_init(kxtj9_init);
 module_exit(kxtj9_exit);
 
 MODULE_DESCRIPTION("KXTJ9 accelerometer driver");

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] input: Add support for Kionix KXTJ9 accelerometer
  2011-05-31  8:03 ` Dmitry Torokhov
@ 2011-06-07 18:04   ` Christopher Hudson
  2011-06-16 19:41     ` Christopher Hudson
  0 siblings, 1 reply; 4+ messages in thread
From: Christopher Hudson @ 2011-06-07 18:04 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input, Jonathan Cameron, Chris Hudson

Hi Dmitry,

My apologies for the delayed reply.  I couldn't get your patch to
apply cleanly to my tree, but I was able to manually add the changes.
I liked many of these changes, but there were 2 main issues when
testing on the hardware:

- There is no poll interval selection available in irq mode.  I was
planning to add my previous versions of get_poll and set_poll to the
#else branch of CONFIG_KXTJ9_POLLED_MODE, and create this sysfs group
in the client->irq branch in probe.  Unfortunately, this seems to
render the use of input_polled_dev a bit redundant.  What do you think
about this implementation?

- For some reason, whenever I try to use le16_to_cpu followed by a
bit-shift operation without first storing the results to memory I end
up with garbled data.  What seems to be happening is the bit-shift is
being performed before the le16_to_cpu operation.  I've tried various
implementations with parentheses to try to separate the two
operations, but the only thing I have found to work is to store the
results of the le16_to_cpu operation first, then bit-shift the result
in another operation:
x = le16_to_cpu(acc_data[tj9->pdata.axis_map_x]);
x >>= tj9->shift;
Note that I also preserved the axis_map platform data variable in this
implementation; was it your intent to do away with this feature?


On Tue, May 31, 2011 at 4:03 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Chris,
> On Thu, May 26, 2011 at 06:40:20PM -0400, Chris Hudson wrote:
>> Quite a few changes this time around (thanks Jonathan and Dmitry!):
>> - The sysfs node for poll_interval was changed to emulate input_polldev.  I
>>   could not actually use input_polldev because some commands need to be sent
>>   to the hardware when the poll interval is changed.
>> - Locking cleaned up throughout
>> - Cleared up distinction between IRQ and polling modes: if client->irq, then
>>   irq mode is enabled.  Removed register bits required for irq from the header;
>>   these will be set in the driver if client->irq is populated.
>> - Put .suspend and .resume into struct dev_pm_ops.
>> - Many other smaller changes, unnecessary variables removed, etc.
>>
>> Signed-off-by: Chris Hudson <chudson@kionix.com>
>> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>
> Could you please tell me how badly the patch below breaks the device?
>
> Thanks!
>
> --
> Dmitry
>
>
> Input: KXTJ9 assorted changes
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>
>  drivers/input/misc/Kconfig |   27 +-
>  drivers/input/misc/kxtj9.c |  654 ++++++++++++++++++++++----------------------
>  2 files changed, 344 insertions(+), 337 deletions(-)
>
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 567f3d2..7010623 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -209,6 +209,23 @@ config INPUT_KEYSPAN_REMOTE
>          To compile this driver as a module, choose M here: the module will
>          be called keyspan_remote.
>
> +config INPUT_KXTJ9
> +       tristate "Kionix KXTJ9 tri-axis digital accelerometer"
> +       depends on I2C
> +       help
> +         If you say yes here you get support for the Kionix KXTJ9 digital
> +         tri-axis accelerometer.
> +
> +         This driver can also be built as a module.  If so, the module
> +         will be called kxtj9.
> +
> +config KXTJ9_POLLED_MODE
> +       bool "Enable polling mode support"
> +       depends on INPUT_KXTJ9
> +       select INPUT_POLLDEV
> +       help
> +         Say Y here if you need accelerometer to work in polling mode.
> +
>  config INPUT_POWERMATE
>        tristate "Griffin PowerMate and Contour Jog support"
>        depends on USB_ARCH_HAS_HCD
> @@ -467,14 +484,4 @@ config INPUT_XEN_KBDDEV_FRONTEND
>          To compile this driver as a module, choose M here: the
>          module will be called xen-kbdfront.
>
> -config INPUT_KXTJ9
> -       tristate "Kionix KXTJ9 tri-axis digital accelerometer"
> -       depends on I2C && SYSFS
> -       help
> -         If you say yes here you get support for the Kionix KXTJ9 digital
> -         tri-axis accelerometer.
> -
> -         This driver can also be built as a module.  If so, the module
> -         will be called kxtj9.
> -
>  endif
> diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
> index ef83182..1536bf4 100644
> --- a/drivers/input/misc/kxtj9.c
> +++ b/drivers/input/misc/kxtj9.c
> @@ -61,63 +61,86 @@ static const struct {
>        unsigned int cutoff;
>        u8 mask;
>  } kxtj9_odr_table[] = {
> -       {
> -       3,      ODR800F}, {
> -       5,      ODR400F}, {
> -       10,     ODR200F}, {
> -       20,     ODR100F}, {
> -       40,     ODR50F}, {
> -       80,     ODR25F}, {
> -       0,      ODR12_5F},
> +       { 3,    ODR800F },
> +       { 5,    ODR400F },
> +       { 10,   ODR200F },
> +       { 20,   ODR100F },
> +       { 40,   ODR50F  },
> +       { 80,   ODR25F  },
> +       { 0,    ODR12_5F},
>  };
>
>  struct kxtj9_data {
>        struct i2c_client *client;
>        struct kxtj9_platform_data pdata;
> -       struct mutex lock;
> -       struct delayed_work input_work;
>        struct input_dev *input_dev;
> -
> -       bool hw_initialized;
> -       bool enabled;
> +#ifdef CONFIG_KXTJ9_POLLED_MODE
> +       struct input_dev *input_polled_dev;
> +#endif
> +       unsigned int last_poll_interval;
>        u8 shift;
> -       u8 resume[RESUME_ENTRIES];
> +       u8 ctrl_reg1;
> +       u8 data_ctrl;
> +       u8 int_ctrl;
>  };
>
>  static int kxtj9_i2c_read(struct kxtj9_data *tj9, u8 addr, u8 *data, int len)
>  {
> -       int err;
> -
>        struct i2c_msg msgs[] = {
>                {
> -                .addr = tj9->client->addr,
> -                .flags = tj9->client->flags,
> -                .len = 1,
> -                .buf = &addr,
> -                },
> +                       .addr = tj9->client->addr,
> +                       .flags = tj9->client->flags,
> +                       .len = 1,
> +                       .buf = &addr,
> +               },
>                {
> -                .addr = tj9->client->addr,
> -                .flags = tj9->client->flags | I2C_M_RD,
> -                .len = len,
> -                .buf = data,
> -                },
> +                       .addr = tj9->client->addr,
> +                       .flags = tj9->client->flags | I2C_M_RD,
> +                       .len = len,
> +                       .buf = data,
> +               },
>        };
> -       err = i2c_transfer(tj9->client->adapter, msgs, 2);
>
> -       return err;
> +       return i2c_transfer(tj9->client->adapter, msgs, 2);
>  }
>
> -static int kxtj9_verify(struct kxtj9_data *tj9)
> +static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
>  {
> -       int ret;
> +       s16 acc_data[3]; /* Data bytes from hardware xL, xH, yL, yH, zL, zH */
> +       s16 x, y, z;
> +       int err;
>
> -       ret = i2c_smbus_read_byte_data(tj9->client, WHO_AM_I);
> -       if (ret < 0)
> -               dev_err(&tj9->client->dev, "read err int source\n");
> -       if (ret != 0x06)
> -               ret = -EIO;
> +       err = kxtj9_i2c_read(tj9, XOUT_L, (u8 *)acc_data, 6);
> +       if (err < 0)
> +               dev_err(&tj9->client->dev, "accelerometer data read failed\n");
> +
> +       x = le16_to_cpu(acc_data[0]) >> tj9->shift;
> +       y = le16_to_cpu(acc_data[1]) >> tj9->shift;
> +       z = le16_to_cpu(acc_data[2]) >> tj9->shift;
> +
> +       input_report_abs(tj9->input_dev, ABS_X,
> +                        tj9->pdata.negate_x ? -x : x);
> +       input_report_abs(tj9->input_dev, ABS_Y,
> +                        tj9->pdata.negate_y ? -y : y);
> +       input_report_abs(tj9->input_dev, ABS_Z,
> +                        tj9->pdata.negate_x ? -y : y);
> +       input_sync(tj9->input_dev);
> +}
>
> -       return ret;
> +static irqreturn_t kxtj9_isr(int irq, void *dev)
> +{
> +       struct kxtj9_data *tj9 = dev;
> +       int err;
> +
> +       /* data ready is the only possible interrupt type */
> +       kxtj9_report_acceleration_data(tj9);
> +
> +       err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
> +       if (err < 0)
> +               dev_err(&tj9->client->dev,
> +                       "error clearing interrupt status: %d\n", err);
> +
> +       return IRQ_HANDLED;
>  }
>
>  static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
> @@ -126,17 +149,21 @@ static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
>        case KXTJ9_G_2G:
>                tj9->shift = SHIFT_ADJ_2G;
>                break;
> +
>        case KXTJ9_G_4G:
>                tj9->shift = SHIFT_ADJ_4G;
>                break;
> +
>        case KXTJ9_G_8G:
>                tj9->shift = SHIFT_ADJ_8G;
>                break;
> +
>        default:
>                return -EINVAL;
>        }
> -       tj9->resume[RES_CTRL_REG1] = (tj9->resume[RES_CTRL_REG1] & 0xE7)
> -                                                               | new_g_range;
> +
> +       tj9->ctrl_reg1 &= 0xe7;
> +       tj9->ctrl_reg1 |= new_g_range;
>
>        return 0;
>  }
> @@ -145,72 +172,40 @@ static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
>  {
>        int err;
>        int i;
> -       u8 config;
> -       u8 temp = 0;
>
>        /* Use the lowest ODR that can support the requested poll interval */
>        for (i = 0; i < ARRAY_SIZE(kxtj9_odr_table); i++) {
> -               config = kxtj9_odr_table[i].mask;
> +               tj9->data_ctrl = kxtj9_odr_table[i].mask;
>                if (poll_interval < kxtj9_odr_table[i].cutoff)
>                        break;
>        }
>
> -       if (tj9->enabled == true) {
> -               err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, temp);
> -               if (err < 0)
> -                       return err;
> -               err = i2c_smbus_write_byte_data(tj9->client, DATA_CTRL, config);
> -               if (err < 0)
> -                       return err;
> -               temp = tj9->resume[RES_CTRL_REG1];
> -               err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, temp);
> -               if (err < 0)
> -                       return err;
> -               /* Cancel and restart polling if not in irq mode */
> -               if (!tj9->client->irq) {
> -                       cancel_delayed_work_sync(&tj9->input_work);
> -                       schedule_delayed_work(&tj9->input_work,
> -                                       msecs_to_jiffies(poll_interval));
> -               }
> -       }
> -       tj9->resume[RES_DATA_CTRL] = config;
> -
> -       return 0;
> -}
> -
> -static int kxtj9_hw_init(struct kxtj9_data *tj9)
> -{
> -       int err;
> -       u8 buf = 0;
> -
> -       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
> -       if (err < 0)
> -               return err;
> -       err = i2c_smbus_write_byte_data(tj9->client, DATA_CTRL,
> -                                               tj9->resume[RES_DATA_CTRL]);
> -       if (err < 0)
> -               return err;
> -       if (tj9->client->irq) /* only write INT_CTRL_REG1 if in irq mode */
> -               err = i2c_smbus_write_byte_data(tj9->client, INT_CTRL1,
> -                                               tj9->resume[RES_INT_CTRL1]);
> +       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, 0);
>        if (err < 0)
>                return err;
>
> -       err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
> +       err = i2c_smbus_write_byte_data(tj9->client,
> +                                       DATA_CTRL, tj9->data_ctrl);
>        if (err < 0)
>                return err;
>
> -       buf = (tj9->resume[RES_CTRL_REG1] | PC1_ON);
> -       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
> +       err = i2c_smbus_write_byte_data(tj9->client,
> +                                       CTRL_REG1, tj9->ctrl_reg1);
>        if (err < 0)
>                return err;
> -       tj9->resume[RES_CTRL_REG1] = buf;
>
> -       err = kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
> -       if (err < 0)
> -               return err;
> +       return 0;
> +}
>
> -       tj9->hw_initialized = true;
> +static int kxtj9_device_power_on(struct kxtj9_data *tj9)
> +{
> +       int err;
> +
> +       if (tj9->pdata.power_on) {
> +               err = tj9->pdata.power_on();
> +               if (err < 0)
> +                       return err;
> +       }
>
>        return 0;
>  }
> @@ -218,319 +213,306 @@ static int kxtj9_hw_init(struct kxtj9_data *tj9)
>  static void kxtj9_device_power_off(struct kxtj9_data *tj9)
>  {
>        int err;
> -       u8 buf;
>
> -       buf = tj9->resume[RES_CTRL_REG1] & PC1_OFF;
> -       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
> +       tj9->ctrl_reg1 &= PC1_OFF;
> +       err = i2c_smbus_write_byte_data(tj9->client,
> +                                       CTRL_REG1, tj9->ctrl_reg1);
>        if (err < 0)
>                dev_err(&tj9->client->dev, "soft power off failed\n");
> +
>        if (tj9->pdata.power_off)
>                tj9->pdata.power_off();
> -       tj9->resume[RES_CTRL_REG1] = buf;
> -       tj9->hw_initialized = false;
>  }
>
> -static int kxtj9_device_power_on(struct kxtj9_data *tj9)
> +static int kxtj9_enable(struct kxtj9_data *tj9)
>  {
>        int err;
>
> -       if (tj9->pdata.power_on) {
> -               err = tj9->pdata.power_on();
> +       err = kxtj9_device_power_on(tj9);
> +       if (err < 0)
> +               return err;
> +
> +       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, 0);
> +       if (err < 0)
> +               return err;
> +
> +       err = i2c_smbus_write_byte_data(tj9->client,
> +                                       DATA_CTRL, tj9->data_ctrl);
> +       if (err < 0)
> +               return err;
> +
> +       /* only write INT_CTRL_REG1 if in irq mode */
> +       if (tj9->client->irq) {
> +               err = i2c_smbus_write_byte_data(tj9->client,
> +                                               INT_CTRL1, tj9->int_ctrl);
>                if (err < 0)
>                        return err;
>        }
> -       if (!tj9->hw_initialized) {
> -               msleep(40);
> -               err = kxtj9_hw_init(tj9);
> -               if (err < 0) {
> -                       kxtj9_device_power_off(tj9);
> -                       return err;
> -               }
> -       }
>
> -       return 0;
> -}
> +       err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
> +       if (err < 0)
> +               return err;
>
> -static void kxtj9_get_acceleration_data(struct kxtj9_data *tj9, s16 *xyz)
> -{
> -       int err;
> -       /* Data bytes from hardware xL, xH, yL, yH, zL, zH */
> -       s16 acc_data[3];
> +       tj9->ctrl_reg1 |= PC1_ON;
> +       err = i2c_smbus_write_byte_data(tj9->client,
> +                                       CTRL_REG1, tj9->ctrl_reg1);
> +       if (err < 0)
> +               return err;
>
> -       err = kxtj9_i2c_read(tj9, XOUT_L, (u8 *)acc_data, 6);
> +       err = kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
>        if (err < 0)
> -               dev_err(&tj9->client->dev, "accelerometer data read failed\n");
> +               return err;
>
> -       acc_data[0] = le16_to_cpu(acc_data[0]);
> -       acc_data[1] = le16_to_cpu(acc_data[1]);
> -       acc_data[2] = le16_to_cpu(acc_data[2]);
> +       err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
> +       if (err < 0) {
> +               dev_err(&tj9->client->dev,
> +                       "error clearing interrupt: %d\n", err);
> +               goto fail;
> +       }
>
> -       acc_data[0] >>= tj9->shift;
> -       acc_data[1] >>= tj9->shift;
> -       acc_data[2] >>= tj9->shift;
> +       return 0;
>
> -       xyz[0] = (tj9->pdata.negate_x) ? (-acc_data[tj9->pdata.axis_map_x])
> -                 : (acc_data[tj9->pdata.axis_map_x]);
> -       xyz[1] = (tj9->pdata.negate_y) ? (-acc_data[tj9->pdata.axis_map_y])
> -                 : (acc_data[tj9->pdata.axis_map_y]);
> -       xyz[2] = (tj9->pdata.negate_z) ? (-acc_data[tj9->pdata.axis_map_z])
> -                 : (acc_data[tj9->pdata.axis_map_z]);
> +fail:
> +       kxtj9_device_power_off(tj9);
> +       return err;
> +}
>
> -       input_report_abs(tj9->input_dev, ABS_X, (int) xyz[0]);
> -       input_report_abs(tj9->input_dev, ABS_Y, (int) xyz[1]);
> -       input_report_abs(tj9->input_dev, ABS_Z, (int) xyz[2]);
> -       input_sync(tj9->input_dev);
> +static void kxtj9_disable(struct kxtj9_data *tj9)
> +{
> +       kxtj9_device_power_off(tj9);
>  }
>
> -static irqreturn_t kxtj9_isr(int irq, void *dev)
> +static int kxtj9_input_open(struct input_dev *input)
>  {
> -       struct kxtj9_data *tj9 = dev;
> -       int ret;
> -       s16 xyz[3];
> +       struct kxtj9_data *tj9 = input_get_drvdata(input);
>
> -       /* data ready is the only possible interrupt type */
> -       kxtj9_get_acceleration_data(tj9, xyz);
> +       return kxtj9_enable(tj9);
> +}
>
> -       ret = i2c_smbus_read_byte_data(tj9->client, INT_REL);
> -       if (ret < 0)
> -               dev_err(&tj9->client->dev,
> -                               "error clearing interrupt status: %d\n", ret);
> +static void kxtj9_input_close(struct input_dev *dev)
> +{
> +       struct kxtj9_data *tj9 = input_get_drvdata(dev);
>
> -       return IRQ_HANDLED;
> +       kxtj9_disable(tj9);
>  }
>
> -static int kxtj9_enable(struct kxtj9_data *tj9)
> +static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
> +                                             struct input_dev *input_dev)
>  {
> -       int err = 0;
> -
> -       mutex_lock(&tj9->lock);
> -       if (tj9->enabled == false) {
> -               err = kxtj9_device_power_on(tj9);
> -               if (err < 0) {
> -                       dev_err(&tj9->client->dev,
> -                                       "error during power-on: %d\n", err);
> -                       goto err0;
> -               }
> -               err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
> -               if (err < 0) {
> -                       dev_err(&tj9->client->dev,
> -                                       "error clearing interrupt: %d\n", err);
> -                       goto err0;
> -               }
> -               if (!tj9->client->irq) /* queue polling if not in irq mode */
> -                       schedule_delayed_work(&tj9->input_work,
> -                               msecs_to_jiffies(tj9->pdata.poll_interval));
> -               tj9->enabled = true;
> -       }
> -err0:
> -       mutex_unlock(&tj9->lock);
> -
> -       return err;
> +       __set_bit(EV_ABS, input_dev->evbit);
> +       input_set_abs_params(input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
> +       input_set_abs_params(input_dev, ABS_Y, -G_MAX, G_MAX, FUZZ, FLAT);
> +       input_set_abs_params(input_dev, ABS_Z, -G_MAX, G_MAX, FUZZ, FLAT);
> +
> +       input_dev->name = "kxtj9_accel";
> +       input_dev->id.bustype = BUS_I2C;
> +       input_dev->dev.parent = &tj9->client->dev;
>  }
>
> -static int kxtj9_disable(struct kxtj9_data *tj9)
> +static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
>  {
> -       mutex_lock(&tj9->lock);
> -       if (tj9->enabled == true) {
> -               if (!tj9->client->irq) /* cancel polling if not in irq mode */
> -                       cancel_delayed_work_sync(&tj9->input_work);
> -               kxtj9_device_power_off(tj9);
> -               tj9->enabled = false;
> +       struct input_dev *input_dev;
> +       int err;
> +
> +       input_dev = input_allocate_device();
> +       if (!tj9->input_dev) {
> +               dev_err(&tj9->client->dev, "input device allocate failed\n");
> +               return -ENOMEM;
> +       }
> +
> +       tj9->input_dev = input_dev;
> +
> +       input_dev->open = kxtj9_input_open;
> +       input_dev->close = kxtj9_input_close;
> +       input_set_drvdata(input_dev, tj9);
> +
> +       kxtj9_init_input_device(tj9, input_dev);
> +
> +       err = input_register_device(tj9->input_dev);
> +       if (err) {
> +               dev_err(&tj9->client->dev,
> +                       "unable to register input polled device %s: %d\n",
> +                       tj9->input_dev->name, err);
> +               input_free_device(tj9->input_dev);
>        }
> -       mutex_unlock(&tj9->lock);
>
>        return 0;
>  }
>
> -static void kxtj9_input_work_func(struct work_struct *work)
> +#ifdef CONIFG_KXTJ9_POLLED_MODE
> +static void kxtj9_poll(struct input_polled_dev *dev)
>  {
> -       struct kxtj9_data *tj9 = container_of(work, struct kxtj9_data,
> -                                                       input_work.work);
> -       s16 xyz[3];
> -       mutex_lock(&tj9->lock);
> +       struct kxtj9_data *tj9 = dev->private;
> +       unsigned int poll_interval = dev->poll_interval;
>
> -       kxtj9_get_acceleration_data(tj9, xyz);
> +       kxtj9_report_acceleration_data(tj9);
>
> -       schedule_delayed_work(&tj9->input_work,
> -                       msecs_to_jiffies(tj9->pdata.poll_interval));
> -       mutex_unlock(&tj9->lock);
> +       if (poll_interval != tj9->last_poll_interval) {
> +               kxtj9_update_odr(tj9, poll_interval);
> +               tj9->last_poll_interval = poll_interval;
> +       }
>  }
>
> -static int kxtj9_input_open(struct input_dev *input)
> +static int kxtj9_polled_input_open(struct input_polled_dev *dev)
>  {
> -       return kxtj9_enable(input_get_drvdata(input));
> +       struct kxtj9_data *tj9 = dev->private;
> +
> +       return kxtj9_enable(tj9);
>  }
>
> -static void kxtj9_input_close(struct input_dev *dev)
> +static void kxtj9_polled_input_close(struct input_polled_dev *dev)
>  {
> -       kxtj9_disable(input_get_drvdata(dev));
> +       struct kxtj9_data *tj9 = dev->private;
> +
> +       kxtj9_disable(tj9);
>  }
>
> -static int kxtj9_input_init(struct kxtj9_data *tj9)
> +static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
>  {
> -       int err;
> +       struct input_polled_dev *poll_dev;
>
> -       if (!tj9->client->irq) /* only use input_work if not in irq mode */
> -               INIT_DELAYED_WORK(&tj9->input_work, kxtj9_input_work_func);
> -       tj9->input_dev = input_allocate_device();
> -       if (!tj9->input_dev) {
> -               err = -ENOMEM;
> -               dev_err(&tj9->client->dev, "input device allocate failed\n");
> -               goto err0;
> +       poll_dev = input_allocate_polled_device();
> +       if (!poll_dev) {
> +               dev_err(&tj9->client->dev,
> +                       "Failed to allocate polled device\n");
> +               return -ENOMEM;
>        }
> -       tj9->input_dev->open = kxtj9_input_open;
> -       tj9->input_dev->close = kxtj9_input_close;
>
> -       input_set_drvdata(tj9->input_dev, tj9);
> +       tj9->polled_dev = poll_dev;
> +       tj9->input_dev = poll_dev->input;
>
> -       set_bit(EV_ABS, tj9->input_dev->evbit);
> +       poll_dev->private = tj9;
> +       poll_dev->poll = kxtj9_poll;
> +       poll_dev->open = kxtj9_polled_input_open;
> +       poll_dev->close = kxtj9_polled_input_close;
>
> -       input_set_abs_params(tj9->input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
> -       input_set_abs_params(tj9->input_dev, ABS_Y, -G_MAX, G_MAX, FUZZ, FLAT);
> -       input_set_abs_params(tj9->input_dev, ABS_Z, -G_MAX, G_MAX, FUZZ, FLAT);
> +       kxtj9_init_input_device(tj9, poll_dev->input);
>
> -       tj9->input_dev->name = "kxtj9_accel";
> -       tj9->input_dev->id.bustype = BUS_I2C;
> -       tj9->input_dev->dev.parent = &tj9->client->dev;
> -
> -       err = input_register_device(tj9->input_dev);
> +       err = input_register_polled_device(poll_dev);
>        if (err) {
>                dev_err(&tj9->client->dev,
> -                       "unable to register input polled device %s: %d\n",
> -                       tj9->input_dev->name, err);
> -               goto err1;
> +                       "Unable to register polled device, err=%d\n", err);
> +               input_free_polled_device(poll_dev);
> +               return 0;
>        }
>
>        return 0;
> -err1:
> -       input_free_device(tj9->input_dev);
> -err0:
> -       return err;
>  }
>
> -/* kxtj9_delay_show: returns the currently selected poll interval (in ms) */
> -static ssize_t kxtj9_get_poll(struct device *dev,
> -                               struct device_attribute *attr, char *buf)
> +static kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
>  {
> -       struct kxtj9_data *tj9 = i2c_get_clientdata(to_i2c_client(dev));
> -       return sprintf(buf, "%d\n", tj9->pdata.poll_interval);
> +       input_unregister_polled_device(tj9->poll_dev);
> +       input_free_polled_device(tj9->poll_dev);
>  }
>
> -/* kxtj9_delay_store: allows the user to select a new poll interval (in ms) */
> -static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
> -                                               const char *buf, size_t count)
> +#else
> +
> +static inline int kxtj9_setup_polled_device(struct kxtj9_data *tj9)
>  {
> -       struct kxtj9_data *tj9 = i2c_get_clientdata(to_i2c_client(dev));
> -       unsigned long interval;
> -       int ret = kstrtoul(buf, 10, &interval);
> -       if (ret < 0)
> -               return ret;
> +       return -ENOSYS;
> +}
>
> -       mutex_lock(&tj9->lock);
> -       /* set current interval to the greater of the minimum interval or
> -       the requested interval */
> -       tj9->pdata.poll_interval = max((int)interval, tj9->pdata.min_interval);
> -       mutex_unlock(&tj9->lock);
> +static inline void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
> +{
> +}
> +#endif
>
> -       kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
> +static int __devinit kxtj9_verify(struct kxtj9_data *tj9)
> +{
> +       int retval;
>
> -       return count;
> -}
> -static DEVICE_ATTR(poll, S_IRUGO|S_IWUSR, kxtj9_get_poll, kxtj9_set_poll);
> +       retval = kxtj9_device_power_on(tj9);
> +       if (retval < 0)
> +               return retval;
>
> -static struct attribute *kxtj9_attributes[] = {
> -       &dev_attr_poll.attr,
> -       NULL
> -};
> +       retval = i2c_smbus_read_byte_data(tj9->client, WHO_AM_I);
> +       if (retval < 0) {
> +               dev_err(&tj9->client->dev, "read err int source\n");
> +               goto out;
> +       }
>
> -static struct attribute_group kxtj9_attribute_group = {
> -       .attrs = kxtj9_attributes
> -};
> +       retval = retval != 0x06 ? - EIO : 0;
> +
> +out:
> +       kxtj9_device_power_off(tj9);
> +       return retval;
> +}
>
>  static int __devinit kxtj9_probe(struct i2c_client *client,
> -                                               const struct i2c_device_id *id)
> +                                const struct i2c_device_id *id)
>  {
> -       int err = -1;
> -       struct kxtj9_data *tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
> -       if (tj9 == NULL) {
> -               dev_err(&client->dev,
> -                       "failed to allocate memory for module data\n");
> -               err = -ENOMEM;
> -               goto err0;
> +       const struct kxtj9_platform_data *pdata = client->dev.platform_data;
> +       struct kxtj9_data *tj9;
> +       int err;
> +
> +       if (!i2c_check_functionality(client->adapter,
> +                               I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE_DATA)) {
> +               dev_err(&client->dev, "client is not i2c capable\n");
> +               return -ENXIO;
>        }
> -       if (client->dev.platform_data == NULL) {
> +
> +       if (!pdata) {
>                dev_err(&client->dev, "platform data is NULL; exiting\n");
> -               err = -ENODEV;
> -               goto err0;
> +               return -EINVAL;
>        }
> -       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
> -                                               I2C_FUNC_SMBUS_BYTE_DATA)) {
> -               dev_err(&client->dev, "client not i2c capable\n");
> -               err = -ENODEV;
> -               goto err0;
> +
> +       tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
> +       if (!tj9) {
> +               dev_err(&client->dev,
> +                       "failed to allocate memory for module data\n");
> +               return -ENOMEM;
>        }
> -       mutex_init(&tj9->lock);
> -       mutex_lock(&tj9->lock);
> +
>        tj9->client = client;
> -       i2c_set_clientdata(client, tj9);
> +       tj9->pdata = *pdata;
>
> -       memcpy(&tj9->pdata, client->dev.platform_data, sizeof(tj9->pdata));
> -       if (tj9->pdata.init) {
> -               err = tj9->pdata.init();
> +       if (pdata->init) {
> +               err = pdata->init();
>                if (err < 0)
> -                       goto err1;
> +                       goto err_free_mem;
>        }
>
> -       err = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
> -       if (err)
> -               goto err2;
> +       err = kxtj9_verify(tj9);
> +       if (err < 0) {
> +               dev_err(&client->dev, "device not recognized\n");
> +               goto err_pdata_exit;
> +       }
>
> -       tj9->resume[RES_CTRL_REG1] = tj9->pdata.res_12bit | tj9->pdata.g_range;
> -       tj9->resume[RES_DATA_CTRL] = tj9->pdata.data_odr_init;
> +       tj9->ctrl_reg1 = tj9->pdata.res_12bit | tj9->pdata.g_range;
> +       tj9->data_ctrl = tj9->pdata.data_odr_init;
>
>        if (client->irq) {
> -               err = request_threaded_irq(client->irq, NULL, kxtj9_isr,
> -                       IRQF_TRIGGER_RISING | IRQF_ONESHOT, "kxtj9-irq", tj9);
> -               if (err < 0) {
> -                       pr_err("%s: request irq failed: %d\n", __func__, err);
> -                       goto err5;
> -               }
> -               /* if in irq mode, populate INT_CTRL_REG1 and enable DRDY */
> -               tj9->resume[RES_INT_CTRL1] = KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
> -               tj9->resume[RES_CTRL_REG1] |= DRDYE;
> -       }
> +               /* If in irq mode, populate INT_CTRL_REG1 and enable DRDY. */
> +               tj9->int_ctrl |= KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
> +               tj9->ctrl_reg1 |= DRDYE;
>
> -       err = kxtj9_input_init(tj9);
> -       if (err < 0)
> -               goto err3;
> +               err = kxtj9_setup_input_device(tj9);
> +               if (err)
> +                       goto err_pdata_exit;
>
> -       err = kxtj9_device_power_on(tj9);
> -       if (err < 0)
> -               goto err4;
> -       tj9->enabled = true;
> +               err = request_threaded_irq(client->irq, NULL, kxtj9_isr,
> +                                          IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +                                          "kxtj9-irq", tj9);
> +               if (err) {
> +                       dev_err(&client->dev, "request irq failed: %d\n", err);
> +                       goto err_destroy_input;
> +               }
>
> -       err = kxtj9_verify(tj9);
> -       if (err < 0) {
> -               dev_err(&client->dev, "device not recognized\n");
> -               goto err5;
> +       } else {
> +               err = kxtj9_setup_polled_device(tj9);
> +               if (err)
> +                       goto err_pdata_exit;
>        }
>
> -       mutex_unlock(&tj9->lock);
> -
> +       i2c_set_clientdata(client, tj9);
>        return 0;
>
> -err5:
> -       kxtj9_device_power_off(tj9);
> -err4:
> +err_destroy_input:
>        input_unregister_device(tj9->input_dev);
> -err3:
> -       sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
> -err2:
> +err_pdata_exit:
>        if (tj9->pdata.exit)
>                tj9->pdata.exit();
> -err1:
> -       mutex_unlock(&tj9->lock);
> -err0:
> +err_free_mem:
>        kfree(tj9);
>        return err;
>  }
> @@ -539,50 +521,69 @@ static int __devexit kxtj9_remove(struct i2c_client *client)
>  {
>        struct kxtj9_data *tj9 = i2c_get_clientdata(client);
>
> -       sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
> -       if (client->irq)
> +       if (client->irq) {
>                free_irq(client->irq, tj9);
> -       input_unregister_device(tj9->input_dev);
> -       kxtj9_device_power_off(tj9);
> +               input_unregister_device(tj9->input_dev);
> +       } else {
> +               kxtj9_teardown_polled_device(tj9);
> +       }
> +
>        if (tj9->pdata.exit)
>                tj9->pdata.exit();
> +
>        kfree(tj9);
>
>        return 0;
>  }
>
> -#ifdef CONFIG_PM
> -static int kxtj9_resume(struct device *dev)
> +#ifdef CONFIG_PM_SLEEP
> +static int kxtj9_suspend(struct device *dev)
>  {
> -       return kxtj9_enable(i2c_get_clientdata(to_i2c_client(dev)));
> +       struct i2c_client *client = to_i2c_client(dev);
> +       struct kxtj9_data *tj9 = i2c_get_clientdata(client);
> +       struct input_dev *input_dev = tj9->input_dev;
> +
> +       mutex_lock(&input_dev->mutex);
> +
> +       if (input_dev->users)
> +               kxtj9_disable(tj9);
> +
> +       mutex_unlock(&input_dev->mutex);
> +       return 0;
>  }
>
> -static int kxtj9_suspend(struct device *dev)
> +static int kxtj9_resume(struct device *dev)
>  {
> -       return kxtj9_disable(i2c_get_clientdata(to_i2c_client(dev)));
> -}
> +       struct i2c_client *client = to_i2c_client(dev);
> +       struct kxtj9_data *tj9 = i2c_get_clientdata(client);
> +       struct input_dev *input_dev = tj9->input_dev;
> +       int retval = 0;
>
> -static const struct dev_pm_ops kxtj9_pm_ops = {
> -       .suspend = kxtj9_suspend,
> -       .resume = kxtj9_resume,
> -};
> +       mutex_lock(&input_dev->mutex);
> +
> +       if (input_dev->users)
> +               kxtj9_enable(tj9);
> +
> +       mutex_unlock(&input_dev->mutex);
> +       return retval;
> +}
>  #endif
>
> +static SIMPLE_DEV_PM_OPS(kxtj9_pm_ops, kxtj9_suspend, kxtj9_resume);
> +
>  static const struct i2c_device_id kxtj9_id[] = {
> -       {NAME, 0},
> -       {},
> +       { NAME, 0 },
> +       { },
>  };
>
>  MODULE_DEVICE_TABLE(i2c, kxtj9_id);
>
>  static struct i2c_driver kxtj9_driver = {
>        .driver = {
> -                  .name = NAME,
> -                  .owner = THIS_MODULE,
> -#ifdef CONFIG_PM
> -                  .pm = &kxtj9_pm_ops,
> -#endif
> -                  },
> +               .name = NAME,
> +               .owner = THIS_MODULE,
> +               .pm = &kxtj9_pm_ops,
> +       },
>        .probe = kxtj9_probe,
>        .remove = __devexit_p(kxtj9_remove),
>        .id_table = kxtj9_id,
> @@ -592,13 +593,12 @@ static int __init kxtj9_init(void)
>  {
>        return i2c_add_driver(&kxtj9_driver);
>  }
> +module_init(kxtj9_init);
>
>  static void __exit kxtj9_exit(void)
>  {
>        i2c_del_driver(&kxtj9_driver);
>  }
> -
> -module_init(kxtj9_init);
>  module_exit(kxtj9_exit);
>
>  MODULE_DESCRIPTION("KXTJ9 accelerometer driver");
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] input: Add support for Kionix KXTJ9 accelerometer
  2011-06-07 18:04   ` Christopher Hudson
@ 2011-06-16 19:41     ` Christopher Hudson
  0 siblings, 0 replies; 4+ messages in thread
From: Christopher Hudson @ 2011-06-16 19:41 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input, Jonathan Cameron, Chris Hudson

On Tue, Jun 7, 2011 at 2:04 PM, Christopher Hudson
<chris.hudson.comp.eng@gmail.com> wrote:
> Hi Dmitry,
>
> My apologies for the delayed reply.  I couldn't get your patch to
> apply cleanly to my tree, but I was able to manually add the changes.
> I liked many of these changes, but there were 2 main issues when
> testing on the hardware:
>
> - There is no poll interval selection available in irq mode.

I just re-read this and realized that it probably created some
confusion.  The issue is that the output data rate of the sensor
controls the frequency of interrupts in IRQ mode.  We need to allow
for control of the data rate (otherwise the sensor will be locked at a
single ODR when using the data ready interrupt), so it seems to make
the most sense to use the same sysfs control whether in polling or IRQ
mode.  Any thoughts?

> I was planning to add my previous versions of get_poll and set_poll to the
> #else branch of CONFIG_KXTJ9_POLLED_MODE, and create this sysfs group
> in the client->irq branch in probe.  Unfortunately, this seems to
> render the use of input_polled_dev a bit redundant.  What do you think
> about this implementation?
>
> - For some reason, whenever I try to use le16_to_cpu followed by a
> bit-shift operation without first storing the results to memory I end
> up with garbled data.  What seems to be happening is the bit-shift is
> being performed before the le16_to_cpu operation.  I've tried various
> implementations with parentheses to try to separate the two
> operations, but the only thing I have found to work is to store the
> results of the le16_to_cpu operation first, then bit-shift the result
> in another operation:
> x = le16_to_cpu(acc_data[tj9->pdata.axis_map_x]);
> x >>= tj9->shift;
> Note that I also preserved the axis_map platform data variable in this
> implementation; was it your intent to do away with this feature?
>
>
> On Tue, May 31, 2011 at 4:03 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> Hi Chris,
>> On Thu, May 26, 2011 at 06:40:20PM -0400, Chris Hudson wrote:
>>> Quite a few changes this time around (thanks Jonathan and Dmitry!):
>>> - The sysfs node for poll_interval was changed to emulate input_polldev.  I
>>>   could not actually use input_polldev because some commands need to be sent
>>>   to the hardware when the poll interval is changed.
>>> - Locking cleaned up throughout
>>> - Cleared up distinction between IRQ and polling modes: if client->irq, then
>>>   irq mode is enabled.  Removed register bits required for irq from the header;
>>>   these will be set in the driver if client->irq is populated.
>>> - Put .suspend and .resume into struct dev_pm_ops.
>>> - Many other smaller changes, unnecessary variables removed, etc.
>>>
>>> Signed-off-by: Chris Hudson <chudson@kionix.com>
>>> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>>
>> Could you please tell me how badly the patch below breaks the device?
>>
>> Thanks!
>>
>> --
>> Dmitry
>>
>>
>> Input: KXTJ9 assorted changes
>>
>> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>
>> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
>> ---
>>
>>  drivers/input/misc/Kconfig |   27 +-
>>  drivers/input/misc/kxtj9.c |  654 ++++++++++++++++++++++----------------------
>>  2 files changed, 344 insertions(+), 337 deletions(-)
>>
>>
>> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
>> index 567f3d2..7010623 100644
>> --- a/drivers/input/misc/Kconfig
>> +++ b/drivers/input/misc/Kconfig
>> @@ -209,6 +209,23 @@ config INPUT_KEYSPAN_REMOTE
>>          To compile this driver as a module, choose M here: the module will
>>          be called keyspan_remote.
>>
>> +config INPUT_KXTJ9
>> +       tristate "Kionix KXTJ9 tri-axis digital accelerometer"
>> +       depends on I2C
>> +       help
>> +         If you say yes here you get support for the Kionix KXTJ9 digital
>> +         tri-axis accelerometer.
>> +
>> +         This driver can also be built as a module.  If so, the module
>> +         will be called kxtj9.
>> +
>> +config KXTJ9_POLLED_MODE
>> +       bool "Enable polling mode support"
>> +       depends on INPUT_KXTJ9
>> +       select INPUT_POLLDEV
>> +       help
>> +         Say Y here if you need accelerometer to work in polling mode.
>> +
>>  config INPUT_POWERMATE
>>        tristate "Griffin PowerMate and Contour Jog support"
>>        depends on USB_ARCH_HAS_HCD
>> @@ -467,14 +484,4 @@ config INPUT_XEN_KBDDEV_FRONTEND
>>          To compile this driver as a module, choose M here: the
>>          module will be called xen-kbdfront.
>>
>> -config INPUT_KXTJ9
>> -       tristate "Kionix KXTJ9 tri-axis digital accelerometer"
>> -       depends on I2C && SYSFS
>> -       help
>> -         If you say yes here you get support for the Kionix KXTJ9 digital
>> -         tri-axis accelerometer.
>> -
>> -         This driver can also be built as a module.  If so, the module
>> -         will be called kxtj9.
>> -
>>  endif
>> diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
>> index ef83182..1536bf4 100644
>> --- a/drivers/input/misc/kxtj9.c
>> +++ b/drivers/input/misc/kxtj9.c
>> @@ -61,63 +61,86 @@ static const struct {
>>        unsigned int cutoff;
>>        u8 mask;
>>  } kxtj9_odr_table[] = {
>> -       {
>> -       3,      ODR800F}, {
>> -       5,      ODR400F}, {
>> -       10,     ODR200F}, {
>> -       20,     ODR100F}, {
>> -       40,     ODR50F}, {
>> -       80,     ODR25F}, {
>> -       0,      ODR12_5F},
>> +       { 3,    ODR800F },
>> +       { 5,    ODR400F },
>> +       { 10,   ODR200F },
>> +       { 20,   ODR100F },
>> +       { 40,   ODR50F  },
>> +       { 80,   ODR25F  },
>> +       { 0,    ODR12_5F},
>>  };
>>
>>  struct kxtj9_data {
>>        struct i2c_client *client;
>>        struct kxtj9_platform_data pdata;
>> -       struct mutex lock;
>> -       struct delayed_work input_work;
>>        struct input_dev *input_dev;
>> -
>> -       bool hw_initialized;
>> -       bool enabled;
>> +#ifdef CONFIG_KXTJ9_POLLED_MODE
>> +       struct input_dev *input_polled_dev;
>> +#endif
>> +       unsigned int last_poll_interval;
>>        u8 shift;
>> -       u8 resume[RESUME_ENTRIES];
>> +       u8 ctrl_reg1;
>> +       u8 data_ctrl;
>> +       u8 int_ctrl;
>>  };
>>
>>  static int kxtj9_i2c_read(struct kxtj9_data *tj9, u8 addr, u8 *data, int len)
>>  {
>> -       int err;
>> -
>>        struct i2c_msg msgs[] = {
>>                {
>> -                .addr = tj9->client->addr,
>> -                .flags = tj9->client->flags,
>> -                .len = 1,
>> -                .buf = &addr,
>> -                },
>> +                       .addr = tj9->client->addr,
>> +                       .flags = tj9->client->flags,
>> +                       .len = 1,
>> +                       .buf = &addr,
>> +               },
>>                {
>> -                .addr = tj9->client->addr,
>> -                .flags = tj9->client->flags | I2C_M_RD,
>> -                .len = len,
>> -                .buf = data,
>> -                },
>> +                       .addr = tj9->client->addr,
>> +                       .flags = tj9->client->flags | I2C_M_RD,
>> +                       .len = len,
>> +                       .buf = data,
>> +               },
>>        };
>> -       err = i2c_transfer(tj9->client->adapter, msgs, 2);
>>
>> -       return err;
>> +       return i2c_transfer(tj9->client->adapter, msgs, 2);
>>  }
>>
>> -static int kxtj9_verify(struct kxtj9_data *tj9)
>> +static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
>>  {
>> -       int ret;
>> +       s16 acc_data[3]; /* Data bytes from hardware xL, xH, yL, yH, zL, zH */
>> +       s16 x, y, z;
>> +       int err;
>>
>> -       ret = i2c_smbus_read_byte_data(tj9->client, WHO_AM_I);
>> -       if (ret < 0)
>> -               dev_err(&tj9->client->dev, "read err int source\n");
>> -       if (ret != 0x06)
>> -               ret = -EIO;
>> +       err = kxtj9_i2c_read(tj9, XOUT_L, (u8 *)acc_data, 6);
>> +       if (err < 0)
>> +               dev_err(&tj9->client->dev, "accelerometer data read failed\n");
>> +
>> +       x = le16_to_cpu(acc_data[0]) >> tj9->shift;
>> +       y = le16_to_cpu(acc_data[1]) >> tj9->shift;
>> +       z = le16_to_cpu(acc_data[2]) >> tj9->shift;
>> +
>> +       input_report_abs(tj9->input_dev, ABS_X,
>> +                        tj9->pdata.negate_x ? -x : x);
>> +       input_report_abs(tj9->input_dev, ABS_Y,
>> +                        tj9->pdata.negate_y ? -y : y);
>> +       input_report_abs(tj9->input_dev, ABS_Z,
>> +                        tj9->pdata.negate_x ? -y : y);
>> +       input_sync(tj9->input_dev);
>> +}
>>
>> -       return ret;
>> +static irqreturn_t kxtj9_isr(int irq, void *dev)
>> +{
>> +       struct kxtj9_data *tj9 = dev;
>> +       int err;
>> +
>> +       /* data ready is the only possible interrupt type */
>> +       kxtj9_report_acceleration_data(tj9);
>> +
>> +       err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
>> +       if (err < 0)
>> +               dev_err(&tj9->client->dev,
>> +                       "error clearing interrupt status: %d\n", err);
>> +
>> +       return IRQ_HANDLED;
>>  }
>>
>>  static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
>> @@ -126,17 +149,21 @@ static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
>>        case KXTJ9_G_2G:
>>                tj9->shift = SHIFT_ADJ_2G;
>>                break;
>> +
>>        case KXTJ9_G_4G:
>>                tj9->shift = SHIFT_ADJ_4G;
>>                break;
>> +
>>        case KXTJ9_G_8G:
>>                tj9->shift = SHIFT_ADJ_8G;
>>                break;
>> +
>>        default:
>>                return -EINVAL;
>>        }
>> -       tj9->resume[RES_CTRL_REG1] = (tj9->resume[RES_CTRL_REG1] & 0xE7)
>> -                                                               | new_g_range;
>> +
>> +       tj9->ctrl_reg1 &= 0xe7;
>> +       tj9->ctrl_reg1 |= new_g_range;
>>
>>        return 0;
>>  }
>> @@ -145,72 +172,40 @@ static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
>>  {
>>        int err;
>>        int i;
>> -       u8 config;
>> -       u8 temp = 0;
>>
>>        /* Use the lowest ODR that can support the requested poll interval */
>>        for (i = 0; i < ARRAY_SIZE(kxtj9_odr_table); i++) {
>> -               config = kxtj9_odr_table[i].mask;
>> +               tj9->data_ctrl = kxtj9_odr_table[i].mask;
>>                if (poll_interval < kxtj9_odr_table[i].cutoff)
>>                        break;
>>        }
>>
>> -       if (tj9->enabled == true) {
>> -               err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, temp);
>> -               if (err < 0)
>> -                       return err;
>> -               err = i2c_smbus_write_byte_data(tj9->client, DATA_CTRL, config);
>> -               if (err < 0)
>> -                       return err;
>> -               temp = tj9->resume[RES_CTRL_REG1];
>> -               err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, temp);
>> -               if (err < 0)
>> -                       return err;
>> -               /* Cancel and restart polling if not in irq mode */
>> -               if (!tj9->client->irq) {
>> -                       cancel_delayed_work_sync(&tj9->input_work);
>> -                       schedule_delayed_work(&tj9->input_work,
>> -                                       msecs_to_jiffies(poll_interval));
>> -               }
>> -       }
>> -       tj9->resume[RES_DATA_CTRL] = config;
>> -
>> -       return 0;
>> -}
>> -
>> -static int kxtj9_hw_init(struct kxtj9_data *tj9)
>> -{
>> -       int err;
>> -       u8 buf = 0;
>> -
>> -       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
>> -       if (err < 0)
>> -               return err;
>> -       err = i2c_smbus_write_byte_data(tj9->client, DATA_CTRL,
>> -                                               tj9->resume[RES_DATA_CTRL]);
>> -       if (err < 0)
>> -               return err;
>> -       if (tj9->client->irq) /* only write INT_CTRL_REG1 if in irq mode */
>> -               err = i2c_smbus_write_byte_data(tj9->client, INT_CTRL1,
>> -                                               tj9->resume[RES_INT_CTRL1]);
>> +       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, 0);
>>        if (err < 0)
>>                return err;
>>
>> -       err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
>> +       err = i2c_smbus_write_byte_data(tj9->client,
>> +                                       DATA_CTRL, tj9->data_ctrl);
>>        if (err < 0)
>>                return err;
>>
>> -       buf = (tj9->resume[RES_CTRL_REG1] | PC1_ON);
>> -       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
>> +       err = i2c_smbus_write_byte_data(tj9->client,
>> +                                       CTRL_REG1, tj9->ctrl_reg1);
>>        if (err < 0)
>>                return err;
>> -       tj9->resume[RES_CTRL_REG1] = buf;
>>
>> -       err = kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
>> -       if (err < 0)
>> -               return err;
>> +       return 0;
>> +}
>>
>> -       tj9->hw_initialized = true;
>> +static int kxtj9_device_power_on(struct kxtj9_data *tj9)
>> +{
>> +       int err;
>> +
>> +       if (tj9->pdata.power_on) {
>> +               err = tj9->pdata.power_on();
>> +               if (err < 0)
>> +                       return err;
>> +       }
>>
>>        return 0;
>>  }
>> @@ -218,319 +213,306 @@ static int kxtj9_hw_init(struct kxtj9_data *tj9)
>>  static void kxtj9_device_power_off(struct kxtj9_data *tj9)
>>  {
>>        int err;
>> -       u8 buf;
>>
>> -       buf = tj9->resume[RES_CTRL_REG1] & PC1_OFF;
>> -       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, buf);
>> +       tj9->ctrl_reg1 &= PC1_OFF;
>> +       err = i2c_smbus_write_byte_data(tj9->client,
>> +                                       CTRL_REG1, tj9->ctrl_reg1);
>>        if (err < 0)
>>                dev_err(&tj9->client->dev, "soft power off failed\n");
>> +
>>        if (tj9->pdata.power_off)
>>                tj9->pdata.power_off();
>> -       tj9->resume[RES_CTRL_REG1] = buf;
>> -       tj9->hw_initialized = false;
>>  }
>>
>> -static int kxtj9_device_power_on(struct kxtj9_data *tj9)
>> +static int kxtj9_enable(struct kxtj9_data *tj9)
>>  {
>>        int err;
>>
>> -       if (tj9->pdata.power_on) {
>> -               err = tj9->pdata.power_on();
>> +       err = kxtj9_device_power_on(tj9);
>> +       if (err < 0)
>> +               return err;
>> +
>> +       err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, 0);
>> +       if (err < 0)
>> +               return err;
>> +
>> +       err = i2c_smbus_write_byte_data(tj9->client,
>> +                                       DATA_CTRL, tj9->data_ctrl);
>> +       if (err < 0)
>> +               return err;
>> +
>> +       /* only write INT_CTRL_REG1 if in irq mode */
>> +       if (tj9->client->irq) {
>> +               err = i2c_smbus_write_byte_data(tj9->client,
>> +                                               INT_CTRL1, tj9->int_ctrl);
>>                if (err < 0)
>>                        return err;
>>        }
>> -       if (!tj9->hw_initialized) {
>> -               msleep(40);
>> -               err = kxtj9_hw_init(tj9);
>> -               if (err < 0) {
>> -                       kxtj9_device_power_off(tj9);
>> -                       return err;
>> -               }
>> -       }
>>
>> -       return 0;
>> -}
>> +       err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
>> +       if (err < 0)
>> +               return err;
>>
>> -static void kxtj9_get_acceleration_data(struct kxtj9_data *tj9, s16 *xyz)
>> -{
>> -       int err;
>> -       /* Data bytes from hardware xL, xH, yL, yH, zL, zH */
>> -       s16 acc_data[3];
>> +       tj9->ctrl_reg1 |= PC1_ON;
>> +       err = i2c_smbus_write_byte_data(tj9->client,
>> +                                       CTRL_REG1, tj9->ctrl_reg1);
>> +       if (err < 0)
>> +               return err;
>>
>> -       err = kxtj9_i2c_read(tj9, XOUT_L, (u8 *)acc_data, 6);
>> +       err = kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
>>        if (err < 0)
>> -               dev_err(&tj9->client->dev, "accelerometer data read failed\n");
>> +               return err;
>>
>> -       acc_data[0] = le16_to_cpu(acc_data[0]);
>> -       acc_data[1] = le16_to_cpu(acc_data[1]);
>> -       acc_data[2] = le16_to_cpu(acc_data[2]);
>> +       err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
>> +       if (err < 0) {
>> +               dev_err(&tj9->client->dev,
>> +                       "error clearing interrupt: %d\n", err);
>> +               goto fail;
>> +       }
>>
>> -       acc_data[0] >>= tj9->shift;
>> -       acc_data[1] >>= tj9->shift;
>> -       acc_data[2] >>= tj9->shift;
>> +       return 0;
>>
>> -       xyz[0] = (tj9->pdata.negate_x) ? (-acc_data[tj9->pdata.axis_map_x])
>> -                 : (acc_data[tj9->pdata.axis_map_x]);
>> -       xyz[1] = (tj9->pdata.negate_y) ? (-acc_data[tj9->pdata.axis_map_y])
>> -                 : (acc_data[tj9->pdata.axis_map_y]);
>> -       xyz[2] = (tj9->pdata.negate_z) ? (-acc_data[tj9->pdata.axis_map_z])
>> -                 : (acc_data[tj9->pdata.axis_map_z]);
>> +fail:
>> +       kxtj9_device_power_off(tj9);
>> +       return err;
>> +}
>>
>> -       input_report_abs(tj9->input_dev, ABS_X, (int) xyz[0]);
>> -       input_report_abs(tj9->input_dev, ABS_Y, (int) xyz[1]);
>> -       input_report_abs(tj9->input_dev, ABS_Z, (int) xyz[2]);
>> -       input_sync(tj9->input_dev);
>> +static void kxtj9_disable(struct kxtj9_data *tj9)
>> +{
>> +       kxtj9_device_power_off(tj9);
>>  }
>>
>> -static irqreturn_t kxtj9_isr(int irq, void *dev)
>> +static int kxtj9_input_open(struct input_dev *input)
>>  {
>> -       struct kxtj9_data *tj9 = dev;
>> -       int ret;
>> -       s16 xyz[3];
>> +       struct kxtj9_data *tj9 = input_get_drvdata(input);
>>
>> -       /* data ready is the only possible interrupt type */
>> -       kxtj9_get_acceleration_data(tj9, xyz);
>> +       return kxtj9_enable(tj9);
>> +}
>>
>> -       ret = i2c_smbus_read_byte_data(tj9->client, INT_REL);
>> -       if (ret < 0)
>> -               dev_err(&tj9->client->dev,
>> -                               "error clearing interrupt status: %d\n", ret);
>> +static void kxtj9_input_close(struct input_dev *dev)
>> +{
>> +       struct kxtj9_data *tj9 = input_get_drvdata(dev);
>>
>> -       return IRQ_HANDLED;
>> +       kxtj9_disable(tj9);
>>  }
>>
>> -static int kxtj9_enable(struct kxtj9_data *tj9)
>> +static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
>> +                                             struct input_dev *input_dev)
>>  {
>> -       int err = 0;
>> -
>> -       mutex_lock(&tj9->lock);
>> -       if (tj9->enabled == false) {
>> -               err = kxtj9_device_power_on(tj9);
>> -               if (err < 0) {
>> -                       dev_err(&tj9->client->dev,
>> -                                       "error during power-on: %d\n", err);
>> -                       goto err0;
>> -               }
>> -               err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
>> -               if (err < 0) {
>> -                       dev_err(&tj9->client->dev,
>> -                                       "error clearing interrupt: %d\n", err);
>> -                       goto err0;
>> -               }
>> -               if (!tj9->client->irq) /* queue polling if not in irq mode */
>> -                       schedule_delayed_work(&tj9->input_work,
>> -                               msecs_to_jiffies(tj9->pdata.poll_interval));
>> -               tj9->enabled = true;
>> -       }
>> -err0:
>> -       mutex_unlock(&tj9->lock);
>> -
>> -       return err;
>> +       __set_bit(EV_ABS, input_dev->evbit);
>> +       input_set_abs_params(input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
>> +       input_set_abs_params(input_dev, ABS_Y, -G_MAX, G_MAX, FUZZ, FLAT);
>> +       input_set_abs_params(input_dev, ABS_Z, -G_MAX, G_MAX, FUZZ, FLAT);
>> +
>> +       input_dev->name = "kxtj9_accel";
>> +       input_dev->id.bustype = BUS_I2C;
>> +       input_dev->dev.parent = &tj9->client->dev;
>>  }
>>
>> -static int kxtj9_disable(struct kxtj9_data *tj9)
>> +static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
>>  {
>> -       mutex_lock(&tj9->lock);
>> -       if (tj9->enabled == true) {
>> -               if (!tj9->client->irq) /* cancel polling if not in irq mode */
>> -                       cancel_delayed_work_sync(&tj9->input_work);
>> -               kxtj9_device_power_off(tj9);
>> -               tj9->enabled = false;
>> +       struct input_dev *input_dev;
>> +       int err;
>> +
>> +       input_dev = input_allocate_device();
>> +       if (!tj9->input_dev) {
>> +               dev_err(&tj9->client->dev, "input device allocate failed\n");
>> +               return -ENOMEM;
>> +       }
>> +
>> +       tj9->input_dev = input_dev;
>> +
>> +       input_dev->open = kxtj9_input_open;
>> +       input_dev->close = kxtj9_input_close;
>> +       input_set_drvdata(input_dev, tj9);
>> +
>> +       kxtj9_init_input_device(tj9, input_dev);
>> +
>> +       err = input_register_device(tj9->input_dev);
>> +       if (err) {
>> +               dev_err(&tj9->client->dev,
>> +                       "unable to register input polled device %s: %d\n",
>> +                       tj9->input_dev->name, err);
>> +               input_free_device(tj9->input_dev);
>>        }
>> -       mutex_unlock(&tj9->lock);
>>
>>        return 0;
>>  }
>>
>> -static void kxtj9_input_work_func(struct work_struct *work)
>> +#ifdef CONIFG_KXTJ9_POLLED_MODE
>> +static void kxtj9_poll(struct input_polled_dev *dev)
>>  {
>> -       struct kxtj9_data *tj9 = container_of(work, struct kxtj9_data,
>> -                                                       input_work.work);
>> -       s16 xyz[3];
>> -       mutex_lock(&tj9->lock);
>> +       struct kxtj9_data *tj9 = dev->private;
>> +       unsigned int poll_interval = dev->poll_interval;
>>
>> -       kxtj9_get_acceleration_data(tj9, xyz);
>> +       kxtj9_report_acceleration_data(tj9);
>>
>> -       schedule_delayed_work(&tj9->input_work,
>> -                       msecs_to_jiffies(tj9->pdata.poll_interval));
>> -       mutex_unlock(&tj9->lock);
>> +       if (poll_interval != tj9->last_poll_interval) {
>> +               kxtj9_update_odr(tj9, poll_interval);
>> +               tj9->last_poll_interval = poll_interval;
>> +       }
>>  }
>>
>> -static int kxtj9_input_open(struct input_dev *input)
>> +static int kxtj9_polled_input_open(struct input_polled_dev *dev)
>>  {
>> -       return kxtj9_enable(input_get_drvdata(input));
>> +       struct kxtj9_data *tj9 = dev->private;
>> +
>> +       return kxtj9_enable(tj9);
>>  }
>>
>> -static void kxtj9_input_close(struct input_dev *dev)
>> +static void kxtj9_polled_input_close(struct input_polled_dev *dev)
>>  {
>> -       kxtj9_disable(input_get_drvdata(dev));
>> +       struct kxtj9_data *tj9 = dev->private;
>> +
>> +       kxtj9_disable(tj9);
>>  }
>>
>> -static int kxtj9_input_init(struct kxtj9_data *tj9)
>> +static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
>>  {
>> -       int err;
>> +       struct input_polled_dev *poll_dev;
>>
>> -       if (!tj9->client->irq) /* only use input_work if not in irq mode */
>> -               INIT_DELAYED_WORK(&tj9->input_work, kxtj9_input_work_func);
>> -       tj9->input_dev = input_allocate_device();
>> -       if (!tj9->input_dev) {
>> -               err = -ENOMEM;
>> -               dev_err(&tj9->client->dev, "input device allocate failed\n");
>> -               goto err0;
>> +       poll_dev = input_allocate_polled_device();
>> +       if (!poll_dev) {
>> +               dev_err(&tj9->client->dev,
>> +                       "Failed to allocate polled device\n");
>> +               return -ENOMEM;
>>        }
>> -       tj9->input_dev->open = kxtj9_input_open;
>> -       tj9->input_dev->close = kxtj9_input_close;
>>
>> -       input_set_drvdata(tj9->input_dev, tj9);
>> +       tj9->polled_dev = poll_dev;
>> +       tj9->input_dev = poll_dev->input;
>>
>> -       set_bit(EV_ABS, tj9->input_dev->evbit);
>> +       poll_dev->private = tj9;
>> +       poll_dev->poll = kxtj9_poll;
>> +       poll_dev->open = kxtj9_polled_input_open;
>> +       poll_dev->close = kxtj9_polled_input_close;
>>
>> -       input_set_abs_params(tj9->input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
>> -       input_set_abs_params(tj9->input_dev, ABS_Y, -G_MAX, G_MAX, FUZZ, FLAT);
>> -       input_set_abs_params(tj9->input_dev, ABS_Z, -G_MAX, G_MAX, FUZZ, FLAT);
>> +       kxtj9_init_input_device(tj9, poll_dev->input);
>>
>> -       tj9->input_dev->name = "kxtj9_accel";
>> -       tj9->input_dev->id.bustype = BUS_I2C;
>> -       tj9->input_dev->dev.parent = &tj9->client->dev;
>> -
>> -       err = input_register_device(tj9->input_dev);
>> +       err = input_register_polled_device(poll_dev);
>>        if (err) {
>>                dev_err(&tj9->client->dev,
>> -                       "unable to register input polled device %s: %d\n",
>> -                       tj9->input_dev->name, err);
>> -               goto err1;
>> +                       "Unable to register polled device, err=%d\n", err);
>> +               input_free_polled_device(poll_dev);
>> +               return 0;
>>        }
>>
>>        return 0;
>> -err1:
>> -       input_free_device(tj9->input_dev);
>> -err0:
>> -       return err;
>>  }
>>
>> -/* kxtj9_delay_show: returns the currently selected poll interval (in ms) */
>> -static ssize_t kxtj9_get_poll(struct device *dev,
>> -                               struct device_attribute *attr, char *buf)
>> +static kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
>>  {
>> -       struct kxtj9_data *tj9 = i2c_get_clientdata(to_i2c_client(dev));
>> -       return sprintf(buf, "%d\n", tj9->pdata.poll_interval);
>> +       input_unregister_polled_device(tj9->poll_dev);
>> +       input_free_polled_device(tj9->poll_dev);
>>  }
>>
>> -/* kxtj9_delay_store: allows the user to select a new poll interval (in ms) */
>> -static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
>> -                                               const char *buf, size_t count)
>> +#else
>> +
>> +static inline int kxtj9_setup_polled_device(struct kxtj9_data *tj9)
>>  {
>> -       struct kxtj9_data *tj9 = i2c_get_clientdata(to_i2c_client(dev));
>> -       unsigned long interval;
>> -       int ret = kstrtoul(buf, 10, &interval);
>> -       if (ret < 0)
>> -               return ret;
>> +       return -ENOSYS;
>> +}
>>
>> -       mutex_lock(&tj9->lock);
>> -       /* set current interval to the greater of the minimum interval or
>> -       the requested interval */
>> -       tj9->pdata.poll_interval = max((int)interval, tj9->pdata.min_interval);
>> -       mutex_unlock(&tj9->lock);
>> +static inline void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
>> +{
>> +}
>> +#endif
>>
>> -       kxtj9_update_odr(tj9, tj9->pdata.poll_interval);
>> +static int __devinit kxtj9_verify(struct kxtj9_data *tj9)
>> +{
>> +       int retval;
>>
>> -       return count;
>> -}
>> -static DEVICE_ATTR(poll, S_IRUGO|S_IWUSR, kxtj9_get_poll, kxtj9_set_poll);
>> +       retval = kxtj9_device_power_on(tj9);
>> +       if (retval < 0)
>> +               return retval;
>>
>> -static struct attribute *kxtj9_attributes[] = {
>> -       &dev_attr_poll.attr,
>> -       NULL
>> -};
>> +       retval = i2c_smbus_read_byte_data(tj9->client, WHO_AM_I);
>> +       if (retval < 0) {
>> +               dev_err(&tj9->client->dev, "read err int source\n");
>> +               goto out;
>> +       }
>>
>> -static struct attribute_group kxtj9_attribute_group = {
>> -       .attrs = kxtj9_attributes
>> -};
>> +       retval = retval != 0x06 ? - EIO : 0;
>> +
>> +out:
>> +       kxtj9_device_power_off(tj9);
>> +       return retval;
>> +}
>>
>>  static int __devinit kxtj9_probe(struct i2c_client *client,
>> -                                               const struct i2c_device_id *id)
>> +                                const struct i2c_device_id *id)
>>  {
>> -       int err = -1;
>> -       struct kxtj9_data *tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
>> -       if (tj9 == NULL) {
>> -               dev_err(&client->dev,
>> -                       "failed to allocate memory for module data\n");
>> -               err = -ENOMEM;
>> -               goto err0;
>> +       const struct kxtj9_platform_data *pdata = client->dev.platform_data;
>> +       struct kxtj9_data *tj9;
>> +       int err;
>> +
>> +       if (!i2c_check_functionality(client->adapter,
>> +                               I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE_DATA)) {
>> +               dev_err(&client->dev, "client is not i2c capable\n");
>> +               return -ENXIO;
>>        }
>> -       if (client->dev.platform_data == NULL) {
>> +
>> +       if (!pdata) {
>>                dev_err(&client->dev, "platform data is NULL; exiting\n");
>> -               err = -ENODEV;
>> -               goto err0;
>> +               return -EINVAL;
>>        }
>> -       if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
>> -                                               I2C_FUNC_SMBUS_BYTE_DATA)) {
>> -               dev_err(&client->dev, "client not i2c capable\n");
>> -               err = -ENODEV;
>> -               goto err0;
>> +
>> +       tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
>> +       if (!tj9) {
>> +               dev_err(&client->dev,
>> +                       "failed to allocate memory for module data\n");
>> +               return -ENOMEM;
>>        }
>> -       mutex_init(&tj9->lock);
>> -       mutex_lock(&tj9->lock);
>> +
>>        tj9->client = client;
>> -       i2c_set_clientdata(client, tj9);
>> +       tj9->pdata = *pdata;
>>
>> -       memcpy(&tj9->pdata, client->dev.platform_data, sizeof(tj9->pdata));
>> -       if (tj9->pdata.init) {
>> -               err = tj9->pdata.init();
>> +       if (pdata->init) {
>> +               err = pdata->init();
>>                if (err < 0)
>> -                       goto err1;
>> +                       goto err_free_mem;
>>        }
>>
>> -       err = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
>> -       if (err)
>> -               goto err2;
>> +       err = kxtj9_verify(tj9);
>> +       if (err < 0) {
>> +               dev_err(&client->dev, "device not recognized\n");
>> +               goto err_pdata_exit;
>> +       }
>>
>> -       tj9->resume[RES_CTRL_REG1] = tj9->pdata.res_12bit | tj9->pdata.g_range;
>> -       tj9->resume[RES_DATA_CTRL] = tj9->pdata.data_odr_init;
>> +       tj9->ctrl_reg1 = tj9->pdata.res_12bit | tj9->pdata.g_range;
>> +       tj9->data_ctrl = tj9->pdata.data_odr_init;
>>
>>        if (client->irq) {
>> -               err = request_threaded_irq(client->irq, NULL, kxtj9_isr,
>> -                       IRQF_TRIGGER_RISING | IRQF_ONESHOT, "kxtj9-irq", tj9);
>> -               if (err < 0) {
>> -                       pr_err("%s: request irq failed: %d\n", __func__, err);
>> -                       goto err5;
>> -               }
>> -               /* if in irq mode, populate INT_CTRL_REG1 and enable DRDY */
>> -               tj9->resume[RES_INT_CTRL1] = KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
>> -               tj9->resume[RES_CTRL_REG1] |= DRDYE;
>> -       }
>> +               /* If in irq mode, populate INT_CTRL_REG1 and enable DRDY. */
>> +               tj9->int_ctrl |= KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
>> +               tj9->ctrl_reg1 |= DRDYE;
>>
>> -       err = kxtj9_input_init(tj9);
>> -       if (err < 0)
>> -               goto err3;
>> +               err = kxtj9_setup_input_device(tj9);
>> +               if (err)
>> +                       goto err_pdata_exit;
>>
>> -       err = kxtj9_device_power_on(tj9);
>> -       if (err < 0)
>> -               goto err4;
>> -       tj9->enabled = true;
>> +               err = request_threaded_irq(client->irq, NULL, kxtj9_isr,
>> +                                          IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>> +                                          "kxtj9-irq", tj9);
>> +               if (err) {
>> +                       dev_err(&client->dev, "request irq failed: %d\n", err);
>> +                       goto err_destroy_input;
>> +               }
>>
>> -       err = kxtj9_verify(tj9);
>> -       if (err < 0) {
>> -               dev_err(&client->dev, "device not recognized\n");
>> -               goto err5;
>> +       } else {
>> +               err = kxtj9_setup_polled_device(tj9);
>> +               if (err)
>> +                       goto err_pdata_exit;
>>        }
>>
>> -       mutex_unlock(&tj9->lock);
>> -
>> +       i2c_set_clientdata(client, tj9);
>>        return 0;
>>
>> -err5:
>> -       kxtj9_device_power_off(tj9);
>> -err4:
>> +err_destroy_input:
>>        input_unregister_device(tj9->input_dev);
>> -err3:
>> -       sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
>> -err2:
>> +err_pdata_exit:
>>        if (tj9->pdata.exit)
>>                tj9->pdata.exit();
>> -err1:
>> -       mutex_unlock(&tj9->lock);
>> -err0:
>> +err_free_mem:
>>        kfree(tj9);
>>        return err;
>>  }
>> @@ -539,50 +521,69 @@ static int __devexit kxtj9_remove(struct i2c_client *client)
>>  {
>>        struct kxtj9_data *tj9 = i2c_get_clientdata(client);
>>
>> -       sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
>> -       if (client->irq)
>> +       if (client->irq) {
>>                free_irq(client->irq, tj9);
>> -       input_unregister_device(tj9->input_dev);
>> -       kxtj9_device_power_off(tj9);
>> +               input_unregister_device(tj9->input_dev);
>> +       } else {
>> +               kxtj9_teardown_polled_device(tj9);
>> +       }
>> +
>>        if (tj9->pdata.exit)
>>                tj9->pdata.exit();
>> +
>>        kfree(tj9);
>>
>>        return 0;
>>  }
>>
>> -#ifdef CONFIG_PM
>> -static int kxtj9_resume(struct device *dev)
>> +#ifdef CONFIG_PM_SLEEP
>> +static int kxtj9_suspend(struct device *dev)
>>  {
>> -       return kxtj9_enable(i2c_get_clientdata(to_i2c_client(dev)));
>> +       struct i2c_client *client = to_i2c_client(dev);
>> +       struct kxtj9_data *tj9 = i2c_get_clientdata(client);
>> +       struct input_dev *input_dev = tj9->input_dev;
>> +
>> +       mutex_lock(&input_dev->mutex);
>> +
>> +       if (input_dev->users)
>> +               kxtj9_disable(tj9);
>> +
>> +       mutex_unlock(&input_dev->mutex);
>> +       return 0;
>>  }
>>
>> -static int kxtj9_suspend(struct device *dev)
>> +static int kxtj9_resume(struct device *dev)
>>  {
>> -       return kxtj9_disable(i2c_get_clientdata(to_i2c_client(dev)));
>> -}
>> +       struct i2c_client *client = to_i2c_client(dev);
>> +       struct kxtj9_data *tj9 = i2c_get_clientdata(client);
>> +       struct input_dev *input_dev = tj9->input_dev;
>> +       int retval = 0;
>>
>> -static const struct dev_pm_ops kxtj9_pm_ops = {
>> -       .suspend = kxtj9_suspend,
>> -       .resume = kxtj9_resume,
>> -};
>> +       mutex_lock(&input_dev->mutex);
>> +
>> +       if (input_dev->users)
>> +               kxtj9_enable(tj9);
>> +
>> +       mutex_unlock(&input_dev->mutex);
>> +       return retval;
>> +}
>>  #endif
>>
>> +static SIMPLE_DEV_PM_OPS(kxtj9_pm_ops, kxtj9_suspend, kxtj9_resume);
>> +
>>  static const struct i2c_device_id kxtj9_id[] = {
>> -       {NAME, 0},
>> -       {},
>> +       { NAME, 0 },
>> +       { },
>>  };
>>
>>  MODULE_DEVICE_TABLE(i2c, kxtj9_id);
>>
>>  static struct i2c_driver kxtj9_driver = {
>>        .driver = {
>> -                  .name = NAME,
>> -                  .owner = THIS_MODULE,
>> -#ifdef CONFIG_PM
>> -                  .pm = &kxtj9_pm_ops,
>> -#endif
>> -                  },
>> +               .name = NAME,
>> +               .owner = THIS_MODULE,
>> +               .pm = &kxtj9_pm_ops,
>> +       },
>>        .probe = kxtj9_probe,
>>        .remove = __devexit_p(kxtj9_remove),
>>        .id_table = kxtj9_id,
>> @@ -592,13 +593,12 @@ static int __init kxtj9_init(void)
>>  {
>>        return i2c_add_driver(&kxtj9_driver);
>>  }
>> +module_init(kxtj9_init);
>>
>>  static void __exit kxtj9_exit(void)
>>  {
>>        i2c_del_driver(&kxtj9_driver);
>>  }
>> -
>> -module_init(kxtj9_init);
>>  module_exit(kxtj9_exit);
>>
>>  MODULE_DESCRIPTION("KXTJ9 accelerometer driver");
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-06-16 19:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-26 22:40 [PATCH v4] input: Add support for Kionix KXTJ9 accelerometer Chris Hudson
2011-05-31  8:03 ` Dmitry Torokhov
2011-06-07 18:04   ` Christopher Hudson
2011-06-16 19:41     ` Christopher Hudson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).