[PATCH v5] input: Add MELFAS mms114 touchscreen driver

[PATCH v5] input: Add MELFAS mms114 touchscreen driver

[Joonyoung Shim]

This is a initial driver for new touchscreen chip mms114 of MELFAS.
It uses I2C interface and supports 10 multi touch.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
This v5 patch was updated from Henrik review mainly.

Changelog

from v1:
- Remove mutex and use one i2c_transfer() to read registers
- Use input_mt_report_pointer_emulation() and optimized input report
- Add ABS_MT_PRESSURE report
- Add core regulator control

from v2:
- Use BUG() when try to read MMS114_MODE_CONTROL
- Move i2c delay code
- Remove too much initialzation in declaration in mms114_proc_mt()
- Fix packet size check
- Use "error" instead of "ret"
- Add open and close function including regulator configuration
- Use module_i2c_driver()

from v3:
- Use check input_dev->users in PM functions

from v4:
- Modify MMS114_PACKET_NUM to 8 for latest firmware
- Define struct mms114_touch
- Some code clean

Thanks.

 drivers/input/touchscreen/Kconfig  |   12 +
 drivers/input/touchscreen/Makefile |    1 +
 drivers/input/touchscreen/mms114.c |  565 ++++++++++++++++++++++++++++++++++++
 include/linux/i2c/mms114.h         |   24 ++
 4 files changed, 602 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/touchscreen/mms114.c
 create mode 100644 include/linux/i2c/mms114.h

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 2a21419..3486795 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -340,6 +340,18 @@ config TOUCHSCREEN_MCS5000
 	  To compile this driver as a module, choose M here: the
 	  module will be called mcs5000_ts.
 
+config TOUCHSCREEN_MMS114
+	tristate "MELFAS MMS114 touchscreen"
+	depends on I2C
+	help
+	  Say Y here if you have the MELFAS MMS114 touchscreen controller
+	  chip in your system.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called mms114.
+
 config TOUCHSCREEN_MTOUCH
 	tristate "MicroTouch serial touchscreens"
 	select SERIO
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 3d5cf8c..882da14 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_TOUCHSCREEN_LPC32XX)	+= lpc32xx_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MAX11801)	+= max11801_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MC13783)	+= mc13783_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MCS5000)	+= mcs5000_ts.o
+obj-$(CONFIG_TOUCHSCREEN_MMS114)	+= mms114.o
 obj-$(CONFIG_TOUCHSCREEN_MIGOR)		+= migor_ts.o
 obj-$(CONFIG_TOUCHSCREEN_MTOUCH)	+= mtouch.o
 obj-$(CONFIG_TOUCHSCREEN_MK712)		+= mk712.o
diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
new file mode 100644
index 0000000..616f463
--- /dev/null
+++ b/drivers/input/touchscreen/mms114.c
@@ -0,0 +1,565 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim <jy0922.shim@samsung.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 Foundationr
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/i2c/mms114.h>
+#include <linux/input/mt.h>
+#include <linux/interrupt.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+/* Write only registers */
+#define MMS114_MODE_CONTROL		0x01
+#define MMS114_OPERATION_MODE_MASK	0xE
+#define MMS114_ACTIVE			(1 << 1)
+
+#define MMS114_XY_RESOLUTION_H		0x02
+#define MMS114_X_RESOLUTION		0x03
+#define MMS114_Y_RESOLUTION		0x04
+#define MMS114_CONTACT_THRESHOLD	0x05
+#define MMS114_MOVING_THRESHOLD		0x06
+
+/* Read only registers */
+#define MMS114_PACKET_SIZE		0x0F
+#define MMS114_INFOMATION		0x10
+#define MMS114_TSP_REV			0xF0
+
+/* Minimum delay time is 50us between stop and start signal of i2c */
+#define MMS114_I2C_DELAY		50
+
+/* 200ms needs after power on */
+#define MMS114_POWERON_DELAY		200
+
+/* Touchscreen absolute values */
+#define MMS114_MAX_AREA			0xff
+
+#define MMS114_MAX_TOUCH		10
+#define MMS114_PACKET_NUM		8
+
+/* Touch type */
+#define MMS114_TYPE_NONE		0
+#define MMS114_TYPE_TOUCHSCREEN		1
+#define MMS114_TYPE_TOUCHKEY		2
+
+struct mms114_data {
+	struct i2c_client	*client;
+	struct input_dev	*input_dev;
+	struct regulator	*core_reg;
+	struct regulator	*io_reg;
+	struct mutex		mutex;
+	bool			enabled;
+	const struct mms114_platform_data	*pdata;
+
+	/* Use cache data for mode control register(write only) */
+	u8			cache_mode_control;
+};
+
+struct mms114_touch {
+	u8 id:4, reserved_bit4:1, type:2, pressed:1;
+	u8 x_hi:4, y_hi:4;
+	u8 x_lo;
+	u8 y_lo;
+	u8 width;
+	u8 strength;
+	u8 reserved[2];
+} __packed;
+
+static int __mms114_read_reg(struct mms114_data *data, unsigned int reg,
+			     unsigned int len, u8 *val)
+{
+	struct i2c_client *client = data->client;
+	struct i2c_msg xfer[2];
+	u8 buf = reg & 0xff;
+	int error;
+
+	if ((reg <= MMS114_MODE_CONTROL) && (reg + len > MMS114_MODE_CONTROL)) {
+		BUG();
+		return -EINVAL;
+	}
+
+	/* Write register: use repeated start */
+	xfer[0].addr = client->addr;
+	xfer[0].flags = I2C_M_TEN | I2C_M_NOSTART;
+	xfer[0].len = 1;
+	xfer[0].buf = &buf;
+
+	/* Read data */
+	xfer[1].addr = client->addr;
+	xfer[1].flags = I2C_M_RD;
+	xfer[1].len = len;
+	xfer[1].buf = val;
+
+	error = i2c_transfer(client->adapter, xfer, 2);
+	if (error != 2) {
+		dev_err(&client->dev, "%s: i2c transfer failed (%d)\n",
+				__func__, error);
+		return error < 0 ? error : -EIO;
+	}
+	udelay(MMS114_I2C_DELAY);
+
+	return 0;
+}
+
+static int mms114_read_reg(struct mms114_data *data, unsigned int reg)
+{
+	u8 val;
+	int error;
+
+	if (reg == MMS114_MODE_CONTROL)
+		return data->cache_mode_control;
+
+	error = __mms114_read_reg(data, reg, 1, &val);
+	return error < 0 ? error : val;
+}
+
+static int mms114_write_reg(struct mms114_data *data, unsigned int reg,
+			    unsigned int val)
+{
+	struct i2c_client *client = data->client;
+	u8 buf[2];
+	int error;
+
+	buf[0] = reg & 0xff;
+	buf[1] = val & 0xff;
+
+	error = i2c_master_send(client, buf, 2);
+	if (error != 2) {
+		dev_err(&client->dev, "%s, i2c send failed (%d)\n", __func__,
+				error);
+		return error < 0 ? error : -EIO;
+	}
+	udelay(MMS114_I2C_DELAY);
+
+	if (reg == MMS114_MODE_CONTROL)
+		data->cache_mode_control = val;
+
+	return 0;
+}
+
+static void mms114_proc_mt(struct mms114_data *data, struct mms114_touch *touch)
+{
+	const struct mms114_platform_data *pdata = data->pdata;
+	struct i2c_client *client = data->client;
+	struct input_dev *input_dev = data->input_dev;
+	unsigned int id;
+	unsigned int x;
+	unsigned int y;
+
+	if (touch->id > MMS114_MAX_TOUCH) {
+		dev_err(&client->dev, "Wrong touch id (%d)\n", touch->id);
+		return;
+	}
+
+	if (touch->type != MMS114_TYPE_TOUCHSCREEN) {
+		dev_err(&client->dev, "Wrong touch type (%d)\n", touch->type);
+		return;
+	}
+
+	id = touch->id - 1;
+	x = touch->x_lo | touch->x_hi << 8;
+	y = touch->y_lo | touch->y_hi << 8;
+	if (x > pdata->x_size || y > pdata->y_size) {
+		dev_err(&client->dev, "Wrong touch coordinates (%d, %d)\n",
+				x, y);
+		return;
+	}
+
+	if (pdata->x_invert)
+		x = pdata->x_size - x;
+	if (pdata->y_invert)
+		y = pdata->y_size - y;
+
+	dev_dbg(&client->dev, "id: %d, type: %d, pressed: %d\n",
+			id, touch->type, touch->pressed);
+	dev_dbg(&client->dev, "x: %d, y: %d, width: %d, strength: %d\n",
+			x, y, touch->width, touch->strength);
+
+	input_mt_slot(input_dev, id);
+	input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, touch->pressed);
+
+	if (touch->pressed) {
+		input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, touch->width);
+		input_report_abs(input_dev, ABS_MT_POSITION_X, x);
+		input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
+		input_report_abs(input_dev, ABS_MT_PRESSURE, touch->strength);
+	}
+}
+
+static irqreturn_t mms114_interrupt(int irq, void *dev_id)
+{
+	struct mms114_data *data = dev_id;
+	struct mms114_touch touch[MMS114_MAX_TOUCH];
+	int packet_size;
+	int touch_size;
+	int index;
+	int error;
+
+	if (!data->enabled)
+		goto out;
+
+	packet_size = mms114_read_reg(data, MMS114_PACKET_SIZE);
+	if (packet_size <= 0)
+		goto out;
+
+	touch_size = packet_size / MMS114_PACKET_NUM;
+
+	error = __mms114_read_reg(data, MMS114_INFOMATION, packet_size,
+			(u8 *)touch);
+	if (error < 0)
+		goto out;
+
+	for (index = 0; index < touch_size; index++)
+		mms114_proc_mt(data, touch + index);
+
+	input_mt_report_pointer_emulation(data->input_dev, true);
+	input_sync(data->input_dev);
+
+out:
+	return IRQ_HANDLED;
+}
+
+static int mms114_set_active(struct mms114_data *data, bool active)
+{
+	int val;
+
+	val = mms114_read_reg(data, MMS114_MODE_CONTROL);
+	if (val < 0)
+		return val;
+
+	val &= ~MMS114_OPERATION_MODE_MASK;
+
+	/* If active is false, sleep mode */
+	if (active)
+		val |= MMS114_ACTIVE;
+
+	return mms114_write_reg(data, MMS114_MODE_CONTROL, val);
+}
+
+static int mms114_get_version(struct mms114_data *data)
+{
+	struct device *dev = &data->client->dev;
+	u8 buf[6];
+	int error;
+
+	error = __mms114_read_reg(data, MMS114_TSP_REV, 6, buf);
+	if (error < 0)
+		return error;
+
+	dev_info(dev, "TSP Rev: 0x%x, HW Rev: 0x%x, Firmware Ver: 0x%x\n",
+			buf[0], buf[1], buf[3]);
+
+	return 0;
+}
+
+static int mms114_setup_regs(struct mms114_data *data)
+{
+	const struct mms114_platform_data *pdata = data->pdata;
+	int val;
+	int error;
+
+	error = mms114_set_active(data, true);
+	if (error < 0)
+		return error;
+
+	val = (pdata->x_size >> 8) & 0xf;
+	val |= ((pdata->y_size >> 8) & 0xf) << 4;
+	error = mms114_write_reg(data, MMS114_XY_RESOLUTION_H, val);
+	if (error < 0)
+		return error;
+
+	val = pdata->x_size & 0xff;
+	error = mms114_write_reg(data, MMS114_X_RESOLUTION, val);
+	if (error < 0)
+		return error;
+
+	val = pdata->y_size & 0xff;
+	error = mms114_write_reg(data, MMS114_Y_RESOLUTION, val);
+	if (error < 0)
+		return error;
+
+	if (pdata->contact_threshold) {
+		error = mms114_write_reg(data, MMS114_CONTACT_THRESHOLD,
+				pdata->contact_threshold);
+		if (error < 0)
+			return error;
+	}
+
+	if (pdata->moving_threshold) {
+		error = mms114_write_reg(data, MMS114_MOVING_THRESHOLD,
+				pdata->moving_threshold);
+		if (error < 0)
+			return error;
+	}
+
+	return 0;
+}
+
+static int mms114_start(struct mms114_data *data)
+{
+	int error;
+
+	mutex_lock(&data->mutex);
+	if (data->enabled)
+		goto out;
+
+	if (data->core_reg)
+		regulator_enable(data->core_reg);
+	if (data->io_reg)
+		regulator_enable(data->io_reg);
+	mdelay(MMS114_POWERON_DELAY);
+
+	error = mms114_setup_regs(data);
+	if (error < 0) {
+		mutex_unlock(&data->mutex);
+		return error;
+	}
+
+	data->enabled = true;
+
+out:
+	mutex_unlock(&data->mutex);
+	return 0;
+}
+
+static void mms114_stop(struct mms114_data *data)
+{
+	mutex_lock(&data->mutex);
+	if (!data->enabled)
+		goto out;
+
+	if (data->io_reg)
+		regulator_disable(data->io_reg);
+	if (data->core_reg)
+		regulator_disable(data->core_reg);
+
+	data->enabled = false;
+
+out:
+	mutex_unlock(&data->mutex);
+}
+
+static int mms114_input_open(struct input_dev *dev)
+{
+	struct mms114_data *data = input_get_drvdata(dev);
+
+	return mms114_start(data);
+}
+
+static void mms114_input_close(struct input_dev *dev)
+{
+	struct mms114_data *data = input_get_drvdata(dev);
+
+	mms114_stop(data);
+}
+
+static int __devinit mms114_probe(struct i2c_client *client,
+				  const struct i2c_device_id *id)
+{
+	struct mms114_data *data;
+	struct input_dev *input_dev;
+	int error;
+
+	if (!client->dev.platform_data) {
+		dev_err(&client->dev, "Need platform data\n");
+		return -EINVAL;
+	}
+
+	if (!i2c_check_functionality(client->adapter,
+				I2C_FUNC_PROTOCOL_MANGLING)) {
+		dev_err(&client->dev,
+			"Need i2c bus that supports protocol mangling\n");
+		return -ENODEV;
+	}
+
+	data = kzalloc(sizeof(struct mms114_data), GFP_KERNEL);
+	input_dev = input_allocate_device();
+	if (!data || !input_dev) {
+		dev_err(&client->dev, "Failed to allocate memory\n");
+		error = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	mutex_init(&data->mutex);
+
+	data->client = client;
+	data->input_dev = input_dev;
+	data->pdata = client->dev.platform_data;
+
+	input_dev->name = "MELPAS MMS114 Touchscreen";
+	input_dev->id.bustype = BUS_I2C;
+	input_dev->dev.parent = &client->dev;
+	input_dev->open = mms114_input_open;
+	input_dev->close = mms114_input_close;
+
+	__set_bit(EV_ABS, input_dev->evbit);
+	__set_bit(EV_KEY, input_dev->evbit);
+	__set_bit(BTN_TOUCH, input_dev->keybit);
+	input_set_abs_params(input_dev, ABS_X, 0, data->pdata->x_size, 0, 0);
+	input_set_abs_params(input_dev, ABS_Y, 0, data->pdata->y_size, 0, 0);
+
+	/* For multi touch */
+	input_mt_init_slots(input_dev, MMS114_MAX_TOUCH);
+	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
+			     0, MMS114_MAX_AREA, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
+			     0, data->pdata->x_size, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
+			     0, data->pdata->y_size, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
+
+	input_set_drvdata(input_dev, data);
+	i2c_set_clientdata(client, data);
+
+	data->core_reg = regulator_get(&client->dev, "avdd");
+	if (IS_ERR(data->core_reg)) {
+		error = PTR_ERR(data->core_reg);
+		dev_err(&client->dev, "Unable to get the Core regulator (%d)\n",
+				error);
+		goto err_free_mem;
+	}
+
+	data->io_reg = regulator_get(&client->dev, "vdd");
+	if (IS_ERR(data->io_reg)) {
+		error = PTR_ERR(data->io_reg);
+		dev_err(&client->dev, "Unable to get the IO regulator (%d)\n",
+				error);
+		goto err_core_reg;
+	}
+
+	error = mms114_start(data);
+	if (error < 0)
+		goto err_io_reg;
+
+	error = mms114_get_version(data);
+	if (error < 0)
+		goto err_io_reg;
+
+	mms114_stop(data);
+
+	error = request_threaded_irq(client->irq, NULL, mms114_interrupt,
+			IRQF_TRIGGER_FALLING, "mms114", data);
+	if (error < 0) {
+		dev_err(&client->dev, "Failed to register interrupt\n");
+		goto err_io_reg;
+	}
+
+	error = input_register_device(data->input_dev);
+	if (error < 0)
+		goto err_free_irq;
+
+	if (data->pdata->cfg_pin)
+		data->pdata->cfg_pin(true);
+
+	return 0;
+
+err_free_irq:
+	free_irq(client->irq, data);
+err_io_reg:
+	regulator_put(data->io_reg);
+err_core_reg:
+	regulator_put(data->core_reg);
+err_free_mem:
+	input_free_device(input_dev);
+	kfree(data);
+	return error;
+}
+
+static int __devexit mms114_remove(struct i2c_client *client)
+{
+	struct mms114_data *data = i2c_get_clientdata(client);
+
+	free_irq(client->irq, data);
+	regulator_put(data->io_reg);
+	regulator_put(data->core_reg);
+	input_unregister_device(data->input_dev);
+	kfree(data);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mms114_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mms114_data *data = i2c_get_clientdata(client);
+	struct input_dev *input_dev = data->input_dev;
+	int id;
+
+	disable_irq(client->irq);
+
+	/* Release all touch */
+	for (id = 0; id < MMS114_MAX_TOUCH; id++) {
+		input_mt_slot(input_dev, id);
+		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, false);
+	}
+
+	input_mt_report_pointer_emulation(input_dev, true);
+	input_sync(input_dev);
+
+	mms114_stop(data);
+
+	if (data->pdata->cfg_pin)
+		data->pdata->cfg_pin(false);
+
+	return 0;
+}
+
+static int mms114_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct mms114_data *data = i2c_get_clientdata(client);
+	struct input_dev *input_dev = data->input_dev;
+	int error;
+
+	if (data->pdata->cfg_pin)
+		data->pdata->cfg_pin(true);
+
+	mutex_lock(&input_dev->mutex);
+	if (input_dev->users) {
+		error = mms114_start(data);
+		if (error < 0) {
+			mutex_unlock(&input_dev->mutex);
+			return error;
+		}
+	}
+	mutex_unlock(&input_dev->mutex);
+
+	enable_irq(client->irq);
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(mms114_pm_ops, mms114_suspend, mms114_resume);
+
+static const struct i2c_device_id mms114_id[] = {
+	{ "mms114", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mms114_id);
+
+static struct i2c_driver mms114_driver = {
+	.driver = {
+		.name	= "mms114",
+		.owner	= THIS_MODULE,
+		.pm	= &mms114_pm_ops,
+	},
+	.probe		= mms114_probe,
+	.remove		= __devexit_p(mms114_remove),
+	.id_table	= mms114_id,
+};
+
+module_i2c_driver(mms114_driver);
+
+/* Module information */
+MODULE_AUTHOR("Joonyoung Shim <jy0922.shim@samsung.com>");
+MODULE_DESCRIPTION("MELFAS mms114 Touchscreen driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c/mms114.h b/include/linux/i2c/mms114.h
new file mode 100644
index 0000000..5722ebf
--- /dev/null
+++ b/include/linux/i2c/mms114.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2012 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim <jy0922.shim@samsung.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 Foundationr
+ */
+
+#ifndef __LINUX_MMS114_H
+#define __LINUX_MMS114_H
+
+struct mms114_platform_data {
+	unsigned int x_size;
+	unsigned int y_size;
+	unsigned int contact_threshold;
+	unsigned int moving_threshold;
+	bool x_invert;
+	bool y_invert;
+
+	void (*cfg_pin)(bool);
+};
+
+#endif	/* __LINUX_MMS114_H */
-- 
1.7.5.4

Re: [PATCH v5] input: Add MELFAS mms114 touchscreen driver

[Henrik Rydberg]

Hi Joonyoung,

> This is a initial driver for new touchscreen chip mms114 of MELFAS.
> It uses I2C interface and supports 10 multi touch.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> This v5 patch was updated from Henrik review mainly.

Looking neat now, thanks for making the changes. One comment and one question:

> +#define MMS114_PACKET_NUM		8

I would have dropped this in favor of sizeof(touch[0]) instead.

> +static irqreturn_t mms114_interrupt(int irq, void *dev_id)
> +{
> +	struct mms114_data *data = dev_id;
> +	struct mms114_touch touch[MMS114_MAX_TOUCH];
> +	int packet_size;
> +	int touch_size;
> +	int index;
> +	int error;
> +
> +	if (!data->enabled)
> +		goto out;
> +
> +	packet_size = mms114_read_reg(data, MMS114_PACKET_SIZE);
> +	if (packet_size <= 0)
> +		goto out;
> +
> +	touch_size = packet_size / MMS114_PACKET_NUM;

Since MMS114_PACKET_NUM changed, this calculation is no longer the
same. Will you still get the correct number of touches for all
firmware versions?

> +
> +	error = __mms114_read_reg(data, MMS114_INFOMATION, packet_size,
> +			(u8 *)touch);
> +	if (error < 0)
> +		goto out;
> +
> +	for (index = 0; index < touch_size; index++)
> +		mms114_proc_mt(data, touch + index);
> +
> +	input_mt_report_pointer_emulation(data->input_dev, true);
> +	input_sync(data->input_dev);
> +
> +out:
> +	return IRQ_HANDLED;
> +}

Thanks,
Henrik

Re: [PATCH v5] input: Add MELFAS mms114 touchscreen driver

[Joonyoung Shim]

On 05/24/2012 04:21 PM, Henrik Rydberg wrote:
> Hi Joonyoung,
>
>> This is a initial driver for new touchscreen chip mms114 of MELFAS.
>> It uses I2C interface and supports 10 multi touch.
>>
>> Signed-off-by: Joonyoung Shim<jy0922.shim@samsung.com>
>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>> ---
>> This v5 patch was updated from Henrik review mainly.
> Looking neat now, thanks for making the changes. One comment and one question:
>
>> +#define MMS114_PACKET_NUM		8
> I would have dropped this in favor of sizeof(touch[0]) instead.

OK.

>> +static irqreturn_t mms114_interrupt(int irq, void *dev_id)
>> +{
>> +	struct mms114_data *data = dev_id;
>> +	struct mms114_touch touch[MMS114_MAX_TOUCH];
>> +	int packet_size;
>> +	int touch_size;
>> +	int index;
>> +	int error;
>> +
>> +	if (!data->enabled)
>> +		goto out;
>> +
>> +	packet_size = mms114_read_reg(data, MMS114_PACKET_SIZE);
>> +	if (packet_size<= 0)
>> +		goto out;
>> +
>> +	touch_size = packet_size / MMS114_PACKET_NUM;
> Since MMS114_PACKET_NUM changed, this calculation is no longer the
> same. Will you still get the correct number of touches for all
> firmware versions?

No, it is working only from firmware version 0x59 and i am using
firmware version 0x66. I don't know Melfas's firmware policy. Melfas
releases sometimes inconsistent updated firmware internally, so driver
needs to check firmware version later but currently this driver can't
get the correct number of touches on lower firmware version than 0x59.

>> +
>> +	error = __mms114_read_reg(data, MMS114_INFOMATION, packet_size,
>> +			(u8 *)touch);
>> +	if (error<  0)
>> +		goto out;
>> +
>> +	for (index = 0; index<  touch_size; index++)
>> +		mms114_proc_mt(data, touch + index);
>> +
>> +	input_mt_report_pointer_emulation(data->input_dev, true);
>> +	input_sync(data->input_dev);
>> +
>> +out:
>> +	return IRQ_HANDLED;
>> +}
> Thanks,
> Henrik
> --
> 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: Add MELFAS mms114 touchscreen driver

[Dmitry Torokhov]

Hi Joonyoung,

On Thu, May 24, 2012 at 03:37:47PM +0900, Joonyoung Shim wrote:
> +
> +static int mms114_start(struct mms114_data *data)
> +{
> +	int error;
> +
> +	mutex_lock(&data->mutex);
> +	if (data->enabled)
> +		goto out;

This seems too complicated. You already take input_dev->mutex in
suspend/resume and open/close are called with this mutex held so you do
not need yet another mutex here.

Same goes for data->enabled - you can go by the fact that number of
users != 0.

It also looks like you want to bring enabling/disabling IRQ into
start/stop and probably cfg_pin as well.

Thanks.

-- 
Dmitry