[PATCH 0/2] Provide a driver for the Apple Magic Mouse

[PATCH 0/2] Provide a driver for the Apple Magic Mouse

[Michael Poole]

I think this patch is ready for real review.  The Magic Mouse requires
that a driver send an unlock Report(Feature) command, similar to the
Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
Input Report that isn't published in the input Report descriptor that
contains touch data (and usually overrides the normal motion and click
Report).

Because the mouse has only one switch and no scroll wheel, the driver
(under control of parameters) emulates a middle button and scroll wheel.
User space could also ignore and/or re-synthesize those events based on
the reported events.

The first patch exports hid_register_report() so the driver can turn on
the multitouch report.  The second patch adds the device ID and the
driver.  Some user-space tools to talk to the mouse directly (that is,
when it is not associated with the host's HIDP stack) are at
http://github.com/entrope/linux-magicmouse .

[PATCH 1/2] Provide a driver for the Apple Magic Mouse

[Michael Poole]

>From 6fd44770774cd672df6f10e4767f4c17bf90bc30 Mon Sep 17 00:00:00 2001
From: Michael Poole <mdpoole-IZmAEv5cUt1AfugRpC6u6w@public.gmane.org>
Date: Sun, 24 Jan 2010 22:32:29 -0500
Subject: [RFC PATCH 3/4] Export hid_register_report().

The Apple Magic Mouse (and probably other devices) publish reports that
are not called out in their HID report descriptors -- they only send them
when enabled through other writes to the device.  This allows a driver to
handle these unlisted reports.

Signed-off-by: Michael Poole <mdpoole-IZmAEv5cUt1AfugRpC6u6w@public.gmane.org>
---
 drivers/hid/hid-core.c |    3 ++-
 include/linux/hid.h    |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index eabe5f8..e39055b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -51,7 +51,7 @@ EXPORT_SYMBOL_GPL(hid_debug);
  * Register a new report for a device.
  */
 
-static struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id)
+struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id)
 {
 	struct hid_report_enum *report_enum = device->report_enum + type;
 	struct hid_report *report;
@@ -75,6 +75,7 @@ static struct hid_report *hid_register_report(struct hid_device *device, unsigne
 
 	return report;
 }
+EXPORT_SYMBOL_GPL(hid_register_report);
 
 /*
  * Register a new field for this report.
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 3661a62..456838c 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -690,6 +690,7 @@ int hid_input_report(struct hid_device *, int type, u8 *, int, int);
 int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field);
 void hid_output_report(struct hid_report *report, __u8 *data);
 struct hid_device *hid_allocate_device(void);
+struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id);
 int hid_parse_report(struct hid_device *hid, __u8 *start, unsigned size);
 int hid_check_keys_pressed(struct hid_device *hid);
 int hid_connect(struct hid_device *hid, unsigned int connect_mask);
-- 
1.6.5.6

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse

[Justin P. Mattock]

On 02/09/10 05:10, Michael Poole wrote:
> I think this patch is ready for real review.  The Magic Mouse requires
> that a driver send an unlock Report(Feature) command, similar to the
> Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> Input Report that isn't published in the input Report descriptor that
> contains touch data (and usually overrides the normal motion and click
> Report).
>
> Because the mouse has only one switch and no scroll wheel, the driver
> (under control of parameters) emulates a middle button and scroll wheel.
> User space could also ignore and/or re-synthesize those events based on
> the reported events.
>
> The first patch exports hid_register_report() so the driver can turn on
> the multitouch report.  The second patch adds the device ID and the
> driver.  Some user-space tools to talk to the mouse directly (that is,
> when it is not associated with the host's HIDP stack) are at
> http://github.com/entrope/linux-magicmouse .
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


First, is I made a mistake with the procedure(early post) i.g.
sudo /usr/bin/test-device trusted D4:9A:20:88:C7:48
forgot to add yes at the end.
(so rebooting works and so forth).

As to this module I pulled the latest mainstream
added:
http://lkml.org/lkml/2010/1/29/146
plus the two other patches you had submitted with this thread and
everything compiled good.

Glad to see the mouse is working with these patches(thank you vary much 
gentlemen).

I can scroll up and down(need to get used to this).
copy/past works
right/left click works.
(need to figure out forward/reverse with firefox probably).

suspend works. I'm getting a resumbit urb, but that's been there
prior to this module. Although there was a moment after wakeup
I turned off the mouse, then reconnecting didnt, but then on another 
go(reboot)it worked(maybe hit some odd thing).

Other than not seeing any menu entry under hid
with make menuconfig, I would have to say vary nice
job with this, all functions I need to function are
there(scroll,right/left click, copy/past).

nice job, and thanks for this..
(without this I would of had a non
functional magic mouse).

Justin P. Mattock

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse

[Jiri Kosina]

On Tue, 9 Feb 2010, Michael Poole wrote:

> I think this patch is ready for real review.  The Magic Mouse requires
> that a driver send an unlock Report(Feature) command, similar to the
> Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> Input Report that isn't published in the input Report descriptor that
> contains touch data (and usually overrides the normal motion and click
> Report).
> 
> Because the mouse has only one switch and no scroll wheel, the driver
> (under control of parameters) emulates a middle button and scroll wheel.
> User space could also ignore and/or re-synthesize those events based on
> the reported events.
> 
> The first patch exports hid_register_report() so the driver can turn on
> the multitouch report.  The second patch adds the device ID and the
> driver.  Some user-space tools to talk to the mouse directly (that is,
> when it is not associated with the host's HIDP stack) are at
> http://github.com/entrope/linux-magicmouse .

I have applied the driver into apple_magic_mouse branch and merged this 
branch into for-next, so it should appear in the upcoming linux-next.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Ed Tomlinson]

On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> On Tue, 9 Feb 2010, Michael Poole wrote:
> 
> > I think this patch is ready for real review.  The Magic Mouse requires
> > that a driver send an unlock Report(Feature) command, similar to the
> > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > Input Report that isn't published in the input Report descriptor that
> > contains touch data (and usually overrides the normal motion and click
> > Report).
> > 
> > Because the mouse has only one switch and no scroll wheel, the driver
> > (under control of parameters) emulates a middle button and scroll wheel.
> > User space could also ignore and/or re-synthesize those events based on
> > the reported events.
> > 
> > The first patch exports hid_register_report() so the driver can turn on
> > the multitouch report.  The second patch adds the device ID and the
> > driver.  Some user-space tools to talk to the mouse directly (that is,
> > when it is not associated with the host's HIDP stack) are at
> > http://github.com/entrope/linux-magicmouse .
> 
> I have applied the driver into apple_magic_mouse branch and merged this 
> branch into for-next, so it should appear in the upcoming linux-next.

This driver (or the hid changes) can triggers an opps.  What I did was start X.  Turn on the magic mouse.  It connected on input7&8.  
Then I powered it off and on.  This time it conneced on input9&10.  Then I exited X and got the opps.  Note my X does not hotplug 
the magic mouse.  I've also included a trace of the udev events that generated the log below (if there was a remove after X stopped 
it would not be included).  To my eyes it looks like we leak an input device (there is not a remove event for input8).

[ 5955.908380] input: Apple Wireless Mouse as /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7
[ 5955.921165] magicmouse 0005:05AC:030D.0004: input,hidraw3: BLUETOOTH HID v0.84 Mouse [Apple Wireless Mouse] on 00:0A:3A:55:07:5A
[ 5955.934120] input: Apple Wireless Mouse as /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input8
[ 6180.899332] input: Apple Wireless Mouse as /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input9
[ 6180.911914] magicmouse 0005:05AC:030D.0005: input,hidraw3: BLUETOOTH HID v0.84 Mouse [Apple Wireless Mouse] on 00:0A:3A:55:07:5A
[ 6180.923988] input: Apple Wireless Mouse as /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input10
[ 6391.991295] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
[ 6391.995818] IP: [<ffffffffa05a604f>] magicmouse_input_open+0x1f/0x30 [hid_magicmouse]
[ 6392.009801] PGD 16c3b6067 PUD 16b139067 PMD 0 
[ 6392.009801] Oops: 0000 [#1] PREEMPT SMP 
[ 6392.009801] last sysfs file: /sys/devices/pci0000:00/0000:00:18.3/temp1_input
[ 6392.009801] CPU 1 
[ 6392.009801] Pid: 2763, comm: gpm Not tainted 2.6.33-rc8-crc #106 M3A78-T/System Product Name
[ 6392.064520] RIP: 0010:[<ffffffffa05a604f>]  [<ffffffffa05a604f>] magicmouse_input_open+0x1f/0x30 [hid_magicmouse]
[ 6392.064520] RSP: 0018:ffff88016c2ddba8  EFLAGS: 00010282
[ 6392.064520] RAX: ffff88016dd90000 RBX: ffff88016bc9b000 RCX: 0000000000000001
[ 6392.107009] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88016dd90000
[ 6392.107009] RBP: ffff88016c2ddba8 R08: 2222222222222222 R09: 2222222222222222
[ 6392.107009] R10: 0000000000000000 R11: 0000000000000001 R12: ffff880133b5a810
[ 6392.107009] R13: ffff88016bc9b820 R14: ffff88016dd60150 R15: ffff88016dd600a8
[ 6392.107009] FS:  00007f5ee305b700(0000) GS:ffff880028280000(0000) knlGS:0000000000000000
[ 6392.107009] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 6392.107009] CR2: 0000000000000010 CR3: 000000016c3b7000 CR4: 00000000000006e0
[ 6392.107009] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 6392.107009] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 6392.107009] Process gpm (pid: 2763, threadinfo ffff88016c2dc000, task ffff88016dde0000)
[ 6392.107009] Stack:
[ 6392.107009]  ffff88016c2ddbe8 ffffffff814010e9 ffff88016dd60150 ffff88016dd600a8
[ 6392.107009] <0> ffff88016c2ddbe8 ffff880133b5a800 ffff880133b5a8d8 0000000000000000
[ 6392.107009] <0> ffff88016c2ddc18 ffffffff81404236 ffff88016c2ddc18 ffff880133b5a800
[ 6392.107009] Call Trace:
[ 6392.107009]  [<ffffffff814010e9>] input_open_device+0x89/0xb0
[ 6392.107009]  [<ffffffff81404236>] mousedev_open_device+0x76/0x100
[ 6392.107009]  [<ffffffff814042a9>] mousedev_open_device+0xe9/0x100
[ 6392.107009]  [<ffffffff8140523f>] mousedev_open+0x19f/0x260
[ 6392.107009]  [<ffffffff814bc243>] ? _lock_kernel+0x143/0x1e0
[ 6392.107009]  [<ffffffff81402ba7>] input_open_file+0x227/0x410
[ 6392.107009]  [<ffffffff814bf981>] ? sub_preempt_count+0x51/0x60
[ 6392.107009]  [<ffffffff8112384d>] chrdev_open+0x17d/0x320
[ 6392.107009]  [<ffffffff814bbb7c>] ? _raw_spin_unlock+0x5c/0x70
[ 6392.107009]  [<ffffffff8111d358>] __dentry_open+0x1a8/0x400
[ 6392.107009]  [<ffffffff811236d0>] ? chrdev_open+0x0/0x320
[ 6392.107009]  [<ffffffff8111d6b4>] nameidata_to_filp+0x54/0x70
[ 6392.107009]  [<ffffffff8112e7f1>] do_filp_open+0x841/0xc00
[ 6392.107009]  [<ffffffff814bf981>] ? sub_preempt_count+0x51/0x60
[ 6392.107009]  [<ffffffff814bbb7c>] ? _raw_spin_unlock+0x5c/0x70
[ 6392.107009]  [<ffffffff81002b0c>] ? sysret_check+0x27/0x62
[ 6392.107009]  [<ffffffff8111e954>] do_sys_open+0xa4/0x180
[ 6392.107009]  [<ffffffff8111ea70>] sys_open+0x20/0x30
[ 6392.107009]  [<ffffffff81002adb>] system_call_fastpath+0x16/0x1b
[ 6392.107009] Code: 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 0f 1f 44 00 00 48 81 c7 a0 08 00 00 e8 3b f0 de e0 48 8b 90 38 1b 00 00 48 89 c7 <ff> 52 10 c9 c3 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 
[ 6392.107009] RIP  [<ffffffffa05a604f>] magicmouse_input_open+0x1f/0x30 [hid_magicmouse]
[ 6392.107009]  RSP <ffff88016c2ddba8>
[ 6392.107009] CR2: 0000000000000010
[ 6392.107321] ---[ end trace c83e80c68826df09 ]---

grover ~ # udevadm monitor
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing
KERNEL - the kernel uevent

KERNEL[1266085238.085802] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41 (bluetooth)
UDEV  [1266085238.113475] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41 (bluetooth)
KERNEL[1266085238.412016] add      /module/hidp (module)
UDEV  [1266085238.412247] add      /module/hidp (module)
KERNEL[1266085238.418193] add      /bus/hid/drivers/generic-bluetooth (drivers)
UDEV  [1266085238.418365] add      /bus/hid/drivers/generic-bluetooth (drivers)
KERNEL[1266085238.418507] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/0005:05AC:030D.0004 (hid)
KERNEL[1266085238.460711] add      /module/hid_magicmouse (module)
UDEV  [1266085238.460906] add      /module/hid_magicmouse (module)
KERNEL[1266085238.460977] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7 (input)
UDEV  [1266085238.462772] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7 (input)
KERNEL[1266085238.473232] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7/mouse1 (input)
KERNEL[1266085238.473323] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7/event7 (input)
KERNEL[1266085238.473662] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/0005:05AC:030D.0004/hidraw/hidraw3 (hidraw)
UDEV  [1266085238.479691] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7/event7 (input)
KERNEL[1266085238.486090] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input8 (input)
UDEV  [1266085238.488291] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7/mouse1 (input)
UDEV  [1266085238.493195] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input8 (input)
KERNEL[1266085238.498954] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input8/mouse2 (input)
KERNEL[1266085238.499066] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input8/event8 (input)
KERNEL[1266085238.499154] add      /bus/hid/drivers/magicmouse (drivers)
UDEV  [1266085238.499624] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/0005:05AC:030D.0004 (hid)
UDEV  [1266085238.501108] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/0005:05AC:030D.0004/hidraw/hidraw3 (hidraw)
UDEV  [1266085238.501311] add      /bus/hid/drivers/magicmouse (drivers)
UDEV  [1266085238.509221] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input8/event8 (input)
UDEV  [1266085238.511459] add      /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input8/mouse2 (input)
KERNEL[1266085311.467279] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7/mouse1 (input)
UDEV  [1266085311.468189] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7/mouse1 (input)
KERNEL[1266085311.471961] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7/event7 (input)
UDEV  [1266085311.472632] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7/event7 (input)
KERNEL[1266085311.475921] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7 (input)
KERNEL[1266085311.475985] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/0005:05AC:030D.0004/hidraw/hidraw3 (hidraw)
KERNEL[1266085311.476153] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/0005:05AC:030D.0004 (hid)
KERNEL[1266085311.476214] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41 (bluetooth)
UDEV  [1266085311.476397] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/input7 (input)
UDEV  [1266085311.476734] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/0005:05AC:030D.0004/hidraw/hidraw3 (hidraw)
UDEV  [1266085311.477000] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41/0005:05AC:030D.0004 (hid)
UDEV  [1266085311.503056] remove   /devices/pci0000:00/0000:00:13.2/usb7/7-4/7-4.4/7-4.4:1.0/bluetooth/hci0/hci0:41 (bluetooth)

Ideas?
Ed Tomlinson

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Dmitry Torokhov]

On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
> On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> > On Tue, 9 Feb 2010, Michael Poole wrote:
> > 
> > > I think this patch is ready for real review.  The Magic Mouse requires
> > > that a driver send an unlock Report(Feature) command, similar to the
> > > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > > Input Report that isn't published in the input Report descriptor that
> > > contains touch data (and usually overrides the normal motion and click
> > > Report).
> > > 
> > > Because the mouse has only one switch and no scroll wheel, the driver
> > > (under control of parameters) emulates a middle button and scroll wheel.
> > > User space could also ignore and/or re-synthesize those events based on
> > > the reported events.
> > > 
> > > The first patch exports hid_register_report() so the driver can turn on
> > > the multitouch report.  The second patch adds the device ID and the
> > > driver.  Some user-space tools to talk to the mouse directly (that is,
> > > when it is not associated with the host's HIDP stack) are at
> > > http://github.com/entrope/linux-magicmouse .
> > 
> > I have applied the driver into apple_magic_mouse branch and merged this 
> > branch into for-next, so it should appear in the upcoming linux-next.
> 

> This driver (or the hid changes) can triggers an opps.  What I did was
> start X.  Turn on the magic mouse.  It connected on input7&8.  Then I
> powered it off and on.  This time it conneced on input9&10.  Then I
> exited X and got the opps.  Note my X does not hotplug the magic
> mouse.  I've also included a trace of the udev events that generated
> the log below (if there was a remove after X stopped it would not be
> included).  To my eyes it looks like we leak an input device (there is
> not a remove event for input8).
>

Indeed, we seem to be missing call to input_unregister_device() in
magicmouse_remove().

-- 
Dmitry

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Ed Tomlinson]

On Sunday 14 February 2010 03:03:44 Dmitry Torokhov wrote:
> On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
> > On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> > > On Tue, 9 Feb 2010, Michael Poole wrote:
> > > 
> > > > I think this patch is ready for real review.  The Magic Mouse requires
> > > > that a driver send an unlock Report(Feature) command, similar to the
> > > > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > > > Input Report that isn't published in the input Report descriptor that
> > > > contains touch data (and usually overrides the normal motion and click
> > > > Report).
> > > > 
> > > > Because the mouse has only one switch and no scroll wheel, the driver
> > > > (under control of parameters) emulates a middle button and scroll wheel.
> > > > User space could also ignore and/or re-synthesize those events based on
> > > > the reported events.
> > > > 
> > > > The first patch exports hid_register_report() so the driver can turn on
> > > > the multitouch report.  The second patch adds the device ID and the
> > > > driver.  Some user-space tools to talk to the mouse directly (that is,
> > > > when it is not associated with the host's HIDP stack) are at
> > > > http://github.com/entrope/linux-magicmouse .
> > > 
> > > I have applied the driver into apple_magic_mouse branch and merged this 
> > > branch into for-next, so it should appear in the upcoming linux-next.
> > 
> 
> > This driver (or the hid changes) can triggers an opps.  What I did was
> > start X.  Turn on the magic mouse.  It connected on input7&8.  Then I
> > powered it off and on.  This time it conneced on input9&10.  Then I
> > exited X and got the opps.  Note my X does not hotplug the magic
> > mouse.  I've also included a trace of the udev events that generated
> > the log below (if there was a remove after X stopped it would not be
> > included).  To my eyes it looks like we leak an input device (there is
> > not a remove event for input8).
> >
> 
> Indeed, we seem to be missing call to input_unregister_device() in
> magicmouse_remove().

How does this look?  With this udevadm shows input8 being removed and
there is no more opps.

----
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index f94b3e4..71a8669 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -429,8 +429,11 @@ static int magicmouse_probe(struct hid_device *hdev,
 
 static void magicmouse_remove(struct hid_device *hdev)
 {
+	struct magicmouse_sc *msc;
+	msc = hid_get_drvdata(hdev);
+	input_unregister_device(msc->input);
 	hid_hw_stop(hdev);
-	kfree(hid_get_drvdata(hdev));
+	kfree(msc);
 }
 
 static const struct hid_device_id magic_mice[] = {
----

Thanks
Ed

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Dmitry Torokhov]

On Sun, Feb 14, 2010 at 09:22:41AM -0500, Ed Tomlinson wrote:
> On Sunday 14 February 2010 03:03:44 Dmitry Torokhov wrote:
> > On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
> > > On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> > > > On Tue, 9 Feb 2010, Michael Poole wrote:
> > > > 
> > > > > I think this patch is ready for real review.  The Magic Mouse requires
> > > > > that a driver send an unlock Report(Feature) command, similar to the
> > > > > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > > > > Input Report that isn't published in the input Report descriptor that
> > > > > contains touch data (and usually overrides the normal motion and click
> > > > > Report).
> > > > > 
> > > > > Because the mouse has only one switch and no scroll wheel, the driver
> > > > > (under control of parameters) emulates a middle button and scroll wheel.
> > > > > User space could also ignore and/or re-synthesize those events based on
> > > > > the reported events.
> > > > > 
> > > > > The first patch exports hid_register_report() so the driver can turn on
> > > > > the multitouch report.  The second patch adds the device ID and the
> > > > > driver.  Some user-space tools to talk to the mouse directly (that is,
> > > > > when it is not associated with the host's HIDP stack) are at
> > > > > http://github.com/entrope/linux-magicmouse .
> > > > 
> > > > I have applied the driver into apple_magic_mouse branch and merged this 
> > > > branch into for-next, so it should appear in the upcoming linux-next.
> > > 
> > 
> > > This driver (or the hid changes) can triggers an opps.  What I did was
> > > start X.  Turn on the magic mouse.  It connected on input7&8.  Then I
> > > powered it off and on.  This time it conneced on input9&10.  Then I
> > > exited X and got the opps.  Note my X does not hotplug the magic
> > > mouse.  I've also included a trace of the udev events that generated
> > > the log below (if there was a remove after X stopped it would not be
> > > included).  To my eyes it looks like we leak an input device (there is
> > > not a remove event for input8).
> > >
> > 
> > Indeed, we seem to be missing call to input_unregister_device() in
> > magicmouse_remove().
> 
> How does this look?  With this udevadm shows input8 being removed and
> there is no more opps.

Almost... you need to do hid_hw_stop() first and only then unregister
input device, Otherwise if you unload the module while moving the mouse
it is likely to still oops.

Thanks.

-- 
Dmitry

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Ed Tomlinson]

On Monday 15 February 2010 02:11:46 Dmitry Torokhov wrote:
> On Sun, Feb 14, 2010 at 09:22:41AM -0500, Ed Tomlinson wrote:
> > On Sunday 14 February 2010 03:03:44 Dmitry Torokhov wrote:
> > > On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
> > > > On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> > > > > On Tue, 9 Feb 2010, Michael Poole wrote:
> > > > > 
> > > > > > I think this patch is ready for real review.  The Magic Mouse requires
> > > > > > that a driver send an unlock Report(Feature) command, similar to the
> > > > > > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > > > > > Input Report that isn't published in the input Report descriptor that
> > > > > > contains touch data (and usually overrides the normal motion and click
> > > > > > Report).
> > > > > > 
> > > > > > Because the mouse has only one switch and no scroll wheel, the driver
> > > > > > (under control of parameters) emulates a middle button and scroll wheel.
> > > > > > User space could also ignore and/or re-synthesize those events based on
> > > > > > the reported events.
> > > > > > 
> > > > > > The first patch exports hid_register_report() so the driver can turn on
> > > > > > the multitouch report.  The second patch adds the device ID and the
> > > > > > driver.  Some user-space tools to talk to the mouse directly (that is,
> > > > > > when it is not associated with the host's HIDP stack) are at
> > > > > > http://github.com/entrope/linux-magicmouse .
> > > > > 
> > > > > I have applied the driver into apple_magic_mouse branch and merged this 
> > > > > branch into for-next, so it should appear in the upcoming linux-next.
> > > > 
> > > 
> > > > This driver (or the hid changes) can triggers an opps.  What I did was
> > > > start X.  Turn on the magic mouse.  It connected on input7&8.  Then I
> > > > powered it off and on.  This time it conneced on input9&10.  Then I
> > > > exited X and got the opps.  Note my X does not hotplug the magic
> > > > mouse.  I've also included a trace of the udev events that generated
> > > > the log below (if there was a remove after X stopped it would not be
> > > > included).  To my eyes it looks like we leak an input device (there is
> > > > not a remove event for input8).
> > > >
> > > 
> > > Indeed, we seem to be missing call to input_unregister_device() in
> > > magicmouse_remove().
> > 
> > How does this look?  With this udevadm shows input8 being removed and
> > there is no more opps.
> 
> Almost... you need to do hid_hw_stop() first and only then unregister
> input device, Otherwise if you unload the module while moving the mouse
> it is likely to still oops.

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Ed Tomlinson]

On Monday 15 February 2010 02:11:46 Dmitry Torokhov wrote:
> On Sun, Feb 14, 2010 at 09:22:41AM -0500, Ed Tomlinson wrote:
> > On Sunday 14 February 2010 03:03:44 Dmitry Torokhov wrote:
> > > On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
> > > > On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
> > > > > On Tue, 9 Feb 2010, Michael Poole wrote:
> > > > > 
> > > > > > I think this patch is ready for real review.  The Magic Mouse requires
> > > > > > that a driver send an unlock Report(Feature) command, similar to the
> > > > > > Wacom wireless tablet and Sixaxis controller quirks.  This turns on an
> > > > > > Input Report that isn't published in the input Report descriptor that
> > > > > > contains touch data (and usually overrides the normal motion and click
> > > > > > Report).
> > > > > > 
> > > > > > Because the mouse has only one switch and no scroll wheel, the driver
> > > > > > (under control of parameters) emulates a middle button and scroll wheel.
> > > > > > User space could also ignore and/or re-synthesize those events based on
> > > > > > the reported events.
> > > > > > 
> > > > > > The first patch exports hid_register_report() so the driver can turn on
> > > > > > the multitouch report.  The second patch adds the device ID and the
> > > > > > driver.  Some user-space tools to talk to the mouse directly (that is,
> > > > > > when it is not associated with the host's HIDP stack) are at
> > > > > > http://github.com/entrope/linux-magicmouse .
> > > > > 
> > > > > I have applied the driver into apple_magic_mouse branch and merged this 
> > > > > branch into for-next, so it should appear in the upcoming linux-next.
> > > > 
> > > 
> > > > This driver (or the hid changes) can triggers an opps.  What I did was
> > > > start X.  Turn on the magic mouse.  It connected on input7&8.  Then I
> > > > powered it off and on.  This time it conneced on input9&10.  Then I
> > > > exited X and got the opps.  Note my X does not hotplug the magic
> > > > mouse.  I've also included a trace of the udev events that generated
> > > > the log below (if there was a remove after X stopped it would not be
> > > > included).  To my eyes it looks like we leak an input device (there is
> > > > not a remove event for input8).
> > > >
> > > 
> > > Indeed, we seem to be missing call to input_unregister_device() in
> > > magicmouse_remove().
> > 
> > How does this look?  With this udevadm shows input8 being removed and
> > there is no more opps.
> 
> Almost... you need to do hid_hw_stop() first and only then unregister
> input device, Otherwise if you unload the module while moving the mouse
> it is likely to still oops.

How about this?  It applies on top of yesterdays patch.

Ed

---
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 7d252d2..46fdeee 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -430,8 +430,8 @@ static void magicmouse_remove(struct hid_device *hdev)
 {
 	struct magicmouse_sc *msc;
 	msc = hid_get_drvdata(hdev);
-	input_unregister_device(msc->input);
 	hid_hw_stop(hdev);
+	input_unregister_device(msc->input);
 	kfree(msc);
 }
 

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Jiri Kosina]

On Mon, 15 Feb 2010, Ed Tomlinson wrote:

> > Almost... you need to do hid_hw_stop() first and only then unregister
> > input device, Otherwise if you unload the module while moving the mouse
> > it is likely to still oops.
> 
> How about this?  It applies on top of yesterdays patch.
> 
> Ed
> 
> ---
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 7d252d2..46fdeee 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -430,8 +430,8 @@ static void magicmouse_remove(struct hid_device *hdev)
>  {
>  	struct magicmouse_sc *msc;
>  	msc = hid_get_drvdata(hdev);
> -	input_unregister_device(msc->input);
>  	hid_hw_stop(hdev);
> +	input_unregister_device(msc->input);
>  	kfree(msc);
>  }

This looks OK.

Could you please send it altogether with short changelog entry and 
Signed-off-by line so that I could queue that up?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Ed Tomlinson]

On Monday 15 February 2010 02:11:46 Dmitry Torokhov wrote:
> Almost... you need to do hid_hw_stop() first and only then unregister
> input device, Otherwise if you unload the module while moving the mouse
> it is likely to still oops.

The exit routing for the module also has a hid_hw_stop.  Is it going to cause
problems when it gets called twice?

TIA
Ed

Re: [PATCH 0/2] Provide a driver for the Apple Magic Mouse - opps

[Jiri Kosina]

On Tue, 16 Feb 2010, Ed Tomlinson wrote:

> > Almost... you need to do hid_hw_stop() first and only then unregister
> > input device, Otherwise if you unload the module while moving the mouse
> > it is likely to still oops.
> 
> The exit routing for the module also has a hid_hw_stop.  Is it going to cause
> problems when it gets called twice?

The routine wasn't probably meant/designed with re-entrancy in mind, but 
looking quickly at all the subsequent callpaths, I don't see why it should 
cause any problem, as skb_queue_purge(), usb_kill_urb() and usb_free_urb() 
should be safe.

Maybe we should even add test_bit() for HID_STARTED and HID_DISCONNECTED 
at the beginning of the low-level drivers' ->stop callbacks, so that we 
don't do all the magic when all the work has been already done.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

[PATCH 1/1] Enable xy scrolling for Apple Magic Mouse

[Ed Tomlinson]

Hi,

Here is a patch that enables xy scrolling with the magic mouse.  I have also
changed the accelleration logic to work better with xy scrolling.

Comments
Ed Tomlinson

---
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index b20484a..3075d78 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -59,7 +59,7 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
  * @delta_time: 18-bit difference between the two most recent touch
  *     reports from the mouse.
  * @ntouches: Number of touches in most recent touch report.
- * @scroll_accel: Number of consecutive scroll motions.
+ * @scroll_accely: Number of consecutive scroll motions.
  * @scroll_jiffies: Time of last scroll motion.
  * @touches: Most recent data for a touch, indexed by tracking ID.
  * @tracking_ids: Mapping of current touch input data to @touches.
@@ -71,11 +71,13 @@ struct magicmouse_sc {
 	int last_timestamp;
 	int delta_time;
 	int ntouches;
-	int scroll_accel;
+	int scroll_accely;
+	int scroll_accelx;
 	unsigned long scroll_jiffies;
 
 	struct {
 		short x;
+		short scroll_x;
 		short y;
 		short scroll_y;
 		u8 size;
@@ -139,8 +141,10 @@ static void magicmouse_emit_buttons(struct magicmouse_sc *msc, int state)
 	input_report_key(msc->input, BTN_LEFT, state & 1);
 	input_report_key(msc->input, BTN_RIGHT, state & 2);
 
-	if (state != last_state)
-		msc->scroll_accel = 0;
+	if (state != last_state) {
+		msc->scroll_accely = 0;
+		msc->scroll_accelx = 0;
+	}
 }
 
 static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tdata)
@@ -159,34 +163,46 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 	msc->touches[id].size = misc & 63;
 
 	/* If requested, emulate a scroll wheel by detecting small
-	 * vertical touch motions along the middle of the mouse.
+	 * touch motions on the mouse.
 	 */
 	if (emulate_scroll_wheel &&
-	    middle_button_start < x && x < middle_button_stop) {
+	    msc->ntouches == 1) {
 		static const int accel_profile[] = {
-			256, 228, 192, 160, 128, 96, 64, 32,
+			192, 160, 128, 96, 64, 48, 32, 24,
 		};
 		unsigned long now = jiffies;
-		int step = msc->touches[id].scroll_y - y;
+		int stepx, stepy;
 
 		/* Reset acceleration after half a second. */
-		if (time_after(now, msc->scroll_jiffies + HZ / 2))
-			msc->scroll_accel = 0;
+		if (time_after(now, msc->scroll_jiffies + HZ / 2)) {
+			msc->scroll_accely = 0;
+			msc->scroll_accelx = 0;
+		}
 
-		/* Calculate and apply the scroll motion. */
 		switch (tdata[7] & TOUCH_STATE_MASK) {
 		case TOUCH_STATE_START:
 			msc->touches[id].scroll_y = y;
-			msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
-						ARRAY_SIZE(accel_profile) - 1);
+			msc->touches[id].scroll_x = x;
 			break;
 		case TOUCH_STATE_DRAG:
-			step = step / accel_profile[msc->scroll_accel];
-			if (step != 0) {
+			/* Calculate and apply the scroll motion. */
+			stepy = (msc->touches[id].scroll_y - y)/accel_profile[msc->scroll_accely];
+			stepx = (msc->touches[id].scroll_x - x)/accel_profile[msc->scroll_accelx];
+
+			/* tell input about any motions */
+			if (stepy != 0) {
 				msc->touches[id].scroll_y = y;
-				msc->scroll_jiffies = now;
-				input_report_rel(input, REL_WHEEL, step);
+				input_report_rel(input, REL_WHEEL, stepy);
+				msc->scroll_accely = min_t(int, msc->scroll_accely + 1,
+							        ARRAY_SIZE(accel_profile) - 1);
 			}
+			if (stepx != 0) {
+				msc->touches[id].scroll_x = x;
+				input_report_rel(input, REL_HWHEEL, stepx);
+				msc->scroll_accelx = min_t(int, msc->scroll_accelx + 1,
+							        ARRAY_SIZE(accel_profile) - 1);
+			}
+			msc->scroll_jiffies = now;
 			break;
 		}
 	}
@@ -300,8 +316,10 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h
 	__set_bit(EV_REL, input->evbit);
 	__set_bit(REL_X, input->relbit);
 	__set_bit(REL_Y, input->relbit);
-	if (emulate_scroll_wheel)
+	if (emulate_scroll_wheel) {
 		__set_bit(REL_WHEEL, input->relbit);
+		__set_bit(REL_HWHEEL, input->relbit);
+	}
 
 	if (report_touches) {
 		__set_bit(EV_ABS, input->evbit);

Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse

[Michael Poole]

Ed Tomlinson writes:

> Hi,
>
> Here is a patch that enables xy scrolling with the magic mouse.  I have also
> changed the accelleration logic to work better with xy scrolling.

Hi Ed,

Your other patch to call input_unregister_device() looks good -- thanks!

I've never used a horizontal scroll wheel -- what are the common uses
for it?  Why should the acceleration be separate for the two directions
rather than using the same factor?  Why does the kernel need to emulate
this rather than having user-space implement the emulation?

(If the answers to the last two questions justify putting this feature
in the kernel, my only suggestions on the patch are style-related:
scroll_accel{x,y} should probably be spelled scroll_accel_{x,y}, and
there are missing spaces around the / operator when calculating stepx
and stepy.)

Michael Poole

Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse

[Ed Tomlinson]

On Sunday 14 February 2010 17:51:15 Michael Poole wrote:
> Ed Tomlinson writes:
> 
> > Hi,
> >
> > Here is a patch that enables xy scrolling with the magic mouse.  I have also
> > changed the accelleration logic to work better with xy scrolling.
> 
> Hi Ed,
> 
> Your other patch to call input_unregister_device() looks good -- thanks!

Thanks.  One question about it though.  Do we have to check if msc is null before
the unregister?

> I've never used a horizontal scroll wheel -- what are the common uses
> for it?  Why should the acceleration be separate for the two directions
> rather than using the same factor?  Why does the kernel need to emulate
> this rather than having user-space implement the emulation?

Its usefull for scrolling left and right while browsing.  If you use kde it can
be used to scroll between applications on the taskbar.   Here most applications
with a horizontial scrollbar work as expected.

I first tried with a single acceleration value for both axies.  It leads to confusing
things happening.   For example.  I quickly scroll down, then nudge the
window to the left.  This works as expect with two values.  With one the
nudge is accelerated and moves too far.

My personal goal is to have the basic, apple defined, gestures working
from kernel space.  This way the device works as expected without needing
to fiddle with X or other managers (think wayland and/or chromeOS).

> (If the answers to the last two questions justify putting this feature
> in the kernel, my only suggestions on the patch are style-related:
> scroll_accel{x,y} should probably be spelled scroll_accel_{x,y}, and
> there are missing spaces around the / operator when calculating stepx
> and stepy.)

I'll fix up the style and resubmit.

Thanks
Ed

Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse

[Dmitry Torokhov]

On Sun, Feb 14, 2010 at 06:58:26PM -0500, Ed Tomlinson wrote:
> On Sunday 14 February 2010 17:51:15 Michael Poole wrote:
> > Ed Tomlinson writes:
> > 
> > > Hi,
> > >
> > > Here is a patch that enables xy scrolling with the magic mouse.  I have also
> > > changed the accelleration logic to work better with xy scrolling.
> > 
> > Hi Ed,
> > 
> > Your other patch to call input_unregister_device() looks good -- thanks!
> 
> Thanks.  One question about it though.  Do we have to check if msc is null before
> the unregister?

If it is NULL whan remove() is running there are much bigger problems
with the driver/HID subsystem.

> 
> > I've never used a horizontal scroll wheel -- what are the common uses
> > for it?  Why should the acceleration be separate for the two directions
> > rather than using the same factor?  Why does the kernel need to emulate
> > this rather than having user-space implement the emulation?
> 
> Its usefull for scrolling left and right while browsing.  If you use kde it can
> be used to scroll between applications on the taskbar.   Here most applications
> with a horizontial scrollbar work as expected.
> 
> I first tried with a single acceleration value for both axies.  It leads to confusing
> things happening.   For example.  I quickly scroll down, then nudge the
> window to the left.  This works as expect with two values.  With one the
> nudge is accelerated and moves too far.
> 
> My personal goal is to have the basic, apple defined, gestures working
> from kernel space.  This way the device works as expected without needing
> to fiddle with X or other managers (think wayland and/or chromeOS).

I am not sure if this is the desired approach. The current idea is to
export useable but minimally processed events to userspace and let them
be turned into gestures there (by evdev, synaptics driver and so forth).

Thanks.
 
-- 
Dmitry

Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse

[Ed Tomlinson]

On Monday 15 February 2010 02:18:50 Dmitry Torokhov wrote:
> On Sun, Feb 14, 2010 at 06:58:26PM -0500, Ed Tomlinson wrote:
> > On Sunday 14 February 2010 17:51:15 Michael Poole wrote:
> > > Ed Tomlinson writes:
> > > 
> > > > Hi,
> > > >
> > > > Here is a patch that enables xy scrolling with the magic mouse.  I have also
> > > > changed the accelleration logic to work better with xy scrolling.
> > > 
> > > Hi Ed,
> > > 
> > > Your other patch to call input_unregister_device() looks good -- thanks!
> > 
> > Thanks.  One question about it though.  Do we have to check if msc is null before
> > the unregister?
> 
> If it is NULL whan remove() is running there are much bigger problems
> with the driver/HID subsystem.
> 
> > 
> > > I've never used a horizontal scroll wheel -- what are the common uses
> > > for it?  Why should the acceleration be separate for the two directions
> > > rather than using the same factor?  Why does the kernel need to emulate
> > > this rather than having user-space implement the emulation?
> > 
> > Its usefull for scrolling left and right while browsing.  If you use kde it can
> > be used to scroll between applications on the taskbar.   Here most applications
> > with a horizontial scrollbar work as expected.
> > 
> > I first tried with a single acceleration value for both axies.  It leads to confusing
> > things happening.   For example.  I quickly scroll down, then nudge the
> > window to the left.  This works as expect with two values.  With one the
> > nudge is accelerated and moves too far.
> > 
> > My personal goal is to have the basic, apple defined, gestures working
> > from kernel space.  This way the device works as expected without needing
> > to fiddle with X or other managers (think wayland and/or chromeOS).
> 
> I am not sure if this is the desired approach. The current idea is to
> export useable but minimally processed events to userspace and let them
> be turned into gestures there (by evdev, synaptics driver and so forth).

I would go along with this if the code to add the emulation in kernel was large or complex.  
Its not.  About 30 more lines should add the pgup/pgdn code.  

Thanks
Ed

Re: [PATCH 1/1] Enable xy scrolling for Apple Magic Mouse

[Ed Tomlinson]

On Sunday 14 February 2010 17:51:15 Michael Poole wrote:
> Ed Tomlinson writes:
> 
> > Hi,
> >
> > Here is a patch that enables xy scrolling with the magic mouse.  I have also
> > changed the accelleration logic to work better with xy scrolling.
> 
> Hi Ed,
> 
> Your other patch to call input_unregister_device() looks good -- thanks!
> 
> I've never used a horizontal scroll wheel -- what are the common uses
> for it?  Why should the acceleration be separate for the two directions
> rather than using the same factor?  Why does the kernel need to emulate
> this rather than having user-space implement the emulation?

With style fixups.

Ed

--
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index b20484a..7d252d2 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -59,7 +59,8 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
  * @delta_time: 18-bit difference between the two most recent touch
  *     reports from the mouse.
  * @ntouches: Number of touches in most recent touch report.
- * @scroll_accel: Number of consecutive scroll motions.
+ * @scroll_accel_y: Number of consecutive scroll motions.
+ * @scroll_accel_x: Number of consecutive scroll motions.
  * @scroll_jiffies: Time of last scroll motion.
  * @touches: Most recent data for a touch, indexed by tracking ID.
  * @tracking_ids: Mapping of current touch input data to @touches.
@@ -71,11 +72,13 @@ struct magicmouse_sc {
 	int last_timestamp;
 	int delta_time;
 	int ntouches;
-	int scroll_accel;
+	int scroll_accel_y;
+	int scroll_accel_x;
 	unsigned long scroll_jiffies;
 
 	struct {
 		short x;
+		short scroll_x;
 		short y;
 		short scroll_y;
 		u8 size;
@@ -139,8 +142,10 @@ static void magicmouse_emit_buttons(struct magicmouse_sc *msc, int state)
 	input_report_key(msc->input, BTN_LEFT, state & 1);
 	input_report_key(msc->input, BTN_RIGHT, state & 2);
 
-	if (state != last_state)
-		msc->scroll_accel = 0;
+	if (state != last_state) {
+		msc->scroll_accel_y = 0;
+		msc->scroll_accel_x = 0;
+	}
 }
 
 static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tdata)
@@ -159,34 +164,46 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 	msc->touches[id].size = misc & 63;
 
 	/* If requested, emulate a scroll wheel by detecting small
-	 * vertical touch motions along the middle of the mouse.
+	 * touch motions on the mouse.
 	 */
 	if (emulate_scroll_wheel &&
-	    middle_button_start < x && x < middle_button_stop) {
+	    msc->ntouches == 1) {
 		static const int accel_profile[] = {
-			256, 228, 192, 160, 128, 96, 64, 32,
+			192, 160, 128, 96, 64, 48, 32, 24,
 		};
 		unsigned long now = jiffies;
-		int step = msc->touches[id].scroll_y - y;
+		int stepx, stepy;
 
 		/* Reset acceleration after half a second. */
-		if (time_after(now, msc->scroll_jiffies + HZ / 2))
-			msc->scroll_accel = 0;
+		if (time_after(now, msc->scroll_jiffies + HZ / 2)) {
+			msc->scroll_accel_y = 0;
+			msc->scroll_accel_x = 0;
+		}
 
-		/* Calculate and apply the scroll motion. */
 		switch (tdata[7] & TOUCH_STATE_MASK) {
 		case TOUCH_STATE_START:
 			msc->touches[id].scroll_y = y;
-			msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
-						ARRAY_SIZE(accel_profile) - 1);
+			msc->touches[id].scroll_x = x;
 			break;
 		case TOUCH_STATE_DRAG:
-			step = step / accel_profile[msc->scroll_accel];
-			if (step != 0) {
+			/* Calculate the scroll motion. */
+			stepy = (msc->touches[id].scroll_y - y) / accel_profile[msc->scroll_accel_y];
+			stepx = (msc->touches[id].scroll_x - x) / accel_profile[msc->scroll_accel_x];
+
+			/* Tell input about any motion. */
+			if (stepy != 0) {
 				msc->touches[id].scroll_y = y;
-				msc->scroll_jiffies = now;
-				input_report_rel(input, REL_WHEEL, step);
+				input_report_rel(input, REL_WHEEL, stepy);
+				msc->scroll_accel_y = min_t(int, msc->scroll_accel_y + 1,
+							        ARRAY_SIZE(accel_profile) - 1);
 			}
+			if (stepx != 0) {
+				msc->touches[id].scroll_x = x;
+				input_report_rel(input, REL_HWHEEL, stepx);
+				msc->scroll_accel_x = min_t(int, msc->scroll_accel_x + 1,
+							        ARRAY_SIZE(accel_profile) - 1);
+			}
+			msc->scroll_jiffies = now;
 			break;
 		}
 	}
@@ -300,8 +317,10 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h
 	__set_bit(EV_REL, input->evbit);
 	__set_bit(REL_X, input->relbit);
 	__set_bit(REL_Y, input->relbit);
-	if (emulate_scroll_wheel)
+	if (emulate_scroll_wheel) {
 		__set_bit(REL_WHEEL, input->relbit);
+		__set_bit(REL_HWHEEL, input->relbit);
+	}
 
 	if (report_touches) {
 		__set_bit(EV_ABS, input->evbit);

[PATCH 2/2] Add a device driver for the Apple Magic Mouse.

[Michael Poole]

>From da3e2b0330fccd584ad79495853e638e9652155b Mon Sep 17 00:00:00 2001
From: Michael Poole <mdpoole@troilus.org>
Date: Sat, 6 Feb 2010 12:24:36 -0500
Subject: Add a device driver for the Apple Magic Mouse.

Signed-off-by: Michael Poole <mdpoole@troilus.org>
---
 drivers/hid/Kconfig          |   10 +
 drivers/hid/Makefile         |    1 +
 drivers/hid/hid-core.c       |    1 +
 drivers/hid/hid-ids.h        |    1 +
 drivers/hid/hid-magicmouse.c |  469 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 482 insertions(+), 0 deletions(-)
 create mode 100644 drivers/hid/hid-magicmouse.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 24d90ea..ba14ec8 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -183,6 +183,16 @@ config LOGIRUMBLEPAD2_FF
 	  Say Y here if you want to enable force feedback support for Logitech
 	  Rumblepad 2 devices.
 
+config HID_MAGICMOUSE
+	tristate "Apple" if EMBEDDED
+	depends on BT_HIDP
+	default !EMBEDDED
+	---help---
+	Support for the Apple Magic Mouse.
+
+	Say Y here if you want support for the multi-touch features of the
+	Apple Wireless "Magic" Mouse.
+
 config HID_MICROSOFT
 	tristate "Microsoft" if EMBEDDED
 	depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 0de2dff..45d81e9 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
 obj-$(CONFIG_HID_KENSINGTON)	+= hid-kensington.o
 obj-$(CONFIG_HID_KYE)		+= hid-kye.o
 obj-$(CONFIG_HID_LOGITECH)	+= hid-logitech.o
+obj-$(CONFIG_HID_MAGICMOUSE)    += hid-magicmouse.o
 obj-$(CONFIG_HID_MICROSOFT)	+= hid-microsoft.o
 obj-$(CONFIG_HID_MONTEREY)	+= hid-monterey.o
 obj-$(CONFIG_HID_NTRIG)		+= hid-ntrig.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e39055b..f23ca76 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1254,6 +1254,7 @@ static const struct hid_device_id hid_blacklist[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ATV_IRCONTROL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MIGHTYMOUSE) },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICMOUSE) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ISO) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 010368e..11e5218 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -56,6 +56,7 @@
 
 #define USB_VENDOR_ID_APPLE		0x05ac
 #define USB_DEVICE_ID_APPLE_MIGHTYMOUSE	0x0304
