Re: [PATCH 4/4] input - wacom: Support 2FGT in MT format

[Ping Cheng <pinglinux@gmail.com>]

On Fri, Feb 11, 2011 at 12:47 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> >> +static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
>> >> +{
>> >> +     struct input_dev *input = wacom->input;
>> >> +     unsigned char *data = wacom->data;
>> >> +     int i;
>> >> +
>> >> +     if (wacom->shared->stylus_in_proximity && !wacom->shared->touch_down)
>> >> +             return 0;
>> >
>> > Seeing one case handled out of four possible always makes me nervous.
>>
>> The above statement is to avoid going through the input_mt_slot and
>> input_mt_report_slot_state routines without posting any meaningful
>> events. I guess it could be considered as a performance enhancement?
>
> It won't be posting events unless something changed.

But it still go through the  input_mt_slot and
input_mt_report_slot_state routines, which is unnecessary. If we know
up front there is no events posting, what benefit do we get to go
through the routines?

>> Which case makes you nervous? I'll take care of it ;).
>
> Well, removing the logic above would suffice. :-)

Yeah, I see one benefit here ;). I can remove that if others see the same issue.

>> > So only false->true is possible here.
>>
>> Yeah, both are bools. What else can they take?
>
> Only the transition false-to-true, that is.
>
>> > What I can see from the patchset, only wacom_tpc_single_touch() will ever set touch_down to
>> > false. Is that sufficient?
>>
>> No,  wacom_tpc_mt_touch can set touch_down to false too. When touch is
>> false, touch_down will be false. This happens when pen in prox or when
>> both fingers are up.
>
> Now, that is what cannot happen, because of the max() function.

This time is real. How about this one?


diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 39c289d..3cdafb9 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -699,7 +699,10 @@ static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
                }

                /* keep touch bit to send proper touch up event */
-               wacom->shared->touch_down = max(touch,
wacom->shared->touch_down);
+               if (i == 1)
+                       wacom->shared->touch_down = max(touch,
wacom->shared->touch_down);
+               else
+                       wacom->shared->touch_down = touch;
        }

        input_mt_report_pointer_emulation(input, true);

---------

Thank you.

Ping
--
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