[PATCH 00/17] Merging of TSC2005 driver

[PATCH 00/17] Merging of TSC2005 driver

[Dmitry Torokhov]

Hi,

So I guess the common core for tsc2005/2007 will not materialize anytime
soon and I should finally merge the standalone version of driver... Still
there were a few issues with it and I would appreciate if you take a look
at the following patches and let me know if the driver still works if
you apply them.

Thanks.

-- 
Dmitry

[PATCH 01/17] Input: tsc2005 - use spi_get/set_drvdata()

[Dmitry Torokhov]

Instead of peeking into underlying device and using dev_get/set_drvdata(),
let's use SPI layer's implementation to access driver-private data
(which may be different from driver-core private data).

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

 drivers/input/touchscreen/tsc2005.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index f95f968..5dad30a 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -366,7 +366,8 @@ static void tsc2005_enable(struct tsc2005 *ts)
 static ssize_t tsc2005_disable_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
-	struct tsc2005 *ts = dev_get_drvdata(dev);
+	struct spi_device *spi = to_spi_device(dev);
+	struct tsc2005 *ts = spi_get_drvdata(spi);
 
 	return sprintf(buf, "%u\n", ts->disabled);
 }
@@ -375,7 +376,8 @@ static ssize_t tsc2005_disable_store(struct device *dev,
 				     struct device_attribute *attr,
 				     const char *buf, size_t count)
 {
-	struct tsc2005 *ts = dev_get_drvdata(dev);
+	struct spi_device *spi = to_spi_device(dev);
+	struct tsc2005 *ts = spi_get_drvdata(spi);
 	unsigned long res;
 	int i;
 
@@ -401,7 +403,8 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
 				     struct device_attribute *attr,
 				     char *buf)
 {
-	struct tsc2005 *ts = dev_get_drvdata(dev);
+	struct spi_device *spi = to_spi_device(dev);
+	struct tsc2005 *ts = spi_get_drvdata(spi);
 	u16 temp_high;
 	u16 temp_high_orig;
 	u16 temp_high_test;
@@ -620,7 +623,7 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 	if (ts == NULL)
 		return -ENOMEM;
 
-	dev_set_drvdata(&spi->dev, ts);
+	spi_set_drvdata(spi, ts);
 	ts->spi = spi;
 	spi->dev.power.power_state = PMSG_ON;
 	spi->mode = SPI_MODE_0;
@@ -637,7 +640,7 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 
 static int __devexit tsc2005_remove(struct spi_device *spi)
 {
-	struct tsc2005 *ts = dev_get_drvdata(&spi->dev);
+	struct tsc2005 *ts = spi_get_drvdata(spi);
 
 	mutex_lock(&ts->mutex);
 	tsc2005_disable(ts);
@@ -661,7 +664,8 @@ static int __devexit tsc2005_remove(struct spi_device *spi)
 #ifdef CONFIG_PM
 static int tsc2005_suspend(struct spi_device *spi, pm_message_t mesg)
 {
-	struct tsc2005 *ts = dev_get_drvdata(&spi->dev);
+	struct spi_device *spi = to_spi_device(dev);
+	struct tsc2005 *ts = spi_get_drvdata(spi);
 
 	mutex_lock(&ts->mutex);
 	tsc2005_disable(ts);
@@ -672,7 +676,8 @@ static int tsc2005_suspend(struct spi_device *spi, pm_message_t mesg)
 
 static int tsc2005_resume(struct spi_device *spi)
 {
-	struct tsc2005 *ts = dev_get_drvdata(&spi->dev);
+	struct spi_device *spi = to_spi_device(dev);
+	struct tsc2005 *ts = spi_get_drvdata(spi);
 
 	mutex_lock(&ts->mutex);
 	tsc2005_enable(ts);

[PATCH 02/17] Input: tsc2005 - convert to using dev_pm_ops

[Dmitry Torokhov]

Newer code should not be using legacy suspend/resume methods but
rather supply dev_pm_ops structure as it allows better control
over power management.

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

 drivers/input/touchscreen/tsc2005.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 5dad30a..109efbf 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -27,6 +27,7 @@
 #include <linux/input.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
+#include <linux/pm.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/tsc2005.h>
 
@@ -661,8 +662,8 @@ static int __devexit tsc2005_remove(struct spi_device *spi)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int tsc2005_suspend(struct spi_device *spi, pm_message_t mesg)
+#ifdef CONFIG_PM_SLEEP
+static int tsc2005_suspend(struct device *dev)
 {
 	struct spi_device *spi = to_spi_device(dev);
 	struct tsc2005 *ts = spi_get_drvdata(spi);
@@ -674,7 +675,7 @@ static int tsc2005_suspend(struct spi_device *spi, pm_message_t mesg)
 	return 0;
 }
 
-static int tsc2005_resume(struct spi_device *spi)
+static int tsc2005_resume(struct device *dev)
 {
 	struct spi_device *spi = to_spi_device(dev);
 	struct tsc2005 *ts = spi_get_drvdata(spi);
@@ -687,17 +688,16 @@ static int tsc2005_resume(struct spi_device *spi)
 }
 #endif
 
+static SIMPLE_DEV_PM_OPS(tsc2005_pm_ops, tsc2005_suspend, tsc2005_resume);
+
 static struct spi_driver tsc2005_driver = {
-	.driver = {
-		.name = "tsc2005",
-		.owner = THIS_MODULE,
+	.driver	= {
+		.name	= "tsc2005",
+		.owner	= THIS_MODULE,
+		.pm	= &tsc2005_pm_ops,
 	},
-#ifdef CONFIG_PM
-	.suspend = tsc2005_suspend,
-	.resume = tsc2005_resume,
-#endif
-	.probe = tsc2005_probe,
-	.remove = __devexit_p(tsc2005_remove),
+	.probe	= tsc2005_probe,
+	.remove	= __devexit_p(tsc2005_remove),
 };
 
 static int __init tsc2005_init(void)

[PATCH 03/17] Input: tsc2005 - remove incorrect module alias

[Dmitry Torokhov]

TSC2005 is not a platform driver so it should not define "platform:tsc2005"
module alias.

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

 drivers/input/touchscreen/tsc2005.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 109efbf..8c2e024 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -715,4 +715,3 @@ module_exit(tsc2005_exit);
 
 MODULE_AUTHOR("Lauri Leukkunen <lauri.leukkunen@nokia.com>");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:tsc2005");

[PATCH 04/17] Input: tsc2005 - remove driver banner message

[Dmitry Torokhov]

The boot process is noisy as it is and input core already announces
all new device so let's get rid of the banner message in the driver.

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

 drivers/input/touchscreen/tsc2005.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 8c2e024..2435235 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -702,7 +702,6 @@ static struct spi_driver tsc2005_driver = {
 
 static int __init tsc2005_init(void)
 {
-	printk(KERN_INFO "TSC2005 driver initializing\n");
 	return spi_register_driver(&tsc2005_driver);
 }
 module_init(tsc2005_init);

[PATCH 05/17] Input: tsc2005 - add module description

[Dmitry Torokhov]

Add proper module description so that it would show in 'modinfo'
output.

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

 drivers/input/touchscreen/tsc2005.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 2435235..596fd1f 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -713,4 +713,5 @@ static void __exit tsc2005_exit(void)
 module_exit(tsc2005_exit);
 
 MODULE_AUTHOR("Lauri Leukkunen <lauri.leukkunen@nokia.com>");
+MODULE_DESCRIPTION("TSC2005 Touchscreen Driver");
 MODULE_LICENSE("GPL");

[PATCH 06/17] Input: tsc2005 - clear driver data after unbinding

[Dmitry Torokhov]

We should not leave garbage pointers in driver structure after we unbind
it from the device or if bind fails.

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

 drivers/input/touchscreen/tsc2005.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 596fd1f..732c81e 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -634,8 +634,10 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 	spi_setup(spi);
 
 	r = tsc2005_setup(ts, pdata);
-	if (r)
+	if (r) {
 		kfree(ts);
+		spi_set_drvdata(spi, NULL);
+	}
 	return r;
 }
 
@@ -659,6 +661,7 @@ static int __devexit tsc2005_remove(struct spi_device *spi)
 	input_unregister_device(ts->idev);
 	kfree(ts);
 
+	spi_set_drvdata(spi, NULL);
 	return 0;
 }
 

[PATCH 07/17] Input: tsc2005 - set up parent device

[Dmitry Torokhov]

Set up SPI device as parent of the input device so it gets placed into
proper place in sysfs tree.

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

 drivers/input/touchscreen/tsc2005.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 732c81e..e294648 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -555,6 +555,7 @@ static int __devinit tsc2005_setup(struct tsc2005 *ts,
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input-ts",
 		 dev_name(&ts->spi->dev));
 	ts->idev->phys = ts->phys;
+	ts->idev->dev.parent = &ts->spi->dev;
 	ts->idev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY);
 	ts->idev->absbit[0] = BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE);
 	ts->idev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);

[PATCH 08/17] Input: tsc2005 - set up bus type in input device

[Dmitry Torokhov]

We know what bus we are residing on (SPI) so let's make this data
available to the users.

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

 drivers/input/touchscreen/tsc2005.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index e294648..09cbcb0 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -555,6 +555,7 @@ static int __devinit tsc2005_setup(struct tsc2005 *ts,
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input-ts",
 		 dev_name(&ts->spi->dev));
 	ts->idev->phys = ts->phys;
+	ts->idev->id.bustype = BUS_SPI;
 	ts->idev->dev.parent = &ts->spi->dev;
 	ts->idev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY);
 	ts->idev->absbit[0] = BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE);

