* [PATCH v3 0/2] generic irq chip and pl061 domain support @ 2012-01-30 17:31 Rob Herring 2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring 2012-01-30 17:31 ` [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding Rob Herring 0 siblings, 2 replies; 11+ messages in thread From: Rob Herring @ 2012-01-30 17:31 UTC (permalink / raw) To: linux-arm-kernel, linux-kernel Cc: Grant Likely, shawn.guo, b-cousson, Rob Herring From: Rob Herring <rob.herring@calxeda.com> This series adds irq domain support for generic irq chip and updates pl061 gpio to use it. This is has been significant re-worked from the previous version based on Grant Likely's irq domain work and to make the domain support optional as some arches don't use domains. A new function irq_setup_generic_chip_domain is added to setup a generic irq chip with a domain. Shawn, can you test on i.MX again. I think your case with multiple generic irq chips for 1 device node should work, but I can't test that. This is based on Grant Likely's irq domain work and is available here: git://sources.calxeda.com/kernel/linux.git pl061-domain-v3 Rob Rob Herring (2): irq: add irq_domain support to generic-chip gpio: pl061: enable interrupts with DT style binding .../devicetree/bindings/gpio/pl061-gpio.txt | 15 +++ drivers/gpio/gpio-pl061.c | 39 ++++--- include/linux/irq.h | 10 ++ kernel/irq/generic-chip.c | 134 +++++++++++++++++--- 4 files changed, 166 insertions(+), 32 deletions(-) -- 1.7.5.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/2] irq: add irq_domain support to generic-chip 2012-01-30 17:31 [PATCH v3 0/2] generic irq chip and pl061 domain support Rob Herring @ 2012-01-30 17:31 ` Rob Herring 2012-01-31 14:13 ` Shawn Guo 2012-02-01 0:02 ` Grant Likely 2012-01-30 17:31 ` [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding Rob Herring 1 sibling, 2 replies; 11+ messages in thread From: Rob Herring @ 2012-01-30 17:31 UTC (permalink / raw) To: linux-arm-kernel, linux-kernel Cc: Grant Likely, shawn.guo, b-cousson, Rob Herring, Thomas Gleixner From: Rob Herring <rob.herring@calxeda.com> Add irq domain support to irq generic-chip. This enables users of generic-chip to support dynamic irq assignment needed for DT interrupt binding. Signed-off-by: Rob Herring <rob.herring@calxeda.com> Cc: Grant Likely <grant.likely@secretlab.ca> Cc: Thomas Gleixner <tglx@linutronix.de> --- include/linux/irq.h | 10 +++ kernel/irq/generic-chip.c | 134 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 129 insertions(+), 15 deletions(-) diff --git a/include/linux/irq.h b/include/linux/irq.h index bff29c5..482d198 100644 --- a/include/linux/irq.h +++ b/include/linux/irq.h @@ -665,6 +665,10 @@ struct irq_chip_generic { void __iomem *reg_base; unsigned int irq_base; unsigned int irq_cnt; + struct irq_domain *domain; + unsigned int flags; + unsigned int irq_set; + unsigned int irq_clr; u32 mask_cache; u32 type_cache; u32 polarity_cache; @@ -707,6 +711,12 @@ irq_alloc_generic_chip(const char *name, int nr_ct, unsigned int irq_base, void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk, enum irq_gc_flags flags, unsigned int clr, unsigned int set); + +struct device_node; +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc, + struct device_node *node, u32 msk, + enum irq_gc_flags flags, unsigned int clr, + unsigned int set); int irq_setup_alt_chip(struct irq_data *d, unsigned int type); void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk, unsigned int clr, unsigned int set); diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c index c89295a..2519663 100644 --- a/kernel/irq/generic-chip.c +++ b/kernel/irq/generic-chip.c @@ -5,6 +5,7 @@ */ #include <linux/io.h> #include <linux/irq.h> +#include <linux/irqdomain.h> #include <linux/slab.h> #include <linux/export.h> #include <linux/interrupt.h> @@ -39,7 +40,7 @@ void irq_gc_noop(struct irq_data *d) void irq_gc_mask_disable_reg(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = 1 << d->hwirq; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable); @@ -57,7 +58,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d) void irq_gc_mask_set_bit(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = 1 << d->hwirq; irq_gc_lock(gc); gc->mask_cache |= mask; @@ -75,7 +76,7 @@ void irq_gc_mask_set_bit(struct irq_data *d) void irq_gc_mask_clr_bit(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = 1 << d->hwirq; irq_gc_lock(gc); gc->mask_cache &= ~mask; @@ -93,7 +94,7 @@ void irq_gc_mask_clr_bit(struct irq_data *d) void irq_gc_unmask_enable_reg(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = 1 << d->hwirq; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable); @@ -108,7 +109,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d) void irq_gc_ack_set_bit(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = 1 << d->hwirq; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack); @@ -122,7 +123,7 @@ void irq_gc_ack_set_bit(struct irq_data *d) void irq_gc_ack_clr_bit(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = ~(1 << (d->irq - gc->irq_base)); + u32 mask = ~(1 << d->hwirq); irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack); @@ -136,7 +137,7 @@ void irq_gc_ack_clr_bit(struct irq_data *d) void irq_gc_mask_disable_reg_and_ack(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = 1 << d->hwirq; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->mask); @@ -151,7 +152,7 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d) void irq_gc_eoi(struct irq_data *d) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = 1 << d->hwirq; irq_gc_lock(gc); irq_reg_writel(mask, gc->reg_base + cur_regs(d)->eoi); @@ -169,7 +170,7 @@ void irq_gc_eoi(struct irq_data *d) int irq_gc_set_wake(struct irq_data *d, unsigned int on) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); - u32 mask = 1 << (d->irq - gc->irq_base); + u32 mask = 1 << d->hwirq; if (!(mask & gc->wake_enabled)) return -EINVAL; @@ -257,11 +258,99 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk, irq_set_chip_and_handler(i, &ct->chip, ct->handler); irq_set_chip_data(i, gc); irq_modify_status(i, clr, set); + irq_get_irq_data(i)->hwirq = i - gc->irq_base; } gc->irq_cnt = i - gc->irq_base; } EXPORT_SYMBOL_GPL(irq_setup_generic_chip); +#ifdef CONFIG_IRQ_DOMAIN +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np) +{ + struct irq_chip_generic *gc; + + if (d->of_node != NULL && d->of_node == np) { + list_for_each_entry(gc, &gc_list, list) { + if ((gc == d->host_data) && (d == gc->domain)) + return 1; + } + } + return 0; +} + +static int irq_gc_irq_domain_map(struct irq_domain *d, unsigned int irq, + irq_hw_number_t hw) +{ + struct irq_chip_generic *gc = d->host_data; + struct irq_chip_type *ct = gc->chip_types; + + if (gc->flags & IRQ_GC_INIT_NESTED_LOCK) + irq_set_lockdep_class(irq, &irq_nested_lock_class); + + irq_set_chip_and_handler(irq, &ct->chip, ct->handler); + irq_set_chip_data(irq, gc); + irq_modify_status(irq, gc->irq_clr, gc->irq_set); + + return 0; +} + +static struct irq_domain_ops irq_gc_irq_domain_ops = { + .match = irq_gc_irq_domain_match, + .map = irq_gc_irq_domain_map, + .xlate = irq_domain_xlate_onetwocell, +}; + +/* + * irq_setup_generic_chip_domain - Setup a range of interrupts with a generic chip and domain + * @gc: Generic irq chip holding all data + * @node: Device tree node pointer for domain + * @msk: Bitmask holding the irqs to initialize relative to gc->irq_base + * @flags: Flags for initialization + * @clr: IRQ_* bits to clear + * @set: IRQ_* bits to set + * + * Set up max. 32 interrupts starting from gc->irq_base using an irq domain. + * Note, this initializes all interrupts to the primary irq_chip_type and its + * associated handler. + */ +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc, + struct device_node *node, u32 msk, + enum irq_gc_flags flags, unsigned int clr, + unsigned int set) +{ + struct irq_chip_type *ct = gc->chip_types; + + if (!node) { + irq_setup_generic_chip(gc, msk, flags, clr, set); + return; + } + + raw_spin_lock(&gc_lock); + list_add_tail(&gc->list, &gc_list); + raw_spin_unlock(&gc_lock); + + /* Init mask cache ? */ + if (flags & IRQ_GC_INIT_MASK_CACHE) + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask); + + gc->flags = flags; + gc->irq_clr = clr; + gc->irq_set = set; + + /* Users of domains should not use irq_base */ + if ((int)gc->irq_base > 0) + gc->domain = irq_domain_add_legacy(node, fls(msk), + gc->irq_base, 0, + &irq_gc_irq_domain_ops, gc); + else { + gc->irq_base = 0; + gc->domain = irq_domain_add_linear(node, fls(msk), + &irq_gc_irq_domain_ops, gc); + } +} +EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain); +#endif + /** * irq_setup_alt_chip - Switch to alternative chip * @d: irq_data for this interrupt @@ -325,8 +414,13 @@ static int irq_gc_suspend(void) list_for_each_entry(gc, &gc_list, list) { struct irq_chip_type *ct = gc->chip_types; - if (ct->chip.irq_suspend) - ct->chip.irq_suspend(irq_get_irq_data(gc->irq_base)); + if (ct->chip.irq_suspend) { + int i; + int irq = gc->irq_base; + for (i = 0; !irq && i < 32; i++) + irq = irq_find_mapping(gc->domain, i); + ct->chip.irq_suspend(irq_get_irq_data(irq)); + } } return 0; } @@ -338,8 +432,13 @@ static void irq_gc_resume(void) list_for_each_entry(gc, &gc_list, list) { struct irq_chip_type *ct = gc->chip_types; - if (ct->chip.irq_resume) - ct->chip.irq_resume(irq_get_irq_data(gc->irq_base)); + if (ct->chip.irq_resume) { + int i; + int irq = gc->irq_base; + for (i = 0; !irq && i < 32; i++) + irq = irq_find_mapping(gc->domain, i); + ct->chip.irq_resume(irq_get_irq_data(irq)); + } } } #else @@ -354,8 +453,13 @@ static void irq_gc_shutdown(void) list_for_each_entry(gc, &gc_list, list) { struct irq_chip_type *ct = gc->chip_types; - if (ct->chip.irq_pm_shutdown) - ct->chip.irq_pm_shutdown(irq_get_irq_data(gc->irq_base)); + if (ct->chip.irq_pm_shutdown) { + int i; + int irq = gc->irq_base; + for (i = 0; !irq && i < 32; i++) + irq = irq_find_mapping(gc->domain, i); + ct->chip.irq_pm_shutdown(irq_get_irq_data(irq)); + } } } -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip 2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring @ 2012-01-31 14:13 ` Shawn Guo 2012-01-31 14:32 ` Rob Herring 2012-02-01 0:02 ` Grant Likely 1 sibling, 1 reply; 11+ messages in thread From: Shawn Guo @ 2012-01-31 14:13 UTC (permalink / raw) To: Rob Herring Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Rob Herring, Thomas Gleixner On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote: ... > +#ifdef CONFIG_IRQ_DOMAIN > +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np) > +{ > + struct irq_chip_generic *gc; > + > + if (d->of_node != NULL && d->of_node == np) { > + list_for_each_entry(gc, &gc_list, list) { > + if ((gc == d->host_data) && (d == gc->domain)) > + return 1; > + } > + } IIRC, we talked about this a little bit, but I'm still unsure how this works for imx5 tzic case, where we have the same one tzic device_node for 4 irqdomains representing 128 irq lines. It seems to me the match function here will always find the first irqdomain of the 4 for any of those 128 tzic irqs. The following is my code change against your branch for testing. Am I missing anything? 8<--- diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c index e1b5edf..45abf11 100644 --- a/arch/arm/mach-imx/imx51-dt.c +++ b/arch/arm/mach-imx/imx51-dt.c @@ -44,13 +44,6 @@ static const struct of_dev_auxdata imx51_auxdata_lookup[] __initconst = { { /* sentinel */ } }; -static int __init imx51_tzic_add_irq_domain(struct device_node *np, - struct device_node *interrupt_parent) -{ - irq_domain_add_legacy(np, 32, 0, 0, &irq_domain_simple_ops, NULL); - return 0; -} - static int __init imx51_gpio_add_irq_domain(struct device_node *np, struct device_node *interrupt_parent) { @@ -63,7 +56,6 @@ static int __init imx51_gpio_add_irq_domain(struct device_node *np, } static const struct of_device_id imx51_irq_match[] __initconst = { - { .compatible = "fsl,imx51-tzic", .data = imx51_tzic_add_irq_domain, }, { .compatible = "fsl,imx51-gpio", .data = imx51_gpio_add_irq_domain, }, { /* sentinel */ } }; diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c index 98308ec..ffb615d 100644 --- a/arch/arm/plat-mxc/tzic.c +++ b/arch/arm/plat-mxc/tzic.c @@ -122,7 +122,9 @@ static __init void tzic_init_gc(unsigned int irq_start) ct->regs.disable = TZIC_ENCLEAR0(idx); ct->regs.enable = TZIC_ENSET0(idx); - irq_setup_generic_chip(gc, IRQ_MSK(32), 0, IRQ_NOREQUEST, 0); + irq_setup_generic_chip_domain(gc, + of_find_compatible_node(NULL, NULL, "fsl,tzic"), + IRQ_MSK(32), 0, IRQ_NOREQUEST, 0); } asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs) --->8 > + return 0; > +} > + ... > +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc, > + struct device_node *node, u32 msk, > + enum irq_gc_flags flags, unsigned int clr, > + unsigned int set) > +{ > + struct irq_chip_type *ct = gc->chip_types; > + > + if (!node) { > + irq_setup_generic_chip(gc, msk, flags, clr, set); > + return; > + } > + > + raw_spin_lock(&gc_lock); > + list_add_tail(&gc->list, &gc_list); > + raw_spin_unlock(&gc_lock); > + > + /* Init mask cache ? */ > + if (flags & IRQ_GC_INIT_MASK_CACHE) > + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask); > + > + gc->flags = flags; > + gc->irq_clr = clr; > + gc->irq_set = set; > + > + /* Users of domains should not use irq_base */ > + if ((int)gc->irq_base > 0) > + gc->domain = irq_domain_add_legacy(node, fls(msk), > + gc->irq_base, 0, > + &irq_gc_irq_domain_ops, gc); > + else { > + gc->irq_base = 0; > + gc->domain = irq_domain_add_linear(node, fls(msk), > + &irq_gc_irq_domain_ops, gc); > + } We have 4 generic_chips for tzic with irq_base as 0, 32, 64, 96. In this case, we end up with having the first domain as the linear, and the other 3 as the legacy? > +} > +EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain); > +#endif -- Regards, Shawn ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip 2012-01-31 14:13 ` Shawn Guo @ 2012-01-31 14:32 ` Rob Herring 2012-01-31 15:06 ` Shawn Guo 0 siblings, 1 reply; 11+ messages in thread From: Rob Herring @ 2012-01-31 14:32 UTC (permalink / raw) To: Shawn Guo Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Thomas Gleixner On 01/31/2012 08:13 AM, Shawn Guo wrote: > On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote: > ... >> +#ifdef CONFIG_IRQ_DOMAIN >> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np) >> +{ >> + struct irq_chip_generic *gc; >> + >> + if (d->of_node != NULL && d->of_node == np) { >> + list_for_each_entry(gc, &gc_list, list) { >> + if ((gc == d->host_data) && (d == gc->domain)) >> + return 1; >> + } >> + } > > IIRC, we talked about this a little bit, but I'm still unsure how this > works for imx5 tzic case, where we have the same one tzic device_node > for 4 irqdomains representing 128 irq lines. It seems to me the match > function here will always find the first irqdomain of the 4 for any > of those 128 tzic irqs. > > The following is my code change against your branch for testing. Am I > missing anything? The irq domain code is a bit different now, so it's matching differently than before. See the match function. The host_data ptr for a domain is set to the gc ptr. I then check that the gc->domain matches the domain passed in. So the fact that 4 domains point to 1 device_node doesn't matter. > > 8<--- > diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c > index e1b5edf..45abf11 100644 > --- a/arch/arm/mach-imx/imx51-dt.c > +++ b/arch/arm/mach-imx/imx51-dt.c > @@ -44,13 +44,6 @@ static const struct of_dev_auxdata > imx51_auxdata_lookup[] __initconst = { > { /* sentinel */ } > }; > > -static int __init imx51_tzic_add_irq_domain(struct device_node *np, > - struct device_node *interrupt_parent) > -{ > - irq_domain_add_legacy(np, 32, 0, 0, &irq_domain_simple_ops, NULL); > - return 0; > -} > - > static int __init imx51_gpio_add_irq_domain(struct device_node *np, > struct device_node *interrupt_parent) > { > @@ -63,7 +56,6 @@ static int __init imx51_gpio_add_irq_domain(struct > device_node *np, > } > > static const struct of_device_id imx51_irq_match[] __initconst = { > - { .compatible = "fsl,imx51-tzic", .data = imx51_tzic_add_irq_domain, }, > { .compatible = "fsl,imx51-gpio", .data = imx51_gpio_add_irq_domain, }, > { /* sentinel */ } > }; > > diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c > index 98308ec..ffb615d 100644 > --- a/arch/arm/plat-mxc/tzic.c > +++ b/arch/arm/plat-mxc/tzic.c > @@ -122,7 +122,9 @@ static __init void tzic_init_gc(unsigned int irq_start) > ct->regs.disable = TZIC_ENCLEAR0(idx); > ct->regs.enable = TZIC_ENSET0(idx); > > - irq_setup_generic_chip(gc, IRQ_MSK(32), 0, IRQ_NOREQUEST, 0); > + irq_setup_generic_chip_domain(gc, > + of_find_compatible_node(NULL, NULL, "fsl,tzic"), > + IRQ_MSK(32), 0, IRQ_NOREQUEST, 0); Looks right, but I wouldn't lookup the node ptr every time. > } > > asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs) > --->8 > >> + return 0; >> +} >> + > ... >> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc, >> + struct device_node *node, u32 msk, >> + enum irq_gc_flags flags, unsigned int clr, >> + unsigned int set) >> +{ >> + struct irq_chip_type *ct = gc->chip_types; >> + >> + if (!node) { >> + irq_setup_generic_chip(gc, msk, flags, clr, set); >> + return; >> + } >> + >> + raw_spin_lock(&gc_lock); >> + list_add_tail(&gc->list, &gc_list); >> + raw_spin_unlock(&gc_lock); >> + >> + /* Init mask cache ? */ >> + if (flags & IRQ_GC_INIT_MASK_CACHE) >> + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask); >> + >> + gc->flags = flags; >> + gc->irq_clr = clr; >> + gc->irq_set = set; >> + >> + /* Users of domains should not use irq_base */ >> + if ((int)gc->irq_base > 0) >> + gc->domain = irq_domain_add_legacy(node, fls(msk), >> + gc->irq_base, 0, >> + &irq_gc_irq_domain_ops, gc); >> + else { >> + gc->irq_base = 0; >> + gc->domain = irq_domain_add_linear(node, fls(msk), >> + &irq_gc_irq_domain_ops, gc); >> + } > > We have 4 generic_chips for tzic with irq_base as 0, 32, 64, 96. In > this case, we end up with having the first domain as the linear, and > the other 3 as the legacy? Umm, yes. So it should be '>= 0' instead until we stop using IRQ0. I could base it on NULL node ptr instead... For the DT case, you want irq_base to be -1. Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip 2012-01-31 14:32 ` Rob Herring @ 2012-01-31 15:06 ` Shawn Guo 2012-01-31 15:37 ` Rob Herring 0 siblings, 1 reply; 11+ messages in thread From: Shawn Guo @ 2012-01-31 15:06 UTC (permalink / raw) To: Rob Herring Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Thomas Gleixner On Tue, Jan 31, 2012 at 08:32:29AM -0600, Rob Herring wrote: > On 01/31/2012 08:13 AM, Shawn Guo wrote: > > On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote: > > ... > >> +#ifdef CONFIG_IRQ_DOMAIN > >> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np) > >> +{ > >> + struct irq_chip_generic *gc; > >> + > >> + if (d->of_node != NULL && d->of_node == np) { > >> + list_for_each_entry(gc, &gc_list, list) { > >> + if ((gc == d->host_data) && (d == gc->domain)) > >> + return 1; > >> + } > >> + } > > > > IIRC, we talked about this a little bit, but I'm still unsure how this > > works for imx5 tzic case, where we have the same one tzic device_node > > for 4 irqdomains representing 128 irq lines. It seems to me the match > > function here will always find the first irqdomain of the 4 for any > > of those 128 tzic irqs. > > > > The following is my code change against your branch for testing. Am I > > missing anything? > > The irq domain code is a bit different now, so it's matching differently > than before. See the match function. The host_data ptr for a domain is > set to the gc ptr. I then check that the gc->domain matches the domain > passed in. So the fact that 4 domains point to 1 device_node doesn't matter. > I do not quite follow on that. The gc->domain always matches the domain passed in anyway for those 4 pairs of domain/gc. That said, the condition is all true for any of those 4 tzic domains. if ((gc == d->host_data) && (d == gc->domain)) It does not help on identifying the correct domain from those 4 with given device_node, which is exactly same for those 4 domains. > > > > 8<--- > > diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c > > index e1b5edf..45abf11 100644 > > --- a/arch/arm/mach-imx/imx51-dt.c > > +++ b/arch/arm/mach-imx/imx51-dt.c > > @@ -44,13 +44,6 @@ static const struct of_dev_auxdata > > imx51_auxdata_lookup[] __initconst = { > > { /* sentinel */ } > > }; > > > > -static int __init imx51_tzic_add_irq_domain(struct device_node *np, > > - struct device_node *interrupt_parent) > > -{ > > - irq_domain_add_legacy(np, 32, 0, 0, &irq_domain_simple_ops, NULL); > > - return 0; > > -} > > - > > static int __init imx51_gpio_add_irq_domain(struct device_node *np, > > struct device_node *interrupt_parent) > > { > > @@ -63,7 +56,6 @@ static int __init imx51_gpio_add_irq_domain(struct > > device_node *np, > > } > > > > static const struct of_device_id imx51_irq_match[] __initconst = { > > - { .compatible = "fsl,imx51-tzic", .data = imx51_tzic_add_irq_domain, }, > > { .compatible = "fsl,imx51-gpio", .data = imx51_gpio_add_irq_domain, }, > > { /* sentinel */ } > > }; > > > > diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c > > index 98308ec..ffb615d 100644 > > --- a/arch/arm/plat-mxc/tzic.c > > +++ b/arch/arm/plat-mxc/tzic.c > > @@ -122,7 +122,9 @@ static __init void tzic_init_gc(unsigned int irq_start) > > ct->regs.disable = TZIC_ENCLEAR0(idx); > > ct->regs.enable = TZIC_ENSET0(idx); > > > > - irq_setup_generic_chip(gc, IRQ_MSK(32), 0, IRQ_NOREQUEST, 0); > > + irq_setup_generic_chip_domain(gc, > > + of_find_compatible_node(NULL, NULL, "fsl,tzic"), > > + IRQ_MSK(32), 0, IRQ_NOREQUEST, 0); > > Looks right, but I wouldn't lookup the node ptr every time. > Yeah, will avoid it in the final patch. > > } > > > > asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs) > > --->8 > > > >> + return 0; > >> +} > >> + > > ... > >> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc, > >> + struct device_node *node, u32 msk, > >> + enum irq_gc_flags flags, unsigned int clr, > >> + unsigned int set) > >> +{ > >> + struct irq_chip_type *ct = gc->chip_types; > >> + > >> + if (!node) { > >> + irq_setup_generic_chip(gc, msk, flags, clr, set); > >> + return; > >> + } > >> + > >> + raw_spin_lock(&gc_lock); > >> + list_add_tail(&gc->list, &gc_list); > >> + raw_spin_unlock(&gc_lock); > >> + > >> + /* Init mask cache ? */ > >> + if (flags & IRQ_GC_INIT_MASK_CACHE) > >> + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask); > >> + > >> + gc->flags = flags; > >> + gc->irq_clr = clr; > >> + gc->irq_set = set; > >> + > >> + /* Users of domains should not use irq_base */ > >> + if ((int)gc->irq_base > 0) > >> + gc->domain = irq_domain_add_legacy(node, fls(msk), > >> + gc->irq_base, 0, > >> + &irq_gc_irq_domain_ops, gc); > >> + else { > >> + gc->irq_base = 0; > >> + gc->domain = irq_domain_add_linear(node, fls(msk), > >> + &irq_gc_irq_domain_ops, gc); > >> + } > > > > We have 4 generic_chips for tzic with irq_base as 0, 32, 64, 96. In > > this case, we end up with having the first domain as the linear, and > > the other 3 as the legacy? > > Umm, yes. So it should be '>= 0' instead until we stop using IRQ0. I > could base it on NULL node ptr instead... > > For the DT case, you want irq_base to be -1. > Unless we have to, I would keep the same tzic code for dt and non-dt to save #ifdef CONFIG_OF. -- Regards, Shawn ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip 2012-01-31 15:06 ` Shawn Guo @ 2012-01-31 15:37 ` Rob Herring 0 siblings, 0 replies; 11+ messages in thread From: Rob Herring @ 2012-01-31 15:37 UTC (permalink / raw) To: Shawn Guo Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Thomas Gleixner On 01/31/2012 09:06 AM, Shawn Guo wrote: > On Tue, Jan 31, 2012 at 08:32:29AM -0600, Rob Herring wrote: >> On 01/31/2012 08:13 AM, Shawn Guo wrote: >>> On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote: >>> ... >>>> +#ifdef CONFIG_IRQ_DOMAIN >>>> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np) >>>> +{ >>>> + struct irq_chip_generic *gc; >>>> + >>>> + if (d->of_node != NULL && d->of_node == np) { >>>> + list_for_each_entry(gc, &gc_list, list) { >>>> + if ((gc == d->host_data) && (d == gc->domain)) >>>> + return 1; >>>> + } >>>> + } >>> >>> IIRC, we talked about this a little bit, but I'm still unsure how this >>> works for imx5 tzic case, where we have the same one tzic device_node >>> for 4 irqdomains representing 128 irq lines. It seems to me the match >>> function here will always find the first irqdomain of the 4 for any >>> of those 128 tzic irqs. >>> >>> The following is my code change against your branch for testing. Am I >>> missing anything? >> >> The irq domain code is a bit different now, so it's matching differently >> than before. See the match function. The host_data ptr for a domain is >> set to the gc ptr. I then check that the gc->domain matches the domain >> passed in. So the fact that 4 domains point to 1 device_node doesn't matter. >> > I do not quite follow on that. The gc->domain always matches the > domain passed in anyway for those 4 pairs of domain/gc. That said, > the condition is all true for any of those 4 tzic domains. > > if ((gc == d->host_data) && (d == gc->domain)) > > It does not help on identifying the correct domain from those 4 with > given device_node, which is exactly same for those 4 domains. > Crap. You're right. It worked in my head... ;) Let me think about this some more. >>> >>> 8<--- >>> diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c >>> index e1b5edf..45abf11 100644 >>> --- a/arch/arm/mach-imx/imx51-dt.c >>> +++ b/arch/arm/mach-imx/imx51-dt.c >>> @@ -44,13 +44,6 @@ static const struct of_dev_auxdata >>> imx51_auxdata_lookup[] __initconst = { >>> { /* sentinel */ } >>> }; >>> >>> -static int __init imx51_tzic_add_irq_domain(struct device_node *np, >>> - struct device_node *interrupt_parent) >>> -{ >>> - irq_domain_add_legacy(np, 32, 0, 0, &irq_domain_simple_ops, NULL); >>> - return 0; >>> -} >>> - >>> static int __init imx51_gpio_add_irq_domain(struct device_node *np, >>> struct device_node *interrupt_parent) >>> { >>> @@ -63,7 +56,6 @@ static int __init imx51_gpio_add_irq_domain(struct >>> device_node *np, >>> } >>> >>> static const struct of_device_id imx51_irq_match[] __initconst = { >>> - { .compatible = "fsl,imx51-tzic", .data = imx51_tzic_add_irq_domain, }, >>> { .compatible = "fsl,imx51-gpio", .data = imx51_gpio_add_irq_domain, }, >>> { /* sentinel */ } >>> }; >>> >>> diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c >>> index 98308ec..ffb615d 100644 >>> --- a/arch/arm/plat-mxc/tzic.c >>> +++ b/arch/arm/plat-mxc/tzic.c >>> @@ -122,7 +122,9 @@ static __init void tzic_init_gc(unsigned int irq_start) >>> ct->regs.disable = TZIC_ENCLEAR0(idx); >>> ct->regs.enable = TZIC_ENSET0(idx); >>> >>> - irq_setup_generic_chip(gc, IRQ_MSK(32), 0, IRQ_NOREQUEST, 0); >>> + irq_setup_generic_chip_domain(gc, >>> + of_find_compatible_node(NULL, NULL, "fsl,tzic"), >>> + IRQ_MSK(32), 0, IRQ_NOREQUEST, 0); >> >> Looks right, but I wouldn't lookup the node ptr every time. >> > Yeah, will avoid it in the final patch. > >>> } >>> >>> asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs) >>> --->8 >>> >>>> + return 0; >>>> +} >>>> + >>> ... >>>> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc, >>>> + struct device_node *node, u32 msk, >>>> + enum irq_gc_flags flags, unsigned int clr, >>>> + unsigned int set) >>>> +{ >>>> + struct irq_chip_type *ct = gc->chip_types; >>>> + >>>> + if (!node) { >>>> + irq_setup_generic_chip(gc, msk, flags, clr, set); >>>> + return; >>>> + } >>>> + >>>> + raw_spin_lock(&gc_lock); >>>> + list_add_tail(&gc->list, &gc_list); >>>> + raw_spin_unlock(&gc_lock); >>>> + >>>> + /* Init mask cache ? */ >>>> + if (flags & IRQ_GC_INIT_MASK_CACHE) >>>> + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask); >>>> + >>>> + gc->flags = flags; >>>> + gc->irq_clr = clr; >>>> + gc->irq_set = set; >>>> + >>>> + /* Users of domains should not use irq_base */ >>>> + if ((int)gc->irq_base > 0) >>>> + gc->domain = irq_domain_add_legacy(node, fls(msk), >>>> + gc->irq_base, 0, >>>> + &irq_gc_irq_domain_ops, gc); >>>> + else { >>>> + gc->irq_base = 0; >>>> + gc->domain = irq_domain_add_linear(node, fls(msk), >>>> + &irq_gc_irq_domain_ops, gc); >>>> + } >>> >>> We have 4 generic_chips for tzic with irq_base as 0, 32, 64, 96. In >>> this case, we end up with having the first domain as the linear, and >>> the other 3 as the legacy? >> >> Umm, yes. So it should be '>= 0' instead until we stop using IRQ0. I >> could base it on NULL node ptr instead... >> >> For the DT case, you want irq_base to be -1. >> > Unless we have to, I would keep the same tzic code for dt and non-dt > to save #ifdef CONFIG_OF. You need to stop using Linux vIRQ0 as a valid vIRQ# and with DT, all knowledge about irq_base should be removed. Whether that can be accomplished with the same code is a separate issue. Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip 2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring 2012-01-31 14:13 ` Shawn Guo @ 2012-02-01 0:02 ` Grant Likely 1 sibling, 0 replies; 11+ messages in thread From: Grant Likely @ 2012-02-01 0:02 UTC (permalink / raw) To: Rob Herring Cc: linux-arm-kernel, linux-kernel, shawn.guo, b-cousson, Rob Herring, Thomas Gleixner On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > Add irq domain support to irq generic-chip. This enables users of > generic-chip to support dynamic irq assignment needed for DT interrupt > binding. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > Cc: Thomas Gleixner <tglx@linutronix.de> > --- > @@ -39,7 +40,7 @@ void irq_gc_noop(struct irq_data *d) > void irq_gc_mask_disable_reg(struct irq_data *d) > { > struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); > - u32 mask = 1 << (d->irq - gc->irq_base); > + u32 mask = 1 << d->hwirq; As discussed on IRC, there needs to be a 1:N relationship between an irq_domain and generic chips, but doing that means that hwirq no longer directly maps to the bit in the register. This could be solved however if a mod is applied to the hwirq number and if we're careful to line up hwirqs to those mod boundaries: u32 mask = 1 << (d->hwirq % gc->bank_size); > } > gc->irq_cnt = i - gc->irq_base; > } > EXPORT_SYMBOL_GPL(irq_setup_generic_chip); > > +#ifdef CONFIG_IRQ_DOMAIN > +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np) > +{ > + struct irq_chip_generic *gc; > + > + if (d->of_node != NULL && d->of_node == np) { > + list_for_each_entry(gc, &gc_list, list) { > + if ((gc == d->host_data) && (d == gc->domain)) > + return 1; > + } > + } > + return 0; > +} > + > +static int irq_gc_irq_domain_map(struct irq_domain *d, unsigned int irq, > + irq_hw_number_t hw) > +{ > + struct irq_chip_generic *gc = d->host_data; > + struct irq_chip_type *ct = gc->chip_types; > + > + if (gc->flags & IRQ_GC_INIT_NESTED_LOCK) > + irq_set_lockdep_class(irq, &irq_nested_lock_class); > + > + irq_set_chip_and_handler(irq, &ct->chip, ct->handler); > + irq_set_chip_data(irq, gc); > + irq_modify_status(irq, gc->irq_clr, gc->irq_set); > + > + return 0; > +} > + > +static struct irq_domain_ops irq_gc_irq_domain_ops = { > + .match = irq_gc_irq_domain_match, > + .map = irq_gc_irq_domain_map, > + .xlate = irq_domain_xlate_onetwocell, > +}; > + > +/* > + * irq_setup_generic_chip_domain - Setup a range of interrupts with a generic chip and domain > + * @gc: Generic irq chip holding all data > + * @node: Device tree node pointer for domain > + * @msk: Bitmask holding the irqs to initialize relative to gc->irq_base > + * @flags: Flags for initialization > + * @clr: IRQ_* bits to clear > + * @set: IRQ_* bits to set > + * > + * Set up max. 32 interrupts starting from gc->irq_base using an irq domain. > + * Note, this initializes all interrupts to the primary irq_chip_type and its > + * associated handler. > + */ > +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc, > + struct device_node *node, u32 msk, > + enum irq_gc_flags flags, unsigned int clr, > + unsigned int set) > +{ > + struct irq_chip_type *ct = gc->chip_types; > + > + if (!node) { > + irq_setup_generic_chip(gc, msk, flags, clr, set); > + return; > + } Calling irq_setup_generic_chip() should always be performed, regardless of whether or not a node is passed in. However, the msk field should be passed as 0 so that none of the irqs get configured before they are allocated. I'd like to see all the domain code paths look the same, regardless of whether a node pointer is provided. The only quirk here is that if a node isn't provided, then that probably means the range of irqs needs to be fixed; which means using the legacy map in the current implementation. We should sit down and talk about this next week at Connect. g. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding 2012-01-30 17:31 [PATCH v3 0/2] generic irq chip and pl061 domain support Rob Herring 2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring @ 2012-01-30 17:31 ` Rob Herring 2012-01-31 14:36 ` Shawn Guo 1 sibling, 1 reply; 11+ messages in thread From: Rob Herring @ 2012-01-30 17:31 UTC (permalink / raw) To: linux-arm-kernel, linux-kernel Cc: Grant Likely, shawn.guo, b-cousson, Rob Herring, Linus Walleij From: Rob Herring <rob.herring@calxeda.com> Enable DT interrupt binding support for pl061 gpio lines. If the gpio node has an interrupt-controller property, then it will be setup to handle interrupts on gpio lines. Signed-off-by: Rob Herring <rob.herring@calxeda.com> Cc: Grant Likely <grant.likely@secretlab.ca> Cc: Linus Walleij <linus.ml.walleij@gmail.com> --- .../devicetree/bindings/gpio/pl061-gpio.txt | 15 ++++++++ drivers/gpio/gpio-pl061.c | 39 +++++++++++--------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/Documentation/devicetree/bindings/gpio/pl061-gpio.txt b/Documentation/devicetree/bindings/gpio/pl061-gpio.txt index a2c416b..9671d4e 100644 --- a/Documentation/devicetree/bindings/gpio/pl061-gpio.txt +++ b/Documentation/devicetree/bindings/gpio/pl061-gpio.txt @@ -8,3 +8,18 @@ Required properties: - gpio-controller : Marks the device node as a GPIO controller. - interrupts : Interrupt mapping for GPIO IRQ. +Optional properties: +- interrupt-controller : Identifies the node as an interrupt controller. Must + be present if using gpios lines for interrupts. +- #interrupt-cells : Specifies the number of cells needed to encode an + interrupt source. The type shall be a <u32> and the value shall be 2. + + The 1st cell contains the interrupt number 0-7 corresponding to the gpio + line. + + The 2nd cell is the flags, encoding trigger type and level flags. + 1 = low-to-high edge triggered + 2 = high-to-low edge triggered + 4 = active high level-sensitive + 8 = active low level-sensitive + diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c index 77c9cc7..1657adb 100644 --- a/drivers/gpio/gpio-pl061.c +++ b/drivers/gpio/gpio-pl061.c @@ -15,6 +15,8 @@ #include <linux/io.h> #include <linux/ioport.h> #include <linux/irq.h> +#include <linux/irqdomain.h> +#include <linux/of.h> #include <linux/bitops.h> #include <linux/workqueue.h> #include <linux/gpio.h> @@ -56,7 +58,6 @@ struct pl061_gpio { spinlock_t lock; /* GPIO registers */ void __iomem *base; - int irq_base; struct irq_chip_generic *irq_gc; struct gpio_chip gc; @@ -126,18 +127,16 @@ static void pl061_set_value(struct gpio_chip *gc, unsigned offset, int value) static int pl061_to_irq(struct gpio_chip *gc, unsigned offset) { struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); - - if (chip->irq_base <= 0) - return -EINVAL; - - return chip->irq_base + offset; + if (!chip->irq_gc) + return -ENXIO; + return irq_find_mapping(chip->irq_gc->domain, offset); } static int pl061_irq_type(struct irq_data *d, unsigned trigger) { struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); struct pl061_gpio *chip = gc->private; - int offset = d->irq - chip->irq_base; + int offset = d->hwirq; unsigned long flags; u8 gpiois, gpioibe, gpioiev; @@ -197,7 +196,8 @@ static void pl061_irq_handler(unsigned irq, struct irq_desc *desc) chained_irq_exit(irqchip, desc); } -static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base) +static void __init pl061_init_gc(struct pl061_gpio *chip, + struct device_node *node, int irq_base) { struct irq_chip_type *ct; @@ -212,15 +212,17 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base) ct->chip.irq_set_wake = irq_gc_set_wake; ct->regs.mask = GPIOIE; - irq_setup_generic_chip(chip->irq_gc, IRQ_MSK(PL061_GPIO_NR), - IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0); + irq_setup_generic_chip_domain(chip->irq_gc, node, + IRQ_MSK(PL061_GPIO_NR), + IRQ_GC_INIT_NESTED_LOCK, + IRQ_NOREQUEST, 0); } static int pl061_probe(struct amba_device *dev, const struct amba_id *id) { struct pl061_platform_data *pdata; struct pl061_gpio *chip; - int ret, irq, i; + int ret, irq, i, irq_base; chip = kzalloc(sizeof(*chip), GFP_KERNEL); if (chip == NULL) @@ -229,10 +231,13 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id) pdata = dev->dev.platform_data; if (pdata) { chip->gc.base = pdata->gpio_base; - chip->irq_base = pdata->irq_base; + irq_base = pdata->irq_base; } else if (dev->dev.of_node) { chip->gc.base = -1; - chip->irq_base = 0; + if (of_get_property(dev->dev.of_node, "interrupt-controller", NULL)) + irq_base = -1; + else + irq_base = 0; } else { ret = -ENODEV; goto free_mem; @@ -267,13 +272,13 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id) goto iounmap; /* - * irq_chip support + * irq_chip support. If irq_base is 0, then we don't support interrupts + * on gpio lines and just return now. Otherwise setup the interrupts. */ - - if (chip->irq_base <= 0) + if (!irq_base) return 0; - pl061_init_gc(chip, chip->irq_base); + pl061_init_gc(chip, of_node_get(dev->dev.of_node), irq_base); writeb(0, chip->base + GPIOIE); /* disable irqs */ irq = dev->irq[0]; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding 2012-01-30 17:31 ` [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding Rob Herring @ 2012-01-31 14:36 ` Shawn Guo 2012-01-31 14:44 ` Rob Herring 0 siblings, 1 reply; 11+ messages in thread From: Shawn Guo @ 2012-01-31 14:36 UTC (permalink / raw) To: Rob Herring Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Rob Herring, Linus Walleij On Mon, Jan 30, 2012 at 11:31:39AM -0600, Rob Herring wrote: ... > -static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base) > +static void __init pl061_init_gc(struct pl061_gpio *chip, > + struct device_node *node, int irq_base) > { > struct irq_chip_type *ct; > > @@ -212,15 +212,17 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base) > ct->chip.irq_set_wake = irq_gc_set_wake; > ct->regs.mask = GPIOIE; > > - irq_setup_generic_chip(chip->irq_gc, IRQ_MSK(PL061_GPIO_NR), > - IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0); > + irq_setup_generic_chip_domain(chip->irq_gc, node, > + IRQ_MSK(PL061_GPIO_NR), > + IRQ_GC_INIT_NESTED_LOCK, > + IRQ_NOREQUEST, 0); > } The function irq_setup_generic_chip_domain() is wrapped by #ifdef CONFIG_IRQ_DOMAIN in patch #1. Is it true that pl061 driver will never work with !IRQ_DOMAIN case? -- Regards, Shawn ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding 2012-01-31 14:36 ` Shawn Guo @ 2012-01-31 14:44 ` Rob Herring 2012-02-01 0:07 ` Grant Likely 0 siblings, 1 reply; 11+ messages in thread From: Rob Herring @ 2012-01-31 14:44 UTC (permalink / raw) To: Shawn Guo Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Linus Walleij On 01/31/2012 08:36 AM, Shawn Guo wrote: > On Mon, Jan 30, 2012 at 11:31:39AM -0600, Rob Herring wrote: > ... >> -static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base) >> +static void __init pl061_init_gc(struct pl061_gpio *chip, >> + struct device_node *node, int irq_base) >> { >> struct irq_chip_type *ct; >> >> @@ -212,15 +212,17 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base) >> ct->chip.irq_set_wake = irq_gc_set_wake; >> ct->regs.mask = GPIOIE; >> >> - irq_setup_generic_chip(chip->irq_gc, IRQ_MSK(PL061_GPIO_NR), >> - IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0); >> + irq_setup_generic_chip_domain(chip->irq_gc, node, >> + IRQ_MSK(PL061_GPIO_NR), >> + IRQ_GC_INIT_NESTED_LOCK, >> + IRQ_NOREQUEST, 0); >> } > > The function irq_setup_generic_chip_domain() is wrapped by > #ifdef CONFIG_IRQ_DOMAIN in patch #1. Is it true that pl061 driver > will never work with !IRQ_DOMAIN case? You're right unless Grant thinks IRQ_DOMAIN should always be enabled for ARM? Otherwise, I'll add something like this for !IRQ_DOMAIN: static inline void irq_setup_generic_chip_domain( struct irq_chip_generic *gc, struct device_node *node, u32 msk, enum irq_gc_flags flags, unsigned int clr, unsigned int set) { irq_setup_generic_chip(gc, msk, flags, clr, set); } Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding 2012-01-31 14:44 ` Rob Herring @ 2012-02-01 0:07 ` Grant Likely 0 siblings, 0 replies; 11+ messages in thread From: Grant Likely @ 2012-02-01 0:07 UTC (permalink / raw) To: Rob Herring Cc: Shawn Guo, linux-arm-kernel, linux-kernel, b-cousson, Linus Walleij On Tue, Jan 31, 2012 at 08:44:19AM -0600, Rob Herring wrote: > On 01/31/2012 08:36 AM, Shawn Guo wrote: > > On Mon, Jan 30, 2012 at 11:31:39AM -0600, Rob Herring wrote: > > ... > >> -static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base) > >> +static void __init pl061_init_gc(struct pl061_gpio *chip, > >> + struct device_node *node, int irq_base) > >> { > >> struct irq_chip_type *ct; > >> > >> @@ -212,15 +212,17 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base) > >> ct->chip.irq_set_wake = irq_gc_set_wake; > >> ct->regs.mask = GPIOIE; > >> > >> - irq_setup_generic_chip(chip->irq_gc, IRQ_MSK(PL061_GPIO_NR), > >> - IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0); > >> + irq_setup_generic_chip_domain(chip->irq_gc, node, > >> + IRQ_MSK(PL061_GPIO_NR), > >> + IRQ_GC_INIT_NESTED_LOCK, > >> + IRQ_NOREQUEST, 0); > >> } > > > > The function irq_setup_generic_chip_domain() is wrapped by > > #ifdef CONFIG_IRQ_DOMAIN in patch #1. Is it true that pl061 driver > > will never work with !IRQ_DOMAIN case? > > You're right unless Grant thinks IRQ_DOMAIN should always be enabled for > ARM? Otherwise, I'll add something like this for !IRQ_DOMAIN: I think always having IRQ_DOMAIN on are is where we want to get to; or at least have platforms that use it depend on it or select it. I don't want to see interrupt controllers both use irq_domain and also their own open-coded translation mechanism. That's just a fail. If irq_domain isn't acceptable to be enabled all the time, then it needs to be refactored until it is. g. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-02-01 0:07 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-01-30 17:31 [PATCH v3 0/2] generic irq chip and pl061 domain support Rob Herring 2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring 2012-01-31 14:13 ` Shawn Guo 2012-01-31 14:32 ` Rob Herring 2012-01-31 15:06 ` Shawn Guo 2012-01-31 15:37 ` Rob Herring 2012-02-01 0:02 ` Grant Likely 2012-01-30 17:31 ` [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding Rob Herring 2012-01-31 14:36 ` Shawn Guo 2012-01-31 14:44 ` Rob Herring 2012-02-01 0:07 ` Grant Likely
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).