+#define USB_DEVICE_ID_APPLE_MAGICMOUSE	0x030d
 #define USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI	0x020e
 #define USB_DEVICE_ID_APPLE_FOUNTAIN_ISO	0x020f
 #define USB_DEVICE_ID_APPLE_GEYSER_ANSI	0x0214
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
new file mode 100644
index 0000000..f94b3e4
--- /dev/null
+++ b/drivers/hid/hid-magicmouse.c
@@ -0,0 +1,469 @@
+/*
+ *   Apple "Magic" Wireless Mouse driver
+ *
+ *   Copyright (c) 2010 Michael Poole <mdpoole@troilus.org>
+ */
+
+/*
+ * 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 (at your option)
+ * any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+
+#include "hid-ids.h"
+
+static bool emulate_3button = 1;
+module_param(emulate_3button, bool, 0644);
+MODULE_PARM_DESC(emulate_3button, "Emulate a middle button");
+
+static int middle_button_start = -350;
+static int middle_button_stop = +350;
+
+static bool emulate_scroll_wheel = 1;
+module_param(emulate_scroll_wheel, bool, 0644);
+MODULE_PARM_DESC(emulate_scroll_wheel, "Emulate a scroll wheel");
+
+static bool report_touches = 1;
+module_param(report_touches, bool, 0644);
+MODULE_PARM_DESC(report_touches, "Emit touch records (otherwise, only use them for emulation)");
+
+static bool report_undeciphered = 0;
+module_param(report_undeciphered, bool, 0644);
+MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");
+
+#define TOUCH_REPORT_ID   0x29
+/* These definitions are not precise, but they're close enough.  (Bits
+ * 0x03 seem to indicate the aspect ratio of the touch, bits 0x70 seem
+ * to be some kind of bit mask -- 0x20 may be a near-field reading,
+ * and 0x40 is actual contact, and 0x10 may be a start/stop or change
+ * indication.)
+ */
+#define TOUCH_STATE_MASK  0xf0
+#define TOUCH_STATE_NONE  0x00
+#define TOUCH_STATE_START 0x30
+#define TOUCH_STATE_DRAG  0x40
+
+/**
+ * struct magicmouse_sc - Tracks Magic Mouse-specific data.
+ * @input: Input device through which we report events.
+ * @quirks: Currently unused.
+ * @last_timestamp: Timestamp from most recent (18-bit) touch report
+ *     (units of milliseconds over short windows, but seems to
+ *     increase faster when there are no touches).
+ * @delta_time: 18-bit difference between the two most recent touch
+ *     reports from the mouse.
+ * @ntouches: Number of touches in most recent touch report.
+ * @scroll_accel: Number of consecutive scroll motions.
+ * @scroll_jiffies: Time of last scroll motion.
+ * @touches: Most recent data for a touch, indexed by tracking ID.
+ * @tracking_ids: Mapping of current touch input data to @touches.
+ */
+struct magicmouse_sc {
+	struct input_dev *input;
+	unsigned long quirks;
+
+	int last_timestamp;
+	int delta_time;
+	int ntouches;
+	int scroll_accel;
+	unsigned long scroll_jiffies;
+
+	struct {
+		short x;
+		short y;
+		short scroll_y;
+		u8 size;
+	} touches[16];
+	int tracking_ids[16];
+};
+
+static int magicmouse_firm_touch(struct magicmouse_sc *msc)
+{
+	int touch = -1;
+	int ii;
+
+	/* If there is only one "firm" touch, set touch to its
+	 * tracking ID.
+	 */
+	for (ii = 0; ii < msc->ntouches; ii++) {
+		int idx = msc->tracking_ids[ii];
+		if (msc->touches[idx].size < 8) {
+			/* Ignore this touch. */
+		} else if (touch >= 0) {
+			touch = -1;
+			break;
+		} else {
+			touch = idx;
+		}
+	}
+
+	return touch;
+}
+
+static void magicmouse_emit_buttons(struct magicmouse_sc *msc, int state)
+{
+        int last_state = test_bit(BTN_LEFT, msc->input->key) << 0 |
+                test_bit(BTN_RIGHT, msc->input->key) << 1 |
+                test_bit(BTN_MIDDLE, msc->input->key) << 2;
+
+	if (emulate_3button) {
+		int id;
+
+		/* If some button was pressed before, keep it held
+		 * down.  Otherwise, if there's exactly one firm
+		 * touch, use that to override the mouse's guess.
+		 */
+		if (state == 0) {
+			/* The button was released. */
+		} else if (last_state != 0) {
+			state = last_state;
+		} else if ((id = magicmouse_firm_touch(msc)) >= 0) {
+			int x = msc->touches[id].x;
+			if (x < middle_button_start)
+				state = 1;
+			else if (x > middle_button_stop)
+				state = 2;
+			else
+				state = 4;
+		} /* else: we keep the mouse's guess */
+
+		input_report_key(msc->input, BTN_MIDDLE, state & 4);
+	}
+
+	input_report_key(msc->input, BTN_LEFT, state & 1);
+	input_report_key(msc->input, BTN_RIGHT, state & 2);
+
+	if (state != last_state)
+		msc->scroll_accel = 0;
+}
+
+static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tdata)
+{
+	struct input_dev *input = msc->input;
+	__s32 x_y = tdata[0] << 8 | tdata[1] << 16 | tdata[2] << 24;
+	int misc = tdata[5] | tdata[6] << 8;
+	int id = (misc >> 6) & 15;
+	int x = x_y << 12 >> 20;
+	int y = -(x_y >> 20);
+
+	/* Store tracking ID and other fields. */
+	msc->tracking_ids[raw_id] = id;
+	msc->touches[id].x = x;
+	msc->touches[id].y = y;
+	msc->touches[id].size = misc & 63;
+
+	/* If requested, emulate a scroll wheel by detecting small
+	 * vertical touch motions along the middle of the mouse.
+	 */
+	if (emulate_scroll_wheel &&
+	    middle_button_start < x && x < middle_button_stop) {
+		static const int accel_profile[] = {
+			256, 228, 192, 160, 128, 96, 64, 32,
+		};
+		unsigned long now = jiffies;
+		int step = msc->touches[id].scroll_y - y;
+
+		/* Reset acceleration after half a second. */
+		if (time_after(now, msc->scroll_jiffies + HZ / 2))
+			msc->scroll_accel = 0;
+
+		/* Calculate and apply the scroll motion. */
+		switch (tdata[7] & TOUCH_STATE_MASK) {
+		case TOUCH_STATE_START:
+			msc->touches[id].scroll_y = y;
+                        msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
+						ARRAY_SIZE(accel_profile) - 1);
+			break;
+		case TOUCH_STATE_DRAG:
+			step = step / accel_profile[msc->scroll_accel];
+			if (step != 0) {
+				msc->touches[id].scroll_y = y;
+				msc->scroll_jiffies = now;
+				input_report_rel(input, REL_WHEEL, step);
+			}
+			break;
+		}
+	}
+
+	/* Generate the input events for this touch. */
+	if (report_touches) {
+                int orientation = (misc >> 10) - 32;
+
+		input_report_abs(input, ABS_MT_TRACKING_ID, id);
+		input_report_abs(input, ABS_MT_TOUCH_MAJOR, tdata[3]);
+		input_report_abs(input, ABS_MT_TOUCH_MINOR, tdata[4]);
+		input_report_abs(input, ABS_MT_ORIENTATION, orientation);
+		input_report_abs(input, ABS_MT_POSITION_X, x);
+		input_report_abs(input, ABS_MT_POSITION_Y, y);
+
+		if (report_undeciphered) {
+			input_event(input, EV_MSC, MSC_RAW, tdata[7]);
+		}
+
+		input_mt_sync(input);
+	}
+}
+
+static int magicmouse_raw_event(struct hid_device *hdev,
+		struct hid_report *report, u8 *data, int size)
+{
+	struct magicmouse_sc *msc = hid_get_drvdata(hdev);
+	struct input_dev *input = msc->input;
+	int x, y, ts, ii, clicks;
+
+	switch (data[0]) {
+	case 0x10:
+		if (size != 6)
+			return 0;
+		x = (__s16)(data[2] | data[3] << 8);
+		y = (__s16)(data[4] | data[5] << 8);
+		clicks = data[1];
+		break;
+	case TOUCH_REPORT_ID:
+		/* Expect six bytes of prefix, and N*8 bytes of touch data. */
+		if (size < 6 || ((size - 6) % 8) != 0)
+			return 0;
+		ts = data[3] >> 6 | data[4] << 2 | data[5] << 10;
+		msc->delta_time = (ts - msc->last_timestamp) & 0x3ffff;
+		msc->last_timestamp = ts;
+		msc->ntouches = (size - 6) / 8;
+		for (ii = 0; ii < msc->ntouches; ii++)
+			magicmouse_emit_touch(msc, ii, data + ii * 8 + 6);
+		/* When emulating three-button mode, it is important
+		 * to have the current touch information before
+		 * generating a click event.
+		 */
+		x = (signed char)data[1];
+		y = (signed char)data[2];
+		clicks = data[3];
+		break;
+	case 0x20: /* Theoretically battery status (0-100), but I have
+		    * never seen it -- maybe it is only upon request.
+		    */
+	case 0x60: /* Unknown, maybe laser on/off. */
+	case 0x61: /* Laser reflection status change.
+		    * data[1]: 0 = spotted, 1 = lost
+		    */
+	default:
+		return 0;
+	}
+
+	magicmouse_emit_buttons(msc, clicks & 3);
+	input_report_rel(input, REL_X, x);
+	input_report_rel(input, REL_Y, y);
+	input_sync(input);
+	return 1;
+}
+
+static int magicmouse_input_open(struct input_dev *dev)
+{
+	struct hid_device *hid = input_get_drvdata(dev);
+
+	return hid->ll_driver->open(hid);
+}
+
+static void magicmouse_input_close(struct input_dev *dev)
+{
+	struct hid_device *hid = input_get_drvdata(dev);
+
+	hid->ll_driver->close(hid);
+}
+
+static void magicmouse_setup_input(struct input_dev *input, struct hid_device *hdev)
+{
+	input_set_drvdata(input, hdev);
+	input->event = hdev->ll_driver->hidinput_input_event;
+	input->open = magicmouse_input_open;
+	input->close = magicmouse_input_close;
+
+	input->name = hdev->name;
+	input->phys = hdev->phys;
+	input->uniq = hdev->uniq;
+	input->id.bustype = hdev->bus;
+	input->id.vendor = hdev->vendor;
+	input->id.product = hdev->product;
+	input->id.version = hdev->version;
+	input->dev.parent = hdev->dev.parent;
+
+	set_bit(EV_KEY, input->evbit);
+	set_bit(BTN_LEFT, input->keybit);
+	set_bit(BTN_RIGHT, input->keybit);
+	if (emulate_3button)
+		set_bit(BTN_MIDDLE, input->keybit);
+	set_bit(BTN_TOOL_FINGER, input->keybit);
+
+	set_bit(EV_REL, input->evbit);
+	set_bit(REL_X, input->relbit);
+	set_bit(REL_Y, input->relbit);
+	if (emulate_scroll_wheel)
+		set_bit(REL_WHEEL, input->relbit);
+
+	if (report_touches) {
+		set_bit(EV_ABS, input->evbit);
+
+		set_bit(ABS_MT_TRACKING_ID, input->absbit);
+		input->absmin[ABS_MT_TRACKING_ID] = 0;
+		input->absmax[ABS_MT_TRACKING_ID] = 15;
+		input->absfuzz[ABS_MT_TRACKING_ID] = 0;
+
+		set_bit(ABS_MT_TOUCH_MAJOR, input->absbit);
+		input->absmin[ABS_MT_TOUCH_MAJOR] = 0;
+		input->absmax[ABS_MT_TOUCH_MAJOR] = 255;
+		input->absfuzz[ABS_MT_TOUCH_MAJOR] = 4;
+
+		set_bit(ABS_MT_TOUCH_MINOR, input->absbit);
+		input->absmin[ABS_MT_TOUCH_MINOR] = 0;
+		input->absmax[ABS_MT_TOUCH_MINOR] = 255;
+		input->absfuzz[ABS_MT_TOUCH_MINOR] = 4;
+
+		set_bit(ABS_MT_ORIENTATION, input->absbit);
+		input->absmin[ABS_MT_ORIENTATION] = -32;
+		input->absmax[ABS_MT_ORIENTATION] = 31;
+		input->absfuzz[ABS_MT_ORIENTATION] = 1;
+
+		set_bit(ABS_MT_POSITION_X, input->absbit);
+		input->absmin[ABS_MT_POSITION_X] = -1100;
+		input->absmax[ABS_MT_POSITION_X] = 1358;
+		input->absfuzz[ABS_MT_POSITION_X] = 4;
+
+		/* Note: Touch Y position from the device is inverted relative
+		 * to how pointer motion is reported (and relative to how USB
+		 * HID recommends the coordinates work).  This driver keeps
+		 * the origin at the same position, and just uses the additive
+		 * inverse of the reported Y.
+		 */
+		set_bit(ABS_MT_POSITION_Y, input->absbit);
+		input->absmin[ABS_MT_POSITION_Y] = -1589;
+		input->absmax[ABS_MT_POSITION_Y] = 2047;
+		input->absfuzz[ABS_MT_POSITION_Y] = 4;
+	}
+
+	if (report_undeciphered) {
+		set_bit(EV_MSC, input->evbit);
+		set_bit(MSC_RAW, input->mscbit);
+	}
+}
+
+static int magicmouse_probe(struct hid_device *hdev,
+	const struct hid_device_id *id)
+{
+	__u8 feature_1[] = { 0xd7, 0x01 };
+	__u8 feature_2[] = { 0xf8, 0x01, 0x32 };
+	struct input_dev *input;
+	struct magicmouse_sc *msc;
+	struct hid_report *report;
+	int ret;
+
+	msc = kzalloc(sizeof(*msc), GFP_KERNEL);
+	if (msc == NULL) {
+		dev_err(&hdev->dev, "can't alloc magicmouse descriptor\n");
+		return -ENOMEM;
+	}
+
+	msc->quirks = id->driver_data;
+	hid_set_drvdata(hdev, msc);
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		dev_err(&hdev->dev, "magicmouse hid parse failed\n");
+		goto err_free;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (ret) {
+		dev_err(&hdev->dev, "magicmouse hw start failed\n");
+		goto err_free;
+	}
+
+	report = hid_register_report(hdev, HID_INPUT_REPORT, TOUCH_REPORT_ID);
+	if (!report) {
+		dev_err(&hdev->dev, "unable to register touch report\n");
+		ret = -ENOMEM;
+		goto err_free;
+	}
+	report->size = 6;
+
+	ret = hdev->hid_output_raw_report(hdev, feature_1, sizeof(feature_1),
+			HID_FEATURE_REPORT);
+	if (ret != sizeof(feature_1)) {
+		dev_err(&hdev->dev, "unable to request touch data (1:%d)\n",
+				ret);
+		goto err_free;
+	}
+	ret = hdev->hid_output_raw_report(hdev, feature_2,
+			sizeof(feature_2), HID_FEATURE_REPORT);
+	if (ret != sizeof(feature_2)) {
+		dev_err(&hdev->dev, "unable to request touch data (2:%d)\n",
+				ret);
+		goto err_free;
+	}
+
+	input = input_allocate_device();
+	if (!input) {
+		dev_err(&hdev->dev, "can't alloc input device\n");
+		ret = -ENOMEM;
+		goto err_free;
+	}
+	magicmouse_setup_input(input, hdev);
+
+	ret = input_register_device(input);
+	if (ret) {
+		dev_err(&hdev->dev, "input device registration failed\n");
+		goto err_both;
+	}
+	msc->input = input;
+
+	return 0;
+ err_both:
+	input_free_device(input);
+ err_free:
+	kfree(msc);
+	return ret;
+}
+
+static void magicmouse_remove(struct hid_device *hdev)
+{
+	hid_hw_stop(hdev);
+	kfree(hid_get_drvdata(hdev));
+}
+
+static const struct hid_device_id magic_mice[] = {
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICMOUSE),
+		.driver_data = 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(hid, magic_mice);
+
+static struct hid_driver magicmouse_driver = {
+	.name = "magicmouse",
+	.id_table = magic_mice,
+	.probe = magicmouse_probe,
+	.remove = magicmouse_remove,
+	.raw_event = magicmouse_raw_event,
+};
+
+static int __init magicmouse_init(void)
+{
+	int ret;
+
+	ret = hid_register_driver(&magicmouse_driver);
+	if (ret)
+		printk(KERN_ERR "can't register magicmouse driver\n");
+
+	return ret;
+}
+
+static void __exit magicmouse_exit(void)
+{
+	hid_unregister_driver(&magicmouse_driver);
+}
+
+module_init(magicmouse_init);
+module_exit(magicmouse_exit);
+MODULE_LICENSE("GPL");
-- 
1.6.5.6

