On Fri, Jul 22, 2016 at 2:09 AM, Benjamin Tissoires
<benjamin.tissoires@xxxxxxxxxx> wrote:
> 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.
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
