Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH 01/02] input: serio: use DEVICE_ATTR_RO()
From: Dmitry Torokhov @ 2013-10-22 15:51 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-input
In-Reply-To: <20131022061219.GA15412@kroah.com>

On Tue, Oct 22, 2013 at 07:12:19AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Oct 21, 2013 at 09:25:32AM -0700, Dmitry Torokhov wrote:
> > Hi Greg,
> > 
> > On Mon, Oct 07, 2013 at 06:08:23PM -0700, Greg Kroah-Hartman wrote:
> > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > 
> > > Convert the serio sysfs fiels to use the DEVICE_ATTR_RO() macros to make
> > > it easier to audit the correct sysfs file permission usage.
> > 
> > Sorry for the delay. Frankly I am not a fan of DEVICE_ATTR macros as it
> > forces particular naming of the attribute structures and methods and
> > forces us to lose the module prefix in names. I prefer all functions and
> > file-scope variables have module prefix because in the absence of
> > language-supported namespaces this is the one thing that insulates
> > drivers from changes in shared headers.
> 
> No sysfs read/write functions should ever be exported out of the local
> file "scope", so why is this an issue?  I have run into it being a
> problem for some files, where people have device/bus/class sysfs files
> all in the same .c file, but that usage is very limited.

It is not about exporting read/write functions, it is about newly
defined data from common headers stomping on the local module
definitions.

> 
> I'm trying to eventually convert the whole kernel to use the
> DEVICE_ATTR_??() macros to make it so that we can audit the sysfs file
> permissions easier, by making them impossible to get wrong.  We have had
> problems in the past where device files had incorrect permissions and
> unprivilidged users could do things they shouldn't have.  Using these
> macros _should_ prevent almost all of that from even being possible.

Sounds more like task for sparse to ensure we have sane permission bits.

> 
> Now odds are, I'll never be able to remove DEVICE_ATTR() from the tree,
> as there are some strange files that use crazy permissions and we have
> to live with that, but for all of the "simple" files, like are done
> here, I'd really like to use them, especially as it provides a
> kernel-wide uniformity to the naming of attribute function calls, which
> is a huge benifit as well.
> 
> Note, I did apply this patch to my tree already, and it's queued up for
> 3.13-rc1.  If you really object to it, I'll revert it, and fix up the
> 02/02 patch in this series to not need it, but I'd really like you to
> reconsider your objection.

Nah, that is fine.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.
From: Dmitry Torokhov @ 2013-10-22 22:35 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Denis Carikli, linux-input, Sascha Hauer, linux-arm-kernel,
	Eric Bénard, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, devicetree
In-Reply-To: <20131022114947.30dc9c07@ipc1.ka-ro>

On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote:
> Hi,
> 
> > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> > >  
> > > +	if (ts->of)
> > > +		return tsc2007_get_pendown_state_dt(ts);
> > > +
> > >  	if (!ts->get_pendown_state)
> > >  		return true;
> > 
> > Instead of special casing "if (ts->of)" all over the place why don't you
> > set up the device structure as:
> > 
> > 	if (<configuring_tsc2007_form_dt>)
> > 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> > 
> > and be done with it?
> >
> I also thought about that, but the existing function does not have any
> parameters, while the DT version of get_pendown_state() requires to get
> the GPIO passed to it somehow.

You can always have tsc2007_get_pendown_state_platform() wrapping the
call. Or we just go and fix board code.

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

^ permalink raw reply

* Re: [PATCH] SOUND/SPI: Fix "clk_round_rate" undefined reference error.
From: Dmitry Torokhov @ 2013-10-22 22:53 UTC (permalink / raw)
  To: Majunath Goudar; +Cc: linux-arm-kernel, nataraja.km, linux-input, linux-kernel
In-Reply-To: <1381228833-24082-1-git-send-email-csmanjuvijay@gmail.com>

Hi Majunath,

On Tue, Oct 08, 2013 at 04:10:33PM +0530, Majunath Goudar wrote:
> This patch adds a COMMON_CLK dependency to configure the
> SND_AT73C213 for Atmel AT73C213 DAC driver. Without this patch,
> build system can lead to build failure. This was observed during
> randconfig testing, in which SND_AT73C213 was enabled w/o COMMON_CLK
> being enabled. leading to the following error:
> 
> sound/built-in.o: In function `snd_at73c213_probe':
> :(.text+0x328c4): undefined reference to `clk_round_rate'
> make: *** [vmlinux] Error 1
> 
> Signed-off-by: Manjunath Goudar <csmanjuvijay@gmail.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  sound/spi/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/spi/Kconfig b/sound/spi/Kconfig
> index e6485be..798d1ee 100644
> --- a/sound/spi/Kconfig
> +++ b/sound/spi/Kconfig
> @@ -11,7 +11,7 @@ if SND_SPI
>  
>  config SND_AT73C213
>  	tristate "Atmel AT73C213 DAC driver"
> -	depends on ATMEL_SSC
> +	depends on ATMEL_SSC && COMMON_CLK

I do not see clk_round_rate() being protected by CONFIG_COMMON_CLK so it
looks to me that ATMEL_SSC is at fault here.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v3 1/1] Input: Improve the performance of alps v5-protocol's touchpad
From: Kevin Cernekee @ 2013-10-23  0:09 UTC (permalink / raw)
  To: Yunkang Tang
  Cc: Dmitry Torokhov, david turvene, linux-input, Niels de Vos,
	yunkang.tang
In-Reply-To: <1382250180-3579-1-git-send-email-yunkang.tang@cn.alps.com>

On Sat, Oct 19, 2013 at 11:23 PM, Yunkang Tang <tommywill2011@gmail.com> wrote:
>  /*
> + * Process bitmap data for V5 protocols. Return value is null.
> + *
> + * The bitmaps don't have enough data to track fingers, so this function
> + * only generates points representing a bounding box of at most two contacts.
> + * These two points are returned in x1, y1, x2, and y2.
> + */
> +static void alps_process_bitmap_dolphin(struct alps_data *priv,
> +                                        struct alps_fields *fields,
> +                                        int *x1, int *y1, int *x2, int *y2)
> +{
> +        int box_middle_x, box_middle_y;
> +        unsigned int x_map, y_map;
> +        unsigned char start_bit, end_bit;
> +
> +        x_map = fields->x_map;
> +        y_map = fields->y_map;
> +
> +        if (!x_map || !y_map)
> +                return;
> +
> +        /* Most-significant bit should never exceed max sensor line number */
> +        if (fls(x_map) > priv->x_bits || fls(y_map) > priv->y_bits)
> +                return;

fls() does require a little computation, so it might be preferable to
calculate it once and store the result.

> +
> +        *x1 = *y1 = *x2 = *y2 = 0;
> +
> +        if (fields->fingers > 1) {
> +                start_bit = priv->x_bits - fls(x_map);
> +                end_bit = priv->x_bits - ffs(x_map);
> +                box_middle_x = (priv->x_max * (start_bit + end_bit)) /
> +                                (2 * (priv->x_bits - 1));
> +
> +                start_bit = ffs(y_map) - 1;
> +                end_bit = fls(y_map) - 1;
> +                box_middle_y = (priv->y_max * (start_bit + end_bit)) /
> +                                (2 * (priv->y_bits - 1));

The revised logic looks good to me.

> +                *x1 = fields->x;
> +                *y1 = fields->y;
> +                *x2 = 2 * box_middle_x - *x1;
> +                *y2 = 2 * box_middle_y - *y1;
> +        }
> +}
> +
> +/*
>   * Process bitmap data from v3 and v4 protocols. Returns the number of
>   * fingers detected. A return value of 0 means at least one of the
>   * bitmaps was empty.
> @@ -461,7 +505,8 @@ static void alps_decode_buttons_v3(struct alps_fields *f, unsigned char *p)
>          f->ts_middle = !!(p[3] & 0x40);
>  }
>
> -static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p)
> +static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p,
> +                                 struct psmouse *psmouse)
>  {
>          f->first_mp = !!(p[4] & 0x40);
>          f->is_mp = !!(p[0] & 0x40);
> @@ -482,48 +527,61 @@ static void alps_decode_pinnacle(struct alps_fields *f, unsigned char *p)
>          alps_decode_buttons_v3(f, p);
>  }
>
> -static void alps_decode_rushmore(struct alps_fields *f, unsigned char *p)
> +static void alps_decode_rushmore(struct alps_fields *f, unsigned char *p,
> +                                 struct psmouse *psmouse)
>  {
> -        alps_decode_pinnacle(f, p);
> +        alps_decode_pinnacle(f, p, psmouse);
>
>          f->x_map |= (p[5] & 0x10) << 11;
>          f->y_map |= (p[5] & 0x20) << 6;
>  }
>
> -static void alps_decode_dolphin(struct alps_fields *f, unsigned char *p)
> +static void alps_decode_dolphin(struct alps_fields *f, unsigned char *p,
> +                                struct psmouse *psmouse)
>  {
> +        u64 palm_data = 0;
> +        struct alps_data *priv = psmouse->private;
> +
>          f->first_mp = !!(p[0] & 0x02);
>          f->is_mp = !!(p[0] & 0x20);
>
> -        f->fingers = ((p[0] & 0x6) >> 1 |
> +        if (!f->is_mp) {
> +                f->x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
> +                f->y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
> +                f->z = (p[0] & 4) ? 0 : p[5] & 0x7f;
> +                alps_decode_buttons_v3(f, p);
> +        } else {
> +                f->fingers = ((p[0] & 0x6) >> 1 |
>                       (p[0] & 0x10) >> 2);
> -        f->x_map = ((p[2] & 0x60) >> 5) |
> -                   ((p[4] & 0x7f) << 2) |
> -                   ((p[5] & 0x7f) << 9) |
> -                   ((p[3] & 0x07) << 16) |
> -                   ((p[3] & 0x70) << 15) |
> -                   ((p[0] & 0x01) << 22);
> -        f->y_map = (p[1] & 0x7f) |
> -                   ((p[2] & 0x1f) << 7);
> -
> -        f->x = ((p[1] & 0x7f) | ((p[4] & 0x0f) << 7));
> -        f->y = ((p[2] & 0x7f) | ((p[4] & 0xf0) << 3));
> -        f->z = (p[0] & 4) ? 0 : p[5] & 0x7f;
>
> -        alps_decode_buttons_v3(f, p);
> +                palm_data = (p[1] & 0x7f) |
> +                            ((p[2] & 0x7f) << 7) |
> +                            ((p[4] & 0x7f) << 14) |
> +                            ((p[5] & 0x7f) << 21) |
> +                            ((p[3] & 0x07) << 28) |
> +                            (((u64)p[3] & 0x70) << 27) |
> +                            (((u64)p[0] & 0x01) << 34);
> +
> +                /* Y-profile is stored in P(0) to p(n-1), n = y_bits; */
> +                f->y_map = palm_data & (BIT(priv->y_bits) - 1);
> +
> +                /* X-profile is stored in p(n) to p(n+m-1), m = x_bits; */
> +                f->x_map = (palm_data >> priv->y_bits) &
> +                           (BIT(priv->x_bits) - 1);
> +        }

This also looks OK.  For a touchpad with x_bits = 23, the computed MT
X/Y maps look equivalent to the old code.

[snip]

