[PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

Subsystems: input (keyboard, mouse, joystick, touchscreen) drivers, the rest

STALE5498d

8 messages, 3 authors, 2011-07-12 · open the first message on its own page

[PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

From: <hidden>
Date: 2011-06-28 19:24:08

From: Chris Hudson <redacted>

- Added Dmitry's changes
- Now using input_polled_dev interface when in polling mode
- IRQ mode provides a separate sysfs node to change the hardware data rate

Signed-off-by: Chris Hudson <redacted>
---
 drivers/input/misc/Kconfig  |   17 ++
 drivers/input/misc/Makefile |    1 +
 drivers/input/misc/kxtj9.c  |  646 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/input/kxtj9.h |   76 +++++
 4 files changed, 740 insertions(+), 0 deletions(-)
 create mode 100755 drivers/input/misc/kxtj9.c
 create mode 100755 include/linux/input/kxtj9.h
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 45dc6aa..cd6a806 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -478,4 +478,21 @@ 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
+       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.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 38efb2c..bfe1057 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
index 0000000..7faf155
--- /dev/null
+++ b/drivers/input/misc/kxtj9.c
@@ -0,0 +1,646 @@
+/*
+ * 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>
+#include <linux/input-polldev.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 input_dev *input_dev;
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+	struct input_polled_dev *poll_dev;
+#else
+	struct mutex lock;
+#endif
+	unsigned int last_poll_interval;
+	u8 shift;
+	u8 ctrl_reg1;
+	u8 data_ctrl;
+	u8 int_ctrl;
+};
+
+static int kxtj9_i2c_read(struct kxtj9_data *tj9, u8 addr, u8 *data, int len)
+{
+	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,
+		},
+	};
+
+	return i2c_transfer(tj9->client->adapter, msgs, 2);
+}
+
+static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
+{
+	s16 acc_data[3]; /* Data bytes from hardware xL, xH, yL, yH, zL, zH */
+	s16 x, y, z;
+	int err;
+
+	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);
+}
+
+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->ctrl_reg1 &= 0xe7;
+	tj9->ctrl_reg1 |= new_g_range;
+
+	return 0;
+}
+
+static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
+{
+	int err;
+	int i;
+
+	/* Use the lowest ODR that can support the requested poll interval */
+	for (i = 0; i < ARRAY_SIZE(kxtj9_odr_table); i++) {
+		tj9->data_ctrl = kxtj9_odr_table[i].mask;
+		if (poll_interval < kxtj9_odr_table[i].cutoff)
+			break;
+	}
+
+	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;
+
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, tj9->ctrl_reg1);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+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;
+}
+
+static void kxtj9_device_power_off(struct kxtj9_data *tj9)
+{
+	int err;
+
+	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();
+}
+
+static int kxtj9_enable(struct kxtj9_data *tj9)
+{
+	int err;
+
+	err = kxtj9_device_power_on(tj9);
+	if (err < 0)
+		return err;
+
+	/* ensure that PC1 is cleared before updating control registers */
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, 0);
+	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;
+	}
+
+	err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
+	if (err < 0)
+		return err;
+
+	/* turn on outputs */
+	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_update_odr(tj9, tj9->last_poll_interval);
+	if (err < 0)
+		return err;
+
+	/* clear initial interrupt if in irq mode */
+	if (tj9->client->irq) {
+		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;
+		}
+	}
+
+	return 0;
+
+fail:
+	kxtj9_device_power_off(tj9);
+	return err;
+}
+
+static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
+						struct input_dev *input_dev)
+{
+	__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;
+}
+
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+static void kxtj9_poll(struct input_polled_dev *dev)
+{
+	struct kxtj9_data *tj9 = dev->private;
+	unsigned int poll_interval = dev->poll_interval;
+
+	kxtj9_report_acceleration_data(tj9);
+
+	if (poll_interval != tj9->last_poll_interval) {
+		kxtj9_update_odr(tj9, poll_interval);
+		tj9->last_poll_interval = poll_interval;
+	}
+}
+
+static void kxtj9_polled_input_open(struct input_polled_dev *dev)
+{
+	struct kxtj9_data *tj9 = dev->private;
+
+	kxtj9_enable(tj9);
+}
+
+static void kxtj9_polled_input_close(struct input_polled_dev *dev)
+{
+	struct kxtj9_data *tj9 = dev->private;
+
+	kxtj9_device_power_off(tj9);
+}
+
+static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
+{
+	int err;
+	struct input_polled_dev *poll_dev;
+	poll_dev = input_allocate_polled_device();
+
+	if (!poll_dev) {
+		dev_err(&tj9->client->dev,
+			"Failed to allocate polled device\n");
+		return -ENOMEM;
+	}
+
+	tj9->poll_dev = poll_dev;
+	tj9->input_dev = poll_dev->input;
+
+	poll_dev->private = tj9;
+	poll_dev->poll = kxtj9_poll;
+	poll_dev->open = kxtj9_polled_input_open;
+	poll_dev->close = kxtj9_polled_input_close;
+
+	kxtj9_init_input_device(tj9, poll_dev->input);
+
+	err = input_register_polled_device(poll_dev);
+	if (err) {
+		dev_err(&tj9->client->dev,
+			"Unable to register polled device, err=%d\n", err);
+		input_free_polled_device(poll_dev);
+		return 0;
+	}
+
+	return 0;
+}
+
+static void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
+{
+	input_unregister_polled_device(tj9->poll_dev);
+	input_free_polled_device(tj9->poll_dev);
+}
+
+#else	/* IRQ Mode is enabled */
+static int kxtj9_input_open(struct input_dev *input)
+{
+	struct kxtj9_data *tj9 = input_get_drvdata(input);
+
+	return kxtj9_enable(tj9);
+}
+
+static void kxtj9_input_close(struct input_dev *dev)
+{
+	struct kxtj9_data *tj9 = input_get_drvdata(dev);
+
+	kxtj9_device_power_off(tj9);
+}
+
+static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
+{
+	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);
+	}
+
+	return 0;
+}
+
+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 inline int kxtj9_setup_polled_device(struct kxtj9_data *tj9)
+{
+	return -ENOSYS;
+}
+
+static inline void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
+{
+}
+
+/* kxtj9_get_poll: 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->last_poll_interval);
+}
+
+/* kxtj9_set_poll: 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->last_poll_interval = max((int)interval, tj9->pdata.min_interval);
+	mutex_unlock(&tj9->lock);
+
+	kxtj9_update_odr(tj9, tj9->last_poll_interval);
+
+	return count;
+}
+/* When IRQ mode is selected, we need to provide an interface to allow the user
+ * to change the output data rate of the part.  For consistency, we are using
+ * the set_poll method, which accepts a poll interval in milliseconds, and then
+ * calls update_odr() while passing this value as an argument.  In IRQ mode, the
+ * data outputs will not be read AT the requested poll interval, rather, the
+ * lowest ODR that can support the requested interval.  The client application
+ * will be responsible for retrieving data from the input node at the desired
+ * interval.
+ */
+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
+};
+#endif
+
+static int __devinit kxtj9_verify(struct kxtj9_data *tj9)
+{
+	int retval;
+	retval = kxtj9_device_power_on(tj9);
+	if (retval < 0)
+		return retval;
+	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;
+	}
+	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 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 (!pdata) {
+		dev_err(&client->dev, "platform data is NULL; exiting\n");
+		return -EINVAL;
+	}
+	tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
+	if (!tj9) {
+		dev_err(&client->dev,
+			"failed to allocate memory for module data\n");
+		return -ENOMEM;
+	}
+
+	tj9->client = client;
+	tj9->pdata = *pdata;
+
+	if (pdata->init) {
+		err = pdata->init();
+		if (err < 0)
+			goto err_free_mem;
+	}
+
+	err = kxtj9_verify(tj9);
+	if (err < 0) {
+		dev_err(&client->dev, "device not recognized\n");
+		goto err_pdata_exit;
+	}
+
+	tj9->ctrl_reg1 = tj9->pdata.res_12bit | tj9->pdata.g_range;
+	tj9->data_ctrl = tj9->pdata.data_odr_init;
+
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+	err = kxtj9_setup_polled_device(tj9);
+	if (err)
+		goto err_pdata_exit;
+#else
+	/* 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_setup_input_device(tj9);
+	if (err)
+		goto err_pdata_exit;
+
+	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 = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
+	if (err) {
+		dev_err(&client->dev, "sysfs create failed: %d\n", err);
+		goto err_destroy_input;
+	}
+#endif
+	i2c_set_clientdata(client, tj9);
+	return 0;
+
+#ifndef CONFIG_KXTJ9_POLLED_MODE
+err_destroy_input:
+	input_unregister_device(tj9->input_dev);
+#endif
+err_pdata_exit:
+	if (tj9->pdata.exit)
+		tj9->pdata.exit();
+err_free_mem:
+	kfree(tj9);
+	return err;
+}
+
+static int __devexit kxtj9_remove(struct i2c_client *client)
+{
+	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+	kxtj9_teardown_polled_device(tj9);
+#else
+	sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
+	free_irq(client->irq, tj9);
+	input_unregister_device(tj9->input_dev);
+#endif
+	if (tj9->pdata.exit)
+		tj9->pdata.exit();
+
+	kfree(tj9);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int kxtj9_suspend(struct device *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_device_power_off(tj9);
+
+	mutex_unlock(&input_dev->mutex);
+	return 0;
+}
+
+static int kxtj9_resume(struct device *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;
+
+	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 },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(i2c, kxtj9_id);
+
+static struct i2c_driver kxtj9_driver = {
+	.driver = {
+		  .name = NAME,
+		  .owner = THIS_MODULE,
+		  .pm = &kxtj9_pm_ops,
+	},
+	.probe = kxtj9_probe,
+	.remove = __devexit_p(kxtj9_remove),
+	.id_table = kxtj9_id,
+};
+
+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_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
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

Re: [PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

From: Jonathan Cameron <hidden>
Date: 2011-06-29 12:31:38

On 06/28/11 20:12, chris.hudson.comp.eng@gmail.com wrote:
From: Chris Hudson <redacted>

- Added Dmitry's changes
- Now using input_polled_dev interface when in polling mode
- IRQ mode provides a separate sysfs node to change the hardware data rate
Hi Chris,

Few minor comments inline.
quoted hunk
Signed-off-by: Chris Hudson <redacted>
---
 drivers/input/misc/Kconfig  |   17 ++
 drivers/input/misc/Makefile |    1 +
 drivers/input/misc/kxtj9.c  |  646 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/input/kxtj9.h |   76 +++++
 4 files changed, 740 insertions(+), 0 deletions(-)
 create mode 100755 drivers/input/misc/kxtj9.c
 create mode 100755 include/linux/input/kxtj9.h
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 45dc6aa..cd6a806 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -478,4 +478,21 @@ 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
+       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.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 38efb2c..bfe1057 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
index 0000000..7faf155
--- /dev/null
+++ b/drivers/input/misc/kxtj9.c
@@ -0,0 +1,646 @@
+/*
+ * 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>
+#include <linux/input-polldev.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 input_dev *input_dev;
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+	struct input_polled_dev *poll_dev;
+#else
+	struct mutex lock;
+#endif
+	unsigned int last_poll_interval;
+	u8 shift;
+	u8 ctrl_reg1;
+	u8 data_ctrl;
+	u8 int_ctrl;
+};
+
+static int kxtj9_i2c_read(struct kxtj9_data *tj9, u8 addr, u8 *data, int len)
+{
+	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,
+		},
+	};
+
+	return i2c_transfer(tj9->client->adapter, msgs, 2);
Could just use i2c_smbus_read_i2c_block_data(tj9->client, addr, len, data);
+}
+
+static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
+{
+	s16 acc_data[3]; /* Data bytes from hardware xL, xH, yL, yH, zL, zH */
+	s16 x, y, z;
+	int err;
+
+	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);
+}
+
This next function does look like it should take an enum instead
of new_g_range then use a single table to look up both the shift and the
value for new_g_range.  Something like.
enum {
     range_2g, range_4g, range_8g
} kxtj9_range;
/* could use a struct for this to make what is what more explicit */
static const u8 rangevals[3][2] = { {KXTJ9_G_2G, SHIFT_ADJ_2G},
   		     		  {KXTJ9_G_4G, SHIFT_ADJ_4G},
		       		  {KXTJ9_G_8G, SHIFT_ADJ_8G}};
