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

login
register
mail settings
Submitter Uwe Kleine-König
Date April 17, 2013, 4:02 p.m.
Message ID <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>
Download mbox | patch
Permalink /patch/237272/
State New
Headers show

Comments

Uwe Kleine-König - April 17, 2013, 4:02 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 v2, sent with
    Message-id:1363612826-15623-1-git-send-email-u.kleine-koenig@pengutronix.de:
    
     - drop superflous check for node != NULL in init function
     - rework for linear irq domain
     - drop seperate function for non-dt init

This patch triggers two checkpatch warnings:

	WARNING: Avoid CamelCase: <nvic_do_IRQ>
	WARNING: Avoid CamelCase: <handle_IRQ>

but I think they are OK for consistency?!
Moreover sparse tells me:

	drivers/irqchip/irq-nvic.c:58:1: warning: symbol 'nvic_do_IRQ' was not declared. Should it be static?

nvic_do_IRQ is called from assembler only, so a declaration couldn't be
shared and I couldn't find a nice place for a declaration. Suggestions
welcome.

Thanks
Uwe

 drivers/irqchip/Kconfig    |   4 ++
 drivers/irqchip/Makefile   |   1 +
 drivers/irqchip/irq-nvic.c | 154 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)
 create mode 100644 drivers/irqchip/irq-nvic.c
Arnd Bergmann - April 17, 2013, 8:23 p.m.
On Wednesday 17 April 2013, Uwe Kleine-König wrote:

> This patch triggers two checkpatch warnings:
> 
> 	WARNING: Avoid CamelCase: <nvic_do_IRQ>
> 	WARNING: Avoid CamelCase: <handle_IRQ>
> 
> but I think they are OK for consistency?!

You obviously have no choice for handle_IRQ, but I think the common way to
name the first-level interrupt handler would be "nvic_handle_irq" here.

> Moreover sparse tells me:
> 
> 	drivers/irqchip/irq-nvic.c:58:1: warning: symbol 'nvic_do_IRQ' was not declared. Should it be static?
> 
> nvic_do_IRQ is called from assembler only, so a declaration couldn't be
> shared and I couldn't find a nice place for a declaration. Suggestions
> welcome.

Can't you make it static and call set_handle_irq() on it from the
probe function?

> + * Each bank handles 32 irqs. Only the 16th (= last) bank handles only
> + * 16 irqs.
> + */
> +#define NVIC_MAX_IRQ		((NVIC_MAX_BANKS - 1) * 32 + 16)

Is this actually inherent to the hardware design, or is the number of irqs
actually customizable? Also, why do you care about the maximum? You only
use it to check against the device tree provided value, but I suppose
you could just as well trust that property to be correct.


	Arnd
Uwe Kleine-König - April 18, 2013, 8:15 a.m.
On Wed, Apr 17, 2013 at 10:23:43PM +0200, Arnd Bergmann wrote:
> On Wednesday 17 April 2013, Uwe Kleine-König wrote:
> 
> > This patch triggers two checkpatch warnings:
> > 
> > 	WARNING: Avoid CamelCase: <nvic_do_IRQ>
> > 	WARNING: Avoid CamelCase: <handle_IRQ>
> > 
> > but I think they are OK for consistency?!
> 
> You obviously have no choice for handle_IRQ, but I think the common way to
> name the first-level interrupt handler would be "nvic_handle_irq" here.
The function I called before is asm_do_IRQ which is another instance of
this naming scheme. But I agree that nvic_handle_irq is nicer and will
change to nvic_handle_irq in the next iteration.
 
> > Moreover sparse tells me:
> > 
> > 	drivers/irqchip/irq-nvic.c:58:1: warning: symbol 'nvic_do_IRQ' was not declared. Should it be static?
> > 
> > nvic_do_IRQ is called from assembler only, so a declaration couldn't be
> > shared and I couldn't find a nice place for a declaration. Suggestions
> > welcome.
> 
> Can't you make it static and call set_handle_irq() on it from the
> probe function?
Yeah that works. Then nvic_handle_irq needs to determine the irq itself
which is currently done in the entry code.

