Re: [PATCH] Add a quirk for the Dell XPS 13 (2015) when in PS/2 mode.

From: "Pali Rohár" <pali.rohar@gmail.com>
To: Mario Limonciello <mario_limonciello@dell.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	Rob <robr.bensson@gmail.com>
Subject: Re: [PATCH] Add a quirk for the Dell XPS 13 (2015) when in PS/2 mode.
Date: Wed, 25 Feb 2015 21:45:22 +0100	[thread overview]
Message-ID: <201502252145.22577@pali> (raw)
In-Reply-To: <54EE1917.4020805@dell.com>

[-- Attachment #1: Type: Text/Plain, Size: 2660 bytes --]

On Wednesday 25 February 2015 19:48:55 Mario Limonciello wrote:
> On 02/20/2015 02:41 PM, Pali Rohár wrote:
> > On Friday 20 February 2015 20:56:23 Mario Limonciello wrote:
> > 
> > resetafter=0 means to never reset (even if driver receive
> > e.g thousand invalid packets). I think this is very
> > dangerous if there will be other bugs either in linux
> > driver or some other HW problems.
> > 
> > For ALPS issue I added resetafter = pktsize * 2 (Allow 2
> > invalid packets without resetting device). Cannot you find
> > something similar for synaptics touchpads on XPS? (pktsize
> > for ALPS is 6, no idea how big are synaptics packets).
> 
> Pali,
> 
> I've done some experimentation with increasing the size to
> resetafter to up to pktsize * 4.  It will decrease the number
> of occurrences of this problem, but the problem still occurs
> eventually.  pktsize for synaptics is 6 as well.  Would you
> recommend to continue to go higher than that?  Since
> out_of_sync_cnt is reset when a full packet gets received,
> some arbitrarily high number should likely fix it to.
> 
> That being said, if you try to more closely follow what
> Windows does for the mouse, it's not issuing a reconnect no
> matter how much bad data is received.

I believe problem is similar to one as with ALPS devices. Driver 
always receive 6 bytes packet of data (no new byte is inserted 
and no byte is never lost), just one byte in packet is incorrect 
(does not match specification).

Setting resetafter to > 0 prevent problems when driver enters 
into undefined state (either by bug in driver of other SW/HW 
problem). So I think setting resetafter to 0 is not good idea.

But if we know that setting resetafter to 4*pktsize is not enough 
(e.g. with experimenting you saw that driver received more then 4 
invalid packets consecutively), set it to higher value.

I think it is still good idea to ignore maximally as many packets 
which can be received in time which is equal to resetting device.

E.g. when period of time in which we are dropping all packets is 
higher then time needed to reset touchpad, we should stop 
dropping packets and immediately reset touchpad. In this case we 
could hit maybe problem in driver (there can be bugs) or touchpad 
is in some bad state and out-of-sync...

So if your tests show that there are never invalid 10 packets 
consecutively, then set resetafter to 10 packets. Value 10 is 
still not high and if it fix problem with touchpad I think it is 
acceptable. But rather ask Dmitry what he thinks about it. This 
is just my opinion.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]