From: "Nori, Sekhar" <nsekhar@ti.com> To: Ido Yariv <ido@wizery.com>, "davinci-linux-open-source@linux.davincidsp.com" <davinci-linux-open-source@linux.davincidsp.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-ker> Cc: Thomas Gleixner <tglx@linutronix.de> Subject: RE: [PATCH v3 1/6] arm: davinci: Fix low level gpio irq handlers' argument Date: Tue, 12 Jul 2011 14:52:17 +0530 [thread overview] Message-ID: <B85A65D85D7EB246BE421B3FB0FBB593024DA7AEF2@dbde02.ent.ti.com> (raw) In-Reply-To: <1310418191-27009-1-git-send-email-ido@wizery.com> 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 <ido@wizery.com> > CC: Thomas Gleixner <tglx@linutronix.de> > --- > 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 <ido@wizery.com> 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 <ido@wizery.com> CC: Thomas Gleixner <tglx@linutronix.de> [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 <nsekhar@ti.com> --- 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);
WARNING: multiple messages have this Message-ID (diff)
From: nsekhar@ti.com (Nori, Sekhar) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v3 1/6] arm: davinci: Fix low level gpio irq handlers' argument Date: Tue, 12 Jul 2011 14:52:17 +0530 [thread overview] Message-ID: <B85A65D85D7EB246BE421B3FB0FBB593024DA7AEF2@dbde02.ent.ti.com> (raw) In-Reply-To: <1310418191-27009-1-git-send-email-ido@wizery.com> 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 <ido@wizery.com> > CC: Thomas Gleixner <tglx@linutronix.de> > --- > 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 <ido@wizery.com> 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 <ido@wizery.com> CC: Thomas Gleixner <tglx@linutronix.de> [nsekhar at 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 <nsekhar@ti.com> --- 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);
next prev parent reply other threads:[~2011-07-12 9:22 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-07-10 13:14 [PATCH v2 0/6] arm: davinci: DA850: wl12xx expansion card Ido Yariv 2011-07-10 13:14 ` Ido Yariv 2011-07-10 13:14 ` [PATCH v2 1/6] arm: davinci: Fix low level gpio irq handlers' argument Ido Yariv 2011-07-10 13:14 ` Ido Yariv 2011-07-11 11:58 ` Nori, Sekhar 2011-07-11 11:58 ` Nori, Sekhar 2011-07-11 20:33 ` Ido Yariv 2011-07-11 20:33 ` Ido Yariv 2011-07-11 21:03 ` [PATCH v3 " Ido Yariv 2011-07-11 21:03 ` Ido Yariv 2011-07-12 9:22 ` Nori, Sekhar [this message] 2011-07-12 9:22 ` Nori, Sekhar 2011-07-12 22:19 ` Ido Yariv 2011-07-12 22:19 ` Ido Yariv 2011-07-10 13:14 ` [PATCH v2 2/6] arm: davinci: Explicitly set channel controllers' default queues Ido Yariv 2011-07-10 13:14 ` Ido Yariv 2011-07-25 13:15 ` Nori, Sekhar 2011-07-25 13:15 ` Nori, Sekhar 2011-07-25 13:33 ` Sergei Shtylyov 2011-07-25 13:33 ` Sergei Shtylyov 2011-07-25 16:31 ` Nori, Sekhar 2011-07-25 16:31 ` Nori, Sekhar 2011-07-29 14:42 ` Ben Gardiner 2011-07-29 14:42 ` Ben Gardiner 2011-07-10 13:14 ` [PATCH v2 3/6] arm: davinci: mmc: Add support for set_power callback Ido Yariv 2011-07-10 13:14 ` Ido Yariv 2011-07-25 13:19 ` Nori, Sekhar 2011-07-25 13:19 ` Nori, Sekhar 2011-07-28 20:49 ` [PATCH REPOST v2] mmc: davinci: " Ido Yariv 2011-07-28 20:49 ` Ido Yariv 2011-07-28 22:02 ` Chris Ball 2011-07-28 22:02 ` Chris Ball 2011-07-10 13:14 ` [PATCH v2 4/6] arm: davinci: DA850: Add MMC/SD1 pinmux configuration Ido Yariv 2011-07-10 13:14 ` Ido Yariv 2011-07-10 13:14 ` [PATCH v2 5/6] arm: davinci: DA850: Add GPIO pinmux configuration for wl1271 Ido Yariv 2011-07-10 13:14 ` Ido Yariv 2011-07-25 13:23 ` Nori, Sekhar 2011-07-25 13:23 ` Nori, Sekhar 2011-07-10 13:14 ` [PATCH v2 6/6] arm: davinci: DA850: Add wl1271/wlan support Ido Yariv 2011-07-10 13:14 ` Ido Yariv 2011-07-25 17:40 ` Nori, Sekhar 2011-07-25 17:40 ` Nori, Sekhar 2011-07-28 18:34 ` Ido Yariv 2011-07-28 18:34 ` Ido Yariv 2011-08-22 7:37 ` Harishkumar V 2011-07-28 20:52 ` [PATCH v3] davinci: Add wl1271/wlan support for AM18x Ido Yariv 2011-07-28 20:52 ` Ido Yariv 2011-07-28 21:15 ` Troy Kisky 2011-07-28 21:15 ` Troy Kisky 2011-07-28 22:06 ` Ido Yariv 2011-07-28 22:06 ` Ido Yariv 2011-07-29 18:51 ` Troy Kisky 2011-07-29 18:51 ` Troy Kisky 2011-08-03 13:27 ` Nori, Sekhar 2011-08-04 7:51 ` [PATCH v4] ARM: davinci: AM18x: Add wl1271/wlan support Ido Yariv 2011-08-04 7:51 ` Ido Yariv 2011-08-04 8:40 ` Jaya Kumar 2011-08-04 8:40 ` Jaya Kumar 2011-08-04 9:22 ` Ido Yariv 2011-08-04 9:22 ` Ido Yariv 2011-09-01 11:59 ` Nori, Sekhar 2011-09-01 11:59 ` Nori, Sekhar
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=B85A65D85D7EB246BE421B3FB0FBB593024DA7AEF2@dbde02.ent.ti.com \ --to=nsekhar@ti.com \ --cc=davinci-linux-open-source@linux.davincidsp.com \ --cc=ido@wizery.com \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.