diff mbox

[v2,02/21] irqchip: tegra: add DT-based support for legacy interrupt controller

Message ID 1420652576-22309-3-git-send-email-marc.zyngier@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marc Zyngier Jan. 7, 2015, 5:42 p.m. UTC
Tegra's LIC (Legacy Interrupt Controller) has been so far only
supported as a weird extension of the GIC, which is not exactly
pretty.

The stacked irq domain framework fits this pretty well, and allows
the LIC code to be turned into a standalone irqchip. In the process,
make the driver DT aware, something that was sorely missing from
the mach-tegra implementation.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/Makefile    |   1 +
 drivers/irqchip/irq-tegra.c | 335 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 336 insertions(+)
 create mode 100644 drivers/irqchip/irq-tegra.c

Comments

Thierry Reding Jan. 8, 2015, 10:13 a.m. UTC | #1
On Wed, Jan 07, 2015 at 05:42:37PM +0000, Marc Zyngier wrote:
> Tegra's LIC (Legacy Interrupt Controller) has been so far only
> supported as a weird extension of the GIC, which is not exactly
> pretty.
> 
> The stacked irq domain framework fits this pretty well, and allows

Nit: s/irq/IRQ/

> the LIC code to be turned into a standalone irqchip. In the process,
> make the driver DT aware, something that was sorely missing from
> the mach-tegra implementation.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/irqchip/Makefile    |   1 +
>  drivers/irqchip/irq-tegra.c | 335 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 336 insertions(+)
>  create mode 100644 drivers/irqchip/irq-tegra.c

This matches largely what I have in a local patch (modulo the stacked
domains vs. gic_arch_extn). A few comments below.

> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 9516a32..59f34be 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
>  obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
>  obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
>  obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
> +obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
>  obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o

Should these be sorted alphabetically?

> diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
[...]
> +#define TEGRA_MAX_NUM_ICTLRS	5
> +
> +static int num_ictlrs;

This could be unsigned int.

> +struct tegra_ictlr_info {
> +	void __iomem *ictlr_reg_base[TEGRA_MAX_NUM_ICTLRS];

Maybe only "reg_base" or "base". The ictlr_ prefix is redundant.

> +#ifdef CONFIG_PM_SLEEP
> +	u32 cop_ier[TEGRA_MAX_NUM_ICTLRS];
> +	u32 cop_iep[TEGRA_MAX_NUM_ICTLRS];
> +	u32 cpu_ier[TEGRA_MAX_NUM_ICTLRS];
> +	u32 cpu_iep[TEGRA_MAX_NUM_ICTLRS];
> +
> +	u32 ictlr_wake_mask[TEGRA_MAX_NUM_ICTLRS];

Same here.

> +#endif
> +};
> +
> +static struct tegra_ictlr_info *tegra_ictlr_info;

This variable name could be shorter, say "lic", to make some of the code
below more terse.

