From: Javier Martinez Canillas <martinez.javier@gmail.com> To: Stephen Warren <swarren@wwwdotorg.org> Cc: Linus Walleij <linus.walleij@linaro.org>, Jon Hunter <jon-hunter@ti.com>, Grant Likely <grant.likely@secretlab.ca>, Alexandre Courbot <acourbot@nvidia.com>, Stephen Warren <swarren@nvidia.com>, Kevin Hilman <khilman@deeprootsystems.com>, "devicetree-discuss@lists.ozlabs.org" <devicetree-discuss@lists.ozlabs.org>, "linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>, Tarun Kanti DebBarma <tarun.kanti@ti.com>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH 3/5] gpio/omap: Add DT support to GPIO driver Date: Sun, 14 Apr 2013 03:35:44 +0200 [thread overview] Message-ID: <CAAwP0s0e2x33jZQhFRY+Hoo61SFqYKRgWk=1mL2+DNxSDJHz+A@mail.gmail.com> (raw) In-Reply-To: <51673D70.3010503@wwwdotorg.org> On Fri, Apr 12, 2013 at 12:47 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 04/11/2013 04:16 PM, Linus Walleij wrote: >> On Thu, Apr 11, 2013 at 10:30 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 04/10/2013 03:28 PM, Linus Walleij wrote: >> >>>> So the only reason I'm rambing on about this is that it breaks the >>> >>> I'm not sure I understand this paragraph; what is "it" in the line above. >>> >>> If "it" is this patch, then should "breaks" be re-establishes? >> >> No I'm replying to Javier Martinez Canillas mail in this thread: >> http://marc.info/?l=linux-arm-kernel&m=136334655902407&w=2 >> which is stating a question to grand, and contains the below >> hunk: >> >>> +static int gpio_irq_request(struct irq_data *d) >>> +{ >>> + struct gpio_bank *bank = irq_data_get_irq_chip_data(d); >>> + >>> + return gpio_request(irq_to_gpio(bank, d->irq), "gpio-irq"); >>> +} >> >> irq_to_gpio(). Notice that. not my_funny_driver_irq_to_gpio(). > > OK, right. sorry for being so confused/confusing. > > Yes, that code should certainly call e.g. omap_gpio__irq_to_gpio() not > irq_to_gpio(). Probably gpio_irq_request() wants renaming to something > more like omap_gpio__irq_request() too, so the names don't look like > they'd clash with global functions. (__ added for clarity but probably > only one _ at a time) Stephen, Linus, Is the following inlined patch [1] what you were thinking that would be the right approach? With this patch an explicit call to call gpio_request() before a call to chip->irq_set_type() is needed. I've tested both with DT and without DT where a explicit call to gpio_request() is made and it works in both cases. So it shouldn't have a functional change for non-DT cases as far as I know. If you agree with [1] then I'll split in two patches (one that adds the irq_request function pointer to irq_chip and another one that adds the implementation for gpio-omap) and send as a patch-set. I just thought that it would be easier for you if I posted here an inlined patch so you could have context. Thanks a lot for your feedback and best regards, Javier [1] diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 685e850..e035e64 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -811,6 +811,13 @@ static void gpio_unmask_irq(struct irq_data *d) spin_unlock_irqrestore(&bank->lock, flags); } +static int omap_gpio_irq_request(struct irq_data *d) +{ + struct gpio_bank *bank = irq_data_get_irq_chip_data(d); + + return omap_gpio_request(&bank->chip, d->hwirq); +} + static struct irq_chip gpio_irq_chip = { .name = "GPIO", .irq_shutdown = gpio_irq_shutdown, @@ -819,6 +826,7 @@ static struct irq_chip gpio_irq_chip = { .irq_unmask = gpio_unmask_irq, .irq_set_type = gpio_irq_type, .irq_set_wake = gpio_wake_enable, + .irq_request = omap_gpio_irq_request, }; /*---------------------------------------------------------------------*/ @@ -1028,6 +1036,7 @@ omap_mpuio_alloc_gc(struct gpio_bank *bank, unsigned int irq_start, ct->chip.irq_mask = irq_gc_mask_set_bit; ct->chip.irq_unmask = irq_gc_mask_clr_bit; ct->chip.irq_set_type = gpio_irq_type; + ct->chip.irq_request = omap_gpio_irq_request; if (bank->regs->wkup_en) ct->chip.irq_set_wake = gpio_wake_enable, diff --git a/include/linux/irq.h b/include/linux/irq.h index 0e8e3a6..85596cc 100644 --- a/include/linux/irq.h +++ b/include/linux/irq.h @@ -303,6 +303,7 @@ struct irq_chip { void (*irq_shutdown)(struct irq_data *data); void (*irq_enable)(struct irq_data *data); void (*irq_disable)(struct irq_data *data); + int (*irq_request)(struct irq_data *data); void (*irq_ack)(struct irq_data *data); void (*irq_mask)(struct irq_data *data); diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index fa17855..a4bd4f7 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -588,6 +588,12 @@ int __irq_set_trigger(struct irq_desc *desc, unsigned int irq, unmask = 1; } + if (chip->irq_request) { + ret = chip->irq_request(&desc->irq_data); + if (ret) + return ret; + } + /* caller masked out all except trigger mode flags */ ret = chip->irq_set_type(&desc->irq_data, flags);
WARNING: multiple messages have this Message-ID (diff)
From: martinez.javier@gmail.com (Javier Martinez Canillas) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 3/5] gpio/omap: Add DT support to GPIO driver Date: Sun, 14 Apr 2013 03:35:44 +0200 [thread overview] Message-ID: <CAAwP0s0e2x33jZQhFRY+Hoo61SFqYKRgWk=1mL2+DNxSDJHz+A@mail.gmail.com> (raw) In-Reply-To: <51673D70.3010503@wwwdotorg.org> On Fri, Apr 12, 2013 at 12:47 AM, Stephen Warren <swarren@wwwdotorg.org> wrote: > On 04/11/2013 04:16 PM, Linus Walleij wrote: >> On Thu, Apr 11, 2013 at 10:30 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: >>> On 04/10/2013 03:28 PM, Linus Walleij wrote: >> >>>> So the only reason I'm rambing on about this is that it breaks the >>> >>> I'm not sure I understand this paragraph; what is "it" in the line above. >>> >>> If "it" is this patch, then should "breaks" be re-establishes? >> >> No I'm replying to Javier Martinez Canillas mail in this thread: >> http://marc.info/?l=linux-arm-kernel&m=136334655902407&w=2 >> which is stating a question to grand, and contains the below >> hunk: >> >>> +static int gpio_irq_request(struct irq_data *d) >>> +{ >>> + struct gpio_bank *bank = irq_data_get_irq_chip_data(d); >>> + >>> + return gpio_request(irq_to_gpio(bank, d->irq), "gpio-irq"); >>> +} >> >> irq_to_gpio(). Notice that. not my_funny_driver_irq_to_gpio(). > > OK, right. sorry for being so confused/confusing. > > Yes, that code should certainly call e.g. omap_gpio__irq_to_gpio() not > irq_to_gpio(). Probably gpio_irq_request() wants renaming to something > more like omap_gpio__irq_request() too, so the names don't look like > they'd clash with global functions. (__ added for clarity but probably > only one _ at a time) Stephen, Linus, Is the following inlined patch [1] what you were thinking that would be the right approach? With this patch an explicit call to call gpio_request() before a call to chip->irq_set_type() is needed. I've tested both with DT and without DT where a explicit call to gpio_request() is made and it works in both cases. So it shouldn't have a functional change for non-DT cases as far as I know. If you agree with [1] then I'll split in two patches (one that adds the irq_request function pointer to irq_chip and another one that adds the implementation for gpio-omap) and send as a patch-set. I just thought that it would be easier for you if I posted here an inlined patch so you could have context. Thanks a lot for your feedback and best regards, Javier [1] diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 685e850..e035e64 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -811,6 +811,13 @@ static void gpio_unmask_irq(struct irq_data *d) spin_unlock_irqrestore(&bank->lock, flags); } +static int omap_gpio_irq_request(struct irq_data *d) +{ + struct gpio_bank *bank = irq_data_get_irq_chip_data(d); + + return omap_gpio_request(&bank->chip, d->hwirq); +} + static struct irq_chip gpio_irq_chip = { .name = "GPIO", .irq_shutdown = gpio_irq_shutdown, @@ -819,6 +826,7 @@ static struct irq_chip gpio_irq_chip = { .irq_unmask = gpio_unmask_irq, .irq_set_type = gpio_irq_type, .irq_set_wake = gpio_wake_enable, + .irq_request = omap_gpio_irq_request, }; /*---------------------------------------------------------------------*/ @@ -1028,6 +1036,7 @@ omap_mpuio_alloc_gc(struct gpio_bank *bank, unsigned int irq_start, ct->chip.irq_mask = irq_gc_mask_set_bit; ct->chip.irq_unmask = irq_gc_mask_clr_bit; ct->chip.irq_set_type = gpio_irq_type; + ct->chip.irq_request = omap_gpio_irq_request; if (bank->regs->wkup_en) ct->chip.irq_set_wake = gpio_wake_enable, diff --git a/include/linux/irq.h b/include/linux/irq.h index 0e8e3a6..85596cc 100644 --- a/include/linux/irq.h +++ b/include/linux/irq.h @@ -303,6 +303,7 @@ struct irq_chip { void (*irq_shutdown)(struct irq_data *data); void (*irq_enable)(struct irq_data *data); void (*irq_disable)(struct irq_data *data); + int (*irq_request)(struct irq_data *data); void (*irq_ack)(struct irq_data *data); void (*irq_mask)(struct irq_data *data); diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c index fa17855..a4bd4f7 100644 --- a/kernel/irq/manage.c +++ b/kernel/irq/manage.c @@ -588,6 +588,12 @@ int __irq_set_trigger(struct irq_desc *desc, unsigned int irq, unmask = 1; } + if (chip->irq_request) { + ret = chip->irq_request(&desc->irq_data); + if (ret) + return ret; + } + /* caller masked out all except trigger mode flags */ ret = chip->irq_set_type(&desc->irq_data, flags);
next prev parent reply other threads:[~2013-04-14 1:35 UTC|newest] Thread overview: 190+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-02-15 16:04 [PATCH 0/5] gpio/omap: Cleanup and adaptation to Device Tree Benoit Cousson 2012-02-15 16:04 ` Benoit Cousson 2012-02-15 16:04 ` [PATCH 1/5] gpio/omap: Remove bank->id information and misc cleanup Benoit Cousson 2012-02-15 16:04 ` Benoit Cousson 2012-02-16 5:53 ` DebBarma, Tarun Kanti 2012-02-16 5:53 ` DebBarma, Tarun Kanti 2012-02-16 9:33 ` Cousson, Benoit 2012-02-16 9:33 ` Cousson, Benoit 2012-02-15 16:04 ` [PATCH 2/5] gpio/omap: Use devm_ API and add request_mem_region Benoit Cousson 2012-02-15 16:04 ` Benoit Cousson 2012-02-16 5:41 ` DebBarma, Tarun Kanti 2012-02-16 5:41 ` DebBarma, Tarun Kanti 2012-02-16 6:35 ` Grant Likely 2012-02-16 6:35 ` Grant Likely 2012-02-16 7:11 ` DebBarma, Tarun Kanti 2012-02-16 7:11 ` DebBarma, Tarun Kanti 2012-02-16 6:37 ` Shubhrajyoti 2012-02-16 6:37 ` Shubhrajyoti 2012-02-16 8:56 ` Cousson, Benoit 2012-02-16 8:56 ` Cousson, Benoit 2012-02-15 16:04 ` [PATCH 3/5] gpio/omap: Add DT support to GPIO driver Benoit Cousson 2012-02-15 16:04 ` Benoit Cousson 2012-02-22 14:23 ` Rob Herring 2012-02-22 14:23 ` Rob Herring 2012-02-22 14:31 ` Cousson, Benoit 2012-02-22 14:31 ` Cousson, Benoit 2012-02-22 17:23 ` Rob Herring 2012-02-22 17:23 ` Rob Herring 2012-02-22 18:29 ` Stephen Warren 2012-02-22 18:29 ` Stephen Warren 2012-02-24 15:30 ` Cousson, Benoit 2012-02-24 15:30 ` Cousson, Benoit 2013-02-26 10:01 ` Javier Martinez Canillas 2013-02-26 10:01 ` Javier Martinez Canillas 2013-02-26 16:33 ` Stephen Warren 2013-02-26 16:33 ` Stephen Warren 2013-02-26 22:40 ` Jon Hunter 2013-02-26 22:40 ` Jon Hunter 2013-02-26 22:44 ` Stephen Warren 2013-02-26 22:44 ` Stephen Warren 2013-02-26 23:01 ` Jon Hunter 2013-02-26 23:01 ` Jon Hunter 2013-02-26 23:06 ` Stephen Warren 2013-02-26 23:06 ` Stephen Warren 2013-02-26 23:45 ` Jon Hunter 2013-02-26 23:45 ` Jon Hunter 2013-02-27 0:13 ` Stephen Warren 2013-02-27 0:13 ` Stephen Warren 2013-02-27 1:07 ` Jon Hunter 2013-02-27 1:07 ` Jon Hunter 2013-02-27 3:57 ` Javier Martinez Canillas 2013-02-27 3:57 ` Javier Martinez Canillas 2013-02-27 17:50 ` Stephen Warren 2013-02-27 17:50 ` Stephen Warren 2013-02-27 20:05 ` Javier Martinez Canillas 2013-02-27 20:05 ` Javier Martinez Canillas 2013-02-27 23:16 ` Jon Hunter 2013-02-27 23:16 ` Jon Hunter 2013-02-28 12:17 ` Javier Martinez Canillas 2013-02-28 12:17 ` Javier Martinez Canillas 2013-02-28 20:49 ` Jon Hunter 2013-02-28 20:49 ` Jon Hunter [not found] ` <512D3EC2.6050408-l0cyMroinI0@public.gmane.org> 2013-03-02 20:05 ` Grant Likely 2013-03-02 20:05 ` Grant Likely 2013-03-07 23:14 ` Jon Hunter 2013-03-07 23:14 ` Jon Hunter 2013-03-15 11:21 ` Javier Martinez Canillas 2013-03-15 11:21 ` Javier Martinez Canillas 2013-03-22 8:10 ` Linus Walleij 2013-03-22 8:10 ` Linus Walleij 2013-03-22 15:33 ` Stephen Warren 2013-03-22 15:33 ` Stephen Warren 2013-03-22 22:52 ` Jon Hunter 2013-03-22 22:52 ` Jon Hunter 2013-03-27 13:52 ` Linus Walleij 2013-03-27 13:52 ` Linus Walleij 2013-03-27 16:09 ` Stephen Warren 2013-03-27 16:09 ` Stephen Warren 2013-03-27 20:55 ` Linus Walleij 2013-03-27 20:55 ` Linus Walleij 2013-03-29 17:01 ` Stephen Warren 2013-03-29 17:01 ` Stephen Warren 2013-04-10 18:12 ` Linus Walleij 2013-04-10 18:12 ` Linus Walleij 2013-04-10 20:29 ` Stephen Warren 2013-04-10 20:29 ` Stephen Warren 2013-04-10 21:28 ` Linus Walleij 2013-04-10 21:28 ` Linus Walleij 2013-04-11 20:30 ` Stephen Warren 2013-04-11 20:30 ` Stephen Warren 2013-04-11 22:16 ` Linus Walleij 2013-04-11 22:16 ` Linus Walleij 2013-04-11 22:47 ` Stephen Warren 2013-04-11 22:47 ` Stephen Warren 2013-04-14 1:35 ` Javier Martinez Canillas [this message] 2013-04-14 1:35 ` Javier Martinez Canillas 2013-04-14 20:53 ` Linus Walleij 2013-04-14 20:53 ` Linus Walleij 2013-04-15 11:25 ` Javier Martinez Canillas 2013-04-15 11:25 ` Javier Martinez Canillas 2013-04-15 16:58 ` Stephen Warren 2013-04-15 16:58 ` Stephen Warren [not found] ` <516C73C6.5050409@ti.co m> 2013-04-15 21:40 ` Jon Hunter 2013-04-15 21:40 ` Jon Hunter 2013-04-15 21:44 ` Jon Hunter 2013-04-15 21:44 ` Jon Hunter 2013-04-15 22:16 ` Stephen Warren 2013-04-15 22:16 ` Stephen Warren 2013-04-15 23:04 ` Jon Hunter 2013-04-15 23:04 ` Jon Hunter 2013-04-16 18:40 ` Stephen Warren 2013-04-16 18:40 ` Stephen Warren 2013-04-16 19:27 ` Jon Hunter 2013-04-16 19:27 ` Jon Hunter 2013-04-16 21:57 ` Jon Hunter 2013-04-16 21:57 ` Jon Hunter 2013-04-16 22:11 ` Stephen Warren 2013-04-16 22:11 ` Stephen Warren 2013-04-16 23:14 ` Jon Hunter 2013-04-16 23:14 ` Jon Hunter 2013-04-17 0:41 ` Javier Martinez Canillas 2013-04-17 0:41 ` Javier Martinez Canillas 2013-04-17 2:00 ` Jon Hunter 2013-04-17 2:00 ` Jon Hunter 2013-04-17 7:55 ` Javier Martinez Canillas 2013-04-17 7:55 ` Javier Martinez Canillas [not found] ` <CAAwP0s2M2pnSydyDvh_rejFO=w8bCo4WE5PkxrYuk0HQDixc-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-04-17 13:25 ` Jon Hunter 2013-04-17 13:25 ` Jon Hunter 2013-04-17 13:42 ` Javier Martinez Canillas 2013-04-17 13:42 ` Javier Martinez Canillas [not found] ` <CAAwP0s2DsJAWuXWvPAkzCT0T0AG_OvMEw2sADW6LqSi1Ofd_Zw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2013-04-17 13:52 ` Jon Hunter 2013-04-17 13:52 ` Jon Hunter 2013-04-17 14:21 ` Javier Martinez Canillas 2013-04-17 14:21 ` Javier Martinez Canillas 2013-04-17 16:18 ` Javier Martinez Canillas 2013-04-17 16:18 ` Javier Martinez Canillas 2013-04-26 7:31 ` Linus Walleij 2013-04-26 7:31 ` Linus Walleij 2013-04-26 21:31 ` Jon Hunter 2013-04-26 21:31 ` Jon Hunter 2013-06-11 21:25 ` Grant Likely 2013-06-11 21:25 ` Grant Likely 2013-06-12 9:43 ` Linus Walleij 2013-06-12 9:43 ` Linus Walleij 2013-04-17 15:41 ` Stephen Warren 2013-04-17 15:41 ` Stephen Warren 2013-04-26 7:27 ` Linus Walleij 2013-04-26 7:27 ` Linus Walleij 2013-04-26 21:25 ` Jon Hunter 2013-04-26 21:25 ` Jon Hunter [not found] ` <517AF0C1.60009-l0cyMroinI0@public.gmane.org> 2013-05-03 14:35 ` Linus Walleij 2013-05-03 14:35 ` Linus Walleij 2013-04-26 7:11 ` Linus Walleij 2013-04-26 7:11 ` Linus Walleij 2013-04-26 6:59 ` Linus Walleij 2013-04-26 6:59 ` Linus Walleij 2013-04-15 16:53 ` Stephen Warren 2013-04-15 16:53 ` Stephen Warren 2013-04-15 20:00 ` Jon Hunter 2013-04-15 20:00 ` Jon Hunter 2013-04-11 22:49 ` Javier Martinez Canillas 2013-04-11 22:49 ` Javier Martinez Canillas 2013-04-11 22:51 ` Stephen Warren 2013-04-11 22:51 ` Stephen Warren 2013-04-10 21:44 ` Arnd Bergmann 2013-04-10 21:44 ` Arnd Bergmann 2013-02-27 3:33 ` Javier Martinez Canillas 2013-02-27 3:33 ` Javier Martinez Canillas 2013-02-27 17:47 ` Stephen Warren 2013-02-27 17:47 ` Stephen Warren 2013-02-27 20:00 ` Javier Martinez Canillas 2013-02-27 20:00 ` Javier Martinez Canillas 2013-02-26 23:08 ` Jon Hunter 2013-02-26 23:08 ` Jon Hunter 2013-02-27 3:47 ` Javier Martinez Canillas 2013-02-27 3:47 ` Javier Martinez Canillas 2013-02-27 20:13 ` Jon Hunter 2013-02-27 20:13 ` Jon Hunter 2013-02-27 23:41 ` Linus Walleij 2013-02-27 23:41 ` Linus Walleij 2013-02-28 13:04 ` Benoit Cousson 2013-02-28 13:04 ` Benoit Cousson 2013-03-01 0:09 ` Linus Walleij 2013-03-01 0:09 ` Linus Walleij 2013-03-01 0:42 ` Jon Hunter 2013-03-01 0:42 ` Jon Hunter 2012-02-15 16:04 ` [PATCH 4/5] arm/dts: OMAP4: Add gpio nodes Benoit Cousson 2012-02-15 16:04 ` Benoit Cousson 2012-02-15 16:04 ` [PATCH 5/5] arm/dts: OMAP3: " Benoit Cousson 2012-02-15 16:04 ` Benoit Cousson
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='CAAwP0s0e2x33jZQhFRY+Hoo61SFqYKRgWk=1mL2+DNxSDJHz+A@mail.gmail.com' \ --to=martinez.javier@gmail.com \ --cc=acourbot@nvidia.com \ --cc=devicetree-discuss@lists.ozlabs.org \ --cc=grant.likely@secretlab.ca \ --cc=jon-hunter@ti.com \ --cc=khilman@deeprootsystems.com \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-omap@vger.kernel.org \ --cc=swarren@nvidia.com \ --cc=swarren@wwwdotorg.org \ --cc=tarun.kanti@ti.com \ /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.