> +static int alps_dolphin_get_device_area(struct psmouse *psmouse,
> +                                        struct alps_data *priv)
> +{
> +        struct ps2dev *ps2dev = &psmouse->ps2dev;
> +        unsigned char param[4] = {0};
> +        int num_x_electrode, num_y_electrode;
> +
> +        if (alps_enter_command_mode(psmouse))
> +                return -1;
> +
> +        if (ps2_command(ps2dev, NULL, 0x00EC) ||
> +            ps2_command(ps2dev, NULL, 0x00F0) ||
> +            ps2_command(ps2dev, NULL, 0x00F0) ||
> +            ps2_command(ps2dev, NULL, 0x00F3) ||
> +            ps2_command(ps2dev, NULL, 0x000A) ||
> +            ps2_command(ps2dev, NULL, 0x00F3) ||
> +            ps2_command(ps2dev, NULL, 0x000A))
> +                return -1;

The other ps2_command() calls in alps.c tend to use the PSMOUSE_*
constants instead of raw hex values.

(Sorry I didn't catch this the first time around.)

Everything else looks fine.  So:

Reviewed-by: Kevin Cernekee <cernekee@gmail.com>

^ permalink raw reply

* [PATCH] Input: add i2c driver for elan i2c touchpad
From: Duson Lin @ 2013-10-23  3:15 UTC (permalink / raw)
  To: linux-kernel, linux-input, dmitry.torokhov; +Cc: agnescheng, phoenix, Duson Lin

From: Duson Lin <dusonlin@emc.com.tw>

This driver adds support for elan i2c touchpad found on some laptops.
---
 drivers/input/mouse/Kconfig    |   10 +
 drivers/input/mouse/Makefile   |    1 +
 drivers/input/mouse/elan_i2c.c | 1439 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1450 insertions(+)
 create mode 100644 drivers/input/mouse/elan_i2c.c

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index effa9c5..8ad4b38 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -215,6 +215,16 @@ config MOUSE_CYAPA
 	  To compile this driver as a module, choose M here: the module will be
 	  called cyapa.
 
+config MOUSE_ELAN_I2C
+	tristate "ELAN I2C Touchpad support"
+	depends on I2C
+	help
+	  This driver adds support for Elan I2C Trackpads.
+	  Say y here if you have a ELAN I2C Touchpad.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called elan_i2c.
+
 config MOUSE_INPORT
 	tristate "InPort/MS/ATIXL busmouse"
 	depends on ISA
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index c25efdb..24a12a6 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_MOUSE_APPLETOUCH)		+= appletouch.o
 obj-$(CONFIG_MOUSE_ATARI)		+= atarimouse.o
 obj-$(CONFIG_MOUSE_BCM5974)		+= bcm5974.o
 obj-$(CONFIG_MOUSE_CYAPA)		+= cyapa.o
+obj-$(CONFIG_MOUSE_ELAN_I2C)		+= elan_i2c.o
 obj-$(CONFIG_MOUSE_GPIO)		+= gpio_mouse.o
 obj-$(CONFIG_MOUSE_INPORT)		+= inport.o
 obj-$(CONFIG_MOUSE_LOGIBM)		+= logibm.o
diff --git a/drivers/input/mouse/elan_i2c.c b/drivers/input/mouse/elan_i2c.c
new file mode 100644
index 0000000..bc6e957
--- /dev/null
+++ b/drivers/input/mouse/elan_i2c.c
@@ -0,0 +1,1439 @@
+/*
+ * Elan I2C Touchpad driver
+ *
+ * Copyright (c) 2013 ELAN Microelectronics Corp.
+ *
+ * Author: 林政維 (Duson Lin) <dusonlin@emc.com.tw>
+ * Version: 1.4.5
+ *
+ * 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.
+ *
+ * Trademarks are the property of their respective owners.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/firmware.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/input/mt.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/debugfs.h>
+#include <linux/cdev.h>
+#include <linux/kernel.h>
+#include <linux/major.h>
+#include <linux/sched.h>
+#include <linux/input.h>
+#include <linux/uaccess.h>
+#include <linux/jiffies.h>
+
+#define DRIVER_NAME		"elan_i2c"
+#define ELAN_DRIVER_VERSION	"1.4.5"
+#define ETP_PRESSURE_OFFSET	25
+#define ETP_MAX_PRESSURE	255
+#define ETP_FWIDTH_REDUCE	90
+#define ETP_FINGER_WIDTH	15
+
+#define ELAN_ADAPTER_FUNC_NONE   0
+#define ELAN_ADAPTER_FUNC_I2C    1
+
+/* Length of Elan touchpad information */
+#define ETP_INF_LENGTH		2
+#define ETP_MAX_FINGERS		5
+#define ETP_FINGER_DATA_LEN	5
+#define ETP_REPORT_ID		0x5D
+#define ETP_MAX_REPORT_LEN	34
+#define ETP_ENABLE_ABS		0x0001
+#define ETP_ENABLE_CALIBRATE	0x0002
+#define ETP_DISABLE_CALIBRATE	0x0000
+
+/* Elan i2c command */
+#define ETP_I2C_RESET			0x0100
+#define ETP_I2C_WAKE_UP			0x0800
+#define ETP_I2C_SLEEP			0x0801
+#define ETP_I2C_DESC_CMD		0x0001
+#define ETP_I2C_REPORT_DESC_CMD		0x0002
+#define ETP_I2C_STAND_CMD		0x0005
+#define ETP_I2C_UNIQUEID_CMD		0x0101
+#define ETP_I2C_FW_VERSION_CMD		0x0102
+#define ETP_I2C_SM_VERSION_CMD		0x0103
+#define ETP_I2C_XY_TRACENUM_CMD		0x0105
+#define ETP_I2C_MAX_X_AXIS_CMD		0x0106
+#define ETP_I2C_MAX_Y_AXIS_CMD		0x0107
+#define ETP_I2C_RESOLUTION_CMD		0x0108
+#define ETP_I2C_IAP_VERSION_CMD		0x0110
+#define ETP_I2C_SET_CMD			0x0300
+#define ETP_I2C_MAX_BASELINE_CMD	0x0306
+#define ETP_I2C_MIN_BASELINE_CMD	0x0307
+#define ETP_I2C_FW_CHECKSUM_CMD		0x030F
+#define ETP_I2C_IAP_CTRL_CMD		0x0310
+#define ETP_I2C_IAP_CMD			0x0311
+#define ETP_I2C_IAP_RESET_CMD		0x0314
+#define ETP_I2C_IAP_CHECKSUM_CMD	0x0315
+#define ETP_I2C_CALIBRATE_CMD		0x0316
+#define ETP_I2C_REPORT_LEN		34
+#define ETP_I2C_FINGER_DATA_OFFSET	4
+#define ETP_I2C_REPORT_ID_OFFSET	2
+#define ETP_I2C_DESC_LENGTH		30
+#define ETP_I2C_REPORT_DESC_LENGTH	158
+
+/* IAP F/W updater */
+#define ETP_FW_NAME		"elan_i2c.bin"
+#define ETP_FW_IAP_REG_L	0x01
+#define ETP_FW_IAP_REG_H	0x06
+#define ETP_IAP_VERSION_ADDR	0x0082
+#define ETP_IAP_START_ADDR	0x0083
+#define ETP_IAP_RESET		0xF0F0
+#define ETP_ENABLE_FWUPDATE	0x1EA5
+#define ETP_FW_IAP_MODE_ON	(1<<9)
+#define ETP_FW_IAP_PAGE_ERR	(1<<5)
+#define ETP_FW_IAP_INTERFACE_ERR (1<<4)
+#define ETP_FW_PAGE_SIZE	64
+#define ETP_FW_PAGE_COUNT	768
+#define ETP_FW_SIZE		(ETP_FW_PAGE_SIZE * ETP_FW_PAGE_COUNT)
+enum {IAP_MODE = 0, MAIN_MODE};
+
+struct dbfs_data {
+	bool	bfetch;
+	u8	buffer[ETP_MAX_REPORT_LEN];
+};
+
+/* The main device structure */
+struct elan_tp_data {
+	struct i2c_client	*client;
+	struct input_dev	*input;
+	unsigned int		max_x;
+	unsigned int		max_y;
+	unsigned int		width_x;
+	unsigned int		width_y;
+	unsigned int		irq;
+
+	/* fields required for IAP firmware updater */
+	u16			unique_id;
+	u16			fw_version;
+	u16			sm_version;
+	u16			iap_version;
+	bool			updated_fw;
+	u16			iap_start_addr;
+	u8			adapter_func;
+
+	/* irq wake is enabled */
+	bool			irq_wake;
+	bool			enable_detail_info;
+
+	/* fields required for debug fs */
+	struct mutex		dbfs_mutex;
+	struct dentry		*dbfs_root;
+	struct dbfs_data	dbfs_buffer;
+};
+
+u8 val[256];
+
+static int elan_i2c_read_block(struct i2c_client *client,
+				 u16 reg, u8 *val, u16 len);
+static int elan_i2c_read_cmd(struct i2c_client *client, u16 reg, u8 *val);
+static int elan_i2c_write_cmd(struct i2c_client *client, u16 reg, u16 cmd);
+static int elan_i2c_reinitialize(struct i2c_client *client);
+static int elan_i2c_enable_calibrate(struct i2c_client *client);
+static int elan_i2c_disable_calibrate(struct i2c_client *client);
+
+/*
+ **************************************************************
+ * debugfs interface
+ **************************************************************
+*/
+static int elan_dbfs_open(struct inode *inode, struct file *file)
+{
+	int retval;
+	struct elan_tp_data *data = inode->i_private;
+
+	if (!data)
+		return -ENODEV;
+
+	retval = mutex_lock_interruptible(&data->dbfs_mutex);
+	if (retval)
+		return retval;
+
+	if (!kobject_get(&data->client->dev.kobj)) {
+		retval = -ENODEV;
+		goto dbfs_out;
+	}
+
+	file->private_data = data;
+dbfs_out:
+	mutex_unlock(&data->dbfs_mutex);
+	return 0;
+}
+
+static int elan_dbfs_release(struct inode *inode, struct file *file)
+{
+	struct elan_tp_data *data = file->private_data;
+	int retval;
+	if (!data)
+		return -ENODEV;
+
+	retval = mutex_lock_interruptible(&data->dbfs_mutex);
+	if (retval)
+		return retval;
+	file->private_data = NULL;
+	kobject_put(&data->client->dev.kobj);
+	mutex_unlock(&data->dbfs_mutex);
+	return 0;
+}
+
+
+static ssize_t elan_dbfs_read(struct file *file,
+		char __user *buffer, size_t count, loff_t *ppos)
+{
+	struct elan_tp_data *data = file->private_data;
+	int retval;
+	if (!data)
+		return -ENODEV;
+
+	retval = mutex_lock_interruptible(&data->dbfs_mutex);
+	if (retval)
+		return -EFAULT;
+	if (data->dbfs_buffer.bfetch == false) {
+		if (!copy_to_user(buffer, data->dbfs_buffer.buffer, count)) {
+			data->dbfs_buffer.bfetch = true;
+			retval = count;
+		} else {
+			retval = -2;
+		}
+	} else {
+		retval = -4;
+	}
+	mutex_unlock(&data->dbfs_mutex);
+	return retval;
+}
+
+static ssize_t elan_dbfs_write(struct file *file,
+		const char __user *buffer, size_t count, loff_t *ppos)
+{
+	struct elan_tp_data *data = file->private_data;
+	int retval;
+	if (!data)
+		return -ENODEV;
+
+	retval = mutex_lock_interruptible(&data->dbfs_mutex);
+	if (retval)
+		return -EFAULT;
+	retval = count;
+	mutex_unlock(&data->dbfs_mutex);
+	return retval;
+}
+
+static long elan_dbfs_ioctrl(struct file *file,
+		unsigned int cmd, unsigned long arg)
+{
+	int retval = 0;
+	struct elan_tp_data *data = file->private_data;
+
+	retval = mutex_lock_interruptible(&data->dbfs_mutex);
+	if (retval)
+		return retval;
+	mutex_unlock(&data->dbfs_mutex);
+	return retval;
+}
+
+static const struct file_operations elan_debug_fops = {
+	.open = elan_dbfs_open,
+	.release = elan_dbfs_release,
+	.read = elan_dbfs_read,
+	.write = elan_dbfs_write,
+	.unlocked_ioctl = elan_dbfs_ioctrl
+};
+
+static int elan_dbfs_init(struct elan_tp_data *data)
+{
+	/* Create a global debugfs root for all elan devices */
+	/* sys/kernel/debug/elan */
+	data->dbfs_root = debugfs_create_dir("elan", NULL);
+	if (!data->dbfs_root) {
+		dev_err(&data->client->dev, "cannot create dbfs_root.\n");
+		return -ENODEV;
+	}
+	mutex_init(&data->dbfs_mutex);
+
+	debugfs_create_file(DRIVER_NAME, 0777,
+				data->dbfs_root, data, &elan_debug_fops);
+	data->dbfs_buffer.bfetch = false;
+	return 0;
+}
+
+/**********************************************************
+ * IAP firmware updater related routines                  *
+ **********************************************************
+*/
+
+static int elan_iap_getmode(struct elan_tp_data *data)
+{
+	u16 constant;
+	int retval = 0;
+	struct i2c_client *client = data->client;
+
+	retval = elan_i2c_read_cmd(client, ETP_I2C_IAP_CTRL_CMD, val);
+	if (retval < 0)
+		return false;
+
+	constant = le16_to_cpup((__le16 *)val);
+	dev_dbg(&client->dev, "control reg: 0x%04x.\n", constant);
+
+	if (constant & ETP_FW_IAP_MODE_ON)
+		return MAIN_MODE;
+
+	return IAP_MODE;
+}
+
+static int elan_iap_checksum(struct elan_tp_data *data)
+{
+	int retval = 0;
+	u16 checksum = -1;
+	struct i2c_client *client = data->client;
+
+	retval = elan_i2c_read_cmd(client, ETP_I2C_IAP_CHECKSUM_CMD, val);
+	if (retval < 0) {
+		dev_err(&client->dev, "Read checksum fail, %d\n", retval);
+		return -1;
+	}
+	checksum = le16_to_cpup((__le16 *)val);
+	return checksum;
+}
+
+static bool elan_iap_reset(struct elan_tp_data *data)
+{
+	int retval = 0;
+	struct i2c_client *client = data->client;
+
+	retval = elan_i2c_write_cmd(client, ETP_I2C_IAP_RESET_CMD,
+						ETP_IAP_RESET);
+	if (retval < 0) {
+		dev_err(&client->dev, "cannot reset IC, %d\n", retval);
+		return false;
+	}
+	return true;
+}
+
+static bool elan_iap_setflashkey(struct elan_tp_data *data, int mode)
+{
+	int retval = 0;
+	struct i2c_client *client = data->client;
+
+	retval = elan_i2c_write_cmd(client, ETP_I2C_IAP_CMD,
+						ETP_ENABLE_FWUPDATE);
+	if (retval < 0) {
+		dev_err(&client->dev, "cannot set flash key, %d\n", retval);
+		return false;
+	}
+
+	/* Wait for F/W IAP initialization */
+	if (mode == MAIN_MODE)
+		msleep(100);
+	else
+		msleep(30);
+
+	return true;
+}
+
+static bool elan_iap_page_write_ok(struct elan_tp_data *data)
+{
+	u16 constant;
+	int retval = 0;
+	struct i2c_client *client = data->client;
+
+	retval = elan_i2c_read_cmd(client, ETP_I2C_IAP_CTRL_CMD, val);
+	if (retval < 0)
+		return false;
+
+	constant = le16_to_cpup((__le16 *)val);
+
+	if (constant & ETP_FW_IAP_PAGE_ERR)
+		return false;
+
+	if (constant & ETP_FW_IAP_INTERFACE_ERR)
+		return false;
+
+	return true;
+}
+
+static int elan_check_fw(struct elan_tp_data *data,
+				const struct firmware *fw)
+{
+	u16 uniqueid_addr, uniqueid;
+	struct device *dev = &data->client->dev;
+
+	/* Firmware must match exact PAGE_NUM * PAGE_SIZE bytes */
+	if (fw->size != ETP_FW_SIZE) {
+		dev_err(dev, "invalid firmware size = %zu, expected %d.\n",
+					fw->size, ETP_FW_SIZE);
+		return -EBADF;
+	}
+
+	/* Check IAP Version */
+	if (!memcmp(&fw->data[ETP_IAP_VERSION_ADDR * 2],
+				&data->iap_version, 2)) {
+		dev_err(dev, "IAP F/W updating version does not match.");
+		return -EEXIST;
+	}
+
+	/* Get IAP Start Address*/
+	memcpy(&data->iap_start_addr, &fw->data[ETP_IAP_START_ADDR * 2], 2);
+	/* Get Unique ID Address */
+	memcpy(&uniqueid_addr, &fw->data[data->iap_start_addr * 2], 2);
+	/* Get Unique ID */
+	memcpy(&uniqueid, &fw->data[uniqueid_addr * 2], 2);
+
+	return 0;
+}
+
+static int elan_prepare_fw_update(struct elan_tp_data *data)
+{
+	struct i2c_client *client = data->client;
+	struct device *dev = &data->client->dev;
+
+	/* Get FW in which mode	(IAP_MODE/MAIN_MODE)  */
+	int mode = elan_iap_getmode(data);
+
+	if (mode == IAP_MODE) {
+		/* Reset IC */
+		if (elan_iap_reset(data) == false)
+			return -1;
+		msleep(30);
+	}
+
+	/* set flash key*/
+	if (elan_iap_setflashkey(data, mode) == false) {
+		dev_err(dev, "cannot set flash key\n");
+		return -1;
+	}
+
+	/* check is in iap mode or not*/
+	if (elan_iap_getmode(data) == MAIN_MODE) {
+		dev_err(dev, "status wrong.\n");
+		return -1;
+	}
+
+	/* set flash key again */
+	if (elan_iap_setflashkey(data, mode) == false) {
+		dev_err(dev, "cannot set flash key\n");
+		return -1;
+	}
+
+	/* read back to check we actually enabled successfully. */
+	if (elan_i2c_read_cmd(client, ETP_I2C_IAP_CMD, val) < 0) {
+		dev_err(dev, "cannot get iap register\n");
+		return -1;
+	}
+
+	if (le16_to_cpup((__le16 *)val) != ETP_ENABLE_FWUPDATE) {
+		dev_err(dev, "wrong iap password = 0x%X\n",
+				le16_to_cpup((__le16 *)val));
+		return -1;
+	}
+	return 0;
+}
+
+static int elan_write_fw_block(struct elan_tp_data *data,
+				const u8 *page, u16 checksum)
+{
+	struct device *dev = &data->client->dev;
+	int ret;
+	int repeat = 3;
+	u8 page_store[ETP_FW_PAGE_SIZE + 4];
+
+	page_store[0] = ETP_FW_IAP_REG_L;
+	page_store[1] = ETP_FW_IAP_REG_H;
+	memcpy(&page_store[2], page, ETP_FW_PAGE_SIZE);
+
+	/* recode checksum at last two bytes */
+	page_store[ETP_FW_PAGE_SIZE+2] = (u8)(checksum & 0xFF);
+	page_store[ETP_FW_PAGE_SIZE+3] = (u8)((checksum >> 8)&0xFF);
+
+	do {
+		ret = i2c_master_send(data->client, page_store,
+						 ETP_FW_PAGE_SIZE + 4);
+
+		/* Wait for F/W to update one page ROM data. */
+		msleep(20);
+
+		if (ret == (ETP_FW_PAGE_SIZE + 4)) {
+			if (elan_iap_page_write_ok(data))
+				break;
+		}
+		dev_dbg(dev, "IAP retry this page!\n");
+		repeat--;
+	} while (repeat == 0);
+
+	if (repeat > 0)
+		return 0;
+	return -1;
+}
+
+static int elan_firmware(struct elan_tp_data *data)
+{
+	struct device *dev = &data->client->dev;
+	const struct firmware *fw;
+	const char *fw_name = ETP_FW_NAME;
+	int i, j, ret;
+	u16 boot_page_count;
+	u16 sw_checksum, fw_checksum;
+
+	ret = request_firmware(&fw, ETP_FW_NAME, dev);
+	if (ret) {
+		dev_err(dev, "cannot load firmware from %s, %d\n",
+							fw_name, ret);
+		goto done;
+	}
+	ret = elan_check_fw(data, fw);
+	if (ret) {
+		dev_err(dev, "Invalid Elan firmware from %s, %d\n",
+							fw_name, ret);
+		goto done;
+	}
+
+	/* set flash key and into IAP mode */
+	ret = elan_prepare_fw_update(data);
+	if (ret)
+		goto done;
+
+	sw_checksum = 0;
+	fw_checksum = 0;
+	boot_page_count = (data->iap_start_addr * 2) / ETP_FW_PAGE_SIZE;
+	for (i = boot_page_count; i < ETP_FW_PAGE_COUNT; i++) {
+		u16 checksum = 0;
+		const u8 *page = &fw->data[i * ETP_FW_PAGE_SIZE];
+
+		for (j = 0; j < ETP_FW_PAGE_SIZE; j += 2)
+			checksum += ((page[j + 1] << 8) | page[j]);
+
+		ret = elan_write_fw_block(data, page, checksum);
+		if (ret)
+			goto done;
+		sw_checksum += checksum;
+	}
+	/* Wait WDT rest and power on reset*/
+	msleep(600);
+
+	/* check checksum */
+	fw_checksum = elan_iap_checksum(data);
+	if (sw_checksum != fw_checksum) {
+		dev_err(dev, "checksum diff sw=[%04X], fw=[%04X]\n",
+					sw_checksum, fw_checksum);
+		ret = -1;
+		goto done;
+	}
+	ret = 0;
+done:
+	if (ret != 0)
+		elan_iap_reset(data);
+	release_firmware(fw);
+	return ret;
+}
+
+/**************************************************************************
+* Genernal functions
+***************************************************************************
+*/
+
+/*
+ * (value from firmware) * 10 + 790 = dpi
+ * we also have to convert dpi to dots/mm (*10/254 to avoid floating point)
+ */
+static unsigned int elan_convert_res(char val)
+{
+	int res;
+	if (val & 0x80) {
+		val = ~val + 1;
+		res = (790 - val * 10) * 10 / 254;
+	} else
+		res = (val * 10 + 790) * 10 / 254;
+	return res;
+}
+
+static int elan_get_iap_version(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_block(data->client, ETP_I2C_IAP_VERSION_CMD, val, 3);
+	ret = le16_to_cpup((__le16 *)val);
+	return ret;
+}
+
+static int elan_get_x_max(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_MAX_X_AXIS_CMD, val);
+	ret = (0x0f & val[1]) << 8 | val[0];
+	return ret;
+}
+
+static int elan_get_y_max(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_MAX_Y_AXIS_CMD, val);
+	ret = (0x0f & val[1]) << 8 | val[0];
+	return ret;
+}
+
+static int elan_get_x_tracenum(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_XY_TRACENUM_CMD, val);
+	ret = (val[0] - 1);
+	return ret;
+}
+
+static int elan_get_y_tracenum(struct elan_tp_data *data)
+{
+	int ret;
+	ret = elan_i2c_read_cmd(data->client, ETP_I2C_XY_TRACENUM_CMD, val);
+	ret = (val[1] - 1);
+	return ret;
+}
+
+static int elan_get_fw_version(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_FW_VERSION_CMD, val);
+	ret = le16_to_cpup((__le16 *)val);
+	return ret;
+}
+
+static int elan_get_sm_version(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_block(data->client, ETP_I2C_SM_VERSION_CMD, val, 1);
+	ret = val[0];
+	return ret;
+}
+
+static int elan_get_unique_id(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_UNIQUEID_CMD, val);
+	ret = le16_to_cpup((__le16 *)val);
+	return ret;
+}
+
+static int elan_get_x_resolution(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_RESOLUTION_CMD, val);
+	ret = elan_convert_res(val[0]);
+	return ret;
+}
+
+static int elan_get_y_resolution(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_RESOLUTION_CMD, val);
+	ret = elan_convert_res(val[1]);
+	return ret;
+}
+
+static int elan_get_fw_checksum(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_FW_CHECKSUM_CMD, val);
+	ret = le16_to_cpup((__le16 *)val);
+	return ret;
+}
+
+static int elan_get_max_baseline(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_MAX_BASELINE_CMD, val);
+	ret = le16_to_cpup((__le16 *)val);
+	return ret;
+}
+
+static int elan_get_min_baseline(struct elan_tp_data *data)
+{
+	int ret;
+	elan_i2c_read_cmd(data->client,	ETP_I2C_MIN_BASELINE_CMD, val);
+	ret = le16_to_cpup((__le16 *)val);
+	return ret;
+}
+
+
+static int elan_enable_calibrate(struct elan_tp_data *data)
+{
+	int ret;
+	ret = elan_i2c_enable_calibrate(data->client);
+	return ret;
+}
+
+static int elan_disable_calibrate(struct elan_tp_data *data)
+{
+	int ret;
+	ret = elan_i2c_disable_calibrate(data->client);
+	return ret;
+}
+
+/********************************************************************
+ * below routines export interfaces to sysfs file system.
+ * so user can get firmware/driver/hardware information using cat command.
+ * e.g.: use below command to get firmware version
+ *      cat /sys/bus/i2c/drivers/elan_i2c/1-0015/firmware_version
+ *******************************************************************
+ */
+static ssize_t elan_enable_detailinfo(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	data->enable_detail_info = true;
+	return sprintf(buf, "enable\n");
+}
+
+static ssize_t elan_read_fw_checksum(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	unsigned int checksum = 0;
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	if (data->enable_detail_info == true) {
+		checksum = elan_get_fw_checksum(data);
+		data->enable_detail_info = false;
+	}
+	return sprintf(buf, "0x%04x\n", checksum);
+}
+
+static ssize_t elan_read_unique_id(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	data->unique_id = elan_get_unique_id(data);
+	return sprintf(buf, "0x%04x\n", data->unique_id);
+}
+
+static ssize_t elan_read_driver_ver(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return sprintf(buf, "%s\n", ELAN_DRIVER_VERSION);
+}
+
+static ssize_t elan_read_fw_ver(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	data->fw_version = elan_get_fw_version(data);
+	return sprintf(buf, "0x%04x\n", data->fw_version);
+}
+
+static ssize_t elan_read_sm_ver(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	data->sm_version = elan_get_sm_version(data);
+	return sprintf(buf, "0x%04x\n", data->sm_version);
+}
+
+static ssize_t elan_read_iap_ver(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buf)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	data->iap_version = elan_get_iap_version(data);
+	return sprintf(buf, "0x%04x\n", data->iap_version);
+}
+
+
+static ssize_t elan_update_fw(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	int ret;
+	data->updated_fw = true;
+	ret = elan_firmware(data);
+	if (ret)
+		dev_err(dev, "firmware update failed.\n");
+	else
+		dev_info(dev, "firmware update succeeded.\n");
+	return ret ? ret : count;
+}
+
+static ssize_t elan_do_calibrate(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	int tries = 20;
+	int ret = 0;
+	val[0] = 0;
+
+	disable_irq(data->irq);
+	elan_enable_calibrate(data);
+	elan_i2c_write_cmd(data->client, ETP_I2C_CALIBRATE_CMD, 1);
+
+	do {
+		/* wait 250ms and check finish or not */
+		msleep(250);
+		elan_i2c_read_block(data->client,
+					ETP_I2C_CALIBRATE_CMD, val, 1);
+
+		/* calibrate finish */
+		if (val[0] == 0)
+			break;
+	} while (--tries);
+
+	elan_disable_calibrate(data);
+	enable_irq(data->irq);
+
+	if (tries == 0) {
+		dev_err(dev, "Failed to calibrate. Timeout.\n");
+		ret = -ETIMEDOUT;
+	}
+	return ret < 0 ? ret : count;
+}
+
+
+static ssize_t elan_read_baseline(struct device *dev,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	int max_baseline, min_baseline;
+
+	disable_irq(data->irq);
+	elan_enable_calibrate(data);
+	msleep(250);
+	max_baseline = elan_get_max_baseline(data);
+	min_baseline = elan_get_min_baseline(data);
+	elan_disable_calibrate(data);
+	enable_irq(data->irq);
+	return sprintf(buf, "max:%d min:%d\n", max_baseline, min_baseline);
+}
+
+static ssize_t elan_do_reinitialize(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+	int ret;
+	disable_irq(data->irq);
+	ret = elan_i2c_reinitialize(data->client);
+	enable_irq(data->irq);
+	if (ret < 0)
+		return sprintf(buf, "reinitialize fail\n");
+
+	return sprintf(buf, "reinitialize success\n");
+}
+
+static DEVICE_ATTR(unique_id, S_IRUGO, elan_read_unique_id, NULL);
+static DEVICE_ATTR(firmware_version, S_IRUGO, elan_read_fw_ver, NULL);
+static DEVICE_ATTR(sample_version, S_IRUGO, elan_read_sm_ver, NULL);
+static DEVICE_ATTR(driver_version, S_IRUGO, elan_read_driver_ver, NULL);
+static DEVICE_ATTR(iap_version, S_IRUGO, elan_read_iap_ver, NULL);
+static DEVICE_ATTR(fw_checksum, S_IRUGO, elan_read_fw_checksum, NULL);
+static DEVICE_ATTR(open_info, S_IRUGO, elan_enable_detailinfo, NULL);
+static DEVICE_ATTR(baseline, S_IRUGO, elan_read_baseline, NULL);
+static DEVICE_ATTR(reinitialize, S_IRUGO, elan_do_reinitialize, NULL);
+static DEVICE_ATTR(calibrate, S_IWUSR, NULL, elan_do_calibrate);
+static DEVICE_ATTR(update_fw, S_IWUSR, NULL, elan_update_fw);
+
+static struct attribute *elan_sysfs_entries[] = {
+	&dev_attr_unique_id.attr,
+	&dev_attr_firmware_version.attr,
+	&dev_attr_sample_version.attr,
+	&dev_attr_driver_version.attr,
+	&dev_attr_iap_version.attr,
+	&dev_attr_fw_checksum.attr,
+	&dev_attr_open_info.attr,
+	&dev_attr_baseline.attr,
+	&dev_attr_reinitialize.attr,
+	&dev_attr_calibrate.attr,
+	&dev_attr_update_fw.attr,
+	NULL,
+};
+
+static const struct attribute_group elan_sysfs_group = {
+	.attrs = elan_sysfs_entries,
+};
+
+/*****************************************************************
+* Elan i2c interface
+******************************************************************
+*/
+static int elan_i2c_read_block(struct i2c_client *client,
+				 u16 reg, u8 *val, u16 len)
+{
+	struct i2c_msg msgs[2];
+	u8 buf[2];
+	int ret;
+
+	buf[0] = reg & 0xff;
+	buf[1] = (reg >> 8) & 0xff;
+
+	msgs[0].addr = client->addr;
+	msgs[0].flags = client->flags & I2C_M_TEN;
+	msgs[0].len = 2;
+	msgs[0].buf = buf;
+
+	msgs[1].addr = client->addr;
+	msgs[1].flags = client->flags & I2C_M_TEN;
+	msgs[1].flags |= I2C_M_RD;
+	msgs[1].len = len;
+	msgs[1].buf = val;
+
+	ret = i2c_transfer(client->adapter, msgs, 2);
+	return ret != 2 ? -EIO : 0;
+}
+
+static int elan_i2c_read_cmd(struct i2c_client *client, u16 reg, u8 *val)
+{
+	int retval;
+
+	retval = elan_i2c_read_block(client, reg, val, ETP_INF_LENGTH);
+	if (retval < 0) {
+		dev_err(&client->dev, "reading cmd (0x%04x) fail.\n", reg);
+		return retval;
+	}
+	return 0;
+}
+
+static int elan_i2c_write_cmd(struct i2c_client *client, u16 reg, u16 cmd)
+{
+	struct i2c_msg msg;
+	u8 buf[4];
+	int ret;
+
+	buf[0] = reg & 0xff;
+	buf[1] = (reg >> 8) & 0xff;
+	buf[2] = cmd & 0xff;
+	buf[3] = (cmd >> 8) & 0xff;
+
+	msg.addr = client->addr;
+	msg.flags = client->flags & I2C_M_TEN;
+	msg.len = 4;
+	msg.buf = buf;
+
+	ret = i2c_transfer(client->adapter, &msg, 1);
+	return ret != 1 ? -EIO : 0;
+}
+
+static int elan_i2c_reset(struct i2c_client *client)
+{
+	return elan_i2c_write_cmd(client, ETP_I2C_STAND_CMD,
+						ETP_I2C_RESET);
+}
+
+static int elan_i2c_wake_up(struct i2c_client *client)
+{
+	return elan_i2c_write_cmd(client, ETP_I2C_STAND_CMD,
+						ETP_I2C_WAKE_UP);
+}
+
+static int elan_i2c_sleep(struct i2c_client *client)
+{
+	return elan_i2c_write_cmd(client, ETP_I2C_STAND_CMD,
+						ETP_I2C_SLEEP);
+}
+
+static int elan_i2c_enable_absolute_mode(struct i2c_client *client)
+{
+	return elan_i2c_write_cmd(client, ETP_I2C_SET_CMD,
+						ETP_ENABLE_ABS);
+}
+
+static int elan_i2c_enable_calibrate(struct i2c_client *client)
+{
+	return elan_i2c_write_cmd(client, ETP_I2C_SET_CMD,
+			ETP_ENABLE_ABS|ETP_ENABLE_CALIBRATE);
+}
+
+static int elan_i2c_disable_calibrate(struct i2c_client *client)
+{
+	return elan_i2c_write_cmd(client, ETP_I2C_SET_CMD,
+			ETP_ENABLE_ABS|ETP_DISABLE_CALIBRATE);
+}
+
+static int elan_i2c_get_desc(struct i2c_client *client, u8 *val)
+{
+	return elan_i2c_read_block(client, ETP_I2C_DESC_CMD, val,
+					ETP_I2C_DESC_LENGTH);
+}
+
+static int elan_i2c_get_report_desc(struct i2c_client *client, u8 *val)
+{
+	return elan_i2c_read_block(client, ETP_I2C_REPORT_DESC_CMD,
+				 val, ETP_I2C_REPORT_DESC_LENGTH);
+}
+
+static int elan_i2c_initialize(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	int rc;
+
+	rc = elan_i2c_reset(client);
+	if (rc < 0) {
+		dev_err(dev, "device reset failed.\n");
+		return -1;
+	}
+
+	/* wait for get reset return flag */
+	msleep(100);
+	/* get reset return flag 0000 */
+	rc = i2c_master_recv(client, val, ETP_INF_LENGTH);
+	if (rc < 0) {
+		dev_err(dev, "get device reset return value failed.\n");
+		return -1;
+	}
+
+	rc = elan_i2c_get_desc(client, val);
+	if (rc < 0) {
+		dev_err(dev, "cannot get device descriptor.\n");
+		return -1;
+	}
+
+	rc = elan_i2c_get_report_desc(client, val);
+	if (rc < 0) {
+		dev_err(dev, "fetching report descriptor failed.\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+static int elan_i2c_reinitialize(struct i2c_client *client)
+{
+	int ret;
+
+	ret = elan_i2c_initialize(client);
+	if (ret < 0) {
+		dev_err(&client->dev, "device initialize failed.\n");
+		goto err_i2c_reinitialize;
+	}
+
+	ret = elan_i2c_enable_absolute_mode(client);
+	if (ret < 0) {
+		dev_err(&client->dev, "cannot switch to absolute mode.\n");
+		goto err_i2c_reinitialize;
+	}
+
+	ret = elan_i2c_wake_up(client);
+	if (ret < 0)
+		dev_err(&client->dev, "device wake up failed.\n");
+err_i2c_reinitialize:
+	return ret;
+}
+
+/*****************************************************************
+* Elan isr functions
+******************************************************************
+*/
+
+static int elan_check_packet(struct elan_tp_data *data, u8 *packet)
+{
+	u8 rid;
+	rid = packet[ETP_I2C_REPORT_ID_OFFSET];
+	/* check report id */
+	if (rid != ETP_REPORT_ID) {
+		dev_err(&data->client->dev, "report id [%x] fail.\n", rid);
+		return -1;
+	}
+	return 0;
+}
+
+static void elan_report_absolute(struct elan_tp_data *data, u8 *packet)
+{
+	struct input_dev *input = data->input;
+	u8 *finger_data;
+	bool finger_on;
+	int pos_x, pos_y;
+	int pressure, mk_x, mk_y;
+	int i, area_x, area_y, major, minor, new_pressure;
+	int finger_count = 0;
+	int btn_click;
+	u8  tp_info;
+
+	finger_data = &packet[ETP_I2C_FINGER_DATA_OFFSET];
+	tp_info = packet[3];
+
+	btn_click = (tp_info & 0x01);
+	for (i = 0; i < ETP_MAX_FINGERS; i++) {
+		finger_on = (tp_info >> (3 + i)) & 0x01;
+
+		/* analyze touched finger raw data*/
+		if (finger_on) {
+			pos_x = ((finger_data[0] & 0xf0) << 4) |
+				finger_data[1];
+			pos_y = ((finger_data[0] & 0x0f) << 8) |
+				finger_data[2];
+			pos_y =  data->max_y - pos_y;
+			mk_x = (finger_data[3] & 0x0f);
+			mk_y = (finger_data[3] >> 4);
+			pressure = finger_data[4];
+
+			/*
+			to avoid fat finger be as palm, so reduce the
+			width x and y per trace
+			*/
+			area_x = mk_x * (data->width_x - ETP_FWIDTH_REDUCE);
+			area_y = mk_y * (data->width_y - ETP_FWIDTH_REDUCE);
+
+			major = max(area_x, area_y);
+			minor = min(area_x, area_y);
+
+			new_pressure = pressure + ETP_PRESSURE_OFFSET;
+			if (new_pressure > ETP_MAX_PRESSURE)
+				new_pressure = ETP_MAX_PRESSURE;
+
+			input_mt_slot(input, i);
+			input_mt_report_slot_state(input, MT_TOOL_FINGER,
+									true);
+			input_report_abs(input, ABS_MT_POSITION_X, pos_x);
+			input_report_abs(input, ABS_MT_POSITION_Y, pos_y);
+			input_report_abs(input, ABS_MT_PRESSURE, new_pressure);
+			input_report_abs(input, ABS_TOOL_WIDTH, mk_x);
+			input_report_abs(input, ABS_MT_TOUCH_MAJOR, major);
+			input_report_abs(input, ABS_MT_TOUCH_MINOR, minor);
+			finger_data += ETP_FINGER_DATA_LEN;
+			finger_count++;
+		} else {
+			input_mt_slot(input, i);
+			input_mt_report_slot_state(input,
+						MT_TOOL_FINGER, false);
+		}
+	}
+
+	input_report_key(input, BTN_LEFT, (btn_click == 1));
+	input_mt_report_pointer_emulation(input, true);
+	input_sync(input);
+}
+
+static irqreturn_t elan_isr(int irq, void *dev_id)
+{
+	struct elan_tp_data *data = dev_id;
+	u8 raw[ETP_MAX_REPORT_LEN];
+	int retval;
+	int report_len;
+
+	retval = mutex_lock_interruptible(&data->dbfs_mutex);
+	if (retval)
+		return IRQ_HANDLED;
+
+	if (data->updated_fw) {
+		retval = i2c_master_recv(data->client, raw,
+						ETP_INF_LENGTH);
+		if (retval == 2 && !le16_to_cpup((__le16 *)raw)) {
+			dev_info(&data->client->dev,
+				"reinitializing after F/W update...");
+			elan_i2c_reinitialize(data->client);
+		}
+		data->updated_fw = false;
+		goto elan_isr_end;
+	}
+
+	report_len = ETP_I2C_REPORT_LEN;
+	retval = i2c_master_recv(data->client, raw, report_len);
+
+	if (retval != report_len) {
+		dev_err(&data->client->dev, "wrong packet len(%d)", retval);
+		goto elan_isr_end;
+	}
+
+	if (elan_check_packet(data, raw) < 0) {
+		dev_err(&data->client->dev, "wrong packet format.");
+		goto elan_isr_end;
+	}
+	elan_report_absolute(data, raw);
+	data->dbfs_buffer.bfetch = false;
+	memcpy(data->dbfs_buffer.buffer, raw, report_len);
+
+elan_isr_end:
+	mutex_unlock(&data->dbfs_mutex);
+	return IRQ_HANDLED;
+}
+
+
+static int elan_input_dev_create(struct elan_tp_data *data)
+{
+	struct i2c_client *client = data->client;
+	struct input_dev *input;
+	unsigned int x_res, y_res;
+	int ret;
+
+	data->input = input = input_allocate_device();
+	if (!input)
+		return -ENOMEM;
+	input->name = "Elan Touchpad";
+	input->id.bustype = BUS_I2C;
+	input->dev.parent = &data->client->dev;
+
+	__set_bit(INPUT_PROP_POINTER, input->propbit);
+	__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(EV_ABS, input->evbit);
+
+	__set_bit(BTN_LEFT, input->keybit);
+	__set_bit(BTN_TOUCH, input->keybit);
+	__set_bit(BTN_TOOL_FINGER, input->keybit);
+	__set_bit(BTN_TOOL_DOUBLETAP, input->keybit);
+	__set_bit(BTN_TOOL_TRIPLETAP, input->keybit);
+	__set_bit(BTN_TOOL_QUADTAP, input->keybit);
+	__set_bit(BTN_TOOL_QUINTTAP, input->keybit);
+
+	__set_bit(ABS_MT_TOUCH_MAJOR, input->absbit);
+	__set_bit(ABS_MT_TOUCH_MINOR, input->absbit);
+	__set_bit(ABS_MT_POSITION_X, input->absbit);
+	__set_bit(ABS_MT_POSITION_Y, input->absbit);
+
+	data->unique_id = elan_get_unique_id(data);
+	data->fw_version = elan_get_fw_version(data);
+	data->sm_version = elan_get_sm_version(data);
+	data->iap_version = elan_get_iap_version(data);
+	data->max_x = elan_get_x_max(data);
+	data->max_y = elan_get_y_max(data);
+	data->width_x = data->max_x / elan_get_x_tracenum(data);
+	data->width_y = data->max_y / elan_get_y_tracenum(data);
+	x_res = elan_get_x_resolution(data);
+	y_res = elan_get_y_resolution(data);
+
+	dev_info(&client->dev,
+		"Elan Touchpad Information:\n"
+		"    Module unique ID:  0x%04x\n"
+		"    Firmware Version:  0x%04x\n"
+		"    Sample Version:  0x%04x\n"
+		"    IAP Version:  0x%04x\n"
+		"    Max ABS X,Y:   %d,%d\n"
+		"    Width X,Y:   %d,%d\n"
+		"    Resolution X,Y:   %d,%d (dots/mm)\n",
+		data->unique_id,
+		data->fw_version,
+		data->sm_version,
+		data->iap_version,
+		data->max_x, data->max_y,
+		data->width_x, data->width_y,
+		(char)x_res, (char)y_res);
+
+	input_set_abs_params(input, ABS_X, 0, data->max_x, 0, 0);
+	input_set_abs_params(input, ABS_Y, 0, data->max_y, 0, 0);
+	input_abs_set_res(input, ABS_X, x_res);
+	input_abs_set_res(input, ABS_Y, y_res);
+	input_set_abs_params(input, ABS_PRESSURE, 0, ETP_MAX_PRESSURE, 0, 0);
+	input_set_abs_params(input, ABS_TOOL_WIDTH, 0, ETP_FINGER_WIDTH, 0, 0);
+
+	/* handle pointer emulation and unused slots in core */
+	ret = input_mt_init_slots(input, ETP_MAX_FINGERS,
+				  INPUT_MT_POINTER | INPUT_MT_DROP_UNUSED);
+	if (ret) {
+		dev_err(&client->dev, "allocate MT slots failed, %d\n", ret);
+		goto err_free_device;
+	}
+	input_set_abs_params(input, ABS_MT_POSITION_X, 0, data->max_x, 0, 0);
+	input_set_abs_params(input, ABS_MT_POSITION_Y, 0, data->max_y, 0, 0);
+	input_abs_set_res(input, ABS_MT_POSITION_X, x_res);
+	input_abs_set_res(input, ABS_MT_POSITION_Y, y_res);
+	input_set_abs_params(input, ABS_MT_PRESSURE, 0,
+				ETP_MAX_PRESSURE, 0, 0);
+	input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0,
+		ETP_FINGER_WIDTH * max(data->width_x, data->width_y), 0, 0);
+	input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0,
+		ETP_FINGER_WIDTH * min(data->width_x, data->width_y), 0, 0);
+
+	/* Register the device in input subsystem */
+	ret = input_register_device(input);
+	if (ret) {
+		dev_err(&client->dev, "input device register failed, %d\n",
+			ret);
+		goto err_free_device;
+	}
+
+	return 0;
+
+err_free_device:
+	input_free_device(input);
+	return ret;
+}
+
+static u8 elan_check_adapter_functionality(struct i2c_client *client)
+{
+	u8 ret = ELAN_ADAPTER_FUNC_NONE;
+
+	if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		ret |= ELAN_ADAPTER_FUNC_I2C;
+	return ret;
+}
+
+static int elan_probe(struct i2c_client *client,
+				    const struct i2c_device_id *dev_id)
+{
+	struct elan_tp_data *data;
+	int ret;
+	u8 adapter_func;
+	struct device *dev = &client->dev;
+
+	adapter_func = elan_check_adapter_functionality(client);
+	if (adapter_func == ELAN_ADAPTER_FUNC_NONE) {
+		dev_err(dev, "not a supported I2C adapter\n");
+		return -EIO;
+	}
+
+	data = kzalloc(sizeof(struct elan_tp_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->client = client;
+	data->updated_fw = false;
+	data->enable_detail_info = false;
+	data->adapter_func = adapter_func;
+	data->irq = client->irq;
+
+	ret = elan_i2c_initialize(client);
+	if (ret < 0)
+		goto err_init;
+
+	ret = elan_input_dev_create(data);
+	if (ret < 0)
+		goto err_input_dev;
+
+	if (elan_dbfs_init(data)) {
+		dev_err(&client->dev, "error create elan debugfs.\n");
+		goto err_input_dev;
+	}
+	ret = request_threaded_irq(client->irq, NULL, elan_isr,
+				  IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+				  client->name, data);
+	if (ret < 0) {
+		dev_err(&client->dev, "cannot register irq=%d\n",
+							 client->irq);
+		goto err_irq;
+	}
+
+	ret = elan_i2c_enable_absolute_mode(client);
+	if (ret < 0) {
+		dev_err(&client->dev, "cannot switch to abs mode.\n");
+		goto err_switch_mode;
+	}
+
+	ret = elan_i2c_wake_up(client);
+	if (ret < 0) {
+		dev_err(&client->dev, "device wake up failed.\n");
+		goto err_switch_mode;
+	}
+
+	device_init_wakeup(&client->dev, 1);
+	ret = sysfs_create_group(&client->dev.kobj, &elan_sysfs_group);
+	if (ret < 0) {
+		dev_err(&client->dev, "cannot register dev attribute %d", ret);
+		goto err_switch_mode;
+	}
+	i2c_set_clientdata(client, data);
+	return 0;
+
+err_switch_mode:
+	free_irq(data->irq, data);
+err_irq:
+	input_free_device(data->input);
+	debugfs_remove_recursive(data->dbfs_root);
+	mutex_destroy(&data->dbfs_mutex);
+err_input_dev:
+	kfree(data);
+err_init:
+	dev_err(&client->dev, "Elan Trackpad probe fail!\n");
+	return ret;
+}
+
+static int elan_remove(struct i2c_client *client)
+{
+	struct elan_tp_data *data = i2c_get_clientdata(client);
+
+	free_irq(data->irq, data);
+	debugfs_remove_recursive(data->dbfs_root);
+	mutex_destroy(&data->dbfs_mutex);
+
+	input_unregister_device(data->input);
+	kfree(data);
+	sysfs_remove_group(&client->dev.kobj, &elan_sysfs_group);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int elan_suspend(struct device *dev)
+{
+	int ret;
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+
+	disable_irq(data->irq);
+	ret = elan_i2c_sleep(data->client);
+
+	if (ret < 0) {
+		dev_err(dev, "suspend mode failed, %d\n", ret);
+	} else {
+		if (device_may_wakeup(dev))
+			data->irq_wake = (enable_irq_wake(data->irq) == 0);
+	}
+	return 0;
+}
+
+static int elan_resume(struct device *dev)
+{
+	int ret = 0;
+	struct elan_tp_data *data = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev) && data->irq_wake)
+		disable_irq_wake(data->irq);
+
+	ret = elan_i2c_reinitialize(data->client);
+
+	if (ret < 0)
+		dev_err(dev, "resume active power failed, %d\n", ret);
+
+	enable_irq(data->irq);
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(elan_pm_ops, elan_suspend, elan_resume);
+
+static const struct i2c_device_id elan_id[] = {
+	{ DRIVER_NAME, 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, elan_id);
+
+static struct i2c_driver elan_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+		.pm	= &elan_pm_ops,
+	},
+	.probe		= elan_probe,
+	.remove		= elan_remove,
+	.id_table	= elan_id,
+};
+
+
+static int __init elan_init(void)
+{
+	int ret;
+	ret = i2c_add_driver(&elan_driver);
+	if (ret) {
+		pr_err("elan driver register FAILED.\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static void __exit elan_exit(void)
+{
+	i2c_del_driver(&elan_driver);
+}
+
+module_init(elan_init);
+module_exit(elan_exit);
+
+MODULE_AUTHOR("Duson Lin <dusonlin@emc.com.tw>");
+MODULE_DESCRIPTION("Elan I2C Touchpad driver");
+MODULE_LICENSE("GPL");
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.
From: Lothar Waßmann @ 2013-10-23  7:25 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Mark Rutland, devicetree, Sascha Hauer, Pawel Moll,
	Stephen Warren, Ian Campbell, Rob Herring, Denis Carikli,
	Eric Bénard, linux-input, linux-arm-kernel
In-Reply-To: <20131022223504.GA19819@core.coreip.homeip.net>

Hi,

> On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote:
> > Hi,
> > 
> > > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> > > >  
> > > > +	if (ts->of)
> > > > +		return tsc2007_get_pendown_state_dt(ts);
> > > > +
> > > >  	if (!ts->get_pendown_state)
> > > >  		return true;
> > > 
> > > Instead of special casing "if (ts->of)" all over the place why don't you
> > > set up the device structure as:
> > > 
> > > 	if (<configuring_tsc2007_form_dt>)
> > > 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> > > 
> > > and be done with it?
> > >
> > I also thought about that, but the existing function does not have any
> > parameters, while the DT version of get_pendown_state() requires to get
> > the GPIO passed to it somehow.
> 
> You can always have tsc2007_get_pendown_state_platform() wrapping the
>
Yes, but how would you pass the GPIO number to the
get_pendown_state_dt() function? Global variables are just ugly.

> call. Or we just go and fix board code.
>
That's IMO a better option, but not easy to achieve without breaking
anything. The in-kernel platforms would be easy to fix, but there may be
external users that still use the old hook, so you can't just remove it
or change its semantics.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.
From: Dmitry Torokhov @ 2013-10-23  7:53 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Mark Rutland, devicetree, Sascha Hauer, Pawel Moll,
	Stephen Warren, Ian Campbell, Rob Herring, Denis Carikli,
	Eric Bénard, linux-input, linux-arm-kernel
In-Reply-To: <20131023092553.6989a592@ipc1.ka-ro>

On Wed, Oct 23, 2013 at 09:25:53AM +0200, Lothar Waßmann wrote:
> Hi,
> 
> > On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote:
> > > Hi,
> > > 
> > > > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> > > > >  
> > > > > +	if (ts->of)
> > > > > +		return tsc2007_get_pendown_state_dt(ts);
> > > > > +
> > > > >  	if (!ts->get_pendown_state)
> > > > >  		return true;
> > > > 
> > > > Instead of special casing "if (ts->of)" all over the place why don't you
> > > > set up the device structure as:
> > > > 
> > > > 	if (<configuring_tsc2007_form_dt>)
> > > > 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> > > > 
> > > > and be done with it?
> > > >
> > > I also thought about that, but the existing function does not have any
> > > parameters, while the DT version of get_pendown_state() requires to get
> > > the GPIO passed to it somehow.
> > 
> > You can always have tsc2007_get_pendown_state_platform() wrapping the
> >
> Yes, but how would you pass the GPIO number to the
> get_pendown_state_dt() function? Global variables are just ugly.

You'd have

	get_pendown_state_dt(struct tsc *)
	get_pendown_state_platform(struct tsc *)
	{
		return ts->get_platform_pendown_state();
	}

and youd'd have

struct tsc {
	...
	bool (*get_pendown_state)(struct tsc);
	bool (*get_platform_pendown_state)(void);

> 
> > call. Or we just go and fix board code.
> >
> That's IMO a better option, but not easy to achieve without breaking
> anything. The in-kernel platforms would be easy to fix, but there may be
> external users that still use the old hook, so you can't just remove it
> or change its semantics.

Sure can - they are out of tree after all.

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

^ permalink raw reply

* Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.
From: Thierry Reding @ 2013-10-23  8:21 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Lothar Waßmann, Mark Rutland, devicetree, Sascha Hauer,
	Pawel Moll, Stephen Warren, Ian Campbell, Rob Herring,
	Denis Carikli, Eric Bénard, linux-input, linux-arm-kernel
In-Reply-To: <20131022223504.GA19819@core.coreip.homeip.net>


[-- Attachment #1.1: Type: text/plain, Size: 1586 bytes --]

On Tue, Oct 22, 2013 at 03:35:05PM -0700, Dmitry Torokhov wrote:
> On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote:
> > Hi,
> > 
> > > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> > > >  
> > > > +	if (ts->of)
> > > > +		return tsc2007_get_pendown_state_dt(ts);
> > > > +
> > > >  	if (!ts->get_pendown_state)
> > > >  		return true;
> > > 
> > > Instead of special casing "if (ts->of)" all over the place why don't you
> > > set up the device structure as:
> > > 
> > > 	if (<configuring_tsc2007_form_dt>)
> > > 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> > > 
> > > and be done with it?
> > >
> > I also thought about that, but the existing function does not have any
> > parameters, while the DT version of get_pendown_state() requires to get
> > the GPIO passed to it somehow.
> 
> You can always have tsc2007_get_pendown_state_platform() wrapping the
> call. Or we just go and fix board code.

I used to have a patch which did exactly that but never got around to
submitting it. Essentially what it did was add a void * parameter to the
.get_pendown_state() and .clear_penirq() callbacks, along with a new
.callback_data field that the driver could set. At the same time there
was some code to unify code for boards that merely use a simple GPIO as
pendown.

I'm attaching what I think is the latest version. I no longer have
access to the hardware so I can't test this, but perhaps it can serve as
an example of how this could work. Sorry this isn't in the form of a
proper patch.

Thierry

[-- Attachment #1.2: tsc2007.patch --]
[-- Type: text/x-diff, Size: 10216 bytes --]

diff --git a/arch/arm/mach-imx/mach-cpuimx35.c b/arch/arm/mach-imx/mach-cpuimx35.c
index d49b0ec..0dd8381 100644
--- a/arch/arm/mach-imx/mach-cpuimx35.c
+++ b/arch/arm/mach-imx/mach-cpuimx35.c
@@ -62,6 +62,7 @@ static int tsc2007_get_pendown_state(void)
 static struct tsc2007_platform_data tsc2007_info = {
 	.model			= 2007,
 	.x_plate_ohms		= 180,
+	.pendown_gpio		= -1,
 	.get_pendown_state = tsc2007_get_pendown_state,
 };
 
diff --git a/arch/arm/mach-imx/mach-cpuimx51sd.c b/arch/arm/mach-imx/mach-cpuimx51sd.c
index b87cc49..ef2a7e6 100644
--- a/arch/arm/mach-imx/mach-cpuimx51sd.c
+++ b/arch/arm/mach-imx/mach-cpuimx51sd.c
@@ -134,6 +134,7 @@ static int tsc2007_get_pendown_state(void)
 static struct tsc2007_platform_data tsc2007_info = {
 	.model			= 2007,
 	.x_plate_ohms		= 180,
+	.pendown_gpio		= -1,
 	.get_pendown_state	= tsc2007_get_pendown_state,
 };
 
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index b85957a..69abf30 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -1199,6 +1199,7 @@ static int ts_init(void)
 static struct tsc2007_platform_data tsc2007_info = {
 	.model			= 2007,
 	.x_plate_ohms		= 180,
+	.pendown_gpio		= -1,
 	.get_pendown_state	= ts_get_pendown_state,
 	.init_platform_hw	= ts_init,
 };
diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 64559e8a..6cfd0ef 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -517,6 +517,7 @@ static int ts_init(void)
 static struct tsc2007_platform_data tsc2007_info = {
 	.model			= 2007,
 	.x_plate_ohms		= 180,
+	.pendown_gpio		= -1,
 	.get_pendown_state	= ts_get_pendown_state,
 	.init_platform_hw	= ts_init,
 };
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 1473d23..c87fdac 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -20,10 +20,15 @@
  *  published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) "tsc2007: " fmt
+
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
 #include <linux/i2c.h>
 #include <linux/i2c/tsc2007.h>
 
@@ -75,13 +80,16 @@ struct tsc2007 {
 	unsigned long		poll_delay;
 	unsigned long		poll_period;
 
+	int			pendown_gpio;
+	int			active_low;
 	int			irq;
 
 	wait_queue_head_t	wait;
 	bool			stopped;
 
-	int			(*get_pendown_state)(void);
-	void			(*clear_penirq)(void);
+	void			*callback_data;
+	int			(*get_pendown_state)(void *data);
+	void			(*clear_penirq)(void *data);
 };
 
 static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
@@ -161,7 +169,7 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 	if (!ts->get_pendown_state)
 		return true;
 
-	return ts->get_pendown_state();
+	return ts->get_pendown_state(ts->callback_data);
 }
 
 static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
@@ -171,6 +179,13 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	struct ts_event tc;
 	u32 rt;
 
+	/*
+	 * With some panels we need to wait a bit otherwise the first value
+	 * is often wrong.
+	 */
+	if (ts->poll_delay > 0)
+		msleep(ts->poll_delay);
+
 	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
 
 		/* pen is down, continue with the measurement */
@@ -219,7 +234,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	input_sync(input);
 
 	if (ts->clear_penirq)
-		ts->clear_penirq();
+		ts->clear_penirq(ts->callback_data);
 
 	return IRQ_HANDLED;
 }
@@ -228,11 +243,11 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
 {
 	struct tsc2007 *ts = handle;
 
-	if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
+	if (!ts->get_pendown_state || likely(ts->get_pendown_state(ts->callback_data)))
 		return IRQ_WAKE_THREAD;
 
 	if (ts->clear_penirq)
-		ts->clear_penirq();
+		ts->clear_penirq(ts->callback_data);
 
 	return IRQ_HANDLED;
 }
@@ -273,6 +288,75 @@ static void tsc2007_close(struct input_dev *input_dev)
 	tsc2007_stop(ts);
 }
 
+static int tsc2007_get_pendown_state(void *data)
+{
+	struct tsc2007 *ts = data;
+	int ret = 0;
+
+	ret = !!gpio_get_value(ts->pendown_gpio);
+	if (ts->active_low)
+		ret = !ret;
+
+	return ret;
+}
+
+static struct tsc2007_platform_data *tsc2007_parse_dt(struct device *dev)
+{
+	struct tsc2007_platform_data *pdata;
+	enum of_gpio_flags flags;
+	u32 value, values[3];
+	int err;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	err = of_get_named_gpio_flags(dev->of_node, "pendown-gpios", 0,
+				      &flags);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->pendown_gpio = err;
+
+	if (flags & OF_GPIO_ACTIVE_LOW)
+		pdata->active_low = true;
+
+	err = of_property_read_u32(dev->of_node, "x-plate-ohms", &value);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->x_plate_ohms = value;
+
+	err = of_property_read_u32(dev->of_node, "max-rt", &value);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->max_rt = value;
+
+	err = of_property_read_u32(dev->of_node, "poll-delay", &value);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->poll_delay = value;
+
+	err = of_property_read_u32(dev->of_node, "poll-period", &value);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->poll_period = value;
+
+	err = of_property_read_u32_array(dev->of_node, "fuzz", values,
+					 ARRAY_SIZE(values));
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->fuzzx = values[0];
+	pdata->fuzzy = values[1];
+	pdata->fuzzz = values[2];
+
+	return pdata;
+}
+
 static int __devinit tsc2007_probe(struct i2c_client *client,
 				   const struct i2c_device_id *id)
 {
@@ -281,18 +365,42 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	struct input_dev *input_dev;
 	int err;
 
+	ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) {
+		client->irq = irq_of_parse_and_map(client->dev.of_node, 0);
+		if (!client->irq) {
+			err = -EPROBE_DEFER;
+			goto err_free_mem;
+		}
+	}
+
 	if (!pdata) {
-		dev_err(&client->dev, "platform data is required!\n");
-		return -EINVAL;
+		if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) {
+			pdata = tsc2007_parse_dt(&client->dev);
+			if (IS_ERR(pdata)) {
+				err = PTR_ERR(pdata);
+				goto err_free_mem;
+			}
+
+			pdata->callback_data = ts;
+		} else {
+			dev_err(&client->dev, "platform data is required!\n");
+			err = -EINVAL;
+			goto err_free_mem;
+		}
 	}
 
 	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EIO;
+				     I2C_FUNC_SMBUS_READ_WORD_DATA)) {
+		err = -EIO;
+		goto err_free_mem;
+	}
 
-	ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
 	input_dev = input_allocate_device();
-	if (!ts || !input_dev) {
+	if (!input_dev) {
 		err = -ENOMEM;
 		goto err_free_mem;
 	}
@@ -307,13 +415,27 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	ts->max_rt            = pdata->max_rt ? : MAX_12BIT;
 	ts->poll_delay        = pdata->poll_delay ? : 1;
 	ts->poll_period       = pdata->poll_period ? : 1;
+	ts->callback_data     = pdata->callback_data;
 	ts->get_pendown_state = pdata->get_pendown_state;
 	ts->clear_penirq      = pdata->clear_penirq;
 
 	if (pdata->x_plate_ohms == 0) {
 		dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
 		err = -EINVAL;
-		goto err_free_mem;
+		goto err_free_dev;
+	}
+
+	if (gpio_is_valid(pdata->pendown_gpio)) {
+		err = gpio_request_one(pdata->pendown_gpio, GPIOF_IN,
+				       "tsc2007");
+		if (err < 0)
+			goto err_free_dev;
+
+		ts->get_pendown_state = tsc2007_get_pendown_state;
+		ts->pendown_gpio = pdata->pendown_gpio;
+		ts->active_low = pdata->active_low;
+	} else {
+		ts->pendown_gpio = -EINVAL;
 	}
 
 	snprintf(ts->phys, sizeof(ts->phys),
@@ -343,7 +465,7 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 				   IRQF_ONESHOT, client->dev.driver->name, ts);
 	if (err < 0) {
 		dev_err(&client->dev, "irq %d busy?\n", ts->irq);
-		goto err_free_mem;
+		goto err_free_gpio;
 	}
 
 	tsc2007_stop(ts);
@@ -360,8 +482,12 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	free_irq(ts->irq, ts);
 	if (pdata->exit_platform_hw)
 		pdata->exit_platform_hw();
- err_free_mem:
+ err_free_gpio:
+	if (gpio_is_valid(pdata->pendown_gpio))
+		gpio_free(pdata->pendown_gpio);
+ err_free_dev:
 	input_free_device(input_dev);
+ err_free_mem:
 	kfree(ts);
 	return err;
 }
@@ -373,6 +499,9 @@ static int __devexit tsc2007_remove(struct i2c_client *client)
 
 	free_irq(ts->irq, ts);
 
+	if (gpio_is_valid(ts->pendown_gpio))
+		gpio_free(ts->pendown_gpio);
+
 	if (pdata->exit_platform_hw)
 		pdata->exit_platform_hw();
 
diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
index a447f4e..249d307 100644
--- a/drivers/mfd/timberdale.c
+++ b/drivers/mfd/timberdale.c
@@ -64,7 +64,8 @@ struct timberdale_device {
 
 static struct tsc2007_platform_data timberdale_tsc2007_platform_data = {
 	.model = 2003,
-	.x_plate_ohms = 100
+	.x_plate_ohms = 100,
+	.pendown_gpio = -1,
 };
 
 static struct i2c_board_info timberdale_i2c_board_info[] = {
diff --git a/include/linux/i2c/tsc2007.h b/include/linux/i2c/tsc2007.h
index 506a9f7..8d72771 100644
--- a/include/linux/i2c/tsc2007.h
+++ b/include/linux/i2c/tsc2007.h
@@ -14,8 +14,12 @@ struct tsc2007_platform_data {
 	int	fuzzy;
 	int	fuzzz;
 
-	int	(*get_pendown_state)(void);
-	void	(*clear_penirq)(void);		/* If needed, clear 2nd level
+	int	pendown_gpio;
+	bool	active_low;
+
+	void	*callback_data;
+	int	(*get_pendown_state)(void *data);
+	void	(*clear_penirq)(void *data);	/* If needed, clear 2nd level
 						   interrupt source */
 	int	(*init_platform_hw)(void);
 	void	(*exit_platform_hw)(void);

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related

* [PATCHv5][ 1/4] Input: tsc2007: Add device tree support.
From: Denis Carikli @ 2013-10-23 12:10 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Thierry Reding, Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
	Pawel Moll, Denis Carikli, Rob Herring, Stephen Warren,
	Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Lothar Waßmann

Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
---
ChangeLog v4->v5:
- Most of the "if (ts->of)" were replaced by wrapping them in the
  tsc2007_is_pen_down_valid and tsc2007_is_pen_down functions.
- Some whitespace cleanups.
- The devm_kzalloc call was fixed to make it compile.
---
 .../bindings/input/touchscreen/tsc2007.txt         |   44 +++++
 drivers/input/touchscreen/tsc2007.c                |  194 +++++++++++++++-----
 2 files changed, 197 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
new file mode 100644
index 0000000..fadd3f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
@@ -0,0 +1,44 @@
+* Texas Instruments tsc2007 touchscreen controller
+
+Required properties:
+- compatible: must be "ti,tsc2007".
+- reg: I2C address of the chip.
+- pinctrl-0: Should specify pin control groups used for this controller
+  (see pinctrl bindings[0]).
+- pinctrl-names: Should contain only one value - "default"
+  (see pinctrl bindings[0]).
+- interrupt-parent: the phandle for the interrupt controller
+  (see interrupt binding[1]).
+- interrupts: interrupt to which the chip is connected
+  (see interrupt binding[1]).
+- ti,x-plate-ohms: X-plate resistance in ohms.
+
+Optional properties:
+- gpios: the interrupt gpio the chip is connected to (trough the penirq pin)
+  (see GPIO binding[2] for more details).
+- max-rt: maximum pressure.
+- fuzzy: specifies the fuzz value that is used to filter noise from the event
+  stream.
+- poll-period: how much time to wait(in millisecond) before reading again the
+  values from the tsc2007.
+
+[0]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+[1]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[2]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+	&i2c1 {
+		/* ... */
+		tsc2007@49 {
+			compatible = "ti,tsc2007";
+			reg = <0x49>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_tsc2007_1>;
+			interrupt-parent = <&gpio4>;
+			interrupts = <0x0 0x8>;
+			gpios = <&gpio4 0 0>;
+			ti,x-plate-ohms = <180>;
+		};
+
+		/* ... */
+	};
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 0b67ba4..0625fe1 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -26,6 +26,9 @@
 #include <linux/interrupt.h>
 #include <linux/i2c.h>
 #include <linux/i2c/tsc2007.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #define TSC2007_MEASURE_TEMP0		(0x0 << 4)
 #define TSC2007_MEASURE_AUX		(0x2 << 4)
@@ -74,7 +77,10 @@ struct tsc2007 {
 	u16			max_rt;
 	unsigned long		poll_delay;
 	unsigned long		poll_period;
+	int			fuzzy;
+	char			of;
 
+	unsigned		gpio;
 	int			irq;
 
 	wait_queue_head_t	wait;
@@ -84,6 +90,11 @@ struct tsc2007 {
 	void			(*clear_penirq)(void);
 };
 
+static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
+{
+	return !gpio_get_value(ts->gpio);
+}
+
 static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
 {
 	s32 data;
@@ -142,6 +153,14 @@ static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
 	return rt;
 }
 
+static bool tsc2007_is_pen_down_valid(struct tsc2007 *ts)
+{
+	if (ts->of)
+		return gpio_is_valid(ts->gpio);
+	else
+		return ts->get_pendown_state ? true : false;
+}
+
 static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 {
 	/*
@@ -158,10 +177,13 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 	 * to fall back on the pressure reading.
 	 */
 
-	if (!ts->get_pendown_state)
+	if (!tsc2007_is_pen_down_valid(ts))
 		return true;
 
-	return ts->get_pendown_state();
+	if (ts->of)
+		return tsc2007_get_pendown_state_dt(ts);
+	else
+		return ts->get_pendown_state();
 }
 
 static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
@@ -178,7 +200,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 
 		rt = tsc2007_calculate_pressure(ts, &tc);
 
-		if (rt == 0 && !ts->get_pendown_state) {
+		if(!rt && !tsc2007_is_pen_down_valid(ts)) {
 			/*
 			 * If pressure reported is 0 and we don't have
 			 * callback to check pendown state, we have to
@@ -228,7 +250,7 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
 {
 	struct tsc2007 *ts = handle;
 
-	if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
+	if (tsc2007_is_pen_down(ts))
 		return IRQ_WAKE_THREAD;
 
 	if (ts->clear_penirq)
@@ -273,34 +295,65 @@ static void tsc2007_close(struct input_dev *input_dev)
 	tsc2007_stop(ts);
 }
 
-static int tsc2007_probe(struct i2c_client *client,
-				   const struct i2c_device_id *id)
+#ifdef CONFIG_OF
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+			    struct device_node *np)
 {
-	struct tsc2007 *ts;
-	struct tsc2007_platform_data *pdata = client->dev.platform_data;
-	struct input_dev *input_dev;
-	int err;
-
-	if (!pdata) {
-		dev_err(&client->dev, "platform data is required!\n");
+	int err = 0;
+	u32 val32;
+	u64 val64;
+
+	if (!of_property_read_u32(np, "max-rt", &val32))
+		ts->max_rt = val32;
+	else
+		ts->max_rt = MAX_12BIT;
+
+	if (!of_property_read_u32(np, "fuzzy", &val32))
+		ts->fuzzy = val32;
+
+	if (!of_property_read_u64(np, "poll-period", &val64))
+		ts->poll_period = val64;
+	else
+		ts->poll_period = 1;
+
+	if (!of_property_read_u32(np, "ti,x-plate-ohms", &val32)) {
+		ts->x_plate_ohms = val32;
+	} else {
+		dev_err(&client->dev,
+			"Error: lacking ti,x-plate-ohms devicetree property. (err %d).",
+			err);
 		return -EINVAL;
 	}
 
-	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EIO;
+	ts->gpio = of_get_gpio(np, 0);
+	if (!gpio_is_valid(ts->gpio))
+		dev_err(&client->dev,
+			"GPIO not found (of_get_gpio returned %d)\n",
+			ts->gpio);
 
-	ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!ts || !input_dev) {
-		err = -ENOMEM;
-		goto err_free_mem;
-	}
+	/* Used to detect if it is probed trough the device tree,
+	 * in order to be able to use that information in the IRQ handler.
+	 */
+	ts->of = 1;
 
-	ts->client = client;
-	ts->irq = client->irq;
-	ts->input = input_dev;
-	init_waitqueue_head(&ts->wait);
+	return 0;
+}
+#else
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+			    struct device_node *np)
+{
+	return -ENODEV;
+}
+#endif
+
+static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
+			      struct tsc2007_platform_data *pdata,
+			      const struct i2c_device_id *id)
+{
+	if (!pdata) {
+		dev_err(&client->dev, "platform data is required!\n");
+		return -EINVAL;
+	}
 
 	ts->model             = pdata->model;
 	ts->x_plate_ohms      = pdata->x_plate_ohms;
@@ -309,13 +362,57 @@ static int tsc2007_probe(struct i2c_client *client,
 	ts->poll_period       = pdata->poll_period ? : 1;
 	ts->get_pendown_state = pdata->get_pendown_state;
 	ts->clear_penirq      = pdata->clear_penirq;
+	ts->fuzzy             = pdata->fuzzy;
 
 	if (pdata->x_plate_ohms == 0) {
 		dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
-		err = -EINVAL;
-		goto err_free_mem;
+		return -EINVAL;
 	}
 
+	/* Used to detect if it is probed trough the device tree,
+	 * in order to be able to use that information in the IRQ handler.
+	 */
+	ts->of = 0;
+
+	return 0;
+}
+
+static int tsc2007_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct device_node *np = client->dev.of_node;
+	struct tsc2007_platform_data *pdata = client->dev.platform_data;
+	struct tsc2007 *ts;
+	struct input_dev *input_dev;
+	int err = 0;
+
+	ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	if (np)
+		err = tsc2007_probe_dt(client, ts, np);
+	else
+		err = tsc2007_probe_pdev(client, ts, pdata, id);
+
+	if (err)
+		return err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -EIO;
+
+	input_dev = input_allocate_device();
+	if (!input_dev) {
+		err = -ENOMEM;
+		goto err_free_input;
+	};
+
+	ts->client = client;
+	ts->irq = client->irq;
+	ts->input = input_dev;
+	init_waitqueue_head(&ts->wait);
+
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));
 
@@ -331,19 +428,21 @@ static int tsc2007_probe(struct i2c_client *client,
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
-	input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, pdata->fuzzx, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, pdata->fuzzy, 0);
+	input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, ts->fuzzy, 0);
+	input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, ts->fuzzy, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
-			pdata->fuzzz, 0);
+			     ts->fuzzy, 0);
 
