From patchwork Wed Oct 23 08:21:23 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thierry Reding X-Patchwork-Id: 3086691 Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 196EA9F2B8 for ; Wed, 23 Oct 2013 08:24:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B2106202E5 for ; Wed, 23 Oct 2013 08:24:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F26B5202E8 for ; Wed, 23 Oct 2013 08:24:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751697Ab3JWIYA (ORCPT ); Wed, 23 Oct 2013 04:24:00 -0400 Received: from mail-bk0-f44.google.com ([209.85.214.44]:56830 "EHLO mail-bk0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751509Ab3JWIX6 (ORCPT ); Wed, 23 Oct 2013 04:23:58 -0400 Received: by mail-bk0-f44.google.com with SMTP id mz10so150992bkb.17 for ; Wed, 23 Oct 2013 01:23:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=Mq/ex5gzLq8ICCcCs0ecHoFTcm07OdtIc4GadZ4TuLc=; b=yBZozkfHxJ2qEJ2lBdwdOmafBI2l/cdNezmajq178l6beW2h506HXM+voG0kw8T3Dn 3r9FVNovFt+lqpIusNmmzPr/b6AwGwl5Khm0qX27kQ2/D9fHQoW1MCrpiRSXsJ5vZ2xp w31j3Dj7Ikq0FHZ3xUEB4t2rqeAKZ6qoI/zsMu8eYfL1Bo1ztCUbK/PPuCaM/fE8JhKh PdpNTwBiJ9nZo+Fb560pWqOqkfFZhyegli2q98eQTKTwIeVz1U7bDZZQSb8V+ofzMhxM Sqo6mSiB0SsJVWx5oxt4wgOIVnRMcKgwe8B8sVFhbQ+6XrtwSa21qfuBcnv5KHTs5MQQ hXTg== X-Received: by 10.204.2.140 with SMTP id 12mr184984bkj.47.1382516634822; Wed, 23 Oct 2013 01:23:54 -0700 (PDT) Received: from localhost (port-35855.pppoe.wtnet.de. [46.59.190.236]) by mx.google.com with ESMTPSA id b7sm15356163bkg.1.2013.10.23.01.23.53 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 23 Oct 2013 01:23:54 -0700 (PDT) Date: Wed, 23 Oct 2013 10:21:23 +0200 From: Thierry Reding To: Dmitry Torokhov Cc: Lothar =?utf-8?Q?Wa=C3=9Fmann?= , Mark Rutland , devicetree@vger.kernel.org, Sascha Hauer , Pawel Moll , Stephen Warren , Ian Campbell , Rob Herring , Denis Carikli , Eric =?utf-8?Q?B=C3=A9nard?= , linux-input@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support. Message-ID: <20131023082123.GC7404@ulmo.nvidia.com> References: <1382363667-10341-1-git-send-email-denis@eukrea.com> <20131021155927.GB4255@core.coreip.homeip.net> <20131022114947.30dc9c07@ipc1.ka-ro> <20131022223504.GA19819@core.coreip.homeip.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20131022223504.GA19819@core.coreip.homeip.net> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, T_TVD_MIME_EPI,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Oct 22, 2013 at 03:35:05PM -0700, Dmitry Torokhov wrote: > On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote: > > Hi, > > > > > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote: > > > > > > > > + if (ts->of) > > > > + return tsc2007_get_pendown_state_dt(ts); > > > > + > > > > if (!ts->get_pendown_state) > > > > return true; > > > > > > Instead of special casing "if (ts->of)" all over the place why don't you > > > set up the device structure as: > > > > > > if () > > > ts->get_pendown_state = tsc2007_get_pendown_state_dt; > > > > > > and be done with it? > > > > > I also thought about that, but the existing function does not have any > > parameters, while the DT version of get_pendown_state() requires to get > > the GPIO passed to it somehow. > > You can always have tsc2007_get_pendown_state_platform() wrapping the > call. Or we just go and fix board code. I used to have a patch which did exactly that but never got around to submitting it. Essentially what it did was add a void * parameter to the .get_pendown_state() and .clear_penirq() callbacks, along with a new .callback_data field that the driver could set. At the same time there was some code to unify code for boards that merely use a simple GPIO as pendown. I'm attaching what I think is the latest version. I no longer have access to the hardware so I can't test this, but perhaps it can serve as an example of how this could work. Sorry this isn't in the form of a proper patch. Thierry diff --git a/arch/arm/mach-imx/mach-cpuimx35.c b/arch/arm/mach-imx/mach-cpuimx35.c index d49b0ec..0dd8381 100644 --- a/arch/arm/mach-imx/mach-cpuimx35.c +++ b/arch/arm/mach-imx/mach-cpuimx35.c @@ -62,6 +62,7 @@ static int tsc2007_get_pendown_state(void) static struct tsc2007_platform_data tsc2007_info = { .model = 2007, .x_plate_ohms = 180, + .pendown_gpio = -1, .get_pendown_state = tsc2007_get_pendown_state, }; diff --git a/arch/arm/mach-imx/mach-cpuimx51sd.c b/arch/arm/mach-imx/mach-cpuimx51sd.c index b87cc49..ef2a7e6 100644 --- a/arch/arm/mach-imx/mach-cpuimx51sd.c +++ b/arch/arm/mach-imx/mach-cpuimx51sd.c @@ -134,6 +134,7 @@ static int tsc2007_get_pendown_state(void) static struct tsc2007_platform_data tsc2007_info = { .model = 2007, .x_plate_ohms = 180, + .pendown_gpio = -1, .get_pendown_state = tsc2007_get_pendown_state, }; diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c index b85957a..69abf30 100644 --- a/arch/arm/mach-shmobile/board-ap4evb.c +++ b/arch/arm/mach-shmobile/board-ap4evb.c @@ -1199,6 +1199,7 @@ static int ts_init(void) static struct tsc2007_platform_data tsc2007_info = { .model = 2007, .x_plate_ohms = 180, + .pendown_gpio = -1, .get_pendown_state = ts_get_pendown_state, .init_platform_hw = ts_init, }; diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c index 64559e8a..6cfd0ef 100644 --- a/arch/sh/boards/mach-ecovec24/setup.c +++ b/arch/sh/boards/mach-ecovec24/setup.c @@ -517,6 +517,7 @@ static int ts_init(void) static struct tsc2007_platform_data tsc2007_info = { .model = 2007, .x_plate_ohms = 180, + .pendown_gpio = -1, .get_pendown_state = ts_get_pendown_state, .init_platform_hw = ts_init, }; diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c index 1473d23..c87fdac 100644 --- a/drivers/input/touchscreen/tsc2007.c +++ b/drivers/input/touchscreen/tsc2007.c @@ -20,10 +20,15 @@ * published by the Free Software Foundation. */ +#define pr_fmt(fmt) "tsc2007: " fmt + #include #include #include #include +#include +#include +#include #include #include @@ -75,13 +80,16 @@ struct tsc2007 { unsigned long poll_delay; unsigned long poll_period; + int pendown_gpio; + int active_low; int irq; wait_queue_head_t wait; bool stopped; - int (*get_pendown_state)(void); - void (*clear_penirq)(void); + void *callback_data; + int (*get_pendown_state)(void *data); + void (*clear_penirq)(void *data); }; static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd) @@ -161,7 +169,7 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts) if (!ts->get_pendown_state) return true; - return ts->get_pendown_state(); + return ts->get_pendown_state(ts->callback_data); } static irqreturn_t tsc2007_soft_irq(int irq, void *handle) @@ -171,6 +179,13 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle) struct ts_event tc; u32 rt; + /* + * With some panels we need to wait a bit otherwise the first value + * is often wrong. + */ + if (ts->poll_delay > 0) + msleep(ts->poll_delay); + while (!ts->stopped && tsc2007_is_pen_down(ts)) { /* pen is down, continue with the measurement */ @@ -219,7 +234,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle) input_sync(input); if (ts->clear_penirq) - ts->clear_penirq(); + ts->clear_penirq(ts->callback_data); return IRQ_HANDLED; } @@ -228,11 +243,11 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle) { struct tsc2007 *ts = handle; - if (!ts->get_pendown_state || likely(ts->get_pendown_state())) + if (!ts->get_pendown_state || likely(ts->get_pendown_state(ts->callback_data))) return IRQ_WAKE_THREAD; if (ts->clear_penirq) - ts->clear_penirq(); + ts->clear_penirq(ts->callback_data); return IRQ_HANDLED; } @@ -273,6 +288,75 @@ static void tsc2007_close(struct input_dev *input_dev) tsc2007_stop(ts); } +static int tsc2007_get_pendown_state(void *data) +{ + struct tsc2007 *ts = data; + int ret = 0; + + ret = !!gpio_get_value(ts->pendown_gpio); + if (ts->active_low) + ret = !ret; + + return ret; +} + +static struct tsc2007_platform_data *tsc2007_parse_dt(struct device *dev) +{ + struct tsc2007_platform_data *pdata; + enum of_gpio_flags flags; + u32 value, values[3]; + int err; + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + err = of_get_named_gpio_flags(dev->of_node, "pendown-gpios", 0, + &flags); + if (err < 0) + return ERR_PTR(err); + + pdata->pendown_gpio = err; + + if (flags & OF_GPIO_ACTIVE_LOW) + pdata->active_low = true; + + err = of_property_read_u32(dev->of_node, "x-plate-ohms", &value); + if (err < 0) + return ERR_PTR(err); + + pdata->x_plate_ohms = value; + + err = of_property_read_u32(dev->of_node, "max-rt", &value); + if (err < 0) + return ERR_PTR(err); + + pdata->max_rt = value; + + err = of_property_read_u32(dev->of_node, "poll-delay", &value); + if (err < 0) + return ERR_PTR(err); + + pdata->poll_delay = value; + + err = of_property_read_u32(dev->of_node, "poll-period", &value); + if (err < 0) + return ERR_PTR(err); + + pdata->poll_period = value; + + err = of_property_read_u32_array(dev->of_node, "fuzz", values, + ARRAY_SIZE(values)); + if (err < 0) + return ERR_PTR(err); + + pdata->fuzzx = values[0]; + pdata->fuzzy = values[1]; + pdata->fuzzz = values[2]; + + return pdata; +} + static int __devinit tsc2007_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -281,18 +365,42 @@ static int __devinit tsc2007_probe(struct i2c_client *client, struct input_dev *input_dev; int err; + ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL); + if (!ts) + return -ENOMEM; + + if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) { + client->irq = irq_of_parse_and_map(client->dev.of_node, 0); + if (!client->irq) { + err = -EPROBE_DEFER; + goto err_free_mem; + } + } + if (!pdata) { - dev_err(&client->dev, "platform data is required!\n"); - return -EINVAL; + if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) { + pdata = tsc2007_parse_dt(&client->dev); + if (IS_ERR(pdata)) { + err = PTR_ERR(pdata); + goto err_free_mem; + } + + pdata->callback_data = ts; + } else { + dev_err(&client->dev, "platform data is required!\n"); + err = -EINVAL; + goto err_free_mem; + } } if (!i2c_check_functionality(client->adapter, - I2C_FUNC_SMBUS_READ_WORD_DATA)) - return -EIO; + I2C_FUNC_SMBUS_READ_WORD_DATA)) { + err = -EIO; + goto err_free_mem; + } - ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL); input_dev = input_allocate_device(); - if (!ts || !input_dev) { + if (!input_dev) { err = -ENOMEM; goto err_free_mem; } @@ -307,13 +415,27 @@ static int __devinit tsc2007_probe(struct i2c_client *client, ts->max_rt = pdata->max_rt ? : MAX_12BIT; ts->poll_delay = pdata->poll_delay ? : 1; ts->poll_period = pdata->poll_period ? : 1; + ts->callback_data = pdata->callback_data; ts->get_pendown_state = pdata->get_pendown_state; ts->clear_penirq = pdata->clear_penirq; if (pdata->x_plate_ohms == 0) { dev_err(&client->dev, "x_plate_ohms is not set up in platform data"); err = -EINVAL; - goto err_free_mem; + goto err_free_dev; + } + + if (gpio_is_valid(pdata->pendown_gpio)) { + err = gpio_request_one(pdata->pendown_gpio, GPIOF_IN, + "tsc2007"); + if (err < 0) + goto err_free_dev; + + ts->get_pendown_state = tsc2007_get_pendown_state; + ts->pendown_gpio = pdata->pendown_gpio; + ts->active_low = pdata->active_low; + } else { + ts->pendown_gpio = -EINVAL; } snprintf(ts->phys, sizeof(ts->phys), @@ -343,7 +465,7 @@ static int __devinit tsc2007_probe(struct i2c_client *client, IRQF_ONESHOT, client->dev.driver->name, ts); if (err < 0) { dev_err(&client->dev, "irq %d busy?\n", ts->irq); - goto err_free_mem; + goto err_free_gpio; } tsc2007_stop(ts); @@ -360,8 +482,12 @@ static int __devinit tsc2007_probe(struct i2c_client *client, free_irq(ts->irq, ts); if (pdata->exit_platform_hw) pdata->exit_platform_hw(); - err_free_mem: + err_free_gpio: + if (gpio_is_valid(pdata->pendown_gpio)) + gpio_free(pdata->pendown_gpio); + err_free_dev: input_free_device(input_dev); + err_free_mem: kfree(ts); return err; } @@ -373,6 +499,9 @@ static int __devexit tsc2007_remove(struct i2c_client *client) free_irq(ts->irq, ts); + if (gpio_is_valid(ts->pendown_gpio)) + gpio_free(ts->pendown_gpio); + if (pdata->exit_platform_hw) pdata->exit_platform_hw(); diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c index a447f4e..249d307 100644 --- a/drivers/mfd/timberdale.c +++ b/drivers/mfd/timberdale.c @@ -64,7 +64,8 @@ struct timberdale_device { static struct tsc2007_platform_data timberdale_tsc2007_platform_data = { .model = 2003, - .x_plate_ohms = 100 + .x_plate_ohms = 100, + .pendown_gpio = -1, }; static struct i2c_board_info timberdale_i2c_board_info[] = { diff --git a/include/linux/i2c/tsc2007.h b/include/linux/i2c/tsc2007.h index 506a9f7..8d72771 100644 --- a/include/linux/i2c/tsc2007.h +++ b/include/linux/i2c/tsc2007.h @@ -14,8 +14,12 @@ struct tsc2007_platform_data { int fuzzy; int fuzzz; - int (*get_pendown_state)(void); - void (*clear_penirq)(void); /* If needed, clear 2nd level + int pendown_gpio; + bool active_low; + + void *callback_data; + int (*get_pendown_state)(void *data); + void (*clear_penirq)(void *data); /* If needed, clear 2nd level interrupt source */ int (*init_platform_hw)(void); void (*exit_platform_hw)(void);