elantech extend version support and add semi-mt

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

Hi Éric,

Quoted 11 lines

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]

(quoted diff) +2 -0

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

(quoted diff) +2 -1

@@ -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 <[email protected]>

on all six patches. Thank you, Éric.

Quoted 1 line

Hi Éric,

Quoted 9 lines

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

[patch 1]

(quoted diff) +2 -0

+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

Quoted 17 lines

[patch 3]

(quoted diff) +2 -1

@@ -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];
  };
 

Quoted 2 lines

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.

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.

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.

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.

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 <[email protected]>

---
 drivers/input/mouse/elantech.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

drivers/input/mouse/elantech.c +14 -14

index 3250356..0d9c547 100644
@@ -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;

Signature · 1 line

1.7.5.1

Hi Éric,

Quoted 5 lines

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?

Quoted 59 lines

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<[email protected]>

---
  drivers/input/mouse/elantech.c |   28 ++++++++++++++--------------
  1 files changed, 14 insertions(+), 14 deletions(-)

drivers/input/mouse/elantech.c +14 -14

index 3250356..0d9c547 100644
@@ -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;
-	}
-

Quoted 6 lines

drivers/input/mouse/elantech.c +0 -1

  	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<[email protected]>