[PATCH 0/6] elantech extend version support and add semi-mt

Hello,

Here is an update of patches which I had sent quite a few monthes ago. At this time they were a bit controversial because the hardware reports only the minimum/maximum positions when two fingers are pressed. With the "semi-mt" property from Henrik that should now be well handled.

The first few patches of this set should also add support for a few more hardware versions. I could not test all the hardware versions but it's based on the old Dell/Ubuntu driver and if it goes into Linus's tree during the merge window I'm confident we can catch and easily fix regressions :-)

Cheers,
Éric
--
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

Hi Éric,

Here is an update of patches which I had sent quite a few monthes
ago. At this time they were a bit controversial because the hardware
reports only the minimum/maximum positions when two fingers are
pressed. With the "semi-mt" property from Henrik that should now be
well handled.

The first few patches of this set should also add support for a few
more hardware versions. I could not test all the hardware versions
but it's based on the old Dell/Ubuntu driver and if it goes into
Linus's tree during the merge window I'm confident we can catch and
easily fix regressions :-)

Excellent work, thank you so much. A couple of tiny tiny details:

[patch 1]

+In the wild, there appear to be more versions, such as 04.03.01, 04.04.11. There
+appear to be almost no difference excepted the EF113 which do not report

no difference, except

[patch 3]

@@ -100,14 +100,22 @@ struct elantech_data {
        unsigned char reg_26;
        unsigned char debug;
        unsigned char capabilities;
-       bool paritycheck;
+       unsigned char paritycheck;
        bool jumpy_cursor;
+       unsigned char reports_pres :1;
        unsigned char hw_version;
        unsigned int fw_version;
        unsigned int single_finger_reports;
        unsigned char parity[256];
 };

Any particular reason to use unsigned char instead of bool here, and to
restrict report_pres to a single bit? Bool for both seems apt.

Other than the above, which do not really need a resend, feel free to add

    Reviewed-by: Henrik Rydberg [off-list ref]

on all six patches. Thank you, Éric.

Cheers,
Henrik
--
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

On Mon, May 16, 2011 at 01:15:52PM +0200, Henrik Rydberg wrote:

Eric,

Thank you for working on this. I applied patches 1 and 4-6; 3rd required
some changes but since they were minor I retained Henrik's reviewed-by.

I am not happy with the protocol checks from Dell, I am not sure they
make much sense; so I dropped patch 3 for now and because of that patch
2 did not make much sense either. Actually patch 2 did not make sense on
its own in any case as it should have been partially rolled into 3 and
partially into 4.

Thanks.

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

Hello,

Thanks to both of you for your comments.

Op 17-05-11 07:44, Dmitry Torokhov schreef:

On Mon, May 16, 2011 at 01:15:52PM +0200, Henrik Rydberg wrote:

quoted

Hi Éric,

:

quoted

Excellent work, thank you so much. A couple of tiny tiny details:

[patch 1]

+In the wild, there appear to be more versions, such as 04.03.01, 04.04.11. There
+appear to be almost no difference excepted the EF113 which do not report

no difference, except

Indeed... I'll try to fix it in another patch.

quoted

[patch 3]

@@ -100,14 +100,22 @@ struct elantech_data {
         unsigned char reg_26;
         unsigned char debug;
         unsigned char capabilities;
-       bool paritycheck;
+       unsigned char paritycheck;
         bool jumpy_cursor;
+       unsigned char reports_pres :1;
         unsigned char hw_version;
         unsigned int fw_version;
         unsigned int single_finger_reports;
         unsigned char parity[256];
  };

Any particular reason to use unsigned char instead of bool here, and to
restrict report_pres to a single bit? Bool for both seems apt.

paritycheck became a type (between 0-3), that's why it can't be bool. Anyway, this was dropped by Dmitry.

reports_pres would make much more sense as a bool, you are completely right!

:

Thank you for working on this. I applied patches 1 and 4-6; 3rd required
some changes but since they were minor I retained Henrik's reviewed-by.

I am not happy with the protocol checks from Dell, I am not sure they
make much sense; so I dropped patch 3 for now and because of that patch
2 did not make much sense either. Actually patch 2 did not make sense on
its own in any case as it should have been partially rolled into 3 and
partially into 4.

Thanks for tweaking the patches. I don't mind dropping patch 3. I had just implemented what was in the other driver, but I let you judge whether it's meaningful. At least on my hardware, I never noticed the detection of any error with them, anyway :-)

I'll try to update and resend patch 2.

See you,
Éric
--
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

Op 17-05-11 07:44, Dmitry Torokhov schreef:
:

I am not happy with the protocol checks from Dell, I am not sure they
make much sense; so I dropped patch 3 for now and because of that patch
2 did not make much sense either. Actually patch 2 did not make sense on
its own in any case as it should have been partially rolled into 3 and
partially into 4.

Hello,
Find below patch 2 rebased. Actually, as half of if is not useful
anymore (we gave up on checking the data for version 2) and you had
integrated the other half already, this doesn't contain a lot anymore.
So it ends up being just a clean up patch, mostly putting all the
version checks in the same place.