-	if (pdata->init_platform_hw)
-		pdata->init_platform_hw();
+	if (!np) {
+		if (pdata->init_platform_hw)
+			pdata->init_platform_hw();
+	}
 
 	err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
 				   IRQF_ONESHOT, client->dev.driver->name, ts);
 	if (err < 0) {
 		dev_err(&client->dev, "irq %d busy?\n", ts->irq);
-		goto err_free_mem;
+		goto err_free_input;
 	}
 
 	tsc2007_stop(ts);
@@ -358,23 +457,27 @@ static int tsc2007_probe(struct i2c_client *client,
 
  err_free_irq:
 	free_irq(ts->irq, ts);
-	if (pdata->exit_platform_hw)
-		pdata->exit_platform_hw();
- err_free_mem:
+	if (!np) {
+		if (pdata->exit_platform_hw)
+			pdata->exit_platform_hw();
+	}
+ err_free_input:
 	input_free_device(input_dev);
-	kfree(ts);
 	return err;
 }
 
 static int tsc2007_remove(struct i2c_client *client)
 {
+	struct device_node *np = client->dev.of_node;
 	struct tsc2007	*ts = i2c_get_clientdata(client);
 	struct tsc2007_platform_data *pdata = client->dev.platform_data;
 
 	free_irq(ts->irq, ts);
 
-	if (pdata->exit_platform_hw)
-		pdata->exit_platform_hw();
+	if (!np) {
+		if (pdata->exit_platform_hw)
+			pdata->exit_platform_hw();
+	}
 
 	input_unregister_device(ts->input);
 	kfree(ts);
@@ -389,10 +492,19 @@ static const struct i2c_device_id tsc2007_idtable[] = {
 
 MODULE_DEVICE_TABLE(i2c, tsc2007_idtable);
 
+#ifdef CONFIG_OF
+static const struct of_device_id tsc2007_of_match[] = {
+	{ .compatible = "ti,tsc2007" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tsc2007_of_match);
+#endif
+
 static struct i2c_driver tsc2007_driver = {
 	.driver = {
 		.owner	= THIS_MODULE,
-		.name	= "tsc2007"
+		.name	= "tsc2007",
+		.of_match_table = of_match_ptr(tsc2007_of_match),
 	},
 	.id_table	= tsc2007_idtable,
 	.probe		= tsc2007_probe,
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCHv5][ 2/4] ARM: dts: cpuimx51 Add touchscreen support.
From: Denis Carikli @ 2013-10-23 12:10 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Thierry Reding, Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
	Pawel Moll, Denis Carikli, Rob Herring, Stephen Warren,
	Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Lothar Waßmann
In-Reply-To: <1382530220-27881-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>

Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
index 8638656..34ca8d3a 100644
--- a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
+++ b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
@@ -42,6 +42,17 @@
 		compatible = "nxp,pcf8563";
 		reg = <0x51>;
 	};
+
+	tsc2007: tsc2007@49 {
+		compatible = "ti,tsc2007";
+		reg = <0x49>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_tsc2007_1>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <0x0 0x8>;
+		gpios = <&gpio4 0 0>;
+		ti,x-plate-ohms = <180>;
+	};
 };
 
 &iomuxc {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCHv5][ 3/4] ARM: dts: cpuimx35 Add touchscreen support.
From: Denis Carikli @ 2013-10-23 12:10 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Thierry Reding, Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
	Pawel Moll, Denis Carikli, Rob Herring, Stephen Warren,
	Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Lothar Waßmann
In-Reply-To: <1382530220-27881-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>

Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
index 2c2d4cd..a22230b 100644
--- a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
+++ b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
@@ -41,6 +41,17 @@
 		compatible = "nxp,pcf8563";
 		reg = <0x51>;
 	};
