* [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()"
@ 2016-10-18 22:56 Jason Gunthorpe
2016-10-19 7:09 ` Gregory CLEMENT
0 siblings, 1 reply; 6+ messages in thread
From: Jason Gunthorpe @ 2016-10-18 22:56 UTC (permalink / raw)
To: linux-arm-kernel
This reverts commit ce931f571b6dcf8534e8740e8cd16565cf362536.
The only difference betwen _simple and _legacy is that _simple
calls irq_alloc_descs, however mvebu_gpio_probe already called
irq_alloc_descs a few lines above.
This fixes these kernel error messages from the double call
to irq_alloc_descs:
irq: Cannot allocate irq_descs @ IRQ34, assuming pre-allocated
irq: Cannot allocate irq_descs @ IRQ66, assuming pre-allocated
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Linus Walleij <linus.walleij@linaro.org>
Fixes: ce931f571b6d ("gpio/mvebu: convert to use irq_domain_add_simple()")
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
drivers/gpio/gpio-mvebu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index cd5dc27320a2..2e0c8d8b7792 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -807,8 +807,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE);
/* Setup irq domain on top of the generic chip. */
- mvchip->domain = irq_domain_add_simple(np, mvchip->chip.ngpio,
- mvchip->irqbase,
+ mvchip->domain = irq_domain_add_legacy(np, mvchip->chip.ngpio,
+ mvchip->irqbase, 0,
&irq_domain_simple_ops,
mvchip);
if (!mvchip->domain) {
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()" 2016-10-18 22:56 [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()" Jason Gunthorpe @ 2016-10-19 7:09 ` Gregory CLEMENT 2016-10-19 21:03 ` Jason Gunthorpe 0 siblings, 1 reply; 6+ messages in thread From: Gregory CLEMENT @ 2016-10-19 7:09 UTC (permalink / raw) To: linux-arm-kernel Hi Jason, On mer., oct. 19 2016, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > This reverts commit ce931f571b6dcf8534e8740e8cd16565cf362536. > > The only difference betwen _simple and _legacy is that _simple > calls irq_alloc_descs, however mvebu_gpio_probe already called > irq_alloc_descs a few lines above. And what about removing the irq_alloc_descs ? Going back to use the _legacy version seems wrong for me. Gregory > > This fixes these kernel error messages from the double call > to irq_alloc_descs: > > irq: Cannot allocate irq_descs @ IRQ34, assuming pre-allocated > irq: Cannot allocate irq_descs @ IRQ66, assuming pre-allocated > > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> > Cc: Andrew Lunn <andrew@lunn.ch> > Cc: Linus Walleij <linus.walleij@linaro.org> > Fixes: ce931f571b6d ("gpio/mvebu: convert to use irq_domain_add_simple()") > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > drivers/gpio/gpio-mvebu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c > index cd5dc27320a2..2e0c8d8b7792 100644 > --- a/drivers/gpio/gpio-mvebu.c > +++ b/drivers/gpio/gpio-mvebu.c > @@ -807,8 +807,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev) > IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE); > > /* Setup irq domain on top of the generic chip. */ > - mvchip->domain = irq_domain_add_simple(np, mvchip->chip.ngpio, > - mvchip->irqbase, > + mvchip->domain = irq_domain_add_legacy(np, mvchip->chip.ngpio, > + mvchip->irqbase, 0, > &irq_domain_simple_ops, > mvchip); > if (!mvchip->domain) { > -- > 2.1.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()" 2016-10-19 7:09 ` Gregory CLEMENT @ 2016-10-19 21:03 ` Jason Gunthorpe 2016-10-23 23:11 ` Linus Walleij 0 siblings, 1 reply; 6+ messages in thread From: Jason Gunthorpe @ 2016-10-19 21:03 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 19, 2016 at 09:09:10AM +0200, Gregory CLEMENT wrote: > On mer., oct. 19 2016, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > > > This reverts commit ce931f571b6dcf8534e8740e8cd16565cf362536. > > > > The only difference betwen _simple and _legacy is that _simple > > calls irq_alloc_descs, however mvebu_gpio_probe already called > > irq_alloc_descs a few lines above. > > And what about removing the irq_alloc_descs ? I didn't think I had a test system for that complex work, but it turns out I do.. > Going back to use the _legacy version seems wrong for me. Both _legacy and _simple are described as deprecated, and using _simple is clearly the wrong choice for this driver, so I can't see how it is 'wrong' to go back. But it is legit to ask if the driver can be converted to use the modern style for setting up irq domains, so here is a patch that does that instead. I was only able to test LEVEL interrupts, but I think both kinds should be OK. >From 7566f05ac445b652ba7607cc1899fed10fea1c76 Mon Sep 17 00:00:00 2001 From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Date: Wed, 19 Oct 2016 14:57:45 -0600 Subject: [PATCH] gpio/mvebu: Use irq_domain_add_linear This fixes the irq allocation in this driver to not print: irq: Cannot allocate irq_descs @ IRQ34, assuming pre-allocated irq: Cannot allocate irq_descs @ IRQ66, assuming pre-allocated Which happens because the driver already called irq_alloc_descs() and so the change to use irq_domain_add_simple resulted in calling irq_alloc_descs() twice. Modernize the irq allocation in this driver to use the irq_domain_add_linear flow directly and eliminate the use of irq_domain_add_simple/legacy Fixes: ce931f571b6d ("gpio/mvebu: convert to use irq_domain_add_simple()") Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- drivers/gpio/gpio-mvebu.c | 92 ++++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 49 deletions(-) diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c index cd5dc27320a2..1ed6132b993c 100644 --- a/drivers/gpio/gpio-mvebu.c +++ b/drivers/gpio/gpio-mvebu.c @@ -293,10 +293,10 @@ static void mvebu_gpio_irq_ack(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; - u32 mask = ~(1 << (d->irq - gc->irq_base)); + u32 mask = d->mask; irq_gc_lock(gc); - writel_relaxed(mask, mvebu_gpioreg_edge_cause(mvchip)); + writel_relaxed(~mask, mvebu_gpioreg_edge_cause(mvchip)); irq_gc_unlock(gc); } @@ -305,7 +305,7 @@ static void mvebu_gpio_edge_irq_mask(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; struct irq_chip_type *ct = irq_data_get_chip_type(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); ct->mask_cache_priv &= ~mask; @@ -319,8 +319,7 @@ static void mvebu_gpio_edge_irq_unmask(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; struct irq_chip_type *ct = irq_data_get_chip_type(d); - - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); ct->mask_cache_priv |= mask; @@ -333,8 +332,7 @@ static void mvebu_gpio_level_irq_mask(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; struct irq_chip_type *ct = irq_data_get_chip_type(d); - - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); ct->mask_cache_priv &= ~mask; @@ -347,8 +345,7 @@ static void mvebu_gpio_level_irq_unmask(struct irq_data *d) struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct mvebu_gpio_chip *mvchip = gc->private; struct irq_chip_type *ct = irq_data_get_chip_type(d); - - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = d->mask; irq_gc_lock(gc); ct->mask_cache_priv |= mask; @@ -462,7 +459,7 @@ static void mvebu_gpio_irq_handler(struct irq_desc *desc) for (i = 0; i < mvchip->chip.ngpio; i++) { int irq; - irq = mvchip->irqbase + i; + irq = irq_find_mapping(mvchip->domain, i); if (!(cause & (1 << i))) continue; @@ -655,6 +652,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev) struct irq_chip_type *ct; struct clk *clk; unsigned int ngpios; + bool have_irqs; int soc_variant; int i, cpu, id; int err; @@ -665,6 +663,9 @@ static int mvebu_gpio_probe(struct platform_device *pdev) else soc_variant = MVEBU_GPIO_SOC_VARIANT_ORION; + /* Some gpio controllers do not provide irq support */ + have_irqs = of_irq_count(np) != 0; + mvchip = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_gpio_chip), GFP_KERNEL); if (!mvchip) @@ -697,7 +698,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev) mvchip->chip.get = mvebu_gpio_get; mvchip->chip.direction_output = mvebu_gpio_direction_output; mvchip->chip.set = mvebu_gpio_set; - mvchip->chip.to_irq = mvebu_gpio_to_irq; + if (have_irqs) + mvchip->chip.to_irq = mvebu_gpio_to_irq; mvchip->chip.base = id * MVEBU_MAX_GPIO_PER_BANK; mvchip->chip.ngpio = ngpios; mvchip->chip.can_sleep = false; @@ -758,34 +760,30 @@ static int mvebu_gpio_probe(struct platform_device *pdev) devm_gpiochip_add_data(&pdev->dev, &mvchip->chip, mvchip); /* Some gpio controllers do not provide irq support */ - if (!of_irq_count(np)) + if (!have_irqs) return 0; - /* Setup the interrupt handlers. Each chip can have up to 4 - * interrupt handlers, with each handler dealing with 8 GPIO - * pins. */ - for (i = 0; i < 4; i++) { - int irq = platform_get_irq(pdev, i); - - if (irq < 0) - continue; - irq_set_chained_handler_and_data(irq, mvebu_gpio_irq_handler, - mvchip); - } - - mvchip->irqbase = irq_alloc_descs(-1, 0, ngpios, -1); - if (mvchip->irqbase < 0) { - dev_err(&pdev->dev, "no irqs\n"); - return mvchip->irqbase; + mvchip->domain = + irq_domain_add_linear(np, ngpios, &irq_generic_chip_ops, NULL); + if (!mvchip->domain) { + dev_err(&pdev->dev, "couldn't allocate irq domain %s (DT).\n", + mvchip->chip.label); + return -ENODEV; } - gc = irq_alloc_generic_chip("mvebu_gpio_irq", 2, mvchip->irqbase, - mvchip->membase, handle_level_irq); - if (!gc) { - dev_err(&pdev->dev, "Cannot allocate generic irq_chip\n"); - return -ENOMEM; + err = irq_alloc_domain_generic_chips( + mvchip->domain, ngpios, 2, np->name, handle_level_irq, + IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_LEVEL, 0, 0); + if (err) { + dev_err(&pdev->dev, "couldn't allocate irq chips %s (DT).\n", + mvchip->chip.label); + goto err_domain; } + /* NOTE: The common accessors cannot be used because of the percpu + * access to the mask registers + */ + gc = irq_get_domain_generic_chip(mvchip->domain, 0); gc->private = mvchip; ct = &gc->chip_types[0]; ct->type = IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW; @@ -803,27 +801,23 @@ static int mvebu_gpio_probe(struct platform_device *pdev) ct->handler = handle_edge_irq; ct->chip.name = mvchip->chip.label; - irq_setup_generic_chip(gc, IRQ_MSK(ngpios), 0, - IRQ_NOREQUEST, IRQ_LEVEL | IRQ_NOPROBE); + /* Setup the interrupt handlers. Each chip can have up to 4 + * interrupt handlers, with each handler dealing with 8 GPIO + * pins. + */ + for (i = 0; i < 4; i++) { + int irq = platform_get_irq(pdev, i); - /* Setup irq domain on top of the generic chip. */ - mvchip->domain = irq_domain_add_simple(np, mvchip->chip.ngpio, - mvchip->irqbase, - &irq_domain_simple_ops, - mvchip); - if (!mvchip->domain) { - dev_err(&pdev->dev, "couldn't allocate irq domain %s (DT).\n", - mvchip->chip.label); - err = -ENODEV; - goto err_generic_chip; + if (irq < 0) + continue; + irq_set_chained_handler_and_data(irq, mvebu_gpio_irq_handler, + mvchip); } return 0; -err_generic_chip: - irq_remove_generic_chip(gc, IRQ_MSK(ngpios), IRQ_NOREQUEST, - IRQ_LEVEL | IRQ_NOPROBE); - kfree(gc); +err_domain: + irq_domain_remove(mvchip->domain); return err; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()" 2016-10-19 21:03 ` Jason Gunthorpe @ 2016-10-23 23:11 ` Linus Walleij 2016-10-27 7:30 ` Gregory CLEMENT 0 siblings, 1 reply; 6+ messages in thread From: Linus Walleij @ 2016-10-23 23:11 UTC (permalink / raw) To: linux-arm-kernel On Wed, Oct 19, 2016 at 11:03 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > From 7566f05ac445b652ba7607cc1899fed10fea1c76 Mon Sep 17 00:00:00 2001 > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Date: Wed, 19 Oct 2016 14:57:45 -0600 > Subject: [PATCH] gpio/mvebu: Use irq_domain_add_linear > > This fixes the irq allocation in this driver to not print: > irq: Cannot allocate irq_descs @ IRQ34, assuming pre-allocated > irq: Cannot allocate irq_descs @ IRQ66, assuming pre-allocated > > Which happens because the driver already called irq_alloc_descs() > and so the change to use irq_domain_add_simple resulted in calling > irq_alloc_descs() twice. > > Modernize the irq allocation in this driver to use the > irq_domain_add_linear flow directly and eliminate the use of > irq_domain_add_simple/legacy > > Fixes: ce931f571b6d ("gpio/mvebu: convert to use irq_domain_add_simple()") > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> So can I just apply this? Gregory? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()" 2016-10-23 23:11 ` Linus Walleij @ 2016-10-27 7:30 ` Gregory CLEMENT 2016-10-28 12:24 ` Linus Walleij 0 siblings, 1 reply; 6+ messages in thread From: Gregory CLEMENT @ 2016-10-27 7:30 UTC (permalink / raw) To: linux-arm-kernel Hi Linus, On lun., oct. 24 2016, Linus Walleij <linus.walleij@linaro.org> wrote: > On Wed, Oct 19, 2016 at 11:03 PM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > >> From 7566f05ac445b652ba7607cc1899fed10fea1c76 Mon Sep 17 00:00:00 2001 >> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> Date: Wed, 19 Oct 2016 14:57:45 -0600 >> Subject: [PATCH] gpio/mvebu: Use irq_domain_add_linear >> >> This fixes the irq allocation in this driver to not print: >> irq: Cannot allocate irq_descs @ IRQ34, assuming pre-allocated >> irq: Cannot allocate irq_descs @ IRQ66, assuming pre-allocated >> >> Which happens because the driver already called irq_alloc_descs() >> and so the change to use irq_domain_add_simple resulted in calling >> irq_alloc_descs() twice. >> >> Modernize the irq allocation in this driver to use the >> irq_domain_add_linear flow directly and eliminate the use of >> irq_domain_add_simple/legacy >> >> Fixes: ce931f571b6d ("gpio/mvebu: convert to use irq_domain_add_simple()") >> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > So can I just apply this? > Gregory? For me it's OK. Gregory > > Yours, > Linus Walleij -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()" 2016-10-27 7:30 ` Gregory CLEMENT @ 2016-10-28 12:24 ` Linus Walleij 0 siblings, 0 replies; 6+ messages in thread From: Linus Walleij @ 2016-10-28 12:24 UTC (permalink / raw) To: linux-arm-kernel On Thu, Oct 27, 2016 at 9:30 AM, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote: > On lun., oct. 24 2016, Linus Walleij <linus.walleij@linaro.org> wrote: >> On Wed, Oct 19, 2016 at 11:03 PM, Jason Gunthorpe >> <jgunthorpe@obsidianresearch.com> wrote: >> >>> From 7566f05ac445b652ba7607cc1899fed10fea1c76 Mon Sep 17 00:00:00 2001 >>> From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>> Date: Wed, 19 Oct 2016 14:57:45 -0600 >>> Subject: [PATCH] gpio/mvebu: Use irq_domain_add_linear >>> >>> This fixes the irq allocation in this driver to not print: >>> irq: Cannot allocate irq_descs @ IRQ34, assuming pre-allocated >>> irq: Cannot allocate irq_descs @ IRQ66, assuming pre-allocated >>> >>> Which happens because the driver already called irq_alloc_descs() >>> and so the change to use irq_domain_add_simple resulted in calling >>> irq_alloc_descs() twice. >>> >>> Modernize the irq allocation in this driver to use the >>> irq_domain_add_linear flow directly and eliminate the use of >>> irq_domain_add_simple/legacy >>> >>> Fixes: ce931f571b6d ("gpio/mvebu: convert to use irq_domain_add_simple()") >>> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> >> So can I just apply this? >> Gregory? > > For me it's OK. APplied this inline version. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-28 12:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-10-18 22:56 [PATCH] Revert "gpio/mvebu: convert to use irq_domain_add_simple()" Jason Gunthorpe 2016-10-19 7:09 ` Gregory CLEMENT 2016-10-19 21:03 ` Jason Gunthorpe 2016-10-23 23:11 ` Linus Walleij 2016-10-27 7:30 ` Gregory CLEMENT 2016-10-28 12:24 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).