Re: [PATCH 2/2] Add a device driver for the Apple Magic Mouse.

[Dmitry Torokhov]

Hi Michael,

On Tue, Feb 09, 2010 at 08:13:08AM -0500, Michael Poole wrote:
> +
> +static bool emulate_3button = 1;

If it is a bool then values should be true/false.

> +module_param(emulate_3button, bool, 0644);
> +MODULE_PARM_DESC(emulate_3button, "Emulate a middle button");
> +
> +static int middle_button_start = -350;
> +static int middle_button_stop = +350;
> +
> +static bool emulate_scroll_wheel = 1;
> +module_param(emulate_scroll_wheel, bool, 0644);
> +MODULE_PARM_DESC(emulate_scroll_wheel, "Emulate a scroll wheel");
> +
> +static bool report_touches = 1;
> +module_param(report_touches, bool, 0644);
> +MODULE_PARM_DESC(report_touches, "Emit touch records (otherwise, only use them for emulation)");
> +
> +static bool report_undeciphered = 0;

No need to initialize statics to 0/false.

> +module_param(report_undeciphered, bool, 0644);
> +MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");
> +
> +#define TOUCH_REPORT_ID   0x29
> +/* These definitions are not precise, but they're close enough.  (Bits
> + * 0x03 seem to indicate the aspect ratio of the touch, bits 0x70 seem
> + * to be some kind of bit mask -- 0x20 may be a near-field reading,
> + * and 0x40 is actual contact, and 0x10 may be a start/stop or change
> + * indication.)
> + */
> +#define TOUCH_STATE_MASK  0xf0
> +#define TOUCH_STATE_NONE  0x00
> +#define TOUCH_STATE_START 0x30
> +#define TOUCH_STATE_DRAG  0x40
> +
> +/**
> + * struct magicmouse_sc - Tracks Magic Mouse-specific data.
> + * @input: Input device through which we report events.
> + * @quirks: Currently unused.
> + * @last_timestamp: Timestamp from most recent (18-bit) touch report
> + *     (units of milliseconds over short windows, but seems to
> + *     increase faster when there are no touches).
> + * @delta_time: 18-bit difference between the two most recent touch
> + *     reports from the mouse.
> + * @ntouches: Number of touches in most recent touch report.
> + * @scroll_accel: Number of consecutive scroll motions.
> + * @scroll_jiffies: Time of last scroll motion.
> + * @touches: Most recent data for a touch, indexed by tracking ID.
> + * @tracking_ids: Mapping of current touch input data to @touches.
> + */
> +struct magicmouse_sc {
> +	struct input_dev *input;
> +	unsigned long quirks;
> +
> +	int last_timestamp;
> +	int delta_time;
> +	int ntouches;
> +	int scroll_accel;
> +	unsigned long scroll_jiffies;
> +
> +	struct {
> +		short x;
> +		short y;
> +		short scroll_y;
> +		u8 size;
> +	} touches[16];
> +	int tracking_ids[16];
> +};
> +
> +static int magicmouse_firm_touch(struct magicmouse_sc *msc)
> +{
> +	int touch = -1;
> +	int ii;
> +
> +	/* If there is only one "firm" touch, set touch to its
> +	 * tracking ID.
> +	 */
> +	for (ii = 0; ii < msc->ntouches; ii++) {
> +		int idx = msc->tracking_ids[ii];
> +		if (msc->touches[idx].size < 8) {
> +			/* Ignore this touch. */
> +		} else if (touch >= 0) {
> +			touch = -1;
> +			break;
> +		} else {
> +			touch = idx;
> +		}
> +	}
> +
> +	return touch;
> +}
> +
> +static void magicmouse_emit_buttons(struct magicmouse_sc *msc, int state)
> +{
> +        int last_state = test_bit(BTN_LEFT, msc->input->key) << 0 |
> +                test_bit(BTN_RIGHT, msc->input->key) << 1 |
> +                test_bit(BTN_MIDDLE, msc->input->key) << 2;

Indented with spaces, we prefer tabs.

> +
> +	if (emulate_3button) {
> +		int id;
> +
> +		/* If some button was pressed before, keep it held
> +		 * down.  Otherwise, if there's exactly one firm
> +		 * touch, use that to override the mouse's guess.
> +		 */
> +		if (state == 0) {
> +			/* The button was released. */
> +		} else if (last_state != 0) {
> +			state = last_state;
> +		} else if ((id = magicmouse_firm_touch(msc)) >= 0) {
> +			int x = msc->touches[id].x;
> +			if (x < middle_button_start)
> +				state = 1;
> +			else if (x > middle_button_stop)
> +				state = 2;
> +			else
> +				state = 4;
> +		} /* else: we keep the mouse's guess */
> +
> +		input_report_key(msc->input, BTN_MIDDLE, state & 4);
> +	}
> +
> +	input_report_key(msc->input, BTN_LEFT, state & 1);
> +	input_report_key(msc->input, BTN_RIGHT, state & 2);
> +
> +	if (state != last_state)
> +		msc->scroll_accel = 0;
> +}
> +
> +static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tdata)
> +{
> +	struct input_dev *input = msc->input;
> +	__s32 x_y = tdata[0] << 8 | tdata[1] << 16 | tdata[2] << 24;
> +	int misc = tdata[5] | tdata[6] << 8;
> +	int id = (misc >> 6) & 15;
> +	int x = x_y << 12 >> 20;
> +	int y = -(x_y >> 20);
> +
> +	/* Store tracking ID and other fields. */
> +	msc->tracking_ids[raw_id] = id;
> +	msc->touches[id].x = x;
> +	msc->touches[id].y = y;
> +	msc->touches[id].size = misc & 63;
> +
> +	/* If requested, emulate a scroll wheel by detecting small
> +	 * vertical touch motions along the middle of the mouse.
> +	 */
> +	if (emulate_scroll_wheel &&
> +	    middle_button_start < x && x < middle_button_stop) {
> +		static const int accel_profile[] = {
> +			256, 228, 192, 160, 128, 96, 64, 32,
> +		};
> +		unsigned long now = jiffies;
> +		int step = msc->touches[id].scroll_y - y;
> +
> +		/* Reset acceleration after half a second. */
> +		if (time_after(now, msc->scroll_jiffies + HZ / 2))
> +			msc->scroll_accel = 0;
> +
> +		/* Calculate and apply the scroll motion. */
> +		switch (tdata[7] & TOUCH_STATE_MASK) {
> +		case TOUCH_STATE_START:
> +			msc->touches[id].scroll_y = y;
> +                        msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
> +						ARRAY_SIZE(accel_profile) - 1);
> +			break;
> +		case TOUCH_STATE_DRAG:
> +			step = step / accel_profile[msc->scroll_accel];
> +			if (step != 0) {
> +				msc->touches[id].scroll_y = y;
> +				msc->scroll_jiffies = now;
> +				input_report_rel(input, REL_WHEEL, step);
> +			}
> +			break;
> +		}
> +	}
> +
> +	/* Generate the input events for this touch. */
> +	if (report_touches) {
> +                int orientation = (misc >> 10) - 32;
> +
> +		input_report_abs(input, ABS_MT_TRACKING_ID, id);
> +		input_report_abs(input, ABS_MT_TOUCH_MAJOR, tdata[3]);
> +		input_report_abs(input, ABS_MT_TOUCH_MINOR, tdata[4]);
> +		input_report_abs(input, ABS_MT_ORIENTATION, orientation);
> +		input_report_abs(input, ABS_MT_POSITION_X, x);
> +		input_report_abs(input, ABS_MT_POSITION_Y, y);
> +
> +		if (report_undeciphered) {
> +			input_event(input, EV_MSC, MSC_RAW, tdata[7]);
> +		}

No need for braces for single statement.

> +
> +		input_mt_sync(input);
> +	}
> +}
> +
> +static int magicmouse_raw_event(struct hid_device *hdev,
> +		struct hid_report *report, u8 *data, int size)
> +{
> +	struct magicmouse_sc *msc = hid_get_drvdata(hdev);
> +	struct input_dev *input = msc->input;
> +	int x, y, ts, ii, clicks;
> +
> +	switch (data[0]) {
> +	case 0x10:
> +		if (size != 6)
> +			return 0;
> +		x = (__s16)(data[2] | data[3] << 8);
> +		y = (__s16)(data[4] | data[5] << 8);
> +		clicks = data[1];
> +		break;
> +	case TOUCH_REPORT_ID:
> +		/* Expect six bytes of prefix, and N*8 bytes of touch data. */
> +		if (size < 6 || ((size - 6) % 8) != 0)
> +			return 0;
> +		ts = data[3] >> 6 | data[4] << 2 | data[5] << 10;
> +		msc->delta_time = (ts - msc->last_timestamp) & 0x3ffff;
> +		msc->last_timestamp = ts;
> +		msc->ntouches = (size - 6) / 8;
> +		for (ii = 0; ii < msc->ntouches; ii++)
> +			magicmouse_emit_touch(msc, ii, data + ii * 8 + 6);
> +		/* When emulating three-button mode, it is important
> +		 * to have the current touch information before
> +		 * generating a click event.
> +		 */
> +		x = (signed char)data[1];
> +		y = (signed char)data[2];
> +		clicks = data[3];
> +		break;
> +	case 0x20: /* Theoretically battery status (0-100), but I have
> +		    * never seen it -- maybe it is only upon request.
> +		    */
> +	case 0x60: /* Unknown, maybe laser on/off. */
> +	case 0x61: /* Laser reflection status change.
> +		    * data[1]: 0 = spotted, 1 = lost
> +		    */
> +	default:
> +		return 0;
> +	}
> +
> +	magicmouse_emit_buttons(msc, clicks & 3);
> +	input_report_rel(input, REL_X, x);
> +	input_report_rel(input, REL_Y, y);
> +	input_sync(input);
> +	return 1;
> +}
> +
> +static int magicmouse_input_open(struct input_dev *dev)
> +{
> +	struct hid_device *hid = input_get_drvdata(dev);
> +
> +	return hid->ll_driver->open(hid);
> +}
> +
> +static void magicmouse_input_close(struct input_dev *dev)
> +{
> +	struct hid_device *hid = input_get_drvdata(dev);
> +
> +	hid->ll_driver->close(hid);
> +}
> +
> +static void magicmouse_setup_input(struct input_dev *input, struct hid_device *hdev)
> +{
> +	input_set_drvdata(input, hdev);
> +	input->event = hdev->ll_driver->hidinput_input_event;
> +	input->open = magicmouse_input_open;
> +	input->close = magicmouse_input_close;
> +
> +	input->name = hdev->name;
> +	input->phys = hdev->phys;
> +	input->uniq = hdev->uniq;
> +	input->id.bustype = hdev->bus;
> +	input->id.vendor = hdev->vendor;
> +	input->id.product = hdev->product;
> +	input->id.version = hdev->version;
> +	input->dev.parent = hdev->dev.parent;
> +
> +	set_bit(EV_KEY, input->evbit);
> +	set_bit(BTN_LEFT, input->keybit);
> +	set_bit(BTN_RIGHT, input->keybit);
> +	if (emulate_3button)
> +		set_bit(BTN_MIDDLE, input->keybit);
> +	set_bit(BTN_TOOL_FINGER, input->keybit);
> +
> +	set_bit(EV_REL, input->evbit);
> +	set_bit(REL_X, input->relbit);
> +	set_bit(REL_Y, input->relbit);
> +	if (emulate_scroll_wheel)
> +		set_bit(REL_WHEEL, input->relbit);

I'd use __set_bit() instead, no need to lock the bus.

> +
> +	if (report_touches) {
> +		set_bit(EV_ABS, input->evbit);
> +
> +		set_bit(ABS_MT_TRACKING_ID, input->absbit);
> +		input->absmin[ABS_MT_TRACKING_ID] = 0;
> +		input->absmax[ABS_MT_TRACKING_ID] = 15;
> +		input->absfuzz[ABS_MT_TRACKING_ID] = 0;

input_set_abs_params() is a bit more compact.

> +
> +		set_bit(ABS_MT_TOUCH_MAJOR, input->absbit);
> +		input->absmin[ABS_MT_TOUCH_MAJOR] = 0;
> +		input->absmax[ABS_MT_TOUCH_MAJOR] = 255;
> +		input->absfuzz[ABS_MT_TOUCH_MAJOR] = 4;
> +
> +		set_bit(ABS_MT_TOUCH_MINOR, input->absbit);
> +		input->absmin[ABS_MT_TOUCH_MINOR] = 0;
> +		input->absmax[ABS_MT_TOUCH_MINOR] = 255;
> +		input->absfuzz[ABS_MT_TOUCH_MINOR] = 4;
> +
> +		set_bit(ABS_MT_ORIENTATION, input->absbit);
> +		input->absmin[ABS_MT_ORIENTATION] = -32;
> +		input->absmax[ABS_MT_ORIENTATION] = 31;
> +		input->absfuzz[ABS_MT_ORIENTATION] = 1;
> +
> +		set_bit(ABS_MT_POSITION_X, input->absbit);
> +		input->absmin[ABS_MT_POSITION_X] = -1100;
> +		input->absmax[ABS_MT_POSITION_X] = 1358;
> +		input->absfuzz[ABS_MT_POSITION_X] = 4;
> +
> +		/* Note: Touch Y position from the device is inverted relative
> +		 * to how pointer motion is reported (and relative to how USB
> +		 * HID recommends the coordinates work).  This driver keeps
> +		 * the origin at the same position, and just uses the additive
> +		 * inverse of the reported Y.
> +		 */
> +		set_bit(ABS_MT_POSITION_Y, input->absbit);
> +		input->absmin[ABS_MT_POSITION_Y] = -1589;
> +		input->absmax[ABS_MT_POSITION_Y] = 2047;
> +		input->absfuzz[ABS_MT_POSITION_Y] = 4;
> +	}
> +
> +	if (report_undeciphered) {
> +		set_bit(EV_MSC, input->evbit);
> +		set_bit(MSC_RAW, input->mscbit);
> +	}
> +}
> +
> +static int magicmouse_probe(struct hid_device *hdev,
> +	const struct hid_device_id *id)
> +{
> +	__u8 feature_1[] = { 0xd7, 0x01 };
> +	__u8 feature_2[] = { 0xf8, 0x01, 0x32 };
> +	struct input_dev *input;
> +	struct magicmouse_sc *msc;
> +	struct hid_report *report;
> +	int ret;
> +
> +	msc = kzalloc(sizeof(*msc), GFP_KERNEL);
> +	if (msc == NULL) {
> +		dev_err(&hdev->dev, "can't alloc magicmouse descriptor\n");
> +		return -ENOMEM;
> +	}
> +
> +	msc->quirks = id->driver_data;
> +	hid_set_drvdata(hdev, msc);
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		dev_err(&hdev->dev, "magicmouse hid parse failed\n");
> +		goto err_free;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> +	if (ret) {
> +		dev_err(&hdev->dev, "magicmouse hw start failed\n");
> +		goto err_free;
> +	}
> +
> +	report = hid_register_report(hdev, HID_INPUT_REPORT, TOUCH_REPORT_ID);
> +	if (!report) {
> +		dev_err(&hdev->dev, "unable to register touch report\n");
> +		ret = -ENOMEM;
> +		goto err_free;
> +	}
> +	report->size = 6;
> +
> +	ret = hdev->hid_output_raw_report(hdev, feature_1, sizeof(feature_1),
> +			HID_FEATURE_REPORT);

This will cause transmission of on-stack data through USB which it does
not like.

> +	if (ret != sizeof(feature_1)) {
> +		dev_err(&hdev->dev, "unable to request touch data (1:%d)\n",
> +				ret);
> +		goto err_free;
> +	}
> +	ret = hdev->hid_output_raw_report(hdev, feature_2,
> +			sizeof(feature_2), HID_FEATURE_REPORT);

Same here.

> +	if (ret != sizeof(feature_2)) {
> +		dev_err(&hdev->dev, "unable to request touch data (2:%d)\n",
> +				ret);
> +		goto err_free;
> +	}
> +
> +	input = input_allocate_device();
> +	if (!input) {
> +		dev_err(&hdev->dev, "can't alloc input device\n");
> +		ret = -ENOMEM;
> +		goto err_free;
> +	}
> +	magicmouse_setup_input(input, hdev);
> +
> +	ret = input_register_device(input);
> +	if (ret) {
> +		dev_err(&hdev->dev, "input device registration failed\n");
> +		goto err_both;
> +	}
> +	msc->input = input;
> +
> +	return 0;
> + err_both:
> +	input_free_device(input);
> + err_free:
> +	kfree(msc);
> +	return ret;

hid_hw_stop() is missing in error path. Also see question about freeing
report below.

> +}
> +
> +static void magicmouse_remove(struct hid_device *hdev)
> +{

Do we need to unregister report (I am not that familiar with HID, who is
responsible for cleaning report lists?)?

> +	hid_hw_stop(hdev);
> +	kfree(hid_get_drvdata(hdev));
> +}
> +
> +static const struct hid_device_id magic_mice[] = {
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICMOUSE),
> +		.driver_data = 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(hid, magic_mice);
> +
> +static struct hid_driver magicmouse_driver = {
> +	.name = "magicmouse",
> +	.id_table = magic_mice,
> +	.probe = magicmouse_probe,
> +	.remove = magicmouse_remove,
> +	.raw_event = magicmouse_raw_event,
> +};
> +
> +static int __init magicmouse_init(void)
> +{
> +	int ret;
> +
> +	ret = hid_register_driver(&magicmouse_driver);
> +	if (ret)
> +		printk(KERN_ERR "can't register magicmouse driver\n");
> +
> +	return ret;
> +}
> +
> +static void __exit magicmouse_exit(void)
> +{
> +	hid_unregister_driver(&magicmouse_driver);
> +}
> +
> +module_init(magicmouse_init);
> +module_exit(magicmouse_exit);
> +MODULE_LICENSE("GPL");
> -- 
> 1.6.5.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Dmitry

