| Message ID | 1416406451-4578-2-git-send-email-yingjoe.chen@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Yingjoe, On Wed, Nov 19 2014 at 2:14:08 pm GMT, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > Add support to use gic as a parent for stacked irq domain. > > Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com> > --- > drivers/irqchip/Kconfig | 1 + > drivers/irqchip/irq-gic.c | 78 ++++++++++++++++++++++++++++++++--------------- > 2 files changed, 55 insertions(+), 24 deletions(-) > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index b21f12f..7f34138 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -5,6 +5,7 @@ config IRQCHIP > config ARM_GIC > bool > select IRQ_DOMAIN > + select IRQ_DOMAIN_HIERARCHY > select MULTI_IRQ_HANDLER > > config GIC_NON_BANKED > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index 38493ff..464dd53 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -788,17 +788,16 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq, > { > if (hw < 32) { > irq_set_percpu_devid(irq); > - irq_set_chip_and_handler(irq, &gic_chip, > - handle_percpu_devid_irq); > + irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data, > + handle_percpu_devid_irq, NULL, NULL); > set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN); > } else { > - irq_set_chip_and_handler(irq, &gic_chip, > - handle_fasteoi_irq); > + irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data, > + handle_fasteoi_irq, NULL, NULL); > set_irq_flags(irq, IRQF_VALID | IRQF_PROBE); > > gic_routable_irq_domain_ops->map(d, irq, hw); > } > - irq_set_chip_data(irq, d->host_data); > return 0; > } > > @@ -858,6 +857,31 @@ static struct notifier_block gic_cpu_notifier = { > }; > #endif > > +static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, > + unsigned int nr_irqs, void *arg) > +{ > + int i, ret; > + irq_hw_number_t hwirq; > + unsigned int type = IRQ_TYPE_NONE; > + struct of_phandle_args *irq_data = arg; > + > + ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args, > + irq_data->args_count, &hwirq, &type); > + if (ret) > + return ret; > + > + for (i = 0; i < nr_irqs; i++) > + gic_irq_domain_map(domain, virq+i, hwirq+i); nit: spacing around '+'. > + > + return 0; > +} > + > +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { > + .xlate = gic_irq_domain_xlate, > + .alloc = gic_irq_domain_alloc, > + .free = irq_domain_free_irqs_top, I'm convinced that irq_domain_free_irqs_top is the wrong function to call here, because you're calling it from the bottom, not the top-level (it has no parent). I cannot verify this with your code as I don't a working platform with GICv2m, but if I enable something similar on GICv3, it dies a very painful way: Unable to handle kernel NULL pointer dereference at virtual address 00000018 pgd = ffffffc03d059000 [00000018] *pgd=0000000081356003, *pud=0000000081356003, *pmd=0000000000000000 Internal error: Oops: 96000006 [#1] SMP Modules linked in: CPU: 4 PID: 1052 Comm: sh Not tainted 3.18.0-rc4+ #3311 task: ffffffc03e320000 ti: ffffffc001390000 task.ti: ffffffc001390000 PC is at irq_domain_free_irqs_recursive+0x1c/0x80 LR is at irq_domain_free_irqs_common+0x88/0x9c pc : [<ffffffc0000ed790>] lr : [<ffffffc0000ede20>] pstate: 60000145 [...] [<ffffffc0000ed790>] irq_domain_free_irqs_recursive+0x1c/0x80 [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c <-- gic_domain.free() [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 [<ffffffc0000ee468>] irq_domain_free_irqs_parent+0x14/0x20 [<ffffffc0003500b8>] its_irq_domain_free+0xc8/0x250 [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c [<ffffffc0000ef518>] msi_domain_free+0x70/0x88 [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 [<ffffffc0000ee3ac>] irq_domain_free_irqs+0x108/0x17c [<ffffffc0000efb68>] msi_domain_free_irqs+0x28/0x4c [<ffffffc000369cac>] free_msi_irqs+0xb4/0x1c0 [<ffffffc00036adec>] pci_disable_msix+0x3c/0x4c [...] and I cannot see how this could work on the standard GIC either. Thomas, Jiang: could you please confirm or infirm my suspicions? My understanding is that irq_domain_free_irqs_top can only be called from the top-level domain. > +}; > + > static const struct irq_domain_ops gic_irq_domain_ops = { > .map = gic_irq_domain_map, > .unmap = gic_irq_domain_unmap, > @@ -948,18 +972,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > gic_cpu_map[i] = 0xff; > > /* > - * For primary GICs, skip over SGIs. > - * For secondary GICs, skip over PPIs, too. > - */ > - if (gic_nr == 0 && (irq_start & 31) > 0) { > - hwirq_base = 16; > - if (irq_start != -1) > - irq_start = (irq_start & ~31) + 16; > - } else { > - hwirq_base = 32; > - } > - > - /* > * Find out how many interrupts are supported. > * The GIC only supports up to 1020 interrupt sources. > */ > @@ -969,10 +981,32 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > gic_irqs = 1020; > gic->gic_irqs = gic_irqs; > > - gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */ > + if (node) { /* DT case */ > + const struct irq_domain_ops *ops = > + &gic_irq_domain_hierarchy_ops; nit: please put this on the same line, even if it is longer than 80 characters. > + > + if (!of_property_read_u32(node, "arm,routable-irqs", > + &nr_routable_irqs)) { > + ops = &gic_irq_domain_ops; > + gic_irqs = nr_routable_irqs; > + } > + > + gic->domain = irq_domain_add_linear(node, gic_irqs, ops, gic); > + } else { /* Non-DT case */ > + /* > + * For primary GICs, skip over SGIs. > + * For secondary GICs, skip over PPIs, too. > + */ > + if (gic_nr == 0 && (irq_start & 31) > 0) { > + hwirq_base = 16; > + if (irq_start != -1) > + irq_start = (irq_start & ~31) + 16; > + } else { > + hwirq_base = 32; > + } > + > + gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */ > > - if (of_property_read_u32(node, "arm,routable-irqs", > - &nr_routable_irqs)) { > irq_base = irq_alloc_descs(irq_start, 16, gic_irqs, > numa_node_id()); > if (IS_ERR_VALUE(irq_base)) { > @@ -983,10 +1017,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > > gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base, > hwirq_base, &gic_irq_domain_ops, gic); > - } else { > - gic->domain = irq_domain_add_linear(node, nr_routable_irqs, > - &gic_irq_domain_ops, > - gic); > } > > if (WARN_ON(!gic->domain)) Thanks, M.
Hi Marc, On Wed, 2014-11-19 at 17:18 +0000, Marc Zyngier wrote: > > +static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, > > + unsigned int nr_irqs, void *arg) > > +{ > > + int i, ret; > > + irq_hw_number_t hwirq; > > + unsigned int type = IRQ_TYPE_NONE; > > + struct of_phandle_args *irq_data = arg; > > + > > + ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args, > > + irq_data->args_count, &hwirq, &type); > > + if (ret) > > + return ret; > > + > > + for (i = 0; i < nr_irqs; i++) > > + gic_irq_domain_map(domain, virq+i, hwirq+i); > > nit: spacing around '+'. OK. > > + > > + return 0; > > +} > > + > > +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { > > + .xlate = gic_irq_domain_xlate, > > + .alloc = gic_irq_domain_alloc, > > + .free = irq_domain_free_irqs_top, > > I'm convinced that irq_domain_free_irqs_top is the wrong function to > call here, because you're calling it from the bottom, not the top-level > (it has no parent). Base on the name, I though this is helper function for top level irq_domain? > I cannot verify this with your code as I don't a working platform with > GICv2m, but if I enable something similar on GICv3, it dies a very > painful way: > > Unable to handle kernel NULL pointer dereference at virtual address 00000018 > pgd = ffffffc03d059000 > [00000018] *pgd=0000000081356003, *pud=0000000081356003, *pmd=0000000000000000 > Internal error: Oops: 96000006 [#1] SMP > Modules linked in: > CPU: 4 PID: 1052 Comm: sh Not tainted 3.18.0-rc4+ #3311 > task: ffffffc03e320000 ti: ffffffc001390000 task.ti: ffffffc001390000 > PC is at irq_domain_free_irqs_recursive+0x1c/0x80 > LR is at irq_domain_free_irqs_common+0x88/0x9c > pc : [<ffffffc0000ed790>] lr : [<ffffffc0000ede20>] pstate: 60000145 > [...] > [<ffffffc0000ed790>] irq_domain_free_irqs_recursive+0x1c/0x80 > [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c > [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c <-- gic_domain.free() > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > [<ffffffc0000ee468>] irq_domain_free_irqs_parent+0x14/0x20 > [<ffffffc0003500b8>] its_irq_domain_free+0xc8/0x250 > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c > [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c > [<ffffffc0000ef518>] msi_domain_free+0x70/0x88 > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > [<ffffffc0000ee3ac>] irq_domain_free_irqs+0x108/0x17c > [<ffffffc0000efb68>] msi_domain_free_irqs+0x28/0x4c > [<ffffffc000369cac>] free_msi_irqs+0xb4/0x1c0 > [<ffffffc00036adec>] pci_disable_msix+0x3c/0x4c > [...] > > and I cannot see how this could work on the standard GIC either. I'm sorry, I just realize my testcase was too simple, irqs are populated by device tree and never got freed. I'll add that and test it again. > Thomas, Jiang: could you please confirm or infirm my suspicions? My > understanding is that irq_domain_free_irqs_top can only be called from > the top-level domain. > > > +}; > > + > > static const struct irq_domain_ops gic_irq_domain_ops = { > > .map = gic_irq_domain_map, > > .unmap = gic_irq_domain_unmap, > > @@ -948,18 +972,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > > gic_cpu_map[i] = 0xff; > > > > /* > > - * For primary GICs, skip over SGIs. > > - * For secondary GICs, skip over PPIs, too. > > - */ > > - if (gic_nr == 0 && (irq_start & 31) > 0) { > > - hwirq_base = 16; > > - if (irq_start != -1) > > - irq_start = (irq_start & ~31) + 16; > > - } else { > > - hwirq_base = 32; > > - } > > - > > - /* > > * Find out how many interrupts are supported. > > * The GIC only supports up to 1020 interrupt sources. > > */ > > @@ -969,10 +981,32 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > > gic_irqs = 1020; > > gic->gic_irqs = gic_irqs; > > > > - gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */ > > + if (node) { /* DT case */ > > + const struct irq_domain_ops *ops = > > + &gic_irq_domain_hierarchy_ops; > > nit: please put this on the same line, even if it is longer than 80 > characters. ok. Joe.C
On 2014/11/20 1:18, Marc Zyngier wrote: > Hi Yingjoe, > > On Wed, Nov 19 2014 at 2:14:08 pm GMT, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: >> Add support to use gic as a parent for stacked irq domain. >> >> Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com> >> --- >> drivers/irqchip/Kconfig | 1 + >> drivers/irqchip/irq-gic.c | 78 ++++++++++++++++++++++++++++++++--------------- >> 2 files changed, 55 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig >> index b21f12f..7f34138 100644 >> --- a/drivers/irqchip/Kconfig >> +++ b/drivers/irqchip/Kconfig >> @@ -5,6 +5,7 @@ config IRQCHIP >> config ARM_GIC >> bool >> select IRQ_DOMAIN >> + select IRQ_DOMAIN_HIERARCHY >> select MULTI_IRQ_HANDLER >> >> config GIC_NON_BANKED >> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c >> index 38493ff..464dd53 100644 >> --- a/drivers/irqchip/irq-gic.c >> +++ b/drivers/irqchip/irq-gic.c >> @@ -788,17 +788,16 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq, >> { >> if (hw < 32) { >> irq_set_percpu_devid(irq); >> - irq_set_chip_and_handler(irq, &gic_chip, >> - handle_percpu_devid_irq); >> + irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data, >> + handle_percpu_devid_irq, NULL, NULL); >> set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN); >> } else { >> - irq_set_chip_and_handler(irq, &gic_chip, >> - handle_fasteoi_irq); >> + irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data, >> + handle_fasteoi_irq, NULL, NULL); >> set_irq_flags(irq, IRQF_VALID | IRQF_PROBE); >> >> gic_routable_irq_domain_ops->map(d, irq, hw); >> } >> - irq_set_chip_data(irq, d->host_data); >> return 0; >> } >> >> @@ -858,6 +857,31 @@ static struct notifier_block gic_cpu_notifier = { >> }; >> #endif >> >> +static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, >> + unsigned int nr_irqs, void *arg) >> +{ >> + int i, ret; >> + irq_hw_number_t hwirq; >> + unsigned int type = IRQ_TYPE_NONE; >> + struct of_phandle_args *irq_data = arg; >> + >> + ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args, >> + irq_data->args_count, &hwirq, &type); >> + if (ret) >> + return ret; >> + >> + for (i = 0; i < nr_irqs; i++) >> + gic_irq_domain_map(domain, virq+i, hwirq+i); > > nit: spacing around '+'. > >> + >> + return 0; >> +} >> + >> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { >> + .xlate = gic_irq_domain_xlate, >> + .alloc = gic_irq_domain_alloc, >> + .free = irq_domain_free_irqs_top, > > I'm convinced that irq_domain_free_irqs_top is the wrong function to > call here, because you're calling it from the bottom, not the top-level > (it has no parent). > > I cannot verify this with your code as I don't a working platform with > GICv2m, but if I enable something similar on GICv3, it dies a very > painful way: > > Unable to handle kernel NULL pointer dereference at virtual address 00000018 > pgd = ffffffc03d059000 > [00000018] *pgd=0000000081356003, *pud=0000000081356003, *pmd=0000000000000000 > Internal error: Oops: 96000006 [#1] SMP > Modules linked in: > CPU: 4 PID: 1052 Comm: sh Not tainted 3.18.0-rc4+ #3311 > task: ffffffc03e320000 ti: ffffffc001390000 task.ti: ffffffc001390000 > PC is at irq_domain_free_irqs_recursive+0x1c/0x80 > LR is at irq_domain_free_irqs_common+0x88/0x9c > pc : [<ffffffc0000ed790>] lr : [<ffffffc0000ede20>] pstate: 60000145 > [...] > [<ffffffc0000ed790>] irq_domain_free_irqs_recursive+0x1c/0x80 > [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c > [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c <-- gic_domain.free() > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > [<ffffffc0000ee468>] irq_domain_free_irqs_parent+0x14/0x20 > [<ffffffc0003500b8>] its_irq_domain_free+0xc8/0x250 > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c > [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c > [<ffffffc0000ef518>] msi_domain_free+0x70/0x88 > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > [<ffffffc0000ee3ac>] irq_domain_free_irqs+0x108/0x17c > [<ffffffc0000efb68>] msi_domain_free_irqs+0x28/0x4c > [<ffffffc000369cac>] free_msi_irqs+0xb4/0x1c0 > [<ffffffc00036adec>] pci_disable_msix+0x3c/0x4c > [...] > > and I cannot see how this could work on the standard GIC either. > > Thomas, Jiang: could you please confirm or infirm my suspicions? My > understanding is that irq_domain_free_irqs_top can only be called from > the top-level domain. Hi Marc, It indicates that irq_domain_free_irqs_top() is not a good name. We have: 1) irq_domain_set_hwirq_and_chip() to set irq_chip and chip_data 2) irq_domain_set_info() to set irq_chip, chip_data, flow_handler and handler_data; 3) irq_domain_reset_irq_data() resets irq_chip and chip_data. 4) irq_domain_free_irqs_common() resets irq_chip, chip_data and calls parent domain's domain_ops.free() callback. 5) irq_domain_free_irqs_top() resets irq_chip, chip_data, flow handler, handler_data and call parent domain's domain_ops.free() callback. So there two possible improvements here: 1) Rename irq_domain_free_irqs_top() with better name, any suggestions? It's named as is because it's always called by the outer-most irqdomains on x86. 2) Change irq_domain_free_irqs_common() and irq_domain_free_irqs_top() to call parent domain's domain_ops.free() callback only if parent exists. By this way, they could be used for inner-most irqdomains. If OK, I will respin a version 4 patch set based on tip/irq/irqdomain. Thoughts? Regards! Gerry > >> +}; >> + >> static const struct irq_domain_ops gic_irq_domain_ops = { >> .map = gic_irq_domain_map, >> .unmap = gic_irq_domain_unmap, >> @@ -948,18 +972,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, >> gic_cpu_map[i] = 0xff; >> >> /* >> - * For primary GICs, skip over SGIs. >> - * For secondary GICs, skip over PPIs, too. >> - */ >> - if (gic_nr == 0 && (irq_start & 31) > 0) { >> - hwirq_base = 16; >> - if (irq_start != -1) >> - irq_start = (irq_start & ~31) + 16; >> - } else { >> - hwirq_base = 32; >> - } >> - >> - /* >> * Find out how many interrupts are supported. >> * The GIC only supports up to 1020 interrupt sources. >> */ >> @@ -969,10 +981,32 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, >> gic_irqs = 1020; >> gic->gic_irqs = gic_irqs; >> >> - gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */ >> + if (node) { /* DT case */ >> + const struct irq_domain_ops *ops = >> + &gic_irq_domain_hierarchy_ops; > > nit: please put this on the same line, even if it is longer than 80 > characters. > >> + >> + if (!of_property_read_u32(node, "arm,routable-irqs", >> + &nr_routable_irqs)) { >> + ops = &gic_irq_domain_ops; >> + gic_irqs = nr_routable_irqs; >> + } >> + >> + gic->domain = irq_domain_add_linear(node, gic_irqs, ops, gic); >> + } else { /* Non-DT case */ >> + /* >> + * For primary GICs, skip over SGIs. >> + * For secondary GICs, skip over PPIs, too. >> + */ >> + if (gic_nr == 0 && (irq_start & 31) > 0) { >> + hwirq_base = 16; >> + if (irq_start != -1) >> + irq_start = (irq_start & ~31) + 16; >> + } else { >> + hwirq_base = 32; >> + } >> + >> + gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */ >> >> - if (of_property_read_u32(node, "arm,routable-irqs", >> - &nr_routable_irqs)) { >> irq_base = irq_alloc_descs(irq_start, 16, gic_irqs, >> numa_node_id()); >> if (IS_ERR_VALUE(irq_base)) { >> @@ -983,10 +1017,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, >> >> gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base, >> hwirq_base, &gic_irq_domain_ops, gic); >> - } else { >> - gic->domain = irq_domain_add_linear(node, nr_routable_irqs, >> - &gic_irq_domain_ops, >> - gic); >> } >> >> if (WARN_ON(!gic->domain)) > > Thanks, > > M. >
Hi Marc, On Thu, 2014-11-20 at 11:57 +0800, Yingjoe Chen wrote: > On Wed, 2014-11-19 at 17:18 +0000, Marc Zyngier wrote: > > > + > > > + return 0; > > > +} > > > + > > > +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { > > > + .xlate = gic_irq_domain_xlate, > > > + .alloc = gic_irq_domain_alloc, > > > + .free = irq_domain_free_irqs_top, > > > > I'm convinced that irq_domain_free_irqs_top is the wrong function to > > call here, because you're calling it from the bottom, not the top-level > > (it has no parent). > > Base on the name, I though this is helper function for top level > irq_domain? > > > I cannot verify this with your code as I don't a working platform with > > GICv2m, but if I enable something similar on GICv3, it dies a very > > painful way: > > > > Unable to handle kernel NULL pointer dereference at virtual address 00000018 > > pgd = ffffffc03d059000 > > [00000018] *pgd=0000000081356003, *pud=0000000081356003, *pmd=0000000000000000 > > Internal error: Oops: 96000006 [#1] SMP > > Modules linked in: > > CPU: 4 PID: 1052 Comm: sh Not tainted 3.18.0-rc4+ #3311 > > task: ffffffc03e320000 ti: ffffffc001390000 task.ti: ffffffc001390000 > > PC is at irq_domain_free_irqs_recursive+0x1c/0x80 > > LR is at irq_domain_free_irqs_common+0x88/0x9c > > pc : [<ffffffc0000ed790>] lr : [<ffffffc0000ede20>] pstate: 60000145 > > [...] > > [<ffffffc0000ed790>] irq_domain_free_irqs_recursive+0x1c/0x80 > > [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c > > [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c <-- gic_domain.free() > > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > > [<ffffffc0000ee468>] irq_domain_free_irqs_parent+0x14/0x20 > > [<ffffffc0003500b8>] its_irq_domain_free+0xc8/0x250 > > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > > [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c > > [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c > > [<ffffffc0000ef518>] msi_domain_free+0x70/0x88 > > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > > [<ffffffc0000ee3ac>] irq_domain_free_irqs+0x108/0x17c > > [<ffffffc0000efb68>] msi_domain_free_irqs+0x28/0x4c > > [<ffffffc000369cac>] free_msi_irqs+0xb4/0x1c0 > > [<ffffffc00036adec>] pci_disable_msix+0x3c/0x4c > > [...] > > > > and I cannot see how this could work on the standard GIC either. > > I'm sorry, I just realize my testcase was too simple, irqs are populated > by device tree and never got freed. I'll add that and test it again. On a second thoughts, unlike the MSI cases, gic_irq_domain_hierarchy_ops is only used when we use DT, so we probably will never use the free function. Is it OK to remove the free support here? Joe.C
On Thu, Nov 20 2014 at 4:26:10 am GMT, Jiang Liu <jiang.liu@linux.intel.com> wrote: Hi Jiang, > On 2014/11/20 1:18, Marc Zyngier wrote: >> Hi Yingjoe, >> >> On Wed, Nov 19 2014 at 2:14:08 pm GMT, Yingjoe Chen >> <yingjoe.chen@mediatek.com> wrote: >>> + >>> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { >>> + .xlate = gic_irq_domain_xlate, >>> + .alloc = gic_irq_domain_alloc, >>> + .free = irq_domain_free_irqs_top, >> >> I'm convinced that irq_domain_free_irqs_top is the wrong function to >> call here, because you're calling it from the bottom, not the top-level >> (it has no parent). >> >> I cannot verify this with your code as I don't a working platform with >> GICv2m, but if I enable something similar on GICv3, it dies a very >> painful way: >> >> Unable to handle kernel NULL pointer dereference at virtual address 00000018 >> pgd = ffffffc03d059000 >> [00000018] *pgd=0000000081356003, *pud=0000000081356003, *pmd=0000000000000000 >> Internal error: Oops: 96000006 [#1] SMP >> Modules linked in: >> CPU: 4 PID: 1052 Comm: sh Not tainted 3.18.0-rc4+ #3311 >> task: ffffffc03e320000 ti: ffffffc001390000 task.ti: ffffffc001390000 >> PC is at irq_domain_free_irqs_recursive+0x1c/0x80 >> LR is at irq_domain_free_irqs_common+0x88/0x9c >> pc : [<ffffffc0000ed790>] lr : [<ffffffc0000ede20>] pstate: 60000145 >> [...] >> [<ffffffc0000ed790>] irq_domain_free_irqs_recursive+0x1c/0x80 >> [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c >> [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c <-- gic_domain.free() >> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >> [<ffffffc0000ee468>] irq_domain_free_irqs_parent+0x14/0x20 >> [<ffffffc0003500b8>] its_irq_domain_free+0xc8/0x250 >> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >> [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c >> [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c >> [<ffffffc0000ef518>] msi_domain_free+0x70/0x88 >> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >> [<ffffffc0000ee3ac>] irq_domain_free_irqs+0x108/0x17c >> [<ffffffc0000efb68>] msi_domain_free_irqs+0x28/0x4c >> [<ffffffc000369cac>] free_msi_irqs+0xb4/0x1c0 >> [<ffffffc00036adec>] pci_disable_msix+0x3c/0x4c >> [...] >> >> and I cannot see how this could work on the standard GIC either. >> >> Thomas, Jiang: could you please confirm or infirm my suspicions? My >> understanding is that irq_domain_free_irqs_top can only be called from >> the top-level domain. > Hi Marc, > It indicates that irq_domain_free_irqs_top() is not a good name. > We have: > 1) irq_domain_set_hwirq_and_chip() to set irq_chip and chip_data > 2) irq_domain_set_info() to set irq_chip, chip_data, flow_handler and > handler_data; > 3) irq_domain_reset_irq_data() resets irq_chip and chip_data. > 4) irq_domain_free_irqs_common() resets irq_chip, chip_data and calls > parent domain's domain_ops.free() callback. > 5) irq_domain_free_irqs_top() resets irq_chip, chip_data, flow handler, > handler_data and call parent domain's domain_ops.free() callback. Yes, and this "call parent domain's free callback" is where the problem lies. Here, it is called from the innermost domain, with no parent. > So there two possible improvements here: > 1) Rename irq_domain_free_irqs_top() with better name, any suggestions? > It's named as is because it's always called by the outer-most > irqdomains on x86. > 2) Change irq_domain_free_irqs_common() and irq_domain_free_irqs_top() > to call parent domain's domain_ops.free() callback only if parent > exists. By this way, they could be used for inner-most irqdomains. > If OK, I will respin a version 4 patch set based on tip/irq/irqdomain. > Thoughts? Checking the parent is probably a safe solution (this is not a hot path anyway). I don't care much about the name though, and I the only thing I can think of is irq_domain_free_irqs_reset_flow, which looks so bad it's not even funny. I'll let the matter rest in your capable hands! ;-) Thanks, M.
Hi Joe, On Thu, Nov 20 2014 at 9:41:51 am GMT, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote: > Hi Marc, > > On Thu, 2014-11-20 at 11:57 +0800, Yingjoe Chen wrote: >> On Wed, 2014-11-19 at 17:18 +0000, Marc Zyngier wrote: >> > > + >> > > + return 0; >> > > +} >> > > + >> > > +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { >> > > + .xlate = gic_irq_domain_xlate, >> > > + .alloc = gic_irq_domain_alloc, >> > > + .free = irq_domain_free_irqs_top, >> > >> > I'm convinced that irq_domain_free_irqs_top is the wrong function to >> > call here, because you're calling it from the bottom, not the top-level >> > (it has no parent). >> >> Base on the name, I though this is helper function for top level >> irq_domain? >> >> > I cannot verify this with your code as I don't a working platform with >> > GICv2m, but if I enable something similar on GICv3, it dies a very >> > painful way: >> > >> > Unable to handle kernel NULL pointer dereference at virtual address 00000018 >> > pgd = ffffffc03d059000 >> > [00000018] *pgd=0000000081356003, *pud=0000000081356003, >> > *pmd=0000000000000000 >> > Internal error: Oops: 96000006 [#1] SMP >> > Modules linked in: >> > CPU: 4 PID: 1052 Comm: sh Not tainted 3.18.0-rc4+ #3311 >> > task: ffffffc03e320000 ti: ffffffc001390000 task.ti: ffffffc001390000 >> > PC is at irq_domain_free_irqs_recursive+0x1c/0x80 >> > LR is at irq_domain_free_irqs_common+0x88/0x9c >> > pc : [<ffffffc0000ed790>] lr : [<ffffffc0000ede20>] pstate: 60000145 >> > [...] >> > [<ffffffc0000ed790>] irq_domain_free_irqs_recursive+0x1c/0x80 >> > [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c >> > [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c <-- >> > gic_domain.free() >> > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >> > [<ffffffc0000ee468>] irq_domain_free_irqs_parent+0x14/0x20 >> > [<ffffffc0003500b8>] its_irq_domain_free+0xc8/0x250 >> > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >> > [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c >> > [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c >> > [<ffffffc0000ef518>] msi_domain_free+0x70/0x88 >> > [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >> > [<ffffffc0000ee3ac>] irq_domain_free_irqs+0x108/0x17c >> > [<ffffffc0000efb68>] msi_domain_free_irqs+0x28/0x4c >> > [<ffffffc000369cac>] free_msi_irqs+0xb4/0x1c0 >> > [<ffffffc00036adec>] pci_disable_msix+0x3c/0x4c >> > [...] >> > >> > and I cannot see how this could work on the standard GIC either. >> >> I'm sorry, I just realize my testcase was too simple, irqs are populated >> by device tree and never got freed. I'll add that and test it again. > > On a second thoughts, unlike the MSI cases, gic_irq_domain_hierarchy_ops > is only used when we use DT, so we probably will never use the free > function. Is it OK to remove the free support here? Well, such thing is coming with GICv2m (SPIs are allocated out of DT). You can drop it if you want, but I will then have to add it back (which seems like a waste of time). I'd prefer if you kept it in with the rest of the conversion. Thanks, M.
Hi, On Thu, 2014-11-20 at 10:07 +0000, Marc Zyngier wrote: > On Thu, Nov 20 2014 at 4:26:10 am GMT, Jiang Liu <jiang.liu@linux.intel.com> wrote: > > Hi Jiang, > > > On 2014/11/20 1:18, Marc Zyngier wrote: > >> Hi Yingjoe, > >> > >> On Wed, Nov 19 2014 at 2:14:08 pm GMT, Yingjoe Chen > >> <yingjoe.chen@mediatek.com> wrote: > >>> + > >>> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { > >>> + .xlate = gic_irq_domain_xlate, > >>> + .alloc = gic_irq_domain_alloc, > >>> + .free = irq_domain_free_irqs_top, > >> > >> I'm convinced that irq_domain_free_irqs_top is the wrong function to > >> call here, because you're calling it from the bottom, not the top-level > >> (it has no parent). > >> > >> I cannot verify this with your code as I don't a working platform with > >> GICv2m, but if I enable something similar on GICv3, it dies a very > >> painful way: > >> > >> Unable to handle kernel NULL pointer dereference at virtual address 00000018 > >> pgd = ffffffc03d059000 > >> [00000018] *pgd=0000000081356003, *pud=0000000081356003, *pmd=0000000000000000 > >> Internal error: Oops: 96000006 [#1] SMP > >> Modules linked in: > >> CPU: 4 PID: 1052 Comm: sh Not tainted 3.18.0-rc4+ #3311 > >> task: ffffffc03e320000 ti: ffffffc001390000 task.ti: ffffffc001390000 > >> PC is at irq_domain_free_irqs_recursive+0x1c/0x80 > >> LR is at irq_domain_free_irqs_common+0x88/0x9c > >> pc : [<ffffffc0000ed790>] lr : [<ffffffc0000ede20>] pstate: 60000145 > >> [...] > >> [<ffffffc0000ed790>] irq_domain_free_irqs_recursive+0x1c/0x80 > >> [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c > >> [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c <-- gic_domain.free() > >> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > >> [<ffffffc0000ee468>] irq_domain_free_irqs_parent+0x14/0x20 > >> [<ffffffc0003500b8>] its_irq_domain_free+0xc8/0x250 > >> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > >> [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c > >> [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c > >> [<ffffffc0000ef518>] msi_domain_free+0x70/0x88 > >> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 > >> [<ffffffc0000ee3ac>] irq_domain_free_irqs+0x108/0x17c > >> [<ffffffc0000efb68>] msi_domain_free_irqs+0x28/0x4c > >> [<ffffffc000369cac>] free_msi_irqs+0xb4/0x1c0 > >> [<ffffffc00036adec>] pci_disable_msix+0x3c/0x4c > >> [...] > >> > >> and I cannot see how this could work on the standard GIC either. > >> > >> Thomas, Jiang: could you please confirm or infirm my suspicions? My > >> understanding is that irq_domain_free_irqs_top can only be called from > >> the top-level domain. > > Hi Marc, > > It indicates that irq_domain_free_irqs_top() is not a good name. > > We have: > > 1) irq_domain_set_hwirq_and_chip() to set irq_chip and chip_data > > 2) irq_domain_set_info() to set irq_chip, chip_data, flow_handler and > > handler_data; > > 3) irq_domain_reset_irq_data() resets irq_chip and chip_data. > > 4) irq_domain_free_irqs_common() resets irq_chip, chip_data and calls > > parent domain's domain_ops.free() callback. > > 5) irq_domain_free_irqs_top() resets irq_chip, chip_data, flow handler, > > handler_data and call parent domain's domain_ops.free() callback. > > Yes, and this "call parent domain's free callback" is where the problem > lies. Here, it is called from the innermost domain, with no parent. > > > So there two possible improvements here: > > 1) Rename irq_domain_free_irqs_top() with better name, any suggestions? > > It's named as is because it's always called by the outer-most > > irqdomains on x86. > > 2) Change irq_domain_free_irqs_common() and irq_domain_free_irqs_top() > > to call parent domain's domain_ops.free() callback only if parent > > exists. By this way, they could be used for inner-most irqdomains. > > If OK, I will respin a version 4 patch set based on tip/irq/irqdomain. > > Thoughts? > > Checking the parent is probably a safe solution (this is not a hot path > anyway). I don't care much about the name though, and I the only thing I > can think of is irq_domain_free_irqs_reset_flow, which looks so bad it's > not even funny. I'll let the matter rest in your capable hands! ;-) I've applied Jiang's "irqdomain: Enhance irq_domain_free_irqs_common() to support parentless irqdomain" patch and it did fix the crash. Thanks Jiang, Marc Joe.C
On 21/11/14 15:51, Yingjoe Chen wrote: > > Hi, > > On Thu, 2014-11-20 at 10:07 +0000, Marc Zyngier wrote: >> On Thu, Nov 20 2014 at 4:26:10 am GMT, Jiang Liu <jiang.liu@linux.intel.com> wrote: >> >> Hi Jiang, >> >>> On 2014/11/20 1:18, Marc Zyngier wrote: >>>> Hi Yingjoe, >>>> >>>> On Wed, Nov 19 2014 at 2:14:08 pm GMT, Yingjoe Chen >>>> <yingjoe.chen@mediatek.com> wrote: >>>>> + >>>>> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { >>>>> + .xlate = gic_irq_domain_xlate, >>>>> + .alloc = gic_irq_domain_alloc, >>>>> + .free = irq_domain_free_irqs_top, >>>> >>>> I'm convinced that irq_domain_free_irqs_top is the wrong function to >>>> call here, because you're calling it from the bottom, not the top-level >>>> (it has no parent). >>>> >>>> I cannot verify this with your code as I don't a working platform with >>>> GICv2m, but if I enable something similar on GICv3, it dies a very >>>> painful way: >>>> >>>> Unable to handle kernel NULL pointer dereference at virtual address 00000018 >>>> pgd = ffffffc03d059000 >>>> [00000018] *pgd=0000000081356003, *pud=0000000081356003, *pmd=0000000000000000 >>>> Internal error: Oops: 96000006 [#1] SMP >>>> Modules linked in: >>>> CPU: 4 PID: 1052 Comm: sh Not tainted 3.18.0-rc4+ #3311 >>>> task: ffffffc03e320000 ti: ffffffc001390000 task.ti: ffffffc001390000 >>>> PC is at irq_domain_free_irqs_recursive+0x1c/0x80 >>>> LR is at irq_domain_free_irqs_common+0x88/0x9c >>>> pc : [<ffffffc0000ed790>] lr : [<ffffffc0000ede20>] pstate: 60000145 >>>> [...] >>>> [<ffffffc0000ed790>] irq_domain_free_irqs_recursive+0x1c/0x80 >>>> [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c >>>> [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c <-- gic_domain.free() >>>> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >>>> [<ffffffc0000ee468>] irq_domain_free_irqs_parent+0x14/0x20 >>>> [<ffffffc0003500b8>] its_irq_domain_free+0xc8/0x250 >>>> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >>>> [<ffffffc0000ede1c>] irq_domain_free_irqs_common+0x84/0x9c >>>> [<ffffffc0000ede98>] irq_domain_free_irqs_top+0x64/0x7c >>>> [<ffffffc0000ef518>] msi_domain_free+0x70/0x88 >>>> [<ffffffc0000ed798>] irq_domain_free_irqs_recursive+0x24/0x80 >>>> [<ffffffc0000ee3ac>] irq_domain_free_irqs+0x108/0x17c >>>> [<ffffffc0000efb68>] msi_domain_free_irqs+0x28/0x4c >>>> [<ffffffc000369cac>] free_msi_irqs+0xb4/0x1c0 >>>> [<ffffffc00036adec>] pci_disable_msix+0x3c/0x4c >>>> [...] >>>> >>>> and I cannot see how this could work on the standard GIC either. >>>> >>>> Thomas, Jiang: could you please confirm or infirm my suspicions? My >>>> understanding is that irq_domain_free_irqs_top can only be called from >>>> the top-level domain. >>> Hi Marc, >>> It indicates that irq_domain_free_irqs_top() is not a good name. >>> We have: >>> 1) irq_domain_set_hwirq_and_chip() to set irq_chip and chip_data >>> 2) irq_domain_set_info() to set irq_chip, chip_data, flow_handler and >>> handler_data; >>> 3) irq_domain_reset_irq_data() resets irq_chip and chip_data. >>> 4) irq_domain_free_irqs_common() resets irq_chip, chip_data and calls >>> parent domain's domain_ops.free() callback. >>> 5) irq_domain_free_irqs_top() resets irq_chip, chip_data, flow handler, >>> handler_data and call parent domain's domain_ops.free() callback. >> >> Yes, and this "call parent domain's free callback" is where the problem >> lies. Here, it is called from the innermost domain, with no parent. >> >>> So there two possible improvements here: >>> 1) Rename irq_domain_free_irqs_top() with better name, any suggestions? >>> It's named as is because it's always called by the outer-most >>> irqdomains on x86. >>> 2) Change irq_domain_free_irqs_common() and irq_domain_free_irqs_top() >>> to call parent domain's domain_ops.free() callback only if parent >>> exists. By this way, they could be used for inner-most irqdomains. >>> If OK, I will respin a version 4 patch set based on tip/irq/irqdomain. >>> Thoughts? >> >> Checking the parent is probably a safe solution (this is not a hot path >> anyway). I don't care much about the name though, and I the only thing I >> can think of is irq_domain_free_irqs_reset_flow, which looks so bad it's >> not even funny. I'll let the matter rest in your capable hands! ;-) > > I've applied Jiang's "irqdomain: Enhance irq_domain_free_irqs_common() > to support parentless irqdomain" patch and it did fix the crash. Excellent. I think this is pretty much getting there now. Any chance you could repost this series with the various fixes? Thanks, M.
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index b21f12f..7f34138 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -5,6 +5,7 @@ config IRQCHIP config ARM_GIC bool select IRQ_DOMAIN + select IRQ_DOMAIN_HIERARCHY select MULTI_IRQ_HANDLER config GIC_NON_BANKED diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff..464dd53 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -788,17 +788,16 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq, { if (hw < 32) { irq_set_percpu_devid(irq); - irq_set_chip_and_handler(irq, &gic_chip, - handle_percpu_devid_irq); + irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data, + handle_percpu_devid_irq, NULL, NULL); set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN); } else { - irq_set_chip_and_handler(irq, &gic_chip, - handle_fasteoi_irq); + irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data, + handle_fasteoi_irq, NULL, NULL); set_irq_flags(irq, IRQF_VALID | IRQF_PROBE); gic_routable_irq_domain_ops->map(d, irq, hw); } - irq_set_chip_data(irq, d->host_data); return 0; } @@ -858,6 +857,31 @@ static struct notifier_block gic_cpu_notifier = { }; #endif +static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, + unsigned int nr_irqs, void *arg) +{ + int i, ret; + irq_hw_number_t hwirq; + unsigned int type = IRQ_TYPE_NONE; + struct of_phandle_args *irq_data = arg; + + ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args, + irq_data->args_count, &hwirq, &type); + if (ret) + return ret; + + for (i = 0; i < nr_irqs; i++) + gic_irq_domain_map(domain, virq+i, hwirq+i); + + return 0; +} + +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = { + .xlate = gic_irq_domain_xlate, + .alloc = gic_irq_domain_alloc, + .free = irq_domain_free_irqs_top, +}; + static const struct irq_domain_ops gic_irq_domain_ops = { .map = gic_irq_domain_map, .unmap = gic_irq_domain_unmap, @@ -948,18 +972,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_cpu_map[i] = 0xff; /* - * For primary GICs, skip over SGIs. - * For secondary GICs, skip over PPIs, too. - */ - if (gic_nr == 0 && (irq_start & 31) > 0) { - hwirq_base = 16; - if (irq_start != -1) - irq_start = (irq_start & ~31) + 16; - } else { - hwirq_base = 32; - } - - /* * Find out how many interrupts are supported. * The GIC only supports up to 1020 interrupt sources. */ @@ -969,10 +981,32 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_irqs = 1020; gic->gic_irqs = gic_irqs; - gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */ + if (node) { /* DT case */ + const struct irq_domain_ops *ops = + &gic_irq_domain_hierarchy_ops; + + if (!of_property_read_u32(node, "arm,routable-irqs", + &nr_routable_irqs)) { + ops = &gic_irq_domain_ops; + gic_irqs = nr_routable_irqs; + } + + gic->domain = irq_domain_add_linear(node, gic_irqs, ops, gic); + } else { /* Non-DT case */ + /* + * For primary GICs, skip over SGIs. + * For secondary GICs, skip over PPIs, too. + */ + if (gic_nr == 0 && (irq_start & 31) > 0) { + hwirq_base = 16; + if (irq_start != -1) + irq_start = (irq_start & ~31) + 16; + } else { + hwirq_base = 32; + } + + gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */ - if (of_property_read_u32(node, "arm,routable-irqs", - &nr_routable_irqs)) { irq_base = irq_alloc_descs(irq_start, 16, gic_irqs, numa_node_id()); if (IS_ERR_VALUE(irq_base)) { @@ -983,10 +1017,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base, hwirq_base, &gic_irq_domain_ops, gic); - } else { - gic->domain = irq_domain_add_linear(node, nr_routable_irqs, - &gic_irq_domain_ops, - gic); } if (WARN_ON(!gic->domain))
Add support to use gic as a parent for stacked irq domain. Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com> --- drivers/irqchip/Kconfig | 1 + drivers/irqchip/irq-gic.c | 78 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 24 deletions(-)