From patchwork Wed Jul 11 23:00:39 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Hilman X-Patchwork-Id: 1186131 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 64CCADF25A for ; Wed, 11 Jul 2012 23:05:03 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1Sp5u9-0007hs-J7; Wed, 11 Jul 2012 23:01:09 +0000 Received: from na3sys009aog130.obsmtp.com ([74.125.149.143]) by merlin.infradead.org with smtps (Exim 4.76 #1 (Red Hat Linux)) id 1Sp5u0-0007hS-C7 for linux-arm-kernel@lists.infradead.org; Wed, 11 Jul 2012 23:01:03 +0000 Received: from mail-pb0-f54.google.com ([209.85.160.54]) (using TLSv1) by na3sys009aob130.postini.com ([74.125.148.12]) with SMTP ID DSNKT/4Fp7KnJnfmsdAgb5uwajtwli03GM0J@postini.com; Wed, 11 Jul 2012 16:01:00 PDT Received: by pbbro2 with SMTP id ro2so3025202pbb.13 for ; Wed, 11 Jul 2012 16:00:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:organization:references:date:in-reply-to :message-id:user-agent:mime-version:content-type:x-gm-message-state; bh=bzvv1jNwMc71EtxS+XHE0+U38o6B2obobI9ckQC92WE=; b=f2FeMBr00rzALR8GxVLnaI1aXGIVytyOHj8RQFDAJY42XO7453zp8ojFYY+AXIFhJT LpZ+K0/9zr7DjBEfsl75BLtxeS4s6vDcVEqFkmUYsNXQyVZ2sn3fWwoQiAdlqSkVy8Jf hOhmWGW6F5vak/L545dkpZ9uhU9DddoBj2b9U9tsORhD5i8AxpHFNMBfqBfwLCuTkZ1t AuBA1fDgEDMddhm6bUX8c3fjiJBHrAL1FIE3RvQjRIB4JxXEJmWUuBHCymcqYc4P6ylU 8q7hBsfcet9ovq13gKaBPBYa05rt4HKQ1eAGz4WumFohpwXd7aGs9+rivEYiyQ0nC/Xw TjCw== Received: by 10.68.233.201 with SMTP id ty9mr156940pbc.34.1342047638373; Wed, 11 Jul 2012 16:00:38 -0700 (PDT) Received: from localhost (c-24-19-7-36.hsd1.wa.comcast.net. [24.19.7.36]) by mx.google.com with ESMTPS id se9sm2533498pbc.25.2012.07.11.16.00.36 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 11 Jul 2012 16:00:37 -0700 (PDT) From: Kevin Hilman To: Igor Grinberg Subject: Re: [PATCH] ARM: OMAP: fix the ads7846 init code Organization: Texas Instruments, Inc. References: <1339661781-5086-1-git-send-email-grinberg@compulab.co.il> Date: Wed, 11 Jul 2012 16:00:39 -0700 In-Reply-To: <1339661781-5086-1-git-send-email-grinberg@compulab.co.il> (Igor Grinberg's message of "Thu, 14 Jun 2012 11:16:21 +0300") Message-ID: <87sjcxhpig.fsf@ti.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) MIME-Version: 1.0 X-Gm-Message-State: ALoCoQn6ApgHYUZbpQONWEbYFievWMDqPDOrMBH4bf3RdVa9n63b72b5ZOiEfJshQNlUVrAq3Z62 X-Spam-Note: CRM114 invocation failed X-Spam-Score: -4.2 (----) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-4.2 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [74.125.149.143 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Tony Lindgren , Zumeng Chen , linux-omap@vger.kernel.org, Vaibhav Hiremath , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Hi Igor, Igor Grinberg writes: > In case a board provides the gpio_pendown and not board_pdata, > the GPIO debounce is not taken care of. > Fix this by taking care of GPIO debounce in any case. > > Signed-off-by: Igor Grinberg I just notice this this patch causing some faults in current l-o master branch. > --- > arch/arm/mach-omap2/common-board-devices.c | 22 ++++++++++++---------- > 1 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c > index 1706ebc..c187586 100644 > --- a/arch/arm/mach-omap2/common-board-devices.c > +++ b/arch/arm/mach-omap2/common-board-devices.c > @@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce, > struct spi_board_info *spi_bi = &ads7846_spi_board_info; > int err; > > - if (board_pdata && board_pdata->get_pendown_state) { > - err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown"); > - if (err) { > - pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err); > - return; > - } > - gpio_export(gpio_pendown, 0); > - > - if (gpio_debounce) > - gpio_set_debounce(gpio_pendown, gpio_debounce); > + err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown"); > + if (err) { > + pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err); > + return; > } > > + if (gpio_debounce) > + gpio_set_debounce(gpio_pendown, gpio_debounce); > + > spi_bi->bus_num = bus_num; > spi_bi->irq = gpio_to_irq(gpio_pendown); > > if (board_pdata) { > board_pdata->gpio_pendown = gpio_pendown; > spi_bi->platform_data = board_pdata; > + if (board_pdata->get_pendown_state) > + gpio_export(gpio_pendown, 0); > } else { > ads7846_config.gpio_pendown = gpio_pendown; > } > > + if (!board_pdata || (board_pdata && !board_pdata->get_pendown_state)) > + gpio_free(gpio_pendown); The logic here for freeing the GPIO doesn't make any sense to me. IIUC, the gpio_pendown is always used, so should not be freed. Moreover, if this GPIO is freed, that allows that GPIO bank to runtime suspend if there are no other GPIOs in use in that bank. So the first attempt to use the GPIO will fault. For example, on my Overo board(s), I noticed this failing as soon as teh ads7846 driver probes, requests the IRQ and the GPIO triggering is set. Getting rid of this free fixes the problem. The changelog doesn't describe why the GPIO is freed here so I'm not sure I follow, but it seems to me that once this GPIO is requesed, it should not be freed as long as it's used, otherwise PM faults can occur. Kevin From bb87c3b5586950e480d0699504997a9ad587fd85 Mon Sep 17 00:00:00 2001 From: Kevin Hilman Date: Wed, 11 Jul 2012 15:47:29 -0700 Subject: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO commit 97ee9f01d6 (ARM: OMAP: fix the ads7846 init code) mistakenly frees the pen-down GPIO even though it will be used by the ads7846 driver. Freeing a GPIO means that the GPIO bank containing that GPIO can be runtime suspended if its the last/only GPIO being used in that bank. If the GPIO bank is runtime suspended, any accesses to that bank will cause faults. Because the current code frees the GPIO, the ads7846 driver probe will fault when it requests its IRQ line. Because the IRQ is a GPIO line, the request IRQ will trickle down into the OMAP GPIO layer: gpio_irq_type() --> _set_gpio_triggering() which can fault if the bank has been runtime suspended. This is exctly what happens on Overo platforms (3530 Water, 3730 Overo FireSTORM) since this is the only GPIO used in the bank. To fix, don't free the GPIO at all since it is always in use. Cc: Igor Grinberg Signed-off-by: Kevin Hilman --- arch/arm/mach-omap2/common-board-devices.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/arm/mach-omap2/common-board-devices.c b/arch/arm/mach-omap2/common-board-devices.c index c187586..1ae6fd6 100644 --- a/arch/arm/mach-omap2/common-board-devices.c +++ b/arch/arm/mach-omap2/common-board-devices.c @@ -84,9 +84,6 @@ void __init omap_ads7846_init(int bus_num, int gpio_pendown, int gpio_debounce, ads7846_config.gpio_pendown = gpio_pendown; } - if (!board_pdata || (board_pdata && !board_pdata->get_pendown_state)) - gpio_free(gpio_pendown); - spi_register_board_info(&ads7846_spi_board_info, 1); } #else