Re: [PATCH 2/2] Add a device driver for the Apple Magic Mouse.

[Michael Poole]

I'll post a patch for these based on Jiri's for-next tree this evening.

Dmitry Torokhov writes:

> Hi Michael,
>
> On Tue, Feb 09, 2010 at 08:13:08AM -0500, Michael Poole wrote:
>> +
>> +static bool emulate_3button = 1;
>
> If it is a bool then values should be true/false.

Thanks.

>> +module_param(emulate_3button, bool, 0644);
>> +MODULE_PARM_DESC(emulate_3button, "Emulate a middle button");
>> +
>> +static int middle_button_start = -350;
>> +static int middle_button_stop = +350;
>> +
>> +static bool emulate_scroll_wheel = 1;
>> +module_param(emulate_scroll_wheel, bool, 0644);
>> +MODULE_PARM_DESC(emulate_scroll_wheel, "Emulate a scroll wheel");
>> +
>> +static bool report_touches = 1;
>> +module_param(report_touches, bool, 0644);
>> +MODULE_PARM_DESC(report_touches, "Emit touch records (otherwise, only use them for emulation)");
>> +
>> +static bool report_undeciphered = 0;
>
> No need to initialize statics to 0/false.

Okay, I'll remote that.

[snip]
>> +        int last_state = test_bit(BTN_LEFT, msc->input->key) << 0 |
>> +                test_bit(BTN_RIGHT, msc->input->key) << 1 |
>> +                test_bit(BTN_MIDDLE, msc->input->key) << 2;
>
> Indented with spaces, we prefer tabs.

