On Tue, Oct 08, 2013 at 01:24:26PM +0100, Sebastian Hesselbarth wrote:[...]This adds an irqchip driver and corresponding devicetree binding for the
secondary interrupt controllers based on Synopsys DesignWare IP dw_apb_ictl.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx>
---
diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
new file mode 100644
index 0000000..7ccd1ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/snps,dw-apb-ictl.txt
@@ -0,0 +1,29 @@
+Synopsys DesignWare APB interrupt controller (dw_apb_ictl)
+
+Synopsys DesignWare provides interrupt controller IP for APB known as
+dw_apb_ictl. The IP is used as secondary interrupt controller in some SoCs with
+APB bus, e.g. Marvell Armada 1500.
+
+Required properties:
+- compatible: shall be "snps,dw-apb-ictl"
+- reg: base address of interrupt registers starting with ENABLE_LOW register
Is ENABLE_LOW the first register? Or are there registers before?
Is there only one bank of registers that needs to be defined?
This isn't just a base address, as it has a size too. The terminology's
rather inconsistent for reg properties in general...
+- interrupt-controller: identifies the node as an interrupt controller
+- #interrupt-cells: number of cells to encode an interrupt source, shall be 1
s/interrupt source/interrupt-specifier/
+- interrupts: interrupt reference to primary interrupt controller
- interrupts: interrupts specifier for the sole interrupt fed to the
parent interrupt controller.
Is there only a single output interrupt?
Is this required? Is it possible for this to be wired directly into a
CPU rather than another interrupt controller?
+- interrupt-parent: (optional) reference specific primary interrupt controller
+
+The interrupt sources map to the corresponding bits in the interrupt
+registers, i.e.
+- 0 maps to bit 0 of low interrupts,
+- 1 maps to bit 1 of low interrupts,
+- 32 maps to bit 0 of high interrupts, and so on.
I couldn't see any public documentation for this, so I can't really
follow the "and so on", but I saw that this had optional FIQ support so
I assume there are more interrupt values that can be encoded?
[...]+Example:
+ aic: interrupt-controller@3000 {
+ compatible = "snps,dw-apb-ictl";
+ reg = <0x3000 0xc00>;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+ };
[...]diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
new file mode 100644
index 0000000..bbcacee
--- /dev/null
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -0,0 +1,142 @@
+/*
+ * Synopsys DW APB ICTL irqchip driver.
+ *
+ * Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx>
+ *
+ * based on GPL'ed 2.6 kernel sources
+ * (c) Marvell International Ltd.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+static int __init dw_apb_ictl_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;
+ void __iomem *iobase;
+ int ret, nrirqs, irq;
+ u32 reg;
+
+ /* Map the parent interrupt for the chained handler */
+ irq = irq_of_parse_and_map(np, 0);
+ if (irq <= 0) {
+ pr_err("%s: unable to parse irq\n", np->name);
+ return -EINVAL;
+ }
+
+ ret = of_address_to_resource(np, 0, &r);
+ if (ret) {
+ pr_err("%s: unable to get resource\n", np->name);
+ return ret;
+ }
+
+ if (!request_mem_region(r.start, resource_size(&r), np->name)) {
+ pr_err("%s: unable to request mem region\n", np->name);
+ return -ENOMEM;
+ }
+
+ iobase = ioremap(r.start, resource_size(&r));
+ if (!iobase) {
+ pr_err("%s: unable to map resource\n", np->name);
+ return -ENOMEM;
+ }
Could you not use of_iomap?
Also, I'd recommend using np->full_name for error messages, as that
gives you the full path for the node, which is far more helpful for
debugging than just the unqualified node name.
+
+ /*
+ * DW IP can be configured to allow 2-64 irqs. We can determine
+ * the number of irqs supported by writing into enable register
+ * and look for bits not set, as corresponding flip-flops will
+ * have been removed by sythesis tool.
+ */
Is that always true?