2006-12-22 19:25:40

by David Brownell

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

From: [email protected] <[email protected]>
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 <[email protected]>
Signed-off-by: Juha Yrjola <[email protected]>
Signed-off-by: David Brownell <[email protected]>


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;


2006-12-22 20:35:26

by Dmitry Torokhov

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

On 12/22/06, David Brownell <[email protected]> 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

2006-12-22 20:40:24

by David Brownell

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

On Friday 22 December 2006 12:35 pm, Dmitry Torokhov wrote:
> On 12/22/06, David Brownell <[email protected]> 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
>

2006-12-27 14:34:19

by Imre Deak

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

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 <[email protected]> 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


Attachments:
(No filename) (1.57 kB)
0001-Input-ads7846-call-input-layer-functions-directly.txt (2.92 kB)
Download all attachments

2006-12-28 22:38:00

by David Brownell

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


> > > 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: [email protected] <[email protected]>
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 <[email protected]>
Signed-off-by: Juha Yrjola <[email protected]>
Signed-off-by: David Brownell <[email protected]>


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;

2006-12-29 06:22:56

by Dmitry Torokhov

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

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 <[email protected]>

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 <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---

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 */

2006-12-29 20:26:50

by David Brownell

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

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

2007-01-04 13:51:36

by Nicolas Ferre

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

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


2007-01-10 20:53:33

by David Brownell

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

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

2007-02-16 18:02:11

by Nicolas Ferre

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

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")


2007-02-16 19:08:55

by David Brownell

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

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
>
>

2007-02-19 12:48:48

by Nicolas Ferre

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

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


2007-02-19 18:59:29

by David Brownell

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

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

2007-02-20 09:20:34

by Nicolas Ferre

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

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 <[email protected]>
---
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



2007-03-01 04:53:45

by Dmitry Torokhov

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

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