> +static int tegra_ictlr_suspend(void)
> +{
> +	unsigned long flags;
> +	int i;

This could be unsigned int, too.

[...]
> +static void tegra_ictlr_resume(void)
> +{
> +	unsigned long flags;
> +	int i;

And here.

> +static struct syscore_ops tegra_ictlr_syscore_ops = {
> +	.suspend	= tegra_ictlr_suspend,
> +	.resume		= tegra_ictlr_resume,
> +};
> +
> +static int tegra_ictlr_syscore_init(void)
> +{
> +	register_syscore_ops(&tegra_ictlr_syscore_ops);
> +
> +	return 0;
> +}
> +#else
> +#define tegra_set_wake	NULL
> +static inline void tegra_ictlr_syscore_init(void) {}
> +#endif

Both prototypes for tegra_ictlr_syscore_init() should match here. Since
it never fails, returning void for both should be fine.

> +static struct irq_chip tegra_ictlr_chip = {
> +	.name		= "ICTLR",

Maybe name it "LIC" since that's the more common name for it?

> +static int tegra_ictlr_domain_xlate(struct irq_domain *domain,
> +				    struct device_node *controller,
> +				    const u32 *intspec,
> +				    unsigned int intsize,
> +				    unsigned long *out_hwirq,
> +				    unsigned int *out_type)
> +{
> +	if (domain->of_node != controller)
> +		return -EINVAL;	/* Shouldn't happen, really... */
> +	if (intsize != 3)
> +		return -EINVAL;	/* Not GIC compliant */
> +	if (intspec[0] != 0)

Maybe use GIC_SPI from dt-bindings/interrupt-controller/arm-gic.h here?
To match the DTS content?

> +static int tegra_ictlr_domain_alloc(struct irq_domain *domain,
> +				    unsigned int virq,
> +				    unsigned int nr_irqs, void *data)
> +{
> +	struct of_phandle_args *args = data;
> +	struct of_phandle_args parent_args;
> +	struct tegra_ictlr_info *info = domain->host_data;
> +	irq_hw_number_t hwirq;
> +	int i;

unsigned int?

> +
> +	if (args->args_count != 3)
> +		return -EINVAL;	/* Not GIC compliant */
> +	if (args->args[0] != 0)

GIC_SPI?

> +	hwirq = args->args[1];
> +	if (hwirq >= (num_ictlrs * 32))
> +		return -EINVAL;	/* Can't deal with this */

Not sure if that comment is necessary here. It's fairly obvious why this
is an error. But it doesn't hurt, so if you prefer to leave it, that's
fine with me.

> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		int ictlr = (hwirq + i) / 32;

irq_hw_number_t? Or unsigned int?

> +
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> +					      &tegra_ictlr_chip,
> +					      &info->ictlr_reg_base[ictlr]);
> +	}
> +
> +	parent_args = *args;
> +	parent_args.np = domain->parent->of_node;
> +	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &parent_args);
> +}
> +
> +

There's an extra blank line here.

> +static void tegra_ictlr_domain_free(struct irq_domain *domain,
> +				    unsigned int virq,
> +				    unsigned int nr_irqs)
> +{
> +	int i;

unsigned int?

> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		struct irq_data *d = irq_domain_get_irq_data(domain, virq + i);
> +		irq_domain_reset_irq_data(d);
> +	}
> +}

Aren't you missing a call to irq_domain_free_irqs_parent() here? If so,
why not use irq_domain_free_irqs_common() instead?

> +static struct irq_domain_ops tegra_ictlr_domain_ops = {

static const?

> +	.xlate	= tegra_ictlr_domain_xlate,
> +	.alloc	= tegra_ictlr_domain_alloc,
> +	.free	= tegra_ictlr_domain_free,
> +};
> +
> +static int __init tegra_ictlr_init(struct device_node *node,
> +				   struct device_node *parent)
> +{
> +	struct irq_domain *parent_domain, *domain;
> +	int err;
> +	int i;

unsigned int?

> +
> +	if (!parent) {
> +		pr_err("%s: no parent, giving up\n", node->full_name);
> +		return -ENODEV;
> +	}
> +
> +	parent_domain = irq_find_host(parent);
> +	if (!parent_domain) {
> +		pr_err("%s: unable to obtain parent domain\n", node->full_name);
> +		return -ENXIO;
> +	}
> +
> +	tegra_ictlr_info = kzalloc(sizeof(*tegra_ictlr_info), GFP_KERNEL);
> +	if (!tegra_ictlr_info)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < TEGRA_MAX_NUM_ICTLRS; i++) {
> +		void __iomem *base;
> +
> +		base = of_iomap(node, i);
> +		if (!base)
> +			break;
> +
> +		tegra_ictlr_info->ictlr_reg_base[i] = base;
> +
> +		/* Disable all interrupts */
> +		writel_relaxed(~0, base + ICTLR_CPU_IER_CLR);

I think you used ~0ul elsewhere, so maybe make this (or the other
instances) consistent?

> +		/* All interrupts target IRQ */
> +		writel_relaxed(0, base + ICTLR_CPU_IEP_CLASS);
> +
> +		num_ictlrs++;
> +	}
> +
> +	if (!num_ictlrs) {
> +		pr_err("%s: no valid regions, giving up\n", node->full_name);
> +		err = -ENOMEM;
> +		goto out_free;
> +	}