See you,
Eric


8<-----------------------------------------------------------------

According to the protocol document, there are a couple of different
versions of the hardware and firmware. Using the version number, it
should be possible to distinguish between them, at least for the
properties we care about. This moves all the version check together.

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

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 3250356..0d9c547 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c

@@ -757,15 +757,25 @@ int elantech_init(struct psmouse *psmouse)
 		etd->hw_version = 2;
 		/* For now show extra debug information */
 		etd->debug = 1;
-		/* Don't know how to do parity checking for version 2 */
-		etd->paritycheck = 0;
+		/* Version 2 doesn't contain control bits */
+		etd->paritycheck = false;
 
 		if (etd->fw_version >= 0x020800)
 			etd->reports_pressure = true;
-
+	} else if ((etd->fw_version == 0x020022) || (etd->fw_version == 0x020600)) {
+		/*
+		 * This firmware suffers from misreporting coordinates when
+		 * a touch action starts causing the mouse cursor or scrolled page
+		 * to jump. Enable a workaround.
+		 */
+		pr_info("firmware version 2.0.34/2.6.0 detected, enabling jumpy cursor workaround\n");
+		etd->jumpy_cursor = 1;
+		etd->debug = 1;
+		etd->hw_version = 1;
+		etd->paritycheck = true;
 	} else {
 		etd->hw_version = 1;
-		etd->paritycheck = 1;
+		etd->paritycheck = true;
 	}
 
 	pr_info("assuming hardware version %d, firmware version %d.%d.%d\n",

@@ -779,16 +789,6 @@ int elantech_init(struct psmouse *psmouse)
 		param[0], param[1], param[2]);
 	etd->capabilities = param[0];
 
-	/*
-	 * 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 || etd->fw_version == 0x020600) {
-		pr_info("firmware version 2.0.34/2.6.0 detected, enabling jumpy cursor workaround\n");
-		etd->jumpy_cursor = true;
-	}
-
 	if (elantech_set_absolute_mode(psmouse)) {
 		pr_err("failed to put touchpad into absolute mode.\n");
 		goto init_fail;

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

Hi Éric,

Find below patch 2 rebased. Actually, as half of if is not useful
anymore (we gave up on checking the data for version 2) and you had
integrated the other half already, this doesn't contain a lot anymore.
So it ends up being just a clean up patch, mostly putting all the
version checks in the same place.

Did you get any response to this one?

    Acked-by: Henrik Rydberg [off-list ref]

Thanks,
Henrik
--
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

Op 04-07-11 22:30, Henrik Rydberg schreef:

Hi Éric,

quoted

Find below patch 2 rebased. Actually, as half of if is not useful
anymore (we gave up on checking the data for version 2) and you had
integrated the other half already, this doesn't contain a lot anymore.
So it ends up being just a clean up patch, mostly putting all the
version checks in the same place.

Did you get any response to this one?

Not yet.

Dmitry, could you take this patch for elantech?

Cheers,
Éric

quoted

According to the protocol document, there are a couple of different
versions of the hardware and firmware. Using the version number, it
should be possible to distinguish between them, at least for the
properties we care about. This moves all the version check together.

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

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 3250356..0d9c547 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c

@@ -757,15 +757,25 @@ int elantech_init(struct psmouse *psmouse)
  		etd->hw_version = 2;
  		/* For now show extra debug information */
  		etd->debug = 1;
-		/* Don't know how to do parity checking for version 2 */
-		etd->paritycheck = 0;
+		/* Version 2 doesn't contain control bits */
+		etd->paritycheck = false;

  		if (etd->fw_version>= 0x020800)
  			etd->reports_pressure = true;
-
+	} else if ((etd->fw_version == 0x020022) || (etd->fw_version == 0x020600)) {
+		/*
+		 * This firmware suffers from misreporting coordinates when
+		 * a touch action starts causing the mouse cursor or scrolled page
+		 * to jump. Enable a workaround.
+		 */
+		pr_info("firmware version 2.0.34/2.6.0 detected, enabling jumpy cursor workaround\n");
+		etd->jumpy_cursor = 1;
+		etd->debug = 1;
+		etd->hw_version = 1;
+		etd->paritycheck = true;
  	} else {
  		etd->hw_version = 1;
-		etd->paritycheck = 1;
+		etd->paritycheck = true;
  	}

  	pr_info("assuming hardware version %d, firmware version %d.%d.%d\n",

@@ -779,16 +789,6 @@ int elantech_init(struct psmouse *psmouse)
  		param[0], param[1], param[2]);
  	etd->capabilities = param[0];

-	/*
-	 * 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 || etd->fw_version == 0x020600) {
-		pr_info("firmware version 2.0.34/2.6.0 detected, enabling jumpy cursor workaround\n");
-		etd->jumpy_cursor = true;
-	}
-
  	if (elantech_set_absolute_mode(psmouse)) {
  		pr_err("failed to put touchpad into absolute mode.\n");
  		goto init_fail;
--

1.7.5.1

     Acked-by: Henrik Rydberg[off-list ref]

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