[PATCH 09/17] Input: tsc2005 - rework driver initialization code

[Dmitry Torokhov]

We need to make sure we have time/work initialized before requesting and
enabling interrupts, otherwise we might start using them way too early.

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

 drivers/input/touchscreen/tsc2005.c |  213 +++++++++++++++++------------------
 1 files changed, 103 insertions(+), 110 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 09cbcb0..d7ea906 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -495,6 +495,16 @@ out:
 	mutex_unlock(&ts->mutex);
 }
 
+static struct attribute *tsc2005_attrs[] = {
+	&dev_attr_disable.attr,
+	&dev_attr_selftest.attr,
+	NULL
+};
+
+static struct attribute_group tsc2005_attr_group = {
+	.attrs = tsc2005_attrs,
+};
+
 static void __devinit tsc2005_setup_spi_xfer(struct tsc2005 *ts)
 {
 	tsc2005_setup_read(&ts->spi_x, TSC2005_REG_X, 0);
@@ -509,144 +519,128 @@ static void __devinit tsc2005_setup_spi_xfer(struct tsc2005 *ts)
 	spi_message_add_tail(&ts->spi_z2.spi_xfer, &ts->spi_read_msg);
 }
 
-static struct attribute *tsc2005_attrs[] = {
-	&dev_attr_disable.attr,
-	&dev_attr_selftest.attr,
-	NULL
-};
-
-static struct attribute_group tsc2005_attr_group = {
-	.attrs = tsc2005_attrs,
-};
-
-static int __devinit tsc2005_setup(struct tsc2005 *ts,
-				   struct tsc2005_platform_data *pdata)
+static int __devinit tsc2005_probe(struct spi_device *spi)
 {
-	int r;
-	int fudge_x;
-	int fudge_y;
-	int fudge_p;
-	int p_max;
-	int x_max;
-	int y_max;
+	const struct tsc2005_platform_data *pdata = spi->dev.platform_data;
+	struct tsc2005 *ts;
+	struct input_dev *input_dev;
+	unsigned int max_x, max_y, max_p;
+	unsigned int fudge_x, fudge_y, fudge_p;
+	int error;
 
-	mutex_init(&ts->mutex);
+	if (!pdata) {
+		dev_dbg(&spi->dev, "no platform data\n");
+		return -ENODEV;
+	}
 
-	tsc2005_setup_spi_xfer(ts);
+	fudge_x	= pdata->ts_x_fudge	   ? : 4;
+	fudge_y	= pdata->ts_y_fudge	   ? : 8;
+	fudge_p	= pdata->ts_pressure_fudge ? : 2;
+	max_x	= pdata->ts_x_max	   ? : MAX_12BIT;
+	max_y	= pdata->ts_y_max	   ? : MAX_12BIT;
+	max_p	= pdata->ts_pressure_max   ? : MAX_12BIT;
 
-	init_timer(&ts->penup_timer);
-	setup_timer(&ts->penup_timer, tsc2005_penup_timer, (unsigned long)ts);
-	INIT_WORK(&ts->penup_work, tsc2005_penup_work);
+	if (spi->irq <= 0) {
+		dev_dbg(&spi->dev, "no irq\n");
+		return -ENODEV;
+	}
 
-	fudge_x		= pdata->ts_x_fudge	   ? : 4;
-	fudge_y		= pdata->ts_y_fudge	   ? : 8;
-	fudge_p		= pdata->ts_pressure_fudge ? : 2;
-	x_max		= pdata->ts_x_max	   ? : MAX_12BIT;
-	y_max		= pdata->ts_y_max	   ? : MAX_12BIT;
-	p_max		= pdata->ts_pressure_max   ? : MAX_12BIT;
-	ts->x_plate_ohm	= pdata->ts_x_plate_ohm	   ? : 280;
-	ts->esd_timeout	= pdata->esd_timeout_ms;
-	ts->set_reset	= pdata->set_reset;
+	spi->mode = SPI_MODE_0;
+	spi->bits_per_word = 8;
+	if (!spi->max_speed_hz)
+		spi->max_speed_hz = TSC2005_SPI_MAX_SPEED_HZ;
 
-	ts->idev = input_allocate_device();
-	if (ts->idev == NULL)
-		return -ENOMEM;
-	ts->idev->name = "TSC2005 touchscreen";
-	snprintf(ts->phys, sizeof(ts->phys), "%s/input-ts",
-		 dev_name(&ts->spi->dev));
-	ts->idev->phys = ts->phys;
-	ts->idev->id.bustype = BUS_SPI;
-	ts->idev->dev.parent = &ts->spi->dev;
-	ts->idev->evbit[0] = BIT(EV_ABS) | BIT(EV_KEY);
-	ts->idev->absbit[0] = BIT(ABS_X) | BIT(ABS_Y) | BIT(ABS_PRESSURE);
-	ts->idev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
-
-	input_set_abs_params(ts->idev, ABS_X, 0, x_max, fudge_x, 0);
-	input_set_abs_params(ts->idev, ABS_Y, 0, y_max, fudge_y, 0);
-	input_set_abs_params(ts->idev, ABS_PRESSURE, 0, p_max, fudge_p, 0);
-
-	r = request_threaded_irq(ts->spi->irq, tsc2005_irq_handler,
-				 tsc2005_irq_thread, IRQF_TRIGGER_RISING,
-				 "tsc2005", ts);
-	if (r) {
-		dev_err(&ts->spi->dev, "request_threaded_irq(): %d\n", r);
-		goto err1;
-	}
-	set_irq_wake(ts->spi->irq, 1);
+	error = spi_setup(spi);
+	if (error)
+		return error;
 
-	r = input_register_device(ts->idev);
-	if (r) {
-		dev_err(&ts->spi->dev, "input_register_device(): %d\n", r);
-		goto err2;
+	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
+	input_dev = input_allocate_device();
+	if (!ts || !input_dev) {
+		error = -ENOMEM;
+		goto err_free_mem;
 	}
 
-	r = sysfs_create_group(&ts->spi->dev.kobj, &tsc2005_attr_group);
-	if (r)
-		dev_warn(&ts->spi->dev, "sysfs entry creation failed: %d\n", r);
+	ts->spi = spi;
+	ts->x_plate_ohm	= pdata->ts_x_plate_ohm	? : 280;
+	ts->esd_timeout	= pdata->esd_timeout_ms;
+	ts->set_reset	= pdata->set_reset;
 
-	tsc2005_start_scan(ts);
+	mutex_init(&ts->mutex);
 
-	if (!ts->esd_timeout || !ts->set_reset)
-		goto done;
+	setup_timer(&ts->penup_timer, tsc2005_penup_timer, (unsigned long)ts);
+	INIT_WORK(&ts->penup_work, tsc2005_penup_work);
 
-	/* start the optional ESD watchdog */
 	setup_timer(&ts->esd_timer, tsc2005_esd_timer, (unsigned long)ts);
 	INIT_WORK(&ts->esd_work, tsc2005_esd_work);
-	mod_timer(&ts->esd_timer,
-		  round_jiffies(jiffies + msecs_to_jiffies(ts->esd_timeout)));
 
-done:
-	return 0;
-
-err2:
-	free_irq(ts->spi->irq, ts);
-
-err1:
-	input_free_device(ts->idev);
-	return r;
-}
-
-static int __devinit tsc2005_probe(struct spi_device *spi)
-{
-	struct tsc2005_platform_data *pdata = spi->dev.platform_data;
-	struct tsc2005 *ts;
-	int r;
+	tsc2005_setup_spi_xfer(ts);
 
-	if (spi->irq < 0) {
-		dev_dbg(&spi->dev, "no irq\n");
-		return -ENODEV;
+	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);
+
+	error = request_threaded_irq(spi->irq,
+				     tsc2005_irq_handler, tsc2005_irq_thread,
+				     IRQF_TRIGGER_RISING, "tsc2005", ts);
+	if (error) {
+		dev_err(&spi->dev, "Failed to request irq, err: %d\n", error);
+		goto err_free_mem;
 	}
 
-	if (!pdata) {
-		dev_dbg(&spi->dev, "no platform data\n");
-		return -ENODEV;
+	spi_set_drvdata(spi, 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 err_clear_drvdata;
 	}
 
-	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
-	if (ts == NULL)
-		return -ENOMEM;
+	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;
+	}
 
-	spi_set_drvdata(spi, ts);
-	ts->spi = spi;
-	spi->dev.power.power_state = PMSG_ON;
-	spi->mode = SPI_MODE_0;
-	spi->bits_per_word = 8;
-	if (!spi->max_speed_hz)
-		spi->max_speed_hz = TSC2005_SPI_MAX_SPEED_HZ;
-	spi_setup(spi);
+	tsc2005_start_scan(ts);
 
-	r = tsc2005_setup(ts, pdata);
-	if (r) {
-		kfree(ts);
-		spi_set_drvdata(spi, NULL);
+	if (ts->esd_timeout && ts->set_reset) {
+		/* start the optional ESD watchdog */
+		mod_timer(&ts->esd_timer, round_jiffies(jiffies +
+					msecs_to_jiffies(ts->esd_timeout)));
 	}
-	return r;
+
+	set_irq_wake(spi->irq, 1);
+	return 0;
+
+err_remove_sysfs:
+	sysfs_remove_group(&spi->dev.kobj, &tsc2005_attr_group);
+err_clear_drvdata:
+	spi_set_drvdata(spi, NULL);
+	free_irq(spi->irq, ts);
+err_free_mem:
+	input_free_device(input_dev);
+	kfree(ts);
+	return error;
 }
 
 static int __devexit tsc2005_remove(struct spi_device *spi)
 {
 	struct tsc2005 *ts = spi_get_drvdata(spi);
 
+	sysfs_remove_group(&ts->spi->dev.kobj, &tsc2005_attr_group);
+
 	mutex_lock(&ts->mutex);
 	tsc2005_disable(ts);
 	mutex_unlock(&ts->mutex);
@@ -658,7 +652,6 @@ static int __devexit tsc2005_remove(struct spi_device *spi)
 	flush_work(&ts->esd_work);
 	flush_work(&ts->penup_work);
 
-	sysfs_remove_group(&ts->spi->dev.kobj, &tsc2005_attr_group);
 	free_irq(ts->spi->irq, ts);
 	input_unregister_device(ts->idev);
 	kfree(ts);

Re: [PATCH 09/17] Input: tsc2005 - rework driver initialization code

[Aaro Koskinen]

Hi,

> -	r = input_register_device(ts->idev);
> -	if (r) {
> -		dev_err(&ts->spi->dev, "input_register_device(): %d\n", r);
> -		goto err2;
> +	ts = kzalloc(sizeof(*ts), GFP_KERNEL);
> +	input_dev = input_allocate_device();
> +	if (!ts || !input_dev) {
> +		error = -ENOMEM;
> +		goto err_free_mem;
> 	}

ts->idev is never initialized, so input_register_device() will crash.

A.

[PATCH 10/17] Input: tsc2005 - hide selftest attribute if we can't reset

[Dmitry Torokhov]

If implementation to perform self-test/reset has not been provided by the
platform code hide 'selftest' sysfs attribute instead of returning error
when someone tries to use it.

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

 drivers/input/touchscreen/tsc2005.c |   46 +++++++++++++++++++++--------------
 1 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index d7ea906..4f1c041 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -411,13 +411,6 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
 	u16 temp_high_test;
 	unsigned int result;
 
-	if (!ts->set_reset) {
-		dev_warn(&ts->spi->dev,
-			 "unable to selftest: no reset function\n");
-		result = 0;
-		goto out;
-	}
-
 	mutex_lock(&ts->mutex);
 
 	/*
@@ -451,11 +444,38 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
 
 	mutex_unlock(&ts->mutex);
 
-out:
 	return sprintf(buf, "%u\n", result);
 }
+
 static DEVICE_ATTR(selftest, S_IRUGO, tsc2005_selftest_show, NULL);
 
+static struct attribute *tsc2005_attrs[] = {
+	&dev_attr_disable.attr,
+	&dev_attr_selftest.attr,
+	NULL
+};
+
+static mode_t tsc2005_attr_is_visible(struct kobject *kobj,
+				      struct attribute *attr, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct spi_device *spi = to_spi_device(dev);
+	struct tsc2005 *ts = spi_get_drvdata(spi);
+	mode_t mode = attr->mode;
+
+	if (attr == &dev_attr_selftest.attr) {
+		if (!ts->set_reset)
+			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_timer(unsigned long data)
 {
 	struct tsc2005 *ts = (struct tsc2005 *)data;
@@ -495,16 +515,6 @@ out:
 	mutex_unlock(&ts->mutex);
 }
 
-static struct attribute *tsc2005_attrs[] = {
-	&dev_attr_disable.attr,
-	&dev_attr_selftest.attr,
-	NULL
-};
-
-static struct attribute_group tsc2005_attr_group = {
-	.attrs = tsc2005_attrs,
-};
-
 static void __devinit tsc2005_setup_spi_xfer(struct tsc2005 *ts)
 {
 	tsc2005_setup_read(&ts->spi_x, TSC2005_REG_X, 0);

[PATCH 11/17] Input: tsc2005 - use true/false for boolean variables

[Dmitry Torokhov]

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

 drivers/input/touchscreen/tsc2005.c |   25 +++++++++++++------------
 1 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 4f1c041..bfcff96 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -140,7 +140,8 @@ struct tsc2005 {
 
 	bool			disabled;
 	unsigned int		disable_depth;
-	unsigned int		pen_down;
+
+	bool			pen_down;
 
 	void			(*set_reset)(bool enable);
 };
@@ -197,7 +198,7 @@ static void tsc2005_read(struct tsc2005 *ts, u8 reg, u16 *value)
 	struct spi_message msg;
 	struct tsc2005_spi_rd spi_rd = { { 0 }, 0, 0 };
 
-	tsc2005_setup_read(&spi_rd, reg, 1);
+	tsc2005_setup_read(&spi_rd, reg, true);
 
 	spi_message_init(&msg);
 	spi_message_add_tail(&spi_rd.spi_xfer, &msg);
@@ -214,13 +215,13 @@ static void tsc2005_update_pen_state(struct tsc2005 *ts,
 		input_report_abs(ts->idev, ABS_PRESSURE, pressure);
 		if (!ts->pen_down) {
 			input_report_key(ts->idev, BTN_TOUCH, !!pressure);
-			ts->pen_down = 1;
+			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 = 0;
+			ts->pen_down = false;
 		}
 	}
 	input_sync(ts->idev);
@@ -429,9 +430,9 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
 	}
 
 	/* hardware reset */
-	ts->set_reset(0);
+	ts->set_reset(false);
 	usleep_range(100, 500); /* only 10us required */
-	ts->set_reset(1);
+	ts->set_reset(true);
 	tsc2005_enable(ts);
 
 	/* test that the reset really happened */
@@ -500,10 +501,10 @@ static void tsc2005_esd_work(struct work_struct *work)
 	tsc2005_read(ts, TSC2005_REG_CFR0, &r);
 	if ((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK) {
 		dev_info(&ts->spi->dev, "TSC2005 not responding - resetting\n");
-		ts->set_reset(0);
+		ts->set_reset(false);
 		tsc2005_update_pen_state(ts, 0, 0, 0);
 		usleep_range(100, 500); /* only 10us required */
-		ts->set_reset(1);
+		ts->set_reset(true);
 		tsc2005_start_scan(ts);
 	}
 
@@ -517,10 +518,10 @@ out:
 
 static void __devinit tsc2005_setup_spi_xfer(struct tsc2005 *ts)
 {
-	tsc2005_setup_read(&ts->spi_x, TSC2005_REG_X, 0);
-	tsc2005_setup_read(&ts->spi_y, TSC2005_REG_Y, 0);
-	tsc2005_setup_read(&ts->spi_z1, TSC2005_REG_Z1, 0);
-	tsc2005_setup_read(&ts->spi_z2, TSC2005_REG_Z2, 1);
+	tsc2005_setup_read(&ts->spi_x, TSC2005_REG_X, false);
+	tsc2005_setup_read(&ts->spi_y, TSC2005_REG_Y, false);
+	tsc2005_setup_read(&ts->spi_z1, TSC2005_REG_Z1, false);
+	tsc2005_setup_read(&ts->spi_z2, TSC2005_REG_Z2, true);
 
 	spi_message_init(&ts->spi_read_msg);
 	spi_message_add_tail(&ts->spi_x.spi_xfer, &ts->spi_read_msg);

[PATCH 12/17] Input: tsc2005 - do not use 0 in place of NULL

[Dmitry Torokhov]

Sparse in unhappy when people use 0 instead of NULL for pointers so
let's rework the way we initialize spi_transfer structure in
tsc2005_cmd() and tsc2005_write().

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

 drivers/input/touchscreen/tsc2005.c |   36 ++++++++++++++++-------------------
 1 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index bfcff96..8a9c072 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -148,16 +148,13 @@ struct tsc2005 {
 
 static void tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
 {
-	u8 tx;
+	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_transfer xfer = { 0 };
-
-	tx = TSC2005_CMD | TSC2005_CMD_12BIT | cmd;
-
-	xfer.tx_buf = &tx;
-	xfer.rx_buf = NULL;
-	xfer.len = 1;
-	xfer.bits_per_word = 8;
 
 	spi_message_init(&msg);
 	spi_message_add_tail(&xfer, &msg);
@@ -166,17 +163,13 @@ static void tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
 
 static void tsc2005_write(struct tsc2005 *ts, u8 reg, u16 value)
 {
-	u32 tx;
+	u32 tx = ((reg | TSC2005_REG_PND0) << 16) | value;
+	struct spi_transfer xfer = {
+		.tx_buf		= &tx,
+		.len		= 4,
+		.bits_per_word	= 24,
+	};
 	struct spi_message msg;
-	struct spi_transfer xfer = { 0 };
-
-	tx = (reg | TSC2005_REG_PND0) << 16;
-	tx |= value;
-
-	xfer.tx_buf = &tx;
-	xfer.rx_buf = NULL;
-	xfer.len = 4;
-	xfer.bits_per_word = 24;
 
 	spi_message_init(&msg);
 	spi_message_add_tail(&xfer, &msg);
@@ -185,6 +178,8 @@ static void tsc2005_write(struct tsc2005 *ts, u8 reg, u16 value)
 
 static void tsc2005_setup_read(struct tsc2005_spi_rd *rd, u8 reg, bool last)
 {
+	memset(rd, 0, sizeof(*rd));
+
 	rd->spi_tx		   = (reg | TSC2005_REG_READ) << 16;
 	rd->spi_xfer.tx_buf	   = &rd->spi_tx;
 	rd->spi_xfer.rx_buf	   = &rd->spi_rx;
@@ -195,14 +190,15 @@ static void tsc2005_setup_read(struct tsc2005_spi_rd *rd, u8 reg, bool last)
 
 static void tsc2005_read(struct tsc2005 *ts, u8 reg, u16 *value)
 {
+	struct tsc2005_spi_rd spi_rd;
 	struct spi_message msg;
-	struct tsc2005_spi_rd spi_rd = { { 0 }, 0, 0 };
 
 	tsc2005_setup_read(&spi_rd, reg, true);
 
 	spi_message_init(&msg);
 	spi_message_add_tail(&spi_rd.spi_xfer, &msg);
 	spi_sync(ts->spi, &msg);
+
 	*value = spi_rd.spi_rx;
 }
 

[PATCH 13/17] Input: tsc2005 - don't use work for 'pen up' handling

[Dmitry Torokhov]

We do not need process context to send input events so let's switch to
a regular timer. I am going to get rid of taking ts->mutex in
tsc2005_irq_thread() later.

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

 drivers/input/touchscreen/tsc2005.c |   58 ++++++++++++++++-------------------
 1 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 8a9c072..c0980ae 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -129,8 +129,8 @@ struct tsc2005 {
 	int                     in_z1;
 	int			in_z2;
 
+	spinlock_t		lock;
 	struct timer_list	penup_timer;
-	struct work_struct	penup_work;
 
 	unsigned int		esd_timeout;
 	struct timer_list	esd_timer;
@@ -239,11 +239,10 @@ static irqreturn_t tsc2005_irq_handler(int irq, void *dev_id)
 static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
 {
 	struct tsc2005 *ts = _ts;
+	unsigned long flags;
 	unsigned int pressure;
-	u32 x;
-	u32 y;
-	u32 z1;
-	u32 z2;
+	u32 x, y;
+	u32 z1, z2;
 
 	mutex_lock(&ts->mutex);
 
@@ -261,46 +260,50 @@ static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
 	if (unlikely(x > MAX_12BIT || y > MAX_12BIT))
 		goto out;
 
-	/* skip coords if the pressure components are out of range */
+	/* Skip reading if the pressure components are out of range */
 	if (unlikely(z1 == 0 || z2 > MAX_12BIT || z1 >= z2))
 		goto out;
 
-       /* skip point if this is a pen down with the exact same values as
+       /*
+	* 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 == x &&
-	ts->in_y == y &&
-	ts->in_z1 == z1 &&
-	ts->in_z2 == z2)
+	    ts->in_x == x && ts->in_y == y &&
+	    ts->in_z1 == z1 && ts->in_z2 == 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
-	*/
+	/*
+	 * 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 = x;
 	ts->in_y = y;
 	ts->in_z1 = z1;
 	ts->in_z2 = z2;
 
-	/* compute touch pressure resistance using equation #1 */
+	/* Compute touch pressure resistance using equation #1 */
 	pressure = x * (z2 - z1) / 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, x, y, pressure);
 
 	/* set the penup timer */
 	mod_timer(&ts->penup_timer,
 		  jiffies + msecs_to_jiffies(TSC2005_PENUP_TIME_MS));
 
-	if (!ts->esd_timeout)
-		goto out;
+	if (ts->esd_timeout && ts->set_reset) {
+		/* update the watchdog timer */
+		mod_timer(&ts->esd_timer, round_jiffies(jiffies +
+					msecs_to_jiffies(ts->esd_timeout)));
+	}
 
-	/* update the watchdog timer */
-	mod_timer(&ts->esd_timer,
-		  round_jiffies(jiffies + msecs_to_jiffies(ts->esd_timeout)));
+	spin_unlock_irqrestore(&ts->lock, flags);
 
 out:
 	mutex_unlock(&ts->mutex);
@@ -310,17 +313,11 @@ out:
 static void tsc2005_penup_timer(unsigned long data)
 {
 	struct tsc2005 *ts = (struct tsc2005 *)data;
+	unsigned long flags;
 
-	schedule_work(&ts->penup_work);
-}
-
-static void tsc2005_penup_work(struct work_struct *work)
-{
-	struct tsc2005 *ts = container_of(work, struct tsc2005, penup_work);
-
-	mutex_lock(&ts->mutex);
+	spin_lock_irqsave(&ts->lock, flags);
 	tsc2005_update_pen_state(ts, 0, 0, 0);
-	mutex_unlock(&ts->mutex);
+	spin_unlock_irqrestore(&ts->lock, flags);
 }
 
 static void tsc2005_start_scan(struct tsc2005 *ts)
@@ -575,8 +572,8 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 
 	mutex_init(&ts->mutex);
 
+	spin_lock_init(&ts->lock);
 	setup_timer(&ts->penup_timer, tsc2005_penup_timer, (unsigned long)ts);
-	INIT_WORK(&ts->penup_work, tsc2005_penup_work);
 
 	setup_timer(&ts->esd_timer, tsc2005_esd_timer, (unsigned long)ts);
 	INIT_WORK(&ts->esd_work, tsc2005_esd_work);
@@ -657,7 +654,6 @@ static int __devexit tsc2005_remove(struct spi_device *spi)
 	del_timer_sync(&ts->penup_timer);
 
 	flush_work(&ts->esd_work);
-	flush_work(&ts->penup_work);
 
 	free_irq(ts->spi->irq, ts);
 	input_unregister_device(ts->idev);

[PATCH 14/17] Input: tsc2005 - do not rearm timer in hardirq handler

[Dmitry Torokhov]

We will most likely rearm it yet again the IRQ thread so doing it here
is pointless.

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

 drivers/input/touchscreen/tsc2005.c |   14 +-------------
 1 files changed, 1 insertions(+), 13 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index c0980ae..16c9f57 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -225,17 +225,6 @@ static void tsc2005_update_pen_state(struct tsc2005 *ts,
 		pressure);
 }
 
-static irqreturn_t tsc2005_irq_handler(int irq, void *dev_id)
-{
-	struct tsc2005 *ts = dev_id;
-
-	/* update the penup timer only if it's pending */
-	mod_timer_pending(&ts->penup_timer,
-			  jiffies + msecs_to_jiffies(TSC2005_PENUP_TIME_MS));
-
-	return IRQ_WAKE_THREAD;
-}
-
 static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
 {
 	struct tsc2005 *ts = _ts;
@@ -594,8 +583,7 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 	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);
 
-	error = request_threaded_irq(spi->irq,
-				     tsc2005_irq_handler, tsc2005_irq_thread,
+	error = request_threaded_irq(spi->irq, NULL, tsc2005_irq_thread,
 				     IRQF_TRIGGER_RISING, "tsc2005", ts);
 	if (error) {
 		dev_err(&spi->dev, "Failed to request irq, err: %d\n", error);

[PATCH 15/17] Input: tsc2005 - handle read errors from SPI layer

[Dmitry Torokhov]

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

 drivers/input/touchscreen/tsc2005.c |  100 ++++++++++++++++++++++++++++-------
 1 files changed, 81 insertions(+), 19 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 16c9f57..f72df969 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -146,7 +146,7 @@ struct tsc2005 {
 	void			(*set_reset)(bool enable);
 };
 
-static void tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
+static int tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
 {
 	u8 tx = TSC2005_CMD | TSC2005_CMD_12BIT | cmd;
 	struct spi_transfer xfer = {
@@ -155,13 +155,22 @@ static void tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
 		.bits_per_word	= 8,
 	};
 	struct spi_message msg;
+	int error;
 
 	spi_message_init(&msg);
 	spi_message_add_tail(&xfer, &msg);
-	spi_sync(ts->spi, &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_write(struct tsc2005 *ts, u8 reg, u16 value)
+static int tsc2005_write(struct tsc2005 *ts, u8 reg, u16 value)
 {
 	u32 tx = ((reg | TSC2005_REG_PND0) << 16) | value;
 	struct spi_transfer xfer = {
@@ -170,10 +179,20 @@ static void tsc2005_write(struct tsc2005 *ts, u8 reg, u16 value)
 		.bits_per_word	= 24,
 	};
 	struct spi_message msg;
+	int error;
 
 	spi_message_init(&msg);
 	spi_message_add_tail(&xfer, &msg);
-	spi_sync(ts->spi, &msg);
+
+	error = spi_sync(ts->spi, &msg);
+	if (error) {
+		dev_err(&ts->spi->dev,
+			"%s: failed, register: %x, value: %x, error: %d\n",
+			__func__, reg, value, error);
+		return error;
+	}
+
+	return 0;
 }
 
 static void tsc2005_setup_read(struct tsc2005_spi_rd *rd, u8 reg, bool last)
@@ -188,18 +207,23 @@ static void tsc2005_setup_read(struct tsc2005_spi_rd *rd, u8 reg, bool last)
 	rd->spi_xfer.cs_change	   = !last;
 }
 
-static void tsc2005_read(struct tsc2005 *ts, u8 reg, u16 *value)
+static int tsc2005_read(struct tsc2005 *ts, u8 reg, u16 *value)
 {
 	struct tsc2005_spi_rd spi_rd;
 	struct spi_message msg;
+	int error;
 
 	tsc2005_setup_read(&spi_rd, reg, true);
 
 	spi_message_init(&msg);
 	spi_message_add_tail(&spi_rd.spi_xfer, &msg);
-	spi_sync(ts->spi, &msg);
+
+	error = spi_sync(ts->spi, &msg);
+	if (error)
+		return error;
 
 	*value = spi_rd.spi_rx;
+	return 0;
 }
 
 static void tsc2005_update_pen_state(struct tsc2005 *ts,
@@ -232,6 +256,7 @@ static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
 	unsigned int pressure;
 	u32 x, y;
 	u32 z1, z2;
+	int error;
 
 	mutex_lock(&ts->mutex);
 
@@ -239,7 +264,10 @@ static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
 		goto out;
 
 	/* read the coordinates */
-	spi_sync(ts->spi, &ts->spi_read_msg);
+	error = spi_sync(ts->spi, &ts->spi_read_msg);
+	if (unlikely(error))
+		goto out;
+
 	x = ts->spi_x.spi_rx;
 	y = ts->spi_y.spi_rx;
 	z1 = ts->spi_z1.spi_rx;
@@ -392,7 +420,8 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
 	u16 temp_high;
 	u16 temp_high_orig;
 	u16 temp_high_test;
-	unsigned int result;
+	bool success = true;
+	int error;
 
 	mutex_lock(&ts->mutex);
 
@@ -400,34 +429,65 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
 	 * Test TSC2005 communications via temp high register.
 	 */
 	tsc2005_disable(ts);
-	result = 1;
-	tsc2005_read(ts, TSC2005_REG_TEMP_HIGH, &temp_high_orig);
+
+	error = tsc2005_read(ts, 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;
-	tsc2005_write(ts, TSC2005_REG_TEMP_HIGH, temp_high_test);
-	tsc2005_read(ts, TSC2005_REG_TEMP_HIGH, &temp_high);
+
+	error = tsc2005_write(ts, TSC2005_REG_TEMP_HIGH, temp_high_test);
+	if (error) {
+		dev_warn(dev, "selftest failed: write error %d\n", error);
+		success = false;
+		goto out;
+	}
+
+	error = tsc2005_read(ts, 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);
-		result = 0;
+		success = false;
 	}
 
 	/* hardware reset */
 	ts->set_reset(false);
 	usleep_range(100, 500); /* only 10us required */
 	ts->set_reset(true);
-	tsc2005_enable(ts);
+
+	if (!success)
+		goto out;
 
 	/* test that the reset really happened */
-	tsc2005_read(ts, TSC2005_REG_TEMP_HIGH, &temp_high);
+	error = tsc2005_read(ts, 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);
-		result = 0;
+		success = false;
 	}
 
+out:
+	tsc2005_enable(ts);
 	mutex_unlock(&ts->mutex);
 
-	return sprintf(buf, "%u\n", result);
+	return sprintf(buf, "%d\n", success);
 }
 
 static DEVICE_ATTR(selftest, S_IRUGO, tsc2005_selftest_show, NULL);
@@ -469,6 +529,7 @@ static void tsc2005_esd_timer(unsigned long data)
 static void tsc2005_esd_work(struct work_struct *work)
 {
 	struct tsc2005 *ts = container_of(work, struct tsc2005, esd_work);
+	int error;
 	u16 r;
 
 	mutex_lock(&ts->mutex);
@@ -480,8 +541,9 @@ static void tsc2005_esd_work(struct work_struct *work)
 	 * If we cannot read our known value from configuration register 0 then
 	 * reset the controller as if from power-up and start scanning again.
 	 */
-	tsc2005_read(ts, TSC2005_REG_CFR0, &r);
-	if ((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK) {
+	error = tsc2005_read(ts, TSC2005_REG_CFR0, &r);
+	if (error ||
+	    ((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK)) {
 		dev_info(&ts->spi->dev, "TSC2005 not responding - resetting\n");
 		ts->set_reset(false);
 		tsc2005_update_pen_state(ts, 0, 0, 0);

[PATCH 16/17] Input: tsc2005 - add open/close

[Dmitry Torokhov]

Introduce open and close methods for the input device to keep the device
powered down when it is not in use. Also rework interaction between
interrupt thread and starting/shutting off/resetting the device: instead
of taking a mutex in the intterrupt thread and elsewhere disable interrupts
before transitioning the device in a new state.

The ESD handling is also separated from the IRQ thread; we poll regularly
at a given interval and simply skip reads if we see that valid interrupt
happened not so long ago. This allows us not cancel and reschedule ESD
work from interrupt context all the time.

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

 drivers/input/touchscreen/tsc2005.c |  216 ++++++++++++++++++++---------------
 1 files changed, 122 insertions(+), 94 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index f72df969..2e8c063 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -133,13 +133,14 @@ struct tsc2005 {
 	struct timer_list	penup_timer;
 
 	unsigned int		esd_timeout;
-	struct timer_list	esd_timer;
-	struct work_struct	esd_work;
+	struct delayed_work	esd_work;
+	unsigned long		last_valid_interrupt;
 
 	unsigned int		x_plate_ohm;
 
 	bool			disabled;
-	unsigned int		disable_depth;
+	bool			opened;
+	bool			suspended;
 
 	bool			pen_down;
 
@@ -258,11 +259,6 @@ static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
 	u32 z1, z2;
 	int error;
 
-	mutex_lock(&ts->mutex);
-
-	if (unlikely(ts->disable_depth))
-		goto out;
-
 	/* read the coordinates */
 	error = spi_sync(ts->spi, &ts->spi_read_msg);
 	if (unlikely(error))
@@ -309,21 +305,13 @@ static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
 	spin_lock_irqsave(&ts->lock, flags);
 
 	tsc2005_update_pen_state(ts, x, y, pressure);
-
-	/* set the penup timer */
 	mod_timer(&ts->penup_timer,
 		  jiffies + msecs_to_jiffies(TSC2005_PENUP_TIME_MS));
 
-	if (ts->esd_timeout && ts->set_reset) {
-		/* update the watchdog timer */
-		mod_timer(&ts->esd_timer, round_jiffies(jiffies +
-					msecs_to_jiffies(ts->esd_timeout)));
-	}
-
 	spin_unlock_irqrestore(&ts->lock, flags);
 
+	ts->last_valid_interrupt = jiffies;
 out:
-	mutex_unlock(&ts->mutex);
 	return IRQ_HANDLED;
 }
 
@@ -350,29 +338,31 @@ static void tsc2005_stop_scan(struct tsc2005 *ts)
 	tsc2005_cmd(ts, TSC2005_CMD_STOP);
 }
 
-/* must be called with mutex held */
-static void tsc2005_disable(struct tsc2005 *ts)
+/* must be called with ts->mutex held */
+static void __tsc2005_disable(struct tsc2005 *ts)
 {
-	if (ts->disable_depth++ != 0)
-		return;
+	tsc2005_stop_scan(ts);
+
 	disable_irq(ts->spi->irq);
-	if (ts->esd_timeout)
-		del_timer_sync(&ts->esd_timer);
 	del_timer_sync(&ts->penup_timer);
-	tsc2005_stop_scan(ts);
+
+	cancel_delayed_work_sync(&ts->esd_work);
+
+	enable_irq(ts->spi->irq);
 }
 
-/* must be called with mutex held */
-static void tsc2005_enable(struct tsc2005 *ts)
+/* must be called with ts->mutex held */
+static void __tsc2005_enable(struct tsc2005 *ts)
 {
-	if (--ts->disable_depth != 0)
-		return;
 	tsc2005_start_scan(ts);
-	enable_irq(ts->spi->irq);
-	if (!ts->esd_timeout)
-		return;
-	mod_timer(&ts->esd_timer,
-		  round_jiffies(jiffies + msecs_to_jiffies(ts->esd_timeout)));
+
+	if (ts->esd_timeout && ts->set_reset) {
+		ts->last_valid_interrupt = jiffies;
+		schedule_delayed_work(&ts->esd_work,
+				round_jiffies(jiffies +
+					msecs_to_jiffies(ts->esd_timeout)));
+	}
+
 }
 
 static ssize_t tsc2005_disable_show(struct device *dev,
@@ -390,23 +380,29 @@ static ssize_t tsc2005_disable_store(struct device *dev,
 {
 	struct spi_device *spi = to_spi_device(dev);
 	struct tsc2005 *ts = spi_get_drvdata(spi);
-	unsigned long res;
-	int i;
+	unsigned long val;
+	int error;
 
-	if (strict_strtoul(buf, 10, &res) < 0)
-		return -EINVAL;
-	i = res ? 1 : 0;
+	error = strict_strtoul(buf, 10, &val);
+	if (error)
+		return error;
 
 	mutex_lock(&ts->mutex);
-	if (i == ts->disabled)
-		goto out;
-	ts->disabled = i;
-	if (i)
-		tsc2005_disable(ts);
-	else
-		tsc2005_enable(ts);
-out:
+
+	if (!ts->suspended && ts->opened) {
+		if (val) {
+			if (!ts->disabled)
+				__tsc2005_disable(ts);
+		} else {
+			if (ts->disabled)
+				__tsc2005_enable(ts);
+		}
+	}
+
+	ts->disabled = !!val;
+
 	mutex_unlock(&ts->mutex);
+
 	return count;
 }
 static DEVICE_ATTR(disable, 0664, tsc2005_disable_show, tsc2005_disable_store);
@@ -428,7 +424,7 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
 	/*
 	 * Test TSC2005 communications via temp high register.
 	 */
-	tsc2005_disable(ts);
+	__tsc2005_disable(ts);
 
 	error = tsc2005_read(ts, TSC2005_REG_TEMP_HIGH, &temp_high_orig);
 	if (error) {
@@ -484,7 +480,7 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
 	}
 
 out:
-	tsc2005_enable(ts);
+	__tsc2005_enable(ts);
 	mutex_unlock(&ts->mutex);
 
 	return sprintf(buf, "%d\n", success);
@@ -519,44 +515,79 @@ static const struct attribute_group tsc2005_attr_group = {
 	.attrs		= tsc2005_attrs,
 };
 
-static void tsc2005_esd_timer(unsigned long data)
-{
-	struct tsc2005 *ts = (struct tsc2005 *)data;
-
-	schedule_work(&ts->esd_work);
-}
-
 static void tsc2005_esd_work(struct work_struct *work)
 {
-	struct tsc2005 *ts = container_of(work, struct tsc2005, esd_work);
+	struct tsc2005 *ts = container_of(work, struct tsc2005, esd_work.work);
 	int error;
 	u16 r;
 
 	mutex_lock(&ts->mutex);
 
-	if (ts->disable_depth)
+	if (time_is_after_jiffies(ts->last_valid_interrupt +
+				  msecs_to_jiffies(ts->esd_timeout)))
 		goto out;
 
-	/*
-	 * If we cannot read our known value from configuration register 0 then
-	 * reset the controller as if from power-up and start scanning again.
-	 */
+	/* We should be able to read register without disabling interrupts. */
 	error = tsc2005_read(ts, TSC2005_REG_CFR0, &r);
-	if (error ||
-	    ((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK)) {
-		dev_info(&ts->spi->dev, "TSC2005 not responding - resetting\n");
-		ts->set_reset(false);
-		tsc2005_update_pen_state(ts, 0, 0, 0);
-		usleep_range(100, 500); /* only 10us required */
-		ts->set_reset(true);
-		tsc2005_start_scan(ts);
+	if (!error &&
+	    !((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK)) {
+		goto out;
 	}
 
-	/* re-arm the watchdog */
-	mod_timer(&ts->esd_timer,
-		  round_jiffies(jiffies + msecs_to_jiffies(ts->esd_timeout)));
+	/*
+	 * 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);
+
+	ts->set_reset(false);
+	usleep_range(100, 500); /* only 10us required */
+	ts->set_reset(true);
+
+	enable_irq(ts->spi->irq);
+	tsc2005_start_scan(ts);
 
 out:
+	/* re-arm the watchdog */
+	schedule_delayed_work(&ts->esd_work,
+			      round_jiffies(jiffies +
+					msecs_to_jiffies(ts->esd_timeout)));
+	mutex_unlock(&ts->mutex);
+}
+
+static int tsc2005_open(struct input_dev *input)
+{
+	struct tsc2005 *ts = input_get_drvdata(input);
+
+	mutex_lock(&ts->mutex);
+
+	if (!ts->suspended && !ts->disabled)
+		__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 && !ts->disabled)
+		__tsc2005_disable(ts);
+
+	ts->opened = false;
+
 	mutex_unlock(&ts->mutex);
 }
 
@@ -626,8 +657,7 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 	spin_lock_init(&ts->lock);
 	setup_timer(&ts->penup_timer, tsc2005_penup_timer, (unsigned long)ts);
 
-	setup_timer(&ts->esd_timer, tsc2005_esd_timer, (unsigned long)ts);
-	INIT_WORK(&ts->esd_work, tsc2005_esd_work);
+	INIT_DELAYED_WORK(&ts->esd_work, tsc2005_esd_work);
 
 	tsc2005_setup_spi_xfer(ts);
 
@@ -645,6 +675,12 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 	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);
 
+	input_dev->open = tsc2005_open;
+	input_dev->close = tsc2005_close;
+
+	/* Ensure the touchscreen is off */
+	tsc2005_stop_scan(ts);
+
 	error = request_threaded_irq(spi->irq, NULL, tsc2005_irq_thread,
 				     IRQF_TRIGGER_RISING, "tsc2005", ts);
 	if (error) {
@@ -667,14 +703,6 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 		goto err_remove_sysfs;
 	}
 
-	tsc2005_start_scan(ts);
-
-	if (ts->esd_timeout && ts->set_reset) {
-		/* start the optional ESD watchdog */
-		mod_timer(&ts->esd_timer, round_jiffies(jiffies +
-					msecs_to_jiffies(ts->esd_timeout)));
-	}
-
 	set_irq_wake(spi->irq, 1);
 	return 0;
 
@@ -695,16 +723,6 @@ static int __devexit tsc2005_remove(struct spi_device *spi)
 
 	sysfs_remove_group(&ts->spi->dev.kobj, &tsc2005_attr_group);
 
-	mutex_lock(&ts->mutex);
-	tsc2005_disable(ts);
-	mutex_unlock(&ts->mutex);
-
-	if (ts->esd_timeout)
-		del_timer_sync(&ts->esd_timer);
-	del_timer_sync(&ts->penup_timer);
-
-	flush_work(&ts->esd_work);
-
 	free_irq(ts->spi->irq, ts);
 	input_unregister_device(ts->idev);
 	kfree(ts);
@@ -720,7 +738,12 @@ static int tsc2005_suspend(struct device *dev)
 	struct tsc2005 *ts = spi_get_drvdata(spi);
 
 	mutex_lock(&ts->mutex);
-	tsc2005_disable(ts);
+
+	if (!ts->suspended && !ts->disabled && ts->opened)
+		__tsc2005_disable(ts);
+
+	ts->suspended = true;
+
 	mutex_unlock(&ts->mutex);
 
 	return 0;
@@ -732,7 +755,12 @@ static int tsc2005_resume(struct device *dev)
 	struct tsc2005 *ts = spi_get_drvdata(spi);
 
 	mutex_lock(&ts->mutex);
-	tsc2005_enable(ts);
+
+	if (ts->suspended && !ts->disabled && ts->opened)
+		__tsc2005_enable(ts);
+
+	ts->suspended = false;
+
 	mutex_unlock(&ts->mutex);
 
 	return 0;

Re: [PATCH 16/17] Input: tsc2005 - add open/close

[Aaro Koskinen]

Hi,

On Wed, 16 Mar 2011, Dmitry Torokhov wrote:
> Introduce open and close methods for the input device to keep the device
> powered down when it is not in use. Also rework interaction between
> interrupt thread and starting/shutting off/resetting the device: instead
> of taking a mutex in the intterrupt thread and elsewhere disable interrupts
> before transitioning the device in a new state.

The driver is missing input_set_drvdata() (I guess that's the function),
so opening the device will crash.

> The ESD handling is also separated from the IRQ thread; we poll regularly
> at a given interval and simply skip reads if we see that valid interrupt
> happened not so long ago. This allows us not cancel and reschedule ESD
> work from interrupt context all the time.
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>
> drivers/input/touchscreen/tsc2005.c |  216 ++++++++++++++++++++---------------
> 1 files changed, 122 insertions(+), 94 deletions(-)
>
> diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
> index f72df969..2e8c063 100644
> --- a/drivers/input/touchscreen/tsc2005.c
> +++ b/drivers/input/touchscreen/tsc2005.c
> @@ -133,13 +133,14 @@ struct tsc2005 {
> 	struct timer_list	penup_timer;
>
> 	unsigned int		esd_timeout;
> -	struct timer_list	esd_timer;
> -	struct work_struct	esd_work;
> +	struct delayed_work	esd_work;
> +	unsigned long		last_valid_interrupt;
>
> 	unsigned int		x_plate_ohm;
>
> 	bool			disabled;
> -	unsigned int		disable_depth;
> +	bool			opened;
> +	bool			suspended;
>
> 	bool			pen_down;
>
> @@ -258,11 +259,6 @@ static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
> 	u32 z1, z2;
> 	int error;
>
> -	mutex_lock(&ts->mutex);
> -
> -	if (unlikely(ts->disable_depth))
> -		goto out;
> -
> 	/* read the coordinates */
> 	error = spi_sync(ts->spi, &ts->spi_read_msg);
> 	if (unlikely(error))
> @@ -309,21 +305,13 @@ static irqreturn_t tsc2005_irq_thread(int irq, void *_ts)
> 	spin_lock_irqsave(&ts->lock, flags);
>
> 	tsc2005_update_pen_state(ts, x, y, pressure);
> -
> -	/* set the penup timer */
> 	mod_timer(&ts->penup_timer,
> 		  jiffies + msecs_to_jiffies(TSC2005_PENUP_TIME_MS));
>
> -	if (ts->esd_timeout && ts->set_reset) {
> -		/* update the watchdog timer */
> -		mod_timer(&ts->esd_timer, round_jiffies(jiffies +
> -					msecs_to_jiffies(ts->esd_timeout)));
> -	}
> -
> 	spin_unlock_irqrestore(&ts->lock, flags);
>
> +	ts->last_valid_interrupt = jiffies;
> out:
> -	mutex_unlock(&ts->mutex);
> 	return IRQ_HANDLED;
> }
>
> @@ -350,29 +338,31 @@ static void tsc2005_stop_scan(struct tsc2005 *ts)
> 	tsc2005_cmd(ts, TSC2005_CMD_STOP);
> }
>
> -/* must be called with mutex held */
> -static void tsc2005_disable(struct tsc2005 *ts)
> +/* must be called with ts->mutex held */
> +static void __tsc2005_disable(struct tsc2005 *ts)
> {
> -	if (ts->disable_depth++ != 0)
> -		return;
> +	tsc2005_stop_scan(ts);
> +
> 	disable_irq(ts->spi->irq);
> -	if (ts->esd_timeout)
> -		del_timer_sync(&ts->esd_timer);
> 	del_timer_sync(&ts->penup_timer);
> -	tsc2005_stop_scan(ts);
> +
> +	cancel_delayed_work_sync(&ts->esd_work);
> +
> +	enable_irq(ts->spi->irq);
> }
>
> -/* must be called with mutex held */
> -static void tsc2005_enable(struct tsc2005 *ts)
> +/* must be called with ts->mutex held */
> +static void __tsc2005_enable(struct tsc2005 *ts)
> {
> -	if (--ts->disable_depth != 0)
> -		return;
> 	tsc2005_start_scan(ts);
> -	enable_irq(ts->spi->irq);
> -	if (!ts->esd_timeout)
> -		return;
> -	mod_timer(&ts->esd_timer,
> -		  round_jiffies(jiffies + msecs_to_jiffies(ts->esd_timeout)));
> +
> +	if (ts->esd_timeout && ts->set_reset) {
> +		ts->last_valid_interrupt = jiffies;
> +		schedule_delayed_work(&ts->esd_work,
> +				round_jiffies(jiffies +
> +					msecs_to_jiffies(ts->esd_timeout)));
> +	}
> +
> }
>
> static ssize_t tsc2005_disable_show(struct device *dev,
> @@ -390,23 +380,29 @@ static ssize_t tsc2005_disable_store(struct device *dev,
> {
> 	struct spi_device *spi = to_spi_device(dev);
> 	struct tsc2005 *ts = spi_get_drvdata(spi);
> -	unsigned long res;
> -	int i;
> +	unsigned long val;
> +	int error;
>
> -	if (strict_strtoul(buf, 10, &res) < 0)
> -		return -EINVAL;
> -	i = res ? 1 : 0;
> +	error = strict_strtoul(buf, 10, &val);
> +	if (error)
> +		return error;
>
> 	mutex_lock(&ts->mutex);
> -	if (i == ts->disabled)
> -		goto out;
> -	ts->disabled = i;
> -	if (i)
> -		tsc2005_disable(ts);
> -	else
> -		tsc2005_enable(ts);
> -out:
> +
> +	if (!ts->suspended && ts->opened) {
> +		if (val) {
> +			if (!ts->disabled)
> +				__tsc2005_disable(ts);
> +		} else {
> +			if (ts->disabled)
> +				__tsc2005_enable(ts);
> +		}
> +	}
> +
> +	ts->disabled = !!val;
> +
> 	mutex_unlock(&ts->mutex);
> +
> 	return count;
> }
> static DEVICE_ATTR(disable, 0664, tsc2005_disable_show, tsc2005_disable_store);
> @@ -428,7 +424,7 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
> 	/*
> 	 * Test TSC2005 communications via temp high register.
> 	 */
> -	tsc2005_disable(ts);
> +	__tsc2005_disable(ts);
>
> 	error = tsc2005_read(ts, TSC2005_REG_TEMP_HIGH, &temp_high_orig);
> 	if (error) {
> @@ -484,7 +480,7 @@ static ssize_t tsc2005_selftest_show(struct device *dev,
> 	}
>
> out:
> -	tsc2005_enable(ts);
> +	__tsc2005_enable(ts);
> 	mutex_unlock(&ts->mutex);
>
> 	return sprintf(buf, "%d\n", success);
> @@ -519,44 +515,79 @@ static const struct attribute_group tsc2005_attr_group = {
> 	.attrs		= tsc2005_attrs,
> };
>
> -static void tsc2005_esd_timer(unsigned long data)
> -{
> -	struct tsc2005 *ts = (struct tsc2005 *)data;
> -
> -	schedule_work(&ts->esd_work);
> -}
> -
> static void tsc2005_esd_work(struct work_struct *work)
> {
> -	struct tsc2005 *ts = container_of(work, struct tsc2005, esd_work);
> +	struct tsc2005 *ts = container_of(work, struct tsc2005, esd_work.work);
> 	int error;
> 	u16 r;
>
> 	mutex_lock(&ts->mutex);
>
> -	if (ts->disable_depth)
> +	if (time_is_after_jiffies(ts->last_valid_interrupt +
> +				  msecs_to_jiffies(ts->esd_timeout)))
> 		goto out;
>
> -	/*
> -	 * If we cannot read our known value from configuration register 0 then
> -	 * reset the controller as if from power-up and start scanning again.
> -	 */
> +	/* We should be able to read register without disabling interrupts. */
> 	error = tsc2005_read(ts, TSC2005_REG_CFR0, &r);
> -	if (error ||
> -	    ((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK)) {
> -		dev_info(&ts->spi->dev, "TSC2005 not responding - resetting\n");
> -		ts->set_reset(false);
> -		tsc2005_update_pen_state(ts, 0, 0, 0);
> -		usleep_range(100, 500); /* only 10us required */
> -		ts->set_reset(true);
> -		tsc2005_start_scan(ts);
> +	if (!error &&
> +	    !((r ^ TSC2005_CFR0_INITVALUE) & TSC2005_CFR0_RW_MASK)) {
> +		goto out;
> 	}
>
> -	/* re-arm the watchdog */
> -	mod_timer(&ts->esd_timer,
> -		  round_jiffies(jiffies + msecs_to_jiffies(ts->esd_timeout)));
> +	/*
> +	 * 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);
> +
> +	ts->set_reset(false);
> +	usleep_range(100, 500); /* only 10us required */
> +	ts->set_reset(true);
> +
> +	enable_irq(ts->spi->irq);
> +	tsc2005_start_scan(ts);
>
> out:
> +	/* re-arm the watchdog */
> +	schedule_delayed_work(&ts->esd_work,
> +			      round_jiffies(jiffies +
> +					msecs_to_jiffies(ts->esd_timeout)));
> +	mutex_unlock(&ts->mutex);
> +}
> +
> +static int tsc2005_open(struct input_dev *input)
> +{
> +	struct tsc2005 *ts = input_get_drvdata(input);
> +
> +	mutex_lock(&ts->mutex);
> +
> +	if (!ts->suspended && !ts->disabled)
> +		__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 && !ts->disabled)
> +		__tsc2005_disable(ts);
> +
> +	ts->opened = false;
> +
> 	mutex_unlock(&ts->mutex);
> }
>
> @@ -626,8 +657,7 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
> 	spin_lock_init(&ts->lock);
> 	setup_timer(&ts->penup_timer, tsc2005_penup_timer, (unsigned long)ts);
>
> -	setup_timer(&ts->esd_timer, tsc2005_esd_timer, (unsigned long)ts);
> -	INIT_WORK(&ts->esd_work, tsc2005_esd_work);
> +	INIT_DELAYED_WORK(&ts->esd_work, tsc2005_esd_work);
>
> 	tsc2005_setup_spi_xfer(ts);
>
> @@ -645,6 +675,12 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
> 	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);
>
> +	input_dev->open = tsc2005_open;
> +	input_dev->close = tsc2005_close;
> +
> +	/* Ensure the touchscreen is off */
> +	tsc2005_stop_scan(ts);
> +
> 	error = request_threaded_irq(spi->irq, NULL, tsc2005_irq_thread,
> 				     IRQF_TRIGGER_RISING, "tsc2005", ts);
> 	if (error) {
> @@ -667,14 +703,6 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
> 		goto err_remove_sysfs;
> 	}
>
> -	tsc2005_start_scan(ts);
> -
> -	if (ts->esd_timeout && ts->set_reset) {
> -		/* start the optional ESD watchdog */
> -		mod_timer(&ts->esd_timer, round_jiffies(jiffies +
> -					msecs_to_jiffies(ts->esd_timeout)));
> -	}
> -
> 	set_irq_wake(spi->irq, 1);
> 	return 0;
>
> @@ -695,16 +723,6 @@ static int __devexit tsc2005_remove(struct spi_device *spi)
>
> 	sysfs_remove_group(&ts->spi->dev.kobj, &tsc2005_attr_group);
>
> -	mutex_lock(&ts->mutex);
> -	tsc2005_disable(ts);
> -	mutex_unlock(&ts->mutex);
> -
> -	if (ts->esd_timeout)
> -		del_timer_sync(&ts->esd_timer);
> -	del_timer_sync(&ts->penup_timer);
> -
> -	flush_work(&ts->esd_work);
> -
> 	free_irq(ts->spi->irq, ts);
> 	input_unregister_device(ts->idev);
> 	kfree(ts);
> @@ -720,7 +738,12 @@ static int tsc2005_suspend(struct device *dev)
> 	struct tsc2005 *ts = spi_get_drvdata(spi);
>
> 	mutex_lock(&ts->mutex);
> -	tsc2005_disable(ts);
> +
> +	if (!ts->suspended && !ts->disabled && ts->opened)
> +		__tsc2005_disable(ts);
> +
> +	ts->suspended = true;
> +
> 	mutex_unlock(&ts->mutex);
>
> 	return 0;
> @@ -732,7 +755,12 @@ static int tsc2005_resume(struct device *dev)
> 	struct tsc2005 *ts = spi_get_drvdata(spi);
>
> 	mutex_lock(&ts->mutex);
> -	tsc2005_enable(ts);
> +
> +	if (ts->suspended && !ts->disabled && ts->opened)
> +		__tsc2005_enable(ts);
> +
> +	ts->suspended = false;
> +
> 	mutex_unlock(&ts->mutex);
>
> 	return 0;
>

[PATCH 17/17] Input: tsc2005 - remove 'disable' sysfs attribute

[Dmitry Torokhov]

I believe that enable/disable functionality should not be implemented on
the individual driver level but rather in device core, potentially
reusing parts of PM framework. Therefore the driver-specific "disable"
attribute is removed from the mainline driver.

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

 drivers/input/touchscreen/tsc2005.c |   52 +++--------------------------------
 1 files changed, 4 insertions(+), 48 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 2e8c063..0e3c403 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -138,7 +138,6 @@ struct tsc2005 {
 
 	unsigned int		x_plate_ohm;
 
-	bool			disabled;
 	bool			opened;
 	bool			suspended;
 
@@ -365,48 +364,6 @@ static void __tsc2005_enable(struct tsc2005 *ts)
 
 }
 
-static ssize_t tsc2005_disable_show(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	struct spi_device *spi = to_spi_device(dev);
-	struct tsc2005 *ts = spi_get_drvdata(spi);
-
-	return sprintf(buf, "%u\n", ts->disabled);
-}
-
-static ssize_t tsc2005_disable_store(struct device *dev,
-				     struct device_attribute *attr,
-				     const char *buf, size_t count)
-{
-	struct spi_device *spi = to_spi_device(dev);
-	struct tsc2005 *ts = spi_get_drvdata(spi);
-	unsigned long val;
-	int error;
-
-	error = strict_strtoul(buf, 10, &val);
-	if (error)
-		return error;
-
-	mutex_lock(&ts->mutex);
-
-	if (!ts->suspended && ts->opened) {
-		if (val) {
-			if (!ts->disabled)
-				__tsc2005_disable(ts);
-		} else {
-			if (ts->disabled)
-				__tsc2005_enable(ts);
-		}
-	}
-
-	ts->disabled = !!val;
-
-	mutex_unlock(&ts->mutex);
-
-	return count;
-}
-static DEVICE_ATTR(disable, 0664, tsc2005_disable_show, tsc2005_disable_store);
-
 static ssize_t tsc2005_selftest_show(struct device *dev,
 				     struct device_attribute *attr,
 				     char *buf)
@@ -489,7 +446,6 @@ out:
 static DEVICE_ATTR(selftest, S_IRUGO, tsc2005_selftest_show, NULL);
 
 static struct attribute *tsc2005_attrs[] = {
-	&dev_attr_disable.attr,
 	&dev_attr_selftest.attr,
 	NULL
 };
@@ -567,7 +523,7 @@ static int tsc2005_open(struct input_dev *input)
 
 	mutex_lock(&ts->mutex);
 
-	if (!ts->suspended && !ts->disabled)
+	if (!ts->suspended)
 		__tsc2005_enable(ts);
 
 	ts->opened = true;
@@ -583,7 +539,7 @@ static void tsc2005_close(struct input_dev *input)
 
 	mutex_lock(&ts->mutex);
 
-	if (!ts->suspended && !ts->disabled)
+	if (!ts->suspended)
 		__tsc2005_disable(ts);
 
 	ts->opened = false;
@@ -739,7 +695,7 @@ static int tsc2005_suspend(struct device *dev)
 
 	mutex_lock(&ts->mutex);
 
-	if (!ts->suspended && !ts->disabled && ts->opened)
+	if (!ts->suspended && ts->opened)
 		__tsc2005_disable(ts);
 
 	ts->suspended = true;
@@ -756,7 +712,7 @@ static int tsc2005_resume(struct device *dev)
 
 	mutex_lock(&ts->mutex);
 
-	if (ts->suspended && !ts->disabled && ts->opened)
+	if (ts->suspended && ts->opened)
 		__tsc2005_enable(ts);
 
 	ts->suspended = false;

Re: [PATCH 00/17] Merging of TSC2005 driver

[Dmitry Torokhov]

On Wed, Mar 16, 2011 at 12:17:58AM -0700, Dmitry Torokhov wrote:
> Hi,
> 
> So I guess the common core for tsc2005/2007 will not materialize anytime
> soon and I should finally merge the standalone version of driver... Still
> there were a few issues with it and I would appreciate if you take a look
> at the following patches and let me know if the driver still works if
> you apply them.
> 
> Thanks.
> 

Also apologies to whoever gets multiple copies of patches - I fudged
stgit invocation a couple of times.

-- 
Dmitry

Re: [PATCH 00/17] Merging of TSC2005 driver

[Aaro Koskinen]

Hi,

On Wed, 16 Mar 2011, Dmitry Torokhov wrote:
> So I guess the common core for tsc2005/2007 will not materialize anytime
> soon and I should finally merge the standalone version of driver... Still
> there were a few issues with it and I would appreciate if you take a look
> at the following patches and let me know if the driver still works if
> you apply them.

Thanks, I think those are good changes. There are two bugs (about which
I already sent mails) but with those fixed it appeared to work fine
with RX-51 (Nokia N900) based on a brief sanity check. I tested with
mainline v2.6.38.

A.

Re: [PATCH 00/17] Merging of TSC2005 driver

[Dmitry Torokhov]

On Wed, Mar 16, 2011 at 05:44:23PM +0200, Aaro Koskinen wrote:
> Hi,
> 
> On Wed, 16 Mar 2011, Dmitry Torokhov wrote:
> >So I guess the common core for tsc2005/2007 will not materialize anytime
> >soon and I should finally merge the standalone version of driver... Still
> >there were a few issues with it and I would appreciate if you take a look
> >at the following patches and let me know if the driver still works if
> >you apply them.
> 
> Thanks, I think those are good changes. There are two bugs (about which
> I already sent mails) but with those fixed it appeared to work fine
> with RX-51 (Nokia N900) based on a brief sanity check. I tested with
> mainline v2.6.38.

Excellent, then I'll tag the patches as "Tested-by" you (after fixing up
the 2 issues you noticed).

Thanks.

-- 
Dmitry