Whoops -- one of my machines must be misconfigured.  I'll fix that.

[snip]
>> +		if (report_undeciphered) {
>> +			input_event(input, EV_MSC, MSC_RAW, tdata[7]);
>> +		}
>
> No need for braces for single statement.

Okay.

[snip]
>> +	if (emulate_scroll_wheel)
>> +		set_bit(REL_WHEEL, input->relbit);
>
> I'd use __set_bit() instead, no need to lock the bus.

Okay.  I wasn't aware of that semantic difference.

>> +
>> +	if (report_touches) {
>> +		set_bit(EV_ABS, input->evbit);
>> +
>> +		set_bit(ABS_MT_TRACKING_ID, input->absbit);
>> +		input->absmin[ABS_MT_TRACKING_ID] = 0;
>> +		input->absmax[ABS_MT_TRACKING_ID] = 15;
>> +		input->absfuzz[ABS_MT_TRACKING_ID] = 0;
>
> input_set_abs_params() is a bit more compact.

Thanks -- more idiomatic is always handy.

[snip]
>> +	ret = hdev->hid_output_raw_report(hdev, feature_1, sizeof(feature_1),
>> +			HID_FEATURE_REPORT);
>
> This will cause transmission of on-stack data through USB which it does
> not like.

Current Magic Mouse devices are Bluetooth-only.  Is this an issue for
that stack as well?  (hid-sony.c and hid-wacom.c also use
hid_output_raw_report() with on-stack buffers for Bluetooth devices --
if they need it too, I can write a patch for those.  I have a PS3
controller that I can test with, but don't have a Wacom tablet.)  Would
making those buffers static put them in an appropriate section, or does
the memory need to be allocated from a heap?

>> +	if (ret != sizeof(feature_1)) {
>> +		dev_err(&hdev->dev, "unable to request touch data (1:%d)\n",
>> +				ret);
>> +		goto err_free;
>> +	}
>> +	ret = hdev->hid_output_raw_report(hdev, feature_2,
>> +			sizeof(feature_2), HID_FEATURE_REPORT);
>
> Same here.
>
>> +	if (ret != sizeof(feature_2)) {
>> +		dev_err(&hdev->dev, "unable to request touch data (2:%d)\n",
>> +				ret);
>> +		goto err_free;
>> +	}
>> +
>> +	input = input_allocate_device();
>> +	if (!input) {
>> +		dev_err(&hdev->dev, "can't alloc input device\n");
>> +		ret = -ENOMEM;
>> +		goto err_free;
>> +	}
>> +	magicmouse_setup_input(input, hdev);
>> +
>> +	ret = input_register_device(input);
>> +	if (ret) {
>> +		dev_err(&hdev->dev, "input device registration failed\n");
>> +		goto err_both;
>> +	}
>> +	msc->input = input;
>> +
>> +	return 0;
>> + err_both:
>> +	input_free_device(input);
>> + err_free:
>> +	kfree(msc);
>> +	return ret;
>
> hid_hw_stop() is missing in error path. Also see question about freeing
> report below.

Thanks, I'll add the hid_hw_stop().

>> +}
>> +
>> +static void magicmouse_remove(struct hid_device *hdev)
>> +{
>
> Do we need to unregister report (I am not that familiar with HID, who is
> responsible for cleaning report lists?)?

hid-core.c's hid_device_release() frees all the reports that registered
with hid_register_report().

Michael Poole

[PATCH] hid-magicmouse: Coding style and probe failure fixes.

[Michael Poole]

>From 04b395dbbd1ad2b836188e6f125940ae8fac6925 Mon Sep 17 00:00:00 2001

As suggested by Dmitry Torokhov on 10 Feb: Use proper values
to initialize bool configuration variables, tabs rather than
spaces, no braces for one-line else clause, __set_bit() when
the operation doesn't have to be atomic, input_set_abs_params()
rather than writing the fields directly, and call hid_hw_stop()
when appropriate to handle failures in the probe.

Signed-off-by: Michael Poole <mdpoole@troilus.org>
---
Dmitry and Jiri,

I haven't had a chance to run-test these changes yet -- hid/for-next
causes corrupt X display on my laptop, whereas v2.6.33-rc6 and -rc7 are
fine; I'm still bisecting to figure out the cause -- but this patch is
not complicated.  (It does compile.)

I left the buffers for hdev->hid_output_raw_report() on the stack
because the Bluetooth HIDP code memcpy's the contents into a freshly
allocated skb.

Michael

 drivers/hid/hid-magicmouse.c |  100 +++++++++++++++++-------------------------
 1 files changed, 40 insertions(+), 60 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index f94b3e4..4a3a94f 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -18,22 +18,22 @@
 
 #include "hid-ids.h"
 
-static bool emulate_3button = 1;
+static bool emulate_3button = true;
 module_param(emulate_3button, bool, 0644);
 MODULE_PARM_DESC(emulate_3button, "Emulate a middle button");
 
 static int middle_button_start = -350;
 static int middle_button_stop = +350;
 
-static bool emulate_scroll_wheel = 1;
+static bool emulate_scroll_wheel = true;
 module_param(emulate_scroll_wheel, bool, 0644);
 MODULE_PARM_DESC(emulate_scroll_wheel, "Emulate a scroll wheel");
 
-static bool report_touches = 1;
+static bool report_touches = true;
 module_param(report_touches, bool, 0644);
 MODULE_PARM_DESC(report_touches, "Emit touch records (otherwise, only use them for emulation)");
 
-static bool report_undeciphered = 0;
+static bool report_undeciphered;
 module_param(report_undeciphered, bool, 0644);
 MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");
 
@@ -108,9 +108,9 @@ static int magicmouse_firm_touch(struct magicmouse_sc *msc)
 
 static void magicmouse_emit_buttons(struct magicmouse_sc *msc, int state)
 {
-        int last_state = test_bit(BTN_LEFT, msc->input->key) << 0 |
-                test_bit(BTN_RIGHT, msc->input->key) << 1 |
-                test_bit(BTN_MIDDLE, msc->input->key) << 2;
+	int last_state = test_bit(BTN_LEFT, msc->input->key) << 0 |
+		test_bit(BTN_RIGHT, msc->input->key) << 1 |
+		test_bit(BTN_MIDDLE, msc->input->key) << 2;
 
 	if (emulate_3button) {
 		int id;
@@ -177,7 +177,7 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 		switch (tdata[7] & TOUCH_STATE_MASK) {
 		case TOUCH_STATE_START:
 			msc->touches[id].scroll_y = y;
-                        msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
+			msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
 						ARRAY_SIZE(accel_profile) - 1);
 			break;
 		case TOUCH_STATE_DRAG:
@@ -193,7 +193,7 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 
 	/* Generate the input events for this touch. */
 	if (report_touches) {
-                int orientation = (misc >> 10) - 32;
+		int orientation = (misc >> 10) - 32;
 
 		input_report_abs(input, ABS_MT_TRACKING_ID, id);
 		input_report_abs(input, ABS_MT_TOUCH_MAJOR, tdata[3]);
@@ -202,9 +202,8 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda
 		input_report_abs(input, ABS_MT_POSITION_X, x);
 		input_report_abs(input, ABS_MT_POSITION_Y, y);
 
-		if (report_undeciphered) {
+		if (report_undeciphered)
 			input_event(input, EV_MSC, MSC_RAW, tdata[7]);
-		}
 
 		input_mt_sync(input);
 	}
@@ -291,62 +290,41 @@ static void magicmouse_setup_input(struct input_dev *input, struct hid_device *h
 	input->id.version = hdev->version;
 	input->dev.parent = hdev->dev.parent;
 
-	set_bit(EV_KEY, input->evbit);
-	set_bit(BTN_LEFT, input->keybit);
-	set_bit(BTN_RIGHT, input->keybit);
+	__set_bit(EV_KEY, input->evbit);
+	__set_bit(BTN_LEFT, input->keybit);
+	__set_bit(BTN_RIGHT, input->keybit);
 	if (emulate_3button)
-		set_bit(BTN_MIDDLE, input->keybit);
-	set_bit(BTN_TOOL_FINGER, input->keybit);
+		__set_bit(BTN_MIDDLE, input->keybit);
+	__set_bit(BTN_TOOL_FINGER, input->keybit);
 
-	set_bit(EV_REL, input->evbit);
-	set_bit(REL_X, input->relbit);
-	set_bit(REL_Y, input->relbit);
+	__set_bit(EV_REL, input->evbit);
+	__set_bit(REL_X, input->relbit);
+	__set_bit(REL_Y, input->relbit);
 	if (emulate_scroll_wheel)
-		set_bit(REL_WHEEL, input->relbit);
+		__set_bit(REL_WHEEL, input->relbit);
 
 	if (report_touches) {
-		set_bit(EV_ABS, input->evbit);
-
-		set_bit(ABS_MT_TRACKING_ID, input->absbit);
-		input->absmin[ABS_MT_TRACKING_ID] = 0;
-		input->absmax[ABS_MT_TRACKING_ID] = 15;
-		input->absfuzz[ABS_MT_TRACKING_ID] = 0;
-
-		set_bit(ABS_MT_TOUCH_MAJOR, input->absbit);
-		input->absmin[ABS_MT_TOUCH_MAJOR] = 0;
-		input->absmax[ABS_MT_TOUCH_MAJOR] = 255;
-		input->absfuzz[ABS_MT_TOUCH_MAJOR] = 4;
-
-		set_bit(ABS_MT_TOUCH_MINOR, input->absbit);
-		input->absmin[ABS_MT_TOUCH_MINOR] = 0;
-		input->absmax[ABS_MT_TOUCH_MINOR] = 255;
-		input->absfuzz[ABS_MT_TOUCH_MINOR] = 4;
-
-		set_bit(ABS_MT_ORIENTATION, input->absbit);
-		input->absmin[ABS_MT_ORIENTATION] = -32;
-		input->absmax[ABS_MT_ORIENTATION] = 31;
-		input->absfuzz[ABS_MT_ORIENTATION] = 1;
-
-		set_bit(ABS_MT_POSITION_X, input->absbit);
-		input->absmin[ABS_MT_POSITION_X] = -1100;
-		input->absmax[ABS_MT_POSITION_X] = 1358;
-		input->absfuzz[ABS_MT_POSITION_X] = 4;
-
+		__set_bit(EV_ABS, input->evbit);
+
+		input_set_abs_params(input, ABS_MT_TRACKING_ID, 0, 15, 0, 0);
+		input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255, 4, 0);
+		input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 255, 4, 0);
+		input_set_abs_params(input, ABS_MT_ORIENTATION, -32, 31, 1, 0);
+		input_set_abs_params(input, ABS_MT_POSITION_X, -1100, 1358,
+				4, 0);
 		/* Note: Touch Y position from the device is inverted relative
 		 * to how pointer motion is reported (and relative to how USB
 		 * HID recommends the coordinates work).  This driver keeps
 		 * the origin at the same position, and just uses the additive
 		 * inverse of the reported Y.
 		 */
-		set_bit(ABS_MT_POSITION_Y, input->absbit);
-		input->absmin[ABS_MT_POSITION_Y] = -1589;
-		input->absmax[ABS_MT_POSITION_Y] = 2047;
-		input->absfuzz[ABS_MT_POSITION_Y] = 4;
+		input_set_abs_params(input, ABS_MT_POSITION_Y, -1589, 2047,
+				4, 0);
 	}
 
 	if (report_undeciphered) {
-		set_bit(EV_MSC, input->evbit);
-		set_bit(MSC_RAW, input->mscbit);
+		__set_bit(EV_MSC, input->evbit);
+		__set_bit(MSC_RAW, input->mscbit);
 	}
 }
 
@@ -385,7 +363,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 	if (!report) {
 		dev_err(&hdev->dev, "unable to register touch report\n");
 		ret = -ENOMEM;
-		goto err_free;
+		goto err_stop_hw;
 	}
 	report->size = 6;
 
@@ -394,35 +372,37 @@ static int magicmouse_probe(struct hid_device *hdev,
 	if (ret != sizeof(feature_1)) {
 		dev_err(&hdev->dev, "unable to request touch data (1:%d)\n",
 				ret);
-		goto err_free;
+		goto err_stop_hw;
 	}
 	ret = hdev->hid_output_raw_report(hdev, feature_2,
 			sizeof(feature_2), HID_FEATURE_REPORT);
 	if (ret != sizeof(feature_2)) {
 		dev_err(&hdev->dev, "unable to request touch data (2:%d)\n",
 				ret);
-		goto err_free;
+		goto err_stop_hw;
 	}
 
 	input = input_allocate_device();
 	if (!input) {
 		dev_err(&hdev->dev, "can't alloc input device\n");
 		ret = -ENOMEM;
-		goto err_free;
+		goto err_stop_hw;
 	}
 	magicmouse_setup_input(input, hdev);
 
 	ret = input_register_device(input);
 	if (ret) {
 		dev_err(&hdev->dev, "input device registration failed\n");
-		goto err_both;
+		goto err_input;
 	}
 	msc->input = input;
 
 	return 0;
- err_both:
+err_input:
 	input_free_device(input);
- err_free:
+err_stop_hw:
+	hid_hw_stop(hdev);
+err_free:
 	kfree(msc);
 	return ret;
 }
-- 
1.6.5.6

Re: [PATCH] hid-magicmouse: Coding style and probe failure fixes.

[Dmitry Torokhov]

On Thu, Feb 11, 2010 at 12:32:57AM -0500, Michael Poole wrote:
> From 04b395dbbd1ad2b836188e6f125940ae8fac6925 Mon Sep 17 00:00:00 2001
> 
> As suggested by Dmitry Torokhov on 10 Feb: Use proper values
> to initialize bool configuration variables, tabs rather than
> spaces, no braces for one-line else clause, __set_bit() when
> the operation doesn't have to be atomic, input_set_abs_params()
> rather than writing the fields directly, and call hid_hw_stop()
> when appropriate to handle failures in the probe.
> 
> Signed-off-by: Michael Poole <mdpoole-IZmAEv5cUt1AfugRpC6u6w@public.gmane.org>
> ---
> Dmitry and Jiri,
> 
> I haven't had a chance to run-test these changes yet -- hid/for-next
> causes corrupt X display on my laptop, whereas v2.6.33-rc6 and -rc7 are
> fine; I'm still bisecting to figure out the cause -- but this patch is
> not complicated.  (It does compile.)
> 
> I left the buffers for hdev->hid_output_raw_report() on the stack
> because the Bluetooth HIDP code memcpy's the contents into a freshly
> allocated skb.
> 

Ah, OK, I missed the fact that itis BT only device.

The resul looks much better now, thank you for making the changes.

-- 
Dmitry

Re: [PATCH] hid-magicmouse: Coding style and probe failure fixes.

[Jiri Kosina]

On Thu, 11 Feb 2010, Michael Poole wrote:

> I haven't had a chance to run-test these changes yet -- hid/for-next
> causes corrupt X display on my laptop, whereas v2.6.33-rc6 and -rc7 are
> fine; 

That's indeed quite strange. I'd be curious to see the result of the 
bisection, thanks a lot for doing this.

I have applied your patch to the branch containing your driver.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH] hid-magicmouse: Coding style and probe failure fixes.

[Michael Poole]

Jiri Kosina writes:

> On Thu, 11 Feb 2010, Michael Poole wrote:
>
>> I haven't had a chance to run-test these changes yet -- hid/for-next
>> causes corrupt X display on my laptop, whereas v2.6.33-rc6 and -rc7 are
>> fine; 
>
> That's indeed quite strange. I'd be curious to see the result of the 
> bisection, thanks a lot for doing this.

The problem was introduced by 859ddf0 and reverted in 6f14a66 (both only
touching lib/idr.c).  You just happened to merge from master during an
unfortunate window :)

