diff mbox

[5/6] Input: elantech - clean up elantech_init

Message ID 1313632629-23603-6-git-send-email-jj_ding@emc.com.tw (mailing list archive)
State New, archived
Headers show

Commit Message

JJ Ding Aug. 18, 2011, 1:57 a.m. UTC
Group property setting code into elantech_set_properties.

Signed-off-by: JJ Ding <jj_ding@emc.com.tw>
---
 drivers/input/mouse/elantech.c |   69 ++++++++++++++++++++++-----------------
 1 files changed, 39 insertions(+), 30 deletions(-)

Comments

Daniel Kurtz Aug. 18, 2011, 3:04 a.m. UTC | #1
On Thu, Aug 18, 2011 at 9:57 AM, JJ Ding <jj_ding@emc.com.tw> wrote:
> Group property setting code into elantech_set_properties.
>
> Signed-off-by: JJ Ding <jj_ding@emc.com.tw>
> ---
>  drivers/input/mouse/elantech.c |   69 ++++++++++++++++++++++-----------------
>  1 files changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 7b9b6e5..ddd40eb 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -791,6 +791,42 @@ static int elantech_reconnect(struct psmouse *psmouse)
>  }
>
>  /*
> + * determine hardware version and set some properties according to it.
> + */
> +static void elantech_set_properties(struct elantech_data *etd)
> +{
> +       /*
> +        * Assume every version greater than 0x020030 is new EeePC style
> +        * hardware with 6 byte packets, except 0x020600
> +        */
> +       if (etd->fw_version < 0x020030 || etd->fw_version == 0x020600)
> +               etd->hw_version = 1;
> +       else
> +               etd->hw_version = 2;
> +
> +       /*
> +        * Turn on packet checking by default.
> +        */
> +       etd->paritycheck = 1;

Assuming paritycheck goes away:

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
--
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
Wanlong Gao Aug. 18, 2011, 3:08 a.m. UTC | #2
On 08/18/2011 11:04 AM, Daniel Kurtz wrote:
> On Thu, Aug 18, 2011 at 9:57 AM, JJ Ding<jj_ding@emc.com.tw>  wrote:
>> Group property setting code into elantech_set_properties.
>>
>> Signed-off-by: JJ Ding<jj_ding@emc.com.tw>
>> ---
>>   drivers/input/mouse/elantech.c |   69 ++++++++++++++++++++++-----------------
>>   1 files changed, 39 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
>> index 7b9b6e5..ddd40eb 100644
>> --- a/drivers/input/mouse/elantech.c
>> +++ b/drivers/input/mouse/elantech.c
>> @@ -791,6 +791,42 @@ static int elantech_reconnect(struct psmouse *psmouse)
>>   }
>>
>>   /*
>> + * determine hardware version and set some properties according to it.
>> + */
>> +static void elantech_set_properties(struct elantech_data *etd)
>> +{
>> +       /*
>> +        * Assume every version greater than 0x020030 is new EeePC style
>> +        * hardware with 6 byte packets, except 0x020600
>> +        */
>> +       if (etd->fw_version<  0x020030 || etd->fw_version == 0x020600)
>> +               etd->hw_version = 1;
>> +       else
>> +               etd->hw_version = 2;
>> +
>> +       /*
>> +        * Turn on packet checking by default.
>> +        */
>> +       etd->paritycheck = 1;
>
> Assuming paritycheck goes away:
Agree.
>
> Reviewed-by: Daniel Kurtz<djkurtz@chromium.org>
> --
> 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
>
JJ Ding Aug. 18, 2011, 5:35 a.m. UTC | #3
Hi Wanlong Gao, Daniel,

On Thu, 18 Aug 2011 11:08:08 +0800, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> On 08/18/2011 11:04 AM, Daniel Kurtz wrote:
> > On Thu, Aug 18, 2011 at 9:57 AM, JJ Ding<jj_ding@emc.com.tw>  wrote:
> >>   /*
> >> + * determine hardware version and set some properties according to it.
> >> + */
> >> +static void elantech_set_properties(struct elantech_data *etd)
> >> +{
> >> +       /*
> >> +        * Assume every version greater than 0x020030 is new EeePC style
> >> +        * hardware with 6 byte packets, except 0x020600
> >> +        */
> >> +       if (etd->fw_version<  0x020030 || etd->fw_version == 0x020600)
> >> +               etd->hw_version = 1;
> >> +       else
> >> +               etd->hw_version = 2;
> >> +
> >> +       /*
> >> +        * Turn on packet checking by default.
> >> +        */
> >> +       etd->paritycheck = 1;
> >
> > Assuming paritycheck goes away:
> Agree.
I thought about removing it, too. But it occured to me that v1 and v2
hardware can still have the sysfs entry to turn off parity check.

And since it's exposed in sysfs, I suppose there might be some init
scripts relying on it.

What do you think, Dmitry?
Shall I remove it?

Thanks,
jj

> >
> > Reviewed-by: Daniel Kurtz<djkurtz@chromium.org>
> > --
> > 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
> >
> 
> 
> -- 
> Thanks
> Wanlong Gao
--
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
Wanlong Gao Aug. 18, 2011, 5:38 a.m. UTC | #4
On 08/18/2011 01:35 PM, JJ Ding wrote:
> Hi Wanlong Gao, Daniel,
>
> On Thu, 18 Aug 2011 11:08:08 +0800, Wanlong Gao<gaowanlong@cn.fujitsu.com>  wrote:
>> On 08/18/2011 11:04 AM, Daniel Kurtz wrote:
>>> On Thu, Aug 18, 2011 at 9:57 AM, JJ Ding<jj_ding@emc.com.tw>   wrote:
>>>>    /*
>>>> + * determine hardware version and set some properties according to it.
>>>> + */
>>>> +static void elantech_set_properties(struct elantech_data *etd)
>>>> +{
>>>> +       /*
>>>> +        * Assume every version greater than 0x020030 is new EeePC style
>>>> +        * hardware with 6 byte packets, except 0x020600
>>>> +        */
>>>> +       if (etd->fw_version<   0x020030 || etd->fw_version == 0x020600)
>>>> +               etd->hw_version = 1;
>>>> +       else
>>>> +               etd->hw_version = 2;
>>>> +
>>>> +       /*
>>>> +        * Turn on packet checking by default.
>>>> +        */
>>>> +       etd->paritycheck = 1;
>>>
>>> Assuming paritycheck goes away:
>> Agree.
> I thought about removing it, too. But it occured to me that v1 and v2
> hardware can still have the sysfs entry to turn off parity check.
>
> And since it's exposed in sysfs, I suppose there might be some init
> scripts relying on it.
>
> What do you think, Dmitry?
> Shall I remove it?
>
> Thanks,
> jj

aha, maybe you can make the sysfs entry func to be noop, and mark it to 
be obsoleted, or just remove it?

Thanks
-Wanlong Gao

>
>>>
>>> Reviewed-by: Daniel Kurtz<djkurtz@chromium.org>
>>> --
>>> 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
>>>
>>
>>
>> --
>> Thanks
>> Wanlong Gao
> --
> 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
>

--
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
Dmitry Torokhov Aug. 18, 2011, 6 a.m. UTC | #5
On Thu, Aug 18, 2011 at 01:35:55PM +0800, JJ Ding wrote:
> Hi Wanlong Gao, Daniel,
> 
> On Thu, 18 Aug 2011 11:08:08 +0800, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> > On 08/18/2011 11:04 AM, Daniel Kurtz wrote:
> > > On Thu, Aug 18, 2011 at 9:57 AM, JJ Ding<jj_ding@emc.com.tw>  wrote:
> > >>   /*
> > >> + * determine hardware version and set some properties according to it.
> > >> + */
> > >> +static void elantech_set_properties(struct elantech_data *etd)
> > >> +{
> > >> +       /*
> > >> +        * Assume every version greater than 0x020030 is new EeePC style
> > >> +        * hardware with 6 byte packets, except 0x020600
> > >> +        */
> > >> +       if (etd->fw_version<  0x020030 || etd->fw_version == 0x020600)
> > >> +               etd->hw_version = 1;
> > >> +       else
> > >> +               etd->hw_version = 2;
> > >> +
> > >> +       /*
> > >> +        * Turn on packet checking by default.
> > >> +        */
> > >> +       etd->paritycheck = 1;
> > >
> > > Assuming paritycheck goes away:
> > Agree.
> I thought about removing it, too. But it occured to me that v1 and v2
> hardware can still have the sysfs entry to turn off parity check.
> 
> And since it's exposed in sysfs, I suppose there might be some init
> scripts relying on it.
> 
> What do you think, Dmitry?
> Shall I remove it?

No, we should not remove it, since it is useful for V1 hardware which we
still support.

How confident are we in the V2/V3 checking not tripping on valid packets?

Thanks.
JJ Ding Aug. 18, 2011, 7:44 a.m. UTC | #6
Hi Dmitry,

On Wed, 17 Aug 2011 23:00:38 -0700, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Thu, Aug 18, 2011 at 01:35:55PM +0800, JJ Ding wrote:
> > Hi Wanlong Gao, Daniel,
> > 
> > On Thu, 18 Aug 2011 11:08:08 +0800, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> > > On 08/18/2011 11:04 AM, Daniel Kurtz wrote:
> > > >
> > > > Assuming paritycheck goes away:
> > > Agree.
> > I thought about removing it, too. But it occured to me that v1 and v2
> > hardware can still have the sysfs entry to turn off parity check.
> > 
> > And since it's exposed in sysfs, I suppose there might be some init
> > scripts relying on it.
> > 
> > What do you think, Dmitry?
> > Shall I remove it?
> 
> No, we should not remove it, since it is useful for V1 hardware which we
> still support.
> 
> How confident are we in the V2/V3 checking not tripping on valid packets?
> 
> Thanks.
With V2 it should work reasonbaly well. Although I don't have test data,
I didn't encounter any problem turning paritycheck off when testing V2.

With V3 we use the check to distinguish first 2-finger packet and the
second one. So it's mandatory with V3.

Thanks
jj
> -- 
> 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
--
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
Éric Piel Aug. 19, 2011, 12:29 p.m. UTC | #7
Op 18-08-11 03:57, JJ Ding schreef:
> Group property setting code into elantech_set_properties.
>
> Signed-off-by: JJ Ding<jj_ding@emc.com.tw>
Looks good.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>

Éric
> ---
>   drivers/input/mouse/elantech.c |   69 ++++++++++++++++++++++-----------------
>   1 files changed, 39 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 7b9b6e5..ddd40eb 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -791,6 +791,42 @@ static int elantech_reconnect(struct psmouse *psmouse)
>   }
>
>   /*
> + * determine hardware version and set some properties according to it.
> + */
> +static void elantech_set_properties(struct elantech_data *etd)
> +{
> +	/*
> +	 * Assume every version greater than 0x020030 is new EeePC style
> +	 * hardware with 6 byte packets, except 0x020600
> +	 */
> +	if (etd->fw_version<  0x020030 || etd->fw_version == 0x020600)
> +		etd->hw_version = 1;
> +	else
> +		etd->hw_version = 2;
> +
> +	/*
> +	 * Turn on packet checking by default.
> +	 */
> +	etd->paritycheck = 1;
> +
> +	/*
> +	 * This firmware suffers from misreporting coordinates when
> +	 * a touch action starts causing the mouse cursor or scrolled page
> +	 * to jump. Enable a workaround.
> +	 */
> +	etd->jumpy_cursor =
> +		(etd->fw_version == 0x020022 || etd->fw_version == 0x020600);
> +
> +	if (etd->hw_version == 2) {
> +		/* For now show extra debug information */
> +		etd->debug = 1;
> +
> +		if (etd->fw_version>= 0x020800)
> +			etd->reports_pressure = true;
> +	}
> +}
> +
> +/*
>    * Initialize the touchpad and create sysfs entries
>    */
>   int elantech_init(struct psmouse *psmouse)
> @@ -816,26 +852,9 @@ int elantech_init(struct psmouse *psmouse)
>   	}
>
>   	etd->fw_version = (param[0]<<  16) | (param[1]<<  8) | param[2];
> -
> -	/*
> -	 * Assume every version greater than this is new EeePC style
> -	 * hardware with 6 byte packets
> -	 */
> -	if (etd->fw_version>= 0x020030) {
> -		etd->hw_version = 2;
> -		/* For now show extra debug information */
> -		etd->debug = 1;
> -		etd->paritycheck = 1;
> -
> -		if (etd->fw_version>= 0x020800)
> -			etd->reports_pressure = true;
> -
> -	} else {
> -		etd->hw_version = 1;
> -		etd->paritycheck = 1;
> -	}
> -
> -	pr_info("assuming hardware version %d, firmware version %d.%d.%d\n",
> +	elantech_set_properties(etd);
> +	pr_info("assuming hardware version %d "
> +		"(with firmware version 0x%02x%02x%02x)\n",
>   		etd->hw_version, param[0], param[1], param[2]);
>
>   	if (synaptics_send_cmd(psmouse, ETP_CAPABILITIES_QUERY,
> @@ -847,16 +866,6 @@ int elantech_init(struct psmouse *psmouse)
>   		etd->capabilities[0], etd->capabilities[1],
>   		etd->capabilities[2]);
>
> -	/*
> -	 * 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;

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

Patch

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 7b9b6e5..ddd40eb 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -791,6 +791,42 @@  static int elantech_reconnect(struct psmouse *psmouse)
 }
 
 /*
+ * determine hardware version and set some properties according to it.
+ */
+static void elantech_set_properties(struct elantech_data *etd)
+{
+	/*
+	 * Assume every version greater than 0x020030 is new EeePC style
+	 * hardware with 6 byte packets, except 0x020600
+	 */
+	if (etd->fw_version < 0x020030 || etd->fw_version == 0x020600)
+		etd->hw_version = 1;
+	else
+		etd->hw_version = 2;
+
+	/*
+	 * Turn on packet checking by default.
+	 */
+	etd->paritycheck = 1;
+
+	/*
+	 * This firmware suffers from misreporting coordinates when
+	 * a touch action starts causing the mouse cursor or scrolled page
+	 * to jump. Enable a workaround.
+	 */
+	etd->jumpy_cursor =
+		(etd->fw_version == 0x020022 || etd->fw_version == 0x020600);
+
+	if (etd->hw_version == 2) {
+		/* For now show extra debug information */
+		etd->debug = 1;
+
+		if (etd->fw_version >= 0x020800)
+			etd->reports_pressure = true;
+	}
+}
+
+/*
  * Initialize the touchpad and create sysfs entries
  */
 int elantech_init(struct psmouse *psmouse)
@@ -816,26 +852,9 @@  int elantech_init(struct psmouse *psmouse)
 	}
 
 	etd->fw_version = (param[0] << 16) | (param[1] << 8) | param[2];
-
-	/*
-	 * Assume every version greater than this is new EeePC style
-	 * hardware with 6 byte packets
-	 */
-	if (etd->fw_version >= 0x020030) {
-		etd->hw_version = 2;
-		/* For now show extra debug information */
-		etd->debug = 1;
-		etd->paritycheck = 1;
-
-		if (etd->fw_version >= 0x020800)
-			etd->reports_pressure = true;
-
-	} else {
-		etd->hw_version = 1;
-		etd->paritycheck = 1;
-	}
-
-	pr_info("assuming hardware version %d, firmware version %d.%d.%d\n",
+	elantech_set_properties(etd);
+	pr_info("assuming hardware version %d "
+		"(with firmware version 0x%02x%02x%02x)\n",
 		etd->hw_version, param[0], param[1], param[2]);
 
 	if (synaptics_send_cmd(psmouse, ETP_CAPABILITIES_QUERY,
@@ -847,16 +866,6 @@  int elantech_init(struct psmouse *psmouse)
 		etd->capabilities[0], etd->capabilities[1],
 		etd->capabilities[2]);
 
-	/*
-	 * 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;