> > + * Each bank handles 32 irqs. Only the 16th (= last) bank handles only
> > + * 16 irqs.
> > + */
> > +#define NVIC_MAX_IRQ		((NVIC_MAX_BANKS - 1) * 32 + 16)
> 
> Is this actually inherent to the hardware design, or is the number of irqs
> actually customizable? Also, why do you care about the maximum? You only
> use it to check against the device tree provided value, but I suppose
> you could just as well trust that property to be correct.
I don't provide a value for the number of irqs in the device tree. There
is only the value INTLINESNUM in the V7M_SCS_ICTR register that is used
to determine the number of interrupt banks.

Best regards
Uwe
Arnd Bergmann - April 18, 2013, 9:01 a.m.
On Thursday 18 April 2013, Uwe Kleine-König wrote:
> > > + * Each bank handles 32 irqs. Only the 16th (= last) bank handles only
> > > + * 16 irqs.
> > > + */
> > > +#define NVIC_MAX_IRQ               ((NVIC_MAX_BANKS - 1) * 32 + 16)
> > 
> > Is this actually inherent to the hardware design, or is the number of irqs
> > actually customizable? Also, why do you care about the maximum? You only
> > use it to check against the device tree provided value, but I suppose
> > you could just as well trust that property to be correct.
> I don't provide a value for the number of irqs in the device tree. There
> is only the value INTLINESNUM in the V7M_SCS_ICTR register that is used
> to determine the number of interrupt banks.

Ah, right. But do you have any reason to believe it could be wrong?

	Arnd
Uwe Kleine-König - April 18, 2013, 9:24 a.m.
On Thu, Apr 18, 2013 at 11:01:13AM +0200, Arnd Bergmann wrote:
> On Thursday 18 April 2013, Uwe Kleine-König wrote:
> > > > + * Each bank handles 32 irqs. Only the 16th (= last) bank handles only
> > > > + * 16 irqs.
> > > > + */
> > > > +#define NVIC_MAX_IRQ               ((NVIC_MAX_BANKS - 1) * 32 + 16)
> > > 
> > > Is this actually inherent to the hardware design, or is the number of irqs
> > > actually customizable? Also, why do you care about the maximum? You only
> > > use it to check against the device tree provided value, but I suppose
> > > you could just as well trust that property to be correct.
> > I don't provide a value for the number of irqs in the device tree. There
> > is only the value INTLINESNUM in the V7M_SCS_ICTR register that is used
> > to determine the number of interrupt banks.
> 
> Ah, right. But do you have any reason to believe it could be wrong?
No it's just that the mapping isn't linear in the end.

	INTLINESNUM | number of irqs
	      0     |     32
	      1     |     64
	      2     |     96
	      3     |    128
	      4     |    160
	      5     |    192
	      6     |    224
	      7     |    256
	      8     |    288
	      9     |    320
	     10     |    352
	     11     |    384
	     12     |    416
	     13     |    448
	     14     |    480
	     15     |    496

That is, there are (INTLINESNUM + 1) * 32 irqs for INTLINESNUM < 15. For
INTLINESNUM == 15 there are only 496 and not 16 * 32 == 512. That's the
same on the gic (just with bigger numbers).

Best regards
Uwe
Thomas Gleixner - April 18, 2013, 9:35 a.m.
On Wed, 17 Apr 2013, Uwe Kleine-König wrote:
> +struct nvic_bank_data {
> +	/*
> +	 * For irq i base holds nvic_base + 4 * i / 32. So you can access the
> +	 * right ISER register (i.e ISER[i / 32]) by just taking base + ISER.
> +	 * Ditto for ICER.
> +	 */
> +	void __iomem *base;
> +};

What's the point of a struct with a single member? Why not having an
array of base pointers ?

> +static struct nvic_chip_data {
> +	struct irq_domain *domain;
> +	struct nvic_bank_data bdata[NVIC_MAX_BANKS];
> +} nvic_chip_data;
> +
> +asmlinkage void __exception_irq_entry
> +nvic_do_IRQ(irq_hw_number_t hwirq, struct pt_regs *regs)
> +{
> +	unsigned int irq = irq_linear_revmap(nvic_chip_data.domain, hwirq);
> +
> +	handle_IRQ(irq, regs);
> +}
> +
> +static inline void __iomem *nvic_bank_base(struct irq_data *d)
> +{
> +	struct nvic_bank_data *bank_data = irq_data_get_irq_chip_data(d);
> +	return bank_data->base;
> +}
> +
> +static void nvic_mask_irq(struct irq_data *d)
> +{
> +	u32 mask = 1 << (d->hwirq % 32);
> +
> +	writel_relaxed(mask, nvic_bank_base(d) + NVIC_ICER);
> +}
> +
> +static void nvic_unmask_irq(struct irq_data *d)
> +{
> +	u32 mask = 1 << (d->hwirq % 32);
> +
> +	writel_relaxed(mask, nvic_bank_base(d) + NVIC_ISER);
> +}

