| Message ID | 1305534783-4917-1-git-send-email-thierry.reding@avionic-design.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4a1a42af0aba011e263098f107a2f45e0de2f279 |
| Headers | show |
On Mon, May 16, 2011 at 10:32:59AM +0200, Thierry Reding wrote: > When the controller signals a pen-down event via the platform-specific > GPIO, while the sample values indicate an invalid measurement, the > measurement needs to be repeated. > Would not we be interrupted again and take another sample then? Thanks.
[Cc'ing Kwangwoo via the alternate email address] * Dmitry Torokhov wrote: > On Mon, May 16, 2011 at 10:32:59AM +0200, Thierry Reding wrote: > > When the controller signals a pen-down event via the platform-specific > > GPIO, while the sample values indicate an invalid measurement, the > > measurement needs to be repeated. > > > > Would not we be interrupted again and take another sample then? Not necessarily. The problem is that if the pendown GPIO reports pendown, it doesn't necessarily mean that the pressure measurement will be valid. This is especially true if max_rt is configurable (as introduced by one of the follow-up patchs). What happens is that we are interrupted, check the GPIO to see that the pen is indeed down and then read the values and compute the pressure to see that it is invalid and we stop sampling. The TSC2007's nPEN_IRQ line never goes high again after that because the pen is still down (according to the GPIO). The comment in the old code of the (rt > max_rt) even says "[...] repeat at least once more the measurement", which the old code actually doesn't. Thierry
On Tue, May 17, 2011 at 08:27:16AM +0200, Thierry Reding wrote: > [Cc'ing Kwangwoo via the alternate email address] > > * Dmitry Torokhov wrote: > > On Mon, May 16, 2011 at 10:32:59AM +0200, Thierry Reding wrote: > > > When the controller signals a pen-down event via the platform-specific > > > GPIO, while the sample values indicate an invalid measurement, the > > > measurement needs to be repeated. > > > > > > > Would not we be interrupted again and take another sample then? > > Not necessarily. The problem is that if the pendown GPIO reports pendown, it > doesn't necessarily mean that the pressure measurement will be valid. This is > especially true if max_rt is configurable (as introduced by one of the > follow-up patchs). > > What happens is that we are interrupted, check the GPIO to see that the pen > is indeed down and then read the values and compute the pressure to see that > it is invalid and we stop sampling. The TSC2007's nPEN_IRQ line never goes > high again after that because the pen is still down (according to the GPIO). > > The comment in the old code of the (rt > max_rt) even says "[...] repeat at > least once more the measurement", which the old code actually doesn't. > I see. I am concerned with resubmitting work over and over when we do not have ts->get_pendown_state method.
* Dmitry Torokhov wrote: > On Tue, May 17, 2011 at 08:27:16AM +0200, Thierry Reding wrote: > > [Cc'ing Kwangwoo via the alternate email address] > > > > * Dmitry Torokhov wrote: > > > On Mon, May 16, 2011 at 10:32:59AM +0200, Thierry Reding wrote: > > > > When the controller signals a pen-down event via the platform-specific > > > > GPIO, while the sample values indicate an invalid measurement, the > > > > measurement needs to be repeated. > > > > > > > > > > Would not we be interrupted again and take another sample then? > > > > Not necessarily. The problem is that if the pendown GPIO reports pendown, it > > doesn't necessarily mean that the pressure measurement will be valid. This is > > especially true if max_rt is configurable (as introduced by one of the > > follow-up patchs). > > > > What happens is that we are interrupted, check the GPIO to see that the pen > > is indeed down and then read the values and compute the pressure to see that > > it is invalid and we stop sampling. The TSC2007's nPEN_IRQ line never goes > > high again after that because the pen is still down (according to the GPIO). > > > > The comment in the old code of the (rt > max_rt) even says "[...] repeat at > > least once more the measurement", which the old code actually doesn't. > > > > I see. I am concerned with resubmitting work over and over when we do > not have ts->get_pendown_state method. I hadn't thought about that case. But it seems as if that case should behave the same and stop rescheduling work as soon as no pressure is applied and rt becomes 0. Perhaps if I change this: debounced = true; into if (ts->get_pendown_state) debounced = true; it becomes more obvious that this "fix" only applies to the case where the GPIO and pressure measurement contradict each other. Thierry
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c index 80467f2..df4ae35 100644 --- a/drivers/input/touchscreen/tsc2007.c +++ b/drivers/input/touchscreen/tsc2007.c @@ -156,6 +156,7 @@ static void tsc2007_work(struct work_struct *work) { struct tsc2007 *ts = container_of(to_delayed_work(work), struct tsc2007, work); + bool debounced = false; struct ts_event tc; u32 rt; @@ -191,6 +192,7 @@ static void tsc2007_work(struct work_struct *work) * repeat at least once more the measurement. */ dev_dbg(&ts->client->dev, "ignored pressure %d\n", rt); + debounced = true; goto out; } @@ -225,7 +227,7 @@ static void tsc2007_work(struct work_struct *work) } out: - if (ts->pendown) + if (ts->pendown || debounced) schedule_delayed_work(&ts->work, msecs_to_jiffies(TS_POLL_PERIOD)); else
When the controller signals a pen-down event via the platform-specific GPIO, while the sample values indicate an invalid measurement, the measurement needs to be repeated. 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 | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)