Cherry-picking the revert (6f14a66) on top of commit 093657 (details
shown below for reference) yields a working kernel with a working Magic
Mouse driver.  I expect that merging the rest of master would also work.

commit 09365737da8b3af76abd9d811d435d3ce379903a
Merge: 1b97e37 71b38bd
Author: Jiri Kosina <jkosina@suse.cz>
Date:   Thu Feb 11 11:22:52 2010 +0100

    Merge branch 'apple_magic_mouse' into for-next

Michael Poole

Re: [PATCH 2/2] Add a device driver for the Apple Magic Mouse.

[Jiri Kosina]

On Tue, 9 Feb 2010, Michael Poole wrote:

> >From da3e2b0330fccd584ad79495853e638e9652155b Mon Sep 17 00:00:00 2001
> From: Michael Poole <mdpoole-IZmAEv5cUt1AfugRpC6u6w@public.gmane.org>
> Date: Sat, 6 Feb 2010 12:24:36 -0500
> Subject: Add a device driver for the Apple Magic Mouse.
> 
> Signed-off-by: Michael Poole <mdpoole-IZmAEv5cUt1AfugRpC6u6w@public.gmane.org>
> ---
>  drivers/hid/Kconfig          |   10 +
>  drivers/hid/Makefile         |    1 +
>  drivers/hid/hid-core.c       |    1 +
>  drivers/hid/hid-ids.h        |    1 +
>  drivers/hid/hid-magicmouse.c |  469 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 482 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/hid/hid-magicmouse.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 24d90ea..ba14ec8 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -183,6 +183,16 @@ config LOGIRUMBLEPAD2_FF
>  	  Say Y here if you want to enable force feedback support for Logitech
>  	  Rumblepad 2 devices.
>  
> +config HID_MAGICMOUSE
> +	tristate "Apple" if EMBEDDED

