[Raring][PULL] Cypress PS/2 Trackpad driver

[Seth Forshee]

On Thu, Jan 17, 2013 at 02:03:16PM -0800, Kamal Mostafa wrote:
> On Thu, 2013-01-17 at 14:38 -0600, Seth Forshee wrote:
> > On Thu, Jan 17, 2013 at 11:04:22AM -0800, Kamal Mostafa wrote:
> > > The Cypress PS/2 Trackpad driver [...]
> > > plus my "simulated multitouch" Cypress
> > > driver enhancement (denied by upstream, but required for Unity
> > > multitouch functionality; is included in our P and Q version).
> 
> 
> > I'm not really so keen on this simulated multitouch patch.
> > 
> 
> Hi Seth-
> 
> Thanks for reviewing this, and for all your help with this driver.  I'm
> certainly open to discussion about the validity of the Cypress simulated
> multitouch patch...

I don't think there's much to discuss as far as validity goes. The
driver just makes stuff up. From a technical perspective I don't see how
that could be considered valid.

> > First, I think you overstate the case for the simulated multitouch
> > functionality.
> 
> The main case for it is that the shipping Precise and Quantal SAUCE
> versions of this driver do include the simulated multitouch enhancement.
> The original driver as supplied by Cypress included a variant of it. 
> 
> So for the Cypress trackpad, Unity multi-touch gestures (>2 fingers)
> work now in P and Q, but if we omit my simul-mt patch then they won't
> work anymore in Raring: a regression.

The fact that this would be a regression is the *only* legitimate
argument I see in favor of keeping the patch. But it's a strong
argument. Too bad we didn't notice this and get rid of it before putting
it into P and Q.

> > As for gestures involving 3+ fingers, the code [Kamal's simul-mt patch]
> > just generates fake touch
> > points offset horizontally from the first one. That means every gesture
> > looks like a drag.
> 
> A drag or a tap.  As coded, it works very nicely for Unity's 3-finger
> drag and 4-finger tap gestures.

Okay, sure. I wasn't really thinking of taps as being gesutres ;-)

> > What's wrong with limiting it to semi-mt, in line with the actual
> > capabilities of the hardware?
> 
> My feeling is that the addition of the simul-mt patch is useful and
> harmless, so what's wrong with including it?

One minor problem would be if any desktop environment running on top of
our kernel supports gestures with >2 fingers other than tap and drag.
Users may be confused to find that every gesture gets treated like a
drag.

> > One more comment, regarding the implementation. The patch lacks
> > protection for report_data->contact_cnt > CTYP_MAX_MT_SLOTS whereas the
> > rest of the driver seems to protect against it. I'm assuming that
> > because you've increased that max number of slots from 2 to 5 there's no
> > longer a concern of having more contacts than slots? Because if that
> > happens you're going to overrun report_data->contacts.
> 
> Your assumption is correct.  contact_cnt cannot ever become > 5, and the
> checks for contact_cnt > CTYP_MAX_MT_SLOTS in the rest of the driver are
> actually necessary *only* to support the semi-mt case.  Those checks
> aren't for overrun protection, they are what implements the regular
> semi-mt "maximum-of-two-coordinate-pairs" behavior.

Good, I just wanted to be sure. Thanks for confirming.

Seth