[PATCH] fix appletouch geyser 1 breakage

[PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

The patch 46249ea60fbb61a72ee6929b831b1f3e6865f024 was obviously done
without testing on a Geyser 1, and I'm a very annoyed that it was
applied. It causes appletouch to continuously printk:

drivers/input/mouse/appletouch.c: Could not do mode read request from device (Geyser 3 mode)

because the Geyser 1 doesn't respond to that. The patch description also
states:

> if we see 10 empty packets the touchpad needs to be reset; good
> touchpads should not send empty packets anyway.

which is *TOTALLY* bogus since Geyser 1 touchpads have no notion of
empty packets, the simply continuously send measurements. One look at
the specification would have confirmed that.

This reverts the clueless commit.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---

--- linux-2.6.orig/drivers/input/mouse/appletouch.c	2007-10-24 12:37:39.140210069 +0200
+++ linux-2.6/drivers/input/mouse/appletouch.c	2007-10-24 12:37:50.000215820 +0200
@@ -504,22 +504,25 @@ static void atp_complete(struct urb* urb
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
-	input_report_key(dev->input, BTN_LEFT, key);
-	input_sync(dev->input);
-
-	/* Many Geysers will continue to send packets continually after
+	/* 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 (!x && !y && !key) {
-		dev->idlecount++;
-		if (dev->idlecount == 10) {
-			dev->valid = 0;
-			schedule_work(&dev->work);
+	if (atp_is_geyser_3(dev)) {
+		if (!x && !y && !key) {
+			dev->idlecount++;
+			if (dev->idlecount == 10) {
+				dev->valid = 0;
+				schedule_work(&dev->work);
+			}
 		}
-	} else
-		dev->idlecount = 0;
+		else
+			dev->idlecount = 0;
+	}
+
+	input_report_key(dev->input, BTN_LEFT, key);
+	input_sync(dev->input);
 
 exit:
 	retval = usb_submit_urb(dev->urb, GFP_ATOMIC);

Re: [PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

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

On Wed, 2007-10-24 at 12:44 +0200, Johannes Berg wrote:
> The patch 46249ea60fbb61a72ee6929b831b1f3e6865f024 was obviously done
> without testing on a Geyser 1, and I'm a very annoyed that it was
> applied. It causes appletouch to continuously printk:

I spoke too soon, I don't have a Geyser 1 but rather a Fountain touchpad
on my powerbook.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH v2] appletouch: fix fountain touchpad breakage

[Johannes Berg]

The patch 46249ea60fbb61a72ee6929b831b1f3e6865f024 was obviously done
without testing on a fountain touchpad. It causes appletouch to
continuously printk:

drivers/input/mouse/appletouch.c: Could not do mode read request from device (Geyser 3 mode)

because the fountain touchpad doesn't respond to that. The patch description
also states:

> if we see 10 empty packets the touchpad needs to be reset; good
> touchpads should not send empty packets anyway.

which is *TOTALLY* bogus since fountain touchpads have no notion of
empty packets, the simply continuously send measurements. One look at
the specification would have confirmed that.

This reverts the clueless commit, a better solution for geyser 1
touchpads must be found.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
What I'd advocate for 2.6.25 is to split appletouch into two drivers:
"appletouch" for fountain touchpads and maybe "appletouch2" for geyser
touchpads, this will get rid of many of the huge if statements in the
packet processing path and make sure that the macbook crowd will no
longer have to workaround the powerbook touchpads seeing that we seem to
hardly talk to each other.

Or maybe Soeren Sonnenburg's rewrite could be used for Geyser touchpads.

 drivers/input/mouse/appletouch.c |   25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

--- linux-2.6.orig/drivers/input/mouse/appletouch.c	2007-10-24 12:37:39.140210069 +0200
+++ linux-2.6/drivers/input/mouse/appletouch.c	2007-10-24 12:37:50.000215820 +0200
@@ -504,22 +504,25 @@ static void atp_complete(struct urb* urb
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
 	}
 
-	input_report_key(dev->input, BTN_LEFT, key);
-	input_sync(dev->input);
-
-	/* Many Geysers will continue to send packets continually after
+	/* 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 (!x && !y && !key) {
-		dev->idlecount++;
-		if (dev->idlecount == 10) {
-			dev->valid = 0;
-			schedule_work(&dev->work);
+	if (atp_is_geyser_3(dev)) {
+		if (!x && !y && !key) {
+			dev->idlecount++;
+			if (dev->idlecount == 10) {
+				dev->valid = 0;
+				schedule_work(&dev->work);
+			}
 		}
-	} else
-		dev->idlecount = 0;
+		else
+			dev->idlecount = 0;
+	}
+
+	input_report_key(dev->input, BTN_LEFT, key);
+	input_sync(dev->input);
 
 exit:
 	retval = usb_submit_urb(dev->urb, GFP_ATOMIC);

Re: [PATCH] fix appletouch geyser 1 breakage

[Dmitry Torokhov]

Hi Johannes,

On 10/24/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> The patch 46249ea60fbb61a72ee6929b831b1f3e6865f024 was obviously done
> without testing on a Geyser 1,

My fault, sorry. However Anton's device has product ID of 90x30B which
is Geyser 1 as far as I understand... But yes, we should not expect
other geysers respond to Geyser 3-specific commands.

>  and I'm a very annoyed that it was
> applied. It causes appletouch to continuously printk:
>
> drivers/input/mouse/appletouch.c: Could not do mode read request from device (Geyser 3 mode)
>
> because the Geyser 1 doesn't respond to that. The patch description also
> states:
>
> > if we see 10 empty packets the touchpad needs to be reset; good
> > touchpads should not send empty packets anyway.
>
> which is *TOTALLY* bogus since Geyser 1 touchpads have no notion of
> empty packets, the simply continuously send measurements. One look at
> the specification would have confirmed that.
>

Is there a way to "plug" these Geysers? Waking up the kernel
continuously is not nice.

-- 
Dmitry

Re: [PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

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

Hi,

> My fault, sorry. 

No, actually, I was wrong about Geyser 1, mine is a fountain.

> Is there a way to "plug" these Geysers? Waking up the kernel
> continuously is not nice.

Not sure really, maybe checking for is_geyser instead of is_geyser_3?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] fix appletouch geyser 1 breakage

[Dmitry Torokhov]

On 10/24/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> Hi,
>
> > My fault, sorry.
>
> No, actually, I was wrong about Geyser 1, mine is a fountain.
>
> > Is there a way to "plug" these Geysers? Waking up the kernel
> > continuously is not nice.
>
> Not sure really, maybe checking for is_geyser instead of is_geyser_3?
>

Well, but what about fountains then? Regardless of the model, if there
is a way to stop "empty" meaurements, we should do it.

-- 
Dmitry

Re: [PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

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

On Wed, 2007-10-24 at 09:34 -0400, Dmitry Torokhov wrote:

> Well, but what about fountains then? Regardless of the model, if there
> is a way to stop "empty" meaurements, we should do it.

There is no way on fountains though. We could check the measurement
ourselves and if no finger is detected decrease the polling frequency or
something, but there's no hw support.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] fix appletouch geyser 1 breakage

[Dmitry Torokhov]

On 10/24/07, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2007-10-24 at 09:34 -0400, Dmitry Torokhov wrote:
>
> > Well, but what about fountains then? Regardless of the model, if there
> > is a way to stop "empty" meaurements, we should do it.
>
> There is no way on fountains though. We could check the measurement
> ourselves and if no finger is detected decrease the polling frequency or
> something, but there's no hw support.
>

Do yo know who has powerbooks with older geyser models (0x214, 215,
216)? It would be nice to know if they send the data continiously and
whether the geyser 3 reset hack works on them.

-- 
Dmitry

Re: [PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

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

On Wed, 2007-10-24 at 10:29 -0400, Dmitry Torokhov wrote:

> Do yo know who has powerbooks with older geyser models (0x214, 215,
> 216)? 

Not sure, Benjamin? We're talking about the touchpad, just lsusb should
be enough.

> It would be nice to know if they send the data continiously and
> whether the geyser 3 reset hack works on them.

That'd be good, but for fountains it definitely doesn't work.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] fix appletouch geyser 1 breakage

[Benjamin Berg]

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

On Thu, 2007-25-10 at 15:23 +0200, Johannes Berg wrote:
> On Wed, 2007-10-24 at 10:29 -0400, Dmitry Torokhov wrote:
> 
> > Do yo know who has powerbooks with older geyser models (0x214, 215,
> > 216)? 
> 
> Not sure, Benjamin? We're talking about the touchpad, just lsusb should
> be enough.

lsusb says I have a 0x215
(Bus 003 Device 002: ID 05ac:0215 Apple Computer, Inc.)

Everything is working fine with a 2.6.24-rc1 kernel (ie. no bogus
messages).

> > It would be nice to know if they send the data continiously and
> > whether the geyser 3 reset hack works on them.
> 
> That'd be good, but for fountains it definitely doesn't work.

Benjamin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] fix appletouch geyser 1 breakage

[Dmitry Torokhov]

On 10/25/07, Benjamin Berg <benjamin@sipsolutions.net> wrote:
> On Thu, 2007-25-10 at 15:23 +0200, Johannes Berg wrote:
> > On Wed, 2007-10-24 at 10:29 -0400, Dmitry Torokhov wrote:
> >
> > > Do yo know who has powerbooks with older geyser models (0x214, 215,
> > > 216)?
> >
> > Not sure, Benjamin? We're talking about the touchpad, just lsusb should
> > be enough.
>
> lsusb says I have a 0x215
> (Bus 003 Device 002: ID 05ac:0215 Apple Computer, Inc.)
>
> Everything is working fine with a 2.6.24-rc1 kernel (ie. no bogus
> messages).
>
> > > It would be nice to know if they send the data continiously and
> > > whether the geyser 3 reset hack works on them.
> >
> > That'd be good, but for fountains it definitely doesn't work.
>
> Benjamin
>

Johannes, and what is product ID for your touchpad?

-- 
Dmitry

Re: [PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

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



> Johannes, and what is product ID for your touchpad?

It's 0x20e, listed as 'fountain'

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] fix appletouch geyser 1 breakage

[Dmitry Torokhov]

On Friday 26 October 2007 16:31, Johannes Berg wrote:
> 
> > Johannes, and what is product ID for your touchpad?
> 
> It's 0x20e, listed as 'fountain'
>

OK, then maybe instead of reverting the change outright we could try the
patch below?

-- 
Dmitry

Input: appletouch - idle reset logic broke older Fountains

Older models of fountains do not support change mode request and
therefore shoudl be excluded from idle reset attempts.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/mouse/appletouch.c |   83 +++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 34 deletions(-)

Index: work/drivers/input/mouse/appletouch.c
===================================================================
--- work.orig/drivers/input/mouse/appletouch.c
+++ work/drivers/input/mouse/appletouch.c
@@ -130,11 +130,11 @@ MODULE_DEVICE_TABLE (usb, atp_table);
 #define ATP_THRESHOLD	 5
 
 /* MacBook Pro (Geyser 3 & 4) initialization constants */
-#define ATP_GEYSER3_MODE_READ_REQUEST_ID 1
-#define ATP_GEYSER3_MODE_WRITE_REQUEST_ID 9
-#define ATP_GEYSER3_MODE_REQUEST_VALUE 0x300
-#define ATP_GEYSER3_MODE_REQUEST_INDEX 0
-#define ATP_GEYSER3_MODE_VENDOR_VALUE 0x04
+#define ATP_GEYSER_MODE_READ_REQUEST_ID		1
+#define ATP_GEYSER_MODE_WRITE_REQUEST_ID	9
+#define ATP_GEYSER_MODE_REQUEST_VALUE		0x300
+#define ATP_GEYSER_MODE_REQUEST_INDEX		0
+#define ATP_GEYSER_MODE_VENDOR_VALUE		0x04
 
 /* Structure to hold all of our device specific stuff */
 struct atp {
@@ -188,6 +188,14 @@ static int debug = 1;
 module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Activate debugging output");
 
+static inline int atp_is_old_fountain(struct atp *dev)
+{
+	u16 productId = le16_to_cpu(dev->udev->descriptor.idProduct);
+
+	return productId == FOUNTAIN_ANSI_PRODUCT_ID ||
+	       productId == FOUNTAIN_ISO_PRODUCT_ID;
+}
+
 /* Checks if the device a Geyser 2 (ANSI, ISO, JIS) */
 static inline int atp_is_geyser_2(struct atp *dev)
 {
@@ -211,52 +219,55 @@ static inline int atp_is_geyser_3(struct
 }
 
 /*
- * By default Geyser 3 device sends standard USB HID mouse
+ * By default newer Geyser devices send standard USB HID mouse
  * packets (Report ID 2). This code changes device mode, so it
  * sends raw sensor reports (Report ID 5).
  */
-static int atp_geyser3_init(struct usb_device *udev)
+static int atp_geyser_init(struct usb_device *udev)
 {
 	char data[8];
 	int size;
 
 	size = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
-			ATP_GEYSER3_MODE_READ_REQUEST_ID,
+			ATP_GEYSER_MODE_READ_REQUEST_ID,
 			USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-			ATP_GEYSER3_MODE_REQUEST_VALUE,
-			ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
+			ATP_GEYSER_MODE_REQUEST_VALUE,
+			ATP_GEYSER_MODE_REQUEST_INDEX, &data, 8, 5000);
 
 	if (size != 8) {
 		err("Could not do mode read request from device"
-		    " (Geyser 3 mode)");
+		    " (Geyser Raw mode)");
 		return -EIO;
 	}
 
 	/* Apply the mode switch */
-	data[0] = ATP_GEYSER3_MODE_VENDOR_VALUE;
+	data[0] = ATP_GEYSER_MODE_VENDOR_VALUE;
 
 	size = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-			ATP_GEYSER3_MODE_WRITE_REQUEST_ID,
+			ATP_GEYSER_MODE_WRITE_REQUEST_ID,
 			USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-			ATP_GEYSER3_MODE_REQUEST_VALUE,
-			ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
+			ATP_GEYSER_MODE_REQUEST_VALUE,
+			ATP_GEYSER_MODE_REQUEST_INDEX, &data, 8, 5000);
 
 	if (size != 8) {
 		err("Could not do mode write request to device"
-		    " (Geyser 3 mode)");
+		    " (Geyser Raw mode)");
 		return -EIO;
 	}
 	return 0;
 }
 
