From patchwork Tue Jul 12 09:22:17 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sekhar Nori X-Patchwork-Id: 967872 Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p6C9N6Pn015851 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 12 Jul 2011 09:23:27 GMT Received: from canuck.infradead.org ([2001:4978:20e::1]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QgZAr-0004mi-My; Tue, 12 Jul 2011 09:22:37 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QgZAr-0002Dc-0P; Tue, 12 Jul 2011 09:22:37 +0000 Received: from bear.ext.ti.com ([192.94.94.41]) by canuck.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QgZAn-0002DJ-N0 for linux-arm-kernel@lists.infradead.org; Tue, 12 Jul 2011 09:22:35 +0000 Received: from dbdp20.itg.ti.com ([172.24.170.38]) by bear.ext.ti.com (8.13.7/8.13.7) with ESMTP id p6C9MPD0008942 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 12 Jul 2011 04:22:27 -0500 Received: from dbde70.ent.ti.com (localhost [127.0.0.1]) by dbdp20.itg.ti.com (8.13.8/8.13.8) with ESMTP id p6C9MJus014864; Tue, 12 Jul 2011 14:52:20 +0530 (IST) Received: from dbde02.ent.ti.com ([172.24.170.145]) by dbde70.ent.ti.com ([172.24.170.148]) with mapi; Tue, 12 Jul 2011 14:52:19 +0530 From: "Nori, Sekhar" To: Ido Yariv , "davinci-linux-open-source@linux.davincidsp.com" , "linux-arm-kernel@lists.infradead.org" , "linux-mmc@vger.kernel.org" Date: Tue, 12 Jul 2011 14:52:17 +0530 Subject: RE: [PATCH v3 1/6] arm: davinci: Fix low level gpio irq handlers' argument Thread-Topic: [PATCH v3 1/6] arm: davinci: Fix low level gpio irq handlers' argument Thread-Index: AcxADg1dXjoecP3cSxK5m87jSApr+QAZbsjQ Message-ID: References: <1310418191-27009-1-git-send-email-ido@wizery.com> In-Reply-To: <1310418191-27009-1-git-send-email-ido@wizery.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: en-US MIME-Version: 1.0 X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110712_052233_883108_F6DA1B44 X-CRM114-Status: GOOD ( 27.91 ) X-Spam-Score: -2.3 (--) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (-2.3 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [192.94.94.41 listed in list.dnswl.org] Cc: Thomas Gleixner X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 12 Jul 2011 09:23:27 +0000 (UTC) Hi Ido, On Tue, Jul 12, 2011 at 02:33:11, Ido Yariv wrote: > Commit 7416401 ("arm: davinci: Fix fallout from generic irq chip > conversion") introduced a bug, causing low level interrupt handlers to > get a bogus irq number as an argument. The gpio irq handler falsely > assumes that the handler data is the irq base number and that is no > longer true. > > Set the irq handler data to be a pointer to the corresponding gpio > controller. The chained irq handler can then use it to extract both the > irq base number and the gpio registers structure. > > Signed-off-by: Ido Yariv > CC: Thomas Gleixner > --- > arch/arm/mach-davinci/gpio.c | 14 +++++++++++--- > 1 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mach-davinci/gpio.c b/arch/arm/mach-davinci/gpio.c > index e722139..7d64a07 100644 > --- a/arch/arm/mach-davinci/gpio.c > +++ b/arch/arm/mach-davinci/gpio.c > @@ -254,8 +254,10 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc) > { > struct davinci_gpio_regs __iomem *g; > u32 mask = 0xffff; > + struct davinci_gpio_controller *ctl; Lets call the variable "d" to be consistent with the rest of the file. > > - g = (__force struct davinci_gpio_regs __iomem *) irq_desc_get_handler_data(desc); > + ctl = (struct davinci_gpio_controller *)irq_desc_get_handler_data(desc); > + g = (struct davinci_gpio_regs __iomem *)ctl->regs; > > /* we only care about one bank */ > if (irq & 1) > @@ -278,7 +280,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc) > status >>= 16; > > /* now demux them to the right lowlevel handler */ > - n = (int)irq_get_handler_data(irq); > + n = ctl->irq_base; I realized that this breaks for odd banks as the status is right shifted by 16. The GPIO you are using must have been in even bank? > while (status) { > res = ffs(status); > n += res; > @@ -424,7 +426,13 @@ static int __init davinci_gpio_irq_setup(void) > > /* set up all irqs in this bank */ > irq_set_chained_handler(bank_irq, gpio_irq_handler); > - irq_set_handler_data(bank_irq, (__force void *)g); > + > + /* > + * Each chip handles 32 gpios, and each irq bank consists of 16 > + * gpio irqs. Pass the irq bank's corresponding controller to > + * the chained irq handler. > + */ > + irq_set_handler_data(bank_irq, &chips[bank * 16 / 32]); This can simply be: irq_set_handler_data(bank_irq, &chips[gpio / 32]); In the interest of time, I did these fixes and pushed the patch to "fixes" branch of git://gitorious.org/linux-davinci/linux-davinci.git Can you please test it out and let me know if it works. Updated patch also attached. Thanks, Sekhar 8<--------------------- From: Ido Yariv Subject: arm: davinci: Fix low level gpio irq handlers' argument Commit 7416401 ("arm: davinci: Fix fallout from generic irq chip conversion") introduced a bug, causing low level interrupt handlers to get a bogus irq number as an argument. The gpio irq handler falsely assumes that the handler data is the irq base number and that is no longer true. Set the irq handler data to be a pointer to the corresponding gpio controller. The chained irq handler can then use it to extract both the irq base number and the gpio registers structure. Signed-off-by: Ido Yariv CC: Thomas Gleixner [nsekhar@ti.com: renamed "ctl" to "d", simplified indexing logic for chips and took care of odd bank handling in irq handler] Signed-off-by: Sekhar Nori --- arch/arm/mach-davinci/gpio.c | 21 ++++++++++++++++----- 1 files changed, 16 insertions(+), 5 deletions(-) diff --git a/arch/arm/mach-davinci/gpio.c b/arch/arm/mach-davinci/gpio.c index e722139..cafbe13 100644 --- a/arch/arm/mach-davinci/gpio.c +++ b/arch/arm/mach-davinci/gpio.c @@ -254,8 +254,10 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc) { struct davinci_gpio_regs __iomem *g; u32 mask = 0xffff; + struct davinci_gpio_controller *d; - g = (__force struct davinci_gpio_regs __iomem *) irq_desc_get_handler_data(desc); + d = (struct davinci_gpio_controller *)irq_desc_get_handler_data(desc); + g = (struct davinci_gpio_regs __iomem *)d->regs; /* we only care about one bank */ if (irq & 1) @@ -274,11 +276,14 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc) if (!status) break; __raw_writel(status, &g->intstat); - if (irq & 1) - status >>= 16; /* now demux them to the right lowlevel handler */ - n = (int)irq_get_handler_data(irq); + n = d->irq_base; + if (irq & 1) { + n += 16; + status >>= 16; + } + while (status) { res = ffs(status); n += res; @@ -424,7 +429,13 @@ static int __init davinci_gpio_irq_setup(void) /* set up all irqs in this bank */ irq_set_chained_handler(bank_irq, gpio_irq_handler); - irq_set_handler_data(bank_irq, (__force void *)g); + + /* + * Each chip handles 32 gpios, and each irq bank consists of 16 + * gpio irqs. Pass the irq bank's corresponding controller to + * the chained irq handler. + */ + irq_set_handler_data(bank_irq, &chips[gpio / 32]); for (i = 0; i < 16 && gpio < ngpio; i++, irq++, gpio++) { irq_set_chip(irq, &gpio_irqchip);