Could you please make this something else than "Apple"? This string is 
already used by HID_APPLE.

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 2/2] Add a device driver for the Apple Magic Mouse.

[Jiri Kosina]

On Wed, 10 Feb 2010, Jiri Kosina wrote:

> > +config HID_MAGICMOUSE
> > +	tristate "Apple" if EMBEDDED
> 
> Could you please make this something else than "Apple"? This string is 
> already used by HID_APPLE.

I have fixed this up on my side, no need for you to respin.

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH 2/2] Add a device driver for the Apple Magic Mouse.

[Ed Tomlinson]

Hi,

When I apply this to head, I get:

drivers/hid/hid-magicmouse.c: In function 'magicmouse_probe':                                                         
drivers/hid/hid-magicmouse.c:393: error: too many arguments to function 'hdev->hid_output_raw_report'                 
drivers/hid/hid-magicmouse.c:400: error: too many arguments to function 'hdev->hid_output_raw_report'                 

I only need the second patch - the first seems to be in tree.

Any ideas as to what I am missing?

TIA
Ed

On Tuesday 09 February 2010 08:13:08 Michael Poole wrote:
> From da3e2b0330fccd584ad79495853e638e9652155b Mon Sep 17 00:00:00 2001
> From: Michael Poole <mdpoole-IZmAEv5cUt1AfugRpC6u6w@public.gmane.org>
> Date: Sat, 6 Feb 2010 12:24:36 -0500
> Subject: Add a device driver for the Apple Magic Mouse.
> 
> Signed-off-by: Michael Poole <mdpoole-IZmAEv5cUt1AfugRpC6u6w@public.gmane.org>
> ---
>  drivers/hid/Kconfig          |   10 +
>  drivers/hid/Makefile         |    1 +
>  drivers/hid/hid-core.c       |    1 +
>  drivers/hid/hid-ids.h        |    1 +
>  drivers/hid/hid-magicmouse.c |  469 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 482 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/hid/hid-magicmouse.c
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 24d90ea..ba14ec8 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -183,6 +183,16 @@ config LOGIRUMBLEPAD2_FF
>  	  Say Y here if you want to enable force feedback support for Logitech
>  	  Rumblepad 2 devices.
>  
> +config HID_MAGICMOUSE
> +	tristate "Apple" if EMBEDDED
> +	depends on BT_HIDP
> +	default !EMBEDDED
> +	---help---
> +	Support for the Apple Magic Mouse.
> +
> +	Say Y here if you want support for the multi-touch features of the
> +	Apple Wireless "Magic" Mouse.
> +
>  config HID_MICROSOFT
>  	tristate "Microsoft" if EMBEDDED
>  	depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 0de2dff..45d81e9 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_HID_GYRATION)	+= hid-gyration.o
>  obj-$(CONFIG_HID_KENSINGTON)	+= hid-kensington.o
>  obj-$(CONFIG_HID_KYE)		+= hid-kye.o
>  obj-$(CONFIG_HID_LOGITECH)	+= hid-logitech.o
> +obj-$(CONFIG_HID_MAGICMOUSE)    += hid-magicmouse.o
>  obj-$(CONFIG_HID_MICROSOFT)	+= hid-microsoft.o
>  obj-$(CONFIG_HID_MONTEREY)	+= hid-monterey.o
>  obj-$(CONFIG_HID_NTRIG)		+= hid-ntrig.o
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index e39055b..f23ca76 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1254,6 +1254,7 @@ static const struct hid_device_id hid_blacklist[] = {
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ATV_IRCONTROL) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_IRCONTROL4) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MIGHTYMOUSE) },
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICMOUSE) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_ISO) },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER_ANSI) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 010368e..11e5218 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -56,6 +56,7 @@
>  
>  #define USB_VENDOR_ID_APPLE		0x05ac
>  #define USB_DEVICE_ID_APPLE_MIGHTYMOUSE	0x0304
> +#define USB_DEVICE_ID_APPLE_MAGICMOUSE	0x030d
>  #define USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI	0x020e
>  #define USB_DEVICE_ID_APPLE_FOUNTAIN_ISO	0x020f
>  #define USB_DEVICE_ID_APPLE_GEYSER_ANSI	0x0214
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> new file mode 100644
> index 0000000..f94b3e4
> --- /dev/null
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -0,0 +1,469 @@
> +/*
> + *   Apple "Magic" Wireless Mouse driver
> + *
> + *   Copyright (c) 2010 Michael Poole <mdpoole-IZmAEv5cUt1AfugRpC6u6w@public.gmane.org>
> + */
> +
> +/*
> + * 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 (at your option)
> + * any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/usb.h>
> +
> +#include "hid-ids.h"
> +
> +static bool emulate_3button = 1;
> +module_param(emulate_3button, bool, 0644);
> +MODULE_PARM_DESC(emulate_3button, "Emulate a middle button");
> +
> +static int middle_button_start = -350;
> +static int middle_button_stop = +350;
> +
> +static bool emulate_scroll_wheel = 1;
> +module_param(emulate_scroll_wheel, bool, 0644);
> +MODULE_PARM_DESC(emulate_scroll_wheel, "Emulate a scroll wheel");
> +
> +static bool report_touches = 1;
> +module_param(report_touches, bool, 0644);
> +MODULE_PARM_DESC(report_touches, "Emit touch records (otherwise, only use them for emulation)");
> +
> +static bool report_undeciphered = 0;
> +module_param(report_undeciphered, bool, 0644);
> +MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state field using a MSC_RAW event");
> +
> +#define TOUCH_REPORT_ID   0x29
> +/* These definitions are not precise, but they're close enough.  (Bits
> + * 0x03 seem to indicate the aspect ratio of the touch, bits 0x70 seem
> + * to be some kind of bit mask -- 0x20 may be a near-field reading,
> + * and 0x40 is actual contact, and 0x10 may be a start/stop or change
> + * indication.)
> + */
> +#define TOUCH_STATE_MASK  0xf0
> +#define TOUCH_STATE_NONE  0x00
> +#define TOUCH_STATE_START 0x30
> +#define TOUCH_STATE_DRAG  0x40
> +
> +/**
> + * struct magicmouse_sc - Tracks Magic Mouse-specific data.
> + * @input: Input device through which we report events.
> + * @quirks: Currently unused.
> + * @last_timestamp: Timestamp from most recent (18-bit) touch report
> + *     (units of milliseconds over short windows, but seems to
> + *     increase faster when there are no touches).
> + * @delta_time: 18-bit difference between the two most recent touch
> + *     reports from the mouse.
> + * @ntouches: Number of touches in most recent touch report.
> + * @scroll_accel: Number of consecutive scroll motions.
> + * @scroll_jiffies: Time of last scroll motion.
> + * @touches: Most recent data for a touch, indexed by tracking ID.
> + * @tracking_ids: Mapping of current touch input data to @touches.
> + */
> +struct magicmouse_sc {
> +	struct input_dev *input;
> +	unsigned long quirks;
> +
> +	int last_timestamp;
> +	int delta_time;
> +	int ntouches;
> +	int scroll_accel;
> +	unsigned long scroll_jiffies;
> +
> +	struct {
> +		short x;
> +		short y;
> +		short scroll_y;
> +		u8 size;
> +	} touches[16];
> +	int tracking_ids[16];
> +};
> +
> +static int magicmouse_firm_touch(struct magicmouse_sc *msc)
> +{
> +	int touch = -1;
> +	int ii;
> +
> +	/* If there is only one "firm" touch, set touch to its
> +	 * tracking ID.
> +	 */
> +	for (ii = 0; ii < msc->ntouches; ii++) {
> +		int idx = msc->tracking_ids[ii];
> +		if (msc->touches[idx].size < 8) {
> +			/* Ignore this touch. */
> +		} else if (touch >= 0) {
> +			touch = -1;
> +			break;
> +		} else {
> +			touch = idx;
> +		}
> +	}
> +
> +	return touch;
> +}
> +
> +static void magicmouse_emit_buttons(struct magicmouse_sc *msc, int state)
> +{
> +        int last_state = test_bit(BTN_LEFT, msc->input->key) << 0 |
> +                test_bit(BTN_RIGHT, msc->input->key) << 1 |
> +                test_bit(BTN_MIDDLE, msc->input->key) << 2;
> +
> +	if (emulate_3button) {
> +		int id;
> +
> +		/* If some button was pressed before, keep it held
> +		 * down.  Otherwise, if there's exactly one firm
> +		 * touch, use that to override the mouse's guess.
> +		 */
> +		if (state == 0) {
> +			/* The button was released. */
> +		} else if (last_state != 0) {
> +			state = last_state;
> +		} else if ((id = magicmouse_firm_touch(msc)) >= 0) {
> +			int x = msc->touches[id].x;
> +			if (x < middle_button_start)
> +				state = 1;
> +			else if (x > middle_button_stop)
> +				state = 2;
> +			else
> +				state = 4;
> +		} /* else: we keep the mouse's guess */
> +
> +		input_report_key(msc->input, BTN_MIDDLE, state & 4);
> +	}
> +
> +	input_report_key(msc->input, BTN_LEFT, state & 1);
> +	input_report_key(msc->input, BTN_RIGHT, state & 2);
> +
> +	if (state != last_state)
> +		msc->scroll_accel = 0;
> +}
> +
> +static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tdata)
> +{
> +	struct input_dev *input = msc->input;
> +	__s32 x_y = tdata[0] << 8 | tdata[1] << 16 | tdata[2] << 24;
> +	int misc = tdata[5] | tdata[6] << 8;
> +	int id = (misc >> 6) & 15;
> +	int x = x_y << 12 >> 20;
> +	int y = -(x_y >> 20);
> +
> +	/* Store tracking ID and other fields. */
> +	msc->tracking_ids[raw_id] = id;
> +	msc->touches[id].x = x;
> +	msc->touches[id].y = y;
> +	msc->touches[id].size = misc & 63;
> +
> +	/* If requested, emulate a scroll wheel by detecting small
> +	 * vertical touch motions along the middle of the mouse.
> +	 */
> +	if (emulate_scroll_wheel &&
> +	    middle_button_start < x && x < middle_button_stop) {
> +		static const int accel_profile[] = {
> +			256, 228, 192, 160, 128, 96, 64, 32,
> +		};
> +		unsigned long now = jiffies;
> +		int step = msc->touches[id].scroll_y - y;
> +
> +		/* Reset acceleration after half a second. */
> +		if (time_after(now, msc->scroll_jiffies + HZ / 2))
> +			msc->scroll_accel = 0;
> +
> +		/* Calculate and apply the scroll motion. */
> +		switch (tdata[7] & TOUCH_STATE_MASK) {
> +		case TOUCH_STATE_START:
> +			msc->touches[id].scroll_y = y;
> +                        msc->scroll_accel = min_t(int, msc->scroll_accel + 1,
> +						ARRAY_SIZE(accel_profile) - 1);
> +			break;
> +		case TOUCH_STATE_DRAG:
> +			step = step / accel_profile[msc->scroll_accel];
> +			if (step != 0) {
> +				msc->touches[id].scroll_y = y;
> +				msc->scroll_jiffies = now;
> +				input_report_rel(input, REL_WHEEL, step);
> +			}
> +			break;
> +		}
> +	}
> +
> +	/* Generate the input events for this touch. */
> +	if (report_touches) {
> +                int orientation = (misc >> 10) - 32;
> +
> +		input_report_abs(input, ABS_MT_TRACKING_ID, id);
> +		input_report_abs(input, ABS_MT_TOUCH_MAJOR, tdata[3]);
> +		input_report_abs(input, ABS_MT_TOUCH_MINOR, tdata[4]);
> +		input_report_abs(input, ABS_MT_ORIENTATION, orientation);
> +		input_report_abs(input, ABS_MT_POSITION_X, x);
> +		input_report_abs(input, ABS_MT_POSITION_Y, y);
> +
> +		if (report_undeciphered) {
> +			input_event(input, EV_MSC, MSC_RAW, tdata[7]);
> +		}
> +
> +		input_mt_sync(input);
> +	}
> +}
> +
> +static int magicmouse_raw_event(struct hid_device *hdev,
> +		struct hid_report *report, u8 *data, int size)
> +{
> +	struct magicmouse_sc *msc = hid_get_drvdata(hdev);
> +	struct input_dev *input = msc->input;
> +	int x, y, ts, ii, clicks;
> +
> +	switch (data[0]) {
> +	case 0x10:
> +		if (size != 6)
> +			return 0;
> +		x = (__s16)(data[2] | data[3] << 8);
> +		y = (__s16)(data[4] | data[5] << 8);
> +		clicks = data[1];
> +		break;
> +	case TOUCH_REPORT_ID:
> +		/* Expect six bytes of prefix, and N*8 bytes of touch data. */
> +		if (size < 6 || ((size - 6) % 8) != 0)
> +			return 0;
> +		ts = data[3] >> 6 | data[4] << 2 | data[5] << 10;
> +		msc->delta_time = (ts - msc->last_timestamp) & 0x3ffff;
> +		msc->last_timestamp = ts;
> +		msc->ntouches = (size - 6) / 8;
> +		for (ii = 0; ii < msc->ntouches; ii++)
> +			magicmouse_emit_touch(msc, ii, data + ii * 8 + 6);
> +		/* When emulating three-button mode, it is important
> +		 * to have the current touch information before
> +		 * generating a click event.
> +		 */
> +		x = (signed char)data[1];
> +		y = (signed char)data[2];
> +		clicks = data[3];
> +		break;
> +	case 0x20: /* Theoretically battery status (0-100), but I have
> +		    * never seen it -- maybe it is only upon request.
> +		    */
> +	case 0x60: /* Unknown, maybe laser on/off. */
> +	case 0x61: /* Laser reflection status change.
> +		    * data[1]: 0 = spotted, 1 = lost
> +		    */
> +	default:
> +		return 0;
> +	}
> +
> +	magicmouse_emit_buttons(msc, clicks & 3);
> +	input_report_rel(input, REL_X, x);
> +	input_report_rel(input, REL_Y, y);
> +	input_sync(input);
> +	return 1;
> +}
> +
> +static int magicmouse_input_open(struct input_dev *dev)
> +{
> +	struct hid_device *hid = input_get_drvdata(dev);
> +
> +	return hid->ll_driver->open(hid);
> +}
> +
> +static void magicmouse_input_close(struct input_dev *dev)
> +{
> +	struct hid_device *hid = input_get_drvdata(dev);
> +
> +	hid->ll_driver->close(hid);
> +}
> +
> +static void magicmouse_setup_input(struct input_dev *input, struct hid_device *hdev)
> +{
> +	input_set_drvdata(input, hdev);
> +	input->event = hdev->ll_driver->hidinput_input_event;
> +	input->open = magicmouse_input_open;
> +	input->close = magicmouse_input_close;
> +
> +	input->name = hdev->name;
> +	input->phys = hdev->phys;
> +	input->uniq = hdev->uniq;
> +	input->id.bustype = hdev->bus;
> +	input->id.vendor = hdev->vendor;
> +	input->id.product = hdev->product;
> +	input->id.version = hdev->version;
> +	input->dev.parent = hdev->dev.parent;
> +
> +	set_bit(EV_KEY, input->evbit);
> +	set_bit(BTN_LEFT, input->keybit);
> +	set_bit(BTN_RIGHT, input->keybit);
> +	if (emulate_3button)
> +		set_bit(BTN_MIDDLE, input->keybit);
> +	set_bit(BTN_TOOL_FINGER, input->keybit);
> +
> +	set_bit(EV_REL, input->evbit);
> +	set_bit(REL_X, input->relbit);
> +	set_bit(REL_Y, input->relbit);
> +	if (emulate_scroll_wheel)
> +		set_bit(REL_WHEEL, input->relbit);
> +
> +	if (report_touches) {
> +		set_bit(EV_ABS, input->evbit);
> +
> +		set_bit(ABS_MT_TRACKING_ID, input->absbit);
> +		input->absmin[ABS_MT_TRACKING_ID] = 0;
> +		input->absmax[ABS_MT_TRACKING_ID] = 15;
> +		input->absfuzz[ABS_MT_TRACKING_ID] = 0;
> +
> +		set_bit(ABS_MT_TOUCH_MAJOR, input->absbit);
> +		input->absmin[ABS_MT_TOUCH_MAJOR] = 0;
> +		input->absmax[ABS_MT_TOUCH_MAJOR] = 255;
> +		input->absfuzz[ABS_MT_TOUCH_MAJOR] = 4;
> +
> +		set_bit(ABS_MT_TOUCH_MINOR, input->absbit);
> +		input->absmin[ABS_MT_TOUCH_MINOR] = 0;
> +		input->absmax[ABS_MT_TOUCH_MINOR] = 255;
> +		input->absfuzz[ABS_MT_TOUCH_MINOR] = 4;
> +
> +		set_bit(ABS_MT_ORIENTATION, input->absbit);
> +		input->absmin[ABS_MT_ORIENTATION] = -32;
> +		input->absmax[ABS_MT_ORIENTATION] = 31;
> +		input->absfuzz[ABS_MT_ORIENTATION] = 1;
> +
> +		set_bit(ABS_MT_POSITION_X, input->absbit);
> +		input->absmin[ABS_MT_POSITION_X] = -1100;
> +		input->absmax[ABS_MT_POSITION_X] = 1358;
> +		input->absfuzz[ABS_MT_POSITION_X] = 4;
> +
> +		/* Note: Touch Y position from the device is inverted relative
> +		 * to how pointer motion is reported (and relative to how USB
> +		 * HID recommends the coordinates work).  This driver keeps
> +		 * the origin at the same position, and just uses the additive
> +		 * inverse of the reported Y.
> +		 */
> +		set_bit(ABS_MT_POSITION_Y, input->absbit);
> +		input->absmin[ABS_MT_POSITION_Y] = -1589;
> +		input->absmax[ABS_MT_POSITION_Y] = 2047;
> +		input->absfuzz[ABS_MT_POSITION_Y] = 4;
> +	}
> +
> +	if (report_undeciphered) {
> +		set_bit(EV_MSC, input->evbit);
> +		set_bit(MSC_RAW, input->mscbit);
> +	}
> +}
> +
> +static int magicmouse_probe(struct hid_device *hdev,
> +	const struct hid_device_id *id)
> +{
> +	__u8 feature_1[] = { 0xd7, 0x01 };
> +	__u8 feature_2[] = { 0xf8, 0x01, 0x32 };
> +	struct input_dev *input;
> +	struct magicmouse_sc *msc;
> +	struct hid_report *report;
> +	int ret;
> +
> +	msc = kzalloc(sizeof(*msc), GFP_KERNEL);
> +	if (msc == NULL) {
> +		dev_err(&hdev->dev, "can't alloc magicmouse descriptor\n");
> +		return -ENOMEM;
> +	}
> +
> +	msc->quirks = id->driver_data;
> +	hid_set_drvdata(hdev, msc);
> +
> +	ret = hid_parse(hdev);
> +	if (ret) {
> +		dev_err(&hdev->dev, "magicmouse hid parse failed\n");
> +		goto err_free;
> +	}
> +
> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> +	if (ret) {
> +		dev_err(&hdev->dev, "magicmouse hw start failed\n");
> +		goto err_free;
> +	}
> +
> +	report = hid_register_report(hdev, HID_INPUT_REPORT, TOUCH_REPORT_ID);
> +	if (!report) {
> +		dev_err(&hdev->dev, "unable to register touch report\n");
> +		ret = -ENOMEM;
> +		goto err_free;
> +	}
> +	report->size = 6;
> +
> +	ret = hdev->hid_output_raw_report(hdev, feature_1, sizeof(feature_1),
> +			HID_FEATURE_REPORT);
> +	if (ret != sizeof(feature_1)) {
> +		dev_err(&hdev->dev, "unable to request touch data (1:%d)\n",
> +				ret);
> +		goto err_free;
> +	}
> +	ret = hdev->hid_output_raw_report(hdev, feature_2,
> +			sizeof(feature_2), HID_FEATURE_REPORT);
> +	if (ret != sizeof(feature_2)) {
> +		dev_err(&hdev->dev, "unable to request touch data (2:%d)\n",
> +				ret);
> +		goto err_free;
> +	}
> +
> +	input = input_allocate_device();
> +	if (!input) {
> +		dev_err(&hdev->dev, "can't alloc input device\n");
> +		ret = -ENOMEM;
> +		goto err_free;
> +	}
> +	magicmouse_setup_input(input, hdev);
> +
> +	ret = input_register_device(input);
> +	if (ret) {
> +		dev_err(&hdev->dev, "input device registration failed\n");
> +		goto err_both;
> +	}
> +	msc->input = input;
> +
> +	return 0;
> + err_both:
> +	input_free_device(input);
> + err_free:
> +	kfree(msc);
> +	return ret;
> +}
> +
> +static void magicmouse_remove(struct hid_device *hdev)
> +{
> +	hid_hw_stop(hdev);
> +	kfree(hid_get_drvdata(hdev));
> +}
> +
> +static const struct hid_device_id magic_mice[] = {
> +	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICMOUSE),
> +		.driver_data = 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(hid, magic_mice);
> +
> +static struct hid_driver magicmouse_driver = {
> +	.name = "magicmouse",
> +	.id_table = magic_mice,
> +	.probe = magicmouse_probe,
> +	.remove = magicmouse_remove,
> +	.raw_event = magicmouse_raw_event,
> +};
> +
> +static int __init magicmouse_init(void)
> +{
> +	int ret;
> +
> +	ret = hid_register_driver(&magicmouse_driver);
> +	if (ret)
> +		printk(KERN_ERR "can't register magicmouse driver\n");
> +
> +	return ret;
> +}
> +
> +static void __exit magicmouse_exit(void)
> +{
> +	hid_unregister_driver(&magicmouse_driver);
> +}
> +
> +module_init(magicmouse_init);
> +module_exit(magicmouse_exit);
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH 2/2] Add a device driver for the Apple Magic Mouse.

