On Apr 03 2015 or thereabouts, David Herrmann wrote: > Hi > > On Fri, Apr 3, 2015 at 4:38 PM, Benjamin Tissoires <btiss...@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 <hdego...@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 <hdego...@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?
It's a little bit messier than that actually :) I think (I never had it in my hands, so this is from reading the driver) that the 27 QHD (Touch) will show up as: - "Wacom Cintiq 27QHD touch Pen" -> regular tablet input (for the stylus) which will be seen by udev as a tablet - "Wacom Cintiq 27QHD touch Touch" -> regular multitouch screen, libwacom will set the ID_INPUT_TABLET tag (no way for udev to detect it except lookking at the siblings and this might not be reliable) - "Wacom Cintiq 27QHD touch Pad" -> this will have INPUT_PROP_ACCELEROMETER, ABS_X|Y|Z, KEY_PROG1|2|3 -> so setting only ID_INPUT_ACCELEROMETER for it is fine, there will be no way to reliably detect the tablet capability (unless the kernel provides it) So I think you are both right (and wrong). Hans' patch works with this current device, but it won't allow to have other ID_INPUT_* tags set. And David is right, but we should not tag as ID_INPUT_TABLET :) Cheers, Benjamin _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel