| Message ID | 1416902662-19281-2-git-send-email-yingjoe.chen@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25/11/14 08:04, Yingjoe Chen wrote: > Add support to use gic as a parent for stacked irq domain. > > Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com> Looks good to me. Acked-by: Marc Zyngier <marc.zyngier@arm.com> M.
On Tue, Nov 25, 2014 at 9:04 AM, 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> This change (which is now in -next as commit 9a1091ef0017c40a) breaks booting on r8a7740/armadillo-legacy. It hangs because the timers cannot get their interrupts: console [tty0] enabled sh-tmu.0: ch0: used for clock events sh-tmu.0: ch0: used for periodic clock events sh-tmu.0: ch0: failed to request irq 230 sh-tmu.0: ch1: used as clock source sh-cmt-48.1: ch0: failed to request irq 90 sh-cmt-48.1: ch0: registration failed earlytimer: unable to probe sh-cmt-48 early. Calibrating delay loop... > --- > drivers/irqchip/Kconfig | 1 + > drivers/irqchip/irq-gic.c | 77 ++++++++++++++++++++++++++++++++--------------- > 2 files changed, 54 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..ab6069b 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,31 @@ 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 +1016,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)) > -- > 1.8.1.1.dirty Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 02/12/14 13:27, Geert Uytterhoeven wrote: > On Tue, Nov 25, 2014 at 9:04 AM, 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> > > This change (which is now in -next as commit 9a1091ef0017c40a) breaks > booting on r8a7740/armadillo-legacy. > > It hangs because the timers cannot get their interrupts: > > console [tty0] enabled > sh-tmu.0: ch0: used for clock events > sh-tmu.0: ch0: used for periodic clock events > sh-tmu.0: ch0: failed to request irq 230 > sh-tmu.0: ch1: used as clock source > sh-cmt-48.1: ch0: failed to request irq 90 > sh-cmt-48.1: ch0: registration failed > earlytimer: unable to probe sh-cmt-48 early. > Calibrating delay loop... Grmbl... Are you, by any (lack of) chance, using a setup where the GIC is probed via DT, but the timers have their IRQs hardcoded? M.
Hi Marc, On Tue, Dec 2, 2014 at 2:37 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > On 02/12/14 13:27, Geert Uytterhoeven wrote: >> On Tue, Nov 25, 2014 at 9:04 AM, 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> >> >> This change (which is now in -next as commit 9a1091ef0017c40a) breaks >> booting on r8a7740/armadillo-legacy. >> >> It hangs because the timers cannot get their interrupts: >> >> console [tty0] enabled >> sh-tmu.0: ch0: used for clock events >> sh-tmu.0: ch0: used for periodic clock events >> sh-tmu.0: ch0: failed to request irq 230 >> sh-tmu.0: ch1: used as clock source >> sh-cmt-48.1: ch0: failed to request irq 90 >> sh-cmt-48.1: ch0: registration failed >> earlytimer: unable to probe sh-cmt-48 early. >> Calibrating delay loop... > > Grmbl... Are you, by any (lack of) chance, using a setup where the GIC > is probed via DT, but the timers have their IRQs hardcoded? Yes, the GIC is declared in arch/arm/boot/dts/r8a7740.dtsi: gic: interrupt-controller@c2800000 { compatible = "arm,cortex-a9-gic"; #interrupt-cells = <3>; interrupt-controller; reg = <0xc2800000 0x1000>, <0xc2000000 0x1000>; }; The timers come from legacy C board code in arch/arm/mach-shmobile/setup-r8a7740.c: /* CMT */ static struct sh_timer_config cmt1_platform_data = { .channels_mask = 0x3f, }; static struct resource cmt1_resources[] = { DEFINE_RES_MEM(0xe6138000, 0x170), DEFINE_RES_IRQ(gic_spi(58)), }; static struct platform_device cmt1_device = { .name = "sh-cmt-48", .id = 1, .dev = { .platform_data = &cmt1_platform_data, }, .resource = cmt1_resources, .num_resources = ARRAY_SIZE(cmt1_resources), }; /* TMU */ static struct sh_timer_config tmu0_platform_data = { .channels_mask = 7, }; static struct resource tmu0_resources[] = { DEFINE_RES_MEM(0xfff80000, 0x2c), DEFINE_RES_IRQ(gic_spi(198)), DEFINE_RES_IRQ(gic_spi(199)), DEFINE_RES_IRQ(gic_spi(200)), }; static struct platform_device tmu0_device = { .name = "sh-tmu", .id = 0, .dev = { .platform_data = &tmu0_platform_data, }, .resource = tmu0_resources, .num_resources = ARRAY_SIZE(tmu0_resources), }; The corresponding r8a7740/armadillo-multiplatform works fine. Still, I'm wondering why sh73a0/kzm9g-legacy is not impacted by this change... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 02/12/14 13:55, Geert Uytterhoeven wrote: > Hi Marc, > > On Tue, Dec 2, 2014 at 2:37 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> On 02/12/14 13:27, Geert Uytterhoeven wrote: >>> On Tue, Nov 25, 2014 at 9:04 AM, 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> >>> >>> This change (which is now in -next as commit 9a1091ef0017c40a) breaks >>> booting on r8a7740/armadillo-legacy. >>> >>> It hangs because the timers cannot get their interrupts: >>> >>> console [tty0] enabled >>> sh-tmu.0: ch0: used for clock events >>> sh-tmu.0: ch0: used for periodic clock events >>> sh-tmu.0: ch0: failed to request irq 230 >>> sh-tmu.0: ch1: used as clock source >>> sh-cmt-48.1: ch0: failed to request irq 90 >>> sh-cmt-48.1: ch0: registration failed >>> earlytimer: unable to probe sh-cmt-48 early. >>> Calibrating delay loop... >> >> Grmbl... Are you, by any (lack of) chance, using a setup where the GIC >> is probed via DT, but the timers have their IRQs hardcoded? > > Yes, the GIC is declared in arch/arm/boot/dts/r8a7740.dtsi: > > gic: interrupt-controller@c2800000 { > compatible = "arm,cortex-a9-gic"; > #interrupt-cells = <3>; > interrupt-controller; > reg = <0xc2800000 0x1000>, > <0xc2000000 0x1000>; > }; > > The timers come from legacy C board code in > arch/arm/mach-shmobile/setup-r8a7740.c: > > /* CMT */ > static struct sh_timer_config cmt1_platform_data = { > .channels_mask = 0x3f, > }; > > static struct resource cmt1_resources[] = { > DEFINE_RES_MEM(0xe6138000, 0x170), > DEFINE_RES_IRQ(gic_spi(58)), > }; Yup. This gic_spi() macro is completely doomed, as it computes a hardware interrupt where the kernel expects a virtual one. The only way to fix this is to init the GIC outside of DT, so that we stick to a legacy irqdomain. You can still use information from DT, but you can't rely on irqchip_init() to produce something that a legacy platform can use. > The corresponding r8a7740/armadillo-multiplatform works fine. OK, that's indeed the expected result. > Still, I'm wondering why sh73a0/kzm9g-legacy is not impacted by this > change... I'm a bit lost looking at the shmobile code. But I agree, kzm9g should die a painful death, as it has the same level of breakage as armadillo. 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..ab6069b 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,31 @@ 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 +1016,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 | 77 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 54 insertions(+), 24 deletions(-)