OMAP5912 touchscreen irq not working

OMAP5912 touchscreen irq not working

[Dirk Behme]

Hi,

using recent git on OMAP5912 OSK with Mistral module 
touchscreen irq isn't working. Using configuration from my 
previous mail, using ts_calibrate no input is recognized.

Can anybody else reproduce this? Maybe still anything with 
latest GPIO irq changes?

Best regards

Dirk

Re: OMAP5912 touchscreen irq not working

[David Brownell]

On Friday 26 May 2006 11:33 pm, Dirk Behme wrote:

> Can anybody else reproduce this?

Just tried it ... /proc/interrupts shows a few IRQs, but I get no
touchscreen data.

- Dave

Re: OMAP5912 touchscreen irq not working

[Imre Deak]

On Sat, 2006-05-27 at 08:51 -0700, ext David Brownell wrote:
> On Friday 26 May 2006 11:33 pm, Dirk Behme wrote:
> 
> > Can anybody else reproduce this?
> 
> Just tried it ... /proc/interrupts shows a few IRQs, but I get no
> touchscreen data.

I noticed that the ads7846 sampling rate is set too high in board-osk.c.
The spec allows a maximum bus frequency of 2.5MHz which gives a ~96k
sample rate. There is also the uwire clock patch in my previous post,
which might have an effect on this issue.

--Imre

diff --git a/arch/arm/mach-omap1/board-osk.c b/arch/arm/mach-omap1/board-osk.c
index e0711d2..bc4269b 100644
--- a/arch/arm/mach-omap1/board-osk.c
+++ b/arch/arm/mach-omap1/board-osk.c
@@ -317,7 +317,7 @@ static struct spi_board_info __initdata 
 	.modalias		= "ads7846",
 	.platform_data		= &mistral_ts_info,
 	.irq			= OMAP_GPIO_IRQ(4),
-	.max_speed_hz		= 120000 /* max sample rate at 3V */
+	.max_speed_hz		= 96000 /* max sample rate at 3V */
 					* 26 /* command + data + overhead */,
 	.bus_num		= 2,
 	.chip_select		= 0,


> 
> - Dave
> _______________________________________________
> Linux-omap-open-source mailing list
> Linux-omap-open-source@linux.omap.com
> http://linux.omap.com/mailman/listinfo/linux-omap-open-source

Re: OMAP5912 touchscreen irq not working

[David Brownell]

On Saturday 27 May 2006 1:10 pm, Imre Deak wrote:

> I noticed that the ads7846 sampling rate is set too high in board-osk.c.
> The spec allows a maximum bus frequency of 2.5MHz which gives a ~96k
> sample rate. 

The spec says max sample rate of 125 KHz (at 5V) and describes Fclk
in terms of Fsample (at an extremely fast 16 bit/sample rate) ...
I saw no limit for Fclk, it's a function of the sample rate.

Regardless, the max sample frequency at 3V is not 96 KHz!  The
graph shows 120 KHz... 96 KHz would be for about 2.1V.


> There is also the uwire clock patch in my previous post, 
> which might have an effect on this issue.

Unlikely; it worked before, why would it stop working suddenly?
Including with the previous SPI clock... it's some other change
causing the breakage.

- Dave

Re: OMAP5912 touchscreen irq not working

[Imre Deak]

ext David Brownell wrote:
> On Saturday 27 May 2006 1:10 pm, Imre Deak wrote:
> 
>> I noticed that the ads7846 sampling rate is set too high in board-osk.c.
>> The spec allows a maximum bus frequency of 2.5MHz which gives a ~96k
>> sample rate. 
> 
> The spec says max sample rate of 125 KHz (at 5V) and describes Fclk
> in terms of Fsample (at an extremely fast 16 bit/sample rate) ...
> I saw no limit for Fclk, it's a function of the sample rate.

There is one table giving min/max timing values for Fclk as well and it
specifies 400ns  minimum clock period. We are using the 24 bit/sample
mode (8 for command, 16 for data) which at 120kHz/sample rate would need
 347ns clock period.