+
+	tsc2007: tsc2007@48 {
+		compatible = "ti,tsc2007";
+		reg = <0x48>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_tsc2007_1>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <0x2 0x8>;
+		gpios = <&gpio3 2 0>;
+		ti,x-plate-ohms = <180>;
+	};
 };
 
 &iomuxc {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCHv5][ 4/4] ARM: imx_v6_v7_defconfig: Enable tsc2007 support.
From: Denis Carikli @ 2013-10-23 12:10 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Thierry Reding, Shawn Guo, Eric Bénard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland,
	Pawel Moll, Denis Carikli, Rob Herring, Stephen Warren,
	Ian Campbell, devicetree-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Lothar Waßmann,
	Fabio Estevam
In-Reply-To: <1382530220-27881-1-git-send-email-denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>

The eukrea cpuimx35 and cpuimx51 have a tsc2007 touchscreen controller,
  so we turn it on.

Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Lothar Waßmann <LW-bxm8fMRDkQLDiMYJYoSAnRvVK+yQ3ZXh@public.gmane.org>
Cc: Fabio Estevam <fabio.estevam-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Cc: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/configs/imx_v6_v7_defconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index d8a52a0..23ba17d 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -130,6 +130,7 @@ CONFIG_MOUSE_PS2_ELANTECH=y
 CONFIG_INPUT_TOUCHSCREEN=y
 CONFIG_TOUCHSCREEN_EGALAX=y
 CONFIG_TOUCHSCREEN_MC13783=y
