* [PATCH v3] Input: synaptics - handle out of bounds values from the hardware
@ 2012-06-29 18:22 Seth Forshee
2012-07-16 22:18 ` Seth Forshee
2012-07-17 2:21 ` Daniel Kurtz
0 siblings, 2 replies; 6+ messages in thread
From: Seth Forshee @ 2012-06-29 18:22 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Daniel Kurtz
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 two's
compliment negative value of higher resolution than the 13-bit value
reported by the hardware, with the high-order bits being truncated. The
safest way to deal with this is to clamp any out of bounds values to the
minimum or maximum value for the axis.
Distinguishing between positive and negative is problematic, since we
lack definitive sign information in the value reported by the hardware.
The approach taken here is to split the difference between the maximum
legitimate value for the axis and the maximum possible value that the
hardware can report, treating values greater than this number as
negative and all other values as positive. This can be tweaked later if
hardware is found that operates outside of these parameters.
BugLink: http://bugs.launchpad.net/bugs/1001251
Cc: stable@vger.kernel.org
Cc: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
drivers/input/mouse/synaptics.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index c6d9869..698f1b8 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -40,6 +40,10 @@
* Note that newer firmware allows querying device for maximum useable
* coordinates.
*/
+#define XMIN 0
+#define XMAX 6143
+#define YMIN 0
+#define YMAX 6143
#define XMIN_NOMINAL 1472
#define XMAX_NOMINAL 5472
#define YMIN_NOMINAL 1408
@@ -555,6 +559,29 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
hw->right = (buf[0] & 0x02) ? 1 : 0;
}
+ /*
+ * Some hardware is known to wrap negative on the y axis,
+ * providing the low-order bits of a higher resolution negative
+ * number as a 13-bit unsigned value. No hardware is known to
+ * give out-of-bounds values on the high end, but since we're
+ * dealing with unspecified behavior we take a conservative
+ * approach to handling the out of bounds values.
+ *
+ * Clamp out of bounds values to the minimum or maximum value
+ * for the axis. Use the midpoint between the maximum axis
+ * value and maximum possible reported value as the dividing
+ * line between positive and negative values.
+ */
+ if (hw->x > ((1 << 13) + XMAX) / 2)
+ hw->x = XMIN;
+ else if (hw->x > XMAX)
+ hw->x = XMAX;
+
+ if (hw->y > ((1 << 13) + YMAX) / 2)
+ hw->y = YMIN;
+ else if (hw->y > YMAX)
+ hw->y = YMAX;
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] Input: synaptics - handle out of bounds values from the hardware
2012-06-29 18:22 [PATCH v3] Input: synaptics - handle out of bounds values from the hardware Seth Forshee
@ 2012-07-16 22:18 ` Seth Forshee
2012-07-17 2:21 ` Daniel Kurtz
1 sibling, 0 replies; 6+ messages in thread
From: Seth Forshee @ 2012-07-16 22:18 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Daniel Kurtz
Ping. Any objections to this version?
Thanks,
Seth
On Fri, Jun 29, 2012 at 01:22:18PM -0500, Seth Forshee wrote:
> 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 two's
> compliment negative value of higher resolution than the 13-bit value
> reported by the hardware, with the high-order bits being truncated. The
> safest way to deal with this is to clamp any out of bounds values to the
> minimum or maximum value for the axis.
>
> Distinguishing between positive and negative is problematic, since we
> lack definitive sign information in the value reported by the hardware.
> The approach taken here is to split the difference between the maximum
> legitimate value for the axis and the maximum possible value that the
> hardware can report, treating values greater than this number as
> negative and all other values as positive. This can be tweaked later if
> hardware is found that operates outside of these parameters.
>
> BugLink: http://bugs.launchpad.net/bugs/1001251
> Cc: stable@vger.kernel.org
> Cc: Daniel Kurtz <djkurtz@chromium.org>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
> drivers/input/mouse/synaptics.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index c6d9869..698f1b8 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -40,6 +40,10 @@
> * Note that newer firmware allows querying device for maximum useable
> * coordinates.
> */
> +#define XMIN 0
> +#define XMAX 6143
> +#define YMIN 0
> +#define YMAX 6143
> #define XMIN_NOMINAL 1472
> #define XMAX_NOMINAL 5472
> #define YMIN_NOMINAL 1408
> @@ -555,6 +559,29 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
> hw->right = (buf[0] & 0x02) ? 1 : 0;
> }
>
> + /*
> + * Some hardware is known to wrap negative on the y axis,
> + * providing the low-order bits of a higher resolution negative
> + * number as a 13-bit unsigned value. No hardware is known to
> + * give out-of-bounds values on the high end, but since we're
> + * dealing with unspecified behavior we take a conservative
> + * approach to handling the out of bounds values.
> + *
> + * Clamp out of bounds values to the minimum or maximum value
> + * for the axis. Use the midpoint between the maximum axis
> + * value and maximum possible reported value as the dividing
> + * line between positive and negative values.
> + */
> + if (hw->x > ((1 << 13) + XMAX) / 2)
> + hw->x = XMIN;
> + else if (hw->x > XMAX)
> + hw->x = XMAX;
> +
> + if (hw->y > ((1 << 13) + YMAX) / 2)
> + hw->y = YMIN;
> + else if (hw->y > YMAX)
> + hw->y = YMAX;
> +
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] Input: synaptics - handle out of bounds values from the hardware
2012-06-29 18:22 [PATCH v3] Input: synaptics - handle out of bounds values from the hardware Seth Forshee
2012-07-16 22:18 ` Seth Forshee
@ 2012-07-17 2:21 ` Daniel Kurtz
2012-07-24 19:31 ` [PATCH v4] " Seth Forshee
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Kurtz @ 2012-07-17 2:21 UTC (permalink / raw)
To: Seth Forshee; +Cc: linux-input, Dmitry Torokhov
On Sat, Jun 30, 2012 at 2:22 AM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> 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 two's
> compliment negative value of higher resolution than the 13-bit value
> reported by the hardware, with the high-order bits being truncated. The
> safest way to deal with this is to clamp any out of bounds values to the
> minimum or maximum value for the axis.
>
> Distinguishing between positive and negative is problematic, since we
> lack definitive sign information in the value reported by the hardware.
> The approach taken here is to split the difference between the maximum
> legitimate value for the axis and the maximum possible value that the
> hardware can report, treating values greater than this number as
> negative and all other values as positive. This can be tweaked later if
> hardware is found that operates outside of these parameters.
>
> BugLink: http://bugs.launchpad.net/bugs/1001251
> Cc: stable@vger.kernel.org
> Cc: Daniel Kurtz <djkurtz@chromium.org>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
> drivers/input/mouse/synaptics.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index c6d9869..698f1b8 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -40,6 +40,10 @@
> * Note that newer firmware allows querying device for maximum useable
> * coordinates.
> */
> +#define XMIN 0
> +#define XMAX 6143
> +#define YMIN 0
> +#define YMAX 6143
> #define XMIN_NOMINAL 1472
> #define XMAX_NOMINAL 5472
> #define YMIN_NOMINAL 1408
> @@ -555,6 +559,29 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
> hw->right = (buf[0] & 0x02) ? 1 : 0;
> }
>
> + /*
> + * Some hardware is known to wrap negative on the y axis,
> + * providing the low-order bits of a higher resolution negative
> + * number as a 13-bit unsigned value. No hardware is known to
> + * give out-of-bounds values on the high end, but since we're
> + * dealing with unspecified behavior we take a conservative
> + * approach to handling the out of bounds values.
> + *
> + * Clamp out of bounds values to the minimum or maximum value
> + * for the axis. Use the midpoint between the maximum axis
> + * value and maximum possible reported value as the dividing
> + * line between positive and negative values.
> + */
> + if (hw->x > ((1 << 13) + XMAX) / 2)
These are constants too, it might be clearer to define:
/*
* Values reported above these limits are considered "wrapped around
negative numbers",
* and are clipped to XMIN/YMIN.
*/
#define X_MAX_POSITIVE ((1 << 13) + XMAX) / 2)
#define Y_MAX_POSITIVE ((1 << 13) + YMAX) / 2)
> + hw->x = XMIN;
IMHO, though, I think it might be better to just report the negative
value rather than clamping. Clamping like this introduces a dead zone
on the trackpad, which leads to a degraded user experience on at least
one real, known, device, whereas clamping might prevent a potential
problem on other, unknown, devices. This doesn't seem like the best
tradeoff.
-Dan
> + else if (hw->x > XMAX)
> + hw->x = XMAX;
> +
> + if (hw->y > ((1 << 13) + YMAX) / 2)
> + hw->y = YMIN;
> + else if (hw->y > YMAX)
> + hw->y = YMAX;
> +
> return 0;
> }
>
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4] Input: synaptics - handle out of bounds values from the hardware
2012-07-17 2:21 ` Daniel Kurtz
@ 2012-07-24 19:31 ` Seth Forshee
2012-07-25 2:50 ` Daniel Kurtz
0 siblings, 1 reply; 6+ messages in thread
From: Seth Forshee @ 2012-07-24 19:31 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Daniel Kurtz
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 two's
compliment negative value of higher resolution than the 13-bit value
reported by the hardware, with the high-order bits being truncated. This
patch adds handling for these values by converting them to the
appropriate negative values.
The only tricky part about this is deciding when to treat a number as
negative. It stands to reason that if out of range values can be
reported on the low end then it could also happen on the high end, so
not all out of range values should be treated as negative. The approach
taken here is to split the difference between the maximum legitimate
value for the axis and the maximum possible value that the hardware can
report, treating values greater than this number as negative and all
other values as positive. This can be tweaked later if hardware is found
that operates outside of these parameters.
BugLink: http://bugs.launchpad.net/bugs/1001251
Cc: stable@vger.kernel.org
Cc: Daniel Kurtz <djkurtz@chromium.org>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
drivers/input/mouse/synaptics.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index c6d9869..ce345c5 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -40,11 +40,27 @@
* Note that newer firmware allows querying device for maximum useable
* coordinates.
*/
+#define XMIN 0
+#define XMAX 6143
+#define YMIN 0
+#define YMAX 6143
#define XMIN_NOMINAL 1472
#define XMAX_NOMINAL 5472
#define YMIN_NOMINAL 1408
#define YMAX_NOMINAL 4448
+/* Size in bits of absolute position values reported by the hardware */
+#define ABS_POS_BITS 13
+
+/*
+ * Any position values from the hardware above the following limits are
+ * treated as "wrapped around negative" values that have been truncated to
+ * the 13-bit reporting range of the hardware. These are just reasonable
+ * guesses and can be adjusted if hardware is found that operates outside
+ * of these parameters.
+ */
+#define X_MAX_POSITIVE (((1 << ABS_POS_BITS) + XMAX) / 2)
+#define Y_MAX_POSITIVE (((1 << ABS_POS_BITS) + YMAX) / 2)
/*****************************************************************************
* Stuff we need even when we do not want native Synaptics support
@@ -555,6 +571,12 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
hw->right = (buf[0] & 0x02) ? 1 : 0;
}
+ /* Convert wrap-around values to negative */
+ if (hw->x > X_MAX_POSITIVE)
+ hw->x = hw->x - (1 << ABS_POS_BITS);
+ if (hw->y > Y_MAX_POSITIVE)
+ hw->y = hw->y - (1 << ABS_POS_BITS);
+
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4] Input: synaptics - handle out of bounds values from the hardware
2012-07-24 19:31 ` [PATCH v4] " Seth Forshee
@ 2012-07-25 2:50 ` Daniel Kurtz
2012-07-25 13:47 ` Seth Forshee
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Kurtz @ 2012-07-25 2:50 UTC (permalink / raw)
To: Seth Forshee; +Cc: linux-input, Dmitry Torokhov
On Wed, Jul 25, 2012 at 3:31 AM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> 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 two's
> compliment negative value of higher resolution than the 13-bit value
> reported by the hardware, with the high-order bits being truncated. This
> patch adds handling for these values by converting them to the
> appropriate negative values.
>
> The only tricky part about this is deciding when to treat a number as
> negative. It stands to reason that if out of range values can be
> reported on the low end then it could also happen on the high end, so
> not all out of range values should be treated as negative. The approach
> taken here is to split the difference between the maximum legitimate
> value for the axis and the maximum possible value that the hardware can
> report, treating values greater than this number as negative and all
> other values as positive. This can be tweaked later if hardware is found
> that operates outside of these parameters.
>
> BugLink: http://bugs.launchpad.net/bugs/1001251
> Cc: stable@vger.kernel.org
> Cc: Daniel Kurtz <djkurtz@chromium.org>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
> drivers/input/mouse/synaptics.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index c6d9869..ce345c5 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -40,11 +40,27 @@
> * Note that newer firmware allows querying device for maximum useable
> * coordinates.
> */
> +#define XMIN 0
> +#define XMAX 6143
> +#define YMIN 0
> +#define YMAX 6143
> #define XMIN_NOMINAL 1472
> #define XMAX_NOMINAL 5472
> #define YMIN_NOMINAL 1408
> #define YMAX_NOMINAL 4448
>
> +/* Size in bits of absolute position values reported by the hardware */
> +#define ABS_POS_BITS 13
> +
> +/*
> + * Any position values from the hardware above the following limits are
> + * treated as "wrapped around negative" values that have been truncated to
> + * the 13-bit reporting range of the hardware. These are just reasonable
> + * guesses and can be adjusted if hardware is found that operates outside
> + * of these parameters.
> + */
> +#define X_MAX_POSITIVE (((1 << ABS_POS_BITS) + XMAX) / 2)
> +#define Y_MAX_POSITIVE (((1 << ABS_POS_BITS) + YMAX) / 2)
>
> /*****************************************************************************
> * Stuff we need even when we do not want native Synaptics support
> @@ -555,6 +571,12 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
> hw->right = (buf[0] & 0x02) ? 1 : 0;
> }
>
> + /* Convert wrap-around values to negative */
> + if (hw->x > X_MAX_POSITIVE)
> + hw->x = hw->x - (1 << ABS_POS_BITS);
Hi Seth,
Perhaps:
hw->x -= (1 << ABS_POS_BITS);
Either way, it looks good, thanks!
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> + if (hw->y > Y_MAX_POSITIVE)
> + hw->y = hw->y - (1 << ABS_POS_BITS);
> +
> return 0;
> }
>
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] Input: synaptics - handle out of bounds values from the hardware
2012-07-25 2:50 ` Daniel Kurtz
@ 2012-07-25 13:47 ` Seth Forshee
0 siblings, 0 replies; 6+ messages in thread
From: Seth Forshee @ 2012-07-25 13:47 UTC (permalink / raw)
To: Daniel Kurtz; +Cc: linux-input, Dmitry Torokhov
On Wed, Jul 25, 2012 at 10:50:15AM +0800, Daniel Kurtz wrote:
> > + /* Convert wrap-around values to negative */
> > + if (hw->x > X_MAX_POSITIVE)
> > + hw->x = hw->x - (1 << ABS_POS_BITS);
>
> Hi Seth,
>
> Perhaps:
> hw->x -= (1 << ABS_POS_BITS);
It makes no difference to me, either way it works the same. I don't
think I'll send another patch just to change this though, unless Dmitry
asks for it :)
> Either way, it looks good, thanks!
>
> Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
Thanks!
Seth
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-25 13:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-29 18:22 [PATCH v3] Input: synaptics - handle out of bounds values from the hardware Seth Forshee
2012-07-16 22:18 ` Seth Forshee
2012-07-17 2:21 ` Daniel Kurtz
2012-07-24 19:31 ` [PATCH v4] " Seth Forshee
2012-07-25 2:50 ` Daniel Kurtz
2012-07-25 13:47 ` Seth Forshee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).