linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] Input: tsc2005 - Add support for tsc2004
@ 2015-10-29  0:12 Michael Welling
  2015-10-29  7:25 ` kbuild test robot
  2015-10-30  1:45 ` Dmitry Torokhov
  0 siblings, 2 replies; 12+ messages in thread
From: Michael Welling @ 2015-10-29  0:12 UTC (permalink / raw)
  To: Dmitry Torokhov, Tony Lindgren, Pavel Machek, Felipe Balbi,
	Sebastian Reichel, Roger Quadros, devicetree, linux-kernel,
	linux-input
  Cc: Michael Welling

Adds support for the i2c based tsc2004.

Due to the overlapping functionality of the tsc2004 and tsc2005
the common code was moved to a core driver (tsc200x-core).

Signed-off-by: Michael Welling <mwelling@ieee.org>
---
v3: Splits the tsc2004 and tsc2005 into separate drivers with
with common routines in tsc200x-core.
v2: Fixes Kconfig based on report for 0-day build bot.
 .../bindings/input/touchscreen/tsc2004.txt         |  38 +
 drivers/input/touchscreen/Kconfig                  |  17 +
 drivers/input/touchscreen/Makefile                 |   2 +
 drivers/input/touchscreen/tsc2004.c                |  73 ++
 drivers/input/touchscreen/tsc2005.c                | 707 +-----------------
 drivers/input/touchscreen/tsc200x-core.c           | 790 +++++++++++++++++++++
 drivers/input/touchscreen/tsc200x-core.h           |  13 +
 7 files changed, 938 insertions(+), 702 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2004.txt
 create mode 100644 drivers/input/touchscreen/tsc2004.c
 create mode 100644 drivers/input/touchscreen/tsc200x-core.c
 create mode 100644 drivers/input/touchscreen/tsc200x-core.h

diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2004.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2004.txt
new file mode 100644
index 0000000..14a37fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2004.txt
@@ -0,0 +1,38 @@
+* Texas Instruments tsc2004 touchscreen controller
+
+Required properties:
+ - compatible		      : "ti,tsc2004"
+ - interrupts		      : IRQ specifier
+ - vio-supply                 : Regulator specifier
+
+Optional properties:
+ - reset-gpios		      : GPIO specifier
+ - ti,x-plate-ohms	      : integer, resistance of the touchscreen's X plates
+				in ohm (defaults to 280)
+ - ti,esd-recovery-timeout-ms : integer, if the touchscreen does not respond after
+				the configured time (in milli seconds), the driver
+				will reset it. This is disabled by default.
+ - properties defined in touchscreen.txt
+
+Example:
+
+&i2c3 {
+	tsc2004@48 {
+		compatible = "ti,tsc2004";
+		reg = <0x48>;
+		vio-supply = <&vio>;
+
+		reset-gpios = <&gpio4 8 GPIO_ACTIVE_HIGH>;
+		interrupts-extended = <&gpio1 27 IRQ_TYPE_EDGE_RISING>;
+
+		touchscreen-fuzz-x = <4>;
+		touchscreen-fuzz-y = <7>;
+		touchscreen-fuzz-pressure = <2>;
+		touchscreen-size-x = <4096>;
+		touchscreen-size-y = <4096>;
+		touchscreen-max-pressure = <2048>;
+
+		ti,x-plate-ohms = <280>;
+		ti,esd-recovery-timeout-ms = <8000>;
+	};
+}
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 80cc698..e574f8c 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -939,10 +939,27 @@ config TOUCHSCREEN_TSC_SERIO
 	  To compile this driver as a module, choose M here: the
 	  module will be called tsc40.
 
+config TOUCHSCREEN_TSC200X
+	tristate
+
+config TOUCHSCREEN_TSC2004
+	tristate "TSC2004 based touchscreens"
+	depends on I2C
+	select REGMAP_I2C
+	select TOUCHSCREEN_TSC200X
+	help
+	  Say Y here if you have a TSC2004 based touchscreen.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called tsc2004.
+
 config TOUCHSCREEN_TSC2005
 	tristate "TSC2005 based touchscreens"
 	depends on SPI_MASTER
 	select REGMAP_SPI
+	select TOUCHSCREEN_TSC200X
 	help
 	  Say Y here if you have a TSC2005 based touchscreen.
 
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 17435c7..810b047 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -69,6 +69,8 @@ obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213)	+= touchit213.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)	+= touchright.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN)	+= touchwin.o
 obj-$(CONFIG_TOUCHSCREEN_TSC_SERIO)	+= tsc40.o
+obj-$(CONFIG_TOUCHSCREEN_TSC200X)	+= tsc200x-core.o
+obj-$(CONFIG_TOUCHSCREEN_TSC2004)	+= tsc2004.o
 obj-$(CONFIG_TOUCHSCREEN_TSC2005)	+= tsc2005.o
 obj-$(CONFIG_TOUCHSCREEN_TSC2007)	+= tsc2007.o
 obj-$(CONFIG_TOUCHSCREEN_UCB1400)	+= ucb1400_ts.o
diff --git a/drivers/input/touchscreen/tsc2004.c b/drivers/input/touchscreen/tsc2004.c
new file mode 100644
index 0000000..01457a2
--- /dev/null
+++ b/drivers/input/touchscreen/tsc2004.c
@@ -0,0 +1,73 @@
+/*
+ * TSC2004 touchscreen driver
+ *
+ * Copyright (C) 2015 EMAC Inc.
+ * Copyright (C) 2015 QWERTY Embedded Design
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/pm.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include "tsc200x-core.h"
+
+static int tsc2004_probe(struct i2c_client *i2c,
+			 const struct i2c_device_id *id)
+
+{
+	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
+			     devm_regmap_init_i2c(i2c,
+						  &tsc2005_regmap_config));
+}
+
+static int tsc2004_remove(struct i2c_client *i2c)
+{
+	return tsc200x_remove(&i2c->dev);
+}
+
+static const struct i2c_device_id tsc2004_idtable[] = {
+	{ "tsc2004", 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(i2c, tsc2004_idtable);
+
+#ifdef CONFIG_OF
+static const struct of_device_id tsc2004_of_match[] = {
+	{ .compatible = "ti,tsc2004" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tsc2004_of_match);
+#endif
+
+static SIMPLE_DEV_PM_OPS(tsc2004_pm_ops, tsc200x_suspend, tsc200x_resume);
+
+static struct i2c_driver tsc2004_driver = {
+	.driver = {
+		.name	= "tsc2004",
+		.of_match_table = of_match_ptr(tsc2004_of_match),
+		.pm	= &tsc2004_pm_ops,
+	},
+	.id_table	= tsc2004_idtable,
+	.probe		= tsc2004_probe,
+	.remove		= tsc2004_remove,
+};
+
+module_i2c_driver(tsc2004_driver);
+
+MODULE_AUTHOR("Michael Welling <mwelling@ieee.org>");
+MODULE_DESCRIPTION("TSC2004 Touchscreen Driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 0f65d02..67d8808 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -16,574 +16,22 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  * GNU General Public License for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
  */
 
-#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/input.h>
-#include <linux/input/touchscreen.h>
-#include <linux/interrupt.h>
-#include <linux/delay.h>
 #include <linux/pm.h>
 #include <linux/of.h>
 #include <linux/spi/spi.h>
-#include <linux/spi/tsc2005.h>
-#include <linux/regulator/consumer.h>
 #include <linux/regmap.h>
-#include <linux/gpio/consumer.h>
-
-/*
- * The touchscreen interface operates as follows:
- *
- * 1) Pen is pressed against the touchscreen.
- * 2) TSC2005 performs AD conversion.
- * 3) After the conversion is done TSC2005 drives DAV line down.
- * 4) GPIO IRQ is received and tsc2005_irq_thread() is scheduled.
- * 5) tsc2005_irq_thread() queues up an spi transfer to fetch the x, y, z1, z2
- *    values.
- * 6) tsc2005_irq_thread() reports coordinates to input layer and sets up
- *    tsc2005_penup_timer() to be called after TSC2005_PENUP_TIME_MS (40ms).
- * 7) When the penup timer expires, there have not been touch or DAV interrupts
- *    during the last 40ms which means the pen has been lifted.
- *
- * ESD recovery via a hardware reset is done if the TSC2005 doesn't respond
- * after a configurable period (in ms) of activity. If esd_timeout is 0, the
- * watchdog is disabled.
- */
-
-/* control byte 1 */
-#define TSC2005_CMD			0x80
-#define TSC2005_CMD_NORMAL		0x00
-#define TSC2005_CMD_STOP		0x01
-#define TSC2005_CMD_12BIT		0x04
-
-/* control byte 0 */
-#define TSC2005_REG_READ		0x01 /* R/W access */
-#define TSC2005_REG_PND0		0x02 /* Power Not Down Control */
-#define TSC2005_REG_X			(0x0 << 3)
-#define TSC2005_REG_Y			(0x1 << 3)
-#define TSC2005_REG_Z1			(0x2 << 3)
-#define TSC2005_REG_Z2			(0x3 << 3)
-#define TSC2005_REG_AUX			(0x4 << 3)
-#define TSC2005_REG_TEMP1		(0x5 << 3)
-#define TSC2005_REG_TEMP2		(0x6 << 3)
-#define TSC2005_REG_STATUS		(0x7 << 3)
-#define TSC2005_REG_AUX_HIGH		(0x8 << 3)
-#define TSC2005_REG_AUX_LOW		(0x9 << 3)
-#define TSC2005_REG_TEMP_HIGH		(0xA << 3)
-#define TSC2005_REG_TEMP_LOW		(0xB << 3)
-#define TSC2005_REG_CFR0		(0xC << 3)
-#define TSC2005_REG_CFR1		(0xD << 3)
-#define TSC2005_REG_CFR2		(0xE << 3)
-#define TSC2005_REG_CONV_FUNC		(0xF << 3)
-
-/* configuration register 0 */
-#define TSC2005_CFR0_PRECHARGE_276US	0x0040
-#define TSC2005_CFR0_STABTIME_1MS	0x0300
-#define TSC2005_CFR0_CLOCK_1MHZ		0x1000
-#define TSC2005_CFR0_RESOLUTION12	0x2000
-#define TSC2005_CFR0_PENMODE		0x8000
-#define TSC2005_CFR0_INITVALUE		(TSC2005_CFR0_STABTIME_1MS    | \
-					 TSC2005_CFR0_CLOCK_1MHZ      | \
-					 TSC2005_CFR0_RESOLUTION12    | \
-					 TSC2005_CFR0_PRECHARGE_276US | \
-					 TSC2005_CFR0_PENMODE)
-
-/* bits common to both read and write of configuration register 0 */
-#define	TSC2005_CFR0_RW_MASK		0x3fff
-
-/* configuration register 1 */
-#define TSC2005_CFR1_BATCHDELAY_4MS	0x0003
-#define TSC2005_CFR1_INITVALUE		TSC2005_CFR1_BATCHDELAY_4MS
-
-/* configuration register 2 */
-#define TSC2005_CFR2_MAVE_Z		0x0004
-#define TSC2005_CFR2_MAVE_Y		0x0008
-#define TSC2005_CFR2_MAVE_X		0x0010
-#define TSC2005_CFR2_AVG_7		0x0800
-#define TSC2005_CFR2_MEDIUM_15		0x3000
-#define TSC2005_CFR2_INITVALUE		(TSC2005_CFR2_MAVE_X	| \
-					 TSC2005_CFR2_MAVE_Y	| \
-					 TSC2005_CFR2_MAVE_Z	| \
-					 TSC2005_CFR2_MEDIUM_15	| \
-					 TSC2005_CFR2_AVG_7)
-
-#define MAX_12BIT			0xfff
-#define TSC2005_DEF_X_FUZZ		4
-#define TSC2005_DEF_Y_FUZZ		8
-#define TSC2005_DEF_P_FUZZ		2
-#define TSC2005_DEF_RESISTOR		280
+#include "tsc200x-core.h"
 
 #define TSC2005_SPI_MAX_SPEED_HZ	10000000