> 
> Regardless, the max sample frequency at 3V is not 96 KHz!  The
> graph shows 120 KHz... 96 KHz would be for about 2.1V.

I think, there should be a graph for each mode (15, 16, 24 bit/sample),
that one is valid for 16 bit/sample only.

> 
>> There is also the uwire clock patch in my previous post, 
>> which might have an effect on this issue.
> 
> Unlikely; it worked before, why would it stop working suddenly?
> Including with the previous SPI clock... it's some other change
> causing the breakage.

Yes, if it worked before with the wrong clock it's probably something else.

--Imre

> 
> - Dave

Re: OMAP5912 touchscreen irq not working

[Dirk Behme]

 >Imre Deak wrote:
>> ext David Brownell wrote:
>>Unlikely; it worked before, why would it stop working suddenly?
>>Including with the previous SPI clock... it's some other change
>>causing the breakage.
> 
> Yes, if it worked before with the wrong clock it's probably something else.

Had a look to it and added some debug printks to ads7846.c. 
One in ads7846_irq() and two in ads7846_rx(). There, one 
before check if samples to be thrown away, one behind (see 
below).

Looking at ads7846_irq(), while initalization, I get 
"ads7846 spi2.0: touchscreen + hwmon, irq 164"

With this irq, I only get two outputs of "ads_irq". The 
first directly while initialization (without touching the 
screen), the second at first screen touch. Then nothing, 
regardless how often I touch the screen again. This is 
reflected by /proc/interrupts as well: 164: 2   spi2.0. I 
think it's wrong to get ads7846_irq() called already while 
init without screen touched? And then this irq seems to be 
locked after first touch.

Looking at ads7846_rx, I permanently get "ads7846_rx 
called". But never "ads7846_rx valid sample". Is it correct 
to call ads7846_rx permanently? Shouldn't ads7846_rx() be 
called only while pen is down?

Please correct me if I'm wrong here, but with irq called at 
init without screen touched, then after one touch locked, 
and permanently called ads7846_rx() throwing away all 
samples, it seems to me that there is anything basically 
wrong with this new code? At least on OSK with mistral.

Best regards

Dirk

--- ./drivers/input/touchscreen/ads7846.c_orig  2006-05-31 
16:57:46.000000000 +0200
+++ ./drivers/input/touchscreen/ads7846.c       2006-05-31 
17:23:52.000000000 +0200
@@ -410,6 +410,8 @@ static void ads7846_rx(void *ads)
         } else
                 Rt = 0;

+               printk("### ads7846_rx called\n");
+
         /* 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 */
@@ -418,6 +420,8 @@ static void ads7846_rx(void *ads)
                 return;
         }