-/* Reinitialise the device if it's a geyser 3 */
+/*
+ * Reinitialise the device. This usually stops stream of empty packets
+ * coming form it.
+ */
 static void atp_reinit(struct work_struct *work)
 {
 	struct atp *dev = container_of(work, struct atp, work);
 	struct usb_device *udev = dev->udev;
 
 	dev->idlecount = 0;
-	atp_geyser3_init(udev);
+	atp_geyser_init(udev);
 }
 
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
@@ -507,19 +518,23 @@ static void atp_complete(struct urb* urb
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
-	/* Many Geysers 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 (!x && !y && !key) {
-		dev->idlecount++;
-		if (dev->idlecount == 10) {
-			dev->valid = 0;
-			schedule_work(&dev->work);
-		}
-	} else
-		dev->idlecount = 0;
+	/*
+	 * Many Geysers 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. Re-initialization does not
+	 * work on older versions of Fountain touchpads.
+	 */
+	if (!atp_is_old_fountain(dev)) {
+		if (!x && !y && !key) {
+			dev->idlecount++;
+			if (dev->idlecount == 10) {
+				dev->valid = 0;
+				schedule_work(&dev->work);
+			}
+		} else
+			dev->idlecount = 0;
+	}
 
 exit:
 	retval = usb_submit_urb(dev->urb, GFP_ATOMIC);
@@ -593,12 +608,12 @@ static int atp_probe(struct usb_interfac
 	else
 		dev->datalen = 81;
 
-	if (atp_is_geyser_3(dev)) {
+	if (!atp_is_old_fountain(dev)) {
 		/* switch to raw sensor mode */
-		if (atp_geyser3_init(udev))
+		if (atp_geyser_init(udev))
 			goto err_free_devs;
 
-		printk("appletouch Geyser 3 inited.\n");
+		printk(KERN_INFO "appletouch: Geyser mode initialized.\n");
 	}
 
 	dev->urb = usb_alloc_urb(0, GFP_KERNEL);

Re: [PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

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


> OK, then maybe instead of reverting the change outright we could try the
> patch below?

That patch works, minor comments:

> Older models of fountains do not support change mode request and

I think there's only one fountain model.

> therefore shoudl be excluded from idle reset attempts.

typo

>  /* MacBook Pro (Geyser 3 & 4) initialization constants */

That comment is no longer correct, you should change it.

> -#define ATP_GEYSER3_MODE_READ_REQUEST_ID 1

> + * Reinitialise the device. This usually stops stream of empty packets
> + * coming form it.

typo "from"

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] fix appletouch geyser 1 breakage

[Dmitry Torokhov]

On Sunday 28 October 2007, Johannes Berg wrote:
> 
> > OK, then maybe instead of reverting the change outright we could try the
> > patch below?
> 
> That patch works,

Any chance Benjamin could also test it? The behaviour is different
from 2.6.24-rc1 since we call atp_geyser_init for all geysers now.
 
> minor comments: 
> 
> > Older models of fountains do not support change mode request and
> 
> I think there's only one fountain model.

I was hoping that FOUNTAIN_TP_ONLY_PRODUCT_ID (0x30A) behaves similar
to Geyser in this regard. If you know that this assumption is incorrect
then we need to rename atp_is_older_fountain() to atp_is_fountain()
anf add this product ID to it.

> 
> > therefore shoudl be excluded from idle reset attempts.
> 
> typo

OK

> 
> >  /* MacBook Pro (Geyser 3 & 4) initialization constants */
> 
> That comment is no longer correct, you should change it.
> 

OK

> > -#define ATP_GEYSER3_MODE_READ_REQUEST_ID 1
> 
> > + * Reinitialise the device. This usually stops stream of empty packets
> > + * coming form it.
> 
> typo "from"
> 

Yep, thanks.

-- 
Dmitry

Re: [PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

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


> I was hoping that FOUNTAIN_TP_ONLY_PRODUCT_ID (0x30A) behaves similar
> to Geyser in this regard. If you know that this assumption is incorrect
> then we need to rename atp_is_older_fountain() to atp_is_fountain()
> anf add this product ID to it.

Ah ok, I forgot about that one. If I were to venture a guess I'd say
that they renamed it to geyser because of this idle behaviour
"technology upgrade", but I can't say, nor do I know anybody with such a
version, sorry.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] fix appletouch geyser 1 breakage

[Dmitry Torokhov]

On Sunday 28 October 2007 11:08, Johannes Berg wrote:
> 
> > I was hoping that FOUNTAIN_TP_ONLY_PRODUCT_ID (0x30A) behaves similar
> > to Geyser in this regard. If you know that this assumption is incorrect
> > then we need to rename atp_is_older_fountain() to atp_is_fountain()
> > anf add this product ID to it.
> 
> Ah ok, I forgot about that one. If I were to venture a guess I'd say
> that they renamed it to geyser because of this idle behaviour
> "technology upgrade", but I can't say, nor do I know anybody with such a
> version, sorry.

OK then let's play safe and don't touch fountains at all. How about the
patch below?

-- 
Dmitry

Input: appletouch - idle reset logic broke older Fountains

Fountains do not support change mode request and therefore
should be excluded from idle reset attempts.

Also:
 - do not re-submit URB when we decide that toucvhpad needs to be
   reinicialized
 - do not repeat size detection when reinitializing the touchpad
 - Add missing KERN_* prefixes to messages

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/mouse/appletouch.c |  125 ++++++++++++++++++++++++---------------
 1 file changed, 77 insertions(+), 48 deletions(-)

Index: work/drivers/input/mouse/appletouch.c
===================================================================
--- work.orig/drivers/input/mouse/appletouch.c
+++ work/drivers/input/mouse/appletouch.c
@@ -129,12 +129,12 @@ MODULE_DEVICE_TABLE (usb, atp_table);
  */
 #define ATP_THRESHOLD	 5
 
-/* MacBook Pro (Geyser 3 & 4) initialization constants */
-#define ATP_GEYSER3_MODE_READ_REQUEST_ID 1
-#define ATP_GEYSER3_MODE_WRITE_REQUEST_ID 9
-#define ATP_GEYSER3_MODE_REQUEST_VALUE 0x300
-#define ATP_GEYSER3_MODE_REQUEST_INDEX 0
-#define ATP_GEYSER3_MODE_VENDOR_VALUE 0x04
+/* Geyser initialization constants */
+#define ATP_GEYSER_MODE_READ_REQUEST_ID		1
+#define ATP_GEYSER_MODE_WRITE_REQUEST_ID	9
+#define ATP_GEYSER_MODE_REQUEST_VALUE		0x300
+#define ATP_GEYSER_MODE_REQUEST_INDEX		0
+#define ATP_GEYSER_MODE_VENDOR_VALUE		0x04
 
 /* Structure to hold all of our device specific stuff */
 struct atp {
@@ -142,9 +142,11 @@ struct atp {
 	struct usb_device *	udev;		/* usb device */
 	struct urb *		urb;		/* usb request block */
 	signed char *		data;		/* transferred data */
-	int			open;		/* non-zero if opened */
-	struct input_dev	*input;		/* input dev */
-	int			valid;		/* are the sensors valid ? */
+	struct input_dev *	input;		/* input dev */
+	unsigned char		open;		/* non-zero if opened */
+	unsigned char		valid;		/* are the sensors valid ? */
+	unsigned char		size_detect_done;
+	unsigned char		overflowwarn;	/* overflow warning printed? */
 	int			x_old;		/* last reported x/y, */
 	int			y_old;		/* used for smoothing */
 						/* current value of the sensors */
@@ -153,7 +155,6 @@ struct atp {
 	signed char		xy_old[ATP_XSENSORS + ATP_YSENSORS];
 						/* accumulated sensors */
 	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
-	int			overflowwarn;	/* overflow warning printed? */
 	int			datalen;	/* size of an USB urb transfer */
 	int			idlecount;      /* number of empty packets */
 	struct work_struct      work;
@@ -170,7 +171,7 @@ struct atp {
 
 #define dprintk(format, a...)						\
 	do {								\
-		if (debug) printk(format, ##a);				\
+		if (debug) printk(KERN_DEBUG format, ##a);				\
 	} while (0)
 
 MODULE_AUTHOR("Johannes Berg, Stelian Pop, Frank Arnold, Michael Hanselmann");
@@ -188,6 +189,15 @@ static int debug = 1;
 module_param(debug, int, 0644);
 MODULE_PARM_DESC(debug, "Activate debugging output");
 
+static inline int atp_is_fountain(struct atp *dev)
+{
+	u16 productId = le16_to_cpu(dev->udev->descriptor.idProduct);
+
+	return productId == FOUNTAIN_ANSI_PRODUCT_ID ||
+	       productId == FOUNTAIN_ISO_PRODUCT_ID ||
+	       productId == FOUNTAIN_TP_ONLY_PRODUCT_ID;
+}
+
 /* Checks if the device a Geyser 2 (ANSI, ISO, JIS) */
 static inline int atp_is_geyser_2(struct atp *dev)
 {
@@ -211,52 +221,63 @@ static inline int atp_is_geyser_3(struct
 }
 
 /*
- * By default Geyser 3 device sends standard USB HID mouse
+ * By default newer Geyser devices send standard USB HID mouse
  * packets (Report ID 2). This code changes device mode, so it
  * sends raw sensor reports (Report ID 5).
  */
-static int atp_geyser3_init(struct usb_device *udev)
+static int atp_geyser_init(struct usb_device *udev)
 {
 	char data[8];
 	int size;
 
 	size = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
-			ATP_GEYSER3_MODE_READ_REQUEST_ID,
+			ATP_GEYSER_MODE_READ_REQUEST_ID,
 			USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-			ATP_GEYSER3_MODE_REQUEST_VALUE,
-			ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
+			ATP_GEYSER_MODE_REQUEST_VALUE,
+			ATP_GEYSER_MODE_REQUEST_INDEX, &data, 8, 5000);
 
 	if (size != 8) {
 		err("Could not do mode read request from device"
-		    " (Geyser 3 mode)");
+		    " (Geyser Raw mode)");
 		return -EIO;
 	}
 
 	/* Apply the mode switch */
-	data[0] = ATP_GEYSER3_MODE_VENDOR_VALUE;
+	data[0] = ATP_GEYSER_MODE_VENDOR_VALUE;
 
 	size = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-			ATP_GEYSER3_MODE_WRITE_REQUEST_ID,
+			ATP_GEYSER_MODE_WRITE_REQUEST_ID,
 			USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-			ATP_GEYSER3_MODE_REQUEST_VALUE,
-			ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
+			ATP_GEYSER_MODE_REQUEST_VALUE,
+			ATP_GEYSER_MODE_REQUEST_INDEX, &data, 8, 5000);
 
 	if (size != 8) {
 		err("Could not do mode write request to device"
-		    " (Geyser 3 mode)");
+		    " (Geyser Raw mode)");
 		return -EIO;
 	}
 	return 0;
 }
 
-/* Reinitialise the device if it's a geyser 3 */
+/*
+ * Reinitialise the device. This usually stops stream of empty packets
+ * coming from it.
+ */
 static void atp_reinit(struct work_struct *work)
 {
 	struct atp *dev = container_of(work, struct atp, work);
 	struct usb_device *udev = dev->udev;
+	int retval;
 
 	dev->idlecount = 0;
-	atp_geyser3_init(udev);
+
+	atp_geyser_init(udev);
+
+	retval = usb_submit_urb(dev->urb, GFP_ATOMIC);
+	if (retval) {
+		err("%s - usb_submit_urb failed with result %d",
+		    __FUNCTION__, retval);
+	}
 }
 
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
@@ -337,7 +358,7 @@ static void atp_complete(struct urb* urb
 		break;
 	case -EOVERFLOW:
 		if(!dev->overflowwarn) {
-			printk("appletouch: OVERFLOW with data "
+			printk(KERN_WARNING "appletouch: OVERFLOW with data "
 				"length %d, actual length is %d\n",
 				dev->datalen, dev->urb->actual_length);
 			dev->overflowwarn = 1;
@@ -426,15 +447,17 @@ static void atp_complete(struct urb* urb
 		dev->x_old = dev->y_old = -1;
 		memcpy(dev->xy_old, dev->xy_cur, sizeof(dev->xy_old));
 
-		if (atp_is_geyser_3(dev)) /* No 17" Macbooks (yet) */
+		if (dev->size_detect_done ||
+		    atp_is_geyser_3(dev)) /* No 17" Macbooks (yet) */
 			goto exit;
 
 		/* 17" Powerbooks have extra X sensors */
-		for (i = (atp_is_geyser_2(dev)?15:16); i < ATP_XSENSORS; i++) {
-			if (!dev->xy_cur[i]) continue;
+		for (i = (atp_is_geyser_2(dev) ? 15 : 16); i < ATP_XSENSORS; i++) {
+			if (!dev->xy_cur[i])
+				continue;
 
-			printk("appletouch: 17\" model detected.\n");
-			if(atp_is_geyser_2(dev))
+			printk(KERN_INFO "appletouch: 17\" model detected.\n");
+			if (atp_is_geyser_2(dev))
 				input_set_abs_params(dev->input, ABS_X, 0,
 						     (20 - 1) *
 						     ATP_XFACT - 1,
@@ -444,10 +467,10 @@ static void atp_complete(struct urb* urb
 						     (ATP_XSENSORS - 1) *
 						     ATP_XFACT - 1,
 						     ATP_FUZZ, 0);
-
 			break;
 		}
 
+		dev->size_detect_done = 1;
 		goto exit;
 	}
 
@@ -479,7 +502,7 @@ static void atp_complete(struct urb* urb
 			dev->y_old = y;
 
 			if (debug > 1)
-				printk("appletouch: X: %3d Y: %3d "
+				printk(KERN_DEBUG "appletouch: X: %3d Y: %3d "
 				       "Xz: %3d Yz: %3d\n",
 				       x, y, x_z, y_z);
 
@@ -507,19 +530,25 @@ static void atp_complete(struct urb* urb
 	input_report_key(dev->input, BTN_LEFT, key);
 	input_sync(dev->input);
 
-	/* Many Geysers 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 (!x && !y && !key) {
-		dev->idlecount++;
-		if (dev->idlecount == 10) {
-			dev->valid = 0;
-			schedule_work(&dev->work);
-		}
-	} else
-		dev->idlecount = 0;
+	/*
+	 * Many Geysers 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. Re-initialization does not
+	 * work on Fountain touchpads.
+	 */
+	if (!atp_is_fountain(dev)) {
+		if (!x && !y && !key) {
+			dev->idlecount++;
+			if (dev->idlecount == 10) {
+				dev->valid = 0;
+				schedule_work(&dev->work);
+				/* Don't resubmit urb here, wait for reinit */
+				return;
+			}
+		} else
+			dev->idlecount = 0;
+	}
 
 exit:
 	retval = usb_submit_urb(dev->urb, GFP_ATOMIC);
@@ -593,12 +622,12 @@ static int atp_probe(struct usb_interfac
 	else
 		dev->datalen = 81;
 
-	if (atp_is_geyser_3(dev)) {
+	if (!atp_is_fountain(dev)) {
 		/* switch to raw sensor mode */
-		if (atp_geyser3_init(udev))
+		if (atp_geyser_init(udev))
 			goto err_free_devs;
 
-		printk("appletouch Geyser 3 inited.\n");
+		printk(KERN_INFO "appletouch: Geyser mode initialized.\n");
 	}
 
 	dev->urb = usb_alloc_urb(0, GFP_KERNEL);

Re: [PATCH] fix appletouch geyser 1 breakage

[Johannes Berg]

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



> OK then let's play safe and don't touch fountains at all. How about the
> patch below?

Looks fine to me, works with my fountain touchpad and should fix
Joseph's error too.

> Input: appletouch - idle reset logic broke older Fountains
> 
> Fountains do not support change mode request and therefore
> should be excluded from idle reset attempts.
> 
> Also:
>  - do not re-submit URB when we decide that toucvhpad needs to be
>    reinicialized
>  - do not repeat size detection when reinitializing the touchpad
>  - Add missing KERN_* prefixes to messages
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
>  drivers/input/mouse/appletouch.c |  125 ++++++++++++++++++++++++---------------
>  1 file changed, 77 insertions(+), 48 deletions(-)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]