| Message ID | 1305534783-4917-2-git-send-email-thierry.reding@avionic-design.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Thierry, On Mon, May 16, 2011 at 10:33:00AM +0200, Thierry Reding wrote: > Finger touch events or very quick stylus events on low-quality panels > can cause the tsc2007 to read bogus values. Looking at oscilloscope > snapshots, this seems to be caused by the touch event disappearing > during the measurements. These bogus values result in misclicks, where > the X and Y values deviate from the real position. > > Most of these misclicks can be filtered out by setting a low enough > threshold for the maximum resistance (which is loosely the inverse of > the pressure) allowed to consider a set of values valid. Since this > behaviour is largely dependent on the type and quality of the panel, > this commit introduces the max_rt parameter. The default value is kept > at MAX_12BIT. I expect that the values, once selected, will not be changed for a given panel, so why don't we pass max_rt (and poll_delay and poll_interval) vi platform (board) data instead of being module parameters? Thanks.
* Dmitry Torokhov wrote: > Hi Thierry, > > On Mon, May 16, 2011 at 10:33:00AM +0200, Thierry Reding wrote: > > Finger touch events or very quick stylus events on low-quality panels > > can cause the tsc2007 to read bogus values. Looking at oscilloscope > > snapshots, this seems to be caused by the touch event disappearing > > during the measurements. These bogus values result in misclicks, where > > the X and Y values deviate from the real position. > > > > Most of these misclicks can be filtered out by setting a low enough > > threshold for the maximum resistance (which is loosely the inverse of > > the pressure) allowed to consider a set of values valid. Since this > > behaviour is largely dependent on the type and quality of the panel, > > this commit introduces the max_rt parameter. The default value is kept > > at MAX_12BIT. > > I expect that the values, once selected, will not be changed for a given > panel, so why don't we pass max_rt (and poll_delay and poll_interval) vi > platform (board) data instead of being module parameters? I was using module parameters because it allows the parameters to be tuned at runtime. You are correct however in that they don't vary after being selected once. I can resend an updated patch series with those parameters added to the platform data if you prefer. By the way, Kwangwoo's email address no longer seems to be valid. Searching the kernel tree, there also seems to be kwangwoo.lee@gmail.com. Do you know if it is current? Thierry
On Tue, May 17, 2011 at 07:46:20AM +0200, Thierry Reding wrote: > * Dmitry Torokhov wrote: > > Hi Thierry, > > > > On Mon, May 16, 2011 at 10:33:00AM +0200, Thierry Reding wrote: > > > Finger touch events or very quick stylus events on low-quality panels > > > can cause the tsc2007 to read bogus values. Looking at oscilloscope > > > snapshots, this seems to be caused by the touch event disappearing > > > during the measurements. These bogus values result in misclicks, where > > > the X and Y values deviate from the real position. > > > > > > Most of these misclicks can be filtered out by setting a low enough > > > threshold for the maximum resistance (which is loosely the inverse of > > > the pressure) allowed to consider a set of values valid. Since this > > > behaviour is largely dependent on the type and quality of the panel, > > > this commit introduces the max_rt parameter. The default value is kept > > > at MAX_12BIT. > > > > I expect that the values, once selected, will not be changed for a given > > panel, so why don't we pass max_rt (and poll_delay and poll_interval) vi > > platform (board) data instead of being module parameters? > > I was using module parameters because it allows the parameters to be tuned at > runtime. You are correct however in that they don't vary after being selected > once. I can resend an updated patch series with those parameters added to the > platform data if you prefer. Yes, please. > > By the way, Kwangwoo's email address no longer seems to be valid. Searching > the kernel tree, there also seems to be kwangwoo.lee@gmail.com. Do you know > if it is current? No, I do not.
* Dmitry Torokhov wrote: > Hi Thierry, > > On Mon, May 16, 2011 at 10:33:00AM +0200, Thierry Reding wrote: > > Finger touch events or very quick stylus events on low-quality panels > > can cause the tsc2007 to read bogus values. Looking at oscilloscope > > snapshots, this seems to be caused by the touch event disappearing > > during the measurements. These bogus values result in misclicks, where > > the X and Y values deviate from the real position. > > > > Most of these misclicks can be filtered out by setting a low enough > > threshold for the maximum resistance (which is loosely the inverse of > > the pressure) allowed to consider a set of values valid. Since this > > behaviour is largely dependent on the type and quality of the panel, > > this commit introduces the max_rt parameter. The default value is kept > > at MAX_12BIT. > > I expect that the values, once selected, will not be changed for a given > panel, so why don't we pass max_rt (and poll_delay and poll_interval) vi > platform (board) data instead of being module parameters? How do I best handle keeping the default values? Should I check whether the value has been set at all in the platform data and set the default otherwise? That would reserve the value 0 as special. Or should I just assume that all the values must be set explicitly? Thierry
On Tue, May 17, 2011 at 08:59:47AM +0200, Thierry Reding wrote: > * Dmitry Torokhov wrote: > > Hi Thierry, > > > > On Mon, May 16, 2011 at 10:33:00AM +0200, Thierry Reding wrote: > > > Finger touch events or very quick stylus events on low-quality panels > > > can cause the tsc2007 to read bogus values. Looking at oscilloscope > > > snapshots, this seems to be caused by the touch event disappearing > > > during the measurements. These bogus values result in misclicks, where > > > the X and Y values deviate from the real position. > > > > > > Most of these misclicks can be filtered out by setting a low enough > > > threshold for the maximum resistance (which is loosely the inverse of > > > the pressure) allowed to consider a set of values valid. Since this > > > behaviour is largely dependent on the type and quality of the panel, > > > this commit introduces the max_rt parameter. The default value is kept > > > at MAX_12BIT. > > > > I expect that the values, once selected, will not be changed for a given > > panel, so why don't we pass max_rt (and poll_delay and poll_interval) vi > > platform (board) data instead of being module parameters? > > How do I best handle keeping the default values? Should I check whether the > value has been set at all in the platform data and set the default otherwise? > That would reserve the value 0 as special. Or should I just assume that all > the values must be set explicitly? We need to try and keep being compatible with existing setups. Treating 0 as "unset" seems reasonable. Thanks.
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c index df4ae35..95d619c 100644 --- a/drivers/input/touchscreen/tsc2007.c +++ b/drivers/input/touchscreen/tsc2007.c @@ -83,6 +83,10 @@ struct tsc2007 { void (*clear_penirq)(void); }; +static int max_rt = MAX_12BIT; +module_param(max_rt, int, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(max_rt, "Maximum resistance above which samples are ignored"); + static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd) { s32 data; @@ -185,7 +189,7 @@ static void tsc2007_work(struct work_struct *work) tsc2007_read_values(ts, &tc); rt = tsc2007_calculate_pressure(ts, &tc); - if (rt > MAX_12BIT) { + if (rt > max_rt) { /* * Sample found inconsistent by debouncing or pressure is * beyond the maximum. Don't report it to user space,
Finger touch events or very quick stylus events on low-quality panels can cause the tsc2007 to read bogus values. Looking at oscilloscope snapshots, this seems to be caused by the touch event disappearing during the measurements. These bogus values result in misclicks, where the X and Y values deviate from the real position. Most of these misclicks can be filtered out by setting a low enough threshold for the maximum resistance (which is loosely the inverse of the pressure) allowed to consider a set of values valid. Since this behaviour is largely dependent on the type and quality of the panel, this commit introduces the max_rt parameter. The default value is kept at MAX_12BIT. Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Kwangwoo Lee <kwlee@mtekvision.com> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de> --- drivers/input/touchscreen/tsc2007.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)