2013-05-02 18:25:33

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH] irqchip: add support for Marvell Orion SoCs

This patch adds an irqchip driver for the main interrupt controller found
on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
Corresponding device tree documentation is also added.

Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Note: This patch triggers a checkpatch warning for
WARNING: Avoid CamelCase: <handle_IRQ>

Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: Jean-Francois Moine <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
.../interrupt-controller/marvell,orion-mpic.txt | 22 ++++
drivers/irqchip/Kconfig | 5 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-orion.c | 129 ++++++++++++++++++++
include/linux/irqchip/orion.h | 18 +++
5 files changed, 175 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,orion-mpic.txt
create mode 100644 drivers/irqchip/irq-orion.c
create mode 100644 include/linux/irqchip/orion.h

diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-mpic.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-mpic.txt
new file mode 100644
index 0000000..3b303ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-mpic.txt
@@ -0,0 +1,22 @@
+Marvell Orion SoC main interrupt controller
+
+Required properties:
+- compatible: shall be "marvell,orion-mpic"
+- reg: base address(es) of interrupt registers starting with CAUSE register
+- interrupt-controller: identifies the node as an interrupt controller
+- #interrupt-cells: number of cells to encode an interrupt source, shall be 1.
+
+The interrupt sources map to the corresponding bits in the interrupt
+registers, i.e.
+- 0 maps to bit 0 of first base address,
+- 1 maps to bit 1 of first base address,
+- 32 maps to bit 0 of second base address, and so on.
+
+Example:
+ intc: interrupt-controller {
+ compatible = "marvell,orion-mpic";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ /* Dove has 64 primary interrupts */
+ reg = <0x20200 0x10>, <0x20210 0x10>;
+ };
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index a350969..8da3559 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -2,6 +2,11 @@ config IRQCHIP
def_bool y
depends on OF_IRQ

+config IRQCHIP_ORION
+ bool
+ select IRQ_DOMAIN
+ select MULTI_IRQ_HANDLER
+
config ARM_GIC
bool
select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 98e3b87..8adbd43 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_IRQCHIP) += irqchip.o

obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
obj-$(CONFIG_ARCH_EXYNOS) += exynos-combiner.o
+obj-$(CONFIG_IRQCHIP_ORION) += irq-orion.o
obj-$(CONFIG_METAG) += irq-metag-ext.o
obj-$(CONFIG_METAG_PERFCOUNTER_IRQS) += irq-metag.o
obj-$(CONFIG_ARCH_SUNXI) += irq-sunxi.o
diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
new file mode 100644
index 0000000..ea02e11
--- /dev/null
+++ b/drivers/irqchip/irq-orion.c
@@ -0,0 +1,129 @@
+/*
+ * Marvell Orion SoCs IRQ chip driver.
+ *
+ * Sebastian Hesselbarth <[email protected]>
+ *
+ * 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.
+ */
+
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip/orion.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <asm/mach/irq.h>
+
+/* max number of handled irq register blocks */
+#define ORION_MAX_IRQREG 2
+
+#define ORION_IRQ_CAUSE 0x00
+#define ORION_IRQ_MASK 0x04
+#define ORION_IRQ_FIQ_MASK 0x08
+#define ORION_IRQ_ENDP_MASK 0x0c
+
+static void __iomem *orion_irq_base[ORION_MAX_IRQREG];
+static unsigned int orion_irq_regs;
+static struct irq_domain *orion_irq_domain;
+
+asmlinkage void __exception_irq_entry orion_handle_irq(struct pt_regs *regs)
+{
+ int n;
+ for (n = 0; n < orion_irq_regs; n++) {
+ u32 hwirq_base = n * 32;
+ u32 stat = readl_relaxed(orion_irq_base[n] + ORION_IRQ_CAUSE) &
+ readl_relaxed(orion_irq_base[n] + ORION_IRQ_MASK);
+ while (stat) {
+ u32 hwirq = ffs(stat) - 1;
+ u32 irq = irq_find_mapping(orion_irq_domain,
+ hwirq_base + hwirq);
+ handle_IRQ(irq, regs);
+ stat &= ~(1 << hwirq);
+ }
+ }
+}
+
+static void orion_irq_mask(struct irq_data *irqd)
+{
+ unsigned int irq = irqd_to_hwirq(irqd);
+ unsigned int irq_off = irq % 32;
+ int reg = irq / 32;
+ u32 val;
+
+ val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
+ writel(val & ~(1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
+}
+
+static void orion_irq_unmask(struct irq_data *irqd)
+{
+ unsigned int irq = irqd_to_hwirq(irqd);
+ unsigned int irq_off = irq % 32;
+ int reg = irq / 32;
+ u32 val;
+
+ val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
+ writel(val | (1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
+}
+
+static struct irq_chip orion_irq_chip = {
+ .name = "orion_irq",
+ .irq_mask = orion_irq_mask,
+ .irq_unmask = orion_irq_unmask,
+};
+
+static int orion_irq_map(struct irq_domain *d, unsigned int virq,
+ irq_hw_number_t hw)
+{
+ irq_set_chip_and_handler(virq, &orion_irq_chip,
+ handle_level_irq);
+ set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
+
+ return 0;
+}
+
+static struct irq_domain_ops orion_irq_ops = {
+ .map = orion_irq_map,
+ .xlate = irq_domain_xlate_onecell,
+};
+
+static int __init orion_of_init(struct device_node *np,
+ struct device_node *parent)
+{
+ int n;
+
+ for (n = 0; n < ORION_MAX_IRQREG; n++) {
+ orion_irq_base[n] = of_iomap(np, n);
+
+ if (!orion_irq_base[n])
+ continue;
+
+ /* mask all interrupts */
+ writel(0, orion_irq_base[n] + ORION_IRQ_MASK);
+ orion_irq_regs++;
+ }
+
+ /* at least one irq reg must be set */
+ if (!orion_irq_regs)
+ panic("%s: unable to map IRQC registers\n", np->full_name);
+
+ orion_irq_domain = irq_domain_add_linear(np, orion_irq_regs * 32,
+ &orion_irq_ops, NULL);
+ if (!orion_irq_domain)
+ panic("%s: unable to create IRQ domain\n", np->full_name);
+
+ set_handle_irq(orion_handle_irq);
+
+ return 0;
+}
+
+static struct of_device_id orion_irq_dt_ids[] __initconst = {
+ { .compatible = "marvell,orion-mpic", .data = orion_of_init },
+ { }
+};
+
+void __init orion_init_irq(void)
+{
+ of_irq_init(orion_irq_dt_ids);
+}
diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h
new file mode 100644
index 0000000..04f7bab
--- /dev/null
+++ b/include/linux/irqchip/orion.h
@@ -0,0 +1,18 @@
+/*
+ * Marvell Orion SoCs IRQ chip driver header.
+ *
+ * Sebastian Hesselbarth <[email protected]>
+ *
+ * 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.
+ */
+
+#ifndef __LINUX_IRQCHIP_ORION_H
+#define __LINUX_IRQCHIP_ORION_H
+
+#include <asm/exception.h>
+
+extern void orion_init_irq(void);
+
+#endif
--
1.7.2.5


2013-05-02 18:33:56

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/2013 08:25 PM, Sebastian Hesselbarth wrote:
> This patch adds an irqchip driver for the main interrupt controller found
> on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
> Corresponding device tree documentation is also added.
>
> Signed-off-by: Sebastian Hesselbarth<[email protected]>
> ---
> Note: This patch triggers a checkpatch warning for
> WARNING: Avoid CamelCase:<handle_IRQ>
>
[...]
> diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h
> new file mode 100644
> index 0000000..04f7bab
> --- /dev/null
> +++ b/include/linux/irqchip/orion.h
> @@ -0,0 +1,18 @@
> +/*
> + * Marvell Orion SoCs IRQ chip driver header.
> + *
> + * Sebastian Hesselbarth<[email protected]>
> + *
> + * 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.
> + */
> +
> +#ifndef __LINUX_IRQCHIP_ORION_H
> +#define __LINUX_IRQCHIP_ORION_H
> +
> +#include<asm/exception.h>

First review by myself. The above include is a left-over and
will be removed in a v2.

> +
> +extern void orion_init_irq(void);
> +
> +#endif

2013-05-02 18:46:40

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thu, May 02, 2013 at 08:33:48PM +0200, Sebastian Hesselbarth wrote:
> On 05/02/2013 08:25 PM, Sebastian Hesselbarth wrote:
>> This patch adds an irqchip driver for the main interrupt controller found
>> on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
>> Corresponding device tree documentation is also added.
>>
>> Signed-off-by: Sebastian Hesselbarth<[email protected]>
>> ---
>> Note: This patch triggers a checkpatch warning for
>> WARNING: Avoid CamelCase:<handle_IRQ>
>>
> [...]
>> diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h
>> new file mode 100644
>> index 0000000..04f7bab
>> --- /dev/null
>> +++ b/include/linux/irqchip/orion.h
>> @@ -0,0 +1,18 @@
>> +/*
>> + * Marvell Orion SoCs IRQ chip driver header.
>> + *
>> + * Sebastian Hesselbarth<[email protected]>
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef __LINUX_IRQCHIP_ORION_H
>> +#define __LINUX_IRQCHIP_ORION_H
>> +
>> +#include<asm/exception.h>
>
> First review by myself. The above include is a left-over and
> will be removed in a v2.

You still need your first level IRQ handlers marked with __exception_irq_entry
which is defined in the above file.

2013-05-02 18:53:43

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thu, May 02, 2013 at 08:25:04PM +0200, Sebastian Hesselbarth wrote:
> +
> +static void __iomem *orion_irq_base[ORION_MAX_IRQREG];
> +static unsigned int orion_irq_regs;
> +static struct irq_domain *orion_irq_domain;
> +
> +asmlinkage void __exception_irq_entry orion_handle_irq(struct
> pt_regs *regs)

This can be static?

> +static int __init orion_of_init(struct device_node *np,
> + struct device_node *parent)
> +{
> + int n;
> +
> + for (n = 0; n < ORION_MAX_IRQREG; n++) {
> + orion_irq_base[n] = of_iomap(np, n);

Is it possible to also reserve the resources for these registers at
this point in the boot sequence?

> +static struct of_device_id orion_irq_dt_ids[] __initconst = {
> + { .compatible = "marvell,orion-mpic", .data = orion_of_init },
> + { }

Is there a strong reason to change the compatible string? Looks to me
like either the new driver or the old driver will bind depending on
what is in the machine description. No need for a new string?

> +};
> +
> +void __init orion_init_irq(void)
> +{
> + of_irq_init(orion_irq_dt_ids);
> +}

Shouldn't this use the new IRQCHIP_DECLARE mechanism?

> diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h

> +extern void orion_init_irq(void);

.. which lets this go away, use the generic irqchip_init instead of
orion_init_irq.

Regards,
Jason

2013-05-02 18:54:28

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/13 20:45, Russell King - ARM Linux wrote:
> On Thu, May 02, 2013 at 08:33:48PM +0200, Sebastian Hesselbarth wrote:
>> On 05/02/2013 08:25 PM, Sebastian Hesselbarth wrote:
>>> This patch adds an irqchip driver for the main interrupt controller found
>>> on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
>>> Corresponding device tree documentation is also added.
>>>
>>> Signed-off-by: Sebastian Hesselbarth<[email protected]>
>>> ---
>>> Note: This patch triggers a checkpatch warning for
>>> WARNING: Avoid CamelCase:<handle_IRQ>
>>>
>> [...]
>>> diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h
>>> new file mode 100644
>>> index 0000000..04f7bab
>>> --- /dev/null
>>> +++ b/include/linux/irqchip/orion.h
>>> @@ -0,0 +1,18 @@
>>> +/*
>>> + * Marvell Orion SoCs IRQ chip driver header.
>>> + *
>>> + * Sebastian Hesselbarth<[email protected]>
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#ifndef __LINUX_IRQCHIP_ORION_H
>>> +#define __LINUX_IRQCHIP_ORION_H
>>> +
>>> +#include<asm/exception.h>
>>
>> First review by myself. The above include is a left-over and
>> will be removed in a v2.
>
> You still need your first level IRQ handlers marked with __exception_irq_entry
> which is defined in the above file.
>

Russell,

I know and it is marked with __exception_irq_entry. The above is in
include/linux/irqchip/orion.h and only used for .init_irq in machine
descriptor later.

The irq handler is never exposed to the board file itself, but set
within orion_init_irq. This approach has been taked by
irqchip/irq-gic.c and irqchip/irq-vic.c rather than adding
.handle_irq to the machine descriptor.

Sebastian

2013-05-02 18:57:27

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thu, May 02, 2013 at 08:54:20PM +0200, Sebastian Hesselbarth wrote:
> On 05/02/13 20:45, Russell King - ARM Linux wrote:
>> On Thu, May 02, 2013 at 08:33:48PM +0200, Sebastian Hesselbarth wrote:
>>> On 05/02/2013 08:25 PM, Sebastian Hesselbarth wrote:
>>>> This patch adds an irqchip driver for the main interrupt controller found
>>>> on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
>>>> Corresponding device tree documentation is also added.
>>>>
>>>> Signed-off-by: Sebastian Hesselbarth<[email protected]>
>>>> ---
>>>> Note: This patch triggers a checkpatch warning for
>>>> WARNING: Avoid CamelCase:<handle_IRQ>
>>>>
>>> [...]
>>>> diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h
>>>> new file mode 100644
>>>> index 0000000..04f7bab
>>>> --- /dev/null
>>>> +++ b/include/linux/irqchip/orion.h
>>>> @@ -0,0 +1,18 @@
>>>> +/*
>>>> + * Marvell Orion SoCs IRQ chip driver header.
>>>> + *
>>>> + * Sebastian Hesselbarth<[email protected]>
>>>> + *
>>>> + * 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.
>>>> + */
>>>> +
>>>> +#ifndef __LINUX_IRQCHIP_ORION_H
>>>> +#define __LINUX_IRQCHIP_ORION_H
>>>> +
>>>> +#include<asm/exception.h>
>>>
>>> First review by myself. The above include is a left-over and
>>> will be removed in a v2.
>>
>> You still need your first level IRQ handlers marked with __exception_irq_entry
>> which is defined in the above file.
>>
>
> Russell,
>
> I know and it is marked with __exception_irq_entry. The above is in
> include/linux/irqchip/orion.h and only used for .init_irq in machine
> descriptor later.
>
> The irq handler is never exposed to the board file itself, but set
> within orion_init_irq. This approach has been taked by
> irqchip/irq-gic.c and irqchip/irq-vic.c rather than adding
> .handle_irq to the machine descriptor.

But I don't find an asm/exception.h include in drivers/irqchip/whateveryour.cfileiscalled

2013-05-02 19:04:38

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/13 20:56, Russell King - ARM Linux wrote:
> On Thu, May 02, 2013 at 08:54:20PM +0200, Sebastian Hesselbarth wrote:
>> On 05/02/13 20:45, Russell King - ARM Linux wrote:
>>> On Thu, May 02, 2013 at 08:33:48PM +0200, Sebastian Hesselbarth wrote:
>>>> On 05/02/2013 08:25 PM, Sebastian Hesselbarth wrote:
>>>>> This patch adds an irqchip driver for the main interrupt controller found
>>>>> on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
>>>>> Corresponding device tree documentation is also added.
>>>>>
>>>>> Signed-off-by: Sebastian Hesselbarth<[email protected]>
>>>>> ---
>>>>> Note: This patch triggers a checkpatch warning for
>>>>> WARNING: Avoid CamelCase:<handle_IRQ>
>>>>>
>>>> [...]
>>>>> diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h
>>>>> new file mode 100644
>>>>> index 0000000..04f7bab
>>>>> --- /dev/null
>>>>> +++ b/include/linux/irqchip/orion.h
>>>>> @@ -0,0 +1,18 @@
>>>>> +/*
>>>>> + * Marvell Orion SoCs IRQ chip driver header.
>>>>> + *
>>>>> + * Sebastian Hesselbarth<[email protected]>
>>>>> + *
>>>>> + * 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.
>>>>> + */
>>>>> +
>>>>> +#ifndef __LINUX_IRQCHIP_ORION_H
>>>>> +#define __LINUX_IRQCHIP_ORION_H
>>>>> +
>>>>> +#include<asm/exception.h>
>>>>
>>>> First review by myself. The above include is a left-over and
>>>> will be removed in a v2.
>>>
>>> You still need your first level IRQ handlers marked with __exception_irq_entry
>>> which is defined in the above file.
>>>
>>
>> Russell,
>>
>> I know and it is marked with __exception_irq_entry. The above is in
>> include/linux/irqchip/orion.h and only used for .init_irq in machine
>> descriptor later.
>>
>> The irq handler is never exposed to the board file itself, but set
>> within orion_init_irq. This approach has been taked by
>> irqchip/irq-gic.c and irqchip/irq-vic.c rather than adding
>> .handle_irq to the machine descriptor.
>
> But I don't find an asm/exception.h include in drivers/irqchip/whateveryour.cfileiscalled
>

Well, that might be because you can magically _see_ compiler warnings
while I have to actually run the compiler ;)

With the review from Jason Gunthorpe things will move anyway and I
expect include/irqchip/orion.h to vanish in v2.

Sebastian

2013-05-02 19:05:45

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/13 20:53, Jason Gunthorpe wrote:
> On Thu, May 02, 2013 at 08:25:04PM +0200, Sebastian Hesselbarth wrote:
>> +
>> +static void __iomem *orion_irq_base[ORION_MAX_IRQREG];
>> +static unsigned int orion_irq_regs;
>> +static struct irq_domain *orion_irq_domain;
>> +
>> +asmlinkage void __exception_irq_entry orion_handle_irq(struct
>> pt_regs *regs)
>
> This can be static?

True, corresponds with the left-over #include in linux/irqchip/orion.h.

>> +static int __init orion_of_init(struct device_node *np,
>> + struct device_node *parent)
>> +{
>> + int n;
>> +
>> + for (n = 0; n < ORION_MAX_IRQREG; n++) {
>> + orion_irq_base[n] = of_iomap(np, n);
>
> Is it possible to also reserve the resources for these registers at
> this point in the boot sequence?

I see what I can do.

>> +static struct of_device_id orion_irq_dt_ids[] __initconst = {
>> + { .compatible = "marvell,orion-mpic", .data = orion_of_init },
>> + { }
>
> Is there a strong reason to change the compatible string? Looks to me
> like either the new driver or the old driver will bind depending on
> what is in the machine description. No need for a new string?

The reason for a new compatible string is, that we will also need an
secondary irq controller for bridge irqs. That could be called
marvell,orion-spic. Dove is again a little bit different than the
others and this will require timer and especially rtc not to share
bridge irqs here. RTC irq is located in PMU regs on Dove instead of
bridge regs.

But I don't have a strong opinion here and we can also reuse
marvell,orion-intc for the irqchip driver.

>> +};
>> +
>> +void __init orion_init_irq(void)
>> +{
>> + of_irq_init(orion_irq_dt_ids);
>> +}
>
> Shouldn't this use the new IRQCHIP_DECLARE mechanism?

I didn't follow irqchip discussion lately, but will catch up.

>
>> diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h
>
>> +extern void orion_init_irq(void);
>
> .. which lets this go away, use the generic irqchip_init instead of
> orion_init_irq.

Same as above.

Thanks for the review,
Sebastian

2013-05-02 19:12:24

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thursday 02 May 2013, Jason Gunthorpe wrote:
> > +static struct of_device_id orion_irq_dt_ids[] __initconst = {
> > + { .compatible = "marvell,orion-mpic", .data = orion_of_init },
> > + { }
>
> Is there a strong reason to change the compatible string? Looks to me
> like either the new driver or the old driver will bind depending on
> what is in the machine description. No need for a new string?
>

The compatible string should change if the binding changes in an
incomptible way, and we should try not to change it unless it's
fundamentally flawed.

Arnd

2013-05-02 19:23:00

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thu, May 02, 2013 at 08:25:04PM +0200, Sebastian Hesselbarth wrote:
> This patch adds an irqchip driver for the main interrupt controller found
> on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
> Corresponding device tree documentation is also added.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
> ---
> Note: This patch triggers a checkpatch warning for
> WARNING: Avoid CamelCase: <handle_IRQ>
>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Rob Landley <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Thomas Petazzoni <[email protected]>
> Cc: Gregory Clement <[email protected]>
> Cc: Ezequiel Garcia <[email protected]>
> Cc: Jean-Francois Moine <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> .../interrupt-controller/marvell,orion-mpic.txt | 22 ++++
> drivers/irqchip/Kconfig | 5 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-orion.c | 129 ++++++++++++++++++++
> include/linux/irqchip/orion.h | 18 +++
> 5 files changed, 175 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,orion-mpic.txt
> create mode 100644 drivers/irqchip/irq-orion.c
> create mode 100644 include/linux/irqchip/orion.h

Sebastian,

Could you please include this patch in the next version of the series
depending on it. I'd like to get an Ack from the irqchip folks for us
to take this patch through mvebu/arm-soc.

This will prevent an external tree dependency, and any resulting merge
conflict will be a trivial add in Kconfig or Makefile.

thx,

Jason.

>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-mpic.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-mpic.txt
> new file mode 100644
> index 0000000..3b303ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-mpic.txt
> @@ -0,0 +1,22 @@
> +Marvell Orion SoC main interrupt controller
> +
> +Required properties:
> +- compatible: shall be "marvell,orion-mpic"
> +- reg: base address(es) of interrupt registers starting with CAUSE register
> +- interrupt-controller: identifies the node as an interrupt controller
> +- #interrupt-cells: number of cells to encode an interrupt source, shall be 1.
> +
> +The interrupt sources map to the corresponding bits in the interrupt
> +registers, i.e.
> +- 0 maps to bit 0 of first base address,
> +- 1 maps to bit 1 of first base address,
> +- 32 maps to bit 0 of second base address, and so on.
> +
> +Example:
> + intc: interrupt-controller {
> + compatible = "marvell,orion-mpic";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + /* Dove has 64 primary interrupts */
> + reg = <0x20200 0x10>, <0x20210 0x10>;
> + };
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index a350969..8da3559 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -2,6 +2,11 @@ config IRQCHIP
> def_bool y
> depends on OF_IRQ
>
> +config IRQCHIP_ORION
> + bool
> + select IRQ_DOMAIN
> + select MULTI_IRQ_HANDLER
> +
> config ARM_GIC
> bool
> select IRQ_DOMAIN
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 98e3b87..8adbd43 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -2,6 +2,7 @@ obj-$(CONFIG_IRQCHIP) += irqchip.o
>
> obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
> obj-$(CONFIG_ARCH_EXYNOS) += exynos-combiner.o
> +obj-$(CONFIG_IRQCHIP_ORION) += irq-orion.o
> obj-$(CONFIG_METAG) += irq-metag-ext.o
> obj-$(CONFIG_METAG_PERFCOUNTER_IRQS) += irq-metag.o
> obj-$(CONFIG_ARCH_SUNXI) += irq-sunxi.o
> diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
> new file mode 100644
> index 0000000..ea02e11
> --- /dev/null
> +++ b/drivers/irqchip/irq-orion.c
> @@ -0,0 +1,129 @@
> +/*
> + * Marvell Orion SoCs IRQ chip driver.
> + *
> + * Sebastian Hesselbarth <[email protected]>
> + *
> + * 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.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip/orion.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <asm/mach/irq.h>
> +
> +/* max number of handled irq register blocks */
> +#define ORION_MAX_IRQREG 2
> +
> +#define ORION_IRQ_CAUSE 0x00
> +#define ORION_IRQ_MASK 0x04
> +#define ORION_IRQ_FIQ_MASK 0x08
> +#define ORION_IRQ_ENDP_MASK 0x0c
> +
> +static void __iomem *orion_irq_base[ORION_MAX_IRQREG];
> +static unsigned int orion_irq_regs;
> +static struct irq_domain *orion_irq_domain;
> +
> +asmlinkage void __exception_irq_entry orion_handle_irq(struct pt_regs *regs)
> +{
> + int n;
> + for (n = 0; n < orion_irq_regs; n++) {
> + u32 hwirq_base = n * 32;
> + u32 stat = readl_relaxed(orion_irq_base[n] + ORION_IRQ_CAUSE) &
> + readl_relaxed(orion_irq_base[n] + ORION_IRQ_MASK);
> + while (stat) {
> + u32 hwirq = ffs(stat) - 1;
> + u32 irq = irq_find_mapping(orion_irq_domain,
> + hwirq_base + hwirq);
> + handle_IRQ(irq, regs);
> + stat &= ~(1 << hwirq);
> + }
> + }
> +}
> +
> +static void orion_irq_mask(struct irq_data *irqd)
> +{
> + unsigned int irq = irqd_to_hwirq(irqd);
> + unsigned int irq_off = irq % 32;
> + int reg = irq / 32;
> + u32 val;
> +
> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
> + writel(val & ~(1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
> +}
> +
> +static void orion_irq_unmask(struct irq_data *irqd)
> +{
> + unsigned int irq = irqd_to_hwirq(irqd);
> + unsigned int irq_off = irq % 32;
> + int reg = irq / 32;
> + u32 val;
> +
> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
> + writel(val | (1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
> +}
> +
> +static struct irq_chip orion_irq_chip = {
> + .name = "orion_irq",
> + .irq_mask = orion_irq_mask,
> + .irq_unmask = orion_irq_unmask,
> +};
> +
> +static int orion_irq_map(struct irq_domain *d, unsigned int virq,
> + irq_hw_number_t hw)
> +{
> + irq_set_chip_and_handler(virq, &orion_irq_chip,
> + handle_level_irq);
> + set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
> +
> + return 0;
> +}
> +
> +static struct irq_domain_ops orion_irq_ops = {
> + .map = orion_irq_map,
> + .xlate = irq_domain_xlate_onecell,
> +};
> +
> +static int __init orion_of_init(struct device_node *np,
> + struct device_node *parent)
> +{
> + int n;
> +
> + for (n = 0; n < ORION_MAX_IRQREG; n++) {
> + orion_irq_base[n] = of_iomap(np, n);
> +
> + if (!orion_irq_base[n])
> + continue;
> +
> + /* mask all interrupts */
> + writel(0, orion_irq_base[n] + ORION_IRQ_MASK);
> + orion_irq_regs++;
> + }
> +
> + /* at least one irq reg must be set */
> + if (!orion_irq_regs)
> + panic("%s: unable to map IRQC registers\n", np->full_name);
> +
> + orion_irq_domain = irq_domain_add_linear(np, orion_irq_regs * 32,
> + &orion_irq_ops, NULL);
> + if (!orion_irq_domain)
> + panic("%s: unable to create IRQ domain\n", np->full_name);
> +
> + set_handle_irq(orion_handle_irq);
> +
> + return 0;
> +}
> +
> +static struct of_device_id orion_irq_dt_ids[] __initconst = {
> + { .compatible = "marvell,orion-mpic", .data = orion_of_init },
> + { }
> +};
> +
> +void __init orion_init_irq(void)
> +{
> + of_irq_init(orion_irq_dt_ids);
> +}
> diff --git a/include/linux/irqchip/orion.h b/include/linux/irqchip/orion.h
> new file mode 100644
> index 0000000..04f7bab
> --- /dev/null
> +++ b/include/linux/irqchip/orion.h
> @@ -0,0 +1,18 @@
> +/*
> + * Marvell Orion SoCs IRQ chip driver header.
> + *
> + * Sebastian Hesselbarth <[email protected]>
> + *
> + * 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.
> + */
> +
> +#ifndef __LINUX_IRQCHIP_ORION_H
> +#define __LINUX_IRQCHIP_ORION_H
> +
> +#include <asm/exception.h>
> +
> +extern void orion_init_irq(void);
> +
> +#endif
> --
> 1.7.2.5
>

2013-05-02 19:34:37

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/2013 09:11 PM, Arnd Bergmann wrote:
> On Thursday 02 May 2013, Jason Gunthorpe wrote:
>>> +static struct of_device_id orion_irq_dt_ids[] __initconst = {
>>> + { .compatible = "marvell,orion-mpic", .data = orion_of_init },
>>> + { }
>>
>> Is there a strong reason to change the compatible string? Looks to me
>> like either the new driver or the old driver will bind depending on
>> what is in the machine description. No need for a new string?
>>
>
> The compatible string should change if the binding changes in an
> incomptible way, and we should try not to change it unless it's
> fundamentally flawed.

Well, there is no _fundamental_ change in the binding syntax as it
is only reg, interrupts, and clocks. But there is a semantic change
in reg properties, as current orion irq controller wants the mask
registers (0x04,0x08) only while this also needs cause register
(0x00).

Nothing that couldn't be handled while moving Orion arch to irqchip
but if there are no further objections, I'd like to stick with the new
compatible string - also having orion-spic in mind.

Sebastian

2013-05-02 19:35:19

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thu, May 02, 2013 at 09:05:38PM +0200, Sebastian Hesselbarth wrote:

> >>+static struct of_device_id orion_irq_dt_ids[] __initconst = {
> >>+ { .compatible = "marvell,orion-mpic", .data = orion_of_init },
> >>+ { }
> >
> >Is there a strong reason to change the compatible string? Looks to me
> >like either the new driver or the old driver will bind depending on
> >what is in the machine description. No need for a new string?
>
> The reason for a new compatible string is, that we will also need an
> secondary irq controller for bridge irqs. That could be called
> marvell,orion-spic. Dove is again a little bit different than the
> others and this will require timer and especially rtc not to share
> bridge irqs here. RTC irq is located in PMU regs on Dove instead of
> bridge regs.

As Arnd mentioned, I would keep the old name then..

The bridge controller can be called marvell,orion-intc-bridge, and if
Dove needs a pmu controller, marvell,dove-intc-pmu ?

> >.. which lets this go away, use the generic irqchip_init instead of
> >orion_init_irq.
>
> Same as above.

I have kirkwood HW but I haven't had time to make newer kernels run on
it, otherwise I'd test it too :(

Jason

2013-05-02 19:37:34

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thu, May 02, 2013 at 09:34:30PM +0200, Sebastian Hesselbarth wrote:

> >The compatible string should change if the binding changes in an
> >incomptible way, and we should try not to change it unless it's
> >fundamentally flawed.
>
> Well, there is no _fundamental_ change in the binding syntax as it
> is only reg, interrupts, and clocks. But there is a semantic change
> in reg properties, as current orion irq controller wants the mask
> registers (0x04,0x08) only while this also needs cause register
> (0x00).

Oh, I didn't notice that, good point - the original binding was flawed
in that regard :|

Jason

2013-05-02 19:39:31

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/2013 09:34 PM, Sebastian Hesselbarth wrote:
> On 05/02/2013 09:11 PM, Arnd Bergmann wrote:
>> On Thursday 02 May 2013, Jason Gunthorpe wrote:
>>>> +static struct of_device_id orion_irq_dt_ids[] __initconst = {
>>>> + { .compatible = "marvell,orion-mpic", .data = orion_of_init },
>>>> + { }
>>>
>>> Is there a strong reason to change the compatible string? Looks to me
>>> like either the new driver or the old driver will bind depending on
>>> what is in the machine description. No need for a new string?
>>>
>>
>> The compatible string should change if the binding changes in an
>> incomptible way, and we should try not to change it unless it's
>> fundamentally flawed.
>
> Well, there is no _fundamental_ change in the binding syntax as it
> is only reg, interrupts, and clocks. But there is a semantic change
> in reg properties, as current orion irq controller wants the mask
> registers (0x04,0x08) only while this also needs cause register
> (0x00).
>
> Nothing that couldn't be handled while moving Orion arch to irqchip
> but if there are no further objections, I'd like to stick with the new
> compatible string - also having orion-spic in mind.

Taking Jason Gunthorpe's last reply, I will stick to the _old_
compatible string and name the bridge irq handler as Jason suggested.

I will send a v2 soon.

Sebastian

2013-05-02 19:48:58

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/2013 09:35 PM, Jason Gunthorpe wrote:
> I have kirkwood HW but I haven't had time to make newer kernels run on
> it, otherwise I'd test it too :(

I also have kirkwood HW but that will cut me from email as I use it as
relay server ;) Maybe I can turn it into a test bed for a while.

There is also Orion5x and Discovery Innovation (mv78xx0) to be tested.

@Jason Cooper: I will merge both irqchip and dove patches into one
patch set. I wasn't earlier because I didn't want the above SoCs to
slow down patch integration. And I will split dtsi changes into
separate patches as requested.

Sebastian

2013-05-02 20:02:29

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thu, May 02, 2013 at 09:48:50PM +0200, Sebastian Hesselbarth wrote:
> On 05/02/2013 09:35 PM, Jason Gunthorpe wrote:
> >I have kirkwood HW but I haven't had time to make newer kernels run on
> >it, otherwise I'd test it too :(
>
> I also have kirkwood HW but that will cut me from email as I use it as
> relay server ;) Maybe I can turn it into a test bed for a while.
>
> There is also Orion5x and Discovery Innovation (mv78xx0) to be tested.

I can test kirkwood and Orion5x.

Andrew

2013-05-02 20:09:03

by Gregory CLEMENT

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/2013 10:02 PM, Andrew Lunn wrote:
> On Thu, May 02, 2013 at 09:48:50PM +0200, Sebastian Hesselbarth wrote:
>> On 05/02/2013 09:35 PM, Jason Gunthorpe wrote:
>>> I have kirkwood HW but I haven't had time to make newer kernels run on
>>> it, otherwise I'd test it too :(
>>
>> I also have kirkwood HW but that will cut me from email as I use it as
>> relay server ;) Maybe I can turn it into a test bed for a while.
>>
>> There is also Orion5x and Discovery Innovation (mv78xx0) to be tested.
>
> I can test kirkwood and Orion5x.

And I can test mv78xx0

Gregory

2013-05-02 21:34:59

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

Sebastian,

please do not take the rant below personally. You just happen to
trigger it.

On Thu, 2 May 2013, Sebastian Hesselbarth wrote:

> +static void orion_irq_mask(struct irq_data *irqd)
> +{
> + unsigned int irq = irqd_to_hwirq(irqd);
> + unsigned int irq_off = irq % 32;
> + int reg = irq / 32;
> + u32 val;
> +
> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
> + writel(val & ~(1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
> +}
> +
> +static void orion_irq_unmask(struct irq_data *irqd)
> +{
> + unsigned int irq = irqd_to_hwirq(irqd);
> + unsigned int irq_off = irq % 32;
> + int reg = irq / 32;
> + u32 val;
> +
> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
> + writel(val | (1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
> +}

I'm really tired of looking at the next incarnation of an OF/DT irq
chip driver, which reimplements stuff which I have consolidated in the
generic irq chip implementation with a lot of effort.

Just look at the various implementations in drivers/irqchip/ and find
out how similar they are. Moving code to drivers/irqchip/ does not
make an excuse for reestablishing the mess which was addressed by the
generic irq chip implementation.

Can you - and that means all of you ARM folks - please get your gear
together and add the missing features to the generic irq chip
implementation? I'm not going to accept more of that OF/DT frenzy.

Thanks,

tglx

2013-05-02 21:56:53

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/02/2013 11:34 PM, Thomas Gleixner wrote:
> please do not take the rant below personally. You just happen to
> trigger it.

Thomas,

it is okay for me - but thanks for the notice! I will comment below.

> On Thu, 2 May 2013, Sebastian Hesselbarth wrote:
>> +static void orion_irq_mask(struct irq_data *irqd)
>> +{
>> + unsigned int irq = irqd_to_hwirq(irqd);
>> + unsigned int irq_off = irq % 32;
>> + int reg = irq / 32;
>> + u32 val;
>> +
>> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
>> + writel(val& ~(1<< irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
>> +}
>> +
>> +static void orion_irq_unmask(struct irq_data *irqd)
>> +{
>> + unsigned int irq = irqd_to_hwirq(irqd);
>> + unsigned int irq_off = irq % 32;
>> + int reg = irq / 32;
>> + u32 val;
>> +
>> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
>> + writel(val | (1<< irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
>> +}
>
> I'm really tired of looking at the next incarnation of an OF/DT irq
> chip driver, which reimplements stuff which I have consolidated in the
> generic irq chip implementation with a lot of effort.

Actually, non-irqchip implementation of orion intc was based on generic
irq chip already. I took a look at drivers/irqchip and realized that
at least sunxi (ARM again) was reimplementing mask/unmask the way
above. So I took the short path and copied that.

> Just look at the various implementations in drivers/irqchip/ and find
> out how similar they are. Moving code to drivers/irqchip/ does not
> make an excuse for reestablishing the mess which was addressed by the
> generic irq chip implementation.
>
> Can you - and that means all of you ARM folks - please get your gear
> together and add the missing features to the generic irq chip
> implementation? I'm not going to accept more of that OF/DT frenzy.

So you are suggesting to have a "linux,generic-intc" or you want me
to have "marvell,orion-intc" make use of generic irq chip again?

The second is easy, the first will take me a while to think about
proper DT properties how to encode mask/unmask/ack/.. availability
and offsets.

Regards,
Sebastian

2013-05-02 22:10:36

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thursday 02 May 2013, Sebastian Hesselbarth wrote:
> > Just look at the various implementations in drivers/irqchip/ and find
> > out how similar they are. Moving code to drivers/irqchip/ does not
> > make an excuse for reestablishing the mess which was addressed by the
> > generic irq chip implementation.
> >
> > Can you - and that means all of you ARM folks - please get your gear
> > together and add the missing features to the generic irq chip
> > implementation? I'm not going to accept more of that OF/DT frenzy.
>
> So you are suggesting to have a "linux,generic-intc" or you want me
> to have "marvell,orion-intc" make use of generic irq chip again?
>
> The second is easy, the first will take me a while to think about
> proper DT properties how to encode mask/unmask/ack/.. availability
> and offsets.

I think it should not be "linux,..." since the description can
well be OS independent. I don't have a good idea for a generic
name, but it's not very important.

The main missing piece in the generic irqchip code at the moment
is support for the linear irqdomain. Once we have that, a lot of
code can be simplified significantly.

Arnd

2013-05-02 22:37:27

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On 05/03/2013 12:09 AM, Arnd Bergmann wrote:
> On Thursday 02 May 2013, Sebastian Hesselbarth wrote:
>>> Just look at the various implementations in drivers/irqchip/ and find
>>> out how similar they are. Moving code to drivers/irqchip/ does not
>>> make an excuse for reestablishing the mess which was addressed by the
>>> generic irq chip implementation.
>>>
>>> Can you - and that means all of you ARM folks - please get your gear
>>> together and add the missing features to the generic irq chip
>>> implementation? I'm not going to accept more of that OF/DT frenzy.
>>
>> So you are suggesting to have a "linux,generic-intc" or you want me
>> to have "marvell,orion-intc" make use of generic irq chip again?
>>
>> The second is easy, the first will take me a while to think about
>> proper DT properties how to encode mask/unmask/ack/.. availability
>> and offsets.
>
> I think it should not be "linux,..." since the description can
> well be OS independent. I don't have a good idea for a generic
> name, but it's not very important.
>
> The main missing piece in the generic irqchip code at the moment
> is support for the linear irqdomain. Once we have that, a lot of
> code can be simplified significantly.

Arnd, Thomas,

I still don't get it completely. Are you requesting a full blown
DT-enabled generic irq chip driver that can be setup by DT properties?

Or is it just to have current generic irq chip play with linear
irqdomain?

If it is just linear irqdomain, then I can have a look at it
now. I would first send v2 to allow the others to integrate
Kirkwood, Orion5x, and Discovery Innovation. Then v3 will
touch generic irq and merge that with orion irqchip.

(@Jason C: Are you sure that I should merge dove and orion
irqchip patches? I doubt that anything touching generic irq
will not go through irq tree.)

Sebastian

2013-05-02 23:48:59

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 0/5] ARM: orion: add orion irqchip driver

This patch set adds an irqchip driver for the main interrupt controller
found on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation)
and the corresponding device tree documentation.

It also moves Dove as the first Orion SoC to this irqchip driver.
As legacy non-DT drivers don't know about virtual irqs provided by irqchip
drivers, init code for timer and mv643xx_eth is added until true DT enabled
drivers are available. Finally, DT enabled boards for Dove are moved to the
orion irqchip driver.

*Note:*
This patch set is _not_ feature complete. There has been a request for
irqdomain support for generic irq and perhaps even a DT enabled generic
irq chip driver.

The patch set still can be used to port other Orion SoCs as I did with Dove.
It also merges irqchip driver patches with Dove patches to use this driver.

Sebastian Hesselbarth (5):
irqchip: add support for Marvell Orion SoCs
ARM: dove: add DT parsing for legacy mv643xx_eth
ARM: dove: add DT parsing for legacy timer
ARM: dove: move DT boards to orion irqchip driver
ARM: dove: add DT nodes for irqchip conversion

.../interrupt-controller/marvell,orion-intc.txt | 22 ++++
arch/arm/boot/dts/dove.dtsi | 16 ++-
arch/arm/mach-dove/Kconfig | 1 +
arch/arm/mach-dove/Makefile | 4 +-
arch/arm/mach-dove/board-dt.c | 77 ++++++++++--
drivers/irqchip/Kconfig | 5 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-orion.c | 133 ++++++++++++++++++++
8 files changed, 247 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,orion-intc.txt
create mode 100644 drivers/irqchip/irq-orion.c
---
Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: Jean-Francois Moine <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
--
1.7.10.4

2013-05-02 23:49:39

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 5/5] ARM: dove: add DT nodes for irqchip conversion

This patch adds required device tree nodes for irqchip conversion
on Dove. The nodes are named and layed out to allow seamless migration
to true DT enabled drivers as soon as they are available.

Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- merge DT changes into this patch (Suggested by Jason Cooper)

Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: Jean-Francois Moine <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/boot/dts/dove.dtsi | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 6cab468..afb719a 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -30,11 +30,18 @@
marvell,tauros2-cache-features = <0>;
};

+ timer: timer {
+ compatible = "marvell,orion-timer";
+ reg = <0x20300 0x20>;
+ interrupts = <0>;
+ clocks = <&core_clk 0>;
+ };
+
intc: interrupt-controller {
compatible = "marvell,orion-intc";
interrupt-controller;
#interrupt-cells = <1>;
- reg = <0x20204 0x04>, <0x20214 0x04>;
+ reg = <0x20200 0x10>, <0x20210 0x10>;
};

core_clk: core-clocks@d0214 {
@@ -202,6 +209,13 @@
status = "disabled";
};

+ gbe: ethernet-controller {
+ compatible = "marvell,mv643xx-eth-block";
+ reg = <0x72000 0x4000>;
+ interrupts = <29>, <30>;
+ status = "disabled";
+ };
+
rtc@d8500 {
compatible = "marvell,orion-rtc";
reg = <0xd8500 0x20>;
--
1.7.10.4

2013-05-02 23:49:37

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 4/5] ARM: dove: move DT boards to orion irqchip driver

With legacy devices mapping their irqs, we can now switch DT enabled
boards to orion irqchip driver.

Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- split off DT changes (Suggested by Jason Cooper)
- use irqchip_init (Suggested by Jason Gunthorpe)

Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: Jean-Francois Moine <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/mach-dove/Kconfig | 1 +
arch/arm/mach-dove/Makefile | 4 ++--
arch/arm/mach-dove/board-dt.c | 4 ++--
3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-dove/Kconfig b/arch/arm/mach-dove/Kconfig
index 36469d8..60d72b4 100644
--- a/arch/arm/mach-dove/Kconfig
+++ b/arch/arm/mach-dove/Kconfig
@@ -22,6 +22,7 @@ config MACH_CM_A510

config MACH_DOVE_DT
bool "Marvell Dove Flattened Device Tree"
+ select IRQCHIP_ORION
select MVEBU_CLK_CORE
select MVEBU_CLK_GATING
select REGULATOR
diff --git a/arch/arm/mach-dove/Makefile b/arch/arm/mach-dove/Makefile
index 3f0a858..4780f78 100644
--- a/arch/arm/mach-dove/Makefile
+++ b/arch/arm/mach-dove/Makefile
@@ -1,5 +1,5 @@
-obj-y += common.o addr-map.o irq.o
-obj-$(CONFIG_DOVE_LEGACY) += mpp.o
+obj-y += common.o addr-map.o
+obj-$(CONFIG_DOVE_LEGACY) += irq.o mpp.o
obj-$(CONFIG_PCI) += pcie.o
obj-$(CONFIG_MACH_DOVE_DB) += dove-db-setup.o
obj-$(CONFIG_MACH_DOVE_DT) += board-dt.o
diff --git a/arch/arm/mach-dove/board-dt.c b/arch/arm/mach-dove/board-dt.c
index cea65b7..404c993 100644
--- a/arch/arm/mach-dove/board-dt.c
+++ b/arch/arm/mach-dove/board-dt.c
@@ -11,6 +11,7 @@
#include <linux/init.h>
#include <linux/clk-provider.h>
#include <linux/clk/mvebu.h>
+#include <linux/irqchip.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
@@ -20,7 +21,6 @@
#include <asm/mach/arch.h>
#include <mach/pm.h>
#include <plat/common.h>
-#include <plat/irq.h>
#include <plat/time.h>
#include "common.h"

@@ -143,7 +143,7 @@ static const char * const dove_dt_board_compat[] = {

DT_MACHINE_START(DOVE_DT, "Marvell Dove (Flattened Device Tree)")
.map_io = dove_map_io,
- .init_irq = orion_dt_init_irq,
+ .init_irq = irqchip_init,
.init_time = dove_legacy_timer_init,
.init_machine = dove_dt_init,
.restart = dove_restart,
--
1.7.10.4

2013-05-02 23:49:35

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 3/5] ARM: dove: add DT parsing for legacy timer

To allow to move to orion irqchip driver, existing legacy devices
have to map their irqs. This patch adds init code to map the
corresponding irqs. It will vanish as soon as there is true device tree
support for orion timer.

Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- split off DT changes (Suggested by Jason Cooper)

Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: Jean-Francois Moine <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/mach-dove/board-dt.c | 42 +++++++++++++++++++++++++++++++++++------
1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-dove/board-dt.c b/arch/arm/mach-dove/board-dt.c
index 9df6dd7..cea65b7 100644
--- a/arch/arm/mach-dove/board-dt.c
+++ b/arch/arm/mach-dove/board-dt.c
@@ -12,6 +12,7 @@
#include <linux/clk-provider.h>
#include <linux/clk/mvebu.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_data/usb-ehci-orion.h>
@@ -20,6 +21,7 @@
#include <mach/pm.h>
#include <plat/common.h>
#include <plat/irq.h>
+#include <plat/time.h>
#include "common.h"

/*
@@ -48,10 +50,42 @@ static void __init dove_legacy_clk_init(void)
of_clk_get_from_provider(&clkspec));
}

-static void __init dove_of_clk_init(void)
+#define BRIDGE_INT_TIMER1_CLR (~0x0004)
+
+static void __init dove_legacy_timer_init(void)
{
+ struct device_node *np = of_find_compatible_node(NULL, NULL,
+ "marvell,orion-timer");
+ struct clk *clk;
+ void __iomem *base;
+ unsigned int tclk, irq;
+
+ /* Setup root of clk tree */
mvebu_clocks_init();
dove_legacy_clk_init();
+
+ if (!np)
+ panic("missing timer node\n");
+
+ base = of_iomap(np, 0);
+ if (!base)
+ panic("%s: missing reg base for timer\n", np->full_name);
+
+ irq = irq_of_parse_and_map(np, 0);
+ if (!irq)
+ panic("%s: missing irq for timer\n", np->full_name);
+
+ clk = of_clk_get(np, 0);
+ if (IS_ERR(clk))
+ panic("%s: missing clock for timer\n", np->full_name);
+
+ clk_prepare_enable(clk);
+ tclk = clk_get_rate(clk);
+ clk_put(clk);
+
+ orion_time_set_base(base);
+ /* legacy timer init gets bridge reg base */
+ orion_time_init(base - 0x300, BRIDGE_INT_TIMER1_CLR, irq, tclk);
}

static struct mv643xx_eth_platform_data dove_dt_ge00_data = {
@@ -95,9 +129,6 @@ static void __init dove_dt_init(void)
#endif
dove_setup_cpu_mbus();

- /* Setup root of clk tree */
- dove_of_clk_init();
-
/* Internal devices not ported to DT yet */
dove_legacy_ge00_init();
dove_pcie_init(1, 1);
@@ -112,9 +143,8 @@ static const char * const dove_dt_board_compat[] = {

DT_MACHINE_START(DOVE_DT, "Marvell Dove (Flattened Device Tree)")
.map_io = dove_map_io,
- .init_early = dove_init_early,
.init_irq = orion_dt_init_irq,
- .init_time = dove_timer_init,
+ .init_time = dove_legacy_timer_init,
.init_machine = dove_dt_init,
.restart = dove_restart,
.dt_compat = dove_dt_board_compat,
--
1.7.10.4

2013-05-02 23:49:32

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 2/5] ARM: dove: add DT parsing for legacy mv643xx_eth

To allow to move to orion irqchip driver, existing legacy devices
have to map their irqs. This patch adds init code to map the
corresponding irqs. It will vanish as soon as there is true device tree
support for mv643xx_eth.

Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Changelog:
v1->v2:
- split off DT changes (Suggested by Jason Cooper)

Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: Jean-Francois Moine <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/mach-dove/board-dt.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-dove/board-dt.c b/arch/arm/mach-dove/board-dt.c
index fbde1dd..9df6dd7 100644
--- a/arch/arm/mach-dove/board-dt.c
+++ b/arch/arm/mach-dove/board-dt.c
@@ -12,6 +12,7 @@
#include <linux/clk-provider.h>
#include <linux/clk/mvebu.h>
#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_data/usb-ehci-orion.h>
#include <asm/hardware/cache-tauros2.h>
@@ -57,6 +58,34 @@ static struct mv643xx_eth_platform_data dove_dt_ge00_data = {
.phy_addr = MV643XX_ETH_PHY_ADDR_DEFAULT,
};

+#define DOVE_GE00_PHYS_BASE 0xf1070000
+
+static void __init dove_legacy_ge00_init(void)
+{
+ struct device_node *np = of_find_compatible_node(NULL, NULL,
+ "marvell,mv643xx-eth-block");
+ int irq_sum, irq_err;
+
+ if (!np)
+ return;
+
+ irq_sum = irq_of_parse_and_map(np, 0);
+ if (!irq_sum) {
+ pr_err("%s: missing sum irq\n", np->full_name);
+ return;
+ }
+
+ irq_err = irq_of_parse_and_map(np, 1);
+ if (!irq_err) {
+ pr_err("%s: missing err irq\n", np->full_name);
+ return;
+ }
+
+ /* legacy ge00_init wants phys base */
+ orion_ge00_init(&dove_dt_ge00_data, DOVE_GE00_PHYS_BASE,
+ irq_sum, irq_err, 1600);
+}
+
static void __init dove_dt_init(void)
{
pr_info("Dove 88AP510 SoC\n");
@@ -70,7 +99,7 @@ static void __init dove_dt_init(void)
dove_of_clk_init();

/* Internal devices not ported to DT yet */
- dove_ge00_init(&dove_dt_ge00_data);
+ dove_legacy_ge00_init();
dove_pcie_init(1, 1);

of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
--
1.7.10.4

2013-05-02 23:49:28

by Sebastian Hesselbarth

[permalink] [raw]
Subject: [PATCH v2 1/5] irqchip: add support for Marvell Orion SoCs

This patch adds an irqchip driver for the main interrupt controller found
on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
Corresponding device tree documentation is also added.

Signed-off-by: Sebastian Hesselbarth <[email protected]>
---
Note: This patch triggers a checkpatch warning for
WARNING: Avoid CamelCase: <handle_IRQ>

Changelog:
v1->v2:
- rename compatible string to "marvell,orion-intc" (Suggested by Jason Gunthorpe)
- request mem regions for irq base registers (Suggested by Jason Gunthorpe)
- make orion_handle_irq static (Suggested by Jason Gunthorpe)
- make use of IRQCHIP_DECLARE (Suggested by Jason Gunthorpe)

Cc: Grant Likely <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Rob Landley <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Russell King <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Gregory Clement <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: Jean-Francois Moine <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
.../interrupt-controller/marvell,orion-intc.txt | 22 ++++
drivers/irqchip/Kconfig | 5 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-orion.c | 133 ++++++++++++++++++++
4 files changed, 161 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/marvell,orion-intc.txt
create mode 100644 drivers/irqchip/irq-orion.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-intc.txt
new file mode 100644
index 0000000..9b7aee9
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/marvell,orion-intc.txt
@@ -0,0 +1,22 @@
+Marvell Orion SoC main interrupt controller
+
+Required properties:
+- compatible: shall be "marvell,orion-intc"
+- reg: base address(es) of interrupt registers starting with CAUSE register
+- interrupt-controller: identifies the node as an interrupt controller
+- #interrupt-cells: number of cells to encode an interrupt source, shall be 1.
+
+The interrupt sources map to the corresponding bits in the interrupt
+registers, i.e.
+- 0 maps to bit 0 of first base address,
+- 1 maps to bit 1 of first base address,
+- 32 maps to bit 0 of second base address, and so on.
+
+Example:
+ intc: interrupt-controller {
+ compatible = "marvell,orion-intc";
+ interrupt-controller;
+ #interrupt-cells = <1>;
+ /* Dove has 64 first level interrupts */
+ reg = <0x20200 0x10>, <0x20210 0x10>;
+ };
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index a350969..8da3559 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -2,6 +2,11 @@ config IRQCHIP
def_bool y
depends on OF_IRQ

+config IRQCHIP_ORION
+ bool
+ select IRQ_DOMAIN
+ select MULTI_IRQ_HANDLER
+
config ARM_GIC
bool
select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 10ef57f..2cad23d 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ARCH_EXYNOS) += exynos-combiner.o
obj-$(CONFIG_ARCH_MXS) += irq-mxs.o
obj-$(CONFIG_METAG) += irq-metag-ext.o
obj-$(CONFIG_METAG_PERFCOUNTER_IRQS) += irq-metag.o
+obj-$(CONFIG_IRQCHIP_ORION) += irq-orion.o
obj-$(CONFIG_ARCH_SUNXI) += irq-sun4i.o
obj-$(CONFIG_ARCH_SPEAR3XX) += spear-shirq.o
obj-$(CONFIG_ARM_GIC) += irq-gic.o
diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
new file mode 100644
index 0000000..21ebe6c
--- /dev/null
+++ b/drivers/irqchip/irq-orion.c
@@ -0,0 +1,133 @@
+/*
+ * Marvell Orion SoCs IRQ chip driver.
+ *
+ * Sebastian Hesselbarth <[email protected]>
+ *
+ * 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.
+ */
+
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <asm/exception.h>
+#include <asm/mach/irq.h>
+
+#include "irqchip.h"
+
+/* max number of handled irq register blocks */
+#define ORION_MAX_IRQREG 2
+
+#define ORION_IRQ_CAUSE 0x00
+#define ORION_IRQ_MASK 0x04
+#define ORION_IRQ_FIQ_MASK 0x08
+#define ORION_IRQ_ENDP_MASK 0x0c
+
+static void __iomem *orion_irq_base[ORION_MAX_IRQREG];
+static unsigned int orion_irq_regs;
+static struct irq_domain *orion_irq_domain;
+
+static asmlinkage void __exception_irq_entry orion_handle_irq(
+ struct pt_regs *regs)
+{
+ int n;
+ for (n = 0; n < orion_irq_regs; n++) {
+ u32 hwirq_base = n * 32;
+ u32 stat = readl_relaxed(orion_irq_base[n] + ORION_IRQ_CAUSE) &
+ readl_relaxed(orion_irq_base[n] + ORION_IRQ_MASK);
+ while (stat) {
+ u32 hwirq = ffs(stat) - 1;
+ u32 irq = irq_find_mapping(orion_irq_domain,
+ hwirq_base + hwirq);
+ handle_IRQ(irq, regs);
+ stat &= ~(1 << hwirq);
+ }
+ }
+}
+
+static void orion_irq_mask(struct irq_data *irqd)
+{
+ unsigned int irq = irqd_to_hwirq(irqd);
+ unsigned int irq_off = irq % 32;
+ int reg = irq / 32;
+ u32 val;
+
+ val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
+ writel(val & ~(1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
+}
+
+static void orion_irq_unmask(struct irq_data *irqd)
+{
+ unsigned int irq = irqd_to_hwirq(irqd);
+ unsigned int irq_off = irq % 32;
+ int reg = irq / 32;
+ u32 val;
+
+ val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
+ writel(val | (1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
+}
+
+static struct irq_chip orion_irq_chip = {
+ .name = "orion_irq",
+ .irq_mask = orion_irq_mask,
+ .irq_unmask = orion_irq_unmask,
+};
+
+static int orion_irq_map(struct irq_domain *d, unsigned int virq,
+ irq_hw_number_t hw)
+{
+ irq_set_chip_and_handler(virq, &orion_irq_chip,
+ handle_level_irq);
+ set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
+
+ return 0;
+}
+
+static struct irq_domain_ops orion_irq_ops = {
+ .map = orion_irq_map,
+ .xlate = irq_domain_xlate_onecell,
+};
+
+static int __init orion_of_init(struct device_node *np,
+ struct device_node *parent)
+{
+ int n;
+
+ for (n = 0; n < ORION_MAX_IRQREG; n++) {
+ struct resource r;
+
+ /* parsing reg property may fail silently here */
+ if (of_address_to_resource(np, n, &r))
+ continue;
+
+ if (!request_mem_region(r.start, resource_size(&r), np->name))
+ panic("%s: unable to request mem region %d",
+ np->full_name, n);
+
+ orion_irq_base[n] = ioremap(r.start, resource_size(&r));
+ if (!orion_irq_base[n])
+ panic("%s: unable to map resource %d",
+ np->full_name, n);
+
+ /* mask all interrupts */
+ writel(0, orion_irq_base[n] + ORION_IRQ_MASK);
+ orion_irq_regs++;
+ }
+
+ /* at least one irq reg must be set */
+ if (!orion_irq_regs)
+ panic("%s: unable to map IRQC registers\n", np->full_name);
+
+ orion_irq_domain = irq_domain_add_linear(np, orion_irq_regs * 32,
+ &orion_irq_ops, NULL);
+ if (!orion_irq_domain)
+ panic("%s: unable to create IRQ domain\n", np->full_name);
+
+ set_handle_irq(orion_handle_irq);
+
+ return 0;
+}
+IRQCHIP_DECLARE(orion_intc, "marvell,orion-intc", orion_of_init);
--
1.7.10.4

2013-05-03 05:07:06

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] ARM: dove: add DT parsing for legacy mv643xx_eth

On Fri, May 03, 2013 at 01:48:36AM +0200, Sebastian Hesselbarth wrote:
> To allow to move to orion irqchip driver, existing legacy devices
> have to map their irqs. This patch adds init code to map the
> corresponding irqs. It will vanish as soon as there is true device tree
> support for mv643xx_eth.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
> ---
> Changelog:
> v1->v2:
> - split off DT changes (Suggested by Jason Cooper)
>
> Cc: Grant Likely <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Rob Landley <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Jason Cooper <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Thomas Petazzoni <[email protected]>
> Cc: Gregory Clement <[email protected]>
> Cc: Ezequiel Garcia <[email protected]>
> Cc: Jean-Francois Moine <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/arm/mach-dove/board-dt.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-dove/board-dt.c b/arch/arm/mach-dove/board-dt.c
> index fbde1dd..9df6dd7 100644
> --- a/arch/arm/mach-dove/board-dt.c
> +++ b/arch/arm/mach-dove/board-dt.c
> @@ -12,6 +12,7 @@
> #include <linux/clk-provider.h>
> #include <linux/clk/mvebu.h>
> #include <linux/of.h>
> +#include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <linux/platform_data/usb-ehci-orion.h>
> #include <asm/hardware/cache-tauros2.h>
> @@ -57,6 +58,34 @@ static struct mv643xx_eth_platform_data dove_dt_ge00_data = {
> .phy_addr = MV643XX_ETH_PHY_ADDR_DEFAULT,
> };
>
> +#define DOVE_GE00_PHYS_BASE 0xf1070000
> +
> +static void __init dove_legacy_ge00_init(void)
> +{
> + struct device_node *np = of_find_compatible_node(NULL, NULL,
> + "marvell,mv643xx-eth-block");
> + int irq_sum, irq_err;
> +
> + if (!np)
> + return;
> +
> + irq_sum = irq_of_parse_and_map(np, 0);
> + if (!irq_sum) {
> + pr_err("%s: missing sum irq\n", np->full_name);
> + return;
> + }
> +
> + irq_err = irq_of_parse_and_map(np, 1);
> + if (!irq_err) {
> + pr_err("%s: missing err irq\n", np->full_name);
> + return;
> + }
> +
> + /* legacy ge00_init wants phys base */
> + orion_ge00_init(&dove_dt_ge00_data, DOVE_GE00_PHYS_BASE,
> + irq_sum, irq_err, 1600);
> +}

Hi Sebastian

I know the above code is throw away, but it might help with getting
Kirkwood, Orion5x, mv78xx00 supported if we refactor this code and
move most of it into plat-orion/common.c. I could imaging a function

orion_ge00_irq_init(struct mv643xx_eth_platform_data *eth_data,
unsigned long mapbase, unsigned int tx_csum_limit)

which does the irq lookup and then calls orion_ge00_init().


Jason: what is the status of the ethernet driver conversion to DT?
Will it get merged this week, or is it material for the next merge
window?

Andrew

2013-05-03 09:58:12

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] ARM: dove: add DT parsing for legacy mv643xx_eth

On 05/03/2013 07:06 AM, Andrew Lunn wrote:
> On Fri, May 03, 2013 at 01:48:36AM +0200, Sebastian Hesselbarth wrote:
>> To allow to move to orion irqchip driver, existing legacy devices
>> have to map their irqs. This patch adds init code to map the
>> corresponding irqs. It will vanish as soon as there is true device tree
>> support for mv643xx_eth.
>>
>> Signed-off-by: Sebastian Hesselbarth<[email protected]>
>> ---
>> Changelog:
>> v1->v2:
>> - split off DT changes (Suggested by Jason Cooper)
>>
>> Cc: Grant Likely<[email protected]>
>> Cc: Rob Herring<[email protected]>
>> Cc: Rob Landley<[email protected]>
>> Cc: Thomas Gleixner<[email protected]>
>> Cc: Russell King<[email protected]>
>> Cc: Arnd Bergmann<[email protected]>
>> Cc: Jason Cooper<[email protected]>
>> Cc: Andrew Lunn<[email protected]>
>> Cc: Jason Gunthorpe<[email protected]>
>> Cc: Thomas Petazzoni<[email protected]>
>> Cc: Gregory Clement<[email protected]>
>> Cc: Ezequiel Garcia<[email protected]>
>> Cc: Jean-Francois Moine<[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> Cc: [email protected]
>> ---
>> arch/arm/mach-dove/board-dt.c | 31 ++++++++++++++++++++++++++++++-
>> 1 file changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-dove/board-dt.c b/arch/arm/mach-dove/board-dt.c
>> index fbde1dd..9df6dd7 100644
>> --- a/arch/arm/mach-dove/board-dt.c
>> +++ b/arch/arm/mach-dove/board-dt.c
>> @@ -12,6 +12,7 @@
>> #include<linux/clk-provider.h>
>> #include<linux/clk/mvebu.h>
>> #include<linux/of.h>
>> +#include<linux/of_irq.h>
>> #include<linux/of_platform.h>
>> #include<linux/platform_data/usb-ehci-orion.h>
>> #include<asm/hardware/cache-tauros2.h>
>> @@ -57,6 +58,34 @@ static struct mv643xx_eth_platform_data dove_dt_ge00_data = {
>> .phy_addr = MV643XX_ETH_PHY_ADDR_DEFAULT,
>> };
>>
>> +#define DOVE_GE00_PHYS_BASE 0xf1070000
>> +
>> +static void __init dove_legacy_ge00_init(void)
>> +{
>> + struct device_node *np = of_find_compatible_node(NULL, NULL,
>> + "marvell,mv643xx-eth-block");
>> + int irq_sum, irq_err;
>> +
>> + if (!np)
>> + return;
>> +
>> + irq_sum = irq_of_parse_and_map(np, 0);
>> + if (!irq_sum) {
>> + pr_err("%s: missing sum irq\n", np->full_name);
>> + return;
>> + }
>> +
>> + irq_err = irq_of_parse_and_map(np, 1);
>> + if (!irq_err) {
>> + pr_err("%s: missing err irq\n", np->full_name);
>> + return;
>> + }
>> +
>> + /* legacy ge00_init wants phys base */
>> + orion_ge00_init(&dove_dt_ge00_data, DOVE_GE00_PHYS_BASE,
>> + irq_sum, irq_err, 1600);
>> +}
>
> Hi Sebastian
>
> I know the above code is throw away, but it might help with getting
> Kirkwood, Orion5x, mv78xx00 supported if we refactor this code and
> move most of it into plat-orion/common.c. I could imaging a function
>
> orion_ge00_irq_init(struct mv643xx_eth_platform_data *eth_data,
> unsigned long mapbase, unsigned int tx_csum_limit)
>
> which does the irq lookup and then calls orion_ge00_init().

Andrew,

sure. Feel free to move legacy code to where all SoCs can use it.

> Jason: what is the status of the ethernet driver conversion to DT?
> Will it get merged this week, or is it material for the next merge
> window?

What I remember from Florian's work it was close to be released. I
never saw Florian sending an update to his patches, maybe he ran out
of time?

I guess DT timer will be a quick patch when we have all SoCs running
on DT irqchip and maybe Florian can update his patches even before we
sort out Thomas request. Then we could also merge DT timer within this
patch set and get rid of legacy helpers before we even introduce them.

Sebastian

2013-05-03 12:56:51

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] irqchip: add support for Marvell Orion SoCs

On Fri, May 03, 2013 at 01:48:35AM +0200, Sebastian Hesselbarth wrote:
> This patch adds an irqchip driver for the main interrupt controller found
> on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
> Corresponding device tree documentation is also added.
>
> Signed-off-by: Sebastian Hesselbarth <[email protected]>
> ---
> Note: This patch triggers a checkpatch warning for
> WARNING: Avoid CamelCase: <handle_IRQ>
>
> Changelog:
> v1->v2:
> - rename compatible string to "marvell,orion-intc" (Suggested by Jason Gunthorpe)
> - request mem regions for irq base registers (Suggested by Jason Gunthorpe)
> - make orion_handle_irq static (Suggested by Jason Gunthorpe)
> - make use of IRQCHIP_DECLARE (Suggested by Jason Gunthorpe)

It would still be a good idea to convert this to use the generic chip
stuff which Thomas created, though exactly how is hard to see at the
moment.

> +static void orion_irq_mask(struct irq_data *irqd)
> +{
> + unsigned int irq = irqd_to_hwirq(irqd);
> + unsigned int irq_off = irq % 32;
> + int reg = irq / 32;
> + u32 val;
> +
> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
> + writel(val & ~(1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
> +}

This could be replaced with irq_gc_mask_clr_bit().

> +
> +static void orion_irq_unmask(struct irq_data *irqd)
> +{
> + unsigned int irq = irqd_to_hwirq(irqd);
> + unsigned int irq_off = irq % 32;
> + int reg = irq / 32;
> + u32 val;
> +
> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
> + writel(val | (1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
> +}

This with irq_gc_mask_set_bit().

> +
> +static struct irq_chip orion_irq_chip = {
> + .name = "orion_irq",
> + .irq_mask = orion_irq_mask,
> + .irq_unmask = orion_irq_unmask,
> +};
> +
> +static int orion_irq_map(struct irq_domain *d, unsigned int virq,
> + irq_hw_number_t hw)
> +{
> + irq_set_chip_and_handler(virq, &orion_irq_chip,
> + handle_level_irq);
> + set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);

This is where it starts to get tricky, because I can't see how you'd
merge the irq_alloc_generic_chip() and irq_setup_generic_chip() with
this. Maybe you don't need to do anything here and just do all that
in orion_of_init() instead? But then you seem to need to know the
virq range before hand, and I can't see how that's known. Maybe Thomas
can provide some enlightenment about how the gc irqchip stuff works
with the irq domain stuff...

However, you wouldn't need the statically defined orion_irq_chip nor
orion_irq_base[] either, because those would be held within the gc
irqchip stuff and stored in the upper layer.

2013-05-03 13:13:26

by Sebastian Hesselbarth

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] irqchip: add support for Marvell Orion SoCs

On 05/03/13 14:55, Russell King - ARM Linux wrote:
> On Fri, May 03, 2013 at 01:48:35AM +0200, Sebastian Hesselbarth wrote:
>> This patch adds an irqchip driver for the main interrupt controller found
>> on Marvell Orion SoCs (Kirkwood, Dove, Orion5x, Discovery Innovation).
>> Corresponding device tree documentation is also added.
>>
>> Signed-off-by: Sebastian Hesselbarth <[email protected]>
>> ---
>> Note: This patch triggers a checkpatch warning for
>> WARNING: Avoid CamelCase: <handle_IRQ>
>>
>> Changelog:
>> v1->v2:
>> - rename compatible string to "marvell,orion-intc" (Suggested by Jason Gunthorpe)
>> - request mem regions for irq base registers (Suggested by Jason Gunthorpe)
>> - make orion_handle_irq static (Suggested by Jason Gunthorpe)
>> - make use of IRQCHIP_DECLARE (Suggested by Jason Gunthorpe)
>
> It would still be a good idea to convert this to use the generic chip
> stuff which Thomas created, though exactly how is hard to see at the
> moment.

Russel,

that is the plan and that's why the whole patch set is preliminary.
Maybe it would have been more precise to call it RFC instead.

>> +static void orion_irq_mask(struct irq_data *irqd)
>> +{
>> + unsigned int irq = irqd_to_hwirq(irqd);
>> + unsigned int irq_off = irq % 32;
>> + int reg = irq / 32;
>> + u32 val;
>> +
>> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
>> + writel(val & ~(1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
>> +}
>
> This could be replaced with irq_gc_mask_clr_bit().
>
>> +
>> +static void orion_irq_unmask(struct irq_data *irqd)
>> +{
>> + unsigned int irq = irqd_to_hwirq(irqd);
>> + unsigned int irq_off = irq % 32;
>> + int reg = irq / 32;
>> + u32 val;
>> +
>> + val = readl(orion_irq_base[reg] + ORION_IRQ_MASK);
>> + writel(val | (1 << irq_off), orion_irq_base[reg] + ORION_IRQ_MASK);
>> +}
>
> This with irq_gc_mask_set_bit().
>
>> +
>> +static struct irq_chip orion_irq_chip = {
>> + .name = "orion_irq",
>> + .irq_mask = orion_irq_mask,
>> + .irq_unmask = orion_irq_unmask,
>> +};
>> +
>> +static int orion_irq_map(struct irq_domain *d, unsigned int virq,
>> + irq_hw_number_t hw)
>> +{
>> + irq_set_chip_and_handler(virq, &orion_irq_chip,
>> + handle_level_irq);
>> + set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
>
> This is where it starts to get tricky, because I can't see how you'd
> merge the irq_alloc_generic_chip() and irq_setup_generic_chip() with
> this. Maybe you don't need to do anything here and just do all that
> in orion_of_init() instead? But then you seem to need to know the
> virq range before hand, and I can't see how that's known. Maybe Thomas
> can provide some enlightenment about how the gc irqchip stuff works
> with the irq domain stuff...

Exactly, and that is what I am looking into right now. But hell, I am
not an expert in linux irq yet. Moreover, I am not even sure if it is
okay to rely on irqdomain or at least irq_data->hw_irq at all.

My current impression is, that generic chip knowns nothing about irq
domains. But my first modification of it was to use irqd_to_hwirq(d)
where ever it uses d->irq instead. This should allow to abstract from
virtual irqs and retain compatibility (_if_ hw_irq is also set on
!CONFIG_IRQ_DOMAIN).

To add more juice: IRQF_VALID and IRQF_PROBE are ARM only flags. I
tried to find out what they are good for, but stopped googl'ing after
a while. (I know you explained that before somewhere)

> However, you wouldn't need the statically defined orion_irq_chip nor
> orion_irq_base[] either, because those would be held within the gc
> irqchip stuff and stored in the upper layer.

Yeah, that would be very nice. But the current limitation to one
register set with max 32 irqs of generic chip would still require to
keep a list of primary generic irq chips to flip through in the
irq_handler.

This also raises the question, how to check if an generic irq chip
flow handler has to be called. Current irq_chip_regs don't know nothing
about a cause/status register. And actually you don't even know if it
is high/low active and how to mask it with the high/low active mask
register or mask_cache.

Sebastian

2013-05-03 14:10:05

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] irqchip: add support for Marvell Orion SoCs

On Fri, 3 May 2013, Sebastian Hesselbarth wrote:
> On 05/03/13 14:55, Russell King - ARM Linux wrote:
> > This is where it starts to get tricky, because I can't see how you'd
> > merge the irq_alloc_generic_chip() and irq_setup_generic_chip() with
> > this. Maybe you don't need to do anything here and just do all that
> > in orion_of_init() instead? But then you seem to need to know the
> > virq range before hand, and I can't see how that's known. Maybe Thomas
> > can provide some enlightenment about how the gc irqchip stuff works
> > with the irq domain stuff...
>
> Exactly, and that is what I am looking into right now. But hell, I am
> not an expert in linux irq yet. Moreover, I am not even sure if it is
> okay to rely on irqdomain or at least irq_data->hw_irq at all.

Here is a solution to that problem.

1) It adds a mask field to irq_data so we dont have to compute the
mask over and over

2) For compability with existing users it populates the mask with
1 << (d->irq - gc->irq_base)

3) It gives you the option to disable that mask setup or let it
generate from d->hwirq

I'm still looking into a way how to proper support the generic chip /
linear domain mapping in the setup path. Will send you a draft patch
to play with later.

Thanks,

tglx


Index: linux-2.6/include/linux/irq.h
===================================================================
--- linux-2.6.orig/include/linux/irq.h
+++ linux-2.6/include/linux/irq.h
@@ -119,6 +119,7 @@ struct irq_domain;

/**
* struct irq_data - per irq and irq chip data passed down to chip functions
+ * @mask: precomputed bitmask for accessing the chip registers
* @irq: interrupt number
* @hwirq: hardware interrupt number, local to the interrupt domain
* @node: node index useful for balancing
@@ -138,6 +139,7 @@ struct irq_domain;
* irq_data.
*/
struct irq_data {
+ u32 mask;
unsigned int irq;
unsigned long hwirq;
unsigned int node;
@@ -700,10 +702,14 @@ struct irq_chip_generic {
* @IRQ_GC_INIT_NESTED_LOCK: Set the lock class of the irqs to nested for
* irq chips which need to call irq_set_wake() on
* the parent irq. Usually GPIO implementations
+ * @IRQ_GC_NO_MASK: Do not calculate irq_data->mask
+ * @IRQ_GC_MASK_FROM_HWIRQ: Calculate irq_data->mask from the hwirq number
*/
enum irq_gc_flags {
IRQ_GC_INIT_MASK_CACHE = 1 << 0,
IRQ_GC_INIT_NESTED_LOCK = 1 << 1,
+ IRQ_GC_NO_MASK = 1 << 2,
+ IRQ_GC_MASK_FROM_HWIRQ = 1 << 4,
};

/* Generic chip callback functions */
Index: linux-2.6/kernel/irq/generic-chip.c
===================================================================
--- linux-2.6.orig/kernel/irq/generic-chip.c
+++ linux-2.6/kernel/irq/generic-chip.c
@@ -39,7 +39,7 @@ void irq_gc_noop(struct irq_data *d)
void irq_gc_mask_disable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = d->mask;

irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable);
@@ -57,7 +57,7 @@ void irq_gc_mask_disable_reg(struct irq_
void irq_gc_mask_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = d->mask;

irq_gc_lock(gc);
gc->mask_cache |= mask;
@@ -75,7 +75,7 @@ void irq_gc_mask_set_bit(struct irq_data
void irq_gc_mask_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = d->mask;

irq_gc_lock(gc);
gc->mask_cache &= ~mask;
@@ -93,7 +93,7 @@ void irq_gc_mask_clr_bit(struct irq_data
void irq_gc_unmask_enable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = d->mask;

irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable);
@@ -108,7 +108,7 @@ void irq_gc_unmask_enable_reg(struct irq
void irq_gc_ack_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = d->mask;

irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
@@ -122,7 +122,7 @@ void irq_gc_ack_set_bit(struct irq_data
void irq_gc_ack_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = ~(1 << (d->irq - gc->irq_base));
+ u32 mask = ~d->mask;

irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
@@ -136,7 +136,7 @@ void irq_gc_ack_clr_bit(struct irq_data
void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = d->mask;

irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->mask);
@@ -151,7 +151,7 @@ void irq_gc_mask_disable_reg_and_ack(str
void irq_gc_eoi(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = d->mask;

irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->eoi);
@@ -169,7 +169,7 @@ void irq_gc_eoi(struct irq_data *d)
int irq_gc_set_wake(struct irq_data *d, unsigned int on)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = d->mask;

if (!(mask & gc->wake_enabled))
return -EINVAL;
@@ -254,6 +254,15 @@ void irq_setup_generic_chip(struct irq_c
if (flags & IRQ_GC_INIT_NESTED_LOCK)
irq_set_lockdep_class(i, &irq_nested_lock_class);

+ if (!(flags & IRQ_GC_NO_MASK)) {
+ struct irq_data *d = irq_get_irq_data(i);
+ u32 mask;
+
+ if (flags & IRQ_GC_MASK_FROM_HWIRQ)
+ d->mask = 1 << (d->hwirq % 32);
+ else
+ d->mask = 1 << (i - gc->irq_base);
+ }
irq_set_chip_and_handler(i, &ct->chip, ct->handler);
irq_set_chip_data(i, gc);
irq_modify_status(i, clr, set);

2013-05-03 21:50:48

by Thomas Gleixner

[permalink] [raw]
Subject: [RFC patch 0/8] genirq: Support for irq domains in generic irq chip

The ongoing device tree support for ARM is creating new irq chip
drivers in drivers/irqchip/ in a frenzy. Quite some of them are
ripping out the generic irq chip implementation from arch/arm/* and
just creating the same mess of duplicated code again, which was
cleaned up with the generic irq chip implementation with a lot of
effort. Sigh!

I already prodded a few people in reviews to tackle that issue with no
outcome. Even more sigh!

Poor Sebastian triggered me into rant mode, but he ad hoc
volunteered to give it a try. YAY!

Though he asked for a bit of kickstart help. So I squeezed out a few
spare cycles and implemented the basics as far as I think that they
should work.

The following series contains the missing bits and pieces including a
somehow forgotten and now slightly modified series from Gerlando
adding support for irq chips which need separate mask caches for
different chip (control flow) types.

At the moment this supports only linear irq domains, but it could be
extended to other types as well if the need arises. Though the ARM
chips are pretty much all about linear domains AFAICT.

It also lacks support for removing an irq domain at the moment, but
that should be rather trivial to fix.

The last patch in the series is a blind conversion of the irq-sun4i
irq chip driver, completely untested and not even compiled. I just
added it for demonstration purposes. As Russell expected, there is a
lot of consolidation potential. The changelog of that patch is:

1 file changed, 29 insertions(+), 71 deletions(-)

The preparing series has

4 files changed, 294 insertions(+), 50 deletions(-)

So for removing 42 lines in a single driver the core grows 244 lines
including header changes and comments. Convert 6 drivers and we are
more than even because we get the benefit of sharing and therefor
exposing the same code to broader testing and utilization.

We have already 11 of those candidates in drivers/irqchips and new
ones are knocking on the door.

There might be even more consolidation potential, but I leave that to the
DT/irq domain experts.


WARNING: It's compile tested only. So if you find bugs you can keep
them and fix them yourself :)


Thanks,

tglx
---
drivers/irqchip/irq-sun4i.c | 100 ++++-----------
include/linux/irq.h | 45 ++++++-
include/linux/irqdomain.h | 12 +
kernel/irq/generic-chip.c | 281 +++++++++++++++++++++++++++++++++++++-------
kernel/irq/irqdomain.c | 6
5 files changed, 323 insertions(+), 121 deletions(-)

2013-05-04 17:59:24

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Thu, May 02, 2013 at 09:48:50PM +0200, Sebastian Hesselbarth wrote:
> On 05/02/2013 09:35 PM, Jason Gunthorpe wrote:
> >I have kirkwood HW but I haven't had time to make newer kernels run on
> >it, otherwise I'd test it too :(
>
> I also have kirkwood HW but that will cut me from email as I use it as
> relay server ;) Maybe I can turn it into a test bed for a while.
>
> There is also Orion5x and Discovery Innovation (mv78xx0) to be tested.
>
> @Jason Cooper: I will merge both irqchip and dove patches into one
> patch set. I wasn't earlier because I didn't want the above SoCs to
> slow down patch integration. And I will split dtsi changes into
> separate patches as requested.

Understood. I'd prefer to keep patches developed together in the same
series, that way, discussions about who is taking what and which patch
depends on what are all in the same thread of the archives.

It's difficult because some series (eg pcie) touch many different areas
and _need_ to be kept together because of the chain of dependencies.
Most other series though, aren't in that situation.

thx,

Jason.

2013-05-04 18:13:05

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH] irqchip: add support for Marvell Orion SoCs

On Fri, May 03, 2013 at 12:37:20AM +0200, Sebastian Hesselbarth wrote:
> (@Jason C: Are you sure that I should merge dove and orion
> irqchip patches? I doubt that anything touching generic irq
> will not go through irq tree.)

Putting them in the same patch series does not imply they have to go
through the same tree. But it *does* allow us to see relationships,
conflicts, etc.

Based on how the finally dependencies work out, we may ask the irq
maintainers for an Ack to take it through arm-soc. This would happen if
we can't remove the dependency between the trees. The resulting
potential merge conflicts weigh into it, and that's where the irqchip
maintainer's Ack get decided.

If the changes to irqchip are just Makefile/Kconfig, then it's easy.
However, if several other files are changed and conflicting, then we let
it go through irqchip and wait one merge window for the board changes
depending on it.

The goal here is to identify and remove branch dependencies within
arm-soc and between arm-soc and other trees. A secondary goal is to
identify high-risk series (risk of being dropped), and keep them
in separate branches from other changes.

thx,

Jason.

2013-05-04 18:30:18

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] ARM: dove: add DT parsing for legacy mv643xx_eth

On Fri, May 03, 2013 at 07:06:32AM +0200, Andrew Lunn wrote:
> Jason: what is the status of the ethernet driver conversion to DT?
> Will it get merged this week, or is it material for the next merge
> window?

You'd have to ask Florian/David Miller. I'm not sure wether David was
able to pull that in for v3.10 or not. He was pretty responsive to all
of the other changes Florian posted.

thx,

Jason.

2013-05-06 09:49:04

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [RFC patch 0/8] genirq: Support for irq domains in generic irq chip

Hello,

On Fri, May 03, 2013 at 09:50:43PM -0000, Thomas Gleixner wrote:
> The ongoing device tree support for ARM is creating new irq chip
> drivers in drivers/irqchip/ in a frenzy. Quite some of them are
> ripping out the generic irq chip implementation from arch/arm/* and
> just creating the same mess of duplicated code again, which was
> cleaned up with the generic irq chip implementation with a lot of
> effort. Sigh!
>
> I already prodded a few people in reviews to tackle that issue with no
> outcome. Even more sigh!
>
> Poor Sebastian triggered me into rant mode, but he ad hoc
> volunteered to give it a try. YAY!
>
> Though he asked for a bit of kickstart help. So I squeezed out a few
> spare cycles and implemented the basics as far as I think that they
> should work.
>
> The following series contains the missing bits and pieces including a
> somehow forgotten and now slightly modified series from Gerlando
> adding support for irq chips which need separate mask caches for
> different chip (control flow) types.
>
> At the moment this supports only linear irq domains, but it could be
> extended to other types as well if the need arises. Though the ARM
> chips are pretty much all about linear domains AFAICT.
Is there a tree/set of patches that have already fixed the issues
pointed out by Russell and Sebastian? I'd like to use it to get forward
with my nvic patch and want to avert double work and merging different
approaches.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2013-05-06 14:30:27

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 0/8] genirq: Support for irq domains in generic irq chip - V2

Changes vs. V1:

- Fixed the generic chip pointer thinko (Sebastian Hesselbarth)

- Proper support for mask cache

- Read mask hardware only for the first map of an generic chip
instance

- sun4i prefix irq functions proper

Thanks,

tglx

2013-05-13 10:57:48

by Gerlando Falauto

[permalink] [raw]
Subject: Re: [patch 0/8] genirq: Support for irq domains in generic irq chip - V2

Hi Thomas,

On 05/06/2013 04:30 PM, Thomas Gleixner wrote:
> Changes vs. V1:
>
> - Fixed the generic chip pointer thinko (Sebastian Hesselbarth)
>
> - Proper support for mask cache
>
> - Read mask hardware only for the first map of an generic chip
> instance
>
> - sun4i prefix irq functions proper
>

[removed all other individual recipients to avoid useless garbage],

thanks for taking care of this. Sorry for the late response, I have been
away for a whole week with no email access.

I understand you are providing generic changes and it would be up to us
to update individual drivers so to take advantage of those semplifications.
Please forgive me if I got this all wrong though.

I've got a few questions then:

1) What happened to your initial proposal of renaming gc->mask_cache to
gc->shared_mask_cache? Does that mean you intend to keep compatibility
so individual drivers can be updated one at a time?

2) What shall we do with such potential patches for individual drivers,
given the dependencies on your work? Wait until yours get merged?

3) [a bit OT] anyone got a clue why I cannot see my previous patch
series on patchwork.kernel.org? Neither can I find from within
http://marc.info/?l=linux-kernel.
I looked up '[PATCH v3b 0/9] refactoring for mask_cache' and could not
find it, only some replies to the thread were shown.
Patches/messages from the same thread are correctly shown on
news.gmane.org under gmane.linux.kernel, though threading is broken on
my reader; they will not be found through the web interface for
gmane.linux.kernel though.
I noticed this was a cross post to [email protected] and
[email protected] (which I guess is something that should not be
done under any circumstances). And all the above works just fine for
-stable as opposed to LKML.
Could this cross-posting be the root cause for this whole mess?

Thanks!
Gerlando

2013-05-13 12:01:49

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/8] genirq: Support for irq domains in generic irq chip - V2

On Mon, 13 May 2013, Gerlando Falauto wrote:
> 1) What happened to your initial proposal of renaming gc->mask_cache to
> gc->shared_mask_cache? Does that mean you intend to keep compatibility so
> individual drivers can be updated one at a time?

Yes.

> 2) What shall we do with such potential patches for individual drivers, given
> the dependencies on your work? Wait until yours get merged?

I'll provide a branch for the SoC folks, which they can pull into
their tree, so we don't have cross tree dependencies.

> Could this cross-posting be the root cause for this whole mess?

No idea, really.

Thanks,

tglx

2013-10-01 15:27:55

by Gerlando Falauto

[permalink] [raw]
Subject: Re: [patch 0/8] genirq: Support for irq domains in generic irq chip - V2

Hi Thomas, Sebastian,

I see these changes made it to 3.11.
AFAICT though, 3.10.9 still has the original bug (the one that got me to
write the patch for handling separate mask registers) and I am bit
confused as to how to integrate that back into 3.10 (or any previous
affected kernels, as they deserve a fix as well!).

The way I understand it, any mainstream patch would be based on this
work, which is not available on previous kernels. And I guess
backporting the whole thing would be overkill.
So I believe the only way to fix it on older kernels would be to write
one (or more) minimal version-specific patch series.
But then I wonder: would that be acceptable material for linux-stable?

Please correct me if I'm totally wrong here. I'm willing to help & test
but I need directions.

Thanks,
Gerlando

On 05/06/2013 04:30 PM, Thomas Gleixner wrote:
> Changes vs. V1:
>
> - Fixed the generic chip pointer thinko (Sebastian Hesselbarth)
>
> - Proper support for mask cache
>
> - Read mask hardware only for the first map of an generic chip
> instance
>
> - sun4i prefix irq functions proper
>
> Thanks,
>
> tglx
>
>