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

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

[Éric Piel]

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

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

[Henrik Rydberg]

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 <rydberg@euromail.se>

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

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

[Dmitry Torokhov]

On Mon, May 16, 2011 at 01:15:52PM +0200, Henrik Rydberg wrote:
> 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 <rydberg@euromail.se>
> 
> on all six patches. Thank you, Éric.
> 

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

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

[Éric Piel]

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:
>> Hi Éric,
:
>>
>> 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.

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

[PATCH] elantech: clean up hardware/firmware version check

[Éric Piel]

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

Re: [PATCH] elantech: clean up hardware/firmware version check

[Henrik Rydberg]

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?

> 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 <rydberg@euromail.se>

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

Re: [PATCH] elantech: clean up hardware/firmware version check

[Éric Piel]

Op 04-07-11 22:30, Henrik Rydberg schreef:
> 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?
Not yet.

Dmitry, could you take this patch for elantech?

Cheers,
Éric

>
>> 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<rydberg@euromail.se>

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