Messages in this thread | | | Date | Tue, 03 Nov 2015 17:01:43 +0000 | From | Marc Zyngier <> | Subject | Re: [PATCH 05/13 v3] irqchip/gic: assign irqchip dynamically |
| |
On 02/11/15 14:34, Linus Walleij wrote: > Instead of having the irqchip being a static struct, make it part > of the per-instance data so we can assign it a dynamic name. This > has the usable side effect of displaying the GIC with an instance > number as GIC0, GIC1 ... GICn in /proc/interrupts, which is helpful > when debugging cascaded GICs, such as on the ARM PB11MPCore. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Jason Cooper <jason@lakedaemon.net> > Cc: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v2->v3: > - Handle the EOIMODE1 chips properly: select a proper &chip pointer > at chip registration and store in the per-GIC .chip member. > Saves hard-to-grasp code in the .map function too. We select the > EOIMODE1 chip if and only if it's GIC0 and the machine suppors > deactivation. > - Name ordinary GIC chips GIC0, ... GICN and the EOIMODE1 chip > GICEOI, as there can be only one such chip. > ChangeLog v1->v2: > - Keep the static structs around, just delete the .name > field assign them to the chips at registration time, updating > the name field with the instance number. > - Also enumerate the EOIMODE1 sub-chips. > - Broke out this irqchip stuff from the rest of the series so as > not to stress the irqchip maintainers. It has no dependencies > on the other patches anyways, and can be merged stand-alone. > > Marc: can't test the EOIMODE1 thing, it's far above me, but it > "should work". Is it correct that there is one unique and coupled > EOIMODE1 instance per GIC instance like this? > --- > drivers/irqchip/irq-gic.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > index 9ec8cf5137d9..9c61ca182433 100644 > --- a/drivers/irqchip/irq-gic.c > +++ b/drivers/irqchip/irq-gic.c > @@ -58,6 +58,7 @@ union gic_base { > }; > > struct gic_chip_data { > + struct irq_chip chip; > union gic_base dist_base; > union gic_base cpu_base; > #ifdef CONFIG_CPU_PM > @@ -370,7 +371,6 @@ static void gic_handle_cascade_irq(struct irq_desc *desc) > } > > static struct irq_chip gic_chip = { > - .name = "GIC", > .irq_mask = gic_mask_irq, > .irq_unmask = gic_unmask_irq, > .irq_eoi = gic_eoi_irq, > @@ -386,7 +386,7 @@ static struct irq_chip gic_chip = { > }; > > static struct irq_chip gic_eoimode1_chip = { > - .name = "GICv2", > + .name = "GICEOI",
Hmmm. I'd rather leave this to GICv2, because EOImode==1 is a feature of the architecture, rather than a piece of HW. Only nitpicking, really.
> .irq_mask = gic_eoimode1_mask_irq, > .irq_unmask = gic_unmask_irq, > .irq_eoi = gic_eoimode1_eoi_irq, > @@ -880,12 +880,8 @@ void __init gic_init_physaddr(struct device_node *node) > static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq, > irq_hw_number_t hw) > { > - struct irq_chip *chip = &gic_chip; > - > - if (static_key_true(&supports_deactivate)) { > - if (d->host_data == (void *)&gic_data[0]) > - chip = &gic_eoimode1_chip; > - } > + struct gic_chip_data *gic = d->host_data; > + struct irq_chip *chip = &gic->chip; > > if (hw < 32) { > irq_set_percpu_devid(irq); > @@ -989,6 +985,9 @@ static void __init __gic_init_bases(unsigned int gic_nr, int irq_start, > BUG_ON(gic_nr >= MAX_GIC_NR); > > gic = &gic_data[gic_nr]; > + gic->chip = gic_chip; > + gic->chip.name = kasprintf(GFP_KERNEL, "GIC%d", gic_nr); > + > #ifdef CONFIG_GIC_NON_BANKED > if (percpu_offset) { /* Frankein-GIC without banked registers... */ > unsigned int cpu; > @@ -1079,8 +1078,10 @@ static void __init __gic_init_bases(unsigned int gic_nr, int irq_start, > register_cpu_notifier(&gic_cpu_notifier); > #endif > set_handle_irq(gic_handle_irq); > - if (static_key_true(&supports_deactivate)) > + if (static_key_true(&supports_deactivate)) { > pr_info("GIC: Using split EOI/Deactivate mode\n"); > + gic->chip = gic_eoimode1_chip; > + } > } > > gic_dist_init(gic); >
Other than the above:
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
M. -- Jazz is not dead. It just smells funny...
| |