The local patch that I have does this slightly differently. We have a
tiny table of struct of_device_ids that contains the number of interrupt
controllers to expect for a given compatible (see also below). The code
then checks that the node indeed has valid reg entries for each of the
controllers. So this could be something like:

	struct tegra_ictlr_soc {
		unsigned int num_ictlrs;
	};

	static const struct tegra_ictlr_soc tegra20_ictlr_soc = {
		.num_ictlrs = 4,
	};

	static const struct tegra_ictlr_soc tegra30_ictlr_soc = {
		.num_ictlrs = 5,
	};

	static const struct of_device_id ictlr_matches[] = {
		{ .compatible = "nvidia,tegra30-ictlr", .data = &tegra30_ictlr_soc },
		{ .compatible = "nvidia,tegra20-ictlr", .data = &tegra20_ictlr_soc },
		{ }
	};

And in tegra_ictrl_init():

	const struct of_device_id *match;

	match = of_match_node(ictlr_matches, node);
	if (!match)
		return -ENODEV;

	soc = match->data;

	...

	WARN(num_ictlrs != soc->num_ictlrs,
	     "Found %u interrupt controllers in DT; expected %u.\n",
	     num_ictlrs, soc->num_ictlrs);

> +
> +	domain = irq_domain_add_hierarchy(parent_domain, 0, num_ictlrs * 32,
> +					  node, &tegra_ictlr_domain_ops,
> +					  tegra_ictlr_info);

Do we need to add a select IRQ_DOMAIN_HIERARCHY somewhere for this to
compile? I guess it's already implied by the select ARM_GIC from
ARCH_TEGRA, so maybe we don't have to be explicit.

> +	if (!domain) {
> +		pr_err("%s: failed to allocated domain\n", node->full_name);
> +		err = -ENOMEM;
> +		goto out_unmap;
> +	}
> +
> +	tegra_ictlr_syscore_init();
> +
> +	pr_info("%s: %d interrupts forwarded to %s\n",
> +		node->full_name, num_ictlrs * 32, parent->full_name);
> +
> +	return 0;
> +
> +out_unmap:
> +	for (i = 0; i < num_ictlrs; i++)
> +		iounmap(tegra_ictlr_info->ictlr_reg_base[i]);
> +out_free:
> +	kfree(tegra_ictlr_info);
> +	return err;
> +}
> +
> +IRQCHIP_DECLARE(tegra_ictlr, "nvidia,tegra-ictlr", tegra_ictlr_init);

In general we name compatible values after the first generation of the
SoC that introduced them. For the LIC this would be:

	compatible = "nvidia,tegra20-ictlr";
	
With Tegra30 the number of interrupt controllers was increased to 5, so
it needs a new compatible:

	compatible = "nvidia,tegra30-ictlr";

Subsequent generations remain compatible with this variant, but they
traditionally get their own compatible, so for Tegra114 we'd get:

	compatible = "nvidia,tegra114-ictlr", "nvidia,tegra30-ictlr";

And similar for Tegra124. So I think the IRQCHIP_DECLARE should match on
"nvidia,tegra20-ictlr" instead. For the same reason we should have a
second entry for the "nvidia,tegra30-ictlr" compatible.

Thierry
Nishanth Menon Jan. 8, 2015, 3:06 p.m. UTC | #2
On 17:42-20150107, Marc Zyngier wrote:
> Tegra's LIC (Legacy Interrupt Controller) has been so far only
> supported as a weird extension of the GIC, which is not exactly
> pretty.
> 
> The stacked irq domain framework fits this pretty well, and allows
> the LIC code to be turned into a standalone irqchip. In the process,
> make the driver DT aware, something that was sorely missing from
> the mach-tegra implementation.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---

Saw a few checkpatch warnings as below: all of them seem minors.

@@ -0,0 +1,35 @@
+WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
+#36: 
+new file mode 100644
+WARNING: line over 80 characters
+#169: FILE: drivers/irqchip/irq-tegra.c:129:
++		tegra_ictlr_info->cpu_ier[i] = readl_relaxed(ictlr + ICTLR_CPU_IER);
+WARNING: line over 80 characters
+#170: FILE: drivers/irqchip/irq-tegra.c:130:
++		tegra_ictlr_info->cpu_iep[i] = readl_relaxed(ictlr + ICTLR_CPU_IEP_CLASS);
+WARNING: line over 80 characters
+#171: FILE: drivers/irqchip/irq-tegra.c:131:
++		tegra_ictlr_info->cop_ier[i] = readl_relaxed(ictlr + ICTLR_COP_IER);
+WARNING: line over 80 characters
+#172: FILE: drivers/irqchip/irq-tegra.c:132:
++		tegra_ictlr_info->cop_iep[i] = readl_relaxed(ictlr + ICTLR_COP_IEP_CLASS);
+WARNING: line over 80 characters
+#181: FILE: drivers/irqchip/irq-tegra.c:141:
++		writel_relaxed(tegra_ictlr_info->ictlr_wake_mask[i], ictlr + ICTLR_CPU_IER_SET);
+WARNING: Missing a blank line after declarations
+#196: FILE: drivers/irqchip/irq-tegra.c:156:
++		void __iomem *ictlr = tegra_ictlr_info->ictlr_reg_base[i];
++		writel_relaxed(tegra_ictlr_info->cpu_iep[i],
+WARNING: line over 80 characters
+#284: FILE: drivers/irqchip/irq-tegra.c:244:
++	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &parent_args);
+CHECK: Please don't use multiple blank lines
+#287: FILE: drivers/irqchip/irq-tegra.c:247:
++
++
+WARNING: Missing a blank line after declarations
+#296: FILE: drivers/irqchip/irq-tegra.c:256:
++		struct irq_data *d = irq_domain_get_irq_data(domain, virq + i);
++		irq_domain_reset_irq_data(d);
`
Marc Zyngier Jan. 10, 2015, 12:28 p.m. UTC | #3
On 2015-01-08 15:06, Nishanth Menon wrote:
> On 17:42-20150107, Marc Zyngier wrote:
>> Tegra's LIC (Legacy Interrupt Controller) has been so far only
>> supported as a weird extension of the GIC, which is not exactly
>> pretty.
>>
>> The stacked irq domain framework fits this pretty well, and allows
>> the LIC code to be turned into a standalone irqchip. In the process,
>> make the driver DT aware, something that was sorely missing from
>> the mach-tegra implementation.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>
> Saw a few checkpatch warnings as below: all of them seem minors.
>
> @@ -0,0 +1,35 @@
> +WARNING: added, moved or deleted file(s), does MAINTAINERS need 
> updating?

I'll leave to the Tegra maintainers to update this file it they want 
to.

> +#36:
> +new file mode 100644
> +WARNING: line over 80 characters
> +#169: FILE: drivers/irqchip/irq-tegra.c:129:
> ++		tegra_ictlr_info->cpu_ier[i] = readl_relaxed(ictlr + 
> ICTLR_CPU_IER);

As a matter of principle, I ignore what checkpatch says about the 
length of lines of code. I trust my eyes more than the tool.

[...]

> +WARNING: Missing a blank line after declarations
> +#196: FILE: drivers/irqchip/irq-tegra.c:156:
> ++		void __iomem *ictlr = tegra_ictlr_info->ictlr_reg_base[i];
> ++		writel_relaxed(tegra_ictlr_info->cpu_iep[i],

Done.

> +WARNING: line over 80 characters
> +#284: FILE: drivers/irqchip/irq-tegra.c:244:
> ++	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, 
> &parent_args);
> +CHECK: Please don't use multiple blank lines
> +#287: FILE: drivers/irqchip/irq-tegra.c:247:
> ++
> ++
> +WARNING: Missing a blank line after declarations
> +#296: FILE: drivers/irqchip/irq-tegra.c:256:
> ++		struct irq_data *d = irq_domain_get_irq_data(domain, virq + i);
> ++		irq_domain_reset_irq_data(d);
> `

