[PATCH 0/2] OMAP: tsc2005: Configuration and stability changes

[PATCH 0/2] OMAP: tsc2005: Configuration and stability changes

[Phil Carmody]

I have attempted to separate what looked like it might be a painful big 
patch into two logically distinct smaller patches. Fortunately no lines
of code are touched by both patches, so it's possible to consider their
changes independently. 0001 is relative to the current master HEAD. 
 
Firstly, 0001 addresses changes in the structure of the driver and brings
the driver up to date with respect to TI's datasheet errata (March 2008),
but doesn't change the bahaviour of the state machine at all.

Secondly, 0002 modifies the state machine behavior for improved stability.
It uses PENDAV mode, rather than DAV mode, which provides a regular stream
of interrupts while the pen is down. In order to compensate for IRQs which
can't be handled synchronously, a saturating counting scheme is added, and 
if there are any IRQs that haven't yet been handled when an SPI read 
completes, or the pen-up timeout expires, then another SPI read is started.
For additional stability, the batch delay and pen-up timeout are increased
in order to limit the rate of the interrupts.

[PATCH 1/2] OMAP: tsc2005: Bringing driver's configuration (e.g. of IRQ/SPI) up to date.

[Phil Carmody]

Configuration of the driver's IRQ now happens in a different place,
it's a function of SPI now.

Some of the magic constants were wrong according to TI data-sheet
errata.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
---
 drivers/input/touchscreen/tsc2005.c |   64 ++++++++++++-----------------------
 1 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index 03c3a10..e3e63a5 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -77,8 +77,8 @@
 #define TSC2005_CMD_AUX_CONT	(8 << 3)
 #define TSC2005_CMD_TEST_X_CONN	(9 << 3)
 #define TSC2005_CMD_TEST_Y_CONN	(10 << 3)
-/* command 11 reserved */
-#define TSC2005_CMD_TEST_SHORT	(12 << 3)
+#define TSC2005_CMD_TEST_SHORT	(11 << 3)
+/* command 12 reserved, according to 2008-03 erratum */
 #define TSC2005_CMD_DRIVE_XX	(13 << 3)
 #define TSC2005_CMD_DRIVE_YY	(14 << 3)
 #define TSC2005_CMD_DRIVE_YX	(15 << 3)