[Michael Poole]

Ed Tomlinson writes:

> Hi,
>
> When I apply this to head, I get:
>
> drivers/hid/hid-magicmouse.c: In function 'magicmouse_probe':                                                         
> drivers/hid/hid-magicmouse.c:393: error: too many arguments to function 'hdev->hid_output_raw_report'                 
> drivers/hid/hid-magicmouse.c:400: error: too many arguments to function 'hdev->hid_output_raw_report'                 
>
> I only need the second patch - the first seems to be in tree.
>
> Any ideas as to what I am missing?
>
> TIA
> Ed

You're missing the patch that makes hid_output_raw_report() more generic.
It is in the HID tree[1] as commit d4bfa033.

[1]- http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git

Michael Poole

Re: [PATCH 2/2] Add a device driver for the Apple Magic Mouse (2.6.32.8)

[Ed Tomlinson]

On Wednesday 10 February 2010 22:20:13 Michael Poole wrote:
> Ed Tomlinson writes:
> 
> > Hi,
> >
> > When I apply this to head, I get:
> >
> > drivers/hid/hid-magicmouse.c: In function 'magicmouse_probe':                                                         
> > drivers/hid/hid-magicmouse.c:393: error: too many arguments to function 'hdev->hid_output_raw_report'                 
> > drivers/hid/hid-magicmouse.c:400: error: too many arguments to function 'hdev->hid_output_raw_report'                 
> >
> > I only need the second patch - the first seems to be in tree.
> >
> > Any ideas as to what I am missing?
> >
> > TIA
> > Ed
> 
> You're missing the patch that makes hid_output_raw_report() more generic.
> It is in the HID tree[1] as commit d4bfa033.
> 
> [1]- http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git

Ok.  With this I have a patchset that applies to 2.6.32.8.  If anyone wants it
just email.  

Of course, Michael's cleanups came just after I booted with the new kernel.  I'll
apply it and update & post my changes that allow both x and y scrolling and almost 
work for a few multitouch gestures.

Ed