Re: [PATCH] macintosh: move mac_hid driver to input/mouse.

[Benjamin Tissoires]

On May 30 2017 or thereabouts, Michal SuchÃnek wrote:
> On Tue, 30 May 2017 12:33:21 +0200
> Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> wrote:
> 
> > Hi Michal,
> > 
> > On May 30 2017 or thereabouts, Michal SuchÃnek wrote:
> > > On Mon, 29 May 2017 22:06:06 -0700
> > > Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
> > >   
> > > > On Mon, May 29, 2017 at 08:03:25PM +0200, Michal SuchÃnek wrote:  
> > > > > On Sun, 28 May 2017 10:55:40 -0700
> > > > > Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote:
> > > > >     
> > > > > > On Sun, May 28, 2017 at 11:47:58AM +0200, Michal Suchanek
> > > > > > wrote:    
> 
> > >   
> > > >   
> > > > > 
> > > > > More importantly how is that mapping supposed to be represented
> > > > > in a hwdb file?
> > > > > 
> > > > > The help text in the hwdb file says:
> > > > > 
> > > > > # Scan codes are specified as:
> > > > > #   KEYBOARD_KEY_<hex scan code>=<key code identifier>
> > > > > # The scan code should be expressed in hex lowercase. The key
> > > > > codes # are retrieved and normalized from the kernel input API
> > > > > header.
> > > > > 
> > > > > So they are converted in some unspecified way.
> > > > > 
> > > > > The example below defines some mappings, presumably:
> > > > > 
> > > > > evdev:atkbd:dmi:bvn*:bvr*:bd*:svnAcer*:pn*
> > > > > evdev:atkbd:dmi:bvn*:bvr*:bd*:svnGateway*:pnA0A1*:pvr*
> > > > > evdev:atkbd:dmi:bvn*:bvr*:bd*:svneMachines:pneMachines*E725:pvr*
> > > > >  KEYBOARD_KEY_a5=help                                   # Fn+F1
> > > > >  KEYBOARD_KEY_a6=setup                                  # Fn+F2
> > > > >  KEYBOARD_KEY_a7=battery                                # Fn+F3
> > > > > 
> > > > > /usr/include/linux/input-event-codes.h has occurence of battery
> > > > > in
> > > > > 
> > > > > #define KEY_BATTERY		236
> > > > > 
> > > > > meaning that the unspecified conversion is probably performed by
> > > > > 
> > > > > 1) stripping KEY_ prefix
> > > > > 2) converting to lowercase
> > > > > 
> > > > > This is what systemd hwdb check script does in reverse when
> > > > > checking the keycode values.
> > > > > 
> > > > > The BTN_LEFT 0x110 value does not conflict with KEY_* values,
> > > > > though. So technically you could include it in the keymap. If
> > > > > you had a tool for that.    
> > > > 
> > > > Hmm, sounds like you want a patch to udev/systemd. For the kernel
> > > > there is no difference between keys and buttons, except symbolic
> > > > names. They all go into dev->keybit and are reported with
> > > > input_report_key().  
> > > 
> > > So the userspace is not ready for this while the kernel already has
> > > the driver for ages and all that is needed is to enable it.  
> > 
> > The problem is more that you can have your particular issue solved
> > here by using this particular driver. But then someone says that the
> > mapping for right/middle doesn't fit to his/her keyboard layout, and
> > then we will have to maintain an new set of fancy new features in a
> > driver that is just legacy.
> 
> The mapping is dynamic. You specify which keycode you translate to
> which button by writing a sysfs file and if you do not nothing is
> translated. So far as plain mouse emulation goes this driver is
> perfectly fine. It will probably not emulate a scrollwheel and other
> fancy features but that can be solved in later more flexible solution
> once it is finished. And once it is finished then is the time for
> retiring this driver.

My bad. Still, it doesn't change the fact that this driver should be
pointless and retired, because it has to be done in userspace.

