Re: Problem with appletouch driver in Linux version 2.6.23-rc7

[Soeren Sonnenburg <kernel@nn7.de>]

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

On Thu, 2007-09-27 at 23:08 +0200, Thomas Rohwer wrote:
> >>> I think there is another bug in this. I mean whenever a mouse button is
> >>> pressed or the mouse is moved the counter should be reset - no?
> >>>
> >>> Currently the idle counter is just increased...
> >>>
> >>> I mean shouldn't it be
> >>>
> >>> if (x || y || key)
> >>>        dev->idlecount=0;
> >>>
> >>> if (!x && !y && !key)
> >>> {
> >>>        dev->idlecount++;
> >>>        if (dev->idlecount == 10) {
> >>>                dev->valid = 0;
> >>>                schedule_work(&dev->work);
> >>>        }
> >>> }
> 
> Hello,
> 
> I agree that this is makes sense. I was wondering if one can remove the whole
> idlecount logic. I tried replacing 10 with 1. This almost works; only if you press
> the touchpad button, after a reset happened, you first get a usb packet with the
> button state still 0 and after that one with button 1. I.e. with count 1 instead of 10
> you reset the device another time in this case. But one could handle this case separately.
> I am currently working with 2 instead of 10.

I am not sure whether it is worth fighting for. For the moment I think
we should just use the attached patch (where I as you suggest set
idlecount to 2).

So if there are no objections, Dmity please apply.

However if the touchpad sets some kind of 'ignore data' status bit on
'wakeup' this could be used to cleanup the code.

> I was also wondering when the procedure atp_reinit is executed after using schedule_work.
> Can it happen that atp_complete is called (even multiple times) before atp_reinit is executed?

I have no idea, Matthew?

Soeren.

[-- Attachment #2: appletouch.patch --]
[-- Type: text/x-patch, Size: 1581 bytes --]

Signed-off-by: Soeren Sonnenburg <kernel@nn7.de>

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index a1804bf..aa2bb4e 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -127,7 +127,8 @@ MODULE_DEVICE_TABLE (usb, atp_table);
  * Threshold for the touchpad sensors. Any change less than ATP_THRESHOLD is
  * ignored.
  */
-#define ATP_THRESHOLD	 5
+#define ATP_THRESHOLD	5
+#define ATP_IDLE_LIMIT	2
 
 /* MacBook Pro (Geyser 3 & 4) initialization constants */
 #define ATP_GEYSER3_MODE_READ_REQUEST_ID 1
@@ -502,18 +503,23 @@ static void atp_complete(struct urb* urb)
 
 		/* reset the accumulator on release */
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
+	}
+
+	/* Geyser 3 will continue to send packets continually after
+	   the first touch unless reinitialised. Do so if it's been
+	   idle for a while in order to avoid waking the kernel up
+	   several hundred times a second */
 
-		/* Geyser 3 will continue to send packets continually after
-		   the first touch unless reinitialised. Do so if it's been
-		   idle for a while in order to avoid waking the kernel up
-		   several hundred times a second */
-		if (!key && atp_is_geyser_3(dev)) {
+	if (atp_is_geyser_3(dev)) {
+		if (!x && !y && !key) {
 			dev->idlecount++;
-			if (dev->idlecount == 10) {
+			if (dev->idlecount == ATP_IDLE_LIMIT) {
 				dev->valid = 0;
 				schedule_work(&dev->work);
 			}
 		}
+		else
+		    dev->idlecount=0;
 	}
 
 	input_report_key(dev->input, BTN_LEFT, key);