Re: [GIT PULL] irqchip: dove: drivers for v3.14

[Thomas Gleixner <tglx@linutronix.de>]

On Mon, 17 Feb 2014, Jason Cooper wrote:
> On Fri, Feb 07, 2014 at 01:08:36PM -0500, Jason Cooper wrote:
> > On Tue, Feb 04, 2014 at 10:30:53PM +0100, Thomas Gleixner wrote:
> > > On Tue, 4 Feb 2014, Jason Cooper wrote:
> > > 
> > > > On Tue, Feb 04, 2014 at 07:59:58PM +0100, Thomas Gleixner wrote:
> > > > > On Tue, 28 Jan 2014, Jason Cooper wrote:
> > > > > > I see you pulled in mvebu/irqchip-fixes.  Thanks for that.  It's getting
> > > > > > near to the end of the merge window and there's been no activity on this
> > > > > > pull request.
> > > > > > 
> > > > > > Please let us know if there's anything we can do to assist.
> > > > > 
> > > > > Nah. I simply forgot about it. About to send a pull request to Linus.
> > > > 
> > > > hmmm.  I see the pull request contains the patches from
> > > > mvebu/irqchip-fixes (armada), but not the patches from mvebu/irqchip
> > > > (dove):
> > > > 
> > > >   40b367d95fb3 irqchip: irq-dove: Add PMU interrupt controller.
> > > > 
> > > > which is what this thread was originally a pull request for.
> > > > 
> > > > 
> > > > Are you planning to send a second pull request to Linus?
> > > 
> > > Duh. I'll pick that up tomorrow
> > 
> > Ping?
> 
> I think it's safe to say that the Dove PMU interrupt controller isn't
> going to make it in to v3.14.  I've posted a patch to remove the DT node
> for v3.14.

Yes, sorry. I messed that one up.
 
> If you don't mind, I'll go ahead and take this pull request through the
> mvebu tree for v3.15.  So that way it's off your plate and you don't
> have to worry about it.

I pushed it out to tip/irq/core now.
 
> There's more mvebu irqchip stuff on the way :)

You have an entry in my mail filter rules now, so you'll end up in the
priority queue with your pull requests :)

Btw, I just looked at that dove driver and if you look at the other
drivers/irqchip variants which use the generic irq domain/chip stuff,
then the code in the init functions +/- some minimalistic fixes is
just the same boiler plate except for quirks or a few register writes.

This the diff of the relevant code for orion and dove.

--- drivers/irqchip/irq-orion.c	2014-02-18 21:27:35.655468837 +0100
+++ drivers/irqchip/irq-dove.c	2014-02-18 21:27:57.099469874 +0100
@@ -1,14 +1,10 @@
+#define DOVE_PMU_IRQ_CAUSE	0x00
+#define DOVE_PMU_IRQ_MASK	0x04
-/*
- * Orion SoC bridge interrupt controller
- */
-#define ORION_BRIDGE_IRQ_CAUSE	0x00
-#define ORION_BRIDGE_IRQ_MASK	0x04
-
-static void orion_bridge_irq_handler(unsigned int irq, struct irq_desc *desc)
+static void dove_pmu_irq_handler(unsigned int irq, struct irq_desc *desc)
 {
 	struct irq_domain *d = irq_get_handler_data(irq);
-	struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, irq);
-	u32 stat = readl_relaxed(gc->reg_base + ORION_BRIDGE_IRQ_CAUSE) &
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0);
+	u32 stat = readl_relaxed(gc->reg_base + DOVE_PMU_IRQ_CAUSE) &
 		   gc->mask_cache;

Identical except for the register constants, which are identical as well.
 
 	while (stat) {
@@ -123,17 +36,14 @@
 	}
 }
 
-static int __init orion_bridge_irq_init(struct device_node *np,
-					struct device_node *parent)
+static int __init dove_pmu_irq_init(struct device_node *np,
+				    struct device_node *parent)
 {
 	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
 	struct resource r;
 	struct irq_domain *domain;
 	struct irq_chip_generic *gc;
-	int ret, irq, nrirqs = 32;
-
-	/* get optional number of interrupts provided */
-	of_property_read_u32(np, "marvell,#interrupts", &nrirqs);
+	int ret, irq, nrirqs = 7;

So the difference here is nrirqs
 
 	domain = irq_domain_add_linear(np, nrirqs,
 				       &irq_generic_chip_ops, NULL);
@@ -174,19 +107,20 @@
 		return -ENOMEM;
 	}
 
-	gc->chip_types[0].regs.ack = ORION_BRIDGE_IRQ_CAUSE;
-	gc->chip_types[0].regs.mask = ORION_BRIDGE_IRQ_MASK;
-	gc->chip_types[0].chip.irq_ack = irq_gc_ack_clr_bit;
+	gc->chip_types[0].regs.ack = DOVE_PMU_IRQ_CAUSE;
+	gc->chip_types[0].regs.mask = DOVE_PMU_IRQ_MASK;
+	gc->chip_types[0].chip.irq_ack = pmu_irq_ack;
 	gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
 	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit;
 


-	/* mask all interrupts */
-	writel(0, gc->reg_base + ORION_BRIDGE_IRQ_MASK);
+	/* mask and clear all interrupts */
+	writel(0, gc->reg_base + DOVE_PMU_IRQ_MASK);
+	writel(0, gc->reg_base + DOVE_PMU_IRQ_CAUSE);
 
 	irq_set_handler_data(irq, domain);
-	irq_set_chained_handler(irq, orion_bridge_irq_handler);
+	irq_set_chained_handler(irq, dove_pmu_irq_handler);
 
 	return 0;
 }
-IRQCHIP_DECLARE(orion_bridge_intc,
-		"marvell,orion-bridge-intc", orion_bridge_irq_init);
+IRQCHIP_DECLARE(dove_pmu_intc,
+		"marvell,dove-pmu-intc", dove_pmu_irq_init);

I omitted the second variant in irq-orion.c which is the same
boilerplate with a few minimal changes.

It'd be really nice if someone would sit down and make one or two
general irqchip init function which gets rid of this copied code and
reduces the init functions to the bare minimum. That's not only true
for the orion/dove case it's the same for lot of irq-*.c variants plus
minus the individual extras and of course bugs.

Thanks,

	tglx