> 
> > 
> > I concede userspace doesn't have the feature, but you should really
> > have a look at libinput (i.e. open a bug on bugs.freedesktop.org) and
> > request the feature here.
> > 
> > >   
> > > >   
> > > > > And if it is not rejected by the kernel.    
> > > > 
> > > > It should not. setkeycodes definitely works on atkbd.
> > > >   
> > > > > And if it does not
> > > > > crash your X server which is very picky about receiving pointer
> > > > > events from a keyboard or the other way around.    
> > > > 
> > > > Sounds like you want to make X server more resilient ;)  
> > > 
> > > This is an inherent design flaw in the X server known for years. It
> > > has separate keyboard and pointer devices and while people are
> > > trying to patch it up occasionally a bug pops out that crashes the
> > > Xserver when an event is received from non-matching device type.
> > > 
> > > Once X server dies and is replaced be Y server or Z server or
> > > whatever it will make life easier in so many ways.  
> > 
> > We already have libinput that tends to replace the Xorg input part (at
> > least everything which is not protocol). 
> 
> And the protocol still has the split to pointer and keyboard. It is
> technically possible to map mouse button presses with XKB but it only
> works in X and is undocumented and really difficult to do. And it does
> not work outside of X.

I never talked about XKB. Libinput has a view on all input devices and
can redirect whatever events it sees into any other input device. From a
X perspective it's as if the emulated button would have been generated
by the touchpad itself.

> 
> > So if the feature is in
> > libinput, it should be available in X.org directly, and you will be
> > saved.
> 
> Not everything uses libinput. So no, using libinput is not the answer.

If you do not want to upgrade and use the latest development tools, then
sure, you can always use the mac_hid emulation. But we will not resurrect
it because it's something from the past.

> 
> Also libinput people suggest to use hwdb for mapping which is back
> to where we were:
> https://lists.freedesktop.org/archives/wayland-devel/2015-December/026223.html

Libinput people are basically Peter Hutterer alone. And in the keyboard
layout mapping, yes users should use hwdb/systemd for that. In your precise
case, libinput should have a setting that redirect the incoming key onto
a button of a different device. It's not implemented, it's doable, but
you won't receive a NACK as you get here, because you have a valid use
case for it.

> 
> > 
> > >   
> > > > 
> > > > But really, it all is better solved in userspace, where you can
> > > > surface all options to the user. For example Chrome OS uses Alt +
> > > > mouse button (or tap) to do right click, I am sure Gnome or KDE
> > > > has similar support for right and middle buttons.  
> > > 
> > > I do not consider Gnome or KDE more suitable driver for my keyboard
> > > than the Linux kernel. In fact I find both very unsuitable as a
> > > driver, heavyweight, and causing problems with graphical desktop
> > > usability. Not  
> > 
> > Well, with wayland, the compositor embeds the input stack and part of
> > the graphic stack, and it's actually faster than X.
> 
> That's pointless when it does not work as a desktop in practice.
> 
> And it does not make GNOME or KDE a suitable driver for keyboard, even
> if you run Wayland.

I am just not following what is your use case. Do you want to enable
this feature in a TTY, in a full blown desktop, in a plain X that you
launched with "/usr/bin/Xorg"?

And if you just want Xorg with whatever desktop manager/environment, you
should already be using libinput given that xorg-xf86-libinput is now
the only maintained X driver for keyboards and touchpads.

> 
> > 
> > > to mention they cannot be used at all when not running a graphical
> > > desktop and often cause system crashes due to stressing the graphics
> > > hardware.  
> > 
> > If you experience crashes, they are probably bugs, and please report
> > them to the appropriate project.
> 
> And the answer is they know it crashes but it's too much work to fix.
> Or that it works for them and you are just unlucky that all of your
> half dozen cards crash with their driver and they cannot do anything
> about it.
> Maybe next year. Or the year after. Or with different piece of
> hardware. Who knows?
> 

But is this a reason to stop proposing patches to those projects or
report bugs?
We already told you the kernel is not the place. The kernel can do a
lot, true, but when the input maintainer tell you this is not the place,
you should have a pretty good hint that it is not the place.