How is that different from what the generic irq chip implementation
does? The only difference is that mask is generated by d->hwirq and
not by d->irq. And due to the fact, that you use a full linear mapping
between hwirq and virq the generic code simply works.

Even if it would not work, it would be trivial to extend the generic
chip with that functionality instead of hacking another slightly
different copy of the same thing.

Thanks,

	tglx
Arnd Bergmann - April 18, 2013, 9:38 a.m.
On Thursday 18 April 2013, Uwe Kleine-König wrote:
> That is, there are (INTLINESNUM + 1) * 32 irqs for INTLINESNUM < 15. For
> INTLINESNUM == 15 there are only 496 and not 16 * 32 == 512. That's the
> same on the gic (just with bigger numbers).

Ok, but since you are now using a linear domain, it doesn't actually hurt
to register 512 in that special case, right?

	Arnd
Uwe Kleine-König - April 19, 2013, 1:51 p.m.
On Thu, Apr 18, 2013 at 11:38:13AM +0200, Arnd Bergmann wrote:
> On Thursday 18 April 2013, Uwe Kleine-König wrote:
> > That is, there are (INTLINESNUM + 1) * 32 irqs for INTLINESNUM < 15. For
> > INTLINESNUM == 15 there are only 496 and not 16 * 32 == 512. That's the
> > same on the gic (just with bigger numbers).
> 
> Ok, but since you are now using a linear domain, it doesn't actually hurt
> to register 512 in that special case, right?
Well, it depends if allocating space for 16 unused unsigned ints hurts
(maybe not). And it makes mapping some irqs successfull while the irq
doesn't really exist. But probably this doesn't hurt either because the
problem already exists.

I don't care much. Is there another advantage beside saving a few source
lines/instructions?

Best regards
Uwe
Arnd Bergmann - April 19, 2013, 2:14 p.m.
On Friday 19 April 2013, Uwe Kleine-König wrote:
> On Thu, Apr 18, 2013 at 11:38:13AM +0200, Arnd Bergmann wrote:
> > On Thursday 18 April 2013, Uwe Kleine-König wrote:
> > > That is, there are (INTLINESNUM + 1) * 32 irqs for INTLINESNUM < 15. For
> > > INTLINESNUM == 15 there are only 496 and not 16 * 32 == 512. That's the
> > > same on the gic (just with bigger numbers).
> > 
> > Ok, but since you are now using a linear domain, it doesn't actually hurt
> > to register 512 in that special case, right?
> Well, it depends if allocating space for 16 unused unsigned ints hurts
> (maybe not).

As long as SPARSE_IRQ is enabled, the space won't actually be allocated.

> And it makes mapping some irqs successfull while the irq
> doesn't really exist. But probably this doesn't hurt either because the
> problem already exists.
> 
> I don't care much. Is there another advantage beside saving a few source
> lines/instructions?

It just feels strange to read the value from hardware and then override
it anyway. But I agree it's not important either way.

	Arnd
Uwe Kleine-König - April 19, 2013, 3:09 p.m.
On Thu, Apr 18, 2013 at 11:35:22AM +0200, Thomas Gleixner wrote:
> On Wed, 17 Apr 2013, Uwe Kleine-König wrote:
> > +struct nvic_bank_data {
> > +	/*
> > +	 * For irq i base holds nvic_base + 4 * i / 32. So you can access the
> > +	 * right ISER register (i.e ISER[i / 32]) by just taking base + ISER.
> > +	 * Ditto for ICER.
> > +	 */
> > +	void __iomem *base;
> > +};
> 
> What's the point of a struct with a single member? Why not having an
> array of base pointers ?
It gives a name to that single member and maybe makes future changes
easier. Obviously you could argue ...

When switching to generic irq chip this struct probably goes away, so I
suggest to postpone this discussion.