static int kxtj9_update_g_range(struct kxtj9_data *tj9, enum_kxtj9_range r)
{
	tj9->shift = rangevals[r][1];
	tj9->ctrl_reg1 &= 0xe7;	
	tj9->ctrl_reg1 |= rangevals[r][0];
}

Actually you could just encode values in the array rather than using defines
at all.  Also, could make the pdata element the enum value.
+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->ctrl_reg1 &= 0xe7;
+	tj9->ctrl_reg1 |= new_g_range;
+
+	return 0;
+}
+
+static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
+{
+	int err;
+	int i;
+
+	/* Use the lowest ODR that can support the requested poll interval */
+	for (i = 0; i < ARRAY_SIZE(kxtj9_odr_table); i++) {
+		tj9->data_ctrl = kxtj9_odr_table[i].mask;
+		if (poll_interval < kxtj9_odr_table[i].cutoff)
+			break;
+	}
+
+	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;
+
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, tj9->ctrl_reg1);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+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;
+}
Slight semantics change to pdata.power_on() returning 0 for success and you'll
have cleaner:

static int kxtj9_device_power_on(struct kxtj9_data *tj9)
{
	if (tj9->pdata.power_on)
		return tj9->pdata.power_on();

	return 0;
}
+
+static void kxtj9_device_power_off(struct kxtj9_data *tj9)
+{
+	int err;
+
+	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();
+}
+
+static int kxtj9_enable(struct kxtj9_data *tj9)
+{
+	int err;
+
+	err = kxtj9_device_power_on(tj9);
+	if (err < 0)
+		return err;
+
+	/* ensure that PC1 is cleared before updating control registers */
+	err = i2c_smbus_write_byte_data(tj9->client, CTRL_REG1, 0);
+	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;
+	}
+
+	err = kxtj9_update_g_range(tj9, tj9->pdata.g_range);
+	if (err < 0)
+		return err;
+
+	/* turn on outputs */
+	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_update_odr(tj9, tj9->last_poll_interval);
+	if (err < 0)
+		return err;
+
+	/* clear initial interrupt if in irq mode */
+	if (tj9->client->irq) {
+		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;
+		}
+	}
+
+	return 0;
+
+fail:
+	kxtj9_device_power_off(tj9);
+	return err;
+}
+
+static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
+						struct input_dev *input_dev)
+{
+	__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";
Is the _accel bit a convention? 
+	input_dev->id.bustype = BUS_I2C;
+	input_dev->dev.parent = &tj9->client->dev;
+}
+
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+static void kxtj9_poll(struct input_polled_dev *dev)
+{
+	struct kxtj9_data *tj9 = dev->private;
+	unsigned int poll_interval = dev->poll_interval;
+
+	kxtj9_report_acceleration_data(tj9);
+
+	if (poll_interval != tj9->last_poll_interval) {
+		kxtj9_update_odr(tj9, poll_interval);
+		tj9->last_poll_interval = poll_interval;
+	}
+}
+
+static void kxtj9_polled_input_open(struct input_polled_dev *dev)
+{
+	struct kxtj9_data *tj9 = dev->private;
+
+	kxtj9_enable(tj9);
Given ktxj9_enable knows what it's getting, squash into

kxtj9_enable(dev->private);
+}
+
+static void kxtj9_polled_input_close(struct input_polled_dev *dev)
+{
+	struct kxtj9_data *tj9 = dev->private;
+
+	kxtj9_device_power_off(tj9);
same as previous.
+}
+
+static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
+{
+	int err;
+	struct input_polled_dev *poll_dev;
+	poll_dev = input_allocate_polled_device();
+
+	if (!poll_dev) {
+		dev_err(&tj9->client->dev,
+			"Failed to allocate polled device\n");
+		return -ENOMEM;
+	}
+
+	tj9->poll_dev = poll_dev;
+	tj9->input_dev = poll_dev->input;
+
+	poll_dev->private = tj9;
+	poll_dev->poll = kxtj9_poll;
+	poll_dev->open = kxtj9_polled_input_open;
+	poll_dev->close = kxtj9_polled_input_close;
+
+	kxtj9_init_input_device(tj9, poll_dev->input);
+
+	err = input_register_polled_device(poll_dev);
+	if (err) {
+		dev_err(&tj9->client->dev,
+			"Unable to register polled device, err=%d\n", err);
+		input_free_polled_device(poll_dev);
+		return 0;
+	}
+
+	return 0;
+}
+
+static void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
+{
+	input_unregister_polled_device(tj9->poll_dev);
+	input_free_polled_device(tj9->poll_dev);
+}
+
+#else	/* IRQ Mode is enabled */
+static int kxtj9_input_open(struct input_dev *input)
+{
+	struct kxtj9_data *tj9 = input_get_drvdata(input);
+
+	return kxtj9_enable(tj9);
return kxtj9_enable(input_get_drvdata(input));
+}
+
+static void kxtj9_input_close(struct input_dev *dev)
+{
+	struct kxtj9_data *tj9 = input_get_drvdata(dev);
+
+	kxtj9_device_power_off(tj9);
snap
+}
+
+static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
+{
+	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);
+	}
+
+	return 0;
+}
+
+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 inline int kxtj9_setup_polled_device(struct kxtj9_data *tj9)
+{
+	return -ENOSYS;
+}
+
+static inline void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
+{
+}
+
+/* kxtj9_get_poll: 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->last_poll_interval);
+}
+
+/* kxtj9_set_poll: 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->last_poll_interval = max((int)interval, tj9->pdata.min_interval);
+	mutex_unlock(&tj9->lock);
+
+	kxtj9_update_odr(tj9, tj9->last_poll_interval);
+
+	return count;
+}
New line here please.
quoted hunk
+/* When IRQ mode is selected, we need to provide an interface to allow the user
+ * to change the output data rate of the part.  For consistency, we are using
+ * the set_poll method, which accepts a poll interval in milliseconds, and then
+ * calls update_odr() while passing this value as an argument.  In IRQ mode, the
+ * data outputs will not be read AT the requested poll interval, rather, the
+ * lowest ODR that can support the requested interval.  The client application
+ * will be responsible for retrieving data from the input node at the desired
+ * interval.
+ */
+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
+};
+#endif
+
+static int __devinit kxtj9_verify(struct kxtj9_data *tj9)
+{
+	int retval;
+	retval = kxtj9_device_power_on(tj9);
+	if (retval < 0)
+		return retval;
+	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;
+	}
+	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 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 (!pdata) {
+		dev_err(&client->dev, "platform data is NULL; exiting\n");
+		return -EINVAL;
+	}
+	tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
+	if (!tj9) {
+		dev_err(&client->dev,
+			"failed to allocate memory for module data\n");
+		return -ENOMEM;
+	}
+
+	tj9->client = client;
+	tj9->pdata = *pdata;
+
+	if (pdata->init) {
+		err = pdata->init();
+		if (err < 0)
+			goto err_free_mem;
+	}
+
+	err = kxtj9_verify(tj9);
+	if (err < 0) {
+		dev_err(&client->dev, "device not recognized\n");
+		goto err_pdata_exit;
+	}
+
+	tj9->ctrl_reg1 = tj9->pdata.res_12bit | tj9->pdata.g_range;
+	tj9->data_ctrl = tj9->pdata.data_odr_init;
+
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+	err = kxtj9_setup_polled_device(tj9);
+	if (err)
+		goto err_pdata_exit;
+#else
+	/* 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_setup_input_device(tj9);
+	if (err)
+		goto err_pdata_exit;
+
+	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 = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
+	if (err) {
+		dev_err(&client->dev, "sysfs create failed: %d\n", err);
+		goto err_destroy_input;
+	}
+#endif
+	i2c_set_clientdata(client, tj9);
+	return 0;
+
+#ifndef CONFIG_KXTJ9_POLLED_MODE
+err_destroy_input:
+	input_unregister_device(tj9->input_dev);
+#endif
+err_pdata_exit:
+	if (tj9->pdata.exit)
+		tj9->pdata.exit();
+err_free_mem:
+	kfree(tj9);
+	return err;
+}
+
+static int __devexit kxtj9_remove(struct i2c_client *client)
+{
+	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+
+#ifdef CONFIG_KXTJ9_POLLED_MODE
+	kxtj9_teardown_polled_device(tj9);
+#else
+	sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
+	free_irq(client->irq, tj9);
+	input_unregister_device(tj9->input_dev);
+#endif
+	if (tj9->pdata.exit)
+		tj9->pdata.exit();
+
+	kfree(tj9);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int kxtj9_suspend(struct device *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_device_power_off(tj9);
+
+	mutex_unlock(&input_dev->mutex);
+	return 0;
+}
+
+static int kxtj9_resume(struct device *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;
+
+	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 },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(i2c, kxtj9_id);
+
+static struct i2c_driver kxtj9_driver = {
+	.driver = {
+		  .name = NAME,
+		  .owner = THIS_MODULE,
+		  .pm = &kxtj9_pm_ops,
+	},
+	.probe = kxtj9_probe,
+	.remove = __devexit_p(kxtj9_remove),
+	.id_table = kxtj9_id,
+};
+
+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_exit(kxtj9_exit);
+
+MODULE_DESCRIPTION("KXTJ9 accelerometer driver");
+MODULE_AUTHOR("Chris Hudson [off-list ref]");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/input/kxtj9.h b/include/linux/input/kxtj9.h
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__ */
+