+CONFIG_TOUCHSCREEN_TSC2007=y
 CONFIG_INPUT_MISC=y
 CONFIG_INPUT_MMA8450=y
 CONFIG_SERIO_SERPORT=m
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v4 1/1] Input: Improve the performance of alps v5-protocol's touchpad
From: Tommy Will @ 2013-10-23 13:56 UTC (permalink / raw)
  To: Justin Clift
  Cc: Dmitry Torokhov, Kevin Cernekee, david turvene, linux-input,
	Niels de Vos, Yunkang Tang
In-Reply-To: <20131023122954.2d63b0e85fd963dabde574f4@redhat.com>

Hi Justin, Niels & All Fedora Community members,

Thank you so much for your help !

Best Regards
Tommy Will

2013/10/23 Justin Clift <jclift@redhat.com>:
> On Mon, 21 Oct 2013 19:41:50 +0800
> Yunkang Tang <tommywill2011@gmail.com> wrote:
>> Hi all,
>>
>> Here is the 4th version of supporting ALPS v5 protocol's device.
>
> Niels de Vos, CC'd, was kind enough to build a Fedora 19 kernel
> with this v4 patch added.  We've been testing it since yesterday.
>
> This patch has been successfully tested on:
>
>  * Dell Inspiron 17R SE / Inspiron 7720
>  * Dell Vostro 3360
>  * Fujitsu Lifebook AH532
>
> With the v4 patch, the touchpad:
>
> * is recognised as a touchpad
> * both KDE and Gnome can control it
> * Both hardware buttons (left/right) work
> * Both 1 & 2 finger scrolling works
>
> So, it seems good here. :)
>
> From my point of view, I would be happy to see this
> patch applied, and you're welcome to use:
>
>   Tested-by: Justin Clift <jclift@redhat.com>
>
> Side note - Many thanks to the Fedora Community members
> who tested this v4 patch on their hardware and reported
> back. :)
>
> * Özgür Gündoğan
> * Stanislav Datskevich
> * Thane
> * Alexander Volovics
>
> Regards and best wishes,
>
> Justin Clift
>
> --
> Justin Clift <jclift@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 1/1] Input: Improve the performance of alps v5-protocol's touchpad
From: Justin Clift @ 2013-10-23 11:29 UTC (permalink / raw)
  To: Yunkang Tang
  Cc: dmitry.torokhov, cernekee, dturvene, linux-input, ndevos,
	yunkang.tang
