Re: [FIX / HP7xx] - Fix touchscreen to work without tsdev
[Picking up old thread...] On Fri, Mar 21, 2008 at 10:43:14PM +, Richard Purdie wrote: Later on in the code, BTN_TOUCH should do that same job but that isn't looked for in the initial check_fd() code. tslib can be easily fixed to address that issue and if nobody else writes a patch I'll aim to do so although it might not be for a few days. Having said that I don't see why exporting ABS_PRESSURE of 0 and 1 isn't a valid thing to do, they are pressure readings, just of a rather limited range! Because it does not convey any new data (BTN_TOUCH already indicates that the screen is being touched) and so the kernel and userspace have to spin wheels passing and processing/discarding useless events. -- Dmitry
Re: [FIX / HP7xx] - Fix touchscreen to work without tsdev
On Fri, 21 Mar 2008 22:43:14 + Richard Purdie [EMAIL PROTECTED] wrote: Hi, On Fri, 2008-03-21 at 15:59 -0400, Dmitry Torokhov wrote: On Fri, Mar 21, 2008 at 08:40:19PM -0700, Kristoffer Ericson wrote: This patch makes the jornada 7xx touchscreen work without tsdev. It changes the ABS values and makes sure the report_key is done in right order (for tslib). We also change the printk's to use ERR instead of INFO. I dont think that this patch should be applied. The hardware does not report pressure and thus the driver should not generate pressure events. Order of events should not matter either, thet should all be accumulated until userspace gets EV_SYN/SYN_REPORT message and then processed all together. It sounds like tslib is pretty broken in that regard. Should we bring tsdev back till the issues are resolved? Richard? Tsdev was ugly and I've got both my machines working now. Lets not jump to that quite yet ;-). I don't understand why the order would matter either. Kristoffer, can you confirm you really have to change the order of the events to make it work? I will make a test of that. Im not sure that order makes any difference I've just replicated the order all other drivers seem to use. I had multiple errors so Im not yet certain what was required. For instance my ABS values was way off and caused alot of problems, and I used BIT() instead of BIT_MASK(). I've looked at the tslib input plugin: http://svn.berlios.de/viewcvs/*checkout*/tslib/trunk/tslib/plugins/input-raw.c?rev=52 and I think I can see the issue Kristoffer has hit: (ioctl(ts-fd, EVIOCGBIT(EV_ABS, sizeof(absbit) * 8), absbit) = 0) (absbit (1 ABS_X)) (absbit (1 ABS_Y)) (absbit (1 ABS_PRESSURE since it seems tslib only looks at devices which export pressure data. Later on in the code, BTN_TOUCH should do that same job but that isn't looked for in the initial check_fd() code. tslib can be easily fixed to address that issue and if nobody else writes a patch I'll aim to do so although it might not be for a few days. Sounds good. I submitted a patch to tslib maintainer while tsdev was still available to disclude ABS_PRESSURE as an requirement. However it seems like tsdev supplied its own ABS_PRESSURE and therefore hid some of the bugs/errors from me. Having said that I don't see why exporting ABS_PRESSURE of 0 and 1 isn't a valid thing to do, they are pressure readings, just of a rather limited range! Personally I would say that ABS_PRESSURE isn't an big issue, if developers are aware of the necessity of ABS_PRESSURE then it shouldnt be any problems. Its better to expect ABS_PRESSURE by default and have the option to set it to 0-1 range rather than having to detect the specifics of each machine. Like if no pressure detection exists then just use on/off 1/0 mode, if pressure is detected then you can add a more proper range. Cheers, Richard -- Kristoffer Ericson [EMAIL PROTECTED]
Re: [FIX / HP7xx] - Fix touchscreen to work without tsdev
Hi Kristoffer,
On Fri, Mar 21, 2008 at 08:40:19PM -0700, Kristoffer Ericson wrote:
This patch makes the jornada 7xx touchscreen work without tsdev. It changes
the ABS values and makes sure the report_key is done in right order (for
tslib).
We also change the printk's to use ERR instead of INFO.
I dont think that this patch should be applied. The hardware does not
report pressure and thus the driver should not generate pressure
events. Order of events should not matter either, thet should all be
accumulated until userspace gets EV_SYN/SYN_REPORT message and then
processed all together.
It sounds like tslib is pretty broken in that regard. Should we bring
tsdev back till the issues are resolved? Richard?
Signed-off-by: Kristoffer Ericson [EMAIL PROTECTED]
diff --git a/drivers/input/touchscreen/jornada720_ts.c
b/drivers/input/touchscreen/jornada720_ts.c
index 42a1c9a..5d68fad 100644
--- a/drivers/input/touchscreen/jornada720_ts.c
+++ b/drivers/input/touchscreen/jornada720_ts.c
@@ -1,6 +1,8 @@
/*
* drivers/input/touchscreen/jornada720_ts.c
*
+ * HP Jornada 710/720/728 Touchscreen Driver
+ *
* Copyright (C) 2007 Kristoffer Ericson [EMAIL PROTECTED]
*
* Copyright (C) 2006 Filip Zyzniewski [EMAIL PROTECTED]
@@ -10,7 +12,6 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*
- * HP Jornada 710/720/729 Touchscreen Driver
*/
#include linux/platform_device.h
@@ -72,6 +73,7 @@ static irqreturn_t jornada720_ts_interrupt(int irq, void
*dev_id)
/* If GPIO_GPIO9 is set to high then report pen up */
if (GPLR GPIO_GPIO(9)) {
+ input_report_abs(input, ABS_PRESSURE, 0);
input_report_key(input, BTN_TOUCH, 0);
input_sync(input);
} else {
@@ -84,12 +86,12 @@ static irqreturn_t jornada720_ts_interrupt(int irq, void
*dev_id)
x = jornada720_ts_average(jornada_ts-x_data);
y = jornada720_ts_average(jornada_ts-y_data);
- input_report_key(input, BTN_TOUCH, 1);
input_report_abs(input, ABS_X, x);
input_report_abs(input, ABS_Y, y);
+ input_report_abs(input, ABS_PRESSURE, 1);
+ input_report_key(input, BTN_TOUCH, 1);
input_sync(input);
}
-
jornada_ssp_end();
}
@@ -118,24 +120,27 @@ static int __devinit jornada720_ts_probe(struct
platform_device *pdev)
input_dev-phys = jornadats/input0;
input_dev-id.bustype = BUS_HOST;
input_dev-dev.parent = pdev-dev;
+ input_dev-evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+ input_dev-absbit[0] = BIT_MASK(ABS_X) | BIT_MASK(ABS_Y) |
BIT_MASK(ABS_PRESSURE);
+ input_dev-keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
- input_dev-evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);
- input_dev-keybit[LONG(BTN_TOUCH)] = BIT(BTN_TOUCH);
- input_set_abs_params(input_dev, ABS_X, 270, 3900, 0, 0);
- input_set_abs_params(input_dev, ABS_Y, 180, 3700, 0, 0);
+ input_set_abs_params(input_dev, ABS_X, 79, 995, 0, 0);
+ input_set_abs_params(input_dev, ABS_Y, 75, 910, 0, 0);
error = request_irq(IRQ_GPIO9,
jornada720_ts_interrupt,
IRQF_DISABLED | IRQF_TRIGGER_RISING,
HP7XX Touchscreen driver, pdev);
if (error) {
- printk(KERN_INFO HP7XX TS : Unable to acquire irq!\n);
+ printk(KERN_ERR ts : unable to acquire irq!\n);
goto fail1;
}
error = input_register_device(jornada_ts-dev);
- if (error)
+ if (error) {
+ printk(KERN_ERR ts : unable to register device\n);
goto fail2;
+ }
return 0;
--
Kristoffer Ericson [EMAIL PROTECTED]
--
Dmitry
Re: [FIX / HP7xx] - Fix touchscreen to work without tsdev
Hi, On Fri, 2008-03-21 at 15:59 -0400, Dmitry Torokhov wrote: On Fri, Mar 21, 2008 at 08:40:19PM -0700, Kristoffer Ericson wrote: This patch makes the jornada 7xx touchscreen work without tsdev. It changes the ABS values and makes sure the report_key is done in right order (for tslib). We also change the printk's to use ERR instead of INFO. I dont think that this patch should be applied. The hardware does not report pressure and thus the driver should not generate pressure events. Order of events should not matter either, thet should all be accumulated until userspace gets EV_SYN/SYN_REPORT message and then processed all together. It sounds like tslib is pretty broken in that regard. Should we bring tsdev back till the issues are resolved? Richard? Lets not jump to that quite yet ;-). I don't understand why the order would matter either. Kristoffer, can you confirm you really have to change the order of the events to make it work? I've looked at the tslib input plugin: http://svn.berlios.de/viewcvs/*checkout*/tslib/trunk/tslib/plugins/input-raw.c?rev=52 and I think I can see the issue Kristoffer has hit: (ioctl(ts-fd, EVIOCGBIT(EV_ABS, sizeof(absbit) * 8), absbit) = 0) (absbit (1 ABS_X)) (absbit (1 ABS_Y)) (absbit (1 ABS_PRESSURE since it seems tslib only looks at devices which export pressure data. Later on in the code, BTN_TOUCH should do that same job but that isn't looked for in the initial check_fd() code. tslib can be easily fixed to address that issue and if nobody else writes a patch I'll aim to do so although it might not be for a few days. Having said that I don't see why exporting ABS_PRESSURE of 0 and 1 isn't a valid thing to do, they are pressure readings, just of a rather limited range! Cheers, Richard