-#define TSC2005_PENUP_TIME_MS		40
-
-static const struct regmap_range tsc2005_writable_ranges[] = {
-	regmap_reg_range(TSC2005_REG_AUX_HIGH, TSC2005_REG_CFR2),
-};
-
-static const struct regmap_access_table tsc2005_writable_table = {
-	.yes_ranges = tsc2005_writable_ranges,
-	.n_yes_ranges = ARRAY_SIZE(tsc2005_writable_ranges),
-};
-
-static struct regmap_config tsc2005_regmap_config = {
-	.reg_bits = 8,
-	.val_bits = 16,
-	.reg_stride = 0x08,
-	.max_register = 0x78,
-	.read_flag_mask = TSC2005_REG_READ,
-	.write_flag_mask = TSC2005_REG_PND0,
-	.wr_table = &tsc2005_writable_table,
-	.use_single_rw = true,
-};
-
-struct tsc2005_data {
-	u16 x;
-	u16 y;
-	u16 z1;
-	u16 z2;
-} __packed;
-#define TSC2005_DATA_REGS 4
-
-struct tsc2005 {
-	struct spi_device	*spi;
-	struct regmap		*regmap;
-
-	struct input_dev	*idev;
-	char			phys[32];
-
-	struct mutex		mutex;
-
-	/* raw copy of previous x,y,z */
-	int			in_x;
-	int			in_y;
-	int                     in_z1;
-	int			in_z2;
-
-	spinlock_t		lock;
-	struct timer_list	penup_timer;
-
-	unsigned int		esd_timeout;
-	struct delayed_work	esd_work;
-	unsigned long		last_valid_interrupt;
-
-	unsigned int		x_plate_ohm;
-
-	bool			opened;
-	bool			suspended;
-
-	bool			pen_down;
-
-	struct regulator	*vio;
-
-	struct gpio_desc	*reset_gpio;
-	void			(*set_reset)(bool enable);
-};
-
-static int tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
-{
-	u8 tx = TSC2005_CMD | TSC2005_CMD_12BIT | cmd;
-	struct spi_transfer xfer = {
-		.tx_buf		= &tx,
-		.len		= 1,
-		.bits_per_word	= 8,
-	};
-	struct spi_message msg;
-	int error;
-
-	spi_message_init(&msg);
-	spi_message_add_tail(&xfer, &msg);
-
-	error = spi_sync(ts->spi, &msg);
-	if (error) {
-		dev_err(&ts->spi->dev, "%s: failed, command: %x, error: %d\n",
-			__func__, cmd, error);
-		return error;
-	}
-
-	return 0;
-}
-
-static void tsc2005_update_pen_state(struct tsc2005 *ts,
-				     int x, int y, int pressure)
-{
-	if (pressure) {
-		input_report_abs(ts->idev, ABS_X, x);
-		input_report_abs(ts->idev, ABS_Y, y);
-		input_report_abs(ts->idev, ABS_PRESSURE, pressure);
-		if (!ts->pen_down) {
-			input_report_key(ts->idev, BTN_TOUCH, !!pressure);
-			ts->pen_down = true;
-		}
-	} else {
-		input_report_abs(ts->idev, ABS_PRESSURE, 0);
-		if (ts->pen_down) {
-			input_report_key(ts->idev, BTN_TOUCH, 0);
-			ts->pen_down = false;
-		}
-	}
-	input_sync(ts->idev);
-	dev_dbg(&ts->spi->dev, "point(%4d,%4d), pressure (%4d)\n", x, y,
-		pressure);
-}
-
-static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
-{
-	struct tsc2005 *ts = _ts;
-	unsigned long flags;
-	unsigned int pressure;
-	struct tsc2005_data tsdata;
-	int error;
-
-	/* read the coordinates */
-	error = regmap_bulk_read(ts->regmap, TSC2005_REG_X, &tsdata,
-				 TSC2005_DATA_REGS);
-	if (unlikely(error))
-		goto out;
-
-	/* validate position */
-	if (unlikely(tsdata.x > MAX_12BIT || tsdata.y > MAX_12BIT))
-		goto out;
-
-	/* Skip reading if the pressure components are out of range */
-	if (unlikely(tsdata.z1 == 0 || tsdata.z2 > MAX_12BIT))
-		goto out;
-	if (unlikely(tsdata.z1 >= tsdata.z2))
-		goto out;
-
-       /*
-	* Skip point if this is a pen down with the exact same values as
-	* the value before pen-up - that implies SPI fed us stale data
-	*/
-	if (!ts->pen_down &&
-	    ts->in_x == tsdata.x && ts->in_y == tsdata.y &&
-	    ts->in_z1 == tsdata.z1 && ts->in_z2 == tsdata.z2) {
-		goto out;
-	}
-
-	/*
-	 * At this point we are happy we have a valid and useful reading.
-	 * Remember it for later comparisons. We may now begin downsampling.
-	 */
-	ts->in_x = tsdata.x;
-	ts->in_y = tsdata.y;
-	ts->in_z1 = tsdata.z1;
-	ts->in_z2 = tsdata.z2;
-
-	/* Compute touch pressure resistance using equation #1 */
-	pressure = tsdata.x * (tsdata.z2 - tsdata.z1) / tsdata.z1;
-	pressure = pressure * ts->x_plate_ohm / 4096;
-	if (unlikely(pressure > MAX_12BIT))
-		goto out;
-
-	spin_lock_irqsave(&ts->lock, flags);
-
-	tsc2005_update_pen_state(ts, tsdata.x, tsdata.y, pressure);
-	mod_timer(&ts->penup_timer,
-		  jiffies + msecs_to_jiffies(TSC2005_PENUP_TIME_MS));
-
-	spin_unlock_irqrestore(&ts->lock, flags);
-
-	ts->last_valid_interrupt = jiffies;
-out:
-	return IRQ_HANDLED;
-}
-
-static void tsc2005_penup_timer(unsigned long data)
-{
-	struct tsc2005 *ts = (struct tsc2005 *)data;
-	unsigned long flags;
-
-	spin_lock_irqsave(&ts->lock, flags);
-	tsc2005_update_pen_state(ts, 0, 0, 0);
-	spin_unlock_irqrestore(&ts->lock, flags);
-}
-
-static void tsc2005_start_scan(struct tsc2005 *ts)
-{
-	regmap_write(ts->regmap, TSC2005_REG_CFR0, TSC2005_CFR0_INITVALUE);
-	regmap_write(ts->regmap, TSC2005_REG_CFR1, TSC2005_CFR1_INITVALUE);
-	regmap_write(ts->regmap, TSC2005_REG_CFR2, TSC2005_CFR2_INITVALUE);
-	tsc2005_cmd(ts, TSC2005_CMD_NORMAL);
-}
-
-static void tsc2005_stop_scan(struct tsc2005 *ts)
-{
-	tsc2005_cmd(ts, TSC2005_CMD_STOP);
-}
-
-static void tsc2005_set_reset(struct tsc2005 *ts, bool enable)
-{
-	if (ts->reset_gpio)
-		gpiod_set_value_cansleep(ts->reset_gpio, enable);
-	else if (ts->set_reset)
-		ts->set_reset(enable);
-}
-
-/* must be called with ts->mutex held */
-static void __tsc2005_disable(struct tsc2005 *ts)
-{
-	tsc2005_stop_scan(ts);
-
-	disable_irq(ts->spi->irq);
-	del_timer_sync(&ts->penup_timer);
-
-	cancel_delayed_work_sync(&ts->esd_work);
-
-	enable_irq(ts->spi->irq);
-}
-
-/* must be called with ts->mutex held */
-static void __tsc2005_enable(struct tsc2005 *ts)
-{
-	tsc2005_start_scan(ts);
-
-	if (ts->esd_timeout && (ts->set_reset || ts->reset_gpio)) {
-		ts->last_valid_interrupt = jiffies;
-		schedule_delayed_work(&ts->esd_work,
-				round_jiffies_relative(
-					msecs_to_jiffies(ts->esd_timeout)));
-	}
-
-}
-
-static ssize_t tsc2005_selftest_show(struct device *dev,
-				     struct device_attribute *attr,
-				     char *buf)
-{
-	struct tsc2005 *ts = dev_get_drvdata(dev);
-	unsigned int temp_high;
-	unsigned int temp_high_orig;
-	unsigned int temp_high_test;
-	bool success = true;
-	int error;
-
-	mutex_lock(&ts->mutex);
-
-	/*
-	 * Test TSC2005 communications via temp high register.
-	 */
-	__tsc2005_disable(ts);
-
-	error = regmap_read(ts->regmap, TSC2005_REG_TEMP_HIGH, &temp_high_orig);
-	if (error) {
-		dev_warn(dev, "selftest failed: read error %d\n", error);
-		success = false;
-		goto out;
-	}
-
-	temp_high_test = (temp_high_orig - 1) & MAX_12BIT;
-
-	error = regmap_write(ts->regmap, TSC2005_REG_TEMP_HIGH, temp_high_test);
-	if (error) {
-		dev_warn(dev, "selftest failed: write error %d\n", error);
-		success = false;
-		goto out;
-	}
-
-	error = regmap_read(ts->regmap, TSC2005_REG_TEMP_HIGH, &temp_high);
-	if (error) {
-		dev_warn(dev, "selftest failed: read error %d after write\n",
-			 error);
-		success = false;
-		goto out;
-	}
-
-	if (temp_high != temp_high_test) {
-		dev_warn(dev, "selftest failed: %d != %d\n",
-			 temp_high, temp_high_test);
-		success = false;
-	}
-
-	/* hardware reset */
-	tsc2005_set_reset(ts, false);
-	usleep_range(100, 500); /* only 10us required */
-	tsc2005_set_reset(ts, true);
-
-	if (!success)
-		goto out;
-
-	/* test that the reset really happened */
-	error = regmap_read(ts->regmap, TSC2005_REG_TEMP_HIGH, &temp_high);
-	if (error) {
-		dev_warn(dev, "selftest failed: read error %d after reset\n",
-			 error);
-		success = false;
-		goto out;
-	}
-
-	if (temp_high != temp_high_orig) {
-		dev_warn(dev, "selftest failed after reset: %d != %d\n",
-			 temp_high, temp_high_orig);
-		success = false;
-	}
-
-out:
-	__tsc2005_enable(ts);
-	mutex_unlock(&ts->mutex);
-
-	return sprintf(buf, "%d\n", success);
-}
-
-static DEVICE_ATTR(selftest, S_IRUGO, tsc2005_selftest_show, NULL);
-
-static struct attribute *tsc2005_attrs[] = {
-	&dev_attr_selftest.attr,
-	NULL
-};
-
-static umode_t tsc2005_attr_is_visible(struct kobject *kobj,
-				      struct attribute *attr, int n)
-{
-	struct device *dev = container_of(kobj, struct device, kobj);
-	struct tsc2005 *ts = dev_get_drvdata(dev);
-	umode_t mode = attr->mode;
-
-	if (attr == &dev_attr_selftest.attr) {
-		if (!ts->set_reset && !ts->reset_gpio)
-			mode = 0;
-	}
-
-	return mode;
-}
-
-static const struct attribute_group tsc2005_attr_group = {
-	.is_visible	= tsc2005_attr_is_visible,
-	.attrs		= tsc2005_attrs,
-};
-
-static void tsc2005_esd_work(struct work_struct *work)
-{
-	struct tsc2005 *ts = container_of(work, struct tsc2005, esd_work.work);
-	int error;
-	unsigned int r;
-
-	if (!mutex_trylock(&ts->mutex)) {
-		/*
-		 * If the mutex is taken, it means that disable or enable is in
-		 * progress. In that case just reschedule the work. If the work
-		 * is not needed, it will be canceled by disable.
-		 */
-		goto reschedule;
-	}
-
-	if (time_is_after_jiffies(ts->last_valid_interrupt +
-				  msecs_to_jiffies(ts->esd_timeout)))
-		goto out;
-
-	/* We should be able to read register without disabling interrupts. */
-	error = regmap_read(ts->regmap, TSC2005_REG_CFR0, &r);
-	if (!error &&
-	    !((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK)) {
-		goto out;
-	}
-
-	/*
-	 * If we could not read our known value from configuration register 0
-	 * then we should reset the controller as if from power-up and start
-	 * scanning again.
-	 */
-	dev_info(&ts->spi->dev, "TSC2005 not responding - resetting\n");
-
-	disable_irq(ts->spi->irq);
-	del_timer_sync(&ts->penup_timer);
-
-	tsc2005_update_pen_state(ts, 0, 0, 0);
-
-	tsc2005_set_reset(ts, false);
-	usleep_range(100, 500); /* only 10us required */
-	tsc2005_set_reset(ts, true);
-
-	enable_irq(ts->spi->irq);
-	tsc2005_start_scan(ts);
-
-out:
-	mutex_unlock(&ts->mutex);
-reschedule:
-	/* re-arm the watchdog */
-	schedule_delayed_work(&ts->esd_work,
-			      round_jiffies_relative(
-					msecs_to_jiffies(ts->esd_timeout)));
-}
-
-static int tsc2005_open(struct input_dev *input)
-{
-	struct tsc2005 *ts = input_get_drvdata(input);
-
-	mutex_lock(&ts->mutex);
-
-	if (!ts->suspended)
-		__tsc2005_enable(ts);
-
-	ts->opened = true;
-
-	mutex_unlock(&ts->mutex);
-
-	return 0;
-}
-
-static void tsc2005_close(struct input_dev *input)
-{
-	struct tsc2005 *ts = input_get_drvdata(input);
-
-	mutex_lock(&ts->mutex);
-
-	if (!ts->suspended)
-		__tsc2005_disable(ts);
-
-	ts->opened = false;
-
-	mutex_unlock(&ts->mutex);
-}
 
 static int tsc2005_probe(struct spi_device *spi)
 {
-	const struct tsc2005_platform_data *pdata = dev_get_platdata(&spi->dev);
-	struct device_node *np = spi->dev.of_node;
-
-	struct tsc2005 *ts;
-	struct input_dev *input_dev;
-	unsigned int max_x = MAX_12BIT;
-	unsigned int max_y = MAX_12BIT;
-	unsigned int max_p = MAX_12BIT;
-	unsigned int fudge_x = TSC2005_DEF_X_FUZZ;
-	unsigned int fudge_y = TSC2005_DEF_Y_FUZZ;
-	unsigned int fudge_p = TSC2005_DEF_P_FUZZ;
-	unsigned int x_plate_ohm = TSC2005_DEF_RESISTOR;
-	unsigned int esd_timeout;
 	int error;
 
-	if (!np && !pdata) {
-		dev_err(&spi->dev, "no platform data\n");
-		return -ENODEV;
-	}
-
-	if (spi->irq <= 0) {
-		dev_err(&spi->dev, "no irq\n");
-		return -ENODEV;
-	}
-
-	if (pdata) {
-		fudge_x	= pdata->ts_x_fudge;
-		fudge_y	= pdata->ts_y_fudge;
-		fudge_p	= pdata->ts_pressure_fudge;
-		max_x	= pdata->ts_x_max;
-		max_y	= pdata->ts_y_max;
-		max_p	= pdata->ts_pressure_max;
-		x_plate_ohm = pdata->ts_x_plate_ohm;
-		esd_timeout = pdata->esd_timeout_ms;
-	} else {
-		x_plate_ohm = TSC2005_DEF_RESISTOR;
-		of_property_read_u32(np, "ti,x-plate-ohms", &x_plate_ohm);
-		esd_timeout = 0;
-		of_property_read_u32(np, "ti,esd-recovery-timeout-ms",
-								&esd_timeout);
-	}
-
 	spi->mode = SPI_MODE_0;
 	spi->bits_per_word = 8;
 	if (!spi->max_speed_hz)
@@ -593,161 +41,16 @@ static int tsc2005_probe(struct spi_device *spi)
 	if (error)
 		return error;
 
-	ts = devm_kzalloc(&spi->dev, sizeof(*ts), GFP_KERNEL);
-	if (!ts)
-		return -ENOMEM;
-
-	input_dev = devm_input_allocate_device(&spi->dev);
-	if (!input_dev)
-		return -ENOMEM;
-
-	ts->spi = spi;
-	ts->idev = input_dev;
-
-	ts->regmap = devm_regmap_init_spi(spi, &tsc2005_regmap_config);
-	if (IS_ERR(ts->regmap))
-		return PTR_ERR(ts->regmap);
-
-	ts->x_plate_ohm = x_plate_ohm;
-	ts->esd_timeout = esd_timeout;
-
-	ts->reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset",
-						 GPIOD_OUT_HIGH);
-	if (IS_ERR(ts->reset_gpio)) {
-		error = PTR_ERR(ts->reset_gpio);
-		dev_err(&spi->dev, "error acquiring reset gpio: %d\n", error);
-		return error;
-	}
-
-	ts->vio = devm_regulator_get_optional(&spi->dev, "vio");
-	if (IS_ERR(ts->vio)) {
-		error = PTR_ERR(ts->vio);
-		dev_err(&spi->dev, "vio regulator missing (%d)", error);
-		return error;
-	}
-
-	if (!ts->reset_gpio && pdata)
-		ts->set_reset = pdata->set_reset;
-
-	mutex_init(&ts->mutex);
-
-	spin_lock_init(&ts->lock);
-	setup_timer(&ts->penup_timer, tsc2005_penup_timer, (unsigned long)ts);
-
-	INIT_DELAYED_WORK(&ts->esd_work, tsc2005_esd_work);
-
-	snprintf(ts->phys, sizeof(ts->phys),
-		 "%s/input-ts", dev_name(&spi->dev));
-
-	input_dev->name = "TSC2005 touchscreen";
-	input_dev->phys = ts->phys;
-	input_dev->id.bustype = BUS_SPI;
-	input_dev->dev.parent = &spi->dev;
-	input_dev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY);
-	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
-
-	input_set_abs_params(input_dev, ABS_X, 0, max_x, fudge_x, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, max_y, fudge_y, 0);
-	input_set_abs_params(input_dev, ABS_PRESSURE, 0, max_p, fudge_p, 0);
-
-	if (np)
-		touchscreen_parse_properties(input_dev, false);
-
-	input_dev->open = tsc2005_open;
-	input_dev->close = tsc2005_close;
-
-	input_set_drvdata(input_dev, ts);
-
-	/* Ensure the touchscreen is off */
-	tsc2005_stop_scan(ts);
-
-	error = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
-					  tsc2005_irq_thread,
-					  IRQF_TRIGGER_RISING | IRQF_ONESHOT,
-					  "tsc2005", ts);
-	if (error) {
-		dev_err(&spi->dev, "Failed to request irq, err: %d\n", error);
-		return error;
-	}
-
-	/* enable regulator for DT */
-	if (ts->vio) {
-		error = regulator_enable(ts->vio);
-		if (error)
-			return error;
-	}
-
-	dev_set_drvdata(&spi->dev, ts);
-	error = sysfs_create_group(&spi->dev.kobj, &tsc2005_attr_group);
-	if (error) {
-		dev_err(&spi->dev,
-			"Failed to create sysfs attributes, err: %d\n", error);
-		goto disable_regulator;
-	}
-
-	error = input_register_device(ts->idev);
-	if (error) {
-		dev_err(&spi->dev,
-			"Failed to register input device, err: %d\n", error);
-		goto err_remove_sysfs;
-	}
-
-	irq_set_irq_wake(spi->irq, 1);
-	return 0;
-
-err_remove_sysfs:
-	sysfs_remove_group(&spi->dev.kobj, &tsc2005_attr_group);
-disable_regulator:
-	if (ts->vio)
-		regulator_disable(ts->vio);
-	return error;
+	return tsc200x_probe(&spi->dev, spi->irq, BUS_SPI,
+			     devm_regmap_init_spi(spi, &tsc2005_regmap_config));
 }
 
 static int tsc2005_remove(struct spi_device *spi)
 {
-	struct tsc2005 *ts = dev_get_drvdata(&spi->dev);
-
-	sysfs_remove_group(&spi->dev.kobj, &tsc2005_attr_group);
-
-	if (ts->vio)
-		regulator_disable(ts->vio);
-
-	return 0;
-}
-
-static int __maybe_unused tsc2005_suspend(struct device *dev)
-{
-	struct tsc2005 *ts = dev_get_drvdata(dev);
-
-	mutex_lock(&ts->mutex);
-
-	if (!ts->suspended && ts->opened)
-		__tsc2005_disable(ts);
-
-	ts->suspended = true;
-
-	mutex_unlock(&ts->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused tsc2005_resume(struct device *dev)
-{
-	struct tsc2005 *ts = dev_get_drvdata(dev);
-
-	mutex_lock(&ts->mutex);
-
-	if (ts->suspended && ts->opened)
-		__tsc2005_enable(ts);
-
-	ts->suspended = false;
-
-	mutex_unlock(&ts->mutex);
-
-	return 0;
+	return tsc200x_remove(&spi->dev);
 }
 
-static SIMPLE_DEV_PM_OPS(tsc2005_pm_ops, tsc2005_suspend, tsc2005_resume);
+static SIMPLE_DEV_PM_OPS(tsc2005_pm_ops, tsc200x_suspend, tsc200x_resume);
 
 static struct spi_driver tsc2005_driver = {
 	.driver	= {
diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
new file mode 100644
index 0000000..b9c8280
--- /dev/null
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -0,0 +1,790 @@
+/*
+ * TSC2004/TSC2005 touchscreen driver core
+ *
+ * Copyright (C) 2006-2010 Nokia Corporation
+ * Copyright (C) 2015 EMAC Inc.
+ * Copyright (C) 2015 QWERTY Embedded Design
+ *
+ * Author: Lauri Leukkunen <lauri.leukkunen@nokia.com>
+ * based on TSC2301 driver by Klaus K. Pedersen <klaus.k.pedersen@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/pm.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/tsc2005.h>
+#include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
+#include <linux/gpio/consumer.h>
+#include "tsc200x-core.h"
+
+/*
+ * The touchscreen interface operates as follows:
+ *
+ * 1) Pen is pressed against the touchscreen.
+ * 2) TSC2005 performs AD conversion.
+ * 3) After the conversion is done TSC2005 drives DAV line down.
+ * 4) GPIO IRQ is received and tsc2005_irq_thread() is scheduled.
+ * 5) tsc2005_irq_thread() queues up an spi transfer to fetch the x, y, z1, z2
+ *    values.
+ * 6) tsc2005_irq_thread() reports coordinates to input layer and sets up
+ *    tsc2005_penup_timer() to be called after TSC2005_PENUP_TIME_MS (40ms).
+ * 7) When the penup timer expires, there have not been touch or DAV interrupts
+ *    during the last 40ms which means the pen has been lifted.
+ *
+ * ESD recovery via a hardware reset is done if the TSC2005 doesn't respond
+ * after a configurable period (in ms) of activity. If esd_timeout is 0, the
+ * watchdog is disabled.
+ */
+
+/* control byte 1 */
+#define TSC2005_CMD			0x80
+#define TSC2005_CMD_NORMAL		0x00
+#define TSC2005_CMD_STOP		0x01
+#define TSC2005_CMD_12BIT		0x04
+
+/* control byte 0 */
+#define TSC2005_REG_READ		0x01 /* R/W access */
+#define TSC2005_REG_PND0		0x02 /* Power Not Down Control */
+#define TSC2005_REG_X			(0x0 << 3)
+#define TSC2005_REG_Y			(0x1 << 3)
+#define TSC2005_REG_Z1			(0x2 << 3)
+#define TSC2005_REG_Z2			(0x3 << 3)
+#define TSC2005_REG_AUX			(0x4 << 3)
+#define TSC2005_REG_TEMP1		(0x5 << 3)
+#define TSC2005_REG_TEMP2		(0x6 << 3)
+#define TSC2005_REG_STATUS		(0x7 << 3)
+#define TSC2005_REG_AUX_HIGH		(0x8 << 3)
+#define TSC2005_REG_AUX_LOW		(0x9 << 3)
+#define TSC2005_REG_TEMP_HIGH		(0xA << 3)
+#define TSC2005_REG_TEMP_LOW		(0xB << 3)
+#define TSC2005_REG_CFR0		(0xC << 3)
+#define TSC2005_REG_CFR1		(0xD << 3)
+#define TSC2005_REG_CFR2		(0xE << 3)
+#define TSC2005_REG_CONV_FUNC		(0xF << 3)
+
+/* configuration register 0 */
+#define TSC2005_CFR0_PRECHARGE_276US	0x0040
+#define TSC2005_CFR0_STABTIME_1MS	0x0300
+#define TSC2005_CFR0_CLOCK_1MHZ		0x1000
+#define TSC2005_CFR0_RESOLUTION12	0x2000
+#define TSC2005_CFR0_PENMODE		0x8000
+#define TSC2005_CFR0_INITVALUE		(TSC2005_CFR0_STABTIME_1MS    | \
+					 TSC2005_CFR0_CLOCK_1MHZ      | \
+					 TSC2005_CFR0_RESOLUTION12    | \
+					 TSC2005_CFR0_PRECHARGE_276US | \
+					 TSC2005_CFR0_PENMODE)
+
+/* bits common to both read and write of configuration register 0 */
+#define	TSC2005_CFR0_RW_MASK		0x3fff
+
+/* configuration register 1 */
+#define TSC2005_CFR1_BATCHDELAY_4MS	0x0003
+#define TSC2005_CFR1_INITVALUE		TSC2005_CFR1_BATCHDELAY_4MS
+
+/* configuration register 2 */
+#define TSC2005_CFR2_MAVE_Z		0x0004
+#define TSC2005_CFR2_MAVE_Y		0x0008
+#define TSC2005_CFR2_MAVE_X		0x0010
+#define TSC2005_CFR2_AVG_7		0x0800
+#define TSC2005_CFR2_MEDIUM_15		0x3000
+#define TSC2005_CFR2_INITVALUE		(TSC2005_CFR2_MAVE_X	| \
+					 TSC2005_CFR2_MAVE_Y	| \
+					 TSC2005_CFR2_MAVE_Z	| \
+					 TSC2005_CFR2_MEDIUM_15	| \
+					 TSC2005_CFR2_AVG_7)
+
+#define MAX_12BIT			0xfff
+#define TSC2005_DEF_X_FUZZ		4
+#define TSC2005_DEF_Y_FUZZ		8
+#define TSC2005_DEF_P_FUZZ		2
+#define TSC2005_DEF_RESISTOR		280
+
+#define TSC2005_PENUP_TIME_MS		40
+
+static const struct regmap_range tsc2005_writable_ranges[] = {
+	regmap_reg_range(TSC2005_REG_AUX_HIGH, TSC2005_REG_CFR2),
+};
+
+static const struct regmap_access_table tsc2005_writable_table = {
+	.yes_ranges = tsc2005_writable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(tsc2005_writable_ranges),
+};
+
+const struct regmap_config tsc2005_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.reg_stride = 0x08,
+	.max_register = 0x78,
+	.read_flag_mask = TSC2005_REG_READ,
+	.write_flag_mask = TSC2005_REG_PND0,
+	.wr_table = &tsc2005_writable_table,
+	.use_single_rw = true,
+};
+EXPORT_SYMBOL_GPL(tsc2005_regmap_config);
+
+struct tsc2005_data {
+	u16 x;
+	u16 y;
+	u16 z1;
+	u16 z2;
+} __packed;
+#define TSC2005_DATA_REGS 4
+
+struct tsc2005 {
+	struct device		*dev;
+	__u16			bustype;
+
+	struct regmap		*regmap;
+
+	struct input_dev	*idev;
+	char			phys[32];
+
+	struct mutex		mutex;
+
+	/* raw copy of previous x,y,z */
+	int			in_x;
+	int			in_y;
+	int                     in_z1;
+	int			in_z2;
+
+	spinlock_t		lock;
+	struct timer_list	penup_timer;
+
+	unsigned int		esd_timeout;
+	struct delayed_work	esd_work;
+	unsigned long		last_valid_interrupt;
+
+	unsigned int		x_plate_ohm;
+
+	bool			opened;
+	bool			suspended;
+
+	bool			pen_down;
+
+	struct regulator	*vio;
+
+	struct gpio_desc	*reset_gpio;
+	void			(*set_reset)(bool enable);
+
+	int			irq;
+};
+
+#if defined(CONFIG_SPI_MASTER)
+static int tsc2005_cmd_spi(struct tsc2005 *ts, u8 cmd)
+{
+	u8 tx = TSC2005_CMD | TSC2005_CMD_12BIT | cmd;
+	struct spi_transfer xfer = {
+		.tx_buf		= &tx,
+		.len		= 1,
+		.bits_per_word	= 8,
+	};
+	struct spi_message msg;
+	struct spi_device *spi = to_spi_device(ts->dev);
+	int error;
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+
+	error = spi_sync(spi, &msg);
+	if (error) {
+		dev_err(ts->dev, "%s: failed, command: %x, spi error: %d\n",
+			__func__, cmd, error);
+		return error;
+	}
+
+	return 0;
+}
+#endif
+
+#if IS_ENABLED(CONFIG_I2C)
+static int tsc2005_cmd_i2c(struct tsc2005 *ts, u8 cmd)
+{
+	u8 tx = TSC2005_CMD | TSC2005_CMD_12BIT | cmd;
+	s32 data;
+	struct i2c_client *i2c = to_i2c_client(ts->dev);
+
+	data = i2c_smbus_write_byte(i2c, tx);
+	if (data < 0) {
+		dev_err(ts->dev, "%s: failed, command: %x i2c error: %d\n",
+			__func__, cmd, data);
+		return data;
+	}
+
+	return 0;
+}
+#endif
+
+static int tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
+{
+#if defined(CONFIG_SPI_MASTER)
+	if (ts->bustype == BUS_SPI)
+		return tsc2005_cmd_spi(ts, cmd);
+#endif
+
+#if IS_ENABLED(CONFIG_I2C)
+	if (ts->bustype == BUS_I2C)
+		return tsc2005_cmd_i2c(ts, cmd);
+#endif
+
+	return -ENODEV;
+}
+
+static void tsc2005_update_pen_state(struct tsc2005 *ts,
+				     int x, int y, int pressure)
+{
+	if (pressure) {
+		input_report_abs(ts->idev, ABS_X, x);
+		input_report_abs(ts->idev, ABS_Y, y);
+		input_report_abs(ts->idev, ABS_PRESSURE, pressure);
+		if (!ts->pen_down) {
+			input_report_key(ts->idev, BTN_TOUCH, !!pressure);
+			ts->pen_down = true;
+		}
+	} else {
+		input_report_abs(ts->idev, ABS_PRESSURE, 0);
+		if (ts->pen_down) {
+			input_report_key(ts->idev, BTN_TOUCH, 0);
+			ts->pen_down = false;
+		}
+	}
+	input_sync(ts->idev);
+	dev_dbg(ts->dev, "point(%4d,%4d), pressure (%4d)\n", x, y,
+		pressure);
+}
+
+static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
+{
+	struct tsc2005 *ts = _ts;
+	unsigned long flags;
+	unsigned int pressure;
+	struct tsc2005_data tsdata;
+	int error;
+
+	/* read the coordinates */
+	error = regmap_bulk_read(ts->regmap, TSC2005_REG_X, &tsdata,
+				 TSC2005_DATA_REGS);
+	if (unlikely(error))
+		goto out;
+
+	/* validate position */
+	if (unlikely(tsdata.x > MAX_12BIT || tsdata.y > MAX_12BIT))
+		goto out;
+
+	/* Skip reading if the pressure components are out of range */
+	if (unlikely(tsdata.z1 == 0 || tsdata.z2 > MAX_12BIT))
+		goto out;
+	if (unlikely(tsdata.z1 >= tsdata.z2))
+		goto out;
+
+       /*
+	* Skip point if this is a pen down with the exact same values as
+	* the value before pen-up - that implies SPI fed us stale data
+	*/
+	if (!ts->pen_down &&
+	    ts->in_x == tsdata.x && ts->in_y == tsdata.y &&
+	    ts->in_z1 == tsdata.z1 && ts->in_z2 == tsdata.z2) {
+		goto out;
+	}
+
+	/*
+	 * At this point we are happy we have a valid and useful reading.
+	 * Remember it for later comparisons. We may now begin downsampling.
+	 */
+	ts->in_x = tsdata.x;
+	ts->in_y = tsdata.y;
+	ts->in_z1 = tsdata.z1;
+	ts->in_z2 = tsdata.z2;
+
+	/* Compute touch pressure resistance using equation #1 */
+	pressure = tsdata.x * (tsdata.z2 - tsdata.z1) / tsdata.z1;
+	pressure = pressure * ts->x_plate_ohm / 4096;
+	if (unlikely(pressure > MAX_12BIT))
+		goto out;
+
+	spin_lock_irqsave(&ts->lock, flags);
+
+	tsc2005_update_pen_state(ts, tsdata.x, tsdata.y, pressure);
+	mod_timer(&ts->penup_timer,
+		  jiffies + msecs_to_jiffies(TSC2005_PENUP_TIME_MS));
+
+	spin_unlock_irqrestore(&ts->lock, flags);
+
+	ts->last_valid_interrupt = jiffies;
+out:
+	return IRQ_HANDLED;
+}
+
+static void tsc2005_penup_timer(unsigned long data)
+{
+	struct tsc2005 *ts = (struct tsc2005 *)data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ts->lock, flags);
+	tsc2005_update_pen_state(ts, 0, 0, 0);
+	spin_unlock_irqrestore(&ts->lock, flags);
+}
+
+static void tsc2005_start_scan(struct tsc2005 *ts)
+{
+	regmap_write(ts->regmap, TSC2005_REG_CFR0, TSC2005_CFR0_INITVALUE);
+	regmap_write(ts->regmap, TSC2005_REG_CFR1, TSC2005_CFR1_INITVALUE);
+	regmap_write(ts->regmap, TSC2005_REG_CFR2, TSC2005_CFR2_INITVALUE);
+	tsc2005_cmd(ts, TSC2005_CMD_NORMAL);
+}
+
+static void tsc2005_stop_scan(struct tsc2005 *ts)
+{
+	tsc2005_cmd(ts, TSC2005_CMD_STOP);
+}
+
+static void tsc2005_set_reset(struct tsc2005 *ts, bool enable)
+{
+	if (ts->reset_gpio)
+		gpiod_set_value_cansleep(ts->reset_gpio, enable);
+	else if (ts->set_reset)
+		ts->set_reset(enable);
+}
+
+/* must be called with ts->mutex held */
+static void __tsc2005_disable(struct tsc2005 *ts)
+{
+	tsc2005_stop_scan(ts);
+
+	disable_irq(ts->irq);
+	del_timer_sync(&ts->penup_timer);
+
+	cancel_delayed_work_sync(&ts->esd_work);
+
+	enable_irq(ts->irq);
+}
+
+/* must be called with ts->mutex held */
+static void __tsc2005_enable(struct tsc2005 *ts)
+{
+	tsc2005_start_scan(ts);
+
+	if (ts->esd_timeout && (ts->set_reset || ts->reset_gpio)) {
+		ts->last_valid_interrupt = jiffies;
+		schedule_delayed_work(&ts->esd_work,
+				round_jiffies_relative(
+					msecs_to_jiffies(ts->esd_timeout)));
+	}
+
+}
+
+static ssize_t tsc2005_selftest_show(struct device *dev,
+				     struct device_attribute *attr,
+				     char *buf)
+{
+	struct tsc2005 *ts = dev_get_drvdata(dev);
+	unsigned int temp_high;
+	unsigned int temp_high_orig;
+	unsigned int temp_high_test;
+	bool success = true;
+	int error;
+
+	mutex_lock(&ts->mutex);
+
+	/*
+	 * Test TSC2005 communications via temp high register.
+	 */
+	__tsc2005_disable(ts);
+
+	error = regmap_read(ts->regmap, TSC2005_REG_TEMP_HIGH, &temp_high_orig);
+	if (error) {
+		dev_warn(dev, "selftest failed: read error %d\n", error);
+		success = false;
+		goto out;
+	}
+
+	temp_high_test = (temp_high_orig - 1) & MAX_12BIT;
+
+	error = regmap_write(ts->regmap, TSC2005_REG_TEMP_HIGH, temp_high_test);
+	if (error) {
+		dev_warn(dev, "selftest failed: write error %d\n", error);
+		success = false;
+		goto out;
+	}
+
+	error = regmap_read(ts->regmap, TSC2005_REG_TEMP_HIGH, &temp_high);
+	if (error) {
+		dev_warn(dev, "selftest failed: read error %d after write\n",
+			 error);
+		success = false;
+		goto out;
+	}
+
+	if (temp_high != temp_high_test) {
+		dev_warn(dev, "selftest failed: %d != %d\n",
+			 temp_high, temp_high_test);
+		success = false;
+	}
+
+	/* hardware reset */
+	tsc2005_set_reset(ts, false);
+	usleep_range(100, 500); /* only 10us required */
+	tsc2005_set_reset(ts, true);
+
+	if (!success)
+		goto out;
+
+	/* test that the reset really happened */
+	error = regmap_read(ts->regmap, TSC2005_REG_TEMP_HIGH, &temp_high);
+	if (error) {
+		dev_warn(dev, "selftest failed: read error %d after reset\n",
+			 error);
+		success = false;
+		goto out;
+	}
+
+	if (temp_high != temp_high_orig) {
+		dev_warn(dev, "selftest failed after reset: %d != %d\n",
+			 temp_high, temp_high_orig);
+		success = false;
+	}
+
+out:
+	__tsc2005_enable(ts);
+	mutex_unlock(&ts->mutex);
+
+	return sprintf(buf, "%d\n", success);
+}
+
+static DEVICE_ATTR(selftest, S_IRUGO, tsc2005_selftest_show, NULL);
+
+static struct attribute *tsc2005_attrs[] = {
+	&dev_attr_selftest.attr,
+	NULL
+};
+
+static umode_t tsc2005_attr_is_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct tsc2005 *ts = dev_get_drvdata(dev);
+	umode_t mode = attr->mode;
+
+	if (attr == &dev_attr_selftest.attr) {
+		if (!ts->set_reset && !ts->reset_gpio)
+			mode = 0;
+	}
+
+	return mode;
+}
+
+static const struct attribute_group tsc2005_attr_group = {
+	.is_visible	= tsc2005_attr_is_visible,
+	.attrs		= tsc2005_attrs,
+};
+
+static void tsc2005_esd_work(struct work_struct *work)
+{
+	struct tsc2005 *ts = container_of(work, struct tsc2005, esd_work.work);
+	int error;
+	unsigned int r;
+
+	if (!mutex_trylock(&ts->mutex)) {
+		/*
+		 * If the mutex is taken, it means that disable or enable is in
+		 * progress. In that case just reschedule the work. If the work
+		 * is not needed, it will be canceled by disable.
+		 */
+		goto reschedule;
+	}
+
+	if (time_is_after_jiffies(ts->last_valid_interrupt +
+				  msecs_to_jiffies(ts->esd_timeout)))
+		goto out;
+
+	/* We should be able to read register without disabling interrupts. */
+	error = regmap_read(ts->regmap, TSC2005_REG_CFR0, &r);
+	if (!error &&
+	    !((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK)) {
+		goto out;
+	}
+
+	/*
+	 * If we could not read our known value from configuration register 0
+	 * then we should reset the controller as if from power-up and start
+	 * scanning again.
+	 */
+	dev_info(ts->dev, "TSC2005 not responding - resetting\n");
+
+	disable_irq(ts->irq);
+	del_timer_sync(&ts->penup_timer);
+
+	tsc2005_update_pen_state(ts, 0, 0, 0);
+
+	tsc2005_set_reset(ts, false);
+	usleep_range(100, 500); /* only 10us required */
+	tsc2005_set_reset(ts, true);
+
+	enable_irq(ts->irq);
+	tsc2005_start_scan(ts);
+
+out:
+	mutex_unlock(&ts->mutex);
+reschedule:
+	/* re-arm the watchdog */
+	schedule_delayed_work(&ts->esd_work,
+			      round_jiffies_relative(
+					msecs_to_jiffies(ts->esd_timeout)));
+}
+
+static int tsc2005_open(struct input_dev *input)
+{
+	struct tsc2005 *ts = input_get_drvdata(input);
+
+	mutex_lock(&ts->mutex);
+
+	if (!ts->suspended)
+		__tsc2005_enable(ts);
+
+	ts->opened = true;
+
+	mutex_unlock(&ts->mutex);
+
+	return 0;
+}
+
+static void tsc2005_close(struct input_dev *input)
+{
+	struct tsc2005 *ts = input_get_drvdata(input);
+
+	mutex_lock(&ts->mutex);
+
+	if (!ts->suspended)
+		__tsc2005_disable(ts);
+
+	ts->opened = false;
+
+	mutex_unlock(&ts->mutex);
+}
+
+int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
+				struct regmap *regmap)
+{
+	const struct tsc2005_platform_data *pdata = dev_get_platdata(dev);
+	struct device_node *np = dev->of_node;
+
+	struct tsc2005 *ts;
+	struct input_dev *input_dev;
+	unsigned int max_x = MAX_12BIT;
+	unsigned int max_y = MAX_12BIT;
+	unsigned int max_p = MAX_12BIT;
+	unsigned int fudge_x = TSC2005_DEF_X_FUZZ;
+	unsigned int fudge_y = TSC2005_DEF_Y_FUZZ;
+	unsigned int fudge_p = TSC2005_DEF_P_FUZZ;
+	unsigned int x_plate_ohm = TSC2005_DEF_RESISTOR;
+	unsigned int esd_timeout;
+	int error;
+
+	if (!np && !pdata) {
+		dev_err(dev, "no platform data\n");
+		return -ENODEV;
+	}
+
+	if (irq <= 0) {
+		dev_err(dev, "no irq\n");
+		return -ENODEV;
+	}
+
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	if (pdata) {
+		fudge_x	= pdata->ts_x_fudge;
+		fudge_y	= pdata->ts_y_fudge;
+		fudge_p	= pdata->ts_pressure_fudge;
+		max_x	= pdata->ts_x_max;
+		max_y	= pdata->ts_y_max;
+		max_p	= pdata->ts_pressure_max;
+		x_plate_ohm = pdata->ts_x_plate_ohm;
+		esd_timeout = pdata->esd_timeout_ms;
+	} else {
+		x_plate_ohm = TSC2005_DEF_RESISTOR;
+		of_property_read_u32(np, "ti,x-plate-ohms", &x_plate_ohm);
+		esd_timeout = 0;
+		of_property_read_u32(np, "ti,esd-recovery-timeout-ms",
+								&esd_timeout);
+	}
+
+	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	input_dev = devm_input_allocate_device(dev);
+	if (!input_dev)
+		return -ENOMEM;
+
+	ts->irq = irq;
+	ts->dev = dev;
+	ts->idev = input_dev;
+	ts->regmap = regmap;
+	ts->bustype = bustype;
+
+	ts->x_plate_ohm = x_plate_ohm;
+	ts->esd_timeout = esd_timeout;
+
+	ts->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						 GPIOD_OUT_HIGH);
+	if (IS_ERR(ts->reset_gpio)) {
+		error = PTR_ERR(ts->reset_gpio);
+		dev_err(dev, "error acquiring reset gpio: %d\n", error);
+		return error;
+	}
+
+	ts->vio = devm_regulator_get_optional(dev, "vio");
+	if (IS_ERR(ts->vio)) {
+		error = PTR_ERR(ts->vio);
+		dev_err(dev, "vio regulator missing (%d)", error);
+		return error;
+	}
+
+	if (!ts->reset_gpio && pdata)
+		ts->set_reset = pdata->set_reset;
+
+	mutex_init(&ts->mutex);
+
+	spin_lock_init(&ts->lock);
+	setup_timer(&ts->penup_timer, tsc2005_penup_timer, (unsigned long)ts);
+
+	INIT_DELAYED_WORK(&ts->esd_work, tsc2005_esd_work);
+
+	snprintf(ts->phys, sizeof(ts->phys),
+		 "%s/input-ts", dev_name(dev));
+
+	input_dev->name = "TSC2005 touchscreen";
+	input_dev->phys = ts->phys;
+	input_dev->id.bustype = bustype;
+	input_dev->dev.parent = dev;
+	input_dev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY);
+	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+	input_set_abs_params(input_dev, ABS_X, 0, max_x, fudge_x, 0);
+	input_set_abs_params(input_dev, ABS_Y, 0, max_y, fudge_y, 0);
+	input_set_abs_params(input_dev, ABS_PRESSURE, 0, max_p, fudge_p, 0);
+
+	if (np)
+		touchscreen_parse_properties(input_dev, false);
+
+	input_dev->open = tsc2005_open;
+	input_dev->close = tsc2005_close;
+
+	input_set_drvdata(input_dev, ts);
+
+	/* Ensure the touchscreen is off */
+	tsc2005_stop_scan(ts);
+
+	error = devm_request_threaded_irq(dev, irq, NULL,
+					  tsc2005_irq_thread,
+					  IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+					  "tsc2005", ts);
+	if (error) {
+		dev_err(dev, "Failed to request irq, err: %d\n", error);
+		return error;
+	}
+
+	/* enable regulator for DT */
+	if (ts->vio) {
+		error = regulator_enable(ts->vio);
+		if (error)
+			return error;
+	}
+
+	dev_set_drvdata(dev, ts);
+	error = sysfs_create_group(&dev->kobj, &tsc2005_attr_group);
+	if (error) {
+		dev_err(dev,
+			"Failed to create sysfs attributes, err: %d\n", error);
+		goto disable_regulator;
+	}
+
+	error = input_register_device(ts->idev);
+	if (error) {
+		dev_err(dev,
+			"Failed to register input device, err: %d\n", error);
+		goto err_remove_sysfs;
+	}
+
+	irq_set_irq_wake(irq, 1);
+	return 0;
+
+err_remove_sysfs:
+	sysfs_remove_group(&dev->kobj, &tsc2005_attr_group);
+disable_regulator:
+	if (ts->vio)
+		regulator_disable(ts->vio);
+	return error;
+}
+EXPORT_SYMBOL_GPL(tsc200x_probe);
+
+int tsc200x_remove(struct device *dev)
+{
+	struct tsc2005 *ts = dev_get_drvdata(dev);
+
+	sysfs_remove_group(&dev->kobj, &tsc2005_attr_group);
+
+	if (ts->vio)
+		regulator_disable(ts->vio);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tsc200x_remove);
+
+int __maybe_unused tsc200x_suspend(struct device *dev)
+{
+	struct tsc2005 *ts = dev_get_drvdata(dev);
+
+	mutex_lock(&ts->mutex);
+
+	if (!ts->suspended && ts->opened)
+		__tsc2005_disable(ts);
+
+	ts->suspended = true;
+
+	mutex_unlock(&ts->mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tsc200x_suspend);
+
+int __maybe_unused tsc200x_resume(struct device *dev)
+{
+	struct tsc2005 *ts = dev_get_drvdata(dev);
+
+	mutex_lock(&ts->mutex);
+
+	if (ts->suspended && ts->opened)
+		__tsc2005_enable(ts);
+
+	ts->suspended = false;
+
+	mutex_unlock(&ts->mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tsc200x_resume);
+
+MODULE_AUTHOR("Lauri Leukkunen <lauri.leukkunen@nokia.com>");
+MODULE_DESCRIPTION("TSC200X Touchscreen Driver Core");
+MODULE_LICENSE("GPL");
diff --git a/drivers/input/touchscreen/tsc200x-core.h b/drivers/input/touchscreen/tsc200x-core.h
new file mode 100644
index 0000000..58519a2
--- /dev/null
+++ b/drivers/input/touchscreen/tsc200x-core.h
@@ -0,0 +1,13 @@
+#ifndef _TSC200X_CORE_H
+#define _TSC200X_CORE_H
+
+extern const struct regmap_config tsc2005_regmap_config;
+
+int __maybe_unused tsc200x_suspend(struct device *dev);
+int __maybe_unused tsc200x_resume(struct device *dev);
+
+int tsc200x_probe(struct device *dev, int irq, __u16 bustype,
+			 struct regmap *regmap);
+int tsc200x_remove(struct device *dev);
+
+#endif
-- 
2.1.4


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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-29  0:12 [PATCH v3] Input: tsc2005 - Add support for tsc2004 Michael Welling
@ 2015-10-29  7:25 ` kbuild test robot
  2015-10-29 14:22   ` Michael Welling
  2015-10-30  1:45 ` Dmitry Torokhov
  1 sibling, 1 reply; 12+ messages in thread
From: kbuild test robot @ 2015-10-29  7:25 UTC (permalink / raw)
  To: Michael Welling
  Cc: kbuild-all, Dmitry Torokhov, Tony Lindgren, Pavel Machek,
	Felipe Balbi, Sebastian Reichel, Roger Quadros, devicetree,
	linux-kernel, linux-input, Michael Welling

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

Hi Michael,

[auto build test ERROR on input/next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Michael-Welling/Input-tsc2005-Add-support-for-tsc2004/20151029-081504
config: x86_64-randconfig-s1-10291412 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `tsc2005_cmd':
>> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23037 bytes --]

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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-29  7:25 ` kbuild test robot
@ 2015-10-29 14:22   ` Michael Welling
  2015-10-29 21:46     ` Arnd Bergmann
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Welling @ 2015-10-29 14:22 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Dmitry Torokhov, Tony Lindgren, Pavel Machek,
	Felipe Balbi, Sebastian Reichel, Roger Quadros, devicetree,
	linux-kernel, linux-input

On Thu, Oct 29, 2015 at 03:25:34PM +0800, kbuild test robot wrote:
> Hi Michael,
> 
> [auto build test ERROR on input/next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
> 
> url:    https://github.com/0day-ci/linux/commits/Michael-Welling/Input-tsc2005-Add-support-for-tsc2004/20151029-081504
> config: x86_64-randconfig-s1-10291412 (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/built-in.o: In function `tsc2005_cmd':
> >> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'

Argh!

How do I fix this one?

> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation



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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-29 14:22   ` Michael Welling
@ 2015-10-29 21:46     ` Arnd Bergmann
  2015-10-29 21:53       ` Michael Welling
  0 siblings, 1 reply; 12+ messages in thread
From: Arnd Bergmann @ 2015-10-29 21:46 UTC (permalink / raw)
  To: Michael Welling
  Cc: kbuild test robot, kbuild-all, Dmitry Torokhov, Tony Lindgren,
	Pavel Machek, Felipe Balbi, Sebastian Reichel, Roger Quadros,
	devicetree, linux-kernel, linux-input

On Thursday 29 October 2015 09:22:37 Michael Welling wrote:
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    drivers/built-in.o: In function `tsc2005_cmd':
> > >> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'
> 
> Argh!
> 
> How do I fix this one?
> 

Move all the I2C specific code into the tsc2004.c file and remove the
#ifdef. The problem is that tsc200x-core.c is used as built-in when
TOUCHSCREEN_TSC2005=y and I2C=m, so it can't call functions that are
defined in i2c-core.c.

	Arnd

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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-29 21:46     ` Arnd Bergmann
@ 2015-10-29 21:53       ` Michael Welling
  2015-10-29 22:23         ` Dmitry Torokhov
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Welling @ 2015-10-29 21:53 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: kbuild test robot, kbuild-all, Dmitry Torokhov, Tony Lindgren,
	Pavel Machek, Felipe Balbi, Sebastian Reichel, Roger Quadros,
	devicetree, linux-kernel, linux-input

On Thu, Oct 29, 2015 at 10:46:20PM +0100, Arnd Bergmann wrote:
> On Thursday 29 October 2015 09:22:37 Michael Welling wrote:
> > > 
> > > All errors (new ones prefixed by >>):
> > > 
> > >    drivers/built-in.o: In function `tsc2005_cmd':
> > > >> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'
> > 
> > Argh!
> > 
> > How do I fix this one?
> > 
> 
> Move all the I2C specific code into the tsc2004.c file and remove the
> #ifdef. The problem is that tsc200x-core.c is used as built-in when
> TOUCHSCREEN_TSC2005=y and I2C=m, so it can't call functions that are
> defined in i2c-core.c.

How would I call the function from the core then?

-Michael
> 
> 	Arnd

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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-29 21:53       ` Michael Welling
@ 2015-10-29 22:23         ` Dmitry Torokhov
  2015-10-30  0:39           ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2015-10-29 22:23 UTC (permalink / raw)
  To: Michael Welling
  Cc: Arnd Bergmann, kbuild test robot, kbuild-all, Tony Lindgren,
	Pavel Machek, Felipe Balbi, Sebastian Reichel, Roger Quadros,
	devicetree, linux-kernel, linux-input, Mark Brown

On Thu, Oct 29, 2015 at 04:53:53PM -0500, Michael Welling wrote:
> On Thu, Oct 29, 2015 at 10:46:20PM +0100, Arnd Bergmann wrote:
> > On Thursday 29 October 2015 09:22:37 Michael Welling wrote:
> > > > 
> > > > All errors (new ones prefixed by >>):
> > > > 
> > > >    drivers/built-in.o: In function `tsc2005_cmd':
> > > > >> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'
> > > 
> > > Argh!
> > > 
> > > How do I fix this one?
> > > 
> > 
> > Move all the I2C specific code into the tsc2004.c file and remove the
> > #ifdef. The problem is that tsc200x-core.c is used as built-in when
> > TOUCHSCREEN_TSC2005=y and I2C=m, so it can't call functions that are
> > defined in i2c-core.c.
> 
> How would I call the function from the core then?

You can pass a cmd function from the transport module into core module
and store and use it.

However, you have regmap in the driver core already. Mark, is it
possible to have regmap API also allow doing raw underlying protocol
transfer so that consumers could issue command requests without needing
to know if they need to do it over i2c or spi or whatever. Or we need a
notion of command registers in regmap...

Thanks.

-- 
Dmitry

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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-29 22:23         ` Dmitry Torokhov
@ 2015-10-30  0:39           ` Mark Brown
  2015-10-30  1:01             ` Michael Welling
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2015-10-30  0:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Michael Welling, Arnd Bergmann, kbuild test robot, kbuild-all,
	Tony Lindgren, Pavel Machek, Felipe Balbi, Sebastian Reichel,
	Roger Quadros, devicetree, linux-kernel, linux-input

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

On Thu, Oct 29, 2015 at 03:23:31PM -0700, Dmitry Torokhov wrote:

> However, you have regmap in the driver core already. Mark, is it
> possible to have regmap API also allow doing raw underlying protocol
> transfer so that consumers could issue command requests without needing
> to know if they need to do it over i2c or spi or whatever. Or we need a
> notion of command registers in regmap...

I don't think it's a good idea to break the encapsulation of the regmap
and export the raw I/O functionality directly, there seem to be more bad
ways of using that than good.  The driver must at some point know what
bus it is dealing with and be able to manage this itself.

I don't know what "command registers" are.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-30  0:39           ` Mark Brown
@ 2015-10-30  1:01             ` Michael Welling
  2015-10-30 10:40               ` Arnd Bergmann
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Welling @ 2015-10-30  1:01 UTC (permalink / raw)
  To: Mark Brown
  Cc: Dmitry Torokhov, Arnd Bergmann, kbuild test robot, kbuild-all,
	Tony Lindgren, Pavel Machek, Felipe Balbi, Sebastian Reichel,
	Roger Quadros, devicetree, linux-kernel, linux-input

On Fri, Oct 30, 2015 at 09:39:05AM +0900, Mark Brown wrote:
> On Thu, Oct 29, 2015 at 03:23:31PM -0700, Dmitry Torokhov wrote:
> 
> > However, you have regmap in the driver core already. Mark, is it
> > possible to have regmap API also allow doing raw underlying protocol
> > transfer so that consumers could issue command requests without needing
> > to know if they need to do it over i2c or spi or whatever. Or we need a
> > notion of command registers in regmap...
> 
> I don't think it's a good idea to break the encapsulation of the regmap
> and export the raw I/O functionality directly, there seem to be more bad
> ways of using that than good.  The driver must at some point know what
> bus it is dealing with and be able to manage this itself.
> 
> I don't know what "command registers" are.

With this device, if the MSB of the first byte of the transaction is 1 then
a convertor command is encoded in that byte instead of a register address.

So here is my plan:
- Add a function pointer for tsc2005_cmd in the struct tsc2005
- Put the spi and i2c tsc2005_cmd versions in their respective drivers
- Pass the cmd functions to the core via tsc200x_probe

Any objections?
Other review suggestions before I code the revision?
Am I doing too much with a single patch?


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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-29  0:12 [PATCH v3] Input: tsc2005 - Add support for tsc2004 Michael Welling
  2015-10-29  7:25 ` kbuild test robot
@ 2015-10-30  1:45 ` Dmitry Torokhov
  2015-10-30  2:08   ` Michael Welling
  1 sibling, 1 reply; 12+ messages in thread
From: Dmitry Torokhov @ 2015-10-30  1:45 UTC (permalink / raw)
  To: Michael Welling
  Cc: Tony Lindgren, Pavel Machek, Felipe Balbi, Sebastian Reichel,
	Roger Quadros, devicetree, linux-kernel, linux-input

Hi Michael,

On Wed, Oct 28, 2015 at 07:12:34PM -0500, Michael Welling wrote:
> Adds support for the i2c based tsc2004.
> 
> Due to the overlapping functionality of the tsc2004 and tsc2005
> the common code was moved to a core driver (tsc200x-core).
> 
> Signed-off-by: Michael Welling <mwelling@ieee.org>

In addition to what has been discussed in the other email:

> ---
> v3: Splits the tsc2004 and tsc2005 into separate drivers with
> with common routines in tsc200x-core.
> v2: Fixes Kconfig based on report for 0-day build bot.
>  .../bindings/input/touchscreen/tsc2004.txt         |  38 +

Can we please combine tsc2004.txt and tsc2005.txt?

>  
> +config TOUCHSCREEN_TSC200X
> +	tristate

Let's call it TOUCHSCREEN_TSC200X_CORE.

> --- /dev/null
> +++ b/drivers/input/touchscreen/tsc2004.c
> @@ -0,0 +1,73 @@
> +/*
> + * TSC2004 touchscreen driver
> + *
> + * Copyright (C) 2015 EMAC Inc.
> + * Copyright (C) 2015 QWERTY Embedded Design
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *

Please drop this empty line in the comment.

> + */
> +
> +#include <linux/module.h>
> +#include <linux/input.h>
> +#include <linux/pm.h>
> +#include <linux/of.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include "tsc200x-core.h"
> +
> +static int tsc2004_probe(struct i2c_client *i2c,
> +			 const struct i2c_device_id *id)
> +
> +{
> +	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> +			     devm_regmap_init_i2c(i2c,
> +						  &tsc2005_regmap_config));
> +}
> +
> +static int tsc2004_remove(struct i2c_client *i2c)
> +{
> +	return tsc200x_remove(&i2c->dev);
> +}
> +
> +static const struct i2c_device_id tsc2004_idtable[] = {
> +	{ "tsc2004", 0 },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, tsc2004_idtable);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id tsc2004_of_match[] = {
> +	{ .compatible = "ti,tsc2004" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, tsc2004_of_match);
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(tsc2004_pm_ops, tsc200x_suspend, tsc200x_resume);

Hmm, maybe you should export tsc200x_pm_ops instead of individual
functions.

> +
> +int __maybe_unused tsc200x_suspend(struct device *dev)
> +{
> +	struct tsc2005 *ts = dev_get_drvdata(dev);
> +
> +	mutex_lock(&ts->mutex);
> +
> +	if (!ts->suspended && ts->opened)
> +		__tsc2005_disable(ts);
> +
> +	ts->suspended = true;
> +
> +	mutex_unlock(&ts->mutex);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tsc200x_suspend);

__maybe_unused does not make sense here - the symbol is exported and
therefore is always used. If you export the pm_ops stucture then you can
keep functions as __maybe_unused.

BTW, can you generate the patch with -M to let git do the rename
detection - it will be easier to see what changed in the core.

Thanks!

-- 
Dmitry

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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-30  1:45 ` Dmitry Torokhov
@ 2015-10-30  2:08   ` Michael Welling
  2015-10-30  2:35     ` Dmitry Torokhov
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Welling @ 2015-10-30  2:08 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Tony Lindgren, Pavel Machek, Felipe Balbi, Sebastian Reichel,
	Roger Quadros, devicetree, linux-kernel, linux-input

Dmitry,

On Thu, Oct 29, 2015 at 06:45:22PM -0700, Dmitry Torokhov wrote:
> Hi Michael,
> 
> On Wed, Oct 28, 2015 at 07:12:34PM -0500, Michael Welling wrote:
> > Adds support for the i2c based tsc2004.
> > 
> > Due to the overlapping functionality of the tsc2004 and tsc2005
> > the common code was moved to a core driver (tsc200x-core).
> > 
> > Signed-off-by: Michael Welling <mwelling@ieee.org>
> 
> In addition to what has been discussed in the other email:
> 
> > ---
> > v3: Splits the tsc2004 and tsc2005 into separate drivers with
> > with common routines in tsc200x-core.
> > v2: Fixes Kconfig based on report for 0-day build bot.
> >  .../bindings/input/touchscreen/tsc2004.txt         |  38 +
> 
> Can we please combine tsc2004.txt and tsc2005.txt?

Sure.

> 
> >  
> > +config TOUCHSCREEN_TSC200X
> > +	tristate
> 
> Let's call it TOUCHSCREEN_TSC200X_CORE.

Okay.

> 
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/tsc2004.c
> > @@ -0,0 +1,73 @@
> > +/*
> > + * TSC2004 touchscreen driver
> > + *
> > + * Copyright (C) 2015 EMAC Inc.
> > + * Copyright (C) 2015 QWERTY Embedded Design
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> 
> Please drop this empty line in the comment.
>

No problem.
 
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/input.h>
> > +#include <linux/pm.h>
> > +#include <linux/of.h>
> > +#include <linux/i2c.h>
> > +#include <linux/regmap.h>
> > +#include "tsc200x-core.h"
> > +
> > +static int tsc2004_probe(struct i2c_client *i2c,
> > +			 const struct i2c_device_id *id)
> > +
> > +{
> > +	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> > +			     devm_regmap_init_i2c(i2c,
> > +						  &tsc2005_regmap_config));
> > +}
> > +
> > +static int tsc2004_remove(struct i2c_client *i2c)
> > +{
> > +	return tsc200x_remove(&i2c->dev);
> > +}
> > +
> > +static const struct i2c_device_id tsc2004_idtable[] = {
> > +	{ "tsc2004", 0 },
> > +	{ }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(i2c, tsc2004_idtable);
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id tsc2004_of_match[] = {
> > +	{ .compatible = "ti,tsc2004" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, tsc2004_of_match);
> > +#endif
> > +
> > +static SIMPLE_DEV_PM_OPS(tsc2004_pm_ops, tsc200x_suspend, tsc200x_resume);
> 
> Hmm, maybe you should export tsc200x_pm_ops instead of individual
> functions.
> 
> > +
> > +int __maybe_unused tsc200x_suspend(struct device *dev)
> > +{
> > +	struct tsc2005 *ts = dev_get_drvdata(dev);
> > +
> > +	mutex_lock(&ts->mutex);
> > +
> > +	if (!ts->suspended && ts->opened)
> > +		__tsc2005_disable(ts);
> > +
> > +	ts->suspended = true;
> > +
> > +	mutex_unlock(&ts->mutex);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(tsc200x_suspend);
> 
> __maybe_unused does not make sense here - the symbol is exported and
> therefore is always used. If you export the pm_ops stucture then you can
> keep functions as __maybe_unused.
> 
> BTW, can you generate the patch with -M to let git do the rename
> detection - it will be easier to see what changed in the core.

Okay.

Here is somthing that I just saw. The tsc2005_cmd function uses a pointer to
struct tsc2005 as a paramter. So the struct needs to moved into the 
tsc200x-core.h. Should all of the defines be moved there too?

Also the old platform data is in a SPI specific include directory.
Do we want to move this?

Also should I change all of the names of the core functions, variables, structs,
and defines to tsc200x for consistency including the driver struct/defines/etc?

Regards,

Michael
> 
> Thanks!
> 
> -- 
> Dmitry

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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-30  2:08   ` Michael Welling
@ 2015-10-30  2:35     ` Dmitry Torokhov
  0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Torokhov @ 2015-10-30  2:35 UTC (permalink / raw)
  To: Michael Welling
  Cc: Tony Lindgren, Pavel Machek, Felipe Balbi, Sebastian Reichel,
	Roger Quadros, devicetree, linux-kernel, linux-input

On Thu, Oct 29, 2015 at 09:08:45PM -0500, Michael Welling wrote:
> Dmitry,
> 
> On Thu, Oct 29, 2015 at 06:45:22PM -0700, Dmitry Torokhov wrote:
> > Hi Michael,
> > 
> > On Wed, Oct 28, 2015 at 07:12:34PM -0500, Michael Welling wrote:
> > > Adds support for the i2c based tsc2004.
> > > 
> > > Due to the overlapping functionality of the tsc2004 and tsc2005
> > > the common code was moved to a core driver (tsc200x-core).
> > > 
> > > Signed-off-by: Michael Welling <mwelling@ieee.org>
> > 
> > In addition to what has been discussed in the other email:
> > 
> > > ---
> > > v3: Splits the tsc2004 and tsc2005 into separate drivers with
> > > with common routines in tsc200x-core.
> > > v2: Fixes Kconfig based on report for 0-day build bot.
> > >  .../bindings/input/touchscreen/tsc2004.txt         |  38 +
> > 
> > Can we please combine tsc2004.txt and tsc2005.txt?
> 
> Sure.
> 
> > 
> > >  
> > > +config TOUCHSCREEN_TSC200X
> > > +	tristate
> > 
> > Let's call it TOUCHSCREEN_TSC200X_CORE.
> 
> Okay.
> 
> > 
> > > --- /dev/null
> > > +++ b/drivers/input/touchscreen/tsc2004.c
> > > @@ -0,0 +1,73 @@
> > > +/*
> > > + * TSC2004 touchscreen driver
> > > + *
> > > + * Copyright (C) 2015 EMAC Inc.
> > > + * Copyright (C) 2015 QWERTY Embedded Design
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > > + * GNU General Public License for more details.
> > > + *
> > 
> > Please drop this empty line in the comment.
> >
> 
> No problem.
>  
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/input.h>
> > > +#include <linux/pm.h>
> > > +#include <linux/of.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/regmap.h>
> > > +#include "tsc200x-core.h"
> > > +
> > > +static int tsc2004_probe(struct i2c_client *i2c,
> > > +			 const struct i2c_device_id *id)
> > > +
> > > +{
> > > +	return tsc200x_probe(&i2c->dev, i2c->irq, BUS_I2C,
> > > +			     devm_regmap_init_i2c(i2c,
> > > +						  &tsc2005_regmap_config));
> > > +}
> > > +
> > > +static int tsc2004_remove(struct i2c_client *i2c)
> > > +{
> > > +	return tsc200x_remove(&i2c->dev);
> > > +}
> > > +
> > > +static const struct i2c_device_id tsc2004_idtable[] = {
> > > +	{ "tsc2004", 0 },
> > > +	{ }
> > > +};
> > > +
> > > +MODULE_DEVICE_TABLE(i2c, tsc2004_idtable);
> > > +
> > > +#ifdef CONFIG_OF
> > > +static const struct of_device_id tsc2004_of_match[] = {
> > > +	{ .compatible = "ti,tsc2004" },
> > > +	{ /* sentinel */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, tsc2004_of_match);
> > > +#endif
> > > +
> > > +static SIMPLE_DEV_PM_OPS(tsc2004_pm_ops, tsc200x_suspend, tsc200x_resume);
> > 
> > Hmm, maybe you should export tsc200x_pm_ops instead of individual
> > functions.
> > 
> > > +
> > > +int __maybe_unused tsc200x_suspend(struct device *dev)
> > > +{
> > > +	struct tsc2005 *ts = dev_get_drvdata(dev);
> > > +
> > > +	mutex_lock(&ts->mutex);
> > > +
> > > +	if (!ts->suspended && ts->opened)
> > > +		__tsc2005_disable(ts);
> > > +
> > > +	ts->suspended = true;
> > > +
> > > +	mutex_unlock(&ts->mutex);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(tsc200x_suspend);
> > 
> > __maybe_unused does not make sense here - the symbol is exported and
> > therefore is always used. If you export the pm_ops stucture then you can
> > keep functions as __maybe_unused.
> > 
> > BTW, can you generate the patch with -M to let git do the rename
> > detection - it will be easier to see what changed in the core.
> 
> Okay.
> 
> Here is somthing that I just saw. The tsc2005_cmd function uses a pointer to
> struct tsc2005 as a paramter. So the struct needs to moved into the 
> tsc200x-core.h. Should all of the defines be moved there too?

You do not actually need tsc2005 structure in the command function, you
only need the command itself and the generic device structure and then you can
get back to either i2c_client or spi device from it. 

> 
> Also the old platform data is in a SPI specific include directory.
> Do we want to move this?

If you do not mind whipping up an additional patch - sure.

> 
> Also should I change all of the names of the core functions, variables, structs,
> and defines to tsc200x for consistency including the driver struct/defines/etc?

That would be nice, but I'd like it split into different patches if
possible:

1. Split off spi/tsc2005 from the core.
2. Rename core functions to tsc200x.
3. Add tsc2004/i2c module.

Thanks!

-- 
Dmitry

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

* Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004
  2015-10-30  1:01             ` Michael Welling
@ 2015-10-30 10:40               ` Arnd Bergmann
  0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2015-10-30 10:40 UTC (permalink / raw)
  To: Michael Welling
  Cc: Mark Brown, Dmitry Torokhov, kbuild test robot, kbuild-all,
	Tony Lindgren, Pavel Machek, Felipe Balbi, Sebastian Reichel,
	Roger Quadros, devicetree, linux-kernel, linux-input

On Thursday 29 October 2015 20:01:51 Michael Welling wrote:
> On Fri, Oct 30, 2015 at 09:39:05AM +0900, Mark Brown wrote:
> > On Thu, Oct 29, 2015 at 03:23:31PM -0700, Dmitry Torokhov wrote:
> > 
> > > However, you have regmap in the driver core already. Mark, is it
> > > possible to have regmap API also allow doing raw underlying protocol
> > > transfer so that consumers could issue command requests without needing
> > > to know if they need to do it over i2c or spi or whatever. Or we need a
> > > notion of command registers in regmap...
> > 
> > I don't think it's a good idea to break the encapsulation of the regmap
> > and export the raw I/O functionality directly, there seem to be more bad
> > ways of using that than good.  The driver must at some point know what
> > bus it is dealing with and be able to manage this itself.
> > 
> > I don't know what "command registers" are.
> 
> With this device, if the MSB of the first byte of the transaction is 1 then
> a convertor command is encoded in that byte instead of a register address.
> 
> So here is my plan:
> - Add a function pointer for tsc2005_cmd in the struct tsc2005
> - Put the spi and i2c tsc2005_cmd versions in their respective drivers
> - Pass the cmd functions to the core via tsc200x_probe
> 
> Any objections?

Sounds good. If you end up needing more than one function pointer,
better pass a 'const' pointer to a structure of function pointers that
you can declare statically in the driver.

> Other review suggestions before I code the revision?
> Am I doing too much with a single patch?

Yes.

Please split out the DT binding changes into a separate patch, and
start with one patch that just moves the common parts out of the SPI
driver before you add the new driver.

	Arnd

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

end of thread, other threads:[~2015-10-30 10:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-29  0:12 [PATCH v3] Input: tsc2005 - Add support for tsc2004 Michael Welling
2015-10-29  7:25 ` kbuild test robot
2015-10-29 14:22   ` Michael Welling
2015-10-29 21:46     ` Arnd Bergmann
2015-10-29 21:53       ` Michael Welling
2015-10-29 22:23         ` Dmitry Torokhov
2015-10-30  0:39           ` Mark Brown
2015-10-30  1:01             ` Michael Welling
2015-10-30 10:40               ` Arnd Bergmann
2015-10-30  1:45 ` Dmitry Torokhov
2015-10-30  2:08   ` Michael Welling
2015-10-30  2:35     ` Dmitry Torokhov

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