> > +static struct nvic_chip_data {
> > +	struct irq_domain *domain;
> > +	struct nvic_bank_data bdata[NVIC_MAX_BANKS];
> > +} nvic_chip_data;
> > +
> > +asmlinkage void __exception_irq_entry
> > +nvic_do_IRQ(irq_hw_number_t hwirq, struct pt_regs *regs)
> > +{
> > +	unsigned int irq = irq_linear_revmap(nvic_chip_data.domain, hwirq);
> > +
> > +	handle_IRQ(irq, regs);
> > +}
> > +
> > +static inline void __iomem *nvic_bank_base(struct irq_data *d)
> > +{
> > +	struct nvic_bank_data *bank_data = irq_data_get_irq_chip_data(d);
> > +	return bank_data->base;
> > +}
> > +
> > +static void nvic_mask_irq(struct irq_data *d)
> > +{
> > +	u32 mask = 1 << (d->hwirq % 32);
> > +
> > +	writel_relaxed(mask, nvic_bank_base(d) + NVIC_ICER);
> > +}
> > +
> > +static void nvic_unmask_irq(struct irq_data *d)
> > +{
> > +	u32 mask = 1 << (d->hwirq % 32);
> > +
> > +	writel_relaxed(mask, nvic_bank_base(d) + NVIC_ISER);
> > +}
> 
> How is that different from what the generic irq chip implementation
> does? The only difference is that mask is generated by d->hwirq and
> not by d->irq. And due to the fact, that you use a full linear mapping
> between hwirq and virq the generic code simply works.
I'm not sure what you mean when you say "full linear mapping". AFAICT
using irq_domain_add_linear doesn't imply that two consecutive hardware
irq numbers get consecutive Linux irq numbers, so using d->irq won't work.
 
> Even if it would not work, it would be trivial to extend the generic
> chip with that functionality instead of hacking another slightly
> different copy of the same thing.
I will try that and report back.

Best regards
Uwe
Uwe Kleine-König - April 22, 2013, 10:02 a.m.
Hello,

(for the new readers of this thread: This is about using

	u32 mask = 1 << (d->hwirq % 32);

instead of

	u32 mask = 1 << (d->irq - gc->irq_base);

in the callbacks for the irq generic chip.)

On Fri, Apr 19, 2013 at 05:09:27PM +0200, Uwe Kleine-König wrote:
> On Thu, Apr 18, 2013 at 11:35:22AM +0200, Thomas Gleixner wrote:
> > On Wed, 17 Apr 2013, Uwe Kleine-König wrote:
> > How is that different from what the generic irq chip implementation
> > does? The only difference is that mask is generated by d->hwirq and
> > not by d->irq. And due to the fact, that you use a full linear mapping
> > between hwirq and virq the generic code simply works.
> I'm not sure what you mean when you say "full linear mapping". AFAICT
> using irq_domain_add_linear doesn't imply that two consecutive hardware
> irq numbers get consecutive Linux irq numbers, so using d->irq won't work.
>  
> > Even if it would not work, it would be trivial to extend the generic
> > chip with that functionality instead of hacking another slightly
> > different copy of the same thing.
> I will try that and report back.
I wonder if using hwirq % 32 should work everywhere where now d->irq -
gc->irqbase is used. Depending on d->hwirq and not d->irq has the upside
of working with non-legacy irq domains, too.

Looking at next-20130419 the affected functions
(irq_gc_mask_disable_reg, irq_gc_mask_set_bit, irq_gc_mask_clr_bit,
irq_gc_unmask_enable_reg, irq_gc_ack_set_bit, irq_gc_ack_clr_bit,
irq_gc_mask_disable_reg_and_ack, irq_gc_eoi, irq_gc_set_wake) are used
in:

arch/arm/mach-davinci/irq.c
arch/arm/mach-imx/avic.c
arch/arm/mach-imx/tzic.c
arch/arm/mach-omap2/irq.c
arch/arm/mach-omap2/prm_common.c

arch/arm/mach-s5p64x0/common.c
	-> uses irq_base=101 for irq_alloc_generic_chip
arch/arm/plat-orion/gpio.c
	-> depends on how orion_gpio_of_init is called. No callers
	found.

arch/arm/plat-orion/irq.c
arch/arm/plat-samsung/irq-vic-timer.c
	-> used for a single irq that isn't a multiple of 32

arch/arm/plat-samsung/s5p-irq-gpioint.c
	-> would need % 8?

arch/mips/jz4740/gpio.c
	-> JZ4740_IRQ_GPIO(0) != JZ4740_IRQ_GPIO0 ?
	-> uses 56 + i * 32 as irqbase

arch/mips/jz4740/irq.c
	-> uses 8 as irqbase

arch/sh/boards/mach-se/7343/irq.c
	-> uses irq_base = irq_linear_revmap(se7343_irq_domain, 0) where
	se7343_irq_domain is a linear domain.
	AFAICT this is a bug. (After adding the domain they map all irqs
	in increasing order which currently seems to guarantee that it
	works. But IMHO it should use a legacy domain.)

arch/sh/boards/mach-se/7722/irq.c
	as above.

drivers/gpio/gpio-mxc.c
drivers/gpio/gpio-mxs.c
drivers/gpio/gpio-omap.c
drivers/gpio/gpio-sodaville.c
drivers/irqchip/irq-sirfsoc.c
drivers/mfd/jz4740-adc.c
	-> uses platform_get_irq(pdev, 1) as irq_base for 5 irqs.

For the uncommented files using %32 instead of -gc->irq_base should
work.

So it seems I cannot just substitute how the mask is called. 

The options I see are:

 - introduce a new set of functions
   Do you have a nice naming scheme?
   irq_gc_unmask_enable_reg_hwirqmod32? Or should I rename the existing
   ones to irq_gc_unmask_enable_reg_irqbaseoffset?
 - use
 	u32 mask = 1 << (d->hwirq - gc->irq_base) % 32;
   This is ugly but might work assuming irq_base == 0 for chips with irq
   domain support and hwirq == irq for the others.

I'm not lucky with the options, so I'm looking forward to suggestions.

Best regards
Uwe
Arnd Bergmann - April 22, 2013, 12:04 p.m.
On Monday 22 April 2013, Uwe Kleine-König wrote:
> Hello,
> 
> (for the new readers of this thread: This is about using
> 
> 	u32 mask = 1 << (d->hwirq % 32);
> 
> instead of
> 
> 	u32 mask = 1 << (d->irq - gc->irq_base);

While not technically any different, I would suggest writing the above
as (d->hwirq & 0x1f), which makes it clear to the reader that this is
intended to be a cheap bit mask operation rather than an expensive
division.
> 
> arch/arm/mach-s5p64x0/common.c
> 	-> uses irq_base=101 for irq_alloc_generic_chip

I think there are plans to replace this code with
drivers/pinctrl/pinctrl-samsung.c, but I don't know if patches
exist already.

> arch/arm/plat-orion/gpio.c
> 	-> depends on how orion_gpio_of_init is called. No callers
> 	found.

As of f9e7592230b, this code has been replaced with a pinctrl driver
and can be killed.

> arch/arm/plat-orion/irq.c
> arch/arm/plat-samsung/irq-vic-timer.c
> 	-> used for a single irq that isn't a multiple of 32

Tomasz Figa has a patch set to remove this file, will likely get merged
in 3.11.

> arch/arm/plat-samsung/s5p-irq-gpioint.c
> 	-> would need % 8?

AFAICT this code is the same driver as arch/arm/mach-s5p64x0/common.c
and will meet the same fate.

> arch/sh/boards/mach-se/7343/irq.c
> 	-> uses irq_base = irq_linear_revmap(se7343_irq_domain, 0) where
> 	se7343_irq_domain is a linear domain.
> 	AFAICT this is a bug. (After adding the domain they map all irqs
> 	in increasing order which currently seems to guarantee that it
> 	works. But IMHO it should use a legacy domain.)
> 
> arch/sh/boards/mach-se/7722/irq.c
> 	as above.

Right. I think irq_domain_add_simple() is the right interface to use
here.

> For the uncommented files using %32 instead of -gc->irq_base should
> work.

I'm not sure I understand why this doesn't work for the ones that use
a base that isn't a multiple of 32. Since you are masking the hwirq
rather than the irq number, it will still be zero-based, won't it?

	Arnd
Thomas Gleixner - April 22, 2013, 1:50 p.m.
On Mon, 22 Apr 2013, Arnd Bergmann wrote:
> On Monday 22 April 2013, Uwe Kleine-König wrote:
> > For the uncommented files using %32 instead of -gc->irq_base should
> > work.
> 
> I'm not sure I understand why this doesn't work for the ones that use
> a base that isn't a multiple of 32. Since you are masking the hwirq
> rather than the irq number, it will still be zero-based, won't it?

The issue is that for anything which uses this w/o irq domains
d->hwirq is 0.

Thanks,

	tglx

Patch

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index a350969..18657fd 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -10,6 +10,10 @@  config ARM_GIC
 config GIC_NON_BANKED
 	bool
 
+config ARM_NVIC
+	bool
+	select IRQ_DOMAIN
+
 config ARM_VIC
 	bool
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 98e3b87..7227c5f 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -7,5 +7,6 @@  obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)	+= irq-metag.o
 obj-$(CONFIG_ARCH_SUNXI)		+= irq-sunxi.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_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
