[PATCH] Input: synaptics - reject out of range position values

[PATCH] Input: synaptics - reject out of range position values

[Seth Forshee]

The synaptics touchpad on the Acer Aspire One D250 will report y
coordinate values in excess of 8000 near the bottom of the touchpad,
well outside of the range of values that the synaptics documentation
says should be reported.

In addition, the y values abruptly change from very low values to these
very high values. After the calculation to invert the y coordinates,
userspace gets y coordinates that can jump suddenly between large
positive and moderately large negative values, causing sudden jumps in
the pointer position.

To work around this odd behavior, reject any coordinates with values
outside of absolute limits specified by Synaptics, which is 6143 for
both axes.

BugLink: http://bugs.launchpad.net/bugs/1001251
Cc: stable@vger.kernel.org
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
I'm not sure whether the limits here should be the hard-coded value or
the coordinates reported by the device when that's supported (i.e.
whether the coordinates reported by the device are always within the
range of 0 to 6143). If the latter is more appropriate I can modify the
patch to use the device-reported values and fall back to the hard-coded
values.

 drivers/input/mouse/synaptics.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index e07eb9b..f62299d 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -40,6 +40,8 @@
  * Note that newer firmware allows querying device for maximum useable
  * coordinates.
  */
+#define XMAX 6143
+#define YMAX 6143
 #define XMIN_NOMINAL 1472
 #define XMAX_NOMINAL 5472
 #define YMIN_NOMINAL 1408
@@ -555,6 +557,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 		hw->right = (buf[0] & 0x02) ? 1 : 0;
 	}
 
+	if (hw->x > XMAX || hw->y > YMAX)
+		return 1;
+
 	return 0;
 }
 
-- 
1.7.9.5

Re: [PATCH] Input: synaptics - reject out of range position values

[Seth Forshee]

On Fri, May 18, 2012 at 10:00:24AM -0500, Seth Forshee wrote:
> The synaptics touchpad on the Acer Aspire One D250 will report y
> coordinate values in excess of 8000 near the bottom of the touchpad,
> well outside of the range of values that the synaptics documentation
> says should be reported.
> 
> In addition, the y values abruptly change from very low values to these
> very high values. After the calculation to invert the y coordinates,
> userspace gets y coordinates that can jump suddenly between large
> positive and moderately large negative values, causing sudden jumps in
> the pointer position.
> 
> To work around this odd behavior, reject any coordinates with values
> outside of absolute limits specified by Synaptics, which is 6143 for
> both axes.
> 
> BugLink: http://bugs.launchpad.net/bugs/1001251
> Cc: stable@vger.kernel.org
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
> I'm not sure whether the limits here should be the hard-coded value or
> the coordinates reported by the device when that's supported (i.e.
> whether the coordinates reported by the device are always within the
> range of 0 to 6143). If the latter is more appropriate I can modify the
> patch to use the device-reported values and fall back to the hard-coded
> values.

Dmitry,

Is there someone else I should Cc who might know the answer?

Thanks,
Seth

> 
>  drivers/input/mouse/synaptics.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index e07eb9b..f62299d 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -40,6 +40,8 @@
>   * Note that newer firmware allows querying device for maximum useable
>   * coordinates.
>   */
> +#define XMAX 6143
> +#define YMAX 6143
>  #define XMIN_NOMINAL 1472
>  #define XMAX_NOMINAL 5472
>  #define YMIN_NOMINAL 1408
> @@ -555,6 +557,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>  		hw->right = (buf[0] & 0x02) ? 1 : 0;
>  	}
>  
> +	if (hw->x > XMAX || hw->y > YMAX)
> +		return 1;
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.9.5
> 

Re: [PATCH] Input: synaptics - reject out of range position values

[Seth Forshee]

Ping. Any problem with this patch?

On Fri, May 18, 2012 at 10:00:24AM -0500, Seth Forshee wrote:
> The synaptics touchpad on the Acer Aspire One D250 will report y
> coordinate values in excess of 8000 near the bottom of the touchpad,
> well outside of the range of values that the synaptics documentation
> says should be reported.
> 
> In addition, the y values abruptly change from very low values to these
> very high values. After the calculation to invert the y coordinates,
> userspace gets y coordinates that can jump suddenly between large
> positive and moderately large negative values, causing sudden jumps in
> the pointer position.
> 
> To work around this odd behavior, reject any coordinates with values
> outside of absolute limits specified by Synaptics, which is 6143 for
> both axes.
> 
> BugLink: http://bugs.launchpad.net/bugs/1001251
> Cc: stable@vger.kernel.org
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
> I'm not sure whether the limits here should be the hard-coded value or
> the coordinates reported by the device when that's supported (i.e.
> whether the coordinates reported by the device are always within the
> range of 0 to 6143). If the latter is more appropriate I can modify the
> patch to use the device-reported values and fall back to the hard-coded
> values.
> 
>  drivers/input/mouse/synaptics.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index e07eb9b..f62299d 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -40,6 +40,8 @@
>   * Note that newer firmware allows querying device for maximum useable
>   * coordinates.
>   */
> +#define XMAX 6143
> +#define YMAX 6143
>  #define XMIN_NOMINAL 1472
>  #define XMAX_NOMINAL 5472
>  #define YMIN_NOMINAL 1408
> @@ -555,6 +557,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>  		hw->right = (buf[0] & 0x02) ? 1 : 0;
>  	}
>  
> +	if (hw->x > XMAX || hw->y > YMAX)
> +		return 1;
> +
>  	return 0;
>  }
>  
> -- 
> 1.7.9.5
> 
> --
> 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] Input: synaptics - reject out of range position values

[Daniel Kurtz]

On Wed, Jun 6, 2012 at 11:35 PM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> Ping. Any problem with this patch?
>
> On Fri, May 18, 2012 at 10:00:24AM -0500, Seth Forshee wrote:
>> The synaptics touchpad on the Acer Aspire One D250 will report y
>> coordinate values in excess of 8000 near the bottom of the touchpad,
>> well outside of the range of values that the synaptics documentation
>> says should be reported.
>>
>> In addition, the y values abruptly change from very low values to these
>> very high values. After the calculation to invert the y coordinates,
>> userspace gets y coordinates that can jump suddenly between large
>> positive and moderately large negative values, causing sudden jumps in
>> the pointer position.
>>
>> To work around this odd behavior, reject any coordinates with values
>> outside of absolute limits specified by Synaptics, which is 6143 for
>> both axes.
>>
>> BugLink: http://bugs.launchpad.net/bugs/1001251
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>> ---
>> I'm not sure whether the limits here should be the hard-coded value or
>> the coordinates reported by the device when that's supported (i.e.
>> whether the coordinates reported by the device are always within the
>> range of 0 to 6143). If the latter is more appropriate I can modify the
>> patch to use the device-reported values and fall back to the hard-coded
>> values.

Hi Seth,

The patch seems to do what it says it is doing, but perhaps we can do
a little better?  The problem is, it would make fingers at the bottom
of the pad simply disappear, which may not be a great user experience,
either.

Do you have access to the hardware?
What is the y value reported at the top of the pad?
Also, what are the max & min values reported by your device when queried?
  Do they match the values that are actually reported?

Judging just from your comments, it sounds like the trackpad might be
wrapping around, and trying to report "negative" 13-bit numbers near
the bottom.

0x17FF = 6143
...
0x0000 = 0 = 0
0x1FFF = 8191 = -1
...
0x1F40 = 8000 = -192

Perhaps we can do this for your pad:
 y = (y + 0x0800) & 0x1FFF;

Which would make those negative values positive.

Or, alternatively, we could actually report negative values to
userspace in this case:

y = (y <= 6143) ? y : (y - 8192);

-Daniel

>>
>>  drivers/input/mouse/synaptics.c |    5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> index e07eb9b..f62299d 100644
>> --- a/drivers/input/mouse/synaptics.c
>> +++ b/drivers/input/mouse/synaptics.c
>> @@ -40,6 +40,8 @@
>>   * Note that newer firmware allows querying device for maximum useable
>>   * coordinates.
>>   */
>> +#define XMAX 6143
>> +#define YMAX 6143
>>  #define XMIN_NOMINAL 1472
>>  #define XMAX_NOMINAL 5472
>>  #define YMIN_NOMINAL 1408
>> @@ -555,6 +557,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>>               hw->right = (buf[0] & 0x02) ? 1 : 0;
>>       }
>>
>> +     if (hw->x > XMAX || hw->y > YMAX)
>> +             return 1;
>> +
>>       return 0;
>>  }
>>
>> --
>> 1.7.9.5
>>
>> --
>> 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
--
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] Input: synaptics - reject out of range position values

[Seth Forshee]

On Fri, Jun 08, 2012 at 12:56:39AM +0800, Daniel Kurtz wrote:
> On Wed, Jun 6, 2012 at 11:35 PM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > Ping. Any problem with this patch?
> >
> > On Fri, May 18, 2012 at 10:00:24AM -0500, Seth Forshee wrote:
> >> The synaptics touchpad on the Acer Aspire One D250 will report y
> >> coordinate values in excess of 8000 near the bottom of the touchpad,
> >> well outside of the range of values that the synaptics documentation
> >> says should be reported.
> >>
> >> In addition, the y values abruptly change from very low values to these
> >> very high values. After the calculation to invert the y coordinates,
> >> userspace gets y coordinates that can jump suddenly between large
> >> positive and moderately large negative values, causing sudden jumps in
> >> the pointer position.
> >>
> >> To work around this odd behavior, reject any coordinates with values
> >> outside of absolute limits specified by Synaptics, which is 6143 for
> >> both axes.
> >>
> >> BugLink: http://bugs.launchpad.net/bugs/1001251
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> >> ---
> >> I'm not sure whether the limits here should be the hard-coded value or
> >> the coordinates reported by the device when that's supported (i.e.
> >> whether the coordinates reported by the device are always within the
> >> range of 0 to 6143). If the latter is more appropriate I can modify the
> >> patch to use the device-reported values and fall back to the hard-coded
> >> values.
> 
> Hi Seth,
> 
> The patch seems to do what it says it is doing, but perhaps we can do
> a little better?  The problem is, it would make fingers at the bottom
> of the pad simply disappear, which may not be a great user experience,
> either.

What we're loosing is pretty minimal, see below. All the same, I'm open
to doing something different.

> Do you have access to the hardware?

Yes.

> What is the y value reported at the top of the pad?
> Also, what are the max & min values reported by your device when queried?
>   Do they match the values that are actually reported?

The largest y value I see reported is 5490.

> Judging just from your comments, it sounds like the trackpad might be
> wrapping around, and trying to report "negative" 13-bit numbers near
> the bottom.

I suspect this is the case. I see a range of values that would go down
to about -7. Obviously we can't simply treat it as a signed value
though, as that's going to cause values at the top to go negative.

> Perhaps we can do this for your pad:
>  y = (y + 0x0800) & 0x1FFF;
> 
> Which would make those negative values positive.

My concern with this is that it would shift the range relative to the
min/max y values we report, but I don't know how those are consumed so
I'm not sure what impact this might have. For instance, could this
negatively impact edge scrolling?

> Or, alternatively, we could actually report negative values to
> userspace in this case:
> 
> y = (y <= 6143) ? y : (y - 8192);

Well, they won't be negative anymore when userspace sees them ;)

But yeah, either of these should work. I don't feel like it's necessary
since this is apparently a one-off problem on aging hardware, and since
Windows also doesn't seem to be using the portion of the touchpad
reporting the out of range values (although humorously I can get the
pointer to jump sometimes under Windows too). Between the two
implementations you suggest I'd feel more comfortable with the latter,
as it should leave the values reported unchanged for other hardware.

I do have one question I'd still like to clear up. Am I correct in
interpreting the Synaptics documentation to say that the touchpads
should never report values greater than 6143, even for models that
report the maximum possible x/y values? That's what I think it's saying,
but I also feel like there's a little ambiguity on the point. Especially
since I'm already seeing values outside of the documented range :)

Thanks,
Seth

Re: [PATCH] Input: synaptics - reject out of range position values

[Seth Forshee]

On Thu, Jun 07, 2012 at 02:56:58PM -0500, Seth Forshee wrote:
> > What is the y value reported at the top of the pad?
> > Also, what are the max & min values reported by your device when queried?
> >   Do they match the values that are actually reported?
> 
> The largest y value I see reported is 5490.

Forgot to say, this touchpad doesn't support reporting the min and max
values.

Re: [PATCH] Input: synaptics - reject out of range position values

[Seth Forshee]

On Thu, Jun 07, 2012 at 02:56:58PM -0500, Seth Forshee wrote:
> > Or, alternatively, we could actually report negative values to
> > userspace in this case:
> > 
> > y = (y <= 6143) ? y : (y - 8192);
> 
> Well, they won't be negative anymore when userspace sees them ;)
> 
> But yeah, either of these should work. I don't feel like it's necessary
> since this is apparently a one-off problem on aging hardware, and since
> Windows also doesn't seem to be using the portion of the touchpad
> reporting the out of range values (although humorously I can get the
> pointer to jump sometimes under Windows too). Between the two
> implementations you suggest I'd feel more comfortable with the latter,
> as it should leave the values reported unchanged for other hardware.
> 
> I do have one question I'd still like to clear up. Am I correct in
> interpreting the Synaptics documentation to say that the touchpads
> should never report values greater than 6143, even for models that
> report the maximum possible x/y values? That's what I think it's saying,
> but I also feel like there's a little ambiguity on the point. Especially
> since I'm already seeing values outside of the documented range :)

The following is working fine for me, if you think this would be
preferable. The only downside I see is that if there are touchpads out
there that exceed 6143 then this is likely to cause them to start
behaving oddly.


>From 2664e38cbebe7e9de652d22d9bc408dd210e6c0b Mon Sep 17 00:00:00 2001
From: Seth Forshee <seth.forshee@canonical.com>
Date: Fri, 8 Jun 2012 10:19:39 -0500
Subject: [PATCH] Input: synaptics - handle negative values on the y axis

The touchpad on the Acer Aspire One D250 will report out of range values
in the extreme lower portion of the touchpad. These appear as abrupt
changes in the values reported by the hardware from very low values to
very high values, which can cause unexpected vertical jumps in the
position of the mouse pointer.

What seems to be happening is that the value is wrapping to a negative
13-bit value, which is being truncated to the 12-bit range reported in
the touchpad packets. In order to deal with this, convert values outside
the maximum y axis value specified by synaptics to the appropriate
negative value.

BugLink: http://bugs.launchpad.net/bugs/1001251
Cc: stable@vger.kernel.org
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/input/mouse/synaptics.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index e07eb9b..7dc1bd5 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -40,6 +40,7 @@
  * Note that newer firmware allows querying device for maximum useable
  * coordinates.
  */
+#define YMAX 6143
 #define XMIN_NOMINAL 1472
 #define XMAX_NOMINAL 5472
 #define YMIN_NOMINAL 1408
@@ -555,6 +556,15 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 		hw->right = (buf[0] & 0x02) ? 1 : 0;
 	}
 
+	/*
+	 * Some Synaptics touchpads have been known to wrap negative
+	 * on the y axis. These appear to be a 13-bit negative numbers
+	 * truncated to a 12-bit value. To work around this, treat any
+	 * value above the upper limit defined by Synaptics as actually
+	 * being a negative 13-bit value.
+	 */
+	hw->y = (hw->y <= YMAX) ? hw->y : (hw->y - 8192);
+
 	return 0;
 }
 
-- 
1.7.9.5

Re: [PATCH] Input: synaptics - reject out of range position values

[Daniel Kurtz]

On Sat, Jun 9, 2012 at 12:53 AM, Seth Forshee
<seth.forshee@canonical.com> wrote:
>
> On Thu, Jun 07, 2012 at 02:56:58PM -0500, Seth Forshee wrote:
> > > Or, alternatively, we could actually report negative values to
> > > userspace in this case:
> > >
> > > y = (y <= 6143) ? y : (y - 8192);
> >
> > Well, they won't be negative anymore when userspace sees them ;)
> >
> > But yeah, either of these should work. I don't feel like it's necessary
> > since this is apparently a one-off problem on aging hardware, and since
> > Windows also doesn't seem to be using the portion of the touchpad
> > reporting the out of range values (although humorously I can get the
> > pointer to jump sometimes under Windows too). Between the two
> > implementations you suggest I'd feel more comfortable with the latter,
> > as it should leave the values reported unchanged for other hardware.
> >
> > I do have one question I'd still like to clear up. Am I correct in
> > interpreting the Synaptics documentation to say that the touchpads
> > should never report values greater than 6143, even for models that
> > report the maximum possible x/y values? That's what I think it's saying,
> > but I also feel like there's a little ambiguity on the point. Especially
> > since I'm already seeing values outside of the documented range :)
>
> The following is working fine for me, if you think this would be
> preferable. The only downside I see is that if there are touchpads out
> there that exceed 6143 then this is likely to cause them to start
> behaving oddly.
>
>
> From 2664e38cbebe7e9de652d22d9bc408dd210e6c0b Mon Sep 17 00:00:00 2001
> From: Seth Forshee <seth.forshee@canonical.com>
> Date: Fri, 8 Jun 2012 10:19:39 -0500
> Subject: [PATCH] Input: synaptics - handle negative values on the y axis
>
> The touchpad on the Acer Aspire One D250 will report out of range values
> in the extreme lower portion of the touchpad. These appear as abrupt
> changes in the values reported by the hardware from very low values to
> very high values, which can cause unexpected vertical jumps in the
> position of the mouse pointer.
>
> What seems to be happening is that the value is wrapping to a negative
> 13-bit value, which is being truncated to the 12-bit range reported in
> the touchpad packets. In order to deal with this, convert values outside
> the maximum y axis value specified by synaptics to the appropriate
> negative value.
>
> BugLink: http://bugs.launchpad.net/bugs/1001251
> Cc: stable@vger.kernel.org
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>

> ---
>  drivers/input/mouse/synaptics.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/input/mouse/synaptics.c
> b/drivers/input/mouse/synaptics.c
> index e07eb9b..7dc1bd5 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -40,6 +40,7 @@
>  * Note that newer firmware allows querying device for maximum useable
>  * coordinates.
>  */
> +#define YMAX 6143
>  #define XMIN_NOMINAL 1472
>  #define XMAX_NOMINAL 5472
>  #define YMIN_NOMINAL 1408
> @@ -555,6 +556,15 @@ static int synaptics_parse_hw_state(const unsigned
> char buf[],
>                hw->right = (buf[0] & 0x02) ? 1 : 0;
>        }
>
> +       /*
> +        * Some Synaptics touchpads have been known to wrap negative
> +        * on the y axis. These appear to be a 13-bit negative numbers
> +        * truncated to a 12-bit value. To work around this, treat any
> +        * value above the upper limit defined by Synaptics as actually
> +        * being a negative 13-bit value.
> +        */
> +       hw->y = (hw->y <= YMAX) ? hw->y : (hw->y - 8192);
> +
>        return 0;
>  }
>
> --
> 1.7.9.5
>
--
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