Re: [PATCH] edt_ts: EDT Touchscreen driver

On Tue, Nov 15, 2011 at 10:17:15AM +0100, Anatolij Gustschin wrote:

Hi Dmitry,

On Mon, 14 Nov 2011 13:32:42 -0800
Dmitry Torokhov [off-list ref] wrote:

quoted

On Tue, Nov 15, 2011 at 01:07:57AM +0400, Ilya Yanok wrote:

quoted

Hi Dmitry,

On 14.11.2011 21:28, Dmitry Torokhov wrote:

quoted

quoted

+	irq = gpio_to_irq(pdata->irq_gpio);
+	if (irq < 0) {
+		dev_err(&client->dev, "can't get IRQ for GPIO\n");
+		error = -EINVAL;
+		goto err0;
+	}

Why can't we use client->irq?

Well, my idea was to avoid duplication (passing both GPIO and IRQ
numbers) and I can't pass only client->irq as we need GPIO number also
and irq_to_gpio is not guaranteed to work with arbitrary IRQ number.

Do you really need to read gpio state in IRQ? Can't you simply rely on
'event' do decide whether to emit input events?

We can't rely on the 'event' only. The peculiar behaviour of the
touch controller enforces this. The touch controller toggles the
interrupt line when you keep the finger down. The rate is not exactly
specified by the the manufacturer (~80/sec). Measured rate deviates
from this specified rate. Valid touch packet data can be read when
interrupt line is pulled down. The designer of the target device
insisted on driver implementation reading touch packet data only
when irq line is pulled down.

quoted

Also:

- please do not report fake pressure events since the device does
not seem to support true pressure reading (tslib has been fixed ages
ago).

Many projects still use tslib-1.0 (e.g. distributions based on
recent yoctoproject releases). Removing pressure event reporting
will certainly break touch screen support there.

Then I recommend notifying yoctoproject that they need to update tslib
to something not so ancient.

Thanks.

-- 
Dmitry