@@ -213,8 +213,6 @@ struct tsc2005 {
 	int			stab_time;
 	int			p_max;
 	int			touch_pressure;
-	int			irq;
-	s16			dav_gpio;
 	/* status */
 	u8			sample_sent;
 	u8			pen_down;
@@ -225,7 +223,7 @@ struct tsc2005 {
 
 static void tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
 {
-	u16 data = TSC2005_CMD | TSC2005_CMD_12BIT | cmd;
+	u8 data = TSC2005_CMD | TSC2005_CMD_12BIT | cmd;
 	struct spi_message msg;
 	struct spi_transfer xfer = { 0 };
 
@@ -304,7 +302,7 @@ static void tsc2005_ts_rx(void *arg)
 		goto out;
 
 	/* skip coords if the pressure-components are out of range */
-	if (z1 < 100 || z2 > 4000)
+	if (z1 < 100 || z2 > MAX_12BIT || z1 >= z2)
 		goto out;
 
 	/* don't run average on the "pen down" event */
@@ -329,11 +327,8 @@ static void tsc2005_ts_rx(void *arg)
 	ts->avg_z1 = 0;
 	ts->avg_z2 = 0;
 
-	if (z1) {
 		pressure = x * (z2 - z1) / z1;
 		pressure = pressure * ts->x_plate_ohm / 4096;
-	} else
-		goto out;
 
 	pressure_limit = ts->sample_sent? ts->p_max: ts->touch_pressure;
 	if (pressure > pressure_limit)
@@ -458,7 +453,7 @@ static void tsc2005_disable(struct tsc2005 *ts)
 	if (ts->disable_depth++ != 0)
 		return;
 
-	disable_irq(ts->irq);
+	disable_irq(ts->spi->irq);
 
 	/* wait until penup timer expire normally */
 	do {
@@ -473,7 +468,7 @@ static void tsc2005_enable(struct tsc2005 *ts)
 	if (--ts->disable_depth != 0)
 		return;
 
-	enable_irq(ts->irq);
+	enable_irq(ts->spi->irq);
 
 	tsc2005_start_scan(ts);
 }
@@ -494,8 +489,9 @@ static ssize_t tsc2005_disable_store(struct device *dev,
 	unsigned long res;
 	int i;
 
-	i = strict_strtoul(buf, 10, &res);
-	i = i ? 1 : 0;
+	if (strict_strtoul(buf, 10, &res) < 0)
+		return -EINVAL;
+	i = res ? 1 : 0;
 
 	mutex_lock(&tsc->mutex);
 	if (i == tsc->disabled)
@@ -519,27 +515,10 @@ static int __devinit tsc2005_ts_init(struct tsc2005 *ts,
 				     struct tsc2005_platform_data *pdata)
 {
 	struct input_dev *idev;
-	int dav_gpio, r;
+	int r;
 	int x_max, y_max;
 	int x_fudge, y_fudge, p_fudge;
 
-	if (pdata->dav_gpio < 0) {
-		dev_err(&ts->spi->dev, "need DAV GPIO");
-		return -EINVAL;
-	}
-	dav_gpio = pdata->dav_gpio;
-	ts->dav_gpio = dav_gpio;
-	dev_dbg(&ts->spi->dev, "TSC2005: DAV GPIO = %d\n", dav_gpio);
-
-	r = gpio_request(dav_gpio, "TSC2005 dav");
-	if (r < 0) {
-		dev_err(&ts->spi->dev, "unable to get DAV GPIO");
-		goto err1;
-	}
-	gpio_direction_input(dav_gpio);
-	ts->irq = gpio_to_irq(dav_gpio);
-	dev_dbg(&ts->spi->dev, "TSC2005: DAV IRQ = %d\n", ts->irq);
-
 	init_timer(&ts->penup_timer);
 	setup_timer(&ts->penup_timer, tsc2005_ts_penup_timer_handler,
 			(unsigned long)ts);
@@ -561,7 +540,7 @@ static int __devinit tsc2005_ts_init(struct tsc2005 *ts,
 	idev = input_allocate_device();
 	if (idev == NULL) {
 		r = -ENOMEM;
-		goto err2;
+		goto err1;
 	}
 
 	idev->name = "TSC2005 touchscreen";
@@ -581,20 +560,20 @@ static int __devinit tsc2005_ts_init(struct tsc2005 *ts,
 
 	tsc2005_start_scan(ts);
 
-	r = request_irq(ts->irq, tsc2005_ts_irq_handler,
+	r = request_irq(ts->spi->irq, tsc2005_ts_irq_handler,
 			IRQF_TRIGGER_FALLING | IRQF_DISABLED |
 			IRQF_SAMPLE_RANDOM, "tsc2005", ts);
 	if (r < 0) {
 		dev_err(&ts->spi->dev, "unable to get DAV IRQ");
-		goto err3;
+		goto err2;
 	}
 
-	set_irq_wake(ts->irq, 1);
+	set_irq_wake(ts->spi->irq, 1);
 
 	r = input_register_device(idev);
 	if (r < 0) {
 		dev_err(&ts->spi->dev, "can't register touchscreen device\n");
-		goto err4;
+		goto err3;
 	}
 
 	/* We can tolerate these failing */
@@ -602,13 +581,11 @@ static int __devinit tsc2005_ts_init(struct tsc2005 *ts,
 	if (device_create_file(&ts->spi->dev, &dev_attr_disable_ts));
 
 	return 0;
-err4:
-	free_irq(ts->irq, ts);
 err3:
+	free_irq(ts->spi->irq, ts);
+err2:
 	tsc2005_stop_scan(ts);
 	input_free_device(idev);
-err2:
-	gpio_free(dav_gpio);
 err1:
 	return r;
 }
@@ -619,6 +596,10 @@ static int __devinit tsc2005_probe(struct spi_device *spi)
 	struct tsc2005_platform_data	*pdata = spi->dev.platform_data;
 	int r;
 
+	if (spi->irq < 0) {
+		dev_dbg(&spi->dev, "no irq?\n");
+		return -ENODEV;
+	}
 	if (!pdata) {
 		dev_dbg(&spi->dev, "no platform data?\n");
 		return -ENODEV;
@@ -663,10 +644,9 @@ static int __devexit tsc2005_remove(struct spi_device *spi)
 	device_remove_file(&ts->spi->dev, &dev_attr_disable_ts);
 	device_remove_file(&ts->spi->dev, &dev_attr_pen_down);
 
-	free_irq(ts->irq, ts);
+	free_irq(ts->spi->irq, ts);
 	input_unregister_device(ts->idev);
 
-	gpio_free(ts->dav_gpio);
 	kfree(ts);
 
 	return 0;
-- 
1.5.4.3

[PATCH 2/2] OMAP: tsc2005: Change state machine to be more stable.

[Phil Carmody]

Firstly, changed the IRQ mode to PENDAV, which acts as a heartbeat
to keep at least part of the IRQ/SPI sequence active while the pen
is down. Secondly, implemented an IRQ counting scheme so that the
SPI can lag far behind the IRQs. This requires ignoring pen-up
timeouts until you know there are no SPI requests pending. For
additional stability, lengthen the pen-up timeout, and decrease
the rate of the interrupts (increase batch delay).

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
---
 drivers/input/touchscreen/tsc2005.c |   56 ++++++++++++++++++++++------------
 1 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/drivers/input/touchscreen/tsc2005.c b/drivers/input/touchscreen/tsc2005.c
index e3e63a5..104ead2 100644
--- a/drivers/input/touchscreen/tsc2005.c
+++ b/drivers/input/touchscreen/tsc2005.c
@@ -145,7 +145,7 @@
 #define TSC2005_CFR1_BATCHDELAY_40MS	(0x0006)
 #define TSC2005_CFR1_BATCHDELAY_100MS	(0x0007)
 
-#define TSC2005_CFR1_INITVALUE	(TSC2005_CFR1_BATCHDELAY_2MS)
+#define TSC2005_CFR1_INITVALUE	(TSC2005_CFR1_BATCHDELAY_4MS)
 
 #define TSC2005_CFR2_MAVE_TEMP	(0x0001)
 #define TSC2005_CFR2_MAVE_AUX	(0x0002)
@@ -160,11 +160,12 @@
 #define TSC2005_CFR2_MEDIUM_7	(0x2000)
 #define TSC2005_CFR2_MEDIUM_15	(0x3000)
 
+#define TSC2005_CFR2_IRQ_MASK   (0xC000)
 #define TSC2005_CFR2_IRQ_DAV	(0x4000)
 #define TSC2005_CFR2_IRQ_PEN	(0x8000)
 #define TSC2005_CFR2_IRQ_PENDAV	(0x0000)
 
-#define TSC2005_CFR2_INITVALUE	(TSC2005_CFR2_IRQ_DAV   |	\
+#define TSC2005_CFR2_INITVALUE	(TSC2005_CFR2_IRQ_PENDAV |	\
 				 TSC2005_CFR2_MAVE_X    |	\
 				 TSC2005_CFR2_MAVE_Y    |	\
 				 TSC2005_CFR2_MAVE_Z    |	\
@@ -174,7 +175,7 @@
 #define MAX_12BIT					((1 << 12) - 1)
 #define TS_SAMPLES					4
 #define TS_RECT_SIZE					8
-#define TSC2005_TS_PENUP_TIME				20
+#define TSC2005_TS_PENUP_TIME				40
 
 static const u32 tsc2005_read_reg[] = {
 	(TSC2005_REG | TSC2005_REG_X | TSC2005_REG_READ) << 16,
@@ -218,7 +219,7 @@ struct tsc2005 {
 	u8			pen_down;
 	u8			disabled;
 	u8			disable_depth;
-	u8			spi_active;
+	u8			spi_pending;
 };
 
 static void tsc2005_cmd(struct tsc2005 *ts, u8 cmd)
@@ -327,8 +328,8 @@ static void tsc2005_ts_rx(void *arg)
 	ts->avg_z1 = 0;
 	ts->avg_z2 = 0;
 
-		pressure = x * (z2 - z1) / z1;
-		pressure = pressure * ts->x_plate_ohm / 4096;
+	pressure = x * (z2 - z1) / z1;
+	pressure = pressure * ts->x_plate_ohm / 4096;
 
 	pressure_limit = ts->sample_sent? ts->p_max: ts->touch_pressure;
 	if (pressure > pressure_limit)
@@ -352,7 +353,19 @@ static void tsc2005_ts_rx(void *arg)
 		ts->p = pressure;
 	}
 out:
-	ts->spi_active = 0;
+	if (ts->spi_pending > 1) {
+		/* One or more interrupts (sometimes several dozens)
+		 * occured while waiting for the SPI read - get
+		 * another read going.
+		 */
+		ts->spi_pending = 1;
+		if (spi_async(ts->spi, &ts->read_msg)) {
+			dev_err(&ts->spi->dev, "ts: spi_async() failed");
+			ts->spi_pending = 0;
+		}
+	} else
+		ts->spi_pending = 0;
+
 	spin_unlock_irqrestore(&ts->lock, flags);
 
 	/* kick pen up timer - to make sure it expires again(!) */
@@ -364,8 +377,7 @@ out:
 static void tsc2005_ts_penup_timer_handler(unsigned long data)
 {
 	struct tsc2005 *ts = (struct tsc2005 *)data;
-
-	if (ts->sample_sent) {
+	if (!ts->spi_pending && ts->sample_sent) {
 		tsc2005_ts_update_pen_state(ts, 0, 0, 0);
 		ts->sample_sent = 0;
 	}
@@ -373,20 +385,21 @@ static void tsc2005_ts_penup_timer_handler(unsigned long data)
 
 /*
  * This interrupt is called when pen is down and coordinates are
- * available. That is indicated by a falling edge on DAV line.
+ * available. That is indicated by a either:
+ * a) a rising edge on PINTDAV or (PENDAV mode)
+ * b) a falling edge on DAV line (DAV mode)
+ * depending on the setting of the IRQ bits in the CFR2 setting above.
  */
 static irqreturn_t tsc2005_ts_irq_handler(int irq, void *dev_id)
 {
 	struct tsc2005 *ts = dev_id;
-	int r;
-
-	if (ts->spi_active)
-		return IRQ_HANDLED;
 
-	ts->spi_active = 1;
-	r = spi_async(ts->spi, &ts->read_msg);
-	if (r)
-		dev_err(&ts->spi->dev, "ts: spi_async() failed");
+	if (!ts->spi_pending) {
+		if (spi_async(ts->spi, &ts->read_msg))
+			dev_err(&ts->spi->dev, "ts: spi_async() failed");
+	}
+	/* By shifting in 1s we can never wrap */
+	ts->spi_pending = (ts->spi_pending<<1)+1;
 
 	/* kick pen up timer */
 	mod_timer(&ts->penup_timer,
@@ -561,8 +574,11 @@ static int __devinit tsc2005_ts_init(struct tsc2005 *ts,
 	tsc2005_start_scan(ts);
 
 	r = request_irq(ts->spi->irq, tsc2005_ts_irq_handler,
-			IRQF_TRIGGER_FALLING | IRQF_DISABLED |
-			IRQF_SAMPLE_RANDOM, "tsc2005", ts);
+			(((TSC2005_CFR2_INITVALUE & TSC2005_CFR2_IRQ_MASK) ==
+			  TSC2005_CFR2_IRQ_PENDAV)
+			 ? IRQF_TRIGGER_RISING
+			 : IRQF_TRIGGER_FALLING) |
+			IRQF_DISABLED | IRQF_SAMPLE_RANDOM, "tsc2005", ts);
 	if (r < 0) {
 		dev_err(&ts->spi->dev, "unable to get DAV IRQ");
 		goto err2;
-- 
1.5.4.3

Re: [PATCH 2/2] OMAP: tsc2005: Change state machine to be more stable.

[Trilok Soni]

Hi Phil,

>        r = request_irq(ts->spi->irq, tsc2005_ts_irq_handler,
> -                       IRQF_TRIGGER_FALLING | IRQF_DISABLED |
> -                       IRQF_SAMPLE_RANDOM, "tsc2005", ts);
> +                       (((TSC2005_CFR2_INITVALUE & TSC2005_CFR2_IRQ_MASK) ==
> +                         TSC2005_CFR2_IRQ_PENDAV)
> +                        ? IRQF_TRIGGER_RISING
> +                        : IRQF_TRIGGER_FALLING) |
> +                       IRQF_DISABLED | IRQF_SAMPLE_RANDOM, "tsc2005", ts);

No need to IRQF_SAMPLE_RANDOM, as it is already handled by input
subsystem. Please check input_report_event.

I had submitted earlier version of tsc2005.c to mainline, and we have
got couple of comments, I can see that you have fixed simple_strtoul
return related problem in sysfs in the first patch. I will send you
that thread link tomorrow.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni

Re: [PATCH 2/2] OMAP: tsc2005: Change state machine to be more stable.

[Tony Lindgren]

* Trilok Soni <soni.trilok@gmail.com> [090217 09:44]:
> Hi Phil,
> 
> >        r = request_irq(ts->spi->irq, tsc2005_ts_irq_handler,
> > -                       IRQF_TRIGGER_FALLING | IRQF_DISABLED |
> > -                       IRQF_SAMPLE_RANDOM, "tsc2005", ts);
> > +                       (((TSC2005_CFR2_INITVALUE & TSC2005_CFR2_IRQ_MASK) ==
> > +                         TSC2005_CFR2_IRQ_PENDAV)
> > +                        ? IRQF_TRIGGER_RISING
> > +                        : IRQF_TRIGGER_FALLING) |
> > +                       IRQF_DISABLED | IRQF_SAMPLE_RANDOM, "tsc2005", ts);
> 
> No need to IRQF_SAMPLE_RANDOM, as it is already handled by input
> subsystem. Please check input_report_event.
> 
> I had submitted earlier version of tsc2005.c to mainline, and we have
> got couple of comments, I can see that you have fixed simple_strtoul
> return related problem in sysfs in the first patch. I will send you
> that thread link tomorrow.

Yeah let's plan on resetting l-o tree drivers/input/touchscreen at
some point soon to mainline. Those driver patches should be discussed
on the input list with l-o list cc'd.

Regards,

Tony

Re: [PATCH 1/2] OMAP: tsc2005: Bringing driver's configuration (e.g. of IRQ/SPI) up to date.

[Trilok Soni]

Hi Phil,

> -       set_irq_wake(ts->irq, 1);
> +       set_irq_wake(ts->spi->irq, 1);

Better if you use enable_irq_wake/disable_irq_wake friends, with
device_init_wakup being done in the probe and suspend/resume checking
device_may_wakeup stuff.

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni