[NEEDS TEST][PATCH] elantech: discard the first 2 positions reports for some firmwares

[NEEDS TEST][PATCH] elantech: discard the first 2 positions reports for some firmwares

[Éric Piel]

Hello,

Here is a patch to implement a better workaround for the "jumpy cursor"
behaviour. Unfortunately, as I don't have such hardware, this is totally
untested (will, it compiles ;-) ). Hopefully, someone around has access
to such hardware and can test it. Or maybe Florian, or Dmitry, you know
such people?

Cheers,
Eric

8<-------------------------------------------------------
According to the Dell/Ubuntu driver, what was previously observed as "jumpy cursor"
corresponds to the hardware sending incorrect data for the first two reports of a 
one touch finger. So let's use the same workaround as in the other driver. Also,
detect another firmware version with the same behaviour, as in the other driver.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index b18862b..1c1d065 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -185,7 +185,7 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	struct elantech_data *etd = psmouse->private;
 	unsigned char *packet = psmouse->packet;
 	int fingers;
-	static int old_fingers;
+	static int one_finger_reports;
 
 	if (etd->fw_version < 0x020000) {
 		/*
@@ -203,11 +203,13 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	}
 
 	if (etd->jumpy_cursor) {
-		/* Discard packets that are likely to have bogus coordinates */
-		if (fingers > old_fingers) {
+		if ((fingers == 1) && (one_finger_reports < 2)) {
+			/* Discard first 2 reports of one finger, bogus */
+			one_finger_reports++;
 			elantech_debug("discarding packet\n");
-			goto discard_packet_v1;
-		}
+			return;
+		} else if (fingers != 1)
+			one_finger_reports = 0;
 	}
 
 	input_report_key(dev, BTN_TOUCH, fingers != 0);
@@ -238,9 +240,6 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	}
 
 	input_sync(dev);
-
- discard_packet_v1:
-	old_fingers = fingers;
 }
 
 /*
@@ -733,12 +732,12 @@ int elantech_init(struct psmouse *psmouse)
 	etd->capabilities = param[0];
 
 	/*
-	 * This firmware seems to suffer from misreporting coordinates when
+	 * This firmware suffers from misreporting coordinates when
 	 * a touch action starts causing the mouse cursor or scrolled page
 	 * to jump. Enable a workaround.
 	 */