In-Reply-To: <1382355711-3211-1-git-send-email-yunkang.tang@cn.alps.com>

On Mon, 21 Oct 2013 19:41:50 +0800
Yunkang Tang <tommywill2011@gmail.com> wrote:
> Hi all,
> 
> Here is the 4th version of supporting ALPS v5 protocol's device.

Niels de Vos, CC'd, was kind enough to build a Fedora 19 kernel
with this v4 patch added.  We've been testing it since yesterday.

This patch has been successfully tested on:

 * Dell Inspiron 17R SE / Inspiron 7720
 * Dell Vostro 3360
 * Fujitsu Lifebook AH532

With the v4 patch, the touchpad:

* is recognised as a touchpad
* both KDE and Gnome can control it
* Both hardware buttons (left/right) work
* Both 1 & 2 finger scrolling works

So, it seems good here. :)

From my point of view, I would be happy to see this
patch applied, and you're welcome to use:

  Tested-by: Justin Clift <jclift@redhat.com>

Side note - Many thanks to the Fedora Community members
who tested this v4 patch on their hardware and reported
back. :)

* Özgür Gündoğan
* Stanislav Datskevich
* Thane
* Alexander Volovics

Regards and best wishes,

Justin Clift

-- 
Justin Clift <jclift@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCHv2 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-23 15:01 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Torokhov
  Cc: Grant Likely, Rob Herring, Sebastian Reichel, Peter Ujfalusi,
	Sachin Kamat, linux-input, linux-kernel, devicetree
