From patchwork Mon May 4 14:57:00 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thierry Reding X-Patchwork-Id: 21718 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n44EuH6X010553 for ; Mon, 4 May 2009 14:56:18 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755811AbZEDO4B (ORCPT ); Mon, 4 May 2009 10:56:01 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756259AbZEDO4B (ORCPT ); Mon, 4 May 2009 10:56:01 -0400 Received: from moutng.kundenserver.de ([212.227.126.177]:60730 "EHLO moutng.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755803AbZEDOz7 (ORCPT ); Mon, 4 May 2009 10:55:59 -0400 Received: from benhur.adnet.avionic-design.de (p548EA526.dip0.t-ipconnect.de [84.142.165.38]) by mrelayeu.kundenserver.de (node=mrelayeu8) with ESMTP (Nemesis) id 0ML31I-1M0zaH3lh4-0000Oz; Mon, 04 May 2009 16:55:58 +0200 Received: from localhost (avionic-0027.adnet.avionic-design.de [172.20.13.142]) by benhur.adnet.avionic-design.de (Postfix) with ESMTP id 8B64F2C4015; Mon, 4 May 2009 16:55:56 +0200 (CEST) Date: Mon, 4 May 2009 16:57:00 +0200 From: Thierry Reding To: Trilok Soni Cc: Kwangwoo Lee , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org Subject: Re: [PATCH] input: Add support for the TSC2003 controller. Message-ID: <20090504145700.GA875@avionic-design.de> References: <1241006627-22811-1-git-send-email-thierry.reding@avionic-design.de> <5d5443650904290623n3712cd7dja653a139cff2cf6d@mail.gmail.com> <483a38b80904291835p51044741p9bd83d33a40031c@mail.gmail.com> <20090504113751.GA22603@avionic-design.de> <5d5443650905040503n501e3b47qc890dc4c71a85b1c@mail.gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5d5443650905040503n501e3b47qc890dc4c71a85b1c@mail.gmail.com> User-Agent: Mutt/1.5.18 (2008-05-17) X-Provags-ID: V01U2FsdGVkX196LO/vUo5NrvMIVjiEm7oSV0LHwMjIz9JKAIZ ulc4FMTNmAVyUxrGxnUOR9jjaQAvnaq7vVYj/py5M8T+kIWVFu 63qY/lkqkUivjgIgogJrw== Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org * Trilok Soni wrote: > Hi Thierry, > > On Mon, May 4, 2009 at 5:07 PM, Thierry Reding > wrote: > > * Kwangwoo Lee wrote: > >> Hi Thierry and Trilok, > >> > >> On Wed, Apr 29, 2009 at 10:23 PM, Trilok Soni wrote: > >> > Hi Thierry, > >> > > >> > I have added linux-omap community. How different is this chip from > >> > tsc2007. It looks to me that this chip is not much different from > >> > tsc2007 (this is just quick look at the driver). If they > >> > are similar please consider using i2c_device_id feature in tsc2007 to > >> > accommodate this chip. > >> > >> I agree with the Trilok's opinion. > > [snip] > > > > I only noticed the tsc2007 driver some time ago, when the tsc2003 was already > > finished (it's actually pretty old, I just never got around to submitting > > it). However I never got the tsc2007 to work on my platform because it uses > > sleeping functions inside the timer handler, which results in an oops right > > after the first touchscreen interrupt. > > > > I guess I could try and fix the tsc2007 properly instead of having a second, > > pretty similar driver in the tree. > > > > This could be because of improper locking? If you share a crash we can > have look at it. Attached is a patch that fixes things for me. The problem was that the I2C transfers were done in interrupt context which fails for the controller I use (PXA270). The attached patch uses a struct work_struct to schedule the I2C transfers so they are executed in non-interrupt context. I've tested the patch on a minimal system with tslib and it work fine with ts_calibrate and friends. Thierry From: Thierry Reding Subject: [PATCH] tsc2007: Fix for I2C controllers that sleep during transfers. This patch fixes the tsc2007 driver with I2C controllers that sleep during transfers. By moving the critical code to a workqueue, I2C transfers can be scheduled to run in non-interrupt context. Signed-off-by: Thierry Reding --- drivers/input/touchscreen/tsc2007.c | 25 ++++++++++++++++++------- 1 files changed, 18 insertions(+), 7 deletions(-) diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c index 4ab0702..6efb54d 100644 --- a/drivers/input/touchscreen/tsc2007.c +++ b/drivers/input/touchscreen/tsc2007.c @@ -70,6 +70,7 @@ struct ts_event { struct tsc2007 { struct input_dev *input; char phys[32]; + struct work_struct work; struct hrtimer timer; struct ts_event tc; @@ -173,6 +174,9 @@ static void tsc2007_send_event(void *tsc) dev_dbg(&ts->client->dev, "point(%4d,%4d), pressure (%4u)\n", x, y, rt); + } else { + if (!ts->pendown) + ts->pendown = 1; } hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_PERIOD), @@ -197,11 +201,19 @@ static int tsc2007_read_values(struct tsc2007 *tsc) return 0; } +static void tsc2007_work(struct work_struct *work) +{ + struct tsc2007 *ts = container_of(work, struct tsc2007, work); + tsc2007_read_values(ts); + tsc2007_send_event(ts); +} + static enum hrtimer_restart tsc2007_timer(struct hrtimer *handle) { struct tsc2007 *ts = container_of(handle, struct tsc2007, timer); + unsigned long flags = 0; - spin_lock_irq(&ts->lock); + spin_lock_irqsave(&ts->lock, flags); if (unlikely(!ts->get_pendown_state() && ts->pendown)) { struct input_dev *input = ts->input; @@ -217,12 +229,10 @@ static enum hrtimer_restart tsc2007_timer(struct hrtimer *handle) } else { /* pen is still down, continue with the measurement */ dev_dbg(&ts->client->dev, "pen is still down\n"); - - tsc2007_read_values(ts); - tsc2007_send_event(ts); + schedule_work(&ts->work); } - spin_unlock_irq(&ts->lock); + spin_unlock_irqrestore(&ts->lock, flags); return HRTIMER_NORESTART; } @@ -235,7 +245,7 @@ static irqreturn_t tsc2007_irq(int irq, void *handle) spin_lock_irqsave(&ts->lock, flags); if (likely(ts->get_pendown_state())) { - disable_irq(ts->irq); + disable_irq_nosync(ts->irq); hrtimer_start(&ts->timer, ktime_set(0, TS_POLL_DELAY), HRTIMER_MODE_REL); } @@ -252,7 +262,7 @@ static int tsc2007_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct tsc2007 *ts; - struct tsc2007_platform_data *pdata = pdata = client->dev.platform_data; + struct tsc2007_platform_data *pdata = client->dev.platform_data; struct input_dev *input_dev; int err; @@ -279,6 +289,7 @@ static int tsc2007_probe(struct i2c_client *client, hrtimer_init(&ts->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); ts->timer.function = tsc2007_timer; + INIT_WORK(&ts->work, tsc2007_work); spin_lock_init(&ts->lock); -- tg: (091438d..) adx/input/tsc2007 (depends on: adx/master)