+       printk("### ads7846_rx valid sample\n");
+
         /* 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).
@@ -543,6 +547,8 @@ static irqreturn_t ads7846_irq(int irq,
         struct ads7846 *ts = handle;
         unsigned long flags;

+       printk("ads_irq\n");
+
         spin_lock_irqsave(&ts->lock, flags);
         if (likely(ts->get_pendown_state())) {
                 if (!ts->irq_disabled) {

Re: OMAP5912 touchscreen irq not working

[David Brownell]

On Wednesday 31 May 2006 8:44 am, Dirk Behme wrote:
> I think it's wrong to get ads7846_irq() called already while 
> init without screen touched? And then this irq seems to be 
> locked after first touch.

Locking out after first touch is a problem, of course.  

IRQ during init happens with some hardware; I recall
concluding it wasn't a problem here, though the details
of that have been discarded like a used coffee filter.

ISTR one issue is that sampling will trigger an IRQ,
so the driver needs to throttle the IRQs.



> Please correct me if I'm wrong here, but with irq called at 
> init without screen touched, then after one touch locked, 
> and permanently called ads7846_rx() throwing away all 
> samples, it seems to me that there is anything basically 
> wrong with this new code? At least on OSK with mistral.

There were glitches related to the problem with irqs not
getting disabled on request, with comments along the lines
of "remove this when the irqs work right" ... I think it's
time to maybe sort that issue out.

That said, this problem did not exist before the GPIO IRQ
patches were merged ... I certainly saw correct IRQ behavior
from my testing.

- Dave

Re: OMAP5912 touchscreen irq not working

[Imre Deak]

On Wed, 2006-05-31 at 09:05 -0700, ext David Brownell wrote:
> On Wednesday 31 May 2006 8:44 am, Dirk Behme wrote:
> > I think it's wrong to get ads7846_irq() called already while 
> > init without screen touched? And then this irq seems to be 
> > locked after first touch.
> 
> Locking out after first touch is a problem, of course.  
> 
> IRQ during init happens with some hardware; I recall
> concluding it wasn't a problem here, though the details
> of that have been discarded like a used coffee filter.
> 
> ISTR one issue is that sampling will trigger an IRQ,
> so the driver needs to throttle the IRQs.

Yes, during init we send send a couple of commands - with IRQs disabled
- to put the device in the initial waiting-for-IRQ state. That will also
result in a latched interrupt which will be delivered as soon as we
enable the IRQ again.

This spurious IRQ could be avoided by set_irq_type(IRQT_NONE) which
would prevent even the latching of it.

In the current code this shouldn't be a problem, since in the IRQ
handler we check the actual pen status.

> > Please correct me if I'm wrong here, but with irq called at 
> > init without screen touched, then after one touch locked, 
> > and permanently called ads7846_rx() throwing away all 
> > samples, it seems to me that there is anything basically 
> > wrong with this new code? At least on OSK with mistral.
> 
> There were glitches related to the problem with irqs not
> getting disabled on request, with comments along the lines
> of "remove this when the irqs work right" ... I think it's
> time to maybe sort that issue out.

Yes, I think the irq_disabled flag is not necessary any more.

> That said, this problem did not exist before the GPIO IRQ
> patches were merged ... I certainly saw correct IRQ behavior
> from my testing.

Might be, but this looks more to me to do something with values returned
to the _rx() function. Since it's continually called I suppose it never
detects the pen up which it determines from the pressure value. Would
help if you could enable the pr_debug in the _rx() function to see the
returned values.

--Imre

> 
> - Dave

[PATCH] input: ads7846: can't disable filtering [was OMAP5912 touchscreen irq not working]

[Imre Deak]

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

[ Didn't try on OSK, but seems to be the problem you had there. ]

When filtering is disabled the driver will ignore all samples and
never detect the pen up event.

Signed-off-by: Imre Deak <imre.deak@nokia.com>

[-- Attachment #2: ads7846-no_filter-patch.diff --]
[-- Type: text/plain, Size: 1484 bytes --]

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 4369845..2d8ea07 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -472,7 +474,8 @@ static void ads7846_debounce(void *ads)
 	m = &ts->msg[ts->msg_idx];
 	t = list_entry(m->transfers.prev, struct spi_transfer, transfer_list);
 	val = (be16_to_cpu(*(__be16 *)t->rx_buf) >> 3) & 0x0fff;
-	if (!ts->read_cnt || (abs(ts->last_read - val) > ts->debounce_tol)) {
+	if (ts->debounce_max && (
+	    !ts->read_cnt || (abs(ts->last_read - val) > ts->debounce_tol))) {
 		/* Repeat it, if this was the first read or the read
 		 * wasn't consistent enough. */
 		if (ts->read_cnt < ts->debounce_max) {
@@ -702,14 +705,9 @@ static int __devinit ads7846_probe(struc
 	ts->vref_delay_usecs = pdata->vref_delay_usecs ? : 100;
 	ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
 	ts->pressure_max = pdata->pressure_max ? : ~0;
-	if (pdata->debounce_max) {
-		ts->debounce_max = pdata->debounce_max;
-		ts->debounce_tol = pdata->debounce_tol;
-		ts->debounce_rep = pdata->debounce_rep;
-		if (ts->debounce_rep > ts->debounce_max + 1)
-			ts->debounce_rep = ts->debounce_max - 1;
-	} else
-		ts->debounce_tol = ~0;
+	ts->debounce_max = pdata->debounce_max;
+	ts->debounce_tol = pdata->debounce_tol;
+	ts->debounce_rep = pdata->debounce_rep;
 	ts->get_pendown_state = pdata->get_pendown_state;
 
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", spi->dev.bus_id);


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] input: ads7846: can't disable filtering [was OMAP5912 touchscreen irq not working]

[Dirk Behme]

Imre Deak wrote:
> [ Didn't try on OSK, but seems to be the problem you had there. ]
> 
> When filtering is disabled the driver will ignore all samples and
> never detect the pen up event.

ts_calibrate now works on OSK, but, uhmm, not very good. 
Because the reaction of the cursor cross is so slow I first 
touched it several times until it moved to the next 
position. Then I noticed that it can take 1-2s after touch 
until it moves.

Using ts_test, the cursor cross is jumping wild if screen is 
touched, but not usable. I wasn't able to switch from Drag 
to Draw mode because I couldn't hit Draw.

Sorry, but at least on OSK with Mistral this new stuff isn't 
usable. Old driver worked quite well.

Regards

Dirk

Re: [PATCH] input: ads7846: can't disable filtering [was OMAP5912 touchscreen irq not working]

[Imre Deak]

On Mon, 2006-06-05 at 14:45 +0200, ext Dirk Behme wrote:
> Imre Deak wrote:
> > [ Didn't try on OSK, but seems to be the problem you had there. ]
> > 
> > When filtering is disabled the driver will ignore all samples and
> > never detect the pen up event.
> 
> ts_calibrate now works on OSK, but, uhmm, not very good. 
> Because the reaction of the cursor cross is so slow I first 
> touched it several times until it moved to the next 
> position. Then I noticed that it can take 1-2s after touch 
> until it moves.
> 
> Using ts_test, the cursor cross is jumping wild if screen is 
> touched, but not usable. I wasn't able to switch from Drag 
> to Draw mode because I couldn't hit Draw.

Thanks a lot for trying it. I don't have an OSK board, so I can't test
it myself, but I found some displays having the same problem.

Currently the ads7846 driver depends on the the pressure value to
determine when the pen is lifted, which can cause the problem you saw.

I think the two problems you described are related. The long delay after
lifting the pen is b/c the pressure value settles rather slow at the
level we interpret as a pen up. The cursor jumping on the screen can be
the result of readings during this time, that is when the pen is already
lifted.

The solution would be to use the IRQ line state - as we already do to
determine the pen down - for the pen up event as well.

Following is a patch that does this. Could you give it a try, maybe with
debugging enabled?

> 
> Sorry, but at least on OSK with Mistral this new stuff isn't 
> usable. Old driver worked quite well.

Yes, I checked the old driver and it uses the pen IRQ for pen down / up
events, so that might be the difference.

Thanks,
Imre

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 4369845..94c36b1 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -370,6 +370,39 @@ static DEVICE_ATTR(disable, 0664, ads784
 
 /*--------------------------------------------------------------------------*/
 
+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.
@@ -381,11 +414,8 @@ static DEVICE_ATTR(disable, 0664, ads784
 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;
 
 	/* adjust:  on-wire is a must-ignore bit, a BE12 value, then padding;
 	 * built from two 8 bit values written msb-first.
@@ -399,7 +429,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;
@@ -414,51 +444,31 @@ static void ads7846_rx(void *ads)
 	* the maximum. Don't report it to user space, repeat at least
 	* 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
 		mod_timer(&ts->timer, jiffies + TS_POLL_PERIOD);
 		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...
-	 *
-	 * REVISIT:  the touchscreen might not be connected; this code
-	 * won't notice that, even if nPENIRQ never fires ...
+	/* NOTE: We can't rely on the pressure to determine the pen down
+	 * state. 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. The only safe way to check for the pen up
+	 * condition is in the timer by reading the pen IRQ state.
 	 */
-	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);
 	mod_timer(&ts->timer, jiffies + TS_POLL_PERIOD);
-
-	spin_unlock_irqrestore(&ts->lock, flags);
 }
 
 static void ads7846_debounce(void *ads)
@@ -472,7 +482,8 @@ static void ads7846_debounce(void *ads)
 	m = &ts->msg[ts->msg_idx];
 	t = list_entry(m->transfers.prev, struct spi_transfer, transfer_list);
 	val = (be16_to_cpu(*(__be16 *)t->rx_buf) >> 3) & 0x0fff;
-	if (!ts->read_cnt || (abs(ts->last_read - val) > ts->debounce_tol)) {
+	if (ts->debounce_max && (
+	    !ts->read_cnt || (abs(ts->last_read - val) > ts->debounce_tol))) {
 		/* Repeat it, if this was the first read or the read
 		 * wasn't consistent enough. */
 		if (ts->read_cnt < ts->debounce_max) {
@@ -519,14 +530,20 @@ static void ads7846_timer(unsigned long 
 
 	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;
+		}
+
 		/* measurment 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;
@@ -702,14 +719,9 @@ static int __devinit ads7846_probe(struc
 	ts->vref_delay_usecs = pdata->vref_delay_usecs ? : 100;
 	ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
 	ts->pressure_max = pdata->pressure_max ? : ~0;
-	if (pdata->debounce_max) {
-		ts->debounce_max = pdata->debounce_max;
-		ts->debounce_tol = pdata->debounce_tol;
-		ts->debounce_rep = pdata->debounce_rep;
-		if (ts->debounce_rep > ts->debounce_max + 1)
-			ts->debounce_rep = ts->debounce_max - 1;
-	} else
-		ts->debounce_tol = ~0;
+	ts->debounce_max = pdata->debounce_max;
+	ts->debounce_tol = pdata->debounce_tol;
+	ts->debounce_rep = pdata->debounce_rep;
 	ts->get_pendown_state = pdata->get_pendown_state;
 
 	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", spi->dev.bus_id);

Re: [PATCH] input: ads7846: can't disable filtering [was OMAP5912 touchscreen irq not working]

[David Brownell]

On Monday 05 June 2006 7:29 am, Imre Deak wrote:
> 
> The solution would be to use the IRQ line state - as we already do to
> determine the pen down - for the pen up event as well.
> ...
> > Sorry, but at least on OSK with Mistral this new stuff isn't 
> > usable. Old driver worked quite well.
> 
> Yes, I checked the old driver and it uses the pen IRQ for pen down / up
> events, so that might be the difference.

Well, there have been recent regressions, but the new code _has_ worked
quite well in the past ... 

Using the pen IRQ for driver state transitions is problematic, since the
driver has to work with a variety of triggering modes ... both edges,
falling only (or, with hw inverter, rising only), low level (or, with
inverter, high).

It's true that having that "poll penirq state" method is a big win.

- Dave

Re: [PATCH] input: ads7846: can't disable filtering

[Dirk Behme]

Imre Deak wrote:
> Thanks a lot for trying it.

Thanks for the patches.

> Following is a patch that does this. Could you give it a try, maybe with
> debugging enabled?

Ah! This works. At least on OSK ;)

Best regards

Dirk

Re: [PATCH] input: ads7846: can't disable filtering [was OMAP5912 touchscreen irq not working]

[Imre Deak]

On Mon, 2006-06-05 at 08:17 -0700, ext David Brownell wrote:
> On Monday 05 June 2006 7:29 am, Imre Deak wrote:
> > 
> > The solution would be to use the IRQ line state - as we already do to
> > determine the pen down - for the pen up event as well.
> > ...
> > > Sorry, but at least on OSK with Mistral this new stuff isn't 
> > > usable. Old driver worked quite well.
> > 
> > Yes, I checked the old driver and it uses the pen IRQ for pen down / up
> > events, so that might be the difference.
> 
> Well, there have been recent regressions, but the new code _has_ worked
> quite well in the past ... 

Ok, it would be good to do some comparison on the readings before and
after the breakage. I'll try to get a board and figure this out.

> 
> Using the pen IRQ for driver state transitions is problematic, since the
> driver has to work with a variety of triggering modes ... both edges,
> falling only (or, with hw inverter, rising only), low level (or, with
> inverter, high).

I understand, but since on some platforms I think it is the only way to
go  and on others it may reduce the latency of pen up events could we
still have this feature as an option? If the platform can't define it's
pen down state function we could fall back on the current pen up
detection method.

--Imre

> 
> It's true that having that "poll penirq state" method is a big win.
> 
> - Dave

Re: [PATCH] input: ads7846: can't disable filtering [was OMAP5912 touchscreen irq not working]

[David Brownell]

On Monday 05 June 2006 9:04 am, Imre Deak wrote:

> > Using the pen IRQ for driver state transitions is problematic, since the
> > driver has to work with a variety of triggering modes ... both edges,
> > falling only (or, with hw inverter, rising only), low level (or, with
> > inverter, high).
> 
> I understand, but since on some platforms I think it is the only way to
> go  and on others it may reduce the latency of pen up events could we
> still have this feature as an option? If the platform can't define it's
> pen down state function we could fall back on the current pen up
> detection method.

Certainly.  I was never a big fan of that method, but it seemed to
be necessary ... if, after sorting out interrelated bugs in gpio irq
handling, omap_uwire, and ads7846 code, it all works without that
messy pressure sense-as-penup logic, I'd be happy!

- Dave



> --Imre
> 
> > 
> > It's true that having that "poll penirq state" method is a big win.
> > 
> > - Dave
> 

Re: [PATCH] input: ads7846: can't disable filtering [was OMAP5912 touchscreen irq not working]

[tony]

* Imre Deak <imre.deak@nokia.com> [060603 12:50]:
> [ Didn't try on OSK, but seems to be the problem you had there. ]
> 
> When filtering is disabled the driver will ignore all samples and
> never detect the pen up event.
> 
> Signed-off-by: Imre Deak <imre.deak@nokia.com>

> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index 4369845..2d8ea07 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -472,7 +474,8 @@ static void ads7846_debounce(void *ads)
>  	m = &ts->msg[ts->msg_idx];
>  	t = list_entry(m->transfers.prev, struct spi_transfer, transfer_list);
>  	val = (be16_to_cpu(*(__be16 *)t->rx_buf) >> 3) & 0x0fff;
> -	if (!ts->read_cnt || (abs(ts->last_read - val) > ts->debounce_tol)) {
> +	if (ts->debounce_max && (
> +	    !ts->read_cnt || (abs(ts->last_read - val) > ts->debounce_tol))) {
>  		/* Repeat it, if this was the first read or the read
>  		 * wasn't consistent enough. */
>  		if (ts->read_cnt < ts->debounce_max) {
> @@ -702,14 +705,9 @@ static int __devinit ads7846_probe(struc
>  	ts->vref_delay_usecs = pdata->vref_delay_usecs ? : 100;
>  	ts->x_plate_ohms = pdata->x_plate_ohms ? : 400;
>  	ts->pressure_max = pdata->pressure_max ? : ~0;
> -	if (pdata->debounce_max) {
> -		ts->debounce_max = pdata->debounce_max;
> -		ts->debounce_tol = pdata->debounce_tol;
> -		ts->debounce_rep = pdata->debounce_rep;
> -		if (ts->debounce_rep > ts->debounce_max + 1)
> -			ts->debounce_rep = ts->debounce_max - 1;
> -	} else
> -		ts->debounce_tol = ~0;
> +	ts->debounce_max = pdata->debounce_max;
> +	ts->debounce_tol = pdata->debounce_tol;
> +	ts->debounce_rep = pdata->debounce_rep;
>  	ts->get_pendown_state = pdata->get_pendown_state;
>  
>  	snprintf(ts->phys, sizeof(ts->phys), "%s/input0", spi->dev.bus_id);
> 

Pushing this one today.

Tony