Patchwork [v4] irqchip: Add support for ARMv7-M's NVIC

login
register
mail settings
Submitter Uwe Kleine-König
Date June 12, 2013, 9:50 p.m.
Message ID <1371073835-10639-1-git-send-email-u.kleine-koenig@pengutronix.de>
Download mbox | patch
Permalink /patch/250892/
State New
Headers show

Comments

Uwe Kleine-König - June 12, 2013, 9:50 p.m.
This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.

Support for this controller appeared in Catalin's Cortex tree based on
2.6.33 but was nearly completely rewritten.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---

Notes:
    Changes since v3, sent with
    Message-Id: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>:
    
     - use generic chip
     - rename nvic_do_IRQ to nvic_handle_irq
    
    This depends on the stuff currently in tip/irq/for-arm

 arch/arm/kernel/entry-v7m.S |   2 +-
 drivers/irqchip/Kconfig     |   5 ++
 drivers/irqchip/Makefile    |   1 +
 drivers/irqchip/irq-nvic.c  | 116 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 drivers/irqchip/irq-nvic.c
Uwe Kleine-König - June 12, 2013, 10 p.m.
Hello,

On Wed, Jun 12, 2013 at 11:50:35PM +0200, Uwe Kleine-König wrote:
> This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.
> 
> Support for this controller appeared in Catalin's Cortex tree based on
> 2.6.33 but was nearly completely rewritten.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> 
> Notes:
>     Changes since v3, sent with
>     Message-Id: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>:
>     
>      - use generic chip
>      - rename nvic_do_IRQ to nvic_handle_irq
>     
>     This depends on the stuff currently in tip/irq/for-arm
It also depends on Russell King's devel-stable branch. So the options to
apply it in case you are otherwise happy with it:

 - apply on top of tip/irq/for-arm dropping the change to
   arch/arm/kernel/entry-v7m.S and let me add that change seperately; or
 - let it go through Russell's tree (the dependency on tip/irq/for-arm
   doesn't stop the patch to be applied on that one, it just won't
   compile); or
 - wait until there is a tree that contains both branches; or
 - merge the two trees and apply it on top of the merge.

Best regards
Uwe
Catalin Marinas - June 14, 2013, 9:40 a.m.
On Wed, Jun 12, 2013 at 10:50:35PM +0100, Uwe Kleine-K??nig wrote:
> This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.
> 
> Support for this controller appeared in Catalin's Cortex tree based on
> 2.6.33 but was nearly completely rewritten.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Uwe Kleine-K??nig <u.kleine-koenig@pengutronix.de>

Since you pretty much re-wrote my original code, you can drop my
Signed-off-by. Feel free to replace with:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Arnd Bergmann - June 14, 2013, 2:24 p.m.
On Wednesday 12 June 2013, Uwe Kleine-König wrote:
> This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.
> 
> Support for this controller appeared in Catalin's Cortex tree based on
> 2.6.33 but was nearly completely rewritten.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Wow, I hadn't followed the details of the generic irqchip patches, but
this looks great, almost no code left at all!

