Skip to contentSkip to conversation

irqchip/gic: assign irqchip dynamically

PATCH Superseded Linus Walleij 2 participants 3 messages Oct 23, 2015 – Nov 2, 2015

Subsystems (2)
irqchip drivers Arm generic interrupt controller drivers

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Linus Walleij

author
#0 lore raw

PATCH v2 5/13 irqchip/gic: assign irqchip dynamically

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 <[email protected]>
Cc Jason Cooper <[email protected]>
Cc Marc Zyngier <[email protected]>
Signed-off-by Linus Walleij <[email protected]>

---
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.

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 | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
drivers/irqchip/irq-gic.c +12 -4
index bd021e1e4847..8c93ff80ec52 100644
@@ -58,6 +58,8 @@ union gic_base {
 };
 
 struct gic_chip_data {
+	struct irq_chip chip;
+	struct irq_chip eoimode1_chip;
 	union gic_base dist_base;
 	union gic_base cpu_base;
 #ifdef CONFIG_CPU_PM

@@ -370,7 +372,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 +387,6 @@ static struct irq_chip gic_chip = {
 };
 
 static struct irq_chip gic_eoimode1_chip = {
-	.name			= "GICv2",
 	.irq_mask		= gic_eoimode1_mask_irq,
 	.irq_unmask		= gic_unmask_irq,
 	.irq_eoi		= gic_eoimode1_eoi_irq,

@@ -880,11 +880,12 @@ 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;
+	struct gic_chip_data *gic = d->host_data;
+	struct irq_chip *chip = &gic->chip;
 
 	if (static_key_true(&supports_deactivate)) {
 		if (d->host_data == (void *)&gic_data[0])
-			chip = &gic_eoimode1_chip;
+			chip = &gic->eoimode1_chip;
 	}
 
 	if (hw < 32) {

@@ -989,6 +990,13 @@ 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];
+
+	/* Initialize irq_chip */
+	gic->chip = gic_chip;
+	gic->eoimode1_chip = gic_eoimode1_chip;
+	gic->chip.name = kasprintf(GFP_KERNEL, "GIC%d", gic_nr);
+	gic->eoimode1_chip.name = kasprintf(GFP_KERNEL, "GICv2%d", gic_nr);
+
 #ifdef CONFIG_GIC_NON_BANKED
 	if (percpu_offset) { /* Frankein-GIC without banked registers... */
 		unsigned int cpu;
Signature · 1 line
2.4.3
#1 lore raw

PATCH v2 5/13 irqchip/gic: assign irqchip dynamically

Hi Linus,

Sorry for the delay, I just enjoyed two weeks off, and I'm slowly making my way through my exploding Inbox...

Quoted 17 lines · Linus Walleij

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 <[email protected]>
Cc Jason Cooper <[email protected]>
Cc Marc Zyngier <[email protected]>
Signed-off-by Linus Walleij <[email protected]>

---
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.

Quoted 3 lines

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?

I'm afraid you misinterpreted the EOIMODE==1 thing. The GIC can either be EOIMODE==0 (which is what most people are using on 32bit when not booting at HYP), or EOIMODE==1 (most 64bit systems). For systems with multiple GICs, the secondary GICs are always EOIMODE==0.

So you should only have a single irqchip structure in gic_chip_data, and populate it depending on the case you're in:

- gic_nr == 0 && static_key_true(&supports_deactivate) -> EOIMODE==1
- otherwise EOIMODE==0.

Does this make more sense?

Thanks,

	M.
Signature · 1 line
Jazz is not dead. It just smells funny...
author
#2 lore raw

PATCH v2 5/13 irqchip/gic: assign irqchip dynamically

Quoted 10 lines · Marc Zyngier
Quoted 3 lines · Linus Walleij

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?

I'm afraid you misinterpreted the EOIMODE==1 thing. The GIC can either be EOIMODE==0 (which is what most people are using on 32bit when not booting at HYP), or EOIMODE==1 (most 64bit systems). For systems with multiple GICs, the secondary GICs are always EOIMODE==0.

So it's an either-or thing not both-and. OK that's the missing piece, thanks Marc, I'll fix up the patch.

Yours,
Linus Walleij