[patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[David Brownell]

From: imre.deak@solidboot.com <imre.deak@solidboot.com>
Date:  Wed Jul 5 19:18:32 2006 +0300

Input: ads7846: detect pen up from GPIO state

We can't depend on the pressure value to determine when the pen was
lifted, so use the GPIO line state instead.  This also helps with
chips (like ads7843) that don't have pressure sensors.

Signed-off-by: Imre Deak <imre.deak@solidboot.com>
Signed-off-by: Juha Yrjola <juha.yrjola@solidboot.com>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>


Index: osk/drivers/input/touchscreen/ads7846.c
===================================================================
--- osk.orig/drivers/input/touchscreen/ads7846.c	2006-12-22 11:08:46.000000000 -0800
+++ osk/drivers/input/touchscreen/ads7846.c	2006-12-22 11:08:47.000000000 -0800
@@ -430,6 +430,39 @@ static struct attribute_group ads7845_at
 
 /*--------------------------------------------------------------------------*/
 
+static void ads7846_report_pen_state(struct ads7846 *ts, int down)
+{
+	struct input_dev	*input_dev = ts->input;
+
+	input_report_key(input_dev, BTN_TOUCH, down);
+	if (!down)
+		input_report_abs(input_dev, ABS_PRESSURE, 0);
+#ifdef VERBOSE
+	pr_debug("%s: %s\n", ts->spi->dev.bus_id, down ? "DOWN" : "UP");
+#endif
+}
+
+static void ads7846_report_pen_position(struct ads7846 *ts, int x, int y,
+					int pressure)
+{
+	struct input_dev	*input_dev = ts->input;
+
+	input_report_abs(input_dev, ABS_X, x);
+	input_report_abs(input_dev, ABS_Y, y);
+	input_report_abs(input_dev, ABS_PRESSURE, pressure);
+
+#ifdef VERBOSE
+	pr_debug("%s: %d/%d/%d\n", ts->spi->dev.bus_id, x, y, pressure);
+#endif
+}
+
+static void ads7846_sync_events(struct ads7846 *ts)
+{
+	struct input_dev	*input_dev = ts->input;
+
+	input_sync(input_dev);
+}
+
 /*
  * PENIRQ only kicks the timer.  The timer only reissues the SPI transfer,
  * to retrieve touchscreen status.
@@ -441,11 +474,8 @@ static struct attribute_group ads7845_at
 static void ads7846_rx(void *ads)
 {
 	struct ads7846		*ts = ads;
-	struct input_dev	*input_dev = ts->input;
 	unsigned		Rt;
-	unsigned		sync = 0;
 	u16			x, y, z1, z2;
-	unsigned long		flags;
 
 	/* ads7846_rx_val() did in-place conversion (including byteswap) from
 	 * on-the-wire format as part of debouncing to get stable readings.
@@ -459,7 +489,7 @@ static void ads7846_rx(void *ads)
 	if (x == MAX_12BIT)
 		x = 0;
 
-	if (likely(x && z1 && !device_suspended(&ts->spi->dev))) {
+	if (likely(x && z1)) {
 		/* compute touch pressure resistance using equation #2 */
 		Rt = z2;
 		Rt -= z1;
@@ -475,52 +505,33 @@ static void ads7846_rx(void *ads)
 	 * once more the measurement
 	 */
 	if (ts->tc.ignore || Rt > ts->pressure_max) {
+#ifdef VERBOSE
+		pr_debug("%s: ignored %d pressure %d\n",
+			ts->spi->dev.bus_id, ts->tc.ignore, Rt);
+#endif
 		hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_PERIOD),
 			      HRTIMER_REL);
 		return;
 	}
 
-	/* NOTE:  "pendown" is inferred from pressure; we don't rely on
-	 * being able to check nPENIRQ status, or "friendly" trigger modes
-	 * (both-edges is much better than just-falling or low-level).
-	 *
-	 * REVISIT:  some boards may require reading nPENIRQ; it's
-	 * needed on 7843.  and 7845 reads pressure differently...
+	/* NOTE: We can't rely on the pressure to determine the pen down
+	 * state, even this controller has a pressure sensor.  The pressure
+	 * value can fluctuate for quite a while after lifting the pen and
+	 * in some cases may not even settle at the expected value.
 	 *
-	 * REVISIT:  the touchscreen might not be connected; this code
-	 * won't notice that, even if nPENIRQ never fires ...
+	 * The only safe way to check for the pen up condition is in the
+	 * timer by reading the pen signal state (it's a GPIO _and_ IRQ).
 	 */
-	if (!ts->pendown && Rt != 0) {
-		input_report_key(input_dev, BTN_TOUCH, 1);
-		sync = 1;
-	} else if (ts->pendown && Rt == 0) {
-		input_report_key(input_dev, BTN_TOUCH, 0);
-		sync = 1;
-	}
-
 	if (Rt) {
-		input_report_abs(input_dev, ABS_X, x);
-		input_report_abs(input_dev, ABS_Y, y);
-		sync = 1;
-	}
-
-	if (sync) {
-		input_report_abs(input_dev, ABS_PRESSURE, Rt);
-		input_sync(input_dev);
+		if (!ts->pendown) {
+			ads7846_report_pen_state(ts, 1);
+			ts->pendown = 1;
+		}
+		ads7846_report_pen_position(ts, x, y, Rt);
+		ads7846_sync_events(ts);
 	}
 
-#ifdef	VERBOSE
-	if (Rt || ts->pendown)
-		pr_debug("%s: %d/%d/%d%s\n", ts->spi->dev.bus_id,
-			x, y, Rt, Rt ? "" : " UP");
-#endif
-
-	spin_lock_irqsave(&ts->lock, flags);
-
-	ts->pendown = (Rt != 0);
 	hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_PERIOD), HRTIMER_REL);
-
-	spin_unlock_irqrestore(&ts->lock, flags);
 }
 
 static int ads7846_debounce(void *ads, int data_idx, int *val)