Thanks,

         M.
diff mbox

Patch

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 9516a32..59f34be 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -6,6 +6,7 @@  obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
 obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o
 obj-$(CONFIG_METAG)			+= irq-metag-ext.o
diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
new file mode 100644
index 0000000..b4fc2e3
--- /dev/null
+++ b/drivers/irqchip/irq-tegra.c
@@ -0,0 +1,335 @@ 
+/*
+ * Driver code for Tegra's Legacy Interrupt Controller
+ *
+ * Author: Marc Zyngier <marc.zyngier@arm.com>
+ *
+ * Heavily based on the original arch/arm/mach-tegra/irq.c code:
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * Author:
+ *	Colin Cross <ccross@android.com>
+ *
+ * Copyright (C) 2010,2013, NVIDIA Corporation
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/syscore_ops.h>
+
+#include "irqchip.h"
+
+#define ICTLR_CPU_IEP_VFIQ	0x08
+#define ICTLR_CPU_IEP_FIR	0x14
+#define ICTLR_CPU_IEP_FIR_SET	0x18
+#define ICTLR_CPU_IEP_FIR_CLR	0x1c
+
+#define ICTLR_CPU_IER		0x20
+#define ICTLR_CPU_IER_SET	0x24
+#define ICTLR_CPU_IER_CLR	0x28
+#define ICTLR_CPU_IEP_CLASS	0x2C
+
+#define ICTLR_COP_IER		0x30
+#define ICTLR_COP_IER_SET	0x34
+#define ICTLR_COP_IER_CLR	0x38
+#define ICTLR_COP_IEP_CLASS	0x3c
+
+#define TEGRA_MAX_NUM_ICTLRS	5
+
+static int num_ictlrs;
+
+struct tegra_ictlr_info {
+	void __iomem *ictlr_reg_base[TEGRA_MAX_NUM_ICTLRS];
+#ifdef CONFIG_PM_SLEEP
+	u32 cop_ier[TEGRA_MAX_NUM_ICTLRS];
+	u32 cop_iep[TEGRA_MAX_NUM_ICTLRS];
+	u32 cpu_ier[TEGRA_MAX_NUM_ICTLRS];
+	u32 cpu_iep[TEGRA_MAX_NUM_ICTLRS];
+
+	u32 ictlr_wake_mask[TEGRA_MAX_NUM_ICTLRS];
+#endif
+};
+
+static struct tegra_ictlr_info *tegra_ictlr_info;
+
+static inline void tegra_ictlr_write_mask(struct irq_data *d, unsigned long reg)
+{
+	void __iomem *base = d->chip_data;
+	u32 mask;
+
+	mask = BIT(d->hwirq % 32);
+	writel_relaxed(mask, base + reg);
+}
+
+static void tegra_mask(struct irq_data *d)
+{
+	tegra_ictlr_write_mask(d, ICTLR_CPU_IER_CLR);
+	irq_chip_mask_parent(d);
+}
+
+static void tegra_unmask(struct irq_data *d)
+{
+	tegra_ictlr_write_mask(d, ICTLR_CPU_IER_SET);
+	irq_chip_unmask_parent(d);
+}
+
+static void tegra_eoi(struct irq_data *d)
+{
+	tegra_ictlr_write_mask(d, ICTLR_CPU_IEP_FIR_CLR);
+	irq_chip_eoi_parent(d);
+}
+
+static int tegra_retrigger(struct irq_data *d)
+{
+	tegra_ictlr_write_mask(d, ICTLR_CPU_IEP_FIR_SET);
+	return irq_chip_retrigger_hierarchy(d);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra_set_wake(struct irq_data *d, unsigned int enable)
+{
+	u32 irq = d->hwirq;
+	u32 index, mask;
+
+	index = (irq / 32);
+	mask = BIT(irq % 32);
+	if (enable)
+		tegra_ictlr_info->ictlr_wake_mask[index] |= mask;
+	else
+		tegra_ictlr_info->ictlr_wake_mask[index] &= ~mask;
+
+	/*
+	 * Do *not* call into the parent, as the GIC doesn't have any
+	 * wake-up facility...
+	 */
+	return 0;
+}
+
+static int tegra_ictlr_suspend(void)
+{
+	unsigned long flags;
+	int i;
+
+	local_irq_save(flags);
+	for (i = 0; i < num_ictlrs; i++) {
+		void __iomem *ictlr = tegra_ictlr_info->ictlr_reg_base[i];
+		/* Save interrupt state */
+		tegra_ictlr_info->cpu_ier[i] = readl_relaxed(ictlr + ICTLR_CPU_IER);
+		tegra_ictlr_info->cpu_iep[i] = readl_relaxed(ictlr + ICTLR_CPU_IEP_CLASS);
+		tegra_ictlr_info->cop_ier[i] = readl_relaxed(ictlr + ICTLR_COP_IER);
+		tegra_ictlr_info->cop_iep[i] = readl_relaxed(ictlr + ICTLR_COP_IEP_CLASS);
+
+		/* Disable COP interrupts */
+		writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
+
+		/* Disable CPU interrupts */
+		writel_relaxed(~0ul, ictlr + ICTLR_CPU_IER_CLR);
+
+		/* Enable the wakeup sources of ictlr */
+		writel_relaxed(tegra_ictlr_info->ictlr_wake_mask[i], ictlr + ICTLR_CPU_IER_SET);
+	}
+	local_irq_restore(flags);
+
+	return 0;
+}
+
+static void tegra_ictlr_resume(void)
+{
+	unsigned long flags;
+	int i;
+
+	local_irq_save(flags);
+	for (i = 0; i < num_ictlrs; i++) {
+		void __iomem *ictlr = tegra_ictlr_info->ictlr_reg_base[i];
+		writel_relaxed(tegra_ictlr_info->cpu_iep[i],
+			       ictlr + ICTLR_CPU_IEP_CLASS);
+		writel_relaxed(~0ul, ictlr + ICTLR_CPU_IER_CLR);
+		writel_relaxed(tegra_ictlr_info->cpu_ier[i],
+			       ictlr + ICTLR_CPU_IER_SET);
+		writel_relaxed(tegra_ictlr_info->cop_iep[i],
+			       ictlr + ICTLR_COP_IEP_CLASS);
+		writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
+		writel_relaxed(tegra_ictlr_info->cop_ier[i],
+			       ictlr + ICTLR_COP_IER_SET);
+	}
+	local_irq_restore(flags);
+}
+
+static struct syscore_ops tegra_ictlr_syscore_ops = {
+	.suspend	= tegra_ictlr_suspend,
+	.resume		= tegra_ictlr_resume,
+};
+
+static int tegra_ictlr_syscore_init(void)
+{
+	register_syscore_ops(&tegra_ictlr_syscore_ops);
+
+	return 0;
+}
+#else
+#define tegra_set_wake	NULL
+static inline void tegra_ictlr_syscore_init(void) {}
+#endif
+
+static struct irq_chip tegra_ictlr_chip = {
+	.name		= "ICTLR",
+	.irq_eoi	= tegra_eoi,
+	.irq_mask	= tegra_mask,
+	.irq_unmask	= tegra_unmask,
+	.irq_retrigger	= tegra_retrigger,
+	.irq_set_wake	= tegra_set_wake,
+	.flags		= IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static int tegra_ictlr_domain_xlate(struct irq_domain *domain,
+				    struct device_node *controller,
+				    const u32 *intspec,
+				    unsigned int intsize,
+				    unsigned long *out_hwirq,
+				    unsigned int *out_type)
+{
+	if (domain->of_node != controller)
+		return -EINVAL;	/* Shouldn't happen, really... */
+	if (intsize != 3)
+		return -EINVAL;	/* Not GIC compliant */
+	if (intspec[0] != 0)
+		return -EINVAL;	/* No PPI should point to this domain */
+
+	*out_hwirq = intspec[1];
+	*out_type = intspec[2];
+	return 0;
+}
+
+static int tegra_ictlr_domain_alloc(struct irq_domain *domain,
+				    unsigned int virq,
+				    unsigned int nr_irqs, void *data)
+{
+	struct of_phandle_args *args = data;
+	struct of_phandle_args parent_args;
+	struct tegra_ictlr_info *info = domain->host_data;
+	irq_hw_number_t hwirq;
+	int i;
+
+	if (args->args_count != 3)
+		return -EINVAL;	/* Not GIC compliant */
+	if (args->args[0] != 0)
+		return -EINVAL;	/* No PPI should point to this domain */
+
+	hwirq = args->args[1];
+	if (hwirq >= (num_ictlrs * 32))
+		return -EINVAL;	/* Can't deal with this */
+
+	for (i = 0; i < nr_irqs; i++) {
+		int ictlr = (hwirq + i) / 32;
+
+		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+					      &tegra_ictlr_chip,
+					      &info->ictlr_reg_base[ictlr]);
+	}
+
+	parent_args = *args;
+	parent_args.np = domain->parent->of_node;
+	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &parent_args);
+}
+
+
+static void tegra_ictlr_domain_free(struct irq_domain *domain,
+				    unsigned int virq,
+				    unsigned int nr_irqs)
+{
+	int i;
+
+	for (i = 0; i < nr_irqs; i++) {
+		struct irq_data *d = irq_domain_get_irq_data(domain, virq + i);
+		irq_domain_reset_irq_data(d);
+	}
+}
+
+static struct irq_domain_ops tegra_ictlr_domain_ops = {
+	.xlate	= tegra_ictlr_domain_xlate,
+	.alloc	= tegra_ictlr_domain_alloc,
+	.free	= tegra_ictlr_domain_free,
+};
+
+static int __init tegra_ictlr_init(struct device_node *node,
+				   struct device_node *parent)
+{
+	struct irq_domain *parent_domain, *domain;
+	int err;
+	int i;
+
+	if (!parent) {
+		pr_err("%s: no parent, giving up\n", node->full_name);
+		return -ENODEV;
+	}
+
+	parent_domain = irq_find_host(parent);
+	if (!parent_domain) {
+		pr_err("%s: unable to obtain parent domain\n", node->full_name);
+		return -ENXIO;
+	}
+
+	tegra_ictlr_info = kzalloc(sizeof(*tegra_ictlr_info), GFP_KERNEL);
+	if (!tegra_ictlr_info)
+		return -ENOMEM;
+
+	for (i = 0; i < TEGRA_MAX_NUM_ICTLRS; i++) {
+		void __iomem *base;
+
+		base = of_iomap(node, i);
+		if (!base)
+			break;
+
+		tegra_ictlr_info->ictlr_reg_base[i] = base;
+
+		/* Disable all interrupts */
+		writel_relaxed(~0, base + ICTLR_CPU_IER_CLR);
+		/* All interrupts target IRQ */
+		writel_relaxed(0, base + ICTLR_CPU_IEP_CLASS);
+
+		num_ictlrs++;
+	}
+
+	if (!num_ictlrs) {
+		pr_err("%s: no valid regions, giving up\n", node->full_name);
+		err = -ENOMEM;
+		goto out_free;
+	}
+
+	domain = irq_domain_add_hierarchy(parent_domain, 0, num_ictlrs * 32,
+					  node, &tegra_ictlr_domain_ops,
+					  tegra_ictlr_info);
+	if (!domain) {
+		pr_err("%s: failed to allocated domain\n", node->full_name);
+		err = -ENOMEM;
+		goto out_unmap;
+	}
+
+	tegra_ictlr_syscore_init();
+
+	pr_info("%s: %d interrupts forwarded to %s\n",
+		node->full_name, num_ictlrs * 32, parent->full_name);
+
+	return 0;
+
+out_unmap:
+	for (i = 0; i < num_ictlrs; i++)
+		iounmap(tegra_ictlr_info->ictlr_reg_base[i]);
+out_free:
+	kfree(tegra_ictlr_info);
+	return err;
+}
+
+IRQCHIP_DECLARE(tegra_ictlr, "nvidia,tegra-ictlr", tegra_ictlr_init);