From patchwork Wed Oct 19 21:03:41 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Gunthorpe X-Patchwork-Id: 9385415 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8CE3160762 for ; Wed, 19 Oct 2016 21:05:47 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 785C1283E1 for ; Wed, 19 Oct 2016 21:05:47 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6596F289AA; Wed, 19 Oct 2016 21:05:47 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9BA54283E1 for ; Wed, 19 Oct 2016 21:05:46 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bwy27-0000Oi-H6; Wed, 19 Oct 2016 21:04:19 +0000 Received: from quartz.orcorp.ca ([184.70.90.242]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bwy22-0000Me-OC for linux-arm-kernel@lists.infradead.org; Wed, 19 Oct 2016 21:04:15 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=obsidianresearch.com; s=rsa1; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=DSHrmqHFiXsp/uw3v7b7E9G7yDlEayCH1naww0w1Dfo=; b=XbGZoOtsO+fd0eaPRYMIMfwRrLUjLHO3zrhVWsc6ZCeJ4NyS8y3J9n6kqW7jX4U4HK0qtpjezdyg7WVkc5ahv4GsIpHJnNM3hGFZ2g+wpQxzIUkvvgNca4HfxRbm4RZdYHMR/4sMa9R0fCW6eeUG+f/eXwxOAEhNoYubwwS97Pg=; Received: from [10.0.0.151] (helo=jggl.edm.orcorp.ca) by quartz.orcorp.ca with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1bwy1V-00045F-O3; Wed, 19 Oct 2016 15:03:41 -0600 Received: from jgg by jggl.edm.orcorp.ca with local (Exim 4.84) (envelope-from ) id 1bwy1V-0007Jo-FO; Wed, 19 Oct 2016 15:03:41 -0600 Date: Wed, 19 Oct 2016 15:03:41 -0600 From: Jason Gunthorpe To: Gregory CLEMENT Subject: Re: [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()" Message-ID: <20161019210341.GA3746@obsidianresearch.com> References: <1476831396-24359-1-git-send-email-jgunthorpe@obsidianresearch.com> <871szcerxl.fsf@free-electrons.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <871szcerxl.fsf@free-electrons.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.151 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20161019_140414_911236_0BE92266 X-CRM114-Status: GOOD ( 27.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Thomas Petazzoni , Andrew Lunn , Linus Walleij , Grant Likely , linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Oct 19, 2016 at 09:09:10AM +0200, Gregory CLEMENT wrote: > On mer., oct. 19 2016, Jason Gunthorpe wrote: > > > This reverts commit ce931f571b6dcf8534e8740e8cd16565cf362536. > > > > The only difference betwen _simple and _legacy is that _simple > > calls irq_alloc_descs, however mvebu_gpio_probe already called > > irq_alloc_descs a few lines above. > > And what about removing the irq_alloc_descs ? I didn't think I had a test system for that complex work, but it turns out I do.. > Going back to use the _legacy version seems wrong for me. Both _legacy and _simple are described as deprecated, and using _simple is clearly the wrong choice for this driver, so I can't see how it is 'wrong' to go back. But it is legit to ask if the driver can be converted to use the modern style for setting up irq domains, so here is a patch that does that instead. I was only able to test LEVEL interrupts, but I think both kinds should be OK. From 7566f05ac445b652ba7607cc1899fed10fea1c76 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe Date: Wed, 19 Oct 2016 14:57:45 -0600 Subject: [PATCH] gpio/mvebu: Use irq_domain_add_linear This fixes the irq allocation in this driver to not print: irq: Cannot allocate irq_descs @ IRQ34, assuming pre-allocated irq: Cannot allocate irq_descs @ IRQ66, assuming pre-allocated Which happens because the driver already called irq_alloc_descs() and so the change to use irq_domain_add_simple resulted in calling irq_alloc_descs() twice. Modernize the irq allocation in this driver to use the irq_domain_add_linear flow directly and eliminate the use of irq_domain_add_simple/legacy Fixes: ce931f571b6d ("gpio/mvebu: convert to use irq_domain_add_simple()") Signed-off-by: Jason Gunthorpe --- drivers/gpio/gpio-mvebu.c | 92 ++++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 49 deletions(-) diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c index cd5dc27320a2..1ed6132b993c 100644 --- a/drivers/gpio/gpio-mvebu.c +++ b/drivers/gpio/gpio-mvebu.c @@ -293,10 +293,10 @@ static void mvebu_gpio_irq_ack(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; - u32 mask = ~(1 << (d->irq - gc->irq_base)); + u32 mask = d->mask; irq_gc_lock(gc); - writel_relaxed(mask, mvebu_gpioreg_edge_cause(mvchip)); + writel_relaxed(~mask, mvebu_gpioreg_edge_cause(mvchip)); irq_gc_unlock(gc); } @@ -305,7 +305,7 @@ static void mvebu_gpio_edge_irq_mask(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; struct irq_chip_type *ct = irq_data_get_chip_type(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); ct->mask_cache_priv &= ~mask; @@ -319,8 +319,7 @@ static void mvebu_gpio_edge_irq_unmask(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; struct irq_chip_type *ct = irq_data_get_chip_type(d); - - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); ct->mask_cache_priv |= mask; @@ -333,8 +332,7 @@ static void mvebu_gpio_level_irq_mask(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; struct irq_chip_type *ct = irq_data_get_chip_type(d); - - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); ct->mask_cache_priv &= ~mask; @@ -347,8 +345,7 @@ static void mvebu_gpio_level_irq_unmask(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; struct irq_chip_type *ct = irq_data_get_chip_type(d); - - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); ct->mask_cache_priv |= mask; @@ -462,7 +459,7 @@ static void mvebu_gpio_irq_handler(struct irq_desc *desc) for (i = 0; i < mvchip->chip.ngpio; i++) { int irq; - irq = mvchip->irqbase + i; + irq = irq_find_mapping(mvchip->domain, i); if (!(cause & (1 << i))) continue; @@ -655,6 +652,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev) struct irq_chip_type *ct; struct clk *clk; unsigned int ngpios; + bool have_irqs; int soc_variant; int i, cpu, id; int err; @@ -665,6 +663,9 @@ static int mvebu_gpio_probe(struct platform_device *pdev) else soc_variant = MVEBU_GPIO_SOC_VARIANT_ORION; + /* Some gpio controllers do not provide irq support */ + have_irqs = of_irq_count(np) != 0; + mvchip = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_gpio_chip), GFP_KERNEL); if (!mvchip) @@ -697,7 +698,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev) mvchip->chip.get = mvebu_gpio_get; mvchip->chip.direction_output = mvebu_gpio_direction_output; mvchip->chip.set = mvebu_gpio_set; - mvchip->chip.to_irq = mvebu_gpio_to_irq; + if (have_irqs) + mvchip->chip.to_irq = mvebu_gpio_to_irq; mvchip->chip.base = id * MVEBU_MAX_GPIO_PER_BANK; mvchip->chip.ngpio = ngpios; mvchip->chip.can_sleep = false; @@ -758,34 +760,30 @@ static int mvebu_gpio_probe(struct platform_device *pdev) devm_gpiochip_add_data(&pdev->dev, &mvchip->chip, mvchip); /* Some gpio controllers do not provide irq support */ - if (!of_irq_count(np)) + if (!have_irqs) return 0; - /* Setup the interrupt handlers. Each chip can have up to 4 - * interrupt handlers, with each handler dealing with 8 GPIO - * pins. */ - for (i = 0; i < 4; i++) { - int irq = platform_get_irq(pdev, i); - - if (irq < 0) - continue; - irq_set_chained_handler_and_data(irq, mvebu_gpio_irq_handler, - mvchip); - } - - mvchip->irqbase = irq_alloc_descs(-1, 0, ngpios, -1); - if (mvchip->irqbase < 0) { - dev_err(&pdev->dev, "no irqs\n"); - return mvchip->irqbase; + mvchip->domain = + irq_domain_add_linear(np, ngpios, &irq_generic_chip_ops, NULL); + if (!mvchip->domain) { + dev_err(&pdev->dev, "couldn't allocate irq domain %s (DT).\n", + mvchip->chip.label); + return -ENODEV; } - gc = irq_alloc_generic_chip("mvebu_gpio_irq", 2, mvchip->irqbase, - mvchip->membase, handle_level_irq); - if (!gc) { - dev_err(&pdev->dev, "Cannot allocate generic irq_chip\n"); - return -ENOMEM; + err = irq_alloc_domain_generic_chips( + mvchip->domain, ngpios, 2, np->name, handle_level_irq, + IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_LEVEL, 0, 0); + if (err) { + dev_err(&pdev->dev, "couldn't allocate irq chips %s (DT).\n", + mvchip->chip.label); + goto err_domain; } + /* NOTE: The common accessors cannot be used because of the percpu + * access to the mask registers + */ + gc = irq_get_domain_generic_chip(mvchip->domain, 0); gc->private = mvchip; ct = &gc->chip_types[0]; ct->type = IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW; @@ -803,27 +801,23 @@ static int mvebu_gpio_probe(struct platform_device *pdev) ct->handler = handle_edge_irq; ct->chip.name = mvchip->chip.label; - irq_setup_generic_chip(gc, IRQ_MSK(ngpios), 0, - IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE); + /* Setup the interrupt handlers. Each chip can have up to 4 + * interrupt handlers, with each handler dealing with 8 GPIO + * pins. + */ + for (i = 0; i < 4; i++) { + int irq = platform_get_irq(pdev, i); - /* Setup irq domain on top of the generic chip. */ - mvchip->domain = irq_domain_add_simple(np, mvchip->chip.ngpio, - mvchip->irqbase, - &irq_domain_simple_ops, - mvchip); - if (!mvchip->domain) { - dev_err(&pdev->dev, "couldn't allocate irq domain %s (DT).\n", - mvchip->chip.label); - err = -ENODEV; - goto err_generic_chip; + if (irq < 0) + continue; + irq_set_chained_handler_and_data(irq, mvebu_gpio_irq_handler, + mvchip); } return 0; -err_generic_chip: - irq_remove_generic_chip(gc, IRQ_MSK(ngpios), IRQ_NOREQUEST, - IRQ_LEVEL | IRQ_NOPROBE); - kfree(gc); +err_domain: + irq_domain_remove(mvchip->domain); return err; }