On Jul 20 2016 or thereabouts, Jason Gerecke wrote:
> On 07/12/2016 12:36 AM, Benjamin Tissoires wrote:
> > On Jul 11 2016 or thereabouts, Jason Gerecke wrote:
> >> "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@xxxxxxxxx>
> >> ---
> >> 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.
> 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.
>
> 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....
>
> > Rest looks good to me.
> >
> > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
> >
> > Cheers,
> > Benjamin
> >
> >>
> >> @@ -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.
Cheers,
Benjamin
> >> +
> >> if (wacom->hdev->bus == BUS_BLUETOOTH)
> >> features->quirks |= WACOM_QUIRK_BATTERY;
> >>
> >> @@ -2516,6 +2531,10 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> >> input_abs_set_res(input_dev, ABS_X, features->x_resolution);
> >> input_abs_set_res(input_dev, ABS_Y, features->y_resolution);
> >>
> >> + if (features->device_type & WACOM_DEVICETYPE_DIRECT)
> >> + __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> >> + else
> >> + __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >>
> >> switch (features->type) {
> >> case GRAPHIRE_BT:
> >> @@ -2540,8 +2559,6 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> >> __set_bit(BTN_TOOL_MOUSE, input_dev->keybit);
> >> __set_bit(BTN_STYLUS, input_dev->keybit);
> >> __set_bit(BTN_STYLUS2, input_dev->keybit);
> >> -
> >> - __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >> break;
> >>
> >> case WACOM_27QHD:
> >> @@ -2556,7 +2573,6 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> >> case CINTIQ_COMPANION_2:
> >> input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
> >> input_abs_set_res(input_dev, ABS_Z, 287);
> >> - __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> >> wacom_setup_cintiq(wacom_wac);
> >> break;
> >>
> >> @@ -2572,8 +2588,6 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> >> /* fall through */
> >>
> >> case INTUOS:
> >> - __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >> -
> >> wacom_setup_intuos(wacom_wac);
> >> break;
> >>
> >> @@ -2583,8 +2597,6 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> >> case INTUOSPL:
> >> case INTUOS5S:
> >> case INTUOSPS:
> >> - __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >> -
> >> input_set_abs_params(input_dev, ABS_DISTANCE, 0,
> >> features->distance_max,
> >> features->distance_fuzz, 0);
> >> @@ -2614,8 +2626,6 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> >> __set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
> >> __set_bit(BTN_STYLUS, input_dev->keybit);
> >> __set_bit(BTN_STYLUS2, input_dev->keybit);
> >> -
> >> - __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> >> break;
> >>
> >> case PTU:
> >> @@ -2626,16 +2636,12 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
> >> __set_bit(BTN_TOOL_PEN, input_dev->keybit);
> >> __set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
> >> __set_bit(BTN_STYLUS, input_dev->keybit);
> >> -
> >> - __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >> break;
> >>
> >> case INTUOSHT:
> >> case BAMBOO_PT:
> >> case BAMBOO_PEN:
> >> case INTUOSHT2:
> >> - __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >> -
> >> if (features->type == INTUOSHT2) {
> >> wacom_setup_basic_pro_pen(wacom_wac);
> >> } else {
> >> @@ -2693,6 +2699,11 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> >> features->y_resolution);
> >> }
> >>
> >> + if (features->device_type & WACOM_DEVICETYPE_DIRECT)
> >> + __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> >> + else
> >> + __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >> +
> >> switch (features->type) {
> >> case INTUOS5:
> >> case INTUOS5L:
> >> @@ -2700,8 +2711,6 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> >> case INTUOSPL:
> >> case INTUOS5S:
> >> case INTUOSPS:
> >> - __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> >> -
> >> input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, features->x_max, 0, 0);
> >> input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR, 0, features->y_max, 0, 0);
> >> input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_POINTER);
> >> @@ -2724,7 +2733,6 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
> >>
> >> case TABLETPC:
> >> case TABLETPCE:
> >> - __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
> >> break;
> >>
> >> case INTUOSHT:
> >> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> >> index 8a8974c..7ad6273 100644
> >> --- a/drivers/hid/wacom_wac.h
> >> +++ b/drivers/hid/wacom_wac.h
> >> @@ -82,6 +82,7 @@
> >> #define WACOM_DEVICETYPE_TOUCH 0x0002
> >> #define WACOM_DEVICETYPE_PAD 0x0004
> >> #define WACOM_DEVICETYPE_WL_MONITOR 0x0008
> >> +#define WACOM_DEVICETYPE_DIRECT 0x0010
> >>
> >> #define WACOM_VENDORDEFINED_PEN 0xff0d0001
> >> #define WACOM_G9_PAGE 0xff090000
> >> --
> >> 2.9.0
> >>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