-	if (etd->fw_version == 0x020022) {
-		pr_info("firmware version 2.0.34 detected, enabling jumpy cursor workaround\n");
+	if ((etd->fw_version == 0x020022) || (etd->fw_version == 0x020600)) {
+		pr_info("firmware version 2.0.34/2.6.0 detected, enabling jumpy cursor workaround\n");
 		etd->jumpy_cursor = 1;
 	}
 
-- 
1.7.1


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

Re: [NEEDS TEST][PATCH] elantech: discard the first 2 positions reports for some firmwares

[Chase Douglas]

On Sun, 2010-05-30 at 20:42 +0200, Éric Piel wrote:
> According to the Dell/Ubuntu driver, what was previously observed as "jumpy cursor"
> corresponds to the hardware sending incorrect data for the first two reports of a 
> one touch finger. So let's use the same workaround as in the other driver. Also,
> detect another firmware version with the same behaviour, as in the other driver.

I can't find this code in any of the Ubuntu released kernels. Where did
you find this patch?

Thanks,

-- Chase

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

Re: [NEEDS TEST][PATCH] elantech: discard the first 2 positions reports for some firmwares

[Dmitry Torokhov]

On Sun, May 30, 2010 at 02:11:54PM -0400, Chase Douglas wrote:
> On Sun, 2010-05-30 at 20:42 +0200, Éric Piel wrote:
> > According to the Dell/Ubuntu driver, what was previously observed as "jumpy cursor"
> > corresponds to the hardware sending incorrect data for the first two reports of a 
> > one touch finger. So let's use the same workaround as in the other driver. Also,
> > detect another firmware version with the same behaviour, as in the other driver.
> 
> I can't find this code in any of the Ubuntu released kernels. Where did
> you find this patch?

http://zinc.ubuntu.com/git?p=mid-team/hardy-netbook.git;a=commitdiff;h=dfc02dc860ccef79e7bce095c872548f914d96bf

Woudl be nice if we did not have to hunt through random repos to find
it...

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

Re: [NEEDS TEST][PATCH] elantech: discard the first 2 positions reports for some firmwares

[Chase Douglas]

On Sun, 2010-05-30 at 11:26 -0700, Dmitry Torokhov wrote:
> On Sun, May 30, 2010 at 02:11:54PM -0400, Chase Douglas wrote:
> > On Sun, 2010-05-30 at 20:42 +0200, Éric Piel wrote:
> > > According to the Dell/Ubuntu driver, what was previously observed as "jumpy cursor"
> > > corresponds to the hardware sending incorrect data for the first two reports of a 
> > > one touch finger. So let's use the same workaround as in the other driver. Also,
> > > detect another firmware version with the same behaviour, as in the other driver.
> > 
> > I can't find this code in any of the Ubuntu released kernels. Where did
> > you find this patch?
> 
> http://zinc.ubuntu.com/git?p=mid-team/hardy-netbook.git;a=commitdiff;h=dfc02dc860ccef79e7bce095c872548f914d96bf
> 
> Woudl be nice if we did not have to hunt through random repos to find
> it...

I completely agree. We are trying to consolidate our repos as much as
possible. The Hardy release was two years ago, and we admittedly had too
many repos with sources for different hardy kernels. We now have just
one repo for each of our recent kernel releases, and probably will have
only one master branch even for Maverick.

As for this patch, I still don't see it in the commit you linked to.
There's nothing in that version of the driver that discards packets,
checks firmware versions, or enables jumpy cursor logic.

If we did apply this patch somewhere and overlooked upstreaming it, then
that was a mistake on our part. We would also be just as concerned as
you are, since we don't seem to carry this patch in any Ubuntu supported
kernels that I am aware of.

Thanks,

-- Chase

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

Re: [NEEDS TEST][PATCH] elantech: discard the first 2 positions reports for some firmwares

[Dmitry Torokhov]

On Sun, May 30, 2010 at 02:55:32PM -0400, Chase Douglas wrote:
> On Sun, 2010-05-30 at 11:26 -0700, Dmitry Torokhov wrote:
> > On Sun, May 30, 2010 at 02:11:54PM -0400, Chase Douglas wrote:
> > > On Sun, 2010-05-30 at 20:42 +0200, Éric Piel wrote:
> > > > According to the Dell/Ubuntu driver, what was previously observed as "jumpy cursor"
> > > > corresponds to the hardware sending incorrect data for the first two reports of a 
> > > > one touch finger. So let's use the same workaround as in the other driver. Also,
> > > > detect another firmware version with the same behaviour, as in the other driver.
> > > 
> > > I can't find this code in any of the Ubuntu released kernels. Where did
> > > you find this patch?
> > 
> > http://zinc.ubuntu.com/git?p=mid-team/hardy-netbook.git;a=commitdiff;h=dfc02dc860ccef79e7bce095c872548f914d96bf
> > 
> > Woudl be nice if we did not have to hunt through random repos to find
> > it...
> 
> I completely agree. We are trying to consolidate our repos as much as
> possible. The Hardy release was two years ago, and we admittedly had too
> many repos with sources for different hardy kernels. We now have just
> one repo for each of our recent kernel releases, and probably will have
> only one master branch even for Maverick.
> 
> As for this patch, I still don't see it in the commit you linked to.
> There's nothing in that version of the driver that discards packets,
> checks firmware versions, or enables jumpy cursor logic.

The patch was not taken from the commit, Eric just referenced the commit
as he gathered some additional information. The code in that commit is
so hideous that it would never get close to mainline as is.

> 
> If we did apply this patch somewhere and overlooked upstreaming it, then
> that was a mistake on our part. We would also be just as concerned as
> you are, since we don't seem to carry this patch in any Ubuntu supported
> kernels that I am aware of.
> 
> Thanks,
> 
> -- Chase
> 

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

Re: [NEEDS TEST][PATCH] elantech: discard the first 2 positions reports for some firmwares

[Chase Douglas]

On Sun, 2010-05-30 at 13:12 -0700, Dmitry Torokhov wrote:
> The patch was not taken from the commit, Eric just referenced the commit
> as he gathered some additional information. The code in that commit is
> so hideous that it would never get close to mainline as is.

Ahhh, yes. Now I see what's going on. I thought, based on Eric's initial
email and his related email describing the protocol, the commit was
something we had in our tree that hadn't made it upstream. I should have
read through everything a second time more thoroughly :).

Thanks,

-- Chase