new file mode 100644
index 0000000..6e9ca8a
--- /dev/null
+++ b/drivers/irqchip/irq-nvic.c
@@ -0,0 +1,154 @@ 
+/*
+ * 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/export.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)
+
+struct nvic_bank_data {
+	/*
+	 * For irq i base holds nvic_base + 4 * i / 32. So you can access the
+	 * right ISER register (i.e ISER[i / 32]) by just taking base + ISER.
+	 * Ditto for ICER.
+	 */
+	void __iomem *base;
+};
+
+static struct nvic_chip_data {
+	struct irq_domain *domain;
+	struct nvic_bank_data bdata[NVIC_MAX_BANKS];
+} nvic_chip_data;
+
+asmlinkage void __exception_irq_entry
+nvic_do_IRQ(irq_hw_number_t hwirq, struct pt_regs *regs)
+{
+	unsigned int irq = irq_linear_revmap(nvic_chip_data.domain, hwirq);
+
+	handle_IRQ(irq, regs);
+}
+
+static inline void __iomem *nvic_bank_base(struct irq_data *d)
+{
+	struct nvic_bank_data *bank_data = irq_data_get_irq_chip_data(d);
+	return bank_data->base;
+}
+
+static void nvic_mask_irq(struct irq_data *d)
+{
+	u32 mask = 1 << (d->hwirq % 32);
+
+	writel_relaxed(mask, nvic_bank_base(d) + NVIC_ICER);
+}
+
+static void nvic_unmask_irq(struct irq_data *d)
+{
+	u32 mask = 1 << (d->hwirq % 32);
+
+	writel_relaxed(mask, nvic_bank_base(d) + NVIC_ISER);
+}
+
+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 struct irq_chip nvic_chip = {
+	.name = "NVIC",
+	.irq_mask = nvic_mask_irq,
+	.irq_unmask = nvic_unmask_irq,
+	.irq_eoi = nvic_eoi,
+};
+
+static int nvic_irq_domain_map(struct irq_domain *domain, unsigned int virq,
+			       irq_hw_number_t hw)
+{
+	irq_set_chip_and_handler(virq, &nvic_chip,
+				 handle_fasteoi_irq);
+	irq_set_chip_data(virq, nvic_chip_data.bdata + hw / 32);
+	set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
+
+	return 0;
+}
+
+static struct irq_domain_ops nvic_irq_domain_ops = {
+	.xlate = irq_domain_xlate_onetwocell,
+	.map = nvic_irq_domain_map,
+};
+
+static int __init nvic_of_init(struct device_node *node,
+			       struct device_node *parent)
+{
+	void __iomem *nvic_base;
+	unsigned int irqs, i;
+	unsigned numbanks = (readl_relaxed(V7M_SCS_ICTR) &
+			     V7M_SCS_ICTR_INTLINESNUM_MASK) + 1;
+
+	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;
+
+	for (i = 0; i < numbanks; ++i)
+		nvic_chip_data.bdata[i].base = nvic_base + 4 * i;
+
+	nvic_chip_data.domain =
+		irq_domain_add_linear(node, irqs, &nvic_irq_domain_ops, NULL);
+	if (!nvic_chip_data.domain) {
+		pr_warn("Failed to allocate irq domain\n");
+		return -ENOMEM;
+	}
+
+	/* 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);