Thread (13 messages) 13 messages, 5 authors, 2010-02-16

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

From: Ed Tomlinson <hidden>
Date: 2010-02-15 12:45:01
Also in: linux-bluetooth, lkml
Subsystem: hid core layer, the rest · Maintainers: Jiri Kosina, Benjamin Tissoires, Linus Torvalds

On Monday 15 February 2010 02:11:46 Dmitry Torokhov wrote:
On Sun, Feb 14, 2010 at 09:22:41AM -0500, Ed Tomlinson wrote:
quoted
On Sunday 14 February 2010 03:03:44 Dmitry Torokhov wrote:
quoted
On Sat, Feb 13, 2010 at 02:29:29PM -0500, Ed Tomlinson wrote:
quoted
On Wednesday 10 February 2010 08:57:37 Jiri Kosina wrote:
quoted
On Tue, 9 Feb 2010, Michael Poole wrote:
quoted
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.
quoted
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);
 }
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help