@@ -616,14 +627,20 @@ static int ads7846_timer(struct hrtimer 
 
 	spin_lock_irq(&ts->lock);
 
-	if (unlikely(ts->msg_idx && !ts->pendown)) {
+	if (unlikely(!ts->get_pendown_state()
+			|| device_suspended(&ts->spi->dev))) {
+		if (ts->pendown) {
+			ads7846_report_pen_state(ts, 0);
+			ads7846_sync_events(ts);
+			ts->pendown = 0;
+		}
+
 		/* measurement cycle ended */
 		if (!device_suspended(&ts->spi->dev)) {
 			ts->irq_disabled = 0;
 			enable_irq(ts->spi->irq);
 		}
 		ts->pending = 0;
-		ts->msg_idx = 0;
 	} else {
 		/* pen is still down, continue with the measurement */
 		ts->msg_idx = 0;

Re: [patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[Dmitry Torokhov]

On 12/22/06, David Brownell <david-b@pacbell.net> wrote:
>
> +static void ads7846_report_pen_state(struct ads7846 *ts, int down)
> +{
> +       struct input_dev        *input_dev = ts->input;
> +
> +       input_report_key(input_dev, BTN_TOUCH, down);
> +       if (!down)
> +               input_report_abs(input_dev, ABS_PRESSURE, 0);
> +#ifdef VERBOSE
> +       pr_debug("%s: %s\n", ts->spi->dev.bus_id, down ? "DOWN" : "UP");
> +#endif
> +}
> +
> +static void ads7846_report_pen_position(struct ads7846 *ts, int x, int y,
> +                                       int pressure)
> +{
> +       struct input_dev        *input_dev = ts->input;
> +
> +       input_report_abs(input_dev, ABS_X, x);
> +       input_report_abs(input_dev, ABS_Y, y);
> +       input_report_abs(input_dev, ABS_PRESSURE, pressure);
> +
> +#ifdef VERBOSE
> +       pr_debug("%s: %d/%d/%d\n", ts->spi->dev.bus_id, x, y, pressure);
> +#endif
> +}
> +
> +static void ads7846_sync_events(struct ads7846 *ts)
> +{
> +       struct input_dev        *input_dev = ts->input;
> +
> +       input_sync(input_dev);
> +}

I think these helpers just obfuscate the code, just call
input_report_*() and input_sync() drectly like you used to do.

-- 
Dmitry

Re: [patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[David Brownell]

On Friday 22 December 2006 12:35 pm, Dmitry Torokhov wrote:
> On 12/22/06, David Brownell <david-b@pacbell.net> wrote:
> >
> > +static void ads7846_report_pen_state(struct ads7846 *ts, int down)
> > +{
> > +       struct input_dev        *input_dev = ts->input;
> > +
> > +       input_report_key(input_dev, BTN_TOUCH, down);
> > +       if (!down)
> > +               input_report_abs(input_dev, ABS_PRESSURE, 0);
> > +#ifdef VERBOSE
> > +       pr_debug("%s: %s\n", ts->spi->dev.bus_id, down ? "DOWN" : "UP");
> > +#endif
> > +}
> > +
> > +static void ads7846_report_pen_position(struct ads7846 *ts, int x, int y,
> > +                                       int pressure)
> > +{
> > +       struct input_dev        *input_dev = ts->input;
> > +
> > +       input_report_abs(input_dev, ABS_X, x);
> > +       input_report_abs(input_dev, ABS_Y, y);
> > +       input_report_abs(input_dev, ABS_PRESSURE, pressure);
> > +
> > +#ifdef VERBOSE
> > +       pr_debug("%s: %d/%d/%d\n", ts->spi->dev.bus_id, x, y, pressure);
> > +#endif
> > +}
> > +
> > +static void ads7846_sync_events(struct ads7846 *ts)
> > +{
> > +       struct input_dev        *input_dev = ts->input;
> > +
> > +       input_sync(input_dev);
> > +}
> 
> I think these helpers just obfuscate the code, just call
> input_report_*() and input_sync() drectly like you used to do.

Fair enough, I had a similar thought.  Imre, could you do that update?


> 
> -- 
> Dmitry
> 

Re: [patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[Imre Deak]

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

On Fri, Dec 22, 2006 at 12:40:20PM -0800, David Brownell wrote:
> On Friday 22 December 2006 12:35 pm, Dmitry Torokhov wrote:
> > On 12/22/06, David Brownell <david-b@pacbell.net> wrote:
> > >
> > > +static void ads7846_report_pen_state(struct ads7846 *ts, int down)
> > > +{
> > > +       struct input_dev        *input_dev = ts->input;
> > > +
> > > +       input_report_key(input_dev, BTN_TOUCH, down);
> > > +       if (!down)
> > > +               input_report_abs(input_dev, ABS_PRESSURE, 0);
> > > +#ifdef VERBOSE
> > > +       pr_debug("%s: %s\n", ts->spi->dev.bus_id, down ? "DOWN" : "UP");
> > > +#endif
> > > +}
> > > +
> > > +static void ads7846_report_pen_position(struct ads7846 *ts, int x, int y,
> > > +                                       int pressure)
> > > +{
> > > +       struct input_dev        *input_dev = ts->input;
> > > +
> > > +       input_report_abs(input_dev, ABS_X, x);
> > > +       input_report_abs(input_dev, ABS_Y, y);
> > > +       input_report_abs(input_dev, ABS_PRESSURE, pressure);
> > > +
> > > +#ifdef VERBOSE
> > > +       pr_debug("%s: %d/%d/%d\n", ts->spi->dev.bus_id, x, y, pressure);
> > > +#endif
> > > +}
> > > +
> > > +static void ads7846_sync_events(struct ads7846 *ts)
> > > +{
> > > +       struct input_dev        *input_dev = ts->input;
> > > +
> > > +       input_sync(input_dev);
> > > +}
> > 
> > I think these helpers just obfuscate the code, just call
> > input_report_*() and input_sync() drectly like you used to do.
> 
> Fair enough, I had a similar thought.  Imre, could you do that update?

Yes, the patch is against the OMAP tree.

--Imre

[-- Attachment #2: 0001-Input-ads7846-call-input-layer-functions-directly.txt --]
[-- Type: text/plain, Size: 2990 bytes --]

>From 34c26895c2cfb2bcc1a1d7994ad695e26b8eaeef Mon Sep 17 00:00:00 2001
From: Imre Deak <imre.deak@solidboot.com>
Date: Wed, 27 Dec 2006 16:07:32 +0200
Subject: [PATCH] Input: ads7846: call input layer functions directly

This reverts an earlier abstraction of these calls, which only
obfuscated the code.

Signed-off-by: Imre Deak <imre.deak@solidboot.com>
---
 drivers/input/touchscreen/ads7846.c |   56 +++++++++++-----------------------
 1 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index a47c95e..d6251ef 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -375,39 +375,6 @@ static DEVICE_ATTR(disable, 0664, ads7846_disable_show, ads7846_disable_store);
 
 /*--------------------------------------------------------------------------*/
 
-static void ads7846_report_pen_state(struct ads7846 *ts, int down)
-{
-	struct input_dev	*input_dev = ts->input;
-
-	input_report_key(input_dev, BTN_TOUCH, down);
-	if (!down)
-		input_report_abs(input_dev, ABS_PRESSURE, 0);
-#ifdef VERBOSE
-	pr_debug("%s: %s\n", ts->spi->dev.bus_id, down ? "DOWN" : "UP");
-#endif
-}
-
-static void ads7846_report_pen_position(struct ads7846 *ts, int x, int y,
-					int pressure)
-{
-	struct input_dev	*input_dev = ts->input;
-
-	input_report_abs(input_dev, ABS_X, x);
-	input_report_abs(input_dev, ABS_Y, y);
-	input_report_abs(input_dev, ABS_PRESSURE, pressure);
-
-#ifdef VERBOSE
-	pr_debug("%s: %d/%d/%d\n", ts->spi->dev.bus_id, x, y, pressure);
-#endif
-}
-
-static void ads7846_sync_events(struct ads7846 *ts)
-{
-	struct input_dev	*input_dev = ts->input;
-
-	input_sync(input_dev);
-}
-
 /*
  * PENIRQ only kicks the timer.  The timer only reissues the SPI transfer,
  * to retrieve touchscreen status.
@@ -466,11 +433,20 @@ static void ads7846_rx(void *ads)
 	 */
 	if (Rt) {
 		if (!ts->pendown) {
-			ads7846_report_pen_state(ts, 1);
+			input_report_key(ts->input, BTN_TOUCH, 1);
 			ts->pendown = 1;
+#ifdef VERBOSE
+			dev_dbg(&ts->spi->dev, "DOWN\n");
+#endif
 		}
-		ads7846_report_pen_position(ts, x, y, Rt);
-		ads7846_sync_events(ts);
+		input_report_abs(ts->input, ABS_X, x);
+		input_report_abs(ts->input, ABS_Y, y);
+		input_report_abs(ts->input, ABS_PRESSURE, Rt);
+
+		input_sync(ts->input);
+#ifdef VERBOSE
+		dev_dbg(&ts->spi->dev, "%4d/%4d/%4d\n", x, y, Rt);
+#endif
 	}
 
 	hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_PERIOD), HRTIMER_REL);
@@ -568,9 +544,13 @@ static int ads7846_timer(struct hrtimer *handle)
 	if (unlikely(!ts->get_pendown_state() ||
 		     device_suspended(&ts->spi->dev))) {
 		if (ts->pendown) {
-			ads7846_report_pen_state(ts, 0);
-			ads7846_sync_events(ts);
+			input_report_key(ts->input, BTN_TOUCH, 0);
+			input_report_abs(ts->input, ABS_PRESSURE, 0);
+			input_sync(ts->input);
 			ts->pendown = 0;
+#ifdef VERBOSE
+			dev_dbg(&ts->spi->dev, "UP\n");
+#endif
 		}
 
 		/* measurment cycle ended */
-- 
1.4.4.2

Re: [patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[David Brownell]

> > > I think these helpers just obfuscate the code, just call
> > > input_report_*() and input_sync() drectly like you used to do.
> > 
> > Fair enough, I had a similar thought.  Imre, could you do that update?
> 
> Yes, the patch is against the OMAP tree.

Thanks ... still hoping that the OMAP tree will just be able
to pull down the kernel.org version of this driver soon!!

OK, here's an updated patch 6/6 that merges Imre's update.

Dmitry, that makes six patches you should have ... I updated
the hwmon patch (#3), and now this one (#6); #4 and #5 will
thus apply with some offsets.

If you don't have other issues, I'd like to see these get into
the 2.6.20 release ... except for that version of the hwmon patch,
everything has been in use for some time through the OMAP tree,
which AFAICT is right now the main user of this driver.  (That'll
change a bit once Atmel's patches merge.)

- Dave

==========	CUT HERE
From: imre.deak@solidboot.com <imre.deak@solidboot.com>
Date: Wed Jul 5 19:18:32 2006 +0300

Input: ads7846: detect pen up from GPIO state

We can't depend on the pressure value to determine when the pen was
lifted, so use the GPIO line state instead.  This also helps with
chips (like ads7843) that don't have pressure sensors.

Signed-off-by: Imre Deak <imre.deak@solidboot.com>
Signed-off-by: Juha Yrjola <juha.yrjola@solidboot.com>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>


Index: at91/drivers/input/touchscreen/ads7846.c
===================================================================
--- at91.orig/drivers/input/touchscreen/ads7846.c	2006-12-22 12:13:27.000000000 -0800
+++ at91/drivers/input/touchscreen/ads7846.c	2006-12-28 14:19:51.000000000 -0800
@@ -441,11 +441,8 @@ static struct attribute_group ads7845_at
 static void ads7846_rx(void *ads)
 {
 	struct ads7846		*ts = ads;
-	struct input_dev	*input_dev = ts->input;
 	unsigned		Rt;
-	unsigned		sync = 0;
 	u16			x, y, z1, z2;
-	unsigned long		flags;
 
 	/* ads7846_rx_val() did in-place conversion (including byteswap) from
 	 * on-the-wire format as part of debouncing to get stable readings.
@@ -459,7 +456,7 @@ static void ads7846_rx(void *ads)
 	if (x == MAX_12BIT)
 		x = 0;
 
-	if (likely(x && z1 && !device_suspended(&ts->spi->dev))) {
+	if (likely(x && z1)) {
 		/* compute touch pressure resistance using equation #2 */
 		Rt = z2;
 		Rt -= z1;
@@ -475,52 +472,42 @@ static void ads7846_rx(void *ads)
 	 * once more the measurement
 	 */
 	if (ts->tc.ignore || Rt > ts->pressure_max) {
+#ifdef VERBOSE
+		pr_debug("%s: ignored %d pressure %d\n",
+			ts->spi->dev.bus_id, ts->tc.ignore, Rt);
+#endif
 		hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_PERIOD),
 			      HRTIMER_REL);
 		return;
 	}
 
-	/* NOTE:  "pendown" is inferred from pressure; we don't rely on
-	 * being able to check nPENIRQ status, or "friendly" trigger modes
-	 * (both-edges is much better than just-falling or low-level).
-	 *
-	 * REVISIT:  some boards may require reading nPENIRQ; it's
-	 * needed on 7843.  and 7845 reads pressure differently...
+	/* NOTE: We can't rely on the pressure to determine the pen down
+	 * state, even this controller has a pressure sensor.  The pressure
+	 * value can fluctuate for quite a while after lifting the pen and
+	 * in some cases may not even settle at the expected value.
 	 *
-	 * REVISIT:  the touchscreen might not be connected; this code
-	 * won't notice that, even if nPENIRQ never fires ...
+	 * The only safe way to check for the pen up condition is in the
+	 * timer by reading the pen signal state (it's a GPIO _and_ IRQ).
 	 */
-	if (!ts->pendown && Rt != 0) {
-		input_report_key(input_dev, BTN_TOUCH, 1);
-		sync = 1;
-	} else if (ts->pendown && Rt == 0) {
-		input_report_key(input_dev, BTN_TOUCH, 0);
-		sync = 1;
-	}
-
 	if (Rt) {
-		input_report_abs(input_dev, ABS_X, x);
-		input_report_abs(input_dev, ABS_Y, y);
-		sync = 1;
-	}
-
-	if (sync) {
-		input_report_abs(input_dev, ABS_PRESSURE, Rt);
-		input_sync(input_dev);
-	}
-
-#ifdef	VERBOSE
-	if (Rt || ts->pendown)
-		pr_debug("%s: %d/%d/%d%s\n", ts->spi->dev.bus_id,
-			x, y, Rt, Rt ? "" : " UP");
+		if (!ts->pendown) {
+			input_report_key(ts->input, BTN_TOUCH, 1);
+			ts->pendown = 1;
+#ifdef VERBOSE
+			dev_dbg(&ts->spi->dev, "DOWN\n");
 #endif
+		}
+		input_report_abs(ts->input, ABS_X, x);
+		input_report_abs(ts->input, ABS_Y, y);
+		input_report_abs(ts->input, ABS_PRESSURE, Rt);
+
+		input_sync(ts->input);
+#ifdef VERBOSE
+		dev_dbg(&ts->spi->dev, "%4d/%4d/%4d\n", x, y, Rt);
+#endif
+	}
 
-	spin_lock_irqsave(&ts->lock, flags);
-
-	ts->pendown = (Rt != 0);
 	hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_PERIOD), HRTIMER_REL);
-
-	spin_unlock_irqrestore(&ts->lock, flags);
 }
 
 static int ads7846_debounce(void *ads, int data_idx, int *val)
@@ -616,14 +603,24 @@ static int ads7846_timer(struct hrtimer 
 
 	spin_lock_irq(&ts->lock);
 
-	if (unlikely(ts->msg_idx && !ts->pendown)) {
+	if (unlikely(!ts->get_pendown_state()
+			|| device_suspended(&ts->spi->dev))) {
+		if (ts->pendown) {
+			input_report_key(ts->input, BTN_TOUCH, 0);
+			input_report_abs(ts->input, ABS_PRESSURE, 0);
+			input_sync(ts->input);
+			ts->pendown = 0;
+#ifdef VERBOSE
+			dev_dbg(&ts->spi->dev, "UP\n");
+#endif
+		}
+
 		/* measurement cycle ended */
 		if (!device_suspended(&ts->spi->dev)) {
 			ts->irq_disabled = 0;
 			enable_irq(ts->spi->irq);
 		}
 		ts->pending = 0;
-		ts->msg_idx = 0;
 	} else {
 		/* pen is still down, continue with the measurement */
 		ts->msg_idx = 0;

Re: [patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[Dmitry Torokhov]

On Thursday 28 December 2006 17:37, David Brownell wrote:
> > > > I think these helpers just obfuscate the code, just call
> > > > input_report_*() and input_sync() drectly like you used to do.
> > > 
> > > Fair enough, I had a similar thought.  Imre, could you do that update?
> > 
> > Yes, the patch is against the OMAP tree.
> 
> Thanks ... still hoping that the OMAP tree will just be able
> to pull down the kernel.org version of this driver soon!!
> 
> OK, here's an updated patch 6/6 that merges Imre's update.
> 
> Dmitry, that makes six patches you should have ... I updated
> the hwmon patch (#3), and now this one (#6); #4 and #5 will
> thus apply with some offsets.
> 

David,

I appied all patches except for hwmon as it had some issues with CONFIG_HWMON
handling. Could you please take a look at the patch below and tell me if it
works for you?

-- 
Dmitry

From: David Brownell <dbrownell@users.sourceforge.net>

Input: ads7846 - be more compatible with the hwmon framework

 - Hook up to hwmon
     * show sensor attributes only if hwmon is present
     * ... and the board's reference voltage is known
     * otherwise be just a touchscreen
 - Report voltages per hwmon convention
     * measure in millivolts
     * voltages are named in[0-8]_input (ugh)
     * for 7846 chips, properly range-adjust vBATT/in1_input

Battery measurements help during recharge monitoring.  On OSK/Mistral,
the measured voltage agreed with a multimeter to several decimal places.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/touchscreen/Kconfig   |    9 -
 drivers/input/touchscreen/ads7846.c |  306 +++++++++++++++++++++++++-----------
 2 files changed, 224 insertions(+), 91 deletions(-)

Index: work/drivers/input/touchscreen/Kconfig
===================================================================
--- work.orig/drivers/input/touchscreen/Kconfig
+++ work/drivers/input/touchscreen/Kconfig
@@ -12,13 +12,18 @@ menuconfig INPUT_TOUCHSCREEN
 if INPUT_TOUCHSCREEN
 
 config TOUCHSCREEN_ADS7846
-	tristate "ADS 7846 based touchscreens"
+	tristate "ADS 7846/7843 based touchscreens"
 	depends on SPI_MASTER
+	depends on HWMON = n || HWMON
 	help
 	  Say Y here if you have a touchscreen interface using the
-	  ADS7846 controller, and your board-specific initialization
+	  ADS7846 or ADS7843 controller, and your board-specific setup
 	  code includes that in its table of SPI devices.
 
+	  If HWMON is selected, and the driver is told the reference voltage
+	  on your board, you will also get hwmon interfaces for the voltage
+	  (and on ads7846, temperature) sensors of this chip.
+
 	  If unsure, say N (but it's safe to say "Y").
 
 	  To compile this driver as a module, choose M here: the
Index: work/drivers/input/touchscreen/ads7846.c
===================================================================
--- work.orig/drivers/input/touchscreen/ads7846.c
+++ work/drivers/input/touchscreen/ads7846.c
@@ -17,8 +17,9 @@
  *  it under the terms of the GNU General Public License version 2 as
  *  published by the Free Software Foundation.
  */
-#include <linux/device.h>
+#include <linux/hwmon.h>
 #include <linux/init.h>
+#include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
@@ -69,7 +70,7 @@ struct ts_event {
 	u16	x;
 	u16	y;
 	u16	z1, z2;
-	int    ignore;
+	int	ignore;
 };
 
 struct ads7846 {
@@ -77,7 +78,12 @@ struct ads7846 {
 	char			phys[32];
 
 	struct spi_device	*spi;
+
+#if defined(CONFIG_HWMON) || (defined(MODULE) && defined(CONFIG_HWMON_MODULE))
 	struct attribute_group	*attr_group;
+	struct class_device	*hwmon;
+#endif
+
 	u16			model;
 	u16			vref_delay_usecs;
 	u16			x_plate_ohms;
@@ -170,7 +176,12 @@ struct ads7846 {
 
 /*
  * Non-touchscreen sensors only use single-ended conversions.
+ * The range is GND..vREF. The ads7843 and ads7835 must use external vREF;
+ * ads7846 lets that pin be unconnected, to use internal vREF.
  */
+static unsigned vREF_mV;
+module_param(vREF_mV, uint, 0);
+MODULE_PARM_DESC(vREF_mV, "external vREF voltage, in milliVolts");
 
 struct ser_req {
 	u8			ref_on;
@@ -198,50 +209,55 @@ static int ads7846_read12_ser(struct dev
 	struct ser_req		*req = kzalloc(sizeof *req, GFP_KERNEL);
 	int			status;
 	int			sample;
-	int			i;
+	int			use_internal;
 
 	if (!req)
 		return -ENOMEM;
 
 	spi_message_init(&req->msg);
 
-	/* activate reference, so it has time to settle; */
-	req->ref_on = REF_ON;
-	req->xfer[0].tx_buf = &req->ref_on;
-	req->xfer[0].len = 1;
-	req->xfer[1].rx_buf = &req->scratch;
-	req->xfer[1].len = 2;
-
-	/*
-	 * for external VREF, 0 usec (and assume it's always on);
-	 * for 1uF, use 800 usec;
-	 * no cap, 100 usec.
-	 */
-	req->xfer[1].delay_usecs = ts->vref_delay_usecs;
+	/* FIXME boards with ads7846 might use external vref instead ... */
+	use_internal = (ts->model == 7846);
+
+	/* maybe turn on internal vREF, and let it settle */
+	if (use_internal) {
+		req->ref_on = REF_ON;
+		req->xfer[0].tx_buf = &req->ref_on;
+		req->xfer[0].len = 1;
+		spi_message_add_tail(&req->xfer[0], &req->msg);
+
+		req->xfer[1].rx_buf = &req->scratch;
+		req->xfer[1].len = 2;
+
+		/* for 1uF, settle for 800 usec; no cap, 100 usec.  */
+		req->xfer[1].delay_usecs = ts->vref_delay_usecs;
+		spi_message_add_tail(&req->xfer[1], &req->msg);
+	}
 
 	/* take sample */
 	req->command = (u8) command;
 	req->xfer[2].tx_buf = &req->command;
 	req->xfer[2].len = 1;
+	spi_message_add_tail(&req->xfer[2], &req->msg);
+
 	req->xfer[3].rx_buf = &req->sample;
 	req->xfer[3].len = 2;
+	spi_message_add_tail(&req->xfer[3], &req->msg);
 
 	/* REVISIT:  take a few more samples, and compare ... */
 
-	/* turn off reference */
-	req->ref_off = REF_OFF;
-	req->xfer[4].tx_buf = &req->ref_off;
-	req->xfer[4].len = 1;
-	req->xfer[5].rx_buf = &req->scratch;
-	req->xfer[5].len = 2;
-
-	CS_CHANGE(req->xfer[5]);
-
-	/* group all the transfers together, so we can't interfere with
-	 * reading touchscreen state; disable penirq while sampling
-	 */
-	for (i = 0; i < 6; i++)
-		spi_message_add_tail(&req->xfer[i], &req->msg);
+	/* maybe off internal vREF */
+	if (use_internal) {
+		req->ref_off = REF_OFF;
+		req->xfer[4].tx_buf = &req->ref_off;
+		req->xfer[4].len = 1;
+		spi_message_add_tail(&req->xfer[4], &req->msg);
+
+		req->xfer[5].rx_buf = &req->scratch;
+		req->xfer[5].len = 2;
+		CS_CHANGE(req->xfer[5]);
+		spi_message_add_tail(&req->xfer[5], &req->msg);
+	}
 
 	ts->irq_disabled = 1;
 	disable_irq(spi->irq);
@@ -261,25 +277,173 @@ static int ads7846_read12_ser(struct dev
 	return status ? status : sample;
 }
 
-#define SHOW(name) static ssize_t \
+#if defined(CONFIG_HWMON) || (defined(MODULE) && defined(CONFIG_HWMON_MODULE))
+
+#define SHOW(name, var, adjust) static ssize_t \
 name ## _show(struct device *dev, struct device_attribute *attr, char *buf) \
 { \
+	struct ads7846 *ts = dev_get_drvdata(dev); \
 	ssize_t v = ads7846_read12_ser(dev, \
-			READ_12BIT_SER(name) | ADS_PD10_ALL_ON); \
+			READ_12BIT_SER(var) | ADS_PD10_ALL_ON); \
 	if (v < 0) \
 		return v; \
-	return sprintf(buf, "%u\n", (unsigned) v); \
+	return sprintf(buf, "%u\n", adjust(ts, v)); \
 } \
 static DEVICE_ATTR(name, S_IRUGO, name ## _show, NULL);
 
-SHOW(temp0)
-SHOW(temp1)
-SHOW(vaux)
-SHOW(vbatt)
+
+/* Sysfs conventions report temperatures in millidegrees Celcius.
+ * ADS7846 could use the low-accuracy two-sample scheme, but can't do the high
+ * accuracy scheme without calibration data.  For now we won't try either;
+ * userspace sees raw sensor values, and must scale/calibrate appropriately.
+ */
+static inline unsigned null_adjust(struct ads7846 *ts, ssize_t v)
+{
+	return v;
+}
+
+SHOW(temp0, temp0, null_adjust)		/* temp1_input */
+SHOW(temp1, temp1, null_adjust)		/* temp2_input */
+
+
+/* sysfs conventions report voltages in millivolts.  We can convert voltages
+ * if we know vREF.  userspace may need to scale vAUX to match the board's
+ * external resistors; we assume that vBATT only uses the internal ones.
+ */
+static inline unsigned vaux_adjust(struct ads7846 *ts, ssize_t v)
+{
+	unsigned retval = v;
+
+	/* external resistors may scale vAUX into 0..vREF */
+	retval *= vREF_mV;
+	retval = retval >> 12;
+	return retval;
+}
+
+static inline unsigned vbatt_adjust(struct ads7846 *ts, ssize_t v)
+{
+	unsigned retval = vaux_adjust(ts, v);
+
+	/* ads7846 has a resistor ladder to scale this signal down */
+	if (ts->model == 7846)
+		retval *= 4;
+	return retval;
+}
+
+SHOW(in0_input, vaux, vaux_adjust)
+SHOW(in1_input, vbatt, vbatt_adjust)
+
+
+static struct attribute *ads7846_attributes[] = {
+	&dev_attr_temp0.attr,
+	&dev_attr_temp1.attr,
+	&dev_attr_in0_input.attr,
+	&dev_attr_in1_input.attr,
+	NULL,
+};
+
+static struct attribute_group ads7846_attr_group = {
+	.attrs = ads7846_attributes,
+};
+
+static struct attribute *ads7843_attributes[] = {
+	&dev_attr_in0_input.attr,
+	&dev_attr_in1_input.attr,
+	NULL,
+};
+
+static struct attribute_group ads7843_attr_group = {
+	.attrs = ads7843_attributes,
+};
+
+static struct attribute *ads7845_attributes[] = {
+	&dev_attr_in0_input.attr,
+	NULL,
+};
+
+static struct attribute_group ads7845_attr_group = {
+	.attrs = ads7845_attributes,
+};
+
+static int ads784x_hwmon_register(struct spi_device *spi, struct ads7846 *ts)
+{
+	struct class_device *hwmon;
+	int err;
+
+	/* hwmon sensors need a reference voltage */
+	switch (ts->model) {
+	case 7846:
+		if (!vREF_mV) {
+			dev_dbg(&spi->dev, "assuming 2.5V internal vREF\n");
+			vREF_mV = 2500;
+		}
+		break;
+	case 7845:
+	case 7843:
+		if (!vREF_mV) {
+			dev_warn(&spi->dev,
+				"external vREF for ADS%d not specified\n",
+				ts->model);
+			return 0;
+		}
+		break;
+	}
+
+	/* different chips have different sensor groups */
+	switch (ts->model) {
+	case 7846:
+		ts->attr_group = &ads7846_attr_group;
+		break;
+	case 7845:
+		ts->attr_group = &ads7845_attr_group;
+		break;
+	case 7843:
+		ts->attr_group = &ads7843_attr_group;
+		break;
+	default:
+		dev_dbg(&spi->dev, "ADS%d not recognized\n", ts->model);
+		return 0;
+	}
+
+	err = sysfs_create_group(&spi->dev.kobj, ts->attr_group);
+	if (err)
+		return err;
+
+	hwmon = hwmon_device_register(&spi->dev);
+	if (IS_ERR(hwmon)) {
+		sysfs_remove_group(&spi->dev.kobj, ts->attr_group);
+		return PTR_ERR(hwmon);
+	}
+
+	ts->hwmon = hwmon;
+	return 0;
+}
+
+static void ads784x_hwmon_unregister(struct spi_device *spi,
+				     struct ads7846 *ts)
+{
+	if (ts->hwmon) {
+		sysfs_remove_group(&spi->dev.kobj, ts->attr_group);
+		hwmon_device_unregister(ts->hwmon);
+	}
+}
+
+#else
+static inline int ads784x_hwmon_register(struct spi_device *spi,
+					 struct ads7846 *ts)
+{
+	return 0;
+}
+
+static inline void ads784x_hwmon_unregister(struct spi_device *spi,
+					    struct ads7846 *ts)
+{
+}
+#endif
 
 static int is_pen_down(struct device *dev)
 {
-	struct ads7846		*ts = dev_get_drvdata(dev);
+	struct ads7846	*ts = dev_get_drvdata(dev);
 
 	return ts->pendown;
 }
@@ -323,46 +487,14 @@ static ssize_t ads7846_disable_store(str
 
 static DEVICE_ATTR(disable, 0664, ads7846_disable_show, ads7846_disable_store);
 
-static struct attribute *ads7846_attributes[] = {
-	&dev_attr_temp0.attr,
-	&dev_attr_temp1.attr,
-	&dev_attr_vbatt.attr,
-	&dev_attr_vaux.attr,
+static struct attribute *ads784x_attributes[] = {
 	&dev_attr_pen_down.attr,
 	&dev_attr_disable.attr,
 	NULL,
 };
 
-static struct attribute_group ads7846_attr_group => > > I think these helpers just obfuscate the code, just call
> > > input_report_*() and input_sync() drectly like you used to do.
> > 
> > Fair enough, I had a similar thought.  Imre, could you do that update?
> 
> Yes, the patch is against the OMAP tree.

Thanks ... still hoping that the OMAP tree will just be able
to pull down the kernel.org version of this driver soon!!

OK, here's an updated patch 6/6 that merges Imre's update.

Dmitry, that makes six patches you should have ... I updated
the hwmon patch (#3), and now this one (#6); #4 and #5 will
thus apply with some offsets.
 {
-	.attrs = ads7846_attributes,
-};
-
-/*
- * ads7843/7845 don't have temperature sensors, and
- * use the other sensors a bit differently too
- */
-
-static struct attribute *ads7843_attributes[] = {
-	&dev_attr_vbatt.attr,
-	&dev_attr_vaux.attr,
-	&dev_attr_pen_down.attr,
-	&dev_attr_disable.attr,
-	NULL,
-};
-
-static struct attribute_group ads7843_attr_group = {
-	.attrs = ads7843_attributes,
-};
-
-static struct attribute *ads7845_attributes[] = {
-	&dev_attr_vaux.attr,
-	&dev_attr_pen_down.attr,
-	&dev_attr_disable.attr,
-	NULL,
-};
-
-static struct attribute_group ads7845_attr_group = {
-	.attrs = ads7845_attributes,
+static struct attribute_group ads784x_attr_group = {
+	.attrs = ads784x_attributes,
 };
 
 /*--------------------------------------------------------------------------*/
@@ -886,28 +1018,21 @@ static int __devinit ads7846_probe(struc
 		goto err_cleanup_filter;
 	}
 
+	err = ads784x_hwmon_register(spi, ts);
+	if (err)
+		goto err_free_irq;
+
 	dev_info(&spi->dev, "touchscreen, irq %d\n", spi->irq);
 
-	/* take a first sample, leaving nPENIRQ active; avoid
+	/* take a first sample, leaving nPENIRQ active and vREF off; avoid
 	 * the touchscreen, in case it's not connected.
 	 */
 	(void) ads7846_read12_ser(&spi->dev,
 			  READ_12BIT_SER(vaux) | ADS_PD10_ALL_ON);
 
-	switch (ts->model) {
-	case 7846:
-		ts->attr_group = &ads7846_attr_group;
-		break;
-	case 7845:
-		ts->attr_group = &ads7845_attr_group;
-		break;
-	default:
-		ts->attr_group = &ads7843_attr_group;
-		break;
-	}
-	err = sysfs_create_group(&spi->dev.kobj, ts->attr_group);
+	err = sysfs_create_group(&spi->dev.kobj, &ads784x_attr_group);
 	if (err)
-		goto err_free_irq;
+		goto err_remove_hwmon;
 
 	err = input_register_device(input_dev);
 	if (err)
@@ -916,7 +1041,9 @@ static int __devinit ads7846_probe(struc
 	return 0;
 
  err_remove_attr_group:
-	sysfs_remove_group(&spi->dev.kobj, ts->attr_group);
+	sysfs_remove_group(&spi->dev.kobj, &ads784x_attr_group);
+ err_remove_hwmon:
+	ads784x_hwmon_unregister(spi, ts);
  err_free_irq:
 	free_irq(spi->irq, ts);
  err_cleanup_filter:
@@ -932,11 +1059,12 @@ static int __devexit ads7846_remove(stru
 {
 	struct ads7846		*ts = dev_get_drvdata(&spi->dev);
 
+	ads784x_hwmon_unregister(spi, ts);
 	input_unregister_device(ts->input);
 
 	ads7846_suspend(spi, PMSG_SUSPEND);
 
-	sysfs_remove_group(&spi->dev.kobj, ts->attr_group);
+	sysfs_remove_group(&spi->dev.kobj, &ads784x_attr_group);
 
 	free_irq(ts->spi->irq, ts);
 	/* suspend left the IRQ disabled */

Re: [patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[David Brownell]

On Thursday 28 December 2006 10:22 pm, Dmitry Torokhov wrote:
> 
> I appied all patches except for hwmon as it had some issues with CONFIG_HWMON
> handling. Could you please take a look at the patch below and tell me if it
> works for you?

Looked OK, except:

> +#if defined(CONFIG_HWMON) || (defined(MODULE) && defined(CONFIG_HWMON_MODULE))

That idiom is more usually written

	#if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)

Thanks!  I'll be glad to see fewer versions of this driver floating around.
And to see the next version of the ads7843 patches ... :) 

- Dave

Re: [patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[Nicolas Ferre]

David Brownell a écrit :
> On Thursday 28 December 2006 10:22 pm, Dmitry Torokhov wrote:
>> I appied all patches except for hwmon as it had some issues with CONFIG_HWMON
>> handling. Could you please take a look at the patch below and tell me if it
>> works for you?
> 
> Looked OK, except:
> 
>> +#if defined(CONFIG_HWMON) || (defined(MODULE) && defined(CONFIG_HWMON_MODULE))
> 
> That idiom is more usually written
> 
> 	#if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
> 
> Thanks!  I'll be glad to see fewer versions of this driver floating around.
> And to see the next version of the ads7843 patches ... :) 

Hi, I am back on this task... I hope I will have a working patchset soon.

I face an issue using the hrtimer instead of the old timer framework 
(your patch #4/6). It seems that I do not sample at a sufficient rate 
using hrtimer : I see squares when drawing circles ;-)

Do you know if the hrtimer framework has an issue on at91 or do I have 
to code something to have a low res timer support in the hrtimer framework ?

Cheers,
-- 
Nicolas Ferre

Re: [patch 2.6.20-rc1 6/6] input: ads7846 directly senses PENUP state

[David Brownell]

On Thursday 04 January 2007 5:49 am, Nicolas Ferre wrote:
> 
> I face an issue using the hrtimer instead of the old timer framework 
> (your patch #4/6). It seems that I do not sample at a sufficient rate 
> using hrtimer : I see squares when drawing circles ;-)

Why do you suspect the sample rate rather than e.g. filtering or lowlevel
SPI issues?  Have you verified that e.g. the voltage sensor on ads7843
is giving you the right results?


> Do you know if the hrtimer framework has an issue on at91 or do I have 
> to code something to have a low res timer support in the hrtimer framework ?

No particular notion here ... I thought that if the hrtimer patch wasn't
installed (plus clocksource and clockevents patches, which struck me as
awkward on the at91sam926x parts compared to the at91rm9200) it would work
automagically in low-res timer mode (jiffies) using the same API.

So while applying highres timer patches would be possible, it's not supposed
to matter.  I suspect you could just revert the ads7846 hrtimer patch, to
see if that's the issue.

Did you rule out the issue being with the lowlevel AT91 SPI driver?  It's
been problematic in the past ... there's a bitbanging driver that could
help rule that out (or in).

Another low-effort tweak would be telling Kconfig to use HZ=512 or somesuch.
(A power-of-two value dividing the 32KHz clock better than multiple-of-ten.)
That'd increase the resolution of your "low res" timestamps.

- Dave

[PATCH] input/spi: add ads7843 support to ads7846 touchscreen driver

[Nicolas Ferre]

David Brownell :
[..]
> Thanks!  I'll be glad to see fewer versions of this driver floating around.
> And to see the next version of the ads7843 patches ... :) 

Hi,

Here is the ads7843 support for the ads7846 touchscreen driver. It is 
very little and takes great advantage of the previous rework.

Tested on Atmel at91sam926[13]ek board with atmel_spi underlying driver.

I also put a use case based on the at91sam9263ek init code. This code is 
just sent as an example, I will send those init patches trough AT91 
maintainer.

Index: linux-2.6.20-at91/drivers/input/touchscreen/ads7846.c
===================================================================
--- linux-2.6.20-at91.orig/drivers/input/touchscreen/ads7846.c
+++ linux-2.6.20-at91/drivers/input/touchscreen/ads7846.c
@@ -39,7 +39,8 @@
 /*
  * This code has been heavily tested on a Nokia 770, and lightly
  * tested on other ads7846 devices (OSK/Mistral, Lubbock).
- * Support for ads7843 and ads7845 has only been stubbed in.
+ * Support for ads7843 tested on Atmel at91sam926x-EK.
+ * Support for ads7845 has only been stubbed in.
  *
  * IRQ handling needs a workaround because of a shortcoming in handling
  * edge triggered IRQs on some platforms like the OMAP1/2. These
@@ -246,18 +247,15 @@ static int ads7846_read12_ser(struct dev
 
 	/* REVISIT:  take a few more samples, and compare ... */
 
-	/* maybe off internal vREF */
-	if (use_internal) {
-		req->ref_off = REF_OFF;
-		req->xfer[4].tx_buf = &req->ref_off;
-		req->xfer[4].len = 1;
-		spi_message_add_tail(&req->xfer[4], &req->msg);
-
-		req->xfer[5].rx_buf = &req->scratch;
-		req->xfer[5].len = 2;
-		CS_CHANGE(req->xfer[5]);
-		spi_message_add_tail(&req->xfer[5], &req->msg);
-	}
+	req->ref_off = REF_OFF;
+	req->xfer[4].tx_buf = &req->ref_off;
+	req->xfer[4].len = 1;
+	spi_message_add_tail(&req->xfer[4], &req->msg);
+
+	req->xfer[5].rx_buf = &req->scratch;
+	req->xfer[5].len = 2;
+	CS_CHANGE(req->xfer[5]);
+	spi_message_add_tail(&req->xfer[5], &req->msg);
 
 	ts->irq_disabled = 1;
 	disable_irq(spi->irq);
@@ -536,6 +534,10 @@ static void ads7846_rx(void *ads)
 	} else
 		Rt = 0;
 
+	if (ts->model == 7843)
+		Rt = ts->pressure_max / 2;
+
+
 	/* Sample found inconsistent by debouncing or pressure is beyond
 	 * the maximum. Don't report it to user space, repeat at least
 	 * once more the measurement


Index: linux-2.6.20-at91/arch/arm/mach-at91rm9200/board-sam9263ek.c
===================================================================
--- linux-2.6.20-at91.orig/arch/arm/mach-at91rm9200/board-sam9263ek.c
+++ linux-2.6.20-at91/arch/arm/mach-at91rm9200/board-sam9263ek.c
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
+#include <linux/spi/ads7846.h>
 
 #include <asm/hardware.h>
 #include <asm/setup.h>
@@ -84,6 +85,40 @@ static struct at91_udc_data __initdata e
 	.pullup_pin	= 0,		/* pull-up driven by UDC */
 };
 
+/*
+ * Touchscreen ads7843
+ */
+#if defined(CONFIG_TOUCHSCREEN_ADS7846) || defined(CONFIG_TOUCHSCREEN_ADS7846_MODULE)
+
+int ads7843_pendown_state(void)
+{
+	return !at91_get_gpio_value(AT91_PIN_PA15);
+}
+
+static struct ads7846_platform_data ads_info = {
+	.model			= 7843,
+	.x_min	= 150,	.x_max	= 3830,
+	.y_min	= 190,	.y_max	= 3830,
+	.vref_delay_usecs	= 100,
+	.x_plate_ohms		= 450,
+	.y_plate_ohms		= 250,
+	.pressure_max		= 15000,
+	.debounce_max		= 1,
+	.debounce_rep		= 0,
+	.debounce_tol		= (~0),
+	.get_pendown_state	= ads7843_pendown_state,
+};
+
+void __init at91_add_device_ts(void)
+{
+	/* Configure Interrupt 1 as external IRQ, with pullup */
+	at91_set_B_periph(AT91_PIN_PA15, 1);		/* IRQ1 */
+	/* ts busy */
+	at91_set_gpio_input(AT91_PIN_PA31, 1);
+}
+#else
+void __init at91_add_device_ts(void) {}
+#endif
 
 /*
  * SPI devices.
@@ -97,6 +132,17 @@ static struct spi_board_info ek_spi_devi
 		.bus_num	= 0,
 	},
 #endif
+#if defined(CONFIG_TOUCHSCREEN_ADS7846) || defined(CONFIG_TOUCHSCREEN_ADS7846_MODULE)
+	{
+		.modalias	= "ads7846",
+		.chip_select	= 3,
+		.max_speed_hz	= 125000	/* max sample rate at 3V */
+					* 26,	/* command + data + overhead */
+		.bus_num	= 0,
+		.platform_data	= &ads_info,
+		.irq		= AT91SAM9263_ID_IRQ1,
+	},
+#endif
 };
 
 
@@ -164,6 +210,8 @@ static void __init ek_board_init(void)
 	at91_add_device_mmc(1, &ek_mmc_data);
 	/* NAND */
 	at91_add_device_nand(&ek_nand_data);
+	/* Touchscreen */
+	at91_add_device_ts();
 }
 
 MACHINE_START(AT91SAM9263EK, "Atmel AT91SAM9263-EK")

Re: [PATCH] input/spi: add ads7843 support to ads7846 touchscreen driver

[David Brownell]

On Friday 16 February 2007 9:37 am, Nicolas Ferre wrote:
> David Brownell :
> [..]
> > Thanks!  I'll be glad to see fewer versions of this driver floating around.
> > And to see the next version of the ads7843 patches ... :) 
> 
> Hi,
> 
> Here is the ads7843 support for the ads7846 touchscreen driver. It is 
> very little and takes great advantage of the previous rework.

Good!  But see below for one fix ... after that, please split out the
ads7846 update by itself, and submit that patch to the input subsystem
maintainer with your signed-off-by.

Did you find out what was causing the jerky behavior (draw a circle,
it looks like a square) before?


> Tested on Atmel at91sam926[13]ek board with atmel_spi underlying driver.

Which has now been merged; minor fixes needed, including a build
fix for AT91.  I expect those will merge by the time 2.6.21 is done.
Looks like that driver is going to get a fair amount of use.  :)

 
> I also put a use case based on the at91sam9263ek init code. This code is 
> just sent as an example, I will send those init patches trough AT91 
> maintainer.

Yeah, support for that board hasn't made it to kernel.org yet.
I hope you'll also update Atmel's other EK boards using this chip.


> Index: linux-2.6.20-at91/drivers/input/touchscreen/ads7846.c
> ===================================================================
> --- linux-2.6.20-at91.orig/drivers/input/touchscreen/ads7846.c
> +++ linux-2.6.20-at91/drivers/input/touchscreen/ads7846.c
> @@ -39,7 +39,8 @@
>  /*
>   * This code has been heavily tested on a Nokia 770, and lightly
>   * tested on other ads7846 devices (OSK/Mistral, Lubbock).
> - * Support for ads7843 and ads7845 has only been stubbed in.
> + * Support for ads7843 tested on Atmel at91sam926x-EK.
> + * Support for ads7845 has only been stubbed in.
>   *
>   * IRQ handling needs a workaround because of a shortcoming in handling
>   * edge triggered IRQs on some platforms like the OMAP1/2. These
> @@ -246,18 +247,15 @@ static int ads7846_read12_ser(struct dev
>  
>  	/* REVISIT:  take a few more samples, and compare ... */
>  
> -	/* maybe off internal vREF */
> -	if (use_internal) {

This part doesn't make sense.  Could you say what you're trying
to do?  The ads7846 requres an external vREF.  Is the issue that
maybe the REF_OFF command isn't just turning off the (non-existent)
internal voltage reference?  If so the comments need updating, and
maybe the name REF_OFF needs to change too...


> -		req->ref_off = REF_OFF;
> -		req->xfer[4].tx_buf = &req->ref_off;
> -		req->xfer[4].len = 1;
> -		spi_message_add_tail(&req->xfer[4], &req->msg);
> -
> -		req->xfer[5].rx_buf = &req->scratch;
> -		req->xfer[5].len = 2;
> -		CS_CHANGE(req->xfer[5]);
> -		spi_message_add_tail(&req->xfer[5], &req->msg);
> -	}
> +	req->ref_off = REF_OFF;
> +	req->xfer[4].tx_buf = &req->ref_off;
> +	req->xfer[4].len = 1;
> +	spi_message_add_tail(&req->xfer[4], &req->msg);
> +
> +	req->xfer[5].rx_buf = &req->scratch;
> +	req->xfer[5].len = 2;
> +	CS_CHANGE(req->xfer[5]);
> +	spi_message_add_tail(&req->xfer[5], &req->msg);
>  
>  	ts->irq_disabled = 1;
>  	disable_irq(spi->irq);
> @@ -536,6 +534,10 @@ static void ads7846_rx(void *ads)
>  	} else
>  		Rt = 0;
>  
> +	if (ts->model == 7843)
> +		Rt = ts->pressure_max / 2;
> +
> +

Heh.  Other than turning whatever-that-is off, this looks like the only
substantive change needed.  You're right, this isn't much at all ... the
ads7843 support has now become almost trivial!


>  	/* Sample found inconsistent by debouncing or pressure is beyond
>  	 * the maximum. Don't report it to user space, repeat at least
>  	 * once more the measurement
> 
> 

Re: [PATCH] input/spi: add ads7843 support to ads7846 touchscreen driver

[Nicolas Ferre]

David Brownell :
> Did you find out what was causing the jerky behavior (draw a circle,
> it looks like a square) before?

No, my previous rework based on your patches (2.6.19) had this
jerky behavior. I was obliged to go back to old style timers.
On this build, with the htimer framework, it seems ok.
  
>> -	/* maybe off internal vREF */
>> -	if (use_internal) {
> 
> This part doesn't make sense.  Could you say what you're trying
> to do?  The ads7846 requres an external vREF.  Is the issue that
> maybe the REF_OFF command isn't just turning off the (non-existent)
> internal voltage reference?  If so the comments need updating, and
> maybe the name REF_OFF needs to change too...

Well, on the ads7843, this command allow to put converter in low power
mode and, more important, enable the PENIRQ (otherwise disabled).

The REF_OFF command can be replaced by the PWRDOWN one (exactly the
same command). This last name matches better the ads7843
datasheet description (and seems ok considering the ads7846 one).

>> -		req->ref_off = REF_OFF;
>> -		req->xfer[4].tx_buf = &req->ref_off;
>> -		req->xfer[4].len = 1;
>> -		spi_message_add_tail(&req->xfer[4], &req->msg);
>> -
>> -		req->xfer[5].rx_buf = &req->scratch;
>> -		req->xfer[5].len = 2;
>> -		CS_CHANGE(req->xfer[5]);
>> -		spi_message_add_tail(&req->xfer[5], &req->msg);
>> -	}
>> +	req->ref_off = REF_OFF;
>> +	req->xfer[4].tx_buf = &req->ref_off;
>> +	req->xfer[4].len = 1;
>> +	spi_message_add_tail(&req->xfer[4], &req->msg);
>> +
>> +	req->xfer[5].rx_buf = &req->scratch;
>> +	req->xfer[5].len = 2;
>> +	CS_CHANGE(req->xfer[5]);
>> +	spi_message_add_tail(&req->xfer[5], &req->msg);
>>  
>>  	ts->irq_disabled = 1;
>>  	disable_irq(spi->irq);

Regards,
-- 
Nicolas Ferre

Re: [PATCH] input/spi: add ads7843 support to ads7846 touchscreen driver

[David Brownell]

On Monday 19 February 2007 4:48 am, Nicolas Ferre wrote:
>   
> >> -	/* maybe off internal vREF */
> >> -	if (use_internal) {
> > 
> > This part doesn't make sense.  Could you say what you're trying
> > to do?  The ads7846 requres an external vREF.  Is the issue that
> > maybe the REF_OFF command isn't just turning off the (non-existent)
> > internal voltage reference?  If so the comments need updating, and
> > maybe the name REF_OFF needs to change too...
> 
> Well, on the ads7843, this command allow to put converter in low power
> mode and, more important, enable the PENIRQ (otherwise disabled).
> 
> The REF_OFF command can be replaced by the PWRDOWN one (exactly the
> same command). This last name matches better the ads7843
> datasheet description (and seems ok considering the ads7846 one).

OK, that would be a better change then, along with comment update.

- Dave

[PATCH] input/spi: add ads7843 support to ads7846 touchscreen driver

[Nicolas Ferre]

Add support for the ads7843 touchscreen controller to the ads7846
driver code.

The ads7843 support has now become almost trivial since the last
rework.

Signed-off-by: Nicolas Ferre <nicolas.ferre@rfo.atmel.com> 
---
Index: linux-2.6.20-at91/drivers/input/touchscreen/ads7846.c
===================================================================
--- linux-2.6.20-at91.orig/drivers/input/touchscreen/ads7846.c
+++ linux-2.6.20-at91/drivers/input/touchscreen/ads7846.c
@@ -39,7 +39,8 @@
 /*
  * This code has been heavily tested on a Nokia 770, and lightly
  * tested on other ads7846 devices (OSK/Mistral, Lubbock).
- * Support for ads7843 and ads7845 has only been stubbed in.
+ * Support for ads7843 tested on Atmel at91sam926x-EK.
+ * Support for ads7845 has only been stubbed in.
  *
  * IRQ handling needs a workaround because of a shortcoming in handling
  * edge triggered IRQs on some platforms like the OMAP1/2. These
@@ -246,18 +247,16 @@ static int ads7846_read12_ser(struct dev
 
 	/* REVISIT:  take a few more samples, and compare ... */
 
-	/* maybe off internal vREF */
-	if (use_internal) {
-		req->ref_off = REF_OFF;
-		req->xfer[4].tx_buf = &req->ref_off;
-		req->xfer[4].len = 1;
-		spi_message_add_tail(&req->xfer[4], &req->msg);
-
-		req->xfer[5].rx_buf = &req->scratch;
-		req->xfer[5].len = 2;
-		CS_CHANGE(req->xfer[5]);
-		spi_message_add_tail(&req->xfer[5], &req->msg);
-	}
+	/* converter in low power mode & enable PENIRQ */
+	req->ref_off = PWRDOWN;
+	req->xfer[4].tx_buf = &req->ref_off;
+	req->xfer[4].len = 1;
+	spi_message_add_tail(&req->xfer[4], &req->msg);
+
+	req->xfer[5].rx_buf = &req->scratch;
+	req->xfer[5].len = 2;
+	CS_CHANGE(req->xfer[5]);
+	spi_message_add_tail(&req->xfer[5], &req->msg);
 
 	ts->irq_disabled = 1;
 	disable_irq(spi->irq);
@@ -536,6 +535,10 @@ static void ads7846_rx(void *ads)
 	} else
 		Rt = 0;
 
+	if (ts->model == 7843)
+		Rt = ts->pressure_max / 2;
+
+
 	/* Sample found inconsistent by debouncing or pressure is beyond
 	 * the maximum. Don't report it to user space, repeat at least
 	 * once more the measurement

Re: [PATCH] input/spi: add ads7843 support to ads7846 touchscreen driver

[Dmitry Torokhov]

On Tuesday 20 February 2007 04:19, Nicolas Ferre wrote:
> Add support for the ads7843 touchscreen controller to the ads7846
> driver code.

Applied to the input tree, thank you.

-- 
Dmitry