Re: [PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2011-07-05 04:36:33

On Wed, Jun 29, 2011 at 01:39:41PM +0100, Jonathan Cameron wrote:
On 06/28/11 20:12, chris.hudson.comp.eng@gmail.com wrote:
quoted
+static int kxtj9_input_open(struct input_dev *input)
+{
+	struct kxtj9_data *tj9 = input_get_drvdata(input);
+
+	return kxtj9_enable(tj9);
return kxtj9_enable(input_get_drvdata(input));
Actually I prefer limiting use of void pointers. If kxtj9_enable()
changes to take something other than "struct kxtj9_data *" then original
will issue a warning while the proposed form will happily accept it.

Thanks.

-- 
Dmitry

Re: [PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2011-07-05 04:36:34

Hi Chris,

On Tue, Jun 28, 2011 at 03:12:28PM -0400, chris.hudson.comp.eng@gmail.com wrote:
From: Chris Hudson <redacted>

- Added Dmitry's changes
- Now using input_polled_dev interface when in polling mode
- IRQ mode provides a separate sysfs node to change the hardware data rate
I am not happy with the fact that this implementation forces users to
select polled or interrupt-driven mode at compile time. The patch I
proposed had polled mode support optional and would automatically select
IRQ mode for clients that have interrupt assigned and try to activate
polled mode if interrupt is not available.
+
+/* kxtj9_set_poll: 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->last_poll_interval = max((int)interval, tj9->pdata.min_interval);
+	mutex_unlock(&tj9->lock);
This lock does not make sense. You are protecting update of last_poll_interval
field and this update can not be partial (i.e. only LSB or MSB is
written) on all our arches, but you do not protect kxtj9_update_odr()
which alters chip state and does need protection.

I tried bringing forward my changes from the older patch into yours,
please let me know if the patch below works on top of yours.

Thanks.

-- 
Dmitry


Input: ktxj9 - misc fixes

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

Signed-off-by: Dmitry Torokhov <redacted>
---

 drivers/input/misc/Kconfig  |   34 ++--
 drivers/input/misc/kxtj9.c  |  355 +++++++++++++++++++++++--------------------
 include/linux/input/kxtj9.h |   18 +-
 3 files changed, 220 insertions(+), 187 deletions(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 41628d2..0134428 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -230,6 +230,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
+	  Say Y here to enable support for the Kionix KXTJ9 digital tri-axis
+	  accelerometer.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called kxtj9.
+
+config INPUT_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
@@ -499,21 +516,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
-       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.
-
 endif
diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
index 7faf155..31eae6a 100644
--- a/drivers/input/misc/kxtj9.c
+++ b/drivers/input/misc/kxtj9.c
@@ -75,10 +75,8 @@ struct kxtj9_data {
 	struct i2c_client *client;
 	struct kxtj9_platform_data pdata;
 	struct input_dev *input_dev;
-#ifdef CONFIG_KXTJ9_POLLED_MODE
+#ifdef CONFIG_INPUT_KXTJ9_POLLED_MODE
 	struct input_polled_dev *poll_dev;
-#else
-	struct mutex lock;
 #endif
 	unsigned int last_poll_interval;
 	u8 shift;
@@ -117,16 +115,32 @@ static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
 	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;
+	x = le16_to_cpu(acc_data[tj9->pdata.axis_map_x]) >> tj9->shift;
+	y = le16_to_cpu(acc_data[tj9->pdata.axis_map_y]) >> tj9->shift;
+	z = le16_to_cpu(acc_data[tj9->pdata.axis_map_z]) >> 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_report_abs(tj9->input_dev, ABS_Z, tj9->pdata.negate_z ? -z : z);
 	input_sync(tj9->input_dev);
 }
 
+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)
 {
 	switch (new_g_range) {
@@ -152,7 +166,7 @@ static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
 	return 0;
 }
 
-static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
+static int kxtj9_update_odr(struct kxtj9_data *tj9, unsigned int poll_interval)
 {
 	int err;
 	int i;
@@ -245,7 +259,7 @@ static int kxtj9_enable(struct kxtj9_data *tj9)
 		err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
 		if (err < 0) {
 			dev_err(&tj9->client->dev,
-					"error clearing interrupt: %d\n", err);
+				"error clearing interrupt: %d\n", err);
 			goto fail;
 		}
 	}
@@ -257,8 +271,27 @@ fail:
 	return err;
 }
 
+static void kxtj9_disable(struct kxtj9_data *tj9)
+{
+	kxtj9_device_power_off(tj9);
+}
+
+static int kxtj9_input_open(struct input_dev *input)
+{
+	struct kxtj9_data *tj9 = input_get_drvdata(input);
+
+	return kxtj9_enable(tj9);
+}
+
+static void kxtj9_input_close(struct input_dev *dev)
+{
+	struct kxtj9_data *tj9 = input_get_drvdata(dev);
+
+	kxtj9_disable(tj9);
+}
+
 static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
-						struct input_dev *input_dev)
+					      struct input_dev *input_dev)
 {
 	__set_bit(EV_ABS, input_dev->evbit);
 	input_set_abs_params(input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
@@ -270,7 +303,104 @@ static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
 	input_dev->dev.parent = &tj9->client->dev;
 }
 
-#ifdef CONFIG_KXTJ9_POLLED_MODE
+static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
+{
+	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);
+		return err;
+	}
+
+	return 0;
+}
+
+/*
+ * When IRQ mode is selected, we need to provide an interface to allow the user
+ * to change the output data rate of the part.  For consistency, we are using
+ * the set_poll method, which accepts a poll interval in milliseconds, and then
+ * calls update_odr() while passing this value as an argument.  In IRQ mode, the
+ * data outputs will not be read AT the requested poll interval, rather, the
+ * lowest ODR that can support the requested interval.  The client application
+ * will be responsible for retrieving data from the input node at the desired
+ * interval.
+ */
+
+/* Returns currently selected poll interval (in ms) */
+static ssize_t kxtj9_get_poll(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+
+	return sprintf(buf, "%d\n", tj9->last_poll_interval);
+}
+
+/* Allow users 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 i2c_client *client = to_i2c_client(dev);
+	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+	struct input_dev *input_dev = tj9->input_dev;
+	unsigned int interval;
+	int error;
+
+	error = kstrtouint(buf, 10, &interval);
+	if (error < 0)
+		return error;
+
+	/* Lock the device to prevent races with open/close (and itself) */
+	mutex_unlock(&input_dev->mutex);
+
+	disable_irq(client->irq);
+
+	/*
+	 * Set current interval to the greater of the minimum interval or
+	 * the requested interval
+	 */
+	tj9->last_poll_interval = max(interval, tj9->pdata.min_interval);
+
+	kxtj9_update_odr(tj9, tj9->last_poll_interval);
+
+	enable_irq(client->irq);
+	mutex_unlock(&input_dev->mutex);
+
+	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
+};
+
+
+#ifdef CONFIG_INPUT_KXTJ9_POLLED_MODE
 static void kxtj9_poll(struct input_polled_dev *dev)
 {
 	struct kxtj9_data *tj9 = dev->private;
@@ -295,7 +425,7 @@ static void kxtj9_polled_input_close(struct input_polled_dev *dev)
 {
 	struct kxtj9_data *tj9 = dev->private;
 
-	kxtj9_device_power_off(tj9);
+	kxtj9_disable(tj9);
 }
 
 static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
@@ -325,7 +455,7 @@ static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
 		dev_err(&tj9->client->dev,
 			"Unable to register polled device, err=%d\n", err);
 		input_free_polled_device(poll_dev);
-		return 0;
+		return err;
 	}
 
 	return 0;
@@ -337,66 +467,7 @@ static void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
 	input_free_polled_device(tj9->poll_dev);
 }
 
-#else	/* IRQ Mode is enabled */
-static int kxtj9_input_open(struct input_dev *input)
-{
-	struct kxtj9_data *tj9 = input_get_drvdata(input);
-
-	return kxtj9_enable(tj9);
-}
-
-static void kxtj9_input_close(struct input_dev *dev)
-{
-	struct kxtj9_data *tj9 = input_get_drvdata(dev);
-
-	kxtj9_device_power_off(tj9);
-}
-
-static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
-{
-	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);
-	}
-
-	return 0;
-}
-
-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;
-}
+#else
 
 static inline int kxtj9_setup_polled_device(struct kxtj9_data *tj9)
 {
@@ -407,67 +478,22 @@ static inline void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
 {
 }
 
-/* kxtj9_get_poll: 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->last_poll_interval);
-}
-
-/* kxtj9_set_poll: 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->last_poll_interval = max((int)interval, tj9->pdata.min_interval);
-	mutex_unlock(&tj9->lock);
-
-	kxtj9_update_odr(tj9, tj9->last_poll_interval);
-
-	return count;
-}
-/* When IRQ mode is selected, we need to provide an interface to allow the user
- * to change the output data rate of the part.  For consistency, we are using
- * the set_poll method, which accepts a poll interval in milliseconds, and then
- * calls update_odr() while passing this value as an argument.  In IRQ mode, the
- * data outputs will not be read AT the requested poll interval, rather, the
- * lowest ODR that can support the requested interval.  The client application
- * will be responsible for retrieving data from the input node at the desired
- * interval.
- */
-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
-};
 #endif
 
 static int __devinit kxtj9_verify(struct kxtj9_data *tj9)
 {
 	int retval;
+
 	retval = kxtj9_device_power_on(tj9);
 	if (retval < 0)
 		return retval;
+
 	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;
 	}
