[PATCH 0/3] Try out the irqdomain + generic chip integration series

[PATCH 0/3] Try out the irqdomain + generic chip integration series

[Grant Likely]

Hi Thomas,

Rather than just straight reviewing the generic irq chip patch series, I
tried it out by converting another driver to use it. As far as I've been
able to tell it does the right thing and you can have my ack. The first
patch in this series adds another feature needed by the versatile
driver. Patch 2 should be okay, but I'd like to get an ack from Ben
before it gets applied.

I need to get someone to test patch 3 on hardware before you pick up
that one.

Oh, and I'll reply directly to patch 7 of your series with an ack and a
minor comment.

Cheers,
g.

[PATCH 1/3] irqchip: Add mask to block out invalid irqs

[Grant Likely]

Some controllers have irqs that aren't wired up and must never be used.
For the generic chip attached to an irq_domain this provides a mask that
can be used to block out particular irqs so that they never get mapped.

Signed-off-by: Grant Likely <grant.likely@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irq.h       | 2 ++
 kernel/irq/generic-chip.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index af7052c..298a9b9 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -679,6 +679,7 @@ struct irq_chip_type {
  * @num_ct:		Number of available irq_chip_type instances (usually 1)
  * @private:		Private data for non generic chip callbacks
  * @installed:		bitfield to denote installed interrupts
+ * @unused:		bitfield to denote unused interrupts
  * @domain:		irq domain pointer
  * @list:		List head for keeping track of instances
  * @chip_types:		Array of interrupt irq_chip_types
@@ -702,6 +703,7 @@ struct irq_chip_generic {
 	unsigned int		num_ct;
 	void			*private;
 	unsigned long		installed;
+	unsigned long		unused;
 	struct irq_domain	*domain;
 	struct list_head	list;
 	struct irq_chip_type	chip_types[0];
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 8e4d6bd..7b44aa2 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -359,6 +359,9 @@ static int irq_map_generic_chip(struct irq_domain *d, unsigned int virq,
 
 	idx = hw_irq % dgc->irqs_per_chip;
 
+	if (test_bit(idx, &gc->unused))
+		return -ENOTSUPP;
+
 	if (test_bit(idx, &gc->installed))
 		return -EBUSY;
 
-- 
1.8.1.2

[tip:irq/core] genirq: irqchip: Add mask to block out invalid irqs

[tip-bot for Grant Likely]

Commit-ID:  e8bd834f73714378ef110a64287db1b77033c8da
Gitweb:     http://git.kernel.org/tip/e8bd834f73714378ef110a64287db1b77033c8da
Author:     Grant Likely <grant.likely@linaro.org>
AuthorDate: Wed, 29 May 2013 03:10:52 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 29 May 2013 10:57:11 +0200

genirq: irqchip: Add mask to block out invalid irqs

Some controllers have irqs that aren't wired up and must never be used.
For the generic chip attached to an irq_domain this provides a mask that
can be used to block out particular irqs so that they never get mapped.

Signed-off-by: Grant Likely <grant.likely@linaro.org>
Link: http://lkml.kernel.org/r/1369793454-19197-2-git-send-email-grant.likely@linaro.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irq.h       | 2 ++
 kernel/irq/generic-chip.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index af7052c..298a9b9 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -679,6 +679,7 @@ struct irq_chip_type {
  * @num_ct:		Number of available irq_chip_type instances (usually 1)
  * @private:		Private data for non generic chip callbacks
  * @installed:		bitfield to denote installed interrupts
+ * @unused:		bitfield to denote unused interrupts
  * @domain:		irq domain pointer
  * @list:		List head for keeping track of instances
  * @chip_types:		Array of interrupt irq_chip_types
@@ -702,6 +703,7 @@ struct irq_chip_generic {
 	unsigned int		num_ct;
 	void			*private;
 	unsigned long		installed;
+	unsigned long		unused;
 	struct irq_domain	*domain;
 	struct list_head	list;
 	struct irq_chip_type	chip_types[0];
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 8743d62..95575d8 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -359,6 +359,9 @@ static int irq_map_generic_chip(struct irq_domain *d, unsigned int virq,
 
 	idx = hw_irq % dgc->irqs_per_chip;
 
+	if (test_bit(idx, &gc->unused))
+		return -ENOTSUPP;
+
 	if (test_bit(idx, &gc->installed))
 		return -EBUSY;
 

[PATCH 2/3] irqdomain: Relax failure path on setting up mappings

[Grant Likely]

Commit 98aa468e, "irqdomain: Support for static IRQ mapping and
association" introduced an API for directly associating blocks of hwirqs
to linux irqs. However, if any irq in that block failed to map (say if
the mapping functions returns an error because the irq is already
mapped) then the whole thing will fail and roll back. This is probably
too aggressive since there are valid reasons why a mapping may fail.
ie. Firmware may have a particular IRQ marked as unusable.

This patch drops the error path out of irq_domain_associate(). If a
mapping fails, then it is simply skipped. There is no reason to fail the
entire allocation.

Signed-off-by: Grant Likely <grant.likely@linaro.org>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/irq/irqdomain.c | 29 +++--------------------------
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 1db9e70..428ae1db 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -455,28 +455,9 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
 		irq_data->domain = domain;
 		if (domain->ops->map) {
 			ret = domain->ops->map(domain, virq, hwirq);
-			if (ret != 0) {
-				/*
-				 * If map() returns -EPERM, this interrupt is protected
-				 * by the firmware or some other service and shall not
-				 * be mapped.
-				 *
-				 * Since on some platforms we blindly try to map everything
-				 * we end up with a log full of backtraces.
-				 *
-				 * So instead, we silently fail on -EPERM, it is the
-				 * responsibility of the PIC driver to display a relevant
-				 * message if needed.
-				 */
-				if (ret != -EPERM) {
-					pr_err("irq-%i==>hwirq-0x%lx mapping failed: %d\n",
-					       virq, hwirq, ret);
-					WARN_ON(1);
-				}
-				irq_data->domain = NULL;
-				irq_data->hwirq = 0;
-				goto err_unmap;
-			}
+			/* Some maps are refused; this is probably just fine */
+			if (ret)
+				continue;
 		}
 
 		switch (domain->revmap_type) {
@@ -495,10 +476,6 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
 	}
 
 	return 0;
-
- err_unmap:
-	irq_domain_disassociate_many(domain, irq_base, i);
-	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(irq_domain_associate_many);
 
-- 
1.8.1.2

[PATCH 3/3] irqchip: Make versatile fpga irq driver a generic chip

[Grant Likely]

This is an RFC patch to convert the versatile FPGA irq controller driver
to use generic irq chip. It builds on the series that extends the
generic chip code to allow a linear irq domain to contain one or more
generic irq chips so that each interrupt controller doesn't need to hand
code the generic chip setup.

I've written this as a proof of concept to see if the new generic irq
code does what it needs to. I had to extend it slightly to properly
handle the valid mask used by the versatile FPGA driver.

Tested on QEMU, but not on real hardware.

Signed-off-by: Grant Likely <grant.likely@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/Kconfig              |   1 +
 drivers/irqchip/irq-versatile-fpga.c | 104 +++++++++++++----------------------
 2 files changed, 39 insertions(+), 66 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 4a33351..8765502 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -14,6 +14,7 @@ config ARM_VIC
 	bool
 	select IRQ_DOMAIN
 	select MULTI_IRQ_HANDLER
+	select GENERIC_IRQ_CHIP
 
 config ARM_VIC_NR
 	int
diff --git a/drivers/irqchip/irq-versatile-fpga.c b/drivers/irqchip/irq-versatile-fpga.c
index 065b7a3..8c7097b 100644
--- a/drivers/irqchip/irq-versatile-fpga.c
+++ b/drivers/irqchip/irq-versatile-fpga.c
@@ -34,37 +34,18 @@
  * @used_irqs: number of active IRQs on this controller
  */
 struct fpga_irq_data {
-	void __iomem *base;
-	struct irq_chip chip;
-	u32 valid;
 	struct irq_domain *domain;
-	u8 used_irqs;
 };
 
 /* we cannot allocate memory when the controllers are initially registered */
 static struct fpga_irq_data fpga_irq_devices[CONFIG_VERSATILE_FPGA_IRQ_NR];
 static int fpga_irq_id;
 
-static void fpga_irq_mask(struct irq_data *d)
-{
-	struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
-	u32 mask = 1 << d->hwirq;
-
-	writel(mask, f->base + IRQ_ENABLE_CLEAR);
-}
-
-static void fpga_irq_unmask(struct irq_data *d)
-{
-	struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
-	u32 mask = 1 << d->hwirq;
-
-	writel(mask, f->base + IRQ_ENABLE_SET);
-}
-
 static void fpga_irq_handle(unsigned int irq, struct irq_desc *desc)
 {
-	struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
-	u32 status = readl(f->base + IRQ_STATUS);
+	struct irq_domain *domain = irq_desc_get_handler_data(desc);
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(domain, 0);
+	u32 status = readl(gc->reg_base + IRQ_STATUS);
 
 	if (status == 0) {
 		do_bad_IRQ(irq, desc);
@@ -74,7 +55,7 @@ static void fpga_irq_handle(unsigned int irq, struct irq_desc *desc)
 	do {
 		irq = ffs(status) - 1;
 		status &= ~(1 << irq);
-		generic_handle_irq(irq_find_mapping(f->domain, irq));
+		generic_handle_irq(irq_find_mapping(domain, irq));
 	} while (status);
 }
 
@@ -85,11 +66,12 @@ static void fpga_irq_handle(unsigned int irq, struct irq_desc *desc)
  */
 static int handle_one_fpga(struct fpga_irq_data *f, struct pt_regs *regs)
 {
+	struct irq_chip_generic *gc = irq_get_domain_generic_chip(f->domain, 0);
 	int handled = 0;
 	int irq;
 	u32 status;
 
-	while ((status  = readl(f->base + IRQ_STATUS))) {
+	while ((status = readl(gc->reg_base + IRQ_STATUS))) {
 		irq = ffs(status) - 1;
 		handle_IRQ(irq_find_mapping(f->domain, irq), regs);
 		handled = 1;
@@ -112,63 +94,53 @@ asmlinkage void __exception_irq_entry fpga_handle_irq(struct pt_regs *regs)
 	} while (handled);
 }
 
-static int fpga_irqdomain_map(struct irq_domain *d, unsigned int irq,
-		irq_hw_number_t hwirq)
-{
-	struct fpga_irq_data *f = d->host_data;
-
-	/* Skip invalid IRQs, only register handlers for the real ones */
-	if (!(f->valid & BIT(hwirq)))
-		return -ENOTSUPP;
-	irq_set_chip_data(irq, f);
-	irq_set_chip_and_handler(irq, &f->chip,
-				handle_level_irq);
-	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
-	return 0;
-}
-
-static struct irq_domain_ops fpga_irqdomain_ops = {
-	.map = fpga_irqdomain_map,
-	.xlate = irq_domain_xlate_onetwocell,
-};
-
 void __init fpga_irq_init(void __iomem *base, const char *name, int irq_start,
 			  int parent_irq, u32 valid, struct device_node *node)
 {
+	struct irq_chip_generic *gc;
 	struct fpga_irq_data *f;
-	int i;
+	int ret;
 
 	if (fpga_irq_id >= ARRAY_SIZE(fpga_irq_devices)) {
 		pr_err("%s: too few FPGA IRQ controllers, increase CONFIG_VERSATILE_FPGA_IRQ_NR\n", __func__);
 		return;
 	}
 	f = &fpga_irq_devices[fpga_irq_id];
-	f->base = base;
-	f->chip.name = name;
-	f->chip.irq_ack = fpga_irq_mask;
-	f->chip.irq_mask = fpga_irq_mask;
-	f->chip.irq_unmask = fpga_irq_unmask;
-	f->valid = valid;
+
+	/* This will also allocate irq descriptors */
+	f->domain = irq_domain_add_linear(node, fls(valid),
+					  &irq_generic_chip_ops, f);
+	if (!f->domain) {
+		pr_err("FPGA IRQ setup failed allocation domain\n");
+		return;
+	}
+
+	ret = irq_alloc_domain_generic_chips(f->domain, fls(valid), 1,
+			name, handle_level_irq,
+			IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN, 0, 0);
+	if (ret) {
+		pr_err("FPGA IRQ setup failed allocating generic chip\n");
+		return;
+	}
+
+	gc = irq_get_domain_generic_chip(f->domain, 0);
+	gc->reg_base = base;
+	gc->unused = ~valid;
+	gc->chip_types[0].regs.enable = IRQ_ENABLE_SET;
+	gc->chip_types[0].regs.disable = IRQ_ENABLE_CLEAR;
+	gc->chip_types[0].chip.irq_ack = irq_gc_mask_disable_reg;
+	gc->chip_types[0].chip.irq_mask = irq_gc_mask_disable_reg;
+	gc->chip_types[0].chip.irq_unmask = irq_gc_unmask_enable_reg;
 
 	if (parent_irq != -1) {
-		irq_set_handler_data(parent_irq, f);
+		irq_set_handler_data(parent_irq, f->domain);
 		irq_set_chained_handler(parent_irq, fpga_irq_handle);
 	}
 
-	/* This will also allocate irq descriptors */
-	f->domain = irq_domain_add_simple(node, fls(valid), irq_start,
-					  &fpga_irqdomain_ops, f);
-
-	/* This will allocate all valid descriptors in the linear case */
-	for (i = 0; i < fls(valid); i++)
-		if (valid & BIT(i)) {
-			if (!irq_start)
-				irq_create_mapping(f->domain, i);
-			f->used_irqs++;
-		}
-
-	pr_info("FPGA IRQ chip %d \"%s\" @ %p, %u irqs\n",
-		fpga_irq_id, name, base, f->used_irqs);
+	if (irq_start)
+		irq_domain_associate_many(f->domain, irq_start, 0, fls(valid));
+
+	pr_info("FPGA IRQ chip %d \"%s\" @ %p\n", fpga_irq_id, name, base);
 
 	fpga_irq_id++;
 }
-- 
1.8.1.2

Re: [PATCH 3/3] irqchip: Make versatile fpga irq driver a generic chip

[Linus Walleij]

On Wed, May 29, 2013 at 4:10 AM, Grant Likely <grant.likely@linaro.org> wrote:

> This is an RFC patch to convert the versatile FPGA irq controller driver
> to use generic irq chip. It builds on the series that extends the
> generic chip code to allow a linear irq domain to contain one or more
> generic irq chips so that each interrupt controller doesn't need to hand
> code the generic chip setup.
>
> I've written this as a proof of concept to see if the new generic irq
> code does what it needs to. I had to extend it slightly to properly
> handle the valid mask used by the versatile FPGA driver.
>
> Tested on QEMU, but not on real hardware.


Hm, I could test this but what is the needed baseline? I think
I need Thomas' patches underneath right?

Could you publish a branch that I can test?

Yours,
Linus Walleij

Re: [PATCH 3/3] irqchip: Make versatile fpga irq driver a generic chip

[Grant Likely]

On Wed, May 29, 2013 at 6:55 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, May 29, 2013 at 4:10 AM, Grant Likely <grant.likely@linaro.org> wrote:
>
>> This is an RFC patch to convert the versatile FPGA irq controller driver
>> to use generic irq chip. It builds on the series that extends the
>> generic chip code to allow a linear irq domain to contain one or more
>> generic irq chips so that each interrupt controller doesn't need to hand
>> code the generic chip setup.
>>
>> I've written this as a proof of concept to see if the new generic irq
>> code does what it needs to. I had to extend it slightly to properly
>> handle the valid mask used by the versatile FPGA driver.
>>
>> Tested on QEMU, but not on real hardware.
>
>
> Hm, I could test this but what is the needed baseline? I think
> I need Thomas' patches underneath right?
>
> Could you publish a branch that I can test?

Try this one:

git://git.secretlab.ca/git/linux irqdomain/generic-chip

g.

Re: [PATCH 3/3] irqchip: Make versatile fpga irq driver a generic chip

[Linus Walleij]

On Wed, May 29, 2013 at 9:56 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Wed, May 29, 2013 at 6:55 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> Hm, I could test this but what is the needed baseline? I think
>> I need Thomas' patches underneath right?
>>
>> Could you publish a branch that I can test?
>
> Try this one:
>
> git://git.secretlab.ca/git/linux irqdomain/generic-chip

Hey it works!

Compiled and booted on Integrator/AP which uses this
controller exclusively.

But you need this hunk:

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 8765502..0cca010 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -36,6 +36,7 @@ config RENESAS_IRQC

 config VERSATILE_FPGA_IRQ
        bool
+       select GENERIC_IRQ_CHIP
        select IRQ_DOMAIN

 config VERSATILE_FPGA_IRQ_NR

With that:
Tested-by: Linus Walleij <linus.walleij@linaro.org>

For the series.

Yours,
Linus Walleij

Re: [PATCH 0/3] Try out the irqdomain + generic chip integration series

[Thomas Gleixner]

Grant,

On Wed, 29 May 2013, Grant Likely wrote:

> Hi Thomas,
> 
> Rather than just straight reviewing the generic irq chip patch series, I
> tried it out by converting another driver to use it. As far as I've been
> able to tell it does the right thing and you can have my ack. The first
> patch in this series adds another feature needed by the versatile
> driver. Patch 2 should be okay, but I'd like to get an ack from Ben
> before it gets applied.

I pick up #1 and push it with the other lot, so people can start
working on top of that. I'll put it into a separate branch
(irq/for-arm) so SoC folks can grab it from there to avoid inter tree
dependencies.
 
> I need to get someone to test patch 3 on hardware before you pick up
> that one.

Sure. It might also go through the SoC tree(s).
 
> Oh, and I'll reply directly to patch 7 of your series with an ack and a
> minor comment.

Thanks,

	tglx