Just a heads up - After some lame hacking I finally have got the trackpad on the PB5,8 (15" Late Oct 2005) to work.
Ok, here is the current definition of working - mouse moves but for that you have to roll over on the trackpad ;)!
Currently negotiating with it so that it's happy with the fingers!! :D
Will post the modified source once for appletouch once it starts working acceptably.
Parag
On Tue, 2005-11-15 at 21:43 +0000, Parag Warudkar wrote:
> Just a heads up - After some lame hacking I finally have got the trackpad on the PB5,8 (15" Late Oct 2005) to work.
>
> Ok, here is the current definition of working - mouse moves but for that you have to roll over on the trackpad ;)!
>
> Currently negotiating with it so that it's happy with the fingers!! :D
>
> Will post the modified source once for appletouch once it starts working acceptably.
hi Parag !
You should probably post those updates to [email protected]
Ben
On Nov 15, 2005, at 6:06 PM, Benjamin Herrenschmidt wrote:
>
> hi Parag !
>
> You should probably post those updates to [email protected]
>
> Ben
>
Hi
Thanks, I wasn't aware of that. Will post there once it's done.
Parg
Parag Warudkar wrote:
> Just a heads up - After some lame hacking I finally have got the trackpad
> on the PB5,8 (15" Late Oct 2005) to work.
Can you tell us what's different? I'd like to work that into my page that
documents the protocol.
johannes
> Parag Warudkar wrote:
> > Just a heads up - After some lame hacking I finally have got the trackpad
> > on the PB5,8 (15" Late Oct 2005) to work.
>
> Can you tell us what's different? I'd like to work that into my page that
> documents the protocol.
>
> johannes
Sure but I haven't worked out the details of the changes yet. All I am fairly certain at this point is that
1) New Trackpad sends in more data than the old one. ATP_DATASIZE has to be increased from 81 to 256. (The original code was dying with EOVERFLOW from the URB complete routine. 256 is the minimum value it is happy with.)
2) The 15" also has 26 x 16 sensors - not sure how many the 17" has - most likely same
3) The FUZZ, PRESSURE, XFACT and YFACT, along with the button reporting all needs to be reworked - this is my preliminary opinion, might be wrong but with the original code, the mouse moves erratically and the button events don't seem to be delivered right.
Will post complete details once I have it working.
Thanks
Parag
Ah and yes I forgot about the dual finger scrolling function - newer trackpads allow you to use 2 fingers to slide up and down to achieve scrolling. I don't see any code in current appletouch.c which handles this. We might be able to live without it for some time but maybe we can't as it might screw up the reporting if not handled. (PPC assembly is fun to work with :)
Parag
> > Parag Warudkar wrote:
> > > Just a heads up - After some lame hacking I finally have got the trackpad
> > > on the PB5,8 (15" Late Oct 2005) to work.
> >
> > Can you tell us what's different? I'd like to work that into my page that
> > documents the protocol.
> >
> > johannes
>
> Sure but I haven't worked out the details of the changes yet. All I am fairly
> certain at this point is that
>
> 1) New Trackpad sends in more data than the old one. ATP_DATASIZE has to be
> increased from 81 to 256. (The original code was dying with EOVERFLOW from the
> URB complete routine. 256 is the minimum value it is happy with.)
>
> 2) The 15" also has 26 x 16 sensors - not sure how many the 17" has - most
> likely same
>
> 3) The FUZZ, PRESSURE, XFACT and YFACT, along with the button reporting all
> needs to be reworked - this is my preliminary opinion, might be wrong but with
> the original code, the mouse moves erratically and the button events don't seem
> to be delivered right.
>
> Will post complete details once I have it working.
>
> Thanks
>
> Parag
>
>
>
On Wed, 2005-11-16 at 15:43 +0000, Parag Warudkar wrote:
> Ah and yes I forgot about the dual finger scrolling function - newer
> trackpads allow you to use 2 fingers to slide up and down to achieve
> scrolling. I don't see any code in current appletouch.c which handles
> this. We might be able to live without it for some time but maybe we
> can't as it might screw up the reporting if not handled. (PPC assembly
> is fun to work with :)
That's a pure driver function, and we didn't implement it because we
emulate synaptics. I think your erratically moving thing might be caused
by a relayout of the order, maybe you should try to observe the
interrupt transfers the device gives you in a systematic way like I did
with my python scripts? Those display the levels of each byte. Maybe
they changed to transmitting not bytes but words for each level?
johannes
> That's a pure driver function, and we didn't implement it because we
> emulate synaptics. I think your erratically moving thing might be caused
> by a relayout of the order, maybe you should try to observe the
> interrupt transfers the device gives you in a systematic way like I did
> with my python scripts? Those display the levels of each byte. Maybe
> they changed to transmitting not bytes but words for each level?
>
> johannes
>
Yep the data layout/ordering is cetainly changed. I was thinking of writing something to relayfs and then use scripts to parse and interpret. But now I think I will be better off using your driver+scripts to sample the data.
Thanks!
Parag
On Wed, 2005-11-16 at 16:07 +0000, Parag Warudkar wrote:
> Yep the data layout/ordering is cetainly changed. I was thinking of
> writing something to relayfs and then use scripts to parse and
> interpret. But now I think I will be better off using your driver
> +scripts to sample the data.
That explains a lot :)
You'll want to start without appletouch then, it only gets in your way
interpreting the data.
relayfs is certainly a better idea then trying to do this via udp or
something as I did first =)
Good luck,
johannes
On Nov 21, 2005, at 6:57 PM, Parag Warudkar wrote:
> <appletouch.c>
Oops - originally attached file has wrong comment, this one has
fixed comments - other wise the code is same.
Also, one more change required was to add ATP_DEVICE(0x0214) into the
usb_device_id table which I forgot
to mention in the last mail.
Parag
Parag Warudkar wrote:
> Ok, the format interpretation of the trackpad data is taking up more
> of my time and brain than I was happy with! :)
Thanks for sending the data to me, I'll take a look later.
I have doubts, however, about using the appletouch driver, if the format
changed significantly then the code will be largely different.
johannes
Hello Parag
> Attached is the code which takes care of above 3 - tested but it
> should be considered half baked for obvious reasons and
I hacked your code some more to make it work on my October 2005
PowerBook 1.67 GHz. The product ID I have, 0x0215, was in none of the
available drivers and the data format is somewhat different.
You find my hacked version attached -- be aware that in its current form
it will not work with any touchpad except 0x0215.
> in addition I added some relayfs write calls [...]
I wrapped them into #if's, so one is not required to have relayfs in the
kernel to use the driver.
> The code might break the existing (old) trackpads as the detection
> might not be correct.
My changes do that definitively, but it's only a hack.
As far as I see it, all methods can be built into one driver. Is there
already someone working on combining them?
Greets,
Michael
--
Gentoo Linux Developer using m0n0wall | http://hansmi.ch/
On Nov 28, 2005, at 7:06 PM, Michael Hanselmann wrote:
> The product ID I have, 0x0215, was in none of the
> available drivers and the data format is somewhat different
Hi Michael
Is yours the 15" model or the 17"? Mine is 15" and the product id is
0x0214.
> You find my hacked version attached -- be aware that in its current
> form
> it will not work with any touchpad except 0x0215.
I haven't looked at your changes completely yet but are you saying it
works? Meaning mouse
moves properly?
Also I find it strange that your model requires 80 bytes ATP_DATASIZE
- mine isn't happy
at all with anything less than 256. The less number of sensors you
defined is again a puzzle.
> As far as I see it, all methods can be built into one driver. Is there
> already someone working on combining them?
If the format of the data is same (which looks like it is with your
model) then yes, but in my case
the data arrives is 64 byte blocks - there are 4 of them in one
transfer, each a reading on it's own.
Hmm. More confusion.
Thanks
Parag
Hello Parag
On Tue, Nov 29, 2005 at 01:11:00AM -0500, Parag Warudkar wrote:
> Is yours the 15" model or the 17"? Mine is 15" and the product id is
> 0x0214.
It's the 15" one.
> I haven't looked at your changes completely yet but are you saying it
> works? Meaning mouse moves properly?
The mouse moves, but slowly. Maybe something isn't correct yet, but it
works basically.
> Also I find it strange that your model requires 80 bytes ATP_DATASIZE
> - mine isn't happy at all with anything less than 256. The less number
> of sensors you defined is again a puzzle.
That are points I need to investigate further.
> If the format of the data is same (which looks like it is with your
> model) then yes, but in my case the data arrives is 64 byte blocks -
> there are 4 of them in one transfer, each a reading on it's own.
I get 256 bytes in each transfer as well, but didn't look at the bytes
behind 40. Maybe that'll help to make it more responsive.
> Hmm. More confusion.
Oh yes. Why does Apple ship the basically same PowerBook with different
Touchpads?
Greets,
Michael
--
Gentoo Linux Developer using m0n0wall | http://hansmi.ch/
Michael Hanselmann wrote:
>> I haven't looked at your changes completely yet but are you saying it
>> works? Meaning mouse moves properly?
>
> The mouse moves, but slowly. Maybe something isn't correct yet, but it
> works basically.
All this sounds almost like your touchpad is much more similar to the old
one. I'll most likely have access to a new powerbook within the next two
weeks, so I'll be able to take a look.
johannes
On Nov 29, 2005, at 2:50 AM, Michael Hanselmann wrote:
> The mouse moves, but slowly. Maybe something isn't correct yet, but it
> works basically.
Yeah, mine works like that too - but sometimes it will go left->right
when you moved the finger
right->left and vice versa.
> I get 256 bytes in each transfer as well, but didn't look at the bytes
> behind 40. Maybe that'll help to make it more responsive.
Nope, if yours works with 80 bytes, it only sends 80. It will still
work with anything > 80 but that's superfluous data.
(E,g, Mine works even with 1024 - anything above 256 seems junk, but
with < 256 it dies with EOVERFLOW )
Right now I have detected that byte # 13 increases when moving finger
along the bottom from left corner to right,
and byte #11 changes when moved from bottom left corner to top left.
Gotta figure out the rest of the movement patterns
along with how many total sensors X and Y are there and how to relate
the data to something to report to the input device!
Hopefully Johannes will have a PowerBook with either of the 0x0214 or
0x0215 touchpads
and we will make some headway!
Parag
Parag Warudkar wrote:
> Hopefully Johannes will have a PowerBook with either of the 0x0214 or
> 0x0215 touchpads and we will make some headway!
If anyone is in the area of Paderborn (Germany) who has such a touchpad,
it may be faster if I just look at that one :) If not, well, we can sort
of hope that the one my brother's ordering will have that touchpad.
johannes
> My changes do that definitively, but it's only a hack.
I now integrated support for the 0x0215 device into the driver of
2.6.15-rc3, in addition to the relayfs functions.
It would be great to get some feedback from people who have touchpads
handled by appletouch.c other than 0x0215.
The patch is attached for easier use. If nobody speaks up, I'll submit
this one, as it's simply required for the touchpad on the new Oct 2005
PowerBooks (at least 15").
--
Gentoo Linux Developer using m0n0wall | http://hansmi.ch/
On Wed, Nov 30, 2005 at 11:39:17PM +0100, Michael Hanselmann wrote:
> The patch is attached for easier use.
There was a mistake in it due to which the mouse button wouldn't work.
Fixed in the now attached patch.
Le jeudi 01 d?cembre 2005 ? 00:46 +0100, Michael Hanselmann a ?crit :
> On Wed, Nov 30, 2005 at 11:39:17PM +0100, Michael Hanselmann wrote:
> > The patch is attached for easier use.
>
> There was a mistake in it due to which the mouse button wouldn't work.
> Fixed in the now attached patch.
Is this version really working well on the new Powerbooks ? From what
I've seen in this thread there are still issues and it's still a work in
progress, so it may be too early to integrate the changes in the kernel.
Also, some other comments on the code itself:
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+#include <linux/relayfs_fs.h>
+#endif
While the relayfs code is ok for debugging, I'm wondering if it should be left in the final version at all.
+ int is0215; /* is the device a 0x0215? */
No need for that, just use udev->descriptor.idProduct == 0x0215 (in a macro perhaps)
+ int overflowwarn; /* overflow warning printed? */
I would use a static variable in the case -OVERFLOW: block here.
+ dev->xy_cur[i++] = dev->data[19];
+ dev->xy_cur[i++] = dev->data[20];
+ dev->xy_cur[i++] = dev->data[22];
+ dev->xy_cur[i++] = dev->data[23];
There is obviously a pattern here:
for (i = 0; i < 15; i++)
dev->xy_cur[i] = dev->data[ 19 + (i * 3) / 2 ]
I'm wondering if the same formula doesn't apply for more X and Y sensors (like 16 X
and 16 Y sensors on the old Powerbooks, 26 for the 17" models)
+#if 0
+ /* Some debug code */
+ for (i = 0; i < dev->urb->actual_length; i++) {
+ printk("%2x,", (unsigned char)dev->data[i]);
+ }
+ printk("\n");
+#endif
Please dump that.
+ /* Prints the read values */
+ if (debug > 1) {
+ printk("appletouch: X=");
+ for (i = 0; i < 15; i++) {
+ printk("%2x,", (unsigned char)dev->xy_cur[i]);
+ }
+ printk(" Y=");
+ for (i = ATP_XSENSORS; i < (ATP_XSENSORS + (9 - 1)); i++) {
+ printk("%2x,", (unsigned char)dev->xy_cur[i]);
+ }
+ printk("\n");
+ }
What is the point in doing this since the dbg_dump is called a few lines
later ? Best is to modify dbg_dump to know about the new number of
sensors...
+ printk(KERN_INFO "appletouch: atp_probe found interrupt "
+ "in endpoint: %d\n", int_in_endpointAddr);
Why is this useful to know ?
+ if (dev->is0215) {
+ dev->datalen = 64;
+ } else {
+ dev->datalen = 81;
+ }
Braces are not needed here.
PS: please inline the patch instead of attaching it to the mail, it's
much more easy to quote it that way.
Stelian.
--
Stelian Pop <[email protected]>
My original patch is still work in progress and was intended as a starting point if someone was already adept with the trackpad stuff and was willing to help.
(Basically it was a call for help - nothing for end users ;)
Things that remained to be done -
Either
1) Support for all PowerBooks in one code base (> Feb 2005)
Or
1) Create different versions for each one or two of them
Depending upon whether or not widely varying algorithms are needed for each variety.
2) Major part - reliably working code for finger movement detection for all Oct 2005 PowerBooks (15" itself seems to come with trackpads having entirely different characteristics.)
3) Possibly Dual Finger Detection for scrolling - this is optional but would be good to have.
I am working on it as time and urges permit and it will speed up only if Johannes doesn't get his PowerBook or the one he gets is 0x0216 !!
Parag
On Fri, Dec 02, 2005 at 03:28:31PM +0100, Stelian Pop wrote:
> Is this version really working well on the new Powerbooks ? From what
> I've seen in this thread there are still issues and it's still a work
> in progress, so it may be too early to integrate the changes in the
> kernel.
It works fine for the 15" PowerBooks (Oct 2005) and has been tested by
at least three people. 17" should work too, but I can't test that until
later this week.
> +#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
> +#include <linux/relayfs_fs.h>
> +#endif
> While the relayfs code is ok for debugging, I'm wondering if it should
> be left in the final version at all.
It doesn't hurt as it's not much code, it makes updating the driver for
newer devices easier and it's only enabled if requested. Because of
that, I would leave it in. However, I can split it into a separate patch
if wanted.
> + int overflowwarn; /* overflow warning printed? */
> I would use a static variable in the case -OVERFLOW: block here.
As there could be, due to whatever reason, multiple devices using the
same driver, one could overflow while another doesn't. Therefore, it
should be device specific.
> + dev->xy_cur[i++] = dev->data[19];
> + dev->xy_cur[i++] = dev->data[20];
> + dev->xy_cur[i++] = dev->data[22];
> + dev->xy_cur[i++] = dev->data[23];
> There is obviously a pattern here:
> for (i = 0; i < 15; i++)
> dev->xy_cur[i] = dev->data[ 19 + (i * 3) / 2 ]
It's not that easy. This code wouldn't work as it should.
I wrote a working loop into the patch below, but as the code's called
about 1'000 times a second I unrolled the loop myself. A friend of mine
calculated that the code would take at least twice the time to run when
written using a loop.
> I'm wondering if the same formula doesn't apply for more X and Y
> sensors (like 16 X and 16 Y sensors on the old Powerbooks, 26 for the
> 17" models)
It does and is addressed in the new patch below.
> What is the point in doing this since the dbg_dump is called a few lines
> later ?
Those were some leftovers from my debugging and would have been deleted
before submitting. I just didn't want to take everything out if someone
else wants to do some extensive debugging or so.
Here's an updated patch including support for the 17" PowerBooks (Oct
2005). The informations about the 17" one are from Alex Harper.
---
--- linux-2.6.15-rc5/drivers/usb/input/appletouch.c.orig 2005-12-04 20:25:21.000000000 +0100
+++ linux-2.6.15-rc5/drivers/usb/input/appletouch.c 2005-12-04 23:37:29.000000000 +0100
@@ -6,9 +6,19 @@
* Copyright (C) 2005 Stelian Pop ([email protected])
* Copyright (C) 2005 Frank Arnold ([email protected])
* Copyright (C) 2005 Peter Osterlund ([email protected])
+ * Copyright (C) 2005 Parag Warudkar ([email protected])
+ * Copyright (C) 2005 Michael Hanselmann ([email protected])
*
* Thanks to Alex Harper <[email protected]> for his inputs.
*
+ * Nov 2005 - Parag Warudkar
+ * o Added ability to export data via relayfs
+ *
+ * Nov 2005 - Michael Hanselmann
+ * o Compile relayfs support only if enabled in the kernel
+ * o Enable relayfs only if requested by the user
+ * o Added support for new October 2005 PowerBooks (Geyser 2)
+ *
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
@@ -35,8 +45,13 @@
#include <linux/input.h>
#include <linux/usb_input.h>
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+#include <linux/relayfs_fs.h>
+#endif
+
/* Apple has powerbooks which have the keyboard with different Product IDs */
#define APPLE_VENDOR_ID 0x05AC
+#define GEYSER_2_PRODUCT_ID 0x0215
#define ATP_DEVICE(prod) \
.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
@@ -51,14 +66,17 @@
static struct usb_device_id atp_table [] = {
{ ATP_DEVICE(0x020E) },
{ ATP_DEVICE(0x020F) },
+ { ATP_DEVICE(GEYSER_2_PRODUCT_ID) }, /* PowerBooks Oct 2005 */
{ ATP_DEVICE(0x030A) },
{ ATP_DEVICE(0x030B) },
{ } /* Terminating entry */
};
MODULE_DEVICE_TABLE (usb, atp_table);
-/* size of a USB urb transfer */
-#define ATP_DATASIZE 81
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+struct rchan* rch = NULL;
+struct rchan_callbacks* rcb = NULL;
+#endif
/*
* number of sensors. Note that only 16 instead of 26 X (horizontal)
@@ -73,6 +91,7 @@
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
+
/*
* multiplication factor for the X and Y coordinates.
* We try to keep the touchpad aspect ratio while still doing only simple
@@ -108,6 +127,8 @@
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 */
};
#define dbg_dump(msg, tab) \
@@ -124,7 +145,11 @@
if (debug) printk(format, ##a); \
} while (0)
-MODULE_AUTHOR("Johannes Berg, Stelian Pop, Frank Arnold");
+/* Checks if the device a Geyser 2 */
+#define IS_GEYSER_2(dev) \
+ (le16_to_cpu(dev->udev->descriptor.idProduct) == GEYSER_2_PRODUCT_ID)
+
+MODULE_AUTHOR("Johannes Berg, Stelian Pop, Frank Arnold, Parag Warudkar, Michael Hanselmann");
MODULE_DESCRIPTION("Apple PowerBooks USB touchpad driver");
MODULE_LICENSE("GPL");
@@ -132,6 +157,10 @@
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Activate debugging output");
+static int relayfs = 0;
+module_param(relayfs, int, 0644);
+MODULE_PARM_DESC(relayfs, "Activate relayfs support");
+
static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
int *z, int *fingers)
{
@@ -175,6 +204,13 @@
case 0:
/* success */
break;
+ case -EOVERFLOW:
+ if(!dev->overflowwarn) {
+ printk("appletouch: OVERFLOW with data "
+ "length %d, actual length is %d\n",
+ dev->datalen, dev->urb->actual_length);
+ dev->overflowwarn = 1;
+ }
case -ECONNRESET:
case -ENOENT:
case -ESHUTDOWN:
@@ -189,23 +225,83 @@
}
/* drop incomplete datasets */
- if (dev->urb->actual_length != ATP_DATASIZE) {
+ if (dev->urb->actual_length != dev->datalen) {
dprintk("appletouch: incomplete data package.\n");
goto exit;
}
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+ if (relayfs && dev->data) {
+ relay_write(rch, dev->data, dev->urb->actual_length);
+ }
+#endif
+
/* reorder the sensors values */
- for (i = 0; i < 8; i++) {
+ if (IS_GEYSER_2(dev)) {
+ memset(dev->xy_cur, 0, sizeof(dev->xy_cur));
+
+ /*
+ * The values are laid out like this:
+ * Y1, Y2, -, Y3, Y4, -, ...
+ * '-' is an unused value.
+ *
+ * The logic in a loop:
+ * for (i = 0, j = 19; i < 20; i += 2, j += 3) {
+ * dev->xy_cur[i] = dev->data[j];
+ * dev->xy_cur[i + 1] = dev->data[j + 1];
+ * }
+ *
+ * This code is called about 1'000 times per second for each
+ * interrupt from the touchpad. Therefore it should be as fast
+ * as possible. Writing the following code in a loop would take
+ * about twice the time to run if not more.
+ */
+
/* X values */
- dev->xy_cur[i ] = dev->data[5 * i + 2];
- dev->xy_cur[i + 8] = dev->data[5 * i + 4];
- dev->xy_cur[i + 16] = dev->data[5 * i + 42];
- if (i < 2)
- dev->xy_cur[i + 24] = dev->data[5 * i + 44];
+ dev->xy_cur[0] = dev->data[19];
+ dev->xy_cur[1] = dev->data[20];
+ dev->xy_cur[2] = dev->data[22];
+ dev->xy_cur[3] = dev->data[23];
+ dev->xy_cur[4] = dev->data[25];
+ dev->xy_cur[5] = dev->data[26];
+ dev->xy_cur[6] = dev->data[28];
+ dev->xy_cur[7] = dev->data[29];
+ dev->xy_cur[8] = dev->data[31];
+ dev->xy_cur[9] = dev->data[32];
+ dev->xy_cur[10] = dev->data[34];
+ dev->xy_cur[11] = dev->data[35];
+ dev->xy_cur[12] = dev->data[37];
+ dev->xy_cur[13] = dev->data[38];
+ dev->xy_cur[14] = dev->data[40];
+ dev->xy_cur[15] = dev->data[41];
+ dev->xy_cur[16] = dev->data[43];
+ dev->xy_cur[17] = dev->data[44];
+ dev->xy_cur[18] = dev->data[46];
+ dev->xy_cur[19] = dev->data[47];
/* Y values */
- dev->xy_cur[i + 26] = dev->data[5 * i + 1];
- dev->xy_cur[i + 34] = dev->data[5 * i + 3];
+ dev->xy_cur[ATP_XSENSORS + 0] = dev->data[1];
+ dev->xy_cur[ATP_XSENSORS + 1] = dev->data[2];
+ dev->xy_cur[ATP_XSENSORS + 2] = dev->data[4];
+ dev->xy_cur[ATP_XSENSORS + 3] = dev->data[5];
+ dev->xy_cur[ATP_XSENSORS + 4] = dev->data[7];
+ dev->xy_cur[ATP_XSENSORS + 5] = dev->data[8];
+ dev->xy_cur[ATP_XSENSORS + 6] = dev->data[10];
+ dev->xy_cur[ATP_XSENSORS + 7] = dev->data[11];
+ dev->xy_cur[ATP_XSENSORS + 8] = dev->data[13];
+ } else {
+ for (i = 0; i < 8; i++) {
+ /* X values */
+ dev->xy_cur[i ] = dev->data[5 * i + 2];
+ dev->xy_cur[i + 8] = dev->data[5 * i + 4];
+ dev->xy_cur[i + 16] = dev->data[5 * i + 42];
+ if (i < 2)
+ dev->xy_cur[i + 24] = dev->data[5 * i + 44];
+
+ /* Y values */
+ dev->xy_cur[i + 26] = dev->data[5 * i + 1];
+ dev->xy_cur[i + 34] = dev->data[5 * i + 3];
+ }
}
dbg_dump("sample", dev->xy_cur);
@@ -216,16 +312,23 @@
dev->x_old = dev->y_old = -1;
memcpy(dev->xy_old, dev->xy_cur, sizeof(dev->xy_old));
- /* 17" Powerbooks have 10 extra X sensors */
- for (i = 16; i < ATP_XSENSORS; i++)
- if (dev->xy_cur[i]) {
- printk("appletouch: 17\" model detected.\n");
+ /* 17" Powerbooks have extra X sensors */
+ for (i = (IS_GEYSER_2(dev)?15:16); i < ATP_XSENSORS; i++) {
+ if (!dev->xy_cur[i]) continue;
+
+ printk("appletouch: 17\" model detected.\n");
+ if(IS_GEYSER_2(dev))
+ input_set_abs_params(dev->input, ABS_X, 0,
+ (20 - 1) *
+ ATP_XFACT - 1,
+ ATP_FUZZ, 0);
+ else
input_set_abs_params(dev->input, ABS_X, 0,
(ATP_XSENSORS - 1) *
ATP_XFACT - 1,
ATP_FUZZ, 0);
- break;
- }
+ break;
+ }
goto exit;
}
@@ -282,7 +385,8 @@
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
- input_report_key(dev->input, BTN_LEFT, !!dev->data[80]);
+ input_report_key(dev->input, BTN_LEFT,
+ !!dev->data[dev->datalen - 1]);
input_sync(dev->input);
@@ -323,7 +427,6 @@
int int_in_endpointAddr = 0;
int i, retval = -ENOMEM;
-
/* set up the endpoint information */
/* use only the first interrupt-in endpoint */
iface_desc = iface->cur_altsetting;
@@ -353,6 +456,8 @@
dev->udev = udev;
dev->input = input_dev;
+ dev->overflowwarn = 0;
+ dev->datalen = (IS_GEYSER_2(dev)?64:81);
dev->urb = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->urb) {
@@ -360,7 +465,7 @@
goto err_free_devs;
}
- dev->data = usb_buffer_alloc(dev->udev, ATP_DATASIZE, GFP_KERNEL,
+ dev->data = usb_buffer_alloc(dev->udev, dev->datalen, GFP_KERNEL,
&dev->urb->transfer_dma);
if (!dev->data) {
retval = -ENOMEM;
@@ -369,7 +474,7 @@
usb_fill_int_urb(dev->urb, udev,
usb_rcvintpipe(udev, int_in_endpointAddr),
- dev->data, ATP_DATASIZE, atp_complete, dev, 1);
+ dev->data, dev->datalen, atp_complete, dev, 1);
usb_make_path(udev, dev->phys, sizeof(dev->phys));
strlcat(dev->phys, "/input0", sizeof(dev->phys));
@@ -385,14 +490,25 @@
set_bit(EV_ABS, input_dev->evbit);
- /*
- * 12" and 15" Powerbooks only have 16 x sensors,
- * 17" models are detected later.
- */
- input_set_abs_params(input_dev, ABS_X, 0,
- (16 - 1) * ATP_XFACT - 1, ATP_FUZZ, 0);
- input_set_abs_params(input_dev, ABS_Y, 0,
- (ATP_YSENSORS - 1) * ATP_YFACT - 1, ATP_FUZZ, 0);
+ if (IS_GEYSER_2(dev)) {
+ /*
+ * Oct 2005 15" PowerBooks have 15 X sensors, 17" are detected
+ * later.
+ */
+ input_set_abs_params(input_dev, ABS_X, 0,
+ ((15 - 1) * ATP_XFACT) - 1, ATP_FUZZ, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0,
+ ((9 - 1) * ATP_YFACT) - 1, ATP_FUZZ, 0);
+ } else {
+ /*
+ * 12" and 15" Powerbooks only have 16 x sensors,
+ * 17" models are detected later.
+ */
+ input_set_abs_params(input_dev, ABS_X, 0,
+ (16 - 1) * ATP_XFACT - 1, ATP_FUZZ, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0,
+ (ATP_YSENSORS - 1) * ATP_YFACT - 1, ATP_FUZZ, 0);
+ }
input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
set_bit(EV_KEY, input_dev->evbit);
@@ -427,7 +543,7 @@
usb_kill_urb(dev->urb);
input_unregister_device(dev->input);
usb_free_urb(dev->urb);
- usb_buffer_free(dev->udev, ATP_DATASIZE,
+ usb_buffer_free(dev->udev, dev->datalen,
dev->data, dev->urb->transfer_dma);
kfree(dev);
}
@@ -463,11 +579,30 @@
static int __init atp_init(void)
{
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+ if (relayfs) {
+ rcb = kmalloc(sizeof(struct rchan_callbacks), GFP_KERNEL);
+ rcb->subbuf_start = NULL;
+ rcb->buf_mapped = NULL;
+ rcb->buf_unmapped = NULL;
+ rch = relay_open("atpdata", NULL, 256, 256, NULL);
+ if (!rch) return -ENOMEM;
+ printk("appletouch: Relayfs enabled.\n");
+ } else {
+ printk("appletouch: Relayfs disabled.\n");
+ }
+#endif
return usb_register(&atp_driver);
}
static void __exit atp_exit(void)
{
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+ if (relayfs) {
+ relay_close(rch);
+ kfree(rcb);
+ }
+#endif
usb_deregister(&atp_driver);
}
I managed to get this working on my 15" PowerBook, but the USB id for my
Keyboard/Trackpad is 0x0214 as opposed to the 0x0215 you have in the
patch. Are you going to add 0x0214 (and any others?) to this patch
before sending it off?
Also, I found that the patch didn't apply cleanly on my kernel
2.6.15-rc5 kernel. I think many of the line numbers were out, so I ended
up patching the file manually.
cheers,
-Andy
On Sun, 2005-12-04 at 23:42 +0100, Michael Hanselmann wrote:
> On Fri, Dec 02, 2005 at 03:28:31PM +0100, Stelian Pop wrote:
> > Is this version really working well on the new Powerbooks ? From what
> > I've seen in this thread there are still issues and it's still a work
> > in progress, so it may be too early to integrate the changes in the
> > kernel.
>
> It works fine for the 15" PowerBooks (Oct 2005) and has been tested by
> at least three people. 17" should work too, but I can't test that until
> later this week.
>
> > +#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
> > +#include <linux/relayfs_fs.h>
> > +#endif
>
> > While the relayfs code is ok for debugging, I'm wondering if it should
> > be left in the final version at all.
>
> It doesn't hurt as it's not much code, it makes updating the driver for
> newer devices easier and it's only enabled if requested. Because of
> that, I would leave it in. However, I can split it into a separate patch
> if wanted.
>
> > + int overflowwarn; /* overflow warning printed? */
>
> > I would use a static variable in the case -OVERFLOW: block here.
>
> As there could be, due to whatever reason, multiple devices using the
> same driver, one could overflow while another doesn't. Therefore, it
> should be device specific.
>
> > + dev->xy_cur[i++] = dev->data[19];
> > + dev->xy_cur[i++] = dev->data[20];
> > + dev->xy_cur[i++] = dev->data[22];
> > + dev->xy_cur[i++] = dev->data[23];
>
> > There is obviously a pattern here:
>
> > for (i = 0; i < 15; i++)
> > dev->xy_cur[i] = dev->data[ 19 + (i * 3) / 2 ]
>
> It's not that easy. This code wouldn't work as it should.
>
> I wrote a working loop into the patch below, but as the code's called
> about 1'000 times a second I unrolled the loop myself. A friend of mine
> calculated that the code would take at least twice the time to run when
> written using a loop.
>
> > I'm wondering if the same formula doesn't apply for more X and Y
> > sensors (like 16 X and 16 Y sensors on the old Powerbooks, 26 for the
> > 17" models)
>
> It does and is addressed in the new patch below.
>
> > What is the point in doing this since the dbg_dump is called a few lines
> > later ?
>
> Those were some leftovers from my debugging and would have been deleted
> before submitting. I just didn't want to take everything out if someone
> else wants to do some extensive debugging or so.
>
> Here's an updated patch including support for the 17" PowerBooks (Oct
> 2005). The informations about the 17" one are from Alex Harper.
>
> ---
> --- linux-2.6.15-rc5/drivers/usb/input/appletouch.c.orig 2005-12-04 20:25:21.000000000 +0100
> +++ linux-2.6.15-rc5/drivers/usb/input/appletouch.c 2005-12-04 23:37:29.000000000 +0100
> @@ -6,9 +6,19 @@
> * Copyright (C) 2005 Stelian Pop ([email protected])
> * Copyright (C) 2005 Frank Arnold ([email protected])
> * Copyright (C) 2005 Peter Osterlund ([email protected])
> + * Copyright (C) 2005 Parag Warudkar ([email protected])
> + * Copyright (C) 2005 Michael Hanselmann ([email protected])
> *
> * Thanks to Alex Harper <[email protected]> for his inputs.
> *
> + * Nov 2005 - Parag Warudkar
> + * o Added ability to export data via relayfs
> + *
> + * Nov 2005 - Michael Hanselmann
> + * o Compile relayfs support only if enabled in the kernel
> + * o Enable relayfs only if requested by the user
> + * o Added support for new October 2005 PowerBooks (Geyser 2)
> + *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> * the Free Software Foundation; either version 2 of the License, or
> @@ -35,8 +45,13 @@
> #include <linux/input.h>
> #include <linux/usb_input.h>
>
> +#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
> +#include <linux/relayfs_fs.h>
> +#endif
> +
> /* Apple has powerbooks which have the keyboard with different Product IDs */
> #define APPLE_VENDOR_ID 0x05AC
> +#define GEYSER_2_PRODUCT_ID 0x0215
>
> #define ATP_DEVICE(prod) \
> .match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
> @@ -51,14 +66,17 @@
> static struct usb_device_id atp_table [] = {
> { ATP_DEVICE(0x020E) },
> { ATP_DEVICE(0x020F) },
> + { ATP_DEVICE(GEYSER_2_PRODUCT_ID) }, /* PowerBooks Oct 2005 */
> { ATP_DEVICE(0x030A) },
> { ATP_DEVICE(0x030B) },
> { } /* Terminating entry */
> };
> MODULE_DEVICE_TABLE (usb, atp_table);
>
> -/* size of a USB urb transfer */
> -#define ATP_DATASIZE 81
> +#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
> +struct rchan* rch = NULL;
> +struct rchan_callbacks* rcb = NULL;
> +#endif
>
> /*
> * number of sensors. Note that only 16 instead of 26 X (horizontal)
> @@ -73,6 +91,7 @@
>
> /* maximum pressure this driver will report */
> #define ATP_PRESSURE 300
> +
> /*
> * multiplication factor for the X and Y coordinates.
> * We try to keep the touchpad aspect ratio while still doing only simple
> @@ -108,6 +127,8 @@
> 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 */
> };
>
> #define dbg_dump(msg, tab) \
> @@ -124,7 +145,11 @@
> if (debug) printk(format, ##a); \
> } while (0)
>
> -MODULE_AUTHOR("Johannes Berg, Stelian Pop, Frank Arnold");
> +/* Checks if the device a Geyser 2 */
> +#define IS_GEYSER_2(dev) \
> + (le16_to_cpu(dev->udev->descriptor.idProduct) == GEYSER_2_PRODUCT_ID)
> +
> +MODULE_AUTHOR("Johannes Berg, Stelian Pop, Frank Arnold, Parag Warudkar, Michael Hanselmann");
> MODULE_DESCRIPTION("Apple PowerBooks USB touchpad driver");
> MODULE_LICENSE("GPL");
>
> @@ -132,6 +157,10 @@
> module_param(debug, int, 0644);
> MODULE_PARM_DESC(debug, "Activate debugging output");
>
> +static int relayfs = 0;
> +module_param(relayfs, int, 0644);
> +MODULE_PARM_DESC(relayfs, "Activate relayfs support");
> +
> static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
> int *z, int *fingers)
> {
> @@ -175,6 +204,13 @@
> case 0:
> /* success */
> break;
> + case -EOVERFLOW:
> + if(!dev->overflowwarn) {
> + printk("appletouch: OVERFLOW with data "
> + "length %d, actual length is %d\n",
> + dev->datalen, dev->urb->actual_length);
> + dev->overflowwarn = 1;
> + }
> case -ECONNRESET:
> case -ENOENT:
> case -ESHUTDOWN:
> @@ -189,23 +225,83 @@
> }
>
> /* drop incomplete datasets */
> - if (dev->urb->actual_length != ATP_DATASIZE) {
> + if (dev->urb->actual_length != dev->datalen) {
> dprintk("appletouch: incomplete data package.\n");
> goto exit;
> }
>
> +#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
> + if (relayfs && dev->data) {
> + relay_write(rch, dev->data, dev->urb->actual_length);
> + }
> +#endif
> +
> /* reorder the sensors values */
> - for (i = 0; i < 8; i++) {
> + if (IS_GEYSER_2(dev)) {
> + memset(dev->xy_cur, 0, sizeof(dev->xy_cur));
> +
> + /*
> + * The values are laid out like this:
> + * Y1, Y2, -, Y3, Y4, -, ...
> + * '-' is an unused value.
> + *
> + * The logic in a loop:
> + * for (i = 0, j = 19; i < 20; i += 2, j += 3) {
> + * dev->xy_cur[i] = dev->data[j];
> + * dev->xy_cur[i + 1] = dev->data[j + 1];
> + * }
> + *
> + * This code is called about 1'000 times per second for each
> + * interrupt from the touchpad. Therefore it should be as fast
> + * as possible. Writing the following code in a loop would take
> + * about twice the time to run if not more.
> + */
> +
> /* X values */
> - dev->xy_cur[i ] = dev->data[5 * i + 2];
> - dev->xy_cur[i + 8] = dev->data[5 * i + 4];
> - dev->xy_cur[i + 16] = dev->data[5 * i + 42];
> - if (i < 2)
> - dev->xy_cur[i + 24] = dev->data[5 * i + 44];
> + dev->xy_cur[0] = dev->data[19];
> + dev->xy_cur[1] = dev->data[20];
> + dev->xy_cur[2] = dev->data[22];
> + dev->xy_cur[3] = dev->data[23];
> + dev->xy_cur[4] = dev->data[25];
> + dev->xy_cur[5] = dev->data[26];
> + dev->xy_cur[6] = dev->data[28];
> + dev->xy_cur[7] = dev->data[29];
> + dev->xy_cur[8] = dev->data[31];
> + dev->xy_cur[9] = dev->data[32];
> + dev->xy_cur[10] = dev->data[34];
> + dev->xy_cur[11] = dev->data[35];
> + dev->xy_cur[12] = dev->data[37];
> + dev->xy_cur[13] = dev->data[38];
> + dev->xy_cur[14] = dev->data[40];
> + dev->xy_cur[15] = dev->data[41];
> + dev->xy_cur[16] = dev->data[43];
> + dev->xy_cur[17] = dev->data[44];
> + dev->xy_cur[18] = dev->data[46];
> + dev->xy_cur[19] = dev->data[47];
>
> /* Y values */
> - dev->xy_cur[i + 26] = dev->data[5 * i + 1];
> - dev->xy_cur[i + 34] = dev->data[5 * i + 3];
> + dev->xy_cur[ATP_XSENSORS + 0] = dev->data[1];
> + dev->xy_cur[ATP_XSENSORS + 1] = dev->data[2];
> + dev->xy_cur[ATP_XSENSORS + 2] = dev->data[4];
> + dev->xy_cur[ATP_XSENSORS + 3] = dev->data[5];
> + dev->xy_cur[ATP_XSENSORS + 4] = dev->data[7];
> + dev->xy_cur[ATP_XSENSORS + 5] = dev->data[8];
> + dev->xy_cur[ATP_XSENSORS + 6] = dev->data[10];
> + dev->xy_cur[ATP_XSENSORS + 7] = dev->data[11];
> + dev->xy_cur[ATP_XSENSORS + 8] = dev->data[13];
> + } else {
> + for (i = 0; i < 8; i++) {
> + /* X values */
> + dev->xy_cur[i ] = dev->data[5 * i + 2];
> + dev->xy_cur[i + 8] = dev->data[5 * i + 4];
> + dev->xy_cur[i + 16] = dev->data[5 * i + 42];
> + if (i < 2)
> + dev->xy_cur[i + 24] = dev->data[5 * i + 44];
> +
> + /* Y values */
> + dev->xy_cur[i + 26] = dev->data[5 * i + 1];
> + dev->xy_cur[i + 34] = dev->data[5 * i + 3];
> + }
> }
>
> dbg_dump("sample", dev->xy_cur);
> @@ -216,16 +312,23 @@
> dev->x_old = dev->y_old = -1;
> memcpy(dev->xy_old, dev->xy_cur, sizeof(dev->xy_old));
>
> - /* 17" Powerbooks have 10 extra X sensors */
> - for (i = 16; i < ATP_XSENSORS; i++)
> - if (dev->xy_cur[i]) {
> - printk("appletouch: 17\" model detected.\n");
> + /* 17" Powerbooks have extra X sensors */
> + for (i = (IS_GEYSER_2(dev)?15:16); i < ATP_XSENSORS; i++) {
> + if (!dev->xy_cur[i]) continue;
> +
> + printk("appletouch: 17\" model detected.\n");
> + if(IS_GEYSER_2(dev))
> + input_set_abs_params(dev->input, ABS_X, 0,
> + (20 - 1) *
> + ATP_XFACT - 1,
> + ATP_FUZZ, 0);
> + else
> input_set_abs_params(dev->input, ABS_X, 0,
> (ATP_XSENSORS - 1) *
> ATP_XFACT - 1,
> ATP_FUZZ, 0);
> - break;
> - }
> + break;
> + }
>
> goto exit;
> }
> @@ -282,7 +385,8 @@
> memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
> }
>
> - input_report_key(dev->input, BTN_LEFT, !!dev->data[80]);
> + input_report_key(dev->input, BTN_LEFT,
> + !!dev->data[dev->datalen - 1]);
>
> input_sync(dev->input);
>
> @@ -323,7 +427,6 @@
> int int_in_endpointAddr = 0;
> int i, retval = -ENOMEM;
>
> -
> /* set up the endpoint information */
> /* use only the first interrupt-in endpoint */
> iface_desc = iface->cur_altsetting;
> @@ -353,6 +456,8 @@
>
> dev->udev = udev;
> dev->input = input_dev;
> + dev->overflowwarn = 0;
> + dev->datalen = (IS_GEYSER_2(dev)?64:81);
>
> dev->urb = usb_alloc_urb(0, GFP_KERNEL);
> if (!dev->urb) {
> @@ -360,7 +465,7 @@
> goto err_free_devs;
> }
>
> - dev->data = usb_buffer_alloc(dev->udev, ATP_DATASIZE, GFP_KERNEL,
> + dev->data = usb_buffer_alloc(dev->udev, dev->datalen, GFP_KERNEL,
> &dev->urb->transfer_dma);
> if (!dev->data) {
> retval = -ENOMEM;
> @@ -369,7 +474,7 @@
>
> usb_fill_int_urb(dev->urb, udev,
> usb_rcvintpipe(udev, int_in_endpointAddr),
> - dev->data, ATP_DATASIZE, atp_complete, dev, 1);
> + dev->data, dev->datalen, atp_complete, dev, 1);
>
> usb_make_path(udev, dev->phys, sizeof(dev->phys));
> strlcat(dev->phys, "/input0", sizeof(dev->phys));
> @@ -385,14 +490,25 @@
>
> set_bit(EV_ABS, input_dev->evbit);
>
> - /*
> - * 12" and 15" Powerbooks only have 16 x sensors,
> - * 17" models are detected later.
> - */
> - input_set_abs_params(input_dev, ABS_X, 0,
> - (16 - 1) * ATP_XFACT - 1, ATP_FUZZ, 0);
> - input_set_abs_params(input_dev, ABS_Y, 0,
> - (ATP_YSENSORS - 1) * ATP_YFACT - 1, ATP_FUZZ, 0);
> + if (IS_GEYSER_2(dev)) {
> + /*
> + * Oct 2005 15" PowerBooks have 15 X sensors, 17" are detected
> + * later.
> + */
> + input_set_abs_params(input_dev, ABS_X, 0,
> + ((15 - 1) * ATP_XFACT) - 1, ATP_FUZZ, 0);
> + input_set_abs_params(input_dev, ABS_Y, 0,
> + ((9 - 1) * ATP_YFACT) - 1, ATP_FUZZ, 0);
> + } else {
> + /*
> + * 12" and 15" Powerbooks only have 16 x sensors,
> + * 17" models are detected later.
> + */
> + input_set_abs_params(input_dev, ABS_X, 0,
> + (16 - 1) * ATP_XFACT - 1, ATP_FUZZ, 0);
> + input_set_abs_params(input_dev, ABS_Y, 0,
> + (ATP_YSENSORS - 1) * ATP_YFACT - 1, ATP_FUZZ, 0);
> + }
> input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
>
> set_bit(EV_KEY, input_dev->evbit);
> @@ -427,7 +543,7 @@
> usb_kill_urb(dev->urb);
> input_unregister_device(dev->input);
> usb_free_urb(dev->urb);
> - usb_buffer_free(dev->udev, ATP_DATASIZE,
> + usb_buffer_free(dev->udev, dev->datalen,
> dev->data, dev->urb->transfer_dma);
> kfree(dev);
> }
> @@ -463,11 +579,30 @@
>
> static int __init atp_init(void)
> {
> +#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
> + if (relayfs) {
> + rcb = kmalloc(sizeof(struct rchan_callbacks), GFP_KERNEL);
> + rcb->subbuf_start = NULL;
> + rcb->buf_mapped = NULL;
> + rcb->buf_unmapped = NULL;
> + rch = relay_open("atpdata", NULL, 256, 256, NULL);
> + if (!rch) return -ENOMEM;
> + printk("appletouch: Relayfs enabled.\n");
> + } else {
> + printk("appletouch: Relayfs disabled.\n");
> + }
> +#endif
> return usb_register(&atp_driver);
> }
>
> static void __exit atp_exit(void)
> {
> +#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
> + if (relayfs) {
> + relay_close(rch);
> + kfree(rcb);
> + }
> +#endif
> usb_deregister(&atp_driver);
> }
>
>
>
Hello Andy
On Tue, Dec 06, 2005 at 02:38:36PM +1100, Andy Botting wrote:
> I managed to get this working on my 15" PowerBook, but the USB id for my
Thanks for testing.
> Keyboard/Trackpad is 0x0214 as opposed to the 0x0215 you have in the
> patch. Are you going to add 0x0214 (and any others?) to this patch
> before sending it off?
Yes, I can add that one. I don't know about any other IDs that will work
with this patch, so I can't add them.
> Also, I found that the patch didn't apply cleanly on my kernel
> 2.6.15-rc5 kernel. I think many of the line numbers were out, so I ended
> up patching the file manually.
A friend of mine tested it today with a fresh unpacked 2.6.15-rc15 and
it applied cleanly.
Greets,
Michael
> I managed to get this working on my 15" PowerBook, but the USB id for my
> Keyboard/Trackpad is 0x0214 as opposed to the 0x0215 you have in the
> patch. Are you going to add 0x0214 (and any others?) to this patch
> before sending it off?
This patch adds support for 0x0214 and 0x0216 in addition to 0x0215. I
found those IDs in the Info.plist of Mac OS X (see comment in the patch).
It applies cleanly to 2.6.15-rc5 (vanilla).
---
--- linux-2.6.15-rc5/drivers/usb/input/appletouch.c.orig 2005-12-04 20:25:21.000000000 +0100
+++ linux-2.6.15-rc5/drivers/usb/input/appletouch.c 2005-12-09 21:02:55.000000000 +0100
@@ -6,9 +6,19 @@
* Copyright (C) 2005 Stelian Pop ([email protected])
* Copyright (C) 2005 Frank Arnold ([email protected])
* Copyright (C) 2005 Peter Osterlund ([email protected])
+ * Copyright (C) 2005 Parag Warudkar ([email protected])
+ * Copyright (C) 2005 Michael Hanselmann ([email protected])
*
* Thanks to Alex Harper <[email protected]> for his inputs.
*
+ * Nov 2005 - Parag Warudkar
+ * o Added ability to export data via relayfs
+ *
+ * Nov 2005 - Michael Hanselmann
+ * o Compile relayfs support only if enabled in the kernel
+ * o Enable relayfs only if requested by the user
+ * o Added support for new October 2005 PowerBooks (Geyser 2)
+ *
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
@@ -35,9 +45,18 @@
#include <linux/input.h>
#include <linux/usb_input.h>
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+#include <linux/relayfs_fs.h>
+#endif
+
/* Apple has powerbooks which have the keyboard with different Product IDs */
#define APPLE_VENDOR_ID 0x05AC
+/* These names come from Info.plist in AppleUSBTrackpad.kext */
+#define GEYSER_ANSI_PRODUCT_ID 0x0214
+#define GEYSER_ISO_PRODUCT_ID 0x0215
+#define GEYSER_JIS_PRODUCT_ID 0x0216
+
#define ATP_DEVICE(prod) \
.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
USB_DEVICE_ID_MATCH_INT_CLASS | \
@@ -53,12 +72,21 @@
{ ATP_DEVICE(0x020F) },
{ ATP_DEVICE(0x030A) },
{ ATP_DEVICE(0x030B) },
- { } /* Terminating entry */
+
+ /* PowerBooks Oct 2005 */
+ { ATP_DEVICE(GEYSER_ANSI_PRODUCT_ID) },
+ { ATP_DEVICE(GEYSER_ISO_PRODUCT_ID) },
+ { ATP_DEVICE(GEYSER_JIS_PRODUCT_ID) },
+
+ /* Terminating entry */
+ { }
};
MODULE_DEVICE_TABLE (usb, atp_table);
-/* size of a USB urb transfer */
-#define ATP_DATASIZE 81
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+struct rchan* rch = NULL;
+struct rchan_callbacks* rcb = NULL;
+#endif
/*
* number of sensors. Note that only 16 instead of 26 X (horizontal)
@@ -73,6 +101,7 @@
/* maximum pressure this driver will report */
#define ATP_PRESSURE 300
+
/*
* multiplication factor for the X and Y coordinates.
* We try to keep the touchpad aspect ratio while still doing only simple
@@ -108,6 +137,8 @@
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 */
};
#define dbg_dump(msg, tab) \
@@ -124,7 +155,7 @@
if (debug) printk(format, ##a); \
} while (0)
-MODULE_AUTHOR("Johannes Berg, Stelian Pop, Frank Arnold");
+MODULE_AUTHOR("Johannes Berg, Stelian Pop, Frank Arnold, Parag Warudkar, Michael Hanselmann");
MODULE_DESCRIPTION("Apple PowerBooks USB touchpad driver");
MODULE_LICENSE("GPL");
@@ -132,6 +163,20 @@
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Activate debugging output");
+static int relayfs = 0;
+module_param(relayfs, int, 0644);
+MODULE_PARM_DESC(relayfs, "Activate relayfs support");
+
+/* Checks if the device a Geyser 2 (ANSI, ISO, JIS) */
+static inline int atp_is_geyser_2(struct atp *dev)
+{
+ int16_t productId = le16_to_cpu(dev->udev->descriptor.idProduct);
+
+ return (productId == GEYSER_ANSI_PRODUCT_ID) ||
+ (productId == GEYSER_ISO_PRODUCT_ID) ||
+ (productId == GEYSER_JIS_PRODUCT_ID);
+}
+
static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
int *z, int *fingers)
{
@@ -175,6 +220,13 @@
case 0:
/* success */
break;
+ case -EOVERFLOW:
+ if(!dev->overflowwarn) {
+ printk("appletouch: OVERFLOW with data "
+ "length %d, actual length is %d\n",
+ dev->datalen, dev->urb->actual_length);
+ dev->overflowwarn = 1;
+ }
case -ECONNRESET:
case -ENOENT:
case -ESHUTDOWN:
@@ -189,23 +241,83 @@
}
/* drop incomplete datasets */
- if (dev->urb->actual_length != ATP_DATASIZE) {
+ if (dev->urb->actual_length != dev->datalen) {
dprintk("appletouch: incomplete data package.\n");
goto exit;
}
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+ if (relayfs && dev->data) {
+ relay_write(rch, dev->data, dev->urb->actual_length);
+ }
+#endif
+
/* reorder the sensors values */
- for (i = 0; i < 8; i++) {
+ if (atp_is_geyser_2(dev)) {
+ memset(dev->xy_cur, 0, sizeof(dev->xy_cur));
+
+ /*
+ * The values are laid out like this:
+ * Y1, Y2, -, Y3, Y4, -, ...
+ * '-' is an unused value.
+ *
+ * The logic in a loop:
+ * for (i = 0, j = 19; i < 20; i += 2, j += 3) {
+ * dev->xy_cur[i] = dev->data[j];
+ * dev->xy_cur[i + 1] = dev->data[j + 1];
+ * }
+ *
+ * This code is called about 100 times per second for each
+ * interrupt from the touchpad. Therefore it should be as fast
+ * as possible. Writing the following code in a loop would take
+ * about twice the time to run if not more.
+ */
+
/* X values */
- dev->xy_cur[i ] = dev->data[5 * i + 2];
- dev->xy_cur[i + 8] = dev->data[5 * i + 4];
- dev->xy_cur[i + 16] = dev->data[5 * i + 42];
- if (i < 2)
- dev->xy_cur[i + 24] = dev->data[5 * i + 44];
+ dev->xy_cur[0] = dev->data[19];
+ dev->xy_cur[1] = dev->data[20];
+ dev->xy_cur[2] = dev->data[22];
+ dev->xy_cur[3] = dev->data[23];
+ dev->xy_cur[4] = dev->data[25];
+ dev->xy_cur[5] = dev->data[26];
+ dev->xy_cur[6] = dev->data[28];
+ dev->xy_cur[7] = dev->data[29];
+ dev->xy_cur[8] = dev->data[31];
+ dev->xy_cur[9] = dev->data[32];
+ dev->xy_cur[10] = dev->data[34];
+ dev->xy_cur[11] = dev->data[35];
+ dev->xy_cur[12] = dev->data[37];
+ dev->xy_cur[13] = dev->data[38];
+ dev->xy_cur[14] = dev->data[40];
+ dev->xy_cur[15] = dev->data[41];
+ dev->xy_cur[16] = dev->data[43];
+ dev->xy_cur[17] = dev->data[44];
+ dev->xy_cur[18] = dev->data[46];
+ dev->xy_cur[19] = dev->data[47];
/* Y values */
- dev->xy_cur[i + 26] = dev->data[5 * i + 1];
- dev->xy_cur[i + 34] = dev->data[5 * i + 3];
+ dev->xy_cur[ATP_XSENSORS + 0] = dev->data[1];
+ dev->xy_cur[ATP_XSENSORS + 1] = dev->data[2];
+ dev->xy_cur[ATP_XSENSORS + 2] = dev->data[4];
+ dev->xy_cur[ATP_XSENSORS + 3] = dev->data[5];
+ dev->xy_cur[ATP_XSENSORS + 4] = dev->data[7];
+ dev->xy_cur[ATP_XSENSORS + 5] = dev->data[8];
+ dev->xy_cur[ATP_XSENSORS + 6] = dev->data[10];
+ dev->xy_cur[ATP_XSENSORS + 7] = dev->data[11];
+ dev->xy_cur[ATP_XSENSORS + 8] = dev->data[13];
+ } else {
+ for (i = 0; i < 8; i++) {
+ /* X values */
+ dev->xy_cur[i ] = dev->data[5 * i + 2];
+ dev->xy_cur[i + 8] = dev->data[5 * i + 4];
+ dev->xy_cur[i + 16] = dev->data[5 * i + 42];
+ if (i < 2)
+ dev->xy_cur[i + 24] = dev->data[5 * i + 44];
+
+ /* Y values */
+ dev->xy_cur[i + 26] = dev->data[5 * i + 1];
+ dev->xy_cur[i + 34] = dev->data[5 * i + 3];
+ }
}
dbg_dump("sample", dev->xy_cur);
@@ -216,16 +328,23 @@
dev->x_old = dev->y_old = -1;
memcpy(dev->xy_old, dev->xy_cur, sizeof(dev->xy_old));
- /* 17" Powerbooks have 10 extra X sensors */
- for (i = 16; i < ATP_XSENSORS; i++)
- if (dev->xy_cur[i]) {
- printk("appletouch: 17\" model detected.\n");
+ /* 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;
+
+ printk("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,
+ ATP_FUZZ, 0);
+ else
input_set_abs_params(dev->input, ABS_X, 0,
(ATP_XSENSORS - 1) *
ATP_XFACT - 1,
ATP_FUZZ, 0);
- break;
- }
+ break;
+ }
goto exit;
}
@@ -282,7 +401,8 @@
memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
}
- input_report_key(dev->input, BTN_LEFT, !!dev->data[80]);
+ input_report_key(dev->input, BTN_LEFT,
+ !!dev->data[dev->datalen - 1]);
input_sync(dev->input);
@@ -323,7 +443,6 @@
int int_in_endpointAddr = 0;
int i, retval = -ENOMEM;
-
/* set up the endpoint information */
/* use only the first interrupt-in endpoint */
iface_desc = iface->cur_altsetting;
@@ -353,6 +472,8 @@
dev->udev = udev;
dev->input = input_dev;
+ dev->overflowwarn = 0;
+ dev->datalen = (atp_is_geyser_2(dev)?64:81);
dev->urb = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->urb) {
@@ -360,7 +481,7 @@
goto err_free_devs;
}
- dev->data = usb_buffer_alloc(dev->udev, ATP_DATASIZE, GFP_KERNEL,
+ dev->data = usb_buffer_alloc(dev->udev, dev->datalen, GFP_KERNEL,
&dev->urb->transfer_dma);
if (!dev->data) {
retval = -ENOMEM;
@@ -369,7 +490,7 @@
usb_fill_int_urb(dev->urb, udev,
usb_rcvintpipe(udev, int_in_endpointAddr),
- dev->data, ATP_DATASIZE, atp_complete, dev, 1);
+ dev->data, dev->datalen, atp_complete, dev, 1);
usb_make_path(udev, dev->phys, sizeof(dev->phys));
strlcat(dev->phys, "/input0", sizeof(dev->phys));
@@ -385,14 +506,25 @@
set_bit(EV_ABS, input_dev->evbit);
- /*
- * 12" and 15" Powerbooks only have 16 x sensors,
- * 17" models are detected later.
- */
- input_set_abs_params(input_dev, ABS_X, 0,
- (16 - 1) * ATP_XFACT - 1, ATP_FUZZ, 0);
- input_set_abs_params(input_dev, ABS_Y, 0,
- (ATP_YSENSORS - 1) * ATP_YFACT - 1, ATP_FUZZ, 0);
+ if (atp_is_geyser_2(dev)) {
+ /*
+ * Oct 2005 15" PowerBooks have 15 X sensors, 17" are detected
+ * later.
+ */
+ input_set_abs_params(input_dev, ABS_X, 0,
+ ((15 - 1) * ATP_XFACT) - 1, ATP_FUZZ, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0,
+ ((9 - 1) * ATP_YFACT) - 1, ATP_FUZZ, 0);
+ } else {
+ /*
+ * 12" and 15" Powerbooks only have 16 x sensors,
+ * 17" models are detected later.
+ */
+ input_set_abs_params(input_dev, ABS_X, 0,
+ (16 - 1) * ATP_XFACT - 1, ATP_FUZZ, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0,
+ (ATP_YSENSORS - 1) * ATP_YFACT - 1, ATP_FUZZ, 0);
+ }
input_set_abs_params(input_dev, ABS_PRESSURE, 0, ATP_PRESSURE, 0, 0);
set_bit(EV_KEY, input_dev->evbit);
@@ -427,7 +559,7 @@
usb_kill_urb(dev->urb);
input_unregister_device(dev->input);
usb_free_urb(dev->urb);
- usb_buffer_free(dev->udev, ATP_DATASIZE,
+ usb_buffer_free(dev->udev, dev->datalen,
dev->data, dev->urb->transfer_dma);
kfree(dev);
}
@@ -463,11 +595,30 @@
static int __init atp_init(void)
{
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+ if (relayfs) {
+ rcb = kmalloc(sizeof(struct rchan_callbacks), GFP_KERNEL);
+ rcb->subbuf_start = NULL;
+ rcb->buf_mapped = NULL;
+ rcb->buf_unmapped = NULL;
+ rch = relay_open("atpdata", NULL, 256, 256, NULL);
+ if (!rch) return -ENOMEM;
+ printk("appletouch: Relayfs enabled.\n");
+ } else {
+ printk("appletouch: Relayfs disabled.\n");
+ }
+#endif
return usb_register(&atp_driver);
}
static void __exit atp_exit(void)
{
+#if defined(CONFIG_RELAYFS_FS) || defined(CONFIG_RELAYFS_FS_MODULE)
+ if (relayfs) {
+ relay_close(rch);
+ kfree(rcb);
+ }
+#endif
usb_deregister(&atp_driver);
}
Ok, I finally received my new laptop (PowerBook5,8 15"). I tried the
latest patch you posted, and while the kernel driver seem to work ok
(though you can feel the lack of a proper acceleration curve), the
synaptics driver in X doesn't work in any useable way. I updated the one
that comes with breezy to whatever was the latest on the author web site
(.44 I think) and while it detected the tracpkad, the result was soooooo
slooooow that it was totally unseable. I've tried the config tool that
comes with KDE for it but couldn't "boost" it to anything useful. Is
that expected or is there still issues to be resolved in the driver ?
I'm tempted to add some minimum support for a proper acceleration curve
in the kernel driver in fact...
Ben.
Hello
On Sat, Dec 24, 2005 at 10:59:44AM +1100, Benjamin Herrenschmidt wrote:
[...]
> comes with KDE for it but couldn't "boost" it to anything useful. Is
> that expected or is there still issues to be resolved in the driver ?
> I'm tempted to add some minimum support for a proper acceleration curve
> in the kernel driver in fact...
I'd the same problem. A quick look into Xorg.0.log tells me, that the
event device was missing. I created it manually and the acceleration and
other feautres are working now. Maybe you've still the same problem?
René
--
Written on a Gentoo Linux-system by René
http://www.forkbomb.ch
«Das Leben ist hart. Forkbomb ist härter.», (c) 2004 by Frank.
Hi!
> Ok, I finally received my new laptop (PowerBook5,8 15"). I tried the
> latest patch you posted, and while the kernel driver seem to work ok
> (though you can feel the lack of a proper acceleration curve), the
> synaptics driver in X doesn't work in any useable way. I updated the one
> that comes with breezy to whatever was the latest on the author web site
> (.44 I think) and while it detected the tracpkad, the result was soooooo
> slooooow that it was totally unseable. I've tried the config tool that
> comes with KDE for it but couldn't "boost" it to anything useful. Is
> that expected or is there still issues to be resolved in the driver ?
> I'm tempted to add some minimum support for a proper acceleration curve
> in the kernel driver in fact...
I do not think you should add it inside *kernel*. Proper acceleration
support really belongs to X...
Pavel
--
Thanks, Sharp!
On Sat, 2005-12-24 at 21:17 +0100, Pavel Machek wrote:
> Hi!
>
> > Ok, I finally received my new laptop (PowerBook5,8 15"). I tried the
> > latest patch you posted, and while the kernel driver seem to work ok
> > (though you can feel the lack of a proper acceleration curve), the
> > synaptics driver in X doesn't work in any useable way. I updated the one
> > that comes with breezy to whatever was the latest on the author web site
> > (.44 I think) and while it detected the tracpkad, the result was soooooo
> > slooooow that it was totally unseable. I've tried the config tool that
> > comes with KDE for it but couldn't "boost" it to anything useful. Is
> > that expected or is there still issues to be resolved in the driver ?
> > I'm tempted to add some minimum support for a proper acceleration curve
> > in the kernel driver in fact...
>
> I do not think you should add it inside *kernel*. Proper acceleration
> support really belongs to X...
X, fbdev apps, gpm ... acceleration curves are typically done in HW,
thus it makes sense in this case to have it in the kernel driver
(besides, it should be fairly simple anyway) when it's in normal mode.
When it's in raw mode for use by the synaptics X driver, if course, it's
expected that those things are to be done by that driver.
Ben.
On Ne 25-12-05 09:27:51, Benjamin Herrenschmidt wrote:
> On Sat, 2005-12-24 at 21:17 +0100, Pavel Machek wrote:
> > Hi!
> >
> > > Ok, I finally received my new laptop (PowerBook5,8 15"). I tried the
> > > latest patch you posted, and while the kernel driver seem to work ok
> > > (though you can feel the lack of a proper acceleration curve), the
> > > synaptics driver in X doesn't work in any useable way. I updated the one
> > > that comes with breezy to whatever was the latest on the author web site
> > > (.44 I think) and while it detected the tracpkad, the result was soooooo
> > > slooooow that it was totally unseable. I've tried the config tool that
> > > comes with KDE for it but couldn't "boost" it to anything useful. Is
> > > that expected or is there still issues to be resolved in the driver ?
> > > I'm tempted to add some minimum support for a proper acceleration curve
> > > in the kernel driver in fact...
> >
> > I do not think you should add it inside *kernel*. Proper acceleration
> > support really belongs to X...
>
> X, fbdev apps, gpm ... acceleration curves are typically done in HW,
> thus it makes sense in this case to have it in the kernel driver
> (besides, it should be fairly simple anyway) when it's in normal
> mode.
It should be doable once in gpm, all other apps can use gpm's repeater
mode...
> When it's in raw mode for use by the synaptics X driver, if course, it's
> expected that those things are to be done by that driver.
...but you are right, doing it in /dev/input/mice emulation layer
makes some sense. OTOH I thought we were moving away from
/dev/input/mice... Its Dmitry's call I guess.
Pavel
--
Thanks, Sharp!
> It should be doable once in gpm, all other apps can use gpm's repeater
> mode...
>
> > When it's in raw mode for use by the synaptics X driver, if course, it's
> > expected that those things are to be done by that driver.
>
> ...but you are right, doing it in /dev/input/mice emulation layer
> makes some sense. OTOH I thought we were moving away from
> /dev/input/mice... Its Dmitry's call I guess.
Heh, yes, it is. No hurry anyway, I finally got synaptics working
properly in X ...
Why would we move away from the mouse mux ? It's proven to be very
useful to me at least :)
Ben
On Saturday 24 December 2005 19:33, Benjamin Herrenschmidt wrote:
>
> > It should be doable once in gpm, all other apps can use gpm's repeater
> > mode...
> >
> > > When it's in raw mode for use by the synaptics X driver, if course, it's
> > > expected that those things are to be done by that driver.
> >
> > ...but you are right, doing it in /dev/input/mice emulation layer
> > makes some sense. OTOH I thought we were moving away from
> > /dev/input/mice... Its Dmitry's call I guess.
>
> Heh, yes, it is. No hurry anyway, I finally got synaptics working
> properly in X ...
>
> Why would we move away from the mouse mux ? It's proven to be very
> useful to me at least :)
>
It is very limited - number of buttons, wheels, etc. Once X supports
hotplugging mouse multipexor outgrows its usefullness (GPM can simply
be restarted every time we detect new input device).
--
Dmitry