+
 	retval = retval != 0x06 ? -EIO : 0;
 
 out:
@@ -476,7 +502,7 @@ out:
 }
 
 static int __devinit kxtj9_probe(struct i2c_client *client,
-				const struct i2c_device_id *id)
+				 const struct i2c_device_id *id)
 {
 	const struct kxtj9_platform_data *pdata = client->dev.platform_data;
 	struct kxtj9_data *tj9;
@@ -487,10 +513,12 @@ static int __devinit kxtj9_probe(struct i2c_client *client,
 		dev_err(&client->dev, "client is not i2c capable\n");
 		return -ENXIO;
 	}
+
 	if (!pdata) {
 		dev_err(&client->dev, "platform data is NULL; exiting\n");
 		return -EINVAL;
 	}
+
 	tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
 	if (!tj9) {
 		dev_err(&client->dev,
@@ -513,42 +541,46 @@ static int __devinit kxtj9_probe(struct i2c_client *client,
 		goto err_pdata_exit;
 	}
 
+	i2c_set_clientdata(client, tj9);
+
 	tj9->ctrl_reg1 = tj9->pdata.res_12bit | tj9->pdata.g_range;
 	tj9->data_ctrl = tj9->pdata.data_odr_init;
 
-#ifdef CONFIG_KXTJ9_POLLED_MODE
-	err = kxtj9_setup_polled_device(tj9);
-	if (err)
-		goto err_pdata_exit;
-#else
-	/* If in irq mode, populate INT_CTRL_REG1 and enable DRDY. */
-	tj9->int_ctrl |= KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
-	tj9->ctrl_reg1 |= DRDYE;
+	if (client->irq) {
+		/* 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_setup_input_device(tj9);
+		if (err)
+			goto err_pdata_exit;
+
+		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_setup_input_device(tj9);
-	if (err)
-		goto err_pdata_exit;
+		err = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
+		if (err) {
+			dev_err(&client->dev, "sysfs create failed: %d\n", err);
+			goto err_free_irq;
+		}
 
-	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 = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
-	if (err) {
-		dev_err(&client->dev, "sysfs create failed: %d\n", err);
-		goto err_destroy_input;
+	} else {
+		err = kxtj9_setup_polled_device(tj9);
+		if (err)
+			goto err_pdata_exit;
 	}
-#endif
-	i2c_set_clientdata(client, tj9);
+
 	return 0;
 
-#ifndef CONFIG_KXTJ9_POLLED_MODE
+err_free_irq:
+	free_irq(client->irq, tj9);
 err_destroy_input:
 	input_unregister_device(tj9->input_dev);
-#endif
 err_pdata_exit:
 	if (tj9->pdata.exit)
 		tj9->pdata.exit();
@@ -561,13 +593,14 @@ static int __devexit kxtj9_remove(struct i2c_client *client)
 {
 	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
 
-#ifdef CONFIG_KXTJ9_POLLED_MODE
-	kxtj9_teardown_polled_device(tj9);
-#else
-	sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
-	free_irq(client->irq, tj9);
-	input_unregister_device(tj9->input_dev);
-#endif
+	if (client->irq) {
+		sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
+		free_irq(client->irq, tj9);
+		input_unregister_device(tj9->input_dev);
+	} else {
+		kxtj9_teardown_polled_device(tj9);
+	}
+
 	if (tj9->pdata.exit)
 		tj9->pdata.exit();
 
@@ -620,13 +653,13 @@ MODULE_DEVICE_TABLE(i2c, kxtj9_id);
 
 static struct i2c_driver kxtj9_driver = {
 	.driver = {
-		  .name = NAME,
-		  .owner = THIS_MODULE,
-		  .pm = &kxtj9_pm_ops,
+		.name	= NAME,
+		.owner	= THIS_MODULE,
+		.pm	= &kxtj9_pm_ops,
 	},
-	.probe = kxtj9_probe,
-	.remove = __devexit_p(kxtj9_remove),
-	.id_table = kxtj9_id,
+	.probe		= kxtj9_probe,
+	.remove		= __devexit_p(kxtj9_remove),
+	.id_table	= kxtj9_id,
 };
 
 static int __init kxtj9_init(void)
diff --git a/include/linux/input/kxtj9.h b/include/linux/input/kxtj9.h
index cc5928b..0b546bd 100644
--- a/include/linux/input/kxtj9.h
+++ b/include/linux/input/kxtj9.h
@@ -27,19 +27,21 @@
 #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) */
+	unsigned 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 */
+	/*
+	 * 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 */
+	/*
+	 * Each axis can be negated to account for sensor orientation within
+	 * the host device.
+	 */
 	bool negate_x;
 	bool negate_y;
 	bool negate_z;
@@ -70,7 +72,5 @@ struct kxtj9_platform_data {
 	int (*power_on)(void);
 	int (*power_off)(void);
 };
-#endif /* __KERNEL__ */
-
 #endif  /* __KXTJ9_H__ */
 

Re: [PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

From: Christopher Hudson <hidden>
Date: 2011-07-05 18:08:02

On Mon, Jul 4, 2011 at 12:26 PM, Dmitry Torokhov
[off-list ref] wrote:
Hi Chris,

On Tue, Jun 28, 2011 at 03:12:28PM -0400, chris.hudson.comp.eng@gmail.com wrote:
quoted
From: Chris Hudson <redacted>

- Added Dmitry's changes
- Now using input_polled_dev interface when in polling mode
- IRQ mode provides a separate sysfs node to change the hardware data rate
I am not happy with the fact that this implementation forces users to
select polled or interrupt-driven mode at compile time. The patch I
proposed had polled mode support optional and would automatically select
IRQ mode for clients that have interrupt assigned and try to activate
polled mode if interrupt is not available.
quoted
+
+/* kxtj9_set_poll: 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->last_poll_interval = max((int)interval, tj9->pdata.min_interval);
+     mutex_unlock(&tj9->lock);
This lock does not make sense. You are protecting update of last_poll_interval
field and this update can not be partial (i.e. only LSB or MSB is
written) on all our arches, but you do not protect kxtj9_update_odr()
which alters chip state and does need protection.

I tried bringing forward my changes from the older patch into yours,
please let me know if the patch below works on top of yours.
Hi Dmitry,
Thanks for all your hard work, and yes it works fine.  There were a
couple of changes proposed by Jonathan that I had already merged into
my version though; should I submit these in a separate patch or resend
the merged version?
Thank you,
Chris
quoted hunk
Thanks.

--
Dmitry


Input: ktxj9 - misc fixes

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

Signed-off-by: Dmitry Torokhov <redacted>
---

 drivers/input/misc/Kconfig  |   34 ++--
 drivers/input/misc/kxtj9.c  |  355 +++++++++++++++++++++++--------------------
 include/linux/input/kxtj9.h |   18 +-
 3 files changed, 220 insertions(+), 187 deletions(-)

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 41628d2..0134428 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -230,6 +230,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
+         Say Y here to enable support for the Kionix KXTJ9 digital tri-axis
+         accelerometer.
+
+         To compile this driver as a module, choose M here: the module will
+         be called kxtj9.
+
+config INPUT_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
@@ -499,21 +516,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
-       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.
-
 endif
diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
index 7faf155..31eae6a 100644
--- a/drivers/input/misc/kxtj9.c
+++ b/drivers/input/misc/kxtj9.c
@@ -75,10 +75,8 @@ struct kxtj9_data {
       struct i2c_client *client;
       struct kxtj9_platform_data pdata;
       struct input_dev *input_dev;
-#ifdef CONFIG_KXTJ9_POLLED_MODE
+#ifdef CONFIG_INPUT_KXTJ9_POLLED_MODE
       struct input_polled_dev *poll_dev;
-#else
-       struct mutex lock;
 #endif
       unsigned int last_poll_interval;
       u8 shift;
@@ -117,16 +115,32 @@ static void kxtj9_report_acceleration_data(struct kxtj9_data *tj9)
       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;
+       x = le16_to_cpu(acc_data[tj9->pdata.axis_map_x]) >> tj9->shift;
+       y = le16_to_cpu(acc_data[tj9->pdata.axis_map_y]) >> tj9->shift;
+       z = le16_to_cpu(acc_data[tj9->pdata.axis_map_z]) >> 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_report_abs(tj9->input_dev, ABS_Z, tj9->pdata.negate_z ? -z : z);
       input_sync(tj9->input_dev);
 }

+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)
 {
       switch (new_g_range) {
@@ -152,7 +166,7 @@ static int kxtj9_update_g_range(struct kxtj9_data *tj9, u8 new_g_range)
       return 0;
 }

-static int kxtj9_update_odr(struct kxtj9_data *tj9, int poll_interval)
+static int kxtj9_update_odr(struct kxtj9_data *tj9, unsigned int poll_interval)
 {
       int err;
       int i;
@@ -245,7 +259,7 @@ static int kxtj9_enable(struct kxtj9_data *tj9)
               err = i2c_smbus_read_byte_data(tj9->client, INT_REL);
               if (err < 0) {
                       dev_err(&tj9->client->dev,
-                                       "error clearing interrupt: %d\n", err);
+                               "error clearing interrupt: %d\n", err);
                       goto fail;
               }
       }
@@ -257,8 +271,27 @@ fail:
       return err;
 }

+static void kxtj9_disable(struct kxtj9_data *tj9)
+{
+       kxtj9_device_power_off(tj9);
+}
+
+static int kxtj9_input_open(struct input_dev *input)
+{
+       struct kxtj9_data *tj9 = input_get_drvdata(input);
+
+       return kxtj9_enable(tj9);
+}
+
+static void kxtj9_input_close(struct input_dev *dev)
+{
+       struct kxtj9_data *tj9 = input_get_drvdata(dev);
+
+       kxtj9_disable(tj9);
+}
+
 static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
-                                               struct input_dev *input_dev)
+                                             struct input_dev *input_dev)
 {
       __set_bit(EV_ABS, input_dev->evbit);
       input_set_abs_params(input_dev, ABS_X, -G_MAX, G_MAX, FUZZ, FLAT);
@@ -270,7 +303,104 @@ static void __devinit kxtj9_init_input_device(struct kxtj9_data *tj9,
       input_dev->dev.parent = &tj9->client->dev;
 }

-#ifdef CONFIG_KXTJ9_POLLED_MODE
+static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
+{
+       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);
+               return err;
+       }
+
+       return 0;
+}
+
+/*
+ * When IRQ mode is selected, we need to provide an interface to allow the user
+ * to change the output data rate of the part.  For consistency, we are using
+ * the set_poll method, which accepts a poll interval in milliseconds, and then
+ * calls update_odr() while passing this value as an argument.  In IRQ mode, the
+ * data outputs will not be read AT the requested poll interval, rather, the
+ * lowest ODR that can support the requested interval.  The client application
+ * will be responsible for retrieving data from the input node at the desired
+ * interval.
+ */
+
+/* Returns currently selected poll interval (in ms) */
+static ssize_t kxtj9_get_poll(struct device *dev,
+                               struct device_attribute *attr, char *buf)
+{
+       struct i2c_client *client = to_i2c_client(dev);
+       struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+
+       return sprintf(buf, "%d\n", tj9->last_poll_interval);
+}
+
+/* Allow users 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 i2c_client *client = to_i2c_client(dev);
+       struct kxtj9_data *tj9 = i2c_get_clientdata(client);
+       struct input_dev *input_dev = tj9->input_dev;
+       unsigned int interval;
+       int error;
+
+       error = kstrtouint(buf, 10, &interval);
+       if (error < 0)
+               return error;
+
+       /* Lock the device to prevent races with open/close (and itself) */
+       mutex_unlock(&input_dev->mutex);
+
+       disable_irq(client->irq);
+
+       /*
+        * Set current interval to the greater of the minimum interval or
+        * the requested interval
+        */
+       tj9->last_poll_interval = max(interval, tj9->pdata.min_interval);
+
+       kxtj9_update_odr(tj9, tj9->last_poll_interval);
+
+       enable_irq(client->irq);
+       mutex_unlock(&input_dev->mutex);
+
+       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
+};
+
+
+#ifdef CONFIG_INPUT_KXTJ9_POLLED_MODE
 static void kxtj9_poll(struct input_polled_dev *dev)
 {
       struct kxtj9_data *tj9 = dev->private;
@@ -295,7 +425,7 @@ static void kxtj9_polled_input_close(struct input_polled_dev *dev)
 {
       struct kxtj9_data *tj9 = dev->private;

-       kxtj9_device_power_off(tj9);
+       kxtj9_disable(tj9);
 }

 static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
@@ -325,7 +455,7 @@ static int __devinit kxtj9_setup_polled_device(struct kxtj9_data *tj9)
               dev_err(&tj9->client->dev,
                       "Unable to register polled device, err=%d\n", err);
               input_free_polled_device(poll_dev);
-               return 0;
+               return err;
       }

       return 0;
@@ -337,66 +467,7 @@ static void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
       input_free_polled_device(tj9->poll_dev);
 }

-#else  /* IRQ Mode is enabled */
-static int kxtj9_input_open(struct input_dev *input)
-{
-       struct kxtj9_data *tj9 = input_get_drvdata(input);
-
-       return kxtj9_enable(tj9);
-}
-
-static void kxtj9_input_close(struct input_dev *dev)
-{
-       struct kxtj9_data *tj9 = input_get_drvdata(dev);
-
-       kxtj9_device_power_off(tj9);
-}
-
-static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
-{
-       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);
-       }
-
-       return 0;
-}
-
-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;
-}
+#else

 static inline int kxtj9_setup_polled_device(struct kxtj9_data *tj9)
 {
@@ -407,67 +478,22 @@ static inline void kxtj9_teardown_polled_device(struct kxtj9_data *tj9)
 {
 }

-/* kxtj9_get_poll: 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->last_poll_interval);
-}
-
-/* kxtj9_set_poll: 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->last_poll_interval = max((int)interval, tj9->pdata.min_interval);
-       mutex_unlock(&tj9->lock);
-
-       kxtj9_update_odr(tj9, tj9->last_poll_interval);
-
-       return count;
-}
-/* When IRQ mode is selected, we need to provide an interface to allow the user
- * to change the output data rate of the part.  For consistency, we are using
- * the set_poll method, which accepts a poll interval in milliseconds, and then
- * calls update_odr() while passing this value as an argument.  In IRQ mode, the
- * data outputs will not be read AT the requested poll interval, rather, the
- * lowest ODR that can support the requested interval.  The client application
- * will be responsible for retrieving data from the input node at the desired
- * interval.
- */
-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
-};
 #endif

 static int __devinit kxtj9_verify(struct kxtj9_data *tj9)
 {
       int retval;
+
       retval = kxtj9_device_power_on(tj9);
       if (retval < 0)
               return retval;
+
       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;
       }
