Bug 13778 - Macintosh mouse button emulation sending both mousebutton and keypress events
Summary: Macintosh mouse button emulation sending both mousebutton and keypress events
Status: CLOSED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Input Devices (show other bugs)
Hardware: All Linux
: P1 enhancement
Assignee: Dmitry Torokhov
URL: https://bugzilla.redhat.com/show_bug....
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-15 14:54 UTC by Olivier Mehani
Modified: 2010-05-24 20:46 UTC (History)
3 users (show)

See Also:
Kernel Version: 2.6.29
Tree: Mainline
Regression: No


Attachments
Add a call to mac_hid_mouse_emulate_buttons() in evdev_event() (2.11 KB, patch)
2009-08-18 21:46 UTC, Olivier Mehani
Details | Diff
Implement input filters (5.12 KB, patch)
2009-09-04 06:47 UTC, Dmitry Torokhov
Details | Diff
Implement mac button emulation as input filter (9.74 KB, patch)
2009-09-04 06:48 UTC, Dmitry Torokhov
Details | Diff
Updated version that properly sets up 'strategy' on sysctl entry (9.87 KB, patch)
2009-09-23 04:13 UTC, Dmitry Torokhov
Details | Diff
Quick 'n' Dirty printk-based debug patch (1.13 KB, patch)
2009-12-26 21:30 UTC, Olivier Mehani
Details | Diff
Kernel log generated with the above patch when using the koyboard (1.00 KB, text/plain)
2009-12-26 21:33 UTC, Olivier Mehani
Details
Implement input filters adding them to the head of dev->h_list in input_register_handle() as is done for handler->h_list (5.45 KB, patch)
2009-12-29 23:55 UTC, Olivier Mehani
Details | Diff

Description Olivier Mehani 2009-07-15 14:54:44 UTC
On Macintosh computers with only one mouse button (e.g. iBook laptops), activating the CONFIG_MAC_EMUMOUSEBTN kernel option allows to associate second and third button events to Fn+Key. This functionnality can be activated and deactivated at runtime. When activated, it creates a new input devices which is recognized as a mouse and only send button 2 and 3 events.

The problem is that, when activated, using one of the key sequences associated to mouse button presses, the kernel generates two events: 
 - the mouse click event as desired and,
 - the normal keypress event.

This causes problems such as the following example: using X.org and an xterm, with keys Fn+LCtrl and Fn+LSuper respectively associated to Buttons 2 and 3. While trying to paste something from the buffer (using button 1), the xterm gets both a Button 2 press event, and a LCtrl event. This in turns triggers another behavior than pasting the content of the buffer in xterm (configuration menu).

The expected behavior would be that when mouse button emulation is activated (dev.mac_hid.mouse_button_emulation = 1), no keypress event is generated on the associated keyboard device when key dev.mac_hid.mouse_button{2,3}_keycode is pressed in combination with the Fn key).

Other reports of this problem (initially diagnozed when Xorg changed its input device grabbing policy) can be found in the RedHat Bugzilla, as given in the URL attribute of this ticket.
Comment 1 Andrew Morton 2009-07-15 20:21:33 UTC
Reassigned to drivers/input.
Comment 2 Olivier Mehani 2009-08-18 19:50:44 UTC
Ok, I think I've narrowed the problem a bit.

The mouse emulation function is called from drivers/char/keyboard.c, in function kbd_keycode(). The event is passed to the mac_hid_mouse_emulate_buttons(), in drivers/input/mac_hid.c, which generates the mouse event and returns 1 if so, or 0 otherwise. Based on this return value, kbd_keycode() stop processing the even further if a mouse button even has been generated. This is expected behavior.

The problem is due to the fact that Xorg now uses evdev. There are thus two handlers connected to the keyboard device (kbd and evdev). The latter doesn't have the same piece of code to handle mouse button emulation and generates the keyboard events regardless.

I tried a quick patch to drivers/input/evdev.c, in function evdev_event(), to catch the relevant keycode in the same way kbd_keycode() does (cut'n paste baby!). I just tried it on my June 2005 iBook G4, and it seems to work as expected.

I suspect fixing the bug properly may take a bit more than that such as
 - testing with other Macs (I understand the same generation of PowerBooks have their keyboard on the USB port instead of ADB);
 - testing with additional keyboards or input devices connected instead of just the ADB keyboard;
 - tightening the condition on the event as all types of inputs, not only keyboard-related, go through evdev_event();
 - maybe also removing the code from kbd_keycode() which may no longer be needed (I have absolutely no idea about that, but it's worth investigating).
Comment 3 Olivier Mehani 2009-08-18 21:46:16 UTC
Created attachment 22765 [details]
Add a call to mac_hid_mouse_emulate_buttons() in evdev_event()

This is a quick fix. It works for me so far (about an hour). Please refer my previous comment for improvement.

The patch also removes some (seemingly) useless code in mac_hid.c (a variable which was never used, and two consecutive ifs checking the same thing).

Somebody with more experience of the input subsystem should check that this patch is correct.
Comment 4 Dmitry Torokhov 2009-09-04 06:46:07 UTC
No, we should not be breeding the horrors from keyboard.c... How about the following 2 patches instead?
Comment 5 Dmitry Torokhov 2009-09-04 06:47:38 UTC
Created attachment 23008 [details]
Implement input filters
Comment 6 Dmitry Torokhov 2009-09-04 06:48:43 UTC
Created attachment 23009 [details]
Implement mac button emulation as input filter
Comment 7 Olivier Mehani 2009-09-22 12:22:06 UTC
I just tried that patch on a Linux 2.6.30 ppc(32) with the Gentoo r4 patchset. It applies and compiles properly, but I have the following error on boot.

sysctl table check failed: /dev/mac_hid/mouse_button_emulation .7.5.3 Missing strategy
Call Trace:
[ef84dd70] [c0008c04] show_stack+0x4c/0x14c (unreliable)
[ef84ddb0] [c004c17c] set_fail+0x50/0x68
[ef84ddd0] [c004c854] sysctl_check_table+0x6c0/0x714
[ef84de40] [c004c868] sysctl_check_table+0x6d4/0x714
[ef84deb0] [c004c868] sysctl_check_table+0x6d4/0x714
[ef84df20] [c0037de4] __register_sysctl_paths+0x100/0x2c8
[ef84df60] [c05a44c4] mac_hid_init+0x18/0x40
[ef84df70] [c0003cc8] do_one_initcall+0x58/0x1ac
[ef84dfe0] [c058a1a4] kernel_init+0x84/0xf4
[ef84dff0] [c0014af0] kernel_thread+0x4c/0x68


I haven't tested it further yet. Soon to come.
Comment 8 Dmitry Torokhov 2009-09-22 16:31:07 UTC
Umm, it's missing

.strategy = sysctl_data,

In the part that uses mac_hid_toggle_emumouse as a proc_handler.
Comment 9 Dmitry Torokhov 2009-09-23 04:13:21 UTC
Created attachment 23145 [details]
Updated version that properly sets up 'strategy' on sysctl entry
Comment 10 Olivier Mehani 2009-09-24 22:52:10 UTC
Ok, the error message doesn't appear with the updated version.

Unfortunately, the filtering still doesn't seem to be working as expected in my setup. Keypresses generating middle and (resp.) right buttons still generated, as seen in xev, a Control_R and (resp.) Alt_R in addition to mouse clicks.

This is confirmed on a plain console (not an X term):
$ sudo sysctl -a | grep mac_hid
dev.mac_hid.mouse_button_emulation = 1
dev.mac_hid.mouse_button2_keycode = 97
dev.mac_hid.mouse_button3_keycode = 100
$ showkey
kb mode was UNICODE
[ if you are trying this under X, it might not work
since the X server is also reading /dev/console ]

press any key (program terminates 10s after last keypress)...
keycode  28 release
keycode 464 press
keycode 100 press
keycode 100 release
keycode  97 press
keycode  97 release
keycode 464 release
$

I notice, however, that X detects two KEYBOARD inputs:
$ grep XINPUT /var/log/Xorg.0.log
(II) XINPUT: Adding extended input device "Macintosh mouse button emulation" (type: MOUSE)
(II) XINPUT: Adding extended input device "ADB mouse" (type: MOUSE)
(II) XINPUT: Adding extended input device "ADB Powerbook buttons" (type: KEYBOARD)
(II) XINPUT: Adding extended input device "ADB keyboard" (type: KEYBOARD)

Could the "ADB Powerbook buttons" be generating letting the events without proper filtering? I've had a read at the patch, but couldn't quite work out how a filter was attached to either one or all the devices of a given type. As far as I could tell, however, it seems to be filtering all KEYBOARD-type inputs?
Comment 11 Olivier Mehani 2009-12-26 21:29:36 UTC
Hello again,

To try to explore my problem with this patch further, I quickly added a couple of printk in the relevant functions to investigate the behavior I'm seeing (see next attachment).

In the output (see next attachments), one can see that the mouse events are both received as an unfiltered event, then as a filtered one, whereas processing should have ceased.

This seems to hint that the handler for a filter is processed twice, first as a normal event, THEN as a filter. I can't quite figure out what is causing that in the code of input.c:input_pass_event(), so I suspect the function may be called twice...
Comment 12 Olivier Mehani 2009-12-26 21:30:25 UTC
Created attachment 24313 [details]
Quick 'n' Dirty printk-based debug patch
Comment 13 Olivier Mehani 2009-12-26 21:33:57 UTC
Created attachment 24314 [details]
Kernel log generated with the above patch when using the koyboard

Normal keypresses generate two unfiltered events: one with the code of the pressed key, and one with keycode 0 which I understand to be a key release. Keypresses which should be interpreted as mouse clicks first generate normal unfiltered events, then the handler is executed as a filter, and the mouse click is generated, before the keypress.
Comment 14 Olivier Mehani 2009-12-29 23:21:43 UTC
I think I found my problem, but  still can't explain it. In /proc/bus/input/devices, I have the following entry.

I: Bus=0017 Vendor=0001 Product=22c4 Version=0200
N: Name="ADB keyboard"
P: Phys=adb2:2.c4/input
S: Sysfs=/devices/virtual/input/input0
U: Uniq=
H: Handlers=kbd event0 mac-btn-emul 
B: EV=120003
B: KEY=10000 0 0 0 0 0 0 0 0 0 0 feb0ffdf 3cfffff ffffffff fffffffe
B: LED=7

The mac-btn-emul filter is last in the "H" list.

The function which generates this list, input.c:input_devices_seq_show() uses 'list_for_each_entry(handle, &dev->h_list, d_node)' to iterate the handler list.

The event function input.c:input_pass_event() uses a slightly different method to iterate the handlers to call: 'list_for_each_entry_rcu(handle, &dev->h_list, d_node)'. Both iteration functions are however quite similar.

This means that the kbd handler will be called before the mac-btn-emul filter, which is coherent with my earlier tests.

Additionaly, /proc/bus/input/handlers confirms 'mac-btn-emul' is properly registered as a filter:
N: Number=0 Name=kbd
N: Number=1 Name=mousedev Minor=32
N: Number=2 Name=evdev Minor=64
N: Number=3 Name=rfkill
N: Number=4 Name=mac-btn-emul(filter)

I don't understand, however, why the filter would find itself at the end of the list as input.c:input_register_handle() properly adds new filters to the head of the list while handlers are added to the tail with ' list_add[_tail](&handle->h_node, &handler->h_list)'.

Looking a bit further, it seems that this head/tail logic is not replicated when registering handleRs. They invariably are added to the end of input_handler_list (in input.c:input_register_handler()) from which they are attached in order of appearance in input.c:input_register_device().

For reference, I'm running these tests on an old G4, but I doubt endianness problems are involved.
Comment 15 Olivier Mehani 2009-12-29 23:55:01 UTC
Created attachment 24370 [details]
Implement input filters adding them to the head of dev->h_list in input_register_handle() as is done for handler->h_list

In input.c:input_register_handle(), handlers were added to the head or tail of the h_list of the handler depending on their type. The same is done when adding the handler to dev->h_list for filters to be at the head of each device's handler list. Actually, this may be the only thingh that needs to be done (i.e. with no conditionnal insertion in handler->h_list).

This patch obsoletes the first proposed implementation of input filters.

With this applied, the filters are inserted at the head of the handler list for each relevant device (as confirmed by /proc/bus/input/devices), the keypresses events related to the mouse emulation are filtered out as expected (as confirmed by xev) and both the keyboard, mouse and button emulation work properly under X.

After further confirmation from third parties, I think this bug can be closed and the patches integrated.
Comment 16 Dmitry Torokhov 2009-12-30 03:01:23 UTC
Right, we just need to make sure the "filter" goes to the head of the list belonging to the device, and leave the handler's list as is. I had fixed patch in my queue for some time, sorry for not updating the bug.
Comment 17 Dmitry Torokhov 2010-05-24 20:46:12 UTC
The patches are upstream so closing.

Note You need to log in before you can comment on or make changes to this bug.