In-Reply-To: <1382540482-12261-1-git-send-email-sre@debian.org>

Add device tree support for twl4030 power button driver.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 drivers/input/misc/twl4030-pwrbutton.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index b9a05fd..a3a0fe3 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -52,7 +52,7 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr)
 	return IRQ_HANDLED;
 }
 
-static int __init twl4030_pwrbutton_probe(struct platform_device *pdev)
+static int twl4030_pwrbutton_probe(struct platform_device *pdev)
 {
 	struct input_dev *pwr;
 	int irq = platform_get_irq(pdev, 0);
@@ -106,16 +106,24 @@ static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
+       { .compatible = "ti,twl4030-pwrbutton" },
+       {},
+};
+MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
+#endif
+
 static struct platform_driver twl4030_pwrbutton_driver = {
+	.probe		= twl4030_pwrbutton_probe,
 	.remove		= __exit_p(twl4030_pwrbutton_remove),
 	.driver		= {
 		.name	= "twl4030_pwrbutton",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table),
 	},
 };
-
-module_platform_driver_probe(twl4030_pwrbutton_driver,
-			twl4030_pwrbutton_probe);
+module_platform_driver(twl4030_pwrbutton_driver);
 
 MODULE_ALIAS("platform:twl4030_pwrbutton");
 MODULE_DESCRIPTION("Triton2 Power Button");
-- 
1.8.4.rc3

^ permalink raw reply related

* [PATCHv2 2/3] Input: twl4030-pwrbutton: use dev_err for errors
From: Sebastian Reichel @ 2013-10-23 15:01 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Torokhov
  Cc: Grant Likely, Rob Herring, Sebastian Reichel, Peter Ujfalusi,
	Sachin Kamat, linux-input, linux-kernel, devicetree
In-Reply-To: <1382540482-12261-1-git-send-email-sre@debian.org>

Use dev_err() to output errors instead of dev_dbg().

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 drivers/input/misc/twl4030-pwrbutton.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index a3a0fe3..3efbb13 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -60,7 +60,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
 
 	pwr = input_allocate_device();
 	if (!pwr) {
-		dev_dbg(&pdev->dev, "Can't allocate power button\n");
+		dev_err(&pdev->dev, "Can't allocate power button\n");
 		return -ENOMEM;
 	}
 
@@ -74,13 +74,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
 			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
 			"twl4030_pwrbutton", pwr);
 	if (err < 0) {
-		dev_dbg(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
+		dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
 		goto free_input_dev;
 	}
 
 	err = input_register_device(pwr);
 	if (err) {
-		dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
+		dev_err(&pdev->dev, "Can't register power button: %d\n", err);
 		goto free_irq;
 	}
 
-- 
1.8.4.rc3

^ permalink raw reply related

* [PATCHv2 3/3] Input: twl4030-pwrbutton: simplify driver using devm_*
From: Sebastian Reichel @ 2013-10-23 15:01 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Torokhov
  Cc: Grant Likely, Rob Herring, Sebastian Reichel, Peter Ujfalusi,
	Sachin Kamat, linux-input, linux-kernel, devicetree
In-Reply-To: <1382540482-12261-1-git-send-email-sre@debian.org>

Use managed irq resource to simplify the driver.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 drivers/input/misc/twl4030-pwrbutton.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
index 3efbb13..49ca7b1 100644
--- a/drivers/input/misc/twl4030-pwrbutton.c
+++ b/drivers/input/misc/twl4030-pwrbutton.c
@@ -70,7 +70,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
 	pwr->phys = "twl4030_pwrbutton/input0";
 	pwr->dev.parent = &pdev->dev;
 
-	err = request_threaded_irq(irq, NULL, powerbutton_irq,
+	err = devm_request_threaded_irq(&pwr->dev, irq, NULL, powerbutton_irq,
 			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
 			"twl4030_pwrbutton", pwr);
 	if (err < 0) {
@@ -81,15 +81,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
 	err = input_register_device(pwr);
 	if (err) {
 		dev_err(&pdev->dev, "Can't register power button: %d\n", err);
-		goto free_irq;
+		goto free_input_dev;
 	}
 
 	platform_set_drvdata(pdev, pwr);
 
 	return 0;
 
-free_irq:
-	free_irq(irq, pwr);
 free_input_dev:
 	input_free_device(pwr);
 	return err;
@@ -98,9 +96,7 @@ free_input_dev:
 static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
 {
 	struct input_dev *pwr = platform_get_drvdata(pdev);
-	int irq = platform_get_irq(pdev, 0);
 
-	free_irq(irq, pwr);
 	input_unregister_device(pwr);
 
 	return 0;
-- 
1.8.4.rc3

^ permalink raw reply related

* [PATCHv2 0/3] DT Support for TWL4030 power button
From: Sebastian Reichel @ 2013-10-23 15:01 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Torokhov
  Cc: Grant Likely, Rob Herring, Sebastian Reichel, Peter Ujfalusi,
	Sachin Kamat, linux-input, linux-kernel, devicetree

Hi,

This is the second iteration of DT support for the TWL4030
power button.

Changes since v1 [0]:
 * Simplify OF handling of twl4030_pwrbutton_dt_match_table
 * Remove Patch 4 (queued by Benoit)

[0] https://lkml.org/lkml/2013/10/22/151

-- Sebastian

Sebastian Reichel (3):
  Input: twl4030-pwrbutton - add device tree support
  Input: twl4030-pwrbutton: use dev_err for errors
  Input: twl4030-pwrbutton: simplify driver using devm_*

 drivers/input/misc/twl4030-pwrbutton.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

-- 
1.8.4.rc3


^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-pwrbutton - add device tree support
From: Mark Rutland @ 2013-10-23 16:09 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, Dmitry Torokhov, grant.likely@linaro.org,
	rob.herring@calxeda.com, Peter Ujfalusi, Sachin Kamat,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <1382540482-12261-2-git-send-email-sre@debian.org>

On Wed, Oct 23, 2013 at 04:01:20PM +0100, Sebastian Reichel wrote:
> Add device tree support for twl4030 power button driver.

This requires a binding document. As it is it's not possible to review.

Mark.

> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
>  drivers/input/misc/twl4030-pwrbutton.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index b9a05fd..a3a0fe3 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -52,7 +52,7 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr)
>  	return IRQ_HANDLED;
>  }
>  
> -static int __init twl4030_pwrbutton_probe(struct platform_device *pdev)
> +static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  {
>  	struct input_dev *pwr;
>  	int irq = platform_get_irq(pdev, 0);
> @@ -106,16 +106,24 @@ static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
> +       { .compatible = "ti,twl4030-pwrbutton" },

There's no need to shorten this, "ti,twl4030-power-button" would be far easier
to understand. Unless the datasheet refers to it as pwrbutton?

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-23 16:17 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Dmitry Torokhov, grant.likely@linaro.org, rob.herring@calxeda.com,
	Peter Ujfalusi, Sachin Kamat, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20131023160934.GA6042@kartoffel>

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

Hi,

On Wed, Oct 23, 2013 at 05:09:36PM +0100, Mark Rutland wrote:
> On Wed, Oct 23, 2013 at 04:01:20PM +0100, Sebastian Reichel wrote:
> > Add device tree support for twl4030 power button driver.
> 
> This requires a binding document. As it is it's not possible to review.

Right. I will add it and sent a v3.

> > 
> > Signed-off-by: Sebastian Reichel <sre@debian.org>
> > ---
> >  drivers/input/misc/twl4030-pwrbutton.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> > index b9a05fd..a3a0fe3 100644
> > --- a/drivers/input/misc/twl4030-pwrbutton.c
> > +++ b/drivers/input/misc/twl4030-pwrbutton.c
> > @@ -52,7 +52,7 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static int __init twl4030_pwrbutton_probe(struct platform_device *pdev)
> > +static int twl4030_pwrbutton_probe(struct platform_device *pdev)
> >  {
> >  	struct input_dev *pwr;
> >  	int irq = platform_get_irq(pdev, 0);
> > @@ -106,16 +106,24 @@ static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > +#if IS_ENABLED(CONFIG_OF)
> > +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
> > +       { .compatible = "ti,twl4030-pwrbutton" },
> 
> There's no need to shorten this, "ti,twl4030-power-button" would be far easier
> to understand. Unless the datasheet refers to it as pwrbutton?

Yes it's abbreviated in the datasheet.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH V4 0/2] Add DT support for tps6507x touchscreen
From: Manish Badarkhe @ 2013-10-23 16:28 UTC (permalink / raw)
  To: devicetree-discuss, devicetree, linux-input, linux-doc,
	linux-kernel, davinci-linux-open-source
  Cc: grant.likely, dmitry.torokhov, rob.herring, rob, badarkhe.manish

Patch set adds DT support for tps6507x based touchscreen.
Also, add DT data for tps6507x touchscreen in da850-evm by
providing touchscreen node inside tps6507x mfd device.

This patch series applies on top of linux-next tree and depends on [1].

[1]tps6507x-ts: update to devm_* API
   https://patchwork.kernel.org/patch/2324441/

Changes since V3:
 - Rebased on top of Dmitry's changes
 - Removed error handling for optional DT properties

Changes since V2:
 - Updated tps6507x documentation.
 - Removed unnecessary code.

Changes since V1:
 - Updated tps6507x documentation.
 - Updated commit message.
 - return proper error value in absence platform and DT data
   for touchscreen.

Manish Badarkhe (2):
  tps6507x-ts: Add DT support
  ARM: davinci: da850: add tps6507x touchscreen DT data

 Documentation/devicetree/bindings/mfd/tps6507x.txt |   24 +++++-
 arch/arm/boot/dts/da850-evm.dts                    |    8 ++
 drivers/input/touchscreen/tps6507x-ts.c            |   79 +++++++++++++-------
 3 files changed, 82 insertions(+), 29 deletions(-)

-- 
1.7.10.4


^ permalink raw reply

* [PATCH V4 1/2] tps6507x-ts: Add DT support
From: Manish Badarkhe @ 2013-10-23 16:28 UTC (permalink / raw)
  To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/
  Cc: grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	rob-VoJi6FS/r0vR7s880joybQ, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ
In-Reply-To: <1382545733-8865-1-git-send-email-badarkhe.manish-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Add device tree based support for TI's tps6507x touchscreen.

Signed-off-by: Manish Badarkhe <badarkhe.manish-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Changes since V3:
 - Rebased on top of Dmitry's changes
 - Removed error handling for optional DT properties

Changes since V2:
 - Removed unnecessary code.
 - Updated Documentation to provide proper names of
   devicetree properties.

Changes since V1:
 - Updated documentation to specify tps6507x as multifunctional
   device.
 - return proper error value in absence of platform and DT
   data for touchscreen.
 - Updated commit message.

:100755 100755 8fffa3c... e1b9cfd... M	Documentation/devicetree/bindings/mfd/tps6507x.txt
:100644 100644 db604e0... 0cf0eb1... M	drivers/input/touchscreen/tps6507x-ts.c
 Documentation/devicetree/bindings/mfd/tps6507x.txt |   24 ++++++-
 drivers/input/touchscreen/tps6507x-ts.c            |   72 ++++++++++++--------
 2 files changed, 67 insertions(+), 29 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
index 8fffa3c..e1b9cfd 100755
--- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
+++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
@@ -1,4 +1,8 @@
-TPS6507x Power Management Integrated Circuit
+TPS6507x Multifunctional Device.
+
+Features provided by TPS6507x:
+        1.Power Management Integrated Circuit.
+        2.Touch-Screen.
 
 Required properties:
 - compatible: "ti,tps6507x"
@@ -23,6 +27,9 @@ Required properties:
        vindcdc1_2-supply: VDCDC1 and VDCDC2 input.
        vindcdc3-supply  : VDCDC3 input.
        vldo1_2-supply   : VLDO1 and VLDO2 input.
+- tsc: This node specifies touch screen data.
+	ti,poll-period : Time at which touch input is getting sampled in ms.
+	ti,min-pressure: Minimum pressure value to trigger touch.
 
 Regulator Optional properties:
 - defdcdc_default: It's property of DCDC2 and DCDC3 regulators.
@@ -30,6 +37,14 @@ Regulator Optional properties:
 			1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
   If this property is not defined, it defaults to 0 (not enabled).
 
+Touchscreen Optional properties:
+- ti,vendor : Touchscreen vendor id to populate
+	      in sysclass interface.
+- ti,product: Touchscreen product id to populate
+	      in sysclass interface.
+- ti,version: Touchscreen version id to populate
+	      in sysclass interface.
+
 Example:
 
 	pmu: tps6507x@48 {
@@ -88,4 +103,11 @@ Example:
 			};
 		};
 
+		tsc {
+			ti,poll-period = <30>;
+			ti,min-pressure = <0x30>;
+			ti,vendor = <0>;
+			ti,product = <65070>;
+			ti,version = <0x100>;
+		};
 	};
diff --git a/drivers/input/touchscreen/tps6507x-ts.c b/drivers/input/touchscreen/tps6507x-ts.c
index db604e0..0cf0eb1 100644
--- a/drivers/input/touchscreen/tps6507x-ts.c
+++ b/drivers/input/touchscreen/tps6507x-ts.c
@@ -22,6 +22,8 @@
 #include <linux/mfd/tps6507x.h>
 #include <linux/input/tps6507x-ts.h>
 #include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #define TSC_DEFAULT_POLL_PERIOD 30 /* ms */
 #define TPS_DEFAULT_MIN_PRESSURE 0x30
@@ -206,33 +208,54 @@ done:
 	tps6507x_adc_standby(tsc);
 }
 
+static void tsc_init_data(struct tps6507x_ts *tsc,
+		struct input_dev *input_dev)
+{
+	struct device_node *node = tsc->dev->of_node;
+	const struct tps6507x_board *tps_board =
+			dev_get_platdata(tsc->dev);
+	const struct touchscreen_init_data *init_data = NULL;
+
+	if (node)
+		node = of_find_node_by_name(node, "tsc");
+	if (tps_board)
+		/*
+		 * init_data points to array of touchscreen_init structure
+		 * coming from the board-evm file.
+		 */
+		init_data = tps_board->tps6507x_ts_init_data;
+
+	if (node == NULL || init_data == NULL) {
+		tsc->poll_dev->poll_interval = TSC_DEFAULT_POLL_PERIOD;
+		tsc->min_pressure = TPS_DEFAULT_MIN_PRESSURE;
+	} else if (init_data) {
+		tsc->poll_dev->poll_interval = init_data->poll_period;
+		tsc->min_pressure = init_data->min_pressure;
+		input_dev->id.vendor = init_data->vendor;
+		input_dev->id.product = init_data->product;
+		input_dev->id.version = init_data->version;
+	} else if (node) {
+		of_property_read_u32(node, "ti,poll-period",
+				&tsc->poll_dev->poll_interval);
+		of_property_read_u16(node, "ti,min-pressure",
+				&tsc->min_pressure);
+		of_property_read_u16(node, "ti,vendor",
+				&input_dev->id.vendor);
+		of_property_read_u16(node, "ti,product",
+				&input_dev->id.product);
+		of_property_read_u16(node, "ti,version",
+				&input_dev->id.version);
+	}
+}
+
 static int tps6507x_ts_probe(struct platform_device *pdev)
 {
 	struct tps6507x_dev *tps6507x_dev = dev_get_drvdata(pdev->dev.parent);
-	const struct tps6507x_board *tps_board;
-	const struct touchscreen_init_data *init_data;
 	struct tps6507x_ts *tsc;
 	struct input_polled_dev *poll_dev;
 	struct input_dev *input_dev;
 	int error;
 
-	/*
-	 * tps_board points to pmic related constants
-	 * coming from the board-evm file.
-	 */
-	tps_board = dev_get_platdata(tps6507x_dev->dev);
-	if (!tps_board) {
-		dev_err(tps6507x_dev->dev,
-			"Could not find tps6507x platform data\n");
-		return -ENODEV;
-	}
-
-	/*
-	 * init_data points to array of regulator_init structures
-	 * coming from the board-evm file.
-	 */
-	init_data = tps_board->tps6507x_ts_init_data;
-
 	tsc = devm_kzalloc(&pdev->dev, sizeof(struct tps6507x_ts), GFP_KERNEL);
 	if (!tsc) {
 		dev_err(tps6507x_dev->dev, "failed to allocate driver data\n");
@@ -241,8 +264,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 
 	tsc->mfd = tps6507x_dev;
 	tsc->dev = tps6507x_dev->dev;
-	tsc->min_pressure = init_data ?
-			init_data->min_pressure : TPS_DEFAULT_MIN_PRESSURE;
 
 	snprintf(tsc->phys, sizeof(tsc->phys),
 		 "%s/input0", dev_name(tsc->dev));
@@ -258,8 +279,6 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 
 	poll_dev->private = tsc;
 	poll_dev->poll = tps6507x_ts_poll;
-	poll_dev->poll_interval = init_data ?
-			init_data->poll_period : TSC_DEFAULT_POLL_PERIOD;
 
 	input_dev = poll_dev->input;
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
@@ -273,11 +292,8 @@ static int tps6507x_ts_probe(struct platform_device *pdev)
 	input_dev->phys = tsc->phys;
 	input_dev->dev.parent = tsc->dev;
 	input_dev->id.bustype = BUS_I2C;
-	if (init_data) {
-		input_dev->id.vendor = init_data->vendor;
-		input_dev->id.product = init_data->product;
-		input_dev->id.version = init_data->version;
-	}
+
+	tsc_init_data(tsc, input_dev);
 
 	error = tps6507x_adc_standby(tsc);
 	if (error)
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH V4 2/2] ARM: davinci: da850: add tps6507x touchscreen DT data
From: Manish Badarkhe @ 2013-10-23 16:28 UTC (permalink / raw)
  To: devicetree-discuss, devicetree, linux-input, linux-doc,
	linux-kernel, davinci-linux-open-source
  Cc: grant.likely, dmitry.torokhov, rob.herring, rob, badarkhe.manish
In-Reply-To: <1382545733-8865-1-git-send-email-badarkhe.manish@gmail.com>

Add tps6507x touchscreen DT node to da850-evm.
Touchscreen DT data is added as per da850 board file.

Signed-off-by: Manish Badarkhe <badarkhe.manish@gmail.com>
---
Changes since V3:
 - Removed vref property.

Changes since V2:
 - Updated tps6507x documentation.
 - Removed unnecessary code.

Changes since V1:
 - Updated tps6507x documentation.
 - Updated commit message.
 - return proper error value in absence platform and DT data
   for touchscreen.

:100644 100644 588ce58... 5a35264... M	arch/arm/boot/dts/da850-evm.dts
 arch/arm/boot/dts/da850-evm.dts |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 588ce58..5a35264 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -166,4 +166,12 @@
 			regulator-boot-on;
 		};
 	};