Acked-by: Arnd Bergmann <arnd@arndb.de>
Uwe Kleine-König - June 16, 2013, 9:21 a.m.
On Sat, Jun 15, 2013 at 01:41:49AM +0100, Grant Likely wrote:
> On Wed, 12 Jun 2013 23:50:35 +0200, Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> wrote:
> > This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.
> > 
> > Support for this controller appeared in Catalin's Cortex tree based on
> > 2.6.33 but was nearly completely rewritten.
> > 
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > 
> > Notes:
> >     Changes since v3, sent with
> >     Message-Id: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>:
> >     
> >      - use generic chip
> >      - rename nvic_do_IRQ to nvic_handle_irq
> >     
> >     This depends on the stuff currently in tip/irq/for-arm
> 
> Minor comments below, but it looks good to me. Nice and small.
> 
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
> 
> > +asmlinkage void __exception_irq_entry
> > +nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
> > +{
> > +	unsigned int irq = irq_linear_revmap(nvic_irq_domain, hwirq);
> > +
> > +	handle_IRQ(irq, regs);
> 
> Or simply:
> 	handle_IRQ(irq_linear_revmap(nvic_irq_domain, hwirq), regs);
I think my version is more readable, so I tend to keep it the way I
suggested.

> > +static int __init nvic_of_init(struct device_node *node,
> > +			       struct device_node *parent)
> > +{
> > +	void __iomem *nvic_base;
> > +	unsigned int irqs, i, ret;
> > +	unsigned numbanks = (readl_relaxed(V7M_SCS_ICTR) &
> > +			     V7M_SCS_ICTR_INTLINESNUM_MASK) + 1;
> > +	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
> > +
> > +	nvic_base = of_iomap(node, 0);
> > +	if (!nvic_base) {
> > +		pr_warn("unable to map nvic registers\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	irqs = numbanks * 32;
> > +	if (irqs > NVIC_MAX_IRQ)
> > +		irqs = NVIC_MAX_IRQ;
> 
> I would display some kind of message here.
That's not an error case. The documentation about INTLINESNUM reads:

	The total number of interrupt lines supported by an
	implementation, defined in groups of 32. That is, the total number of
	interrupt lines is up to (32*(INTLINESNUM+1)). However, the absolute
	maximum number of interrupts is 496, corresponding to the INTLINESNUM
	value 0b1111.

And the documentation for e.g. NVIC_ISER0 - NVIC_ISER15 reads:

	Usage constraints NVIC_ISERn[31:0] are the set-enable bits for
	interrupts (31+(32*n)) - (32*n).
	When n=15, bits [31:16] are reserved.

So it's just an implementation detail that the last bank can only
contain 16 interrupts. There is a comment describing that fact at the
definition of NVIC_MAX_IRQ.

So I think this case is important enough to clutter the kernel log.

Best regards
Uwe
Uwe Kleine-König - June 25, 2013, 10:34 a.m.
Hello Thomas, hello Grant,

On Wed, Jun 12, 2013 at 11:50:35PM +0200, Uwe Kleine-König wrote:
> This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.
> 
> Support for this controller appeared in Catalin's Cortex tree based on
> 2.6.33 but was nearly completely rewritten.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> 
> Notes:
>     Changes since v3, sent with
>     Message-Id: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>:
>     
>      - use generic chip
>      - rename nvic_do_IRQ to nvic_handle_irq
>     
>     This depends on the stuff currently in tip/irq/for-arm
I see that tip/irq/for-arm went into next via Grant's irqdomain tree. I
assume that means the changes go in for 3.11?

Are you OK with my patch? I can resend with the accumulated Acks and the
arch/arm hunk dropped if it helps you. Just tell me what you want.

Best regards
Uwe
Grant Likely - June 25, 2013, 11:04 a.m.
On Tue, Jun 25, 2013 at 11:34 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Hello Thomas, hello Grant,
>
> On Wed, Jun 12, 2013 at 11:50:35PM +0200, Uwe Kleine-König wrote:
>> This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.
>>
>> Support for this controller appeared in Catalin's Cortex tree based on
>> 2.6.33 but was nearly completely rewritten.
>>
>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>>
>> Notes:
>>     Changes since v3, sent with
>>     Message-Id: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>:
>>
>>      - use generic chip
>>      - rename nvic_do_IRQ to nvic_handle_irq
>>
>>     This depends on the stuff currently in tip/irq/for-arm
> I see that tip/irq/for-arm went into next via Grant's irqdomain tree. I
> assume that means the changes go in for 3.11?

Yes, the plan is to put that branch into v3.11. I don't know if Thomas
plans to send a separate pull request for the irq/for-arm branch, or
if he is going to let it go as part of my pull request. I'll work it
out with him when the merge window opens. It would probably be best
for your patch to be applied on the irq/for-arm branch and pushed
separately from my irqdomain branch since I don't maintain any of the
irqchip drivers.

g.
Thomas Gleixner - June 25, 2013, 12:23 p.m.
On Tue, 25 Jun 2013, Grant Likely wrote:
> On Tue, Jun 25, 2013 at 11:34 AM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > Hello Thomas, hello Grant,
> >
> > On Wed, Jun 12, 2013 at 11:50:35PM +0200, Uwe Kleine-König wrote:
> >> This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.
> >>
> >> Support for this controller appeared in Catalin's Cortex tree based on
> >> 2.6.33 but was nearly completely rewritten.
> >>
> >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> >> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >> ---
> >>
> >> Notes:
> >>     Changes since v3, sent with
> >>     Message-Id: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>:
> >>
> >>      - use generic chip
> >>      - rename nvic_do_IRQ to nvic_handle_irq
> >>
> >>     This depends on the stuff currently in tip/irq/for-arm
> > I see that tip/irq/for-arm went into next via Grant's irqdomain tree. I
> > assume that means the changes go in for 3.11?
> 
> Yes, the plan is to put that branch into v3.11. I don't know if Thomas
> plans to send a separate pull request for the irq/for-arm branch, or
> if he is going to let it go as part of my pull request. I'll work it

irq/for-arm is in irq/core as well. So whoever sends first will merge
it and the later pull request will just contain the same commits.

> out with him when the merge window opens. It would probably be best
> for your patch to be applied on the irq/for-arm branch and pushed
> separately from my irqdomain branch since I don't maintain any of the
> irqchip drivers.

I can take it via tip/irq/core.

Thanks,

	tglx
Thomas Gleixner - June 25, 2013, 9:29 p.m.
On Wed, 12 Jun 2013, Uwe Kleine-König wrote:

> This interrupt controller is found on Cortex-M3 and Cortex-M4 machines.

I don't think that anyone is searching for interrupt controllers as
they are simply an essential part of the M3/4 SoCs.
 
>     
>     This depends on the stuff currently in tip/irq/for-arm
> 
>  arch/arm/kernel/entry-v7m.S |   2 +-

This is not depending on my tree. I can take the patch if

 - you drop the arm related change and do that cleanup later

or

 - the relevant maintainers provide their ACK.

> +	for (i = 0; i < numbanks; ++i) {
> +		struct irq_chip_generic *gc =
> +			irq_get_domain_generic_chip(nvic_irq_domain, 32 * i);
> +		gc->reg_base = nvic_base + 4 * i;
> +		gc->chip_types[0].regs.enable = NVIC_ISER;
> +		gc->chip_types[0].regs.disable = NVIC_ICER;
> +		gc->chip_types[0].chip.irq_mask = irq_gc_mask_disable_reg;
> +		gc->chip_types[0].chip.irq_unmask = irq_gc_unmask_enable_reg;
> +		gc->chip_types[0].chip.irq_eoi = nvic_eoi;
> +	}
> +
> +	/* Disable all interrupts */
> +	for (i = 0; i < irqs; i += 32)
> +		writel_relaxed(~0, nvic_base + NVIC_ICER + i * 4 / 32);

So this is another, slightly different loop than the previous one

> +	for (i = 0; i < numbanks; ++i) {

This time based on irqs and increasing the irq number per loop by a
full bank. Why not doing this in the above loop which does exaclty the
same and you don't have to do the odd register base math. A simple

	writel_relaxed(~0, gc->reg_base + NVIC_ICER);

in the banks loop is sufficient, right?

Thanks,

	tglx

Patch

diff --git a/arch/arm/kernel/entry-v7m.S b/arch/arm/kernel/entry-v7m.S
index e00621f..52b2643 100644
--- a/arch/arm/kernel/entry-v7m.S
+++ b/arch/arm/kernel/entry-v7m.S
@@ -49,7 +49,7 @@  __irq_entry:
 	mov	r1, sp
 	stmdb	sp!, {lr}
 	@ routine called with r0 = irq number, r1 = struct pt_regs *
-	bl	nvic_do_IRQ
+	bl	nvic_handle_irq
 
 	pop	{lr}
 	@
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 4a33351..8317a7e 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -10,6 +10,11 @@  config ARM_GIC
 config GIC_NON_BANKED
 	bool
 
+config ARM_NVIC
+	bool
+	select IRQ_DOMAIN
+	select GENERIC_IRQ_CHIP
+
 config ARM_VIC
 	bool
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index cda4cb5..4bb277c 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -10,6 +10,7 @@  obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)	+= irq-metag.o
 obj-$(CONFIG_ARCH_SUNXI)		+= irq-sun4i.o
 obj-$(CONFIG_ARCH_SPEAR3XX)		+= spear-shirq.o
 obj-$(CONFIG_ARM_GIC)			+= irq-gic.o
+obj-$(CONFIG_ARM_NVIC)			+= irq-nvic.o
 obj-$(CONFIG_ARM_VIC)			+= irq-vic.o
 obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
new file mode 100644
index 0000000..2f1d305
--- /dev/null
+++ b/drivers/irqchip/irq-nvic.c
@@ -0,0 +1,116 @@ 
+/*
+ * drivers/irq/irq-nvic.c
+ *
+ * Copyright (C) 2008 ARM Limited, All Rights Reserved.
+ * Copyright (C) 2013 Pengutronix
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Support for the Nested Vectored Interrupt Controller found on the
+ * ARMv7-M CPUs (Cortex-M3/M4)
+ */
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+
+#include <asm/v7m.h>
+#include <asm/exception.h>
+
+#include "irqchip.h"
+
+#define NVIC_ISER			0x000
+#define NVIC_ICER			0x080
+#define NVIC_IPR			0x300
+
+#define NVIC_MAX_BANKS		16
+/*
+ * Each bank handles 32 irqs. Only the 16th (= last) bank handles only
+ * 16 irqs.
+ */
+#define NVIC_MAX_IRQ		((NVIC_MAX_BANKS - 1) * 32 + 16)
+
+static struct irq_domain *nvic_irq_domain;
+
+asmlinkage void __exception_irq_entry
+nvic_handle_irq(irq_hw_number_t hwirq, struct pt_regs *regs)
+{
+	unsigned int irq = irq_linear_revmap(nvic_irq_domain, hwirq);
+
+	handle_IRQ(irq, regs);
+}
+
+static void nvic_eoi(struct irq_data *d)
+{
+	/*
+	 * This is a no-op as end of interrupt is signaled by the exception
+	 * return sequence.
+	 */
+}
+
+static int __init nvic_of_init(struct device_node *node,
+			       struct device_node *parent)
+{
+	void __iomem *nvic_base;
+	unsigned int irqs, i, ret;
+	unsigned numbanks = (readl_relaxed(V7M_SCS_ICTR) &
+			     V7M_SCS_ICTR_INTLINESNUM_MASK) + 1;
+	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
+
+	nvic_base = of_iomap(node, 0);
+	if (!nvic_base) {
+		pr_warn("unable to map nvic registers\n");
+		return -ENOMEM;
+	}
+
+	irqs = numbanks * 32;
+	if (irqs > NVIC_MAX_IRQ)
+		irqs = NVIC_MAX_IRQ;
+
+	nvic_irq_domain =
+		irq_domain_add_linear(node, irqs, &irq_generic_chip_ops, NULL);
+	if (!nvic_irq_domain) {
+		pr_warn("Failed to allocate irq domain\n");
+		return -ENOMEM;
+	}
+
+	ret = irq_alloc_domain_generic_chips(nvic_irq_domain, 32, numbanks,
+					     "nvic_irq", handle_fasteoi_irq,
+					     clr, 0, IRQ_GC_INIT_MASK_CACHE);
+	if (ret) {
+		pr_warn("Failed to allocate irq chips\n");
+		irq_domain_remove(nvic_irq_domain);
+		return ret;
+	}
+
+	for (i = 0; i < numbanks; ++i) {
+		struct irq_chip_generic *gc =
+			irq_get_domain_generic_chip(nvic_irq_domain, 32 * i);
+		gc->reg_base = nvic_base + 4 * i;
+		gc->chip_types[0].regs.enable = NVIC_ISER;
+		gc->chip_types[0].regs.disable = NVIC_ICER;
+		gc->chip_types[0].chip.irq_mask = irq_gc_mask_disable_reg;
+		gc->chip_types[0].chip.irq_unmask = irq_gc_unmask_enable_reg;
+		gc->chip_types[0].chip.irq_eoi = nvic_eoi;
+	}
+
+	/* Disable all interrupts */
+	for (i = 0; i < irqs; i += 32)
+		writel_relaxed(~0, nvic_base + NVIC_ICER + i * 4 / 32);
+
+	/* Set priority on all interrupts */
+	for (i = 0; i < irqs; i += 4)
+		writel_relaxed(0, nvic_base + NVIC_IPR + i);
+
+	return 0;
+}
+IRQCHIP_DECLARE(armv7m_nvic, "arm,armv7m-nvic", nvic_of_init);