+
       retval = retval != 0x06 ? -EIO : 0;

 out:
@@ -476,7 +502,7 @@ out:
 }

 static int __devinit kxtj9_probe(struct i2c_client *client,
-                               const struct i2c_device_id *id)
+                                const struct i2c_device_id *id)
 {
       const struct kxtj9_platform_data *pdata = client->dev.platform_data;
       struct kxtj9_data *tj9;
@@ -487,10 +513,12 @@ static int __devinit kxtj9_probe(struct i2c_client *client,
               dev_err(&client->dev, "client is not i2c capable\n");
               return -ENXIO;
       }
+
       if (!pdata) {
               dev_err(&client->dev, "platform data is NULL; exiting\n");
               return -EINVAL;
       }
+
       tj9 = kzalloc(sizeof(*tj9), GFP_KERNEL);
       if (!tj9) {
               dev_err(&client->dev,
@@ -513,42 +541,46 @@ static int __devinit kxtj9_probe(struct i2c_client *client,
               goto err_pdata_exit;
       }

+       i2c_set_clientdata(client, tj9);
+
       tj9->ctrl_reg1 = tj9->pdata.res_12bit | tj9->pdata.g_range;
       tj9->data_ctrl = tj9->pdata.data_odr_init;

-#ifdef CONFIG_KXTJ9_POLLED_MODE
-       err = kxtj9_setup_polled_device(tj9);
-       if (err)
-               goto err_pdata_exit;
-#else
-       /* If in irq mode, populate INT_CTRL_REG1 and enable DRDY. */
-       tj9->int_ctrl |= KXTJ9_IEN | KXTJ9_IEA | KXTJ9_IEL;
-       tj9->ctrl_reg1 |= DRDYE;
+       if (client->irq) {
+               /* 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_setup_input_device(tj9);
+               if (err)
+                       goto err_pdata_exit;
+
+               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_setup_input_device(tj9);
-       if (err)
-               goto err_pdata_exit;
+               err = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
+               if (err) {
+                       dev_err(&client->dev, "sysfs create failed: %d\n", err);
+                       goto err_free_irq;
+               }

-       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 = sysfs_create_group(&client->dev.kobj, &kxtj9_attribute_group);
-       if (err) {
-               dev_err(&client->dev, "sysfs create failed: %d\n", err);
-               goto err_destroy_input;
+       } else {
+               err = kxtj9_setup_polled_device(tj9);
+               if (err)
+                       goto err_pdata_exit;
       }
-#endif
-       i2c_set_clientdata(client, tj9);
+
       return 0;

-#ifndef CONFIG_KXTJ9_POLLED_MODE
+err_free_irq:
+       free_irq(client->irq, tj9);
 err_destroy_input:
       input_unregister_device(tj9->input_dev);
-#endif
 err_pdata_exit:
       if (tj9->pdata.exit)
               tj9->pdata.exit();
@@ -561,13 +593,14 @@ static int __devexit kxtj9_remove(struct i2c_client *client)
 {
       struct kxtj9_data *tj9 = i2c_get_clientdata(client);

-#ifdef CONFIG_KXTJ9_POLLED_MODE
-       kxtj9_teardown_polled_device(tj9);
-#else
-       sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
-       free_irq(client->irq, tj9);
-       input_unregister_device(tj9->input_dev);
-#endif
+       if (client->irq) {
+               sysfs_remove_group(&client->dev.kobj, &kxtj9_attribute_group);
+               free_irq(client->irq, tj9);
+               input_unregister_device(tj9->input_dev);
+       } else {
+               kxtj9_teardown_polled_device(tj9);
+       }
+
       if (tj9->pdata.exit)
               tj9->pdata.exit();
@@ -620,13 +653,13 @@ MODULE_DEVICE_TABLE(i2c, kxtj9_id);
 static struct i2c_driver kxtj9_driver = {
       .driver = {
-                 .name = NAME,
-                 .owner = THIS_MODULE,
-                 .pm = &kxtj9_pm_ops,
+               .name   = NAME,
+               .owner  = THIS_MODULE,
+               .pm     = &kxtj9_pm_ops,
       },
-       .probe = kxtj9_probe,
-       .remove = __devexit_p(kxtj9_remove),
-       .id_table = kxtj9_id,
+       .probe          = kxtj9_probe,
+       .remove         = __devexit_p(kxtj9_remove),
+       .id_table       = kxtj9_id,
 };

 static int __init kxtj9_init(void)
diff --git a/include/linux/input/kxtj9.h b/include/linux/input/kxtj9.h
index cc5928b..0b546bd 100644
--- a/include/linux/input/kxtj9.h
+++ b/include/linux/input/kxtj9.h
@@ -27,19 +27,21 @@
 #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) */
+       unsigned 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 */
+       /*
+        * 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 */
+       /*
+        * Each axis can be negated to account for sensor orientation within
+        * the host device.
+        */
       bool negate_x;
       bool negate_y;
       bool negate_z;
@@ -70,7 +72,5 @@ struct kxtj9_platform_data {
       int (*power_on)(void);
       int (*power_off)(void);
 };
-#endif /* __KERNEL__ */
-
 #endif  /* __KXTJ9_H__ */
--
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

Re: [PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2011-07-05 18:39:23

On Tue, Jul 05, 2011 at 02:08:01PM -0400, Christopher Hudson wrote:
On Mon, Jul 4, 2011 at 12:26 PM, Dmitry Torokhov
[off-list ref] wrote:
quoted
Hi Chris,

On Tue, Jun 28, 2011 at 03:12:28PM -0400, chris.hudson.comp.eng@gmail.com wrote:
quoted
From: Chris Hudson <redacted>

- Added Dmitry's changes
- Now using input_polled_dev interface when in polling mode
- IRQ mode provides a separate sysfs node to change the hardware data rate
I am not happy with the fact that this implementation forces users to
select polled or interrupt-driven mode at compile time. The patch I
proposed had polled mode support optional and would automatically select
IRQ mode for clients that have interrupt assigned and try to activate
polled mode if interrupt is not available.
quoted
+
+/* kxtj9_set_poll: 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->last_poll_interval = max((int)interval, tj9->pdata.min_interval);
+     mutex_unlock(&tj9->lock);
This lock does not make sense. You are protecting update of last_poll_interval
field and this update can not be partial (i.e. only LSB or MSB is
written) on all our arches, but you do not protect kxtj9_update_odr()
which alters chip state and does need protection.

I tried bringing forward my changes from the older patch into yours,
please let me know if the patch below works on top of yours.
Hi Dmitry,
Thanks for all your hard work, and yes it works fine.  There were a
couple of changes proposed by Jonathan that I had already merged into
my version though; should I submit these in a separate patch or resend
the merged version?
Cris,

Could you please send the changes proposed by Jonathan as a separate
patch relative to the v5 you posted earlier? Then I can fold v5, mine
and your new patch together and queue the driver for 3.1.

Thanks.

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

Re: [PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

From: Christopher Hudson <hidden>
Date: 2011-07-12 21:52:21

Hi Dmitry,

I found a small bug in this driver when using the code as the basis
for testing another piece of hardware...I'm not sure how this got
through before:

+static int __devinit kxtj9_setup_input_device(struct kxtj9_data *tj9)
+{
+       struct input_dev *input_dev;
+       int err;
+
+       input_dev = input_allocate_device();
+       if (!tj9->input_dev) {
Should be:

if (!input_dev) {

if (!tj9->input_dev) always returns true in this context, thereby
always following the error path; you can see that nothing is assigned
to tj9->input_dev until after this check.

+               dev_err(&tj9->client->dev, "input device allocate failed\n");
+               return -ENOMEM;
+       }
+
+       tj9->input_dev = input_dev;

Should I submit a small patch against your queued version to get this fixed?

On Tue, Jul 5, 2011 at 2:39 PM, Dmitry Torokhov
[off-list ref] wrote:
On Tue, Jul 05, 2011 at 02:08:01PM -0400, Christopher Hudson wrote:
quoted
On Mon, Jul 4, 2011 at 12:26 PM, Dmitry Torokhov
[off-list ref] wrote:
quoted
Hi Chris,

On Tue, Jun 28, 2011 at 03:12:28PM -0400, chris.hudson.comp.eng@gmail.com wrote:
quoted
From: Chris Hudson <redacted>

- Added Dmitry's changes
- Now using input_polled_dev interface when in polling mode
- IRQ mode provides a separate sysfs node to change the hardware data rate
I am not happy with the fact that this implementation forces users to
select polled or interrupt-driven mode at compile time. The patch I
proposed had polled mode support optional and would automatically select
IRQ mode for clients that have interrupt assigned and try to activate
polled mode if interrupt is not available.
quoted
+
+/* kxtj9_set_poll: 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->last_poll_interval = max((int)interval, tj9->pdata.min_interval);
+     mutex_unlock(&tj9->lock);
This lock does not make sense. You are protecting update of last_poll_interval
field and this update can not be partial (i.e. only LSB or MSB is
written) on all our arches, but you do not protect kxtj9_update_odr()
which alters chip state and does need protection.

I tried bringing forward my changes from the older patch into yours,
please let me know if the patch below works on top of yours.
Hi Dmitry,
Thanks for all your hard work, and yes it works fine.  There were a
couple of changes proposed by Jonathan that I had already merged into
my version though; should I submit these in a separate patch or resend
the merged version?
Cris,

Could you please send the changes proposed by Jonathan as a separate
patch relative to the v5 you posted earlier? Then I can fold v5, mine
and your new patch together and queue the driver for 3.1.

Thanks.

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

Re: [PATCH v5] input: Initial support for Kionix KXTJ9 accelerometer

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2011-07-12 22:08:16

On Tuesday, July 12, 2011 02:52:20 PM Christopher Hudson wrote:
Should I submit a small patch against your queued version to get this
fixed?
That'd be great since you could verify that it actually runs
on the hardware.

Thanks.

-- 
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help