+
+	tsc {
+		ti,poll-period = <30>;
+		ti,min-pressure = <0x30>;
+		ti,vendor = <0>;
+		ti,product = <65070>;
+		ti,version = <0x100>;
+	};
 };
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCHv2 3/3] Input: twl4030-pwrbutton: simplify driver using devm_*
From: Aaro Koskinen @ 2013-10-23 16:30 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, Dmitry Torokhov, Grant Likely, Rob Herring,
	Peter Ujfalusi, Sachin Kamat, linux-input, linux-kernel,
	devicetree
In-Reply-To: <1382540482-12261-4-git-send-email-sre@debian.org>

Hi,

On Wed, Oct 23, 2013 at 05:01:22PM +0200, Sebastian Reichel wrote:
>  static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
>  {
>  	struct input_dev *pwr = platform_get_drvdata(pdev);
> -	int irq = platform_get_irq(pdev, 0);
>  
> -	free_irq(irq, pwr);
>  	input_unregister_device(pwr);

You need convert the driver to use devm_input_allocate_device()
first. Otherwise driver will crash the kernel here if you get interrupt
after unregistering the device.

A.

^ permalink raw reply

* Re: [PATCH V4 1/2] tps6507x-ts: Add DT support
From: Mark Rutland @ 2013-10-23 16:45 UTC (permalink / raw)
  To: Manish Badarkhe
  Cc: devicetree-discuss@lists.ozlabs.org, devicetree@vger.kernel.org,
	linux-input@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	davinci-linux-open-source@linux.davincidsp.com,
	grant.likely@secretlab.ca, dmitry.torokhov@gmail.com,
	rob.herring@calxeda.com, rob@landley.net
In-Reply-To: <1382545733-8865-2-git-send-email-badarkhe.manish@gmail.com>

On Wed, Oct 23, 2013 at 05:28:52PM +0100, Manish Badarkhe wrote:
> Add device tree based support for TI's tps6507x touchscreen.
> 
> Signed-off-by: Manish Badarkhe <badarkhe.manish@gmail.com>
> ---
> Changes since V3:
>  - Rebased on top of Dmitry's changes
>  - Removed error handling for optional DT properties
> 
> Changes since V2:
>  - Removed unnecessary code.
>  - Updated Documentation to provide proper names of
>    devicetree properties.
> 
> Changes since V1:
>  - Updated documentation to specify tps6507x as multifunctional
>    device.
>  - return proper error value in absence of platform and DT
>    data for touchscreen.
>  - Updated commit message.
> 
> :100755 100755 8fffa3c... e1b9cfd... M	Documentation/devicetree/bindings/mfd/tps6507x.txt
> :100644 100644 db604e0... 0cf0eb1... M	drivers/input/touchscreen/tps6507x-ts.c
>  Documentation/devicetree/bindings/mfd/tps6507x.txt |   24 ++++++-
>  drivers/input/touchscreen/tps6507x-ts.c            |   72 ++++++++++++--------
>  2 files changed, 67 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> index 8fffa3c..e1b9cfd 100755
> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> @@ -1,4 +1,8 @@
> -TPS6507x Power Management Integrated Circuit
> +TPS6507x Multifunctional Device.
> +
> +Features provided by TPS6507x:
> +        1.Power Management Integrated Circuit.
> +        2.Touch-Screen.
>  
>  Required properties:
>  - compatible: "ti,tps6507x"
> @@ -23,6 +27,9 @@ Required properties:
>         vindcdc1_2-supply: VDCDC1 and VDCDC2 input.
>         vindcdc3-supply  : VDCDC3 input.
>         vldo1_2-supply   : VLDO1 and VLDO2 input.
> +- tsc: This node specifies touch screen data.

This is a node, but is listed un "Required properties". That seems odd.

When is it required?

Why is the data under the tsc subnode? Why not directly under the main node?

> +	ti,poll-period : Time at which touch input is getting sampled in ms.

Is this for debounce? Other bindings have similar but differently named
properties.

Why is this necessary? Can the driver not choose a sane value?

> +	ti,min-pressure: Minimum pressure value to trigger touch.

What type are these? Single (u32) cells?

What units is ti,min-pressure in?

>  
>  Regulator Optional properties:
>  - defdcdc_default: It's property of DCDC2 and DCDC3 regulators.
> @@ -30,6 +37,14 @@ Regulator Optional properties:
>  			1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>    If this property is not defined, it defaults to 0 (not enabled).
>  
> +Touchscreen Optional properties:
> +- ti,vendor : Touchscreen vendor id to populate
> +	      in sysclass interface.
> +- ti,product: Touchscreen product id to populate
> +	      in sysclass interface.
> +- ti,version: Touchscreen version id to populate
> +	      in sysclass interface.

Are these integers, strings, or something else?

What values make sense?

What's the sysclass interface? That sounds like a Linux detail. The properties
might be fine but the description shouldn't need to refer to Linux internals.

> +
>  Example:
>  
>  	pmu: tps6507x@48 {
> @@ -88,4 +103,11 @@ Example:
>  			};
>  		};
>  
> +		tsc {
> +			ti,poll-period = <30>;
> +			ti,min-pressure = <0x30>;
> +			ti,vendor = <0>;
> +			ti,product = <65070>;
> +			ti,version = <0x100>;
> +		};
>  	};
> diff --git a/drivers/input/touchscreen/tps6507x-ts.c b/drivers/input/touchscreen/tps6507x-ts.c
> index db604e0..0cf0eb1 100644
> --- a/drivers/input/touchscreen/tps6507x-ts.c
> +++ b/drivers/input/touchscreen/tps6507x-ts.c
> @@ -22,6 +22,8 @@
>  #include <linux/mfd/tps6507x.h>
>  #include <linux/input/tps6507x-ts.h>
>  #include <linux/delay.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  
>  #define TSC_DEFAULT_POLL_PERIOD 30 /* ms */
>  #define TPS_DEFAULT_MIN_PRESSURE 0x30
> @@ -206,33 +208,54 @@ done:
>  	tps6507x_adc_standby(tsc);
>  }
>  
> +static void tsc_init_data(struct tps6507x_ts *tsc,
> +		struct input_dev *input_dev)
> +{
> +	struct device_node *node = tsc->dev->of_node;
> +	const struct tps6507x_board *tps_board =
> +			dev_get_platdata(tsc->dev);
> +	const struct touchscreen_init_data *init_data = NULL;
> +
> +	if (node)
> +		node = of_find_node_by_name(node, "tsc");

As of_find_node_by_name traverses the list of all nodes (not just children),
this may match nodes other than what you expect.

> +	if (tps_board)
> +		/*
> +		 * init_data points to array of touchscreen_init structure
> +		 * coming from the board-evm file.
> +		 */
> +		init_data = tps_board->tps6507x_ts_init_data;
> +
> +	if (node == NULL || init_data == NULL) {
> +		tsc->poll_dev->poll_interval = TSC_DEFAULT_POLL_PERIOD;
> +		tsc->min_pressure = TPS_DEFAULT_MIN_PRESSURE;
> +	} else if (init_data) {
> +		tsc->poll_dev->poll_interval = init_data->poll_period;
> +		tsc->min_pressure = init_data->min_pressure;
> +		input_dev->id.vendor = init_data->vendor;
> +		input_dev->id.product = init_data->product;
> +		input_dev->id.version = init_data->version;
> +	} else if (node) {
> +		of_property_read_u32(node, "ti,poll-period",
> +				&tsc->poll_dev->poll_interval);
> +		of_property_read_u16(node, "ti,min-pressure",
> +				&tsc->min_pressure);

You didn't mention these were 16-bit values in the binding.

As DTB is encoded big-endian, and you didn't use a /bits/ 16 declaration
(making the property a u32 cell), won't this read 0 in all cases you have a
value in the expected range (as in your example)?

> +		of_property_read_u16(node, "ti,vendor",
> +				&input_dev->id.vendor);
> +		of_property_read_u16(node, "ti,product",
> +				&input_dev->id.product);
> +		of_property_read_u16(node, "ti,version",
> +				&input_dev->id.version);

Similarly for these three.

Thanks,
Mark.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox