Bug 13778
Summary: | Macintosh mouse button emulation sending both mousebutton and keypress events | ||
---|---|---|---|
Product: | Drivers | Reporter: | Olivier Mehani (shtrom-linux) |
Component: | Input Devices | Assignee: | Dmitry Torokhov (dmitry.torokhov) |
Status: | CLOSED CODE_FIX | ||
Severity: | enhancement | CC: | akpm, dmitry.torokhov, peter.hutterer |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | https://bugzilla.redhat.com/show_bug.cgi?id=459018#c5 | ||
Kernel Version: | 2.6.29 | Subsystem: | |
Regression: | No | Bisected commit-id: | |
Attachments: |
Add a call to mac_hid_mouse_emulate_buttons() in evdev_event()
Implement input filters Implement mac button emulation as input filter Updated version that properly sets up 'strategy' on sysctl entry Quick 'n' Dirty printk-based debug patch Kernel log generated with the above patch when using the koyboard Implement input filters adding them to the head of dev->h_list in input_register_handle() as is done for handler->h_list |
Description
Olivier Mehani
2009-07-15 14:54:44 UTC
Reassigned to drivers/input. 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). 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.
No, we should not be breeding the horrors from keyboard.c... How about the following 2 patches instead? Created attachment 23008 [details]
Implement input filters
Created attachment 23009 [details]
Implement mac button emulation as input filter
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. Umm, it's missing .strategy = sysctl_data, In the part that uses mac_hid_toggle_emumouse as a proc_handler. Created attachment 23145 [details]
Updated version that properly sets up 'strategy' on sysctl entry
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? 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... Created attachment 24313 [details]
Quick 'n' Dirty printk-based debug patch
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.
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. 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.
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. The patches are upstream so closing. |