Bug 91563

can you attach the output of evemu-describe for this device please? makes it easier to triage.
http://wayland.freedesktop.org/libinput/doc/latest/reporting_bugs.html

ping?

(In reply to Peter Hutterer from comment #1)
> can you attach the output of evemu-describe for this device please? makes it
> easier to triage.
> http://wayland.freedesktop.org/libinput/doc/latest/reporting_bugs.html

Matthieu is away and my MacBook Pro is in a different town.

The driver is at:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git//tree/drivers/hwmon/applesmc.c
in the meanwhile.

right, looks like this thing has an x/y axis and is likely tagged as mouse. It's missing the (relatively recent) INPUT_PROP_ACCELEROMETER, once that is set systemd will tag it as accelerometer and we'll ignore it.

$ sudo libinput-list-devices 
Device:           Power Button
Kernel:           /dev/input/event3
Group:            1
Seat:             seat0, default
Capabilities:     keyboard 
Tap-to-click:     n/a
Tap drag lock:    n/a
Left-handed:      n/a
Nat.scrolling:    n/a
Middle emulation: n/a
Calibration:      n/a
Scroll methods:   none
Click methods:    none

Device:           Power Button
Kernel:           /dev/input/event1
Group:            2
Seat:             seat0, default
Capabilities:     keyboard 
Tap-to-click:     n/a
Tap drag lock:    n/a
Left-handed:      n/a
Nat.scrolling:    n/a
Middle emulation: n/a
Calibration:      n/a
Scroll methods:   none
Click methods:    none

Device:           Sleep Button
Kernel:           /dev/input/event2
Group:            3
Seat:             seat0, default
Capabilities:     keyboard 
Tap-to-click:     n/a
Tap drag lock:    n/a
Left-handed:      n/a
Nat.scrolling:    n/a
Middle emulation: n/a
Calibration:      n/a
Scroll methods:   none
Click methods:    none

Device:           Apple Inc. Apple Internal Keyboard / Trackpad
Kernel:           /dev/input/event4
Group:            4
Seat:             seat0, default
Capabilities:     keyboard 
Tap-to-click:     n/a
Tap drag lock:    n/a
Left-handed:      n/a
Nat.scrolling:    n/a
Middle emulation: n/a
Calibration:      n/a
Scroll methods:   none
Click methods:    none

Device:           bcm5974
Kernel:           /dev/input/event6
Group:            5
Seat:             seat0, default
Size:             104.48x75.60mm
Capabilities:     pointer 
Tap-to-click:     disabled
Tap drag lock:    disabled
Left-handed:      disabled
Nat.scrolling:    disabled
Middle emulation: n/a
Calibration:      n/a
Scroll methods:   *two-finger edge 
Click methods:    button-areas *clickfinger 

Device:           FaceTime HD Camera (Built-in)
Kernel:           /dev/input/event10
Group:            6
Seat:             seat0, default
Capabilities:     keyboard 
Tap-to-click:     n/a
Tap drag lock:    n/a
Left-handed:      n/a
Nat.scrolling:    n/a
Middle emulation: n/a
Calibration:      n/a
Scroll methods:   none
Click methods:    none

Device:           Apple Computer, Inc. IR Receiver
Kernel:           /dev/input/event7
Group:            7
Seat:             seat0, default
Capabilities:     keyboard 
Tap-to-click:     n/a
Tap drag lock:    n/a
Left-handed:      n/a
Nat.scrolling:    n/a
Middle emulation: n/a
Calibration:      n/a
Scroll methods:   none
Click methods:    none

And udev output:
P: /devices/platform/applesmc.768/input/input11
E: ABS=3
E: DEVPATH=/devices/platform/applesmc.768/input/input11
E: EV=9
E: ID_FOR_SEAT=input-platform-applesmc_768
E: ID_INPUT=1
E: ID_PATH=platform-applesmc.768
E: ID_PATH_TAG=platform-applesmc_768
E: MODALIAS=input:b0019v0000p0000e0000-e0,3,kra0,1,mlsfw
E: NAME="applesmc"
E: PRODUCT=19/0/0/0
E: PROP=0
E: SUBSYSTEM=input
E: TAGS=:seat:
E: USEC_INITIALIZED=57875

P: /devices/platform/applesmc.768/input/input11/event5
N: input/event5
S: input/by-path/platform-applesmc.768-event
E: DEVLINKS=/dev/input/by-path/platform-applesmc.768-event
E: DEVNAME=/dev/input/event5
E: DEVPATH=/devices/platform/applesmc.768/input/input11/event5
E: ID_INPUT=1
E: ID_PATH=platform-applesmc.768
E: ID_PATH_TAG=platform-applesmc_768
E: MAJOR=13
E: MINOR=69
E: SUBSYSTEM=input
E: USEC_INITIALIZED=90248

Seems that it's already ignored but lacks the accelerometer tag.

Created attachment 118007 [details]
patch to add to linux tree, to add INPUT_PROP_ACCELEROMETER

Sorry for my silence but, as said Bastien, I was away.
(And, as I'm leaving again, I'll be silent for the next few weeks..)


Indeed, the tag INPUT_PROP_ACCELEROMETER is missing in applesmc. Adding it [1] doesn't fix the problem. It shows that it is more complex that it seems ...

With the tag INPUT_PROP_ACCELEROMETER, a recent systemd version (>219, the fedora22 version[2]) properly add the udev ID_INPUT_ACCELEROMETER=1 tag.

With this tag, libinput still open the device. However, it seems to be the right thing to do. Gnome(-shell?) now detect the device is a accelerometer and display a button to deactivate screen rotation, so we should keep the device open.

So I've add a custom udev rules to remove the udev ID_INPUT (and ID_INPUT_ACCELEROMETER) tag:

SUBSYSTEM=="input" ATTRS{properties}=="40" ENV{ID_INPUT}="" ENV{ID_INPUT_ACCELEROMETER}=""

Without a ID_INPUT, libinput should ignore the device, but.. it doesn't.
The device is still open, but it seems to be never closed.
Each time, I go back to gdm (close user session), the device is open one more time. lsof report each time that the gnome-shell process open N time the same device.

I have still not found why. Once I'll go back, I'll investigate this again.

[1] I haven't submit my patch to kernel, but just apply the attached patch to linux tree if you what to test.

[2] If you what to test with 219 systemd version, you can add a custom rules to add the ID_INPUT_ACCELEROMETER yourself :
SUBSYSTEM=="input" ATTRS{properties}=="40" ENV{ID_INPUT_ACCELEROMETER}="1"

(In reply to Matthieu Gautier from comment #6)
> Created attachment 118007 [details]
> patch to add to linux tree, to add INPUT_PROP_ACCELEROMETER
> 
> Sorry for my silence but, as said Bastien, I was away.
> (And, as I'm leaving again, I'll be silent for the next few weeks..)
> 
> 
> Indeed, the tag INPUT_PROP_ACCELEROMETER is missing in applesmc. Adding it
> [1] doesn't fix the problem. It shows that it is more complex that it seems
> ...

The problem is that it's not an accelerometer, it only has 2 axis.

> With the tag INPUT_PROP_ACCELEROMETER, a recent systemd version (>219, the
> fedora22 version[2]) properly add the udev ID_INPUT_ACCELEROMETER=1 tag.
> 
> With this tag, libinput still open the device.

It shouldn't though.

> However, it seems to be the
> right thing to do. Gnome(-shell?) now detect the device is a accelerometer
> and display a button to deactivate screen rotation, so we should keep the
> device open.

That would be iio-sensor-proxy opening it (once) and letting go of it until it receives a kevent saying it's changed coarse orientation (which we'll never receive because the driver doesn't send it).

I think that libinput should just ignore it. Is there a udev tag for that?

(In reply to Bastien Nocera from comment #7)
> (In reply to Matthieu Gautier from comment #6)
> > Created attachment 118007 [details]
> > patch to add to linux tree, to add INPUT_PROP_ACCELEROMETER
> > 
> > Sorry for my silence but, as said Bastien, I was away.
> > (And, as I'm leaving again, I'll be silent for the next few weeks..)
> > 
> > 
> > Indeed, the tag INPUT_PROP_ACCELEROMETER is missing in applesmc. Adding it
> > [1] doesn't fix the problem. It shows that it is more complex that it seems
> > ...
> 
> The problem is that it's not an accelerometer, it only has 2 axis.

Yes, systemd 219 tag input as accelerometer if they have 3 axis.
Recent version add a test on INPUT_PROP_ACCELEROMETER property to also tag a input device correctly.

> 
> > With the tag INPUT_PROP_ACCELEROMETER, a recent systemd version (>219, the
> > fedora22 version[2]) properly add the udev ID_INPUT_ACCELEROMETER=1 tag.
> > 
> > With this tag, libinput still open the device.
> 
> It shouldn't though.
> 
> > However, it seems to be the
> > right thing to do. Gnome(-shell?) now detect the device is a accelerometer
> > and display a button to deactivate screen rotation, so we should keep the
> > device open.
> 
> That would be iio-sensor-proxy opening it (once) and letting go of it until
> it receives a kevent saying it's changed coarse orientation (which we'll
> never receive because the driver doesn't send it).
> 
> I think that libinput should just ignore it. Is there a udev tag for that?

There is no test on ID_INPUT_ACCELEROMETER in libinput code to discard a input device. We could add it if needed (as it is done for TABLET_PAD and JOYSTICK).
Another way, is to remove the ID_INPUT tag (What I've done with udev rule). 

Libinput should not open a device without ID_INPUT tag (but there is this bug of gdm opening it at each "resume" without closing it).

Is it so that gdm/gnome-shell doesn't close the input device because mutter is missing this <https://git.gnome.org/browse/mutter/commit/?id=7ce06928e24843c7f625616a526fa7bfc1e0bddb> patch?

(In reply to Jonas Ådahl from comment #9)
> Is it so that gdm/gnome-shell doesn't close the input device because mutter
> is missing this
> <https://git.gnome.org/browse/mutter/commit/
> ?id=7ce06928e24843c7f625616a526fa7bfc1e0bddb> patch?

Possibly. Did that get cherry-picked to stable releases (the gnome-3-16 branch)?

(In reply to Bastien Nocera from comment #10)
> (In reply to Jonas Ådahl from comment #9)
> > Is it so that gdm/gnome-shell doesn't close the input device because mutter
> > is missing this
> > <https://git.gnome.org/browse/mutter/commit/
> > ?id=7ce06928e24843c7f625616a526fa7bfc1e0bddb> patch?
> 
> Possibly. Did that get cherry-picked to stable releases (the gnome-3-16
> branch)?

No, doesn't seem like it. Sounds like a safe and reasonable thing to do though.

the libinput-list-devices output shows that libinput isn't handling the device, otherwise it'd be listed there. And it's ignored because it doesn't have ID_INPUT_* set, only the ID_INPUT tag but none of keyboard, mouse, etc. We require at least one of those to determine device type. So right now I'm putting my money on the mutter bug.

(In reply to Peter Hutterer from comment #12)
> the libinput-list-devices output shows that libinput isn't handling the
> device, otherwise it'd be listed there.

This is on my machine, not on Matthieu's though. Which probably doesn't have the same configuration and/or problem. Not sure this was helpful from me then.

should still be the same, systemd only assigns one of the ID_INPUT_ pointer types to the device. the device doesn't have any keys and I double-checked the udev output from comment #0, only ID_INPUT is set. so libinput doesn't do anything with it, we still require one of the ID_INPUT_foo to be set in addition:
http://cgit.freedesktop.org/wayland/libinput/tree/src/evdev.c#n1899

that aside, patch to explicitly ignore accelerometers is coming soon, but this bug isn't caused by libinput.

commit f0fa5903946f7b8d57bc625f58fb048f9b17ef2d
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Tue Sep 1 08:11:58 2015 +1000

    evdev: ignore accelerometer devices


Marking this bug as fixed for the libinput side, with the above commit we definitely ignore accelerometer devices.