From patchwork Wed Jan 14 02:32:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yingjoe Chen X-Patchwork-Id: 5625351 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.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id ED919C058D for ; Wed, 14 Jan 2015 02:35:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DF68B20386 for ; Wed, 14 Jan 2015 02:35:09 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D26A3201F5 for ; Wed, 14 Jan 2015 02:35:08 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YBDlV-0001ca-Fv; Wed, 14 Jan 2015 02:33:01 +0000 Received: from [210.61.82.184] (helo=mailgw02.mediatek.com) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YBDlR-0001Zd-Sv for linux-arm-kernel@lists.infradead.org; Wed, 14 Jan 2015 02:32:59 +0000 X-Listener-Flag: 11101 Received: from mtkhts09.mediatek.inc [(172.21.101.70)] by mailgw02.mediatek.com (envelope-from ) (mhqrelay.mediatek.com ESMTP with TLS) with ESMTP id 1890644134; Wed, 14 Jan 2015 10:32:26 +0800 Received: from [172.21.77.4] (172.21.77.4) by mtkhts09.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 14.3.181.6; Wed, 14 Jan 2015 10:32:25 +0800 Subject: Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver. From: Yingjoe Chen To: Linus Walleij In-Reply-To: References: <1418772873-19747-1-git-send-email-hongzhou.yang@mediatek.com> <1418772873-19747-5-git-send-email-hongzhou.yang@mediatek.com> <1418807365.6443.17.camel@mtksdaap41> <1420535817.17567.10.camel@mtksdaap41> Date: Wed, 14 Jan 2015 10:32:25 +0800 Message-ID: <1421202745.27675.148.camel@mtksdaap41> MIME-Version: 1.0 X-Mailer: Evolution 2.28.3 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150113_183258_357980_F791781F X-CRM114-Status: GOOD ( 30.80 ) X-Spam-Score: 1.3 (+) Cc: Mark Rutland , "devicetree@vger.kernel.org" , Vladimir Murzin , Russell King , Pawel Moll , srv_heupstream@mediatek.com, Ian Campbell , Hongzhou Yang , Catalin Marinas , Ashwin Chaugule , "linux-kernel@vger.kernel.org" , Rob Herring , Grant Likely , maoguang.meng@mediatek.com, toby.liu@mediatek.com, Sascha Hauer , alan.cheng@mediatek.com, Matthias Brugger , Kumar Gala , dandan.he@mediatek.com, "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 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=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_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 Tue, 2015-01-13 at 14:24 +0100, Linus Walleij wrote: > On Tue, Jan 6, 2015 at 10:16 AM, Yingjoe Chen wrote: > > On Wed, 2014-12-17 at 17:09 +0800, Yingjoe Chen wrote: > >> On Wed, 2014-12-17 at 07:34 +0800, Hongzhou Yang wrote: > >> > From: Maoguang Meng > >> > > >> > MTK SoC support external interrupt(EINT) from most SoC pins. > >> > Add EINT support to pinctrl driver. > >> > > >> > Signed-off-by: Maoguang Meng > >> > Signed-off-by: Hongzhou Yang > >> > >> Hi Linus, > >> > >> This patch add EINT support to the pinctrl driver. We've surveyed > >> GPIOLIB_IRQCHIP, but we didn't use it because: > >> > >> - Not every GPIO pin support interrupt. > >> - EINT use a different numbering to GPIO. eg, from the mt8135 table, > >> GPIO29 is EINT158. It is more nature & efficient to use EINT number as > >> hwirq. > >> > >> + MTK_EINT_FUNCTION(2, 158), > >> + MTK_FUNCTION(0, "GPIO29"), > > > > After further looking into this, we could use GPIOLIB_IRQCHIP if we add > > an extension gpiochip_irqchip_add() to accept interrupt numbers and > > custom .to_irq function for our SoC. We could still reuse other code > > GPIOLIB_IRQCHIP provide. > > I see, and I still want to see all possibilities to centralize code > surveyed. > > If I understand correctly what you actually need is a linear > irqdomain with "holes" (invalid offsets) in it. > So this is what we should design for. > > The .to_irq() function should not really perform anything but a > simple lookup in the domain. > > What you do here: > > +static int mtk_gpio_to_irq(struct gpio_chip *chip, unsigned offset) > +{ > + const struct mtk_desc_pin *pin; > + struct mtk_pinctrl *pctl = dev_get_drvdata(chip->dev); > + int irq; > + > + pin = pctl->devdata->pins + offset; > + if (pin->eint.eintnum == NO_EINT_SUPPORT) > + return -EINVAL; > + > + irq = irq_find_mapping(pctl->domain, pin->eint.eintnum); > + if (!irq) > + return -EINVAL; > + > + return irq; > +} > > Is *avoiding* to translate some IRQs from a certain offset using > the domain, I think that is the wrong way to go. Hi Linus, Yes, it have holes and avoiding translate some gpio to irq, but it also using a different hwirq number to do the translate. Let's me describe my problem more clearly. On our SoC, if a pin support interrupt it will have 2 different numbers for it. For examples, here's a partial list for the gpio and EINT number mappings on mt8135: gpio EINT 0 49 1 48 ........... 36 97 37 19 ........... To control interrupt related function, we'll need EINT number to locate corresponding register bits. When interrupt occurs, the interrupt handler will know which EINT interrupt occurs. In irq_chip functions, only .irq_request_resources and .irq_release_resources use gpio number to set pinmux to EINT mode and all the others need EINT number. Because EINT number is used more frequently in interrupt related functions, it make sense to use EINT number as hwirq instead of gpio number. That means irq_domain will translate EINT number to virq. So what mtk_gpio_to_irq actually do is translate gpio number to EINT number and use irq domain to translate it to virq. Below is a draft of what I have in mind. For SoC that can use gpio number to control irq they still use gpiochip_irqchip_add(). For SoC that need to use another number to control irq, like us, can use gpiochip_irqchip_add_with_map. We can't reuse gpiochip_to_irq or gpiochip_irq_reqres/relres in GPIOLIB_IRQCHIP, but we can still reuse others code. Let me know if this is the direction you want. Thanks Joe.C unsigned int offset; @@ -604,15 +607,17 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip, gpiochip->irqchip = NULL; return -EINVAL; } - irqchip->irq_request_resources = gpiochip_irq_reqres; - irqchip->irq_release_resources = gpiochip_irq_relres; + if (!irqchip->irq_request_resources) { + irqchip->irq_request_resources = gpiochip_irq_reqres; + irqchip->irq_release_resources = gpiochip_irq_relres; + } /* * Prepare the mapping since the irqchip shall be orthogonal to * any gpiochip calls. If the first_irq was zero, this is * necessary to allocate descriptors for all IRQs. */ - for (offset = 0; offset < gpiochip->ngpio; offset++) { + for (offset = 0; offset < size; offset++) { irq_base = irq_create_mapping(gpiochip->irqdomain, offset); if (offset == 0) /* @@ -626,6 +631,18 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip, return 0; } + +int gpiochip_irqchip_add(struct gpio_chip *gpiochip, + struct irq_chip *irqchip, + unsigned int first_irq, + irq_flow_handler_t handler, + unsigned int type) +{ + return gpiochip_irqchip_add_with_map(gpiochip, irqchip, first_irq, + handler, type, + gpiochip->ngpiog, piochip_to_irq); +} ======================= --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -567,11 +567,14 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip) * the pins on the gpiochip can generate a unique IRQ. Everything else * need to be open coded. */ -int gpiochip_irqchip_add(struct gpio_chip *gpiochip, - struct irq_chip *irqchip, - unsigned int first_irq, - irq_flow_handler_t handler, - unsigned int type) +int gpiochip_irqchip_add_with_map(struct gpio_chip *gpiochip, + struct irq_chip *irqchip, + unsigned int first_irq, + irq_flow_handler_t handler, + unsigned int type, + unsigned int size, + int (*to_irq_func)(struct gpio_chip *chip, + unsigned offset)) { struct device_node *of_node;