From patchwork Tue Sep 23 12:59:17 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Grygorii Strashko X-Patchwork-Id: 392489 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id E495D14009C for ; Tue, 23 Sep 2014 22:59:30 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755306AbaIWM7a (ORCPT ); Tue, 23 Sep 2014 08:59:30 -0400 Received: from arroyo.ext.ti.com ([192.94.94.40]:37896 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755295AbaIWM73 (ORCPT ); Tue, 23 Sep 2014 08:59:29 -0400 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id s8NCxK20003238; Tue, 23 Sep 2014 07:59:20 -0500 Received: from DLEE71.ent.ti.com (dlee71.ent.ti.com [157.170.170.114]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id s8NCxJdo018627; Tue, 23 Sep 2014 07:59:19 -0500 Received: from dflp32.itg.ti.com (10.64.6.15) by DLEE71.ent.ti.com (157.170.170.114) with Microsoft SMTP Server id 14.3.174.1; Tue, 23 Sep 2014 07:59:19 -0500 Received: from [192.168.192.66] (ileax41-snat.itg.ti.com [10.172.224.153]) by dflp32.itg.ti.com (8.14.3/8.13.8) with ESMTP id s8NCxIdY014370; Tue, 23 Sep 2014 07:59:18 -0500 Message-ID: <54216EA5.5010506@ti.com> Date: Tue, 23 Sep 2014 15:59:17 +0300 From: Grygorii Strashko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.1 MIME-Version: 1.0 To: "Wang, Yalin" , Linus Walleij CC: "gnurou@gmail.com" , "linux-gpio@vger.kernel.org" , "akpm@linux-foundation.org" Subject: Re: [PATCH] gpiolib:change to use irq_alloc_descs_from to alloc virqs References: <35FD53F367049845BC99AC72306C23D103CDBFBFB017@CNBJMBX05.corpusers.net>, <35FD53F367049845BC99AC72306C23D103D6DB4D6F29@CNBJMBX05.corpusers.net> In-Reply-To: <35FD53F367049845BC99AC72306C23D103D6DB4D6F29@CNBJMBX05.corpusers.net> Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org Hi Wang, On 09/23/2014 03:03 PM, Wang, Yalin wrote: > hi > > this is because , here: > > gpiochip->irqdomain = irq_domain_add_simple(of_node, > gpiochip->ngpio, first_irq, > &gpiochip_domain_ops, gpiochip); > > > irq_domain_add_simple() in this function, > if (first_irq > 0) { > if (IS_ENABLED(CONFIG_SPARSE_IRQ)) { > /* attempt to allocated irq_descs */ > int rc = irq_alloc_descs(first_irq, first_irq, size, > of_node_to_nid(of_node)); > if (rc < 0) > pr_info("Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n", > first_irq); > } > irq_domain_associate_many(domain, first_irq, 0, size); > } > > if first_irq > 0 , it will allocate it , > and make sure the return virq is equal to first_irq . > so we don't need allocate it again . Could provide a little bit more information about issue you've observed, pls? As for me, you patch will completely disable Sparse IRQ feature :( Also, I'm sure that struct gpio_chip->irq_base field can be removed from gpiolib irqchip code - GPIO drivers shouldn't use it also, because otherwise they will be incompatible with Sparse IRQ feature. Now the "irq_base" is used only twice in gpiolib code and below diff should allow to drop it completely from gpiolib code. not tested. > ________________________________________ > From: Linus Walleij [linus.walleij@linaro.org] > Sent: Tuesday, September 23, 2014 6:21 PM > To: Wang, Yalin > Cc: gnurou@gmail.com; linux-gpio@vger.kernel.org; akpm@linux-foundation.org > Subject: Re: [PATCH] gpiolib:change to use irq_alloc_descs_from to alloc virqs > > On Tue, Sep 9, 2014 at 9:12 AM, Wang, Yalin wrote: > >> this patch change use from irq_create_mapping to irq_alloc_descs_from, >> use irq_create_mapping to alloc virq one by one is not safe, >> it can't promise the allcated virqs are continuous, >> in stead, we use irq_alloc_descs_from() to alloc virqs in one time, >> so that the allocated virqs are in continuous bitmaps. >> >> Signed-off-by: Yalin Wang > > (...) > >> - for (offset = 0; offset < gpiochip->ngpio; offset++) { >> - irq_base = irq_create_mapping(gpiochip->irqdomain, offset); >> - if (offset == 0) >> - /* >> - * Store the base into the gpiochip to be used when >> - * unmapping the irqs. >> - */ >> - gpiochip->irq_base = irq_base; >> + if (first_irq > 0) { >> + gpiochip->irq_base = first_irq; > > Wait is this safe? Now you assume all descriptors are pre-allocated > and associated in this case, atleast explain what is going on. > >> + } else { >> + gpiochip->irq_base = irq_alloc_descs_from(1, gpiochip->ngpio, >> + of_node_to_nid(of_node)); >> + irq_domain_associate_many(gpiochip->irqdomain, >> + gpiochip->irq_base, 0, gpiochip->ngpio); > > This part looks OK. > > I'm holding this patch back until the above is clarified. > > Yours, > Linus Walleij-- > To unsubscribe from this list: send the line "unsubscribe linux-gpio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Best regards, -grygorii --- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 15cc0bb..81762ed 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -524,7 +524,7 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip) /* Remove all IRQ mappings and delete the domain */ if (gpiochip->irqdomain) { for (offset = 0; offset < gpiochip->ngpio; offset++) - irq_dispose_mapping(gpiochip->irq_base + offset); + irq_dispose_mapping(irq_find_mapping(gpiochip->irqdomain, offset)); irq_domain_remove(gpiochip->irqdomain); }