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

On Fri, Feb 11, 2011 at 12:47 PM, Henrik Rydberg [off-list ref] wrote:

quoted

quoted

quoted

+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?

quoted

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.

quoted

quoted

So only false->true is possible here.

Yeah, both are bools. What else can they take?

Only the transition false-to-true, that is.

quoted

quoted

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