From patchwork Mon Jul 29 09:39:48 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Javier Martinez Canillas X-Patchwork-Id: 2834867 Return-Path: X-Original-To: patchwork-linux-arm@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 0A9BCC0319 for ; Mon, 29 Jul 2013 09:40:24 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F2C0220212 for ; Mon, 29 Jul 2013 09:40:22 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 92B7C2020C for ; Mon, 29 Jul 2013 09:40:21 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1V3jwB-000094-JK; Mon, 29 Jul 2013 09:40:19 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1V3jw9-0008Mx-8q; Mon, 29 Jul 2013 09:40:17 +0000 Received: from bhuna.collabora.co.uk ([93.93.135.160]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1V3jw5-0008LG-1y for linux-arm-kernel@lists.infradead.org; Mon, 29 Jul 2013 09:40:14 +0000 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: javier) with ESMTPSA id A8841169875A Message-ID: <51F63864.9000601@collabora.co.uk> Date: Mon, 29 Jul 2013 11:39:48 +0200 From: Javier Martinez Canillas User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Linus Walleij Subject: Re: OMAP5912 boot broken by "gpio/omap: don't create an IRQ mapping for every GPIO on DT" References: <51F633AE.8090308@collabora.co.uk> In-Reply-To: <51F633AE.8090308@collabora.co.uk> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130729_054013_305501_34798637 X-CRM114-Status: GOOD ( 33.20 ) X-Spam-Score: -3.4 (---) Cc: Paul Walmsley , Kevin Hilman , Enric Balletbo i Serra , Grant Likely , Santosh Shilimkar , Linux-OMAP , Jean-Christophe PLAGNIOL-VILLARD , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 07/29/2013 11:19 AM, Javier Martinez Canillas wrote: > On 07/29/2013 11:01 AM, Linus Walleij wrote: >> Hi Paul, >> >> On Mon, Jul 29, 2013 at 9:52 AM, Paul Walmsley wrote: >> >>> your commit 0e970cec05635adbe7b686063e2548a8e4afb8f4 ("gpio/omap: don't >>> create an IRQ mapping for every GPIO on DT") breaks the boot on the >>> OMAP5912 OSK: >> >> I'm contemplating just reverting this whole series, as I didn't like >> the approach from the beginning and it has exploded in exactly >> the way I thought it would. >> >> If we revert these three patches: >> >> commit 949eb1a4d29dc75e0b5b16b03747886b52ecf854 >> "gpio/omap: fix build error when OF_GPIO is not defined." >> commit b4419e1a15905191661ffe75ba2f9e649f5d565e >> "gpio/omap: auto request GPIO as input if used as IRQ via DT" >> commit 0e970cec05635adbe7b686063e2548a8e4afb8f4 >> "gpio/omap: don't create an IRQ mapping for every GPIO on DT" >> >> Does the OMAP1 boot again after this? >> >> I think it's a way better idea to proceed with input-hogs on the gpiochip >> DT node and use that to get auto-request on the GPIO lines that >> will be used as IRQs only. >> >> Yours, >> Linus Walleij >> > > Hi Paul, > > I've looked at this and it seems that irq_create_mapping() does not call the > irq_domain_ops .map function handler since OMAP1 still uses legacy domain > mapping. I don't have an OMAP1 platform to test but could you please see if the > following patch [1] makes your OMAP1 platforms to boot again? > > But I agree with Linus and probably we should just go and revert the whole > series since it is very hard to get it right. In another thread a user reported > that this change also broke his DTS tree. > > I really tried to get this right without breaking anything but there are just > too many OMAP platforms behaving differently and most OMAP drivers are only half > converted to DT so this is really a can of worms. > > Thanks a lot and sorry for the inconvenience, > Javier > > [1]: > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index c57244e..f1c6da8 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -1090,8 +1090,18 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) > * are used as interrupts. > */ > if (!omap_gpio_chip_boot_dt(&bank->chip)) > - for (j = 0; j < bank->width; j++) > - irq_create_mapping(bank->domain, j); > + for (j = 0; j < bank->width; j++) { > + int irq = irq_create_mapping(bank->domain, j); > + irq_set_lockdep_class(irq, &gpio_lock_class); > + irq_set_chip_data(irq, bank); > + if (bank->is_mpuio) { > + omap_mpuio_alloc_gc(bank, irq, bank->width); > + } else { > + irq_set_chip_and_handler(irq, &gpio_irq_chip, > + handle_simple_irq); > + set_irq_flags(irq, IRQF_VALID); > + } > + } > irq_set_chained_handler(bank->irq, gpio_irq_handler); > irq_set_handler_data(bank->irq, bank); > In case this solves Paul issue, a cleaned patch with a commit message is [2]. But we should decide if is better to fix this or just drop the patches and go with Linus' input-hogs idea to do the GPIO auto request. Santosh, Kevin, Grant, what do you think we should do? Thanks a lot and best regards, Javier commit 486402aee8f31f067dacc6a83c4a3509fc9d3bfa Author: Javier Martinez Canillas Date: Mon Jul 29 11:03:43 2013 +0200 gpio/omap: setup IRQ in chip init instead .map for OMAP1 OMAP1 still uses the legacy domain mapping and was not converted to use the linear mapping like OMAP2+ platforms. So, irq_create_mapping() does not call the omap irq_domain_ops .map function handler and the GPIO-IRQ are not correctly initialized. Do the IRQ setup in omap_gpio_chip_init() for OMAP1 platforms to avoid this issue. Signed-off-by: Javier Martinez Canillas diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index c57244e..c6f7c56 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1053,6 +1053,7 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) { int j; static int gpio; + int irq = 0; /* * REVISIT eventually switch from OMAP-specific gpio structs @@ -1090,8 +1091,20 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) * are used as interrupts. */ if (!omap_gpio_chip_boot_dt(&bank->chip)) - for (j = 0; j < bank->width; j++) - irq_create_mapping(bank->domain, j); + for (j = 0; j < bank->width; j++) { + irq = irq_create_mapping(bank->domain, j); +#ifdef CONFIG_ARCH_OMAP1 + irq_set_lockdep_class(irq, &gpio_lock_class); + irq_set_chip_data(irq, bank); + if (bank->is_mpuio) { + omap_mpuio_alloc_gc(bank, irq, bank->width); + } else { + irq_set_chip_and_handler(irq, &gpio_irq_chip, + handle_simple_irq); + set_irq_flags(irq, IRQF_VALID); + } +#endif + } irq_set_chained_handler(bank->irq, gpio_irq_handler); irq_set_handler_data(bank->irq, bank); }