[systemd-devel] [PATCH] udev: input_id: tag accelerometers as ID_INPUT_ACCELEROMETER

[David Herrmann]

Hi

On Fri, Apr 3, 2015 at 4:38 PM, Benjamin Tissoires <btissoir at redhat.com> wrote:
> On Apr 03 2015 or thereabouts, Hans de Goede wrote:
>> Hi,
>>
>> On 03-04-15 15:51, David Herrmann wrote:
>> >Hi
>> >
>> >On Fri, Apr 3, 2015 at 12:07 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>> >>input_id already (tries to) tag accelerometers as such, but this only works
>> >>for absolute accelerometers. Recent kernels mark accelerometers through an
>> >>input prop. Trust that prop and always tag devices with it with
>> >>ID_INPUT_ACCELEROMETER.
>> >>
>> >>Note that detection by the prop bit works the same as the existing detection
>> >>and will ensure that no other tags get set on the device.
>> >>
>> >>Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> >>---
>> >>  src/shared/missing.h             | 4 ++++
>> >>  src/udev/udev-builtin-input_id.c | 5 +++++
>> >>  2 files changed, 9 insertions(+)
>> >>
>> >>diff --git a/src/shared/missing.h b/src/shared/missing.h
>> >>index 3bdfd8f..4464e35 100644
>> >>--- a/src/shared/missing.h
>> >>+++ b/src/shared/missing.h
>> >>@@ -944,3 +944,7 @@ static inline int kcmp(pid_t pid1, pid_t pid2, int type, unsigned long idx1, uns
>> >>  #ifndef INPUT_PROP_POINTING_STICK
>> >>  #define INPUT_PROP_POINTING_STICK 0x05
>> >>  #endif
>> >>+
>> >>+#ifndef INPUT_PROP_ACCELEROMETER
>> >>+#define INPUT_PROP_ACCELEROMETER  0x06
>> >>+#endif
>> >>diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
>> >>index d4c38ca..ecfc447 100644
>> >>--- a/src/udev/udev-builtin-input_id.c
>> >>+++ b/src/udev/udev-builtin-input_id.c
>> >>@@ -136,6 +136,11 @@ static void test_pointers (struct udev_device *dev,
>> >>          int is_mouse = 0;
>> >>          int is_touchpad = 0;
>> >>
>> >>+        if (test_bit (INPUT_PROP_ACCELEROMETER, bitmask_props)) {
>> >>+                udev_builtin_add_property(dev, test, "ID_INPUT_ACCELEROMETER", "1");
>> >>+                return;
>> >>+        }
>> >>+
>> >
>> >So this property is only set for accelerometer-only devices?
>>
>> Hmm, good question. Currently I see only one user of it in the kernel:
>>
>> drivers/hid/wacom_wac.c
>
> Yep, this is the first and only for now.
>
>>
>> Which most likely does not count as an accelerometer only device, so maybe
>> my idea to follow the existing accelerometer detection code and short-circuit
>> the other tests was not such a good idea.
>>
>> Benjamin, do you have any input on this ?
>
> See Peter's documentation patch here:
> https://patchwork.kernel.org/patch/6102851/
>
> If the property is set, "Directional axes on this device (absolute
> and/or relative x, y, z) represent accelerometer data. All other axes
> retain their meaning. A device must not mix regular directional axes and
> accelerometer axes on the same event node."
>
> So we can have multiple axis, multiple buttons but X,Y,Z will be accel
> values.
>
> The Cintiq 27 QHD has a builtin accelerometer and it is reported by this
> property to explicitly say that the X,Y,Z are not stylus data but
> accelerometers values.
>
> For this device, there is no problem because libwacom will set
> ID_INPUT_TABLET through a custom udev rule, but for others, the "return"
> will be a problem IMO (note that Wacom already breaks the ID_INPUT_* tag
> should be set only once per device).

Yeah, thanks!

Hans, I'm not sure your patch is correct. I mean, just because the
input-device exports an accelerometer, doesn't necessarily mean that
we should tag it as such, right? Or does the Cintiq 27 QHD export
_multiple_ input devices, and the accelerometer has its own evdev
node? In that case, I'm fine with it. But otherwise, we should still
tag it as tablet, right?

Thanks
David