From patchwork Wed Apr 23 14:50:58 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Javier Martinez Canillas X-Patchwork-Id: 4041891 Return-Path: X-Original-To: patchwork-linux-omap@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 7EF8BBFF02 for ; Wed, 23 Apr 2014 14:51:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7DAA6201F2 for ; Wed, 23 Apr 2014 14:51:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D8D08201ED for ; Wed, 23 Apr 2014 14:51:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754437AbaDWOvC (ORCPT ); Wed, 23 Apr 2014 10:51:02 -0400 Received: from mail-wi0-f179.google.com ([209.85.212.179]:43214 "EHLO mail-wi0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752912AbaDWOvA (ORCPT ); Wed, 23 Apr 2014 10:51:00 -0400 Received: by mail-wi0-f179.google.com with SMTP id z2so1285274wiv.0 for ; Wed, 23 Apr 2014 07:50:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=MQSXTi10FRkyvROIFuf/V+peD8GMo9RwDRFOduXzEXg=; b=FnS6Il8xgiIj+YSzgDryOKz7QDQ1ET8KTri0sG+H9hhsUqJvVUkvyrYc08pxzU2/Ab GuxPPZJ24CGUPXVyVHePXwdkGe5Ex55Bj5ZZ2Y9pKTSFUh/DkHxcnhXk9TZck31gEdm2 YOY/MvLhNKuPWq+OvPvQTv5fuWxMp4qVgU5K2TxpeyvcAOIxSrio5g+vCrjp+WlaJgst 79FfjklBJLoTOjIDn1lBzYNmGhyX38UIh4nQBdD20FJXZYTiqOWZwI277cv1s+PLYuL/ nIo3o9/5Iw9zPAVvl568zR5V94a9DzRJXFKF1RHvBovcP7934BSDxRlXxYbjMPChyX62 rx0A== X-Gm-Message-State: ALoCoQlU2BAOUOgcMH9GTSdDIDCSkjVUJVbCrgtwkIuk34yHrgIzycC/eGO49RNlUHhixSmy/ERq MIME-Version: 1.0 X-Received: by 10.194.203.170 with SMTP id kr10mr39363109wjc.19.1398264658688; Wed, 23 Apr 2014 07:50:58 -0700 (PDT) Received: by 10.180.8.33 with HTTP; Wed, 23 Apr 2014 07:50:58 -0700 (PDT) X-Originating-IP: [95.23.110.23] In-Reply-To: References: <53566C20.6000208@ti.com> <53568706.1050204@ti.com> <5356E5E4.1060509@ti.com> <5356F2A4.6010106@ti.com> <20140423013022.GA18648@kahuna> <5357C04F.3060902@ti.com> Date: Wed, 23 Apr 2014 16:50:58 +0200 Message-ID: Subject: Re: regressions in linux-next? From: Javier Martinez Canillas To: Linus Walleij Cc: Nishanth Menon , Tony Lindgren , Santosh Shilimkar , Kevin Hilman , linux-omap , Peter Ujfalusi Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, 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 Wed, Apr 23, 2014 at 4:38 PM, Linus Walleij wrote: > On Wed, Apr 23, 2014 at 3:29 PM, Nishanth Menon wrote: >> On 04/23/2014 08:01 AM, Linus Walleij wrote: > >>> What about this: >>> >>> if (chip->irq_default_type != IRQ_TYPE_NONE) >>> irq_set_irq_type(irq, chip->irq_default_type); >>> >>> This way you can pass IRQ_TYPE_NONE and nothing happens in >>> the mapping. >> >> What if these drivers depend on IRQ_TYPE_NONE to do something for the >> GPIO pins? > > Yeah :-( > >> would you expect these drivers to pass IRQ_TYPE_DEFAULT? > > Actually that sounds like a good idea. Maybe we can go over the few > drivers that pass IRQ_TYPE_NONE and see what they actually want. > There are not *too* many users of this call yet. > >> OR I wonder >> if we could pass some flag like -1 for platforms that dont care? > > The flags parameter to gpiochip_irqchip_add() is unsigned... > Switching to IRQ_TYPE_DEFAULT for drivers that want this is likely > the sane thing to do. > > Yours, > Linus Walleij I prefer to have an explicit way to tell gpiolib whether it has to set a default IRQ type or not than relying on passing magic numbers like -1. What do you think about the following patch? Although I agree that if we can use IRQ_TYPE_NONE as you propose then tht is a much better and efficient solution that this patch. Best regards, Javier From 8720c6798f86b71d8b11c57ecb7bae86a4ee656b Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Wed, 23 Apr 2014 16:45:05 +0200 Subject: [RFC] gpio: don't set IRQ default type unconditionally Creating a mapping for a GPIO pin into the Linux virtual IRQ space does not necessarily mean that the GPIO is going to be used as an interrupt line, it only mean that it could be used. But current gpiolib irqchip .map function handler calls to irq_set_irq_type() for each GPIO pin in the bank. Some drivers assume that this function is called either because a driver has explicitly requested an IRQ with request_irq() or that a DT device node has defined a GPIO controller phandle as its "interrupt-parent" and setups the chip accordingly. Others drivers have different semantics and expects that a irq_set_irq_type() would setup the hardware to be some default state. So is better to allow each driver to choose whether they want to set a default IRQ type on the mapping function or not. Signed-off-by: Javier Martinez Canillas --- drivers/gpio/gpio-omap.c | 1 + drivers/gpio/gpiolib.c | 3 ++- include/linux/gpio/driver.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 8cc9e91..2e23858 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1101,6 +1101,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank) gpio += bank->width; } bank->chip.ngpio = bank->width; + bank->chip.set_irq_default = false; ret = gpiochip_add(&bank->chip); if (ret) { diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index c12fe9d..f12aea3 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1402,7 +1402,8 @@ static int gpiochip_irq_map(struct irq_domain *d, unsigned int irq, #else irq_set_noprobe(irq); #endif - irq_set_irq_type(irq, chip->irq_default_type); + if (chip->set_irq_default) + irq_set_irq_type(irq, chip->irq_default_type); return 0; } diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 573e4f3..168c93e 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -113,6 +113,7 @@ struct gpio_chip { unsigned int irq_base; irq_flow_handler_t irq_handler; unsigned int irq_default_type; + bool set_irq_default; #endif #if defined(CONFIG_OF_GPIO)