> > 
> > >   
> > > > 
> > > > Solving this at kernel is wrong place, similarly how we avoid
> > > > parsing user gestures (edge scrolling, 2-finger scrolling,
> > > > pitch/zoom, etc) in kernel. Yes, we have legacy drivers (like
> > > > mousedev) that are artefacts of times when userspace support was
> > > > not there and it made sense to covert everything to emulated
> > > > PS/2, but that time is long gone.  
> > > 
> > > Unlike 2-finger scrolling, pitch/zoom, and whatnot key mapping is
> > > something kernel can do, does, and should do.  
> > 
> > Looks like we are all on the same page. Kernel does key mapping
> > already and doesn't do 2-finger scrolling and other gestures.
> > 
> > The difference in approach is that you want the kernel to emulate
> > devices when userspace has much more information to know how to
> > emulate those devices.
> 
> Any piece of software has the information when the user tells it. The
> thing is the kernel is also able to emulate the device and the
> userspace is not. And seriously, this is a hardware problem which is
> simple to solve once for good in kernel. Do you advocate that instead

It is a hardware problem that is not solvable in the kernel (or should
not). Would the keyboard had specific keys designed to send buttons,
yes, we would have do our best. But here you are trying to counter a
hardware design issue by using a hack at the wrong level. That's all we
are saying.

> everyone should set up mouse emulation in N application frameworks and
> implement it in those that miss it?

Well, given the amount of people involved in the input community, you
can be sure that adding this in libinput is just enough.

> 
> > 
> > The kernel job is to forward events from the devices to userspace
> > without losing information and in a standard way. Anything beyond that
> > should be handled in userspace.
>  
> Then why do you do keycode translation at all? 

Because keycode translations are used for fixing hardware when the
keycode sent is non standard, and instead of requiring people to send
kernel patches for it, it's better to ask them to drop a hwdb entry for
them. This doesn't contradict what I said: it helps forwarding a random
event from the device into a standard one.

keycode translation also helps setting layouts on TTY. Because when you
are on a TTY there is not much you can do in userspace to fix it.

> I guess because it is the job of the kernel to make the information
> usable by the applications as well.

Yes and no. The kernel forwards the information in a standard way to
applications. If you want to locally remap any key to any other event,
fine. But it shouldn't be the default.

> 
> > 
> > > 
> > > The difference between the plain mapping and mac mouse emulation is
> > > that mac mouse emulation probably creates a separate pointer device
> > > which is as compatible with legacy software as it gets. Unlike
> > > application-specific solutions it works in any software that accepts
> > > input events including X. And unlike the systemd solution it works
> > > today.  
> > 
> > And we can argue that the issue with the mac mouse emulation is that
> > it filters on all connected keyboard.
> > Meaning that if you connect an external keyboard just from time to
> > time, those two keys will send the emulated buttons instead of
> > sending plain key events.
> 
> Well, that's a limitation of simple solution. 
> 
> > 
> > While on a libinput approach, the filtering will only affect the
> > internal keyboard, making the whole experience better.
> 
> Unless it happens to crash the X server.

How can you know it'll crash the X server when the feature is not even
implemented?

Luckily, Peter is also in charge of XInput, so I guess if it crashes the
server, it will be detected/fixed way before it is public.

> 
> > 
> > > 
> > > Given the state of userspace at this point I find using the mac
> > > mouse emulation most sensible thing to do. Improving the keymap
> > > support in systemd is certainly more flexible so it is the way to
> > > go long term but if a patch is written and accepted today it will
> > > take years until support is commonplace. On the other hand, the
> > > change required in Linux is trivial and updating the kernel without
> > > breaking the whole system is much more viable than updating
> > > systemd.  
> > 
> > With a libinput approach, the time to distro is much reduced (unless
> > you are running a *very* old libinput release that is not ABI
> > compatible anymore).
> 
> There is no libinput approach. They eschew mapping since there is one
> in systemd and one in X already. 

Please see above. Open a bug and we can start talking.

> 
> > 
> > And we can return the argument for the kernel. The time for a new
> > option in the kernel to land in some distribution can also be counted
> > in years.
> > 
> 
> Indeed. However, running own kernel is much more likely to succeed than
> running own systemd.

And recompiling a local libinput is way faster than recompiling the
kernel. I never talked about systemd, but libinput.

Cheers,
Benjamin

> 
> Thanks
> 
> Michal