Re: [RFC v2 03/10] hid-multitouch: support for PixCir-based panels

["Henrik Rydberg" <rydberg@euromail.se>]

On Fri, Jan 07, 2011 at 04:12:24PM +0100, Benjamin Tissoires wrote:
> Hi Henrik,
> 
> I've made the changes and I pushed them on the git of our lab. I
> thought it was easier for you to have a look at the changes instead of
> resending the whole patch. But v3 will be for this afternoon.

Well, it is not really easier, since there is no easy way to comment
on the code. Also, transparency is lost. Please let the review process
take its time.

> 
> the repo is at:
> http://lii-enac.fr/cgi-bin/gitweb.cgi?p=linux-input/enac-drivers.git;a=summary
> the changes are on the branch "hid-multitouch-dtor" from "Copyright
> notice" to the head ("hid-input: better way of handling
> HID_FEATURE_REPORT").
> 
> To summarize:
> 
> - I used touch_state and seen_in_this_frame -> I hope the semantic is
> now clearer. I was able to test it on stantum and cypress device. I
> should do the test for pixcir as soon as I can have some time in the
> afternoon, and the test against generaltouch should happend in the
> beginning of the week.

The code looks clearer, but there are still two more or less identical
structures in there. Please remove, it just looks silly.

> - I integrated fuzz (please check)

But the mt_input_mapped function did not change - needs to change too.

> - I re-factorized set_abs (just copied-pasted your code)

Ok.

> - I made other small changes
> 
> - Concerning the quirks, I am not very in favor ATM: a flag for
> compute slot will infer a switch of 5 cases in the emit_event, and the
> idea was to be able to have a constant time access (1) when using
> specific function.
> The Quirk MT_QUIRK_NOT_SEEN_MEANS_UP is better (it will speed up a
> little the code: 1 test against 1 test and 1 affectation) but I don't
> think it worse the effort for further device additions.

Given everything that happens within a hid packet, this really, really
does not matter much. And the quirks can be implemented efficiently
too, if really needed.

> - The Egalax problem: I am pretty sure that Stéphane took this driver
> into account when writing the original patch. BTW I propose to
> postpone the problem for 2.6.39.

This driver is aiming at engulfing a larger set of drivers, and as
such, should be prepared sensibly, IMO. Rushing things will only cause
us grief further down the road.

Henrik