On Fri, Jul 22, 2016 at 2:09 AM, Benjamin Tissoires
[off-list ref] wrote:

On Jul 20 2016 or thereabouts, Jason Gerecke wrote:

quoted

On 07/12/2016 12:36 AM, Benjamin Tissoires wrote:

quoted

On Jul 11 2016 or thereabouts, Jason Gerecke wrote:

quoted

"Direct" input deviecs like Cintiqs and Tablet PCs set the INPUT_PROP_DIRECT
property to notify userspace that the sensor and screen are overlayed. This
information can also be useful elsewhere within the kernel driver, however,
so we introduce a new WACOM_DEVICETYPE_DIRECT that signals this to other
kernel code.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/wacom_wac.c | 40 ++++++++++++++++++++++++----------------
 drivers/hid/wacom_wac.h |  1 +
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index e499cdb..2523a29 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c

@@ -1757,8 +1757,10 @@ void wacom_wac_usage_mapping(struct hid_device *hdev,
 {
   struct wacom *wacom = hid_get_drvdata(hdev);
   struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+  struct wacom_features *features = &wacom_wac->features;

   /* currently, only direct devices have proper hid report descriptors */
+  features->device_type |= WACOM_DEVICETYPE_DIRECT;
   __set_bit(INPUT_PROP_DIRECT, wacom_wac->pen_input->propbit);
   __set_bit(INPUT_PROP_DIRECT, wacom_wac->touch_input->propbit);

nitpick: you might as well remove these additions of INPUT_PROP_DIRECT,
as you are handling the test later and adding them no matter what.

Are you seeing something I'm missing? The new calls in
wacom_setup_{pen,touch}_input_capabilities that setup INPUT_PROP_DIRECT
won't be called for HID_GENERIC devices since we bail out at the top of

Oh, yes, you are right.

quoted

the functions. Perhaps you'd prefer me to move their setting of
INPUT_PROP_DIRECT to before the point we bail, letting me remove them
from here?

I think it would be better to set the flag in wacom_wac_usage_mapping()
and rely on this flag to set INPUT_PROP_DIRECT (by moving up the
setting of INPUT_PROP_DIRECT). It feels weird to set the flag + the
consequences of the flag one after the other. This way, we could also
imagine that if indirect devices are around and use generic, we could
just remove the flag, and the INPUT_PROP will be removed without any
extra effort.

quoted

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....

quoted

Rest looks good to me.

Reviewed-by: Benjamin Tissoires <redacted>

Cheers,
Benjamin

quoted

@@ -2465,6 +2467,19 @@ void wacom_setup_device_quirks(struct wacom *wacom)
   if (features->type == REMOTE)
           features->device_type = WACOM_DEVICETYPE_PAD;

+  if (features->type == PL          || features->type == DTU           ||
+      features->type == DTUS        || features->type == DTUSX         ||
+      features->type == WACOM_21UX2 || features->type == WACOM_22HD    ||
+      features->type == DTK         || features->type == WACOM_24HD    ||
+      features->type == WACOM_27QHD || features->type == CINTIQ_HYBRID ||
+      features->type == CINTIQ_COMPANION_2 || features->type == CINTIQ ||
+      features->type == WACOM_BEE   || features->type == WACOM_13HD    ||
+      features->type == WACOM_24HDT || features->type == WACOM_27QHDT  ||
+      features->type == TABLETPC    || features->type == TABLETPCE     ||
+      features->type == TABLETPC2FG || features->type == MTSCREEN      ||
+      features->type == MTTPC       || features->type == MTTPC_B)
+      features->device_type |= WACOM_DEVICETYPE_DIRECT;

As Bastien said, this is ugly, and a switch/case fits better.

Or have an array and iterate through it...

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html